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

[Ramalingam C <ramalingam.c@intel.com>]

On Friday 27 February 2015 09:07 PM, Daniel Vetter wrote:
> On Fri, Feb 27, 2015 at 07:59:43PM +0530, Ramalingam C wrote:
>> On Tuesday 24 February 2015 06:21 AM, Daniel Vetter wrote:
>>> On Fri, Feb 13, 2015 at 03:32:58PM +0530, Ramalingam C wrote:
>>>> This series includes a preparation patch for drrs support across differnt
>>>> platforms in intel_dp_set_m_n along with last 5 pending patches of V3 eDP
>>>> DRRS patch series.
>>>>
>>>> New series is submitted to make the review more comfortable and
>>>> to display the dependancy of the patches explicitly.
>>>>
>>>> Durgadoss R (1):
>>>>    drm/i915: Enable eDP DRRS for CHV
>>>>
>>>> Ramalingam C (1):
>>>>    drm/i915: Add support for DRRS in intel_dp_set_m_n
>>>>
>>>> Vandana Kannan (4):
>>>>    drm/i915/bdw: Add support for DRRS to switch RR
>>>>    drm/i915: Support for RR switching on VLV
>>>>    Documentation/drm: DocBook integration for DRRS
>>>>    drm/i915: Add debugfs entry for DRRS
>>> Ok I've reviewed the locking for DRRS quickly now that it's all merged and
>>> it's deadlock-y:
>>>
>>> intel_edp_drrs_downclock_work grabs the drrs mutex. But in the disable
>>> function we cancel that work and wait for that to complete (cancel_sync)
>>> while holding the lock.
>>>
>>> Which means if the async work is running this will deadlock. The work
>>> cancel must be moved out of the critical section, and the work must
>>> double-check (after taking the lock) that drrs hasn't been disabled
>>> meanwhile (by checking drrs.dp, which we already do). intel_psr.c contains
>>> all that logic as an example.
>>>
>>> While you do that follow-up patch could we extract the drrs code into a
>>> new intel_drrs.c file? That would also simplify the kerneldoc includes a
>>> bit.
>>>
>>> Cheers, Daniel
>> Missed the possible deadlock window. I will upload a patch moving the
>> cancellation of the deferred work out of the mutex protection asap.
>>
>> As a heads up, in VPG we have implemented MIPI DRRS and also content based
>> DRRS.
>>
>> MIPI DRRS:
>> We have extended the DRRS to DSI encoder also on android codelines for BYT
>> and CHV.
>>
>>
>> Content based DRRS:
>> We have implemented the interfaces
> What kind of interface? Imo we should do this by adjusting the frequecy of
> the mode, with a suitable modeset fastpath.
> -Daniel
We have used the specific path in modeset to request for the DRRS 
vrefresh. And used a drm property to expose the DRRS type.
>
>>          to expose the DRRS capability and and the vrefresh supported
>>          to receive the request for the new vrefresh.
>>
>> So based on required FPS for the display content to be rendered userspace,
>> will place a request for the new vrefresh.
>>
>> We have verified the functionality of this implementation on android devices
>> for almost an year now.
>> I will rework on the implementation to adapt to the DRM-intel and submit a
>> RFC to explain the complete design.
>>
>> This RFC will have the generalized DRRS state machine for all encoders like
>> eDP and DSI along with encoder support for DSI and eDP.
>> So as a result we will have the generic code for drrs state machine in
>> intel_drrs.c totally separated from the encoder related code. Can we wait
>> till then?
> The deadlock fix should land asap. Or do you mean something else?
Patch to fix deadlock will be submitted asap in early monday after 
required testing. I was asking regarding the DRRS code separation.
> -Daniel
>
>>
>> --Ram
>>>>   Documentation/DocBook/drm.tmpl       |   11 ++++
>>>>   drivers/gpu/drm/i915/i915_debugfs.c  |   99 ++++++++++++++++++++++++++++
>>>>   drivers/gpu/drm/i915/i915_reg.h      |    1 +
>>>>   drivers/gpu/drm/i915/intel_display.c |   32 ++++++---
>>>>   drivers/gpu/drm/i915/intel_dp.c      |  121 ++++++++++++++++++++++++++++++++--
>>>>   drivers/gpu/drm/i915/intel_drv.h     |   22 ++++++-
>>>>   6 files changed, 273 insertions(+), 13 deletions(-)
>>>>
>>>> -- 
>>>> 1.7.9.5
>>>>

-- 
Ram

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx