[PATCH] drm/i915: disable_shared_pll doesn't work on pre-gen5

[PATCH] drm/i915: disable_shared_pll doesn't work on pre-gen5

[Jesse Barnes]

Looks like

commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6
Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Date:   Mon Jun 15 12:33:53 2015 +0200
    drm/i915: Update less state during modeset.

introduced the unconditional calling of disable_shared_dpll, but didn't
fix up pre-gen5 to avoid the BUG_ON at the top of the function.

So change the BUG_ON into a gen check (alternately we could move the
BUG_ON until later, since we shouldn't have a pll struct here either,
but this seems clearer to read).

This fixes a crash on load on my x200s platform.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_display.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 43b0f17..b5e79b3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -1936,7 +1936,9 @@ static void intel_disable_shared_dpll(struct intel_crtc *crtc)
 	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
 
 	/* PCH only available on ILK+ */
-	BUG_ON(INTEL_INFO(dev)->gen < 5);
+	if (INTEL_INFO(dev)->gen < 5)
+		return;
+
 	if (pll == NULL)
 		return;
 
-- 
1.9.1

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

Re: [PATCH] drm/i915: disable_shared_pll doesn't work on pre-gen5

[Maarten Lankhorst]

Op 03-08-15 om 22:09 schreef Jesse Barnes:
> Looks like
>
> commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6
> Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Date:   Mon Jun 15 12:33:53 2015 +0200
>     drm/i915: Update less state during modeset.
>
> introduced the unconditional calling of disable_shared_dpll, but didn't
> fix up pre-gen5 to avoid the BUG_ON at the top of the function.
>
> So change the BUG_ON into a gen check (alternately we could move the
> BUG_ON until later, since we shouldn't have a pll struct here either,
> but this seems clearer to read).
>
> This fixes a crash on load on my x200s platform.
>
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 43b0f17..b5e79b3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1936,7 +1936,9 @@ static void intel_disable_shared_dpll(struct intel_crtc *crtc)
>  	struct intel_shared_dpll *pll = intel_crtc_to_shared_dpll(crtc);
>  
>  	/* PCH only available on ILK+ */
> -	BUG_ON(INTEL_INFO(dev)->gen < 5);
> +	if (INTEL_INFO(dev)->gen < 5)
> +		return;
> +
>  	if (pll == NULL)
>  		return;
>  
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Intel test bot doesn't run on < ilk? :(
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable_shared_pll doesn't work on pre-gen5

[Chris Wilson]

On Mon, Aug 03, 2015 at 01:09:11PM -0700, Jesse Barnes wrote:
> Looks like
> 
> commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6
> Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Date:   Mon Jun 15 12:33:53 2015 +0200
>     drm/i915: Update less state during modeset.
> 
> introduced the unconditional calling of disable_shared_dpll, but didn't
> fix up pre-gen5 to avoid the BUG_ON at the top of the function.
> 
> So change the BUG_ON into a gen check (alternately we could move the
> BUG_ON until later, since we shouldn't have a pll struct here either,
> but this seems clearer to read).
> 
> This fixes a crash on load on my x200s platform.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

This blows up in 4.1 (a BUG_ON during boot causing a hard lockup).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: disable_shared_pll doesn't work on pre-gen5

[Daniel Vetter]

On Mon, Aug 10, 2015 at 11:15:30AM +0100, Chris Wilson wrote:
> On Mon, Aug 03, 2015 at 01:09:11PM -0700, Jesse Barnes wrote:
> > Looks like
> > 
> > commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6
> > Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Date:   Mon Jun 15 12:33:53 2015 +0200
> >     drm/i915: Update less state during modeset.
> > 
> > introduced the unconditional calling of disable_shared_dpll, but didn't
> > fix up pre-gen5 to avoid the BUG_ON at the top of the function.
> > 
> > So change the BUG_ON into a gen check (alternately we could move the
> > BUG_ON until later, since we shouldn't have a pll struct here either,
> > but this seems clearer to read).
> > 
> > This fixes a crash on load on my x200s platform.
> > 
> > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> This blows up in 4.1 (a BUG_ON during boot causing a hard lockup).

The reference offending commit is 4.3 only. Are you sure it's the same
BUG_ON? Which machine?
-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: [PATCH] drm/i915: disable_shared_pll doesn't work on pre-gen5

[Chris Wilson]

On Tue, Aug 11, 2015 at 11:30:48AM +0200, Daniel Vetter wrote:
> On Mon, Aug 10, 2015 at 11:15:30AM +0100, Chris Wilson wrote:
> > On Mon, Aug 03, 2015 at 01:09:11PM -0700, Jesse Barnes wrote:
> > > Looks like
> > > 
> > > commit eddfcbcdc27fbecb33bff098967bbdd7ca75bfa6
> > > Author: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > Date:   Mon Jun 15 12:33:53 2015 +0200
> > >     drm/i915: Update less state during modeset.
> > > 
> > > introduced the unconditional calling of disable_shared_dpll, but didn't
> > > fix up pre-gen5 to avoid the BUG_ON at the top of the function.
> > > 
> > > So change the BUG_ON into a gen check (alternately we could move the
> > > BUG_ON until later, since we shouldn't have a pll struct here either,
> > > but this seems clearer to read).
> > > 
> > > This fixes a crash on load on my x200s platform.
> > > 
> > > Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > This blows up in 4.1 (a BUG_ON during boot causing a hard lockup).
> 
> The reference offending commit is 4.3 only. Are you sure it's the same
> BUG_ON? Which machine?

It's entirely probably that I messed up my bisect, something about
compiling a kernel a day for a couple of weeks and forgetting which step
it is at...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx