| Message ID | 1446553874-22986-5-git-send-email-patrik.jakobsson@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 03, 2015 at 01:31:10PM +0100, Patrik Jakobsson wrote: > We need DC5/DC6 to be disabled around modesets to prevent confusing the > DMC. Also, we've run out of bits in the 32 bit power domain mask so now > it's a 64 bit mask. We could get back 4 bits by squashing each 2 and 4 lane DDI power doamin into just one power domain. I don't think we use the 2 vs. 4 lane distinciton anywhere. It was basically added for VLV but turns it it can't be used there, so I think we could just get rid of it. > > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_debugfs.c | 2 ++ > drivers/gpu/drm/i915/i915_drv.h | 3 ++- > drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +- > 3 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index f7b85fe..ae9a2ed 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -2746,6 +2746,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain) > return "AUX_D"; > case POWER_DOMAIN_GMBUS: > return "GMBUS"; > + case POWER_DOMAIN_MODESET: > + return "MODESET"; > case POWER_DOMAIN_INIT: > return "INIT"; > default: > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index c3d3b2a..efb6a00 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -200,6 +200,7 @@ enum intel_display_power_domain { > POWER_DOMAIN_AUX_C, > POWER_DOMAIN_AUX_D, > POWER_DOMAIN_GMBUS, > + POWER_DOMAIN_MODESET, > POWER_DOMAIN_INIT, > > POWER_DOMAIN_NUM, > @@ -1226,7 +1227,7 @@ struct i915_power_well { > int count; > /* cached hw enabled state */ > bool hw_enabled; > - unsigned long domains; > + unsigned long long domains; > unsigned long data; > const struct i915_power_well_ops *ops; > }; > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index b6ce77f..d0ed38b 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -1815,7 +1815,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv) > { > struct i915_power_domains *power_domains = &dev_priv->power_domains; > > - BUILD_BUG_ON(POWER_DOMAIN_NUM > 31); > + BUILD_BUG_ON(POWER_DOMAIN_NUM > 63); > > mutex_init(&power_domains->lock); > > -- > 2.1.4
On Wed, Nov 4, 2015 at 6:29 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Tue, Nov 03, 2015 at 01:31:10PM +0100, Patrik Jakobsson wrote: >> We need DC5/DC6 to be disabled around modesets to prevent confusing the >> DMC. Also, we've run out of bits in the 32 bit power domain mask so now >> it's a 64 bit mask. > > We could get back 4 bits by squashing each 2 and 4 lane DDI power doamin > into just one power domain. I don't think we use the 2 vs. 4 lane > distinciton anywhere. It was basically added for VLV but turns it it > can't be used there, so I think we could just get rid of it. Good idea. I can't see that we're making a distinction anywhere either. > >> >> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> >> --- >> drivers/gpu/drm/i915/i915_debugfs.c | 2 ++ >> drivers/gpu/drm/i915/i915_drv.h | 3 ++- >> drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +- >> 3 files changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c >> index f7b85fe..ae9a2ed 100644 >> --- a/drivers/gpu/drm/i915/i915_debugfs.c >> +++ b/drivers/gpu/drm/i915/i915_debugfs.c >> @@ -2746,6 +2746,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain) >> return "AUX_D"; >> case POWER_DOMAIN_GMBUS: >> return "GMBUS"; >> + case POWER_DOMAIN_MODESET: >> + return "MODESET"; >> case POWER_DOMAIN_INIT: >> return "INIT"; >> default: >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index c3d3b2a..efb6a00 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -200,6 +200,7 @@ enum intel_display_power_domain { >> POWER_DOMAIN_AUX_C, >> POWER_DOMAIN_AUX_D, >> POWER_DOMAIN_GMBUS, >> + POWER_DOMAIN_MODESET, >> POWER_DOMAIN_INIT, >> >> POWER_DOMAIN_NUM, >> @@ -1226,7 +1227,7 @@ struct i915_power_well { >> int count; >> /* cached hw enabled state */ >> bool hw_enabled; >> - unsigned long domains; >> + unsigned long long domains; >> unsigned long data; >> const struct i915_power_well_ops *ops; >> }; >> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c >> index b6ce77f..d0ed38b 100644 >> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c >> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c >> @@ -1815,7 +1815,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv) >> { >> struct i915_power_domains *power_domains = &dev_priv->power_domains; >> >> - BUILD_BUG_ON(POWER_DOMAIN_NUM > 31); >> + BUILD_BUG_ON(POWER_DOMAIN_NUM > 63); >> >> mutex_init(&power_domains->lock); >> >> -- >> 2.1.4 > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi, On 3 November 2015 at 12:31, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote: > We need DC5/DC6 to be disabled around modesets to prevent confusing the > DMC. Also, we've run out of bits in the 32 bit power domain mask so now > it's a 64 bit mask. There are quite a lot of users in intel_display.c (search for put_domains, display_power_put, put_power_domains) which need updating for the unsigned long long change. Cheers, Daniel
On Thu, Nov 5, 2015 at 4:02 PM, Daniel Stone <daniel@fooishbar.org> wrote: > Hi, > > On 3 November 2015 at 12:31, Patrik Jakobsson > <patrik.jakobsson@linux.intel.com> wrote: >> We need DC5/DC6 to be disabled around modesets to prevent confusing the >> DMC. Also, we've run out of bits in the 32 bit power domain mask so now >> it's a 64 bit mask. > > There are quite a lot of users in intel_display.c (search for > put_domains, display_power_put, put_power_domains) which need updating > for the unsigned long long change. Ah yes, we carry the mask around there as well. Thanks for catching that. I like the move of POWER_DOMAIN_MODESET into put_domain as well. I will resend the whole series again rebased on Imre's latest series. Ok if I incorporate your changes directly? Thanks Patrik > > Cheers, > Daniel > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi, On 5 November 2015 at 17:12, Patrik Jakobsson <patrik.r.jakobsson@gmail.com> wrote: > On Thu, Nov 5, 2015 at 4:02 PM, Daniel Stone <daniel@fooishbar.org> wrote: >> On 3 November 2015 at 12:31, Patrik Jakobsson >> <patrik.jakobsson@linux.intel.com> wrote: >>> We need DC5/DC6 to be disabled around modesets to prevent confusing the >>> DMC. Also, we've run out of bits in the 32 bit power domain mask so now >>> it's a 64 bit mask. >> >> There are quite a lot of users in intel_display.c (search for >> put_domains, display_power_put, put_power_domains) which need updating >> for the unsigned long long change. > > Ah yes, we carry the mask around there as well. Thanks for catching > that. I like the move of POWER_DOMAIN_MODESET into put_domain as well. > I will resend the whole series again rebased on Imre's latest series. > Ok if I incorporate your changes directly? Of course! Sending it like that wasn't fishing for attribution, just that as I rebased and shifted around a bunch of other changes, git-send-email was the most laziness-friendly option. :) Thanks for pulling it in. If you're an admin on intel-gfx Patchwork, would you mind marking my patch as superseded or something so it doesn't show up? I assume you've co-ordinated with Imre, but he does have a combined tree on github.com/ideak/linux#dmc-fixes, which may save you some time. Cheers, Daniel
On pe, 2015-11-06 at 13:53 +0000, Daniel Stone wrote: > Hi, > > On 5 November 2015 at 17:12, Patrik Jakobsson > <patrik.r.jakobsson@gmail.com> wrote: > > On Thu, Nov 5, 2015 at 4:02 PM, Daniel Stone <daniel@fooishbar.org> > > wrote: > > > On 3 November 2015 at 12:31, Patrik Jakobsson > > > <patrik.jakobsson@linux.intel.com> wrote: > > > > We need DC5/DC6 to be disabled around modesets to prevent > > > > confusing the > > > > DMC. Also, we've run out of bits in the 32 bit power domain > > > > mask so now > > > > it's a 64 bit mask. > > > > > > There are quite a lot of users in intel_display.c (search for > > > put_domains, display_power_put, put_power_domains) which need > > > updating > > > for the unsigned long long change. > > > > Ah yes, we carry the mask around there as well. Thanks for catching > > that. I like the move of POWER_DOMAIN_MODESET into put_domain as > > well. > > I will resend the whole series again rebased on Imre's latest > > series. > > Ok if I incorporate your changes directly? > > Of course! Sending it like that wasn't fishing for attribution, just > that as I rebased and shifted around a bunch of other changes, > git-send-email was the most laziness-friendly option. :) Thanks for > pulling it in. If you're an admin on intel-gfx Patchwork, would you > mind marking my patch as superseded or something so it doesn't show > up? > > I assume you've co-ordinated with Imre, but he does have a combined > tree on github.com/ideak/linux#dmc-fixes, which may save you some > time. Well, that one has my patches on top while we agreed with Patrik to have the opposite order in the end, since I think that makes more sense for bisectability: Patrik's change to move DC6 enabling earlier might reveal some issues that are supposed to be fixed in my patchset. Anyway the end result should be the same so hopefully that branch was useful for testing. > > Cheers, > Daniel > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Hi, On 6 November 2015 at 14:17, Imre Deak <imre.deak@intel.com> wrote: > On pe, 2015-11-06 at 13:53 +0000, Daniel Stone wrote: >> On 5 November 2015 at 17:12, Patrik Jakobsson >> <patrik.r.jakobsson@gmail.com> wrote: >> > Ah yes, we carry the mask around there as well. Thanks for catching >> > that. I like the move of POWER_DOMAIN_MODESET into put_domain as >> > well. >> > I will resend the whole series again rebased on Imre's latest >> > series. >> > Ok if I incorporate your changes directly? >> >> Of course! Sending it like that wasn't fishing for attribution, just >> that as I rebased and shifted around a bunch of other changes, >> git-send-email was the most laziness-friendly option. :) Thanks for >> pulling it in. If you're an admin on intel-gfx Patchwork, would you >> mind marking my patch as superseded or something so it doesn't show >> up? >> >> I assume you've co-ordinated with Imre, but he does have a combined >> tree on github.com/ideak/linux#dmc-fixes, which may save you some >> time. > > Well, that one has my patches on top while we agreed with Patrik to > have the opposite order in the end, since I think that makes more sense > for bisectability: Patrik's change to move DC6 enabling earlier might > reveal some issues that are supposed to be fixed in my patchset. Anyway > the end result should be the same so hopefully that branch was useful > for testing. Yeah, having the combined branch in that order was very useful - thanks a lot! I have no useful opinion on which order is better, so whatever you guys decide will be best I'm sure. Cheers, Daniel
Hi, On 6 November 2015 at 13:53, Daniel Stone <daniel@fooishbar.org> wrote: > On 5 November 2015 at 17:12, Patrik Jakobsson > <patrik.r.jakobsson@gmail.com> wrote: >> On Thu, Nov 5, 2015 at 4:02 PM, Daniel Stone <daniel@fooishbar.org> wrote: >>> On 3 November 2015 at 12:31, Patrik Jakobsson >>> <patrik.jakobsson@linux.intel.com> wrote: >>>> We need DC5/DC6 to be disabled around modesets to prevent confusing the >>>> DMC. Also, we've run out of bits in the 32 bit power domain mask so now >>>> it's a 64 bit mask. >>> >>> There are quite a lot of users in intel_display.c (search for >>> put_domains, display_power_put, put_power_domains) which need updating >>> for the unsigned long long change. >> >> Ah yes, we carry the mask around there as well. Thanks for catching >> that. I like the move of POWER_DOMAIN_MODESET into put_domain as well. >> I will resend the whole series again rebased on Imre's latest series. >> Ok if I incorporate your changes directly? > > Of course! Sending it like that wasn't fishing for attribution, just > that as I rebased and shifted around a bunch of other changes, > git-send-email was the most laziness-friendly option. :) Thanks for > pulling it in. If you're an admin on intel-gfx Patchwork, would you > mind marking my patch as superseded or something so it doesn't show > up? Safe to say though that I'd recommend work->put_power_domains |= (1 << POWER_MODESET), rather than |= POWER_DOMAIN_MODESET ... Cheers, Daniel
On 3 November 2015 at 22:31, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote: > We need DC5/DC6 to be disabled around modesets to prevent confusing the > DMC. Also, we've run out of bits in the 32 bit power domain mask so now > it's a 64 bit mask. > It was bad enough the original code used unsigned long so was different on 32/64 bit, but don't keep going with it. uint64_t already please. Dave.
On Sat, Nov 07, 2015 at 08:29:49AM +1000, Dave Airlie wrote: > On 3 November 2015 at 22:31, Patrik Jakobsson > <patrik.jakobsson@linux.intel.com> wrote: > > We need DC5/DC6 to be disabled around modesets to prevent confusing the > > DMC. Also, we've run out of bits in the 32 bit power domain mask so now > > it's a 64 bit mask. > > > > It was bad enough the original code used unsigned long so was > different on 32/64 bit, > > but don't keep going with it. uint64_t already please. Concurred. Also I don't like typing ;-) -Daniel
On Wed, Nov 18, 2015 at 10:02 AM, Daniel Vetter <daniel@ffwll.ch> wrote: > On Sat, Nov 07, 2015 at 08:29:49AM +1000, Dave Airlie wrote: >> On 3 November 2015 at 22:31, Patrik Jakobsson >> <patrik.jakobsson@linux.intel.com> wrote: >> > We need DC5/DC6 to be disabled around modesets to prevent confusing the >> > DMC. Also, we've run out of bits in the 32 bit power domain mask so now >> > it's a 64 bit mask. >> > >> >> It was bad enough the original code used unsigned long so was >> different on 32/64 bit, >> >> but don't keep going with it. uint64_t already please. > > Concurred. Also I don't like typing ;-) We removed a few unused power domains so didn't need to bump it to 64 bits. Can do that anyway or just change it to an uint32_t in the next round of PW/DMC cleanups. -Patrik > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index f7b85fe..ae9a2ed 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2746,6 +2746,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain) return "AUX_D"; case POWER_DOMAIN_GMBUS: return "GMBUS"; + case POWER_DOMAIN_MODESET: + return "MODESET"; case POWER_DOMAIN_INIT: return "INIT"; default: diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c3d3b2a..efb6a00 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -200,6 +200,7 @@ enum intel_display_power_domain { POWER_DOMAIN_AUX_C, POWER_DOMAIN_AUX_D, POWER_DOMAIN_GMBUS, + POWER_DOMAIN_MODESET, POWER_DOMAIN_INIT, POWER_DOMAIN_NUM, @@ -1226,7 +1227,7 @@ struct i915_power_well { int count; /* cached hw enabled state */ bool hw_enabled; - unsigned long domains; + unsigned long long domains; unsigned long data; const struct i915_power_well_ops *ops; }; diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c index b6ce77f..d0ed38b 100644 --- a/drivers/gpu/drm/i915/intel_runtime_pm.c +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c @@ -1815,7 +1815,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv) { struct i915_power_domains *power_domains = &dev_priv->power_domains; - BUILD_BUG_ON(POWER_DOMAIN_NUM > 31); + BUILD_BUG_ON(POWER_DOMAIN_NUM > 63); mutex_init(&power_domains->lock);
We need DC5/DC6 to be disabled around modesets to prevent confusing the DMC. Also, we've run out of bits in the 32 bit power domain mask so now it's a 64 bit mask. Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 3 ++- drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-)