Re: [Intel-gfx] [RFC PATCH 01/18] drm/i915: Removing the eDP specific DRRS implementation

2015-06-29 Thread Ramalingam C


On Friday 26 June 2015 10:20 PM, Daniel Vetter wrote:

On Fri, Jun 26, 2015 at 07:21:45PM +0530, Ramalingam C wrote:

EDP specific DRRS implementation is removed to implement a
generic DRRS stack extentable accross the supportable encoders.

Signed-off-by: Ramalingam C 

Nack. You don't make something generic by first throwing out the existing
solution (and all the lessons learned implementing that).

Instead convert the edp DRRS implementation over to use generic
structures, step-by-step, and then once that's done it should be simply to
implement DRRS for other outputs.
Sure agreed. We will make the existing code as generic and implement for 
other encoders too.

I will work on that. thanks.

-Daniel


---
  drivers/gpu/drm/i915/i915_debugfs.c  |  110 -
  drivers/gpu/drm/i915/i915_drv.h  |   22 --
  drivers/gpu/drm/i915/intel_ddi.c |2 -
  drivers/gpu/drm/i915/intel_dp.c  |  392 --
  drivers/gpu/drm/i915/intel_drv.h |5 -
  drivers/gpu/drm/i915/intel_frontbuffer.c |2 -
  6 files changed, 533 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index c49fe2a..7dbf170 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2962,115 +2962,6 @@ static int i915_ddb_info(struct seq_file *m, void 
*unused)
return 0;
  }
  
-static void drrs_status_per_crtc(struct seq_file *m,

-   struct drm_device *dev, struct intel_crtc *intel_crtc)
-{
-   struct intel_encoder *intel_encoder;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   struct i915_drrs *drrs = &dev_priv->drrs;
-   int vrefresh = 0;
-
-   for_each_encoder_on_crtc(dev, &intel_crtc->base, intel_encoder) {
-   /* Encoder connected on this CRTC */
-   switch (intel_encoder->type) {
-   case INTEL_OUTPUT_EDP:
-   seq_puts(m, "eDP:\n");
-   break;
-   case INTEL_OUTPUT_DSI:
-   seq_puts(m, "DSI:\n");
-   break;
-   case INTEL_OUTPUT_HDMI:
-   seq_puts(m, "HDMI:\n");
-   break;
-   case INTEL_OUTPUT_DISPLAYPORT:
-   seq_puts(m, "DP:\n");
-   break;
-   default:
-   seq_printf(m, "Other encoder (id=%d).\n",
-   intel_encoder->type);
-   return;
-   }
-   }
-
-   if (dev_priv->vbt.drrs_type == STATIC_DRRS_SUPPORT)
-   seq_puts(m, "\tVBT: DRRS_type: Static");
-   else if (dev_priv->vbt.drrs_type == SEAMLESS_DRRS_SUPPORT)
-   seq_puts(m, "\tVBT: DRRS_type: Seamless");
-   else if (dev_priv->vbt.drrs_type == DRRS_NOT_SUPPORTED)
-   seq_puts(m, "\tVBT: DRRS_type: None");
-   else
-   seq_puts(m, "\tVBT: DRRS_type: FIXME: Unrecognized Value");
-
-   seq_puts(m, "\n\n");
-
-   if (to_intel_crtc_state(intel_crtc->base.state)->has_drrs) {
-   struct intel_panel *panel;
-
-   mutex_lock(&drrs->mutex);
-   /* DRRS Supported */
-   seq_puts(m, "\tDRRS Supported: Yes\n");
-
-   /* disable_drrs() will make drrs->dp NULL */
-   if (!drrs->dp) {
-   seq_puts(m, "Idleness DRRS: Disabled");
-   mutex_unlock(&drrs->mutex);
-   return;
-   }
-
-   panel = &drrs->dp->attached_connector->panel;
-   seq_printf(m, "\t\tBusy_frontbuffer_bits: 0x%X",
-   drrs->busy_frontbuffer_bits);
-
-   seq_puts(m, "\n\t\t");
-   if (drrs->refresh_rate_type == DRRS_HIGH_RR) {
-   seq_puts(m, "DRRS_State: DRRS_HIGH_RR\n");
-   vrefresh = panel->fixed_mode->vrefresh;
-   } else if (drrs->refresh_rate_type == DRRS_LOW_RR) {
-   seq_puts(m, "DRRS_State: DRRS_LOW_RR\n");
-   vrefresh = panel->downclock_mode->vrefresh;
-   } else {
-   seq_printf(m, "DRRS_State: Unknown(%d)\n",
-   drrs->refresh_rate_type);
-   mutex_unlock(&drrs->mutex);
-   return;
-   }
-   seq_printf(m, "\t\tVrefresh: %d", vrefresh);
-
-   seq_puts(m, "\n\t\t");
-   mutex_unlock(&drrs->mutex);
-   } else {
-   /* DRRS not supported. Print the VBT parameter*/
-   seq_puts(m, "\tDRRS Supported : No");
-   }
-   seq_puts(m, "\n");
-}
-
-static int i915_drrs_status(struct seq_file *m, void *unused)
-{
-   struct drm_info_node *node = m->private;
-   struct drm_device *dev = node->minor->dev;
-   

Re: [Intel-gfx] [RFC PATCH 01/18] drm/i915: Removing the eDP specific DRRS implementation

2015-06-26 Thread Daniel Vetter
On Fri, Jun 26, 2015 at 07:21:45PM +0530, Ramalingam C wrote:
> EDP specific DRRS implementation is removed to implement a
> generic DRRS stack extentable accross the supportable encoders.
> 
> Signed-off-by: Ramalingam C 

Nack. You don't make something generic by first throwing out the existing
solution (and all the lessons learned implementing that).

Instead convert the edp DRRS implementation over to use generic
structures, step-by-step, and then once that's done it should be simply to
implement DRRS for other outputs.
-Daniel

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  110 -
>  drivers/gpu/drm/i915/i915_drv.h  |   22 --
>  drivers/gpu/drm/i915/intel_ddi.c |2 -
>  drivers/gpu/drm/i915/intel_dp.c  |  392 
> --
>  drivers/gpu/drm/i915/intel_drv.h |5 -
>  drivers/gpu/drm/i915/intel_frontbuffer.c |2 -
>  6 files changed, 533 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index c49fe2a..7dbf170 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2962,115 +2962,6 @@ static int i915_ddb_info(struct seq_file *m, void 
> *unused)
>   return 0;
>  }
>  
> -static void drrs_status_per_crtc(struct seq_file *m,
> - struct drm_device *dev, struct intel_crtc *intel_crtc)
> -{
> - struct intel_encoder *intel_encoder;
> - struct drm_i915_private *dev_priv = dev->dev_private;
> - struct i915_drrs *drrs = &dev_priv->drrs;
> - int vrefresh = 0;
> -
> - for_each_encoder_on_crtc(dev, &intel_crtc->base, intel_encoder) {
> - /* Encoder connected on this CRTC */
> - switch (intel_encoder->type) {
> - case INTEL_OUTPUT_EDP:
> - seq_puts(m, "eDP:\n");
> - break;
> - case INTEL_OUTPUT_DSI:
> - seq_puts(m, "DSI:\n");
> - break;
> - case INTEL_OUTPUT_HDMI:
> - seq_puts(m, "HDMI:\n");
> - break;
> - case INTEL_OUTPUT_DISPLAYPORT:
> - seq_puts(m, "DP:\n");
> - break;
> - default:
> - seq_printf(m, "Other encoder (id=%d).\n",
> - intel_encoder->type);
> - return;
> - }
> - }
> -
> - if (dev_priv->vbt.drrs_type == STATIC_DRRS_SUPPORT)
> - seq_puts(m, "\tVBT: DRRS_type: Static");
> - else if (dev_priv->vbt.drrs_type == SEAMLESS_DRRS_SUPPORT)
> - seq_puts(m, "\tVBT: DRRS_type: Seamless");
> - else if (dev_priv->vbt.drrs_type == DRRS_NOT_SUPPORTED)
> - seq_puts(m, "\tVBT: DRRS_type: None");
> - else
> - seq_puts(m, "\tVBT: DRRS_type: FIXME: Unrecognized Value");
> -
> - seq_puts(m, "\n\n");
> -
> - if (to_intel_crtc_state(intel_crtc->base.state)->has_drrs) {
> - struct intel_panel *panel;
> -
> - mutex_lock(&drrs->mutex);
> - /* DRRS Supported */
> - seq_puts(m, "\tDRRS Supported: Yes\n");
> -
> - /* disable_drrs() will make drrs->dp NULL */
> - if (!drrs->dp) {
> - seq_puts(m, "Idleness DRRS: Disabled");
> - mutex_unlock(&drrs->mutex);
> - return;
> - }
> -
> - panel = &drrs->dp->attached_connector->panel;
> - seq_printf(m, "\t\tBusy_frontbuffer_bits: 0x%X",
> - drrs->busy_frontbuffer_bits);
> -
> - seq_puts(m, "\n\t\t");
> - if (drrs->refresh_rate_type == DRRS_HIGH_RR) {
> - seq_puts(m, "DRRS_State: DRRS_HIGH_RR\n");
> - vrefresh = panel->fixed_mode->vrefresh;
> - } else if (drrs->refresh_rate_type == DRRS_LOW_RR) {
> - seq_puts(m, "DRRS_State: DRRS_LOW_RR\n");
> - vrefresh = panel->downclock_mode->vrefresh;
> - } else {
> - seq_printf(m, "DRRS_State: Unknown(%d)\n",
> - drrs->refresh_rate_type);
> - mutex_unlock(&drrs->mutex);
> - return;
> - }
> - seq_printf(m, "\t\tVrefresh: %d", vrefresh);
> -
> - seq_puts(m, "\n\t\t");
> - mutex_unlock(&drrs->mutex);
> - } else {
> - /* DRRS not supported. Print the VBT parameter*/
> - seq_puts(m, "\tDRRS Supported : No");
> - }
> - seq_puts(m, "\n");
> -}
> -
> -static int i915_drrs_status(struct seq_file *m, void *unused)
> -{
> - struct drm_info_node *node = m->private;
> - struct drm_device *dev = node->minor->dev;
> - struct intel_crtc *intel_crtc;
> - int active_crtc_cnt = 0;
> -
> - for_each_intel_crtc(dev, intel_crtc) 

[Intel-gfx] [RFC PATCH 01/18] drm/i915: Removing the eDP specific DRRS implementation

2015-06-26 Thread Ramalingam C
EDP specific DRRS implementation is removed to implement a
generic DRRS stack extentable accross the supportable encoders.

Signed-off-by: Ramalingam C 
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  110 -
 drivers/gpu/drm/i915/i915_drv.h  |   22 --
 drivers/gpu/drm/i915/intel_ddi.c |2 -
 drivers/gpu/drm/i915/intel_dp.c  |  392 --
 drivers/gpu/drm/i915/intel_drv.h |5 -
 drivers/gpu/drm/i915/intel_frontbuffer.c |2 -
 6 files changed, 533 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index c49fe2a..7dbf170 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2962,115 +2962,6 @@ static int i915_ddb_info(struct seq_file *m, void 
*unused)
return 0;
 }
 
-static void drrs_status_per_crtc(struct seq_file *m,
-   struct drm_device *dev, struct intel_crtc *intel_crtc)
-{
-   struct intel_encoder *intel_encoder;
-   struct drm_i915_private *dev_priv = dev->dev_private;
-   struct i915_drrs *drrs = &dev_priv->drrs;
-   int vrefresh = 0;
-
-   for_each_encoder_on_crtc(dev, &intel_crtc->base, intel_encoder) {
-   /* Encoder connected on this CRTC */
-   switch (intel_encoder->type) {
-   case INTEL_OUTPUT_EDP:
-   seq_puts(m, "eDP:\n");
-   break;
-   case INTEL_OUTPUT_DSI:
-   seq_puts(m, "DSI:\n");
-   break;
-   case INTEL_OUTPUT_HDMI:
-   seq_puts(m, "HDMI:\n");
-   break;
-   case INTEL_OUTPUT_DISPLAYPORT:
-   seq_puts(m, "DP:\n");
-   break;
-   default:
-   seq_printf(m, "Other encoder (id=%d).\n",
-   intel_encoder->type);
-   return;
-   }
-   }
-
-   if (dev_priv->vbt.drrs_type == STATIC_DRRS_SUPPORT)
-   seq_puts(m, "\tVBT: DRRS_type: Static");
-   else if (dev_priv->vbt.drrs_type == SEAMLESS_DRRS_SUPPORT)
-   seq_puts(m, "\tVBT: DRRS_type: Seamless");
-   else if (dev_priv->vbt.drrs_type == DRRS_NOT_SUPPORTED)
-   seq_puts(m, "\tVBT: DRRS_type: None");
-   else
-   seq_puts(m, "\tVBT: DRRS_type: FIXME: Unrecognized Value");
-
-   seq_puts(m, "\n\n");
-
-   if (to_intel_crtc_state(intel_crtc->base.state)->has_drrs) {
-   struct intel_panel *panel;
-
-   mutex_lock(&drrs->mutex);
-   /* DRRS Supported */
-   seq_puts(m, "\tDRRS Supported: Yes\n");
-
-   /* disable_drrs() will make drrs->dp NULL */
-   if (!drrs->dp) {
-   seq_puts(m, "Idleness DRRS: Disabled");
-   mutex_unlock(&drrs->mutex);
-   return;
-   }
-
-   panel = &drrs->dp->attached_connector->panel;
-   seq_printf(m, "\t\tBusy_frontbuffer_bits: 0x%X",
-   drrs->busy_frontbuffer_bits);
-
-   seq_puts(m, "\n\t\t");
-   if (drrs->refresh_rate_type == DRRS_HIGH_RR) {
-   seq_puts(m, "DRRS_State: DRRS_HIGH_RR\n");
-   vrefresh = panel->fixed_mode->vrefresh;
-   } else if (drrs->refresh_rate_type == DRRS_LOW_RR) {
-   seq_puts(m, "DRRS_State: DRRS_LOW_RR\n");
-   vrefresh = panel->downclock_mode->vrefresh;
-   } else {
-   seq_printf(m, "DRRS_State: Unknown(%d)\n",
-   drrs->refresh_rate_type);
-   mutex_unlock(&drrs->mutex);
-   return;
-   }
-   seq_printf(m, "\t\tVrefresh: %d", vrefresh);
-
-   seq_puts(m, "\n\t\t");
-   mutex_unlock(&drrs->mutex);
-   } else {
-   /* DRRS not supported. Print the VBT parameter*/
-   seq_puts(m, "\tDRRS Supported : No");
-   }
-   seq_puts(m, "\n");
-}
-
-static int i915_drrs_status(struct seq_file *m, void *unused)
-{
-   struct drm_info_node *node = m->private;
-   struct drm_device *dev = node->minor->dev;
-   struct intel_crtc *intel_crtc;
-   int active_crtc_cnt = 0;
-
-   for_each_intel_crtc(dev, intel_crtc) {
-   drm_modeset_lock(&intel_crtc->base.mutex, NULL);
-
-   if (intel_crtc->base.state->active) {
-   active_crtc_cnt++;
-   seq_printf(m, "\nCRTC %d:  ", active_crtc_cnt);
-
-   drrs_status_per_crtc(m, dev, intel_crtc);
-   }
-
-   drm_modeset_unlock(&intel_crtc->base.mutex);
-   }
-
-   if (!active_crtc_cnt)
-   seq_puts(m,