RE: [EXT] Re: [PATCH v6 5/8] drm: bridge: Cadence: Add MHDP8501 HDMI driver
Hi Sandor, Am Montag, 19. Juni 2023, 05:11:02 CEST schrieb Sandor Yu: > Hi Alexander, > > Thanks for your comments, > > > > -Original Message- > > From: Alexander Stein > > Sent: 2023年6月16日 17:30 > > To: andrzej.ha...@intel.com; neil.armstr...@linaro.org; > > robert.f...@linaro.org; laurent.pinch...@ideasonboard.com; > > jo...@kwiboo.se; jernej.skra...@gmail.com; airl...@gmail.com; > > dan...@ffwll.ch; robh...@kernel.org; krzysztof.kozlowski...@linaro.org; > > shawn...@kernel.org; s.ha...@pengutronix.de; feste...@gmail.com; > > vk...@kernel.org; dri-devel@lists.freedesktop.org; > > devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > > linux-ker...@vger.kernel.org; linux-...@lists.infradead.org > > Cc: Oliver Brown ; Sandor Yu ; > > dl-linux-imx ; ker...@pengutronix.de; Sandor Yu > > > > Subject: [EXT] Re: [PATCH v6 5/8] drm: bridge: Cadence: Add MHDP8501 > > HDMI driver > > > > > > > > Caution: This is an external email. Please take care when clicking links > > or opening attachments. When in doubt, report the message using the > > 'Report this email' button > > > > > > > > > > Hi Sandor, > > > > > > > > thanks for sending a new version. > > > > > > > > Am Donnerstag, 15. Juni 2023, 03:38:15 CEST schrieb Sandor Yu: > > > > > Add a new DRM HDMI bridge driver for Cadence MHDP8501 that used in > > > Freescale i.MX8MQ SoC. > > > MHDP8501 could support HDMI or DisplayPort standards according > > > embedded Firmware running in the uCPU. > > > > > > > > > > > > For iMX8MQ SoC, the HDMI FW was loaded and activated by SOC ROM > > > > code. > > > > > Bootload binary included HDMI FW was required for the driver. > > > > > > > > > > > > Signed-off-by: Sandor Yu > > > --- > > > > > > drivers/gpu/drm/bridge/cadence/Kconfig| 12 + > > > drivers/gpu/drm/bridge/cadence/Makefile |1 + > > > .../drm/bridge/cadence/cdns-mhdp8501-hdmi.c | 1024 > > > > + > > > > > 3 files changed, 1037 insertions(+) > > > create mode 100644 > > > > > > drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c > > > > > > > > > > > > diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig > > > b/drivers/gpu/drm/bridge/cadence/Kconfig index > > > 5b7ec4e49aa1..bee05e834055 > > > 100644 > > > --- a/drivers/gpu/drm/bridge/cadence/Kconfig > > > +++ b/drivers/gpu/drm/bridge/cadence/Kconfig > > > @@ -59,3 +59,15 @@ config DRM_CDNS_MHDP8501_DP > > > > > > Support Cadence MHDP8501 DisplayPort driver. > > > Cadence MHDP8501 Controller support one or more protocols, > > > DisplayPort firmware is required for this driver. > > > > > > + > > > +config DRM_CDNS_MHDP8501_HDMI > > > + tristate "Cadence MHDP8501 HDMI DRM driver" > > > + select DRM_KMS_HELPER > > > + select DRM_PANEL_BRIDGE > > > + select DRM_DISPLAY_HELPER > > > + select DRM_CDNS_AUDIO > > > + depends on OF > > > + help > > > + Support Cadence MHDP8501 HDMI driver. > > > + Cadence MHDP8501 Controller support one or more protocols, > > > + HDMI firmware is required for this driver. > > > diff --git a/drivers/gpu/drm/bridge/cadence/Makefile > > > b/drivers/gpu/drm/bridge/cadence/Makefile index > > > 5842e4540c62..8a129c14ac14 > > > 100644 > > > --- a/drivers/gpu/drm/bridge/cadence/Makefile > > > +++ b/drivers/gpu/drm/bridge/cadence/Makefile > > > @@ -7,3 +7,4 @@ cdns-mhdp8546-y := cdns-mhdp8546-core.o > > > cdns-mhdp8546-hdcp.o > > > cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += > > > cdns-mhdp8546-j721e.o > > > > > > > > > > > > obj-$(CONFIG_DRM_CDNS_MHDP8501_DP) += cdns-mhdp8501-dp.o > > > > > > +obj-$(CONFIG_DRM_CDNS_MHDP8501_HDMI) += cdns-mhdp8501-hdmi.o > > > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c > > > b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c new file mode > > > 100644 index ..43673f1b50f6 > > > --- /dev/null > > > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c > > > > [...] > > > > > +static int cdns_hdmi_bridge_attach(struct drm_bridge *bridge, > > > + enum drm_bridge_attach_flags flags) > > > > { > > > > > + struct cdns_mhdp_device *mhdp = bridge->driver_private; > > > + struct drm_mode_config *config = >dev->mode_config; > > > + struct drm_encoder *encoder = bridge->encoder; > > > + struct drm_connector *connector = >connector; > > > + > > > + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) { > > > + connector->interlace_allowed = 0; > > > + connector->polled = DRM_CONNECTOR_POLL_HPD; > > > + > > > + drm_connector_helper_add(connector, > > > > _hdmi_connector_helper_funcs); > > > > > + > > > + drm_connector_init(bridge->dev, connector, > > > > _hdmi_connector_funcs, > > > > > + > > > > DRM_MODE_CONNECTOR_HDMIA); > > > > > + > > > + drm_object_attach_property(>base, > > > +config- > > > > > >hdr_output_metadata_property, 0);
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi, On 2023/6/19 11:02, Maciej W. Rozycki wrote: On Tue, 13 Jun 2023, Sui Jingfeng wrote: Deal only with the VGA devcie(pdev->class == 0x0300), so replace the Typo here: s/devcie/device/. Thanks a lot, pci_get_subsys() function with pci_get_class(). Filter the non-PCI display device(pdev->class != 0x0300) out. There no need to process the non-display PCI device. I've only come across this patch series now. Without diving into what this code actually does I have just one question as a matter of interest. diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c index c1bc6c983932..22a505e877dc 100644 --- a/drivers/pci/vgaarb.c +++ b/drivers/pci/vgaarb.c @@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned long action, struct pci_dev *pdev = to_pci_dev(dev); bool notify = false; - vgaarb_dbg(dev, "%s\n", __func__); + /* Only deal with VGA class devices */ + if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8) + return 0; Hmm, shouldn't this also handle PCI_CLASS_NOT_DEFINED_VGA? If your machine have only one such a VGA card, it probably don't hurt. But, such a card will also get ignored originally (before applying this patch). As far as I know it is the equivalent of PCI_CLASS_DISPLAY_VGA for PCI <= 2.0 devices that were implemented before the idea of PCI device classes has developed into its current form. If multiple video card problems on your machine is matter, then I think it do deserve another patch to clarify this issue and to explain the rationale. I may have such a VGA device somewhere. Maciej
Re: [14/14] drm/ast: Merge config and chip detection
Hi, On 2023/6/16 21:52, Thomas Zimmermann wrote: Detection of the configuration mode and the chipset model are linked to each other. They don't has a strong relationship, chipset model detection should the first function to be run(should be run early). chipset model detection should only rely on the information come with PCI device itself. Then ast_detect_config_mode() follows, ast_detect_config_mode() should depend on ast_detect_chip() One uses values from the other; namely the PCI device revision and the SCU revision. Merge this code into a single function. In last patch, you split one into three, then in this patch, you merge the two into one. Then you finally got one more patch function only(+2 - 1 = 1). This is fine, but I noticed that the ast_detect_widescreen() function is also depend on the chip identify. Then, why the ast_detect_widescreen() function is not get merged to ast_device_config_init() ? Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_main.c | 108 + 1 file changed, 57 insertions(+), 51 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index f028b5b47c56e..5fcddd0dac5e8 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -97,68 +97,75 @@ static void ast_open_key(struct ast_device *ast) ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0x80, 0xA8); } -static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev) +static int ast_device_config_init(struct ast_device *ast) { - struct device_node *np = dev->dev->of_node; - struct ast_device *ast = to_ast_device(dev); + struct drm_device *dev = >base; struct pci_dev *pdev = to_pci_dev(dev->dev); - uint32_t data, jregd0, jregd1; + struct device_node *np = dev->dev->of_node; The OF itself deserve a separated function, as its only available when DT is available. The no need twist so much local variables together. + uint32_t scu_rev = 0x; + u32 data; + u8 jregd0, jregd1; + + /* +* Find configuration mode and read SCU revision +*/ - /* Defaults */ ast->config_mode = ast_use_defaults; /* Check if we have device-tree properties */ - if (np && !of_property_read_u32(np, "aspeed,scu-revision-id", - scu_rev)) { + if (np && !of_property_read_u32(np, "aspeed,scu-revision-id", )) { /* We do, disable P2A access */ ast->config_mode = ast_use_dt; - drm_info(dev, "Using device-tree for configuration\n"); - return; - } + scu_rev = data; + } else if (pdev->device == PCI_CHIP_AST2000) { // Not all families have a P2A bridge + /* +* The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge +* is disabled. We force using P2A if VGA only mode bit +* is set D[7] +*/ + jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff); + jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff); + if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) { + + /* +* We have a P2A bridge and it is enabled. +*/ + + /* Patch AST2500/AST2510 */ + if ((pdev->revision & 0xf0) == 0x40) { + if (!(jregd0 & AST_VRAM_INIT_STATUS_MASK)) + ast_patch_ahb_2500(ast); + } - /* Not all families have a P2A bridge */ - if (pdev->device != PCI_CHIP_AST2000) - return; + /* Double check that it's actually working */ + data = ast_read32(ast, 0xf004); + if ((data != 0x) && (data != 0x00)) { + ast->config_mode = ast_use_p2a; - /* -* The BMC will set SCU 0x40 D[12] to 1 if the P2 bridge -* is disabled. We force using P2A if VGA only mode bit -* is set D[7] -*/ - jregd0 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd0, 0xff); - jregd1 = ast_get_index_reg_mask(ast, AST_IO_CRTC_PORT, 0xd1, 0xff); - if (!(jregd0 & 0x80) || !(jregd1 & 0x10)) { - /* Patch GEN6 */ - if (((pdev->revision & 0xF0) == 0x40) - && ((jregd0 & AST_VRAM_INIT_STATUS_MASK) == 0)) - ast_patch_ahb_2500(ast); - - /* Double check it's actually working */ - data = ast_read32(ast, 0xf004); - if ((data != 0x) && (data != 0x00)) { - /* P2A works, grab silicon revision */ - ast->config_mode = ast_use_p2a; - - drm_info(dev, "Using P2A bridge for
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
On Tue, 13 Jun 2023, Sui Jingfeng wrote: > Deal only with the VGA devcie(pdev->class == 0x0300), so replace the Typo here: s/devcie/device/. > pci_get_subsys() function with pci_get_class(). Filter the non-PCI display > device(pdev->class != 0x0300) out. There no need to process the non-display > PCI device. I've only come across this patch series now. Without diving into what this code actually does I have just one question as a matter of interest. > diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c > index c1bc6c983932..22a505e877dc 100644 > --- a/drivers/pci/vgaarb.c > +++ b/drivers/pci/vgaarb.c > @@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, > unsigned long action, > struct pci_dev *pdev = to_pci_dev(dev); > bool notify = false; > > - vgaarb_dbg(dev, "%s\n", __func__); > + /* Only deal with VGA class devices */ > + if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8) > + return 0; Hmm, shouldn't this also handle PCI_CLASS_NOT_DEFINED_VGA? As far as I know it is the equivalent of PCI_CLASS_DISPLAY_VGA for PCI <= 2.0 devices that were implemented before the idea of PCI device classes has developed into its current form. I may have such a VGA device somewhere. Maciej
RE: [EXT] Re: [PATCH v6 5/8] drm: bridge: Cadence: Add MHDP8501 HDMI driver
Hi Alexander, Thanks for your comments, > -Original Message- > From: Alexander Stein > Sent: 2023年6月16日 17:30 > To: andrzej.ha...@intel.com; neil.armstr...@linaro.org; > robert.f...@linaro.org; laurent.pinch...@ideasonboard.com; > jo...@kwiboo.se; jernej.skra...@gmail.com; airl...@gmail.com; > dan...@ffwll.ch; robh...@kernel.org; krzysztof.kozlowski...@linaro.org; > shawn...@kernel.org; s.ha...@pengutronix.de; feste...@gmail.com; > vk...@kernel.org; dri-devel@lists.freedesktop.org; > devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > linux-ker...@vger.kernel.org; linux-...@lists.infradead.org > Cc: Oliver Brown ; Sandor Yu ; > dl-linux-imx ; ker...@pengutronix.de; Sandor Yu > > Subject: [EXT] Re: [PATCH v6 5/8] drm: bridge: Cadence: Add MHDP8501 > HDMI driver > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > Hi Sandor, > > thanks for sending a new version. > > Am Donnerstag, 15. Juni 2023, 03:38:15 CEST schrieb Sandor Yu: > > Add a new DRM HDMI bridge driver for Cadence MHDP8501 that used in > > Freescale i.MX8MQ SoC. > > MHDP8501 could support HDMI or DisplayPort standards according > > embedded Firmware running in the uCPU. > > > > For iMX8MQ SoC, the HDMI FW was loaded and activated by SOC ROM > code. > > Bootload binary included HDMI FW was required for the driver. > > > > Signed-off-by: Sandor Yu > > --- > > drivers/gpu/drm/bridge/cadence/Kconfig| 12 + > > drivers/gpu/drm/bridge/cadence/Makefile |1 + > > .../drm/bridge/cadence/cdns-mhdp8501-hdmi.c | 1024 > + > > 3 files changed, 1037 insertions(+) > > create mode 100644 > > drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c > > > > diff --git a/drivers/gpu/drm/bridge/cadence/Kconfig > > b/drivers/gpu/drm/bridge/cadence/Kconfig index > > 5b7ec4e49aa1..bee05e834055 > > 100644 > > --- a/drivers/gpu/drm/bridge/cadence/Kconfig > > +++ b/drivers/gpu/drm/bridge/cadence/Kconfig > > @@ -59,3 +59,15 @@ config DRM_CDNS_MHDP8501_DP > > Support Cadence MHDP8501 DisplayPort driver. > > Cadence MHDP8501 Controller support one or more protocols, > > DisplayPort firmware is required for this driver. > > + > > +config DRM_CDNS_MHDP8501_HDMI > > + tristate "Cadence MHDP8501 HDMI DRM driver" > > + select DRM_KMS_HELPER > > + select DRM_PANEL_BRIDGE > > + select DRM_DISPLAY_HELPER > > + select DRM_CDNS_AUDIO > > + depends on OF > > + help > > + Support Cadence MHDP8501 HDMI driver. > > + Cadence MHDP8501 Controller support one or more protocols, > > + HDMI firmware is required for this driver. > > diff --git a/drivers/gpu/drm/bridge/cadence/Makefile > > b/drivers/gpu/drm/bridge/cadence/Makefile index > > 5842e4540c62..8a129c14ac14 > > 100644 > > --- a/drivers/gpu/drm/bridge/cadence/Makefile > > +++ b/drivers/gpu/drm/bridge/cadence/Makefile > > @@ -7,3 +7,4 @@ cdns-mhdp8546-y := cdns-mhdp8546-core.o > > cdns-mhdp8546-hdcp.o > > cdns-mhdp8546-$(CONFIG_DRM_CDNS_MHDP8546_J721E) += > > cdns-mhdp8546-j721e.o > > > > obj-$(CONFIG_DRM_CDNS_MHDP8501_DP) += cdns-mhdp8501-dp.o > > +obj-$(CONFIG_DRM_CDNS_MHDP8501_HDMI) += cdns-mhdp8501-hdmi.o > > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c > > b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c new file mode > > 100644 index ..43673f1b50f6 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8501-hdmi.c > [...] > > +static int cdns_hdmi_bridge_attach(struct drm_bridge *bridge, > > + enum drm_bridge_attach_flags flags) > { > > + struct cdns_mhdp_device *mhdp = bridge->driver_private; > > + struct drm_mode_config *config = >dev->mode_config; > > + struct drm_encoder *encoder = bridge->encoder; > > + struct drm_connector *connector = >connector; > > + > > + if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) { > > + connector->interlace_allowed = 0; > > + connector->polled = DRM_CONNECTOR_POLL_HPD; > > + > > + drm_connector_helper_add(connector, > _hdmi_connector_helper_funcs); > > + > > + drm_connector_init(bridge->dev, connector, > _hdmi_connector_funcs, > > + > DRM_MODE_CONNECTOR_HDMIA); > > + > > + drm_object_attach_property(>base, > > +config- > >hdr_output_metadata_property, 0); > > + > > + if > > + (!drm_mode_create_hdmi_colorspace_property(connector)) > > This is missing a 2nd parameter. I have not found function drm_mode_create_hdmi_colorspace_property need 2nd parameter in L6.1. And those connector init functions will be remove in the next version according Sam's comments, because they are not really needed. B.R Sandor > > > + > drm_object_attach_property(>base, > > + connector- > >colorspace_property, 0);
RE: [EXT] Re: [PATCH v6 2/8] dt-bindings: display: bridge: Add Cadence MHDP8501 HDMI and DP
Hi Alexander, Thanks for your comments, > -Original Message- > From: Alexander Stein > Sent: 2023年6月16日 17:32 > To: andrzej.ha...@intel.com; neil.armstr...@linaro.org; > robert.f...@linaro.org; laurent.pinch...@ideasonboard.com; > jo...@kwiboo.se; jernej.skra...@gmail.com; airl...@gmail.com; > dan...@ffwll.ch; robh...@kernel.org; krzysztof.kozlowski...@linaro.org; > shawn...@kernel.org; s.ha...@pengutronix.de; feste...@gmail.com; > vk...@kernel.org; dri-devel@lists.freedesktop.org; > devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > linux-ker...@vger.kernel.org; linux-...@lists.infradead.org > Cc: Oliver Brown ; Sandor Yu ; > dl-linux-imx ; ker...@pengutronix.de; Sandor Yu > > Subject: [EXT] Re: [PATCH v6 2/8] dt-bindings: display: bridge: Add Cadence > MHDP8501 HDMI and DP > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > Hi Sandor, > > Am Donnerstag, 15. Juni 2023, 03:38:12 CEST schrieb Sandor Yu: > > Add bindings for Cadence MHDP8501 DisplayPort and HDMI driver. > > > > Signed-off-by: Sandor Yu > > --- > > .../display/bridge/cdns,mhdp8501.yaml | 105 > ++ > > 1 file changed, 105 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml > > b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.yaml > > new file mode 100644 index ..a54756815e6f > > --- /dev/null > > +++ > b/Documentation/devicetree/bindings/display/bridge/cdns,mhdp8501.y > > +++ aml > > @@ -0,0 +1,105 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +http://devi/ > > > +cetree.org%2Fschemas%2Fdisplay%2Fbridge%2Fcdns%2Cmhdp8501.yaml% > 23 > > > +a=05%7C01%7CSandor.yu%40nxp.com%7C6ef2c732b3674cb2896c08db6e4 > c827b%7C > > > +686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63822504710561225 > 8%7CUnkno > > > +wn%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1 > haWwi > > > +LCJXVCI6Mn0%3D%7C3000%7C%7C%7C=DGYYt2LQ%2FhlNBVd2m0s > aXTm9IoKKwn > > +X7CTTplhbLxcI%3D=0 > > +$schema: > > +http://devi/ > > > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23=05%7C01%7CSandor. > yu%40n > > > +xp.com%7C6ef2c732b3674cb2896c08db6e4c827b%7C686ea1d3bc2b4c6fa9 > 2cd99c5 > > > +c301635%7C0%7C0%7C638225047105612258%7CUnknown%7CTWFpbGZs > b3d8eyJWIjoi > > > +MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3 > 000%7C% > > > +7C%7C=SkTFYM7HHgJmFUkyo3Ftf%2B8FdGqlnty0Ch6ggwSPeLY%3D > > > +=0 > > + > > +title: Cadence MHDP8501 Displayport bridge > > + > > +maintainers: > > + - Sandor Yu > > + > > +description: > > + The Cadence MHDP8501 Displayport/HDMI TX interface. > > + > > +properties: > > + compatible: > > +enum: > > + - cdns,mhdp8501-dp > > + - cdns,mhdp8501-hdmi > > + - fsl,imx8mq-mhdp8501-dp > > + - fsl,imx8mq-mhdp8501-hdmi > > + > > + reg: > > +maxItems: 1 > > + > > + clocks: > > +maxItems: 1 > > +description: MHDP8501 DP/HDMI APB clock. > > + > > + phys: > > +maxItems: 1 > > + > > + interrupts: > > +items: > > + - description: Hotplug cable plugin. > > + - description: Hotplug cable plugout. > > + > > + interrupt-names: > > +items: > > + - const: plug_in > > + - const: plug_out > > + > > + ports: > > +$ref: /schemas/graph.yaml#/properties/ports > > + > > +properties: > > + port@0: > > +$ref: /schemas/graph.yaml#/properties/port > > +description: > > + Input port from display controller output. > > + port@1: > > +$ref: /schemas/graph.yaml#/properties/port > > +description: > > + Output port to DP/HDMI connector. > > + > > +required: > > + - port@0 > > + - port@1 > > You mark these ports as required, but apparently the drivers do not use them, > AFAICT. E.g. missing port@1 is not resulting in an error, at lease for HDMI > one. > Yes, port@1 is not really needed, I add it just to follow HDMI/DP framework that same as other platforms in community code. B.R Sandor > Best regards, > Alexander > > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - interrupts > > + - interrupt-names > > + - phys > > + - ports > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > +#include > > +#include > > + > > +mhdp_dp: dp-bridge@32c0 { > > +compatible = "fsl,imx8mq-mhdp8501-dp"; > > +reg = <0x32c0 0x10>; > > +interrupts = , > > + ; > > +interrupt-names = "plug_in", "plug_out"; > > +clocks = < IMX8MQ_CLK_DISP_APB_ROOT>; > > +phys = <_phy>; > > + > > +ports { > > +#address-cells = <1>; > > +#size-cells = <0>; > > + > > +
RE: [EXT] Re: [PATCH v6 6/8] dt-bindings: phy: Add Freescale iMX8MQ DP and HDMI PHY
Hi Alexander, Thanks for your comments, > -Original Message- > From: Alexander Stein > Sent: 2023年6月16日 17:35 > To: andrzej.ha...@intel.com; neil.armstr...@linaro.org; > robert.f...@linaro.org; laurent.pinch...@ideasonboard.com; > jo...@kwiboo.se; jernej.skra...@gmail.com; airl...@gmail.com; > dan...@ffwll.ch; robh...@kernel.org; krzysztof.kozlowski...@linaro.org; > shawn...@kernel.org; s.ha...@pengutronix.de; feste...@gmail.com; > vk...@kernel.org; dri-devel@lists.freedesktop.org; > devicet...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; > linux-ker...@vger.kernel.org; linux-...@lists.infradead.org > Cc: Oliver Brown ; Sandor Yu ; > dl-linux-imx ; ker...@pengutronix.de; Sandor Yu > > Subject: [EXT] Re: [PATCH v6 6/8] dt-bindings: phy: Add Freescale iMX8MQ DP > and HDMI PHY > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > Hi Sandor, > > Am Donnerstag, 15. Juni 2023, 03:38:16 CEST schrieb Sandor Yu: > > Add bindings for Freescale iMX8MQ DP and HDMI PHY. > > > > Signed-off-by: Sandor Yu > > --- > > .../bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml | 53 > > +++ > > 1 file changed, 53 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml > > > > diff --git > > a/Documentation/devicetree/bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml > > b/Documentation/devicetree/bindings/phy/fsl,imx8mq-dp-hdmi-phy.yaml > > new file mode 100644 index ..917f113503dc > > --- /dev/null > > +++ > b/Documentation/devicetree/bindings/phy/fsl,imx8mq-dp-hdmi-phy.yam > > +++ l > > @@ -0,0 +1,53 @@ > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2 > > +--- > > +$id: > > +http://devi/ > > > +cetree.org%2Fschemas%2Fphy%2Ffsl%2Cimx8mq-dp-hdmi-phy.yaml%23 > ata=05 > > > +%7C01%7CSandor.yu%40nxp.com%7Cf2d4e5ea99fa4f5776bf08db6e4ce7ba > %7C686e > > > +a1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638225048817792176%7C > Unknown%7 > > > +CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW > wiLCJX > > > +VCI6Mn0%3D%7C3000%7C%7C%7C=ayYA0rayDR2w1LDgU53VtCitw > MH9PnoblX2k > > +Jhbu6Gs%3D=0 > > +$schema: > > +http://devi/ > > > +cetree.org%2Fmeta-schemas%2Fcore.yaml%23=05%7C01%7CSandor. > yu%40n > > > +xp.com%7Cf2d4e5ea99fa4f5776bf08db6e4ce7ba%7C686ea1d3bc2b4c6fa92 > cd99c5 > > > +c301635%7C0%7C0%7C638225048817792176%7CUnknown%7CTWFpbGZs > b3d8eyJWIjoi > > > +MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3 > 000%7C% > > > +7C%7C=DagQtrIblGEk3T1mamSmI2010SRszqhIpJ4piXy3L4M%3D > served=0 > > + > > +title: Cadence HDP-TX DP/HDMI PHY for Freescale i.MX8MQ SoC > > + > > +maintainers: > > + - Sandor Yu > > + > > +properties: > > + compatible: > > +enum: > > + - fsl,imx8mq-dp-phy > > + - fsl,imx8mq-hdmi-phy > > How is it intended to select DP or HDMI? E.g. provide a single default dp-phy > node in imx8mq.dtsi and change the compatible to HDMI on board-level? > The PHY driver select should align with HDMI/DP driver base on board type. For HDMI board: fsl,imx8mq-hdmi-phy + fsl,imx8mq-mhdp8501-hdmi For DP board: fsl,imx8mq-dp-phy + fsl,imx8mq-mhdp8501-dp B.R Sandor > Best regards, > Alexander > > > + > > + reg: > > +maxItems: 1 > > + > > + clocks: > > +items: > > + - description: PHY reference clock. > > + - description: APB clock. > > + > > + clock-names: > > +items: > > + - const: ref > > + - const: apb > > + > > + "#phy-cells": > > +const: 0 > > + > > +required: > > + - compatible > > + - reg > > + - clocks > > + - clock-names > > + - "#phy-cells" > > + > > +additionalProperties: false > > + > > +examples: > > + - | > > +#include > > +#include > > +dp_phy: phy@32c0 { > > +compatible = "fsl,imx8mq-dp-phy"; > > +reg = <0x32c0 0x10>; > > +#phy-cells = <0>; > > +clocks = <_phy_27m>, < > IMX8MQ_CLK_DISP_APB_ROOT>; > > +clock-names = "ref", "apb"; > > +}; > > > -- > TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany > Amtsgericht München, HRB 105018 > Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider > http://www.tq/ > -group.com%2F=05%7C01%7CSandor.yu%40nxp.com%7Cf2d4e5ea99fa > 4f5776bf08db6e4ce7ba%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C > 0%7C638225048817792176%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4w > LjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C > %7C%7C=6uih84XByS6aX519z7l50amZl2XnPIMSggyfSD4xd4M%3D > served=0 >
Re: [13/14] drm/ast: Move widescreen- and tx-chip detection into separate helpers
Hi, Split ast_detect_chip() into three functions made it more clear. As tx-chip is typicality suffer from changed, and it is a choice of the PCB board designer. tx-chip belong to the output, which chip identify and wide screen support is a capability of the chip itself, once the chip is taped, it is fixed. So this patch looks fine. On 2023/6/16 21:52, Thomas Zimmermann wrote: Split ast_detect_chip() into three functions and call them one by one. The new functions detect the transmitter chip and widescreen support. This will allow for further refactoring. Signed-off-by: Thomas Zimmermann Reviewed-by: Sui Jingfeng Tested-by: Sui Jingfeng --- drivers/gpu/drm/ast/ast_main.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 7f8fb9a613604..f028b5b47c56e 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -157,7 +157,6 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev) { struct ast_device *ast = to_ast_device(dev); struct pci_dev *pdev = to_pci_dev(dev->dev); - uint32_t jreg; /* Identify chipset */ if (pdev->revision >= 0x50) { @@ -218,6 +217,13 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev) drm_info(dev, "AST 2000 detected\n"); } + return 0; +} + +static void ast_detect_widescreen(struct ast_device *ast) +{ + u8 jreg; + /* Check if we support wide screen */ switch (AST_GEN(ast)) { case 1: @@ -242,6 +248,12 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev) } break; } +} + +static void ast_detect_tx_chip(struct ast_device *ast, bool need_post) +{ + struct drm_device *dev = >base; + u8 jreg; /* Check 3rd Tx option (digital output afaik) */ ast->tx_chip_types |= AST_TX_NONE_BIT; @@ -301,8 +313,6 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev) drm_info(dev, "Using DP501 DisplayPort transmitter\n"); if (ast->tx_chip_types & AST_TX_ASTDP_BIT) drm_info(dev, "Using ASPEED DisplayPort transmitter\n"); - - return 0; } static int ast_get_dram_info(struct drm_device *dev) @@ -494,6 +504,8 @@ struct ast_device *ast_device_create(const struct drm_driver *drv, ast_detect_config_mode(dev, _rev); ast_detect_chip(dev, need_post, scu_rev); + ast_detect_widescreen(ast); + ast_detect_tx_chip(ast, need_post); ret = ast_get_dram_info(dev); if (ret) -- Jingfeng
Re: [PATCH V3 7/7] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.7
On 6/16/23 01:57, Evan Quan wrote: Fulfill the SMU13.0.7 support for Wifi RFI mitigation feature. Signed-off-by: Evan Quan Reviewed-by: Mario Limonciello --- .../drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 59 +++ 1 file changed, 59 insertions(+) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c index 98a33f8ee209..16c1c04e2034 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c @@ -125,6 +125,7 @@ static struct cmn2asic_msg_mapping smu_v13_0_7_message_map[SMU_MSG_MAX_COUNT] = MSG_MAP(ArmD3, PPSMC_MSG_ArmD3, 0), MSG_MAP(AllowGpo, PPSMC_MSG_SetGpoAllow, 0), MSG_MAP(GetPptLimit,PPSMC_MSG_GetPptLimit, 0), + MSG_MAP(EnableUCLKShadow, PPSMC_MSG_EnableUCLKShadow, 0), }; static struct cmn2asic_mapping smu_v13_0_7_clk_map[SMU_CLK_COUNT] = { @@ -205,6 +206,7 @@ static struct cmn2asic_mapping smu_v13_0_7_table_map[SMU_TABLE_COUNT] = { TAB_MAP(DRIVER_SMU_CONFIG), TAB_MAP(ACTIVITY_MONITOR_COEFF), [SMU_TABLE_COMBO_PPTABLE] = {1, TABLE_COMBO_PPTABLE}, + TAB_MAP(WIFIBAND), }; static struct cmn2asic_mapping smu_v13_0_7_pwr_src_map[SMU_POWER_SOURCE_COUNT] = { @@ -487,6 +489,9 @@ static int smu_v13_0_7_tables_init(struct smu_context *smu) AMDGPU_GEM_DOMAIN_VRAM); SMU_TABLE_INIT(tables, SMU_TABLE_COMBO_PPTABLE, MP0_MP1_DATA_REGION_SIZE_COMBOPPTABLE, PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM); + SMU_TABLE_INIT(tables, SMU_TABLE_WIFIBAND, + sizeof(WifiBandEntryTable_t), PAGE_SIZE, + AMDGPU_GEM_DOMAIN_VRAM); smu_table->metrics_table = kzalloc(sizeof(SmuMetricsExternal_t), GFP_KERNEL); if (!smu_table->metrics_table) @@ -1721,6 +1726,57 @@ static int smu_v13_0_7_set_df_cstate(struct smu_context *smu, NULL); } +static bool smu_v13_0_7_wbrf_support_check(struct smu_context *smu) +{ + return smu->smc_fw_version > 0x00524600; +} + +static int smu_v13_0_7_set_wbrf_exclusion_ranges(struct smu_context *smu, +struct exclusion_range *exclusion_ranges) +{ + WifiBandEntryTable_t wifi_bands; + int valid_entries = 0; + int ret, i; + + memset(_bands, 0, sizeof(wifi_bands)); + for (i = 0; i < ARRAY_SIZE(wifi_bands.WifiBandEntry); i++) { + if (!exclusion_ranges[i].start && + !exclusion_ranges[i].end) + break; + + /* PMFW expects the inputs to be in Mhz unit */ + wifi_bands.WifiBandEntry[valid_entries].LowFreq = + DIV_ROUND_DOWN_ULL(exclusion_ranges[i].start, HZ_IN_MHZ); + wifi_bands.WifiBandEntry[valid_entries++].HighFreq = + DIV_ROUND_UP_ULL(exclusion_ranges[i].end, HZ_IN_MHZ); + } + wifi_bands.WifiBandEntryNum = valid_entries; + + /* +* Per confirm with PMFW team, WifiBandEntryNum = 0 is a valid setting. +* Considering the scenarios below: +* - At first the wifi device adds an exclusion range e.g. (2400,2500) to +* BIOS and our driver gets notified. We will set WifiBandEntryNum = 1 +* and pass the WifiBandEntry (2400, 2500) to PMFW. +* +* - Later the wifi device removes the wifiband list added above and +* our driver gets notified again. At this time, driver will set +* WifiBandEntryNum = 0 and pass an empty WifiBandEntry list to PMFW. +* - PMFW may still need to do some uclk shadow update(e.g. switching +* from shadow clock back to primary clock) on receiving this. +*/ + + ret = smu_cmn_update_table(smu, + SMU_TABLE_WIFIBAND, + 0, + (void *)(_bands), + true); + if (ret) + dev_err(smu->adev->dev, "Failed to set wifiband!"); + + return ret; +} + static const struct pptable_funcs smu_v13_0_7_ppt_funcs = { .get_allowed_feature_mask = smu_v13_0_7_get_allowed_feature_mask, .set_default_dpm_table = smu_v13_0_7_set_default_dpm_table, @@ -1786,6 +1842,9 @@ static const struct pptable_funcs smu_v13_0_7_ppt_funcs = { .set_mp1_state = smu_v13_0_7_set_mp1_state, .set_df_cstate = smu_v13_0_7_set_df_cstate, .gpo_control = smu_v13_0_gpo_control, + .is_asic_wbrf_supported = smu_v13_0_7_wbrf_support_check, + .enable_uclk_shadow = smu_v13_0_enable_uclk_shadow, + .set_wbrf_exclusion_ranges = smu_v13_0_7_set_wbrf_exclusion_ranges, };
Re: [PATCH V3 6/7] drm/amd/pm: enable Wifi RFI mitigation feature support for SMU13.0.0
On 6/16/23 01:57, Evan Quan wrote: Fulfill the SMU13.0.0 support for Wifi RFI mitigation feature. Signed-off-by: Evan Quan Reviewed-by: Mario Limonciello --- drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 3 + drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h | 3 +- drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h | 3 + .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c| 9 +++ .../drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 60 +++ 5 files changed, 77 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h index aa63cc43d41c..a8a4be32cc59 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h @@ -323,6 +323,7 @@ enum smu_table_id SMU_TABLE_PACE, SMU_TABLE_ECCINFO, SMU_TABLE_COMBO_PPTABLE, + SMU_TABLE_WIFIBAND, SMU_TABLE_COUNT, }; @@ -1496,6 +1497,8 @@ enum smu_baco_seq { __dst_size); \ }) +#define HZ_IN_MHZ 100U + #if !defined(SWSMU_CODE_LAYER_L2) && !defined(SWSMU_CODE_LAYER_L3) && !defined(SWSMU_CODE_LAYER_L4) int smu_get_power_limit(void *handle, uint32_t *limit, diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h index 297b70b9388f..5bbb60289a79 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_types.h @@ -245,7 +245,8 @@ __SMU_DUMMY_MAP(AllowGpo), \ __SMU_DUMMY_MAP(Mode2Reset),\ __SMU_DUMMY_MAP(RequestI2cTransaction), \ - __SMU_DUMMY_MAP(GetMetricsTable), + __SMU_DUMMY_MAP(GetMetricsTable), \ + __SMU_DUMMY_MAP(EnableUCLKShadow), #undef __SMU_DUMMY_MAP #define __SMU_DUMMY_MAP(type) SMU_MSG_##type diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h index df3baaab0037..b6fae9b92303 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/smu_v13_0.h @@ -303,5 +303,8 @@ int smu_v13_0_get_pptable_from_firmware(struct smu_context *smu, uint32_t *size, uint32_t pptable_id); +int smu_v13_0_enable_uclk_shadow(struct smu_context *smu, +bool enablement); + #endif #endif diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c index 393c6a7b9609..8c2230d1d862 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0.c @@ -2453,3 +2453,12 @@ int smu_v13_0_mode1_reset(struct smu_context *smu) return ret; } + +int smu_v13_0_enable_uclk_shadow(struct smu_context *smu, +bool enablement) +{ + return smu_cmn_send_smc_msg_with_param(smu, + SMU_MSG_EnableUCLKShadow, + enablement, + NULL); +} diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c index 09405ef1e3c8..cf75feaee779 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c @@ -155,6 +155,7 @@ static struct cmn2asic_msg_mapping smu_v13_0_0_message_map[SMU_MSG_MAX_COUNT] = MSG_MAP(AllowGpo, PPSMC_MSG_SetGpoAllow, 0), MSG_MAP(AllowIHHostInterrupt, PPSMC_MSG_AllowIHHostInterrupt, 0), MSG_MAP(ReenableAcDcInterrupt, PPSMC_MSG_ReenableAcDcInterrupt, 0), + MSG_MAP(EnableUCLKShadow, PPSMC_MSG_EnableUCLKShadow, 0), }; static struct cmn2asic_mapping smu_v13_0_0_clk_map[SMU_CLK_COUNT] = { @@ -235,6 +236,7 @@ static struct cmn2asic_mapping smu_v13_0_0_table_map[SMU_TABLE_COUNT] = { TAB_MAP(DRIVER_SMU_CONFIG), TAB_MAP(ACTIVITY_MONITOR_COEFF), [SMU_TABLE_COMBO_PPTABLE] = {1, TABLE_COMBO_PPTABLE}, + TAB_MAP(WIFIBAND), TAB_MAP(I2C_COMMANDS), TAB_MAP(ECCINFO), }; @@ -472,6 +474,9 @@ static int smu_v13_0_0_tables_init(struct smu_context *smu) PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM); SMU_TABLE_INIT(tables, SMU_TABLE_ECCINFO, sizeof(EccInfoTable_t), PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM); + SMU_TABLE_INIT(tables, SMU_TABLE_WIFIBAND, + sizeof(WifiBandEntryTable_t), PAGE_SIZE, + AMDGPU_GEM_DOMAIN_VRAM); smu_table->metrics_table = kzalloc(sizeof(SmuMetricsExternal_t), GFP_KERNEL); if (!smu_table->metrics_table) @@ -2112,6 +2117,58 @@ static ssize_t smu_v13_0_0_get_ecc_info(struct smu_context *smu,
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
On 2023/6/18 20:11, Sui Jingfeng wrote: And screen_info is more about video specifci thing. screen_info is something more about video specific thing.
Re: [PATCH V3 5/7] drm/amd/pm: add flood detection for wbrf events
On 6/16/23 01:57, Evan Quan wrote: To protect PMFW from being overloaded. Signed-off-by: Evan Quan Reviewed-by: Mario Limonciello --- drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 28 --- drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 7 + 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c index 89f876cc60e6..2619e310ef54 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c @@ -1272,6 +1272,22 @@ static void smu_wbrf_event_handler(struct amdgpu_device *adev) { struct smu_context *smu = adev->powerplay.pp_handle; + schedule_delayed_work(>wbrf_delayed_work, + msecs_to_jiffies(SMU_WBRF_EVENT_HANDLING_PACE)); +} + +/** + * smu_wbrf_delayed_work_handler - callback on delayed work timer expired + * + * @work: struct work_struct pointer + * + * Flood is over and driver will consume the latest exclusion ranges. + */ +static void smu_wbrf_delayed_work_handler(struct work_struct *work) +{ + struct smu_context *smu = + container_of(work, struct smu_context, wbrf_delayed_work.work); + smu_wbrf_handle_exclusion_ranges(smu); } @@ -1311,6 +1327,9 @@ static int smu_wbrf_init(struct smu_context *smu) if (!smu->wbrf_supported) return 0; + INIT_DELAYED_WORK(>wbrf_delayed_work, + smu_wbrf_delayed_work_handler); + ret = amdgpu_acpi_register_wbrf_notify_handler(adev, smu_wbrf_event_handler); if (ret) @@ -1321,11 +1340,10 @@ static int smu_wbrf_init(struct smu_context *smu) * before our driver loaded. To make sure our driver * is awared of those exclusion ranges. */ - ret = smu_wbrf_handle_exclusion_ranges(smu); - if (ret) - dev_err(adev->dev, "Failed to handle wbrf exclusion ranges\n"); + schedule_delayed_work(>wbrf_delayed_work, + msecs_to_jiffies(SMU_WBRF_EVENT_HANDLING_PACE)); - return ret; + return 0; } /** @@ -1343,6 +1361,8 @@ static void smu_wbrf_fini(struct smu_context *smu) return; amdgpu_acpi_unregister_wbrf_notify_handler(adev); + + cancel_delayed_work_sync(>wbrf_delayed_work); } static int smu_smc_hw_setup(struct smu_context *smu) diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h index ff0af3da0be2..aa63cc43d41c 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h @@ -478,6 +478,12 @@ struct stb_context { #define WORKLOAD_POLICY_MAX 7 +/* + * Configure wbrf event handling pace as there can be only one + * event processed every SMU_WBRF_EVENT_HANDLING_PACE ms. + */ +#define SMU_WBRF_EVENT_HANDLING_PACE 10 + struct smu_context { struct amdgpu_device*adev; @@ -576,6 +582,7 @@ struct smu_context /* data structures for wbrf feature support */ boolwbrf_supported; + struct delayed_work wbrf_delayed_work; }; struct i2c_adapter;
Re: [PATCH V3 4/7] drm/amd/pm: setup the framework to support Wifi RFI mitigation feature
On 6/16/23 01:57, Evan Quan wrote: With WBRF feature supported, as a driver responding to the frequencies, amdgpu driver is able to do shadow pstate switching to mitigate possible interference(between its (G-)DDR memory clocks and local radio module frequency bands used by Wifi 6/6e/7). To make WBRF feature functional, the kernel needs to be configured with CONFIG_ACPI_WBRF and the platform is equipped with necessary ACPI based mechanism to get amdgpu driver notified. Signed-off-by: Evan Quan Reviewed-by: Mario Limonciello --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 26 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c | 63 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 19 ++ drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 184 ++ drivers/gpu/drm/amd/pm/swsmu/inc/amdgpu_smu.h | 20 ++ drivers/gpu/drm/amd/pm/swsmu/smu_internal.h | 3 + 6 files changed, 315 insertions(+) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 02b827785e39..2f2ec64ed1b2 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -50,6 +50,7 @@ #include #include #include +#include #include #include @@ -241,6 +242,7 @@ extern int amdgpu_num_kcq; #define AMDGPU_VCNFW_LOG_SIZE (32 * 1024) extern int amdgpu_vcnfw_log; extern int amdgpu_sg_display; +extern int amdgpu_wbrf; #define AMDGPU_VM_MAX_NUM_CTX 4096 #define AMDGPU_SG_THRESHOLD (256*1024*1024) @@ -741,6 +743,9 @@ struct amdgpu_reset_domain; */ #define AMDGPU_HAS_VRAM(_adev) ((_adev)->gmc.real_vram_size) +typedef +void (*wbrf_notify_handler) (struct amdgpu_device *adev); + struct amdgpu_device { struct device *dev; struct pci_dev *pdev; @@ -1050,6 +1055,8 @@ struct amdgpu_device { booljob_hang; booldc_enabled; + + wbrf_notify_handler wbrf_event_handler; }; static inline struct amdgpu_device *drm_to_adev(struct drm_device *ddev) @@ -1381,6 +1388,25 @@ static inline int amdgpu_acpi_smart_shift_update(struct drm_device *dev, enum amdgpu_ss ss_state) { return 0; } #endif +#if defined(CONFIG_ACPI_WBRF) +bool amdgpu_acpi_is_wbrf_supported(struct amdgpu_device *adev); +int amdgpu_acpi_wbrf_retrieve_exclusions(struct amdgpu_device *adev, +struct wbrf_ranges_out *exclusions_out); +int amdgpu_acpi_register_wbrf_notify_handler(struct amdgpu_device *adev, +wbrf_notify_handler handler); +int amdgpu_acpi_unregister_wbrf_notify_handler(struct amdgpu_device *adev); +#else +static inline bool amdgpu_acpi_is_wbrf_supported(struct amdgpu_device *adev) { return false; } +static inline +int amdgpu_acpi_wbrf_retrieve_exclusions(struct amdgpu_device *adev, +struct wbrf_ranges_out *exclusions_out) { return 0; } +static inline +int amdgpu_acpi_register_wbrf_notify_handler(struct amdgpu_device *adev, +wbrf_notify_handler handler) { return 0; } +static inline +int amdgpu_acpi_unregister_wbrf_notify_handler(struct amdgpu_device *adev) { return 0; } +#endif + #if defined(CONFIG_ACPI) && defined(CONFIG_SUSPEND) bool amdgpu_acpi_is_s3_active(struct amdgpu_device *adev); bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c index aeeec211861c..efbe6dd91d1a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_acpi.c @@ -1105,3 +1105,66 @@ bool amdgpu_acpi_is_s0ix_active(struct amdgpu_device *adev) } #endif /* CONFIG_SUSPEND */ + +#ifdef CONFIG_ACPI_WBRF +bool amdgpu_acpi_is_wbrf_supported(struct amdgpu_device *adev) +{ + struct acpi_device *acpi_dev = ACPI_COMPANION(adev->dev); + + if (!acpi_dev) + return false; + + return wbrf_supported_consumer(acpi_dev); +} + +int amdgpu_acpi_wbrf_retrieve_exclusions(struct amdgpu_device *adev, +struct wbrf_ranges_out *exclusions_out) +{ + struct acpi_device *acpi_dev = ACPI_COMPANION(adev->dev); + + if (!acpi_dev) + return -ENODEV; + + return wbrf_retrieve_exclusions(acpi_dev, exclusions_out); +} + +#define CPM_GPU_NOTIFY_COMMAND 0x55 +static void amdgpu_acpi_wbrf_event(acpi_handle handle, u32 event, void *data) +{ + struct amdgpu_device *adev = (struct amdgpu_device *)data; + + if (event == CPM_GPU_NOTIFY_COMMAND && + adev->wbrf_event_handler) + adev->wbrf_event_handler(adev); +} + +int amdgpu_acpi_register_wbrf_notify_handler(struct amdgpu_device *adev, +
Re: [PATCH V3 3/7] drm/amd/pm: update driver_if and ppsmc headers for coming wbrf feature
On 6/16/23 01:57, Evan Quan wrote: Add those data structures to support Wifi RFI mitigation feature. Signed-off-by: Evan Quan Reviewed-by: Mario Limonciello --- .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h | 14 +- .../pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h | 14 +- .../amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h | 3 ++- .../amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h | 3 ++- 4 files changed, 30 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h index b686fb68a6e7..d64188fb5839 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_0.h @@ -388,6 +388,17 @@ typedef struct { EccInfo_t EccInfo[24]; } EccInfoTable_t; +typedef struct { + uint16_t LowFreq; + uint16_t HighFreq; +} WifiOneBand_t; + +typedef struct { + uint32_t WifiBandEntryNum; + WifiOneBand_tWifiBandEntry[11]; + uint32_t MmHubPadding[8]; +} WifiBandEntryTable_t; + //D3HOT sequences typedef enum { BACO_SEQUENCE, @@ -1592,7 +1603,8 @@ typedef struct { #define TABLE_I2C_COMMANDS9 #define TABLE_DRIVER_INFO 10 #define TABLE_ECCINFO 11 -#define TABLE_COUNT 12 +#define TABLE_WIFIBAND12 +#define TABLE_COUNT 13 //IH Interupt ID #define IH_INTERRUPT_ID_TO_DRIVER 0xFE diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h index 4c46a0392451..77483e8485e7 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu13_driver_if_v13_0_7.h @@ -392,6 +392,17 @@ typedef struct { EccInfo_t EccInfo[24]; } EccInfoTable_t; +typedef struct { + uint16_t LowFreq; + uint16_t HighFreq; +} WifiOneBand_t; + +typedef struct { + uint32_t WifiBandEntryNum; + WifiOneBand_tWifiBandEntry[11]; + uint32_t MmHubPadding[8]; +} WifiBandEntryTable_t; + //D3HOT sequences typedef enum { BACO_SEQUENCE, @@ -1624,7 +1635,8 @@ typedef struct { #define TABLE_I2C_COMMANDS9 #define TABLE_DRIVER_INFO 10 #define TABLE_ECCINFO 11 -#define TABLE_COUNT 12 +#define TABLE_WIFIBAND12 +#define TABLE_COUNT 13 //IH Interupt ID #define IH_INTERRUPT_ID_TO_DRIVER 0xFE diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h index 10cff75b44d5..c98cc32d11bd 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_0_ppsmc.h @@ -138,7 +138,8 @@ #define PPSMC_MSG_SetBadMemoryPagesRetiredFlagsPerChannel 0x4A #define PPSMC_MSG_SetPriorityDeltaGain 0x4B #define PPSMC_MSG_AllowIHHostInterrupt 0x4C -#define PPSMC_Message_Count 0x4D +#define PPSMC_MSG_EnableUCLKShadow 0x51 +#define PPSMC_Message_Count 0x52 //Debug Dump Message #define DEBUGSMC_MSG_TestMessage0x1 diff --git a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h index 6aaefca9b595..a6bf9cdd130e 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h +++ b/drivers/gpu/drm/amd/pm/swsmu/inc/pmfw_if/smu_v13_0_7_ppsmc.h @@ -134,6 +134,7 @@ #define PPSMC_MSG_SetBadMemoryPagesRetiredFlagsPerChannel 0x4A #define PPSMC_MSG_SetPriorityDeltaGain 0x4B #define PPSMC_MSG_AllowIHHostInterrupt 0x4C -#define PPSMC_Message_Count 0x4D +#define PPSMC_MSG_EnableUCLKShadow 0x51 +#define PPSMC_Message_Count 0x52 #endif
Re: [PATCH V3 2/7] wifi: mac80211: Add support for ACPI WBRF
On 6/16/23 01:57, Evan Quan wrote: From: Mario Limonciello To support AMD's WBRF interference mitigation mechanism, Wifi adapters utilized in the system must register the frequencies in use(or unregister those frequencies no longer used) via the dedicated APCI calls. So that, other drivers responding to the frequencies can take proper actions to mitigate possible interference. To make WBRF feature functional, the kernel needs to be configured with CONFIG_ACPI_WBRF and the platform is equipped with WBRF support(from BIOS and drivers). Signed-off-by: Mario Limonciello Co-developed-by: Evan Quan Signed-off-by: Evan Quan -- v1->v2: - place the new added member(`wbrf_supported`) in ieee80211_local(Johannes) - handle chandefs change scenario properly(Johannes) - some minor fixes around code sharing and possible invalid input checks(Johannes) --- include/net/cfg80211.h | 8 +++ net/mac80211/Makefile | 2 + net/mac80211/chan.c| 11 +++ net/mac80211/ieee80211_i.h | 19 + net/mac80211/main.c| 2 + net/mac80211/wbrf.c| 137 + net/wireless/chan.c| 3 +- 7 files changed, 181 insertions(+), 1 deletion(-) create mode 100644 net/mac80211/wbrf.c diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 9e04f69712b1..c6dc337eafce 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -920,6 +920,14 @@ const struct cfg80211_chan_def * cfg80211_chandef_compatible(const struct cfg80211_chan_def *chandef1, const struct cfg80211_chan_def *chandef2); +/** + * nl80211_chan_width_to_mhz - get the channel width in Mhz + * @chan_width: the channel width from nl80211_chan_width + * Return: channel width in Mhz if the chan_width from nl80211_chan_width + * is valid. -1 otherwise. + */ +int nl80211_chan_width_to_mhz(enum nl80211_chan_width chan_width); + It's up to mac80211 maintainers, but I would think that the changes to change nl80211_chan_width_to_mhz from static to exported should be separate from the patch to introduced WBRF support in the series. /** * cfg80211_chandef_valid - check if a channel definition is valid * @chandef: the channel definition to check diff --git a/net/mac80211/Makefile b/net/mac80211/Makefile index b8de44da1fb8..709eb678f42a 100644 --- a/net/mac80211/Makefile +++ b/net/mac80211/Makefile @@ -65,4 +65,6 @@ rc80211_minstrel-$(CONFIG_MAC80211_DEBUGFS) += \ mac80211-$(CONFIG_MAC80211_RC_MINSTREL) += $(rc80211_minstrel-y) +mac80211-$(CONFIG_ACPI_WBRF) += wbrf.o + ccflags-y += -DDEBUG diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c index 77c90ed8f5d7..0c5289a9aa6c 100644 --- a/net/mac80211/chan.c +++ b/net/mac80211/chan.c @@ -506,11 +506,16 @@ static void _ieee80211_change_chanctx(struct ieee80211_local *local, WARN_ON(!cfg80211_chandef_compatible(>conf.def, chandef)); + ieee80211_remove_wbrf(local, >conf.def); + ctx->conf.def = *chandef; /* check if min chanctx also changed */ changed = IEEE80211_CHANCTX_CHANGE_WIDTH | _ieee80211_recalc_chanctx_min_def(local, ctx, rsvd_for); + + ieee80211_add_wbrf(local, >conf.def); + drv_change_chanctx(local, ctx, changed); if (!local->use_chanctx) { @@ -668,6 +673,10 @@ static int ieee80211_add_chanctx(struct ieee80211_local *local, lockdep_assert_held(>mtx); lockdep_assert_held(>chanctx_mtx); + err = ieee80211_add_wbrf(local, >conf.def); + if (err) + return err; + if (!local->use_chanctx) local->hw.conf.radar_enabled = ctx->conf.radar_enabled; @@ -748,6 +757,8 @@ static void ieee80211_del_chanctx(struct ieee80211_local *local, } ieee80211_recalc_idle(local); + + ieee80211_remove_wbrf(local, >conf.def); } static void ieee80211_free_chanctx(struct ieee80211_local *local, diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index b0372e76f373..f832de16073b 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -1591,6 +1591,10 @@ struct ieee80211_local { /* extended capabilities provided by mac80211 */ u8 ext_capa[8]; + +#ifdef CONFIG_ACPI_WBRF + bool wbrf_supported; +#endif }; static inline struct ieee80211_sub_if_data * @@ -2615,4 +2619,19 @@ ieee80211_eht_cap_ie_to_sta_eht_cap(struct ieee80211_sub_if_data *sdata, const struct ieee80211_eht_cap_elem *eht_cap_ie_elem, u8 eht_cap_len, struct link_sta_info *link_sta); + +#ifdef CONFIG_ACPI_WBRF +void ieee80211_check_wbrf_support(struct ieee80211_local *local); +int ieee80211_add_wbrf(struct ieee80211_local *local, + struct cfg80211_chan_def *chandef); +void ieee80211_remove_wbrf(struct ieee80211_local *local, + struct
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi, On 2023/6/18 20:11, Sui Jingfeng wrote: call back to use if successful Call back to us if the drm device driver bound to the PCI GPU successfully
Re: [PATCH V3 1/7] drivers/acpi: Add support for Wifi band RF mitigations
On 6/16/23 01:57, Evan Quan wrote: From: Mario Limonciello Due to electrical and mechanical constraints in certain platform designs there may be likely interference of relatively high-powered harmonics of the (G-)DDR memory clocks with local radio module frequency bands used by Wifi 6/6e/7. To mitigate this, AMD has introduced an ACPI based mechanism that devices can use to notify active use of particular frequencies so that devices can make relative internal adjustments as necessary to avoid this resonance. In order for a device to support this, the expected flow for device driver or subsystems: Drivers/subsystems contributing frequencies: 1) During probe, check `wbrf_supported_producer` to see if WBRF supported for the device. 2) If adding frequencies, then call `wbrf_add_exclusion` with the start and end ranges of the frequencies. 3) If removing frequencies, then call `wbrf_remove_exclusion` with start and end ranges of the frequencies. Drivers/subsystems responding to frequencies: 1) During probe, check `wbrf_supported_consumer` to see if WBRF is supported for the device. 2) Call the `wbrf_retrieve_exclusions` to retrieve the current exclusions on receiving an ACPI notification for a new frequency change. Signed-off-by: Mario Limonciello Co-developed-by: Evan Quan Signed-off-by: Evan Quan -- v1->v2: - move those wlan specific implementations to net/mac80211(Mario) --- drivers/acpi/Kconfig | 7 ++ drivers/acpi/Makefile| 2 + drivers/acpi/acpi_wbrf.c | 215 +++ include/linux/wbrf.h | 55 ++ 4 files changed, 279 insertions(+) create mode 100644 drivers/acpi/acpi_wbrf.c create mode 100644 include/linux/wbrf.h diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index ccbeab9500ec..9ee7c7dcc3e6 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -611,3 +611,10 @@ config X86_PM_TIMER You should nearly always say Y here because many modern systems require this timer. + +config ACPI_WBRF + bool "ACPI Wifi band RF mitigation mechanism" + help + Wifi band RF mitigation mechanism allows multiple drivers from + different domains to notify the frequencies in use so that hardware + can be reconfigured to avoid harmonic conflicts. \ No newline at end of file There should be a newline at the end of the Kconfig file. diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile index feb36c0b9446..be173e76aa62 100644 --- a/drivers/acpi/Makefile +++ b/drivers/acpi/Makefile @@ -131,3 +131,5 @@ obj-y += dptf/ obj-$(CONFIG_ARM64) += arm64/ obj-$(CONFIG_ACPI_VIOT) += viot.o + +obj-$(CONFIG_ACPI_WBRF)+= acpi_wbrf.o \ No newline at end of file diff --git a/drivers/acpi/acpi_wbrf.c b/drivers/acpi/acpi_wbrf.c new file mode 100644 index ..8c275998ac29 --- /dev/null +++ b/drivers/acpi/acpi_wbrf.c @@ -0,0 +1,215 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * AMD Wifi Band Exclusion Interface + * Copyright (C) 2023 Advanced Micro Devices + * + */ + +#include + +/* functions */ +#define WBRF_RECORD0x1 +#define WBRF_RETRIEVE 0x2 + +/* record actions */ +#define WBRF_RECORD_ADD0x0 +#define WBRF_RECORD_REMOVE 0x1 + +#define WBRF_REVISION 0x1 + +static const guid_t wifi_acpi_dsm_guid = + GUID_INIT(0x7b7656cf, 0xdc3d, 0x4c1c, + 0x83, 0xe9, 0x66, 0xe7, 0x21, 0xde, 0x30, 0x70); + +static int wbrf_dsm(struct acpi_device *adev, u8 fn, + union acpi_object *argv4, + union acpi_object **out) +{ + union acpi_object *obj; + int rc; + + obj = acpi_evaluate_dsm(adev->handle, _acpi_dsm_guid, + WBRF_REVISION, fn, argv4); + if (!obj) + return -ENXIO; + + switch (obj->type) { + case ACPI_TYPE_BUFFER: + if (!*out) { + rc = -EINVAL; + break; + } + *out = obj; + return 0; + + case ACPI_TYPE_INTEGER: + rc = obj->integer.value ? -EINVAL : 0; + break; + default: + rc = -EOPNOTSUPP; + } + ACPI_FREE(obj); + + return rc; +} + +static int wbrf_record(struct acpi_device *adev, uint8_t action, + struct wbrf_ranges_in *in) +{ + union acpi_object *argv4; + uint32_t num_of_ranges = 0; + uint32_t arg_idx = 0; + uint32_t loop_idx; + int ret; + + if (!in) + return -EINVAL; + + for (loop_idx = 0; loop_idx < ARRAY_SIZE(in->band_list); +loop_idx++) + if (in->band_list[loop_idx].start && + in->band_list[loop_idx].end) + num_of_ranges++; + + argv4 = kzalloc(sizeof(*argv4) * (2 * num_of_ranges + 2 + 1),
Re: [08/14] drm/ast: Set up release action right after enabling MMIO
Hi, Tested with ast2400 On 2023/6/16 21:52, Thomas Zimmermann wrote: Ast sets up a managed release of the MMIO access flags. Move this code next to the MMIO access code, so that it runs if other errors occur during the device initialization. Signed-off-by: Thomas Zimmermann Tested-by: Sui Jingfeng Reviewed-by: Sui Jingfeng --- drivers/gpu/drm/ast/ast_main.c | 38 +- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 3295876c09b35..6ff4b837e64d7 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -71,11 +71,25 @@ static void ast_enable_vga(struct drm_device *dev) ast_io_write8(ast, AST_IO_MISC_PORT_WRITE, 0x01); } -static void ast_enable_mmio(struct drm_device *dev) +/* + * Run this function as part of the HW device cleanup; not + * when the DRM device gets released. + */ +static void ast_enable_mmio_release(void *data) { - struct ast_device *ast = to_ast_device(dev); + struct ast_device *ast = data; + + /* enable standard VGA decode */ + ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04); +} + +static int ast_enable_mmio(struct ast_device *ast) +{ + struct drm_device *dev = >base; ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x06); + + return devm_add_action_or_reset(dev->dev, ast_enable_mmio_release, ast); } static void ast_open_key(struct ast_device *ast) @@ -392,18 +406,6 @@ static int ast_get_dram_info(struct drm_device *dev) return 0; } -/* - * Run this function as part of the HW device cleanup; not - * when the DRM device gets released. - */ -static void ast_device_release(void *data) -{ - struct ast_device *ast = data; - - /* enable standard VGA decode */ - ast_set_index_reg(ast, AST_IO_CRTC_PORT, 0xa1, 0x04); -} - struct ast_device *ast_device_create(const struct drm_driver *drv, struct pci_dev *pdev, unsigned long flags) @@ -465,7 +467,9 @@ struct ast_device *ast_device_create(const struct drm_driver *drv, /* Enable extended register access */ ast_open_key(ast); - ast_enable_mmio(dev); + ret = ast_enable_mmio(ast); + if (ret) + return ERR_PTR(ret); /* Find out whether P2A works or whether to use device-tree */ ast_detect_config_mode(dev, _rev); @@ -498,9 +502,5 @@ struct ast_device *ast_device_create(const struct drm_driver *drv, if (ret) return ERR_PTR(ret); - ret = devm_add_action_or_reset(dev->dev, ast_device_release, ast); - if (ret) - return ERR_PTR(ret); - return ast; } -- Jingfeng
linux-next: manual merge of the drm tree with Linus' tree
Hi all, Today's linux-next merge of the drm tree got a conflict in: drivers/gpu/drm/amd/amdgpu/amdgpu_object.c between commit: 34e5a54327dc ("Revert "drm/amdgpu: remove TOPDOWN flags when allocating VRAM in large bar system"") from Linus' tree and commit: 7f6db89418f9 ("drm/amdgpu: dGPU mode placement support memory partition") from the drm tree. I fixed it up (see below) and can carry the fix as necessary. This is now fixed as far as linux-next is concerned, but any non trivial conflicts should be mentioned to your upstream maintainer when your tree is submitted for merging. You may also want to consider cooperating with the maintainer of the conflicting tree to minimise any particularly complex conflicts. -- Cheers, Stephen Rothwell diff --cc drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index a70103ac0026,f76649e523a0.. --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@@ -139,8 -149,8 +149,8 @@@ void amdgpu_bo_placement_from_domain(st places[c].flags = 0; if (flags & AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED) - places[c].lpfn = visible_pfn; + places[c].lpfn = min_not_zero(places[c].lpfn, visible_pfn); - else if (adev->gmc.real_vram_size != adev->gmc.visible_vram_size) + else places[c].flags |= TTM_PL_FLAG_TOPDOWN; if (flags & AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS) pgpffntsKMy3e.pgp Description: OpenPGP digital signature
[pull] drm/msm: drm-msm-next-2023-06-18 for v6.5
Hi Dave, This is the pull for v6.5, see below for description. It includes a back-merge of drm-next for DSC helpers. Sorry it is somewhat late, I was out of town last week. (resend without all the back-merge commits in the msg) The following changes since commit ba57b9b11f78530146f02b776854b2b6b6d344a4: Merge tag 'drm-intel-gt-next-2023-06-08' of git://anongit.freedesktop.org/drm/drm-intel into drm-next (2023-06-09 16:43:36 +1000) are available in the Git repository at: https://gitlab.freedesktop.org/drm/msm.git tags/drm-msm-next-2023-06-18 for you to fetch changes up to cd036d542afb82adfbbd43c5dbeb7010e8e91ee7: drm/msm/a6xx: Add A610 speedbin support (2023-06-18 11:35:27 -0700) Updates for v6.5.. this includes a backmerg of drm-next tree to be able to use new DRM DSC helpers. Core: + Add Marijn Suijten as drm/msm reviewer + Adreno A660 bindings + SM8350 MDSS bindings fix + Fix adreno_is_a690() warnings + More generic (DRM) and MSM-specific DSC helpers DP: + Removed obsolete USB-PD remains + Documented DP compatible string for sm8550 platform DPU: + Enable missing features (DSPP, DSC, split display) on sc8180x, sc8280xp, sm8450 + Enabled writeback on sc7280 + Implemented tearcheck support to support vsync on SM150 and newer platforms + Native HDMI output support + Dropped unused features: regdma, GC, IGC + Fixed the DSC flush operations + Simplified QoS handling, removing obsolete and unused features and merging SSPP and WB code paths + Reworked dpu_encoder initialisation path + Enabled DSPP support on sdm845 + Disabled color-management if DSPP blocks are not available + Added support for DSC 1.2 blocks found on sm8350 and later + Added .fb_dirty to fix CMD panels DSI: + Drop powerup quirks in favour of using pre_enable_prev_first for downstream bridges + Fixed 14nm DSI PHY programming + Added support for DSI and 28nm DSI PHY on MSM8226 platform + Make use of DRM and MSM DSC helpers MDP5: + Added support for display controller on MSM8226 platform GPU: + A690 support + Don't set IO_PGTABLE_QUIRK_ARM_OUTER_WBWA on devices with coherent SMMU (like A690) + Move cmdstream dumping out of fence signaling path + Cleanups + Support for a6xx devices without GMU (aka "GMU wrapper" + a610 support + a619_holi support (a619 variant without GMU) Abhinav Kumar (8): drm/msm/dpu: remove DPU_DSPP_GC handling in dspp flush drm/msm/dpu: remove DPU_DSPP_IGC handling in dspp flush drm/msm/dpu: remove GC and IGC related code from dpu catalog drm/msm/dpu: drop DSPP_MSM8998_MASK from hw catalog drm/msm/dpu: add writeback support for sc7280 drm/msm/dp: add module parameter for PSR drm/msm/dpu: add DSC blocks to the catalog of MSM8998 drm/msm/dpu: add DSC 1.2 hw blocks for relevant chipsets Arnaud Vrac (5): drm/msm/dpu: tweak msm8998 hw catalog values drm/msm/dpu: tweak lm pairings in msm8998 hw catalog drm/msm/dpu: use hsync/vsync polarity set by the encoder drm/msm/dpu: fix cursor block register bit offset in msm8998 hw catalog drm/msm/dpu: set max cursor width to 512x512 Bjorn Andersson (5): drm/msm/dp: Clean up logs dp_power module drm/msm/dp: Clean up pdev/dev duplication in dp_power drm/msm/adreno: Add Adreno A690 support drm/msm/dp: Drop aux devices together with DP controller drm/msm/dp: Free resources after unregistering them Dan Carpenter (2): drm/msm/dpu: clean up dpu_kms_get_clk_rate() returns drm/msm/dpu: tidy up some error checking Dmitry Baryshkov (61): dt-bindings: display/msm/gmu: add Adreno 660 support drm/msm/dpu: enable DPU_CTL_SPLIT_DISPLAY for sc8280xp drm/msm/dpu: enable DSPP_2/3 for LM_2/3 on sm8450 drm/msm/dpu: enable DSPP and DSC on sc8180x drm/msm/dpu: use CTL_SC7280_MASK for sm8450's ctl_0 drm/msm/dpu: simplify intf allocation code drm/msm/dsi: don't allow enabling 14nm VCO with unprogrammed rate drm/msm/dpu: add HDMI output support drm/msm/dpu: drop the regdma configuration drm/msm/dpu: stop mapping the regdma region drm/msm/dpu: drop unused SSPP sub-block information drm/msm/dpu: drop SSPP's SRC subblock drm/msm/dpu: access QSEED registers directly drm/msm/dpu: access CSC/CSC10 registers directly drm/msm/dpu: replace IS_ERR_OR_NULL with IS_ERR during DSC init drm/msm/dpu: remove futile checks from dpu_rm_init() drm/msm/dpu: use PINGPONG_NONE for LMs with no PP attached drm/msm/dpu: move PINGPONG_NONE check to dpu_lm_init() drm/msm/dpu: fix SSPP register definitions drm/msm/dpu: simplify CDP programming drm/msm/dpu: fix the condition for (not) applying QoS to CURSOR SSPP drm/msm/dpu: rearrange QoS setting code drm/msm/dpu: drop DPU_PLANE_QOS_VBLANK_CTRL drm/msm/dpu: simplify
[PATCH v4 1/2] drm/fourcc: Add NV20 and NV30 YUV formats
DRM_FORMAT_NV20 and DRM_FORMAT_NV30 formats is the 2x1 and non-subsampled variant of NV15, a 10-bit 2-plane YUV format that has no padding between components. Instead, luminance and chrominance samples are grouped into 4s so that each group is packed into an integer number of bytes: = UVUV = 4 * 10 bits = 40 bits = 5 bytes The '20' and '30' suffix refers to the optimum effective bits per pixel which is achieved when the total number of luminance samples is a multiple of 4. V2: Added NV30 format Signed-off-by: Jonas Karlman Reviewed-by: Sandy Huang --- drivers/gpu/drm/drm_fourcc.c | 8 include/uapi/drm/drm_fourcc.h | 2 ++ 2 files changed, 10 insertions(+) diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c index 0f17dfa8702b..193cf8ed7912 100644 --- a/drivers/gpu/drm/drm_fourcc.c +++ b/drivers/gpu/drm/drm_fourcc.c @@ -299,6 +299,14 @@ const struct drm_format_info *__drm_format_info(u32 format) .num_planes = 2, .char_per_block = { 5, 5, 0 }, .block_w = { 4, 2, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2, .vsub = 2, .is_yuv = true }, + { .format = DRM_FORMAT_NV20,.depth = 0, + .num_planes = 2, .char_per_block = { 5, 5, 0 }, + .block_w = { 4, 2, 0 }, .block_h = { 1, 1, 0 }, .hsub = 2, + .vsub = 1, .is_yuv = true }, + { .format = DRM_FORMAT_NV30,.depth = 0, + .num_planes = 2, .char_per_block = { 5, 5, 0 }, + .block_w = { 4, 2, 0 }, .block_h = { 1, 1, 0 }, .hsub = 1, + .vsub = 1, .is_yuv = true }, { .format = DRM_FORMAT_Q410,.depth = 0, .num_planes = 3, .char_per_block = { 2, 2, 2 }, .block_w = { 1, 1, 1 }, .block_h = { 1, 1, 1 }, .hsub = 1, diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h index 8db7fd3f743e..3151f1fc7ebb 100644 --- a/include/uapi/drm/drm_fourcc.h +++ b/include/uapi/drm/drm_fourcc.h @@ -323,6 +323,8 @@ extern "C" { * index 1 = Cr:Cb plane, [39:0] Cr1:Cb1:Cr0:Cb0 little endian */ #define DRM_FORMAT_NV15fourcc_code('N', 'V', '1', '5') /* 2x2 subsampled Cr:Cb plane */ +#define DRM_FORMAT_NV20fourcc_code('N', 'V', '2', '0') /* 2x1 subsampled Cr:Cb plane */ +#define DRM_FORMAT_NV30fourcc_code('N', 'V', '3', '0') /* non-subsampled Cr:Cb plane */ /* * 2 plane YCbCr MSB aligned -- 2.40.1
[PATCH v4 2/2] drm/rockchip: vop: Add NV15, NV20 and NV30 support
Add support for displaying 10-bit 4:2:0 and 4:2:2 formats produced by the Rockchip Video Decoder on RK322X, RK3288, RK3328 and RK3399. Also add support for 10-bit 4:4:4 format while at it. V4: Rework RK3328/RK3399 win0/1 data to not affect RK3368 V2: Added NV30 support Signed-off-by: Jonas Karlman Reviewed-by: Sandy Huang --- drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 29 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 + drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 63 + 3 files changed, 81 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c index a530ecc4d207..fa0405ad0acf 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c @@ -273,6 +273,18 @@ static bool has_uv_swapped(uint32_t format) } } +static bool is_fmt_10(uint32_t format) +{ + switch (format) { + case DRM_FORMAT_NV15: + case DRM_FORMAT_NV20: + case DRM_FORMAT_NV30: + return true; + default: + return false; + } +} + static enum vop_data_format vop_convert_format(uint32_t format) { switch (format) { @@ -288,12 +300,15 @@ static enum vop_data_format vop_convert_format(uint32_t format) case DRM_FORMAT_BGR565: return VOP_FMT_RGB565; case DRM_FORMAT_NV12: + case DRM_FORMAT_NV15: case DRM_FORMAT_NV21: return VOP_FMT_YUV420SP; case DRM_FORMAT_NV16: + case DRM_FORMAT_NV20: case DRM_FORMAT_NV61: return VOP_FMT_YUV422SP; case DRM_FORMAT_NV24: + case DRM_FORMAT_NV30: case DRM_FORMAT_NV42: return VOP_FMT_YUV444SP; default: @@ -944,7 +959,12 @@ static void vop_plane_atomic_update(struct drm_plane *plane, dsp_sty = dest->y1 + crtc->mode.vtotal - crtc->mode.vsync_start; dsp_st = dsp_sty << 16 | (dsp_stx & 0x); - offset = (src->x1 >> 16) * fb->format->cpp[0]; + if (fb->format->block_w[0]) + offset = (src->x1 >> 16) * fb->format->char_per_block[0] / +fb->format->block_w[0]; + else + offset = (src->x1 >> 16) * fb->format->cpp[0]; + offset += (src->y1 >> 16) * fb->pitches[0]; dma_addr = rk_obj->dma_addr + offset + fb->offsets[0]; @@ -970,6 +990,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane, } VOP_WIN_SET(vop, win, format, format); + VOP_WIN_SET(vop, win, fmt_10, is_fmt_10(fb->format->format)); VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4)); VOP_WIN_SET(vop, win, yrgb_mst, dma_addr); VOP_WIN_YUV2YUV_SET(vop, win_yuv2yuv, y2r_en, is_yuv); @@ -986,7 +1007,11 @@ static void vop_plane_atomic_update(struct drm_plane *plane, uv_obj = fb->obj[1]; rk_uv_obj = to_rockchip_obj(uv_obj); - offset = (src->x1 >> 16) * bpp / hsub; + if (fb->format->block_w[1]) + offset = (src->x1 >> 16) * bpp / +fb->format->block_w[1] / hsub; + else + offset = (src->x1 >> 16) * bpp / hsub; offset += (src->y1 >> 16) * fb->pitches[1] / vsub; dma_addr = rk_uv_obj->dma_addr + offset + fb->offsets[1]; diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h index 5f56e0597df8..4b2daefeb8c1 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.h +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.h @@ -186,6 +186,7 @@ struct vop_win_phy { struct vop_reg enable; struct vop_reg gate; struct vop_reg format; + struct vop_reg fmt_10; struct vop_reg rb_swap; struct vop_reg uv_swap; struct vop_reg act_info; diff --git a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c index 20ac7811c5eb..4f1134010498 100644 --- a/drivers/gpu/drm/rockchip/rockchip_vop_reg.c +++ b/drivers/gpu/drm/rockchip/rockchip_vop_reg.c @@ -53,6 +53,23 @@ static const uint32_t formats_win_full[] = { DRM_FORMAT_NV42, }; +static const uint32_t formats_win_full_10[] = { + DRM_FORMAT_XRGB, + DRM_FORMAT_ARGB, + DRM_FORMAT_XBGR, + DRM_FORMAT_ABGR, + DRM_FORMAT_RGB888, + DRM_FORMAT_BGR888, + DRM_FORMAT_RGB565, + DRM_FORMAT_BGR565, + DRM_FORMAT_NV12, + DRM_FORMAT_NV16, + DRM_FORMAT_NV24, + DRM_FORMAT_NV15, + DRM_FORMAT_NV20, + DRM_FORMAT_NV30, +}; + static const uint64_t format_modifiers_win_full[] = { DRM_FORMAT_MOD_LINEAR, DRM_FORMAT_MOD_INVALID, @@ -627,11 +644,12 @@ static const struct vop_scl_regs rk3288_win_full_scl = { static const struct vop_win_phy rk3288_win01_data = { .scl =
[PATCH v4 0/2] drm/rockchip: vop: Add NV15, NV20 and NV30 support
This series add support for displaying 10-bit 4:2:0 and 4:2:2 formats produced by the Rockchip Video Decoder on RK322X, RK3288, RK3328, RK3368 and RK3399. Also include 10-bit 4:4:4 support since VOP can support that also. First patch adds new fourcc 10-bit YUV formats with 4:2:2/4:4:4 sub-sampling. Second patch adds support for displaying the new fourcc formats. These patches has been in use by LibreELEC and other distros for the past 3+ years, hoping they can be merged this time around :-) Changes in v4: - Rework RK3328/RK3399 win0/1 data to not affect RK3368 Changes in v3: - No changes, rebased on next-20230616 - R-B tags was collected Changes in v2: - Add NV30 format - R-B tags was not collected due to NV30 changes This series is also available at [1]. [1] https://github.com/Kwiboo/linux-rockchip/commits/next-20230616-vop-nv15 Jonas Karlman (2): drm/fourcc: Add NV20 and NV30 YUV formats drm/rockchip: vop: Add NV15, NV20 and NV30 support drivers/gpu/drm/drm_fourcc.c| 8 +++ drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 29 +- drivers/gpu/drm/rockchip/rockchip_drm_vop.h | 1 + drivers/gpu/drm/rockchip/rockchip_vop_reg.c | 63 + include/uapi/drm/drm_fourcc.h | 2 + 5 files changed, 91 insertions(+), 12 deletions(-) -- 2.40.1
[PATCH] video/hdmi: Reorder fields in 'struct hdmi_avi_infoframe'
Group some variables based on their sizes to reduce hole and avoid padding. On x86_64, this shrinks the size of 'struct hdmi_avi_infoframe' from 68 to 60 bytes. It saves a few bytes of memory and is more cache-line friendly. This also reduces the union hdmi_infoframe the same way. Signed-off-by: Christophe JAILLET --- Using pahole Before: == struct hdmi_avi_infoframe { enum hdmi_infoframe_type type; /* 0 4 */ unsigned char version; /* 4 1 */ unsigned char length; /* 5 1 */ /* XXX 2 bytes hole, try to pack */ enum hdmi_colorspace colorspace; /* 8 4 */ enum hdmi_scan_modescan_mode;/*12 4 */ enum hdmi_colorimetry colorimetry; /*16 4 */ enum hdmi_picture_aspect picture_aspect; /*20 4 */ enum hdmi_active_aspectactive_aspect;/*24 4 */ bool itc; /*28 1 */ /* XXX 3 bytes hole, try to pack */ enum hdmi_extended_colorimetry extended_colorimetry; /*32 4 */ enum hdmi_quantization_range quantization_range; /*36 4 */ enum hdmi_nups nups; /*40 4 */ unsigned char video_code; /*44 1 */ /* XXX 3 bytes hole, try to pack */ enum hdmi_ycc_quantization_range ycc_quantization_range; /*48 4 */ enum hdmi_content_type content_type; /*52 4 */ unsigned char pixel_repeat; /*56 1 */ /* XXX 1 byte hole, try to pack */ short unsigned int top_bar; /*58 2 */ short unsigned int bottom_bar; /*60 2 */ short unsigned int left_bar; /*62 2 */ /* --- cacheline 1 boundary (64 bytes) --- */ short unsigned int right_bar;/*64 2 */ /* size: 68, cachelines: 2, members: 20 */ /* sum members: 57, holes: 4, sum holes: 9 */ /* padding: 2 */ /* last cacheline: 4 bytes */ }; After: = struct hdmi_avi_infoframe { enum hdmi_infoframe_type type; /* 0 4 */ unsigned char version; /* 4 1 */ unsigned char length; /* 5 1 */ bool itc; /* 6 1 */ unsigned char pixel_repeat; /* 7 1 */ enum hdmi_colorspace colorspace; /* 8 4 */ enum hdmi_scan_modescan_mode;/*12 4 */ enum hdmi_colorimetry colorimetry; /*16 4 */ enum hdmi_picture_aspect picture_aspect; /*20 4 */ enum hdmi_active_aspectactive_aspect;/*24 4 */ enum hdmi_extended_colorimetry extended_colorimetry; /*28 4 */ enum hdmi_quantization_range quantization_range; /*32 4 */ enum hdmi_nups nups; /*36 4 */ unsigned char video_code; /*40 1 */ /* XXX 3 bytes hole, try to pack */ enum hdmi_ycc_quantization_range ycc_quantization_range; /*44 4 */ enum hdmi_content_type content_type; /*48 4 */ short unsigned int top_bar; /*52 2 */ short unsigned int bottom_bar; /*54 2 */ short unsigned int left_bar; /*56 2 */ short unsigned int right_bar;/*58 2 */ /* size: 60, cachelines: 1, members: 20 */ /* sum members: 57, holes: 1, sum holes: 3 */ /* last cacheline: 60 bytes */ }; --- include/linux/hdmi.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/linux/hdmi.h b/include/linux/hdmi.h index 2f4dcc8d060e..3bb87bf6bc65 100644 --- a/include/linux/hdmi.h +++ b/include/linux/hdmi.h @@ -170,19 +170,19 @@ struct hdmi_avi_infoframe { enum hdmi_infoframe_type type; unsigned char version; unsigned char length; + bool itc; + unsigned char pixel_repeat; enum hdmi_colorspace colorspace; enum hdmi_scan_mode scan_mode; enum hdmi_colorimetry colorimetry; enum hdmi_picture_aspect picture_aspect; enum hdmi_active_aspect active_aspect; - bool itc; enum hdmi_extended_colorimetry extended_colorimetry; enum hdmi_quantization_range quantization_range; enum hdmi_nups nups; unsigned char video_code; enum hdmi_ycc_quantization_range ycc_quantization_range; enum
Re: [PATCH v2] drm/i915: Replace kmap() with kmap_local_page()
Sumitra Sharma wrote: > kmap() has been deprecated in favor of the kmap_local_page() > due to high cost, restricted mapping space, the overhead of a > global lock for synchronization, and making the process sleep > in the absence of free slots. > > kmap_local_page() is faster than kmap() and offers thread-local > and CPU-local mappings, take pagefaults in a local kmap region > and preserves preemption by saving the mappings of outgoing tasks > and restoring those of the incoming one during a context switch. > > The mapping is kept thread local in the function > “i915_vma_coredump_create” in i915_gpu_error.c > > Therefore, replace kmap() with kmap_local_page(). > > Suggested-by: Ira Weiny > NIT: No need for the line break between Suggested-by and your signed off line. > Signed-off-by: Sumitra Sharma > --- > > Changes in v2: > - Replace kmap() with kmap_local_page(). Generally it is customary to attribute a change like this to those who suggested it in a V1 review. For example: - Tvrtko/Thomas: Use kmap_local_page() instead of page_address() Also I don't see Thomas on the new email list. Since he took the time to review V1 he might want to check this version out. I've added him to the 'To:' list. Also a link to V1 is nice. B4 formats it like this: - Link to v1: https://lore.kernel.org/all/20230614123556.ga381...@sumitra.com/ All that said the code looks good to me. So with the above changes. Reviewed-by: Ira Weiny > - Change commit subject and message. > > drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > b/drivers/gpu/drm/i915/i915_gpu_error.c > index f020c0086fbc..bc41500eedf5 100644 > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > @@ -1164,9 +1164,9 @@ i915_vma_coredump_create(const struct intel_gt *gt, > > drm_clflush_pages(, 1); > > - s = kmap(page); > + s = kmap_local_page(page); > ret = compress_page(compress, s, dst, false); > - kunmap(page); > + kunmap_local(s); > > drm_clflush_pages(, 1); > > -- > 2.25.1 >
Re: [PATCH] drm: bridge: dw-mipi-dsi: Fix enable/disable of DSI controller
Tested-by: Julian Fairfax On 17.06.23 17:06, Ondřej Jirman wrote: > From: Ondrej Jirman > > Before this patch, booting to Linux VT and doing a simple: > >echo 2 > /sys/class/graphics/fb0/blank >echo 0 > /sys/class/graphics/fb0/blank > > would result in failures to re-enable the panel. Mode set callback is > called only once during boot in this scenario, while calls to > enable/disable callbacks are balanced afterwards. The driver doesn't > work unless userspace calls modeset before enabling the CRTC/connector. > > This patch moves enabling of the DSI host from mode_set into pre_enable > callback, and removes some old hacks where this bridge driver is > directly calling into other bridge driver's callbacks. > > pre_enable_prev_first flag is set on the panel's bridge so that panel > drivers will get their prepare function called between DSI host's > pre_enable and enable callbacks, so that they get a chance to > perform panel setup while DSI host is already enabled in command > mode. Otherwise panel's prepare would be called before DSI host > is enabled, and any DSI communication used in prepare callback > would fail. > > With all these changes, the enable/disable sequence is now well > balanced, and host's and panel's callbacks are called in proper order > documented in the drm_panel API documentation without needing the old > hacks. (Mainly that panel->prepare is called when DSI host is ready to > allow the panel driver to send DSI commands and vice versa during > disable.) > > Tested on Pinephone Pro. Trace of the callbacks follows. > > Before: > > [1.253882] dw-mipi-dsi-rockchip ff96.dsi: mode_set > [1.290732] panel-himax-hx8394 ff96.dsi.0: prepare > [1.475576] dw-mipi-dsi-rockchip ff96.dsi: enable > [1.475593] panel-himax-hx8394 ff96.dsi.0: enable > > echo 2 > /sys/class/graphics/fb0/blank > > [ 13.722799] panel-himax-hx8394 ff96.dsi.0: disable > [ 13.774502] dw-mipi-dsi-rockchip ff96.dsi: post_disable > [ 13.774526] panel-himax-hx8394 ff96.dsi.0: unprepare > > echo 0 > /sys/class/graphics/fb0/blank > > [ 17.735796] panel-himax-hx8394 ff96.dsi.0: prepare > [ 17.923522] dw-mipi-dsi-rockchip ff96.dsi: enable > [ 17.923540] panel-himax-hx8394 ff96.dsi.0: enable > [ 17.944330] dw-mipi-dsi-rockchip ff96.dsi: failed to write command FIFO > [ 17.944335] panel-himax-hx8394 ff96.dsi.0: sending command 0xb9 > failed: -110 > [ 17.944340] panel-himax-hx8394 ff96.dsi.0: Panel init sequence failed: > -110 > > echo 2 > /sys/class/graphics/fb0/blank > > [ 431.148583] panel-himax-hx8394 ff96.dsi.0: disable > [ 431.169259] dw-mipi-dsi-rockchip ff96.dsi: failed to write command FIFO > [ 431.169268] panel-himax-hx8394 ff96.dsi.0: Failed to enter sleep mode: > -110 > [ 431.169282] dw-mipi-dsi-rockchip ff96.dsi: post_disable > [ 431.169316] panel-himax-hx8394 ff96.dsi.0: unprepare > [ 431.169357] pclk_mipi_dsi0 already disabled > > echo 0 > /sys/class/graphics/fb0/blank > > [ 432.796851] panel-himax-hx8394 ff96.dsi.0: prepare > [ 432.981537] dw-mipi-dsi-rockchip ff96.dsi: enable > [ 432.981568] panel-himax-hx8394 ff96.dsi.0: enable > [ 433.002290] dw-mipi-dsi-rockchip ff96.dsi: failed to write command FIFO > [ 433.002299] panel-himax-hx8394 ff96.dsi.0: sending command 0xb9 > failed: -110 > [ 433.002312] panel-himax-hx8394 ff96.dsi.0: Panel init sequence failed: > -110 > > --- > > After: > > [1.248372] dw-mipi-dsi-rockchip ff96.dsi: mode_set > [1.248704] dw-mipi-dsi-rockchip ff96.dsi: pre_enable > [1.285377] panel-himax-hx8394 ff96.dsi.0: prepare > [1.468392] dw-mipi-dsi-rockchip ff96.dsi: enable > [1.468421] panel-himax-hx8394 ff96.dsi.0: enable > > echo 2 > /sys/class/graphics/fb0/blank > > [ 16.210357] panel-himax-hx8394 ff96.dsi.0: disable > [ 16.261315] dw-mipi-dsi-rockchip ff96.dsi: post_disable > [ 16.261339] panel-himax-hx8394 ff96.dsi.0: unprepare > > echo 0 > /sys/class/graphics/fb0/blank > > [ 19.161453] dw-mipi-dsi-rockchip ff96.dsi: pre_enable > [ 19.197869] panel-himax-hx8394 ff96.dsi.0: prepare > [ 19.382141] dw-mipi-dsi-rockchip ff96.dsi: enable > [ 19.382158] panel-himax-hx8394 ff96.dsi.0: enable > > Signed-off-by: Ondrej Jirman > --- > drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 28 +++ > 1 file changed, 16 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > index b2efecf7d160..352c6829259a 100644 > --- a/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c > @@ -265,6 +265,7 @@ struct dw_mipi_dsi { > struct dw_mipi_dsi *master; /* dual-dsi master ptr */ > struct dw_mipi_dsi *slave; /* dual-dsi slave ptr */ > > + struct
Re: [PATCH v2 2/6] dt-bindings: display: st7789v: bound the number of Rx data lines
Hello Maxime, max...@cerno.tech wrote on Sun, 18 Jun 2023 16:37:58 +0200: > Hi, > > On Fri, Jun 16, 2023 at 06:32:51PM +0200, Miquel Raynal wrote: > > The ST7789V LCD controller supports regular SPI wiring, as well as no Rx > > data line at all. The operating system needs to know whether it can read > > registers from the device or not. Let's detail this specific design > > possibility by bounding the spi-rx-bus-width property. > > > > Signed-off-by: Miquel Raynal > > --- > > .../devicetree/bindings/display/panel/sitronix,st7789v.yaml | 4 > > 1 file changed, 4 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml > > b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml > > index 0ccf0487fd8e..a25df7e1df88 100644 > > --- a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml > > +++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml > > @@ -29,6 +29,10 @@ properties: > >spi-cpha: true > >spi-cpol: true > > > > + spi-rx-bus-width: > > +minimum: 0 > > +maximum: 1 > > + > > It's not clear to me what the default would be? This binding references spi-peripheral-props.yaml which sets the default to 1, I believe it is sane to keep it that way? Thanks, Miquèl
Re: [PATCH 2/2] drm/panel: simple: Add support for Mitsubishi AA084XE01
Hi Miquel, On Fri, Jun 16, 2023 at 06:45:24PM +0200, Miquel Raynal wrote: > From: Thomas Weber > > Add support for the Mitsubishi AA084XE01 panel which is an 8.4 inch XGA > TFT-LCD module for industrial use. > > Link: https://www.mouser.fr/datasheet/2/274/aa084xe01_e-364171.pdf > Signed-off-by: Thomas Weber > Signed-off-by: Miquel Raynal > --- > drivers/gpu/drm/panel/panel-simple.c | 27 +++ > 1 file changed, 27 insertions(+) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > b/drivers/gpu/drm/panel/panel-simple.c > index 8a3b685c2fcc..f79c9f9124a0 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -2670,6 +2670,30 @@ static const struct panel_desc mitsubishi_aa070mc01 = { > .bus_flags = DRM_BUS_FLAG_DE_HIGH, > }; > > +static const struct drm_display_mode mitsubishi_aa084xe01_mode = { > + .clock = 8, > + .hdisplay = 1024, > + .hsync_start = 1024 + 24, > + .hsync_end = 1024 + 24 + 63, > + .htotal = 1024 + 24 + 63 + 1, > + .vdisplay = 768, > + .vsync_start = 768 + 3, > + .vsync_end = 768 + 3 + 6, > + .vtotal = 768 + 3 + 6 + 1, > + .flags = DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC, > +}; > + > +static const struct panel_desc mitsubishi_aa084xe01 = { > + .modes = _aa084xe01_mode, > + .num_modes = 1, > + .bpc = 8, > + .size = { > + .width = 1024, > + .height = 768, > + }, > + .bus_format = MEDIA_BUS_FMT_RGB565_1X16, > +}; The .connector_type must be set. Also consider if .bus_flags needs to be specified. Sam
Re: [PATCH v2 6/6] drm/panel: sitronix-st7789v: Check display ID
On Fri, 16 Jun 2023 18:32:55 +0200, Miquel Raynal wrote: > A very basic debugging rule when a device is connected for the first > time is to access a read-only register which contains known data in > order to ensure the communication protocol is properly working. This > driver lacked any read helper which is often a critical piece for > speed-up bring-ups. > > [ ... ] Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH v2 5/6] drm/panel: sitronix-st7789v: Add EDT ET028013DMA panel support
On Fri, 16 Jun 2023 18:32:54 +0200, Miquel Raynal wrote: > This panel from Emerging Display Technologies Corporation features an > ST7789V2 LCD controller panel inside which is almost identical to what > the Sitronix panel driver supports. > > In practice, the module physical size is specific, and experiments show > > [ ... ] Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH v2 4/6] drm/panel: sitronix-st7789v: Clarify a definition
On Fri, 16 Jun 2023 18:32:53 +0200, Miquel Raynal wrote: > The Sitronix datasheet explains BIT(1) of the RGBCTRL register as the > DOTCLK/PCLK edge used to sample the data lines: > > â0â The data is input on the positive edge of DOTCLK > â1â The data is input on the negative edge of DOTCLK > > [ ... ] Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH v2 3/6] drm/panel: sitronix-st7789v: Use 9 bits per spi word by default
On Fri, 16 Jun 2023 18:32:52 +0200, Miquel Raynal wrote: > The Sitronix controller expects 9-bit words, provide this as default at > probe time rather than specifying this in each and every access. > > Signed-off-by: Miquel Raynal > Reviewed-by: Sam Ravnborg > > [ ... ] Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH v2 2/6] dt-bindings: display: st7789v: bound the number of Rx data lines
Hi, On Fri, Jun 16, 2023 at 06:32:51PM +0200, Miquel Raynal wrote: > The ST7789V LCD controller supports regular SPI wiring, as well as no Rx > data line at all. The operating system needs to know whether it can read > registers from the device or not. Let's detail this specific design > possibility by bounding the spi-rx-bus-width property. > > Signed-off-by: Miquel Raynal > --- > .../devicetree/bindings/display/panel/sitronix,st7789v.yaml | 4 > 1 file changed, 4 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml > b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml > index 0ccf0487fd8e..a25df7e1df88 100644 > --- a/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml > +++ b/Documentation/devicetree/bindings/display/panel/sitronix,st7789v.yaml > @@ -29,6 +29,10 @@ properties: >spi-cpha: true >spi-cpol: true > > + spi-rx-bus-width: > +minimum: 0 > +maximum: 1 > + It's not clear to me what the default would be? Maxime signature.asc Description: PGP signature
Re: [PATCH v2 1/6] dt-bindings: display: st7789v: Add the edt,et028013dma panel compatible
On Fri, 16 Jun 2023 18:32:50 +0200, Miquel Raynal wrote: > The ST7789V LCD controller is also embedded in the ET028013DMA > panel. Add a compatible string to describe this other panel. > > Signed-off-by: Miquel Raynal Acked-by: Maxime Ripard Thanks! Maxime
Re: [PATCH v7 2/8] PCI/VGA: Deal only with VGA class devices
Hi, On 2023/6/16 22:34, Alex Deucher wrote: On Fri, Jun 16, 2023 at 10:22 AM Sui Jingfeng wrote: On 2023/6/16 21:41, Alex Deucher wrote: On Fri, Jun 16, 2023 at 3:11 AM Sui Jingfeng wrote: Hi, On 2023/6/16 05:11, Alex Deucher wrote: On Wed, Jun 14, 2023 at 6:50 AM Sui Jingfeng wrote: Hi, On 2023/6/13 11:01, Sui Jingfeng wrote: From: Sui Jingfeng Deal only with the VGA devcie(pdev->class == 0x0300), so replace the pci_get_subsys() function with pci_get_class(). Filter the non-PCI display device(pdev->class != 0x0300) out. There no need to process the non-display PCI device. Cc: Bjorn Helgaas Signed-off-by: Sui Jingfeng --- drivers/pci/vgaarb.c | 22 -- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c index c1bc6c983932..22a505e877dc 100644 --- a/drivers/pci/vgaarb.c +++ b/drivers/pci/vgaarb.c @@ -754,10 +754,6 @@ static bool vga_arbiter_add_pci_device(struct pci_dev *pdev) struct pci_dev *bridge; u16 cmd; - /* Only deal with VGA class devices */ - if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA) - return false; - Hi, here is probably a bug fixing. For an example, nvidia render only GPU typically has 0x0380. as its PCI class number, but render only GPU should not participate in the arbitration. As it shouldn't snoop the legacy fixed VGA address. It(render only GPU) can not display anything. But 0x0380 >> 8 = 0x03, the filter failed. /* Allocate structure */ vgadev = kzalloc(sizeof(struct vga_device), GFP_KERNEL); if (vgadev == NULL) { @@ -1500,7 +1496,9 @@ static int pci_notify(struct notifier_block *nb, unsigned long action, struct pci_dev *pdev = to_pci_dev(dev); bool notify = false; - vgaarb_dbg(dev, "%s\n", __func__); + /* Only deal with VGA class devices */ + if (pdev->class != PCI_CLASS_DISPLAY_VGA << 8) + return 0; So here we only care 0x0300, my initial intent is to make an optimization, nowadays sane display graphic card should all has 0x0300 as its PCI class number, is this complete right? ``` #define PCI_BASE_CLASS_DISPLAY0x03 #define PCI_CLASS_DISPLAY_VGA0x0300 #define PCI_CLASS_DISPLAY_XGA0x0301 #define PCI_CLASS_DISPLAY_3D0x0302 #define PCI_CLASS_DISPLAY_OTHER0x0380 ``` Any ideas ? I'm not quite sure what you are asking about here. To be honest, I'm worried about the PCI devices which has a PCI_CLASS_DISPLAY_XGA as its PCI class number. As those devices are very uncommon in the real world. $ find . -name "*.c" -type f | xargs grep "PCI_CLASS_DISPLAY_XGA" Grep the "PCI_CLASS_DISPLAY_XGA" in the linux kernel tree got ZERO, there no code reference this macro. So I think it seems safe to ignore the XGA ? PCI_CLASS_DISPLAY_3D and PCI_CLASS_DISPLAY_OTHER are used to annotate the render-only GPU. And render-only GPU can't decode the fixed VGA address space, it is safe to ignore them. For vga_arb, we only care about VGA class devices since those should be on the only ones that might have VGA routed to them. However, as VGA gets deprecated, We need the vgaarb for a system with multiple video card. Not only because some Legacy VGA devices implemented on PCI will typically have the same "hard-decoded" addresses; But also these video card need to participate in the arbitration, determine the default boot device. But couldn't the boot device be determined via what whatever resources were used by the pre-OS console? I don't know what you are refer to by saying pre-OS console, UEFI SHELL, UEFI GOP or something like that. Right. Before the OS loads the platform firmware generally sets up something for display. That could be GOP or vesa or some other platform specific protocol. If you are referring to the framebuffer driver which light up the screen before the Linux kernel is loaded . Then, what you have said is true, the boot device is determined by the pre-OS console. But the problem is how does the Linux kernel(vgaarb) could know which one is the default boot device on a multiple GPU machine. Relaying on the firmware fb's address and size is what the mechanism we already in using. Right. It shouldn't need to depend on vgaarb. I feel like that should be separate from vgaarb. Emm, this really deserved another patch, please ? vgaarb should handle PCI VGA routing and some other mechanism should be used to determine what device provided the pre-OS console. If the new mechanism need the firmware changed, then this probably break the old machine. Also, this probably will get all arch involved. to get the new mechanism supported. The testing pressure and review power needed is quite large. drm/amdgpu and drm/radeon already being used on X86, ARM64, Mips and more arch... The reviewing process will became quite difficult then. vgaarb is really what we already in use, and being
[PATCH v2 3/3] drm/amd/display: Implement the retrieval of checksum_region's CRC data
Retrieve and store checksum_region's CRC data from the DC hardware in vline0 irq handler. A new function amdgpu_dm_crtc_update_checksum_region_crc() is implemented and hooked to CRTC callback for updating the latest CRC values to the checksum CRC blob. Signed-off-by: Alan Liu --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++- .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 55 +++ .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h | 11 .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 47 4 files changed, 110 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 26da07a25085..9fd08281fe27 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -8877,7 +8877,12 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) (struct drm_checksum_region *)new_crtc_state->checksum_region.region_blob->data; if (region_data->checksum_region_enable) { + struct secure_display_context *secure_display_ctx = + >secure_display_ctxs[acrtc->crtc_id]; + if (!amdgpu_dm_crc_window_is_activated(crtc)) { + init_completion(_display_ctx->crc.completion); + /* Enable secure display: set crc source to "crtc" */ amdgpu_dm_crtc_set_secure_display_crc_source(crtc, "crtc"); @@ -8887,7 +8892,8 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) spin_lock_irqsave(_to_drm(adev)->event_lock, flags); acrtc->dm_irq_params.window_param.activated = true; spin_unlock_irqrestore(_to_drm(adev)->event_lock, flags); - } + } else + reinit_completion(_display_ctx->crc.completion); /* Update ROI: copy ROI from crtc_state to dm_irq_params */ spin_lock_irqsave(_to_drm(adev)->event_lock, flags); diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c index 26017e9fbc4a..f881ccd93a25 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c @@ -529,6 +529,8 @@ void amdgpu_dm_crtc_handle_crc_window_irq(struct drm_crtc *crtc) struct amdgpu_crtc *acrtc = NULL; struct amdgpu_device *adev = NULL; struct secure_display_context *secure_display_ctx = NULL; + bool reset_crc_frame_count = false, crc_is_updated = false; + uint32_t crc[3] = {0}; unsigned long flags1; if (crtc == NULL) @@ -543,15 +545,14 @@ void amdgpu_dm_crtc_handle_crc_window_irq(struct drm_crtc *crtc) /* Early return if CRC capture is not enabled. */ if (!amdgpu_dm_is_valid_crc_source(cur_crc_src) || - !dm_is_crc_source_crtc(cur_crc_src)) - goto cleanup; - - if (!acrtc->dm_irq_params.window_param.activated) - goto cleanup; + !dm_is_crc_source_crtc(cur_crc_src)) { + spin_unlock_irqrestore(_dev->event_lock, flags1); + return; + } - if (acrtc->dm_irq_params.window_param.skip_frame_cnt) { - acrtc->dm_irq_params.window_param.skip_frame_cnt -= 1; - goto cleanup; + if (!acrtc->dm_irq_params.window_param.activated) { + spin_unlock_irqrestore(_dev->event_lock, flags1); + return; } secure_display_ctx = >dm.secure_display_ctxs[acrtc->crtc_id]; @@ -562,16 +563,23 @@ void amdgpu_dm_crtc_handle_crc_window_irq(struct drm_crtc *crtc) secure_display_ctx->crtc = crtc; } + if (acrtc->dm_irq_params.window_param.skip_frame_cnt) { + acrtc->dm_irq_params.window_param.skip_frame_cnt -= 1; + goto cleanup; + } + if (acrtc->dm_irq_params.window_param.update_win) { /* prepare work for dmub to update ROI */ secure_display_ctx->rect.x = acrtc->dm_irq_params.window_param.x_start; secure_display_ctx->rect.y = acrtc->dm_irq_params.window_param.y_start; secure_display_ctx->rect.width = acrtc->dm_irq_params.window_param.x_end - - acrtc->dm_irq_params.window_param.x_start; + acrtc->dm_irq_params.window_param.x_start; secure_display_ctx->rect.height =
[PATCH v2 2/3] drm/amd/display: Create checksum_region properties and handle new region update
This commit creates checksum_region properties at CRTC initialization, and update the new region during the atomic atomic. A new function amdgpu_dm_crtc_set_secure_display_crc_source() is implemented to control the state of CRC engine of DC hardware. Signed-off-by: Alan Liu --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 38 + .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c | 57 +++ .../drm/amd/display/amdgpu_dm/amdgpu_dm_crc.h | 3 + .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c| 5 ++ 4 files changed, 103 insertions(+) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 90de0d37f1d2..26da07a25085 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -8870,6 +8870,44 @@ static void amdgpu_dm_atomic_commit_tail(struct drm_atomic_state *state) } } #endif + +#ifdef CONFIG_DRM_AMD_SECURE_DISPLAY + if (new_crtc_state->active && new_crtc_state->checksum_region.region_changed) { + struct drm_checksum_region *region_data = + (struct drm_checksum_region *)new_crtc_state->checksum_region.region_blob->data; + + if (region_data->checksum_region_enable) { + if (!amdgpu_dm_crc_window_is_activated(crtc)) { + /* Enable secure display: set crc source to "crtc" */ + amdgpu_dm_crtc_set_secure_display_crc_source(crtc, "crtc"); + + /* wait 1 more frame for CRC engine to start */ + acrtc->dm_irq_params.window_param.skip_frame_cnt = 1; + + spin_lock_irqsave(_to_drm(adev)->event_lock, flags); + acrtc->dm_irq_params.window_param.activated = true; + spin_unlock_irqrestore(_to_drm(adev)->event_lock, flags); + } + + /* Update ROI: copy ROI from crtc_state to dm_irq_params */ + spin_lock_irqsave(_to_drm(adev)->event_lock, flags); + acrtc->dm_irq_params.window_param.x_start = region_data->x_start; + acrtc->dm_irq_params.window_param.y_start = region_data->y_start; + acrtc->dm_irq_params.window_param.x_end = region_data->x_end; + acrtc->dm_irq_params.window_param.y_end = region_data->y_end; + acrtc->dm_irq_params.window_param.update_win = true; + spin_unlock_irqrestore(_to_drm(adev)->event_lock, flags); + + } else { + if (amdgpu_dm_crc_window_is_activated(crtc)) { + /* Disable secure display: set crc source to "none" */ + amdgpu_dm_crtc_set_secure_display_crc_source(crtc, "none"); + } + } + + new_crtc_state->checksum_region.region_changed = false; + } +#endif } for_each_new_crtc_in_state(state, crtc, new_crtc_state, j) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c index 0802f8e8fac5..26017e9fbc4a 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crc.c @@ -465,6 +465,63 @@ void amdgpu_dm_crtc_handle_crc_irq(struct drm_crtc *crtc) } #if defined(CONFIG_DRM_AMD_SECURE_DISPLAY) +int amdgpu_dm_crtc_set_secure_display_crc_source(struct drm_crtc *crtc, const char *src_name) +{ + enum amdgpu_dm_pipe_crc_source source = dm_parse_crc_source(src_name); + enum amdgpu_dm_pipe_crc_source cur_crc_src; + struct dm_crtc_state *crtc_state; + struct drm_device *drm_dev = crtc->dev; + struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc); + bool enable = false; + bool enabled = false; + int ret = 0; + unsigned long flag; + + if (source < 0) { + DRM_DEBUG_DRIVER("Unknown CRC source %s for CRTC%d\n", +src_name, crtc->index); + return -EINVAL; + } + + enable = amdgpu_dm_is_valid_crc_source(source); + crtc_state = to_dm_crtc_state(crtc->state); + spin_lock_irqsave(_dev->event_lock, flag); + cur_crc_src = acrtc->dm_irq_params.crc_src; + spin_unlock_irqrestore(_dev->event_lock, flag); + + /* Reset secure_display when we change crc source */ + amdgpu_dm_set_crc_window_default(crtc, crtc_state->stream); + + if
[PATCH v2 0/3] Checksum Region with new CRTC properties
Dear DRM development community, We'd like to introduce the implementation of the new crtc properties. First of all, please let me introduce the problem we try to address. With the popularity of electric vehicles, the car vendors have increasing requirement for ensuring the integrity of the critical content on the display. For example, tell-tales are icons to indicate malfunction or operation on a car system. For safty concern, car vendors always want to make sure these icons are not tampered and can be correctly displayed on the instrument cluster. To address this problem, since modern display control hardware is able to calculate the CRC checksum of the display content, we are thinking of a feature to let userspace specify a region on display, and we can utilize the hardware to calculate the CRC checksum as frames scanned out, and finally, provide the checksum for userspace for validation purpose. In this case, since the icons themselves are often fixed over static backgrounds, the CRC of the pixels in the region can be known in advance. So one of the usage of the region and corresponding CRC result is that as users know the CRC checksum of the tell-tales in advance, at runtime they can retrieve the CRC value from kernel for validation as frames are scanned out. We implement this feature and call it checksum region. To let userspace set a region and retrieve the corresponding CRC value, we provide 2 new properties, CHECKSUM_REGION and CHECKSUM_CRC. Both of them are blob properties under CRTC, and the detailed struct of the two properties are listed below. One for userspace to set the region to kernel, and the other for userspace to retrieve CRC values from kernel. Upon userspace submitting the 4 coordinate values with checksum_region_enable true, kernel instructs DC hardware to calculate the CRC value accordingly as frames scanned out. The result CRC value of RGB colors are then stored in CHECKSUM_CRC property, with a reference frame count for userspace to know which frame the CRCs are calculated at. /** * struct drm_checksum_region - The enablement and region of checksum_region * @x_start: Horizontal starting coordinate of the region. * @y_start: Vertical starting coordinate of the region. * @x_end: Horizontal ending coordinate of the region. * @y_end: Vertical ending coordinate of the region. * @checksum_region_enable: To enable or disable checksum_region. * * Userspace uses this structure to configure the region and enablement for * checksum_region. Userspace should not submit a region out of the displayable * region because there is nothing to display and need protection. */ struct drm_checksum_region { __u32 x_start; __u32 y_start; __u32 x_end; __u32 y_end; __u8 checksum_region_enable; __u8 pad[7]; }; /** * struct drm_checksum_crc - The CRC value of the corresponding checksum region. * @crc_r: CRC value of red color. * @crc_g: CRC value of green color. * @crc_b: CRC value of blue color. * @frame_count: a referenced frame count to indicate which frame the CRC values * are generated at. * * Userspace uses this structure to retrieve the CRC values of the current * checksum region. @frame_count will be reset once a new region is updated or * it reaches a maximum value. Currently these CRC values are designed to * be validated with pre-saved CRC values, so userspace doesn't need to concern * about the algorithm used to compute the CRC. */ struct drm_checksum_crc { __u32 crc_r; __u32 crc_g; __u32 crc_b; __u32 frame_count; }; To better introduce the usage of this feature, we also have a paired Weston application as an reference app to use secure display via the properties. Please check the Weston application and see how the application set the region and validate the content from the CRC here: https://gitlab.freedesktop.org/wayland/weston/-/merge_requests/1236 This application can draw patterns on the display, and allow users to set the region and submit it to kernel via properties. With kernel keeping calculating the CRC, this example application takes the first CRC as source CRC, and keeps retrieving the new CRCs at each frame later. By comparing source CRC with the following CRC value, the application can validate if the display content got changed down the road. Finally, let me briefly introduce the patches. There are 3 patches in this patch series. The first patch is the main patch that contains change to drm, including the new CRTC properties, the property creation function and a update_checksum_region_crc() CRTC callback. 1. drm: Introduce CRTC checksum region and CRC properties The remaining 2 patches are only related to the processing of region and CRC data in our driver, also listed here for your reference. 2. drm/amd/display: Create checksum_region properties and handle new region update 3. drm/amd/display: Implement the
[PATCH v2 1/3] drm: Introduce CRTC checksum region and CRC properties
Introduce per-CRTC properties: CHECKSUM_REGION and CHECKSUM_CRC. Userspace can configure a region by setting the region property and retrieve the CRC values from the CRC property to validate the content of the region. Apon userspace submits the 4 coordinate values with checksum_region_enable true, kernel instructs DC hardware to calculate the CRC value accordingly as frames scanned out. The result CRC value of RGB colors are then stored in CHECKSUM_CRC property, with a reference frame count for userspace to know which frame the CRCs are calculated at. Driver can set up these properties for a CRTC by calling drm_crtc_create_checksum_region_properties() and hook its own implementation on new CRTC function update_chechsum_region_crc() to update the values of the CRC property for the incoming userspace request. Signed-off-by: Alan Liu --- drivers/gpu/drm/drm_atomic_state_helper.c | 7 drivers/gpu/drm/drm_atomic_uapi.c | 21 ++- drivers/gpu/drm/drm_crtc.c| 44 +++ include/drm/drm_crtc.h| 43 ++ include/uapi/drm/drm_mode.h | 42 ++ 5 files changed, 156 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_atomic_state_helper.c b/drivers/gpu/drm/drm_atomic_state_helper.c index dfb57217253b..a8f25575edef 100644 --- a/drivers/gpu/drm/drm_atomic_state_helper.c +++ b/drivers/gpu/drm/drm_atomic_state_helper.c @@ -143,6 +143,11 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc, drm_property_blob_get(state->ctm); if (state->gamma_lut) drm_property_blob_get(state->gamma_lut); + if (state->checksum_region.region_blob) + drm_property_blob_get(state->checksum_region.region_blob); + if (state->checksum_region.crc_blob) + drm_property_blob_get(state->checksum_region.crc_blob); + state->mode_changed = false; state->active_changed = false; state->planes_changed = false; @@ -215,6 +220,8 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state) drm_property_blob_put(state->degamma_lut); drm_property_blob_put(state->ctm); drm_property_blob_put(state->gamma_lut); + drm_property_blob_put(state->checksum_region.region_blob); + drm_property_blob_put(state->checksum_region.crc_blob); } EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state); diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index c06d0639d552..5a934f191940 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -450,6 +450,17 @@ static int drm_atomic_crtc_set_property(struct drm_crtc *crtc, set_out_fence_for_crtc(state->state, crtc, fence_ptr); } else if (property == crtc->scaling_filter_property) { state->scaling_filter = val; + } else if (property == crtc->checksum_region_property) { + ret = drm_atomic_replace_property_blob_from_id(dev, + >checksum_region.region_blob, + val, + -1, sizeof(struct drm_checksum_region), + ); + state->checksum_region.region_changed |= replaced; + return ret; + } else if (property == crtc->checksum_crc_property) { + /* don't let user set CRC data */ + return -EPERM; } else if (crtc->funcs->atomic_set_property) { return crtc->funcs->atomic_set_property(crtc, state, property, val); } else { @@ -487,7 +498,15 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc, *val = 0; else if (property == crtc->scaling_filter_property) *val = state->scaling_filter; - else if (crtc->funcs->atomic_get_property) + else if (property == crtc->checksum_region_property) + *val = (state->checksum_region.region_blob) + ? state->checksum_region.region_blob->base.id : 0; + else if (property == crtc->checksum_crc_property) { + if (crtc->funcs->update_checksum_region_crc) + crtc->funcs->update_checksum_region_crc(crtc); + *val = (state->checksum_region.crc_blob) + ? state->checksum_region.crc_blob->base.id : 0; + } else if (crtc->funcs->atomic_get_property) return crtc->funcs->atomic_get_property(crtc, state, property, val); else return -EINVAL; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index df9bf3c9206e..07186cb8bfd4 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -955,3 +955,47 @@ int drm_crtc_create_scaling_filter_property(struct drm_crtc *crtc, return 0; }
Re: [13/14] drm/ast: Move widescreen- and tx-chip detection into separate helpers
Hi, "widescreen-" -> "widescreen" On 2023/6/16 21:52, Thomas Zimmermann wrote: Split ast_detect_chip() into three functions and call them one by one. The new functions detect the transmitter chip and widescreen support. This will allow for further refactoring. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/ast/ast_main.c | 18 +++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 7f8fb9a613604..f028b5b47c56e 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -157,7 +157,6 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev) { struct ast_device *ast = to_ast_device(dev); struct pci_dev *pdev = to_pci_dev(dev->dev); - uint32_t jreg; /* Identify chipset */ if (pdev->revision >= 0x50) { @@ -218,6 +217,13 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev) drm_info(dev, "AST 2000 detected\n"); } + return 0; +} + +static void ast_detect_widescreen(struct ast_device *ast) +{ + u8 jreg; + /* Check if we support wide screen */ switch (AST_GEN(ast)) { case 1: @@ -242,6 +248,12 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev) } break; } +} + +static void ast_detect_tx_chip(struct ast_device *ast, bool need_post) +{ + struct drm_device *dev = >base; + u8 jreg; /* Check 3rd Tx option (digital output afaik) */ ast->tx_chip_types |= AST_TX_NONE_BIT; @@ -301,8 +313,6 @@ static int ast_detect_chip(struct drm_device *dev, bool need_post, u32 scu_rev) drm_info(dev, "Using DP501 DisplayPort transmitter\n"); if (ast->tx_chip_types & AST_TX_ASTDP_BIT) drm_info(dev, "Using ASPEED DisplayPort transmitter\n"); - - return 0; } static int ast_get_dram_info(struct drm_device *dev) @@ -494,6 +504,8 @@ struct ast_device *ast_device_create(const struct drm_driver *drv, ast_detect_config_mode(dev, _rev); ast_detect_chip(dev, need_post, scu_rev); + ast_detect_widescreen(ast); + ast_detect_tx_chip(ast, need_post); ret = ast_get_dram_info(dev); if (ret) -- Jingfeng
[PATCH v2 2/2] ARM: tegra: transformers: add connector node
All ASUS Transformers have micro-HDMI connector directly available. After Tegra HDMI got bridge/connector support, we should use connector framework for proper HW description. Tested-by: Andreas Westman Dorcsak # ASUS TF T30 Tested-by: Robert Eckelmann # ASUS TF101 T20 Tested-by: Svyatoslav Ryhel # ASUS TF201 T30 Signed-off-by: Svyatoslav Ryhel --- arch/arm/boot/dts/tegra20-asus-tf101.dts | 22 --- .../dts/tegra30-asus-transformer-common.dtsi | 21 -- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/arch/arm/boot/dts/tegra20-asus-tf101.dts b/arch/arm/boot/dts/tegra20-asus-tf101.dts index c2a9c3fb5b33..97350f566539 100644 --- a/arch/arm/boot/dts/tegra20-asus-tf101.dts +++ b/arch/arm/boot/dts/tegra20-asus-tf101.dts @@ -82,9 +82,11 @@ hdmi@5428 { pll-supply = <_pll_reg>; hdmi-supply = <_hdmi_en>; - nvidia,ddc-i2c-bus = <_ddc>; - nvidia,hpd-gpio = < TEGRA_GPIO(N, 7) - GPIO_ACTIVE_HIGH>; + port@0 { + hdmi_out: endpoint { + remote-endpoint = <_in>; + }; + }; }; }; @@ -963,6 +965,20 @@ clk32k_in: clock-32k-in { #clock-cells = <0>; }; + connector { + compatible = "hdmi-connector"; + type = "d"; + + hpd-gpios = < TEGRA_GPIO(N, 7) GPIO_ACTIVE_HIGH>; + ddc-i2c-bus = <_ddc>; + + port { + connector_in: endpoint { + remote-endpoint = <_out>; + }; + }; + }; + cpus { cpu0: cpu@0 { cpu-supply = <_cpu>; diff --git a/arch/arm/boot/dts/tegra30-asus-transformer-common.dtsi b/arch/arm/boot/dts/tegra30-asus-transformer-common.dtsi index bdb898ad6262..153d34a012bd 100644 --- a/arch/arm/boot/dts/tegra30-asus-transformer-common.dtsi +++ b/arch/arm/boot/dts/tegra30-asus-transformer-common.dtsi @@ -80,8 +80,11 @@ hdmi: hdmi@5428 { pll-supply = <_1v8_vio>; vdd-supply = <_3v3_sys>; - nvidia,hpd-gpio = < TEGRA_GPIO(N, 7) GPIO_ACTIVE_HIGH>; - nvidia,ddc-i2c-bus = <_ddc>; + port@0 { + hdmi_out: endpoint { + remote-endpoint = <_in>; + }; + }; }; }; @@ -1492,6 +1495,20 @@ clk32k_in: clock-32k { clock-output-names = "pmic-oscillator"; }; + connector { + compatible = "hdmi-connector"; + type = "d"; + + hpd-gpios = < TEGRA_GPIO(N, 7) GPIO_ACTIVE_HIGH>; + ddc-i2c-bus = <_ddc>; + + port { + connector_in: endpoint { + remote-endpoint = <_out>; + }; + }; + }; + cpus { cpu0: cpu@0 { cpu-supply = <_cpu>; -- 2.39.2
[PATCH v2 1/2] drm/tegra: output: hdmi: Support bridge/connector
From: Maxim Schwalm Some Tegra device-trees may specify a video output graph, which involves MHL bridge/simple bridge and/or connector framework. This patch adds support for the bridge/connector attached to the HDMI output, allowing us to model the hardware properly. Inspired by: 29efdc2 ("drm/tegra: output: rgb: Support LVDS encoder bridge") Tested-by: Andreas Westman Dorcsak # ASUS TF T30 Tested-by: Maxim Schwalm # ASUS P1801-T T30 Tested-by: Robert Eckelmann # ASUS TF101 T20 Tested-by: Svyatoslav Ryhel # ASUS TF201 T30 Signed-off-by: Maxim Schwalm Signed-off-by: Svyatoslav Ryhel --- drivers/gpu/drm/tegra/hdmi.c | 44 +++- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c index 6eac54ae1205..a5b12b169e57 100644 --- a/drivers/gpu/drm/tegra/hdmi.c +++ b/drivers/gpu/drm/tegra/hdmi.c @@ -19,6 +19,7 @@ #include #include +#include #include #include #include @@ -1544,26 +1545,47 @@ static int tegra_hdmi_init(struct host1x_client *client) { struct tegra_hdmi *hdmi = host1x_client_to_hdmi(client); struct drm_device *drm = dev_get_drvdata(client->host); + struct drm_connector *connector; int err; hdmi->output.dev = client->dev; - drm_connector_init_with_ddc(drm, >output.connector, - _hdmi_connector_funcs, - DRM_MODE_CONNECTOR_HDMIA, - hdmi->output.ddc); - drm_connector_helper_add(>output.connector, -_hdmi_connector_helper_funcs); - hdmi->output.connector.dpms = DRM_MODE_DPMS_OFF; - drm_simple_encoder_init(drm, >output.encoder, DRM_MODE_ENCODER_TMDS); drm_encoder_helper_add(>output.encoder, _hdmi_encoder_helper_funcs); - drm_connector_attach_encoder(>output.connector, - >output.encoder); - drm_connector_register(>output.connector); + if (hdmi->output.bridge) { + err = drm_bridge_attach(>output.encoder, hdmi->output.bridge, + NULL, DRM_BRIDGE_ATTACH_NO_CONNECTOR); + if (err) { + dev_err(client->dev, "failed to attach bridge: %d\n", + err); + return err; + } + + connector = drm_bridge_connector_init(drm, >output.encoder); + if (IS_ERR(connector)) { + dev_err(client->dev, + "failed to initialize bridge connector: %pe\n", + connector); + return PTR_ERR(connector); + } + + drm_connector_attach_encoder(connector, >output.encoder); + } else { + drm_connector_init_with_ddc(drm, >output.connector, + _hdmi_connector_funcs, + DRM_MODE_CONNECTOR_HDMIA, + hdmi->output.ddc); + drm_connector_helper_add(>output.connector, +_hdmi_connector_helper_funcs); + hdmi->output.connector.dpms = DRM_MODE_DPMS_OFF; + + drm_connector_attach_encoder(>output.connector, +>output.encoder); + drm_connector_register(>output.connector); + } err = tegra_output_init(drm, >output); if (err < 0) { -- 2.39.2
[PATCH v2 0/2] Support bridge/connector by Tegra HDMI
This patch adds support for the bridge/connector attached to the HDMI output, allowing to model the hardware properly. It keeps backwards compatibility with existing bindings and is required by devices which have a simple or MHL bridge connected to HDMI output like ASUS P1801-T or LG P880/P895 or HTC One X. Tested on ASUS Transformers which have no dedicated bridge but have type d HDMI connector directly available. Tests went smoothly. --- Chandes from v1: - no changes, re-sending --- Maxim Schwalm (1): drm/tegra: output: hdmi: Support bridge/connector Svyatoslav Ryhel (1): ARM: tegra: transformers: add connector node arch/arm/boot/dts/tegra20-asus-tf101.dts | 22 -- .../dts/tegra30-asus-transformer-common.dtsi | 21 - drivers/gpu/drm/tegra/hdmi.c | 44 ++- 3 files changed, 71 insertions(+), 16 deletions(-) -- 2.39.2
Re: [PATCH v2 3/4] drm/mediatek: Add casting before assign
Hi Angelo, Thanks for the reviews. On Wed, 2023-06-14 at 10:43 +0200, AngeloGioacchino Del Regno wrote: External email : Please do not click links or open attachments until you have verified the sender or the content. Il 13/06/23 13:32, Jason-JH.Lin ha scritto: > 1. Add casting before assign to avoid the unintentional integer > overflow or unintended sign extension. > 2. Add a int varriable for multiplier calculation instead of calculating > different types multiplier with dma_addr_t varriable directly. > > Signed-off-by: Jason-JH.Lin > Fixes: 1a64a7aff8da ("drm/mediatek: Fix cursor plane no update") > --- > drivers/gpu/drm/mediatek/mtk_drm_gem.c | 2 +- > drivers/gpu/drm/mediatek/mtk_drm_plane.c | 22 +- > 2 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > index a25b28d3ee90..0c7878bc0b37 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_gem.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_gem.c > @@ -121,7 +121,7 @@ int mtk_drm_gem_dumb_create(struct drm_file *file_priv, > struct drm_device *dev, > int ret; > > args->pitch = DIV_ROUND_UP(args->width * args->bpp, 8); > -args->size = args->pitch * args->height; > +args->size = (__u64)args->pitch * args->height; We could avoid explicit casting here if you do args->size = args->pitch; args->size *= args->height; OK, Thanks for your advice. I'll take this. > > mtk_gem = mtk_drm_gem_create(dev, args->size, false); > if (IS_ERR(mtk_gem)) > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > index 31f9420aff6f..1cd41454d545 100644 > --- a/drivers/gpu/drm/mediatek/mtk_drm_plane.c > +++ b/drivers/gpu/drm/mediatek/mtk_drm_plane.c > @@ -145,6 +145,7 @@ static void mtk_plane_update_new_state(struct > drm_plane_state *new_state, > dma_addr_t addr; > dma_addr_t hdr_addr = 0; > unsigned int hdr_pitch = 0; > +int offset; ...but offset can never be negative, can it? in that case, this should be unsigned int. src.x1 and src.y1 are 'int', so they can be negative. But I'm not sure does anyone would set the negative to them. I think using 'unsigned int' for offset would be very big after multiply with negative src.x1 or src.y1. So I just use 'int' here to avoid that case. Do you think I should use 'unsigned int' for offset and add the boundary checking for addr? Regard, Jason-JH.Lin > > gem = fb->obj[0]; > mtk_gem = to_mtk_gem_obj(gem); > @@ -154,8 +155,10 @@ static void mtk_plane_update_new_state(struct > drm_plane_state *new_state, > modifier = fb->modifier; > > if (modifier == DRM_FORMAT_MOD_LINEAR) { > -addr += (new_state->src.x1 >> 16) * fb->format->cpp[0]; > -addr += (new_state->src.y1 >> 16) * pitch; > +offset = (new_state->src.x1 >> 16) * fb->format->cpp[0]; > +addr += offset; > +offset = (new_state->src.y1 >> 16) * pitch; > +addr += offset; > } else { > int width_in_blocks = ALIGN(fb->width, AFBC_DATA_BLOCK_WIDTH) > / AFBC_DATA_BLOCK_WIDTH; > @@ -163,21 +166,22 @@ static void mtk_plane_update_new_state(struct > drm_plane_state *new_state, > / AFBC_DATA_BLOCK_HEIGHT; > int x_offset_in_blocks = (new_state->src.x1 >> 16) / AFBC_DATA_BLOCK_WIDTH; > int y_offset_in_blocks = (new_state->src.y1 >> 16) / AFBC_DATA_BLOCK_HEIGHT; > -int hdr_size; > +int hdr_size, hdr_offset; > > hdr_pitch = width_in_blocks * AFBC_HEADER_BLOCK_SIZE; > pitch = width_in_blocks * AFBC_DATA_BLOCK_WIDTH * > AFBC_DATA_BLOCK_HEIGHT * fb->format->cpp[0]; > > hdr_size = ALIGN(hdr_pitch * height_in_blocks, AFBC_HEADER_ALIGNMENT); > +hdr_offset = hdr_pitch * y_offset_in_blocks + > +AFBC_HEADER_BLOCK_SIZE * x_offset_in_blocks; > +hdr_addr = addr + hdr_offset; > > -hdr_addr = addr + hdr_pitch * y_offset_in_blocks + > - AFBC_HEADER_BLOCK_SIZE * x_offset_in_blocks; > /* The data plane is offset by 1 additional block. */ > -addr = addr + hdr_size + > - pitch * y_offset_in_blocks + > - AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT * > - fb->format->cpp[0] * (x_offset_in_blocks + 1); > +offset = pitch * y_offset_in_blocks + > + AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT * > + fb->format->cpp[0] * (x_offset_in_blocks + 1); > +addr = addr + hdr_size + offset; > } > > mtk_plane_state->pending.enable = true;
Re: [Intel-xe] [RFC PATCH 1/1] drm/xe: Introduce function pointers for MMIO functions
On Thu, Jun 15, 2023 at 7:34 PM Matt Roper wrote: > > On Thu, Jun 15, 2023 at 04:04:18PM +0300, Oded Gabbay wrote: > > On Thu, Jun 15, 2023 at 3:01 AM Matt Roper > > wrote: > > > > > > On Mon, Jun 12, 2023 at 06:31:57PM +0200, Francois Dugast wrote: > > > > On Thu, Jun 08, 2023 at 10:35:29AM -0700, Lucas De Marchi wrote: > > > > > On Fri, Jun 02, 2023 at 02:25:01PM +, Francois Dugast wrote: > > > > > > A local structure of function pointers is used as a minimal hardware > > > > > > abstraction layer to prepare for platform independent MMIO calls. > > > > > > > > > > > > Cc: Oded Gabbay > > > > > > Cc: Ofir Bitton > > > > > > Cc: Ohad Sharabi > > > > > > Signed-off-by: Francois Dugast > > > > > > --- > > > > > > drivers/gpu/drm/xe/xe_device_types.h | 3 ++ > > > > > > drivers/gpu/drm/xe/xe_mmio.c | 81 > > > > > > > > > > > > drivers/gpu/drm/xe/xe_mmio.h | 35 ++-- > > > > > > 3 files changed, 99 insertions(+), 20 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_device_types.h > > > > > > b/drivers/gpu/drm/xe/xe_device_types.h > > > > > > index 17b6b1cc5adb..3f8fd0d8129b 100644 > > > > > > --- a/drivers/gpu/drm/xe/xe_device_types.h > > > > > > +++ b/drivers/gpu/drm/xe/xe_device_types.h > > > > > > @@ -378,6 +378,9 @@ struct xe_device { > > > > > > /** @d3cold_allowed: Indicates if d3cold is a valid device state > > > > > > */ > > > > > > bool d3cold_allowed; > > > > > > > > > > > > + /** @mmio_funcs: function pointers for MMIO related functions */ > > > > > > + const struct xe_mmio_funcs *mmio_funcs; > > > > > > + > > > > > > /* private: */ > > > > > > > > > > > > #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY) > > > > > > diff --git a/drivers/gpu/drm/xe/xe_mmio.c > > > > > > b/drivers/gpu/drm/xe/xe_mmio.c > > > > > > index 475b14fe4356..f3d08676a77a 100644 > > > > > > --- a/drivers/gpu/drm/xe/xe_mmio.c > > > > > > +++ b/drivers/gpu/drm/xe/xe_mmio.c > > > > > > @@ -25,6 +25,62 @@ > > > > > > > > > > > > #define BAR_SIZE_SHIFT 20 > > > > > > > > > > > > +static void xe_mmio_write32_device(struct xe_gt *gt, > > > > > > +struct xe_reg reg, u32 val); > > > > > > +static u32 xe_mmio_read32_device(struct xe_gt *gt, struct xe_reg > > > > > > reg); > > > > > > +static void xe_mmio_write64_device(struct xe_gt *gt, > > > > > > +struct xe_reg reg, u64 val); > > > > > > +static u64 xe_mmio_read64_device(struct xe_gt *gt, struct xe_reg > > > > > > reg); > > > > > > + > > > > > > +static const struct xe_mmio_funcs xe_mmio_funcs_device = { > > > > > > + .write32 = xe_mmio_write32_device, > > > > > > + .read32 = xe_mmio_read32_device, > > > > > > + .write64 = xe_mmio_write64_device, > > > > > > + .read64 = xe_mmio_read64_device, > > > > > > +}; > > > > > > + > > > > > > +static inline void xe_mmio_write32_device(struct xe_gt *gt, > > > > > > +struct xe_reg reg, u32 val) > > > > > > +{ > > > > > > + struct xe_tile *tile = gt_to_tile(gt); > > > > > > + > > > > > > + if (reg.addr < gt->mmio.adj_limit) > > > > > > + reg.addr += gt->mmio.adj_offset; > > > > > > + > > > > > > + writel(val, tile->mmio.regs + reg.addr); > > > > > > +} > > > > > > + > > > > > > +static inline u32 xe_mmio_read32_device(struct xe_gt *gt, struct > > > > > > xe_reg reg) > > > > > > +{ > > > > > > + struct xe_tile *tile = gt_to_tile(gt); > > > > > > + > > > > > > + if (reg.addr < gt->mmio.adj_limit) > > > > > > + reg.addr += gt->mmio.adj_offset; > > > > > > + > > > > > > + return readl(tile->mmio.regs + reg.addr); > > > > > > +} > > > > > > + > > > > > > +static inline void xe_mmio_write64_device(struct xe_gt *gt, > > > > > > +struct xe_reg reg, u64 val) > > > > > > +{ > > > > > > + struct xe_tile *tile = gt_to_tile(gt); > > > > > > + > > > > > > + if (reg.addr < gt->mmio.adj_limit) > > > > > > + reg.addr += gt->mmio.adj_offset; > > > > > > + > > > > > > + writeq(val, tile->mmio.regs + reg.addr); > > > > > > +} > > > > > > + > > > > > > +static inline u64 xe_mmio_read64_device(struct xe_gt *gt, struct > > > > > > xe_reg reg) > > > > > > +{ > > > > > > + struct xe_tile *tile = gt_to_tile(gt); > > > > > > + > > > > > > + if (reg.addr < gt->mmio.adj_limit) > > > > > > + reg.addr += gt->mmio.adj_offset; > > > > > > + > > > > > > + return readq(tile->mmio.regs + reg.addr); > > > > > > +} > > > > > > + > > > > > > static int xe_set_dma_info(struct xe_device *xe) > > > > > > { > > > > > > unsigned int mask_size = xe->info.dma_mask_size; > > > > > > @@ -377,6 +433,29 @@ static void mmio_fini(struct drm_device *drm, > > > > > > void *arg) > > > > > > iounmap(xe->mem.vram.mapping); > > > > > > } > > > > > > > > > > > > +static void xe_mmio_set_funcs(struct xe_device *xe) > > > > > > +{ > > > > > > + /* For now all platforms use the set of MMIO functions for a > > > > > > + * physical device. > > > > > > + */ > > > > >
Re: [Intel-xe] [RFC PATCH 1/1] drm/xe: Introduce function pointers for MMIO functions
On Thu, Jun 15, 2023 at 7:34 PM Matt Roper wrote: > > On Thu, Jun 15, 2023 at 04:04:18PM +0300, Oded Gabbay wrote: > > On Thu, Jun 15, 2023 at 3:01 AM Matt Roper > > wrote: > > > > > > On Mon, Jun 12, 2023 at 06:31:57PM +0200, Francois Dugast wrote: > > > > On Thu, Jun 08, 2023 at 10:35:29AM -0700, Lucas De Marchi wrote: > > > > > On Fri, Jun 02, 2023 at 02:25:01PM +, Francois Dugast wrote: > > > > > > A local structure of function pointers is used as a minimal hardware > > > > > > abstraction layer to prepare for platform independent MMIO calls. > > > > > > > > > > > > Cc: Oded Gabbay > > > > > > Cc: Ofir Bitton > > > > > > Cc: Ohad Sharabi > > > > > > Signed-off-by: Francois Dugast > > > > > > --- > > > > > > drivers/gpu/drm/xe/xe_device_types.h | 3 ++ > > > > > > drivers/gpu/drm/xe/xe_mmio.c | 81 > > > > > > > > > > > > drivers/gpu/drm/xe/xe_mmio.h | 35 ++-- > > > > > > 3 files changed, 99 insertions(+), 20 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_device_types.h > > > > > > b/drivers/gpu/drm/xe/xe_device_types.h > > > > > > index 17b6b1cc5adb..3f8fd0d8129b 100644 > > > > > > --- a/drivers/gpu/drm/xe/xe_device_types.h > > > > > > +++ b/drivers/gpu/drm/xe/xe_device_types.h > > > > > > @@ -378,6 +378,9 @@ struct xe_device { > > > > > > /** @d3cold_allowed: Indicates if d3cold is a valid device state > > > > > > */ > > > > > > bool d3cold_allowed; > > > > > > > > > > > > + /** @mmio_funcs: function pointers for MMIO related functions */ > > > > > > + const struct xe_mmio_funcs *mmio_funcs; > > > > > > + > > > > > > /* private: */ > > > > > > > > > > > > #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY) > > > > > > diff --git a/drivers/gpu/drm/xe/xe_mmio.c > > > > > > b/drivers/gpu/drm/xe/xe_mmio.c > > > > > > index 475b14fe4356..f3d08676a77a 100644 > > > > > > --- a/drivers/gpu/drm/xe/xe_mmio.c > > > > > > +++ b/drivers/gpu/drm/xe/xe_mmio.c > > > > > > @@ -25,6 +25,62 @@ > > > > > > > > > > > > #define BAR_SIZE_SHIFT 20 > > > > > > > > > > > > +static void xe_mmio_write32_device(struct xe_gt *gt, > > > > > > +struct xe_reg reg, u32 val); > > > > > > +static u32 xe_mmio_read32_device(struct xe_gt *gt, struct xe_reg > > > > > > reg); > > > > > > +static void xe_mmio_write64_device(struct xe_gt *gt, > > > > > > +struct xe_reg reg, u64 val); > > > > > > +static u64 xe_mmio_read64_device(struct xe_gt *gt, struct xe_reg > > > > > > reg); > > > > > > + > > > > > > +static const struct xe_mmio_funcs xe_mmio_funcs_device = { > > > > > > + .write32 = xe_mmio_write32_device, > > > > > > + .read32 = xe_mmio_read32_device, > > > > > > + .write64 = xe_mmio_write64_device, > > > > > > + .read64 = xe_mmio_read64_device, > > > > > > +}; > > > > > > + > > > > > > +static inline void xe_mmio_write32_device(struct xe_gt *gt, > > > > > > +struct xe_reg reg, u32 val) > > > > > > +{ > > > > > > + struct xe_tile *tile = gt_to_tile(gt); > > > > > > + > > > > > > + if (reg.addr < gt->mmio.adj_limit) > > > > > > + reg.addr += gt->mmio.adj_offset; > > > > > > + > > > > > > + writel(val, tile->mmio.regs + reg.addr); > > > > > > +} > > > > > > + > > > > > > +static inline u32 xe_mmio_read32_device(struct xe_gt *gt, struct > > > > > > xe_reg reg) > > > > > > +{ > > > > > > + struct xe_tile *tile = gt_to_tile(gt); > > > > > > + > > > > > > + if (reg.addr < gt->mmio.adj_limit) > > > > > > + reg.addr += gt->mmio.adj_offset; > > > > > > + > > > > > > + return readl(tile->mmio.regs + reg.addr); > > > > > > +} > > > > > > + > > > > > > +static inline void xe_mmio_write64_device(struct xe_gt *gt, > > > > > > +struct xe_reg reg, u64 val) > > > > > > +{ > > > > > > + struct xe_tile *tile = gt_to_tile(gt); > > > > > > + > > > > > > + if (reg.addr < gt->mmio.adj_limit) > > > > > > + reg.addr += gt->mmio.adj_offset; > > > > > > + > > > > > > + writeq(val, tile->mmio.regs + reg.addr); > > > > > > +} > > > > > > + > > > > > > +static inline u64 xe_mmio_read64_device(struct xe_gt *gt, struct > > > > > > xe_reg reg) > > > > > > +{ > > > > > > + struct xe_tile *tile = gt_to_tile(gt); > > > > > > + > > > > > > + if (reg.addr < gt->mmio.adj_limit) > > > > > > + reg.addr += gt->mmio.adj_offset; > > > > > > + > > > > > > + return readq(tile->mmio.regs + reg.addr); > > > > > > +} > > > > > > + > > > > > > static int xe_set_dma_info(struct xe_device *xe) > > > > > > { > > > > > > unsigned int mask_size = xe->info.dma_mask_size; > > > > > > @@ -377,6 +433,29 @@ static void mmio_fini(struct drm_device *drm, > > > > > > void *arg) > > > > > > iounmap(xe->mem.vram.mapping); > > > > > > } > > > > > > > > > > > > +static void xe_mmio_set_funcs(struct xe_device *xe) > > > > > > +{ > > > > > > + /* For now all platforms use the set of MMIO functions for a > > > > > > + * physical device. > > > > > > + */ > > > > >
Re: [PATCH v2 4/4] drm/mediatek: Fix dereference before null check
Hi Angelo, On Wed, 2023-06-14 at 10:43 +0200, AngeloGioacchino Del Regno wrote: External email : Please do not click links or open attachments until you have verified the sender or the content. Il 13/06/23 13:32, Jason-JH.Lin ha scritto: > Null-checking state suggests that it may be null, but it has already > been dereferenced on drm_atomic_get_new_plane_state(state, plane). > > The parameter state will never be NULL currently, so just remove the > state is NULL flow in this function. > > Signed-off-by: Jason-JH.Lin > Fixes: 5ddb0bd4ddc3 ("drm/atomic: Pass the full state to planes async atomic > check and update") Fixes before S-o-b... Reviewed-by: AngeloGioacchino Del Regno Thanks, I'll swap it at the next version. Regards, Jason-JH.Lin
Re: patches dropped from drm-misc-next [Was: Re: [PATCH 00/53] drm: Convert to platform remove callback returning] void
Hi, On Sat, Jun 17, 2023 at 9:15 AM Uwe Kleine-König wrote: > > [expanding recipents by the other affected persons] > > On Thu, Jun 08, 2023 at 09:08:15AM -0700, Doug Anderson wrote: > > On Thu, Jun 1, 2023 at 8:40 AM Uwe Kleine-König > > wrote: > > > > > > Hello, > > > > > > On Sun, May 07, 2023 at 06:25:23PM +0200, Uwe Kleine-König wrote: > > > > this patch series adapts the platform drivers below drivers/gpu/drm > > > > to use the .remove_new() callback. Compared to the traditional .remove() > > > > callback .remove_new() returns no value. This is a good thing because > > > > the driver core doesn't (and cannot) cope for errors during remove. The > > > > only effect of a non-zero return value in .remove() is that the driver > > > > core emits a warning. The device is removed anyhow and an early return > > > > from .remove() usually yields a resource leak. > > > > > > > > By changing the remove callback to return void driver authors cannot > > > > reasonably (but wrongly) assume any more that there happens some kind of > > > > cleanup later. > > > > > > I wonder if someone would volunteer to add the whole series to > > > drm-misc-next?! > > > > It looks as if Neil applied quite a few of them already, so I looked > > at what was left... > > > > I'm a little hesitant to just apply the whole kit-and-caboodle to > > drm-misc-next since there are specific DRM trees for a bunch of them > > and it would be better if they landed there. ...so I went through all > > the patches that still applied to drm-misc-next, then used > > 'scripts/get_maintainer.pl --scm' to check if they were maintained > > through drm-misc. That still left quite a few patches. I've applied > > those ones and pushed to drm-misc-next: > > > > 71722685cd17 drm/xlnx/zynqmp_dpsub: Convert to platform remove > > callback returning void > > 1ed54a19f3b3 drm/vc4: Convert to platform remove callback returning void > > b957812839f8 drm/v3d: Convert to platform remove callback returning void > > e2fd3192e267 drm/tve200: Convert to platform remove callback returning void > > 84e6da7ad553 drm/tiny: Convert to platform remove callback returning void > > 34cdd1f691ad drm/tidss: Convert to platform remove callback returning void > > d665e3c9d37a drm/sun4i: Convert to platform remove callback returning void > > 0c259ab19146 drm/stm: Convert to platform remove callback returning void > > 9a865e45884a drm/sti: Convert to platform remove callback returning void > > 3c855610840e drm/rockchip: Convert to platform remove callback returning > > void > > e41977a83b71 drm/panfrost: Convert to platform remove callback returning > > void > > cef3776d0b5a drm/panel: Convert to platform remove callback returning void > > bd296a594e87 drm/mxsfb: Convert to platform remove callback returning void > > 38ca2d93d323 drm/meson: Convert to platform remove callback returning void > > fd1457d84bae drm/mcde: Convert to platform remove callback returning void > > 41a56a18615c drm/logicvc: Convert to platform remove callback returning void > > 980ec6444372 drm/lima: Convert to platform remove callback returning void > > 82a2c0cc1a22 drm/hisilicon: Convert to platform remove callback returning > > void > > c3b28b29ac0a drm/fsl-dcu: Convert to platform remove callback returning void > > a118fc6e71f9 drm/atmel-hlcdc: Convert to platform remove callback returning > > void > > 9a32dd324c46 drm/aspeed: Convert to platform remove callback returning void > > 2c7d291c498c drm/arm/malidp: Convert to platform remove callback returning > > void > > a920028df679 drm/arm/hdlcd: Convert to platform remove callback returning > > void > > 1bf3d76a7d15 drm/komeda: Convert to platform remove callback returning void > > Together with the patches that were applied later the topmost commit > from this series is c2807ecb5290 ("drm/omap: Convert to platform remove > callback returning void"). This commit was part for the following next > tags: > > $ git tag -l --contains c2807ecb5290 > next-20230609 > next-20230613 > next-20230614 > next-20230615 > > However in next-20230616 they are missing. In next-20230616 > drm-misc/for-linux-next was cf683e8870bd4be0fd6b98639286700a35088660. > Compared to c2807ecb5290 this adds 1149 patches but drops 37 (that are > also not included with a different commit id). The 37 patches dropped > are 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290: > > $ git shortlog -s > 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290 > 1 Christophe JAILLET > 2 Jessica Zhang > 5 Karol Wachowski > 1 Laura Nao > 27 Uwe Kleine-König > 1 Wang Jianzheng > > > I guess this was done by mistake because nobody told me about dropping > my/these patches? Can c2807ecb5290 please be merged into drm-misc-next > again? Actually, it was probably a mistake that these patches got merged to linuxnext during the 4 days that you noticed. However, your patches
Re: [Intel-gfx] [PATCH] drm/i915: Call page_address() on page acquired with GFP_KERNEL flag
On Wed, Jun 14, 2023 at 05:30:25PM +0200, Thomas Hellström (Intel) wrote: > > On 6/14/23 15:22, Tvrtko Ursulin wrote: > > > > On 14/06/2023 13:35, Sumitra Sharma wrote: > > > Pages allocated with GFP_KERNEL cannot come from Highmem. > > > That is why there is no need to call kmap() on them. > > > > Are you sure it is GFP_KERNEL backed and not tmpfs? I am not sure myself > > so let me copy Matt and Thomas if they happen to know off hand. > Hello, Yes it is true that the pages have not been acquired using the GFP_KERNEL. I confused the allocation of the struct 'i915_vma_resource' tracking the pages with the allocation of the pages themselves. This was noted by my mentor Ira Weiny . > It looks to me these are shmem pages or TTM pages. Both could be highmem. So > I think kmap is the correct choice here. > However, the kmap() will not be the correct choice here and kmap_local_page() must be used instead. I have created a v2 patch for the same https://lore.kernel.org/lkml/20230617180420.ga410...@sumitra.com/ Thank you for helping me. Regards Sumitra > /Thomas > > > > > > > > Regards, > > > > Tvrtko > > > > > Therefore, don't call kmap() on the page coming from > > > vma_res->bi.pages using for_each_sgt_page() in > > > i915_vma_coredump_create(). > > > > > > Use a plain page_address() to get the kernel address instead. > > > > > > Signed-off-by: Sumitra Sharma > > > --- > > > drivers/gpu/drm/i915/i915_gpu_error.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c > > > b/drivers/gpu/drm/i915/i915_gpu_error.c > > > index f020c0086fbc..6f51cb4fc55c 100644 > > > --- a/drivers/gpu/drm/i915/i915_gpu_error.c > > > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c > > > @@ -1164,9 +1164,8 @@ i915_vma_coredump_create(const struct intel_gt > > > *gt, > > > drm_clflush_pages(, 1); > > > - s = kmap(page); > > > + s = page_address(page); > > > ret = compress_page(compress, s, dst, false); > > > - kunmap(page); > > > drm_clflush_pages(, 1);
patches dropped from drm-misc-next [Was: Re: [PATCH 00/53] drm: Convert to platform remove callback returning] void
[expanding recipents by the other affected persons] On Thu, Jun 08, 2023 at 09:08:15AM -0700, Doug Anderson wrote: > On Thu, Jun 1, 2023 at 8:40 AM Uwe Kleine-König > wrote: > > > > Hello, > > > > On Sun, May 07, 2023 at 06:25:23PM +0200, Uwe Kleine-König wrote: > > > this patch series adapts the platform drivers below drivers/gpu/drm > > > to use the .remove_new() callback. Compared to the traditional .remove() > > > callback .remove_new() returns no value. This is a good thing because > > > the driver core doesn't (and cannot) cope for errors during remove. The > > > only effect of a non-zero return value in .remove() is that the driver > > > core emits a warning. The device is removed anyhow and an early return > > > from .remove() usually yields a resource leak. > > > > > > By changing the remove callback to return void driver authors cannot > > > reasonably (but wrongly) assume any more that there happens some kind of > > > cleanup later. > > > > I wonder if someone would volunteer to add the whole series to > > drm-misc-next?! > > It looks as if Neil applied quite a few of them already, so I looked > at what was left... > > I'm a little hesitant to just apply the whole kit-and-caboodle to > drm-misc-next since there are specific DRM trees for a bunch of them > and it would be better if they landed there. ...so I went through all > the patches that still applied to drm-misc-next, then used > 'scripts/get_maintainer.pl --scm' to check if they were maintained > through drm-misc. That still left quite a few patches. I've applied > those ones and pushed to drm-misc-next: > > 71722685cd17 drm/xlnx/zynqmp_dpsub: Convert to platform remove > callback returning void > 1ed54a19f3b3 drm/vc4: Convert to platform remove callback returning void > b957812839f8 drm/v3d: Convert to platform remove callback returning void > e2fd3192e267 drm/tve200: Convert to platform remove callback returning void > 84e6da7ad553 drm/tiny: Convert to platform remove callback returning void > 34cdd1f691ad drm/tidss: Convert to platform remove callback returning void > d665e3c9d37a drm/sun4i: Convert to platform remove callback returning void > 0c259ab19146 drm/stm: Convert to platform remove callback returning void > 9a865e45884a drm/sti: Convert to platform remove callback returning void > 3c855610840e drm/rockchip: Convert to platform remove callback returning void > e41977a83b71 drm/panfrost: Convert to platform remove callback returning void > cef3776d0b5a drm/panel: Convert to platform remove callback returning void > bd296a594e87 drm/mxsfb: Convert to platform remove callback returning void > 38ca2d93d323 drm/meson: Convert to platform remove callback returning void > fd1457d84bae drm/mcde: Convert to platform remove callback returning void > 41a56a18615c drm/logicvc: Convert to platform remove callback returning void > 980ec6444372 drm/lima: Convert to platform remove callback returning void > 82a2c0cc1a22 drm/hisilicon: Convert to platform remove callback returning void > c3b28b29ac0a drm/fsl-dcu: Convert to platform remove callback returning void > a118fc6e71f9 drm/atmel-hlcdc: Convert to platform remove callback returning > void > 9a32dd324c46 drm/aspeed: Convert to platform remove callback returning void > 2c7d291c498c drm/arm/malidp: Convert to platform remove callback returning > void > a920028df679 drm/arm/hdlcd: Convert to platform remove callback returning void > 1bf3d76a7d15 drm/komeda: Convert to platform remove callback returning void Together with the patches that were applied later the topmost commit from this series is c2807ecb5290 ("drm/omap: Convert to platform remove callback returning void"). This commit was part for the following next tags: $ git tag -l --contains c2807ecb5290 next-20230609 next-20230613 next-20230614 next-20230615 However in next-20230616 they are missing. In next-20230616 drm-misc/for-linux-next was cf683e8870bd4be0fd6b98639286700a35088660. Compared to c2807ecb5290 this adds 1149 patches but drops 37 (that are also not included with a different commit id). The 37 patches dropped are 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290: $ git shortlog -s 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290 1 Christophe JAILLET 2 Jessica Zhang 5 Karol Wachowski 1 Laura Nao 27 Uwe Kleine-König 1 Wang Jianzheng I guess this was done by mistake because nobody told me about dropping my/these patches? Can c2807ecb5290 please be merged into drm-misc-next again? Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
Re: [PATCH] drm: bridge: dw-mipi-dsi: Fix enable/disable of DSI controller
Tested-by: Julian Fairfax
Re: patches dropped from drm-misc-next [Was: Re: [PATCH 00/53] drm: Convert to platform remove callback returning] void
On Sun, Jun 18, 2023 at 12:13 AM Uwe Kleine-König wrote: > > [expanding recipents by the other affected persons] > > On Thu, Jun 08, 2023 at 09:08:15AM -0700, Doug Anderson wrote: > > On Thu, Jun 1, 2023 at 8:40 AM Uwe Kleine-König > > wrote: > > > > > > Hello, > > > > > > On Sun, May 07, 2023 at 06:25:23PM +0200, Uwe Kleine-König wrote: > > > > this patch series adapts the platform drivers below drivers/gpu/drm > > > > to use the .remove_new() callback. Compared to the traditional .remove() > > > > callback .remove_new() returns no value. This is a good thing because > > > > the driver core doesn't (and cannot) cope for errors during remove. The > > > > only effect of a non-zero return value in .remove() is that the driver > > > > core emits a warning. The device is removed anyhow and an early return > > > > from .remove() usually yields a resource leak. > > > > > > > > By changing the remove callback to return void driver authors cannot > > > > reasonably (but wrongly) assume any more that there happens some kind of > > > > cleanup later. > > > > > > I wonder if someone would volunteer to add the whole series to > > > drm-misc-next?! > > > > It looks as if Neil applied quite a few of them already, so I looked > > at what was left... > > > > I'm a little hesitant to just apply the whole kit-and-caboodle to > > drm-misc-next since there are specific DRM trees for a bunch of them > > and it would be better if they landed there. ...so I went through all > > the patches that still applied to drm-misc-next, then used > > 'scripts/get_maintainer.pl --scm' to check if they were maintained > > through drm-misc. That still left quite a few patches. I've applied > > those ones and pushed to drm-misc-next: > > > > 71722685cd17 drm/xlnx/zynqmp_dpsub: Convert to platform remove > > callback returning void > > 1ed54a19f3b3 drm/vc4: Convert to platform remove callback returning void > > b957812839f8 drm/v3d: Convert to platform remove callback returning void > > e2fd3192e267 drm/tve200: Convert to platform remove callback returning void > > 84e6da7ad553 drm/tiny: Convert to platform remove callback returning void > > 34cdd1f691ad drm/tidss: Convert to platform remove callback returning void > > d665e3c9d37a drm/sun4i: Convert to platform remove callback returning void > > 0c259ab19146 drm/stm: Convert to platform remove callback returning void > > 9a865e45884a drm/sti: Convert to platform remove callback returning void > > 3c855610840e drm/rockchip: Convert to platform remove callback returning > > void > > e41977a83b71 drm/panfrost: Convert to platform remove callback returning > > void > > cef3776d0b5a drm/panel: Convert to platform remove callback returning void > > bd296a594e87 drm/mxsfb: Convert to platform remove callback returning void > > 38ca2d93d323 drm/meson: Convert to platform remove callback returning void > > fd1457d84bae drm/mcde: Convert to platform remove callback returning void > > 41a56a18615c drm/logicvc: Convert to platform remove callback returning void > > 980ec6444372 drm/lima: Convert to platform remove callback returning void > > 82a2c0cc1a22 drm/hisilicon: Convert to platform remove callback returning > > void > > c3b28b29ac0a drm/fsl-dcu: Convert to platform remove callback returning void > > a118fc6e71f9 drm/atmel-hlcdc: Convert to platform remove callback returning > > void > > 9a32dd324c46 drm/aspeed: Convert to platform remove callback returning void > > 2c7d291c498c drm/arm/malidp: Convert to platform remove callback returning > > void > > a920028df679 drm/arm/hdlcd: Convert to platform remove callback returning > > void > > 1bf3d76a7d15 drm/komeda: Convert to platform remove callback returning void > > Together with the patches that were applied later the topmost commit > from this series is c2807ecb5290 ("drm/omap: Convert to platform remove > callback returning void"). This commit was part for the following next > tags: > > $ git tag -l --contains c2807ecb5290 > next-20230609 > next-20230613 > next-20230614 > next-20230615 > > However in next-20230616 they are missing. In next-20230616 > drm-misc/for-linux-next was cf683e8870bd4be0fd6b98639286700a35088660. > Compared to c2807ecb5290 this adds 1149 patches but drops 37 (that are > also not included with a different commit id). The 37 patches dropped > are 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290: > > $ git shortlog -s > 13cdd12a9f934158f4ec817cf048fcb4384aa9dc..c2807ecb5290 > 1 Christophe JAILLET > 2 Jessica Zhang > 5 Karol Wachowski > 1 Laura Nao > 27 Uwe Kleine-König > 1 Wang Jianzheng > > > I guess this was done by mistake because nobody told me about dropping > my/these patches? Can c2807ecb5290 please be merged into drm-misc-next > again? AFAIK drm-misc/for-linux-next cuts off at -rc6, at which point any patches merged get queued up for -next-next. I think that's what happened
[PATCH v2] drm/i915: Replace kmap() with kmap_local_page()
kmap() has been deprecated in favor of the kmap_local_page() due to high cost, restricted mapping space, the overhead of a global lock for synchronization, and making the process sleep in the absence of free slots. kmap_local_page() is faster than kmap() and offers thread-local and CPU-local mappings, take pagefaults in a local kmap region and preserves preemption by saving the mappings of outgoing tasks and restoring those of the incoming one during a context switch. The mapping is kept thread local in the function “i915_vma_coredump_create” in i915_gpu_error.c Therefore, replace kmap() with kmap_local_page(). Suggested-by: Ira Weiny Signed-off-by: Sumitra Sharma --- Changes in v2: - Replace kmap() with kmap_local_page(). - Change commit subject and message. drivers/gpu/drm/i915/i915_gpu_error.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index f020c0086fbc..bc41500eedf5 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -1164,9 +1164,9 @@ i915_vma_coredump_create(const struct intel_gt *gt, drm_clflush_pages(, 1); - s = kmap(page); + s = kmap_local_page(page); ret = compress_page(compress, s, dst, false); - kunmap(page); + kunmap_local(s); drm_clflush_pages(, 1); -- 2.25.1
Re: [PATCH 2/3] dt-bindings: backlight: lm3630a: add entries to control boost frequency
Hi, On 17.06.23 19:42, Krzysztof Kozlowski wrote: > On 17/06/2023 18:34, Heiko Stübner wrote: >> Am Samstag, 17. Juni 2023, 12:12:17 CEST schrieb Krzysztof Kozlowski: >>> On 14/06/2023 21:08, Maximilian Weigand wrote: From: Maximilian Weigand Add 'ti,boost_use_1mhz' to switch between 500 kHz and 1 MHz boost converter switching frequency, and add 'ti,boost_frequency_shift' to activate a frequency shift to 560 kHz or 1.12 MHz, respectively. Signed-off-by: Maximilian Weigand --- .../bindings/leds/backlight/lm3630a-backlight.yaml | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml index 3c9b4054ed9a..ef7ea0ad2d25 100644 --- a/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml +++ b/Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml @@ -33,6 +33,18 @@ properties: description: GPIO to use to enable/disable the backlight (HWEN pin). maxItems: 1 + ti,boost_use_1mhz: >>> >>> No underscores in property names. >>> +description: | >>> >>> Do not need '|' unless you need to preserve formatting. >>> + If present, change the boost converter switching frequency from the + default 500 kHz to 1 MHz. Refer to data sheet for hardware requirements. +type: boolean + + ti,boost_frequency_shift: +description: | + If present, change boost converter switching frequency from 500 kHz to + 560 kHz or from 1 Mhz to 1.12 Mhz, respectively. >>> >>> So just make it a property choosing the frequency, not bools, with >>> proper unit suffix. >> >> i.e. >> ti,boost-frequency-hz = ; >> with x being 50, 56, 100, 112 >> >> with the driver failing when the frequency is not achievable >> with the two knobs of 1mhz and shift. > > Yeah, with a default value (50, I guess). Thanks for the feedback, this is quite obviously the better solution! I will rework the submission accordingly. Best regards Maximilian
Re: [PATCH v2] drm/ingenic: Kconfig: select REGMAP and REGMAP_MMIO
Hi, On 2023/6/18 04:41, Paul Cercueil wrote: Hi, Le samedi 17 juin 2023 à 21:48 +0200, Sam Ravnborg a écrit : Hi Paul, On Sat, Jun 17, 2023 at 09:13:37PM +0200, Paul Cercueil wrote: Hi, Le mercredi 07 juin 2023 à 19:06 +0800, Sui Jingfeng a écrit : Otherwise its failed to pass basic compile test on platform without REGMAP_MMIO selected by defconfig make -j$(nproc) ARCH=mips CROSS_COMPILE=mips64el-linux-gnuabi64- SYNC include/config/auto.conf.cmd Checking missing-syscalls for N32 CALL scripts/checksyscalls.sh Checking missing-syscalls for O32 CALL scripts/checksyscalls.sh CALL scripts/checksyscalls.sh MODPOST Module.symvers ERROR: modpost: "__devm_regmap_init_mmio_clk" [drivers/gpu/drm/ingenic/ingenic-drm.ko] undefined! make[1]: *** [scripts/Makefile.modpost:136: Module.symvers] Error 1 make: *** [Makefile:1978: modpost] Error 2 V2: Order alphabetically Signed-off-by: Sui Jingfeng The patch looks good to me. But I need an ACK from someone else to apply to drm-misc-next. Reviewed-by: Sam Ravnborg Thanks Sam! Sam and Paul, thanks a lot, :-) Applied to drm-misc-next. Cheers, -Paul -- Jingfeng