Re: [Intel-gfx] [RFC PATCH 09/18] drm/i915: Cloned mode check
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 ramalinga...@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.
And for non-cloned mode (e.g. video only on external TV) things will be
controlled independantly.
Smells a lot like trying to encode policy instead of making sure that the
resulting behaviour matches what we want. And I think it should already.
-Daniel
+
+bool is_cloned_mode_active(struct drm_device *dev)
+{
+ struct drm_crtc *crtc = NULL, *tmp_crtc;
+
+ list_for_each_entry(tmp_crtc, dev-mode_config.crtc_list, head) {
+ if (crtc intel_crtc_active(tmp_crtc)) {
+ DRM_DEBUG_KMS(
+ more than one crtc active. Declared as clonec mode\n);
+ return true;
+ }
+
+ if (intel_crtc_active(tmp_crtc))
+ crtc = tmp_crtc;
+ }
+ return false;
+}
+
void intel_set_drrs_state(struct i915_drrs *drrs)
{
struct drrs_info *drrs_state;
@@ -158,7 +184,10 @@ static void intel_idleness_drrs_work_fn(struct work_struct
*__work)
panel = drrs-connector-panel;
- /* TODO: If DRRS is not supported on clone mode act here */
+ /* Double check if the dual-display mode is active. */
+ if (drrs-is_clone)
+ return;
+
mutex_lock(drrs-drrs_mutex);
if (panel-target_mode != NULL)
DRM_ERROR(FIXME: We shouldn't be here\n);
@@ -192,6 +221,11 @@ static void intel_enable_idleness_drrs(struct i915_drrs
*drrs)
mutex_lock(drrs-drrs_mutex);
+ drrs-is_clone = is_cloned_mode_active(drrs-connector-base.dev);
+
+ if (drrs-is_clone)
+ return;
+
/* Capturing the deferred request for disable_drrs */
if (drrs-drrs_state.type == SEAMLESS_DRRS_SUPPORT_SW
drrs-encoder_ops-is_drrs_hr_state_pending) {
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Thanks,
--Ram
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 09/18] drm/i915: Cloned mode check
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 ramalinga...@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
[Intel-gfx] [RFC PATCH 09/18] drm/i915: Cloned mode check
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 ramalinga...@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.
+ */
+
+bool is_cloned_mode_active(struct drm_device *dev)
+{
+ struct drm_crtc *crtc = NULL, *tmp_crtc;
+
+ list_for_each_entry(tmp_crtc, dev-mode_config.crtc_list, head) {
+ if (crtc intel_crtc_active(tmp_crtc)) {
+ DRM_DEBUG_KMS(
+ more than one crtc active. Declared as clonec mode\n);
+ return true;
+ }
+
+ if (intel_crtc_active(tmp_crtc))
+ crtc = tmp_crtc;
+ }
+ return false;
+}
+
void intel_set_drrs_state(struct i915_drrs *drrs)
{
struct drrs_info *drrs_state;
@@ -158,7 +184,10 @@ static void intel_idleness_drrs_work_fn(struct work_struct
*__work)
panel = drrs-connector-panel;
- /* TODO: If DRRS is not supported on clone mode act here */
+ /* Double check if the dual-display mode is active. */
+ if (drrs-is_clone)
+ return;
+
mutex_lock(drrs-drrs_mutex);
if (panel-target_mode != NULL)
DRM_ERROR(FIXME: We shouldn't be here\n);
@@ -192,6 +221,11 @@ static void intel_enable_idleness_drrs(struct i915_drrs
*drrs)
mutex_lock(drrs-drrs_mutex);
+ drrs-is_clone = is_cloned_mode_active(drrs-connector-base.dev);
+
+ if (drrs-is_clone)
+ return;
+
/* Capturing the deferred request for disable_drrs */
if (drrs-drrs_state.type == SEAMLESS_DRRS_SUPPORT_SW
drrs-encoder_ops-is_drrs_hr_state_pending) {
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 09/18] drm/i915: Cloned mode check
On Fri, Jun 26, 2015 at 07:08:40PM +0200, 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 ramalinga...@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. It's simpler than than: cloned mode == 1 pipe, 1 framebuffer, 1 pageflip. Does having a vblank_ref count as activity? There is a large body of code (like OML_sync_control) that assumes a fixed refresh rate for an output. (Though OML_sync_control is vague about multiple monitor setups and ignore mode changed altogether.) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 09/18] drm/i915: Cloned mode check
On Fri, Jun 26, 2015 at 06:14:07PM +0100, Chris Wilson wrote: On Fri, Jun 26, 2015 at 07:08:40PM +0200, 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 ramalinga...@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. It's simpler than than: cloned mode == 1 pipe, 1 framebuffer, 1 pageflip. Does having a vblank_ref count as activity? There is a large body of code (like OML_sync_control) that assumes a fixed refresh rate for an output. (Though OML_sync_control is vague about multiple monitor setups and ignore mode changed altogether.) Atm we don't yet count vblank refcounts as activity, but would definitely make tons of sense. Problem is that drm_irq.c is a mess, so for the meantime I think we have to live with tuning the idleness timer of DRRS such that for most timing critical apps it's long enough for them not to notice the tricks we're playing. And yeah fixing up drm_irq.c is somewhere on my list too - I want a proper driver entry point for modeset drivers, where we could intercept vblank waits and everything as we see fit. Last piece of modern drm that's still midlayered (well ignoring that most drivers don't have a demidlayered driver init yet). -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
Re: [Intel-gfx] [RFC PATCH 09/18] drm/i915: Cloned mode check
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 ramalinga...@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.
And for non-cloned mode (e.g. video only on external TV) things will be
controlled independantly.
Smells a lot like trying to encode policy instead of making sure that the
resulting behaviour matches what we want. And I think it should already.
-Daniel
+
+bool is_cloned_mode_active(struct drm_device *dev)
+{
+ struct drm_crtc *crtc = NULL, *tmp_crtc;
+
+ list_for_each_entry(tmp_crtc, dev-mode_config.crtc_list, head) {
+ if (crtc intel_crtc_active(tmp_crtc)) {
+ DRM_DEBUG_KMS(
+ more than one crtc active. Declared as clonec mode\n);
+ return true;
+ }
+
+ if (intel_crtc_active(tmp_crtc))
+ crtc = tmp_crtc;
+ }
+ return false;
+}
+
void intel_set_drrs_state(struct i915_drrs *drrs)
{
struct drrs_info *drrs_state;
@@ -158,7 +184,10 @@ static void intel_idleness_drrs_work_fn(struct
work_struct *__work)
panel = drrs-connector-panel;
- /* TODO: If DRRS is not supported on clone mode act here */
+ /* Double check if the dual-display mode is active. */
+ if (drrs-is_clone)
+ return;
+
mutex_lock(drrs-drrs_mutex);
if (panel-target_mode != NULL)
DRM_ERROR(FIXME: We shouldn't be here\n);
@@ -192,6 +221,11 @@ static void intel_enable_idleness_drrs(struct i915_drrs
*drrs)
mutex_lock(drrs-drrs_mutex);
+ drrs-is_clone = is_cloned_mode_active(drrs-connector-base.dev);
+
+ if (drrs-is_clone)
+ return;
+
/* Capturing the deferred request for disable_drrs */
if (drrs-drrs_state.type == SEAMLESS_DRRS_SUPPORT_SW
drrs-encoder_ops-is_drrs_hr_state_pending) {
--
1.7.9.5
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
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
