| Message ID | 1445005751-14918-15-git-send-email-shashank.sharma@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: > FYI - this shouldn't block the commits, but should be optimized later (fairly soon). > > I believe the current implementation ends up executing > while (count < CHV_DEGAMMA_MAX_VALS) { > // Do lots of caclulation > I915_WRITE(cgm_degamma_reg, word); > Every frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. > Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. > > My suggestion here is to set a boolean when the property has been set as > part of a flip and only apply it on the atomic commit after the update > was received. Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. for changed planes tracked in the crtc_state) in the state where we need to commit the update. That /should/ be there really, didn't ever realize it's not done. Note that that for legacy cursors we update them without waiting for vblanks and legacy userspace does that a _lot_ (*cough* X server *cough*), if the overhead is severe this might be a problem and needs to be fixed before merging. -Daniel > > Thanks > Gary > > -----Original Message----- > From: Sharma, Shashank > Sent: Friday, October 16, 2015 3:29 PM > To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Bish, Jim > Cc: Matheson, Annie J; kausalmalladi@gmail.com; Kumar, Kiran S; Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; Sharma, Shashank > Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction > > CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. > > This patch does the following: > 1. Attach deGamma property to CRTC > 2. Add the core function to program DeGamma correction values for > CHV/BSW platform > 2. Add DeGamma correction macros/defines > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> > --- > drivers/gpu/drm/i915/i915_reg.h | 6 ++ > drivers/gpu/drm/i915/intel_color_manager.c | 92 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ > 3 files changed, 103 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ > (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA)) > > +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) > +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) > +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) > +#define _PIPE_DEGAMMA_BASE(pipe) \ > + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, > +PIPEC_CGM_DEGAMMA)) > + > #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 acb9647..1bbad79 100644 > --- a/drivers/gpu/drm/i915/intel_color_manager.c > +++ b/drivers/gpu/drm/i915/intel_color_manager.c > @@ -27,6 +27,92 @@ > > #include "intel_color_manager.h" > > +static int chv_set_degamma(struct drm_device *dev, > + struct drm_property_blob *blob, struct drm_crtc *crtc) { > + u16 red_fract, green_fract, blue_fract; > + u32 red, green, blue; > + u32 num_samples; > + u32 word = 0; > + u32 count, cgm_control_reg, cgm_degamma_reg; > + enum pipe pipe; > + struct drm_palette *degamma_data; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_r32g32b32 *correction_values = NULL; > + struct drm_crtc_state *state = crtc->state; > + > + if (WARN_ON(!blob)) > + return -EINVAL; > + > + degamma_data = (struct drm_palette *)blob->data; > + pipe = to_intel_crtc(crtc)->pipe; > + num_samples = blob->length / sizeof(struct drm_r32g32b32); > + > + switch (num_samples) { > + case GAMMA_DISABLE_VALS: > + /* Disable DeGamma functionality on Pipe - CGM Block */ > + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); > + cgm_control_reg &= ~CGM_DEGAMMA_EN; > + state->palette_before_ctm_blob = NULL; > + > + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); > + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", > + pipe_name(pipe)); > + break; > + > + case CHV_DEGAMMA_MAX_VALS: > + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); > + count = 0; > + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; > + while (count < CHV_DEGAMMA_MAX_VALS) { > + blue = correction_values[count].b32; > + green = correction_values[count].g32; > + red = correction_values[count].r32; > + > + if (blue > CHV_MAX_GAMMA) > + blue = CHV_MAX_GAMMA; > + > + if (green > CHV_MAX_GAMMA) > + green = CHV_MAX_GAMMA; > + > + if (red > CHV_MAX_GAMMA) > + red = CHV_MAX_GAMMA; > + > + blue_fract = GET_BITS(blue, 8, 14); > + green_fract = GET_BITS(green, 8, 14); > + red_fract = GET_BITS(red, 8, 14); > + > + /* Green (29:16) and Blue (13:0) in DWORD1 */ > + SET_BITS(word, green_fract, 16, 14); > + SET_BITS(word, green_fract, 0, 14); > + I915_WRITE(cgm_degamma_reg, word); > + cgm_degamma_reg += 4; > + > + /* Red (13:0) to be written to DWORD2 */ > + word = red_fract; > + I915_WRITE(cgm_degamma_reg, word); > + cgm_degamma_reg += 4; > + count++; > + } > + > + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", > + pipe_name(pipe)); > + > + /* Enable DeGamma on Pipe */ > + I915_WRITE(_PIPE_CGM_CONTROL(pipe), > + I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN); > + > + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", > + pipe_name(pipe)); > + break; > + > + default: > + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); > + return -EINVAL; > + } > + return 0; > +} > + > static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, > struct drm_crtc *crtc) > { > @@ -155,4 +241,10 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev, > DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); > } > > + /* Degamma correction */ > + if (config->cm_palette_before_ctm_property) { > + drm_object_attach_property(mode_obj, > + config->cm_palette_before_ctm_property, 0); > + DRM_DEBUG_DRIVER("degamma property attached to CRTC\n"); > + } > } > diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > index de706d9..77a2119 100644 > --- a/drivers/gpu/drm/i915/intel_color_manager.h > +++ b/drivers/gpu/drm/i915/intel_color_manager.h > @@ -63,5 +63,10 @@ > #define CHV_GAMMA_SHIFT_GREEN 16 > #define CHV_MAX_GAMMA ((1 << 24) - 1) > > +/* Degamma on CHV */ > +#define CHV_DEGAMMA_MSB_SHIFT 2 > +#define CHV_DEGAMMA_GREEN_SHIFT 16 > + > /* CHV CGM Block */ > #define CGM_GAMMA_EN (1 << 2) > +#define CGM_DEGAMMA_EN (1 << 0) > -- > 1.9.1 > > --------------------------------------------------------------------- > Intel Corporation (UK) Limited > Registered No. 1134945 (England) > Registered Office: Pipers Way, Swindon SN3 1RJ > VAT No: 860 2173 47 > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 10/19/2015 11:54 AM, Daniel Vetter wrote: > On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: >> FYI - this shouldn't block the commits, but should be optimized later (fairly soon). >> >> I believe the current implementation ends up executing >> while (count < CHV_DEGAMMA_MAX_VALS) { >> // Do lots of caclulation >> I915_WRITE(cgm_degamma_reg, word); >> Every frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. >> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. >> >> My suggestion here is to set a boolean when the property has been set as >> part of a flip and only apply it on the atomic commit after the update >> was received. > > Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. > for changed planes tracked in the crtc_state) in the state where we need > to commit the update. That /should/ be there really, didn't ever realize > it's not done. Note that that for legacy cursors we update them without > waiting for vblanks and legacy userspace does that a _lot_ (*cough* X > server *cough*), if the overhead is severe this might be a problem and > needs to be fixed before merging. > > -Daniel Severe enough to block? I wanted to get this closed out this week but... I see your point Gary but need a reading on Daniel's last sentence. Jim > >> >> Thanks >> Gary >> >> -----Original Message----- >> From: Sharma, Shashank >> Sent: Friday, October 16, 2015 3:29 PM >> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Bish, Jim >> Cc: Matheson, Annie J; kausalmalladi@gmail.com; Kumar, Kiran S; Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; Sharma, Shashank >> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction >> >> CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. >> >> This patch does the following: >> 1. Attach deGamma property to CRTC >> 2. Add the core function to program DeGamma correction values for >> CHV/BSW platform >> 2. Add DeGamma correction macros/defines >> >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> >> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> >> --- >> drivers/gpu/drm/i915/i915_reg.h | 6 ++ >> drivers/gpu/drm/i915/intel_color_manager.c | 92 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ >> 3 files changed, 103 insertions(+) >> >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 >> --- a/drivers/gpu/drm/i915/i915_reg.h >> +++ b/drivers/gpu/drm/i915/i915_reg.h >> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ >> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA)) >> >> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) >> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) >> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) >> +#define _PIPE_DEGAMMA_BASE(pipe) \ >> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, >> +PIPEC_CGM_DEGAMMA)) >> + >> #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 acb9647..1bbad79 100644 >> --- a/drivers/gpu/drm/i915/intel_color_manager.c >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c >> @@ -27,6 +27,92 @@ >> >> #include "intel_color_manager.h" >> >> +static int chv_set_degamma(struct drm_device *dev, >> + struct drm_property_blob *blob, struct drm_crtc *crtc) { >> + u16 red_fract, green_fract, blue_fract; >> + u32 red, green, blue; >> + u32 num_samples; >> + u32 word = 0; >> + u32 count, cgm_control_reg, cgm_degamma_reg; >> + enum pipe pipe; >> + struct drm_palette *degamma_data; >> + struct drm_i915_private *dev_priv = dev->dev_private; >> + struct drm_r32g32b32 *correction_values = NULL; >> + struct drm_crtc_state *state = crtc->state; >> + >> + if (WARN_ON(!blob)) >> + return -EINVAL; >> + >> + degamma_data = (struct drm_palette *)blob->data; >> + pipe = to_intel_crtc(crtc)->pipe; >> + num_samples = blob->length / sizeof(struct drm_r32g32b32); >> + >> + switch (num_samples) { >> + case GAMMA_DISABLE_VALS: >> + /* Disable DeGamma functionality on Pipe - CGM Block */ >> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); >> + cgm_control_reg &= ~CGM_DEGAMMA_EN; >> + state->palette_before_ctm_blob = NULL; >> + >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); >> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", >> + pipe_name(pipe)); >> + break; >> + >> + case CHV_DEGAMMA_MAX_VALS: >> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); >> + count = 0; >> + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; >> + while (count < CHV_DEGAMMA_MAX_VALS) { >> + blue = correction_values[count].b32; >> + green = correction_values[count].g32; >> + red = correction_values[count].r32; >> + >> + if (blue > CHV_MAX_GAMMA) >> + blue = CHV_MAX_GAMMA; >> + >> + if (green > CHV_MAX_GAMMA) >> + green = CHV_MAX_GAMMA; >> + >> + if (red > CHV_MAX_GAMMA) >> + red = CHV_MAX_GAMMA; >> + >> + blue_fract = GET_BITS(blue, 8, 14); >> + green_fract = GET_BITS(green, 8, 14); >> + red_fract = GET_BITS(red, 8, 14); >> + >> + /* Green (29:16) and Blue (13:0) in DWORD1 */ >> + SET_BITS(word, green_fract, 16, 14); >> + SET_BITS(word, green_fract, 0, 14); >> + I915_WRITE(cgm_degamma_reg, word); >> + cgm_degamma_reg += 4; >> + >> + /* Red (13:0) to be written to DWORD2 */ >> + word = red_fract; >> + I915_WRITE(cgm_degamma_reg, word); >> + cgm_degamma_reg += 4; >> + count++; >> + } >> + >> + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", >> + pipe_name(pipe)); >> + >> + /* Enable DeGamma on Pipe */ >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), >> + I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN); >> + >> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", >> + pipe_name(pipe)); >> + break; >> + >> + default: >> + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); >> + return -EINVAL; >> + } >> + return 0; >> +} >> + >> static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, >> struct drm_crtc *crtc) >> { >> @@ -155,4 +241,10 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev, >> DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); >> } >> >> + /* Degamma correction */ >> + if (config->cm_palette_before_ctm_property) { >> + drm_object_attach_property(mode_obj, >> + config->cm_palette_before_ctm_property, 0); >> + DRM_DEBUG_DRIVER("degamma property attached to CRTC\n"); >> + } >> } >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h >> index de706d9..77a2119 100644 >> --- a/drivers/gpu/drm/i915/intel_color_manager.h >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h >> @@ -63,5 +63,10 @@ >> #define CHV_GAMMA_SHIFT_GREEN 16 >> #define CHV_MAX_GAMMA ((1 << 24) - 1) >> >> +/* Degamma on CHV */ >> +#define CHV_DEGAMMA_MSB_SHIFT 2 >> +#define CHV_DEGAMMA_GREEN_SHIFT 16 >> + >> /* CHV CGM Block */ >> #define CGM_GAMMA_EN (1 << 2) >> +#define CGM_DEGAMMA_EN (1 << 0) >> -- >> 1.9.1 >> >> --------------------------------------------------------------------- >> Intel Corporation (UK) Limited >> Registered No. 1134945 (England) >> Registered Office: Pipers Way, Swindon SN3 1RJ >> VAT No: 860 2173 47 >> >> This e-mail and any attachments may contain confidential material for >> the sole use of the intended recipient(s). Any review or distribution >> by others is strictly prohibited. If you are not the intended >> recipient, please contact the sender and delete all copies. >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >
On Mon, Oct 19, 2015 at 08:39:54PM +0000, Bish, Jim wrote: > > > On 10/19/2015 11:54 AM, Daniel Vetter wrote: > > On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: > >> FYI - this shouldn't block the commits, but should be optimized later (fairly soon). > >> > >> I believe the current implementation ends up executing > >> while (count < CHV_DEGAMMA_MAX_VALS) { > >> // Do lots of caclulation > >> I915_WRITE(cgm_degamma_reg, word); > >> Every frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. > >> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. > >> > >> My suggestion here is to set a boolean when the property has been set as > >> part of a flip and only apply it on the atomic commit after the update > >> was received. > > > > Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. > > for changed planes tracked in the crtc_state) in the state where we need > > to commit the update. That /should/ be there really, didn't ever realize > > it's not done. Note that that for legacy cursors we update them without > > waiting for vblanks and legacy userspace does that a _lot_ (*cough* X > > server *cough*), if the overhead is severe this might be a problem and > > needs to be fixed before merging. > > > > -Daniel > Severe enough to block? I wanted to get this closed out this week but... > I see your point Gary but need a reading on Daniel's last sentence. X server doing silly amounts of setcursor calls is a known issue that has bitten us in the past (with people complaining about seriously sluggish desktops). Just needs to be tested, and depending upon results either fixed before or after merging. I hope we can get away with fixing up post-merge. But writing a few kb through mmio for each cursor is a lot, so it might show up in real world. -Daniel > > Jim > > > >> > >> Thanks > >> Gary > >> > >> -----Original Message----- > >> From: Sharma, Shashank > >> Sent: Friday, October 16, 2015 3:29 PM > >> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Bish, Jim > >> Cc: Matheson, Annie J; kausalmalladi@gmail.com; Kumar, Kiran S; Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; Sharma, Shashank > >> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction > >> > >> CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. > >> > >> This patch does the following: > >> 1. Attach deGamma property to CRTC > >> 2. Add the core function to program DeGamma correction values for > >> CHV/BSW platform > >> 2. Add DeGamma correction macros/defines > >> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > >> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> > >> --- > >> drivers/gpu/drm/i915/i915_reg.h | 6 ++ > >> drivers/gpu/drm/i915/intel_color_manager.c | 92 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ > >> 3 files changed, 103 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ > >> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA)) > >> > >> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) > >> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) > >> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) > >> +#define _PIPE_DEGAMMA_BASE(pipe) \ > >> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, > >> +PIPEC_CGM_DEGAMMA)) > >> + > >> #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 acb9647..1bbad79 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.c > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c > >> @@ -27,6 +27,92 @@ > >> > >> #include "intel_color_manager.h" > >> > >> +static int chv_set_degamma(struct drm_device *dev, > >> + struct drm_property_blob *blob, struct drm_crtc *crtc) { > >> + u16 red_fract, green_fract, blue_fract; > >> + u32 red, green, blue; > >> + u32 num_samples; > >> + u32 word = 0; > >> + u32 count, cgm_control_reg, cgm_degamma_reg; > >> + enum pipe pipe; > >> + struct drm_palette *degamma_data; > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + struct drm_r32g32b32 *correction_values = NULL; > >> + struct drm_crtc_state *state = crtc->state; > >> + > >> + if (WARN_ON(!blob)) > >> + return -EINVAL; > >> + > >> + degamma_data = (struct drm_palette *)blob->data; > >> + pipe = to_intel_crtc(crtc)->pipe; > >> + num_samples = blob->length / sizeof(struct drm_r32g32b32); > >> + > >> + switch (num_samples) { > >> + case GAMMA_DISABLE_VALS: > >> + /* Disable DeGamma functionality on Pipe - CGM Block */ > >> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); > >> + cgm_control_reg &= ~CGM_DEGAMMA_EN; > >> + state->palette_before_ctm_blob = NULL; > >> + > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); > >> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + case CHV_DEGAMMA_MAX_VALS: > >> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); > >> + count = 0; > >> + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; > >> + while (count < CHV_DEGAMMA_MAX_VALS) { > >> + blue = correction_values[count].b32; > >> + green = correction_values[count].g32; > >> + red = correction_values[count].r32; > >> + > >> + if (blue > CHV_MAX_GAMMA) > >> + blue = CHV_MAX_GAMMA; > >> + > >> + if (green > CHV_MAX_GAMMA) > >> + green = CHV_MAX_GAMMA; > >> + > >> + if (red > CHV_MAX_GAMMA) > >> + red = CHV_MAX_GAMMA; > >> + > >> + blue_fract = GET_BITS(blue, 8, 14); > >> + green_fract = GET_BITS(green, 8, 14); > >> + red_fract = GET_BITS(red, 8, 14); > >> + > >> + /* Green (29:16) and Blue (13:0) in DWORD1 */ > >> + SET_BITS(word, green_fract, 16, 14); > >> + SET_BITS(word, green_fract, 0, 14); > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + > >> + /* Red (13:0) to be written to DWORD2 */ > >> + word = red_fract; > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + count++; > >> + } > >> + > >> + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", > >> + pipe_name(pipe)); > >> + > >> + /* Enable DeGamma on Pipe */ > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), > >> + I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN); > >> + > >> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + default: > >> + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); > >> + return -EINVAL; > >> + } > >> + return 0; > >> +} > >> + > >> static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, > >> struct drm_crtc *crtc) > >> { > >> @@ -155,4 +241,10 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev, > >> DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); > >> } > >> > >> + /* Degamma correction */ > >> + if (config->cm_palette_before_ctm_property) { > >> + drm_object_attach_property(mode_obj, > >> + config->cm_palette_before_ctm_property, 0); > >> + DRM_DEBUG_DRIVER("degamma property attached to CRTC\n"); > >> + } > >> } > >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > >> index de706d9..77a2119 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.h > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h > >> @@ -63,5 +63,10 @@ > >> #define CHV_GAMMA_SHIFT_GREEN 16 > >> #define CHV_MAX_GAMMA ((1 << 24) - 1) > >> > >> +/* Degamma on CHV */ > >> +#define CHV_DEGAMMA_MSB_SHIFT 2 > >> +#define CHV_DEGAMMA_GREEN_SHIFT 16 > >> + > >> /* CHV CGM Block */ > >> #define CGM_GAMMA_EN (1 << 2) > >> +#define CGM_DEGAMMA_EN (1 << 0) > >> -- > >> 1.9.1 > >> > >> --------------------------------------------------------------------- > >> Intel Corporation (UK) Limited > >> Registered No. 1134945 (England) > >> Registered Office: Pipers Way, Swindon SN3 1RJ > >> VAT No: 860 2173 47 > >> > >> This e-mail and any attachments may contain confidential material for > >> the sole use of the intended recipient(s). Any review or distribution > >> by others is strictly prohibited. If you are not the intended > >> recipient, please contact the sender and delete all copies. > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >
Bear in mind that it will only happen after the property has been set. Initially there will be no clients setting the property - so I think it should be OK. Thanks Gary Daniel Vetter <daniel@ffwll.ch> wrote: On Mon, Oct 19, 2015 at 08:39:54PM +0000, Bish, Jim wrote: > > > On 10/19/2015 11:54 AM, Daniel Vetter wrote: > > On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: > >> FYI - this shouldn't block the commits, but should be optimized later (fairly soon). > >> > >> I believe the current implementation ends up executing > >> while (count < CHV_DEGAMMA_MAX_VALS) { > >> // Do lots of caclulation > >> I915_WRITE(cgm_degamma_reg, word); > >> Every frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. > >> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. > >> > >> My suggestion here is to set a boolean when the property has been set as > >> part of a flip and only apply it on the atomic commit after the update > >> was received. > > > > Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. > > for changed planes tracked in the crtc_state) in the state where we need > > to commit the update. That /should/ be there really, didn't ever realize > > it's not done. Note that that for legacy cursors we update them without > > waiting for vblanks and legacy userspace does that a _lot_ (*cough* X > > server *cough*), if the overhead is severe this might be a problem and > > needs to be fixed before merging. > > > > -Daniel > Severe enough to block? I wanted to get this closed out this week but... > I see your point Gary but need a reading on Daniel's last sentence. X server doing silly amounts of setcursor calls is a known issue that has bitten us in the past (with people complaining about seriously sluggish desktops). Just needs to be tested, and depending upon results either fixed before or after merging. I hope we can get away with fixing up post-merge. But writing a few kb through mmio for each cursor is a lot, so it might show up in real world. -Daniel > > Jim > > > >> > >> Thanks > >> Gary > >> > >> -----Original Message----- > >> From: Sharma, Shashank > >> Sent: Friday, October 16, 2015 3:29 PM > >> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Bish, Jim > >> Cc: Matheson, Annie J; kausalmalladi@gmail.com; Kumar, Kiran S; Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; Sharma, Shashank > >> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction > >> > >> CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. > >> > >> This patch does the following: > >> 1. Attach deGamma property to CRTC > >> 2. Add the core function to program DeGamma correction values for > >> CHV/BSW platform > >> 2. Add DeGamma correction macros/defines > >> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > >> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> > >> --- > >> drivers/gpu/drm/i915/i915_reg.h | 6 ++ > >> drivers/gpu/drm/i915/intel_color_manager.c | 92 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ > >> 3 files changed, 103 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ > >> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA)) > >> > >> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) > >> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) > >> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) > >> +#define _PIPE_DEGAMMA_BASE(pipe) \ > >> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, > >> +PIPEC_CGM_DEGAMMA)) > >> + > >> #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 acb9647..1bbad79 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.c > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c > >> @@ -27,6 +27,92 @@ > >> > >> #include "intel_color_manager.h" > >> > >> +static int chv_set_degamma(struct drm_device *dev, > >> + struct drm_property_blob *blob, struct drm_crtc *crtc) { > >> + u16 red_fract, green_fract, blue_fract; > >> + u32 red, green, blue; > >> + u32 num_samples; > >> + u32 word = 0; > >> + u32 count, cgm_control_reg, cgm_degamma_reg; > >> + enum pipe pipe; > >> + struct drm_palette *degamma_data; > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + struct drm_r32g32b32 *correction_values = NULL; > >> + struct drm_crtc_state *state = crtc->state; > >> + > >> + if (WARN_ON(!blob)) > >> + return -EINVAL; > >> + > >> + degamma_data = (struct drm_palette *)blob->data; > >> + pipe = to_intel_crtc(crtc)->pipe; > >> + num_samples = blob->length / sizeof(struct drm_r32g32b32); > >> + > >> + switch (num_samples) { > >> + case GAMMA_DISABLE_VALS: > >> + /* Disable DeGamma functionality on Pipe - CGM Block */ > >> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); > >> + cgm_control_reg &= ~CGM_DEGAMMA_EN; > >> + state->palette_before_ctm_blob = NULL; > >> + > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); > >> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + case CHV_DEGAMMA_MAX_VALS: > >> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); > >> + count = 0; > >> + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; > >> + while (count < CHV_DEGAMMA_MAX_VALS) { > >> + blue = correction_values[count].b32; > >> + green = correction_values[count].g32; > >> + red = correction_values[count].r32; > >> + > >> + if (blue > CHV_MAX_GAMMA) > >> + blue = CHV_MAX_GAMMA; > >> + > >> + if (green > CHV_MAX_GAMMA) > >> + green = CHV_MAX_GAMMA; > >> + > >> + if (red > CHV_MAX_GAMMA) > >> + red = CHV_MAX_GAMMA; > >> + > >> + blue_fract = GET_BITS(blue, 8, 14); > >> + green_fract = GET_BITS(green, 8, 14); > >> + red_fract = GET_BITS(red, 8, 14); > >> + > >> + /* Green (29:16) and Blue (13:0) in DWORD1 */ > >> + SET_BITS(word, green_fract, 16, 14); > >> + SET_BITS(word, green_fract, 0, 14); > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + > >> + /* Red (13:0) to be written to DWORD2 */ > >> + word = red_fract; > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + count++; > >> + } > >> + > >> + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", > >> + pipe_name(pipe)); > >> + > >> + /* Enable DeGamma on Pipe */ > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), > >> + I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN); > >> + > >> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + default: > >> + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); > >> + return -EINVAL; > >> + } > >> + return 0; > >> +} > >> + > >> static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, > >> struct drm_crtc *crtc) > >> { > >> @@ -155,4 +241,10 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev, > >> DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); > >> } > >> > >> + /* Degamma correction */ > >> + if (config->cm_palette_before_ctm_property) { > >> + drm_object_attach_property(mode_obj, > >> + config->cm_palette_before_ctm_property, 0); > >> + DRM_DEBUG_DRIVER("degamma property attached to CRTC\n"); > >> + } > >> } > >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > >> index de706d9..77a2119 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.h > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h > >> @@ -63,5 +63,10 @@ > >> #define CHV_GAMMA_SHIFT_GREEN 16 > >> #define CHV_MAX_GAMMA ((1 << 24) - 1) > >> > >> +/* Degamma on CHV */ > >> +#define CHV_DEGAMMA_MSB_SHIFT 2 > >> +#define CHV_DEGAMMA_GREEN_SHIFT 16 > >> + > >> /* CHV CGM Block */ > >> #define CGM_GAMMA_EN (1 << 2) > >> +#define CGM_DEGAMMA_EN (1 << 0) > >> -- > >> 1.9.1 > >> > >> --------------------------------------------------------------------- > >> Intel Corporation (UK) Limited > >> Registered No. 1134945 (England) > >> Registered Office: Pipers Way, Swindon SN3 1RJ > >> VAT No: 860 2173 47 > >> > >> This e-mail and any attachments may contain confidential material for > >> the sole use of the intended recipient(s). Any review or distribution > >> by others is strictly prohibited. If you are not the intended > >> recipient, please contact the sender and delete all copies. > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
Unless legacy mode enables it of course. Thanks Gary "Smith, Gary K" <gary.k.smith@intel.com> wrote: Bear in mind that it will only happen after the property has been set. Initially there will be no clients setting the property - so I think it should be OK. Thanks Gary Daniel Vetter <daniel@ffwll.ch> wrote: On Mon, Oct 19, 2015 at 08:39:54PM +0000, Bish, Jim wrote: > > > On 10/19/2015 11:54 AM, Daniel Vetter wrote: > > On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: > >> FYI - this shouldn't block the commits, but should be optimized later (fairly soon). > >> > >> I believe the current implementation ends up executing > >> while (count < CHV_DEGAMMA_MAX_VALS) { > >> // Do lots of caclulation > >> I915_WRITE(cgm_degamma_reg, word); > >> Every frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. > >> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. > >> > >> My suggestion here is to set a boolean when the property has been set as > >> part of a flip and only apply it on the atomic commit after the update > >> was received. > > > > Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. > > for changed planes tracked in the crtc_state) in the state where we need > > to commit the update. That /should/ be there really, didn't ever realize > > it's not done. Note that that for legacy cursors we update them without > > waiting for vblanks and legacy userspace does that a _lot_ (*cough* X > > server *cough*), if the overhead is severe this might be a problem and > > needs to be fixed before merging. > > > > -Daniel > Severe enough to block? I wanted to get this closed out this week but... > I see your point Gary but need a reading on Daniel's last sentence. X server doing silly amounts of setcursor calls is a known issue that has bitten us in the past (with people complaining about seriously sluggish desktops). Just needs to be tested, and depending upon results either fixed before or after merging. I hope we can get away with fixing up post-merge. But writing a few kb through mmio for each cursor is a lot, so it might show up in real world. -Daniel > > Jim > > > >> > >> Thanks > >> Gary > >> > >> -----Original Message----- > >> From: Sharma, Shashank > >> Sent: Friday, October 16, 2015 3:29 PM > >> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Bish, Jim > >> Cc: Matheson, Annie J; kausalmalladi@gmail.com; Kumar, Kiran S; Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; Sharma, Shashank > >> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction > >> > >> CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. > >> > >> This patch does the following: > >> 1. Attach deGamma property to CRTC > >> 2. Add the core function to program DeGamma correction values for > >> CHV/BSW platform > >> 2. Add DeGamma correction macros/defines > >> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > >> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> > >> --- > >> drivers/gpu/drm/i915/i915_reg.h | 6 ++ > >> drivers/gpu/drm/i915/intel_color_manager.c | 92 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ > >> 3 files changed, 103 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ > >> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA)) > >> > >> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) > >> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) > >> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) > >> +#define _PIPE_DEGAMMA_BASE(pipe) \ > >> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, > >> +PIPEC_CGM_DEGAMMA)) > >> + > >> #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 acb9647..1bbad79 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.c > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c > >> @@ -27,6 +27,92 @@ > >> > >> #include "intel_color_manager.h" > >> > >> +static int chv_set_degamma(struct drm_device *dev, > >> + struct drm_property_blob *blob, struct drm_crtc *crtc) { > >> + u16 red_fract, green_fract, blue_fract; > >> + u32 red, green, blue; > >> + u32 num_samples; > >> + u32 word = 0; > >> + u32 count, cgm_control_reg, cgm_degamma_reg; > >> + enum pipe pipe; > >> + struct drm_palette *degamma_data; > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + struct drm_r32g32b32 *correction_values = NULL; > >> + struct drm_crtc_state *state = crtc->state; > >> + > >> + if (WARN_ON(!blob)) > >> + return -EINVAL; > >> + > >> + degamma_data = (struct drm_palette *)blob->data; > >> + pipe = to_intel_crtc(crtc)->pipe; > >> + num_samples = blob->length / sizeof(struct drm_r32g32b32); > >> + > >> + switch (num_samples) { > >> + case GAMMA_DISABLE_VALS: > >> + /* Disable DeGamma functionality on Pipe - CGM Block */ > >> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); > >> + cgm_control_reg &= ~CGM_DEGAMMA_EN; > >> + state->palette_before_ctm_blob = NULL; > >> + > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); > >> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + case CHV_DEGAMMA_MAX_VALS: > >> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); > >> + count = 0; > >> + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; > >> + while (count < CHV_DEGAMMA_MAX_VALS) { > >> + blue = correction_values[count].b32; > >> + green = correction_values[count].g32; > >> + red = correction_values[count].r32; > >> + > >> + if (blue > CHV_MAX_GAMMA) > >> + blue = CHV_MAX_GAMMA; > >> + > >> + if (green > CHV_MAX_GAMMA) > >> + green = CHV_MAX_GAMMA; > >> + > >> + if (red > CHV_MAX_GAMMA) > >> + red = CHV_MAX_GAMMA; > >> + > >> + blue_fract = GET_BITS(blue, 8, 14); > >> + green_fract = GET_BITS(green, 8, 14); > >> + red_fract = GET_BITS(red, 8, 14); > >> + > >> + /* Green (29:16) and Blue (13:0) in DWORD1 */ > >> + SET_BITS(word, green_fract, 16, 14); > >> + SET_BITS(word, green_fract, 0, 14); > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + > >> + /* Red (13:0) to be written to DWORD2 */ > >> + word = red_fract; > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + count++; > >> + } > >> + > >> + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", > >> + pipe_name(pipe)); > >> + > >> + /* Enable DeGamma on Pipe */ > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), > >> + I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN); > >> + > >> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + default: > >> + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); > >> + return -EINVAL; > >> + } > >> + return 0; > >> +} > >> + > >> static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, > >> struct drm_crtc *crtc) > >> { > >> @@ -155,4 +241,10 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev, > >> DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); > >> } > >> > >> + /* Degamma correction */ > >> + if (config->cm_palette_before_ctm_property) { > >> + drm_object_attach_property(mode_obj, > >> + config->cm_palette_before_ctm_property, 0); > >> + DRM_DEBUG_DRIVER("degamma property attached to CRTC\n"); > >> + } > >> } > >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > >> index de706d9..77a2119 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.h > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h > >> @@ -63,5 +63,10 @@ > >> #define CHV_GAMMA_SHIFT_GREEN 16 > >> #define CHV_MAX_GAMMA ((1 << 24) - 1) > >> > >> +/* Degamma on CHV */ > >> +#define CHV_DEGAMMA_MSB_SHIFT 2 > >> +#define CHV_DEGAMMA_GREEN_SHIFT 16 > >> + > >> /* CHV CGM Block */ > >> #define CGM_GAMMA_EN (1 << 2) > >> +#define CGM_DEGAMMA_EN (1 << 0) > >> -- > >> 1.9.1 > >> > >> --------------------------------------------------------------------- > >> Intel Corporation (UK) Limited > >> Registered No. 1134945 (England) > >> Registered Office: Pipers Way, Swindon SN3 1RJ > >> VAT No: 860 2173 47 > >> > >> This e-mail and any attachments may contain confidential material for > >> the sole use of the intended recipient(s). Any review or distribution > >> by others is strictly prohibited. If you are not the intended > >> recipient, please contact the sender and delete all copies. > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch --------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
Daniel? Annie Matheson Intel Corporation Phone: (503) 712-0586 Email: annie.j.matheson@intel.com<mailto:annie.j.von.moos@intel.com> From: Smith, Gary K Sent: Monday, October 19, 2015 3:27 PM To: Daniel Vetter Cc: Bish, Jim; Sharma, Shashank; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Matheson, Annie J; kausalmalladi@gmail.com; Vetter, Daniel Subject: Re: [Intel-gfx] [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction Unless legacy mode enables it of course. Thanks Gary "Smith, Gary K" <gary.k.smith@intel.com<mailto:gary.k.smith@intel.com>> wrote: Bear in mind that it will only happen after the property has been set. Initially there will be no clients setting the property - so I think it should be OK. Thanks Gary Daniel Vetter <daniel@ffwll.ch<mailto:daniel@ffwll.ch>> wrote: On Mon, Oct 19, 2015 at 08:39:54PM +0000, Bish, Jim wrote: > > > On 10/19/2015 11:54 AM, Daniel Vetter wrote: > > On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: > >> FYI - this shouldn't block the commits, but should be optimized later (fairly soon). > >> > >> I believe the current implementation ends up executing > >> while (count < CHV_DEGAMMA_MAX_VALS) { > >> // Do lots of caclulation > >> I915_WRITE(cgm_degamma_reg, word); > >> Every frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. > >> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. > >> > >> My suggestion here is to set a boolean when the property has been set as > >> part of a flip and only apply it on the atomic commit after the update > >> was received. > > > > Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. > > for changed planes tracked in the crtc_state) in the state where we need > > to commit the update. That /should/ be there really, didn't ever realize > > it's not done. Note that that for legacy cursors we update them without > > waiting for vblanks and legacy userspace does that a _lot_ (*cough* X > > server *cough*), if the overhead is severe this might be a problem and > > needs to be fixed before merging. > > > > -Daniel > Severe enough to block? I wanted to get this closed out this week but... > I see your point Gary but need a reading on Daniel's last sentence. X server doing silly amounts of setcursor calls is a known issue that has bitten us in the past (with people complaining about seriously sluggish desktops). Just needs to be tested, and depending upon results either fixed before or after merging. I hope we can get away with fixing up post-merge. But writing a few kb through mmio for each cursor is a lot, so it might show up in real world. -Daniel > > Jim > > > >> > >> Thanks > >> Gary > >> > >> -----Original Message----- > >> From: Sharma, Shashank > >> Sent: Friday, October 16, 2015 3:29 PM > >> To: dri-devel@lists.freedesktop.org<mailto:dri-devel@lists.freedesktop.org>; intel-gfx@lists.freedesktop.org<mailto:intel-gfx@lists.freedesktop.org>; emil.l.velikov@gmail.com<mailto:emil.l.velikov@gmail.com>; Roper, Matthew D; Bradford, Robert; Bish, Jim > >> Cc: Matheson, Annie J; kausalmalladi@gmail.com<mailto:kausalmalladi@gmail.com>; Kumar, Kiran S; Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; Sharma, Shashank > >> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction > >> > >> CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. > >> > >> This patch does the following: > >> 1. Attach deGamma property to CRTC > >> 2. Add the core function to program DeGamma correction values for > >> CHV/BSW platform > >> 2. Add DeGamma correction macros/defines > >> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com<mailto:shashank.sharma@intel.com>> > >> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com<mailto:kausalmalladi@gmail.com>> > >> --- > >> drivers/gpu/drm/i915/i915_reg.h | 6 ++ > >> drivers/gpu/drm/i915/intel_color_manager.c | 92 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ > >> 3 files changed, 103 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ > >> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA)) > >> > >> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) > >> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) > >> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) > >> +#define _PIPE_DEGAMMA_BASE(pipe) \ > >> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, > >> +PIPEC_CGM_DEGAMMA)) > >> + > >> #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 acb9647..1bbad79 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.c > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c > >> @@ -27,6 +27,92 @@ > >> > >> #include "intel_color_manager.h" > >> > >> +static int chv_set_degamma(struct drm_device *dev, > >> + struct drm_property_blob *blob, struct drm_crtc *crtc) { > >> + u16 red_fract, green_fract, blue_fract; > >> + u32 red, green, blue; > >> + u32 num_samples; > >> + u32 word = 0; > >> + u32 count, cgm_control_reg, cgm_degamma_reg; > >> + enum pipe pipe; > >> + struct drm_palette *degamma_data; > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + struct drm_r32g32b32 *correction_values = NULL; > >> + struct drm_crtc_state *state = crtc->state; > >> + > >> + if (WARN_ON(!blob)) > >> + return -EINVAL; > >> + > >> + degamma_data = (struct drm_palette *)blob->data; > >> + pipe = to_intel_crtc(crtc)->pipe; > >> + num_samples = blob->length / sizeof(struct drm_r32g32b32); > >> + > >> + switch (num_samples) { > >> + case GAMMA_DISABLE_VALS: > >> + /* Disable DeGamma functionality on Pipe - CGM Block */ > >> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); > >> + cgm_control_reg &= ~CGM_DEGAMMA_EN; > >> + state->palette_before_ctm_blob = NULL; > >> + > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); > >> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + case CHV_DEGAMMA_MAX_VALS: > >> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); > >> + count = 0; > >> + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; > >> + while (count < CHV_DEGAMMA_MAX_VALS) { > >> + blue = correction_values[count].b32; > >> + green = correction_values[count].g32; > >> + red = correction_values[count].r32; > >> + > >> + if (blue > CHV_MAX_GAMMA) > >> + blue = CHV_MAX_GAMMA; > >> + > >> + if (green > CHV_MAX_GAMMA) > >> + green = CHV_MAX_GAMMA; > >> + > >> + if (red > CHV_MAX_GAMMA) > >> + red = CHV_MAX_GAMMA; > >> + > >> + blue_fract = GET_BITS(blue, 8, 14); > >> + green_fract = GET_BITS(green, 8, 14); > >> + red_fract = GET_BITS(red, 8, 14); > >> + > >> + /* Green (29:16) and Blue (13:0) in DWORD1 */ > >> + SET_BITS(word, green_fract, 16, 14); > >> + SET_BITS(word, green_fract, 0, 14); > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + > >> + /* Red (13:0) to be written to DWORD2 */ > >> + word = red_fract; > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + count++; > >> + } > >> + > >> + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", > >> + pipe_name(pipe)); > >> + > >> + /* Enable DeGamma on Pipe */ > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), > >> + I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN); > >> + > >> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + default: > >> + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); > >> + return -EINVAL; > >> + } > >> + return 0; > >> +} > >> + > >> static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, > >> struct drm_crtc *crtc) > >> { > >> @@ -155,4 +241,10 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev, > >> DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); > >> } > >> > >> + /* Degamma correction */ > >> + if (config->cm_palette_before_ctm_property) { > >> + drm_object_attach_property(mode_obj, > >> + config->cm_palette_before_ctm_property, 0); > >> + DRM_DEBUG_DRIVER("degamma property attached to CRTC\n"); > >> + } > >> } > >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > >> index de706d9..77a2119 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.h > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h > >> @@ -63,5 +63,10 @@ > >> #define CHV_GAMMA_SHIFT_GREEN 16 > >> #define CHV_MAX_GAMMA ((1 << 24) - 1) > >> > >> +/* Degamma on CHV */ > >> +#define CHV_DEGAMMA_MSB_SHIFT 2 > >> +#define CHV_DEGAMMA_GREEN_SHIFT 16 > >> + > >> /* CHV CGM Block */ > >> #define CGM_GAMMA_EN (1 << 2) > >> +#define CGM_DEGAMMA_EN (1 << 0) > >> -- > >> 1.9.1 > >> > >> --------------------------------------------------------------------- > >> Intel Corporation (UK) Limited > >> Registered No. 1134945 (England) > >> Registered Office: Pipers Way, Swindon SN3 1RJ > >> VAT No: 860 2173 47 > >> > >> This e-mail and any attachments may contain confidential material for > >> the sole use of the intended recipient(s). Any review or distribution > >> by others is strictly prohibited. If you are not the intended > >> recipient, please contact the sender and delete all copies. > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org<mailto:Intel-gfx@lists.freedesktop.org> > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Sounds like a good suggestion to me, it would be efficient to do so. By the time we discus on this, I would see a possibility of adding one patch on top of the series, with this optimization. Regards Shashank From: Matheson, Annie J Sent: Tuesday, October 20, 2015 5:18 AM To: Smith, Gary K; Daniel Vetter Cc: Bish, Jim; Sharma, Shashank; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; kausalmalladi@gmail.com; Vetter, Daniel; Gilliam, Amy Subject: RE: [Intel-gfx] [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction Daniel? Annie Matheson Intel Corporation Phone: (503) 712-0586 Email: annie.j.matheson@intel.com<mailto:annie.j.von.moos@intel.com> From: Smith, Gary K Sent: Monday, October 19, 2015 3:27 PM To: Daniel Vetter Cc: Bish, Jim; Sharma, Shashank; dri-devel@lists.freedesktop.org<mailto:dri-devel@lists.freedesktop.org>; intel-gfx@lists.freedesktop.org<mailto:intel-gfx@lists.freedesktop.org>; emil.l.velikov@gmail.com<mailto:emil.l.velikov@gmail.com>; Roper, Matthew D; Bradford, Robert; Matheson, Annie J; kausalmalladi@gmail.com<mailto:kausalmalladi@gmail.com>; Vetter, Daniel Subject: Re: [Intel-gfx] [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction Unless legacy mode enables it of course. Thanks Gary "Smith, Gary K" <gary.k.smith@intel.com<mailto:gary.k.smith@intel.com>> wrote: Bear in mind that it will only happen after the property has been set. Initially there will be no clients setting the property - so I think it should be OK. Thanks Gary Daniel Vetter <daniel@ffwll.ch<mailto:daniel@ffwll.ch>> wrote: On Mon, Oct 19, 2015 at 08:39:54PM +0000, Bish, Jim wrote: > > > On 10/19/2015 11:54 AM, Daniel Vetter wrote: > > On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: > >> FYI - this shouldn't block the commits, but should be optimized later (fairly soon). > >> > >> I believe the current implementation ends up executing > >> while (count < CHV_DEGAMMA_MAX_VALS) { > >> // Do lots of caclulation > >> I915_WRITE(cgm_degamma_reg, word); > >> Every frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. > >> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. > >> > >> My suggestion here is to set a boolean when the property has been set as > >> part of a flip and only apply it on the atomic commit after the update > >> was received. > > > > Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. > > for changed planes tracked in the crtc_state) in the state where we need > > to commit the update. That /should/ be there really, didn't ever realize > > it's not done. Note that that for legacy cursors we update them without > > waiting for vblanks and legacy userspace does that a _lot_ (*cough* X > > server *cough*), if the overhead is severe this might be a problem and > > needs to be fixed before merging. > > > > -Daniel > Severe enough to block? I wanted to get this closed out this week but... > I see your point Gary but need a reading on Daniel's last sentence. X server doing silly amounts of setcursor calls is a known issue that has bitten us in the past (with people complaining about seriously sluggish desktops). Just needs to be tested, and depending upon results either fixed before or after merging. I hope we can get away with fixing up post-merge. But writing a few kb through mmio for each cursor is a lot, so it might show up in real world. -Daniel > > Jim > > > >> > >> Thanks > >> Gary > >> > >> -----Original Message----- > >> From: Sharma, Shashank > >> Sent: Friday, October 16, 2015 3:29 PM > >> To: dri-devel@lists.freedesktop.org<mailto:dri-devel@lists.freedesktop.org>; intel-gfx@lists.freedesktop.org<mailto:intel-gfx@lists.freedesktop.org>; emil.l.velikov@gmail.com<mailto:emil.l.velikov@gmail.com>; Roper, Matthew D; Bradford, Robert; Bish, Jim > >> Cc: Matheson, Annie J; kausalmalladi@gmail.com<mailto:kausalmalladi@gmail.com>; Kumar, Kiran S; Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; Sharma, Shashank > >> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction > >> > >> CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. > >> > >> This patch does the following: > >> 1. Attach deGamma property to CRTC > >> 2. Add the core function to program DeGamma correction values for > >> CHV/BSW platform > >> 2. Add DeGamma correction macros/defines > >> > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com<mailto:shashank.sharma@intel.com>> > >> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com<mailto:kausalmalladi@gmail.com>> > >> --- > >> drivers/gpu/drm/i915/i915_reg.h | 6 ++ > >> drivers/gpu/drm/i915/intel_color_manager.c | 92 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ > >> 3 files changed, 103 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 > >> --- a/drivers/gpu/drm/i915/i915_reg.h > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > >> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ > >> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA)) > >> > >> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) > >> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) > >> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) > >> +#define _PIPE_DEGAMMA_BASE(pipe) \ > >> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, > >> +PIPEC_CGM_DEGAMMA)) > >> + > >> #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 acb9647..1bbad79 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.c > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c > >> @@ -27,6 +27,92 @@ > >> > >> #include "intel_color_manager.h" > >> > >> +static int chv_set_degamma(struct drm_device *dev, > >> + struct drm_property_blob *blob, struct drm_crtc *crtc) { > >> + u16 red_fract, green_fract, blue_fract; > >> + u32 red, green, blue; > >> + u32 num_samples; > >> + u32 word = 0; > >> + u32 count, cgm_control_reg, cgm_degamma_reg; > >> + enum pipe pipe; > >> + struct drm_palette *degamma_data; > >> + struct drm_i915_private *dev_priv = dev->dev_private; > >> + struct drm_r32g32b32 *correction_values = NULL; > >> + struct drm_crtc_state *state = crtc->state; > >> + > >> + if (WARN_ON(!blob)) > >> + return -EINVAL; > >> + > >> + degamma_data = (struct drm_palette *)blob->data; > >> + pipe = to_intel_crtc(crtc)->pipe; > >> + num_samples = blob->length / sizeof(struct drm_r32g32b32); > >> + > >> + switch (num_samples) { > >> + case GAMMA_DISABLE_VALS: > >> + /* Disable DeGamma functionality on Pipe - CGM Block */ > >> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); > >> + cgm_control_reg &= ~CGM_DEGAMMA_EN; > >> + state->palette_before_ctm_blob = NULL; > >> + > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); > >> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + case CHV_DEGAMMA_MAX_VALS: > >> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); > >> + count = 0; > >> + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; > >> + while (count < CHV_DEGAMMA_MAX_VALS) { > >> + blue = correction_values[count].b32; > >> + green = correction_values[count].g32; > >> + red = correction_values[count].r32; > >> + > >> + if (blue > CHV_MAX_GAMMA) > >> + blue = CHV_MAX_GAMMA; > >> + > >> + if (green > CHV_MAX_GAMMA) > >> + green = CHV_MAX_GAMMA; > >> + > >> + if (red > CHV_MAX_GAMMA) > >> + red = CHV_MAX_GAMMA; > >> + > >> + blue_fract = GET_BITS(blue, 8, 14); > >> + green_fract = GET_BITS(green, 8, 14); > >> + red_fract = GET_BITS(red, 8, 14); > >> + > >> + /* Green (29:16) and Blue (13:0) in DWORD1 */ > >> + SET_BITS(word, green_fract, 16, 14); > >> + SET_BITS(word, green_fract, 0, 14); > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + > >> + /* Red (13:0) to be written to DWORD2 */ > >> + word = red_fract; > >> + I915_WRITE(cgm_degamma_reg, word); > >> + cgm_degamma_reg += 4; > >> + count++; > >> + } > >> + > >> + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", > >> + pipe_name(pipe)); > >> + > >> + /* Enable DeGamma on Pipe */ > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), > >> + I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN); > >> + > >> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", > >> + pipe_name(pipe)); > >> + break; > >> + > >> + default: > >> + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); > >> + return -EINVAL; > >> + } > >> + return 0; > >> +} > >> + > >> static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, > >> struct drm_crtc *crtc) > >> { > >> @@ -155,4 +241,10 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev, > >> DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); > >> } > >> > >> + /* Degamma correction */ > >> + if (config->cm_palette_before_ctm_property) { > >> + drm_object_attach_property(mode_obj, > >> + config->cm_palette_before_ctm_property, 0); > >> + DRM_DEBUG_DRIVER("degamma property attached to CRTC\n"); > >> + } > >> } > >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > >> index de706d9..77a2119 100644 > >> --- a/drivers/gpu/drm/i915/intel_color_manager.h > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h > >> @@ -63,5 +63,10 @@ > >> #define CHV_GAMMA_SHIFT_GREEN 16 > >> #define CHV_MAX_GAMMA ((1 << 24) - 1) > >> > >> +/* Degamma on CHV */ > >> +#define CHV_DEGAMMA_MSB_SHIFT 2 > >> +#define CHV_DEGAMMA_GREEN_SHIFT 16 > >> + > >> /* CHV CGM Block */ > >> #define CGM_GAMMA_EN (1 << 2) > >> +#define CGM_DEGAMMA_EN (1 << 0) > >> -- > >> 1.9.1 > >> > >> --------------------------------------------------------------------- > >> Intel Corporation (UK) Limited > >> Registered No. 1134945 (England) > >> Registered Office: Pipers Way, Swindon SN3 1RJ > >> VAT No: 860 2173 47 > >> > >> This e-mail and any attachments may contain confidential material for > >> the sole use of the intended recipient(s). Any review or distribution > >> by others is strictly prohibited. If you are not the intended > >> recipient, please contact the sender and delete all copies. > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org<mailto:Intel-gfx@lists.freedesktop.org> > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
On Mon, Oct 19, 2015 at 10:27:17PM +0000, Smith, Gary K wrote: > Unless legacy mode enables it of course. I thought we decided to ignore legacy gamma stuff (at least for now, until someone complains about the inconsistency). So yeah, I think we're fine. -Daniel > > Thanks > Gary > > > "Smith, Gary K" <gary.k.smith@intel.com> wrote: > > Bear in mind that it will only happen after the property has been set. Initially there will be no clients setting the property - so I think it should be OK. > > Thanks > Gary > > > Daniel Vetter <daniel@ffwll.ch> wrote: > > On Mon, Oct 19, 2015 at 08:39:54PM +0000, Bish, Jim wrote: > > > > > > On 10/19/2015 11:54 AM, Daniel Vetter wrote: > > > On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: > > >> FYI - this shouldn't block the commits, but should be optimized later (fairly soon). > > >> > > >> I believe the current implementation ends up executing > > >> while (count < CHV_DEGAMMA_MAX_VALS) { > > >> // Do lots of caclulation > > >> I915_WRITE(cgm_degamma_reg, word); > > >> Every frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. > > >> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. > > >> > > >> My suggestion here is to set a boolean when the property has been set as > > >> part of a flip and only apply it on the atomic commit after the update > > >> was received. > > > > > > Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. > > > for changed planes tracked in the crtc_state) in the state where we need > > > to commit the update. That /should/ be there really, didn't ever realize > > > it's not done. Note that that for legacy cursors we update them without > > > waiting for vblanks and legacy userspace does that a _lot_ (*cough* X > > > server *cough*), if the overhead is severe this might be a problem and > > > needs to be fixed before merging. > > > > > > -Daniel > > Severe enough to block? I wanted to get this closed out this week but... > > I see your point Gary but need a reading on Daniel's last sentence. > > X server doing silly amounts of setcursor calls is a known issue that has > bitten us in the past (with people complaining about seriously sluggish > desktops). Just needs to be tested, and depending upon results either > fixed before or after merging. I hope we can get away with fixing up > post-merge. But writing a few kb through mmio for each cursor is a lot, so > it might show up in real world. > -Daniel > > > > > Jim > > > > > >> > > >> Thanks > > >> Gary > > >> > > >> -----Original Message----- > > >> From: Sharma, Shashank > > >> Sent: Friday, October 16, 2015 3:29 PM > > >> To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Bish, Jim > > >> Cc: Matheson, Annie J; kausalmalladi@gmail.com; Kumar, Kiran S; Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; Sharma, Shashank > > >> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction > > >> > > >> CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. > > >> > > >> This patch does the following: > > >> 1. Attach deGamma property to CRTC > > >> 2. Add the core function to program DeGamma correction values for > > >> CHV/BSW platform > > >> 2. Add DeGamma correction macros/defines > > >> > > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > > >> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> > > >> --- > > >> drivers/gpu/drm/i915/i915_reg.h | 6 ++ > > >> drivers/gpu/drm/i915/intel_color_manager.c | 92 ++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ > > >> 3 files changed, 103 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 > > >> --- a/drivers/gpu/drm/i915/i915_reg.h > > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > > >> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ > > >> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA)) > > >> > > >> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) > > >> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) > > >> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) > > >> +#define _PIPE_DEGAMMA_BASE(pipe) \ > > >> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, > > >> +PIPEC_CGM_DEGAMMA)) > > >> + > > >> #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 acb9647..1bbad79 100644 > > >> --- a/drivers/gpu/drm/i915/intel_color_manager.c > > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c > > >> @@ -27,6 +27,92 @@ > > >> > > >> #include "intel_color_manager.h" > > >> > > >> +static int chv_set_degamma(struct drm_device *dev, > > >> + struct drm_property_blob *blob, struct drm_crtc *crtc) { > > >> + u16 red_fract, green_fract, blue_fract; > > >> + u32 red, green, blue; > > >> + u32 num_samples; > > >> + u32 word = 0; > > >> + u32 count, cgm_control_reg, cgm_degamma_reg; > > >> + enum pipe pipe; > > >> + struct drm_palette *degamma_data; > > >> + struct drm_i915_private *dev_priv = dev->dev_private; > > >> + struct drm_r32g32b32 *correction_values = NULL; > > >> + struct drm_crtc_state *state = crtc->state; > > >> + > > >> + if (WARN_ON(!blob)) > > >> + return -EINVAL; > > >> + > > >> + degamma_data = (struct drm_palette *)blob->data; > > >> + pipe = to_intel_crtc(crtc)->pipe; > > >> + num_samples = blob->length / sizeof(struct drm_r32g32b32); > > >> + > > >> + switch (num_samples) { > > >> + case GAMMA_DISABLE_VALS: > > >> + /* Disable DeGamma functionality on Pipe - CGM Block */ > > >> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); > > >> + cgm_control_reg &= ~CGM_DEGAMMA_EN; > > >> + state->palette_before_ctm_blob = NULL; > > >> + > > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); > > >> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", > > >> + pipe_name(pipe)); > > >> + break; > > >> + > > >> + case CHV_DEGAMMA_MAX_VALS: > > >> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); > > >> + count = 0; > > >> + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; > > >> + while (count < CHV_DEGAMMA_MAX_VALS) { > > >> + blue = correction_values[count].b32; > > >> + green = correction_values[count].g32; > > >> + red = correction_values[count].r32; > > >> + > > >> + if (blue > CHV_MAX_GAMMA) > > >> + blue = CHV_MAX_GAMMA; > > >> + > > >> + if (green > CHV_MAX_GAMMA) > > >> + green = CHV_MAX_GAMMA; > > >> + > > >> + if (red > CHV_MAX_GAMMA) > > >> + red = CHV_MAX_GAMMA; > > >> + > > >> + blue_fract = GET_BITS(blue, 8, 14); > > >> + green_fract = GET_BITS(green, 8, 14); > > >> + red_fract = GET_BITS(red, 8, 14); > > >> + > > >> + /* Green (29:16) and Blue (13:0) in DWORD1 */ > > >> + SET_BITS(word, green_fract, 16, 14); > > >> + SET_BITS(word, green_fract, 0, 14); > > >> + I915_WRITE(cgm_degamma_reg, word); > > >> + cgm_degamma_reg += 4; > > >> + > > >> + /* Red (13:0) to be written to DWORD2 */ > > >> + word = red_fract; > > >> + I915_WRITE(cgm_degamma_reg, word); > > >> + cgm_degamma_reg += 4; > > >> + count++; > > >> + } > > >> + > > >> + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", > > >> + pipe_name(pipe)); > > >> + > > >> + /* Enable DeGamma on Pipe */ > > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), > > >> + I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN); > > >> + > > >> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", > > >> + pipe_name(pipe)); > > >> + break; > > >> + > > >> + default: > > >> + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); > > >> + return -EINVAL; > > >> + } > > >> + return 0; > > >> +} > > >> + > > >> static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, > > >> struct drm_crtc *crtc) > > >> { > > >> @@ -155,4 +241,10 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev, > > >> DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); > > >> } > > >> > > >> + /* Degamma correction */ > > >> + if (config->cm_palette_before_ctm_property) { > > >> + drm_object_attach_property(mode_obj, > > >> + config->cm_palette_before_ctm_property, 0); > > >> + DRM_DEBUG_DRIVER("degamma property attached to CRTC\n"); > > >> + } > > >> } > > >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h > > >> index de706d9..77a2119 100644 > > >> --- a/drivers/gpu/drm/i915/intel_color_manager.h > > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h > > >> @@ -63,5 +63,10 @@ > > >> #define CHV_GAMMA_SHIFT_GREEN 16 > > >> #define CHV_MAX_GAMMA ((1 << 24) - 1) > > >> > > >> +/* Degamma on CHV */ > > >> +#define CHV_DEGAMMA_MSB_SHIFT 2 > > >> +#define CHV_DEGAMMA_GREEN_SHIFT 16 > > >> + > > >> /* CHV CGM Block */ > > >> #define CGM_GAMMA_EN (1 << 2) > > >> +#define CGM_DEGAMMA_EN (1 << 0) > > >> -- > > >> 1.9.1 > > >> > > >> --------------------------------------------------------------------- > > >> Intel Corporation (UK) Limited > > >> Registered No. 1134945 (England) > > >> Registered Office: Pipers Way, Swindon SN3 1RJ > > >> VAT No: 860 2173 47 > > >> > > >> This e-mail and any attachments may contain confidential material for > > >> the sole use of the intended recipient(s). Any review or distribution > > >> by others is strictly prohibited. If you are not the intended > > >> recipient, please contact the sender and delete all copies. > > >> > > >> _______________________________________________ > > >> Intel-gfx mailing list > > >> Intel-gfx@lists.freedesktop.org > > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > --------------------------------------------------------------------- > Intel Corporation (UK) Limited > Registered No. 1134945 (England) > Registered Office: Pipers Way, Swindon SN3 1RJ > VAT No: 860 2173 47 > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. >
Yes, please note that as per the discussion, the legacy gamma IOCTL is still existing, to maintain the backward compatibility, we have not broken it. And we have added provision to program legacy-8bit gamma via color manager also, so everyone should be happy :) Regards Shashank -----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, October 20, 2015 12:58 PM To: Smith, Gary K Cc: Daniel Vetter; Bish, Jim; Sharma, Shashank; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Matheson, Annie J; kausalmalladi@gmail.com; Vetter, Daniel Subject: Re: [Intel-gfx] [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction On Mon, Oct 19, 2015 at 10:27:17PM +0000, Smith, Gary K wrote: > Unless legacy mode enables it of course. I thought we decided to ignore legacy gamma stuff (at least for now, until someone complains about the inconsistency). So yeah, I think we're fine. -Daniel > > Thanks > Gary > > > "Smith, Gary K" <gary.k.smith@intel.com> wrote: > > Bear in mind that it will only happen after the property has been set. Initially there will be no clients setting the property - so I think it should be OK. > > Thanks > Gary > > > Daniel Vetter <daniel@ffwll.ch> wrote: > > On Mon, Oct 19, 2015 at 08:39:54PM +0000, Bish, Jim wrote: > > > > > > On 10/19/2015 11:54 AM, Daniel Vetter wrote: > > > On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: > > >> FYI - this shouldn't block the commits, but should be optimized later (fairly soon). > > >> > > >> I believe the current implementation ends up executing > > >> while (count < CHV_DEGAMMA_MAX_VALS) { > > >> // Do lots of caclulation > > >> I915_WRITE(cgm_degamma_reg, word); Every > > >> frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. > > >> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. > > >> > > >> My suggestion here is to set a boolean when the property has been > > >> set as part of a flip and only apply it on the atomic commit > > >> after the update was received. > > > > > > Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. > > > for changed planes tracked in the crtc_state) in the state where > > > we need to commit the update. That /should/ be there really, > > > didn't ever realize it's not done. Note that that for legacy > > > cursors we update them without waiting for vblanks and legacy > > > userspace does that a _lot_ (*cough* X server *cough*), if the > > > overhead is severe this might be a problem and needs to be fixed before merging. > > > > > > -Daniel > > Severe enough to block? I wanted to get this closed out this week but... > > I see your point Gary but need a reading on Daniel's last sentence. > > X server doing silly amounts of setcursor calls is a known issue that > has bitten us in the past (with people complaining about seriously > sluggish desktops). Just needs to be tested, and depending upon > results either fixed before or after merging. I hope we can get away > with fixing up post-merge. But writing a few kb through mmio for each > cursor is a lot, so it might show up in real world. > -Daniel > > > > > Jim > > > > > >> > > >> Thanks > > >> Gary > > >> > > >> -----Original Message----- > > >> From: Sharma, Shashank > > >> Sent: Friday, October 16, 2015 3:29 PM > > >> To: dri-devel@lists.freedesktop.org; > > >> intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, > > >> Matthew D; Bradford, Robert; Bish, Jim > > >> Cc: Matheson, Annie J; kausalmalladi@gmail.com; Kumar, Kiran S; > > >> Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, > > >> Avinash Reddy; Sharma, Shashank > > >> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma > > >> correction > > >> > > >> CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. > > >> > > >> This patch does the following: > > >> 1. Attach deGamma property to CRTC 2. Add the core function to > > >> program DeGamma correction values for > > >> CHV/BSW platform > > >> 2. Add DeGamma correction macros/defines > > >> > > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > > >> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> > > >> --- > > >> drivers/gpu/drm/i915/i915_reg.h | 6 ++ > > >> drivers/gpu/drm/i915/intel_color_manager.c | 92 > > >> ++++++++++++++++++++++++++++++ > > >> drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ > > >> 3 files changed, 103 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h > > >> b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 > > >> --- a/drivers/gpu/drm/i915/i915_reg.h > > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > > >> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ > > >> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, > > >> PIPEC_CGM_GAMMA)) > > >> > > >> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) > > >> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) > > >> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) > > >> +#define _PIPE_DEGAMMA_BASE(pipe) \ > > >> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, > > >> +PIPEC_CGM_DEGAMMA)) > > >> + > > >> #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 acb9647..1bbad79 100644 > > >> --- a/drivers/gpu/drm/i915/intel_color_manager.c > > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c > > >> @@ -27,6 +27,92 @@ > > >> > > >> #include "intel_color_manager.h" > > >> > > >> +static int chv_set_degamma(struct drm_device *dev, > > >> + struct drm_property_blob *blob, struct drm_crtc *crtc) { > > >> + u16 red_fract, green_fract, blue_fract; > > >> + u32 red, green, blue; > > >> + u32 num_samples; > > >> + u32 word = 0; > > >> + u32 count, cgm_control_reg, cgm_degamma_reg; > > >> + enum pipe pipe; > > >> + struct drm_palette *degamma_data; > > >> + struct drm_i915_private *dev_priv = dev->dev_private; > > >> + struct drm_r32g32b32 *correction_values = NULL; > > >> + struct drm_crtc_state *state = crtc->state; > > >> + > > >> + if (WARN_ON(!blob)) > > >> + return -EINVAL; > > >> + > > >> + degamma_data = (struct drm_palette *)blob->data; pipe = > > >> + to_intel_crtc(crtc)->pipe; num_samples = blob->length / > > >> + sizeof(struct drm_r32g32b32); > > >> + > > >> + switch (num_samples) { > > >> + case GAMMA_DISABLE_VALS: > > >> + /* Disable DeGamma functionality on Pipe - CGM Block */ > > >> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); > > >> + cgm_control_reg &= ~CGM_DEGAMMA_EN; > > >> + state->palette_before_ctm_blob = NULL; > > >> + > > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); > > >> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", > > >> + pipe_name(pipe)); > > >> + break; > > >> + > > >> + case CHV_DEGAMMA_MAX_VALS: > > >> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); > > >> + count = 0; > > >> + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; > > >> + while (count < CHV_DEGAMMA_MAX_VALS) { > > >> + blue = correction_values[count].b32; > > >> + green = correction_values[count].g32; > > >> + red = correction_values[count].r32; > > >> + > > >> + if (blue > CHV_MAX_GAMMA) > > >> + blue = CHV_MAX_GAMMA; > > >> + > > >> + if (green > CHV_MAX_GAMMA) > > >> + green = CHV_MAX_GAMMA; > > >> + > > >> + if (red > CHV_MAX_GAMMA) > > >> + red = CHV_MAX_GAMMA; > > >> + > > >> + blue_fract = GET_BITS(blue, 8, 14); > > >> + green_fract = GET_BITS(green, 8, 14); > > >> + red_fract = GET_BITS(red, 8, 14); > > >> + > > >> + /* Green (29:16) and Blue (13:0) in DWORD1 */ > > >> + SET_BITS(word, green_fract, 16, 14); > > >> + SET_BITS(word, green_fract, 0, 14); > > >> + I915_WRITE(cgm_degamma_reg, word); > > >> + cgm_degamma_reg += 4; > > >> + > > >> + /* Red (13:0) to be written to DWORD2 */ > > >> + word = red_fract; > > >> + I915_WRITE(cgm_degamma_reg, word); > > >> + cgm_degamma_reg += 4; > > >> + count++; > > >> + } > > >> + > > >> + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", > > >> + pipe_name(pipe)); > > >> + > > >> + /* Enable DeGamma on Pipe */ > > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), > > >> + I915_READ(_PIPE_CGM_CONTROL(pipe)) | > > >> + CGM_DEGAMMA_EN); > > >> + > > >> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", > > >> + pipe_name(pipe)); > > >> + break; > > >> + > > >> + default: > > >> + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); > > >> + return -EINVAL; > > >> + } > > >> + return 0; > > >> +} > > >> + > > >> static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, > > >> struct drm_crtc *crtc) { @@ -155,4 +241,10 @@ void > > >> intel_attach_color_properties_to_crtc(struct drm_device *dev, > > >> DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); > > >> } > > >> > > >> + /* Degamma correction */ > > >> + if (config->cm_palette_before_ctm_property) { > > >> + drm_object_attach_property(mode_obj, > > >> + config->cm_palette_before_ctm_property, 0); > > >> + DRM_DEBUG_DRIVER("degamma property attached to > > >> + CRTC\n"); } > > >> } > > >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h > > >> b/drivers/gpu/drm/i915/intel_color_manager.h > > >> index de706d9..77a2119 100644 > > >> --- a/drivers/gpu/drm/i915/intel_color_manager.h > > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h > > >> @@ -63,5 +63,10 @@ > > >> #define CHV_GAMMA_SHIFT_GREEN 16 > > >> #define CHV_MAX_GAMMA ((1 << 24) - 1) > > >> > > >> +/* Degamma on CHV */ > > >> +#define CHV_DEGAMMA_MSB_SHIFT 2 > > >> +#define CHV_DEGAMMA_GREEN_SHIFT 16 > > >> + > > >> /* CHV CGM Block */ > > >> #define CGM_GAMMA_EN (1 << 2) > > >> +#define CGM_DEGAMMA_EN (1 << 0) > > >> -- > > >> 1.9.1 > > >> > > >> ----------------------------------------------------------------- > > >> ---- > > >> Intel Corporation (UK) Limited > > >> Registered No. 1134945 (England) > > >> Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 > > >> 47 > > >> > > >> This e-mail and any attachments may contain confidential material > > >> for the sole use of the intended recipient(s). Any review or > > >> distribution by others is strictly prohibited. If you are not the > > >> intended recipient, please contact the sender and delete all copies. > > >> > > >> _______________________________________________ > > >> Intel-gfx mailing list > > >> Intel-gfx@lists.freedesktop.org > > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > --------------------------------------------------------------------- > Intel Corporation (UK) Limited > Registered No. 1134945 (England) > Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
We have added two patches to optimize multiple commit calls, to address Gary's comment, using one additional flag in CRTC state. We have tested this, and it's working for both Android and Linux. I am sending this new patch set now (v7), which has these two additional patches, in total 25 in count. Please have a look. Regards Shashank -----Original Message----- From: Sharma, Shashank Sent: Tuesday, October 20, 2015 1:46 PM To: 'Daniel Vetter'; Smith, Gary K Cc: Bish, Jim; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Matheson, Annie J; kausalmalladi@gmail.com; Vetter, Daniel Subject: RE: [Intel-gfx] [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction Yes, please note that as per the discussion, the legacy gamma IOCTL is still existing, to maintain the backward compatibility, we have not broken it. And we have added provision to program legacy-8bit gamma via color manager also, so everyone should be happy :) Regards Shashank -----Original Message----- From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter Sent: Tuesday, October 20, 2015 12:58 PM To: Smith, Gary K Cc: Daniel Vetter; Bish, Jim; Sharma, Shashank; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, Matthew D; Bradford, Robert; Matheson, Annie J; kausalmalladi@gmail.com; Vetter, Daniel Subject: Re: [Intel-gfx] [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma correction On Mon, Oct 19, 2015 at 10:27:17PM +0000, Smith, Gary K wrote: > Unless legacy mode enables it of course. I thought we decided to ignore legacy gamma stuff (at least for now, until someone complains about the inconsistency). So yeah, I think we're fine. -Daniel > > Thanks > Gary > > > "Smith, Gary K" <gary.k.smith@intel.com> wrote: > > Bear in mind that it will only happen after the property has been set. Initially there will be no clients setting the property - so I think it should be OK. > > Thanks > Gary > > > Daniel Vetter <daniel@ffwll.ch> wrote: > > On Mon, Oct 19, 2015 at 08:39:54PM +0000, Bish, Jim wrote: > > > > > > On 10/19/2015 11:54 AM, Daniel Vetter wrote: > > > On Mon, Oct 19, 2015 at 06:08:52PM +0000, Smith, Gary K wrote: > > >> FYI - this shouldn't block the commits, but should be optimized later (fairly soon). > > >> > > >> I believe the current implementation ends up executing > > >> while (count < CHV_DEGAMMA_MAX_VALS) { > > >> // Do lots of caclulation > > >> I915_WRITE(cgm_degamma_reg, word); Every > > >> frame after you set the property, whether you change the contents of the blob or not. Clearly this is really inefficient when there are 100s of gamma values to write. > > >> Same with the Gamma and CTM. CTM is less of an issue with only 9 entries. > > >> > > >> My suggestion here is to set a boolean when the property has been > > >> set as part of a flip and only apply it on the atomic commit > > >> after the update was received. > > > > > > Yeah the usual design is to add a foo_changed boolean (or bitmask, e.g. > > > for changed planes tracked in the crtc_state) in the state where > > > we need to commit the update. That /should/ be there really, > > > didn't ever realize it's not done. Note that that for legacy > > > cursors we update them without waiting for vblanks and legacy > > > userspace does that a _lot_ (*cough* X server *cough*), if the > > > overhead is severe this might be a problem and needs to be fixed before merging. > > > > > > -Daniel > > Severe enough to block? I wanted to get this closed out this week but... > > I see your point Gary but need a reading on Daniel's last sentence. > > X server doing silly amounts of setcursor calls is a known issue that > has bitten us in the past (with people complaining about seriously > sluggish desktops). Just needs to be tested, and depending upon > results either fixed before or after merging. I hope we can get away > with fixing up post-merge. But writing a few kb through mmio for each > cursor is a lot, so it might show up in real world. > -Daniel > > > > > Jim > > > > > >> > > >> Thanks > > >> Gary > > >> > > >> -----Original Message----- > > >> From: Sharma, Shashank > > >> Sent: Friday, October 16, 2015 3:29 PM > > >> To: dri-devel@lists.freedesktop.org; > > >> intel-gfx@lists.freedesktop.org; emil.l.velikov@gmail.com; Roper, > > >> Matthew D; Bradford, Robert; Bish, Jim > > >> Cc: Matheson, Annie J; kausalmalladi@gmail.com; Kumar, Kiran S; > > >> Smith, Gary K; Vetter, Daniel; Mukherjee, Indranil; Palleti, > > >> Avinash Reddy; Sharma, Shashank > > >> Subject: [PATCH v6 14/23] drm/i915: CHV: Pipe level degamma > > >> correction > > >> > > >> CHV/BSW supports Degamma color correction, which linearizes all the non-linear color values. This will be applied before Color Transformation. > > >> > > >> This patch does the following: > > >> 1. Attach deGamma property to CRTC 2. Add the core function to > > >> program DeGamma correction values for > > >> CHV/BSW platform > > >> 2. Add DeGamma correction macros/defines > > >> > > >> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > > >> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> > > >> --- > > >> drivers/gpu/drm/i915/i915_reg.h | 6 ++ > > >> drivers/gpu/drm/i915/intel_color_manager.c | 92 > > >> ++++++++++++++++++++++++++++++ > > >> drivers/gpu/drm/i915/intel_color_manager.h | 5 ++ > > >> 3 files changed, 103 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/i915/i915_reg.h > > >> b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 > > >> --- a/drivers/gpu/drm/i915/i915_reg.h > > >> +++ b/drivers/gpu/drm/i915/i915_reg.h > > >> @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ > > >> (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, > > >> PIPEC_CGM_GAMMA)) > > >> > > >> +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) > > >> +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) > > >> +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) > > >> +#define _PIPE_DEGAMMA_BASE(pipe) \ > > >> + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, > > >> +PIPEC_CGM_DEGAMMA)) > > >> + > > >> #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 acb9647..1bbad79 100644 > > >> --- a/drivers/gpu/drm/i915/intel_color_manager.c > > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.c > > >> @@ -27,6 +27,92 @@ > > >> > > >> #include "intel_color_manager.h" > > >> > > >> +static int chv_set_degamma(struct drm_device *dev, > > >> + struct drm_property_blob *blob, struct drm_crtc *crtc) { > > >> + u16 red_fract, green_fract, blue_fract; > > >> + u32 red, green, blue; > > >> + u32 num_samples; > > >> + u32 word = 0; > > >> + u32 count, cgm_control_reg, cgm_degamma_reg; > > >> + enum pipe pipe; > > >> + struct drm_palette *degamma_data; > > >> + struct drm_i915_private *dev_priv = dev->dev_private; > > >> + struct drm_r32g32b32 *correction_values = NULL; > > >> + struct drm_crtc_state *state = crtc->state; > > >> + > > >> + if (WARN_ON(!blob)) > > >> + return -EINVAL; > > >> + > > >> + degamma_data = (struct drm_palette *)blob->data; pipe = > > >> + to_intel_crtc(crtc)->pipe; num_samples = blob->length / > > >> + sizeof(struct drm_r32g32b32); > > >> + > > >> + switch (num_samples) { > > >> + case GAMMA_DISABLE_VALS: > > >> + /* Disable DeGamma functionality on Pipe - CGM Block */ > > >> + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); > > >> + cgm_control_reg &= ~CGM_DEGAMMA_EN; > > >> + state->palette_before_ctm_blob = NULL; > > >> + > > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); > > >> + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", > > >> + pipe_name(pipe)); > > >> + break; > > >> + > > >> + case CHV_DEGAMMA_MAX_VALS: > > >> + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); > > >> + count = 0; > > >> + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; > > >> + while (count < CHV_DEGAMMA_MAX_VALS) { > > >> + blue = correction_values[count].b32; > > >> + green = correction_values[count].g32; > > >> + red = correction_values[count].r32; > > >> + > > >> + if (blue > CHV_MAX_GAMMA) > > >> + blue = CHV_MAX_GAMMA; > > >> + > > >> + if (green > CHV_MAX_GAMMA) > > >> + green = CHV_MAX_GAMMA; > > >> + > > >> + if (red > CHV_MAX_GAMMA) > > >> + red = CHV_MAX_GAMMA; > > >> + > > >> + blue_fract = GET_BITS(blue, 8, 14); > > >> + green_fract = GET_BITS(green, 8, 14); > > >> + red_fract = GET_BITS(red, 8, 14); > > >> + > > >> + /* Green (29:16) and Blue (13:0) in DWORD1 */ > > >> + SET_BITS(word, green_fract, 16, 14); > > >> + SET_BITS(word, green_fract, 0, 14); > > >> + I915_WRITE(cgm_degamma_reg, word); > > >> + cgm_degamma_reg += 4; > > >> + > > >> + /* Red (13:0) to be written to DWORD2 */ > > >> + word = red_fract; > > >> + I915_WRITE(cgm_degamma_reg, word); > > >> + cgm_degamma_reg += 4; > > >> + count++; > > >> + } > > >> + > > >> + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", > > >> + pipe_name(pipe)); > > >> + > > >> + /* Enable DeGamma on Pipe */ > > >> + I915_WRITE(_PIPE_CGM_CONTROL(pipe), > > >> + I915_READ(_PIPE_CGM_CONTROL(pipe)) | > > >> + CGM_DEGAMMA_EN); > > >> + > > >> + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", > > >> + pipe_name(pipe)); > > >> + break; > > >> + > > >> + default: > > >> + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); > > >> + return -EINVAL; > > >> + } > > >> + return 0; > > >> +} > > >> + > > >> static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, > > >> struct drm_crtc *crtc) { @@ -155,4 +241,10 @@ void > > >> intel_attach_color_properties_to_crtc(struct drm_device *dev, > > >> DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); > > >> } > > >> > > >> + /* Degamma correction */ > > >> + if (config->cm_palette_before_ctm_property) { > > >> + drm_object_attach_property(mode_obj, > > >> + config->cm_palette_before_ctm_property, 0); > > >> + DRM_DEBUG_DRIVER("degamma property attached to > > >> + CRTC\n"); } > > >> } > > >> diff --git a/drivers/gpu/drm/i915/intel_color_manager.h > > >> b/drivers/gpu/drm/i915/intel_color_manager.h > > >> index de706d9..77a2119 100644 > > >> --- a/drivers/gpu/drm/i915/intel_color_manager.h > > >> +++ b/drivers/gpu/drm/i915/intel_color_manager.h > > >> @@ -63,5 +63,10 @@ > > >> #define CHV_GAMMA_SHIFT_GREEN 16 > > >> #define CHV_MAX_GAMMA ((1 << 24) - 1) > > >> > > >> +/* Degamma on CHV */ > > >> +#define CHV_DEGAMMA_MSB_SHIFT 2 > > >> +#define CHV_DEGAMMA_GREEN_SHIFT 16 > > >> + > > >> /* CHV CGM Block */ > > >> #define CGM_GAMMA_EN (1 << 2) > > >> +#define CGM_DEGAMMA_EN (1 << 0) > > >> -- > > >> 1.9.1 > > >> > > >> ----------------------------------------------------------------- > > >> ---- > > >> Intel Corporation (UK) Limited > > >> Registered No. 1134945 (England) > > >> Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 > > >> 47 > > >> > > >> This e-mail and any attachments may contain confidential material > > >> for the sole use of the intended recipient(s). Any review or > > >> distribution by others is strictly prohibited. If you are not the > > >> intended recipient, please contact the sender and delete all copies. > > >> > > >> _______________________________________________ > > >> Intel-gfx mailing list > > >> Intel-gfx@lists.freedesktop.org > > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > --------------------------------------------------------------------- > Intel Corporation (UK) Limited > Registered No. 1134945 (England) > Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47 > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 45ddd84..1e46562 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8160,4 +8160,10 @@ enum skl_disp_power_wells { #define _PIPE_GAMMA_BASE(pipe) \ (_PIPE3(pipe, PIPEA_CGM_GAMMA, PIPEB_CGM_GAMMA, PIPEC_CGM_GAMMA)) +#define PIPEA_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x66000) +#define PIPEB_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x68000) +#define PIPEC_CGM_DEGAMMA (VLV_DISPLAY_BASE + 0x6A000) +#define _PIPE_DEGAMMA_BASE(pipe) \ + (_PIPE3(pipe, PIPEA_CGM_DEGAMMA, PIPEB_CGM_DEGAMMA, PIPEC_CGM_DEGAMMA)) + #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 acb9647..1bbad79 100644 --- a/drivers/gpu/drm/i915/intel_color_manager.c +++ b/drivers/gpu/drm/i915/intel_color_manager.c @@ -27,6 +27,92 @@ #include "intel_color_manager.h" +static int chv_set_degamma(struct drm_device *dev, + struct drm_property_blob *blob, struct drm_crtc *crtc) +{ + u16 red_fract, green_fract, blue_fract; + u32 red, green, blue; + u32 num_samples; + u32 word = 0; + u32 count, cgm_control_reg, cgm_degamma_reg; + enum pipe pipe; + struct drm_palette *degamma_data; + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_r32g32b32 *correction_values = NULL; + struct drm_crtc_state *state = crtc->state; + + if (WARN_ON(!blob)) + return -EINVAL; + + degamma_data = (struct drm_palette *)blob->data; + pipe = to_intel_crtc(crtc)->pipe; + num_samples = blob->length / sizeof(struct drm_r32g32b32); + + switch (num_samples) { + case GAMMA_DISABLE_VALS: + /* Disable DeGamma functionality on Pipe - CGM Block */ + cgm_control_reg = I915_READ(_PIPE_CGM_CONTROL(pipe)); + cgm_control_reg &= ~CGM_DEGAMMA_EN; + state->palette_before_ctm_blob = NULL; + + I915_WRITE(_PIPE_CGM_CONTROL(pipe), cgm_control_reg); + DRM_DEBUG_DRIVER("DeGamma disabled on Pipe %c\n", + pipe_name(pipe)); + break; + + case CHV_DEGAMMA_MAX_VALS: + cgm_degamma_reg = _PIPE_DEGAMMA_BASE(pipe); + count = 0; + correction_values = (struct drm_r32g32b32 *)°amma_data->lut; + while (count < CHV_DEGAMMA_MAX_VALS) { + blue = correction_values[count].b32; + green = correction_values[count].g32; + red = correction_values[count].r32; + + if (blue > CHV_MAX_GAMMA) + blue = CHV_MAX_GAMMA; + + if (green > CHV_MAX_GAMMA) + green = CHV_MAX_GAMMA; + + if (red > CHV_MAX_GAMMA) + red = CHV_MAX_GAMMA; + + blue_fract = GET_BITS(blue, 8, 14); + green_fract = GET_BITS(green, 8, 14); + red_fract = GET_BITS(red, 8, 14); + + /* Green (29:16) and Blue (13:0) in DWORD1 */ + SET_BITS(word, green_fract, 16, 14); + SET_BITS(word, green_fract, 0, 14); + I915_WRITE(cgm_degamma_reg, word); + cgm_degamma_reg += 4; + + /* Red (13:0) to be written to DWORD2 */ + word = red_fract; + I915_WRITE(cgm_degamma_reg, word); + cgm_degamma_reg += 4; + count++; + } + + DRM_DEBUG_DRIVER("DeGamma LUT loaded for Pipe %c\n", + pipe_name(pipe)); + + /* Enable DeGamma on Pipe */ + I915_WRITE(_PIPE_CGM_CONTROL(pipe), + I915_READ(_PIPE_CGM_CONTROL(pipe)) | CGM_DEGAMMA_EN); + + DRM_DEBUG_DRIVER("DeGamma correction enabled on Pipe %c\n", + pipe_name(pipe)); + break; + + default: + DRM_ERROR("Invalid number of samples for DeGamma LUT\n"); + return -EINVAL; + } + return 0; +} + static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, struct drm_crtc *crtc) { @@ -155,4 +241,10 @@ void intel_attach_color_properties_to_crtc(struct drm_device *dev, DRM_DEBUG_DRIVER("gamma property attached to CRTC\n"); } + /* Degamma correction */ + if (config->cm_palette_before_ctm_property) { + drm_object_attach_property(mode_obj, + config->cm_palette_before_ctm_property, 0); + DRM_DEBUG_DRIVER("degamma property attached to CRTC\n"); + } } diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h index de706d9..77a2119 100644 --- a/drivers/gpu/drm/i915/intel_color_manager.h +++ b/drivers/gpu/drm/i915/intel_color_manager.h @@ -63,5 +63,10 @@ #define CHV_GAMMA_SHIFT_GREEN 16 #define CHV_MAX_GAMMA ((1 << 24) - 1) +/* Degamma on CHV */ +#define CHV_DEGAMMA_MSB_SHIFT 2 +#define CHV_DEGAMMA_GREEN_SHIFT 16 + /* CHV CGM Block */ #define CGM_GAMMA_EN (1 << 2) +#define CGM_DEGAMMA_EN (1 << 0)