Re: [PATCH 10/11] drm/i915: Improve PSR debugfs status.

[Vijay Purushothaman <vijay.a.purushothaman@intel.com>]

On 6/3/2014 1:10 PM, Daniel Vetter wrote:
> On Mon, Jun 02, 2014 at 11:54:10PM +0530, Vijay Purushothaman wrote:
>> On 5/16/2014 5:43 AM, Rodrigo Vivi wrote:
>>> Now we have the active/inactive state for exit and this actually changes the
>>> HW enable bit the status was a bit confusing for users. So let's provide
>>> more info.
>>>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_debugfs.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index 6636ca2..0ca9376 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -1975,10 +1975,12 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
>>>
>>>   	seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support));
>>>   	seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok));
>>> +	seq_printf(m, "Enabled: %s\n", yesno(dev_priv->psr.enabled));
>>> +	seq_printf(m, "Active: %s\n", yesno(dev_priv->psr.active));
>>>
>>>   	enabled = HAS_PSR(dev) &&
>>>   		I915_READ(EDP_PSR_CTL(dev)) & EDP_PSR_ENABLE;
>>> -	seq_printf(m, "Enabled: %s\n", yesno(enabled));
>>> +	seq_printf(m, "HW Enabled & Active bit: %s\n", yesno(enabled));
>>>
>>>   	if (HAS_PSR(dev))
>>>   		psrperf = I915_READ(EDP_PSR_PERF_CNT(dev)) &
>>
>> Please remove all references to PSR performance counter. This register is
>> primarily meant as a debug register and its implementation is broken in the
>> h/w. Whenever the cdclk is gated to save power, the performance counter is
>> stopped. But when the clk is re-enabled it doesn't reset the counter. This
>> unnecessarily confuses the end users.. When the system goes through suspend
>> / resume cycle the performance counter most likely will transition from a
>> non-zero value to zero.. I already received few queries from our customers
>> related to this performance customer and they refuse to believe me when i
>> tell them PSR is still functional when the performance counter reports 0 :-)
>
> We expose other such perf registers and imo this is handy for debugging.
> Also we have a big push to expose all this perf stuff recently ...
>
> Imo we should keep this, if we can. If confused customers noodle around in
> debugfs without clue, maybe they shouldn't.
> -Daniel

In that case here is my

Reviewed-by: Vijay Purushothaman <vijay.a.purushothaman@intel.com>

Thanks,
Vijay
>