On Fri, Sep 28, 2012 at 01:55:19PM +0200, Daniel Vetter wrote: > On Fri, Sep 28, 2012 at 12:54 PM, Rob Clark <rob.clark@xxxxxxxxxx> wrote: > > Maybe it just makes sense to always do connector->dpms(OFF) before > > unhooking the chain, rather than directly calling dpms on the > > encoder/crtc? > > Well, that makes the entire (optional) ->disable stuff a bit more > awkward. The thing imo really is that the crtc helper assume that the > connectors do not hold any hw state, whereas you're omap driver > violates that assumption. > > For the intel driver we've fixed this by doing any sink handling (e.g. > for dp) in the encoder->dpms functions. So I think the right way for > omap is to do the same (or conclude that the crtc helpers are a bad > fit and ditch them). Having connectors that are special, but only in > some of the cases imo makes squat sense for a generic helper library. > > >> 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. > > I see a few ways forward with the crtc helpers and atomic modeset: > - bake the assumption into the code that drivers using the crtc > helpers don't have any shared global resources and drop all these > checks. This would boil down to writing a new set_config to handle > global modeset changes (instead of changes to just one crtc). > Obviously the current possible_encoders/possible_crtcs would still be > checked. > - like above, but add a driver-global ->check callback before starting > the modeset sequence, but with the new configuration already applied > to the kms structures. That's essentially what my intel_atomic.c code does. Somewhat ironically, since your modeset rework, that code is now more suited for other drivers and I need to go and rewrite it for i915. -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
