[Intel-gfx] [PATCH] drm/fb: Proper support of boundary conditions in bitmasks.

[Tomasz Lis]

The recently introduced patch changed behavior of masks when
the bit number is negative. Instead of no bits set, the new way
makes all bits set. Problematic patch:
  drm/i915: Avoid BIT(max) - 1 and use GENMASK(max - 1, 0)

This behaviour was not considered when making changes, and boundary
value of count (=0) is now resulting in a mask with all bits on, since
the value is directly decreased and therefore negative. Checking if
all bits are set leads to infinite loop.

This patch introduces an additional check to avoid empty masks. It
reverts the control flow to the exact same way it worked before
the problematic patch.

Signed-off-by: Tomasz Lis 
---
 drivers/gpu/drm/i915/intel_fbdev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
b/drivers/gpu/drm/i915/intel_fbdev.c
index e6f3eb2d..bc65ecf 100644
--- a/drivers/gpu/drm/i915/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/intel_fbdev.c
@@ -496,7 +496,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
*fb_helper,
conn_configured |= BIT(i);
}
 
-   if ((conn_configured & mask) != mask) {
+   if (count > 0 && (conn_configured & mask) != mask) {
pass++;
goto retry;
}
-- 
2.7.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/fb: Proper support of boundary conditions in bitmasks.

[Arkadiusz Hiler]

On Fri, Feb 17, 2017 at 11:17:45AM +0100, Tomasz Lis wrote:
> The recently introduced patch changed behavior of masks when
> the bit number is negative. Instead of no bits set, the new way
> makes all bits set. Problematic patch:
>   drm/i915: Avoid BIT(max) - 1 and use GENMASK(max - 1, 0)
> 
> This behaviour was not considered when making changes, and boundary
> value of count (=0) is now resulting in a mask with all bits on, since
> the value is directly decreased and therefore negative. Checking if
> all bits are set leads to infinite loop.
> 
> This patch introduces an additional check to avoid empty masks. It
> reverts the control flow to the exact same way it worked before
> the problematic patch.
> 
> Signed-off-by: Tomasz Lis 

Change commit tile to "drm/i915/fbdev: Proper support of boundary".

Other than that:
Reviewed-by: Arkadiusz Hiler 



-- 
Cheers,
Arek
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/fb: Proper support of boundary conditions in bitmasks.

[Jani Nikula]

On Fri, 17 Feb 2017, Tomasz Lis  wrote:
> The recently introduced patch changed behavior of masks when
> the bit number is negative. Instead of no bits set, the new way
> makes all bits set. Problematic patch:
>   drm/i915: Avoid BIT(max) - 1 and use GENMASK(max - 1, 0)

For future reference, please find the commit id of the committed patch,
and reference that with the Fixes: tag. Please Cc the folks from the
commit.

Whoever commits this must add:

Fixes: 3c779a49bd7c ("drm/i915: Avoid BIT(max) - 1 and use GENMASK(max - 1, 0)")
Cc: Joonas Lahtinen 
Cc: Chris Wilson 
Cc: Daniel Vetter 
Cc: Jani Nikula 

Joonas, Chris, please check the rest of the regressing commit that it
doesn't suffer from the same issue.

Thanks,
Jani.

> This behaviour was not considered when making changes, and boundary
> value of count (=0) is now resulting in a mask with all bits on, since
> the value is directly decreased and therefore negative. Checking if
> all bits are set leads to infinite loop.
>
> This patch introduces an additional check to avoid empty masks. It
> reverts the control flow to the exact same way it worked before
> the problematic patch.
>
> Signed-off-by: Tomasz Lis 
> ---
>  drivers/gpu/drm/i915/intel_fbdev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c 
> b/drivers/gpu/drm/i915/intel_fbdev.c
> index e6f3eb2d..bc65ecf 100644
> --- a/drivers/gpu/drm/i915/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/intel_fbdev.c
> @@ -496,7 +496,7 @@ static bool intel_fb_initial_config(struct drm_fb_helper 
> *fb_helper,
>   conn_configured |= BIT(i);
>   }
>  
> - if ((conn_configured & mask) != mask) {
> + if (count > 0 && (conn_configured & mask) != mask) {
>   pass++;
>   goto retry;
>   }

-- 
Jani Nikula, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/fb: Proper support of boundary conditions in bitmasks.

[Joonas Lahtinen]

On ma, 2017-02-20 at 10:00 +0200, Jani Nikula wrote:
> On Fri, 17 Feb 2017, Tomasz Lis  wrote:
> > 
> > The recently introduced patch changed behavior of masks when
> > the bit number is negative. Instead of no bits set, the new way
> > makes all bits set. Problematic patch:
> >   drm/i915: Avoid BIT(max) - 1 and use GENMASK(max - 1, 0)
> 
> For future reference, please find the commit id of the committed patch,
> and reference that with the Fixes: tag. Please Cc the folks from the
> commit.
> 
> Whoever commits this must add:
> 
> Fixes: 3c779a49bd7c ("drm/i915: Avoid BIT(max) - 1 and use GENMASK(max - 1, 
> 0)")
> Cc: Joonas Lahtinen 
> Cc: Chris Wilson 
> Cc: Daniel Vetter 
> Cc: Jani Nikula 
> 
> Joonas, Chris, please check the rest of the regressing commit that it
> doesn't suffer from the same issue.
> 
> Thanks,
> Jani.

This line was actually rest of the commit. So all good, thanks for
catching this.

Reviewed-by: Joonas Lahtinen 

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx