[PATCH v2] drm/msm/dpu: Fix memory leak in msm_mdss_parse_data_bus_icc_path
of_icc_get() alloc resources for path1, we should release it when not
need anymore. Early return when IS_ERR_OR_NULL(path0) may leak path1.
Defer getting path1 to fix this.
Fixes: b9364eed9232 ("drm/msm/dpu: Move min BW request and full BW disable back
to mdss")
Signed-off-by: Miaoqian Lin
---
changes in v2:
- move getting path1 after error check for path0.
---
drivers/gpu/drm/msm/msm_mdss.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index e13c5c12b775..3b8d6991b04e 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -46,15 +46,17 @@ struct msm_mdss {
static int msm_mdss_parse_data_bus_icc_path(struct device *dev,
struct msm_mdss *msm_mdss)
{
- struct icc_path *path0 = of_icc_get(dev, "mdp0-mem");
- struct icc_path *path1 = of_icc_get(dev, "mdp1-mem");
+ struct icc_path *path0;
+ struct icc_path *path1;
+ path0 = of_icc_get(dev, "mdp0-mem");
if (IS_ERR_OR_NULL(path0))
return PTR_ERR_OR_ZERO(path0);
msm_mdss->path[0] = path0;
msm_mdss->num_paths = 1;
+ path1 = of_icc_get(dev, "mdp1-mem");
if (!IS_ERR_OR_NULL(path1)) {
msm_mdss->path[1] = path1;
msm_mdss->num_paths++;
--
2.25.1
Re: [PATCH 4/5] drm/rockchip: vop2: add support for the rgb output block
On Wed, Nov 30, 2022 at 03:02:16PM +0100, Michael Riesch wrote:
> The Rockchip VOP2 features an internal RGB output block, which can be
> attached to the video port 2 of the VOP2. Add support for this output
> block.
>
> Signed-off-by: Michael Riesch
> ---
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 21
> 1 file changed, 21 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> index 94fddbf70ff6..16041c79d228 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> @@ -39,6 +39,7 @@
> #include "rockchip_drm_gem.h"
> #include "rockchip_drm_fb.h"
> #include "rockchip_drm_vop2.h"
> +#include "rockchip_rgb.h"
>
> /*
> * VOP2 architecture
> @@ -212,6 +213,9 @@ struct vop2 {
> struct clk *hclk;
> struct clk *aclk;
>
> + /* optional internal rgb encoder */
> + struct rockchip_rgb *rgb;
> +
> /* must be put at the end of the struct */
> struct vop2_win win[];
> };
> @@ -2697,11 +2701,25 @@ static int vop2_bind(struct device *dev, struct
> device *master, void *data)
> if (ret)
> return ret;
>
> + vop2->rgb = rockchip_rgb_init(dev, >vps[2].crtc, vop2->drm, 2);
Here you assume that the RGB output can only be connected to VP2, but it
could be connected to any other VP as well, and we can find the
description where it actually shall be connected in the device tree.
As mentioned in my comment to patch 1, the question is "Is there
something connected to VPx at endpoint ROCKCHIP_VOP2_EP_RGB0?"
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
Re: [PATCH 1/5] drm/rockchip: rgb: embed drm_encoder into rockchip_encoder
On Wed, Nov 30, 2022 at 03:02:13PM +0100, Michael Riesch wrote:
> Commit 540b8f271e53 ("drm/rockchip: Embed drm_encoder into
> rockchip_decoder") provides the means to pass the endpoint ID to the
> VOP2 driver, which sets the interface MUX accordingly. However, this
> step has not yet been carried out for the RGB output block. Embed the
> drm_encoder structure into the rockchip_encoder structure and set the
> endpoint ID correctly.
>
> Signed-off-by: Michael Riesch
> ---
> drivers/gpu/drm/rockchip/rockchip_rgb.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c
> b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> index 75eb7cca3d82..16201a5cf1e8 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
> @@ -18,17 +18,17 @@
> #include
> #include
>
> +#include
> +
> #include "rockchip_drm_drv.h"
> #include "rockchip_drm_vop.h"
> #include "rockchip_rgb.h"
>
> -#define encoder_to_rgb(c) container_of(c, struct rockchip_rgb, encoder)
> -
> struct rockchip_rgb {
> struct device *dev;
> struct drm_device *drm_dev;
> struct drm_bridge *bridge;
> - struct drm_encoder encoder;
> + struct rockchip_encoder encoder;
> struct drm_connector connector;
> int output_mode;
> };
> @@ -125,7 +125,7 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
> return ERR_PTR(ret);
> }
>
> - encoder = >encoder;
> + encoder = >encoder.encoder;
> encoder->possible_crtcs = drm_crtc_mask(crtc);
>
> ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_NONE);
> @@ -161,6 +161,8 @@ struct rockchip_rgb *rockchip_rgb_init(struct device *dev,
> goto err_free_encoder;
> }
>
> + rgb->encoder.crtc_endpoint_id = ROCKCHIP_VOP2_EP_RGB0;
This is vop2 specific. This code is used with the vop1 as well, so it
doesn't look like it belongs here, at least not hidden in a patch titled
"embed drm_encoder into rockchip_encoder".
Normally the crtc_endpoint_id is set by the encoder, coming from the
encoder node, asking the question "To which port on the VOP am I
connected to?"
Here the situation is different. We are called from the VOP and the
question instead is: "Is there something connected to VPx endpoint id
ROCKCHIP_VOP2_EP_RGB0?"
You might need a vop2 specific entry to this code.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
[PATCH v4 2/5] arm64: dts: qcom: sm8450: add display hardware devices
Add devices tree nodes describing display hardware on SM8450:
- Display Clock Controller
- MDSS
- MDP
- two DSI controllers and DSI PHYs
This does not provide support for DP controllers present on SM8450.
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 283 ++-
1 file changed, 279 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi
b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 8cc9f62f7645..3a3819852eae 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -2394,6 +2394,281 @@ camcc: clock-controller@ade {
status = "disabled";
};
+ mdss: display-subsystem@ae0 {
+ compatible = "qcom,sm8450-mdss";
+ reg = <0 0x0ae0 0 0x1000>;
+ reg-names = "mdss";
+
+ /* same path used twice */
+ interconnects = <_noc MASTER_MDP_DISP 0 _virt
SLAVE_EBI1_DISP 0>,
+ <_noc MASTER_MDP_DISP 0 _virt
SLAVE_EBI1_DISP 0>;
+ interconnect-names = "mdp0-mem", "mdp1-mem";
+
+ resets = < DISP_CC_MDSS_CORE_BCR>;
+
+ power-domains = < MDSS_GDSC>;
+
+ clocks = < DISP_CC_MDSS_AHB_CLK>,
+< GCC_DISP_HF_AXI_CLK>,
+< GCC_DISP_SF_AXI_CLK>,
+< DISP_CC_MDSS_MDP_CLK>;
+
+ interrupts = ;
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ iommus = <_smmu 0x2800 0x402>;
+
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ status = "disabled";
+
+ mdss_mdp: display-controller@ae01000 {
+ compatible = "qcom,sm8450-dpu";
+ reg = <0 0x0ae01000 0 0x8f000>,
+ <0 0x0aeb 0 0x2008>;
+ reg-names = "mdp", "vbif";
+
+ clocks = < GCC_DISP_HF_AXI_CLK>,
+ < GCC_DISP_SF_AXI_CLK>,
+ < DISP_CC_MDSS_AHB_CLK>,
+ < DISP_CC_MDSS_MDP_LUT_CLK>,
+ < DISP_CC_MDSS_MDP_CLK>,
+ < DISP_CC_MDSS_VSYNC_CLK>;
+ clock-names = "bus",
+ "nrt_bus",
+ "iface",
+ "lut",
+ "core",
+ "vsync";
+
+ assigned-clocks = <
DISP_CC_MDSS_VSYNC_CLK>;
+ assigned-clock-rates = <1920>;
+
+ operating-points-v2 = <_opp_table>;
+ power-domains = < SM8450_MMCX>;
+
+ interrupt-parent = <>;
+ interrupts = <0>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dpu_intf1_out: endpoint {
+ remote-endpoint =
<_dsi0_in>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ dpu_intf2_out: endpoint {
+ remote-endpoint =
<_dsi1_in>;
+ };
+ };
+
+ };
+
+ mdp_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-17200 {
+ opp-hz = /bits/ 64 <17200>;
+ required-opps =
<_opp_low_svs_d1>;
+ };
+
+ opp-2 {
+ opp-hz = /bits/ 64 <2>;
+ required-opps =
<_opp_low_svs>;
+ };
+
+
[PATCH v4 5/5] arm64: dts: qcom: sm8450-hdk: Enable HDMI Display
From: Vinod Koul
Add the HDMI display nodes and link it to DSI.
Signed-off-by: Vinod Koul
Reviewed-by: Krzysztof Kozlowski
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sm8450-hdk.dts | 37 +
1 file changed, 37 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
index 4f345786352a..166458963c2f 100644
--- a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
+++ b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
@@ -20,6 +20,17 @@ chosen {
stdout-path = "serial0:115200n8";
};
+ hdmi-out {
+ compatible = "hdmi-connector";
+ type = "a";
+
+ port {
+ hdmi_connector_out: endpoint {
+ remote-endpoint = <_out>;
+ };
+ };
+ };
+
lt9611_1v2: lt9611-vdd12-regulator {
compatible = "regulator-fixed";
regulator-name = "LT9611_1V2";
@@ -392,6 +403,27 @@ lt9611_codec: hdmi-bridge@2b {
pinctrl-names = "default";
pinctrl-0 = <_irq_pin _rst_pin>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ lt9611_a: endpoint {
+ remote-endpoint = <_dsi0_out>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+
+ lt9611_out: endpoint {
+ remote-endpoint = <_connector_out>;
+ };
+ };
+ };
};
};
@@ -404,6 +436,11 @@ _dsi0 {
status = "okay";
};
+_dsi0_out {
+ remote-endpoint = <_a>;
+ data-lanes = <0 1 2 3>;
+};
+
_dsi0_phy {
vdds-supply = <_l5b_0p88>;
status = "okay";
--
2.35.1
[PATCH v4 3/5] arm64: dts: qcom: sm8450-hdk: enable display hardware
Enable MDSS/DPU/DSI0 on SM8450-HDK device.
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sm8450-hdk.dts | 22 ++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
index 2dd4f8c8f931..75b7aecb7d8e 100644
--- a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
+++ b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
@@ -349,6 +349,28 @@ vreg_l7e_2p8: ldo7 {
};
};
+ {
+ status = "okay";
+};
+
+ {
+ status = "okay";
+};
+
+_dsi0 {
+ vdda-supply = <_l6b_1p2>;
+ status = "okay";
+};
+
+_dsi0_phy {
+ vdds-supply = <_l5b_0p88>;
+ status = "okay";
+};
+
+_mdp {
+ status = "okay";
+};
+
{
status = "okay";
max-link-speed = <2>;
--
2.35.1
[PATCH v4 4/5] arm64: dts: qcom: sm8450-hdk: Add LT9611uxc HDMI bridge
From: Vinod Koul
Add the LT9611uxc DSI-HDMI bridge and supplies
Signed-off-by: Vinod Koul
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sm8450-hdk.dts | 58 +
1 file changed, 58 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
index 75b7aecb7d8e..4f345786352a 100644
--- a/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
+++ b/arch/arm64/boot/dts/qcom/sm8450-hdk.dts
@@ -20,6 +20,28 @@ chosen {
stdout-path = "serial0:115200n8";
};
+ lt9611_1v2: lt9611-vdd12-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "LT9611_1V2";
+
+ vin-supply = <_pwr>;
+ regulator-min-microvolt = <120>;
+ regulator-max-microvolt = <120>;
+ gpio = < 9 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ lt9611_3v3: lt9611-3v3-regulator {
+ compatible = "regulator-fixed";
+ regulator-name = "LT9611_3V3";
+
+ vin-supply = <_bob>;
+ gpio = < 109 GPIO_ACTIVE_HIGH>;
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+ enable-active-high;
+ };
+
vph_pwr: vph-pwr-regulator {
compatible = "regulator-fixed";
regulator-name = "vph_pwr";
@@ -353,6 +375,26 @@ {
status = "okay";
};
+ {
+ clock-frequency = <40>;
+ status = "okay";
+
+ lt9611_codec: hdmi-bridge@2b {
+ compatible = "lontium,lt9611uxc";
+ reg = <0x2b>;
+
+ interrupts-extended = < 44 IRQ_TYPE_EDGE_FALLING>;
+
+ reset-gpios = < 107 GPIO_ACTIVE_HIGH>;
+
+ vdd-supply = <_1v2>;
+ vcc-supply = <_3v3>;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <_irq_pin _rst_pin>;
+ };
+};
+
{
status = "okay";
};
@@ -416,6 +458,10 @@ _id_0 {
status = "okay";
};
+_id_1 {
+ status = "okay";
+};
+
_2 {
cd-gpios = < 92 GPIO_ACTIVE_HIGH>;
pinctrl-names = "default", "sleep";
@@ -431,6 +477,18 @@ _2 {
{
gpio-reserved-ranges = <28 4>, <36 4>;
+ lt9611_irq_pin: lt9611-irq-state {
+ pins = "gpio44";
+ function = "gpio";
+ bias-disable;
+ };
+
+ lt9611_rst_pin: lt9611-rst-state {
+ pins = "gpio107";
+ function = "gpio";
+ output-high;
+ };
+
sdc2_card_det_n: sd-card-det-n-state {
pins = "gpio92";
function = "gpio";
--
2.35.1
[PATCH v4 1/5] arm64: dts: qcom: sm8450: add RPMH_REGULATOR_LEVEL_LOW_SVS_D1
Add another power saving state used on SM8450. Unfortunately adding it
in proper place causes renumbering of all the opp states in sm8450.dtsi
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 20
include/dt-bindings/power/qcom-rpmpd.h | 1 +
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi
b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index f20db5456765..8cc9f62f7645 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -3211,35 +3211,39 @@ rpmhpd_opp_min_svs: opp2 {
opp-level =
;
};
- rpmhpd_opp_low_svs: opp3 {
+ rpmhpd_opp_low_svs_d1: opp3 {
+ opp-level =
;
+ };
+
+ rpmhpd_opp_low_svs: opp4 {
opp-level =
;
};
- rpmhpd_opp_svs: opp4 {
+ rpmhpd_opp_svs: opp5 {
opp-level =
;
};
- rpmhpd_opp_svs_l1: opp5 {
+ rpmhpd_opp_svs_l1: opp6 {
opp-level =
;
};
- rpmhpd_opp_nom: opp6 {
+ rpmhpd_opp_nom: opp7 {
opp-level =
;
};
- rpmhpd_opp_nom_l1: opp7 {
+ rpmhpd_opp_nom_l1: opp8 {
opp-level =
;
};
- rpmhpd_opp_nom_l2: opp8 {
+ rpmhpd_opp_nom_l2: opp9 {
opp-level =
;
};
- rpmhpd_opp_turbo: opp9 {
+ rpmhpd_opp_turbo: opp10 {
opp-level =
;
};
- rpmhpd_opp_turbo_l1: opp10 {
+ rpmhpd_opp_turbo_l1: opp11 {
opp-level =
;
};
};
diff --git a/include/dt-bindings/power/qcom-rpmpd.h
b/include/dt-bindings/power/qcom-rpmpd.h
index 7b2e4b66419a..701401c8b945 100644
--- a/include/dt-bindings/power/qcom-rpmpd.h
+++ b/include/dt-bindings/power/qcom-rpmpd.h
@@ -174,6 +174,7 @@
/* SDM845 Power Domain performance levels */
#define RPMH_REGULATOR_LEVEL_RETENTION 16
#define RPMH_REGULATOR_LEVEL_MIN_SVS 48
+#define RPMH_REGULATOR_LEVEL_LOW_SVS_D156
#define RPMH_REGULATOR_LEVEL_LOW_SVS 64
#define RPMH_REGULATOR_LEVEL_SVS 128
#define RPMH_REGULATOR_LEVEL_SVS_L0144
--
2.35.1
[PATCH v4 0/5] arm64: dts: qcom: sm8450-hdk: enable HDMI output
Add device tree nodes for MDSS, DPU and DSI devices on Qualcomm SM8450 platform. Enable these devices and add the HDMI bridge configuration on SM8450 HDK. Changes since v3: - Renamed mdss node to display-subsystem@ (Krzysztof) - Dropped empty line from the patch4 (Krzysztof) - Renamed HDMI connector endpoint to hdmi_connector_out Changes since v2: - Dropped clock-names from mdss device node - Fixed pinctrl configuration used by lt9611uxc (Krzysztof) Changes since v1: - Reorder properties, making status the last one - Rename opp nodes to follow the schema - Renamed display-controller and phy device nodes - Dropped phy-names for DSI PHYs - Renamed DSI and DSI PHY labels to include mdss_ prefix - Renamed 3v3 regulator device node to add -regulator suffix Dmitry Baryshkov (3): arm64: dts: qcom: sm8450: add RPMH_REGULATOR_LEVEL_LOW_SVS_D1 arm64: dts: qcom: sm8450: add display hardware devices arm64: dts: qcom: sm8450-hdk: enable display hardware Vinod Koul (2): arm64: dts: qcom: sm8450-hdk: Add LT9611uxc HDMI bridge arm64: dts: qcom: sm8450-hdk: Enable HDMI Display arch/arm64/boot/dts/qcom/sm8450-hdk.dts | 117 + arch/arm64/boot/dts/qcom/sm8450.dtsi| 303 +++- include/dt-bindings/power/qcom-rpmpd.h | 1 + 3 files changed, 409 insertions(+), 12 deletions(-) -- 2.35.1
[PATCH v6 09/11] drm/msm/dpu: add support for MDP_TOP blackhole
On sm8450 a register block was removed from MDP TOP. Accessing it during
snapshotting results in NoC errors / immediate reboot. Skip accessing
these registers during snapshot.
Tested-by: Vinod Koul
Reviewed-by: Vinod Koul
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 3 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 3 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 11 +--
3 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 3b645d5aa9aa..a9d161daf786 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -83,6 +83,8 @@ enum {
* @DPU_MDP_UBWC_1_0, This chipsets supports Universal Bandwidth
* compression initial revision
* @DPU_MDP_UBWC_1_5, Universal Bandwidth compression version 1.5
+ * @DPU_MDP_PERIPH_0_REMOVED Indicates that access to periph top0 block results
+ *in a failure
* @DPU_MDP_MAXMaximum value
*/
@@ -93,6 +95,7 @@ enum {
DPU_MDP_UBWC_1_0,
DPU_MDP_UBWC_1_5,
DPU_MDP_AUDIO_SELECT,
+ DPU_MDP_PERIPH_0_REMOVED,
DPU_MDP_MAX
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
index 86c423e63b61..feb9a729844a 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
@@ -63,4 +63,7 @@
#define MDP_WD_TIMER_4_LOAD_VALUE 0x448
#define DCE_SEL 0x450
+#define MDP_PERIPH_TOP0MDP_WD_TIMER_0_CTL
+#define MDP_PERIPH_TOP0_ENDCLK_CTRL3
+
#endif /*_DPU_HWIO_H */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index b71199511a52..987a74fb7fad 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -927,8 +927,15 @@ static void dpu_kms_mdp_snapshot(struct msm_disp_state
*disp_state, struct msm_k
msm_disp_snapshot_add_block(disp_state, cat->wb[i].len,
dpu_kms->mmio + cat->wb[i].base, "wb_%d", i);
- msm_disp_snapshot_add_block(disp_state, cat->mdp[0].len,
- dpu_kms->mmio + cat->mdp[0].base, "top");
+ if (cat->mdp[0].features & BIT(DPU_MDP_PERIPH_0_REMOVED)) {
+ msm_disp_snapshot_add_block(disp_state, MDP_PERIPH_TOP0,
+ dpu_kms->mmio + cat->mdp[0].base, "top");
+ msm_disp_snapshot_add_block(disp_state, cat->mdp[0].len -
MDP_PERIPH_TOP0_END,
+ dpu_kms->mmio + cat->mdp[0].base +
MDP_PERIPH_TOP0_END, "top_2");
+ } else {
+ msm_disp_snapshot_add_block(disp_state, cat->mdp[0].len,
+ dpu_kms->mmio + cat->mdp[0].base, "top");
+ }
pm_runtime_put_sync(_kms->pdev->dev);
}
--
2.35.1
[PATCH v6 11/11] drm/msm: mdss add support for SM8450
Add support for the MDSS block on SM8450 platform.
Tested-by: Vinod Koul
Reviewed-by: Vinod Koul
Reviewed-by: Konrad Dybcio
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/msm_mdss.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 86b28add1fff..144c8dd82be1 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -287,6 +287,10 @@ static int msm_mdss_enable(struct msm_mdss *msm_mdss)
case DPU_HW_VER_720:
msm_mdss_setup_ubwc_dec_40(msm_mdss, UBWC_3_0, 6, 1, 1, 1);
break;
+ case DPU_HW_VER_810:
+ /* TODO: highest_bank_bit = 2 for LP_DDR4 */
+ msm_mdss_setup_ubwc_dec_40(msm_mdss, UBWC_4_0, 6, 1, 3, 1);
+ break;
}
return ret;
@@ -516,6 +520,7 @@ static const struct of_device_id mdss_dt_match[] = {
{ .compatible = "qcom,sm6115-mdss" },
{ .compatible = "qcom,sm8150-mdss" },
{ .compatible = "qcom,sm8250-mdss" },
+ { .compatible = "qcom,sm8450-mdss" },
{}
};
MODULE_DEVICE_TABLE(of, mdss_dt_match);
--
2.35.1
[PATCH v6 08/11] drm/msm/dpu: merge all MDP TOP registers to dpu_hwio.h
There is a separate header containing some of MDP TOP register
definitions, dpu_hwio.h. Move missing register definitions from
dpu_hw_top.c to the mentioned header.
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 25 --
drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 18
2 files changed, 18 insertions(+), 25 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
index c3110a25a30d..2bb02e17ee52 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c
@@ -7,40 +7,17 @@
#include "dpu_hw_top.h"
#include "dpu_kms.h"
-#define SSPP_SPARE0x28
-
#define FLD_SPLIT_DISPLAY_CMD BIT(1)
#define FLD_SMART_PANEL_FREE_RUN BIT(2)
#define FLD_INTF_1_SW_TRG_MUX BIT(4)
#define FLD_INTF_2_SW_TRG_MUX BIT(8)
#define FLD_TE_LINE_INTER_WATERLEVEL_MASK 0x
-#define DANGER_STATUS 0x360
-#define SAFE_STATUS 0x364
-
-#define TE_LINE_INTERVAL 0x3F4
-
#define TRAFFIC_SHAPER_EN BIT(31)
#define TRAFFIC_SHAPER_RD_CLIENT(num) (0x030 + (num * 4))
#define TRAFFIC_SHAPER_WR_CLIENT(num) (0x060 + (num * 4))
#define TRAFFIC_SHAPER_FIXPOINT_FACTOR4
-#define MDP_WD_TIMER_0_CTL0x380
-#define MDP_WD_TIMER_0_CTL2 0x384
-#define MDP_WD_TIMER_0_LOAD_VALUE 0x388
-#define MDP_WD_TIMER_1_CTL0x390
-#define MDP_WD_TIMER_1_CTL2 0x394
-#define MDP_WD_TIMER_1_LOAD_VALUE 0x398
-#define MDP_WD_TIMER_2_CTL0x420
-#define MDP_WD_TIMER_2_CTL2 0x424
-#define MDP_WD_TIMER_2_LOAD_VALUE 0x428
-#define MDP_WD_TIMER_3_CTL0x430
-#define MDP_WD_TIMER_3_CTL2 0x434
-#define MDP_WD_TIMER_3_LOAD_VALUE 0x438
-#define MDP_WD_TIMER_4_CTL0x440
-#define MDP_WD_TIMER_4_CTL2 0x444
-#define MDP_WD_TIMER_4_LOAD_VALUE 0x448
-
#define MDP_TICK_COUNT16
#define XO_CLK_RATE 19200
#define MS_TICKS_IN_SEC 1000
@@ -48,8 +25,6 @@
#define CALCULATE_WD_LOAD_VALUE(fps) \
((uint32_t)((MS_TICKS_IN_SEC * XO_CLK_RATE)/(MDP_TICK_COUNT * fps)))
-#define DCE_SEL 0x450
-
static void dpu_hw_setup_split_pipe(struct dpu_hw_mdp *mdp,
struct split_pipe_cfg *cfg)
{
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
index c8156ed4b7fb..86c423e63b61 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h
@@ -16,6 +16,7 @@
#define INTR_CLEAR 0x018
#define INTR2_EN0x008
#define INTR2_STATUS0x00c
+#define SSPP_SPARE 0x028
#define INTR2_CLEAR 0x02c
#define HIST_INTR_EN0x01c
#define HIST_INTR_STATUS0x020
@@ -28,7 +29,15 @@
#define DSPP_IGC_COLOR0_RAM_LUTN0x300
#define DSPP_IGC_COLOR1_RAM_LUTN0x304
#define DSPP_IGC_COLOR2_RAM_LUTN0x308
+#define DANGER_STATUS 0x360
+#define SAFE_STATUS 0x364
#define HW_EVENTS_CTL 0x37C
+#define MDP_WD_TIMER_0_CTL 0x380
+#define MDP_WD_TIMER_0_CTL2 0x384
+#define MDP_WD_TIMER_0_LOAD_VALUE 0x388
+#define MDP_WD_TIMER_1_CTL 0x390
+#define MDP_WD_TIMER_1_CTL2 0x394
+#define MDP_WD_TIMER_1_LOAD_VALUE 0x398
#define CLK_CTRL3 0x3A8
#define CLK_STATUS3 0x3AC
#define CLK_CTRL4 0x3B0
@@ -43,6 +52,15 @@
#define HDMI_DP_CORE_SELECT 0x408
#define MDP_OUT_CTL_0 0x410
#define MDP_VSYNC_SEL 0x414
+#define MDP_WD_TIMER_2_CTL 0x420
+#define MDP_WD_TIMER_2_CTL2 0x424
+#define MDP_WD_TIMER_2_LOAD_VALUE 0x428
+#define MDP_WD_TIMER_3_CTL 0x430
+#define MDP_WD_TIMER_3_CTL2 0x434
+#define MDP_WD_TIMER_3_LOAD_VALUE 0x438
+#define MDP_WD_TIMER_4_CTL 0x440
+#define MDP_WD_TIMER_4_CTL2 0x444
+#define MDP_WD_TIMER_4_LOAD_VALUE 0x448
#define DCE_SEL 0x450
#endif /*_DPU_HWIO_H */
--
2.35.1
[PATCH v6 06/11] drm/msm/dsi: add support for DSI-PHY on SM8350 and SM8450
SM8350 and SM8450 use 5nm DSI PHYs, which share register definitions
with 7nm DSI PHYs. Rather than duplicating the driver, handle 5nm
variants inside the common 5+7nm driver.
Co-developed-by: Robert Foss
Tested-by: Vinod Koul
Reviewed-by: Vinod Koul
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/Kconfig | 6 +-
drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 4 +
drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 2 +
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 119 --
4 files changed, 118 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 3c9dfdb0b328..e7b100d97f88 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -140,12 +140,12 @@ config DRM_MSM_DSI_10NM_PHY
Choose this option if DSI PHY on SDM845 is used on the platform.
config DRM_MSM_DSI_7NM_PHY
- bool "Enable DSI 7nm PHY driver in MSM DRM"
+ bool "Enable DSI 7nm/5nm PHY driver in MSM DRM"
depends on DRM_MSM_DSI
default y
help
- Choose this option if DSI PHY on SM8150/SM8250/SC7280 is used on
- the platform.
+ Choose this option if DSI PHY on SM8150/SM8250/SM8350/SM8450/SC7280
+ is used on the platform.
config DRM_MSM_HDMI
bool "Enable HDMI support in MSM DRM driver"
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
index ee6051367679..0c956fdab23e 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.c
@@ -569,6 +569,10 @@ static const struct of_device_id dsi_phy_dt_match[] = {
.data = _phy_7nm_8150_cfgs },
{ .compatible = "qcom,sc7280-dsi-phy-7nm",
.data = _phy_7nm_7280_cfgs },
+ { .compatible = "qcom,dsi-phy-5nm-8350",
+ .data = _phy_5nm_8350_cfgs },
+ { .compatible = "qcom,dsi-phy-5nm-8450",
+ .data = _phy_5nm_8450_cfgs },
#endif
{}
};
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
index 1096afedd616..f7a907ed2b4b 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy.h
@@ -57,6 +57,8 @@ extern const struct msm_dsi_phy_cfg dsi_phy_10nm_8998_cfgs;
extern const struct msm_dsi_phy_cfg dsi_phy_7nm_cfgs;
extern const struct msm_dsi_phy_cfg dsi_phy_7nm_8150_cfgs;
extern const struct msm_dsi_phy_cfg dsi_phy_7nm_7280_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8350_cfgs;
+extern const struct msm_dsi_phy_cfg dsi_phy_5nm_8450_cfgs;
struct msm_dsi_dphy_timing {
u32 clk_zero;
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 0b780f9d3d0a..7b2c16b3a36c 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -39,8 +39,14 @@
#define VCO_REF_CLK_RATE 1920
#define FRAC_BITS 18
+/* Hardware is pre V4.1 */
+#define DSI_PHY_7NM_QUIRK_PRE_V4_1 BIT(0)
/* Hardware is V4.1 */
-#define DSI_PHY_7NM_QUIRK_V4_1 BIT(0)
+#define DSI_PHY_7NM_QUIRK_V4_1 BIT(1)
+/* Hardware is V4.2 */
+#define DSI_PHY_7NM_QUIRK_V4_2 BIT(2)
+/* Hardware is V4.3 */
+#define DSI_PHY_7NM_QUIRK_V4_3 BIT(3)
struct dsi_pll_config {
bool enable_ssc;
@@ -116,7 +122,7 @@ static void dsi_pll_calc_dec_frac(struct dsi_pll_7nm *pll,
struct dsi_pll_config
dec_multiple = div_u64(pll_freq * multiplier, divider);
dec = div_u64_rem(dec_multiple, multiplier, );
- if (!(pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1))
+ if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1)
config->pll_clock_inverters = 0x28;
else if (pll_freq <= 10ULL)
config->pll_clock_inverters = 0xa0;
@@ -197,16 +203,25 @@ static void dsi_pll_config_hzindep_reg(struct dsi_pll_7nm
*pll)
void __iomem *base = pll->phy->pll_base;
u8 analog_controls_five_1 = 0x01, vco_config_1 = 0x00;
- if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
+ if (!(pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_PRE_V4_1))
if (pll->vco_current_rate >= 31ULL)
analog_controls_five_1 = 0x03;
+ if (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
if (pll->vco_current_rate < 152000ULL)
vco_config_1 = 0x08;
else if (pll->vco_current_rate < 299000ULL)
vco_config_1 = 0x01;
}
+ if ((pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_2) ||
+ (pll->phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_3)) {
+ if (pll->vco_current_rate < 152000ULL)
+ vco_config_1 = 0x08;
+ else if (pll->vco_current_rate >= 299000ULL)
+ vco_config_1 = 0x01;
+ }
+
dsi_phy_write(base +
[PATCH v6 10/11] drm/msm/dpu: add support for SM8450
Add definitions for the display hardware used on Qualcomm SM8450
platform.
Tested-by: Vinod Koul
Reviewed-by: Vinod Koul
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 224 ++
.../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 1 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 +
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 +
4 files changed, 229 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
index 2196e205efa5..b4ca123d8e69 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
@@ -124,6 +124,15 @@
BIT(MDP_AD4_0_INTR) | \
BIT(MDP_AD4_1_INTR))
+#define IRQ_SM8450_MASK (BIT(MDP_SSPP_TOP0_INTR) | \
+BIT(MDP_SSPP_TOP0_INTR2) | \
+BIT(MDP_SSPP_TOP0_HIST_INTR) | \
+BIT(MDP_INTF0_7xxx_INTR) | \
+BIT(MDP_INTF1_7xxx_INTR) | \
+BIT(MDP_INTF2_7xxx_INTR) | \
+BIT(MDP_INTF3_7xxx_INTR) | \
+0)
+
#define WB_SM8250_MASK (BIT(DPU_WB_LINE_MODE) | \
BIT(DPU_WB_UBWC) | \
BIT(DPU_WB_YUV_CONFIG) | \
@@ -379,6 +388,20 @@ static const struct dpu_caps sm8250_dpu_caps = {
.pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
};
+static const struct dpu_caps sm8450_dpu_caps = {
+ .max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
+ .max_mixer_blendstages = 0xb,
+ .qseed_type = DPU_SSPP_SCALER_QSEED4,
+ .smart_dma_rev = DPU_SSPP_SMART_DMA_V2, /* TODO: v2.5 */
+ .ubwc_version = DPU_HW_UBWC_VER_40,
+ .has_src_split = true,
+ .has_dim_layer = true,
+ .has_idle_pc = true,
+ .has_3d_merge = true,
+ .max_linewidth = 5120,
+ .pixel_ram_size = DEFAULT_PIXEL_RAM_SIZE,
+};
+
static const struct dpu_caps sc7280_dpu_caps = {
.max_mixer_width = DEFAULT_DPU_OUTPUT_LINE_WIDTH,
.max_mixer_blendstages = 0x7,
@@ -529,6 +552,33 @@ static const struct dpu_mdp_cfg sm8250_mdp[] = {
},
};
+static const struct dpu_mdp_cfg sm8450_mdp[] = {
+ {
+ .name = "top_0", .id = MDP_TOP,
+ .base = 0x0, .len = 0x494,
+ .features = BIT(DPU_MDP_PERIPH_0_REMOVED),
+ .highest_bank_bit = 0x3, /* TODO: 2 for LP_DDR4 */
+ .clk_ctrls[DPU_CLK_CTRL_VIG0] = {
+ .reg_off = 0x2AC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG1] = {
+ .reg_off = 0x2B4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG2] = {
+ .reg_off = 0x2BC, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_VIG3] = {
+ .reg_off = 0x2C4, .bit_off = 0},
+ .clk_ctrls[DPU_CLK_CTRL_DMA0] = {
+ .reg_off = 0x2AC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_DMA1] = {
+ .reg_off = 0x2B4, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR0] = {
+ .reg_off = 0x2BC, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = {
+ .reg_off = 0x2C4, .bit_off = 8},
+ .clk_ctrls[DPU_CLK_CTRL_REG_DMA] = {
+ .reg_off = 0x2BC, .bit_off = 20},
+ },
+};
+
static const struct dpu_mdp_cfg sc7280_mdp[] = {
{
.name = "top_0", .id = MDP_TOP,
@@ -687,6 +737,45 @@ static const struct dpu_ctl_cfg sm8150_ctl[] = {
},
};
+static const struct dpu_ctl_cfg sm8450_ctl[] = {
+ {
+ .name = "ctl_0", .id = CTL_0,
+ .base = 0x15000, .len = 0x204,
+ .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_SPLIT_DISPLAY) |
BIT(DPU_CTL_FETCH_ACTIVE),
+ .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
+ },
+ {
+ .name = "ctl_1", .id = CTL_1,
+ .base = 0x16000, .len = 0x204,
+ .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_SPLIT_DISPLAY) |
BIT(DPU_CTL_FETCH_ACTIVE),
+ .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10),
+ },
+ {
+ .name = "ctl_2", .id = CTL_2,
+ .base = 0x17000, .len = 0x204,
+ .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE),
+ .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 11),
+ },
+ {
+ .name = "ctl_3", .id = CTL_3,
+ .base = 0x18000, .len = 0x204,
+ .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE),
+ .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 12),
+ },
+ {
+ .name = "ctl_4", .id = CTL_4,
+ .base = 0x19000, .len = 0x204,
+ .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE),
+ .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 13),
+ },
+ {
+ .name = "ctl_5", .id = CTL_5,
+ .base = 0x1a000, .len = 0x204,
+ .features =
[PATCH v6 03/11] dt-bindings: display/msm: add sm8350 and sm8450 DSI PHYs
SM8350 and SM8450 platforms use the same driver and same bindings as the existing 7nm DSI PHYs. Add corresponding compatibility strings. Acked-by: Krzysztof Kozlowski Signed-off-by: Dmitry Baryshkov --- Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml index c851770bbdf2..bffd161fedfd 100644 --- a/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml +++ b/Documentation/devicetree/bindings/display/msm/dsi-phy-7nm.yaml @@ -15,6 +15,8 @@ allOf: properties: compatible: enum: + - qcom,dsi-phy-5nm-8350 + - qcom,dsi-phy-5nm-8450 - qcom,dsi-phy-7nm - qcom,dsi-phy-7nm-8150 - qcom,sc7280-dsi-phy-7nm -- 2.35.1
[PATCH v6 01/11] dt-bindings: display/msm: *dpu.yaml: split required properties clauses
Require only properties declared in given schema, which makes the code a bit more readable and easy to follow. Suggested-by: Krzysztof Kozlowski Reviewed-by: Krzysztof Kozlowski Signed-off-by: Dmitry Baryshkov --- .../devicetree/bindings/display/msm/dpu-common.yaml| 4 .../devicetree/bindings/display/msm/qcom,msm8998-dpu.yaml | 7 +++ .../devicetree/bindings/display/msm/qcom,qcm2290-dpu.yaml | 7 +++ .../devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml | 7 +++ .../devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml | 7 +++ .../devicetree/bindings/display/msm/qcom,sdm845-dpu.yaml | 7 +++ .../devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml | 7 +++ 7 files changed, 42 insertions(+), 4 deletions(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu-common.yaml b/Documentation/devicetree/bindings/display/msm/dpu-common.yaml index 8ffbc30c6b7f..870158bb2aa0 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu-common.yaml +++ b/Documentation/devicetree/bindings/display/msm/dpu-common.yaml @@ -40,10 +40,6 @@ properties: - port@0 required: - - compatible - - reg - - reg-names - - clocks - interrupts - power-domains - operating-points-v2 diff --git a/Documentation/devicetree/bindings/display/msm/qcom,msm8998-dpu.yaml b/Documentation/devicetree/bindings/display/msm/qcom,msm8998-dpu.yaml index b02adba36e9e..479ce75bd451 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,msm8998-dpu.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,msm8998-dpu.yaml @@ -46,6 +46,13 @@ properties: - const: core - const: vsync +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-dpu.yaml b/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-dpu.yaml index a7b382f01b56..e794f0dd8ef4 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-dpu.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-dpu.yaml @@ -42,6 +42,13 @@ properties: - const: lut - const: vsync +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml index bd590a6b5b96..0dfdf8f3c5b4 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-dpu.yaml @@ -44,6 +44,13 @@ properties: - const: core - const: vsync +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml index 924059b387b6..512d23f8d629 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-dpu.yaml @@ -43,6 +43,13 @@ properties: - const: core - const: vsync +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-dpu.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-dpu.yaml index 5719b45f2860..d5a55e898b11 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-dpu.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-dpu.yaml @@ -42,6 +42,13 @@ properties: - const: core - const: vsync +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml index 9ff8a265c85f..687c8c170cd4 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8250-dpu.yaml @@ -39,6 +39,13 @@ properties: - const: core - const: vsync +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + unevaluatedProperties: false examples: -- 2.35.1
[PATCH v6 05/11] drm/msm/dsi/phy: rework register setting for 7nm PHY
In preparation to adding the sm8350 and sm8450 PHYs support, rearrange
register values calculations in dsi_7nm_phy_enable(). This change bears
no functional changes itself, it is merely a preparation for the next
patch.
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 26 +++
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
index 9e7fa7d88ead..0b780f9d3d0a 100644
--- a/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
+++ b/drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c
@@ -858,23 +858,34 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
/* Alter PHY configurations if data rate less than 1.5GHZ*/
less_than_1500_mhz = (clk_req->bitclk_rate <= 15);
+ if (phy->cphy_mode) {
+ vreg_ctrl_0 = 0x51;
+ vreg_ctrl_1 = 0x55;
+ glbl_pemph_ctrl_0 = 0x11;
+ lane_ctrl0 = 0x17;
+ } else {
+ vreg_ctrl_1 = 0x5c;
+ glbl_pemph_ctrl_0 = 0x00;
+ lane_ctrl0 = 0x1f;
+ }
+
if (phy->cfg->quirks & DSI_PHY_7NM_QUIRK_V4_1) {
- vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
if (phy->cphy_mode) {
glbl_rescode_top_ctrl = 0x00;
glbl_rescode_bot_ctrl = 0x3c;
} else {
+ vreg_ctrl_0 = less_than_1500_mhz ? 0x53 : 0x52;
glbl_rescode_top_ctrl = less_than_1500_mhz ? 0x3d :
0x00;
glbl_rescode_bot_ctrl = less_than_1500_mhz ? 0x39 :
0x3c;
}
glbl_str_swi_cal_sel_ctrl = 0x00;
glbl_hstx_str_ctrl_0 = 0x88;
} else {
- vreg_ctrl_0 = less_than_1500_mhz ? 0x5B : 0x59;
if (phy->cphy_mode) {
glbl_str_swi_cal_sel_ctrl = 0x03;
glbl_hstx_str_ctrl_0 = 0x66;
} else {
+ vreg_ctrl_0 = less_than_1500_mhz ? 0x5B : 0x59;
glbl_str_swi_cal_sel_ctrl = less_than_1500_mhz ? 0x03 :
0x00;
glbl_hstx_str_ctrl_0 = less_than_1500_mhz ? 0x66 : 0x88;
}
@@ -882,17 +893,6 @@ static int dsi_7nm_phy_enable(struct msm_dsi_phy *phy,
glbl_rescode_bot_ctrl = 0x3c;
}
- if (phy->cphy_mode) {
- vreg_ctrl_0 = 0x51;
- vreg_ctrl_1 = 0x55;
- glbl_pemph_ctrl_0 = 0x11;
- lane_ctrl0 = 0x17;
- } else {
- vreg_ctrl_1 = 0x5c;
- glbl_pemph_ctrl_0 = 0x00;
- lane_ctrl0 = 0x1f;
- }
-
/* de-assert digital and pll power down */
data = BIT(6) | BIT(5);
dsi_phy_write(base + REG_DSI_7nm_PHY_CMN_CTRL_0, data);
--
2.35.1
[PATCH v6 02/11] dt-bindings: display/msm: *mdss.yaml: split required properties clauses
Require only properties declared in given schema, which makes the code a bit more readable and easy to follow. Suggested-by: Krzysztof Kozlowski Reviewed-by: Krzysztof Kozlowski Signed-off-by: Dmitry Baryshkov --- Documentation/devicetree/bindings/display/msm/mdss-common.yaml | 1 - .../devicetree/bindings/display/msm/qcom,msm8998-mdss.yaml | 3 +++ .../devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml | 3 +++ .../devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml | 3 +++ .../devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml | 3 +++ .../devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml | 3 +++ .../devicetree/bindings/display/msm/qcom,sm8250-mdss.yaml | 3 +++ 7 files changed, 18 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml index 27d7242657b2..59f17ac898aa 100644 --- a/Documentation/devicetree/bindings/display/msm/mdss-common.yaml +++ b/Documentation/devicetree/bindings/display/msm/mdss-common.yaml @@ -70,7 +70,6 @@ properties: - description: MDSS_CORE reset required: - - compatible - reg - reg-names - power-domains diff --git a/Documentation/devicetree/bindings/display/msm/qcom,msm8998-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,msm8998-mdss.yaml index cf52ff77a41a..fc6969c9c52e 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,msm8998-mdss.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,msm8998-mdss.yaml @@ -55,6 +55,9 @@ patternProperties: compatible: const: qcom,dsi-phy-10nm-8998 +required: + - compatible + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml index d6f043a4b08d..0c2f9755125e 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,qcm2290-mdss.yaml @@ -61,6 +61,9 @@ patternProperties: compatible: const: qcom,dsi-phy-14nm-2290 +required: + - compatible + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml index 13e396d61a51..fb835a4d9114 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7180-mdss.yaml @@ -67,6 +67,9 @@ patternProperties: compatible: const: qcom,dsi-phy-10nm +required: + - compatible + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml index a3de1744ba11..a4e3ada2affc 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sc7280-mdss.yaml @@ -74,6 +74,9 @@ patternProperties: - qcom,sc7280-dsi-phy-7nm - qcom,sc7280-edp-phy +required: + - compatible + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml index 31ca6f99fc22..2a0960bf3052 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sdm845-mdss.yaml @@ -59,6 +59,9 @@ patternProperties: compatible: const: qcom,dsi-phy-10nm +required: + - compatible + unevaluatedProperties: false examples: diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8250-mdss.yaml b/Documentation/devicetree/bindings/display/msm/qcom,sm8250-mdss.yaml index 0d3be5386b3f..d752fd022ac5 100644 --- a/Documentation/devicetree/bindings/display/msm/qcom,sm8250-mdss.yaml +++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8250-mdss.yaml @@ -63,6 +63,9 @@ patternProperties: compatible: const: qcom,dsi-phy-7nm +required: + - compatible + unevaluatedProperties: false examples: -- 2.35.1
[PATCH v6 07/11] drm/msm/dsi: add support for DSI 2.6.0
Add support for DSI 2.6.0 (block used on sm8450).
Tested-by: Vinod Koul
Reviewed-by: Vinod Koul
Reviewed-by: Konrad Dybcio
Reviewed-by: Abhinav Kumar
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/dsi/dsi_cfg.c | 2 ++
drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 7e97c239ed48..59a4cc95a251 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -300,6 +300,8 @@ static const struct msm_dsi_cfg_handler dsi_cfg_handlers[]
= {
_dsi_cfg, _dsi_6g_v2_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_5_0,
_dsi_cfg, _dsi_6g_v2_host_ops},
+ {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_6_0,
+ _dsi_cfg, _dsi_6g_v2_host_ops},
};
const struct msm_dsi_cfg_handler *msm_dsi_cfg_get(u32 major, u32 minor)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
index 8f04e685a74e..95957fab499d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.h
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.h
@@ -25,6 +25,7 @@
#define MSM_DSI_6G_VER_MINOR_V2_4_00x2004
#define MSM_DSI_6G_VER_MINOR_V2_4_10x20040001
#define MSM_DSI_6G_VER_MINOR_V2_5_00x2005
+#define MSM_DSI_6G_VER_MINOR_V2_6_00x2006
#define MSM_DSI_V2_VER_MINOR_8064 0x0
--
2.35.1
[PATCH v6 04/11] dt-bindings: display/msm: add support for the display on SM8450
Add DPU and MDSS schemas to describe MDSS and DPU blocks on the Qualcomm
SM8450 platform.
Reviewed-by: Krzysztof Kozlowski
Signed-off-by: Dmitry Baryshkov
---
.../bindings/display/msm/qcom,sm8450-dpu.yaml | 139 +++
.../display/msm/qcom,sm8450-mdss.yaml | 343 ++
2 files changed, 482 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/msm/qcom,sm8450-dpu.yaml
create mode 100644
Documentation/devicetree/bindings/display/msm/qcom,sm8450-mdss.yaml
diff --git a/Documentation/devicetree/bindings/display/msm/qcom,sm8450-dpu.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sm8450-dpu.yaml
new file mode 100644
index ..0d17ece1c453
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8450-dpu.yaml
@@ -0,0 +1,139 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/msm/qcom,sm8450-dpu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SM8450 Display DPU
+
+maintainers:
+ - Dmitry Baryshkov
+
+$ref: /schemas/display/msm/dpu-common.yaml#
+
+properties:
+ compatible:
+const: qcom,sm8450-dpu
+
+ reg:
+items:
+ - description: Address offset and size for mdp register set
+ - description: Address offset and size for vbif register set
+
+ reg-names:
+items:
+ - const: mdp
+ - const: vbif
+
+ clocks:
+items:
+ - description: Display hf axi
+ - description: Display sf axi
+ - description: Display ahb
+ - description: Display lut
+ - description: Display core
+ - description: Display vsync
+
+ clock-names:
+items:
+ - const: bus
+ - const: nrt_bus
+ - const: iface
+ - const: lut
+ - const: core
+ - const: vsync
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - clocks
+ - clock-names
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+#include
+#include
+#include
+#include
+
+display-controller@ae01000 {
+compatible = "qcom,sm8450-dpu";
+reg = <0x0ae01000 0x8f000>,
+ <0x0aeb 0x2008>;
+reg-names = "mdp", "vbif";
+
+clocks = < GCC_DISP_HF_AXI_CLK>,
+< GCC_DISP_SF_AXI_CLK>,
+< DISP_CC_MDSS_AHB_CLK>,
+< DISP_CC_MDSS_MDP_LUT_CLK>,
+< DISP_CC_MDSS_MDP_CLK>,
+< DISP_CC_MDSS_VSYNC_CLK>;
+clock-names = "bus",
+ "nrt_bus",
+ "iface",
+ "lut",
+ "core",
+ "vsync";
+
+assigned-clocks = < DISP_CC_MDSS_VSYNC_CLK>;
+assigned-clock-rates = <1920>;
+
+operating-points-v2 = <_opp_table>;
+power-domains = < SM8450_MMCX>;
+
+interrupt-parent = <>;
+interrupts = <0>;
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+
+port@0 {
+reg = <0>;
+dpu_intf1_out: endpoint {
+remote-endpoint = <_in>;
+};
+};
+
+port@1 {
+reg = <1>;
+dpu_intf2_out: endpoint {
+remote-endpoint = <_in>;
+};
+};
+};
+
+mdp_opp_table: opp-table {
+compatible = "operating-points-v2";
+
+opp-17200{
+opp-hz = /bits/ 64 <17200>;
+required-opps = <_opp_low_svs_d1>;
+};
+
+opp-2 {
+opp-hz = /bits/ 64 <2>;
+required-opps = <_opp_low_svs>;
+};
+
+opp-32500 {
+opp-hz = /bits/ 64 <32500>;
+required-opps = <_opp_svs>;
+};
+
+opp-37500 {
+opp-hz = /bits/ 64 <37500>;
+required-opps = <_opp_svs_l1>;
+};
+
+opp-5 {
+opp-hz = /bits/ 64 <5>;
+required-opps = <_opp_nom>;
+};
+};
+};
+...
diff --git
a/Documentation/devicetree/bindings/display/msm/qcom,sm8450-mdss.yaml
b/Documentation/devicetree/bindings/display/msm/qcom,sm8450-mdss.yaml
new file mode 100644
index ..c268e0b662cf
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/msm/qcom,sm8450-mdss.yaml
@@ -0,0 +1,343 @@
+# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/msm/qcom,sm8450-mdss.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SM8450 Display MDSS
+
+maintainers:
+ - Dmitry Baryshkov
+
+description:
+ SM8450 MSM Mobile Display Subsystem(MDSS), which encapsulates sub-blocks like
+ DPU display controller, DSI and DP interfaces etc.
+
+$ref: /schemas/display/msm/mdss-common.yaml#
+
[PATCH v6 00/11] drm/msm: add support for SM8450
This adds support for the MDSS/DPU/DSI on the Qualcomm SM8450 platform. Change since v5: - Added defines to be used for the MDP_PERIPH_TOP0 blackhole Change since v4: - Fixed commit messages for the first two patches (Krzysztof) - Dropped clock-names requirement patch - Removed clock-names from qcom,sm8450-mdss.yaml schema - Fixed the schema changes lost between v3 and v4 (thanks Krzysztof) - Added kernel doc for DPU_MDP_PERIPH_0_REMOVED (Abhinav) - Fixed build issue in dpu_kms_mdp_snapshot() (Niel) Change since v3: - Reworked the dpu-common.yaml / mdss-common.yaml to require properties from the same schema where they are defined (Krzysztof) - Reworked PHY register settings to make it easier to understand (Konrad) Change since v2: - Rebased onto msm-next-lumag - Cleaned up bindings according to Krzysztof's suggestions Change since v1: - Fixed the regdma pointer in sm8450_dpu_cfg - Rebased onto pending msm-next-lumag - Added DT bindings for corresponding devices Dmitry Baryshkov (11): dt-bindings: display/msm: *dpu.yaml: split required properties clauses dt-bindings: display/msm: *mdss.yaml: split required properties clauses dt-bindings: display/msm: add sm8350 and sm8450 DSI PHYs dt-bindings: display/msm: add support for the display on SM8450 drm/msm/dsi/phy: rework register setting for 7nm PHY drm/msm/dsi: add support for DSI-PHY on SM8350 and SM8450 drm/msm/dsi: add support for DSI 2.6.0 drm/msm/dpu: merge all MDP TOP registers to dpu_hwio.h drm/msm/dpu: add support for MDP_TOP blackhole drm/msm/dpu: add support for SM8450 drm/msm: mdss add support for SM8450 .../bindings/display/msm/dpu-common.yaml | 4 - .../bindings/display/msm/dsi-phy-7nm.yaml | 2 + .../bindings/display/msm/mdss-common.yaml | 1 - .../display/msm/qcom,msm8998-dpu.yaml | 7 + .../display/msm/qcom,msm8998-mdss.yaml| 3 + .../display/msm/qcom,qcm2290-dpu.yaml | 7 + .../display/msm/qcom,qcm2290-mdss.yaml| 3 + .../bindings/display/msm/qcom,sc7180-dpu.yaml | 7 + .../display/msm/qcom,sc7180-mdss.yaml | 3 + .../bindings/display/msm/qcom,sc7280-dpu.yaml | 7 + .../display/msm/qcom,sc7280-mdss.yaml | 3 + .../bindings/display/msm/qcom,sdm845-dpu.yaml | 7 + .../display/msm/qcom,sdm845-mdss.yaml | 3 + .../bindings/display/msm/qcom,sm8250-dpu.yaml | 7 + .../display/msm/qcom,sm8250-mdss.yaml | 3 + .../bindings/display/msm/qcom,sm8450-dpu.yaml | 139 +++ .../display/msm/qcom,sm8450-mdss.yaml | 343 ++ drivers/gpu/drm/msm/Kconfig | 6 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c| 224 .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 4 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h | 3 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c| 25 -- drivers/gpu/drm/msm/disp/dpu1/dpu_hwio.h | 21 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 12 +- drivers/gpu/drm/msm/dsi/dsi_cfg.c | 2 + drivers/gpu/drm/msm/dsi/dsi_cfg.h | 1 + drivers/gpu/drm/msm/dsi/phy/dsi_phy.c | 4 + drivers/gpu/drm/msm/dsi/phy/dsi_phy.h | 2 + drivers/gpu/drm/msm/dsi/phy/dsi_phy_7nm.c | 141 +-- drivers/gpu/drm/msm/msm_mdss.c| 5 + 30 files changed, 943 insertions(+), 56 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/msm/qcom,sm8450-dpu.yaml create mode 100644 Documentation/devicetree/bindings/display/msm/qcom,sm8450-mdss.yaml -- 2.35.1
[PATCH linux-next] drm/virtio: use strscpy() to instead of strncpy()
From: Xu Panda The implementation of strscpy() is more robust and safer. That's now the recommended way to copy NUL terminated strings. Signed-off-by: Xu Panda Signed-off-by: Yang Yang --- drivers/gpu/drm/virtio/virtgpu_vq.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 9ff8660b50ad..7d95bc74b307 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -916,8 +916,7 @@ void virtio_gpu_cmd_context_create(struct virtio_gpu_device *vgdev, uint32_t id, cmd_p->hdr.ctx_id = cpu_to_le32(id); cmd_p->nlen = cpu_to_le32(nlen); cmd_p->context_init = cpu_to_le32(context_init); - strncpy(cmd_p->debug_name, name, sizeof(cmd_p->debug_name) - 1); - cmd_p->debug_name[sizeof(cmd_p->debug_name) - 1] = 0; + strscpy(cmd_p->debug_name, name, sizeof(cmd_p->debug_name)); virtio_gpu_queue_ctrl_buffer(vgdev, vbuf); } -- 2.15.2
[PATCH linux-next] drm/nouveau/nvkm/core/firmware: replace strncpy() with strscpy()
From: Xu Panda
The implementation of strscpy() is more robust and safer.
That's now the recommended way to copy NUL terminated strings.
Signed-off-by: Xu Panda
Signed-off-by: Yang Yang
---
drivers/gpu/drm/nouveau/nvkm/core/firmware.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
index fcf2a002f6cb..5554f907e0d4 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/firmware.c
@@ -79,8 +79,7 @@ nvkm_firmware_get(const struct nvkm_subdev *subdev, const
char *fwname, int ver,
int i;
/* Convert device name to lowercase */
- strncpy(cname, device->chip->name, sizeof(cname));
- cname[sizeof(cname) - 1] = '\0';
+ strscpy(cname, device->chip->name, sizeof(cname));
i = strlen(cname);
while (i) {
--i;
--
2.15.2
Re: [PATCH v5 08/10] drm/msm/dpu: add support for MDP_TOP blackhole
On 25/11/2022 08:01, Abhinav Kumar wrote:
On 11/23/2022 1:04 PM, Dmitry Baryshkov wrote:
On sm8450 a register block was removed from MDP TOP. Accessing it during
snapshotting results in NoC errors / immediate reboot. Skip accessing
these registers during snapshot.
Tested-by: Vinod Koul
Reviewed-by: Vinod Koul
Reviewed-by: Konrad Dybcio
Signed-off-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 3 +++
drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 11 +--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
index 38aa38ab1568..8da4c5ba6dc3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -82,6 +82,8 @@ enum {
* @DPU_MDP_UBWC_1_0, This chipsets supports Universal Bandwidth
* compression initial revision
* @DPU_MDP_UBWC_1_5, Universal Bandwidth compression version 1.5
+ * @DPU_MDP_PERIPH_0_REMOVED Indicates that access to periph top0
block results
+ * in a failure
shouldnt this be that "indicates that the top register block is not
contiguous and the two sub-blocks are separated by an offset"
Not so sure. Your suggestion is closer to the dynamic case, where all
the sizes are dynamic in catalog. Since the patch uses fixed offsets,
I'd mention periph0 instead (like the downstream does).
* @DPU_MDP_MAX Maximum value
*/
@@ -92,6 +94,7 @@ enum {
DPU_MDP_UBWC_1_0,
DPU_MDP_UBWC_1_5,
DPU_MDP_AUDIO_SELECT,
+ DPU_MDP_PERIPH_0_REMOVED,
DPU_MDP_MAX
};
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
index f3660cd14f4f..4ac14de55139 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
@@ -927,8 +927,15 @@ static void dpu_kms_mdp_snapshot(struct
msm_disp_state *disp_state, struct msm_k
msm_disp_snapshot_add_block(disp_state, cat->wb[i].len,
dpu_kms->mmio + cat->wb[i].base, "wb_%d", i);
- msm_disp_snapshot_add_block(disp_state, cat->mdp[0].len,
- dpu_kms->mmio + cat->mdp[0].base, "top");
+ if (cat->mdp[0].features & BIT(DPU_MDP_PERIPH_0_REMOVED)) {
+ msm_disp_snapshot_add_block(disp_state, 0x380,
+ dpu_kms->mmio + cat->mdp[0].base, "top");
+ msm_disp_snapshot_add_block(disp_state, cat->mdp[0].len - 0x3a8,
+ dpu_kms->mmio + cat->mdp[0].base + 0x3a8, "top_2");
I recall one of the comments from konrad that this should come from the
catalog rather than a hard-coded offset which you wanted to keep it for
a later time. I am fine with that.
But instead of a hard-coded offset, do you want to have a macro so that
atleast we know what the value means and can fix it in the future?
Otherwise it would end up being one of those numbers which someone later
on wouldnt understand where it comes from and what it means.
Yes, I macro makes sense to me. I'll fix in v6.
+ } else {
+ msm_disp_snapshot_add_block(disp_state, cat->mdp[0].len,
+ dpu_kms->mmio + cat->mdp[0].base, "top");
+ }
pm_runtime_put_sync(_kms->pdev->dev);
}
--
With best wishes
Dmitry
Re: [PATCH] dma-buf: fix dma_buf_export init order
Hi Christian, I love your patch! Yet something to improve: [auto build test ERROR on v6.1-rc8] [also build test ERROR on linus/master] [cannot apply to drm-misc/drm-misc-next next-20221206] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/dma-buf-fix-dma_buf_export-init-order/20221206-231311 patch link: https://lore.kernel.org/r/20221206151207.8801-1-christian.koenig%40amd.com patch subject: [PATCH] dma-buf: fix dma_buf_export init order config: hexagon-randconfig-r022-20221206 compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 6e4cea55f0d1104408b26ac574566a0e4de48036) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/aa1f28d40a4dc78408327baefb83ee8fd34b6ef9 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Christian-K-nig/dma-buf-fix-dma_buf_export-init-order/20221206-231311 git checkout aa1f28d40a4dc78408327baefb83ee8fd34b6ef9 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/dma-buf/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All errors (new ones prefixed by >>): In file included from drivers/dma-buf/dma-buf.c:16: In file included from include/linux/dma-buf.h:16: In file included from include/linux/iosys-map.h:10: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:334: include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __raw_readb(PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) ((__force __u16)(__le16)(x)) ^ In file included from drivers/dma-buf/dma-buf.c:16: In file included from include/linux/dma-buf.h:16: In file included from include/linux/iosys-map.h:10: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:334: include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) ((__force __u32)(__le32)(x)) ^ In file included from drivers/dma-buf/dma-buf.c:16: In file included from include/linux/dma-buf.h:16: In file included from include/linux/iosys-map.h:10: In file included from include/linux/io.h:13: In file included from arch/hexagon/include/asm/io.h:334: include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writeb(value, PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr); ~~ ^ >> drivers/dma-buf/dma-buf.c:633:25: error: incompatible integer to pointer >> conversion passing 'const size_t' (aka 'const unsigned int') to parameter of >> type 'struct dma_buf *' [-Wint-conversion] file = dma_buf_getfile(exp_info->size, exp_info->flags); ^~ drivers/dma-buf/dma-buf.c:526:5
Re: [PATCH] dma-buf: fix dma_buf_export init order
Hi Christian, I love your patch! Yet something to improve: [auto build test ERROR on v6.1-rc8] [also build test ERROR on linus/master] [cannot apply to drm-misc/drm-misc-next next-20221206] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Christian-K-nig/dma-buf-fix-dma_buf_export-init-order/20221206-231311 patch link: https://lore.kernel.org/r/20221206151207.8801-1-christian.koenig%40amd.com patch subject: [PATCH] dma-buf: fix dma_buf_export init order config: s390-randconfig-r044-20221206 compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 6e4cea55f0d1104408b26ac574566a0e4de48036) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install s390 cross compiling tool for clang build # apt-get install binutils-s390x-linux-gnu # https://github.com/intel-lab-lkp/linux/commit/aa1f28d40a4dc78408327baefb83ee8fd34b6ef9 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Christian-K-nig/dma-buf-fix-dma_buf_export-init-order/20221206-231311 git checkout aa1f28d40a4dc78408327baefb83ee8fd34b6ef9 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash drivers/dma-buf/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot All errors (new ones prefixed by >>): In file included from drivers/dma-buf/dma-buf.c:16: In file included from include/linux/dma-buf.h:16: In file included from include/linux/iosys-map.h:10: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __raw_readb(PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu' #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x)) ^ include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16' #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x)) ^ In file included from drivers/dma-buf/dma-buf.c:16: In file included from include/linux/dma-buf.h:16: In file included from include/linux/iosys-map.h:10: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); ~~ ^ include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu' #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) ^ include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32' #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) ^ In file included from drivers/dma-buf/dma-buf.c:16: In file included from include/linux/dma-buf.h:16: In file included from include/linux/iosys-map.h:10: In file included from include/linux/io.h:13: In file included from arch/s390/include/asm/io.h:75: include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writeb(value, PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); ~~ ^ include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] __raw_write
[PATCH v10 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
Starting with MTL, there will be two GT-tiles, a render and media tile. PXP as a service for supporting workloads with protected contexts and protected buffers can be subscribed by process workloads on any tile. However, depending on the platform, only one of the t iles is used for control events pertaining to PXP operation (such as creating the arbitration session and session tear-down). PXP as a global feature is accessible via batch buffer instructions on any engine/tile and the coherency across tiles is handled implicitly by the HW. In fact, for the foreseeable future, we are expecting this single-control-tile for the PXP subsystem. In MTL, it's the standalone media tile (not the root tile) because it contains the VDBOX and KCR engine (among the assets PXP relies on for those events). Looking at the current code design, each tile is represented by the intel_gt structure while the intel_pxp structure currently hangs off the intel_gt structure. Keeping the intel_pxp structure within the intel_gt structure makes some internal functionalities more straight forward but adds code complexity to code readibility and maintainibility to many external-to-pxp subsystems which may need to pick the correct intel_gt structure. An example of this would be the intel_pxp_is_active or intel_pxp_is_enabled functionality which should be viewed as a global level inquiry, not a per-gt inquiry. That said, this series promotes the intel_pxp structure into the drm_i915_private structure making it a top-level subsystem and the PXP subsystem will select the control gt internally and keep a pointer to it for internal reference. This promotion comes with two noteworthy changes: 1. Exported pxp functions that are called by external subsystems (such as intel_pxp_enabled/active) will have to check implicitly if i915->pxp is valid as that structure will not be allocated for HW that doesn't support PXP. 2. Since GT is now considered a soft-dependency of PXP we are ensuring that GT init happens before PXP init and vice versa for fini. This causes a minor ordering change whereby we previously called intel_pxp_suspend after intel_uc_suspend but now is before i915_gem_suspend_late but the change is required for correct dependency flows. Additionally, this re-order change doesn't have any impact because at that point in either case, the top level entry to i915 won't observe any PXP events (since the GPU was quiesced during suspend_prepare). Also, any PXP event doesn't really matter when we disable the PXP HW (global GT irqs are already off anyway, so even if there was a bug that generated spurious events we wouldn't see it and we would just clean it up on resume which is okay since the default fallback action for PXP would be to keep the sessions off at this suspend stage). Changes from prior revs: v9: - Cosmetic cleanups in supported/enabled/active. (Daniele). - Add comments for intel_pxp_init and pxp_get_ctrl_gt that explain the functional flow for when PXP is not supported but the backend-assets are needed for HuC authentication (Daniele and Tvrtko). - Fix two remaining functions that are accessible outside PXP that need to be checking pxp ptrs before using them: intel_pxp_irq_handler and intel_pxp_huc_load_and_auth (Tvrtko and Daniele). - User helper macro in pxp-debugfs (Tvrtko). v8: - Remove pxp_to_gt macro (Daniele). - Fix a bug in pxp_get_ctrl_gt for the case of MTL and we don't support GSC-FW on it. (Daniele). - Leave i915->pxp as NULL if we dont support PXP and in line with that, do additional validity check on i915->pxp for intel_pxp_is_supported/enabled/active (Daniele). - Remove unncessary include header from intel_gt_debugfs.c and check drm_minor i915->drm.primary (Daniele). - Other cosmetics / minor issues / more comments on suspend flow order change (Daniele). v7: - Drop i915_dev_to_pxp and in intel_pxp_init use 'i915->pxp' through out instead of local variable newpxp. (Rodrigo) - In the case intel_pxp_fini is called during driver unload but after i915 loading failed without pxp being allocated, check i915->pxp before referencing it. (Alan) v6: - Remove HAS_PXP macro and replace it with intel_pxp_is_supported because : [1] introduction of 'ctrl_gt' means we correct this for MTL's upcoming series now. [2] Also, this has little impact globally as its only used by PXP-internal callers at the moment. - Change intel_pxp_init/fini to take in i915 as its input to avoid ptr-to-ptr in init/fini calls.(Jani). - Remove the backpointer from pxp->i915 since we can use pxp->ctrl_gt->i915 if we need it. (Rodrigo). v5: - Switch from series to single patch (Rodrigo). - change function name from pxp_get_kcr_owner_gt to
Re: [PATCH v9 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
Apologies, ignore this - typo on rev count - will resend with proper v10 subject (and cancel CI) On Tue, 2022-12-06 at 13:27 -0800, Teres Alexis, Alan Previn wrote: > Starting with MTL, there will be two GT-tiles, a render and media > tile. PXP as a service for supporting workloads with protected
[PATCH v9 1/1] drm/i915/pxp: Promote pxp subsystem to top-level of i915
Starting with MTL, there will be two GT-tiles, a render and media tile. PXP as a service for supporting workloads with protected contexts and protected buffers can be subscribed by process workloads on any tile. However, depending on the platform, only one of the tiles is used for control events pertaining to PXP operation (such as creating the arbitration session and session tear-down). PXP as a global feature is accessible via batch buffer instructions on any engine/tile and the coherency across tiles is handled implicitly by the HW. In fact, for the foreseeable future, we are expecting this single-control-tile for the PXP subsystem. In MTL, it's the standalone media tile (not the root tile) because it contains the VDBOX and KCR engine (among the assets PXP relies on for those events). Looking at the current code design, each tile is represented by the intel_gt structure while the intel_pxp structure currently hangs off the intel_gt structure. Keeping the intel_pxp structure within the intel_gt structure makes some internal functionalities more straight forward but adds code complexity to code readibility and maintainibility to many external-to-pxp subsystems which may need to pick the correct intel_gt structure. An example of this would be the intel_pxp_is_active or intel_pxp_is_enabled functionality which should be viewed as a global level inquiry, not a per-gt inquiry. That said, this series promotes the intel_pxp structure into the drm_i915_private structure making it a top-level subsystem and the PXP subsystem will select the control gt internally and keep a pointer to it for internal reference. This promotion comes with two noteworthy changes: 1. Exported pxp functions that are called by external subsystems (such as intel_pxp_enabled/active) will have to check implicitly if i915->pxp is valid as that structure will not be allocated for HW that doesn't support PXP. 2. Since GT is now considered a soft-dependency of PXP we are ensuring that GT init happens before PXP init and vice versa for fini. This causes a minor ordering change whereby we previously called intel_pxp_suspend after intel_uc_suspend but now is before i915_gem_suspend_late but the change is required for correct dependency flows. Additionally, this re-order change doesn't have any impact because at that point in either case, the top level entry to i915 won't observe any PXP events (since the GPU was quiesced during suspend_prepare). Also, any PXP event doesn't really matter when we disable the PXP HW (global GT irqs are already off anyway, so even if there was a bug that generated spurious events we wouldn't see it and we would just clean it up on resume which is okay since the default fallback action for PXP would be to keep the sessions off at this suspend stage). Changes from prior revs: v9: - Cosmetic cleanups in supported/enabled/active. (Daniele). - Add comments for intel_pxp_init and pxp_get_ctrl_gt that explain the functional flow for when PXP is not supported but the backend-assets are needed for HuC authentication (Daniele and Tvrtko). - Fix two remaining functions that are accessible outside PXP that need to be checking pxp ptrs before using them: intel_pxp_irq_handler and intel_pxp_huc_load_and_auth (Tvrtko and Daniele). - User helper macro in pxp-debugfs (Tvrtko). v8: - Remove pxp_to_gt macro (Daniele). - Fix a bug in pxp_get_ctrl_gt for the case of MTL and we don't support GSC-FW on it. (Daniele). - Leave i915->pxp as NULL if we dont support PXP and in line with that, do additional validity check on i915->pxp for intel_pxp_is_supported/enabled/active (Daniele). - Remove unncessary include header from intel_gt_debugfs.c and check drm_minor i915->drm.primary (Daniele). - Other cosmetics / minor issues / more comments on suspend flow order change (Daniele). v7: - Drop i915_dev_to_pxp and in intel_pxp_init use 'i915->pxp' through out instead of local variable newpxp. (Rodrigo) - In the case intel_pxp_fini is called during driver unload but after i915 loading failed without pxp being allocated, check i915->pxp before referencing it. (Alan) v6: - Remove HAS_PXP macro and replace it with intel_pxp_is_supported because : [1] introduction of 'ctrl_gt' means we correct this for MTL's upcoming series now. [2] Also, this has little impact globally as its only used by PXP-internal callers at the moment. - Change intel_pxp_init/fini to take in i915 as its input to avoid ptr-to-ptr in init/fini calls.(Jani). - Remove the backpointer from pxp->i915 since we can use pxp->ctrl_gt->i915 if we need it. (Rodrigo). v5: - Switch from series to single patch (Rodrigo). - change function name from pxp_get_kcr_owner_gt to
Re: [PATCH 1/4] drm/msm/adreno: Fix null ptr access in adreno_gpu_cleanup()
On 12/5/2022 2:10 PM, Dan Carpenter wrote:
> On Sun, Dec 04, 2022 at 04:11:41AM +0530, Akhil P Oommen wrote:
>> Fix the below kernel panic due to null pointer access:
>> [ 18.504431] Unable to handle kernel NULL pointer dereference at virtual
>> address 0048
>> [ 18.513464] Mem abort info:
>> [ 18.516346] ESR = 0x9605
>> [ 18.520204] EC = 0x25: DABT (current EL), IL = 32 bits
>> [ 18.525706] SET = 0, FnV = 0
>> [ 18.528878] EA = 0, S1PTW = 0
>> [ 18.532117] FSC = 0x05: level 1 translation fault
>> [ 18.537138] Data abort info:
>> [ 18.540110] ISV = 0, ISS = 0x0005
>> [ 18.544060] CM = 0, WnR = 0
>> [ 18.547109] user pgtable: 4k pages, 39-bit VAs, pgdp=000112826000
>> [ 18.553738] [0048] pgd=,
>> p4d=, pud=
>> [ 18.562690] Internal error: Oops: 9605 [#1] PREEMPT SMP
>> **Snip**
>> [ 18.696758] Call trace:
>> [ 18.699278] adreno_gpu_cleanup+0x30/0x88
>> [ 18.703396] a6xx_destroy+0xc0/0x130
>> [ 18.707066] a6xx_gpu_init+0x308/0x424
> Fixes: 17e822f7591f ("drm/msm: fix unbalanced pm_runtime_enable in
> adreno_gpu_{init, cleanup}")
>
> Let's add Jonathan to the CC list so he can Ack your patch.
Thanks, will post a v2.
-Akhil.
>
> Although the real issue is that a6xx_gpu_init has bad error handling.
>
> The a6xx_destroy() function supposed to free *everything* so then the
> question becomes how do we avoid freeing something which was not
> allocated? With normal kernel style we just free things one by one
> in the reverse order from how they were allocated. See my blog for more
> details:
> https://staticthinking.wordpress.com/2022/04/28/free-the-last-thing-style/
Nice post. Thanks for sharing.
>
> However this code is written in One Function Frees Everything Style
> which is difficult to review and prone to bugs. The common mistakes are
> the kind of NULL dereference that you've seen, double frees, and missing
> frees.
>
> The only way to read this code is to open a new text editor window and
> line up the allocations with the frees.
>
> 1725 static void a6xx_destroy(struct msm_gpu *gpu)
> 1726 {
> 1727 struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> 1728 struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> 1729
> 1730 if (a6xx_gpu->sqe_bo) {
> 1731 msm_gem_unpin_iova(a6xx_gpu->sqe_bo, gpu->aspace);
> 1732 drm_gem_object_put(a6xx_gpu->sqe_bo);
>
> These unpin/put must be done together and should be in their own
> function. The ->sqe_bo pointer is allocated in a6xx_ucode_init(). It's
> assigned to an error pointer, but then set to NULL on error or after a
> free. So this is okay.
Agree. This warrants a helper function. I count 11 instances where it will be
useful.
>
> 1733 }
> 1734
> 1735 if (a6xx_gpu->shadow_bo) {
>
> ->shadow_bo is allocated in hw_init(). Should there be a call to
> msm_gem_put_vaddr(a6xx_gpu->shadow)? It's unclear. [QUESTION #1]
Yes. This should be freed with msm_gem_kernel_put() which takes care of freeing
vaddr.
>
> 1736 msm_gem_unpin_iova(a6xx_gpu->shadow_bo, gpu->aspace);
> 1737 drm_gem_object_put(a6xx_gpu->shadow_bo);
> 1738 }
> 1739
> 1740 a6xx_llc_slices_destroy(a6xx_gpu);
>
> This has IS_ERR_OR_NULL() checks so it's okay.
>
> 1741
> 1742 a6xx_gmu_remove(a6xx_gpu);
>
> This uses a gmu->initialized flag which allows it to safely clean up
> everything. Fine.
>
> 1743
> 1744 adreno_gpu_cleanup(adreno_gpu);
>
> This function has the bug that you identified. Let's dig into it.
> (With normal kernel error handling you can read the error handling by
> looking at the label name but with this style we need to jump around and
> compare code from different files).
>
> 1745
> 1746 kfree(a6xx_gpu);
> 1747 }
>
> drivers/gpu/drm/msm/adreno/adreno_gpu.c
> 1079 void adreno_gpu_cleanup(struct adreno_gpu *adreno_gpu)
> 1080 {
> 1081 struct msm_gpu *gpu = _gpu->base;
> 1082 struct msm_drm_private *priv = gpu->dev->dev_private;
> 1083 unsigned int i;
> 1084
> 1085 for (i = 0; i < ARRAY_SIZE(adreno_gpu->info->fw); i++)
> 1086 release_firmware(adreno_gpu->fw[i]);
>
> This is okay. ->fw[i] is either valid or NULL and releasing a NULL is
> fine.
>
> 1087
> 1088 if (pm_runtime_enabled(>gpu_pdev->dev))
>
> This is the bug you found.
>
> 1089 pm_runtime_disable(>gpu_pdev->dev);
> 1090
> 1091 msm_gpu_cleanup(_gpu->base);
>
> Let's dig into msm_gpu_cleanup().
>
> 1092 }
>
> drivers/gpu/drm/msm/msm_gpu.c
> 1006 void msm_gpu_cleanup(struct msm_gpu *gpu)
> 1007 {
> 1008 int i;
> 1009
> 1010 DBG("%s", gpu->name);
> 1011
> 1012 for (i = 0; i <
Re: [Freedreno] [PATCH v7 0/6] clk/qcom: Support gdsc collapse polling using 'reset' interface
On 12/2/2022 12:30 PM, Akhil P Oommen wrote: > On 12/2/2022 4:27 AM, Bjorn Andersson wrote: >> On Wed, Oct 05, 2022 at 02:36:58PM +0530, Akhil P Oommen wrote: >> @Ulf, Akhil has a power-domain for a piece of hardware which may be >> voted active by multiple different subsystems (co-processors/execution >> contexts) in the system. >> >> As such, during the powering down sequence we don't wait for the >> power-domain to turn off. But in the event of an error, the recovery >> mechanism relies on waiting for the hardware to settle in a powered off >> state. >> >> The proposal here is to use the reset framework to wait for this state >> to be reached, before continuing with the recovery mechanism in the >> client driver. >> >> Given our other discussions on quirky behavior, do you have any >> input/suggestions on this? Ulf, Gentle ping! Could you please share your feedback? -Akhil. >> >>> Some clients like adreno gpu driver would like to ensure that its gdsc >>> is collapsed at hardware during a gpu reset sequence. This is because it >>> has a votable gdsc which could be ON due to a vote from another subsystem >>> like tz, hyp etc or due to an internal hardware signal. To allow >>> this, gpucc driver can expose an interface to the client driver using >>> reset framework. Using this the client driver can trigger a polling within >>> the gdsc driver. >> @Akhil, this description is fairly generic. As we've reached the state >> where the hardware has settled and we return to the client, what >> prevents it from being powered up again? >> >> Or is it simply a question of it hitting the powered-off state, not >> necessarily staying there? > Correct. It doesn't need to stay there. The intention is to hit the > powered-off state at least once to clear all the internal hw states > (basically a hw reset). > > -Akhil. >> Regards, >> Bjorn >> >>> This series is rebased on top of qcom/linux:for-next branch. >>> >>> Related discussion: https://patchwork.freedesktop.org/patch/493144/ >>> >>> Changes in v7: >>> - Update commit message (Bjorn) >>> - Rebased on top of qcom/linux:for-next branch. >>> >>> Changes in v6: >>> - No code changes in this version. Just captured the Acked-by tags >>> >>> Changes in v5: >>> - Nit: Remove a duplicate blank line (Krzysztof) >>> >>> Changes in v4: >>> - Update gpu dt-binding schema >>> - Typo fix in commit text >>> >>> Changes in v3: >>> - Use pointer to const for "struct qcom_reset_ops" in qcom_reset_map >>> (Krzysztof) >>> >>> Changes in v2: >>> - Return error when a particular custom reset op is not implemented. >>> (Dmitry) >>> >>> Akhil P Oommen (6): >>> dt-bindings: clk: qcom: Support gpu cx gdsc reset >>> clk: qcom: Allow custom reset ops >>> clk: qcom: gdsc: Add a reset op to poll gdsc collapse >>> clk: qcom: gpucc-sc7280: Add cx collapse reset support >>> dt-bindings: drm/msm/gpu: Add optional resets >>> arm64: dts: qcom: sc7280: Add Reset support for gpu >>> >>> .../devicetree/bindings/display/msm/gpu.yaml | 6 + >>> arch/arm64/boot/dts/qcom/sc7280.dtsi | 3 +++ >>> drivers/clk/qcom/gdsc.c| 23 ++ >>> drivers/clk/qcom/gdsc.h| 7 ++ >>> drivers/clk/qcom/gpucc-sc7280.c| 10 >>> drivers/clk/qcom/reset.c | 27 >>> +- >>> drivers/clk/qcom/reset.h | 8 +++ >>> include/dt-bindings/clock/qcom,gpucc-sc7280.h | 3 +++ >>> 8 files changed, 82 insertions(+), 5 deletions(-) >>> >>> -- >>> 2.7.4 >>>
[PATCH] drm/msm: Add MSM_SUBMIT_BO_NO_IMPLICIT
From: Rob Clark
In cases where implicit sync is used, it is still useful (for things
like sub-allocation, etc) to allow userspace to opt-out of implicit
sync on per-BO basis.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/msm_drv.c| 3 ++-
drivers/gpu/drm/msm/msm_gem_submit.c | 11 +++
include/uapi/drm/msm_drm.h | 4 +++-
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 017a512982a2..e0e1199a822f 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -45,9 +45,10 @@
* - 1.7.0 - Add MSM_PARAM_SUSPENDS to access suspend count
* - 1.8.0 - Add MSM_BO_CACHED_COHERENT for supported GPUs (a6xx)
* - 1.9.0 - Add MSM_SUBMIT_FENCE_SN_IN
+ * - 1.10.0 - Add MSM_SUBMIT_BO_NO_IMPLICIT
*/
#define MSM_VERSION_MAJOR 1
-#define MSM_VERSION_MINOR 9
+#define MSM_VERSION_MINOR 10
#define MSM_VERSION_PATCHLEVEL 0
static const struct drm_mode_config_funcs mode_config_funcs = {
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index eb3536e3d66a..8bad07a04f85 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -334,9 +334,20 @@ static int submit_fence_sync(struct msm_gem_submit
*submit, bool no_implicit)
if (ret)
return ret;
+ /* If userspace has determined that explicit fencing is
+* used, it can disable implicit sync on the entire
+* submit:
+*/
if (no_implicit)
continue;
+ /* Otherwise userspace can ask for implicit sync to be
+* disabled on specific buffers. This is useful for internal
+* usermode driver managed buffers, suballocation, etc.
+*/
+ if (submit->bos[i].flags & MSM_SUBMIT_BO_NO_IMPLICIT)
+ continue;
+
ret = drm_sched_job_add_implicit_dependencies(>base,
obj,
write);
diff --git a/include/uapi/drm/msm_drm.h b/include/uapi/drm/msm_drm.h
index f54b48ef6a2d..329100016e7c 100644
--- a/include/uapi/drm/msm_drm.h
+++ b/include/uapi/drm/msm_drm.h
@@ -222,10 +222,12 @@ struct drm_msm_gem_submit_cmd {
#define MSM_SUBMIT_BO_READ 0x0001
#define MSM_SUBMIT_BO_WRITE0x0002
#define MSM_SUBMIT_BO_DUMP 0x0004
+#define MSM_SUBMIT_BO_NO_IMPLICIT 0x0008
#define MSM_SUBMIT_BO_FLAGS(MSM_SUBMIT_BO_READ | \
MSM_SUBMIT_BO_WRITE | \
- MSM_SUBMIT_BO_DUMP)
+ MSM_SUBMIT_BO_DUMP | \
+ MSM_SUBMIT_BO_NO_IMPLICIT)
struct drm_msm_gem_submit_bo {
__u32 flags; /* in, mask of MSM_SUBMIT_BO_x */
--
2.38.1
Re: [PATCH] dma-buf: fix dma_buf_export init order
On Tue, Dec 6, 2022 at 7:12 AM Christian König
wrote:
>
> The init order and resulting error handling in dma_buf_export
> was pretty messy.
>
> Subordinate objects like the file and the sysfs kernel objects
> were initializing and wiring itself up with the object in the
> wrong order resulting not only in complicating and partially
> incorrect error handling, but also in publishing only halve
> initialized DMA-buf objects.
>
> Clean this up thoughtfully by allocating the file independent
> of the DMA-buf object. Then allocate and initialize the DMA-buf
> object itself, before publishing it through sysfs. If everything
> works as expected the file is then connected with the DMA-buf
> object and publish it through debugfs.
>
> Also adds the missing dma_resv_fini() into the error handling.
>
> Signed-off-by: Christian König
> ---
> drivers/dma-buf/dma-buf-sysfs-stats.c | 7 +--
> drivers/dma-buf/dma-buf-sysfs-stats.h | 4 +-
> drivers/dma-buf/dma-buf.c | 65 +--
> 3 files changed, 34 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c
> b/drivers/dma-buf/dma-buf-sysfs-stats.c
> index 2bba0babcb62..4b680e10c15a 100644
> --- a/drivers/dma-buf/dma-buf-sysfs-stats.c
> +++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
> @@ -168,14 +168,11 @@ void dma_buf_uninit_sysfs_statistics(void)
> kset_unregister(dma_buf_stats_kset);
> }
>
> -int dma_buf_stats_setup(struct dma_buf *dmabuf)
> +int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file)
> {
> struct dma_buf_sysfs_entry *sysfs_entry;
> int ret;
>
> - if (!dmabuf || !dmabuf->file)
> - return -EINVAL;
> -
> if (!dmabuf->exp_name) {
> pr_err("exporter name must not be empty if stats needed\n");
> return -EINVAL;
> @@ -192,7 +189,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
>
> /* create the directory for buffer stats */
> ret = kobject_init_and_add(_entry->kobj, _buf_ktype, NULL,
> - "%lu", file_inode(dmabuf->file)->i_ino);
> + "%lu", file_inode(file)->i_ino);
> if (ret)
> goto err_sysfs_dmabuf;
>
> diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h
> b/drivers/dma-buf/dma-buf-sysfs-stats.h
> index a49c6e2650cc..7a8a995b75ba 100644
> --- a/drivers/dma-buf/dma-buf-sysfs-stats.h
> +++ b/drivers/dma-buf/dma-buf-sysfs-stats.h
> @@ -13,7 +13,7 @@
> int dma_buf_init_sysfs_statistics(void);
> void dma_buf_uninit_sysfs_statistics(void);
>
> -int dma_buf_stats_setup(struct dma_buf *dmabuf);
> +int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file);
>
> void dma_buf_stats_teardown(struct dma_buf *dmabuf);
> #else
> @@ -25,7 +25,7 @@ static inline int dma_buf_init_sysfs_statistics(void)
>
> static inline void dma_buf_uninit_sysfs_statistics(void) {}
>
> -static inline int dma_buf_stats_setup(struct dma_buf *dmabuf)
> +static inline int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file
> *file)
> {
> return 0;
> }
> diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> index e6f36c014c4c..ea08049b70ae 100644
> --- a/drivers/dma-buf/dma-buf.c
> +++ b/drivers/dma-buf/dma-buf.c
> @@ -614,19 +614,11 @@ struct dma_buf *dma_buf_export(const struct
> dma_buf_export_info *exp_info)
> size_t alloc_size = sizeof(struct dma_buf);
> int ret;
>
> - if (!exp_info->resv)
> - alloc_size += sizeof(struct dma_resv);
> - else
> - /* prevent _buf[1] == dma_buf->resv */
> - alloc_size += 1;
> -
> - if (WARN_ON(!exp_info->priv
> - || !exp_info->ops
> - || !exp_info->ops->map_dma_buf
> - || !exp_info->ops->unmap_dma_buf
> - || !exp_info->ops->release)) {
> + if (WARN_ON(!exp_info->priv || !exp_info->ops
> + || !exp_info->ops->map_dma_buf
> + || !exp_info->ops->unmap_dma_buf
> + || !exp_info->ops->release))
> return ERR_PTR(-EINVAL);
> - }
>
> if (WARN_ON(exp_info->ops->cache_sgt_mapping &&
> (exp_info->ops->pin || exp_info->ops->unpin)))
> @@ -638,10 +630,21 @@ struct dma_buf *dma_buf_export(const struct
> dma_buf_export_info *exp_info)
> if (!try_module_get(exp_info->owner))
> return ERR_PTR(-ENOENT);
>
> + file = dma_buf_getfile(exp_info->size, exp_info->flags);
> + if (IS_ERR(file)) {
> + ret = PTR_ERR(file);
> + goto err_module;
> + }
> +
> + if (!exp_info->resv)
> + alloc_size += sizeof(struct dma_resv);
> + else
> + /* prevent _buf[1] == dma_buf->resv */
> + alloc_size += 1;
> dmabuf = kzalloc(alloc_size, GFP_KERNEL);
> if (!dmabuf) {
> ret
Re: Try to address the DMA-buf coherency problem
Am 06.12.22 um 19:26 schrieb Nicolas Dufresne: Le lundi 05 décembre 2022 à 09:28 +0100, Christian König a écrit : Hi Tomasz, Am 05.12.22 um 07:41 schrieb Tomasz Figa: [SNIP] In other words explicit ownership transfer is not something we would want as requirement in the framework, cause otherwise we break tons of use cases which require concurrent access to the underlying buffer. When a device driver needs explicit ownership transfer it's perfectly possible to implement this using the dma_fence objects mentioned above. E.g. drivers can already look at who is accessing a buffer currently and can even grab explicit ownership of it by adding their own dma_fence objects. The only exception is CPU based access, e.g. when something is written with the CPU a cache flush might be necessary and when something is read with the CPU a cache invalidation might be necessary. Okay, that's much clearer now, thanks for clarifying this. So we should be covered for the cache maintenance needs originating from CPU accesses already, +/- the broken cases which don't call the begin/end CPU access routines that I mentioned above. Similarly, for any ownership transfer between different DMA engines, we should be covered either by the userspace explicitly flushing the hardware pipeline or attaching a DMA-buf fence to the buffer. But then, what's left to be solved? :) (Besides the cases of missing begin/end CPU access calls.) Well there are multiple problems here: 1. A lot of userspace applications/frameworks assume that it can allocate the buffer anywhere and it just works. I know you have said that about 10 times, perhaps I'm about to believe it, but why do you think userspace assumes this ? Did you actually read code that does this (that isn't meant to run on controlled environment). Yes, absolutely. Kodi for example used to do this and it was actually me who pointed out that this whole approach is flawed in the first place. And we had tons of customer projects which ran into trouble with that. It became better in the last few years, but only after pushing back on this many many times. Regards, Christian. And can you provide some example of broken generic userspace ? The DMABuf flow is meant to be trial and error. At least in GStreamer, yes, mostly only device allocation (when genericly usable) is implemented, but the code that has been contribute will try and fallback back like documented. Still fails sometimes, but that's exactly the kind of kernel bugs your patchset is trying to address. I don't blame anyone here, since why would folks on GStreamer/FFMPEG or any other "generic media framework" spend so much time implement "per linux device code", when non- embedded (constraint) linux is just handful of users (compare to Windows, Android, iOS users). To me, this shouldn't be #1 issue. Perhaps it should simply be replaced by userspace not supporting DMABuf Heaps. Perhaps add that Linux distribution don't always enable (or allow normal users to access) heaps (though you point 2. gets in the way) ? Unlike virtual memory, I don't think there is very good accounting and reclaiming mechanism for that memory, hence opening these means any userspace could possibly impair the system functioning. If you can't e.g. limit their usage within containers, this is pretty difficult for generic linux to carry. This is a wider problem of course, which likely affect a lot of GPU usage too, but perhaps it should be in the lower priority part of the todo. This isn't true at all, we have tons of cases where device can only access their special memory for certain use cases. Just look at scanout for displaying on dGPU, neither AMD nor NVidia supports system memory here. Similar cases exists for audio/video codecs where intermediate memory is only accessible by certain devices because of content protection. nit: content protection is not CODEC specific, its a platform feature, its also not really a thing upstream yet from what I'm aware of. This needs unified design and documentation imho, but also enough standardisation so that a generic application can use it. Right now, content protection people have been complaining that V4L2 (and most generic userspace) don't work with their design, rather then trying to figure-out a design that works with existing API. 2. We don't properly communicate allocation requirements to userspace. E.g. even if you allocate from DMA-Heaps userspace can currently only guess if normal, CMA or even device specific memory is needed. 3. We seem to lack some essential parts of those restrictions in the documentation. Agreed (can't always disagree). regards, Nicolas So if a device driver uses cached system memory on an architecture which devices which can't access it the right approach is clearly to reject the access. I'd like to accent the fact that "requires cache maintenance" != "can't access". Well that depends. As said above the exporter exports the buffer as it was allocated. If
Re: Try to address the DMA-buf coherency problem
Le lundi 05 décembre 2022 à 09:28 +0100, Christian König a écrit : > Hi Tomasz, > > Am 05.12.22 um 07:41 schrieb Tomasz Figa: > > [SNIP] > > > In other words explicit ownership transfer is not something we would > > > want as requirement in the framework, cause otherwise we break tons of > > > use cases which require concurrent access to the underlying buffer. > > > > > > When a device driver needs explicit ownership transfer it's perfectly > > > possible to implement this using the dma_fence objects mentioned above. > > > E.g. drivers can already look at who is accessing a buffer currently and > > > can even grab explicit ownership of it by adding their own dma_fence > > > objects. > > > > > > The only exception is CPU based access, e.g. when something is written > > > with the CPU a cache flush might be necessary and when something is read > > > with the CPU a cache invalidation might be necessary. > > > > > Okay, that's much clearer now, thanks for clarifying this. So we > > should be covered for the cache maintenance needs originating from CPU > > accesses already, +/- the broken cases which don't call the begin/end > > CPU access routines that I mentioned above. > > > > Similarly, for any ownership transfer between different DMA engines, > > we should be covered either by the userspace explicitly flushing the > > hardware pipeline or attaching a DMA-buf fence to the buffer. > > > > But then, what's left to be solved? :) (Besides the cases of missing > > begin/end CPU access calls.) > > Well there are multiple problems here: > > 1. A lot of userspace applications/frameworks assume that it can > allocate the buffer anywhere and it just works. I know you have said that about 10 times, perhaps I'm about to believe it, but why do you think userspace assumes this ? Did you actually read code that does this (that isn't meant to run on controlled environment). And can you provide some example of broken generic userspace ? The DMABuf flow is meant to be trial and error. At least in GStreamer, yes, mostly only device allocation (when genericly usable) is implemented, but the code that has been contribute will try and fallback back like documented. Still fails sometimes, but that's exactly the kind of kernel bugs your patchset is trying to address. I don't blame anyone here, since why would folks on GStreamer/FFMPEG or any other "generic media framework" spend so much time implement "per linux device code", when non- embedded (constraint) linux is just handful of users (compare to Windows, Android, iOS users). To me, this shouldn't be #1 issue. Perhaps it should simply be replaced by userspace not supporting DMABuf Heaps. Perhaps add that Linux distribution don't always enable (or allow normal users to access) heaps (though you point 2. gets in the way) ? Unlike virtual memory, I don't think there is very good accounting and reclaiming mechanism for that memory, hence opening these means any userspace could possibly impair the system functioning. If you can't e.g. limit their usage within containers, this is pretty difficult for generic linux to carry. This is a wider problem of course, which likely affect a lot of GPU usage too, but perhaps it should be in the lower priority part of the todo. > > This isn't true at all, we have tons of cases where device can only > access their special memory for certain use cases. > Just look at scanout for displaying on dGPU, neither AMD nor NVidia > supports system memory here. Similar cases exists for audio/video codecs > where intermediate memory is only accessible by certain devices because > of content protection. nit: content protection is not CODEC specific, its a platform feature, its also not really a thing upstream yet from what I'm aware of. This needs unified design and documentation imho, but also enough standardisation so that a generic application can use it. Right now, content protection people have been complaining that V4L2 (and most generic userspace) don't work with their design, rather then trying to figure-out a design that works with existing API. > > 2. We don't properly communicate allocation requirements to userspace. > > E.g. even if you allocate from DMA-Heaps userspace can currently only > guess if normal, CMA or even device specific memory is needed. > > 3. We seem to lack some essential parts of those restrictions in the > documentation. Agreed (can't always disagree). regards, Nicolas > > > > > > So if a device driver uses cached system memory on an architecture > > > > > which > > > > > devices which can't access it the right approach is clearly to reject > > > > > the access. > > > > I'd like to accent the fact that "requires cache maintenance" != "can't > > > > access". > > > Well that depends. As said above the exporter exports the buffer as it > > > was allocated. > > > > > > If that means the the exporter provides a piece of memory which requires > > > CPU cache snooping to access correctly then the best
Re: (subset) [PATCH v3 1/2] dt-bindings: backlight: qcom-wled: Add PMI8950 compatible
On Tue, 1 Nov 2022 17:17:59 +0100, Luca Weiss wrote: > Document the compatible for the wled block found in PMI8950. > > Applied, thanks! [2/2] arm64: dts: qcom: Add configuration for PMI8950 peripheral commit: 0d97fdf380b478c358c94f50f1b942e87f407b9b Best regards, -- Bjorn Andersson
Re: (subset) [PATCH v2 00/12] SM6115 DTS changes
On Wed, 30 Nov 2022 21:09:38 +0100, Adam Skladowski wrote: > This patch series adds bunch of new nodes > also it fixes some small nitpicks in yamls and adds compatible. > > Changes since v1 > > 1. Changed title for mdss yaml patch > 2. Added missing dmas to spi0 > 3. Wired freq domains to CPUs > 4. Added R-b/Ack tags > 5. Reworded smmu dts patch > > [...] Applied, thanks! [03/12] arm64: dts: qcom: sm6115: Add cpufreq-hw support commit: aff96846c63ed3e3ed7d5212ea636a422d9694a3 [04/12] arm64: dts: qcom: sm6115: Add TSENS node commit: 7b74cba6b13f4bbe1f15e3417f386ed1907ab0ef [05/12] arm64: dts: qcom: sm6115: Add PRNG node commit: fc676b15c065b8d4c750bbaab9914f24829a7a13 [06/12] arm64: dts: qcom: sm6115: Add rpm-stats node commit: d18c0077963ae2b6d232f6f3f25fb1ceb875ce7f [07/12] arm64: dts: qcom: sm6115: Add dispcc node commit: 884f95411ba4030ca44436217c6d8df4a960c555 [08/12] arm64: dts: qcom: sm6115: Add mdss/dpu node commit: 705e50427d8148211ffd05922bfa6a2520781338 [09/12] arm64: dts: qcom: sm6115: Add GPI DMA commit: 1586c5793511d7fb389139ab7aa5dae9118666ad [10/12] arm64: dts: qcom: sm6115: Add i2c/spi nodes commit: 323647d32e83fae7f1a81b40e12ca6b0b63e880c [11/12] arm64: dts: qcom: sm6115: Add WCN node. commit: 245bb9a37c16dc324be60764aa2597aa4704a8e3 [12/12] arm64: dts: qcom: sm6115: Add smmu fallback to qcom generic compatible commit: 58a9e83605478e931139b574e43d453851de3a26 Best regards, -- Bjorn Andersson
Re: [PATCH] dma-buf: fix dma_buf_export init order
Am 06.12.22 um 17:20 schrieb Ruhl, Michael J:
-Original Message-
From: dri-devel On Behalf Of
Christian König
Sent: Tuesday, December 6, 2022 10:12 AM
To: quic_chara...@quicinc.com; cuigaoshe...@huawei.com;
tjmerc...@google.com; sumit.sem...@linaro.org
Cc: linaro-mm-...@lists.linaro.org; dri-devel@lists.freedesktop.org; linux-
me...@vger.kernel.org
Subject: [PATCH] dma-buf: fix dma_buf_export init order
The init order and resulting error handling in dma_buf_export
was pretty messy.
Subordinate objects like the file and the sysfs kernel objects
were initializing and wiring itself up with the object in the
wrong order resulting not only in complicating and partially
incorrect error handling, but also in publishing only halve
initialized DMA-buf objects.
Clean this up thoughtfully by allocating the file independent
of the DMA-buf object. Then allocate and initialize the DMA-buf
object itself, before publishing it through sysfs. If everything
works as expected the file is then connected with the DMA-buf
object and publish it through debugfs.
Also adds the missing dma_resv_fini() into the error handling.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-buf-sysfs-stats.c | 7 +--
drivers/dma-buf/dma-buf-sysfs-stats.h | 4 +-
drivers/dma-buf/dma-buf.c | 65 +--
3 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-
buf-sysfs-stats.c
index 2bba0babcb62..4b680e10c15a 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
@@ -168,14 +168,11 @@ void dma_buf_uninit_sysfs_statistics(void)
kset_unregister(dma_buf_stats_kset);
}
-int dma_buf_stats_setup(struct dma_buf *dmabuf)
+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file)
{
struct dma_buf_sysfs_entry *sysfs_entry;
int ret;
- if (!dmabuf || !dmabuf->file)
- return -EINVAL;
-
if (!dmabuf->exp_name) {
pr_err("exporter name must not be empty if stats
needed\n");
return -EINVAL;
@@ -192,7 +189,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
/* create the directory for buffer stats */
ret = kobject_init_and_add(_entry->kobj, _buf_ktype,
NULL,
- "%lu", file_inode(dmabuf->file)->i_ino);
+ "%lu", file_inode(file)->i_ino);
if (ret)
goto err_sysfs_dmabuf;
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h b/drivers/dma-buf/dma-
buf-sysfs-stats.h
index a49c6e2650cc..7a8a995b75ba 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.h
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.h
@@ -13,7 +13,7 @@
int dma_buf_init_sysfs_statistics(void);
void dma_buf_uninit_sysfs_statistics(void);
-int dma_buf_stats_setup(struct dma_buf *dmabuf);
+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file);
void dma_buf_stats_teardown(struct dma_buf *dmabuf);
#else
@@ -25,7 +25,7 @@ static inline int dma_buf_init_sysfs_statistics(void)
static inline void dma_buf_uninit_sysfs_statistics(void) {}
-static inline int dma_buf_stats_setup(struct dma_buf *dmabuf)
+static inline int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file
*file)
{
return 0;
}
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e6f36c014c4c..ea08049b70ae 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -614,19 +614,11 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
size_t alloc_size = sizeof(struct dma_buf);
int ret;
- if (!exp_info->resv)
- alloc_size += sizeof(struct dma_resv);
- else
- /* prevent _buf[1] == dma_buf->resv */
- alloc_size += 1;
-
- if (WARN_ON(!exp_info->priv
- || !exp_info->ops
- || !exp_info->ops->map_dma_buf
- || !exp_info->ops->unmap_dma_buf
- || !exp_info->ops->release)) {
+ if (WARN_ON(!exp_info->priv || !exp_info->ops
+ || !exp_info->ops->map_dma_buf
+ || !exp_info->ops->unmap_dma_buf
+ || !exp_info->ops->release))
return ERR_PTR(-EINVAL);
- }
if (WARN_ON(exp_info->ops->cache_sgt_mapping &&
(exp_info->ops->pin || exp_info->ops->unpin)))
@@ -638,10 +630,21 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
if (!try_module_get(exp_info->owner))
return ERR_PTR(-ENOENT);
+ file = dma_buf_getfile(exp_info->size, exp_info->flags);
Hi Christian,
dma_buf_getfile takes a dmabuf, here you have a size?
Did you change this function somewhere?
Ups forgot to add that change to the patch. I shouldn't code when I'm in
a hurry.
Addressed this and Charans comment in v2.
Thanks,
Christian.
with that
Re: [PATCH v2 1/2] drm/shmem-helper: Remove errant put in error path
On Sun, Dec 4, 2022 at 12:45 PM Dmitry Osipenko
wrote:
>
> On 11/30/22 21:57, Rob Clark wrote:
> > From: Rob Clark
> >
> > drm_gem_shmem_mmap() doesn't own this reference, resulting in the GEM
> > object getting prematurely freed leading to a later use-after-free.
> >
> > Link: https://syzkaller.appspot.com/bug?extid=c8ae65286134dd1b800d
> > Reported-by: syzbot+c8ae65286134dd1b8...@syzkaller.appspotmail.com
> > Fixes: 2194a63a818d ("drm: Add library for shmem backed GEM objects")
> > Cc: sta...@vger.kernel.org
> > Signed-off-by: Rob Clark
> > Reviewed-by: Daniel Vetter
> > ---
> > drivers/gpu/drm/drm_gem_shmem_helper.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > index 35138f8a375c..3b7b71391a4c 100644
> > --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> > @@ -622,10 +622,8 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object
> > *shmem, struct vm_area_struct
> > }
> >
> > ret = drm_gem_shmem_get_pages(shmem);
> > - if (ret) {
> > - drm_gem_vm_close(vma);
> > + if (ret)
> > return ret;
> > - }
> >
> > vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
> > vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
>
> AFAICS, the dmabuf mmaping code path needs a similar fix, isn't it?
>
> - /* Drop the reference drm_gem_mmap_obj() acquired.*/
> - drm_gem_object_put(obj);
> vma->vm_private_data = NULL;
>
> - return dma_buf_mmap(obj->dma_buf, vma, 0);
> + ret = dma_buf_mmap(obj->dma_buf, vma, 0);
> +
> + /* Drop the reference drm_gem_mmap_obj() acquired.*/
> + if (!ret)
> + drm_gem_object_put(obj);
> +
> + return ret;
>
Yes, it seems that way.. I wish the shmem helpers worked in a less
"special" way with regards to refcnting :-(
BR,
-R
Re: [Intel-gfx] [PATCH 7/9] drm/i915: stop using ttm_bo_wait
Am 06.12.22 um 19:03 schrieb Matthew Auld:
On 05/12/2022 19:58, Christian König wrote:
Am 30.11.22 um 15:06 schrieb Daniel Vetter:
On Wed, 30 Nov 2022 at 14:03, Tvrtko Ursulin
wrote:
On 29/11/2022 18:05, Matthew Auld wrote:
On Fri, 25 Nov 2022 at 11:14, Tvrtko Ursulin
wrote:
+ Matt
On 25/11/2022 10:21, Christian König wrote:
TTM is just wrapping core DMA functionality here, remove the
mid-layer.
No functional change.
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 5247d88b3c13..d409a77449a3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -599,13 +599,16 @@ i915_ttm_resource_get_st(struct
drm_i915_gem_object *obj,
static int i915_ttm_truncate(struct drm_i915_gem_object *obj)
{
struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
- int err;
+ long err;
WARN_ON_ONCE(obj->mm.madv == I915_MADV_WILLNEED);
- err = ttm_bo_wait(bo, true, false);
- if (err)
+ err = dma_resv_wait_timeout(bo->base.resv,
DMA_RESV_USAGE_BOOKKEEP,
+ true, 15 * HZ);
This 15 second stuck out a bit for me and then on a slightly
deeper look
it seems this timeout will "leak" into a few of i915 code paths.
If we
look at the difference between the legacy shmem and ttm backend I
am not
sure if the legacy one is blocking or not - but if it can block I
don't
think it would have an arbitrary timeout like this. Matt your
thoughts?
Not sure what is meant by leak here, but the legacy shmem must also
wait/block when unbinding each VMA, before calling truncate. It's the
By "leak" I meant if 15s timeout propagates into some code paths
visible
from userspace which with a legacy backend instead have an indefinite
wait. If we have that it's probably not very good to have this
inconsistency, or to apply an arbitrary timeout to those path to
start with.
same story for the ttm backend, except slightly more complicated in
that there might be no currently bound VMA, and yet the GPU could
still be accessing the pages due to async unbinds, kernel moves etc,
which the wait here (and in i915_ttm_shrink) is meant to protect
against. If the wait times out it should just fail gracefully. I
guess
we could just use MAX_SCHEDULE_TIMEOUT here? Not sure if it really
matters though.
Right, depends if it can leak or not to userspace and diverge between
backends.
Generally lock_timeout() is a design bug. It's either
lock_interruptible (or maybe lock_killable) or try_lock, but
lock_timeout is just duct-tape. I haven't dug in to figure out what
should be here, but it smells fishy.
Independent of this discussion could I get an rb for removing
ttm_bo_wait() from i915?
Exactly hiding this timeout inside TTM is what always made me quite
nervous here.
There are also a few places in i915 calling bo_wait_ctx(), which
appears to just wrap ttm_bo_wait(). I guess that should also be
converted to dma_resv_wait_timeout()? Or what is the story with that?
If you don't want the ctx timeout then yes, calling
dma_resv_wait_timeout() instead is the right approach.
Anyway,
Reviewed-by: Matthew Auld
Thanks, going to push this through drm-misc-next.
Regards,
Christian.
Regards,
Christian.
-Daniel
Re: [Intel-gfx] [PATCH 7/9] drm/i915: stop using ttm_bo_wait
On 05/12/2022 19:58, Christian König wrote:
Am 30.11.22 um 15:06 schrieb Daniel Vetter:
On Wed, 30 Nov 2022 at 14:03, Tvrtko Ursulin
wrote:
On 29/11/2022 18:05, Matthew Auld wrote:
On Fri, 25 Nov 2022 at 11:14, Tvrtko Ursulin
wrote:
+ Matt
On 25/11/2022 10:21, Christian König wrote:
TTM is just wrapping core DMA functionality here, remove the
mid-layer.
No functional change.
Signed-off-by: Christian König
---
drivers/gpu/drm/i915/gem/i915_gem_ttm.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 5247d88b3c13..d409a77449a3 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -599,13 +599,16 @@ i915_ttm_resource_get_st(struct
drm_i915_gem_object *obj,
static int i915_ttm_truncate(struct drm_i915_gem_object *obj)
{
struct ttm_buffer_object *bo = i915_gem_to_ttm(obj);
- int err;
+ long err;
WARN_ON_ONCE(obj->mm.madv == I915_MADV_WILLNEED);
- err = ttm_bo_wait(bo, true, false);
- if (err)
+ err = dma_resv_wait_timeout(bo->base.resv,
DMA_RESV_USAGE_BOOKKEEP,
+ true, 15 * HZ);
This 15 second stuck out a bit for me and then on a slightly deeper
look
it seems this timeout will "leak" into a few of i915 code paths. If we
look at the difference between the legacy shmem and ttm backend I
am not
sure if the legacy one is blocking or not - but if it can block I
don't
think it would have an arbitrary timeout like this. Matt your
thoughts?
Not sure what is meant by leak here, but the legacy shmem must also
wait/block when unbinding each VMA, before calling truncate. It's the
By "leak" I meant if 15s timeout propagates into some code paths visible
from userspace which with a legacy backend instead have an indefinite
wait. If we have that it's probably not very good to have this
inconsistency, or to apply an arbitrary timeout to those path to
start with.
same story for the ttm backend, except slightly more complicated in
that there might be no currently bound VMA, and yet the GPU could
still be accessing the pages due to async unbinds, kernel moves etc,
which the wait here (and in i915_ttm_shrink) is meant to protect
against. If the wait times out it should just fail gracefully. I guess
we could just use MAX_SCHEDULE_TIMEOUT here? Not sure if it really
matters though.
Right, depends if it can leak or not to userspace and diverge between
backends.
Generally lock_timeout() is a design bug. It's either
lock_interruptible (or maybe lock_killable) or try_lock, but
lock_timeout is just duct-tape. I haven't dug in to figure out what
should be here, but it smells fishy.
Independent of this discussion could I get an rb for removing
ttm_bo_wait() from i915?
Exactly hiding this timeout inside TTM is what always made me quite
nervous here.
There are also a few places in i915 calling bo_wait_ctx(), which appears
to just wrap ttm_bo_wait(). I guess that should also be converted to
dma_resv_wait_timeout()? Or what is the story with that?
Anyway,
Reviewed-by: Matthew Auld
Regards,
Christian.
-Daniel
[Bug 216780] problem of Mesa drawing boxes after resume from suspend with Geforce4 420 Go
https://bugzilla.kernel.org/show_bug.cgi?id=216780 --- Comment #5 from Elmar Stellnberger (estel...@elstel.org) --- done: https://gitlab.freedesktop.org/drm/nouveau/-/issues/194 -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [Intel-gfx] [PATCH v8 22/22] drm/i915/vm_bind: Support capture of persistent mappings
On 01/12/2022 18:43, Niranjana Vishwanathapura wrote:
On Thu, Dec 01, 2022 at 07:27:31AM -0800, Niranjana Vishwanathapura wrote:
On Thu, Dec 01, 2022 at 10:49:15AM +, Matthew Auld wrote:
On 29/11/2022 07:26, Niranjana Vishwanathapura wrote:
Support dump capture of persistent mappings upon user request.
Signed-off-by: Brian Welty
Signed-off-by: Niranjana Vishwanathapura
---
.../drm/i915/gem/i915_gem_vm_bind_object.c | 11 +++
drivers/gpu/drm/i915/gt/intel_gtt.c | 3 +++
drivers/gpu/drm/i915/gt/intel_gtt.h | 5 +
drivers/gpu/drm/i915/i915_gpu_error.c | 19 +++
drivers/gpu/drm/i915/i915_vma.c | 1 +
drivers/gpu/drm/i915/i915_vma_types.h | 2 ++
include/uapi/drm/i915_drm.h | 3 ++-
7 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
index 78e7c0642c5f..50969613daf6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_vm_bind_object.c
@@ -88,6 +88,11 @@ static void i915_gem_vm_bind_remove(struct
i915_vma *vma, bool release_obj)
{
lockdep_assert_held(>vm->vm_bind_lock);
+ spin_lock(>vm->vm_capture_lock);
+ if (!list_empty(>vm_capture_link))
+ list_del_init(>vm_capture_link);
+ spin_unlock(>vm->vm_capture_lock);
+
spin_lock(>vm->vm_rebind_lock);
if (!list_empty(>vm_rebind_link))
list_del_init(>vm_rebind_link);
@@ -357,6 +362,12 @@ static int i915_gem_vm_bind_obj(struct
i915_address_space *vm,
continue;
}
+ if (va->flags & I915_GEM_VM_BIND_CAPTURE) {
+ spin_lock(>vm_capture_lock);
+ list_add_tail(>vm_capture_link,
>vm_capture_list);
+ spin_unlock(>vm_capture_lock);
+ }
+
list_add_tail(>vm_bind_link, >vm_bound_list);
i915_vm_bind_it_insert(vma, >va);
if (!obj->priv_root)
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index ebf6830574a0..bdabe13fc30e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -297,6 +297,9 @@ void i915_address_space_init(struct
i915_address_space *vm, int subclass)
spin_lock_init(>vm_rebind_lock);
spin_lock_init(>userptr_invalidated_lock);
INIT_LIST_HEAD(>userptr_invalidated_list);
+
+ INIT_LIST_HEAD(>vm_capture_list);
+ spin_lock_init(>vm_capture_lock);
}
void *__px_vaddr(struct drm_i915_gem_object *p)
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 87e5b6568a00..8e4ddd073348 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -281,6 +281,11 @@ struct i915_address_space {
/** @root_obj: root object for dma-resv sharing by private
objects */
struct drm_i915_gem_object *root_obj;
+ /* @vm_capture_list: list of vm captures */
+ struct list_head vm_capture_list;
+ /* @vm_capture_lock: protects vm_capture_list */
+ spinlock_t vm_capture_lock;
+
/* Global GTT */
bool is_ggtt:1;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
b/drivers/gpu/drm/i915/i915_gpu_error.c
index 9d5d5a397b64..3b2b12a739f7 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1460,6 +1460,22 @@ capture_vma(struct intel_engine_capture_vma
*next,
return next;
}
+static struct intel_engine_capture_vma *
+capture_user_vm(struct intel_engine_capture_vma *capture,
+ struct i915_address_space *vm, gfp_t gfp)
+{
+ struct i915_vma *vma;
+
+ spin_lock(>vm_capture_lock);
Does it make sense to move this into the eb3 submission stage, like
we do for eb2? IIRC the gfp flags here are quite limiting due to
potentially being in a fence critical section. If we can use
rq->capture_list for eb3, we shouldn't need to change much here?
But that will add latency on submission path as we will have to iterate
over capture_list in each submission. Besides, unlike eb2 case, we can't
just transfer the list to rq->capture_list as we will have to do this
for each submission. It was discussed long time back and decided not to
bother the fast path (submision) scenario with this error capture which
is only required upon gpu hang (slow path).
Maybe add some of this to the commit message, just to give a bit more
back story/history. From my pov I'm coming into this semi-blind.
Also there is the existing CONFIG_DRM_I915_CAPTURE_ERROR. Should we
take that into account?
Ok, will fix.
+ /* vma->resource must be valid here as persistent vmas are
bound */
+ list_for_each_entry(vma, >vm_capture_list, vm_capture_link)
+ capture = capture_vma_snapshot(capture, vma->resource,
+ gfp, "user");
+ spin_unlock(>vm_capture_lock);
+
+ return capture;
+}
+
static struct intel_engine_capture_vma *
Re: [PATCH 2/7] media: Add Y210, Y212 and Y216 formats
Hi,
Le mardi 06 décembre 2022 à 15:39 +0200, Tomi Valkeinen a écrit :
> Add Y210, Y212 and Y216 formats.
>
> Signed-off-by: Tomi Valkeinen
This patch is simply missing an update to:
Documentation/userspace-api/media/v4l/yuv-formats.rst
regards,
Nicolas
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
> include/uapi/linux/videodev2.h | 8
> 2 files changed, 11 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 964300deaf62..ba95389a59b5 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1449,6 +1449,9 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> case V4L2_META_FMT_RK_ISP1_STAT_3A: descr = "Rockchip ISP1 3A
> Statistics"; break;
> case V4L2_PIX_FMT_NV12M_8L128: descr = "NV12M (8x128 Linear)"; break;
> case V4L2_PIX_FMT_NV12M_10BE_8L128: descr = "10-bit NV12M (8x128
> Linear, BE)"; break;
> + case V4L2_PIX_FMT_Y210: descr = "10-bit YUYV Packed"; break;
> + case V4L2_PIX_FMT_Y212: descr = "12-bit YUYV Packed"; break;
> + case V4L2_PIX_FMT_Y216: descr = "16-bit YUYV Packed"; break;
>
> default:
> /* Compressed formats */
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 877fd61693b8..15b640d2da8a 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -621,6 +621,14 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_YUVX32 v4l2_fourcc('Y', 'U', 'V', 'X') /* 32
> YUVX-8-8-8-8 */
> #define V4L2_PIX_FMT_M420v4l2_fourcc('M', '4', '2', '0') /* 12 YUV
> 4:2:0 2 lines y, 1 line uv interleaved */
>
> +/*
> + * YCbCr packed format. For each Y2xx format, xx bits of valid data occupy
> the MSBs
> + * of the 16 bit components, and 16-xx bits of zero padding occupy the LSBs.
> + */
> +#define V4L2_PIX_FMT_Y210v4l2_fourcc('Y', '2', '1', '0') /* 32 YUYV
> 4:2:2 */
> +#define V4L2_PIX_FMT_Y212v4l2_fourcc('Y', '2', '1', '2') /* 32 YUYV
> 4:2:2 */
> +#define V4L2_PIX_FMT_Y216v4l2_fourcc('Y', '2', '1', '6') /* 32 YUYV
> 4:2:2 */
> +
> /* two planes -- one Y, one Cr + Cb interleaved */
> #define V4L2_PIX_FMT_NV12v4l2_fourcc('N', 'V', '1', '2') /* 12 Y/CbCr
> 4:2:0 */
> #define V4L2_PIX_FMT_NV21v4l2_fourcc('N', 'V', '2', '1') /* 12 Y/CrCb
> 4:2:0 */
Re: [PATCH 1/7] media: Add 2-10-10-10 RGB formats
Hi,
Le mardi 06 décembre 2022 à 15:39 +0200, Tomi Valkeinen a écrit :
> Add XBGR2101010, ABGR2101010 and BGRA1010102 formats.
>
> Signed-off-by: Tomi Valkeinen
This patch is simply missing an update to
Documentation/userspace-api/media/v4l/pixfmt-rgb.rst
regards,
Nicolas
> ---
> drivers/media/v4l2-core/v4l2-ioctl.c | 3 +++
> include/uapi/linux/videodev2.h | 3 +++
> 2 files changed, 6 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> b/drivers/media/v4l2-core/v4l2-ioctl.c
> index fddba75d9074..964300deaf62 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1304,6 +1304,9 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
> case V4L2_PIX_FMT_BGRX32: descr = "32-bit XBGR 8-8-8-8"; break;
> case V4L2_PIX_FMT_RGBA32: descr = "32-bit RGBA 8-8-8-8"; break;
> case V4L2_PIX_FMT_RGBX32: descr = "32-bit RGBX 8-8-8-8"; break;
> + case V4L2_PIX_FMT_XBGR2101010: descr = "32-bit XBGR 2-10-10-10"; break;
> + case V4L2_PIX_FMT_ABGR2101010: descr = "32-bit ABGR 2-10-10-10"; break;
> + case V4L2_PIX_FMT_BGRA1010102: descr = "32-bit BGRA 10-10-10-2"; break;
> case V4L2_PIX_FMT_GREY: descr = "8-bit Greyscale"; break;
> case V4L2_PIX_FMT_Y4: descr = "4-bit Greyscale"; break;
> case V4L2_PIX_FMT_Y6: descr = "6-bit Greyscale"; break;
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 29da1f4b4578..877fd61693b8 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -576,6 +576,9 @@ struct v4l2_pix_format {
> #define V4L2_PIX_FMT_RGBX32 v4l2_fourcc('X', 'B', '2', '4') /* 32
> RGBX-8-8-8-8 */
> #define V4L2_PIX_FMT_ARGB32 v4l2_fourcc('B', 'A', '2', '4') /* 32
> ARGB-8-8-8-8 */
> #define V4L2_PIX_FMT_XRGB32 v4l2_fourcc('B', 'X', '2', '4') /* 32
> XRGB-8-8-8-8 */
> +#define V4L2_PIX_FMT_XBGR2101010 v4l2_fourcc('R', 'X', '3', '0') /* 32
> XBGR-2-10-10-10 */
> +#define V4L2_PIX_FMT_ABGR2101010 v4l2_fourcc('R', 'A', '3', '0') /* 32
> ABGR-2-10-10-10 */
> +#define V4L2_PIX_FMT_BGRA1010102 v4l2_fourcc('A', 'R', '3', '0') /* 32
> BGRA-10-10-10-2 */
>
> /* Grey formats */
> #define V4L2_PIX_FMT_GREYv4l2_fourcc('G', 'R', 'E', 'Y') /* 8
> Greyscale */
[Bug 216780] problem of Mesa drawing boxes after resume from suspend with Geforce4 420 Go
https://bugzilla.kernel.org/show_bug.cgi?id=216780 Artem S. Tashkinov (a...@gmx.com) changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |ANSWERED --- Comment #4 from Artem S. Tashkinov (a...@gmx.com) --- Please report here instead https://gitlab.freedesktop.org/drm/nouveau/-/issues/ -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH] dma-buf: fix dma_buf_export init order
Thanks Christian for this nice cleanup!!
On 12/6/2022 8:42 PM, Christian König wrote:
> @@ -638,10 +630,21 @@ struct dma_buf *dma_buf_export(const struct
> dma_buf_export_info *exp_info)
> if (!try_module_get(exp_info->owner))
> return ERR_PTR(-ENOENT);
>
> + file = dma_buf_getfile(exp_info->size, exp_info->flags);
> + if (IS_ERR(file)) {
> + ret = PTR_ERR(file);
> + goto err_module;
> + }
> +
> + if (!exp_info->resv)
> + alloc_size += sizeof(struct dma_resv);
> + else
> + /* prevent _buf[1] == dma_buf->resv */
> + alloc_size += 1;
> dmabuf = kzalloc(alloc_size, GFP_KERNEL);
> if (!dmabuf) {
> ret = -ENOMEM;
> - goto err_module;
> + goto err_file;
> }
>
> dmabuf->priv = exp_info->priv;
> @@ -653,44 +656,36 @@ struct dma_buf *dma_buf_export(const struct
> dma_buf_export_info *exp_info)
> init_waitqueue_head(>poll);
> dmabuf->cb_in.poll = dmabuf->cb_out.poll = >poll;
> dmabuf->cb_in.active = dmabuf->cb_out.active = 0;
> + mutex_init(>lock);
> + INIT_LIST_HEAD(>attachments);
>
> if (!resv) {
> - resv = (struct dma_resv *)[1];
> - dma_resv_init(resv);
> + dmabuf->resv = (struct dma_resv *)[1];
> + dma_resv_init(dmabuf->resv);
> + } else {
> + dmabuf->resv = resv;
> }
> - dmabuf->resv = resv;
>
> - file = dma_buf_getfile(dmabuf, exp_info->flags);
> - if (IS_ERR(file)) {
> - ret = PTR_ERR(file);
> + ret = dma_buf_stats_setup(dmabuf, file);
> + if (ret)
> goto err_dmabuf;
> - }
>
> + file->private_data = dmabuf;
> + file->f_path.dentry->d_fsdata = dmabuf;
> dmabuf->file = file;
>
> - mutex_init(>lock);
> - INIT_LIST_HEAD(>attachments);
> -
> mutex_lock(_list.lock);
> list_add(>list_node, _list.head);
> mutex_unlock(_list.lock);
>
> - ret = dma_buf_stats_setup(dmabuf);
> - if (ret)
> - goto err_sysfs;
> -
> return dmabuf;
>
> -err_sysfs:
> - /*
> - * Set file->f_path.dentry->d_fsdata to NULL so that when
> - * dma_buf_release() gets invoked by dentry_ops, it exits
> - * early before calling the release() dma_buf op.
> - */
> - file->f_path.dentry->d_fsdata = NULL;
> - fput(file);
> err_dmabuf:
> + if (!resv)
> + dma_resv_fini(dmabuf->resv);
> kfree(dmabuf);
> +err_file:
> + fput(file);
This fput() still endup in calling the dma_buf_file_release() where
there are no checks before accessing the file->private_data(=dmabuf)
which can result in null pointer access. Am I missing something trivial?
> err_module:
> module_put(exp_info->owner);
> return ERR_PTR(ret);
> -- 2.34.1
[Bug 216780] problem of Mesa drawing boxes after resume from suspend with Geforce4 420 Go
https://bugzilla.kernel.org/show_bug.cgi?id=216780 --- Comment #3 from Elmar Stellnberger (estel...@elstel.org) --- Created attachment 303371 --> https://bugzilla.kernel.org/attachment.cgi?id=303371=edit before resume from s2ram: background box of login screen is drawn correctly just tested:: same issue with suspend to disk -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 216780] problem of Mesa drawing boxes after resume from suspend with Geforce4 420 Go
https://bugzilla.kernel.org/show_bug.cgi?id=216780 --- Comment #2 from Elmar Stellnberger (estel...@elstel.org) --- Created attachment 303370 --> https://bugzilla.kernel.org/attachment.cgi?id=303370=edit after resume from s2ram: the background box of the login screen is not drawn -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 216780] problem of Mesa drawing boxes after resume from suspend with Geforce4 420 Go
https://bugzilla.kernel.org/show_bug.cgi?id=216780 --- Comment #1 from Elmar Stellnberger (estel...@elstel.org) --- Created attachment 303369 --> https://bugzilla.kernel.org/attachment.cgi?id=303369=edit after resume from s2ram: screen distortions in the status tray and on moving the Pidgin window -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 216780] New: problem of Mesa drawing boxes after resume from suspend with Geforce4 420 Go
https://bugzilla.kernel.org/show_bug.cgi?id=216780 Bug ID: 216780 Summary: problem of Mesa drawing boxes after resume from suspend with Geforce4 420 Go Product: Drivers Version: 2.5 Kernel Version: 5.15.79-desktop586 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: estel...@elstel.org Regression: No When it has finished to boot everything works correctly: I can move the Pidgin window or I can lock the screen: boxes are drawn correctly. Yet after resume from s2ram/suspend you can see a black box as the background of the status bar tray instead of a filled one, moving pidgin leads to box-shaped screen distortions and the background box of the login dialogue is not drawn any more. Using Mesa patches from Karol Herbst for gtk3 apps (https://gitlab.freedesktop.org/drm/nouveau/-/issues/174); xscreensaver as well as the drawing of the lxde status tray bar shall be independent from this however (non-gtk). kernel modules used for dri (as returned by lspci): rivafb, nvidiafb, nouveau kernel: 5.15.79-desktop586 SMP i686 i386 GNU/Linux graphics card: NVIDIA Corporation NV17M [GeForce4 420 Go] (rev a3) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [PATCH] drm/msm/dpu: Add check for cstate
On 12/6/2022 12:05 AM, Jiasheng Jiang wrote:
As kzalloc may fail and return NULL pointer,
it should be better to check cstate
in order to avoid the NULL pointer dereference
in __drm_atomic_helper_crtc_reset.
You have wrapped around your lines too short. Please try to utilize the
full word limit for each line.
With that fixed,
Reviewed-by: Abhinav Kumar
Fixes: 1cff7440a86e ("drm/msm: Convert to using __drm_atomic_helper_crtc_reset() for
reset.")
Signed-off-by: Jiasheng Jiang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 13ce321283ff..22c2787b7b38 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -968,7 +968,10 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
if (crtc->state)
dpu_crtc_destroy_state(crtc, crtc->state);
- __drm_atomic_helper_crtc_reset(crtc, >base);
+ if (cstate)
+ __drm_atomic_helper_crtc_reset(crtc, >base);
+ else
+ __drm_atomic_helper_crtc_reset(crtc, NULL);
}
/**
RE: [PATCH] dma-buf: fix dma_buf_export init order
>-Original Message-
>From: dri-devel On Behalf Of
>Christian König
>Sent: Tuesday, December 6, 2022 10:12 AM
>To: quic_chara...@quicinc.com; cuigaoshe...@huawei.com;
>tjmerc...@google.com; sumit.sem...@linaro.org
>Cc: linaro-mm-...@lists.linaro.org; dri-devel@lists.freedesktop.org; linux-
>me...@vger.kernel.org
>Subject: [PATCH] dma-buf: fix dma_buf_export init order
>
>The init order and resulting error handling in dma_buf_export
>was pretty messy.
>
>Subordinate objects like the file and the sysfs kernel objects
>were initializing and wiring itself up with the object in the
>wrong order resulting not only in complicating and partially
>incorrect error handling, but also in publishing only halve
>initialized DMA-buf objects.
>
>Clean this up thoughtfully by allocating the file independent
>of the DMA-buf object. Then allocate and initialize the DMA-buf
>object itself, before publishing it through sysfs. If everything
>works as expected the file is then connected with the DMA-buf
>object and publish it through debugfs.
>
>Also adds the missing dma_resv_fini() into the error handling.
>
>Signed-off-by: Christian König
>---
> drivers/dma-buf/dma-buf-sysfs-stats.c | 7 +--
> drivers/dma-buf/dma-buf-sysfs-stats.h | 4 +-
> drivers/dma-buf/dma-buf.c | 65 +--
> 3 files changed, 34 insertions(+), 42 deletions(-)
>
>diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c b/drivers/dma-buf/dma-
>buf-sysfs-stats.c
>index 2bba0babcb62..4b680e10c15a 100644
>--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
>+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
>@@ -168,14 +168,11 @@ void dma_buf_uninit_sysfs_statistics(void)
> kset_unregister(dma_buf_stats_kset);
> }
>
>-int dma_buf_stats_setup(struct dma_buf *dmabuf)
>+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file)
> {
> struct dma_buf_sysfs_entry *sysfs_entry;
> int ret;
>
>- if (!dmabuf || !dmabuf->file)
>- return -EINVAL;
>-
> if (!dmabuf->exp_name) {
> pr_err("exporter name must not be empty if stats
>needed\n");
> return -EINVAL;
>@@ -192,7 +189,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
>
> /* create the directory for buffer stats */
> ret = kobject_init_and_add(_entry->kobj, _buf_ktype,
>NULL,
>- "%lu", file_inode(dmabuf->file)->i_ino);
>+ "%lu", file_inode(file)->i_ino);
> if (ret)
> goto err_sysfs_dmabuf;
>
>diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h b/drivers/dma-buf/dma-
>buf-sysfs-stats.h
>index a49c6e2650cc..7a8a995b75ba 100644
>--- a/drivers/dma-buf/dma-buf-sysfs-stats.h
>+++ b/drivers/dma-buf/dma-buf-sysfs-stats.h
>@@ -13,7 +13,7 @@
> int dma_buf_init_sysfs_statistics(void);
> void dma_buf_uninit_sysfs_statistics(void);
>
>-int dma_buf_stats_setup(struct dma_buf *dmabuf);
>+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file);
>
> void dma_buf_stats_teardown(struct dma_buf *dmabuf);
> #else
>@@ -25,7 +25,7 @@ static inline int dma_buf_init_sysfs_statistics(void)
>
> static inline void dma_buf_uninit_sysfs_statistics(void) {}
>
>-static inline int dma_buf_stats_setup(struct dma_buf *dmabuf)
>+static inline int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file
>*file)
> {
> return 0;
> }
>diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
>index e6f36c014c4c..ea08049b70ae 100644
>--- a/drivers/dma-buf/dma-buf.c
>+++ b/drivers/dma-buf/dma-buf.c
>@@ -614,19 +614,11 @@ struct dma_buf *dma_buf_export(const struct
>dma_buf_export_info *exp_info)
> size_t alloc_size = sizeof(struct dma_buf);
> int ret;
>
>- if (!exp_info->resv)
>- alloc_size += sizeof(struct dma_resv);
>- else
>- /* prevent _buf[1] == dma_buf->resv */
>- alloc_size += 1;
>-
>- if (WARN_ON(!exp_info->priv
>-|| !exp_info->ops
>-|| !exp_info->ops->map_dma_buf
>-|| !exp_info->ops->unmap_dma_buf
>-|| !exp_info->ops->release)) {
>+ if (WARN_ON(!exp_info->priv || !exp_info->ops
>+ || !exp_info->ops->map_dma_buf
>+ || !exp_info->ops->unmap_dma_buf
>+ || !exp_info->ops->release))
> return ERR_PTR(-EINVAL);
>- }
>
> if (WARN_ON(exp_info->ops->cache_sgt_mapping &&
> (exp_info->ops->pin || exp_info->ops->unpin)))
>@@ -638,10 +630,21 @@ struct dma_buf *dma_buf_export(const struct
>dma_buf_export_info *exp_info)
> if (!try_module_get(exp_info->owner))
> return ERR_PTR(-ENOENT);
>
>+ file = dma_buf_getfile(exp_info->size, exp_info->flags);
Hi Christian,
dma_buf_getfile takes a dmabuf, here you have a size?
Did you change this function somewhere?
with that addressed
This cleanup makes sense to me.
Reviewed-by: Michael J. Ruhl
M
>+ if
[linux-next:master] BUILD REGRESSION 5d562c48a21eeb029a8fd3f18e1b31fd83660474
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master branch HEAD: 5d562c48a21eeb029a8fd3f18e1b31fd83660474 Add linux-next specific files for 20221206 Error/Warning reports: https://lore.kernel.org/oe-kbuild-all/202211231857.0dmueoa1-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211242120.mzzvguln-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211290656.vhedfthu-...@intel.com https://lore.kernel.org/oe-kbuild-all/202211301840.y7rrob13-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212020520.0okmino3-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212032205.iehbbyyp-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212060700.njmecjxs-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212061249.u0basqzk-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212061341.gnalcbx6-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212061455.6ge7y0jg-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212061633.u9qhpe62-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212061758.tlpqnuof-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212061918.w5iupcya-...@intel.com https://lore.kernel.org/oe-kbuild-all/202212062250.tr0othcz-...@intel.com Error/Warning: (recently discovered and may have been fixed) Error: failed to load BTF from vmlinux: No such file or directory arch/loongarch/kernel/asm-offsets.c:262:6: warning: no previous prototype for 'output_pbe_defines' [-Wmissing-prototypes] arch/loongarch/power/hibernate.c:14:6: warning: no previous prototype for 'save_processor_state' [-Wmissing-prototypes] arch/loongarch/power/hibernate.c:26:6: warning: no previous prototype for 'restore_processor_state' [-Wmissing-prototypes] arch/loongarch/power/hibernate.c:38:5: warning: no previous prototype for 'pfn_is_nosave' [-Wmissing-prototypes] arch/loongarch/power/hibernate.c:48:5: warning: no previous prototype for 'swsusp_arch_suspend' [-Wmissing-prototypes] arch/loongarch/power/hibernate.c:56:5: warning: no previous prototype for 'swsusp_arch_resume' [-Wmissing-prototypes] arch/powerpc/kernel/kvm_emul.o: warning: objtool: kvm_template_end(): can't find starting instruction arch/powerpc/kernel/optprobes_head.o: warning: objtool: optprobe_template_end(): can't find starting instruction arch/riscv/kernel/crash_core.c:12:57: warning: format specifies type 'unsigned long' but the argument has type 'int' [-Wformat] arch/riscv/kernel/crash_core.c:14:57: error: use of undeclared identifier 'VMEMMAP_START' arch/riscv/kernel/crash_core.c:15:55: error: use of undeclared identifier 'VMEMMAP_END'; did you mean 'MEMREMAP_ENC'? arch/riscv/kernel/crash_core.c:17:57: error: use of undeclared identifier 'MODULES_VADDR' arch/riscv/kernel/crash_core.c:18:55: error: use of undeclared identifier 'MODULES_END' arch/riscv/kernel/crash_core.c:8:20: error: use of undeclared identifier 'VA_BITS' clang-16: error: no such file or directory: 'liburandom_read.so' drivers/gpu/drm/amd/amdgpu/../display/dc/irq/dcn201/irq_service_dcn201.c:40:20: warning: no previous prototype for 'to_dal_irq_source_dcn201' [-Wmissing-prototypes] drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c:353:5: warning: no previous prototype for 'amdgpu_mcbp_scan' [-Wmissing-prototypes] drivers/gpu/drm/amd/amdgpu/amdgpu_ring_mux.c:373:5: warning: no previous prototype for 'amdgpu_mcbp_trigger_preempt' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gf100.c:451:1: warning: no previous prototype for 'gf100_fifo_nonstall_block' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/gf100.c:451:1: warning: no previous prototype for function 'gf100_fifo_nonstall_block' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c:34:1: warning: no previous prototype for 'nvkm_engn_cgrp_get' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/fifo/runl.c:34:1: warning: no previous prototype for function 'nvkm_engn_cgrp_get' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c:210:1: warning: no previous prototype for 'tu102_gr_load' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/engine/gr/tu102.c:210:1: warning: no previous prototype for function 'tu102_gr_load' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:49:1: warning: no previous prototype for 'wpr_generic_header_dump' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/nvfw/acr.c:49:1: warning: no previous prototype for function 'wpr_generic_header_dump' [-Wmissing-prototypes] drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c:221:21: warning: variable 'loc' set but not used [-Wunused-but-set-variable] drivers/media/i2c/tc358746.c:816:13: warning: 'm_best' is used uninitialized [-Wuninitialized] drivers/media/i2c/tc358746.c:817:13: warning: 'p_best' is used uninitialized [-Wuninitialized] drivers/media/platform/renesas/rzg2l-cru/rzg2l-csi2.c:445:7: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
Re: [PATCH v2 0/6] drm/gud: Use the shadow plane helper
Den 30.11.2022 20.26, skrev Noralf Trønnes via B4 Submission Endpoint: > Hi, > > I have started to look at igt for testing and want to use CRC tests. To > implement support for this I need to move away from the simple kms > helper. > > When looking around for examples I came across Thomas' nice shadow > helper and thought, yes this is perfect for drm/gud. So I'll switch to > that before I move away from the simple kms helper. > > The async framebuffer flushing code path now uses a shadow buffer and > doesn't touch the framebuffer when it shouldn't. I have also taken the > opportunity to inline the synchronous flush code path and make this the > default flushing stategy. > > Noralf. > > Cc: Maxime Ripard > Cc: Thomas Zimmermann > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Noralf Trønnes > > --- > Changes in v2: > - Drop patch (Thomas): > drm/gem: shadow_fb_access: Prepare imported buffers for CPU access > - Use src as variable name for iosys_map (Thomas) > - Prepare imported buffer for CPU access in the driver (Thomas) > - New patch: make sync flushing the default (Thomas) > - Link to v1: > https://lore.kernel.org/r/20221122-gud-shadow-plane-v1-0-9de3afa33...@tronnes.org > > --- > Noralf Trønnes (6): > drm/gud: Fix UBSAN warning > drm/gud: Don't retry a failed framebuffer flush > drm/gud: Split up gud_flush_work() > drm/gud: Prepare buffer for CPU access in gud_flush_work() > drm/gud: Use the shadow plane helper > drm/gud: Enable synchronous flushing by default > Applied to drm-misc-next, thanks for reviewing! Noralf.
Re: [PATCH v4 2/6] dt-bindings: mediatek: modify VDOSYS0 mmsys device tree Documentations for MT8188
On 06/12/2022 03:00, nathan.lu wrote: > From: Nathan Lu > > modify VDOSYS0 mmsys device tree Documentations for MT8188. > > Signed-off-by: Nathan Lu Acked-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH] drm/msm/dpu: Fix memory leak in msm_mdss_parse_data_bus_icc_path
Hi,
On Mon, Dec 5, 2022 at 11:55 PM Miaoqian Lin wrote:
>
> of_icc_get() alloc resources for path1, we should release it when not
> need anymore. Early return when IS_ERR_OR_NULL(path0) may leak path1.
> Add icc_put(path1) in the error path to fix this.
>
> Fixes: b9364eed9232 ("drm/msm/dpu: Move min BW request and full BW disable
> back to mdss")
> Signed-off-by: Miaoqian Lin
> ---
> drivers/gpu/drm/msm/msm_mdss.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index e13c5c12b775..a38fa9a9a3d6 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -49,8 +49,10 @@ static int msm_mdss_parse_data_bus_icc_path(struct device
> *dev,
> struct icc_path *path0 = of_icc_get(dev, "mdp0-mem");
> struct icc_path *path1 = of_icc_get(dev, "mdp1-mem");
>
> - if (IS_ERR_OR_NULL(path0))
> + if (IS_ERR_OR_NULL(path0)) {
> + icc_put(path1);
> return PTR_ERR_OR_ZERO(path0);
> + }
>
> msm_mdss->path[0] = path0;
> msm_mdss->num_paths = 1;
Hmmm. I guess the original author of the code (which wasn't me--I just
restored the code that was deleted by a previous change) was assuming
that if mdp0-mem had a problem that mdp1-mem would also have a
problem. That would mean that you wouldn't need to call icc_put() on
it.
...and, in fact, your patch doesn't handle that case, does it? If
_both_ of the two are error or NULL then you'll be calling icc_put()
on something invalid. I guess icc_put() handles those cases without
crashing but it will give a WARN_ON() splat if it happens to be an
error...
Really, there's a better solution anyway. Instead, you should do:
path0 = of_icc_get(dev, "mdp0-mem");
if (IS_ERR_OR_NULL(path0))
return PTR_ERR_OR_ZERO(path0);
msm_mdss->path[0] = path0;
msm_mdss->num_paths = 1;
path1 = of_icc_get(dev, "mdp1-mem");
if (!IS_ERR_OR_NULL(path1)) {
...
}
In other words just defer getting path1 until after you've checked
path0 for an error.
-Doug
[PATCH] dma-buf: fix dma_buf_export init order
The init order and resulting error handling in dma_buf_export
was pretty messy.
Subordinate objects like the file and the sysfs kernel objects
were initializing and wiring itself up with the object in the
wrong order resulting not only in complicating and partially
incorrect error handling, but also in publishing only halve
initialized DMA-buf objects.
Clean this up thoughtfully by allocating the file independent
of the DMA-buf object. Then allocate and initialize the DMA-buf
object itself, before publishing it through sysfs. If everything
works as expected the file is then connected with the DMA-buf
object and publish it through debugfs.
Also adds the missing dma_resv_fini() into the error handling.
Signed-off-by: Christian König
---
drivers/dma-buf/dma-buf-sysfs-stats.c | 7 +--
drivers/dma-buf/dma-buf-sysfs-stats.h | 4 +-
drivers/dma-buf/dma-buf.c | 65 +--
3 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c
b/drivers/dma-buf/dma-buf-sysfs-stats.c
index 2bba0babcb62..4b680e10c15a 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
@@ -168,14 +168,11 @@ void dma_buf_uninit_sysfs_statistics(void)
kset_unregister(dma_buf_stats_kset);
}
-int dma_buf_stats_setup(struct dma_buf *dmabuf)
+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file)
{
struct dma_buf_sysfs_entry *sysfs_entry;
int ret;
- if (!dmabuf || !dmabuf->file)
- return -EINVAL;
-
if (!dmabuf->exp_name) {
pr_err("exporter name must not be empty if stats needed\n");
return -EINVAL;
@@ -192,7 +189,7 @@ int dma_buf_stats_setup(struct dma_buf *dmabuf)
/* create the directory for buffer stats */
ret = kobject_init_and_add(_entry->kobj, _buf_ktype, NULL,
- "%lu", file_inode(dmabuf->file)->i_ino);
+ "%lu", file_inode(file)->i_ino);
if (ret)
goto err_sysfs_dmabuf;
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.h
b/drivers/dma-buf/dma-buf-sysfs-stats.h
index a49c6e2650cc..7a8a995b75ba 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.h
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.h
@@ -13,7 +13,7 @@
int dma_buf_init_sysfs_statistics(void);
void dma_buf_uninit_sysfs_statistics(void);
-int dma_buf_stats_setup(struct dma_buf *dmabuf);
+int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file *file);
void dma_buf_stats_teardown(struct dma_buf *dmabuf);
#else
@@ -25,7 +25,7 @@ static inline int dma_buf_init_sysfs_statistics(void)
static inline void dma_buf_uninit_sysfs_statistics(void) {}
-static inline int dma_buf_stats_setup(struct dma_buf *dmabuf)
+static inline int dma_buf_stats_setup(struct dma_buf *dmabuf, struct file
*file)
{
return 0;
}
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e6f36c014c4c..ea08049b70ae 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -614,19 +614,11 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
size_t alloc_size = sizeof(struct dma_buf);
int ret;
- if (!exp_info->resv)
- alloc_size += sizeof(struct dma_resv);
- else
- /* prevent _buf[1] == dma_buf->resv */
- alloc_size += 1;
-
- if (WARN_ON(!exp_info->priv
- || !exp_info->ops
- || !exp_info->ops->map_dma_buf
- || !exp_info->ops->unmap_dma_buf
- || !exp_info->ops->release)) {
+ if (WARN_ON(!exp_info->priv || !exp_info->ops
+ || !exp_info->ops->map_dma_buf
+ || !exp_info->ops->unmap_dma_buf
+ || !exp_info->ops->release))
return ERR_PTR(-EINVAL);
- }
if (WARN_ON(exp_info->ops->cache_sgt_mapping &&
(exp_info->ops->pin || exp_info->ops->unpin)))
@@ -638,10 +630,21 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
if (!try_module_get(exp_info->owner))
return ERR_PTR(-ENOENT);
+ file = dma_buf_getfile(exp_info->size, exp_info->flags);
+ if (IS_ERR(file)) {
+ ret = PTR_ERR(file);
+ goto err_module;
+ }
+
+ if (!exp_info->resv)
+ alloc_size += sizeof(struct dma_resv);
+ else
+ /* prevent _buf[1] == dma_buf->resv */
+ alloc_size += 1;
dmabuf = kzalloc(alloc_size, GFP_KERNEL);
if (!dmabuf) {
ret = -ENOMEM;
- goto err_module;
+ goto err_file;
}
dmabuf->priv = exp_info->priv;
@@ -653,44 +656,36 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
init_waitqueue_head(>poll);
Re: [PATCH 5/7] media: renesas: vsp1: Add new formats (2-10-10-10 ARGB, Y210)
Hi Tomi,
On Tue, Dec 6, 2022 at 2:44 PM Tomi Valkeinen
wrote:
> Add new pixel formats: XBGR2101010, ABGR2101010, BGRA1010102 and Y210.
>
> Signed-off-by: Tomi Valkeinen
Thanks for your patch!
> --- a/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> +++ b/drivers/media/platform/renesas/vsp1/vsp1_rpf.c
> @@ -109,6 +109,56 @@ static void rpf_configure_stream(struct vsp1_entity
> *entity,
> vsp1_rpf_write(rpf, dlb, VI6_RPF_INFMT, infmt);
> vsp1_rpf_write(rpf, dlb, VI6_RPF_DSWAP, fmtinfo->swap);
>
> + if ((entity->vsp1->version & VI6_IP_VERSION_MODEL_MASK) ==
> VI6_IP_VERSION_MODEL_VSPD_GEN4) {
> + u32 ext_infmt0;
> + u32 ext_infmt1;
> + u32 ext_infmt2;
> +
> + switch (fmtinfo->fourcc) {
> + case V4L2_PIX_FMT_XBGR2101010:
> + ext_infmt0 = VI6_RPF_EXT_INFMT0_BYPP_M1_RGB10;
> + ext_infmt1 = (0 << 24) | (10 << 16) |
> +(20 << 8) | (30 << 0);
Introducing PACK_CPOS(a, b, c, d)...
> + ext_infmt2 = (10 << 24) | (10 << 16) |
> +(10 << 8) | (0 << 0);
... and PACK_CLEN(a, b, c, d) macros (or a single PACK4() macro)
can make this less error-prone.
> + break;
> +
> + case V4L2_PIX_FMT_ABGR2101010:
> + ext_infmt0 = VI6_RPF_EXT_INFMT0_BYPP_M1_RGB10;
> + ext_infmt1 = (0 << 24) | (10 << 16) |
> +(20 << 8) | (30 << 0);
> + ext_infmt2 = (10 << 24) | (10 << 16) |
> +(10 << 8) | (2 << 0);
> + break;
> +
> + case V4L2_PIX_FMT_BGRA1010102:
> + ext_infmt0 = VI6_RPF_EXT_INFMT0_BYPP_M1_RGB10;
> + ext_infmt1 = (2 << 24) | (12 << 16) |
> +(22 << 8) | (22 << 0);
> + ext_infmt2 = (10 << 24) | (10 << 16) |
> +(10 << 8) | (2 << 0);
> + break;
> +
> + case V4L2_PIX_FMT_Y210:
> + ext_infmt0 = VI6_RPF_EXT_INFMT0_F2B_MSB |
> +VI6_RPF_EXT_INFMT0_IPBD_Y_10 |
> +VI6_RPF_EXT_INFMT0_IPBD_C_10;
> + ext_infmt1 = 0x0;
> + ext_infmt2 = 0x0;
> + break;
> +
> + default:
> + ext_infmt0 = 0;
> + ext_infmt1 = 0;
> + ext_infmt2 = 0;
> + break;
> + }
> +
> + vsp1_rpf_write(rpf, dlb, VI6_RPF_EXT_INFMT0, ext_infmt0);
> + vsp1_rpf_write(rpf, dlb, VI6_RPF_EXT_INFMT1, ext_infmt1);
> + vsp1_rpf_write(rpf, dlb, VI6_RPF_EXT_INFMT2, ext_infmt2);
> + }
> +
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH V4 2/3] drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel
On Sat, Dec 3, 2022 at 8:01 PM Chris Morgan wrote:
> Will do. I'll make the changes and resubmit. For what it's worth the
> documentation says this one is a Samsung AMS495QA01 panel on a
> Magnachip D53E6EA8966 controller IC.
I would name the driver panel-magnachip-d53e6ea8966.c and
KConfig PANEL_MAGNACHIP_D53E6EA8966 for now
but keep the Samsung compatible string & match.
Maybe this driver will match more magnachips in the future,
maybe not. Sometimes people get hold of datasheets and submit
proper code and #defines etc.
> > > + if (db->reg_elvdd) {
> >
> > Do you really need to if() this? I thought the regulator
> > framework would just ignore the calls for an optional
> > regulator.
>
> I don't know for sure, but I'll make the change if you request it. I
> think other drivers had an if in this scenario which is why I did it.
Okay but I don't think that is necessary.
> > > + mode->type = DRM_MODE_TYPE_DRIVER;
> > > + if (panel_info->num_modes == 1)
> > > + mode->type |= DRM_MODE_TYPE_PREFERRED;
> >
> > I think you should probably set the preferred mode even
> > if there are several of them? But maybe just on the first
> > or something. (A bit unsure here, Sam?)
>
> I'll keep 60hz as the preferred. 50hz was added at the request of some
> userspace folks for running PAL based emulators and stuff.
OK
> This is the path that "works", but I'll happily change to something
> else.
>
> > > + db->dsi_dev->lanes = 2;
> > > + db->dsi_dev->format = MIPI_DSI_FMT_RGB888;
> > > + db->dsi_dev->mode_flags = MIPI_DSI_MODE_VIDEO |
> > > MIPI_DSI_MODE_VIDEO_BURST |
> > > + MIPI_DSI_MODE_LPM | MIPI_DSI_MODE_NO_EOT_PACKET;
> > > +
> > > + drm_panel_init(>panel, dev, _drm_funcs,
> > > + DRM_MODE_CONNECTOR_DSI);
> >
> > Pixel data passes to the display using DSI but all display control
> > is done over SPI, and the core will not help with this.
> >
> > So from the display controller POV this is a DSI display
> > and from the display POV this is an SPI-controlled display.
> > So it sits on two buses. Data path is on DSI, control path
> > is on SPI.
> >
> > This would be kind of odd actually, normally DSI displays
> > do the display control over DSI as well. SPI control is usually
> > used on DPI displays. But I'm not surprised.
> >
> > If this is necessary, isn't this something we need to teach the
> > core to handle instead of adding quirks like this to all drivers that
> > have this characteristic?
> >
>
> You are correct, this panel is controlled via 3-wire SPI in my example.
> The panel can be controlled either by 3-wire SPI or DSI commands
> depending on whether or not pin 15 is driven high or low. Unfortunately
> in my case it's hardwired high, so I am forced to do it via 3-wire SPI.
> I have no way of testing it with pure DSI but that would simplify
> things quite a bit. Pixel data is transmitted soley through DSI.
OK
> The way I have it implemented currently is to put the panel on the SPI
> bus as a DBI panel; traverse through the DT bindings to find the
> associated DSI controller, then attach it as a DSI device so the DSI
> bus can transmit the pixel data.
>
> I'm absolutely cool with making those functions part of the core and
> not just specific to this panel, only I might need a bit of help on
> that part to make sure I do it the right way. I just wasn't sure how
> often that would be needed since this is the only panel I've ever seen
> driven this way, especially since it seems like any sane person would
> just want to do the whole control data/pixel data over DSI to keep
> things simple.
It doesn't seem all that unique.
Can you put some helper in drivers/gpu/drm/drm_of.c with the rest
and it will (hopefully) not be linked in unless used anyway.
Yours,
Linus Walleij
Re: [PATCH v3 3/3] drm/tiny: ili9486: remove conflicting framebuffers
On 12/6/22 10:52, Thomas Zimmermann wrote: > Hi > > Am 06.12.22 um 10:41 schrieb Neil Armstrong: >> Hi Carlo, >> >> On 06/12/2022 09:34, Carlo Caione wrote: >>> For platforms using simplefb / efifb, call >>> drm_aperture_remove_framebuffers() to remove the conflicting >>> framebuffer. >> >> Conflicting framebuffer on the SPI display ? How is that possible ? > > Calling drm_aperture_remove_framebuffers() is only required if the > graphics card may have been pre-initialized by the system, such as a > VGA-compatible card on a PC. > > Could the SPI display have been initialized by the firmware? If not, the > call should be left out. > Agree. -- Best regards, Javier Martinez Canillas Core Platforms Red Hat
Re: [PATCH V4 2/3] drm/panel: Add Samsung AMS495QA01 MIPI-DSI LCD panel
On Sat, Dec 3, 2022 at 11:18 AM Heiko Stübner wrote: > Though in past projects I've seen the same display-controller used with > different panels (and different dsi-init-sequences). In one project the > display manufacturer even EOL'ed the first panel and provided a replacement > with said same display controller (and a different init) - but the > datasheets for the display-controller were for the same chip still. > > So while in my experience the actual display name from the manufacturer > identifies the display + controller combo, I don't really think you can > go the other way with the controller name identifying the display+controller > combination. I don't mean we should do that. What I mean is: - Name the driver after the controller, if we know which one it is, such as panel-novatek-nt35510.c - Provide identifications of the controller+panel combo in e.g. DT compatible strings, provide init data as arrays in the per-variant match data (or similar for ACPI). Yours, Linus Walleij
Re: [PATCH 0/7] media/drm: renesas: Add new pixel formats
On 06/12/2022 15:39, Tomi Valkeinen wrote: From: Tomi Valkeinen Hi, These add new pixel formats for Renesas V3U and V4H SoCs. As the display pipeline is split between DRM and V4L2 components, this series touches both subsystems. I'm sending all these together to simplify review. If needed, I can later split this to V4L2 and DRM parts, of which the V4L2 part needs to be merged first. I forgot to mention: this is based on the "[PATCH v5 0/7] Renesas V4H DSI & DP output support" series. Tomi Tomi Valkeinen (7): media: Add 2-10-10-10 RGB formats media: Add Y210, Y212 and Y216 formats media: renesas: vsp1: Change V3U to be gen4 media: renesas: vsp1: Add V4H SoC version media: renesas: vsp1: Add new formats (2-10-10-10 ARGB, Y210) drm: rcar-du: Bump V3U to gen 4 drm: rcar-du: Add new formats (2-10-10-10 ARGB, Y210) drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_kms.c | 24 +++ drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 49 ++- .../media/platform/renesas/vsp1/vsp1_drv.c| 4 +- .../media/platform/renesas/vsp1/vsp1_hgo.c| 4 +- .../media/platform/renesas/vsp1/vsp1_lif.c| 1 + .../media/platform/renesas/vsp1/vsp1_pipe.c | 15 + .../media/platform/renesas/vsp1/vsp1_regs.h | 25 +++- .../media/platform/renesas/vsp1/vsp1_rpf.c| 62 +-- .../media/platform/renesas/vsp1/vsp1_video.c | 4 +- .../media/platform/renesas/vsp1/vsp1_wpf.c| 4 +- drivers/media/v4l2-core/v4l2-ioctl.c | 6 ++ include/uapi/linux/videodev2.h| 11 13 files changed, 193 insertions(+), 18 deletions(-)
Re: [PATCH v5 5/7] arm64: dts: renesas: white-hawk-cpu: Add DP output support
Hi Tomi,
On Tue, Dec 6, 2022 at 2:45 PM Tomi Valkeinen
wrote:
> On 05/12/2022 12:10, Geert Uytterhoeven wrote:
> > On Thu, Dec 1, 2022 at 10:56 AM Tomi Valkeinen
> > wrote:
> >>
> >> Add DT nodes needed for the mini DP connector. The DP is driven by
> >> sn65dsi86, which in turn gets the pixel data from the SoC via DSI.
> >>
> >> Signed-off-by: Tomi Valkeinen
> >> Reviewed-by: Kieran Bingham
> >> Reviewed-by: Laurent Pinchart
> >
> > (same comments as v2)
> > Reviewed-by: Geert Uytterhoeven
> > i.e. will queue in renesas-devel for v6.3, with the mini-dp-con node
> > moved up.
>
> Ah, sorry, I had missed this change. I'll update my branch too.
Np. IIRC, you were sending out v5 while I was reviewing v2/v4.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH 1/1] crypto: ccp - Allocate TEE ring and cmd buffer using DMA APIs
On 12/6/2022 6:26 PM, Christian König wrote:
> Am 06.12.22 um 13:54 schrieb Rijo Thomas:
>>
>> On 12/6/2022 6:11 PM, Christian König wrote:
>>> Am 06.12.22 um 13:30 schrieb Rijo Thomas:
For AMD Secure Processor (ASP) to map and access TEE ring buffer, the
ring buffer address sent by host to ASP must be a real physical
address and the pages must be physically contiguous.
In a virtualized environment though, when the driver is running in a
guest VM, the pages allocated by __get_free_pages() may not be
contiguous in the host (or machine) physical address space. Guests
will see a guest (or pseudo) physical address and not the actual host
(or machine) physical address. The TEE running on ASP cannot decipher
pseudo physical addresses. It needs host or machine physical address.
To resolve this problem, use DMA APIs for allocating buffers that must
be shared with TEE. This will ensure that the pages are contiguous in
host (or machine) address space. If the DMA handle is an IOVA,
translate it into a physical address before sending it to ASP.
This patch also exports two APIs (one for buffer allocation and
another to free the buffer). This API can be used by AMD-TEE driver to
share buffers with TEE.
>>> Maybe use some other name than dma_buffer for your structure, cause that is
>>> usually something completely different in the Linux kernel.
>>>
>> Sure Christian. I shall rename "struct dma_buffer" to "struct shm_buffer"
>> (Shared Memory Buffer) in the file include/linux/psp-tee.h
>> The functions psp_tee_alloc_dmabuf() and psp_tee_free_dmabuf() shall be
>> renamed to psp_tee_alloc_shmbuf() and psp_tee_free_shmbuf(), respectively.
>> I shall do correction in next patch revision. I will wait for others to
>> review as well before I post update.
>
> I strongly suggest to use the name psp_buffer or something similar because
> shm_buffer is usually used for something else as well.
I see that the TEE subsystem defines "struct tee_shm".
Okay, I will name the newly added structure as "struct psp_tee_buffer" and the
APIs as psp_tee_alloc_buffer() and psp_tee_free_buffer().
Complete function prototype:
struct psp_tee_buffer *psp_tee_alloc_buffer(unsigned long size, gfp_t gfp);
void psp_tee_free_buffer(struct psp_tee_buffer *buffer);
Does this look okay?
Thanks,
Rijo
>
> Regards,
> Christian.
>
>>
>> Thanks,
>> Rijo
>>
>>> Regards,
>>> Christian.
>>>
Fixes: 33960acccfbd ("crypto: ccp - add TEE support for Raven Ridge")
Cc: Tom Lendacky
Cc: sta...@vger.kernel.org
Signed-off-by: Rijo Thomas
Co-developed-by: Jeshwanth
Signed-off-by: Jeshwanth
Reviewed-by: Devaraj Rangasamy
---
drivers/crypto/ccp/psp-dev.c | 6 +-
drivers/crypto/ccp/tee-dev.c | 116 ++-
drivers/crypto/ccp/tee-dev.h | 9 +--
include/linux/psp-tee.h | 47 ++
4 files changed, 127 insertions(+), 51 deletions(-)
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index c9c741ac8442..2b86158d7435 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -161,13 +161,13 @@ int psp_dev_init(struct sp_device *sp)
goto e_err;
}
+ if (sp->set_psp_master_device)
+ sp->set_psp_master_device(sp);
+
ret = psp_init(psp);
if (ret)
goto e_irq;
- if (sp->set_psp_master_device)
- sp->set_psp_master_device(sp);
-
/* Enable interrupt */
iowrite32(-1, psp->io_regs + psp->vdata->inten_reg);
diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index 5c9d47f3be37..1631d9851e54 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -12,8 +12,9 @@
#include
#include
#include
+#include
+#include
#include
-#include
#include
#include "psp-dev.h"
@@ -21,25 +22,64 @@
static bool psp_dead;
+struct dma_buffer *psp_tee_alloc_dmabuf(unsigned long size, gfp_t gfp)
+{
+ struct psp_device *psp = psp_get_master_device();
+ struct dma_buffer *dma_buf;
+ struct iommu_domain *dom;
+
+ if (!psp || !size)
+ return NULL;
+
+ dma_buf = kzalloc(sizeof(*dma_buf), GFP_KERNEL);
+ if (!dma_buf)
+ return NULL;
+
+ dma_buf->vaddr = dma_alloc_coherent(psp->dev, size, _buf->dma,
gfp);
+ if (!dma_buf->vaddr || !dma_buf->dma) {
+ kfree(dma_buf);
+ return NULL;
+ }
+
+ dma_buf->size = size;
+
+ dom = iommu_get_domain_for_dev(psp->dev);
+ if (dom)
+ dma_buf->paddr = iommu_iova_to_phys(dom, dma_buf->dma);
Re: [Linaro-mm-sig] Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export
Am 06.12.22 um 13:55 schrieb Charan Teja Kalla:
Thanks Christian/TJ for all your inputs!!
On 11/24/2022 6:25 PM, Christian König wrote:
I was already wondering why the order is this way.
Why is dma_buf_stats_setup() needing the file in the first place?
dmabuf->file will be used in dma_buf_stats_setup(), the
dma_buf_stats_setup() as follows:
171 int dma_buf_stats_setup(struct dma_buf *dmabuf)
172 {
173 struct dma_buf_sysfs_entry *sysfs_entry;
174 int ret;
175
176 if (!dmabuf || !dmabuf->file)
177 return -EINVAL;
178
179 if (!dmabuf->exp_name) {
180 pr_err("exporter name must not be empty if stats
needed\n");
181 return -EINVAL;
182 }
183
184 sysfs_entry = kzalloc(sizeof(struct dma_buf_sysfs_entry),
GFP_KERNEL);
185 if (!sysfs_entry)
186 return -ENOMEM;
187
188 sysfs_entry->kobj.kset = dma_buf_per_buffer_stats_kset;
189 sysfs_entry->dmabuf = dmabuf;
190
191 dmabuf->sysfs_entry = sysfs_entry;
192
193 /* create the directory for buffer stats */
194 ret = kobject_init_and_add(_entry->kobj,
_buf_ktype, NULL,
195 "%lu",
file_inode(dmabuf->file)->i_ino);
Ah, so it uses the i_ino of the file for the sysfs unique name.
I'm going to take another look how to properly clean this up.
How about deleting the dmabuf from the db_list directly in the error
path (which is usually done by the fput()) and then continue with the
normal fput() here.
No, that's not really clean either.
Give me 10 Minutes, going to come up with something.
Regards,
Christian.
Just compile tested the below code and If the logic make sense for you,
will send the final tested patch.
--><-
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e6f36c0..10a1727 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -87,19 +87,28 @@ static void dma_buf_release(struct dentry *dentry)
kfree(dmabuf);
}
-static int dma_buf_file_release(struct inode *inode, struct file *file)
+static void dma_buf_db_list_remove(struct file *file)
{
struct dma_buf *dmabuf;
- if (!is_dma_buf_file(file))
- return -EINVAL;
-
dmabuf = file->private_data;
+ if (!dmabuf)
+ return;
mutex_lock(_list.lock);
list_del(>list_node);
mutex_unlock(_list.lock);
+ file->private_data = NULL;
+}
+
+static int dma_buf_file_release(struct inode *inode, struct file *file)
+{
+ if (!is_dma_buf_file(file))
+ return -EINVAL;
+
+ dma_buf_db_list_remove(file);
+
return 0;
}
@@ -688,6 +697,8 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
* early before calling the release() dma_buf op.
*/
file->f_path.dentry->d_fsdata = NULL;
+
+ dma_buf_db_list_remove(file);
fput(file);
err_dmabuf:
kfree(dmabuf);
><-
Thanks for pointing this out,
Christian.
Re: [PATCH 1/1] crypto: ccp - Allocate TEE ring and cmd buffer using DMA APIs
Am 06.12.22 um 13:54 schrieb Rijo Thomas:
On 12/6/2022 6:11 PM, Christian König wrote:
Am 06.12.22 um 13:30 schrieb Rijo Thomas:
For AMD Secure Processor (ASP) to map and access TEE ring buffer, the
ring buffer address sent by host to ASP must be a real physical
address and the pages must be physically contiguous.
In a virtualized environment though, when the driver is running in a
guest VM, the pages allocated by __get_free_pages() may not be
contiguous in the host (or machine) physical address space. Guests
will see a guest (or pseudo) physical address and not the actual host
(or machine) physical address. The TEE running on ASP cannot decipher
pseudo physical addresses. It needs host or machine physical address.
To resolve this problem, use DMA APIs for allocating buffers that must
be shared with TEE. This will ensure that the pages are contiguous in
host (or machine) address space. If the DMA handle is an IOVA,
translate it into a physical address before sending it to ASP.
This patch also exports two APIs (one for buffer allocation and
another to free the buffer). This API can be used by AMD-TEE driver to
share buffers with TEE.
Maybe use some other name than dma_buffer for your structure, cause that is
usually something completely different in the Linux kernel.
Sure Christian. I shall rename "struct dma_buffer" to "struct shm_buffer"
(Shared Memory Buffer) in the file include/linux/psp-tee.h
The functions psp_tee_alloc_dmabuf() and psp_tee_free_dmabuf() shall be renamed
to psp_tee_alloc_shmbuf() and psp_tee_free_shmbuf(), respectively.
I shall do correction in next patch revision. I will wait for others to review
as well before I post update.
I strongly suggest to use the name psp_buffer or something similar
because shm_buffer is usually used for something else as well.
Regards,
Christian.
Thanks,
Rijo
Regards,
Christian.
Fixes: 33960acccfbd ("crypto: ccp - add TEE support for Raven Ridge")
Cc: Tom Lendacky
Cc: sta...@vger.kernel.org
Signed-off-by: Rijo Thomas
Co-developed-by: Jeshwanth
Signed-off-by: Jeshwanth
Reviewed-by: Devaraj Rangasamy
---
drivers/crypto/ccp/psp-dev.c | 6 +-
drivers/crypto/ccp/tee-dev.c | 116 ++-
drivers/crypto/ccp/tee-dev.h | 9 +--
include/linux/psp-tee.h | 47 ++
4 files changed, 127 insertions(+), 51 deletions(-)
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index c9c741ac8442..2b86158d7435 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -161,13 +161,13 @@ int psp_dev_init(struct sp_device *sp)
goto e_err;
}
+ if (sp->set_psp_master_device)
+ sp->set_psp_master_device(sp);
+
ret = psp_init(psp);
if (ret)
goto e_irq;
- if (sp->set_psp_master_device)
- sp->set_psp_master_device(sp);
-
/* Enable interrupt */
iowrite32(-1, psp->io_regs + psp->vdata->inten_reg);
diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index 5c9d47f3be37..1631d9851e54 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -12,8 +12,9 @@
#include
#include
#include
+#include
+#include
#include
-#include
#include
#include "psp-dev.h"
@@ -21,25 +22,64 @@
static bool psp_dead;
+struct dma_buffer *psp_tee_alloc_dmabuf(unsigned long size, gfp_t gfp)
+{
+ struct psp_device *psp = psp_get_master_device();
+ struct dma_buffer *dma_buf;
+ struct iommu_domain *dom;
+
+ if (!psp || !size)
+ return NULL;
+
+ dma_buf = kzalloc(sizeof(*dma_buf), GFP_KERNEL);
+ if (!dma_buf)
+ return NULL;
+
+ dma_buf->vaddr = dma_alloc_coherent(psp->dev, size, _buf->dma, gfp);
+ if (!dma_buf->vaddr || !dma_buf->dma) {
+ kfree(dma_buf);
+ return NULL;
+ }
+
+ dma_buf->size = size;
+
+ dom = iommu_get_domain_for_dev(psp->dev);
+ if (dom)
+ dma_buf->paddr = iommu_iova_to_phys(dom, dma_buf->dma);
+ else
+ dma_buf->paddr = dma_buf->dma;
+
+ return dma_buf;
+}
+EXPORT_SYMBOL(psp_tee_alloc_dmabuf);
+
+void psp_tee_free_dmabuf(struct dma_buffer *dma_buf)
+{
+ struct psp_device *psp = psp_get_master_device();
+
+ if (!psp || !dma_buf)
+ return;
+
+ dma_free_coherent(psp->dev, dma_buf->size,
+ dma_buf->vaddr, dma_buf->dma);
+
+ kfree(dma_buf);
+}
+EXPORT_SYMBOL(psp_tee_free_dmabuf);
+
static int tee_alloc_ring(struct psp_tee_device *tee, int ring_size)
{
struct ring_buf_manager *rb_mgr = >rb_mgr;
- void *start_addr;
if (!ring_size)
return -EINVAL;
- /* We need actual physical address instead of DMA address, since
- * Trusted OS running on AMD Secure Processor will map this region
- */
- start_addr = (void *)__get_free_pages(GFP_KERNEL, get_order(ring_size));
- if (!start_addr)
+ rb_mgr->ring_buf = psp_tee_alloc_dmabuf(ring_size,
+
Re: [Linaro-mm-sig] Re: [PATCH] dma-buf: Fix possible UAF in dma_buf_export
Thanks Christian/TJ for all your inputs!!
On 11/24/2022 6:25 PM, Christian König wrote:
>>> I was already wondering why the order is this way.
>>>
>>> Why is dma_buf_stats_setup() needing the file in the first place?
>>
>> dmabuf->file will be used in dma_buf_stats_setup(), the
>> dma_buf_stats_setup() as follows:
>>
>>> 171 int dma_buf_stats_setup(struct dma_buf *dmabuf)
>>> 172 {
>>> 173 struct dma_buf_sysfs_entry *sysfs_entry;
>>> 174 int ret;
>>> 175
>>> 176 if (!dmabuf || !dmabuf->file)
>>> 177 return -EINVAL;
>>> 178
>>> 179 if (!dmabuf->exp_name) {
>>> 180 pr_err("exporter name must not be empty if stats
>>> needed\n");
>>> 181 return -EINVAL;
>>> 182 }
>>> 183
>>> 184 sysfs_entry = kzalloc(sizeof(struct dma_buf_sysfs_entry),
>>> GFP_KERNEL);
>>> 185 if (!sysfs_entry)
>>> 186 return -ENOMEM;
>>> 187
>>> 188 sysfs_entry->kobj.kset = dma_buf_per_buffer_stats_kset;
>>> 189 sysfs_entry->dmabuf = dmabuf;
>>> 190
>>> 191 dmabuf->sysfs_entry = sysfs_entry;
>>> 192
>>> 193 /* create the directory for buffer stats */
>>> 194 ret = kobject_init_and_add(_entry->kobj,
>>> _buf_ktype, NULL,
>>> 195 "%lu",
>>> file_inode(dmabuf->file)->i_ino);
>
> Ah, so it uses the i_ino of the file for the sysfs unique name.
>
> I'm going to take another look how to properly clean this up.
>
How about deleting the dmabuf from the db_list directly in the error
path (which is usually done by the fput()) and then continue with the
normal fput() here.
Just compile tested the below code and If the logic make sense for you,
will send the final tested patch.
--><-
diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
index e6f36c0..10a1727 100644
--- a/drivers/dma-buf/dma-buf.c
+++ b/drivers/dma-buf/dma-buf.c
@@ -87,19 +87,28 @@ static void dma_buf_release(struct dentry *dentry)
kfree(dmabuf);
}
-static int dma_buf_file_release(struct inode *inode, struct file *file)
+static void dma_buf_db_list_remove(struct file *file)
{
struct dma_buf *dmabuf;
- if (!is_dma_buf_file(file))
- return -EINVAL;
-
dmabuf = file->private_data;
+ if (!dmabuf)
+ return;
mutex_lock(_list.lock);
list_del(>list_node);
mutex_unlock(_list.lock);
+ file->private_data = NULL;
+}
+
+static int dma_buf_file_release(struct inode *inode, struct file *file)
+{
+ if (!is_dma_buf_file(file))
+ return -EINVAL;
+
+ dma_buf_db_list_remove(file);
+
return 0;
}
@@ -688,6 +697,8 @@ struct dma_buf *dma_buf_export(const struct
dma_buf_export_info *exp_info)
* early before calling the release() dma_buf op.
*/
file->f_path.dentry->d_fsdata = NULL;
+
+ dma_buf_db_list_remove(file);
fput(file);
err_dmabuf:
kfree(dmabuf);
><-
> Thanks for pointing this out,
> Christian.
Re: [PATCH 1/1] crypto: ccp - Allocate TEE ring and cmd buffer using DMA APIs
On 12/6/2022 6:11 PM, Christian König wrote:
> Am 06.12.22 um 13:30 schrieb Rijo Thomas:
>> For AMD Secure Processor (ASP) to map and access TEE ring buffer, the
>> ring buffer address sent by host to ASP must be a real physical
>> address and the pages must be physically contiguous.
>>
>> In a virtualized environment though, when the driver is running in a
>> guest VM, the pages allocated by __get_free_pages() may not be
>> contiguous in the host (or machine) physical address space. Guests
>> will see a guest (or pseudo) physical address and not the actual host
>> (or machine) physical address. The TEE running on ASP cannot decipher
>> pseudo physical addresses. It needs host or machine physical address.
>>
>> To resolve this problem, use DMA APIs for allocating buffers that must
>> be shared with TEE. This will ensure that the pages are contiguous in
>> host (or machine) address space. If the DMA handle is an IOVA,
>> translate it into a physical address before sending it to ASP.
>>
>> This patch also exports two APIs (one for buffer allocation and
>> another to free the buffer). This API can be used by AMD-TEE driver to
>> share buffers with TEE.
>
> Maybe use some other name than dma_buffer for your structure, cause that is
> usually something completely different in the Linux kernel.
>
Sure Christian. I shall rename "struct dma_buffer" to "struct shm_buffer"
(Shared Memory Buffer) in the file include/linux/psp-tee.h
The functions psp_tee_alloc_dmabuf() and psp_tee_free_dmabuf() shall be renamed
to psp_tee_alloc_shmbuf() and psp_tee_free_shmbuf(), respectively.
I shall do correction in next patch revision. I will wait for others to review
as well before I post update.
Thanks,
Rijo
> Regards,
> Christian.
>
>>
>> Fixes: 33960acccfbd ("crypto: ccp - add TEE support for Raven Ridge")
>> Cc: Tom Lendacky
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Rijo Thomas
>> Co-developed-by: Jeshwanth
>> Signed-off-by: Jeshwanth
>> Reviewed-by: Devaraj Rangasamy
>> ---
>> drivers/crypto/ccp/psp-dev.c | 6 +-
>> drivers/crypto/ccp/tee-dev.c | 116 ++-
>> drivers/crypto/ccp/tee-dev.h | 9 +--
>> include/linux/psp-tee.h | 47 ++
>> 4 files changed, 127 insertions(+), 51 deletions(-)
>>
>> diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
>> index c9c741ac8442..2b86158d7435 100644
>> --- a/drivers/crypto/ccp/psp-dev.c
>> +++ b/drivers/crypto/ccp/psp-dev.c
>> @@ -161,13 +161,13 @@ int psp_dev_init(struct sp_device *sp)
>> goto e_err;
>> }
>> + if (sp->set_psp_master_device)
>> + sp->set_psp_master_device(sp);
>> +
>> ret = psp_init(psp);
>> if (ret)
>> goto e_irq;
>> - if (sp->set_psp_master_device)
>> - sp->set_psp_master_device(sp);
>> -
>> /* Enable interrupt */
>> iowrite32(-1, psp->io_regs + psp->vdata->inten_reg);
>> diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
>> index 5c9d47f3be37..1631d9851e54 100644
>> --- a/drivers/crypto/ccp/tee-dev.c
>> +++ b/drivers/crypto/ccp/tee-dev.c
>> @@ -12,8 +12,9 @@
>> #include
>> #include
>> #include
>> +#include
>> +#include
>> #include
>> -#include
>> #include
>> #include "psp-dev.h"
>> @@ -21,25 +22,64 @@
>> static bool psp_dead;
>> +struct dma_buffer *psp_tee_alloc_dmabuf(unsigned long size, gfp_t gfp)
>> +{
>> + struct psp_device *psp = psp_get_master_device();
>> + struct dma_buffer *dma_buf;
>> + struct iommu_domain *dom;
>> +
>> + if (!psp || !size)
>> + return NULL;
>> +
>> + dma_buf = kzalloc(sizeof(*dma_buf), GFP_KERNEL);
>> + if (!dma_buf)
>> + return NULL;
>> +
>> + dma_buf->vaddr = dma_alloc_coherent(psp->dev, size, _buf->dma, gfp);
>> + if (!dma_buf->vaddr || !dma_buf->dma) {
>> + kfree(dma_buf);
>> + return NULL;
>> + }
>> +
>> + dma_buf->size = size;
>> +
>> + dom = iommu_get_domain_for_dev(psp->dev);
>> + if (dom)
>> + dma_buf->paddr = iommu_iova_to_phys(dom, dma_buf->dma);
>> + else
>> + dma_buf->paddr = dma_buf->dma;
>> +
>> + return dma_buf;
>> +}
>> +EXPORT_SYMBOL(psp_tee_alloc_dmabuf);
>> +
>> +void psp_tee_free_dmabuf(struct dma_buffer *dma_buf)
>> +{
>> + struct psp_device *psp = psp_get_master_device();
>> +
>> + if (!psp || !dma_buf)
>> + return;
>> +
>> + dma_free_coherent(psp->dev, dma_buf->size,
>> + dma_buf->vaddr, dma_buf->dma);
>> +
>> + kfree(dma_buf);
>> +}
>> +EXPORT_SYMBOL(psp_tee_free_dmabuf);
>> +
>> static int tee_alloc_ring(struct psp_tee_device *tee, int ring_size)
>> {
>> struct ring_buf_manager *rb_mgr = >rb_mgr;
>> - void *start_addr;
>> if (!ring_size)
>> return -EINVAL;
>> - /* We need actual physical address instead of DMA address, since
>> - * Trusted OS running on AMD Secure Processor will map this region
>> - */
>> - start_addr = (void
Re: [PATCH v9 2/5] dt-bindings: msm/dp: add data-lanes and link-frequencies property
On Mon, 05 Dec 2022 15:08:11 -0800, Kuogee Hsieh wrote: > Add both data-lanes and link-frequencies property into endpoint > > Changes in v7: > -- split yaml out of dtsi patch > -- link-frequencies from link rate to symbol rate > -- deprecation of old data-lanes property > > Changes in v8: > -- correct Bjorn mail address to kernel.org > > Signed-off-by: Kuogee Hsieh ` > --- > Documentation/devicetree/bindings/display/msm/dp-controller.yaml | 9 > + > 1 file changed, 9 insertions(+) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: ./Documentation/devicetree/bindings/display/msm/dp-controller.yaml:108:21: [error] syntax error: mapping values are not allowed here (syntax) dtschema/dtc warnings/errors: make[1]: *** Deleting file 'Documentation/devicetree/bindings/display/msm/dp-controller.example.dts' Documentation/devicetree/bindings/display/msm/dp-controller.yaml:108:21: mapping values are not allowed here make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/display/msm/dp-controller.example.dts] Error 1 make[1]: *** Waiting for unfinished jobs ./Documentation/devicetree/bindings/display/msm/dp-controller.yaml:108:21: mapping values are not allowed here /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/msm/dp-controller.yaml: ignoring, error parsing file make: *** [Makefile:1492: dt_binding_check] Error 2 doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1670281694-13281-3-git-send-email-quic_khs...@quicinc.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
Re: [PATCH 1/1] crypto: ccp - Allocate TEE ring and cmd buffer using DMA APIs
Am 06.12.22 um 13:30 schrieb Rijo Thomas:
For AMD Secure Processor (ASP) to map and access TEE ring buffer, the
ring buffer address sent by host to ASP must be a real physical
address and the pages must be physically contiguous.
In a virtualized environment though, when the driver is running in a
guest VM, the pages allocated by __get_free_pages() may not be
contiguous in the host (or machine) physical address space. Guests
will see a guest (or pseudo) physical address and not the actual host
(or machine) physical address. The TEE running on ASP cannot decipher
pseudo physical addresses. It needs host or machine physical address.
To resolve this problem, use DMA APIs for allocating buffers that must
be shared with TEE. This will ensure that the pages are contiguous in
host (or machine) address space. If the DMA handle is an IOVA,
translate it into a physical address before sending it to ASP.
This patch also exports two APIs (one for buffer allocation and
another to free the buffer). This API can be used by AMD-TEE driver to
share buffers with TEE.
Maybe use some other name than dma_buffer for your structure, cause that
is usually something completely different in the Linux kernel.
Regards,
Christian.
Fixes: 33960acccfbd ("crypto: ccp - add TEE support for Raven Ridge")
Cc: Tom Lendacky
Cc: sta...@vger.kernel.org
Signed-off-by: Rijo Thomas
Co-developed-by: Jeshwanth
Signed-off-by: Jeshwanth
Reviewed-by: Devaraj Rangasamy
---
drivers/crypto/ccp/psp-dev.c | 6 +-
drivers/crypto/ccp/tee-dev.c | 116 ++-
drivers/crypto/ccp/tee-dev.h | 9 +--
include/linux/psp-tee.h | 47 ++
4 files changed, 127 insertions(+), 51 deletions(-)
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index c9c741ac8442..2b86158d7435 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -161,13 +161,13 @@ int psp_dev_init(struct sp_device *sp)
goto e_err;
}
+ if (sp->set_psp_master_device)
+ sp->set_psp_master_device(sp);
+
ret = psp_init(psp);
if (ret)
goto e_irq;
- if (sp->set_psp_master_device)
- sp->set_psp_master_device(sp);
-
/* Enable interrupt */
iowrite32(-1, psp->io_regs + psp->vdata->inten_reg);
diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index 5c9d47f3be37..1631d9851e54 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -12,8 +12,9 @@
#include
#include
#include
+#include
+#include
#include
-#include
#include
#include "psp-dev.h"
@@ -21,25 +22,64 @@
static bool psp_dead;
+struct dma_buffer *psp_tee_alloc_dmabuf(unsigned long size, gfp_t gfp)
+{
+ struct psp_device *psp = psp_get_master_device();
+ struct dma_buffer *dma_buf;
+ struct iommu_domain *dom;
+
+ if (!psp || !size)
+ return NULL;
+
+ dma_buf = kzalloc(sizeof(*dma_buf), GFP_KERNEL);
+ if (!dma_buf)
+ return NULL;
+
+ dma_buf->vaddr = dma_alloc_coherent(psp->dev, size, _buf->dma, gfp);
+ if (!dma_buf->vaddr || !dma_buf->dma) {
+ kfree(dma_buf);
+ return NULL;
+ }
+
+ dma_buf->size = size;
+
+ dom = iommu_get_domain_for_dev(psp->dev);
+ if (dom)
+ dma_buf->paddr = iommu_iova_to_phys(dom, dma_buf->dma);
+ else
+ dma_buf->paddr = dma_buf->dma;
+
+ return dma_buf;
+}
+EXPORT_SYMBOL(psp_tee_alloc_dmabuf);
+
+void psp_tee_free_dmabuf(struct dma_buffer *dma_buf)
+{
+ struct psp_device *psp = psp_get_master_device();
+
+ if (!psp || !dma_buf)
+ return;
+
+ dma_free_coherent(psp->dev, dma_buf->size,
+ dma_buf->vaddr, dma_buf->dma);
+
+ kfree(dma_buf);
+}
+EXPORT_SYMBOL(psp_tee_free_dmabuf);
+
static int tee_alloc_ring(struct psp_tee_device *tee, int ring_size)
{
struct ring_buf_manager *rb_mgr = >rb_mgr;
- void *start_addr;
if (!ring_size)
return -EINVAL;
- /* We need actual physical address instead of DMA address, since
-* Trusted OS running on AMD Secure Processor will map this region
-*/
- start_addr = (void *)__get_free_pages(GFP_KERNEL, get_order(ring_size));
- if (!start_addr)
+ rb_mgr->ring_buf = psp_tee_alloc_dmabuf(ring_size,
+ GFP_KERNEL | __GFP_ZERO);
+ if (!rb_mgr->ring_buf) {
+ dev_err(tee->dev, "ring allocation failed\n");
return -ENOMEM;
-
- memset(start_addr, 0x0, ring_size);
- rb_mgr->ring_start = start_addr;
- rb_mgr->ring_size = ring_size;
- rb_mgr->ring_pa = __psp_pa(start_addr);
+ }
mutex_init(_mgr->mutex);
return 0;
@@ -49,15 +89,8 @@ static void tee_free_ring(struct psp_tee_device *tee)
{
[PATCH 1/1] crypto: ccp - Allocate TEE ring and cmd buffer using DMA APIs
For AMD Secure Processor (ASP) to map and access TEE ring buffer, the
ring buffer address sent by host to ASP must be a real physical
address and the pages must be physically contiguous.
In a virtualized environment though, when the driver is running in a
guest VM, the pages allocated by __get_free_pages() may not be
contiguous in the host (or machine) physical address space. Guests
will see a guest (or pseudo) physical address and not the actual host
(or machine) physical address. The TEE running on ASP cannot decipher
pseudo physical addresses. It needs host or machine physical address.
To resolve this problem, use DMA APIs for allocating buffers that must
be shared with TEE. This will ensure that the pages are contiguous in
host (or machine) address space. If the DMA handle is an IOVA,
translate it into a physical address before sending it to ASP.
This patch also exports two APIs (one for buffer allocation and
another to free the buffer). This API can be used by AMD-TEE driver to
share buffers with TEE.
Fixes: 33960acccfbd ("crypto: ccp - add TEE support for Raven Ridge")
Cc: Tom Lendacky
Cc: sta...@vger.kernel.org
Signed-off-by: Rijo Thomas
Co-developed-by: Jeshwanth
Signed-off-by: Jeshwanth
Reviewed-by: Devaraj Rangasamy
---
drivers/crypto/ccp/psp-dev.c | 6 +-
drivers/crypto/ccp/tee-dev.c | 116 ++-
drivers/crypto/ccp/tee-dev.h | 9 +--
include/linux/psp-tee.h | 47 ++
4 files changed, 127 insertions(+), 51 deletions(-)
diff --git a/drivers/crypto/ccp/psp-dev.c b/drivers/crypto/ccp/psp-dev.c
index c9c741ac8442..2b86158d7435 100644
--- a/drivers/crypto/ccp/psp-dev.c
+++ b/drivers/crypto/ccp/psp-dev.c
@@ -161,13 +161,13 @@ int psp_dev_init(struct sp_device *sp)
goto e_err;
}
+ if (sp->set_psp_master_device)
+ sp->set_psp_master_device(sp);
+
ret = psp_init(psp);
if (ret)
goto e_irq;
- if (sp->set_psp_master_device)
- sp->set_psp_master_device(sp);
-
/* Enable interrupt */
iowrite32(-1, psp->io_regs + psp->vdata->inten_reg);
diff --git a/drivers/crypto/ccp/tee-dev.c b/drivers/crypto/ccp/tee-dev.c
index 5c9d47f3be37..1631d9851e54 100644
--- a/drivers/crypto/ccp/tee-dev.c
+++ b/drivers/crypto/ccp/tee-dev.c
@@ -12,8 +12,9 @@
#include
#include
#include
+#include
+#include
#include
-#include
#include
#include "psp-dev.h"
@@ -21,25 +22,64 @@
static bool psp_dead;
+struct dma_buffer *psp_tee_alloc_dmabuf(unsigned long size, gfp_t gfp)
+{
+ struct psp_device *psp = psp_get_master_device();
+ struct dma_buffer *dma_buf;
+ struct iommu_domain *dom;
+
+ if (!psp || !size)
+ return NULL;
+
+ dma_buf = kzalloc(sizeof(*dma_buf), GFP_KERNEL);
+ if (!dma_buf)
+ return NULL;
+
+ dma_buf->vaddr = dma_alloc_coherent(psp->dev, size, _buf->dma, gfp);
+ if (!dma_buf->vaddr || !dma_buf->dma) {
+ kfree(dma_buf);
+ return NULL;
+ }
+
+ dma_buf->size = size;
+
+ dom = iommu_get_domain_for_dev(psp->dev);
+ if (dom)
+ dma_buf->paddr = iommu_iova_to_phys(dom, dma_buf->dma);
+ else
+ dma_buf->paddr = dma_buf->dma;
+
+ return dma_buf;
+}
+EXPORT_SYMBOL(psp_tee_alloc_dmabuf);
+
+void psp_tee_free_dmabuf(struct dma_buffer *dma_buf)
+{
+ struct psp_device *psp = psp_get_master_device();
+
+ if (!psp || !dma_buf)
+ return;
+
+ dma_free_coherent(psp->dev, dma_buf->size,
+ dma_buf->vaddr, dma_buf->dma);
+
+ kfree(dma_buf);
+}
+EXPORT_SYMBOL(psp_tee_free_dmabuf);
+
static int tee_alloc_ring(struct psp_tee_device *tee, int ring_size)
{
struct ring_buf_manager *rb_mgr = >rb_mgr;
- void *start_addr;
if (!ring_size)
return -EINVAL;
- /* We need actual physical address instead of DMA address, since
-* Trusted OS running on AMD Secure Processor will map this region
-*/
- start_addr = (void *)__get_free_pages(GFP_KERNEL, get_order(ring_size));
- if (!start_addr)
+ rb_mgr->ring_buf = psp_tee_alloc_dmabuf(ring_size,
+ GFP_KERNEL | __GFP_ZERO);
+ if (!rb_mgr->ring_buf) {
+ dev_err(tee->dev, "ring allocation failed\n");
return -ENOMEM;
-
- memset(start_addr, 0x0, ring_size);
- rb_mgr->ring_start = start_addr;
- rb_mgr->ring_size = ring_size;
- rb_mgr->ring_pa = __psp_pa(start_addr);
+ }
mutex_init(_mgr->mutex);
return 0;
@@ -49,15 +89,8 @@ static void tee_free_ring(struct psp_tee_device *tee)
{
struct ring_buf_manager *rb_mgr = >rb_mgr;
- if (!rb_mgr->ring_start)
- return;
+ psp_tee_free_dmabuf(rb_mgr->ring_buf);
- free_pages((unsigned long)rb_mgr->ring_start,
-
Re: [Intel-gfx] [PATCH v2 4/5] drm/i915/guc: Add GuC CT specific debug print wrappers
On 05/12/2022 18:44, Michal Wajdeczko wrote: On 05.12.2022 14:16, Tvrtko Ursulin wrote: On 02/12/2022 20:14, John Harrison wrote: and while for dbg level messages it doesn't matter, I assume we should be consistent for err/warn/info messages (as those will eventually show up to the end user) so let maintainers decide here what is expectation here Could we have some examples pasted here, of the end result of this series, for all message "categories" (origins, macros, whatever)? GT initialisation: gt_err(gt, "Failed to allocate scratch page\n"); i915 :00:02.0: [drm] GT0: Failed to allocate scratch page G2H notification handler: guc_err(guc, "notification: Invalid length %u for deregister done\n", len); i915 :00:02.0: [drm] GT0: GuC notification: Invalid length 0 for deregister done please note that today this message is coded as: drm_err(_to_gt(guc)->i915->drm, "Invalid length %u\n", len); -> i915 :00:02.0: [drm] Invalid length %u which makes this rather an example of meaningless log Okay, so log text needs improving anyway which is orthogonal. I'm not liking the inconsistency between gt_err and guc_err where with latter callers either need to start the message with lower case because of the unstructured "GuC " prefix added. Which then reads bad if callers do guc_err(guc, "Error X happend"). Looks like Michal was pointing out the same thing, AFAIU at least when re-reading the thread now. Why wouldn't this work: guc_err(guc, "Invalid length %u for deregister done notification\n", len); i915 :00:02.0: [drm] GT0: GuC: Invalid length 0 for deregister done notification +1 Or if the use case for adding custom prefixes is strong then maybe consider: guc_err(guc, "notification", "Invalid length 0 for deregister done"); i915 :00:02.0: [drm] GT0: GuC notification: Invalid length 0 for deregister done guc_err(guc, "", "Error X"); i915 :00:02.0: [drm] GT0: GuC: Error X -1 this will make logging macros too different from others (unless we hide/use prefixes inside macros only, but I'm not sure there is any ROI) Yeah I said if the use case is strong, no strong opinion either way. CTB initialisation: ct_probe_error(ct, "Failed to control/%s CTB (%pe)\n", str_enable_disable(enable), ERR_PTR(err)); i915 :00:02.0: [drm] GT0: GuC CT Failed to control/enable CTB (EINVAL) Okay same as above. Random meaningless (to me) message that is apparently a display thing: drm_dbg_kms(_priv->drm, "disabling %s\n", pll->info->name); i915 :00:02.0: [drm:intel_disable_shared_dpll [i915]] disabling PORT PLL B Plan is to not touch outside gt/. I'm sure you can extrapolate to all other forms of dbg, notice, info, etc. without me having to manually type each one out, given that they are all identical. Personally, I think the above should be just: gt_err(gt, "Failed to allocate scratch page\n"); i915 :00:02.0: [drm] GT0: Failed to allocate scratch page gt_err(guc_to_gt(guc), "G2H: Invalid length for deregister done: %u\n", len); i915 :00:02.0: [drm] GT0: G2H: Invalid length for deregister done: 0 that's probably should be: "Invalid length for G2H deregister done: %u\n and it will still just look fine if we auto append the 'GuC' prefix: i915 :00:02.0: [drm] GT0: GuC: Invalid length for G2H deregister gt_probe_error(ct_to_gt(ct), "Failed to %s CT %d buffer (%pe)\n", str_enable_disable(enable), send ? "SEND" : "RECV", ERR_PTR(err)); i915 :00:02.0: [drm] GT0: Failed to enable CT SEND buffer (EINVAL) having "GuC/CT" prefix here will also look fine: i915 :00:02.0: [drm] GT0: GuC: Failed to enable CT SEND buffer i915 :00:02.0: [drm] GT0: GuC: CT: Failed to enable SEND buffer i915 :00:02.0: [drm] GT0: CT: Failed to enable SEND buffer Works for me. We could but it seems we agreed some weeks ago to consolidate the existing CT_ERROR macros and such in this exercise. At least no objections were raised to that plan. If now we want to go back on that, and if you want to have guc_to_gt(guc) in all gt/uc/ call sites that's fine by me, but please get some acks and consensus from people who work in that area. And under that option someone would also need to convert the CT code to new macros. while the main goal of this series was to have GT# appended to the log messages but we also wanted to simplify the use of the logging macros by passing the component pointer directly (with extra *bonus* that allows to auto append component specific prefix, if any, like CT macros do) IMHO adding guc_xxx() macros with "GuC:" prefix will do the trick and since many of the existing GuC related logs are already broken or incomplete, we might fix them accordingly. In other words in addition to gt_xxx() I still want additional guc_xxx() macros (as it will allow us to fix related messages) and ct_xxx() macros (as we already have CT_xxx so no need to change anything) Both approaches are fine by me as long as it's logical and consistent and we manage
Re: [PATCH] drm/amd/display: Added pointer check
Am 06.12.22 um 11:47 schrieb Denis Arefev:
Return value of a function 'dc_create_state' is
dereferenced at amdgpu_dm.c:2027 without checking for null
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0f7749e9424d..529483997154 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1960,7 +1960,9 @@ static int dm_resume(void *handle)
dc_release_state(dm_state->context);
dm_state->context = dc_create_state(dm->dc);
/* TODO: Remove dc_state->dccg, use dc->dccg directly. */
- dc_resource_state_construct(dm->dc, dm_state->context);
+ if (dm_state->context) {
+ dc_resource_state_construct(dm->dc, dm_state->context);
+ }
Single line statements shouldn't have an {} around them.
Apart from that I think error handling when dm_state->context can't be
created would be more appropriate, but that's something our display team
needs to look at.
Regards,
Christian.
/* Before powering on DC we need to re-initialize DMUB. */
r = dm_dmub_hw_init(adev);
[PATCH] drm/amd/display: Added pointer check
Return value of a function 'dc_create_state' is
dereferenced at amdgpu_dm.c:2027 without checking for null
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0f7749e9424d..529483997154 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1960,7 +1960,9 @@ static int dm_resume(void *handle)
dc_release_state(dm_state->context);
dm_state->context = dc_create_state(dm->dc);
/* TODO: Remove dc_state->dccg, use dc->dccg directly. */
- dc_resource_state_construct(dm->dc, dm_state->context);
+ if (dm_state->context) {
+ dc_resource_state_construct(dm->dc, dm_state->context);
+ }
/* Before powering on DC we need to re-initialize DMUB. */
r = dm_dmub_hw_init(adev);
--
2.25.1
RE: [PATCH v4] drm/edid: ignore the CEA modes not defined in CEA-861-D
Hi Jani
May I have your comment about this patch?
BTW, this is the link to the binary EDID,
https://gitlab.freedesktop.org/drm/intel/-/issues/6153#note_1558419.
Thank you.
Regards
William
-Original Message-
From: Tseng, William
Sent: Tuesday, September 20, 2022 4:23 PM
To: Jani Nikula ; dri-devel@lists.freedesktop.org
Cc: Ville Syrjälä ; Wayne Lin
; Lee, Shawn C
Subject: RE: [PATCH v4] drm/edid: ignore the CEA modes not defined in CEA-861-D
For EDID, please refer to the attachment on the link.
https://gitlab.freedesktop.org/drm/intel/-/issues/6153#note_1558419
Regards
William
-Original Message-
From: Jani Nikula
Sent: Tuesday, September 20, 2022 2:49 PM
To: Tseng, William ; dri-devel@lists.freedesktop.org
Cc: Tseng, William ; Ville Syrjälä
; Wayne Lin ; Lee, Shawn C
Subject: Re: [PATCH v4] drm/edid: ignore the CEA modes not defined in CEA-861-D
On Tue, 20 Sep 2022, William Tseng wrote:
> This is a workaround for HDMI 1.4 sink which has a CEA mode with
> higher vic than what is defined in CEA-861-D.
>
> As an example, a HDMI 1.4 sink has the video format 2560x1080p to be
> displayed and the video format is indicated by both SVD (with vic 90
> and picture aspect ratio 64:27) and DTD. When connecting to such
> sink, source can't output the video format in SVD because an error is
> returned by drm_hdmi_avi_infoframe_from_display_mode(), which can't
> fill the infoframe with picture aspect ratio 64:27 and the vic, which
> is originally 90 and is changed to 0 by drm_mode_cea_vic().
>
> To work around it, do not set the vic 0 so the corresponding mode may
> be accepted in drm_hdmi_avi_infoframe_from_display_mode() and be dispalyed.
>
> v1: initial version.
> v2: change the logic in drm_hdmi_avi_infoframe_from_display_mode().
> v3: fix typo.
> v4: add revision history.
>
> Cc: Ville Syrjälä
> Cc: Jani Nikula
> Cc: Wayne Lin
> Cc: Lee Shawn C
> Signed-off-by: William Tseng
Please attach the offending EDID to the bug [1]. I won't ack this before we see
the EDID in question.
BR,
Jani.
[1] https://gitlab.freedesktop.org/drm/intel/-/issues/6153
> ---
> drivers/gpu/drm/drm_edid.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index eaa819381281..3c6a4e09b2d6 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -6640,7 +6640,8 @@ static u8 drm_mode_hdmi_vic(const struct
> drm_connector *connector, }
>
> static u8 drm_mode_cea_vic(const struct drm_connector *connector,
> -const struct drm_display_mode *mode)
> +const struct drm_display_mode *mode,
> +bool is_hdmi2_sink)
> {
> u8 vic;
>
> @@ -6660,7 +6661,7 @@ static u8 drm_mode_cea_vic(const struct drm_connector
> *connector,
>* HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
>* have to make sure we dont break HDMI 1.4 sinks.
>*/
> - if (!is_hdmi2_sink(connector) && vic > 64)
> + if (!is_hdmi2_sink && vic > 64)
> return 0;
>
> return vic;
> @@ -6691,7 +6692,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct
> hdmi_avi_infoframe *frame,
> if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> frame->pixel_repeat = 1;
>
> - vic = drm_mode_cea_vic(connector, mode);
> + vic = drm_mode_cea_vic(connector, mode, true);
> hdmi_vic = drm_mode_hdmi_vic(connector, mode);
>
> frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE; @@ -6735,7 +6736,8
> @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> picture_aspect = HDMI_PICTURE_ASPECT_NONE;
> }
>
> - frame->video_code = vic;
> + frame->video_code = drm_mode_cea_vic(connector, mode,
> + is_hdmi2_sink(connector));
> frame->picture_aspect = picture_aspect;
> frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
> frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
--
Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v3 3/3] drm/tiny: ili9486: remove conflicting framebuffers
Hi Am 06.12.22 um 10:41 schrieb Neil Armstrong: Hi Carlo, On 06/12/2022 09:34, Carlo Caione wrote: For platforms using simplefb / efifb, call drm_aperture_remove_framebuffers() to remove the conflicting framebuffer. Conflicting framebuffer on the SPI display ? How is that possible ? Calling drm_aperture_remove_framebuffers() is only required if the graphics card may have been pre-initialized by the system, such as a VGA-compatible card on a PC. Could the SPI display have been initialized by the firmware? If not, the call should be left out. Best regards Thomas The meson_drm should already do this, no ? Neil Signed-off-by: Carlo Caione --- drivers/gpu/drm/tiny/ili9486.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c index 14a9e6ad2d15..6fd4d42437fd 100644 --- a/drivers/gpu/drm/tiny/ili9486.c +++ b/drivers/gpu/drm/tiny/ili9486.c @@ -14,6 +14,7 @@ #include +#include #include #include #include @@ -238,6 +239,10 @@ static int ili9486_probe(struct spi_device *spi) if (ret) return ret; + ret = drm_aperture_remove_framebuffers(false, _driver); + if (ret) + return ret; + drm_mode_config_reset(drm); ret = drm_dev_register(drm, 0); -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
[no subject]
Date: Thu, 10 Nov 2022 16:47:26 +0300
Subject: [PATCH] drm/amdgpu/display: Add pointer check
Return value of a function 'dc_create_state' is
dereferenced at amdgpu_dm.c:2027 without checking for null
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 0f7749e9424d..529483997154 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1960,7 +1960,9 @@ static int dm_resume(void *handle)
dc_release_state(dm_state->context);
dm_state->context = dc_create_state(dm->dc);
/* TODO: Remove dc_state->dccg, use dc->dccg directly. */
- dc_resource_state_construct(dm->dc, dm_state->context);
+ if (dm_state->context) {
+ dc_resource_state_construct(dm->dc, dm_state->context);
+ }
/* Before powering on DC we need to re-initialize DMUB. */
r = dm_dmub_hw_init(adev);
--
2.25.1
Re: [PATCH v3 3/3] drm/tiny: ili9486: remove conflicting framebuffers
Hi Carlo, On 06/12/2022 09:34, Carlo Caione wrote: For platforms using simplefb / efifb, call drm_aperture_remove_framebuffers() to remove the conflicting framebuffer. Conflicting framebuffer on the SPI display ? How is that possible ? The meson_drm should already do this, no ? Neil Signed-off-by: Carlo Caione --- drivers/gpu/drm/tiny/ili9486.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c index 14a9e6ad2d15..6fd4d42437fd 100644 --- a/drivers/gpu/drm/tiny/ili9486.c +++ b/drivers/gpu/drm/tiny/ili9486.c @@ -14,6 +14,7 @@ #include +#include #include #include #include @@ -238,6 +239,10 @@ static int ili9486_probe(struct spi_device *spi) if (ret) return ret; + ret = drm_aperture_remove_framebuffers(false, _driver); + if (ret) + return ret; + drm_mode_config_reset(drm); ret = drm_dev_register(drm, 0);
Re: [PATCH v8 06/14] drm: bridge: samsung-dsim: Handle proper DSI host initialization
On 05.12.22 16:37, Frieder Schrempf wrote:
> Hi Dave,
>
> On 05.12.22 16:20, Dave Stevenson wrote:
>> Hi Frieder
>>
>> On Mon, 5 Dec 2022 at 07:30, Frieder Schrempf
>> wrote:
>>>
>>> On 02.12.22 15:55, Dave Stevenson wrote:
Hi Marek
On Fri, 2 Dec 2022 at 12:21, Marek Vasut wrote:
>
> On 12/2/22 11:52, Marek Szyprowski wrote:
>> Hi,
>>
>> Sorry for delay, I was on a sick leave last 2 weeks.
>>
>> On 28.11.2022 15:43, Jagan Teki wrote:
>>> ,On Sat, Nov 26, 2022 at 3:44 AM Marek Vasut wrote:
On 11/23/22 21:09, Jagan Teki wrote:
> On Sat, Nov 19, 2022 at 7:45 PM Marek Vasut wrote:
>> On 11/17/22 14:04, Marek Szyprowski wrote:
>>> On 17.11.2022 05:58, Marek Vasut wrote:
On 11/10/22 19:38, Jagan Teki wrote:
> DSI host initialization handling in previous exynos dsi driver has
> some pitfalls. It initializes the host during host transfer() hook
> that is indeed not the desired call flow for I2C and any other DSI
> configured downstream bridges.
>
> Host transfer() is usually triggered for downstream DSI panels or
> bridges and I2C-configured-DSI bridges miss these host
> initialization
> as these downstream bridges use bridge operations hooks like
> pre_enable,
> and enable in order to initialize or set up the host.
>
> This patch is trying to handle the host init handler to satisfy
> all
> downstream panels and bridges. Added the DSIM_STATE_REINITIALIZED
> state
> flag to ensure that host init is also done on first cmd transfer,
> this
> helps existing DSI panels work on exynos platform (form Marek
> Szyprowski).
>
> v8, v7, v6, v5:
> * none
>
> v4:
> * update init handling to ensure host init done on first cmd
> transfer
>
> v3:
> * none
>
> v2:
> * check initialized state in samsung_dsim_init
>
> v1:
> * keep DSI init in host transfer
>
> Signed-off-by: Marek Szyprowski
> Signed-off-by: Jagan Teki
> ---
> drivers/gpu/drm/bridge/samsung-dsim.c | 25
> +
> include/drm/bridge/samsung-dsim.h | 5 +++--
> 2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c
> b/drivers/gpu/drm/bridge/samsung-dsim.c
> index bb1f45fd5a88..ec7e01ae02ea 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1234,12 +1234,17 @@ static void
> samsung_dsim_disable_irq(struct
> samsung_dsim *dsi)
> disable_irq(dsi->irq);
> }
> -static int samsung_dsim_init(struct samsung_dsim *dsi)
> +static int samsung_dsim_init(struct samsung_dsim *dsi, unsigned
> int
> flag)
> {
> const struct samsung_dsim_driver_data *driver_data =
> dsi->driver_data;
> +if (dsi->state & flag)
> +return 0;
> +
> samsung_dsim_reset(dsi);
> -samsung_dsim_enable_irq(dsi);
> +
> +if (!(dsi->state & DSIM_STATE_INITIALIZED))
> +samsung_dsim_enable_irq(dsi);
> if (driver_data->reg_values[RESET_TYPE] ==
> DSIM_FUNCRST)
> samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
> @@ -1250,6 +1255,8 @@ static int samsung_dsim_init(struct
> samsung_dsim *dsi)
> samsung_dsim_set_phy_ctrl(dsi);
> samsung_dsim_init_link(dsi);
> +dsi->state |= flag;
> +
> return 0;
> }
> @@ -1269,6 +1276,10 @@ static void
> samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
> }
> dsi->state |= DSIM_STATE_ENABLED;
> +
> +ret = samsung_dsim_init(dsi, DSIM_STATE_INITIALIZED);
> +if (ret)
> +return;
> }
> static void samsung_dsim_atomic_enable(struct drm_bridge
> *bridge,
> @@ -1458,12 +1469,9 @@ static ssize_t
> samsung_dsim_host_transfer(struct mipi_dsi_host *host,
> if (!(dsi->state & DSIM_STATE_ENABLED))
Re: [Intel-gfx] [PATCH v2 4/6] drm/i915/gsc: Do a driver-FLR on unload if GSC was loaded
On Mon, Dec 05, 2022 at 05:19:06PM -0800, Daniele Ceraolo Spurio wrote: > If the GSC was loaded, the only way to stop it during the driver unload > flow is to do a driver-FLR. > The driver-initiated FLR is not the same as PCI config space FLR in > that it doesn't reset the SGUnit and doesn't modify the PCI config > space. Thus, it doesn't require a re-enumeration of the PCI BARs. > However, the driver-FLR does cause a memory wipe of graphics memory > on all discrete GPU platforms or a wipe limited to stolen memory > on the integrated GPU platforms. > > We perform the FLR as the last action before releasing the MMIO bar, so > that we don't have to care about the consequences of the reset on the > unload flow. > > v2: rename FLR function, add comment to explain FLR impact (Rodrigo), > better explain why GSC needs FLR (Alan) > > Signed-off-by: Daniele Ceraolo Spurio > Signed-off-by: Alan Previn > Cc: Rodrigo Vivi Reviewed-by: Rodrigo Vivi > --- > drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c | 23 + > drivers/gpu/drm/i915/i915_reg.h | 3 ++ > drivers/gpu/drm/i915/intel_uncore.c | 58 +++ > drivers/gpu/drm/i915/intel_uncore.h | 13 + > 4 files changed, 97 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c > b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c > index f88069ab71ab..e73d4440c5e8 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_fw.c > @@ -166,6 +166,29 @@ int intel_gsc_uc_fw_upload(struct intel_gsc_uc *gsc) > if (err) > goto fail; > > + /* > + * GSC is only killed by an FLR, so we need to trigger one on unload to > + * make sure we stop it. This is because we assign a chunk of memory to > + * the GSC as part of the FW load , so we need to make sure it stops > + * using it when we release it to the system on driver unload. Note that > + * this is not a problem of the unload per-se, because the GSC will not > + * touch that memory unless there are requests for it coming from the > + * driver; therefore, no accesses will happen while i915 is not loaded, > + * but if we re-load the driver then the GSC might wake up and try to > + * access that old memory location again. > + * Given that an FLR is a very disruptive action (see the FLR function > + * for details), we want to do it as the last action before releasing > + * the access to the MMIO bar, which means we need to do it as part of > + * the primary uncore cleanup. > + * An alternative approach to the FLR would be to use a memory location > + * that survives driver unload, like e.g. stolen memory, and keep the > + * GSC loaded across reloads. However, this requires us to make sure we > + * preserve that memory location on unload and then determine and > + * reserve its offset on each subsequent load, which is not trivial, so > + * it is easier to just kill everything and start fresh. > + */ > + intel_uncore_set_flr_on_fini(>i915->uncore); > + > err = gsc_fw_load(gsc); > if (err) > goto fail; > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 0b90fe6a28f7..b95d533652a4 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -118,6 +118,9 @@ > > #define GU_CNTL _MMIO(0x101010) > #define LMEM_INIT REG_BIT(7) > +#define DRIVERFLR REG_BIT(31) > +#define GU_DEBUG _MMIO(0x101018) > +#define DRIVERFLR_STATUS REG_BIT(31) > > #define GEN6_STOLEN_RESERVED _MMIO(0x1082C0) > #define GEN6_STOLEN_RESERVED_ADDR_MASK (0xFFF << 20) > diff --git a/drivers/gpu/drm/i915/intel_uncore.c > b/drivers/gpu/drm/i915/intel_uncore.c > index 8006a6c61466..3bfb4af0df78 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.c > +++ b/drivers/gpu/drm/i915/intel_uncore.c > @@ -2703,6 +2703,61 @@ void intel_uncore_prune_engine_fw_domains(struct > intel_uncore *uncore, > } > } > > +/* > + * The driver-initiated FLR is the highest level of reset that we can trigger > + * from within the driver. It is different from the PCI FLR in that it > doesn't > + * fully reset the SGUnit and doesn't modify the PCI config space and > therefore > + * it doesn't require a re-enumeration of the PCI BARs. However, the > + * driver-initiated FLR does still cause a reset of both GT and display and a > + * memory wipe of local and stolen memory, so recovery would require a full > HW > + * re-init and saving/restoring (or re-populating) the wiped memory. Since we > + * perform the FLR as the very last action before releasing access to the HW > + * during the driver release flow, we don't attempt recovery at all, because > + * if/when a new instance of i915 is bound to the device it will do a full > + * re-init anyway. >
[PATCH] drm/msm/dpu: Add check for cstate
As kzalloc may fail and return NULL pointer,
it should be better to check cstate
in order to avoid the NULL pointer dereference
in __drm_atomic_helper_crtc_reset.
Fixes: 1cff7440a86e ("drm/msm: Convert to using
__drm_atomic_helper_crtc_reset() for reset.")
Signed-off-by: Jiasheng Jiang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 13ce321283ff..22c2787b7b38 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -968,7 +968,10 @@ static void dpu_crtc_reset(struct drm_crtc *crtc)
if (crtc->state)
dpu_crtc_destroy_state(crtc, crtc->state);
- __drm_atomic_helper_crtc_reset(crtc, >base);
+ if (cstate)
+ __drm_atomic_helper_crtc_reset(crtc, >base);
+ else
+ __drm_atomic_helper_crtc_reset(crtc, NULL);
}
/**
--
2.25.1
[PATCH] drm/msm/dpu: Add check for pstates
As kzalloc may fail and return NULL pointer,
it should be better to check pstates
in order to avoid the NULL pointer dereference.
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Jiasheng Jiang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 13ce321283ff..f51cb46ecfd6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1150,6 +1150,8 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
pstates = kzalloc(sizeof(*pstates) * DPU_STAGE_MAX * 4, GFP_KERNEL);
+ if (!pstates)
+ return -ENOMEM;
if (!crtc_state->enable || !crtc_state->active) {
DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip
atomic_check\n",
--
2.25.1
[PATCH] drm/msm/dpu: Add check for cstate
As kzalloc may fail and return NULL pointer,
it should be better to check pstates
in order to avoid the NULL pointer dereference
later.
Fixes: 25fdd5933e4c ("drm/msm: Add SDM845 DPU support")
Signed-off-by: Jiasheng Jiang
---
drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 13ce321283ff..f51cb46ecfd6 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1150,6 +1150,8 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
bool needs_dirtyfb = dpu_crtc_needs_dirtyfb(crtc_state);
pstates = kzalloc(sizeof(*pstates) * DPU_STAGE_MAX * 4, GFP_KERNEL);
+ if (!pstates)
+ return -ENOMEM;
if (!crtc_state->enable || !crtc_state->active) {
DRM_DEBUG_ATOMIC("crtc%d -> enable %d, active %d, skip
atomic_check\n",
--
2.25.1
