Re: [Intel-gfx] [PATCH] drm/i915: Fix divide by zero on watermark update

2015-08-12 Thread Jani Nikula
On Thu, 16 Jul 2015, 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 
>> Cc: Damien Lespiau 
>> Signed-off-by: Mika Kuoppala 
>
> 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
>
>> ---
>>  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


Re: [Intel-gfx] [PATCH] drm/i915: Fix divide by zero on watermark update

2015-07-17 Thread Daniel Vetter
On Thu, Jul 16, 2015 at 01:43:15PM -0300, Paulo Zanoni wrote:
> 2015-07-16 13:36 GMT-03:00 Mika Kuoppala :
> > 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 

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

> 
> >
> > Cc: Paulo Zanoni 
> > Cc: Damien Lespiau 
> > Signed-off-by: Mika Kuoppala 
> > ---
> >  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


Re: [Intel-gfx] [PATCH] drm/i915: Fix divide by zero on watermark update

2015-07-16 Thread Damien Lespiau
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 
> > Cc: Damien Lespiau 
> > Signed-off-by: Mika Kuoppala 
> 
> I want to say a loading module is more important than a proper fix, so:
> 
> Reviewed-by: Damien Lespiau 

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


Re: [Intel-gfx] [PATCH] drm/i915: Fix divide by zero on watermark update

2015-07-16 Thread Damien Lespiau
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 
> Cc: Damien Lespiau 
> Signed-off-by: Mika Kuoppala 

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

Reviewed-by: Damien Lespiau 

-- 
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


Re: [Intel-gfx] [PATCH] drm/i915: Fix divide by zero on watermark update

2015-07-16 Thread Paulo Zanoni
2015-07-16 13:36 GMT-03:00 Mika Kuoppala :
> 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 

>
> Cc: Paulo Zanoni 
> Cc: Damien Lespiau 
> Signed-off-by: Mika Kuoppala 
> ---
>  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


Re: [Intel-gfx] [PATCH] drm/i915: Fix divide by zero on watermark update

2015-07-16 Thread Paulo Zanoni
2015-07-16 13:00 GMT-03:00 Mika Kuoppala :
> 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 
> Signed-off-by: Mika Kuoppala 
> ---
>  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