[PATCH] drm/i915: Fix Limited Range Color Handling

[PATCH] drm/i915: Fix Limited Range Color Handling

[Uma Shankar]

From: Johnson Lin <johnson.lin@intel.com>

Some panels support limited range output (16-235) compared
to full range RGB values (0-255). Also userspace can control
the RGB range using "Broadcast RGB" property. Currently the
code to handle full range to limited range is broken. This
patch fixes the same by properly scaling down all the full
range co-efficients with limited range scaling factor.

Signed-off-by: Johnson Lin <johnson.lin@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index aa66e95..777ce26 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
 	for (i = 0; i < 9; i++)
 		result[i] = 0;
 
-	for (i = 0; i < 3; i++) {
-		int64_t user_coeff = input[i * 3 + i];
+	for (i = 0; i < 9; i++) {
+		int64_t user_coeff = input[i];
 		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
 		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
 					       0,
 					       CTM_COEFF_4_0 - 1) >> 2;
 
-		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
+		/*
+		 * By scaling every co-efficient with limited range (235-16)
+		 * vs full range (0-255) the final o/p will be scaled down to
+		 * fit in the limited range supported by the panel.
+		 * Need to shift the multiplication result by 28 as the floating
+		 * count expected is of 32bits, multiplication here is done in
+		 * U2.30 so result need to be right shifted by 60-32 = 28
+		 */
+		result[i] = (limited_coeff * abs_coeff) >> 28;
 		if (CTM_COEFF_NEGATIVE(user_coeff))
-			result[i * 3 + i] |= CTM_COEFF_SIGN;
+			result[i] |= CTM_COEFF_SIGN;
 	}
 }
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Fix Limited Range Color Handling

[Patchwork]

== Series Details ==

Series: drm/i915: Fix Limited Range Color Handling
URL   : https://patchwork.freedesktop.org/series/35725/
State : success

== Summary ==

Series 35725v1 drm/i915: Fix Limited Range Color Handling
https://patchwork.freedesktop.org/api/1.0/series/35725/revisions/1/mbox/

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#104172 +1

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:435s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:439s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:381s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:493s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:276s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:489s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:492s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:476s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:462s
fi-elk-e7500     total:224  pass:163  dwarn:15  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:262s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:411s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:414s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:427s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:478s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:431s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:475s
fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:522s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:470s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:528s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:590s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:447s
fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:529s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:555s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:500s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:497s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:445s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:549s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:411s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:592s
fi-glk-dsi       total:288  pass:176  dwarn:1   dfail:4   fail:0   skip:107 time:329s

34783c4253df6206c731735524b0297c6894384b drm-tip: 2017y-12m-22d-12h-46m-57s UTC integration manifest
48e20b9cbe1e drm/i915: Fix Limited Range Color Handling

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7568/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Ville Syrjälä]

On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
> From: Johnson Lin <johnson.lin@intel.com>
> 
> Some panels support limited range output (16-235) compared
> to full range RGB values (0-255). Also userspace can control
> the RGB range using "Broadcast RGB" property. Currently the
> code to handle full range to limited range is broken. This
> patch fixes the same by properly scaling down all the full
> range co-efficients with limited range scaling factor.
> 
> Signed-off-by: Johnson Lin <johnson.lin@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index aa66e95..777ce26 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
>  	for (i = 0; i < 9; i++)
>  		result[i] = 0;

This can go.

>  
> -	for (i = 0; i < 3; i++) {
> -		int64_t user_coeff = input[i * 3 + i];
> +	for (i = 0; i < 9; i++) {
> +		int64_t user_coeff = input[i];

That's not a two's complement number so should probably be u64.

>  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
>  		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
>  					       0,
>  					       CTM_COEFF_4_0 - 1) >> 2;

Seems to me that we should be able to drop the >>2 from the limited_coeff
since it's always < 1.0, make both of these u32 and then use
'mul_u32_u32() >> 30' to get the answer.

>  
> -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
> +		/*
> +		 * By scaling every co-efficient with limited range (235-16)
> +		 * vs full range (0-255) the final o/p will be scaled down to
> +		 * fit in the limited range supported by the panel.
> +		 * Need to shift the multiplication result by 28 as the floating
> +		 * count expected is of 32bits, multiplication here is done in
> +		 * U2.30 so result need to be right shifted by 60-32 = 28
> +		 */
> +		result[i] = (limited_coeff * abs_coeff) >> 28;
>  		if (CTM_COEFF_NEGATIVE(user_coeff))

And this can be replaced with just

result[i] |= user_coeff & CTM_COEFF_SIGN;

to eliminate the silly branch.


> -			result[i * 3 + i] |= CTM_COEFF_SIGN;
> +			result[i] |= CTM_COEFF_SIGN;
>  	}
>  }
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Ville Syrjälä]

On Fri, Dec 22, 2017 at 05:54:31PM +0200, Ville Syrjälä wrote:
> On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
> > From: Johnson Lin <johnson.lin@intel.com>
> > 
> > Some panels support limited range output (16-235) compared
> > to full range RGB values (0-255). Also userspace can control
> > the RGB range using "Broadcast RGB" property. Currently the
> > code to handle full range to limited range is broken. This
> > patch fixes the same by properly scaling down all the full
> > range co-efficients with limited range scaling factor.
> > 
> > Signed-off-by: Johnson Lin <johnson.lin@intel.com>
> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > index aa66e95..777ce26 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
> >  	for (i = 0; i < 9; i++)
> >  		result[i] = 0;
> 
> This can go.
> 
> >  
> > -	for (i = 0; i < 3; i++) {
> > -		int64_t user_coeff = input[i * 3 + i];
> > +	for (i = 0; i < 9; i++) {
> > +		int64_t user_coeff = input[i];
> 
> That's not a two's complement number so should probably be u64.
> 
> >  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
> >  		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
> >  					       0,
> >  					       CTM_COEFF_4_0 - 1) >> 2;
> 
> Seems to me that we should be able to drop the >>2 from the limited_coeff
> since it's always < 1.0, make both of these u32 and then use
> 'mul_u32_u32() >> 30' to get the answer.
> 
> >  
> > -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
> > +		/*
> > +		 * By scaling every co-efficient with limited range (235-16)
> > +		 * vs full range (0-255) the final o/p will be scaled down to
> > +		 * fit in the limited range supported by the panel.
> > +		 * Need to shift the multiplication result by 28 as the floating
> > +		 * count expected is of 32bits, multiplication here is done in
> > +		 * U2.30 so result need to be right shifted by 60-32 = 28
> > +		 */
> > +		result[i] = (limited_coeff * abs_coeff) >> 28;
> >  		if (CTM_COEFF_NEGATIVE(user_coeff))
> 
> And this can be replaced with just
> 
> result[i] |= user_coeff & CTM_COEFF_SIGN;
> 
> to eliminate the silly branch.

I wonder if we could also get some igts for this limited range stuff.
Maybe something like:
1. grab crc from black/white screen with limited range
2. grab cdc from almost black/white screen with full range
3. compare the two

Repeat with an identity csc matrix set.

Not sure if we can get the crcs to match in these two cases though.
Hard to say without trying it out.

> 
> 
> > -			result[i * 3 + i] |= CTM_COEFF_SIGN;
> > +			result[i] |= CTM_COEFF_SIGN;
> >  	}
> >  }
> >  
> > -- 
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Friday, December 22, 2017 9:43 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson <johnson.lin@intel.com>;
>Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color Handling
>
>On Fri, Dec 22, 2017 at 05:54:31PM +0200, Ville Syrjälä wrote:
>> On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
>> > From: Johnson Lin <johnson.lin@intel.com>
>> >
>> > Some panels support limited range output (16-235) compared to full
>> > range RGB values (0-255). Also userspace can control the RGB range
>> > using "Broadcast RGB" property. Currently the code to handle full
>> > range to limited range is broken. This patch fixes the same by
>> > properly scaling down all the full range co-efficients with limited
>> > range scaling factor.
>> >
>> > Signed-off-by: Johnson Lin <johnson.lin@intel.com>
>> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
>> >  1 file changed, 12 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/intel_color.c
>> > b/drivers/gpu/drm/i915/intel_color.c
>> > index aa66e95..777ce26 100644
>> > --- a/drivers/gpu/drm/i915/intel_color.c
>> > +++ b/drivers/gpu/drm/i915/intel_color.c
>> > @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t *result,
>int64_t *input)
>> >  	for (i = 0; i < 9; i++)
>> >  		result[i] = 0;
>>
>> This can go.
>>
>> >
>> > -	for (i = 0; i < 3; i++) {
>> > -		int64_t user_coeff = input[i * 3 + i];
>> > +	for (i = 0; i < 9; i++) {
>> > +		int64_t user_coeff = input[i];
>>
>> That's not a two's complement number so should probably be u64.
>>
>> >  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
>> >  		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
>> >  					       0,
>> >  					       CTM_COEFF_4_0 - 1) >> 2;
>>
>> Seems to me that we should be able to drop the >>2 from the
>> limited_coeff since it's always < 1.0, make both of these u32 and then
>> use
>> 'mul_u32_u32() >> 30' to get the answer.
>>
>> >
>> > -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
>> > +		/*
>> > +		 * By scaling every co-efficient with limited range (235-16)
>> > +		 * vs full range (0-255) the final o/p will be scaled down to
>> > +		 * fit in the limited range supported by the panel.
>> > +		 * Need to shift the multiplication result by 28 as the floating
>> > +		 * count expected is of 32bits, multiplication here is done in
>> > +		 * U2.30 so result need to be right shifted by 60-32 = 28
>> > +		 */
>> > +		result[i] = (limited_coeff * abs_coeff) >> 28;
>> >  		if (CTM_COEFF_NEGATIVE(user_coeff))
>>
>> And this can be replaced with just
>>
>> result[i] |= user_coeff & CTM_COEFF_SIGN;
>>
>> to eliminate the silly branch.
>
>I wonder if we could also get some igts for this limited range stuff.
>Maybe something like:
>1. grab crc from black/white screen with limited range 2. grab cdc from almost
>black/white screen with full range 3. compare the two
>
>Repeat with an identity csc matrix set.
>
>Not sure if we can get the crcs to match in these two cases though.
>Hard to say without trying it out.
>

Hi Ville,
We will try this approach and update. Ideally crc's should match, will confirm the
behavior on hardware.

Thanks & Regards,
Uma Shankar

>>
>>
>> > -			result[i * 3 + i] |= CTM_COEFF_SIGN;
>> > +			result[i] |= CTM_COEFF_SIGN;
>> >  	}
>> >  }
>> >
>> > --
>> > 1.7.9.5
>> >
>> > _______________________________________________
>> > Intel-gfx mailing list
>> > Intel-gfx@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>
>> --
>> Ville Syrjälä
>> Intel OTC
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Ville Syrjälä]

