On Mon, Oct 15, 2018 at 12:17:39PM +0100, Chris Wilson wrote:
> We try to avoid a deadlock of synchronizing the async fbdev task by
> skipping the synchronisation from the async worker, but that prevents us
> from using an async worker for the device probe. As we have our own
> barrier and do not rely on the global async flush, we can simply replace
> the async task with an explicit worker.
>
> References: 366e39b4d2c5 ("drm/i915: Tear down fbdev if initialization fails")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Does this now mean that fbdev setup lags behind module load? Afaiui that
will annoy userspace, which assumes that once the kms driver is loaded,
fbdev works. Or at least I have vague memories of pains in this area.
-Daniel
> ---
> drivers/gpu/drm/i915/intel_drv.h | 3 +--
> drivers/gpu/drm/i915/intel_fbdev.c | 35 +++++++++++++++---------------
> 2 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3dea7a1bda7f..15bbf604724d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -25,7 +25,6 @@
> #ifndef __INTEL_DRV_H__
> #define __INTEL_DRV_H__
>
> -#include <linux/async.h>
> #include <linux/i2c.h>
> #include <linux/hdmi.h>
> #include <linux/sched/clock.h>
> @@ -207,8 +206,8 @@ struct intel_fbdev {
> struct intel_framebuffer *fb;
> struct i915_vma *vma;
> unsigned long vma_flags;
> - async_cookie_t cookie;
> int preferred_bpp;
> + struct work_struct work;
> };
>
> struct intel_encoder {
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index 2480c7d6edee..265cc947aede 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -24,7 +24,6 @@
> * David Airlie
> */
>
> -#include <linux/async.h>
> #include <linux/module.h>
> #include <linux/kernel.h>
> #include <linux/console.h>
> @@ -666,6 +665,16 @@ static void intel_fbdev_suspend_worker(struct work_struct *work)
> true);
> }
>
> +static void intel_fbdev_initial_config(struct work_struct *work)
> +{
> + struct intel_fbdev *ifbdev = container_of(work, typeof(*ifbdev), work);
> +
> + /* Due to peculiar init order wrt to hpd handling this is separate. */
> + if (drm_fb_helper_initial_config(&ifbdev->helper,
> + ifbdev->preferred_bpp))
> + intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
> +}
> +
> int intel_fbdev_init(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> @@ -679,6 +688,8 @@ int intel_fbdev_init(struct drm_device *dev)
> if (ifbdev == NULL)
> return -ENOMEM;
>
> + INIT_WORK(&ifbdev->work, intel_fbdev_initial_config);
> +
> drm_fb_helper_prepare(dev, &ifbdev->helper, &intel_fb_helper_funcs);
>
> if (!intel_fbdev_init_bios(dev, ifbdev))
> @@ -698,16 +709,6 @@ int intel_fbdev_init(struct drm_device *dev)
> return 0;
> }
>
> -static void intel_fbdev_initial_config(void *data, async_cookie_t cookie)
> -{
> - struct intel_fbdev *ifbdev = data;
> -
> - /* Due to peculiar init order wrt to hpd handling this is separate. */
> - if (drm_fb_helper_initial_config(&ifbdev->helper,
> - ifbdev->preferred_bpp))
> - intel_fbdev_unregister(to_i915(ifbdev->helper.dev));
> -}
> -
> void intel_fbdev_initial_config_async(struct drm_device *dev)
> {
> struct intel_fbdev *ifbdev = to_i915(dev)->fbdev;
> @@ -715,17 +716,16 @@ void intel_fbdev_initial_config_async(struct drm_device *dev)
> if (!ifbdev)
> return;
>
> - ifbdev->cookie = async_schedule(intel_fbdev_initial_config, ifbdev);
> + schedule_work(&ifbdev->work);
> }
>
> static void intel_fbdev_sync(struct intel_fbdev *ifbdev)
> {
> - if (!ifbdev->cookie)
> + if (!READ_ONCE(ifbdev->work.func))
> return;
>
> - /* Only serialises with all preceding async calls, hence +1 */
> - async_synchronize_cookie(ifbdev->cookie + 1);
> - ifbdev->cookie = 0;
> + flush_work(&ifbdev->work);
> + ifbdev->work.func = NULL;
> }
>
> void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
> @@ -736,8 +736,7 @@ void intel_fbdev_unregister(struct drm_i915_private *dev_priv)
> return;
>
> cancel_work_sync(&dev_priv->fbdev_suspend_work);
> - if (!current_is_async())
> - intel_fbdev_sync(ifbdev);
> + intel_fbdev_sync(ifbdev);
>
> drm_fb_helper_unregister_fbi(&ifbdev->helper);
> }
> --
> 2.19.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
