Re: [PATCH 2/3] drm: lcdif: Use dedicated set/clr registers for polarity/edge

[Lucas Stach]

Am Dienstag, dem 31.03.2026 um 17:17 +0200 schrieb Paul Kocialkowski:
> Hi Lucas,
> 
> Le Tue 31 Mar 26, 11:09, Lucas Stach a écrit :
> > Hi Paul,
> > 
> > Am Dienstag, dem 31.03.2026 um 00:46 +0200 schrieb Paul Kocialkowski:
> > > The lcdif v3 hardware comes with dedicated registers to set and clear
> > > polarity bits in the CTRL register. It is unclear if there is a
> > > difference with writing to the CTRL register directly.
> > > 
> > > Follow the NXP BSP reference by using these registers, in case there is
> > > a subtle difference caused by using them.
> > > 
> > I don't really like that patch, as it blows up what is currently a
> > single register access to three separate ones. If there is no clear
> > benefit (as in it has been shown to fix any issue), I would prefer this
> > code to stay as-is.
> 
> Well I guess the cost of a few writes vs a single one is rather
> negligible.
> 
Yea, a few writes don't really hurt. But I don't think there is a very
good reason to set this register this way, see below.

> I'm rather worried that there might be an undocumented
> reason why these registers exist in the first place and why they are
> used in the BSP.
> 
> But yes this is only speculation and I could not witness any actual
> issue. My setup (lcdif3 with hdmi) uses all positive polarities which is
> the default state, so not a good way to check.
> 
> It would be great if somebody from NXP could confirm whether this is
> needed or not. In the meantime I guess we can drop the patch. It'll stay
> on the list in case someone has polarity issues later :)

The separate clr/set registers are a rather common design feat found on
Freescale/NXP IP blocks from the MXS era. On some of those IP blocks
_all_ registers are presented as a base/clr/set triplet in the
registers space. As far as I can tell they are mostly useful when you
want to set/clear individual bits from a register without having to
remember or executing a readback of the current state.

In cases like the one changed in this patch, where the full register
state is set in one go, directly writing to the base register is the
right thing to do.

Regards,
Lucas