Re: [PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Liviu Dudau <Liviu.Dudau@arm.com>, Jyri Sarha <jsarha@ti.com>,
	dri-devel <dri-devel@lists.freedesktop.org>,
	Tomi Valkeinen <tomi.valkeinen@ti.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Subject: Re: [PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane
Date: Fri, 5 May 2017 20:35:43 +0300	[thread overview]
Message-ID: <20170505173543.GJ12629@intel.com> (raw)
In-Reply-To: <CAKMK7uFsYr7aMG14nLz=dv8cY1G8nQKieOOac2=4ojmo1Gg=Gw@mail.gmail.com>

On Fri, May 05, 2017 at 07:17:43PM +0200, Daniel Vetter wrote:
> On Fri, May 5, 2017 at 4:36 PM, Laurent Pinchart
> <laurent.pinchart@ideasonboard.com> wrote:
> > I'm a bit concerned about this. The YCBCR_ENCODING property specifies how the
> > content of the framebuffer is encoded. If I understand correctly, you're
> > proposing adding an enumeration value that tells the driver not to try to be
> > clever and multiply the CTM matrix by the CSC matrix corresponding to the
> > encoding. That would probably work in most cases, but it would combine two
> > pieces of information in a single property. The driver would then lose the
> > knowledge of how the plane framebuffer is encoded. Couldn't there be cases
> > where that knowledge is needed for other purposes than picking the right CSC
> > matrix ? If so, it might be better to always set the YCBCR_ENCODING property
> > to the actual encoding, and have another property to tell the driver to skip
> > multiplication by the CSC matrix. Or could that be conveyed through the CTM
> > blob property ? Some kind of flag that would essentially tell that the CTM
> > matrix has been pre-multiplied already ?
> >
> > Before I forget, how do you plan to handle backward compatibility with
> > userspace that won't set the YCBCR_ENCODING property ? Is that done by picking
> > a driver-specific default value ? Do you think there would be a need for
> > drivers to know that the property has not been set ?
> 
> Where do we need this? Afaik the encoding is to spec the yuv2rbg
> conversion function, and that's it. But I'm fairly ignorant about
> video and yuv and all these things. so does this specify something
> else? If not, I don't see any possibilities that someone could
> retrofit more meaning onto these conversion rules.

The question, I believe, is how do we deal with existing
userspace that doesn't know about this knob.

I think BT.709 is probably what we should use as the default
since I'm expecting that would match most non-ancient source
material out there. i915 currently uses BT.601 for no
particular good reason, but I'm very happy to change that
to BT.709. I think for the old video overlay (which isn't
exposed as a plane currently) we actually default to BT.709.

The only problatic case is when the hardware can't to BT.709
but I'm not sure if that's relevant for any currently supported
hardware. I guess we could have the core pick the default based
on some priority list eg. BT.709->BT.601->BT.2020.

The other option of course being that we just let each driver
pick their own default. I guess that might make sense if there's
some userspace already out there that expects eg. BT.601. But
frankly the difference between 601 and 709 is small enough that
I wouldn't expect anyone to scream too loudly if we end up
changing the default for someone.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel