[Intel-gfx] [PATCH v5 7/7] drm: create hdmi output property

[Daniel Vetter]

On Fri, Jun 23, 2017 at 03:35:30PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 6/23/2017 2:50 PM, Daniel Vetter wrote:
> > On Thu, Jun 22, 2017 at 10:33 AM, Sharma, Shashank
> > <shashank.sharma at intel.com> wrote:
> > > > - The property values should be limited to what the driver can support, I
> > > >     guess that would mean limiting the available ycbcr modes? Or does all
> > > >     our hw support all the modes, including 420 (on the sink side)?
> > > This property is targeted at DRM layer, so naturally its for all the HWs
> > > along with Intel HW, so it serves a big range.
> > > All our HDMI 1.4 sources support RGB444, and after this series, can support
> > > YCBCR444.
> > > All HDMI 2.0 sources should support YCBCR420, but they can declare this
> > > using a bool variable which I added in patch 3 (ycbcr420_allowed)
> > > As we are targeting both HDMI 1.4 as well as HDMI 2.0 (Src and Sink), as a
> > > whole we are covering all options.
> > Yes, we need to define values for everything, since it's a generic
> > property. But for a given driver imo we should only allow the values
> > that are actually supported. An example would be the rotation
> > property, which supporsts X/Y-mirror and rotation by 90° steps. But on
> > a given i915 platform we only register support for the stuff the
> > driver/hw can do, e.g. pre-gen9 do not register 90/270° rotation. I
> > think we should do the same here. See
> > drm_plane_create_rotation_property(), specifically the
> > supported_rotations parameter.
> Ah, I got your point now, and its valid.
> If you see the I915 level handlers of YCBCR functions (added in patch 10)
> they are taking care of blocking
> anything which is not supported by driver for this platform, based on:
> - The source capability
> - The sink capability
> - User preference of the property.

Yeah, runtime checks are needed on top (sometimes at least). But if we
can't support a given mode for a given sink then we should take that into
account when registering the property. Otherwise userspace tries stuff
that can never succeed, which isn't a big problem with the TEST_ONLY
atomic mode, just a bit silly.
-Daniel

> So on a whole, set of Intel platforms cover all the values of property, but
> at driver level we make sure to allow only what is suitable for this source
> + sink combination.
> - Shashank
> > 
> > Cheers, Daniel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch