Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 08/10] drm/i915: move pipe bpp computation to pipe_config
Date: Tue, 26 Mar 2013 14:12:39 -0700	[thread overview]
Message-ID: <20130326141239.64ee1d09@jbarnes-desktop> (raw)
In-Reply-To: <1361491014-13888-9-git-send-email-daniel.vetter@ffwll.ch>

On Fri, 22 Feb 2013 00:56:52 +0100
Daniel Vetter <daniel.vetter@ffwll.ch> wrote:

> The procedure has now 3 steps:
> 
> 1. Compute the bpp that the plane will output, this is done in
>    pipe_config_set_bpp and stored into pipe_config->pipe_bpp. Also,
>    this function clamps the pipe_bpp to whatever limit the EDID of any
>    connected output specifies.
> 2. Adjust the pipe_bpp in the encoder and crtc functions, according to
>    whatever constraints there are.
> 3. Decide whether to use dither by comparing the stored plane bpp with
>    computed pipe_bpp.
> 
> There are a few slight functional changes in this patch:
> - LVDS connector are now also going through the EDID clamping. But in
>   a 2nd change we now unconditionally force the lvds bpc value - this
>   shouldn't matter in reality when the panel setup is consistent, but
>   better safe than sorry.
> - HDMI now forces the pipe_bpp to the selected value - I think that's
>   what we actually want, since otherwise at least the pixelclock
>   computations are wrong (I'm not sure whether the port would accept
>   e.g. 10 bpc when in 12bpc mode). Contrary to the old code, we pick
>   the next higher bpc value, since otherwise there's no way to make
>   use of the 12 bpc mode (since the next patch will remove the 12bpc
>   plane format, it doesn't exist).
> 

This is a big patch; maybe there's a reasonable way to split it up?
Maybe by leaving the _FORCE_6BPC flag in then removing in a second
patch?  May as well squash in 9/10 while you're at it, since it mainly
affects the new function.

I wonder if we want a pipe_dither and a connector_dither flag too?
On Cantiga, we can dither either at LVDS or in the pipe for example.
Not sure which is better...

And as usual, we need better tests for this stuff.  The color ramps in
testdisplay should show artifacts if we get the dithering wrong, but we
could probably improve it.

Otherwise this mostly looks good.  I definitely like the direction.

-- 
Jesse Barnes, Intel Open Source Technology Center

  reply	other threads:[~2013-03-26 21:12 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-21 23:56 [PATCH 00/10] basic infrastructure for pipe_config Daniel Vetter
2013-02-21 23:56 ` [PATCH 01/10] drm/i915: introduce struct intel_crtc_config Daniel Vetter
2013-02-22 10:23   ` Ville Syrjälä
2013-02-22 11:05   ` [PATCH] " Daniel Vetter
2013-02-26 17:09     ` Paulo Zanoni
2013-02-21 23:56 ` [PATCH 02/10] drm/i915: compute pipe_config earlier Daniel Vetter
2013-02-26 17:11   ` Paulo Zanoni
2013-02-21 23:56 ` [PATCH 03/10] drm/i915: add pipe_config->timings_set Daniel Vetter
2013-02-22 13:51   ` Ville Syrjälä
2013-03-03 18:01     ` Daniel Vetter
2013-03-04 14:18       ` Ville Syrjälä
2013-02-26 17:23   ` Paulo Zanoni
2013-03-03 17:58     ` Daniel Vetter
2013-02-21 23:56 ` [PATCH 04/10] drm/i915: add pipe_config->pixel_multiplier Daniel Vetter
2013-02-26 17:27   ` Paulo Zanoni
2013-03-03 18:05     ` Daniel Vetter
2013-02-21 23:56 ` [PATCH 05/10] drm/i915: drop helper vtable for sdvo encoder Daniel Vetter
2013-02-21 23:56 ` [PATCH 06/10] drm/i915: add pipe_config->has_pch_encoder Daniel Vetter
2013-02-26 17:39   ` Paulo Zanoni
2013-03-03 18:13     ` Daniel Vetter
2013-03-03 18:16       ` [PATCH] " Daniel Vetter
2013-03-04 22:24       ` Daniel Vetter
2013-02-21 23:56 ` [PATCH 07/10] drm/i915: add pipe_config->limited_color_range Daniel Vetter
2013-02-26 17:46   ` Paulo Zanoni
2013-02-21 23:56 ` [PATCH 08/10] drm/i915: move pipe bpp computation to pipe_config Daniel Vetter
2013-03-26 21:12   ` Jesse Barnes [this message]
2013-03-26 21:32     ` Daniel Vetter
2013-02-21 23:56 ` [PATCH 09/10] drm/i915: clean up plane bpp confusion Daniel Vetter
2013-03-26 21:04   ` Jesse Barnes
2013-02-21 23:56 ` [PATCH 10/10] drm/i915: clean up pipe " Daniel Vetter
2013-03-26 21:06   ` Jesse Barnes

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=20130326141239.64ee1d09@jbarnes-desktop \
    --to=jbarnes@virtuousgeek.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox