From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 08/10] drm/i915: move pipe bpp computation to pipe_config Date: Tue, 26 Mar 2013 22:32:49 +0100 Message-ID: <20130326213249.GC9021@phenom.ffwll.local> References: <1361491014-13888-1-git-send-email-daniel.vetter@ffwll.ch> <1361491014-13888-9-git-send-email-daniel.vetter@ffwll.ch> <20130326141239.64ee1d09@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f50.google.com (mail-ee0-f50.google.com [74.125.83.50]) by gabe.freedesktop.org (Postfix) with ESMTP id 7897EE5C54 for ; Tue, 26 Mar 2013 14:30:00 -0700 (PDT) Received: by mail-ee0-f50.google.com with SMTP id e53so634948eek.23 for ; Tue, 26 Mar 2013 14:29:59 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130326141239.64ee1d09@jbarnes-desktop> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jesse Barnes Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Tue, Mar 26, 2013 at 02:12:39PM -0700, Jesse Barnes wrote: > On Fri, 22 Feb 2013 00:56:52 +0100 > Daniel Vetter 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. FORCE_6BPC is just the DP encoder's way to communicate bpp requirements, so I've figured I might as well include it. Looking at the patch again I admit it's pretty big, so I'll try to smash it into pieces. Should be doable from a quick look. > 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... I've thought that the lvds dither flag in the port disappeared with the appearance of the pipe dither modes. Might be wrong though, but I think I've completely check all docs covering that range (i.e. gen3-cantiga). The only place where we have 2 dither bits is on pch ports (one on the cpu transcoder, one in the pch transcoder). But imo it doesn't make much sense to dither down/up in two steps, so we should be fine with just one flag. The only case I've come up with is a 8bpc panel which _requires_ 8bpc (the apple retina are like that iirc), and we try to feed it a 16bpp plane. Then it would make sense to shovel 6bpc over the fdi link and up-dither to 8bpc on the pch. But since that'll only happen with a desktop/all-in-one eDP config, who cares. And if your driving your expensive 8bpc edp panel at 6bpc, you're doing it wrong anyway. > 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. My self-imposed merge criterion for the bpp fixes (an entire pile of follow-up patches) is that I want to improve testdisplay to also test the different pixel depths. And it needs testing (10bpc screen is on order). But that part of the pipe_config rework is pretty orthogonal (imo the dp unconfusion part is more important to get things going), so can wait. > Otherwise this mostly looks good. I definitely like the direction. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch