| Message ID | 1407234579-16934-3-git-send-email-damien.lespiau@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2014-08-05 7:29 GMT-03:00 Damien Lespiau <damien.lespiau@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.lespiau@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
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.
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[] = {
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.lespiau@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(-)