[PATCH v2 4/7] drm: mxsfb: Move mxsfb_get_fb_paddr() away from register IO functions

[Lucas Stach]

Am Montag, dem 11.04.2022 um 00:17 +0200 schrieb Marek Vasut:
> On 4/7/22 11:47, Lucas Stach wrote:
> > Am Donnerstag, dem 07.04.2022 um 00:05 +0200 schrieb Marek Vasut:
> > > On 4/6/22 21:45, Lucas Stach wrote:
> > > > Am Freitag, dem 11.03.2022 um 18:05 +0100 schrieb Marek Vasut:
> > > > > Move mxsfb_get_fb_paddr() out of the way, away from register IO functions.
> > > > > This is a clean up. No functional change.
> > > > > 
> > > > > Signed-off-by: Marek Vasut <marex at denx.de>
> > > > > Cc: Alexander Stein <alexander.stein at ew.tq-group.com>
> > > > > Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > > > Cc: Lucas Stach <l.stach at pengutronix.de>
> > > > > Cc: Peng Fan <peng.fan at nxp.com>
> > > > > Cc: Robby Cai <robby.cai at nxp.com>
> > > > > Cc: Sam Ravnborg <sam at ravnborg.org>
> > > > > Cc: Stefan Agner <stefan at agner.ch>
> > > > 
> > > > Hm, I don't see any real benefit, but I also fail to see why it
> > > > shouldn't be done so:
> > > 
> > > The entire point of this series is to clean up the mxsfb and isolate
> > > lcdif (the original lcdif) from any of the common code.
> > 
> > Actually, just use drm_fb_cma_get_gem_addr() instead?
> 
> That function seems to add only extra code that is executed, 
> 
Yep, and thus it is the correct way to do it, as it actually takes into
account the FB offset parameter. Currently mxsfb seems to just do the
wrong thing when the FB is not at offset 0 in the GEM object.

> but does not do away with the !fb check anyway. 
> 
And that one seems bogus. If you have no FB there is no way you can
reasonably start scanout or flip to the next buffer. What would you
scan out in that case? Random memory? FB should never be NULL in those
code paths.

> So, why ? (Also, seems unrelated to this patch)

Because you aim to clean up the driver and make the code reusable, so
why not use the reusable and correct DRM helper?

Regards,
Lucas