[PATCH] drm: introduce drm_can_sleep and use in intel/radeon drivers.

[PATCH] drm: introduce drm_can_sleep and use in intel/radeon drivers.

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

So we have a few places where the drm drivers would like to sleep to
be nice to the system, mainly in the modesetting paths, but we also
have two cases were atomic modesetting must take place, panic writing
and kernel debugger. So provide a central inline to determine if a
sleep or delay should be used and use this in the intel and radeon drivers.

Based on patch from Michel Dänzer <michel.daenzer@amd.com>

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43941

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/i915/intel_drv.h |    4 ++--
 drivers/gpu/drm/radeon/atom.c    |    2 ++
 include/drm/drmP.h               |    8 ++++++++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a1b4343..cf2326b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -39,7 +39,7 @@
 			ret__ = -ETIMEDOUT;				\
 			break;						\
 		}							\
-		if (W && !(in_atomic() || in_dbg_master())) msleep(W);	\
+		if (W && drm_can_sleep()) msleep(W);	\
 	}								\
 	ret__;								\
 })
@@ -48,7 +48,7 @@
 #define wait_for_atomic(COND, MS) _wait_for(COND, MS, 0)
 
 #define MSLEEP(x) do { \
-	if (in_dbg_master()) \
+	if (!drm_can_sleeep()) \
 		mdelay(x); \
 	else \
 		msleep(x); \
diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
index 14cc88a..d1bd239 100644
--- a/drivers/gpu/drm/radeon/atom.c
+++ b/drivers/gpu/drm/radeon/atom.c
@@ -665,6 +665,8 @@ static void atom_op_delay(atom_exec_context *ctx, int *ptr, int arg)
 	SDEBUG("   count: %d\n", count);
 	if (arg == ATOM_UNIT_MICROSEC)
 		udelay(count);
+	else if (!drm_can_sleep())
+		mdelay(count);
 	else
 		msleep(count);
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 1f9e951..f1c01bc 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1696,5 +1696,13 @@ extern void drm_platform_exit(struct drm_driver *driver, struct platform_device
 extern int drm_get_platform_dev(struct platform_device *pdev,
 				struct drm_driver *driver);
 
+/* returns true if currently okay to sleep */
+static __inline__ bool drm_can_sleep(void)
+{
+	if (in_atomic() || in_dbg_master() || irqs_disabled())
+		return false;
+	return true;
+}
+
 #endif				/* __KERNEL__ */
 #endif
-- 
1.7.7.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: introduce drm_can_sleep and use in intel/radeon drivers.

[Michel Dänzer]

On Don, 2012-01-05 at 09:57 +0000, Dave Airlie wrote: 
> From: Dave Airlie <airlied@redhat.com>
> 
> So we have a few places where the drm drivers would like to sleep to
> be nice to the system, mainly in the modesetting paths, but we also
> have two cases were atomic modesetting must take place, panic writing
> and kernel debugger. So provide a central inline to determine if a
> sleep or delay should be used and use this in the intel and radeon drivers.
> 
> Based on patch from Michel Dänzer <michel.daenzer@amd.com>
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43941
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h |    4 ++--
>  drivers/gpu/drm/radeon/atom.c    |    2 ++
>  include/drm/drmP.h               |    8 ++++++++
>  3 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a1b4343..cf2326b 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
[...] 
> @@ -48,7 +48,7 @@
>  #define wait_for_atomic(COND, MS) _wait_for(COND, MS, 0)
>  
>  #define MSLEEP(x) do { \
> -	if (in_dbg_master()) \
> +	if (!drm_can_sleeep()) \
>  		mdelay(x); \

So for MSLEEP it's not enough if it can sleep, it has to be able to
'sleeep'? ;) Looks like this macro is unused.

Looks good to me otherwise, thanks.


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: introduce drm_can_sleep and use in intel/radeon drivers.

[Daniel Vetter]

On Thu, Jan 05, 2012 at 09:57:56AM +0000, Dave Airlie wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> So we have a few places where the drm drivers would like to sleep to
> be nice to the system, mainly in the modesetting paths, but we also
> have two cases were atomic modesetting must take place, panic writing
> and kernel debugger. So provide a central inline to determine if a
> sleep or delay should be used and use this in the intel and radeon drivers.
> 
> Based on patch from Michel Dänzer <michel.daenzer@amd.com>
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43941
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>

If MSLEEP is indeed unused, I think it should just die - we have way too
much yelling cruft from dri1 yonder lying around that hides important
details like this. Otherwise

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH] drm: introduce drm_can_sleep and use in intel/radeon drivers.

[Chris Wilson]

On Thu, 5 Jan 2012 17:00:52 +0100, Daniel Vetter <daniel@ffwll.ch> wrote:
> If MSLEEP is indeed unused, I think it should just die - we have way too
> much yelling cruft from dri1 yonder lying around that hides important
> details like this. Otherwise

It was used, they just happened to all be redundant or mispellings of
wait-for-vblank. All the remaining sleeps now look to be away from the
atomic paths, and so MSLEEP can die for the time being.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] drm: introduce drm_can_sleep and use in intel/radeon drivers.

[Dave Airlie]

>>
>> So we have a few places where the drm drivers would like to sleep to
>> be nice to the system, mainly in the modesetting paths, but we also
>> have two cases were atomic modesetting must take place, panic writing
>> and kernel debugger. So provide a central inline to determine if a
>> sleep or delay should be used and use this in the intel and radeon drivers.
>>
>> Based on patch from Michel Dänzer <michel.daenzer@amd.com>
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43941
>>
>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>
> If MSLEEP is indeed unused, I think it should just die - we have way too
> much yelling cruft from dri1 yonder lying around that hides important
> details like this. Otherwise
>
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I've pushed the version that just drops MSLEEP since nobody used it.

Thanks,
Dave.