Re: [PATCH v3 2/4] drm: bridge: dw_hdmi: Enable GCP only for Deep Color
On 15/04/2022 04:42, sandor...@nxp.com wrote:
From: Sandor Yu
HDMI1.4b specification section 6.5.3:
Source shall only send GCPs with non-zero CD to sinks
that indicate support for Deep Color.
DW HDMI GCP default enabled, but only transmit CD
and do not handle AVMUTE, PP norDefault_Phase (yet).
Disable Auto GCP when 24-bit color for sinks that not support Deep Color.
Signed-off-by: Sandor Yu
---
drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 17 +
drivers/gpu/drm/bridge/synopsys/dw-hdmi.h | 3 +++
2 files changed, 20 insertions(+)
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 02d8f7e08814..312500921754 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1108,6 +1108,8 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
unsigned int output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_PP;
struct hdmi_data_info *hdmi_data = &hdmi->hdmi_data;
u8 val, vp_conf;
+ u8 clear_gcp_auto = 0;
+
if (hdmi_bus_fmt_is_rgb(hdmi->hdmi_data.enc_out_bus_format) ||
hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format) ||
@@ -1117,6 +1119,7 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
case 8:
color_depth = 4;
output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS;
+ clear_gcp_auto = 1;
break;
case 10:
color_depth = 5;
@@ -1136,6 +1139,7 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
case 0:
case 8:
remap_size = HDMI_VP_REMAP_YCC422_16bit;
+ clear_gcp_auto = 1;
break;
case 10:
remap_size = HDMI_VP_REMAP_YCC422_20bit;
@@ -1160,6 +1164,19 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
HDMI_VP_PR_CD_DESIRED_PR_FACTOR_MASK);
hdmi_writeb(hdmi, val, HDMI_VP_PR_CD);
+ /* HDMI1.4b specification section 6.5.3:
+* Source shall only send GCPs with non-zero CD to sinks
+* that indicate support for Deep Color.
+* GCP only transmit CD and do not handle AVMUTE, PP norDefault_Phase
(yet).
+* Disable Auto GCP when 24-bit color for sinks that not support Deep
Color.
+*/
+ val = hdmi_readb(hdmi, HDMI_FC_DATAUTO3);
+ if (clear_gcp_auto == 1)
+ val &= ~HDMI_FC_DATAUTO3_GCP_AUTO;
+ else
+ val |= HDMI_FC_DATAUTO3_GCP_AUTO;
+ hdmi_writeb(hdmi, val, HDMI_FC_DATAUTO3);
+
hdmi_modb(hdmi, HDMI_VP_STUFF_PR_STUFFING_STUFFING_MODE,
HDMI_VP_STUFF_PR_STUFFING_MASK, HDMI_VP_STUFF);
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
index 1999db05bc3b..18df3e119553 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.h
@@ -850,6 +850,9 @@ enum {
HDMI_FC_DATAUTO0_VSD_MASK = 0x08,
HDMI_FC_DATAUTO0_VSD_OFFSET = 3,
+/* FC_DATAUTO3 field values */
+ HDMI_FC_DATAUTO3_GCP_AUTO = 0x04,
+
/* PHY_CONF0 field values */
HDMI_PHY_CONF0_PDZ_MASK = 0x80,
HDMI_PHY_CONF0_PDZ_OFFSET = 7,
Reviewed-by: Neil Armstrong
Re: [PATCH 9/9] vfio: Remove calls to vfio_group_add_container_user()
On 4/12/22 11:53 AM, Jason Gunthorpe wrote: When the open_device() op is called the container_users is incremented and held incremented until close_device(). Thus, so long as drivers call functions within their open_device()/close_device() region they do not need to worry about the container_users. These functions can all only be called between open_device()/close_device(): vfio_pin_pages() vfio_unpin_pages() vfio_dma_rw() vfio_register_notifier() vfio_unregister_notifier() So eliminate the calls to vfio_group_add_container_user() and add a simple WARN_ON to detect mis-use by drivers. vfio_device_fops_release decrements dev->open_count immediately before calling dev->ops->close_device, which means we could enter close_device with a dev_count of 0. Maybe vfio_device_fops_release should handle the same way as vfio_group_get_device_fd? if (device->open_count == 1 && device->ops->close_device) device->ops->close_device(device); device->open_count--;
Re: [PATCH v2 1/2] dt-bindings: display: bridge: Document RZ/G2L MIPI DSI TX bindings
Hi Biju,
Thank you for the patch.
On Mon, Mar 28, 2022 at 07:49:30AM +0100, Biju Das wrote:
> The RZ/G2L MIPI DSI TX is embedded in the Renesas RZ/G2L family SoC's. It
> can operate in DSI mode, with up to four data lanes.
>
> Signed-off-by: Biju Das
> ---
> v1->v2:
> * Added full path for dsi-controller.yaml
> * Modeled DSI + D-PHY as single block and updated reg property
> * Fixed typo D_PHY->D-PHY
> * Updated description
> * Added interrupts and interrupt-names and updated the example
> RFC->v1:
> * Added a ref to dsi-controller.yaml.
> RFC:-
> *
> https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-22-biju.das...@bp.renesas.com/
> ---
> .../bindings/display/bridge/renesas,dsi.yaml | 175 ++
> 1 file changed, 175 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/display/bridge/renesas,dsi.yaml
>
> diff --git
> a/Documentation/devicetree/bindings/display/bridge/renesas,dsi.yaml
> b/Documentation/devicetree/bindings/display/bridge/renesas,dsi.yaml
> new file mode 100644
> index ..eebbf617c484
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dsi.yaml
> @@ -0,0 +1,175 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/bridge/renesas,dsi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas RZ/G2L MIPI DSI Encoder
> +
> +maintainers:
> + - Biju Das
> +
> +description: |
> + This binding describes the MIPI DSI encoder embedded in the Renesas
> + RZ/G2L alike family of SoC's. The encoder can operate in DSI mode, with
> + up to four data lanes.
> +
> +allOf:
> + - $ref: /schemas/display/dsi-controller.yaml#
> +
> +properties:
> + compatible:
> +enum:
> + - renesas,rzg2l-mipi-dsi # RZ/G2L and RZ/V2L
> +
> + reg:
> +maxItems: 1
> +
> + interrupts:
> +items:
> + - description: Sequence operation channel 0 interrupt
> + - description: Sequence operation channel 1 interrupt
> + - description: Video-Input operation channel 1 interrupt
> + - description: DSI Packet Receive interrupt
> + - description: DSI Fatal Error interrupt
> + - description: DSI D-PHY PPI interrupt
> + - description: Debug interrupt
> +
> + interrupt-names:
> +items:
> + - const: seq0
> + - const: seq1
> + - const: vin1
> + - const: rcv
> + - const: ferr
> + - const: ppi
> + - const: debug
> +
> + clocks:
> +items:
> + - description: DSI D-PHY PLL multiplied clock
> + - description: DSI D-PHY system clock
> + - description: DSI AXI bus clock
> + - description: DSI Register access clock
> + - description: DSI Video clock
> + - description: DSI D-PHY Escape mode Receive clock
Isn't this the escape mode *transmit* clock ?
> +
> + clock-names:
> +items:
> + - const: pllclk
> + - const: sysclk
> + - const: aclk
> + - const: pclk
> + - const: vclk
> + - const: lpclk
> +
> + resets:
> +items:
> + - description: MIPI_DSI_CMN_RSTB
> + - description: MIPI_DSI_ARESET_N
> + - description: MIPI_DSI_PRESET_N
> +
> + reset-names:
> +items:
> + - const: rst
> + - const: arst
> + - const: prst
> +
> + power-domains:
> +maxItems: 1
> +
> + ports:
> +$ref: /schemas/graph.yaml#/properties/ports
> +
> +properties:
> + port@0:
> +$ref: /schemas/graph.yaml#/properties/port
> +description: Parallel input port
> +
> + port@1:
> +$ref: /schemas/graph.yaml#/$defs/port-base
> +unevaluatedProperties: false
> +description: DSI output port
> +
> +properties:
> + endpoint:
> +$ref: /schemas/media/video-interfaces.yaml#
> +unevaluatedProperties: false
> +
> +properties:
> + data-lanes:
> +minItems: 1
> +maxItems: 4
You should specify the acceptable values, especially given that the
hardware doesn't seem to support lane reordering.
> +
> +required:
> + - data-lanes
> +
> +required:
> + - port@0
> + - port@1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - interrupt-names
> + - clocks
> + - clock-names
> + - resets
> + - reset-names
> + - power-domains
> + - ports
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> +#include
> +#include
Could you please swap those two lines to get them sorted alphabetically
?
With these comments addressed,
Reviewed-by: Laurent Pinchart
> +
> +dsi0: dsi@1085 {
> +compatible = "renesas,rzg2l-mipi-dsi";
> +reg = <0x1085 0x2>;
> +interrupts = ,
> + ,
> + ,
> + ,
> + ,
> + ,
> + ;
> +interr
Re: [PATCH v19 03/10] soc: mediatek: add mtk-mmsys support for mt8195 vdosys0
Il 15/04/22 10:39, jason-jh.lin ha scritto:
1. Add mt8195 mmsys compatible for 2 vdosys.
2. Add io_start into each driver data of mt8195 vdosys.
3. Add get match data function to identify mmsys by io_start.
4. Add mt8195 routing table settings of vdosys0.
Signed-off-by: jason-jh.lin
Unless anyone wants this commit to be split in two, one for the match data
and one for the mt8195 addition, it looks almost good to me, apart one small
change that has to be done, check below:
--- > drivers/soc/mediatek/mt8195-mmsys.h| 370 +
drivers/soc/mediatek/mtk-mmsys.c | 152 +-
drivers/soc/mediatek/mtk-mmsys.h | 6 +
include/linux/soc/mediatek/mtk-mmsys.h | 11 +
4 files changed, 528 insertions(+), 11 deletions(-)
create mode 100644 drivers/soc/mediatek/mt8195-mmsys.h
..snip..
diff --git a/drivers/soc/mediatek/mtk-mmsys.h b/drivers/soc/mediatek/mtk-mmsys.h
index 77f37f8c715b..21cf300ba864 100644
--- a/drivers/soc/mediatek/mtk-mmsys.h
+++ b/drivers/soc/mediatek/mtk-mmsys.h
@@ -87,12 +87,18 @@ struct mtk_mmsys_routes {
};
struct mtk_mmsys_driver_data {
+ const u32 io_start;
I agree with you that this iostart is in 32bits boundaries, and that this will
practically never change... and I was in doubt whether this would be acceptable
or not, because of saving some memory.
Even though I would really love to have the savings, in order to avoid having
any
"surprises" in the future (any future breakage for "reasons"), we shall comply
with the kernel types, so, this has to be...
const resource_size_t io_start;
...and this is the same for both this file and mtk_drm_drv.h, which you modify
in
patch 07/10.
After fixing that:
Reviewed-by: AngeloGioacchino Del Regno
Cheers,
Angelo
Re: [PATCH v19 07/10] drm/mediatek: add mediatek-drm of vdosys0 support for mt8195
Il 15/04/22 10:39, jason-jh.lin ha scritto:
1. Add driver data of mt8195 vdosys0 to mediatek-drm and the sub driver.
2. Add get driver data function to identify which vdosys by io_start.
Signed-off-by: jason-jh.lin
---
drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 6 +
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 141 +--
drivers/gpu/drm/mediatek/mtk_drm_drv.h | 6 +
3 files changed, 145 insertions(+), 8 deletions(-)
..snip..
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index a58cebd01d35..e1a37ca091f3 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -21,6 +21,7 @@ struct drm_property;
struct regmap;
struct mtk_mmsys_driver_data {
+ const unsigned int io_start;
As mentioned in patch 03/10, this has to be
const resource_size_t io_start;
Thanks!
Re: [PATCH v19 10/10] soc: mediatek: remove DDP_DOMPONENT_DITHER enum
Il 15/04/22 10:39, jason-jh.lin ha scritto: After mmsys and drm change DITHER enum to DDP_COMPONENT_DITHER0, mmsys header can remove the useless DDP_COMPONENT_DITHER enum. Signed-off-by: jason-jh.lin Can you please fix the commit title with: soc: mediatek: remove DDP_DOMPONENT_DITHER from enum that "from" is important, as you're not removing *an enum*, but *from an enum*. After that: Reviewed-by: AngeloGioacchino Del Regno Thanks! Angelo
Re: [PATCH v19 09/10] drm/mediatek: add postfix 0 to DDP_COMPONENT_DITHER for mt8195 vdosys0
Il 15/04/22 10:39, jason-jh.lin ha scritto: Because mt8195 vdosys0 has 2 DITHER components, so the postfix 0 need to be added to DDP_COMPONENT_DITHER. Then DITHER enum will become: DDP_COMPONENT_DITHER0 and DDP_COMPONENT_DITHER1. Signed-off-by: jason-jh.lin I think that "postfix" should be "suffix" instead :))) In any case: Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v19 08/10] soc: mediatek: add DDP_DOMPONENT_DITHER0 enum for mt8195 vdosys0
Il 15/04/22 10:39, jason-jh.lin ha scritto: The mmsys routing table of mt8195 vdosys0 has 2 DITHER components, so mmsys need to add DDP_COMPONENT_DITHER1 and change all usages of DITHER enum form DDP_COMPONENT_DITHER to DDP_COMPONENT_DITHER0. But its header need to keep DDP_COMPONENT_DITHER enum until drm/mediatek also changed it. Signed-off-by: jason-jh.lin Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH v2 2/2] drm: rcar-du: Add RZ/G2L DSI driver
Hi Biju,
Thank you for the patch.
On Mon, Mar 28, 2022 at 07:51:15AM +0100, Biju Das wrote:
> This driver supports the MIPI DSI encoder found in the RZ/G2L
> SoC. It currently supports DSI mode only.
>
> Signed-off-by: Biju Das
> ---
> v1->v2:
> * Rework based on dt-binding change (DSI + D-PHY) as single block
> * Replaced link_mmio and phy_mmio with mmio in struct rzg2l_mipi_dsi
> * Replaced rzg2l_mipi_phy_write with rzg2l_mipi_dsi_phy_write
>and rzg2l_mipi_dsi_link_write
> * Replaced rzg2l_mipi_phy_read->rzg2l_mipi_dsi_link_read
> RFC->v1:
> * Added "depends on ARCH_RENESAS || COMPILE_TEST" on KCONFIG
>and dropped DRM as it is implied by DRM_BRIDGE
> * Used devm_reset_control_get_exclusive() for reset handle
> * Removed bool hsclkmode from struct rzg2l_mipi_dsi
> * Added error check for pm, using pm_runtime_resume_and_get() instead of
>pm_runtime_get_sync()
> * Added check for unsupported formats in rzg2l_mipi_dsi_host_attach()
> * Avoided read-modify-write stopping hsclock
> * Used devm_platform_ioremap_resource for resource allocation
> * Removed unnecessary assert call from probe and remove.
> * wrap the line after the PTR_ERR() in probe()
> * Updated reset failure messages in probe
> * Fixed the typo arstc->prstc
> * Made hex constants to lower case.
> RFC:
> *
> https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-23-biju.das...@bp.renesas.com/
> ---
> drivers/gpu/drm/rcar-du/Kconfig | 8 +
> drivers/gpu/drm/rcar-du/Makefile | 1 +
> drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c | 686 ++
> drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h | 149
> 4 files changed, 844 insertions(+)
> create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> create mode 100644 drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi_regs.h
>
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index e40fb0c53f9b..83d7d28214a0 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -64,6 +64,14 @@ config DRM_RCAR_MIPI_DSI
> help
> Enable support for the R-Car Display Unit embedded MIPI DSI encoders.
>
> +config DRM_RZG2L_MIPI_DSI
> + tristate "RZ/G2L MIPI DSI Encoder Support"
> + depends on DRM_BRIDGE && OF
> + depends on ARCH_RENESAS || COMPILE_TEST
> + select DRM_MIPI_DSI
> + help
> + Enable support for the RZ/G2L Display Unit embedded MIPI DSI encoders.
> +
> config DRM_RCAR_VSP
> bool "R-Car DU VSP Compositor Support" if ARM
> default y if ARM64
> diff --git a/drivers/gpu/drm/rcar-du/Makefile
> b/drivers/gpu/drm/rcar-du/Makefile
> index 01ba78e56b53..7fb4f8717fc4 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -27,6 +27,7 @@ obj-$(CONFIG_DRM_RCAR_LVDS) += rcar_lvds.o
> obj-$(CONFIG_DRM_RCAR_MIPI_DSI) += rcar_mipi_dsi.o
>
> obj-$(CONFIG_DRM_RZG2L_DU) += rzg2l-du-drm.o
> +obj-$(CONFIG_DRM_RZG2L_MIPI_DSI) += rzg2l_mipi_dsi.o
>
> # 'remote-endpoint' is fixed up at run-time
> DTC_FLAGS_rcar_du_of_lvds_r8a7790 += -Wno-graph_endpoint
> diff --git a/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> new file mode 100644
> index ..3b785041ba5e
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rzg2l_mipi_dsi.c
> @@ -0,0 +1,686 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * RZ/G2L MIPI DSI Encoder Driver
> + *
> + * Copyright (C) 2022 Renesas Electronics Corporation
> + */
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "rzg2l_mipi_dsi_regs.h"
> +
> +struct rzg2l_mipi_dsi {
> + struct device *dev;
> + void __iomem *mmio;
> +
> + struct reset_control *rstc;
> + struct reset_control *arstc;
> + struct reset_control *prstc;
> +
> + struct mipi_dsi_host host;
> + struct drm_bridge bridge;
> + struct drm_bridge *next_bridge;
> +
> + struct clk *vclk;
> +
> + enum mipi_dsi_pixel_format format;
> + unsigned int num_data_lanes;
> + unsigned int lanes;
> + unsigned long mode_flags;
> +};
> +
> +static inline struct rzg2l_mipi_dsi *
> +bridge_to_rzg2l_mipi_dsi(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct rzg2l_mipi_dsi, bridge);
> +}
> +
> +static inline struct rzg2l_mipi_dsi *
> +host_to_rzg2l_mipi_dsi(struct mipi_dsi_host *host)
> +{
> + return container_of(host, struct rzg2l_mipi_dsi, host);
> +}
> +
> +static void rzg2l_mipi_dsi_phy_write(void __iomem *mem, u32 reg, u32 data)
You can pass a rzg2l_mipi_dsi pointer to this function instead of the
memory pointer. Same for the two functions below.
> +{
> + iowrite32(data, mem + reg);
> +}
> +
> +static void rzg2l_mipi_
Re: [PATCH v9 00/23] drm/rockchip: RK356x VOP2 support
> Wiadomość napisana przez Daniel Stone w dniu > 12.04.2022, o godz. 13:30: > >> Testing Sacha patch (see today's email from Sascha) i'm getting >> >> Qt: EGL Error : Could not create the egl surface: error = 0x3009 >> >> i'm reading this as: Qt tries allocate EGL surface and EGL returns error. >> or i'm wrong? > > Correct, that's EGL_BAD_MATCH. There are very few ways that can > happen; by far the most likely is that Qt has chosen an EGLConfig > which does not correctly correspond to the format. (If it was an > impossible format/modifier combination, then this would be already > caught when allocating the gbm_surface.) > > Either way, it seems quite clear that the VOP2 driver is totally fine > here, and that you have a Qt (likely) or Mesa (tbh less likely) issue > to debug to get the app working. > > Cheers, > Daniel Thx Daniel! Indeed - this is probably another case where I see: writing DRM planes rendering mediaplayer with help of Qt is (too)corner case for Qt @all Looking on Qt sources it looks to me this format should be supported: https://code.qt.io/cgit/qt/qtbase.git/tree/src/platformsupport/kmsconvenience/qkmsdevice.cpp?h=5.15.2#n380 Interesting that with custom Qt config1: "format": "argb", DRI state shows: format=AR24 little-endian (0x34325241) UI is OK, playback is OK. OSD not visible (*) custom config2: "format": "abgr" Qt crashes with EGL_BAD_MATCH So it looks forcing some Qt formats works while other - not. Looking why this: Qt logging says nothing. export MESA_DEBUG=1 gives no any message. I'm a lost here a bit... But from a bit more distant perspective: 1. Sascha concludes in (*) that issue is most probably in format negotiation by app. 2. imho app probably correctly negotiates accordingly to inputs it gets from providers (DRM) and clients (mesa). Test with patch excluding DRM_FORMAT_XRGB, DRM_FORMAT_ARGB, shows imho proper app reaction on this (new format elected; but Qt fails with this format). Indirectly i conclude app logic is ok 3. Sum of p1 & p2 tells me: i need to add extra logic in format election to specifically deal with constrains of rk356x (see explanation in (*)) 4. Even if i implement p3 - then user world - (this using Qt) - will be not happy as: - majority users are using pre-build Qt - I don't believe Trolltech will fix this soon So i see following path: we will verify is issue of Qt with abgr an Qt root cause issue, If Yes - then: - Investigate is there reasonable way to workaround with this outside of Qt? If not - then: - conclude: vop2 - due Qt bug - will not work ok with Qt until Qt will be fixed. If you think this make sense - i need some help with: verify is issue of Qt with abgr an Qt root cause issue let me know what you think (*) Sascha mentioned: > Somehow negotiation of the format goes wrong. Applications shouldn't > pick these formats when the GPU is used for rendering. I don't know how > and where this should be fixed properly, but your application should use > DRM_FORMAT_ABGR aka AB24 aka PIPE_FORMAT_R8G8B8A8_UNORM instead of > DRM_FORMAT_ARGB aka AR24 aka PIPE_FORMAT_B8G8R8A8_UNORM.
Re: [PATCH v2 1/7] dt-bindings: display: renesas,du: Document r9a07g044l bindings
Hi Biju, Thank you for the patch. On Wed, Mar 16, 2022 at 01:10:54PM +, Biju Das wrote: > Extend the Renesas DU display bindings to support the r9a07g044l > DU module found on RZ/G2L LCDC. Stupid question, but as this DU and the R-Car DU are completely different pieces of hardware, wouldn't a separate bindings file make sense ? The DT description in this patch looks good to me. > Signed-off-by: Biju Das > --- > v1->v2: > * Updated commit description. > * Removed LCDC references > * Changed clock name from du.0->aclk > * Changed reset name from du.0->du > RFC->v1: > * Changed minItems->maxItems for renesas,vsps. > RFC: > > https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-18-biju.das...@bp.renesas.com/ > --- > .../bindings/display/renesas,du.yaml | 54 +++ > 1 file changed, 54 insertions(+) > > diff --git a/Documentation/devicetree/bindings/display/renesas,du.yaml > b/Documentation/devicetree/bindings/display/renesas,du.yaml > index 13efea574584..f560608bf4e8 100644 > --- a/Documentation/devicetree/bindings/display/renesas,du.yaml > +++ b/Documentation/devicetree/bindings/display/renesas,du.yaml > @@ -40,6 +40,7 @@ properties: >- renesas,du-r8a77990 # for R-Car E3 compatible DU >- renesas,du-r8a77995 # for R-Car D3 compatible DU >- renesas,du-r8a779a0 # for R-Car V3U compatible DU > + - renesas,du-r9a07g044l # for RZ/G2L compatible DU > >reg: > maxItems: 1 > @@ -824,6 +825,59 @@ allOf: > - reset-names > - renesas,vsps > > + - if: > + properties: > +compatible: > + contains: > +enum: > + - renesas,du-r9a07g044l > +then: > + properties: > +clocks: > + items: > +- description: Main clock > +- description: Register access clock > +- description: Video clock > + > +clock-names: > + items: > +- const: aclk > +- const: pclk > +- const: vclk > + > +interrupts: > + maxItems: 1 > + > +resets: > + maxItems: 1 > + > +reset-names: > + items: > +- const: du > + > +ports: > + properties: > +port@0: > + description: DPAD 0 > +port@1: > + description: DSI 0 > +port@2: false > +port@3: false > + > + required: > +- port@0 > +- port@1 > + > +renesas,vsps: > + maxItems: 1 > + > + required: > +- clock-names > +- interrupts > +- resets > +- reset-names > +- renesas,vsps > + > additionalProperties: false > > examples: -- Regards, Laurent Pinchart
Re: [PATCH v2 2/7] drm: rcar-du: Add num_rpf to struct rcar_du_device_info
Hi Biju,
Thank you for the patch.
On Wed, Mar 16, 2022 at 01:10:55PM +, Biju Das wrote:
> Number of RPF's VSP is different on R-Car and RZ/G2L
> R-Car Gen3 -> 5 RPFs
> R-Car Gen2 -> 4 RPFs
> RZ/G2L -> 2 RPFs
>
> Add num_rpf to struct rcar_du_device_info to support later
> SoC without any code changes.
>
> Signed-off-by: Biju Das
> ---
> v1->v2:
> * No change
> RFC->v1:
> * Fixed the comment for num_rpf s/rpf's/RPFs/ and s/vsp/VSP/
> RFC:
> *
> https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-12-biju.das...@bp.renesas.com/
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 17 +
> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 2 ++
> drivers/gpu/drm/rcar-du/rcar_du_vsp.c | 6 +-
> 3 files changed, 20 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 957ea97541d5..1bc7325aa356 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -55,6 +55,7 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7743_info = {
> },
> },
> .num_lvds = 1,
> + .num_rpf = 4,
> };
>
> static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
> @@ -77,6 +78,7 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7745_info = {
> .port = 1,
> },
> },
> + .num_rpf = 4,
> };
>
> static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
> @@ -104,6 +106,7 @@ static const struct rcar_du_device_info
> rzg1_du_r8a77470_info = {
> .port = 2,
> },
> },
> + .num_rpf = 4,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a774a1_info = {
> @@ -133,6 +136,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a774a1_info = {
> },
> },
> .num_lvds = 1,
> + .num_rpf = 5,
> .dpll_mask = BIT(1),
> };
>
> @@ -163,6 +167,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a774b1_info = {
> },
> },
> .num_lvds = 1,
> + .num_rpf = 5,
> .dpll_mask = BIT(1),
> };
>
> @@ -190,6 +195,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a774c0_info = {
> },
> },
> .num_lvds = 2,
> + .num_rpf = 4,
> .lvds_clk_mask = BIT(1) | BIT(0),
> };
>
> @@ -220,6 +226,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a774e1_info = {
> },
> },
> .num_lvds = 1,
> + .num_rpf = 5,
> .dpll_mask = BIT(1),
> };
>
> @@ -272,6 +279,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7790_info = {
> },
> },
> .num_lvds = 2,
> + .num_rpf = 4,
> };
>
> /* M2-W (r8a7791) and M2-N (r8a7793) are identical */
> @@ -297,6 +305,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7791_info = {
> },
> },
> .num_lvds = 1,
> + .num_rpf = 4,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7792_info = {
> @@ -317,6 +326,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7792_info = {
> .port = 1,
> },
> },
> + .num_rpf = 4,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7794_info = {
> @@ -340,6 +350,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7794_info = {
> .port = 1,
> },
> },
> + .num_rpf = 4,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7795_info = {
> @@ -373,6 +384,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7795_info = {
> },
> },
> .num_lvds = 1,
> + .num_rpf = 5,
> .dpll_mask = BIT(2) | BIT(1),
> };
>
> @@ -403,6 +415,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7796_info = {
> },
> },
> .num_lvds = 1,
> + .num_rpf = 5,
> .dpll_mask = BIT(1),
> };
>
> @@ -433,6 +446,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a77965_info = {
> },
> },
> .num_lvds = 1,
> + .num_rpf = 5,
> .dpll_mask = BIT(1),
> };
>
> @@ -459,6 +473,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a77970_info = {
> },
> },
> .num_lvds = 1,
> + .num_rpf = 5,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7799x_info = {
> @@ -486,6 +501,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7799x_info = {
> },
> },
> .num_lvds = 2,
> + .num_rpf = 5,
> .lvds_clk_mask = BIT(1) | BIT(0),
> };
>
> @@ -505,6 +521,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a779a0_info = {
> .port = 1,
> },
> },
> + .num_rpf = 5,
> .dsi_clk_mask = BIT(1) | BIT(0),
> };
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> b/drivers/gpu/d
Re: [PATCH v2 3/7] drm: rcar-du: Add max_width and max_height to struct rcar_du_device_info
Hi Biju,
Thank you for the patch.
On Wed, Mar 16, 2022 at 01:10:56PM +, Biju Das wrote:
> There are some differences related to max frame size supported by different
> R-Car/RZ-G family of SoC's
>
> Max frame size supported by R-Car Gen1 & R-Car Gen2 is 4095x2047
> Max frame size supported by R-Car Gen3 is 8190x8190
> Max frame size supported by RZ/G2L is 1920x1080
>
> Add max_width and max_height to struct rcar_du_device_info to support later
> SoC without any code changes.
>
> Signed-off-by: Biju Das
> ---
> v1->v2:
> * No Change
> RFC->V1:
> * No Change
> RFC:
> *
> https://patchwork.kernel.org/project/linux-renesas-soc/patch/20220112174612.10773-13-biju.das...@bp.renesas.com/
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 36 +++
> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 4 +++
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 17 +
> 3 files changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 1bc7325aa356..4640c356a532 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -56,6 +56,8 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7743_info = {
> },
> .num_lvds = 1,
> .num_rpf = 4,
> + .max_width = 4095,
> + .max_height = 2047,
> };
>
> static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
> @@ -79,6 +81,8 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7745_info = {
> },
> },
> .num_rpf = 4,
> + .max_width = 4095,
> + .max_height = 2047,
> };
>
> static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
> @@ -107,6 +111,8 @@ static const struct rcar_du_device_info
> rzg1_du_r8a77470_info = {
> },
> },
> .num_rpf = 4,
> + .max_width = 4095,
> + .max_height = 2047,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a774a1_info = {
> @@ -137,6 +143,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a774a1_info = {
> },
> .num_lvds = 1,
> .num_rpf = 5,
> + .max_width = 8190,
> + .max_height = 8190,
> .dpll_mask = BIT(1),
> };
>
> @@ -168,6 +176,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a774b1_info = {
> },
> .num_lvds = 1,
> .num_rpf = 5,
> + .max_width = 8190,
> + .max_height = 8190,
> .dpll_mask = BIT(1),
> };
>
> @@ -196,6 +206,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a774c0_info = {
> },
> .num_lvds = 2,
> .num_rpf = 4,
> + .max_width = 8190,
> + .max_height = 8190,
> .lvds_clk_mask = BIT(1) | BIT(0),
> };
>
> @@ -227,6 +239,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a774e1_info = {
> },
> .num_lvds = 1,
> .num_rpf = 5,
> + .max_width = 8190,
> + .max_height = 8190,
> .dpll_mask = BIT(1),
> };
>
> @@ -249,6 +263,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a7779_info = {
> .port = 1,
> },
> },
> + .max_width = 4095,
> + .max_height = 2047,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7790_info = {
> @@ -280,6 +296,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a7790_info = {
> },
> .num_lvds = 2,
> .num_rpf = 4,
> + .max_width = 4095,
> + .max_height = 2047,
> };
>
> /* M2-W (r8a7791) and M2-N (r8a7793) are identical */
> @@ -306,6 +324,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a7791_info = {
> },
> .num_lvds = 1,
> .num_rpf = 4,
> + .max_width = 4095,
> + .max_height = 2047,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7792_info = {
> @@ -327,6 +347,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a7792_info = {
> },
> },
> .num_rpf = 4,
> + .max_width = 4095,
> + .max_height = 2047,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7794_info = {
> @@ -351,6 +373,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a7794_info = {
> },
> },
> .num_rpf = 4,
> + .max_width = 4095,
> + .max_height = 2047,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7795_info = {
> @@ -385,6 +409,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a7795_info = {
> },
> .num_lvds = 1,
> .num_rpf = 5,
> + .max_width = 8190,
> + .max_height = 8190,
> .dpll_mask = BIT(2) | BIT(1),
> };
>
> @@ -416,6 +442,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a7796_info = {
> },
> .num_lvds = 1,
> .num_rpf = 5,
> + .max_width = 8190,
> + .max_height = 8190,
> .dpll_mask = BIT(1),
> };
>
> @@ -447,6 +475,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a77965_info = {
> },
> .num_lvds = 1,
> .
Re: [PATCH v2 4/7] drm: rcar-du: Move rcar_du_output_name() to rcar_du_common.c
Hi Biju,
Thank you for the patch.
On Wed, Mar 16, 2022 at 01:10:57PM +, Biju Das wrote:
> RZ/G2L SoC's does not have group/plane registers compared to RCar, hence it
> needs a different CRTC implementation.
>
> Move rcar_du_output_name() to a new common file rcar_du_common.c, So that
> the same function can be reused by RZ/G2L SoC later.
>
> Signed-off-by: Biju Das
> ---
> v1->v2:
> * No change
> RFC->v1:
> New patch
> ---
> drivers/gpu/drm/rcar-du/Makefile | 1 +
> drivers/gpu/drm/rcar-du/rcar_du_common.c | 30
> drivers/gpu/drm/rcar-du/rcar_du_drv.c| 20
> 3 files changed, 31 insertions(+), 20 deletions(-)
> create mode 100644 drivers/gpu/drm/rcar-du/rcar_du_common.c
>
> diff --git a/drivers/gpu/drm/rcar-du/Makefile
> b/drivers/gpu/drm/rcar-du/Makefile
> index e7275b5e7ec8..331e12d65a6b 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> rcar-du-drm-y := rcar_du_crtc.o \
> + rcar_du_common.o \
>rcar_du_drv.o \
>rcar_du_encoder.o \
>rcar_du_group.o \
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_common.c
> b/drivers/gpu/drm/rcar-du/rcar_du_common.c
> new file mode 100644
> index ..f9f9908cda6d
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_common.c
> @@ -0,0 +1,30 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * rcar_du_common.c -- R-Car Display Unit Common
> + *
> + * Copyright (C) 2013-2022 Renesas Electronics Corporation
> + *
> + * Contact: Laurent Pinchart (laurent.pinch...@ideasonboard.com)
> + */
> +
> +#include "rcar_du_drv.h"
> +
> +const char *rcar_du_output_name(enum rcar_du_output output)
> +{
> + static const char * const names[] = {
> + [RCAR_DU_OUTPUT_DPAD0] = "DPAD0",
> + [RCAR_DU_OUTPUT_DPAD1] = "DPAD1",
> + [RCAR_DU_OUTPUT_DSI0] = "DSI0",
> + [RCAR_DU_OUTPUT_DSI1] = "DSI1",
> + [RCAR_DU_OUTPUT_HDMI0] = "HDMI0",
> + [RCAR_DU_OUTPUT_HDMI1] = "HDMI1",
> + [RCAR_DU_OUTPUT_LVDS0] = "LVDS0",
> + [RCAR_DU_OUTPUT_LVDS1] = "LVDS1",
> + [RCAR_DU_OUTPUT_TCON] = "TCON",
> + };
> +
> + if (output >= ARRAY_SIZE(names) || !names[output])
> + return "UNKNOWN";
> +
> + return names[output];
> +}
As we have nothing else than this function in this file, how about
moving it to rcar_du_drv.c instead, to avoid adding a new file ?
You also need to add a declaration for rcar_du_output_name() in the
appropriate header.
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 4640c356a532..f6e234dafb72 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -591,26 +591,6 @@ static const struct of_device_id rcar_du_of_table[] = {
>
> MODULE_DEVICE_TABLE(of, rcar_du_of_table);
>
> -const char *rcar_du_output_name(enum rcar_du_output output)
> -{
> - static const char * const names[] = {
> - [RCAR_DU_OUTPUT_DPAD0] = "DPAD0",
> - [RCAR_DU_OUTPUT_DPAD1] = "DPAD1",
> - [RCAR_DU_OUTPUT_DSI0] = "DSI0",
> - [RCAR_DU_OUTPUT_DSI1] = "DSI1",
> - [RCAR_DU_OUTPUT_HDMI0] = "HDMI0",
> - [RCAR_DU_OUTPUT_HDMI1] = "HDMI1",
> - [RCAR_DU_OUTPUT_LVDS0] = "LVDS0",
> - [RCAR_DU_OUTPUT_LVDS1] = "LVDS1",
> - [RCAR_DU_OUTPUT_TCON] = "TCON",
> - };
> -
> - if (output >= ARRAY_SIZE(names) || !names[output])
> - return "UNKNOWN";
> -
> - return names[output];
> -}
> -
> /*
> -
> * DRM operations
> */
--
Regards,
Laurent Pinchart
Re: [RFC PATCH 02/16] dt-bindings: display: rockchip: Add EBC binding
On Thu, 14 Apr 2022 22:00:09 -0500 Samuel Holland wrote: > Hi Andreas, > > Thanks for the comments. > > On 4/14/22 3:15 AM, Andreas Kemnade wrote: > > Hi Samuel, > > > > for comparison, here is my submission for the IMX EPDC bindings: > > > > https://lore.kernel.org/linux-devicetree/20220206080016.796556-2-andr...@kemnade.info/ > > > > On Wed, 13 Apr 2022 17:19:02 -0500 > > Samuel Holland wrote: > > > > [...] > > we have sy7636a driver in kernel which should be suitable for powering a EPD > > and temperature measurement. So I would expect that to be > >> + io-channels: > >> +maxItems: 1 > >> +description: I/O channel for panel temperature measurement > >> + > > so how would I reference the hwmon/thermal(-zone) of the sy7636a here? > > It seems the consensus is to use a thermal zone for panel temperature, so I > will > need to change this. > I am open to anything here as long as it fits together. > I think it's best to reference the thermal zone by phandle, not by name, even > if > it requires extending the thermal zone API to support this. > maybe referencing the hwmon might be interesting, or we add a hwmon_iio adaptor. The other way round it is there. The thermal zone stuff is only needed because hwmon cannot referenced directly. Regards, Andreas
Re: [PATCH 9/9] vfio: Remove calls to vfio_group_add_container_user()
On Fri, Apr 15, 2022 at 02:32:08AM +, Tian, Kevin wrote: > While it's a welcomed fix is it actually related to this series? The point > of this patch is that those functions are called when container_users > is non-zero. This is true even without this fix given container_users > is decremented after calling device->ops->close_device(). It isn't, it is decremented before which causes it to be 0 when the assertions are called. Jason
[pull] amdgpu, amdkfd, radeon drm-next-5.19
Hi Dave, Daniel, New features for 5.19. There is a new DP define added in drm_dp_helper.h to support some new DC code. That file has moved in drm-misc. Just a heads up there when you merge. The following changes since commit 15f9cd4334c83716fa32647652a609e3ba6c998d: drm/amdgpu/gfx10: enable gfx1037 clock counter retrieval function (2022-03-25 12:40:25 -0400) are available in the Git repository at: https://gitlab.freedesktop.org/agd5f/linux.git tags/amd-drm-next-5.19-2022-04-15 for you to fetch changes up to d68cf992ded575928cf4ddf7c64faff0d8dcce14: drm/amd/amdgpu: Remove static from variable in RLCG Reg RW (2022-04-14 15:29:20 -0400) amd-drm-next-5.19-2022-04-15: amdgpu: - USB-C updates - GPUVM updates - TMZ fixes for RV - DCN 3.1 pstate fixes - Display z state fixes - RAS fixes - Misc code cleanups and spelling fixes - More DC FP rework - GPUVM TLB handling rework - Power management sysfs code cleanup - Add RAS support for VCN - Backlight fix - Add unique id support for more asics - Misc display updates - SR-IOV fixes - Extend CG and PG flags to 64 bits - Enable VCN clk sysfs nodes for navi12 amdkfd: - Fix IO link cleanup during device removal - RAS fixes - Retry fault fixes - Asynchronously free events - SVM fixes radeon: - Drop some dead code - Misc code cleanups Aashish Sharma (1): drm/amd/display: Fix unused-but-set-variable warning Ahmad Othman (1): drm/amd/display: Fix HDCP SEND AKI INIT error Alex Deucher (7): drm/amdgpu: make amdgpu_display_framebuffer_init() static drm/amdgpu: drop amdgpu_display_gem_fb_init() drm/amdgpu: make amdgpu_display_gem_fb_verify_and_init() static drm/amdgpu: don't use BACO for reset in S3 drm/amdgpu/smu10: fix SoC/fclk units in auto mode drm/amdgpu: fix VCN 3.1.2 firmware name drm/amd/display: fix 64 bit divide in freesync code Angus Wang (4): drm/amd/display: Create underflow interrupt IRQ type drm/amd/display: Add flip interval workaround drm/amd/display: Remove underflow IRQ type drm/amd/display: Fix inconsistent timestamp type Anthony Koo (3): drm/amd/display: [FW Promotion] Release 0.0.109.0 drm/amd/display: [FW Promotion] Release 0.0.110.0 drm/amd/display: [FW Promotion] Release 0.0.111.0 Aric Cyr (4): drm/amd/display: 3.2.178 drm/amd/display: 3.2.179 drm/amd/display: 3.2.180 drm/amd/display: 3.2.181 Becle Lee (1): drm/amd/display: fix missing-prototypes warning Benjamin Marty (1): drm/amdgpu/display: change pipe policy for DCN 2.1 Boyuan Zhang (1): drm/amdgpu/vcn3: send smu interface type CHANDAN VURDIGERE NATARAJ (1): drm/amd/display: Fix by adding FPU protection for dcn30_internal_validate_bw Charlene Liu (2): drm/amd/display: Clear optc false state when disable otg drm/amd/display: remove dtbclk_ss compensation for dcn316 Chris Park (1): drm/amd/display: Correct Slice reset calculation Christian König (10): drm/amdgpu: move VM PDEs to idle after update drm/amdgpu: separate VM PT handling into amdgpu_vm_pt.c drm/amdgpu: simplify VM update tracking a bit drm/amdgpu: rework TLB flushing drm/amdkfd: start using tlb_seq from the VM subsystem drm/amdkfd: use tlb_seq from the VM subsystem for SVM as well v2 drm/amdgpu: remove table_freed param from the VM code drm/amdgpu: fix some kerneldoc in the VM code v2 drm/amdgpu: fix incorrect size printing in error msg drm/amdgpu: fix TLB flushing during eviction Colin Ian King (1): drm/amdgpu: Fix spelling mistake "regiser" -> "register" Dan Carpenter (1): drm/amdkfd: potential NULL dereference in kfd_set/reset_event() Darren Powell (2): amdgpu/pm: Add new hwmgr API function "emit_clock_levels" amdgpu/pm: Implement emit_clk_levels for vega10 David Zhang (2): drm: add PSR2 support and capability definition as per eDP 1.5 drm/amd/display: implement shared PSR-SU sink validation helper Dillon Varone (2): drm/amd/display: Add dtb clock to dc_clocks drm/amd/display: Select correct DTO source Dmytro Laktyushkin (1): drm/amd/display: update dcn315 clock table read Duncan Ma (1): drm/amd/display: Add odm seamless boot support Eric Bernstein (1): drm/amd/display: remove assert for odm transition case Eric Yang (1): drm/amd/display: undo clearing of z10 related function pointers Evan Quan (1): drm/amdgpu: expand cg_flags from u32 to u64 Evgenii Krasnikov (1): drm/amd/display: ensure PSR force_static flag can always be set Felix Kuehling (4): drm/amdkfd: Improve concurrency of event handling drm/amdkfd: Fix NULL pointer dereference drm/amdkfd: Asynchronously free events drm/amdkfd: fix race condition in kfd_wait_on_events Gavin Wan (1)
[PATCH v2 0/2] drm/vkms: check plane_composer->map[0] before using it
Hi, this is a follow-up of my last patch. Thanks for the reviews! Changes from v1: Edit the first commit message as suggested by Melissa and add a commit to enhance the error handling (André) Tales Lelo da Aparecida (2): drm/vkms: check plane_composer->map[0] before using it drm/vkms: return early if compose_plane fails drivers/gpu/drm/vkms/vkms_composer.c | 32 ++-- 1 file changed, 21 insertions(+), 11 deletions(-) -- 2.35.1
[PATCH v2 1/2] drm/vkms: check plane_composer->map[0] before using it
Fix a copypasta error. The caller of compose_plane() already checks
primary_composer->map. In contrast, plane_composer->map is never
verified here before handling.
Fixes: 7938f4218168 ("dma-buf-map: Rename to iosys-map")
Reviewed-by: André Almeida
Signed-off-by: Tales Lelo da Aparecida
---
v2: detail the commit message with more information
drivers/gpu/drm/vkms/vkms_composer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index c6a1036bf2ea..b47ac170108c 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -157,7 +157,7 @@ static void compose_plane(struct vkms_composer
*primary_composer,
void *vaddr;
void (*pixel_blend)(const u8 *p_src, u8 *p_dst);
- if (WARN_ON(iosys_map_is_null(&primary_composer->map[0])))
+ if (WARN_ON(iosys_map_is_null(&plane_composer->map[0])))
return;
vaddr = plane_composer->map[0].vaddr;
--
2.35.1
[PATCH v2 2/2] drm/vkms: return early if compose_plane fails
Do not exit quietly from compose_plane. If any plane has an invalid
map, propagate the error value upwards. While here, add log messages
for the invalid index.
Signed-off-by: Tales Lelo da Aparecida
---
drivers/gpu/drm/vkms/vkms_composer.c | 30 ++--
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
b/drivers/gpu/drm/vkms/vkms_composer.c
index b47ac170108c..c0a3b53cd155 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -149,16 +149,16 @@ static void blend(void *vaddr_dst, void *vaddr_src,
}
}
-static void compose_plane(struct vkms_composer *primary_composer,
- struct vkms_composer *plane_composer,
- void *vaddr_out)
+static int compose_plane(struct vkms_composer *primary_composer,
+struct vkms_composer *plane_composer,
+void *vaddr_out)
{
struct drm_framebuffer *fb = &plane_composer->fb;
void *vaddr;
void (*pixel_blend)(const u8 *p_src, u8 *p_dst);
if (WARN_ON(iosys_map_is_null(&plane_composer->map[0])))
- return;
+ return -EINVAL;
vaddr = plane_composer->map[0].vaddr;
@@ -168,6 +168,8 @@ static void compose_plane(struct vkms_composer
*primary_composer,
pixel_blend = &x_blend;
blend(vaddr_out, vaddr, primary_composer, plane_composer, pixel_blend);
+
+ return 0;
}
static int compose_active_planes(void **vaddr_out,
@@ -177,7 +179,7 @@ static int compose_active_planes(void **vaddr_out,
struct drm_framebuffer *fb = &primary_composer->fb;
struct drm_gem_object *gem_obj = drm_gem_fb_get_obj(fb, 0);
const void *vaddr;
- int i;
+ int i, ret;
if (!*vaddr_out) {
*vaddr_out = kzalloc(gem_obj->size, GFP_KERNEL);
@@ -187,8 +189,10 @@ static int compose_active_planes(void **vaddr_out,
}
}
- if (WARN_ON(iosys_map_is_null(&primary_composer->map[0])))
+ if (WARN_ON(iosys_map_is_null(&primary_composer->map[0]))) {
+ DRM_DEBUG_DRIVER("Failed to compose. Invalid map in the primary
plane.");
return -EINVAL;
+ }
vaddr = primary_composer->map[0].vaddr;
@@ -198,10 +202,16 @@ static int compose_active_planes(void **vaddr_out,
* planes should be in z-order and compose them associatively:
* ((primary <- overlay) <- cursor)
*/
- for (i = 1; i < crtc_state->num_active_planes; i++)
- compose_plane(primary_composer,
- crtc_state->active_planes[i]->composer,
- *vaddr_out);
+ for (i = 1; i < crtc_state->num_active_planes; i++) {
+ ret = compose_plane(primary_composer,
+ crtc_state->active_planes[i]->composer,
+ *vaddr_out);
+ if (ret) {
+ DRM_DEBUG_DRIVER("Failed to compose. Invalid map in the
active_planes[%d].",
+i);
+ return ret;
+ }
+ }
return 0;
}
--
2.35.1
[PATCH v6] drm/msm/dp: stop event kernel thread when DP unbind
Current DP driver implementation, event thread is kept running
after DP display is unbind. This patch fix this problem by disabling
DP irq and stop event thread to exit gracefully at dp_display_unbind().
Changes in v2:
-- start event thread at dp_display_bind()
Changes in v3:
-- disable all HDP interrupts at unbind
-- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
-- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
-- move init_waitqueue_head(&dp->event_q) to probe()
-- move spin_lock_init(&dp->event_lock) to probe()
Changes in v4:
-- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
Changes in v5:
-- cancel relocation of both dp_display_bind() and dp_display_unbind()
Changes in v5:
-- move empty event q to dp_event_thread_start()
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
Reviewed-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_display.c | 35 ---
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..92c9819 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -113,6 +113,7 @@ struct dp_display_private {
u32 hpd_state;
u32 event_pndx;
u32 event_gndx;
+ struct task_struct *ev_tsk;
struct dp_event event_list[DP_EVENT_Q_MAX];
spinlock_t event_lock;
@@ -230,6 +231,14 @@ void dp_display_signal_audio_complete(struct msm_dp
*dp_display)
complete_all(&dp->audio_comp);
}
+static void dp_hpd_event_thread_stop(struct dp_display_private *dp_priv)
+{
+ kthread_stop(dp_priv->ev_tsk);
+
+}
+
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -269,6 +278,7 @@ static int dp_display_bind(struct device *dev, struct
device *master,
if (rc)
DRM_ERROR("Audio registration Dp failed\n");
+ rc = dp_hpd_event_thread_start(dp);
end:
return rc;
}
@@ -280,6 +290,9 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
struct drm_device *drm = dev_get_drvdata(master);
struct msm_drm_private *priv = drm->dev_private;
+ /* disable all HPD interrupts */
+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
+ dp_hpd_event_thread_stop(dp);
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
@@ -1054,7 +1067,7 @@ static int hpd_event_thread(void *data)
dp_priv = (struct dp_display_private *)data;
- while (1) {
+ while (!kthread_should_stop()) {
if (timeout_mode) {
wait_event_timeout(dp_priv->event_q,
(dp_priv->event_pndx == dp_priv->event_gndx),
@@ -1132,12 +1145,19 @@ static int hpd_event_thread(void *data)
return 0;
}
-static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
{
- init_waitqueue_head(&dp_priv->event_q);
- spin_lock_init(&dp_priv->event_lock);
+ /* set event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 0;
+
+ dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
"dp_hpd_handler");
+ if (IS_ERR(dp_priv->ev_tsk)) {
+ DRM_ERROR("failed to create DP event thread\n");
+ return PTR_ERR(dp_priv->ev_tsk);
+ }
- kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
+ return 0;
}
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
@@ -1266,7 +1286,10 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ /* setup event q */
mutex_init(&dp->event_mutex);
+ init_waitqueue_head(&dp->event_q);
+ spin_lock_init(&dp->event_lock);
/* Store DP audio handle inside DP display */
dp->dp_display.dp_audio = dp->audio;
@@ -1441,8 +1464,6 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
dp = container_of(dp_display, struct dp_display_private, dp_display);
- dp_hpd_event_setup(dp);
-
dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v2 5/7] drm: rcar-du: Factorise rcar_du_{atomic_check,modeset_init}
Hi Biju,
Thank you for the patch.
On Wed, Mar 16, 2022 at 01:10:58PM +, Biju Das wrote:
> RZ/G2L SoC's does not have group/plane registers compared to RCar, hence it
> needs a different CRTC implementation. Factorise rcar_du_{atomic_check,
> modeset_init} by adding struct rcar_du_crtc_helper_funcs to struct
> rcar_du_device_info, so that it can support RZ/G2L SoC without any code
> changes.
I'd like to go the other way around. Instead of adding a layer of
indirection in the DU driver, could you create a new DRM driver, and
share common code with the DU driver ?
> Signed-off-by: Biju Das
> ---
> v1->v2:
> * Changed crtc_helper_funcs->rcar_crtc_helper_funcs
> RFC->v1:
> * New patch
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 24
> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 16
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 6 +++---
> 3 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index f6e234dafb72..0df1430b9664 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -34,6 +34,12 @@
> * Device Information
> */
>
> +static struct rcar_du_crtc_helper_funcs rcar_crtc_helper_funcs = {
> + .du_planes_init = rcar_du_planes_init,
> + .du_crtc_create = rcar_du_crtc_create,
> + .du_atomic_check_planes = rcar_du_atomic_check_planes,
> +};
> +
> static const struct rcar_du_device_info rzg1_du_r8a7743_info = {
> .gen = 2,
> .features = RCAR_DU_FEATURE_CRTC_IRQ
> @@ -58,6 +64,7 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7743_info = {
> .num_rpf = 4,
> .max_width = 4095,
> .max_height = 2047,
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rzg1_du_r8a7745_info = {
> @@ -83,6 +90,7 @@ static const struct rcar_du_device_info
> rzg1_du_r8a7745_info = {
> .num_rpf = 4,
> .max_width = 4095,
> .max_height = 2047,
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rzg1_du_r8a77470_info = {
> @@ -113,6 +121,7 @@ static const struct rcar_du_device_info
> rzg1_du_r8a77470_info = {
> .num_rpf = 4,
> .max_width = 4095,
> .max_height = 2047,
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a774a1_info = {
> @@ -146,6 +155,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a774a1_info = {
> .max_width = 8190,
> .max_height = 8190,
> .dpll_mask = BIT(1),
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a774b1_info = {
> @@ -179,6 +189,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a774b1_info = {
> .max_width = 8190,
> .max_height = 8190,
> .dpll_mask = BIT(1),
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a774c0_info = {
> @@ -209,6 +220,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a774c0_info = {
> .max_width = 8190,
> .max_height = 8190,
> .lvds_clk_mask = BIT(1) | BIT(0),
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a774e1_info = {
> @@ -242,6 +254,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a774e1_info = {
> .max_width = 8190,
> .max_height = 8190,
> .dpll_mask = BIT(1),
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7779_info = {
> @@ -265,6 +278,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7779_info = {
> },
> .max_width = 4095,
> .max_height = 2047,
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7790_info = {
> @@ -298,6 +312,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7790_info = {
> .num_rpf = 4,
> .max_width = 4095,
> .max_height = 2047,
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> /* M2-W (r8a7791) and M2-N (r8a7793) are identical */
> @@ -326,6 +341,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7791_info = {
> .num_rpf = 4,
> .max_width = 4095,
> .max_height = 2047,
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7792_info = {
> @@ -349,6 +365,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7792_info = {
> .num_rpf = 4,
> .max_width = 4095,
> .max_height = 2047,
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static const struct rcar_du_device_info rcar_du_r8a7794_info = {
> @@ -375,6 +392,7 @@ static const struct rcar_du_device_info
> rcar_du_r8a7794_info = {
> .num_rpf = 4,
> .max_width = 4095,
> .max_height = 2047,
> + .fns = &rcar_crtc_helper_funcs,
> };
>
> static cons
[PATCH v2 0/2] drm/vc4: hdmi: Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Hi everyone, These patches replace the calls to drm_detect_hdmi_monitor() with the more efficient drm_display_info.is_hdmi in the VC4 driver. As I mentioned in v1, vc4_hdmi_encoder.hdmi_monitor (removed by this series) is used by some code not present in the mainline kernel but present in the Raspberry Pi tree [1]. Let me know if you want me to open a PR in the Raspberry Pi kernel project applying this series and fixing this issue. Thanks, José Expósito [1] https://github.com/raspberrypi/linux/blob/rpi-5.15.y/drivers/gpu/drm/vc4/vc4_firmware_kms.c#L1410 v1: https://lore.kernel.org/dri-devel/20220406165514.6106-1-jose.exposit...@gmail.com/ v2: Add the ftrace command used in the first patch Remove vc4_hdmi_encoder.hdmi_monitor (Thanks to Maxime for suggesting these changes) José Expósito (2): drm/vc4: hdmi: Replace drm_detect_hdmi_monitor() with is_hdmi drm/vc4: hdmi: Remove vc4_hdmi_encoder.hdmi_monitor drivers/gpu/drm/vc4/vc4_hdmi.c | 17 ++--- drivers/gpu/drm/vc4/vc4_hdmi.h | 1 - 2 files changed, 6 insertions(+), 12 deletions(-) -- 2.25.1
[PATCH v2 1/2] drm/vc4: hdmi: Replace drm_detect_hdmi_monitor() with is_hdmi
Once EDID is parsed, the monitor HDMI support information is cached in
drm_display_info.is_hdmi by drm_parse_hdmi_vsdb_video().
This driver calls drm_detect_hdmi_monitor() to receive the same
information and stores its own cached value, which is less efficient.
Avoid calling drm_detect_hdmi_monitor() and use drm_display_info.is_hdmi
instead.
drm_detect_hdmi_monitor() is called in vc4_hdmi_connector_detect() and
vc4_hdmi_connector_get_modes(). In both cases it is safe to rely on
drm_display_info.is_hdmi as shown by ftrace:
$ sudo trace-cmd record -p function_graph -l "vc4_hdmi_*" -l "drm_*"
vc4_hdmi_connector_detect:
vc4_hdmi_connector_detect() {
drm_get_edid() {
drm_connector_update_edid_property() {
drm_add_display_info() {
drm_reset_display_info();
drm_for_each_detailed_block.part.0();
drm_parse_cea_ext() {
drm_find_cea_extension();
drm_parse_hdmi_vsdb_video();
/* drm_display_info.is_hdmi is cached here */
}
}
}
}
/* drm_display_info.is_hdmi is used here */
}
vc4_hdmi_connector_get_modes:
vc4_hdmi_connector_get_modes() {
drm_get_edid() {
drm_connector_update_edid_property() {
drm_add_display_info() {
drm_reset_display_info();
drm_for_each_detailed_block.part.0();
drm_parse_cea_ext() {
drm_find_cea_extension();
drm_parse_hdmi_vsdb_video();
/* drm_display_info.is_hdmi is cached here */
}
}
}
}
/* drm_display_info.is_hdmi is used here */
drm_connector_update_edid_property();
}
Signed-off-by: José Expósito
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 6c58b0fd13fb..ecdb1ffc2023 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -216,7 +216,7 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (edid) {
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- vc4_hdmi->encoder.hdmi_monitor =
drm_detect_hdmi_monitor(edid);
+ vc4_hdmi->encoder.hdmi_monitor =
connector->display_info.is_hdmi;
kfree(edid);
}
}
@@ -255,7 +255,7 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
goto out;
}
- vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
+ vc4_encoder->hdmi_monitor = connector->display_info.is_hdmi;
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
--
2.25.1
[PATCH v2 2/2] drm/vc4: hdmi: Remove vc4_hdmi_encoder.hdmi_monitor
The vc4_hdmi_encoder.hdmi_monitor field was used to cache the value
returned by drm_detect_hdmi_monitor() in order to avoid calling it
multiple times.
Now that drm_detect_hdmi_monitor() has been replaced with
drm_display_info.is_hdmi, there is no need to cache it in the driver
encoder struct.
Remove vc4_hdmi_encoder.hdmi_monitor and use drm_display_info.is_hdmi
instead.
Signed-off-by: José Expósito
---
drivers/gpu/drm/vc4/vc4_hdmi.c | 17 ++---
drivers/gpu/drm/vc4/vc4_hdmi.h | 1 -
2 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index ecdb1ffc2023..9c73a211ae80 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -107,9 +107,9 @@ static bool vc4_hdmi_mode_needs_scrambling(const struct
drm_display_mode *mode)
static bool vc4_hdmi_is_full_range_rgb(struct vc4_hdmi *vc4_hdmi,
const struct drm_display_mode *mode)
{
- struct vc4_hdmi_encoder *vc4_encoder = &vc4_hdmi->encoder;
+ struct drm_display_info *display = &vc4_hdmi->connector.display_info;
- return !vc4_encoder->hdmi_monitor ||
+ return !display->is_hdmi ||
drm_default_rgb_quant_range(mode) ==
HDMI_QUANTIZATION_RANGE_FULL;
}
@@ -216,7 +216,6 @@ vc4_hdmi_connector_detect(struct drm_connector *connector,
bool force)
if (edid) {
cec_s_phys_addr_from_edid(vc4_hdmi->cec_adap,
edid);
- vc4_hdmi->encoder.hdmi_monitor =
connector->display_info.is_hdmi;
kfree(edid);
}
}
@@ -242,7 +241,6 @@ static void vc4_hdmi_connector_destroy(struct drm_connector
*connector)
static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
{
struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
- struct vc4_hdmi_encoder *vc4_encoder = &vc4_hdmi->encoder;
int ret = 0;
struct edid *edid;
@@ -255,8 +253,6 @@ static int vc4_hdmi_connector_get_modes(struct
drm_connector *connector)
goto out;
}
- vc4_encoder->hdmi_monitor = connector->display_info.is_hdmi;
-
drm_connector_update_edid_property(connector, edid);
ret = drm_add_edid_modes(connector, edid);
kfree(edid);
@@ -578,13 +574,12 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder
*encoder)
static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder,
struct drm_display_mode *mode)
{
- struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_info *display = &vc4_hdmi->connector.display_info;
lockdep_assert_held(&vc4_hdmi->mutex);
- if (!vc4_encoder->hdmi_monitor)
+ if (!display->is_hdmi)
return false;
if (!display->hdmi.scdc.supported ||
@@ -1147,7 +1142,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode;
- struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
+ struct drm_display_info *display = &vc4_hdmi->connector.display_info;
bool hsync_pos = mode->flags & DRM_MODE_FLAG_PHSYNC;
bool vsync_pos = mode->flags & DRM_MODE_FLAG_PVSYNC;
unsigned long flags;
@@ -1168,7 +1163,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
HDMI_WRITE(HDMI_VID_CTL,
HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_BLANKPIX);
- if (vc4_encoder->hdmi_monitor) {
+ if (display->is_hdmi) {
HDMI_WRITE(HDMI_SCHEDULER_CONTROL,
HDMI_READ(HDMI_SCHEDULER_CONTROL) |
VC4_HDMI_SCHEDULER_CONTROL_MODE_HDMI);
@@ -1195,7 +1190,7 @@ static void vc4_hdmi_encoder_post_crtc_enable(struct
drm_encoder *encoder,
"!VC4_HDMI_SCHEDULER_CONTROL_HDMI_ACTIVE\n");
}
- if (vc4_encoder->hdmi_monitor) {
+ if (display->is_hdmi) {
spin_lock_irqsave(&vc4_hdmi->hw_lock, flags);
WARN_ON(!(HDMI_READ(HDMI_SCHEDULER_CONTROL) &
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index 1076faeab616..44977002445f 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -11,7 +11,6 @@
/* VC4 HDMI encoder KMS struct */
struct vc4_hdmi_encoder {
struct vc4_encoder base;
- bool hdmi_monitor;
};
static inline struct vc4_hdmi_encoder *
--
2.25.1
[Bug 215839] New: distorted video playback with hybrid GPU (DRI_PRIME=1, Radeon HD 6470M and Intel-GPU)
https://bugzilla.kernel.org/show_bug.cgi?id=215839
Bug ID: 215839
Summary: distorted video playback with hybrid GPU (DRI_PRIME=1,
Radeon HD 6470M and Intel-GPU)
Product: Drivers
Version: 2.5
Kernel Version: 5.15.28
Hardware: All
OS: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: Video(DRI - non Intel)
Assignee: drivers_video-...@kernel-bugs.osdl.org
Reporter: kolafl...@kolahilft.de
Regression: No
At least Kodi and VLC show garbage when being run with DRI_PRIME=1
I guess this is either a bug in the "radeon" kernel module or in Mesa3D /
Gallium.
Hardware:
HP Notebook
Product: LR294EA#ABD
Model: Pavilion g6-1024sg
CPU + integrated graphics: Intel® Core™ i5 ("i915" driver)
Dedicated GPU: Radeon HD 6470M ("radeon" driver)
https://icecat.biz/de-at/p/hp/lr294ea/pavilion-notebooks-0886111633067-g6-1024sg-8025113.html
OS:
Tested Debian-11 (Linux-5.10.106)
manjaro-xfce-21.2.5-minimal-220314-linux515.iso (Linux-5.15.28)
(both running X)
$ cat /sys/kernel/debug/vgaswitcheroo/switch
0:DIS: :DynOff::01:00.0
1:IGD:+:Pwr::00:02.0
Console output (both VLC and Kodi):
radeon: The kernel rejected CS, see dmesg for more information (-22).
dmesg:
[ 2210.372308] radeon :01:00.0: evergreen_surface_check_linear_aligned:214
texture pitch 1920 invalid must be aligned with 256
[ 2210.372320] radeon :01:00.0: evergreen_cs_track_validate_texture:829
texture invalid 0x1dfc3bc1 0x1437 0x0b20 0x 0x
0x80020001
[ 2210.372327] [drm:radeon_cs_ioctl [radeon]] *ERROR* Invalid command stream !
I connected an external full HD (1920x1080) monitor and disabled the notebooks
internal monitor via xrandr.
(especially in Kodi you may not be able to reproduce slow rendering without an
1920x1080 monitor)
(I want to use the notebook as media computer for my TV)
When running Kodi-19.4 or VLC-3.0.17.3 without specific settings, full HD
(1920x1080 H264) videos don't render smoothly.
On Debian-11 additionally Firefox-ESR-91.8 also becomes slow (all CPU cores
under heavy load) when playing full HD videos with JavaScript controls.
Try these:
https://www.ardmediathek.de/video/seehund-puma-und-co/fuenf-mann-ueber-bord-215/das-erste/Y3JpZDovL2Rhc2Vyc3RlLmRlL3NlZWh1bmQsIHB1bWEgJmFtcDsgY28uLzI1MDdkNTZlLWI0ZjYtNDAxNS1iYTI1LTE1N2FjOWNjZTE3NA
https://pdvideosdaserste-a.akamaihd.net/int/2021/06/10/6528b8d0-72d0-45b4-9e7c-29dce5bb796a/1920-1_917111.mp4
(caution: baby seals shown compete hard with baby kittens ;-))
In Kodi install the "ARDundZDF" add-on from repo.kodinerds.net and open:
ARDundZDF -> TV-Livestreams -> Überregional (second option) -> Das Erste ->
1920x1080
To solve this for Firefox I simply added "export DRI_PRIME=1" to ~/.profile
Now Firefox-ESR-91.8 on Debian-11 works fine!
But the issue wasn't fixed for Kodi and VLC. Instead they now show complete
garbage :-(
For VLC I was able to nail down the problem:
When running VLC without specific settings, video output "OpenGL" and decoding
via "VA-API" (without DRM) are being chosen.
So when running VLC with these settings and with DRI_PRIME=1 the output is
distorted.
So for VLC the workaround is pretty easy:
Choose any other video decoder (VDPAU, "VA-API video decoder via DRM" or simply
"Disable").
In this case the video will playback fine and smooth with and without setting
DRI_PRIME=1.
Some other video output methods like "XVideo output (XCB)" also solve the
problem while keeping VA-API as video decoder. Actually I also reported a bug
to VLC about the questionable default settings:
https://code.videolan.org/videolan/vlc/-/issues/26831
Until now I found no solution for Kodi!
= Notes =
ffplay renders smoothly without further settings. (with and without
DRI_PRIME=1)
Maybe related:
https://bugzilla.kernel.org/show_bug.cgi?id=89331
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
[PATCH 2/2] drm/panel/raspberrypi-touchscreen: Initialise the bridge in prepare
From: Dave Stevenson
The panel has a prepare call which is before video starts, and an
enable call which is after.
The Toshiba bridge should be configured before video, so move
the relevant power and initialisation calls to prepare.
Fixes: 2f733d6194bd ("drm/panel: Add support for the Raspberry Pi 7"
Touchscreen.")
Signed-off-by: Dave Stevenson
Signed-off-by: Stefan Wahren
---
drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c | 11 +--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
index 1f805eb8fdb5..145047e19394 100644
--- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
+++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
@@ -265,7 +265,7 @@ static int rpi_touchscreen_noop(struct drm_panel *panel)
return 0;
}
-static int rpi_touchscreen_enable(struct drm_panel *panel)
+static int rpi_touchscreen_prepare(struct drm_panel *panel)
{
struct rpi_touchscreen *ts = panel_to_ts(panel);
int i;
@@ -295,6 +295,13 @@ static int rpi_touchscreen_enable(struct drm_panel *panel)
rpi_touchscreen_write(ts, DSI_STARTDSI, 0x01);
msleep(100);
+ return 0;
+}
+
+static int rpi_touchscreen_enable(struct drm_panel *panel)
+{
+ struct rpi_touchscreen *ts = panel_to_ts(panel);
+
/* Turn on the backlight. */
rpi_touchscreen_i2c_write(ts, REG_PWM, 255);
@@ -349,7 +356,7 @@ static int rpi_touchscreen_get_modes(struct drm_panel
*panel,
static const struct drm_panel_funcs rpi_touchscreen_funcs = {
.disable = rpi_touchscreen_disable,
.unprepare = rpi_touchscreen_noop,
- .prepare = rpi_touchscreen_noop,
+ .prepare = rpi_touchscreen_prepare,
.enable = rpi_touchscreen_enable,
.get_modes = rpi_touchscreen_get_modes,
};
--
2.25.1
[PATCH 0/2] drm/panel/raspberrypi-touchscreen: Fix minor issues
This small patch series tries to upstream 2 minor issues which has been fixed in the vendor tree by Dave Stevenson. Dave Stevenson (2): drm/panel/raspberrypi-touchscreen: Avoid NULL deref if not initialised drm/panel/raspberrypi-touchscreen: Initialise the bridge in prepare .../gpu/drm/panel/panel-raspberrypi-touchscreen.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) -- 2.25.1
[PATCH 1/2] drm/panel/raspberrypi-touchscreen: Avoid NULL deref if not initialised
From: Dave Stevenson
If a call to rpi_touchscreen_i2c_write from rpi_touchscreen_probe
fails before mipi_dsi_device_register_full is called, then
in trying to log the error message if uses ts->dsi->dev when
it is still NULL.
Use ts->i2c->dev instead, which is initialised earlier in probe.
Fixes: 2f733d6194bd ("drm/panel: Add support for the Raspberry Pi 7"
Touchscreen.")
Signed-off-by: Dave Stevenson
Signed-off-by: Stefan Wahren
---
drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
index 46029c5610c8..1f805eb8fdb5 100644
--- a/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
+++ b/drivers/gpu/drm/panel/panel-raspberrypi-touchscreen.c
@@ -229,7 +229,7 @@ static void rpi_touchscreen_i2c_write(struct
rpi_touchscreen *ts,
ret = i2c_smbus_write_byte_data(ts->i2c, reg, val);
if (ret)
- dev_err(&ts->dsi->dev, "I2C write failed: %d\n", ret);
+ dev_err(&ts->i2c->dev, "I2C write failed: %d\n", ret);
}
static int rpi_touchscreen_write(struct rpi_touchscreen *ts, u16 reg, u32 val)
--
2.25.1
[Bug 215839] distorted video playback with hybrid GPU (DRI_PRIME=1, Radeon HD 6470M and Intel-GPU)
https://bugzilla.kernel.org/show_bug.cgi?id=215839 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- This is a mesa bug. I'd suggest filing a bug here: https://gitlab.freedesktop.org/groups/mesa/-/issues There is either a memory alignment bug in the r600 mesa driver or the intel mesa driver is not allocating memory with the proper alignment requirements for the radeon hardware so when the radeon driver tries to use the buffer the commands get rejected causing the image distortions. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[Bug 215839] distorted video playback with hybrid GPU (DRI_PRIME=1, Radeon HD 6470M and Intel-GPU)
https://bugzilla.kernel.org/show_bug.cgi?id=215839 --- Comment #2 from kolAflash (kolafl...@kolahilft.de) --- Thanks for your assessment Alex! Here is the Mesa bug: https://gitlab.freedesktop.org/mesa/mesa/-/issues/6326 I can only set the status of this kernel.org bug to "resolved". But I guess it should be "invalid" or something else!? I also found a workaround for Kodi: Settings -> Player Set UI mode to >= Advanced Under "Videos" disable "Allow hardware acceleration - VAAPI" Then Kodi will work fine with DRI_PRIME=1 Interestingly even without DRI_PRIME=1 full HD videos now play smooth too. (maybe Kodi should leave VAAPI disabled by default...) -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
[PATCH] drm/amd/display: make hubp1_wait_pipe_read_start() static
It's a local function, let's make it static.
Signed-off-by: Tales Lelo da Aparecida
---
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
index fbff6beb78be..3a7f76e2c598 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn10/dcn10_hubp.c
@@ -1316,7 +1316,7 @@ void hubp1_set_flip_int(struct hubp *hubp)
*
* @hubp: hubp struct reference.
*/
-void hubp1_wait_pipe_read_start(struct hubp *hubp)
+static void hubp1_wait_pipe_read_start(struct hubp *hubp)
{
struct dcn10_hubp *hubp1 = TO_DCN10_HUBP(hubp);
--
2.35.1
[PATCH] drm/msm: remove unused hotplug and edid macros from msm_drv.h
Remove unused MSM_DISPLAY_CAP_HOT_PLUG and MSM_DISPLAY_CAP_EDID
macros from msm_drv.h.
Even if we need these, there are drm equivalent ones present.
Reported-by: Dmitry Baryshkov
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/msm_drv.h | 4
1 file changed, 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 9f68aa6..17de795 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -74,14 +74,10 @@ enum msm_dp_controller {
* enum msm_display_caps - features/capabilities supported by displays
* @MSM_DISPLAY_CAP_VID_MODE: Video or "active" mode supported
* @MSM_DISPLAY_CAP_CMD_MODE: Command mode supported
- * @MSM_DISPLAY_CAP_HOT_PLUG: Hot plug detection supported
- * @MSM_DISPLAY_CAP_EDID: EDID supported
*/
enum msm_display_caps {
MSM_DISPLAY_CAP_VID_MODE= BIT(0),
MSM_DISPLAY_CAP_CMD_MODE= BIT(1),
- MSM_DISPLAY_CAP_HOT_PLUG= BIT(2),
- MSM_DISPLAY_CAP_EDID= BIT(3),
};
/**
--
2.7.4
Re: [Freedreno] [PATCH 06/12] drm/msm/dpu: make changes to dpu_encoder to support virtual encoder
Hi Marijn
Looking at msm-next tip, this code has already been refactored in
https://gitlab.freedesktop.org/drm/msm/-/commit/ef58e0ad34365e2c8274b74e6e745b8c180ff0d3
So, I will just rebase my changes on msm-next tip and it should address
below comments as well.
Thanks
Abhinav
On 4/14/2022 3:30 PM, Abhinav Kumar wrote:
Hi Marijn
Thank you for your suggestion.
I will address it and add your "Reported by".
Thanks
Abhinav
On 4/14/2022 3:26 PM, Marijn Suijten wrote:
On 2022-02-04 13:17:19, Abhinav Kumar wrote:
Make changes to dpu_encoder to support virtual encoder needed
to support writeback for dpu.
Signed-off-by: Abhinav Kumar
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 57
+
1 file changed, 42 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index e977c05..947069b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -974,6 +974,7 @@ static void dpu_encoder_virt_mode_set(struct
drm_encoder *drm_enc,
struct dpu_hw_blk *hw_ctl[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_lm[MAX_CHANNELS_PER_ENC];
struct dpu_hw_blk *hw_dspp[MAX_CHANNELS_PER_ENC] = { NULL };
+ enum dpu_hw_blk_type blk_type;
int num_lm, num_ctl, num_pp;
int i, j;
@@ -1061,20 +1062,36 @@ static void dpu_encoder_virt_mode_set(struct
drm_encoder *drm_enc,
phys->hw_pp = dpu_enc->hw_pp[i];
phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
+ if (phys->intf_mode == INTF_MODE_WB_LINE)
+ blk_type = DPU_HW_BLK_WB;
+ else
+ blk_type = DPU_HW_BLK_INTF;
+
num_blk = dpu_rm_get_assigned_resources(&dpu_kms->rm,
- global_state, drm_enc->base.id, DPU_HW_BLK_INTF,
+ global_state, drm_enc->base.id, blk_type,
hw_blk, ARRAY_SIZE(hw_blk));
- for (j = 0; j < num_blk; j++) {
- struct dpu_hw_intf *hw_intf;
- hw_intf = to_dpu_hw_intf(hw_blk[i]);
- if (hw_intf->idx == phys->intf_idx)
- phys->hw_intf = hw_intf;
+ if (blk_type == DPU_HW_BLK_WB) {
+ for (j = 0; j < num_blk; j++) {
+ struct dpu_hw_wb *hw_wb;
+
+ hw_wb = to_dpu_hw_wb(hw_blk[i]);
+ if (hw_wb->idx == phys->intf_idx)
+ phys->hw_wb = hw_wb;
+ }
+ } else {
+ for (j = 0; j < num_blk; j++) {
+ struct dpu_hw_intf *hw_intf;
+
+ hw_intf = to_dpu_hw_intf(hw_blk[i]);
+ if (hw_intf->idx == phys->intf_idx)
+ phys->hw_intf = hw_intf;
+ }
It appears the original bit of code iterates j from 0 to num_blks yet
only uses i as iteration value. All of this code seems reentrant
meaning that executing it more than one times is redundant. You've
adopted this mistake, though I'd argue it should use j in hw_blk[i] as
that follows the number of elements in hw_blk returned by
dpu_rm_get_assigned_resources.
I suggest to address this in a separate patch (I can send that too, feel
free to add my Reported-by otherwise) and rebase this patch on top of
that, substituting i with j here as well. It seems to have been
introduced by b954fa6baaca ("drm/msm/dpu: Refactor rm iterator").
- Marijn
}
- if (!phys->hw_intf) {
+ if (!phys->hw_intf && !phys->hw_wb) {
DPU_ERROR_ENC(dpu_enc,
- "no intf block assigned at idx: %d\n", i);
+ "no intf or WB block assigned at idx: %d\n", i);
return;
}
@@ -1224,15 +1241,22 @@ static void dpu_encoder_virt_disable(struct
drm_encoder *drm_enc)
mutex_unlock(&dpu_enc->enc_lock);
}
-static enum dpu_intf dpu_encoder_get_intf(struct dpu_mdss_cfg *catalog,
+static enum dpu_intf dpu_encoder_get_intf_or_wb(struct dpu_mdss_cfg
*catalog,
enum dpu_intf_type type, u32 controller_id)
{
int i = 0;
- for (i = 0; i < catalog->intf_count; i++) {
- if (catalog->intf[i].type == type
- && catalog->intf[i].controller_id == controller_id) {
- return catalog->intf[i].id;
+ if (type != INTF_WB) {
+ for (i = 0; i < catalog->intf_count; i++) {
+ if (catalog->intf[i].type == type
+ && catalog->intf[i].controller_id == controller_id) {
+ return catalog->intf[i].id;
+ }
+ }
+ } else {
+ for (i = 0; i < catalog->wb_count; i++) {
+ if (catalog->wb[i].id == controller_id)
+ return catalog->wb[i].id;
}
}
@@ -2096,6 +2120,9 @@ static int dpu_encoder_setup_display(struct
dpu_encoder_virt *dpu_enc,
case DRM_MODE_ENCODER_TMDS:
intf_type = INTF_DP;
break;
+ case DRM_MODE_ENCODER_VIRTUAL:
+ intf_type = INTF_WB;
+ break;
}
WARN_ON(disp_info->num_of_h_tile
Re: [PATCH] drm/msm: remove unused hotplug and edid macros from msm_drv.h
Quoting Abhinav Kumar (2022-04-15 12:09:42) > Remove unused MSM_DISPLAY_CAP_HOT_PLUG and MSM_DISPLAY_CAP_EDID > macros from msm_drv.h. > > Even if we need these, there are drm equivalent ones present. > > Reported-by: Dmitry Baryshkov > Signed-off-by: Abhinav Kumar > --- Reviewed-by: Stephen Boyd
Re: [PATCH v6] drm/msm/dp: stop event kernel thread when DP unbind
Quoting Kuogee Hsieh (2022-04-15 08:41:16)
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 01453db..92c9819 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -230,6 +231,14 @@ void dp_display_signal_audio_complete(struct msm_dp
> *dp_display)
> complete_all(&dp->audio_comp);
> }
>
> +static void dp_hpd_event_thread_stop(struct dp_display_private *dp_priv)
> +{
> + kthread_stop(dp_priv->ev_tsk);
> +
> +}
> +
> +static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
> +
> static int dp_display_bind(struct device *dev, struct device *master,
>void *data)
> {
> @@ -280,6 +290,9 @@ static void dp_display_unbind(struct device *dev, struct
> device *master,
> struct drm_device *drm = dev_get_drvdata(master);
> struct msm_drm_private *priv = drm->dev_private;
>
> + /* disable all HPD interrupts */
> + dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
> + dp_hpd_event_thread_stop(dp);
It's a one line function. How about just
kthread_stop(dp->ev_tsk)
> dp_power_client_deinit(dp->power);
> dp_aux_unregister(dp->aux);
> priv->dp[dp->id] = NULL;
[PATCH 0/2] Update AMDGPU glossary and MAINTAINERS
I was handling the request from [0] and then I noticed that some AMD developers were missing from get_maintainers output due to the lack of a reference to their documentation in the MAINTAINERS file. [0] https://gitlab.freedesktop.org/drm/amd/-/issues/1939#note_1309737 Tales Lelo da Aparecida (2): Documentation/gpu: Add entries to amdgpu glossary MAINTAINERS: add docs entry to AMDGPU Documentation/gpu/amdgpu/amdgpu-glossary.rst | 13 + MAINTAINERS | 1 + 2 files changed, 14 insertions(+) -- 2.35.1
[PATCH 1/2] Documentation/gpu: Add entries to amdgpu glossary
Add missing acronyms to the amdgppu glossary. Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/1939#note_1309737 Signed-off-by: Tales Lelo da Aparecida --- Documentation/gpu/amdgpu/amdgpu-glossary.rst | 13 + 1 file changed, 13 insertions(+) diff --git a/Documentation/gpu/amdgpu/amdgpu-glossary.rst b/Documentation/gpu/amdgpu/amdgpu-glossary.rst index 859dcec6c6f9..48829d097f40 100644 --- a/Documentation/gpu/amdgpu/amdgpu-glossary.rst +++ b/Documentation/gpu/amdgpu/amdgpu-glossary.rst @@ -8,12 +8,19 @@ we have a dedicated glossary for Display Core at .. glossary:: +active_cu_number + The number of CUs that are active on the system. The number of active + CUs may be less than SE * SH * CU depending on the board configuration. + CP Command Processor CPLIB Content Protection Library +CU + Compute unit + DFS Digital Frequency Synthesizer @@ -74,6 +81,12 @@ we have a dedicated glossary for Display Core at SDMA System DMA +SE + Shader Engine + +SH + SHader array + SMU System Management Unit -- 2.35.1
[PATCH 2/2] MAINTAINERS: add docs entry to AMDGPU
To make sure maintainers of amdgpu drivers are aware of any changes in their documentation, add its entry to MAINTAINERS. Signed-off-by: Tales Lelo da Aparecida --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index d54b9f15ffce..b3594b2a09de 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16449,6 +16449,7 @@ S: Supported T: git https://gitlab.freedesktop.org/agd5f/linux.git B: https://gitlab.freedesktop.org/drm/amd/-/issues C: irc://irc.oftc.net/radeon +F: Documentation/gpu/amdgpu/ F: drivers/gpu/drm/amd/ F: drivers/gpu/drm/radeon/ F: include/uapi/drm/amdgpu_drm.h -- 2.35.1
[Bug 215842] New: PC freeze sidn kernel 5.17.x
https://bugzilla.kernel.org/show_bug.cgi?id=215842 Bug ID: 215842 Summary: PC freeze sidn kernel 5.17.x Product: Drivers Version: 2.5 Kernel Version: 5.17.3 Hardware: Intel OS: Linux Tree: Mainline Status: NEW Severity: blocking Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: andreykolt...@live.de Regression: No Hello, the display of my PC freezes (i cant also go to thy tty and ssh also down). I can always recreate this by going to https://github.com/jonaburg/picom and scrolling to the video. I can only see this behavior since the update to 5.17.x. Firefox version 99.0.1 (64-bit) is used as the browser. When switching to the LTS kernel, the problem disappears and the PC no longer freezes. This was tested many times on my productive system. I also tried different WMs (xmonad, qtile, KDE, i3) and switching the filesystem from btrfs to ext4 - but the problem remained. The PC has a SWAP. The XORG version is X.Org X Server 1.21.1.3. This Problem is also reproduceable on Wayland. I tested also by uninstall of picom and reinstall it etc. This are my components: Resolution: 2560x1440 OS: Arch Linux x86_64 CPU: Intel i5-6600K (4) @ 4.600GHz GPU: Intel HD Graphics 530 GPU: AMD ATI Radeon RX 470/480/570/570X/580/580X/590 Memory: 1049MiB / 14947MiB -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly
Hi,
On Thu, Apr 14, 2022 at 4:52 PM Stephen Boyd wrote:
>
> Quoting Douglas Anderson (2022-04-08 19:36:23)
> > As talked about in the kerneldoc for "struct dp_aux_ep_client" in this
> > patch and also in the past in commit a1e3667a9835 ("drm/bridge:
> > ti-sn65dsi86: Promote the AUX channel to its own sub-dev"), to use the
> > DP AUX bus properly we really need two "struct device"s. One "struct
> > device" is in charge of providing the DP AUX bus and the other is
> > where we'll try to get a reference to the newly probed endpoint
> > devices.
> >
> > In ti-sn65dsi86 this wasn't too difficult to accomplish. That driver
> > is already broken up into several "struct devices" anyway because it
> > also provides a PWM and some GPIOs. Adding one more wasn't that
> > difficult / ugly.
> >
> > When I tried to do the same solution in parade-ps8640, it felt like I
> > was copying too much boilerplate code. I made the realization that I
> > didn't _really_ need a separate "driver" for each person that wanted
> > to do the same thing. By putting all the "driver" related code in a
> > common place then we could save a bit of hassle. This change
> > effectively adds a new "ep_client" driver that can be used by
> > anyone. The devices instantiated by this driver will just call through
> > to the probe/remove/shutdown calls provided.
> >
> > At the moment, the "ep_client" driver is backed by the Linux auxiliary
> > bus (unfortunate naming--this has nothing to do with DP AUX). I didn't
> > want to expose this to clients, though, so as far as clients are
> > concerned they get a vanilla "struct device".
> >
> > Signed-off-by: Douglas Anderson
> > ---
>
> Is it correct that the struct dp_aux_ep_client is largely equivalent to
> a struct dp_aux_ep_device? What's the difference? I think it is a fully
> probed dp_aux_ep_device? Or a way to tell the driver that calls
> of_dp_aux_populate_ep_devices() that the endpoint has probed now?
They're not the same. The "DP AUX Endpoint Device" is essentially the
panel, though at one point in time someone argued that conceivably one
could put other devices on it even though this might be a really bad
idea. At some point in the discussion someone mentioned the concept of
a touchscreen running over DP Aux had been discussed and, indeed, "dp
aux touchscreen" gets hits if you search for it. The idea that it
could be something different is one reason why I refrained from
calling it a panel in all the function names and always tried to call
it a "DP AUX Endpoint".
The "DP AUX Endpoint Device Client" is the client of the panel, or the
code that needs to get a reference to the panel. Logically, I guess
this is the part of the eDP controller that's responsible for shoving
bits to the panel. Essentially the drm_bridge. Most importantly, it's
_not_ the part of the eDP controller providing the AUX bus.
> I read the commit text but it didn't click until I read the next patch
> that this is solving a probe ordering/loop problem. The driver that
> creates the 'struct drm_dp_aux' and populates devices on the DP aux bus
> with of_dp_aux_populate_ep_devices() wants to be sure that the
> drm_bridge made by the 'struct dp_aux_ep_driver' probe routine in
> edp-panel is going to be there before calling drm_of_get_bridge().
>
> of_dp_aux_populate_ep_devices();
> [No idea if the bridge is registered yet]
> drm_of_get_bridge();
>
> The solution is to retry the drm_of_get_bridge() until 'struct
> dp_aux_ep_driver' probes and registers the next bridge. It looks like a
> wait_for_completion() on top of drm_of_get_bridge() implemented through
> driver probe and -EPROBE_DEFER; no disrespect!
Yes, that's exactly what it is.
> Is there another problem here that the driver that creates the 'struct
> drm_dp_aux' and populates devices on the DP aux bus with
> of_dp_aux_populate_ep_devices() wants to use that same 'struct
> drm_dp_aux' directly to poke at some 'struct dp_aux_ep_device' that was
> populated? And the 'struct dp_aux_ep_device' may either not be probed
> and bound to a 'struct dp_aux_ep_driver' or it may not be powered on
> because it went to runtime PM suspend?
>
> Put another way, I worry that the owner of 'struct drm_dp_aux' wants to
> use some function in include/drm/dp/drm_dp_helper.h that takes the
> 'struct drm_dp_aux' directly without considering the device model state
> of the endpoint device (the 'struct dp_aux_ep_device'). That would be a
> similar problem as waiting for the endpoint to be powered on and probed.
> Solving that through a sub-driver feels awkward.
>
> What if we had some function like drm_dp_get_aux_ep() that took a
> 'struct drm_dp_aux' and looked for the endpoint device (there can only
> be one?),
The code is designed to handle the fact that there could be more than
one AUX endpoint device. I don't know if this will ever happen but it
is plausible. The "touchscreen over DP AUX", if that ever became a
thing supported in Linux, could be an example. In
Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly
Hi,
On Thu, Apr 14, 2022 at 5:47 PM Dmitry Baryshkov
wrote:
>
> On 09/04/2022 05:36, Douglas Anderson wrote:
> > As talked about in the kerneldoc for "struct dp_aux_ep_client" in this
> > patch and also in the past in commit a1e3667a9835 ("drm/bridge:
> > ti-sn65dsi86: Promote the AUX channel to its own sub-dev"), to use the
> > DP AUX bus properly we really need two "struct device"s. One "struct
> > device" is in charge of providing the DP AUX bus and the other is
> > where we'll try to get a reference to the newly probed endpoint
> > devices.
> >
> > In ti-sn65dsi86 this wasn't too difficult to accomplish. That driver
> > is already broken up into several "struct devices" anyway because it
> > also provides a PWM and some GPIOs. Adding one more wasn't that
> > difficult / ugly.
> >
> > When I tried to do the same solution in parade-ps8640, it felt like I
> > was copying too much boilerplate code. I made the realization that I
> > didn't _really_ need a separate "driver" for each person that wanted
> > to do the same thing. By putting all the "driver" related code in a
> > common place then we could save a bit of hassle. This change
> > effectively adds a new "ep_client" driver that can be used by
> > anyone. The devices instantiated by this driver will just call through
> > to the probe/remove/shutdown calls provided.
> >
> > At the moment, the "ep_client" driver is backed by the Linux auxiliary
> > bus (unfortunate naming--this has nothing to do with DP AUX). I didn't
> > want to expose this to clients, though, so as far as clients are
> > concerned they get a vanilla "struct device".
>
> I have been thinking about your approach for quite some time. I think
> that enforcing a use of auxilliary device is an overkill. What do we
> really need is the the set callbacks in the bus struct or a notifier. We
> have to notify the aux_bus controller side that the client has been
> probed successfully or that the client is going to be removed.
It seems like these new callbacks would be nearly the same as the
probe/remove callbacks in my proposal except:
* They rely on there being exactly 1 AUX device, or we make it a rule
that we wait for all AUX devices to probe (?)
* We need to come up with a system for detecting when everything
probes or is going to be removed, though that's probably not too hard.
I guess the DP AUX bus could just replace the panel's probe function
with its own and essentially "tail patch" it. I guess it could "head
patch" the remove call? ...or is there some better way you were
thinking of knowing when all our children probed?
* The callback on the aux bus controller side would not be able to
DEFER. In other words trying to acquire a reference to the panel can
always be the last thing we do so we know there can be no reasons to
defer after. This should be doable, but at least in the ps8640 case it
will require changing the code a bit. I notice that today it actually
tries to get the panel side _before_ it gets the MIPI side and it
potentially can return -EPROBE_DEFER if it can't find the MIPI side. I
guess I have a niggling feeling that we'll find some reason in the
future that we can't be last, but we can probably ignore that. ;-)
I can switch this all to normal callbacks if that's what everyone
wants, but it doesn't feel significantly cleaner to me and does seem
to have some (small) downsides.
> And this
> approach would make driver's life easier, since e.g. the bus code can
> pm_get the EP device before calling callbacks/notifiers and
> pm_put_autosuspend it afterwards.
Not sure about doing the pm calls on behalf of the EP device. What's
the goal there?
Re: [RFC PATCH 4/6] drm/panel-edp: Take advantage of is_hpd_asserted() in struct drm_dp_aux
Hi,
On Thu, Apr 14, 2022 at 5:51 PM Dmitry Baryshkov
wrote:
>
> On 09/04/2022 05:36, Douglas Anderson wrote:
> > Let's add support for being able to read the HPD pin even if it's
> > hooked directly to the controller. This will allow us to get more
> > accurate delays also lets us take away the waiting in the AUX transfer
> > functions of the eDP controller drivers.
> >
> > Signed-off-by: Douglas Anderson
> > ---
> >
> > drivers/gpu/drm/panel/panel-edp.c | 37 ++-
> > 1 file changed, 31 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/panel/panel-edp.c
> > b/drivers/gpu/drm/panel/panel-edp.c
> > index 1732b4f56e38..4a143eb9544b 100644
> > --- a/drivers/gpu/drm/panel/panel-edp.c
> > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > @@ -417,6 +417,19 @@ static int panel_edp_get_hpd_gpio(struct device *dev,
> > struct panel_edp *p)
> > return 0;
> > }
> >
> > +static bool panel_edp_can_read_hpd(struct panel_edp *p)
> > +{
> > + return !p->no_hpd && (p->hpd_gpio || (p->aux &&
> > p->aux->is_hpd_asserted));
> > +}
> > +
> > +static bool panel_edp_read_hpd(struct panel_edp *p)
> > +{
> > + if (p->hpd_gpio)
> > + return gpiod_get_value_cansleep(p->hpd_gpio);
> > +
> > + return p->aux->is_hpd_asserted(p->aux);
> > +}
> > +
> > static int panel_edp_prepare_once(struct panel_edp *p)
> > {
> > struct device *dev = p->base.dev;
> > @@ -441,13 +454,21 @@ static int panel_edp_prepare_once(struct panel_edp *p)
> > if (delay)
> > msleep(delay);
> >
> > - if (p->hpd_gpio) {
> > + if (panel_edp_can_read_hpd(p)) {
> > if (p->desc->delay.hpd_absent)
> > hpd_wait_us = p->desc->delay.hpd_absent * 1000UL;
> > else
> > hpd_wait_us = 200;
> >
> > - err = readx_poll_timeout(gpiod_get_value_cansleep,
> > p->hpd_gpio,
> > + /*
> > + * Extra max delay, mostly to account for ps8640. ps8640
> > + * is crazy and the bridge chip driver itself has over 200 ms
> > + * of delay if it needs to do the pm_runtime resume of the
> > + * bridge chip to read the HPD.
> > + */
> > + hpd_wait_us += 300;
>
> I think this should come in a separate commit and ideally this should be
> configurable somehow. Other hosts wouldn't need such 'additional' delay.
>
> With this change removed:
>
> Reviewed-by: Dmitry Baryshkov
What would you think about changing the API slightly? Instead of
is_hpd_asserted(), we change it to wait_hpd_asserted() and it takes a
timeout in microseconds. If you pass 0 for the timeout the function is
defined to behave the same as is_hpd_asserted() today--AKA a single
poll of the line.
-Doug
Re: [RFC PATCH 4/6] drm/panel-edp: Take advantage of is_hpd_asserted() in struct drm_dp_aux
On Sat, 16 Apr 2022 at 00:17, Doug Anderson wrote:
>
> Hi,
>
> On Thu, Apr 14, 2022 at 5:51 PM Dmitry Baryshkov
> wrote:
> >
> > On 09/04/2022 05:36, Douglas Anderson wrote:
> > > Let's add support for being able to read the HPD pin even if it's
> > > hooked directly to the controller. This will allow us to get more
> > > accurate delays also lets us take away the waiting in the AUX transfer
> > > functions of the eDP controller drivers.
> > >
> > > Signed-off-by: Douglas Anderson
> > > ---
> > >
> > > drivers/gpu/drm/panel/panel-edp.c | 37 ++-
> > > 1 file changed, 31 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/panel/panel-edp.c
> > > b/drivers/gpu/drm/panel/panel-edp.c
> > > index 1732b4f56e38..4a143eb9544b 100644
> > > --- a/drivers/gpu/drm/panel/panel-edp.c
> > > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > > @@ -417,6 +417,19 @@ static int panel_edp_get_hpd_gpio(struct device
> > > *dev, struct panel_edp *p)
> > > return 0;
> > > }
> > >
> > > +static bool panel_edp_can_read_hpd(struct panel_edp *p)
> > > +{
> > > + return !p->no_hpd && (p->hpd_gpio || (p->aux &&
> > > p->aux->is_hpd_asserted));
> > > +}
> > > +
> > > +static bool panel_edp_read_hpd(struct panel_edp *p)
> > > +{
> > > + if (p->hpd_gpio)
> > > + return gpiod_get_value_cansleep(p->hpd_gpio);
> > > +
> > > + return p->aux->is_hpd_asserted(p->aux);
> > > +}
> > > +
> > > static int panel_edp_prepare_once(struct panel_edp *p)
> > > {
> > > struct device *dev = p->base.dev;
> > > @@ -441,13 +454,21 @@ static int panel_edp_prepare_once(struct panel_edp
> > > *p)
> > > if (delay)
> > > msleep(delay);
> > >
> > > - if (p->hpd_gpio) {
> > > + if (panel_edp_can_read_hpd(p)) {
> > > if (p->desc->delay.hpd_absent)
> > > hpd_wait_us = p->desc->delay.hpd_absent * 1000UL;
> > > else
> > > hpd_wait_us = 200;
> > >
> > > - err = readx_poll_timeout(gpiod_get_value_cansleep,
> > > p->hpd_gpio,
> > > + /*
> > > + * Extra max delay, mostly to account for ps8640. ps8640
> > > + * is crazy and the bridge chip driver itself has over 200
> > > ms
> > > + * of delay if it needs to do the pm_runtime resume of the
> > > + * bridge chip to read the HPD.
> > > + */
> > > + hpd_wait_us += 300;
> >
> > I think this should come in a separate commit and ideally this should be
> > configurable somehow. Other hosts wouldn't need such 'additional' delay.
> >
> > With this change removed:
> >
> > Reviewed-by: Dmitry Baryshkov
>
> What would you think about changing the API slightly? Instead of
> is_hpd_asserted(), we change it to wait_hpd_asserted() and it takes a
> timeout in microseconds. If you pass 0 for the timeout the function is
> defined to behave the same as is_hpd_asserted() today--AKA a single
> poll of the line.
This might work. Can you check it, please?
BTW: are these changes dependent on the first part of the patchset? It
might be worth splitting the patchset into two parts.
--
With best wishes
Dmitry
[PATCH v7] drm/msm/dp: stop event kernel thread when DP unbind
Current DP driver implementation, event thread is kept running
after DP display is unbind. This patch fix this problem by disabling
DP irq and stop event thread to exit gracefully at dp_display_unbind().
Changes in v2:
-- start event thread at dp_display_bind()
Changes in v3:
-- disable all HDP interrupts at unbind
-- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
-- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
-- move init_waitqueue_head(&dp->event_q) to probe()
-- move spin_lock_init(&dp->event_lock) to probe()
Changes in v4:
-- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
Changes in v5:
-- cancel relocation of both dp_display_bind() and dp_display_unbind()
Changes in v6:
-- move empty event q to dp_event_thread_start()
Changes in v7:
-- call ktheread_stop() directly instead of dp_hpd_event_thread_stop() function
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
Reviewed-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_display.c | 31 ---
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..680e500 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -113,6 +113,7 @@ struct dp_display_private {
u32 hpd_state;
u32 event_pndx;
u32 event_gndx;
+ struct task_struct *ev_tsk;
struct dp_event event_list[DP_EVENT_Q_MAX];
spinlock_t event_lock;
@@ -230,6 +231,8 @@ void dp_display_signal_audio_complete(struct msm_dp
*dp_display)
complete_all(&dp->audio_comp);
}
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -269,6 +272,7 @@ static int dp_display_bind(struct device *dev, struct
device *master,
if (rc)
DRM_ERROR("Audio registration Dp failed\n");
+ rc = dp_hpd_event_thread_start(dp);
end:
return rc;
}
@@ -280,6 +284,11 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
struct drm_device *drm = dev_get_drvdata(master);
struct msm_drm_private *priv = drm->dev_private;
+ /* disable all HPD interrupts */
+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
+
+ kthread_stop(dp->ev_tsk);
+
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
@@ -1054,7 +1063,7 @@ static int hpd_event_thread(void *data)
dp_priv = (struct dp_display_private *)data;
- while (1) {
+ while (!kthread_should_stop()) {
if (timeout_mode) {
wait_event_timeout(dp_priv->event_q,
(dp_priv->event_pndx == dp_priv->event_gndx),
@@ -1132,12 +1141,19 @@ static int hpd_event_thread(void *data)
return 0;
}
-static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
{
- init_waitqueue_head(&dp_priv->event_q);
- spin_lock_init(&dp_priv->event_lock);
+ /* set event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 0;
+
+ dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
"dp_hpd_handler");
+ if (IS_ERR(dp_priv->ev_tsk)) {
+ DRM_ERROR("failed to create DP event thread\n");
+ return PTR_ERR(dp_priv->ev_tsk);
+ }
- kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
+ return 0;
}
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
@@ -1266,7 +1282,10 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ /* setup event q */
mutex_init(&dp->event_mutex);
+ init_waitqueue_head(&dp->event_q);
+ spin_lock_init(&dp->event_lock);
/* Store DP audio handle inside DP display */
dp->dp_display.dp_audio = dp->audio;
@@ -1441,8 +1460,6 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
dp = container_of(dp_display, struct dp_display_private, dp_display);
- dp_hpd_event_setup(dp);
-
dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly
On Sat, 16 Apr 2022 at 00:13, Doug Anderson wrote:
>
> Hi,
>
> On Thu, Apr 14, 2022 at 5:47 PM Dmitry Baryshkov
> wrote:
> >
> > On 09/04/2022 05:36, Douglas Anderson wrote:
> > > As talked about in the kerneldoc for "struct dp_aux_ep_client" in this
> > > patch and also in the past in commit a1e3667a9835 ("drm/bridge:
> > > ti-sn65dsi86: Promote the AUX channel to its own sub-dev"), to use the
> > > DP AUX bus properly we really need two "struct device"s. One "struct
> > > device" is in charge of providing the DP AUX bus and the other is
> > > where we'll try to get a reference to the newly probed endpoint
> > > devices.
> > >
> > > In ti-sn65dsi86 this wasn't too difficult to accomplish. That driver
> > > is already broken up into several "struct devices" anyway because it
> > > also provides a PWM and some GPIOs. Adding one more wasn't that
> > > difficult / ugly.
> > >
> > > When I tried to do the same solution in parade-ps8640, it felt like I
> > > was copying too much boilerplate code. I made the realization that I
> > > didn't _really_ need a separate "driver" for each person that wanted
> > > to do the same thing. By putting all the "driver" related code in a
> > > common place then we could save a bit of hassle. This change
> > > effectively adds a new "ep_client" driver that can be used by
> > > anyone. The devices instantiated by this driver will just call through
> > > to the probe/remove/shutdown calls provided.
> > >
> > > At the moment, the "ep_client" driver is backed by the Linux auxiliary
> > > bus (unfortunate naming--this has nothing to do with DP AUX). I didn't
> > > want to expose this to clients, though, so as far as clients are
> > > concerned they get a vanilla "struct device".
> >
> > I have been thinking about your approach for quite some time. I think
> > that enforcing a use of auxilliary device is an overkill. What do we
> > really need is the the set callbacks in the bus struct or a notifier. We
> > have to notify the aux_bus controller side that the client has been
> > probed successfully or that the client is going to be removed.
>
> It seems like these new callbacks would be nearly the same as the
> probe/remove callbacks in my proposal except:
>
> * They rely on there being exactly 1 AUX device, or we make it a rule
> that we wait for all AUX devices to probe (?)
Is the backlight a separate device on an AUX bus? Judging from
drm_panel_dp_aux_backlight(), it isn't. I assumed that aux bus is just
a point-to-point bus, so there is always a single client.
>
> * We need to come up with a system for detecting when everything
> probes or is going to be removed, though that's probably not too hard.
> I guess the DP AUX bus could just replace the panel's probe function
> with its own and essentially "tail patch" it. I guess it could "head
> patch" the remove call? ...or is there some better way you were
> thinking of knowing when all our children probed?
>
> * The callback on the aux bus controller side would not be able to
> DEFER. In other words trying to acquire a reference to the panel can
> always be the last thing we do so we know there can be no reasons to
> defer after. This should be doable, but at least in the ps8640 case it
> will require changing the code a bit. I notice that today it actually
> tries to get the panel side _before_ it gets the MIPI side and it
> potentially can return -EPROBE_DEFER if it can't find the MIPI side. I
> guess I have a niggling feeling that we'll find some reason in the
> future that we can't be last, but we can probably ignore that. ;-)
>
> I can switch this all to normal callbacks if that's what everyone
> wants, but it doesn't feel significantly cleaner to me and does seem
> to have some (small) downsides.
>
>
> > And this
> > approach would make driver's life easier, since e.g. the bus code can
> > pm_get the EP device before calling callbacks/notifiers and
> > pm_put_autosuspend it afterwards.
>
> Not sure about doing the pm calls on behalf of the EP device. What's
> the goal there?
I think any driver can pm_runtime_get another device. The goal is to
let the 'post_probe' callback to power up the panel, read the EDID,
etc.
BTW: as I'm slowly diving into DP vs eDP differences. Do we need to
write the EDID checksum like we do for DP?
Do you have any good summary for eDP vs DP differences?
--
With best wishes
Dmitry
Re: [PATCH v7] drm/msm/dp: stop event kernel thread when DP unbind
On Sat, 16 Apr 2022 at 01:34, Kuogee Hsieh wrote:
>
> Current DP driver implementation, event thread is kept running
> after DP display is unbind. This patch fix this problem by disabling
> DP irq and stop event thread to exit gracefully at dp_display_unbind().
>
> Changes in v2:
> -- start event thread at dp_display_bind()
>
> Changes in v3:
> -- disable all HDP interrupts at unbind
> -- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
> -- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
> -- move init_waitqueue_head(&dp->event_q) to probe()
> -- move spin_lock_init(&dp->event_lock) to probe()
>
> Changes in v4:
> -- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
>
> Changes in v5:
> -- cancel relocation of both dp_display_bind() and dp_display_unbind()
>
> Changes in v6:
> -- move empty event q to dp_event_thread_start()
>
> Changes in v7:
> -- call ktheread_stop() directly instead of dp_hpd_event_thread_stop()
> function
>
> Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
> Chipsets")
> Signed-off-by: Kuogee Hsieh
> Reported-by: Dmitry Baryshkov
> Reviewed-by: Stephen Boyd
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 31 ---
> 1 file changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 01453db..680e500 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -113,6 +113,7 @@ struct dp_display_private {
> u32 hpd_state;
> u32 event_pndx;
> u32 event_gndx;
> + struct task_struct *ev_tsk;
> struct dp_event event_list[DP_EVENT_Q_MAX];
> spinlock_t event_lock;
>
> @@ -230,6 +231,8 @@ void dp_display_signal_audio_complete(struct msm_dp
> *dp_display)
> complete_all(&dp->audio_comp);
> }
>
> +static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
> +
> static int dp_display_bind(struct device *dev, struct device *master,
>void *data)
> {
> @@ -269,6 +272,7 @@ static int dp_display_bind(struct device *dev, struct
> device *master,
> if (rc)
> DRM_ERROR("Audio registration Dp failed\n");
I think I asked it in v3 and didn't get an answer:
Isn't 'goto end' missing here?
>
> + rc = dp_hpd_event_thread_start(dp);
if (rc) { DRM_ERROR(); goto end; }
return 0;
Please.
> end:
> return rc;
> }
> @@ -280,6 +284,11 @@ static void dp_display_unbind(struct device *dev, struct
> device *master,
> struct drm_device *drm = dev_get_drvdata(master);
> struct msm_drm_private *priv = drm->dev_private;
>
> + /* disable all HPD interrupts */
> + dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
> +
> + kthread_stop(dp->ev_tsk);
> +
> dp_power_client_deinit(dp->power);
> dp_aux_unregister(dp->aux);
> priv->dp[dp->id] = NULL;
> @@ -1054,7 +1063,7 @@ static int hpd_event_thread(void *data)
>
> dp_priv = (struct dp_display_private *)data;
>
> - while (1) {
> + while (!kthread_should_stop()) {
> if (timeout_mode) {
> wait_event_timeout(dp_priv->event_q,
> (dp_priv->event_pndx == dp_priv->event_gndx),
> @@ -1132,12 +1141,19 @@ static int hpd_event_thread(void *data)
> return 0;
> }
>
> -static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
> +static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
> {
> - init_waitqueue_head(&dp_priv->event_q);
> - spin_lock_init(&dp_priv->event_lock);
> + /* set event q to empty */
> + dp_priv->event_gndx = 0;
> + dp_priv->event_pndx = 0;
> +
> + dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
> "dp_hpd_handler");
> + if (IS_ERR(dp_priv->ev_tsk)) {
> + DRM_ERROR("failed to create DP event thread\n");
> + return PTR_ERR(dp_priv->ev_tsk);
> + }
>
> - kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
> + return 0;
> }
>
> static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
> @@ -1266,7 +1282,10 @@ static int dp_display_probe(struct platform_device
> *pdev)
> return -EPROBE_DEFER;
> }
>
> + /* setup event q */
> mutex_init(&dp->event_mutex);
> + init_waitqueue_head(&dp->event_q);
> + spin_lock_init(&dp->event_lock);
>
> /* Store DP audio handle inside DP display */
> dp->dp_display.dp_audio = dp->audio;
> @@ -1441,8 +1460,6 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
>
> dp = container_of(dp_display, struct dp_display_private, dp_display);
>
> - dp_hpd_event_setup(dp);
> -
> dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
> }
>
> --
> The Qualcomm Innovation Center, Inc. is a member of the Cod
Re: [PATCH v7] drm/msm/dp: stop event kernel thread when DP unbind
On 4/15/2022 3:48 PM, Dmitry Baryshkov wrote:
On Sat, 16 Apr 2022 at 01:34, Kuogee Hsieh wrote:
Current DP driver implementation, event thread is kept running
after DP display is unbind. This patch fix this problem by disabling
DP irq and stop event thread to exit gracefully at dp_display_unbind().
Changes in v2:
-- start event thread at dp_display_bind()
Changes in v3:
-- disable all HDP interrupts at unbind
-- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
-- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
-- move init_waitqueue_head(&dp->event_q) to probe()
-- move spin_lock_init(&dp->event_lock) to probe()
Changes in v4:
-- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
Changes in v5:
-- cancel relocation of both dp_display_bind() and dp_display_unbind()
Changes in v6:
-- move empty event q to dp_event_thread_start()
Changes in v7:
-- call ktheread_stop() directly instead of dp_hpd_event_thread_stop() function
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
Reviewed-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_display.c | 31 ---
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..680e500 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -113,6 +113,7 @@ struct dp_display_private {
u32 hpd_state;
u32 event_pndx;
u32 event_gndx;
+ struct task_struct *ev_tsk;
struct dp_event event_list[DP_EVENT_Q_MAX];
spinlock_t event_lock;
@@ -230,6 +231,8 @@ void dp_display_signal_audio_complete(struct msm_dp
*dp_display)
complete_all(&dp->audio_comp);
}
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -269,6 +272,7 @@ static int dp_display_bind(struct device *dev, struct
device *master,
if (rc)
DRM_ERROR("Audio registration Dp failed\n");
I think I asked it in v3 and didn't get an answer:
Isn't 'goto end' missing here?
I did follow you suggestion at v3 and made changes at
dp_hpd_event_thread_start()
+ rc = dp_hpd_event_thread_start(dp);
if (rc) { DRM_ERROR(); goto end; }
return 0;
Please.
This is at dp_display_bind(), "end" is the next statement why goto needed?
end:
return rc;
}
@@ -280,6 +284,11 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
struct drm_device *drm = dev_get_drvdata(master);
struct msm_drm_private *priv = drm->dev_private;
+ /* disable all HPD interrupts */
+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
+
+ kthread_stop(dp->ev_tsk);
+
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
@@ -1054,7 +1063,7 @@ static int hpd_event_thread(void *data)
dp_priv = (struct dp_display_private *)data;
- while (1) {
+ while (!kthread_should_stop()) {
if (timeout_mode) {
wait_event_timeout(dp_priv->event_q,
(dp_priv->event_pndx == dp_priv->event_gndx),
@@ -1132,12 +1141,19 @@ static int hpd_event_thread(void *data)
return 0;
}
-static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
{
- init_waitqueue_head(&dp_priv->event_q);
- spin_lock_init(&dp_priv->event_lock);
+ /* set event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 0;
+
+ dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
"dp_hpd_handler");
+ if (IS_ERR(dp_priv->ev_tsk)) {
+ DRM_ERROR("failed to create DP event thread\n");
+ return PTR_ERR(dp_priv->ev_tsk);
+ }
- kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
+ return 0;
}
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
@@ -1266,7 +1282,10 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ /* setup event q */
mutex_init(&dp->event_mutex);
+ init_waitqueue_head(&dp->event_q);
+ spin_lock_init(&dp->event_lock);
/* Store DP audio handle inside DP display */
dp->dp_display.dp_audio = dp->audio;
@@ -1441,8 +1460,6 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
dp = container_of(dp_display, struct dp_display_private, dp_display);
- dp_hpd_event_setup(dp);
-
dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
Re: [Freedreno] [PATCH 06/12] drm/msm/dpu: make changes to dpu_encoder to support virtual encoder
Hi Abhinav, On 2022-04-15 12:25:55, Abhinav Kumar wrote: > Hi Marijn > > Looking at msm-next tip, this code has already been refactored in > > https://gitlab.freedesktop.org/drm/msm/-/commit/ef58e0ad34365e2c8274b74e6e745b8c180ff0d3 > > So, I will just rebase my changes on msm-next tip and it should address > below comments as well. That's actually great, I love patches that remove a whole lot of (especially bug-containing) lines, awesome Dmitry! Looking forward to the next revision, spotted some minor nits in this revision but it probably makes little sense to point them out here presuming they might have already been addressed on your end or removed altogether. - Marijn
Re: [PATCH v7] drm/msm/dp: stop event kernel thread when DP unbind
On Sat, 16 Apr 2022 at 02:10, Kuogee Hsieh wrote:
>
>
> On 4/15/2022 3:48 PM, Dmitry Baryshkov wrote:
> > On Sat, 16 Apr 2022 at 01:34, Kuogee Hsieh wrote:
> >> Current DP driver implementation, event thread is kept running
> >> after DP display is unbind. This patch fix this problem by disabling
> >> DP irq and stop event thread to exit gracefully at dp_display_unbind().
> >>
> >> Changes in v2:
> >> -- start event thread at dp_display_bind()
> >>
> >> Changes in v3:
> >> -- disable all HDP interrupts at unbind
> >> -- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
> >> -- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
> >> -- move init_waitqueue_head(&dp->event_q) to probe()
> >> -- move spin_lock_init(&dp->event_lock) to probe()
> >>
> >> Changes in v4:
> >> -- relocate both dp_display_bind() and dp_display_unbind() to bottom of
> >> file
> >>
> >> Changes in v5:
> >> -- cancel relocation of both dp_display_bind() and dp_display_unbind()
> >>
> >> Changes in v6:
> >> -- move empty event q to dp_event_thread_start()
> >>
> >> Changes in v7:
> >> -- call ktheread_stop() directly instead of dp_hpd_event_thread_stop()
> >> function
> >>
> >> Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
> >> Chipsets")
> >> Signed-off-by: Kuogee Hsieh
> >> Reported-by: Dmitry Baryshkov
> >> Reviewed-by: Stephen Boyd
> >> ---
> >> drivers/gpu/drm/msm/dp/dp_display.c | 31 ---
> >> 1 file changed, 24 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> >> b/drivers/gpu/drm/msm/dp/dp_display.c
> >> index 01453db..680e500 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> >> @@ -113,6 +113,7 @@ struct dp_display_private {
> >> u32 hpd_state;
> >> u32 event_pndx;
> >> u32 event_gndx;
> >> + struct task_struct *ev_tsk;
> >> struct dp_event event_list[DP_EVENT_Q_MAX];
> >> spinlock_t event_lock;
> >>
> >> @@ -230,6 +231,8 @@ void dp_display_signal_audio_complete(struct msm_dp
> >> *dp_display)
> >> complete_all(&dp->audio_comp);
> >> }
> >>
> >> +static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
> >> +
> >> static int dp_display_bind(struct device *dev, struct device *master,
> >> void *data)
> >> {
> >> @@ -269,6 +272,7 @@ static int dp_display_bind(struct device *dev, struct
> >> device *master,
> >> if (rc)
> >> DRM_ERROR("Audio registration Dp failed\n");
> > I think I asked it in v3 and didn't get an answer:
> > Isn't 'goto end' missing here?
> I did follow you suggestion at v3 and made changes at
> dp_hpd_event_thread_start()
I'm asking about this place. Should we return an immediate error if
audio registration has failed or not?
> >> + rc = dp_hpd_event_thread_start(dp);
> > if (rc) { DRM_ERROR(); goto end; }
> > return 0;
> >
> > Please.
> This is at dp_display_bind(), "end" is the next statement why goto needed?
To be explicit that this is a fatal error. Compare this to my question
regarding the audio registration failure.
> >> end:
> >> return rc;
> >> }
> >> @@ -280,6 +284,11 @@ static void dp_display_unbind(struct device *dev,
> >> struct device *master,
> >> struct drm_device *drm = dev_get_drvdata(master);
> >> struct msm_drm_private *priv = drm->dev_private;
> >>
> >> + /* disable all HPD interrupts */
> >> + dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
> >> +
> >> + kthread_stop(dp->ev_tsk);
> >> +
> >> dp_power_client_deinit(dp->power);
> >> dp_aux_unregister(dp->aux);
> >> priv->dp[dp->id] = NULL;
> >> @@ -1054,7 +1063,7 @@ static int hpd_event_thread(void *data)
> >>
> >> dp_priv = (struct dp_display_private *)data;
> >>
> >> - while (1) {
> >> + while (!kthread_should_stop()) {
> >> if (timeout_mode) {
> >> wait_event_timeout(dp_priv->event_q,
> >> (dp_priv->event_pndx ==
> >> dp_priv->event_gndx),
> >> @@ -1132,12 +1141,19 @@ static int hpd_event_thread(void *data)
> >> return 0;
> >> }
> >>
> >> -static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
> >> +static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
> >> {
> >> - init_waitqueue_head(&dp_priv->event_q);
> >> - spin_lock_init(&dp_priv->event_lock);
> >> + /* set event q to empty */
> >> + dp_priv->event_gndx = 0;
> >> + dp_priv->event_pndx = 0;
> >> +
> >> + dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
> >> "dp_hpd_handler");
> >> + if (IS_ERR(dp_priv->ev_tsk)) {
> >> + DRM_ERROR("failed to create DP event thread\n");
> >> + return PTR_ERR(dp_priv->ev_tsk);
> >> + }
> >>
> >> - kthre
Re: [PATCH v7] drm/msm/dp: stop event kernel thread when DP unbind
On 4/15/2022 4:27 PM, Dmitry Baryshkov wrote:
On Sat, 16 Apr 2022 at 02:10, Kuogee Hsieh wrote:
On 4/15/2022 3:48 PM, Dmitry Baryshkov wrote:
On Sat, 16 Apr 2022 at 01:34, Kuogee Hsieh wrote:
Current DP driver implementation, event thread is kept running
after DP display is unbind. This patch fix this problem by disabling
DP irq and stop event thread to exit gracefully at dp_display_unbind().
Changes in v2:
-- start event thread at dp_display_bind()
Changes in v3:
-- disable all HDP interrupts at unbind
-- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
-- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
-- move init_waitqueue_head(&dp->event_q) to probe()
-- move spin_lock_init(&dp->event_lock) to probe()
Changes in v4:
-- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
Changes in v5:
-- cancel relocation of both dp_display_bind() and dp_display_unbind()
Changes in v6:
-- move empty event q to dp_event_thread_start()
Changes in v7:
-- call ktheread_stop() directly instead of dp_hpd_event_thread_stop() function
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
Reviewed-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_display.c | 31 ---
1 file changed, 24 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..680e500 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -113,6 +113,7 @@ struct dp_display_private {
u32 hpd_state;
u32 event_pndx;
u32 event_gndx;
+ struct task_struct *ev_tsk;
struct dp_event event_list[DP_EVENT_Q_MAX];
spinlock_t event_lock;
@@ -230,6 +231,8 @@ void dp_display_signal_audio_complete(struct msm_dp
*dp_display)
complete_all(&dp->audio_comp);
}
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -269,6 +272,7 @@ static int dp_display_bind(struct device *dev, struct
device *master,
if (rc)
DRM_ERROR("Audio registration Dp failed\n");
I think I asked it in v3 and didn't get an answer:
Isn't 'goto end' missing here?
I did follow you suggestion at v3 and made changes at
dp_hpd_event_thread_start()
I'm asking about this place. Should we return an immediate error if
audio registration has failed or not?
ok,
it is not in this patch.
I will make changes to it.
+ rc = dp_hpd_event_thread_start(dp);
if (rc) { DRM_ERROR(); goto end; }
return 0;
Please.
This is at dp_display_bind(), "end" is the next statement why goto needed?
To be explicit that this is a fatal error. Compare this to my question
regarding the audio registration failure.
end:
return rc;
}
@@ -280,6 +284,11 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
struct drm_device *drm = dev_get_drvdata(master);
struct msm_drm_private *priv = drm->dev_private;
+ /* disable all HPD interrupts */
+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
+
+ kthread_stop(dp->ev_tsk);
+
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
@@ -1054,7 +1063,7 @@ static int hpd_event_thread(void *data)
dp_priv = (struct dp_display_private *)data;
- while (1) {
+ while (!kthread_should_stop()) {
if (timeout_mode) {
wait_event_timeout(dp_priv->event_q,
(dp_priv->event_pndx == dp_priv->event_gndx),
@@ -1132,12 +1141,19 @@ static int hpd_event_thread(void *data)
return 0;
}
-static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
{
- init_waitqueue_head(&dp_priv->event_q);
- spin_lock_init(&dp_priv->event_lock);
+ /* set event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 0;
+
+ dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
"dp_hpd_handler");
+ if (IS_ERR(dp_priv->ev_tsk)) {
+ DRM_ERROR("failed to create DP event thread\n");
+ return PTR_ERR(dp_priv->ev_tsk);
+ }
- kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
+ return 0;
}
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
@@ -1266,7 +1282,10 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ /* setup event q */
mutex_init(&dp->event_mutex);
+ init_waitqueue_head(&dp->event_q);
+ spin_lock_init(&dp->event_lock);
[PATCH v8] drm/msm/dp: stop event kernel thread when DP unbind
Current DP driver implementation, event thread is kept running
after DP display is unbind. This patch fix this problem by disabling
DP irq and stop event thread to exit gracefully at dp_display_unbind().
Changes in v2:
-- start event thread at dp_display_bind()
Changes in v3:
-- disable all HDP interrupts at unbind
-- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
-- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
-- move init_waitqueue_head(&dp->event_q) to probe()
-- move spin_lock_init(&dp->event_lock) to probe()
Changes in v4:
-- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
Changes in v5:
-- cancel relocation of both dp_display_bind() and dp_display_unbind()
Changes in v6:
-- move empty event q to dp_event_thread_start()
Changes in v7:
-- call ktheread_stop() directly instead of dp_hpd_event_thread_stop() function
Changes in v8:
-- return error immediately if audio registration failed.
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
Reviewed-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_display.c | 35 +++
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..590f90b 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -113,6 +113,7 @@ struct dp_display_private {
u32 hpd_state;
u32 event_pndx;
u32 event_gndx;
+ struct task_struct *ev_tsk;
struct dp_event event_list[DP_EVENT_Q_MAX];
spinlock_t event_lock;
@@ -230,6 +231,8 @@ void dp_display_signal_audio_complete(struct msm_dp
*dp_display)
complete_all(&dp->audio_comp);
}
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -266,9 +269,12 @@ static int dp_display_bind(struct device *dev, struct
device *master,
}
rc = dp_register_audio_driver(dev, dp->audio);
- if (rc)
+ if (rc) {
DRM_ERROR("Audio registration Dp failed\n");
+ goto end;
+ }
+ rc = dp_hpd_event_thread_start(dp);
end:
return rc;
}
@@ -280,6 +286,11 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
struct drm_device *drm = dev_get_drvdata(master);
struct msm_drm_private *priv = drm->dev_private;
+ /* disable all HPD interrupts */
+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
+
+ kthread_stop(dp->ev_tsk);
+
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
@@ -1054,7 +1065,7 @@ static int hpd_event_thread(void *data)
dp_priv = (struct dp_display_private *)data;
- while (1) {
+ while (!kthread_should_stop()) {
if (timeout_mode) {
wait_event_timeout(dp_priv->event_q,
(dp_priv->event_pndx == dp_priv->event_gndx),
@@ -1132,12 +1143,19 @@ static int hpd_event_thread(void *data)
return 0;
}
-static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
{
- init_waitqueue_head(&dp_priv->event_q);
- spin_lock_init(&dp_priv->event_lock);
+ /* set event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 0;
+
+ dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
"dp_hpd_handler");
+ if (IS_ERR(dp_priv->ev_tsk)) {
+ DRM_ERROR("failed to create DP event thread\n");
+ return PTR_ERR(dp_priv->ev_tsk);
+ }
- kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
+ return 0;
}
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
@@ -1266,7 +1284,10 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ /* setup event q */
mutex_init(&dp->event_mutex);
+ init_waitqueue_head(&dp->event_q);
+ spin_lock_init(&dp->event_lock);
/* Store DP audio handle inside DP display */
dp->dp_display.dp_audio = dp->audio;
@@ -1441,8 +1462,6 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
dp = container_of(dp_display, struct dp_display_private, dp_display);
- dp_hpd_event_setup(dp);
-
dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
Re: [PATCH v8] drm/msm/dp: stop event kernel thread when DP unbind
On Sat, 16 Apr 2022 at 02:36, Kuogee Hsieh wrote:
>
> Current DP driver implementation, event thread is kept running
> after DP display is unbind. This patch fix this problem by disabling
> DP irq and stop event thread to exit gracefully at dp_display_unbind().
>
> Changes in v2:
> -- start event thread at dp_display_bind()
>
> Changes in v3:
> -- disable all HDP interrupts at unbind
> -- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
> -- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
> -- move init_waitqueue_head(&dp->event_q) to probe()
> -- move spin_lock_init(&dp->event_lock) to probe()
>
> Changes in v4:
> -- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
>
> Changes in v5:
> -- cancel relocation of both dp_display_bind() and dp_display_unbind()
>
> Changes in v6:
> -- move empty event q to dp_event_thread_start()
>
> Changes in v7:
> -- call ktheread_stop() directly instead of dp_hpd_event_thread_stop()
> function
>
> Changes in v8:
> -- return error immediately if audio registration failed.
>
> Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
> Chipsets")
> Signed-off-by: Kuogee Hsieh
> Reported-by: Dmitry Baryshkov
> Reviewed-by: Stephen Boyd
> ---
> drivers/gpu/drm/msm/dp/dp_display.c | 35 +++
> 1 file changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
> b/drivers/gpu/drm/msm/dp/dp_display.c
> index 01453db..590f90b 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -113,6 +113,7 @@ struct dp_display_private {
> u32 hpd_state;
> u32 event_pndx;
> u32 event_gndx;
> + struct task_struct *ev_tsk;
> struct dp_event event_list[DP_EVENT_Q_MAX];
> spinlock_t event_lock;
>
> @@ -230,6 +231,8 @@ void dp_display_signal_audio_complete(struct msm_dp
> *dp_display)
> complete_all(&dp->audio_comp);
> }
>
> +static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
> +
> static int dp_display_bind(struct device *dev, struct device *master,
>void *data)
> {
> @@ -266,9 +269,12 @@ static int dp_display_bind(struct device *dev, struct
> device *master,
> }
>
> rc = dp_register_audio_driver(dev, dp->audio);
> - if (rc)
> + if (rc) {
> DRM_ERROR("Audio registration Dp failed\n");
> + goto end;
> + }
>
> + rc = dp_hpd_event_thread_start(dp);
At least a DRM_ERROR is missing here. And yes, I'd ask again for a
goto/return 0;
> end:
> return rc;
> }
> @@ -280,6 +286,11 @@ static void dp_display_unbind(struct device *dev, struct
> device *master,
> struct drm_device *drm = dev_get_drvdata(master);
> struct msm_drm_private *priv = drm->dev_private;
>
> + /* disable all HPD interrupts */
> + dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
> +
> + kthread_stop(dp->ev_tsk);
> +
> dp_power_client_deinit(dp->power);
> dp_aux_unregister(dp->aux);
> priv->dp[dp->id] = NULL;
> @@ -1054,7 +1065,7 @@ static int hpd_event_thread(void *data)
>
> dp_priv = (struct dp_display_private *)data;
>
> - while (1) {
> + while (!kthread_should_stop()) {
> if (timeout_mode) {
> wait_event_timeout(dp_priv->event_q,
> (dp_priv->event_pndx == dp_priv->event_gndx),
> @@ -1132,12 +1143,19 @@ static int hpd_event_thread(void *data)
> return 0;
> }
>
> -static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
> +static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
> {
> - init_waitqueue_head(&dp_priv->event_q);
> - spin_lock_init(&dp_priv->event_lock);
> + /* set event q to empty */
> + dp_priv->event_gndx = 0;
> + dp_priv->event_pndx = 0;
> +
> + dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
> "dp_hpd_handler");
> + if (IS_ERR(dp_priv->ev_tsk)) {
> + DRM_ERROR("failed to create DP event thread\n");
> + return PTR_ERR(dp_priv->ev_tsk);
> + }
>
> - kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
> + return 0;
> }
>
> static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
> @@ -1266,7 +1284,10 @@ static int dp_display_probe(struct platform_device
> *pdev)
> return -EPROBE_DEFER;
> }
>
> + /* setup event q */
> mutex_init(&dp->event_mutex);
> + init_waitqueue_head(&dp->event_q);
> + spin_lock_init(&dp->event_lock);
>
> /* Store DP audio handle inside DP display */
> dp->dp_display.dp_audio = dp->audio;
> @@ -1441,8 +1462,6 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
>
> dp = container_of(dp_display, struct dp_display_private, dp_display);
>
> - d
RE: [PATCH 9/9] vfio: Remove calls to vfio_group_add_container_user()
> From: Jason Gunthorpe > Sent: Friday, April 15, 2022 8:07 PM > > On Fri, Apr 15, 2022 at 02:32:08AM +, Tian, Kevin wrote: > > > While it's a welcomed fix is it actually related to this series? The point > > of this patch is that those functions are called when container_users > > is non-zero. This is true even without this fix given container_users > > is decremented after calling device->ops->close_device(). > > It isn't, it is decremented before which causes it to be 0 when the > assertions are called. > right, it's quite obvious when I read it the second time.
[PATCH v9] drm/msm/dp: stop event kernel thread when DP unbind
Current DP driver implementation, event thread is kept running
after DP display is unbind. This patch fix this problem by disabling
DP irq and stop event thread to exit gracefully at dp_display_unbind().
Changes in v2:
-- start event thread at dp_display_bind()
Changes in v3:
-- disable all HDP interrupts at unbind
-- replace dp_hpd_event_setup() with dp_hpd_event_thread_start()
-- replace dp_hpd_event_stop() with dp_hpd_event_thread_stop()
-- move init_waitqueue_head(&dp->event_q) to probe()
-- move spin_lock_init(&dp->event_lock) to probe()
Changes in v4:
-- relocate both dp_display_bind() and dp_display_unbind() to bottom of file
Changes in v5:
-- cancel relocation of both dp_display_bind() and dp_display_unbind()
Changes in v6:
-- move empty event q to dp_event_thread_start()
Changes in v7:
-- call ktheread_stop() directly instead of dp_hpd_event_thread_stop() function
Changes in v8:
-- return error immediately if audio registration failed.
Changes in v9:
-- return error immediately if event thread create failed.
Fixes: e91e3065a806 ("drm/msm/dp: Add DP compliance tests on Snapdragon
Chipsets")
Signed-off-by: Kuogee Hsieh
Reported-by: Dmitry Baryshkov
Reviewed-by: Stephen Boyd
---
drivers/gpu/drm/msm/dp/dp_display.c | 41 +
1 file changed, 33 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c
b/drivers/gpu/drm/msm/dp/dp_display.c
index 01453db..5b289b9 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -113,6 +113,7 @@ struct dp_display_private {
u32 hpd_state;
u32 event_pndx;
u32 event_gndx;
+ struct task_struct *ev_tsk;
struct dp_event event_list[DP_EVENT_Q_MAX];
spinlock_t event_lock;
@@ -230,6 +231,8 @@ void dp_display_signal_audio_complete(struct msm_dp
*dp_display)
complete_all(&dp->audio_comp);
}
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv);
+
static int dp_display_bind(struct device *dev, struct device *master,
void *data)
{
@@ -266,9 +269,18 @@ static int dp_display_bind(struct device *dev, struct
device *master,
}
rc = dp_register_audio_driver(dev, dp->audio);
- if (rc)
+ if (rc) {
DRM_ERROR("Audio registration Dp failed\n");
+ goto end;
+ }
+ rc = dp_hpd_event_thread_start(dp);
+ if (rc) {
+ DRM_ERROR("Event thread create failed\n");
+ goto end;
+ }
+
+ return 0;
end:
return rc;
}
@@ -280,6 +292,11 @@ static void dp_display_unbind(struct device *dev, struct
device *master,
struct drm_device *drm = dev_get_drvdata(master);
struct msm_drm_private *priv = drm->dev_private;
+ /* disable all HPD interrupts */
+ dp_catalog_hpd_config_intr(dp->catalog, DP_DP_HPD_INT_MASK, false);
+
+ kthread_stop(dp->ev_tsk);
+
dp_power_client_deinit(dp->power);
dp_aux_unregister(dp->aux);
priv->dp[dp->id] = NULL;
@@ -1054,7 +1071,7 @@ static int hpd_event_thread(void *data)
dp_priv = (struct dp_display_private *)data;
- while (1) {
+ while (!kthread_should_stop()) {
if (timeout_mode) {
wait_event_timeout(dp_priv->event_q,
(dp_priv->event_pndx == dp_priv->event_gndx),
@@ -1132,12 +1149,19 @@ static int hpd_event_thread(void *data)
return 0;
}
-static void dp_hpd_event_setup(struct dp_display_private *dp_priv)
+static int dp_hpd_event_thread_start(struct dp_display_private *dp_priv)
{
- init_waitqueue_head(&dp_priv->event_q);
- spin_lock_init(&dp_priv->event_lock);
+ /* set event q to empty */
+ dp_priv->event_gndx = 0;
+ dp_priv->event_pndx = 0;
+
+ dp_priv->ev_tsk = kthread_run(hpd_event_thread, dp_priv,
"dp_hpd_handler");
+ if (IS_ERR(dp_priv->ev_tsk)) {
+ DRM_ERROR("failed to create DP event thread\n");
+ return PTR_ERR(dp_priv->ev_tsk);
+ }
- kthread_run(hpd_event_thread, dp_priv, "dp_hpd_handler");
+ return 0;
}
static irqreturn_t dp_display_irq_handler(int irq, void *dev_id)
@@ -1266,7 +1290,10 @@ static int dp_display_probe(struct platform_device *pdev)
return -EPROBE_DEFER;
}
+ /* setup event q */
mutex_init(&dp->event_mutex);
+ init_waitqueue_head(&dp->event_q);
+ spin_lock_init(&dp->event_lock);
/* Store DP audio handle inside DP display */
dp->dp_display.dp_audio = dp->audio;
@@ -1441,8 +1468,6 @@ void msm_dp_irq_postinstall(struct msm_dp *dp_display)
dp = container_of(dp_display, struct dp_display_private, dp_display);
- dp_hpd_event_setup(dp);
-
dp_add_event(dp, EV_HPD_INIT_SETUP, 0, 100);
}
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foun
Re: [PATCH v2] drm/msm/dp: enhance both connect and disconnect pending_timeout handle
On 4/14/2022 4:34 PM, Dmitry Baryshkov wrote: On 15/04/2022 01:06, Kuogee Hsieh wrote: On 4/8/2022 4:59 PM, Dmitry Baryshkov wrote: On Fri, 8 Apr 2022 at 23:30, Kuogee Hsieh wrote: On 4/8/2022 5:27 AM, Dmitry Baryshkov wrote: On 07/04/2022 00:28, Kuogee Hsieh wrote: dp_hpd_plug_handle() is responsible for setting up main link and send uevent to notify user space framework to start video stream. Similarly, dp_hdp_unplug_handle is responsible to send uevent to notify user space framework to stop video stream and then tear down main link. However there are rare cases, such as in the middle of system suspending, that uevent could not be delivered to user space framework. Therefore some kind of recover mechanism armed by timer need to be in place in the case of user space framework does not respond to uevent. Hmm, how does userpsace 'respond' to the uevent? The driver should send hotplug notifications to userspace, but it must not expect any particular reaction. The userspace might be as simple, as fbdev emulation, but the driver still should function correctly. yes, driver is function correctly by setting up main link. but it does not know which resolution to display. It send hotplug notification through uevent to framework after main link is ready. Framework is responsible to set up MDP timing engine to start video stream. However it does not know which It's of no concern to the driver. It is completely the userspace problem. After resuming, it should reread available video output properties. The display could have been changed while the system is suspended. From your description I still do not understand why you need the 'recovery' mechanism. I mean "recovery" means dp driver may leave stay at link ready mistakenly after dongle unplugged due to missing framework action to tear down main link so that next dongle plug in will not work. Currently it was armed with timeout function and it will fired if framework did not call msm_dp_display_disable() after 5 second. framework = userspace? Is my understanding correct? Currently DP driver sends a notification to userspace that DP is unplugged, waits for userspace to disable DP output, and only after that it will shutdown the link. Is this a correct description? Yes, you are correct, connection need to be tear down from top to bottom. If so, then yes, your change is correct. I mean that the kernel shouldn't wait for clients to stop using video pipeline if the sink gets unplugged. Instead it should tear the video stream down and let the DRM client cope with that. I'm not sure how should the driver react if the client doesn't disable the output, but then the sink gets reattached? I do not know that either. But it should not happen as long as framework response to uevent. And a bit more interesting question: how should the driver behave if the new sink is not compatible with the existing video stream. When dongle plug/replug in, driver will always read sink's edid. Therefore resolution database should be updated accordingly. Anyway, we think this approach is not good. therefore it is replaced with below patch. drm/msm/dp: tear down main link at unplug handle immediately
Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly
Hi,
On Fri, Apr 15, 2022 at 3:45 PM Dmitry Baryshkov
wrote:
>
> On Sat, 16 Apr 2022 at 00:13, Doug Anderson wrote:
> >
> > Hi,
> >
> > On Thu, Apr 14, 2022 at 5:47 PM Dmitry Baryshkov
> > wrote:
> > >
> > > On 09/04/2022 05:36, Douglas Anderson wrote:
> > > > As talked about in the kerneldoc for "struct dp_aux_ep_client" in this
> > > > patch and also in the past in commit a1e3667a9835 ("drm/bridge:
> > > > ti-sn65dsi86: Promote the AUX channel to its own sub-dev"), to use the
> > > > DP AUX bus properly we really need two "struct device"s. One "struct
> > > > device" is in charge of providing the DP AUX bus and the other is
> > > > where we'll try to get a reference to the newly probed endpoint
> > > > devices.
> > > >
> > > > In ti-sn65dsi86 this wasn't too difficult to accomplish. That driver
> > > > is already broken up into several "struct devices" anyway because it
> > > > also provides a PWM and some GPIOs. Adding one more wasn't that
> > > > difficult / ugly.
> > > >
> > > > When I tried to do the same solution in parade-ps8640, it felt like I
> > > > was copying too much boilerplate code. I made the realization that I
> > > > didn't _really_ need a separate "driver" for each person that wanted
> > > > to do the same thing. By putting all the "driver" related code in a
> > > > common place then we could save a bit of hassle. This change
> > > > effectively adds a new "ep_client" driver that can be used by
> > > > anyone. The devices instantiated by this driver will just call through
> > > > to the probe/remove/shutdown calls provided.
> > > >
> > > > At the moment, the "ep_client" driver is backed by the Linux auxiliary
> > > > bus (unfortunate naming--this has nothing to do with DP AUX). I didn't
> > > > want to expose this to clients, though, so as far as clients are
> > > > concerned they get a vanilla "struct device".
> > >
> > > I have been thinking about your approach for quite some time. I think
> > > that enforcing a use of auxilliary device is an overkill. What do we
> > > really need is the the set callbacks in the bus struct or a notifier. We
> > > have to notify the aux_bus controller side that the client has been
> > > probed successfully or that the client is going to be removed.
> >
> > It seems like these new callbacks would be nearly the same as the
> > probe/remove callbacks in my proposal except:
> >
> > * They rely on there being exactly 1 AUX device, or we make it a rule
> > that we wait for all AUX devices to probe (?)
>
> Is the backlight a separate device on an AUX bus? Judging from
> drm_panel_dp_aux_backlight(), it isn't. I assumed that aux bus is just
> a point-to-point bus, so there is always a single client.
Define "device". ;-)
It's a seperate "struct device" from a Linux point of view since it's
a backlight class device. Certainly it's highly correlated to the
display, but one can conceptually think of them as different devices,
sorta. ;-)
I actually dug a tiny bit more into the whole "touchscreen over aux".
I guess DP 1.2 has a standard of "USB over DP AUX". No idea how that
would be modeled, of course.
I guess the summary is that I'm OK w/ changing it to assume one device
for now, but I'm still not sure it's compelling to move to normal
callbacks. The API for callbacks is pretty much the same as the one I
proposed and IMO leaving it the way it is (with an extra struct
device) doesn't really add much complexity and has a few (small) nice
benefits.
> > * We need to come up with a system for detecting when everything
> > probes or is going to be removed, though that's probably not too hard.
> > I guess the DP AUX bus could just replace the panel's probe function
> > with its own and essentially "tail patch" it. I guess it could "head
> > patch" the remove call? ...or is there some better way you were
> > thinking of knowing when all our children probed?
> >
> > * The callback on the aux bus controller side would not be able to
> > DEFER. In other words trying to acquire a reference to the panel can
> > always be the last thing we do so we know there can be no reasons to
> > defer after. This should be doable, but at least in the ps8640 case it
> > will require changing the code a bit. I notice that today it actually
> > tries to get the panel side _before_ it gets the MIPI side and it
> > potentially can return -EPROBE_DEFER if it can't find the MIPI side. I
> > guess I have a niggling feeling that we'll find some reason in the
> > future that we can't be last, but we can probably ignore that. ;-)
> >
> > I can switch this all to normal callbacks if that's what everyone
> > wants, but it doesn't feel significantly cleaner to me and does seem
> > to have some (small) downsides.
> >
> >
> > > And this
> > > approach would make driver's life easier, since e.g. the bus code can
> > > pm_get the EP device before calling callbacks/notifiers and
> > > pm_put_autosuspend it afterwards.
> >
> > Not sure about doing the pm calls on behalf of the EP device. What's
> > t
Re: [RFC PATCH 4/6] drm/panel-edp: Take advantage of is_hpd_asserted() in struct drm_dp_aux
Hi,
On Fri, Apr 15, 2022 at 3:12 PM Dmitry Baryshkov
wrote:
>
> On Sat, 16 Apr 2022 at 00:17, Doug Anderson wrote:
> >
> > Hi,
> >
> > On Thu, Apr 14, 2022 at 5:51 PM Dmitry Baryshkov
> > wrote:
> > >
> > > On 09/04/2022 05:36, Douglas Anderson wrote:
> > > > Let's add support for being able to read the HPD pin even if it's
> > > > hooked directly to the controller. This will allow us to get more
> > > > accurate delays also lets us take away the waiting in the AUX transfer
> > > > functions of the eDP controller drivers.
> > > >
> > > > Signed-off-by: Douglas Anderson
> > > > ---
> > > >
> > > > drivers/gpu/drm/panel/panel-edp.c | 37 ++-
> > > > 1 file changed, 31 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/panel/panel-edp.c
> > > > b/drivers/gpu/drm/panel/panel-edp.c
> > > > index 1732b4f56e38..4a143eb9544b 100644
> > > > --- a/drivers/gpu/drm/panel/panel-edp.c
> > > > +++ b/drivers/gpu/drm/panel/panel-edp.c
> > > > @@ -417,6 +417,19 @@ static int panel_edp_get_hpd_gpio(struct device
> > > > *dev, struct panel_edp *p)
> > > > return 0;
> > > > }
> > > >
> > > > +static bool panel_edp_can_read_hpd(struct panel_edp *p)
> > > > +{
> > > > + return !p->no_hpd && (p->hpd_gpio || (p->aux &&
> > > > p->aux->is_hpd_asserted));
> > > > +}
> > > > +
> > > > +static bool panel_edp_read_hpd(struct panel_edp *p)
> > > > +{
> > > > + if (p->hpd_gpio)
> > > > + return gpiod_get_value_cansleep(p->hpd_gpio);
> > > > +
> > > > + return p->aux->is_hpd_asserted(p->aux);
> > > > +}
> > > > +
> > > > static int panel_edp_prepare_once(struct panel_edp *p)
> > > > {
> > > > struct device *dev = p->base.dev;
> > > > @@ -441,13 +454,21 @@ static int panel_edp_prepare_once(struct
> > > > panel_edp *p)
> > > > if (delay)
> > > > msleep(delay);
> > > >
> > > > - if (p->hpd_gpio) {
> > > > + if (panel_edp_can_read_hpd(p)) {
> > > > if (p->desc->delay.hpd_absent)
> > > > hpd_wait_us = p->desc->delay.hpd_absent * 1000UL;
> > > > else
> > > > hpd_wait_us = 200;
> > > >
> > > > - err = readx_poll_timeout(gpiod_get_value_cansleep,
> > > > p->hpd_gpio,
> > > > + /*
> > > > + * Extra max delay, mostly to account for ps8640. ps8640
> > > > + * is crazy and the bridge chip driver itself has over
> > > > 200 ms
> > > > + * of delay if it needs to do the pm_runtime resume of the
> > > > + * bridge chip to read the HPD.
> > > > + */
> > > > + hpd_wait_us += 300;
> > >
> > > I think this should come in a separate commit and ideally this should be
> > > configurable somehow. Other hosts wouldn't need such 'additional' delay.
> > >
> > > With this change removed:
> > >
> > > Reviewed-by: Dmitry Baryshkov
> >
> > What would you think about changing the API slightly? Instead of
> > is_hpd_asserted(), we change it to wait_hpd_asserted() and it takes a
> > timeout in microseconds. If you pass 0 for the timeout the function is
> > defined to behave the same as is_hpd_asserted() today--AKA a single
> > poll of the line.
>
> This might work. Can you check it, please?
Cool. I'll spin with this. Hopefully early next week unless my inbox
blows up. ...or my main PC's SSD like happened this week. ;-)
> BTW: are these changes dependent on the first part of the patchset? It
> might be worth splitting the patchset into two parts.
Definitely not. As per the cover letter, this is two series jammed
into one. I'm happy to split them up. The 2nd half seems much less
controversial.
-Doug
Re: [RFC PATCH 4/6] drm/panel-edp: Take advantage of is_hpd_asserted() in struct drm_dp_aux
On 16/04/2022 03:12, Doug Anderson wrote:
Hi,
On Fri, Apr 15, 2022 at 3:12 PM Dmitry Baryshkov
wrote:
On Sat, 16 Apr 2022 at 00:17, Doug Anderson wrote:
Hi,
On Thu, Apr 14, 2022 at 5:51 PM Dmitry Baryshkov
wrote:
On 09/04/2022 05:36, Douglas Anderson wrote:
Let's add support for being able to read the HPD pin even if it's
hooked directly to the controller. This will allow us to get more
accurate delays also lets us take away the waiting in the AUX transfer
functions of the eDP controller drivers.
Signed-off-by: Douglas Anderson
---
drivers/gpu/drm/panel/panel-edp.c | 37 ++-
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panel/panel-edp.c
b/drivers/gpu/drm/panel/panel-edp.c
index 1732b4f56e38..4a143eb9544b 100644
--- a/drivers/gpu/drm/panel/panel-edp.c
+++ b/drivers/gpu/drm/panel/panel-edp.c
@@ -417,6 +417,19 @@ static int panel_edp_get_hpd_gpio(struct device *dev,
struct panel_edp *p)
return 0;
}
+static bool panel_edp_can_read_hpd(struct panel_edp *p)
+{
+ return !p->no_hpd && (p->hpd_gpio || (p->aux && p->aux->is_hpd_asserted));
+}
+
+static bool panel_edp_read_hpd(struct panel_edp *p)
+{
+ if (p->hpd_gpio)
+ return gpiod_get_value_cansleep(p->hpd_gpio);
+
+ return p->aux->is_hpd_asserted(p->aux);
+}
+
static int panel_edp_prepare_once(struct panel_edp *p)
{
struct device *dev = p->base.dev;
@@ -441,13 +454,21 @@ static int panel_edp_prepare_once(struct panel_edp *p)
if (delay)
msleep(delay);
- if (p->hpd_gpio) {
+ if (panel_edp_can_read_hpd(p)) {
if (p->desc->delay.hpd_absent)
hpd_wait_us = p->desc->delay.hpd_absent * 1000UL;
else
hpd_wait_us = 200;
- err = readx_poll_timeout(gpiod_get_value_cansleep, p->hpd_gpio,
+ /*
+ * Extra max delay, mostly to account for ps8640. ps8640
+ * is crazy and the bridge chip driver itself has over 200 ms
+ * of delay if it needs to do the pm_runtime resume of the
+ * bridge chip to read the HPD.
+ */
+ hpd_wait_us += 300;
I think this should come in a separate commit and ideally this should be
configurable somehow. Other hosts wouldn't need such 'additional' delay.
With this change removed:
Reviewed-by: Dmitry Baryshkov
What would you think about changing the API slightly? Instead of
is_hpd_asserted(), we change it to wait_hpd_asserted() and it takes a
timeout in microseconds. If you pass 0 for the timeout the function is
defined to behave the same as is_hpd_asserted() today--AKA a single
poll of the line.
This might work. Can you check it, please?
Cool. I'll spin with this. Hopefully early next week unless my inbox
blows up. ...or my main PC's SSD like happened this week. ;-)
BTW: are these changes dependent on the first part of the patchset? It
might be worth splitting the patchset into two parts.
Definitely not. As per the cover letter, this is two series jammed
into one. I'm happy to split them up. The 2nd half seems much less
controversial.
Great, let's get it in then. As you have time.
--
With best wishes
Dmitry
[PATCH 1/3] drm/msm: remove explicit devfreq status reset
It is redundant since commit 7c0ffcd40b16 ("drm/msm/gpu: Respect PM QoS
constraints") because dev_pm_qos_update_request triggers get_dev_status.
Signed-off-by: Chia-I Wu
Cc: Rob Clark
---
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index 12641616acd3..317a95d42922 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -224,7 +224,6 @@ void msm_devfreq_boost(struct msm_gpu *gpu, unsigned factor)
void msm_devfreq_active(struct msm_gpu *gpu)
{
struct msm_gpu_devfreq *df = &gpu->devfreq;
- struct devfreq_dev_status status;
unsigned int idle_time;
if (!has_devfreq(gpu))
@@ -248,12 +247,6 @@ void msm_devfreq_active(struct msm_gpu *gpu)
dev_pm_qos_update_request(&df->idle_freq,
PM_QOS_MAX_FREQUENCY_DEFAULT_VALUE);
-
- /*
-* Reset the polling interval so we aren't inconsistent
-* about freq vs busy/total cycles
-*/
- msm_devfreq_get_dev_status(&gpu->pdev->dev, &status);
}
--
2.36.0.rc0.470.gd361397f0d-goog
[PATCH 2/3] drm/msm: simplify gpu_busy callback
Move tracking and busy time calculation to msm_devfreq_get_dev_status.
Signed-off-by: Chia-I Wu
Cc: Rob Clark
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 19 ++--
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 15 +
drivers/gpu/drm/msm/msm_gpu.h | 9 +++-
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 32 ++-
4 files changed, 41 insertions(+), 34 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 407f50a15faa..217615e0e850 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -1662,28 +1662,23 @@ static struct msm_ringbuffer *a5xx_active_ring(struct
msm_gpu *gpu)
return a5xx_gpu->cur_ring;
}
-static unsigned long a5xx_gpu_busy(struct msm_gpu *gpu)
+static u64 a5xx_gpu_busy(struct msm_gpu *gpu, unsigned long *out_sample_rate)
{
- u64 busy_cycles, busy_time;
+ u64 busy_cycles;
/* Only read the gpu busy if the hardware is already active */
- if (pm_runtime_get_if_in_use(&gpu->pdev->dev) == 0)
+ if (pm_runtime_get_if_in_use(&gpu->pdev->dev) == 0) {
+ *out_sample_rate = 1;
return 0;
+ }
busy_cycles = gpu_read64(gpu, REG_A5XX_RBBM_PERFCTR_RBBM_0_LO,
REG_A5XX_RBBM_PERFCTR_RBBM_0_HI);
-
- busy_time = busy_cycles - gpu->devfreq.busy_cycles;
- do_div(busy_time, clk_get_rate(gpu->core_clk) / 100);
-
- gpu->devfreq.busy_cycles = busy_cycles;
+ *out_sample_rate = clk_get_rate(gpu->core_clk);
pm_runtime_put(&gpu->pdev->dev);
- if (WARN_ON(busy_time > ~0LU))
- return ~0LU;
-
- return (unsigned long)busy_time;
+ return busy_cycles;
}
static uint32_t a5xx_get_rptr(struct msm_gpu *gpu, struct msm_ringbuffer *ring)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index ccc4fcf7a630..fba6259395dd 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1649,12 +1649,14 @@ static void a6xx_destroy(struct msm_gpu *gpu)
kfree(a6xx_gpu);
}
-static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu)
+static u64 a6xx_gpu_busy(struct msm_gpu *gpu, unsigned long *out_sample_rate)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
- u64 busy_cycles, busy_time;
+ u64 busy_cycles;
+ /* 19.2MHz */
+ *out_sample_rate = 1920;
/* Only read the gpu busy if the hardware is already active */
if (pm_runtime_get_if_in_use(a6xx_gpu->gmu.dev) == 0)
@@ -1664,17 +1666,10 @@ static unsigned long a6xx_gpu_busy(struct msm_gpu *gpu)
REG_A6XX_GMU_CX_GMU_POWER_COUNTER_XOCLK_0_L,
REG_A6XX_GMU_CX_GMU_POWER_COUNTER_XOCLK_0_H);
- busy_time = (busy_cycles - gpu->devfreq.busy_cycles) * 10;
- do_div(busy_time, 192);
-
- gpu->devfreq.busy_cycles = busy_cycles;
pm_runtime_put(a6xx_gpu->gmu.dev);
- if (WARN_ON(busy_time > ~0LU))
- return ~0LU;
-
- return (unsigned long)busy_time;
+ return busy_cycles;
}
static void a6xx_gpu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 02419f2ca2bc..389c6dab751b 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -62,7 +62,7 @@ struct msm_gpu_funcs {
/* for generation specific debugfs: */
void (*debugfs_init)(struct msm_gpu *gpu, struct drm_minor *minor);
#endif
- unsigned long (*gpu_busy)(struct msm_gpu *gpu);
+ u64 (*gpu_busy)(struct msm_gpu *gpu, unsigned long *out_sample_rate);
struct msm_gpu_state *(*gpu_state_get)(struct msm_gpu *gpu);
int (*gpu_state_put)(struct msm_gpu_state *state);
unsigned long (*gpu_get_freq)(struct msm_gpu *gpu);
@@ -106,11 +106,8 @@ struct msm_gpu_devfreq {
struct dev_pm_qos_request boost_freq;
/**
-* busy_cycles:
-*
-* Used by implementation of gpu->gpu_busy() to track the last
-* busy counter value, for calculating elapsed busy cycles since
-* last sampling period.
+* busy_cycles: Last busy counter value, for calculating elapsed busy
+* cycles since last sampling period.
*/
u64 busy_cycles;
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index 317a95d42922..f531015107c3 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -49,18 +49,38 @@ static unsigned long get_freq(struct msm_gpu *gpu)
return clk_get_rate(gpu->core_clk);
}
-static int msm_devfreq_get_dev_status(struct device *dev,
+static void get_raw_dev_status(struct msm_gpu *gpu,
struct devfreq_dev_statu
[PATCH 3/3] drm/msm: return the average load over the polling period
simple_ondemand interacts poorly with clamp_to_idle. It only looks at
the load since the last get_dev_status call, while it should really look
at the load over polling_ms. When clamp_to_idle true, it almost always
picks the lowest frequency on active because the gpu is idle between
msm_devfreq_idle/msm_devfreq_active.
This logic could potentially be moved into devfreq core.
Fixes: 7c0ffcd40b16 ("drm/msm/gpu: Respect PM QoS constraints")
Signed-off-by: Chia-I Wu
Cc: Rob Clark
---
drivers/gpu/drm/msm/msm_gpu.h | 3 ++
drivers/gpu/drm/msm/msm_gpu_devfreq.c | 60 ++-
2 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 389c6dab751b..143c56f5185b 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -9,6 +9,7 @@
#include
#include
+#include
#include
#include
#include
@@ -117,6 +118,8 @@ struct msm_gpu_devfreq {
/** idle_time: Time of last transition to idle: */
ktime_t idle_time;
+ struct devfreq_dev_status average_status;
+
/**
* idle_work:
*
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index f531015107c3..d2539ca78c29 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -9,6 +9,7 @@
#include
#include
+#include
#include
/*
@@ -75,12 +76,69 @@ static void get_raw_dev_status(struct msm_gpu *gpu,
status->busy_time = busy_time;
}
+static void update_average_dev_status(struct msm_gpu *gpu,
+ const struct devfreq_dev_status *raw)
+{
+ struct msm_gpu_devfreq *df = &gpu->devfreq;
+ const u32 polling_ms = df->devfreq->profile->polling_ms;
+ const u32 max_history_ms = polling_ms * 11 / 10;
+ struct devfreq_dev_status *avg = &df->average_status;
+ u64 avg_freq;
+
+ /* simple_ondemand governor interacts poorly with gpu->clamp_to_idle.
+* When we enforce the constraint on idle, it calls get_dev_status
+* which would normally reset the stats. When we remove the
+* constraint on active, it calls get_dev_status again where busy_time
+* would be 0.
+*
+* To remedy this, we always return the average load over the past
+* polling_ms.
+*/
+
+ /* raw is longer than polling_ms or avg has no history */
+ if (div_u64(raw->total_time, USEC_PER_MSEC) >= polling_ms ||
+ !avg->total_time) {
+ *avg = *raw;
+ return;
+ }
+
+ /* Truncate the oldest history first.
+*
+* Because we keep the history with a single devfreq_dev_status,
+* rather than a list of devfreq_dev_status, we have to assume freq
+* and load are the same over avg->total_time. We can scale down
+* avg->busy_time and avg->total_time by the same factor to drop
+* history.
+*/
+ if (div_u64(avg->total_time + raw->total_time, USEC_PER_MSEC) >=
+ max_history_ms) {
+ const u32 new_total_time = polling_ms * USEC_PER_MSEC -
+ raw->total_time;
+ avg->busy_time = div_u64(
+ mul_u32_u32(avg->busy_time, new_total_time),
+ avg->total_time);
+ avg->total_time = new_total_time;
+ }
+
+ /* compute the average freq over avg->total_time + raw->total_time */
+ avg_freq = mul_u32_u32(avg->current_frequency, avg->total_time);
+ avg_freq += mul_u32_u32(raw->current_frequency, raw->total_time);
+ do_div(avg_freq, avg->total_time + raw->total_time);
+
+ avg->current_frequency = avg_freq;
+ avg->busy_time += raw->busy_time;
+ avg->total_time += raw->total_time;
+}
+
static int msm_devfreq_get_dev_status(struct device *dev,
struct devfreq_dev_status *status)
{
struct msm_gpu *gpu = dev_to_gpu(dev);
+ struct devfreq_dev_status raw;
- get_raw_dev_status(gpu, status);
+ get_raw_dev_status(gpu, &raw);
+ update_average_dev_status(gpu, &raw);
+ *status = gpu->devfreq.average_status;
return 0;
}
--
2.36.0.rc0.470.gd361397f0d-goog
Re: [PATCH 3/3] drm/msm: return the average load over the polling period
On Fri, Apr 15, 2022 at 5:33 PM Chia-I Wu wrote:
>
> simple_ondemand interacts poorly with clamp_to_idle. It only looks at
> the load since the last get_dev_status call, while it should really look
> at the load over polling_ms. When clamp_to_idle true, it almost always
> picks the lowest frequency on active because the gpu is idle between
> msm_devfreq_idle/msm_devfreq_active.
>
> This logic could potentially be moved into devfreq core.
The idea is to extend devfreq_simple_ondemand_data to specify whether
devfreq_simple_ondemand_func should use "last status" or "average
status" to determine the target frequency. devfreq core will need to
store "struct devfreq_dev_status average_status", which will be
updated when a device uses simple_ondemand and asks for average_status
instead of last_status.
>
> Fixes: 7c0ffcd40b16 ("drm/msm/gpu: Respect PM QoS constraints")
> Signed-off-by: Chia-I Wu
> Cc: Rob Clark
> ---
> drivers/gpu/drm/msm/msm_gpu.h | 3 ++
> drivers/gpu/drm/msm/msm_gpu_devfreq.c | 60 ++-
> 2 files changed, 62 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
> index 389c6dab751b..143c56f5185b 100644
> --- a/drivers/gpu/drm/msm/msm_gpu.h
> +++ b/drivers/gpu/drm/msm/msm_gpu.h
> @@ -9,6 +9,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -117,6 +118,8 @@ struct msm_gpu_devfreq {
> /** idle_time: Time of last transition to idle: */
> ktime_t idle_time;
>
> + struct devfreq_dev_status average_status;
> +
> /**
> * idle_work:
> *
> diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> index f531015107c3..d2539ca78c29 100644
> --- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> +++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
> @@ -9,6 +9,7 @@
>
> #include
> #include
> +#include
> #include
>
> /*
> @@ -75,12 +76,69 @@ static void get_raw_dev_status(struct msm_gpu *gpu,
> status->busy_time = busy_time;
> }
>
> +static void update_average_dev_status(struct msm_gpu *gpu,
> + const struct devfreq_dev_status *raw)
> +{
> + struct msm_gpu_devfreq *df = &gpu->devfreq;
> + const u32 polling_ms = df->devfreq->profile->polling_ms;
> + const u32 max_history_ms = polling_ms * 11 / 10;
> + struct devfreq_dev_status *avg = &df->average_status;
> + u64 avg_freq;
> +
> + /* simple_ondemand governor interacts poorly with gpu->clamp_to_idle.
> +* When we enforce the constraint on idle, it calls get_dev_status
> +* which would normally reset the stats. When we remove the
> +* constraint on active, it calls get_dev_status again where busy_time
> +* would be 0.
> +*
> +* To remedy this, we always return the average load over the past
> +* polling_ms.
> +*/
> +
> + /* raw is longer than polling_ms or avg has no history */
> + if (div_u64(raw->total_time, USEC_PER_MSEC) >= polling_ms ||
> + !avg->total_time) {
> + *avg = *raw;
> + return;
> + }
> +
> + /* Truncate the oldest history first.
> +*
> +* Because we keep the history with a single devfreq_dev_status,
> +* rather than a list of devfreq_dev_status, we have to assume freq
> +* and load are the same over avg->total_time. We can scale down
> +* avg->busy_time and avg->total_time by the same factor to drop
> +* history.
> +*/
> + if (div_u64(avg->total_time + raw->total_time, USEC_PER_MSEC) >=
> + max_history_ms) {
> + const u32 new_total_time = polling_ms * USEC_PER_MSEC -
> + raw->total_time;
> + avg->busy_time = div_u64(
> + mul_u32_u32(avg->busy_time, new_total_time),
> + avg->total_time);
> + avg->total_time = new_total_time;
> + }
> +
> + /* compute the average freq over avg->total_time + raw->total_time */
> + avg_freq = mul_u32_u32(avg->current_frequency, avg->total_time);
> + avg_freq += mul_u32_u32(raw->current_frequency, raw->total_time);
> + do_div(avg_freq, avg->total_time + raw->total_time);
> +
> + avg->current_frequency = avg_freq;
> + avg->busy_time += raw->busy_time;
> + avg->total_time += raw->total_time;
> +}
> +
> static int msm_devfreq_get_dev_status(struct device *dev,
> struct devfreq_dev_status *status)
> {
> struct msm_gpu *gpu = dev_to_gpu(dev);
> + struct devfreq_dev_status raw;
>
> - get_raw_dev_status(gpu, status);
> + get_raw_dev_status(gpu, &raw);
> + update_average_dev_status(gpu, &raw);
> + *status = gpu->devfreq.average_status;
>
> return 0;
> }
> --
> 2.36.0.rc0.470.gd361397f0d-goog
>
Re: [RFC PATCH 1/6] drm/dp: Helpers to make it easier for drivers to use DP AUX bus properly
On 16/04/2022 03:09, Doug Anderson wrote:
Hi,
On Fri, Apr 15, 2022 at 3:45 PM Dmitry Baryshkov
wrote:
On Sat, 16 Apr 2022 at 00:13, Doug Anderson wrote:
Hi,
On Thu, Apr 14, 2022 at 5:47 PM Dmitry Baryshkov
wrote:
On 09/04/2022 05:36, Douglas Anderson wrote:
As talked about in the kerneldoc for "struct dp_aux_ep_client" in this
patch and also in the past in commit a1e3667a9835 ("drm/bridge:
ti-sn65dsi86: Promote the AUX channel to its own sub-dev"), to use the
DP AUX bus properly we really need two "struct device"s. One "struct
device" is in charge of providing the DP AUX bus and the other is
where we'll try to get a reference to the newly probed endpoint
devices.
In ti-sn65dsi86 this wasn't too difficult to accomplish. That driver
is already broken up into several "struct devices" anyway because it
also provides a PWM and some GPIOs. Adding one more wasn't that
difficult / ugly.
When I tried to do the same solution in parade-ps8640, it felt like I
was copying too much boilerplate code. I made the realization that I
didn't _really_ need a separate "driver" for each person that wanted
to do the same thing. By putting all the "driver" related code in a
common place then we could save a bit of hassle. This change
effectively adds a new "ep_client" driver that can be used by
anyone. The devices instantiated by this driver will just call through
to the probe/remove/shutdown calls provided.
At the moment, the "ep_client" driver is backed by the Linux auxiliary
bus (unfortunate naming--this has nothing to do with DP AUX). I didn't
want to expose this to clients, though, so as far as clients are
concerned they get a vanilla "struct device".
I have been thinking about your approach for quite some time. I think
that enforcing a use of auxilliary device is an overkill. What do we
really need is the the set callbacks in the bus struct or a notifier. We
have to notify the aux_bus controller side that the client has been
probed successfully or that the client is going to be removed.
It seems like these new callbacks would be nearly the same as the
probe/remove callbacks in my proposal except:
* They rely on there being exactly 1 AUX device, or we make it a rule
that we wait for all AUX devices to probe (?)
Is the backlight a separate device on an AUX bus? Judging from
drm_panel_dp_aux_backlight(), it isn't. I assumed that aux bus is just
a point-to-point bus, so there is always a single client.
Define "device". ;-)
"a device on the AUX bus" = the device, which lists dp_aux_bus_type as
dev->bus_type.
It's a seperate "struct device" from a Linux point of view since it's
a backlight class device. Certainly it's highly correlated to the
display, but one can conceptually think of them as different devices,
sorta. ;-)
I actually dug a tiny bit more into the whole "touchscreen over aux".
I guess DP 1.2 has a standard of "USB over DP AUX". No idea how that
would be modeled, of course.
Ugh. Do you have any details of the standard itself? Like how does it
looks like from the host point of view. And if the AUX is required to be
powered for this USB bus to work?
In other words: if we were to model it at this moment, would it be the
child of the panel device (like backlight) or a separate device sitting
on the same AUX bus?
I guess the summary is that I'm OK w/ changing it to assume one device
for now, but I'm still not sure it's compelling to move to normal
callbacks. The API for callbacks is pretty much the same as the one I
proposed and IMO leaving it the way it is (with an extra struct
device) doesn't really add much complexity and has a few (small) nice
benefits.
I think Stephen didn't like too many similarities between
dp_aux_ep_client and dp_aux_ep_device. And I'd second him here.
* We need to come up with a system for detecting when everything
probes or is going to be removed, though that's probably not too hard.
I guess the DP AUX bus could just replace the panel's probe function
with its own and essentially "tail patch" it. I guess it could "head
patch" the remove call? ...or is there some better way you were
thinking of knowing when all our children probed?
* The callback on the aux bus controller side would not be able to
DEFER. In other words trying to acquire a reference to the panel can
always be the last thing we do so we know there can be no reasons to
defer after. This should be doable, but at least in the ps8640 case it
will require changing the code a bit. I notice that today it actually
tries to get the panel side _before_ it gets the MIPI side and it
potentially can return -EPROBE_DEFER if it can't find the MIPI side. I
guess I have a niggling feeling that we'll find some reason in the
future that we can't be last, but we can probably ignore that. ;-)
I can switch this all to normal callbacks if that's what everyone
wants, but it doesn't feel significantly cleaner to me and does seem
to have some (small) downsides.
And this
approach would make driver's li
[PATCH 1/2] dt-bindings: display: simple: Add DataImage FG040346DSSWBG04 compatible string
Add DataImage FG040346DSSWBG04 4.3" 480x272 TFT LCD 24bit DPI panel compatible string. Signed-off-by: Marek Vasut Cc: Rob Herring Cc: Sam Ravnborg Cc: Thomas Zimmermann Cc: devicet...@vger.kernel.org To: dri-devel@lists.freedesktop.org --- .../devicetree/bindings/display/panel/panel-simple.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml index 1eb9dd4f8f58e..cfe7bb9f89de6 100644 --- a/Documentation/devicetree/bindings/display/panel/panel-simple.yaml +++ b/Documentation/devicetree/bindings/display/panel/panel-simple.yaml @@ -105,6 +105,8 @@ properties: - chunghwa,claa101wb01 # Chunghwa Picture Tubes Ltd. 10.1" WXGA TFT LCD panel - chunghwa,claa101wb03 +# DataImage, Inc. 4.3" WQVGA (480x272) TFT LCD panel with 24-bit parallel interface. + - dataimage,fg040346dsswbg04 # DataImage, Inc. 7" WVGA (800x480) TFT LCD panel with 24-bit parallel interface. - dataimage,scf0700c48ggu18 # DLC Display Co. DLC1010GIG 10.1" WXGA TFT LCD Panel -- 2.35.1
[PATCH 2/2] drm/panel: simple: Add DataImage FG040346DSSWBG04 panel support
Add DataImage FG040346DSSWBG04 4.3" 480x272 TFT LCD 24bit DPI panel
support.
Signed-off-by: Marek Vasut
Cc: Sam Ravnborg
Cc: Thomas Zimmermann
To: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/panel/panel-simple.c | 28
1 file changed, 28 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index a34f4198a5341..3c35f8a32d327 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -1402,6 +1402,31 @@ static const struct panel_desc chunghwa_claa101wb01 = {
.connector_type = DRM_MODE_CONNECTOR_LVDS,
};
+static const struct display_timing dataimage_fg040346dsswbg04_timing = {
+ .pixelclock = { 500, 900, 1200 },
+ .hactive = { 480, 480, 480 },
+ .hfront_porch = { 12, 12, 12 },
+ .hback_porch = { 12, 12, 12 },
+ .hsync_len = { 21, 21, 21 },
+ .vactive = { 272, 272, 272 },
+ .vfront_porch = { 4, 4, 4 },
+ .vback_porch = { 4, 4, 4 },
+ .vsync_len = { 8, 8, 8 },
+};
+
+static const struct panel_desc dataimage_fg040346dsswbg04 = {
+ .timings = &dataimage_fg040346dsswbg04_timing,
+ .num_timings = 1,
+ .bpc = 8,
+ .size = {
+ .width = 95,
+ .height = 54,
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+ .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE,
+ .connector_type = DRM_MODE_CONNECTOR_DPI,
+};
+
static const struct drm_display_mode dataimage_scf0700c48ggu18_mode = {
.clock = 33260,
.hdisplay = 800,
@@ -3768,6 +3793,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "chunghwa,claa101wb01",
.data = &chunghwa_claa101wb01
+ }, {
+ .compatible = "dataimage,fg040346dsswbg04",
+ .data = &dataimage_fg040346dsswbg04,
}, {
.compatible = "dataimage,scf0700c48ggu18",
.data = &dataimage_scf0700c48ggu18,
--
2.35.1
