| Message ID | 1552984218-3357-8-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Multi Segment Gamma Support | expand |
On Tue, Mar 19, 2019 at 02:00:18PM +0530, Uma Shankar wrote: > Added support for ICL platform multi segment gamma > capabilties and attached the property, exposing the > same to userspace. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/intel_color.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 7733c256..1e9f784 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -1011,6 +1011,8 @@ int intel_color_check(struct intel_crtc_state *crtc_state) > void intel_color_init(struct intel_crtc *crtc) > { > struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + struct multi_segment_gamma_lut multi_segment_lut; > + > > drm_mode_crtc_set_gamma_size(&crtc->base, 256); > > @@ -1049,6 +1051,24 @@ void intel_color_init(struct intel_crtc *crtc) > > intel_attach_gamma_mode_property(crtc); > > - if (INTEL_GEN(dev_priv) >= 11) > + if (IS_ICELAKE(dev_priv)) { Is it intentional to limit this specifically to Icelake? This is basically the opposite of the type of generalization that we've been moving toward with patches like commit 2dd24a9c2c8d767a976da37d59680f09b9d111ab Author: Rodrigo Vivi <rodrigo.vivi@intel.com> Date: Fri Mar 8 13:42:58 2019 -0800 drm/i915/gen11+: First assume next platforms will inherit stuff Also, we already support another gen11 platform (or will once the mailing list patches land) in the form of EHL; is there any reason that this shouldn't apply to it? > + multi_segment_lut.segment_cnt = 3; > + multi_segment_lut.precision_bits = 16; > + multi_segment_lut.segment_lut_cnt_ptr = kzalloc(3 * sizeof(int), > + GFP_KERNEL); > + if (!multi_segment_lut.segment_lut_cnt_ptr) > + return; > + multi_segment_lut.segment_lut_cnt_ptr[0] = 9; > + multi_segment_lut.segment_lut_cnt_ptr[1] = 256; > + multi_segment_lut.segment_lut_cnt_ptr[2] = 256; On this patch and the previous one we have a few magic numbers representing how the segmented gamma is laid out. Can we move stuff like this into the device info structure and then just setup the appropriate userspace properties on any platform that has the device info fields filled in (i.e., the same way we do the basic color management properties)? Matt > + > intel_attach_multi_segment_gamma_mode_property(crtc); > + > + drm_property_replace_global_blob(crtc->base.dev, > + &crtc->config->multi_segment_gamma_lut, > + sizeof(struct multi_segment_gamma_lut), > + &multi_segment_lut, > + &crtc->base.base, > + dev_priv->multi_segment_gamma_mode_property); > + } > } > -- > 1.9.1 >
>-----Original Message----- >From: Roper, Matthew D >Sent: Friday, March 22, 2019 3:34 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 7/7] drm/i915: Add multi segment gamma for icl > >On Tue, Mar 19, 2019 at 02:00:18PM +0530, Uma Shankar wrote: >> Added support for ICL platform multi segment gamma capabilties and >> attached the property, exposing the same to userspace. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/intel_color.c | 22 +++++++++++++++++++++- >> 1 file changed, 21 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 7733c256..1e9f784 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -1011,6 +1011,8 @@ int intel_color_check(struct intel_crtc_state >> *crtc_state) void intel_color_init(struct intel_crtc *crtc) { >> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); >> + struct multi_segment_gamma_lut multi_segment_lut; >> + >> >> drm_mode_crtc_set_gamma_size(&crtc->base, 256); >> >> @@ -1049,6 +1051,24 @@ void intel_color_init(struct intel_crtc *crtc) >> >> intel_attach_gamma_mode_property(crtc); >> >> - if (INTEL_GEN(dev_priv) >= 11) >> + if (IS_ICELAKE(dev_priv)) { > >Is it intentional to limit this specifically to Icelake? This is basically the opposite of the >type of generalization that we've been moving toward with patches like > > commit 2dd24a9c2c8d767a976da37d59680f09b9d111ab > Author: Rodrigo Vivi <rodrigo.vivi@intel.com> > Date: Fri Mar 8 13:42:58 2019 -0800 > > drm/i915/gen11+: First assume next platforms will inherit stuff > >Also, we already support another gen11 platform (or will once the mailing list patches >land) in the form of EHL; is there any reason that this shouldn't apply to it? Yeah, it will/can-be re-used for future platforms with no/some slight changes. I will modify this to align to our overall feature enabling plans in driver. > >> + multi_segment_lut.segment_cnt = 3; >> + multi_segment_lut.precision_bits = 16; >> + multi_segment_lut.segment_lut_cnt_ptr = kzalloc(3 * sizeof(int), >> + GFP_KERNEL); >> + if (!multi_segment_lut.segment_lut_cnt_ptr) >> + return; >> + multi_segment_lut.segment_lut_cnt_ptr[0] = 9; >> + multi_segment_lut.segment_lut_cnt_ptr[1] = 256; >> + multi_segment_lut.segment_lut_cnt_ptr[2] = 256; > >On this patch and the previous one we have a few magic numbers representing how >the segmented gamma is laid out. Can we move stuff like this into the device info >structure and then just setup the appropriate userspace properties on any platform >that has the device info fields filled in (i.e., the same way we do the basic color >management properties)? Sure, I can move it to platform device info structures and make it transparent here. Regards, Uma Shankar > >Matt > >> + >> intel_attach_multi_segment_gamma_mode_property(crtc); >> + >> + drm_property_replace_global_blob(crtc->base.dev, >> + &crtc->config- >>multi_segment_gamma_lut, >> + sizeof(struct >multi_segment_gamma_lut), >> + &multi_segment_lut, >> + &crtc->base.base, >> + dev_priv- >>multi_segment_gamma_mode_property); >> + } >> } >> -- >> 1.9.1 >> > >-- >Matt Roper >Graphics Software Engineer >IoTG Platform Enabling & Development >Intel Corporation >(916) 356-2795
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 7733c256..1e9f784 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -1011,6 +1011,8 @@ int intel_color_check(struct intel_crtc_state *crtc_state) void intel_color_init(struct intel_crtc *crtc) { struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + struct multi_segment_gamma_lut multi_segment_lut; + drm_mode_crtc_set_gamma_size(&crtc->base, 256); @@ -1049,6 +1051,24 @@ void intel_color_init(struct intel_crtc *crtc) intel_attach_gamma_mode_property(crtc); - if (INTEL_GEN(dev_priv) >= 11) + if (IS_ICELAKE(dev_priv)) { + multi_segment_lut.segment_cnt = 3; + multi_segment_lut.precision_bits = 16; + multi_segment_lut.segment_lut_cnt_ptr = kzalloc(3 * sizeof(int), + GFP_KERNEL); + if (!multi_segment_lut.segment_lut_cnt_ptr) + return; + multi_segment_lut.segment_lut_cnt_ptr[0] = 9; + multi_segment_lut.segment_lut_cnt_ptr[1] = 256; + multi_segment_lut.segment_lut_cnt_ptr[2] = 256; + intel_attach_multi_segment_gamma_mode_property(crtc); + + drm_property_replace_global_blob(crtc->base.dev, + &crtc->config->multi_segment_gamma_lut, + sizeof(struct multi_segment_gamma_lut), + &multi_segment_lut, + &crtc->base.base, + dev_priv->multi_segment_gamma_mode_property); + } }
Added support for ICL platform multi segment gamma capabilties and attached the property, exposing the same to userspace. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/intel_color.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-)