Re: [Intel-gfx] [PATCH] drm/i915/bxt: Use intel_encoder->hpd_pin to check live status

2015-09-14 Thread Daniel Vetter
On Mon, Sep 14, 2015 at 02:08:41PM +0530, Jindal, Sonika wrote:
> 
> 
> On 9/14/2015 2:04 PM, Daniel Vetter wrote:
> >On Fri, Sep 11, 2015 at 04:58:32PM +0530, Sonika Jindal wrote:
> >>Using intel_encoder's hpd_pin to check the live status
> >>because of BXT A0/A1 WA for HPD pins and hpd_pin contains the
> >>updated pin for the corresponding port.
> >>
> >>Signed-off-by: Sonika Jindal 
> >>---
> >>  drivers/gpu/drm/i915/intel_dp.c |9 ++---
> >>  1 file changed, 6 insertions(+), 3 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> >>b/drivers/gpu/drm/i915/intel_dp.c
> >>index 796f930..bf17030 100644
> >>--- a/drivers/gpu/drm/i915/intel_dp.c
> >>+++ b/drivers/gpu/drm/i915/intel_dp.c
> >>@@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct 
> >>drm_i915_private *dev_priv,
> >>  }
> >>
> >>  static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
> >>-  struct intel_digital_port *port)
> >>+  struct intel_digital_port 
> >>*intel_dig_port)
> >>  {
> >>+   struct intel_encoder *intel_encoder = &intel_dig_port->base;
> >>+   enum port port;
> >>u32 bit;
> >>
> >>-   switch (port->port) {
> >>+   intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port);
> >
> >This looks very wrong - the function you're calling here maps from hpd_pin
> >to the port, but what you actually want is to map from port to pin. That
> >mapping is in intel_encoder->hpd_pin.
> >-Daniel
> No, I want to find out the correct port whose hpd bit we need to check.
> That port we are finding based upon the hpd_pin we have set for that
> encoder. This was the whole point of using hpd_pin for the BXT A0/A1 WA.
> That instead of checking different HPD bit based upon the port, just change
> the hpd_pin itself in intel_encoder.

Ah right I was getting confused. Patch applied now, thanks for clarifying.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/bxt: Use intel_encoder->hpd_pin to check live status

2015-09-14 Thread Jindal, Sonika



On 9/14/2015 2:04 PM, Daniel Vetter wrote:

On Fri, Sep 11, 2015 at 04:58:32PM +0530, Sonika Jindal wrote:

Using intel_encoder's hpd_pin to check the live status
because of BXT A0/A1 WA for HPD pins and hpd_pin contains the
updated pin for the corresponding port.

Signed-off-by: Sonika Jindal 
---
  drivers/gpu/drm/i915/intel_dp.c |9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796f930..bf17030 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct 
drm_i915_private *dev_priv,
  }

  static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
-  struct intel_digital_port *port)
+  struct intel_digital_port 
*intel_dig_port)
  {
+   struct intel_encoder *intel_encoder = &intel_dig_port->base;
+   enum port port;
u32 bit;

-   switch (port->port) {
+   intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port);


This looks very wrong - the function you're calling here maps from hpd_pin
to the port, but what you actually want is to map from port to pin. That
mapping is in intel_encoder->hpd_pin.
-Daniel

No, I want to find out the correct port whose hpd bit we need to check.
That port we are finding based upon the hpd_pin we have set for that 
encoder. This was the whole point of using hpd_pin for the BXT A0/A1 WA.
That instead of checking different HPD bit based upon the port, just 
change the hpd_pin itself in intel_encoder.


Regards,
Sonika



+   switch (port) {
case PORT_A:
bit = BXT_DE_PORT_HP_DDIA;
break;
@@ -4678,7 +4681,7 @@ static bool bxt_digital_port_connected(struct 
drm_i915_private *dev_priv,
bit = BXT_DE_PORT_HP_DDIC;
break;
default:
-   MISSING_CASE(port->port);
+   MISSING_CASE(port);
return false;
}

--
1.7.10.4

___
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


Re: [Intel-gfx] [PATCH] drm/i915/bxt: Use intel_encoder->hpd_pin to check live status

2015-09-14 Thread Daniel Vetter
On Fri, Sep 11, 2015 at 04:58:32PM +0530, Sonika Jindal wrote:
> Using intel_encoder's hpd_pin to check the live status
> because of BXT A0/A1 WA for HPD pins and hpd_pin contains the
> updated pin for the corresponding port.
> 
> Signed-off-by: Sonika Jindal 
> ---
>  drivers/gpu/drm/i915/intel_dp.c |9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 796f930..bf17030 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct 
> drm_i915_private *dev_priv,
>  }
>  
>  static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
> -struct intel_digital_port *port)
> +struct intel_digital_port 
> *intel_dig_port)
>  {
> + struct intel_encoder *intel_encoder = &intel_dig_port->base;
> + enum port port;
>   u32 bit;
>  
> - switch (port->port) {
> + intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port);

This looks very wrong - the function you're calling here maps from hpd_pin
to the port, but what you actually want is to map from port to pin. That
mapping is in intel_encoder->hpd_pin.
-Daniel

> + switch (port) {
>   case PORT_A:
>   bit = BXT_DE_PORT_HP_DDIA;
>   break;
> @@ -4678,7 +4681,7 @@ static bool bxt_digital_port_connected(struct 
> drm_i915_private *dev_priv,
>   bit = BXT_DE_PORT_HP_DDIC;
>   break;
>   default:
> - MISSING_CASE(port->port);
> + MISSING_CASE(port);
>   return false;
>   }
>  
> -- 
> 1.7.10.4
> 
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH] drm/i915/bxt: Use intel_encoder->hpd_pin to check live status

2015-09-12 Thread Jindal, Sonika
Thanks Rodrigo.
This is always true. The hpd_pin is the right pin which we set for particular 
port. So no fallback needed.

Regards,
Sonika

From: Rodrigo Vivi [mailto:rodrigo.v...@gmail.com]
Sent: Friday, September 11, 2015 11:31 PM
To: Jindal, Sonika; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915/bxt: Use intel_encoder->hpd_pin to 
check live status


On Fri, Sep 11, 2015 at 4:40 AM Sonika Jindal 
mailto:sonika.jin...@intel.com>> wrote:
Using intel_encoder's hpd_pin to check the live status
because of BXT A0/A1 WA for HPD pins and hpd_pin contains the
updated pin for the corresponding port.

It makes sense, but is it always true? or we should have a fallback to 
intel_dig_port->port in some case or in some stepping after A0/A1?

With this clarified feel free to use:
Reviewed-by: Rodrigo Vivi 
mailto:rodrigo.v...@intel.com>>


Signed-off-by: Sonika Jindal 
mailto:sonika.jin...@intel.com>>
---
 drivers/gpu/drm/i915/intel_dp.c |9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796f930..bf17030 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct 
drm_i915_private *dev_priv,
 }

 static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
