[Intel-gfx] [PATCH 0/6] eDP DRRS based on frontbuffer tracking

[Daniel Vetter]

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

>         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?
-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
> >>
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch