Re: [RFC PATCH 09/18] drm/i915: Cloned mode check

From: Daniel Vetter <daniel@ffwll.ch>
To: Ramalingam C <ramalingam.c@intel.com>
Cc: paulo.r.zanoni@intel.com, daniel.vetter@ffwll.ch,
	intel-gfx@lists.freedesktop.org, rodrigo.vivi@intel.com
Subject: Re: [RFC PATCH 09/18] drm/i915: Cloned mode check
Date: Mon, 29 Jun 2015 18:16:05 +0200	[thread overview]
Message-ID: <20150629161605.GA30960@phenom.ffwll.local> (raw)
In-Reply-To: <55913085.6050301@intel.com>

On Mon, Jun 29, 2015 at 05:18:21PM +0530, Ramalingam C wrote:
> 
> On Friday 26 June 2015 10:38 PM, Daniel Vetter wrote:
> >On Fri, Jun 26, 2015 at 07:21:53PM +0530, Ramalingam C wrote:
> >>If crtc is in clone mode, DRRS will be disabled. Because if the both
> >>the displays are not sharing the same vrefresh, then userspace
> >>activities based on vsync will go for toss.
> >>
> >>Clone mode will be rechecked on every restarting Idleness DRRS events.
> >>
> >>Signed-off-by: Ramalingam C <ramalingam.c@intel.com>
> >>---
> >>  drivers/gpu/drm/i915/intel_drrs.c |   36 +++++++++++++++++++++++++++++++++++-
> >>  1 file changed, 35 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_drrs.c b/drivers/gpu/drm/i915/intel_drrs.c
> >>index e5d8bcd..42b420d 100644
> >>--- a/drivers/gpu/drm/i915/intel_drrs.c
> >>+++ b/drivers/gpu/drm/i915/intel_drrs.c
> >>@@ -16,6 +16,7 @@
> >>  #include <drm/i915_drm.h>
> >>  #include <linux/delay.h>
> >>+#include <linux/list.h>
> >>  #include "i915_drv.h"
> >>  #include "intel_drv.h"
> >>@@ -85,6 +86,31 @@ int get_free_drrs_struct_index(struct drm_i915_private *dev_priv)
> >>  	return -EBUSY;
> >>  }
> >>+/*
> >>+ * TODO: This is identifying the multiple active crtcs at a time.
> >>+ * Here we assume that this is clone state and disable DRRS.
> >>+ * But need to implement a proper method to find the real cloned mode
> >>+ * state. DRRS need not be disabled incase of multiple crtcs with
> >>+ * different content.
> >>+ */
> >This is a pretty big hack. Why do you need it? fb tracking should keep any
> >display in the high mode as long as there's activity, so as long as
> >userspace flips both buffers for both pipes (which is should for cloned
> >mode) they'll both be running at max.
> Yup. This is not needed for Idleness as fb tracking will keep the DRRS at
> High refresh rate.
> But at content based DRRS, we had some concern from android Userspace team
> that we cant
> support content based DRRS for cloned modes (eDP + HDMI or DSI + HDMI).
> 
> Of course at this point in time, we can enable the DRRS for all scenarios
> and test. Based on results we can plan ahead.

DRRS tracks each pipe individually and will not be confused if you use the
same framebuffer for more than one pipe (i.e. cloned use-case). The exact
same thing is how X usually is set up.

What exactly are the concerns that DRRS won't work? "concerns" is really
unspecific. We need a really clear description of what's expected, why it
doesn't work with the current DRRS and then figure out a suitable way to
address it. Adding code without a clear use-case is bad.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx