| Message ID | 1358540953-3979-7-git-send-email-przanoni@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 18, 2013 at 06:29:08PM -0200, Paulo Zanoni wrote: > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > If the power well is disabled, we should not try to read its > registers, otherwise we'll get "unclaimed register" messages. > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index a7fb7e1..921b020 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -1214,9 +1214,15 @@ void assert_pipe(struct drm_i915_private *dev_priv, > if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) > state = true; > > - reg = PIPECONF(cpu_transcoder); > - val = I915_READ(reg); > - cur_state = !!(val & PIPECONF_ENABLE); > + if (cpu_transcoder == TRANSCODER_EDP || > + (I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_STATE)) { Should that also check HSW_PWR_WELL_ENABLE? KVMR might have the well enabled, while the driver has it disabled. But KVMR might have already disabled the well, and it might get disabled just after this check, and then you would hit the unclaimed register issue again. > + reg = PIPECONF(cpu_transcoder); > + val = I915_READ(reg); > + cur_state = !!(val & PIPECONF_ENABLE); > + } else { > + cur_state = false; > + } > + > WARN(cur_state != state, > "pipe %c assertion failure (expected %s, current %s)\n", > pipe_name(pipe), state_string(state), state_string(cur_state)); > -- > 1.7.10.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Jan 21, 2013 at 03:45:48PM +0200, Ville Syrjälä wrote: > On Fri, Jan 18, 2013 at 06:29:08PM -0200, Paulo Zanoni wrote: > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > If the power well is disabled, we should not try to read its > > registers, otherwise we'll get "unclaimed register" messages. > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index a7fb7e1..921b020 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1214,9 +1214,15 @@ void assert_pipe(struct drm_i915_private *dev_priv, > > if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) > > state = true; > > > > - reg = PIPECONF(cpu_transcoder); > > - val = I915_READ(reg); > > - cur_state = !!(val & PIPECONF_ENABLE); > > + if (cpu_transcoder == TRANSCODER_EDP || > > + (I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_STATE)) { > > Should that also check HSW_PWR_WELL_ENABLE? KVMR might have the well > enabled, while the driver has it disabled. But KVMR might have already > disabled the well, and it might get disabled just after this check, > and then you would hit the unclaimed register issue again. The important matter is to not read registers in the power well if it's off, for which checking just one of the three bits is enough. If the kvm keeps the power well on, we just avoid checking the pipe state if we don't need the power well, but otherwise no side effect. Otoh just one set bit makes sure that the power well is on and we can read the regs). So looks good to me. -Daniel > > > + reg = PIPECONF(cpu_transcoder); > > + val = I915_READ(reg); > > + cur_state = !!(val & PIPECONF_ENABLE); > > + } else { > > + cur_state = false; > > + } > > + > > WARN(cur_state != state, > > "pipe %c assertion failure (expected %s, current %s)\n", > > pipe_name(pipe), state_string(state), state_string(cur_state)); > > -- > > 1.7.10.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jan 22, 2013 at 02:04:09PM +0100, Daniel Vetter wrote: > On Mon, Jan 21, 2013 at 03:45:48PM +0200, Ville Syrjälä wrote: > > On Fri, Jan 18, 2013 at 06:29:08PM -0200, Paulo Zanoni wrote: > > > From: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > > > > If the power well is disabled, we should not try to read its > > > registers, otherwise we'll get "unclaimed register" messages. > > > > > > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- > > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index a7fb7e1..921b020 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -1214,9 +1214,15 @@ void assert_pipe(struct drm_i915_private *dev_priv, > > > if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) > > > state = true; > > > > > > - reg = PIPECONF(cpu_transcoder); > > > - val = I915_READ(reg); > > > - cur_state = !!(val & PIPECONF_ENABLE); > > > + if (cpu_transcoder == TRANSCODER_EDP || > > > + (I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_STATE)) { > > > > Should that also check HSW_PWR_WELL_ENABLE? KVMR might have the well > > enabled, while the driver has it disabled. But KVMR might have already > > disabled the well, and it might get disabled just after this check, > > and then you would hit the unclaimed register issue again. > > The important matter is to not read registers in the power well if it's > off, for which checking just one of the three bits is enough. If the kvm > keeps the power well on, we just avoid checking the pipe state if we don't > need the power well, but otherwise no side effect. Otoh just one set bit > makes sure that the power well is on and we can read the regs). The power well may be on when you're reading the status bit, but assuming it was KVMR who caused the power well to be powered on, we can't be sure the power well will remain powered long enough to read the registers.
Hi 2013/1/21 Ville Syrjälä <ville.syrjala@linux.intel.com>: > On Fri, Jan 18, 2013 at 06:29:08PM -0200, Paulo Zanoni wrote: >> From: Paulo Zanoni <paulo.r.zanoni@intel.com> >> >> If the power well is disabled, we should not try to read its >> registers, otherwise we'll get "unclaimed register" messages. >> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >> --- >> drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> index a7fb7e1..921b020 100644 >> --- a/drivers/gpu/drm/i915/intel_display.c >> +++ b/drivers/gpu/drm/i915/intel_display.c >> @@ -1214,9 +1214,15 @@ void assert_pipe(struct drm_i915_private *dev_priv, >> if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) >> state = true; >> >> - reg = PIPECONF(cpu_transcoder); >> - val = I915_READ(reg); >> - cur_state = !!(val & PIPECONF_ENABLE); >> + if (cpu_transcoder == TRANSCODER_EDP || >> + (I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_STATE)) { > > Should that also check HSW_PWR_WELL_ENABLE? KVMR might have the well > enabled, while the driver has it disabled. But KVMR might have already > disabled the well, and it might get disabled just after this check, > and then you would hit the unclaimed register issue again. By reading your text it seems you think that if bit 31 is 0 then bit 30 will necessarily be 0. This is not true. If any entity (including KVMr) needs the power well enabled (and is requesting so), then bit 30 of HSW_PWR_WELL_DRIVER will be 1, no matter the status of bit 31. Another thing to mention is that no entity can force the power well to be "off", it can only force to be "on". Still, your text does contain an example of a problem with the current code. Let me explicitly write it: - Our driver doesn't need it to be enabled, so bit 31 of HSW_PWR_WELL_DRIVER is 0 - KVMr needs it enabled, so HSW_PWR_WELL_DRIVER bit 30 is 1 - We do the I915_READ above - After the read, KVMr says it doesn't need the power well to be enabled anymore, so it gets disabled because no one else is requesting it - Then we touch an unclaimed register. I think the only policy to avoid these race conditions should be "if I don't need the power well to be enabled, then I won't touch registers that require it to be enabled, no matter what the real state of the power well is".This should prevent any kinds of problems, because if I need the power well to be enabled, then I requested it to be enabled and it is enabled because I asked so. I'll send a second version of the patch with this. > >> + reg = PIPECONF(cpu_transcoder); >> + val = I915_READ(reg); >> + cur_state = !!(val & PIPECONF_ENABLE); >> + } else { >> + cur_state = false; >> + } >> + >> WARN(cur_state != state, >> "pipe %c assertion failure (expected %s, current %s)\n", >> pipe_name(pipe), state_string(state), state_string(cur_state)); >> -- >> 1.7.10.4 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC
On Fri, Jan 25, 2013 at 01:40:08PM -0200, Paulo Zanoni wrote: > Hi > > 2013/1/21 Ville Syrjälä <ville.syrjala@linux.intel.com>: > > On Fri, Jan 18, 2013 at 06:29:08PM -0200, Paulo Zanoni wrote: > >> From: Paulo Zanoni <paulo.r.zanoni@intel.com> > >> > >> If the power well is disabled, we should not try to read its > >> registers, otherwise we'll get "unclaimed register" messages. > >> > >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > >> --- > >> drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- > >> 1 file changed, 9 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >> index a7fb7e1..921b020 100644 > >> --- a/drivers/gpu/drm/i915/intel_display.c > >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> @@ -1214,9 +1214,15 @@ void assert_pipe(struct drm_i915_private *dev_priv, > >> if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) > >> state = true; > >> > >> - reg = PIPECONF(cpu_transcoder); > >> - val = I915_READ(reg); > >> - cur_state = !!(val & PIPECONF_ENABLE); > >> + if (cpu_transcoder == TRANSCODER_EDP || > >> + (I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_STATE)) { > > > > Should that also check HSW_PWR_WELL_ENABLE? KVMR might have the well > > enabled, while the driver has it disabled. But KVMR might have already > > disabled the well, and it might get disabled just after this check, > > and then you would hit the unclaimed register issue again. > > By reading your text it seems you think that if bit 31 is 0 then bit > 30 will necessarily be 0. This is not true. If any entity (including > KVMr) needs the power well enabled (and is requesting so), then bit 30 > of HSW_PWR_WELL_DRIVER will be 1, no matter the status of bit 31. Did you verify that on real HW? It makes little sense when you consider that the spec says not to toggle the request bit when there's a state transition currently happening. If all the status bits read as the same thing, then there's no way to know when a transition is actually happening, and hence the text in the spec is complete nonsense. The only sensible interpretation of that text I could think of is that you're not supposed to abort a state transition you yourself caused. That would be achieved by polling the status bit after toggling the request bit, assuming that the status bit is just a delayed version of request bit. > Another thing to mention is that no entity can force the power well to > be "off", it can only force to be "on". > > Still, your text does contain an example of a problem with the current > code. Let me explicitly write it: > - Our driver doesn't need it to be enabled, so bit 31 of > HSW_PWR_WELL_DRIVER is 0 > - KVMr needs it enabled, so HSW_PWR_WELL_DRIVER bit 30 is 1 > - We do the I915_READ above > - After the read, KVMr says it doesn't need the power well to be > enabled anymore, so it gets disabled because no one else is requesting > it > - Then we touch an unclaimed register. > > I think the only policy to avoid these race conditions should be "if I > don't need the power well to be enabled, then I won't touch registers > that require it to be enabled, no matter what the real state of the > power well is".This should prevent any kinds of problems, because if I > need the power well to be enabled, then I requested it to be enabled > and it is enabled because I asked so. I'll send a second version of > the patch with this. > > > > >> + reg = PIPECONF(cpu_transcoder); > >> + val = I915_READ(reg); > >> + cur_state = !!(val & PIPECONF_ENABLE); > >> + } else { > >> + cur_state = false; > >> + } > >> + > >> WARN(cur_state != state, > >> "pipe %c assertion failure (expected %s, current %s)\n", > >> pipe_name(pipe), state_string(state), state_string(cur_state)); > >> -- > >> 1.7.10.4 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > -- > > Ville Syrjälä > > Intel OTC > > > > -- > Paulo Zanoni
Hi 2013/1/25 Ville Syrjälä <ville.syrjala@linux.intel.com>: > On Fri, Jan 25, 2013 at 01:40:08PM -0200, Paulo Zanoni wrote: >> Hi >> >> 2013/1/21 Ville Syrjälä <ville.syrjala@linux.intel.com>: >> > On Fri, Jan 18, 2013 at 06:29:08PM -0200, Paulo Zanoni wrote: >> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com> >> >> >> >> If the power well is disabled, we should not try to read its >> >> registers, otherwise we'll get "unclaimed register" messages. >> >> >> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- >> >> 1 file changed, 9 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c >> >> index a7fb7e1..921b020 100644 >> >> --- a/drivers/gpu/drm/i915/intel_display.c >> >> +++ b/drivers/gpu/drm/i915/intel_display.c >> >> @@ -1214,9 +1214,15 @@ void assert_pipe(struct drm_i915_private *dev_priv, >> >> if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) >> >> state = true; >> >> >> >> - reg = PIPECONF(cpu_transcoder); >> >> - val = I915_READ(reg); >> >> - cur_state = !!(val & PIPECONF_ENABLE); >> >> + if (cpu_transcoder == TRANSCODER_EDP || >> >> + (I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_STATE)) { >> > >> > Should that also check HSW_PWR_WELL_ENABLE? KVMR might have the well >> > enabled, while the driver has it disabled. But KVMR might have already >> > disabled the well, and it might get disabled just after this check, >> > and then you would hit the unclaimed register issue again. >> >> By reading your text it seems you think that if bit 31 is 0 then bit >> 30 will necessarily be 0. This is not true. If any entity (including >> KVMr) needs the power well enabled (and is requesting so), then bit 30 >> of HSW_PWR_WELL_DRIVER will be 1, no matter the status of bit 31. > > Did you verify that on real HW? Yes. > > It makes little sense when you consider that the spec says not > to toggle the request bit when there's a state transition currently > happening. If all the status bits read as the same thing, then > there's no way to know when a transition is actually happening, > and hence the text in the spec is complete nonsense. > I agree... I'm investigating this problem. That's why there is still no new version of patch 3/10 here :) > The only sensible interpretation of that text I could think of is > that you're not supposed to abort a state transition you yourself > caused. That would be achieved by polling the status bit after > toggling the request bit, assuming that the status bit is just a > delayed version of request bit. If we're enabling then it's fine: we requested it to enable, then at some point it will be enabled, then we can start using the registers. We can always avoid touching the registers when the well is still being enabled. The only situation where the "restriction" doesn't make sense is on the "disable" case. OTOH, if we requested it to be disabled, the fact that we can't predict whether it's really going to be disabled or not shouldn't really matter that much since we're not supposed to touch the "possibly disabled" registers, and if anybody needs to touch those registers they will have to request the well to be enabled first... This still doesn't explain the "Restriction", but maybe that restriction is just for the "enable" case. > >> Another thing to mention is that no entity can force the power well to >> be "off", it can only force to be "on". >> >> Still, your text does contain an example of a problem with the current >> code. Let me explicitly write it: >> - Our driver doesn't need it to be enabled, so bit 31 of >> HSW_PWR_WELL_DRIVER is 0 >> - KVMr needs it enabled, so HSW_PWR_WELL_DRIVER bit 30 is 1 >> - We do the I915_READ above >> - After the read, KVMr says it doesn't need the power well to be >> enabled anymore, so it gets disabled because no one else is requesting >> it >> - Then we touch an unclaimed register. >> >> I think the only policy to avoid these race conditions should be "if I >> don't need the power well to be enabled, then I won't touch registers >> that require it to be enabled, no matter what the real state of the >> power well is".This should prevent any kinds of problems, because if I >> need the power well to be enabled, then I requested it to be enabled >> and it is enabled because I asked so. I'll send a second version of >> the patch with this. >> >> > >> >> + reg = PIPECONF(cpu_transcoder); >> >> + val = I915_READ(reg); >> >> + cur_state = !!(val & PIPECONF_ENABLE); >> >> + } else { >> >> + cur_state = false; >> >> + } >> >> + >> >> WARN(cur_state != state, >> >> "pipe %c assertion failure (expected %s, current %s)\n", >> >> pipe_name(pipe), state_string(state), state_string(cur_state)); >> >> -- >> >> 1.7.10.4 >> >> >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@lists.freedesktop.org >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx >> > >> > -- >> > Ville Syrjälä >> > Intel OTC >> >> >> >> -- >> Paulo Zanoni > > -- > Ville Syrjälä > Intel OTC
On Fri, Jan 25, 2013 at 02:04:03PM -0200, Paulo Zanoni wrote: > Hi > > 2013/1/25 Ville Syrjälä <ville.syrjala@linux.intel.com>: > > On Fri, Jan 25, 2013 at 01:40:08PM -0200, Paulo Zanoni wrote: > >> Hi > >> > >> 2013/1/21 Ville Syrjälä <ville.syrjala@linux.intel.com>: > >> > On Fri, Jan 18, 2013 at 06:29:08PM -0200, Paulo Zanoni wrote: > >> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com> > >> >> > >> >> If the power well is disabled, we should not try to read its > >> >> registers, otherwise we'll get "unclaimed register" messages. > >> >> > >> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com> > >> >> --- > >> >> drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- > >> >> 1 file changed, 9 insertions(+), 3 deletions(-) > >> >> > >> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > >> >> index a7fb7e1..921b020 100644 > >> >> --- a/drivers/gpu/drm/i915/intel_display.c > >> >> +++ b/drivers/gpu/drm/i915/intel_display.c > >> >> @@ -1214,9 +1214,15 @@ void assert_pipe(struct drm_i915_private *dev_priv, > >> >> if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) > >> >> state = true; > >> >> > >> >> - reg = PIPECONF(cpu_transcoder); > >> >> - val = I915_READ(reg); > >> >> - cur_state = !!(val & PIPECONF_ENABLE); > >> >> + if (cpu_transcoder == TRANSCODER_EDP || > >> >> + (I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_STATE)) { > >> > > >> > Should that also check HSW_PWR_WELL_ENABLE? KVMR might have the well > >> > enabled, while the driver has it disabled. But KVMR might have already > >> > disabled the well, and it might get disabled just after this check, > >> > and then you would hit the unclaimed register issue again. > >> > >> By reading your text it seems you think that if bit 31 is 0 then bit > >> 30 will necessarily be 0. This is not true. If any entity (including > >> KVMr) needs the power well enabled (and is requesting so), then bit 30 > >> of HSW_PWR_WELL_DRIVER will be 1, no matter the status of bit 31. > > > > Did you verify that on real HW? > > Yes. > > > > > It makes little sense when you consider that the spec says not > > to toggle the request bit when there's a state transition currently > > happening. If all the status bits read as the same thing, then > > there's no way to know when a transition is actually happening, > > and hence the text in the spec is complete nonsense. > > > > I agree... I'm investigating this problem. That's why there is still > no new version of patch 3/10 here :) > > > The only sensible interpretation of that text I could think of is > > that you're not supposed to abort a state transition you yourself > > caused. That would be achieved by polling the status bit after > > toggling the request bit, assuming that the status bit is just a > > delayed version of request bit. > > If we're enabling then it's fine: we requested it to enable, then at > some point it will be enabled, then we can start using the registers. > We can always avoid touching the registers when the well is still > being enabled. I think you can only assume this if the status bit goes to 0 immediately when the well starts powering down. And it won't go back to 1 until the well is fully powered again. Well, I guess that's the only design that would make sense anyway. But even then you can't tell the fully powered down case apart from the currently going down case. > The only situation where the "restriction" doesn't make > sense is on the "disable" case. > > OTOH, if we requested it to be disabled, the fact that we can't > predict whether it's really going to be disabled or not shouldn't > really matter that much since we're not supposed to touch the > "possibly disabled" registers, and if anybody needs to touch those > registers they will have to request the well to be enabled first... > This still doesn't explain the "Restriction", but maybe that > restriction is just for the "enable" case. > > > > >> Another thing to mention is that no entity can force the power well to > >> be "off", it can only force to be "on". > >> > >> Still, your text does contain an example of a problem with the current > >> code. Let me explicitly write it: > >> - Our driver doesn't need it to be enabled, so bit 31 of > >> HSW_PWR_WELL_DRIVER is 0 > >> - KVMr needs it enabled, so HSW_PWR_WELL_DRIVER bit 30 is 1 > >> - We do the I915_READ above > >> - After the read, KVMr says it doesn't need the power well to be > >> enabled anymore, so it gets disabled because no one else is requesting > >> it > >> - Then we touch an unclaimed register. > >> > >> I think the only policy to avoid these race conditions should be "if I > >> don't need the power well to be enabled, then I won't touch registers > >> that require it to be enabled, no matter what the real state of the > >> power well is".This should prevent any kinds of problems, because if I > >> need the power well to be enabled, then I requested it to be enabled > >> and it is enabled because I asked so. I'll send a second version of > >> the patch with this. > >> > >> > > >> >> + reg = PIPECONF(cpu_transcoder); > >> >> + val = I915_READ(reg); > >> >> + cur_state = !!(val & PIPECONF_ENABLE); > >> >> + } else { > >> >> + cur_state = false; > >> >> + } > >> >> + > >> >> WARN(cur_state != state, > >> >> "pipe %c assertion failure (expected %s, current %s)\n", > >> >> pipe_name(pipe), state_string(state), state_string(cur_state)); > >> >> -- > >> >> 1.7.10.4 > >> >> > >> >> _______________________________________________ > >> >> Intel-gfx mailing list > >> >> Intel-gfx@lists.freedesktop.org > >> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx > >> > > >> > -- > >> > Ville Syrjälä > >> > Intel OTC > >> > >> > >> > >> -- > >> Paulo Zanoni > > > > -- > > Ville Syrjälä > > Intel OTC > > > > -- > Paulo Zanoni
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a7fb7e1..921b020 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1214,9 +1214,15 @@ void assert_pipe(struct drm_i915_private *dev_priv, if (pipe == PIPE_A && dev_priv->quirks & QUIRK_PIPEA_FORCE) state = true; - reg = PIPECONF(cpu_transcoder); - val = I915_READ(reg); - cur_state = !!(val & PIPECONF_ENABLE); + if (cpu_transcoder == TRANSCODER_EDP || + (I915_READ(HSW_PWR_WELL_DRIVER) & HSW_PWR_WELL_STATE)) { + reg = PIPECONF(cpu_transcoder); + val = I915_READ(reg); + cur_state = !!(val & PIPECONF_ENABLE); + } else { + cur_state = false; + } + WARN(cur_state != state, "pipe %c assertion failure (expected %s, current %s)\n", pipe_name(pipe), state_string(state), state_string(cur_state));