On Fri, Sep 28, 2012 at 11:56 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Fri, Sep 28, 2012 at 09:27:18AM +0200, Rob Clark wrote:
From: Rob Clark rob@ti.com
When disabling unused connectors, be sure to call connector->dpms(OFF), so if there is actually some IP to turn off (such as external bridge chips, etc), these actually do get turned off.
Signed-off-by: Rob Clark rob@ti.com Tested-by: Roger Quadros rogerq@ti.com
I think this runs counter to the crtc helper design: connectors are pretty much just dummy entities, and any dpms handling is done in the encoders/crtcs. If you call connector->funcs->dpms you don't call a crtc helper function, but actually the official dpms interface (which then again calls down into the encoder/crtc dpms callbacks (which are again crtc helper private). This functions are called again a few lines down in disable_unused_functions anyway, so the only way this changes behaviour is if you already overwrite the dpms interface and don't directly use drm_helper_connector_dpms.
yeah, I do this.. I have my own fxn in the connector which does some stuff, and then calls drm_helper_connector_dpms().
Maybe it just makes sense to always do connector->dpms(OFF) before unhooking the chain, rather than directly calling dpms on the encoder/crtc?
Which is imo a clear sign that the crtc helper is a misfit for your hw and you should stop using it ;-) And adding special-case handling like this into common code, especially if it weakens the semantic concepts in the helper layer is a recipe for a maintainance disaster a few years down the road. Hence
well, I think by the time we start adding atomic-modeset and common/dsi panel framework, there might be need for a new set of helpers.. but I'm not sure that the hw is quite strange enough to need an omap specific set of helpers. Maybe I start w/ something in omapdrm but then refactor into common functions once a few drivers are converted to atomic modeset and panel framework.
BR, -R
NACKed-by: Daniel Vetter daniel.vetter@ffwll.ch
Cheers, Daniel
drivers/gpu/drm/drm_crtc_helper.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 3252e70..000cda4 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -244,16 +244,16 @@ void drm_helper_disable_unused_functions(struct drm_device *dev) struct drm_crtc *crtc;
list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
if (!connector->encoder)continue; if (connector->status == connector_status_disconnected) connector->encoder = NULL;
if (!connector->encoder)connector->funcs->dpms(connector, DRM_MODE_DPMS_OFF); } list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) { if (!drm_helper_encoder_in_use(encoder)) { drm_encoder_disable(encoder);
/* disconnector encoder from any connector */
/* disconnect encoder from any connector */ encoder->crtc = NULL; } }-- 1.7.9.5
dri-devel mailing list
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel