diff mbox series

[2/6] drm: ci: Force db410c to host mode

Message ID 20230825122435.316272-3-vignesh.raman@collabora.com (mailing list archive)
State New, archived
Delegated to: Neil Armstrong
Headers show
Series drm: ci: fixes | expand

Commit Message

Vignesh Raman Aug. 25, 2023, 12:24 p.m. UTC
Force db410c to host mode to fix network issue which results in failure
to mount root fs via NFS.
See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8

Since this fix is not sent upstream, add it to build.sh script
before building the kernel and dts. Better approach would be
to use devicetree overlays.

Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
---
 drivers/gpu/drm/ci/build.sh | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Jani Nikula Aug. 25, 2023, 1:56 p.m. UTC | #1
On Fri, 25 Aug 2023, Vignesh Raman <vignesh.raman@collabora.com> wrote:
> Force db410c to host mode to fix network issue which results in failure
> to mount root fs via NFS.
> See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
>
> Since this fix is not sent upstream, add it to build.sh script
> before building the kernel and dts. Better approach would be
> to use devicetree overlays.
>
> Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
> ---
>  drivers/gpu/drm/ci/build.sh | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
> index 7b014287a041..c39834bd6bd7 100644
> --- a/drivers/gpu/drm/ci/build.sh
> +++ b/drivers/gpu/drm/ci/build.sh
> @@ -70,6 +70,10 @@ if [ -z "$CI_MERGE_REQUEST_PROJECT_PATH" ]; then
>      fi
>  fi
>  
> +# Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS.
> +# See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> +sed -i '/&usb {/,/status = "okay";/s/status = "okay";/&\n\tdr_mode = "host";/' arch/arm64/boot/dts/qcom/apq8016-sbc.dts
> +

It seems like a really bad idea to me to have the CI build modify the
source tree before building.

The kernel being built will have a dirty git repo, and the localversion
will have -dirty in it.

I think it would be better to do out-of-tree builds and assume the
source is read-only.

>  for opt in $ENABLE_KCONFIGS; do
>    echo CONFIG_$opt=y >> drivers/gpu/drm/ci/${KERNEL_ARCH}.config
>  done

Ditto for the config changes in the context here. Those are files in
git, don't change them.

Shouldn't this use something like 'scripts/config --enable' or
'scripts/config --disable' on the .config file to be used for building
instead?


BR,
Jani.
Helen Koike Aug. 25, 2023, 2:09 p.m. UTC | #2
Hi Jani, thanks for your comments

On Friday, August 25, 2023 10:56 -03, Jani Nikula <jani.nikula@linux.intel.com> wrote:

> On Fri, 25 Aug 2023, Vignesh Raman <vignesh.raman@collabora.com> wrote:
> > Force db410c to host mode to fix network issue which results in failure
> > to mount root fs via NFS.
> > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> >
> > Since this fix is not sent upstream, add it to build.sh script
> > before building the kernel and dts. Better approach would be
> > to use devicetree overlays.
> >
> > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
> > ---
> >  drivers/gpu/drm/ci/build.sh | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
> > index 7b014287a041..c39834bd6bd7 100644
> > --- a/drivers/gpu/drm/ci/build.sh
> > +++ b/drivers/gpu/drm/ci/build.sh
> > @@ -70,6 +70,10 @@ if [ -z "$CI_MERGE_REQUEST_PROJECT_PATH" ]; then
> >      fi
> >  fi
> >  
> > +# Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS.
> > +# See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > +sed -i '/&usb {/,/status = "okay";/s/status = "okay";/&\n\tdr_mode = "host";/' arch/arm64/boot/dts/qcom/apq8016-sbc.dts
> > +
> 
> It seems like a really bad idea to me to have the CI build modify the
> source tree before building.
> 
> The kernel being built will have a dirty git repo, and the localversion
> will have -dirty in it.

Is it bad?

The other option was to work with device tree overlays (but we still need to spend some time to
see how to fit it all together)

> 
> I think it would be better to do out-of-tree builds and assume the
> source is read-only.

I'm not sure I get what do you call out-of-tree builds.

Another option would be to apply .patch file, or to have another branch
just with fix ups for ci that would be applied in the tree before building.

> 
> >  for opt in $ENABLE_KCONFIGS; do
> >    echo CONFIG_$opt=y >> drivers/gpu/drm/ci/${KERNEL_ARCH}.config
> >  done
> 
> Ditto for the config changes in the context here. Those are files in
> git, don't change them.

Probably these changes could go directly into drivers/gpu/drm/ci/${KERNEL_ARCH}.config
files, no need to modify them on the fly here

> 
> Shouldn't this use something like 'scripts/config --enable' or
> 'scripts/config --disable' on the .config file to be used for building
> instead?

I wasn't aware about this possibility, looks cleaner indeed.

Regards,
Helen

> 
> 
> BR,
> Jani.
> 
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
Maxime Ripard Aug. 25, 2023, 2:27 p.m. UTC | #3
Hi Helen,

On Fri, Aug 25, 2023 at 03:09:04PM +0100, Helen Mae Koike Fornazier wrote:
> Hi Jani, thanks for your comments
> 
> On Friday, August 25, 2023 10:56 -03, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> 
> > On Fri, 25 Aug 2023, Vignesh Raman <vignesh.raman@collabora.com> wrote:
> > > Force db410c to host mode to fix network issue which results in failure
> > > to mount root fs via NFS.
> > > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > >
> > > Since this fix is not sent upstream, add it to build.sh script
> > > before building the kernel and dts. Better approach would be
> > > to use devicetree overlays.
> > >
> > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
> > > ---
> > >  drivers/gpu/drm/ci/build.sh | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
> > > index 7b014287a041..c39834bd6bd7 100644
> > > --- a/drivers/gpu/drm/ci/build.sh
> > > +++ b/drivers/gpu/drm/ci/build.sh
> > > @@ -70,6 +70,10 @@ if [ -z "$CI_MERGE_REQUEST_PROJECT_PATH" ]; then
> > >      fi
> > >  fi
> > >  
> > > +# Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS.
> > > +# See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > > +sed -i '/&usb {/,/status = "okay";/s/status = "okay";/&\n\tdr_mode = "host";/' arch/arm64/boot/dts/qcom/apq8016-sbc.dts
> > > +
> > 
> > It seems like a really bad idea to me to have the CI build modify the
> > source tree before building.
> > 
> > The kernel being built will have a dirty git repo, and the localversion
> > will have -dirty in it.
> 
> Is it bad?
> 
> The other option was to work with device tree overlays (but we still
> need to spend some time to see how to fit it all together)

That would be much better. libfdt provides an fdtoverlay command to
merge a base device tree with an overlay.

Do that while setting up the DUT and you're done :)

Maxime
Rob Clark Aug. 25, 2023, 2:30 p.m. UTC | #4
On Fri, Aug 25, 2023 at 6:56 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
>
> On Fri, 25 Aug 2023, Vignesh Raman <vignesh.raman@collabora.com> wrote:
> > Force db410c to host mode to fix network issue which results in failure
> > to mount root fs via NFS.
> > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> >
> > Since this fix is not sent upstream, add it to build.sh script
> > before building the kernel and dts. Better approach would be
> > to use devicetree overlays.
> >
> > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
> > ---
> >  drivers/gpu/drm/ci/build.sh | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
> > index 7b014287a041..c39834bd6bd7 100644
> > --- a/drivers/gpu/drm/ci/build.sh
> > +++ b/drivers/gpu/drm/ci/build.sh
> > @@ -70,6 +70,10 @@ if [ -z "$CI_MERGE_REQUEST_PROJECT_PATH" ]; then
> >      fi
> >  fi
> >
> > +# Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS.
> > +# See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > +sed -i '/&usb {/,/status = "okay";/s/status = "okay";/&\n\tdr_mode = "host";/' arch/arm64/boot/dts/qcom/apq8016-sbc.dts
> > +
>
> It seems like a really bad idea to me to have the CI build modify the
> source tree before building.
>
> The kernel being built will have a dirty git repo, and the localversion
> will have -dirty in it.
>
> I think it would be better to do out-of-tree builds and assume the
> source is read-only.

We have the ${target_branch}-external-fixes mechanism to merge
necessary changes before building the kernel for CI.  Which is
necessary for a couple of reasons:

1) patches like this which aren't appropriate upstream but necessary
due to the CI lab setup
2) target branch if often based on an early -rc, and it isn't unheard
of to need some fix for some board or another which isn't appropriate
to land via drm-next

We should use the -external-fixes branch mechanism for patches like this one.

BR,
-R

> >  for opt in $ENABLE_KCONFIGS; do
> >    echo CONFIG_$opt=y >> drivers/gpu/drm/ci/${KERNEL_ARCH}.config
> >  done
>
> Ditto for the config changes in the context here. Those are files in
> git, don't change them.
>
> Shouldn't this use something like 'scripts/config --enable' or
> 'scripts/config --disable' on the .config file to be used for building
> instead?
>
>
> BR,
> Jani.
>
>
> --
> Jani Nikula, Intel Open Source Graphics Center
Helen Koike Aug. 25, 2023, 2:34 p.m. UTC | #5
On Friday, August 25, 2023 11:30 -03, Rob Clark <robdclark@gmail.com> wrote:

> On Fri, Aug 25, 2023 at 6:56 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> >
> > On Fri, 25 Aug 2023, Vignesh Raman <vignesh.raman@collabora.com> wrote:
> > > Force db410c to host mode to fix network issue which results in failure
> > > to mount root fs via NFS.
> > > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > >
> > > Since this fix is not sent upstream, add it to build.sh script
> > > before building the kernel and dts. Better approach would be
> > > to use devicetree overlays.
> > >
> > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
> > > ---
> > >  drivers/gpu/drm/ci/build.sh | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
> > > index 7b014287a041..c39834bd6bd7 100644
> > > --- a/drivers/gpu/drm/ci/build.sh
> > > +++ b/drivers/gpu/drm/ci/build.sh
> > > @@ -70,6 +70,10 @@ if [ -z "$CI_MERGE_REQUEST_PROJECT_PATH" ]; then
> > >      fi
> > >  fi
> > >
> > > +# Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS.
> > > +# See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > > +sed -i '/&usb {/,/status = "okay";/s/status = "okay";/&\n\tdr_mode = "host";/' arch/arm64/boot/dts/qcom/apq8016-sbc.dts
> > > +
> >
> > It seems like a really bad idea to me to have the CI build modify the
> > source tree before building.
> >
> > The kernel being built will have a dirty git repo, and the localversion
> > will have -dirty in it.
> >
> > I think it would be better to do out-of-tree builds and assume the
> > source is read-only.
> 
> We have the ${target_branch}-external-fixes mechanism to merge
> necessary changes before building the kernel for CI.  Which is
> necessary for a couple of reasons:

Should we create an official topic/drm-ci-external-fixes branch ?

Regards,
Helen

> 
> 1) patches like this which aren't appropriate upstream but necessary
> due to the CI lab setup
> 2) target branch if often based on an early -rc, and it isn't unheard
> of to need some fix for some board or another which isn't appropriate
> to land via drm-next
> 
> We should use the -external-fixes branch mechanism for patches like this one.
> 
> BR,
> -R
> 
> > >  for opt in $ENABLE_KCONFIGS; do
> > >    echo CONFIG_$opt=y >> drivers/gpu/drm/ci/${KERNEL_ARCH}.config
> > >  done
> >
> > Ditto for the config changes in the context here. Those are files in
> > git, don't change them.
> >
> > Shouldn't this use something like 'scripts/config --enable' or
> > 'scripts/config --disable' on the .config file to be used for building
> > instead?
> >
> >
> > BR,
> > Jani.
> >
> >
> > --
> > Jani Nikula, Intel Open Source Graphics Center
Rob Clark Aug. 25, 2023, 2:41 p.m. UTC | #6
On Fri, Aug 25, 2023 at 7:34 AM Helen Mae Koike Fornazier
<helen.koike@collabora.com> wrote:
>
> On Friday, August 25, 2023 11:30 -03, Rob Clark <robdclark@gmail.com> wrote:
>
> > On Fri, Aug 25, 2023 at 6:56 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > >
> > > On Fri, 25 Aug 2023, Vignesh Raman <vignesh.raman@collabora.com> wrote:
> > > > Force db410c to host mode to fix network issue which results in failure
> > > > to mount root fs via NFS.
> > > > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > > >
> > > > Since this fix is not sent upstream, add it to build.sh script
> > > > before building the kernel and dts. Better approach would be
> > > > to use devicetree overlays.
> > > >
> > > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
> > > > ---
> > > >  drivers/gpu/drm/ci/build.sh | 4 ++++
> > > >  1 file changed, 4 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
> > > > index 7b014287a041..c39834bd6bd7 100644
> > > > --- a/drivers/gpu/drm/ci/build.sh
> > > > +++ b/drivers/gpu/drm/ci/build.sh
> > > > @@ -70,6 +70,10 @@ if [ -z "$CI_MERGE_REQUEST_PROJECT_PATH" ]; then
> > > >      fi
> > > >  fi
> > > >
> > > > +# Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS.
> > > > +# See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > > > +sed -i '/&usb {/,/status = "okay";/s/status = "okay";/&\n\tdr_mode = "host";/' arch/arm64/boot/dts/qcom/apq8016-sbc.dts
> > > > +
> > >
> > > It seems like a really bad idea to me to have the CI build modify the
> > > source tree before building.
> > >
> > > The kernel being built will have a dirty git repo, and the localversion
> > > will have -dirty in it.
> > >
> > > I think it would be better to do out-of-tree builds and assume the
> > > source is read-only.
> >
> > We have the ${target_branch}-external-fixes mechanism to merge
> > necessary changes before building the kernel for CI.  Which is
> > necessary for a couple of reasons:
>
> Should we create an official topic/drm-ci-external-fixes branch ?

Hmm, maybe.. I guess as we expand this to more driver trees, and want
to be able to re-run CI in the drm tree after merges to
drm-next/drm-fixes, we maybe want to have central
drm-next-external-fixes and drm-fixes-external-fixes.  I guess we can
keep those based on drm-next and drm-fixes?  And if there would be
conflicts because, say, ${driver}-next is behind drm-next, then
${driver}-next could be rebased on drm-next?

BR,
-R

> Regards,
> Helen
>
> >
> > 1) patches like this which aren't appropriate upstream but necessary
> > due to the CI lab setup
> > 2) target branch if often based on an early -rc, and it isn't unheard
> > of to need some fix for some board or another which isn't appropriate
> > to land via drm-next
> >
> > We should use the -external-fixes branch mechanism for patches like this one.
> >
> > BR,
> > -R
> >
> > > >  for opt in $ENABLE_KCONFIGS; do
> > > >    echo CONFIG_$opt=y >> drivers/gpu/drm/ci/${KERNEL_ARCH}.config
> > > >  done
> > >
> > > Ditto for the config changes in the context here. Those are files in
> > > git, don't change them.
> > >
> > > Shouldn't this use something like 'scripts/config --enable' or
> > > 'scripts/config --disable' on the .config file to be used for building
> > > instead?
> > >
> > >
> > > BR,
> > > Jani.
> > >
> > >
> > > --
> > > Jani Nikula, Intel Open Source Graphics Center
>
Helen Koike Aug. 25, 2023, 3:06 p.m. UTC | #7
On Friday, August 25, 2023 11:41 -03, Rob Clark <robdclark@gmail.com> wrote:

> On Fri, Aug 25, 2023 at 7:34 AM Helen Mae Koike Fornazier
> <helen.koike@collabora.com> wrote:
> >
> > On Friday, August 25, 2023 11:30 -03, Rob Clark <robdclark@gmail.com> wrote:
> >
> > > On Fri, Aug 25, 2023 at 6:56 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > > >
> > > > On Fri, 25 Aug 2023, Vignesh Raman <vignesh.raman@collabora.com> wrote:
> > > > > Force db410c to host mode to fix network issue which results in failure
> > > > > to mount root fs via NFS.
> > > > > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > > > >
> > > > > Since this fix is not sent upstream, add it to build.sh script
> > > > > before building the kernel and dts. Better approach would be
> > > > > to use devicetree overlays.
> > > > >
> > > > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
> > > > > ---
> > > > >  drivers/gpu/drm/ci/build.sh | 4 ++++
> > > > >  1 file changed, 4 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
> > > > > index 7b014287a041..c39834bd6bd7 100644
> > > > > --- a/drivers/gpu/drm/ci/build.sh
> > > > > +++ b/drivers/gpu/drm/ci/build.sh
> > > > > @@ -70,6 +70,10 @@ if [ -z "$CI_MERGE_REQUEST_PROJECT_PATH" ]; then
> > > > >      fi
> > > > >  fi
> > > > >
> > > > > +# Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS.
> > > > > +# See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > > > > +sed -i '/&usb {/,/status = "okay";/s/status = "okay";/&\n\tdr_mode = "host";/' arch/arm64/boot/dts/qcom/apq8016-sbc.dts
> > > > > +
> > > >
> > > > It seems like a really bad idea to me to have the CI build modify the
> > > > source tree before building.
> > > >
> > > > The kernel being built will have a dirty git repo, and the localversion
> > > > will have -dirty in it.
> > > >
> > > > I think it would be better to do out-of-tree builds and assume the
> > > > source is read-only.
> > >
> > > We have the ${target_branch}-external-fixes mechanism to merge
> > > necessary changes before building the kernel for CI.  Which is
> > > necessary for a couple of reasons:
> >
> > Should we create an official topic/drm-ci-external-fixes branch ?
> 
> Hmm, maybe.. I guess as we expand this to more driver trees, and want
> to be able to re-run CI in the drm tree after merges to
> drm-next/drm-fixes, we maybe want to have central
> drm-next-external-fixes and drm-fixes-external-fixes.  I guess we can
> keep those based on drm-next and drm-fixes?  And if there would be
> conflicts because, say, ${driver}-next is behind drm-next, then
> ${driver}-next could be rebased on drm-next?
> 

tbh this is one of the reasons I would prefer in-code fixes instead of
commits on a -external-fixes branch, since it seems things start to become
complex to manage all different trees for people executing ci tests
on different history points, but I don't oppose going for -external-fixes
either.

Regards,
Helen

> BR,
> -R
> 
> > Regards,
> > Helen
> >
> > >
> > > 1) patches like this which aren't appropriate upstream but necessary
> > > due to the CI lab setup
> > > 2) target branch if often based on an early -rc, and it isn't unheard
> > > of to need some fix for some board or another which isn't appropriate
> > > to land via drm-next
> > >
> > > We should use the -external-fixes branch mechanism for patches like this one.
> > >
> > > BR,
> > > -R
> > >
> > > > >  for opt in $ENABLE_KCONFIGS; do
> > > > >    echo CONFIG_$opt=y >> drivers/gpu/drm/ci/${KERNEL_ARCH}.config
> > > > >  done
> > > >
> > > > Ditto for the config changes in the context here. Those are files in
> > > > git, don't change them.
> > > >
> > > > Shouldn't this use something like 'scripts/config --enable' or
> > > > 'scripts/config --disable' on the .config file to be used for building
> > > > instead?
> > > >
> > > >
> > > > BR,
> > > > Jani.
> > > >
> > > >
> > > > --
> > > > Jani Nikula, Intel Open Source Graphics Center
> >
Jani Nikula Aug. 25, 2023, 5:50 p.m. UTC | #8
On Fri, 25 Aug 2023, "Helen Mae Koike Fornazier" <helen.koike@collabora.com> wrote:
> I'm not sure I get what do you call out-of-tree builds.

Using 'make O=dir' (see make help) to separate source and and output
directories.

BR,
Jani.
Rob Clark Aug. 25, 2023, 6:15 p.m. UTC | #9
On Fri, Aug 25, 2023 at 8:06 AM Helen Mae Koike Fornazier
<helen.koike@collabora.com> wrote:
>
> On Friday, August 25, 2023 11:41 -03, Rob Clark <robdclark@gmail.com> wrote:
>
> > On Fri, Aug 25, 2023 at 7:34 AM Helen Mae Koike Fornazier
> > <helen.koike@collabora.com> wrote:
> > >
> > > On Friday, August 25, 2023 11:30 -03, Rob Clark <robdclark@gmail.com> wrote:
> > >
> > > > On Fri, Aug 25, 2023 at 6:56 AM Jani Nikula <jani.nikula@linux.intel.com> wrote:
> > > > >
> > > > > On Fri, 25 Aug 2023, Vignesh Raman <vignesh.raman@collabora.com> wrote:
> > > > > > Force db410c to host mode to fix network issue which results in failure
> > > > > > to mount root fs via NFS.
> > > > > > See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > > > > >
> > > > > > Since this fix is not sent upstream, add it to build.sh script
> > > > > > before building the kernel and dts. Better approach would be
> > > > > > to use devicetree overlays.
> > > > > >
> > > > > > Signed-off-by: Vignesh Raman <vignesh.raman@collabora.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/ci/build.sh | 4 ++++
> > > > > >  1 file changed, 4 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
> > > > > > index 7b014287a041..c39834bd6bd7 100644
> > > > > > --- a/drivers/gpu/drm/ci/build.sh
> > > > > > +++ b/drivers/gpu/drm/ci/build.sh
> > > > > > @@ -70,6 +70,10 @@ if [ -z "$CI_MERGE_REQUEST_PROJECT_PATH" ]; then
> > > > > >      fi
> > > > > >  fi
> > > > > >
> > > > > > +# Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS.
> > > > > > +# See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
> > > > > > +sed -i '/&usb {/,/status = "okay";/s/status = "okay";/&\n\tdr_mode = "host";/' arch/arm64/boot/dts/qcom/apq8016-sbc.dts
> > > > > > +
> > > > >
> > > > > It seems like a really bad idea to me to have the CI build modify the
> > > > > source tree before building.
> > > > >
> > > > > The kernel being built will have a dirty git repo, and the localversion
> > > > > will have -dirty in it.
> > > > >
> > > > > I think it would be better to do out-of-tree builds and assume the
> > > > > source is read-only.
> > > >
> > > > We have the ${target_branch}-external-fixes mechanism to merge
> > > > necessary changes before building the kernel for CI.  Which is
> > > > necessary for a couple of reasons:
> > >
> > > Should we create an official topic/drm-ci-external-fixes branch ?
> >
> > Hmm, maybe.. I guess as we expand this to more driver trees, and want
> > to be able to re-run CI in the drm tree after merges to
> > drm-next/drm-fixes, we maybe want to have central
> > drm-next-external-fixes and drm-fixes-external-fixes.  I guess we can
> > keep those based on drm-next and drm-fixes?  And if there would be
> > conflicts because, say, ${driver}-next is behind drm-next, then
> > ${driver}-next could be rebased on drm-next?
> >
>
> tbh this is one of the reasons I would prefer in-code fixes instead of
> commits on a -external-fixes branch, since it seems things start to become
> complex to manage all different trees for people executing ci tests
> on different history points, but I don't oppose going for -external-fixes
> either.

If by in-code you mean in the same branch that we are running CI on, I
think that will be difficult to do without having force-pushes later
to remove the unrelated patch.

It doesn't happen every release, but sometimes there is an issue
requiring a fix outside of drm, which really shouldn't land via
drm-next.  In some cases even, we might need a temporary hack fix when
a proper solution is still being worked out.  Since the kernel
development is unlike mesa, where we have a single main branch and CI
runs on every change entering that main branch, we are going to have
to deal occasionally with breakage that comes in via another tree that
isn't running CI.  But IME so far with msm-next-external-fixes, it
hasn't been so bad.. I haven't even had to rebase it every kernel
cycle.  (If the fix cherry-picked into -external-fixes from a previous
release cycle already exists in msm-next then the merge of that commit
is a no-op.)

BR,
-R

> Regards,
> Helen
>
> > BR,
> > -R
> >
> > > Regards,
> > > Helen
> > >
> > > >
> > > > 1) patches like this which aren't appropriate upstream but necessary
> > > > due to the CI lab setup
> > > > 2) target branch if often based on an early -rc, and it isn't unheard
> > > > of to need some fix for some board or another which isn't appropriate
> > > > to land via drm-next
> > > >
> > > > We should use the -external-fixes branch mechanism for patches like this one.
> > > >
> > > > BR,
> > > > -R
> > > >
> > > > > >  for opt in $ENABLE_KCONFIGS; do
> > > > > >    echo CONFIG_$opt=y >> drivers/gpu/drm/ci/${KERNEL_ARCH}.config
> > > > > >  done
> > > > >
> > > > > Ditto for the config changes in the context here. Those are files in
> > > > > git, don't change them.
> > > > >
> > > > > Shouldn't this use something like 'scripts/config --enable' or
> > > > > 'scripts/config --disable' on the .config file to be used for building
> > > > > instead?
> > > > >
> > > > >
> > > > > BR,
> > > > > Jani.
> > > > >
> > > > >
> > > > > --
> > > > > Jani Nikula, Intel Open Source Graphics Center
> > >
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/ci/build.sh b/drivers/gpu/drm/ci/build.sh
index 7b014287a041..c39834bd6bd7 100644
--- a/drivers/gpu/drm/ci/build.sh
+++ b/drivers/gpu/drm/ci/build.sh
@@ -70,6 +70,10 @@  if [ -z "$CI_MERGE_REQUEST_PROJECT_PATH" ]; then
     fi
 fi
 
+# Force db410c to host mode to fix network issue which results in failure to mount root fs via NFS.
+# See https://gitlab.freedesktop.org/gfx-ci/linux/-/commit/cb72a629b8c15c80a54dda510743cefd1c4b65b8
+sed -i '/&usb {/,/status = "okay";/s/status = "okay";/&\n\tdr_mode = "host";/' arch/arm64/boot/dts/qcom/apq8016-sbc.dts
+
 for opt in $ENABLE_KCONFIGS; do
   echo CONFIG_$opt=y >> drivers/gpu/drm/ci/${KERNEL_ARCH}.config
 done