| Message ID | 20221122-gud-shadow-plane-v2-0-435037990a83@tronnes.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/gud: Use the shadow plane helper | expand |
On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 Submission Endpoint wrote: > Hi, > > I have started to look at igt for testing and want to use CRC tests. To > implement support for this I need to move away from the simple kms > helper. > > When looking around for examples I came across Thomas' nice shadow > helper and thought, yes this is perfect for drm/gud. So I'll switch to > that before I move away from the simple kms helper. > > The async framebuffer flushing code path now uses a shadow buffer and > doesn't touch the framebuffer when it shouldn't. I have also taken the > opportunity to inline the synchronous flush code path and make this the > default flushing stategy. > > Noralf. > > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > --- > Changes in v2: > - Drop patch (Thomas): > drm/gem: shadow_fb_access: Prepare imported buffers for CPU access > - Use src as variable name for iosys_map (Thomas) > - Prepare imported buffer for CPU access in the driver (Thomas) > - New patch: make sync flushing the default (Thomas) > - Link to v1: https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa3383e@tronnes.org <formletter> This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. </formletter>
Den 01.12.2022 06.55, skrev Greg KH: > On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 Submission Endpoint wrote: >> Hi, >> >> I have started to look at igt for testing and want to use CRC tests. To >> implement support for this I need to move away from the simple kms >> helper. >> >> When looking around for examples I came across Thomas' nice shadow >> helper and thought, yes this is perfect for drm/gud. So I'll switch to >> that before I move away from the simple kms helper. >> >> The async framebuffer flushing code path now uses a shadow buffer and >> doesn't touch the framebuffer when it shouldn't. I have also taken the >> opportunity to inline the synchronous flush code path and make this the >> default flushing stategy. >> >> Noralf. >> >> Cc: Maxime Ripard <mripard@kernel.org> >> Cc: Thomas Zimmermann <tzimmermann@suse.de> >> Cc: dri-devel@lists.freedesktop.org >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> >> --- >> Changes in v2: >> - Drop patch (Thomas): >> drm/gem: shadow_fb_access: Prepare imported buffers for CPU access >> - Use src as variable name for iosys_map (Thomas) >> - Prepare imported buffer for CPU access in the driver (Thomas) >> - New patch: make sync flushing the default (Thomas) >> - Link to v1: https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa3383e@tronnes.org > > <formletter> > > This is not the correct way to submit patches for inclusion in the > stable kernel tree. Please read: > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > for how to do this properly. > > </formletter> Care to elaborate? Is it because stable got the whole patchset and not just the one fix patch that cc'ed stable? This patchset was sent using the b4 tool and I can't control this aspect. Everyone mentioned in the patches gets the whole set. Noralf.
On Thu, Dec 01, 2022 at 11:00:44AM +0100, Noralf Trønnes wrote: > > > Den 01.12.2022 06.55, skrev Greg KH: > > On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 Submission Endpoint wrote: > >> Hi, > >> > >> I have started to look at igt for testing and want to use CRC tests. To > >> implement support for this I need to move away from the simple kms > >> helper. > >> > >> When looking around for examples I came across Thomas' nice shadow > >> helper and thought, yes this is perfect for drm/gud. So I'll switch to > >> that before I move away from the simple kms helper. > >> > >> The async framebuffer flushing code path now uses a shadow buffer and > >> doesn't touch the framebuffer when it shouldn't. I have also taken the > >> opportunity to inline the synchronous flush code path and make this the > >> default flushing stategy. > >> > >> Noralf. > >> > >> Cc: Maxime Ripard <mripard@kernel.org> > >> Cc: Thomas Zimmermann <tzimmermann@suse.de> > >> Cc: dri-devel@lists.freedesktop.org > >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >> > >> --- > >> Changes in v2: > >> - Drop patch (Thomas): > >> drm/gem: shadow_fb_access: Prepare imported buffers for CPU access > >> - Use src as variable name for iosys_map (Thomas) > >> - Prepare imported buffer for CPU access in the driver (Thomas) > >> - New patch: make sync flushing the default (Thomas) > >> - Link to v1: https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa3383e@tronnes.org > > > > <formletter> > > > > This is not the correct way to submit patches for inclusion in the > > stable kernel tree. Please read: > > https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > > for how to do this properly. > > > > </formletter> > > Care to elaborate? > Is it because stable got the whole patchset and not just the one fix > patch that cc'ed stable? That is what triggered this, yes. > This patchset was sent using the b4 tool and I can't control this > aspect. Everyone mentioned in the patches gets the whole set. Fair enough, but watch out, bots will report this as being a problem as they can't always read through all patches in a series to notice this... thanks, greg k-h
Den 01.12.2022 13.12, skrev Greg KH: > On Thu, Dec 01, 2022 at 11:00:44AM +0100, Noralf Trønnes wrote: >> >> >> Den 01.12.2022 06.55, skrev Greg KH: >>> On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 Submission Endpoint wrote: >>>> Hi, >>>> >>>> I have started to look at igt for testing and want to use CRC tests. To >>>> implement support for this I need to move away from the simple kms >>>> helper. >>>> >>>> When looking around for examples I came across Thomas' nice shadow >>>> helper and thought, yes this is perfect for drm/gud. So I'll switch to >>>> that before I move away from the simple kms helper. >>>> >>>> The async framebuffer flushing code path now uses a shadow buffer and >>>> doesn't touch the framebuffer when it shouldn't. I have also taken the >>>> opportunity to inline the synchronous flush code path and make this the >>>> default flushing stategy. >>>> >>>> Noralf. >>>> >>>> Cc: Maxime Ripard <mripard@kernel.org> >>>> Cc: Thomas Zimmermann <tzimmermann@suse.de> >>>> Cc: dri-devel@lists.freedesktop.org >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >>>> >>>> --- >>>> Changes in v2: >>>> - Drop patch (Thomas): >>>> drm/gem: shadow_fb_access: Prepare imported buffers for CPU access >>>> - Use src as variable name for iosys_map (Thomas) >>>> - Prepare imported buffer for CPU access in the driver (Thomas) >>>> - New patch: make sync flushing the default (Thomas) >>>> - Link to v1: https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa3383e@tronnes.org >>> >>> <formletter> >>> >>> This is not the correct way to submit patches for inclusion in the >>> stable kernel tree. Please read: >>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html >>> for how to do this properly. >>> >>> </formletter> >> >> Care to elaborate? >> Is it because stable got the whole patchset and not just the one fix >> patch that cc'ed stable? > > That is what triggered this, yes. > >> This patchset was sent using the b4 tool and I can't control this >> aspect. Everyone mentioned in the patches gets the whole set. > > Fair enough, but watch out, bots will report this as being a problem as > they can't always read through all patches in a series to notice this... > Konstantin, Can you add a rule in b4 to exclude stable@vger.kernel.org (stable@kernel.org as well?) from getting the whole patchset? Noralf.
On Thu, Dec 01, 2022 at 02:14:42PM +0100, Noralf Trønnes wrote: > > > Den 01.12.2022 13.12, skrev Greg KH: > > On Thu, Dec 01, 2022 at 11:00:44AM +0100, Noralf Trønnes wrote: > >> > >> > >> Den 01.12.2022 06.55, skrev Greg KH: > >>> On Wed, Nov 30, 2022 at 08:26:48PM +0100, Noralf Trønnes via B4 Submission Endpoint wrote: > >>>> Hi, > >>>> > >>>> I have started to look at igt for testing and want to use CRC tests. To > >>>> implement support for this I need to move away from the simple kms > >>>> helper. > >>>> > >>>> When looking around for examples I came across Thomas' nice shadow > >>>> helper and thought, yes this is perfect for drm/gud. So I'll switch to > >>>> that before I move away from the simple kms helper. > >>>> > >>>> The async framebuffer flushing code path now uses a shadow buffer and > >>>> doesn't touch the framebuffer when it shouldn't. I have also taken the > >>>> opportunity to inline the synchronous flush code path and make this the > >>>> default flushing stategy. > >>>> > >>>> Noralf. > >>>> > >>>> Cc: Maxime Ripard <mripard@kernel.org> > >>>> Cc: Thomas Zimmermann <tzimmermann@suse.de> > >>>> Cc: dri-devel@lists.freedesktop.org > >>>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > >>>> > >>>> --- > >>>> Changes in v2: > >>>> - Drop patch (Thomas): > >>>> drm/gem: shadow_fb_access: Prepare imported buffers for CPU access > >>>> - Use src as variable name for iosys_map (Thomas) > >>>> - Prepare imported buffer for CPU access in the driver (Thomas) > >>>> - New patch: make sync flushing the default (Thomas) > >>>> - Link to v1: https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa3383e@tronnes.org > >>> > >>> <formletter> > >>> > >>> This is not the correct way to submit patches for inclusion in the > >>> stable kernel tree. Please read: > >>> https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html > >>> for how to do this properly. > >>> > >>> </formletter> > >> > >> Care to elaborate? > >> Is it because stable got the whole patchset and not just the one fix > >> patch that cc'ed stable? > > > > That is what triggered this, yes. > > > >> This patchset was sent using the b4 tool and I can't control this > >> aspect. Everyone mentioned in the patches gets the whole set. > > > > Fair enough, but watch out, bots will report this as being a problem as > > they can't always read through all patches in a series to notice this... > > > > Konstantin, > > Can you add a rule in b4 to exclude stable@vger.kernel.org > (stable@kernel.org as well?) from getting the whole patchset? stable@kernel.org is a pipe to /dev/null so that's not needed to be messed with. As for this needing special casing in b4, it's rare that you send out a patch series and only want 1 or 2 of them in stable, right? thanks, greg k-h
Hello Greg, On 12/1/22 14:21, Greg KH wrote: [...] >>>> This patchset was sent using the b4 tool and I can't control this >>>> aspect. Everyone mentioned in the patches gets the whole set. >>> >>> Fair enough, but watch out, bots will report this as being a problem as >>> they can't always read through all patches in a series to notice this... >>> >> >> Konstantin, >> >> Can you add a rule in b4 to exclude stable@vger.kernel.org >> (stable@kernel.org as well?) from getting the whole patchset? > > stable@kernel.org is a pipe to /dev/null so that's not needed to be > messed with. > > As for this needing special casing in b4, it's rare that you send out a > patch series and only want 1 or 2 of them in stable, right? > Not really, it's very common for a patch-series to contain fixes (that could go to stable if applicable) and change that are not suitable for stable. The problem as Noralf mentioned is that the b4 tool doesn't seem to allow Cc'ing individual patches to different recipients, and everyone get the whole set. So either b4 needs to have this support, exclude stable@vger.kernel.org when sending a set or stable@vger.kernel.org ignore patches without a Fixes: tag.
On Thu, Dec 01, 2022 at 02:34:41PM +0100, Javier Martinez Canillas wrote: > >> Konstantin, > >> > >> Can you add a rule in b4 to exclude stable@vger.kernel.org > >> (stable@kernel.org as well?) from getting the whole patchset? > > > > stable@kernel.org is a pipe to /dev/null so that's not needed to be > > messed with. > > > > As for this needing special casing in b4, it's rare that you send out a > > patch series and only want 1 or 2 of them in stable, right? > > > > Not really, it's very common for a patch-series to contain fixes (that could > go to stable if applicable) and change that are not suitable for stable. The > problem as Noralf mentioned is that the b4 tool doesn't seem to allow Cc'ing > individual patches to different recipients, and everyone get the whole set. > > So either b4 needs to have this support, exclude stable@vger.kernel.org when > sending a set or stable@vger.kernel.org ignore patches without a Fixes: tag. I think what I can do is a special logic for Cc: trailers: - Any Cc: trailers we find in the cover letter receive the entire series - Any Cc: trailers in individual patches only receive these individual patches Thank you for being patient -- we'll get this right, I promise. -K
On 12/1/22 15:16, Konstantin Ryabitsev wrote: > On Thu, Dec 01, 2022 at 02:34:41PM +0100, Javier Martinez Canillas wrote: >>>> Konstantin, >>>> >>>> Can you add a rule in b4 to exclude stable@vger.kernel.org >>>> (stable@kernel.org as well?) from getting the whole patchset? >>> >>> stable@kernel.org is a pipe to /dev/null so that's not needed to be >>> messed with. >>> >>> As for this needing special casing in b4, it's rare that you send out a >>> patch series and only want 1 or 2 of them in stable, right? >>> >> >> Not really, it's very common for a patch-series to contain fixes (that could >> go to stable if applicable) and change that are not suitable for stable. The >> problem as Noralf mentioned is that the b4 tool doesn't seem to allow Cc'ing >> individual patches to different recipients, and everyone get the whole set. >> >> So either b4 needs to have this support, exclude stable@vger.kernel.org when >> sending a set or stable@vger.kernel.org ignore patches without a Fixes: tag. > > I think what I can do is a special logic for Cc: trailers: > > - Any Cc: trailers we find in the cover letter receive the entire series > - Any Cc: trailers in individual patches only receive these individual patches > I think that make sense. It's similar to how for example patman works. > Thank you for being patient -- we'll get this right, I promise. > On the contrary, thanks a lot for working on this tool and for being that responsive to the feedback.
On 12/1/22 15:16, Konstantin Ryabitsev wrote: > On Thu, Dec 01, 2022 at 02:34:41PM +0100, Javier Martinez Canillas wrote: >>>> Konstantin, >>>> >>>> Can you add a rule in b4 to exclude stable@vger.kernel.org >>>> (stable@kernel.org as well?) from getting the whole patchset? >>> >>> stable@kernel.org is a pipe to /dev/null so that's not needed to be >>> messed with. >>> >>> As for this needing special casing in b4, it's rare that you send out a >>> patch series and only want 1 or 2 of them in stable, right? >>> >> >> Not really, it's very common for a patch-series to contain fixes (that could >> go to stable if applicable) and change that are not suitable for stable. The >> problem as Noralf mentioned is that the b4 tool doesn't seem to allow Cc'ing >> individual patches to different recipients, and everyone get the whole set. >> >> So either b4 needs to have this support, exclude stable@vger.kernel.org when >> sending a set or stable@vger.kernel.org ignore patches without a Fixes: tag. > > I think what I can do is a special logic for Cc: trailers: > > - Any Cc: trailers we find in the cover letter receive the entire series > - Any Cc: trailers in individual patches only receive these individual patches I usually do that with git send-email and a custom --cc-cmd script, but additionally it sends the cover letter also to everyone that's on any individual patch's Cc, so everyone gets at least the cover letter + their specific patch(es). But that extra logic could be optional. > Thank you for being patient -- we'll get this right, I promise. > > -K >
On Thu, Dec 01, 2022 at 03:27:32PM +0100, Vlastimil Babka wrote: > I usually do that with git send-email and a custom --cc-cmd script, but > additionally it sends the cover letter also to everyone that's on any > individual patch's Cc, so everyone gets at least the cover letter + their > specific patch(es). > But that extra logic could be optional. Yeah, without the cover letter if you've just got an individual patch it can be unclear what's going on with dependencies and so on for getting the patches merged.
Den 01.12.2022 15.16, skrev Konstantin Ryabitsev: > On Thu, Dec 01, 2022 at 02:34:41PM +0100, Javier Martinez Canillas wrote: >>>> Konstantin, >>>> >>>> Can you add a rule in b4 to exclude stable@vger.kernel.org >>>> (stable@kernel.org as well?) from getting the whole patchset? >>> >>> stable@kernel.org is a pipe to /dev/null so that's not needed to be >>> messed with. >>> >>> As for this needing special casing in b4, it's rare that you send out a >>> patch series and only want 1 or 2 of them in stable, right? >>> >> >> Not really, it's very common for a patch-series to contain fixes (that could >> go to stable if applicable) and change that are not suitable for stable. The >> problem as Noralf mentioned is that the b4 tool doesn't seem to allow Cc'ing >> individual patches to different recipients, and everyone get the whole set. >> >> So either b4 needs to have this support, exclude stable@vger.kernel.org when >> sending a set or stable@vger.kernel.org ignore patches without a Fixes: tag. > > I think what I can do is a special logic for Cc: trailers: > > - Any Cc: trailers we find in the cover letter receive the entire series > - Any Cc: trailers in individual patches only receive these individual patches > That should cover my use cases. I can now do 'b4 prep --auto-to-cc' and then trim down the cc list in the cover letter if necessary. > Thank you for being patient -- we'll get this right, I promise. > Thanks for getting it right. b4 can replace parts of my own tooling and do it smoother so I think I'll continue to use it. Noralf.
Den 30.11.2022 20.26, skrev Noralf Trønnes via B4 Submission Endpoint: > Hi, > > I have started to look at igt for testing and want to use CRC tests. To > implement support for this I need to move away from the simple kms > helper. > > When looking around for examples I came across Thomas' nice shadow > helper and thought, yes this is perfect for drm/gud. So I'll switch to > that before I move away from the simple kms helper. > > The async framebuffer flushing code path now uses a shadow buffer and > doesn't touch the framebuffer when it shouldn't. I have also taken the > opportunity to inline the synchronous flush code path and make this the > default flushing stategy. > > Noralf. > > Cc: Maxime Ripard <mripard@kernel.org> > Cc: Thomas Zimmermann <tzimmermann@suse.de> > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > > --- > Changes in v2: > - Drop patch (Thomas): > drm/gem: shadow_fb_access: Prepare imported buffers for CPU access > - Use src as variable name for iosys_map (Thomas) > - Prepare imported buffer for CPU access in the driver (Thomas) > - New patch: make sync flushing the default (Thomas) > - Link to v1: https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa3383e@tronnes.org > > --- > Noralf Trønnes (6): > drm/gud: Fix UBSAN warning > drm/gud: Don't retry a failed framebuffer flush > drm/gud: Split up gud_flush_work() > drm/gud: Prepare buffer for CPU access in gud_flush_work() > drm/gud: Use the shadow plane helper > drm/gud: Enable synchronous flushing by default > Applied to drm-misc-next, thanks for reviewing! Noralf.
On Thu, Dec 01, 2022 at 02:32:15PM +0000, Mark Brown wrote: > On Thu, Dec 01, 2022 at 03:27:32PM +0100, Vlastimil Babka wrote: > > > I usually do that with git send-email and a custom --cc-cmd script, but > > additionally it sends the cover letter also to everyone that's on any > > individual patch's Cc, so everyone gets at least the cover letter + their > > specific patch(es). > > > But that extra logic could be optional. > > Yeah, without the cover letter if you've just got an individual patch it > can be unclear what's going on with dependencies and so on for getting > the patches merged. +1 on including the cover letter for any recipient. If b4 can do this right by default, that would be a really good reason for me to look into it and switch. -Daniel
On Thu, Jan 05, 2023 at 01:35:37PM +0100, Daniel Vetter wrote: > > Yeah, without the cover letter if you've just got an individual patch it > > can be unclear what's going on with dependencies and so on for getting > > the patches merged. > > +1 on including the cover letter for any recipient. If b4 can do this > right by default, that would be a really good reason for me to look into > it and switch. That's the behaviour in the latest releases. I will release 0.11.2 later today with some hotfixes, so if you want to try it out, that's the version I'd recommend. -K
Hi, I have started to look at igt for testing and want to use CRC tests. To implement support for this I need to move away from the simple kms helper. When looking around for examples I came across Thomas' nice shadow helper and thought, yes this is perfect for drm/gud. So I'll switch to that before I move away from the simple kms helper. The async framebuffer flushing code path now uses a shadow buffer and doesn't touch the framebuffer when it shouldn't. I have also taken the opportunity to inline the synchronous flush code path and make this the default flushing stategy. Noralf. Cc: Maxime Ripard <mripard@kernel.org> Cc: Thomas Zimmermann <tzimmermann@suse.de> Cc: dri-devel@lists.freedesktop.org Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- Changes in v2: - Drop patch (Thomas): drm/gem: shadow_fb_access: Prepare imported buffers for CPU access - Use src as variable name for iosys_map (Thomas) - Prepare imported buffer for CPU access in the driver (Thomas) - New patch: make sync flushing the default (Thomas) - Link to v1: https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa3383e@tronnes.org --- Noralf Trønnes (6): drm/gud: Fix UBSAN warning drm/gud: Don't retry a failed framebuffer flush drm/gud: Split up gud_flush_work() drm/gud: Prepare buffer for CPU access in gud_flush_work() drm/gud: Use the shadow plane helper drm/gud: Enable synchronous flushing by default drivers/gpu/drm/gud/gud_drv.c | 1 + drivers/gpu/drm/gud/gud_internal.h | 1 + drivers/gpu/drm/gud/gud_pipe.c | 222 ++++++++++++++++++------------------- 3 files changed, 112 insertions(+), 112 deletions(-) --- base-commit: 7257702951305b1f0259c3468c39fc59d1ad4d8b change-id: 20221122-gud-shadow-plane-ae37a95d4d8d Best regards,