| Message ID | 1546933053-10731-5-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:31PM +0530, Uma Shankar wrote: > Add support for icl pipe degamma and gamma. > > v2: Removed a POSTING_READ and corrected the Bit > Definition as per Maarten's comments. > > v3: Addressed Matt's review comments. Removed rmw patterns > as suggested by Matt. > > v4: Fixed Matt's review comments. > > v5: Corrected macro alignment as per Jani Nikula's comments. > Addressed Ville and Matt's review comments. > > Signed-off-by: Uma Shankar <uma.shankar@intel.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 12 ++++--- > drivers/gpu/drm/i915/intel_color.c | 65 ++++++++++++++++++++++++++++++++++++++ > 2 files changed, 72 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 44958d9..f29eef7 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7088,11 +7088,13 @@ enum { > #define _GAMMA_MODE_A 0x4a480 > #define _GAMMA_MODE_B 0x4ac80 > #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B) > -#define GAMMA_MODE_MODE_MASK (3 << 0) > -#define GAMMA_MODE_MODE_8BIT (0 << 0) > -#define GAMMA_MODE_MODE_10BIT (1 << 0) > -#define GAMMA_MODE_MODE_12BIT (2 << 0) > -#define GAMMA_MODE_MODE_SPLIT (3 << 0) > +#define PRE_CSC_GAMMA_ENABLE (1 << 31) > +#define POST_CSC_GAMMA_ENABLE (1 << 30) > +#define GAMMA_MODE_MODE_MASK (3 << 0) > +#define GAMMA_MODE_MODE_8BIT (0 << 0) > +#define GAMMA_MODE_MODE_10BIT (1 << 0) > +#define GAMMA_MODE_MODE_12BIT (2 << 0) > +#define GAMMA_MODE_MODE_SPLIT (3 << 0) > > /* DMC/CSR */ > #define CSR_PROGRAM(i) _MMIO(0x80000 + (i) * 4) > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 9a72e64..9cd4646 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -520,6 +520,69 @@ static void glk_load_luts(struct intel_crtc_state *crtc_state) > POSTING_READ(GAMMA_MODE(pipe)); > } > > +static void icl_load_degamma_lut(struct intel_crtc_state *crtc_state) As Ville noted, I think the degamma LUT works the same way on GLK-ICL; the gamma part may be different (and there's extra stuff like the extra output CSC on ICL), but for degamma specifically I think the code you wrote below could just be used to replace the current body of glk_load_degamma_lut rather than adding it as a separate function. Since GLK-ICL only support equal r/g/b values, we also need to land the LUT validation patches I wrote in December. Those are fully reviewed so I'll do that as soon as I get an ack from Dave/Daniel to merge the drm core patch through the Intel tree. Matt > +{ > + struct drm_device *dev = crtc_state->base.crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe; > + const uint32_t lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size; > + uint32_t i; > + > + /* > + * When setting the auto-increment bit, the hardware seems to > + * ignore the index bits, so we need to reset it to index 0 > + * separately. > + */ > + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0); > + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT); > + > + if (crtc_state->base.degamma_lut) { > + struct drm_color_lut *lut = crtc_state->base.degamma_lut->data; > + > + for (i = 0; i < lut_size; i++) { > + /* > + * First 33 entries represent range from 0 to 1.0 > + * 34th and 35th entry will represent extended range > + * inputs 3.0 and 7.0 respectively, currently clamped > + * at 1.0. Since the precision is 16bit, the user value > + * can be directly filled to register. > + * ToDo: Extend to max 7.0. > + */ > + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), lut[i].green); > + } > + } else { > + /* load a linear table. */ > + for (i = 0; i < lut_size; i++) { > + uint32_t v = (i * (1 << 16)) / (lut_size - 1); > + > + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v); > + } > + } > + > + /* Clamp values > 1.0. */ > + while (i++ < 35) > + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16)); > +} > + > +static void icl_load_luts(struct intel_crtc_state *crtc_state) > +{ > + struct drm_crtc *crtc = crtc_state->base.crtc; > + struct drm_device *dev = crtc_state->base.crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + enum pipe pipe = to_intel_crtc(crtc)->pipe; > + > + if (crtc_state_is_legacy_gamma(crtc_state)) { > + haswell_load_luts(crtc_state); > + return; > + } > + > + icl_load_degamma_lut(crtc_state); > + bdw_load_gamma_lut(crtc_state, 0); > + > + I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT | > + PRE_CSC_GAMMA_ENABLE | POST_CSC_GAMMA_ENABLE); > +} > + > /* Loads the palette/gamma unit for the CRTC on CherryView. */ > static void cherryview_load_luts(struct intel_crtc_state *crtc_state) > { > @@ -636,6 +699,8 @@ void intel_color_init(struct intel_crtc *crtc) > } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) { > dev_priv->display.load_csc_matrix = ilk_load_csc_matrix; > dev_priv->display.load_luts = glk_load_luts; > + } else if (IS_ICELAKE(dev_priv)) { > + dev_priv->display.load_luts = icl_load_luts; > } else { > dev_priv->display.load_luts = i9xx_load_luts; > } > -- > 1.9.1 >
>-----Original Message----- >From: Roper, Matthew D >Sent: Saturday, January 12, 2019 3:49 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 4/6] drm/i915/icl: Add icl pipe degamma and gamma support > >On Tue, Jan 08, 2019 at 01:07:31PM +0530, Uma Shankar wrote: >> Add support for icl pipe degamma and gamma. >> >> v2: Removed a POSTING_READ and corrected the Bit Definition as per >> Maarten's comments. >> >> v3: Addressed Matt's review comments. Removed rmw patterns as >> suggested by Matt. >> >> v4: Fixed Matt's review comments. >> >> v5: Corrected macro alignment as per Jani Nikula's comments. >> Addressed Ville and Matt's review comments. >> >> Signed-off-by: Uma Shankar <uma.shankar@intel.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 12 ++++--- >> drivers/gpu/drm/i915/intel_color.c | 65 >> ++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 72 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h >> b/drivers/gpu/drm/i915/i915_reg.h index 44958d9..f29eef7 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -7088,11 +7088,13 @@ enum { >> #define _GAMMA_MODE_A 0x4a480 >> #define _GAMMA_MODE_B 0x4ac80 >> #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A, >_GAMMA_MODE_B) >> -#define GAMMA_MODE_MODE_MASK (3 << 0) >> -#define GAMMA_MODE_MODE_8BIT (0 << 0) >> -#define GAMMA_MODE_MODE_10BIT (1 << 0) >> -#define GAMMA_MODE_MODE_12BIT (2 << 0) >> -#define GAMMA_MODE_MODE_SPLIT (3 << 0) >> +#define PRE_CSC_GAMMA_ENABLE (1 << 31) >> +#define POST_CSC_GAMMA_ENABLE (1 << 30) >> +#define GAMMA_MODE_MODE_MASK (3 << 0) >> +#define GAMMA_MODE_MODE_8BIT (0 << 0) >> +#define GAMMA_MODE_MODE_10BIT (1 << 0) >> +#define GAMMA_MODE_MODE_12BIT (2 << 0) >> +#define GAMMA_MODE_MODE_SPLIT (3 << 0) >> >> /* DMC/CSR */ >> #define CSR_PROGRAM(i) _MMIO(0x80000 + (i) * 4) >> diff --git a/drivers/gpu/drm/i915/intel_color.c >> b/drivers/gpu/drm/i915/intel_color.c >> index 9a72e64..9cd4646 100644 >> --- a/drivers/gpu/drm/i915/intel_color.c >> +++ b/drivers/gpu/drm/i915/intel_color.c >> @@ -520,6 +520,69 @@ static void glk_load_luts(struct intel_crtc_state >*crtc_state) >> POSTING_READ(GAMMA_MODE(pipe)); >> } >> >> +static void icl_load_degamma_lut(struct intel_crtc_state *crtc_state) > >As Ville noted, I think the degamma LUT works the same way on GLK-ICL; the >gamma part may be different (and there's extra stuff like the extra output CSC on >ICL), but for degamma specifically I think the code you wrote below could just be >used to replace the current body of glk_load_degamma_lut rather than adding it >as a separate function. Ok, will merge that to one function and extend GLK to handle this instead of the current pass through. >Since GLK-ICL only support equal r/g/b values, we also need to land the LUT >validation patches I wrote in December. Those are fully reviewed so I'll do that as >soon as I get an ack from Dave/Daniel to merge the drm core patch through the >Intel tree. Yes, this would be needed. Thanks Matt for taking this up. Regards, Uma Shankar > >Matt > >> +{ >> + struct drm_device *dev = crtc_state->base.crtc->dev; >> + struct drm_i915_private *dev_priv = to_i915(dev); >> + enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe; >> + const uint32_t lut_size = INTEL_INFO(dev_priv)- >>color.degamma_lut_size; >> + uint32_t i; >> + >> + /* >> + * When setting the auto-increment bit, the hardware seems to >> + * ignore the index bits, so we need to reset it to index 0 >> + * separately. >> + */ >> + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0); >> + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), >PRE_CSC_GAMC_AUTO_INCREMENT); >> + >> + if (crtc_state->base.degamma_lut) { >> + struct drm_color_lut *lut = crtc_state->base.degamma_lut- >>data; >> + >> + for (i = 0; i < lut_size; i++) { >> + /* >> + * First 33 entries represent range from 0 to 1.0 >> + * 34th and 35th entry will represent extended range >> + * inputs 3.0 and 7.0 respectively, currently clamped >> + * at 1.0. Since the precision is 16bit, the user value >> + * can be directly filled to register. >> + * ToDo: Extend to max 7.0. >> + */ >> + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), lut[i].green); >> + } >> + } else { >> + /* load a linear table. */ >> + for (i = 0; i < lut_size; i++) { >> + uint32_t v = (i * (1 << 16)) / (lut_size - 1); >> + >> + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v); >> + } >> + } >> + >> + /* Clamp values > 1.0. */ >> + while (i++ < 35) >> + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16)); } >> + >> +static void icl_load_luts(struct intel_crtc_state *crtc_state) { >> + struct drm_crtc *crtc = crtc_state->base.crtc; >> + struct drm_device *dev = crtc_state->base.crtc->dev; >> + struct drm_i915_private *dev_priv = to_i915(dev); >> + enum pipe pipe = to_intel_crtc(crtc)->pipe; >> + >> + if (crtc_state_is_legacy_gamma(crtc_state)) { >> + haswell_load_luts(crtc_state); >> + return; >> + } >> + >> + icl_load_degamma_lut(crtc_state); >> + bdw_load_gamma_lut(crtc_state, 0); >> + >> + I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT | >> + PRE_CSC_GAMMA_ENABLE | POST_CSC_GAMMA_ENABLE); } >> + >> /* Loads the palette/gamma unit for the CRTC on CherryView. */ >> static void cherryview_load_luts(struct intel_crtc_state *crtc_state) >> { @@ -636,6 +699,8 @@ void intel_color_init(struct intel_crtc *crtc) >> } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) { >> dev_priv->display.load_csc_matrix = ilk_load_csc_matrix; >> dev_priv->display.load_luts = glk_load_luts; >> + } else if (IS_ICELAKE(dev_priv)) { >> + dev_priv->display.load_luts = icl_load_luts; >> } else { >> dev_priv->display.load_luts = i9xx_load_luts; >> } >> -- >> 1.9.1 >> > >-- >Matt Roper >Graphics Software Engineer >IoTG Platform Enabling & Development >Intel Corporation >(916) 356-2795
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 44958d9..f29eef7 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7088,11 +7088,13 @@ enum { #define _GAMMA_MODE_A 0x4a480 #define _GAMMA_MODE_B 0x4ac80 #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B) -#define GAMMA_MODE_MODE_MASK (3 << 0) -#define GAMMA_MODE_MODE_8BIT (0 << 0) -#define GAMMA_MODE_MODE_10BIT (1 << 0) -#define GAMMA_MODE_MODE_12BIT (2 << 0) -#define GAMMA_MODE_MODE_SPLIT (3 << 0) +#define PRE_CSC_GAMMA_ENABLE (1 << 31) +#define POST_CSC_GAMMA_ENABLE (1 << 30) +#define GAMMA_MODE_MODE_MASK (3 << 0) +#define GAMMA_MODE_MODE_8BIT (0 << 0) +#define GAMMA_MODE_MODE_10BIT (1 << 0) +#define GAMMA_MODE_MODE_12BIT (2 << 0) +#define GAMMA_MODE_MODE_SPLIT (3 << 0) /* DMC/CSR */ #define CSR_PROGRAM(i) _MMIO(0x80000 + (i) * 4) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 9a72e64..9cd4646 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -520,6 +520,69 @@ static void glk_load_luts(struct intel_crtc_state *crtc_state) POSTING_READ(GAMMA_MODE(pipe)); } +static void icl_load_degamma_lut(struct intel_crtc_state *crtc_state) +{ + struct drm_device *dev = crtc_state->base.crtc->dev; + struct drm_i915_private *dev_priv = to_i915(dev); + enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe; + const uint32_t lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size; + uint32_t i; + + /* + * When setting the auto-increment bit, the hardware seems to + * ignore the index bits, so we need to reset it to index 0 + * separately. + */ + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0); + I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT); + + if (crtc_state->base.degamma_lut) { + struct drm_color_lut *lut = crtc_state->base.degamma_lut->data; + + for (i = 0; i < lut_size; i++) { + /* + * First 33 entries represent range from 0 to 1.0 + * 34th and 35th entry will represent extended range + * inputs 3.0 and 7.0 respectively, currently clamped + * at 1.0. Since the precision is 16bit, the user value + * can be directly filled to register. + * ToDo: Extend to max 7.0. + */ + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), lut[i].green); + } + } else { + /* load a linear table. */ + for (i = 0; i < lut_size; i++) { + uint32_t v = (i * (1 << 16)) / (lut_size - 1); + + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v); + } + } + + /* Clamp values > 1.0. */ + while (i++ < 35) + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16)); +} + +static void icl_load_luts(struct intel_crtc_state *crtc_state) +{ + struct drm_crtc *crtc = crtc_state->base.crtc; + struct drm_device *dev = crtc_state->base.crtc->dev; + struct drm_i915_private *dev_priv = to_i915(dev); + enum pipe pipe = to_intel_crtc(crtc)->pipe; + + if (crtc_state_is_legacy_gamma(crtc_state)) { + haswell_load_luts(crtc_state); + return; + } + + icl_load_degamma_lut(crtc_state); + bdw_load_gamma_lut(crtc_state, 0); + + I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT | + PRE_CSC_GAMMA_ENABLE | POST_CSC_GAMMA_ENABLE); +} + /* Loads the palette/gamma unit for the CRTC on CherryView. */ static void cherryview_load_luts(struct intel_crtc_state *crtc_state) { @@ -636,6 +699,8 @@ void intel_color_init(struct intel_crtc *crtc) } else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) { dev_priv->display.load_csc_matrix = ilk_load_csc_matrix; dev_priv->display.load_luts = glk_load_luts; + } else if (IS_ICELAKE(dev_priv)) { + dev_priv->display.load_luts = icl_load_luts; } else { dev_priv->display.load_luts = i9xx_load_luts; }
Add support for icl pipe degamma and gamma. v2: Removed a POSTING_READ and corrected the Bit Definition as per Maarten's comments. v3: Addressed Matt's review comments. Removed rmw patterns as suggested by Matt. v4: Fixed Matt's review comments. v5: Corrected macro alignment as per Jani Nikula's comments. Addressed Ville and Matt's review comments. Signed-off-by: Uma Shankar <uma.shankar@intel.com> --- drivers/gpu/drm/i915/i915_reg.h | 12 ++++--- drivers/gpu/drm/i915/intel_color.c | 65 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 5 deletions(-)