[PATCH 09/12] drm/i915: Add pipe level Gamma correction for CHV/BSW
On Wednesday 08 July 2015 04:53 AM, Matt Roper wrote:
> On Fri, Jul 03, 2015 at 09:01:44AM +0530, Kausal Malladi wrote:
>> CHV/BSW platform supports various Gamma correction modes, which are:
>> 1. Legacy 8-bit mode
>> 2. 10-bit CGM (Color Gamut Mapping) mode
>>
>> This patch does the following:
>> 1. Adds the core function to program Gamma correction values for CHV/BSW
>> platform
>> 2. Adds Gamma correction macros/defines
>>
>> Signed-off-by: Shashank Sharma
>> Signed-off-by: Kausal Malladi
>> ---
>> drivers/gpu/drm/i915/i915_reg.h| 10 ++
>> drivers/gpu/drm/i915/intel_atomic.c| 6 ++
>> drivers/gpu/drm/i915/intel_color_manager.c | 154
>> +
>> drivers/gpu/drm/i915/intel_color_manager.h | 12 +++
>> drivers/gpu/drm/i915/intel_drv.h | 2 +
>> 5 files changed, 184 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 313b1f9..36672e7 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7900,4 +7900,14 @@ enum skl_disp_power_wells {
>> #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
>> #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
>>
>> +/* Color Management */
>> +#define PIPEA_CGM_CONTROL (VLV_DISPLAY_BASE + 0x67A00)
>> +#define PIPEA_CGM_GAMMA_MIN (VLV_DISPLAY_BASE + 0x67000)
>> +#define CGM_OFFSET 0x2000
>> +#define GAMMA_OFFSET0x2000
>> +#define _PIPE_CGM_CONTROL(pipe) \
>> +(PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
>> +#define _PIPE_GAMMA_BASE(pipe) \
>> +(PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
>> +
>> #endif /* _I915_REG_H_ */
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
>> b/drivers/gpu/drm/i915/intel_atomic.c
>> index d2674a6..21f0ac2 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -473,6 +473,12 @@ int intel_crtc_atomic_set_property(struct drm_crtc
>> *crtc,
>> struct drm_property *property,
>> uint64_t val)
>> {
>> +struct drm_device *dev = crtc->dev;
>> +struct drm_mode_config *config = >mode_config;
>> +
>> +if (property == config->prop_palette_after_ctm)
>> +return intel_color_manager_set_gamma(dev, >base, val);
>> +
> I think we discussed this on a previous iteration of the patch series,
> but .atomic_set_property() isn't supposed to actually update the
> hardware at all itself (as you're doing here); it's only supposed to
> update the 'state' parameter that was passed here. Further down the
> atomic pipeline the driver will decide whether it really wants to
> program any of the state or not and then the CRTC's atomic commit
> function should program the registers according to whatever value is set
> in the state object.
Thanks Matt for your suggestions offline. We will implement it this way
in our next set of patches.
>
>> DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>> return -EINVAL;
>> }
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
>> b/drivers/gpu/drm/i915/intel_color_manager.c
>> index 71b4c05..84cc3e47 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -27,6 +27,160 @@
>>
>> #include "intel_color_manager.h"
>>
>> +int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
>> + struct drm_crtc *crtc)
>> +{
>> +struct drm_palette *gamma_data;
>> +struct drm_i915_private *dev_priv = dev->dev_private;
>> +struct drm_property_blob *blob;
>> +struct drm_mode_config *config = >mode_config;
>> +u32 cgm_control_reg = 0;
>> +u32 cgm_gamma_reg = 0;
>> +u32 reg, val;
>> +enum pipe pipe;
>> +u16 red, green, blue;
>> +u32 count = 0;
>> +struct drm_r32g32b32 *correction_values = NULL;
>> +u32 num_samples;
>> +u32 word;
>> +u32 palette;
>> +int ret = 0, length;
>> +
>> +blob = drm_property_lookup_blob(dev, blob_id);
>> +if (!blob) {
>> +DRM_ERROR("Invalid Blob ID\n");
>> +return -EINVAL;
>> +}
>> +
>> +gamma_data = (struct drm_palette *)blob->data;
> Do we need to validate that the blob we receive is of the expected size
> or does something in the DRM core's blob handling take care of that for
> us? We don't want userspace to be able to trigger a panic by passing a
> smaller than expected blob here.
Yes, it was an oversight.
>
>
>> +
>> +if (gamma_data->version != CHV_GAMMA_DATA_STRUCT_VERSION) {
>> +DRM_ERROR("Invalid Gamma Data struct version\n");
>> +return -EINVAL;
>> +}
>> +
>> +pipe = to_intel_crtc(crtc)->pipe;
>> +num_samples = gamma_data->palette_num_samples;
>> +length = num_samples * sizeof(struct drm_r32g32b32);
>> +
>> +if (num_samples
[PATCH 09/12] drm/i915: Add pipe level Gamma correction for CHV/BSW
On Fri, Jul 03, 2015 at 09:01:44AM +0530, Kausal Malladi wrote:
> CHV/BSW platform supports various Gamma correction modes, which are:
> 1. Legacy 8-bit mode
> 2. 10-bit CGM (Color Gamut Mapping) mode
>
> This patch does the following:
> 1. Adds the core function to program Gamma correction values for CHV/BSW
>platform
> 2. Adds Gamma correction macros/defines
>
> Signed-off-by: Shashank Sharma
> Signed-off-by: Kausal Malladi
> ---
> drivers/gpu/drm/i915/i915_reg.h| 10 ++
> drivers/gpu/drm/i915/intel_atomic.c| 6 ++
> drivers/gpu/drm/i915/intel_color_manager.c | 154
> +
> drivers/gpu/drm/i915/intel_color_manager.h | 12 +++
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 5 files changed, 184 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 313b1f9..36672e7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7900,4 +7900,14 @@ enum skl_disp_power_wells {
> #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
> #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
>
> +/* Color Management */
> +#define PIPEA_CGM_CONTROL(VLV_DISPLAY_BASE + 0x67A00)
> +#define PIPEA_CGM_GAMMA_MIN (VLV_DISPLAY_BASE + 0x67000)
> +#define CGM_OFFSET 0x2000
> +#define GAMMA_OFFSET 0x2000
> +#define _PIPE_CGM_CONTROL(pipe) \
> + (PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
> +#define _PIPE_GAMMA_BASE(pipe) \
> + (PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
> +
> #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index d2674a6..21f0ac2 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -473,6 +473,12 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
> struct drm_property *property,
> uint64_t val)
> {
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = >mode_config;
> +
> + if (property == config->prop_palette_after_ctm)
> + return intel_color_manager_set_gamma(dev, >base, val);
> +
I think we discussed this on a previous iteration of the patch series,
but .atomic_set_property() isn't supposed to actually update the
hardware at all itself (as you're doing here); it's only supposed to
update the 'state' parameter that was passed here. Further down the
atomic pipeline the driver will decide whether it really wants to
program any of the state or not and then the CRTC's atomic commit
function should program the registers according to whatever value is set
in the state object.
> DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
> return -EINVAL;
> }
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index 71b4c05..84cc3e47 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,160 @@
>
> #include "intel_color_manager.h"
>
> +int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
> + struct drm_crtc *crtc)
> +{
> + struct drm_palette *gamma_data;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct drm_property_blob *blob;
> + struct drm_mode_config *config = >mode_config;
> + u32 cgm_control_reg = 0;
> + u32 cgm_gamma_reg = 0;
> + u32 reg, val;
> + enum pipe pipe;
> + u16 red, green, blue;
> + u32 count = 0;
> + struct drm_r32g32b32 *correction_values = NULL;
> + u32 num_samples;
> + u32 word;
> + u32 palette;
> + int ret = 0, length;
> +
> + blob = drm_property_lookup_blob(dev, blob_id);
> + if (!blob) {
> + DRM_ERROR("Invalid Blob ID\n");
> + return -EINVAL;
> + }
> +
> + gamma_data = (struct drm_palette *)blob->data;
Do we need to validate that the blob we receive is of the expected size
or does something in the DRM core's blob handling take care of that for
us? We don't want userspace to be able to trigger a panic by passing a
smaller than expected blob here.
> +
> + if (gamma_data->version != CHV_GAMMA_DATA_STRUCT_VERSION) {
> + DRM_ERROR("Invalid Gamma Data struct version\n");
> + return -EINVAL;
> + }
> +
> + pipe = to_intel_crtc(crtc)->pipe;
> + num_samples = gamma_data->palette_num_samples;
> + length = num_samples * sizeof(struct drm_r32g32b32);
> +
> + if (num_samples == 0) {
> +
> + /* Disable Gamma functionality on Pipe - CGM Block */
> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
> + cgm_control_reg &= ~CGM_GAMMA_EN;
> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
> +
> +
[Intel-gfx] [PATCH 09/12] drm/i915: Add pipe level Gamma correction for CHV/BSW
Kausal
On Friday 03 July 2015 01:33 PM, Jani Nikula wrote:
> On Thu, 02 Jul 2015, Damien Lespiau wrote:
>> On Wed, Jul 01, 2015 at 09:18:19PM +0530, Kausal Malladi wrote:
>>> From: Kausal Malladi
> I didn't get the series at all, and it's not in the moderation queue
> either. The same happened to the last series from Kausal. What gives?
>
> BR,
> Jani.
Yesterday I realized what was the issue and fixed it. Re-sent patches
today to the entire list. Now I can see all the patches on the archives too.
--Kausal
>
>>> CHV/BSW platform supports various Gamma correction modes, which are:
>>> 1. Legacy 8-bit mode
>>> 2. 10-bit CGM (Color Gamut Mapping) mode
>>>
>>> This patch does the following:
>>> 1. Adds the core function to program Gamma correction values for CHV/BSW
>>> platform
>>> 2. Adds Gamma correction macros/defines
>>>
>>> Signed-off-by: Shashank Sharma
>>> Signed-off-by: Kausal Malladi
>>> ---
>>> drivers/gpu/drm/i915/i915_reg.h| 10 ++
>>> drivers/gpu/drm/i915/intel_atomic.c| 6 ++
>>> drivers/gpu/drm/i915/intel_color_manager.c | 154
>>> +
>>> drivers/gpu/drm/i915/intel_color_manager.h | 12 +++
>>> drivers/gpu/drm/i915/intel_drv.h | 2 +
>>> 5 files changed, 184 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>>> b/drivers/gpu/drm/i915/i915_reg.h
>>> index 313b1f9..36672e7 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -7900,4 +7900,14 @@ enum skl_disp_power_wells {
>>> #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
>>> #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
>>>
>>> +/* Color Management */
>>> +#define PIPEA_CGM_CONTROL (VLV_DISPLAY_BASE + 0x67A00)
>>> +#define PIPEA_CGM_GAMMA_MIN(VLV_DISPLAY_BASE +
>>> 0x67000)
>>> +#define CGM_OFFSET 0x2000
>>> +#define GAMMA_OFFSET 0x2000
>>> +#define _PIPE_CGM_CONTROL(pipe) \
>>> + (PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
>>> +#define _PIPE_GAMMA_BASE(pipe) \
>>> + (PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
>> We use the _PIPE() macro with tha pipe A and B registers for those ,
>> instead of having to defies the offsets.
>>
>>> #endif /* _I915_REG_H_ */
>>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
>>> b/drivers/gpu/drm/i915/intel_atomic.c
>>> index d2674a6..21f0ac2 100644
>>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>>> @@ -473,6 +473,12 @@ int intel_crtc_atomic_set_property(struct drm_crtc
>>> *crtc,
>>>struct drm_property *property,
>>>uint64_t val)
>>> {
>>> + struct drm_device *dev = crtc->dev;
>>> + struct drm_mode_config *config = >mode_config;
>>> +
>>> + if (property == config->prop_palette_after_ctm)
>>> + return intel_color_manager_set_gamma(dev, >base, val);
>>> +
>>> DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>>> return -EINVAL;
>>> }
>> You are touching the hardware instead of staging the configuration?
>>
>>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
>>> b/drivers/gpu/drm/i915/intel_color_manager.c
>>> index 71b4c05..84cc3e47 100644
>>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>>> @@ -27,6 +27,160 @@
>>>
>>> #include "intel_color_manager.h"
>>>
>>> +int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
>>> + struct drm_crtc *crtc)
>>> +{
>>> + if (num_samples == 0) {
>> [...]
>>
>>> + } else if (num_samples == CHV_8BIT_GAMMA_MAX_VALS) {
>> [...]
>>> + } else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS) {
>> [...]
>>
>>> + } else {
>>> + DRM_ERROR("Invalid number of samples for Gamma LUT\n");
>>> + return -EINVAL;
>>> + }
>> This means you're not accepting 256 values, something we do today with
>> the legacy ioctl() and something we probably want for generic userspace.
>>
>>> + ret = drm_property_replace_global_blob(dev, , length,
>>> + (void *) gamma_data, >base,
>>> + config->prop_palette_after_ctm);
>>> +
>>> + if (ret) {
>>> + DRM_ERROR("Error updating Gamma blob\n");
>>> + return -EFAULT;
>>> + }
>>> +
>>> + return ret;
>>> +}
>>> +
>>> +int intel_color_manager_set_gamma(struct drm_device *dev,
>>> + struct drm_mode_object *obj, uint32_t blob_id)
>>> +{
>>> + struct drm_crtc *crtc = obj_to_crtc(obj);
>>> +
>>> + if (IS_CHERRYVIEW(dev))
>>> + return chv_set_gamma(dev, blob_id, crtc);
>>> +
>>> + return -EINVAL;
>>> +}
>>> +
>>> int get_chv_pipe_capabilities(struct drm_device *dev,
>>> struct drm_color_caps *color_caps, struct drm_crtc *crtc)
>>> {
>>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h
>>>
[Intel-gfx] [PATCH 09/12] drm/i915: Add pipe level Gamma correction for CHV/BSW
On Fri, 03 Jul 2015, "Malladi, Kausal" wrote: > On Friday 03 July 2015 01:33 PM, Jani Nikula wrote: >> I didn't get the series at all, and it's not in the moderation queue >> either. The same happened to the last series from Kausal. What gives? >> >> BR, >> Jani. > Yesterday I realized what was the issue and fixed it. Re-sent patches > today to the entire list. Now I can see all the patches on the archives too. Ah okay, sorry for the noise, I wasn't far enough in my inbox yet. :) Thanks, Jani. -- Jani Nikula, Intel Open Source Technology Center
[Intel-gfx] [PATCH 09/12] drm/i915: Add pipe level Gamma correction for CHV/BSW
On Thu, 02 Jul 2015, Damien Lespiau wrote:
> On Wed, Jul 01, 2015 at 09:18:19PM +0530, Kausal Malladi wrote:
>> From: Kausal Malladi
I didn't get the series at all, and it's not in the moderation queue
either. The same happened to the last series from Kausal. What gives?
BR,
Jani.
>>
>> CHV/BSW platform supports various Gamma correction modes, which are:
>> 1. Legacy 8-bit mode
>> 2. 10-bit CGM (Color Gamut Mapping) mode
>>
>> This patch does the following:
>> 1. Adds the core function to program Gamma correction values for CHV/BSW
>>platform
>> 2. Adds Gamma correction macros/defines
>>
>> Signed-off-by: Shashank Sharma
>> Signed-off-by: Kausal Malladi
>> ---
>> drivers/gpu/drm/i915/i915_reg.h| 10 ++
>> drivers/gpu/drm/i915/intel_atomic.c| 6 ++
>> drivers/gpu/drm/i915/intel_color_manager.c | 154
>> +
>> drivers/gpu/drm/i915/intel_color_manager.h | 12 +++
>> drivers/gpu/drm/i915/intel_drv.h | 2 +
>> 5 files changed, 184 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 313b1f9..36672e7 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7900,4 +7900,14 @@ enum skl_disp_power_wells {
>> #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
>> #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
>>
>> +/* Color Management */
>> +#define PIPEA_CGM_CONTROL (VLV_DISPLAY_BASE + 0x67A00)
>> +#define PIPEA_CGM_GAMMA_MIN (VLV_DISPLAY_BASE + 0x67000)
>> +#define CGM_OFFSET 0x2000
>> +#define GAMMA_OFFSET0x2000
>> +#define _PIPE_CGM_CONTROL(pipe) \
>> +(PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
>> +#define _PIPE_GAMMA_BASE(pipe) \
>> +(PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
>
> We use the _PIPE() macro with tha pipe A and B registers for those ,
> instead of having to defies the offsets.
>
>> #endif /* _I915_REG_H_ */
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
>> b/drivers/gpu/drm/i915/intel_atomic.c
>> index d2674a6..21f0ac2 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -473,6 +473,12 @@ int intel_crtc_atomic_set_property(struct drm_crtc
>> *crtc,
>> struct drm_property *property,
>> uint64_t val)
>> {
>> +struct drm_device *dev = crtc->dev;
>> +struct drm_mode_config *config = >mode_config;
>> +
>> +if (property == config->prop_palette_after_ctm)
>> +return intel_color_manager_set_gamma(dev, >base, val);
>> +
>> DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
>> return -EINVAL;
>> }
>
> You are touching the hardware instead of staging the configuration?
>
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
>> b/drivers/gpu/drm/i915/intel_color_manager.c
>> index 71b4c05..84cc3e47 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.c
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
>> @@ -27,6 +27,160 @@
>>
>> #include "intel_color_manager.h"
>>
>> +int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
>> + struct drm_crtc *crtc)
>> +{
>> +if (num_samples == 0) {
>
> [...]
>
>> +} else if (num_samples == CHV_8BIT_GAMMA_MAX_VALS) {
>
> [...]
>> +} else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS) {
>
> [...]
>
>> +} else {
>> +DRM_ERROR("Invalid number of samples for Gamma LUT\n");
>> +return -EINVAL;
>> +}
>
> This means you're not accepting 256 values, something we do today with
> the legacy ioctl() and something we probably want for generic userspace.
>
>> +ret = drm_property_replace_global_blob(dev, , length,
>> +(void *) gamma_data, >base,
>> +config->prop_palette_after_ctm);
>> +
>> +if (ret) {
>> +DRM_ERROR("Error updating Gamma blob\n");
>> +return -EFAULT;
>> +}
>> +
>> +return ret;
>> +}
>> +
>> +int intel_color_manager_set_gamma(struct drm_device *dev,
>> +struct drm_mode_object *obj, uint32_t blob_id)
>> +{
>> +struct drm_crtc *crtc = obj_to_crtc(obj);
>> +
>> +if (IS_CHERRYVIEW(dev))
>> +return chv_set_gamma(dev, blob_id, crtc);
>> +
>> +return -EINVAL;
>> +}
>> +
>> int get_chv_pipe_capabilities(struct drm_device *dev,
>> struct drm_color_caps *color_caps, struct drm_crtc *crtc)
>> {
>> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h
>> b/drivers/gpu/drm/i915/intel_color_manager.h
>> index 32262ac..d83567a 100644
>> --- a/drivers/gpu/drm/i915/intel_color_manager.h
>> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
>> @@ -31,6 +31,8 @@
>> #define CHV_PALETTE_STRUCT_VERSION 1
>> #define CHV_CTM_STRUCT_VERSION 1
>> #define CHV_PLATFORM_STRUCT_VERSION
[PATCH 09/12] drm/i915: Add pipe level Gamma correction for CHV/BSW
CHV/BSW platform supports various Gamma correction modes, which are:
1. Legacy 8-bit mode
2. 10-bit CGM (Color Gamut Mapping) mode
This patch does the following:
1. Adds the core function to program Gamma correction values for CHV/BSW
platform
2. Adds Gamma correction macros/defines
Signed-off-by: Shashank Sharma
Signed-off-by: Kausal Malladi
---
drivers/gpu/drm/i915/i915_reg.h| 10 ++
drivers/gpu/drm/i915/intel_atomic.c| 6 ++
drivers/gpu/drm/i915/intel_color_manager.c | 154 +
drivers/gpu/drm/i915/intel_color_manager.h | 12 +++
drivers/gpu/drm/i915/intel_drv.h | 2 +
5 files changed, 184 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 313b1f9..36672e7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7900,4 +7900,14 @@ enum skl_disp_power_wells {
#define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
#define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
+/* Color Management */
+#define PIPEA_CGM_CONTROL (VLV_DISPLAY_BASE + 0x67A00)
+#define PIPEA_CGM_GAMMA_MIN(VLV_DISPLAY_BASE + 0x67000)
+#define CGM_OFFSET 0x2000
+#define GAMMA_OFFSET 0x2000
+#define _PIPE_CGM_CONTROL(pipe) \
+ (PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
+#define _PIPE_GAMMA_BASE(pipe) \
+ (PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
+
#endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_atomic.c
b/drivers/gpu/drm/i915/intel_atomic.c
index d2674a6..21f0ac2 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -473,6 +473,12 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
struct drm_property *property,
uint64_t val)
{
+ struct drm_device *dev = crtc->dev;
+ struct drm_mode_config *config = >mode_config;
+
+ if (property == config->prop_palette_after_ctm)
+ return intel_color_manager_set_gamma(dev, >base, val);
+
DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
return -EINVAL;
}
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
b/drivers/gpu/drm/i915/intel_color_manager.c
index 71b4c05..84cc3e47 100644
--- a/drivers/gpu/drm/i915/intel_color_manager.c
+++ b/drivers/gpu/drm/i915/intel_color_manager.c
@@ -27,6 +27,160 @@
#include "intel_color_manager.h"
+int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
+ struct drm_crtc *crtc)
+{
+ struct drm_palette *gamma_data;
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct drm_property_blob *blob;
+ struct drm_mode_config *config = >mode_config;
+ u32 cgm_control_reg = 0;
+ u32 cgm_gamma_reg = 0;
+ u32 reg, val;
+ enum pipe pipe;
+ u16 red, green, blue;
+ u32 count = 0;
+ struct drm_r32g32b32 *correction_values = NULL;
+ u32 num_samples;
+ u32 word;
+ u32 palette;
+ int ret = 0, length;
+
+ blob = drm_property_lookup_blob(dev, blob_id);
+ if (!blob) {
+ DRM_ERROR("Invalid Blob ID\n");
+ return -EINVAL;
+ }
+
+ gamma_data = (struct drm_palette *)blob->data;
+
+ if (gamma_data->version != CHV_GAMMA_DATA_STRUCT_VERSION) {
+ DRM_ERROR("Invalid Gamma Data struct version\n");
+ return -EINVAL;
+ }
+
+ pipe = to_intel_crtc(crtc)->pipe;
+ num_samples = gamma_data->palette_num_samples;
+ length = num_samples * sizeof(struct drm_r32g32b32);
+
+ if (num_samples == 0) {
+
+ /* Disable Gamma functionality on Pipe - CGM Block */
+ cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
+ cgm_control_reg &= ~CGM_GAMMA_EN;
+ I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
+
+ DRM_DEBUG_DRIVER("Gamma disabled on Pipe %c\n",
+ pipe_name(pipe));
+ ret = 0;
+ } else if (num_samples == CHV_8BIT_GAMMA_MAX_VALS) {
+
+ /* Disable CGM Gamma, if already set */
+ cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe));
+ cgm_control_reg &= ~CGM_GAMMA_EN;
+ I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg);
+
+ /* Write LUT values to registers */
+ palette = _PIPE_GAMMA_BASE(pipe);
+ count = 0;
+ correction_values =
+ (struct drm_r32g32b32 *)_data->palette_lut;
+ while (count < num_samples) {
+ blue = correction_values[count].b32;
+ green = correction_values[count].g32;
+ red = correction_values[count].r32;
+
+ blue = blue >>
[PATCH 09/12] drm/i915: Add pipe level Gamma correction for CHV/BSW
On Wed, Jul 01, 2015 at 09:18:19PM +0530, Kausal Malladi wrote:
> From: Kausal Malladi
>
> CHV/BSW platform supports various Gamma correction modes, which are:
> 1. Legacy 8-bit mode
> 2. 10-bit CGM (Color Gamut Mapping) mode
>
> This patch does the following:
> 1. Adds the core function to program Gamma correction values for CHV/BSW
>platform
> 2. Adds Gamma correction macros/defines
>
> Signed-off-by: Shashank Sharma
> Signed-off-by: Kausal Malladi
> ---
> drivers/gpu/drm/i915/i915_reg.h| 10 ++
> drivers/gpu/drm/i915/intel_atomic.c| 6 ++
> drivers/gpu/drm/i915/intel_color_manager.c | 154
> +
> drivers/gpu/drm/i915/intel_color_manager.h | 12 +++
> drivers/gpu/drm/i915/intel_drv.h | 2 +
> 5 files changed, 184 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 313b1f9..36672e7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7900,4 +7900,14 @@ enum skl_disp_power_wells {
> #define _PALETTE_A (dev_priv->info.display_mmio_offset + 0xa000)
> #define _PALETTE_B (dev_priv->info.display_mmio_offset + 0xa800)
>
> +/* Color Management */
> +#define PIPEA_CGM_CONTROL(VLV_DISPLAY_BASE + 0x67A00)
> +#define PIPEA_CGM_GAMMA_MIN (VLV_DISPLAY_BASE + 0x67000)
> +#define CGM_OFFSET 0x2000
> +#define GAMMA_OFFSET 0x2000
> +#define _PIPE_CGM_CONTROL(pipe) \
> + (PIPEA_CGM_CONTROL + (pipe * CGM_OFFSET))
> +#define _PIPE_GAMMA_BASE(pipe) \
> + (PIPEA_CGM_GAMMA_MIN + (pipe * GAMMA_OFFSET))
We use the _PIPE() macro with tha pipe A and B registers for those ,
instead of having to defies the offsets.
> #endif /* _I915_REG_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c
> b/drivers/gpu/drm/i915/intel_atomic.c
> index d2674a6..21f0ac2 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -473,6 +473,12 @@ int intel_crtc_atomic_set_property(struct drm_crtc *crtc,
> struct drm_property *property,
> uint64_t val)
> {
> + struct drm_device *dev = crtc->dev;
> + struct drm_mode_config *config = >mode_config;
> +
> + if (property == config->prop_palette_after_ctm)
> + return intel_color_manager_set_gamma(dev, >base, val);
> +
> DRM_DEBUG_KMS("Unknown crtc property '%s'\n", property->name);
> return -EINVAL;
> }
You are touching the hardware instead of staging the configuration?
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.c
> b/drivers/gpu/drm/i915/intel_color_manager.c
> index 71b4c05..84cc3e47 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.c
> +++ b/drivers/gpu/drm/i915/intel_color_manager.c
> @@ -27,6 +27,160 @@
>
> #include "intel_color_manager.h"
>
> +int chv_set_gamma(struct drm_device *dev, uint32_t blob_id,
> + struct drm_crtc *crtc)
> +{
> + if (num_samples == 0) {
[...]
> + } else if (num_samples == CHV_8BIT_GAMMA_MAX_VALS) {
[...]
> + } else if (num_samples == CHV_10BIT_GAMMA_MAX_VALS) {
[...]
> + } else {
> + DRM_ERROR("Invalid number of samples for Gamma LUT\n");
> + return -EINVAL;
> + }
This means you're not accepting 256 values, something we do today with
the legacy ioctl() and something we probably want for generic userspace.
> + ret = drm_property_replace_global_blob(dev, , length,
> + (void *) gamma_data, >base,
> + config->prop_palette_after_ctm);
> +
> + if (ret) {
> + DRM_ERROR("Error updating Gamma blob\n");
> + return -EFAULT;
> + }
> +
> + return ret;
> +}
> +
> +int intel_color_manager_set_gamma(struct drm_device *dev,
> + struct drm_mode_object *obj, uint32_t blob_id)
> +{
> + struct drm_crtc *crtc = obj_to_crtc(obj);
> +
> + if (IS_CHERRYVIEW(dev))
> + return chv_set_gamma(dev, blob_id, crtc);
> +
> + return -EINVAL;
> +}
> +
> int get_chv_pipe_capabilities(struct drm_device *dev,
> struct drm_color_caps *color_caps, struct drm_crtc *crtc)
> {
> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h
> b/drivers/gpu/drm/i915/intel_color_manager.h
> index 32262ac..d83567a 100644
> --- a/drivers/gpu/drm/i915/intel_color_manager.h
> +++ b/drivers/gpu/drm/i915/intel_color_manager.h
> @@ -31,6 +31,8 @@
> #define CHV_PALETTE_STRUCT_VERSION 1
> #define CHV_CTM_STRUCT_VERSION 1
> #define CHV_PLATFORM_STRUCT_VERSION 1
> +#define CHV_GAMMA_DATA_STRUCT_VERSION1
> +
> #define CHV_MAX_PALETTE_CAPS_BEFORE_CTM 1
> #define CHV_MAX_PALETTE_CAPS_AFTER_CTM 2
> #define CHV_DEGAMMA_PRECISION14
> @@ -43,6 +45,16 @@
> #define CHV_10BIT_GAMMA_MAX_VALS
