From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH] drm/i915: use pipe_config for lvds dithering
Date: Thu, 25 Apr 2013 19:09:43 +0300 [thread overview]
Message-ID: <20130425160943.GH4469@intel.com> (raw)
In-Reply-To: <1366905284-25232-1-git-send-email-daniel.vetter@ffwll.ch>
On Thu, Apr 25, 2013 at 05:54:44PM +0200, Daniel Vetter wrote:
> Up to now we've relied on the bios to get this right for us. Let's try
> out whether our code has improved a bit, since we should dither
> always when the output bpp doesn't match the plane bpp.
> - gen5+ should be fine, since we only use the bios hint as an upgrade.
> - gen4 changes, since here dithering is still controlled in the lvds
> register.
> - gen2/3 has implicit dithering depeding upon whether you use 2 or 3
> lvds pairs (which makes sense, since it only supports 8bpc pipe
> outpu configurations).
> - hsw doesn't support lvds.
>
> v2: Remove redudant dither setting.
>
> v3: Completly drop reliance on dev_priv->lvds_dither.
>
> v4: Enable dithering on gen2/3 only when we have a 18bpp panel, since
> up-dithering to a 24bpp panel is not supported by the hw. Spotted by
> Ville.
>
> v5: Also only enable lvds port dithering on gen4 for 18bpp modes. In
> practice this only excludes dithering a 10bpc plane down for a 24bpp
> lvds panel. Not something we truly care about. Again noticed by Ville.
>
> v6: Actually git add.
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 25 +++++++------------------
> drivers/gpu/drm/i915/intel_drv.h | 5 +++++
> drivers/gpu/drm/i915/intel_lvds.c | 15 ++++++++++-----
> 3 files changed, 22 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 16106fe..f26a867 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5156,8 +5156,7 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
> }
>
> static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> - struct drm_display_mode *adjusted_mode,
> - bool dither)
> + struct drm_display_mode *adjusted_mode)
> {
> struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -5186,7 +5185,7 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> }
>
> val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
> - if (dither)
> + if (intel_crtc->config.dither)
> val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>
> val &= ~PIPECONF_INTERLACE_MASK;
> @@ -5269,8 +5268,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
> }
>
> static void haswell_set_pipeconf(struct drm_crtc *crtc,
> - struct drm_display_mode *adjusted_mode,
> - bool dither)
> + struct drm_display_mode *adjusted_mode)
> {
> struct drm_i915_private *dev_priv = crtc->dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> @@ -5280,7 +5278,7 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc,
> val = I915_READ(PIPECONF(cpu_transcoder));
>
> val &= ~(PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_MASK);
> - if (dither)
> + if (intel_crtc->config.dither)
> val |= (PIPECONF_DITHER_EN | PIPECONF_DITHER_TYPE_SP);
>
> val &= ~PIPECONF_INTERLACE_MASK_HSW;
> @@ -5641,7 +5639,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> bool is_lvds = false;
> struct intel_encoder *encoder;
> int ret;
> - bool dither, fdi_config_ok;
> + bool fdi_config_ok;
>
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> switch (encoder->type) {
> @@ -5676,11 +5674,6 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> /* Ensure that the cursor is valid for the new mode before changing... */
> intel_crtc_update_cursor(crtc, true);
>
> - /* determine panel color depth */
> - dither = intel_crtc->config.dither;
> - if (is_lvds && dev_priv->lvds_dither)
> - dither = true;
> -
> DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe_name(pipe));
> drm_mode_debug_printmodeline(mode);
>
> @@ -5747,7 +5740,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
>
> fdi_config_ok = ironlake_check_fdi_lanes(intel_crtc);
>
> - ironlake_set_pipeconf(crtc, adjusted_mode, dither);
> + ironlake_set_pipeconf(crtc, adjusted_mode);
>
> /* Set up the display plane register */
> I915_WRITE(DSPCNTR(plane), DISPPLANE_GAMMA_ENABLE);
> @@ -5824,7 +5817,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> bool is_cpu_edp = false;
> struct intel_encoder *encoder;
> int ret;
> - bool dither;
>
> for_each_encoder_on_crtc(dev, crtc, encoder) {
> switch (encoder->type) {
> @@ -5860,9 +5852,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> /* Ensure that the cursor is valid for the new mode before changing... */
> intel_crtc_update_cursor(crtc, true);
>
> - /* determine panel color depth */
> - dither = intel_crtc->config.dither;
> -
> DRM_DEBUG_KMS("Mode for pipe %c:\n", pipe_name(pipe));
> drm_mode_debug_printmodeline(mode);
>
> @@ -5876,7 +5865,7 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> if (intel_crtc->config.has_pch_encoder)
> ironlake_fdi_set_m_n(crtc);
>
> - haswell_set_pipeconf(crtc, adjusted_mode, dither);
> + haswell_set_pipeconf(crtc, adjusted_mode);
>
> intel_set_pipe_csc(crtc);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 66922f8..723cac9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -213,6 +213,11 @@ struct intel_crtc_config {
> /* DP has a bunch of special case unfortunately, so mark the pipe
> * accordingly. */
> bool has_dp_encoder;
> +
> + /*
> + * Enable dithering, used when the selected pipe bpp doesn't match the
> + * plane bpp.
> + */
> bool dither;
>
> /* Controls for the clock computation, to override various stages. */
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 563f505..8408545 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -136,7 +136,10 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
> * special lvds dither control bit on pch-split platforms, dithering is
> * only controlled through the PIPECONF reg. */
> if (INTEL_INFO(dev)->gen == 4) {
> - if (dev_priv->lvds_dither)
> + /* Bspec wording suggests that LVDS port dithering only exists
> + * for 18bpp panels. */
> + if (intel_crtc->config.dither &&
> + intel_crtc->config.pipe_bpp == 18)
> temp |= LVDS_ENABLE_DITHER;
> else
> temp &= ~LVDS_ENABLE_DITHER;
> @@ -335,7 +338,13 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
> DRM_DEBUG_KMS("forcing display bpp (was %d) to LVDS (%d)\n",
> pipe_config->pipe_bpp, lvds_bpp);
> pipe_config->pipe_bpp = lvds_bpp;
> +
> + /* Make sure pre-965 set dither correctly for 18bpp panels. */
> + if (INTEL_INFO(dev)->gen < 4 && lvds_bpp == 18)
> + pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> +
> }
> +
> /*
> * We have timings from the BIOS for the panel, put them in
> * to the adjusted mode. The CRTC will be set up for this mode,
> @@ -470,10 +479,6 @@ out:
> pfit_pgm_ratios = 0;
> }
>
> - /* Make sure pre-965 set dither correctly */
> - if (INTEL_INFO(dev)->gen < 4 && dev_priv->lvds_dither)
> - pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> -
> if (pfit_control != lvds_encoder->pfit_control ||
> pfit_pgm_ratios != lvds_encoder->pfit_pgm_ratios) {
> lvds_encoder->pfit_control = pfit_control;
> --
> 1.7.11.7
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2013-04-25 16:11 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-19 9:14 [PATCH 0/7] dp dpll pipe_config conversion + random stuff Daniel Vetter
2013-04-19 9:14 ` [PATCH 1/7] drm/i915: consolidate pch pll computations a bit Daniel Vetter
2013-04-25 10:58 ` Ville Syrjälä
2013-04-19 9:14 ` [PATCH 2/7] drm/i915: shovel compute clock into crtc->config.dpll on ilk Daniel Vetter
2013-04-19 10:14 ` Ville Syrjälä
2013-04-19 11:36 ` [RFC][PATCH] drm/i915: Make struct dpll == intel_clock_t ville.syrjala
2013-04-20 14:50 ` Daniel Vetter
2013-04-20 15:19 ` [PATCH] drm/i915: shovel compute clock into crtc->config.dpll on ilk Daniel Vetter
2013-04-22 11:13 ` Ville Syrjälä
2013-04-22 15:12 ` Daniel Vetter
2013-04-25 11:00 ` Ville Syrjälä
2013-04-19 9:14 ` [PATCH 3/7] drm/i915: move dp clock computations to encoder->compute_config Daniel Vetter
2013-04-25 11:34 ` Ville Syrjälä
2013-04-25 12:04 ` Daniel Vetter
2013-04-25 12:21 ` Ville Syrjälä
2013-04-19 9:14 ` [PATCH 4/7] drm/i915: use pipe_config for lvds dithering Daniel Vetter
2013-04-25 11:57 ` Ville Syrjälä
2013-04-25 12:24 ` Daniel Vetter
2013-04-25 12:42 ` Ville Syrjälä
2013-04-25 13:16 ` Daniel Vetter
2013-04-25 13:20 ` [PATCH] " Daniel Vetter
2013-04-25 15:08 ` Ville Syrjälä
2013-04-25 15:54 ` Daniel Vetter
2013-04-25 15:54 ` Daniel Vetter
2013-04-25 16:09 ` Ville Syrjälä [this message]
2013-04-19 9:14 ` [PATCH 5/7] drm/i915: don't force matching p1 for g4x/ilk+ reduced pll settings Daniel Vetter
2013-04-19 14:53 ` Sean Paul
2013-04-19 9:14 ` [PATCH 6/7] drm/i915: remove redundant has_pch_encoder check Daniel Vetter
2013-04-25 11:59 ` Ville Syrjälä
2013-04-19 9:14 ` [PATCH 7/7] drm/i915: simplify config->pixel_multiplier handling Daniel Vetter
2013-04-25 12:08 ` Ville Syrjälä
2013-04-25 12:27 ` Daniel Vetter
2013-04-25 19:22 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130425160943.GH4469@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.