diff mbox

[v7,11/25] drm/i915: Register color correction capabilities

Message ID 1445344492-8296-12-git-send-email-shashank.sharma@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sharma, Shashank Oct. 20, 2015, 12:34 p.m. UTC
From DRM color management:

Comments

Bish, Jim Oct. 21, 2015, 11:19 p.m. UTC | #1
On Tue, 2015-10-20 at 18:04 +0530, Shashank Sharma wrote:
> From DRM color management:

> ============================

> DRM color manager supports these color properties:

> 1. "ctm": Color transformation matrix property, where a

>    color transformation matrix of 9 correction values gets

>    applied as correction.

> 2. "palette_before_ctm": for corrections which get applied

>    beore color transformation matrix correction.

> 3. "palette_after_ctm": for corrections which get applied

>    after color transformation matrix correction.

> 

> These color correction capabilities may differ per platform, 

> supporting

> various different no. of correction coefficients. So DRM color 

> manager

> support few properties using which a user space can query the 

> platform's

> capability, and prepare color correction accordingly.

> These query properties are:

> 1. cm_coeff_after_ctm_property

> 2. cm_coeff_before_ctm_property

>   (CTM is fix to 9 coefficients across industry)

> 

> Now, Intel color manager registers:

> ======================================

> 1. Gamma correction property as "palette_after_ctm" property

> 2. Degamma correction capability as "palette_bafore_ctm" property

>    capability as "palette_after_ctm" DRM color property hook.

> 3. CSC as "ctm" property.

> 

> So finally, This patch does the following:

> 1. Add a function which loads the platform's color correction

>    capabilities in the cm_crtc_palette_capabilities_property 

> structure.

> 2. Attaches the cm_crtc_palette_capabilities_property to every CRTC

>    getting initiaized.

> 3. Adds two new parameters "num_samples_after_ctm" and

>    "num_samples_before_ctm" in intel_device_info as gamma and

>    degamma coefficients vary per platform basis.

> 

> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>

> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>

> ---

>  drivers/gpu/drm/i915/i915_drv.h            |  2 ++

>  drivers/gpu/drm/i915/intel_color_manager.c | 31 

> ++++++++++++++++++++++++++++++

>  2 files changed, 33 insertions(+)

> 

> diff --git a/drivers/gpu/drm/i915/i915_drv.h 

> b/drivers/gpu/drm/i915/i915_drv.h

> index 8afda45..613bee2 100644

> --- a/drivers/gpu/drm/i915/i915_drv.h

> +++ b/drivers/gpu/drm/i915/i915_drv.h

> @@ -789,6 +789,8 @@ struct intel_device_info {

>       u8 num_sprites[I915_MAX_PIPES];

>       u8 gen;

>       u8 ring_mask; /* Rings supported by the HW */

> +     u16 num_samples_after_ctm;

> +     u16 num_samples_before_ctm;

thought we agreed last week that num_samples was going to be 
removed.  May be ok to handle in a later patch unless
someone has strong objection.

Jim
>       DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);

>       /* Register offsets for the various display pipes and transcoders 

> */

>       int pipe_offsets[I915_MAX_TRANSCODERS];

> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 

> b/drivers/gpu/drm/i915/intel_color_manager.c

> index b03ee94..334bfff 100644

> --- a/drivers/gpu/drm/i915/intel_color_manager.c

> +++ b/drivers/gpu/drm/i915/intel_color_manager.c

> @@ -30,4 +30,35 @@

>  void intel_attach_color_properties_to_crtc(struct drm_device *dev,

>               struct drm_crtc *crtc)

>  {

> +     struct drm_mode_config *config = &dev->mode_config;

> +     struct drm_mode_object *mode_obj = &crtc->base;

> +

> +     /*

> +     * Register:

> +     * =========

> +     * Gamma correction as palette_after_ctm property

> +     * Degamma correction as palette_before_ctm property

> +     *

> +     * Load:

> +     * =====

> +     * no. of coefficients supported on this platform for gamma

> +     * and degamma with the query properties. A user

> +     * space agent should read these query property, and prepare

> +     * the color correction values accordingly. Its expected from the

> +     * driver to load the right number of coefficients during the init

> +     * phase.

> +     */

> +     if (config->cm_coeff_after_ctm_property) {

> +             drm_object_attach_property(mode_obj,

> +                     config->cm_coeff_after_ctm_property,

> +             INTEL_INFO(dev)->num_samples_after_ctm);

> +             DRM_DEBUG_DRIVER("Gamma query property initialized\n");

> +     }

> +

> +     if (config->cm_coeff_before_ctm_property) {

> +             drm_object_attach_property(mode_obj,

> +                     config->cm_coeff_before_ctm_property,

> +             INTEL_INFO(dev)->num_samples_before_ctm);

> +             DRM_DEBUG_DRIVER("Degamma query property initialized\n");

> +     }

>  }
Daniel Vetter Oct. 22, 2015, 7:19 a.m. UTC | #2
On Wed, Oct 21, 2015 at 11:19:10PM +0000, Bish, Jim wrote:
> On Tue, 2015-10-20 at 18:04 +0530, Shashank Sharma wrote:
> > From DRM color management:
> > ============================
> > DRM color manager supports these color properties:
> > 1. "ctm": Color transformation matrix property, where a
> >    color transformation matrix of 9 correction values gets
> >    applied as correction.
> > 2. "palette_before_ctm": for corrections which get applied
> >    beore color transformation matrix correction.
> > 3. "palette_after_ctm": for corrections which get applied
> >    after color transformation matrix correction.
> > 
> > These color correction capabilities may differ per platform, 
> > supporting
> > various different no. of correction coefficients. So DRM color 
> > manager
> > support few properties using which a user space can query the 
> > platform's
> > capability, and prepare color correction accordingly.
> > These query properties are:
> > 1. cm_coeff_after_ctm_property
> > 2. cm_coeff_before_ctm_property
> >   (CTM is fix to 9 coefficients across industry)
> > 
> > Now, Intel color manager registers:
> > ======================================
> > 1. Gamma correction property as "palette_after_ctm" property
> > 2. Degamma correction capability as "palette_bafore_ctm" property
> >    capability as "palette_after_ctm" DRM color property hook.
> > 3. CSC as "ctm" property.
> > 
> > So finally, This patch does the following:
> > 1. Add a function which loads the platform's color correction
> >    capabilities in the cm_crtc_palette_capabilities_property 
> > structure.
> > 2. Attaches the cm_crtc_palette_capabilities_property to every CRTC
> >    getting initiaized.
> > 3. Adds two new parameters "num_samples_after_ctm" and
> >    "num_samples_before_ctm" in intel_device_info as gamma and
> >    degamma coefficients vary per platform basis.
> > 
> > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> > Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h            |  2 ++
> >  drivers/gpu/drm/i915/intel_color_manager.c | 31 
> > ++++++++++++++++++++++++++++++
> >  2 files changed, 33 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 8afda45..613bee2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -789,6 +789,8 @@ struct intel_device_info {
> >       u8 num_sprites[I915_MAX_PIPES];
> >       u8 gen;
> >       u8 ring_mask; /* Rings supported by the HW */
> > +     u16 num_samples_after_ctm;
> > +     u16 num_samples_before_ctm;
> thought we agreed last week that num_samples was going to be 
> removed.  May be ok to handle in a later patch unless
> someone has strong objection.

That's another num_samples. The discussion was about the userspace abi on
in include/uapi.
-Daniel

> 
> Jim
> >       DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
> >       /* Register offsets for the various display pipes and transcoders 
> > */
> >       int pipe_offsets[I915_MAX_TRANSCODERS];
> > diff --git a/drivers/gpu/drm/i915/intel_color_manager.c 
> > b/drivers/gpu/drm/i915/intel_color_manager.c
> > index b03ee94..334bfff 100644
> > --- a/drivers/gpu/drm/i915/intel_color_manager.c
> > +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> > @@ -30,4 +30,35 @@
> >  void intel_attach_color_properties_to_crtc(struct drm_device *dev,
> >               struct drm_crtc *crtc)
> >  {
> > +     struct drm_mode_config *config = &dev->mode_config;
> > +     struct drm_mode_object *mode_obj = &crtc->base;
> > +
> > +     /*
> > +     * Register:
> > +     * =========
> > +     * Gamma correction as palette_after_ctm property
> > +     * Degamma correction as palette_before_ctm property
> > +     *
> > +     * Load:
> > +     * =====
> > +     * no. of coefficients supported on this platform for gamma
> > +     * and degamma with the query properties. A user
> > +     * space agent should read these query property, and prepare
> > +     * the color correction values accordingly. Its expected from the
> > +     * driver to load the right number of coefficients during the init
> > +     * phase.
> > +     */
> > +     if (config->cm_coeff_after_ctm_property) {
> > +             drm_object_attach_property(mode_obj,
> > +                     config->cm_coeff_after_ctm_property,
> > +             INTEL_INFO(dev)->num_samples_after_ctm);
> > +             DRM_DEBUG_DRIVER("Gamma query property initialized\n");
> > +     }
> > +
> > +     if (config->cm_coeff_before_ctm_property) {
> > +             drm_object_attach_property(mode_obj,
> > +                     config->cm_coeff_before_ctm_property,
> > +             INTEL_INFO(dev)->num_samples_before_ctm);
> > +             DRM_DEBUG_DRIVER("Degamma query property initialized\n");
> > +     }
> >  }
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rob Bradford Oct. 22, 2015, 12:05 p.m. UTC | #3
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 8afda45..613bee2 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -789,6 +789,8 @@ struct intel_device_info {
> >       u8 num_sprites[I915_MAX_PIPES];
> >       u8 gen;
> >       u8 ring_mask; /* Rings supported by the HW */
> > +     u16 num_samples_after_ctm;
> > +     u16 num_samples_before_ctm;
> thought we agreed last week that num_samples was going to be 
> removed.  May be ok to handle in a later patch unless
> someone has strong objection.

This is for the reporting out of platform capability and is still
needed as userspace needs to know the recommended table size so that it
can resample as necessary.

Rob
Sharma, Shashank Oct. 26, 2015, 3:59 a.m. UTC | #4
Thanks Daniel, Rob, for providing the clarification. 
Jim, Does this sound good to you, now ? 

Regards
Shashank
-----Original Message-----
From: Bradford, Robert 

Sent: Thursday, October 22, 2015 5:36 PM
To: Bish, Jim; Sharma, Shashank
Cc: Palleti, Avinash Reddy; kausalmalladi@gmail.com; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Smith, Gary K; Roper, Matthew D; Vetter, Daniel; dri-devel@lists.freedesktop.org; Mukherjee, Indranil; Matheson, Annie J; Kumar, Kiran S
Subject: Re: [PATCH v7 11/25] drm/i915: Register color correction capabilities


> > diff --git a/drivers/gpu/drm/i915/i915_drv.h 

> > b/drivers/gpu/drm/i915/i915_drv.h index 8afda45..613bee2 100644

> > --- a/drivers/gpu/drm/i915/i915_drv.h

> > +++ b/drivers/gpu/drm/i915/i915_drv.h

> > @@ -789,6 +789,8 @@ struct intel_device_info {

> >       u8 num_sprites[I915_MAX_PIPES];

> >       u8 gen;

> >       u8 ring_mask; /* Rings supported by the HW */

> > +     u16 num_samples_after_ctm;

> > +     u16 num_samples_before_ctm;

> thought we agreed last week that num_samples was going to be removed.  

> May be ok to handle in a later patch unless someone has strong 

> objection.


This is for the reporting out of platform capability and is still needed as userspace needs to know the recommended table size so that it can resample as necessary.

Rob
diff mbox

Patch

============================
DRM color manager supports these color properties:
1. "ctm": Color transformation matrix property, where a
   color transformation matrix of 9 correction values gets
   applied as correction.
2. "palette_before_ctm": for corrections which get applied
   beore color transformation matrix correction.
3. "palette_after_ctm": for corrections which get applied
   after color transformation matrix correction.

These color correction capabilities may differ per platform, supporting
various different no. of correction coefficients. So DRM color manager
support few properties using which a user space can query the platform's
capability, and prepare color correction accordingly.
These query properties are:
1. cm_coeff_after_ctm_property
2. cm_coeff_before_ctm_property
  (CTM is fix to 9 coefficients across industry)

Now, Intel color manager registers:
======================================
1. Gamma correction property as "palette_after_ctm" property
2. Degamma correction capability as "palette_bafore_ctm" property
   capability as "palette_after_ctm" DRM color property hook.
3. CSC as "ctm" property.

So finally, This patch does the following:
1. Add a function which loads the platform's color correction
   capabilities in the cm_crtc_palette_capabilities_property structure.
2. Attaches the cm_crtc_palette_capabilities_property to every CRTC
   getting initiaized.
3. Adds two new parameters "num_samples_after_ctm" and
   "num_samples_before_ctm" in intel_device_info as gamma and
   degamma coefficients vary per platform basis.

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.h            |  2 ++
 drivers/gpu/drm/i915/intel_color_manager.c | 31 ++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8afda45..613bee2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -789,6 +789,8 @@  struct intel_device_info {
 	u8 num_sprites[I915_MAX_PIPES];
 	u8 gen;
 	u8 ring_mask; /* Rings supported by the HW */
+	u16 num_samples_after_ctm;
+	u16 num_samples_before_ctm;
 	DEV_INFO_FOR_EACH_FLAG(DEFINE_FLAG, SEP_SEMICOLON);
 	/* Register offsets for the various display pipes and transcoders */
 	int pipe_offsets[I915_MAX_TRANSCODERS];
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c
index b03ee94..334bfff 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -30,4 +30,35 @@ 
 void intel_attach_color_properties_to_crtc(struct drm_device *dev,
 		struct drm_crtc *crtc)
 {
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_mode_object *mode_obj = &crtc->base;
+
+	/*
+	* Register:
+	* =========
+	* Gamma correction as palette_after_ctm property
+	* Degamma correction as palette_before_ctm property
+	*
+	* Load:
+	* =====
+	* no. of coefficients supported on this platform for gamma
+	* and degamma with the query properties. A user
+	* space agent should read these query property, and prepare
+	* the color correction values accordingly. Its expected from the
+	* driver to load the right number of coefficients during the init
+	* phase.
+	*/
+	if (config->cm_coeff_after_ctm_property) {
+		drm_object_attach_property(mode_obj,
+			config->cm_coeff_after_ctm_property,
+		INTEL_INFO(dev)->num_samples_after_ctm);
+		DRM_DEBUG_DRIVER("Gamma query property initialized\n");
+	}
+
+	if (config->cm_coeff_before_ctm_property) {
+		drm_object_attach_property(mode_obj,
+			config->cm_coeff_before_ctm_property,
+		INTEL_INFO(dev)->num_samples_before_ctm);
+		DRM_DEBUG_DRIVER("Degamma query property initialized\n");
+	}
 }