public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: fix for_each_digital_port
@ 2015-04-17 11:58 Imre Deak
  2015-04-17 12:27 ` Imre Deak
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Imre Deak @ 2015-04-17 11:58 UTC (permalink / raw)
  To: intel-gfx

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;
+
+}
+
 /*
  * Returns the number of planes for this pipe, ie the number of sprites + 1
  * (primary plane). This doesn't count the cursor plane then.
-- 
2.1.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: fix for_each_digital_port
  2015-04-17 11:58 [PATCH] drm/i915: fix for_each_digital_port Imre Deak
@ 2015-04-17 12:27 ` Imre Deak
  2015-04-17 12:36 ` Paulo Zanoni
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 14+ messages in thread
From: Imre Deak @ 2015-04-17 12:27 UTC (permalink / raw)
  To: intel-gfx

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: fix for_each_digital_port
  2015-04-17 11:58 [PATCH] drm/i915: fix for_each_digital_port Imre Deak
  2015-04-17 12:27 ` Imre Deak
@ 2015-04-17 12:36 ` Paulo Zanoni
  2015-04-17 16:29   ` Imre Deak
  2015-04-17 16:31 ` [PATCH v2 1/2] drm/i915: factor out ddi_get_encoder_port Imre Deak
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Paulo Zanoni @ 2015-04-17 12:36 UTC (permalink / raw)
  To: Imre Deak; +Cc: Intel Graphics Development

2015-04-17 8:58 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> 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) {

The problem is that now we're not gonna call
intel_prepare_ddi_buffers() for PORT_E, which is the CRT port.

Perhaps we could make that function accept intel_encoder() and then
somehow call intel_ddi_get_encoder_port() or something... Or maybe
just special-case the CRT encoder. Just suggestions...

>                 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;
> +
> +}
> +
>  /*
>   * Returns the number of planes for this pipe, ie the number of sprites + 1
>   * (primary plane). This doesn't count the cursor plane then.
> --
> 2.1.0
>
> _______________________________________________
> 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: fix for_each_digital_port
  2015-04-17 12:36 ` Paulo Zanoni
@ 2015-04-17 16:29   ` Imre Deak
  0 siblings, 0 replies; 14+ messages in thread
From: Imre Deak @ 2015-04-17 16:29 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Intel Graphics Development

On pe, 2015-04-17 at 09:36 -0300, Paulo Zanoni wrote:
> 2015-04-17 8:58 GMT-03:00 Imre Deak <imre.deak@intel.com>:
> > 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) {
> 
> The problem is that now we're not gonna call
> intel_prepare_ddi_buffers() for PORT_E, which is the CRT port.
> 
> Perhaps we could make that function accept intel_encoder() and then
> somehow call intel_ddi_get_encoder_port() or something... Or maybe
> just special-case the CRT encoder. Just suggestions...

Thanks for catching this, completely missed that. I also noticed now
that MST encoders don't have an embedding digital_port object either, so
for those instead of the digital_port port field we were looking at the
mst_encoder pipe field :/

I'll try again with the above things fixed.

> 
> >                 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;
> > +
> > +}
> > +
> >  /*
> >   * Returns the number of planes for this pipe, ie the number of sprites + 1
> >   * (primary plane). This doesn't count the cursor plane then.
> > --
> > 2.1.0
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/2] drm/i915: factor out ddi_get_encoder_port
  2015-04-17 11:58 [PATCH] drm/i915: fix for_each_digital_port Imre Deak
  2015-04-17 12:27 ` Imre Deak
  2015-04-17 12:36 ` Paulo Zanoni
@ 2015-04-17 16:31 ` Imre Deak
  2015-04-27 17:24   ` Damien Lespiau
  2015-04-17 16:31 ` [PATCH v2 2/2] drm/i915: fix intel_prepare_ddi Imre Deak
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Imre Deak @ 2015-04-17 16:31 UTC (permalink / raw)
  To: intel-gfx; +Cc: paulo.r.zanoni

In the next patch we'll need to get at both the encoder's intel_digital_port
object - which maybe NULL for a CRT - and it's port, so factor out this
functionality.

No functional change.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90067
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 455d44b..903d395 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -210,29 +210,39 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
 	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
 };
 
-enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
+static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
+				 struct intel_digital_port **dig_port,
+				 enum port *port)
 {
 	struct drm_encoder *encoder = &intel_encoder->base;
 	int type = intel_encoder->type;
 
 	if (type == INTEL_OUTPUT_DP_MST) {
-		struct intel_digital_port *intel_dig_port = enc_to_mst(encoder)->primary;
-		return intel_dig_port->port;
+		*dig_port = enc_to_mst(encoder)->primary;
+		*port = (*dig_port)->port;
 	} else if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP ||
 	    type == INTEL_OUTPUT_HDMI || type == INTEL_OUTPUT_UNKNOWN) {
-		struct intel_digital_port *intel_dig_port =
-			enc_to_dig_port(encoder);
-		return intel_dig_port->port;
-
+		*dig_port = enc_to_dig_port(encoder);
+		*port = (*dig_port)->port;
 	} else if (type == INTEL_OUTPUT_ANALOG) {
-		return PORT_E;
-
+		*dig_port = NULL;
+		*port = PORT_E;
 	} else {
 		DRM_ERROR("Invalid DDI encoder type %d\n", type);
 		BUG();
 	}
 }
 
+enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
+{
+	struct intel_digital_port *dig_port;
+	enum port port;
+
+	ddi_get_encoder_port(intel_encoder, &dig_port, &port);
+
+	return port;
+}
+
 static bool
 intel_dig_port_supports_hdmi(const struct intel_digital_port *intel_dig_port)
 {
-- 
2.1.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/2] drm/i915: fix intel_prepare_ddi
  2015-04-17 11:58 [PATCH] drm/i915: fix for_each_digital_port Imre Deak
                   ` (2 preceding siblings ...)
  2015-04-17 16:31 ` [PATCH v2 1/2] drm/i915: factor out ddi_get_encoder_port Imre Deak
@ 2015-04-17 16:31 ` Imre Deak
  2015-04-23 21:37   ` shuang.he
  2015-04-27 17:29   ` Damien Lespiau
  2015-04-23 21:39 ` [PATCH] drm/i915: fix for_each_digital_port shuang.he
  2015-04-27 17:05 ` Damien Lespiau
  5 siblings, 2 replies; 14+ messages in thread
From: Imre Deak @ 2015-04-17 16:31 UTC (permalink / raw)
  To: intel-gfx; +Cc: paulo.r.zanoni

At the moment intel_prepare_ddi buffer will iterate through both MST and
CRT encoders, which is incorrect. Neither of these encoder types have an
embedding intel_digital_port object, so for these encoder types we will
use random data when dereferencing the corresponding
intel_digital_port->port field.

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

v2:
- fix getting at the port for MST encoders too
- make sure that intel_prepare_ddi_buffers() gets called for port E too
  (Paulo)

Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  5 -----
 drivers/gpu/drm/i915/intel_ddi.c | 28 ++++++++++++++++++----------
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 40ef672..c461b56 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -251,11 +251,6 @@ 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_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 903d395..9c1e74a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -256,12 +256,11 @@ intel_dig_port_supports_hdmi(const struct intel_digital_port *intel_dig_port)
  * 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,
-				      struct intel_digital_port *intel_dig_port)
+static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port,
+				      bool supports_hdmi)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 reg;
-	int port = intel_dig_port->port;
 	int i, n_hdmi_entries, n_dp_entries, n_edp_entries, hdmi_default_entry,
 	    size;
 	int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
@@ -272,7 +271,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
 	const struct ddi_buf_trans *ddi_translations;
 
 	if (IS_BROXTON(dev)) {
-		if (!intel_dig_port_supports_hdmi(intel_dig_port))
+		if (!supports_hdmi)
 			return;
 
 		/* Vswing programming for HDMI */
@@ -360,7 +359,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
 		reg += 4;
 	}
 
-	if (!intel_dig_port_supports_hdmi(intel_dig_port))
+	if (!supports_hdmi)
 		return;
 
 	/* Choose a good default if VBT is badly populated */
@@ -380,18 +379,27 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
  */
 void intel_prepare_ddi(struct drm_device *dev)
 {
-	struct intel_digital_port *intel_dig_port;
+	struct intel_encoder *intel_encoder;
 	bool visited[I915_MAX_PORTS] = { 0, };
 
 	if (!HAS_DDI(dev))
 		return;
 
-	for_each_digital_port(dev, intel_dig_port) {
-		if (visited[intel_dig_port->port])
+	for_each_intel_encoder(dev, intel_encoder) {
+		struct intel_digital_port *intel_dig_port;
+		enum port port;
+		bool supports_hdmi;
+
+		ddi_get_encoder_port(intel_encoder, &intel_dig_port, &port);
+
+		if (visited[port])
 			continue;
 
-		intel_prepare_ddi_buffers(dev, intel_dig_port);
-		visited[intel_dig_port->port] = true;
+		supports_hdmi = intel_dig_port &&
+				intel_dig_port_supports_hdmi(intel_dig_port);
+
+		intel_prepare_ddi_buffers(dev, port, supports_hdmi);
+		visited[port] = true;
 	}
 }
 
-- 
2.1.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] drm/i915: fix intel_prepare_ddi
  2015-04-17 16:31 ` [PATCH v2 2/2] drm/i915: fix intel_prepare_ddi Imre Deak
@ 2015-04-23 21:37   ` shuang.he
  2015-04-27 17:29   ` Damien Lespiau
  1 sibling, 0 replies; 14+ messages in thread
From: shuang.he @ 2015-04-23 21:37 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, imre.deak

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6225
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  301/301              301/301
SNB                                  318/318              318/318
IVB                                  345/345              345/345
BYT                 -1              285/285              284/285
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@kms_setmode@invalid-clone-exclusive-crtc      PASS(5)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: fix for_each_digital_port
  2015-04-17 11:58 [PATCH] drm/i915: fix for_each_digital_port Imre Deak
                   ` (3 preceding siblings ...)
  2015-04-17 16:31 ` [PATCH v2 2/2] drm/i915: fix intel_prepare_ddi Imre Deak
@ 2015-04-23 21:39 ` shuang.he
  2015-04-27 17:05 ` Damien Lespiau
  5 siblings, 0 replies; 14+ messages in thread
From: shuang.he @ 2015-04-23 21:39 UTC (permalink / raw)
  To: shuang.he, ethan.gao, intel-gfx, imre.deak

Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6223
-------------------------------------Summary-------------------------------------
Platform          Delta          drm-intel-nightly          Series Applied
PNV                                  276/276              276/276
ILK                                  301/301              301/301
SNB                                  318/318              318/318
IVB                                  345/345              345/345
BYT                 -1              285/285              284/285
BDW                                  321/321              321/321
-------------------------------------Detailed-------------------------------------
Platform  Test                                drm-intel-nightly          Series Applied
*BYT  igt@kms_setmode@invalid-clone-exclusive-crtc      PASS(5)      DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:check_crtc_state[i915]]*ERROR*mismatch_in_has_infoframe(expected#,found#)@mismatch in has_infoframe .* found
WARNING:at_drivers/gpu/drm/i915/intel_display.c:#check_crtc_state[i915]()@WARNING:.* at .* check_crtc_state+0x
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: fix for_each_digital_port
  2015-04-17 11:58 [PATCH] drm/i915: fix for_each_digital_port Imre Deak
                   ` (4 preceding siblings ...)
  2015-04-23 21:39 ` [PATCH] drm/i915: fix for_each_digital_port shuang.he
@ 2015-04-27 17:05 ` Damien Lespiau
  2015-04-27 17:15   ` Damien Lespiau
  5 siblings, 1 reply; 14+ messages in thread
From: Damien Lespiau @ 2015-04-27 17:05 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Fri, Apr 17, 2015 at 02:58:10PM +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>

Reviewed-by: Damien Lespiau <damien.lespiau@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;
> +
> +}
> +
>  /*
>   * Returns the number of planes for this pipe, ie the number of sprites + 1
>   * (primary plane). This doesn't count the cursor plane then.
> -- 
> 2.1.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: fix for_each_digital_port
  2015-04-27 17:05 ` Damien Lespiau
@ 2015-04-27 17:15   ` Damien Lespiau
  2015-04-27 17:18     ` Damien Lespiau
  0 siblings, 1 reply; 14+ messages in thread
From: Damien Lespiau @ 2015-04-27 17:15 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Mon, Apr 27, 2015 at 06:05:31PM +0100, Damien Lespiau wrote:
> On Fri, Apr 17, 2015 at 02:58:10PM +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>
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Huuum, scrap that r-b, I think we're missing PORT_E initialization on
HSW, because that's the one driver the CRT encoder through FDI.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm/i915: fix for_each_digital_port
  2015-04-27 17:15   ` Damien Lespiau
@ 2015-04-27 17:18     ` Damien Lespiau
  0 siblings, 0 replies; 14+ messages in thread
From: Damien Lespiau @ 2015-04-27 17:18 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx

On Mon, Apr 27, 2015 at 06:15:45PM +0100, Damien Lespiau wrote:
> On Mon, Apr 27, 2015 at 06:05:31PM +0100, Damien Lespiau wrote:
> > On Fri, Apr 17, 2015 at 02:58:10PM +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>
> > 
> > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> 
> Huuum, scrap that r-b, I think we're missing PORT_E initialization on
> HSW, because that's the one driver the CRT encoder through FDI.

Which is exactly what Paulo found as well... Did we already mentionned
how hard is was to follow reviews :)

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/2] drm/i915: factor out ddi_get_encoder_port
  2015-04-17 16:31 ` [PATCH v2 1/2] drm/i915: factor out ddi_get_encoder_port Imre Deak
@ 2015-04-27 17:24   ` Damien Lespiau
  0 siblings, 0 replies; 14+ messages in thread
From: Damien Lespiau @ 2015-04-27 17:24 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx, paulo.r.zanoni

On Fri, Apr 17, 2015 at 07:31:21PM +0300, Imre Deak wrote:
> In the next patch we'll need to get at both the encoder's intel_digital_port
> object - which maybe NULL for a CRT - and it's port, so factor out this
> functionality.
> 
> No functional change.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90067
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_ddi.c | 28 +++++++++++++++++++---------
>  1 file changed, 19 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 455d44b..903d395 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -210,29 +210,39 @@ static const struct bxt_ddi_buf_trans bxt_ddi_translations_hdmi[] = {
>  	{ 154, 0x9A, 1, 128, true },	/* 9:	1200		0   */
>  };
>  
> -enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
> +static void ddi_get_encoder_port(struct intel_encoder *intel_encoder,
> +				 struct intel_digital_port **dig_port,
> +				 enum port *port)
>  {
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	int type = intel_encoder->type;
>  
>  	if (type == INTEL_OUTPUT_DP_MST) {
> -		struct intel_digital_port *intel_dig_port = enc_to_mst(encoder)->primary;
> -		return intel_dig_port->port;
> +		*dig_port = enc_to_mst(encoder)->primary;
> +		*port = (*dig_port)->port;
>  	} else if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP ||
>  	    type == INTEL_OUTPUT_HDMI || type == INTEL_OUTPUT_UNKNOWN) {
> -		struct intel_digital_port *intel_dig_port =
> -			enc_to_dig_port(encoder);
> -		return intel_dig_port->port;
> -
> +		*dig_port = enc_to_dig_port(encoder);
> +		*port = (*dig_port)->port;
>  	} else if (type == INTEL_OUTPUT_ANALOG) {
> -		return PORT_E;
> -
> +		*dig_port = NULL;
> +		*port = PORT_E;
>  	} else {
>  		DRM_ERROR("Invalid DDI encoder type %d\n", type);
>  		BUG();
>  	}
>  }
>  
> +enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder)
> +{
> +	struct intel_digital_port *dig_port;
> +	enum port port;
> +
> +	ddi_get_encoder_port(intel_encoder, &dig_port, &port);
> +
> +	return port;
> +}
> +
>  static bool
>  intel_dig_port_supports_hdmi(const struct intel_digital_port *intel_dig_port)
>  {
> -- 
> 2.1.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] drm/i915: fix intel_prepare_ddi
  2015-04-17 16:31 ` [PATCH v2 2/2] drm/i915: fix intel_prepare_ddi Imre Deak
  2015-04-23 21:37   ` shuang.he
