[PATCH 1/2] drm/i915: fix panel unlock register mask

[PATCH 1/2] drm/i915: fix panel unlock register mask

[Jani Nikula]

Use the correct mask for the unlock bits. In theory this could have lead
to incorrect asserts but this is unlikely in practise.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0b327ebb2d9e..fe1d00dc9ef5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1208,7 +1208,7 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
 
 	val = I915_READ(pp_reg);
 	if (!(val & PANEL_POWER_ON) ||
-	    ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS))
+	    ((val & PANEL_UNLOCK_MASK) == PANEL_UNLOCK_REGS))
 		locked = false;
 
 	if (I915_READ(lvds_reg) & LVDS_PIPEB_SELECT)
-- 
1.9.1

[PATCH 2/2] drm/i915: improve assert_panel_unlocked

[Jani Nikula]

Fix assert_panel_unlocked for vlv/chv, and improve it a bit for
non-LVDS. Also don't pretend it works for DDI. There's still work to do
to get this right for eDP on PCH platforms, but this is a start.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>

---

So I wanted to quickly fix assert_panel_unlocked, but for such a short
piece of code it's too involved to _quickly_ get right across all
platforms. I think this is a worthwhile improvement though.
---
 drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fe1d00dc9ef5..d6b48496d7f4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1193,17 +1193,33 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
 static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
 				  enum pipe pipe)
 {
-	int pp_reg, lvds_reg;
+	struct drm_device *dev = dev_priv->dev;
+	int pp_reg;
 	u32 val;
 	enum pipe panel_pipe = PIPE_A;
 	bool locked = true;
 
-	if (HAS_PCH_SPLIT(dev_priv->dev)) {
+	if (HAS_DDI(dev)) {
+		/* XXX: this neither works nor gets called for DDI */
+		return;
+	} else if (HAS_PCH_SPLIT(dev)) {
+		u32 port_sel;
+
 		pp_reg = PCH_PP_CONTROL;
-		lvds_reg = PCH_LVDS;
+		port_sel = I915_READ(PCH_PP_ON_DELAYS) & PANEL_PORT_SELECT_MASK;
+
+		if (port_sel == PANEL_PORT_SELECT_LVDS &&
+		    I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT)
+			panel_pipe = PIPE_B;
+		/* XXX: else fix for eDP */
+	} else if (IS_VALLEYVIEW(dev)) {
+		/* presumably write lock depends on pipe, not port select */
+		pp_reg = VLV_PIPE_PP_CONTROL(pipe);
+		panel_pipe = pipe;
 	} else {
 		pp_reg = PP_CONTROL;
-		lvds_reg = LVDS;
+		if (I915_READ(LVDS) & LVDS_PIPEB_SELECT)
+			panel_pipe = PIPE_B;
 	}
 
 	val = I915_READ(pp_reg);
@@ -1211,9 +1227,6 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
 	    ((val & PANEL_UNLOCK_MASK) == PANEL_UNLOCK_REGS))
 		locked = false;
 
-	if (I915_READ(lvds_reg) & LVDS_PIPEB_SELECT)
-		panel_pipe = PIPE_B;
-
 	WARN(panel_pipe == pipe && locked,
 	     "panel assertion failure, pipe %c regs locked\n",
 	     pipe_name(pipe));
-- 
1.9.1

Re: [PATCH 2/2] drm/i915: improve assert_panel_unlocked

[Ville Syrjälä]

On Thu, Aug 21, 2014 at 03:06:26PM +0300, Jani Nikula wrote:
> Fix assert_panel_unlocked for vlv/chv, and improve it a bit for
> non-LVDS. Also don't pretend it works for DDI. There's still work to do
> to get this right for eDP on PCH platforms, but this is a start.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> 
> ---
> 
> So I wanted to quickly fix assert_panel_unlocked, but for such a short
> piece of code it's too involved to _quickly_ get right across all
> platforms. I think this is a worthwhile improvement though.
> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fe1d00dc9ef5..d6b48496d7f4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1193,17 +1193,33 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
>  static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
>  				  enum pipe pipe)
>  {
> -	int pp_reg, lvds_reg;
> +	struct drm_device *dev = dev_priv->dev;
> +	int pp_reg;
>  	u32 val;
>  	enum pipe panel_pipe = PIPE_A;
>  	bool locked = true;
>  
> -	if (HAS_PCH_SPLIT(dev_priv->dev)) {
> +	if (HAS_DDI(dev)) {
> +		/* XXX: this neither works nor gets called for DDI */

Not sure why the XXX here. Seems to me there's nothing to fix here for
DDI. Maybe just make that a WARN_ON(HAS_DDI()) or just remove the check
entirely.

> +		return;
> +	} else if (HAS_PCH_SPLIT(dev)) {
> +		u32 port_sel;
> +
>  		pp_reg = PCH_PP_CONTROL;
> -		lvds_reg = PCH_LVDS;
> +		port_sel = I915_READ(PCH_PP_ON_DELAYS) & PANEL_PORT_SELECT_MASK;
> +
> +		if (port_sel == PANEL_PORT_SELECT_LVDS &&
> +		    I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT)
> +			panel_pipe = PIPE_B;
> +		/* XXX: else fix for eDP */
> +	} else if (IS_VALLEYVIEW(dev)) {
> +		/* presumably write lock depends on pipe, not port select */

Hmm. This is a good question. Needs a bit if testing I suppose. In the
worst case it somehow gets tied in with how the power sequencer gets locked
to the port. For that we'd probably just have to check both power sequencers
here and complain if either has the registers locked. Or maybe we should
just do that anyway because it's such a simple solution? But we could
do that simply by calling assert_panel_unlocked() twice (once for each pipe)
from VLV specific code, so this patch seems to be exactly what we need as
a first step.

Apart from the XXX in the comment:
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +		pp_reg = VLV_PIPE_PP_CONTROL(pipe);
> +		panel_pipe = pipe;
>  	} else {
>  		pp_reg = PP_CONTROL;
> -		lvds_reg = LVDS;
> +		if (I915_READ(LVDS) & LVDS_PIPEB_SELECT)
> +			panel_pipe = PIPE_B;
>  	}
>  
>  	val = I915_READ(pp_reg);
> @@ -1211,9 +1227,6 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
>  	    ((val & PANEL_UNLOCK_MASK) == PANEL_UNLOCK_REGS))
>  		locked = false;
>  
> -	if (I915_READ(lvds_reg) & LVDS_PIPEB_SELECT)
> -		panel_pipe = PIPE_B;
> -
>  	WARN(panel_pipe == pipe && locked,
>  	     "panel assertion failure, pipe %c regs locked\n",
>  	     pipe_name(pipe));
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 2/2] drm/i915: improve assert_panel_unlocked

[Paulo Zanoni]

2014-08-21 11:56 GMT-03:00 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Thu, Aug 21, 2014 at 03:06:26PM +0300, Jani Nikula wrote:
>> Fix assert_panel_unlocked for vlv/chv, and improve it a bit for
>> non-LVDS. Also don't pretend it works for DDI. There's still work to do
>> to get this right for eDP on PCH platforms, but this is a start.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>
>> ---
>>
>> So I wanted to quickly fix assert_panel_unlocked, but for such a short
>> piece of code it's too involved to _quickly_ get right across all
>> platforms. I think this is a worthwhile improvement though.
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++-------
>>  1 file changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index fe1d00dc9ef5..d6b48496d7f4 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -1193,17 +1193,33 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
>>  static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
>>                                 enum pipe pipe)
>>  {
>> -     int pp_reg, lvds_reg;
>> +     struct drm_device *dev = dev_priv->dev;
>> +     int pp_reg;
>>       u32 val;
>>       enum pipe panel_pipe = PIPE_A;
>>       bool locked = true;
>>
>> -     if (HAS_PCH_SPLIT(dev_priv->dev)) {
>> +     if (HAS_DDI(dev)) {
>> +             /* XXX: this neither works nor gets called for DDI */
>
> Not sure why the XXX here. Seems to me there's nothing to fix here for
> DDI. Maybe just make that a WARN_ON(HAS_DDI()) or just remove the check
> entirely.

As far as I remember, the "abcd" stuff is not even used/needed on DDI.
But this is just what my memory tells me, it may be wrong. Someone
needs to double-check.

>
>> +             return;
>> +     } else if (HAS_PCH_SPLIT(dev)) {
>> +             u32 port_sel;
>> +
>>               pp_reg = PCH_PP_CONTROL;
>> -             lvds_reg = PCH_LVDS;
>> +             port_sel = I915_READ(PCH_PP_ON_DELAYS) & PANEL_PORT_SELECT_MASK;
>> +
>> +             if (port_sel == PANEL_PORT_SELECT_LVDS &&
>> +                 I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT)
>> +                     panel_pipe = PIPE_B;
>> +             /* XXX: else fix for eDP */
>> +     } else if (IS_VALLEYVIEW(dev)) {
>> +             /* presumably write lock depends on pipe, not port select */
>
> Hmm. This is a good question. Needs a bit if testing I suppose. In the
> worst case it somehow gets tied in with how the power sequencer gets locked
> to the port. For that we'd probably just have to check both power sequencers
> here and complain if either has the registers locked. Or maybe we should
> just do that anyway because it's such a simple solution? But we could
> do that simply by calling assert_panel_unlocked() twice (once for each pipe)
> from VLV specific code, so this patch seems to be exactly what we need as
> a first step.
>
> Apart from the XXX in the comment:
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>> +             pp_reg = VLV_PIPE_PP_CONTROL(pipe);
>> +             panel_pipe = pipe;
>>       } else {
>>               pp_reg = PP_CONTROL;
>> -             lvds_reg = LVDS;
>> +             if (I915_READ(LVDS) & LVDS_PIPEB_SELECT)
>> +                     panel_pipe = PIPE_B;
>>       }
>>
>>       val = I915_READ(pp_reg);
>> @@ -1211,9 +1227,6 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
>>           ((val & PANEL_UNLOCK_MASK) == PANEL_UNLOCK_REGS))
>>               locked = false;
>>
>> -     if (I915_READ(lvds_reg) & LVDS_PIPEB_SELECT)
>> -             panel_pipe = PIPE_B;
>> -
>>       WARN(panel_pipe == pipe && locked,
>>            "panel assertion failure, pipe %c regs locked\n",
>>            pipe_name(pipe));
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> 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

Re: [PATCH 2/2] drm/i915: improve assert_panel_unlocked

[Ville Syrjälä]

On Thu, Aug 21, 2014 at 12:01:07PM -0300, Paulo Zanoni wrote:
> 2014-08-21 11:56 GMT-03:00 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> > On Thu, Aug 21, 2014 at 03:06:26PM +0300, Jani Nikula wrote:
> >> Fix assert_panel_unlocked for vlv/chv, and improve it a bit for
> >> non-LVDS. Also don't pretend it works for DDI. There's still work to do
> >> to get this right for eDP on PCH platforms, but this is a start.
> >>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >>
> >> ---
> >>
> >> So I wanted to quickly fix assert_panel_unlocked, but for such a short
> >> piece of code it's too involved to _quickly_ get right across all
> >> platforms. I think this is a worthwhile improvement though.
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++-------
> >>  1 file changed, 20 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index fe1d00dc9ef5..d6b48496d7f4 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -1193,17 +1193,33 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
> >>  static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
> >>                                 enum pipe pipe)
> >>  {
> >> -     int pp_reg, lvds_reg;
> >> +     struct drm_device *dev = dev_priv->dev;
> >> +     int pp_reg;
> >>       u32 val;
> >>       enum pipe panel_pipe = PIPE_A;
> >>       bool locked = true;
> >>
> >> -     if (HAS_PCH_SPLIT(dev_priv->dev)) {
> >> +     if (HAS_DDI(dev)) {
> >> +             /* XXX: this neither works nor gets called for DDI */
> >
> > Not sure why the XXX here. Seems to me there's nothing to fix here for
> > DDI. Maybe just make that a WARN_ON(HAS_DDI()) or just remove the check
> > entirely.
> 
> As far as I remember, the "abcd" stuff is not even used/needed on DDI.
> But this is just what my memory tells me, it may be wrong. Someone
> needs to double-check.

Bspec just says "spare" for those bits.

> 
> >
> >> +             return;
> >> +     } else if (HAS_PCH_SPLIT(dev)) {
> >> +             u32 port_sel;
> >> +
> >>               pp_reg = PCH_PP_CONTROL;
> >> -             lvds_reg = PCH_LVDS;
> >> +             port_sel = I915_READ(PCH_PP_ON_DELAYS) & PANEL_PORT_SELECT_MASK;
> >> +
> >> +             if (port_sel == PANEL_PORT_SELECT_LVDS &&
> >> +                 I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT)
> >> +                     panel_pipe = PIPE_B;
> >> +             /* XXX: else fix for eDP */
> >> +     } else if (IS_VALLEYVIEW(dev)) {
> >> +             /* presumably write lock depends on pipe, not port select */
> >
> > Hmm. This is a good question. Needs a bit if testing I suppose. In the
> > worst case it somehow gets tied in with how the power sequencer gets locked
> > to the port. For that we'd probably just have to check both power sequencers
> > here and complain if either has the registers locked. Or maybe we should
> > just do that anyway because it's such a simple solution? But we could
> > do that simply by calling assert_panel_unlocked() twice (once for each pipe)
> > from VLV specific code, so this patch seems to be exactly what we need as
> > a first step.
> >
> > Apart from the XXX in the comment:
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >> +             pp_reg = VLV_PIPE_PP_CONTROL(pipe);
> >> +             panel_pipe = pipe;
> >>       } else {
> >>               pp_reg = PP_CONTROL;
> >> -             lvds_reg = LVDS;
> >> +             if (I915_READ(LVDS) & LVDS_PIPEB_SELECT)
> >> +                     panel_pipe = PIPE_B;
> >>       }
> >>
> >>       val = I915_READ(pp_reg);
> >> @@ -1211,9 +1227,6 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
> >>           ((val & PANEL_UNLOCK_MASK) == PANEL_UNLOCK_REGS))
> >>               locked = false;
> >>
> >> -     if (I915_READ(lvds_reg) & LVDS_PIPEB_SELECT)
> >> -             panel_pipe = PIPE_B;
> >> -
> >>       WARN(panel_pipe == pipe && locked,
> >>            "panel assertion failure, pipe %c regs locked\n",
> >>            pipe_name(pipe));
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 2/2] drm/i915: improve assert_panel_unlocked

[Daniel Vetter]

On Fri, Aug 22, 2014 at 11:00:15AM +0300, Ville Syrjälä wrote:
> On Thu, Aug 21, 2014 at 12:01:07PM -0300, Paulo Zanoni wrote:
> > 2014-08-21 11:56 GMT-03:00 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> > > On Thu, Aug 21, 2014 at 03:06:26PM +0300, Jani Nikula wrote:
> > >> Fix assert_panel_unlocked for vlv/chv, and improve it a bit for
> > >> non-LVDS. Also don't pretend it works for DDI. There's still work to do
> > >> to get this right for eDP on PCH platforms, but this is a start.
> > >>
> > >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > >>
> > >> ---
> > >>
> > >> So I wanted to quickly fix assert_panel_unlocked, but for such a short
> > >> piece of code it's too involved to _quickly_ get right across all
> > >> platforms. I think this is a worthwhile improvement though.
> > >> ---
> > >>  drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++-------
> > >>  1 file changed, 20 insertions(+), 7 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > >> index fe1d00dc9ef5..d6b48496d7f4 100644
> > >> --- a/drivers/gpu/drm/i915/intel_display.c
> > >> +++ b/drivers/gpu/drm/i915/intel_display.c
> > >> @@ -1193,17 +1193,33 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
> > >>  static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
> > >>                                 enum pipe pipe)
> > >>  {
> > >> -     int pp_reg, lvds_reg;
> > >> +     struct drm_device *dev = dev_priv->dev;
> > >> +     int pp_reg;
> > >>       u32 val;
> > >>       enum pipe panel_pipe = PIPE_A;
> > >>       bool locked = true;
> > >>
> > >> -     if (HAS_PCH_SPLIT(dev_priv->dev)) {
> > >> +     if (HAS_DDI(dev)) {
> > >> +             /* XXX: this neither works nor gets called for DDI */
> > >
> > > Not sure why the XXX here. Seems to me there's nothing to fix here for
> > > DDI. Maybe just make that a WARN_ON(HAS_DDI()) or just remove the check
> > > entirely.
> > 
> > As far as I remember, the "abcd" stuff is not even used/needed on DDI.
> > But this is just what my memory tells me, it may be wrong. Someone
> > needs to double-check.
> 
> Bspec just says "spare" for those bits.

Iirc it's also not used for edp on older platforms, but only for lvds.
There's definitely a lot of fun in this area ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH v2 2/2] drm/i915: improve assert_panel_unlocked

[Jani Nikula]

Fix assert_panel_unlocked for vlv/chv, and improve it a bit for
non-LVDS. Also don't pretend it works for DDI. There's still work to do
to get this right for eDP on PCH platforms, but this is a start.

v2: WARN_ON(HAS_DDI)

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fe1d00dc9ef5..51b4cd29f932 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1193,17 +1193,33 @@ void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
 static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
 				  enum pipe pipe)
 {
-	int pp_reg, lvds_reg;
+	struct drm_device *dev = dev_priv->dev;
+	int pp_reg;
 	u32 val;
 	enum pipe panel_pipe = PIPE_A;
 	bool locked = true;
 
-	if (HAS_PCH_SPLIT(dev_priv->dev)) {
+	if (WARN_ON(HAS_DDI(dev)))
+		return;
+
+	if (HAS_PCH_SPLIT(dev)) {
+		u32 port_sel;
+
 		pp_reg = PCH_PP_CONTROL;
-		lvds_reg = PCH_LVDS;
+		port_sel = I915_READ(PCH_PP_ON_DELAYS) & PANEL_PORT_SELECT_MASK;
+
+		if (port_sel == PANEL_PORT_SELECT_LVDS &&
+		    I915_READ(PCH_LVDS) & LVDS_PIPEB_SELECT)
+			panel_pipe = PIPE_B;
+		/* XXX: else fix for eDP */
+	} else if (IS_VALLEYVIEW(dev)) {
+		/* presumably write lock depends on pipe, not port select */
+		pp_reg = VLV_PIPE_PP_CONTROL(pipe);
+		panel_pipe = pipe;
 	} else {
 		pp_reg = PP_CONTROL;
-		lvds_reg = LVDS;
+		if (I915_READ(LVDS) & LVDS_PIPEB_SELECT)
+			panel_pipe = PIPE_B;
 	}
 
 	val = I915_READ(pp_reg);
@@ -1211,9 +1227,6 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
 	    ((val & PANEL_UNLOCK_MASK) == PANEL_UNLOCK_REGS))
 		locked = false;
 
-	if (I915_READ(lvds_reg) & LVDS_PIPEB_SELECT)
-		panel_pipe = PIPE_B;
-
 	WARN(panel_pipe == pipe && locked,
 	     "panel assertion failure, pipe %c regs locked\n",
 	     pipe_name(pipe));
-- 
1.9.1

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

Re: [PATCH 1/2] drm/i915: fix panel unlock register mask

[Paulo Zanoni]

2014-08-21 9:06 GMT-03:00 Jani Nikula <jani.nikula@intel.com>:
> Use the correct mask for the unlock bits. In theory this could have lead
> to incorrect asserts but this is unlikely in practise.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0b327ebb2d9e..fe1d00dc9ef5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1208,7 +1208,7 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
>
>         val = I915_READ(pp_reg);
>         if (!(val & PANEL_POWER_ON) ||
> -           ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS))
> +           ((val & PANEL_UNLOCK_MASK) == PANEL_UNLOCK_REGS))
>                 locked = false;
>
>         if (I915_READ(lvds_reg) & LVDS_PIPEB_SELECT)
> --
> 1.9.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

Re: [PATCH 1/2] drm/i915: fix panel unlock register mask

[Ville Syrjälä]

On Thu, Aug 21, 2014 at 03:06:25PM +0300, Jani Nikula wrote:
> Use the correct mask for the unlock bits. In theory this could have lead
> to incorrect asserts but this is unlikely in practise.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0b327ebb2d9e..fe1d00dc9ef5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1208,7 +1208,7 @@ static void assert_panel_unlocked(struct drm_i915_private *dev_priv,
>  
>  	val = I915_READ(pp_reg);
>  	if (!(val & PANEL_POWER_ON) ||
> -	    ((val & PANEL_UNLOCK_REGS) == PANEL_UNLOCK_REGS))
> +	    ((val & PANEL_UNLOCK_MASK) == PANEL_UNLOCK_REGS))
>  		locked = false;
>  
>  	if (I915_READ(lvds_reg) & LVDS_PIPEB_SELECT)
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC