Re: [Intel-gfx] [PATCH 2/3] drm/i915: Iterate through the initialized DDIs to prepare their buffers

2014-08-07 Thread Paulo Zanoni 
2014-08-05 7:29 GMT-03:00 Damien Lespiau damien.lesp...@intel.com:
 Not every DDIs is necessarily connected can be strapped off and, in the
 future, we'll have platforms with a different number of default DDI
 ports. So, let's only call intel_prepare_ddi_buffers() on DDI ports that
 are actually detected.

 We also use the opportunity to give a struct intel_digital_port to
 intel_prepare_ddi_buffers() as we'll need it in a following patch to
 query if the port supports HMDI or not.

 On my HSW machine this removes the initialization of a couple of
 (unused) DDIs.

 Signed-off-by: Damien Lespiau damien.lesp...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h  |  5 +
  drivers/gpu/drm/i915/intel_ddi.c | 16 
  2 files changed, 17 insertions(+), 4 deletions(-)

 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index dcf318b..701aae0 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -176,6 +176,11 @@ enum hpd_pin {
 dev-mode_config.encoder_list, \
 base.head)

 +#define for_each_digital_port(dev, digital_port)   \
 +   list_for_each_entry(digital_port,   \
 +   dev-mode_config.encoder_list, \
 +   base.base.head)

We can't really assume that every encoder is intel_digital_port since
we still have the CRT encoder on HSW/BDW.

And we can't run this code just for the dig_ports since CRT needs it too.


 +
  #define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \
 list_for_each_entry((intel_encoder), 
 (dev)-mode_config.encoder_list, base.head) \
 if ((intel_encoder)-base.crtc == (__crtc))
 diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
 b/drivers/gpu/drm/i915/intel_ddi.c
 index ca1f9a8..fcbddd9 100644
 --- a/drivers/gpu/drm/i915/intel_ddi.c
 +++ b/drivers/gpu/drm/i915/intel_ddi.c
 @@ -152,10 +152,12 @@ enum port intel_ddi_get_encoder_port(struct 
 intel_encoder *intel_encoder)
   * in either FDI or DP modes only, as HDMI connections will work with both
   * of those
   */
 -static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port)
 +static void intel_prepare_ddi_buffers(struct drm_device *dev,
 + struct intel_digital_port 
 *intel_dig_port)
  {
 struct drm_i915_private *dev_priv = dev-dev_private;
 u32 reg;
 +   int port = intel_dig_port-port;
 int i, n_hdmi_entries, hdmi_800mV_0dB;
 int hdmi_level = dev_priv-vbt.ddi_port_info[port].hdmi_level_shift;
 const u32 *ddi_translations_fdi;
 @@ -232,13 +234,19 @@ static void intel_prepare_ddi_buffers(struct drm_device 
 *dev, enum port port)
   */
  void intel_prepare_ddi(struct drm_device *dev)
  {
 -   int port;
 +   struct intel_digital_port *intel_dig_port;
 +   bool visited[I915_MAX_PORTS] = { 0, };

 if (!HAS_DDI(dev))
 return;

 -   for (port = PORT_A; port = PORT_E; port++)
 -   intel_prepare_ddi_buffers(dev, port);
 +   for_each_digital_port(dev, intel_dig_port) {
 +   if (visited[intel_dig_port-port])
 +   continue;
 +
 +   intel_prepare_ddi_buffers(dev, intel_dig_port);
 +   visited[intel_dig_port-port] = true;
 +   }

A comment on why we need the visited array is much appreciated,
because it appears to be useless for the code reader. Why is it here?
Will we ever have more than one encoder per port?


  }

  static const long hsw_ddi_buf_ctl_values[] = {
 --
 1.8.3.1

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



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

Re: [Intel-gfx] [PATCH 2/3] drm/i915: Iterate through the initialized DDIs to prepare their buffers

2014-08-07 Thread Damien Lespiau 
On Thu, Aug 07, 2014 at 11:17:35AM -0300, Paulo Zanoni wrote:
  +#define for_each_digital_port(dev, digital_port)   \
  +   list_for_each_entry(digital_port,   \
  +   dev-mode_config.encoder_list, \
  +   base.base.head)
 
 We can't really assume that every encoder is intel_digital_port since
 we still have the CRT encoder on HSW/BDW.
 
 And we can't run this code just for the dig_ports since CRT needs it too.

Ah, missed that, of course...

  +   for_each_digital_port(dev, intel_dig_port) {
  +   if (visited[intel_dig_port-port])
  +   continue;
  +
  +   intel_prepare_ddi_buffers(dev, intel_dig_port);
  +   visited[intel_dig_port-port] = true;
  +   }
 
 A comment on why we need the visited array is much appreciated,
 because it appears to be useless for the code reader. Why is it here?
 Will we ever have more than one encoder per port?

Because of the MST fake encoder.

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

[Intel-gfx] [PATCH 2/3] drm/i915: Iterate through the initialized DDIs to prepare their buffers

2014-08-05 Thread Damien Lespiau 
Not every DDIs is necessarily connected can be strapped off and, in the
future, we'll have platforms with a different number of default DDI
ports. So, let's only call intel_prepare_ddi_buffers() on DDI ports that
are actually detected.

We also use the opportunity to give a struct intel_digital_port to
intel_prepare_ddi_buffers() as we'll need it in a following patch to
query if the port supports HMDI or not.

On my HSW machine this removes the initialization of a couple of
(unused) DDIs.

Signed-off-by: Damien Lespiau damien.lesp...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h  |  5 +
 drivers/gpu/drm/i915/intel_ddi.c | 16 
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dcf318b..701aae0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -176,6 +176,11 @@ enum hpd_pin {
dev-mode_config.encoder_list, \
base.head)
 
+#define for_each_digital_port(dev, digital_port)   \
+   list_for_each_entry(digital_port,   \
+   dev-mode_config.encoder_list, \
+   base.base.head)
+
 #define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \
list_for_each_entry((intel_encoder), (dev)-mode_config.encoder_list, 
base.head) \
if ((intel_encoder)-base.crtc == (__crtc))
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index ca1f9a8..fcbddd9 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -152,10 +152,12 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder 
*intel_encoder)
  * in either FDI or DP modes only, as HDMI connections will work with both
  * of those
  */
-static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port)
+static void intel_prepare_ddi_buffers(struct drm_device *dev,
+ struct intel_digital_port *intel_dig_port)
 {
struct drm_i915_private *dev_priv = dev-dev_private;
u32 reg;
+   int port = intel_dig_port-port;
int i, n_hdmi_entries, hdmi_800mV_0dB;
int hdmi_level = dev_priv-vbt.ddi_port_info[port].hdmi_level_shift;
const u32 *ddi_translations_fdi;
@@ -232,13 +234,19 @@ static void intel_prepare_ddi_buffers(struct drm_device 
*dev, enum port port)
  */
 void intel_prepare_ddi(struct drm_device *dev)
 {
-   int port;
+   struct intel_digital_port *intel_dig_port;
+   bool visited[I915_MAX_PORTS] = { 0, };
 
if (!HAS_DDI(dev))
return;
 
-   for (port = PORT_A; port = PORT_E; port++)
-   intel_prepare_ddi_buffers(dev, port);
+   for_each_digital_port(dev, intel_dig_port) {
+   if (visited[intel_dig_port-port])
+   continue;
+
+   intel_prepare_ddi_buffers(dev, intel_dig_port);
+   visited[intel_dig_port-port] = true;
+   }
 }
 
 static const long hsw_ddi_buf_ctl_values[] = {
-- 
1.8.3.1

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