* Re: [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev
@ 2016-03-03 18:30 Lukas Wunner
2016-03-04 10:17 ` Jani Nikula
2016-03-04 12:16 ` Damien Lespiau
0 siblings, 2 replies; 4+ messages in thread
From: Lukas Wunner @ 2016-03-03 18:30 UTC (permalink / raw)
To: intel-gfx, Damien Lespiau; +Cc: gustav.fagerlind, Li, Weinan Z
Hi Damien, Hi Daniel,
I've submitted the patch below for the third time now in an attempt
to get CI to test it, again to no avail. This time I didn't set the
In-Reply-To header and only submitted it as a single patch instead
of as a series because I expected this might confuse CI.
Nevertheless CI misclassified it under "Series" instead of "Patches",
reports that the series consists of 0 patches and doesn't test it:
https://patchwork.freedesktop.org/series/4068/
Earlier attempts:
https://patchwork.freedesktop.org/series/3453/
https://patchwork.freedesktop.org/series/3126/
Sorry, I give up.
Best regards,
Lukas
On Thu, Mar 03, 2016 at 06:02:53PM +0100, Lukas Wunner wrote:
> The ->lastclose callback invokes intel_fbdev_restore_mode() and has
> been witnessed to run before intel_fbdev_initial_config_async()
> has finished.
>
> We might likewise receive hotplug events or be suspended before
> we've had a chance to fully set up the fbdev.
>
> Fix by waiting for the asynchronous thread to finish.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580
> Reported-by: Gustav Fägerlind <gustav.fagerlind@gmail.com>
> Reported-by: "Li, Weinan Z" <weinan.z.li@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: stable@vger.kernel.org
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 8 +++-----
> drivers/gpu/drm/i915/intel_fbdev.c | 4 ++++
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 4aa3db6..9d76dfb 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -430,11 +430,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
> * Some ports require correctly set-up hpd registers for detection to
> * work properly (leading to ghost connected connector status), e.g. VGA
> * on gm45. Hence we can only set up the initial fbdev config after hpd
> - * irqs are fully enabled. Now we should scan for the initial config
> - * only once hotplug handling is enabled, but due to screwed-up locking
> - * around kms/fbdev init we can't protect the fdbev initial config
> - * scanning against hotplug events. Hence do this first and ignore the
> - * tiny window where we will loose hotplug notifactions.
> + * irqs are fully enabled. We protect the fbdev initial config scanning
> + * against hotplug events by waiting in intel_fbdev_output_poll_changed
> + * until the asynchronous thread has finished.
> */
> intel_fbdev_initial_config_async(dev);
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> index ae9cf6f..936c3d7 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -754,6 +754,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> struct intel_fbdev *ifbdev = dev_priv->fbdev;
> struct fb_info *info;
>
> + async_synchronize_full();
> if (!ifbdev)
> return;
>
> @@ -800,6 +801,8 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> void intel_fbdev_output_poll_changed(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + async_synchronize_full();
> if (dev_priv->fbdev)
> drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
> }
> @@ -811,6 +814,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
> struct intel_fbdev *ifbdev = dev_priv->fbdev;
> struct drm_fb_helper *fb_helper;
>
> + async_synchronize_full();
> if (!ifbdev)
> return;
>
> --
> 1.8.5.2 (Apple Git-48)
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev
2016-03-03 18:30 [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev Lukas Wunner
@ 2016-03-04 10:17 ` Jani Nikula
2016-03-04 12:16 ` Damien Lespiau
1 sibling, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2016-03-04 10:17 UTC (permalink / raw)
To: Lukas Wunner, intel-gfx, Damien Lespiau; +Cc: gustav.fagerlind, Li, Weinan Z
On Thu, 03 Mar 2016, Lukas Wunner <lukas@wunner.de> wrote:
> Hi Damien, Hi Daniel,
>
> I've submitted the patch below for the third time now in an attempt
> to get CI to test it, again to no avail. This time I didn't set the
> In-Reply-To header and only submitted it as a single patch instead
> of as a series because I expected this might confuse CI.
>
> Nevertheless CI misclassified it under "Series" instead of "Patches",
> reports that the series consists of 0 patches and doesn't test it:
> https://patchwork.freedesktop.org/series/4068/
>
> Earlier attempts:
> https://patchwork.freedesktop.org/series/3453/
> https://patchwork.freedesktop.org/series/3126/
>
> Sorry, I give up.
git send-email is all it takes.
BR,
Jani.
>
> Best regards,
>
> Lukas
>
> On Thu, Mar 03, 2016 at 06:02:53PM +0100, Lukas Wunner wrote:
>> The ->lastclose callback invokes intel_fbdev_restore_mode() and has
>> been witnessed to run before intel_fbdev_initial_config_async()
>> has finished.
>>
>> We might likewise receive hotplug events or be suspended before
>> we've had a chance to fully set up the fbdev.
>>
>> Fix by waiting for the asynchronous thread to finish.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580
>> Reported-by: Gustav Fägerlind <gustav.fagerlind@gmail.com>
>> Reported-by: "Li, Weinan Z" <weinan.z.li@intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Lukas Wunner <lukas@wunner.de>
>> ---
>> drivers/gpu/drm/i915/i915_dma.c | 8 +++-----
>> drivers/gpu/drm/i915/intel_fbdev.c | 4 ++++
>> 2 files changed, 7 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 4aa3db6..9d76dfb 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -430,11 +430,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
>> * Some ports require correctly set-up hpd registers for detection to
>> * work properly (leading to ghost connected connector status), e.g. VGA
>> * on gm45. Hence we can only set up the initial fbdev config after hpd
>> - * irqs are fully enabled. Now we should scan for the initial config
>> - * only once hotplug handling is enabled, but due to screwed-up locking
>> - * around kms/fbdev init we can't protect the fdbev initial config
>> - * scanning against hotplug events. Hence do this first and ignore the
>> - * tiny window where we will loose hotplug notifactions.
>> + * irqs are fully enabled. We protect the fbdev initial config scanning
>> + * against hotplug events by waiting in intel_fbdev_output_poll_changed
>> + * until the asynchronous thread has finished.
>> */
>> intel_fbdev_initial_config_async(dev);
>>
>> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
>> index ae9cf6f..936c3d7 100644
>> --- a/drivers/gpu/drm/i915/intel_fbdev.c
>> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
>> @@ -754,6 +754,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>> struct intel_fbdev *ifbdev = dev_priv->fbdev;
>> struct fb_info *info;
>>
>> + async_synchronize_full();
>> if (!ifbdev)
>> return;
>>
>> @@ -800,6 +801,8 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
>> void intel_fbdev_output_poll_changed(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> + async_synchronize_full();
>> if (dev_priv->fbdev)
>> drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
>> }
>> @@ -811,6 +814,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
>> struct intel_fbdev *ifbdev = dev_priv->fbdev;
>> struct drm_fb_helper *fb_helper;
>>
>> + async_synchronize_full();
>> if (!ifbdev)
>> return;
>>
>> --
>> 1.8.5.2 (Apple Git-48)
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev
2016-03-03 18:30 [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev Lukas Wunner
2016-03-04 10:17 ` Jani Nikula
@ 2016-03-04 12:16 ` Damien Lespiau
1 sibling, 0 replies; 4+ messages in thread
From: Damien Lespiau @ 2016-03-04 12:16 UTC (permalink / raw)
To: Lukas Wunner; +Cc: gustav.fagerlind, intel-gfx, Li, Weinan Z
Hi Lukas,
I'm sorry I haven't reacted sooner. I've enabled the option to only
consider git send-email patches on intel-gfx as we were having a lot of
false positives (ie patchwork considering emails were people were making
suggestions with diffs as patches to test).
I'm not sure how you send your emails, but I can tell why that it didn't
pass the "is this a git send-email patch?" question. We're looking at
two things:
- The X-Mailer header
- The Message-Id (git send-email has a special way to craft its
message IDs)
I've replayed your email by hand with an extra X-Mailer header and
patchwork picked it up (and hopefully we should have a run of our basic
acceptance tests done on it):
https://patchwork.freedesktop.org/series/4068/
You can keep whatever you're doing now and trick patchwork to take your
patches by added this extra header to your emails:
X-Mailer: git-send-email 2.4.3
Or use git send-email.
HTH,
--
Damien
On Thu, Mar 03, 2016 at 07:30:33PM +0100, Lukas Wunner wrote:
> Hi Damien, Hi Daniel,
>
> I've submitted the patch below for the third time now in an attempt
> to get CI to test it, again to no avail. This time I didn't set the
> In-Reply-To header and only submitted it as a single patch instead
> of as a series because I expected this might confuse CI.
>
> Nevertheless CI misclassified it under "Series" instead of "Patches",
> reports that the series consists of 0 patches and doesn't test it:
> https://patchwork.freedesktop.org/series/4068/
>
> Earlier attempts:
> https://patchwork.freedesktop.org/series/3453/
> https://patchwork.freedesktop.org/series/3126/
>
> Sorry, I give up.
>
> Best regards,
>
> Lukas
>
> On Thu, Mar 03, 2016 at 06:02:53PM +0100, Lukas Wunner wrote:
> > The ->lastclose callback invokes intel_fbdev_restore_mode() and has
> > been witnessed to run before intel_fbdev_initial_config_async()
> > has finished.
> >
> > We might likewise receive hotplug events or be suspended before
> > we've had a chance to fully set up the fbdev.
> >
> > Fix by waiting for the asynchronous thread to finish.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580
> > Reported-by: Gustav Fägerlind <gustav.fagerlind@gmail.com>
> > Reported-by: "Li, Weinan Z" <weinan.z.li@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Lukas Wunner <lukas@wunner.de>
> > ---
> > drivers/gpu/drm/i915/i915_dma.c | 8 +++-----
> > drivers/gpu/drm/i915/intel_fbdev.c | 4 ++++
> > 2 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 4aa3db6..9d76dfb 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -430,11 +430,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
> > * Some ports require correctly set-up hpd registers for detection to
> > * work properly (leading to ghost connected connector status), e.g. VGA
> > * on gm45. Hence we can only set up the initial fbdev config after hpd
> > - * irqs are fully enabled. Now we should scan for the initial config
> > - * only once hotplug handling is enabled, but due to screwed-up locking
> > - * around kms/fbdev init we can't protect the fdbev initial config
> > - * scanning against hotplug events. Hence do this first and ignore the
> > - * tiny window where we will loose hotplug notifactions.
> > + * irqs are fully enabled. We protect the fbdev initial config scanning
> > + * against hotplug events by waiting in intel_fbdev_output_poll_changed
> > + * until the asynchronous thread has finished.
> > */
> > intel_fbdev_initial_config_async(dev);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
> > index ae9cf6f..936c3d7 100644
> > --- a/drivers/gpu/drm/i915/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> > @@ -754,6 +754,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> > struct intel_fbdev *ifbdev = dev_priv->fbdev;
> > struct fb_info *info;
> >
> > + async_synchronize_full();
> > if (!ifbdev)
> > return;
> >
> > @@ -800,6 +801,8 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
> > void intel_fbdev_output_poll_changed(struct drm_device *dev)
> > {
> > struct drm_i915_private *dev_priv = dev->dev_private;
> > +
> > + async_synchronize_full();
> > if (dev_priv->fbdev)
> > drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
> > }
> > @@ -811,6 +814,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
> > struct intel_fbdev *ifbdev = dev_priv->fbdev;
> > struct drm_fb_helper *fb_helper;
> >
> > + async_synchronize_full();
> > if (!ifbdev)
> > return;
> >
> > --
> > 1.8.5.2 (Apple Git-48)
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev
@ 2016-03-03 17:02 Lukas Wunner
0 siblings, 0 replies; 4+ messages in thread
From: Lukas Wunner @ 2016-03-03 17:02 UTC (permalink / raw)
To: intel-gfx; +Cc: gustav.fagerlind, Li, Weinan Z
The ->lastclose callback invokes intel_fbdev_restore_mode() and has
been witnessed to run before intel_fbdev_initial_config_async()
has finished.
We might likewise receive hotplug events or be suspended before
we've had a chance to fully set up the fbdev.
Fix by waiting for the asynchronous thread to finish.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93580
Reported-by: Gustav Fägerlind <gustav.fagerlind@gmail.com>
Reported-by: "Li, Weinan Z" <weinan.z.li@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: stable@vger.kernel.org
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
drivers/gpu/drm/i915/i915_dma.c | 8 +++-----
drivers/gpu/drm/i915/intel_fbdev.c | 4 ++++
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 4aa3db6..9d76dfb 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -430,11 +430,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
* Some ports require correctly set-up hpd registers for detection to
* work properly (leading to ghost connected connector status), e.g. VGA
* on gm45. Hence we can only set up the initial fbdev config after hpd
- * irqs are fully enabled. Now we should scan for the initial config
- * only once hotplug handling is enabled, but due to screwed-up locking
- * around kms/fbdev init we can't protect the fdbev initial config
- * scanning against hotplug events. Hence do this first and ignore the
- * tiny window where we will loose hotplug notifactions.
+ * irqs are fully enabled. We protect the fbdev initial config scanning
+ * against hotplug events by waiting in intel_fbdev_output_poll_changed
+ * until the asynchronous thread has finished.
*/
intel_fbdev_initial_config_async(dev);
diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c
index ae9cf6f..936c3d7 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -754,6 +754,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
struct intel_fbdev *ifbdev = dev_priv->fbdev;
struct fb_info *info;
+ async_synchronize_full();
if (!ifbdev)
return;
@@ -800,6 +801,8 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous
void intel_fbdev_output_poll_changed(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = dev->dev_private;
+
+ async_synchronize_full();
if (dev_priv->fbdev)
drm_fb_helper_hotplug_event(&dev_priv->fbdev->helper);
}
@@ -811,6 +814,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev)
struct intel_fbdev *ifbdev = dev_priv->fbdev;
struct drm_fb_helper *fb_helper;
+ async_synchronize_full();
if (!ifbdev)
return;
--
1.8.5.2 (Apple Git-48)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-03-04 12:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-03 18:30 [PATCH RESEND FOR CI] drm/i915: Fix races on fbdev Lukas Wunner
2016-03-04 10:17 ` Jani Nikula
2016-03-04 12:16 ` Damien Lespiau
-- strict thread matches above, loose matches on Subject: below --
2016-03-03 17:02 Lukas Wunner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox