Re: [PATCH v8 00/10] vgaarb: Rework default VGA device selection
For the whole series: Tested-by: Huacai Chen On Fri, Jan 7, 2022 at 12:30 AM Bjorn Helgaas wrote: > > [+to Maarten, Maxime, Thomas: sorry, I forgot to use > get_maintainer.pl so I missed you the first time. Beginning of thread: > https://lore.kernel.org/all/20220106000658.243509-1-helg...@kernel.org/#t > Git branch with this v8 + a couple trivial renames, based on v5.16-rc1: > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/log/?h=0f4caffa1297] > > On Wed, Jan 05, 2022 at 06:06:48PM -0600, Bjorn Helgaas wrote: > > From: Bjorn Helgaas > > > > Current default VGA device selection fails in some cases because part of it > > is done in the vga_arb_device_init() subsys_initcall, and some arches > > enumerate PCI devices in pcibios_init(), which runs *after* that. > > > > For example: > > > > - On BMC system, the AST2500 bridge [1a03:1150] does not implement > > PCI_BRIDGE_CTL_VGA. This is perfectly legal but means the legacy VGA > > resources won't reach downstream devices unless they're included in the > > usual bridge windows. > > > > - vga_arb_select_default_device() will set a device below such a bridge > > as the default VGA device as long as it has PCI_COMMAND_IO and > > PCI_COMMAND_MEMORY enabled. > > > > - vga_arbiter_add_pci_device() is called for every VGA device, either at > > boot-time or at hot-add time, and it will also set the device as the > > default VGA device, but ONLY if all bridges leading to it implement > > PCI_BRIDGE_CTL_VGA. > > > > - This difference between vga_arb_select_default_device() and > > vga_arbiter_add_pci_device() means that a device below an AST2500 or > > similar bridge can only be set as the default if it is enumerated > > before vga_arb_device_init(). > > > > - On ACPI-based systems, PCI devices are enumerated by acpi_init(), which > > runs before vga_arb_device_init(). > > > > - On non-ACPI systems, like on MIPS system, they are enumerated by > > pcibios_init(), which typically runs *after* vga_arb_device_init(). > > > > This series consolidates all the default VGA device selection in > > vga_arbiter_add_pci_device(), which is always called after enumerating a > > PCI device. > > > > Almost all the work here is Huacai's. I restructured it a little bit and > > added a few trivial patches on top. > > > > I'd like to move vgaarb.c to drivers/pci eventually, but there's another > > initcall ordering snag that needs to be resolved first, so this leaves > > it where it is. > > > > Bjorn > > > > Version history: > > V0 original implementation as final quirk to set default device. > > https://lore.kernel.org/r/20210514080025.1828197-6-chenhua...@loongson.cn > > > > V1 rework vgaarb to do all default device selection in > > vga_arbiter_add_pci_device(). > > https://lore.kernel.org/r/20210705100503.1120643-1-chenhua...@loongson.cn > > > > V2 move arbiter to PCI subsystem, fix nits. > > https://lore.kernel.org/r/20210722212920.347118-1-helg...@kernel.org > > > > V3 rewrite the commit log of the last patch (which is also summarized > > by Bjorn). > > https://lore.kernel.org/r/20210820100832.663931-1-chenhua...@loongson.cn > > > > V4 split the last patch to two steps. > > https://lore.kernel.org/r/20210827083129.2781420-1-chenhua...@loongson.cn > > > > V5 split Patch-9 again and sort the patches. > > https://lore.kernel.org/r/20210911093056.1555274-1-chenhua...@loongson.cn > > > > V6 split Patch-5 again and sort the patches again. > > https://lore.kernel.org/r/20210916082941.3421838-1-chenhua...@loongson.cn > > > > V7 stop moving vgaarb to drivers/pci because of ordering issues with > > misc_init(). > > https://lore.kernel.org/r/20211015061512.2941859-1-chenhua...@loongson.cn > > https://lore.kernel.org/r/caahv-h7fhajm-ha42z1dlre4pvc9frfyeu27khwcywkkmft...@mail.gmail.com > > > > > > Bjorn Helgaas (8): > > vgaarb: Factor out vga_select_framebuffer_device() > > vgaarb: Factor out default VGA device selection > > vgaarb: Move framebuffer detection to ADD_DEVICE path > > vgaarb: Move non-legacy VGA detection to ADD_DEVICE path > > vgaarb: Move disabled VGA device detection to ADD_DEVICE path > > vgaarb: Remove empty vga_arb_device_card_gone() > > vgaarb: Use unsigned format string to print lock counts > > vgaarb: Replace full MIT license text with SPDX identifier > > > > Huacai Chen (2): > > vgaarb: Move vga_arb_integrated_gpu() earlier in file > > vgaarb: Log bridge control messages when adding devices > > > > drivers/gpu/vga/vgaarb.c | 311 +++ > > 1 file changed, 154 insertions(+), 157 deletions(-) > > > > -- > > 2.25.1 > > > >
Re: [PATCH] drm/atomic: Check new_crtc_state->active to determine if CRTC needs disable in self refresh mode
On Fri, 2022-01-07 at 14:53 -0500, Alex Deucher wrote:
> On Wed, Dec 29, 2021 at 11:07 PM Liu Ying wrote:
> >
> > Actual hardware state of CRTC is controlled by the member 'active'
> > in
> > struct drm_crtc_state instead of the member 'enable', according to
> > the
> > kernel doc of the member 'enable'. In fact, the drm client modeset
> > and atomic helpers are using the member 'active' to do the control.
> >
> > Referencing the member 'enable' of new_crtc_state, the function
> > crtc_needs_disable() may fail to reflect if CRTC needs disable in
> > self refresh mode, e.g., when the framebuffer emulation will be
> > blanked
> > through the client modeset helper with the next commit, the member
> > 'enable' of new_crtc_state is still true while the member 'active'
> > is
> > false, hence the relevant potential encoder and bridges won't be
> > disabled.
> >
> > So, let's check new_crtc_state->active to determine if CRTC needs
> > disable
> > in self refresh mode instead of new_crtc_state->enable.
> >
> > Fixes: 1452c25b0e60 ("drm: Add helpers to kick off self refresh
> > mode in drivers")
> > Cc: Sean Paul
> > Cc: Rob Clark
> > Cc: Maarten Lankhorst
> > Cc: Maxime Ripard
> > Cc: Thomas Zimmermann
> > Cc: David Airlie
> > Cc: Daniel Vetter
> > Signed-off-by: Liu Ying
>
> Reviewed-by: Alex Deucher
>
> Do you need someone to push this for you?
Yes, please. Thanks.
Liu Ying
>
> Alex
>
> > ---
> > drivers/gpu/drm/drm_atomic_helper.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index a7a05e1e26bb..9603193d2fa1 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1016,7 +1016,7 @@ crtc_needs_disable(struct drm_crtc_state
> > *old_state,
> > * it's in self refresh mode and needs to be fully
> > disabled.
> > */
> > return old_state->active ||
> > - (old_state->self_refresh_active && !new_state-
> > >enable) ||
> > + (old_state->self_refresh_active && !new_state-
> > >active) ||
> >new_state->self_refresh_active;
> > }
> >
> > --
> > 2.25.1
> >
Re: [PATCH 1/2] drm/msm/gpu: Wait for idle before suspending
Quoting Rob Clark (2022-01-06 10:14:46)
> From: Rob Clark
>
> System suspend uses pm_runtime_force_suspend(), which cheekily bypasses
> the runpm reference counts. This doesn't actually work so well when the
> GPU is active. So add a reasonable delay waiting for the GPU to become
> idle.
Maybe also say:
Failure to wait during system wide suspend leads to GPU hangs seen on
resume.
>
> Alternatively we could just return -EBUSY in this case, but that has the
> disadvantage of causing system suspend to fail.
>
> Signed-off-by: Rob Clark
> ---
> drivers/gpu/drm/msm/adreno/adreno_device.c | 9 +
> drivers/gpu/drm/msm/msm_gpu.c | 3 +++
> drivers/gpu/drm/msm/msm_gpu.h | 3 +++
> 3 files changed, 15 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
> b/drivers/gpu/drm/msm/adreno/adreno_device.c
> index 93005839b5da..b677ca3fd75e 100644
> --- a/drivers/gpu/drm/msm/adreno/adreno_device.c
> +++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
> @@ -611,6 +611,15 @@ static int adreno_resume(struct device *dev)
> static int adreno_suspend(struct device *dev)
> {
> struct msm_gpu *gpu = dev_to_gpu(dev);
> + int ret = 0;
Please don't assign and then immediately overwrite.
> +
> + ret = wait_event_timeout(gpu->retire_event,
> +!msm_gpu_active(gpu),
> +msecs_to_jiffies(1000));
> + if (ret == 0) {
The usual pattern is
long timeleft;
timeleft = wait_event_timeout(...)
if (!timeleft) {
/* no time left; timed out */
Can it be the same pattern here? It helps because people sometimes
forget that wait_event_timeout() returns the time that is left and not
an error code when it times out.
> + dev_err(dev, "Timeout waiting for GPU to suspend\n");
> + return -EBUSY;
> + }
>
> return gpu->funcs->pm_suspend(gpu);
> }
Re: [PATCH] drm/virtio: Fix NULL vs IS_ERR checking in virtio_gpu_object_shmem_init
On Tue, Dec 21, 2021 at 11:26 PM Miaoqian Lin wrote:
> Since drm_prime_pages_to_sg() function return error pointers.
> The drm_gem_shmem_get_sg_table() function returns error pointers too.
> Using IS_ERR() to check the return value to fix this.
>
> Fixes: f651c8b05542("drm/virtio: factor out the sg_table from
> virtio_gpu_object")
> Signed-off-by: Miaoqian Lin
>
Reviewed-by: Gurchetan Singh
> ---
> drivers/gpu/drm/virtio/virtgpu_object.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
> b/drivers/gpu/drm/virtio/virtgpu_object.c
> index f648b0e24447..8bb80289672c 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_object.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_object.c
> @@ -168,9 +168,9 @@ static int virtio_gpu_object_shmem_init(struct
> virtio_gpu_device *vgdev,
> * since virtio_gpu doesn't support dma-buf import from other
> devices.
> */
> shmem->pages = drm_gem_shmem_get_sg_table(&bo->base.base);
> - if (!shmem->pages) {
> + if (IS_ERR(shmem->pages)) {
> drm_gem_shmem_unpin(&bo->base.base);
> - return -EINVAL;
> + return PTR_ERR(shmem->pages);
> }
>
> if (use_dma_api) {
> --
> 2.17.1
>
>
[Bug 215001] Regression in 5.15, Firmware-initialized graphics console selects FB_VGA16, screen corruption
https://bugzilla.kernel.org/show_bug.cgi?id=215001 --- Comment #10 from Kris Karas (bugs-...@moonlit-rail.com) --- Hi Thorsten - Thanks for the helpful pointer on bug reporting (comment 9). I've been so used to dealing with LKML and Bugzilla (since 1993) that I'd rather forgotten about Lore. :-) I didn't know whether the regression trackers were a real "bot" sort of thing, or just folks pitching in to try to keep track by hand. Good to know that it's still human-powered. I've got another bug to file now, multiple flavors of OOPSen showing up on boot in _kmalloc due to creation/manipulation of network devices (bridges, veth), perhaps as a result of the "igb" patches in 5.15 as it seems hardware-specific. I'll try to cc those lists when I have it bisected or narrowed down a bit... Kris -- You may reply to this email to add a comment. You are receiving this mail because: You are on the CC list for the bug.
Re: [PATCH 0/2]
Hi Zhenneng, + some display folks First of all, thanks a lot for your patch. We had a similar patch in the past, but we had to revert it because we cannot simply enable DCN for ARM-based systems. You can refer to this commit message to get a better context: https://gitlab.freedesktop.org/agd5f/linux/-/commit/c241ed2f0ea549c18cff62a3708b43846b84dae3 Before enabling ARM, we first need to isolate all FPU code in the DML folder fully. You can read more about our strategy at the below link: https://patchwork.freedesktop.org/series/93042/ And you can see some examples of this effort in the below links: - https://patchwork.freedesktop.org/series/95504/ - https://patchwork.freedesktop.org/patch/455465/?series=94441&rev=3 - https://patchwork.freedesktop.org/series/98247/ Soon we will submit another series that isolate DCN302, but we still need to isolate code from DCN20, DCN10, DCN303, and DCN301. If you want to help us speed up this process, feel free to look at DCN301 or DCN10. Best Regards Siqueira On 2022-01-07 4:57 a.m., Zhenneng Li wrote: For adapting radeon rx6600 xt on arm64 platform, there report some compile errors. Zhenneng Li (2): drm/amdgpu: fix compile error for dcn on arm64 drm/amdgpu: enable dcn support on arm64 drivers/gpu/drm/amd/display/Kconfig | 2 +- drivers/gpu/drm/amd/display/dc/calcs/Makefile | 6 + .../gpu/drm/amd/display/dc/clk_mgr/Makefile | 7 ++ drivers/gpu/drm/amd/display/dc/dcn10/Makefile | 4 +++ drivers/gpu/drm/amd/display/dc/dcn20/Makefile | 4 +++ .../gpu/drm/amd/display/dc/dcn201/Makefile| 6 + drivers/gpu/drm/amd/display/dc/dcn21/Makefile | 4 +++ drivers/gpu/drm/amd/display/dc/dcn30/Makefile | 6 + .../gpu/drm/amd/display/dc/dcn302/Makefile| 6 + .../gpu/drm/amd/display/dc/dcn303/Makefile| 6 + drivers/gpu/drm/amd/display/dc/dcn31/Makefile | 6 + drivers/gpu/drm/amd/display/dc/dml/Makefile | 25 +++ drivers/gpu/drm/amd/display/dc/dsc/Makefile | 7 ++ 13 files changed, 88 insertions(+), 1 deletion(-)
Re: [PATCH 6/6] drm/meson: add support for MIPI-DSI transceiver
Hi Neil,
some high-level comments from me below.
On Fri, Jan 7, 2022 at 3:58 PM Neil Armstrong wrote:
[...]
> +/* MIPI DSI Relative REGISTERs Definitions */
> +/* For MIPI_DSI_TOP_CNTL */
> +#define BIT_DPI_COLOR_MODE20
> +#define BIT_IN_COLOR_MODE 16
> +#define BIT_CHROMA_SUBSAMPLE 14
> +#define BIT_COMP2_SEL 12
> +#define BIT_COMP1_SEL 10
> +#define BIT_COMP0_SEL 8
> +#define BIT_DE_POL 6
> +#define BIT_HSYNC_POL 5
> +#define BIT_VSYNC_POL 4
> +#define BIT_DPICOLORM 3
> +#define BIT_DPISHUTDN 2
> +#define BIT_EDPITE_INTR_PULSE 1
> +#define BIT_ERR_INTR_PULSE 0
Why not use BIT() and GENMASK() for these and prefixing them with
MIPI_DSI_TOP_CNTL_?
That would make them consistent with other parts of the meson sub-driver.
[...]
> +static void meson_dw_mipi_dsi_hw_init(struct meson_dw_mipi_dsi *mipi_dsi)
> +{
> + writel_relaxed((1 << 4) | (1 << 5) | (0 << 6),
> + mipi_dsi->base + MIPI_DSI_TOP_CNTL);
please use the macros from above
> + writel_bits_relaxed(0xf, 0xf, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET);
> + writel_bits_relaxed(0xf, 0, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET);
[...]
> + phy_power_on(mipi_dsi->phy);
Please propagate the error code here.
Also shouldn't this go to a new dw_mipi_dsi_phy_power_on() as the PHY
driver uses the updated settings from phy_configure only in it's
.power_on callback?
[...]
> + phy_configure(mipi_dsi->phy, &mipi_dsi->phy_opts);
please propagate the error code here as the PHY driver has some
explicit code to return an error in it's .phy_configure callback
[...]
> + phy_init(mipi_dsi->phy);
please propagate the error code here
[...]
> + phy_exit(mipi_dsi->phy);
please propagate the error code here
[...]
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mipi_dsi->base = devm_ioremap_resource(&pdev->dev, res);
other parts of the meson DRM driver have been converted to use
devm_platform_ioremap_resource()
I suggest updating this as well to simplify the code here
[...]
> + mipi_dsi->phy = devm_phy_get(&pdev->dev, "dphy");
> + if (IS_ERR(mipi_dsi->phy)) {
> + ret = PTR_ERR(mipi_dsi->phy);
> + dev_err(&pdev->dev, "failed to get mipi dphy: %d\n", ret);
> + return ret;
you can simplify this with:
return dev_err_probe(&pdev->dev, PTR_ERR(mipi_dsi->phy, "failed to
get mipi dphy\n");
[...]
> + mipi_dsi->px_clk = devm_clk_get(&pdev->dev, "px_clk");
> + if (IS_ERR(mipi_dsi->px_clk)) {
> + dev_err(&pdev->dev, "Unable to get PLL clk\n");
> + return PTR_ERR(mipi_dsi->px_clk);
you can simplify this with:
return dev_err_probe(&pdev->dev, PTR_ERR(mipi_dsi->px_clk, "Unable
to get PLL clk\n");
Also should it say s/PLL clk/px clock/?
[...]
> + top_rst = devm_reset_control_get_exclusive(&pdev->dev, "top");
> + if (IS_ERR(top_rst)) {
> + ret = PTR_ERR(top_rst);
> +
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev, "Unable to get reset control:
> %d\n", ret);
> +
> + return ret;
you can simplify this with:
return dev_err_probe(&pdev->dev, PTR_ERR(top_rst, "Unable to get
reset control\n");
[...]
> + mipi_dsi->dmd = dw_mipi_dsi_probe(pdev, &mipi_dsi->pdata);
> + if (IS_ERR(mipi_dsi->dmd)) {
> + ret = PTR_ERR(mipi_dsi->dmd);
> + if (ret != -EPROBE_DEFER)
> + dev_err(&pdev->dev,
> + "Failed to probe dw_mipi_dsi: %d\n", ret);
you can simplify this with:
dev_err_probe(&pdev->dev, ret, "Failed to probe dw_mipi_dsi\n");
Best regards,
Martin
Re: [PATCH 5/6] drm/meson: add DSI encoder
Hi Neil, On Fri, Jan 7, 2022 at 3:57 PM Neil Armstrong wrote: [...] > + writel_bits_relaxed(BIT(3), BIT(3), priv->io_base + > _REG(ENCL_VIDEO_MODE_ADV)); see my comment on patch #3 from this series for BIT(3) Best regards, Martin
Re: [PATCH 3/6] drm/meson: venc: add ENCL encoder setup for MIPI-DSI output
Hi Neil,
On Fri, Jan 7, 2022 at 3:57 PM Neil Armstrong wrote:
>
> This adds supports for the ENCL encoder connected to a MIPI-DSI transceiver
> on the
> Amlogic AXG SoCs.
Should this be "AXG and newer SoCs" or is this really AXG specific?
[...]
> +#define GAMMA_VCOM_POL7 /* RW */
> +#define GAMMA_RVS_OUT 6 /* RW */
> +#define ADR_RDY 5 /* Read Only */
> +#define WR_RDY4 /* Read Only */
> +#define RD_RDY3 /* Read Only */
> +#define GAMMA_TR 2 /* RW */
> +#define GAMMA_SET 1 /* RW */
> +#define GAMMA_EN 0 /* RW */
> +
> +#define H_RD 12
> +#define H_AUTO_INC11
> +#define H_SEL_R 10
> +#define H_SEL_G 9
> +#define H_SEL_B 8
I think all values above can be wrapped in the BIT() macro, then you
don't need that below.
> +#define HADR_MSB 7/* 7:0 */
> +#define HADR 0/* 7:0 */
Here GENMASK(7, 0) can be used for HADR
Also I think prefixing all macros above with their register name
(L_GAMMA_CNTL_PORT_ or L_GAMMA_ADDR_PORT_) will make the code easier
to read.
[...]
> + writel_relaxed(0x8000, priv->io_base + _REG(ENCL_VIDEO_MODE));
The public S905 datasheet calls 0x8000 ENCL_PX_LN_CNT_SHADOW_EN
> + writel_relaxed(0x0418, priv->io_base + _REG(ENCL_VIDEO_MODE_ADV));
According to the public S905 datasheet this is:
- BIT(3): ENCL_VIDEO_MODE_ADV_VFIFO_EN
- BIT(4): ENCL_VIDEO_MODE_ADV_GAIN_HDTV
- BIT(10): ENCL_SEL_GAMMA_RGB_IN
> + writel_relaxed(0x1000, priv->io_base + _REG(ENCL_VIDEO_FILT_CTRL));
I don't know the exact name but the 32-bit vendor kernel sources have
a comment [0] saying that 0x1000 is "bypass filter"
But maybe we can simply call it ENCL_VIDEO_FILT_CTRL_BYPASS_FILTER
[...]
> + writel_relaxed(3, priv->io_base + _REG(ENCL_VIDEO_RGBIN_CTRL));
The public S905 datasheet says:
- BIT(0): USE RGB data from VIU, furthermore a comment in the 3.10
kernel sources make this more clear: bit[0] 1:RGB, 0:YUV
- BIT(1): CFG_VIDEO_RGBIN_ZBLK
> + /* default black pattern */
> + writel_relaxed(0, priv->io_base + _REG(ENCL_TST_MDSEL));
> + writel_relaxed(0, priv->io_base + _REG(ENCL_TST_Y));
> + writel_relaxed(0, priv->io_base + _REG(ENCL_TST_CB));
> + writel_relaxed(0, priv->io_base + _REG(ENCL_TST_CR));
> + writel_relaxed(1, priv->io_base + _REG(ENCL_TST_EN));
> + writel_bits_relaxed(BIT(3), 0, priv->io_base +
> _REG(ENCL_VIDEO_MODE_ADV));
same as above: ENCL_VIDEO_MODE_ADV_VFIFO_EN
> +
> + writel_relaxed(1, priv->io_base + _REG(ENCL_VIDEO_EN));
> +
> + writel_relaxed(0, priv->io_base + _REG(L_RGB_BASE_ADDR));
> + writel_relaxed(0x400, priv->io_base + _REG(L_RGB_COEFF_ADDR));
note to self: L_RGB_COEFF_ADDR seems to contain some "magic" value,
there's no further info in the 3.10 kernel sources or datasheet
> + writel_relaxed(0x400, priv->io_base + _REG(L_DITH_CNTL_ADDR));
According to the public S905 datasheet BIT(10) is DITH10_EN (10-bits
Dithering to 8 Bits Enable).
I am not sure if this would belong to the selected video mode/bit depth.
I'll let other reviewers decide if this is relevant or not because I don't know.
[...]
> + writel_relaxed(0, priv->io_base + _REG(L_INV_CNT_ADDR));
> + writel_relaxed(BIT(4) | BIT(5),
> + priv->io_base + _REG(L_TCON_MISC_SEL_ADDR));
the public S905 datasheet states:
- BIT(4): STV1_SEL (STV1 is frame Signal)
- BIT(5): STV2_SEL (STV2 is frame Signal)
This doesn't seem helpful to me though, but maybe you can still create
preprocessor macros for this (for consistency)?
[...]
> + switch (priv->venc.current_mode) {
> + case MESON_VENC_MODE_MIPI_DSI:
> + writel_relaxed(0x200,
> + priv->io_base + _REG(VENC_INTCTRL));
the public S905 datasheet documents this as:
- BIT(9): ENCP_LNRST_INT_EN (Progressive encoder filed change interrupt enable)
Please add a preprocessor macro to make it consistent with
VENC_INTCTRL_ENCI_LNRST_INT_EN which already exists and is used below.
Best regards,
Martin
Re: [PATCH 2/6] dt-bindings: display: meson-vpu: add third DPI output port
On Fri, Jan 7, 2022 at 3:56 PM Neil Armstrong wrote: > > Add third port corresponding to the ENCL DPI encoder used to connect > to DSI or LVDS transceivers. > > Signed-off-by: Neil Armstrong Reviewed-by: Martin Blumenstingl
Re: [PATCH] drm/amdkfd: use default_groups in kobj_type
On Thu, Jan 6, 2022 at 4:57 AM Greg Kroah-Hartman
wrote:
>
> There are currently 2 ways to create a set of sysfs files for a
> kobj_type, through the default_attrs field, and the default_groups
> field. Move the amdkfd sysfs code to use default_groups field which has
> been the preferred way since aa30f47cf666 ("kobject: Add support for
> default attribute groups to kobj_type") so that we can soon get rid of
> the obsolete default_attrs field.
>
> Cc: Felix Kuehling
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: "Pan, Xinhui"
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: amd-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Greg Kroah-Hartman
Applied. Thanks!
Alex
> ---
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index b993011cfa64..1f4a07f984eb 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -462,6 +462,7 @@ static struct attribute *procfs_queue_attrs[] = {
> &attr_queue_gpuid,
> NULL
> };
> +ATTRIBUTE_GROUPS(procfs_queue);
>
> static const struct sysfs_ops procfs_queue_ops = {
> .show = kfd_procfs_queue_show,
> @@ -469,7 +470,7 @@ static const struct sysfs_ops procfs_queue_ops = {
>
> static struct kobj_type procfs_queue_type = {
> .sysfs_ops = &procfs_queue_ops,
> - .default_attrs = procfs_queue_attrs,
> + .default_groups = procfs_queue_groups,
> };
>
> static const struct sysfs_ops procfs_stats_ops = {
> --
> 2.34.1
>
Re: [PATCH] drm/amdgpu: use default_groups in kobj_type
On Thu, Jan 6, 2022 at 4:56 AM Greg Kroah-Hartman
wrote:
>
> There are currently 2 ways to create a set of sysfs files for a
> kobj_type, through the default_attrs field, and the default_groups
> field. Move the amdgpu sysfs code to use default_groups field which has
> been the preferred way since aa30f47cf666 ("kobject: Add support for
> default attribute groups to kobj_type") so that we can soon get rid of
> the obsolete default_attrs field.
>
> Cc: Alex Deucher
> Cc: "Christian König"
> Cc: "Pan, Xinhui"
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Hawking Zhang
> Cc: John Clements
> Cc: Felix Kuehling
> Cc: Jonathan Kim
> Cc: Kevin Wang
> Cc: shaoyunl
> Cc: Tao Zhou
> Cc: amd-...@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Greg Kroah-Hartman
Applied. Thanks!
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index 567df2db23ac..94dcb004988d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -208,6 +208,7 @@ static struct attribute *amdgpu_xgmi_hive_attrs[] = {
> &amdgpu_xgmi_hive_id,
> NULL
> };
> +ATTRIBUTE_GROUPS(amdgpu_xgmi_hive);
>
> static ssize_t amdgpu_xgmi_show_attrs(struct kobject *kobj,
> struct attribute *attr, char *buf)
> @@ -237,7 +238,7 @@ static const struct sysfs_ops amdgpu_xgmi_hive_ops = {
> struct kobj_type amdgpu_xgmi_hive_type = {
> .release = amdgpu_xgmi_hive_release,
> .sysfs_ops = &amdgpu_xgmi_hive_ops,
> - .default_attrs = amdgpu_xgmi_hive_attrs,
> + .default_groups = amdgpu_xgmi_hive_groups,
> };
>
> static ssize_t amdgpu_xgmi_show_device_id(struct device *dev,
> --
> 2.34.1
>
Re: [PATCH v5 03/32] component: Move struct aggregate_device out to header file
Quoting Jani Nikula (2022-01-07 05:07:59)
> On Thu, 06 Jan 2022, Stephen Boyd wrote:
> > This allows aggregate driver writers to use the device passed to their
> > probe/remove/shutdown functions properly instead of treating it as an
> > opaque pointer.
>
> You say it like having opaque pointers with interfaces instead of
> exposed data is a bad thing.
I didn't intend to convey that message at all and in fact I didn't write
that opaque pointers are a bad thing.
>
> Data is not an interface. IMO if you can get by with keeping the types
> private, go for it. Unless I'm missing something, you only need the
> parent dev pointer. Maybe add a helper function for it?
Sure I'll add a function for that.
>
> It's trivial to expose the guts like this, but it's usually a lot of
> hard work to go the other way. Look at the dependencies of component.h
> now. To keep it self-contained, i.e. buildable without implicit
> dependencies, you'd need to add #include , which goes on to
> include the world. Then have a look at [1].
>
> Please at least let's not do this lightly.
>
Got it. Thanks! How does this look?
---8<---
diff --git a/drivers/base/component.c b/drivers/base/component.c
index cd50137753b4..e8f09945c261 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -56,6 +56,27 @@ struct component_match {
struct component_match_array *compare;
};
+struct aggregate_device {
+ const struct component_master_ops *ops;
+ struct device *parent;
+ struct device dev;
+ struct component_match *match;
+ struct aggregate_driver *adrv;
+
+ int id;
+};
+
+static inline struct aggregate_device *to_aggregate_device(struct device *d)
+{
+ return container_of(d, struct aggregate_device, dev);
+}
+
+struct device *aggregate_device_parent(struct aggregate_device *adev)
+{
+ return adev->parent;
+}
+EXPORT_SYMBOL_GPL(aggregate_device_parent);
+
struct component {
struct list_head node;
struct aggregate_device *adev;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index 0463386a6ed2..5fa868cf9825 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -27,7 +27,7 @@ struct komeda_dev *dev_to_mdev(struct device *dev)
static void komeda_unbind(struct aggregate_device *adev)
{
- struct device *dev = adev->parent;
+ struct device *dev = aggregate_device_parent(adev);
struct komeda_drv *mdrv = dev_get_drvdata(dev);
if (!mdrv)
@@ -48,7 +48,7 @@ static void komeda_unbind(struct aggregate_device *adev)
static int komeda_bind(struct aggregate_device *adev)
{
- struct device *dev = adev->parent;
+ struct device *dev = aggregate_device_parent(adev);
struct komeda_drv *mdrv;
int err;
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 5c03eb98d814..e3ed925797d5 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -272,7 +272,7 @@ static const struct drm_driver hdlcd_driver = {
static int hdlcd_drm_bind(struct aggregate_device *adev)
{
- struct device *dev = adev->parent;
+ struct device *dev = aggregate_device_parent(adev);
struct drm_device *drm;
struct hdlcd_drm_private *hdlcd;
int ret;
@@ -347,7 +347,7 @@ static int hdlcd_drm_bind(struct aggregate_device *adev)
static void hdlcd_drm_unbind(struct aggregate_device *adev)
{
- struct device *dev = adev->parent;
+ struct device *dev = aggregate_device_parent(adev);
struct drm_device *drm = dev_get_drvdata(dev);
struct hdlcd_drm_private *hdlcd = drm->dev_private;
diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index e6ee4d1e3bb8..7b946b962b22 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -704,7 +704,7 @@ static int malidp_runtime_pm_resume(struct device *dev)
static int malidp_bind(struct aggregate_device *adev)
{
- struct device *dev = adev->parent;
+ struct device *dev = aggregate_device_parent(adev);
struct resource *res;
struct drm_device *drm;
struct malidp_drm *malidp;
@@ -897,7 +897,7 @@ static int malidp_bind(struct aggregate_device *adev)
static void malidp_unbind(struct aggregate_device *adev)
{
- struct device *dev = adev->parent;
+ struct device *dev = aggregate_device_parent(adev);
struct drm_device *drm = dev_get_drvdata(dev);
struct malidp_drm *malidp = drm->dev_private;
struct malidp_hw_device *hwdev = malidp->dev;
diff --git a/drivers/gpu/drm/armada/armada_drv.c
b/drivers/gpu/drm/armada/armada_drv.c
index b3559363ea43..27739cbe2291 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -62,7 +62,7 @@ static const struct drm_mode_config_funcs
armada_drm_mode_config_funcs = {
static
Re: [PATCH] drm/atomic: Check new_crtc_state->active to determine if CRTC needs disable in self refresh mode
On Wed, Dec 29, 2021 at 11:07 PM Liu Ying wrote:
>
> Actual hardware state of CRTC is controlled by the member 'active' in
> struct drm_crtc_state instead of the member 'enable', according to the
> kernel doc of the member 'enable'. In fact, the drm client modeset
> and atomic helpers are using the member 'active' to do the control.
>
> Referencing the member 'enable' of new_crtc_state, the function
> crtc_needs_disable() may fail to reflect if CRTC needs disable in
> self refresh mode, e.g., when the framebuffer emulation will be blanked
> through the client modeset helper with the next commit, the member
> 'enable' of new_crtc_state is still true while the member 'active' is
> false, hence the relevant potential encoder and bridges won't be disabled.
>
> So, let's check new_crtc_state->active to determine if CRTC needs disable
> in self refresh mode instead of new_crtc_state->enable.
>
> Fixes: 1452c25b0e60 ("drm: Add helpers to kick off self refresh mode in
> drivers")
> Cc: Sean Paul
> Cc: Rob Clark
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
> Signed-off-by: Liu Ying
Reviewed-by: Alex Deucher
Do you need someone to push this for you?
Alex
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index a7a05e1e26bb..9603193d2fa1 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1016,7 +1016,7 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
> * it's in self refresh mode and needs to be fully disabled.
> */
> return old_state->active ||
> - (old_state->self_refresh_active && !new_state->enable) ||
> + (old_state->self_refresh_active && !new_state->active) ||
>new_state->self_refresh_active;
> }
>
> --
> 2.25.1
>
[PATCH v5 1/3] drm/privacy_screen: Add drvdata in drm_privacy_screen
Allow a privacy screen provider to stash its private data pointer in the
drm_privacy_screen, and update the drm_privacy_screen_register() call to
accept that. Also introduce a *_get_drvdata() so that it can retrieved
back when needed.
This also touches the IBM Thinkpad platform driver, the only user of
privacy screen today, to pass NULL for now to the updated API.
Signed-off-by: Rajat Jain
Reviewed-by: Hans de Goede
---
v5: Same as v4
v4: Added "Reviewed-by" from Hans
v3: Initial version. Came up due to review comments on v2 of other patches.
v2: No v2
v1: No v1
drivers/gpu/drm/drm_privacy_screen.c| 5 -
drivers/platform/x86/thinkpad_acpi.c| 2 +-
include/drm/drm_privacy_screen_driver.h | 13 -
3 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_privacy_screen.c
b/drivers/gpu/drm/drm_privacy_screen.c
index beaf99e9120a..03b149cc455b 100644
--- a/drivers/gpu/drm/drm_privacy_screen.c
+++ b/drivers/gpu/drm/drm_privacy_screen.c
@@ -387,7 +387,8 @@ static void drm_privacy_screen_device_release(struct device
*dev)
* * An ERR_PTR(errno) on failure.
*/
struct drm_privacy_screen *drm_privacy_screen_register(
- struct device *parent, const struct drm_privacy_screen_ops *ops)
+ struct device *parent, const struct drm_privacy_screen_ops *ops,
+ void *data)
{
struct drm_privacy_screen *priv;
int ret;
@@ -404,6 +405,7 @@ struct drm_privacy_screen *drm_privacy_screen_register(
priv->dev.parent = parent;
priv->dev.release = drm_privacy_screen_device_release;
dev_set_name(&priv->dev, "privacy_screen-%s", dev_name(parent));
+ priv->drvdata = data;
priv->ops = ops;
priv->ops->get_hw_state(priv);
@@ -439,6 +441,7 @@ void drm_privacy_screen_unregister(struct
drm_privacy_screen *priv)
mutex_unlock(&drm_privacy_screen_devs_lock);
mutex_lock(&priv->lock);
+ priv->drvdata = NULL;
priv->ops = NULL;
mutex_unlock(&priv->lock);
diff --git a/drivers/platform/x86/thinkpad_acpi.c
b/drivers/platform/x86/thinkpad_acpi.c
index 341655d711ce..ccbfda2b0095 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -9782,7 +9782,7 @@ static int tpacpi_lcdshadow_init(struct ibm_init_struct
*iibm)
return 0;
lcdshadow_dev = drm_privacy_screen_register(&tpacpi_pdev->dev,
- &lcdshadow_ops);
+ &lcdshadow_ops, NULL);
if (IS_ERR(lcdshadow_dev))
return PTR_ERR(lcdshadow_dev);
diff --git a/include/drm/drm_privacy_screen_driver.h
b/include/drm/drm_privacy_screen_driver.h
index 24591b607675..4ef246d5706f 100644
--- a/include/drm/drm_privacy_screen_driver.h
+++ b/include/drm/drm_privacy_screen_driver.h
@@ -73,10 +73,21 @@ struct drm_privacy_screen {
* for more info.
*/
enum drm_privacy_screen_status hw_state;
+ /**
+* @drvdata: Private data owned by the privacy screen provider
+*/
+ void *drvdata;
};
+static inline
+void *drm_privacy_screen_get_drvdata(struct drm_privacy_screen *priv)
+{
+ return priv->drvdata;
+}
+
struct drm_privacy_screen *drm_privacy_screen_register(
- struct device *parent, const struct drm_privacy_screen_ops *ops);
+ struct device *parent, const struct drm_privacy_screen_ops *ops,
+ void *data);
void drm_privacy_screen_unregister(struct drm_privacy_screen *priv);
void drm_privacy_screen_call_notifier_chain(struct drm_privacy_screen *priv);
--
2.34.1.575.g55b058a8bb-goog
[PATCH RESEND v2 3/3] drm/vkms: drop "Multiple overlay planes" TODO
Remove the task from the TODO list. Signed-off-by: José Expósito --- Documentation/gpu/vkms.rst | 2 -- 1 file changed, 2 deletions(-) diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst index 941f0e7e5eef..9c873c3912cc 100644 --- a/Documentation/gpu/vkms.rst +++ b/Documentation/gpu/vkms.rst @@ -124,8 +124,6 @@ Add Plane Features There's lots of plane features we could add support for: -- Multiple overlay planes. [Good to get started] - - Clearing primary plane: clear primary plane before plane composition (at the start) for correctness of pixel blend ops. It also guarantees alpha channel is cleared in the target buffer for stable crc. [Good to get started] -- 2.25.1
[PATCH RESEND v2 2/3] drm/vkms: add support for multiple overlay planes
Create 8 overlay planes instead of 1 when the "enable_overlay" module
parameter is set.
The following igt-gpu-tools tests were executed without finding
regressions. Notice than the numbers are identical:
| master branch | this patch |
| SUCCESS | SKIP | FAIL | SUCCESS | SKIP | FAIL |
kms_atomic | 10 | 02 | 00 | 10 | 02 | 00 |
kms_plane_cursor | 09 | 45 | 00 | 09 | 45 | 00 |
kms_plane_multiple | 01 | 23 | 00 | 01 | 23 | 00 |
kms_writeback | 04 | 00 | 00 | 04 | 00 | 00 |
Signed-off-by: José Expósito
---
v2:
- Set the number of overlay planes as a constant instead of as a
module parameter (Melissa Wen)
- Add a test results in the commit message (Melissa Wen)
---
drivers/gpu/drm/vkms/vkms_drv.h| 2 ++
drivers/gpu/drm/vkms/vkms_output.c | 9 ++---
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index d48c23d40ce5..9496fdc900b8 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -20,6 +20,8 @@
#define XRES_MAX 8192
#define YRES_MAX 8192
+#define NUM_OVERLAY_PLANES 8
+
struct vkms_writeback_job {
struct dma_buf_map map[DRM_FORMAT_MAX_PLANES];
struct dma_buf_map data[DRM_FORMAT_MAX_PLANES];
diff --git a/drivers/gpu/drm/vkms/vkms_output.c
b/drivers/gpu/drm/vkms/vkms_output.c
index 2e805b2d36ae..ba0e82ae549a 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -57,15 +57,18 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
struct vkms_plane *primary, *cursor = NULL;
int ret;
int writeback;
+ unsigned int n;
primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
if (IS_ERR(primary))
return PTR_ERR(primary);
if (vkmsdev->config->overlay) {
- ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
- if (ret)
- return ret;
+ for (n = 0; n < NUM_OVERLAY_PLANES; n++) {
+ ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
+ if (ret)
+ return ret;
+ }
}
if (vkmsdev->config->cursor) {
--
2.25.1
[PATCH RESEND v2 1/3] drm/vkms: refactor overlay plane creation
Move the logic to create an overlay plane to its own function.
Refactor, no functional changes.
Signed-off-by: José Expósito
---
drivers/gpu/drm/vkms/vkms_output.c | 26 +++---
1 file changed, 19 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_output.c
b/drivers/gpu/drm/vkms/vkms_output.c
index 04406bd3ff02..2e805b2d36ae 100644
--- a/drivers/gpu/drm/vkms/vkms_output.c
+++ b/drivers/gpu/drm/vkms/vkms_output.c
@@ -32,6 +32,21 @@ static const struct drm_connector_helper_funcs
vkms_conn_helper_funcs = {
.get_modes= vkms_conn_get_modes,
};
+static int vkms_add_overlay_plane(struct vkms_device *vkmsdev, int index,
+ struct drm_crtc *crtc)
+{
+ struct vkms_plane *overlay;
+
+ overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY, index);
+ if (IS_ERR(overlay))
+ return PTR_ERR(overlay);
+
+ if (!overlay->base.possible_crtcs)
+ overlay->base.possible_crtcs = drm_crtc_mask(crtc);
+
+ return 0;
+}
+
int vkms_output_init(struct vkms_device *vkmsdev, int index)
{
struct vkms_output *output = &vkmsdev->output;
@@ -39,7 +54,7 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
struct drm_connector *connector = &output->connector;
struct drm_encoder *encoder = &output->encoder;
struct drm_crtc *crtc = &output->crtc;
- struct vkms_plane *primary, *cursor = NULL, *overlay = NULL;
+ struct vkms_plane *primary, *cursor = NULL;
int ret;
int writeback;
@@ -48,12 +63,9 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index)
return PTR_ERR(primary);
if (vkmsdev->config->overlay) {
- overlay = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_OVERLAY,
index);
- if (IS_ERR(overlay))
- return PTR_ERR(overlay);
-
- if (!overlay->base.possible_crtcs)
- overlay->base.possible_crtcs = drm_crtc_mask(crtc);
+ ret = vkms_add_overlay_plane(vkmsdev, index, crtc);
+ if (ret)
+ return ret;
}
if (vkmsdev->config->cursor) {
--
2.25.1
[PATCH RESEND] drm/doc: Fix TTM acronym
The TTM acronym is defined for the first time in the documentation as "Translation Table Maps". Afterwards, "Translation Table Manager" is used as definition. Fix the first definition to avoid confusion. Signed-off-by: José Expósito --- Documentation/gpu/drm-mm.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst index e0538083a2c0..198bcc1affa1 100644 --- a/Documentation/gpu/drm-mm.rst +++ b/Documentation/gpu/drm-mm.rst @@ -8,7 +8,7 @@ the very dynamic nature of many of that data, managing graphics memory efficiently is thus crucial for the graphics stack and plays a central role in the DRM infrastructure. -The DRM core includes two memory managers, namely Translation Table Maps +The DRM core includes two memory managers, namely Translation Table Manager (TTM) and Graphics Execution Manager (GEM). TTM was the first DRM memory manager to be developed and tried to be a one-size-fits-them all solution. It provides a single userspace API to accommodate the need of -- 2.25.1
Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process
Am 2022-01-07 um 3:56 a.m. schrieb Christian König: > Am 06.01.22 um 17:51 schrieb Felix Kuehling: >> Am 2022-01-06 um 11:48 a.m. schrieb Christian König: >>> Am 06.01.22 um 17:45 schrieb Felix Kuehling: Am 2022-01-06 um 4:05 a.m. schrieb Christian König: [SNIP] Also, why does your ACK or NAK depend on this at all. If it's the right thing to do, it's the right thing to do regardless of who benefits from it. In addition, how can a child process that doesn't even use the GPU be in violation of any GPU-driver related specifications. >>> The argument is that the application is broken and needs to be fixed >>> instead of worked around inside the kernel. >> I still don't get how they the application is broken. Like I said, the >> child process is not using the GPU. How can the application be fixed in >> this case? > > Sounds like I'm still missing some important puzzle pieces for the > full picture to figure out why this doesn't work. > >> Are you saying, any application that forks and doesn't immediately call >> exec is broken? > > More or less yes. We essentially have three possible cases here: > > 1. Application is already using (for example) OpenGL or OpenCL to do > some rendering on the GPU and then calls fork() and expects to use > OpenGL both in the parent and the child at the same time. > As far as I know that's illegal from the Khronos specification > point of view and working around inside the kernel for something not > allowed in the first place is seen as futile effort. > > 2. Application opened the file descriptor, forks and then initializes > OpenGL/Vulkan/OpenCL. > That's what some compositors are doing to drop into the backround > and is explicitly legal. > > 3. Application calls fork() and then doesn't use the GPU in the child. > Either by not using it or calling exec. > That should be legal and not cause any problems in the first place. > > But from your description I still don't get why we are running into > problems here. > > I was assuming that you have case #1 because we previously had some > examples of this with this python library, but from your description > it seems to be case #3. Correct. #3 has at least one issue we previously worked around in the Thunk: The inherited VMAs prevent BOs from being freed in the parent process. This manifests as an apparent memory leak. Therefore the Thunk calls madvise(..., MADV_DONTFORK) on all its VRAM allocation. The BOs that are causing problems with CRIU are GTT BOs that weren't covered by this previous workaround. The new issue with CRIU is, that CRIU saves and restores all the VMAs. When trying to restore the inherited VMAs in the child process, the mmap call fails because the child process' render node FD is no longer inherited from the parent, but is instead created by its own "open" system call. The mmap call in the child fails for at least two reasons: * The child process' render node FD doesn't have permission to map the parent process' BO any more * The parent BO doesn't get restored in the child process, so its mmap offset doesn't get updated to the new mmap offset of the restored parent BO by the amdgpu CRIU plugin We could maybe add a whole bunch of complexity in CRIU and our CRIU plugin to fix this. But it's pointless because like you said, actually doing anything with the BO in the child process is illegal anyway (scenario #1 above). The easiest solution seems to be, to just not inherit the VMA in the first place. Regards, Felix > >> Or does an application that forks need to be aware that some other part >> of the application used the GPU and explicitly free any GPU resources? > > Others might fill that information in, but I think that was the plan > for newer APIs like Vulkan. > > Regards, > Christian. > >> >> Thanks, >> Felix >> >> >>> Regards, >>> Christian. >>> Regards, Felix > Let's talk about this on Mondays call. Thanks for giving the whole > context. > > Regards, > Christian. > >> Regards, >> Felix >> >
Re: [git pull] drm final fixes for 5.16
On Thu, Jan 6, 2022 at 7:23 PM Dave Airlie wrote: > > There is only the amdgpu runtime pm regression fix in here. Thanks, from a quick test it works for me - the backlight actually does eventually go away. It does so only on the second time the monitors say "no signal, going to power save", but that has been true before too. So I think there's still some confusion in this area, but it might be elsewhere - who knows what Wayland and friends do. At least it doesn't look like a regression to me any more. Thanks, Linus
Re: [git pull] drm final fixes for 5.16
The pull request you sent on Fri, 7 Jan 2022 13:23:45 +1000: > git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2022-01-07 has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/7a6043cc2e863ab45016622c30879e23ee13 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html
Re: [PATCH v3 2/6] drm/plane: Fix typo in format_mod_supported documentation
On Friday, January 7th, 2022 at 18:26, José Expósito wrote: > Is there something that needs to improve in the other 4 patches? > Or just waiting on maintainers input? Yeah, these patches look good to me. Feel free to add my R-b. Maintainers for these drivers still need to review/ack/apply them.
Re: [PATCH v3 2/6] drm/plane: Fix typo in format_mod_supported documentation
Hi Simon, On Wed, Jan 05, 2022 at 11:54:43PM +, Simon Ser wrote: > Pushed patches 1 & 2 to drm-misc-next. Thanks for your contribution! Thanks a lot for the review and for applying the changes, appreciate it. Is there something that needs to improve in the other 4 patches? Or just waiting on maintainers input? Thanks, José Expósito
Re: [PATCH v3 2/3] drm/bridge: anx7625: add HDCP support
This series and this patch looks good to me. I'll hold off a few days before merging this in order to give Andrzej some time to have a second look.
Re: [PATCH v2] drm/bridge/tc358775: Fix for dual-link LVDS
On Thu, 6 Jan 2022 at 20:22, Vinay Simha B N wrote:
>
> Reviewed-by: Vinay Simha BN
>
> Jiri Vanek,
> Could you please share the part number or datasheet of the dual-link LVDS
> display/panel used.
>
>
> On Fri, Jan 7, 2022 at 12:30 AM Jiri Vanek wrote:
>>
>> Fixed wrong register shift for single/dual link LVDS output.
>>
>> Tested-by: Jiri Vanek
>> Signed-off-by: Jiri Vanek
>>
>> ---
>> v1:
>> * Initial version
>>
>> v2:
>> * Tested-by tag added
>>
>> ---
>> drivers/gpu/drm/bridge/tc358775.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/tc358775.c
>> b/drivers/gpu/drm/bridge/tc358775.c
>> index 2272adcc5b4a..1d6ec1baeff2 100644
>> --- a/drivers/gpu/drm/bridge/tc358775.c
>> +++ b/drivers/gpu/drm/bridge/tc358775.c
>> @@ -241,7 +241,7 @@ static inline u32 TC358775_LVCFG_PCLKDIV(uint32_t val)
>> }
>>
>> #define TC358775_LVCFG_LVDLINK__MASK 0x0002
>> -#define TC358775_LVCFG_LVDLINK__SHIFT0
>> +#define TC358775_LVCFG_LVDLINK__SHIFT1
>> static inline u32 TC358775_LVCFG_LVDLINK(uint32_t val)
>> {
>> return ((val) << TC358775_LVCFG_LVDLINK__SHIFT) &
>> --
Applied to drm-misc-next
Re: [RFC v2 8/8] drm/amd/virt: Drop concurrent GPU reset protection for SRIOV
On 2022-01-07 12:46 a.m., JingWen Chen wrote: On 2022/1/7 上午11:57, JingWen Chen wrote: On 2022/1/7 上午3:13, Andrey Grodzovsky wrote: On 2022-01-06 12:18 a.m., JingWen Chen wrote: On 2022/1/6 下午12:59, JingWen Chen wrote: On 2022/1/6 上午2:24, Andrey Grodzovsky wrote: On 2022-01-05 2:59 a.m., Christian König wrote: Am 05.01.22 um 08:34 schrieb JingWen Chen: On 2022/1/5 上午12:56, Andrey Grodzovsky wrote: On 2022-01-04 6:36 a.m., Christian König wrote: Am 04.01.22 um 11:49 schrieb Liu, Monk: [AMD Official Use Only] See the FLR request from the hypervisor is just another source of signaling the need for a reset, similar to each job timeout on each queue. Otherwise you have a race condition between the hypervisor and the scheduler. No it's not, FLR from hypervisor is just to notify guest the hw VF FLR is about to start or was already executed, but host will do FLR anyway without waiting for guest too long Then we have a major design issue in the SRIOV protocol and really need to question this. How do you want to prevent a race between the hypervisor resetting the hardware and the client trying the same because of a timeout? As far as I can see the procedure should be: 1. We detect that a reset is necessary, either because of a fault a timeout or signal from hypervisor. 2. For each of those potential reset sources a work item is send to the single workqueue. 3. One of those work items execute first and prepares the reset. 4. We either do the reset our self or notify the hypervisor that we are ready for the reset. 5. Cleanup after the reset, eventually resubmit jobs etc.. 6. Cancel work items which might have been scheduled from other reset sources. It does make sense that the hypervisor resets the hardware without waiting for the clients for too long, but if we don't follow this general steps we will always have a race between the different components. Monk, just to add to this - if indeed as you say that 'FLR from hypervisor is just to notify guest the hw VF FLR is about to start or was already executed, but host will do FLR anyway without waiting for guest too long' and there is no strict waiting from the hypervisor for IDH_READY_TO_RESET to be recived from guest before starting the reset then setting in_gpu_reset and locking reset_sem from guest side is not really full proof protection from MMIO accesses by the guest - it only truly helps if hypervisor waits for that message before initiation of HW reset. Hi Andrey, this cannot be done. If somehow guest kernel hangs and never has the chance to send the response back, then other VFs will have to wait it reset. All the vfs will hang in this case. Or sometimes the mailbox has some delay and other VFs will also wait. The user of other VFs will be affected in this case. Yeah, agree completely with JingWen. The hypervisor is the one in charge here, not the guest. What the hypervisor should do (and it already seems to be designed that way) is to send the guest a message that a reset is about to happen and give it some time to response appropriately. The guest on the other hand then tells the hypervisor that all processing has stopped and it is ready to restart. If that doesn't happen in time the hypervisor should eliminate the guest probably trigger even more severe consequences, e.g. restart the whole VM etc... Christian. So what's the end conclusion here regarding dropping this particular patch ? Seems to me we still need to drop it to prevent driver's MMIO access to the GPU during reset from various places in the code. Andrey Hi Andrey & Christian, I have ported your patch(drop the reset_sem and in_gpu_reset in flr work) and run some tests. If a engine hang during an OCL benchmark(using kfd), we can see the logs below: Did you port the entire patchset or just 'drm/amd/virt: Drop concurrent GPU reset protection for SRIOV' ? I ported the entire patchset [ 397.190727] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 397.301496] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 397.406601] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 397.532343] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 397.642251] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 397.746634] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 397.850761] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 397.960544] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 398.065218] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 398.182173] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 398.288264] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 398.394712] amdgpu :00:07.0: amdgpu: wait for kiq fence error: 0. [ 428.400582] [drm] clean up the vf2pf work item [ 428.500528] amdgpu :00:07.0: amdgpu: [gfxhub] page fault (src_id:0 ring:153 vmid:8 pasid:32771, for process xgemmStandalone pid 3557 thr
Re: [PATCH v6 1/6] drm: move the buddy allocator from i915 into common drm
Am 07.01.22 um 16:49 schrieb Matthew Auld:
On 26/12/2021 22:24, Arunpravin wrote:
Move the base i915 buddy allocator code into drm
- Move i915_buddy.h to include/drm
- Move i915_buddy.c to drm root folder
- Rename "i915" string with "drm" string wherever applicable
- Rename "I915" string with "DRM" string wherever applicable
- Fix header file dependencies
- Fix alignment issues
- add Makefile support for drm buddy
- export functions and write kerneldoc description
- Remove i915 selftest config check condition as buddy selftest
will be moved to drm selftest folder
cleanup i915 buddy references in i915 driver module
and replace with drm buddy
v2:
- include header file in alphabetical order(Thomas)
- merged changes listed in the body section into a single patch
to keep the build intact(Christian, Jani)
v3:
- make drm buddy a separate module(Thomas, Christian)
v4:
- Fix build error reported by kernel test robot
- removed i915 buddy selftest from i915_mock_selftests.h to
avoid build error
- removed selftests/i915_buddy.c file as we create a new set of
buddy test cases in drm/selftests folder
v5:
- Fix merge conflict issue
Signed-off-by: Arunpravin
+int drm_buddy_init(struct drm_buddy_mm *mm, u64 size, u64 chunk_size)
+{
+ unsigned int i;
+ u64 offset;
+
+ if (size < chunk_size)
+ return -EINVAL;
+
+ if (chunk_size < PAGE_SIZE)
+ return -EINVAL;
+
+ if (!is_power_of_2(chunk_size))
+ return -EINVAL;
+
+ size = round_down(size, chunk_size);
+
+ mm->size = size;
+ mm->avail = size;
+ mm->chunk_size = chunk_size;
+ mm->max_order = ilog2(size) - ilog2(chunk_size);
+
+ BUG_ON(mm->max_order > DRM_BUDDY_MAX_ORDER);
+
+ mm->slab_blocks = KMEM_CACHE(drm_buddy_block, 0);
+ if (!mm->slab_blocks)
+ return -ENOMEM;
It looks like every KMEM_CACHE() also creates a debugfs entry? See the
error here[1]. I guess because we end with multiple instances in i915.
If so, is it possible to have a single KMEM_CACHE() as part of the
buddy module, similar to what i915 was doing previously?
Oh, that is a really good point, this code here doesn't make to much sense.
The value of a KMEM_CACHE() is to allow speeding up allocation of the
same structure size between different drm_buddy object. If you allocate
one cache per drm_buddy that makes the whole functionality useless.
Please fix, this is actually a bug.
Christian.
[1]
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fintel-gfx-ci.01.org%2Ftree%2Fdrm-tip%2FTrybot_8217%2Fshard-skl4%2Figt%40i915_selftest%40mock%40memory_region.html&data=04%7C01%7Cchristian.koenig%40amd.com%7C56202fbe886f415c3b8308d9d1f5409c%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637771673545453215%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=ZnRmAQo%2BjX414hbqHigL4R18oBDKLIugUQIVcwhFI%2BY%3D&reserved=0
Re: [PATCH v6 1/6] drm: move the buddy allocator from i915 into common drm
On 26/12/2021 22:24, Arunpravin wrote:
Move the base i915 buddy allocator code into drm
- Move i915_buddy.h to include/drm
- Move i915_buddy.c to drm root folder
- Rename "i915" string with "drm" string wherever applicable
- Rename "I915" string with "DRM" string wherever applicable
- Fix header file dependencies
- Fix alignment issues
- add Makefile support for drm buddy
- export functions and write kerneldoc description
- Remove i915 selftest config check condition as buddy selftest
will be moved to drm selftest folder
cleanup i915 buddy references in i915 driver module
and replace with drm buddy
v2:
- include header file in alphabetical order(Thomas)
- merged changes listed in the body section into a single patch
to keep the build intact(Christian, Jani)
v3:
- make drm buddy a separate module(Thomas, Christian)
v4:
- Fix build error reported by kernel test robot
- removed i915 buddy selftest from i915_mock_selftests.h to
avoid build error
- removed selftests/i915_buddy.c file as we create a new set of
buddy test cases in drm/selftests folder
v5:
- Fix merge conflict issue
Signed-off-by: Arunpravin
+int drm_buddy_init(struct drm_buddy_mm *mm, u64 size, u64 chunk_size)
+{
+ unsigned int i;
+ u64 offset;
+
+ if (size < chunk_size)
+ return -EINVAL;
+
+ if (chunk_size < PAGE_SIZE)
+ return -EINVAL;
+
+ if (!is_power_of_2(chunk_size))
+ return -EINVAL;
+
+ size = round_down(size, chunk_size);
+
+ mm->size = size;
+ mm->avail = size;
+ mm->chunk_size = chunk_size;
+ mm->max_order = ilog2(size) - ilog2(chunk_size);
+
+ BUG_ON(mm->max_order > DRM_BUDDY_MAX_ORDER);
+
+ mm->slab_blocks = KMEM_CACHE(drm_buddy_block, 0);
+ if (!mm->slab_blocks)
+ return -ENOMEM;
It looks like every KMEM_CACHE() also creates a debugfs entry? See the
error here[1]. I guess because we end with multiple instances in i915.
If so, is it possible to have a single KMEM_CACHE() as part of the buddy
module, similar to what i915 was doing previously?
[1]
https://intel-gfx-ci.01.org/tree/drm-tip/Trybot_8217/shard-skl4/igt@i915_selftest@mock@memory_region.html
[PATCH 5/6] drm/meson: add DSI encoder
This adds an encoder bridge designed to drive a MIPI-DSI display
by using the ENCL encoder through the internal MIPI DSI transceiver
connected to the output of the ENCL pixel encoder.
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/meson/Makefile| 2 +-
drivers/gpu/drm/meson/meson_drv.c | 7 +
drivers/gpu/drm/meson/meson_encoder_dsi.c | 159 ++
drivers/gpu/drm/meson/meson_encoder_dsi.h | 12 ++
4 files changed, 179 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/meson/meson_encoder_dsi.c
create mode 100644 drivers/gpu/drm/meson/meson_encoder_dsi.h
diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile
index 3afa31bdc950..833e18c20603 100644
--- a/drivers/gpu/drm/meson/Makefile
+++ b/drivers/gpu/drm/meson/Makefile
@@ -2,7 +2,7 @@
meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_encoder_cvbs.o
meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o
meson_overlay.o
meson-drm-y += meson_rdma.o meson_osd_afbcd.o
-meson-drm-y += meson_encoder_hdmi.o
+meson-drm-y += meson_encoder_hdmi.o meson_encoder_dsi.o
obj-$(CONFIG_DRM_MESON) += meson-drm.o
obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o
diff --git a/drivers/gpu/drm/meson/meson_drv.c
b/drivers/gpu/drm/meson/meson_drv.c
index 80f1d439841a..ff278a2b9e6e 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -33,6 +33,7 @@
#include "meson_registers.h"
#include "meson_encoder_cvbs.h"
#include "meson_encoder_hdmi.h"
+#include "meson_encoder_dsi.h"
#include "meson_viu.h"
#include "meson_vpp.h"
#include "meson_rdma.h"
@@ -323,6 +324,12 @@ static int meson_drv_bind_master(struct device *dev, bool
has_components)
if (ret)
goto free_drm;
+ if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
+ ret = meson_encoder_dsi_init(priv);
+ if (ret)
+ goto free_drm;
+ }
+
ret = meson_plane_create(priv);
if (ret)
goto free_drm;
diff --git a/drivers/gpu/drm/meson/meson_encoder_dsi.c
b/drivers/gpu/drm/meson/meson_encoder_dsi.c
new file mode 100644
index ..90347821cf96
--- /dev/null
+++ b/drivers/gpu/drm/meson/meson_encoder_dsi.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2016 BayLibre, SAS
+ * Author: Neil Armstrong
+ * Copyright (C) 2015 Amlogic, Inc. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "meson_drv.h"
+#include "meson_encoder_dsi.h"
+#include "meson_registers.h"
+#include "meson_venc.h"
+#include "meson_vclk.h"
+
+struct meson_encoder_dsi {
+ struct drm_encoder encoder;
+ struct drm_bridge bridge;
+ struct drm_bridge *next_bridge;
+ struct meson_drm *priv;
+};
+
+#define bridge_to_meson_encoder_dsi(x) \
+ container_of(x, struct meson_encoder_dsi, bridge)
+
+static int meson_encoder_dsi_attach(struct drm_bridge *bridge,
+ enum drm_bridge_attach_flags flags)
+{
+ struct meson_encoder_dsi *encoder_dsi =
bridge_to_meson_encoder_dsi(bridge);
+
+ return drm_bridge_attach(bridge->encoder, encoder_dsi->next_bridge,
+&encoder_dsi->bridge, flags);
+}
+
+static void meson_encoder_dsi_mode_set(struct drm_bridge *bridge,
+ const struct drm_display_mode *mode,
+ const struct drm_display_mode
*adjusted_mode)
+{
+ struct meson_encoder_dsi *encoder_dsi =
bridge_to_meson_encoder_dsi(bridge);
+ struct meson_drm *priv = encoder_dsi->priv;
+
+ meson_vclk_setup(priv, MESON_VCLK_TARGET_DSI, mode->clock, 0, 0, 0,
false);
+
+ meson_venc_mipi_dsi_mode_set(priv, mode);
+ meson_encl_load_gamma(priv);
+
+ writel_relaxed(0, priv->io_base + _REG(ENCL_VIDEO_EN));
+
+ writel_bits_relaxed(BIT(3), BIT(3), priv->io_base +
_REG(ENCL_VIDEO_MODE_ADV));
+ writel_relaxed(0, priv->io_base + _REG(ENCL_TST_EN));
+}
+
+static void meson_encoder_dsi_atomic_enable(struct drm_bridge *bridge,
+ struct drm_bridge_state
*bridge_state)
+{
+ struct meson_encoder_dsi *encoder_dsi =
bridge_to_meson_encoder_dsi(bridge);
+ struct meson_drm *priv = encoder_dsi->priv;
+
+ writel_bits_relaxed(BIT(0), 0, priv->io_base +
_REG(VPP_WRAP_OSD1_MATRIX_EN_CTRL));
+
+ writel_relaxed(1, priv->io_base + _REG(ENCL_VIDEO_EN));
+}
+
+static void meson_encoder_dsi_atomic_disable(struct drm_bridge *bridge,
+struct drm_bridge_state
*bridge_state)
+{
+ struct meson_encoder_dsi *meson_encoder_dsi =
+ bridge_to_meson_encoder_dsi(bridge);
+ struct meson_drm *priv = meson_encoder_dsi->priv;
+
+ writel_relaxed(0
[PATCH 6/6] drm/meson: add support for MIPI-DSI transceiver
The Amlogic G12A/G12B/SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver
1.21a),
with a custom glue managing the IP resets, clock and data input similar to the
DW-HDMI
Glue on other Amlogic SoCs.
This adds support for the Glue managing the transceiver, mimicing the init flow
provided
by Amlogic to setup the ENCL encoder, the glue, the transceiver, the digital
D-PHY and the
Analog PHY in the proper way.
An optional "MEAS" clock can be enabled to measure the delay between each vsync
feeding the
DW-MIPI-DSI transceiver.
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/meson/Kconfig | 7 +
drivers/gpu/drm/meson/Makefile| 1 +
drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 383 ++
drivers/gpu/drm/meson/meson_dw_mipi_dsi.h | 115 +++
4 files changed, 506 insertions(+)
create mode 100644 drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
create mode 100644 drivers/gpu/drm/meson/meson_dw_mipi_dsi.h
diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig
index 6c70fc3214af..71a1364b51e1 100644
--- a/drivers/gpu/drm/meson/Kconfig
+++ b/drivers/gpu/drm/meson/Kconfig
@@ -17,3 +17,10 @@ config DRM_MESON_DW_HDMI
default y if DRM_MESON
select DRM_DW_HDMI
imply DRM_DW_HDMI_I2S_AUDIO
+
+config DRM_MESON_DW_MIPI_DSI
+ tristate "MIPI DSI Synopsys Controller support for Amlogic Meson
Display"
+ depends on DRM_MESON
+ default y if DRM_MESON
+ select DRM_DW_MIPI_DSI
+ select GENERIC_PHY_MIPI_DPHY
diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile
index 833e18c20603..43071bdbd4b9 100644
--- a/drivers/gpu/drm/meson/Makefile
+++ b/drivers/gpu/drm/meson/Makefile
@@ -6,3 +6,4 @@ meson-drm-y += meson_encoder_hdmi.o meson_encoder_dsi.o
obj-$(CONFIG_DRM_MESON) += meson-drm.o
obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o
+obj-$(CONFIG_DRM_MESON_DW_MIPI_DSI) += meson_dw_mipi_dsi.o
diff --git a/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
new file mode 100644
index ..75af3eec1b74
--- /dev/null
+++ b/drivers/gpu/drm/meson/meson_dw_mipi_dsi.c
@@ -0,0 +1,383 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2021 BayLibre, SAS
+ * Author: Neil Armstrong
+ * Copyright (C) 2015 Amlogic, Inc. All rights reserved.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#include "meson_drv.h"
+#include "meson_dw_mipi_dsi.h"
+#include "meson_registers.h"
+#include "meson_venc.h"
+
+#define DRIVER_NAME "meson-dw-mipi-dsi"
+#define DRIVER_DESC "Amlogic Meson MIPI-DSI DRM driver"
+
+/* MIPI DSI/VENC Color Format Definitions */
+#define MIPI_DSI_VENC_COLOR_30B 0x0
+#define MIPI_DSI_VENC_COLOR_24B 0x1
+#define MIPI_DSI_VENC_COLOR_18B 0x2
+#define MIPI_DSI_VENC_COLOR_16B 0x3
+
+#define COLOR_16BIT_CFG_1 0x0
+#define COLOR_16BIT_CFG_2 0x1
+#define COLOR_16BIT_CFG_3 0x2
+#define COLOR_18BIT_CFG_1 0x3
+#define COLOR_18BIT_CFG_2 0x4
+#define COLOR_24BIT 0x5
+#define COLOR_20BIT_LOOSE 0x6
+#define COLOR_24_BIT_YCBCR0x7
+#define COLOR_16BIT_YCBCR 0x8
+#define COLOR_30BIT 0x9
+#define COLOR_36BIT 0xa
+#define COLOR_12BIT 0xb
+#define COLOR_RGB_111 0xc
+#define COLOR_RGB_332 0xd
+#define COLOR_RGB_444 0xe
+
+/* MIPI DSI Relative REGISTERs Definitions */
+/* For MIPI_DSI_TOP_CNTL */
+#define BIT_DPI_COLOR_MODE20
+#define BIT_IN_COLOR_MODE 16
+#define BIT_CHROMA_SUBSAMPLE 14
+#define BIT_COMP2_SEL 12
+#define BIT_COMP1_SEL 10
+#define BIT_COMP0_SEL 8
+#define BIT_DE_POL 6
+#define BIT_HSYNC_POL 5
+#define BIT_VSYNC_POL 4
+#define BIT_DPICOLORM 3
+#define BIT_DPISHUTDN 2
+#define BIT_EDPITE_INTR_PULSE 1
+#define BIT_ERR_INTR_PULSE 0
+
+struct meson_dw_mipi_dsi {
+ struct meson_drm *priv;
+ struct device *dev;
+ void __iomem *base;
+ struct phy *phy;
+ union phy_configure_opts phy_opts;
+ struct dw_mipi_dsi *dmd;
+ struct dw_mipi_dsi_plat_data pdata;
+ struct mipi_dsi_device *dsi_device;
+ const struct drm_display_mode *mode;
+ struct clk *px_clk;
+};
+
+#define encoder_to_meson_dw_mipi_dsi(x) \
+ container_of(x, struct meson_dw_mipi_dsi, encoder)
+
+static void meson_dw_mipi_dsi_hw_init(struct meson_dw_mipi_dsi *mipi_dsi)
+{
+ writel_relaxed((1 << 4) | (1 << 5) | (0 << 6),
+ mipi_dsi->base + MIPI_DSI_TOP_CNTL);
+
+ writel_bits_relaxed(0xf, 0xf, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET);
+ writel_bits_relaxed(0xf, 0, mipi_dsi->base + MIPI_DSI_TOP_SW_RESET);
+
+ writel_bits_relaxed(0x3, 0x3, mipi_ds
[PATCH 3/6] drm/meson: venc: add ENCL encoder setup for MIPI-DSI output
This adds supports for the ENCL encoder connected to a MIPI-DSI transceiver on
the
Amlogic AXG SoCs.
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/meson/meson_venc.c | 230 -
drivers/gpu/drm/meson/meson_venc.h | 6 +
drivers/gpu/drm/meson/meson_vpp.h | 2 +
3 files changed, 236 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/meson/meson_venc.c
b/drivers/gpu/drm/meson/meson_venc.c
index 3c55ed003359..b430dc06aa34 100644
--- a/drivers/gpu/drm/meson/meson_venc.c
+++ b/drivers/gpu/drm/meson/meson_venc.c
@@ -6,6 +6,7 @@
*/
#include
+#include
#include
@@ -1557,6 +1558,224 @@ void meson_venc_hdmi_mode_set(struct meson_drm *priv,
int vic,
}
EXPORT_SYMBOL_GPL(meson_venc_hdmi_mode_set);
+static unsigned short meson_encl_gamma_table[256] = {
+ 0, 4, 8, 12, 16, 20, 24, 28, 32, 36, 40, 44, 48, 52, 56, 60,
+ 64, 68, 72, 76, 80, 84, 88, 92, 96, 100, 104, 108, 112, 116, 120, 124,
+ 128, 132, 136, 140, 144, 148, 152, 156, 160, 164, 168, 172, 176, 180,
184, 188,
+ 192, 196, 200, 204, 208, 212, 216, 220, 224, 228, 232, 236, 240, 244,
248, 252,
+ 256, 260, 264, 268, 272, 276, 280, 284, 288, 292, 296, 300, 304, 308,
312, 316,
+ 320, 324, 328, 332, 336, 340, 344, 348, 352, 356, 360, 364, 368, 372,
376, 380,
+ 384, 388, 392, 396, 400, 404, 408, 412, 416, 420, 424, 428, 432, 436,
440, 444,
+ 448, 452, 456, 460, 464, 468, 472, 476, 480, 484, 488, 492, 496, 500,
504, 508,
+ 512, 516, 520, 524, 528, 532, 536, 540, 544, 548, 552, 556, 560, 564,
568, 572,
+ 576, 580, 584, 588, 592, 596, 600, 604, 608, 612, 616, 620, 624, 628,
632, 636,
+ 640, 644, 648, 652, 656, 660, 664, 668, 672, 676, 680, 684, 688, 692,
696, 700,
+ 704, 708, 712, 716, 720, 724, 728, 732, 736, 740, 744, 748, 752, 756,
760, 764,
+ 768, 772, 776, 780, 784, 788, 792, 796, 800, 804, 808, 812, 816, 820,
824, 828,
+ 832, 836, 840, 844, 848, 852, 856, 860, 864, 868, 872, 876, 880, 884,
888, 892,
+ 896, 900, 904, 908, 912, 916, 920, 924, 928, 932, 936, 940, 944, 948,
952, 956,
+ 960, 964, 968, 972, 976, 980, 984, 988, 992, 996, 1000, 1004, 1008,
1012, 1016, 1020,
+};
+
+#define GAMMA_VCOM_POL7 /* RW */
+#define GAMMA_RVS_OUT 6 /* RW */
+#define ADR_RDY 5 /* Read Only */
+#define WR_RDY4 /* Read Only */
+#define RD_RDY3 /* Read Only */
+#define GAMMA_TR 2 /* RW */
+#define GAMMA_SET 1 /* RW */
+#define GAMMA_EN 0 /* RW */
+
+#define H_RD 12
+#define H_AUTO_INC11
+#define H_SEL_R 10
+#define H_SEL_G 9
+#define H_SEL_B 8
+#define HADR_MSB 7/* 7:0 */
+#define HADR 0/* 7:0 */
+
+#define GAMMA_RETRY 1000
+
+static void meson_encl_set_gamma_table(struct meson_drm *priv, u16 *data,
+ u32 rgb_mask)
+{
+ int i, ret;
+ u32 reg;
+
+ writel_bits_relaxed(BIT(GAMMA_EN), 0,
+ priv->io_base + _REG(L_GAMMA_CNTL_PORT));
+
+ ret = readl_relaxed_poll_timeout(priv->io_base +
+ _REG(L_GAMMA_CNTL_PORT),
+reg, reg & BIT(ADR_RDY), 10, 1);
+ if (ret)
+ pr_warn("%s: GAMMA ADR_RDY timeout\n", __func__);
+
+ writel_relaxed(BIT(H_AUTO_INC) |
+ BIT(rgb_mask) |
+ (0 << HADR),
+ priv->io_base + _REG(L_GAMMA_ADDR_PORT));
+
+ for (i = 0; i < 256; i++) {
+ ret = readl_relaxed_poll_timeout(priv->io_base +
+ _REG(L_GAMMA_CNTL_PORT),
+reg, reg & BIT(WR_RDY),
+10, 1);
+ if (ret)
+ pr_warn_once("%s: GAMMA WR_RDY timeout\n", __func__);
+
+ writel_relaxed(data[i],
+ priv->io_base + _REG(L_GAMMA_DATA_PORT));
+ }
+
+ ret = readl_relaxed_poll_timeout(priv->io_base +
+ _REG(L_GAMMA_CNTL_PORT),
+reg, reg & BIT(ADR_RDY), 10, 1);
+ if (ret)
+ pr_warn("%s: GAMMA ADR_RDY timeout\n", __func__);
+
+ writel_relaxed(BIT(H_AUTO_INC) |
+ BIT(rgb_mask) |
+ (0x23 << HADR),
+ priv->io_base + _REG(L_GAMMA_ADDR_PORT));
+}
+
+void meson_encl_load_gamma(struct meson_drm *priv)
+{
+ meson_encl_set_gamma_table(priv, meson_encl_gamma_table, H_SEL_R);
+ meson_encl_set_gamma_table(priv, meson_encl_gamma_table, H_SEL_G);
+ meson_encl_set_gamma_table(priv, meson_encl_gamma_table, H_SEL_B);
+
+ writel_bits_relaxed(BIT(GAMMA_EN), BIT(GAMMA_EN),
+
[PATCH 4/6] drm/meson: vclk: add DSI clock config
The DSI path used the ENCL pixel encoder, thus this adds a clock
config using the HDMI PLL in order to feed the ENCL encoder via the
VCLK2 path and the CTS_ENCL clock output.
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/meson/meson_vclk.c | 47 ++
drivers/gpu/drm/meson/meson_vclk.h | 1 +
2 files changed, 48 insertions(+)
diff --git a/drivers/gpu/drm/meson/meson_vclk.c
b/drivers/gpu/drm/meson/meson_vclk.c
index 2a82119eb58e..5e4d982be1c8 100644
--- a/drivers/gpu/drm/meson/meson_vclk.c
+++ b/drivers/gpu/drm/meson/meson_vclk.c
@@ -55,6 +55,8 @@
#define VCLK2_DIV_MASK 0xff
#define VCLK2_DIV_EN BIT(16)
#define VCLK2_DIV_RESETBIT(17)
+#define CTS_ENCL_SEL_MASK (0xf << 12)
+#define CTS_ENCL_SEL_SHIFT 12
#define CTS_VDAC_SEL_MASK (0xf << 28)
#define CTS_VDAC_SEL_SHIFT 28
#define HHI_VIID_CLK_CNTL 0x12c /* 0x4b offset in data sheet */
@@ -83,6 +85,7 @@
#define VCLK_DIV12_EN BIT(4)
#define HHI_VID_CLK_CNTL2 0x194 /* 0x65 offset in data sheet */
#define CTS_ENCI_ENBIT(0)
+#define CTS_ENCL_ENBIT(3)
#define CTS_ENCP_ENBIT(2)
#define CTS_VDAC_ENBIT(4)
#define HDMI_TX_PIXEL_EN BIT(5)
@@ -1024,6 +1027,47 @@ static void meson_vclk_set(struct meson_drm *priv,
unsigned int pll_base_freq,
regmap_update_bits(priv->hhi, HHI_VID_CLK_CNTL, VCLK_EN, VCLK_EN);
}
+static void meson_dsi_clock_config(struct meson_drm *priv, unsigned int freq)
+{
+ meson_hdmi_pll_generic_set(priv, freq * 10);
+
+ /* Setup vid_pll divider value /5 */
+ meson_vid_pll_set(priv, VID_PLL_DIV_5);
+
+ /* Disable VCLK2 */
+ regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_EN, 0);
+
+ /* Setup the VCLK2 divider value /2 */
+ regmap_update_bits(priv->hhi, HHI_VIID_CLK_DIV, VCLK2_DIV_MASK, 2 - 1);
+
+ /* select vid_pll for vclk2 */
+ regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL,
+ VCLK2_SEL_MASK, (0 << VCLK2_SEL_SHIFT));
+
+ /* enable vclk2 gate */
+ regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_EN, VCLK2_EN);
+
+ /* select vclk2_div1 for encl */
+ regmap_update_bits(priv->hhi, HHI_VIID_CLK_DIV,
+ CTS_ENCL_SEL_MASK, (8 << CTS_ENCL_SEL_SHIFT));
+
+ /* release vclk2_div_reset and enable vclk2_div */
+ regmap_update_bits(priv->hhi, HHI_VIID_CLK_DIV, VCLK2_DIV_EN |
VCLK2_DIV_RESET,
+ VCLK2_DIV_EN);
+
+ /* enable vclk2_div1 gate */
+ regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_DIV1_EN,
VCLK2_DIV1_EN);
+
+ /* reset vclk2 */
+ regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_SOFT_RESET,
VCLK2_SOFT_RESET);
+ regmap_update_bits(priv->hhi, HHI_VIID_CLK_CNTL, VCLK2_SOFT_RESET, 0);
+
+ /* enable encl_clk */
+ regmap_update_bits(priv->hhi, HHI_VID_CLK_CNTL2, CTS_ENCL_EN,
CTS_ENCL_EN);
+
+ usleep_range(1, 11000);
+}
+
void meson_vclk_setup(struct meson_drm *priv, unsigned int target,
unsigned int phy_freq, unsigned int vclk_freq,
unsigned int venc_freq, unsigned int dac_freq,
@@ -1050,6 +1094,9 @@ void meson_vclk_setup(struct meson_drm *priv, unsigned
int target,
meson_vclk_set(priv, phy_freq, 0, 0, 0,
VID_PLL_DIV_5, 2, 1, 1, false, false);
return;
+ } else if (target == MESON_VCLK_TARGET_DSI) {
+ meson_dsi_clock_config(priv, phy_freq);
+ return;
}
hdmi_tx_div = vclk_freq / dac_freq;
diff --git a/drivers/gpu/drm/meson/meson_vclk.h
b/drivers/gpu/drm/meson/meson_vclk.h
index 60617aaf18dd..1152b3af8d2e 100644
--- a/drivers/gpu/drm/meson/meson_vclk.h
+++ b/drivers/gpu/drm/meson/meson_vclk.h
@@ -17,6 +17,7 @@ enum {
MESON_VCLK_TARGET_CVBS = 0,
MESON_VCLK_TARGET_HDMI = 1,
MESON_VCLK_TARGET_DMT = 2,
+ MESON_VCLK_TARGET_DSI = 3,
};
/* 27MHz is the CVBS Pixel Clock */
--
2.25.1
[PATCH 2/6] dt-bindings: display: meson-vpu: add third DPI output port
Add third port corresponding to the ENCL DPI encoder used to connect to DSI or LVDS transceivers. Signed-off-by: Neil Armstrong --- .../devicetree/bindings/display/amlogic,meson-vpu.yaml | 5 + 1 file changed, 5 insertions(+) diff --git a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml index 851cb0781217..525a01a38568 100644 --- a/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml +++ b/Documentation/devicetree/bindings/display/amlogic,meson-vpu.yaml @@ -92,6 +92,11 @@ properties: description: A port node pointing to the HDMI-TX port node. + port@2: +$ref: /schemas/graph.yaml#/properties/port +description: + A port node pointing to the DPI port node (e.g. DSI or LVDS transceiver). + "#address-cells": const: 1 -- 2.25.1
[PATCH 1/6] dt-bindings: display: add Amlogic MIPI DSI Host Controller bindings
The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver
(ver 1.21a),
with a custom glue managing the IP resets, clock and data input similar to the
DW-HDMI Glue
on the same Amlogic SoCs.
Signed-off-by: Neil Armstrong
---
.../display/amlogic,meson-dw-mipi-dsi.yaml| 118 ++
1 file changed, 118 insertions(+)
create mode 100644
Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml
diff --git
a/Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml
b/Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml
new file mode 100644
index ..f3070783d606
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml
@@ -0,0 +1,118 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 BayLibre, SAS
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/display/amlogic,meson-dw-mipi-dsi.yaml#";
+$schema: "http://devicetree.org/meta-schemas/core.yaml#";
+
+title: Amlogic specific extensions to the Synopsys Designware MIPI DSI Host
Controller
+
+maintainers:
+ - Neil Armstrong
+
+description: |
+ The Amlogic Meson Synopsys Designware Integration is composed of
+ - A Synopsys DesignWare MIPI DSI Host Controller IP
+ - A TOP control block controlling the Clocks & Resets of the IP
+
+allOf:
+ - $ref: dsi-controller.yaml#
+
+properties:
+ compatible:
+enum:
+ - amlogic,meson-g12a-dw-mipi-dsi
+
+ reg:
+maxItems: 1
+
+ clocks:
+minItems: 2
+
+ clock-names:
+minItems: 2
+items:
+ - const: pclk
+ - const: px_clk
+ - const: meas_clk
+
+ resets:
+minItems: 1
+
+ reset-names:
+items:
+ - const: top
+
+ phys:
+minItems: 1
+
+ phy-names:
+items:
+ - const: dphy
+
+ ports:
+$ref: /schemas/graph.yaml#/properties/ports
+
+properties:
+ port@0:
+$ref: /schemas/graph.yaml#/$defs/port-base
+unevaluatedProperties: false
+description: Input node to receive pixel data.
+
+ port@1:
+$ref: /schemas/graph.yaml#/$defs/port-base
+unevaluatedProperties: false
+description: DSI output node to panel.
+
+required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - resets
+ - reset-names
+ - phys
+ - phy-names
+ - ports
+
+unevaluatedProperties: false
+
+examples:
+ - |
+dsi@7000 {
+ compatible = "amlogic,meson-g12a-dw-mipi-dsi";
+ reg = <0x6000 0x400>;
+ resets = <&reset_top>;
+ reset-names = "top";
+ clocks = <&clk_pclk>, <&clk_px>;
+ clock-names = "pclk", "px_clk";
+ phys = <&mipi_dphy>;
+ phy-names = "dphy";
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ /* VPU VENC Input */
+ mipi_dsi_venc_port: port@0 {
+ reg = <0>;
+
+ mipi_dsi_in: endpoint {
+ remote-endpoint = <&dpi_out>;
+ };
+ };
+
+ /* DSI Output */
+ mipi_dsi_panel_port: port@1 {
+ reg = <1>;
+
+ mipi_out_panel: endpoint {
+ remote-endpoint = <&mipi_in_panel>;
+ };
+ };
+ };
+};
--
2.25.1
[PATCH 0/6] drm/meson: add support for MIPI DSI Display
The Amlogic G12A, G12B & SM1 SoCs embeds a Synopsys DW-MIPI-DSI transceiver (ver 1.21a), with a custom glue managing the IP resets, clock and data input similar to the DW-HDMI glue on the same Amlogic SoCs. This adds support for the glue managing the transceiver, mimicing the init flow provided by Amlogic to setup the ENCL encoder, the glue, the transceiver, the digital D-PHY and the Analog PHY in the proper way. The DW-MIPI-DSI transceiver + D-PHY are clocked by the GP0 PLL, and the ENCL encoder + VIU pixel reader by the VCLK2 clock using the HDMI PLL. The DW-MIPI-DSI transceiver gets this pixel stream as input clocked with the VCLK2 clock. An optional "MEAS" clock can be enabled to measure the delay between each vsync feeding the DW-MIPI-DSI transceiver. This patchset is based on an earlier attempt at [1] for the AXG SoCs, but: - the AXG has a single clock source for both transceiver + pixel, which makes it an exception instead of a rule, it's simpler to add support for G12A then add AXG on it - previous glue code was a single monolitic code mixing encoders & bridges, this version is aligned on the previous cleanup done on HDMI & CVBS bridges architecture at [2] - since the only output of AXG is DSI, AXG VPU support is post-poned until we implement single-clock DSI support specific case on top of this. [1] https://lore.kernel.org/r/20200907081825.1654-1-narmstr...@baylibre.com [2] https://lore.kernel.org/r/20211020123947.2585572-1-narmstr...@baylibre.com Neil Armstrong (6): dt-bindings: display: add Amlogic MIPI DSI Host Controller bindings dt-bindings: display: meson-vpu: add third DPI output port drm/meson: venc: add ENCL encoder setup for MIPI-DSI output drm/meson: vclk: add DSI clock config drm/meson: add DSI encoder drm/meson: add support for MIPI-DSI transceiver .../display/amlogic,meson-dw-mipi-dsi.yaml| 118 ++ .../bindings/display/amlogic,meson-vpu.yaml | 5 + drivers/gpu/drm/meson/Kconfig | 7 + drivers/gpu/drm/meson/Makefile| 3 +- drivers/gpu/drm/meson/meson_drv.c | 7 + drivers/gpu/drm/meson/meson_dw_mipi_dsi.c | 383 ++ drivers/gpu/drm/meson/meson_dw_mipi_dsi.h | 115 ++ drivers/gpu/drm/meson/meson_encoder_dsi.c | 159 drivers/gpu/drm/meson/meson_encoder_dsi.h | 12 + drivers/gpu/drm/meson/meson_vclk.c| 47 +++ drivers/gpu/drm/meson/meson_vclk.h| 1 + drivers/gpu/drm/meson/meson_venc.c| 230 ++- drivers/gpu/drm/meson/meson_venc.h| 6 + drivers/gpu/drm/meson/meson_vpp.h | 2 + 14 files changed, 1092 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/amlogic,meson-dw-mipi-dsi.yaml create mode 100644 drivers/gpu/drm/meson/meson_dw_mipi_dsi.c create mode 100644 drivers/gpu/drm/meson/meson_dw_mipi_dsi.h create mode 100644 drivers/gpu/drm/meson/meson_encoder_dsi.c create mode 100644 drivers/gpu/drm/meson/meson_encoder_dsi.h -- 2.25.1
[PATCH v6 6/6] drm/i915: Use struct vma_resource instead of struct vma_snapshot
There is always a struct vma_resource guaranteed to be alive when we
access a corresponding struct vma_snapshot.
So ditch the latter and instead of allocating vma_snapshots, reference
the already existning vma_resource.
This requires a couple of extra members in struct vma_resource but that's
a small price to pay for the simplification.
v2:
- Fix a missing include and declaration (kernel test robot )
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/Makefile | 1 -
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 15 +--
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 9 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 87 ++--
drivers/gpu/drm/i915/i915_request.c | 12 +-
drivers/gpu/drm/i915/i915_request.h | 6 +-
drivers/gpu/drm/i915/i915_vma.c | 16 +--
drivers/gpu/drm/i915/i915_vma_resource.c | 4 +
drivers/gpu/drm/i915/i915_vma_resource.h | 28 +++-
drivers/gpu/drm/i915/i915_vma_snapshot.c | 125 --
drivers/gpu/drm/i915/i915_vma_snapshot.h | 101 --
11 files changed, 90 insertions(+), 314 deletions(-)
delete mode 100644 drivers/gpu/drm/i915/i915_vma_snapshot.c
delete mode 100644 drivers/gpu/drm/i915/i915_vma_snapshot.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 98433ad74194..aa86ac33effc 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -175,7 +175,6 @@ i915-y += \
i915_ttm_buddy_manager.o \
i915_vma.o \
i915_vma_resource.o \
- i915_vma_snapshot.o \
intel_wopcm.o
# general-purpose microcontroller (GuC) support
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 020e56a58096..5f2ca39d9482 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -29,7 +29,6 @@
#include "i915_gem_ioctls.h"
#include "i915_trace.h"
#include "i915_user_extensions.h"
-#include "i915_vma_snapshot.h"
struct eb_vma {
struct i915_vma *vma;
@@ -1952,7 +1951,6 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
{
const unsigned int count = eb->buffer_count;
unsigned int i = count, j;
- struct i915_vma_snapshot *vsnap;
while (i--) {
struct eb_vma *ev = &eb->vma[i];
@@ -1962,11 +1960,6 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
if (!(flags & EXEC_OBJECT_CAPTURE))
continue;
- vsnap = i915_vma_snapshot_alloc(GFP_KERNEL);
- if (!vsnap)
- continue;
-
- i915_vma_snapshot_init(vsnap, vma, "user");
for_each_batch_create_order(eb, j) {
struct i915_capture_list *capture;
@@ -1975,10 +1968,9 @@ static void eb_capture_stage(struct i915_execbuffer *eb)
continue;
capture->next = eb->capture_lists[j];
- capture->vma_snapshot = i915_vma_snapshot_get(vsnap);
+ capture->vma_res = i915_vma_resource_get(vma->resource);
eb->capture_lists[j] = capture;
}
- i915_vma_snapshot_put(vsnap);
}
}
@@ -3281,9 +3273,8 @@ eb_requests_create(struct i915_execbuffer *eb, struct
dma_fence *in_fence,
* _onstack interface.
*/
if (eb->batches[i]->vma)
-
i915_vma_snapshot_init_onstack(&eb->requests[i]->batch_snapshot,
- eb->batches[i]->vma,
- "batch");
+ eb->requests[i]->batch_res =
+
i915_vma_resource_get(eb->batches[i]->vma->resource);
if (eb->batch_pool) {
GEM_BUG_ON(intel_context_is_parallel(eb->context));
intel_gt_buffer_pool_mark_active(eb->batch_pool,
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 74aa90587061..d1daa4cc2895 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -1708,18 +1708,15 @@ static void intel_engine_print_registers(struct
intel_engine_cs *engine,
static void print_request_ring(struct drm_printer *m, struct i915_request *rq)
{
- struct i915_vma_snapshot *vsnap = &rq->batch_snapshot;
+ struct i915_vma_resource *vma_res = rq->batch_res;
void *ring;
int size;
- if (!i915_vma_snapshot_present(vsnap))
- vsnap = NULL;
-
drm_printf(m,
"[head %04x, postfix %04x, tail %04x, batch 0x%08x_%08x]:\n",
rq->head, rq->postfix, rq->tail,
- vsnap ? upper_32_bits(vsnap->vma_reso
[PATCH v6 5/6] drm/i915: Asynchronous migration selftest
Add a selftest to exercise asynchronous migration and -unbining.
Extend the gem_migrate selftest to perform the migrations while
depending on a spinner and a bound vma set up on the migrated
buffer object.
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_object.c| 12 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 3 +
.../drm/i915/gem/selftests/i915_gem_migrate.c | 192 --
3 files changed, 192 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c
b/drivers/gpu/drm/i915/gem/i915_gem_object.c
index d87b508b59b1..1a9e1f940a7d 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c
@@ -756,6 +756,18 @@ i915_gem_object_get_moving_fence(struct
drm_i915_gem_object *obj)
return dma_fence_get(i915_gem_to_ttm(obj)->moving);
}
+void i915_gem_object_set_moving_fence(struct drm_i915_gem_object *obj,
+ struct dma_fence *fence)
+{
+ struct dma_fence **moving = &i915_gem_to_ttm(obj)->moving;
+
+ if (*moving == fence)
+ return;
+
+ dma_fence_put(*moving);
+ *moving = dma_fence_get(fence);
+}
+
/**
* i915_gem_object_wait_moving_fence - Wait for the object's moving fence if
any
* @obj: The object whose moving fence to wait for.
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h
b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index f66d46882ea7..1d178236 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -524,6 +524,9 @@ i915_gem_object_finish_access(struct drm_i915_gem_object
*obj)
struct dma_fence *
i915_gem_object_get_moving_fence(struct drm_i915_gem_object *obj);
+void i915_gem_object_set_moving_fence(struct drm_i915_gem_object *obj,
+ struct dma_fence *fence);
+
int i915_gem_object_wait_moving_fence(struct drm_i915_gem_object *obj,
bool intr);
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c
index ecb691c81d1e..d534141b2cf7 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_migrate.c
@@ -4,8 +4,13 @@
*/
#include "gt/intel_migrate.h"
+#include "gt/intel_gpu_commands.h"
#include "gem/i915_gem_ttm_move.h"
+#include "i915_deps.h"
+
+#include "selftests/igt_spinner.h"
+
static int igt_fill_check_buffer(struct drm_i915_gem_object *obj,
bool fill)
{
@@ -101,7 +106,8 @@ static int igt_same_create_migrate(void *arg)
}
static int lmem_pages_migrate_one(struct i915_gem_ww_ctx *ww,
- struct drm_i915_gem_object *obj)
+ struct drm_i915_gem_object *obj,
+ struct i915_vma *vma)
{
int err;
@@ -109,6 +115,24 @@ static int lmem_pages_migrate_one(struct i915_gem_ww_ctx
*ww,
if (err)
return err;
+ if (vma) {
+ err = i915_vma_pin_ww(vma, ww, obj->base.size, 0,
+ 0UL | PIN_OFFSET_FIXED |
+ PIN_USER);
+ if (err) {
+ if (err != -EINTR && err != ERESTARTSYS &&
+ err != -EDEADLK)
+ pr_err("Failed to pin vma.\n");
+ return err;
+ }
+
+ i915_vma_unpin(vma);
+ }
+
+ /*
+* Migration will implicitly unbind (asynchronously) any bound
+* vmas.
+*/
if (i915_gem_object_is_lmem(obj)) {
err = i915_gem_object_migrate(obj, ww, INTEL_REGION_SMEM);
if (err) {
@@ -149,11 +173,15 @@ static int lmem_pages_migrate_one(struct i915_gem_ww_ctx
*ww,
return err;
}
-static int igt_lmem_pages_migrate(void *arg)
+static int __igt_lmem_pages_migrate(struct intel_gt *gt,
+ struct i915_address_space *vm,
+ struct i915_deps *deps,
+ struct igt_spinner *spin,
+ struct dma_fence *spin_fence)
{
- struct intel_gt *gt = arg;
struct drm_i915_private *i915 = gt->i915;
struct drm_i915_gem_object *obj;
+ struct i915_vma *vma = NULL;
struct i915_gem_ww_ctx ww;
struct i915_request *rq;
int err;
@@ -165,6 +193,14 @@ static int igt_lmem_pages_migrate(void *arg)
if (IS_ERR(obj))
return PTR_ERR(obj);
+ if (vm) {
+ vma = i915_vma_instance(obj, vm, NULL);
+ if (IS_ERR(vma)) {
+ err = PTR_ERR(vma);
+ goto out_put;
+ }
+ }
+
/* Initial GPU fill, sync, CPU initialization. */
f
[PATCH v6 4/6] drm/i915: Use vma resources for async unbinding
Implement async (non-blocking) unbinding by not syncing the vma before
calling unbind on the vma_resource.
Add the resulting unbind fence to the object's dma_resv from where it is
picked up by the ttm migration code.
Ideally these unbind fences should be coalesced with the migration blit
fence to avoid stalling the migration blit waiting for unbind, as they
can certainly go on in parallel, but since we don't yet have a
reasonable data structure to use to coalesce fences and attach the
resulting fence to a timeline, we defer that for now.
Note that with async unbinding, even while the unbind waits for the
preceding bind to complete before unbinding, the vma itself might have been
destroyed in the process, clearing the vma pages. Therefore we can
only allow async unbinding if we have a refcounted sg-list and keep a
refcount on that for the vma resource pages to stay intact until
binding occurs. If this condition is not met, a request for an async
unbind is diverted to a sync unbind.
v2:
- Use a separate kmem_cache for vma resources for now to isolate their
memory allocation and aid debugging.
- Move the check for vm closed to the actual unbinding thread. Regardless
of whether the vm is closed, we need the unbind fence to properly wait
for capture.
- Clear vma_res::vm on unbind and update its documentation.
v4:
- Take cache coloring into account when searching for vma resources
pending unbind. (Matthew Auld)
v5:
- Fix timeout and error check in i915_vma_resource_bind_dep_await().
- Avoid taking a reference on the object for async binding if
async unbind capable.
- Fix braces around a single-line if statement.
v6:
- Fix up the cache coloring adjustment. (Kernel test robot )
- Don't allow async unbinding if the vma_res pages are not the same as
the object pages.
Signed-off-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 11 +-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +-
drivers/gpu/drm/i915/gt/intel_gtt.c | 4 +
drivers/gpu/drm/i915/gt/intel_gtt.h | 3 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 12 +-
drivers/gpu/drm/i915/i915_module.c | 3 +
drivers/gpu/drm/i915/i915_vma.c | 205 +--
drivers/gpu/drm/i915/i915_vma.h | 3 +-
drivers/gpu/drm/i915/i915_vma_resource.c | 354 +--
drivers/gpu/drm/i915/i915_vma_resource.h | 48 +++
11 files changed, 579 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
index 8653855d808b..1de306c03aaf 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c
@@ -142,7 +142,16 @@ int i915_ttm_move_notify(struct ttm_buffer_object *bo)
struct drm_i915_gem_object *obj = i915_ttm_to_gem(bo);
int ret;
- ret = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE);
+ /*
+* Note: The async unbinding here will actually transform the
+* blocking wait for unbind into a wait before finally submitting
+* evict / migration blit and thus stall the migration timeline
+* which may not be good for overall throughput. We should make
+* sure we await the unbind fences *after* the migration blit
+* instead of *before* as we currently do.
+*/
+ ret = i915_gem_object_unbind(obj, I915_GEM_OBJECT_UNBIND_ACTIVE |
+I915_GEM_OBJECT_UNBIND_ASYNC);
if (ret)
return ret;
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index e49b6250c4b7..a1b2761bc16e 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -142,7 +142,7 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm)
continue;
if (!i915_vma_is_bound(vma, I915_VMA_GLOBAL_BIND)) {
- __i915_vma_evict(vma);
+ __i915_vma_evict(vma, false);
drm_mm_remove_node(&vma->node);
}
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c
b/drivers/gpu/drm/i915/gt/intel_gtt.c
index a94be0306464..46be4197b93f 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -161,6 +161,9 @@ static void __i915_vm_release(struct work_struct *work)
struct i915_address_space *vm =
container_of(work, struct i915_address_space, release_work);
+ /* Synchronize async unbinds. */
+ i915_vma_resource_bind_dep_sync_all(vm);
+
vm->cleanup(vm);
i915_address_space_fini(vm);
@@ -189,6 +192,7 @@ void i915_address_space_init(struct i915_address_space *vm,
int subclass)
if (!kref_read(&vm->resv_ref))
kref_init(&vm->resv_ref);
+ vm->pending_unbind = RB_ROOT_CACHED;
[PATCH v6 3/6] drm/i915: Don't pin the object pages during pending vma binds
A pin-count is already held by vma->pages so taking an additional pin
during async binds is not necessary.
When we introduce async unbinding we have other means of keeping the
object pages alive.
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/i915_vma.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 1d4e448d22d9..8fa3e0b2fe26 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -305,10 +305,8 @@ static void __vma_release(struct dma_fence_work *work)
{
struct i915_vma_work *vw = container_of(work, typeof(*vw), base);
- if (vw->pinned) {
- __i915_gem_object_unpin_pages(vw->pinned);
+ if (vw->pinned)
i915_gem_object_put(vw->pinned);
- }
i915_vm_free_pt_stash(vw->vm, &vw->stash);
i915_vm_put(vw->vm);
@@ -477,7 +475,6 @@ int i915_vma_bind(struct i915_vma *vma,
work->base.dma.error = 0; /* enable the queue_work() */
- __i915_gem_object_pin_pages(vma->obj);
work->pinned = i915_gem_object_get(vma->obj);
} else {
if (vma->obj) {
--
2.31.1
[PATCH v6 2/6] drm/i915: Use the vma resource as argument for gtt binding / unbinding
When introducing asynchronous unbinding, the vma itself may no longer
be alive when the actual binding or unbinding takes place.
Update the gtt i915_vma_ops accordingly to take a struct i915_vma_resource
instead of a struct i915_vma for the bind_vma() and unbind_vma() ops.
Similarly change the insert_entries() op for struct i915_address_space.
Replace a couple of i915_vma_snapshot members with their newly introduced
i915_vma_resource counterparts, since they have the same lifetime.
Also make sure to avoid changing the struct i915_vma_flags (in particular
the bind flags) async. That should now only be done sync under the
vm mutex.
v2:
- Update the vma_res::bound_flags when binding to the aliased ggtt
v6:
- Remove I915_VMA_ALLOC_BIT (Matthew Auld)
- Change some members of struct i915_vma_resource from unsigned long to u64
(Matthew Auld)
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/display/intel_dpt.c | 27 ++---
.../gpu/drm/i915/gem/i915_gem_object_types.h | 27 +
.../gpu/drm/i915/gem/selftests/huge_pages.c | 37 +++
drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 19 ++--
drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 37 +++
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 4 +-
drivers/gpu/drm/i915/gt/intel_ggtt.c | 70 ++---
drivers/gpu/drm/i915/gt/intel_gtt.h | 16 +--
drivers/gpu/drm/i915/gt/intel_ppgtt.c | 22 +++--
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 13 ++-
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +-
drivers/gpu/drm/i915/i915_debugfs.c | 3 +-
drivers/gpu/drm/i915/i915_gpu_error.c | 6 +-
drivers/gpu/drm/i915/i915_vma.c | 24 -
drivers/gpu/drm/i915/i915_vma.h | 11 +--
drivers/gpu/drm/i915/i915_vma_resource.c | 9 +-
drivers/gpu/drm/i915/i915_vma_resource.h | 99 ++-
drivers/gpu/drm/i915/i915_vma_snapshot.c | 4 -
drivers/gpu/drm/i915/i915_vma_snapshot.h | 8 --
drivers/gpu/drm/i915/i915_vma_types.h | 14 ++-
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 64
drivers/gpu/drm/i915/selftests/mock_gtt.c | 12 +--
22 files changed, 314 insertions(+), 214 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dpt.c
b/drivers/gpu/drm/i915/display/intel_dpt.c
index 8f674745e7e0..63a83d5f85a1 100644
--- a/drivers/gpu/drm/i915/display/intel_dpt.c
+++ b/drivers/gpu/drm/i915/display/intel_dpt.c
@@ -48,7 +48,7 @@ static void dpt_insert_page(struct i915_address_space *vm,
}
static void dpt_insert_entries(struct i915_address_space *vm,
- struct i915_vma *vma,
+ struct i915_vma_resource *vma_res,
enum i915_cache_level level,
u32 flags)
{
@@ -64,8 +64,8 @@ static void dpt_insert_entries(struct i915_address_space *vm,
* not to allow the user to override access to a read only page.
*/
- i = vma->node.start / I915_GTT_PAGE_SIZE;
- for_each_sgt_daddr(addr, sgt_iter, vma->pages)
+ i = vma_res->start / I915_GTT_PAGE_SIZE;
+ for_each_sgt_daddr(addr, sgt_iter, vma_res->bi.pages)
gen8_set_pte(&base[i++], pte_encode | addr);
}
@@ -76,35 +76,38 @@ static void dpt_clear_range(struct i915_address_space *vm,
static void dpt_bind_vma(struct i915_address_space *vm,
struct i915_vm_pt_stash *stash,
-struct i915_vma *vma,
+struct i915_vma_resource *vma_res,
enum i915_cache_level cache_level,
u32 flags)
{
- struct drm_i915_gem_object *obj = vma->obj;
u32 pte_flags;
+ if (vma_res->bound_flags)
+ return;
+
/* Applicable to VLV (gen8+ do not support RO in the GGTT) */
pte_flags = 0;
- if (vma->vm->has_read_only && i915_gem_object_is_readonly(obj))
+ if (vm->has_read_only && vma_res->bi.readonly)
pte_flags |= PTE_READ_ONLY;
- if (i915_gem_object_is_lmem(obj))
+ if (vma_res->bi.lmem)
pte_flags |= PTE_LM;
- vma->vm->insert_entries(vma->vm, vma, cache_level, pte_flags);
+ vm->insert_entries(vm, vma_res, cache_level, pte_flags);
- vma->page_sizes.gtt = I915_GTT_PAGE_SIZE;
+ vma_res->page_sizes_gtt = I915_GTT_PAGE_SIZE;
/*
* Without aliasing PPGTT there's no difference between
* GLOBAL/LOCAL_BIND, it's all the same ptes. Hence unconditionally
* upgrade to both bound if we bind either to avoid double-binding.
*/
- atomic_or(I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND, &vma->flags);
+ vma_res->bound_flags = I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND;
}
-static void dpt_unbind_vma(struct i915_address_space *vm, struct i915_vma *vma)
+static void dpt_unbind_vma(struct i915_address_space *vm,
+
[PATCH v6 1/6] drm/i915: Initial introduction of vma resources
Introduce vma resources, sort of similar to TTM resources, needed for
asynchronous bind management. Initially we will use them to hold
completion of unbinding when we capture data from a vma, but they will
be used extensively in upcoming patches for asynchronous vma unbinding.
v6:
- Some documentation updates
Signed-off-by: Thomas Hellström
Reviewed-by: Matthew Auld
---
drivers/gpu/drm/i915/Makefile | 1 +
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 2 +-
drivers/gpu/drm/i915/i915_vma.c | 55 +++-
drivers/gpu/drm/i915/i915_vma.h | 19 ++-
drivers/gpu/drm/i915/i915_vma_resource.c | 124 ++
drivers/gpu/drm/i915/i915_vma_resource.h | 69 ++
drivers/gpu/drm/i915/i915_vma_snapshot.c | 15 +--
drivers/gpu/drm/i915/i915_vma_snapshot.h | 7 +-
drivers/gpu/drm/i915/i915_vma_types.h | 5 +
drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 99 --
10 files changed, 333 insertions(+), 63 deletions(-)
create mode 100644 drivers/gpu/drm/i915/i915_vma_resource.c
create mode 100644 drivers/gpu/drm/i915/i915_vma_resource.h
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 1b62b9f65196..98433ad74194 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -174,6 +174,7 @@ i915-y += \
i915_trace_points.o \
i915_ttm_buddy_manager.o \
i915_vma.o \
+ i915_vma_resource.o \
i915_vma_snapshot.o \
intel_wopcm.o
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 5ecc85b96a3d..020e56a58096 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -1422,7 +1422,7 @@ eb_relocate_entry(struct i915_execbuffer *eb,
mutex_lock(&vma->vm->mutex);
err = i915_vma_bind(target->vma,
target->vma->obj->cache_level,
- PIN_GLOBAL, NULL);
+ PIN_GLOBAL, NULL, NULL);
mutex_unlock(&vma->vm->mutex);
reloc_cache_remap(&eb->reloc_cache, ev->vma->obj);
if (err)
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index be208a8f1ed0..7097c5016431 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -37,6 +37,7 @@
#include "i915_sw_fence_work.h"
#include "i915_trace.h"
#include "i915_vma.h"
+#include "i915_vma_resource.h"
static struct kmem_cache *slab_vmas;
@@ -380,6 +381,8 @@ static int i915_vma_verify_bind_complete(struct i915_vma
*vma)
* @cache_level: mapping cache level
* @flags: flags like global or local mapping
* @work: preallocated worker for allocating and binding the PTE
+ * @vma_res: pointer to a preallocated vma resource. The resource is either
+ * consumed or freed.
*
* DMA addresses are taken from the scatter-gather table of this object (or of
* this VMA in case of non-default GGTT views) and PTE entries set up.
@@ -388,7 +391,8 @@ static int i915_vma_verify_bind_complete(struct i915_vma
*vma)
int i915_vma_bind(struct i915_vma *vma,
enum i915_cache_level cache_level,
u32 flags,
- struct i915_vma_work *work)
+ struct i915_vma_work *work,
+ struct i915_vma_resource *vma_res)
{
u32 bind_flags;
u32 vma_flags;
@@ -399,11 +403,15 @@ int i915_vma_bind(struct i915_vma *vma,
if (GEM_DEBUG_WARN_ON(range_overflows(vma->node.start,
vma->node.size,
- vma->vm->total)))
+ vma->vm->total))) {
+ kfree(vma_res);
return -ENODEV;
+ }
- if (GEM_DEBUG_WARN_ON(!flags))
+ if (GEM_DEBUG_WARN_ON(!flags)) {
+ kfree(vma_res);
return -EINVAL;
+ }
bind_flags = flags;
bind_flags &= I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND;
@@ -412,11 +420,21 @@ int i915_vma_bind(struct i915_vma *vma,
vma_flags &= I915_VMA_GLOBAL_BIND | I915_VMA_LOCAL_BIND;
bind_flags &= ~vma_flags;
- if (bind_flags == 0)
+ if (bind_flags == 0) {
+ kfree(vma_res);
return 0;
+ }
GEM_BUG_ON(!atomic_read(&vma->pages_count));
+ if (vma->resource || !vma_res) {
+ /* Rebinding with an additional I915_VMA_*_BIND */
+ GEM_WARN_ON(!vma_flags);
+ kfree(vma_res);
+ } else {
+ i915_vma_resource_init(vma_res);
+ vma->resource = vma_res;
+ }
trace_i915_vma_bind(vma, bind_flags);
if (work && bind_flags & vma->v
[PATCH v6 0/6] drm/i915: Asynchronous vma unbinding
This patch series introduces infrastructure for asynchronous vma unbinding. The single enabled use-case is initially at buffer object migration where we otherwise sync when unbinding vmas before migration. This in theory allows us to pipeline any number of migrations, but in practice the number is restricted by a sync wait when filling the migration context ring. We might want to look at that moving forward if needed. The other main use-case is to be able to pipeline vma evictions, for example with softpinning where a new vma wants to reuse the vm range of an already active vma. We can't support this just yet because we need dma_resv locking around vma eviction for that, which is under implementation. Patch 1 introduces vma resource first for error capture purposes Patch 2 changes the vm backend interface to take vma resources rather than vmas Patch 3 removes and unneeded page pinning Patch 4 introduces the async unbinding itself, and finally Patch 5 introduces a selftest Patch 6 realizes we have duplicated functionality and removes the vma snapshots v2: -- Some kernel test robot reports addressed. -- kmem cache for vma resources, See patch 4 -- Various fixes all over the place. See separate commit messages. v3: -- Re-add a missing i915_vma_resource_put() -- Remove a stray debug printout v4: -- Patch series split in two. This is the second part. -- Take cache coloring into account when searching for vma_resources pending unbind. (Matthew Auld) v5: -- Add a selftest. -- Remove page pinning while sync binding. -- A couple of fixes in i915_vma_resource_bind_dep_await() v6: -- Some documentation updates -- Remove I915_VMA_ALLOC_BIT (Matthew Auld) -- Change some members of struct i915_vma_resource from unsigned long to u64 (Matthew Auld) -- Fix up the cache coloring adjustment. (Kernel test robot ) -- Don't allow async unbinding if the vma_res pages are not the same as the object pages. (Matthew Auld) Thomas Hellström (6): drm/i915: Initial introduction of vma resources drm/i915: Use the vma resource as argument for gtt binding / unbinding drm/i915: Don't pin the object pages during pending vma binds drm/i915: Use vma resources for async unbinding drm/i915: Asynchronous migration selftest drm/i915: Use struct vma_resource instead of struct vma_snapshot drivers/gpu/drm/i915/Makefile | 2 +- drivers/gpu/drm/i915/display/intel_dpt.c | 27 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 17 +- drivers/gpu/drm/i915/gem/i915_gem_object.c| 12 + drivers/gpu/drm/i915/gem/i915_gem_object.h| 3 + .../gpu/drm/i915/gem/i915_gem_object_types.h | 27 +- drivers/gpu/drm/i915/gem/i915_gem_ttm_move.c | 11 +- .../gpu/drm/i915/gem/selftests/huge_pages.c | 37 +- .../drm/i915/gem/selftests/i915_gem_migrate.c | 192 +++- drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 19 +- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 37 +- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 9 +- drivers/gpu/drm/i915/gt/intel_ggtt.c | 72 +-- drivers/gpu/drm/i915/gt/intel_gtt.c | 4 + drivers/gpu/drm/i915/gt/intel_gtt.h | 19 +- drivers/gpu/drm/i915/gt/intel_ppgtt.c | 22 +- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 13 +- drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +- drivers/gpu/drm/i915/i915_debugfs.c | 3 +- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_gem.c | 12 +- drivers/gpu/drm/i915/i915_gpu_error.c | 87 ++-- drivers/gpu/drm/i915/i915_module.c| 3 + drivers/gpu/drm/i915/i915_request.c | 12 +- drivers/gpu/drm/i915/i915_request.h | 6 +- drivers/gpu/drm/i915/i915_vma.c | 241 +- drivers/gpu/drm/i915/i915_vma.h | 33 +- drivers/gpu/drm/i915/i915_vma_resource.c | 417 ++ drivers/gpu/drm/i915/i915_vma_resource.h | 234 ++ drivers/gpu/drm/i915/i915_vma_snapshot.c | 134 -- drivers/gpu/drm/i915/i915_vma_snapshot.h | 112 - drivers/gpu/drm/i915/i915_vma_types.h | 19 +- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 159 --- drivers/gpu/drm/i915/selftests/mock_gtt.c | 12 +- 34 files changed, 1421 insertions(+), 589 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_vma_resource.c create mode 100644 drivers/gpu/drm/i915/i915_vma_resource.h delete mode 100644 drivers/gpu/drm/i915/i915_vma_snapshot.c delete mode 100644 drivers/gpu/drm/i915/i915_vma_snapshot.h -- 2.31.1
Re: [PATCH v5 03/32] component: Move struct aggregate_device out to header file
On Thu, 06 Jan 2022, Stephen Boyd wrote:
> This allows aggregate driver writers to use the device passed to their
> probe/remove/shutdown functions properly instead of treating it as an
> opaque pointer.
You say it like having opaque pointers with interfaces instead of
exposed data is a bad thing.
Data is not an interface. IMO if you can get by with keeping the types
private, go for it. Unless I'm missing something, you only need the
parent dev pointer. Maybe add a helper function for it?
It's trivial to expose the guts like this, but it's usually a lot of
hard work to go the other way. Look at the dependencies of component.h
now. To keep it self-contained, i.e. buildable without implicit
dependencies, you'd need to add #include , which goes on to
include the world. Then have a look at [1].
Please at least let's not do this lightly.
BR,
Jani.
[1] https://lwn.net/ml/linux-kernel/ydifz+lmewets...@gmail.com/
>
> Cc: Daniel Vetter
> Cc: Greg Kroah-Hartman
> Cc: Laurent Pinchart
> Cc: "Rafael J. Wysocki"
> Cc: Rob Clark
> Cc: Russell King
> Cc: Saravana Kannan
> Signed-off-by: Stephen Boyd
> ---
> drivers/base/component.c | 15 ---
> include/linux/component.h | 19 ---
> 2 files changed, 16 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/base/component.c b/drivers/base/component.c
> index eec82caeae5e..dc38a8939ae6 100644
> --- a/drivers/base/component.c
> +++ b/drivers/base/component.c
> @@ -56,21 +56,6 @@ struct component_match {
> struct component_match_array *compare;
> };
>
> -struct aggregate_device {
> - const struct component_master_ops *ops;
> - struct device *parent;
> - struct device dev;
> - struct component_match *match;
> - struct aggregate_driver *adrv;
> -
> - int id;
> -};
> -
> -static inline struct aggregate_device *to_aggregate_device(struct device *d)
> -{
> - return container_of(d, struct aggregate_device, dev);
> -}
> -
> struct component {
> struct list_head node;
> struct aggregate_device *adev;
> diff --git a/include/linux/component.h b/include/linux/component.h
> index 95d1b23ede8a..e99cf8e910f0 100644
> --- a/include/linux/component.h
> +++ b/include/linux/component.h
> @@ -5,6 +5,8 @@
> #include
> #include
>
> +struct component_match;
> +
> /**
> * struct component_ops - callbacks for component drivers
> *
> @@ -39,8 +41,6 @@ void component_del(struct device *, const struct
> component_ops *);
> int component_bind_all(struct device *master, void *master_data);
> void component_unbind_all(struct device *master, void *master_data);
>
> -struct aggregate_device;
> -
> /**
> * struct component_master_ops - callback for the aggregate driver
> *
> @@ -80,7 +80,20 @@ struct component_master_ops {
> void (*unbind)(struct device *master);
> };
>
> -struct component_match;
> +struct aggregate_device {
> + const struct component_master_ops *ops;
> + struct device *parent;
> + struct device dev;
> + struct component_match *match;
> + struct aggregate_driver *adrv;
> +
> + int id;
> +};
> +
> +static inline struct aggregate_device *to_aggregate_device(struct device *d)
> +{
> + return container_of(d, struct aggregate_device, dev);
> +}
>
> /**
> * struct aggregate_driver - Aggregate driver (made up of other drivers)
--
Jani Nikula, Intel Open Source Graphics Center
[Bug 215001] Regression in 5.15, Firmware-initialized graphics console selects FB_VGA16, screen corruption
https://bugzilla.kernel.org/show_bug.cgi?id=215001 --- Comment #9 from The Linux kernel's regression tracker (Thorsten Leemhuis) (regressi...@leemhuis.info) --- (In reply to Kris Karas from comment #7) > > ... I filed this bug nearly 2 months ago with the "Regression = Y" metadata > clearly set; but the various kernel regression trackers Which trackers? Humans or software? I'm running a bot, but that is still in the early stages -- and has no bugzilla integration as of now (but it's on the todo list, but only in the middle), as the kernel's docs discourage from using it most of the time. See https://www.kernel.org/doc/html/latest/admin-guide/reporting-issues.html (written by yours truly). See also the last section of that document. > What's the best way to get a bugzilla report > for a regression tracked so that it doesn't get forgotten or lost in the > shuffle? Docs on that are in the work: https://lore.kernel.org/linux-doc/cover.1641203216.git.li...@leemhuis.info/ > For example, bug 199533 was filed back in 2018, with a proper > bisect even, but hasn't garnered even so much as a single comment. > (Probably ought to be closed as I no longer have the hardware to test a fix > :-) There are many things wrt kernel bug/regression reporting that IMHO need to be fixed or improved. I'm working on this, but I'm mostly doing it in my spare time (except for the regzbot, for which I found external funding). -- You may reply to this email to add a comment. You are receiving this mail because: You are on the CC list for the bug.
[Bug 215001] Regression in 5.15, Firmware-initialized graphics console selects FB_VGA16, screen corruption
https://bugzilla.kernel.org/show_bug.cgi?id=215001 --- Comment #8 from Javier Martinez Canillas (jav...@dowhile0.org) --- Hello Kris, (In reply to Kris Karas from comment #7) > Hi Javier - I tested the (updated) patch from comment 6 on three different > systems, two servers with a character-graphic BIOS (expected to use VGA16), > and my development system with a graphical UEFI boot (expected to use > EFIFB). I am happy to report that in all cases, the patch worked perfectly. > > Thanks for having whipped up this patch! > > Feel free to submit upstream to Linus, and also to Greg for inclusion in > 5.15.y. > Tested-By: Kris Karas > Thanks for testing the patch! I've just posted it along with another fix for a bug I noticed by reading the code (EGA is never used and the driver always assume to be VGA): https://lkml.org/lkml/2022/1/7/270 -- You may reply to this email to add a comment. You are receiving this mail because: You are on the CC list for the bug.
[PATCH 0/2] video: A couple of fixes for the vga16fb driver
This patch series contains two fixes for the vga16fb driver. I looked at
the driver due a regression reported [0], caused by commit d391c5827107
("drivers/firmware: move x86 Generic System Framebuffers support").
The mentioned commit didn't change any logic but just moved the platform
device registration that matches the vesafb and efifb drivers to happen
later. And this caused the vga16fb driver to be probed even in machines
that don't have an EGA or VGA video adapter.
Patch #1 is fixing the wrong check to determine if either EGA or VGA is
used and patch #2 adds a check to the driver to only be loaded for EGA
and VGA 16 color graphic cards.
[0]: https://bugzilla.kernel.org/show_bug.cgi?id=215001
Best regards,
Javier
Javier Martinez Canillas (2):
video: vga16fb: Fix logic that checks for the display standard
video: vga16fb: Only probe for EGA and VGA 16 color graphic cards
drivers/video/fbdev/vga16fb.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
--
2.33.1
[PATCH 2/2] video: vga16fb: Only probe for EGA and VGA 16 color graphic cards
The vga16fb framebuffer driver only supports Enhanced Graphics Adapter
(EGA) and Video Graphics Array (VGA) 16 color graphic cards.
But it doesn't check if the adapter is one of those or if a VGA16 mode
is used. This means that the driver will be probed even if a VESA BIOS
Extensions (VBE) or Graphics Output Protocol (GOP) interface is used.
This issue has been present for a long time but it was only exposed by
commit d391c5827107 ("drivers/firmware: move x86 Generic System
Framebuffers support") since the platform device registration to match
the {vesa,efi}fb drivers is done later as a consequence of that change.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=215001
Fixes: d391c5827107 ("drivers/firmware: move x86 Generic System Framebuffers
support")
Reported-by: Kris Karas
Cc: # 5.15.x
Signed-off-by: Javier Martinez Canillas
Tested-by: Kris Karas
---
drivers/video/fbdev/vga16fb.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index 3347c9b6a332..72b6aeceeff8 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -1422,6 +1422,18 @@ static int __init vga16fb_init(void)
vga16fb_setup(option);
#endif
+
+ /* only EGA and VGA in 16 color graphic mode are supported */
+ if (screen_info.orig_video_isVGA != VIDEO_TYPE_EGAC &&
+ screen_info.orig_video_isVGA != VIDEO_TYPE_VGAC)
+ return -ENODEV;
+
+ if (screen_info.orig_video_mode != 0x0D && /* 320x200/4 (EGA) */
+ screen_info.orig_video_mode != 0x0E && /* 640x200/4 (EGA) */
+ screen_info.orig_video_mode != 0x10 && /* 640x350/4 (EGA) */
+ screen_info.orig_video_mode != 0x12)/* 640x480/4 (VGA) */
+ return -ENODEV;
+
ret = platform_driver_register(&vga16fb_driver);
if (!ret) {
--
2.33.1
[PATCH 1/2] video: vga16fb: Fix logic that checks for the display standard
The vga16fb framebuffer driver supports both Enhanced Graphics Adapter (EGA) and Video Graphics Array (VGA) 16 color graphic cards. But the logic to check whether the EGA or VGA standard are used is not correct. It just checks if screen_info.orig_video_isVGA is set, but it should check if is set to VIDEO_TYPE_VGAC instead. This means that it assumes to be VGA even if is set to VIDEO_TYPE_EGAC. Signed-off-by: Javier Martinez Canillas --- drivers/video/fbdev/vga16fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c index e2757ff1c23d..3347c9b6a332 100644 --- a/drivers/video/fbdev/vga16fb.c +++ b/drivers/video/fbdev/vga16fb.c @@ -1332,7 +1332,7 @@ static int vga16fb_probe(struct platform_device *dev) printk(KERN_INFO "vga16fb: mapped to 0x%p\n", info->screen_base); par = info->par; - par->isVGA = screen_info.orig_video_isVGA; + par->isVGA = screen_info.orig_video_isVGA == VIDEO_TYPE_VGAC; par->palette_blanked = 0; par->vesa_blanked = 0; -- 2.33.1
Re: [PATCH v2] drm/v3d: Fix PM disable depth imbalance in v3d_platform_drm_probe
On Thu, 6 Jan 2022 at 12:47, Miaoqian Lin wrote:
>
> The pm_runtime_enable will increase power disable depth.
> If the probe fails, we should use pm_runtime_disable() to balance
> pm_runtime_enable().
>
> Fixes: 57692c9 ("drm/v3d: Introduce a new DRM driver for Broadcom V3D V3.x+")
> Signed-off-by: Miaoqian Lin
Thanks for the update - that looks good to me.
Reviewed-by: Dave Stevenson
> ---
> Changes in v2
> - put pm_runtime_disable before dma_free_wc
> - rename dma_free to pm_disable
> ---
> drivers/gpu/drm/v3d/v3d_drv.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
> index bd46396a1ae0..7d500dd5314e 100644
> --- a/drivers/gpu/drm/v3d/v3d_drv.c
> +++ b/drivers/gpu/drm/v3d/v3d_drv.c
> @@ -282,7 +282,7 @@ static int v3d_platform_drm_probe(struct platform_device
> *pdev)
>
> ret = v3d_gem_init(drm);
> if (ret)
> - goto dma_free;
> + goto pm_disable;
>
> ret = v3d_irq_init(v3d);
> if (ret)
> @@ -298,7 +298,8 @@ static int v3d_platform_drm_probe(struct platform_device
> *pdev)
> v3d_irq_disable(v3d);
> gem_destroy:
> v3d_gem_destroy(drm);
> -dma_free:
> +pm_disable:
> + pm_runtime_disable(dev);
> dma_free_wc(dev, 4096, v3d->mmu_scratch, v3d->mmu_scratch_paddr);
> return ret;
> }
> --
> 2.17.1
>
Re: [PATCH] drm/dp: Remove common Post Cursor2 register handling
On Wed, 05 Jan 2022, Kees Cook wrote:
> On Wed, Jan 05, 2022 at 08:00:50PM +0200, Jani Nikula wrote:
>> On Wed, 05 Jan 2022, Kees Cook wrote:
>> > The link_status array was not large enough to read the Adjust Request
>> > Post Cursor2 register, so remove the common helper function to avoid
>> > an OOB read, found with a -Warray-bounds build:
>> >
>> > drivers/gpu/drm/drm_dp_helper.c: In function
>> > 'drm_dp_get_adjust_request_post_cursor':
>> > drivers/gpu/drm/drm_dp_helper.c:59:27: error: array subscript 10 is
>> > outside array bounds of 'const u8[6]' {aka 'const unsigned char[6]'}
>> > [-Werror=array-bounds]
>> >59 | return link_status[r - DP_LANE0_1_STATUS];
>> > |~~~^~~
>> > drivers/gpu/drm/drm_dp_helper.c:147:51: note: while referencing
>> > 'link_status'
>> > 147 | u8 drm_dp_get_adjust_request_post_cursor(const u8
>> > link_status[DP_LINK_STATUS_SIZE],
>> > |
>> > ~^~~~
>> >
>> > Replace the only user of the helper with an open-coded fetch and decode,
>> > similar to drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c.
>> >
>> > Fixes: 79465e0ffeb9 ("drm/dp: Add helper to get post-cursor adjustments")
>> > Cc: Maarten Lankhorst
>> > Cc: Maxime Ripard
>> > Cc: Thomas Zimmermann
>> > Cc: David Airlie
>> > Cc: Daniel Vetter
>> > Cc: dri-devel@lists.freedesktop.org
>> > Signed-off-by: Kees Cook
>> > ---
>> > This is the alternative to:
>> > https://lore.kernel.org/lkml/20211203084354.3105253-1-keesc...@chromium.org/
>> > ---
>> > drivers/gpu/drm/drm_dp_helper.c | 10 --
>> > drivers/gpu/drm/tegra/dp.c | 11 ++-
>> > include/drm/drm_dp_helper.h | 2 --
>> > 3 files changed, 10 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
>> > b/drivers/gpu/drm/drm_dp_helper.c
>> > index 23f9073bc473..c9528aa62c9c 100644
>> > --- a/drivers/gpu/drm/drm_dp_helper.c
>> > +++ b/drivers/gpu/drm/drm_dp_helper.c
>> > @@ -144,16 +144,6 @@ u8 drm_dp_get_adjust_tx_ffe_preset(const u8
>> > link_status[DP_LINK_STATUS_SIZE],
>> > }
>> > EXPORT_SYMBOL(drm_dp_get_adjust_tx_ffe_preset);
>> >
>> > -u8 drm_dp_get_adjust_request_post_cursor(const u8
>> > link_status[DP_LINK_STATUS_SIZE],
>> > - unsigned int lane)
>> > -{
>> > - unsigned int offset = DP_ADJUST_REQUEST_POST_CURSOR2;
>> > - u8 value = dp_link_status(link_status, offset);
>> > -
>> > - return (value >> (lane << 1)) & 0x3;
>> > -}
>> > -EXPORT_SYMBOL(drm_dp_get_adjust_request_post_cursor);
>> > -
>> > static int __8b10b_clock_recovery_delay_us(const struct drm_dp_aux *aux,
>> > u8 rd_interval)
>> > {
>> >if (rd_interval > 4)
>> > diff --git a/drivers/gpu/drm/tegra/dp.c b/drivers/gpu/drm/tegra/dp.c
>> > index 70dfb7d1dec5..f5535eb04c6b 100644
>> > --- a/drivers/gpu/drm/tegra/dp.c
>> > +++ b/drivers/gpu/drm/tegra/dp.c
>> > @@ -549,6 +549,15 @@ static void drm_dp_link_get_adjustments(struct
>> > drm_dp_link *link,
>> > {
>> >struct drm_dp_link_train_set *adjust = &link->train.adjust;
>> >unsigned int i;
>> > + u8 post_cursor;
>> > + int err;
>> > +
>> > + err = drm_dp_dpcd_read(link->aux, DP_ADJUST_REQUEST_POST_CURSOR2,
>> > + &post_cursor, sizeof(post_cursor));
>>
>> There's a drm_dp_dpcd_readb() for the common 1-byte reads. Other than
>> that,
>>
>> Reviewed-by: Jani Nikula
>
> Thanks!
>
>>
>> Though obviously that's not enough to actually merge to tegra.
>
> As in, "a review by Jani isn't sufficient to land via the tegra tree"?
Yeah. Or, in this case, via any tree, really.
> What should next steps be?
Get an ack from tegra and/or drm-misc maintainers. All the relevant
folks and lists are in the recipients already.
BR,
Jani.
>
> -Kees
>
>>
>> > + if (err < 0) {
>> > + DRM_ERROR("failed to read post_cursor2: %d\n", err);
>> > + post_cursor = 0;
>> > + }
>> >
>> >for (i = 0; i < link->lanes; i++) {
>> >adjust->voltage_swing[i] =
>> > @@ -560,7 +569,7 @@ static void drm_dp_link_get_adjustments(struct
>> > drm_dp_link *link,
>> >DP_TRAIN_PRE_EMPHASIS_SHIFT;
>> >
>> >adjust->post_cursor[i] =
>> > - drm_dp_get_adjust_request_post_cursor(status, i);
>> > + (post_cursor >> (i << 1)) & 0x3;
>> >}
>> > }
>> >
>> > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
>> > index 472dac376284..fdf3cf6ccc02 100644
>> > --- a/include/drm/drm_dp_helper.h
>> > +++ b/include/drm/drm_dp_helper.h
>> > @@ -1528,8 +1528,6 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8
>> > link_status[DP_LINK_STATUS_SI
>> > int lane);
>> > u8 drm_dp_get_adjust_tx_ffe_preset(const u8
>> > link_status[DP_LINK_STATUS_SIZE],
>> > int lane);
>> > -u8 drm_dp_get_adjust_request_post_cursor(co
Re: [PATCH] dma-buf: Move sysfs work out of DMA-BUF export/release path
Am 06.01.22 um 20:04 schrieb Hridya Valsaraju:
On Thu, Jan 6, 2022 at 12:59 AM Christian König
wrote:
Am 05.01.22 um 00:51 schrieb Hridya Valsaraju:
Recently, we noticed an issue where a process went into direct reclaim
while holding the kernfs rw semaphore for sysfs in write(exclusive)
mode. This caused processes who were doing DMA-BUF exports and releases
to go into uninterruptible sleep since they needed to acquire the same
semaphore for the DMA-BUF sysfs entry creation/deletion. In order to avoid
blocking DMA-BUF export/release for an indeterminate amount of time
while another process is holding the sysfs rw semaphore in exclusive
mode, this patch moves the per-buffer sysfs file creation/deleteion to
a kthread.
Well I absolutely don't think that this is justified.
You adding tons of complexity here just to avoid the overhead of
creating the sysfs files while exporting DMA-bufs which is an operation
which should be done exactly once in the lifecycle for the most common
use case.
Please explain further why that should be necessary.
Hi Christian,
We noticed that the issue sometimes causes the exporting process to go
to the uninterrupted sleep state for tens of milliseconds which
unfortunately leads to user-perceptible UI jank. This is the reason
why we are trying to move the sysfs entry creation and deletion out of
the DMA-BUF export/release path. I will update the commit message to
include the same in the next revision.
That is still not a justification for this change. The question is why
do you need that in the first place?
Exporting a DMA-buf should be something would should be very rarely,
e.g. only at the start of an application.
So this strongly looks like you are trying to optimize for an use case
where we should probably rethink what userspace is doing here instead.
If we would want to go down this route you would need to change all the
drivers implementing the DMA-buf export functionality to avoid
uninterruptible sleep as well and that is certainly something I would NAK.
Regards,
Christian.
Thanks,
Hridya
Regards,
Christian.
Fixes: bdb8d06dfefd ("dmabuf: Add the capability to expose DMA-BUF stats in
sysfs")
Signed-off-by: Hridya Valsaraju
---
drivers/dma-buf/dma-buf-sysfs-stats.c | 343 --
include/linux/dma-buf.h | 46
2 files changed, 366 insertions(+), 23 deletions(-)
diff --git a/drivers/dma-buf/dma-buf-sysfs-stats.c
b/drivers/dma-buf/dma-buf-sysfs-stats.c
index 053baadcada9..3251fdf2f05f 100644
--- a/drivers/dma-buf/dma-buf-sysfs-stats.c
+++ b/drivers/dma-buf/dma-buf-sysfs-stats.c
@@ -7,13 +7,39 @@
#include
#include
+#include
#include
+#include
+#include
#include
+#include
#include
#include
#include "dma-buf-sysfs-stats.h"
+struct dmabuf_kobj_work {
+ struct list_head list;
+ struct dma_buf_sysfs_entry *sysfs_entry;
+ struct dma_buf_sysfs_entry_metadata *sysfs_metadata;
+ unsigned long uid;
+};
+
+/* Both kobject setup and teardown work gets queued on the list. */
+static LIST_HEAD(dmabuf_kobj_work_list);
+
+/* dmabuf_kobj_list_lock protects dmabuf_kobj_work_list. */
+static DEFINE_SPINLOCK(dmabuf_kobj_list_lock);
+
+/*
+ * dmabuf_sysfs_show_lock prevents a race between a DMA-BUF sysfs file being
+ * read and the DMA-BUF being freed by protecting sysfs_entry->dmabuf.
+ */
+static DEFINE_SPINLOCK(dmabuf_sysfs_show_lock);
+
+static struct task_struct *dmabuf_kobject_task;
+static wait_queue_head_t dmabuf_kobject_waitqueue;
+
#define to_dma_buf_entry_from_kobj(x) container_of(x, struct
dma_buf_sysfs_entry, kobj)
/**
@@ -64,15 +90,26 @@ static ssize_t dma_buf_stats_attribute_show(struct kobject
*kobj,
struct dma_buf_stats_attribute *attribute;
struct dma_buf_sysfs_entry *sysfs_entry;
struct dma_buf *dmabuf;
+ int ret;
attribute = to_dma_buf_stats_attr(attr);
sysfs_entry = to_dma_buf_entry_from_kobj(kobj);
+
+ /*
+ * acquire dmabuf_sysfs_show_lock to prevent a race with the DMA-BUF
+ * being freed while sysfs_entry->dmabuf is being accessed.
+ */
+ spin_lock(&dmabuf_sysfs_show_lock);
dmabuf = sysfs_entry->dmabuf;
- if (!dmabuf || !attribute->show)
+ if (!dmabuf || !attribute->show) {
+ spin_unlock(&dmabuf_sysfs_show_lock);
return -EIO;
+ }
- return attribute->show(dmabuf, attribute, buf);
+ ret = attribute->show(dmabuf, attribute, buf);
+ spin_unlock(&dmabuf_sysfs_show_lock);
+ return ret;
}
static const struct sysfs_ops dma_buf_stats_sysfs_ops = {
@@ -118,33 +155,275 @@ static struct kobj_type dma_buf_ktype = {
.default_groups = dma_buf_stats_default_groups,
};
-void dma_buf_stats_teardown(struct dma_buf *dmabuf)
+/* Statistics files do not need to send uevents. */
+static int dmabuf_sysfs_uevent_filter(struct kset *kset, struct kobject *kobj)
{
- struct dma_buf_sysfs_entry *sysfs_entry;
+ return 0
[PATCH 2/2] drm/amdgpu: enable dcn support on arm64
Signed-off-by: Zhenneng Li --- drivers/gpu/drm/amd/display/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/display/Kconfig b/drivers/gpu/drm/amd/display/Kconfig index 127667e549c1..1c6c4cb1fd0a 100644 --- a/drivers/gpu/drm/amd/display/Kconfig +++ b/drivers/gpu/drm/amd/display/Kconfig @@ -6,7 +6,7 @@ config DRM_AMD_DC bool "AMD DC - Enable new display engine" default y select SND_HDA_COMPONENT if SND_HDA_CORE - select DRM_AMD_DC_DCN if (X86 || PPC64) && !(KCOV_INSTRUMENT_ALL && KCOV_ENABLE_COMPARISONS) + select DRM_AMD_DC_DCN if (X86 || PPC64 || (ARM64 && KERNEL_MODE_NEON)) && !(KCOV_INSTRUMENT_ALL && KCOV_ENABLE_COMPARISONS) help Choose this option if you want to use the new display engine support for AMDGPU. This adds required support for Vega and -- 2.25.1 No virus found Checked by Hillstone Network AntiVirus
[PATCH 1/2] drm/amdgpu: fix compile error for dcn on arm64
For adapting radeon rx6600 xt on arm64, I enabled dcn on arm64 platform, compiler report not compatible with -mgeneral-regs-only and -mhard-float when compiling some source file. Signed-off-by: Zhenneng Li --- drivers/gpu/drm/amd/display/dc/calcs/Makefile | 6 + .../gpu/drm/amd/display/dc/clk_mgr/Makefile | 7 ++ drivers/gpu/drm/amd/display/dc/dcn10/Makefile | 4 +++ drivers/gpu/drm/amd/display/dc/dcn20/Makefile | 4 +++ .../gpu/drm/amd/display/dc/dcn201/Makefile| 6 + drivers/gpu/drm/amd/display/dc/dcn21/Makefile | 4 +++ drivers/gpu/drm/amd/display/dc/dcn30/Makefile | 6 + .../gpu/drm/amd/display/dc/dcn302/Makefile| 6 + .../gpu/drm/amd/display/dc/dcn303/Makefile| 6 + drivers/gpu/drm/amd/display/dc/dcn31/Makefile | 6 + drivers/gpu/drm/amd/display/dc/dml/Makefile | 25 +++ drivers/gpu/drm/amd/display/dc/dsc/Makefile | 7 ++ 12 files changed, 87 insertions(+) diff --git a/drivers/gpu/drm/amd/display/dc/calcs/Makefile b/drivers/gpu/drm/amd/display/dc/calcs/Makefile index f3c00f479e1c..1cb9925d9949 100644 --- a/drivers/gpu/drm/amd/display/dc/calcs/Makefile +++ b/drivers/gpu/drm/amd/display/dc/calcs/Makefile @@ -57,6 +57,12 @@ CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calcs.o := $(calcs_rcflags) CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calc_auto.o := $(calcs_rcflags) CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calc_math.o := $(calcs_rcflags) +ifdef CONFIG_ARM64 +CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calcs.o += -mgeneral-regs-only +CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calc_auto.o += -mgeneral-regs-only +CFLAGS_REMOVE_$(AMDDALPATH)/dc/calcs/dcn_calc_math.o += -mgeneral-regs-only +endif + BW_CALCS = dce_calcs.o bw_fixed.o custom_float.o ifdef CONFIG_DRM_AMD_DC_DCN diff --git a/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile b/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile index 6bd73e49a6d2..6779c236ea30 100644 --- a/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile +++ b/drivers/gpu/drm/amd/display/dc/clk_mgr/Makefile @@ -145,6 +145,13 @@ AMD_DAL_CLK_MGR_DCN301 = $(addprefix $(AMDDALPATH)/dc/clk_mgr/dcn301/,$(CLK_MGR_ AMD_DISPLAY_FILES += $(AMD_DAL_CLK_MGR_DCN301) +ifdef CONFIG_ARM64 +CFLAGS_REMOVE_$(AMDDALPATH)/dc/clk_mgr/dcn21/rn_clk_mgr.o += -mgeneral-regs-only +CFLAGS_REMOVE_$(AMDDALPATH)/dc/clk_mgr/dcn301/dcn30_clk_mgr.o += -mgeneral-regs-only +CFLAGS_REMOVE_$(AMDDALPATH)/dc/clk_mgr/dcn301/vg_clk_mgr.o += -mgeneral-regs-only +CFLAGS_REMOVE_$(AMDDALPATH)/dc/clk_mgr/dcn30/dcn30_clk_mgr.o += -mgeneral-regs-only +endif + ### # DCN31 ### diff --git a/drivers/gpu/drm/amd/display/dc/dcn10/Makefile b/drivers/gpu/drm/amd/display/dc/dcn10/Makefile index 62ad1a11bff9..5dc61ea5070c 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn10/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dcn10/Makefile @@ -29,6 +29,10 @@ DCN10 = dcn10_init.o dcn10_resource.o dcn10_ipp.o dcn10_hw_sequencer.o \ dcn10_dpp_dscl.o dcn10_dpp_cm.o dcn10_cm_common.o \ dcn10_hubbub.o dcn10_stream_encoder.o dcn10_link_encoder.o +ifdef CONFIG_ARM64 +CFLAGS_REMOVE_$(AMDDALPATH)/dc/dcn10/dcn10_resource.o += -mgeneral-regs-only +endif + AMD_DAL_DCN10 = $(addprefix $(AMDDALPATH)/dc/dcn10/,$(DCN10)) AMD_DISPLAY_FILES += $(AMD_DAL_DCN10) diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/Makefile b/drivers/gpu/drm/amd/display/dc/dcn20/Makefile index 5fcaf78334ff..88c35a24e0cf 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn20/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dcn20/Makefile @@ -17,6 +17,10 @@ ifdef CONFIG_PPC64 CFLAGS_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o := -mhard-float -maltivec endif +ifdef CONFIG_ARM64 +CFLAGS_REMOVE_$(AMDDALPATH)/dc/dcn20/dcn20_resource.o += -mgeneral-regs-only +endif + ifdef CONFIG_CC_IS_GCC ifeq ($(call cc-ifversion, -lt, 0701, y), y) IS_OLD_GCC = 1 diff --git a/drivers/gpu/drm/amd/display/dc/dcn201/Makefile b/drivers/gpu/drm/amd/display/dc/dcn201/Makefile index f68038ceb1b1..2124414d629f 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn201/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dcn201/Makefile @@ -14,12 +14,18 @@ ifdef CONFIG_PPC64 CFLAGS_$(AMDDALPATH)/dc/dcn201/dcn201_resource.o := -mhard-float -maltivec endif +ifdef CONFIG_ARM64 +CFLAGS_REMOVE_$(AMDDALPATH)/dc/dcn201/dcn201_resource.o += -mgeneral-regs-only +endif + ifdef CONFIG_CC_IS_GCC ifeq ($(call cc-ifversion, -lt, 0701, y), y) IS_OLD_GCC = 1 endif +ifndef CONFIG_ARM64 CFLAGS_$(AMDDALPATH)/dc/dcn201/dcn201_resource.o += -mhard-float endif +endif ifdef CONFIG_X86 ifdef IS_OLD_GCC diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile index bb8c95141082..df8ed3401b7f 100644 --- a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile +++ b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile @@ -30,6 +30,10 @@ CF
[PATCH 0/2]
For adapting radeon rx6600 xt on arm64 platform, there report some compile errors. Zhenneng Li (2): drm/amdgpu: fix compile error for dcn on arm64 drm/amdgpu: enable dcn support on arm64 drivers/gpu/drm/amd/display/Kconfig | 2 +- drivers/gpu/drm/amd/display/dc/calcs/Makefile | 6 + .../gpu/drm/amd/display/dc/clk_mgr/Makefile | 7 ++ drivers/gpu/drm/amd/display/dc/dcn10/Makefile | 4 +++ drivers/gpu/drm/amd/display/dc/dcn20/Makefile | 4 +++ .../gpu/drm/amd/display/dc/dcn201/Makefile| 6 + drivers/gpu/drm/amd/display/dc/dcn21/Makefile | 4 +++ drivers/gpu/drm/amd/display/dc/dcn30/Makefile | 6 + .../gpu/drm/amd/display/dc/dcn302/Makefile| 6 + .../gpu/drm/amd/display/dc/dcn303/Makefile| 6 + drivers/gpu/drm/amd/display/dc/dcn31/Makefile | 6 + drivers/gpu/drm/amd/display/dc/dml/Makefile | 25 +++ drivers/gpu/drm/amd/display/dc/dsc/Makefile | 7 ++ 13 files changed, 88 insertions(+), 1 deletion(-) -- 2.25.1 No virus found Checked by Hillstone Network AntiVirus
Re: [PATCH 3/3] phy: mediatek: phy-mtk-mipi-dsi: Simplify with dev_err_probe()
Il 06/01/22 10:13, Chunfeng Yun ha scritto:
On Mon, 2022-01-03 at 15:53 +0100, AngeloGioacchino Del Regno wrote:
Use the dev_err_probe() helper to simplify error handling during
probe.
Signed-off-by: AngeloGioacchino Del Regno <
angelogioacchino.delre...@collabora.com>
---
drivers/phy/mediatek/phy-mtk-mipi-dsi.c | 29 +
1 file changed, 10 insertions(+), 19 deletions(-)
diff --git a/drivers/phy/mediatek/phy-mtk-mipi-dsi.c
b/drivers/phy/mediatek/phy-mtk-mipi-dsi.c
index 6f7425b0bf5b..4b77508f5241 100644
--- a/drivers/phy/mediatek/phy-mtk-mipi-dsi.c
+++ b/drivers/phy/mediatek/phy-mtk-mipi-dsi.c
@@ -148,11 +148,9 @@ static int mtk_mipi_tx_probe(struct
platform_device *pdev)
return PTR_ERR(mipi_tx->regmap);
ref_clk = devm_clk_get(dev, NULL);
- if (IS_ERR(ref_clk)) {
- ret = PTR_ERR(ref_clk);
- dev_err(dev, "Failed to get reference clock: %d\n",
ret);
- return ret;
- }
+ if (IS_ERR(ref_clk))
+ return dev_err_probe(dev, PTR_ERR(ref_clk),
+"Failed to get reference
clock\n");
ret = of_property_read_u32(dev->of_node, "drive-strength-
microamp",
&mipi_tx->mipitx_drive);
@@ -172,27 +170,20 @@ static int mtk_mipi_tx_probe(struct
platform_device *pdev)
ret = of_property_read_string(dev->of_node, "clock-output-
names",
&clk_init.name);
- if (ret < 0) {
- dev_err(dev, "Failed to read clock-output-names: %d\n",
ret);
- return ret;
- }
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to read clock-
output-names\n");
Seems no need change it here. (no EPROBE_DEFER error)
Hello Chunfeng,
pasting from kernel driver-api infrastructure guidelines:
[...]Note that it is deemed acceptable to use this function for error prints during
probe even if the err is known to never be -EPROBE_DEFER. The benefit compared to a
normal dev_err() is the standardized format of the error code and the fact that the
error code is returned.
https://www.kernel.org/doc/html/latest/driver-api/infrastructure.html
Regards,
- Angelo
Thanks
clk_init.ops = mipi_tx->driver_data->mipi_tx_clk_ops;
mipi_tx->pll_hw.init = &clk_init;
mipi_tx->pll = devm_clk_register(dev, &mipi_tx->pll_hw);
- if (IS_ERR(mipi_tx->pll)) {
- ret = PTR_ERR(mipi_tx->pll);
- dev_err(dev, "Failed to register PLL: %d\n", ret);
- return ret;
- }
+ if (IS_ERR(mipi_tx->pll))
+ return dev_err_probe(dev, PTR_ERR(mipi_tx->pll),
"Cannot register PLL\n");
phy = devm_phy_create(dev, NULL, &mtk_mipi_tx_ops);
- if (IS_ERR(phy)) {
- ret = PTR_ERR(phy);
- dev_err(dev, "Failed to create MIPI D-PHY: %d\n", ret);
- return ret;
- }
+ if (IS_ERR(phy))
+ return dev_err_probe(dev, PTR_ERR(phy), "Failed to
create MIPI D-PHY\n");
+
phy_set_drvdata(phy, mipi_tx);
phy_provider = devm_of_phy_provider_register(dev,
of_phy_simple_xlate);
Re: [PATCH 1/2] drm/msm/gpu: Wait for idle before suspending
Il 06/01/22 19:14, Rob Clark ha scritto: From: Rob Clark System suspend uses pm_runtime_force_suspend(), which cheekily bypasses the runpm reference counts. This doesn't actually work so well when the GPU is active. So add a reasonable delay waiting for the GPU to become idle. Alternatively we could just return -EBUSY in this case, but that has the disadvantage of causing system suspend to fail. Signed-off-by: Rob Clark Reviewed-by: AngeloGioacchino Del Regno
Re: [PATCH] drm/ttm: Don't inherit GEM object VMAs in child process
Am 06.01.22 um 17:51 schrieb Felix Kuehling: Am 2022-01-06 um 11:48 a.m. schrieb Christian König: Am 06.01.22 um 17:45 schrieb Felix Kuehling: Am 2022-01-06 um 4:05 a.m. schrieb Christian König: [SNIP] Also, why does your ACK or NAK depend on this at all. If it's the right thing to do, it's the right thing to do regardless of who benefits from it. In addition, how can a child process that doesn't even use the GPU be in violation of any GPU-driver related specifications. The argument is that the application is broken and needs to be fixed instead of worked around inside the kernel. I still don't get how they the application is broken. Like I said, the child process is not using the GPU. How can the application be fixed in this case? Sounds like I'm still missing some important puzzle pieces for the full picture to figure out why this doesn't work. Are you saying, any application that forks and doesn't immediately call exec is broken? More or less yes. We essentially have three possible cases here: 1. Application is already using (for example) OpenGL or OpenCL to do some rendering on the GPU and then calls fork() and expects to use OpenGL both in the parent and the child at the same time. As far as I know that's illegal from the Khronos specification point of view and working around inside the kernel for something not allowed in the first place is seen as futile effort. 2. Application opened the file descriptor, forks and then initializes OpenGL/Vulkan/OpenCL. That's what some compositors are doing to drop into the backround and is explicitly legal. 3. Application calls fork() and then doesn't use the GPU in the child. Either by not using it or calling exec. That should be legal and not cause any problems in the first place. But from your description I still don't get why we are running into problems here. I was assuming that you have case #1 because we previously had some examples of this with this python library, but from your description it seems to be case #3. Or does an application that forks need to be aware that some other part of the application used the GPU and explicitly free any GPU resources? Others might fill that information in, but I think that was the plan for newer APIs like Vulkan. Regards, Christian. Thanks, Felix Regards, Christian. Regards, Felix Let's talk about this on Mondays call. Thanks for giving the whole context. Regards, Christian. Regards, Felix
[PATCH] drm/msm/hdmi: Fix missing put_device() call in msm_hdmi_get_phy
The reference taken by 'of_find_device_by_node()' must be released when
not needed anymore.
Add the corresponding 'put_device()' in the error handling path.
Fixes: e00012b256d4 ("drm/msm/hdmi: Make HDMI core get its PHY")
Signed-off-by: Miaoqian Lin
---
drivers/gpu/drm/msm/hdmi/hdmi.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 75b64e6ae035..a439794a32e8 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -95,10 +95,15 @@ static int msm_hdmi_get_phy(struct hdmi *hdmi)
of_node_put(phy_node);
- if (!phy_pdev || !hdmi->phy) {
+ if (!phy_pdev) {
DRM_DEV_ERROR(&pdev->dev, "phy driver is not ready\n");
return -EPROBE_DEFER;
}
+ if (!hdmi->phy) {
+ DRM_DEV_ERROR(&pdev->dev, "phy driver is not ready\n");
+ put_device(&phy_pdev->dev);
+ return -EPROBE_DEFER;
+ }
hdmi->phy_dev = get_device(&phy_pdev->dev);
--
2.17.1
[PATCH] drm/sun4i: dw-hdmi: Fix missing put_device() call in sun8i_hdmi_phy_get
The reference taken by 'of_find_device_by_node()' must be released when
not needed anymore.
Add the corresponding 'put_device()' in the error handling path.
Fixes: 9bf3797796f5 ("drm/sun4i: dw-hdmi: Make HDMI PHY into a platform device")
Signed-off-by: Miaoqian Lin
---
drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
index b64d93da651d..5e2b0175df36 100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -658,8 +658,10 @@ int sun8i_hdmi_phy_get(struct sun8i_dw_hdmi *hdmi, struct
device_node *node)
return -EPROBE_DEFER;
phy = platform_get_drvdata(pdev);
- if (!phy)
+ if (!phy) {
+ put_device(&pdev->dev);
return -EPROBE_DEFER;
+ }
hdmi->phy = phy;
--
2.17.1
Re: [PATCH v3] drm/mediatek: Fix unused-but-set variable warning
Hi Matthias, >> I'm still not happy with the commit subject, I think it is misleading. Clang >> only helped to find the bug, but the we are fixing something else, that's >> not >> just a clang warning. But I don't want to nit-pick too much so: >> >> Reviewed-by: Matthias Brugger > thanks. I think you are right. > I will change the subject to "drm/mediatek: Fix mtk_cec_mask()", remove the > clang part and submit patch v4. I posted patch v4 https://lore.kernel.org/linux-mediatek/20220103054706.8072-1-miles.c...@mediatek.com/ without your reviewed-by tag. Would you mind taking a look at the patch? Miles
Re: [PATCH v2] drm/bridge/tc358775: Fix for dual-link LVDS
Reviewed-by: Vinay Simha BN
Jiri Vanek,
Could you please share the part number or datasheet of the dual-link LVDS
display/panel used.
On Fri, Jan 7, 2022 at 12:30 AM Jiri Vanek wrote:
> Fixed wrong register shift for single/dual link LVDS output.
>
> Tested-by: Jiri Vanek
> Signed-off-by: Jiri Vanek
>
> ---
> v1:
> * Initial version
>
> v2:
> * Tested-by tag added
>
> ---
> drivers/gpu/drm/bridge/tc358775.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/tc358775.c
> b/drivers/gpu/drm/bridge/tc358775.c
> index 2272adcc5b4a..1d6ec1baeff2 100644
> --- a/drivers/gpu/drm/bridge/tc358775.c
> +++ b/drivers/gpu/drm/bridge/tc358775.c
> @@ -241,7 +241,7 @@ static inline u32 TC358775_LVCFG_PCLKDIV(uint32_t val)
> }
>
> #define TC358775_LVCFG_LVDLINK__MASK 0x0002
> -#define TC358775_LVCFG_LVDLINK__SHIFT0
> +#define TC358775_LVCFG_LVDLINK__SHIFT1
> static inline u32 TC358775_LVCFG_LVDLINK(uint32_t val)
> {
> return ((val) << TC358775_LVCFG_LVDLINK__SHIFT) &
> --
> 2.30.2
>
>
--
regards,
vinaysimha
Re: [PATCH v3 3/4] drm/i915/ttm: add unmap_virtual callback
On 1/6/22 18:49, Matthew Auld wrote:
Ensure we call ttm_bo_unmap_virtual when releasing the pages.
Importantly this should now handle the ttm swapping case, and all other
places that already call into i915_ttm_move_notify().
v2: fix up the selftest
Fixes: cf3e3e86d779 ("drm/i915: Use ttm mmap handling for ttm bo's.")
Signed-off-by: Matthew Auld
Cc: Thomas Hellström
Hm.
I guess we've been saved here previously by the fact that TTM calls
ttm_bo_unmap_virtual()
before calling the move callback.
Reviewed-by: Thomas Hellström
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 3 +++
.../gpu/drm/i915/gem/i915_gem_object_types.h | 1 +
drivers/gpu/drm/i915/gem/i915_gem_ttm.c| 6 ++
.../gpu/drm/i915/gem/selftests/i915_gem_mman.c | 18 --
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index ee5ec0fd4807..5ac2506f4ee8 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -538,6 +538,9 @@ void i915_gem_object_release_mmap_offset(struct
drm_i915_gem_object *obj)
{
struct i915_mmap_offset *mmo, *mn;
+ if (obj->ops->unmap_virtual)
+ obj->ops->unmap_virtual(obj);
+
spin_lock(&obj->mmo.lock);
rbtree_postorder_for_each_entry_safe(mmo, mn,
&obj->mmo.offsets, offset) {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
index f9f7e44099fe..4b4829eb16c2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object_types.h
@@ -67,6 +67,7 @@ struct drm_i915_gem_object_ops {
int (*pwrite)(struct drm_i915_gem_object *obj,
const struct drm_i915_gem_pwrite *arg);
u64 (*mmap_offset)(struct drm_i915_gem_object *obj);
+ void (*unmap_virtual)(struct drm_i915_gem_object *obj);
int (*dmabuf_export)(struct drm_i915_gem_object *obj);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
index 8d61d4538a64..1530d9f0bc81 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_ttm.c
@@ -950,6 +950,11 @@ static u64 i915_ttm_mmap_offset(struct drm_i915_gem_object
*obj)
return drm_vma_node_offset_addr(&obj->base.vma_node);
}
+static void i915_ttm_unmap_virtual(struct drm_i915_gem_object *obj)
+{
+ ttm_bo_unmap_virtual(i915_gem_to_ttm(obj));
+}
+
static const struct drm_i915_gem_object_ops i915_gem_ttm_obj_ops = {
.name = "i915_gem_object_ttm",
.flags = I915_GEM_OBJECT_IS_SHRINKABLE |
@@ -965,6 +970,7 @@ static const struct drm_i915_gem_object_ops
i915_gem_ttm_obj_ops = {
.migrate = i915_ttm_migrate,
.mmap_offset = i915_ttm_mmap_offset,
+ .unmap_virtual = i915_ttm_unmap_virtual,
.mmap_ops = &vm_ops_ttm,
};
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 743a098facf2..f61356b72b1c 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -1369,20 +1369,10 @@ static int __igt_mmap_revoke(struct drm_i915_private
*i915,
}
}
- if (!obj->ops->mmap_ops) {
- err = check_absent(addr, obj->base.size);
- if (err) {
- pr_err("%s: was not absent\n", obj->mm.region->name);
- goto out_unmap;
- }
- } else {
- /* ttm allows access to evicted regions by design */
-
- err = check_present(addr, obj->base.size);
- if (err) {
- pr_err("%s: was not present\n", obj->mm.region->name);
- goto out_unmap;
- }
+ err = check_absent(addr, obj->base.size);
+ if (err) {
+ pr_err("%s: was not absent\n", obj->mm.region->name);
+ goto out_unmap;
}
out_unmap:
Re: [PATCH v3 4/4] drm/i915/ttm: ensure we unmap when purging
On 1/6/22 18:49, Matthew Auld wrote:
Purging can happen during swapping out, or directly invoked with the
madvise ioctl. In such cases this doesn't involve a ttm move, which
skips umapping the object.
v2(Thomas):
- add ttm_truncate helper, and just call into i915_ttm_move_notify() to
handle the unmapping step
Fixes: cf3e3e86d779 ("drm/i915: Use ttm mmap handling for ttm bo's.")
Should this Fixes: tag be when we we introduce truncate for the TTM
backend. IIRC that was in a later commit?
Otherwise
Reviewed-by: Thomas Hellström
/Thomas
