Message-ID: <5552B8AD.9020200@linux.intel.com>
Date:	Wed, 13 May 2015 10:36:29 +0800
From:	"Lu, Baolu" <baolu.lu@...ux.intel.com>
To:	Alan Stern <stern@...land.harvard.edu>
CC:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Mathias Nyman <mathias.nyman@...el.com>,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/3] usb: notify hcd when USB device suspend or resume



On 05/12/2015 11:54 PM, Alan Stern wrote:
> On Tue, 12 May 2015, Lu, Baolu wrote:
>
>> I'm sorry that I confused you.
>>
>> FSC is a different thing from what this patch series does.
> I know that.  The patch series, in its current form, is fine.  Now I'm
> trying to understand what you originally wanted to do.
>
>>> Let's see if I understand it correctly:
>>>
>>> 	When the controller goes into suspend, you want the cache to
>>> 	be pushed into memory so that the cache can be powered down,
>>> 	thereby saving additional energy.
>> Not the controller goes into suspend, but USB devices.
> Suppose a USB device goes into runtime suspend, and suppose the
> hardware pushes the cached information for the endpoints on that device
> into memory.  The cache will still contain information for other,
> non-suspended devices.  Consequently the cache _can't_ be powered down
> at this time, right?  You seem to be saying:
>
> 	When a USB device goes into suspend, you want the cache to
> 	be pushed into memory so that the cache can be powered down,
> 	thereby saving additional energy.
>
> But that is just wrong!  The only time you know the hardware can safely
> power-down the cache is when the _controller_ goes into suspend.  At
> that time you _know_ all the attached devices are suspended, so the
> cache isn't needed.
>
> Therefore, when the controller goes into suspend, you want to make sure
> that the cache has been pushed into memory.  The actual push operation
> can occur earlier, during xhci_device_suspend, but the time when the
> information's location matters is during xhci_suspend.
>
>> In order to talking to USB devices, xHCI has an endpoint state for each
>> endpoint of a device. When a USB device goes into suspend, xHC driver
>> could ask the hardware to push the state from cache to the memory.
>> Thereby saving additional energy. This is the intention of this patch
>> series.
> You don't save any energy when the _device_ goes into suspend.  You
> save energy only when the _controller_ goes into suspend, because
> that's the only time when the cache can be powered down.  Right?

Yes, I agree with you.

Hardware could only power down the cache after all data has been
pushed out. That could happen during host suspend as far as I can see.

>
>>> 	If the hardware supports FSC, this will happen automatically.
>>>
>>> 	If the hardware doesn't support FSC, the cached data won't get
>>> 	pushed to memory unless the driver tells the controller to do
>>> 	so at the time the device is suspended.  But this will slow
>>> 	things down, so the driver should avoid doing it when it's not
>>> 	needed.
>>>
>>> 	During system suspend you know in advance that the controller
>>> 	will be suspended.  Therefore the driver should push the cache
>>> 	to memory if the hardware doesn't support FSC.  During runtime
>>> 	suspend you don't know in advance whether the controller will
>>> 	be suspended, so the driver should not push the cache to
>>> 	memory.
>>>
>>> 	But what happens in the case where all the devices have gone
>>> 	into runtime suspend, so the controller also goes into runtime
>>> 	suspend, and the hardware doesn't support FSC?  It seems that
>>> 	in this case the cache would have to remain powered on.
>> FSC is not part of this patch series. It was introduced when I tried to
>> explain the reason why I kept 'msg' parameter in the callback.
>>
>> If the hardware support FSC, another operation named "save context
>> operation" will push everything in hardware cache into memory. So when
>> a USB device is going to suspend and xhci_device_suspend() callback is
>> being called, software can do an optimization. That is, if "save context
>> operation" will push everything in hardware cache to memory later,
>> xhci_device_suspend() could skip asking hardware for cache operation
>> and let "save context operation" do it later.
> That's more or less the same as what I wrote above, except that I said
> it would happen automatically if the hardware supports FSC.  It isn't
> automatic; it requires the driver to issue a "save context operation"
> command.

Yes, exactly.

>
>> One example of this situation is system suspend. During system suspend,
>> below process will be done for a USB3 fabric.
>>
>> 1) all USB devices suspend
>> 2) root hub suspend
>> 3) host controller suspend
>>
>> In 1), xhci_device_suspend() call back will be called for each device
>> suspend.
>> In 3) "save context operation" will be executed.
>>
>> In this case, if FSC is supported, xhci_device_suspend() could skip asking
>> host hardware for cache operation.
> That's basically what I said.
>
> But now why should "msg" matter?  It seems that xhci_device_suspend()
> should skip asking for the cache operation whenever FSC is supported,
> regardless of whether you're talking about runtime suspend or system
> suspend.

In runtime case, a USB device could be selected to suspend while host
controller is busy with other USB devices. Since FSC only impacts the
behavior of save context operation, it's probably that a USB device
is selectively suspended while it's state is still kept in cache.

My thinking is that although cache management is hardware implementation
dependent, it's always a good behavior for software to ask for the cache
operation whenever it realizes that caching the state is unnecessary. This
is good not only for power saving, but also for better cache utilization.

>
>>> Also, it's still not clear what the driver needs to do differently in
>>> order to push out the cached data.  You have managed to imply both:
>>>
>>> 	Issuing Stop Endpoint with SP set is mandatory whenever
>>> 	a device is suspended, and
>>>
>>> 	The SP bit is what tells the controller to push the endpoint
>>> 	state to memory.
>>>
>>> This doesn't make sense.
>> Software doesn't handle the cache directly. XHC driver just tell hardware
>> that a device is going to suspend. XHC driver does this through stop
>> endpoint
>> command by setting the SP (suspend) bit in the command trb.
>>
>> During xHC handles stop endpoint command, it will check SP bit, if it's set,
>> xHC knows that the endpoint state could be pushed out to memory now,
>> since device is going to suspend. The operation of cache is very hardware
>> implementation dependent.
> Does that mean the driver has no control over whether "stop endpoint
> with SP set" will push the cached information into memory?  In that
> case, how can xhci_device_suspend skip asking the hardware for the
> cache operation?

XHCI spec doesn't define what hardware does when it sees SP bit set in
stop endpoint command. An implementation can even be designed without
an internal cache. But whenever cache is used, spec does recommend
and assume that hardware should push cached state into system memory
when SP bit set.

Spec (1.1) has an implementation note on page 253. It says,

If an implementation uses the Scratchpad or non-volatile internal memory
to cache endpoint state rather than Endpoint Contexts, then the Save State
operation can flush that state to the Scratchpad and software does not have
to issue Stop Endpoint Commands as part of the suspend process.

>
>>> One last question: How much extra time does it take to push the cached
>>> data to memory?  Bear in mind that suspending a device is not a rapid
>>> operation; if pushing the data takes no more than a few microseconds, I
>>> think it would be worthwhile to do it always, even when it's not
>>> necessary.
>> It depends on how much time software spends in xhci_device_suspend().
>> I don't have data in hand, but I agree with you that if this time is no more
>> than a few microseconds, we can let do it always.
> Above you seem to be saying that there's no way to prevent it!  Your
> exact words are: "The operation of cache is very hardware
> implementation dependent."

I explained my intention of "the operation of cache is very hardware
implementation dependent" above.

 From software point of view, the extra time it takes to ask for cache
operation can be measured in xhci_device_suspend(). I can measure
the data later when I complete my tasks in hand. If that data is ignorable
comparing to the suspend time, we can simply do it always.

>
> Alan Stern

Thank you!
-Baolu

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

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

Powered by blists - more mailing lists