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. 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
