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

[Mario Kleiner <mario.kleiner.de@gmail.com>]

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