[Intel-gfx] [PATCH] drm/i915: remove duplicated memsets in skl_allocate_pipe_ddb()

[Matt Roper]

On Wed, Nov 09, 2016 at 11:27:37AM +0200, Jani Nikula wrote:
> On Wed, 09 Nov 2016, Matt Roper <matthew.d.roper at intel.com> wrote:
> > On Tue, Nov 08, 2016 at 03:21:22PM -0200, Paulo Zanoni wrote:
> >> One of the memsets was added by 5a920b85f2c6 ("drm/i915/gen9: fix DDB
> >> partitioning for multi-screen cases"), and the other was added by
> >> 01c72d6c17 ("drm/i915/gen9: fix DDB partitioning for multi-screen
> >> cases"). I'm confused and I'll let the maintainers find out what went
> >> wrong here.
> >> 
> >> Cc: Jani Nikula <jani.nikula at intel.com>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >
> > I think this is just an artifact of having the patch in both -next and
> > -fixes.  The context probably changed later in -next causing a conflict
> > when the two were merged, and then the merge resolution accidentally
> > duplicated this hunk.  The merge commit ac4139ed7 is where we wound up
> > with the two copies:
> 
> I don't have that merge commit. Is that a nightly rebuild?

Yeah, it's part of a nightly rebuild.  Looks like
82282577545 is the equivalent now.

> I don't think we even have this issue in anywhere other than
> nightly. There isn't a branch we could apply this fix to.

Yeah, the patch can't apply anywhere directly, but I thought we could
make rerere forget its resolution of intel_pm.c for that merge and then
fix it by hand so that future merges behave correctly?


Matt


> 
> BR,
> Jani.
> 
> 
> >
> >         diff --cc drivers/gpu/drm/i915/intel_pm.c
> >         index db24f89,cc9e0c0..88e28c9
> >         --- a/drivers/gpu/drm/i915/intel_pm.c
> >         +++ b/drivers/gpu/drm/i915/intel_pm.c
> >         @@@ -3361,11 -3404,13 +3404,17 @@@ skl_allocate_pipe_ddb(struct intel_crtc
> >                 unsigned int total_data_rate;
> >                 int num_active;
> >                 int id, i;
> >         +       unsigned plane_data_rate[I915_MAX_PLANES] = {};
> >         +       unsigned plane_y_data_rate[I915_MAX_PLANES] = {};
> >         + 
> >         +       /* Clear the partitioning for disabled planes. */
> >         +       memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> >         +       memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
> >           
> >          +      /* Clear the partitioning for disabled planes. */
> >          +      memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> >          +      memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
> >          +
> >                 if (WARN_ON(!state))
> >                         return 0;
> >           
> > Dropping one looks good to me.
> >
> > Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
> >
> >> ---
> >>  drivers/gpu/drm/i915/intel_pm.c | 4 ----
> >>  1 file changed, 4 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> >> index 88e28c9..cc9e0c0 100644
> >> --- a/drivers/gpu/drm/i915/intel_pm.c
> >> +++ b/drivers/gpu/drm/i915/intel_pm.c
> >> @@ -3411,10 +3411,6 @@ skl_allocate_pipe_ddb(struct intel_crtc_state *cstate,
> >>  	memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> >>  	memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
> >>  
> >> -	/* Clear the partitioning for disabled planes. */
> >> -	memset(ddb->plane[pipe], 0, sizeof(ddb->plane[pipe]));
> >> -	memset(ddb->y_plane[pipe], 0, sizeof(ddb->y_plane[pipe]));
> >> -
> >>  	if (WARN_ON(!state))
> >>  		return 0;
> >>  
> >> -- 
> >> 2.7.4
> >> 
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx at lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795