[PATCH] gpu/drm: set gamma_lut or degamma_lut based on HW in setcmap_atomic

[PATCH] gpu/drm: set gamma_lut or degamma_lut based on HW in setcmap_atomic

[Vamsi Krishna Brahmajosyula]

setcmap_atomic always sets gamma_lut in setcmap_atomic

Address the following FIXME to set gamma or degamma lut
	FIXME: This always uses gamma_lut. Some HW have only
	degamma_lut, in which case we should reset gamma_lut and set
	degamma_lut. See drm_crtc_legacy_gamma_set().

Tested by calling setcmap_atomic in drm_fb_helper_setcmap with out
the condition check.

Signed-off-by: Vamsi Krishna Brahmajosyula <vamsikrishna.brahmajosyula@gmail.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 50 ++++++++++++++++++++-------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 29c53f9f449c..48f053f7ac89 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -871,11 +871,11 @@ static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info)
 	return ret;
 }
 
-static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
+static struct drm_property_blob *setcmap_new_blob_lut(struct drm_crtc *crtc,
 						       struct fb_cmap *cmap)
 {
 	struct drm_device *dev = crtc->dev;
-	struct drm_property_blob *gamma_lut;
+	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
 	int size = crtc->gamma_size;
 	int i;
@@ -883,11 +883,11 @@ static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
 	if (!size || cmap->start + cmap->len > size)
 		return ERR_PTR(-EINVAL);
 
-	gamma_lut = drm_property_create_blob(dev, sizeof(*lut) * size, NULL);
-	if (IS_ERR(gamma_lut))
-		return gamma_lut;
+	blob = drm_property_create_blob(dev, sizeof(*lut) * size, NULL);
+	if (IS_ERR(blob))
+		return blob;
 
-	lut = gamma_lut->data;
+	lut = blob->data;
 	if (cmap->start || cmap->len != size) {
 		u16 *r = crtc->gamma_store;
 		u16 *g = r + crtc->gamma_size;
@@ -911,14 +911,14 @@ static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
 		lut[cmap->start + i].blue = cmap->blue[i];
 	}
 
-	return gamma_lut;
+	return blob;
 }
 
 static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 {
 	struct drm_fb_helper *fb_helper = info->par;
 	struct drm_device *dev = fb_helper->dev;
-	struct drm_property_blob *gamma_lut = NULL;
+	struct drm_property_blob *blob = NULL;
 	struct drm_modeset_acquire_ctx ctx;
 	struct drm_crtc_state *crtc_state;
 	struct drm_atomic_state *state;
@@ -926,6 +926,9 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 	struct drm_crtc *crtc;
 	u16 *r, *g, *b;
 	bool replaced;
+	u32 gamma_id = dev->mode_config.gamma_lut_property->base.id;
+	u32 degamma_id = dev->mode_config.degamma_lut_property->base.id;
+	bool use_gamma_lut;
 	int ret = 0;
 
 	drm_modeset_acquire_init(&ctx, 0);
@@ -941,11 +944,21 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 	drm_client_for_each_modeset(modeset, &fb_helper->client) {
 		crtc = modeset->crtc;
 
-		if (!gamma_lut)
-			gamma_lut = setcmap_new_gamma_lut(crtc, cmap);
-		if (IS_ERR(gamma_lut)) {
-			ret = PTR_ERR(gamma_lut);
-			gamma_lut = NULL;
+		if (drm_mode_obj_find_prop_id(&crtc->base, gamma_id))
+			use_gamma_lut = true;
+		else if (drm_mode_obj_find_prop_id(&crtc->base, degamma_id))
+			use_gamma_lut = false;
+		else {
+			ret = -ENODEV;
+			blob = NULL;
+			goto out_state;
+		}
+
+		if (!blob)
+			blob = setcmap_new_blob_lut(crtc, cmap);
+		if (IS_ERR(blob)) {
+			ret = PTR_ERR(blob);
+			blob = NULL;
 			goto out_state;
 		}
 
@@ -956,15 +969,14 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 		}
 
 		/*
-		 * FIXME: This always uses gamma_lut. Some HW have only
-		 * degamma_lut, in which case we should reset gamma_lut and set
-		 * degamma_lut. See drm_crtc_legacy_gamma_set().
+		 * Some HW have only degamma_lut, in which case we should
+		 * reset gamma_lut and set degamma_lut.
 		 */
 		replaced  = drm_property_replace_blob(&crtc_state->degamma_lut,
-						      NULL);
+						      use_gamma_lut ? NULL : blob);
 		replaced |= drm_property_replace_blob(&crtc_state->ctm, NULL);
 		replaced |= drm_property_replace_blob(&crtc_state->gamma_lut,
-						      gamma_lut);
+						      use_gamma_lut ? blob : NULL);
 		crtc_state->color_mgmt_changed |= replaced;
 	}
 
@@ -988,7 +1000,7 @@ static int setcmap_atomic(struct fb_cmap *cmap, struct fb_info *info)
 	if (ret == -EDEADLK)
 		goto backoff;
 
-	drm_property_blob_put(gamma_lut);
+	drm_property_blob_put(blob);
 	drm_atomic_state_put(state);
 out_ctx:
 	drm_modeset_drop_locks(&ctx);

base-commit: 7ec462100ef9142344ddbf86f2c3008b97acddbe
-- 
2.46.2

Re: [PATCH] gpu/drm: set gamma_lut or degamma_lut based on HW in setcmap_atomic

[kernel test robot]

Hi Vamsi,

kernel test robot noticed the following build errors:

[auto build test ERROR on 7ec462100ef9142344ddbf86f2c3008b97acddbe]

url:    https://github.com/intel-lab-lkp/linux/commits/Vamsi-Krishna-Brahmajosyula/gpu-drm-set-gamma_lut-or-degamma_lut-based-on-HW-in-setcmap_atomic/20241003-200835
base:   7ec462100ef9142344ddbf86f2c3008b97acddbe
patch link:    https://lore.kernel.org/r/20241003120655.53663-1-vamsikrishna.brahmajosyula%40gmail.com
patch subject: [PATCH] gpu/drm: set gamma_lut or degamma_lut based on HW in setcmap_atomic
config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20241006/202410060802.Ln9ygpuY-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241006/202410060802.Ln9ygpuY-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410060802.Ln9ygpuY-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "drm_mode_obj_find_prop_id" [drivers/gpu/drm/drm_kms_helper.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] gpu/drm: set gamma_lut or degamma_lut based on HW in setcmap_atomic

[Vamsi Krishna Brahmajosyula]

On Sun, Oct 6, 2024 at 6:31 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Vamsi,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on 7ec462100ef9142344ddbf86f2c3008b97acddbe]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Vamsi-Krishna-Brahmajosyula/gpu-drm-set-gamma_lut-or-degamma_lut-based-on-HW-in-setcmap_atomic/20241003-200835
> base:   7ec462100ef9142344ddbf86f2c3008b97acddbe
> patch link:    https://lore.kernel.org/r/20241003120655.53663-1-vamsikrishna.brahmajosyula%40gmail.com
> patch subject: [PATCH] gpu/drm: set gamma_lut or degamma_lut based on HW in setcmap_atomic
> config: x86_64-rhel-8.3 (https://download.01.org/0day-ci/archive/20241006/202410060802.Ln9ygpuY-lkp@intel.com/config)
> compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241006/202410060802.Ln9ygpuY-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202410060802.Ln9ygpuY-lkp@intel.com/
>
> All errors (new ones prefixed by >>, old ones prefixed by <<):
>
Based on the kernel bot compile failure I received,
> >> ERROR: modpost: "drm_mode_obj_find_prop_id" [drivers/gpu/drm/drm_kms_helper.ko] undefined!
I realised I can not use drm_mode_obj_find_prop_id in setcmap_atomic
(in atomic context).
I am exploring atomic functions which access gamma_lut and degamma_lut
via state.
I will send a v2 with a new approach.
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki