[Intel-gfx] [PATCH 4/6] drm/i915: Cleanup if the EDP transcoder has a bobug input value

[Paulo Zanoni]

Hi

2013/3/7 Damien Lespiau <damien.lespiau at intel.com>:
> In the case where the hardware has been wrongly programmed and the EDP
> TRANS_DDI_FUNC_CTL register has a bogus value in its EDP Input field, we
> were using the pipe variable uninitialized.
>
> In this case, shutdown the transcoder. It will be programmed correctly
> the next time we try to enabled eDP.
>
> Signed-off-by: Damien Lespiau <damien.lespiau at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 502cb28..875baf1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9107,6 +9107,13 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>                         case TRANS_DDI_EDP_INPUT_C_ONOFF:
>                                 pipe = PIPE_C;
>                                 break;
> +                       default:
> +                               /* A bogus value has been programmed, disable
> +                                * the transcoder */
> +                               WARN(1, "Bogus eDP source %08x\n", tmp);
> +                               intel_ddi_disable_transcoder_func(dev_priv,
> +                                               TRANSCODER_EDP);

I imagine this patch is a result of checking some code analyzer and
not real hardware, so we didn't really test this code on real
hardware, right? The WARN is definitely a must-have, but I kinda fear
this "disable" code since we can easily freeze the machine if we get
the mode set sequence wrong. Anyway, if the hardware is programmed in
an inconsistent way I don't really think there's a "right way" to
disable it, so:

Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

But we need to watch out in case users start reporting about machines
that freeze during boot.

> +                               goto setup_pipes;
>                         }
>
>                         crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
> @@ -9117,6 +9124,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev,
>                 }
>         }
>
> +setup_pipes:
>         for_each_pipe(pipe) {
>                 crtc = to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
>
> --
> 1.7.7.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni