[PATCH] drm: sun4i: hdmi: Fix inverted HPD result
From: Chen-Yu Tsai When the extra HPD polling in sun4i_hdmi was removed, the result of HPD was accidentally inverted. Fix this by inverting the check. Fixes: bda8eaa6dee7 ("drm: sun4i: hdmi: Remove extra HPD polling") Signed-off-by: Chen-Yu Tsai --- Sorry for the screw-up. --- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c index 557cbe5ab35f..2f2c9f0a1071 100644 --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c @@ -260,7 +260,7 @@ sun4i_hdmi_connector_detect(struct drm_connector *connector, bool force) unsigned long reg; reg = readl(hdmi->base + SUN4I_HDMI_HPD_REG); - if (reg & SUN4I_HDMI_HPD_HIGH) { + if (!(reg & SUN4I_HDMI_HPD_HIGH)) { cec_phys_addr_invalidate(hdmi->cec_adap); return connector_status_disconnected; } -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/msm: handle for EPROBE_DEFER for of_icc_get
On Fri, Jul 10, 2020 at 4:11 PM Jonathan Marek wrote: > > On 7/9/20 11:15 AM, Rob Clark wrote: > > On Thu, Jul 9, 2020 at 7:35 AM Jonathan Marek wrote: > >> > >> Check for errors instead of silently not using icc if the msm driver > >> probes before the interconnect driver. > >> > >> Allow ENODATA for ocmem path, as it is optional and this error > >> is returned when "gfx-mem" path is provided but not "ocmem". > >> > >> Remove the WARN_ON in msm_gpu_cleanup because INIT_LIST_HEAD won't have > >> been called on the list yet when going through the defer error path. > >> > >> Changes in v2: > >> * Changed to not only check for EPROBE_DEFER > >> > >> Signed-off-by: Jonathan Marek > >> --- > >> drivers/gpu/drm/msm/adreno/adreno_gpu.c | 17 ++--- > >> drivers/gpu/drm/msm/msm_gpu.c | 2 -- > >> 2 files changed, 14 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > >> b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > >> index 89673c7ed473..0f5217202eb5 100644 > >> --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c > >> +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c > >> @@ -940,12 +940,20 @@ static int adreno_get_pwrlevels(struct device *dev, > >> */ > >> gpu->icc_path = of_icc_get(dev, NULL); > >> } > >> - if (IS_ERR(gpu->icc_path)) > >> + if (IS_ERR(gpu->icc_path)) { > >> + ret = PTR_ERR(gpu->icc_path); > >> gpu->icc_path = NULL; > >> + return ret; > >> + } > >> > >> gpu->ocmem_icc_path = of_icc_get(dev, "ocmem"); > >> - if (IS_ERR(gpu->ocmem_icc_path)) > >> + if (IS_ERR(gpu->ocmem_icc_path)) { > >> + ret = PTR_ERR(gpu->ocmem_icc_path); > >> gpu->ocmem_icc_path = NULL; > >> + /* allow -ENODATA, ocmem icc is optional */ > >> + if (ret != -ENODATA) > >> + return ret; > >> + } > >> > >> return 0; > >> } > >> @@ -996,6 +1004,7 @@ int adreno_gpu_init(struct drm_device *drm, struct > >> platform_device *pdev, > >> struct adreno_platform_config *config = pdev->dev.platform_data; > >> struct msm_gpu_config adreno_gpu_config = { 0 }; > >> struct msm_gpu *gpu = _gpu->base; > >> + int ret; > >> > >> adreno_gpu->funcs = funcs; > >> adreno_gpu->info = adreno_info(config->rev); > >> @@ -1007,7 +1016,9 @@ int adreno_gpu_init(struct drm_device *drm, struct > >> platform_device *pdev, > >> > >> adreno_gpu_config.nr_rings = nr_rings; > >> > >> - adreno_get_pwrlevels(>dev, gpu); > >> + ret = adreno_get_pwrlevels(>dev, gpu); > >> + if (ret) > >> + return ret; > >> > >> pm_runtime_set_autosuspend_delay(>dev, > >> adreno_gpu->info->inactive_period); > >> diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c > >> index a22d30622306..ccf9a0dd9706 100644 > >> --- a/drivers/gpu/drm/msm/msm_gpu.c > >> +++ b/drivers/gpu/drm/msm/msm_gpu.c > >> @@ -959,8 +959,6 @@ void msm_gpu_cleanup(struct msm_gpu *gpu) > >> > >> DBG("%s", gpu->name); > >> > >> - WARN_ON(!list_empty(>active_list)); > >> - > > > > hmm, not a huge fan of removing the WARN_ON().. can we just init the > > list head earlier? > > > > There doesn't seem to be a nice way of doing that. Would it be > reasonable to instead detect that msm_gpu_init wasn't called (checking > if gpu->dev is NULL?), and just skip the msm_gpu_cleanup() call in > adreno_gpu_cleanup() in that case? Hmm, you can't just call msm_gpu_init() before looking up the icc path in adreno_gpu_init()? BR, -R > > > BR, > > -R > > > >> for (i = 0; i < ARRAY_SIZE(gpu->rb); i++) { > >> msm_ringbuffer_destroy(gpu->rb[i]); > >> gpu->rb[i] = NULL; > >> -- > >> 2.26.1 > >> ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/9] dt-bindings: msm/mdp5: Add simple-bus to dpu bindings
This is just like the patch ("dt-bindings: msm/disp: Add simple-bus to dpu bindings") but for the mdp5 bindings. Signed-off-by: Douglas Anderson --- Documentation/devicetree/bindings/display/msm/mdp5.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/msm/mdp5.txt b/Documentation/devicetree/bindings/display/msm/mdp5.txt index 43d11279c925..bb57b07b7546 100644 --- a/Documentation/devicetree/bindings/display/msm/mdp5.txt +++ b/Documentation/devicetree/bindings/display/msm/mdp5.txt @@ -9,7 +9,7 @@ controller found in SoCs like MSM8974, APQ8084, MSM8916, MSM8994 and MSM8996. MDSS: Required properties: - compatible: - * "qcom,mdss" - MDSS + * "qcom,mdss", "simple-bus" - MDSS - reg: Physical base address and length of the controller's registers. - reg-names: The names of register regions. The following regions are required: * "mdss_phys" -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/9] drm/msm: Avoid possible infinite probe deferral and speed booting
I found that if I ever had a little mistake in my kernel config, or device tree, or graphics driver that my system would sit in a loop at bootup trying again and again and again. An example log was: msm ae0.mdss: bound ae01000.mdp (ops 0xffe596e951f8) msm_dsi ae94000.dsi: ae94000.dsi supply gdsc not found, using dummy regulator msm_dsi_manager_register: failed to register mipi dsi host for DSI 0 [drm:ti_sn_bridge_probe] *ERROR* could not find any panel node ... I finally tracked it down where this was happening: - msm_pdev_probe() is called. - msm_pdev_probe() registers drivers. Registering drivers kicks off processing of probe deferrals. - component_master_add_with_match() could return -EPROBE_DEFER. making msm_pdev_probe() return -EPROBE_DEFER. - When msm_pdev_probe() returned the processing of probe deferrals happens. - Loop back to the start. It looks like we can fix this by marking "mdss" as a "simple-bus". I have no idea if people consider this the right thing to do or a hack. Hopefully it's the right thing to do. :-) Once I do this I notice that my boot gets marginally faster (you don't need to probe the sub devices over and over) and also if I have a problem it doesn't loop forever (on my system it still gets upset about some stuck clocks in that case, but at least I can boot up). Unless someone hates this, I'd expect: - Get Rob H to say that the bindings are OK (if they are) and yell that these really need to be converted to yaml (they do). - Get Sean or Rob C to land the bindings and driver patch. - Get Andy or Bjorn to land the dts bits. NOTES: - The first patch could land either way. It's just a cleanup. - I tried to split the dts files into separate patches to ease backporting if desired. Also because I can't actually test most of this hardware myself. Douglas Anderson (9): drm/msm: Use the devm variant of of_platform_populate() dt-bindings: msm/dpu: Add simple-bus to dpu bindings dt-bindings: msm/mdp5: Add simple-bus to dpu bindings drm/msm: Avoid manually populating our children if "simple-bus" is there arm64: dts: qcom: sc7180: Add "simple-bus" to our mdss node arm64: dts: qcom: sdm845: Add "simple-bus" to our mdss node arm64: dts: qcom: msm8916: Add "simple-bus" to our mdss node arm64: dts: qcom: msm8996: Add "simple-bus" to our mdss node ARM: dts: qcom: msm8974: Add "simple-bus" to our mdss node .../devicetree/bindings/display/msm/dpu.txt | 4 ++- .../devicetree/bindings/display/msm/mdp5.txt | 2 +- arch/arm/boot/dts/qcom-msm8974.dtsi | 2 +- arch/arm64/boot/dts/qcom/msm8916.dtsi | 2 +- arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +- arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +- drivers/gpu/drm/msm/msm_drv.c | 34 --- 8 files changed, 24 insertions(+), 26 deletions(-) -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/9] arm64: dts: qcom: msm8916: Add "simple-bus" to our mdss node
As described in the bindings patch, this means that our child nodes are devices in their own right. Signed-off-by: Douglas Anderson --- If this patch lands before the patch ("drm/msm: Avoid manually populating our children if "simple-bus" is there") it doesn't seem to be the end of the world. The first time through add_display_components() it'll notice that the children are already populated and it will be a no-op. When it gets a defer it will then depouplate them (even though it didn't populate them) and future calls will just populate them again. NOTE: I have no way to test this patch, but I'm assuming it works OK? arch/arm64/boot/dts/qcom/msm8916.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/msm8916.dtsi b/arch/arm64/boot/dts/qcom/msm8916.dtsi index 67cae5f9e47e..491362fe02ac 100644 --- a/arch/arm64/boot/dts/qcom/msm8916.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8916.dtsi @@ -1023,7 +1023,7 @@ opp-1920 { }; mdss: mdss@1a0 { - compatible = "qcom,mdss"; + compatible = "qcom,mdss", "simple-bus"; reg = <0x1a0 0x1000>, <0x1ac8000 0x3000>; reg-names = "mdss_phys", "vbif_phys"; -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/9] arm64: dts: qcom: sdm845: Add "simple-bus" to our mdss node
As described in the bindings patch, this means that our child nodes are devices in their own right. Signed-off-by: Douglas Anderson --- If this patch lands before the patch ("drm/msm: Avoid manually populating our children if "simple-bus" is there") it doesn't seem to be the end of the world. The first time through add_display_components() it'll notice that the children are already populated and it will be a no-op. When it gets a defer it will then depouplate them (even though it didn't populate them) and future calls will just populate them again. arch/arm64/boot/dts/qcom/sdm845.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index b0d8308a3e95..e52a5e95168a 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -3582,7 +3582,7 @@ clock_camcc: clock-controller@ad0 { }; mdss: mdss@ae0 { - compatible = "qcom,sdm845-mdss"; + compatible = "qcom,sdm845-mdss", "simple-bus"; reg = <0 0x0ae0 0 0x1000>; reg-names = "mdss"; -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv1 1/4] dt-bindings: display: panel-dsi-cm: convert to YAML
Hi Sebastian, Thank you for the patch. On Tue, Jun 30, 2020 at 07:50:31AM +0200, Sam Ravnborg wrote: > On Tue, Jun 30, 2020 at 12:33:12AM +0200, Sebastian Reichel wrote: > > Convert panel-dsi-cm bindings to YAML and add > > missing properties while at it. > > > > Signed-off-by: Sebastian Reichel > > Thanks, one of the few panel bindings still pending. > And you added some nice explanations too, good. > > Some small comments in the folllowing. > > > --- > > .../bindings/display/panel/panel-dsi-cm.txt | 29 - > > .../bindings/display/panel/panel-dsi-cm.yaml | 100 ++ > > 2 files changed, 100 insertions(+), 29 deletions(-) > > delete mode 100644 > > Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt > > create mode 100644 > > Documentation/devicetree/bindings/display/panel/panel-dsi-cm.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt > > b/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt > > deleted file mode 100644 > > index dce48eb9db57.. > > --- a/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.txt > > +++ /dev/null > > @@ -1,29 +0,0 @@ > > -Generic MIPI DSI Command Mode Panel > > -=== > > - > > -Required properties: > > -- compatible: "panel-dsi-cm" > > - > > -Optional properties: > > -- label: a symbolic name for the panel > > -- reset-gpios: panel reset gpio > > -- te-gpios: panel TE gpio > > - > > -Required nodes: > > -- Video port for DSI input > > - > > -Example > > > > - > > -lcd0: display { > > - compatible = "tpo,taal", "panel-dsi-cm"; > > - label = "lcd0"; > > - > > - reset-gpios = < 6 GPIO_ACTIVE_HIGH>; > > - > > - port { > > - lcd0_in: endpoint { > > - remote-endpoint = <_out_ep>; > > - }; > > - }; > > -}; > > diff --git > > a/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.yaml > > b/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.yaml > > new file mode 100644 > > index ..8d6a20f26470 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/panel/panel-dsi-cm.yaml > > @@ -0,0 +1,100 @@ > > +# SPDX-License-Identifier: (GPL-2.0-only or BSD-2-Clause) > > +%YAML 1.2 > > +--- > > +$id: http://devicetree.org/schemas/display/panel/panel-dsi-cm.yaml# > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > + > > +title: DSI command mode panels > > + > > +maintainers: > > + - Tomi Valkeinen > > + - Sebastian Reichel > > + > > +description: | > > + This binding file is a collection of the DSI panels that > > + are usually driven in command mode. If no backlight is > > + referenced via the optional backlight property, the DSI > > + panel is assumed to have native backlight support. > > > + The panel may use an OF graph binding for the association > > + to the display, or it may be a direct child node of the > > + display. > > Later port: is required which does not really match this explanation. > > > + > > +allOf: > > + - $ref: panel-common.yaml# > > + > > +properties: > > + > > + compatible: > > +items: > > + - enum: > > +- motorola,droid4-panel# Panel from Motorola Droid4 phone > > +- nokia,himalaya # Panel from Nokia N950 phone > > +- tpo,taal # Panel from OMAP4 SDP board > > + - const: panel-dsi-cm# Generic DSI command mode panel > > compatible fallback > > + > > + reg: > > +maxItems: 1 > > +description: DSI virtual channel > > + > > + vddi-supply: > > +description: > > + Display panels require power to be supplied. While several panels > > need > > + more than one power supply with panel-specific constraints governing > > the > > + order and timings of the power supplies, in many cases a single power > > + supply is sufficient, either because the panel has a single power > > rail, or > > + because all its power rails can be driven by the same supply. In > > that case > > + the vddi-supply property specifies the supply powering the panel as a > > + phandle to a regulator. > > + > > + vpnl-supply: > > +description: > > + When the display panel needs a second power supply, this property > > can be > > + used in addition to vddi-supply. Both supplies will be enabled at the > > + same time before the panel is being accessed. > > + > > + width-mm: true > > + height-mm: true > > + label: true > > + rotation: true > > + panel-timing: true > > + port: true > > + reset-gpios: true > > + te-gpios: true > > + backlight: true > > + > > +additionalProperties: false > > + > > +required: > > + - compatible > > + - port > > + - reg > > + > > +examples: > > My personal preference is indent 4 spaces. > But there is no hard rule so do what you like. > > > + - | > > +#include > > + > > +dsi-controller { > > + #address-cells =
[PATCH 8/9] arm64: dts: qcom: msm8996: Add "simple-bus" to our mdss node
As described in the bindings patch, this means that our child nodes are devices in their own right. Signed-off-by: Douglas Anderson --- If this patch lands before the patch ("drm/msm: Avoid manually populating our children if "simple-bus" is there") it doesn't seem to be the end of the world. The first time through add_display_components() it'll notice that the children are already populated and it will be a no-op. When it gets a defer it will then depouplate them (even though it didn't populate them) and future calls will just populate them again. NOTE: I have no way to test this patch, but I'm assuming it works OK? arch/arm64/boot/dts/qcom/msm8996.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/msm8996.dtsi b/arch/arm64/boot/dts/qcom/msm8996.dtsi index 9951286db775..e303b0e644ac 100644 --- a/arch/arm64/boot/dts/qcom/msm8996.dtsi +++ b/arch/arm64/boot/dts/qcom/msm8996.dtsi @@ -499,7 +499,7 @@ mmcc: clock-controller@8c { }; mdss: mdss@90 { - compatible = "qcom,mdss"; + compatible = "qcom,mdss", "simple-bus"; reg = <0x0090 0x1000>, <0x009b 0x1040>, -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/9] arm64: dts: qcom: sc7180: Add "simple-bus" to our mdss node
As described in the bindings patch, this means that our child nodes are devices in their own right. Signed-off-by: Douglas Anderson --- If this patch lands before the patch ("drm/msm: Avoid manually populating our children if "simple-bus" is there") it doesn't seem to be the end of the world. The first time through add_display_components() it'll notice that the children are already populated and it will be a no-op. When it gets a defer it will then depouplate them (even though it didn't populate them) and future calls will just populate them again. arch/arm64/boot/dts/qcom/sc7180.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi b/arch/arm64/boot/dts/qcom/sc7180.dtsi index 2be81a2a1512..cae6c69fd85a 100644 --- a/arch/arm64/boot/dts/qcom/sc7180.dtsi +++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi @@ -2594,7 +2594,7 @@ camnoc_virt: interconnect@ac0 { }; mdss: mdss@ae0 { - compatible = "qcom,sc7180-mdss"; + compatible = "qcom,sc7180-mdss", "simple-bus"; reg = <0 0x0ae0 0 0x1000>; reg-names = "mdss"; -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/9] drm/msm: Avoid manually populating our children if "simple-bus" is there
If our compatible string has "simple-bus" at the end of it then the system will handle probing our children for us. Doing this has a few advantages: 1. If we defer we don't have to keep probing / removing our children which should speed up boot. The system just probes them once. 2. It fixes a problem where we could get into a big loop where we'd have: - msm_pdev_probe() is called. - msm_pdev_probe() registers drivers. Registering drivers kicks off processing of probe deferrals. - component_master_add_with_match() could return -EPROBE_DEFER. making msm_pdev_probe() return -EPROBE_DEFER. - When msm_pdev_probe() returned the processing of probe deferrals happens. - Loop back to the start. Signed-off-by: Douglas Anderson --- drivers/gpu/drm/msm/msm_drv.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index 78b09fde9e29..f7c6ef147095 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1208,10 +1208,18 @@ static int add_display_components(struct device *dev, if (of_device_is_compatible(dev->of_node, "qcom,mdss") || of_device_is_compatible(dev->of_node, "qcom,sdm845-mdss") || of_device_is_compatible(dev->of_node, "qcom,sc7180-mdss")) { - ret = devm_of_platform_populate(dev); - if (ret) { - DRM_DEV_ERROR(dev, "failed to populate children devices\n"); - return ret; + /* +* Old device tree files didn't include "simple-bus" +* in their compatible string so we had to manually pouplate +* our children. When existing device trees are fixed this +* can be removed. +*/ + if (!of_device_is_compatible(dev->of_node, "simple-bus")) { + ret = devm_of_platform_populate(dev); + if (ret) { + DRM_DEV_ERROR(dev, "failed to populate children devices\n"); + return ret; + } } mdp_dev = device_find_child(dev, NULL, compare_name_mdp); -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 9/9] ARM: dts: qcom: msm8974: Add "simple-bus" to our mdss node
As described in the bindings patch, this means that our child nodes are devices in their own right. Signed-off-by: Douglas Anderson --- If this patch lands before the patch ("drm/msm: Avoid manually populating our children if "simple-bus" is there") it doesn't seem to be the end of the world. The first time through add_display_components() it'll notice that the children are already populated and it will be a no-op. When it gets a defer it will then depouplate them (even though it didn't populate them) and future calls will just populate them again. NOTE: I have no way to test this patch, but I'm assuming it works OK? arch/arm/boot/dts/qcom-msm8974.dtsi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi index 51f5f904f9eb..9f84c9fd716c 100644 --- a/arch/arm/boot/dts/qcom-msm8974.dtsi +++ b/arch/arm/boot/dts/qcom-msm8974.dtsi @@ -1402,7 +1402,7 @@ cnoc: interconnect@fc48 { mdss: mdss@fd90 { status = "disabled"; - compatible = "qcom,mdss"; + compatible = "qcom,mdss", "simple-bus"; reg = <0xfd90 0x100>, <0xfd924000 0x1000>; reg-names = "mdss_phys", -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/9] dt-bindings: msm/dpu: Add simple-bus to dpu bindings
We have a whole bunch of child devices that we really just want treated as other devices to instantiate. Add the "simple-bus" property for this. Commit-notes This bindings file really needs to move over to yaml. Hopefully someone at Qualcomm who's working on display code can help with that. Right now on Linux we have a manual call to of_platform_populate() to populate our children. That's pretty non-ideal as described in another patch in this series and I'm trying to remove it. I'm not sure how much of a hack to consider "simple-bus". I've seen it used like this before, but if folks tell me that it's terrible then I guess we'll have to figure some other way out of our crazy -EPROBE_DEFER loop in Linux. END Signed-off-by: Douglas Anderson --- Documentation/devicetree/bindings/display/msm/dpu.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/display/msm/dpu.txt b/Documentation/devicetree/bindings/display/msm/dpu.txt index 551ae26f60da..b88269524365 100644 --- a/Documentation/devicetree/bindings/display/msm/dpu.txt +++ b/Documentation/devicetree/bindings/display/msm/dpu.txt @@ -8,7 +8,9 @@ The DPU display controller is found in SDM845 SoC. MDSS: Required properties: -- compatible: "qcom,sdm845-mdss", "qcom,sc7180-mdss" +- compatible: One of: + - "qcom,sdm845-mdss", "simple-bus" + - "qcom,sc7180-mdss", "simple-bus" - reg: physical base address and length of contoller's registers. - reg-names: register region names. The following region is required: * "mdss" -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/9] drm/msm: Use the devm variant of of_platform_populate()
Let's save ourselves some hassle and use the devm variant of of_platform_populate() do we don't need to worry about manually depopulating. Signed-off-by: Douglas Anderson --- drivers/gpu/drm/msm/msm_drv.c | 20 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c index f6ce40bf3699..78b09fde9e29 100644 --- a/drivers/gpu/drm/msm/msm_drv.c +++ b/drivers/gpu/drm/msm/msm_drv.c @@ -1208,7 +1208,7 @@ static int add_display_components(struct device *dev, if (of_device_is_compatible(dev->of_node, "qcom,mdss") || of_device_is_compatible(dev->of_node, "qcom,sdm845-mdss") || of_device_is_compatible(dev->of_node, "qcom,sc7180-mdss")) { - ret = of_platform_populate(dev->of_node, NULL, NULL, dev); + ret = devm_of_platform_populate(dev); if (ret) { DRM_DEV_ERROR(dev, "failed to populate children devices\n"); return ret; @@ -1217,7 +1217,6 @@ static int add_display_components(struct device *dev, mdp_dev = device_find_child(dev, NULL, compare_name_mdp); if (!mdp_dev) { DRM_DEV_ERROR(dev, "failed to find MDSS MDP node\n"); - of_platform_depopulate(dev); return -ENODEV; } @@ -1232,8 +1231,6 @@ static int add_display_components(struct device *dev, } ret = add_components_mdp(mdp_dev, matchptr); - if (ret) - of_platform_depopulate(dev); return ret; } @@ -1300,30 +1297,21 @@ static int msm_pdev_probe(struct platform_device *pdev) ret = add_gpu_components(>dev, ); if (ret) - goto fail; + return ret; /* on all devices that I am aware of, iommu's which can map * any address the cpu can see are used: */ ret = dma_set_mask_and_coherent(>dev, ~0); if (ret) - goto fail; - - ret = component_master_add_with_match(>dev, _drm_ops, match); - if (ret) - goto fail; - - return 0; + return ret; -fail: - of_platform_depopulate(>dev); - return ret; + return component_master_add_with_match(>dev, _drm_ops, match); } static int msm_pdev_remove(struct platform_device *pdev) { component_master_del(>dev, _drm_ops); - of_platform_depopulate(>dev); return 0; } -- 2.27.0.383.g050319c2ae-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv1 4/4] ARM: dts: omap4-droid4: add panel orientation
Hi Sebastian, Thank you for the patch. On Tue, Jun 30, 2020 at 12:33:15AM +0200, Sebastian Reichel wrote: > Add information about panel orientation, so that the > system boots into a properly rotated shell. > > Signed-off-by: Sebastian Reichel Reviewed-by: Laurent Pinchart > --- > arch/arm/boot/dts/motorola-mapphone-common.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/boot/dts/motorola-mapphone-common.dtsi > b/arch/arm/boot/dts/motorola-mapphone-common.dtsi > index 0e22fdfa42aa..e672e714fcbe 100644 > --- a/arch/arm/boot/dts/motorola-mapphone-common.dtsi > +++ b/arch/arm/boot/dts/motorola-mapphone-common.dtsi > @@ -218,6 +218,7 @@ lcd0: panel@0 { > > width-mm = <50>; > height-mm = <89>; > + rotation = <90>; > > panel-timing { > clock-frequency = <0>; /* Calculated by dsi */ -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv1 2/4] ARM: dts: omap: add channel to DSI panels
Hi Sebastian, Thank you for the patch. On Tue, Jun 30, 2020 at 12:33:13AM +0200, Sebastian Reichel wrote: > The standard binding for DSI requires, that the channel number s/requires,/requires/ > of the panel is encoded in DT. This adds the channel number in > all OMAP3-5 boards, in preparation for using common infrastructure. > > Signed-off-by: Sebastian Reichel Reviewed-by: Laurent Pinchart > --- > arch/arm/boot/dts/motorola-mapphone-common.dtsi | 3 ++- > arch/arm/boot/dts/omap3-n950.dts| 3 ++- > arch/arm/boot/dts/omap3.dtsi| 3 +++ > arch/arm/boot/dts/omap4-sdp.dts | 6 -- > arch/arm/boot/dts/omap4.dtsi| 6 ++ > arch/arm/boot/dts/omap5.dtsi| 6 ++ > 6 files changed, 23 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/boot/dts/motorola-mapphone-common.dtsi > b/arch/arm/boot/dts/motorola-mapphone-common.dtsi > index 06fbffa81636..4ffe461c3808 100644 > --- a/arch/arm/boot/dts/motorola-mapphone-common.dtsi > +++ b/arch/arm/boot/dts/motorola-mapphone-common.dtsi > @@ -207,8 +207,9 @@ dsi1_out_ep: endpoint { > }; > }; > > - lcd0: display { > + lcd0: panel@0 { > compatible = "panel-dsi-cm"; > + reg = <0>; > label = "lcd0"; > vddi-supply = <_regulator>; > reset-gpios = < 5 GPIO_ACTIVE_HIGH>; /* gpio101 */ > diff --git a/arch/arm/boot/dts/omap3-n950.dts > b/arch/arm/boot/dts/omap3-n950.dts > index 31d47a1fad84..80cf4e1177da 100644 > --- a/arch/arm/boot/dts/omap3-n950.dts > +++ b/arch/arm/boot/dts/omap3-n950.dts > @@ -225,8 +225,9 @@ dsi_out_ep: endpoint { > }; > }; > > - lcd0: display { > + lcd0: panel@0 { > compatible = "nokia,himalaya", "panel-dsi-cm"; > + reg = <0>; > label = "lcd0"; > > pinctrl-names = "default"; > diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi > index 1296d0643943..0ebbb6c11f04 100644 > --- a/arch/arm/boot/dts/omap3.dtsi > +++ b/arch/arm/boot/dts/omap3.dtsi > @@ -898,6 +898,9 @@ dsi: encoder@4804fc00 { > ti,hwmods = "dss_dsi1"; > clocks = <_alwon_fck>, <_alwon_fck>; > clock-names = "fck", "sys_clk"; > + > + #address-cells = <1>; > + #size-cells = <0>; > }; > > rfbi: encoder@48050800 { > diff --git a/arch/arm/boot/dts/omap4-sdp.dts b/arch/arm/boot/dts/omap4-sdp.dts > index 91480ac1f328..8a8307517dab 100644 > --- a/arch/arm/boot/dts/omap4-sdp.dts > +++ b/arch/arm/boot/dts/omap4-sdp.dts > @@ -662,8 +662,9 @@ dsi1_out_ep: endpoint { > }; > }; > > - lcd0: display { > + lcd0: panel@0 { > compatible = "tpo,taal", "panel-dsi-cm"; > + reg = <0>; > label = "lcd0"; > > reset-gpios = < 6 GPIO_ACTIVE_HIGH>; /* 102 */ > @@ -687,8 +688,9 @@ dsi2_out_ep: endpoint { > }; > }; > > - lcd1: display { > + lcd1: panel@0 { > compatible = "tpo,taal", "panel-dsi-cm"; > + reg = <0>; > label = "lcd1"; > > reset-gpios = < 8 GPIO_ACTIVE_HIGH>; /* 104 */ > diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi > index 4400f5f8e099..c5b426616443 100644 > --- a/arch/arm/boot/dts/omap4.dtsi > +++ b/arch/arm/boot/dts/omap4.dtsi > @@ -551,6 +551,9 @@ dsi1: encoder@0 { > clocks = <_dss_clkctrl > OMAP4_DSS_CORE_CLKCTRL 8>, ><_dss_clkctrl > OMAP4_DSS_CORE_CLKCTRL 10>; > clock-names = "fck", "sys_clk"; > + > + #address-cells = <1>; > + #size-cells = <0>; > }; > }; > > @@ -583,6 +586,9 @@ dsi2: encoder@0 { > clocks = <_dss_clkctrl > OMAP4_DSS_CORE_CLKCTRL 8>, ><_dss_clkctrl > OMAP4_DSS_CORE_CLKCTRL 10>; > clock-names = "fck", "sys_clk"; > + > + #address-cells = <1>; > + #size-cells = <0>; > }; > }; > > diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi > index fb889c5b00c9..0855c0a4050f 100644 > --- a/arch/arm/boot/dts/omap5.dtsi > +++ b/arch/arm/boot/dts/omap5.dtsi > @@ -491,6 +491,9 @@ dsi1: encoder@0 { > status = "disabled"; >
Re: [PATCHv1 3/4] ARM: dts: omap4-droid4: add panel compatible
Hi Sebastian, Thank you for the patch, and for your continuous effort on this. On Tue, Jun 30, 2020 at 12:33:14AM +0200, Sebastian Reichel wrote: > Add Droid 4 specific compatible value in addition to the > generic one, so that we have the ability to add panel > specific quirks in the future. > > Signed-off-by: Sebastian Reichel Reviewed-by: Laurent Pinchart > --- > arch/arm/boot/dts/motorola-mapphone-common.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/motorola-mapphone-common.dtsi > b/arch/arm/boot/dts/motorola-mapphone-common.dtsi > index 4ffe461c3808..0e22fdfa42aa 100644 > --- a/arch/arm/boot/dts/motorola-mapphone-common.dtsi > +++ b/arch/arm/boot/dts/motorola-mapphone-common.dtsi > @@ -208,7 +208,7 @@ dsi1_out_ep: endpoint { > }; > > lcd0: panel@0 { > - compatible = "panel-dsi-cm"; > + compatible = "motorola,droid4-panel", "panel-dsi-cm"; > reg = <0>; > label = "lcd0"; > vddi-supply = <_regulator>; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 21/21] drm/bridge: ti-sn65dsi86: add drm_panel_bridge support
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:17PM +0200, Sam Ravnborg wrote: > Prepare the bridge driver for use in a chained setup by > replacing direct use of drm_panel with drm_panel_bridge support. > > Note: the bridge panel will use the connector type from the panel. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 27 +-- > 1 file changed, 13 insertions(+), 14 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > index 0f75bb2d7f56..ecf0693e3018 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > @@ -144,7 +144,7 @@ struct ti_sn_bridge { > struct device_node *host_node; > struct mipi_dsi_device *dsi; > struct clk *refclk; > - struct drm_panel*panel; > + struct drm_bridge *panel_bridge; > struct gpio_desc*enable_gpio; > struct regulator_bulk_data supplies[SN_REGULATOR_SUPPLY_NUM]; > int dp_lanes; > @@ -263,7 +263,7 @@ static int ti_sn_bridge_connector_get_modes(struct > drm_connector *connector) > { > struct ti_sn_bridge *pdata = connector_to_ti_sn_bridge(connector); > > - return drm_panel_get_modes(pdata->panel, connector); > + return drm_bridge_get_modes(pdata->panel_bridge, connector); > } > > static enum drm_mode_status > @@ -395,9 +395,8 @@ static int ti_sn_bridge_attach(struct drm_bridge *bridge, > pdata->dsi = dsi; > > /* attach panel to bridge */ > - drm_panel_attach(pdata->panel, >connector); > - > - return 0; > + return drm_bridge_attach(bridge->encoder, pdata->panel_bridge, > + bridge, flags); Same comment as earlier in this series regarding the flags. I suppose attaching the panel bridge will be moved before creating the connector in a future patch series ? > > err_dsi_attach: > mipi_dsi_device_unregister(dsi); > @@ -410,16 +409,12 @@ static void ti_sn_bridge_disable(struct drm_bridge > *bridge) > { > struct ti_sn_bridge *pdata = bridge_to_ti_sn_bridge(bridge); > > - drm_panel_disable(pdata->panel); > - > /* disable video stream */ > regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, 0); > /* semi auto link training mode OFF */ > regmap_write(pdata->regmap, SN_ML_TX_MODE_REG, 0); > /* disable DP PLL */ > regmap_write(pdata->regmap, SN_PLL_ENABLE_REG, 0); > - > - drm_panel_unprepare(pdata->panel); > } > > static u32 ti_sn_bridge_get_dsi_freq(struct ti_sn_bridge *pdata) > @@ -780,8 +775,6 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge) > /* enable video stream */ > regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, VSTREAM_ENABLE, > VSTREAM_ENABLE); > - > - drm_panel_enable(pdata->panel); > } > > static void ti_sn_bridge_pre_enable(struct drm_bridge *bridge) > @@ -811,8 +804,6 @@ static void ti_sn_bridge_pre_enable(struct drm_bridge > *bridge) >*/ > regmap_update_bits(pdata->regmap, SN_HPD_DISABLE_REG, HPD_DISABLE, > HPD_DISABLE); > - > - drm_panel_prepare(pdata->panel); > } > > static void ti_sn_bridge_post_disable(struct drm_bridge *bridge) > @@ -1163,6 +1154,8 @@ static int ti_sn_bridge_probe(struct i2c_client *client, > const struct i2c_device_id *id) > { > struct ti_sn_bridge *pdata; > + struct drm_bridge *bridge; > + struct drm_panel *panel; > int ret; > > if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C)) { > @@ -1185,12 +1178,18 @@ static int ti_sn_bridge_probe(struct i2c_client > *client, > pdata->dev = >dev; > > ret = drm_of_find_panel_or_bridge(pdata->dev->of_node, 1, 0, > - >panel, NULL); > + , NULL); > if (ret) { > DRM_ERROR("could not find any panel node\n"); > return ret; > } > > + bridge = devm_drm_panel_bridge_add(pdata->dev, panel); > + if (IS_ERR(bridge)) > + return PTR_ERR(bridge); > + > + pdata->panel_bridge = bridge; > + > dev_set_drvdata(>dev, pdata); > > pdata->enable_gpio = devm_gpiod_get(pdata->dev, "enable", -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 20/21] drm/bridge: nxp-ptn3460: make connector creation optional
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:16PM +0200, Sam Ravnborg wrote: > Make the connector creation optional to enable usage of the > nxp-ptn3460 bridge with the DRM bridge connector helper. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/bridge/nxp-ptn3460.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c > b/drivers/gpu/drm/bridge/nxp-ptn3460.c > index e253c185f94c..6a65657087f9 100644 > --- a/drivers/gpu/drm/bridge/nxp-ptn3460.c > +++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c > @@ -229,10 +229,8 @@ static int ptn3460_bridge_attach(struct drm_bridge > *bridge, > if (ret < 0) > return ret; > > - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { > - DRM_ERROR("Fix bridge driver to make connector optional!"); > - return -EINVAL; > - } > + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) > + return 0; > > if (!bridge->encoder) { > DRM_ERROR("Parent encoder object not found"); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 19/21] drm/bridge: nxp-ptn3460: add get_modes bridge operation
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:15PM +0200, Sam Ravnborg wrote: > Add the get_modes() bridge operation to prepare for > use as a chained bridge. > Add helper function that is also used by the connector. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/nxp-ptn3460.c | 52 ++-- > 1 file changed, 33 insertions(+), 19 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c > b/drivers/gpu/drm/bridge/nxp-ptn3460.c > index 0bd9f0e451b3..e253c185f94c 100644 > --- a/drivers/gpu/drm/bridge/nxp-ptn3460.c > +++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c > @@ -154,17 +154,13 @@ static void ptn3460_disable(struct drm_bridge *bridge) > gpiod_set_value(ptn_bridge->gpio_pd_n, 0); > } > > -static int ptn3460_get_modes(struct drm_connector *connector) > +static struct edid *ptn3460_get_edid(struct drm_bridge *bridge, > + struct drm_connector *connector) > { > - struct ptn3460_bridge *ptn_bridge; > - u8 *edid; > - int ret, num_modes = 0; > + struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge); > bool power_off; > - > - ptn_bridge = connector_to_ptn3460(connector); > - > - if (ptn_bridge->edid) > - return drm_add_edid_modes(connector, ptn_bridge->edid); > + u8 *edid; > + int ret; > > power_off = !ptn_bridge->enabled; > ptn3460_pre_enable(_bridge->bridge); > @@ -172,30 +168,46 @@ static int ptn3460_get_modes(struct drm_connector > *connector) > edid = kmalloc(EDID_LENGTH, GFP_KERNEL); > if (!edid) { > DRM_ERROR("Failed to allocate EDID\n"); > - return 0; > + return NULL; Don't you need to power off in the error path ? > } > > ret = ptn3460_read_bytes(ptn_bridge, PTN3460_EDID_ADDR, edid, > - EDID_LENGTH); > + EDID_LENGTH); > if (ret) { > kfree(edid); > - goto out; > + return NULL; And here too ? > } > > + if (power_off) > + ptn3460_disable(_bridge->bridge); > + > + kfree(ptn_bridge->edid); > ptn_bridge->edid = (struct edid *)edid; > - drm_connector_update_edid_property(connector, ptn_bridge->edid); > > - num_modes = drm_add_edid_modes(connector, ptn_bridge->edid); > + return ptn_bridge->edid; Same comment as earlier in this series about storing the edid. > +} > > -out: > - if (power_off) > - ptn3460_disable(_bridge->bridge); > +static int ptn3460_connector_get_modes(struct drm_connector *connector) > +{ > + struct ptn3460_bridge *ptn_bridge; > + struct edid *edid; > + > + ptn_bridge = connector_to_ptn3460(connector); Maybe struct ptn3460_bridge *ptn_bridge = connector_to_ptn3460(connector); ? > + > + if (ptn_bridge->edid) > + return drm_add_edid_modes(connector, ptn_bridge->edid); > + > + edid = ptn3460_get_edid(_bridge->bridge, connector); > + if (!edid) > + return 0; > + > + drm_connector_update_edid_property(connector, edid); > > - return num_modes; > + return drm_add_edid_modes(connector, edid); > } > > static const struct drm_connector_helper_funcs > ptn3460_connector_helper_funcs = { > - .get_modes = ptn3460_get_modes, > + .get_modes = ptn3460_connector_get_modes, > }; > > static const struct drm_connector_funcs ptn3460_connector_funcs = { > @@ -249,6 +261,7 @@ static const struct drm_bridge_funcs ptn3460_bridge_funcs > = { > .pre_enable = ptn3460_pre_enable, > .disable = ptn3460_disable, > .attach = ptn3460_bridge_attach, > + .get_edid = ptn3460_get_edid, > }; > > static int ptn3460_probe(struct i2c_client *client, > @@ -304,6 +317,7 @@ static int ptn3460_probe(struct i2c_client *client, > } > > ptn_bridge->bridge.funcs = _bridge_funcs; > + ptn_bridge->bridge.ops = DRM_BRIDGE_OP_EDID; > ptn_bridge->bridge.of_node = dev->of_node; > drm_bridge_add(_bridge->bridge); > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 18/21] drm/bridge: nxp-ptn3460: add drm_panel_bridge support
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:14PM +0200, Sam Ravnborg wrote: > Prepare the bridge driver for use in a chained setup by > replacing direct use of drm_panel with drm_panel_bridge support. > > Note: the bridge panel will use the connector type from the panel. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/nxp-ptn3460.c | 51 > 1 file changed, 14 insertions(+), 37 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c > b/drivers/gpu/drm/bridge/nxp-ptn3460.c > index 438e566ce0a4..0bd9f0e451b3 100644 > --- a/drivers/gpu/drm/bridge/nxp-ptn3460.c > +++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c > @@ -30,7 +30,7 @@ struct ptn3460_bridge { > struct i2c_client *client; > struct drm_bridge bridge; > struct edid *edid; > - struct drm_panel *panel; > + struct drm_bridge *panel_bridge; > struct gpio_desc *gpio_pd_n; > struct gpio_desc *gpio_rst_n; > u32 edid_emulation; > @@ -127,11 +127,6 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) > usleep_range(10, 20); > gpiod_set_value(ptn_bridge->gpio_rst_n, 1); > > - if (drm_panel_prepare(ptn_bridge->panel)) { > - DRM_ERROR("failed to prepare panel\n"); > - return; > - } > - > /* >* There's a bug in the PTN chip where it falsely asserts hotplug before >* it is fully functional. We're forced to wait for the maximum start up > @@ -146,16 +141,6 @@ static void ptn3460_pre_enable(struct drm_bridge *bridge) > ptn_bridge->enabled = true; > } > > -static void ptn3460_enable(struct drm_bridge *bridge) > -{ > - struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge); > - > - if (drm_panel_enable(ptn_bridge->panel)) { > - DRM_ERROR("failed to enable panel\n"); > - return; > - } > -} > - > static void ptn3460_disable(struct drm_bridge *bridge) > { > struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge); > @@ -165,25 +150,10 @@ static void ptn3460_disable(struct drm_bridge *bridge) > > ptn_bridge->enabled = false; > > - if (drm_panel_disable(ptn_bridge->panel)) { > - DRM_ERROR("failed to disable panel\n"); > - return; > - } > - > gpiod_set_value(ptn_bridge->gpio_rst_n, 1); > gpiod_set_value(ptn_bridge->gpio_pd_n, 0); > } > > -static void ptn3460_post_disable(struct drm_bridge *bridge) > -{ > - struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge); > - > - if (drm_panel_unprepare(ptn_bridge->panel)) { > - DRM_ERROR("failed to unprepare panel\n"); > - return; > - } > -} > - > static int ptn3460_get_modes(struct drm_connector *connector) > { > struct ptn3460_bridge *ptn_bridge; > @@ -242,6 +212,11 @@ static int ptn3460_bridge_attach(struct drm_bridge > *bridge, > struct ptn3460_bridge *ptn_bridge = bridge_to_ptn3460(bridge); > int ret; > > + ret = drm_bridge_attach(bridge->encoder, ptn_bridge->panel_bridge, > + bridge, flags); Same as earlier in the series about the flags. > + if (ret < 0) > + return ret; > + > if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { > DRM_ERROR("Fix bridge driver to make connector optional!"); > return -EINVAL; > @@ -265,9 +240,6 @@ static int ptn3460_bridge_attach(struct drm_bridge > *bridge, > drm_connector_attach_encoder(_bridge->connector, > bridge->encoder); > > - if (ptn_bridge->panel) > - drm_panel_attach(ptn_bridge->panel, _bridge->connector); > - > drm_helper_hpd_irq_event(ptn_bridge->connector.dev); > > return ret; > @@ -275,9 +247,7 @@ static int ptn3460_bridge_attach(struct drm_bridge > *bridge, > > static const struct drm_bridge_funcs ptn3460_bridge_funcs = { > .pre_enable = ptn3460_pre_enable, > - .enable = ptn3460_enable, > .disable = ptn3460_disable, > - .post_disable = ptn3460_post_disable, > .attach = ptn3460_bridge_attach, > }; > > @@ -286,6 +256,8 @@ static int ptn3460_probe(struct i2c_client *client, > { > struct device *dev = >dev; > struct ptn3460_bridge *ptn_bridge; > + struct drm_bridge *pbridge; > + struct drm_panel *panel; > int ret; > > ptn_bridge = devm_kzalloc(dev, sizeof(*ptn_bridge), GFP_KERNEL); > @@ -293,10 +265,15 @@ static int ptn3460_probe(struct i2c_client *client, > return -ENOMEM; > } > > - ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, > _bridge->panel, NULL); > + ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, , NULL); > if (ret) > return ret; > > + pbridge = devm_drm_panel_bridge_add(dev, panel); > +
Re: [PATCH v3 17/21] drm/bridge: megachips: make connector creation optional
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:13PM +0200, Sam Ravnborg wrote: > Make the connector creation optional to enable usage of the > megachips-stdp-ge-b850v3-fw bridge with the DRM bridge connector helper. > > Signed-off-by: Sam Ravnborg > Cc: Peter Senna Tschudin > Cc: Martin Donnelly > Cc: Martyn Welch > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec Reviewed-by: Laurent Pinchart > --- > .../drm/bridge/megachips-stdp-ge-b850v3-fw.c | 15 --- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > index 5f06e18f0a61..991417ab35b6 100644 > --- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > +++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > @@ -251,16 +251,6 @@ static int ge_b850v3_lvds_attach(struct drm_bridge > *bridge, > { > struct i2c_client *stdp4028_i2c > = ge_b850v3_lvds_ptr->stdp4028_i2c; > - int ret; > - > - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { > - DRM_ERROR("Fix bridge driver to make connector optional!"); > - return -EINVAL; > - } > - > - ret = ge_b850v3_lvds_create_connector(bridge); > - if (ret) > - return ret; > > /* Configures the bridge to re-enable interrupts after each ack. */ > i2c_smbus_write_word_data(stdp4028_i2c, > @@ -272,7 +262,10 @@ static int ge_b850v3_lvds_attach(struct drm_bridge > *bridge, > STDP4028_DPTX_IRQ_EN_REG, > STDP4028_DPTX_IRQ_CONFIG); > > - return 0; > + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) > + return 0; > + > + return ge_b850v3_lvds_create_connector(bridge); > } > > static const struct drm_bridge_funcs ge_b850v3_lvds_funcs = { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 16/21] drm/bridge: megachips: add get_edid bridge operation
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:12PM +0200, Sam Ravnborg wrote: > To prepare for a chained bridge setup add support for the > get_edid bridge operation. > > Signed-off-by: Sam Ravnborg > Cc: Peter Senna Tschudin > Cc: Martin Donnelly > Cc: Martyn Welch > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > .../bridge/megachips-stdp-ge-b850v3-fw.c | 26 +-- > 1 file changed, 18 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > index 78a9afe8f063..5f06e18f0a61 100644 > --- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > +++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > @@ -131,21 +131,29 @@ static u8 *stdp2690_get_edid(struct i2c_client *client) > return NULL; > } > > -static int ge_b850v3_lvds_get_modes(struct drm_connector *connector) > +static struct edid *ge_b850v3_lvds_get_edid( > + struct drm_bridge *bridge, struct drm_connector *connector) > { > struct i2c_client *client; > - int num_modes = 0; > > client = ge_b850v3_lvds_ptr->stdp2690_i2c; > > kfree(ge_b850v3_lvds_ptr->edid); > ge_b850v3_lvds_ptr->edid = (struct edid *)stdp2690_get_edid(client); > > - if (ge_b850v3_lvds_ptr->edid) { > - drm_connector_update_edid_property(connector, > - ge_b850v3_lvds_ptr->edid); > - num_modes = drm_add_edid_modes(connector, > -ge_b850v3_lvds_ptr->edid); > + return ge_b850v3_lvds_ptr->edid; As pointed out earlier in this series, you can't store a pointer to the edid, if will get freed by the caller. Fortunately it doesn't seem to be needed here either. > +} > + > +static int ge_b850v3_lvds_get_modes(struct drm_connector *connector) > +{ > + struct edid *edid; > + int num_modes = 0; > + > + edid = ge_b850v3_lvds_get_edid(_b850v3_lvds_ptr->bridge, connector); > + > + if (edid) { > + drm_connector_update_edid_property(connector, edid); > + num_modes = drm_add_edid_modes(connector, edid); > } > > return num_modes; > @@ -270,6 +278,7 @@ static int ge_b850v3_lvds_attach(struct drm_bridge > *bridge, > static const struct drm_bridge_funcs ge_b850v3_lvds_funcs = { > .attach = ge_b850v3_lvds_attach, > .detect = ge_b850v3_lvds_bridge_detect, > + .get_edid = ge_b850v3_lvds_get_edid, > }; > > static int ge_b850v3_lvds_init(struct device *dev) > @@ -324,7 +333,8 @@ static int stdp4028_ge_b850v3_fw_probe(struct i2c_client > *stdp4028_i2c, > > /* drm bridge initialization */ > ge_b850v3_lvds_ptr->bridge.funcs = _b850v3_lvds_funcs; > - ge_b850v3_lvds_ptr->bridge.ops = DRM_BRIDGE_OP_DETECT; > + ge_b850v3_lvds_ptr->bridge.ops = DRM_BRIDGE_OP_DETECT | > + DRM_BRIDGE_OP_EDID; > ge_b850v3_lvds_ptr->bridge.of_node = dev->of_node; > drm_bridge_add(_b850v3_lvds_ptr->bridge); > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/7] drm: msm: a6xx: set gpu freq through hfi
On 7/11/2020 2:43 AM, Akhil P Oommen wrote: On 7/10/2020 1:34 AM, Jonathan Marek wrote: On 7/9/20 4:00 PM, Akhil P Oommen wrote: Newer targets support changing gpu frequency through HFI. So use that wherever supported instead of the legacy method. It was already using HFI on newer targets. Don't break it in one commit then fix it in the next. Oops. I somehow got confused. Will fix and resend. -Akhil I broke the pm_runtime_get_if_in_use() check too. Other than that, just squashing this patch with the previous one should be enough. -Akhil. Signed-off-by: Akhil P Oommen --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 233afea..b547339 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -121,6 +121,12 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp) if (gpu_freq == gmu->gpu_freqs[perf_index]) break; + if (!gmu->legacy) { + a6xx_hfi_set_freq(gmu, gmu->current_perf_index); + icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); + return; + } + gmu->current_perf_index = perf_index; gmu->freq = gmu->gpu_freqs[perf_index]; @@ -893,10 +899,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) enable_irq(gmu->hfi_irq); /* Set the GPU to the current freq */ - if (gmu->legacy) - a6xx_gmu_set_initial_freq(gpu, gmu); - else - a6xx_hfi_set_freq(gmu, gmu->current_perf_index); + a6xx_gmu_set_initial_freq(gpu, gmu); /* * "enable" the GX power domain which won't actually do anything but it ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 15/21] drm/bridge: megachips: enable detect bridge operation
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:11PM +0200, Sam Ravnborg wrote: > To prepare for use in a chained bridge setup enable the > detect operation. > > Signed-off-by: Sam Ravnborg > Cc: Peter Senna Tschudin > Cc: Martin Donnelly > Cc: Martyn Welch > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec Reviewed-by: Laurent Pinchart > --- > .../gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 12 ++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > index cf1dfbc88acf..78a9afe8f063 100644 > --- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > +++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > @@ -163,8 +163,8 @@ drm_connector_helper_funcs > ge_b850v3_lvds_connector_helper_funcs = { > .mode_valid = ge_b850v3_lvds_mode_valid, > }; > > -static enum drm_connector_status ge_b850v3_lvds_detect( > - struct drm_connector *connector, bool force) > +static enum drm_connector_status ge_b850v3_lvds_bridge_detect( > + struct drm_bridge *bridge) > { > struct i2c_client *stdp4028_i2c = > ge_b850v3_lvds_ptr->stdp4028_i2c; > @@ -182,6 +182,12 @@ static enum drm_connector_status ge_b850v3_lvds_detect( > return connector_status_unknown; > } > > +static enum drm_connector_status ge_b850v3_lvds_detect( > + struct drm_connector *connector, bool force) > +{ > + return ge_b850v3_lvds_bridge_detect(_b850v3_lvds_ptr->bridge); > +} > + > static const struct drm_connector_funcs ge_b850v3_lvds_connector_funcs = { > .fill_modes = drm_helper_probe_single_connector_modes, > .detect = ge_b850v3_lvds_detect, > @@ -263,6 +269,7 @@ static int ge_b850v3_lvds_attach(struct drm_bridge > *bridge, > > static const struct drm_bridge_funcs ge_b850v3_lvds_funcs = { > .attach = ge_b850v3_lvds_attach, > + .detect = ge_b850v3_lvds_bridge_detect, > }; > > static int ge_b850v3_lvds_init(struct device *dev) > @@ -317,6 +324,7 @@ static int stdp4028_ge_b850v3_fw_probe(struct i2c_client > *stdp4028_i2c, > > /* drm bridge initialization */ > ge_b850v3_lvds_ptr->bridge.funcs = _b850v3_lvds_funcs; > + ge_b850v3_lvds_ptr->bridge.ops = DRM_BRIDGE_OP_DETECT; > ge_b850v3_lvds_ptr->bridge.of_node = dev->of_node; > drm_bridge_add(_b850v3_lvds_ptr->bridge); > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 13/21] drm/bridge: megachips: add helper to create connector
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:09PM +0200, Sam Ravnborg wrote: > Factor out connector creation to a small helper function. > > Signed-off-by: Sam Ravnborg > Cc: Peter Senna Tschudin > Cc: Martin Donnelly > Cc: Martyn Welch > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > .../bridge/megachips-stdp-ge-b850v3-fw.c | 47 +++ > 1 file changed, 27 insertions(+), 20 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > index 6200f12a37e6..258e0525cdcc 100644 > --- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > +++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > @@ -191,6 +191,32 @@ static const struct drm_connector_funcs > ge_b850v3_lvds_connector_funcs = { > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > }; > > +static int ge_b850v3_lvds_create_connector(struct drm_bridge *bridge) > +{ > + struct drm_connector *connector = _b850v3_lvds_ptr->connector; Wow, storing device state in a global variable... :-( How did this go past review ? Reviewed-by: Laurent Pinchart > + int ret; > + > + if (!bridge->encoder) { > + DRM_ERROR("Parent encoder object not found"); > + return -ENODEV; > + } > + > + connector->polled = DRM_CONNECTOR_POLL_HPD; > + > + drm_connector_helper_add(connector, > + _b850v3_lvds_connector_helper_funcs); > + > + ret = drm_connector_init(bridge->dev, connector, > + _b850v3_lvds_connector_funcs, > + DRM_MODE_CONNECTOR_DisplayPort); > + if (ret) { > + DRM_ERROR("Failed to initialize connector with drm\n"); > + return ret; > + } > + > + return drm_connector_attach_encoder(connector, bridge->encoder); > +} > + > static irqreturn_t ge_b850v3_lvds_irq_handler(int irq, void *dev_id) > { > struct i2c_client *stdp4028_i2c > @@ -209,7 +235,6 @@ static irqreturn_t ge_b850v3_lvds_irq_handler(int irq, > void *dev_id) > static int ge_b850v3_lvds_attach(struct drm_bridge *bridge, >enum drm_bridge_attach_flags flags) > { > - struct drm_connector *connector = _b850v3_lvds_ptr->connector; > struct i2c_client *stdp4028_i2c > = ge_b850v3_lvds_ptr->stdp4028_i2c; > int ret; > @@ -219,25 +244,7 @@ static int ge_b850v3_lvds_attach(struct drm_bridge > *bridge, > return -EINVAL; > } > > - if (!bridge->encoder) { > - DRM_ERROR("Parent encoder object not found"); > - return -ENODEV; > - } > - > - connector->polled = DRM_CONNECTOR_POLL_HPD; > - > - drm_connector_helper_add(connector, > - _b850v3_lvds_connector_helper_funcs); > - > - ret = drm_connector_init(bridge->dev, connector, > - _b850v3_lvds_connector_funcs, > - DRM_MODE_CONNECTOR_DisplayPort); > - if (ret) { > - DRM_ERROR("Failed to initialize connector with drm\n"); > - return ret; > - } > - > - ret = drm_connector_attach_encoder(connector, bridge->encoder); > + ret = ge_b850v3_lvds_create_connector(bridge); > if (ret) > return ret; > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 14/21] drm/bridge: megachips: get drm_device from bridge
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:10PM +0200, Sam Ravnborg wrote: > Fix so drm_device is read from the bridge. > This is a preparation for the connector being optional. > > Signed-off-by: Sam Ravnborg > Cc: Peter Senna Tschudin > Cc: Martin Donnelly > Cc: Martyn Welch > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec Reviewed-by: Laurent Pinchart > --- > drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > index 258e0525cdcc..cf1dfbc88acf 100644 > --- a/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > +++ b/drivers/gpu/drm/bridge/megachips-stdp-ge-b850v3-fw.c > @@ -226,8 +226,8 @@ static irqreturn_t ge_b850v3_lvds_irq_handler(int irq, > void *dev_id) > STDP4028_DPTX_IRQ_STS_REG, > STDP4028_DPTX_IRQ_CLEAR); > > - if (ge_b850v3_lvds_ptr->connector.dev) > - drm_kms_helper_hotplug_event(ge_b850v3_lvds_ptr->connector.dev); > + if (ge_b850v3_lvds_ptr->bridge.dev) > + drm_kms_helper_hotplug_event(ge_b850v3_lvds_ptr->bridge.dev); > > return IRQ_HANDLED; > } -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 12/21] drm/bridge: parade-ps8622: make connector creation optional
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:08PM +0200, Sam Ravnborg wrote: > Make the connector creation optional to enable usage of the > parade-ps8622 bridge with the DRM bridge connector helper. > > This change moves drm_helper_hpd_irq_event() call in the attach > function up before the connector creation. No it doesn't :-) And I wonder why that call is actually needed. > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/parade-ps8622.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/parade-ps8622.c > b/drivers/gpu/drm/bridge/parade-ps8622.c > index 6ab6f60b9091..54aa5270d2c9 100644 > --- a/drivers/gpu/drm/bridge/parade-ps8622.c > +++ b/drivers/gpu/drm/bridge/parade-ps8622.c > @@ -459,10 +459,8 @@ static int ps8622_attach(struct drm_bridge *bridge, > ret = drm_bridge_attach(ps8622->bridge.encoder, ps8622->panel_bridge, > >bridge, flags); > > - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { > - DRM_ERROR("Fix bridge driver to make connector optional!"); > - return -EINVAL; > - } > + if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) > + return 0; > > if (!bridge->encoder) { > DRM_ERROR("Parent encoder object not found"); > @@ -482,7 +480,7 @@ static int ps8622_attach(struct drm_bridge *bridge, > drm_connector_attach_encoder(>connector, > bridge->encoder); > > - drm_helper_hpd_irq_event(ps8622->connector.dev); > + drm_helper_hpd_irq_event(ps8622->bridge.dev); > > return ret; > } -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v4 4/7] drm: msm: a6xx: use dev_pm_opp_set_bw to scale DDR
On Fri, Jul 10, 2020 at 2:03 PM Akhil P Oommen wrote: > > > On 7/11/2020 1:11 AM, Rob Clark wrote: > > On Thu, Jul 9, 2020 at 1:01 PM Akhil P Oommen > > wrote: > >> From: Sharat Masetty > >> > >> This patches replaces the previously used static DDR vote and uses > >> dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling > >> GPU frequency. Also since the icc path voting is handled completely > >> in the opp driver, remove the icc_path handle and its usage in the > >> drm driver. > >> > >> Signed-off-by: Sharat Masetty > >> Signed-off-by: Akhil P Oommen > >> --- > >> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 25 + > >> 1 file changed, 17 insertions(+), 8 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >> b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >> index b547339..6fbfd7d 100644 > >> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > >> @@ -123,7 +123,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct > >> dev_pm_opp *opp) > >> > >> if (!gmu->legacy) { > >> a6xx_hfi_set_freq(gmu, gmu->current_perf_index); > >> - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > >> + dev_pm_opp_set_bw(>pdev->dev, opp); > > What is the status of the patch to add dev_pm_opp_set_bw()? If it is > > ready to go, and I get an ack-by from the OPP maintainer, I suppose I > > could merge it via drm/msm. > > > > Otherwise should we consider pulling in a private copy of it into > > drm/msm (and then drop it to use the helper in, hopefully, the next > > cycle)? > > > > I'm pulling the patches preceding this one into msm-next-staging to do > > some testing. And the dt patches following this one would normally > > get merged via Bjorn. At the moment, I'm not sure what to do with > > this one. > > > > BR, > > -R > I see Sibi's patch is already picked in opp/linux-next branch. > https://kernel.googlesource.com/pub/scm/linux/kernel/git/vireshk/pm/+/b466542f331e221a3628c1cfe5ccff307d7d787f > ok, I guess we can try and do a 2nd late pull-req for msm-next, after the opp pull-req lands.. BR, -R > > Thanks, > -Akhil > > >> return; > >> } > >> > >> @@ -149,11 +149,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct > >> dev_pm_opp *opp) > >> if (ret) > >> dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", > >> ret); > >> > >> - /* > >> -* Eventually we will want to scale the path vote with the > >> frequency but > >> -* for now leave it at max so that the performance is nominal. > >> -*/ > >> - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > >> + dev_pm_opp_set_bw(>pdev->dev, opp); > >> } > >> > >> unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu) > >> @@ -840,6 +836,19 @@ static void a6xx_gmu_set_initial_freq(struct msm_gpu > >> *gpu, struct a6xx_gmu *gmu) > >> dev_pm_opp_put(gpu_opp); > >> } > >> > >> +static void a6xx_gmu_set_initial_bw(struct msm_gpu *gpu, struct a6xx_gmu > >> *gmu) > >> +{ > >> + struct dev_pm_opp *gpu_opp; > >> + unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index]; > >> + > >> + gpu_opp = dev_pm_opp_find_freq_exact(>pdev->dev, gpu_freq, > >> true); > >> + if (IS_ERR_OR_NULL(gpu_opp)) > >> + return; > >> + > >> + dev_pm_opp_set_bw(>pdev->dev, gpu_opp); > >> + dev_pm_opp_put(gpu_opp); > >> +} > >> + > >> int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > >> { > >> struct adreno_gpu *adreno_gpu = _gpu->base; > >> @@ -864,7 +873,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > >> } > >> > >> /* Set the bus quota to a reasonable value for boot */ > >> - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(3072)); > >> + a6xx_gmu_set_initial_bw(gpu, gmu); > >> > >> /* Enable the GMU interrupt */ > >> gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_CLR, ~0); > >> @@ -1040,7 +1049,7 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) > >> a6xx_gmu_shutdown(gmu); > >> > >> /* Remove the bus vote */ > >> - icc_set_bw(gpu->icc_path, 0, 0); > >> + dev_pm_opp_set_bw(>pdev->dev, NULL); > >> > >> /* > >> * Make sure the GX domain is off before turning off the GMU (CX) > >> -- > >> 2.7.4 > >> > >> ___ > >> Freedreno mailing list > >> freedr...@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/freedreno ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 11/21] drm/bridge: parade-ps8622: add drm_panel_bridge support
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:07PM +0200, Sam Ravnborg wrote: > Prepare the bridge driver for use in a chained setup by > replacing direct use of drm_panel with drm_panel_bridge support. > > Note: the bridge panel will use the connector type from the panel. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/parade-ps8622.c | 46 -- > 1 file changed, 13 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/parade-ps8622.c > b/drivers/gpu/drm/bridge/parade-ps8622.c > index d789ea2a7fb9..6ab6f60b9091 100644 > --- a/drivers/gpu/drm/bridge/parade-ps8622.c > +++ b/drivers/gpu/drm/bridge/parade-ps8622.c > @@ -45,7 +45,7 @@ struct ps8622_bridge { > struct drm_connector connector; > struct i2c_client *client; > struct drm_bridge bridge; > - struct drm_panel *panel; > + struct drm_bridge *panel_bridge; > struct regulator *v12; > struct backlight_device *bl; > > @@ -365,11 +365,6 @@ static void ps8622_pre_enable(struct drm_bridge *bridge) > DRM_ERROR("fails to enable ps8622->v12"); > } > > - if (drm_panel_prepare(ps8622->panel)) { > - DRM_ERROR("failed to prepare panel\n"); > - return; > - } > - > gpiod_set_value(ps8622->gpio_slp, 1); > > /* > @@ -399,24 +394,8 @@ static void ps8622_pre_enable(struct drm_bridge *bridge) > ps8622->enabled = true; > } > > -static void ps8622_enable(struct drm_bridge *bridge) > -{ > - struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge); > - > - if (drm_panel_enable(ps8622->panel)) { > - DRM_ERROR("failed to enable panel\n"); > - return; > - } > -} > - > static void ps8622_disable(struct drm_bridge *bridge) > { > - struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge); > - > - if (drm_panel_disable(ps8622->panel)) { > - DRM_ERROR("failed to disable panel\n"); > - return; > - } > msleep(PS8622_PWMO_END_T12_MS); What's the point in just waiting in the disable path ? > } > > @@ -436,11 +415,6 @@ static void ps8622_post_disable(struct drm_bridge > *bridge) >*/ > gpiod_set_value(ps8622->gpio_slp, 0); > > - if (drm_panel_unprepare(ps8622->panel)) { > - DRM_ERROR("failed to unprepare panel\n"); > - return; > - } > - > if (ps8622->v12) > regulator_disable(ps8622->v12); > > @@ -461,7 +435,7 @@ static int ps8622_get_modes(struct drm_connector > *connector) > > ps8622 = connector_to_ps8622(connector); > > - return drm_panel_get_modes(ps8622->panel, connector); > + return drm_bridge_get_modes(ps8622->panel_bridge, connector); > } > > static const struct drm_connector_helper_funcs ps8622_connector_helper_funcs > = { > @@ -482,6 +456,9 @@ static int ps8622_attach(struct drm_bridge *bridge, > struct ps8622_bridge *ps8622 = bridge_to_ps8622(bridge); > int ret; > > + ret = drm_bridge_attach(ps8622->bridge.encoder, ps8622->panel_bridge, > + >bridge, flags); Same comment as earlier in this series about double connector creation. Don't you need to check ret here ? > + > if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { > DRM_ERROR("Fix bridge driver to make connector optional!"); > return -EINVAL; > @@ -505,9 +482,6 @@ static int ps8622_attach(struct drm_bridge *bridge, > drm_connector_attach_encoder(>connector, > bridge->encoder); > > - if (ps8622->panel) > - drm_panel_attach(ps8622->panel, >connector); > - > drm_helper_hpd_irq_event(ps8622->connector.dev); > > return ret; > @@ -515,7 +489,6 @@ static int ps8622_attach(struct drm_bridge *bridge, > > static const struct drm_bridge_funcs ps8622_bridge_funcs = { > .pre_enable = ps8622_pre_enable, > - .enable = ps8622_enable, > .disable = ps8622_disable, > .post_disable = ps8622_post_disable, > .attach = ps8622_attach, > @@ -533,16 +506,23 @@ static int ps8622_probe(struct i2c_client *client, > { > struct device *dev = >dev; > struct ps8622_bridge *ps8622; > + struct drm_bridge *pbridge; > + struct drm_panel *panel; > int ret; > > ps8622 = devm_kzalloc(dev, sizeof(*ps8622), GFP_KERNEL); > if (!ps8622) > return -ENOMEM; > > - ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, >panel, > NULL); > + ret = drm_of_find_panel_or_bridge(dev->of_node, 0, 0, , NULL); > if (ret) > return ret; > > + pbridge = devm_drm_panel_bridge_add(dev, panel); > + if (IS_ERR(pbridge)) > + return PTR_ERR(pbridge); > + > + ps8622->panel_bridge = pbridge; >
Re: [PATCH v3 10/21] drm/bridge: ti-tpd12s015: make connector creation optional
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:06PM +0200, Sam Ravnborg wrote: > The ti-tpd12s015 do not create any connector, so ignore > the flags argument, just pass it on to the next bridge > in the chain. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/ti-tpd12s015.c | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c > b/drivers/gpu/drm/bridge/ti-tpd12s015.c > index 514cbf0eac75..4f1666422ab2 100644 > --- a/drivers/gpu/drm/bridge/ti-tpd12s015.c > +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c > @@ -43,9 +43,6 @@ static int tpd12s015_attach(struct drm_bridge *bridge, > struct tpd12s015_device *tpd = to_tpd12s015(bridge); > int ret; > > - if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) > - return -EINVAL; > - The driver is only used by devices that use DRM_BRIDGE_ATTACH_NO_CONNECTOR. I'd rather keep this check and port other potential users to DRM_BRIDGE_ATTACH_NO_CONNECTOR instead of allowing operation in !DRM_BRIDGE_ATTACH_NO_CONNECTOR mode. > ret = drm_bridge_attach(bridge->encoder, tpd->next_bridge, > bridge, flags); > if (ret < 0) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 08/21] drm/bridge: tc358767: add get_edid bride operation
Hi Sam, Thank you for the patch. s/bride/bridge/ in the subject line. On Fri, Jul 03, 2020 at 09:24:04PM +0200, Sam Ravnborg wrote: > Prepare for chained bridge with the addition of > get_edid support. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/tc358767.c | 24 +--- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > b/drivers/gpu/drm/bridge/tc358767.c > index 85973ae728db..fb9d57967b2c 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -1317,6 +1317,20 @@ static void tc_bridge_mode_set(struct drm_bridge > *bridge, > tc->mode = *mode; > } > > +static struct edid *tc_get_edid(struct drm_bridge *bridge, > + struct drm_connector *connector) > +{ > + struct tc_data *tc = bridge_to_tc(bridge); > + struct edid *edid; > + > + edid = drm_get_edid(connector, >aux.ddc); > + > + kfree(tc->edid); > + tc->edid = edid; The caller (drm_bridge_connector_get_modes_edid()) calls kfree(edid), so if you want to store it internally, you'll have to make a copy. Can you skip internal storage altogether by freeing the memory in tc_connector_get_modes() ? > + > + return edid; > +} > + > static int tc_connector_get_modes(struct drm_connector *connector) > { > struct tc_data *tc = connector_to_tc(connector); > @@ -1336,12 +1350,7 @@ static int tc_connector_get_modes(struct drm_connector > *connector) > return count; > } > > - edid = drm_get_edid(connector, >aux.ddc); > - > - kfree(tc->edid); > - tc->edid = edid; > - if (!edid) > - return 0; > + edid = tc_get_edid(>bridge, connector); > > drm_connector_update_edid_property(connector, edid); > count = drm_add_edid_modes(connector, edid); > @@ -1452,6 +1461,7 @@ static const struct drm_bridge_funcs tc_bridge_funcs = { > .disable = tc_bridge_disable, > .mode_fixup = tc_bridge_mode_fixup, > .detect = tc_bridge_detect, > + .get_edid = tc_get_edid, > }; > > static bool tc_readable_reg(struct device *dev, unsigned int reg) > @@ -1685,7 +1695,7 @@ static int tc_probe(struct i2c_client *client, const > struct i2c_device_id *id) > return ret; > > tc->bridge.funcs = _bridge_funcs; > - tc->bridge.ops = DRM_BRIDGE_OP_DETECT; > + tc->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID; > > tc->bridge.of_node = dev->of_node; > drm_bridge_add(>bridge); > -- > 2.25.1 > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 09/21] drm/bridge: tc358767: make connector creation optional
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:05PM +0200, Sam Ravnborg wrote: > Display drivers are in the new model expected to create > the connector using drm_bridge_connector_init(). > Allow users of this bridge driver to support the new > model by introducing support for optional connector creation. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/tc358767.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > b/drivers/gpu/drm/bridge/tc358767.c > index fb9d57967b2c..2eb00d39f619 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -1421,8 +1421,7 @@ static int tc_bridge_attach(struct drm_bridge *bridge, > } > > if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { > - DRM_ERROR("Fix bridge driver to make connector optional!"); > - return -EINVAL; > + return 0; > } I'd drop the braces too. Reviewed-by: Laurent Pinchart > > /* Create DP/eDP connector */ -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 2/2] drm/i915/mst: filter out the display mode exceed sink's capability
On Fri, Jul 10, 2020 at 03:31:44PM -0400, Lyude Paul wrote: > From: Lee Shawn C > > So far, max dot clock rate for MST mode rely on physcial > bandwidth limitation. It would caused compatibility issue > if source display resolution exceed MST hub output ability. > > For example, source DUT had DP 1.2 output capability. > And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 > monitor connected. Source would retrieve EDID from external > and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps > because it did not surpass DP physical bandwidth limitation. > Do modeset to 4k@60fps, source output display data but MST > docking can't output HDMI properly due to this resolution > already over HDMI 1.4 spec. > > Refer to commit ("drm/dp_mst: Use full_pbn > instead of available_pbn for bandwidth checks"). > Source driver should refer to full_pbn to evaluate sink > output capability. And filter out the resolution surpass > sink output limitation. > > Changes since v1: > * Using mgr->base.lock to protect full_pbn. > Changes since v2: > * Add ctx lock. > Changes since v3: > * s/intel_dp_mst_mode_clock_exceed_pbn_bandwidth/ > intel_dp_mst_mode_clock_exceeds_pbn_bw/ > * Use the new drm_connector_helper_funcs.mode_valid_ctx to properly pipe > down the drm_modeset_acquire_ctx that the probe helpers are using, so > we can safely grab >base.lock without deadlocking > Changes since v4: > * Move drm_dp_calc_pbn_mode(mode->clock, bpp, false) > port->full_pbn > check > * Fix the bpp we use in drm_dp_calc_pbn_mode() > * Drop leftover (!mgr) check > * Don't check for if full_pbn is unset. To be clear - it _can_ be unset, > but if it is then it's certainly a bug in DRM or a non-compliant sink > as full_pbn should always be populated by the time we call > ->mode_valid_ctx. > We should workaround non-compliant sinks with full_pbn=0, but that > should happen in the DP MST helpers so we can estimate the full_pbn > value as best we can. I guess when the connector gets unplugged full_pbn would also get reset but pruning all modes in that case is ok. > > Cc: Manasi Navare > Cc: Jani Nikula > Cc: Ville Syrjala > Cc: Cooper Chiou > Co-developed-by: Lyude Paul > Signed-off-by: Lee Shawn C > Signed-off-by: Lyude Paul Reviewed-and-tested-by: Imre Deak > --- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 55 ++--- > 1 file changed, 38 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index bdc19b04b2c10..a2d91a4997001 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c > @@ -639,39 +639,60 @@ static int intel_dp_mst_get_modes(struct drm_connector > *connector) > return intel_dp_mst_get_ddc_modes(connector); > } > > -static enum drm_mode_status > -intel_dp_mst_mode_valid(struct drm_connector *connector, > - struct drm_display_mode *mode) > +static int > +intel_dp_mst_mode_valid_ctx(struct drm_connector *connector, > + struct drm_display_mode *mode, > + struct drm_modeset_acquire_ctx *ctx, > + enum drm_mode_status *status) > { > struct drm_i915_private *dev_priv = to_i915(connector->dev); > struct intel_connector *intel_connector = to_intel_connector(connector); > struct intel_dp *intel_dp = intel_connector->mst_port; > + struct drm_dp_mst_topology_mgr *mgr = _dp->mst_mgr; > + struct drm_dp_mst_port *port = intel_connector->port; > + const int min_bpp = 18; > int max_dotclk = to_i915(connector->dev)->max_dotclk_freq; > int max_rate, mode_rate, max_lanes, max_link_clock; > + int ret; > > - if (drm_connector_is_unregistered(connector)) > - return MODE_ERROR; > + if (drm_connector_is_unregistered(connector)) { > + *status = MODE_ERROR; > + return 0; > + } > > - if (mode->flags & DRM_MODE_FLAG_DBLSCAN) > - return MODE_NO_DBLESCAN; > + if (mode->flags & DRM_MODE_FLAG_DBLSCAN) { > + *status = MODE_NO_DBLESCAN; > + return 0; > + } > > max_link_clock = intel_dp_max_link_rate(intel_dp); > max_lanes = intel_dp_max_lane_count(intel_dp); > > max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); > - mode_rate = intel_dp_link_required(mode->clock, 18); > + mode_rate = intel_dp_link_required(mode->clock, min_bpp); > > - /* TODO - validate mode against available PBN for link */ > - if (mode->clock < 1) > - return MODE_CLOCK_LOW; > + ret = drm_modeset_lock(>base.lock, ctx); > + if (ret) > + return ret; > > - if (mode->flags & DRM_MODE_FLAG_DBLCLK) > - return MODE_H_ILLEGAL; > + if (mode_rate > max_rate || mode->clock > max_dotclk || > + drm_dp_calc_pbn_mode(mode->clock, min_bpp, false) >
Re: [PATCH v3 07/21] drm/bridge: tc358767: add detect bridge operation
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:03PM +0200, Sam Ravnborg wrote: > Prepare the bridge driver for chained operation by adding > support for the detect operation. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/tc358767.c | 16 +--- > 1 file changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > b/drivers/gpu/drm/bridge/tc358767.c > index 08d483664258..85973ae728db 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -1353,10 +1353,9 @@ static const struct drm_connector_helper_funcs > tc_connector_helper_funcs = { > .get_modes = tc_connector_get_modes, > }; > > -static enum drm_connector_status tc_connector_detect(struct drm_connector > *connector, > - bool force) > +static enum drm_connector_status tc_bridge_detect(struct drm_bridge *bridge) > { > - struct tc_data *tc = connector_to_tc(connector); > + struct tc_data *tc = bridge_to_tc(bridge); > bool conn; > u32 val; > int ret; > @@ -1380,6 +1379,14 @@ static enum drm_connector_status > tc_connector_detect(struct drm_connector *conne > return connector_status_disconnected; > } > > +static enum drm_connector_status > +tc_connector_detect(struct drm_connector *connector, bool force) > +{ > + struct tc_data *tc = connector_to_tc(connector); > + > + return tc_bridge_detect(>bridge); > +} > + > static const struct drm_connector_funcs tc_connector_funcs = { > .detect = tc_connector_detect, > .fill_modes = drm_helper_probe_single_connector_modes, > @@ -1444,6 +1451,7 @@ static const struct drm_bridge_funcs tc_bridge_funcs = { > .enable = tc_bridge_enable, > .disable = tc_bridge_disable, > .mode_fixup = tc_bridge_mode_fixup, > + .detect = tc_bridge_detect, > }; > > static bool tc_readable_reg(struct device *dev, unsigned int reg) > @@ -1677,6 +1685,8 @@ static int tc_probe(struct i2c_client *client, const > struct i2c_device_id *id) > return ret; > > tc->bridge.funcs = _bridge_funcs; > + tc->bridge.ops = DRM_BRIDGE_OP_DETECT; I think you should set DRM_BRIDGE_OP_DETECT only when tc->hpd_pin is valid. I would also move the if (tc->hpd_pin < 0) case from tc_bridge_detect to +tc_connector_detect. > + > tc->bridge.of_node = dev->of_node; > drm_bridge_add(>bridge); > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 06/21] drm/bridge: tc358767: add drm_panel_bridge support
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:24:02PM +0200, Sam Ravnborg wrote: > Prepare the bridge driver for use in a chained setup by > replacing direct use of drm_panel with drm_panel_bridge support. > > The bridge driver assume the panel is optional. > The relevant tests are migrated over to check for the > pnale bridge to keep the same functionality. s/pnale/panel/ > Note: the bridge panel will use the connector type from the panel. > > Signed-off-by: Sam Ravnborg > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > --- > drivers/gpu/drm/bridge/tc358767.c | 57 +++ > 1 file changed, 27 insertions(+), 30 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c > b/drivers/gpu/drm/bridge/tc358767.c > index c2777b226c75..08d483664258 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -244,8 +244,8 @@ struct tc_data { > struct drm_dp_aux aux; > > struct drm_bridge bridge; > + struct drm_bridge *panel_bridge; > struct drm_connectorconnector; > - struct drm_panel*panel; > > /* link settings */ > struct tc_edp_link link; > @@ -1236,13 +1236,6 @@ static int tc_stream_disable(struct tc_data *tc) > return 0; > } > > -static void tc_bridge_pre_enable(struct drm_bridge *bridge) > -{ > - struct tc_data *tc = bridge_to_tc(bridge); > - > - drm_panel_prepare(tc->panel); > -} > - > static void tc_bridge_enable(struct drm_bridge *bridge) > { > struct tc_data *tc = bridge_to_tc(bridge); > @@ -1266,8 +1259,6 @@ static void tc_bridge_enable(struct drm_bridge *bridge) > tc_main_link_disable(tc); > return; > } > - > - drm_panel_enable(tc->panel); > } > > static void tc_bridge_disable(struct drm_bridge *bridge) > @@ -1275,8 +1266,6 @@ static void tc_bridge_disable(struct drm_bridge *bridge) > struct tc_data *tc = bridge_to_tc(bridge); > int ret; > > - drm_panel_disable(tc->panel); > - > ret = tc_stream_disable(tc); > if (ret < 0) > dev_err(tc->dev, "main link stream stop error: %d\n", ret); > @@ -1286,13 +1275,6 @@ static void tc_bridge_disable(struct drm_bridge > *bridge) > dev_err(tc->dev, "main link disable error: %d\n", ret); > } > > -static void tc_bridge_post_disable(struct drm_bridge *bridge) > -{ > - struct tc_data *tc = bridge_to_tc(bridge); > - > - drm_panel_unprepare(tc->panel); > -} > - > static bool tc_bridge_mode_fixup(struct drm_bridge *bridge, >const struct drm_display_mode *mode, >struct drm_display_mode *adj) > @@ -1348,9 +1330,11 @@ static int tc_connector_get_modes(struct drm_connector > *connector) > return 0; > } > > - count = drm_panel_get_modes(tc->panel, connector); > - if (count > 0) > - return count; > + if (tc->panel_bridge) { > + count = drm_bridge_get_modes(tc->panel_bridge, connector); > + if (count > 0) > + return count; > + } > > edid = drm_get_edid(connector, >aux.ddc); > > @@ -1378,7 +1362,7 @@ static enum drm_connector_status > tc_connector_detect(struct drm_connector *conne > int ret; > > if (tc->hpd_pin < 0) { > - if (tc->panel) > + if (tc->panel_bridge) > return connector_status_connected; > else > return connector_status_unknown; > @@ -1413,6 +1397,13 @@ static int tc_bridge_attach(struct drm_bridge *bridge, > struct drm_device *drm = bridge->dev; > int ret; > > + if (tc->panel_bridge) { > + ret = drm_bridge_attach(tc->bridge.encoder, tc->panel_bridge, > + >bridge, flags); > + if (ret < 0) > + return ret; > + } With this both this driver and the panel bridge driver will create a connector. The simplest way to handle that is probably to pass flags & ~DRM_BRIDGE_ATTACH_NO_CONNECTOR to drm_bridge_attach(). It's a bit of a hack, but should go away once all users are converted to !DRM_BRIDGE_ATTACH_NO_CONNECTOR. Reviewed-by: Laurent Pinchart > + > if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) { > DRM_ERROR("Fix bridge driver to make connector optional!"); > return -EINVAL; > @@ -1421,7 +1412,7 @@ static int tc_bridge_attach(struct drm_bridge *bridge, > /* Create DP/eDP connector */ > drm_connector_helper_add(>connector, _connector_helper_funcs); > ret = drm_connector_init(drm, >connector, _connector_funcs, > - tc->panel ? DRM_MODE_CONNECTOR_eDP : > + tc->panel_bridge ? DRM_MODE_CONNECTOR_eDP : >
Re: [Intel-gfx] [PATCH v3 1/2] drm/probe_helper: Add drm_connector_helper_funcs.mode_valid_ctx
On Fri, Jul 10, 2020 at 03:31:43PM -0400, Lyude Paul wrote: > This is just an atomic version of mode_valid, which is intended to be > used for situations where a driver might need to check the atomic state > of objects other than the connector itself. One such example is with > MST, where the maximum possible bandwidth on a connector can change > dynamically irregardless of the display configuration. > > Changes since v1: > * Use new drm logging functions > * Make some corrections in the mode_valid_ctx kdoc > * Return error codes or 0 from ->mode_valid_ctx() on fail, and store the > drm_mode_status in an additional function parameter > Changes since v2: > * Don't accidentally assign ret to mode->status on success, or we'll > squash legitimate mode validation results > * Don't forget to assign MODE_OK to status in drm_connector_mode_valid() > if we have no callbacks > * Drop leftover hunk in drm_modes.h around enum drm_mode_status > > Signed-off-by: Lyude Paul > Cc: Lee Shawn C Some nits below, but looks good in either way: Reviewed-and-tested-by: Imre Deak > --- > drivers/gpu/drm/drm_crtc_helper_internal.h | 7 +- > drivers/gpu/drm/drm_probe_helper.c | 94 ++ > include/drm/drm_modeset_helper_vtables.h | 42 ++ > 3 files changed, 109 insertions(+), 34 deletions(-) > > diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h > b/drivers/gpu/drm/drm_crtc_helper_internal.h > index f0a66ef47e5ad..25ce42e799952 100644 > --- a/drivers/gpu/drm/drm_crtc_helper_internal.h > +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h > @@ -73,8 +73,11 @@ enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc > *crtc, >const struct drm_display_mode *mode); > enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder, > const struct drm_display_mode > *mode); > -enum drm_mode_status drm_connector_mode_valid(struct drm_connector > *connector, > - struct drm_display_mode *mode); > +int > +drm_connector_mode_valid(struct drm_connector *connector, > + struct drm_display_mode *mode, > + struct drm_modeset_acquire_ctx *ctx, > + enum drm_mode_status *status); > > struct drm_encoder * > drm_connector_get_single_encoder(struct drm_connector *connector); > diff --git a/drivers/gpu/drm/drm_probe_helper.c > b/drivers/gpu/drm/drm_probe_helper.c > index e0ed58d291ed9..f7bd1d35aa805 100644 > --- a/drivers/gpu/drm/drm_probe_helper.c > +++ b/drivers/gpu/drm/drm_probe_helper.c > @@ -86,17 +86,19 @@ drm_mode_validate_flag(const struct drm_display_mode > *mode, > return MODE_OK; > } > > -static enum drm_mode_status > +static int > drm_mode_validate_pipeline(struct drm_display_mode *mode, > - struct drm_connector *connector) > +struct drm_connector *connector, > +struct drm_modeset_acquire_ctx *ctx, > +enum drm_mode_status *status) > { > struct drm_device *dev = connector->dev; > - enum drm_mode_status ret = MODE_OK; > struct drm_encoder *encoder; > + int ret; > > /* Step 1: Validate against connector */ > - ret = drm_connector_mode_valid(connector, mode); > - if (ret != MODE_OK) > + ret = drm_connector_mode_valid(connector, mode, ctx, status); > + if (ret || *status != MODE_OK) > return ret; After this point ret is guaranteed to stay 0, so would be clearer to just s/return ret/return 0/ later in this func. > > /* Step 2: Validate against encoders and crtcs */ > @@ -104,8 +106,8 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, > struct drm_bridge *bridge; > struct drm_crtc *crtc; > > - ret = drm_encoder_mode_valid(encoder, mode); > - if (ret != MODE_OK) { > + *status = drm_encoder_mode_valid(encoder, mode); > + if (*status != MODE_OK) { > /* No point in continuing for crtc check as this encoder >* will not accept the mode anyway. If all encoders >* reject the mode then, at exit, ret will not be > @@ -114,10 +116,10 @@ drm_mode_validate_pipeline(struct drm_display_mode > *mode, > } > > bridge = drm_bridge_chain_get_first_bridge(encoder); > - ret = drm_bridge_chain_mode_valid(bridge, > - >display_info, > - mode); > - if (ret != MODE_OK) { > + *status = drm_bridge_chain_mode_valid(bridge, > + >display_info, > + mode); > + if (*status != MODE_OK) { > /* There is also no
Re: [PATCH v3 02/21] drm/panel: panel-simple: add default connector_type
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:23:58PM +0200, Sam Ravnborg wrote: > All panels shall report a connector type. > panel-simple has a lot of panels with no connector_type, > and for these fall back to DPI as the default. > > Signed-off-by: Sam Ravnborg > Cc: Thierry Reding > Cc: Sam Ravnborg > --- > drivers/gpu/drm/panel/panel-simple.c | 10 -- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > b/drivers/gpu/drm/panel/panel-simple.c > index 7b5d212215e0..b3ec965721b0 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -500,6 +500,7 @@ static int panel_simple_probe(struct device *dev, const > struct panel_desc *desc) > struct panel_simple *panel; > struct display_timing dt; > struct device_node *ddc; > + int connector_type; > int err; > > panel = devm_kzalloc(dev, sizeof(*panel), GFP_KERNEL); > @@ -566,8 +567,13 @@ static int panel_simple_probe(struct device *dev, const > struct panel_desc *desc) > desc->bpc != 8); > } > > - drm_panel_init(>base, dev, _simple_funcs, > -desc->connector_type); > + /* Default DRM_MODE_CONNECTOR_DPI if no connector_type is set */ > + if (desc->connector_type != 0) > + connector_type = desc->connector_type; > + else > + connector_type = DRM_MODE_CONNECTOR_DPI; This avoids a WARN_ON(), which is good, but should we add a dev_warn() to get this fixed ? > + > + drm_panel_init(>base, dev, _simple_funcs, connector_type); > > err = drm_panel_of_backlight(>base); > if (err) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 01/21] drm/panel: add connector type to boe,hv070wsa-100 panel
Hi Sam, Thank you for the patch. On Fri, Jul 03, 2020 at 09:23:57PM +0200, Sam Ravnborg wrote: > The boe,hv070wsa-100 panel is a LVDS panel. > Fix connector type to reflect this. > > With this change users of this panel do not have to specify the > connector type. > > v3: > - Drop PIXDATA bus_flag, not relevant > > v2: > - Add .bus_format (Laurent) > - Add .bus_flags > > Signed-off-by: Sam Ravnborg > Cc: Laurent Pinchart > Cc: Thierry Reding > Cc: Sam Ravnborg > --- > drivers/gpu/drm/panel/panel-simple.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > b/drivers/gpu/drm/panel/panel-simple.c > index 4f0ec5e5b0aa..7b5d212215e0 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -1188,6 +1188,9 @@ static const struct panel_desc boe_hv070wsa = { > .width = 154, > .height = 90, > }, > + .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG, > + .bus_flags = DRM_BUS_FLAG_DE_HIGH, > + .connector_type = DRM_MODE_CONNECTOR_LVDS, While at it, should you set .bpc to 8 ? Reviewed-by: Laurent Pinchart > }; > > static const struct drm_display_mode boe_nv101wxmn51_modes[] = { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 3/7] drm: msm: a6xx: set gpu freq through hfi
On 7/10/2020 1:34 AM, Jonathan Marek wrote: On 7/9/20 4:00 PM, Akhil P Oommen wrote: Newer targets support changing gpu frequency through HFI. So use that wherever supported instead of the legacy method. It was already using HFI on newer targets. Don't break it in one commit then fix it in the next. Oops. I somehow got confused. Will fix and resend. -Akhil Signed-off-by: Akhil P Oommen --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 233afea..b547339 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -121,6 +121,12 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp) if (gpu_freq == gmu->gpu_freqs[perf_index]) break; + if (!gmu->legacy) { + a6xx_hfi_set_freq(gmu, gmu->current_perf_index); + icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); + return; + } + gmu->current_perf_index = perf_index; gmu->freq = gmu->gpu_freqs[perf_index]; @@ -893,10 +899,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) enable_irq(gmu->hfi_irq); /* Set the GPU to the current freq */ - if (gmu->legacy) - a6xx_gmu_set_initial_freq(gpu, gmu); - else - a6xx_hfi_set_freq(gmu, gmu->current_perf_index); + a6xx_gmu_set_initial_freq(gpu, gmu); /* * "enable" the GX power domain which won't actually do anything but it ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/drm_fb_helper: fix fbdev with sparc64
> > Acked-by: Thomas Zimmermann > > > > but I'd like to have someone with more architecture expertise ack this > > as well. > > Agreed. All my testing is using the bochs_drm framebuffer under > qemu-system-sparc64 > (a sun4u machine) so it would be nice to get an ACK from Dave or someone else > who can > vouch for this on real hardware. I'm not sure there exists real hardware that would come down this path, I believe the last sparc64 with a GPU is a mach64, or maybe an r128 card. Otherwise I think using the memcpy io should be fine here on all architectures. Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v4 4/7] drm: msm: a6xx: use dev_pm_opp_set_bw to scale DDR
On 7/11/2020 1:11 AM, Rob Clark wrote: On Thu, Jul 9, 2020 at 1:01 PM Akhil P Oommen wrote: From: Sharat Masetty This patches replaces the previously used static DDR vote and uses dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling GPU frequency. Also since the icc path voting is handled completely in the opp driver, remove the icc_path handle and its usage in the drm driver. Signed-off-by: Sharat Masetty Signed-off-by: Akhil P Oommen --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 25 + 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index b547339..6fbfd7d 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -123,7 +123,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp) if (!gmu->legacy) { a6xx_hfi_set_freq(gmu, gmu->current_perf_index); - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); + dev_pm_opp_set_bw(>pdev->dev, opp); What is the status of the patch to add dev_pm_opp_set_bw()? If it is ready to go, and I get an ack-by from the OPP maintainer, I suppose I could merge it via drm/msm. Otherwise should we consider pulling in a private copy of it into drm/msm (and then drop it to use the helper in, hopefully, the next cycle)? I'm pulling the patches preceding this one into msm-next-staging to do some testing. And the dt patches following this one would normally get merged via Bjorn. At the moment, I'm not sure what to do with this one. BR, -R I see Sibi's patch is already picked in opp/linux-next branch. https://kernel.googlesource.com/pub/scm/linux/kernel/git/vireshk/pm/+/b466542f331e221a3628c1cfe5ccff307d7d787f Thanks, -Akhil return; } @@ -149,11 +149,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp) if (ret) dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret); - /* -* Eventually we will want to scale the path vote with the frequency but -* for now leave it at max so that the performance is nominal. -*/ - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); + dev_pm_opp_set_bw(>pdev->dev, opp); } unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu) @@ -840,6 +836,19 @@ static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu) dev_pm_opp_put(gpu_opp); } +static void a6xx_gmu_set_initial_bw(struct msm_gpu *gpu, struct a6xx_gmu *gmu) +{ + struct dev_pm_opp *gpu_opp; + unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index]; + + gpu_opp = dev_pm_opp_find_freq_exact(>pdev->dev, gpu_freq, true); + if (IS_ERR_OR_NULL(gpu_opp)) + return; + + dev_pm_opp_set_bw(>pdev->dev, gpu_opp); + dev_pm_opp_put(gpu_opp); +} + int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) { struct adreno_gpu *adreno_gpu = _gpu->base; @@ -864,7 +873,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) } /* Set the bus quota to a reasonable value for boot */ - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(3072)); + a6xx_gmu_set_initial_bw(gpu, gmu); /* Enable the GMU interrupt */ gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_CLR, ~0); @@ -1040,7 +1049,7 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) a6xx_gmu_shutdown(gmu); /* Remove the bus vote */ - icc_set_bw(gpu->icc_path, 0, 0); + dev_pm_opp_set_bw(>pdev->dev, NULL); /* * Make sure the GX domain is off before turning off the GMU (CX) -- 2.7.4 ___ Freedreno mailing list freedr...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amdgpu: fix spelling mistake "Falied" -> "Failed"
On Fri, Jul 10, 2020 at 4:38 AM Colin King wrote: > > From: Colin Ian King > > There is a spelling mistake in a DRM_ERROR error message. Fix it. > > Signed-off-by: Colin Ian King Applied. Thanks! Alex > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index e20695b44dbe..40706334f7a8 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -1984,7 +1984,7 @@ static int psp_suspend(void *handle) > > ret = psp_tmr_terminate(psp); > if (ret) { > - DRM_ERROR("Falied to terminate tmr\n"); > + DRM_ERROR("Failed to terminate tmr\n"); > return ret; > } > > -- > 2.27.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] dt-bindings: display: sun8i-mixer: Remove duplicated 'iommus'
Commit 13871279ff5c ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage") introduced a double instance of 'iommus' causing the following build error with 'make dt_binding_check': found duplicate key "iommus" with value "{}" (original value: "{}") in "", line 45, column 3 Remove the double occurrence to fix this problem. Fixes: 13871279ff5c ("arm64: dts: allwinner: h6: Fix Cedrus IOMMU usage") Signed-off-by: Fabio Estevam --- .../bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml | 3 --- 1 file changed, 3 deletions(-) diff --git a/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml b/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml index c2fbf0b06d32..c040eef56518 100644 --- a/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml +++ b/Documentation/devicetree/bindings/display/allwinner,sun8i-a83t-de2-mixer.yaml @@ -42,9 +42,6 @@ properties: resets: maxItems: 1 - iommus: -maxItems: 1 - ports: type: object description: | -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 02/25] dma-fence: prime lockdep annotations
On Fri, Jul 10, 2020 at 4:23 PM Jason Gunthorpe wrote: > > On Fri, Jul 10, 2020 at 04:02:35PM +0200, Daniel Vetter wrote: > > > > dma_fence only possibly makes some sense if you intend to expose the > > > completion outside a single driver. > > > > > > The prefered kernel design pattern for this is to connect things with > > > a function callback. > > > > > > So the actual use case of dma_fence is quite narrow and tightly linked > > > to DRM. > > > > > > I don't think we should spread this beyond DRM, I can't see a reason. > > > > Yeah v4l has a legit reason to use dma_fence, android wants that > > there. > > 'legit' in the sense the v4l is supposed to trigger stuff in DRM when > V4L DMA completes? I would still see that as part of DRM Yes, and also the other way around. But thus far it didn't land. -Daniel > Or is it building a parallel DRM like DMA completion graph? > > > > Trying to improve performance of limited HW by using sketchy > > > techniques at the cost of general system stability should be a NAK. > > > > Well that's pretty much gpu drivers, all the horrors for a bit more speed > > :-) > > > > On the text itself, should I upgrade to "must not" instead of "should > > not"? Or more needed? > > Fundamentally having some unknowable graph of dependencies where parts > of the graph can be placed in critical regions like notifiers is a > complete maintenance nightmare. > > I think building systems like this should be discouraged :\ -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v2 PATCH] drm/panel: auo, b116xw03: fix flash backlight when power on
On Sun, Jul 05, 2020 at 05:45:14PM +0800, Jitao Shi wrote: > Delay the backlight on to make sure the video stable. > > Signed-off-by: Jitao Shi Thanks, fixed up and applied to drm-misc-next. Sam > --- > drivers/gpu/drm/panel/panel-simple.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > b/drivers/gpu/drm/panel/panel-simple.c > index 3ad828eaefe1..61781ffa7840 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -724,6 +724,7 @@ static const struct drm_display_mode auo_b116xw03_mode = { > .vsync_end = 768 + 10 + 12, > .vtotal = 768 + 10 + 12 + 6, > .vrefresh = 60, > + .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, > }; > > static const struct panel_desc auo_b116xw03 = { > @@ -734,6 +735,12 @@ static const struct panel_desc auo_b116xw03 = { > .width = 256, > .height = 144, > }, > + .delay = { > + .enable = 400, > + }, > + .bus_flags = DRM_BUS_FLAG_SYNC_DRIVE_NEGEDGE, > + .bus_format = MEDIA_BUS_FMT_RGB666_1X18, > + .connector_type = DRM_MODE_CONNECTOR_eDP, > }; > > static const struct drm_display_mode auo_b133xtn01_mode = { > -- > 2.25.1 > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH v4 4/7] drm: msm: a6xx: use dev_pm_opp_set_bw to scale DDR
On Thu, Jul 9, 2020 at 1:01 PM Akhil P Oommen wrote: > > From: Sharat Masetty > > This patches replaces the previously used static DDR vote and uses > dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling > GPU frequency. Also since the icc path voting is handled completely > in the opp driver, remove the icc_path handle and its usage in the > drm driver. > > Signed-off-by: Sharat Masetty > Signed-off-by: Akhil P Oommen > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 25 + > 1 file changed, 17 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > index b547339..6fbfd7d 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c > @@ -123,7 +123,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct > dev_pm_opp *opp) > > if (!gmu->legacy) { > a6xx_hfi_set_freq(gmu, gmu->current_perf_index); > - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > + dev_pm_opp_set_bw(>pdev->dev, opp); What is the status of the patch to add dev_pm_opp_set_bw()? If it is ready to go, and I get an ack-by from the OPP maintainer, I suppose I could merge it via drm/msm. Otherwise should we consider pulling in a private copy of it into drm/msm (and then drop it to use the helper in, hopefully, the next cycle)? I'm pulling the patches preceding this one into msm-next-staging to do some testing. And the dt patches following this one would normally get merged via Bjorn. At the moment, I'm not sure what to do with this one. BR, -R > return; > } > > @@ -149,11 +149,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct > dev_pm_opp *opp) > if (ret) > dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret); > > - /* > -* Eventually we will want to scale the path vote with the frequency > but > -* for now leave it at max so that the performance is nominal. > -*/ > - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216)); > + dev_pm_opp_set_bw(>pdev->dev, opp); > } > > unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu) > @@ -840,6 +836,19 @@ static void a6xx_gmu_set_initial_freq(struct msm_gpu > *gpu, struct a6xx_gmu *gmu) > dev_pm_opp_put(gpu_opp); > } > > +static void a6xx_gmu_set_initial_bw(struct msm_gpu *gpu, struct a6xx_gmu > *gmu) > +{ > + struct dev_pm_opp *gpu_opp; > + unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index]; > + > + gpu_opp = dev_pm_opp_find_freq_exact(>pdev->dev, gpu_freq, true); > + if (IS_ERR_OR_NULL(gpu_opp)) > + return; > + > + dev_pm_opp_set_bw(>pdev->dev, gpu_opp); > + dev_pm_opp_put(gpu_opp); > +} > + > int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > { > struct adreno_gpu *adreno_gpu = _gpu->base; > @@ -864,7 +873,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu) > } > > /* Set the bus quota to a reasonable value for boot */ > - icc_set_bw(gpu->icc_path, 0, MBps_to_icc(3072)); > + a6xx_gmu_set_initial_bw(gpu, gmu); > > /* Enable the GMU interrupt */ > gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_CLR, ~0); > @@ -1040,7 +1049,7 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) > a6xx_gmu_shutdown(gmu); > > /* Remove the bus vote */ > - icc_set_bw(gpu->icc_path, 0, 0); > + dev_pm_opp_set_bw(>pdev->dev, NULL); > > /* > * Make sure the GX domain is off before turning off the GMU (CX) > -- > 2.7.4 > > ___ > Freedreno mailing list > freedr...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/2] drm/probe_helper, i915: Validate MST modes against PBN limits
Something we've been missing for a while with drivers that support MST is being able to prune modes that can't be set due to bandwidth limitations. So, let's go ahead and add that. This also adds a new hook that was needed, mode_valid_ctx, so that we can grab additional locks as needed when validating modes. Lee Shawn C (1): drm/i915/mst: filter out the display mode exceed sink's capability Lyude Paul (1): drm/probe_helper: Add drm_connector_helper_funcs.mode_valid_ctx drivers/gpu/drm/drm_crtc_helper_internal.h | 7 +- drivers/gpu/drm/drm_probe_helper.c | 94 ++--- drivers/gpu/drm/i915/display/intel_dp_mst.c | 55 include/drm/drm_modeset_helper_vtables.h| 42 + 4 files changed, 147 insertions(+), 51 deletions(-) -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 1/2] drm/probe_helper: Add drm_connector_helper_funcs.mode_valid_ctx
This is just an atomic version of mode_valid, which is intended to be used for situations where a driver might need to check the atomic state of objects other than the connector itself. One such example is with MST, where the maximum possible bandwidth on a connector can change dynamically irregardless of the display configuration. Changes since v1: * Use new drm logging functions * Make some corrections in the mode_valid_ctx kdoc * Return error codes or 0 from ->mode_valid_ctx() on fail, and store the drm_mode_status in an additional function parameter Changes since v2: * Don't accidentally assign ret to mode->status on success, or we'll squash legitimate mode validation results * Don't forget to assign MODE_OK to status in drm_connector_mode_valid() if we have no callbacks * Drop leftover hunk in drm_modes.h around enum drm_mode_status Signed-off-by: Lyude Paul Cc: Lee Shawn C --- drivers/gpu/drm/drm_crtc_helper_internal.h | 7 +- drivers/gpu/drm/drm_probe_helper.c | 94 ++ include/drm/drm_modeset_helper_vtables.h | 42 ++ 3 files changed, 109 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/drm_crtc_helper_internal.h b/drivers/gpu/drm/drm_crtc_helper_internal.h index f0a66ef47e5ad..25ce42e799952 100644 --- a/drivers/gpu/drm/drm_crtc_helper_internal.h +++ b/drivers/gpu/drm/drm_crtc_helper_internal.h @@ -73,8 +73,11 @@ enum drm_mode_status drm_crtc_mode_valid(struct drm_crtc *crtc, const struct drm_display_mode *mode); enum drm_mode_status drm_encoder_mode_valid(struct drm_encoder *encoder, const struct drm_display_mode *mode); -enum drm_mode_status drm_connector_mode_valid(struct drm_connector *connector, - struct drm_display_mode *mode); +int +drm_connector_mode_valid(struct drm_connector *connector, +struct drm_display_mode *mode, +struct drm_modeset_acquire_ctx *ctx, +enum drm_mode_status *status); struct drm_encoder * drm_connector_get_single_encoder(struct drm_connector *connector); diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c index e0ed58d291ed9..f7bd1d35aa805 100644 --- a/drivers/gpu/drm/drm_probe_helper.c +++ b/drivers/gpu/drm/drm_probe_helper.c @@ -86,17 +86,19 @@ drm_mode_validate_flag(const struct drm_display_mode *mode, return MODE_OK; } -static enum drm_mode_status +static int drm_mode_validate_pipeline(struct drm_display_mode *mode, - struct drm_connector *connector) + struct drm_connector *connector, + struct drm_modeset_acquire_ctx *ctx, + enum drm_mode_status *status) { struct drm_device *dev = connector->dev; - enum drm_mode_status ret = MODE_OK; struct drm_encoder *encoder; + int ret; /* Step 1: Validate against connector */ - ret = drm_connector_mode_valid(connector, mode); - if (ret != MODE_OK) + ret = drm_connector_mode_valid(connector, mode, ctx, status); + if (ret || *status != MODE_OK) return ret; /* Step 2: Validate against encoders and crtcs */ @@ -104,8 +106,8 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, struct drm_bridge *bridge; struct drm_crtc *crtc; - ret = drm_encoder_mode_valid(encoder, mode); - if (ret != MODE_OK) { + *status = drm_encoder_mode_valid(encoder, mode); + if (*status != MODE_OK) { /* No point in continuing for crtc check as this encoder * will not accept the mode anyway. If all encoders * reject the mode then, at exit, ret will not be @@ -114,10 +116,10 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, } bridge = drm_bridge_chain_get_first_bridge(encoder); - ret = drm_bridge_chain_mode_valid(bridge, - >display_info, - mode); - if (ret != MODE_OK) { + *status = drm_bridge_chain_mode_valid(bridge, + >display_info, + mode); + if (*status != MODE_OK) { /* There is also no point in continuing for crtc check * here. */ continue; @@ -127,8 +129,8 @@ drm_mode_validate_pipeline(struct drm_display_mode *mode, if (!drm_encoder_crtc_ok(encoder, crtc)) continue; - ret = drm_crtc_mode_valid(crtc, mode); - if (ret ==
[PATCH v3 2/2] drm/i915/mst: filter out the display mode exceed sink's capability
From: Lee Shawn C So far, max dot clock rate for MST mode rely on physcial bandwidth limitation. It would caused compatibility issue if source display resolution exceed MST hub output ability. For example, source DUT had DP 1.2 output capability. And MST docking just support HDMI 1.4 spec. When a HDMI 2.0 monitor connected. Source would retrieve EDID from external and get max resolution 4k@60fps. DP 1.2 can support 4K@60fps because it did not surpass DP physical bandwidth limitation. Do modeset to 4k@60fps, source output display data but MST docking can't output HDMI properly due to this resolution already over HDMI 1.4 spec. Refer to commit ("drm/dp_mst: Use full_pbn instead of available_pbn for bandwidth checks"). Source driver should refer to full_pbn to evaluate sink output capability. And filter out the resolution surpass sink output limitation. Changes since v1: * Using mgr->base.lock to protect full_pbn. Changes since v2: * Add ctx lock. Changes since v3: * s/intel_dp_mst_mode_clock_exceed_pbn_bandwidth/ intel_dp_mst_mode_clock_exceeds_pbn_bw/ * Use the new drm_connector_helper_funcs.mode_valid_ctx to properly pipe down the drm_modeset_acquire_ctx that the probe helpers are using, so we can safely grab >base.lock without deadlocking Changes since v4: * Move drm_dp_calc_pbn_mode(mode->clock, bpp, false) > port->full_pbn check * Fix the bpp we use in drm_dp_calc_pbn_mode() * Drop leftover (!mgr) check * Don't check for if full_pbn is unset. To be clear - it _can_ be unset, but if it is then it's certainly a bug in DRM or a non-compliant sink as full_pbn should always be populated by the time we call ->mode_valid_ctx. We should workaround non-compliant sinks with full_pbn=0, but that should happen in the DP MST helpers so we can estimate the full_pbn value as best we can. Cc: Manasi Navare Cc: Jani Nikula Cc: Ville Syrjala Cc: Cooper Chiou Co-developed-by: Lyude Paul Signed-off-by: Lee Shawn C Signed-off-by: Lyude Paul --- drivers/gpu/drm/i915/display/intel_dp_mst.c | 55 ++--- 1 file changed, 38 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index bdc19b04b2c10..a2d91a4997001 100644 --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c @@ -639,39 +639,60 @@ static int intel_dp_mst_get_modes(struct drm_connector *connector) return intel_dp_mst_get_ddc_modes(connector); } -static enum drm_mode_status -intel_dp_mst_mode_valid(struct drm_connector *connector, - struct drm_display_mode *mode) +static int +intel_dp_mst_mode_valid_ctx(struct drm_connector *connector, + struct drm_display_mode *mode, + struct drm_modeset_acquire_ctx *ctx, + enum drm_mode_status *status) { struct drm_i915_private *dev_priv = to_i915(connector->dev); struct intel_connector *intel_connector = to_intel_connector(connector); struct intel_dp *intel_dp = intel_connector->mst_port; + struct drm_dp_mst_topology_mgr *mgr = _dp->mst_mgr; + struct drm_dp_mst_port *port = intel_connector->port; + const int min_bpp = 18; int max_dotclk = to_i915(connector->dev)->max_dotclk_freq; int max_rate, mode_rate, max_lanes, max_link_clock; + int ret; - if (drm_connector_is_unregistered(connector)) - return MODE_ERROR; + if (drm_connector_is_unregistered(connector)) { + *status = MODE_ERROR; + return 0; + } - if (mode->flags & DRM_MODE_FLAG_DBLSCAN) - return MODE_NO_DBLESCAN; + if (mode->flags & DRM_MODE_FLAG_DBLSCAN) { + *status = MODE_NO_DBLESCAN; + return 0; + } max_link_clock = intel_dp_max_link_rate(intel_dp); max_lanes = intel_dp_max_lane_count(intel_dp); max_rate = intel_dp_max_data_rate(max_link_clock, max_lanes); - mode_rate = intel_dp_link_required(mode->clock, 18); + mode_rate = intel_dp_link_required(mode->clock, min_bpp); - /* TODO - validate mode against available PBN for link */ - if (mode->clock < 1) - return MODE_CLOCK_LOW; + ret = drm_modeset_lock(>base.lock, ctx); + if (ret) + return ret; - if (mode->flags & DRM_MODE_FLAG_DBLCLK) - return MODE_H_ILLEGAL; + if (mode_rate > max_rate || mode->clock > max_dotclk || + drm_dp_calc_pbn_mode(mode->clock, min_bpp, false) > port->full_pbn) { + *status = MODE_CLOCK_HIGH; + return 0; + } + + if (mode->clock < 1) { + *status = MODE_CLOCK_LOW; + return 0; + } - if (mode_rate > max_rate || mode->clock > max_dotclk) - return MODE_CLOCK_HIGH; + if (mode->flags &
Re: [PATCH v1 0/1] dt-bindings: fix simple-framebuffer warning
On Sat, Jul 04, 2020 at 04:35:43PM +0200, Sam Ravnborg wrote: > Trivial fix for a long standing warning. > At least not fixed in drm-msc-next for now. > Just in case it was not fixed by someone else (Rob?) already. > > Sam > > Sam Ravnborg (1): > dt-bindings: fix simple-framebuffer example Applied to drm-misc-next. Sam > > .../bindings/display/simple-framebuffer.yaml | 45 > +++--- > 1 file changed, 23 insertions(+), 22 deletions(-) > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/3] dt-bindings: display: convert panel bindings to DT Schema
On Sat, Jul 04, 2020 at 12:28:03PM +0200, Sam Ravnborg wrote: > This patch-set convert 3 of the remaining panel bindings to yaml. > > This is a follow-up on v2 that converted a lot of panel bindings: > https://lore.kernel.org/dri-devel/20200408195109.32692-1-...@ravnborg.org/ > All was applied except for the reaming three patches included here. > > One binding is a DSI binding so just added to panel-simple-dsi. > The other two bindings addressed review feedback from Rob. > > Sebastian Reichel has a pending patch to address the remaining > panel binding in display/panel/ > > All bindings pass dt-binding-check. > Based on top of drm-misc-next. > > Sam > > > Sam Ravnborg (3): > dt-bindings: display: convert innolux,p079zca to DT Schema > dt-bindings: display: convert samsung,s6e8aa0 to DT Schema > dt-bindings: display: convert sharp,lq101r1sx01 to DT Schema All applied to drm-misc-next. Sam > > .../bindings/display/panel/innolux,p079zca.txt | 22 - > .../bindings/display/panel/panel-simple-dsi.yaml | 2 + > .../bindings/display/panel/samsung,s6e8aa0.txt | 56 > .../bindings/display/panel/samsung,s6e8aa0.yaml| 100 > + > .../bindings/display/panel/sharp,lq101r1sx01.txt | 49 -- > .../bindings/display/panel/sharp,lq101r1sx01.yaml | 87 ++ > 6 files changed, 189 insertions(+), 127 deletions(-) > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/drm_fb_helper: fix fbdev with sparc64
On 10/07/2020 07:28, Thomas Zimmermann wrote: Hi Sam, Thanks again for the patch. I've spotted some small typos that you may like to fix if you repost the patch: > Hi > > Am 09.07.20 um 21:30 schrieb Sam Ravnborg: >> Mark reported that sparc64 would panic while booting using qemu. >> Mark bisected this to a patch that introduced generic fbdev emulation to >> the bochs DRM driver. >> Mark pointed out that a similar bug was fixed before where >> the sys helpers was replaced by cfb helpers. >> The culprint here is that the framebuffer reside in IO memory which Typo here: culprit >> requires SPARC ASI_PHYS (physical) loads and stores. >> >> The current bohcs DRM driver uses a shadow buffer. And another here: bochs >> So all copying to the framebuffer happens in >> drm_fb_helper_dirty_blit_real(). How about this as an alternative to the above paragraphs which might be a bit easier to read: Recent kernels have been reported to panic using the bochs_drm framebuffer under qemu-system-sparc64 which was bisected to commit 7a0483ac4ffc "drm/bochs: switch to generic drm fbdev emulation". The backtrace indicates that the shadow framebuffer copy in drm_fb_helper_dirty_blit_real() is trying to access the real framebuffer using a virtual address rather than use an IO access typically implemented using a physical (ASI_PHYS) access on SPARC. >> The fix is to replace the memcpy with memcpy_toio() from io.h. >> >> memcpy_toio() uses writeb() where the original fbdev code >> used sbus_memcpy_toio(). The latter uses sbus_writeb(). >> >> The difference between writeb() and sbus_memcpy_toio() is >> that writeb() writes bytes in little-endian, where sbus_writeb() writes >> bytes in big-endian. As endian does not matter for byte writes they are >> the same. So we can safely use memcpy_toio() here. >> >> For many architectures memcpy_toio() is a simple memcpy(). >> One sideeffect that is unknow is if this has any impact on other side-effect >> architectures. >> So far the analysis tells that this change is OK for other arch's. >> but testing would be good. >> >> Signed-off-by: Sam Ravnborg >> Reported-by: Mark Cave-Ayland >> Tested-by: Mark Cave-Ayland >> Cc: Mark Cave-Ayland >> Cc: Thomas Zimmermann >> Cc: Gerd Hoffmann >> Cc: "David S. Miller" >> Cc: sparcli...@vger.kernel.org > > So this actually is a problem in practice. Do you know how userspace > handles this? > > For this patch > > Acked-by: Thomas Zimmermann > > but I'd like to have someone with more architecture expertise ack this > as well. Agreed. All my testing is using the bochs_drm framebuffer under qemu-system-sparc64 (a sun4u machine) so it would be nice to get an ACK from Dave or someone else who can vouch for this on real hardware. ATB, Mark. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 0/3] dt-bindings: display: convert panel bindings to DT Schema
Hi Rob. > > On Sat, Jul 04, 2020 at 12:28:03PM +0200, Sam Ravnborg wrote: > > > This patch-set convert 3 of the remaining panel bindings to yaml. > > > > > > This is a follow-up on v2 that converted a lot of panel bindings: > > > https://lore.kernel.org/dri-devel/20200408195109.32692-1-...@ravnborg.org/ > > > All was applied except for the reaming three patches included here. > > > > > > One binding is a DSI binding so just added to panel-simple-dsi. > > > The other two bindings addressed review feedback from Rob. > > > > > > Sebastian Reichel has a pending patch to address the remaining > > > panel binding in display/panel/ > > > > > > All bindings pass dt-binding-check. > > > Based on top of drm-misc-next. > > > > > > Sam > > > > > > > > > Sam Ravnborg (3): > > > dt-bindings: display: convert innolux,p079zca to DT Schema > > > dt-bindings: display: convert samsung,s6e8aa0 to DT Schema > > > dt-bindings: display: convert sharp,lq101r1sx01 to DT Schema > > > > git format-patch adds a space after the ',' - in the subject. > > This is often a good idea, but not for binding files. > > Will fix when I apply - if I do not forget that is.. > > Sure about that? I'm pretty sure it's dri-devel doing it. Look at > lore.kernel.org copies for different lists. I've been fighting with that > first in patchwork (which had this bug) and then in b4 (which is where > it got nailed down to dri-devel). You are right. The patches that I copied myself on had the correct subject. Only the dri-devel mails had the mangled subject. Sam > > Rob ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/exynos: gem: Fix sparse warning
Hi Marek. On Tue, Jul 07, 2020 at 01:08:59PM +0200, Marek Szyprowski wrote: > Add a proper cast on the exynos_gem->kvaddr assignment to avoid a sparse > warning. > > Reported-by: kernel test robot > Fixes: 9940d9d93406 ("drm/exynos: gem: Get rid of the internal 'pages' array") > Signed-off-by: Marek Szyprowski > --- > drivers/gpu/drm/exynos/exynos_drm_gem.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_gem.c > b/drivers/gpu/drm/exynos/exynos_drm_gem.c > index efa476858db5..65d11784f29f 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_gem.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_gem.c > @@ -59,7 +59,7 @@ static int exynos_drm_alloc_buf(struct exynos_drm_gem > *exynos_gem, bool kvmap) > } > > if (kvmap) > - exynos_gem->kvaddr = exynos_gem->cookie; > + exynos_gem->kvaddr = (void __iomem *)exynos_gem->cookie; >From a brif look at the code the correct fix looks to me to drop the __iomem annotation of kvaddr. And then no cast is needed. Care to take a look at this? Sam > > DRM_DEV_DEBUG_KMS(to_dma_dev(dev), "dma_addr(0x%lx), size(0x%lx)\n", > (unsigned long)exynos_gem->dma_addr, exynos_gem->size); > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 2/2] drm/msm/adreno: un-open-code some packets
On Tue, Jul 07, 2020 at 01:35:00PM -0700, Rob Clark wrote: > From: Rob Clark Might need a commit log here, but otherwise makes sense. Reviewed-by: Jordan Crouse > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 5 +++-- > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 11 +++ > 2 files changed, 10 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > index d95970a73fb4..7f4526b3283d 100644 > --- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c > @@ -186,7 +186,8 @@ static void a5xx_submit(struct msm_gpu *gpu, struct > msm_gem_submit *submit, >* timestamp is written to the memory and then triggers the interrupt >*/ > OUT_PKT7(ring, CP_EVENT_WRITE, 4); > - OUT_RING(ring, CACHE_FLUSH_TS | (1 << 31)); > + OUT_RING(ring, CP_EVENT_WRITE_0_EVENT(CACHE_FLUSH_TS) | > + CP_EVENT_WRITE_0_IRQ); > OUT_RING(ring, lower_32_bits(rbmemptr(ring, fence))); > OUT_RING(ring, upper_32_bits(rbmemptr(ring, fence))); > OUT_RING(ring, submit->seqno); > @@ -730,7 +731,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu) >*/ > if (adreno_is_a530(adreno_gpu)) { > OUT_PKT7(gpu->rb[0], CP_EVENT_WRITE, 1); > - OUT_RING(gpu->rb[0], 0x0F); > + OUT_RING(gpu->rb[0], CP_EVENT_WRITE_0_EVENT(STAT_EVENT)); > > gpu->funcs->flush(gpu, gpu->rb[0]); > if (!a5xx_idle(gpu, gpu->rb[0])) > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > index 7768557cdfb2..1ed325bea430 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c > @@ -74,7 +74,9 @@ static void get_stats_counter(struct msm_ringbuffer *ring, > u32 counter, > u64 iova) > { > OUT_PKT7(ring, CP_REG_TO_MEM, 3); > - OUT_RING(ring, counter | (1 << 30) | (2 << 18)); > + OUT_RING(ring, CP_REG_TO_MEM_0_REG(counter) | > + CP_REG_TO_MEM_0_CNT(2) | > + CP_REG_TO_MEM_0_64B); > OUT_RING(ring, lower_32_bits(iova)); > OUT_RING(ring, upper_32_bits(iova)); > } > @@ -102,10 +104,10 @@ static void a6xx_submit(struct msm_gpu *gpu, struct > msm_gem_submit *submit, > > /* Invalidate CCU depth and color */ > OUT_PKT7(ring, CP_EVENT_WRITE, 1); > - OUT_RING(ring, PC_CCU_INVALIDATE_DEPTH); > + OUT_RING(ring, CP_EVENT_WRITE_0_EVENT(PC_CCU_INVALIDATE_DEPTH)); > > OUT_PKT7(ring, CP_EVENT_WRITE, 1); > - OUT_RING(ring, PC_CCU_INVALIDATE_COLOR); > + OUT_RING(ring, CP_EVENT_WRITE_0_EVENT(PC_CCU_INVALIDATE_COLOR)); > > /* Submit the commands */ > for (i = 0; i < submit->nr_cmds; i++) { > @@ -139,7 +141,8 @@ static void a6xx_submit(struct msm_gpu *gpu, struct > msm_gem_submit *submit, >* timestamp is written to the memory and then triggers the interrupt >*/ > OUT_PKT7(ring, CP_EVENT_WRITE, 4); > - OUT_RING(ring, CACHE_FLUSH_TS | (1 << 31)); > + OUT_RING(ring, CP_EVENT_WRITE_0_EVENT(CACHE_FLUSH_TS) | > + CP_EVENT_WRITE_0_IRQ); > OUT_RING(ring, lower_32_bits(rbmemptr(ring, fence))); > OUT_RING(ring, upper_32_bits(rbmemptr(ring, fence))); > OUT_RING(ring, submit->seqno); > -- > 2.26.2 > > ___ > Freedreno mailing list > freedr...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm: fb-helper: Convert logging to drm_* functions.
On Tue, Jul 07, 2020 at 10:07:21PM +0530, Suraj Upadhyay wrote: > Change logging information from dev_info() to drm_info(). > > Signed-off-by: Suraj Upadhyay Thanks. Applied to drm-misc-next. Also, because there was no more logging functions that needed an update. Sam > --- > drivers/gpu/drm/drm_fb_helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index 5609e164805f..88146f7245c5 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1819,7 +1819,7 @@ __drm_fb_helper_initial_config_and_unlock(struct > drm_fb_helper *fb_helper, > if (ret < 0) > return ret; > > - dev_info(dev->dev, "fb%d: %s frame buffer device\n", > + drm_info(dev, "fb%d: %s frame buffer device\n", >info->node, info->fix.id); > > mutex_lock(_fb_helper_lock); > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm: mipi-dsi: Convert logging to drm_* functions.
On Tue, Jul 07, 2020 at 09:58:48PM +0530, Suraj Upadhyay wrote: > Convert logging errors from dev_err() to drm_err(). > > Signed-off-by: Suraj Upadhyay Thanks. Applied to drm-misc-next as there was no more logging conversion to do in this file. Sam > --- > drivers/gpu/drm/drm_mipi_dsi.c | 15 +++ > 1 file changed, 7 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c > index 07102d8da58f..5dd475e82995 100644 > --- a/drivers/gpu/drm/drm_mipi_dsi.c > +++ b/drivers/gpu/drm/drm_mipi_dsi.c > @@ -34,6 +34,7 @@ > #include > > #include > +#include > #include > > /** > @@ -155,19 +156,18 @@ static int mipi_dsi_device_add(struct mipi_dsi_device > *dsi) > static struct mipi_dsi_device * > of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node) > { > - struct device *dev = host->dev; > struct mipi_dsi_device_info info = { }; > int ret; > u32 reg; > > if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) { > - dev_err(dev, "modalias failure on %pOF\n", node); > + drm_err(host, "modalias failure on %pOF\n", node); > return ERR_PTR(-EINVAL); > } > > ret = of_property_read_u32(node, "reg", ); > if (ret) { > - dev_err(dev, "device node %pOF has no valid reg property: %d\n", > + drm_err(host, "device node %pOF has no valid reg property: > %d\n", > node, ret); > return ERR_PTR(-EINVAL); > } > @@ -202,22 +202,21 @@ mipi_dsi_device_register_full(struct mipi_dsi_host > *host, > const struct mipi_dsi_device_info *info) > { > struct mipi_dsi_device *dsi; > - struct device *dev = host->dev; > int ret; > > if (!info) { > - dev_err(dev, "invalid mipi_dsi_device_info pointer\n"); > + drm_err(host, "invalid mipi_dsi_device_info pointer\n"); > return ERR_PTR(-EINVAL); > } > > if (info->channel > 3) { > - dev_err(dev, "invalid virtual channel: %u\n", info->channel); > + drm_err(host, "invalid virtual channel: %u\n", info->channel); > return ERR_PTR(-EINVAL); > } > > dsi = mipi_dsi_device_alloc(host); > if (IS_ERR(dsi)) { > - dev_err(dev, "failed to allocate DSI device %ld\n", > + drm_err(host, "failed to allocate DSI device %ld\n", > PTR_ERR(dsi)); > return dsi; > } > @@ -228,7 +227,7 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host, > > ret = mipi_dsi_device_add(dsi); > if (ret) { > - dev_err(dev, "failed to add DSI device %d\n", ret); > + drm_err(host, "failed to add DSI device %d\n", ret); > kfree(dsi); > return ERR_PTR(ret); > } > -- > 2.17.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.
Hi Suraj. On Tue, Jul 07, 2020 at 10:04:14PM +0530, Suraj Upadhyay wrote: > > This patchset converts logging to drm_* functions in drm core. > > The following functions have been converted to their respective > DRM alternatives : > dev_info() --> drm_info() > dev_err() --> drm_err() > dev_warn() --> drm_warn() > dev_err_once() --> drm_err_once(). I would prefer that DRM_* logging in the same files are converted in the same patch. So we have one logging conversion patch for each file you touches and that we do not need to re-vist the files later to change another set of logging functions. If possible WARN_* should also be converted to drm_WARN_* If patch is too large, then split them up but again lets have all logging updated when we touch a file. Care to take a look at this approach? Also please consider if coccinelle can make this job easier. There is a lot of files... Sam > > Suraj Upadhyay (4): > drm: mipi-dsi: Convert logging to drm_* functions. > drm: mipi-dbi: Convert logging to drm_* functions. > drm: edid: Convert logging to drm_* functions. > drm: fb-helper: Convert logging to drm_* functions. > > drivers/gpu/drm/drm_edid.c | 7 +++ > drivers/gpu/drm/drm_fb_helper.c | 2 +- > drivers/gpu/drm/drm_mipi_dbi.c | 4 ++-- > drivers/gpu/drm/drm_mipi_dsi.c | 15 +++ > 4 files changed, 13 insertions(+), 15 deletions(-) > > -- > 2.17.1 > > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Replace HTTP links with HTTPS ones: DRM DRIVERS FOR BRIDGE CHIPS
On Wed, Jul 08, 2020 at 02:16:04PM +0200, Alexander A. Klimov wrote: > Rationale: > Reduces attack surface on kernel devs opening the links for MITM > as HTTPS traffic is much harder to manipulate. > > Deterministic algorithm: > For each file: > If not .svg: > For each line: > If doesn't contain `\bxmlns\b`: > For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`: > If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`: > If both the HTTP and HTTPS versions > return 200 OK and serve the same content: > Replace HTTP with HTTPS. > > Signed-off-by: Alexander A. Klimov Adjusted subject and applied to drm-misc-next. Please be more careful with subjects in the future. They cannot be generated by a script - you need to look at previous commits to the same file(s) and adjust. Sam > --- > Continuing my work started at 93431e0607e5. > See also: git log --oneline '--author=Alexander A. Klimov > ' v5.7..master > (Actually letting a shell for loop submit all this stuff for me.) > > If there are any URLs to be removed completely or at least not HTTPSified: > Just clearly say so and I'll *undo my change*. > See also: https://lkml.org/lkml/2020/6/27/64 > > If there are any valid, but yet not changed URLs: > See: https://lkml.org/lkml/2020/6/26/837 > > If you apply the patch, please let me know. > > > drivers/gpu/drm/bridge/tc358768.c | 2 +- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358768.c > b/drivers/gpu/drm/bridge/tc358768.c > index 6650fe4cfc20..ac3757ecade6 100644 > --- a/drivers/gpu/drm/bridge/tc358768.c > +++ b/drivers/gpu/drm/bridge/tc358768.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > /* > - * Copyright (C) 2020 Texas Instruments Incorporated - http://www.ti.com > + * Copyright (C) 2020 Texas Instruments Incorporated - https://www.ti.com > * Author: Peter Ujfalusi > */ > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > index 6ad688b320ae..a17f3e3a4157 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > @@ -1,7 +1,7 @@ > // SPDX-License-Identifier: GPL-2.0 > /* > * Copyright (c) 2018, The Linux Foundation. All rights reserved. > - * datasheet: http://www.ti.com/lit/ds/symlink/sn65dsi86.pdf > + * datasheet: https://www.ti.com/lit/ds/symlink/sn65dsi86.pdf > */ > > #include > -- > 2.27.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Replace HTTP links with HTTPS ones: BACKLIGHT CLASS/SUBSYSTEM
Hi Alexander. On Wed, Jul 08, 2020 at 06:38:47PM +0200, Alexander A. Klimov wrote: > Rationale: > Reduces attack surface on kernel devs opening the links for MITM > as HTTPS traffic is much harder to manipulate. > > Deterministic algorithm: > For each file: > If not .svg: > For each line: > If doesn't contain `\bxmlns\b`: > For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`: > If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`: > If both the HTTP and HTTPS versions > return 200 OK and serve the same content: > Replace HTTP with HTTPS. > > Signed-off-by: Alexander A. Klimov > --- > Continuing my work started at 93431e0607e5. > See also: git log --oneline '--author=Alexander A. Klimov > ' v5.7..master > (Actually letting a shell for loop submit all this stuff for me.) > > If there are any URLs to be removed completely or at least not HTTPSified: > Just clearly say so and I'll *undo my change*. > See also: https://lkml.org/lkml/2020/6/27/64 > > If there are any valid, but yet not changed URLs: > See: https://lkml.org/lkml/2020/6/26/837 > > If you apply the patch, please let me know. > > > drivers/video/backlight/led_bl.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/video/backlight/led_bl.c > b/drivers/video/backlight/led_bl.c > index 3f66549997c8..2c48957e9b60 100644 > --- a/drivers/video/backlight/led_bl.c > +++ b/drivers/video/backlight/led_bl.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > /* > - * Copyright (C) 2015-2019 Texas Instruments Incorporated - > http://www.ti.com/ > + * Copyright (C) 2015-2019 Texas Instruments Incorporated - > https://www.ti.com/ > * Author: Tomi Valkeinen > * > * Based on pwm_bl.c $grep http drivers/video/backlight/* drivers/video/backlight/cr_bllcd.c: * Intel funded Tungsten Graphics (http://www.tungstengraphics.com) to drivers/video/backlight/ili9320.c: *http://armlinux.simtec.co.uk/ drivers/video/backlight/ili9320.h: * http://armlinux.simtec.co.uk/ drivers/video/backlight/led_bl.c: * Copyright (C) 2015-2019 Texas Instruments Incorporated - http://www.ti.com/ drivers/video/backlight/vgg2432a4.c: * http://armlinux.simtec.co.uk/ Why are they not covered? Sam > -- > 2.27.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/a6xx: add build_bw_table for A640/A650
On Tue, Jun 30, 2020 at 11:09:57PM -0400, Jonathan Marek wrote: > This sets up bw tables for A640/A650 similar to A618/A630, 0 DDR bandwidth > vote, and the CNOC vote. A640 has the same CNOC addresses as A630 and was > working, but this is required for A650 to work. > > Eventually the bw table should be filled by querying the interconnect > driver for each BW in the dts, but use these dummy tables for now. Reviewed-by: Jordan Crouse And yes, I agree that we need to move this into the generic API sooner rather than later, but this should be good enough to get a working GPU/GMU. > Signed-off-by: Jonathan Marek > --- > drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 74 +++ > 1 file changed, 74 insertions(+) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c > b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c > index 9921e632f1ca..ccd44d0418f8 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.c > @@ -281,6 +281,76 @@ static void a618_build_bw_table(struct > a6xx_hfi_msg_bw_table *msg) > msg->cnoc_cmds_data[1][0] = 0x6001; > } > > +static void a640_build_bw_table(struct a6xx_hfi_msg_bw_table *msg) > +{ > + /* > + * Send a single "off" entry just to get things running > + * TODO: bus scaling > + */ > + msg->bw_level_num = 1; > + > + msg->ddr_cmds_num = 3; > + msg->ddr_wait_bitmask = 0x01; > + > + msg->ddr_cmds_addrs[0] = 0x5; > + msg->ddr_cmds_addrs[1] = 0x5003c; > + msg->ddr_cmds_addrs[2] = 0x5000c; > + > + msg->ddr_cmds_data[0][0] = 0x4000; > + msg->ddr_cmds_data[0][1] = 0x4000; > + msg->ddr_cmds_data[0][2] = 0x4000; > + > + /* > + * These are the CX (CNOC) votes - these are used by the GMU but the > + * votes are known and fixed for the target > + */ > + msg->cnoc_cmds_num = 3; > + msg->cnoc_wait_bitmask = 0x01; > + > + msg->cnoc_cmds_addrs[0] = 0x50034; > + msg->cnoc_cmds_addrs[1] = 0x5007c; > + msg->cnoc_cmds_addrs[2] = 0x5004c; > + > + msg->cnoc_cmds_data[0][0] = 0x4000; > + msg->cnoc_cmds_data[0][1] = 0x; > + msg->cnoc_cmds_data[0][2] = 0x4000; > + > + msg->cnoc_cmds_data[1][0] = 0x6001; > + msg->cnoc_cmds_data[1][1] = 0x2001; > + msg->cnoc_cmds_data[1][2] = 0x6001; > +} > + > +static void a650_build_bw_table(struct a6xx_hfi_msg_bw_table *msg) > +{ > + /* > + * Send a single "off" entry just to get things running > + * TODO: bus scaling > + */ > + msg->bw_level_num = 1; > + > + msg->ddr_cmds_num = 3; > + msg->ddr_wait_bitmask = 0x01; > + > + msg->ddr_cmds_addrs[0] = 0x5; > + msg->ddr_cmds_addrs[1] = 0x50004; > + msg->ddr_cmds_addrs[2] = 0x5007c; > + > + msg->ddr_cmds_data[0][0] = 0x4000; > + msg->ddr_cmds_data[0][1] = 0x4000; > + msg->ddr_cmds_data[0][2] = 0x4000; > + > + /* > + * These are the CX (CNOC) votes - these are used by the GMU but the > + * votes are known and fixed for the target > + */ > + msg->cnoc_cmds_num = 1; > + msg->cnoc_wait_bitmask = 0x01; > + > + msg->cnoc_cmds_addrs[0] = 0x500a4; > + msg->cnoc_cmds_data[0][0] = 0x4000; > + msg->cnoc_cmds_data[1][0] = 0x6001; > +} > + > static void a6xx_build_bw_table(struct a6xx_hfi_msg_bw_table *msg) > { > /* Send a single "off" entry since the 630 GMU doesn't do bus scaling */ > @@ -327,6 +397,10 @@ static int a6xx_hfi_send_bw_table(struct a6xx_gmu *gmu) > > if (adreno_is_a618(adreno_gpu)) > a618_build_bw_table(); > + else if (adreno_is_a640(adreno_gpu)) > + a640_build_bw_table(); > + else if (adreno_is_a650(adreno_gpu)) > + a650_build_bw_table(); > else > a6xx_build_bw_table(); > > -- > 2.26.1 > -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RESEND v2 2/2] drm/bridge: dw-hdmi: Always add the bridge in the global bridge list
On Thu, Jul 09, 2020 at 10:02:36AM +0800, Liu Ying wrote: > It doesn't hurt to add the bridge in the global bridge list also for > platform specific dw-hdmi drivers which are based on the component > framework. This can be achieved by moving the drm_bridge_add() function > call from dw_hdmi_probe() to __dw_hdmi_probe(). A counterpart movement > for drm_bridge_remove() is also needed then. Moreover, since drm_bridge_add() > initializes >hpd_mutex, this may help those platform specific > dw-hdmi drivers(based on the component framework) avoid accessing the > uninitialized mutex in drm_bridge_hpd_notify() which is called in > dw_hdmi_irq(). Putting drm_bridge_add() in __dw_hdmi_probe() just before > it returns successfully should bring no logic change for platforms based > on the DRM bridge API, which is a good choice from safety point of view. > Also, __dw_hdmi_probe() is renamed to dw_hdmi_probe() since dw_hdmi_probe() > does nothing else but calling __dw_hdmi_probe(). Similar renaming applies > to the __dw_hdmi_remove()/dw_hdmi_remove() pair. Hmm, it took me some attempts to read this. Anyway, applied as-is to drm-misc-next. Sam > > Fixes: ec971aaa6775 ("drm: bridge: dw-hdmi: Make connector creation optional") > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: David Airlie > Cc: Daniel Vetter > Cc: Boris Brezillon > Cc: Jerome Brunet > Cc: Cheng-Yi Chiang > Cc: Dariusz Marcinkiewicz > Cc: Archit Taneja > Cc: Jose Abreu > Cc: Sam Ravnborg > Cc: dri-devel@lists.freedesktop.org > Cc: NXP Linux Team > Signed-off-by: Liu Ying > --- > v1->v2: > * Put drm_bridge_add() in __dw_hdmi_probe() just before it returns > successfully so that the bridge shows up in the global bridge list > late enough to avoid potential race condition with other display > drivers. (Laurent) > * Rename __dw_hdmi_probe/remove() to dw_hdmi_probe/remove() > respectively. (Laurent) > * Cc'ed Sam. > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 44 > +-- > 1 file changed, 13 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 137b6eb..748df1c 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -3179,9 +3179,11 @@ static void dw_hdmi_init_hw(struct dw_hdmi *hdmi) > hdmi->phy.ops->setup_hpd(hdmi, hdmi->phy.data); > } > > -static struct dw_hdmi * > -__dw_hdmi_probe(struct platform_device *pdev, > - const struct dw_hdmi_plat_data *plat_data) > +/* > - > + * Probe/remove API, used from platforms based on the DRM bridge API. > + */ > +struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev, > + const struct dw_hdmi_plat_data *plat_data) > { > struct device *dev = >dev; > struct device_node *np = dev->of_node; > @@ -3438,6 +3440,8 @@ __dw_hdmi_probe(struct platform_device *pdev, > hdmi->cec = platform_device_register_full(); > } > > + drm_bridge_add(>bridge); > + > return hdmi; > > err_iahb: > @@ -3451,9 +3455,12 @@ __dw_hdmi_probe(struct platform_device *pdev, > > return ERR_PTR(ret); > } > +EXPORT_SYMBOL_GPL(dw_hdmi_probe); > > -static void __dw_hdmi_remove(struct dw_hdmi *hdmi) > +void dw_hdmi_remove(struct dw_hdmi *hdmi) > { > + drm_bridge_remove(>bridge); > + > if (hdmi->audio && !IS_ERR(hdmi->audio)) > platform_device_unregister(hdmi->audio); > if (!IS_ERR(hdmi->cec)) > @@ -3472,31 +3479,6 @@ static void __dw_hdmi_remove(struct dw_hdmi *hdmi) > else > i2c_put_adapter(hdmi->ddc); > } > - > -/* > - > - * Probe/remove API, used from platforms based on the DRM bridge API. > - */ > -struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev, > - const struct dw_hdmi_plat_data *plat_data) > -{ > - struct dw_hdmi *hdmi; > - > - hdmi = __dw_hdmi_probe(pdev, plat_data); > - if (IS_ERR(hdmi)) > - return hdmi; > - > - drm_bridge_add(>bridge); > - > - return hdmi; > -} > -EXPORT_SYMBOL_GPL(dw_hdmi_probe); > - > -void dw_hdmi_remove(struct dw_hdmi *hdmi) > -{ > - drm_bridge_remove(>bridge); > - > - __dw_hdmi_remove(hdmi); > -} > EXPORT_SYMBOL_GPL(dw_hdmi_remove); > > /* > - > @@ -3509,7 +3491,7 @@ struct dw_hdmi *dw_hdmi_bind(struct platform_device > *pdev, > struct dw_hdmi *hdmi; > int ret; > > - hdmi = __dw_hdmi_probe(pdev, plat_data); > + hdmi = dw_hdmi_probe(pdev, plat_data); > if (IS_ERR(hdmi)) > return hdmi; > > @@ -3526,7 +3508,7 @@ EXPORT_SYMBOL_GPL(dw_hdmi_bind);
Re: [PATCH RESEND v2 1/2] drm/bridge: dw-hdmi: Don't cleanup i2c adapter and ddc ptr in __dw_hdmi_probe() bailout path
On Thu, Jul 09, 2020 at 10:02:35AM +0800, Liu Ying wrote: > It's unnecessary to cleanup the i2c adapter and the ddc pointer in > the bailout path of __dw_hdmi_probe(), since the adapter is not > added and the ddc pointer is not set. > > Fixes: a23d6265f033 (drm: bridge: dw-hdmi: Extract PHY interrupt setup to a > function) > Cc: Andrzej Hajda > Cc: Neil Armstrong > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: David Airlie > Cc: Daniel Vetter > Cc: Boris Brezillon > Cc: Jerome Brunet > Cc: Cheng-Yi Chiang > Cc: Dariusz Marcinkiewicz > Cc: Archit Taneja > Cc: Jose Abreu > Cc: dri-devel@lists.freedesktop.org > Cc: NXP Linux Team > Signed-off-by: Liu Ying > Reviewed-by: Laurent Pinchart Thanks, applied to drm-misc-next. Sam > --- > v1->v2: > * Add Laurent's R-b tag. > > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 5 - > 1 file changed, 5 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > index 6148a02..137b6eb 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c > @@ -3441,11 +3441,6 @@ __dw_hdmi_probe(struct platform_device *pdev, > return hdmi; > > err_iahb: > - if (hdmi->i2c) { > - i2c_del_adapter(>i2c->adap); > - hdmi->ddc = NULL; > - } > - > clk_disable_unprepare(hdmi->iahb_clk); > if (hdmi->cec_clk) > clk_disable_unprepare(hdmi->cec_clk); > -- > 2.7.4 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/msm/a6xx: fix crashstate capture for A650
On Mon, Jun 29, 2020 at 08:10:06PM -0400, Jonathan Marek wrote: > A650 has a separate RSCC region, so dump RSCC registers separately, reading > them from the RSCC base. Without this change a GPU hang will cause a system > reset if CONFIG_DEV_COREDUMP is enabled. Reviewed-by: Jordan Crouse > Signed-off-by: Jonathan Marek > --- > drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 5 + > drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c | 25 +++-- > drivers/gpu/drm/msm/adreno/a6xx_gpu_state.h | 12 ++ > 3 files changed, 31 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > index 47df4745db50..c6d2bced8e5d 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h > @@ -127,6 +127,11 @@ static inline u64 gmu_read64(struct a6xx_gmu *gmu, u32 > lo, u32 hi) > readl_poll_timeout((gmu)->mmio + ((addr) << 2), val, cond, \ > interval, timeout) > > +static inline u32 gmu_read_rscc(struct a6xx_gmu *gmu, u32 offset) > +{ > + return msm_readl(gmu->rscc + (offset << 2)); > +} > + > static inline void gmu_write_rscc(struct a6xx_gmu *gmu, u32 offset, u32 > value) > { > return msm_writel(value, gmu->rscc + (offset << 2)); > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > index d6023ba8033c..959656ad6987 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.c > @@ -736,7 +736,8 @@ static void a6xx_get_ahb_gpu_registers(struct msm_gpu > *gpu, > static void _a6xx_get_gmu_registers(struct msm_gpu *gpu, > struct a6xx_gpu_state *a6xx_state, > const struct a6xx_registers *regs, > - struct a6xx_gpu_state_obj *obj) > + struct a6xx_gpu_state_obj *obj, > + bool rscc) > { > struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); > struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); > @@ -755,9 +756,17 @@ static void _a6xx_get_gmu_registers(struct msm_gpu *gpu, > u32 count = RANGE(regs->registers, i); > int j; > > - for (j = 0; j < count; j++) > - obj->data[index++] = gmu_read(gmu, > - regs->registers[i] + j); > + for (j = 0; j < count; j++) { > + u32 offset = regs->registers[i] + j; > + u32 val; > + > + if (rscc) > + val = gmu_read_rscc(gmu, offset); > + else > + val = gmu_read(gmu, offset); > + > + obj->data[index++] = val; > + } > } > } > > @@ -777,7 +786,9 @@ static void a6xx_get_gmu_registers(struct msm_gpu *gpu, > > /* Get the CX GMU registers from AHB */ > _a6xx_get_gmu_registers(gpu, a6xx_state, _gmu_reglist[0], > - _state->gmu_registers[0]); > + _state->gmu_registers[0], false); > + _a6xx_get_gmu_registers(gpu, a6xx_state, _gmu_reglist[1], > + _state->gmu_registers[1], true); > > if (!a6xx_gmu_gx_is_on(_gpu->gmu)) > return; > @@ -785,8 +796,8 @@ static void a6xx_get_gmu_registers(struct msm_gpu *gpu, > /* Set the fence to ALLOW mode so we can access the registers */ > gpu_write(gpu, REG_A6XX_GMU_AO_AHB_FENCE_CTRL, 0); > > - _a6xx_get_gmu_registers(gpu, a6xx_state, _gmu_reglist[1], > - _state->gmu_registers[1]); > + _a6xx_get_gmu_registers(gpu, a6xx_state, _gmu_reglist[2], > + _state->gmu_registers[2], false); > } > > #define A6XX_GBIF_REGLIST_SIZE 1 > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.h > b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.h > index 24c974c293e5..846fd5b54c23 100644 > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.h > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu_state.h > @@ -341,10 +341,6 @@ static const u32 a6xx_gmu_cx_registers[] = { > 0x5157, 0x5158, 0x515d, 0x515d, 0x5162, 0x5162, 0x5164, 0x5165, > 0x5180, 0x5186, 0x5190, 0x519e, 0x51c0, 0x51c0, 0x51c5, 0x51cc, > 0x51e0, 0x51e2, 0x51f0, 0x51f0, 0x5200, 0x5201, > - /* GPU RSCC */ > - 0x8c8c, 0x8c8c, 0x8d01, 0x8d02, 0x8f40, 0x8f42, 0x8f44, 0x8f47, > - 0x8f4c, 0x8f87, 0x8fec, 0x8fef, 0x8ff4, 0x902f, 0x9094, 0x9097, > - 0x909c, 0x90d7, 0x913c, 0x913f, 0x9144, 0x917f, > /* GMU AO */ > 0x9300, 0x9316, 0x9400, 0x9400, > /* GPU CC */ > @@ -357,8 +353,16 @@ static const u32 a6xx_gmu_cx_registers[] = { > 0xbc00, 0xbc16, 0xbc20, 0xbc27, > }; > > +static const u32 a6xx_gmu_cx_rscc_registers[] = { > + /* GPU RSCC */ > + 0x008c, 0x008c, 0x0101, 0x0102, 0x0340, 0x0342, 0x0344, 0x0347, > + 0x034c, 0x0387, 0x03ec, 0x03ef, 0x03f4, 0x042f, 0x0494, 0x0497, > + 0x049c, 0x04d7, 0x053c, 0x053f, 0x0544, 0x057f, > +}; > + >
RE: [PATCH v8 2/5] driver core: add deferring probe reason to devices_deferred property
>-Original Message- >From: dri-devel On Behalf Of >Andrzej Hajda >Sent: Friday, July 10, 2020 11:30 AM >To: Greg Kroah-Hartman >Cc: Jernej Skrabec ; Rafael J. Wysocki >; Jonas Karlman ; Bartlomiej >Zolnierkiewicz ; linux-ker...@vger.kernel.org; >open list:DRM DRIVERS ; Russell King - ARM >Linux ; Neil Armstrong ; >Andrzej Hajda ; andy.shevche...@gmail.com; Mark >Brown ; Laurent Pinchart >; linux-arm- >ker...@lists.infradead.org; Marek Szyprowski > >Subject: [PATCH v8 2/5] driver core: add deferring probe reason to >devices_deferred property > >/sys/kernel/debug/devices_deferred property contains list of deferred >devices. >This list does not contain reason why the driver deferred probe, the patch >improves it. >The natural place to set the reason is dev_err_probe function introduced >recently, ie. if dev_err_probe will be called with -EPROBE_DEFER instead of >printk the message will be attached to a deferred device and printed when >user >reads devices_deferred property. > >Signed-off-by: Andrzej Hajda >Reviewed-by: Mark Brown >Reviewed-by: Javier Martinez Canillas >Reviewed-by: Andy Shevchenko >Reviewed-by: Rafael J. Wysocki >--- >v8: >- improved commit message >--- > drivers/base/base.h | 3 +++ > drivers/base/core.c | 8 ++-- > drivers/base/dd.c | 23 ++- > 3 files changed, 31 insertions(+), 3 deletions(-) > >diff --git a/drivers/base/base.h b/drivers/base/base.h >index 95c22c0f9036..6954fccab3d7 100644 >--- a/drivers/base/base.h >+++ b/drivers/base/base.h >@@ -93,6 +93,7 @@ struct device_private { > struct klist_node knode_class; > struct list_head deferred_probe; > struct device_driver *async_driver; >+ char *deferred_probe_reason; > struct device *device; > u8 dead:1; > }; >@@ -134,6 +135,8 @@ extern void device_release_driver_internal(struct >device *dev, > extern void driver_detach(struct device_driver *drv); > extern int driver_probe_device(struct device_driver *drv, struct device >*dev); > extern void driver_deferred_probe_del(struct device *dev); >+extern void device_set_deferred_probe_reson(const struct device *dev, >+ struct va_format *vaf); > static inline int driver_match_device(struct device_driver *drv, > struct device *dev) > { >diff --git a/drivers/base/core.c b/drivers/base/core.c >index 3a827c82933f..fee047f03681 100644 >--- a/drivers/base/core.c >+++ b/drivers/base/core.c >@@ -3963,6 +3963,8 @@ define_dev_printk_level(_dev_info, KERN_INFO); > * This helper implements common pattern present in probe functions for >error > * checking: print debug or error message depending if the error value is > * -EPROBE_DEFER and propagate error upwards. >+ * In case of -EPROBE_DEFER it sets also defer probe reason, which can be >+ * checked later by reading devices_deferred debugfs attribute. > * It replaces code sequence: > *if (err != -EPROBE_DEFER) > *dev_err(dev, ...); >@@ -3984,10 +3986,12 @@ int dev_err_probe(const struct device *dev, int >err, const char *fmt, ...) > vaf.fmt = fmt; > vaf.va = > >- if (err != -EPROBE_DEFER) >+ if (err != -EPROBE_DEFER) { > dev_err(dev, "error %d: %pV", err, ); >- else >+ } else { >+ device_set_deferred_probe_reson(dev, ); > dev_dbg(dev, "error %d: %pV", err, ); >+ } > > va_end(args); > >diff --git a/drivers/base/dd.c b/drivers/base/dd.c >index 9a1d940342ac..dd5683b61f74 100644 >--- a/drivers/base/dd.c >+++ b/drivers/base/dd.c >@@ -27,6 +27,7 @@ > #include > #include > #include >+#include > > #include "base.h" > #include "power/power.h" >@@ -136,6 +137,8 @@ void driver_deferred_probe_del(struct device *dev) > if (!list_empty(>p->deferred_probe)) { > dev_dbg(dev, "Removed from deferred list\n"); > list_del_init(>p->deferred_probe); >+ kfree(dev->p->deferred_probe_reason); >+ dev->p->deferred_probe_reason = NULL; > } > mutex_unlock(_probe_mutex); > } >@@ -211,6 +214,23 @@ void device_unblock_probing(void) > driver_deferred_probe_trigger(); > } > >+/** >+ * device_set_deferred_probe_reson() - Set defer probe reason message >for device >+ * @dev: the pointer to the struct device >+ * @vaf: the pointer to va_format structure with message >+ */ >+void device_set_deferred_probe_reson(const struct device *dev, struct Is 'reson' supposed to be 'reason'? Same spelling on the above kernel-doc, but the comment says "reason". mike >va_format *vaf) >+{ >+ const char *drv = dev_driver_string(dev); >+ >+ mutex_lock(_probe_mutex); >+ >+ kfree(dev->p->deferred_probe_reason); >+ dev->p->deferred_probe_reason = kasprintf(GFP_KERNEL, "%s: >%pV", drv, vaf); >+ >+ mutex_unlock(_probe_mutex); >+} >+ > /* > * deferred_devs_show() - Show the devices in the deferred probe pending >list. > */ >@@ -221,7 +241,8 @@ static
Re: [PATCH] DRM PANEL DRIVERS: Replace HTTP links with HTTPS ones
On Thu, Jul 09, 2020 at 08:47:55PM +0200, Alexander A. Klimov wrote: > Rationale: > Reduces attack surface on kernel devs opening the links for MITM > as HTTPS traffic is much harder to manipulate. > > Deterministic algorithm: > For each file: > If not .svg: > For each line: > If doesn't contain `\bxmlns\b`: > For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`: > If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`: > If both the HTTP and HTTPS versions > return 200 OK and serve the same content: > Replace HTTP with HTTPS. > > Signed-off-by: Alexander A. Klimov Thanks. Adjusted subject and applied to drm-misc-next. Sam > --- > Continuing my work started at 93431e0607e5. > See also: git log --oneline '--author=Alexander A. Klimov > ' v5.7..master > (Actually letting a shell for loop submit all this stuff for me.) > > If there are any URLs to be removed completely or at least not HTTPSified: > Just clearly say so and I'll *undo my change*. > See also: https://lkml.org/lkml/2020/6/27/64 > > If there are any valid, but yet not changed URLs: > See: https://lkml.org/lkml/2020/6/26/837 > > If you apply the patch, please let me know. > > > drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c > b/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c > index 3a0229d60095..09f6b0cac854 100644 > --- a/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c > +++ b/drivers/gpu/drm/panel/panel-osd-osd101t2587-53ts.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > /* > - * Copyright (C) 2019 Texas Instruments Incorporated - http://www.ti.com > + * Copyright (C) 2019 Texas Instruments Incorporated - https://www.ti.com > * Author: Peter Ujfalusi > */ > > -- > 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v8 0/6] Add support for DisplayPort driver on SnapDragon
Thanks for reviews Rob. On 2020-07-09 13:21, Rob Herring wrote: On Tue, Jun 30, 2020 at 11:45:01AM -0700, Tanmay Shah wrote: These patches add Display-Port driver on SnapDragon/msm hardware. This series also contains device-tree bindings for msm DP driver. It also contains Makefile and Kconfig changes to compile msm DP driver. The block diagram of DP driver is shown below: +-+ |DRM FRAMEWORK| +--+--+ | +v+ | DP DRM | +++ | +v+ ++| DP+--++--+ ++---+| DISPLAY |+---+ | | |++-+-+-+| | | || | | | | | || | | | | | || | | | | | vv v v v v v +--+ +--+ +---+ ++ ++ +---+ +-+ | DP | | DP | |DP | | DP | | DP | |DP | | DP | |PARSER| | HPD | |AUX| |LINK| |CTRL| |PHY| |POWER| +--+---+ +---+--+ +---+ ++ +--+-+ +-+-+ +-+ | | | +--v---+ +v-v+ |DEVICE| | DP | | TREE | |CATALOG| +--+ +---+---+ | +---v+ |CTRL/PHY| | HW | ++ Changes in v7: - Modify cover letter description and fix title. - Introduce dp-controller.yaml for common bindings across SOC - Rename dp-sc7180.yaml to dp-controller-sc7180.yaml for SC7180 bindings - Rename compatible string to qcom,sc7180-dp - Add assigned-clocks and assigned-clock-parents properties in bindings - Remove redundant code from driver - Extend series to include HPD detection logic Changes in v8: - Add MDSS AHB clock in bindings - Replace mode->vrefresh use with drm_mode_vrefresh API - Remove redundant aux config code from parser and aux module - Assign default max lanes if data-lanes property is not available - Fix use-after-free during DP driver remove - Unregister hardware clocks during driver cleanup This series depends-on: https://patchwork.freedesktop.org/patch/366159/ If a single patch is a dependency, please coordinate your work and send as 1 series. To put it another way, I'm just going to ignore this series until the dependency is sorted out. Sure I will wait till previous patch is resolved. https://patchwork.freedesktop.org/patch/369859/ Probably the same goes for this too, but I care less as it's not the binding... Above patch is not compile time dependency, but without it we see issues during runtime. So will be removed from dependency list. Thanks. Chandan Uddaraju (4): dt-bindings: msm/dp: add bindings of DP/DP-PLL driver for Snapdragon drm: add constant N value in helper file drm/msm/dp: add displayPort driver support drm/msm/dp: add support for DP PLL driver Jeykumar Sankaran (1): drm/msm/dpu: add display port support in DPU Tanmay Shah (1): drm/msm/dp: Add Display Port HPD feature .../display/msm/dp-controller-sc7180.yaml | 144 ++ .../bindings/display/msm/dp-controller.yaml | 61 + .../bindings/display/msm/dpu-sc7180.yaml | 11 + drivers/gpu/drm/i915/display/intel_display.c |2 +- drivers/gpu/drm/msm/Kconfig | 16 + drivers/gpu/drm/msm/Makefile | 14 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 29 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c |8 + drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 83 +- drivers/gpu/drm/msm/dp/dp_aux.c | 510 + drivers/gpu/drm/msm/dp/dp_aux.h | 29 + drivers/gpu/drm/msm/dp/dp_catalog.c | 1060 ++ drivers/gpu/drm/msm/dp/dp_catalog.h | 104 + drivers/gpu/drm/msm/dp/dp_ctrl.c | 1707 + drivers/gpu/drm/msm/dp/dp_ctrl.h | 35 + drivers/gpu/drm/msm/dp/dp_display.c | 1017 ++ drivers/gpu/drm/msm/dp/dp_display.h | 31 + drivers/gpu/drm/msm/dp/dp_drm.c | 168 ++ drivers/gpu/drm/msm/dp/dp_drm.h | 18 + drivers/gpu/drm/msm/dp/dp_hpd.c | 69 + drivers/gpu/drm/msm/dp/dp_hpd.h | 79 + drivers/gpu/drm/msm/dp/dp_link.c | 1216 drivers/gpu/drm/msm/dp/dp_link.h | 132 ++ drivers/gpu/drm/msm/dp/dp_panel.c | 490 + drivers/gpu/drm/msm/dp/dp_panel.h | 95 + drivers/gpu/drm/msm/dp/dp_parser.c| 267 +++ drivers/gpu/drm/msm/dp/dp_parser.h| 138 ++ drivers/gpu/drm/msm/dp/dp_pll.c | 99 +
Re: [v1] drm/msm/dpu: add support for clk and bw scaling for display
On Thu, Jun 18, 2020 at 7:09 AM Kalyan Thota wrote: > > This change adds support to scale src clk and bandwidth as > per composition requirements. > > Interconnect registration for bw has been moved to mdp > device node from mdss to facilitate the scaling. > > Changes in v1: > - Address armv7 compilation issues with the patch (Rob) > > Signed-off-by: Kalyan Thota > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 109 > + > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 5 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 4 + > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c| 37 - > drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h| 4 + > drivers/gpu/drm/msm/disp/dpu1/dpu_mdss.c | 9 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 84 +++ > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.h | 4 + > 8 files changed, 233 insertions(+), 23 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > index 7c230f7..e52bc44 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c > @@ -29,6 +29,74 @@ enum dpu_perf_mode { > DPU_PERF_MODE_MAX > }; > > +/** > + * @_dpu_core_perf_calc_bw() - to calculate BW per crtc > + * @kms - pointer to the dpu_kms > + * @crtc - pointer to a crtc > + * Return: returns aggregated BW for all planes in crtc. > + */ > +static u64 _dpu_core_perf_calc_bw(struct dpu_kms *kms, > + struct drm_crtc *crtc) > +{ > + struct drm_plane *plane; > + struct dpu_plane_state *pstate; > + u64 crtc_plane_bw = 0; > + u32 bw_factor; > + > + drm_atomic_crtc_for_each_plane(plane, crtc) { > + pstate = to_dpu_plane_state(plane->state); > + if (!pstate) > + continue; > + > + crtc_plane_bw += pstate->plane_fetch_bw; > + } > + > + bw_factor = kms->catalog->perf.bw_inefficiency_factor; > + if (bw_factor) { > + crtc_plane_bw *= bw_factor; > + do_div(crtc_plane_bw, 100); > + } > + > + return crtc_plane_bw; > +} > + > +/** > + * _dpu_core_perf_calc_clk() - to calculate clock per crtc > + * @kms - pointer to the dpu_kms > + * @crtc - pointer to a crtc > + * @state - pointer to a crtc state > + * Return: returns max clk for all planes in crtc. > + */ > +static u64 _dpu_core_perf_calc_clk(struct dpu_kms *kms, > + struct drm_crtc *crtc, struct drm_crtc_state *state) > +{ > + struct drm_plane *plane; > + struct dpu_plane_state *pstate; > + struct drm_display_mode *mode; > + u64 crtc_clk; > + u32 clk_factor; > + > + mode = >adjusted_mode; > + > + crtc_clk = mode->vtotal * mode->hdisplay * drm_mode_vrefresh(mode); > + > + drm_atomic_crtc_for_each_plane(plane, crtc) { > + pstate = to_dpu_plane_state(plane->state); > + if (!pstate) > + continue; > + > + crtc_clk = max(pstate->plane_clk, crtc_clk); > + } > + > + clk_factor = kms->catalog->perf.clk_inefficiency_factor; > + if (clk_factor) { > + crtc_clk *= clk_factor; > + do_div(crtc_clk, 100); > + } > + > + return crtc_clk; > +} > + > static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc) > { > struct msm_drm_private *priv; > @@ -51,12 +119,7 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms, > dpu_cstate = to_dpu_crtc_state(state); > memset(perf, 0, sizeof(struct dpu_core_perf_params)); > > - if (!dpu_cstate->bw_control) { > - perf->bw_ctl = kms->catalog->perf.max_bw_high * > - 1000ULL; > - perf->max_per_pipe_ib = perf->bw_ctl; > - perf->core_clk_rate = kms->perf.max_core_clk_rate; > - } else if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) { > + if (kms->perf.perf_tune.mode == DPU_PERF_MODE_MINIMUM) { > perf->bw_ctl = 0; > perf->max_per_pipe_ib = 0; > perf->core_clk_rate = 0; > @@ -64,6 +127,10 @@ static void _dpu_core_perf_calc_crtc(struct dpu_kms *kms, > perf->bw_ctl = kms->perf.fix_core_ab_vote; > perf->max_per_pipe_ib = kms->perf.fix_core_ib_vote; > perf->core_clk_rate = kms->perf.fix_core_clk_rate; > + } else { > + perf->bw_ctl = _dpu_core_perf_calc_bw(kms, crtc); > + perf->max_per_pipe_ib = kms->catalog->perf.min_dram_ib; > + perf->core_clk_rate = _dpu_core_perf_calc_clk(kms, crtc, > state); > } > > DPU_DEBUG( > @@ -115,11 +182,7 @@ int dpu_core_perf_crtc_check(struct drm_crtc *crtc, > DPU_DEBUG("crtc:%d bw:%llu ctrl:%d\n", >
Re: [PATCH] drm/vkms: add wait_for_vblanks in atomic_commit_tail
On 07/02, Daniel Vetter wrote: > On Wed, Jul 01, 2020 at 03:31:34PM +, Sidong Yang wrote: > > there is an error when igt test is run continuously. > > vkms_atomic_commit_tail() > > need to call drm_atomic_helper_wait_for_vblanks() for give up ownership of > > vblank events. without this code, next atomic commit will not enable vblank > > and raise timeout error. > > > > Signed-off-by: Sidong Yang > > --- > > drivers/gpu/drm/vkms/vkms_drv.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c > > b/drivers/gpu/drm/vkms/vkms_drv.c > > index 1e8b2169d834..10b9be67a068 100644 > > --- a/drivers/gpu/drm/vkms/vkms_drv.c > > +++ b/drivers/gpu/drm/vkms/vkms_drv.c > > @@ -93,6 +93,8 @@ static void vkms_atomic_commit_tail(struct > > drm_atomic_state *old_state) > > flush_work(_state->composer_work); > > } > > > > + drm_atomic_helper_wait_for_vblanks(dev, old_state); > > Uh, we have a wait_for_flip_done right above, which should be doing > exactly the same, but more precisely: Instead of just waiting for any > vblank to happen, we wait for exactly the vblank corresponding to this > atomic commit. So no races possible. If this is papering over some issue, > then I think more debugging is needed. > > What exactly is going wrong here for you? Hi Daniel and Sidong, I noticed a similar issue when running the IGT test kms_cursor_crc. For example, a subtest that passes on the first run (alpha-opaque) fails on the second due to a kind of busy waiting in subtest preparation (the subtest fails before actually running). In addition, in the same test, the dpms subtest started to fail since the commit that change from wait_for_vblanks to wait_for_flip_done. By reverting this commit, the dpms subtest passes again and the sequential subtests return to normal. I am trying to figure out what's missing from using flip_done op on vkms, since I am also interested in solving this problem and I understand that the change for flip_done has been discussed in the past. Do you have any idea? Melissa > -Daniel > > > + > > drm_atomic_helper_cleanup_planes(dev, old_state); > > } > > > > -- > > 2.17.1 > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 205815] amdgpu: When playing a wine game, got black screen then screen flickers, game crashes and back to normal
https://bugzilla.kernel.org/show_bug.cgi?id=205815 --- Comment #3 from Bernard MAUDRY (ramaspaces...@free.fr) --- Using latest kernel: juil. 10 12:39:47 Rama kernel: Linux version 5.7.0-1-amd64 (debian-ker...@lists.debian.org) (gcc version 9.3.0 (Debian 9.3.0-14), GNU ld (GNU Binutils for Debian) 2.34) #1 SMP Debian 5.7.6-1 (2020-06-24) using latest firmware from https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git: -rw-r--r-- 1 bernard bernard 160256 juin 30 00:37 amdgpu/picasso_asd.bin -rw-r--r-- 1 bernard bernard 9344 juin 30 00:37 amdgpu/picasso_ce.bin -rw-r--r-- 1 bernard bernard316 déc. 14 2019 amdgpu/picasso_gpu_info.bin -rw-r--r-- 1 bernard bernard 17536 juin 30 00:37 amdgpu/picasso_me.bin -rw-r--r-- 1 bernard bernard 268048 juin 30 00:37 amdgpu/picasso_mec2.bin -rw-r--r-- 1 bernard bernard 268048 juin 30 00:37 amdgpu/picasso_mec.bin -rw-r--r-- 1 bernard bernard 21632 juin 30 00:37 amdgpu/picasso_pfp.bin -rw-r--r-- 1 bernard bernard 39140 déc. 14 2019 amdgpu/picasso_rlc_am4.bin -rw-r--r-- 1 bernard bernard 39140 juin 30 00:37 amdgpu/picasso_rlc.bin -rw-r--r-- 1 bernard bernard 17408 déc. 14 2019 amdgpu/picasso_sdma.bin -rw-r--r-- 1 bernard bernard 29440 juin 30 00:37 amdgpu/picasso_ta.bin -rw-r--r-- 1 bernard bernard 355264 janv. 20 17:55 amdgpu/picasso_vcn.bin using latest BIOS from MSI: 7B86vAD2(Beta version) 2020-06-12 - Updated AMD AGESA ComboAm4PI 1.0.0.6 Log of the crash: juil. 10 16:56:17 Rama kernel: [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out! juil. 10 16:56:22 Rama kernel: [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out! juil. 10 16:56:22 Rama kernel: [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out! juil. 10 16:56:22 Rama kernel: [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for fences timed out! juil. 10 16:56:22 Rama kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=768498, emitted seq=768500 juil. 10 16:56:22 Rama kernel: [drm:amdgpu_job_timedout [amdgpu]] *ERROR* Process information: process Homeworld2.exe pid 20994 thread Homeworld2:cs0 pid 21034 juil. 10 16:56:22 Rama kernel: amdgpu :27:00.0: GPU reset begin! juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: GPU reset succeeded, trying to resume juil. 10 16:56:23 Rama kernel: [drm] PCIE GART of 1024M enabled (table at 0x00F40090). juil. 10 16:56:23 Rama kernel: [drm] PSP is resuming... juil. 10 16:56:23 Rama kernel: [drm] reserve 0x40 from 0xf40f80 for PSP TMR juil. 10 16:56:23 Rama kernel: [drm] psp command (0x5) failed and response status is (0x0007) juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: RAS: optional ras ta ucode is not available juil. 10 16:56:23 Rama kernel: [drm] kiq ring mec 2 pipe 1 q 0 juil. 10 16:56:23 Rama kernel: [drm] VCN decode and encode initialized successfully(under DPG Mode). juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring gfx uses VM inv eng 0 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring comp_1.0.0 uses VM inv eng 1 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring comp_1.1.0 uses VM inv eng 4 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring comp_1.2.0 uses VM inv eng 5 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring comp_1.3.0 uses VM inv eng 6 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring comp_1.0.1 uses VM inv eng 7 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring comp_1.1.1 uses VM inv eng 8 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring comp_1.2.1 uses VM inv eng 9 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring comp_1.3.1 uses VM inv eng 10 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring kiq_2.1.0 uses VM inv eng 11 on hub 0 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring sdma0 uses VM inv eng 0 on hub 1 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring vcn_dec uses VM inv eng 1 on hub 1 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring vcn_enc0 uses VM inv eng 4 on hub 1 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring vcn_enc1 uses VM inv eng 5 on hub 1 juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: ring jpeg_dec uses VM inv eng 6 on hub 1 juil. 10 16:56:23 Rama kernel: [drm] recover vram bo from shadow start juil. 10 16:56:23 Rama kernel: [drm] recover vram bo from shadow done juil. 10 16:56:23 Rama kernel: [drm] Skip scheduling IBs! juil. 10 16:56:23 Rama kernel: [drm] Skip scheduling IBs! juil. 10 16:56:23 Rama kernel: amdgpu :27:00.0: GPU reset(2) succeeded! juil. 10 16:56:23 Rama kernel: [drm] Skip scheduling IBs! juil. 10 16:56:23 Rama kernel: [drm:amdgpu_cs_ioctl [amdgpu]] *ERROR* Failed to initialize parser -125! juil. 10 16:56:23 Rama kernel: [drm] Skip scheduling IBs! juil. 10 16:56:23 Rama kernel: [drm] Skip scheduling IBs! juil. 10 16:56:23 Rama
Re: [v1] drm/msm/dpu: enumerate second cursor pipe for external interface
On Thu, Jun 25, 2020 at 5:46 AM Kalyan Thota wrote: > > Setup an RGB HW pipe as cursor which can be used on > secondary interface. > > For SC7180 2 HW pipes are enumerated as cursors > 1 - primary interface > 2 - secondary interface > > Signed-off-by: Kalyan Thota > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 12 ++-- > 1 file changed, 6 insertions(+), 6 deletions(-) > > 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 8f2357d..23061fd 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c > @@ -117,10 +117,10 @@ > .reg_off = 0x2AC, .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}, > + .reg_off = 0x2B4, .bit_off = 8}, > + .clk_ctrls[DPU_CLK_CTRL_CURSOR1] = { > + .reg_off = 0x2C4, .bit_off = 8}, It looks like you shifted the register offset here from 0x2bc to 0x2c4, was that intentional? BR, -R > }, > }; > > @@ -272,10 +272,10 @@ > sc7180_vig_sblk_0, 0, SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0), > SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, DMA_SDM845_MASK, > sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0), > - SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_SDM845_MASK, > - sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1), > + SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, DMA_CURSOR_SDM845_MASK, > + sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0), > SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, DMA_CURSOR_SDM845_MASK, > - sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0), > + sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1), > }; > > /* > -- > 1.9.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 208513] Radeon RX480 graphics freeze with RIP: 0010:amdgpu_dm_atomic_commit_tail+0x273/0x1100 [amdgpu]
https://bugzilla.kernel.org/show_bug.cgi?id=208513 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #3 from Alex Deucher (alexdeuc...@gmail.com) --- Is this a regression? If so, can you bisect? Possibly a duplicate of: https://bugzilla.kernel.org/show_bug.cgi?id=207383 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/vkms: change the max cursor width/height
This change expands the coverage for the IGT kms_cursor_crc test, where the size varies between 64 and 512 for a square cursor. With this, in addition to the cursor 64x64, this patch enables the test of cursors with sizes: 128x128, 256x256, and 512x512. Signed-off-by: Melissa Wen --- drivers/gpu/drm/vkms/vkms_drv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c index 1e8b2169d834..57a8a397d5e8 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.c +++ b/drivers/gpu/drm/vkms/vkms_drv.c @@ -133,6 +133,8 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev) dev->mode_config.min_height = YRES_MIN; dev->mode_config.max_width = XRES_MAX; dev->mode_config.max_height = YRES_MAX; + dev->mode_config.cursor_width = 512; + dev->mode_config.cursor_height = 512; dev->mode_config.preferred_depth = 24; dev->mode_config.helper_private = _mode_config_helpers; -- 2.27.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 4/5] drm/bridge: lvds-codec: simplify error handling
Using dev_err_probe code has following advantages: - shorter code, - recorded defer probe reason for debugging, - uniform error code logging. Signed-off-by: Andrzej Hajda Reviewed-by: Neil Armstrong --- drivers/gpu/drm/bridge/lvds-codec.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/bridge/lvds-codec.c b/drivers/gpu/drm/bridge/lvds-codec.c index 24fb1befdfa2..f19d9f7a5db2 100644 --- a/drivers/gpu/drm/bridge/lvds-codec.c +++ b/drivers/gpu/drm/bridge/lvds-codec.c @@ -71,13 +71,9 @@ static int lvds_codec_probe(struct platform_device *pdev) lvds_codec->connector_type = (uintptr_t)of_device_get_match_data(dev); lvds_codec->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown", GPIOD_OUT_HIGH); - if (IS_ERR(lvds_codec->powerdown_gpio)) { - int err = PTR_ERR(lvds_codec->powerdown_gpio); - - if (err != -EPROBE_DEFER) - dev_err(dev, "powerdown GPIO failure: %d\n", err); - return err; - } + if (IS_ERR(lvds_codec->powerdown_gpio)) + return dev_err_probe(dev, PTR_ERR(lvds_codec->powerdown_gpio), +"powerdown GPIO failure\n"); /* Locate the panel DT node. */ panel_node = of_graph_get_remote_node(dev->of_node, 1, 0); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 2/5] driver core: add deferring probe reason to devices_deferred property
/sys/kernel/debug/devices_deferred property contains list of deferred devices. This list does not contain reason why the driver deferred probe, the patch improves it. The natural place to set the reason is dev_err_probe function introduced recently, ie. if dev_err_probe will be called with -EPROBE_DEFER instead of printk the message will be attached to a deferred device and printed when user reads devices_deferred property. Signed-off-by: Andrzej Hajda Reviewed-by: Mark Brown Reviewed-by: Javier Martinez Canillas Reviewed-by: Andy Shevchenko Reviewed-by: Rafael J. Wysocki --- v8: - improved commit message --- drivers/base/base.h | 3 +++ drivers/base/core.c | 8 ++-- drivers/base/dd.c | 23 ++- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/drivers/base/base.h b/drivers/base/base.h index 95c22c0f9036..6954fccab3d7 100644 --- a/drivers/base/base.h +++ b/drivers/base/base.h @@ -93,6 +93,7 @@ struct device_private { struct klist_node knode_class; struct list_head deferred_probe; struct device_driver *async_driver; + char *deferred_probe_reason; struct device *device; u8 dead:1; }; @@ -134,6 +135,8 @@ extern void device_release_driver_internal(struct device *dev, extern void driver_detach(struct device_driver *drv); extern int driver_probe_device(struct device_driver *drv, struct device *dev); extern void driver_deferred_probe_del(struct device *dev); +extern void device_set_deferred_probe_reson(const struct device *dev, + struct va_format *vaf); static inline int driver_match_device(struct device_driver *drv, struct device *dev) { diff --git a/drivers/base/core.c b/drivers/base/core.c index 3a827c82933f..fee047f03681 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -3963,6 +3963,8 @@ define_dev_printk_level(_dev_info, KERN_INFO); * This helper implements common pattern present in probe functions for error * checking: print debug or error message depending if the error value is * -EPROBE_DEFER and propagate error upwards. + * In case of -EPROBE_DEFER it sets also defer probe reason, which can be + * checked later by reading devices_deferred debugfs attribute. * It replaces code sequence: * if (err != -EPROBE_DEFER) * dev_err(dev, ...); @@ -3984,10 +3986,12 @@ int dev_err_probe(const struct device *dev, int err, const char *fmt, ...) vaf.fmt = fmt; vaf.va = - if (err != -EPROBE_DEFER) + if (err != -EPROBE_DEFER) { dev_err(dev, "error %d: %pV", err, ); - else + } else { + device_set_deferred_probe_reson(dev, ); dev_dbg(dev, "error %d: %pV", err, ); + } va_end(args); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index 9a1d940342ac..dd5683b61f74 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -27,6 +27,7 @@ #include #include #include +#include #include "base.h" #include "power/power.h" @@ -136,6 +137,8 @@ void driver_deferred_probe_del(struct device *dev) if (!list_empty(>p->deferred_probe)) { dev_dbg(dev, "Removed from deferred list\n"); list_del_init(>p->deferred_probe); + kfree(dev->p->deferred_probe_reason); + dev->p->deferred_probe_reason = NULL; } mutex_unlock(_probe_mutex); } @@ -211,6 +214,23 @@ void device_unblock_probing(void) driver_deferred_probe_trigger(); } +/** + * device_set_deferred_probe_reson() - Set defer probe reason message for device + * @dev: the pointer to the struct device + * @vaf: the pointer to va_format structure with message + */ +void device_set_deferred_probe_reson(const struct device *dev, struct va_format *vaf) +{ + const char *drv = dev_driver_string(dev); + + mutex_lock(_probe_mutex); + + kfree(dev->p->deferred_probe_reason); + dev->p->deferred_probe_reason = kasprintf(GFP_KERNEL, "%s: %pV", drv, vaf); + + mutex_unlock(_probe_mutex); +} + /* * deferred_devs_show() - Show the devices in the deferred probe pending list. */ @@ -221,7 +241,8 @@ static int deferred_devs_show(struct seq_file *s, void *data) mutex_lock(_probe_mutex); list_for_each_entry(curr, _probe_pending_list, deferred_probe) - seq_printf(s, "%s\n", dev_name(curr->device)); + seq_printf(s, "%s\t%s", dev_name(curr->device), + curr->device->p->deferred_probe_reason ?: "\n"); mutex_unlock(_probe_mutex); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 1/5] driver core: add device probe log helper
During probe every time driver gets resource it should usually check for error printk some message if it is not -EPROBE_DEFER and return the error. This pattern is simple but requires adding few lines after any resource acquisition code, as a result it is often omitted or implemented only partially. dev_err_probe helps to replace such code sequences with simple call, so code: if (err != -EPROBE_DEFER) dev_err(dev, ...); return err; becomes: return dev_err_probe(dev, err, ...); Signed-off-by: Andrzej Hajda Reviewed-by: Rafael J. Wysocki Reviewed-by: Mark Brown --- drivers/base/core.c| 42 ++ include/linux/device.h | 3 +++ 2 files changed, 45 insertions(+) diff --git a/drivers/base/core.c b/drivers/base/core.c index 67d39a90b45c..3a827c82933f 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -3953,6 +3953,48 @@ define_dev_printk_level(_dev_info, KERN_INFO); #endif +/** + * dev_err_probe - probe error check and log helper + * @dev: the pointer to the struct device + * @err: error value to test + * @fmt: printf-style format string + * @...: arguments as specified in the format string + * + * This helper implements common pattern present in probe functions for error + * checking: print debug or error message depending if the error value is + * -EPROBE_DEFER and propagate error upwards. + * It replaces code sequence: + * if (err != -EPROBE_DEFER) + * dev_err(dev, ...); + * else + * dev_dbg(dev, ...); + * return err; + * with + * return dev_err_probe(dev, err, ...); + * + * Returns @err. + * + */ +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...) +{ + struct va_format vaf; + va_list args; + + va_start(args, fmt); + vaf.fmt = fmt; + vaf.va = + + if (err != -EPROBE_DEFER) + dev_err(dev, "error %d: %pV", err, ); + else + dev_dbg(dev, "error %d: %pV", err, ); + + va_end(args); + + return err; +} +EXPORT_SYMBOL_GPL(dev_err_probe); + static inline bool fwnode_is_primary(struct fwnode_handle *fwnode) { return fwnode && !IS_ERR(fwnode->secondary); diff --git a/include/linux/device.h b/include/linux/device.h index 15460a5ac024..6b2272ae9af8 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -964,6 +964,9 @@ void device_link_remove(void *consumer, struct device *supplier); void device_links_supplier_sync_state_pause(void); void device_links_supplier_sync_state_resume(void); +extern __printf(3, 4) +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...); + /* Create alias, so I can be autoloaded. */ #define MODULE_ALIAS_CHARDEV(major,minor) \ MODULE_ALIAS("char-major-" __stringify(major) "-" __stringify(minor)) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 0/5] driver core: add probe error check helper
Hi All, Thanks for comments. Changes since v7: - improved commit message - added R-Bs Changes since v6: - removed leftovers from old naming scheme in commit descritions, - added R-Bs. Changes since v5: - removed patch adding macro, dev_err_probe(dev, PTR_ERR(ptr), ...) should be used instead, - added dev_dbg logging in case of -EPROBE_DEFER, - renamed functions and vars according to comments, - extended docs, - cosmetics. Original message (with small adjustments): Recently I took some time to re-check error handling in drivers probe code, and I have noticed that number of incorrect resource acquisition error handling increased and there are no other propositions which can cure the situation. So I have decided to resend my old proposition of probe_err helper which should simplify resource acquisition error handling, it also extend it with adding defer probe reason to devices_deferred debugfs property, which should improve debugging experience for developers/testers. I have also added two patches showing usage and benefits of the helper. My dirty/ad-hoc cocci scripts shows that this helper can be used in at least 2700 places saving about 3500 lines of code. Regards Andrzej Andrzej Hajda (5): driver core: add device probe log helper driver core: add deferring probe reason to devices_deferred property drm/bridge/sii8620: fix resource acquisition error handling drm/bridge: lvds-codec: simplify error handling coccinelle: add script looking for cases where probe__err can be used drivers/base/base.h | 3 + drivers/base/core.c | 46 + drivers/base/dd.c| 23 ++- drivers/gpu/drm/bridge/lvds-codec.c | 10 +- drivers/gpu/drm/bridge/sil-sii8620.c | 21 +-- include/linux/device.h | 3 + probe_err.cocci | 247 +++ 7 files changed, 333 insertions(+), 20 deletions(-) create mode 100644 probe_err.cocci -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 3/5] drm/bridge/sii8620: fix resource acquisition error handling
In case of error during resource acquisition driver should print error message only in case it is not deferred probe, using dev_err_probe helper solves the issue. Moreover it records defer probe reason for debugging. Signed-off-by: Andrzej Hajda Reviewed-by: Neil Armstrong --- drivers/gpu/drm/bridge/sil-sii8620.c | 21 + 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c index 92acd336aa89..389c1f029774 100644 --- a/drivers/gpu/drm/bridge/sil-sii8620.c +++ b/drivers/gpu/drm/bridge/sil-sii8620.c @@ -2299,10 +2299,9 @@ static int sii8620_probe(struct i2c_client *client, INIT_LIST_HEAD(>mt_queue); ctx->clk_xtal = devm_clk_get(dev, "xtal"); - if (IS_ERR(ctx->clk_xtal)) { - dev_err(dev, "failed to get xtal clock from DT\n"); - return PTR_ERR(ctx->clk_xtal); - } + if (IS_ERR(ctx->clk_xtal)) + return dev_err_probe(dev, PTR_ERR(ctx->clk_xtal), +"failed to get xtal clock from DT\n"); if (!client->irq) { dev_err(dev, "no irq provided\n"); @@ -2313,16 +2312,14 @@ static int sii8620_probe(struct i2c_client *client, sii8620_irq_thread, IRQF_TRIGGER_HIGH | IRQF_ONESHOT, "sii8620", ctx); - if (ret < 0) { - dev_err(dev, "failed to install IRQ handler\n"); - return ret; - } + if (ret < 0) + return dev_err_probe(dev, ret, +"failed to install IRQ handler\n"); ctx->gpio_reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH); - if (IS_ERR(ctx->gpio_reset)) { - dev_err(dev, "failed to get reset gpio from DT\n"); - return PTR_ERR(ctx->gpio_reset); - } + if (IS_ERR(ctx->gpio_reset)) + return dev_err_probe(dev, PTR_ERR(ctx->gpio_reset), +"failed to get reset gpio from DT\n"); ctx->supplies[0].supply = "cvcc10"; ctx->supplies[1].supply = "iovcc18"; -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v8 5/5] coccinelle: add script looking for cases where probe__err can be used
This is AD-HOC script, it should nt be merged. Signed-off-by: Andrzej Hajda --- probe_err.cocci | 247 1 file changed, 247 insertions(+) create mode 100644 probe_err.cocci diff --git a/probe_err.cocci b/probe_err.cocci new file mode 100644 index ..0ef9a9b4c9bc --- /dev/null +++ b/probe_err.cocci @@ -0,0 +1,247 @@ +virtual context +virtual patch + +@initialize:python@ +@@ + +import re + +@@ +expression err, dev; +constant char [] fmt; +expression list args; +@@ + +- if (err != -EPROBE_DEFER) { dev_err(dev, fmt, args); } ++ dev_err_probe(dev, err, fmt, args); + +@@ +expression ptr, dev; +constant char [] fmt; +expression list args; +@@ + +- if (ptr != ERR_PTR(-EPROBE_DEFER)) { dev_err(dev, fmt, args); } ++ dev_err_probe(dev, PTR_ERR(ptr), fmt, args); + +@@ +expression e, dev; +identifier err; +identifier fget =~ "^(devm_)?(clk_get|gpiod_get|gpiod_get_optional|gpiod_get_index|gpiod_get_index_optional|regulator_get|regulator_get_optional|reset_control_get|reset_control_get_exclusive|reset_control_get_shared|phy_get|pinctrl_get|iio_channel_get|pwm_get)$"; +identifier flog =~ "^(dev_err|dev_warn|dev_info)$"; +expression list args; +@@ +e = fget(...); +if (IS_ERR(e)) { +( + err = PTR_ERR(e); +- flog(dev, args); ++ dev_err_probe(dev, err, args); +| +- flog(dev, args); ++ dev_err_probe(dev, PTR_ERR(e), args); +) + ... +} + +@@ +expression dev; +identifier err; +identifier fget =~ "^(devm_)?(request_irq|request_threaded_irq|regulator_bulk_get)$"; +identifier flog =~ "^(dev_err|dev_warn|dev_info)$"; +expression list args; +@@ +err = fget(...); +if ( \( err \| err < 0 \) ) { + ... +- flog(dev, args); ++ dev_err_probe(dev, err, args); + ... +} + +@catch_no_nl@ +expression dev, err; +constant char [] fmt !~ "\\n$"; +@@ +dev_err_probe(dev, err, fmt, ...) + +@script:python add_nl depends on catch_no_nl@ +fmt << catch_no_nl.fmt; +nfmt; +@@ +print "add_nl " + fmt +coccinelle.nfmt = fmt[:-1] + '\\n"'; + +@fix_no_nl depends on catch_no_nl@ +constant char [] catch_no_nl.fmt; +identifier add_nl.nfmt; +@@ +- fmt ++ nfmt + +@catch_fmt@ +expression err, dev; +expression fmt; +position p; +@@ + +dev_err_probe@p(dev, err, fmt, ..., \( (int)err \| err \) ) + +@script:python trim_fmt@ +fmt << catch_fmt.fmt; +new_fmt; +@@ + +tmp = fmt +tmp = re.sub('failed: irq request (IRQ: %d, error :%d)', 'irq request %d', tmp) +tmp = re.sub('Error %l?[di] ', 'Error ', tmp) +tmp = re.sub(' as irq = %dn', ', bad irqn', tmp) +tmp = re.sub('[:,]? ?((ret|err|with|error)[ =]?)?%l?[di]\.?n', 'n', tmp) +tmp = re.sub(' ?\(((err|ret|error)\s*=?\s*)?%l?[diu]\)[!.]?n', 'n', tmp) + +assert tmp != fmt, "cannot trim_fmt in: " + fmt +print "trim_fmt " + fmt + " " + tmp +coccinelle.new_fmt = tmp + +@fix_fmt@ +expression err, err1, dev; +expression fmt; +expression list l; +identifier trim_fmt.new_fmt; +position catch_fmt.p; +@@ + +- dev_err_probe@p(dev, err, fmt, l, err1) ++ dev_err_probe(dev, err, new_fmt, l) + +@err_ass1@ +identifier err; +expression dev, ptr; +expression list args; +@@ + +- err = PTR_ERR(ptr); +- dev_err_probe(dev, err, args); +- return ERR_PTR(err); ++ dev_err_probe(dev, PTR_ERR(ptr), args); ++ return ERR_CAST(ptr); + +@err_ass2@ +identifier err, f1, f2; +expression dev, e; +expression list args; +@@ +- err = PTR_ERR(e); +- dev_err_probe(dev, err, args); +( +| +f1(...); +| +f1(...); +f2(...); +) +- return err; ++ return dev_err_probe(dev, PTR_ERR(e), args); + +@@ +identifier err; +expression dev, e; +expression list args; +@@ + +- int err = e; +- dev_err_probe(dev, err, args); +- return err; ++ return dev_err_probe(dev, e, args); + +@@ +expression err, dev; +expression list args; +@@ + +- dev_err_probe(dev, err, args); +- return err; ++ return dev_err_probe(dev, err, args); + +@@ +expression err, dev, ptr; +expression list args; +@@ + +- dev_err_probe(dev, PTR_ERR(ptr), args); +err = PTR_ERR(ptr); ++ dev_err_probe(dev, err, args); + +@@ +expression e; +expression list args; +statement s, s1; +@@ + +// without s1 spatch generates extra empty line after s +- if (e) { return dev_err_probe(args); } else s s1 ++ if (e) return dev_err_probe(args); s s1 + +@@ +expression e; +expression list args; +@@ + +- if (e) { return dev_err_probe(args); } ++ if (e) return dev_err_probe(args); + +@@ +expression e, s, v; +expression list args; +@@ + +- if (e == v) { s; } else { return dev_err_probe(args); } ++ if (e != v) return dev_err_probe(args); s; + +@err_ass3@ +identifier err; +expression dev, ptr; +expression list args; +@@ + +- err = PTR_ERR_OR_ZERO(ptr); +- if (err) return dev_err_probe(dev, err, args); ++ if (IS_ERR(ptr)) return dev_err_probe(dev, PTR_ERR(ptr), args); + +@@ +expression dev, ptr; +expression list args; +@@ + +- DROP_probe_err(dev, PTR_ERR(ptr), args) ++ probe_err(dev, ptr,
Re: [PATCH v8 2/5] driver core: add deferring probe reason to devices_deferred property
On 10.07.2020 15:31, Greg Kroah-Hartman wrote: > On Thu, Jul 02, 2020 at 03:44:21PM +0200, Andrzej Hajda wrote: >> /sys/kernel/debug/devices_deferred property contains list of deferred >> devices. >> This list does not contain reason why the driver deferred probe, the patch >> improves it. >> The natural place to set the reason is dev_err_probe function introduced >> recently, ie. if dev_err_probe will be called with -EPROBE_DEFER instead of >> printk the message will be attached to a deferred device and printed when >> user >> reads devices_deferred property. >> >> Signed-off-by: Andrzej Hajda >> Reviewed-by: Mark Brown >> Reviewed-by: Javier Martinez Canillas >> Reviewed-by: Andy Shevchenko >> Reviewed-by: Rafael J. Wysocki >> --- >> v8: >> - improved commit message > I'm totally confused by this series. Can you resend the whole thing, > as a full series, not just random individual patches in the series > incremented? It's a pain to try to fish them all out as to which is the > "latest" with all of the needed reviewed by lines :( v7 is the latest except this one,which contains only commit message change. Anyway I will send v8 to make things simple. Regards Andrzej > > thanks, > > greg k-h > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://protect2.fireeye.com/v1/url?k=563dadd0-0bf16175-563c269f-0cc47a30d446-7237066d193b28b5=1=54779b9e-347e-4d0c-9845-da31d4cce7e4=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/tilcdc: Use standard drm_atomic_helper_commit
Thank you Daniel, Now this works perfectly, all while I was on vacation. On 08/07/2020 17:20, Daniel Vetter wrote: > Gives us proper nonblocking support for free, and a pile of other > things. The tilcdc code is simply old enough that it was never > converted over, but was stuck forever with the copypasta from when it > was initially merged. > > The riskiest thing with this conversion is maybe that there's an issue > with the vblank handling or vblank event handling, which will upset > the modern commit support in atomic helpers. But from a cursory review > drm_crtc_vblank_on/off is called in the right places, and the event > handling also seems to exist (albeit with much hand-rolling and > probably some races, could perhaps be converted over to > drm_crtc_arm_vblank_event without any real loss). > > Motivated by me not having to hand-roll the dma-fence annotations for > this. > > v2: Clear out crtc_state->event when we're handling the event, to > avoid upsetting the helpers (reported by Jyri). > > v3: Also send out even whent the crtc is getting disabled. Tilcdc looks a > bit like conversion to simple display helpers would work out really > nice. > Probably. Should take a closer looks some day when I have time. > Signed-off-by: Daniel Vetter > Cc: Jyri Sarha > Cc: Tomi Valkeinen Tested-by: Jyri Sarha Reviewed-by: Jyri Sarha > -- > From logs looks like we're not stuck when disabling the display, so I > hacked in a bit of code for that too. Like mentioned above, tilcdc > looks like a perfect candidate for simple display helpers, I think > that would simplify a _lot_ of code here. > -Daniel > --- > drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 13 > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 47 +-- > drivers/gpu/drm/tilcdc/tilcdc_plane.c | 8 +++-- > 3 files changed, 19 insertions(+), 49 deletions(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > index e9dd5e5cb4e7..1856962411c7 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c > @@ -537,6 +537,18 @@ static void tilcdc_crtc_atomic_disable(struct drm_crtc > *crtc, > tilcdc_crtc_disable(crtc); > } > > +static void tilcdc_crtc_atomic_flush(struct drm_crtc *crtc, > + struct drm_crtc_state *old_state) > +{ > + if (!crtc->state->event) > + return; > + > + spin_lock_irq(>dev->event_lock); > + drm_crtc_send_vblank_event(crtc, crtc->state->event); > + crtc->state->event = NULL; > + spin_unlock_irq(>dev->event_lock); > +} > + > void tilcdc_crtc_shutdown(struct drm_crtc *crtc) > { > tilcdc_crtc_off(crtc, true); > @@ -822,6 +834,7 @@ static const struct drm_crtc_helper_funcs > tilcdc_crtc_helper_funcs = { > .atomic_check = tilcdc_crtc_atomic_check, > .atomic_enable = tilcdc_crtc_atomic_enable, > .atomic_disable = tilcdc_crtc_atomic_disable, > + .atomic_flush = tilcdc_crtc_atomic_flush, > }; > > void tilcdc_crtc_set_panel_info(struct drm_crtc *crtc, > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > index 0d74a6443263..4f5fc3e87383 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > @@ -87,55 +87,10 @@ static int tilcdc_atomic_check(struct drm_device *dev, > return ret; > } > > -static int tilcdc_commit(struct drm_device *dev, > - struct drm_atomic_state *state, > - bool async) > -{ > - int ret; > - > - ret = drm_atomic_helper_prepare_planes(dev, state); > - if (ret) > - return ret; > - > - ret = drm_atomic_helper_swap_state(state, true); > - if (ret) { > - drm_atomic_helper_cleanup_planes(dev, state); > - return ret; > - } > - > - /* > - * Everything below can be run asynchronously without the need to grab > - * any modeset locks at all under one condition: It must be guaranteed > - * that the asynchronous work has either been cancelled (if the driver > - * supports it, which at least requires that the framebuffers get > - * cleaned up with drm_atomic_helper_cleanup_planes()) or completed > - * before the new state gets committed on the software side with > - * drm_atomic_helper_swap_state(). > - * > - * This scheme allows new atomic state updates to be prepared and > - * checked in parallel to the asynchronous completion of the previous > - * update. Which is important since compositors need to figure out the > - * composition of the next frame right after having submitted the > - * current layout. > - */ > - > - drm_atomic_helper_commit_modeset_disables(dev, state); > - > - drm_atomic_helper_commit_planes(dev, state, 0); > - > - drm_atomic_helper_commit_modeset_enables(dev, state); > - > - drm_atomic_helper_wait_for_vblanks(dev, state); > - > -
Re: [PATCH v3 6/9] drm/bridge: ti-sn65dsi86: Use 18-bit DP if we can
Hi, On Thu, Jul 9, 2020 at 11:15 PM Steev Klimaszewski wrote: > > Hi, > > On 7/9/20 11:12 PM, Doug Anderson wrote: > >> root@c630:~# bus=$(i2cdetect -l | grep sn65 | sed > >> 's/i2c-\([0-9]*\).*$/\1/') > >> root@c630:~# i2cdump ${bus} 0x50 i > edid > >> WARNING! This program can confuse your I2C bus, cause data loss and worse! > >> I will probe file /dev/i2c-16, address 0x50, mode i2c block > >> Continue? [Y/n] > >> root@c630:~# edid-decode edid > >> edid-decode (hex): > >> > >> 00 ff ff ff ff ff ff 00 09 e5 d1 07 00 00 00 00 > >> 01 1c 01 04 a5 1d 11 78 0a 1d b0 a6 58 54 9e 26 > >> 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 > >> 01 01 01 01 01 01 c0 39 80 18 71 38 28 40 30 20 > >> 36 00 26 a5 10 00 00 1a 00 00 00 00 00 00 00 00 > >> 00 00 00 00 00 00 00 00 00 1a 00 00 00 fe 00 42 > >> 4f 45 20 43 51 0a 20 20 20 20 20 20 00 00 00 fe > >> 00 4e 56 31 33 33 46 48 4d 2d 4e 36 31 0a 00 9a > >> > >> 03 26 0a 77 ab 1c 05 71 6f 1d 8c f1 43 ce 6a bb > >> fb d3 11 20 39 07 22 6e 65 68 77 70 d3 05 34 73 > >> 44 21 8b fd f5 6d 11 62 94 2a 7c fa 93 ba 6a 61 > >> 92 da 15 53 4c 39 eb f7 86 23 97 48 e9 39 09 d2 > >> 66 02 70 bb e2 77 0f 4a a3 a0 4c 72 6e 5d 47 70 > >> 43 c2 13 f3 b2 d9 b9 78 02 be 41 82 15 6a 28 dc > >> 45 0f 9d eb 0f 2a cc e8 35 8d 34 7f 3e 84 5e a3 > >> 30 5e 1e 29 0a 48 0c d1 0a c4 08 31 03 a9 3b 29 > >> > >> > >> > >> EDID version: 1.4 > >> Manufacturer: BOE Model 2001 Serial Number 0 > >> Made in week 1 of 2018 > >> Digital display > >> 8 bits per primary color channel > >> DisplayPort interface > >> Maximum image size: 29 cm x 17 cm > >> Gamma: 2.20 > >> Supported color formats: RGB 4:4:4, YCrCb 4:4:4 > >> First detailed timing includes the native pixel format and preferred > >> refresh rate > >> Color Characteristics > >> Red: 0.6484, 0.3447 > >> Green: 0.3310, 0.6181 > >> Blue: 0.1503, 0.0615 > >> White: 0.3125, 0.3281 > >> Established Timings I & II: none > >> Standard Timings: none > >> Detailed mode: Clock 147.840 MHz, 294 mm x 165 mm > >> 1920 1968 2000 2200 ( 48 32 200) > >> 1080 1083 1089 1120 ( 3 6 31) > >> +hsync -vsync > >> VertFreq: 60.000 Hz, HorFreq: 67.200 kHz > >> Manufacturer-Specified Display Descriptor (0x00): 00 00 00 00 00 00 00 > >> 00 00 00 00 00 00 00 00 1a > >> Alphanumeric Data String: BOE CQ > >> Alphanumeric Data String: NV133FHM-N61 > >> Checksum: 0x9a > >> > >> > >> > >> Unknown EDID Extension Block 0x03 > >> 03 26 0a 77 ab 1c 05 71 6f 1d 8c f1 43 ce 6a bb .&.w...qo...C.j. > >> fb d3 11 20 39 07 22 6e 65 68 77 70 d3 05 34 73 ... 9."nehwp..4s > >> 44 21 8b fd f5 6d 11 62 94 2a 7c fa 93 ba 6a 61 D!...m.b.*|...ja > >> 92 da 15 53 4c 39 eb f7 86 23 97 48 e9 39 09 d2 ...SL9...#.H.9.. > >> 66 02 70 bb e2 77 0f 4a a3 a0 4c 72 6e 5d 47 70 f.p..w.J..Lrn]Gp > >> 43 c2 13 f3 b2 d9 b9 78 02 be 41 82 15 6a 28 dc C..x..A..j(. > >> 45 0f 9d eb 0f 2a cc e8 35 8d 34 7f 3e 84 5e a3 E*..5.4.>.^. > >> 30 5e 1e 29 0a 48 0c d1 0a c4 08 31 03 a9 3b 29 0^.).H.1..;) > >> Checksum: 0x29 (should be 0x82) > >> > >> > >> - My edid does in fact say it's 8bit > > Crazy! Mine: > > > > Extracted contents: > > header: 00 ff ff ff ff ff ff 00 > > serial number: 09 e5 2d 08 00 00 00 00 23 1c > > version: 01 04 > > basic params:95 1d 11 78 02 > > chroma info: d5 00 a6 58 54 9f 27 0f 4f 57 > > established: 00 00 00 > > standard:01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 > > descriptor 1:c0 39 80 18 71 38 28 40 30 20 36 00 26 a5 10 00 00 1a > > descriptor 2:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > descriptor 3:00 00 00 fe 00 42 4f 45 20 43 51 0a 20 20 20 20 20 20 > > descriptor 4:00 00 00 fe 00 4e 56 31 33 33 46 48 4d 2d 4e 36 32 0a > > extensions: 00 > > checksum:40 > > > > Manufacturer: BOE Model 82d Serial Number 0 > > Made week 35 of 2018 > > EDID version: 1.4 > > Digital display > > 6 bits per primary color channel > > DisplayPort interface > > Maximum image size: 29 cm x 17 cm > > Gamma: 2.20 > > Supported color formats: RGB 4:4:4 > > First detailed timing is preferred timing > > Established timings supported: > > Standard timings supported: > > Detailed mode: Clock 147.840 MHz, 294 mm x 165 mm > > 1920 1968 2000 2200 hborder 0 > > 1080 1083 1089 1120 vborder 0 > > +hsync -vsync > > Manufacturer-specified data, tag 0 > > ASCII string: BOE > > ASCII string: NV133FHM-N62 > > Checksum: 0x40 (valid) > > > > Unknown extension block > > > > EDID block does NOT conform to EDID 1.3! > > Missing name descriptor > > Missing monitor ranges > > Detailed block string not properly terminated > > EDID block does not conform at all! > > Has 128 nonconformant extension block(s) > > I did attempt to modify the patch, and I don't think I did it
[Bug 207383] [Regression] 5.7 amdgpu/polaris11 gpf: amdgpu_atomic_commit_tail
https://bugzilla.kernel.org/show_bug.cgi?id=207383 --- Comment #58 from Anthony Ruhier (anthony.ruh...@gmail.com) --- Sorry, I forgot to say that I have a vega64. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] efi: avoid error message when booting under Xen
On Fri, 10 Jul 2020 at 17:24, Juergen Gross wrote: > > efifb_probe() will issue an error message in case the kernel is booted > as Xen dom0 from UEFI as EFI_MEMMAP won't be set in this case. Avoid > that message by calling efi_mem_desc_lookup() only if EFI_MEMMAP is set. > > Fixes: 38ac0287b7f4 ("fbdev/efifb: Honour UEFI memory map attributes when > mapping the FB") > Signed-off-by: Juergen Gross Acked-by: Ard Biesheuvel > --- > drivers/video/fbdev/efifb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c > index 65491ae74808..e57c00824965 100644 > --- a/drivers/video/fbdev/efifb.c > +++ b/drivers/video/fbdev/efifb.c > @@ -453,7 +453,7 @@ static int efifb_probe(struct platform_device *dev) > info->apertures->ranges[0].base = efifb_fix.smem_start; > info->apertures->ranges[0].size = size_remap; > > - if (efi_enabled(EFI_BOOT) && > + if (efi_enabled(EFI_MEMMAP) && > !efi_mem_desc_lookup(efifb_fix.smem_start, )) { > if ((efifb_fix.smem_start + efifb_fix.smem_len) > > (md.phys_addr + (md.num_pages << EFI_PAGE_SHIFT))) { > -- > 2.26.2 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: fbdev: vt8623fb: Constify static vga_regsets
On 7/1/20 11:02 PM, Rikard Falkeborn wrote: > These are not modified so make them const to allow the compiler to put > them in read-only memory. > > Before: >textdata bss dec hex filename > 255097928 64 3350182dd drivers/video/fbdev/vt8623fb.o > > After: >textdata bss dec hex filename > 265336904 64 3350182dd drivers/video/fbdev/vt8623fb.o > > Signed-off-by: Rikard Falkeborn Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/vt8623fb.c | 36 +- > 1 file changed, 18 insertions(+), 18 deletions(-) > > diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c > index 7b3eef1b893f..98ff8235c9e9 100644 > --- a/drivers/video/fbdev/vt8623fb.c > +++ b/drivers/video/fbdev/vt8623fb.c > @@ -62,24 +62,24 @@ static const struct svga_pll vt8623_pll = {2, 127, 2, 7, > 0, 3, > > /* CRT timing register sets */ > > -static struct vga_regset vt8623_h_total_regs[] = {{0x00, 0, 7}, {0x36, > 3, 3}, VGA_REGSET_END}; > -static struct vga_regset vt8623_h_display_regs[] = {{0x01, 0, 7}, > VGA_REGSET_END}; > -static struct vga_regset vt8623_h_blank_start_regs[] = {{0x02, 0, 7}, > VGA_REGSET_END}; > -static struct vga_regset vt8623_h_blank_end_regs[] = {{0x03, 0, 4}, {0x05, > 7, 7}, {0x33, 5, 5}, VGA_REGSET_END}; > -static struct vga_regset vt8623_h_sync_start_regs[] = {{0x04, 0, 7}, {0x33, > 4, 4}, VGA_REGSET_END}; > -static struct vga_regset vt8623_h_sync_end_regs[]= {{0x05, 0, 4}, > VGA_REGSET_END}; > - > -static struct vga_regset vt8623_v_total_regs[] = {{0x06, 0, 7}, {0x07, > 0, 0}, {0x07, 5, 5}, {0x35, 0, 0}, VGA_REGSET_END}; > -static struct vga_regset vt8623_v_display_regs[] = {{0x12, 0, 7}, {0x07, > 1, 1}, {0x07, 6, 6}, {0x35, 2, 2}, VGA_REGSET_END}; > -static struct vga_regset vt8623_v_blank_start_regs[] = {{0x15, 0, 7}, {0x07, > 3, 3}, {0x09, 5, 5}, {0x35, 3, 3}, VGA_REGSET_END}; > -static struct vga_regset vt8623_v_blank_end_regs[] = {{0x16, 0, 7}, > VGA_REGSET_END}; > -static struct vga_regset vt8623_v_sync_start_regs[] = {{0x10, 0, 7}, {0x07, > 2, 2}, {0x07, 7, 7}, {0x35, 1, 1}, VGA_REGSET_END}; > -static struct vga_regset vt8623_v_sync_end_regs[]= {{0x11, 0, 3}, > VGA_REGSET_END}; > - > -static struct vga_regset vt8623_offset_regs[]= {{0x13, 0, 7}, {0x35, > 5, 7}, VGA_REGSET_END}; > -static struct vga_regset vt8623_line_compare_regs[] = {{0x18, 0, 7}, {0x07, > 4, 4}, {0x09, 6, 6}, {0x33, 0, 2}, {0x35, 4, 4}, VGA_REGSET_END}; > -static struct vga_regset vt8623_fetch_count_regs[] = {{0x1C, 0, 7}, {0x1D, > 0, 1}, VGA_REGSET_END}; > -static struct vga_regset vt8623_start_address_regs[] = {{0x0d, 0, 7}, {0x0c, > 0, 7}, {0x34, 0, 7}, {0x48, 0, 1}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_h_total_regs[] = {{0x00, 0, 7}, > {0x36, 3, 3}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_h_display_regs[] = {{0x01, 0, 7}, > VGA_REGSET_END}; > +static const struct vga_regset vt8623_h_blank_start_regs[] = {{0x02, 0, 7}, > VGA_REGSET_END}; > +static const struct vga_regset vt8623_h_blank_end_regs[] = {{0x03, 0, 4}, > {0x05, 7, 7}, {0x33, 5, 5}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_h_sync_start_regs[] = {{0x04, 0, 7}, > {0x33, 4, 4}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_h_sync_end_regs[]= {{0x05, 0, 4}, > VGA_REGSET_END}; > + > +static const struct vga_regset vt8623_v_total_regs[] = {{0x06, 0, 7}, > {0x07, 0, 0}, {0x07, 5, 5}, {0x35, 0, 0}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_v_display_regs[] = {{0x12, 0, 7}, > {0x07, 1, 1}, {0x07, 6, 6}, {0x35, 2, 2}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_v_blank_start_regs[] = {{0x15, 0, 7}, > {0x07, 3, 3}, {0x09, 5, 5}, {0x35, 3, 3}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_v_blank_end_regs[] = {{0x16, 0, 7}, > VGA_REGSET_END}; > +static const struct vga_regset vt8623_v_sync_start_regs[] = {{0x10, 0, 7}, > {0x07, 2, 2}, {0x07, 7, 7}, {0x35, 1, 1}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_v_sync_end_regs[]= {{0x11, 0, 3}, > VGA_REGSET_END}; > + > +static const struct vga_regset vt8623_offset_regs[]= {{0x13, 0, 7}, > {0x35, 5, 7}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_line_compare_regs[] = {{0x18, 0, 7}, > {0x07, 4, 4}, {0x09, 6, 6}, {0x33, 0, 2}, {0x35, 4, 4}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_fetch_count_regs[] = {{0x1C, 0, 7}, > {0x1D, 0, 1}, VGA_REGSET_END}; > +static const struct vga_regset vt8623_start_address_regs[] = {{0x0d, 0, 7}, > {0x0c, 0, 7}, {0x34, 0, 7}, {0x48, 0, 1}, VGA_REGSET_END}; > > static const struct svga_timing_regs vt8623_timing_regs = { > vt8623_h_total_regs, vt8623_h_display_regs, vt8623_h_blank_start_regs, >
Re: [PATCH] fbdev: sm712fb: set error code in probe
On 7/6/20 5:53 PM, Evgeny Novikov wrote: > If smtcfb_pci_probe() does not detect a valid chip it cleans up > everything and returns 0. This can result in various bad things later. > The patch sets the error code on the corresponding path. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Evgeny Novikov Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/sm712fb.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c > index 6a1b4a853d9e..fbe97340b8e0 100644 > --- a/drivers/video/fbdev/sm712fb.c > +++ b/drivers/video/fbdev/sm712fb.c > @@ -1614,7 +1614,7 @@ static int smtcfb_pci_probe(struct pci_dev *pdev, > default: > dev_err(>dev, > "No valid Silicon Motion display chip was detected!\n"); > - > + err = -ENODEV; > goto failed_fb; > } > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] efi: avoid error message when booting under Xen
efifb_probe() will issue an error message in case the kernel is booted as Xen dom0 from UEFI as EFI_MEMMAP won't be set in this case. Avoid that message by calling efi_mem_desc_lookup() only if EFI_MEMMAP is set. Fixes: 38ac0287b7f4 ("fbdev/efifb: Honour UEFI memory map attributes when mapping the FB") Signed-off-by: Juergen Gross --- drivers/video/fbdev/efifb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c index 65491ae74808..e57c00824965 100644 --- a/drivers/video/fbdev/efifb.c +++ b/drivers/video/fbdev/efifb.c @@ -453,7 +453,7 @@ static int efifb_probe(struct platform_device *dev) info->apertures->ranges[0].base = efifb_fix.smem_start; info->apertures->ranges[0].size = size_remap; - if (efi_enabled(EFI_BOOT) && + if (efi_enabled(EFI_MEMMAP) && !efi_mem_desc_lookup(efifb_fix.smem_start, )) { if ((efifb_fix.smem_start + efifb_fix.smem_len) > (md.phys_addr + (md.num_pages << EFI_PAGE_SHIFT))) { -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] fbdev/fb.h: Use struct_size() helper in kzalloc()
On 6/20/20 1:27 PM, Sam Ravnborg wrote: > Hi Gustavo. > > On Wed, Jun 17, 2020 at 12:56:47PM -0500, Gustavo A. R. Silva wrote: >> Make use of the struct_size() helper instead of an open-coded version >> in order to avoid any potential type mistakes. >> >> This code was detected with the help of Coccinelle and, audited and >> fixed manually. >> >> Signed-off-by: Gustavo A. R. Silva > > struct_size is defined in overflow.h - which is not included by fs.h. > So we rely on overflow.h being pulled in by some other header - maybe > slab.h in this case. > Seems fragile, should this patch add an include of overflow.h? $ git grep struct_size drivers/|wc -l 697 $ git grep overflow\\.h drivers/|wc -l 8 $ git grep overflow\\.h include/linux/ include/linux/device.h:#include include/linux/mm.h:#include include/linux/slab.h:#include include/linux/vmalloc.h:#include so I've applied the patch as it is (hoping that the issue is so widespread that no-one tries to remove overflow.h from slab.h without fixing drivers at the same time).. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > Sam > >> --- >> include/linux/fb.h | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/fb.h b/include/linux/fb.h >> index 3b4b2f0c6994..2b530e6d86e4 100644 >> --- a/include/linux/fb.h >> +++ b/include/linux/fb.h >> @@ -506,8 +506,9 @@ struct fb_info { >> }; >> >> static inline struct apertures_struct *alloc_apertures(unsigned int >> max_num) { >> -struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct) >> -+ max_num * sizeof(struct aperture), GFP_KERNEL); >> +struct apertures_struct *a; >> + >> +a = kzalloc(struct_size(a, ranges, max_num), GFP_KERNEL); >> if (!a) >> return NULL; >> a->count = max_num; >> -- >> 2.27.0 >> >> ___ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://protect2.fireeye.com/url?k=7bae4d09-26604cda-7bafc646-000babff317b-7eab3a2caa4b8b73=1=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: fbdev: neofb: fix memory leak in neo_scan_monitor()
On 6/30/20 9:54 PM, Evgeny Novikov wrote: > neofb_probe() calls neo_scan_monitor() that can successfully allocate a > memory for info->monspecs.modedb and proceed to case 0x03. There it does > not free the memory and returns -1. neofb_probe() goes to label > err_scan_monitor, thus, it does not free this memory through calling > fb_destroy_modedb() as well. We can not go to label err_init_hw since > neo_scan_monitor() can fail during memory allocation. So, the patch frees > the memory directly for case 0x03. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Evgeny Novikov Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/neofb.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/video/fbdev/neofb.c b/drivers/video/fbdev/neofb.c > index f5a676bfd67a..09a20d4ab35f 100644 > --- a/drivers/video/fbdev/neofb.c > +++ b/drivers/video/fbdev/neofb.c > @@ -1819,6 +1819,7 @@ static int neo_scan_monitor(struct fb_info *info) > #else > printk(KERN_ERR > "neofb: Only 640x480, 800x600/480 and 1024x768 panels > are currently supported\n"); > + kfree(info->monspecs.modedb); > return -1; > #endif > default: > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] omapfb: fix multiple reference count leaks due to pm_runtime_get_sync
On 6/14/20 5:05 AM, Aditya Pakki wrote: > On calling pm_runtime_get_sync() the reference count of the device > is incremented. In case of failure, decrement the > reference count before returning the error. > > Signed-off-by: Aditya Pakki Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/omap2/omapfb/dss/dispc.c | 7 +-- > drivers/video/fbdev/omap2/omapfb/dss/dsi.c | 7 +-- > drivers/video/fbdev/omap2/omapfb/dss/dss.c | 7 +-- > drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c | 5 +++-- > drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c | 5 +++-- > drivers/video/fbdev/omap2/omapfb/dss/venc.c | 7 +-- > 6 files changed, 26 insertions(+), 12 deletions(-) > > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c > b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c > index 4a16798b2ecd..e2b572761bf6 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c > @@ -520,8 +520,11 @@ int dispc_runtime_get(void) > DSSDBG("dispc_runtime_get\n"); > > r = pm_runtime_get_sync(>dev); > - WARN_ON(r < 0); > - return r < 0 ? r : 0; > + if (WARN_ON(r < 0)) { > + pm_runtime_put_sync(>dev); > + return r; > + } > + return 0; > } > EXPORT_SYMBOL(dispc_runtime_get); > > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c > b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c > index d620376216e1..6f9c25fec994 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/dsi.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dsi.c > @@ -1137,8 +1137,11 @@ static int dsi_runtime_get(struct platform_device > *dsidev) > DSSDBG("dsi_runtime_get\n"); > > r = pm_runtime_get_sync(>pdev->dev); > - WARN_ON(r < 0); > - return r < 0 ? r : 0; > + if (WARN_ON(r < 0)) { > + pm_runtime_put_sync(>pdev->dev); > + return r; > + } > + return 0; > } > > static void dsi_runtime_put(struct platform_device *dsidev) > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c > b/drivers/video/fbdev/omap2/omapfb/dss/dss.c > index 7252d22dd117..3586579c838f 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c > @@ -768,8 +768,11 @@ int dss_runtime_get(void) > DSSDBG("dss_runtime_get\n"); > > r = pm_runtime_get_sync(>dev); > - WARN_ON(r < 0); > - return r < 0 ? r : 0; > + if (WARN_ON(r < 0)) { > + pm_runtime_put_sync(>dev); > + return r; > + } > + return 0; > } > > void dss_runtime_put(void) > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c > b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c > index 7060ae56c062..4804aab34298 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi4.c > @@ -39,9 +39,10 @@ static int hdmi_runtime_get(void) > DSSDBG("hdmi_runtime_get\n"); > > r = pm_runtime_get_sync(>dev); > - WARN_ON(r < 0); > - if (r < 0) > + if (WARN_ON(r < 0)) { > + pm_runtime_put_sync(>dev); > return r; > + } > > return 0; > } > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c > b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c > index ac49531e4732..a06b6f1355bd 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi5.c > @@ -43,9 +43,10 @@ static int hdmi_runtime_get(void) > DSSDBG("hdmi_runtime_get\n"); > > r = pm_runtime_get_sync(>dev); > - WARN_ON(r < 0); > - if (r < 0) > + if (WARN_ON(r < 0)) { > + pm_runtime_put_sync(>dev); > return r; > + } > > return 0; > } > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/venc.c > b/drivers/video/fbdev/omap2/omapfb/dss/venc.c > index d5404d56c922..0b0ad20afd63 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/venc.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/venc.c > @@ -348,8 +348,11 @@ static int venc_runtime_get(void) > DSSDBG("venc_runtime_get\n"); > > r = pm_runtime_get_sync(>dev); > - WARN_ON(r < 0); > - return r < 0 ? r : 0; > + if (WARN_ON(r < 0)) { > + pm_runtime_put_sync(>dev); > + return r; > + } > + return 0; > } > > static void venc_runtime_put(void) > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] fbdev: da8xx-fb: go to proper label on error handling paths in probe
On 7/2/20 6:05 PM, Evgeny Novikov wrote: > fb_probe() can successfully allocate a new frame buffer, but then fail > to perform some operations with regulator. In these cases fb_probe() > goes to label err_pm_runtime_disable where the frame buffer is not > released. The patch makes fb_probe() to go to label err_release_fb on > corresponding error handling paths. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Evgeny Novikov Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/da8xx-fb.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/fbdev/da8xx-fb.c b/drivers/video/fbdev/da8xx-fb.c > index 73c3c4c8cc12..e38c0e3f9c61 100644 > --- a/drivers/video/fbdev/da8xx-fb.c > +++ b/drivers/video/fbdev/da8xx-fb.c > @@ -1402,14 +1402,14 @@ static int fb_probe(struct platform_device *device) > if (IS_ERR(par->lcd_supply)) { > if (PTR_ERR(par->lcd_supply) == -EPROBE_DEFER) { > ret = -EPROBE_DEFER; > - goto err_pm_runtime_disable; > + goto err_release_fb; > } > > par->lcd_supply = NULL; > } else { > ret = regulator_enable(par->lcd_supply); > if (ret) > - goto err_pm_runtime_disable; > + goto err_release_fb; > } > > fb_videomode_to_var(_fb_var, lcdc_info); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] omapfb: dss: Fix max fclk divider for omap36xx
On 6/30/20 8:26 PM, Adam Ford wrote: > The drm/omap driver was fixed to correct an issue where using a > divider of 32 breaks the DSS despite the TRM stating 32 is a valid > number. Through experimentation, it appears that 31 works, and > it is consistent with the value used by the drm/omap driver. > > This patch fixes the divider for fbdev driver instead of the drm. > > Fixes: f76ee892a99e ("omapfb: copy omapdss & displays for omapfb") > > Cc: #4.9+ > Signed-off-by: Adam Ford Applied to drm-misc-next tree, thanks. (I marked patch as applicable to stable 4.5+ while merging) Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > Linux 4.4 will need a similar patch, but it doesn't apply cleanly. > > The DRM version of this same fix is: > e2c4ed148cf3 ("drm/omap: fix max fclk divider for omap36xx") > > > diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c > b/drivers/video/fbdev/omap2/omapfb/dss/dss.c > index 7252d22dd117..bfc5c4c5a26a 100644 > --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c > +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c > @@ -833,7 +833,7 @@ static const struct dss_features omap34xx_dss_feats = { > }; > > static const struct dss_features omap3630_dss_feats = { > - .fck_div_max= 32, > + .fck_div_max= 31, > .dss_fck_multiplier = 1, > .parent_clk_name= "dpll4_ck", > .dpi_select_source = _dpi_select_source_omap2_omap3, > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: fbdev: savage: fix memory leak on error handling path in probe
On 6/19/20 6:21 PM, Evgeny Novikov wrote: > savagefb_probe() calls savage_init_fb_info() that can successfully > allocate memory for info->pixmap.addr but then fail when > fb_alloc_cmap() fails. savagefb_probe() goes to label failed_init and > does not free allocated memory. It is not valid to go to label > failed_mmio since savage_init_fb_info() can fail during memory > allocation as well. So, the patch free allocated memory on the error > handling path in savage_init_fb_info() itself. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Evgeny Novikov Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/savage/savagefb_driver.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/video/fbdev/savage/savagefb_driver.c > b/drivers/video/fbdev/savage/savagefb_driver.c > index 3c8ae87f0ea7..3fd87aeb6c79 100644 > --- a/drivers/video/fbdev/savage/savagefb_driver.c > +++ b/drivers/video/fbdev/savage/savagefb_driver.c > @@ -2157,6 +2157,8 @@ static int savage_init_fb_info(struct fb_info *info, > struct pci_dev *dev, > info->flags |= FBINFO_HWACCEL_COPYAREA | > FBINFO_HWACCEL_FILLRECT | > FBINFO_HWACCEL_IMAGEBLIT; > + else > + kfree(info->pixmap.addr); > } > #endif > return err; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] fbcon: Use array3_size() helper in scr_memcpyw()
On 6/16/20 1:15 AM, Gustavo A. R. Silva wrote: > Use array3_size() helper instead of the open-coded version in scr_memcpyw() > and scr_memsetw(). These sorts of multiplication factors need to be wrapped > in array3_size(). > > This issue was found with the help of Coccinelle and, audited and fixed > manually. > > Addresses-KSPP-ID: https://github.com/KSPP/linux/issues/83 > Signed-off-by: Gustavo A. R. Silva Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > drivers/video/fbdev/core/fbcon.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/fbdev/core/fbcon.c > b/drivers/video/fbdev/core/fbcon.c > index 9d28a8e3328f..6af2734f2a7b 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -639,7 +639,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct > fb_info *info, > GFP_KERNEL); > if (save) { > int i = cols < new_cols ? cols : new_cols; > - scr_memsetw(save, erase, logo_lines * new_cols * 2); > + scr_memsetw(save, erase, array3_size(logo_lines, > new_cols, 2)); > r = q - step; > for (cnt = 0; cnt < logo_lines; cnt++, r += i) > scr_memcpyw(save + cnt * new_cols, r, 2 * i); > @@ -676,7 +676,7 @@ static void fbcon_prepare_logo(struct vc_data *vc, struct > fb_info *info, > q = (unsigned short *) (vc->vc_origin + > vc->vc_size_row * > rows); > - scr_memcpyw(q, save, logo_lines * new_cols * 2); > + scr_memcpyw(q, save, array3_size(logo_lines, new_cols, 2)); > vc->vc_y += logo_lines; > vc->vc_pos += logo_lines * vc->vc_size_row; > kfree(save); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] video: fbdev: amifb: add FIXME about dead APUS support
On 6/2/20 2:03 PM, Geert Uytterhoeven wrote: > On Tue, Jun 2, 2020 at 1:50 PM Bartlomiej Zolnierkiewicz > wrote: >> On 5/14/20 10:21 PM, Geert Uytterhoeven wrote: >>> These #ifdefs are relics from APUS (Amiga Power-Up System), which >>> added a PPC board. APUS support was killed off a long time ago, >>> when arch/ppc/ was still king, but these #ifdefs were missed, because >>> they didn't test for CONFIG_APUS. >> >> Add FIXME about using the C code variants (APUS ones) in the future. >> >> Reported-by: Al Viro >> Reported-by: Geert Uytterhoeven >> Signed-off-by: Bartlomiej Zolnierkiewicz > > Reviewed-by: Geert Uytterhoeven Applied to drm-misc-next tree. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] video: fbdev: ssd1307fb: Added support to Column offset
[ added dri-devel ML to Cc: ] Hi, Sorry for the delay. On 5/13/20 8:48 PM, Rodrigo Alencar wrote: > From: Rodrigo Rolim Mendes de Alencar > > This patch provides support for displays like VGM128064B0W10, > which requires a column offset of 2, i.e., its segments starts > in SEG2 and ends in SEG129. > > Signed-off-by: Rodrigo Alencar <455.rodrigo.alen...@gmail.com> Please resend with "From:" & "Signed-off-by:" tags fixed to match so I can merge the patch. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > Documentation/devicetree/bindings/display/ssd1307fb.txt | 1 + > drivers/video/fbdev/ssd1307fb.c | 8 ++-- > 2 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/ssd1307fb.txt > b/Documentation/devicetree/bindings/display/ssd1307fb.txt > index 27333b9551b3..2dcb6d12d137 100644 > --- a/Documentation/devicetree/bindings/display/ssd1307fb.txt > +++ b/Documentation/devicetree/bindings/display/ssd1307fb.txt > @@ -19,6 +19,7 @@ Optional properties: >- vbat-supply: The supply for VBAT >- solomon,segment-no-remap: Display needs normal (non-inverted) data column >to segment mapping > + - solomon,col-offset: Offset of columns (COL/SEG) that the screen is > mapped to. >- solomon,com-seq: Display uses sequential COM pin configuration >- solomon,com-lrremap: Display uses left-right COM pin remap >- solomon,com-invdir: Display uses inverted COM pin scan direction > diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c > index 8e06ba912d60..102f941104c0 100644 > --- a/drivers/video/fbdev/ssd1307fb.c > +++ b/drivers/video/fbdev/ssd1307fb.c > @@ -74,6 +74,7 @@ struct ssd1307fb_par { > struct fb_info *info; > u8 lookup_table[4]; > u32 page_offset; > + u32 col_offset; > u32 prechargep1; > u32 prechargep2; > struct pwm_device *pwm; > @@ -458,11 +459,11 @@ static int ssd1307fb_init(struct ssd1307fb_par *par) > if (ret < 0) > return ret; > > - ret = ssd1307fb_write_cmd(par->client, 0x0); > + ret = ssd1307fb_write_cmd(par->client, par->col_offset); > if (ret < 0) > return ret; > > - ret = ssd1307fb_write_cmd(par->client, par->width - 1); > + ret = ssd1307fb_write_cmd(par->client, par->col_offset + par->width - > 1); > if (ret < 0) > return ret; > > @@ -626,6 +627,9 @@ static int ssd1307fb_probe(struct i2c_client *client) > if (device_property_read_u32(dev, "solomon,page-offset", > >page_offset)) > par->page_offset = 1; > > + if (device_property_read_u32(dev, "solomon,col-offset", > >col_offset)) > + par->col_offset = 0; > + > if (device_property_read_u32(dev, "solomon,com-offset", > >com_offset)) > par->com_offset = 0; > > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 2/2] video: fbdev: amifb: add FIXMEs about {put,get}_user() failures
On 6/2/20 2:03 PM, Geert Uytterhoeven wrote: > On Tue, Jun 2, 2020 at 1:52 PM Bartlomiej Zolnierkiewicz > wrote: >> Since we lack the hardware (or proper emulator setup) for >> testing needed changes add FIXMEs to document the issues >> (so at least they are not forgotten). >> >> Cc: Al Viro >> Cc: Geert Uytterhoeven >> Signed-off-by: Bartlomiej Zolnierkiewicz > > Reviewed-by: Geert Uytterhoeven Applied to drm-misc-next tree. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] fbdev/fb.h: Use struct_size() helper in kzalloc()
On 6/17/20 7:56 PM, Gustavo A. R. Silva wrote: > Make use of the struct_size() helper instead of an open-coded version > in order to avoid any potential type mistakes. > > This code was detected with the help of Coccinelle and, audited and > fixed manually. > > Signed-off-by: Gustavo A. R. Silva Applied to drm-misc-next tree, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R Institute Poland Samsung Electronics > --- > include/linux/fb.h | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/include/linux/fb.h b/include/linux/fb.h > index 3b4b2f0c6994..2b530e6d86e4 100644 > --- a/include/linux/fb.h > +++ b/include/linux/fb.h > @@ -506,8 +506,9 @@ struct fb_info { > }; > > static inline struct apertures_struct *alloc_apertures(unsigned int max_num) > { > - struct apertures_struct *a = kzalloc(sizeof(struct apertures_struct) > - + max_num * sizeof(struct aperture), GFP_KERNEL); > + struct apertures_struct *a; > + > + a = kzalloc(struct_size(a, ranges, max_num), GFP_KERNEL); > if (!a) > return NULL; > a->count = max_num; > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 6/9] drm/bridge: ti-sn65dsi86: Use 18-bit DP if we can
On Thu, Jul 9, 2020 at 11:15 PM Steev Klimaszewski wrote: > > Hi, > > On 7/9/20 11:12 PM, Doug Anderson wrote: > >> root@c630:~# bus=$(i2cdetect -l | grep sn65 | sed > >> 's/i2c-\([0-9]*\).*$/\1/') > >> root@c630:~# i2cdump ${bus} 0x50 i > edid > >> WARNING! This program can confuse your I2C bus, cause data loss and worse! > >> I will probe file /dev/i2c-16, address 0x50, mode i2c block > >> Continue? [Y/n] > >> root@c630:~# edid-decode edid > >> edid-decode (hex): > >> > >> 00 ff ff ff ff ff ff 00 09 e5 d1 07 00 00 00 00 > >> 01 1c 01 04 a5 1d 11 78 0a 1d b0 a6 58 54 9e 26 > >> 0f 50 54 00 00 00 01 01 01 01 01 01 01 01 01 01 > >> 01 01 01 01 01 01 c0 39 80 18 71 38 28 40 30 20 > >> 36 00 26 a5 10 00 00 1a 00 00 00 00 00 00 00 00 > >> 00 00 00 00 00 00 00 00 00 1a 00 00 00 fe 00 42 > >> 4f 45 20 43 51 0a 20 20 20 20 20 20 00 00 00 fe > >> 00 4e 56 31 33 33 46 48 4d 2d 4e 36 31 0a 00 9a > >> > >> 03 26 0a 77 ab 1c 05 71 6f 1d 8c f1 43 ce 6a bb > >> fb d3 11 20 39 07 22 6e 65 68 77 70 d3 05 34 73 > >> 44 21 8b fd f5 6d 11 62 94 2a 7c fa 93 ba 6a 61 > >> 92 da 15 53 4c 39 eb f7 86 23 97 48 e9 39 09 d2 > >> 66 02 70 bb e2 77 0f 4a a3 a0 4c 72 6e 5d 47 70 > >> 43 c2 13 f3 b2 d9 b9 78 02 be 41 82 15 6a 28 dc > >> 45 0f 9d eb 0f 2a cc e8 35 8d 34 7f 3e 84 5e a3 > >> 30 5e 1e 29 0a 48 0c d1 0a c4 08 31 03 a9 3b 29 > >> > >> > >> > >> EDID version: 1.4 > >> Manufacturer: BOE Model 2001 Serial Number 0 > >> Made in week 1 of 2018 > >> Digital display > >> 8 bits per primary color channel > >> DisplayPort interface > >> Maximum image size: 29 cm x 17 cm > >> Gamma: 2.20 > >> Supported color formats: RGB 4:4:4, YCrCb 4:4:4 > >> First detailed timing includes the native pixel format and preferred > >> refresh rate > >> Color Characteristics > >> Red: 0.6484, 0.3447 > >> Green: 0.3310, 0.6181 > >> Blue: 0.1503, 0.0615 > >> White: 0.3125, 0.3281 > >> Established Timings I & II: none > >> Standard Timings: none > >> Detailed mode: Clock 147.840 MHz, 294 mm x 165 mm > >> 1920 1968 2000 2200 ( 48 32 200) > >> 1080 1083 1089 1120 ( 3 6 31) > >> +hsync -vsync > >> VertFreq: 60.000 Hz, HorFreq: 67.200 kHz > >> Manufacturer-Specified Display Descriptor (0x00): 00 00 00 00 00 00 00 > >> 00 00 00 00 00 00 00 00 1a > >> Alphanumeric Data String: BOE CQ > >> Alphanumeric Data String: NV133FHM-N61 > >> Checksum: 0x9a > >> > >> > >> > >> Unknown EDID Extension Block 0x03 > >> 03 26 0a 77 ab 1c 05 71 6f 1d 8c f1 43 ce 6a bb .&.w...qo...C.j. > >> fb d3 11 20 39 07 22 6e 65 68 77 70 d3 05 34 73 ... 9."nehwp..4s > >> 44 21 8b fd f5 6d 11 62 94 2a 7c fa 93 ba 6a 61 D!...m.b.*|...ja > >> 92 da 15 53 4c 39 eb f7 86 23 97 48 e9 39 09 d2 ...SL9...#.H.9.. > >> 66 02 70 bb e2 77 0f 4a a3 a0 4c 72 6e 5d 47 70 f.p..w.J..Lrn]Gp > >> 43 c2 13 f3 b2 d9 b9 78 02 be 41 82 15 6a 28 dc C..x..A..j(. > >> 45 0f 9d eb 0f 2a cc e8 35 8d 34 7f 3e 84 5e a3 E*..5.4.>.^. > >> 30 5e 1e 29 0a 48 0c d1 0a c4 08 31 03 a9 3b 29 0^.).H.1..;) > >> Checksum: 0x29 (should be 0x82) > >> > >> > >> - My edid does in fact say it's 8bit > > Crazy! Mine: > > > > Extracted contents: > > header: 00 ff ff ff ff ff ff 00 > > serial number: 09 e5 2d 08 00 00 00 00 23 1c > > version: 01 04 > > basic params:95 1d 11 78 02 > > chroma info: d5 00 a6 58 54 9f 27 0f 4f 57 > > established: 00 00 00 > > standard:01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 01 > > descriptor 1:c0 39 80 18 71 38 28 40 30 20 36 00 26 a5 10 00 00 1a > > descriptor 2:00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > > descriptor 3:00 00 00 fe 00 42 4f 45 20 43 51 0a 20 20 20 20 20 20 > > descriptor 4:00 00 00 fe 00 4e 56 31 33 33 46 48 4d 2d 4e 36 32 0a > > extensions: 00 > > checksum:40 > > > > Manufacturer: BOE Model 82d Serial Number 0 > > Made week 35 of 2018 > > EDID version: 1.4 > > Digital display > > 6 bits per primary color channel > > DisplayPort interface > > Maximum image size: 29 cm x 17 cm > > Gamma: 2.20 > > Supported color formats: RGB 4:4:4 > > First detailed timing is preferred timing > > Established timings supported: > > Standard timings supported: > > Detailed mode: Clock 147.840 MHz, 294 mm x 165 mm > > 1920 1968 2000 2200 hborder 0 > > 1080 1083 1089 1120 vborder 0 > > +hsync -vsync > > Manufacturer-specified data, tag 0 > > ASCII string: BOE > > ASCII string: NV133FHM-N62 > > Checksum: 0x40 (valid) > > > > Unknown extension block > > > > EDID block does NOT conform to EDID 1.3! > > Missing name descriptor > > Missing monitor ranges > > Detailed block string not properly terminated > > EDID block does not conform at all! > > Has 128 nonconformant extension block(s) > > I did attempt to modify the patch, and I don't think I did it correctly > >
Re: [PATCH 02/25] dma-fence: prime lockdep annotations
On Fri, Jul 10, 2020 at 3:48 PM Jason Gunthorpe wrote: > > On Fri, Jul 10, 2020 at 03:01:10PM +0200, Christian König wrote: > > Am 10.07.20 um 14:54 schrieb Jason Gunthorpe: > > > On Fri, Jul 10, 2020 at 02:48:16PM +0200, Christian König wrote: > > > > Am 10.07.20 um 14:43 schrieb Jason Gunthorpe: > > > > > On Thu, Jul 09, 2020 at 10:09:11AM +0200, Daniel Vetter wrote: > > > > > > Hi Jason, > > > > > > > > > > > > Below the paragraph I've added after our discussions around > > > > > > dma-fences > > > > > > outside of drivers/gpu. Good enough for an ack on this, or want > > > > > > something > > > > > > changed? > > > > > > > > > > > > Thanks, Daniel > > > > > > > > > > > > > + * Note that only GPU drivers have a reasonable excuse for both > > > > > > > requiring > > > > > > > + * _interval_notifier and callbacks at the same > > > > > > > time as having to > > > > > > > + * track asynchronous compute work using _fence. No driver > > > > > > > outside of > > > > > > > + * drivers/gpu should ever call dma_fence_wait() in such > > > > > > > contexts. > > > > > I was hoping we'd get to 'no driver outside GPU should even use > > > > > dma_fence()' > > > > My last status was that V4L could come use dma_fences as well. > > > I'm sure lots of places *could* use it, but I think I understood that > > > it is a bad idea unless you have to fit into the DRM uAPI? > > > > It would be a bit questionable if you use the container objects we came up > > with in the DRM subsystem outside of it. > > > > But using the dma_fence itself makes sense for everything which could do > > async DMA in general. > > dma_fence only possibly makes some sense if you intend to expose the > completion outside a single driver. > > The prefered kernel design pattern for this is to connect things with > a function callback. > > So the actual use case of dma_fence is quite narrow and tightly linked > to DRM. > > I don't think we should spread this beyond DRM, I can't see a reason. Yeah v4l has a legit reason to use dma_fence, android wants that there. There's even been patches proposed years ago, but never landed because android is using some vendor hack horror show for camera drivers right now. But there is an effort going on to fix that (under the libcamera heading), and I expect that once we have that, it'll want dma_fence support. So outright excluding everyone from dma_fence is a bit too much. They definitely shouldn't be used though for entirely independent stuff. > > > You are better to do something contained in the single driver where > > > locking can be analyzed. > > > > > > > I'm not 100% sure, but wouldn't MMU notifier + dma_fence be a valid use > > > > case > > > > for things like custom FPGA interfaces as well? > > > I don't think we should expand the list of drivers that use this > > > technique. > > > Drivers that can't suspend should pin memory, not use blocked > > > notifiers to created pinned memory. > > > > Agreed totally, it's a complete pain to maintain even for the GPU drivers. > > > > Unfortunately that doesn't change users from requesting it. So I'm pretty > > sure we are going to see more of this. > > Kernel maintainers need to say no. > > The proper way to do DMA on no-faulting hardware is page pinning. > > Trying to improve performance of limited HW by using sketchy > techniques at the cost of general system stability should be a NAK. Well that's pretty much gpu drivers, all the horrors for a bit more speed :-) On the text itself, should I upgrade to "must not" instead of "should not"? Or more needed? -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel