Re: [PATCH v2 02/10] drm/i915: Attach color properties to CRTC

From: Damien Lespiau <damien.lespiau@intel.com>
To: Kausal Malladi <Kausal.Malladi@intel.com>
Cc: annie.j.matheson@intel.com, dhanya.p.r@intel.com,
	avinash.reddy.palleti@intel.com, intel-gfx@lists.freedesktop.org,
	dri-devel@lists.freedesktop.org, vijay.a.purushothaman@intel.com,
	indranil.mukherjee@intel.com, jesse.barnes@intel.com,
	daniel.vetter@intel.com, sunil.kamath@intel.com,
	shashank.sharma@intel.com
Subject: Re: [PATCH v2 02/10] drm/i915: Attach color properties to CRTC
Date: Tue, 9 Jun 2015 11:54:29 +0100	[thread overview]
Message-ID: <20150609105429.GC1671@strange.amr.corp.intel.com> (raw)
In-Reply-To: <1433425361-17864-3-git-send-email-Kausal.Malladi@intel.com>

On Thu, Jun 04, 2015 at 07:12:33PM +0530, Kausal Malladi wrote:
> From: Kausal Malladi <Kausal.Malladi@intel.com>
> 
> Every CRTC would be carrying its own instances of CSC and Gamma color
> correction values. This patch adds a new function to attach color
> properties to respective CRTCs while initialization.
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Kausal Malladi <Kausal.Malladi@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color_manager.c | 24 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color_manager.h |  2 ++
>  drivers/gpu/drm/i915/intel_display.c       |  3 +++
>  3 files changed, 29 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
> index f7e2380..8d4ee8f 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,30 @@
>  
>  #include "intel_color_manager.h"
>  
> +void intel_color_manager_attach(struct drm_device *dev,
> +		struct drm_mode_object *mode_obj)
> +{
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	/* Attach all properties generic to crtc and plane */
> +	if (config->gamma_property) {
> +		drm_object_attach_property(mode_obj,
> +				config->gamma_property, 0);

No platform check? I doubt all of our platforms have LUTs on all planes.
Ah but we don't attach this property to any plane just yet, so the
comment is misleading.

> +
> +		DRM_DEBUG_DRIVER("Initialized gamma property\n");

That looks too verbose to me.

> +	}
> +
> +	/* Attach properties specific to crtc only */
> +	if (mode_obj->type == DRM_MODE_OBJECT_CRTC) {
> +		if (config->csc_property) {
> +			drm_object_attach_property(mode_obj,
> +					config->csc_property, 0);
> +
> +			DRM_DEBUG_DRIVER("Initialized CSC property\n");
> +		}
> +	}
> +}
> +
>  void intel_color_manager_init(struct drm_device *dev)
>  {
>  	struct drm_mode_config *config = &dev->mode_config;
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h
> index 154bf16..a55ce23 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.h
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -30,3 +30,5 @@
>  
>  /* Generic Function prototypes */
>  void intel_color_manager_init(struct drm_device *dev);
> +void intel_color_manager_attach(struct drm_device *dev,
> +		struct drm_mode_object *mode_obj);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2322dee..d4e9aa3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13794,6 +13794,9 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>  	intel_crtc->cursor_cntl = ~0;
>  	intel_crtc->cursor_size = ~0;
>  
> +	/* Attaching color properties to the CRTC */
> +	intel_color_manager_attach(dev, &intel_crtc->base.base);
> +
>  	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>  	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>  	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
> -- 
> 2.4.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel