| Message ID | 20190130181359.20693-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Pick the first unused PLL once again | expand |
Em qua, 2019-01-30 às 20:13 +0200, Ville Syrjala escreveu: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > commit 5b0bd14dcc6b ("drm/i915/icl: keep track of unused pll while > looping") inadvertently (I presume) changed the code to pick the > last unused dpll rather than the first unused one like we did before. > > While there should most likely be no harm in changing the order > let's change back just to avoid a change in the behaviour. At > least it might reduce the confusion when staring at logs (took > me a while to figure out why DPLL1 being picked over DPLL0 > when the latter was most definitely available). Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Cc: Lucas De Marchi <lucas.demarchi@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > index 8f70838ac7d8..0a42d11c4c33 100644 > --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > @@ -258,7 +258,8 @@ intel_find_shared_dpll(struct intel_crtc *crtc, > > /* Only want to check enabled timings first */ > if (shared_dpll[i].crtc_mask == 0) { > - unused_pll = pll; > + if (!unused_pll) > + unused_pll = pll; > continue; > } >
On Wed, Jan 30, 2019 at 08:13:59PM +0200, Ville Syrjälä wrote: >From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >commit 5b0bd14dcc6b ("drm/i915/icl: keep track of unused pll while >looping") inadvertently (I presume) changed the code to pick the >last unused dpll rather than the first unused one like we did before. > >While there should most likely be no harm in changing the order >let's change back just to avoid a change in the behaviour. At >least it might reduce the confusion when staring at logs (took >me a while to figure out why DPLL1 being picked over DPLL0 >when the latter was most definitely available). I assumed the no harm and simplest approach so I didn't bother checking if the it was already set. So I'd not say it was "inadvertently", it was on purpose. But if it makes life easier to read the logs: Acked-by: Lucas De Marchi <lucas.demarchi@intel.com> Lucas De Marchi > >Cc: Lucas De Marchi <lucas.demarchi@intel.com> >Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> >--- > drivers/gpu/drm/i915/intel_dpll_mgr.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > >diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c >index 8f70838ac7d8..0a42d11c4c33 100644 >--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c >+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c >@@ -258,7 +258,8 @@ intel_find_shared_dpll(struct intel_crtc *crtc, > > /* Only want to check enabled timings first */ > if (shared_dpll[i].crtc_mask == 0) { >- unused_pll = pll; >+ if (!unused_pll) >+ unused_pll = pll; > continue; > } > >-- >2.19.2 >
On Wed, Jan 30, 2019 at 05:24:07PM -0800, Lucas De Marchi wrote: > On Wed, Jan 30, 2019 at 08:13:59PM +0200, Ville Syrjälä wrote: > >From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > >commit 5b0bd14dcc6b ("drm/i915/icl: keep track of unused pll while > >looping") inadvertently (I presume) changed the code to pick the > >last unused dpll rather than the first unused one like we did before. > > > >While there should most likely be no harm in changing the order > >let's change back just to avoid a change in the behaviour. At > >least it might reduce the confusion when staring at logs (took > >me a while to figure out why DPLL1 being picked over DPLL0 > >when the latter was most definitely available). > > > I assumed the no harm and simplest approach so I didn't bother > checking if the it was already set. So I'd not say it was > "inadvertently", it was on purpose. That would have been good to mention in the commit msg. > > But if it makes life easier to read the logs: > > Acked-by: Lucas De Marchi <lucas.demarchi@intel.com> Ta. Pushed. > > > Lucas De Marchi > > > > >Cc: Lucas De Marchi <lucas.demarchi@intel.com> > >Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >--- > > drivers/gpu/drm/i915/intel_dpll_mgr.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > >diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c > >index 8f70838ac7d8..0a42d11c4c33 100644 > >--- a/drivers/gpu/drm/i915/intel_dpll_mgr.c > >+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c > >@@ -258,7 +258,8 @@ intel_find_shared_dpll(struct intel_crtc *crtc, > > > > /* Only want to check enabled timings first */ > > if (shared_dpll[i].crtc_mask == 0) { > >- unused_pll = pll; > >+ if (!unused_pll) > >+ unused_pll = pll; > > continue; > > } > > > >-- > >2.19.2 > >
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.c b/drivers/gpu/drm/i915/intel_dpll_mgr.c index 8f70838ac7d8..0a42d11c4c33 100644 --- a/drivers/gpu/drm/i915/intel_dpll_mgr.c +++ b/drivers/gpu/drm/i915/intel_dpll_mgr.c @@ -258,7 +258,8 @@ intel_find_shared_dpll(struct intel_crtc *crtc, /* Only want to check enabled timings first */ if (shared_dpll[i].crtc_mask == 0) { - unused_pll = pll; + if (!unused_pll) + unused_pll = pll; continue; }