diff mbox

drm/core: Change declaration for gamma_set.

Message ID 1465204689-24489-1-git-send-email-maarten.lankhorst@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maarten Lankhorst June 6, 2016, 9:18 a.m. UTC
Change return value to int to propagate errors from gamma_set,
and remove start parameter. Updates always use the full size,
and some drivers even ignore the start parameter altogether.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Christian König <christian.koenig@amd.com>
Cc: David Airlie <airlied@linux.ie>
Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
Cc: Mathieu Larouche <mathieu.larouche@matrox.com>
Cc: Thierry Reding <treding@nvidia.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  | 10 ++++++----
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  | 10 ++++++----
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   | 10 ++++++----
 drivers/gpu/drm/ast/ast_mode.c          | 10 ++++++----
 drivers/gpu/drm/cirrus/cirrus_mode.c    |  8 +++++---
 drivers/gpu/drm/drm_atomic_helper.c     | 13 ++++++-------
 drivers/gpu/drm/drm_crtc.c              |  2 +-
 drivers/gpu/drm/drm_fb_helper.c         |  4 +---
 drivers/gpu/drm/gma500/gma_display.c    |  9 +++++----
 drivers/gpu/drm/gma500/gma_display.h    |  4 ++--
 drivers/gpu/drm/mgag200/mgag200_mode.c  |  9 +++++----
 drivers/gpu/drm/nouveau/dispnv04/crtc.c | 12 +++++++-----
 drivers/gpu/drm/nouveau/nv50_display.c  |  9 +++++----
 drivers/gpu/drm/radeon/radeon_display.c | 11 +++++++----
 drivers/gpu/drm/vc4/vc4_crtc.c          |  8 +++++---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c     |  8 +++++---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h     |  4 ++--
 include/drm/drm_atomic_helper.h         |  6 +++---
 include/drm/drm_crtc.h                  |  4 ++--
 19 files changed, 85 insertions(+), 66 deletions(-)

Comments

Daniel Vetter June 6, 2016, 4 p.m. UTC | #1
On Mon, Jun 06, 2016 at 11:18:09AM +0200, Maarten Lankhorst wrote:
> Change return value to int to propagate errors from gamma_set,
> and remove start parameter. Updates always use the full size,
> and some drivers even ignore the start parameter altogether.

Commit message should explain why we suddenly need to pass up the error
code, too:

"This is needed for atomic drivers, where an atomic commit can fail with
EAGAIN and should be restarted."

I'll add that when merging, but will wait for a bit more acks/r-b first.
-Daniel
> 
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Christian König <christian.koenig@amd.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> Cc: Mathieu Larouche <mathieu.larouche@matrox.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  | 10 ++++++----
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  | 10 ++++++----
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   | 10 ++++++----
>  drivers/gpu/drm/ast/ast_mode.c          | 10 ++++++----
>  drivers/gpu/drm/cirrus/cirrus_mode.c    |  8 +++++---
>  drivers/gpu/drm/drm_atomic_helper.c     | 13 ++++++-------
>  drivers/gpu/drm/drm_crtc.c              |  2 +-
>  drivers/gpu/drm/drm_fb_helper.c         |  4 +---
>  drivers/gpu/drm/gma500/gma_display.c    |  9 +++++----
>  drivers/gpu/drm/gma500/gma_display.h    |  4 ++--
>  drivers/gpu/drm/mgag200/mgag200_mode.c  |  9 +++++----
>  drivers/gpu/drm/nouveau/dispnv04/crtc.c | 12 +++++++-----
>  drivers/gpu/drm/nouveau/nv50_display.c  |  9 +++++----
>  drivers/gpu/drm/radeon/radeon_display.c | 11 +++++++----
>  drivers/gpu/drm/vc4/vc4_crtc.c          |  8 +++++---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c     |  8 +++++---
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h     |  4 ++--
>  include/drm/drm_atomic_helper.h         |  6 +++---
>  include/drm/drm_crtc.h                  |  4 ++--
>  19 files changed, 85 insertions(+), 66 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 8227344d2ff6..16de862ea51f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -2667,19 +2667,21 @@ static void dce_v10_0_cursor_reset(struct drm_crtc *crtc)
>  	}
>  }
>  
> -static void dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				    u16 *blue, uint32_t start, uint32_t size)
> +static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> +				    u16 *blue, uint32_t size)
>  {
>  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -	int end = (start + size > 256) ? 256 : start + size, i;
> +	int i;
>  
>  	/* userspace palettes are always correct as is */
> -	for (i = start; i < end; i++) {
> +	for (i = 0; i < size; i++) {
>  		amdgpu_crtc->lut_r[i] = red[i] >> 6;
>  		amdgpu_crtc->lut_g[i] = green[i] >> 6;
>  		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
>  	}
>  	dce_v10_0_crtc_load_lut(crtc);
> +
> +	return 0;
>  }
>  
>  static void dce_v10_0_crtc_destroy(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index af26ec0bc59d..6e0ae8e2e65c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -2678,19 +2678,21 @@ static void dce_v11_0_cursor_reset(struct drm_crtc *crtc)
>  	}
>  }
>  
> -static void dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				    u16 *blue, uint32_t start, uint32_t size)
> +static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> +				    u16 *blue, uint32_t size)
>  {
>  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -	int end = (start + size > 256) ? 256 : start + size, i;
> +	int i;
>  
>  	/* userspace palettes are always correct as is */
> -	for (i = start; i < end; i++) {
> +	for (i = 0; i < size; i++) {
>  		amdgpu_crtc->lut_r[i] = red[i] >> 6;
>  		amdgpu_crtc->lut_g[i] = green[i] >> 6;
>  		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
>  	}
>  	dce_v11_0_crtc_load_lut(crtc);
> +
> +	return 0;
>  }
>  
>  static void dce_v11_0_crtc_destroy(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index 3fb65e41a6ef..7b5fae4c74f1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -2574,19 +2574,21 @@ static void dce_v8_0_cursor_reset(struct drm_crtc *crtc)
>  	}
>  }
>  
> -static void dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				    u16 *blue, uint32_t start, uint32_t size)
> +static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> +				   u16 *blue, uint32_t size)
>  {
>  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> -	int end = (start + size > 256) ? 256 : start + size, i;
> +	int i;
>  
>  	/* userspace palettes are always correct as is */
> -	for (i = start; i < end; i++) {
> +	for (i = 0; i < size; i++) {
>  		amdgpu_crtc->lut_r[i] = red[i] >> 6;
>  		amdgpu_crtc->lut_g[i] = green[i] >> 6;
>  		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
>  	}
>  	dce_v8_0_crtc_load_lut(crtc);
> +
> +	return 0;
>  }
>  
>  static void dce_v8_0_crtc_destroy(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index c337922606e3..5957c3e659fe 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -624,19 +624,21 @@ static void ast_crtc_reset(struct drm_crtc *crtc)
>  
>  }
>  
> -static void ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				 u16 *blue, uint32_t start, uint32_t size)
> +static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> +			      u16 *blue, uint32_t size)
>  {
>  	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> -	int end = (start + size > 256) ? 256 : start + size, i;
> +	int i;
>  
>  	/* userspace palettes are always correct as is */
> -	for (i = start; i < end; i++) {
> +	for (i = 0; i < size; i++) {
>  		ast_crtc->lut_r[i] = red[i] >> 8;
>  		ast_crtc->lut_g[i] = green[i] >> 8;
>  		ast_crtc->lut_b[i] = blue[i] >> 8;
>  	}
>  	ast_crtc_load_lut(crtc);
> +
> +	return 0;
>  }
>  
>  
> diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
> index 0b1a411cb89e..17c915d9a03e 100644
> --- a/drivers/gpu/drm/cirrus/cirrus_mode.c
> +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
> @@ -325,18 +325,20 @@ static void cirrus_crtc_commit(struct drm_crtc *crtc)
>   * use this for 8-bit mode so can't perform smooth fades on deeper modes,
>   * but it's a requirement that we provide the function
>   */
> -static void cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				  u16 *blue, uint32_t start, uint32_t size)
> +static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> +				 u16 *blue, uint32_t size)
>  {
>  	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
>  	int i;
>  
> -	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
> +	for (i = 0; i < size; i++) {
>  		cirrus_crtc->lut_r[i] = red[i];
>  		cirrus_crtc->lut_g[i] = green[i];
>  		cirrus_crtc->lut_b[i] = blue[i];
>  	}
>  	cirrus_crtc_load_lut(crtc);
> +
> +	return 0;
>  }
>  
>  /* Simple cleanup function */
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 939df900dcaa..266c11018a19 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2933,16 +2933,15 @@ EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
>   * @red: red correction table
>   * @green: green correction table
>   * @blue: green correction table
> - * @start:
>   * @size: size of the tables
>   *
>   * Implements support for legacy gamma correction table for drivers
>   * that support color management through the DEGAMMA_LUT/GAMMA_LUT
>   * properties.
>   */
> -void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> -					u16 *red, u16 *green, u16 *blue,
> -					uint32_t start, uint32_t size)
> +int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> +				       u16 *red, u16 *green, u16 *blue,
> +				       uint32_t size)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_mode_config *config = &dev->mode_config;
> @@ -2954,7 +2953,7 @@ void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
>  
>  	state = drm_atomic_state_alloc(crtc->dev);
>  	if (!state)
> -		return;
> +		return -ENOMEM;
>  
>  	blob = drm_property_create_blob(dev,
>  					sizeof(struct drm_color_lut) * size,
> @@ -3005,7 +3004,7 @@ retry:
>  
>  	drm_property_unreference_blob(blob);
>  
> -	return;
> +	return 0;
>  fail:
>  	if (ret == -EDEADLK)
>  		goto backoff;
> @@ -3013,7 +3012,7 @@ fail:
>  	drm_atomic_state_free(state);
>  	drm_property_unreference_blob(blob);
>  
> -	return;
> +	return ret;
>  backoff:
>  	drm_atomic_state_clear(state);
>  	drm_atomic_legacy_backoff(state);
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d2a6d958ca76..4d61ceae789c 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -5215,7 +5215,7 @@ int drm_mode_gamma_set_ioctl(struct drm_device *dev,
>  		goto out;
>  	}
>  
> -	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, crtc->gamma_size);
> +	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc->gamma_size);
>  
>  out:
>  	drm_modeset_unlock_all(dev);
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 7590df5e2e72..4accf6039058 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -227,7 +227,7 @@ static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
>  	g_base = r_base + crtc->gamma_size;
>  	b_base = g_base + crtc->gamma_size;
>  
> -	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, crtc->gamma_size);
> +	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc->gamma_size);
>  }
>  
>  /**
> @@ -1076,8 +1076,6 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>  	WARN_ON(fb->bits_per_pixel != 8);
>  
>  	fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> -
> -	return 0;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
> index c95406e6f44d..5b636bf0b467 100644
> --- a/drivers/gpu/drm/gma500/gma_display.c
> +++ b/drivers/gpu/drm/gma500/gma_display.c
> @@ -175,20 +175,21 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
>  	}
>  }
>  
> -void gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
> -			u32 start, u32 size)
> +int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
> +		       u32 size)
>  {
>  	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
>  	int i;
> -	int end = (start + size > 256) ? 256 : start + size;
>  
> -	for (i = start; i < end; i++) {
> +	for (i = 0; i < size; i++) {
>  		gma_crtc->lut_r[i] = red[i] >> 8;
>  		gma_crtc->lut_g[i] = green[i] >> 8;
>  		gma_crtc->lut_b[i] = blue[i] >> 8;
>  	}
>  
>  	gma_crtc_load_lut(crtc);
> +
> +	return 0;
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/gma500/gma_display.h b/drivers/gpu/drm/gma500/gma_display.h
> index b2491c65f053..e72dd08b701b 100644
> --- a/drivers/gpu/drm/gma500/gma_display.h
> +++ b/drivers/gpu/drm/gma500/gma_display.h
> @@ -72,8 +72,8 @@ extern int gma_crtc_cursor_set(struct drm_crtc *crtc,
>  			       uint32_t width, uint32_t height);
>  extern int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
>  extern void gma_crtc_load_lut(struct drm_crtc *crtc);
> -extern void gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -			       u16 *blue, u32 start, u32 size);
> +extern int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> +			      u16 *blue, u32 size);
>  extern void gma_crtc_dpms(struct drm_crtc *crtc, int mode);
>  extern void gma_crtc_prepare(struct drm_crtc *crtc);
>  extern void gma_crtc_commit(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
> index 14e64e08909e..f6d5892d03f1 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> @@ -1344,19 +1344,20 @@ static void mga_crtc_commit(struct drm_crtc *crtc)
>   * use this for 8-bit mode so can't perform smooth fades on deeper modes,
>   * but it's a requirement that we provide the function
>   */
> -static void mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				  u16 *blue, uint32_t start, uint32_t size)
> +static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> +			      u16 *blue, uint32_t size)
>  {
>  	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
> -	int end = (start + size > MGAG200_LUT_SIZE) ? MGAG200_LUT_SIZE : start + size;
>  	int i;
>  
> -	for (i = start; i < end; i++) {
> +	for (i = 0; i < size; i++) {
>  		mga_crtc->lut_r[i] = red[i] >> 8;
>  		mga_crtc->lut_g[i] = green[i] >> 8;
>  		mga_crtc->lut_b[i] = blue[i] >> 8;
>  	}
>  	mga_crtc_load_lut(crtc);
> +
> +	return 0;
>  }
>  
>  /* Simple cleanup function */
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> index 6f318c54da33..0cb7a18cde26 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> @@ -785,14 +785,14 @@ nv_crtc_disable(struct drm_crtc *crtc)
>  	nouveau_bo_ref(NULL, &disp->image[nv_crtc->index]);
>  }
>  
> -static void
> -nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, uint32_t start,
> +static int
> +nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
>  		  uint32_t size)
>  {
> -	int end = (start + size > 256) ? 256 : start + size, i;
>  	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
> +	int i;
>  
> -	for (i = start; i < end; i++) {
> +	for (i = 0; i < size; i++) {
>  		nv_crtc->lut.r[i] = r[i];
>  		nv_crtc->lut.g[i] = g[i];
>  		nv_crtc->lut.b[i] = b[i];
> @@ -805,10 +805,12 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, uint32_t start,
>  	 */
>  	if (!nv_crtc->base.primary->fb) {
>  		nv_crtc->lut.depth = 0;
> -		return;
> +		return 0;
>  	}
>  
>  	nv_crtc_gamma_load(crtc);
> +
> +	return 0;
>  }
>  
>  static int
> diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
> index 3ffc2b0057bf..7a7788212df7 100644
> --- a/drivers/gpu/drm/nouveau/nv50_display.c
> +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> @@ -1346,21 +1346,22 @@ nv50_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
>  	return 0;
>  }
>  
> -static void
> +static int
>  nv50_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> -		    uint32_t start, uint32_t size)
> +		    uint32_t size)
>  {
>  	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
> -	u32 end = min_t(u32, start + size, 256);
>  	u32 i;
>  
> -	for (i = start; i < end; i++) {
> +	for (i = 0; i < size; i++) {
>  		nv_crtc->lut.r[i] = r[i];
>  		nv_crtc->lut.g[i] = g[i];
>  		nv_crtc->lut.b[i] = b[i];
>  	}
>  
>  	nv50_crtc_lut_load(crtc);
> +
> +	return 0;
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
> index 6a41b4982647..80f06db1dd34 100644
> --- a/drivers/gpu/drm/radeon/radeon_display.c
> +++ b/drivers/gpu/drm/radeon/radeon_display.c
> @@ -231,19 +231,21 @@ void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
>  	*blue = radeon_crtc->lut_b[regno] << 6;
>  }
>  
> -static void radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> -				  u16 *blue, uint32_t start, uint32_t size)
> +static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> +				 u16 *blue, uint32_t size)
>  {
>  	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> -	int end = (start + size > 256) ? 256 : start + size, i;
> +	int i;
>  
>  	/* userspace palettes are always correct as is */
> -	for (i = start; i < end; i++) {
> +	for (i = 0; i < size; i++) {
>  		radeon_crtc->lut_r[i] = red[i] >> 6;
>  		radeon_crtc->lut_g[i] = green[i] >> 6;
>  		radeon_crtc->lut_b[i] = blue[i] >> 6;
>  	}
>  	radeon_crtc_load_lut(crtc);
> +
> +	return 0;
>  }
>  
>  static void radeon_crtc_destroy(struct drm_crtc *crtc)
> @@ -688,6 +690,7 @@ radeon_crtc_set_config(struct drm_mode_set *set)
>  	pm_runtime_put_autosuspend(dev->dev);
>  	return ret;
>  }
> +
>  static const struct drm_crtc_funcs radeon_crtc_funcs = {
>  	.cursor_set2 = radeon_crtc_cursor_set2,
>  	.cursor_move = radeon_crtc_cursor_move,
> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
> index 904d0754ad78..4e34f054da5c 100644
> --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> @@ -175,20 +175,22 @@ vc4_crtc_lut_load(struct drm_crtc *crtc)
>  		HVS_WRITE(SCALER_GAMDATA, vc4_crtc->lut_b[i]);
>  }
>  
> -static void
> +static int
>  vc4_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> -		   uint32_t start, uint32_t size)
> +		   uint32_t size)
>  {
>  	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
>  	u32 i;
>  
> -	for (i = start; i < start + size; i++) {
> +	for (i = 0; i < size; i++) {
>  		vc4_crtc->lut_r[i] = r[i] >> 8;
>  		vc4_crtc->lut_g[i] = g[i] >> 8;
>  		vc4_crtc->lut_b[i] = b[i] >> 8;
>  	}
>  
>  	vc4_crtc_lut_load(crtc);
> +
> +	return 0;
>  }
>  
>  static u32 vc4_get_fifo_full_level(u32 format)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 55231cce73a0..8a69d4da40b5 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1404,9 +1404,9 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
>  	return 0;
>  }
>  
> -void vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
> -			   u16 *r, u16 *g, u16 *b,
> -			   uint32_t start, uint32_t size)
> +int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
> +			  u16 *r, u16 *g, u16 *b,
> +			  uint32_t size)
>  {
>  	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
>  	int i;
> @@ -1418,6 +1418,8 @@ void vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
>  		vmw_write(dev_priv, SVGA_PALETTE_BASE + i * 3 + 1, g[i] >> 8);
>  		vmw_write(dev_priv, SVGA_PALETTE_BASE + i * 3 + 2, b[i] >> 8);
>  	}
> +
> +	return 0;
>  }
>  
>  int vmw_du_connector_dpms(struct drm_connector *connector, int mode)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index 57203212c501..ff4803c107bc 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -195,9 +195,9 @@ struct vmw_display_unit {
>  void vmw_du_cleanup(struct vmw_display_unit *du);
>  void vmw_du_crtc_save(struct drm_crtc *crtc);
>  void vmw_du_crtc_restore(struct drm_crtc *crtc);
> -void vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
> +int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
>  			   u16 *r, u16 *g, u16 *b,
> -			   uint32_t start, uint32_t size);
> +			   uint32_t size);
>  int vmw_du_crtc_cursor_set2(struct drm_crtc *crtc, struct drm_file *file_priv,
>  			    uint32_t handle, uint32_t width, uint32_t height,
>  			    int32_t hot_x, int32_t hot_y);
> diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
> index d473dcc91f54..11896c84c4f8 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -147,9 +147,9 @@ void
>  __drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state);
>  void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
>  					  struct drm_connector_state *state);
> -void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> -					u16 *red, u16 *green, u16 *blue,
> -					uint32_t start, uint32_t size);
> +int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> +				       u16 *red, u16 *green, u16 *blue,
> +				       uint32_t size);
>  
>  /**
>   * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index d1559cd04e3d..8622bb2f4f25 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -478,8 +478,8 @@ struct drm_crtc_funcs {
>  	 * going on, which should eventually be unified to just one set of
>  	 * hooks.
>  	 */
> -	void (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> -			  uint32_t start, uint32_t size);
> +	int (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> +			 uint32_t size);
>  
>  	/**
>  	 * @destroy:
> -- 
> 2.5.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Alex Deucher June 6, 2016, 4:01 p.m. UTC | #2
> -----Original Message-----
> From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel
> Vetter
> Sent: Monday, June 06, 2016 12:00 PM
> To: Maarten Lankhorst
> Cc: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org;
> Mathieu Larouche; VMware Graphics; Ben Skeggs; Deucher, Alexander;
> Thierry Reding; Koenig, Christian
> Subject: Re: [PATCH] drm/core: Change declaration for gamma_set.
> 
> On Mon, Jun 06, 2016 at 11:18:09AM +0200, Maarten Lankhorst wrote:
> > Change return value to int to propagate errors from gamma_set,
> > and remove start parameter. Updates always use the full size,
> > and some drivers even ignore the start parameter altogether.
> 
> Commit message should explain why we suddenly need to pass up the error
> code, too:
> 
> "This is needed for atomic drivers, where an atomic commit can fail with
> EAGAIN and should be restarted."
> 
> I'll add that when merging, but will wait for a bit more acks/r-b first.

Thanks.  I was just about to ask why this was necessary.   Patch is:
Acked-by: Alex Deucher <alexander.deucher@amd.com>

> -Daniel
> >
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Cc: David Airlie <airlied@linux.ie>
> > Cc: Patrik Jakobsson <patrik.r.jakobsson@gmail.com>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > Cc: Mathieu Larouche <mathieu.larouche@matrox.com>
> > Cc: Thierry Reding <treding@nvidia.com>
> > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c  | 10 ++++++----
> >  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c  | 10 ++++++----
> >  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c   | 10 ++++++----
> >  drivers/gpu/drm/ast/ast_mode.c          | 10 ++++++----
> >  drivers/gpu/drm/cirrus/cirrus_mode.c    |  8 +++++---
> >  drivers/gpu/drm/drm_atomic_helper.c     | 13 ++++++-------
> >  drivers/gpu/drm/drm_crtc.c              |  2 +-
> >  drivers/gpu/drm/drm_fb_helper.c         |  4 +---
> >  drivers/gpu/drm/gma500/gma_display.c    |  9 +++++----
> >  drivers/gpu/drm/gma500/gma_display.h    |  4 ++--
> >  drivers/gpu/drm/mgag200/mgag200_mode.c  |  9 +++++----
> >  drivers/gpu/drm/nouveau/dispnv04/crtc.c | 12 +++++++-----
> >  drivers/gpu/drm/nouveau/nv50_display.c  |  9 +++++----
> >  drivers/gpu/drm/radeon/radeon_display.c | 11 +++++++----
> >  drivers/gpu/drm/vc4/vc4_crtc.c          |  8 +++++---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c     |  8 +++++---
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h     |  4 ++--
> >  include/drm/drm_atomic_helper.h         |  6 +++---
> >  include/drm/drm_crtc.h                  |  4 ++--
> >  19 files changed, 85 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > index 8227344d2ff6..16de862ea51f 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > @@ -2667,19 +2667,21 @@ static void dce_v10_0_cursor_reset(struct
> drm_crtc *crtc)
> >  	}
> >  }
> >
> > -static void dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
> u16 *green,
> > -				    u16 *blue, uint32_t start, uint32_t size)
> > +static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > +				    u16 *blue, uint32_t size)
> >  {
> >  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> > -	int end = (start + size > 256) ? 256 : start + size, i;
> > +	int i;
> >
> >  	/* userspace palettes are always correct as is */
> > -	for (i = start; i < end; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		amdgpu_crtc->lut_r[i] = red[i] >> 6;
> >  		amdgpu_crtc->lut_g[i] = green[i] >> 6;
> >  		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> >  	}
> >  	dce_v10_0_crtc_load_lut(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  static void dce_v10_0_crtc_destroy(struct drm_crtc *crtc)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > index af26ec0bc59d..6e0ae8e2e65c 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > @@ -2678,19 +2678,21 @@ static void dce_v11_0_cursor_reset(struct
> drm_crtc *crtc)
> >  	}
> >  }
> >
> > -static void dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red,
> u16 *green,
> > -				    u16 *blue, uint32_t start, uint32_t size)
> > +static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > +				    u16 *blue, uint32_t size)
> >  {
> >  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> > -	int end = (start + size > 256) ? 256 : start + size, i;
> > +	int i;
> >
> >  	/* userspace palettes are always correct as is */
> > -	for (i = start; i < end; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		amdgpu_crtc->lut_r[i] = red[i] >> 6;
> >  		amdgpu_crtc->lut_g[i] = green[i] >> 6;
> >  		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> >  	}
> >  	dce_v11_0_crtc_load_lut(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  static void dce_v11_0_crtc_destroy(struct drm_crtc *crtc)
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > index 3fb65e41a6ef..7b5fae4c74f1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > @@ -2574,19 +2574,21 @@ static void dce_v8_0_cursor_reset(struct
> drm_crtc *crtc)
> >  	}
> >  }
> >
> > -static void dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > -				    u16 *blue, uint32_t start, uint32_t size)
> > +static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > +				   u16 *blue, uint32_t size)
> >  {
> >  	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
> > -	int end = (start + size > 256) ? 256 : start + size, i;
> > +	int i;
> >
> >  	/* userspace palettes are always correct as is */
> > -	for (i = start; i < end; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		amdgpu_crtc->lut_r[i] = red[i] >> 6;
> >  		amdgpu_crtc->lut_g[i] = green[i] >> 6;
> >  		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
> >  	}
> >  	dce_v8_0_crtc_load_lut(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  static void dce_v8_0_crtc_destroy(struct drm_crtc *crtc)
> > diff --git a/drivers/gpu/drm/ast/ast_mode.c
> b/drivers/gpu/drm/ast/ast_mode.c
> > index c337922606e3..5957c3e659fe 100644
> > --- a/drivers/gpu/drm/ast/ast_mode.c
> > +++ b/drivers/gpu/drm/ast/ast_mode.c
> > @@ -624,19 +624,21 @@ static void ast_crtc_reset(struct drm_crtc *crtc)
> >
> >  }
> >
> > -static void ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > -				 u16 *blue, uint32_t start, uint32_t size)
> > +static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > +			      u16 *blue, uint32_t size)
> >  {
> >  	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
> > -	int end = (start + size > 256) ? 256 : start + size, i;
> > +	int i;
> >
> >  	/* userspace palettes are always correct as is */
> > -	for (i = start; i < end; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		ast_crtc->lut_r[i] = red[i] >> 8;
> >  		ast_crtc->lut_g[i] = green[i] >> 8;
> >  		ast_crtc->lut_b[i] = blue[i] >> 8;
> >  	}
> >  	ast_crtc_load_lut(crtc);
> > +
> > +	return 0;
> >  }
> >
> >
> > diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c
> b/drivers/gpu/drm/cirrus/cirrus_mode.c
> > index 0b1a411cb89e..17c915d9a03e 100644
> > --- a/drivers/gpu/drm/cirrus/cirrus_mode.c
> > +++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
> > @@ -325,18 +325,20 @@ static void cirrus_crtc_commit(struct drm_crtc
> *crtc)
> >   * use this for 8-bit mode so can't perform smooth fades on deeper
> modes,
> >   * but it's a requirement that we provide the function
> >   */
> > -static void cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > -				  u16 *blue, uint32_t start, uint32_t size)
> > +static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > +				 u16 *blue, uint32_t size)
> >  {
> >  	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
> >  	int i;
> >
> > -	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		cirrus_crtc->lut_r[i] = red[i];
> >  		cirrus_crtc->lut_g[i] = green[i];
> >  		cirrus_crtc->lut_b[i] = blue[i];
> >  	}
> >  	cirrus_crtc_load_lut(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  /* Simple cleanup function */
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> > index 939df900dcaa..266c11018a19 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -2933,16 +2933,15 @@
> EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
> >   * @red: red correction table
> >   * @green: green correction table
> >   * @blue: green correction table
> > - * @start:
> >   * @size: size of the tables
> >   *
> >   * Implements support for legacy gamma correction table for drivers
> >   * that support color management through the
> DEGAMMA_LUT/GAMMA_LUT
> >   * properties.
> >   */
> > -void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> > -					u16 *red, u16 *green, u16 *blue,
> > -					uint32_t start, uint32_t size)
> > +int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> > +				       u16 *red, u16 *green, u16 *blue,
> > +				       uint32_t size)
> >  {
> >  	struct drm_device *dev = crtc->dev;
> >  	struct drm_mode_config *config = &dev->mode_config;
> > @@ -2954,7 +2953,7 @@ void
> drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> >
> >  	state = drm_atomic_state_alloc(crtc->dev);
> >  	if (!state)
> > -		return;
> > +		return -ENOMEM;
> >
> >  	blob = drm_property_create_blob(dev,
> >  					sizeof(struct drm_color_lut) * size,
> > @@ -3005,7 +3004,7 @@ retry:
> >
> >  	drm_property_unreference_blob(blob);
> >
> > -	return;
> > +	return 0;
> >  fail:
> >  	if (ret == -EDEADLK)
> >  		goto backoff;
> > @@ -3013,7 +3012,7 @@ fail:
> >  	drm_atomic_state_free(state);
> >  	drm_property_unreference_blob(blob);
> >
> > -	return;
> > +	return ret;
> >  backoff:
> >  	drm_atomic_state_clear(state);
> >  	drm_atomic_legacy_backoff(state);
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index d2a6d958ca76..4d61ceae789c 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -5215,7 +5215,7 @@ int drm_mode_gamma_set_ioctl(struct
> drm_device *dev,
> >  		goto out;
> >  	}
> >
> > -	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, crtc-
> >gamma_size);
> > +	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc-
> >gamma_size);
> >
> >  out:
> >  	drm_modeset_unlock_all(dev);
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c
> > index 7590df5e2e72..4accf6039058 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -227,7 +227,7 @@ static void
> drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
> >  	g_base = r_base + crtc->gamma_size;
> >  	b_base = g_base + crtc->gamma_size;
> >
> > -	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, crtc-
> >gamma_size);
> > +	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc-
> >gamma_size);
> >  }
> >
> >  /**
> > @@ -1076,8 +1076,6 @@ static int setcolreg(struct drm_crtc *crtc, u16 red,
> u16 green,
> >  	WARN_ON(fb->bits_per_pixel != 8);
> >
> >  	fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> > -
> > -	return 0;
> >  }
> >
> >  /**
> > diff --git a/drivers/gpu/drm/gma500/gma_display.c
> b/drivers/gpu/drm/gma500/gma_display.c
> > index c95406e6f44d..5b636bf0b467 100644
> > --- a/drivers/gpu/drm/gma500/gma_display.c
> > +++ b/drivers/gpu/drm/gma500/gma_display.c
> > @@ -175,20 +175,21 @@ void gma_crtc_load_lut(struct drm_crtc *crtc)
> >  	}
> >  }
> >
> > -void gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> u16 *blue,
> > -			u32 start, u32 size)
> > +int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
> u16 *blue,
> > +		       u32 size)
> >  {
> >  	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
> >  	int i;
> > -	int end = (start + size > 256) ? 256 : start + size;
> >
> > -	for (i = start; i < end; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		gma_crtc->lut_r[i] = red[i] >> 8;
> >  		gma_crtc->lut_g[i] = green[i] >> 8;
> >  		gma_crtc->lut_b[i] = blue[i] >> 8;
> >  	}
> >
> >  	gma_crtc_load_lut(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  /**
> > diff --git a/drivers/gpu/drm/gma500/gma_display.h
> b/drivers/gpu/drm/gma500/gma_display.h
> > index b2491c65f053..e72dd08b701b 100644
> > --- a/drivers/gpu/drm/gma500/gma_display.h
> > +++ b/drivers/gpu/drm/gma500/gma_display.h
> > @@ -72,8 +72,8 @@ extern int gma_crtc_cursor_set(struct drm_crtc *crtc,
> >  			       uint32_t width, uint32_t height);
> >  extern int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
> >  extern void gma_crtc_load_lut(struct drm_crtc *crtc);
> > -extern void gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > -			       u16 *blue, u32 start, u32 size);
> > +extern int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > +			      u16 *blue, u32 size);
> >  extern void gma_crtc_dpms(struct drm_crtc *crtc, int mode);
> >  extern void gma_crtc_prepare(struct drm_crtc *crtc);
> >  extern void gma_crtc_commit(struct drm_crtc *crtc);
> > diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
> b/drivers/gpu/drm/mgag200/mgag200_mode.c
> > index 14e64e08909e..f6d5892d03f1 100644
> > --- a/drivers/gpu/drm/mgag200/mgag200_mode.c
> > +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
> > @@ -1344,19 +1344,20 @@ static void mga_crtc_commit(struct drm_crtc
> *crtc)
> >   * use this for 8-bit mode so can't perform smooth fades on deeper
> modes,
> >   * but it's a requirement that we provide the function
> >   */
> > -static void mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > -				  u16 *blue, uint32_t start, uint32_t size)
> > +static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > +			      u16 *blue, uint32_t size)
> >  {
> >  	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
> > -	int end = (start + size > MGAG200_LUT_SIZE) ? MGAG200_LUT_SIZE :
> start + size;
> >  	int i;
> >
> > -	for (i = start; i < end; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		mga_crtc->lut_r[i] = red[i] >> 8;
> >  		mga_crtc->lut_g[i] = green[i] >> 8;
> >  		mga_crtc->lut_b[i] = blue[i] >> 8;
> >  	}
> >  	mga_crtc_load_lut(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  /* Simple cleanup function */
> > diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > index 6f318c54da33..0cb7a18cde26 100644
> > --- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
> > @@ -785,14 +785,14 @@ nv_crtc_disable(struct drm_crtc *crtc)
> >  	nouveau_bo_ref(NULL, &disp->image[nv_crtc->index]);
> >  }
> >
> > -static void
> > -nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, uint32_t
> start,
> > +static int
> > +nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> >  		  uint32_t size)
> >  {
> > -	int end = (start + size > 256) ? 256 : start + size, i;
> >  	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
> > +	int i;
> >
> > -	for (i = start; i < end; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		nv_crtc->lut.r[i] = r[i];
> >  		nv_crtc->lut.g[i] = g[i];
> >  		nv_crtc->lut.b[i] = b[i];
> > @@ -805,10 +805,12 @@ nv_crtc_gamma_set(struct drm_crtc *crtc, u16
> *r, u16 *g, u16 *b, uint32_t start,
> >  	 */
> >  	if (!nv_crtc->base.primary->fb) {
> >  		nv_crtc->lut.depth = 0;
> > -		return;
> > +		return 0;
> >  	}
> >
> >  	nv_crtc_gamma_load(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  static int
> > diff --git a/drivers/gpu/drm/nouveau/nv50_display.c
> b/drivers/gpu/drm/nouveau/nv50_display.c
> > index 3ffc2b0057bf..7a7788212df7 100644
> > --- a/drivers/gpu/drm/nouveau/nv50_display.c
> > +++ b/drivers/gpu/drm/nouveau/nv50_display.c
> > @@ -1346,21 +1346,22 @@ nv50_crtc_cursor_move(struct drm_crtc *crtc,
> int x, int y)
> >  	return 0;
> >  }
> >
> > -static void
> > +static int
> >  nv50_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> > -		    uint32_t start, uint32_t size)
> > +		    uint32_t size)
> >  {
> >  	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
> > -	u32 end = min_t(u32, start + size, 256);
> >  	u32 i;
> >
> > -	for (i = start; i < end; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		nv_crtc->lut.r[i] = r[i];
> >  		nv_crtc->lut.g[i] = g[i];
> >  		nv_crtc->lut.b[i] = b[i];
> >  	}
> >
> >  	nv50_crtc_lut_load(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  static void
> > diff --git a/drivers/gpu/drm/radeon/radeon_display.c
> b/drivers/gpu/drm/radeon/radeon_display.c
> > index 6a41b4982647..80f06db1dd34 100644
> > --- a/drivers/gpu/drm/radeon/radeon_display.c
> > +++ b/drivers/gpu/drm/radeon/radeon_display.c
> > @@ -231,19 +231,21 @@ void radeon_crtc_fb_gamma_get(struct drm_crtc
> *crtc, u16 *red, u16 *green,
> >  	*blue = radeon_crtc->lut_b[regno] << 6;
> >  }
> >
> > -static void radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > -				  u16 *blue, uint32_t start, uint32_t size)
> > +static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16
> *green,
> > +				 u16 *blue, uint32_t size)
> >  {
> >  	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
> > -	int end = (start + size > 256) ? 256 : start + size, i;
> > +	int i;
> >
> >  	/* userspace palettes are always correct as is */
> > -	for (i = start; i < end; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		radeon_crtc->lut_r[i] = red[i] >> 6;
> >  		radeon_crtc->lut_g[i] = green[i] >> 6;
> >  		radeon_crtc->lut_b[i] = blue[i] >> 6;
> >  	}
> >  	radeon_crtc_load_lut(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  static void radeon_crtc_destroy(struct drm_crtc *crtc)
> > @@ -688,6 +690,7 @@ radeon_crtc_set_config(struct drm_mode_set
> *set)
> >  	pm_runtime_put_autosuspend(dev->dev);
> >  	return ret;
> >  }
> > +
> >  static const struct drm_crtc_funcs radeon_crtc_funcs = {
> >  	.cursor_set2 = radeon_crtc_cursor_set2,
> >  	.cursor_move = radeon_crtc_cursor_move,
> > diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c
> b/drivers/gpu/drm/vc4/vc4_crtc.c
> > index 904d0754ad78..4e34f054da5c 100644
> > --- a/drivers/gpu/drm/vc4/vc4_crtc.c
> > +++ b/drivers/gpu/drm/vc4/vc4_crtc.c
> > @@ -175,20 +175,22 @@ vc4_crtc_lut_load(struct drm_crtc *crtc)
> >  		HVS_WRITE(SCALER_GAMDATA, vc4_crtc->lut_b[i]);
> >  }
> >
> > -static void
> > +static int
> >  vc4_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> > -		   uint32_t start, uint32_t size)
> > +		   uint32_t size)
> >  {
> >  	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
> >  	u32 i;
> >
> > -	for (i = start; i < start + size; i++) {
> > +	for (i = 0; i < size; i++) {
> >  		vc4_crtc->lut_r[i] = r[i] >> 8;
> >  		vc4_crtc->lut_g[i] = g[i] >> 8;
> >  		vc4_crtc->lut_b[i] = b[i] >> 8;
> >  	}
> >
> >  	vc4_crtc_lut_load(crtc);
> > +
> > +	return 0;
> >  }
> >
> >  static u32 vc4_get_fifo_full_level(u32 format)
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > index 55231cce73a0..8a69d4da40b5 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> > @@ -1404,9 +1404,9 @@ static int vmw_du_update_layout(struct
> vmw_private *dev_priv, unsigned num,
> >  	return 0;
> >  }
> >
> > -void vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
> > -			   u16 *r, u16 *g, u16 *b,
> > -			   uint32_t start, uint32_t size)
> > +int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
> > +			  u16 *r, u16 *g, u16 *b,
> > +			  uint32_t size)
> >  {
> >  	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
> >  	int i;
> > @@ -1418,6 +1418,8 @@ void vmw_du_crtc_gamma_set(struct drm_crtc
> *crtc,
> >  		vmw_write(dev_priv, SVGA_PALETTE_BASE + i * 3 + 1, g[i] >>
> 8);
> >  		vmw_write(dev_priv, SVGA_PALETTE_BASE + i * 3 + 2, b[i]
> >> 8);
> >  	}
> > +
> > +	return 0;
> >  }
> >
> >  int vmw_du_connector_dpms(struct drm_connector *connector, int
> mode)
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> > index 57203212c501..ff4803c107bc 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> > @@ -195,9 +195,9 @@ struct vmw_display_unit {
> >  void vmw_du_cleanup(struct vmw_display_unit *du);
> >  void vmw_du_crtc_save(struct drm_crtc *crtc);
> >  void vmw_du_crtc_restore(struct drm_crtc *crtc);
> > -void vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
> > +int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
> >  			   u16 *r, u16 *g, u16 *b,
> > -			   uint32_t start, uint32_t size);
> > +			   uint32_t size);
> >  int vmw_du_crtc_cursor_set2(struct drm_crtc *crtc, struct drm_file
> *file_priv,
> >  			    uint32_t handle, uint32_t width, uint32_t height,
> >  			    int32_t hot_x, int32_t hot_y);
> > diff --git a/include/drm/drm_atomic_helper.h
> b/include/drm/drm_atomic_helper.h
> > index d473dcc91f54..11896c84c4f8 100644
> > --- a/include/drm/drm_atomic_helper.h
> > +++ b/include/drm/drm_atomic_helper.h
> > @@ -147,9 +147,9 @@ void
> >  __drm_atomic_helper_connector_destroy_state(struct
> drm_connector_state *state);
> >  void drm_atomic_helper_connector_destroy_state(struct drm_connector
> *connector,
> >  					  struct drm_connector_state *state);
> > -void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> > -					u16 *red, u16 *green, u16 *blue,
> > -					uint32_t start, uint32_t size);
> > +int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
> > +				       u16 *red, u16 *green, u16 *blue,
> > +				       uint32_t size);
> >
> >  /**
> >   * drm_atomic_crtc_for_each_plane - iterate over planes currently
> attached to CRTC
> > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > index d1559cd04e3d..8622bb2f4f25 100644
> > --- a/include/drm/drm_crtc.h
> > +++ b/include/drm/drm_crtc.h
> > @@ -478,8 +478,8 @@ struct drm_crtc_funcs {
> >  	 * going on, which should eventually be unified to just one set of
> >  	 * hooks.
> >  	 */
> > -	void (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> > -			  uint32_t start, uint32_t size);
> > +	int (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
> > +			 uint32_t size);
> >
> >  	/**
> >  	 * @destroy:
> > --
> > 2.5.5
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Emil Velikov June 6, 2016, 4:11 p.m. UTC | #3
On 6 June 2016 at 17:00, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Jun 06, 2016 at 11:18:09AM +0200, Maarten Lankhorst wrote:
>> Change return value to int to propagate errors from gamma_set,
>> and remove start parameter. Updates always use the full size,
>> and some drivers even ignore the start parameter altogether.
>
> Commit message should explain why we suddenly need to pass up the error
> code, too:
>
> "This is needed for atomic drivers, where an atomic commit can fail with
> EAGAIN and should be restarted."
>
Since that can happen only on the DRM core side, shouldn't one refrain
from adding the "always returns 0" return status for the drivers'
.gamma_set hook ? Or there's is something in the works that would lead
to drivers being able to return a non 0 value ?

Thanks
Emil
Maarten Lankhorst June 6, 2016, 4:15 p.m. UTC | #4
Op 06-06-16 om 18:11 schreef Emil Velikov:
> On 6 June 2016 at 17:00, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Mon, Jun 06, 2016 at 11:18:09AM +0200, Maarten Lankhorst wrote:
>>> Change return value to int to propagate errors from gamma_set,
>>> and remove start parameter. Updates always use the full size,
>>> and some drivers even ignore the start parameter altogether.
>> Commit message should explain why we suddenly need to pass up the error
>> code, too:
>>
>> "This is needed for atomic drivers, where an atomic commit can fail with
>> EAGAIN and should be restarted."
>>
> Since that can happen only on the DRM core side, shouldn't one refrain
> from adding the "always returns 0" return status for the drivers'
> .gamma_set hook ? Or there's is something in the works that would lead
> to drivers being able to return a non 0 value ?
Look at drm_atomic_helper.c :-)

Being implemented as an atomic update means any errno can be returned.

~Maarten
Emil Velikov June 6, 2016, 4:17 p.m. UTC | #5
Hi Maarten,

On 6 June 2016 at 10:18, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:

> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c

> @@ -1076,8 +1076,6 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>         WARN_ON(fb->bits_per_pixel != 8);
>
>         fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> -
> -       return 0;
>  }
This seems like unrelated change for which the compiler should give
you a nice big warning :-)

Thanks
Emil
Emil Velikov June 6, 2016, 4:25 p.m. UTC | #6
On 6 June 2016 at 17:15, Maarten Lankhorst
<maarten.lankhorst@linux.intel.com> wrote:
> Op 06-06-16 om 18:11 schreef Emil Velikov:
>> On 6 June 2016 at 17:00, Daniel Vetter <daniel@ffwll.ch> wrote:
>>> On Mon, Jun 06, 2016 at 11:18:09AM +0200, Maarten Lankhorst wrote:
>>>> Change return value to int to propagate errors from gamma_set,
>>>> and remove start parameter. Updates always use the full size,
>>>> and some drivers even ignore the start parameter altogether.
>>> Commit message should explain why we suddenly need to pass up the error
>>> code, too:
>>>
>>> "This is needed for atomic drivers, where an atomic commit can fail with
>>> EAGAIN and should be restarted."
>>>
>> Since that can happen only on the DRM core side, shouldn't one refrain
>> from adding the "always returns 0" return status for the drivers'
>> .gamma_set hook ? Or there's is something in the works that would lead
>> to drivers being able to return a non 0 value ?
> Look at drm_atomic_helper.c :-)
>
> Being implemented as an atomic update means any errno can be returned.
>
Ouch. Looks like only the i915 driver uses the helper while I was
looking at other ATOMIC drivers. So things short circuited on my
noggin'. I guess it's up-to the respective drivers to be updated and
use the drm_atomic_helper_legacy_gamma_set helper.

Thanks for sticking with my silly question :-)
Emil
Maarten Lankhorst June 6, 2016, 4:32 p.m. UTC | #7
Op 06-06-16 om 18:17 schreef Emil Velikov:
> Hi Maarten,
>
> On 6 June 2016 at 10:18, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -1076,8 +1076,6 @@ static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
>>         WARN_ON(fb->bits_per_pixel != 8);
>>
>>         fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
>> -
>> -       return 0;
>>  }
> This seems like unrelated change for which the compiler should give
> you a nice big warning :-)
>
> Thanks
> Emil

Oops, was a leftover. Ignore this hunk.

I originally added a return, but fb_helper is left unchanged for now.

~Maarten
Maarten Lankhorst June 6, 2016, 4:41 p.m. UTC | #8
Op 06-06-16 om 18:25 schreef Emil Velikov:
> On 6 June 2016 at 17:15, Maarten Lankhorst
> <maarten.lankhorst@linux.intel.com> wrote:
>> Op 06-06-16 om 18:11 schreef Emil Velikov:
>>> On 6 June 2016 at 17:00, Daniel Vetter <daniel@ffwll.ch> wrote:
>>>> On Mon, Jun 06, 2016 at 11:18:09AM +0200, Maarten Lankhorst wrote:
>>>>> Change return value to int to propagate errors from gamma_set,
>>>>> and remove start parameter. Updates always use the full size,
>>>>> and some drivers even ignore the start parameter altogether.
>>>> Commit message should explain why we suddenly need to pass up the error
>>>> code, too:
>>>>
>>>> "This is needed for atomic drivers, where an atomic commit can fail with
>>>> EAGAIN and should be restarted."
>>>>
>>> Since that can happen only on the DRM core side, shouldn't one refrain
>>> from adding the "always returns 0" return status for the drivers'
>>> .gamma_set hook ? Or there's is something in the works that would lead
>>> to drivers being able to return a non 0 value ?
>> Look at drm_atomic_helper.c :-)
>>
>> Being implemented as an atomic update means any errno can be returned.
>>
> Ouch. Looks like only the i915 driver uses the helper while I was
> looking at other ATOMIC drivers. So things short circuited on my
> noggin'. I guess it's up-to the respective drivers to be updated and
> use the drm_atomic_helper_legacy_gamma_set helper.
>
> Thanks for sticking with my silly question :-)
Feel free to convert some drivers over, would be nice to have more widespread support for changing gamma through atomic properties. :-)
diff mbox

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 8227344d2ff6..16de862ea51f 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -2667,19 +2667,21 @@  static void dce_v10_0_cursor_reset(struct drm_crtc *crtc)
 	}
 }
 
-static void dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				    u16 *blue, uint32_t start, uint32_t size)
+static int dce_v10_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+				    u16 *blue, uint32_t size)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int end = (start + size > 256) ? 256 : start + size, i;
+	int i;
 
 	/* userspace palettes are always correct as is */
