[Intel-gfx] [RFC PATCH 00/18] Generic DRRS implementation across the encoders

[Ramalingam C]

On Friday 26 June 2015 10:46 PM, Daniel Vetter wrote:
> On Fri, Jun 26, 2015 at 07:21:44PM +0530, Ramalingam C wrote:
>> Display Refresh Rate Switching (DRRS) is a power conservation feature
>> which enables swtching between low and high refresh rates,
>> dynamically, based on the usage scenario to save power.
>>
>> This feature is applicable for internal panels.
>>
>> Indication that the panel supports DRRS is given by the panel EDID, which
>> would list multiple refresh rates for one resolution along with VBT fields
>> like drrs_min_vrefresh.
>>
>> DRRS is of 2 types - static and seamless.
>> Static needs full modeset. Which can be used in docking and undocking
>> scenarios.
>> Seamless DRRS involves changing RR without any visual effect to the user
>> and can be used during normal system usage. This is done by programming
>> certain registers.
>>
>> Support for static/seamless DRRS may be indicated in the VBT based on
>> inputs from the panel spec.
>>
>> We have implemented two DRRS use case scenrios :
>> 1. Idleness DRRS,	2. Media playback DRRS (Content's FPS based)
>>
>> Idleness DRRS:-
>> 	The implementation is based on frontbuffer tracking implementation.
>> When there is a disturbance on the screen triggered by user activity or a
>> periodic system activity, DRRS is disabled (RR is changed to high RR).
>> When there is no movement on screen, after a timeout, a switch
>> to low RR is made.
>> 	For integration with frontbuffer tracking code,
>> intel_drrs_invalidate() and intel_drrs_flush() are called.
>>
>> Media playback DRRS (Content's FPS based):
>> 	Based on the content's Frames Per Second, minimum required vrefresh
>> is called in userspace. If the panel supports the minimum required vrefresh
>> and if the DRRS is supported on the platform, userspace will make a modeset
>> request with same mode and FB but with different vrefresh.
>> 	Panel's vrefresh range will be exposed through connector's modelist and
>> connector property is created to expose the DRRS capability to the userspace.
>>
>> 	In DRRS state machine Media plyabck DRRS holds the highest priority.
>> Idleness DRRS is disabled as soon as Media playback DRRS is requested.
>> Idleness DRRS is reenabled when the userspace indicates the exit of Media
>> playback DRRS by requesting for highest vrefresh supported.
>>
>> NOTE:
>> 1. DRM documentation will be made once the RFC is closed along with IGT.
>> 2. Currently in RFC, platform support is developed only for VLV.
>>     it will be extended to other platform once the design is approved.
>> 3. Basic design of DSI DRRS and Content based DRRS is functionally tested on
>>     Android(VLV & CHT).
Daniel.

First of all Thank you so much for your comments.
Sorry for such a huge patch series. Tolerate me on that one.

> I've made a few drive-by comments, but big picture comments repeated here:
> - Don't throw existing code away as in patch 1. When extending a
>    feature/code, we'll refactor the existing stuff and keep it working all
>    the time. Otherwise we invalidate all the previous testing, which throws
>    out a lot of value.
Ok agreed. I will work on this comment. And will bring the current 
existing eDP DRRS code to the generic design,
retaining the bug fixes we have done.
>
> - Static DRRS and generalized seamless DRRS are imo separate features and
>    we should split the patch series. seamless DRRS is already implemented
>    using the fb tracking, maybe extended with some hints to userspace.
>
>    Static DRRS otoh is just a modeset with a different clock (plus a better
>    internal implementation to avoid flicker). So from that pov two
>    completely different features, both in the implementation and the
>    userspace ABI.
Yup. Static is not even in our development radar :). All the code that I 
have shared is
concerned only with seamless DRRS and it's two usecase scenarios( 
Idleness and Content based).
Mentioned the Static DRRS in cover letter, just to give the two 
different DRRS supports available in general.
>
> - I guess we'll have a proper design discussion in internal mtgs about
>    this too?
Sure Daniel we will have one.
>
> Thanks, Daniel
>> Ramalingam C (18):
>>    drm/i915: Removing the eDP specific DRRS implementation
>>    drm/i915: Generic DRRS state Machine
>>    drm/i915: Addition of the drrs_min_vrefresh in VBT
>>    drm/i915: Implementation of Generic DSI DRRS
>>    drm/i915: Adjusting the pclk for dual link and burst mode
>>    drm/i915: VLV dsi drrs support
>>    drm/i915: Generic eDP DRRS implementation
>>    drm/i915: VLV eDP DRRS methods
>>    drm/i915: Cloned mode check
>>    drm/i915: Initializing DRRS for all connectors
>>    drm/i915: Updating the crtc modes in DRRS transitions
>>    drm/i915: Redesigning dp_set_m_n to receive divider values
>>    drm/i915: MEDIA_RR support in general DRRS state machine
>>    drm/i915: MEDIA_RR support in eDP DRRS module
>>    drm/i915: MEDIA_RR support in DSI DRRS module
>>    drm/i915: Filtering media playback DRRS requests
>>    drm/i915: Addition of downclock mode to connector modelist
>>    drm/i915: Connector property for DRRS capability
>>
>>   drivers/gpu/drm/i915/Makefile            |    3 +
>>   drivers/gpu/drm/i915/i915_debugfs.c      |  110 -----
>>   drivers/gpu/drm/i915/i915_drv.h          |   91 +++-
>>   drivers/gpu/drm/i915/i915_reg.h          |    4 +
>>   drivers/gpu/drm/i915/intel_bios.c        |    3 +
>>   drivers/gpu/drm/i915/intel_bios.h        |    9 +-
>>   drivers/gpu/drm/i915/intel_ddi.c         |    2 -
>>   drivers/gpu/drm/i915/intel_display.c     |   57 ++-
>>   drivers/gpu/drm/i915/intel_dp.c          |  397 +---------------
>>   drivers/gpu/drm/i915/intel_drrs.c        |  728 ++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_drrs.h        |   38 ++
>>   drivers/gpu/drm/i915/intel_drv.h         |   45 +-
>>   drivers/gpu/drm/i915/intel_dsi.c         |   29 +-
>>   drivers/gpu/drm/i915/intel_dsi.h         |    3 +
>>   drivers/gpu/drm/i915/intel_dsi_drrs.c    |  415 +++++++++++++++++
>>   drivers/gpu/drm/i915/intel_dsi_drrs.h    |   66 +++
>>   drivers/gpu/drm/i915/intel_dsi_pll.c     |  303 +++++++++++--
>>   drivers/gpu/drm/i915/intel_edp_drrs.c    |  196 ++++++++
>>   drivers/gpu/drm/i915/intel_edp_drrs.h    |   29 ++
>>   drivers/gpu/drm/i915/intel_frontbuffer.c |    4 +-
>>   drivers/gpu/drm/i915/intel_modes.c       |   28 ++
>>   21 files changed, 1959 insertions(+), 601 deletions(-)
>>   create mode 100644 drivers/gpu/drm/i915/intel_drrs.c
>>   create mode 100644 drivers/gpu/drm/i915/intel_drrs.h
>>   create mode 100644 drivers/gpu/drm/i915/intel_dsi_drrs.c
>>   create mode 100644 drivers/gpu/drm/i915/intel_dsi_drrs.h
>>   create mode 100644 drivers/gpu/drm/i915/intel_edp_drrs.c
>>   create mode 100644 drivers/gpu/drm/i915/intel_edp_drrs.h
>>
>> -- 
>> 1.7.9.5
>>
-- Thanks,
Ram