* [PATCH RFC] drm/atomic: Fall back to legacy call back in *_connector_dpms() @ 2016-02-12 15:17 Jyri Sarha 2016-02-12 15:34 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Jyri Sarha @ 2016-02-12 15:17 UTC (permalink / raw) To: dri-devel; +Cc: rmk+kernel, tomi.valkeinen, Jyri Sarha 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. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- This is just an alternative to this: https://lists.freedesktop.org/archives/dri-devel/2016-January/098867.html Either one of the patches should be taken for Beaglebone-black, or any other non atomic driver using tda998x, to work. drivers/gpu/drm/drm_atomic_helper.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 57cccd6..6ad0303 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -2344,6 +2344,9 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector, bool active = false; int old_mode = connector->dpms; + if (!drm_core_check_feature(connector->dev, DRIVER_ATOMIC)) + return drm_helper_connector_dpms(connector, mode); + if (mode != DRM_MODE_DPMS_ON) mode = DRM_MODE_DPMS_OFF; -- 1.9.1 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] drm/atomic: Fall back to legacy call back in *_connector_dpms() 2016-02-12 15:17 [PATCH RFC] drm/atomic: Fall back to legacy call back in *_connector_dpms() Jyri Sarha @ 2016-02-12 15:34 ` Daniel Vetter 2016-02-12 17:08 ` Tomi Valkeinen 0 siblings, 1 reply; 6+ messages in thread From: Daniel Vetter @ 2016-02-12 15:34 UTC (permalink / raw) To: Jyri Sarha; +Cc: rmk+kernel, tomi.valkeinen, dri-devel 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. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > 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. -Daniel > > Either one of the patches should be taken for Beaglebone-black, or any > other non atomic driver using tda998x, to work. > > drivers/gpu/drm/drm_atomic_helper.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > index 57cccd6..6ad0303 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2344,6 +2344,9 @@ int drm_atomic_helper_connector_dpms(struct drm_connector *connector, > bool active = false; > int old_mode = connector->dpms; > > + if (!drm_core_check_feature(connector->dev, DRIVER_ATOMIC)) > + return drm_helper_connector_dpms(connector, mode); > + > if (mode != DRM_MODE_DPMS_ON) > mode = DRM_MODE_DPMS_OFF; > > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] drm/atomic: Fall back to legacy call back in *_connector_dpms() 2016-02-12 15:34 ` Daniel Vetter @ 2016-02-12 17:08 ` Tomi Valkeinen 2016-02-12 17:15 ` Ville Syrjälä 0 siblings, 1 reply; 6+ messages in thread From: Tomi Valkeinen @ 2016-02-12 17:08 UTC (permalink / raw) To: Daniel Vetter, Jyri Sarha; +Cc: rmk+kernel, dri-devel [-- Attachment #1.1: Type: text/plain, Size: 1376 bytes --] 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. >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> >> --- >> 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. Tomi [-- Attachment #1.2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] [-- Attachment #2: Type: text/plain, Size: 160 bytes --] _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] drm/atomic: Fall back to legacy call back in *_connector_dpms() 2016-02-12 17:08 ` Tomi Valkeinen @ 2016-02-12 17:15 ` Ville Syrjälä 2016-02-15 9:47 ` Jyri Sarha 0 siblings, 1 reply; 6+ messages in thread From: Ville Syrjälä @ 2016-02-12 17:15 UTC (permalink / raw) To: Tomi Valkeinen; +Cc: rmk+kernel, dri-devel, Jyri Sarha 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. > >> > >> Signed-off-by: Jyri Sarha <jsarha@ti.com> > >> --- > >> 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. -- Ville Syrjälä Intel OTC _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] drm/atomic: Fall back to legacy call back in *_connector_dpms() 2016-02-12 17:15 ` Ville Syrjälä @ 2016-02-15 9:47 ` Jyri Sarha 2016-02-15 11:09 ` Daniel Vetter 0 siblings, 1 reply; 6+ messages in thread From: Jyri Sarha @ 2016-02-15 9:47 UTC (permalink / raw) To: Ville Syrjälä, Tomi Valkeinen; +Cc: rmk+kernel, dri-devel 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@ti.com> >>>> --- >>>> 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. Best regards, Jyri _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] drm/atomic: Fall back to legacy call back in *_connector_dpms() 2016-02-15 9:47 ` Jyri Sarha @ 2016-02-15 11:09 ` Daniel Vetter 0 siblings, 0 replies; 6+ messages in thread From: Daniel Vetter @ 2016-02-15 11:09 UTC (permalink / raw) To: Jyri Sarha; +Cc: Russell King, Tomi Valkeinen, dri-devel On Mon, Feb 15, 2016 at 10:47 AM, Jyri Sarha <jsarha@ti.com> 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@ti.com> >>>>> --- >>>>> 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-02-15 11:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-02-12 15:17 [PATCH RFC] drm/atomic: Fall back to legacy call back in *_connector_dpms() Jyri Sarha 2016-02-12 15:34 ` Daniel Vetter 2016-02-12 17:08 ` Tomi Valkeinen 2016-02-12 17:15 ` Ville Syrjälä 2016-02-15 9:47 ` Jyri Sarha 2016-02-15 11:09 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.