-	for (i = start; i < end; i++) {
+	for (i = 0; i < size; i++) {
 		amdgpu_crtc->lut_r[i] = red[i] >> 6;
 		amdgpu_crtc->lut_g[i] = green[i] >> 6;
 		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
 	}
 	dce_v10_0_crtc_load_lut(crtc);
+
+	return 0;
 }
 
 static void dce_v10_0_crtc_destroy(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index af26ec0bc59d..6e0ae8e2e65c 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -2678,19 +2678,21 @@  static void dce_v11_0_cursor_reset(struct drm_crtc *crtc)
 	}
 }
 
-static void dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				    u16 *blue, uint32_t start, uint32_t size)
+static int dce_v11_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+				    u16 *blue, uint32_t size)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int end = (start + size > 256) ? 256 : start + size, i;
+	int i;
 
 	/* userspace palettes are always correct as is */
-	for (i = start; i < end; i++) {
+	for (i = 0; i < size; i++) {
 		amdgpu_crtc->lut_r[i] = red[i] >> 6;
 		amdgpu_crtc->lut_g[i] = green[i] >> 6;
 		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
 	}
 	dce_v11_0_crtc_load_lut(crtc);
+
+	return 0;
 }
 
 static void dce_v11_0_crtc_destroy(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 3fb65e41a6ef..7b5fae4c74f1 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -2574,19 +2574,21 @@  static void dce_v8_0_cursor_reset(struct drm_crtc *crtc)
 	}
 }
 
-static void dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				    u16 *blue, uint32_t start, uint32_t size)
+static int dce_v8_0_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+				   u16 *blue, uint32_t size)
 {
 	struct amdgpu_crtc *amdgpu_crtc = to_amdgpu_crtc(crtc);
-	int end = (start + size > 256) ? 256 : start + size, i;
+	int i;
 
 	/* userspace palettes are always correct as is */
-	for (i = start; i < end; i++) {
+	for (i = 0; i < size; i++) {
 		amdgpu_crtc->lut_r[i] = red[i] >> 6;
 		amdgpu_crtc->lut_g[i] = green[i] >> 6;
 		amdgpu_crtc->lut_b[i] = blue[i] >> 6;
 	}
 	dce_v8_0_crtc_load_lut(crtc);
+
+	return 0;
 }
 
 static void dce_v8_0_crtc_destroy(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index c337922606e3..5957c3e659fe 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -624,19 +624,21 @@  static void ast_crtc_reset(struct drm_crtc *crtc)
 
 }
 
-static void ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				 u16 *blue, uint32_t start, uint32_t size)
+static int ast_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+			      u16 *blue, uint32_t size)
 {
 	struct ast_crtc *ast_crtc = to_ast_crtc(crtc);
-	int end = (start + size > 256) ? 256 : start + size, i;
+	int i;
 
 	/* userspace palettes are always correct as is */
-	for (i = start; i < end; i++) {
+	for (i = 0; i < size; i++) {
 		ast_crtc->lut_r[i] = red[i] >> 8;
 		ast_crtc->lut_g[i] = green[i] >> 8;
 		ast_crtc->lut_b[i] = blue[i] >> 8;
 	}
 	ast_crtc_load_lut(crtc);
+
+	return 0;
 }
 
 
diff --git a/drivers/gpu/drm/cirrus/cirrus_mode.c b/drivers/gpu/drm/cirrus/cirrus_mode.c
index 0b1a411cb89e..17c915d9a03e 100644
--- a/drivers/gpu/drm/cirrus/cirrus_mode.c
+++ b/drivers/gpu/drm/cirrus/cirrus_mode.c
@@ -325,18 +325,20 @@  static void cirrus_crtc_commit(struct drm_crtc *crtc)
  * use this for 8-bit mode so can't perform smooth fades on deeper modes,
  * but it's a requirement that we provide the function
  */
-static void cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				  u16 *blue, uint32_t start, uint32_t size)
+static int cirrus_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+				 u16 *blue, uint32_t size)
 {
 	struct cirrus_crtc *cirrus_crtc = to_cirrus_crtc(crtc);
 	int i;
 
-	for (i = 0; i < CIRRUS_LUT_SIZE; i++) {
+	for (i = 0; i < size; i++) {
 		cirrus_crtc->lut_r[i] = red[i];
 		cirrus_crtc->lut_g[i] = green[i];
 		cirrus_crtc->lut_b[i] = blue[i];
 	}
 	cirrus_crtc_load_lut(crtc);
+
+	return 0;
 }
 
 /* Simple cleanup function */
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 939df900dcaa..266c11018a19 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2933,16 +2933,15 @@  EXPORT_SYMBOL(drm_atomic_helper_connector_destroy_state);
  * @red: red correction table
  * @green: green correction table
  * @blue: green correction table
- * @start:
  * @size: size of the tables
  *
  * Implements support for legacy gamma correction table for drivers
  * that support color management through the DEGAMMA_LUT/GAMMA_LUT
  * properties.
  */
-void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
-					u16 *red, u16 *green, u16 *blue,
-					uint32_t start, uint32_t size)
+int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
+				       u16 *red, u16 *green, u16 *blue,
+				       uint32_t size)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_mode_config *config = &dev->mode_config;
@@ -2954,7 +2953,7 @@  void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
 
 	state = drm_atomic_state_alloc(crtc->dev);
 	if (!state)
-		return;
+		return -ENOMEM;
 
 	blob = drm_property_create_blob(dev,
 					sizeof(struct drm_color_lut) * size,
@@ -3005,7 +3004,7 @@  retry:
 
 	drm_property_unreference_blob(blob);
 
-	return;
+	return 0;
 fail:
 	if (ret == -EDEADLK)
 		goto backoff;
@@ -3013,7 +3012,7 @@  fail:
 	drm_atomic_state_free(state);
 	drm_property_unreference_blob(blob);
 
-	return;
+	return ret;
 backoff:
 	drm_atomic_state_clear(state);
 	drm_atomic_legacy_backoff(state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d2a6d958ca76..4d61ceae789c 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -5215,7 +5215,7 @@  int drm_mode_gamma_set_ioctl(struct drm_device *dev,
 		goto out;
 	}
 
-	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, crtc->gamma_size);
+	ret = crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc->gamma_size);
 
 out:
 	drm_modeset_unlock_all(dev);
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 7590df5e2e72..4accf6039058 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -227,7 +227,7 @@  static void drm_fb_helper_restore_lut_atomic(struct drm_crtc *crtc)
 	g_base = r_base + crtc->gamma_size;
 	b_base = g_base + crtc->gamma_size;
 
-	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, 0, crtc->gamma_size);
+	crtc->funcs->gamma_set(crtc, r_base, g_base, b_base, crtc->gamma_size);
 }
 
 /**
@@ -1076,8 +1076,6 @@  static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
 	WARN_ON(fb->bits_per_pixel != 8);
 
 	fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
-
-	return 0;
 }
 
 /**
diff --git a/drivers/gpu/drm/gma500/gma_display.c b/drivers/gpu/drm/gma500/gma_display.c
index c95406e6f44d..5b636bf0b467 100644
--- a/drivers/gpu/drm/gma500/gma_display.c
+++ b/drivers/gpu/drm/gma500/gma_display.c
@@ -175,20 +175,21 @@  void gma_crtc_load_lut(struct drm_crtc *crtc)
 	}
 }
 
-void gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
-			u32 start, u32 size)
+int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue,
+		       u32 size)
 {
 	struct gma_crtc *gma_crtc = to_gma_crtc(crtc);
 	int i;
-	int end = (start + size > 256) ? 256 : start + size;
 
-	for (i = start; i < end; i++) {
+	for (i = 0; i < size; i++) {
 		gma_crtc->lut_r[i] = red[i] >> 8;
 		gma_crtc->lut_g[i] = green[i] >> 8;
 		gma_crtc->lut_b[i] = blue[i] >> 8;
 	}
 
 	gma_crtc_load_lut(crtc);
+
+	return 0;
 }
 
 /**
diff --git a/drivers/gpu/drm/gma500/gma_display.h b/drivers/gpu/drm/gma500/gma_display.h
index b2491c65f053..e72dd08b701b 100644
--- a/drivers/gpu/drm/gma500/gma_display.h
+++ b/drivers/gpu/drm/gma500/gma_display.h
@@ -72,8 +72,8 @@  extern int gma_crtc_cursor_set(struct drm_crtc *crtc,
 			       uint32_t width, uint32_t height);
 extern int gma_crtc_cursor_move(struct drm_crtc *crtc, int x, int y);
 extern void gma_crtc_load_lut(struct drm_crtc *crtc);
-extern void gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-			       u16 *blue, u32 start, u32 size);
+extern int gma_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+			      u16 *blue, u32 size);
 extern void gma_crtc_dpms(struct drm_crtc *crtc, int mode);
 extern void gma_crtc_prepare(struct drm_crtc *crtc);
 extern void gma_crtc_commit(struct drm_crtc *crtc);
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c
index 14e64e08909e..f6d5892d03f1 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1344,19 +1344,20 @@  static void mga_crtc_commit(struct drm_crtc *crtc)
  * use this for 8-bit mode so can't perform smooth fades on deeper modes,
  * but it's a requirement that we provide the function
  */
-static void mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				  u16 *blue, uint32_t start, uint32_t size)
+static int mga_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+			      u16 *blue, uint32_t size)
 {
 	struct mga_crtc *mga_crtc = to_mga_crtc(crtc);
-	int end = (start + size > MGAG200_LUT_SIZE) ? MGAG200_LUT_SIZE : start + size;
 	int i;
 
-	for (i = start; i < end; i++) {
+	for (i = 0; i < size; i++) {
 		mga_crtc->lut_r[i] = red[i] >> 8;
 		mga_crtc->lut_g[i] = green[i] >> 8;
 		mga_crtc->lut_b[i] = blue[i] >> 8;
 	}
 	mga_crtc_load_lut(crtc);
+
+	return 0;
 }
 
 /* Simple cleanup function */
diff --git a/drivers/gpu/drm/nouveau/dispnv04/crtc.c b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
index 6f318c54da33..0cb7a18cde26 100644
--- a/drivers/gpu/drm/nouveau/dispnv04/crtc.c
+++ b/drivers/gpu/drm/nouveau/dispnv04/crtc.c
@@ -785,14 +785,14 @@  nv_crtc_disable(struct drm_crtc *crtc)
 	nouveau_bo_ref(NULL, &disp->image[nv_crtc->index]);
 }
 
-static void
-nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, uint32_t start,
+static int
+nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
 		  uint32_t size)
 {
-	int end = (start + size > 256) ? 256 : start + size, i;
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
+	int i;
 
-	for (i = start; i < end; i++) {
+	for (i = 0; i < size; i++) {
 		nv_crtc->lut.r[i] = r[i];
 		nv_crtc->lut.g[i] = g[i];
 		nv_crtc->lut.b[i] = b[i];
@@ -805,10 +805,12 @@  nv_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b, uint32_t start,
 	 */
 	if (!nv_crtc->base.primary->fb) {
 		nv_crtc->lut.depth = 0;
-		return;
+		return 0;
 	}
 
 	nv_crtc_gamma_load(crtc);
+
+	return 0;
 }
 
 static int
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c b/drivers/gpu/drm/nouveau/nv50_display.c
index 3ffc2b0057bf..7a7788212df7 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -1346,21 +1346,22 @@  nv50_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
 	return 0;
 }
 
-static void
+static int
 nv50_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-		    uint32_t start, uint32_t size)
+		    uint32_t size)
 {
 	struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
-	u32 end = min_t(u32, start + size, 256);
 	u32 i;
 
-	for (i = start; i < end; i++) {
+	for (i = 0; i < size; i++) {
 		nv_crtc->lut.r[i] = r[i];
 		nv_crtc->lut.g[i] = g[i];
 		nv_crtc->lut.b[i] = b[i];
 	}
 
 	nv50_crtc_lut_load(crtc);
+
+	return 0;
 }
 
 static void
diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c
index 6a41b4982647..80f06db1dd34 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -231,19 +231,21 @@  void radeon_crtc_fb_gamma_get(struct drm_crtc *crtc, u16 *red, u16 *green,
 	*blue = radeon_crtc->lut_b[regno] << 6;
 }
 
-static void radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				  u16 *blue, uint32_t start, uint32_t size)
+static int radeon_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+				 u16 *blue, uint32_t size)
 {
 	struct radeon_crtc *radeon_crtc = to_radeon_crtc(crtc);
-	int end = (start + size > 256) ? 256 : start + size, i;
+	int i;
 
 	/* userspace palettes are always correct as is */
-	for (i = start; i < end; i++) {
+	for (i = 0; i < size; i++) {
 		radeon_crtc->lut_r[i] = red[i] >> 6;
 		radeon_crtc->lut_g[i] = green[i] >> 6;
 		radeon_crtc->lut_b[i] = blue[i] >> 6;
 	}
 	radeon_crtc_load_lut(crtc);
+
+	return 0;
 }
 
 static void radeon_crtc_destroy(struct drm_crtc *crtc)
@@ -688,6 +690,7 @@  radeon_crtc_set_config(struct drm_mode_set *set)
 	pm_runtime_put_autosuspend(dev->dev);
 	return ret;
 }
+
 static const struct drm_crtc_funcs radeon_crtc_funcs = {
 	.cursor_set2 = radeon_crtc_cursor_set2,
 	.cursor_move = radeon_crtc_cursor_move,
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 904d0754ad78..4e34f054da5c 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -175,20 +175,22 @@  vc4_crtc_lut_load(struct drm_crtc *crtc)
 		HVS_WRITE(SCALER_GAMDATA, vc4_crtc->lut_b[i]);
 }
 
-static void
+static int
 vc4_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-		   uint32_t start, uint32_t size)
+		   uint32_t size)
 {
 	struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
 	u32 i;
 
-	for (i = start; i < start + size; i++) {
+	for (i = 0; i < size; i++) {
 		vc4_crtc->lut_r[i] = r[i] >> 8;
 		vc4_crtc->lut_g[i] = g[i] >> 8;
 		vc4_crtc->lut_b[i] = b[i] >> 8;
 	}
 
 	vc4_crtc_lut_load(crtc);
+
+	return 0;
 }
 
 static u32 vc4_get_fifo_full_level(u32 format)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 55231cce73a0..8a69d4da40b5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1404,9 +1404,9 @@  static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
 	return 0;
 }
 
-void vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
-			   u16 *r, u16 *g, u16 *b,
-			   uint32_t start, uint32_t size)
+int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
+			  u16 *r, u16 *g, u16 *b,
+			  uint32_t size)
 {
 	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
 	int i;
@@ -1418,6 +1418,8 @@  void vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
 		vmw_write(dev_priv, SVGA_PALETTE_BASE + i * 3 + 1, g[i] >> 8);
 		vmw_write(dev_priv, SVGA_PALETTE_BASE + i * 3 + 2, b[i] >> 8);
 	}
+
+	return 0;
 }
 
 int vmw_du_connector_dpms(struct drm_connector *connector, int mode)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 57203212c501..ff4803c107bc 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -195,9 +195,9 @@  struct vmw_display_unit {
 void vmw_du_cleanup(struct vmw_display_unit *du);
 void vmw_du_crtc_save(struct drm_crtc *crtc);
 void vmw_du_crtc_restore(struct drm_crtc *crtc);
-void vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
+int vmw_du_crtc_gamma_set(struct drm_crtc *crtc,
 			   u16 *r, u16 *g, u16 *b,
-			   uint32_t start, uint32_t size);
+			   uint32_t size);
 int vmw_du_crtc_cursor_set2(struct drm_crtc *crtc, struct drm_file *file_priv,
 			    uint32_t handle, uint32_t width, uint32_t height,
 			    int32_t hot_x, int32_t hot_y);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index d473dcc91f54..11896c84c4f8 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -147,9 +147,9 @@  void
 __drm_atomic_helper_connector_destroy_state(struct drm_connector_state *state);
 void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
 					  struct drm_connector_state *state);
-void drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
-					u16 *red, u16 *green, u16 *blue,
-					uint32_t start, uint32_t size);
+int drm_atomic_helper_legacy_gamma_set(struct drm_crtc *crtc,
+				       u16 *red, u16 *green, u16 *blue,
+				       uint32_t size);
 
 /**
  * drm_atomic_crtc_for_each_plane - iterate over planes currently attached to CRTC
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d1559cd04e3d..8622bb2f4f25 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -478,8 +478,8 @@  struct drm_crtc_funcs {
 	 * going on, which should eventually be unified to just one set of
 	 * hooks.
 	 */
-	void (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
-			  uint32_t start, uint32_t size);
+	int (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
+			 uint32_t size);
 
 	/**
 	 * @destroy: