[PATCH v3] video: sunxi_display: Convert to DM_VIDEO

Maxime Ripard maxime at cerno.tech
Mon Feb 22 10:02:23 CET 2021


On Sun, Feb 21, 2021 at 09:24:18AM -0700, Simon Glass wrote:
> > > > +static int sunxi_de_bind(struct udevice *dev)
> > > > +{
> > > > +       struct video_uc_plat *plat = dev_get_uclass_plat(dev);
> > > > +
> > > > +       plat->size = LCD_MAX_WIDTH * LCD_MAX_HEIGHT *
> > > > +                    (1 << LCD_MAX_LOG2_BPP) / 8;
> > >
> > > Should use enum video_log2_bpp here. Also see VNBYTES().
> >
> > LCD_MAX_LOG2_BPP is defined as VIDEO_BPP32 at the very top. Sure will
> > use VNBYTES.
> >
> > On a related topic: IIUC this is called several times, for a start once
> > before relocation, where it's supposed to give an upper bound?
> > Are subsequent calls then expected to be more precise? Our 32MB frame
> > buffer is *very* generous, for the usual FullHD display we just need
> > 8MB. But we would only know this in probe(), when we have learned the
> > output device and the video modes it supports.
> > So is there a way to restrict (and possibly also move) the framebuffer
> > in probe()?
> 
> I suppose you can, but that is not what is expected. You don't save
> memory for U-Boot (and it doesn't matter), but making it smaller so
> that linux uses less would be worthwhile. We just need to make sure
> this is documented in video.h and tested.
> 
> To be clear, you have my review thg, so these are things that can be done later.

Seems Andre seems to be motivated to rework that driver, I guess we
could also rework how this is done.

Ideally, we should be describing the reserved buffer through a
reserved-memory node in the DT instead of carving out some memory for
Linux. That way, if we don't have simplefb support, or it's replaced by
something else eventually, we have a chance a claiming that memory back
at some point, instead of just ignoring it.

Maxime
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210222/bf7f06fc/attachment.sig>


More information about the U-Boot mailing list