[PATCH] drm/i915: Fix crtc_y assignment in intel_find_initial_plane_obj()

[PATCH] drm/i915: Fix crtc_y assignment in intel_find_initial_plane_obj()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Let's set crtc_y to 0 instead of setting src_y twice.

Multiple assignments in one statement is a good way to hide bugs.
Please don't do that.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: be5651f2d581 ("drm/i915: Update missing properties in find_initial_plane_obj")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d7c8a0f..e3db46d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2640,11 +2640,13 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 	return;
 
 valid_fb:
-	plane_state->src_x = plane_state->src_y = 0;
+	plane_state->src_x = 0;
+	plane_state->src_y = 0;
 	plane_state->src_w = fb->width << 16;
 	plane_state->src_h = fb->height << 16;
 
-	plane_state->crtc_x = plane_state->src_y = 0;
+	plane_state->crtc_x = 0;
+	plane_state->crtc_y = 0;
 	plane_state->crtc_w = fb->width;
 	plane_state->crtc_h = fb->height;
 
-- 
2.4.10

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

Re: [PATCH] drm/i915: Fix crtc_y assignment in intel_find_initial_plane_obj()

[Jani Nikula]

On Fri, 13 Nov 2015, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's set crtc_y to 0 instead of setting src_y twice.
>
> Multiple assignments in one statement is a good way to hide bugs.
> Please don't do that.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: be5651f2d581 ("drm/i915: Update missing properties in find_initial_plane_obj")

Cc: stable@vger.kernel.org # v4.3+

right?


> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d7c8a0f..e3db46d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2640,11 +2640,13 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
>  	return;
>  
>  valid_fb:
> -	plane_state->src_x = plane_state->src_y = 0;
> +	plane_state->src_x = 0;
> +	plane_state->src_y = 0;
>  	plane_state->src_w = fb->width << 16;
>  	plane_state->src_h = fb->height << 16;
>  
> -	plane_state->crtc_x = plane_state->src_y = 0;
> +	plane_state->crtc_x = 0;
> +	plane_state->crtc_y = 0;
>  	plane_state->crtc_w = fb->width;
>  	plane_state->crtc_h = fb->height;

-- 
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: [PATCH] drm/i915: Fix crtc_y assignment in intel_find_initial_plane_obj()

[Ville Syrjälä]

On Fri, Nov 13, 2015 at 07:36:08PM +0200, Jani Nikula wrote:
> On Fri, 13 Nov 2015, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Let's set crtc_y to 0 instead of setting src_y twice.
> >
> > Multiple assignments in one statement is a good way to hide bugs.
> > Please don't do that.
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Fixes: be5651f2d581 ("drm/i915: Update missing properties in find_initial_plane_obj")
> 
> Cc: stable@vger.kernel.org # v4.3+
> 
> right?

I think the state is kzalloc()ed, so there's probably no real bug here.
Should have mentioned that in the commit message I suppose.

> 
> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index d7c8a0f..e3db46d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2640,11 +2640,13 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> >  	return;
> >  
> >  valid_fb:
> > -	plane_state->src_x = plane_state->src_y = 0;
> > +	plane_state->src_x = 0;
> > +	plane_state->src_y = 0;
> >  	plane_state->src_w = fb->width << 16;
> >  	plane_state->src_h = fb->height << 16;
> >  
> > -	plane_state->crtc_x = plane_state->src_y = 0;
> > +	plane_state->crtc_x = 0;
> > +	plane_state->crtc_y = 0;
> >  	plane_state->crtc_w = fb->width;
> >  	plane_state->crtc_h = fb->height;
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Fix crtc_y assignment in intel_find_initial_plane_obj()

[Maarten Lankhorst]

Op 13-11-15 om 18:16 schreef ville.syrjala@linux.intel.com:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Let's set crtc_y to 0 instead of setting src_y twice.
>
> Multiple assignments in one statement is a good way to hide bugs.
> Please don't do that.
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: be5651f2d581 ("drm/i915: Update missing properties in find_initial_plane_obj")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
Oops.

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

Re: [PATCH] drm/i915: Fix crtc_y assignment in intel_find_initial_plane_obj()

[Jani Nikula]

On Mon, 16 Nov 2015, Maarten Lankhorst <maarten.lankhorst@linux.intel.com> wrote:
> Op 13-11-15 om 18:16 schreef ville.syrjala@linux.intel.com:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Let's set crtc_y to 0 instead of setting src_y twice.
>>
>> Multiple assignments in one statement is a good way to hide bugs.
>> Please don't do that.
>>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Fixes: be5651f2d581 ("drm/i915: Update missing properties in find_initial_plane_obj")
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
> Oops.
>
> R-b

Pushed to drm-intel-fixes, thanks for the patch and review.

Maarten, please give Damien and our tooling a chance and spell out the
R-b...

BR,
Jani.



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