[PATCH 0/1] On DRM -> stable process

[PATCH 0/1] On DRM -> stable process

[Fedor Pchelkin]

Hi all,

I'm writing as a bystander working with 6.1.y stable branch and possibly
lacking some context with the established DRM -> stable patch flow, Cc'ing
a large number of people.

The commit being reverted from 6.1.y is the one that duplicates the
changes already backported to that branch with another commit. It is
essentially a "similar" commit but cherry-picked at some point during the
DRM development process.

The duplicate has no runtime effect but should not actually remain in the
stable trees. It was already reverted [1] from 6.6/6.10/6.11 but still made
its way later to 6.1.

[1]: https://lore.kernel.org/stable/20241007035711.46624-1-jsg@jsg.id.au/T/#u

At [1] Greg KH also stated that the observed problems are quite common
while backporting DRM patches to stable trees. The current duplicate patch
has in every sense a cosmetic impact but in other circumstances and for
other patches this may have gone wrong.

So, is there any way to adjust this process?

BTW, a question to the stable-team: what Git magic (3-way-merge?) let the
duplicate patch be applied successfully? The patch context in stable trees
was different to that moment so should the duplicate have been expected to
fail to be applied?

--
Fedor

[PATCH 6.1 1/1] Revert "drm/amd/display: Skip Recompute DSC Params if no Stream on Link"

[Fedor Pchelkin]

This reverts commit 7c887efda1201110211fed8921a92a713e0b6bcd.

It is a duplicate of the change made in 6.1.105 by commit 282f0a482ee6
("drm/amd/display: Skip Recompute DSC Params if no Stream on Link").

This is a consequence of two "similar" upstream commits existence, one of
which has been cherry-picked.

Reported-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 1acef5f3838f..855cd71f636f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -1255,9 +1255,6 @@ static bool is_dsc_need_re_compute(
 		}
 	}
 
-	if (new_stream_on_link_num == 0)
-		return false;
-
 	if (new_stream_on_link_num == 0)
 		return false;
 
-- 
2.39.5

Re: [PATCH 0/1] On DRM -> stable process

[Sasha Levin]

On Tue, Oct 29, 2024 at 04:31:40PM +0300, Fedor Pchelkin wrote:
>BTW, a question to the stable-team: what Git magic (3-way-merge?) let the
>duplicate patch be applied successfully? The patch context in stable trees
>was different to that moment so should the duplicate have been expected to
>fail to be applied?

Just plain git... Try it yourself :)

$ git checkout 282f0a482ee6
HEAD is now at 282f0a482ee61 drm/amd/display: Skip Recompute DSC Params if no Stream on Link

$ git cherry-pick 7c887efda1
Auto-merging drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
[detached HEAD 2a1c937960abd] drm/amd/display: Skip Recompute DSC Params if no Stream on Link
  Author: Fangzhi Zuo <Jerry.Zuo@amd.com>
  Date: Fri Jul 12 16:30:03 2024 -0400
  1 file changed, 3 insertions(+)

$ git log -2 --oneline
2a1c937960abd (HEAD) drm/amd/display: Skip Recompute DSC Params if no Stream on Link
282f0a482ee61 drm/amd/display: Skip Recompute DSC Params if no Stream on Link

-- 
Thanks,
Sasha

Re: [PATCH 0/1] On DRM -> stable process

[Fedor Pchelkin]

On Tue, 29. Oct 10:20, Sasha Levin wrote:
> On Tue, Oct 29, 2024 at 04:31:40PM +0300, Fedor Pchelkin wrote:
> > BTW, a question to the stable-team: what Git magic (3-way-merge?) let the
> > duplicate patch be applied successfully? The patch context in stable trees
> > was different to that moment so should the duplicate have been expected to
> > fail to be applied?
> 
> Just plain git... Try it yourself :)
> 
> $ git checkout 282f0a482ee6
> HEAD is now at 282f0a482ee61 drm/amd/display: Skip Recompute DSC Params if no Stream on Link
> 
> $ git cherry-pick 7c887efda1

7c887efda1 is the commit backported to linux-6.1.y. Of course it will apply
there.

What I mean is that the upstream commit for 7c887efda1 is 8151a6c13111b465dbabe07c19f572f7cbd16fef.

And cherry-picking 8151a6c13111b465dbabe07c19f572f7cbd16fef to linux-6.1.y
on top of 282f0a482ee6 will not result in duplicating the change, at least
with my git configuration.

I just don't understand how a duplicating if-statement could be produced in
result of those cherry-pick'ings and how the content of 7c887efda1 was
generated.

$ git checkout 282f0a482ee6
HEAD is now at 282f0a482ee6 drm/amd/display: Skip Recompute DSC Params if no Stream on Link

