| Message ID | 20220128103757.22461-13-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: M/N cleanup | expand |
On Fri, 28 Jan 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > M2/N2 values are present for all ilk-ivb,vlv,chv (and hsw edp). > Make the code reflect that. Nitpick, it's not called intel_cpu_transcoder_has_m2_n2() until in the next patch. Side note, I've also been looking at this bit in intel_drrs_set_state(): if (DISPLAY_VER(dev_priv) >= 8 && !IS_CHERRYVIEW(dev_priv)) intel_drrs_set_refresh_rate_m_n(crtc_state, refresh_type); else if (DISPLAY_VER(dev_priv) > 6) intel_drrs_set_refresh_rate_pipeconf(crtc_state, refresh_type); and wondering if that should be deduplicated with the transcoder_has_m2_n2() somehow. This is all a bit confusing with the slightly different conditions. BR, Jani. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 6 +----- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 1e97279ba268..67c7bbbe5c88 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3149,11 +3149,7 @@ static bool transcoder_has_m2_n2(struct drm_i915_private *dev_priv, > if (IS_HASWELL(dev_priv)) > return transcoder == TRANSCODER_EDP; > > - /* > - * Strictly speaking some registers are available before > - * gen7, but we only support DRRS on gen7+ > - */ > - return DISPLAY_VER(dev_priv) == 7 || IS_CHERRYVIEW(dev_priv); > + return IS_DISPLAY_VER(dev_priv, 5, 7) || IS_CHERRYVIEW(dev_priv); > } > > void intel_cpu_transcoder_set_m1_n1(struct intel_crtc *crtc,
On Mon, Jan 31, 2022 at 05:05:53PM +0200, Jani Nikula wrote: > On Fri, 28 Jan 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > M2/N2 values are present for all ilk-ivb,vlv,chv (and hsw edp). > > Make the code reflect that. > > Nitpick, it's not called intel_cpu_transcoder_has_m2_n2() until in the > next patch. > > Side note, I've also been looking at this bit in intel_drrs_set_state(): > > if (DISPLAY_VER(dev_priv) >= 8 && !IS_CHERRYVIEW(dev_priv)) > intel_drrs_set_refresh_rate_m_n(crtc_state, refresh_type); > else if (DISPLAY_VER(dev_priv) > 6) > intel_drrs_set_refresh_rate_pipeconf(crtc_state, refresh_type); > > and wondering if that should be deduplicated with the > transcoder_has_m2_n2() somehow. This is all a bit confusing with the > slightly different conditions. Yeah, I have a patch to use intel_cpu_transcoder_has_m2_n2() for this already on my drrs branch. It just didn't make the cut for this series for some arbitrary reason. The other place we could perhaps use intel_cpu_transcoder_has_m2_n2() is the PIPE_CONF_CHECK_ALT vs. checking both m_n and m2_n2. But I don't really want the logic there to depend on the states it's trying to compare, so I think a naked platform check there is better.
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 1e97279ba268..67c7bbbe5c88 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3149,11 +3149,7 @@ static bool transcoder_has_m2_n2(struct drm_i915_private *dev_priv, if (IS_HASWELL(dev_priv)) return transcoder == TRANSCODER_EDP; - /* - * Strictly speaking some registers are available before - * gen7, but we only support DRRS on gen7+ - */ - return DISPLAY_VER(dev_priv) == 7 || IS_CHERRYVIEW(dev_priv); + return IS_DISPLAY_VER(dev_priv, 5, 7) || IS_CHERRYVIEW(dev_priv); } void intel_cpu_transcoder_set_m1_n1(struct intel_crtc *crtc,