dri-devel April 2021

dri-devel@lists.freedesktop.org

317 participants
715 discussions

[PATCH v1, 0/3] gamma set with cmdq
by Yongqiang Niu 13 Apr '21

13 Apr '21

This series are based on 5.12-rc2 and provide 3 patch to set gamma lut with cmdq Yongqiang Niu (3): drm/mediatek: Separate aal module arm64: dts: mt8183: refine aal compatible name drm/mediatek: gamma set with cmdq arch/arm64/boot/dts/mediatek/mt8183.dtsi | 3 +- drivers/gpu/drm/mediatek/Makefile | 3 +- drivers/gpu/drm/mediatek/mtk_disp_aal.c | 167 ++++++++++++++++++++++++++++ drivers/gpu/drm/mediatek/mtk_disp_drv.h | 14 ++- drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 11 +- drivers/gpu/drm/mediatek/mtk_drm_crtc.c | 18 +-- drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c | 39 +------ drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h | 8 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 8 +- drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 + 10 files changed, 213 insertions(+), 59 deletions(-) create mode 100644 drivers/gpu/drm/mediatek/mtk_disp_aal.c -- 1.8.1.1.dirty

3 9

[PATCH v2 1/5] drm/connector: Create a helper to attach the hdr_output_metadata property
by Maxime Ripard 13 Apr '21

13 Apr '21

All the drivers that implement HDR output call pretty much the same function to initialise the hdr_output_metadata property, and while the creation of that property is in a helper, every driver uses the same code to attach it. Provide a helper for it as well Reviewed-by: Harry Wentland <harry.wentland(a)amd.com> Reviewed-by: Jernej Skrabec <jernej.skrabec(a)siol.net> Signed-off-by: Maxime Ripard <maxime(a)cerno.tech> --- Changes from v1: - Rebased on latest drm-misc-next tag - Added the tags --- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 4 +--- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 3 +-- drivers/gpu/drm/drm_connector.c | 21 +++++++++++++++++++ drivers/gpu/drm/i915/display/intel_hdmi.c | 3 +-- include/drm/drm_connector.h | 1 + 5 files changed, 25 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 55e39b462a5e..1e22ce718010 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7078,9 +7078,7 @@ void amdgpu_dm_connector_init_helper(struct amdgpu_display_manager *dm, if (connector_type == DRM_MODE_CONNECTOR_HDMIA || connector_type == DRM_MODE_CONNECTOR_DisplayPort || connector_type == DRM_MODE_CONNECTOR_eDP) { - drm_object_attach_property( - &aconnector->base.base, - dm->ddev->mode_config.hdr_output_metadata_property, 0); + drm_connector_attach_hdr_output_metadata_property(&aconnector->base); if (!aconnector->mst_port) drm_connector_attach_vrr_capable_property(&aconnector->base); diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c index dda4fa9a1a08..f24bbb840dbf 100644 --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c @@ -2492,8 +2492,7 @@ static int dw_hdmi_connector_create(struct dw_hdmi *hdmi) drm_connector_attach_max_bpc_property(connector, 8, 16); if (hdmi->version >= 0x200a && hdmi->plat_data->use_drm_infoframe) - drm_object_attach_property(&connector->base, - connector->dev->mode_config.hdr_output_metadata_property, 0); + drm_connector_attach_hdr_output_metadata_property(connector); drm_connector_attach_encoder(connector, hdmi->bridge.encoder); diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 7631f76e7f34..a4aa2d87af35 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -2150,6 +2150,27 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector, } EXPORT_SYMBOL(drm_connector_attach_max_bpc_property); +/** + * drm_connector_attach_hdr_output_metadata_property - attach "HDR_OUTPUT_METADA" property + * @connector: connector to attach the property on. + * + * This is used to allow the userspace to send HDR Metadata to the + * driver. + * + * Returns: + * Zero on success, negative errno on failure. + */ +int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *connector) +{ + struct drm_device *dev = connector->dev; + struct drm_property *prop = dev->mode_config.hdr_output_metadata_property; + + drm_object_attach_property(&connector->base, prop, 0); + + return 0; +} +EXPORT_SYMBOL(drm_connector_attach_hdr_output_metadata_property); + /** * drm_connector_set_vrr_capable_property - sets the variable refresh rate * capable property for a connector diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index 95919d325b0b..f2f1b025e6ba 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2965,8 +2965,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c drm_connector_attach_content_type_property(connector); if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) - drm_object_attach_property(&connector->base, - connector->dev->mode_config.hdr_output_metadata_property, 0); + drm_connector_attach_hdr_output_metadata_property(connector); if (!HAS_GMCH(dev_priv)) drm_connector_attach_max_bpc_property(connector, 8, 12); diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 1922b278ffad..32172dab8427 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -1671,6 +1671,7 @@ int drm_connector_attach_scaling_mode_property(struct drm_connector *connector, u32 scaling_mode_mask); int drm_connector_attach_vrr_capable_property( struct drm_connector *connector); +int drm_connector_attach_hdr_output_metadata_property(struct drm_connector *connector); int drm_mode_create_aspect_ratio_property(struct drm_device *dev); int drm_mode_create_hdmi_colorspace_property(struct drm_connector *connector); int drm_mode_create_dp_colorspace_property(struct drm_connector *connector); -- 2.30.2

1 4

[PATCH 1/3] dma-buf: Require VM_PFNMAP vma for mmap
by Daniel Vetter 13 Apr '21

13 Apr '21

tldr; DMA buffers aren't normal memory, expecting that you can use them like that (like calling get_user_pages works, or that they're accounting like any other normal memory) cannot be guaranteed. Since some userspace only runs on integrated devices, where all buffers are actually all resident system memory, there's a huge temptation to assume that a struct page is always present and useable like for any more pagecache backed mmap. This has the potential to result in a uapi nightmare. To stop this gap require that DMA buffer mmaps are VM_PFNMAP, which blocks get_user_pages and all the other struct page based infrastructure for everyone. In spirit this is the uapi counterpart to the kernel-internal CONFIG_DMABUF_DEBUG. Motivated by a recent patch which wanted to swich the system dma-buf heap to vm_insert_page instead of vm_insert_pfn. v2: Jason brought up that we also want to guarantee that all ptes have the pte_special flag set, to catch fast get_user_pages (on architectures that support this). Allowing VM_MIXEDMAP (like VM_SPECIAL does) would still allow vm_insert_page, but limiting to VM_PFNMAP will catch that. >From auditing the various functions to insert pfn pte entires (vm_insert_pfn_prot, remap_pfn_range and all it's callers like dma_mmap_wc) it looks like VM_PFNMAP is already required anyway, so this should be the correct flag to check for. References: https://lore.kernel.org/lkml/CAKMK7uHi+mG0z0HUmNt13QCCvutuRVjpcR0NjRL12k-Wb… Acked-by: Christian König <christian.koenig(a)amd.com> Cc: Jason Gunthorpe <jgg(a)ziepe.ca> Cc: Suren Baghdasaryan <surenb(a)google.com> Cc: Matthew Wilcox <willy(a)infradead.org> Cc: John Stultz <john.stultz(a)linaro.org> Signed-off-by: Daniel Vetter <daniel.vetter(a)intel.com> Cc: Sumit Semwal <sumit.semwal(a)linaro.org> Cc: "Christian König" <christian.koenig(a)amd.com> Cc: linux-media(a)vger.kernel.org Cc: linaro-mm-sig(a)lists.linaro.org --- drivers/dma-buf/dma-buf.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index f264b70c383e..06cb1d2e9fdc 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -127,6 +127,7 @@ static struct file_system_type dma_buf_fs_type = { static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma) { struct dma_buf *dmabuf; + int ret; if (!is_dma_buf_file(file)) return -EINVAL; @@ -142,7 +143,11 @@ static int dma_buf_mmap_internal(struct file *file, struct vm_area_struct *vma) dmabuf->size >> PAGE_SHIFT) return -EINVAL; - return dmabuf->ops->mmap(dmabuf, vma); + ret = dmabuf->ops->mmap(dmabuf, vma); + + WARN_ON(!(vma->vm_flags & VM_PFNMAP)); + + return ret; } static loff_t dma_buf_llseek(struct file *file, loff_t offset, int whence) @@ -1244,6 +1249,8 @@ EXPORT_SYMBOL_GPL(dma_buf_end_cpu_access); int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, unsigned long pgoff) { + int ret; + if (WARN_ON(!dmabuf || !vma)) return -EINVAL; @@ -1264,7 +1271,11 @@ int dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma, vma_set_file(vma, dmabuf->file); vma->vm_pgoff = pgoff; - return dmabuf->ops->mmap(dmabuf, vma); + ret = dmabuf->ops->mmap(dmabuf, vma); + + WARN_ON(!(vma->vm_flags & VM_PFNMAP)); + + return ret; } EXPORT_SYMBOL_GPL(dma_buf_mmap); -- 2.31.0

2 3

[PATCH v2] drm/vmwgfx: Make sure unpinning handles reservations
by Zack Rusin 13 Apr '21

13 Apr '21

Quite often it's a little hard to tell if reservations are already held in code paths that unpin bo's. While our pinning/unpinning code should be more explicit that requires a substential amount of work so instead we can avoid the issues by making sure we try to reserve before unpinning. Because we unpin those bo's only on destruction/error paths just that check tells us if we're already reserved or not and allows to cleanly unpin. Reviewed-by: Martin Krastev <krastevm(a)vmware.com> Reviewed-by: Roland Scheidegger <sroland(a)vmware.com> Fixes: d1a73c641afd ("drm/vmwgfx: Make sure we unpin no longer needed buffers") Cc: dri-devel(a)lists.freedesktop.org Signed-off-by: Zack Rusin <zackr(a)vmware.com> --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 11 +++++++++++ drivers/gpu/drm/vmwgfx/vmwgfx_mob.c | 18 ++++++++++++++---- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h index 8087a9013455..9a3133cb5cc1 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h @@ -1523,8 +1523,19 @@ static inline void vmw_bo_unreference(struct vmw_buffer_object **buf) *buf = NULL; if (tmp_buf != NULL) { + /* + * currently pinning requires a reservation to be held + * but unreference can be called either reserved or not. + * To avoid deadloacks we have to try to get a reservation. + * This is exlusively to satisfy the requirements of the + * unpin, ttm_bo_put will handle delayed deletion for us + * in case the bo is really busy. + */ + bool locked = dma_resv_trylock(tmp_buf->base.base.resv); if (tmp_buf->base.pin_count > 0) ttm_bo_unpin(&tmp_buf->base); + if (locked) + dma_resv_unlock(tmp_buf->base.base.resv); ttm_bo_put(&tmp_buf->base); } } diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c index a0b53141dded..f2d625415458 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_mob.c @@ -94,6 +94,16 @@ static void vmw_mob_pt_setup(struct vmw_mob *mob, struct vmw_piter data_iter, unsigned long num_data_pages); + +static inline void vmw_bo_unpin_unlocked(struct ttm_buffer_object *bo) +{ + int ret = ttm_bo_reserve(bo, false, true, NULL); + BUG_ON(ret != 0); + ttm_bo_unpin(bo); + ttm_bo_unreserve(bo); +} + + /* * vmw_setup_otable_base - Issue an object table base setup command to * the device @@ -277,7 +287,7 @@ static int vmw_otable_batch_setup(struct vmw_private *dev_priv, &batch->otables[i]); } - ttm_bo_unpin(batch->otable_bo); + vmw_bo_unpin_unlocked(batch->otable_bo); ttm_bo_put(batch->otable_bo); batch->otable_bo = NULL; return ret; @@ -341,9 +351,9 @@ static void vmw_otable_batch_takedown(struct vmw_private *dev_priv, BUG_ON(ret != 0); vmw_bo_fence_single(bo, NULL); + ttm_bo_unpin(bo); ttm_bo_unreserve(bo); - ttm_bo_unpin(batch->otable_bo); ttm_bo_put(batch->otable_bo); batch->otable_bo = NULL; } @@ -530,7 +540,7 @@ static void vmw_mob_pt_setup(struct vmw_mob *mob, void vmw_mob_destroy(struct vmw_mob *mob) { if (mob->pt_bo) { - ttm_bo_unpin(mob->pt_bo); + vmw_bo_unpin_unlocked(mob->pt_bo); ttm_bo_put(mob->pt_bo); mob->pt_bo = NULL; } @@ -646,7 +656,7 @@ int vmw_mob_bind(struct vmw_private *dev_priv, out_no_cmd_space: vmw_fifo_resource_dec(dev_priv); if (pt_set_up) { - ttm_bo_unpin(mob->pt_bo); + vmw_bo_unpin_unlocked(mob->pt_bo); ttm_bo_put(mob->pt_bo); mob->pt_bo = NULL; } -- 2.27.0

2 1

[PATCH v6 0/2] enable HDCP in Cadence MHDP bridge driver
by Parshuram Thombare 13 Apr '21

13 Apr '21

This patch series enables HDCP in Cadence MHDP DPI/DP bridge driver. Changes since v1: - Move sapb reg block right after apb reg block - Corresponding changes in binding and example Changes since v2: - Revert reg resource sequence in binding and use resource mapping by name - Remove hdcp_config from binding and use DRM HDCP Content Type property to select HDCP version Changes since v3: - Fix kernel test robot warning Changes since v4: - Fix binding issue Changes since v5: - Maintain backward compatibility of driver on platforms without SAPB port by allowing driver to continue without HDCP feature instead of returning error Parshuram Thombare (2): dt-bindings: drm/bridge: MHDP8546 bridge binding changes for HDCP drm: bridge: cdns-mhdp8546: Enable HDCP .../display/bridge/cdns,mhdp8546.yaml | 15 +- drivers/gpu/drm/bridge/cadence/Makefile | 2 +- .../drm/bridge/cadence/cdns-mhdp8546-core.c | 128 +++- .../drm/bridge/cadence/cdns-mhdp8546-core.h | 22 + .../drm/bridge/cadence/cdns-mhdp8546-hdcp.c | 570 ++++++++++++++++++ .../drm/bridge/cadence/cdns-mhdp8546-hdcp.h | 92 +++ 6 files changed, 812 insertions(+), 17 deletions(-) create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.c create mode 100644 drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-hdcp.h -- 2.25.1

3 4

[PATCH] drm/amdgpu: remove set but not used variables
by Tian Tao 13 Apr '21

13 Apr '21

The value of ret set but will rewriten, so just delete. Signed-off-by: Tian Tao <tiantao6(a)hisilicon.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c index 1fb2a91..f303ad6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c @@ -1815,10 +1815,8 @@ int amdgpu_ras_recovery_init(struct amdgpu_device *adev) return 0; *data = kmalloc(sizeof(**data), GFP_KERNEL | __GFP_ZERO); - if (!*data) { - ret = -ENOMEM; - goto out; - } + if (!*data) + return -ENOMEM; mutex_init(&con->recovery_lock); INIT_WORK(&con->recovery_work, amdgpu_ras_do_recovery); @@ -1848,7 +1846,7 @@ int amdgpu_ras_recovery_init(struct amdgpu_device *adev) kfree((*data)->bps); kfree(*data); con->eh_data = NULL; -out: + dev_warn(adev->dev, "Failed to initialize ras recovery!\n"); /* -- 2.7.4

1 0

[PATCH v2 1/4] drm/vkms: init plane using drmm_universal_plane_alloc
by Melissa Wen 13 Apr '21

13 Apr '21

By using drmm_universal_plane_alloc instead of drm_universal_plane_init, we let the DRM infrastructure handles resource allocation and cleanup. We can also get rid of some code repetitions for plane cleanup, improving code maintainability in vkms. Signed-off-by: Melissa Wen <melissa.srw(a)gmail.com> --- drivers/gpu/drm/vkms/vkms_drv.h | 8 ++++++-- drivers/gpu/drm/vkms/vkms_output.c | 19 +++++-------------- drivers/gpu/drm/vkms/vkms_plane.c | 29 +++++++++++------------------ 3 files changed, 22 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 35540c7c4416..70fb79621617 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -37,6 +37,10 @@ struct vkms_plane_state { struct vkms_composer *composer; }; +struct vkms_plane { + struct drm_plane base; +}; + /** * vkms_crtc_state - Driver specific CRTC state * @base: base CRTC state @@ -114,8 +118,8 @@ int vkms_crtc_init(struct drm_device *dev, struct drm_crtc *crtc, int vkms_output_init(struct vkms_device *vkmsdev, int index); -struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, - enum drm_plane_type type, int index); +struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, + enum drm_plane_type type, int index); /* CRC Support */ const char *const *vkms_get_crc_sources(struct drm_crtc *crtc, diff --git a/drivers/gpu/drm/vkms/vkms_output.c b/drivers/gpu/drm/vkms/vkms_output.c index f5f6f15c362c..6979fbc7f821 100644 --- a/drivers/gpu/drm/vkms/vkms_output.c +++ b/drivers/gpu/drm/vkms/vkms_output.c @@ -39,7 +39,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 drm_plane *primary, *cursor = NULL; + struct vkms_plane *primary, *cursor = NULL; int ret; int writeback; @@ -49,15 +49,13 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) if (vkmsdev->config->cursor) { cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index); - if (IS_ERR(cursor)) { - ret = PTR_ERR(cursor); - goto err_cursor; - } + if (IS_ERR(cursor)) + return PTR_ERR(cursor); } - ret = vkms_crtc_init(dev, crtc, primary, cursor); + ret = vkms_crtc_init(dev, crtc, &primary->base, &cursor->base); if (ret) - goto err_crtc; + return ret; ret = drm_connector_init(dev, connector, &vkms_connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL); @@ -100,12 +98,5 @@ int vkms_output_init(struct vkms_device *vkmsdev, int index) err_connector: drm_crtc_cleanup(crtc); -err_crtc: - if (vkmsdev->config->cursor) - drm_plane_cleanup(cursor); - -err_cursor: - drm_plane_cleanup(primary); - return ret; } diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 6d310d31b75d..36562846f6cb 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -86,7 +86,6 @@ static void vkms_plane_reset(struct drm_plane *plane) static const struct drm_plane_funcs vkms_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, - .destroy = drm_plane_cleanup, .reset = vkms_plane_reset, .atomic_duplicate_state = vkms_plane_duplicate_state, .atomic_destroy_state = vkms_plane_destroy_state, @@ -191,18 +190,14 @@ static const struct drm_plane_helper_funcs vkms_primary_helper_funcs = { .cleanup_fb = vkms_cleanup_fb, }; -struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, - enum drm_plane_type type, int index) +struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev, + enum drm_plane_type type, int index) { struct drm_device *dev = &vkmsdev->drm; const struct drm_plane_helper_funcs *funcs; - struct drm_plane *plane; + struct vkms_plane *plane; const u32 *formats; - int ret, nformats; - - plane = kzalloc(sizeof(*plane), GFP_KERNEL); - if (!plane) - return ERR_PTR(-ENOMEM); + int nformats; if (type == DRM_PLANE_TYPE_CURSOR) { formats = vkms_cursor_formats; @@ -214,16 +209,14 @@ struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev, funcs = &vkms_primary_helper_funcs; } - ret = drm_universal_plane_init(dev, plane, 1 << index, - &vkms_plane_funcs, - formats, nformats, - NULL, type, NULL); - if (ret) { - kfree(plane); - return ERR_PTR(ret); - } + plane = drmm_universal_plane_alloc(dev, struct vkms_plane, base, 1 << index, + &vkms_plane_funcs, + formats, nformats, + NULL, type, NULL); + if (IS_ERR(plane)) + return plane; - drm_plane_helper_add(plane, funcs); + drm_plane_helper_add(&plane->base, &vkms_primary_helper_funcs); return plane; } -- 2.30.2

2 1

RE: [PATCH] drm/virtio: Create Dumb BOs as guest Blobs (v2)
by Zhang, Tina 13 Apr '21

13 Apr '21

> -----Original Message----- > From: Gurchetan Singh <gurchetansingh(a)chromium.org> > Sent: Tuesday, April 13, 2021 8:58 AM > To: Gerd Hoffmann <kraxel(a)redhat.com> > Cc: Kasireddy, Vivek <vivek.kasireddy(a)intel.com>; ML dri-devel <dri- > devel(a)lists.freedesktop.org>; Zhang, Tina <tina.zhang(a)intel.com> > Subject: Re: [PATCH] drm/virtio: Create Dumb BOs as guest Blobs (v2) > > > > On Fri, Apr 9, 2021 at 12:48 AM Gerd Hoffmann <mailto:kraxel@redhat.com> > wrote: > Hi, > > > > IIRC the VIRTGPU_BLOB_FLAG_USE_SHAREABLE flag means that the host > *can* > > > create a shared mapping (i.e. the host seeing guest-side changes without > > > explicit transfer doesn't cause problems for the guest). It doesn not > > > mean the host *must* create a shared mapping (note that there is no > > > negotiation whenever the host supports shared mappings or not). > > > > > > > VIRTGPU_BLOB_FLAG_USE_SHAREABLE means guest userspace intends to > share the > > blob resource with another virtgpu driver instance via > drmPrimeHandleToFd. > > It's a rough analogue to VkExportMemoryAllocateInfoKHR or > > PIPE_BIND_USE_SHARED. > > Oh. My memory was failing me then. We should *really* clarify the spec > for BLOB_MEM_GUEST. > > So shared mappings are allowed for all BLOB_MEM_GUEST resources, right? > > The guest iovecs are always shared with the host, so they may be copied > to/from directly depending on the operation. In the case of > RESOURCE_FLUSH + BLOB_MEM_GUEST, it could be a copy from the guest > iovecs to the host framebuffer [host framebuffer != host shadow memory]. > > > > > So the transfer calls are still needed, and the host can decide to > > > shortcut them in case it can create a shared mapping. In case there is > > > no shared mapping (say due to missing udmabuf support) the host can > > > fallback to copying. > > > > Transfers are a bit under-defined for BLOB_MEM_GUEST. Even without > udmabuf > > on the host, there is no host side resource for guest-only blobs? Before > > blob resources, the dumb flow was: > > > > 1) update guest side resource > > 2) TRANSFER_TO_HOST_2D to copy guest side contents to host side private > > resource [Pixman??] > > 3) RESOURCE_FLUSH to copy the host-side contents to the framebuffer and > > page-flip > > Yes. > > > At least for crosvm, this is possible: > > > > 1) update guest side resource > > 2) RESOURCE_FLUSH to copy the guest-side contents to the framebuffer and > > pageflip > > > > With implicit udmabuf, it may be possible to do this: > > > > 1) update guest side resource > > 2) RESOURCE_FLUSH to page-flip > > > > > So I think crosvm should be fixed to not consider transfer commands for > > > VIRTGPU_BLOB_MEM_GUEST resources an error. > > > > It's a simple change to make and we can definitely do it, if TRANSFER_2D is > > helpful for the QEMU case. I haven't looked at the QEMU side patches. > > Well, we have two different cases: > > (1) No udmabuf available. qemu will have a host-side shadow then and > the workflow will be largely identical to the non-blob resource > workflow. > > I think this is the key difference. With BLOB_MEM_GUEST, crosvm can only > have a guest side iovecs and no host-side shadow memory. With > BLOB_MEM_GUEST_HOST3D, host-side shadow memory will exist. > > I guess it boils down the Pixman dependency. crosvm sits right on top of > display APIs (X, wayland) rather than having intermediary layers. Adding a > new Pixman API takes time too. > > There's a bunch of options: > > 1) Don't use BLOB_MEM_GUEST dumb buffers in 3D mode. > 2) virglrenderer or crosvm modified to implicitly ignore > TRANSFER_TO_HOST_2D for BLOB_MEM_GUEST when in 3D mode. > 3) It's probably possible to create an implicit udmabuf > for RESOURCE_CREATE_2D resources and ignore the transfer there too. The > benefit of this is TRANSFER_TO_HOST_2D makes a ton of sense for non-blob > resources. No kernel side change needed here, just QEMU. > 4) modify QEMU display integration > > I would choose (1) since it solves the log spam problem and it advances blob > support in QEMU. Though I leave the decision to QEMU devs. > > > (2) With udmabuf support. qemu can create udmabufs for the resources, > mmap() the dmabuf to get a linear mapping, create a pixman buffer > backed by that dmabuf (no copying needed then). Depending on > capabilities pass either the pixman image (gl=off) or the dmabuf > handle (gl=on) to the UI code to actually show the guest display. > > The guest doesn't need to know any of this, it'll just send transfer and > flush commands. In case (1) qemu must process the transfer commands and > for case (2) qemu can simply ignore them. > > > For the PCI-passthrough + guest blob case, the end goal is to share it with > > the host compositor. If there is no guarantee the guest memory can be > > converted to an OS-handle (to share with the host compositor), then I think > > the guest user space should fallback to another technique involving > > memcpy() to share the memory. > > This is what happens today (using non-blob resources). > > > So essentially, thinking for two new protocol additions: > > > > F_CREATE_GUEST_HANDLE (or F_HANDLE_FROM_GUEST) --> means an OS- > specific > > udmabuf-like mechanism exists on the host. > > > > BLOB_FLAG_CREATE_GUEST_HANDLE (or > BLOB_FLAG_HANDLE_FROM_GUEST)--> tells > > host userspace "you must create a udmabuf" [or OS-specific equivalent] > upon > > success > > Again: Why do we actually need that? Is there any benefit other than > the guest knowing it doesn't need to send transfer commands? > I see the whole udmabuf thing as a host-side performance optimization > and I think this should be fully transparent to the guest as the host > can easily just ignore the transfer commands. > > So the use case I'm most interested in (and Vivek/Tina?) is tiled/compressed > udmabufs, so they may be eventually shared with the host compositor via the > DRM modifier API. Hi, Yes, eventually, we would like to have the tiled/compressed framebuffers shared by the udmabufs mechanism. BR, Tina > > Transfers to linear udmabufs make sense. Maybe transfers to > tiled/compressed udmabufs shouldn't even be attempted. > > It's a complicated case with many ambiguities, especially with PCI > passthrough involved. Explicit tiled/compressed udmabufs are just an idea, > will have to think more about it / have some proof of concept [with virgl and > PCI passthrough], before making any concrete proposals. Will keep your idea > of just ignoring transfers on the host in mind. > > Given we batch commands > the extra commands don't lead to extra context switches, so there > shouldn't be much overhead. > > If we really want make the guest aware of the hosts udmabuf state I > think this should be designed the other way around: Add some way for > the host to tell the guest transfer commands are not needed for a > specific BLOB_MEM_GUEST resource. > > take care, > Gerd

1 0

[PATCH 2/2] drm/msm/dp: do not re initialize of audio_comp
by Kuogee Hsieh 13 Apr '21

13 Apr '21

At dp_display_disable(), do not re initialize audio_comp if hdp_state == ST_DISCONNECT_PENDING (unplug event) to avoid race condition which cause 5 second timeout expired. Also add abort mechanism to reduce time spinning at dp_aux_transfer() during dpcd read if type-c connection had been broken. Signed-off-by: Kuogee Hsieh <khsieh(a)codeaurora.org> --- drivers/gpu/drm/msm/dp/dp_aux.c | 18 ++++++++++++++++++ drivers/gpu/drm/msm/dp/dp_aux.h | 1 + drivers/gpu/drm/msm/dp/dp_display.c | 16 ++++++++++++---- drivers/gpu/drm/msm/dp/dp_link.c | 20 +++++++++++++++----- 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_aux.c b/drivers/gpu/drm/msm/dp/dp_aux.c index 7c22bfe..e5ece8c 100644 --- a/drivers/gpu/drm/msm/dp/dp_aux.c +++ b/drivers/gpu/drm/msm/dp/dp_aux.c @@ -28,6 +28,7 @@ struct dp_aux_private { u32 offset; u32 segment; u32 isr; + atomic_t aborted; struct drm_dp_aux dp_aux; }; @@ -343,6 +344,11 @@ static ssize_t dp_aux_transfer(struct drm_dp_aux *dp_aux, mutex_lock(&aux->mutex); + if (atomic_read(&aux->aborted)) { + ret = -ETIMEDOUT; + goto unlock_exit; + } + aux->native = msg->request & (DP_AUX_NATIVE_WRITE & DP_AUX_NATIVE_READ); /* Ignore address only message */ @@ -533,3 +539,15 @@ void dp_aux_put(struct drm_dp_aux *dp_aux) devm_kfree(aux->dev, aux); } + +void dp_aux_abort(struct drm_dp_aux *dp_aux, bool abort) +{ + struct dp_aux_private *aux; + + if (!dp_aux) + return; + + aux = container_of(dp_aux, struct dp_aux_private, dp_aux); + + atomic_set(&aux->aborted, abort); +} diff --git a/drivers/gpu/drm/msm/dp/dp_aux.h b/drivers/gpu/drm/msm/dp/dp_aux.h index f8b8ba9..c17df7f 100644 --- a/drivers/gpu/drm/msm/dp/dp_aux.h +++ b/drivers/gpu/drm/msm/dp/dp_aux.h @@ -23,6 +23,7 @@ void dp_aux_isr(struct drm_dp_aux *dp_aux); void dp_aux_init(struct drm_dp_aux *dp_aux); void dp_aux_deinit(struct drm_dp_aux *dp_aux); void dp_aux_reconfig(struct drm_dp_aux *dp_aux); +void dp_aux_abort(struct drm_dp_aux *dp_aux, bool abort); struct drm_dp_aux *dp_aux_get(struct device *dev, struct dp_catalog *catalog); void dp_aux_put(struct drm_dp_aux *aux); diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 4992a049..8960333 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -898,8 +898,10 @@ static int dp_display_disable(struct dp_display_private *dp, u32 data) /* wait only if audio was enabled */ if (dp_display->audio_enabled) { /* signal the disconnect event */ - reinit_completion(&dp->audio_comp); - dp_display_handle_plugged_change(dp_display, false); + if (dp->hpd_state != ST_DISCONNECT_PENDING) { + reinit_completion(&dp->audio_comp); + dp_display_handle_plugged_change(dp_display, false); + } if (!wait_for_completion_timeout(&dp->audio_comp, HZ * 5)) DRM_ERROR("audio comp timeout\n"); @@ -1137,20 +1139,26 @@ static irqreturn_t dp_display_irq_handler(int irq, void *dev_id) /* hpd related interrupts */ if (hpd_isr_status & DP_DP_HPD_PLUG_INT_MASK || hpd_isr_status & DP_DP_HPD_REPLUG_INT_MASK) { + dp_aux_abort(dp->aux, false); dp_add_event(dp, EV_HPD_PLUG_INT, 0, 0); } if (hpd_isr_status & DP_DP_IRQ_HPD_INT_MASK) { /* stop sentinel connect pending checking */ + dp_aux_abort(dp->aux, false); dp_del_event(dp, EV_CONNECT_PENDING_TIMEOUT); dp_add_event(dp, EV_IRQ_HPD_INT, 0, 0); } - if (hpd_isr_status & DP_DP_HPD_REPLUG_INT_MASK) + if (hpd_isr_status & DP_DP_HPD_REPLUG_INT_MASK) { + dp_aux_abort(dp->aux, false); dp_add_event(dp, EV_HPD_REPLUG_INT, 0, 0); + } - if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK) + if (hpd_isr_status & DP_DP_HPD_UNPLUG_INT_MASK) { + dp_aux_abort(dp->aux, true); dp_add_event(dp, EV_HPD_UNPLUG_INT, 0, 0); + } } /* DP controller isr */ diff --git a/drivers/gpu/drm/msm/dp/dp_link.c b/drivers/gpu/drm/msm/dp/dp_link.c index be986da..d35b18e 100644 --- a/drivers/gpu/drm/msm/dp/dp_link.c +++ b/drivers/gpu/drm/msm/dp/dp_link.c @@ -737,18 +737,25 @@ static int dp_link_parse_sink_count(struct dp_link *dp_link) return 0; } -static void dp_link_parse_sink_status_field(struct dp_link_private *link) +static int dp_link_parse_sink_status_field(struct dp_link_private *link) { int len = 0; link->prev_sink_count = link->dp_link.sink_count; - dp_link_parse_sink_count(&link->dp_link); + len = dp_link_parse_sink_count(&link->dp_link); + if (len < 0) { + DRM_ERROR("DP lparse sink count failed\n"); + return len; + } len = drm_dp_dpcd_read_link_status(link->aux, link->link_status); - if (len < DP_LINK_STATUS_SIZE) + if (len < DP_LINK_STATUS_SIZE) { DRM_ERROR("DP link status read failed\n"); - dp_link_parse_request(link); + return len; + } + + return dp_link_parse_request(link); } /** @@ -1032,7 +1039,10 @@ int dp_link_process_request(struct dp_link *dp_link) dp_link_reset_data(link); - dp_link_parse_sink_status_field(link); + ret = dp_link_parse_sink_status_field(link); + if (ret) { + return ret; + } if (link->request.test_requested == DP_TEST_LINK_EDID_READ) { dp_link->sink_request |= DP_TEST_LINK_EDID_READ; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project

2 1

[PATCH 1/2] drm/msm/dp: check sink_count before update is_connected status
by Kuogee Hsieh 13 Apr '21

13 Apr '21

At pm_resume check link sisnk_count before update is_connected status base on HPD real time link status. Also print out error message only when either EV_CONNECT_PENDING_TIMEOUT or EV_DISCONNECT_PENDING_TIMEOUT happen. Signed-off-by: Kuogee Hsieh <khsieh(a)codeaurora.org> --- drivers/gpu/drm/msm/dp/dp_display.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 5a39da6..4992a049 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -587,7 +587,7 @@ static int dp_connect_pending_timeout(struct dp_display_private *dp, u32 data) state = dp->hpd_state; if (state == ST_CONNECT_PENDING) { - dp_display_enable(dp, 0); + DRM_ERROR("EV_CONNECT_PENDING_TIMEOUT error\n"); dp->hpd_state = ST_CONNECTED; } @@ -670,7 +670,7 @@ static int dp_disconnect_pending_timeout(struct dp_display_private *dp, u32 data state = dp->hpd_state; if (state == ST_DISCONNECT_PENDING) { - dp_display_disable(dp, 0); + DRM_ERROR("EV_DISCONNECT_PENDING_TIMEOUT error\n"); dp->hpd_state = ST_DISCONNECTED; } @@ -1272,7 +1272,7 @@ static int dp_pm_resume(struct device *dev) status = dp_catalog_link_is_connected(dp->catalog); - if (status) + if (status && dp->link->sink_count) dp->dp_display.is_connected = true; else dp->dp_display.is_connected = false; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project

2 2

← Newer
1
...
47
48
49
50
51
52
53
...
72
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

dri-devel April 2021