Re: [Intel-gfx] [PATCH 09/13] drm/i915: precompute pipe bpp before touching the hw
On Wed, 27 Mar 2013 00:44:58 +0100 Daniel Vetter daniel.vet...@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). Both of these changes are due to the removal of the pipe_bpp = min(display_bpp, plane_bpp); statement. Another slight change is the reworking of the dp bpc code: - For the mode_valid callback it's sufficient to only check whether the mode would fit at the lowest bpc. - The bandwidth computation code is a bit restructured: It now walks all available bpp values in an outer loop and the codeblock that computes derived values (once a good configuration is found) has been moved out of the for loop maze. This is prep work to allow us to successively fall back on bpc values, and also correctly support bpc values != 8 or 6. v2: Rebased on top of Paulo Zanoni's little refactoring to use more drm dp helper functions. v3: Rebased on top of Jani's eDP bpp fix and Ville's limited color range work. v4: Remove the INTEL_MODE_DP_FORCE_6BPC #define, no longer needed. v5: Remove intel_crtc-bpp, too, and fix up the 12bpc check in the hdmi code. Also fixup the bpp check in intel_dp.c, it'll get reworked in a later patch though again. v6: Fix spelling in a comment. v7: Debug output improvements for the bpp computation. v8: Fixup 6bpc lvds check - dual-link and 8bpc mode are different things! v9: Reinstate the fix to properly ignore the firmware edp bpp ... this was lost in a rebase. v10: Both g4x and vlv lack 12bpc pipes, so don't enforce that we have that. Still unsure whether this is the way to go, but at least 6bpc for a 8bpc hdmi output seems to work. v11: And g4x/vlv also lack 12bpc hdmi support, so only support high depth on DP. Adjust the code. v12: Rebased. v13: Split out the introduction of pipe_config-dither|pipe_bpp, as requested from Jesse Barnes. v14: Split out the special 6BPC handling for DP, as requested by Jesse Barnes. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_ddi.c | 7 +- drivers/gpu/drm/i915/intel_display.c | 224 --- drivers/gpu/drm/i915/intel_hdmi.c| 13 ++ drivers/gpu/drm/i915/intel_lvds.c| 12 ++ 4 files changed, 100 insertions(+), 156 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 3d09df0..6c6b012 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -945,9 +945,7 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc) temp |= TRANS_MSA_12_BPC; break; default: - temp |= TRANS_MSA_8_BPC; - WARN(1, %d bpp unsupported by DDI function\n, - intel_crtc-config.pipe_bpp); + BUG(); } I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp); } @@ -983,8 +981,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc) temp |= TRANS_DDI_BPC_12; break; default: - WARN(1, %d bpp unsupported by transcoder DDI function\n, - intel_crtc-config.pipe_bpp); + BUG(); } if (crtc-mode.flags DRM_MODE_FLAG_PVSYNC) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b495629..6a60bf2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4059,142 +4059,6 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) !(dev_priv-quirks QUIRK_LVDS_SSC_DISABLE); } -/** - * intel_choose_pipe_bpp_dither - figure out what color depth the pipe should send - *
Re: [Intel-gfx] [PATCH 09/13] drm/i915: precompute pipe bpp before touching the hw
On Wed, Mar 27, 2013 at 6:24 PM, Jesse Barnes jbar...@virtuousgeek.org wrote: This looks good and seems to cover the bugs we've had here before. My only concern is the one I mentioned before: on older chipsets we could dither between plane, pipe, *and* port. Nowadays the pipe always does it. So in the old LVDS case it would be cool if someone could test the difference. The LVDS port may do a better job on 6bpc panels than the pipe... I've considered this again, and it should fit neatly into the existing framework. If we want to use the dither on the lvds port on those platforms, but keep dithering on the pipe for e.g. DP we could switch lvds_compute_config to not clamp the bpp and then just enable the port dithering if config.pipe_bpp != 18. There's some further patches which unify this a bit, I guess we can discuss this a bit more once I resend them. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 09/13] drm/i915: precompute pipe bpp before touching the hw
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). Both of these changes are due to the removal of the pipe_bpp = min(display_bpp, plane_bpp); statement. Another slight change is the reworking of the dp bpc code: - For the mode_valid callback it's sufficient to only check whether the mode would fit at the lowest bpc. - The bandwidth computation code is a bit restructured: It now walks all available bpp values in an outer loop and the codeblock that computes derived values (once a good configuration is found) has been moved out of the for loop maze. This is prep work to allow us to successively fall back on bpc values, and also correctly support bpc values != 8 or 6. v2: Rebased on top of Paulo Zanoni's little refactoring to use more drm dp helper functions. v3: Rebased on top of Jani's eDP bpp fix and Ville's limited color range work. v4: Remove the INTEL_MODE_DP_FORCE_6BPC #define, no longer needed. v5: Remove intel_crtc-bpp, too, and fix up the 12bpc check in the hdmi code. Also fixup the bpp check in intel_dp.c, it'll get reworked in a later patch though again. v6: Fix spelling in a comment. v7: Debug output improvements for the bpp computation. v8: Fixup 6bpc lvds check - dual-link and 8bpc mode are different things! v9: Reinstate the fix to properly ignore the firmware edp bpp ... this was lost in a rebase. v10: Both g4x and vlv lack 12bpc pipes, so don't enforce that we have that. Still unsure whether this is the way to go, but at least 6bpc for a 8bpc hdmi output seems to work. v11: And g4x/vlv also lack 12bpc hdmi support, so only support high depth on DP. Adjust the code. v12: Rebased. v13: Split out the introduction of pipe_config-dither|pipe_bpp, as requested from Jesse Barnes. v14: Split out the special 6BPC handling for DP, as requested by Jesse Barnes. Signed-off-by: Daniel Vetter daniel.vet...@ffwll.ch --- drivers/gpu/drm/i915/intel_ddi.c | 7 +- drivers/gpu/drm/i915/intel_display.c | 224 --- drivers/gpu/drm/i915/intel_hdmi.c| 13 ++ drivers/gpu/drm/i915/intel_lvds.c| 12 ++ 4 files changed, 100 insertions(+), 156 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 3d09df0..6c6b012 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -945,9 +945,7 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc) temp |= TRANS_MSA_12_BPC; break; default: - temp |= TRANS_MSA_8_BPC; - WARN(1, %d bpp unsupported by DDI function\n, -intel_crtc-config.pipe_bpp); + BUG(); } I915_WRITE(TRANS_MSA_MISC(cpu_transcoder), temp); } @@ -983,8 +981,7 @@ void intel_ddi_enable_transcoder_func(struct drm_crtc *crtc) temp |= TRANS_DDI_BPC_12; break; default: - WARN(1, %d bpp unsupported by transcoder DDI function\n, -intel_crtc-config.pipe_bpp); + BUG(); } if (crtc-mode.flags DRM_MODE_FLAG_PVSYNC) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b495629..6a60bf2 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4059,142 +4059,6 @@ static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) !(dev_priv-quirks QUIRK_LVDS_SSC_DISABLE); } -/** - * intel_choose_pipe_bpp_dither - figure out what color depth the pipe should send - * @crtc: CRTC structure - * @mode: requested mode - * - * A pipe may be connected to one or more outputs. Based on the depth of the - * attached framebuffer, choose a good