$ git cherry-pick 8151a6c13111b465dbabe07c19f572f7cbd16fef
Auto-merging drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
HEAD detached at 282f0a482ee6
You are currently cherry-picking commit 8151a6c13111.
  (all conflicts fixed: run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)
The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:

    git commit --allow-empty

Otherwise, please use 'git cherry-pick --skip'

> Auto-merging drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> [detached HEAD 2a1c937960abd] drm/amd/display: Skip Recompute DSC Params if no Stream on Link
>  Author: Fangzhi Zuo <Jerry.Zuo@amd.com>
>  Date: Fri Jul 12 16:30:03 2024 -0400
>  1 file changed, 3 insertions(+)
> 
> $ git log -2 --oneline
> 2a1c937960abd (HEAD) drm/amd/display: Skip Recompute DSC Params if no Stream on Link
> 282f0a482ee61 drm/amd/display: Skip Recompute DSC Params if no Stream on Link
> 
> -- 
> Thanks,
> Sasha

Re: [PATCH 0/1] On DRM -> stable process

[Fedor Pchelkin]

On Tue, 29. Oct 18:12, Fedor Pchelkin wrote:
> On Tue, 29. Oct 10:20, Sasha Levin wrote:
> > On Tue, Oct 29, 2024 at 04:31:40PM +0300, Fedor Pchelkin wrote:
> > > BTW, a question to the stable-team: what Git magic (3-way-merge?) let the
> > > duplicate patch be applied successfully? The patch context in stable trees
> > > was different to that moment so should the duplicate have been expected to
> > > fail to be applied?
> > 
> > Just plain git... Try it yourself :)
> > 
> > $ git checkout 282f0a482ee6
> > HEAD is now at 282f0a482ee61 drm/amd/display: Skip Recompute DSC Params if no Stream on Link
> > 
> > $ git cherry-pick 7c887efda1
> 
> 7c887efda1 is the commit backported to linux-6.1.y. Of course it will apply
> there.
> 
> What I mean is that the upstream commit for 7c887efda1 is 8151a6c13111b465dbabe07c19f572f7cbd16fef.
> 
> And cherry-picking 8151a6c13111b465dbabe07c19f572f7cbd16fef to linux-6.1.y
> on top of 282f0a482ee6 will not result in duplicating the change, at least
> with my git configuration.
> 
> I just don't understand how a duplicating if-statement could be produced in
> result of those cherry-pick'ings and how the content of 7c887efda1 was
> generated.
> 
> $ git checkout 282f0a482ee6
> HEAD is now at 282f0a482ee6 drm/amd/display: Skip Recompute DSC Params if no Stream on Link
> 
> $ git cherry-pick 8151a6c13111b465dbabe07c19f572f7cbd16fef
> Auto-merging drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> HEAD detached at 282f0a482ee6
> You are currently cherry-picking commit 8151a6c13111.
>   (all conflicts fixed: run "git cherry-pick --continue")
>   (use "git cherry-pick --skip" to skip this patch)
>   (use "git cherry-pick --abort" to cancel the cherry-pick operation)
> The previous cherry-pick is now empty, possibly due to conflict resolution.
> If you wish to commit it anyway, use:
> 
>     git commit --allow-empty
> 
> Otherwise, please use 'git cherry-pick --skip'

Sasha,

my concern is that maybe there is some issue with the scripts used for the
preparation of backport patches.

There are two different upstream commits performing the exact same change:
- 50e376f1fe3bf571d0645ddf48ad37eb58323919
- 8151a6c13111b465dbabe07c19f572f7cbd16fef

50e376f1fe3bf571d0645ddf48ad37eb58323919 was backported to stable kernels
at first. After that, attempts to backport 8151a6c13111b465dbabe07c19f572f7cbd16fef
to those stables should be expected to fail, no? Git would have complained
about this - the patch was already applied.

It is just strange that the (exact same) change made by the commits is
duplicated by backporting tools. As it is not the first case where DRM
patches are involved per Greg's statement [1], I wonder if something can be
done on stable-team's side to avoid such odd behavior in future.

[1]: https://lore.kernel.org/stable/20241007035711.46624-1-jsg@jsg.id.au/T/#u

--
Thanks,
Fedor

Re: [PATCH 0/1] On DRM -> stable process

[Alex Deucher]

On Mon, Nov 4, 2024 at 9:55 AM Fedor Pchelkin <pchelkin@ispras.ru> wrote:
>
> On Tue, 29. Oct 18:12, Fedor Pchelkin wrote:
> > On Tue, 29. Oct 10:20, Sasha Levin wrote:
> > > On Tue, Oct 29, 2024 at 04:31:40PM +0300, Fedor Pchelkin wrote:
> > > > BTW, a question to the stable-team: what Git magic (3-way-merge?) let the
> > > > duplicate patch be applied successfully? The patch context in stable trees
> > > > was different to that moment so should the duplicate have been expected to
> > > > fail to be applied?
> > >
> > > Just plain git... Try it yourself :)
> > >
> > > $ git checkout 282f0a482ee6
> > > HEAD is now at 282f0a482ee61 drm/amd/display: Skip Recompute DSC Params if no Stream on Link
> > >
> > > $ git cherry-pick 7c887efda1
> >
> > 7c887efda1 is the commit backported to linux-6.1.y. Of course it will apply
> > there.
> >
> > What I mean is that the upstream commit for 7c887efda1 is 8151a6c13111b465dbabe07c19f572f7cbd16fef.
> >
> > And cherry-picking 8151a6c13111b465dbabe07c19f572f7cbd16fef to linux-6.1.y
> > on top of 282f0a482ee6 will not result in duplicating the change, at least
> > with my git configuration.
> >
> > I just don't understand how a duplicating if-statement could be produced in
> > result of those cherry-pick'ings and how the content of 7c887efda1 was
> > generated.
> >
> > $ git checkout 282f0a482ee6
> > HEAD is now at 282f0a482ee6 drm/amd/display: Skip Recompute DSC Params if no Stream on Link
> >
> > $ git cherry-pick 8151a6c13111b465dbabe07c19f572f7cbd16fef
> > Auto-merging drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> > HEAD detached at 282f0a482ee6
> > You are currently cherry-picking commit 8151a6c13111.
> >   (all conflicts fixed: run "git cherry-pick --continue")
> >   (use "git cherry-pick --skip" to skip this patch)
> >   (use "git cherry-pick --abort" to cancel the cherry-pick operation)
> > The previous cherry-pick is now empty, possibly due to conflict resolution.
> > If you wish to commit it anyway, use:
> >
> >     git commit --allow-empty
> >
> > Otherwise, please use 'git cherry-pick --skip'
>
> Sasha,
>
> my concern is that maybe there is some issue with the scripts used for the
> preparation of backport patches.
>
> There are two different upstream commits performing the exact same change:
> - 50e376f1fe3bf571d0645ddf48ad37eb58323919
> - 8151a6c13111b465dbabe07c19f572f7cbd16fef

There were cases where I needed to cherry-pick fixes from -next to
-fixes.  In the past I had not used -x when cherry-picking because I
got warnings about references to commits that were not yet in
mainline.  I have since started using -x when cherry-picking things
from -next to -fixes.

Alex

>
> 50e376f1fe3bf571d0645ddf48ad37eb58323919 was backported to stable kernels
> at first. After that, attempts to backport 8151a6c13111b465dbabe07c19f572f7cbd16fef
> to those stables should be expected to fail, no? Git would have complained
> about this - the patch was already applied.
>
> It is just strange that the (exact same) change made by the commits is
> duplicated by backporting tools. As it is not the first case where DRM
> patches are involved per Greg's statement [1], I wonder if something can be
> done on stable-team's side to avoid such odd behavior in future.
>
> [1]: https://lore.kernel.org/stable/20241007035711.46624-1-jsg@jsg.id.au/T/#u
>
> --
> Thanks,
> Fedor

Re: [PATCH 0/1] On DRM -> stable process

[Greg Kroah-Hartman]

On Mon, Nov 04, 2024 at 05:55:28PM +0300, Fedor Pchelkin wrote:
> On Tue, 29. Oct 18:12, Fedor Pchelkin wrote:
> > On Tue, 29. Oct 10:20, Sasha Levin wrote:
> > > On Tue, Oct 29, 2024 at 04:31:40PM +0300, Fedor Pchelkin wrote:
> > > > BTW, a question to the stable-team: what Git magic (3-way-merge?) let the
> > > > duplicate patch be applied successfully? The patch context in stable trees
> > > > was different to that moment so should the duplicate have been expected to
> > > > fail to be applied?
> > > 
> > > Just plain git... Try it yourself :)
> > > 
> > > $ git checkout 282f0a482ee6
> > > HEAD is now at 282f0a482ee61 drm/amd/display: Skip Recompute DSC Params if no Stream on Link
> > > 
> > > $ git cherry-pick 7c887efda1
> > 
> > 7c887efda1 is the commit backported to linux-6.1.y. Of course it will apply
> > there.
> > 
> > What I mean is that the upstream commit for 7c887efda1 is 8151a6c13111b465dbabe07c19f572f7cbd16fef.
> > 
> > And cherry-picking 8151a6c13111b465dbabe07c19f572f7cbd16fef to linux-6.1.y
> > on top of 282f0a482ee6 will not result in duplicating the change, at least
> > with my git configuration.
> > 
> > I just don't understand how a duplicating if-statement could be produced in
> > result of those cherry-pick'ings and how the content of 7c887efda1 was
> > generated.
> > 
> > $ git checkout 282f0a482ee6
> > HEAD is now at 282f0a482ee6 drm/amd/display: Skip Recompute DSC Params if no Stream on Link
> > 
> > $ git cherry-pick 8151a6c13111b465dbabe07c19f572f7cbd16fef
> > Auto-merging drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
> > HEAD detached at 282f0a482ee6
> > You are currently cherry-picking commit 8151a6c13111.
> >   (all conflicts fixed: run "git cherry-pick --continue")
> >   (use "git cherry-pick --skip" to skip this patch)
> >   (use "git cherry-pick --abort" to cancel the cherry-pick operation)
> > The previous cherry-pick is now empty, possibly due to conflict resolution.
> > If you wish to commit it anyway, use:
> > 
> >     git commit --allow-empty
> > 
> > Otherwise, please use 'git cherry-pick --skip'
> 
> Sasha,
> 
> my concern is that maybe there is some issue with the scripts used for the
> preparation of backport patches.
> 
> There are two different upstream commits performing the exact same change:
> - 50e376f1fe3bf571d0645ddf48ad37eb58323919
> - 8151a6c13111b465dbabe07c19f572f7cbd16fef
> 
> 50e376f1fe3bf571d0645ddf48ad37eb58323919 was backported to stable kernels
> at first. After that, attempts to backport 8151a6c13111b465dbabe07c19f572f7cbd16fef
> to those stables should be expected to fail, no? Git would have complained
> about this - the patch was already applied.
> 
> It is just strange that the (exact same) change made by the commits is
> duplicated by backporting tools. As it is not the first case where DRM
> patches are involved per Greg's statement [1], I wonder if something can be
> done on stable-team's side to avoid such odd behavior in future.

No, all of this mess needs to be fixed up on the drm developer's side,
they are the ones doing this type of crazy "let's commit the same patch
to multiple branches and then reference a commit that will show up at an
unknown time in the future and hope for the best!" workflow.

I'm amazed it works at all, they get to keep fixing up this mess as this
is entirely self-inflicted.

greg k-h

Re: [PATCH 0/1] On DRM -> stable process

[Fedor Pchelkin]

On Tue, 05. Nov 07:57, Greg Kroah-Hartman wrote:
> On Mon, Nov 04, 2024 at 05:55:28PM +0300, Fedor Pchelkin wrote:
> > It is just strange that the (exact same) change made by the commits is
> > duplicated by backporting tools. As it is not the first case where DRM
> > patches are involved per Greg's statement [1], I wonder if something can be
> > done on stable-team's side to avoid such odd behavior in future.
> 
> No, all of this mess needs to be fixed up on the drm developer's side,
> they are the ones doing this type of crazy "let's commit the same patch
> to multiple branches and then reference a commit that will show up at an
> unknown time in the future and hope for the best!" workflow.
> 
> I'm amazed it works at all, they get to keep fixing up this mess as this
> is entirely self-inflicted.

Thanks for reply, I get your remark. DRM people are mostly CC'ed here,
hopefully it won't be that difficult to tune their established workflow to
make the stable process easier and more straightforward.

As of now, would you mind to take the revert for 6.1? It's [PATCH 1/1] in
this thread. No point to keep it there, and the duplicated commits were
already reverted from the fresher stable kernels.

Re: [PATCH 0/1] On DRM -> stable process

[Greg Kroah-Hartman]

On Fri, Nov 08, 2024 at 11:41:18AM +0300, Fedor Pchelkin wrote:
> On Tue, 05. Nov 07:57, Greg Kroah-Hartman wrote:
> > On Mon, Nov 04, 2024 at 05:55:28PM +0300, Fedor Pchelkin wrote:
> > > It is just strange that the (exact same) change made by the commits is
> > > duplicated by backporting tools. As it is not the first case where DRM
> > > patches are involved per Greg's statement [1], I wonder if something can be
> > > done on stable-team's side to avoid such odd behavior in future.
> > 
> > No, all of this mess needs to be fixed up on the drm developer's side,
> > they are the ones doing this type of crazy "let's commit the same patch
> > to multiple branches and then reference a commit that will show up at an
> > unknown time in the future and hope for the best!" workflow.
> > 
> > I'm amazed it works at all, they get to keep fixing up this mess as this
> > is entirely self-inflicted.
> 
> Thanks for reply, I get your remark. DRM people are mostly CC'ed here,
> hopefully it won't be that difficult to tune their established workflow to
> make the stable process easier and more straightforward.
> 
> As of now, would you mind to take the revert for 6.1? It's [PATCH 1/1] in
> this thread. No point to keep it there, and the duplicated commits were
> already reverted from the fresher stable kernels.
> 

I don't see it in my review queue anymore, can you please resend it?

thanks,

greg k-h