On Mon, Feb 15, 2016 at 10:47 AM, Jyri Sarha <jsarha@xxxxxx> wrote: > On 02/12/16 19:15, Ville Syrjälä wrote: >> >> On Fri, Feb 12, 2016 at 07:08:46PM +0200, Tomi Valkeinen wrote: >>> >>> On 12/02/16 17:34, Daniel Vetter wrote: >>>> >>>> On Fri, Feb 12, 2016 at 05:17:19PM +0200, Jyri Sarha wrote: >>>>> >>>>> Fall back to legacy drm_helper_connector_dpms() call back in >>>>> drm_atomic_helper_connector_dpms() if DRIVER_ATOMIC feature is not >>>>> present. >>>>> >>>>> Calling drm_atomic_helper_connector_dpms() from non atomic driver >>>>> causes undefined behavior. This is a problem with componentized >>>>> encoder/connector drivers that may be bound to both atomic and >>>>> non atomic drivers.drm-next-tilcdc-fixes >>>>> >>>>> Signed-off-by: Jyri Sarha <jsarha@xxxxxx> >>>>> --- >>>>> This is just an alternative to this: >>>>> >>>>> https://lists.freedesktop.org/archives/dri-devel/2016-January/098867.html >>>> >>>> >>>> I like the linked patch much better, since non-atomic really should die. >>>> Inflicting non-atomic on atomic helpers is bad imo, so nack from me on >>>> this patch here. >>> >>> >>> I mostly agree, but we are in a transition period from non-atomic to >>> atomic. When all drivers have been converted to atomic, it should be >>> easy to grep for code using DRIVER_ATOMIC (or similar), and remove all >>> the non-atomic support code. >>> >>> In my opinion it should be considered case by case if the >>> non-atomic/atomic compat code should be added to the generic functions >>> or to all the drivers using that functionality. >>> >>> In this case, if it only affects tda998x (but does it?), perhaps the >>> patch in the link is better. >> >> >> Maybe we need a hybrid helper that just calls the atomic >> or non-atomic helper approppriately. >> > > Would it actually be better if the legacy callback would automatically call > atomic version if the driver has DRIVER_ATOMIC set? > > I do not have any strong opinions on this, just as long as one of the > patches gets merged. With quick grepping I can see that there are several > DRM component drivers, but I have no idea if anything else but the tda998x > is being used by more than one DRM master driver. I prefer open-coding since atomic vs. legacy is just one part of the entire problem that the last element in the chain needs to register the drm_connector, but in coordination with everyone else in the chain. E.g. if you have upscale properties or anything else like that it gets a lot more complicated, especially around the question of "who owns drm_connector_state". Given that I think it's better to add driver-private code for special cases until we have a few examples and can go about creating a useful helper library for this problem space. Instead of an inconsistent pile of hacks enshrined forever. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
