| Message ID | 1364922237-3620-13-git-send-email-jbarnes@virtuousgeek.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 02, 2013 at 10:03:56AM -0700, Jesse Barnes wrote: > If we couldn't find a pipe we shouldn't return true. This might be even > better as a WARN though, since it should be impossible to have the port > enabled without a pipe selected. > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> These two fixes are merged for -next, thanks. -Daniel > --- > drivers/gpu/drm/i915/intel_dp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index e1b0c94..720ff50 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -1366,7 +1366,7 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder, > intel_dp->output_reg); > } > > - return true; > + return false; > } > > static unsigned intel_dp_get_mode_flags(struct intel_encoder *encoder) > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Apr 02, 2013 at 08:11:05PM +0200, Daniel Vetter wrote: > On Tue, Apr 02, 2013 at 10:03:56AM -0700, Jesse Barnes wrote: > > If we couldn't find a pipe we shouldn't return true. This might be even > > better as a WARN though, since it should be impossible to have the port > > enabled without a pipe selected. > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > These two fixes are merged for -next, thanks. Actually this one here is broken, so I've had to revert it. -Daniel
On Thu, Apr 04, 2013 at 01:15:28AM +0200, Daniel Vetter wrote: > On Tue, Apr 02, 2013 at 08:11:05PM +0200, Daniel Vetter wrote: > > On Tue, Apr 02, 2013 at 10:03:56AM -0700, Jesse Barnes wrote: > > > If we couldn't find a pipe we shouldn't return true. This might be even > > > better as a WARN though, since it should be impossible to have the port > > > enabled without a pipe selected. > > > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > > These two fixes are merged for -next, thanks. > > Actually this one here is broken, so I've had to revert it. I'm also a bit unsure about the DDI one, since if we ever have a port enabled with a bogus pipe link, we probably still want to shut down the port. At least that part is pretty important to not end up with a black screen in a lot of cases, atm Egbert Eich is debugging such a case for sdvo ... -Daniel
On Thu, 4 Apr 2013 01:15:28 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Tue, Apr 02, 2013 at 08:11:05PM +0200, Daniel Vetter wrote: > > On Tue, Apr 02, 2013 at 10:03:56AM -0700, Jesse Barnes wrote: > > > If we couldn't find a pipe we shouldn't return true. This might be even > > > better as a WARN though, since it should be impossible to have the port > > > enabled without a pipe selected. > > > > > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > > > > These two fixes are merged for -next, thanks. > > Actually this one here is broken, so I've had to revert it. What failed? How is it possible we'd have a DP port without a pipe? Every pattern in the register field should correspond to a pipe right?
On Thu, Apr 4, 2013 at 2:12 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > On Thu, 4 Apr 2013 01:15:28 +0200 > Daniel Vetter <daniel@ffwll.ch> wrote: > >> On Tue, Apr 02, 2013 at 08:11:05PM +0200, Daniel Vetter wrote: >> > On Tue, Apr 02, 2013 at 10:03:56AM -0700, Jesse Barnes wrote: >> > > If we couldn't find a pipe we shouldn't return true. This might be even >> > > better as a WARN though, since it should be impossible to have the port >> > > enabled without a pipe selected. >> > > >> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> >> > >> > These two fixes are merged for -next, thanks. >> >> Actually this one here is broken, so I've had to revert it. > > What failed? How is it possible we'd have a DP port without a pipe? > Every pattern in the register field should correspond to a pipe right? Review failed on my side - you've changed the return which is used by all the success cases ... There's another return for one failure case, and the no-pipe one just falls through. The only case this patch did _not_ break is pch ports on cpt/ppt. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Thu, 4 Apr 2013 09:56:07 +0200 Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Apr 4, 2013 at 2:12 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote: > > On Thu, 4 Apr 2013 01:15:28 +0200 > > Daniel Vetter <daniel@ffwll.ch> wrote: > > > >> On Tue, Apr 02, 2013 at 08:11:05PM +0200, Daniel Vetter wrote: > >> > On Tue, Apr 02, 2013 at 10:03:56AM -0700, Jesse Barnes wrote: > >> > > If we couldn't find a pipe we shouldn't return true. This might be even > >> > > better as a WARN though, since it should be impossible to have the port > >> > > enabled without a pipe selected. > >> > > > >> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> > >> > > >> > These two fixes are merged for -next, thanks. > >> > >> Actually this one here is broken, so I've had to revert it. > > > > What failed? How is it possible we'd have a DP port without a pipe? > > Every pattern in the register field should correspond to a pipe right? > > Review failed on my side - you've changed the return which is used by > all the success cases ... There's another return for one failure case, > and the no-pipe one just falls through. The only case this patch did > _not_ break is pch ports on cpt/ppt. Testing fail on my part; I was testing PCH ports but not DP with the config I had. Jesse
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index e1b0c94..720ff50 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1366,7 +1366,7 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder, intel_dp->output_reg); } - return true; + return false; } static unsigned intel_dp_get_mode_flags(struct intel_encoder *encoder)
If we couldn't find a pipe we shouldn't return true. This might be even better as a WARN though, since it should be impossible to have the port enabled without a pipe selected. Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org> --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)