[PATCH] drm/i915: Only dither on 6bpc panels

[PATCH] drm/i915: Only dither on 6bpc panels

[Daniel Vetter]

In

commit d328c9d78d64ca11e744fe227096990430a88477
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Apr 10 16:22:37 2015 +0200

    drm/i915: Select starting pipe bpp irrespective or the primary plane

we started to select the pipe bpp from sink capabilities and not from
the primary framebuffer - that one might change (and we don't want to
incur a modeset) and sprites might contain higher bpp content too.

Problem is that now if you have a 10bpc screen and display 24bpp rgb
primary then we select dithering, and apparently that mangles the high
8 bits even (even thought you'd expect dithering only to affect how
12bpc gets mapped into 10bpc). And that mangling upsets certain users.

Hence only enable dithering on 6bpc screens where we difinitely and
always want it.

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Reported-by: Mario Kleiner <mario.kleiner.de@gmail.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9a2f229a1c3a..128462e0a0b5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12186,7 +12186,9 @@ encoder_retry:
 		goto encoder_retry;
 	}
 
-	pipe_config->dither = pipe_config->pipe_bpp != base_bpp;
+	/* Dithering seems to not pass-through bits correctly when it should, so
+	 * only enable it on 6bpc panels. */
+	pipe_config->dither = pipe_config->pipe_bpp == 6*3;
 	DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
 		      base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
 
-- 
2.5.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915: Only dither on 6bpc panels

[Mario Kleiner]

Thanks for the quick fix! Comments below...

On 08/12/2015 11:43 AM, Daniel Vetter wrote:
> In
>
> commit d328c9d78d64ca11e744fe227096990430a88477
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Fri Apr 10 16:22:37 2015 +0200
>
>      drm/i915: Select starting pipe bpp irrespective or the primary plane
>
> we started to select the pipe bpp from sink capabilities and not from
> the primary framebuffer - that one might change (and we don't want to
> incur a modeset) and sprites might contain higher bpp content too.
>
> Problem is that now if you have a 10bpc screen and display 24bpp rgb
> primary then we select dithering, and apparently that mangles the high
> 8 bits even (even thought you'd expect dithering only to affect how
> 12bpc gets mapped into 10bpc). And that mangling upsets certain users.
>

Probably doesn't matter, but your explanation of the former problem here 
is slightly off. We also selected dithering on a 8 bpc screen displaying 
a 24bpp rgb primary, because pipe_bpp is 24 for such a typical 8 bpc 
sink, but since the commit mentioned above, base_bpp is always the 
absolute maximum supported by the hardware, e.g., 36 bpp on my Ironlake 
chip. Iow. the only way to not get dithering would have been to connect 
a deep color 12 bpc display, so pipe_bpp == 36 == base_bpp.

> Hence only enable dithering on 6bpc screens where we difinitely and
> always want it.
>

Other than that, i tested the patch on both 8 bpc output with my 
measurement equipment and on the internal laptop 6 bpc panel, and 
everything is fine now - No banding on the 6 bpc panel, no banding or 
equipment failure on the external 8 bpc output. Life is good again :)

Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>

thanks,
-mario

> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Reported-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9a2f229a1c3a..128462e0a0b5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12186,7 +12186,9 @@ encoder_retry:
>   		goto encoder_retry;
>   	}
>
> -	pipe_config->dither = pipe_config->pipe_bpp != base_bpp;
> +	/* Dithering seems to not pass-through bits correctly when it should, so
> +	 * only enable it on 6bpc panels. */
> +	pipe_config->dither = pipe_config->pipe_bpp == 6*3;
>   	DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
>   		      base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/i915: Only dither on 6bpc panels

[Jani Nikula]

On Thu, 13 Aug 2015, Mario Kleiner <mario.kleiner.de@gmail.com> wrote:
> Thanks for the quick fix! Comments below...
>
> On 08/12/2015 11:43 AM, Daniel Vetter wrote:
>> In
>>
>> commit d328c9d78d64ca11e744fe227096990430a88477
>> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Date:   Fri Apr 10 16:22:37 2015 +0200
>>
>>      drm/i915: Select starting pipe bpp irrespective or the primary plane
>>
>> we started to select the pipe bpp from sink capabilities and not from
>> the primary framebuffer - that one might change (and we don't want to
>> incur a modeset) and sprites might contain higher bpp content too.
>>
>> Problem is that now if you have a 10bpc screen and display 24bpp rgb
>> primary then we select dithering, and apparently that mangles the high
>> 8 bits even (even thought you'd expect dithering only to affect how
>> 12bpc gets mapped into 10bpc). And that mangling upsets certain users.
>>
>
> Probably doesn't matter, but your explanation of the former problem here 
> is slightly off. We also selected dithering on a 8 bpc screen displaying 
> a 24bpp rgb primary, because pipe_bpp is 24 for such a typical 8 bpc 
> sink, but since the commit mentioned above, base_bpp is always the 
> absolute maximum supported by the hardware, e.g., 36 bpp on my Ironlake 
> chip. Iow. the only way to not get dithering would have been to connect 
> a deep color 12 bpc display, so pipe_bpp == 36 == base_bpp.
>
>> Hence only enable dithering on 6bpc screens where we difinitely and
>> always want it.
>>
>
> Other than that, i tested the patch on both 8 bpc output with my 
> measurement equipment and on the internal laptop 6 bpc panel, and 
> everything is fine now - No banding on the 6 bpc panel, no banding or 
> equipment failure on the external 8 bpc output. Life is good again :)
>
> Reviewed-and-tested-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Pushed to drm-intel-fixes, thanks for the patch and review and testing.

BR,
Jani.

>
> thanks,
> -mario
>
>> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
>> Reported-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_display.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 9a2f229a1c3a..128462e0a0b5 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12186,7 +12186,9 @@ encoder_retry:
>>   		goto encoder_retry;
>>   	}
>>
>> -	pipe_config->dither = pipe_config->pipe_bpp != base_bpp;
>> +	/* Dithering seems to not pass-through bits correctly when it should, so
>> +	 * only enable it on 6bpc panels. */
>> +	pipe_config->dither = pipe_config->pipe_bpp == 6*3;
>>   	DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
>>   		      base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
>>
>>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Only dither on 6bpc panels

[shuang.he]

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 7155
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
ILK                 -1              302/302              301/302
SNB                                  315/315              315/315
IVB                                  336/336              336/336
BYT                 -3              283/283              280/283
HSW                                  378/378              378/378
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*ILK  igt@kms_flip@wf_vblank-vs-modeset-interruptible      PASS(1)      DMESG_WARN(1)
*BYT  igt@gem_partial_pwrite_pread@reads-uncached      PASS(1)      FAIL(1)
*BYT  igt@gem_persistent_relocs@forked-interruptible      PASS(1)      FAIL(1)
*BYT  igt@gem_tiled_partial_pwrite_pread@reads      PASS(1)      FAIL(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx