On Tue, Mar 26, 2013 at 02:12:39PM -0700, Jesse Barnes wrote: > On Fri, 22 Feb 2013 00:56:52 +0100 > Daniel Vetter <daniel.vetter at 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. 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
