mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Jayant Shekhar <jayant.shekhar@oss.qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>,
	Jayant Shekhar <jayant.shekhar@oss.qualcomm.com>
Cc: "Rob Clark" <robin.clark@oss.qualcomm.com>,
	"Dmitry Baryshkov" <lumag@kernel.org>,
	"Abhinav Kumar" <abhinav.kumar@linux.dev>,
	"Jessica Zhang" <jesszhan0024@gmail.com>,
	"Sean Paul" <sean@poorly.run>,
	"Marijn Suijten" <marijn.suijten@somainline.org>,
	"Maarten Lankhorst" <maarten.lankhorst@linux.intel.com>,
	"Maxime Ripard" <mripard@kernel.org>,
	"Thomas Zimmermann" <tzimmermann@suse.de>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Rob Herring" <robh@kernel.org>,
	"Krzysztof Kozlowski" <krzk+dt@kernel.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	"Sumit Semwal" <sumit.semwal@linaro.org>,
	"Christian König" <christian.koenig@amd.com>,
	"Philipp Stanner" <phasta@kernel.org>,
	"Bjorn Andersson" <andersson@kernel.org>,
	"Konrad Dybcio" <konradybcio@kernel.org>,
	"Abel Vesa" <abelvesa@kernel.org>,
	"Joerg Roedel (AMD)" <joro@8bytes.org>,
	"Will Deacon" <will@kernel.org>,
	"Robin Murphy" <robin.murphy@arm.com>,
	linux-arm-msm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	freedreno@lists.freedesktop.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-media@vger.kernel.org,
	linaro-mm-sig@lists.linaro.org, iommu@lists.linux.dev
Subject: Re: [PATCH 10/13] drm/msm/dcp: Add VirtIO transport and platform driver
Date: Fri, 23 Oct 2026 13:39:17 +0530	[thread overview]
Message-ID: <524d5dfc-c6fe-4346-9264-c5ec98850b23@oss.qualcomm.com> (raw)
In-Reply-To: <gegmh2qb7odywjesxlvrm6knjjwsx6g2xfif2s6wq5zn7y33q4@gpdwz2ajlm4i>


On 15-10-2026 18:36, Dmitry Baryshkov wrote:
> On Thu, Oct 15, 2026 at 01:08:30PM +0530, Jayant Shekhar wrote:
>> DCP display hardware is controlled by firmware and exposed to Linux as a
>> VirtIO function. The MSM DRM driver needs a platform driver to establish
>> the transport, DMA/IOMMU address spaces, power-notify path, and display
>> component.
>>
>> Add the VirtIO-MMIO transport and DCP platform-function driver. Set up the
>> DMA/IOMMU identity, IPCC client, VirtIO queues, HFI transport, firmware
>> capability handling, and MSM DRM registration. Synchronize suspend, resume,
>> remove, and shutdown with in-flight HFI activity.
>>
>> Signed-off-by: Jayant Shekhar <jayant.shekhar@oss.qualcomm.com>
>> Assisted-by: LLM
>> ---
>>   drivers/gpu/drm/msm/Makefile         |   2 +
>>   drivers/gpu/drm/msm/dcp/dcp_drv.c    | 245 ++++++++++++++++++++++++++++
>>   drivers/gpu/drm/msm/dcp/dcp_drv.h    |  19 +++
>>   drivers/gpu/drm/msm/dcp/dcp_virtio.c | 302 +++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/msm/dcp/dcp_virtio.h |  22 +++
>>   5 files changed, 590 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
>> index 9910bd4f306d..e57aee483137 100644
>> --- a/drivers/gpu/drm/msm/Makefile
>> +++ b/drivers/gpu/drm/msm/Makefile
>> @@ -83,6 +83,8 @@ msm-display-$(CONFIG_DRM_MSM_DCP) += \
>>   	dcp/dcp_ipcc.o \
>>   	dcp/dcp_plane.o \
>>   	dcp/dcp_encoder.o \
>> +	dcp/dcp_drv.o \
>> +	dcp/dcp_virtio.o \
>>   	dcp/hfi/hfi_packer.o \
>>   	dcp/hfi/hfi_unpacker.o \
>>   	dcp/hfi/hfi_utils.o
>> diff --git a/drivers/gpu/drm/msm/dcp/dcp_drv.c b/drivers/gpu/drm/msm/dcp/dcp_drv.c
>> new file mode 100644
>> index 000000000000..e7016937a028
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/dcp/dcp_drv.c
>> @@ -0,0 +1,245 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + * DCP VirtIO driver.
>> + */
>> +
>> +#include <linux/component.h>
>> +#include <linux/device.h>
>> +#include <linux/dma-mapping.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/panic_notifier.h>
>> +#include <linux/pm_runtime.h>
>> +
>> +#define DCP_DPU_AUTOSUSPEND_DELAY_MS	5000
>> +
>> +#include "dcp_drv.h"
>> +#include "dcp_hfi.h"
>> +#include "dcp_ipcc.h"
>> +#include "dcp_virtio.h"
>> +#include "msm_drv.h"
>> +#include "msm_kms.h"
>> +
>> +int dcp_subsystem_dev_init(struct dcp_drv *drv)
>> +{
>> +	struct virtio_device *vdev = drv->vdev;
>> +	struct device_node *dcp_subsystem_node;
>> +	struct platform_device *pdev;
>> +	struct device *dev;
>> +	char busid[32];
>> +	int ret;
>> +
>> +	dcp_subsystem_node = of_get_child_by_name(vdev->dev.parent->of_node,
>> +						  "dcp-subsystem");
>> +	if (!dcp_subsystem_node) {
>> +		dev_err(&vdev->dev, "no dcp-subsystem DT node\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	snprintf(busid, sizeof(busid), "dcp-subsystem.%s", dev_name(vdev->dev.parent));
>> +
>> +	/*
>> +	 * DCP is discovered through VirtIO, but display/HFI buffers must be
>> +	 * mapped through the dcp-subsystem node's DMA/IOMMU identity. Create a
>> +	 * platform device for that DT node so DMA API and msm_kms_init_vm() use
>> +	 * the display function's IOMMU domain.
>> +	 */
>> +	pdev = of_platform_device_create(dcp_subsystem_node, busid, vdev->dev.parent);
>> +	if (!pdev) {
>> +		dev_err(&vdev->dev, "dcp_subsystem_dev creation failed\n");
>> +		of_node_put(dcp_subsystem_node);
>> +		return -ENODEV;
>> +	}
>> +	dev = &pdev->dev;
>> +
>> +	dev->dma_parms = &drv->dcp_subsystem_dma_parms;
>> +	dma_set_max_seg_size(dev, UINT_MAX);
>> +
>> +	ret = of_dma_configure_id(dev, dcp_subsystem_node, true, NULL);
>> +	if (ret) {
>> +		dev_err(dev, "of_dma_configure_id failed: %d\n", ret);
>> +		goto err_destroy;
>> +	}
>> +
>> +	if (!device_iommu_mapped(dev)) {
>> +		dev_err(dev, "no IOMMU group attached\n");
>> +		ret = -ENODEV;
>> +		goto err_destroy;
>> +	}
>> +
>> +	dev_dbg(dev, "dcp-subsystem: iommu_mapped=%d iommu_group=%d\n",
>> +		device_iommu_mapped(dev),
>> +		 dev->iommu_group ? iommu_group_id(dev->iommu_group) : -1);
>> +
>> +	pm_runtime_set_active(dev);
>> +	pm_runtime_enable(dev);
>> +	pm_runtime_set_autosuspend_delay(dev, DCP_DPU_AUTOSUSPEND_DELAY_MS);
>> +	pm_runtime_use_autosuspend(dev);
>> +
>> +	drv->dcp_subsystem_dev = pdev;
>> +	drv->dcp_subsystem_node = dcp_subsystem_node;
>> +
>> +	return 0;
>> +
>> +err_destroy:
>> +	of_platform_device_destroy(dev, NULL);
>> +	of_node_put(dcp_subsystem_node);
>> +	return ret;
>> +}
>> +
>> +void dcp_subsystem_dev_deinit(struct dcp_drv *drv)
>> +{
>> +	if (!drv->dcp_subsystem_dev)
>> +		return;
>> +
>> +	pm_runtime_disable(&drv->dcp_subsystem_dev->dev);
>> +	of_platform_device_destroy(&drv->dcp_subsystem_dev->dev, NULL);
>> +	of_node_put(drv->dcp_subsystem_node);
>> +	drv->dcp_subsystem_dev = NULL;
>> +}
>> +
>> +int dcp_pm_prepare(struct device *vdev_dev)
>> +{
>> +	struct dcp_drv *drv = vdev_dev ? ((struct virtio_device *)
>> +					  dev_to_virtio(vdev_dev))->priv : NULL;
>> +
>> +	if (!drv || !drv->dcp_subsystem_dev)
>> +		return 0;
>> +
>> +	return msm_kms_pm_prepare(&drv->dcp_subsystem_dev->dev);
>> +}
>> +
>> +void dcp_pm_complete(struct device *vdev_dev)
>> +{
>> +	struct dcp_drv *drv = vdev_dev ? ((struct virtio_device *)
>> +					  dev_to_virtio(vdev_dev))->priv : NULL;
>> +
>> +	if (!drv || !drv->dcp_subsystem_dev)
>> +		return;
>> +
>> +	msm_kms_pm_complete(&drv->dcp_subsystem_dev->dev);
>> +}
>> +
>> +static int dcp_pm_ipcc_notify(struct dcp_drv *drv)
>> +{
>> +	int ret;
>> +
>> +	if (!drv->ipcc_valid)
>> +		return 0;
>> +
>> +	ret = dcp_ipcc_send_power_notify(&drv->ipcc);
> An indirection just for the sake of indirection? Can we drop one-line
> wrappers?
>

Will fix this in v2


>> +	if (ret)
>> +		dev_warn(&drv->vdev->dev,
>> +			 "IPCC power notification failed during PM: %d\n", ret);
>> +
>> +	return ret;
>> +}
>> +
>> +int dcp_pm_suspend(struct device *vdev_dev)
>> +{
>> +	struct dcp_drv *drv = vdev_dev ? ((struct virtio_device *)
>> +					  dev_to_virtio(vdev_dev))->priv : NULL;
>> +
>> +	if (!drv || !drv->dcp_subsystem_dev)
>> +		return 0;
>> +
>> +	return dcp_pm_ipcc_notify(drv);
>> +}
>> +
>> +int dcp_pm_resume(struct device *vdev_dev)
>> +{
>> +	struct dcp_drv *drv = vdev_dev ? ((struct virtio_device *)
>> +					  dev_to_virtio(vdev_dev))->priv : NULL;
>> +
>> +	if (!drv || !drv->dcp_subsystem_dev)
>> +		return 0;
>> +
>> +	return dcp_pm_ipcc_notify(drv);
>> +}
>> +
>> +static int dcp_self_bind(struct device *master, struct device *dev, void *data)
>> +{
>> +	return 0;
>> +}
>> +
>> +static void dcp_self_unbind(struct device *master, struct device *dev,
>> +			    void *data)
>> +{
>> +}
>> +
>> +static const struct component_ops dcp_self_ops = {
>> +	.bind   = dcp_self_bind,
>> +	.unbind = dcp_self_unbind,
>> +};
>> +
>> +int dcp_component_register(struct dcp_drv *drv)
>> +{
>> +	struct device *dev = &drv->dcp_subsystem_dev->dev;
>> +	struct component_match *match = NULL;
>> +	int ret;
>> +
>> +	/*
>> +	 * DCP has no host-visible display subcomponents: firmware owns the
>> +	 * underlying display blocks. The self component is only the minimal shim
>> +	 * needed to enter the existing MSM DRM component-master init path without
>> +	 * adding a DCP-specific entry point to common MSM DRM code.
>> +	 */
> And how does that work with the DSI probing sequences? The panel driver
> might be not (yet) available when you probe the DCP. The driver should
> register the DSI host, create DSI devices, but the DRM device
> registration is only possible when the panel / bridge has attached to
> the DSI host. See how it's handled by the mdp4/mdp5/dpu drivers.
>

I will add dsi component binding in v2.


>> +	ret = component_add(dev, &dcp_self_ops);
>> +	if (ret) {
>> +		dev_err(dev, "component_add failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	component_match_add(dev, &match, component_compare_dev, dev);
> Missing optional GPU match. I think I wrote that before...
>

Will add in v2.


>> +
>> +	ret = component_master_add_with_match(dev, &msm_drm_ops, match);
>> +	if (ret) {
>> +		dev_err(dev, "component_master_add_with_match failed: %d\n",
>> +			ret);
>> +		component_del(dev, &dcp_self_ops);
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +void dcp_component_unregister(struct dcp_drv *drv)
>> +{
>> +	struct device *dev = &drv->dcp_subsystem_dev->dev;
>> +
>> +	component_master_del(dev, &msm_drm_ops);
>> +	component_del(dev, &dcp_self_ops);
>> +}
>> +
>> +static int dcp_panic_notifier_cb(struct notifier_block *nb,
>> +				 unsigned long action, void *data)
>> +{
>> +	struct dcp_drv *drv = container_of(nb, struct dcp_drv, panic_notifier);
>> +
>> +	if (drv->hfi)
>> +		atomic_set(&drv->hfi->shutdown_in_progress, 1);
>> +
>> +	return NOTIFY_OK;
>> +}
>> +
>> +void dcp_panic_notifier_register(struct dcp_drv *drv)
>> +{
>> +	drv->panic_notifier.notifier_call = dcp_panic_notifier_cb;
>> +	atomic_notifier_chain_register(&panic_notifier_list, &drv->panic_notifier);
>> +}
>> +
>> +void dcp_panic_notifier_unregister(struct dcp_drv *drv)
>> +{
>> +	atomic_notifier_chain_unregister(&panic_notifier_list, &drv->panic_notifier);
>> +}
>> +
>> +void __init msm_dcp_register(void)
>> +{
>> +	dcp_virtio_register();
>> +}
>> +
>> +void __exit msm_dcp_unregister(void)
>> +{
>> +	dcp_virtio_unregister();
>> +}
>> diff --git a/drivers/gpu/drm/msm/dcp/dcp_drv.h b/drivers/gpu/drm/msm/dcp/dcp_drv.h
>> index af2db3cf9066..ca8e5bd284e0 100644
>> --- a/drivers/gpu/drm/msm/dcp/dcp_drv.h
>> +++ b/drivers/gpu/drm/msm/dcp/dcp_drv.h
>> @@ -26,6 +26,25 @@ struct dcp_drv {
>>   	struct dcp_dpu_kms	*kms;
>>   	struct dcp_caps		 caps;
>>   	struct notifier_block	 panic_notifier;
>> +	struct dcp_ipcc		 ipcc;
>> +	bool			 ipcc_valid;
>>   };
>>   
>> +int dcp_subsystem_dev_init(struct dcp_drv *drv);
>> +void dcp_subsystem_dev_deinit(struct dcp_drv *drv);
>> +
>> +int dcp_component_register(struct dcp_drv *drv);
>> +void dcp_component_unregister(struct dcp_drv *drv);
>> +
>> +int dcp_pm_prepare(struct device *vdev_dev);
>> +void dcp_pm_complete(struct device *vdev_dev);
>> +int dcp_pm_suspend(struct device *vdev_dev);
>> +int dcp_pm_resume(struct device *vdev_dev);
>> +
>> +void dcp_panic_notifier_register(struct dcp_drv *drv);
>> +void dcp_panic_notifier_unregister(struct dcp_drv *drv);
>> +
>> +void __init msm_dcp_register(void);
>> +void __exit msm_dcp_unregister(void);
>> +
>>   #endif /* __DCP_DRV_H__ */
>> diff --git a/drivers/gpu/drm/msm/dcp/dcp_virtio.c b/drivers/gpu/drm/msm/dcp/dcp_virtio.c
>> new file mode 100644
>> index 000000000000..b2aeb03688c9
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/dcp/dcp_virtio.c
>> @@ -0,0 +1,302 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + * DCP VirtIO device driver.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/err.h>
>> +#include <linux/pm_runtime.h>
>> +#include <linux/virtio.h>
>> +#include <linux/virtio_config.h>
>> +#include <linux/scatterlist.h>
>> +#include <linux/slab.h>
>> +
>> +#include "dcp_drv.h"
>> +#include "dcp_dpu_kms.h"
>> +#include "dcp_hfi.h"
>> +#include "dcp_ipcc.h"
>> +#include "dcp_virtio.h"
>> +#include "msm_drv.h"
>> +#include "msm_kms.h"
>> +
>> +static void dcp_virtio_hfi_cleanup(struct dcp_drv *drv);
>> +
>> +static const unsigned int dcp_virtio_features[] = {
>> +	VIRTIO_F_VERSION_1,
>> +	VIRTIO_F_NOTIFICATION_DATA,
>> +	VIRTIO_F_RING_RESET,
>> +	VIRTIO_F_ACCESS_PLATFORM,
>> +};
>> +
>> +/*
>> + * The VirtIO core owns transport discovery. This driver owns HFI setup and
>> + * creates the DRM component only after firmware capabilities are available.
>> + */
>> +static int dcp_virtio_probe(struct virtio_device *vdev)
>> +{
>> +	struct dcp_drv *drv;
>> +	struct virtqueue *vqs[DCP_NUM_VQS];
>> +	struct virtqueue_info vqs_info[DCP_NUM_VQS] = {
>> +		[DCP_VQ_CMD]  = { .name = "cmd",  .callback = dcp_hfi_cmd_callback },
>> +		[DCP_VQ_RESP] = { .name = "resp", .callback = dcp_hfi_resp_callback },
>> +	};
>> +	struct dcp_dpu_kms *dpu_kms;
>> +	struct msm_drm_private *priv;
>> +	int ret;
>> +
>> +	dev_dbg(&vdev->dev, "DCP VirtIO probe started\n");
>> +
>> +	drv = devm_kzalloc(&vdev->dev, sizeof(*drv), GFP_KERNEL);
>> +	if (!drv)
>> +		return -ENOMEM;
>> +
>> +	drv->vdev = vdev;
>> +	vdev->priv = drv;
>> +
>> +	if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>> +		dev_err(&vdev->dev, "VIRTIO_F_VERSION_1 not negotiated\n");
>> +		return -ENODEV;
>> +	}
>> +	/* Access-platform is required for the firmware-visible IOVA mappings. */
>> +	if (!virtio_has_feature(vdev, VIRTIO_F_ACCESS_PLATFORM)) {
>> +		dev_err(&vdev->dev, "VIRTIO_F_ACCESS_PLATFORM not negotiated\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	ret = dcp_subsystem_dev_init(drv);
>> +	if (ret)
>> +		return ret;
>> +
>> +	ret = dcp_ipcc_init(&drv->ipcc, vdev->dev.parent);
>> +	if (ret) {
>> +		if (ret == -EPROBE_DEFER)
>> +			dev_err_probe(&vdev->dev, ret,
>> +				      "IPCC not ready, deferring probe\n");
>> +		else
>> +			dev_err(&vdev->dev, "IPCC init failed: %d\n", ret);
>> +		goto err_fn_dev;
>> +	}
>> +	drv->ipcc_valid = !!drv->ipcc.power_chan;
>> +
>> +	ret = virtio_find_vqs(vdev, DCP_NUM_VQS, vqs, vqs_info, NULL);
>> +	if (ret) {
>> +		dev_err(&vdev->dev, "virtqueue allocation failed: %d\n", ret);
>> +		goto err_ipcc;
>> +	}
>> +
>> +	ret = dcp_hfi_init(drv, vdev, vqs[DCP_VQ_CMD], vqs[DCP_VQ_RESP],
>> +			   vdev->dev.parent);
>> +	if (ret) {
>> +		dev_err(&vdev->dev, "HFI init failed: %d\n", ret);
>> +		goto err_del_vqs;
>> +	}
>> +
>> +	if (drv->ipcc_valid)
>> +		dcp_ipcc_set_resp_vq(&drv->ipcc, vqs[DCP_VQ_RESP]);
>> +
>> +	/*
>> +	 * Keep the device unready until HFI state and virtqueues are initialized.
>> +	 * KMS registration below depends on synchronous firmware initialization.
>> +	 */
>> +	virtio_device_ready(vdev);
>> +	virtqueue_kick(drv->hfi->resp_vq);
>> +
>> +	ret = dcp_hfi_send_device_init(drv->hfi);
>> +	if (ret) {
>> +		dev_err(&vdev->dev, "HFI DEVICE_INIT failed: %d\n", ret);
>> +		goto err_hfi_deinit;
>> +	}
>> +
>> +	ret = dcp_hfi_get_capabilities(drv->hfi, &drv->caps);
>> +	if (ret) {
>> +		dev_err(&vdev->dev, "HFI GET_CAPS failed: %d\n", ret);
>> +		goto err_hfi_deinit;
>> +	}
>> +
>> +	/* Register the display resources before exposing the DRM component. */
>> +	ret = dcp_hfi_send_resource_register(drv->hfi);
>> +	if (ret) {
>> +		dev_err(&vdev->dev, "HFI RESOURCE_REGISTER failed: %d\n", ret);
>> +		goto err_hfi_deinit;
>> +	}
>> +
>> +	dev_dbg(&vdev->dev,
>> +		"DCP caps: %u displays, max %ux%u, %u planes, features=0x%x\n",
>> +		 drv->caps.num_crtcs, drv->caps.max_width, drv->caps.max_height,
>> +		 drv->caps.max_planes, drv->caps.features);
>> +
>> +	dpu_kms = devm_kzalloc(&drv->dcp_subsystem_dev->dev, sizeof(*dpu_kms), GFP_KERNEL);
>> +	if (!dpu_kms) {
>> +		ret = -ENOMEM;
>> +		goto err_hfi_deinit;
>> +	}
>> +
>> +	dpu_kms->drv       = drv;
>> +	dpu_kms->pm_dev    = &drv->dcp_subsystem_dev->dev;
>> +	dpu_kms->hfi       = drv->hfi;
>> +	dpu_kms->caps      = &drv->caps;
>> +	dpu_kms->dpus_np   = drv->dcp_subsystem_node;
>> +	dpu_kms->iommu_dev = &drv->dcp_subsystem_dev->dev;
>> +
>> +	ret = msm_kms_init(&dpu_kms->base, &dcp_dpu_kms_funcs);
>> +	if (ret) {
>> +		dev_err(&vdev->dev, "msm_kms_init failed: %d\n", ret);
>> +		goto err_hfi_deinit;
>> +	}
>> +
>> +	priv = devm_kzalloc(&drv->dcp_subsystem_dev->dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv) {
>> +		ret = -ENOMEM;
>> +		goto err_hfi_deinit;
>> +	}
>> +
>> +	priv->kms      = &dpu_kms->base;
>> +	priv->kms_init = dcp_kms_init;
>> +	dev_set_drvdata(&drv->dcp_subsystem_dev->dev, priv);
>> +	dpu_kms->rpm_enabled = true;
>> +
>> +	/* Component binding now sees initialized HFI, capabilities, and IOMMU state. */
>> +	ret = dcp_component_register(drv);
>> +	if (ret) {
>> +		dev_err(&vdev->dev, "component register failed: %d\n", ret);
>> +		goto err_hfi_deinit;
>> +	}
>> +
>> +	dcp_panic_notifier_register(drv);
>> +
>> +	dev_dbg(&vdev->dev, "DCP VirtIO probe complete\n");
>> +	return 0;
>> +
>> +err_hfi_deinit:
>> +	dcp_virtio_hfi_cleanup(drv);
>> +	goto err_ipcc;
>> +err_del_vqs:
>> +	virtio_reset_device(vdev);
>> +	vdev->config->del_vqs(vdev);
>> +err_ipcc:
>> +	if (drv->ipcc_valid) {
>> +		dcp_ipcc_deinit(&drv->ipcc);
>> +		drv->ipcc_valid = false;
>> +	}
>> +err_fn_dev:
>> +	dcp_subsystem_dev_deinit(drv);
>> +	return ret;
>> +}
>> +
>> +static void dcp_virtio_hfi_cleanup(struct dcp_drv *drv)
>> +{
>> +	struct virtio_device *vdev = drv->vdev;
>> +
>> +	if (!drv->hfi)
>> +		return;
>> +
>> +	dcp_hfi_stop(drv->hfi);
>> +	virtio_reset_device(vdev);
>> +	virtio_synchronize_cbs(vdev);
>> +	vdev->priv = NULL;
>> +	dcp_hfi_quiesce_queues(drv->hfi);
>> +	vdev->config->del_vqs(vdev);
>> +	dcp_hfi_deinit(drv->hfi);
>> +	drv->hfi = NULL;
> What's the point of setting pointers to NULL? Won't this memory be
> kfree()'d soon?
>

Will be removed in v2.


>> +}
>> +
>> +/*
>> + * Remove DRM users before tearing down HFI, IPCC, or the VirtIO queues.
>> + */
>> +static void dcp_virtio_remove(struct virtio_device *vdev)
>> +{
>> +	struct dcp_drv *drv = vdev->priv;
>> +
>> +	dev_dbg(&vdev->dev, "DCP VirtIO remove\n");
>> +
>> +	if (!drv)
>> +		return;
>> +
>> +	dcp_component_unregister(drv);
>> +	dcp_panic_notifier_unregister(drv);
>> +
>> +	dcp_virtio_hfi_cleanup(drv);
>> +
>> +	if (drv->ipcc_valid) {
>> +		dcp_ipcc_deinit(&drv->ipcc);
>> +		drv->ipcc_valid = false;
>> +	}
>> +
>> +	dcp_subsystem_dev_deinit(drv);
>> +}
>> +
>> +/* KMS shutdown must complete while the firmware transport is available. */
>> +static void dcp_virtio_shutdown(struct virtio_device *vdev)
>> +{
>> +	struct dcp_drv *drv = vdev->priv;
>> +
>> +	if (!drv || !drv->dcp_subsystem_dev)
>> +		return;
>> +
>> +	pm_runtime_get_sync(&drv->dcp_subsystem_dev->dev);
>> +	msm_kms_shutdown(drv->dcp_subsystem_dev);
>> +	pm_runtime_put_noidle(&drv->dcp_subsystem_dev->dev);
>> +}
>> +
>> +/*
>> + * Firmware capabilities may change independently of the VirtIO transport;
>> + * refresh them without rebuilding the DRM device.
> And what does that mean? How can they change?
>

Will be removed.


>> + */
>> +static void dcp_virtio_config_changed(struct virtio_device *vdev)
>> +{
>> +	struct dcp_drv *drv = vdev->priv;
>> +	int ret;
>> +
>> +	dev_dbg(&vdev->dev, "VirtIO config changed — re-querying DCP capabilities\n");
>> +
>> +	if (!drv || !drv->hfi)
>> +		return;
>> +
>> +	ret = dcp_hfi_get_capabilities(drv->hfi, &drv->caps);
>> +	if (ret)
>> +		dev_err(&vdev->dev,
>> +			"config_changed: capability re-query failed: %d\n", ret);
>> +	else
>> +		dev_dbg(&vdev->dev,
>> +			"config_changed: DCP caps updated: %u displays, features=0x%x\n",
>> +			 drv->caps.num_crtcs, drv->caps.features);
>> +}
>> +
> [...]
>
>> diff --git a/drivers/gpu/drm/msm/dcp/dcp_virtio.h b/drivers/gpu/drm/msm/dcp/dcp_virtio.h
>> new file mode 100644
>> index 000000000000..fe244bc5b51a
>> --- /dev/null
>> +++ b/drivers/gpu/drm/msm/dcp/dcp_virtio.h
>> @@ -0,0 +1,22 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + * DCP VirtIO driver interface.
>> + */
>> +
>> +#ifndef __DCP_VIRTIO_H__
>> +#define __DCP_VIRTIO_H__
>> +
>> +#include <linux/device.h>
>> +#include <linux/virtio.h>
>> +#include <linux/virtio_ids.h>
>> +
>> +struct dcp_drv;
>> +
>> +#define VIRTIO_ID_QPSFI		                52
>> +#define VIRTIO_VENDOR_ID_DCP_HFI		    1
> I think this was questioned by Bjorn. Is it described somewhere? Is it a
> part of the VirtIO spec? If so, add these ids to the VirtIO headers. If
> not... then it's squatting. We can't just randomly settle on the ID.
>

I will add patch to move VIRTIO_ID_QPSFI to virtio_ids.h


>> +
>> +int dcp_virtio_register(void);
>> +void dcp_virtio_unregister(void);
>> +
>> +#endif /* __DCP_VIRTIO_H__ */
>>
>> -- 
>> 2.43.0
>>

  reply	other threads:[~2026-10-23  8:09 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-15  7:38 [PATCH 00/13] drm/msm/dcp: Add Qualcomm DCP display support Jayant Shekhar
2026-10-15  7:38 ` [PATCH 01/13] dt-bindings: display: msm: Add Qualcomm DCP bindings Jayant Shekhar
2026-10-15 10:50   ` Krzysztof Kozlowski
2026-10-23  8:10     ` Jayant Shekhar
2026-10-23  8:24       ` Krzysztof Kozlowski
2026-10-15 12:17   ` Dmitry Baryshkov
2026-10-21  8:45     ` Jayant Shekhar
2026-10-21 11:39       ` Jayant Shekhar
2026-10-22 12:52         ` Dmitry Baryshkov
2026-10-22 12:58           ` Dmitry Baryshkov
2026-10-22 12:55         ` Dmitry Baryshkov
2026-10-15  7:38 ` [PATCH 02/13] drm/msm/dcp: Add HFI protocol definitions Jayant Shekhar
2026-10-15  7:38 ` [PATCH 03/13] drm/msm/dcp: Add HFI transport over VirtIO Jayant Shekhar
2026-10-15 12:36   ` Dmitry Baryshkov
2026-10-23  7:00     ` Jayant Shekhar
2026-10-15  7:38 ` [PATCH 04/13] drm/msm/dcp: Add IPCC power notification Jayant Shekhar
2026-10-15 12:42   ` Dmitry Baryshkov
2026-10-23  7:07     ` Jayant Shekhar
2026-10-15  7:38 ` [PATCH 05/13] drm/msm/dcp: Add HFI display command implementations Jayant Shekhar
2026-10-15  7:38 ` [PATCH 06/13] drm/msm/dcp: Add planes, CRTCs, and HFI event routing Jayant Shekhar
2026-10-15 13:18   ` Dmitry Baryshkov
2026-10-23  7:11     ` Jayant Shekhar
2026-10-15  7:38 ` [PATCH 07/13] drm/msm/dcp: Add DCP encoder Jayant Shekhar
2026-10-15 12:44   ` Dmitry Baryshkov
2026-10-15  7:38 ` [PATCH 08/13] drm/msm/dcp: Add DSI bridge Jayant Shekhar
2026-10-15 12:56   ` Dmitry Baryshkov
2026-10-23  7:17     ` Jayant Shekhar
2026-10-15  7:38 ` [PATCH 09/13] drm/msm/dcp: Register DCP display pipeline Jayant Shekhar
2026-10-21 12:05   ` Krzysztof Kozlowski
2026-10-23  8:13     ` Jayant Shekhar
2026-10-21 12:05   ` Krzysztof Kozlowski
2026-10-23  8:12     ` Jayant Shekhar
2026-10-15  7:38 ` [PATCH 10/13] drm/msm/dcp: Add VirtIO transport and platform driver Jayant Shekhar
2026-10-15 13:06   ` Dmitry Baryshkov
2026-10-23  8:09     ` Jayant Shekhar [this message]
2026-10-15  7:38 ` [PATCH 11/13] drm/msm: Support KMS backends without a display IRQ Jayant Shekhar
2026-10-15  7:38 ` [PATCH 12/13] drm/msm: Register the DCP backend Jayant Shekhar
2026-10-15  7:38 ` [PATCH 13/13] arm64: dts: qcom: hawi: Add DCP support Jayant Shekhar
2026-10-15 10:52   ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=524d5dfc-c6fe-4346-9264-c5ec98850b23@oss.qualcomm.com \
    --to=jayant.shekhar@oss.qualcomm.com \
    --cc=abelvesa@kernel.org \
    --cc=abhinav.kumar@linux.dev \
    --cc=airlied@gmail.com \
    --cc=andersson@kernel.org \
    --cc=christian.koenig@amd.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.baryshkov@oss.qualcomm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=freedreno@lists.freedesktop.org \
    --cc=iommu@lists.linux.dev \
    --cc=jesszhan0024@gmail.com \
    --cc=joro@8bytes.org \
    --cc=konradybcio@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=lumag@kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=marijn.suijten@somainline.org \
    --cc=mripard@kernel.org \
    --cc=phasta@kernel.org \
    --cc=robh@kernel.org \
    --cc=robin.clark@oss.qualcomm.com \
    --cc=robin.murphy@arm.com \
    --cc=sean@poorly.run \
    --cc=simona@ffwll.ch \
    --cc=sumit.semwal@linaro.org \
    --cc=tzimmermann@suse.de \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®