* [PATCH 1/2] drm/i915: fix panel unlock register mask
@ 2014-08-21 12:06 Jani Nikula
2014-08-21 12:06 ` [PATCH 2/2] drm/i915: improve assert_panel_unlocked Jani Nikula
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Jani Nikula @ 2014-08-21 12:06 UTC (permalink / raw)
To: intel-gfx; +Cc: 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] drm/i915: improve assert_panel_unlocked
2014-08-21 12:06 [PATCH 1/2] drm/i915: fix panel unlock register mask Jani Nikula
@ 2014-08-21 12:06 ` Jani Nikula
2014-08-21 14:56 ` Ville Syrjälä
2014-08-22 12:04 ` [PATCH v2 " Jani Nikula
2014-08-21 13:36 ` [PATCH 1/2] drm/i915: fix panel unlock register mask Paulo Zanoni
2014-08-21 13:39 ` Ville Syrjälä
2 siblings, 2 replies; 9+ messages in thread
From: Jani Nikula @ 2014-08-21 12:06 UTC (permalink / raw)
To: intel-gfx; +Cc: 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] drm/i915: fix panel unlock register mask
2014-08-21 12:06 [PATCH 1/2] drm/i915: fix panel unlock register mask Jani Nikula
2014-08-21 12:06 ` [PATCH 2/2] drm/i915: improve assert_panel_unlocked Jani Nikula
@ 2014-08-21 13:36 ` Paulo Zanoni
2014-08-21 13:39 ` Ville Syrjälä
2 siblings, 0 replies; 9+ messages in thread
From: Paulo Zanoni @ 2014-08-21 13:36 UTC (permalink / raw)
To: Jani Nikula; +Cc: Intel Graphics Development
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] drm/i915: fix panel unlock register mask
2014-08-21 12:06 [PATCH 1/2] drm/i915: fix panel unlock register mask Jani Nikula
2014-08-21 12:06 ` [PATCH 2/2] drm/i915: improve assert_panel_unlocked Jani Nikula
2014-08-21 13:36 ` [PATCH 1/2] drm/i915: fix panel unlock register mask Paulo Zanoni
@ 2014-08-21 13:39 ` Ville Syrjälä
2 siblings, 0 replies; 9+ messages in thread
From: Ville Syrjälä @ 2014-08-21 13:39 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/i915: improve assert_panel_unlocked
2014-08-21 12:06 ` [PATCH 2/2] drm/i915: improve assert_panel_unlocked Jani Nikula
@ 2014-08-21 14:56 ` Ville Syrjälä
2014-08-21 15:01 ` Paulo Zanoni
2014-08-22 12:04 ` [PATCH v2 " Jani Nikula
1 sibling, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2014-08-21 14:56 UTC (permalink / raw)
To: Jani Nikula; +Cc: intel-gfx
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/i915: improve assert_panel_unlocked
2014-08-21 14:56 ` Ville Syrjälä
@ 2014-08-21 15:01 ` Paulo Zanoni
2014-08-22 8:00 ` Ville Syrjälä
0 siblings, 1 reply; 9+ messages in thread
From: Paulo Zanoni @ 2014-08-21 15:01 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Jani Nikula, Intel Graphics Development
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/i915: improve assert_panel_unlocked
2014-08-21 15:01 ` Paulo Zanoni
@ 2014-08-22 8:00 ` Ville Syrjälä
2014-08-26 14:38 ` Daniel Vetter
0 siblings, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2014-08-22 8:00 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Jani Nikula, Intel Graphics Development
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/2] drm/i915: improve assert_panel_unlocked
2014-08-21 12:06 ` [PATCH 2/2] drm/i915: improve assert_panel_unlocked Jani Nikula
2014-08-21 14:56 ` Ville Syrjälä
@ 2014-08-22 12:04 ` Jani Nikula
1 sibling, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2014-08-22 12:04 UTC (permalink / raw)
To: intel-gfx; +Cc: 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/i915: improve assert_panel_unlocked
2014-08-22 8:00 ` Ville Syrjälä
@ 2014-08-26 14:38 ` Daniel Vetter
0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2014-08-26 14:38 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Jani Nikula, Intel Graphics Development
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
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-08-26 14:38 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-21 12:06 [PATCH 1/2] drm/i915: fix panel unlock register mask Jani Nikula
2014-08-21 12:06 ` [PATCH 2/2] drm/i915: improve assert_panel_unlocked Jani Nikula
2014-08-21 14:56 ` Ville Syrjälä
2014-08-21 15:01 ` Paulo Zanoni
2014-08-22 8:00 ` Ville Syrjälä
2014-08-26 14:38 ` Daniel Vetter
2014-08-22 12:04 ` [PATCH v2 " Jani Nikula
2014-08-21 13:36 ` [PATCH 1/2] drm/i915: fix panel unlock register mask Paulo Zanoni
2014-08-21 13:39 ` Ville Syrjälä
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.