>-----Original Message-----
>From: Syrjala, Ville
>Sent: Tuesday, March 19, 2019 10:29 PM
>To: Lankhorst, Maarten <[email protected]>
>Cc: Shankar, Uma <[email protected]>; [email protected];
>Sharma, Shashank <[email protected]>; Roper, Matthew D
><[email protected]>
>Subject: Re: [RFC v1 3/7] drm/i915: Add Support for Multi Segment Gamma Mode
>
>On Tue, Mar 19, 2019 at 10:46:27AM +0200, Lankhorst, Maarten wrote:
>> tis 2019-03-19 klockan 14:00 +0530 skrev Uma Shankar:
>> > Multi Segment Gamma Mode is added in Gen11+ platforms.
>> > Added a property interface to enable that.
>> >
>> > Signed-off-by: Uma Shankar <[email protected]>
>> > ---
>> > drivers/gpu/drm/i915/i915_drv.h | 1 +
>> > drivers/gpu/drm/i915/intel_color.c | 23 +++++++++++++++++++++++
>> > include/uapi/drm/i915_drm.h | 14 ++++++++++++++
>> > 3 files changed, 38 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> > b/drivers/gpu/drm/i915/i915_drv.h index 02231ae..f20d418 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -1736,6 +1736,7 @@ struct drm_i915_private {
>> > struct drm_property *force_audio_property;
>> >
>> > struct drm_property *gamma_mode_property;
>> > + struct drm_property *multi_segment_gamma_mode_property;
>>
>> Seems to me both properties should be part of drm core?
Sure Maarten, we can move gamma_mode property to drm.
>>
>> > /* hda/i915 audio component */
>> > struct i915_audio_component *audio_component; diff --git
>> > a/drivers/gpu/drm/i915/intel_color.c
>> > b/drivers/gpu/drm/i915/intel_color.c
>> > index 9d43d19..399d63d 100644
>> > --- a/drivers/gpu/drm/i915/intel_color.c
>> > +++ b/drivers/gpu/drm/i915/intel_color.c
>> > @@ -149,6 +149,26 @@ static bool crtc_state_is_legacy_gamma(const
>> > struct intel_crtc_state *crtc_state
>> > drm_object_attach_property(&crtc->base.base, prop, 0); }
>> >
>> > +void
>> > +intel_attach_multi_segment_gamma_mode_property(struct intel_crtc
>> > *crtc)
>> > +{
>> > + struct drm_device *dev = crtc->base.dev;
>> > + struct drm_i915_private *dev_priv = to_i915(dev);
>> > + struct drm_property *prop;
>> > +
>> > + prop = dev_priv->multi_segment_gamma_mode_property;
>> > + if (!prop) {
>> > + prop = drm_property_create(dev, DRM_MODE_PROP_BLOB,
>> > + "Multi-segment Gamma",
>> > 0);
>> > + if (!prop)
>> > + return;
>> > +
>> > + dev_priv->multi_segment_gamma_mode_property = prop;
>> > + }
>> > +
>> > + drm_object_attach_property(&crtc->base.base, prop, 0); }
>> > +
>> > /*
>> > * When using limited range, multiply the matrix given by userspace
>> > by
>> > * the matrix that we would use for the limited range.
>> > @@ -953,4 +973,7 @@ void intel_color_init(struct intel_crtc *crtc)
>> > INTEL_INFO(dev_priv)-
>> > >color.gamma_lut_size);
>> >
>> > intel_attach_gamma_mode_property(crtc);
>> > +
>> > + if (INTEL_GEN(dev_priv) >= 11)
>> > + intel_attach_multi_segment_gamma_mode_property(crtc)
>> > ;
>> > }
>> > diff --git a/include/uapi/drm/i915_drm.h
>> > b/include/uapi/drm/i915_drm.h index aa2d4c7..8f1974e 100644
>> > --- a/include/uapi/drm/i915_drm.h
>> > +++ b/include/uapi/drm/i915_drm.h
>> > @@ -1842,6 +1842,20 @@ struct drm_i915_query_topology_info {
>> > __u8 data[];
>> > };
>> >
>> > +/*
>> > + * Structure for muti segmented gamma lut */ struct
>> > +multi_segment_gamma_lut {
>> > + /* Number of Lut Segments */
>> > + __u8 segment_cnt;
>> > + /* Precison of LUT entries in bits */
>> > + __u8 precision_bits;
>> > + /* Pointer having number of LUT elements in each segment */
>> > + __u32 *segment_lut_cnt_ptr;
>> > + /* Pointer to store exact lut values for each segment */
>> > + __u32 *segment_lut_ptr;
>> > +};
>> >
>> And perhaps a variation of this as description for all gamma mode
>> types.
>
>This is my old idea how to represent fancier LUTs:
>https://github.com/vsyrjala/linux/commit/1aab7625dca77b831e05e32af17904c2130
>0ff95
>https://github.com/vsyrjala/linux/commit/74ffa5d441702c53830f6d71bb687bb0ae5a
>a58f
>
>Each distinct segment of the curve would be just described by a fixed size
>range
>descriptor, and the entire blob would be made up of however many of those are
>needed.
>
>That way we don't even need any multi-segment uapi for setting up the LUT. The
>blob
>for that would just contain as many entries as the LUT needs in that specific
>mode.
Hi Ville,
This also sounds good. What is your suggestion on this. Any recommendation on
how to take this
forward.
Thanks & Regards,
Uma Shankar
>--
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
