On Fri, Mar 22, 2019 at 04:02:57PM +0200, Ville Syrjälä wrote: > 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.shan...@intel.com> > > > >Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten > > > ><maarten.lankho...@intel.com>; Syrjala, Ville <ville.syrj...@intel.com>; > > > >Sharma, > > > >Shashank <shashank.sha...@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.shan...@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.?
Yep, precisely. In the future, maybe multiple fixed LUTs to choose from. > > 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. Yeah that (ENUM | BLOB) API let's us be quite "rich" with the interface, so it certainly could fit in there. If I understand the code correctly, each value in the enum list is the ID of a blob, and userspace can query that blob to figure out what the entry means (instead of needing _really really long_ descriptive strings). I wonder if that property type in general is a little _too_ rich. I worry that it would be easy to just defer loads of vendor-specific detail into the blob, making the API look "generic" on the surface, when really it's effectively a list of vendor-defined (void *)s. ...that said, in some cases vendor-defined (void *)s is what we might want (e.g. scaler coefficient tables). Still looks like a neat idea, perhaps just needs some policy. Thanks, -Brian > > -- > Ville Syrjälä > Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
