On Wed, 25 Jan 2012 12:51:16 -0800, Jesse Barnes <jbarnes at virtuousgeek.org> wrote: > Yeah, looks like I got this wrong when I added the pipe bpp field. > Wonder if there's a good way to catch this sort of bug with an assert > so we don't get the order mixed up again... Ideally, we'd figure out a way to compute this only once. I think the only relevant piece of data here is what bpc the pipe should run at, and that can be computed at any time during the mode set. We'd then be able to compare the pipe bpc with the frame buffer bpp to decide when to dither in the crtc mode set code. > The big downside here is that we'll be very pessimistic about the link > bw requirements for say 16 or 8bpp modes. Right, with this patch, we'll choose link parameters capable of supporting 8bpc, even for 16bpp video modes. Note that 8bpp video modes need 8bpc links as they have colormaps with 8bpc data in them. > I see you have another patch to address some of this, but I wonder if > we have enough info to calculate the bpp at prepare time so it's set > early on for use by both the crtc and encoder code? fixup gets exactly the same info as mode set, so it should end up with exactly the same resulting bpc, which will set the link bandwidth to support 6bpc mode if that's all that is needed. One problem with the patch is that the pre-PCH mode set path doesn't use intel_choose_pipe_bpp_dither, so the condition for 6bpc mode is duplicated in i9xx_crtc_mode_set and intel_dp_mode_fixup. If crtc->fixup was called before encoder->fixup, we could have computed bpp in the crtc->fixup function and then used it in encoder->fixup. Alternatively, crtc->fixup could call into the DP code to set the link_bw, lane_count and adjusted_mode->clock values after it sets the bpp, but that seems to break the abstraction even more. -- keith.packard at intel.com -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 827 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120125/69402a60/attachment.pgp>
