Re: [RFC 0/6] eDP DRRS based on frontbuffer tracking

["Kannan, Vandana" <vandana.kannan@intel.com>]

On 23-Sep-14 2:31 PM, Daniel Vetter wrote:
> On Fri, Sep 12, 2014 at 11:13:03PM +0530, Vandana Kannan wrote:
>> As part of implementing DRRS based on frontbuffer tracking, this patch series
>> includes some modifications related to the data for DRRS, code to enable/
>> disable DRRS during init or uninit, calls to switch between high and low
>> refresh rates based on calls to mark_fb_busy.
>> Instead of having a module param to specify delay before scheduling DRRS
>> work, (as there was in the old patch series), a delay of 100ms is used.
>> Additional patches including changes specific to bdw and vlv have been
>> included.
>
> Sorry for the delay in answering, been a bit busy. Looks good overall,
> just spotted a few issue with the integration into the frontbuffer
> tracking. That's probably just because the documentation we have is awful,
> so I've improved that and replied with what I think we need.
>
Hi Daniel,

Sorry for the delay on this. I've been working on and off on it.
I went through the comment added in intel_frontbuffer.c, and just wanted 
to confirm if my understanding is correct.

In intel_frontbuffer.c, we have the comment
"
  * The other type of display power saving feature only cares about busyness
  * (e.g. DRRS). In that case all three (invalidate, flush and flip) 
indicate
  * busyness. There is no direct way to detect idleness.
"
Here, flip refers to intel_crtc_page_flip?
Since a flip also indicates busyness, it would mean that a switch back 
to high RR (invalidate DRRS) would be required at this point ? (apart 
from the GEM tracking).

Please let me know.
Thanks,
Vandana

> It unfortunately means that you need to rebase the integration patch since
> the frontbuffer functions moved. But since that needs to be redone anyway
> I hope that's not too much fuzz.
>
> And if you think the documentation is still lacking please raise this - we
> need to get much better at documenting i915 internals and that will only
> work if people critize and improve what's there.
>
>> TODO:-
>> Need more analysis into clone mode scenario
>
> Hm, I don't really understand what the issue is here? Currently i915
> doesn't support hw cloning for (e)DP ports, so you always have a 1:1 link
> between eDP panel and pipe. And if there's more than one pipe scanning out
> the same frontbuffer gem object then the frontbuffer tracking keeps them
> separate since it tracks frontbuffer attachments per-pipe.
>
> So can you please explain what you thing might be an issue? Very well
> possible that I'm just blind ;-)
>
> Thanks, Daniel
>
>
>>
>> Vandana Kannan (6):
>>    drm/i915: Modifying structures related to DRRS
>>    drm/i915: Initialize DRRS delayed work
>>    drm/i915: Enable/disable DRRS
>>    drm/i915: DRRS calls based on frontbuffer
>>    drm/i915/bdw: Add support for DRRS to switch RR
>>    drm/i915: Support for RR switching on VLV
>>
>>   drivers/gpu/drm/i915/i915_drv.h      |  32 ++++--
>>   drivers/gpu/drm/i915/i915_reg.h      |   1 +
>>   drivers/gpu/drm/i915/intel_ddi.c     |   2 +
>>   drivers/gpu/drm/i915/intel_display.c |  14 +--
>>   drivers/gpu/drm/i915/intel_dp.c      | 201 +++++++++++++++++++++++++++++------
>>   drivers/gpu/drm/i915/intel_drv.h     |  27 ++---
>>   6 files changed, 211 insertions(+), 66 deletions(-)
>>
>> --
>> 2.0.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx