Op 11-10-18 om 13:56 schreef Ville Syrjälä: > 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]> Hm looks like this is causing a unclaimed register read in debugfs. Could we limit reads/writes to eDP trasncoder? >> 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
_______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
