Message ID | 20220806122636.43068-1-tomas.winkler@intel.com (mailing list archive) |
---|---|
Headers | show |
Series | GSC support for XeHP SDV and DG2 | expand |
On Sat, Aug 06, 2022 at 03:26:21PM +0300, Tomas Winkler wrote: > Add GSC support for XeHP SDV and DG2 platforms. > > The series includes changes for the mei driver: > - add ability to use polling instead of interrupts > - add ability to use extended timeouts > - setup extended operational memory for GSC > > The series includes changes for the i915 driver: > - allocate extended operational memory for GSC > - GSC on XeHP SDV offsets and definitions > > This patch set should be merged via gfx tree as > the auxiliary device belongs there. > Greg, your ACK is required for the drives/misc/mei code base, > please review the patches. With the exception that you all don't know what year it is: Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Dave, do you have a preference how to deal with the mishap here, shall I do a force-push to drm-intel-gt-next to correctly record the Acked-by or revert and re-push? Or just leave it as is? Quoting Greg Kroah-Hartman (2022-09-01 18:09:09) > On Sat, Aug 06, 2022 at 03:26:21PM +0300, Tomas Winkler wrote: > > Add GSC support for XeHP SDV and DG2 platforms. > > > > The series includes changes for the mei driver: > > - add ability to use polling instead of interrupts > > - add ability to use extended timeouts > > - setup extended operational memory for GSC > > > > The series includes changes for the i915 driver: > > - allocate extended operational memory for GSC > > - GSC on XeHP SDV offsets and definitions > > > > This patch set should be merged via gfx tree as > > the auxiliary device belongs there. > > Greg, your ACK is required for the drives/misc/mei code base, > > please review the patches. > > With the exception that you all don't know what year it is: > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Daniele, why were the patches applied without this A-b? I'm just preparing the drm-intel-gt-next pull request and now it appears like we're pushing a lot of commits outside of drm without any Acks. Please reach out to the maintainers *before* pushing code for other subsystems. Unless you get an explicit ack to do so, do not push such code. Quoting from the committer guidelines[1] the first rule is: "Only push patches changing drivers/gpu/drm/i915." In those cases, please ping a maintainer and don't rush things. Regards, Joonas [1] https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-intel.html#high-level-guidelines
On 9/9/2022 3:24 AM, Joonas Lahtinen wrote: > Dave, do you have a preference how to deal with the mishap here, shall I do a > force-push to drm-intel-gt-next to correctly record the Acked-by or revert and > re-push? Or just leave it as is? > > Quoting Greg Kroah-Hartman (2022-09-01 18:09:09) >> On Sat, Aug 06, 2022 at 03:26:21PM +0300, Tomas Winkler wrote: >>> Add GSC support for XeHP SDV and DG2 platforms. >>> >>> The series includes changes for the mei driver: >>> - add ability to use polling instead of interrupts >>> - add ability to use extended timeouts >>> - setup extended operational memory for GSC >>> >>> The series includes changes for the i915 driver: >>> - allocate extended operational memory for GSC >>> - GSC on XeHP SDV offsets and definitions >>> >>> This patch set should be merged via gfx tree as >>> the auxiliary device belongs there. >>> Greg, your ACK is required for the drives/misc/mei code base, >>> please review the patches. >> With the exception that you all don't know what year it is: >> >> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Daniele, why were the patches applied without this A-b? Apologies, I usually rely on dim to pick up all the correct r-bs and acks from the ML and to warn me if something is missing, and I didn't realize that it hadn't automagically picked up the ack. > > I'm just preparing the drm-intel-gt-next pull request and now it appears > like we're pushing a lot of commits outside of drm without any Acks. > > Please reach out to the maintainers *before* pushing code for other > subsystems. Unless you get an explicit ack to do so, do not push such > code. I'm assuming you mean the i915 maintainers here, given that there is an ack from Greg in this email? Rodrigo was in the loop of us needing to merge this via drm, so I thought I was good on that side. I'll make sure to have an explicit ack on the ML next time (which is coming relatively soon, because there are some more mei patches in the DG2 HuC series). > > Quoting from the committer guidelines[1] the first rule is: > "Only push patches changing drivers/gpu/drm/i915." > > In those cases, please ping a maintainer and don't rush things. Will do. And apologies again for the mistake. Daniele > Regards, Joonas > > [1] https://drm.pages.freedesktop.org/maintainer-tools/committer-drm-intel.html#high-level-guidelines >
On Fri, 2022-09-09 at 08:17 -0700, Ceraolo Spurio, Daniele wrote: > > > On 9/9/2022 3:24 AM, Joonas Lahtinen wrote: > > Dave, do you have a preference how to deal with the mishap here, > > shall I do a > > force-push to drm-intel-gt-next to correctly record the Acked-by or > > revert and > > re-push? Or just leave it as is? Dave and Daniel, this question is still pertinent. > > > > Quoting Greg Kroah-Hartman (2022-09-01 18:09:09) > > > On Sat, Aug 06, 2022 at 03:26:21PM +0300, Tomas Winkler wrote: > > > > Add GSC support for XeHP SDV and DG2 platforms. > > > > > > > > The series includes changes for the mei driver: > > > > - add ability to use polling instead of interrupts > > > > - add ability to use extended timeouts > > > > - setup extended operational memory for GSC > > > > > > > > The series includes changes for the i915 driver: > > > > - allocate extended operational memory for GSC > > > > - GSC on XeHP SDV offsets and definitions > > > > > > > > This patch set should be merged via gfx tree as > > > > the auxiliary device belongs there. > > > > Greg, your ACK is required for the drives/misc/mei code base, > > > > please review the patches. > > > With the exception that you all don't know what year it is: > > > > > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Daniele, why were the patches applied without this A-b? > > Apologies, I usually rely on dim to pick up all the correct r-bs and > acks from the ML and to warn me if something is missing, and I didn't > realize that it hadn't automagically picked up the ack. I understand the feeling. Recently I merged a patch from Vinay relying on patchwork to get the reviewed-by and I forgot to double check. dim picks up the "Link:", but I don't believe it picks any ack or rv-b from the mailing list. Patchwork does if you use pwclient or something like that. Anyway, lesson to both of us to always double-check, regardless the tool used. > > > > > I'm just preparing the drm-intel-gt-next pull request and now it > > appears > > like we're pushing a lot of commits outside of drm without any > > Acks. > > > > Please reach out to the maintainers *before* pushing code for other > > subsystems. Unless you get an explicit ack to do so, do not push > > such > > code. > > I'm assuming you mean the i915 maintainers here, given that there is > an > ack from Greg in this email? Rodrigo was in the loop of us needing to > merge this via drm, so I thought I was good on that side. I'll make > sure > to have an explicit ack on the ML next time (which is coming > relatively > soon, because there are some more mei patches in the DG2 HuC series). That's my fault indeed. I was following the movement, but I failed to step up right after I saw Greg's ack. Although I also noticed some re-send and reviews in progress even after the ack, I should had been more active there. Sorry, Rodrigo. > > > > > Quoting from the committer guidelines[1] the first rule is: > > "Only push patches changing drivers/gpu/drm/i915." > > > > In those cases, please ping a maintainer and don't rush things. > > Will do. And apologies again for the mistake. > > Daniele > > > Regards, Joonas > > > > [1] https://drm.pages.freedesktop.org/maintainer-tools/committer- > > drm-intel.html#high-level-guidelines > > >
On Fri, Sep 09, 2022 at 04:33:45PM +0000, Rodrigo Vivi wrote: >On Fri, 2022-09-09 at 08:17 -0700, Ceraolo Spurio, Daniele wrote: >> >> >> On 9/9/2022 3:24 AM, Joonas Lahtinen wrote: >> > Dave, do you have a preference how to deal with the mishap here, >> > shall I do a >> > force-push to drm-intel-gt-next to correctly record the Acked-by or >> > revert and >> > re-push? Or just leave it as is? > >Dave and Daniel, this question is still pertinent. > >> > >> > Quoting Greg Kroah-Hartman (2022-09-01 18:09:09) >> > > On Sat, Aug 06, 2022 at 03:26:21PM +0300, Tomas Winkler wrote: >> > > > Add GSC support for XeHP SDV and DG2 platforms. >> > > > >> > > > The series includes changes for the mei driver: >> > > > - add ability to use polling instead of interrupts >> > > > - add ability to use extended timeouts >> > > > - setup extended operational memory for GSC >> > > > >> > > > The series includes changes for the i915 driver: >> > > > - allocate extended operational memory for GSC >> > > > - GSC on XeHP SDV offsets and definitions >> > > > >> > > > This patch set should be merged via gfx tree as >> > > > the auxiliary device belongs there. >> > > > Greg, your ACK is required for the drives/misc/mei code base, >> > > > please review the patches. >> > > With the exception that you all don't know what year it is: >> > > >> > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >> > Daniele, why were the patches applied without this A-b? >> >> Apologies, I usually rely on dim to pick up all the correct r-bs and >> acks from the ML and to warn me if something is missing, and I didn't >> realize that it hadn't automagically picked up the ack. > >I understand the feeling. Recently I merged a patch from Vinay relying >on patchwork to get the reviewed-by and I forgot to double check. > >dim picks up the "Link:", but I don't believe it picks any ack or rv-b >from the mailing list. Patchwork does if you use pwclient or something >like that. When you download the patch from patchwork, it will include the r-b/a-b for the patches, not the cover letter. $ curl https://patchwork.freedesktop.org/api/1.0/series/106638/revisions/5/mbox/ | \ grep Acked-by $ Patchwork simply ignores the cover and does nothing. b4 has an option to propagate the a-b on cover to the patches (-t): $ b4 am -o - -t YxDLFWjIllqqh9de@kroah.com | grep Acked ... Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ... b4 also has options to work on your local mailbox instead of relying on the external server to apply a-b/r-b. Maybe people should be using it more instead of relying on patchwork. Should the Acked-by be recorded on each patch? That is what is usually done, but if preference would be for a merge commit to be created and Acked-by recorded in the merge commit, dim would need to learn a few things. b4 is already prepared: $ b4 shazam -M -P1-15 YxDLFWjIllqqh9de@kroah.com Lucas De Marchi
Quoting Vivi, Rodrigo (2022-09-09 19:33:45) > On Fri, 2022-09-09 at 08:17 -0700, Ceraolo Spurio, Daniele wrote: > > > > > > On 9/9/2022 3:24 AM, Joonas Lahtinen wrote: > > > Dave, do you have a preference how to deal with the mishap here, > > > shall I do a > > > force-push to drm-intel-gt-next to correctly record the Acked-by or > > > revert and > > > re-push? Or just leave it as is? > > Dave and Daniel, this question is still pertinent. Discussed with Dave and I did a force-push to add the missing Acked-by's. Daniele, I think the tradition is that you have volunteered yourself to improve dim to nag about missing Acked-by's for patches outside of i915 when pushing to drm-intel-gt-next. Regards, Joonas > > > > > > > Quoting Greg Kroah-Hartman (2022-09-01 18:09:09) > > > > On Sat, Aug 06, 2022 at 03:26:21PM +0300, Tomas Winkler wrote: > > > > > Add GSC support for XeHP SDV and DG2 platforms. > > > > > > > > > > The series includes changes for the mei driver: > > > > > - add ability to use polling instead of interrupts > > > > > - add ability to use extended timeouts > > > > > - setup extended operational memory for GSC > > > > > > > > > > The series includes changes for the i915 driver: > > > > > - allocate extended operational memory for GSC > > > > > - GSC on XeHP SDV offsets and definitions > > > > > > > > > > This patch set should be merged via gfx tree as > > > > > the auxiliary device belongs there. > > > > > Greg, your ACK is required for the drives/misc/mei code base, > > > > > please review the patches. > > > > With the exception that you all don't know what year it is: > > > > > > > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > Daniele, why were the patches applied without this A-b? > > > > Apologies, I usually rely on dim to pick up all the correct r-bs and > > acks from the ML and to warn me if something is missing, and I didn't > > realize that it hadn't automagically picked up the ack. > > I understand the feeling. Recently I merged a patch from Vinay relying > on patchwork to get the reviewed-by and I forgot to double check. > > dim picks up the "Link:", but I don't believe it picks any ack or rv-b > from the mailing list. Patchwork does if you use pwclient or something > like that. > > Anyway, lesson to both of us to always double-check, regardless the > tool used. > > > > > > > > > I'm just preparing the drm-intel-gt-next pull request and now it > > > appears > > > like we're pushing a lot of commits outside of drm without any > > > Acks. > > > > > > Please reach out to the maintainers *before* pushing code for other > > > subsystems. Unless you get an explicit ack to do so, do not push > > > such > > > code. > > > > I'm assuming you mean the i915 maintainers here, given that there is > > an > > ack from Greg in this email? Rodrigo was in the loop of us needing to > > merge this via drm, so I thought I was good on that side. I'll make > > sure > > to have an explicit ack on the ML next time (which is coming > > relatively > > soon, because there are some more mei patches in the DG2 HuC series). > > That's my fault indeed. I was following the movement, but I failed > to step up right after I saw Greg's ack. > Although I also noticed some re-send and reviews in progress even > after the ack, I should had been more active there. > > Sorry, > Rodrigo. > > > > > > > > > Quoting from the committer guidelines[1] the first rule is: > > > "Only push patches changing drivers/gpu/drm/i915." > > > > > > In those cases, please ping a maintainer and don't rush things. > > > > Will do. And apologies again for the mistake. > > > > Daniele > > > > > Regards, Joonas > > > > > > [1] https://drm.pages.freedesktop.org/maintainer-tools/committer- > > > drm-intel.html#high-level-guidelines > > > > > >