On Mon, Mar 30, 2020 at 7:44 PM Andrzej Pietrasiewicz <andrzej.p@xxxxxxxxxxxxx> wrote: > > Hi Daniel, > > W dniu 30.03.2020 o 19:01, Daniel Vetter pisze: > > On Wed, Mar 11, 2020 at 3:55 PM Andrzej Pietrasiewicz > > <andrzej.p@xxxxxxxxxxxxx> wrote: > >> > >> The new struct contains afbc-specific data. > > <snip> > > >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > >> index 439656f55c5d..37a3a023c114 100644 > >> --- a/Documentation/gpu/todo.rst > >> +++ b/Documentation/gpu/todo.rst > >> @@ -404,6 +404,21 @@ Contact: Laurent Pinchart, respective driver maintainers > >> > >> Level: Intermediate > >> > >> +Encode cpp properly in malidp > >> +----------------------------- > >> + > >> +cpp (chars per pixel) is not encoded properly in malidp, zero is > >> +used instead. afbc implementation needs bpp or cpp, but if it is > >> +zero it needs to be provided elsewhere, and so the bpp field exists > >> +in struct drm_afbc_framebuffer. > >> + > >> +Properly encode cpp in malidp and remove the bpp field in struct > >> +drm_afbc_framebuffer. > >> + > >> +Contact: malidp maintainers > >> + > >> +Level: Intermediate > > > > Just stumbled over this todo, which is really surprising. Also > > definitely not something I wanted to ack, earlier versions at least > > didn't have this. > > > > Why is this needed? drm_afbc_framebuffer contains a drm_framebuffer, > > which has a pointer to drm_format_info, which we're always setting > > (the core does that for all drivers, both for addfb and addfb2). Why > > is that not good enough to get at cpp for everyone? > > > > Cheers, Daniel > > > > Let me quote James https://patchwork.freedesktop.org/patch/345603/#comment_653081: > > "Seems we can remove this bpp or no need to define it as a pass in argument > for size check, maybe the komeda/malidp get_afbc_bpp() function mislead > you that afbc formats may have vendor specific bpp. > > But the story is: > > for afbc only formats like DRM_FORMAT_YUV420_8BIT/10BIT, we have set > nothing in drm_format_info, neither cpp nor block_size, so both malidp > or komeda introduce a get_bpp(), but actually the two funcs basically > are same. > > So my suggestion is we can temporary use the get_afbc_bpp() in malidp > or komeda. and eventually I think we'd better set the block size > for these formats, then we can defines a common get_bpp() like pitch". Hm iirc we had some good reasons not to set the block size for these, but maybe with these afbc helpers that's changed. We could also compute cpp/bpp in the helper, starting from the format_info/fourcc code, for these special cases. Essentially move the exact computation that komeda/malidp do right now to set afbc->bpp and move it into the helper. Just kinda feels like unfinished work that we still leave this to drivers, that's some very quirky api. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel