Regards Shashank
On 10/10/2015 4:37 AM, Emil Velikov wrote:
Hi Shashank,
On 9 October 2015 at 20:29, Shashank Sharma shashank.sharma@intel.com wrote:
CHV/BSW platform supports two different pipe level gamma correction modes, which are:
- Legacy 8-bit mode
- 10-bit CGM (Color Gamut Mapping) mode
This patch does the following:
- Attaches Gamma property to CRTC
- Adds the core Gamma correction function for CHV/BSW
- Adds Gamma correction macros
Signed-off-by: Shashank Sharma shashank.sharma@intel.com Signed-off-by: Kausal Malladi kausalmalladi@gmail.com
drivers/gpu/drm/i915/i915_reg.h | 12 +++ drivers/gpu/drm/i915/intel_color_manager.c | 114 +++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 13 ++++ 3 files changed, 139 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 60e6a73..885ac8a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8038,4 +8038,16 @@ enum skl_disp_power_wells { #define GEN9_VEBOX_MOCS_0 0xcb00 /* Video MOCS base register*/ #define GEN9_BLT_MOCS_0 0xcc00 /* Blitter MOCS base register*/
+/* Color Management */ +#define PIPEA_CGM_CONTROL (VLV_DISPLAY_BASE + 0x67A00) +#define PIPEB_CGM_CONTROL (VLV_DISPLAY_BASE + 0x69A00) +#define PIPEC_CGM_CONTROL (VLV_DISPLAY_BASE + 0x6BA00) +#define PIPEA_CGM_GAMMA (VLV_DISPLAY_BASE + 0x67000) +#define PIPEB_CGM_GAMMA (VLV_DISPLAY_BASE + 0x69000) +#define PIPEC_CGM_GAMMA (VLV_DISPLAY_BASE + 0x6B000) +#define _PIPE_CGM_CONTROL(pipe) \
(_PIPE3(pipe, PIPEA_CGM_CONTROL, PIPEB_CGM_CONTROL, PIPEC_CGM_CONTROL))+#define _PIPE_GAMMA_BASE(pipe) \
(_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA))- #endif /* _I915_REG_H_ */
diff --git a/drivers/gpu/drm/i915/intel_color_manager.c b/drivers/gpu/drm/i915/intel_color_manager.c index e466748..cf381b8 100644 --- a/drivers/gpu/drm/i915/intel_color_manager.c +++ b/drivers/gpu/drm/i915/intel_color_manager.c @@ -27,6 +27,112 @@
#include "intel_color_manager.h"
+static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob,
struct drm_crtc *crtc)+{
bool flag = false;enum pipe pipe;u16 red_fract, green_fract, blue_fract;u32 red, green, blue, num_samples;u32 word = 0;u32 count = 0;u32 cgm_gamma_reg = 0;u32 cgm_control_reg = 0;int ret = 0, length;struct drm_r32g32b32 *correction_values = NULL;You can drop the useless initialization of correction_values. Same goes for patches 19 and 21.
Agree, thanks for pointing it out.
struct drm_palette *gamma_data;struct drm_i915_private *dev_priv = dev->dev_private;if (WARN_ON(!blob))return -EINVAL;gamma_data = (struct drm_palette *)blob->data;pipe = to_intel_crtc(crtc)->pipe;num_samples = gamma_data->num_samples;length = num_samples * sizeof(struct drm_r32g32b32);Calculation can overflow.
good catch, will take care of this.
switch (num_samples) {case GAMMA_DISABLE_VALS:/* 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));Indentation looks wrong here.
Oops, bloody SI :).
ret = 0;Drop the variable and return 0, at the bottom of the function ?
Let me check this out.
break;case CHV_8BIT_GAMMA_MAX_VALS:case CHV_10BIT_GAMMA_MAX_VALS:count = 0;cgm_gamma_reg = _PIPE_GAMMA_BASE(pipe);correction_values = (struct drm_r32g32b32 *)&gamma_data->lut;while (count < num_samples) {Using for(i = 0;....) loop seems the more common approach ?
Nah, we are good with while. The whole color management series prefers while (and me too.... :))
[snip]
/** On CHV, the best supported Gamma capability is* CGM block, that takes 257 samples.* If user gives 256 samples (legacy mode), then* duplicate the 256th value to 257th.* "flag" is used to make sure it happens only once*/if (num_samples == CHV_8BIT_GAMMA_MAX_VALS &&count == CHV_8BIT_GAMMA_MAX_VALS && !flag) {count--;flag = true;}There is little point in going over this if statement 256 odd times. Split it out of the loop ?
Makes sense for sure, let me try this out.
}/* Enable (CGM) Gamma on Pipe */I915_WRITE(_PIPE_CGM_CONTROL(pipe),I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_GAMMA_EN);DRM_DEBUG_DRIVER("CGM Gamma enabled on Pipe %c\n",pipe_name(pipe));Indentation.
Guilty.
ret = 0;Drop the variable ?
Agree
break;default:DRM_ERROR("Invalid number of samples for Gamma LUT\n");ret = -EINVAL;return -EINVAL;
Agree
}return ret;return 0;
Agree
Regards, Emil