linux-kernel - Re: [PATCH v3 3/6] drm: lcdif: Determine bus format and flags in ->atomic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 14 Feb 2023 16:16:41 +0200
From:   Ville Syrjälä <ville.syrjala@...ux.intel.com>
To:     Alexander Stein <alexander.stein@...tq-group.com>
Cc:     dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        Liu Ying <victor.liu@....com>, marex@...x.de,
        shawnguo@...nel.org, s.hauer@...gutronix.de,
        krzysztof.kozlowski@...aro.org, robh+dt@...nel.org,
        linux-imx@....com, krzysztof.kozlowski+dt@...aro.org,
        kernel@...gutronix.de, LW@...o-electronics.de
Subject: Re: [PATCH v3 3/6] drm: lcdif: Determine bus format and flags in
 ->atomic_check()

On Tue, Feb 14, 2023 at 03:12:55PM +0100, Alexander Stein wrote:
> Hi Liu,
> 
> thanks for the update.
> 
> Am Montag, 13. Februar 2023, 09:56:09 CET schrieb Liu Ying:
> > Instead of determining LCDIF output bus format and bus flags in
> > ->atomic_enable(), do that in ->atomic_check().  This is a
> > preparation for the upcoming patch to check consistent bus format
> > and bus flags across all first downstream bridges in ->atomic_check().
> > New lcdif_crtc_state structure is introduced to cache bus format
> > and bus flags states in ->atomic_check() so that they can be read
> > in ->atomic_enable().
> > 
> > Signed-off-by: Liu Ying <victor.liu@....com>
> > ---
> > v2->v3:
> > * No change.
> > 
> > v1->v2:
> > * Split from patch 2/2 in v1. (Marek, Alexander)
> > * Add comment on the 'base' member of lcdif_crtc_state structure to
> >   note it should always be the first member. (Lothar)
> > 
> >  drivers/gpu/drm/mxsfb/lcdif_kms.c | 138 ++++++++++++++++++++++--------
> >  1 file changed, 100 insertions(+), 38 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> > b/drivers/gpu/drm/mxsfb/lcdif_kms.c index e54200a9fcb9..294cecdf5439 100644
> > --- a/drivers/gpu/drm/mxsfb/lcdif_kms.c
> > +++ b/drivers/gpu/drm/mxsfb/lcdif_kms.c
> > @@ -30,6 +30,18 @@
> >  #include "lcdif_drv.h"
> >  #include "lcdif_regs.h"
> > 
> > +struct lcdif_crtc_state {
> > +	struct drm_crtc_state	base;	/* always be the first 
> member */
> 
> Is it strictly necessary that base is the first member? to_lcdif_crtc_state() 
> should be able to handle any position within the struct. I mean it's sensible 
> to put it in first place. But the comment somehow bugs me.

NULL pointers is where these things go bad if it't not at
offset 0. Unless you're willing to always handle those
explicitly.

-- 
Ville Syrjälä
Intel