On Tue, Jan 30, 2018 at 03:23:32PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >Sent: Friday, December 22, 2017 9:43 PM
> >To: Shankar, Uma <uma.shankar@intel.com>
> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson <johnson.lin@intel.com>;
> >Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
> ><maarten.lankhorst@intel.com>
> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color Handling
> >
> >On Fri, Dec 22, 2017 at 05:54:31PM +0200, Ville Syrjälä wrote:
> >> On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
> >> > From: Johnson Lin <johnson.lin@intel.com>
> >> >
> >> > Some panels support limited range output (16-235) compared to full
> >> > range RGB values (0-255). Also userspace can control the RGB range
> >> > using "Broadcast RGB" property. Currently the code to handle full
> >> > range to limited range is broken. This patch fixes the same by
> >> > properly scaling down all the full range co-efficients with limited
> >> > range scaling factor.
> >> >
> >> > Signed-off-by: Johnson Lin <johnson.lin@intel.com>
> >> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
> >> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/intel_color.c
> >> > b/drivers/gpu/drm/i915/intel_color.c
> >> > index aa66e95..777ce26 100644
> >> > --- a/drivers/gpu/drm/i915/intel_color.c
> >> > +++ b/drivers/gpu/drm/i915/intel_color.c
> >> > @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t *result,
> >int64_t *input)
> >> >  	for (i = 0; i < 9; i++)
> >> >  		result[i] = 0;
> >>
> >> This can go.
> >>
> >> >
> >> > -	for (i = 0; i < 3; i++) {
> >> > -		int64_t user_coeff = input[i * 3 + i];
> >> > +	for (i = 0; i < 9; i++) {
> >> > +		int64_t user_coeff = input[i];
> >>
> >> That's not a two's complement number so should probably be u64.
> >>
> >> >  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
> >> >  		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
> >> >  					       0,
> >> >  					       CTM_COEFF_4_0 - 1) >> 2;
> >>
> >> Seems to me that we should be able to drop the >>2 from the
> >> limited_coeff since it's always < 1.0, make both of these u32 and then
> >> use
> >> 'mul_u32_u32() >> 30' to get the answer.
> >>
> >> >
> >> > -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
> >> > +		/*
> >> > +		 * By scaling every co-efficient with limited range (235-16)
> >> > +		 * vs full range (0-255) the final o/p will be scaled down to
> >> > +		 * fit in the limited range supported by the panel.
> >> > +		 * Need to shift the multiplication result by 28 as the floating
> >> > +		 * count expected is of 32bits, multiplication here is done in
> >> > +		 * U2.30 so result need to be right shifted by 60-32 = 28
> >> > +		 */
> >> > +		result[i] = (limited_coeff * abs_coeff) >> 28;
> >> >  		if (CTM_COEFF_NEGATIVE(user_coeff))
> >>
> >> And this can be replaced with just
> >>
> >> result[i] |= user_coeff & CTM_COEFF_SIGN;
> >>
> >> to eliminate the silly branch.
> >
> >I wonder if we could also get some igts for this limited range stuff.
> >Maybe something like:
> >1. grab crc from black/white screen with limited range 2. grab cdc from almost
> >black/white screen with full range 3. compare the two
> >
> >Repeat with an identity csc matrix set.
> >
> >Not sure if we can get the crcs to match in these two cases though.
> >Hard to say without trying it out.
> >
> 
> Hi Ville,
> We will try this approach and update. Ideally crc's should match, will confirm the
> behavior on hardware.

I suspect it's not going to work on some older platforms because we use the
magic bit in the pipe/port register to do the range compression. The
effect of that bit may not be visible in the pipe crc. But starting from
hsw it should be ok.

> >>
> >>
> >> > -			result[i * 3 + i] |= CTM_COEFF_SIGN;
> >> > +			result[i] |= CTM_COEFF_SIGN;
> >> >  	}
> >> >  }
> >> >
> >> > --
> >> > 1.7.9.5
> >> >
> >> > _______________________________________________
> >> > Intel-gfx mailing list
> >> > Intel-gfx@lists.freedesktop.org
> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >>
> >> --
> >> Ville Syrjälä
> >> Intel OTC
> >
> >--
> >Ville Syrjälä
> >Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Wednesday, January 31, 2018 1:44 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson <johnson.lin@intel.com>;
>Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color Handling
>
>On Tue, Jan 30, 2018 at 03:23:32PM +0000, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>> >Sent: Friday, December 22, 2017 9:43 PM
>> >To: Shankar, Uma <uma.shankar@intel.com>
>> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson
>> ><johnson.lin@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
>> >Lankhorst, Maarten <maarten.lankhorst@intel.com>
>> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color
>> >Handling
>> >
>> >On Fri, Dec 22, 2017 at 05:54:31PM +0200, Ville Syrjälä wrote:
>> >> On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
>> >> > From: Johnson Lin <johnson.lin@intel.com>
>> >> >
>> >> > Some panels support limited range output (16-235) compared to
>> >> > full range RGB values (0-255). Also userspace can control the RGB
>> >> > range using "Broadcast RGB" property. Currently the code to
>> >> > handle full range to limited range is broken. This patch fixes
>> >> > the same by properly scaling down all the full range
>> >> > co-efficients with limited range scaling factor.
>> >> >
>> >> > Signed-off-by: Johnson Lin <johnson.lin@intel.com>
>> >> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> >> > ---
>> >> >  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
>> >> >  1 file changed, 12 insertions(+), 4 deletions(-)
>> >> >
>> >> > diff --git a/drivers/gpu/drm/i915/intel_color.c
>> >> > b/drivers/gpu/drm/i915/intel_color.c
>> >> > index aa66e95..777ce26 100644
>> >> > --- a/drivers/gpu/drm/i915/intel_color.c
>> >> > +++ b/drivers/gpu/drm/i915/intel_color.c
>> >> > @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t
>> >> > *result,
>> >int64_t *input)
>> >> >  	for (i = 0; i < 9; i++)
>> >> >  		result[i] = 0;
>> >>
>> >> This can go.
>> >>
>> >> >
>> >> > -	for (i = 0; i < 3; i++) {
>> >> > -		int64_t user_coeff = input[i * 3 + i];
>> >> > +	for (i = 0; i < 9; i++) {
>> >> > +		int64_t user_coeff = input[i];
>> >>
>> >> That's not a two's complement number so should probably be u64.
>> >>
>> >> >  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
>> >> >  		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
>> >> >  					       0,
>> >> >  					       CTM_COEFF_4_0 - 1) >> 2;
>> >>
>> >> Seems to me that we should be able to drop the >>2 from the
>> >> limited_coeff since it's always < 1.0, make both of these u32 and
>> >> then use
>> >> 'mul_u32_u32() >> 30' to get the answer.
>> >>
>> >> >
>> >> > -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
>> >> > +		/*
>> >> > +		 * By scaling every co-efficient with limited range (235-16)
>> >> > +		 * vs full range (0-255) the final o/p will be scaled down to
>> >> > +		 * fit in the limited range supported by the panel.
>> >> > +		 * Need to shift the multiplication result by 28 as the floating
>> >> > +		 * count expected is of 32bits, multiplication here is done in
>> >> > +		 * U2.30 so result need to be right shifted by 60-32 = 28
>> >> > +		 */
>> >> > +		result[i] = (limited_coeff * abs_coeff) >> 28;
>> >> >  		if (CTM_COEFF_NEGATIVE(user_coeff))
>> >>
>> >> And this can be replaced with just
>> >>
>> >> result[i] |= user_coeff & CTM_COEFF_SIGN;
>> >>
>> >> to eliminate the silly branch.
>> >
>> >I wonder if we could also get some igts for this limited range stuff.
>> >Maybe something like:
>> >1. grab crc from black/white screen with limited range 2. grab cdc
>> >from almost black/white screen with full range 3. compare the two
>> >
>> >Repeat with an identity csc matrix set.
>> >
>> >Not sure if we can get the crcs to match in these two cases though.
>> >Hard to say without trying it out.
>> >
>>
>> Hi Ville,
>> We will try this approach and update. Ideally crc's should match, will
>> confirm the behavior on hardware.
>
>I suspect it's not going to work on some older platforms because we use the
>magic bit in the pipe/port register to do the range compression. The effect of
>that bit may not be visible in the pipe crc. But starting from hsw it should be ok.
>

Hi Ville,
We tried to test this on GLK and the crc results are not matching. Tried below scenarios:
1. Flip FB (full white)with Broadcast RGB property set to "Full Range" and then with "Limited Range".
     Without applying/changing the CTM.
2. Flip FB (full white) with CTM set as unity and, one flip with "Full Range" and one with "Limited Range".

In both the cases, crc is not matching. Not sure if this is an expected behavior. 

Below is the IGT code snippet used, for reference:

static void test_pipe_limited_range_ctm(data_t *data, igt_plane_t *primary)
{
        color_t white[] = { 
                { 1.0, 1.0, 1.0 },
                { 1.0, 1.0, 1.0 },
                { 1.0, 1.0, 1.0 }
        };
        color_t black[] = { 
                { 0.0, 0.0, 0.0 },
                { 0.0, 0.0, 0.0 },
                { 0.0, 0.0, 0.0 }
        };
        double ctm[] = { 
	1.0, 0.0, 0.0,
                0.0, 1.0, 0.0,
                0.0, 0.0, 1.0 };
        double *degamma_linear, *gamma_linear;
        igt_output_t *output;
        bool has_broadcast_rgb_output = false;

        degamma_linear = generate_table(data->degamma_lut_size, 1.0);
        gamma_linear = generate_table(data->gamma_lut_size, 1.0);

        for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
                drmModeModeInfo *mode;
                struct igt_fb fb_modeset, fb;
                igt_crc_t crc_full, crc_limited;
                int fb_id, fb_modeset_id;

                if (!igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
                        continue;
                has_broadcast_rgb_output = true;
                igt_output_set_pipe(output, primary->pipe->pipe);
                mode = igt_output_get_mode(output);

                /* Create a framebuffer at the size of the output. */
                fb_id = igt_create_fb(data->drm_fd, mode->hdisplay,  mode->vdisplay,
                                      DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,  &fb);
                igt_assert(fb_id);

                fb_modeset_id = igt_create_fb(data->drm_fd, mode->hdisplay,
                                              mode->vdisplay, DRM_FORMAT_XRGB8888,
                                              LOCAL_DRM_FORMAT_MOD_NONE,  &fb_modeset);
                igt_assert(fb_modeset_id);
                igt_plane_set_fb(primary, &fb_modeset);

                set_degamma(data, primary->pipe, degamma_linear);
                set_gamma(data, primary->pipe, gamma_linear);
                set_ctm(primary->pipe, ctm);

                igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
                paint_rectangles(data, mode, black, &fb);
                igt_plane_set_fb(primary, &fb);
                igt_display_commit(&data->display);
                igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
                igt_pipe_crc_collect_crc(data->pipe_crc, &crc_full);
                
                /* Set the output into limited range. */                
                igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_16_235);
                paint_rectangles(data, mode, black, &fb);
                igt_plane_set_fb(primary, &fb);
                igt_display_commit(&data->display);
                igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
                igt_pipe_crc_collect_crc(data->pipe_crc, &crc_limited);

                igt_assert_crc_equal(&crc_full, &crc_limited);

Regards,
Uma Shankar
>> >>
>> >>
>> >> > -			result[i * 3 + i] |= CTM_COEFF_SIGN;
>> >> > +			result[i] |= CTM_COEFF_SIGN;
>> >> >  	}
>> >> >  }
>> >> >
>> >> > --
>> >> > 1.7.9.5
>> >> >
>> >> > _______________________________________________
>> >> > Intel-gfx mailing list
>> >> > Intel-gfx@lists.freedesktop.org
>> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >>
>> >> --
>> >> Ville Syrjälä
>> >> Intel OTC
>> >
>> >--
>> >Ville Syrjälä
>> >Intel OTC
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Ville Syrjälä]

