[Intel-gfx] [PATCH 47/66] drm/i915: Move pch fifo underrun report enabling to hsw_crt_pre_enable

[Paulo Zanoni]

2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter at ffwll.ch>:
> With this all the pch pre-enable work has been moved into the special
> hsw crt encoder functions.

For the same reasons I gave in the other patches, I'm not convinced
this is an improvement to our code. It looks like we're just breaking
the abstraction exploiting the fact that CRT is the only FDI/PCH
output on Haswell. Now the HSW code will be significantly different
from the ILK/SNB/HSW code, and I really think this can be confusing to
people reading the code. I really think we shouldn't hide things
aren't specific to CRT inside CRT code.

Since on this series we're basically disagreeing on our opinions on
which coding style is the better, I think we should ask the other
developers to give their opinion too :)

>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_crt.c     | 4 ++++
>  drivers/gpu/drm/i915/intel_display.c | 2 --
>  2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
> index 2d8f4fe1b450..d3cae57d942a 100644
> --- a/drivers/gpu/drm/i915/intel_crt.c
> +++ b/drivers/gpu/drm/i915/intel_crt.c
> @@ -275,6 +275,10 @@ static void hsw_fdi_link_train(struct drm_crtc *crtc)
>
>  static void hsw_crt_pre_enable(struct intel_encoder *encoder)
>  {
> +       struct drm_device *dev = encoder->base.dev;
> +
> +       intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
> +
>         hsw_fdi_link_train(encoder->base.crtc);
>  }
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 80b34ac31d0a..43a40594841f 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3855,8 +3855,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>         intel_crtc->active = true;
>
>         intel_set_cpu_fifo_underrun_reporting(dev, pipe, true);
> -       if (intel_crtc->config.has_pch_encoder)
> -               intel_set_pch_fifo_underrun_reporting(dev, TRANSCODER_A, true);
>
>         for_each_encoder_on_crtc(dev, crtc, encoder)
>                 if (encoder->pre_enable)
> --
> 1.8.1.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni