On Thu, Oct 11, 2018 at 12:04:51PM +0200, Maarten Lankhorst wrote:
> has_drrs is a flag we can't read out. We set it when seamless DRRS is
> enabled in pipe_config, so intel_dump_pipe_config() and
> intel_pipe_config_compare() will continue to do the right thing when
> has_drrs is set on the real state.
> 
> This removes one more dereference of crtc->config.
> While at it, fixup the comment and also read out M2_N2 for CHV, since
> we program it in the set_m_n function.
> 
> Signed-off-by: Maarten Lankhorst <[email protected]>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index bb16a9acf117..7812fab31646 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6893,8 +6893,8 @@ static void intel_cpu_transcoder_set_m_n(const struct 
> intel_crtc_state *crtc_sta
>               I915_WRITE(PIPE_LINK_M1(transcoder), m_n->link_m);
>               I915_WRITE(PIPE_LINK_N1(transcoder), m_n->link_n);
>               /* M2_N2 registers to be set only for gen < 8 (M2_N2 available
> -              * for gen < 8) and if DRRS is supported (to make sure the
> -              * registers are not unnecessarily accessed).
> +              * for gen < 8 and CHV) and if DRRS is supported (to make sure
> +              * the registers are not unnecessarily accessed).
>                */
>               if (m2_n2 && (IS_CHERRYVIEW(dev_priv) ||
>                   INTEL_GEN(dev_priv) < 8) && crtc_state->has_drrs) {

I think what I'd really like to see is splitting the m/n and m2/n2 stuff
into two distinct pieces, and when we don't use drrs we could just
set m2_n2 = m_n, and program the everything exactly the same way as
when drrs is enabled.

Also we could nuke that "m/n vs. m2/n2" enum and just pass the correct
struct to foo_set_m_n() directly.

> @@ -8747,11 +8747,10 @@ static void intel_cpu_transcoder_get_m_n(struct 
> intel_crtc *crtc,
>               m_n->tu = ((I915_READ(PIPE_DATA_M1(transcoder))
>                           & TU_SIZE_MASK) >> TU_SIZE_SHIFT) + 1;
>               /* Read M2_N2 registers only for gen < 8 (M2_N2 available for
> -              * gen < 8) and if DRRS is supported (to make sure the
> -              * registers are not unnecessarily read).
> +              * gen < 8 and CHV).
>                */
> -             if (m2_n2 && INTEL_GEN(dev_priv) < 8 &&
> -                     crtc->config->has_drrs) {
> +             if (m2_n2 && (INTEL_GEN(dev_priv) < 8 ||
> +                           IS_CHERRYVIEW(dev_priv))) {

I think this is maybe the third installment of this check. Could
perhaps use a small has_m2_n2() helper to avoid the repetition?

Either way
Reviewed-by: Ville Syrjälä <[email protected]>

>                       m2_n2->link_m = I915_READ(PIPE_LINK_M2(transcoder));
>                       m2_n2->link_n = I915_READ(PIPE_LINK_N2(transcoder));
>                       m2_n2->gmch_m = I915_READ(PIPE_DATA_M2(transcoder))
> -- 
> 2.19.0
> 
> _______________________________________________
> Intel-gfx mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to