On Fri, Feb 09, 2018 at 01:34:40PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >Sent: Wednesday, January 31, 2018 1:44 AM
> >To: Shankar, Uma <uma.shankar@intel.com>
> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson <johnson.lin@intel.com>;
> >Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
> ><maarten.lankhorst@intel.com>
> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color Handling
> >
> >On Tue, Jan 30, 2018 at 03:23:32PM +0000, Shankar, Uma wrote:
> >>
> >>
> >> >-----Original Message-----
> >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >> >Sent: Friday, December 22, 2017 9:43 PM
> >> >To: Shankar, Uma <uma.shankar@intel.com>
> >> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson
> >> ><johnson.lin@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
> >> >Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color
> >> >Handling
> >> >
> >> >On Fri, Dec 22, 2017 at 05:54:31PM +0200, Ville Syrjälä wrote:
> >> >> On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
> >> >> > From: Johnson Lin <johnson.lin@intel.com>
> >> >> >
> >> >> > Some panels support limited range output (16-235) compared to
> >> >> > full range RGB values (0-255). Also userspace can control the RGB
> >> >> > range using "Broadcast RGB" property. Currently the code to
> >> >> > handle full range to limited range is broken. This patch fixes
> >> >> > the same by properly scaling down all the full range
> >> >> > co-efficients with limited range scaling factor.
> >> >> >
> >> >> > Signed-off-by: Johnson Lin <johnson.lin@intel.com>
> >> >> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> >> > ---
> >> >> >  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
> >> >> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >> >> >
> >> >> > diff --git a/drivers/gpu/drm/i915/intel_color.c
> >> >> > b/drivers/gpu/drm/i915/intel_color.c
> >> >> > index aa66e95..777ce26 100644
> >> >> > --- a/drivers/gpu/drm/i915/intel_color.c
> >> >> > +++ b/drivers/gpu/drm/i915/intel_color.c
> >> >> > @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t
> >> >> > *result,
> >> >int64_t *input)
> >> >> >  	for (i = 0; i < 9; i++)
> >> >> >  		result[i] = 0;
> >> >>
> >> >> This can go.
> >> >>
> >> >> >
> >> >> > -	for (i = 0; i < 3; i++) {
> >> >> > -		int64_t user_coeff = input[i * 3 + i];
> >> >> > +	for (i = 0; i < 9; i++) {
> >> >> > +		int64_t user_coeff = input[i];
> >> >>
> >> >> That's not a two's complement number so should probably be u64.
> >> >>
> >> >> >  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
> >> >> >  		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
> >> >> >  					       0,
> >> >> >  					       CTM_COEFF_4_0 - 1) >> 2;
> >> >>
> >> >> Seems to me that we should be able to drop the >>2 from the
> >> >> limited_coeff since it's always < 1.0, make both of these u32 and
> >> >> then use
> >> >> 'mul_u32_u32() >> 30' to get the answer.
> >> >>
> >> >> >
> >> >> > -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
> >> >> > +		/*
> >> >> > +		 * By scaling every co-efficient with limited range (235-16)
> >> >> > +		 * vs full range (0-255) the final o/p will be scaled down to
> >> >> > +		 * fit in the limited range supported by the panel.
> >> >> > +		 * Need to shift the multiplication result by 28 as the floating
> >> >> > +		 * count expected is of 32bits, multiplication here is done in
> >> >> > +		 * U2.30 so result need to be right shifted by 60-32 = 28
> >> >> > +		 */
> >> >> > +		result[i] = (limited_coeff * abs_coeff) >> 28;
> >> >> >  		if (CTM_COEFF_NEGATIVE(user_coeff))
> >> >>
> >> >> And this can be replaced with just
> >> >>
> >> >> result[i] |= user_coeff & CTM_COEFF_SIGN;
> >> >>
> >> >> to eliminate the silly branch.
> >> >
> >> >I wonder if we could also get some igts for this limited range stuff.
> >> >Maybe something like:
> >> >1. grab crc from black/white screen with limited range 2. grab cdc
> >> >from almost black/white screen with full range 3. compare the two
> >> >
> >> >Repeat with an identity csc matrix set.
> >> >
> >> >Not sure if we can get the crcs to match in these two cases though.
> >> >Hard to say without trying it out.
> >> >
> >>
> >> Hi Ville,
> >> We will try this approach and update. Ideally crc's should match, will
> >> confirm the behavior on hardware.
> >
> >I suspect it's not going to work on some older platforms because we use the
> >magic bit in the pipe/port register to do the range compression. The effect of
> >that bit may not be visible in the pipe crc. But starting from hsw it should be ok.
> >
> 
> Hi Ville,
> We tried to test this on GLK and the crc results are not matching. Tried below scenarios:
> 1. Flip FB (full white)with Broadcast RGB property set to "Full Range" and then with "Limited Range".
>      Without applying/changing the CTM.
> 2. Flip FB (full white) with CTM set as unity and, one flip with "Full Range" and one with "Limited Range".
> 
> In both the cases, crc is not matching. Not sure if this is an expected behavior. 

Well, if you want them to match you need a fb with 16,16,16 and
235,235,235 rgb data for the full range test.

> 
> Below is the IGT code snippet used, for reference:
> 
> static void test_pipe_limited_range_ctm(data_t *data, igt_plane_t *primary)
> {
>         color_t white[] = { 
>                 { 1.0, 1.0, 1.0 },
>                 { 1.0, 1.0, 1.0 },
>                 { 1.0, 1.0, 1.0 }
>         };
>         color_t black[] = { 
>                 { 0.0, 0.0, 0.0 },
>                 { 0.0, 0.0, 0.0 },
>                 { 0.0, 0.0, 0.0 }
>         };
>         double ctm[] = { 
> 	1.0, 0.0, 0.0,
>                 0.0, 1.0, 0.0,
>                 0.0, 0.0, 1.0 };
>         double *degamma_linear, *gamma_linear;
>         igt_output_t *output;
>         bool has_broadcast_rgb_output = false;
> 
>         degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>         gamma_linear = generate_table(data->gamma_lut_size, 1.0);
> 
>         for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe, output) {
>                 drmModeModeInfo *mode;
>                 struct igt_fb fb_modeset, fb;
>                 igt_crc_t crc_full, crc_limited;
>                 int fb_id, fb_modeset_id;
> 
>                 if (!igt_output_has_prop(output, IGT_CONNECTOR_BROADCAST_RGB))
>                         continue;
>                 has_broadcast_rgb_output = true;
>                 igt_output_set_pipe(output, primary->pipe->pipe);
>                 mode = igt_output_get_mode(output);
> 
>                 /* Create a framebuffer at the size of the output. */
>                 fb_id = igt_create_fb(data->drm_fd, mode->hdisplay,  mode->vdisplay,
>                                       DRM_FORMAT_XRGB8888, LOCAL_DRM_FORMAT_MOD_NONE,  &fb);
>                 igt_assert(fb_id);
> 
>                 fb_modeset_id = igt_create_fb(data->drm_fd, mode->hdisplay,
>                                               mode->vdisplay, DRM_FORMAT_XRGB8888,
>                                               LOCAL_DRM_FORMAT_MOD_NONE,  &fb_modeset);
>                 igt_assert(fb_modeset_id);
>                 igt_plane_set_fb(primary, &fb_modeset);
> 
>                 set_degamma(data, primary->pipe, degamma_linear);
>                 set_gamma(data, primary->pipe, gamma_linear);
>                 set_ctm(primary->pipe, ctm);
> 
>                 igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
>                 paint_rectangles(data, mode, black, &fb);
>                 igt_plane_set_fb(primary, &fb);
>                 igt_display_commit(&data->display);
>                 igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>                 igt_pipe_crc_collect_crc(data->pipe_crc, &crc_full);
>                 
>                 /* Set the output into limited range. */                
>                 igt_output_set_prop_value(output, IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_16_235);
>                 paint_rectangles(data, mode, black, &fb);
>                 igt_plane_set_fb(primary, &fb);
>                 igt_display_commit(&data->display);
>                 igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>                 igt_pipe_crc_collect_crc(data->pipe_crc, &crc_limited);
> 
>                 igt_assert_crc_equal(&crc_full, &crc_limited);
> 
> Regards,
> Uma Shankar
> >> >>
> >> >>
> >> >> > -			result[i * 3 + i] |= CTM_COEFF_SIGN;
> >> >> > +			result[i] |= CTM_COEFF_SIGN;
> >> >> >  	}
> >> >> >  }
> >> >> >
> >> >> > --
> >> >> > 1.7.9.5
> >> >> >
> >> >> > _______________________________________________
> >> >> > Intel-gfx mailing list
> >> >> > Intel-gfx@lists.freedesktop.org
> >> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >>
> >> >> --
> >> >> Ville Syrjälä
> >> >> Intel OTC
> >> >
> >> >--
> >> >Ville Syrjälä
> >> >Intel OTC
> >
> >--
> >Ville Syrjälä
> >Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Friday, February 9, 2018 9:09 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson <johnson.lin@intel.com>;
>Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color Handling
>
>On Fri, Feb 09, 2018 at 01:34:40PM +0000, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>> >Sent: Wednesday, January 31, 2018 1:44 AM
>> >To: Shankar, Uma <uma.shankar@intel.com>
>> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson
>> ><johnson.lin@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
>> >Lankhorst, Maarten <maarten.lankhorst@intel.com>
>> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color
>> >Handling
>> >
>> >On Tue, Jan 30, 2018 at 03:23:32PM +0000, Shankar, Uma wrote:
>> >>
>> >>
>> >> >-----Original Message-----
>> >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>> >> >Sent: Friday, December 22, 2017 9:43 PM
>> >> >To: Shankar, Uma <uma.shankar@intel.com>
>> >> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson
>> >> ><johnson.lin@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
>> >> >Lankhorst, Maarten <maarten.lankhorst@intel.com>
>> >> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color
>> >> >Handling
>> >> >
>> >> >On Fri, Dec 22, 2017 at 05:54:31PM +0200, Ville Syrjälä wrote:
>> >> >> On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
>> >> >> > From: Johnson Lin <johnson.lin@intel.com>
>> >> >> >
>> >> >> > Some panels support limited range output (16-235) compared to
>> >> >> > full range RGB values (0-255). Also userspace can control the
>> >> >> > RGB range using "Broadcast RGB" property. Currently the code
>> >> >> > to handle full range to limited range is broken. This patch
>> >> >> > fixes the same by properly scaling down all the full range
>> >> >> > co-efficients with limited range scaling factor.
>> >> >> >
>> >> >> > Signed-off-by: Johnson Lin <johnson.lin@intel.com>
>> >> >> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> >> >> > ---
>> >> >> >  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
>> >> >> >  1 file changed, 12 insertions(+), 4 deletions(-)
>> >> >> >
>> >> >> > diff --git a/drivers/gpu/drm/i915/intel_color.c
>> >> >> > b/drivers/gpu/drm/i915/intel_color.c
>> >> >> > index aa66e95..777ce26 100644
>> >> >> > --- a/drivers/gpu/drm/i915/intel_color.c
>> >> >> > +++ b/drivers/gpu/drm/i915/intel_color.c
>> >> >> > @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t
>> >> >> > *result,
>> >> >int64_t *input)
>> >> >> >  	for (i = 0; i < 9; i++)
>> >> >> >  		result[i] = 0;
>> >> >>
>> >> >> This can go.
>> >> >>
>> >> >> >
>> >> >> > -	for (i = 0; i < 3; i++) {
>> >> >> > -		int64_t user_coeff = input[i * 3 + i];
>> >> >> > +	for (i = 0; i < 9; i++) {
>> >> >> > +		int64_t user_coeff = input[i];
>> >> >>
>> >> >> That's not a two's complement number so should probably be u64.
>> >> >>
>> >> >> >  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE
>>> 2;
>> >> >> >  		uint64_t abs_coeff =
>clamp_val(CTM_COEFF_ABS(user_coeff),
>> >> >> >  					       0,
>> >> >> >  					       CTM_COEFF_4_0 - 1) >> 2;
>> >> >>
>> >> >> Seems to me that we should be able to drop the >>2 from the
>> >> >> limited_coeff since it's always < 1.0, make both of these u32
>> >> >> and then use
>> >> >> 'mul_u32_u32() >> 30' to get the answer.
>> >> >>
>> >> >> >
>> >> >> > -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
>> >> >> > +		/*
>> >> >> > +		 * By scaling every co-efficient with limited range (235-
>16)
>> >> >> > +		 * vs full range (0-255) the final o/p will be scaled down
>to
>> >> >> > +		 * fit in the limited range supported by the panel.
>> >> >> > +		 * Need to shift the multiplication result by 28 as the
>floating
>> >> >> > +		 * count expected is of 32bits, multiplication here is done
>in
>> >> >> > +		 * U2.30 so result need to be right shifted by 60-32 = 28
>> >> >> > +		 */
>> >> >> > +		result[i] = (limited_coeff * abs_coeff) >> 28;
>> >> >> >  		if (CTM_COEFF_NEGATIVE(user_coeff))
>> >> >>
>> >> >> And this can be replaced with just
>> >> >>
>> >> >> result[i] |= user_coeff & CTM_COEFF_SIGN;
>> >> >>
>> >> >> to eliminate the silly branch.
>> >> >
>> >> >I wonder if we could also get some igts for this limited range stuff.
>> >> >Maybe something like:
>> >> >1. grab crc from black/white screen with limited range 2. grab cdc
>> >> >from almost black/white screen with full range 3. compare the two
>> >> >
>> >> >Repeat with an identity csc matrix set.
>> >> >
>> >> >Not sure if we can get the crcs to match in these two cases though.
>> >> >Hard to say without trying it out.
>> >> >
>> >>
>> >> Hi Ville,
>> >> We will try this approach and update. Ideally crc's should match,
>> >> will confirm the behavior on hardware.
>> >
>> >I suspect it's not going to work on some older platforms because we
>> >use the magic bit in the pipe/port register to do the range
>> >compression. The effect of that bit may not be visible in the pipe crc. But
>starting from hsw it should be ok.
>> >
>>
>> Hi Ville,
>> We tried to test this on GLK and the crc results are not matching. Tried below
>scenarios:
>> 1. Flip FB (full white)with Broadcast RGB property set to "Full Range" and then
>with "Limited Range".
>>      Without applying/changing the CTM.
>> 2. Flip FB (full white) with CTM set as unity and, one flip with "Full Range" and
>one with "Limited Range".
>>
>> In both the cases, crc is not matching. Not sure if this is an expected behavior.
>
>Well, if you want them to match you need a fb with 16,16,16 and
>235,235,235 rgb data for the full range test.
>

We tried to give input as lr = ((1.0/255)*235) in order to scale down all RGB values to 235.
     color_t white_lr[] = {
           { lr, lr, lr },
               { lr, lr, lr },
               { lr, lr, lr }
       };

We flipped this with Full Range but still CRC didn't matched on GLK. I am not sure if h/w is modifying
the data wrt Full Range and Limited Range.

Regards,
Uma Shankar

>> Below is the IGT code snippet used, for reference:
>>
>> static void test_pipe_limited_range_ctm(data_t *data, igt_plane_t
>> *primary) {
>>         color_t white[] = {
>>                 { 1.0, 1.0, 1.0 },
>>                 { 1.0, 1.0, 1.0 },
>>                 { 1.0, 1.0, 1.0 }
>>         };
>>         color_t black[] = {
>>                 { 0.0, 0.0, 0.0 },
>>                 { 0.0, 0.0, 0.0 },
>>                 { 0.0, 0.0, 0.0 }
>>         };
>>         double ctm[] = {
>> 	1.0, 0.0, 0.0,
>>                 0.0, 1.0, 0.0,
>>                 0.0, 0.0, 1.0 };
>>         double *degamma_linear, *gamma_linear;
>>         igt_output_t *output;
>>         bool has_broadcast_rgb_output = false;
>>
>>         degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>>         gamma_linear = generate_table(data->gamma_lut_size, 1.0);
>>
>>         for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe,
>output) {
>>                 drmModeModeInfo *mode;
>>                 struct igt_fb fb_modeset, fb;
>>                 igt_crc_t crc_full, crc_limited;
>>                 int fb_id, fb_modeset_id;
>>
>>                 if (!igt_output_has_prop(output,
>IGT_CONNECTOR_BROADCAST_RGB))
>>                         continue;
>>                 has_broadcast_rgb_output = true;
>>                 igt_output_set_pipe(output, primary->pipe->pipe);
>>                 mode = igt_output_get_mode(output);
>>
>>                 /* Create a framebuffer at the size of the output. */
>>                 fb_id = igt_create_fb(data->drm_fd, mode->hdisplay,  mode->vdisplay,
>>                                       DRM_FORMAT_XRGB8888,
>LOCAL_DRM_FORMAT_MOD_NONE,  &fb);
>>                 igt_assert(fb_id);
>>
>>                 fb_modeset_id = igt_create_fb(data->drm_fd, mode->hdisplay,
>>                                               mode->vdisplay, DRM_FORMAT_XRGB8888,
>>                                               LOCAL_DRM_FORMAT_MOD_NONE,  &fb_modeset);
>>                 igt_assert(fb_modeset_id);
>>                 igt_plane_set_fb(primary, &fb_modeset);
>>
>>                 set_degamma(data, primary->pipe, degamma_linear);
>>                 set_gamma(data, primary->pipe, gamma_linear);
>>                 set_ctm(primary->pipe, ctm);
>>
>>                 igt_output_set_prop_value(output,
>IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
>>                 paint_rectangles(data, mode, black, &fb);
>>                 igt_plane_set_fb(primary, &fb);
>>                 igt_display_commit(&data->display);
>>                 igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>>                 igt_pipe_crc_collect_crc(data->pipe_crc, &crc_full);
>>
>>                 /* Set the output into limited range. */
>>                 igt_output_set_prop_value(output,
>IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_16_235);
>>                 paint_rectangles(data, mode, black, &fb);
>>                 igt_plane_set_fb(primary, &fb);
>>                 igt_display_commit(&data->display);
>>                 igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>>                 igt_pipe_crc_collect_crc(data->pipe_crc,
>> &crc_limited);
>>
>>                 igt_assert_crc_equal(&crc_full, &crc_limited);
>>
>> Regards,
>> Uma Shankar
>> >> >>
>> >> >>
>> >> >> > -			result[i * 3 + i] |= CTM_COEFF_SIGN;
>> >> >> > +			result[i] |= CTM_COEFF_SIGN;
>> >> >> >  	}
>> >> >> >  }
>> >> >> >
>> >> >> > --
>> >> >> > 1.7.9.5
>> >> >> >
>> >> >> > _______________________________________________
>> >> >> > Intel-gfx mailing list
>> >> >> > Intel-gfx@lists.freedesktop.org
>> >> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> >> >>
>> >> >> --
>> >> >> Ville Syrjälä
>> >> >> Intel OTC
>> >> >
>> >> >--
>> >> >Ville Syrjälä
>> >> >Intel OTC
>> >
>> >--
>> >Ville Syrjälä
>> >Intel OTC
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Ville Syrjälä]

On Tue, Feb 13, 2018 at 10:37:03AM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >Sent: Friday, February 9, 2018 9:09 PM
> >To: Shankar, Uma <uma.shankar@intel.com>
> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson <johnson.lin@intel.com>;
> >Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
> ><maarten.lankhorst@intel.com>
> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color Handling
> >
> >On Fri, Feb 09, 2018 at 01:34:40PM +0000, Shankar, Uma wrote:
> >>
> >>
> >> >-----Original Message-----
> >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >> >Sent: Wednesday, January 31, 2018 1:44 AM
> >> >To: Shankar, Uma <uma.shankar@intel.com>
> >> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson
> >> ><johnson.lin@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
> >> >Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color
> >> >Handling
> >> >
> >> >On Tue, Jan 30, 2018 at 03:23:32PM +0000, Shankar, Uma wrote:
> >> >>
> >> >>
> >> >> >-----Original Message-----
> >> >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
> >> >> >Sent: Friday, December 22, 2017 9:43 PM
> >> >> >To: Shankar, Uma <uma.shankar@intel.com>
> >> >> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson
> >> >> ><johnson.lin@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>;
> >> >> >Lankhorst, Maarten <maarten.lankhorst@intel.com>
> >> >> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color
> >> >> >Handling
> >> >> >
> >> >> >On Fri, Dec 22, 2017 at 05:54:31PM +0200, Ville Syrjälä wrote:
> >> >> >> On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
> >> >> >> > From: Johnson Lin <johnson.lin@intel.com>
> >> >> >> >
> >> >> >> > Some panels support limited range output (16-235) compared to
> >> >> >> > full range RGB values (0-255). Also userspace can control the
> >> >> >> > RGB range using "Broadcast RGB" property. Currently the code
> >> >> >> > to handle full range to limited range is broken. This patch
> >> >> >> > fixes the same by properly scaling down all the full range
> >> >> >> > co-efficients with limited range scaling factor.
> >> >> >> >
> >> >> >> > Signed-off-by: Johnson Lin <johnson.lin@intel.com>
> >> >> >> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> >> >> >> > ---
> >> >> >> >  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
> >> >> >> >  1 file changed, 12 insertions(+), 4 deletions(-)
> >> >> >> >
> >> >> >> > diff --git a/drivers/gpu/drm/i915/intel_color.c
> >> >> >> > b/drivers/gpu/drm/i915/intel_color.c
> >> >> >> > index aa66e95..777ce26 100644
> >> >> >> > --- a/drivers/gpu/drm/i915/intel_color.c
> >> >> >> > +++ b/drivers/gpu/drm/i915/intel_color.c
> >> >> >> > @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t
> >> >> >> > *result,
> >> >> >int64_t *input)
> >> >> >> >  	for (i = 0; i < 9; i++)
> >> >> >> >  		result[i] = 0;
> >> >> >>
> >> >> >> This can go.
> >> >> >>
> >> >> >> >
> >> >> >> > -	for (i = 0; i < 3; i++) {
> >> >> >> > -		int64_t user_coeff = input[i * 3 + i];
> >> >> >> > +	for (i = 0; i < 9; i++) {
> >> >> >> > +		int64_t user_coeff = input[i];
> >> >> >>
> >> >> >> That's not a two's complement number so should probably be u64.
> >> >> >>
> >> >> >> >  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE
> >>> 2;
> >> >> >> >  		uint64_t abs_coeff =
> >clamp_val(CTM_COEFF_ABS(user_coeff),
> >> >> >> >  					       0,
> >> >> >> >  					       CTM_COEFF_4_0 - 1) >> 2;
> >> >> >>
> >> >> >> Seems to me that we should be able to drop the >>2 from the
> >> >> >> limited_coeff since it's always < 1.0, make both of these u32
> >> >> >> and then use
> >> >> >> 'mul_u32_u32() >> 30' to get the answer.
> >> >> >>
> >> >> >> >
> >> >> >> > -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
> >> >> >> > +		/*
> >> >> >> > +		 * By scaling every co-efficient with limited range (235-
> >16)
> >> >> >> > +		 * vs full range (0-255) the final o/p will be scaled down
> >to
> >> >> >> > +		 * fit in the limited range supported by the panel.
> >> >> >> > +		 * Need to shift the multiplication result by 28 as the
> >floating
> >> >> >> > +		 * count expected is of 32bits, multiplication here is done
> >in
> >> >> >> > +		 * U2.30 so result need to be right shifted by 60-32 = 28
> >> >> >> > +		 */
> >> >> >> > +		result[i] = (limited_coeff * abs_coeff) >> 28;
> >> >> >> >  		if (CTM_COEFF_NEGATIVE(user_coeff))
> >> >> >>
> >> >> >> And this can be replaced with just
> >> >> >>
> >> >> >> result[i] |= user_coeff & CTM_COEFF_SIGN;
> >> >> >>
> >> >> >> to eliminate the silly branch.
> >> >> >
> >> >> >I wonder if we could also get some igts for this limited range stuff.
> >> >> >Maybe something like:
> >> >> >1. grab crc from black/white screen with limited range 2. grab cdc
> >> >> >from almost black/white screen with full range 3. compare the two
> >> >> >
> >> >> >Repeat with an identity csc matrix set.
> >> >> >
> >> >> >Not sure if we can get the crcs to match in these two cases though.
> >> >> >Hard to say without trying it out.
> >> >> >
> >> >>
> >> >> Hi Ville,
> >> >> We will try this approach and update. Ideally crc's should match,
> >> >> will confirm the behavior on hardware.
> >> >
> >> >I suspect it's not going to work on some older platforms because we
> >> >use the magic bit in the pipe/port register to do the range
> >> >compression. The effect of that bit may not be visible in the pipe crc. But
> >starting from hsw it should be ok.
> >> >
> >>
> >> Hi Ville,
> >> We tried to test this on GLK and the crc results are not matching. Tried below
> >scenarios:
> >> 1. Flip FB (full white)with Broadcast RGB property set to "Full Range" and then
> >with "Limited Range".
> >>      Without applying/changing the CTM.
> >> 2. Flip FB (full white) with CTM set as unity and, one flip with "Full Range" and
> >one with "Limited Range".
> >>
> >> In both the cases, crc is not matching. Not sure if this is an expected behavior.
> >
> >Well, if you want them to match you need a fb with 16,16,16 and
> >235,235,235 rgb data for the full range test.
> >
> 
> We tried to give input as lr = ((1.0/255)*235) in order to scale down all RGB values to 235.
>      color_t white_lr[] = {
>            { lr, lr, lr },
>                { lr, lr, lr },
>                { lr, lr, lr }
>        };
> 
> We flipped this with Full Range but still CRC didn't matched on GLK. I am not sure if h/w is modifying
> the data wrt Full Range and Limited Range.

Hmm. I suppose there may be two factors here: the csc's limited
precision, and the pipe's internally using more than 8bpc. I think
it's 12bpc internally always.

Looks like the csc should have 9 bit mantissa, so the closest
to 219/255 we're going to get is 0x1b7. So assuming 12bpc internal
precision we'll get 0xfff*0x1b7/0x200+0x100=0xeb7. Which actually
overshoots the 0xeb0 max specified by the HDMI spec. So looks like
we should perhaps be using 219/256 as the coefficient instead, which
would give us a final value of 0xeaf, which is only one off from the
0xeb0. And the 0x100 I used in the calculation is also based on
on the HDMI spec and matches 16/256 instead of 16/255, so that too
should be changed in the code.

Anyways even with the coefficients/postoffset fixed, we'll be one
off from the expected 12bpc value. And presumably if we use the a
8bpc framebuffer we'll overshoot again to 0xebe (assuming the hw
expands the precision in the way I expect). If we filled the fb
with 0xea instead we'd get 0xeae, which is one off from the csc
value, so not good either. With 10bpc fb we should be able to
match the csc output of 0xeaf if we fill the fb with 0x3ab instead
of the correct 0x3ac.

Oh and we should actually have to consider the precision of the gamma
LUT as well. By default we use the 8bpc one which presumably will
chop off the low bits from the data. Hmm. Actually that should make
things match even if we overshoot the target value a bit. Are you
frobbing with the GAMMA_LUT property in your test?

Anyway it seems to me that we should change the csc programming to
use 219/256 and 16/256 values instead of the current ones. Whether
that can help us get the crc to match I don't really know.

> 
> Regards,
> Uma Shankar
> 
> >> Below is the IGT code snippet used, for reference:
> >>
> >> static void test_pipe_limited_range_ctm(data_t *data, igt_plane_t
> >> *primary) {
> >>         color_t white[] = {
> >>                 { 1.0, 1.0, 1.0 },
> >>                 { 1.0, 1.0, 1.0 },
> >>                 { 1.0, 1.0, 1.0 }
> >>         };
> >>         color_t black[] = {
> >>                 { 0.0, 0.0, 0.0 },
> >>                 { 0.0, 0.0, 0.0 },
> >>                 { 0.0, 0.0, 0.0 }
> >>         };
> >>         double ctm[] = {
> >> 	1.0, 0.0, 0.0,
> >>                 0.0, 1.0, 0.0,
> >>                 0.0, 0.0, 1.0 };
> >>         double *degamma_linear, *gamma_linear;
> >>         igt_output_t *output;
> >>         bool has_broadcast_rgb_output = false;
> >>
> >>         degamma_linear = generate_table(data->degamma_lut_size, 1.0);
> >>         gamma_linear = generate_table(data->gamma_lut_size, 1.0);
> >>
> >>         for_each_valid_output_on_pipe(&data->display, primary->pipe->pipe,
> >output) {
> >>                 drmModeModeInfo *mode;
> >>                 struct igt_fb fb_modeset, fb;
> >>                 igt_crc_t crc_full, crc_limited;
> >>                 int fb_id, fb_modeset_id;
> >>
> >>                 if (!igt_output_has_prop(output,
> >IGT_CONNECTOR_BROADCAST_RGB))
> >>                         continue;
> >>                 has_broadcast_rgb_output = true;
> >>                 igt_output_set_pipe(output, primary->pipe->pipe);
> >>                 mode = igt_output_get_mode(output);
> >>
> >>                 /* Create a framebuffer at the size of the output. */
> >>                 fb_id = igt_create_fb(data->drm_fd, mode->hdisplay,  mode->vdisplay,
> >>                                       DRM_FORMAT_XRGB8888,
> >LOCAL_DRM_FORMAT_MOD_NONE,  &fb);
> >>                 igt_assert(fb_id);
> >>
> >>                 fb_modeset_id = igt_create_fb(data->drm_fd, mode->hdisplay,
> >>                                               mode->vdisplay, DRM_FORMAT_XRGB8888,
> >>                                               LOCAL_DRM_FORMAT_MOD_NONE,  &fb_modeset);
> >>                 igt_assert(fb_modeset_id);
> >>                 igt_plane_set_fb(primary, &fb_modeset);
> >>
> >>                 set_degamma(data, primary->pipe, degamma_linear);
> >>                 set_gamma(data, primary->pipe, gamma_linear);
> >>                 set_ctm(primary->pipe, ctm);
> >>
> >>                 igt_output_set_prop_value(output,
> >IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
> >>                 paint_rectangles(data, mode, black, &fb);
> >>                 igt_plane_set_fb(primary, &fb);
> >>                 igt_display_commit(&data->display);
> >>                 igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> >>                 igt_pipe_crc_collect_crc(data->pipe_crc, &crc_full);
> >>
> >>                 /* Set the output into limited range. */
> >>                 igt_output_set_prop_value(output,
> >IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_16_235);
> >>                 paint_rectangles(data, mode, black, &fb);
> >>                 igt_plane_set_fb(primary, &fb);
> >>                 igt_display_commit(&data->display);
> >>                 igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
> >>                 igt_pipe_crc_collect_crc(data->pipe_crc,
> >> &crc_limited);
> >>
> >>                 igt_assert_crc_equal(&crc_full, &crc_limited);
> >>
> >> Regards,
> >> Uma Shankar
> >> >> >>
> >> >> >>
> >> >> >> > -			result[i * 3 + i] |= CTM_COEFF_SIGN;
> >> >> >> > +			result[i] |= CTM_COEFF_SIGN;
> >> >> >> >  	}
> >> >> >> >  }
> >> >> >> >
> >> >> >> > --
> >> >> >> > 1.7.9.5
> >> >> >> >
> >> >> >> > _______________________________________________
> >> >> >> > Intel-gfx mailing list
> >> >> >> > Intel-gfx@lists.freedesktop.org
> >> >> >> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >> >> >>
> >> >> >> --
> >> >> >> Ville Syrjälä
> >> >> >> Intel OTC
> >> >> >
> >> >> >--
> >> >> >Ville Syrjälä
> >> >> >Intel OTC
> >> >
> >> >--
> >> >Ville Syrjälä
> >> >Intel OTC
> >
> >--
> >Ville Syrjälä
> >Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Shankar, Uma]

>> >> >> >-----Original Message-----
>> >> >> >From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>> >> >> >Sent: Friday, December 22, 2017 9:43 PM
>> >> >> >To: Shankar, Uma <uma.shankar@intel.com>
>> >> >> >Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson
>> >> >> ><johnson.lin@intel.com>; Syrjala, Ville
>> >> >> ><ville.syrjala@intel.com>; Lankhorst, Maarten
>> >> >> ><maarten.lankhorst@intel.com>
>> >> >> >Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range
>> >> >> >Color Handling
>> >> >> >
>> >> >> >On Fri, Dec 22, 2017 at 05:54:31PM +0200, Ville Syrjälä wrote:
>> >> >> >> On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
>> >> >> >> > From: Johnson Lin <johnson.lin@intel.com>
>> >> >> >> >
>> >> >> >> > Some panels support limited range output (16-235) compared
>> >> >> >> > to full range RGB values (0-255). Also userspace can
>> >> >> >> > control the RGB range using "Broadcast RGB" property.
>> >> >> >> > Currently the code to handle full range to limited range is
>> >> >> >> > broken. This patch fixes the same by properly scaling down
>> >> >> >> > all the full range co-efficients with limited range scaling factor.
>> >> >> >> >
>> >> >> >> > Signed-off-by: Johnson Lin <johnson.lin@intel.com>
>> >> >> >> > Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> >> >> >> > ---
>> >> >> >> >  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
>> >> >> >> >  1 file changed, 12 insertions(+), 4 deletions(-)
>> >> >> >> >
>> >> >> >> > diff --git a/drivers/gpu/drm/i915/intel_color.c
>> >> >> >> > b/drivers/gpu/drm/i915/intel_color.c
>> >> >> >> > index aa66e95..777ce26 100644
>> >> >> >> > --- a/drivers/gpu/drm/i915/intel_color.c
>> >> >> >> > +++ b/drivers/gpu/drm/i915/intel_color.c
>> >> >> >> > @@ -94,16 +94,24 @@ static void
>> >> >> >> > ctm_mult_by_limited(uint64_t *result,
>> >> >> >int64_t *input)
>> >> >> >> >  	for (i = 0; i < 9; i++)
>> >> >> >> >  		result[i] = 0;
>> >> >> >>
>> >> >> >> This can go.
>> >> >> >>
>> >> >> >> >
>> >> >> >> > -	for (i = 0; i < 3; i++) {
>> >> >> >> > -		int64_t user_coeff = input[i * 3 + i];
>> >> >> >> > +	for (i = 0; i < 9; i++) {
>> >> >> >> > +		int64_t user_coeff = input[i];
>> >> >> >>
>> >> >> >> That's not a two's complement number so should probably be u64.
>> >> >> >>
>> >> >> >> >  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE
>> >>> 2;
>> >> >> >> >  		uint64_t abs_coeff =
>> >clamp_val(CTM_COEFF_ABS(user_coeff),
>> >> >> >> >  					       0,
>> >> >> >> >  					       CTM_COEFF_4_0 - 1) >> 2;
>> >> >> >>
>> >> >> >> Seems to me that we should be able to drop the >>2 from the
>> >> >> >> limited_coeff since it's always < 1.0, make both of these u32
>> >> >> >> and then use
>> >> >> >> 'mul_u32_u32() >> 30' to get the answer.
>> >> >> >>
>> >> >> >> >
>> >> >> >> > -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
>> >> >> >> > +		/*
>> >> >> >> > +		 * By scaling every co-efficient with limited range
>> >> >> >> > +(235-
>> >16)
>> >> >> >> > +		 * vs full range (0-255) the final o/p will be scaled
>> >> >> >> > +down
>> >to
>> >> >> >> > +		 * fit in the limited range supported by the panel.
>> >> >> >> > +		 * Need to shift the multiplication result by 28 as the
>> >floating
>> >> >> >> > +		 * count expected is of 32bits, multiplication here is
>> >> >> >> > +done
>> >in
>> >> >> >> > +		 * U2.30 so result need to be right shifted by 60-32 = 28
>> >> >> >> > +		 */
>> >> >> >> > +		result[i] = (limited_coeff * abs_coeff) >> 28;
>> >> >> >> >  		if (CTM_COEFF_NEGATIVE(user_coeff))
>> >> >> >>
>> >> >> >> And this can be replaced with just
>> >> >> >>
>> >> >> >> result[i] |= user_coeff & CTM_COEFF_SIGN;
>> >> >> >>
>> >> >> >> to eliminate the silly branch.
>> >> >> >
>> >> >> >I wonder if we could also get some igts for this limited range stuff.
>> >> >> >Maybe something like:
>> >> >> >1. grab crc from black/white screen with limited range 2. grab
>> >> >> >cdc from almost black/white screen with full range 3. compare
>> >> >> >the two
>> >> >> >
>> >> >> >Repeat with an identity csc matrix set.
>> >> >> >
>> >> >> >Not sure if we can get the crcs to match in these two cases though.
>> >> >> >Hard to say without trying it out.
>> >> >> >
>> >> >>
>> >> >> Hi Ville,
>> >> >> We will try this approach and update. Ideally crc's should
>> >> >> match, will confirm the behavior on hardware.
>> >> >
>> >> >I suspect it's not going to work on some older platforms because
>> >> >we use the magic bit in the pipe/port register to do the range
>> >> >compression. The effect of that bit may not be visible in the pipe
>> >> >crc. But
>> >starting from hsw it should be ok.
>> >> >
>> >>
>> >> Hi Ville,
>> >> We tried to test this on GLK and the crc results are not matching.
>> >> Tried below
>> >scenarios:
>> >> 1. Flip FB (full white)with Broadcast RGB property set to "Full
>> >> Range" and then
>> >with "Limited Range".
>> >>      Without applying/changing the CTM.
>> >> 2. Flip FB (full white) with CTM set as unity and, one flip with
>> >> "Full Range" and
>> >one with "Limited Range".
>> >>
>> >> In both the cases, crc is not matching. Not sure if this is an expected
>behavior.
>> >
>> >Well, if you want them to match you need a fb with 16,16,16 and
>> >235,235,235 rgb data for the full range test.
>> >
>>
>> We tried to give input as lr = ((1.0/255)*235) in order to scale down all RGB
>values to 235.
>>      color_t white_lr[] = {
>>            { lr, lr, lr },
>>                { lr, lr, lr },
>>                { lr, lr, lr }
>>        };
>>
>> We flipped this with Full Range but still CRC didn't matched on GLK. I
>> am not sure if h/w is modifying the data wrt Full Range and Limited Range.
>
>Hmm. I suppose there may be two factors here: the csc's limited precision, and
>the pipe's internally using more than 8bpc. I think it's 12bpc internally always.
>
>Looks like the csc should have 9 bit mantissa, so the closest to 219/255 we're
>going to get is 0x1b7. So assuming 12bpc internal precision we'll get
>0xfff*0x1b7/0x200+0x100=0xeb7. Which actually overshoots the 0xeb0 max
>specified by the HDMI spec. So looks like we should perhaps be using 219/256 as
>the coefficient instead, which would give us a final value of 0xeaf, which is only
>one off from the 0xeb0. And the 0x100 I used in the calculation is also based on
>on the HDMI spec and matches 16/256 instead of 16/255, so that too should be
>changed in the code.
>
>Anyways even with the coefficients/postoffset fixed, we'll be one off from the
>expected 12bpc value. And presumably if we use the a 8bpc framebuffer we'll
>overshoot again to 0xebe (assuming the hw expands the precision in the way I
>expect). If we filled the fb with 0xea instead we'd get 0xeae, which is one off
>from the csc value, so not good either. With 10bpc fb we should be able to match
>the csc output of 0xeaf if we fill the fb with 0x3ab instead of the correct 0x3ac.
>
>Oh and we should actually have to consider the precision of the gamma LUT as
>well. By default we use the 8bpc one which presumably will chop off the low bits
>from the data. Hmm. Actually that should make things match even if we
>overshoot the target value a bit. Are you frobbing with the GAMMA_LUT property
>in your test?

We tried to test with gamma/degamma disable just to rule out any precision loss. 
Also, to even rule out CSC we tried disabling CTM block so as to make sure the reason of
delta remains only the pipe precision (12 bits in case of GLK). 

And we used 219/256 as the element of the RGB matrix while creating the Frame Buffer.
So we gave 219/256 as buffer (8bit)- Flip with limited range and with Full Range.
In this case also, CRC's are not matching but the result was very close (not sure how the CRC
algorithms are implemented). I don't think close is any good, till it matches exactly :(

Another approach I can think of is - to use some golden references. Pre-computed CRC's
(obtained experimentally) can be compared with CRC obtained while executing the tests
to catch any regression if any. Not sure if this is a good idea though !!!

Regards,
Uma Shankar

>
>Anyway it seems to me that we should change the csc programming to use
>219/256 and 16/256 values instead of the current ones. Whether that can help us
>get the crc to match I don't really know.
>
>>
>> Regards,
>> Uma Shankar
>>
>> >> Below is the IGT code snippet used, for reference:
>> >>
>> >> static void test_pipe_limited_range_ctm(data_t *data, igt_plane_t
>> >> *primary) {
>> >>         color_t white[] = {
>> >>                 { 1.0, 1.0, 1.0 },
>> >>                 { 1.0, 1.0, 1.0 },
>> >>                 { 1.0, 1.0, 1.0 }
>> >>         };
>> >>         color_t black[] = {
>> >>                 { 0.0, 0.0, 0.0 },
>> >>                 { 0.0, 0.0, 0.0 },
>> >>                 { 0.0, 0.0, 0.0 }
>> >>         };
>> >>         double ctm[] = {
>> >> 	1.0, 0.0, 0.0,
>> >>                 0.0, 1.0, 0.0,
>> >>                 0.0, 0.0, 1.0 };
>> >>         double *degamma_linear, *gamma_linear;
>> >>         igt_output_t *output;
>> >>         bool has_broadcast_rgb_output = false;
>> >>
>> >>         degamma_linear = generate_table(data->degamma_lut_size, 1.0);
>> >>         gamma_linear = generate_table(data->gamma_lut_size, 1.0);
>> >>
>> >>         for_each_valid_output_on_pipe(&data->display,
>> >> primary->pipe->pipe,
>> >output) {
>> >>                 drmModeModeInfo *mode;
>> >>                 struct igt_fb fb_modeset, fb;
>> >>                 igt_crc_t crc_full, crc_limited;
>> >>                 int fb_id, fb_modeset_id;
>> >>
>> >>                 if (!igt_output_has_prop(output,
>> >IGT_CONNECTOR_BROADCAST_RGB))
>> >>                         continue;
>> >>                 has_broadcast_rgb_output = true;
>> >>                 igt_output_set_pipe(output, primary->pipe->pipe);
>> >>                 mode = igt_output_get_mode(output);
>> >>
>> >>                 /* Create a framebuffer at the size of the output. */
>> >>                 fb_id = igt_create_fb(data->drm_fd, mode->hdisplay,  mode-
>>vdisplay,
>> >>                                       DRM_FORMAT_XRGB8888,
>> >LOCAL_DRM_FORMAT_MOD_NONE,  &fb);
>> >>                 igt_assert(fb_id);
>> >>
>> >>                 fb_modeset_id = igt_create_fb(data->drm_fd, mode->hdisplay,
>> >>                                               mode->vdisplay, DRM_FORMAT_XRGB8888,
>> >>                                               LOCAL_DRM_FORMAT_MOD_NONE,
>&fb_modeset);
>> >>                 igt_assert(fb_modeset_id);
>> >>                 igt_plane_set_fb(primary, &fb_modeset);
>> >>
>> >>                 set_degamma(data, primary->pipe, degamma_linear);
>> >>                 set_gamma(data, primary->pipe, gamma_linear);
>> >>                 set_ctm(primary->pipe, ctm);
>> >>
>> >>                 igt_output_set_prop_value(output,
>> >IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_FULL);
>> >>                 paint_rectangles(data, mode, black, &fb);
>> >>                 igt_plane_set_fb(primary, &fb);
>> >>                 igt_display_commit(&data->display);
>> >>                 igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>> >>                 igt_pipe_crc_collect_crc(data->pipe_crc,
>> >> &crc_full);
>> >>
>> >>                 /* Set the output into limited range. */
>> >>                 igt_output_set_prop_value(output,
>> >IGT_CONNECTOR_BROADCAST_RGB, BROADCAST_RGB_16_235);
>> >>                 paint_rectangles(data, mode, black, &fb);
>> >>                 igt_plane_set_fb(primary, &fb);
>> >>                 igt_display_commit(&data->display);
>> >>                 igt_wait_for_vblank(data->drm_fd, primary->pipe->pipe);
>> >>                 igt_pipe_crc_collect_crc(data->pipe_crc,
>> >> &crc_limited);
>> >>
>> >>                 igt_assert_crc_equal(&crc_full, &crc_limited);
>> >>
>> >> Regards,
>> >> Uma Shankar
>> >> >> >>
>> >> >> >>
>> >> >> >> > -			result[i * 3 + i] |= CTM_COEFF_SIGN;
>> >> >> >> > +			result[i] |= CTM_COEFF_SIGN;
>> >> >> >> >  	}
>> >> >> >> >  }
>> >> >> >> >
>> >> >> >> > --
>> >> >> >> > 1.7.9.5
>> >> >> >> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Friday, December 22, 2017 9:25 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson <johnson.lin@intel.com>;
>Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color Handling
>
>On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
>> From: Johnson Lin <johnson.lin@intel.com>
>>
>> Some panels support limited range output (16-235) compared to full
>> range RGB values (0-255). Also userspace can control the RGB range
>> using "Broadcast RGB" property. Currently the code to handle full
>> range to limited range is broken. This patch fixes the same by
>> properly scaling down all the full range co-efficients with limited
>> range scaling factor.
>>
>> Signed-off-by: Johnson Lin <johnson.lin@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index aa66e95..777ce26 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t *result,
>int64_t *input)
>>  	for (i = 0; i < 9; i++)
>>  		result[i] = 0;
>
>This can go.
>

Yes, will remove this.

>>
>> -	for (i = 0; i < 3; i++) {
>> -		int64_t user_coeff = input[i * 3 + i];
>> +	for (i = 0; i < 9; i++) {
>> +		int64_t user_coeff = input[i];
>
>That's not a two's complement number so should probably be u64.

Ok, will change this.

>
>>  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
>>  		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
>>  					       0,
>>  					       CTM_COEFF_4_0 - 1) >> 2;
>
>Seems to me that we should be able to drop the >>2 from the limited_coeff since
>it's always < 1.0, make both of these u32 and then use
>'mul_u32_u32() >> 30' to get the answer.

Yes, this should work. Will change this.

>
>>
>> -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
>> +		/*
>> +		 * By scaling every co-efficient with limited range (235-16)
>> +		 * vs full range (0-255) the final o/p will be scaled down to
>> +		 * fit in the limited range supported by the panel.
>> +		 * Need to shift the multiplication result by 28 as the floating
>> +		 * count expected is of 32bits, multiplication here is done in
>> +		 * U2.30 so result need to be right shifted by 60-32 = 28
>> +		 */
>> +		result[i] = (limited_coeff * abs_coeff) >> 28;
>>  		if (CTM_COEFF_NEGATIVE(user_coeff))
>
>And this can be replaced with just
>
>result[i] |= user_coeff & CTM_COEFF_SIGN;
>
>to eliminate the silly branch.
>

Sure, this can be changed. Will update this.

Thanks Ville for your comments.

Regards,
Uma Shankar
>
>> -			result[i * 3 + i] |= CTM_COEFF_SIGN;
>> +			result[i] |= CTM_COEFF_SIGN;
>>  	}
>>  }
>>
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Fix Limited Range Color Handling

[Uma Shankar]

From: Johnson Lin <johnson.lin@intel.com>

Some panels support limited range output (16-235) compared
to full range RGB values (0-255). Also userspace can control
the RGB range using "Broadcast RGB" property. Currently the
code to handle full range to limited range is broken. This
patch fixes the same by properly scaling down all the full
range co-efficients with limited range scaling factor.

v2: Fixed Ville's review comments.

Signed-off-by: Johnson Lin <johnson.lin@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c |   29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index aa66e95..55408e2 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -84,26 +84,27 @@ static bool crtc_state_is_legacy_gamma(struct drm_crtc_state *state)
 
 /*
  * When using limited range, multiply the matrix given by userspace by
- * the matrix that we would use for the limited range. We do the
- * multiplication in U2.30 format.
+ * the matrix that we would use for the limited range.
  */
 static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
 {
 	int i;
 
-	for (i = 0; i < 9; i++)
-		result[i] = 0;
+	for (i = 0; i < 9; i++) {
+		uint64_t user_coeff = input[i];
+		uint32_t limited_coeff = CTM_COEFF_LIMITED_RANGE;
+		uint32_t abs_coeff = (uint32_t)(clamp_val(
+						CTM_COEFF_ABS(user_coeff),
+						0,
+						CTM_COEFF_4_0 - 1) >> 2);
 
-	for (i = 0; i < 3; i++) {
-		int64_t user_coeff = input[i * 3 + i];
-		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
-		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
-					       0,
-					       CTM_COEFF_4_0 - 1) >> 2;
-
-		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
-		if (CTM_COEFF_NEGATIVE(user_coeff))
-			result[i * 3 + i] |= CTM_COEFF_SIGN;
+		/*
+		 * By scaling every co-efficient with limited range (16-235)
+		 * vs full range (0-255) the final o/p will be scaled down to
+		 * fit in the limited range supported by the panel.
+		 */
+		result[i] = mul_u32_u32(limited_coeff, abs_coeff) >> 30;
+		result[i] |= user_coeff & CTM_COEFF_SIGN;
 	}
 }
 
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix Limited Range Color Handling

