Re: [PATCH] drm/i915: implement high-bpc + pipeconf-dither support for g4x/vlv

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Apr 19, 2013 at 08:17:10PM +0200, Daniel Vetter wrote:
> The current code is rather ... ugly. The only thing it managed to pull
> off is getting 6bpc on DP working on g4x. Then someone added another
> custom hack for 6bpc eDP on vlv. Fix up this entire mess by properly
> implementing the PIPECONF-based dither/bpc controls on g4x/vlv.
> 
> Note that compared to pch based platforms g4x/vlv don't support 12bpc
> modes. g4x is already caught, extend the check for vlv.
> 
> The other fixup is to restrict the lvds-specific dithering to early
> gen4 devices - g4x should use the pipeconf dither controls. Note that
> on gen2/3 the dither control is in the panel fitter even.
> 
> v2: Don't enable dithering when the pipe is in 10 bpc mode. Quoting
> from Bspec "PIPEACONF - Pipe A Configuration Register, bit 4":
> 
> "Programming note: Dithering should only be enabled for 8 bpc or 6
> bpc."
> 
> v3: Actually drop the old ugly dither code.
> 
> v4: Explain in a short comment why g4x/vlv shouldn't dither for 30 bpp
> pipes (Jesse).
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 35 +++++++++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_lvds.c    |  2 +-
>  2 files changed, 22 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8c36376..7f1ab8b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4605,22 +4605,29 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
>  			pipeconf &= ~PIPECONF_DOUBLE_WIDE;
>  	}
>  
> -	/* default to 8bpc */
> -	pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
> -	if (intel_crtc->config.has_dp_encoder) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -				    PIPECONF_DITHER_EN |
> +	/* only g4x and later have fancy bpc/dither controls */
> +	if (IS_G4X(dev) || IS_VALLEYVIEW(dev)) {
> +		pipeconf &= ~(PIPECONF_BPC_MASK | PIPECONF_DITHER_EN);
                              ^^^^^^^^^^^^^^^^^

PIPECONF_DITHER_TYPE_MASK

The g4x docs are a bit confusing though. They seem to indicate the the
PIPECONF dither controls only affect DP.

> +
> +		/* Bspec claims that we can't use dithering for 30bpp pipes. */
> +		if (intel_crtc->config.dither && intel_crtc->config.pipe_bpp != 30)
> +			pipeconf |= PIPECONF_DITHER_EN |
>  				    PIPECONF_DITHER_TYPE_SP;
> -		}
> -	}
>  
> -	if (IS_VALLEYVIEW(dev) && intel_pipe_has_type(&intel_crtc->base,
> -						      INTEL_OUTPUT_EDP)) {
> -		if (intel_crtc->config.dither) {
> -			pipeconf |= PIPECONF_6BPC |
> -					PIPECONF_ENABLE |
> -					I965_PIPECONF_ACTIVE;
> +		pipeconf &= ~PIPECONF_BPC_MASK;
> +		switch (intel_crtc->config.pipe_bpp) {
> +		case 18:
> +			pipeconf |= PIPECONF_6BPC;
> +			break;
> +		case 24:
> +			pipeconf |= PIPECONF_8BPC;
> +			break;
> +		case 30:
> +			pipeconf |= PIPECONF_10BPC;
> +			break;
> +		default:
> +			/* Case prevented by intel_choose_pipe_bpp_dither. */
> +			BUG();
>  		}
>  	}
>  
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 58a98ff..094f3c5 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -135,7 +135,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
>  	/* Set the dithering flag on LVDS as needed, note that there is no
>  	 * special lvds dither control bit on pch-split platforms, dithering is
>  	 * only controlled through the PIPECONF reg. */
> -	if (INTEL_INFO(dev)->gen == 4) {
> +	if (INTEL_INFO(dev)->gen == 4 && !IS_G4X(dev)) {
>  		if (intel_crtc->config.dither)
>  			temp |= LVDS_ENABLE_DITHER;
>  		else
> -- 
> 1.7.11.7
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC