| Message ID | 1556989900-21972-2-git-send-email-swati2.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: adding state checker for gamma lut values | expand |
On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote: > In this patch, a vfunc read_luts() is introduced to create a hw lut > i.e. lut having values read from gamma/degamma registers which will > later be used to compare with sw lut to validate gamma/degamma lut values. > > v3: -Rebase > v4: -Renamed intel_get_color_config to intel_color_get_config [Jani] > -Wrapped get_color_config() [Jani] > v5: -Renamed intel_color_get_config() to intel_color_read_luts() Ville, did you really want this chage? I sure didn't. BR, Jani. > -Renamed get_color_config to read_luts > > Signed-off-by: Swati Sharma <swati2.sharma@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/intel_color.c | 8 ++++++++ > drivers/gpu/drm/i915/intel_color.h | 1 + > 3 files changed, 10 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 1cea98f..1b6d891 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -342,6 +342,7 @@ struct drm_i915_display_funcs { > * involved with the same commit. > */ > void (*load_luts)(const struct intel_crtc_state *crtc_state); > + void (*read_luts)(struct intel_crtc_state *crtc_state); > }; > > #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 962db12..0048d8a 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state) > return dev_priv->display.color_check(crtc_state); > } > > +void intel_color_read_luts(struct intel_crtc_state *crtc_state) > +{ > + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); > + > + if (dev_priv->display.read_luts) > + dev_priv->display.read_luts(crtc_state); > +} > + > static bool need_plane_update(struct intel_plane *plane, > const struct intel_crtc_state *crtc_state) > { > diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h > index b8a3ce6..fc53de9 100644 > --- a/drivers/gpu/drm/i915/intel_color.h > +++ b/drivers/gpu/drm/i915/intel_color.h > @@ -13,5 +13,6 @@ > int intel_color_check(struct intel_crtc_state *crtc_state); > void intel_color_commit(const struct intel_crtc_state *crtc_state); > void intel_color_load_luts(const struct intel_crtc_state *crtc_state); > +void intel_color_read_luts(struct intel_crtc_state *crtc_state); > > #endif /* __INTEL_COLOR_H__ */
On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote: > On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote: > > In this patch, a vfunc read_luts() is introduced to create a hw lut > > i.e. lut having values read from gamma/degamma registers which will > > later be used to compare with sw lut to validate gamma/degamma lut values. > > > > v3: -Rebase > > v4: -Renamed intel_get_color_config to intel_color_get_config [Jani] > > -Wrapped get_color_config() [Jani] > > v5: -Renamed intel_color_get_config() to intel_color_read_luts() > > Ville, did you really want this chage? I sure didn't. I want the low level funcs to be called something like ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc. The name of the vfunc is more of a meh. > > BR, > Jani. > > > > -Renamed get_color_config to read_luts > > > > Signed-off-by: Swati Sharma <swati2.sharma@intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 1 + > > drivers/gpu/drm/i915/intel_color.c | 8 ++++++++ > > drivers/gpu/drm/i915/intel_color.h | 1 + > > 3 files changed, 10 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 1cea98f..1b6d891 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -342,6 +342,7 @@ struct drm_i915_display_funcs { > > * involved with the same commit. > > */ > > void (*load_luts)(const struct intel_crtc_state *crtc_state); > > + void (*read_luts)(struct intel_crtc_state *crtc_state); > > }; > > > > #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > > index 962db12..0048d8a 100644 > > --- a/drivers/gpu/drm/i915/intel_color.c > > +++ b/drivers/gpu/drm/i915/intel_color.c > > @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state) > > return dev_priv->display.color_check(crtc_state); > > } > > > > +void intel_color_read_luts(struct intel_crtc_state *crtc_state) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); > > + > > + if (dev_priv->display.read_luts) > > + dev_priv->display.read_luts(crtc_state); > > +} > > + > > static bool need_plane_update(struct intel_plane *plane, > > const struct intel_crtc_state *crtc_state) > > { > > diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h > > index b8a3ce6..fc53de9 100644 > > --- a/drivers/gpu/drm/i915/intel_color.h > > +++ b/drivers/gpu/drm/i915/intel_color.h > > @@ -13,5 +13,6 @@ > > int intel_color_check(struct intel_crtc_state *crtc_state); > > void intel_color_commit(const struct intel_crtc_state *crtc_state); > > void intel_color_load_luts(const struct intel_crtc_state *crtc_state); > > +void intel_color_read_luts(struct intel_crtc_state *crtc_state); > > > > #endif /* __INTEL_COLOR_H__ */ > > -- > Jani Nikula, Intel Open Source Graphics Center
On Mon, 06 May 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote: >> On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote: >> > In this patch, a vfunc read_luts() is introduced to create a hw lut >> > i.e. lut having values read from gamma/degamma registers which will >> > later be used to compare with sw lut to validate gamma/degamma lut values. >> > >> > v3: -Rebase >> > v4: -Renamed intel_get_color_config to intel_color_get_config [Jani] >> > -Wrapped get_color_config() [Jani] >> > v5: -Renamed intel_color_get_config() to intel_color_read_luts() >> >> Ville, did you really want this chage? I sure didn't. > > I want the low level funcs to be called something like > ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc. > The name of the vfunc is more of a meh. And I don't care about the low level functions so much, I care about the interface which I should emphasize the "get config" stage. :) BR, Jani. > >> >> BR, >> Jani. >> >> >> > -Renamed get_color_config to read_luts >> > >> > Signed-off-by: Swati Sharma <swati2.sharma@intel.com> >> > --- >> > drivers/gpu/drm/i915/i915_drv.h | 1 + >> > drivers/gpu/drm/i915/intel_color.c | 8 ++++++++ >> > drivers/gpu/drm/i915/intel_color.h | 1 + >> > 3 files changed, 10 insertions(+) >> > >> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> > index 1cea98f..1b6d891 100644 >> > --- a/drivers/gpu/drm/i915/i915_drv.h >> > +++ b/drivers/gpu/drm/i915/i915_drv.h >> > @@ -342,6 +342,7 @@ struct drm_i915_display_funcs { >> > * involved with the same commit. >> > */ >> > void (*load_luts)(const struct intel_crtc_state *crtc_state); >> > + void (*read_luts)(struct intel_crtc_state *crtc_state); >> > }; >> > >> > #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) >> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c >> > index 962db12..0048d8a 100644 >> > --- a/drivers/gpu/drm/i915/intel_color.c >> > +++ b/drivers/gpu/drm/i915/intel_color.c >> > @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state) >> > return dev_priv->display.color_check(crtc_state); >> > } >> > >> > +void intel_color_read_luts(struct intel_crtc_state *crtc_state) >> > +{ >> > + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); >> > + >> > + if (dev_priv->display.read_luts) >> > + dev_priv->display.read_luts(crtc_state); >> > +} >> > + >> > static bool need_plane_update(struct intel_plane *plane, >> > const struct intel_crtc_state *crtc_state) >> > { >> > diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h >> > index b8a3ce6..fc53de9 100644 >> > --- a/drivers/gpu/drm/i915/intel_color.h >> > +++ b/drivers/gpu/drm/i915/intel_color.h >> > @@ -13,5 +13,6 @@ >> > int intel_color_check(struct intel_crtc_state *crtc_state); >> > void intel_color_commit(const struct intel_crtc_state *crtc_state); >> > void intel_color_load_luts(const struct intel_crtc_state *crtc_state); >> > +void intel_color_read_luts(struct intel_crtc_state *crtc_state); >> > >> > #endif /* __INTEL_COLOR_H__ */ >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
On 06-May-19 7:11 PM, Jani Nikula wrote: > On Mon, 06 May 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote: >>> On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote: >>>> In this patch, a vfunc read_luts() is introduced to create a hw lut >>>> i.e. lut having values read from gamma/degamma registers which will >>>> later be used to compare with sw lut to validate gamma/degamma lut values. >>>> >>>> v3: -Rebase >>>> v4: -Renamed intel_get_color_config to intel_color_get_config [Jani] >>>> -Wrapped get_color_config() [Jani] >>>> v5: -Renamed intel_color_get_config() to intel_color_read_luts() >>> Ville, did you really want this chage? I sure didn't. >> I want the low level funcs to be called something like >> ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc. >> The name of the vfunc is more of a meh. > And I don't care about the low level functions so much, I care about the > interface which I should emphasize the "get config" stage. :) Sure, will do that.But my major concerns are in patch 11 :/ Please review and guide. Also, on CI tests are getting aborted..don't know why. Locally it's OK. Debugging. > > BR, > Jani. > >>> BR, >>> Jani. >>> >>> >>>> -Renamed get_color_config to read_luts >>>> >>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com> >>>> --- >>>> drivers/gpu/drm/i915/i915_drv.h | 1 + >>>> drivers/gpu/drm/i915/intel_color.c | 8 ++++++++ >>>> drivers/gpu/drm/i915/intel_color.h | 1 + >>>> 3 files changed, 10 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >>>> index 1cea98f..1b6d891 100644 >>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>> @@ -342,6 +342,7 @@ struct drm_i915_display_funcs { >>>> * involved with the same commit. >>>> */ >>>> void (*load_luts)(const struct intel_crtc_state *crtc_state); >>>> + void (*read_luts)(struct intel_crtc_state *crtc_state); >>>> }; >>>> >>>> #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) >>>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c >>>> index 962db12..0048d8a 100644 >>>> --- a/drivers/gpu/drm/i915/intel_color.c >>>> +++ b/drivers/gpu/drm/i915/intel_color.c >>>> @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state) >>>> return dev_priv->display.color_check(crtc_state); >>>> } >>>> >>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state) >>>> +{ >>>> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); >>>> + >>>> + if (dev_priv->display.read_luts) >>>> + dev_priv->display.read_luts(crtc_state); >>>> +} >>>> + >>>> static bool need_plane_update(struct intel_plane *plane, >>>> const struct intel_crtc_state *crtc_state) >>>> { >>>> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h >>>> index b8a3ce6..fc53de9 100644 >>>> --- a/drivers/gpu/drm/i915/intel_color.h >>>> +++ b/drivers/gpu/drm/i915/intel_color.h >>>> @@ -13,5 +13,6 @@ >>>> int intel_color_check(struct intel_crtc_state *crtc_state); >>>> void intel_color_commit(const struct intel_crtc_state *crtc_state); >>>> void intel_color_load_luts(const struct intel_crtc_state *crtc_state); >>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state); >>>> >>>> #endif /* __INTEL_COLOR_H__ */ >>> -- >>> Jani Nikula, Intel Open Source Graphics Center
On Mon, 06 May 2019, "Sharma, Swati2" <swati2.sharma@intel.com> wrote: > On 06-May-19 7:11 PM, Jani Nikula wrote: >> On Mon, 06 May 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >>> On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote: >>>> On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote: >>>>> In this patch, a vfunc read_luts() is introduced to create a hw lut >>>>> i.e. lut having values read from gamma/degamma registers which will >>>>> later be used to compare with sw lut to validate gamma/degamma lut values. >>>>> >>>>> v3: -Rebase >>>>> v4: -Renamed intel_get_color_config to intel_color_get_config [Jani] >>>>> -Wrapped get_color_config() [Jani] >>>>> v5: -Renamed intel_color_get_config() to intel_color_read_luts() >>>> Ville, did you really want this chage? I sure didn't. >>> I want the low level funcs to be called something like >>> ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc. >>> The name of the vfunc is more of a meh. >> And I don't care about the low level functions so much, I care about the >> interface which I should emphasize the "get config" stage. :) > > Sure, will do that.But my major concerns are in patch 11 :/ Please review and guide. > Also, on CI tests are getting aborted..don't know why. Locally it's OK. Debugging. Please move patch 11 right after current patch 2. This was the point in making the vfuncs optional; you can then add platforms one by one and each change is meaningful. Now you add everything but only really enable the checks in one big lump at the end. BR, Jani. > >> >> BR, >> Jani. >> >>>> BR, >>>> Jani. >>>> >>>> >>>>> -Renamed get_color_config to read_luts >>>>> >>>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com> >>>>> --- >>>>> drivers/gpu/drm/i915/i915_drv.h | 1 + >>>>> drivers/gpu/drm/i915/intel_color.c | 8 ++++++++ >>>>> drivers/gpu/drm/i915/intel_color.h | 1 + >>>>> 3 files changed, 10 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >>>>> index 1cea98f..1b6d891 100644 >>>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>>> @@ -342,6 +342,7 @@ struct drm_i915_display_funcs { >>>>> * involved with the same commit. >>>>> */ >>>>> void (*load_luts)(const struct intel_crtc_state *crtc_state); >>>>> + void (*read_luts)(struct intel_crtc_state *crtc_state); >>>>> }; >>>>> >>>>> #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) >>>>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c >>>>> index 962db12..0048d8a 100644 >>>>> --- a/drivers/gpu/drm/i915/intel_color.c >>>>> +++ b/drivers/gpu/drm/i915/intel_color.c >>>>> @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state) >>>>> return dev_priv->display.color_check(crtc_state); >>>>> } >>>>> >>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state) >>>>> +{ >>>>> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); >>>>> + >>>>> + if (dev_priv->display.read_luts) >>>>> + dev_priv->display.read_luts(crtc_state); >>>>> +} >>>>> + >>>>> static bool need_plane_update(struct intel_plane *plane, >>>>> const struct intel_crtc_state *crtc_state) >>>>> { >>>>> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h >>>>> index b8a3ce6..fc53de9 100644 >>>>> --- a/drivers/gpu/drm/i915/intel_color.h >>>>> +++ b/drivers/gpu/drm/i915/intel_color.h >>>>> @@ -13,5 +13,6 @@ >>>>> int intel_color_check(struct intel_crtc_state *crtc_state); >>>>> void intel_color_commit(const struct intel_crtc_state *crtc_state); >>>>> void intel_color_load_luts(const struct intel_crtc_state *crtc_state); >>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state); >>>>> >>>>> #endif /* __INTEL_COLOR_H__ */ >>>> -- >>>> Jani Nikula, Intel Open Source Graphics Center
On Tue, 07 May 2019, Jani Nikula <jani.nikula@intel.com> wrote: > On Mon, 06 May 2019, "Sharma, Swati2" <swati2.sharma@intel.com> wrote: >> On 06-May-19 7:11 PM, Jani Nikula wrote: >>> On Mon, 06 May 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >>>> On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote: >>>>> On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote: >>>>>> In this patch, a vfunc read_luts() is introduced to create a hw lut >>>>>> i.e. lut having values read from gamma/degamma registers which will >>>>>> later be used to compare with sw lut to validate gamma/degamma lut values. >>>>>> >>>>>> v3: -Rebase >>>>>> v4: -Renamed intel_get_color_config to intel_color_get_config [Jani] >>>>>> -Wrapped get_color_config() [Jani] >>>>>> v5: -Renamed intel_color_get_config() to intel_color_read_luts() >>>>> Ville, did you really want this chage? I sure didn't. >>>> I want the low level funcs to be called something like >>>> ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc. >>>> The name of the vfunc is more of a meh. >>> And I don't care about the low level functions so much, I care about the >>> interface which I should emphasize the "get config" stage. :) >> >> Sure, will do that.But my major concerns are in patch 11 :/ Please review and guide. >> Also, on CI tests are getting aborted..don't know why. Locally it's OK. Debugging. > > Please move patch 11 right after current patch 2. This was the point in > making the vfuncs optional; you can then add platforms one by one and > each change is meaningful. Now you add everything but only really enable > the checks in one big lump at the end. Please add a "color" line to intel_dump_pipe_config() with gamma_enable, gamma_mode, csc_enable, cgm_mode (chv) / csc_mode (others). We don't see their values in the compare failure, but they check out so they're equal. My quick guess is gamma is disabled at takeover and the the lut comparison is pointless. So I think we'll need to make the lut check in intel_pipe_config_compare() conditional to gamma actually being enabled. If that still fails, we'll need to add a lut specific error printer in PIPE_CONF_CHECK_COLOR_LUT() that will dump the luts. Similar to pipe_config_infoframe_err() in PIPE_CONF_CHECK_INFOFRAME(). BR, Jani. > > BR, > Jani. > > >> >>> >>> BR, >>> Jani. >>> >>>>> BR, >>>>> Jani. >>>>> >>>>> >>>>>> -Renamed get_color_config to read_luts >>>>>> >>>>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/i915/i915_drv.h | 1 + >>>>>> drivers/gpu/drm/i915/intel_color.c | 8 ++++++++ >>>>>> drivers/gpu/drm/i915/intel_color.h | 1 + >>>>>> 3 files changed, 10 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >>>>>> index 1cea98f..1b6d891 100644 >>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h >>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h >>>>>> @@ -342,6 +342,7 @@ struct drm_i915_display_funcs { >>>>>> * involved with the same commit. >>>>>> */ >>>>>> void (*load_luts)(const struct intel_crtc_state *crtc_state); >>>>>> + void (*read_luts)(struct intel_crtc_state *crtc_state); >>>>>> }; >>>>>> >>>>>> #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) >>>>>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c >>>>>> index 962db12..0048d8a 100644 >>>>>> --- a/drivers/gpu/drm/i915/intel_color.c >>>>>> +++ b/drivers/gpu/drm/i915/intel_color.c >>>>>> @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state) >>>>>> return dev_priv->display.color_check(crtc_state); >>>>>> } >>>>>> >>>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state) >>>>>> +{ >>>>>> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); >>>>>> + >>>>>> + if (dev_priv->display.read_luts) >>>>>> + dev_priv->display.read_luts(crtc_state); >>>>>> +} >>>>>> + >>>>>> static bool need_plane_update(struct intel_plane *plane, >>>>>> const struct intel_crtc_state *crtc_state) >>>>>> { >>>>>> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h >>>>>> index b8a3ce6..fc53de9 100644 >>>>>> --- a/drivers/gpu/drm/i915/intel_color.h >>>>>> +++ b/drivers/gpu/drm/i915/intel_color.h >>>>>> @@ -13,5 +13,6 @@ >>>>>> int intel_color_check(struct intel_crtc_state *crtc_state); >>>>>> void intel_color_commit(const struct intel_crtc_state *crtc_state); >>>>>> void intel_color_load_luts(const struct intel_crtc_state *crtc_state); >>>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state); >>>>>> >>>>>> #endif /* __INTEL_COLOR_H__ */ >>>>> -- >>>>> Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 1cea98f..1b6d891 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -342,6 +342,7 @@ struct drm_i915_display_funcs { * involved with the same commit. */ void (*load_luts)(const struct intel_crtc_state *crtc_state); + void (*read_luts)(struct intel_crtc_state *crtc_state); }; #define CSR_VERSION(major, minor) ((major) << 16 | (minor)) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 962db12..0048d8a 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state) return dev_priv->display.color_check(crtc_state); } +void intel_color_read_luts(struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); + + if (dev_priv->display.read_luts) + dev_priv->display.read_luts(crtc_state); +} + static bool need_plane_update(struct intel_plane *plane, const struct intel_crtc_state *crtc_state) { diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h index b8a3ce6..fc53de9 100644 --- a/drivers/gpu/drm/i915/intel_color.h +++ b/drivers/gpu/drm/i915/intel_color.h @@ -13,5 +13,6 @@ int intel_color_check(struct intel_crtc_state *crtc_state); void intel_color_commit(const struct intel_crtc_state *crtc_state); void intel_color_load_luts(const struct intel_crtc_state *crtc_state); +void intel_color_read_luts(struct intel_crtc_state *crtc_state); #endif /* __INTEL_COLOR_H__ */
In this patch, a vfunc read_luts() is introduced to create a hw lut i.e. lut having values read from gamma/degamma registers which will later be used to compare with sw lut to validate gamma/degamma lut values. v3: -Rebase v4: -Renamed intel_get_color_config to intel_color_get_config [Jani] -Wrapped get_color_config() [Jani] v5: -Renamed intel_color_get_config() to intel_color_read_luts() -Renamed get_color_config to read_luts Signed-off-by: Swati Sharma <swati2.sharma@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/intel_color.c | 8 ++++++++ drivers/gpu/drm/i915/intel_color.h | 1 + 3 files changed, 10 insertions(+)