>-----Original Message----- >From: Roper, Matthew D >Sent: Friday, March 22, 2019 3:34 AM >To: Shankar, Uma <uma.shankar@xxxxxxxxx> >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Lankhorst, Maarten ><maarten.lankhorst@xxxxxxxxx>; Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Sharma, >Shashank <shashank.sharma@xxxxxxxxx> >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@xxxxxxxxx> >> --- >> 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@xxxxxxxxx> > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx