Hi Emil, On Sun, May 31, 2020 at 05:54:04PM +0100, Emil Velikov wrote: > HI Laurent, > > From a quick glance the series looks really good and neat. Thank you :-) > Then again, I don't know much about the hardware to provide meaningful > review. > > A couple of small ideas below. > > On Sat, 30 May 2020 at 04:11, Laurent Pinchart wrote: > > > +#define LCDC_AS_BUF 0x220 > > +#define LCDC_AS_NEXT_BUF 0x230 > > s/LCDC_AS_BUF/LCDC_AS_CUR_BUF/ - to stay consistent with the primary > plane name scheme. The register names come directly from the datasheet (and yes, the datasheet uses CUR_BUF and NEXT_BUF for the primary plane, and BUF and NEXT_BUF for the overlay plane :-S). I'd thus rather keep them aligned with the documentation. > Would it make sense to store the above registers in mxsfb_devdata, > just like we do for the primary planes? The reason the register addresses are stored in mxsfb_devdata for the primary plane is because they differ between hardware revisions (don't they teach hardware engineers in school these days *not* to move registers around ? :-)). The overlay plane is only supported in the latest versions of the IP core, and are always located at the same address as far as I can tell. I don't think we need an extra level of indirection. -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
