diff mbox series

[1/3] drm/msm: support firmware-name for zap fw

Message ID 20200108013847.899170-2-robdclark@gmail.com (mailing list archive)
State Superseded
Headers show
Series drm/msm: use firmware-name to find zap fw | expand

Commit Message

Rob Clark Jan. 8, 2020, 1:38 a.m. UTC
From: Rob Clark <robdclark@chromium.org>

Since zap firmware can be device specific, allow for a firmware-name
property in the zap node to specify which firmware to load, similarly to
the scheme used for dsp/wifi/etc.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/adreno/adreno_gpu.c | 32 ++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

Comments

Bjorn Andersson Jan. 8, 2020, 5 a.m. UTC | #1
On Tue 07 Jan 17:38 PST 2020, Rob Clark wrote:

> From: Rob Clark <robdclark@chromium.org>
> 
> Since zap firmware can be device specific, allow for a firmware-name
> property in the zap node to specify which firmware to load, similarly to
> the scheme used for dsp/wifi/etc.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 32 ++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 112e8b8a261e..aa8737bd58db 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -26,6 +26,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  {
>  	struct device *dev = &gpu->pdev->dev;
>  	const struct firmware *fw;
> +	const char *signed_fwname = NULL;
>  	struct device_node *np, *mem_np;
>  	struct resource r;
>  	phys_addr_t mem_phys;
> @@ -58,8 +59,33 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  
>  	mem_phys = r.start;
>  
> -	/* Request the MDT file for the firmware */
> -	fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> +	/*
> +	 * Check for a firmware-name property.  This is the new scheme
> +	 * to handle firmware that may be signed with device specific
> +	 * keys, allowing us to have a different zap fw path for different
> +	 * devices.
> +	 *
> +	 * If the firmware-name property is found, we bypass the
> +	 * adreno_request_fw() mechanism, because we don't need to handle
> +	 * the /lib/firmware/qcom/* vs /lib/firmware/* case.
> +	 *
> +	 * If the firmware-name property is not found, for backwards
> +	 * compatibility we fall back to the fwname from the gpulist
> +	 * table.
> +	 */
> +	of_property_read_string_index(np, "firmware-name", 0, &signed_fwname);
> +	if (signed_fwname) {
> +		fwname = signed_fwname;
> +		ret = request_firmware_direct(&fw, signed_fwname, gpu->dev->dev);
> +		if (ret) {
> +			DRM_DEV_ERROR(dev, "could not load signed zap firmware: %d\n", ret);
> +			fw = ERR_PTR(ret);

Perhaps just return ret; here, or omit the error print here any rely
solely on the one below?

> +		}
> +	} else {
> +		/* Request the MDT file for the firmware */

How about changing this to "Request the MDT file from the default
location" or something like that?

Rest looks good.

Regards,
Bjorn

> +		fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> +	}
> +
>  	if (IS_ERR(fw)) {
>  		DRM_DEV_ERROR(dev, "Unable to load %s\n", fwname);
>  		return PTR_ERR(fw);
> @@ -95,7 +121,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  	 * not.  But since we've already gotten through adreno_request_fw()
>  	 * we know which of the two cases it is:
>  	 */
> -	if (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY) {
> +	if (signed_fwname || (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY)) {
>  		ret = qcom_mdt_load(dev, fw, fwname, pasid,
>  				mem_region, mem_phys, mem_size, NULL);
>  	} else {
> -- 
> 2.24.1
>
Tom Rix Jan. 8, 2020, 3:38 p.m. UTC | #2
On 1/7/20 5:38 PM, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
>
> Since zap firmware can be device specific, allow for a firmware-name
> property in the zap node to specify which firmware to load, similarly to
> the scheme used for dsp/wifi/etc.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 32 ++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 112e8b8a261e..aa8737bd58db 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -26,6 +26,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  {
>  	struct device *dev = &gpu->pdev->dev;
>  	const struct firmware *fw;
> +	const char *signed_fwname = NULL;
>  	struct device_node *np, *mem_np;
>  	struct resource r;
>  	phys_addr_t mem_phys;
> @@ -58,8 +59,33 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  
>  	mem_phys = r.start;
>  
> -	/* Request the MDT file for the firmware */
> -	fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> +	/*
> +	 * Check for a firmware-name property.  This is the new scheme
> +	 * to handle firmware that may be signed with device specific
> +	 * keys, allowing us to have a different zap fw path for different
> +	 * devices.
> +	 *
> +	 * If the firmware-name property is found, we bypass the
> +	 * adreno_request_fw() mechanism, because we don't need to handle
> +	 * the /lib/firmware/qcom/* vs /lib/firmware/* case.
> +	 *
> +	 * If the firmware-name property is not found, for backwards
> +	 * compatibility we fall back to the fwname from the gpulist
> +	 * table.
> +	 */
> +	of_property_read_string_index(np, "firmware-name", 0, &signed_fwname);
> +	if (signed_fwname) {
> +		fwname = signed_fwname;
> +		ret = request_firmware_direct(&fw, signed_fwname, gpu->dev->dev);
> +		if (ret) {
> +			DRM_DEV_ERROR(dev, "could not load signed zap firmware: %d\n", ret);
Could adreno_request_fw be called with fwname if request_firmware_direct fails ?
> +			fw = ERR_PTR(ret);
> +		}
> +	} else {
> +		/* Request the MDT file for the firmware */
> +		fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> +	}
> +
>  	if (IS_ERR(fw)) {
>  		DRM_DEV_ERROR(dev, "Unable to load %s\n", fwname);
>  		return PTR_ERR(fw);
> @@ -95,7 +121,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  	 * not.  But since we've already gotten through adreno_request_fw()
>  	 * we know which of the two cases it is:
>  	 */
> -	if (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY) {
> +	if (signed_fwname || (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY)) {
>  		ret = qcom_mdt_load(dev, fw, fwname, pasid,
>  				mem_region, mem_phys, mem_size, NULL);
>  	} else {
Rob Clark Jan. 8, 2020, 4:30 p.m. UTC | #3
On Wed, Jan 8, 2020 at 7:38 AM Tom Rix <trix@redhat.com> wrote:
>
>
> On 1/7/20 5:38 PM, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > Since zap firmware can be device specific, allow for a firmware-name
> > property in the zap node to specify which firmware to load, similarly to
> > the scheme used for dsp/wifi/etc.
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 32 ++++++++++++++++++++++---
> >  1 file changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index 112e8b8a261e..aa8737bd58db 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -26,6 +26,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
> >  {
> >       struct device *dev = &gpu->pdev->dev;
> >       const struct firmware *fw;
> > +     const char *signed_fwname = NULL;
> >       struct device_node *np, *mem_np;
> >       struct resource r;
> >       phys_addr_t mem_phys;
> > @@ -58,8 +59,33 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
> >
> >       mem_phys = r.start;
> >
> > -     /* Request the MDT file for the firmware */
> > -     fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> > +     /*
> > +      * Check for a firmware-name property.  This is the new scheme
> > +      * to handle firmware that may be signed with device specific
> > +      * keys, allowing us to have a different zap fw path for different
> > +      * devices.
> > +      *
> > +      * If the firmware-name property is found, we bypass the
> > +      * adreno_request_fw() mechanism, because we don't need to handle
> > +      * the /lib/firmware/qcom/* vs /lib/firmware/* case.
> > +      *
> > +      * If the firmware-name property is not found, for backwards
> > +      * compatibility we fall back to the fwname from the gpulist
> > +      * table.
> > +      */
> > +     of_property_read_string_index(np, "firmware-name", 0, &signed_fwname);
> > +     if (signed_fwname) {
> > +             fwname = signed_fwname;
> > +             ret = request_firmware_direct(&fw, signed_fwname, gpu->dev->dev);
> > +             if (ret) {
> > +                     DRM_DEV_ERROR(dev, "could not load signed zap firmware: %d\n", ret);
> Could adreno_request_fw be called with fwname if request_firmware_direct fails ?


*possibly*.. initially I avoided this because the failure mode for
incorrectly signed firmware was silent and catestrophic.  But Bjorn
tells me this has been fixed.. in which case we could try and detect
if it is the incorrect fw.  I need to try some experiments to confirm
we can detect this case properly.

BR,
-R

> > +                     fw = ERR_PTR(ret);
> > +             }
> > +     } else {
> > +             /* Request the MDT file for the firmware */
> > +             fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> > +     }
> > +
> >       if (IS_ERR(fw)) {
> >               DRM_DEV_ERROR(dev, "Unable to load %s\n", fwname);
> >               return PTR_ERR(fw);
> > @@ -95,7 +121,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
> >        * not.  But since we've already gotten through adreno_request_fw()
> >        * we know which of the two cases it is:
> >        */
> > -     if (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY) {
> > +     if (signed_fwname || (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY)) {
> >               ret = qcom_mdt_load(dev, fw, fwname, pasid,
> >                               mem_region, mem_phys, mem_size, NULL);
> >       } else {
>
Jordan Crouse Jan. 8, 2020, 6:48 p.m. UTC | #4
On Tue, Jan 07, 2020 at 05:38:42PM -0800, Rob Clark wrote:
> From: Rob Clark <robdclark@chromium.org>
> 
> Since zap firmware can be device specific, allow for a firmware-name
> property in the zap node to specify which firmware to load, similarly to
> the scheme used for dsp/wifi/etc.
> 
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
>  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 32 ++++++++++++++++++++++---
>  1 file changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> index 112e8b8a261e..aa8737bd58db 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> @@ -26,6 +26,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  {
>  	struct device *dev = &gpu->pdev->dev;
>  	const struct firmware *fw;
> +	const char *signed_fwname = NULL;
>  	struct device_node *np, *mem_np;
>  	struct resource r;
>  	phys_addr_t mem_phys;
> @@ -58,8 +59,33 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  
>  	mem_phys = r.start;
>  
> -	/* Request the MDT file for the firmware */
> -	fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> +	/*
> +	 * Check for a firmware-name property.  This is the new scheme
> +	 * to handle firmware that may be signed with device specific
> +	 * keys, allowing us to have a different zap fw path for different
> +	 * devices.
> +	 *
> +	 * If the firmware-name property is found, we bypass the
> +	 * adreno_request_fw() mechanism, because we don't need to handle
> +	 * the /lib/firmware/qcom/* vs /lib/firmware/* case.
> +	 *
> +	 * If the firmware-name property is not found, for backwards
> +	 * compatibility we fall back to the fwname from the gpulist
> +	 * table.
> +	 */
> +	of_property_read_string_index(np, "firmware-name", 0, &signed_fwname);
> +	if (signed_fwname) {
> +		fwname = signed_fwname;
> +		ret = request_firmware_direct(&fw, signed_fwname, gpu->dev->dev);
> +		if (ret) {
> +			DRM_DEV_ERROR(dev, "could not load signed zap firmware: %d\n", ret);
> +			fw = ERR_PTR(ret);
> +		}
> +	} else {
> +		/* Request the MDT file for the firmware */
> +		fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> +	}
> +

Since DT seems to be the trend for target specific firmware names I think we
should plan to quickly deprecate the legacy name and not require new targets to
set it. If a zap node is going to be opt in then it isn't onerous to ask
the developer to set the additional property for each target platform.

Jordan

>  	if (IS_ERR(fw)) {
>  		DRM_DEV_ERROR(dev, "Unable to load %s\n", fwname);
>  		return PTR_ERR(fw);
> @@ -95,7 +121,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
>  	 * not.  But since we've already gotten through adreno_request_fw()
>  	 * we know which of the two cases it is:
>  	 */
> -	if (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY) {
> +	if (signed_fwname || (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY)) {
>  		ret = qcom_mdt_load(dev, fw, fwname, pasid,
>  				mem_region, mem_phys, mem_size, NULL);
>  	} else {
> -- 
> 2.24.1
>
Bjorn Andersson Jan. 8, 2020, 7:04 p.m. UTC | #5
On Wed 08 Jan 10:48 PST 2020, Jordan Crouse wrote:

> On Tue, Jan 07, 2020 at 05:38:42PM -0800, Rob Clark wrote:
> > From: Rob Clark <robdclark@chromium.org>
> > 
> > Since zap firmware can be device specific, allow for a firmware-name
> > property in the zap node to specify which firmware to load, similarly to
> > the scheme used for dsp/wifi/etc.
> > 
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > ---
> >  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 32 ++++++++++++++++++++++---
> >  1 file changed, 29 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > index 112e8b8a261e..aa8737bd58db 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > @@ -26,6 +26,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
> >  {
> >  	struct device *dev = &gpu->pdev->dev;
> >  	const struct firmware *fw;
> > +	const char *signed_fwname = NULL;
> >  	struct device_node *np, *mem_np;
> >  	struct resource r;
> >  	phys_addr_t mem_phys;
> > @@ -58,8 +59,33 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
> >  
> >  	mem_phys = r.start;
> >  
> > -	/* Request the MDT file for the firmware */
> > -	fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> > +	/*
> > +	 * Check for a firmware-name property.  This is the new scheme
> > +	 * to handle firmware that may be signed with device specific
> > +	 * keys, allowing us to have a different zap fw path for different
> > +	 * devices.
> > +	 *
> > +	 * If the firmware-name property is found, we bypass the
> > +	 * adreno_request_fw() mechanism, because we don't need to handle
> > +	 * the /lib/firmware/qcom/* vs /lib/firmware/* case.
> > +	 *
> > +	 * If the firmware-name property is not found, for backwards
> > +	 * compatibility we fall back to the fwname from the gpulist
> > +	 * table.
> > +	 */
> > +	of_property_read_string_index(np, "firmware-name", 0, &signed_fwname);
> > +	if (signed_fwname) {
> > +		fwname = signed_fwname;
> > +		ret = request_firmware_direct(&fw, signed_fwname, gpu->dev->dev);
> > +		if (ret) {
> > +			DRM_DEV_ERROR(dev, "could not load signed zap firmware: %d\n", ret);
> > +			fw = ERR_PTR(ret);
> > +		}
> > +	} else {
> > +		/* Request the MDT file for the firmware */
> > +		fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> > +	}
> > +
> 
> Since DT seems to be the trend for target specific firmware names I think we
> should plan to quickly deprecate the legacy name and not require new targets to
> set it. If a zap node is going to be opt in then it isn't onerous to ask
> the developer to set the additional property for each target platform.
> 

For the zap specifically I agree that it would be nice to require this
property, but for non-zap firmware it seems reasonable to continue with
the existing scheme.

Regards,
Bjorn
Rob Clark Jan. 12, 2020, 7:36 p.m. UTC | #6
On Wed, Jan 8, 2020 at 8:30 AM Rob Clark <robdclark@gmail.com> wrote:
>
> On Wed, Jan 8, 2020 at 7:38 AM Tom Rix <trix@redhat.com> wrote:
> >
> >
> > On 1/7/20 5:38 PM, Rob Clark wrote:
> > > From: Rob Clark <robdclark@chromium.org>
> > >
> > > Since zap firmware can be device specific, allow for a firmware-name
> > > property in the zap node to specify which firmware to load, similarly to
> > > the scheme used for dsp/wifi/etc.
> > >
> > > Signed-off-by: Rob Clark <robdclark@chromium.org>
> > > ---
> > >  drivers/gpu/drm/msm/adreno/adreno_gpu.c | 32 ++++++++++++++++++++++---
> > >  1 file changed, 29 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > index 112e8b8a261e..aa8737bd58db 100644
> > > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
> > > @@ -26,6 +26,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
> > >  {
> > >       struct device *dev = &gpu->pdev->dev;
> > >       const struct firmware *fw;
> > > +     const char *signed_fwname = NULL;
> > >       struct device_node *np, *mem_np;
> > >       struct resource r;
> > >       phys_addr_t mem_phys;
> > > @@ -58,8 +59,33 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
> > >
> > >       mem_phys = r.start;
> > >
> > > -     /* Request the MDT file for the firmware */
> > > -     fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> > > +     /*
> > > +      * Check for a firmware-name property.  This is the new scheme
> > > +      * to handle firmware that may be signed with device specific
> > > +      * keys, allowing us to have a different zap fw path for different
> > > +      * devices.
> > > +      *
> > > +      * If the firmware-name property is found, we bypass the
> > > +      * adreno_request_fw() mechanism, because we don't need to handle
> > > +      * the /lib/firmware/qcom/* vs /lib/firmware/* case.
> > > +      *
> > > +      * If the firmware-name property is not found, for backwards
> > > +      * compatibility we fall back to the fwname from the gpulist
> > > +      * table.
> > > +      */
> > > +     of_property_read_string_index(np, "firmware-name", 0, &signed_fwname);
> > > +     if (signed_fwname) {
> > > +             fwname = signed_fwname;
> > > +             ret = request_firmware_direct(&fw, signed_fwname, gpu->dev->dev);
> > > +             if (ret) {
> > > +                     DRM_DEV_ERROR(dev, "could not load signed zap firmware: %d\n", ret);
> > Could adreno_request_fw be called with fwname if request_firmware_direct fails ?
>
>
> *possibly*.. initially I avoided this because the failure mode for
> incorrectly signed firmware was silent and catestrophic.  But Bjorn
> tells me this has been fixed.. in which case we could try and detect
> if it is the incorrect fw.  I need to try some experiments to confirm
> we can detect this case properly.

I've been thinking about fallback to using fwname from the gpulist
table, but I think I don't really like the idea.

The failure mode for not falling back to $firmware/qcom/$fwname is
pretty safe.. you simply don't get gpu accel, but display and
everything else works, so you can boot up far enough to see what the
problem is and fix it.  But the failure mode if, with some device's
scm fw doesn't report incorrectly signed zap fw, is pretty
catastrophic (insta-reboot, which can be hard to debug on phone/laptop
without debug uart).

Since we haven't pushed the test-key signed qcom/sdm835/a630_zap.mbn
to linux-firmware yet, I think this is a good time to make this switch
for a6xx.

For a530 devices, maybe we hold of adding firmware-name to dt until
linux-firmware gains a qcom/msm8996/a530_zap.mbn (which would be a
good time to switch to the packed .mbn instead of split .mdt + .b0*
files).. or just not specify a firmware-name for device using test-key
signed zap and fallback to adreno_request_fw() and the existing path.

Fortunately zap is not used prior to a5xx, so we mainly just need to
care about backwards compat for a530.

BR,
-R

> BR,
> -R
>
> > > +                     fw = ERR_PTR(ret);
> > > +             }
> > > +     } else {
> > > +             /* Request the MDT file for the firmware */
> > > +             fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
> > > +     }
> > > +
> > >       if (IS_ERR(fw)) {
> > >               DRM_DEV_ERROR(dev, "Unable to load %s\n", fwname);
> > >               return PTR_ERR(fw);
> > > @@ -95,7 +121,7 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
> > >        * not.  But since we've already gotten through adreno_request_fw()
> > >        * we know which of the two cases it is:
> > >        */
> > > -     if (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY) {
> > > +     if (signed_fwname || (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY)) {
> > >               ret = qcom_mdt_load(dev, fw, fwname, pasid,
> > >                               mem_region, mem_phys, mem_size, NULL);
> > >       } else {
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index 112e8b8a261e..aa8737bd58db 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -26,6 +26,7 @@  static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
 {
 	struct device *dev = &gpu->pdev->dev;
 	const struct firmware *fw;
+	const char *signed_fwname = NULL;
 	struct device_node *np, *mem_np;
 	struct resource r;
 	phys_addr_t mem_phys;
@@ -58,8 +59,33 @@  static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
 
 	mem_phys = r.start;
 
-	/* Request the MDT file for the firmware */
-	fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
+	/*
+	 * Check for a firmware-name property.  This is the new scheme
+	 * to handle firmware that may be signed with device specific
+	 * keys, allowing us to have a different zap fw path for different
+	 * devices.
+	 *
+	 * If the firmware-name property is found, we bypass the
+	 * adreno_request_fw() mechanism, because we don't need to handle
+	 * the /lib/firmware/qcom/* vs /lib/firmware/* case.
+	 *
+	 * If the firmware-name property is not found, for backwards
+	 * compatibility we fall back to the fwname from the gpulist
+	 * table.
+	 */
+	of_property_read_string_index(np, "firmware-name", 0, &signed_fwname);
+	if (signed_fwname) {
+		fwname = signed_fwname;
+		ret = request_firmware_direct(&fw, signed_fwname, gpu->dev->dev);
+		if (ret) {
+			DRM_DEV_ERROR(dev, "could not load signed zap firmware: %d\n", ret);
+			fw = ERR_PTR(ret);
+		}
+	} else {
+		/* Request the MDT file for the firmware */
+		fw = adreno_request_fw(to_adreno_gpu(gpu), fwname);
+	}
+
 	if (IS_ERR(fw)) {
 		DRM_DEV_ERROR(dev, "Unable to load %s\n", fwname);
 		return PTR_ERR(fw);
@@ -95,7 +121,7 @@  static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname,
 	 * not.  But since we've already gotten through adreno_request_fw()
 	 * we know which of the two cases it is:
 	 */
-	if (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY) {
+	if (signed_fwname || (to_adreno_gpu(gpu)->fwloc == FW_LOCATION_LEGACY)) {
 		ret = qcom_mdt_load(dev, fw, fwname, pasid,
 				mem_region, mem_phys, mem_size, NULL);
 	} else {