[PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 state and altivec idle

Wang Dongsheng-B40534 B40534 at freescale.com
Fri Oct 18 13:36:37 EST 2013



> -----Original Message-----
> From: Wood Scott-B07421
> Sent: Friday, October 18, 2013 12:52 AM
> To: Wang Dongsheng-B40534
> Cc: Bhushan Bharat-R65777; Wood Scott-B07421; linuxppc-
> dev at lists.ozlabs.org
> Subject: Re: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 state and
> altivec idle
> 
> On Thu, 2013-10-17 at 00:51 -0500, Wang Dongsheng-B40534 wrote:
> >
> > > -----Original Message-----
> > > From: Bhushan Bharat-R65777
> > > Sent: Thursday, October 17, 2013 11:20 AM
> > > To: Wang Dongsheng-B40534; Wood Scott-B07421
> > > Cc: linuxppc-dev at lists.ozlabs.org
> > > Subject: RE: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 state
> > > and altivec idle
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wang Dongsheng-B40534
> > > > Sent: Thursday, October 17, 2013 8:16 AM
> > > > To: Bhushan Bharat-R65777; Wood Scott-B07421
> > > > Cc: linuxppc-dev at lists.ozlabs.org
> > > > Subject: RE: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 state
> > > > and altivec idle
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Bhushan Bharat-R65777
> > > > > Sent: Thursday, October 17, 2013 1:01 AM
> > > > > To: Wang Dongsheng-B40534; Wood Scott-B07421
> > > > > Cc: linuxppc-dev at lists.ozlabs.org
> > > > > Subject: RE: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20
> > > > > state and altivec idle
> > > > >
> > > > >
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Wang Dongsheng-B40534
> > > > > > Sent: Tuesday, October 15, 2013 2:51 PM
> > > > > > To: Wood Scott-B07421
> > > > > > Cc: Bhushan Bharat-R65777; linuxppc-dev at lists.ozlabs.org; Wang
> > > > > Dongsheng-B40534
> > > > > > Subject: [PATCH v5 4/4] powerpc/85xx: add sysfs for pw20 state
> > > > > > and
> > > > > altivec idle
> > > > > >
> > > > > > +static ssize_t show_pw20_wait_time(struct device *dev,
> > > > > > +				struct device_attribute *attr, char *buf) {
> > > > > > +	u32 value;
> > > > > > +	u64 tb_cycle;
> > > > > > +	s64 time;
> > > > > > +
> > > > > > +	unsigned int cpu = dev->id;
> > > > > > +
> > > > > > +	if (!pw20_wt) {
> > > > > > +		smp_call_function_single(cpu, do_show_pwrmgtcr0, &value,
> > > 1);
> > > > > > +		value = (value & PWRMGTCR0_PW20_ENT) >>
> > > > > > +					PWRMGTCR0_PW20_ENT_SHIFT;
> > > > > > +
> > > > > > +		tb_cycle = (1 << (MAX_BIT - value)) * 2;
> > > > >
> > > > > Is value = 0 and value = 1 legal? These will make tb_cycle = 0,
> > > > >
> > > > > > +		time = div_u64(tb_cycle * 1000, tb_ticks_per_usec) - 1;
> > > > >
> > > > > And time = -1;
> > > > >
> > > > Please look at the end of the function, :)
> > > >
> > > > "return sprintf(buf, "%llu\n", time > 0 ? time : 0);"
> > >
> > > I know you return 0 if value = 0/1, my question was that, is this
> > > correct as per specification?
> > >
> > > Ahh, also for "value" upto 7 you will return 0, no?
> > >
> > If value = 0, MAX_BIT - value = 63
> > tb_cycle = 0xffffffff_ffffffff,
> 
> Actually, tb_cycle will be undefined because you shifted a 32-bit value
> (1) by more than 31 bits.  s/1/1ULL/
> 
Actually, we have been discussing this situation that could not have happened.
See !pw20_wt branch, this branch is read default wait bit.
The default wait bit is 50, the time is about 1ms.
The default wait bit cannot less than 50, means the wait entry time cannot greater than 1ms.
We have already begun benchmark test, and we got a preliminary results.
55, 56, 57bit looks good, but we need more benchmark to get the default bit.

	if (!pw20_wt) {
		smp_call_function_single(cpu, do_show_pwrmgtcr0, &value, 1);
		value = (value & PWRMGTCR0_PW20_ENT) >>
					PWRMGTCR0_PW20_ENT_SHIFT;

		tb_cycle = (1 << (MAX_BIT - value)) * 2;
		time = div_u64(tb_cycle * 1000, tb_ticks_per_usec) - 1;
	} else {
		time = pw20_wt;
	}

If it caused confusion, we can add a comment. As I discuss with Bharat.

> > tb_cycle * 1000 will overflow, but this situation is not possible.
> > Because if the "value = 0" means this feature will be "disable".
> > Now The default wait bit is 50(MAX_BIT - value, value = 13), the
> > PW20/Altivec Idle wait entry time is about 1ms, this time is very long
> > for wait idle time, and it's cannot be increased(means (MAX_BIT -
> > value) cannot greater than 50).
> 
> Why can it not be increased?
>
see above, :)

-dongsheng
> -Scott
> 



More information about the Linuxppc-dev mailing list