| Message ID | 1439206448-8938-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Aug 10, 2015 at 01:34:08PM +0200, Daniel Vetter wrote: > Instead of our own duplicated one. This fixes a bug in the driver > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > try to unregister the nonexistent fbdev drm_framebuffer. > > Cc: Archit Taneja <architt@codeaurora.org> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/Kconfig | 15 --------------- > drivers/gpu/drm/i915/Makefile | 2 +- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > 7 files changed, 8 insertions(+), 23 deletions(-) Isn't this going to cause some pain to users because .config may not have this symbol yet? Arguably this is somewhat mitigated by the fact that both symbols are "default y", but technically somebody could have DRM_I915_FBDEV=n in their .config and after this change fbdev emulation will be switched on again. I'm not sure how to upgrade more sanely, though, so perhaps this is just a bullet that needs biting. Thierry
On Mon, Aug 10, 2015 at 01:48:53PM +0200, Thierry Reding wrote: > On Mon, Aug 10, 2015 at 01:34:08PM +0200, Daniel Vetter wrote: > > Instead of our own duplicated one. This fixes a bug in the driver > > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > > try to unregister the nonexistent fbdev drm_framebuffer. > > > > Cc: Archit Taneja <architt@codeaurora.org> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/i915/Kconfig | 15 --------------- > > drivers/gpu/drm/i915/Makefile | 2 +- > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > 7 files changed, 8 insertions(+), 23 deletions(-) > > Isn't this going to cause some pain to users because .config may not > have this symbol yet? Arguably this is somewhat mitigated by the fact > that both symbols are "default y", but technically somebody could have > DRM_I915_FBDEV=n in their .config and after this change fbdev emulation > will be switched on again. > > I'm not sure how to upgrade more sanely, though, so perhaps this is just > a bullet that needs biting. There are other drivers two with their private FBDEV option (like msm) so I don't think we can do any sensible upgrade logic that just works. -Daniel
Op 10-08-15 om 13:34 schreef Daniel Vetter: > Instead of our own duplicated one. This fixes a bug in the driver > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > try to unregister the nonexistent fbdev drm_framebuffer. > > Cc: Archit Taneja <architt@codeaurora.org> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/Kconfig | 15 --------------- > drivers/gpu/drm/i915/Makefile | 2 +- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > 7 files changed, 8 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > index eb87e2538861..051eab33e4c7 100644 > --- a/drivers/gpu/drm/i915/Kconfig > +++ b/drivers/gpu/drm/i915/Kconfig > @@ -36,21 +36,6 @@ config DRM_I915 > i810 driver instead, and the Atom z5xx series has an entirely > different implementation. > > -config DRM_I915_FBDEV > - bool "Enable legacy fbdev support for the modesetting intel driver" > - depends on DRM_I915 > - select DRM_KMS_FB_HELPER > - select FB_CFB_FILLRECT > - select FB_CFB_COPYAREA > - select FB_CFB_IMAGEBLIT > - default y > - help > - Choose this option if you have a need for the legacy fbdev > - support. Note that this support also provide the linux console > - support on top of the intel modesetting driver. > - > - If in doubt, say "Y". > - > config DRM_I915_PRELIMINARY_HW_SUPPORT > bool "Enable preliminary support for prerelease Intel hardware by default" > depends on DRM_I915 > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 41fb8a9c5bef..998b4643109f 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -62,7 +62,7 @@ i915-y += intel_audio.o \ > intel_sideband.o \ > intel_sprite.o > i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o > -i915-$(CONFIG_DRM_I915_FBDEV) += intel_fbdev.o > +i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o > > # modesetting output/encoder code > i915-y += dvo_ch7017.o \ > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 95e7b82f05d1..86734be84a65 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -1868,7 +1868,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data) > struct intel_framebuffer *fb; > struct drm_framebuffer *drm_fb; > > -#ifdef CONFIG_DRM_I915_FBDEV > +#ifdef CONFIG_DRM_FBDEV_EMULATION > struct drm_i915_private *dev_priv = dev->dev_private; > > ifbdev = dev_priv->fbdev; > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 4932d298e3af..95c115f6a4c7 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1855,7 +1855,7 @@ struct drm_i915_private { > > struct drm_i915_gem_object *vlv_pctx; > > -#ifdef CONFIG_DRM_I915_FBDEV > +#ifdef CONFIG_DRM_FBDEV_EMULATION > /* list of fbdev register on this device */ > struct intel_fbdev *fbdev; > struct work_struct fbdev_suspend_work; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 16c8052f7eab..9a2f229a1c3a 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -10131,7 +10131,7 @@ static struct drm_framebuffer * > mode_fits_in_fbdev(struct drm_device *dev, > struct drm_display_mode *mode) > { > -#ifdef CONFIG_DRM_I915_FBDEV > +#ifdef CONFIG_DRM_FBDEV_EMULATION > struct drm_i915_private *dev_priv = dev->dev_private; > struct drm_i915_gem_object *obj; > struct drm_framebuffer *fb; > @@ -14313,7 +14313,7 @@ intel_user_framebuffer_create(struct drm_device *dev, > return intel_framebuffer_create(dev, mode_cmd, obj); > } > > -#ifndef CONFIG_DRM_I915_FBDEV > +#ifndef CONFIG_DRM_FBDEV_EMULATION > static inline void intel_fbdev_output_poll_changed(struct drm_device *dev) > { > } > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c > index f4fe1183bae6..369f8b6b804f 100644 > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > @@ -406,7 +406,7 @@ static bool intel_dp_mst_get_hw_state(struct intel_connector *connector) > > static void intel_connector_add_to_fbdev(struct intel_connector *connector) > { > -#ifdef CONFIG_DRM_I915_FBDEV > +#ifdef CONFIG_DRM_FBDEV_EMULATION > struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper, &connector->base); > #endif > @@ -414,7 +414,7 @@ static void intel_connector_add_to_fbdev(struct intel_connector *connector) > > static void intel_connector_remove_from_fbdev(struct intel_connector *connector) > { > -#ifdef CONFIG_DRM_I915_FBDEV > +#ifdef CONFIG_DRM_FBDEV_EMULATION > struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > drm_fb_helper_remove_one_connector(&dev_priv->fbdev->helper, &connector->base); > #endif > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 6b3e2c6e6c54..80aaed6d4c0b 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1205,7 +1205,7 @@ void intel_dvo_init(struct drm_device *dev); > > > /* legacy fbdev emulation in intel_fbdev.c */ > -#ifdef CONFIG_DRM_I915_FBDEV > +#ifdef CONFIG_DRM_FBDEV_EMULATION > extern int intel_fbdev_init(struct drm_device *dev); > extern void intel_fbdev_initial_config(void *data, async_cookie_t cookie); > extern void intel_fbdev_fini(struct drm_device *dev);
Op 10-08-15 om 13:34 schreef Daniel Vetter: > Instead of our own duplicated one. This fixes a bug in the driver > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > try to unregister the nonexistent fbdev drm_framebuffer. > > Cc: Archit Taneja <architt@codeaurora.org> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/Kconfig | 15 --------------- > drivers/gpu/drm/i915/Makefile | 2 +- > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > drivers/gpu/drm/i915/intel_drv.h | 2 +- > 7 files changed, 8 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > index eb87e2538861..051eab33e4c7 100644 > --- a/drivers/gpu/drm/i915/Kconfig > +++ b/drivers/gpu/drm/i915/Kconfig > @@ -36,21 +36,6 @@ config DRM_I915 > i810 driver instead, and the Atom z5xx series has an entirely > different implementation. > > -config DRM_I915_FBDEV > - bool "Enable legacy fbdev support for the modesetting intel driver" > - depends on DRM_I915 > - select DRM_KMS_FB_HELPER > - select FB_CFB_FILLRECT > - select FB_CFB_COPYAREA > - select FB_CFB_IMAGEBLIT > Did some testing and didn't find a way to break this. Has the bug been fixed where if some option selects X that selects Y it also had to select Y? Oh well, have my r-b. :-) Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> ~Maarten
On Tue, Aug 11, 2015 at 01:25:08PM +0200, Maarten Lankhorst wrote: > Op 10-08-15 om 13:34 schreef Daniel Vetter: > > Instead of our own duplicated one. This fixes a bug in the driver > > unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we > > try to unregister the nonexistent fbdev drm_framebuffer. > > > > Cc: Archit Taneja <architt@codeaurora.org> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/i915/Kconfig | 15 --------------- > > drivers/gpu/drm/i915/Makefile | 2 +- > > drivers/gpu/drm/i915/i915_debugfs.c | 2 +- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > > drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- > > drivers/gpu/drm/i915/intel_drv.h | 2 +- > > 7 files changed, 8 insertions(+), 23 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig > > index eb87e2538861..051eab33e4c7 100644 > > --- a/drivers/gpu/drm/i915/Kconfig > > +++ b/drivers/gpu/drm/i915/Kconfig > > @@ -36,21 +36,6 @@ config DRM_I915 > > i810 driver instead, and the Atom z5xx series has an entirely > > different implementation. > > > > -config DRM_I915_FBDEV > > - bool "Enable legacy fbdev support for the modesetting intel driver" > > - depends on DRM_I915 > > - select DRM_KMS_FB_HELPER > > - select FB_CFB_FILLRECT > > - select FB_CFB_COPYAREA > > - select FB_CFB_IMAGEBLIT > > > Did some testing and didn't find a way to break this. Has the bug been fixed where if some > option selects X that selects Y it also had to select Y? Nope, but DRM_FBDEV_EMULATION should carry all the same selects as we did. > > Oh well, have my r-b. :-) > > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Thanks, applied to drm-misc. -Daniel
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig index eb87e2538861..051eab33e4c7 100644 --- a/drivers/gpu/drm/i915/Kconfig +++ b/drivers/gpu/drm/i915/Kconfig @@ -36,21 +36,6 @@ config DRM_I915 i810 driver instead, and the Atom z5xx series has an entirely different implementation. -config DRM_I915_FBDEV - bool "Enable legacy fbdev support for the modesetting intel driver" - depends on DRM_I915 - select DRM_KMS_FB_HELPER - select FB_CFB_FILLRECT - select FB_CFB_COPYAREA - select FB_CFB_IMAGEBLIT - default y - help - Choose this option if you have a need for the legacy fbdev - support. Note that this support also provide the linux console - support on top of the intel modesetting driver. - - If in doubt, say "Y". - config DRM_I915_PRELIMINARY_HW_SUPPORT bool "Enable preliminary support for prerelease Intel hardware by default" depends on DRM_I915 diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 41fb8a9c5bef..998b4643109f 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -62,7 +62,7 @@ i915-y += intel_audio.o \ intel_sideband.o \ intel_sprite.o i915-$(CONFIG_ACPI) += intel_acpi.o intel_opregion.o -i915-$(CONFIG_DRM_I915_FBDEV) += intel_fbdev.o +i915-$(CONFIG_DRM_FBDEV_EMULATION) += intel_fbdev.o # modesetting output/encoder code i915-y += dvo_ch7017.o \ diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 95e7b82f05d1..86734be84a65 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -1868,7 +1868,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data) struct intel_framebuffer *fb; struct drm_framebuffer *drm_fb; -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION struct drm_i915_private *dev_priv = dev->dev_private; ifbdev = dev_priv->fbdev; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4932d298e3af..95c115f6a4c7 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1855,7 +1855,7 @@ struct drm_i915_private { struct drm_i915_gem_object *vlv_pctx; -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION /* list of fbdev register on this device */ struct intel_fbdev *fbdev; struct work_struct fbdev_suspend_work; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 16c8052f7eab..9a2f229a1c3a 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -10131,7 +10131,7 @@ static struct drm_framebuffer * mode_fits_in_fbdev(struct drm_device *dev, struct drm_display_mode *mode) { -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; struct drm_framebuffer *fb; @@ -14313,7 +14313,7 @@ intel_user_framebuffer_create(struct drm_device *dev, return intel_framebuffer_create(dev, mode_cmd, obj); } -#ifndef CONFIG_DRM_I915_FBDEV +#ifndef CONFIG_DRM_FBDEV_EMULATION static inline void intel_fbdev_output_poll_changed(struct drm_device *dev) { } diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index f4fe1183bae6..369f8b6b804f 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -406,7 +406,7 @@ static bool intel_dp_mst_get_hw_state(struct intel_connector *connector) static void intel_connector_add_to_fbdev(struct intel_connector *connector) { -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION struct drm_i915_private *dev_priv = to_i915(connector->base.dev); drm_fb_helper_add_one_connector(&dev_priv->fbdev->helper, &connector->base); #endif @@ -414,7 +414,7 @@ static void intel_connector_add_to_fbdev(struct intel_connector *connector) static void intel_connector_remove_from_fbdev(struct intel_connector *connector) { -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION struct drm_i915_private *dev_priv = to_i915(connector->base.dev); drm_fb_helper_remove_one_connector(&dev_priv->fbdev->helper, &connector->base); #endif diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 6b3e2c6e6c54..80aaed6d4c0b 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1205,7 +1205,7 @@ void intel_dvo_init(struct drm_device *dev); /* legacy fbdev emulation in intel_fbdev.c */ -#ifdef CONFIG_DRM_I915_FBDEV +#ifdef CONFIG_DRM_FBDEV_EMULATION extern int intel_fbdev_init(struct drm_device *dev); extern void intel_fbdev_initial_config(void *data, async_cookie_t cookie); extern void intel_fbdev_fini(struct drm_device *dev);
Instead of our own duplicated one. This fixes a bug in the driver unload code if DRM_FBDEV_EMULATION=n but DRM_I915_FBDEV=y because we try to unregister the nonexistent fbdev drm_framebuffer. Cc: Archit Taneja <architt@codeaurora.org> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Reported-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/Kconfig | 15 --------------- drivers/gpu/drm/i915/Makefile | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_display.c | 4 ++-- drivers/gpu/drm/i915/intel_dp_mst.c | 4 ++-- drivers/gpu/drm/i915/intel_drv.h | 2 +- 7 files changed, 8 insertions(+), 23 deletions(-)