Hi Tomi,
On Thu, Aug 06, 2020 at 12:21:39PM +0300, Tomi Valkeinen wrote:
> On 06/08/2020 05:18, Laurent Pinchart wrote:
> > The BO pitches are unconditionally set to the frame buffer pitch, for
> > all planes. This is correct for semiplanar YUV formats, as they
> > subsample chroma horizontally by two but combined U and V in a single
> > plane, cancelling each other. For fully planar YUV formats, however, the
> > horizontal subsampling need to be taken into account to compute the
> > pitch. Fix it.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> > ---
> > kms++/src/dumbframebuffer.cpp | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/kms++/src/dumbframebuffer.cpp b/kms++/src/dumbframebuffer.cpp
> > index 18f3f152943d..4c3c03164a90 100644
> > --- a/kms++/src/dumbframebuffer.cpp
> > +++ b/kms++/src/dumbframebuffer.cpp
> > @@ -42,6 +42,14 @@ DumbFramebuffer::DumbFramebuffer(Card& card, uint32_t width, uint32_t height, Pi
> > struct drm_mode_create_dumb creq = drm_mode_create_dumb();
> > creq.width = width;
> > creq.height = height / pi.ysub;
> > + /*
> > + * For fully planar YUV buffers, the chroma planes don't combine
> > + * U and V components, their width must thus be divided by the
> > + * horizontal subsampling factor.
> > + */
> > + if (format_info.type == PixelColorType::YUV &&
> > + format_info.num_planes == 3)
> > + creq.width /= pi.xsub;
>
> This feels a bit of a hack... I think we should somehow have the
> relevant information in the PixelFormatInfo. Having the same plane
> info, { 8, 2, 2 }, for both NV12 UV plane and YUV420 U and V planes
> doesn't sound correct.
>
> Should NV12's UV plane be { 16, 2, 2 }? Subsampled formats are
> confusing... =)
I'll give it a try. I however wonder if all drivers will expect 16bpp.
The ones based on drm_gem_cma_dumb_create() will be fine, but other
drivers may not expect this.
--
Regards,
Laurent Pinchart
