Re: [Intel-gfx] [PATCH 04/13] drm/i915: Constify the state arguments to the color management stuff
>-Original Message-
>From: Roper, Matthew D
>Sent: Saturday, January 12, 2019 6:12 AM
>To: Ville Syrjala
>Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma
>Subject: Re: [PATCH 04/13] drm/i915: Constify the state arguments to the color
>management stuff
>
>On Fri, Jan 11, 2019 at 07:08:14PM +0200, Ville Syrjala wrote:
>> From: Ville Syrjälä
>>
>> Pass the crtc state etc. as const to the color management commit
>> functions. And while at it polish some of the local variables.
>>
>> Signed-off-by: Ville Syrjälä
>
>Reviewed-by: Matt Roper
Looks ok to me.
Reviewed-by: Uma Shankar
>> ---
>> drivers/gpu/drm/i915/i915_drv.h| 4 +-
>> drivers/gpu/drm/i915/intel_color.c | 128 -
>> drivers/gpu/drm/i915/intel_drv.h | 4 +-
>> 3 files changed, 73 insertions(+), 63 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h index 5df26ccda8a4..7182a580002c
>> 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -320,8 +320,8 @@ struct drm_i915_display_funcs {
>> /* display clock increase/decrease */
>> /* pll clock increase/decrease */
>>
>> -void (*load_csc_matrix)(struct intel_crtc_state *crtc_state);
>> -void (*load_luts)(struct intel_crtc_state *crtc_state);
>> +void (*load_csc_matrix)(const struct intel_crtc_state *crtc_state);
>> +void (*load_luts)(const struct intel_crtc_state *crtc_state);
>> };
>>
>> #define CSR_VERSION(major, minor) ((major) << 16 | (minor))
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index b10e66ce3970..0dfd104b89d7 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -74,12 +74,12 @@
>> #define ILK_CSC_COEFF_1_0 \
>> ((7 << 12) | ILK_CSC_COEFF_FP(CTM_COEFF_1_0, 8))
>>
>> -static bool lut_is_legacy(struct drm_property_blob *lut)
>> +static bool lut_is_legacy(const struct drm_property_blob *lut)
>> {
>> return drm_color_lut_size(lut) == LEGACY_LUT_LENGTH; }
>>
>> -static bool crtc_state_is_legacy_gamma(struct intel_crtc_state
>> *crtc_state)
>> +static bool crtc_state_is_legacy_gamma(const struct intel_crtc_state
>> +*crtc_state)
>> {
>> return !crtc_state->base.degamma_lut &&
>> !crtc_state->base.ctm &&
>> @@ -115,8 +115,8 @@ static u64 *ctm_mult_by_limited(u64 *result, const
>> u64 *input)
>>
>> static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc)
>> {
>> -int pipe = crtc->pipe;
>> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +enum pipe pipe = crtc->pipe;
>>
>> I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
>> I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); @@ -137,13 +137,14 @@
>> static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc)
>> I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>> }
>>
>> -static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
>> +static void ilk_load_csc_matrix(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);
>> -int i, pipe = crtc->pipe;
>> -uint16_t coeffs[9] = { 0, };
>> bool limited_color_range = false;
>> +enum pipe pipe = crtc->pipe;
>> +u16 coeffs[9] = {};
>> +int i;
>>
>> /*
>> * FIXME if there's a gamma LUT after the CSC, we should @@ -256,15
>> +257,15 @@ static void ilk_load_csc_matrix(struct intel_crtc_state
>> *crtc_state)
>> /*
>> * Set up the pipe CSC unit on CherryView.
>> */
>> -static void cherryview_load_csc_matrix(struct intel_crtc_state
>> *crtc_state)
>> +static void cherryview_load_csc_matrix(const struct intel_crtc_state
>> +*crtc_state)
>> {
>> -struct drm_device *dev = crtc_state->base.crtc->dev;
>> -struct drm_i915_private *dev_priv = to_i915(dev);
>> -int pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
>> +struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +enum pipe pipe = crtc->pipe;
>> uint32_t mode;
>>
>> if (crtc_state->base.ctm) {
>> -struct drm_color_ctm *ctm = crtc_state->base.ctm->data;
>> +const struct drm_color_ctm *ctm = crtc_state->base.ctm->data;
>> uint16_t coeffs[9] = { 0, };
>> int i;
>>
>> @@ -303,18 +304,17 @@ static void cherryview_load_csc_matrix(struct
>intel_crtc_state *crtc_state)
>> I915_WRITE(CGM_PIPE_MODE(pipe), mode); }
>>
>> -void intel_color_set_csc(struct intel_crtc_state *crtc_state)
>> +void intel_color_set_csc(const struct intel_crtc_state *crtc_state)
>> {
>> -struct drm_device *dev = crtc_state->base.crtc->dev;
>> -struct drm_i915_private *dev_priv = to_i915(dev);
>> +struct drm_i915_private *dev_priv =
>> +to_i915(crtc_state->base.crtc->dev);
>>
>> if
Re: [Intel-gfx] [PATCH 04/13] drm/i915: Constify the state arguments to the color management stuff
On Fri, Jan 11, 2019 at 07:08:14PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä
>
> Pass the crtc state etc. as const to the color management commit
> functions. And while at it polish some of the local variables.
>
> Signed-off-by: Ville Syrjälä
Reviewed-by: Matt Roper
> ---
> drivers/gpu/drm/i915/i915_drv.h| 4 +-
> drivers/gpu/drm/i915/intel_color.c | 128 -
> drivers/gpu/drm/i915/intel_drv.h | 4 +-
> 3 files changed, 73 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5df26ccda8a4..7182a580002c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -320,8 +320,8 @@ struct drm_i915_display_funcs {
> /* display clock increase/decrease */
> /* pll clock increase/decrease */
>
> - void (*load_csc_matrix)(struct intel_crtc_state *crtc_state);
> - void (*load_luts)(struct intel_crtc_state *crtc_state);
> + void (*load_csc_matrix)(const struct intel_crtc_state *crtc_state);
> + void (*load_luts)(const struct intel_crtc_state *crtc_state);
> };
>
> #define CSR_VERSION(major, minor)((major) << 16 | (minor))
> diff --git a/drivers/gpu/drm/i915/intel_color.c
> b/drivers/gpu/drm/i915/intel_color.c
> index b10e66ce3970..0dfd104b89d7 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -74,12 +74,12 @@
> #define ILK_CSC_COEFF_1_0\
> ((7 << 12) | ILK_CSC_COEFF_FP(CTM_COEFF_1_0, 8))
>
> -static bool lut_is_legacy(struct drm_property_blob *lut)
> +static bool lut_is_legacy(const struct drm_property_blob *lut)
> {
> return drm_color_lut_size(lut) == LEGACY_LUT_LENGTH;
> }
>
> -static bool crtc_state_is_legacy_gamma(struct intel_crtc_state *crtc_state)
> +static bool crtc_state_is_legacy_gamma(const struct intel_crtc_state
> *crtc_state)
> {
> return !crtc_state->base.degamma_lut &&
> !crtc_state->base.ctm &&
> @@ -115,8 +115,8 @@ static u64 *ctm_mult_by_limited(u64 *result, const u64
> *input)
>
> static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc)
> {
> - int pipe = crtc->pipe;
> struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
>
> I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
> I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
> @@ -137,13 +137,14 @@ static void ilk_load_ycbcr_conversion_matrix(struct
> intel_crtc *crtc)
> I915_WRITE(PIPE_CSC_MODE(pipe), 0);
> }
>
> -static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
> +static void ilk_load_csc_matrix(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);
> - int i, pipe = crtc->pipe;
> - uint16_t coeffs[9] = { 0, };
> bool limited_color_range = false;
> + enum pipe pipe = crtc->pipe;
> + u16 coeffs[9] = {};
> + int i;
>
> /*
>* FIXME if there's a gamma LUT after the CSC, we should
> @@ -256,15 +257,15 @@ static void ilk_load_csc_matrix(struct intel_crtc_state
> *crtc_state)
> /*
> * Set up the pipe CSC unit on CherryView.
> */
> -static void cherryview_load_csc_matrix(struct intel_crtc_state *crtc_state)
> +static void cherryview_load_csc_matrix(const struct intel_crtc_state
> *crtc_state)
> {
> - struct drm_device *dev = crtc_state->base.crtc->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> - int pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> + enum pipe pipe = crtc->pipe;
> uint32_t mode;
>
> if (crtc_state->base.ctm) {
> - struct drm_color_ctm *ctm = crtc_state->base.ctm->data;
> + const struct drm_color_ctm *ctm = crtc_state->base.ctm->data;
> uint16_t coeffs[9] = { 0, };
> int i;
>
> @@ -303,18 +304,17 @@ static void cherryview_load_csc_matrix(struct
> intel_crtc_state *crtc_state)
> I915_WRITE(CGM_PIPE_MODE(pipe), mode);
> }
>
> -void intel_color_set_csc(struct intel_crtc_state *crtc_state)
> +void intel_color_set_csc(const struct intel_crtc_state *crtc_state)
> {
> - struct drm_device *dev = crtc_state->base.crtc->dev;
> - struct drm_i915_private *dev_priv = to_i915(dev);
> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>
> if (dev_priv->display.load_csc_matrix)
> dev_priv->display.load_csc_matrix(crtc_state);
> }
>
> /* Loads the legacy palette/gamma unit for the CRTC. */
> -static void i9xx_load_luts_internal(struct intel_crtc_state *crtc_state,
> - struct drm_property_blob *blob)
> +static void i9xx_load_luts_internal(const
[Intel-gfx] [PATCH 04/13] drm/i915: Constify the state arguments to the color management stuff
From: Ville Syrjälä
Pass the crtc state etc. as const to the color management commit
functions. And while at it polish some of the local variables.
Signed-off-by: Ville Syrjälä
---
drivers/gpu/drm/i915/i915_drv.h| 4 +-
drivers/gpu/drm/i915/intel_color.c | 128 -
drivers/gpu/drm/i915/intel_drv.h | 4 +-
3 files changed, 73 insertions(+), 63 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5df26ccda8a4..7182a580002c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -320,8 +320,8 @@ struct drm_i915_display_funcs {
/* display clock increase/decrease */
/* pll clock increase/decrease */
- void (*load_csc_matrix)(struct intel_crtc_state *crtc_state);
- void (*load_luts)(struct intel_crtc_state *crtc_state);
+ void (*load_csc_matrix)(const struct intel_crtc_state *crtc_state);
+ void (*load_luts)(const struct intel_crtc_state *crtc_state);
};
#define CSR_VERSION(major, minor) ((major) << 16 | (minor))
diff --git a/drivers/gpu/drm/i915/intel_color.c
b/drivers/gpu/drm/i915/intel_color.c
index b10e66ce3970..0dfd104b89d7 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -74,12 +74,12 @@
#define ILK_CSC_COEFF_1_0 \
((7 << 12) | ILK_CSC_COEFF_FP(CTM_COEFF_1_0, 8))
-static bool lut_is_legacy(struct drm_property_blob *lut)
+static bool lut_is_legacy(const struct drm_property_blob *lut)
{
return drm_color_lut_size(lut) == LEGACY_LUT_LENGTH;
}
-static bool crtc_state_is_legacy_gamma(struct intel_crtc_state *crtc_state)
+static bool crtc_state_is_legacy_gamma(const struct intel_crtc_state
*crtc_state)
{
return !crtc_state->base.degamma_lut &&
!crtc_state->base.ctm &&
@@ -115,8 +115,8 @@ static u64 *ctm_mult_by_limited(u64 *result, const u64
*input)
static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc)
{
- int pipe = crtc->pipe;
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
@@ -137,13 +137,14 @@ static void ilk_load_ycbcr_conversion_matrix(struct
intel_crtc *crtc)
I915_WRITE(PIPE_CSC_MODE(pipe), 0);
}
-static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
+static void ilk_load_csc_matrix(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);
- int i, pipe = crtc->pipe;
- uint16_t coeffs[9] = { 0, };
bool limited_color_range = false;
+ enum pipe pipe = crtc->pipe;
+ u16 coeffs[9] = {};
+ int i;
/*
* FIXME if there's a gamma LUT after the CSC, we should
@@ -256,15 +257,15 @@ static void ilk_load_csc_matrix(struct intel_crtc_state
*crtc_state)
/*
* Set up the pipe CSC unit on CherryView.
*/
-static void cherryview_load_csc_matrix(struct intel_crtc_state *crtc_state)
+static void cherryview_load_csc_matrix(const struct intel_crtc_state
*crtc_state)
{
- struct drm_device *dev = crtc_state->base.crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
- int pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+ struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+ struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+ enum pipe pipe = crtc->pipe;
uint32_t mode;
if (crtc_state->base.ctm) {
- struct drm_color_ctm *ctm = crtc_state->base.ctm->data;
+ const struct drm_color_ctm *ctm = crtc_state->base.ctm->data;
uint16_t coeffs[9] = { 0, };
int i;
@@ -303,18 +304,17 @@ static void cherryview_load_csc_matrix(struct
intel_crtc_state *crtc_state)
I915_WRITE(CGM_PIPE_MODE(pipe), mode);
}
-void intel_color_set_csc(struct intel_crtc_state *crtc_state)
+void intel_color_set_csc(const struct intel_crtc_state *crtc_state)
{
- struct drm_device *dev = crtc_state->base.crtc->dev;
- struct drm_i915_private *dev_priv = to_i915(dev);
+ struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
if (dev_priv->display.load_csc_matrix)
dev_priv->display.load_csc_matrix(crtc_state);
}
/* Loads the legacy palette/gamma unit for the CRTC. */
-static void i9xx_load_luts_internal(struct intel_crtc_state *crtc_state,
- struct drm_property_blob *blob)
+static void i9xx_load_luts_internal(const struct intel_crtc_state *crtc_state,
+ const struct drm_property_blob *blob)
{
struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
struct drm_i915_private *dev_priv =
