> -----Original Message----- > From: Jani Nikula [mailto:jani.nikula@xxxxxxxxxxxxxxx] > Sent: Thursday, December 22, 2016 6:48 PM > To: Wang, Elaine <elaine.wang@xxxxxxxxx>; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; > Wang, Elaine <elaine.wang@xxxxxxxxx> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>; Joonas Lahtinen > <joonas.lahtinen@xxxxxxxxxxxxxxx> > Subject: Re: [PATCH v2 2/2] drm/i915: Check num_pipes before initializing or > calling display hooks > > On Mon, 19 Dec 2016, Wang Elaine <elaine.wang@xxxxxxxxx> wrote: > > From: Elaine Wang <elaine.wang@xxxxxxxxx> > > > > when num_pipes is zero, it indicates display doesn't exist, so there > > is no need to initialize display hooks. And to avoid calling these > > uninitialized display hooks, respect num_pipes at the beginning of > > intel_modeset_init_hw and intel_init_clock_gating. > > > > intel_init_pm() calls FBC init function and then initializes water > > mark hooks. Both aren't needed when display doesn't exist. Add > > checking num_pipes at the beginning of intel_init_pm(). > > > > v2: Move one check from caller to callee for consistency. > > > > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > > Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> > > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > > Signed-off-by: Elaine Wang <elaine.wang@xxxxxxxxx> > > --- > > drivers/gpu/drm/i915/intel_display.c | 6 ++++++ > > drivers/gpu/drm/i915/intel_pm.c | 10 +++++++++- > > 2 files changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c > > b/drivers/gpu/drm/i915/intel_display.c > > index 9cc5dbf..e079ea7 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -16030,6 +16030,9 @@ static void intel_atomic_state_free(struct > drm_atomic_state *state) > > */ > > void intel_init_display_hooks(struct drm_i915_private *dev_priv) { > > + if (INTEL_INFO(dev_priv)->num_pipes == 0) > > + return; > > + > > if (INTEL_INFO(dev_priv)->gen >= 9) { > > dev_priv->display.get_pipe_config = > haswell_get_pipe_config; > > dev_priv->display.get_initial_plane_config = @@ -16412,6 > +16415,9 > > @@ void intel_modeset_init_hw(struct drm_device *dev) { > > struct drm_i915_private *dev_priv = to_i915(dev); > > > > + if (INTEL_INFO(dev_priv)->num_pipes == 0) > > + return; > > + > > intel_update_cdclk(dev_priv); > > > > dev_priv->atomic_cdclk_freq = dev_priv->cdclk_freq; diff --git > > a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > > index d0834b3..6438ada 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -7619,7 +7619,8 @@ static void i830_init_clock_gating(struct > > drm_i915_private *dev_priv) > > > > void intel_init_clock_gating(struct drm_i915_private *dev_priv) { > > - dev_priv->display.init_clock_gating(dev_priv); > > + if (INTEL_INFO(dev_priv)->num_pipes) > > + dev_priv->display.init_clock_gating(dev_priv); > > } > > > > void intel_suspend_hw(struct drm_i915_private *dev_priv) @@ -7644,6 > > +7645,10 @@ static void nop_init_clock_gating(struct drm_i915_private > *dev_priv) > > */ > > void intel_init_clock_gating_hooks(struct drm_i915_private *dev_priv) > > { > > + > > + if (INTEL_INFO(dev_priv)->num_pipes == 0) > > + return; > > + > > Looks like this would be neater by doing > > if (INTEL_INFO(dev_priv)->num_pipes == 0) > dev_priv->display.init_clock_gating = nop_init_clock_gating; > else if (IS_SKYLAKE(dev_priv)) > ... > > and you can leave out the check in intel_init_clock_gating(). > > We really want to minimize the amount of these checks sprinkled around. > > BR, > Jani. > I see. Thanks for the comment. I'll update this patch later. Thanks, Elaine > > > > dev_priv->display.init_clock_gating = > skylake_init_clock_gating; > > else if (IS_KABYLAKE(dev_priv)) > > @@ -7685,6 +7690,9 @@ void intel_init_clock_gating_hooks(struct > > drm_i915_private *dev_priv) > > /* Set up chip specific power management-related functions */ void > > intel_init_pm(struct drm_i915_private *dev_priv) { > > + if (INTEL_INFO(dev_priv)->num_pipes == 0) > > + return; > > + > > intel_fbc_init(dev_priv); > > > > /* For cxsr */ > > -- > Jani Nikula, Intel Open Source Technology Center _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
