[Intel-gfx] [PATCH v3 10/11] drm/i915/dsb: Enable gamma lut programming using DSB.

Animesh Manna animesh.manna at intel.com
Thu Aug 29 13:48:13 UTC 2019


Hi,


On 8/28/2019 11:45 PM, Sharma, Shashank wrote:
>
> On 8/28/2019 12:40 AM, Animesh Manna wrote:
>> Gamma lut programming can be programmed using DSB
>> where bulk register programming can be done using indexed
>> register write which takes number of data and the mmio offset
>> to be written.
>>
>> v1: Initial version.
>> v2: Directly call dsb-api at callsites. (Jani)
>>
>> Cc: Jani Nikula <jani.nikula at intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> Signed-off-by: Animesh Manna <animesh.manna at intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_color.c | 64 ++++++++++++++--------
>>   drivers/gpu/drm/i915/i915_drv.h            |  2 +
>>   2 files changed, 43 insertions(+), 23 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
>> b/drivers/gpu/drm/i915/display/intel_color.c
>> index 71a0201437a9..e4090d8e4547 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -589,34 +589,38 @@ static void bdw_load_lut_10(struct intel_crtc 
>> *crtc,
>>       const struct drm_color_lut *lut = blob->data;
>>       int i, lut_size = drm_color_lut_size(blob);
>>       enum pipe pipe = crtc->pipe;
>> +    struct intel_dsb *dsb = dev_priv->dsb;
>>   -    I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
>> -           PAL_PREC_AUTO_INCREMENT);
> I dont see any gen check in any of the functions before calling dsb() 
> functions.

Explained before, as per agreed design with Jani the plan here is to 
replace I915_WRITE call with dsb-write
and for platforms which are not supporting DSB will fallback into MMIO 
write.

Regards,
Animesh

>> +    intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe),
>> +                prec_index | PAL_PREC_AUTO_INCREMENT);
>>         for (i = 0; i < hw_lut_size; i++) {
>>           /* We discard half the user entries in split gamma mode */
>>           const struct drm_color_lut *entry =
>>               &lut[i * (lut_size - 1) / (hw_lut_size - 1)];
>>   -        I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
>> +        intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
>> +                        ilk_lut_10(entry));
>>       }
>>         /*
>>        * Reset the index, otherwise it prevents the legacy palette to be
>>        * written properly.
>>        */
>> -    I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>> +    intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), 0);
>>   }
>>     static void ivb_load_lut_ext_max(struct intel_crtc *crtc)
>>   {
>>       struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +    struct intel_dsb *dsb = dev_priv->dsb;
>>       enum pipe pipe = crtc->pipe;
>>         /* Program the max register to clamp values > 1.0. */
>> -    I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
>> -    I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
>> -    I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
>> +    intel_dsb_reg_write(dsb, PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
>> +    intel_dsb_reg_write(dsb, PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
>> +    intel_dsb_reg_write(dsb, PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
>> +
>>         /*
>>        * Program the gc max 2 register to clamp values > 1.0.
>> @@ -624,9 +628,12 @@ static void ivb_load_lut_ext_max(struct 
>> intel_crtc *crtc)
>>        * from 3.0 to 7.0
>>        */
>>       if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
>> -        I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
>> -        I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
>> -        I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
>> +        intel_dsb_reg_write(dsb, PREC_PAL_EXT2_GC_MAX(pipe, 0),
>> +                    1 << 16);
>> +        intel_dsb_reg_write(dsb, PREC_PAL_EXT2_GC_MAX(pipe, 1),
>> +                    1 << 16);
>> +        intel_dsb_reg_write(dsb, PREC_PAL_EXT2_GC_MAX(pipe, 2),
>> +                    1 << 16);
>>       }
>>   }
>>   @@ -788,12 +795,13 @@ icl_load_gcmax(const struct intel_crtc_state 
>> *crtc_state,
>>   {
>>       struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>       struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +    struct intel_dsb *dsb = dev_priv->dsb;
>>       enum pipe pipe = crtc->pipe;
>>         /* Fixme: LUT entries are 16 bit only, so we can prog 0xFFFF 
>> max */
>> -    I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), color->red);
>> -    I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), color->green);
>> -    I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), color->blue);
>> +    intel_dsb_reg_write(dsb, PREC_PAL_GC_MAX(pipe, 0), color->red);
>> +    intel_dsb_reg_write(dsb, PREC_PAL_GC_MAX(pipe, 1), color->green);
>> +    intel_dsb_reg_write(dsb, PREC_PAL_GC_MAX(pipe, 2), color->blue);
>>   }
>>     static void
>> @@ -803,6 +811,7 @@ icl_program_gamma_superfine_segment(const struct 
>> intel_crtc_state *crtc_state)
>>       struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>       const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
>>       const struct drm_color_lut *lut = blob->data;
>> +    struct intel_dsb *dsb = dev_priv->dsb;
>>       enum pipe pipe = crtc->pipe;
>>       u32 i;
>>   @@ -813,15 +822,16 @@ icl_program_gamma_superfine_segment(const 
>> struct intel_crtc_state *crtc_state)
>>        * Superfine segment has 9 entries, corresponding to values
>>        * 0, 1/(8 * 128 * 256), 2/(8 * 128 * 256) .... 8/(8 * 128 * 256).
>>        */
>> -    I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), 
>> PAL_PREC_AUTO_INCREMENT);
>> +    intel_dsb_reg_write(dsb, PREC_PAL_MULTI_SEG_INDEX(pipe),
>> +                PAL_PREC_AUTO_INCREMENT);
>>         for (i = 0; i < 9; i++) {
>>           const struct drm_color_lut *entry = &lut[i];
>>   -        I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
>> -               ilk_lut_12p4_ldw(entry));
>> -        I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
>> -               ilk_lut_12p4_udw(entry));
>> +        intel_dsb_indexed_reg_write(dsb, PREC_PAL_MULTI_SEG_DATA(pipe),
>> +                        ilk_lut_12p4_ldw(entry));
>> +        intel_dsb_indexed_reg_write(dsb, PREC_PAL_MULTI_SEG_DATA(pipe),
>> +                        ilk_lut_12p4_udw(entry));
>>       }
>>   }
>>   @@ -833,6 +843,7 @@ icl_program_gamma_multi_segment(const struct 
>> intel_crtc_state *crtc_state)
>>       const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
>>       const struct drm_color_lut *lut = blob->data;
>>       const struct drm_color_lut *entry;
>> +    struct intel_dsb *dsb = dev_priv->dsb;
>>       enum pipe pipe = crtc->pipe;
>>       u32 i;
>>   @@ -847,11 +858,13 @@ icl_program_gamma_multi_segment(const struct 
>> intel_crtc_state *crtc_state)
>>        * PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
>>        * with seg2[0] being unused by the hardware.
>>        */
>> -    I915_WRITE(PREC_PAL_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
>> +    intel_dsb_reg_write(dsb, PREC_PAL_INDEX(pipe), 
>> PAL_PREC_AUTO_INCREMENT);
>>       for (i = 1; i < 257; i++) {
>>           entry = &lut[i * 8];
>> -        I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
>> -        I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
>> +        intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
>> +                        ilk_lut_12p4_ldw(entry));
>> +        intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
>> +                        ilk_lut_12p4_udw(entry));
>>       }
>>         /*
>> @@ -868,8 +881,10 @@ icl_program_gamma_multi_segment(const struct 
>> intel_crtc_state *crtc_state)
>>        */
>>       for (i = 0; i < 256; i++) {
>>           entry = &lut[i * 8 * 128];
>> -        I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
>> -        I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
>> +        intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
>> +                        ilk_lut_12p4_ldw(entry));
>> +        intel_dsb_indexed_reg_write(dsb, PREC_PAL_DATA(pipe),
>> +                        ilk_lut_12p4_udw(entry));
>>       }
>>         /* The last entry in the LUT is to be programmed in GCMAX */
>> @@ -980,7 +995,10 @@ void intel_color_load_luts(const struct 
>> intel_crtc_state *crtc_state)
>>   {
>>       struct drm_i915_private *dev_priv = 
>> to_i915(crtc_state->base.crtc->dev);
>>   +    dev_priv->dsb = 
>> intel_dsb_get(to_intel_crtc(crtc_state->base.crtc));
>>       dev_priv->display.load_luts(crtc_state);
>> +    intel_dsb_commit(dev_priv->dsb);
>> +    intel_dsb_put(dev_priv->dsb);
>>   }
>>     void intel_color_commit(const struct intel_crtc_state *crtc_state)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index dd6cfb89b830..ace81ccbd766 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1759,6 +1759,8 @@ struct drm_i915_private {
>>       /* Mutex to protect the above hdcp component related values. */
>>       struct mutex hdcp_comp_mutex;
>>   +    struct intel_dsb *dsb;
>> +
>>       /*
>>        * NOTE: This is the dri1/ums dungeon, don't add stuff here. 
>> Your patch
>>        * will be rejected. Instead look for a better place.



More information about the Intel-gfx mailing list