diff mbox

drm/i915: Fix divide by zero on watermark update

Message ID 1437064611-6261-1-git-send-email-mika.kuoppala@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Mika Kuoppala July 16, 2015, 4:36 p.m. UTC
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(-)

Comments

Paulo Zanoni July 16, 2015, 4:43 p.m. UTC | #1
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
>
Lespiau, Damien July 16, 2015, 4:44 p.m. UTC | #2
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>
Lespiau, Damien July 16, 2015, 5:26 p.m. UTC | #3
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'.
Daniel Vetter July 17, 2015, 7:10 a.m. UTC | #4
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
Jani Nikula Aug. 12, 2015, 2:26 p.m. UTC | #5
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
diff mbox

Patch

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,