Re: [PATCH 1/7] drm/msm/dpu: enable PINGPONG TE operations only when supported by HW

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023-07-30 02:18:10, Dmitry Baryshkov wrote:
> On 29/07/2023 21:31, Marijn Suijten wrote:
> > On 2023-07-29 02:59:32, Dmitry Baryshkov wrote:
> >> On 27/07/2023 23:03, Marijn Suijten wrote:
> >>> On 2023-07-27 19:20:58, Dmitry Baryshkov wrote:
> >>>> The DPU_PINGPONG_TE bit is set for all PINGPONG blocks on DPU < 5.0.
> >>>> Rather than checking for the flag, check for the presense of the
> >>>> corresponding interrupt line.
> >>>>
> >>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> >>>
> >>> That's a smart use of the interrupt field.  I both like it, and I do
> >>> not.  While we didn't do any validation for consistency previously, this
> >>> means we now have multiple ways of controlling available "features":
> >>>
> >>> - Feature flags on hardware blocks;
> >>> - Presence of certain IRQs;
> >>> - DPU core revision.
> >>
> >> I hesitated here too. For INTF it is clear that there is no other good
> >> way to check for the TE feature. For PP we can just check for the DPU
> >> revision.
> > 
> > For both we could additionally check the DPU revision, and for INTF we
> > could check for TYPE_DSI.  Both would aid in extra validation, if we
> > require the IRQ to be present or absent under these conditions.
> 
> Yep, maybe that's better.
> 
> > 
> > It might also help to document this, either here and on the respective
> > struct fields so that catalog implementers know when they should supply
> > or leave out an IRQ?
> 
> Probably a WARN_ON would be enough.

SGTM, it is after all only for bring-up as after that (additions have
been validated, reviewed and merged) we "trust the kernel" including our
catalog, so errors like this should pretty much be unreachable.

- Marijn



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux