drm/msm: use firmware-name to find zap fw
Subsystems (7)
v2 cover drm/msm: use firmware-name to find zap fw
From: Rob Clark <[email protected]>
For devices which use zap fw to take the GPU out of secure mode on reset, the firmware is likely to be signed with a device specific key. Meaning that we can't have a single filesystem (or /lib/firmware) that works on multiple devices.
So allow a firmware-name to be specified in the zap-shader node in dt. This moves the zap-shader node out of the core sdm845.dtsi and into per- device dts files. Which also removes the need for /delete-node/ in sdm845-cheza.dtsi (as cheza devices do not use zap).
This aligns with how Bjorn has been handling the similar situation with adsp/cdsp/mpss fw:
https://patchwork.kernel.org/patch/11160089/
Rob Clark (4):
drm/msm: support firmware-name for zap fw (v2) drm/msm: allow zapfw to not be specified in gpulist
dt-bindings: drm/msm/gpu: Document firmware-name
arm64: dts: sdm845: move gpu zap nodes to per-device dts
.../devicetree/bindings/display/msm/gpu.txt | 3 ++ arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 1 - arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 7 +++ arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 7 +++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 6 +-- .../boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 7 +++ drivers/gpu/drm/msm/adreno/adreno_gpu.c | 50 +++++++++++++++---- 7 files changed, 64 insertions(+), 17 deletions(-)
Signature · 6 lines
2.24.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 1/4 drm/msm: support firmware-name for zap fw (v2)
From: Rob Clark <[email protected]>
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.
v2: only need a single error msg when we can't load from firmware-name
specified path, and fix comment [Bjorn A.]
--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 30 ++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-)
drivers/gpu/drm/msm/adreno/adreno_gpu.c +27 -3
@@ -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,31 @@ 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, fwname, gpu->dev->dev); +if (ret) + fw =ERR_PTR (ret); + }else { +/* Request the MDT file from the default location: */ + 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 +119,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 {
Signature · 1 line
2.24.1
Re: [PATCH v2 1/4] drm/msm: support firmware-name for zap fw (v2)
Quoted 8 lines · Rob Clark
From: Rob Clark <[email protected]>
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.
v2: only need a single error msg when we can't load from firmware-name
specified path, and fix comment [Bjorn A.]
Quoted 63 lines
Signed-off-by Rob Clark <[email protected]>--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 30 ++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-)drivers/gpu/drm/msm/adreno/adreno_gpu.c +27
-3@@ -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,31 @@ 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, fwname, gpu->dev->dev); +if (ret) + fw =ERR_PTR (ret); + }else { +/* Request the MDT file from the default location: */ + 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 +119,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 {Signature · 1 line
2.24.1
Signature · 2 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
Re: [PATCH v2 1/4] drm/msm: support firmware-name for zap fw (v2)
In reply to Rob ClarkQuoted 9 lines · Rob Clark
From: Rob Clark <[email protected]>
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.
v2: only need a single error msg when we can't load from firmware-name
specified path, and fix comment [Bjorn A.]
Quoted 63 lines
Signed-off-by Rob Clark <[email protected]>--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 30 ++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-)drivers/gpu/drm/msm/adreno/adreno_gpu.c +27
-3@@ -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,31 @@ 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, fwname, gpu->dev->dev); +if (ret) + fw =ERR_PTR (ret); + }else { +/* Request the MDT file from the default location: */ + 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 +119,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 {Signature · 1 line
2.24.1
PATCH v2 2/4 drm/msm: allow zapfw to not be specified in gpulist
In reply to Rob ClarkFrom: Rob Clark <[email protected]>
For newer devices we want to require the path to come from the firmware-name property in the zap-shader dt node.
--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)
drivers/gpu/drm/msm/adreno/adreno_gpu.c +13 -9
@@ -79,9 +79,21 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname, ret =request_firmware_direct (&fw, fwname, gpu->dev->dev);if (ret) fw =ERR_PTR (ret); - }else { + }else if (fwname) {/* Request the MDT file from the default location: */ fw =adreno_request_fw (to_adreno_gpu (gpu), fwname); + }else { +/* +* For new targets, we require the firmware-name property, +* if a zap-shader is required, rather than falling back +* to a firmware name specified in gpulist. +* +* Because the firmware is signed with a (potentially) +* device specific key, having the name come from gpulist +* was a bad idea, and is only provided for backwards +* compatibility for older targets. +*/ +return -ENODEV ; }if (IS_ERR (fw)) {
@@ -170,14 +182,6 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, u32 pasid)return -EPROBE_DEFER ; } -/* Each GPU has a target specific zap shader firmware name to use */ -if (!adreno_gpu->info->zapfw) { - zap_available = false; -DRM_DEV_ERROR (&pdev->dev, -"Zap shader firmware file not specified for this target\n" ); -return -ENODEV ; - } -return zap_shader_load_mdt (gpu, adreno_gpu->info->zapfw, pasid); }
Signature · 6 lines
2.24.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/4] drm/msm: allow zapfw to not be specified in gpulist
Quoted 4 lines · Rob Clark
From: Rob Clark <[email protected]>
For newer devices we want to require the path to come from the firmware-name property in the zap-shader dt node.
Quoted 20 lines
Signed-off-by Rob Clark <[email protected]>--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)drivers/gpu/drm/msm/adreno/adreno_gpu.c +4
-1@@ -79,9 +79,21 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname, ret =request_firmware_direct (&fw, fwname, gpu->dev->dev);if (ret) fw =ERR_PTR (ret); - }else { + }else if (fwname) {/* Request the MDT file from the default location: */ fw =adreno_request_fw (to_adreno_gpu (gpu), fwname); + }else { +/* +* For new targets, we require the firmware-name property,
drivers/gpu/drm/msm/adreno/adreno_gpu.c static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname, · line 97
Quoted 10 lines
drivers/gpu/drm/msm/adreno/adreno_gpu.c +9
-0+* if a zap-shader is required, rather than falling back +* to a firmware name specified in gpulist. +* +* Because the firmware is signed with a (potentially) +* device specific key, having the name come from gpulist +* was a bad idea, and is only provided for backwards +* compatibility for older targets. +*/ +return -ENODEV ; }
A possible future optimization would be to move a lot of this to the target specific code but we can do that once we're sure that all the rest of the fallout has bee militated.
Quoted 20 lines
drivers/gpu/drm/msm/adreno/adreno_gpu.c +0
-8if (IS_ERR (fw)) {@@ -170,14 +182,6 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, u32 pasid)return -EPROBE_DEFER ; } -/* Each GPU has a target specific zap shader firmware name to use */ -if (!adreno_gpu->info->zapfw) { - zap_available = false; -DRM_DEV_ERROR (&pdev->dev, -"Zap shader firmware file not specified for this target\n" ); -return -ENODEV ; - } -return zap_shader_load_mdt (gpu, adreno_gpu->info->zapfw, pasid); }Signature · 1 line
2.24.1
Signature · 6 lines
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/4] drm/msm: allow zapfw to not be specified in gpulist
In reply to Rob ClarkQuoted 5 lines · Rob Clark
From: Rob Clark <[email protected]>
For newer devices we want to require the path to come from the firmware-name property in the zap-shader dt node.
Quoted 50 lines
Signed-off-by Rob Clark <[email protected]>--- drivers/gpu/drm/msm/adreno/adreno_gpu.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)drivers/gpu/drm/msm/adreno/adreno_gpu.c +13
-9@@ -79,9 +79,21 @@ static int zap_shader_load_mdt(struct msm_gpu *gpu, const char *fwname, ret =request_firmware_direct (&fw, fwname, gpu->dev->dev);if (ret) fw =ERR_PTR (ret); - }else { + }else if (fwname) {/* Request the MDT file from the default location: */ fw =adreno_request_fw (to_adreno_gpu (gpu), fwname); + }else { +/* +* For new targets, we require the firmware-name property, +* if a zap-shader is required, rather than falling back +* to a firmware name specified in gpulist. +* +* Because the firmware is signed with a (potentially) +* device specific key, having the name come from gpulist +* was a bad idea, and is only provided for backwards +* compatibility for older targets. +*/ +return -ENODEV ; }if (IS_ERR (fw)) {@@ -170,14 +182,6 @@ int adreno_zap_shader_load(struct msm_gpu *gpu, u32 pasid)return -EPROBE_DEFER ; } -/* Each GPU has a target specific zap shader firmware name to use */ -if (!adreno_gpu->info->zapfw) { - zap_available = false; -DRM_DEV_ERROR (&pdev->dev, -"Zap shader firmware file not specified for this target\n" ); -return -ENODEV ; - } -return zap_shader_load_mdt (gpu, adreno_gpu->info->zapfw, pasid); }Signature · 1 line
2.24.1
PATCH v2 3/4 dt-bindings: drm/msm/gpu: Document firmware-name
In reply to Rob ClarkFrom: Rob Clark <[email protected]>
The firmware-name property in the zap node can be used to specify a device specific zap firmware.
--- Documentation/devicetree/bindings/display/msm/gpu.txt | 3 +++ 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/display/msm/gpu.txt +3 -0
@@ -33,6 +33,8 @@ Required properties: - zap-shader: For a5xx and a6xx devices this node contains a memory-region that points to reserved memory to store the zap shader that can be used to help bring the GPU out of secure mode. +- firmware-name: optional property of the 'zap-shader' node, listing the + relative path of the device specific zap firmware. Example 3xx/4xx/a5xx:
@@ -85,6 +87,7 @@ Example a6xx (with GMU): zap-shader { memory-region = <&zap_shader_region>; + firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn" }; }; };
Signature · 6 lines
2.24.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH v2 4/4 arm64: dts: sdm845: move gpu zap nodes to per-device dts
In reply to Rob ClarkFrom: Rob Clark <[email protected]>
We want to specify per-device firmware-name, so move the zap node into the .dts file for individual boards/devices. This lets us get rid of the /delete-node/ for cheza, which does not use zap.
---
v2: use 'sdm845' for subdir for devices that use test-key signed fw
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 1 - arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 7 +++++++ arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 7 +++++++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 6 +----- arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 7 +++++++ 5 files changed, 22 insertions(+), 6 deletions(-)
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi +0 -1
@@ -165,7 +165,6 @@ panel_in_edp: endpoint { /delete-node/ &venus_mem; /delete-node/ &cdsp_mem; /delete-node/ &cdsp_pas; -/delete-node/ &zap_shader; /delete-node/ &gpu_mem; /* Increase the size from 120 MB to 128 MB */
arch/arm64/boot/dts/qcom/sdm845-db845c.dts +7 -0
@@ -352,6 +352,13 @@ &gcc { <GCC_QSPI_CNOC_PERIPH_AHB_CLK>; }; +&gpu { + zap-shader { + memory-region = <&gpu_mem>; + firmware-name = "qcom/sdm845/a630_zap.mbn"; + }; +}; + &pm8998_gpio { vol_up_pin_a: vol-up-active { pins = "gpio6";
arch/arm64/boot/dts/qcom/sdm845-mtp.dts +7 -0
@@ -360,6 +360,13 @@ &gcc { <GCC_LPASS_SWAY_CLK>; }; +&gpu { + zap-shader { + memory-region = <&gpu_mem>; + firmware-name = "qcom/sdm845/a630_zap.mbn"; + }; +}; + &i2c10 { status = "okay"; clock-frequency = <400000>;
arch/arm64/boot/dts/qcom/sdm845.dtsi +1 -5
@@ -2804,7 +2804,7 @@ dsi1_phy: dsi-phy@ae96400 { }; }; - gpu@5000000 { + gpu: gpu@5000000 { compatible = "qcom,adreno-630.2", "qcom,adreno"; #stream-id-cells = <16>;
@@ -2824,10 +2824,6 @@ gpu@5000000 { qcom,gmu = <&gmu>; - zap_shader: zap-shader { - memory-region = <&gpu_mem>; - }; - gpu_opp_table: opp-table { compatible = "operating-points-v2";
arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts +7 -0
@@ -245,6 +245,13 @@ &gcc { <GCC_QSPI_CNOC_PERIPH_AHB_CLK>; }; +&gpu { + zap-shader { + memory-region = <&gpu_mem>; + firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn"; + }; +}; + &i2c1 { status = "okay"; clock-frequency = <400000>;
Signature · 6 lines
2.24.1 _______________________________________________ dri-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/4] arm64: dts: sdm845: move gpu zap nodes to per-device dts
Quoted 6 lines · Rob Clark
From: Rob Clark <[email protected]>
We want to specify per-device firmware-name, so move the zap node into the .dts file for individual boards/devices. This lets us get rid of the /delete-node/ for cheza, which does not use zap.
Quoted 104 lines
Signed-off-by Rob Clark <[email protected]>---
v2: use 'sdm845' for subdir for devices that use test-key signed fwarch/arm64/boot/dts/qcom/sdm845-cheza.dtsi | 1 - arch/arm64/boot/dts/qcom/sdm845-db845c.dts | 7 +++++++ arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 7 +++++++ arch/arm64/boot/dts/qcom/sdm845.dtsi | 6 +----- arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts | 7 +++++++ 5 files changed, 22 insertions(+), 6 deletions(-)arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi +0
-1@@ -165,7 +165,6 @@ panel_in_edp: endpoint { /delete-node/ &venus_mem; /delete-node/ &cdsp_mem; /delete-node/ &cdsp_pas; -/delete-node/ &zap_shader; /delete-node/ &gpu_mem; /* Increase the size from 120 MB to 128 MB */arch/arm64/boot/dts/qcom/sdm845-db845c.dts +7
-0@@ -352,6 +352,13 @@ &gcc { <GCC_QSPI_CNOC_PERIPH_AHB_CLK>; }; +&gpu { + zap-shader { + memory-region = <&gpu_mem>; + firmware-name = "qcom/sdm845/a630_zap.mbn"; + }; +}; + &pm8998_gpio { vol_up_pin_a: vol-up-active { pins = "gpio6";arch/arm64/boot/dts/qcom/sdm845-mtp.dts +7
-0@@ -360,6 +360,13 @@ &gcc { <GCC_LPASS_SWAY_CLK>; }; +&gpu { + zap-shader { + memory-region = <&gpu_mem>; + firmware-name = "qcom/sdm845/a630_zap.mbn"; + }; +}; + &i2c10 { status = "okay"; clock-frequency = <400000>;arch/arm64/boot/dts/qcom/sdm845.dtsi +1
-5@@ -2804,7 +2804,7 @@ dsi1_phy: dsi-phy@ae96400 { }; }; - gpu@5000000 { + gpu: gpu@5000000 { compatible = "qcom,adreno-630.2", "qcom,adreno"; #stream-id-cells = <16>;@@ -2824,10 +2824,6 @@ gpu@5000000 { qcom,gmu = <&gmu>; - zap_shader: zap-shader { - memory-region = <&gpu_mem>; - }; - gpu_opp_table: opp-table { compatible = "operating-points-v2";arch/arm64/boot/dts/qcom/sdm850-lenovo-yoga-c630.dts +7
-0@@ -245,6 +245,13 @@ &gcc { <GCC_QSPI_CNOC_PERIPH_AHB_CLK>; }; +&gpu { + zap-shader { + memory-region = <&gpu_mem>; + firmware-name = "qcom/LENOVO/81JL/qcdxkmsuc850.mbn"; + }; +}; + &i2c1 { status = "okay"; clock-frequency = <400000>;Signature · 1 line
2.24.1