Re: [PATCH] acpi-cpufreq: Remove P-state read after a P-state write in normal path

[Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>]

On Sun, Aug 28, 2005 at 08:09:41PM +0200, Dominik Brodowski wrote:
> Hi,
> 
> On Fri, Aug 26, 2005 at 05:10:52PM -0700, Venkatesh Pallipadi wrote:
> >  	/*
> > -	 * Then we read the 'status_register' and compare the value with the
> > -	 * target state's 'status' to make sure the transition was successful.
> > -	 * Note that we'll poll for up to 1ms (100 cycles of 10us) before
> > -	 * giving up.
> > +	 * Assume the write went through when acpi_pstate_strict is not used.
> > +	 * As read status_register is an expensive operation and there 
> > +	 * are no specific error cases where an IO port write will fail.
> >  	 */
> 
> Well, the IO port write itself might not fail, but the transition itself --
> and we're reading the _status_ register here, not the control register where
> we've written to. And 8.4.4.1 of ACPI-sepc 3.0 does specifically mention
> that transitions _can_ fail, so I think we should handle this possibility.

Yes. ACPI spec says transitions can fail. But, it doesn't fail often in 
practise. And even if it fails, I think, we should handle it without this 
read os STATUS register. The speedstep-centrino driver, which does similar
thing as acpi-cpufreq, does not do this status check after control MSR write.
We can skip the read of STATUS in cpi-cpufreq in a similar way. No?

I feel the overhead of doing the status read here is too high. As it uses SMM,
the latency for read of STATUS is almost same as write into CONTROL. If we 
think retaining the STATUS read is better, we should atleast double the 
transition time reported in _PSS to compensate for this extra overhead.

And reading the STATUS in a loop should go away. I don't see that it being 
mentioned in ACPI spec. The 1mS loop seems totally redundant.

Thanks,
Venki
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/