Re: [PATCH] drm/i915: fix for_each_digital_port

[Imre Deak <imre.deak@intel.com>]

On pe, 2015-04-17 at 14:58 +0300, Imre Deak wrote:
> We should check if a given encoder is of a digital type before casting
> it to a digital port object. This broke on HSW when iterating the VGA
> encoder.
> 
> Introduced in
> commit b403745c84592b26a0713e6944c2b109f6df5c82
> Author: Damien Lespiau <damien.lespiau@intel.com>
> Date:   Mon Aug 4 22:01:33 2014 +0100
> 
>     drm/i915: Iterate through the initialized DDIs to prepare their buffers
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90067
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  | 10 ++++++----
>  drivers/gpu/drm/i915/intel_ddi.c |  3 ++-
>  drivers/gpu/drm/i915/intel_drv.h | 10 ++++++++++
>  3 files changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 40ef672..7160fc8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -251,10 +251,12 @@ enum hpd_pin {
>  			    &dev->mode_config.connector_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_digital_port(dev, __intel_encoder, digital_port)		\
> +	list_for_each_entry(__intel_encoder,				\
> +			    &dev->mode_config.encoder_list,		\
> +			    base.head)					\
> +		if (intel_enc_is_digital(__intel_encoder) &&		\
> +		    ((digital_port) = enc_to_dig_port(&__intel_encoder->base)))
>  
>  #define for_each_encoder_on_crtc(dev, __crtc, intel_encoder) \
>  	list_for_each_entry((intel_encoder), &(dev)->mode_config.encoder_list, base.head) \
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 455d44b..b5d7e74 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -370,13 +370,14 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
>   */
>  void intel_prepare_ddi(struct drm_device *dev)
>  {
> +	struct intel_encoder *__intel_encoder;
>  	struct intel_digital_port *intel_dig_port;
>  	bool visited[I915_MAX_PORTS] = { 0, };
>  
>  	if (!HAS_DDI(dev))
>  		return;
>  
> -	for_each_digital_port(dev, intel_dig_port) {
> +	for_each_digital_port(dev, __intel_encoder, intel_dig_port) {
>  		if (visited[intel_dig_port->port])
>  			continue;
>  
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 23a42a4..a354f26 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -870,6 +870,16 @@ hdmi_to_dig_port(struct intel_hdmi *intel_hdmi)
>  	return container_of(intel_hdmi, struct intel_digital_port, hdmi);
>  }
>  
> +static inline bool intel_enc_is_digital(struct intel_encoder *intel_encoder)
> +{
> +	return	intel_encoder->type == INTEL_OUTPUT_HDMI ||
> +		intel_encoder->type == INTEL_OUTPUT_DISPLAYPORT ||
> +		intel_encoder->type == INTEL_OUTPUT_EDP ||
> +		intel_encoder->type == INTEL_OUTPUT_UNKNOWN ||
> +		intel_encoder->type == INTEL_OUTPUT_DP_MST;
> +
> +}

This should probably have a MISSING_CASE for the other types, I can
change that if there are no other comments.

> +
>  /*
>   * Returns the number of planes for this pipe, ie the number of sprites + 1
>   * (primary plane). This doesn't count the cursor plane then.


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