diff mbox series

[CI] drm/i915/mtl: Define GuC firmware version for MTL

Message ID 20230331225216.4188274-1-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show
Series [CI] drm/i915/mtl: Define GuC firmware version for MTL | expand

Commit Message

John Harrison March 31, 2023, 10:52 p.m. UTC 
From: John Harrison <John.C.Harrison@Intel.com>

First release of GuC for Meteorlake.

NB: As this is still pre-release and likely to change, use explicit
versioning for now. The official, full release will use reduced
version naming.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Lucas De Marchi April 7, 2023, 12:37 a.m. UTC | #1 
On Fri, Mar 31, 2023 at 03:52:16PM -0700, John.C.Harrison@Intel.com wrote:
>From: John Harrison <John.C.Harrison@Intel.com>
>
>First release of GuC for Meteorlake.
>
>NB: As this is still pre-release and likely to change, use explicit
>versioning for now. The official, full release will use reduced
>version naming.
>
>Signed-off-by: John Harrison <John.C.Harrison@Intel.com>

Applied to topic/core-for-CI with acks by Rodrigo and Tvrtko.
Reference issue: https://gitlab.freedesktop.org/drm/intel/-/issues/8343

Going forward we should plan to have these kind of patches in
drm-intel-gt-next itself rather than using the CI branch. The CI branch
is not the proper place.

To be clear, since MTL is under force probe and not normally enabled,
it's ok to bump the major version without retaining compabibility with
the previous version, as per
https://docs.kernel.org/driver-api/firmware/firmware-usage-guidelines.html


I think the main blocker right now to use drm-intel-gt-next would be
because MODULE_FIRMWARE() is unaware of force_probe. Then distros
would start getting a warning when creating their initrd that they may
have missed a firmware. But that firmware itself is not present in the
upstream linux-firmware repo.  We can think about a way to hide the fw
def for *new* firmware (not the legacy ones) that are using the mmp
version.

For now, let's keep this as is and use the CI branch to assess the
driver health with GuC.


thanks
Lucas De Marchi

>---
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>index 264c952f777bb..1ac6f9f340e31 100644
>--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>@@ -79,6 +79,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
>  * security fixes, etc. to be enabled.
>  */
> #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \
>+	fw_def(METEORLAKE,   0, guc_mmp(mtl,  70, 6, 5)) \
> 	fw_def(DG2,          0, guc_maj(dg2,  70, 5)) \
> 	fw_def(ALDERLAKE_P,  0, guc_maj(adlp, 70, 5)) \
> 	fw_def(ALDERLAKE_P,  0, guc_mmp(adlp, 70, 1, 1)) \
>-- 
>2.39.1
>
Rodrigo Vivi April 7, 2023, 9:45 a.m. UTC | #2 
On Thu, Apr 06, 2023 at 05:37:58PM -0700, Lucas De Marchi wrote:
> On Fri, Mar 31, 2023 at 03:52:16PM -0700, John.C.Harrison@Intel.com wrote:
> > From: John Harrison <John.C.Harrison@Intel.com>
> > 
> > First release of GuC for Meteorlake.
> > 
> > NB: As this is still pre-release and likely to change, use explicit
> > versioning for now. The official, full release will use reduced
> > version naming.
> > 
> > Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> 
> Applied to topic/core-for-CI with acks by Rodrigo and Tvrtko.
> Reference issue: https://gitlab.freedesktop.org/drm/intel/-/issues/8343
> 
> Going forward we should plan to have these kind of patches in
> drm-intel-gt-next itself rather than using the CI branch. The CI branch
> is not the proper place.
> 
> To be clear, since MTL is under force probe and not normally enabled,
> it's ok to bump the major version without retaining compabibility with
> the previous version, as per
> https://docs.kernel.org/driver-api/firmware/firmware-usage-guidelines.html
> 
> 
> I think the main blocker right now to use drm-intel-gt-next would be
> because MODULE_FIRMWARE() is unaware of force_probe. Then distros
> would start getting a warning when creating their initrd that they may
> have missed a firmware. But that firmware itself is not present in the
> upstream linux-firmware repo.  We can think about a way to hide the fw
> def for *new* firmware (not the legacy ones) that are using the mmp
> version.

Maybe we should simply move to the final version path sooner than later?

Even if that means more updates in the blobs at linux-firmware.git, that
would allow the OSVs to have this final patch sooner in their trees.

> 
> For now, let's keep this as is and use the CI branch to assess the
> driver health with GuC.
> 
> 
> thanks
> Lucas De Marchi
> 
> > ---
> > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 +
> > 1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> > index 264c952f777bb..1ac6f9f340e31 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> > @@ -79,6 +79,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
> >  * security fixes, etc. to be enabled.
> >  */
> > #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \
> > +	fw_def(METEORLAKE,   0, guc_mmp(mtl,  70, 6, 5)) \
> > 	fw_def(DG2,          0, guc_maj(dg2,  70, 5)) \
> > 	fw_def(ALDERLAKE_P,  0, guc_maj(adlp, 70, 5)) \
> > 	fw_def(ALDERLAKE_P,  0, guc_mmp(adlp, 70, 1, 1)) \
> > -- 
> > 2.39.1
> >
Lucas De Marchi April 7, 2023, 1:47 p.m. UTC | #3 
On Fri, Apr 07, 2023 at 05:45:52AM -0400, Rodrigo Vivi wrote:
>On Thu, Apr 06, 2023 at 05:37:58PM -0700, Lucas De Marchi wrote:
>> On Fri, Mar 31, 2023 at 03:52:16PM -0700, John.C.Harrison@Intel.com wrote:
>> > From: John Harrison <John.C.Harrison@Intel.com>
>> >
>> > First release of GuC for Meteorlake.
>> >
>> > NB: As this is still pre-release and likely to change, use explicit
>> > versioning for now. The official, full release will use reduced
>> > version naming.
>> >
>> > Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>
>> Applied to topic/core-for-CI with acks by Rodrigo and Tvrtko.
>> Reference issue: https://gitlab.freedesktop.org/drm/intel/-/issues/8343
>>
>> Going forward we should plan to have these kind of patches in
>> drm-intel-gt-next itself rather than using the CI branch. The CI branch
>> is not the proper place.
>>
>> To be clear, since MTL is under force probe and not normally enabled,
>> it's ok to bump the major version without retaining compabibility with
>> the previous version, as per
>> https://docs.kernel.org/driver-api/firmware/firmware-usage-guidelines.html
>>
>>
>> I think the main blocker right now to use drm-intel-gt-next would be
>> because MODULE_FIRMWARE() is unaware of force_probe. Then distros
>> would start getting a warning when creating their initrd that they may
>> have missed a firmware. But that firmware itself is not present in the
>> upstream linux-firmware repo.  We can think about a way to hide the fw
>> def for *new* firmware (not the legacy ones) that are using the mmp
>> version.
>
>Maybe we should simply move to the final version path sooner than later?
>
>Even if that means more updates in the blobs at linux-firmware.git, that
>would allow the OSVs to have this final patch sooner in their trees.

it doesn't help much OSVs:  the firmware being used here is the full
version, not the major-only version.  And if we commit this firmware
with the major-only version, there is a chance the distro will update
the kernel without updating the firmware and we will have the wrong
firmware there, one that is not supposed to be used.

Also, if we commit all the "temporary" versions in the linux-firmware.git
repo, we may start to blow up its size. I'm not sure this is a best
practice for HW that is not available to the general public.

Options I see for future:

1)
	a) keep these temporary fw in our drm-firmware repo;
	b) hide the fw def under a kconfig

2)
	a) accept this kind o changes in the CI branch

3)
	a) commit to linux-firmware.git, knowing this is not a version
	that will be generally used
	b) commit to kernel in the usual way

Any other?

Note that this also impacts xe. Right now xe is not merged upstream,
so this is just ignored: there we have already the firmware version
for MTL and for PVC in the kernel.  Eventually we will need a similar
mechanism though, so better we agree upfront what that is.

Lucas De Marchi

>
>>
>> For now, let's keep this as is and use the CI branch to assess the
>> driver health with GuC.
>>
>>
>> thanks
>> Lucas De Marchi
>>
>> > ---
>> > drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 1 +
>> > 1 file changed, 1 insertion(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> > index 264c952f777bb..1ac6f9f340e31 100644
>> > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> > @@ -79,6 +79,7 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
>> >  * security fixes, etc. to be enabled.
>> >  */
>> > #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \
>> > +	fw_def(METEORLAKE,   0, guc_mmp(mtl,  70, 6, 5)) \
>> > 	fw_def(DG2,          0, guc_maj(dg2,  70, 5)) \
>> > 	fw_def(ALDERLAKE_P,  0, guc_maj(adlp, 70, 5)) \
>> > 	fw_def(ALDERLAKE_P,  0, guc_mmp(adlp, 70, 1, 1)) \
>> > --
>> > 2.39.1
>> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 264c952f777bb..1ac6f9f340e31 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -79,6 +79,7 @@  void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
  * security fixes, etc. to be enabled.
  */
 #define INTEL_GUC_FIRMWARE_DEFS(fw_def, guc_maj, guc_mmp) \
+	fw_def(METEORLAKE,   0, guc_mmp(mtl,  70, 6, 5)) \
 	fw_def(DG2,          0, guc_maj(dg2,  70, 5)) \
 	fw_def(ALDERLAKE_P,  0, guc_maj(adlp, 70, 5)) \
 	fw_def(ALDERLAKE_P,  0, guc_mmp(adlp, 70, 1, 1)) \