public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Fix divide by zero on watermark update
@ 2015-07-16 16:00 Mika Kuoppala
  2015-07-16 16:07 ` Paulo Zanoni
  0 siblings, 1 reply; 8+ messages in thread
From: Mika Kuoppala @ 2015-07-16 16:00 UTC (permalink / raw)
  To: intel-gfx

Fix divide by zero if we end up updating the watermarks
with zero dotclock.

This is a stop gap measure to allow module load in cases
where our state keeping fails.

Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5eeddc9..755f0e8 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3316,8 +3316,10 @@ skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
 	if (!to_intel_crtc(crtc)->active)
 		return 0;
 
-	return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
+	if (p->pixel_rate == 0)
+		return 0;
 
+	return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
 }
 
 static void skl_compute_transition_wm(struct drm_crtc *crtc,
-- 
2.1.4

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

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

* Re: [PATCH] drm/i915: Fix divide by zero on watermark update
  2015-07-16 16:00 [PATCH] drm/i915: Fix divide by zero on watermark update Mika Kuoppala
@ 2015-07-16 16:07 ` Paulo Zanoni
  2015-07-16 16:36   ` Mika Kuoppala
  0 siblings, 1 reply; 8+ messages in thread
From: Paulo Zanoni @ 2015-07-16 16:07 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: Intel Graphics Development

2015-07-16 13:00 GMT-03:00 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
> Fix divide by zero if we end up updating the watermarks
> with zero dotclock.
>
> This is a stop gap measure to allow module load in cases
> where our state keeping fails.

Shouldn't we WARN() here?

>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5eeddc9..755f0e8 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3316,8 +3316,10 @@ skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
>         if (!to_intel_crtc(crtc)->active)
>                 return 0;
>
> -       return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
> +       if (p->pixel_rate == 0)
> +               return 0;
>
> +       return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
>  }
>
>  static void skl_compute_transition_wm(struct drm_crtc *crtc,
> --
> 2.1.4
>
> _______________________________________________
> 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] 8+ messages in thread

* [PATCH] drm/i915: Fix divide by zero on watermark update
  2015-07-16 16:07 ` Paulo Zanoni
@ 2015-07-16 16:36   ` Mika Kuoppala
  2015-07-16 16:43     ` Paulo Zanoni
  2015-07-16 16:44     ` Damien Lespiau
  0 siblings, 2 replies; 8+ messages in thread
From: Mika Kuoppala @ 2015-07-16 16:36 UTC (permalink / raw)
  To: intel-gfx

Fix divide by zero if we end up updating the watermarks
with zero dotclock.

This is a stop gap measure to allow module load in cases
where our state keeping fails.

v2: WARN_ON added (Paulo)

Cc: Paulo Zanoni <przanoni@gmail.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_pm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5eeddc9..0d3e014 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3316,8 +3316,10 @@ skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
 	if (!to_intel_crtc(crtc)->active)
 		return 0;
 
-	return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
+	if (WARN_ON(p->pixel_rate == 0))
+		return 0;
 
+	return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
 }
 
 static void skl_compute_transition_wm(struct drm_crtc *crtc,
-- 
2.1.4

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

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

* Re: [PATCH] drm/i915: Fix divide by zero on watermark update
  2015-07-16 16:36   ` Mika Kuoppala
@ 2015-07-16 16:43     ` Paulo Zanoni
  2015-07-17  7:10       ` Daniel Vetter
  2015-07-16 16:44     ` Damien Lespiau
  1 sibling, 1 reply; 8+ messages in thread
From: Paulo Zanoni @ 2015-07-16 16:43 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: Intel Graphics Development

2015-07-16 13:36 GMT-03:00 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
> Fix divide by zero if we end up updating the watermarks
> with zero dotclock.
>
> This is a stop gap measure to allow module load in cases
> where our state keeping fails.
>
> v2: WARN_ON added (Paulo)

Since we're not hiding the problem (due to the WARN_ON) and the patch
improves the current situation:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5eeddc9..0d3e014 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3316,8 +3316,10 @@ skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
>         if (!to_intel_crtc(crtc)->active)
>                 return 0;
>
> -       return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
> +       if (WARN_ON(p->pixel_rate == 0))
> +               return 0;
>
> +       return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
>  }
>
>  static void skl_compute_transition_wm(struct drm_crtc *crtc,
> --
> 2.1.4
>



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

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

* Re: [PATCH] drm/i915: Fix divide by zero on watermark update
  2015-07-16 16:36   ` Mika Kuoppala
  2015-07-16 16:43     ` Paulo Zanoni
@ 2015-07-16 16:44     ` Damien Lespiau
  2015-07-16 17:26       ` Damien Lespiau
  2015-08-12 14:26       ` Jani Nikula
  1 sibling, 2 replies; 8+ messages in thread
From: Damien Lespiau @ 2015-07-16 16:44 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Thu, Jul 16, 2015 at 07:36:51PM +0300, Mika Kuoppala wrote:
> Fix divide by zero if we end up updating the watermarks
> with zero dotclock.
> 
> This is a stop gap measure to allow module load in cases
> where our state keeping fails.
> 
> v2: WARN_ON added (Paulo)
> 
> Cc: Paulo Zanoni <przanoni@gmail.com>
> Cc: Damien Lespiau <damien.lespiau@intel.com>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>

I want to say a loading module is more important than a proper fix, so:

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

-- 
Damien

