Re: [Freedreno] [PATCH] devcoredump: increase the device delete timeout to 10 mins
On Tue, 2022-02-08 at 17:55 -0800, Abhinav Kumar wrote: > > Are you suggesting something like below? > > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c > index 42dcf96..14203d0 100644 > --- a/fs/sysfs/file.c > No, for sure not, but I guess from the looks of this patch there's no way to do something like that for just an individual attribute... Oh well. johannes
Re: [Freedreno] [PATCH v2 2/4] arm64: dts: qcom: sc7280: Add support for eDP panel on CRD
Hi Bjorn,
1. I will change the edp_out label to mdss_edp_out.
2. The "pm8350c_pwm" node is part of the dependent series mentioned in the
cover letter. Below is the patch for the same:
https://patchwork.kernel.org/project/linux-arm-msm/patch/1637917920-22041-4-git-send-email-quic_c_ska...@quicinc.com/
3. I will move the edp_backlight and edp_panel nodes to the root.
Thank you,
Sankeerth
-Original Message-
From: Bjorn Andersson
Sent: Wednesday, February 9, 2022 5:23 AM
To: Sankeerth Billakanti (QUIC)
Cc: dri-de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org;
freedreno@lists.freedesktop.org; linux-ker...@vger.kernel.org;
devicet...@vger.kernel.org; agr...@kernel.org; robh...@kernel.org;
robdcl...@gmail.com; seanp...@chromium.org; swb...@chromium.org;
diand...@chromium.org; krzysztof.kozlow...@canonical.com;
thierry.red...@gmail.com; s...@ravnborg.org; airl...@linux.ie; dan...@ffwll.ch;
quic_kalyant ; Abhinav Kumar (QUIC)
; Kuogee Hsieh (QUIC) ;
quic_mkrishn
Subject: Re: [PATCH v2 2/4] arm64: dts: qcom: sc7280: Add support for eDP panel
on CRD
On Tue 08 Feb 07:18 PST 2022, Sankeerth Billakanti wrote:
> Enable the eDP display panel support without HPD on sc7280 platform.
>
> Signed-off-by: Sankeerth Billakanti
> ---
>
> Changes in v2:
> - sort node references alphabetically
> - improve readability
> - move the pwm pinctrl to pwm node
> - move the regulators to root
> - define backlight power
> - remove dummy regulator node
> - cleanup pinctrl definitions
>
> arch/arm64/boot/dts/qcom/sc7280-crd.dts | 122
>
> arch/arm64/boot/dts/qcom/sc7280.dtsi| 2 -
> 2 files changed, 122 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index e2efbdd..bff2707 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -21,6 +21,34 @@
> chosen {
> stdout-path = "serial0:115200n8";
> };
> +
> + backlight_power: backlight-power {
> + compatible = "regulator-fixed";
> + regulator-name = "backlight_power";
> +
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> +
> + gpio = <_gpios 7 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_bl_power>;
> + };
> +
> + edp_power: edp-power {
> + compatible = "regulator-fixed";
> + regulator-name = "edp_power";
> +
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> +
> + gpio = < 80 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_panel_power>;
> + };
> };
>
> _rsc {
> @@ -76,6 +104,42 @@ ap_ts_pen_1v8: {
> };
> };
>
> +_out {
Sorry for missing this while merging changes in sc7280.dtsi. But it would be
really nice if this was labeled mdss_edp_out instead (or possibly defined
within the _edp node).
Now you will have _out and _out floating around away from the edp and dp
nodes...
> + remote-endpoint = <_panel_in>;
> +};
> +
> + {
> + status = "okay";
> +};
> +
> +_edp {
> + status = "okay";
> +
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l10c_0p8>;
> +};
> +
> +_edp_phy {
> + status = "okay";
> +
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l10c_0p8>;
> +};
> +
> +_dp {
> + status = "okay";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_hot_plug_det>;
> + data-lanes = <0 1>;
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l1b_0p8>;
> +};
> +
> +_mdp {
> + status = "okay";
> +};
> +
> _3v3_regulator {
> gpio = < 51 GPIO_ACTIVE_HIGH>;
> };
> @@ -84,7 +148,65 @@ ap_ts_pen_1v8: {
> pins = "gpio51";
> };
>
> +_pwm {
This label doesn't exist, so I won't be able to merge this patch.
> + status = "okay";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_bl_pwm>;
> +};
> +
> +_gpios {
> + edp_bl_power: edp-bl-power {
> + pins = "gpio7";
> + function = "normal";
> + qcom,drive-strength = ;
> + bias-disable;
> + output-low;
> + };
> +
> + edp_bl_pwm: edp-bl-pwm {
> + pins = "gpio8";
> + function = "func1";
> + qcom,drive-strength = ;
> + bias-disable;
> + output-low;
> + };
> +};
> +
> + {
> + edp_backlight: edp-backlight {
> + compatible = "pwm-backlight";
This is not a device on the mmio bus, so it should not love within the
> +
> + power-supply = <_power>;
> + pwms = <_pwm 3 65535>;
> + };
> +
> + edp_panel: edp_panel {
Ditto.
Regards,
Bjorn
> +
Re: [Freedreno] [PATCH v2 2/4] arm64: dts: qcom: sc7280: Add support for eDP panel on CRD
Hi Matthias,
I will implement the changes.
Thank you,
Sankeerth
-Original Message-
From: Matthias Kaehlcke
Sent: Wednesday, February 9, 2022 3:54 AM
To: Sankeerth Billakanti (QUIC)
Cc: dri-de...@lists.freedesktop.org; linux-arm-...@vger.kernel.org;
freedreno@lists.freedesktop.org; linux-ker...@vger.kernel.org;
devicet...@vger.kernel.org; agr...@kernel.org; bjorn.anders...@linaro.org;
robh...@kernel.org; robdcl...@gmail.com; seanp...@chromium.org;
swb...@chromium.org; diand...@chromium.org; krzysztof.kozlow...@canonical.com;
thierry.red...@gmail.com; s...@ravnborg.org; airl...@linux.ie; dan...@ffwll.ch;
quic_kalyant ; Abhinav Kumar (QUIC)
; Kuogee Hsieh (QUIC) ;
quic_mkrishn
Subject: Re: [PATCH v2 2/4] arm64: dts: qcom: sc7280: Add support for eDP panel
on CRD
On Tue, Feb 08, 2022 at 08:48:43PM +0530, Sankeerth Billakanti wrote:
> Enable the eDP display panel support without HPD on sc7280 platform.
>
> Signed-off-by: Sankeerth Billakanti
> ---
>
> Changes in v2:
> - sort node references alphabetically
> - improve readability
> - move the pwm pinctrl to pwm node
> - move the regulators to root
> - define backlight power
> - remove dummy regulator node
> - cleanup pinctrl definitions
>
> arch/arm64/boot/dts/qcom/sc7280-crd.dts | 122
>
> arch/arm64/boot/dts/qcom/sc7280.dtsi| 2 -
> 2 files changed, 122 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index e2efbdd..bff2707 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -21,6 +21,34 @@
> chosen {
> stdout-path = "serial0:115200n8";
> };
> +
> + backlight_power: backlight-power {
nit: the other fixed regulator in sc7280-idp.dtsi is called
'nvme_3v3_regulator', if you wanted to be consistent you could call this
backlight_3v3_regulator.
> + compatible = "regulator-fixed";
> + regulator-name = "backlight_power";
> +
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> +
> + gpio = <_gpios 7 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_bl_power>;
> + };
> +
> + edp_power: edp-power {
nit: see above
> + compatible = "regulator-fixed";
> + regulator-name = "edp_power";
> +
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> +
> + gpio = < 80 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_panel_power>;
> + };
> };
>
> _rsc {
> @@ -76,6 +104,42 @@ ap_ts_pen_1v8: {
> };
> };
>
> +_out {
> + remote-endpoint = <_panel_in>;
> +};
> +
> + {
> + status = "okay";
> +};
> +
> +_edp {
> + status = "okay";
> +
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l10c_0p8>;
> +};
> +
> +_edp_phy {
> + status = "okay";
> +
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l10c_0p8>;
> +};
> +
> +_dp {
should be before 'mdss_edp'.
> + status = "okay";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_hot_plug_det>;
> + data-lanes = <0 1>;
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l1b_0p8>;
> +};
> +
> +_mdp {
> + status = "okay";
> +};
> +
> _3v3_regulator {
> gpio = < 51 GPIO_ACTIVE_HIGH>;
> };
> @@ -84,7 +148,65 @@ ap_ts_pen_1v8: {
> pins = "gpio51";
> };
>
> +_pwm {
> + status = "okay";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_bl_pwm>;
> +};
> +
> +_gpios {
should be before 'pm8350c_pwm'
> + edp_bl_power: edp-bl-power {
> + pins = "gpio7";
> + function = "normal";
> + qcom,drive-strength = ;
> + bias-disable;
> + output-low;
> + };
> +
> + edp_bl_pwm: edp-bl-pwm {
> + pins = "gpio8";
> + function = "func1";
> + qcom,drive-strength = ;
> + bias-disable;
> + output-low;
> + };
> +};
> +
> + {
> + edp_backlight: edp-backlight {
> + compatible = "pwm-backlight";
> +
> + power-supply = <_power>;
> + pwms = <_pwm 3 65535>;
> + };
> +
> + edp_panel: edp_panel {
in difference to labels node names should use dashes as separator, not
underscores (i.e. 'edp-panel')
> + compatible = "sharp,lq140m1jw46";
> +
> + power-supply = <_power>;
> + backlight = <_backlight>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + edp_panel_in:
Re: [Freedreno] [PATCH 3/3] dt-bindings: display: msm: Add binding for msm8998 dpu
On Thu, 13 Jan 2022 16:51:11 +0200, Jami Kettunen wrote: > From: AngeloGioacchino Del Regno > > Add yaml binding for msm8998 dpu1 support. > > Signed-off-by: AngeloGioacchino Del Regno > > Signed-off-by: Jami Kettunen > --- > .../bindings/display/msm/dpu-msm8998.yaml | 219 ++ > 1 file changed, 219 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-msm8998.yaml > Reviewed-by: Rob Herring
Re: [Freedreno] [PATCH] devcoredump: increase the device delete timeout to 10 mins
Hi Johannes
On 2/8/2022 1:54 PM, Johannes Berg wrote:
On Tue, 2022-02-08 at 13:40 -0800, Abhinav Kumar wrote:
I am checking what usermode sees and will get back ( I didnt see an
error do most likely it was EOF ). I didnt follow the second part.
I think probably it got -ENODEV, looking at kernfs_file_read_iter().
If the file descriptor read returns EOF, even if we consider them
separate how will it resolve this issue?
My earlier questions were related to fixing it in devcoredump to detect
and fix it there. Are you suggesting to fix in usermode instead? How?
Yeah, no, you cannot fix it in userspace.
But I just followed the rabbit hole down kernfs and all, and it looks
like indeed the read would be cut short with -ENODEV, sorry.
It doesn't look like there's good API for this, but it seems at least
from the underlying kernfs POV it should be possible to get_device() in
open and put_device() in release, so that the device sticks around while
somebody has the file open? It's entirely virtual, so this should be OK?
johannes
Are you suggesting something like below?
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 42dcf96..14203d0 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -32,6 +32,22 @@ static const struct sysfs_ops *sysfs_file_ops(struct
kernfs_node *kn)
return kobj->ktype ? kobj->ktype->sysfs_ops : NULL;
}
+static int sysfs_kf_open(struct kernfs_open_file *of)
+{
+ struct kobject *kobj = of->kn->parent->priv;
+ struct device *dev = kobj_to_dev(kobj);
+
+ get_device(dev);
+}
+
+static void sysfs_kf_release(struct kernfs_open_file *of)
+{
+ struct kobject *kobj = of->kn->parent->priv;
+ struct device *dev = kobj_to_dev(kobj);
+
+ put_device(dev);
+}
+
/*
* Reads on sysfs are handled through seq_file, which takes care of hairy
* details like buffering and seeking. The following function pipes
@@ -211,6 +227,8 @@ static const struct kernfs_ops sysfs_file_kfops_wo = {
};
static const struct kernfs_ops sysfs_file_kfops_rw = {
+ .open = sysfs_kf_open;
+ .release= sysfs_kf_release;
.seq_show = sysfs_kf_seq_show,
.write = sysfs_kf_write,
};
If so, dont you think this will be a more intrusive change just for the
sake of devcoredump? Any other way to keep the changes limited to
devcoredump?
Thanks
Abhinav
Re: [Freedreno] [PATCH 1/3] drm/msm/dpu1: Add DMA2, DMA3 clock control to enum
On 13/01/2022 17:51, Jami Kettunen wrote:
From: AngeloGioacchino Del Regno
The enum dpu_clk_ctrl_type misses DPU_CLK_CTRL_DMA{2,3} even though
this driver does actually handle both, if present: add the two in
preparation for adding support for SoCs having them.
Signed-off-by: AngeloGioacchino Del Regno
Signed-off-by: Jami Kettunen
Reviewed-by: Dmitry Baryshkov
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 2 ++
1 file changed, 2 insertions(+)
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 31af04afda7d..736f52c742fb 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
@@ -435,6 +435,8 @@ enum dpu_clk_ctrl_type {
DPU_CLK_CTRL_RGB3,
DPU_CLK_CTRL_DMA0,
DPU_CLK_CTRL_DMA1,
+ DPU_CLK_CTRL_DMA2,
+ DPU_CLK_CTRL_DMA3,
DPU_CLK_CTRL_CURSOR0,
DPU_CLK_CTRL_CURSOR1,
DPU_CLK_CTRL_INLINE_ROT0_SSPP,
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v2 2/4] arm64: dts: qcom: sc7280: Add support for eDP panel on CRD
On Tue 08 Feb 07:18 PST 2022, Sankeerth Billakanti wrote:
> Enable the eDP display panel support without HPD on sc7280 platform.
>
> Signed-off-by: Sankeerth Billakanti
> ---
>
> Changes in v2:
> - sort node references alphabetically
> - improve readability
> - move the pwm pinctrl to pwm node
> - move the regulators to root
> - define backlight power
> - remove dummy regulator node
> - cleanup pinctrl definitions
>
> arch/arm64/boot/dts/qcom/sc7280-crd.dts | 122
>
> arch/arm64/boot/dts/qcom/sc7280.dtsi| 2 -
> 2 files changed, 122 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index e2efbdd..bff2707 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -21,6 +21,34 @@
> chosen {
> stdout-path = "serial0:115200n8";
> };
> +
> + backlight_power: backlight-power {
> + compatible = "regulator-fixed";
> + regulator-name = "backlight_power";
> +
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> +
> + gpio = <_gpios 7 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_bl_power>;
> + };
> +
> + edp_power: edp-power {
> + compatible = "regulator-fixed";
> + regulator-name = "edp_power";
> +
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> +
> + gpio = < 80 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_panel_power>;
> + };
> };
>
> _rsc {
> @@ -76,6 +104,42 @@ ap_ts_pen_1v8: {
> };
> };
>
> +_out {
Sorry for missing this while merging changes in sc7280.dtsi. But it
would be really nice if this was labeled mdss_edp_out instead (or
possibly defined within the _edp node).
Now you will have _out and _out floating around away from the edp
and dp nodes...
> + remote-endpoint = <_panel_in>;
> +};
> +
> + {
> + status = "okay";
> +};
> +
> +_edp {
> + status = "okay";
> +
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l10c_0p8>;
> +};
> +
> +_edp_phy {
> + status = "okay";
> +
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l10c_0p8>;
> +};
> +
> +_dp {
> + status = "okay";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_hot_plug_det>;
> + data-lanes = <0 1>;
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l1b_0p8>;
> +};
> +
> +_mdp {
> + status = "okay";
> +};
> +
> _3v3_regulator {
> gpio = < 51 GPIO_ACTIVE_HIGH>;
> };
> @@ -84,7 +148,65 @@ ap_ts_pen_1v8: {
> pins = "gpio51";
> };
>
> +_pwm {
This label doesn't exist, so I won't be able to merge this patch.
> + status = "okay";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_bl_pwm>;
> +};
> +
> +_gpios {
> + edp_bl_power: edp-bl-power {
> + pins = "gpio7";
> + function = "normal";
> + qcom,drive-strength = ;
> + bias-disable;
> + output-low;
> + };
> +
> + edp_bl_pwm: edp-bl-pwm {
> + pins = "gpio8";
> + function = "func1";
> + qcom,drive-strength = ;
> + bias-disable;
> + output-low;
> + };
> +};
> +
> + {
> + edp_backlight: edp-backlight {
> + compatible = "pwm-backlight";
This is not a device on the mmio bus, so it should not love within the
> +
> + power-supply = <_power>;
> + pwms = <_pwm 3 65535>;
> + };
> +
> + edp_panel: edp_panel {
Ditto.
Regards,
Bjorn
> + compatible = "sharp,lq140m1jw46";
> +
> + power-supply = <_power>;
> + backlight = <_backlight>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + edp_panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> + };
> +};
> +
> {
> + edp_panel_power: edp-panel-power {
> + pins = "gpio80";
> + function = "gpio";
> + bias-pull-down;
> + };
> +
> tp_int_odl: tp-int-odl {
> pins = "gpio7";
> function = "gpio";
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 3572399..f8fa716 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3012,8 +3012,6 @@
>
> mdss_edp:
Re: [Freedreno] [PATCH] devcoredump: increase the device delete timeout to 10 mins
On Tue, 2022-02-08 at 13:04 -0800, Abhinav Kumar wrote: > > It opened the file rightaway but could not finish reading. > > The device gets deleted so the corresponding /data will disappear too ( > as the data node is under devcd*/data) Yeah but even if the file disappears, the open file descriptor is still there, no? Does sysfs somehow make those disappear? I know debugfs does (now, to some extent, it didn't always), but I thought sysfs was refcounting things and didn't do that? What did the userspace actually see? read() returned 0 so EOF? (I guess I could test it, but it's getting late) Your other questions are related - you need to consider the file in sysfs and the open file descriptor separately. johannes
Re: [Freedreno] [PATCH] devcoredump: increase the device delete timeout to 10 mins
On Tue, 2022-02-08 at 11:44 -0800, Abhinav Kumar wrote: > There are cases where depending on the size of the devcoredump and the speed > at which the usermode reads the dump, it can take longer than the current 5 > mins > timeout. > > This can lead to incomplete dumps as the device is deleted once the timeout > expires. > > One example is below where it took 6 mins for the devcoredump to be > completely read. > > 04:22:24.668 23916 23994 I HWDeviceDRM::DumpDebugData: Opening > /sys/class/devcoredump/devcd6/data > 04:28:35.377 23916 23994 W HWDeviceDRM::DumpDebugData: Freeing devcoredump > node > > Increase the timeout to 10 mins to accommodate system delays and large > coredump > sizes. > No real objection, I guess, but can the data actually disappear *while* the sysfs file is open?! Or did it take 5 minutes to open the file? If the former, maybe we should fix that too (or instead)? johannes
Re: [Freedreno] [PATCH] devcoredump: increase the device delete timeout to 10 mins
On Tue, 2022-02-08 at 13:40 -0800, Abhinav Kumar wrote: > > > I am checking what usermode sees and will get back ( I didnt see an > error do most likely it was EOF ). I didnt follow the second part. I think probably it got -ENODEV, looking at kernfs_file_read_iter(). > If the file descriptor read returns EOF, even if we consider them > separate how will it resolve this issue? > > My earlier questions were related to fixing it in devcoredump to detect > and fix it there. Are you suggesting to fix in usermode instead? How? > Yeah, no, you cannot fix it in userspace. But I just followed the rabbit hole down kernfs and all, and it looks like indeed the read would be cut short with -ENODEV, sorry. It doesn't look like there's good API for this, but it seems at least from the underlying kernfs POV it should be possible to get_device() in open and put_device() in release, so that the device sticks around while somebody has the file open? It's entirely virtual, so this should be OK? johannes
Re: [Freedreno] [PATCH] drm/msm/dp: Add DisplayPort controller for SM8350
On Wed, 9 Feb 2022 at 00:21, Bjorn Andersson wrote:
>
> On Wed 19 Jan 15:14 PST 2022, Dmitry Baryshkov wrote:
>
> > On 28/12/2021 07:59, Bjorn Andersson wrote:
> > > The Qualcomm SM8350 platform comes with a single DisplayPort controller,
> > > add support for this in the DisplayPort driver.
> > >
> > > Signed-off-by: Bjorn Andersson
> >
> > Reviewed-by: Dmitry Baryshkov
> >
>
> I don't see this in linux-next, would it be possible to pick it up now
> that we're past the merge window etc?
I'll work on my staging tree (and send it to Rob) before the EoW.
>
> Regards,
> Bjorn
>
> > > ---
> > > .../devicetree/bindings/display/msm/dp-controller.yaml| 1 +
> > > drivers/gpu/drm/msm/dp/dp_display.c | 8
> > > 2 files changed, 9 insertions(+)
> > >
> > > diff --git
> > > a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > > index 5457612ab136..cd05cfd76536 100644
> > > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > > @@ -21,6 +21,7 @@ properties:
> > > - qcom,sc7280-edp
> > > - qcom,sc8180x-dp
> > > - qcom,sc8180x-edp
> > > + - qcom,sm8350-dp
> > > reg:
> > > items:
> > > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> > > b/drivers/gpu/drm/msm/dp/dp_display.c
> > > index 8d9c19dbf33e..fd0fd03f8fed 100644
> > > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > > @@ -143,10 +143,18 @@ static const struct msm_dp_config sc7280_dp_cfg = {
> > > .num_descs = 2,
> > > };
> > > +static const struct msm_dp_config sm8350_dp_cfg = {
> > > + .descs = (const struct msm_dp_desc[]) {
> > > + [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae9,
> > > .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
> > > + },
> > > + .num_descs = 1,
> > > +};
> > > +
> > > static const struct of_device_id dp_dt_match[] = {
> > > { .compatible = "qcom,sc7180-dp", .data = _dp_cfg },
> > > { .compatible = "qcom,sc7280-dp", .data = _dp_cfg },
> > > { .compatible = "qcom,sc7280-edp", .data = _dp_cfg },
> > > + { .compatible = "qcom,sm8350-dp", .data = _dp_cfg },
> > > {}
> > > };
> >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
Re: [Freedreno] [PATCH v2 2/4] arm64: dts: qcom: sc7280: Add support for eDP panel on CRD
On Tue, Feb 08, 2022 at 08:48:43PM +0530, Sankeerth Billakanti wrote:
> Enable the eDP display panel support without HPD on sc7280 platform.
>
> Signed-off-by: Sankeerth Billakanti
> ---
>
> Changes in v2:
> - sort node references alphabetically
> - improve readability
> - move the pwm pinctrl to pwm node
> - move the regulators to root
> - define backlight power
> - remove dummy regulator node
> - cleanup pinctrl definitions
>
> arch/arm64/boot/dts/qcom/sc7280-crd.dts | 122
>
> arch/arm64/boot/dts/qcom/sc7280.dtsi| 2 -
> 2 files changed, 122 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> index e2efbdd..bff2707 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
> @@ -21,6 +21,34 @@
> chosen {
> stdout-path = "serial0:115200n8";
> };
> +
> + backlight_power: backlight-power {
nit: the other fixed regulator in sc7280-idp.dtsi is called
'nvme_3v3_regulator', if you wanted to be consistent you
could call this backlight_3v3_regulator.
> + compatible = "regulator-fixed";
> + regulator-name = "backlight_power";
> +
> + regulator-min-microvolt = <180>;
> + regulator-max-microvolt = <180>;
> +
> + gpio = <_gpios 7 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_bl_power>;
> + };
> +
> + edp_power: edp-power {
nit: see above
> + compatible = "regulator-fixed";
> + regulator-name = "edp_power";
> +
> + regulator-min-microvolt = <330>;
> + regulator-max-microvolt = <330>;
> +
> + gpio = < 80 GPIO_ACTIVE_HIGH>;
> + enable-active-high;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_panel_power>;
> + };
> };
>
> _rsc {
> @@ -76,6 +104,42 @@ ap_ts_pen_1v8: {
> };
> };
>
> +_out {
> + remote-endpoint = <_panel_in>;
> +};
> +
> + {
> + status = "okay";
> +};
> +
> +_edp {
> + status = "okay";
> +
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l10c_0p8>;
> +};
> +
> +_edp_phy {
> + status = "okay";
> +
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l10c_0p8>;
> +};
> +
> +_dp {
should be before 'mdss_edp'.
> + status = "okay";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_hot_plug_det>;
> + data-lanes = <0 1>;
> + vdda-1p2-supply = <_l6b_1p2>;
> + vdda-0p9-supply = <_l1b_0p8>;
> +};
> +
> +_mdp {
> + status = "okay";
> +};
> +
> _3v3_regulator {
> gpio = < 51 GPIO_ACTIVE_HIGH>;
> };
> @@ -84,7 +148,65 @@ ap_ts_pen_1v8: {
> pins = "gpio51";
> };
>
> +_pwm {
> + status = "okay";
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <_bl_pwm>;
> +};
> +
> +_gpios {
should be before 'pm8350c_pwm'
> + edp_bl_power: edp-bl-power {
> + pins = "gpio7";
> + function = "normal";
> + qcom,drive-strength = ;
> + bias-disable;
> + output-low;
> + };
> +
> + edp_bl_pwm: edp-bl-pwm {
> + pins = "gpio8";
> + function = "func1";
> + qcom,drive-strength = ;
> + bias-disable;
> + output-low;
> + };
> +};
> +
> + {
> + edp_backlight: edp-backlight {
> + compatible = "pwm-backlight";
> +
> + power-supply = <_power>;
> + pwms = <_pwm 3 65535>;
> + };
> +
> + edp_panel: edp_panel {
in difference to labels node names should use dashes as separator, not
underscores (i.e. 'edp-panel')
> + compatible = "sharp,lq140m1jw46";
> +
> + power-supply = <_power>;
> + backlight = <_backlight>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + edp_panel_in: endpoint {
> + remote-endpoint = <_out>;
> + };
> + };
> + };
> + };
> +};
> +
> {
> + edp_panel_power: edp-panel-power {
> + pins = "gpio80";
> + function = "gpio";
> + bias-pull-down;
> + };
> +
> tp_int_odl: tp-int-odl {
> pins = "gpio7";
> function = "gpio";
> diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> index 3572399..f8fa716 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
> @@ -3012,8 +3012,6 @@
>
> mdss_edp: edp@aea {
> compatible = "qcom,sc7280-edp";
Re: [Freedreno] [PATCH] devcoredump: increase the device delete timeout to 10 mins
Hi Johannes On 2/8/2022 1:12 PM, Johannes Berg wrote: On Tue, 2022-02-08 at 13:04 -0800, Abhinav Kumar wrote: It opened the file rightaway but could not finish reading. The device gets deleted so the corresponding /data will disappear too ( as the data node is under devcd*/data) Yeah but even if the file disappears, the open file descriptor is still there, no? Does sysfs somehow make those disappear? I know debugfs does (now, to some extent, it didn't always), but I thought sysfs was refcounting things and didn't do that? What did the userspace actually see? read() returned 0 so EOF? (I guess I could test it, but it's getting late) Your other questions are related - you need to consider the file in sysfs and the open file descriptor separately. johannes I am checking what usermode sees and will get back ( I didnt see an error do most likely it was EOF ). I didnt follow the second part. If the file descriptor read returns EOF, even if we consider them separate how will it resolve this issue? My earlier questions were related to fixing it in devcoredump to detect and fix it there. Are you suggesting to fix in usermode instead? How? Thanks Abhinav
Re: [Freedreno] [PATCH] drm/msm/dp: Add DisplayPort controller for SM8350
On Wed 19 Jan 15:14 PST 2022, Dmitry Baryshkov wrote:
> On 28/12/2021 07:59, Bjorn Andersson wrote:
> > The Qualcomm SM8350 platform comes with a single DisplayPort controller,
> > add support for this in the DisplayPort driver.
> >
> > Signed-off-by: Bjorn Andersson
>
> Reviewed-by: Dmitry Baryshkov
>
I don't see this in linux-next, would it be possible to pick it up now
that we're past the merge window etc?
Regards,
Bjorn
> > ---
> > .../devicetree/bindings/display/msm/dp-controller.yaml| 1 +
> > drivers/gpu/drm/msm/dp/dp_display.c | 8
> > 2 files changed, 9 insertions(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > index 5457612ab136..cd05cfd76536 100644
> > --- a/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > +++ b/Documentation/devicetree/bindings/display/msm/dp-controller.yaml
> > @@ -21,6 +21,7 @@ properties:
> > - qcom,sc7280-edp
> > - qcom,sc8180x-dp
> > - qcom,sc8180x-edp
> > + - qcom,sm8350-dp
> > reg:
> > items:
> > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> > b/drivers/gpu/drm/msm/dp/dp_display.c
> > index 8d9c19dbf33e..fd0fd03f8fed 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_display.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> > @@ -143,10 +143,18 @@ static const struct msm_dp_config sc7280_dp_cfg = {
> > .num_descs = 2,
> > };
> > +static const struct msm_dp_config sm8350_dp_cfg = {
> > + .descs = (const struct msm_dp_desc[]) {
> > + [MSM_DP_CONTROLLER_0] = { .io_start = 0x0ae9,
> > .connector_type = DRM_MODE_CONNECTOR_DisplayPort },
> > + },
> > + .num_descs = 1,
> > +};
> > +
> > static const struct of_device_id dp_dt_match[] = {
> > { .compatible = "qcom,sc7180-dp", .data = _dp_cfg },
> > { .compatible = "qcom,sc7280-dp", .data = _dp_cfg },
> > { .compatible = "qcom,sc7280-edp", .data = _dp_cfg },
> > + { .compatible = "qcom,sm8350-dp", .data = _dp_cfg },
> > {}
> > };
>
>
> --
> With best wishes
> Dmitry
Re: [Freedreno] [PATCH] devcoredump: increase the device delete timeout to 10 mins
Hi Johannes
Thanks for the response.
On 2/8/2022 12:35 PM, Johannes Berg wrote:
On Tue, 2022-02-08 at 11:44 -0800, Abhinav Kumar wrote:
There are cases where depending on the size of the devcoredump and the speed
at which the usermode reads the dump, it can take longer than the current 5 mins
timeout.
This can lead to incomplete dumps as the device is deleted once the timeout
expires.
One example is below where it took 6 mins for the devcoredump to be completely
read.
04:22:24.668 23916 23994 I HWDeviceDRM::DumpDebugData: Opening
/sys/class/devcoredump/devcd6/data
04:28:35.377 23916 23994 W HWDeviceDRM::DumpDebugData: Freeing devcoredump node
Increase the timeout to 10 mins to accommodate system delays and large coredump
sizes.
No real objection, I guess, but can the data actually disappear *while*
the sysfs file is open?!
Or did it take 5 minutes to open the file?
If the former, maybe we should fix that too (or instead)?
johannes
It opened the file rightaway but could not finish reading.
The device gets deleted so the corresponding /data will disappear too (
as the data node is under devcd*/data)
60 static void devcd_del(struct work_struct *wk)
61 {
62 struct devcd_entry *devcd;
63
64 devcd = container_of(wk, struct devcd_entry, del_wk.work);
65
66 device_del(>devcd_dev);
67 put_device(>devcd_dev);
68 }
Are you suggesting we implement a logic like :
a) if the usermode has started reading the data but has not finished yet
( we can detect the former with something like devcd->data_read_ongoing
= 1 and we know it has finished when it acks and we can clear this flag
then), in the timeout del_wk then we can delay the the delete timer by
another TIMEOUT amount of time to give usermode time to finish the data?
b) If usermode acks, we will clear both the flag and delete the device
as usual
But there is a corner case here:
c) If usermode starts the read, but then for some reason crashes, the
timer will timeout and try to delete the device but will detect that
usermode is still reading and will keep the device. How do we detect
this case?
Thats why i thought maybe the easier way right now is to try increasing
the timeout.
[Freedreno] [PATCH] devcoredump: increase the device delete timeout to 10 mins
There are cases where depending on the size of the devcoredump and the speed
at which the usermode reads the dump, it can take longer than the current 5 mins
timeout.
This can lead to incomplete dumps as the device is deleted once the timeout
expires.
One example is below where it took 6 mins for the devcoredump to be completely
read.
04:22:24.668 23916 23994 I HWDeviceDRM::DumpDebugData: Opening
/sys/class/devcoredump/devcd6/data
04:28:35.377 23916 23994 W HWDeviceDRM::DumpDebugData: Freeing devcoredump node
Increase the timeout to 10 mins to accommodate system delays and large coredump
sizes.
Signed-off-by: Abhinav Kumar
---
drivers/base/devcoredump.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
index f4d794d..6b83ae5 100644
--- a/drivers/base/devcoredump.c
+++ b/drivers/base/devcoredump.c
@@ -18,8 +18,8 @@ static struct class devcd_class;
/* global disable flag, for security purposes */
static bool devcd_disabled;
-/* if data isn't read by userspace after 5 minutes then delete it */
-#define DEVCD_TIMEOUT (HZ * 60 * 5)
+/* if data isn't read by userspace after 10 minutes then delete it */
+#define DEVCD_TIMEOUT (HZ * 60 * 10)
struct devcd_entry {
struct device devcd_dev;
--
2.7.4
[Freedreno] [PATCH v2 4/4] drm/msm/dp: Add driver support to utilize drm panel
Add support in the DP driver to utilize the custom eDP panels
from drm/panels.
An eDP panel is always connected to the platform. So, the eDP
connector can be reported as always connected. The display mode
will be sourced from the panel. The panel mode will be set after
the link training is completed.
The eDP driver needs to register for IRQ_HPD only.
This support will be added later.
Signed-off-by: Sankeerth Billakanti
---
drivers/gpu/drm/msm/dp/dp_display.c | 8 ++
drivers/gpu/drm/msm/dp/dp_drm.c | 54 +
drivers/gpu/drm/msm/dp/dp_parser.h | 3 +++
3 files changed, 60 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 7cc4d21..410fda4 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -1513,6 +1513,10 @@ int msm_dp_display_enable(struct msm_dp *dp, struct
drm_encoder *encoder)
return -EINVAL;
}
+ /* handle eDP on */
+ if (dp->connector_type == DRM_MODE_CONNECTOR_eDP)
+ dp_hpd_plug_handle(dp_display, 0);
+
mutex_lock(_display->event_mutex);
/* stop sentinel checking */
@@ -1577,6 +1581,10 @@ int msm_dp_display_disable(struct msm_dp *dp, struct
drm_encoder *encoder)
dp_display = container_of(dp, struct dp_display_private, dp_display);
+ /* handle edp off */
+ if (dp->connector_type == DRM_MODE_CONNECTOR_eDP)
+ dp_hpd_unplug_handle(dp_display, 0);
+
mutex_lock(_display->event_mutex);
/* stop sentinel checking */
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index d4d360d..12fa8c1 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -39,6 +39,10 @@ static enum drm_connector_status dp_connector_detect(struct
drm_connector *conn,
dp = to_dp_connector(conn)->dp_display;
+ /* eDP is always connected */
+ if (dp->connector_type == DRM_MODE_CONNECTOR_eDP)
+ return connector_status_connected;
+
DRM_DEBUG_DP("is_connected = %s\n",
(dp->is_connected) ? "true" : "false");
@@ -123,6 +127,35 @@ static enum drm_mode_status dp_connector_mode_valid(
return dp_display_validate_mode(dp_disp, mode->clock);
}
+static int edp_connector_get_modes(struct drm_connector *connector)
+{
+ struct msm_dp *dp;
+
+ if (!connector)
+ return 0;
+
+ dp = to_dp_connector(connector)->dp_display;
+
+ return drm_bridge_get_modes(dp->panel_bridge, connector);
+}
+
+static enum drm_mode_status edp_connector_mode_valid(
+ struct drm_connector *connector,
+ struct drm_display_mode *mode)
+{
+ struct msm_dp *dp;
+
+ if (!connector)
+ return 0;
+
+ dp = to_dp_connector(connector)->dp_display;
+
+ if (mode->clock > EDP_MAX_PIXEL_CLK_KHZ)
+ return MODE_BAD;
+
+ return MODE_OK;
+}
+
static const struct drm_connector_funcs dp_connector_funcs = {
.detect = dp_connector_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
@@ -137,6 +170,12 @@ static const struct drm_connector_helper_funcs
dp_connector_helper_funcs = {
.mode_valid = dp_connector_mode_valid,
};
+static const struct drm_connector_helper_funcs edp_connector_helper_funcs = {
+ .get_modes = edp_connector_get_modes,
+ .mode_valid = edp_connector_mode_valid,
+
+};
+
/* connector initialization */
struct drm_connector *dp_drm_connector_init(struct msm_dp *dp_display)
{
@@ -160,12 +199,17 @@ struct drm_connector *dp_drm_connector_init(struct msm_dp
*dp_display)
if (ret)
return ERR_PTR(ret);
- drm_connector_helper_add(connector, _connector_helper_funcs);
+ if (dp_display->connector_type == DRM_MODE_CONNECTOR_eDP) {
+ drm_connector_helper_add(connector,
+ _connector_helper_funcs);
+ } else {
+ drm_connector_helper_add(connector, _connector_helper_funcs);
- /*
-* Enable HPD to let hpd event is handled when cable is connected.
-*/
- connector->polled = DRM_CONNECTOR_POLL_HPD;
+ /*
+* Enable HPD to let hpd event is handled when cable is
connected.
+*/
+ connector->polled = DRM_CONNECTOR_POLL_HPD;
+ }
drm_connector_attach_encoder(connector, dp_display->encoder);
diff --git a/drivers/gpu/drm/msm/dp/dp_parser.h
b/drivers/gpu/drm/msm/dp/dp_parser.h
index 3172da0..58c4f27 100644
--- a/drivers/gpu/drm/msm/dp/dp_parser.h
+++ b/drivers/gpu/drm/msm/dp/dp_parser.h
@@ -17,6 +17,9 @@
#define DP_MAX_PIXEL_CLK_KHZ 675000
#define DP_MAX_NUM_DP_LANES4
+/* Maximum validated clock */
+#define EDP_MAX_PIXEL_CLK_KHZ 285550
+
enum dp_pm_type {
DP_CORE_PM,
DP_CTRL_PM,
--
2.7.4
[Freedreno] [PATCH v2 3/4] drm/panel-edp: Add eDP sharp panel support
Add support for the 14" sharp,lq140m1jw46 eDP panel.
Signed-off-by: Sankeerth Billakanti
---
Changes in v2:
- add mode when not using hpd
- add delays
- put dt-bindings
drivers/gpu/drm/panel/panel-edp.c | 31 +++
1 file changed, 31 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index a394a15..5d13ccc 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -1605,6 +1605,34 @@ static const struct panel_desc sharp_lq123p1jx31 = {
},
};
+static const struct drm_display_mode sharp_lq140m1jw46_mode = {
+ .clock = 144370,
+ .hdisplay = 1920,
+ .hsync_start = 1920 + 48,
+ .hsync_end = 1920 + 48 + 32,
+ .htotal = 1920 + 48 + 32 + 80,
+ .vdisplay = 1080,
+ .vsync_start = 1080 + 3,
+ .vsync_end = 1080 + 3 + 5,
+ .vtotal = 1080 + 3 + 5 + 69,
+ .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static const struct panel_desc sharp_lq140m1jw46 = {
+ .modes = _lq140m1jw46_mode,
+ .num_modes = 1,
+ .bpc = 8,
+ .size = {
+ .width = 309,
+ .height = 174,
+ },
+ .delay = {
+ .hpd_absent = 80,
+ .enable = 50,
+ .unprepare = 500,
+ },
+};
+
static const struct drm_display_mode starry_kr122ea0sra_mode = {
.clock = 147000,
.hdisplay = 1920,
@@ -1719,6 +1747,9 @@ static const struct of_device_id platform_of_match[] = {
.compatible = "sharp,lq123p1jx31",
.data = _lq123p1jx31,
}, {
+ .compatible = "sharp,lq140m1jw46",
+ .data = _lq140m1jw46,
+ }, {
.compatible = "starry,kr122ea0sra",
.data = _kr122ea0sra,
}, {
--
2.7.4
[Freedreno] [PATCH v2 2/4] arm64: dts: qcom: sc7280: Add support for eDP panel on CRD
Enable the eDP display panel support without HPD on sc7280 platform.
Signed-off-by: Sankeerth Billakanti
---
Changes in v2:
- sort node references alphabetically
- improve readability
- move the pwm pinctrl to pwm node
- move the regulators to root
- define backlight power
- remove dummy regulator node
- cleanup pinctrl definitions
arch/arm64/boot/dts/qcom/sc7280-crd.dts | 122
arch/arm64/boot/dts/qcom/sc7280.dtsi| 2 -
2 files changed, 122 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
index e2efbdd..bff2707 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-crd.dts
@@ -21,6 +21,34 @@
chosen {
stdout-path = "serial0:115200n8";
};
+
+ backlight_power: backlight-power {
+ compatible = "regulator-fixed";
+ regulator-name = "backlight_power";
+
+ regulator-min-microvolt = <180>;
+ regulator-max-microvolt = <180>;
+
+ gpio = <_gpios 7 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <_bl_power>;
+ };
+
+ edp_power: edp-power {
+ compatible = "regulator-fixed";
+ regulator-name = "edp_power";
+
+ regulator-min-microvolt = <330>;
+ regulator-max-microvolt = <330>;
+
+ gpio = < 80 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+
+ pinctrl-names = "default";
+ pinctrl-0 = <_panel_power>;
+ };
};
_rsc {
@@ -76,6 +104,42 @@ ap_ts_pen_1v8: {
};
};
+_out {
+ remote-endpoint = <_panel_in>;
+};
+
+ {
+ status = "okay";
+};
+
+_edp {
+ status = "okay";
+
+ vdda-1p2-supply = <_l6b_1p2>;
+ vdda-0p9-supply = <_l10c_0p8>;
+};
+
+_edp_phy {
+ status = "okay";
+
+ vdda-1p2-supply = <_l6b_1p2>;
+ vdda-0p9-supply = <_l10c_0p8>;
+};
+
+_dp {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <_hot_plug_det>;
+ data-lanes = <0 1>;
+ vdda-1p2-supply = <_l6b_1p2>;
+ vdda-0p9-supply = <_l1b_0p8>;
+};
+
+_mdp {
+ status = "okay";
+};
+
_3v3_regulator {
gpio = < 51 GPIO_ACTIVE_HIGH>;
};
@@ -84,7 +148,65 @@ ap_ts_pen_1v8: {
pins = "gpio51";
};
+_pwm {
+ status = "okay";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <_bl_pwm>;
+};
+
+_gpios {
+ edp_bl_power: edp-bl-power {
+ pins = "gpio7";
+ function = "normal";
+ qcom,drive-strength = ;
+ bias-disable;
+ output-low;
+ };
+
+ edp_bl_pwm: edp-bl-pwm {
+ pins = "gpio8";
+ function = "func1";
+ qcom,drive-strength = ;
+ bias-disable;
+ output-low;
+ };
+};
+
+ {
+ edp_backlight: edp-backlight {
+ compatible = "pwm-backlight";
+
+ power-supply = <_power>;
+ pwms = <_pwm 3 65535>;
+ };
+
+ edp_panel: edp_panel {
+ compatible = "sharp,lq140m1jw46";
+
+ power-supply = <_power>;
+ backlight = <_backlight>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ port@0 {
+ reg = <0>;
+ edp_panel_in: endpoint {
+ remote-endpoint = <_out>;
+ };
+ };
+ };
+ };
+};
+
{
+ edp_panel_power: edp-panel-power {
+ pins = "gpio80";
+ function = "gpio";
+ bias-pull-down;
+ };
+
tp_int_odl: tp-int-odl {
pins = "gpio7";
function = "gpio";
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 3572399..f8fa716 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -3012,8 +3012,6 @@
mdss_edp: edp@aea {
compatible = "qcom,sc7280-edp";
- pinctrl-names = "default";
- pinctrl-0 = <_hot_plug_det>;
reg = <0 0xaea 0 0x200>,
<0 0xaea0200 0 0x200>,
--
2.7.4
[Freedreno] [PATCH v2 1/4] dt-bindings: display: simple: Add sharp LQ140M1JW46 panel
Add support for sharp LQ140M1JW46 display panel. It is a 14" eDP panel with 1920x1080 display resolution. Signed-off-by: Sankeerth Billakanti --- Documentation/devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 9cf5588..1eb9dd4 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -284,6 +284,8 @@ properties: - sharp,lq101k1ly04 # Sharp 12.3" (2400x1600 pixels) TFT LCD panel - sharp,lq123p1jx31 +# Sharp 14" (1920x1080 pixels) TFT LCD panel + - sharp,lq140m1jw46 # Sharp LS020B1DD01D 2.0" HQVGA TFT LCD panel - sharp,ls020b1dd01d # Shelly SCA07010-BFN-LNN 7.0" WVGA TFT LCD panel -- 2.7.4
[Freedreno] [PATCH v2 0/4] Add support for the eDP panel on sc7280 CRD
Add support for the eDP panel on sc7280 CRD platform. The eDP panel does not need HPD line for connect disconnect. So, this series will report eDP as always connected. The driver needs to register for IRQ_HPD only for eDP. This support will be added later. These changes are dependent on the following series: https://patchwork.kernel.org/project/linux-arm-msm/list/?series=586263=both=* https://patchwork.kernel.org/project/linux-arm-msm/list/?series=560587=%2A=both Sankeerth Billakanti (4): dt-bindings: display: simple: Add sharp LQ140M1JW46 panel arm64: dts: qcom: sc7280: Add support for eDP panel on CRD drm/panel-edp: Add eDP sharp panel support drm/msm/dp: Add driver support to utilize drm panel .../bindings/display/panel/panel-simple.yaml | 2 + arch/arm64/boot/dts/qcom/sc7280-crd.dts| 122 + arch/arm64/boot/dts/qcom/sc7280.dtsi | 2 - drivers/gpu/drm/msm/dp/dp_display.c| 8 ++ drivers/gpu/drm/msm/dp/dp_drm.c| 54 - drivers/gpu/drm/msm/dp/dp_parser.h | 3 + drivers/gpu/drm/panel/panel-edp.c | 31 ++ 7 files changed, 215 insertions(+), 7 deletions(-) -- 2.7.4
Re: [Freedreno] [PATCH v6 02/35] component: Introduce the aggregate bus_type
On Mon, Jan 31, 2022 at 05:34:26PM +0100, Greg Kroah-Hartman wrote: > On Mon, Jan 31, 2022 at 04:15:09PM +0100, Daniel Vetter wrote: > > On Mon, Jan 31, 2022 at 2:48 PM Greg Kroah-Hartman > > wrote: > > > > > > On Thu, Jan 27, 2022 at 12:01:08PM -0800, Stephen Boyd wrote: > > > > The component framework only provides 'bind' and 'unbind' callbacks to > > > > tell the host driver that it is time to assemble the aggregate driver > > > > now that all the components have probed. The component framework doesn't > > > > attempt to resolve runtime PM or suspend/resume ordering, and explicitly > > > > mentions this in the code. This lack of support leads to some pretty > > > > gnarly usages of the 'prepare' and 'complete' power management hooks in > > > > drivers that host the aggregate device, and it fully breaks down when > > > > faced with ordering shutdown between the various components, the > > > > aggregate driver, and the host driver that registers the whole thing. > > > > > > > > In a concrete example, the MSM display driver at drivers/gpu/drm/msm is > > > > using 'prepare' and 'complete' to call the drm helpers > > > > drm_mode_config_helper_suspend() and drm_mode_config_helper_resume() > > > > respectively, so that it can move the aggregate driver suspend/resume > > > > callbacks to be before and after the components that make up the drm > > > > device call any suspend/resume hooks they have. This only works as long > > > > as the component devices don't do anything in their own 'prepare' and > > > > 'complete' callbacks. If they did, then the ordering would be incorrect > > > > and we would be doing something in the component drivers before the > > > > aggregate driver could do anything. Yuck! > > > > > > > > Similarly, when trying to add shutdown support to the MSM driver we run > > > > across a problem where we're trying to shutdown the drm device via > > > > drm_atomic_helper_shutdown(), but some of the devices in the encoder > > > > chain have already been shutdown. This time, the component devices > > > > aren't the problem (although they could be if they did anything in their > > > > shutdown callbacks), but there's a DSI to eDP bridge in the encoder > > > > chain that has already been shutdown before the driver hosting the > > > > aggregate device runs shutdown. The ordering of driver probe is like > > > > this: > > > > > > > > 1. msm_pdev_probe() (host driver) > > > > 2. DSI bridge > > > > 3. aggregate bind > > > > > > > > When it comes to shutdown we have this order: > > > > > > > > 1. DSI bridge > > > > 2. msm_pdev_shutdown() (host driver) > > > > > > > > and so the bridge is already off, but we want to communicate to it to > > > > turn things off on the display during msm_pdev_shutdown(). Double yuck! > > > > Unfortunately, this time we can't split shutdown into multiple phases > > > > and swap msm_pdev_shutdown() with the DSI bridge. > > > > > > > > Let's make the component_master_ops into an actual device driver that > > > > has > > > > probe/remove/shutdown functions. The driver will only be bound to the > > > > aggregate device once all component drivers have called component_add() > > > > to indicate they're ready to assemble the aggregate driver. This allows > > > > us to attach shutdown logic (and in the future runtime PM logic) to the > > > > aggregate driver so that it runs the hooks in the correct order. > > > > > > I know I asked before, but I can not remember the answer. > > > > > > This really looks like it is turning into the aux bus code. Why can't > > > you just use that instead here for this type of thing? You are creating > > > another bus and drivers for that bus that are "fake" which is great, but > > > that's what the aux bus code was supposed to help out with, so we > > > wouldn't have to write more of these. > > > > > > So, if this really is different, can you document it here so I remember > > > next time you resend this patch series? > > > > aux takes a device and splits it into a lot of sub-devices, each with > > their own driver. > > > > This takes a pile of devices, and turns it into a single logical > > device with a single driver. > > > > So aux is 1:N, component is N:1. > > > > And yes you asked this already, I typed this up already :-) > > Ok, thanks. But then why is a bus needed if there's a single driver? > I guess a bus for that driver? So one bus, one driver, and one device? Maybe? I have honestly no idea how this should be best modelled in the linux device model. > I think we need better documentation here... https://dri.freedesktop.org/docs/drm/driver-api/component.html?highlight=component_del#component-helper-for-aggregate-drivers There's a kerneldoc overview for component, but it's for driver authors that want to use component to glue different hw pieces into a logical driver, so it skips over these internals. And I'm honestly not sure how we want to leak implementation internals like the bus/driver/device structure ot users of
Re: [Freedreno] [PATCH 1/2] drm: Add HPD state to drm_connector_oob_hotplug_event()
On Mon, Feb 07, 2022 at 08:43:27PM -0800, Bjorn Andersson wrote:
> In some implementations, such as the Qualcomm platforms, the display
> driver has no way to query the current HPD state and as such it's
> impossible to distinguish between disconnect and attention events.
>
> Add a parameter to drm_connector_oob_hotplug_event() to pass the HPD
> state.
>
> Also push the test for unchanged state in the displayport altmode driver
> into the i915 driver, to allow other drivers to act upon each update.
>
> Signed-off-by: Bjorn Andersson
> ---
>
> Note that the Intel driver has only been compile tested with this patch.
>
> drivers/gpu/drm/drm_connector.c | 6 --
> drivers/gpu/drm/i915/display/intel_dp.c | 14 +++---
> drivers/gpu/drm/i915/i915_drv.h | 3 +++
> drivers/usb/typec/altmodes/displayport.c | 9 ++---
> include/drm/drm_connector.h | 5 +++--
> 5 files changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index a50c82bc2b2f..ad7295597c0f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2825,6 +2825,7 @@ struct drm_connector
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
> /**
> * drm_connector_oob_hotplug_event - Report out-of-band hotplug event to
> connector
> * @connector_fwnode: fwnode_handle to report the event on
> + * @hpd_state: number of data lanes available
"number"?
> *
> * On some hardware a hotplug event notification may come from outside the
> display
> * driver / device. An example of this is some USB Type-C setups where the
> hardware
> @@ -2834,7 +2835,8 @@ struct drm_connector
> *drm_connector_find_by_fwnode(struct fwnode_handle *fwnode)
> * This function can be used to report these out-of-band events after
> obtaining
> * a drm_connector reference through calling drm_connector_find_by_fwnode().
> */
> -void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode)
> +void drm_connector_oob_hotplug_event(struct fwnode_handle *connector_fwnode,
> + bool hpd_state)
This is a boolean, how can it be a number?
And having a "flag" like this is a pain, how do you know what the
parameter really means?
> {
> struct drm_connector *connector;
>
> @@ -2843,7 +2845,7 @@ void drm_connector_oob_hotplug_event(struct
> fwnode_handle *connector_fwnode)
> return;
>
> if (connector->funcs->oob_hotplug_event)
> - connector->funcs->oob_hotplug_event(connector);
> + connector->funcs->oob_hotplug_event(connector, hpd_state);
>
> drm_connector_put(connector);
> }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 146b83916005..00520867d37b 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -4816,15 +4816,23 @@ static int intel_dp_connector_atomic_check(struct
> drm_connector *conn,
> return intel_modeset_synced_crtcs(state, conn);
> }
>
> -static void intel_dp_oob_hotplug_event(struct drm_connector *connector)
> +static void intel_dp_oob_hotplug_event(struct drm_connector *connector, bool
> hpd_state)
> {
> struct intel_encoder *encoder =
> intel_attached_encoder(to_intel_connector(connector));
> struct drm_i915_private *i915 = to_i915(connector->dev);
> + bool need_work = false;
>
> spin_lock_irq(>irq_lock);
> - i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
> + if (hpd_state != i915->hotplug.oob_hotplug_state) {
> + i915->hotplug.event_bits |= BIT(encoder->hpd_pin);
> +
> + i915->hotplug.oob_hotplug_state = hpd_state;
> + need_work = true;
> + }
> spin_unlock_irq(>irq_lock);
> - queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
> +
> + if (need_work)
> + queue_delayed_work(system_wq, >hotplug.hotplug_work, 0);
> }
>
> static const struct drm_connector_funcs intel_dp_connector_funcs = {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8c1706fd81f9..543ebf1cfcf4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -149,6 +149,9 @@ struct i915_hotplug {
> /* Whether or not to count short HPD IRQs in HPD storms */
> u8 hpd_short_storm_enabled;
>
> + /* Last state reported by oob_hotplug_event */
> + bool oob_hotplug_state;
> +
> /*
>* if we get a HPD irq from DP and a HPD irq from non-DP
>* the non-DP HPD could block the workqueue on a mode config
> diff --git a/drivers/usb/typec/altmodes/displayport.c
> b/drivers/usb/typec/altmodes/displayport.c
> index c1d8c23baa39..a4596be4d34a 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -59,7 +59,6 @@ struct dp_altmode {
>
