Cc: Ville and Daniel for archeology...
On Fri, 11 Jun 2021, Nischal Varide <nischal.varide@xxxxxxxxx> wrote:
> If the panel is 12bpc then Dithering is not enabled in the Legacy
> dithering block , instead its Enabled after the C1 CC1 pipe post
> color space conversion.For a 6bpc pannel Dithering is enabled in
> Legacy block.
Currently, we only ever enable dithering for 6 bpc displays. See commit
e8fa4270536d ("drm/i915: Only dither on 6bpc panels"). This is decided
at the end of intel_modeset_pipe_config().
The big question here is if we want to expand the use of dithering. I
guess we could be able to reduce banding if we did?
> Signed-off-by: Nischal Varide <nischal.varide@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_color.c | 7 +++++++
> drivers/gpu/drm/i915/display/intel_display.c | 11 ++++++++++-
> drivers/gpu/drm/i915/i915_reg.h | 1 +
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index dab892d2251b..c7af583200c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1574,6 +1574,7 @@ static int glk_color_check(struct intel_crtc_state *crtc_state)
> static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
> {
> u32 gamma_mode = 0;
> + struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
>
> if (crtc_state->hw.degamma_lut)
> gamma_mode |= PRE_CSC_GAMMA_ENABLE;
> @@ -1588,6 +1589,12 @@ static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
> else
> gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
>
> + if (DISPLAY_VER(i915) >= 13) {
> + if (!crtc_state->dither_force_disable &&
> + (crtc_state->pipe_bpp == 36))
> + gamma_mode |= POST_CC1_DITHER_ENABLE;
> + }
This enables dithering independent of crtc_state->dither. That doesn't
seem like a good idea. I think the decision should be made at the end of
intel_modeset_pipe_config().
if (DISPLAY_VER(i915) >= 13 && crtc_state->dither &&
crtc_state->pipe_bpp == 36)
gamma_mode |= POST_CC1_DITHER_ENABLE;
Obviously, as we currently only enable dithering for 6 bpc, this would
become a nop if it looked at crtc_state->dither and pipe_bpp only.
> +
> return gamma_mode;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 362bff9beb5c..3a7feb246745 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5762,7 +5762,16 @@ static void bdw_set_pipemisc(const struct intel_crtc_state *crtc_state)
> break;
> }
>
> - if (crtc_state->dither)
> + /*
> + * If 12bpc panel then, Enables dithering after the CC1 pipe
> + * post color space conversion and not here for display_ver
> + * greater than or equal to thirteen.
> + */
> +
> + if (crtc_state->dither && (crtc_state->pipe_bpp != 36))
> + val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
> +
> + if (crtc_state->dither && (crtc_state->pipe_bpp == 36) && (DISPLAY_VER(dev_priv) < 13))
> val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
This is what you're trying to say:
/* 12 bpc dithering is done at post CSC gamma for display 13+ */
if (crtc_state->dither &&
(crtc_state->pipe_bpp != 36 || DISPLAY_VER(dev_priv) < 13))
val |= PIPEMISC_DITHER_ENABLE | PIPEMISC_DITHER_TYPE_SP;
Again, this is a nop until we decide to use dithering more.
>
> if (crtc_state->output_format == INTEL_OUTPUT_FORMAT_YCBCR420 ||
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e915ec034c98..33dba13fa94d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7743,6 +7743,7 @@ enum {
> #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
> #define PRE_CSC_GAMMA_ENABLE (1 << 31)
> #define POST_CSC_GAMMA_ENABLE (1 << 30)
> +#define POST_CC1_DITHER_ENABLE (1 << 26)
> #define GAMMA_MODE_MODE_MASK (3 << 0)
> #define GAMMA_MODE_MODE_8BIT (0 << 0)
> #define GAMMA_MODE_MODE_10BIT (1 << 0)
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
