From mboxrd@z Thu Jan 1 00:00:00 1970 From: Maxime Ripard Date: Mon, 22 Feb 2021 10:02:23 +0100 Subject: [PATCH v3] video: sunxi_display: Convert to DM_VIDEO In-Reply-To: References: <20210205010748.2646-1-andre.przywara@arm.com> <20210221000735.5c20c847@slackpad.fritz.box> Message-ID: <20210222090223.4uvwdlcyq3lpcyb4@gilmour> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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: