* [PATCH v4 0/3] Geminilake pipe CSC
@ 2017-02-03 13:11 Ander Conselvan de Oliveira
2017-02-03 13:11 ` [PATCH 1/3] drm/i915: Merge BDW pipe gamma and degamma table code Ander Conselvan de Oliveira
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-03 13:11 UTC (permalink / raw)
To: intel-gfx; +Cc: Ander Conselvan de Oliveira
Ander Conselvan de Oliveira (3):
drm/i915: Merge BDW pipe gamma and degamma table code
drm/i915/glk: Load the degamma LUT even in legacy gamma mode
drm/i915/glk: Enable pipe CSC
drivers/gpu/drm/i915/intel_color.c | 62 ++++++++++--------------------------
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_sprite.c | 1 +
3 files changed, 19 insertions(+), 45 deletions(-)
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] drm/i915: Merge BDW pipe gamma and degamma table code 2017-02-03 13:11 [PATCH v4 0/3] Geminilake pipe CSC Ander Conselvan de Oliveira @ 2017-02-03 13:11 ` Ander Conselvan de Oliveira 2017-02-09 14:47 ` Ville Syrjälä 2017-02-03 13:11 ` [PATCH 2/3] drm/i915/glk: Load the degamma LUT even in legacy gamma mode Ander Conselvan de Oliveira ` (2 subsequent siblings) 3 siblings, 1 reply; 10+ messages in thread From: Ander Conselvan de Oliveira @ 2017-02-03 13:11 UTC (permalink / raw) To: intel-gfx; +Cc: Ander Conselvan de Oliveira 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. v2: Pass PAL_PREC_SPLIT_MODE from the caller. (Ville) Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- drivers/gpu/drm/i915/intel_color.c | 59 ++++++++++---------------------------- 1 file changed, 15 insertions(+), 44 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 0627eee..82e1809 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -340,54 +340,20 @@ 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, + u32 flags) { 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) | - PAL_PREC_AUTO_INCREMENT | - offset); - - if (state->gamma_lut) { - struct drm_color_lut *lut = - (struct drm_color_lut *) state->gamma_lut->data; + flags | PAL_PREC_AUTO_INCREMENT | offset); + if (lut) { for (i = 0; i < lut_size; i++) { uint32_t word = (drm_color_lut_extract(lut[i].red, 10) << 20) | @@ -430,9 +396,13 @@ 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, + PAL_PREC_SPLIT_MODE); + 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, + PAL_PREC_SPLIT_MODE); intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); @@ -489,7 +459,8 @@ 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, 0); intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT; I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT); -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm/i915: Merge BDW pipe gamma and degamma table code 2017-02-03 13:11 ` [PATCH 1/3] drm/i915: Merge BDW pipe gamma and degamma table code Ander Conselvan de Oliveira @ 2017-02-09 14:47 ` Ville Syrjälä 2017-02-10 10:14 ` Ander Conselvan De Oliveira 0 siblings, 1 reply; 10+ messages in thread From: Ville Syrjälä @ 2017-02-09 14:47 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: intel-gfx On Fri, Feb 03, 2017 at 03:11:13PM +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. > > v2: Pass PAL_PREC_SPLIT_MODE from the caller. (Ville) > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > drivers/gpu/drm/i915/intel_color.c | 59 ++++++++++---------------------------- > 1 file changed, 15 insertions(+), 44 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 0627eee..82e1809 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -340,54 +340,20 @@ 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, Maybe just pass the intel_crtc instead of the state? I presume we don't need anything else from the state really? > + struct drm_color_lut *lut, u32 lut_size, > + u32 flags) > { > 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) | > - PAL_PREC_AUTO_INCREMENT | > - offset); > - > - if (state->gamma_lut) { > - struct drm_color_lut *lut = > - (struct drm_color_lut *) state->gamma_lut->data; > + flags | PAL_PREC_AUTO_INCREMENT | offset); > > + if (lut) { > for (i = 0; i < lut_size; i++) { > uint32_t word = > (drm_color_lut_extract(lut[i].red, 10) << 20) | > @@ -430,9 +396,13 @@ 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, ->data ? Such explicit casts are a good recipe for bugs I think, so would be nice if someone could make this blob stuff more type safe. Dunno, maybe some kind of 'void *blob_get_data()' type of thing, or maybe decouple the blob data from the blob once again so that we'd have 'void *data' in there. > + INTEL_INFO(dev_priv)->color.degamma_lut_size, > + PAL_PREC_SPLIT_MODE); > + 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, > + PAL_PREC_SPLIT_MODE); > > intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; > I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); > @@ -489,7 +459,8 @@ 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, 0); > > intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT; > I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT); > -- > 2.9.3 -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm/i915: Merge BDW pipe gamma and degamma table code 2017-02-09 14:47 ` Ville Syrjälä @ 2017-02-10 10:14 ` Ander Conselvan De Oliveira 2017-02-10 14:16 ` Ville Syrjälä 0 siblings, 1 reply; 10+ messages in thread From: Ander Conselvan De Oliveira @ 2017-02-10 10:14 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Thu, 2017-02-09 at 16:47 +0200, Ville Syrjälä wrote: > On Fri, Feb 03, 2017 at 03:11:13PM +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. > > > > v2: Pass PAL_PREC_SPLIT_MODE from the caller. (Ville) > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@inte > > l.com> > > --- > > drivers/gpu/drm/i915/intel_color.c | 59 ++++++++++------------------------- > > --- > > 1 file changed, 15 insertions(+), 44 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_color.c > > b/drivers/gpu/drm/i915/intel_color.c > > index 0627eee..82e1809 100644 > > --- a/drivers/gpu/drm/i915/intel_color.c > > +++ b/drivers/gpu/drm/i915/intel_color.c > > @@ -340,54 +340,20 @@ 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, > > Maybe just pass the intel_crtc instead of the state? I presume we don't > need anything else from the state really? > > > + struct drm_color_lut *lut, u32 lut_size, > > + u32 flags) > > { > > 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) | > > - PAL_PREC_AUTO_INCREMENT | > > - offset); > > - > > - if (state->gamma_lut) { > > - struct drm_color_lut *lut = > > - (struct drm_color_lut *) state->gamma_lut->data; > > + flags | PAL_PREC_AUTO_INCREMENT | offset); > > > > + if (lut) { > > for (i = 0; i < lut_size; i++) { > > uint32_t word = > > (drm_color_lut_extract(lut[i].red, 10) << 20) | > > @@ -430,9 +396,13 @@ 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, > > ->data ? > > Such explicit casts are a good recipe for bugs I think, so would be nice > if someone could make this blob stuff more type safe. Yikes! Totally missed that. Indeed the cast is a really bad idea. > Dunno, maybe some kind of 'void *blob_get_data()' type of thing, or > maybe decouple the blob data from the blob once again so that we'd have > 'void *data' in there. For this patch, how about a static struct drm_color_lut * blob_data_as_lut(struct drm_property_blob *blob) { if (!blob) return NULL; return (struct drm_color_lut *) blob->data; } I think it is better than passing a void*. Ander > > > + INTEL_INFO(dev_priv)->color.degamma_lut_size, > > + PAL_PREC_SPLIT_MODE); > > + 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, > > + PAL_PREC_SPLIT_MODE); > > > > intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; > > I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); > > @@ -489,7 +459,8 @@ 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, 0); > > > > intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT; > > I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT); > > -- > > 2.9.3 > > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm/i915: Merge BDW pipe gamma and degamma table code 2017-02-10 10:14 ` Ander Conselvan De Oliveira @ 2017-02-10 14:16 ` Ville Syrjälä 0 siblings, 0 replies; 10+ messages in thread From: Ville Syrjälä @ 2017-02-10 14:16 UTC (permalink / raw) To: Ander Conselvan De Oliveira; +Cc: intel-gfx On Fri, Feb 10, 2017 at 12:14:45PM +0200, Ander Conselvan De Oliveira wrote: > On Thu, 2017-02-09 at 16:47 +0200, Ville Syrjälä wrote: > > On Fri, Feb 03, 2017 at 03:11:13PM +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. > > > > > > v2: Pass PAL_PREC_SPLIT_MODE from the caller. (Ville) > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@inte > > > l.com> > > > --- > > > drivers/gpu/drm/i915/intel_color.c | 59 ++++++++++------------------------- > > > --- > > > 1 file changed, 15 insertions(+), 44 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_color.c > > > b/drivers/gpu/drm/i915/intel_color.c > > > index 0627eee..82e1809 100644 > > > --- a/drivers/gpu/drm/i915/intel_color.c > > > +++ b/drivers/gpu/drm/i915/intel_color.c > > > @@ -340,54 +340,20 @@ 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, > > > > Maybe just pass the intel_crtc instead of the state? I presume we don't > > need anything else from the state really? > > > > > + struct drm_color_lut *lut, u32 lut_size, > > > + u32 flags) > > > { > > > 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) | > > > - PAL_PREC_AUTO_INCREMENT | > > > - offset); > > > - > > > - if (state->gamma_lut) { > > > - struct drm_color_lut *lut = > > > - (struct drm_color_lut *) state->gamma_lut->data; > > > + flags | PAL_PREC_AUTO_INCREMENT | offset); > > > > > > + if (lut) { > > > for (i = 0; i < lut_size; i++) { > > > uint32_t word = > > > (drm_color_lut_extract(lut[i].red, 10) << 20) | > > > @@ -430,9 +396,13 @@ 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, > > > > ->data ? > > > > Such explicit casts are a good recipe for bugs I think, so would be nice > > if someone could make this blob stuff more type safe. > > Yikes! Totally missed that. Indeed the cast is a really bad idea. > > > Dunno, maybe some kind of 'void *blob_get_data()' type of thing, or > > maybe decouple the blob data from the blob once again so that we'd have > > 'void *data' in there. > > For this patch, how about a > > static struct drm_color_lut * > blob_data_as_lut(struct drm_property_blob *blob) > { > if (!blob) > return NULL; > > return (struct drm_color_lut *) blob->data; > } I could live with that. > > I think it is better than passing a void*. Perhaps. But it does require a cast to the specific type, so every user would have to roll their own thing. void* you don't have to cast at all. Of course you could by accident pass the wrong void*. > > Ander > > > > > > + INTEL_INFO(dev_priv)->color.degamma_lut_size, > > > + PAL_PREC_SPLIT_MODE); > > > + 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, > > > + PAL_PREC_SPLIT_MODE); > > > > > > intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; > > > I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); > > > @@ -489,7 +459,8 @@ 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, 0); > > > > > > intel_state->gamma_mode = GAMMA_MODE_MODE_10BIT; > > > I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT); > > > -- > > > 2.9.3 > > > > -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] drm/i915/glk: Load the degamma LUT even in legacy gamma mode 2017-02-03 13:11 [PATCH v4 0/3] Geminilake pipe CSC Ander Conselvan de Oliveira 2017-02-03 13:11 ` [PATCH 1/3] drm/i915: Merge BDW pipe gamma and degamma table code Ander Conselvan de Oliveira @ 2017-02-03 13:11 ` Ander Conselvan de Oliveira 2017-02-09 8:29 ` Ander Conselvan De Oliveira 2017-02-09 14:47 ` Ville Syrjälä 2017-02-03 13:11 ` [PATCH 3/3] drm/i915/glk: Enable pipe CSC Ander Conselvan de Oliveira 2017-02-03 15:26 ` ✓ Fi.CI.BAT: success for Geminilake pipe CSC (rev3) Patchwork 3 siblings, 2 replies; 10+ messages in thread From: Ander Conselvan de Oliveira @ 2017-02-03 13:11 UTC (permalink / raw) To: intel-gfx; +Cc: Ander Conselvan de Oliveira In Geminilake, the degamma table is enabled or disabled by the pipe CSC enable bit, so its active even when running in the legacy gamma mode. So always set sane values for that table, since the default value is all zeroes. This fixes blank screens after a suspend/resume cycle while legacy gamma is in use. Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- 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 82e1809..c221891 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -453,12 +453,13 @@ static void glk_load_luts(struct drm_crtc_state *state) struct intel_crtc_state *intel_state = to_intel_crtc_state(state); enum pipe pipe = to_intel_crtc(crtc)->pipe; + glk_load_degamma_lut(state); + if (crtc_state_is_legacy(state)) { haswell_load_luts(state); return; } - glk_load_degamma_lut(state); bdw_load_lut(state, 0, (struct drm_color_lut *) state->gamma_lut, INTEL_INFO(dev_priv)->color.gamma_lut_size, 0); -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] drm/i915/glk: Load the degamma LUT even in legacy gamma mode 2017-02-03 13:11 ` [PATCH 2/3] drm/i915/glk: Load the degamma LUT even in legacy gamma mode Ander Conselvan de Oliveira @ 2017-02-09 8:29 ` Ander Conselvan De Oliveira 2017-02-09 14:47 ` Ville Syrjälä 1 sibling, 0 replies; 10+ messages in thread From: Ander Conselvan De Oliveira @ 2017-02-09 8:29 UTC (permalink / raw) To: intel-gfx, ville.syrjala On Fri, 2017-02-03 at 15:11 +0200, Ander Conselvan de Oliveira wrote: > In Geminilake, the degamma table is enabled or disabled by the pipe CSC > enable bit, so its active even when running in the legacy gamma mode. > So always set sane values for that table, since the default value is all > zeroes. > > This fixes blank screens after a suspend/resume cycle while legacy gamma > is in use. Ville, any comments on this? This is the only thing preventing me from merging the other patch that flips the switch. Thanks, Ander > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > 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 82e1809..c221891 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -453,12 +453,13 @@ static void glk_load_luts(struct drm_crtc_state *state) > > struct intel_crtc_state *intel_state = to_intel_crtc_state(state); > > enum pipe pipe = to_intel_crtc(crtc)->pipe; > > > + glk_load_degamma_lut(state); > + > > if (crtc_state_is_legacy(state)) { > > haswell_load_luts(state); > > return; > > } > > > - glk_load_degamma_lut(state); > > bdw_load_lut(state, 0, (struct drm_color_lut *) state->gamma_lut, > > INTEL_INFO(dev_priv)->color.gamma_lut_size, 0); > _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] drm/i915/glk: Load the degamma LUT even in legacy gamma mode 2017-02-03 13:11 ` [PATCH 2/3] drm/i915/glk: Load the degamma LUT even in legacy gamma mode Ander Conselvan de Oliveira 2017-02-09 8:29 ` Ander Conselvan De Oliveira @ 2017-02-09 14:47 ` Ville Syrjälä 1 sibling, 0 replies; 10+ messages in thread From: Ville Syrjälä @ 2017-02-09 14:47 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: intel-gfx On Fri, Feb 03, 2017 at 03:11:14PM +0200, Ander Conselvan de Oliveira wrote: > In Geminilake, the degamma table is enabled or disabled by the pipe CSC > enable bit, so its active even when running in the legacy gamma mode. > So always set sane values for that table, since the default value is all > zeroes. > > This fixes blank screens after a suspend/resume cycle while legacy gamma > is in use. > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Makes sense Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > 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 82e1809..c221891 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -453,12 +453,13 @@ static void glk_load_luts(struct drm_crtc_state *state) > struct intel_crtc_state *intel_state = to_intel_crtc_state(state); > enum pipe pipe = to_intel_crtc(crtc)->pipe; > > + glk_load_degamma_lut(state); > + > if (crtc_state_is_legacy(state)) { > haswell_load_luts(state); > return; > } > > - glk_load_degamma_lut(state); > bdw_load_lut(state, 0, (struct drm_color_lut *) state->gamma_lut, > INTEL_INFO(dev_priv)->color.gamma_lut_size, 0); > > -- > 2.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] drm/i915/glk: Enable pipe CSC 2017-02-03 13:11 [PATCH v4 0/3] Geminilake pipe CSC Ander Conselvan de Oliveira 2017-02-03 13:11 ` [PATCH 1/3] drm/i915: Merge BDW pipe gamma and degamma table code Ander Conselvan de Oliveira 2017-02-03 13:11 ` [PATCH 2/3] drm/i915/glk: Load the degamma LUT even in legacy gamma mode Ander Conselvan de Oliveira @ 2017-02-03 13:11 ` Ander Conselvan de Oliveira 2017-02-03 15:26 ` ✓ Fi.CI.BAT: success for Geminilake pipe CSC (rev3) Patchwork 3 siblings, 0 replies; 10+ messages in thread From: Ander Conselvan de Oliveira @ 2017-02-03 13:11 UTC (permalink / raw) To: intel-gfx; +Cc: Ander Conselvan de Oliveira Now that the pre-csc degamma table is set up correctly in Geminilake, pipe CSC can be enabled without causing a black screen. v2: Rebase. Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_display.c | 1 + drivers/gpu/drm/i915/intel_sprite.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 88689a0..65e1869 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -3391,6 +3391,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane, if (IS_GEMINILAKE(dev_priv)) { I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id), PLANE_COLOR_PIPE_GAMMA_ENABLE | + PLANE_COLOR_PIPE_CSC_ENABLE | PLANE_COLOR_PLANE_GAMMA_DISABLE); } else { plane_ctl |= diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index b16a295..27e0752 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -224,6 +224,7 @@ skl_update_plane(struct drm_plane *drm_plane, if (IS_GEMINILAKE(dev_priv)) { I915_WRITE(PLANE_COLOR_CTL(pipe, plane_id), PLANE_COLOR_PIPE_GAMMA_ENABLE | + PLANE_COLOR_PIPE_CSC_ENABLE | PLANE_COLOR_PLANE_GAMMA_DISABLE); } else { plane_ctl |= -- 2.9.3 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 10+ messages in thread
* ✓ Fi.CI.BAT: success for Geminilake pipe CSC (rev3) 2017-02-03 13:11 [PATCH v4 0/3] Geminilake pipe CSC Ander Conselvan de Oliveira ` (2 preceding siblings ...) 2017-02-03 13:11 ` [PATCH 3/3] drm/i915/glk: Enable pipe CSC Ander Conselvan de Oliveira @ 2017-02-03 15:26 ` Patchwork 3 siblings, 0 replies; 10+ messages in thread From: Patchwork @ 2017-02-03 15:26 UTC (permalink / raw) To: Ander Conselvan de Oliveira; +Cc: intel-gfx == Series Details == Series: Geminilake pipe CSC (rev3) URL : https://patchwork.freedesktop.org/series/18596/ State : success == Summary == Series 18596v3 Geminilake pipe CSC https://patchwork.freedesktop.org/api/1.0/series/18596/revisions/3/mbox/ fi-bdw-5557u total:247 pass:233 dwarn:0 dfail:0 fail:0 skip:14 fi-bsw-n3050 total:247 pass:208 dwarn:0 dfail:0 fail:0 skip:39 fi-bxt-j4205 total:247 pass:225 dwarn:0 dfail:0 fail:0 skip:22 fi-bxt-t5700 total:78 pass:65 dwarn:0 dfail:0 fail:0 skip:12 fi-byt-j1900 total:247 pass:220 dwarn:0 dfail:0 fail:0 skip:27 fi-byt-n2820 total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-hsw-4770 total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-hsw-4770r total:247 pass:228 dwarn:0 dfail:0 fail:0 skip:19 fi-ilk-650 total:14 pass:13 dwarn:0 dfail:0 fail:0 skip:0 fi-ivb-3520m total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-ivb-3770 total:247 pass:226 dwarn:0 dfail:0 fail:0 skip:21 fi-kbl-7500u total:247 pass:224 dwarn:0 dfail:0 fail:2 skip:21 fi-skl-6260u total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-skl-6700hq total:247 pass:227 dwarn:0 dfail:0 fail:0 skip:20 fi-skl-6700k total:247 pass:222 dwarn:4 dfail:0 fail:0 skip:21 fi-skl-6770hq total:247 pass:234 dwarn:0 dfail:0 fail:0 skip:13 fi-snb-2520m total:247 pass:216 dwarn:0 dfail:0 fail:0 skip:31 fi-snb-2600 total:247 pass:215 dwarn:0 dfail:0 fail:0 skip:32 e7f4379a8f59bebb1bdefe0584e128dfdd27a86a drm-tip: 2017y-02m-03d-13h-03m-49s UTC integration manifest 99490d8 drm/i915/glk: Enable pipe CSC e111e90 drm/i915/glk: Load the degamma LUT even in legacy gamma mode f94cf65 drm/i915: Merge BDW pipe gamma and degamma table code == Logs == For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3692/ _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-02-10 14:16 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-02-03 13:11 [PATCH v4 0/3] Geminilake pipe CSC Ander Conselvan de Oliveira 2017-02-03 13:11 ` [PATCH 1/3] drm/i915: Merge BDW pipe gamma and degamma table code Ander Conselvan de Oliveira 2017-02-09 14:47 ` Ville Syrjälä 2017-02-10 10:14 ` Ander Conselvan De Oliveira 2017-02-10 14:16 ` Ville Syrjälä 2017-02-03 13:11 ` [PATCH 2/3] drm/i915/glk: Load the degamma LUT even in legacy gamma mode Ander Conselvan de Oliveira 2017-02-09 8:29 ` Ander Conselvan De Oliveira 2017-02-09 14:47 ` Ville Syrjälä 2017-02-03 13:11 ` [PATCH 3/3] drm/i915/glk: Enable pipe CSC Ander Conselvan de Oliveira 2017-02-03 15:26 ` ✓ Fi.CI.BAT: success for Geminilake pipe CSC (rev3) Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.