> ---
>  drivers/gpu/drm/i915/intel_pm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5eeddc9..0d3e014 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3316,8 +3316,10 @@ skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
>  	if (!to_intel_crtc(crtc)->active)
>  		return 0;
>  
> -	return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
> +	if (WARN_ON(p->pixel_rate == 0))
> +		return 0;
>  
> +	return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
>  }
>  
>  static void skl_compute_transition_wm(struct drm_crtc *crtc,
> -- 
> 2.1.4
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Fix divide by zero on watermark update
  2015-07-16 16:44     ` Damien Lespiau
@ 2015-07-16 17:26       ` Damien Lespiau
  2015-08-12 14:26       ` Jani Nikula
  1 sibling, 0 replies; 8+ messages in thread
From: Damien Lespiau @ 2015-07-16 17:26 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Thu, Jul 16, 2015 at 05:44:17PM +0100, Damien Lespiau wrote:
> On Thu, Jul 16, 2015 at 07:36:51PM +0300, Mika Kuoppala wrote:
> > Fix divide by zero if we end up updating the watermarks
> > with zero dotclock.
> > 
> > This is a stop gap measure to allow module load in cases
> > where our state keeping fails.
> > 
> > v2: WARN_ON added (Paulo)
> > 
> > Cc: Paulo Zanoni <przanoni@gmail.com>
> > Cc: Damien Lespiau <damien.lespiau@intel.com>
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> I want to say a loading module is more important than a proper fix, so:
> 
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>

Also it'd be great to ammend the prefix: 'drm/i915/skl'.

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

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

* Re: [PATCH] drm/i915: Fix divide by zero on watermark update
  2015-07-16 16:43     ` Paulo Zanoni
@ 2015-07-17  7:10       ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2015-07-17  7:10 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: Intel Graphics Development

On Thu, Jul 16, 2015 at 01:43:15PM -0300, Paulo Zanoni wrote:
> 2015-07-16 13:36 GMT-03:00 Mika Kuoppala <mika.kuoppala@linux.intel.com>:
> > Fix divide by zero if we end up updating the watermarks
> > with zero dotclock.
> >
> > This is a stop gap measure to allow module load in cases
> > where our state keeping fails.
> >
> > v2: WARN_ON added (Paulo)
> 
> Since we're not hiding the problem (due to the WARN_ON) and the patch
> improves the current situation:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> 
> >
> > Cc: Paulo Zanoni <przanoni@gmail.com>
> > Cc: Damien Lespiau <damien.lespiau@intel.com>
> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> > index 5eeddc9..0d3e014 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3316,8 +3316,10 @@ skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
> >         if (!to_intel_crtc(crtc)->active)
> >                 return 0;
> >
> > -       return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
> > +       if (WARN_ON(p->pixel_rate == 0))
> > +               return 0;
> >
> > +       return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
> >  }
> >
> >  static void skl_compute_transition_wm(struct drm_crtc *crtc,
> > --
> > 2.1.4
> >
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> 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

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

* Re: [PATCH] drm/i915: Fix divide by zero on watermark update
  2015-07-16 16:44     ` Damien Lespiau
  2015-07-16 17:26       ` Damien Lespiau
@ 2015-08-12 14:26       ` Jani Nikula
  1 sibling, 0 replies; 8+ messages in thread
From: Jani Nikula @ 2015-08-12 14:26 UTC (permalink / raw)
  To: Damien Lespiau, Mika Kuoppala; +Cc: intel-gfx

On Thu, 16 Jul 2015, Damien Lespiau <damien.lespiau@intel.com> wrote:
> On Thu, Jul 16, 2015 at 07:36:51PM +0300, Mika Kuoppala wrote:
>> Fix divide by zero if we end up updating the watermarks
>> with zero dotclock.
>> 
>> This is a stop gap measure to allow module load in cases
>> where our state keeping fails.
>> 
>> v2: WARN_ON added (Paulo)
>> 
>> Cc: Paulo Zanoni <przanoni@gmail.com>
>> Cc: Damien Lespiau <damien.lespiau@intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>
> I want to say a loading module is more important than a proper fix, so:

Any ideas on the proper fix? Patches on the list, sketches on a post-it,
anything? I've got a machine here hitting this.

BR,
Jani.


>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
>
> -- 
> Damien
>
>> ---
>>  drivers/gpu/drm/i915/intel_pm.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 5eeddc9..0d3e014 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -3316,8 +3316,10 @@ skl_compute_linetime_wm(struct drm_crtc *crtc, struct skl_pipe_wm_parameters *p)
>>  	if (!to_intel_crtc(crtc)->active)
>>  		return 0;
>>  
>> -	return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
>> +	if (WARN_ON(p->pixel_rate == 0))
>> +		return 0;
>>  
>> +	return DIV_ROUND_UP(8 * p->pipe_htotal * 1000, p->pixel_rate);
>>  }
>>  
>>  static void skl_compute_transition_wm(struct drm_crtc *crtc,
>> -- 
>> 2.1.4
>> 
> _______________________________________________
> 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] 8+ messages in thread

end of thread, other threads:[~2015-08-12 14:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-16 16:00 [PATCH] drm/i915: Fix divide by zero on watermark update Mika Kuoppala
2015-07-16 16:07 ` Paulo Zanoni
2015-07-16 16:36   ` Mika Kuoppala
2015-07-16 16:43     ` Paulo Zanoni
2015-07-17  7:10       ` Daniel Vetter
2015-07-16 16:44     ` Damien Lespiau
2015-07-16 17:26       ` Damien Lespiau
2015-08-12 14:26       ` Jani Nikula

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