| Message ID | 1546933053-10731-2-git-send-email-uma.shankar@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for Gen 11 pipe color features | expand |
On Tue, Jan 08, 2019 at 01:07:28PM +0530, Uma Shankar wrote: > Check if de-gamma/gamma lut callback is assigned before > calling the same. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> Is it possible for this test to fail? intel_color_init() seems to always assign a value (even for platforms that don't actually support color management). It seem like if you're going to make this change, you'd also want to update intel_color_init() to only set the load_luts for platforms where we actually have color management? Matt > --- > drivers/gpu/drm/i915/intel_color.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 37fd9dd..4ff4db6 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -602,7 +602,8 @@ void intel_color_load_luts(struct intel_crtc_state *crtc_state) > struct drm_device *dev = crtc_state->base.crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > > - dev_priv->display.load_luts(crtc_state); > + if (dev_priv->display.load_luts) > + dev_priv->display.load_luts(crtc_state); > } > > int intel_color_check(struct intel_crtc_state *crtc_state) > -- > 1.9.1 >
>-----Original Message----- >From: Roper, Matthew D >Sent: Friday, January 11, 2019 3:57 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: [v5 1/6] drm/i915: Check for Null for color lut callbacks > >On Tue, Jan 08, 2019 at 01:07:28PM +0530, Uma Shankar wrote: >> Check if de-gamma/gamma lut callback is assigned before calling the >> same. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> > >Is it possible for this test to fail? intel_color_init() seems to always assign a value >(even for platforms that don't actually support color management). > >It seem like if you're going to make this change, you'd also want to update >intel_color_init() to only set the load_luts for platforms where we actually have >color management? Yeah, I was trying to add this check to avoid any processing if call-backs are not registered. Currently that is not the case so can be dropped and added later if such a case arise. Touching and disabling it for legacy platforms, requires to get details from GEN2 onwards and can break older platforms if not done right. Will drop this change. Regards, Uma Shankar > >Matt > >> --- >> drivers/gpu/drm/i915/intel_color.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 37fd9dd..4ff4db6 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -602,7 +602,8 @@ void intel_color_load_luts(struct intel_crtc_state >*crtc_state) >> struct drm_device *dev = crtc_state->base.crtc->dev; >> struct drm_i915_private *dev_priv = to_i915(dev); >> >> - dev_priv->display.load_luts(crtc_state); >> + if (dev_priv->display.load_luts) >> + dev_priv->display.load_luts(crtc_state); >> } >> >> int intel_color_check(struct intel_crtc_state *crtc_state) >> -- >> 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 37fd9dd..4ff4db6 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -602,7 +602,8 @@ void intel_color_load_luts(struct intel_crtc_state *crtc_state) struct drm_device *dev = crtc_state->base.crtc->dev; struct drm_i915_private *dev_priv = to_i915(dev); - dev_priv->display.load_luts(crtc_state); + if (dev_priv->display.load_luts) + dev_priv->display.load_luts(crtc_state); } int intel_color_check(struct intel_crtc_state *crtc_state)
Check if de-gamma/gamma lut callback is assigned before calling the same. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/intel_color.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)