On Wed, Feb 14, 2024 at 03:50:49PM +0200, Jani Nikula wrote:
> On Fri, 09 Feb 2024, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Chunk up the humenguous dpll_hw_state comparison check into per-platform
> > variants, implemented in the dpll_mgr. This is step one in allowing
> > each platform (or perhaps even PLL) type to have a custom hw state
> > structure instead of having to smash it all into one.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 78 ++++++++-------
> > drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 95 +++++++++++++++++++
> > drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 3 +
> > 3 files changed, 141 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index 1d381fa96c84..66ee6749fdae 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -4907,6 +4907,36 @@ pipe_config_mismatch(bool fastset, const struct intel_crtc *crtc,
> > va_end(args);
> > }
> >
> > +static void
> > +pipe_config_pll_mismatch(bool fastset,
> > + const struct intel_crtc *crtc,
> > + const char *name,
> > + const struct intel_dpll_hw_state *a,
> > + const struct intel_dpll_hw_state *b)
> > +{
> > + struct drm_i915_private *i915 = to_i915(crtc->base.dev);
> > +
> > + if (fastset) {
> > + if (!drm_debug_enabled(DRM_UT_KMS))
> > + return;
> > +
> > + drm_dbg_kms(&i915->drm,
> > + "[CRTC:%d:%s] fastset requirement not met in %s\n",
> > + crtc->base.base.id, crtc->base.name, name);
> > + drm_dbg_kms(&i915->drm, "expected:\n");
> > + intel_dpll_dump_hw_state(i915, a);
> > + drm_dbg_kms(&i915->drm, "found:\n");
> > + intel_dpll_dump_hw_state(i915, b);
> > + } else {
> > + drm_err(&i915->drm, "[CRTC:%d:%s] mismatch in %s buffer\n",
> > + crtc->base.base.id, crtc->base.name, name);
> > + drm_err(&i915->drm, "expected:\n");
> > + intel_dpll_dump_hw_state(i915, a);
> > + drm_err(&i915->drm, "found:\n");
> > + intel_dpll_dump_hw_state(i915, b);
> > + }
> > +}
>
> As follow-up, would be great to see this move towards drm_printer based
> approach, similar to pipe_config_dp_vsc_sdp_mismatch(). Reduces
> duplication.
Why did we convert just that single thing and not everything?
--
Ville Syrjälä
Intel
