| Message ID | 1370172384-24026-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Jun 02, 2013 at 01:26:24PM +0200, Daniel Vetter wrote: > Since this is run in the compute config stage we need to check > the new_ pointers, i.e the stage output routing, not the current > modeset layout. Also there was a little logic bug in properly skipping > connectors: The old code did not skip any unused connectors and so > clamped to whatever was left in there (usually 0 if that connector > hasn't seen a EDID 1.4 screen ever since boot-up). > > This has been broken when moving the pipe bpp selection in > > commit 4e53c2e010e531b4a014692199e978482d471c7e > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > Date: Wed Mar 27 00:44:58 2013 +0100 > > drm/i915: precompute pipe bpp before touching the hw > > To avoid too much casting switch from drm_ to intel_ types. > > Also add a bit of debug output to help reconstructing what's going > on. > > v2: Try to clarify this a bit: > - s/pipe_config_set_bpp/compute_baseline_pipe_bpp/ to make it clearer > at which stage this function is run. Also add a comment about what > it does. > - Extract the sink clamping into it's own function. > > v3: Actually make it compile. > > v4: Split out all the prep refactoring to make the bugfix stick out > really badly. Also elaborate a bit in the commit message about the > nature of the bugfix. > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Yes, that does show how badly I misread the first patch! For both, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On Mon, Jun 03, 2013 at 08:50:29AM +0100, Chris Wilson wrote: > On Sun, Jun 02, 2013 at 01:26:24PM +0200, Daniel Vetter wrote: > > Since this is run in the compute config stage we need to check > > the new_ pointers, i.e the stage output routing, not the current > > modeset layout. Also there was a little logic bug in properly skipping > > connectors: The old code did not skip any unused connectors and so > > clamped to whatever was left in there (usually 0 if that connector > > hasn't seen a EDID 1.4 screen ever since boot-up). > > > > This has been broken when moving the pipe bpp selection in > > > > commit 4e53c2e010e531b4a014692199e978482d471c7e > > Author: Daniel Vetter <daniel.vetter@ffwll.ch> > > Date: Wed Mar 27 00:44:58 2013 +0100 > > > > drm/i915: precompute pipe bpp before touching the hw > > > > To avoid too much casting switch from drm_ to intel_ types. > > > > Also add a bit of debug output to help reconstructing what's going > > on. > > > > v2: Try to clarify this a bit: > > - s/pipe_config_set_bpp/compute_baseline_pipe_bpp/ to make it clearer > > at which stage this function is run. Also add a comment about what > > it does. > > - Extract the sink clamping into it's own function. > > > > v3: Actually make it compile. > > > > v4: Split out all the prep refactoring to make the bugfix stick out > > really badly. Also elaborate a bit in the commit message about the > > nature of the bugfix. > > > > Cc: Chris Wilson <chris@chris-wilson.co.uk> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Yes, that does show how badly I misread the first patch! > For both, Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Both merged, thanks for the review. I'll ping QA to test the 30bpp patch, too. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 6539edb..d4b56cb 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7643,8 +7643,8 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc, /* Clamp display bpp to EDID value */ list_for_each_entry(connector, &dev->mode_config.connector_list, base.head) { - if (connector->base.encoder && - connector->base.encoder->crtc != crtc) + if (!connector->new_encoder || + connector->new_encoder->new_crtc != crtc) continue; connected_sink_compute_bpp(connector, pipe_config);
Since this is run in the compute config stage we need to check the new_ pointers, i.e the stage output routing, not the current modeset layout. Also there was a little logic bug in properly skipping connectors: The old code did not skip any unused connectors and so clamped to whatever was left in there (usually 0 if that connector hasn't seen a EDID 1.4 screen ever since boot-up). This has been broken when moving the pipe bpp selection in commit 4e53c2e010e531b4a014692199e978482d471c7e Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Wed Mar 27 00:44:58 2013 +0100 drm/i915: precompute pipe bpp before touching the hw To avoid too much casting switch from drm_ to intel_ types. Also add a bit of debug output to help reconstructing what's going on. v2: Try to clarify this a bit: - s/pipe_config_set_bpp/compute_baseline_pipe_bpp/ to make it clearer at which stage this function is run. Also add a comment about what it does. - Extract the sink clamping into it's own function. v3: Actually make it compile. v4: Split out all the prep refactoring to make the bugfix stick out really badly. Also elaborate a bit in the commit message about the nature of the bugfix. Cc: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)