Message ID | 1442425040-32185-22-git-send-email-shashank.sharma@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Shashank, some feedback below that you would be great to get addressed before your next version. On Wed, 2015-09-16 at 23:07 +0530, Shashank Sharma wrote: > BDW/SKL/BXT platforms support various Gamma correction modes > which are: > 1. Legacy 8-bit mode > 2. 10-bit mode > 3. 10-bit Split Gamma mode > 4. 12-bit mode > > This patch does the following: > 1. Adds the core function to program Gamma correction values > for BDW/SKL/BXT platforms > 2. Adds Gamma 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 | 17 +- > drivers/gpu/drm/i915/intel_color_manager.c | 270 > +++++++++++++++++++++++++++++ *snip* > +static u32 bdw_write_10bit_gamma_precision(u32 red, u32 green, u32 > blue) > +{ > + u32 word; > + u8 blue_int, green_int, red_int; > + u16 blue_fract, green_fract, red_fract; > + > + blue_int = _GAMMA_INT_PART(blue); > + if (blue_int > GAMMA_INT_MAX) > + blue = BDW_MAX_GAMMA; > + > + green_int = _GAMMA_INT_PART(green); > + if (green_int > GAMMA_INT_MAX) > + green = BDW_MAX_GAMMA; > + > + red_int = _GAMMA_INT_PART(red); > + if (red_int > GAMMA_INT_MAX) > + red = BDW_MAX_GAMMA; > + > + blue_fract = _GAMMA_FRACT_PART(blue); > + green_fract = _GAMMA_FRACT_PART(green); > + red_fract = _GAMMA_FRACT_PART(red); > + > + blue_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; > + green_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; > + red_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; > + > + /* Red (29:20) Green (19:10) and Blue (9:0) */ > + word = red_fract; > + word <<= BDW_GAMMA_SHIFT; > + word = word | green_fract; > + word <<= BDW_GAMMA_SHIFT; > + word = word | blue_fract; > + > + return word; > +} > + I think the above function, and perhaps others in this series have the same flaw with respect to maximum colour value. In our discussions we agreed that we would follow the "GL style" where maximum colour (i.e. 255 in 8-bit) would be represented by 1.0f. 1.0f when converted to fixed point 8.24 notation is 1 << 24. I observed that with my test code that a linear ramp (where the last entry is set to 1.0) gives me black for white. I tracked it down to this function. In order to map 1.0 to the maximum value for the table entry in the hardware this function needs to be changed. One way to achieve this would be change the test "blue_int > GAMMA_INT_MAX" to be "blue_int >= GAMMA_INT_MAX" but since GAMMA_INT_MAX = 1 then it might be clearer to say blue_int > 0. BDW_MAX_GAMMA also looks wrong. I think it should be (1 << 24) - 1 not the 0x10000 (see below). As well as correct clamping it is also necessary to scale as Daniel suggested on IRC: " 13:54 < danvet> robster, so we'd need to rescale in the kernel from 1.0 to 0.1111111111b ? 13:55 < danvet> well substract 0.0000000001b for all values > 0.5 probably since float math in the kernel is evil 13:56 < danvet> also this probably needs an igt - check that all black with an all 1.0 gamma table is the same as all white with a linear one " You won't see this with your test program (color-correction.c) as the largest value you program in appears to be 0.ff *snip* > +/* Gen 9 */ > +#define BDW_MAX_GAMMA 0x10000 > *snip* I look forward to testing against your next version. Rob
Hey Rob, Thanks for the feedback, and the testing efforts. I will check into your suggestions and get back. Regards Shashank -----Original Message----- From: Bradford, Robert Sent: Wednesday, September 30, 2015 8:02 PM To: Sharma, Shashank; Roper, Matthew D; Bish, Jim; Smith, Gary K; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org Cc: Vetter, Daniel; Matheson, Annie J; Mukherjee, Indranil; Palleti, Avinash Reddy; kausalmalladi@gmail.com Subject: Re: [PATCH 21/23] drm/i915: BDW: Pipe level Gamma correction Hi Shashank, some feedback below that you would be great to get addressed before your next version. On Wed, 2015-09-16 at 23:07 +0530, Shashank Sharma wrote: > BDW/SKL/BXT platforms support various Gamma correction modes which > are: > 1. Legacy 8-bit mode > 2. 10-bit mode > 3. 10-bit Split Gamma mode > 4. 12-bit mode > > This patch does the following: > 1. Adds the core function to program Gamma correction values > for BDW/SKL/BXT platforms > 2. Adds Gamma 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 | 17 +- > drivers/gpu/drm/i915/intel_color_manager.c | 270 > +++++++++++++++++++++++++++++ *snip* > +static u32 bdw_write_10bit_gamma_precision(u32 red, u32 green, u32 > blue) > +{ > + u32 word; > + u8 blue_int, green_int, red_int; > + u16 blue_fract, green_fract, red_fract; > + > + blue_int = _GAMMA_INT_PART(blue); > + if (blue_int > GAMMA_INT_MAX) > + blue = BDW_MAX_GAMMA; > + > + green_int = _GAMMA_INT_PART(green); > + if (green_int > GAMMA_INT_MAX) > + green = BDW_MAX_GAMMA; > + > + red_int = _GAMMA_INT_PART(red); > + if (red_int > GAMMA_INT_MAX) > + red = BDW_MAX_GAMMA; > + > + blue_fract = _GAMMA_FRACT_PART(blue); > + green_fract = _GAMMA_FRACT_PART(green); > + red_fract = _GAMMA_FRACT_PART(red); > + > + blue_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; > + green_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; > + red_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; > + > + /* Red (29:20) Green (19:10) and Blue (9:0) */ > + word = red_fract; > + word <<= BDW_GAMMA_SHIFT; > + word = word | green_fract; > + word <<= BDW_GAMMA_SHIFT; > + word = word | blue_fract; > + > + return word; > +} > + I think the above function, and perhaps others in this series have the same flaw with respect to maximum colour value. In our discussions we agreed that we would follow the "GL style" where maximum colour (i.e. 255 in 8-bit) would be represented by 1.0f. 1.0f when converted to fixed point 8.24 notation is 1 << 24. I observed that with my test code that a linear ramp (where the last entry is set to 1.0) gives me black for white. I tracked it down to this function. In order to map 1.0 to the maximum value for the table entry in the hardware this function needs to be changed. One way to achieve this would be change the test "blue_int > GAMMA_INT_MAX" to be "blue_int >= GAMMA_INT_MAX" but since GAMMA_INT_MAX = 1 then it might be clearer to say blue_int > 0. BDW_MAX_GAMMA also looks wrong. I think it should be (1 << 24) - 1 not the 0x10000 (see below). As well as correct clamping it is also necessary to scale as Daniel suggested on IRC: " 13:54 < danvet> robster, so we'd need to rescale in the kernel from 1.0 to 0.1111111111b ? 13:55 < danvet> well substract 0.0000000001b for all values > 0.5 probably since float math in the kernel is evil 13:56 < danvet> also this probably needs an igt - check that all black with an all 1.0 gamma table is the same as all white with a linear one " You won't see this with your test program (color-correction.c) as the largest value you program in appears to be 0.ff *snip* > +/* Gen 9 */ > +#define BDW_MAX_GAMMA 0x10000 > *snip* I look forward to testing against your next version. Rob
Shashank, Please let us know when the next patch series would be ready assuming you incorporate the feedback from Rob. I would like to hope we're done with feedback at this point. Thanks. Annie Matheson > On Sep 30, 2015, at 9:25 AM, Sharma, Shashank <shashank.sharma@intel.com> wrote: > > Hey Rob, > > Thanks for the feedback, and the testing efforts. > I will check into your suggestions and get back. > > Regards > Shashank > -----Original Message----- > From: Bradford, Robert > Sent: Wednesday, September 30, 2015 8:02 PM > To: Sharma, Shashank; Roper, Matthew D; Bish, Jim; Smith, Gary K; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Cc: Vetter, Daniel; Matheson, Annie J; Mukherjee, Indranil; Palleti, Avinash Reddy; kausalmalladi@gmail.com > Subject: Re: [PATCH 21/23] drm/i915: BDW: Pipe level Gamma correction > > Hi Shashank, some feedback below that you would be great to get addressed before your next version. > >> On Wed, 2015-09-16 at 23:07 +0530, Shashank Sharma wrote: >> BDW/SKL/BXT platforms support various Gamma correction modes which >> are: >> 1. Legacy 8-bit mode >> 2. 10-bit mode >> 3. 10-bit Split Gamma mode >> 4. 12-bit mode >> >> This patch does the following: >> 1. Adds the core function to program Gamma correction values >> for BDW/SKL/BXT platforms >> 2. Adds Gamma 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 | 17 +- >> drivers/gpu/drm/i915/intel_color_manager.c | 270 >> +++++++++++++++++++++++++++++ > > *snip* > >> +static u32 bdw_write_10bit_gamma_precision(u32 red, u32 green, u32 >> blue) >> +{ >> + u32 word; >> + u8 blue_int, green_int, red_int; >> + u16 blue_fract, green_fract, red_fract; >> + >> + blue_int = _GAMMA_INT_PART(blue); >> + if (blue_int > GAMMA_INT_MAX) >> + blue = BDW_MAX_GAMMA; >> + >> + green_int = _GAMMA_INT_PART(green); >> + if (green_int > GAMMA_INT_MAX) >> + green = BDW_MAX_GAMMA; >> + >> + red_int = _GAMMA_INT_PART(red); >> + if (red_int > GAMMA_INT_MAX) >> + red = BDW_MAX_GAMMA; >> + >> + blue_fract = _GAMMA_FRACT_PART(blue); >> + green_fract = _GAMMA_FRACT_PART(green); >> + red_fract = _GAMMA_FRACT_PART(red); >> + >> + blue_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; >> + green_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; >> + red_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; >> + >> + /* Red (29:20) Green (19:10) and Blue (9:0) */ >> + word = red_fract; >> + word <<= BDW_GAMMA_SHIFT; >> + word = word | green_fract; >> + word <<= BDW_GAMMA_SHIFT; >> + word = word | blue_fract; >> + >> + return word; >> +} >> + > > I think the above function, and perhaps others in this series have the same flaw with respect to maximum colour value. > > In our discussions we agreed that we would follow the "GL style" where maximum colour (i.e. 255 in 8-bit) would be represented by 1.0f. 1.0f when converted to fixed point 8.24 notation is 1 << 24. > > I observed that with my test code that a linear ramp (where the last entry is set to 1.0) gives me black for white. I tracked it down to this function. > > In order to map 1.0 to the maximum value for the table entry in the hardware this function needs to be changed. One way to achieve this would be change the test "blue_int > GAMMA_INT_MAX" to be "blue_int >= GAMMA_INT_MAX" but since GAMMA_INT_MAX = 1 then it might be clearer to say blue_int > 0. > > BDW_MAX_GAMMA also looks wrong. I think it should be (1 << 24) - 1 not the 0x10000 (see below). As well as correct clamping it is also necessary to scale as Daniel suggested on IRC: > > " > 13:54 < danvet> robster, so we'd need to rescale in the kernel from 1.0 to 0.1111111111b ? > 13:55 < danvet> well substract 0.0000000001b for all values > 0.5 probably since float math in the kernel is evil > 13:56 < danvet> also this probably needs an igt - check that all black with an all 1.0 gamma table is the same as > all white with a linear one " > > You won't see this with your test program (color-correction.c) as the largest value you program in appears to be 0.ff > > *snip* > >> +/* Gen 9 */ >> +#define BDW_MAX_GAMMA 0x10000 > > *snip* > > I look forward to testing against your next version. > > Rob
On Wed, Sep 30, 2015 at 03:31:34PM +0100, Rob Bradford wrote: > Hi Shashank, some feedback below that you would be great to get > addressed before your next version. > > On Wed, 2015-09-16 at 23:07 +0530, Shashank Sharma wrote: > > BDW/SKL/BXT platforms support various Gamma correction modes > > which are: > > 1. Legacy 8-bit mode > > 2. 10-bit mode > > 3. 10-bit Split Gamma mode > > 4. 12-bit mode > > > > This patch does the following: > > 1. Adds the core function to program Gamma correction values > > for BDW/SKL/BXT platforms > > 2. Adds Gamma 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 | 17 +- > > drivers/gpu/drm/i915/intel_color_manager.c | 270 > > +++++++++++++++++++++++++++++ > > *snip* > > > +static u32 bdw_write_10bit_gamma_precision(u32 red, u32 green, u32 > > blue) > > +{ > > + u32 word; > > + u8 blue_int, green_int, red_int; > > + u16 blue_fract, green_fract, red_fract; > > + > > + blue_int = _GAMMA_INT_PART(blue); > > + if (blue_int > GAMMA_INT_MAX) > > + blue = BDW_MAX_GAMMA; > > + > > + green_int = _GAMMA_INT_PART(green); > > + if (green_int > GAMMA_INT_MAX) > > + green = BDW_MAX_GAMMA; > > + > > + red_int = _GAMMA_INT_PART(red); > > + if (red_int > GAMMA_INT_MAX) > > + red = BDW_MAX_GAMMA; > > + > > + blue_fract = _GAMMA_FRACT_PART(blue); > > + green_fract = _GAMMA_FRACT_PART(green); > > + red_fract = _GAMMA_FRACT_PART(red); > > + > > + blue_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; > > + green_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; > > + red_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; > > + > > + /* Red (29:20) Green (19:10) and Blue (9:0) */ > > + word = red_fract; > > + word <<= BDW_GAMMA_SHIFT; > > + word = word | green_fract; > > + word <<= BDW_GAMMA_SHIFT; > > + word = word | blue_fract; > > + > > + return word; > > +} > > + > > I think the above function, and perhaps others in this series have the > same flaw with respect to maximum colour value. > > In our discussions we agreed that we would follow the "GL style" where > maximum colour (i.e. 255 in 8-bit) would be represented by 1.0f. 1.0f > when converted to fixed point 8.24 notation is 1 << 24. > > I observed that with my test code that a linear ramp (where the last > entry is set to 1.0) gives me black for white. I tracked it down to > this function. > > In order to map 1.0 to the maximum value for the table entry in the > hardware this function needs to be changed. One way to achieve this > would be change the test "blue_int > GAMMA_INT_MAX" to be "blue_int >= > GAMMA_INT_MAX" but since GAMMA_INT_MAX = 1 then it might be clearer to > say blue_int > 0. > > BDW_MAX_GAMMA also looks wrong. I think it should be (1 << 24) - 1 not > the 0x10000 (see below). As well as correct clamping it is also > necessary to scale as Daniel suggested on IRC: > > " > 13:54 < danvet> robster, so we'd need to rescale in the kernel from 1.0 > to 0.1111111111b ? > 13:55 < danvet> well substract 0.0000000001b for all values > 0.5 > probably since float math in the kernel is evil Or just clamp to '(1<<bits)-1'. I think the end result is the same actually.
Hi Annie, This might add a day or two, but considering this is a long weekend in India, you can expect the patches by mid next week. Regards Shashank -----Original Message----- From: Matheson, Annie J Sent: Wednesday, September 30, 2015 10:01 PM To: Sharma, Shashank Cc: Bradford, Robert; Roper, Matthew D; Bish, Jim; Smith, Gary K; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; Vetter, Daniel; Mukherjee, Indranil; Palleti, Avinash Reddy; kausalmalladi@gmail.com Subject: Re: [PATCH 21/23] drm/i915: BDW: Pipe level Gamma correction Shashank, Please let us know when the next patch series would be ready assuming you incorporate the feedback from Rob. I would like to hope we're done with feedback at this point. Thanks. Annie Matheson > On Sep 30, 2015, at 9:25 AM, Sharma, Shashank <shashank.sharma@intel.com> wrote: > > Hey Rob, > > Thanks for the feedback, and the testing efforts. > I will check into your suggestions and get back. > > Regards > Shashank > -----Original Message----- > From: Bradford, Robert > Sent: Wednesday, September 30, 2015 8:02 PM > To: Sharma, Shashank; Roper, Matthew D; Bish, Jim; Smith, Gary K; dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org > Cc: Vetter, Daniel; Matheson, Annie J; Mukherjee, Indranil; Palleti, Avinash Reddy; kausalmalladi@gmail.com > Subject: Re: [PATCH 21/23] drm/i915: BDW: Pipe level Gamma correction > > Hi Shashank, some feedback below that you would be great to get addressed before your next version. > >> On Wed, 2015-09-16 at 23:07 +0530, Shashank Sharma wrote: >> BDW/SKL/BXT platforms support various Gamma correction modes which >> are: >> 1. Legacy 8-bit mode >> 2. 10-bit mode >> 3. 10-bit Split Gamma mode >> 4. 12-bit mode >> >> This patch does the following: >> 1. Adds the core function to program Gamma correction values >> for BDW/SKL/BXT platforms >> 2. Adds Gamma 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 | 17 +- >> drivers/gpu/drm/i915/intel_color_manager.c | 270 >> +++++++++++++++++++++++++++++ > > *snip* > >> +static u32 bdw_write_10bit_gamma_precision(u32 red, u32 green, u32 >> blue) >> +{ >> + u32 word; >> + u8 blue_int, green_int, red_int; >> + u16 blue_fract, green_fract, red_fract; >> + >> + blue_int = _GAMMA_INT_PART(blue); >> + if (blue_int > GAMMA_INT_MAX) >> + blue = BDW_MAX_GAMMA; >> + >> + green_int = _GAMMA_INT_PART(green); >> + if (green_int > GAMMA_INT_MAX) >> + green = BDW_MAX_GAMMA; >> + >> + red_int = _GAMMA_INT_PART(red); >> + if (red_int > GAMMA_INT_MAX) >> + red = BDW_MAX_GAMMA; >> + >> + blue_fract = _GAMMA_FRACT_PART(blue); >> + green_fract = _GAMMA_FRACT_PART(green); >> + red_fract = _GAMMA_FRACT_PART(red); >> + >> + blue_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; >> + green_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; >> + red_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; >> + >> + /* Red (29:20) Green (19:10) and Blue (9:0) */ >> + word = red_fract; >> + word <<= BDW_GAMMA_SHIFT; >> + word = word | green_fract; >> + word <<= BDW_GAMMA_SHIFT; >> + word = word | blue_fract; >> + >> + return word; >> +} >> + > > I think the above function, and perhaps others in this series have the same flaw with respect to maximum colour value. > > In our discussions we agreed that we would follow the "GL style" where maximum colour (i.e. 255 in 8-bit) would be represented by 1.0f. 1.0f when converted to fixed point 8.24 notation is 1 << 24. > > I observed that with my test code that a linear ramp (where the last entry is set to 1.0) gives me black for white. I tracked it down to this function. > > In order to map 1.0 to the maximum value for the table entry in the hardware this function needs to be changed. One way to achieve this would be change the test "blue_int > GAMMA_INT_MAX" to be "blue_int >= GAMMA_INT_MAX" but since GAMMA_INT_MAX = 1 then it might be clearer to say blue_int > 0. > > BDW_MAX_GAMMA also looks wrong. I think it should be (1 << 24) - 1 not the 0x10000 (see below). As well as correct clamping it is also necessary to scale as Daniel suggested on IRC: > > " > 13:54 < danvet> robster, so we'd need to rescale in the kernel from 1.0 to 0.1111111111b ? > 13:55 < danvet> well substract 0.0000000001b for all values > 0.5 probably since float math in the kernel is evil > 13:56 < danvet> also this probably needs an igt - check that all black with an all 1.0 gamma table is the same as > all white with a linear one " > > You won't see this with your test program (color-correction.c) as the largest value you program in appears to be 0.ff > > *snip* > >> +/* Gen 9 */ >> +#define BDW_MAX_GAMMA 0x10000 > > *snip* > > I look forward to testing against your next version. > > Rob
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5c7759d..88f4e41 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -5634,7 +5634,9 @@ enum skl_disp_power_wells { #define _GAMMA_MODE_A 0x4a480 #define _GAMMA_MODE_B 0x4ac80 -#define GAMMA_MODE(pipe) _PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B) +#define _GAMMA_MODE_C 0x4b480 +#define GAMMA_MODE(pipe) \ + _PIPE3(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B, _GAMMA_MODE_C) #define GAMMA_MODE_MODE_MASK (3 << 0) #define GAMMA_MODE_MODE_8BIT (0 << 0) #define GAMMA_MODE_MODE_10BIT (1 << 0) @@ -8001,6 +8003,17 @@ enum skl_disp_power_wells { #define _PIPE_CSC_BASE(pipe) \ (_PIPE3(pipe, PIPEA_CGM_CSC, PIPEB_CGM_CSC, PIPEC_CGM_CSC)) - +/* BDW gamma correction */ +#define PAL_PREC_INDEX_A 0x4A400 +#define PAL_PREC_INDEX_B 0x4AC00 +#define PAL_PREC_INDEX_C 0x4B400 +#define PAL_PREC_DATA_A 0x4A404 +#define PAL_PREC_DATA_B 0x4AC04 +#define PAL_PREC_DATA_C 0x4B404 + +#define _PREC_PAL_INDEX(pipe) \ + (_PIPE3(pipe, PAL_PREC_INDEX_A, PAL_PREC_INDEX_B, PAL_PREC_INDEX_C)) +#define _PREC_PAL_DATA(pipe) \ + (_PIPE3(pipe, PAL_PREC_DATA_A, PAL_PREC_DATA_B, PAL_PREC_DATA_C)) #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 d33a2be..d935fd8 100644 --- a/drivers/gpu/drm/i915/intel_color_manager.c +++ b/drivers/gpu/drm/i915/intel_color_manager.c @@ -264,6 +264,274 @@ static int chv_set_degamma(struct drm_device *dev, return ret; } +static u32 bdw_write_10bit_gamma_precision(u32 red, u32 green, u32 blue) +{ + u32 word; + u8 blue_int, green_int, red_int; + u16 blue_fract, green_fract, red_fract; + + blue_int = _GAMMA_INT_PART(blue); + if (blue_int > GAMMA_INT_MAX) + blue = BDW_MAX_GAMMA; + + green_int = _GAMMA_INT_PART(green); + if (green_int > GAMMA_INT_MAX) + green = BDW_MAX_GAMMA; + + red_int = _GAMMA_INT_PART(red); + if (red_int > GAMMA_INT_MAX) + red = BDW_MAX_GAMMA; + + blue_fract = _GAMMA_FRACT_PART(blue); + green_fract = _GAMMA_FRACT_PART(green); + red_fract = _GAMMA_FRACT_PART(red); + + blue_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; + green_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; + red_fract >>= BDW_10BIT_GAMMA_MSB_SHIFT; + + /* Red (29:20) Green (19:10) and Blue (9:0) */ + word = red_fract; + word <<= BDW_GAMMA_SHIFT; + word = word | green_fract; + word <<= BDW_GAMMA_SHIFT; + word = word | blue_fract; + + return word; +} + +/* Apply unity gamma for gamma reset */ +static void bdw_reset_gamma(struct drm_i915_private *dev_priv, + enum pipe pipe) +{ + u16 count = 0; + u32 val; + u32 pal_prec_data = LGC_PALETTE(pipe); + + DRM_DEBUG_DRIVER("\n"); + + /* Reset the palette for unit gamma */ + while (count < BDW_8BIT_GAMMA_MAX_VALS) { + /* Red (23:16) Green (15:8) and Blue (7:0) */ + val = (count << 16) | (count << 8) | count; + I915_WRITE(pal_prec_data, val); + pal_prec_data += 4; + count++; + } +} + +static int bdw_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, + struct drm_crtc *crtc) +{ + u8 blue_int, green_int, red_int; + u16 blue_fract, green_fract, red_fract; + u16 blue_odd, green_odd, red_odd; + u16 blue_even, green_even, red_even; + + enum pipe pipe; + int count, num_samples; + u32 blue, green, red; + u32 mode, pal_prec_index, pal_prec_data; + u32 index, word; + struct drm_palette *gamma_data; + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_r32g32b32 *correction_values = NULL; + + if (!blob) { + DRM_ERROR("Null Blob\n"); + return -EINVAL; + } + + gamma_data = (struct drm_palette *)blob->data; + + if (gamma_data->version != GAMMA_DATA_STRUCT_VERSION) { + DRM_DEBUG_KMS("Invalid Gamma Data struct version\n"); + return -EINVAL; + } + + pipe = to_intel_crtc(crtc)->pipe; + num_samples = gamma_data->num_samples; + + pal_prec_index = _PREC_PAL_INDEX(pipe); + pal_prec_data = _PREC_PAL_DATA(pipe); + + correction_values = (struct drm_r32g32b32 *)&gamma_data->lut; + index = I915_READ(pal_prec_index); + switch (num_samples) { + + case 0: + + /* Disable Gamma functionality on Pipe */ + DRM_DEBUG_DRIVER("Disabling gamma on Pipe %c\n", + pipe_name(pipe)); + bdw_reset_gamma(dev_priv, pipe); + word = GAMMA_MODE_MODE_8BIT; + break; + + case BDW_8BIT_GAMMA_MAX_VALS: + + /* Legacy palette */ + pal_prec_data = LGC_PALETTE(pipe); + + count = 0; + while (count < BDW_8BIT_GAMMA_MAX_VALS) { + blue = correction_values[count].b32; + green = correction_values[count].g32; + red = correction_values[count].r32; + + blue_int = _GAMMA_INT_PART(blue); + if (blue_int > GAMMA_INT_MAX) + blue = BDW_MAX_GAMMA; + green_int = _GAMMA_INT_PART(green); + if (green_int > GAMMA_INT_MAX) + green = BDW_MAX_GAMMA; + red_int = _GAMMA_INT_PART(red); + if (red_int > GAMMA_INT_MAX) + red = BDW_MAX_GAMMA; + + blue_fract = _GAMMA_FRACT_PART(blue); + green_fract = _GAMMA_FRACT_PART(green); + red_fract = _GAMMA_FRACT_PART(red); + + blue_fract >>= BDW_8BIT_GAMMA_MSB_SHIFT; + green_fract >>= BDW_8BIT_GAMMA_MSB_SHIFT; + red_fract >>= BDW_8BIT_GAMMA_MSB_SHIFT; + + /* Red (23:16) Green (15:8) and Blue (7:0) */ + word = red_fract; + word <<= BDW_8BIT_GAMMA_SHIFT; + word = word | green_fract; + word <<= BDW_8BIT_GAMMA_SHIFT; + word = word | blue_fract; + + I915_WRITE(pal_prec_data, word); + pal_prec_data += 4; + + count++; + } + + word = GAMMA_MODE_MODE_8BIT; + break; + + case BDW_SPLITGAMMA_MAX_VALS: + + index |= BDW_INDEX_AUTO_INCREMENT | BDW_INDEX_SPLIT_MODE; + I915_WRITE(pal_prec_index, index); + + count = 0; + while (count < BDW_SPLITGAMMA_MAX_VALS) { + blue = correction_values[count].b32; + green = correction_values[count].g32; + red = correction_values[count].r32; + + word = bdw_write_10bit_gamma_precision(red, + green, blue); + I915_WRITE(pal_prec_data, word); + count++; + } + + word = GAMMA_MODE_MODE_SPLIT; + break; + + case BDW_12BIT_GAMMA_MAX_VALS: + + index |= BDW_INDEX_AUTO_INCREMENT; + index &= ~BDW_INDEX_SPLIT_MODE; + I915_WRITE(pal_prec_index, index); + + count = 0; + while (count < BDW_12BIT_GAMMA_MAX_VALS) { + blue = correction_values[count].b32; + green = correction_values[count].g32; + red = correction_values[count].r32; + + blue_int = _GAMMA_INT_PART(blue); + if (blue_int > GAMMA_INT_MAX) + blue = BDW_MAX_GAMMA; + green_int = _GAMMA_INT_PART(green); + if (green_int > GAMMA_INT_MAX) + green = BDW_MAX_GAMMA; + red_int = _GAMMA_INT_PART(red); + if (red_int > GAMMA_INT_MAX) + red = BDW_MAX_GAMMA; + + blue_fract = _GAMMA_FRACT_PART(blue); + green_fract = _GAMMA_FRACT_PART(green); + red_fract = _GAMMA_FRACT_PART(red); + + /* Odd index */ + if (count % 2 == 0) { + blue_odd = blue_fract >> + BDW_12BIT_GAMMA_ODD_SHIFT; + green_odd = green_fract >> + BDW_12BIT_GAMMA_ODD_SHIFT; + red_odd = red_fract >> + BDW_12BIT_GAMMA_ODD_SHIFT; + + word = red_odd; + word = word << BDW_GAMMA_SHIFT; + word = word | green_odd; + word = word << BDW_GAMMA_SHIFT; + word = word | blue_odd; + } else { + blue_even = blue_fract << + BDW_12BIT_GAMMA_EVEN_SHIFT1 >> + BDW_12BIT_GAMMA_EVEN_SHIFT2; + green_even = green_fract << + BDW_12BIT_GAMMA_EVEN_SHIFT1 >> + BDW_12BIT_GAMMA_EVEN_SHIFT2; + red_even = red_fract << + BDW_12BIT_GAMMA_EVEN_SHIFT1 >> + BDW_12BIT_GAMMA_EVEN_SHIFT2; + + word = red_even; + word = word << BDW_GAMMA_SHIFT; + word = word | green_even; + word = word << BDW_GAMMA_SHIFT; + word = word | blue_even; + } + I915_WRITE(pal_prec_data, word); + count++; + } + + word = GAMMA_MODE_MODE_12BIT; + break; + + case BDW_10BIT_GAMMA_MAX_VALS: + + index |= BDW_INDEX_AUTO_INCREMENT; + index &= ~BDW_INDEX_SPLIT_MODE; + I915_WRITE(pal_prec_index, index); + + count = 0; + while (count < BDW_10BIT_GAMMA_MAX_VALS) { + blue = correction_values[count].b32; + green = correction_values[count].g32; + red = correction_values[count].r32; + + word = bdw_write_10bit_gamma_precision(red, + green, blue); + I915_WRITE(pal_prec_data, word); + count++; + } + + word = GAMMA_MODE_MODE_10BIT; + break; + + default: + DRM_ERROR("Invalid number of samples\n"); + return -EINVAL; + } + + /* Set gamma mode on pipe control reg */ + mode = I915_READ(GAMMA_MODE(pipe)); + mode &= ~GAMMA_MODE_MODE_MASK; + I915_WRITE(GAMMA_MODE(pipe), mode | word); + DRM_DEBUG_DRIVER("Gamma applied on pipe %c\n", + pipe_name(pipe)); + return 0; +} + static int chv_set_gamma(struct drm_device *dev, struct drm_property_blob *blob, struct drm_crtc *crtc) { @@ -418,6 +686,8 @@ void intel_color_manager_crtc_commit(struct drm_device *dev, /* Gamma correction is platform specific */ if (IS_CHERRYVIEW(dev)) ret = chv_set_gamma(dev, blob, crtc); + else if (IS_BROADWELL(dev) || IS_GEN9(dev)) + ret = bdw_set_gamma(dev, blob, crtc); if (ret) DRM_ERROR("set Gamma correction failed\n"); diff --git a/drivers/gpu/drm/i915/intel_color_manager.h b/drivers/gpu/drm/i915/intel_color_manager.h index 3687989..17fcf3d 100644 --- a/drivers/gpu/drm/i915/intel_color_manager.h +++ b/drivers/gpu/drm/i915/intel_color_manager.h @@ -43,6 +43,9 @@ #define CHV_GAMMA_SHIFT_GREEN 16 #define BDW_SPLITGAMMA_MAX_VALS 512 +#define BDW_8BIT_GAMMA_MAX_VALS 256 +#define BDW_10BIT_GAMMA_MAX_VALS 1024 +#define BDW_12BIT_GAMMA_MAX_VALS 513 /* Gamma values are u8.24 format */ #define GAMMA_INT_SHIFT 24 @@ -52,6 +55,18 @@ /* Max value for Gamma on CHV */ #define CHV_MAX_GAMMA 0x10000 +/* Gen 9 */ +#define BDW_MAX_GAMMA 0x10000 +#define BDW_10BIT_GAMMA_MSB_SHIFT 6 +#define BDW_GAMMA_SHIFT 10 +#define BDW_INDEX_AUTO_INCREMENT (1 << 15) +#define BDW_INDEX_SPLIT_MODE (1 << 31) +#define BDW_8BIT_GAMMA_MSB_SHIFT 8 +#define BDW_8BIT_GAMMA_SHIFT 8 +#define BDW_12BIT_GAMMA_ODD_SHIFT 6 +#define BDW_12BIT_GAMMA_EVEN_SHIFT1 10 +#define BDW_12BIT_GAMMA_EVEN_SHIFT2 6 + /* DeGamma correction */ #define DEGAMMA_DATA_STRUCT_VERSION 1 #define CHV_DEGAMMA_MSB_SHIFT 2