| Message ID | 20170127091949.6618-1-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 27, 2017 at 11:19:49AM +0200, Ander Conselvan de Oliveira wrote: > The only difference between the code loading the pipe gamma and degamma > tables in BDW is that the gamma code also writes the registers that hold > the maximum values. So we can use the gamma code for the degamma table, > at the expense of writing the maximum value register twice, with > potenttially wrong values in the first time. > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > > Ville, does this help with the split gamma enable/disable confusion? I might. Certainly it avoids a bunch of duplicated code. One comment below... > > Note that I didn't test this. > > Ander > > --- > drivers/gpu/drm/i915/intel_color.c | 57 ++++++++++---------------------------- > 1 file changed, 15 insertions(+), 42 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 08345e5..c686c37 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -340,54 +340,22 @@ static void haswell_load_luts(struct drm_crtc_state *crtc_state) > hsw_enable_ips(intel_crtc); > } > > -static void bdw_load_degamma_lut(struct drm_crtc_state *state) > +static void bdw_load_lut(struct drm_crtc_state *state, u32 offset, > + struct drm_color_lut *lut, u32 lut_size, > + bool split_mode) Maybe pass PAL_PREC_SPLIT_MODE or 0 here directly? To make the callers even more clear we could go as far as defining a symbolic name for the 0. > { > struct drm_i915_private *dev_priv = to_i915(state->crtc->dev); > enum pipe pipe = to_intel_crtc(state->crtc)->pipe; > - uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size; > - > - I915_WRITE(PREC_PAL_INDEX(pipe), > - PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT); > - > - if (state->degamma_lut) { > - struct drm_color_lut *lut = > - (struct drm_color_lut *) state->degamma_lut->data; > - > - for (i = 0; i < lut_size; i++) { > - uint32_t word = > - drm_color_lut_extract(lut[i].red, 10) << 20 | > - drm_color_lut_extract(lut[i].green, 10) << 10 | > - drm_color_lut_extract(lut[i].blue, 10); > - > - I915_WRITE(PREC_PAL_DATA(pipe), word); > - } > - } else { > - for (i = 0; i < lut_size; i++) { > - uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1); > - > - I915_WRITE(PREC_PAL_DATA(pipe), > - (v << 20) | (v << 10) | v); > - } > - } > -} > - > -static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset) > -{ > - struct drm_i915_private *dev_priv = to_i915(state->crtc->dev); > - enum pipe pipe = to_intel_crtc(state->crtc)->pipe; > - uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size; > + uint32_t i; > > WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK); > > I915_WRITE(PREC_PAL_INDEX(pipe), > - (offset ? PAL_PREC_SPLIT_MODE : 0) | > + (split_mode ? PAL_PREC_SPLIT_MODE : 0) | > PAL_PREC_AUTO_INCREMENT | > offset); > > - if (state->gamma_lut) { > - struct drm_color_lut *lut = > - (struct drm_color_lut *) state->gamma_lut->data; > - > + if (lut) { > for (i = 0; i < lut_size; i++) { > uint32_t word = > (drm_color_lut_extract(lut[i].red, 10) << 20) | > @@ -430,9 +398,12 @@ static void broadwell_load_luts(struct drm_crtc_state *state) > return; > } > > - bdw_load_degamma_lut(state); > - bdw_load_gamma_lut(state, > - INTEL_INFO(dev_priv)->color.degamma_lut_size); > + bdw_load_lut(state, 0, (struct drm_color_lut *) state->degamma_lut, > + INTEL_INFO(dev_priv)->color.degamma_lut_size, true); > + bdw_load_lut(state, INTEL_INFO(dev_priv)->color.degamma_lut_size, > + (struct drm_color_lut *) state->gamma_lut, > + INTEL_INFO(dev_priv)->color.gamma_lut_size, > + true); > > intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; > I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); > @@ -489,7 +460,9 @@ static void glk_load_luts(struct drm_crtc_state *state) > } > > glk_load_degamma_lut(state); > - bdw_load_gamma_lut(state, 0); > + bdw_load_lut(state, 0, (struct drm_color_lut *) state->gamma_lut, > + INTEL_INFO(dev_priv)->color.gamma_lut_size, > + false); > > intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT; > I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT); > -- > 2.9.3
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 08345e5..c686c37 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -340,54 +340,22 @@ static void haswell_load_luts(struct drm_crtc_state *crtc_state) hsw_enable_ips(intel_crtc); } -static void bdw_load_degamma_lut(struct drm_crtc_state *state) +static void bdw_load_lut(struct drm_crtc_state *state, u32 offset, + struct drm_color_lut *lut, u32 lut_size, + bool split_mode) { struct drm_i915_private *dev_priv = to_i915(state->crtc->dev); enum pipe pipe = to_intel_crtc(state->crtc)->pipe; - uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size; - - I915_WRITE(PREC_PAL_INDEX(pipe), - PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT); - - if (state->degamma_lut) { - struct drm_color_lut *lut = - (struct drm_color_lut *) state->degamma_lut->data; - - for (i = 0; i < lut_size; i++) { - uint32_t word = - drm_color_lut_extract(lut[i].red, 10) << 20 | - drm_color_lut_extract(lut[i].green, 10) << 10 | - drm_color_lut_extract(lut[i].blue, 10); - - I915_WRITE(PREC_PAL_DATA(pipe), word); - } - } else { - for (i = 0; i < lut_size; i++) { - uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1); - - I915_WRITE(PREC_PAL_DATA(pipe), - (v << 20) | (v << 10) | v); - } - } -} - -static void bdw_load_gamma_lut(struct drm_crtc_state *state, u32 offset) -{ - struct drm_i915_private *dev_priv = to_i915(state->crtc->dev); - enum pipe pipe = to_intel_crtc(state->crtc)->pipe; - uint32_t i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size; + uint32_t i; WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK); I915_WRITE(PREC_PAL_INDEX(pipe), - (offset ? PAL_PREC_SPLIT_MODE : 0) | + (split_mode ? PAL_PREC_SPLIT_MODE : 0) | PAL_PREC_AUTO_INCREMENT | offset); - if (state->gamma_lut) { - struct drm_color_lut *lut = - (struct drm_color_lut *) state->gamma_lut->data; - + if (lut) { for (i = 0; i < lut_size; i++) { uint32_t word = (drm_color_lut_extract(lut[i].red, 10) << 20) | @@ -430,9 +398,12 @@ static void broadwell_load_luts(struct drm_crtc_state *state) return; } - bdw_load_degamma_lut(state); - bdw_load_gamma_lut(state, - INTEL_INFO(dev_priv)->color.degamma_lut_size); + bdw_load_lut(state, 0, (struct drm_color_lut *) state->degamma_lut, + INTEL_INFO(dev_priv)->color.degamma_lut_size, true); + bdw_load_lut(state, INTEL_INFO(dev_priv)->color.degamma_lut_size, + (struct drm_color_lut *) state->gamma_lut, + INTEL_INFO(dev_priv)->color.gamma_lut_size, + true); intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); @@ -489,7 +460,9 @@ static void glk_load_luts(struct drm_crtc_state *state) } glk_load_degamma_lut(state); - bdw_load_gamma_lut(state, 0); + bdw_load_lut(state, 0, (struct drm_color_lut *) state->gamma_lut, + INTEL_INFO(dev_priv)->color.gamma_lut_size, + false); intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT; I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
The only difference between the code loading the pipe gamma and degamma tables in BDW is that the gamma code also writes the registers that hold the maximum values. So we can use the gamma code for the degamma table, at the expense of writing the maximum value register twice, with potenttially wrong values in the first time. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- Ville, does this help with the split gamma enable/disable confusion? Note that I didn't test this. Ander --- drivers/gpu/drm/i915/intel_color.c | 57 ++++++++++---------------------------- 1 file changed, 15 insertions(+), 42 deletions(-)