[PATCH] radeon: Use mdelay() instead of msleep() in atom_op_delay().

Alan Cox alan at lxorguk.ukuu.org.uk
Wed Jan 4 04:20:49 PST 2012


> Every modesetting operation can happen in atomic context or not in
> atomic context, thanks to the wonder of oops printing and also kgdb,
> trying to figure out which table in this one person's bios is the
> "root" cause and special casing for it is insane since tomorrow 20
> other BIOSes could contain that table or a new variant we haven't seen
> before.
> 
> Unless one of us is totally misinterpreting what the other is saying
> or what the atombios interpreter is.

Its a sort of bytecode engine that allows the BIOS to describe to the
OS how the GPU should be handled. So what we want to avoid is
accidentally having some other path become spinlock based due to a
random unrelated change and ending up doing 20mS spinning delays without
us getting a warning.

Explicitly calling out that mode setting does this isn't a big deal - we
don't modeset often while doing stuff that is latency sensitive (gaming,
live music, telephony, etc) and if you have the in_atomic check as well
we'd probably only hit it on panic().

The problem is if we just band-aid it without doing this it will (and
always has) ended in tears later.

So as per the other mail all I'm really saying is we want something like


	radeon_atom_mode_atomic_begin(rdev)
	{
		rdev->mode_info.atom_context->atomic = 1;
	}

	radeon_atom_mode_atomic_end(rdev)
	{
		rdev->mode_info.atom_context->atomic = 0;
	}

	/* above maybe with sanity checks to stop missed ones */


	and to do

	else {
		if (!in_atomic())
			msleep(count);
		else {
			WARN_ON(!ctx->atomic);
			mdelay(count);
		}
	}

so that we know it isn't getting hit in places we've not carefully
considered the impact of a huge stall.

Alan


More information about the dri-devel mailing list