Re: [RFC v1 1/7] drm/i915: Add gamma mode property

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Brian Starkey <Brian.Starkey@arm.com>
Cc: nd <nd@arm.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"Syrjala, Ville" <ville.syrjala@intel.com>,
	"Lankhorst, Maarten" <maarten.lankhorst@intel.com>
Subject: Re: [RFC v1 1/7] drm/i915: Add gamma mode property
Date: Fri, 22 Mar 2019 16:02:57 +0200	[thread overview]
Message-ID: <20190322140257.GT3888@intel.com> (raw)
In-Reply-To: <20190322133903.fjbxnozdxscjtgoc@DESKTOP-E1NTVVP.localdomain>

On Fri, Mar 22, 2019 at 01:39:04PM +0000, Brian Starkey wrote:
> Hi,
> 
> On Fri, Mar 22, 2019 at 01:06:01PM +0000, Shankar, Uma wrote:
> > 
> > 
> > >-----Original Message-----
> > >From: Roper, Matthew D
> > >Sent: Friday, March 22, 2019 2:50 AM
> > >To: Shankar, Uma <uma.shankar@intel.com>
> > >Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten
> > ><maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Sharma,
> > >Shashank <shashank.sharma@intel.com>
> > >Subject: Re: [RFC v1 1/7] drm/i915: Add gamma mode property
> > >
> > >On Tue, Mar 19, 2019 at 02:00:12PM +0530, Uma Shankar wrote:
> > >> Gen platforms support multiple gamma modes, currently it's hard coded
> > >> to operate only in 1 specific mode.
> > >> This patch adds a property to make gamma mode programmable.
> > >> User can select which mode should be used for a particular usecase or
> > >> scenario.
> > >>
> > >> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > >
> > >I haven't reviewed the series in depth, but I'm a bit confused on how userspace would
> > >approach using this property.  This seems to be exposing hardware implementation
> > >details that I wouldn't expect userspace to need to worry about (plus I don't think any
> > >of the property values here convey any specific meaning to someone who hasn't read
> > >the Intel bspec/PRM).  E.g., why does userspace care about "split gamma" when the
> > >driver takes care of the programming details and userspace never sees the actual way
> > >the registers are laid out and written?
> > >My understanding is that what really matters is how many table entries there are for
> > >userspace to fill in, what input range(s) they cover, and how the bits of each table
> > >entry are interpreted.  I think we'd want to handle this in a vendor-agnostic way in the
> > >DRM core if possible; most of the display servers that get used these days are cross-
> > >platform and probably won't want to add Intel-specific logic (or platform-specific
> > >logic if we wind up with a different set of options on future Intel platforms).
> > 
> > Ok, will try to re-structure this to make it vendor agnostic way. Also will add enough
> > documentation for the usage of the property. 
> > 
> > @Ville- What do you recommend or suggest for these interfaces.
> 
> Just to add to the conversation - some of our HW has fixed LUTs, which
> isn't really very well exposed by the current UAPI. We do it by having
> the kernel driver just look at the userspace-provided blob, and it if
> matches the fixed curve we accept it.

So you just have say "Use the sRGB curve" bit in some register etc.?

I think we should be able to integrate that somehow into my design:
https://github.com/vsyrjala/linux/commit/1aab7625dca77b831e05e32af17904c21300ff95                                       

Eg. just add a new DRM_MODE_LUT_FIXED_CURVE flag, and then I suppose
just add another member into the struct to indicate which curve it
is. And we could embed the fixed blob ID in there as well. That way
userspace wouldn't even have to actually get the blob for the curve.

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx