Re: [PATCH 02/11] [v2] drm/i915: Extract i9xx_get_color_config()

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 16 Apr 2019, Jani Nikula <jani.nikula@intel.com> wrote:
> On Mon, 15 Apr 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h    |  3 +++
>>  drivers/gpu/drm/i915/intel_color.c | 51 ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 54 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 9c206e8..8f2ae8a 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7175,6 +7175,9 @@ enum {
>>  /* legacy palette */
>>  #define _LGC_PALETTE_A           0x4a000
>>  #define _LGC_PALETTE_B           0x4a800
>> +#define LGC_PALETTE_RED_MASK     REG_GENMASK(23, 16)
>> +#define LGC_PALETTE_GREEN_MASK   REG_GENMASK(15, 8)
>> +#define LGC_PALETTE_BLUE_MASK    REG_GENMASK(7, 0)
>>  #define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) + (i) * 4)
>>  
>>  /* ilk/snb precision palette */
>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>> index 321cf52..f394402 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -1228,6 +1228,56 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
>>  	return 0;
>>  }
>>  
>> +/* convert hw value with given bit_precision to lut property val */
>> +static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
>> +{
>> +	u32 max = 0xffff >> (16 - bit_precision);
>> +
>> +	val = clamp_val(val, 0, max);
>> +
>> +	if (bit_precision < 16)
>> +		val <<= 16 - bit_precision;
>> +
>> +	return val;
>> +}
>> +
>> +static void i9xx_get_config_internal(struct intel_crtc_state *crtc_state)
>> +{
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	struct drm_device *dev = crtc->base.dev;
>> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +	enum pipe pipe = crtc->pipe;
>> +	struct drm_property_blob *blob = NULL;
>
> No need to initialize.
>
>> +	struct drm_color_lut *blob_data;
>> +	u32 i, val;
>> +
>> +	blob = drm_property_create_blob(dev,
>
> You can drop the dev local var as it's simply &dev_priv->drm.
>
>> +					sizeof(struct drm_color_lut) * 256,
>> +					NULL);
>> +	if (IS_ERR(blob))
>> +		return;
>> +
>> +	blob_data = blob->data;
>> +
>> +	for (i = 0; i < 256; i++) {
>> +		if (HAS_GMCH(dev_priv))
>> +			val = I915_READ(PALETTE(pipe, i));
>> +		else
>> +			val = I915_READ(LGC_PALETTE(pipe, i));
>> +
>> +		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val) >> 16, 8);
>> +		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val) >> 8, 8);
>> +		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);
>
> REG_FIELD_GET() return the fields already shifted down according to the
> mask. So the right shifts here are not needed.

So the same comments apply to the rest of the platform specific get
config patches in the series, I'm not going to repeat them individually.

BR,
Jani.


>
>> +	}
>> +
>> +	crtc_state->base.gamma_lut = blob;
>> +}
>> +
>> +static void i9xx_get_color_config(struct intel_crtc_state *crtc_state)
>> +{
>> +	i9xx_get_config_internal(crtc_state);
>> +}
>> +
>>  void intel_color_init(struct intel_crtc *crtc)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> @@ -1248,6 +1298,7 @@ void intel_color_init(struct intel_crtc *crtc)
>>  			dev_priv->display.color_check = i9xx_color_check;
>>  			dev_priv->display.color_commit = i9xx_color_commit;
>>  			dev_priv->display.load_luts = i9xx_load_luts;
>> +			dev_priv->display.get_color_config = i9xx_get_color_config;
>>  		}
>>  	} else {
>>  		if (INTEL_GEN(dev_priv) >= 11)

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx