[PATCH v2] radeon: Use mdelay() instead of msleep() when we can't sleep in atom_op_delay().

Daniel Vetter daniel at ffwll.ch
Wed Jan 4 03:44:48 PST 2012


On Wed, Jan 04, 2012 at 12:29:24PM +0100, Michel Dänzer wrote:
> On Mit, 2012-01-04 at 11:54 +0100, Daniel Vetter wrote: 
> > On Wed, Jan 04, 2012 at 11:33:40AM +0100, Michel Dänzer wrote:
> > > From: Michel Dänzer <michel.daenzer at amd.com>
> > > 
> > > It can be the case e.g. when switching to console for panic output.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=43941
> > > 
> > > Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
> > > ---
> > > 
> > > v2: Still call msleep() in the normal case. Only compile tested.
> > > 
> > >  drivers/gpu/drm/radeon/atom.c |    2 ++
> > >  1 files changed, 2 insertions(+), 0 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
> > > index 14cc88a..4092e59 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 (in_interrupt() || irqs_disabled() || in_atomic())
> > > +		mdelay(count);
> > 
> > Afaics in_atomic subsumes in_interrupt. irqs_disabled looks like a nice
> > addition to cover up the !CONFIG_PREEMPT case. i915 (in intel_drv.h) also
> > checks for in_dbg_master() to take care of kdbg.
> > 
> > Can I bother you to create a small helper like in_atomic_kms_context that
> > checks these three things (and also use it in drm/i915)?
> 
> Sorry, I've already spent way more time on this than I meant to, and
> Alan just killed what little motivation I still had to spend more.

I think Alan's simply wrong. The msleep checks in i915 are only used for
two cases:
- when using kdbg
- when displaying a panic
Afaics radeon has the exact same issue, at least I've seen my radeon
machine here go down after an oops. Splattering the entire driver for
these two corner cases which don't happen at all under normal
circumstances is imho utter nonsense.

I.e. I'd be very happy to smash a r-b onto your patch if you unifiy the
magical checks with i915 in a common function and add a small comment.
Does the prospect of an up-front r-b and me promising to harass Dave to
merge it restore your motivation?

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48


More information about the dri-devel mailing list