-  struct intel_digital_port *port)
+  struct intel_digital_port 
*intel_dig_port)
 {
+   struct intel_encoder *intel_encoder = &intel_dig_port->base;
+   enum port port;
u32 bit;

-   switch (port->port) {
+   intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port);
+   switch (port) {
case PORT_A:
bit = BXT_DE_PORT_HP_DDIA;
break;
@@ -4678,7 +4681,7 @@ static bool bxt_digital_port_connected(struct 
drm_i915_private *dev_priv,
bit = BXT_DE_PORT_HP_DDIC;
break;
default:
-   MISSING_CASE(port->port);
+   MISSING_CASE(port);
return false;
}

--
1.7.10.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org<mailto: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


Re: [Intel-gfx] [PATCH] drm/i915/bxt: Use intel_encoder->hpd_pin to check live status

2015-09-11 Thread Rodrigo Vivi
On Fri, Sep 11, 2015 at 4:40 AM Sonika Jindal 
wrote:

> Using intel_encoder's hpd_pin to check the live status
> because of BXT A0/A1 WA for HPD pins and hpd_pin contains the
> updated pin for the corresponding port.
>

It makes sense, but is it always true? or we should have a fallback to
intel_dig_port->port in some case or in some stepping after A0/A1?

With this clarified feel free to use:
Reviewed-by: Rodrigo Vivi 


>
> Signed-off-by: Sonika Jindal 
> ---
>  drivers/gpu/drm/i915/intel_dp.c |9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 796f930..bf17030 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct
> drm_i915_private *dev_priv,
>  }
>
>  static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
> -  struct intel_digital_port *port)
> +  struct intel_digital_port
> *intel_dig_port)
>  {
> +   struct intel_encoder *intel_encoder = &intel_dig_port->base;
> +   enum port port;
> u32 bit;
>
> -   switch (port->port) {
> +   intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port);
> +   switch (port) {
> case PORT_A:
> bit = BXT_DE_PORT_HP_DDIA;
> break;
> @@ -4678,7 +4681,7 @@ static bool bxt_digital_port_connected(struct
> drm_i915_private *dev_priv,
> bit = BXT_DE_PORT_HP_DDIC;
> break;
> default:
> -   MISSING_CASE(port->port);
> +   MISSING_CASE(port);
> return false;
> }
>
> --
> 1.7.10.4
>
> ___
> 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


[Intel-gfx] [PATCH] drm/i915/bxt: Use intel_encoder->hpd_pin to check live status

2015-09-11 Thread Sonika Jindal
Using intel_encoder's hpd_pin to check the live status
because of BXT A0/A1 WA for HPD pins and hpd_pin contains the
updated pin for the corresponding port.

Signed-off-by: Sonika Jindal 
---
 drivers/gpu/drm/i915/intel_dp.c |9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796f930..bf17030 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4663,11 +4663,14 @@ static bool vlv_digital_port_connected(struct 
drm_i915_private *dev_priv,
 }
 
 static bool bxt_digital_port_connected(struct drm_i915_private *dev_priv,
-  struct intel_digital_port *port)
+  struct intel_digital_port 
*intel_dig_port)
 {
+   struct intel_encoder *intel_encoder = &intel_dig_port->base;
+   enum port port;
u32 bit;
 
-   switch (port->port) {
+   intel_hpd_pin_to_port(intel_encoder->hpd_pin, &port);
+   switch (port) {
case PORT_A:
bit = BXT_DE_PORT_HP_DDIA;
break;
@@ -4678,7 +4681,7 @@ static bool bxt_digital_port_connected(struct 
drm_i915_private *dev_priv,
bit = BXT_DE_PORT_HP_DDIC;
break;
default:
-   MISSING_CASE(port->port);
+   MISSING_CASE(port);
return false;
}
 
-- 
1.7.10.4

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