@ 2015-04-27 17:29   ` Damien Lespiau
  2015-04-30  9:37     ` Jani Nikula
  1 sibling, 1 reply; 14+ messages in thread
From: Damien Lespiau @ 2015-04-27 17:29 UTC (permalink / raw)
  To: Imre Deak; +Cc: intel-gfx, paulo.r.zanoni

On Fri, Apr 17, 2015 at 07:31:22PM +0300, Imre Deak wrote:
> At the moment intel_prepare_ddi buffer will iterate through both MST and
> CRT encoders, which is incorrect. Neither of these encoder types have an
> embedding intel_digital_port object, so for these encoder types we will
> use random data when dereferencing the corresponding
> intel_digital_port->port field.
> 
> 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
> 
> v2:
> - fix getting at the port for MST encoders too
> - make sure that intel_prepare_ddi_buffers() gets called for port E too
>   (Paulo)
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>

Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

-- 
Damien

> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  5 -----
>  drivers/gpu/drm/i915/intel_ddi.c | 28 ++++++++++++++++++----------
>  2 files changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 40ef672..c461b56 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -251,11 +251,6 @@ 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_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 903d395..9c1e74a 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -256,12 +256,11 @@ intel_dig_port_supports_hdmi(const struct intel_digital_port *intel_dig_port)
>   * 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,
> -				      struct intel_digital_port *intel_dig_port)
> +static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port,
> +				      bool supports_hdmi)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 reg;
> -	int port = intel_dig_port->port;
>  	int i, n_hdmi_entries, n_dp_entries, n_edp_entries, hdmi_default_entry,
>  	    size;
>  	int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
> @@ -272,7 +271,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
>  	const struct ddi_buf_trans *ddi_translations;
>  
>  	if (IS_BROXTON(dev)) {
> -		if (!intel_dig_port_supports_hdmi(intel_dig_port))
> +		if (!supports_hdmi)
>  			return;
>  
>  		/* Vswing programming for HDMI */
> @@ -360,7 +359,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
>  		reg += 4;
>  	}
>  
> -	if (!intel_dig_port_supports_hdmi(intel_dig_port))
> +	if (!supports_hdmi)
>  		return;
>  
>  	/* Choose a good default if VBT is badly populated */
> @@ -380,18 +379,27 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
>   */
>  void intel_prepare_ddi(struct drm_device *dev)
>  {
> -	struct intel_digital_port *intel_dig_port;
> +	struct intel_encoder *intel_encoder;
>  	bool visited[I915_MAX_PORTS] = { 0, };
>  
>  	if (!HAS_DDI(dev))
>  		return;
>  
> -	for_each_digital_port(dev, intel_dig_port) {
> -		if (visited[intel_dig_port->port])
> +	for_each_intel_encoder(dev, intel_encoder) {
> +		struct intel_digital_port *intel_dig_port;
> +		enum port port;
> +		bool supports_hdmi;
> +
> +		ddi_get_encoder_port(intel_encoder, &intel_dig_port, &port);
> +
> +		if (visited[port])
>  			continue;
>  
> -		intel_prepare_ddi_buffers(dev, intel_dig_port);
> -		visited[intel_dig_port->port] = true;
> +		supports_hdmi = intel_dig_port &&
> +				intel_dig_port_supports_hdmi(intel_dig_port);
> +
> +		intel_prepare_ddi_buffers(dev, port, supports_hdmi);
> +		visited[port] = true;
>  	}
>  }
>  
> -- 
> 2.1.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/2] drm/i915: fix intel_prepare_ddi
  2015-04-27 17:29   ` Damien Lespiau
@ 2015-04-30  9:37     ` Jani Nikula
  0 siblings, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2015-04-30  9:37 UTC (permalink / raw)
  To: Damien Lespiau, Imre Deak; +Cc: intel-gfx, paulo.r.zanoni

On Mon, 27 Apr 2015, Damien Lespiau <damien.lespiau@intel.com> wrote:
> On Fri, Apr 17, 2015 at 07:31:22PM +0300, Imre Deak wrote:
>> At the moment intel_prepare_ddi buffer will iterate through both MST and
>> CRT encoders, which is incorrect. Neither of these encoder types have an
>> embedding intel_digital_port object, so for these encoder types we will
>> use random data when dereferencing the corresponding
>> intel_digital_port->port field.
>> 
>> 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
>> 
>> v2:
>> - fix getting at the port for MST encoders too
>> - make sure that intel_prepare_ddi_buffers() gets called for port E too
>>   (Paulo)
>> 
>> Signed-off-by: Imre Deak <imre.deak@intel.com>
>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Both pushed to drm-intel-next-queued, with the Bugzilla: tag moved to
2/2 which actually fixes the bug. Thanks for the patches and review.

BR,
Jani.


>
> -- 
> Damien
>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  5 -----
>>  drivers/gpu/drm/i915/intel_ddi.c | 28 ++++++++++++++++++----------
>>  2 files changed, 18 insertions(+), 15 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 40ef672..c461b56 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -251,11 +251,6 @@ 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_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 903d395..9c1e74a 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -256,12 +256,11 @@ intel_dig_port_supports_hdmi(const struct intel_digital_port *intel_dig_port)
>>   * 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,
>> -				      struct intel_digital_port *intel_dig_port)
>> +static void intel_prepare_ddi_buffers(struct drm_device *dev, enum port port,
>> +				      bool supports_hdmi)
>>  {
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	u32 reg;
>> -	int port = intel_dig_port->port;
>>  	int i, n_hdmi_entries, n_dp_entries, n_edp_entries, hdmi_default_entry,
>>  	    size;
>>  	int hdmi_level = dev_priv->vbt.ddi_port_info[port].hdmi_level_shift;
>> @@ -272,7 +271,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
>>  	const struct ddi_buf_trans *ddi_translations;
>>  
>>  	if (IS_BROXTON(dev)) {
>> -		if (!intel_dig_port_supports_hdmi(intel_dig_port))
>> +		if (!supports_hdmi)
>>  			return;
>>  
>>  		/* Vswing programming for HDMI */
>> @@ -360,7 +359,7 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
>>  		reg += 4;
>>  	}
>>  
>> -	if (!intel_dig_port_supports_hdmi(intel_dig_port))
>> +	if (!supports_hdmi)
>>  		return;
>>  
>>  	/* Choose a good default if VBT is badly populated */
>> @@ -380,18 +379,27 @@ static void intel_prepare_ddi_buffers(struct drm_device *dev,
>>   */
>>  void intel_prepare_ddi(struct drm_device *dev)
>>  {
>> -	struct intel_digital_port *intel_dig_port;
>> +	struct intel_encoder *intel_encoder;
>>  	bool visited[I915_MAX_PORTS] = { 0, };
>>  
>>  	if (!HAS_DDI(dev))
>>  		return;
>>  
>> -	for_each_digital_port(dev, intel_dig_port) {
>> -		if (visited[intel_dig_port->port])
>> +	for_each_intel_encoder(dev, intel_encoder) {
>> +		struct intel_digital_port *intel_dig_port;
>> +		enum port port;
>> +		bool supports_hdmi;
>> +
>> +		ddi_get_encoder_port(intel_encoder, &intel_dig_port, &port);
>> +
>> +		if (visited[port])
>>  			continue;
>>  
>> -		intel_prepare_ddi_buffers(dev, intel_dig_port);
>> -		visited[intel_dig_port->port] = true;
>> +		supports_hdmi = intel_dig_port &&
>> +				intel_dig_port_supports_hdmi(intel_dig_port);
>> +
>> +		intel_prepare_ddi_buffers(dev, port, supports_hdmi);
>> +		visited[port] = true;
>>  	}
>>  }
>>  
>> -- 
>> 2.1.0
>> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-04-30  9:35 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-17 11:58 [PATCH] drm/i915: fix for_each_digital_port Imre Deak
2015-04-17 12:27 ` Imre Deak
2015-04-17 12:36 ` Paulo Zanoni
2015-04-17 16:29   ` Imre Deak
2015-04-17 16:31 ` [PATCH v2 1/2] drm/i915: factor out ddi_get_encoder_port Imre Deak
2015-04-27 17:24   ` Damien Lespiau
2015-04-17 16:31 ` [PATCH v2 2/2] drm/i915: fix intel_prepare_ddi Imre Deak
2015-04-23 21:37   ` shuang.he
2015-04-27 17:29   ` Damien Lespiau
2015-04-30  9:37     ` Jani Nikula
2015-04-23 21:39 ` [PATCH] drm/i915: fix for_each_digital_port shuang.he
2015-04-27 17:05 ` Damien Lespiau
2015-04-27 17:15   ` Damien Lespiau
2015-04-27 17:18     ` Damien Lespiau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox