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

[Jani Nikula]

On Wed, 09 Nov 2016, Matt Roper <matthew.d.roper at intel.com> wrote:
> 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?

Only if it was a manual fixup that went wrong. For silent conflicts, we
need to add a fixup patch for the merge.

BR,
Jani.


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

-- 
Jani Nikula, Intel Open Source Technology Center