[Ville Syrjälä]

On Fri, Dec 29, 2017 at 08:29:11PM +0530, Uma Shankar wrote:
> From: Johnson Lin <johnson.lin@intel.com>
> 
> Some panels support limited range output (16-235) compared
> to full range RGB values (0-255). Also userspace can control
> the RGB range using "Broadcast RGB" property. Currently the
> code to handle full range to limited range is broken. This
> patch fixes the same by properly scaling down all the full
> range co-efficients with limited range scaling factor.
> 
> v2: Fixed Ville's review comments.
> 
> Signed-off-by: Johnson Lin <johnson.lin@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c |   29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index aa66e95..55408e2 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -84,26 +84,27 @@ static bool crtc_state_is_legacy_gamma(struct drm_crtc_state *state)
>  
>  /*
>   * When using limited range, multiply the matrix given by userspace by
> - * the matrix that we would use for the limited range. We do the
> - * multiplication in U2.30 format.
> + * the matrix that we would use for the limited range.
>   */
>  static void ctm_mult_by_limited(uint64_t *result, int64_t *input)

Since your touching pretty much every line anyway, maybe
s/uint64/u64/ etc. all over. 

'input' should really be const u64 *.

>  {
>  	int i;
>  
> -	for (i = 0; i < 9; i++)
> -		result[i] = 0;
> +	for (i = 0; i < 9; i++) {
> +		uint64_t user_coeff = input[i];
> +		uint32_t limited_coeff = CTM_COEFF_LIMITED_RANGE;
> +		uint32_t abs_coeff = (uint32_t)(clamp_val(

The cast is not needed.

Apart from those nits this lgtm.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +						CTM_COEFF_ABS(user_coeff),
> +						0,
> +						CTM_COEFF_4_0 - 1) >> 2);
>  
> -	for (i = 0; i < 3; i++) {
> -		int64_t user_coeff = input[i * 3 + i];
> -		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
> -		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
> -					       0,
> -					       CTM_COEFF_4_0 - 1) >> 2;
> -
> -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
> -		if (CTM_COEFF_NEGATIVE(user_coeff))
> -			result[i * 3 + i] |= CTM_COEFF_SIGN;
> +		/*
> +		 * By scaling every co-efficient with limited range (16-235)
> +		 * vs full range (0-255) the final o/p will be scaled down to
> +		 * fit in the limited range supported by the panel.
> +		 */
> +		result[i] = mul_u32_u32(limited_coeff, abs_coeff) >> 30;
> +		result[i] |= user_coeff & CTM_COEFF_SIGN;
>  	}
>  }
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[v3] drm/i915: Fix Limited Range Color Handling

[Uma Shankar]

From: Johnson Lin <johnson.lin@intel.com>

Some panels support limited range output (16-235) compared
to full range RGB values (0-255). Also userspace can control
the RGB range using "Broadcast RGB" property. Currently the
code to handle full range to limited range is broken. This
patch fixes the same by properly scaling down all the full
range co-efficients with limited range scaling factor.

v2: Fixed Ville's review comments.

v3: Changed input to const and used correct data types as
    suggested by Ville

Signed-off-by: Johnson Lin <johnson.lin@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Ville Syrjä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 333a23b..3b708a9 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -84,26 +84,27 @@ static bool crtc_state_is_legacy_gamma(struct drm_crtc_state *state)
 
 /*
  * When using limited range, multiply the matrix given by userspace by
- * the matrix that we would use for the limited range. We do the
- * multiplication in U2.30 format.
+ * the matrix that we would use for the limited range.
  */
-static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
+static void ctm_mult_by_limited(u64 *result, const u64 *input)
 {
 	int i;
 
-	for (i = 0; i < 9; i++)
-		result[i] = 0;
+	for (i = 0; i < 9; i++) {
+		u64 user_coeff = input[i];
+		uint32_t limited_coeff = CTM_COEFF_LIMITED_RANGE;
+		uint32_t abs_coeff = clamp_val(
+						CTM_COEFF_ABS(user_coeff),
+						0,
+						CTM_COEFF_4_0 - 1) >> 2;
 
-	for (i = 0; i < 3; i++) {
-		int64_t user_coeff = input[i * 3 + i];
-		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
-		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
-					       0,
-					       CTM_COEFF_4_0 - 1) >> 2;
-
-		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
-		if (CTM_COEFF_NEGATIVE(user_coeff))
-			result[i * 3 + i] |= CTM_COEFF_SIGN;
+		/*
+		 * By scaling every co-efficient with limited range (16-235)
+		 * vs full range (0-255) the final o/p will be scaled down to
+		 * fit in the limited range supported by the panel.
+		 */
+		result[i] = mul_u32_u32(limited_coeff, abs_coeff) >> 30;
+		result[i] |= user_coeff & CTM_COEFF_SIGN;
 	}
 }
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v3] drm/i915: Fix Limited Range Color Handling

[Ville Syrjälä]

On Tue, Jan 30, 2018 at 08:49:22PM +0530, Uma Shankar wrote:
> From: Johnson Lin <johnson.lin@intel.com>
> 
> Some panels support limited range output (16-235) compared
> to full range RGB values (0-255). Also userspace can control
> the RGB range using "Broadcast RGB" property. Currently the
> code to handle full range to limited range is broken. This
> patch fixes the same by properly scaling down all the full
> range co-efficients with limited range scaling factor.
> 
> v2: Fixed Ville's review comments.
> 
> v3: Changed input to const and used correct data types as
>     suggested by Ville
> 
> Signed-off-by: Johnson Lin <johnson.lin@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Ville Syrjä <ville.syrjala@linux.intel.com>

Name fail.

> ---
>  drivers/gpu/drm/i915/intel_color.c | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 333a23b..3b708a9 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -84,26 +84,27 @@ static bool crtc_state_is_legacy_gamma(struct drm_crtc_state *state)
>  
>  /*
>   * When using limited range, multiply the matrix given by userspace by
> - * the matrix that we would use for the limited range. We do the
> - * multiplication in U2.30 format.
> + * the matrix that we would use for the limited range.
>   */
> -static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
> +static void ctm_mult_by_limited(u64 *result, const u64 *input)
>  {
>  	int i;
>  
> -	for (i = 0; i < 9; i++)
> -		result[i] = 0;
> +	for (i = 0; i < 9; i++) {
> +		u64 user_coeff = input[i];
> +		uint32_t limited_coeff = CTM_COEFF_LIMITED_RANGE;

You forgot s/uint32_t/u32/ here...

> +		uint32_t abs_coeff = clamp_val(

and here. And aving a newline here seems weird.

> +						CTM_COEFF_ABS(user_coeff),
> +						0,
> +						CTM_COEFF_4_0 - 1) >> 2;
>  
> -	for (i = 0; i < 3; i++) {
> -		int64_t user_coeff = input[i * 3 + i];
> -		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
> -		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
> -					       0,
> -					       CTM_COEFF_4_0 - 1) >> 2;
> -
> -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
> -		if (CTM_COEFF_NEGATIVE(user_coeff))
> -			result[i * 3 + i] |= CTM_COEFF_SIGN;
> +		/*
> +		 * By scaling every co-efficient with limited range (16-235)
> +		 * vs full range (0-255) the final o/p will be scaled down to
> +		 * fit in the limited range supported by the panel.
> +		 */
> +		result[i] = mul_u32_u32(limited_coeff, abs_coeff) >> 30;
> +		result[i] |= user_coeff & CTM_COEFF_SIGN;
>  	}
>  }
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v3] drm/i915: Fix Limited Range Color Handling

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Tuesday, January 30, 2018 8:53 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lin, Johnson <johnson.lin@intel.com>;
>Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v3] drm/i915: Fix Limited Range Color Handling
>
>On Tue, Jan 30, 2018 at 08:49:22PM +0530, Uma Shankar wrote:
>> From: Johnson Lin <johnson.lin@intel.com>
>>
>> Some panels support limited range output (16-235) compared to full
>> range RGB values (0-255). Also userspace can control the RGB range
>> using "Broadcast RGB" property. Currently the code to handle full
>> range to limited range is broken. This patch fixes the same by
>> properly scaling down all the full range co-efficients with limited
>> range scaling factor.
>>
>> v2: Fixed Ville's review comments.
>>
>> v3: Changed input to const and used correct data types as
>>     suggested by Ville
>>
>> Signed-off-by: Johnson Lin <johnson.lin@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> Reviewed-by: Ville Syrjä <ville.syrjala@linux.intel.com>
>
>Name fail.
>

Have updated this.

>> ---
>>  drivers/gpu/drm/i915/intel_color.c | 31
>> ++++++++++++++++---------------
>>  1 file changed, 16 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 333a23b..3b708a9 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -84,26 +84,27 @@ static bool crtc_state_is_legacy_gamma(struct
>> drm_crtc_state *state)
>>
>>  /*
>>   * When using limited range, multiply the matrix given by userspace
>> by
>> - * the matrix that we would use for the limited range. We do the
>> - * multiplication in U2.30 format.
>> + * the matrix that we would use for the limited range.
>>   */
>> -static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
>> +static void ctm_mult_by_limited(u64 *result, const u64 *input)
>>  {
>>  	int i;
>>
>> -	for (i = 0; i < 9; i++)
>> -		result[i] = 0;
>> +	for (i = 0; i < 9; i++) {
>> +		u64 user_coeff = input[i];
>> +		uint32_t limited_coeff = CTM_COEFF_LIMITED_RANGE;
>
>You forgot s/uint32_t/u32/ here...

This file is using this extensively, have changed this function though. I guess it will
need a bigger cleanup.

>
>> +		uint32_t abs_coeff = clamp_val(
>
>and here. And aving a newline here seems weird.

Have updated this and re-sent a new patch.

Thanks & Regards,
Uma Shankar
>
>> +						CTM_COEFF_ABS(user_coeff),
>> +						0,
>> +						CTM_COEFF_4_0 - 1) >> 2;
>>
>> -	for (i = 0; i < 3; i++) {
>> -		int64_t user_coeff = input[i * 3 + i];
>> -		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
>> -		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
>> -					       0,
>> -					       CTM_COEFF_4_0 - 1) >> 2;
>> -
>> -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
>> -		if (CTM_COEFF_NEGATIVE(user_coeff))
>> -			result[i * 3 + i] |= CTM_COEFF_SIGN;
>> +		/*
>> +		 * By scaling every co-efficient with limited range (16-235)
>> +		 * vs full range (0-255) the final o/p will be scaled down to
>> +		 * fit in the limited range supported by the panel.
>> +		 */
>> +		result[i] = mul_u32_u32(limited_coeff, abs_coeff) >> 30;
>> +		result[i] |= user_coeff & CTM_COEFF_SIGN;
>>  	}
>>  }
>>
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Ville Syrjälä
>Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[v4] drm/i915: Fix Limited Range Color Handling

[Uma Shankar]

From: Johnson Lin <johnson.lin@intel.com>

Some panels support limited range output (16-235) compared
to full range RGB values (0-255). Also userspace can control
the RGB range using "Broadcast RGB" property. Currently the
code to handle full range to limited range is broken. This
patch fixes the same by properly scaling down all the full
range co-efficients with limited range scaling factor.

v2: Fixed Ville's review comments.

v3: Changed input to const and used correct data types as
    suggested by Ville

v4: Fixed some missing data type corrections.

Signed-off-by: Johnson Lin <johnson.lin@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 333a23b..0b505d4 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -84,26 +84,25 @@ static bool crtc_state_is_legacy_gamma(struct drm_crtc_state *state)
 
 /*
  * When using limited range, multiply the matrix given by userspace by
- * the matrix that we would use for the limited range. We do the
- * multiplication in U2.30 format.
+ * the matrix that we would use for the limited range.
  */
-static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
+static void ctm_mult_by_limited(u64 *result, const u64 *input)
 {
 	int i;
 
-	for (i = 0; i < 9; i++)
-		result[i] = 0;
+	for (i = 0; i < 9; i++) {
+		u64 user_coeff = input[i];
+		u32 limited_coeff = CTM_COEFF_LIMITED_RANGE;
+		u32 abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff), 0,
+					  CTM_COEFF_4_0 - 1) >> 2;
 
-	for (i = 0; i < 3; i++) {
-		int64_t user_coeff = input[i * 3 + i];
-		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
-		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
-					       0,
-					       CTM_COEFF_4_0 - 1) >> 2;
-
-		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
-		if (CTM_COEFF_NEGATIVE(user_coeff))
-			result[i * 3 + i] |= CTM_COEFF_SIGN;
+		/*
+		 * By scaling every co-efficient with limited range (16-235)
+		 * vs full range (0-255) the final o/p will be scaled down to
+		 * fit in the limited range supported by the panel.
+		 */
+		result[i] = mul_u32_u32(limited_coeff, abs_coeff) >> 30;
+		result[i] |= user_coeff & CTM_COEFF_SIGN;
 	}
 }
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v4] drm/i915: Fix Limited Range Color Handling

[Ville Syrjälä]

On Tue, Jan 30, 2018 at 09:21:29PM +0530, Uma Shankar wrote:
> From: Johnson Lin <johnson.lin@intel.com>
> 
> Some panels support limited range output (16-235) compared
> to full range RGB values (0-255). Also userspace can control
> the RGB range using "Broadcast RGB" property. Currently the
> code to handle full range to limited range is broken. This
> patch fixes the same by properly scaling down all the full
> range co-efficients with limited range scaling factor.
> 
> v2: Fixed Ville's review comments.
> 
> v3: Changed input to const and used correct data types as
>     suggested by Ville
> 
> v4: Fixed some missing data type corrections.
> 
> Signed-off-by: Johnson Lin <johnson.lin@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

OK, so maybe we can't get the test done or maybe we can. Either way
I've gone and pushed this to dinq. Thanks for the patch.

> ---
>  drivers/gpu/drm/i915/intel_color.c | 29 ++++++++++++++---------------
>  1 file changed, 14 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 333a23b..0b505d4 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -84,26 +84,25 @@ static bool crtc_state_is_legacy_gamma(struct drm_crtc_state *state)
>  
>  /*
>   * When using limited range, multiply the matrix given by userspace by
> - * the matrix that we would use for the limited range. We do the
> - * multiplication in U2.30 format.
> + * the matrix that we would use for the limited range.
>   */
> -static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
> +static void ctm_mult_by_limited(u64 *result, const u64 *input)
>  {
>  	int i;
>  
> -	for (i = 0; i < 9; i++)
> -		result[i] = 0;
> +	for (i = 0; i < 9; i++) {
> +		u64 user_coeff = input[i];
> +		u32 limited_coeff = CTM_COEFF_LIMITED_RANGE;
> +		u32 abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff), 0,
> +					  CTM_COEFF_4_0 - 1) >> 2;
>  
> -	for (i = 0; i < 3; i++) {
> -		int64_t user_coeff = input[i * 3 + i];
> -		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
> -		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
> -					       0,
> -					       CTM_COEFF_4_0 - 1) >> 2;
> -
> -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
> -		if (CTM_COEFF_NEGATIVE(user_coeff))
> -			result[i * 3 + i] |= CTM_COEFF_SIGN;
> +		/*
> +		 * By scaling every co-efficient with limited range (16-235)
> +		 * vs full range (0-255) the final o/p will be scaled down to
> +		 * fit in the limited range supported by the panel.
> +		 */
> +		result[i] = mul_u32_u32(limited_coeff, abs_coeff) >> 30;
> +		result[i] |= user_coeff & CTM_COEFF_SIGN;
>  	}
>  }
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: warning for drm/i915: Fix Limited Range Color Handling

[Patchwork]

== Series Details ==

Series: drm/i915: Fix Limited Range Color Handling
URL   : https://patchwork.freedesktop.org/series/35725/
State : warning

== Summary ==

Test kms_flip:
        Subgroup vblank-vs-modeset-suspend-interruptible:
                pass       -> SKIP       (shard-snb)
                skip       -> PASS       (shard-hsw) fdo#103540 +1
Test gem_mmap_wc:
        Subgroup set-cache-level:
                skip       -> PASS       (shard-snb)
Test kms_flip_event_leak:
                skip       -> PASS       (shard-snb)
Test kms_chv_cursor_fail:
        Subgroup pipe-c-128x128-top-edge:
                incomplete -> PASS       (shard-hsw)
Test pm_rpm:
        Subgroup system-suspend-execbuf:
                skip       -> PASS       (shard-hsw) fdo#103375

fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375

shard-hsw        total:2712 pass:1537 dwarn:1   dfail:0   fail:10  skip:1164 time:9460s
shard-snb        total:2712 pass:1309 dwarn:1   dfail:0   fail:10  skip:1392 time:8043s
Blacklisted hosts:
shard-apl        total:2712 pass:1687 dwarn:1   dfail:0   fail:23  skip:1001 time:13731s
shard-kbl        total:2689 pass:1793 dwarn:1   dfail:0   fail:22  skip:872 time:10974s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7568/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Fix Limited Range Color Handling (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Fix Limited Range Color Handling (rev2)
URL   : https://patchwork.freedesktop.org/series/35725/
State : success

== Summary ==

Series 35725v2 drm/i915: Fix Limited Range Color Handling
https://patchwork.freedesktop.org/api/1.0/series/35725/revisions/2/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> DMESG-FAIL (fi-elk-e7500) fdo#103989 +1
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-kbl-r) fdo#104172 +1
Test prime_vgem:
        Subgroup basic-fence-flip:
                fail       -> PASS       (fi-bwr-2160) fdo#103182

fdo#103989 https://bugs.freedesktop.org/show_bug.cgi?id=103989
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#104172 https://bugs.freedesktop.org/show_bug.cgi?id=104172
fdo#103182 https://bugs.freedesktop.org/show_bug.cgi?id=103182

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:435s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:381s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:491s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:275s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:488s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:493s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:473s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:472s
fi-elk-e7500     total:224  pass:163  dwarn:14  dfail:1   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:261s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:536s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:403s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:412s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:421s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:474s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:428s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:472s
fi-kbl-7560u     total:288  pass:268  dwarn:1   dfail:0   fail:0   skip:19  time:519s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:465s
fi-kbl-r         total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:522s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:583s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:444s
fi-skl-6600u     total:288  pass:260  dwarn:1   dfail:0   fail:0   skip:27  time:523s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:550s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:506s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:494s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:446s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:546s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:409s
Blacklisted hosts:
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:592s
fi-cnl-y         total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:628s
fi-bdw-gvtdvm failed to collect. IGT log at Patchwork_7592/fi-bdw-gvtdvm/igt.log
fi-glk-dsi failed to collect. IGT log at Patchwork_7592/fi-glk-dsi/igt.log

8cb9e9870b50ddf65ebe233d38194cafdd811d03 drm-tip: 2017y-12m-28d-17h-00m-27s UTC integration manifest
9f0a777e5ff0 drm/i915: Fix Limited Range Color Handling

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7592/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Fix Limited Range Color Handling (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Fix Limited Range Color Handling (rev4)
URL   : https://patchwork.freedesktop.org/series/35725/
State : success

== Summary ==

Series 35725v4 drm/i915: Fix Limited Range Color Handling
https://patchwork.freedesktop.org/api/1.0/series/35725/revisions/4/mbox/

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (fi-snb-2520m) fdo#103713

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:418s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:422s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:372s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:483s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:283s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:481s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:485s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:464s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:452s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:563s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:180  dwarn:0   dfail:0   fail:0   skip:108 time:279s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:513s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:391s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:400s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:413s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:414s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:458s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:495s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:505s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:582s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:425s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:512s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:527s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:490s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:476s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:422s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:431s
fi-snb-2520m     total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:525s
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:394s
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:469s

9b156ad76b2e755bf527297486646cf27bbf1abd drm-tip: 2018y-01m-30d-17h-06m-58s UTC integration manifest
b61e18d62a90 drm/i915: Fix Limited Range Color Handling

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7827/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915: Fix Limited Range Color Handling (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915: Fix Limited Range Color Handling (rev4)
URL   : https://patchwork.freedesktop.org/series/35725/
State : failure

== Summary ==

Test pm_rc6_residency:
        Subgroup rc6-accuracy:
                skip       -> PASS       (shard-snb)
Test kms_vblank:
        Subgroup pipe-c-accuracy-idle:
                pass       -> FAIL       (shard-hsw)
Test gem_exec_schedule:
        Subgroup reorder-wide-vebox:
                pass       -> FAIL       (shard-apl) fdo#102848
Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-apl) fdo#100047
Test perf:
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252
Test kms_plane:
        Subgroup plane-panning-bottom-right-suspend-pipe-b-planes:
                dmesg-warn -> PASS       (shard-snb)
Test kms_flip:
        Subgroup 2x-plain-flip-fb-recreate:
                fail       -> PASS       (shard-hsw) fdo#100368
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-spr-indfb-draw-render:
                pass       -> FAIL       (shard-apl) fdo#103167

fdo#102848 https://bugs.freedesktop.org/show_bug.cgi?id=102848
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167

shard-apl        total:2838 pass:1748 dwarn:1   dfail:0   fail:24  skip:1064 time:12755s
shard-hsw        total:2838 pass:1735 dwarn:1   dfail:0   fail:11  skip:1090 time:12072s
shard-snb        total:2838 pass:1331 dwarn:1   dfail:0   fail:10  skip:1496 time:6591s
Blacklisted hosts:
shard-kbl        total:2824 pass:1867 dwarn:1   dfail:0   fail:21  skip:934 time:9561s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7827/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx