| Message ID | 1370108756-1948-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Jun 01, 2013 at 07:45:55PM +0200, Daniel Vetter wrote: > Since this is run in the compute config stage we need to check > the new_ pointers, not the current modeset layout. Also there > was a little logic bug in properly skipping connectors. 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. Baffling. It is really hard to tell what is going on, so perhaps it is worth investing in some more descriptive names. How about: struct drm_display_info *desired = &connector->base.display_info ? Or some other name? And an indication in the function name at which stage this is meant to run? -Chris
On Sat, Jun 01, 2013 at 06:52:39PM +0100, Chris Wilson wrote: > On Sat, Jun 01, 2013 at 07:45:55PM +0200, Daniel Vetter wrote: > > Since this is run in the compute config stage we need to check > > the new_ pointers, not the current modeset layout. Also there > > was a little logic bug in properly skipping connectors. 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. > > Baffling. It is really hard to tell what is going on, so perhaps it is > worth investing in some more descriptive names. How about: > struct drm_display_info *desired = &connector->base.display_info ? > Or some other name? And an indication in the function name at which > stage this is meant to run? I guess renaming the function to pipe_config_compute_bpp would help (since it doesn't really set anything). And maybe extracting the sink based bpp limiting into it's own function since it's a bit magic. I'll see whether I can polish this turd some more. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f410ede..3f24d7f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -7568,12 +7568,12 @@ static void intel_modeset_commit_output_state(struct drm_device *dev) } static int -pipe_config_set_bpp(struct drm_crtc *crtc, +pipe_config_set_bpp(struct intel_crtc *crtc, struct drm_framebuffer *fb, struct intel_crtc_config *pipe_config) { - struct drm_device *dev = crtc->dev; - struct drm_connector *connector; + struct drm_device *dev = crtc->base.dev; + struct intel_connector *connector; int bpp; switch (fb->pixel_format) { @@ -7616,20 +7616,25 @@ pipe_config_set_bpp(struct drm_crtc *crtc, /* Clamp display bpp to EDID value */ list_for_each_entry(connector, &dev->mode_config.connector_list, - head) { - if (connector->encoder && connector->encoder->crtc != crtc) + base.head) { + if (!connector->new_encoder || + connector->new_encoder->new_crtc != crtc) continue; + DRM_DEBUG_KMS("[CONNECTOR:%d:%s] checking for sink bpp constrains\n", + connector->base.base.id, + drm_get_connector_name(&connector->base)); + /* Don't use an invalid EDID bpc value */ - if (connector->display_info.bpc && - connector->display_info.bpc * 3 < bpp) { + if (connector->base.display_info.bpc && + connector->base.display_info.bpc * 3 < bpp) { DRM_DEBUG_KMS("clamping display bpp (was %d) to EDID reported max of %d\n", - bpp, connector->display_info.bpc*3); - pipe_config->pipe_bpp = connector->display_info.bpc*3; + bpp, connector->base.display_info.bpc*3); + pipe_config->pipe_bpp = connector->base.display_info.bpc*3; } /* Clamp bpp to 8 on screens without EDID 1.4 */ - if (connector->display_info.bpc == 0 && bpp > 24) { + if (connector->base.display_info.bpc == 0 && bpp > 24) { DRM_DEBUG_KMS("clamping display bpp (was %d) to default limit of 24\n", bpp); pipe_config->pipe_bpp = 24; @@ -7714,7 +7719,8 @@ intel_modeset_pipe_config(struct drm_crtc *crtc, pipe_config->cpu_transcoder = to_intel_crtc(crtc)->pipe; pipe_config->shared_dpll = DPLL_ID_PRIVATE; - plane_bpp = pipe_config_set_bpp(crtc, fb, pipe_config); + plane_bpp = pipe_config_set_bpp(to_intel_crtc(crtc), + fb, pipe_config); if (plane_bpp < 0) goto fail;
Since this is run in the compute config stage we need to check the new_ pointers, not the current modeset layout. Also there was a little logic bug in properly skipping connectors. 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. Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-)