[lkml]   [2022]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [v10] drm/msm/disp/dpu1: add support for dspp sub block flush in sc7280
On 2022-12-12 11:35:15, Kalyan Thota wrote:
> [..]
> >> + if (ctx->pending_dspp_flush_mask[dspp - DSPP_0])
> >> + DPU_REG_WRITE(&ctx->hw, CTL_DSPP_n_FLUSH(dspp - DSPP_0),
> >> + ctx->pending_dspp_flush_mask[dspp -
> >> + DSPP_0]);
> >
> >Shouldn't this loop as a whole check if _any_ DSPP flush is requested via
> >`pending_flush_mask & BIT(29)`? The other flushes don't check the per-block
> >mask value either (and could write zero that way) but only base this check on the
> >presence of a global flush mask for that block.
> >
> BIT(29) enables dspp flush only from DPU rev 7.x.x where hierarchal flush is introduced. For other targets that supports CTL_ACTIVE, it's a NOP.

The only way this patch ever writes pending_dspp_flush_mask is followed
by unconditionally setting BIT(29) in pending_flush_mask. I was under
the assumption that pending_dspp_flush_mask should be considered invalid
or irrelevant unless BIT(29) is set.

> With the check "pending_flush_mask & BIT(29)", unintended DSPP registers for that CTL path will be programmed to "0" which is not correct IMO.

You can also keep the second `if` to guard against that; as said the
code above does exactly this though, but I think we could assume that
if a pending sub-block flush is set, pending_dspp_flush_mask is nonzero?

> Secondly "pending_flush_mask & BIT(29)" although will not be true for DPU 6.x.x versions but can be confusing w.r.t code readability.
> Let me know your thoughts.

Ack, it is /super/ confusing that BIT(29) is used for DSPP (sub-block)
flush, but also to flash INTF_2??

In fact there are many overlapping flush bits used for different
components. Only few are clarified via a #define. Can you confirm
whether this is correct? And whether these should all be pulled out
into numerically-sorted defines to improve readability and document
intentional overlap?

- Marijn

 \ /
  Last update: 2022-12-14 22:05    [W:0.078 / U:4.276 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site