On Tue, Oct 23, 2012 at 06:29:58PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
>
> We need to check if any of the pipes is using TRANSCODER_EDP.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
One thing that irks me is that you add new state readout code here, but
don't call the same code in the modeset_check_state code. Now the design
behind all these ->get_hw_state functions is very much that we use the
exact same code to check the modeset state as we do for fastboot. For
otherwise we simply won't get enough testcoverage for the fastboot code -
but if we also use it to check all our own state, we should be able to
take over at least all the crazy configs our driver leaves behind.
I'm ok with merging this as-is, but I'd like you to fix this up in a
follow-up series. As a rule of thumb the exact same hw state readout code
should be used in setup_hw_state (for fastboot) as for the modeset state
checks. Might be that we need to add a get_crtc_hw_state function and a
intel_crtc_hw_state struct which contains all the interesting bits (which
transcoder, pch resources, eventually modes and shared plls, ...) that we
could either store in the intel_crtc (for setup_hw_state) or compare with
the stored state in intel_crtc.
Also, modeset_check_state should grow cross checks imo to ensure that we
don't wire up these cpu transcoders in a stupid way (or leave an unused
transcoder enabled).
Cheers, Daniel
> ---
> drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d63da7b..2f546e8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8690,6 +8690,31 @@ void intel_modeset_setup_hw_state(struct drm_device *dev)
> struct intel_encoder *encoder;
> struct intel_connector *connector;
>
> + if (IS_HASWELL(dev)) {
> + tmp = I915_READ(DDI_FUNC_CTL(TRANSCODER_EDP));
> +
> + if (tmp & TRANS_DDI_FUNC_ENABLE) {
> + switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
> + case TRANS_DDI_EDP_INPUT_A_ON:
> + case TRANS_DDI_EDP_INPUT_A_ONOFF:
> + pipe = PIPE_A;
> + break;
> + case TRANS_DDI_EDP_INPUT_B_ONOFF:
> + pipe = PIPE_B;
> + break;
> + case TRANS_DDI_EDP_INPUT_C_ONOFF:
> + pipe = PIPE_C;
> + break;
> + }
> +
> + crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> + crtc->cpu_transcoder = TRANSCODER_EDP;
> +
> + DRM_DEBUG_KMS("Pipe %c using transcoder EDP\n",
> + pipe_name(pipe));
> + }
> + }
> +
> for_each_pipe(pipe) {
> crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>
> --
> 1.7.11.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
