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

[Paulo Zanoni <przanoni@gmail.com>]

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



-- 
Paulo Zanoni