| Message ID | 1465388359-8070-26-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Daniel, Thank you for the patch. On Wednesday 08 Jun 2016 14:19:18 Daniel Vetter wrote: > Rockchip just blew up here on testing, because I removed some "is this > crtc already disabled/enabled" state tracking from callbacks (not needed > with atomic). Turns out that was needed to work around rockchip still > calling legacy helper code. > > Since me explaining on irc/mailing-list plus kerneldoc isn't enough, > be more verbose and add dmesg output. Not that anyone actually reads that, > either. How about also removing it from atomic drivers then ? At least omapdrm calls this function. > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_crtc_helper.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > b/drivers/gpu/drm/drm_crtc_helper.c index a6e42433ef0e..b47ec24939a0 100644 > --- a/drivers/gpu/drm/drm_crtc_helper.c > +++ b/drivers/gpu/drm/drm_crtc_helper.c > @@ -232,6 +232,9 @@ static void __drm_helper_disable_unused_functions(struct > drm_device *dev) */ > void drm_helper_disable_unused_functions(struct drm_device *dev) > { > + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) > + DRM_ERROR("Called for atomic driver, this is not what you want.\n"); > + > drm_modeset_lock_all(dev); > __drm_helper_disable_unused_functions(dev); > drm_modeset_unlock_all(dev);
On Thu, Jun 09, 2016 at 01:36:30AM +0300, Laurent Pinchart wrote: > Hi Daniel, > > Thank you for the patch. > > On Wednesday 08 Jun 2016 14:19:18 Daniel Vetter wrote: > > Rockchip just blew up here on testing, because I removed some "is this > > crtc already disabled/enabled" state tracking from callbacks (not needed > > with atomic). Turns out that was needed to work around rockchip still > > calling legacy helper code. > > > > Since me explaining on irc/mailing-list plus kerneldoc isn't enough, > > be more verbose and add dmesg output. Not that anyone actually reads that, > > either. > > How about also removing it from atomic drivers then ? At least omapdrm calls > this function. I tried, and thought I caught them all. See preceeding sti patch. Not sure why I missed it, will remedy. -Daniel > > > Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > --- > > drivers/gpu/drm/drm_crtc_helper.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_crtc_helper.c > > b/drivers/gpu/drm/drm_crtc_helper.c index a6e42433ef0e..b47ec24939a0 100644 > > --- a/drivers/gpu/drm/drm_crtc_helper.c > > +++ b/drivers/gpu/drm/drm_crtc_helper.c > > @@ -232,6 +232,9 @@ static void __drm_helper_disable_unused_functions(struct > > drm_device *dev) */ > > void drm_helper_disable_unused_functions(struct drm_device *dev) > > { > > + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) > > + DRM_ERROR("Called for atomic driver, this is not what you want.\n"); > > + > > drm_modeset_lock_all(dev); > > __drm_helper_disable_unused_functions(dev); > > drm_modeset_unlock_all(dev); > > -- > Regards, > > Laurent Pinchart >
Hi Daniel, On Thursday 09 Jun 2016 10:26:44 Daniel Vetter wrote: > On Thu, Jun 09, 2016 at 01:36:30AM +0300, Laurent Pinchart wrote: > > On Wednesday 08 Jun 2016 14:19:18 Daniel Vetter wrote: > >> Rockchip just blew up here on testing, because I removed some "is this > >> crtc already disabled/enabled" state tracking from callbacks (not needed > >> with atomic). Turns out that was needed to work around rockchip still > >> calling legacy helper code. > >> > >> Since me explaining on irc/mailing-list plus kerneldoc isn't enough, > >> be more verbose and add dmesg output. Not that anyone actually reads > >> that, > >> either. > > > > How about also removing it from atomic drivers then ? At least omapdrm > > calls this function. > > I tried, and thought I caught them all. See preceeding sti patch. Not sure > why I missed it, will remedy. With the call removed from drivers, Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > >> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > >> --- > >> > >> drivers/gpu/drm/drm_crtc_helper.c | 3 +++ > >> 1 file changed, 3 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c > >> b/drivers/gpu/drm/drm_crtc_helper.c index a6e42433ef0e..b47ec24939a0 > >> 100644 > >> --- a/drivers/gpu/drm/drm_crtc_helper.c > >> +++ b/drivers/gpu/drm/drm_crtc_helper.c > >> @@ -232,6 +232,9 @@ static void > >> __drm_helper_disable_unused_functions(struct drm_device *dev) */ > >> > >> void drm_helper_disable_unused_functions(struct drm_device *dev) > >> { > >> + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) > >> + DRM_ERROR("Called for atomic driver, this is not what you > >> want.\n"); > >> + > >> drm_modeset_lock_all(dev); > >> __drm_helper_disable_unused_functions(dev); > >> drm_modeset_unlock_all(dev);
On Tue, Jun 21, 2016 at 12:12:54PM +0300, Laurent Pinchart wrote: > Hi Daniel, > > On Thursday 09 Jun 2016 10:26:44 Daniel Vetter wrote: > > On Thu, Jun 09, 2016 at 01:36:30AM +0300, Laurent Pinchart wrote: > > > On Wednesday 08 Jun 2016 14:19:18 Daniel Vetter wrote: > > >> Rockchip just blew up here on testing, because I removed some "is this > > >> crtc already disabled/enabled" state tracking from callbacks (not needed > > >> with atomic). Turns out that was needed to work around rockchip still > > >> calling legacy helper code. > > >> > > >> Since me explaining on irc/mailing-list plus kerneldoc isn't enough, > > >> be more verbose and add dmesg output. Not that anyone actually reads > > >> that, > > >> either. > > > > > > How about also removing it from atomic drivers then ? At least omapdrm > > > calls this function. > > > > I tried, and thought I caught them all. See preceeding sti patch. Not sure > > why I missed it, will remedy. > > With the call removed from drivers, > > Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Applied to drm-misc, thanks for double-checking. -Daniel > > > >> Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> > > >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > >> --- > > >> > > >> drivers/gpu/drm/drm_crtc_helper.c | 3 +++ > > >> 1 file changed, 3 insertions(+) > > >> > > >> diff --git a/drivers/gpu/drm/drm_crtc_helper.c > > >> b/drivers/gpu/drm/drm_crtc_helper.c index a6e42433ef0e..b47ec24939a0 > > >> 100644 > > >> --- a/drivers/gpu/drm/drm_crtc_helper.c > > >> +++ b/drivers/gpu/drm/drm_crtc_helper.c > > >> @@ -232,6 +232,9 @@ static void > > >> __drm_helper_disable_unused_functions(struct drm_device *dev) */ > > >> > > >> void drm_helper_disable_unused_functions(struct drm_device *dev) > > >> { > > >> + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) > > >> + DRM_ERROR("Called for atomic driver, this is not what you > > >> want.\n"); > > >> + > > >> drm_modeset_lock_all(dev); > > >> __drm_helper_disable_unused_functions(dev); > > >> drm_modeset_unlock_all(dev); > > -- > Regards, > > Laurent Pinchart >
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index a6e42433ef0e..b47ec24939a0 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -232,6 +232,9 @@ static void __drm_helper_disable_unused_functions(struct drm_device *dev) */ void drm_helper_disable_unused_functions(struct drm_device *dev) { + if (drm_core_check_feature(dev, DRIVER_ATOMIC)) + DRM_ERROR("Called for atomic driver, this is not what you want.\n"); + drm_modeset_lock_all(dev); __drm_helper_disable_unused_functions(dev); drm_modeset_unlock_all(dev);
Rockchip just blew up here on testing, because I removed some "is this crtc already disabled/enabled" state tracking from callbacks (not needed with atomic). Turns out that was needed to work around rockchip still calling legacy helper code. Since me explaining on irc/mailing-list plus kerneldoc isn't enough, be more verbose and add dmesg output. Not that anyone actually reads that, either. Cc: Tomeu Vizoso <tomeu.vizoso@collabora.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/drm_crtc_helper.c | 3 +++ 1 file changed, 3 insertions(+)