Re: [PATCH v8] i2c: virtio: add a virtio i2c frontend driver
On 2021/3/19 11:54, Viresh Kumar wrote: On 18-03-21, 15:52, Arnd Bergmann wrote: Allowing multiple virtio-i2c controllers in one system, and multiple i2c devices attached to each controller is clearly something that has to work. Good. I don't actually see a limitation though. Viresh, what is the problem you see for having multiple controllers? I thought this would be a problem in that case as we are using the global virtio_adapter here. + vi->adap = _adapter; + i2c_set_adapdata(vi->adap, vi); Multiple calls to probe() will end up updating the same pointer inside adap. + vi->adap->dev.parent = >dev; Same here, overwrite. + /* Setup ACPI node for controlled devices which will be probed through ACPI */ + ACPI_COMPANION_SET(>adap->dev, ACPI_COMPANION(pdev)); + vi->adap->timeout = HZ / 10; These may be fine, but still not ideal I believe. + ret = i2c_add_adapter(vi->adap); i This should be a problem as well, we must be adding this to some sort of list, doing some RPM stuff, etc ? Jie, the solution is to allocate memory for adap at runtime in probe and remove the virtio_adapter structure completely. If you want to support that. Then I think we don't need to change the following at all. + .algo = _algorithm, + + return ret; + + vi->adap = virtio_adapter; This is strange, why are you allocating memory for adapter twice ? Once for virtio_adapter and once for vi->adap ? Either fill the fields directly for v->adap here and remove virtio_adapter or make vi->adap a pointer. ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 2/3] ACPI: Add driver for the VIOT table
On 2021-03-16 19:16, Jean-Philippe Brucker wrote:
The ACPI Virtual I/O Translation Table describes topology of
para-virtual platforms. For now it describes the relation between
virtio-iommu and the endpoints it manages. Supporting that requires
three steps:
(1) acpi_viot_init(): parse the VIOT table, build a list of endpoints
and vIOMMUs.
(2) acpi_viot_set_iommu_ops(): when the vIOMMU driver is loaded and the
device probed, register it to the VIOT driver. This step is required
because unlike similar drivers, VIOT doesn't create the vIOMMU
device.
Note that you're basically the same as the DT case in this regard, so
I'd expect things to be closer to that pattern than to that of IORT.
[...]
@@ -1506,12 +1507,17 @@ int acpi_dma_configure_id(struct device *dev, enum
dev_dma_attr attr,
{
const struct iommu_ops *iommu;
u64 dma_addr = 0, size = 0;
+ int ret;
if (attr == DEV_DMA_NOT_SUPPORTED) {
set_dma_ops(dev, _dummy_ops);
return 0;
}
+ ret = acpi_viot_dma_setup(dev, attr);
+ if (ret)
+ return ret > 0 ? 0 : ret;
I think things could do with a fair bit of refactoring here. Ideally we
want to process a possible _DMA method (acpi_dma_get_range()) regardless
of which flavour of IOMMU table might be present, and the amount of
duplication we fork into at this point is unfortunate.
+
iort_dma_setup(dev, _addr, );
For starters I think most of that should be dragged out to this level
here - it's really only the {rc,nc}_dma_get_range() bit that deserves to
be the IORT-specific call.
iommu = iort_iommu_configure_id(dev, input_id);
Similarly, it feels like it's only the table scan part in the middle of
that that needs dispatching between IORT/VIOT, and its head and tail
pulled out into a common path.
[...]
+static const struct iommu_ops *viot_iommu_setup(struct device *dev)
+{
+ struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
+ struct viot_iommu *viommu = NULL;
+ struct viot_endpoint *ep;
+ u32 epid;
+ int ret;
+
+ /* Already translated? */
+ if (fwspec && fwspec->ops)
+ return NULL;
+
+ mutex_lock(_lock);
+ list_for_each_entry(ep, _endpoints, list) {
+ if (viot_device_match(dev, >dev_id, )) {
+ epid += ep->endpoint_id;
+ viommu = ep->viommu;
+ break;
+ }
+ }
+ mutex_unlock(_lock);
+ if (!viommu)
+ return NULL;
+
+ /* We're not translating ourself */
+ if (viot_device_match(dev, >dev_id, ))
+ return NULL;
+
+ /*
+* If we found a PCI range managed by the viommu, we're the one that has
+* to request ACS.
+*/
+ if (dev_is_pci(dev))
+ pci_request_acs();
+
+ if (!viommu->ops || WARN_ON(!viommu->dev))
+ return ERR_PTR(-EPROBE_DEFER);
Can you create (or look up) a viommu->fwnode when initially parsing the
VIOT to represent the IOMMU devices to wait for, such that the
viot_device_match() lookup can resolve to that and let you fall into the
standard iommu_ops_from_fwnode() path? That's what I mean about
following the DT pattern - I guess it might need a bit of trickery to
rewrite things if iommu_device_register() eventually turns up with a new
fwnode, so I doubt we can get away without *some* kind of private
interface between virtio-iommu and VIOT, but it would be nice for the
common(ish) DMA paths to stay as unaware of the specifics as possible.
+
+ ret = iommu_fwspec_init(dev, viommu->dev->fwnode, viommu->ops);
+ if (ret)
+ return ERR_PTR(ret);
+
+ iommu_fwspec_add_ids(dev, , 1);
+
+ /*
+* If we have reason to believe the IOMMU driver missed the initial
+* add_device callback for dev, replay it to get things in order.
+*/
+ if (dev->bus && !device_iommu_mapped(dev))
+ iommu_probe_device(dev);
+
+ return viommu->ops;
+}
+
+/**
+ * acpi_viot_dma_setup - Configure DMA for an endpoint described in VIOT
+ * @dev: the endpoint
+ * @attr: coherency property of the endpoint
+ *
+ * Setup the DMA and IOMMU ops for an endpoint described by the VIOT table.
+ *
+ * Return:
+ * * 0 - @dev doesn't match any VIOT node
+ * * 1 - ops for @dev were successfully installed
+ * * -EPROBE_DEFER - ops for @dev aren't yet available
+ */
+int acpi_viot_dma_setup(struct device *dev, enum dev_dma_attr attr)
+{
+ const struct iommu_ops *iommu_ops = viot_iommu_setup(dev);
+
+ if (IS_ERR_OR_NULL(iommu_ops)) {
+ int ret = PTR_ERR(iommu_ops);
+
+ if (ret == -EPROBE_DEFER || ret == 0)
+ return ret;
+ dev_err(dev, "error %d while setting up virt IOMMU\n", ret);
+ return 0;
+ }
+
+#ifdef CONFIG_ARCH_HAS_SETUP_DMA_OPS
Re: [PATCH 3/3] iommu/virtio: Enable x86 support
On Tue, Mar 16, 2021 at 08:16:54PM +0100, Jean-Philippe Brucker wrote: > With the VIOT support in place, x86 platforms can now use the > virtio-iommu. > > The arm64 Kconfig selects IOMMU_DMA, while x86 IOMMU drivers select it > themselves. > > Signed-off-by: Jean-Philippe Brucker Acked-by: Michael S. Tsirkin > --- > drivers/iommu/Kconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index 2819b5c8ec30..ccca83ef2f06 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -400,8 +400,9 @@ config HYPERV_IOMMU > config VIRTIO_IOMMU > tristate "Virtio IOMMU driver" > depends on VIRTIO > - depends on ARM64 > + depends on (ARM64 || X86) > select IOMMU_API > + select IOMMU_DMA if X86 Would it hurt to just select unconditionally? Seems a bit cleaner ... > select INTERVAL_TREE > select ACPI_VIOT if ACPI > help > -- > 2.30.2 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [GIT PULL] vhost: cleanups and fixes
The pull request you sent on Thu, 18 Mar 2021 14:11:35 -0400: > https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus has been merged into torvalds/linux.git: https://git.kernel.org/torvalds/c/bf152b0b41dc141c8d32eb6e974408f5804f4d00 Thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/prtracker.html ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[GIT PULL] vhost: cleanups and fixes
The following changes since commit 16c10bede8b3d8594279752bf53153491f3f944f:
virtio-input: add multi-touch support (2021-02-23 07:52:59 -0500)
are available in the Git repository at:
https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus
for you to fetch changes up to 0bde59c1723a29e294765c96dbe5c7fb639c2f96:
vhost-vdpa: set v->config_ctx to NULL if eventfd_ctx_fdget() fails
(2021-03-14 18:10:07 -0400)
virtio: fixes, cleanups
Some fixes and cleanups all over the place.
Signed-off-by: Michael S. Tsirkin
Gautam Dawar (1):
vhost_vdpa: fix the missing irq_bypass_unregister_producer() invocation
Jason Wang (1):
vdpa: set the virtqueue num during register
Laurent Vivier (1):
vhost: Fix vhost_vq_reset()
Parav Pandit (1):
vdpa_sim: Skip typecasting from void*
Stefano Garzarella (2):
vhost-vdpa: fix use-after-free of v->config_ctx
vhost-vdpa: set v->config_ctx to NULL if eventfd_ctx_fdget() fails
Tang Bin (1):
virtio-mmio: Use to_virtio_mmio_device() to simply code
Xianting Tian (1):
virtio: remove export for virtio_config_{enable, disable}
drivers/vdpa/ifcvf/ifcvf_main.c | 5 ++---
drivers/vdpa/mlx5/net/mlx5_vnet.c| 4 ++--
drivers/vdpa/vdpa.c | 18 ++
drivers/vdpa/vdpa_sim/vdpa_sim.c | 2 +-
drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 5 ++---
drivers/vhost/vdpa.c | 20 +++-
drivers/vhost/vhost.c| 2 +-
drivers/virtio/virtio.c | 6 ++
drivers/virtio/virtio_mmio.c | 3 +--
include/linux/vdpa.h | 10 +-
include/linux/virtio.h | 2 --
11 files changed, 37 insertions(+), 40 deletions(-)
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [External] Re: [RFC PATCH] virtio-vsock: add description for datagram type
On Wed, Mar 17, 2021 at 8:45 AM Stefan Hajnoczi wrote:
>
> On Tue, Mar 16, 2021 at 09:56:44PM +, jiang.wang wrote:
> > Add supports for datagram type for virtio-vsock. Datagram
> > sockets are connectionless and unreliable. To avoid contention
> > with stream and other sockets, add two more virtqueues and
> > a new feature bit to identify if those two new queues exist or not.
> >
> > Also add descriptions for resouce management of datagram, which
> > does not use the existing credit update mechanism associated with
> > stream sockets.
> >
> > Signed-off-by: Jiang Wang
> > ---
> > virtio-vsock.tex | 72
> >
> > 1 file changed, 62 insertions(+), 10 deletions(-)
> >
> > diff --git a/virtio-vsock.tex b/virtio-vsock.tex
> > index da7e641..a2ff0a4 100644
> > --- a/virtio-vsock.tex
> > +++ b/virtio-vsock.tex
> > @@ -9,14 +9,27 @@ \subsection{Device ID}\label{sec:Device Types / Socket
> > Device / Device ID}
> >
> > \subsection{Virtqueues}\label{sec:Device Types / Socket Device /
> > Virtqueues}
> > \begin{description}
> > +\item[0] stream rx
> > +\item[1] stream tx
> > +\item[2] dgram rx
> > +\item[3] dgram tx
> > +\item[4] event
> > +\end{description}
> > +The virtio socket device uses 5 queues if feature bit VIRTIO_VSOCK_DRGAM
> > is set. Otherwise, it
> > +only uses 3 queues, as the following.
> > +
> > +\begin{description}
> > \item[0] rx
> > \item[1] tx
> > \item[2] event
> > \end{description}
> >
> > +
> > \subsection{Feature bits}\label{sec:Device Types / Socket Device / Feature
> > bits}
> >
> > -There are currently no feature bits defined for this device.
> > +\begin{description}
> > +\item[VIRTIO_VSOCK_DGRAM (0)] Device has support for datagram sockets type.
> > +\end{description}
>
> By convention feature bits are named VIRTIO__F_. Please
> add the "_F_".
>
Sure.
> >
> > \subsection{Device configuration layout}\label{sec:Device Types / Socket
> > Device / Device configuration layout}
> >
> > @@ -76,6 +89,7 @@ \subsection{Device Operation}\label{sec:Device Types /
> > Socket Device / Device Op
> > le32 flags;
> > le32 buf_alloc;
> > le32 fwd_cnt;
> > + le64 timestamp;
>
> Adding this field breaks old devices and drivers. Please make this field
> conditional on the dgram socket type or the VIRTIO_VSOCK_F_DGRAM feature
> bit.
>
> Also, are all the other fields still used with dgram sockets? Maybe you
> can use a union instead to reuse some space?
I will make this new field depending on the dgram socket type.
For the union idea, could I change the order of those fields? Dgram does not use
flags and fwd_cnt fields, and I want to put them together with a union
of a le64 timestamp.
Something like:
struct virtio_vsock_hdr {
le64 src_cid;
le64 dst_cid;
le32 src_port;
le32 dst_port;
le32 len;
le16 type;
le16 op;
le32 buf_alloc;
union {
struct {
le32 flags;
le32 fwd_cnt;
} stream;
le64 dgram_timestamp;
} internal; // or a better name
};
> > };
> > \end{lstlisting}
> >
> > @@ -121,11 +135,14 @@ \subsubsection{Virtqueue Flow
> > Control}\label{sec:Device Types / Socket Device /
> > packets. With additional resources, it becomes possible to process
> > incoming
> > packets even when outgoing packets cannot be sent.
> >
> > -Eventually even the additional resources will be exhausted and further
> > +For stream type, eventually even the additional resources will be
> > exhausted and further
> > processing is not possible until the other side processes the virtqueue
> > that
> > it has neglected. This stop to processing prevents one side from causing
> > unbounded resource consumption in the other side.
> >
> > +For datagram type, the additional resources have a fixed size limit to
> > prevent
> > +unbounded resource consumption.
> > +
> > \drivernormative{\paragraph}{Device Operation: Virtqueue Flow
> > Control}{Device Types / Socket Device / Device Operation / Virtqueue Flow
> > Control}
> >
> > The rx virtqueue MUST be processed even when the tx virtqueue is full so
> > long as there are additional resources available to hold packets outside
> > the tx virtqueue.
> > @@ -140,12 +157,15 @@ \subsubsection{Addressing}\label{sec:Device Types /
> > Socket Device / Device Opera
> > consists of a (cid, port number) tuple. The header fields used for this are
> > \field{src_cid}, \field{src_port}, \field{dst_cid}, and \field{dst_port}.
> >
> > -Currently only stream sockets are supported. \field{type} is 1 for stream
> > -socket types.
> > +Currently stream and datagram (dgram) sockets are supported. \field{type}
> > is 1 for stream
> > +socket types. \field{type} is 3 for dgram socket types.
> >
> > Stream sockets provide in-order, guaranteed, connection-oriented delivery
> > without message boundaries.
> >
> > +Datagram socekts provide connectionless unreliable messages of
>
> s/socekts/sockets/
>
> > +a fixed maximum
Re: [PATCH 1/3] ACPICA: iASL: Add definitions for the VIOT table
Hi Jean,
On 3/16/21 8:16 PM, Jean-Philippe Brucker wrote:
> Just here for reference, don't merge!
>
> The actual commits will be pulled from the next ACPICA release.
> I squashed the three relevant commits:
>
> ACPICA commit fc4e33319c1ee08f20f5c44853dd8426643f6dfd
> ACPICA commit 2197e354fb5dcafaddd2016ffeb0620e5bc3d5e2
> ACPICA commit 856a96fdf4b51b2b8da17529df0255e6f51f1b5b
>
> Link: https://github.com/acpica/acpica/commit/fc4e3331
> Link: https://github.com/acpica/acpica/commit/2197e354
> Link: https://github.com/acpica/acpica/commit/856a96fd
> Signed-off-by: Bob Moore
> Signed-off-by: Jean-Philippe Brucker
> ---
> include/acpi/actbl3.h | 67 +++
> 1 file changed, 67 insertions(+)
>
> diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h
> index df5f4b27f3aa..09d15898e9a8 100644
> --- a/include/acpi/actbl3.h
> +++ b/include/acpi/actbl3.h
> @@ -33,6 +33,7 @@
> #define ACPI_SIG_TCPA "TCPA" /* Trusted Computing Platform
> Alliance table */
> #define ACPI_SIG_TPM2 "TPM2" /* Trusted Platform Module 2.0
> H/W interface table */
> #define ACPI_SIG_UEFI "UEFI" /* Uefi Boot Optimization Table
> */
> +#define ACPI_SIG_VIOT "VIOT" /* Virtual I/O Translation
> Table */
> #define ACPI_SIG_WAET "WAET" /* Windows ACPI Emulated
> devices Table */
> #define ACPI_SIG_WDAT "WDAT" /* Watchdog Action Table */
> #define ACPI_SIG_WDDT "WDDT" /* Watchdog Timer Description
> Table */
> @@ -483,6 +484,72 @@ struct acpi_table_uefi {
> u16 data_offset;/* Offset of remaining data in table */
> };
>
> +/***
> + *
> + * VIOT - Virtual I/O Translation Table
> + *Version 1
For other tables I see
Conforms to ../.. Shouldn't we have such section too
> + *
> +
> **/
> +
> +struct acpi_table_viot {
> + struct acpi_table_header header;/* Common ACPI table header */
> + u16 node_count;
> + u16 node_offset;
> + u8 reserved[8];
> +};
> +
> +/* VIOT subtable header */
> +
> +struct acpi_viot_header {
> + u8 type;
> + u8 reserved;
> + u16 length;
> +};
> +
> +/* Values for Type field above */
> +
> +enum acpi_viot_node_type {
> + ACPI_VIOT_NODE_PCI_RANGE = 0x01,
> + ACPI_VIOT_NODE_MMIO = 0x02,
> + ACPI_VIOT_NODE_VIRTIO_IOMMU_PCI = 0x03,
> + ACPI_VIOT_NODE_VIRTIO_IOMMU_MMIO = 0x04,
> + ACPI_VIOT_RESERVED = 0x05
> +};
> +
> +/* VIOT subtables */
> +
> +struct acpi_viot_pci_range {
> + struct acpi_viot_header header;
> + u32 endpoint_start;
> + u16 segment_start;
> + u16 segment_end;
> + u16 bdf_start;
> + u16 bdf_end;
> + u16 output_node;
> + u8 reserved[6];
> +};
> +
> +struct acpi_viot_mmio {
> + struct acpi_viot_header header;
> + u32 endpoint;
> + u64 base_address;
> + u16 output_node;
> + u8 reserved[6];
> +};
> +
> +struct acpi_viot_virtio_iommu_pci {
> + struct acpi_viot_header header;
> + u16 segment;
> + u16 bdf;
> + u8 reserved[8];
> +};
> +
> +struct acpi_viot_virtio_iommu_mmio {
> + struct acpi_viot_header header;
> + u8 reserved[4];
> + u64 base_address;
> +};
> +
>
> /***
> *
> * WAET - Windows ACPI Emulated devices Table
>
Besides
Reviewed-by: Eric Auger
Thanks
Eric
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v8] i2c: virtio: add a virtio i2c frontend driver
On Thu, Mar 18, 2021 at 3:42 PM Enrico Weigelt, metux IT consult wrote: > > On 16.03.21 08:44, Viresh Kumar wrote: > > > FWIW, this limits this driver to support a single device ever. We > > can't bind multiple devices to this driver now. Yeah, perhaps we will > > never be required to do so, but who knows. > > Actually, I believe multiple devices really should be possible. > > The major benefit of virtio-i2c is either bridging certan real bus'es > into a confined workload, or creating virtual hw testbeds w/o having to > write a complete emulation (in this case, for dozens of different i2c > controllers) - and having multiple i2c interfaces in one machine isn't > exactly rare. Allowing multiple virtio-i2c controllers in one system, and multiple i2c devices attached to each controller is clearly something that has to work. I don't actually see a limitation though. Viresh, what is the problem you see for having multiple controllers? Arnd ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: Question about sg_count_fuse_req() in linux/fs/fuse/virtio_fs.c
On Wed, Mar 17, 2021 at 01:12:01PM -0500, Connor Kuehl wrote:
> Hi,
>
> I've been familiarizing myself with the virtiofs guest kernel module and I'm
> trying to better understand how virtiofs maps a FUSE request into
> scattergather lists.
>
> sg_count_fuse_req() starts knowing that there will be at least one in
> header, as shown here (which makes sense):
>
> unsigned int size, total_sgs = 1 /* fuse_in_header */;
>
> However, I'm confused about this snippet right beneath it:
>
> if (args->in_numargs - args->in_pages)
> total_sgs += 1;
>
> What is the significance of the sg that is needed in the cases where this
> branch is taken? I'm not sure what its relationship is with args->in_numargs
> since it will increment total_sgs regardless args->in_numargs is 3, 2, or
> even 1 if args->in_pages is false.
Hi Conor,
I think all the in args are being mapped into a single scatter gather
element and that's why it does not matter whether in_numargs is 3, 2 or 1.
They will be mapped in a single element.
sg_init_fuse_args()
{
len = fuse_len_args(numargs - argpages, args);
if (len)
sg_init_one([total_sgs++], argbuf, len);
}
out_sgs += sg_init_fuse_args([out_sgs], req,
(struct fuse_arg *)args->in_args,
args->in_numargs, args->in_pages,
req->argbuf, _used);
When we are sending some data in some pages, then we set args->in_pages
to true. And in that case, last element of args->in_args[] contains the
total size of bytes in additional pages we are sending and is not part
of in_args being mapped to scatter gather element. That's why this
check.
if (args->in_numargs - args->in_pages)
total_sgs += 1;
Not sure when we will have a case where args->in_numargs = 1 and
args->in_pages=true. Do we ever hit that.
Thanks
Vivek
>
> Especially since the block right below it counts pages if args->in_pages is
> true:
>
> if (args->in_pages) {
> size = args->in_args[args->in_numargs - 1].size;
> total_sgs += sg_count_fuse_pages(ap->descs, ap->num_pages,
> size);
> }
>
> The rest of the routine goes on similarly but for the 'out' components.
>
> I doubt incrementing 'total_sgs' in the first if-statement I showed above is
> vestigial, I just think my mental model of what is happening here is
> incomplete.
>
> Any clarification is much appreciated!
>
> Connor
>
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 3/3] iommu/virtio: Enable x86 support
On 2021-03-16 19:16, Jean-Philippe Brucker wrote: With the VIOT support in place, x86 platforms can now use the virtio-iommu. The arm64 Kconfig selects IOMMU_DMA, while x86 IOMMU drivers select it themselves. Actually, now that both AMD and Intel are converted over, maybe it's finally time to punt that to x86 arch code to match arm64? Robin. Signed-off-by: Jean-Philippe Brucker --- drivers/iommu/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig index 2819b5c8ec30..ccca83ef2f06 100644 --- a/drivers/iommu/Kconfig +++ b/drivers/iommu/Kconfig @@ -400,8 +400,9 @@ config HYPERV_IOMMU config VIRTIO_IOMMU tristate "Virtio IOMMU driver" depends on VIRTIO - depends on ARM64 + depends on (ARM64 || X86) select IOMMU_API + select IOMMU_DMA if X86 select INTERVAL_TREE select ACPI_VIOT if ACPI help ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 08/10] drm/simpledrm: Acquire clocks from DT device node
Hi
Am 18.03.21 um 11:39 schrieb Geert Uytterhoeven:
Hi Thomas,
On Thu, Mar 18, 2021 at 11:29 AM Thomas Zimmermann wrote:
Make sure required hardware clocks are enabled while the firmware
framebuffer is in use.
The basic code has been taken from the simplefb driver and adapted
to DRM. Clocks are released automatically via devres helpers.
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
Thanks for your patch!
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
+static int simpledrm_device_init_clocks(struct simpledrm_device *sdev)
+{
+ struct drm_device *dev = >dev;
+ struct platform_device *pdev = sdev->pdev;
+ struct device_node *of_node = pdev->dev.of_node;
+ struct clk *clock;
+ unsigned int i;
+ int ret;
+
+ if (dev_get_platdata(>dev) || !of_node)
+ return 0;
+
+ sdev->clk_count = of_clk_get_parent_count(of_node);
+ if (!sdev->clk_count)
+ return 0;
+
+ sdev->clks = drmm_kzalloc(dev, sdev->clk_count * sizeof(sdev->clks[0]),
+ GFP_KERNEL);
+ if (!sdev->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < sdev->clk_count; ++i) {
+ clock = of_clk_get(of_node, i);
+ if (IS_ERR(clock)) {
+ ret = PTR_ERR(clock);
+ if (ret == -EPROBE_DEFER)
+ goto err;
+ drm_err(dev, "clock %u not found: %d\n", i, ret);
+ continue;
+ }
+ ret = clk_prepare_enable(clock);
+ if (ret) {
+ drm_err(dev, "failed to enable clock %u: %d\n",
+ i, ret);
+ clk_put(clock);
+ }
+ sdev->clks[i] = clock;
+ }
of_clk_bulk_get_all() + clk_bulk_prepare_enable()?
There's also devm_clk_bulk_get_all(), but not for the OF variant.
Right, you mentioned this on the original patch set. I tried to use the
functions, but TBH I found them to obfuscate the overall logic of the
function. So I went back to the original code. Hopefully this is not too
much of an issue.
Best regards
Thomas
Gr{oetje,eeting}s,
Geert
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
OpenPGP_signature
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH 3/3] iommu/virtio: Enable x86 support
On Tue, Mar 16, 2021 at 08:16:54PM +0100, Jean-Philippe Brucker wrote: > With the VIOT support in place, x86 platforms can now use the > virtio-iommu. > > The arm64 Kconfig selects IOMMU_DMA, while x86 IOMMU drivers select it > themselves. > > Signed-off-by: Jean-Philippe Brucker > --- > drivers/iommu/Kconfig | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig > index 2819b5c8ec30..ccca83ef2f06 100644 > --- a/drivers/iommu/Kconfig > +++ b/drivers/iommu/Kconfig > @@ -400,8 +400,9 @@ config HYPERV_IOMMU > config VIRTIO_IOMMU > tristate "Virtio IOMMU driver" > depends on VIRTIO > - depends on ARM64 > + depends on (ARM64 || X86) > select IOMMU_API > + select IOMMU_DMA if X86 > select INTERVAL_TREE > select ACPI_VIOT if ACPI > help Acked-by: Joerg Roedel ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v2 08/10] drm/simpledrm: Acquire clocks from DT device node
Hi Thomas,
On Thu, Mar 18, 2021 at 11:29 AM Thomas Zimmermann wrote:
> Make sure required hardware clocks are enabled while the firmware
> framebuffer is in use.
>
> The basic code has been taken from the simplefb driver and adapted
> to DRM. Clocks are released automatically via devres helpers.
>
> Signed-off-by: Thomas Zimmermann
> Tested-by: nerdopolis
Thanks for your patch!
> --- a/drivers/gpu/drm/tiny/simpledrm.c
> +++ b/drivers/gpu/drm/tiny/simpledrm.c
> +static int simpledrm_device_init_clocks(struct simpledrm_device *sdev)
> +{
> + struct drm_device *dev = >dev;
> + struct platform_device *pdev = sdev->pdev;
> + struct device_node *of_node = pdev->dev.of_node;
> + struct clk *clock;
> + unsigned int i;
> + int ret;
> +
> + if (dev_get_platdata(>dev) || !of_node)
> + return 0;
> +
> + sdev->clk_count = of_clk_get_parent_count(of_node);
> + if (!sdev->clk_count)
> + return 0;
> +
> + sdev->clks = drmm_kzalloc(dev, sdev->clk_count *
> sizeof(sdev->clks[0]),
> + GFP_KERNEL);
> + if (!sdev->clks)
> + return -ENOMEM;
> +
> + for (i = 0; i < sdev->clk_count; ++i) {
> + clock = of_clk_get(of_node, i);
> + if (IS_ERR(clock)) {
> + ret = PTR_ERR(clock);
> + if (ret == -EPROBE_DEFER)
> + goto err;
> + drm_err(dev, "clock %u not found: %d\n", i, ret);
> + continue;
> + }
> + ret = clk_prepare_enable(clock);
> + if (ret) {
> + drm_err(dev, "failed to enable clock %u: %d\n",
> + i, ret);
> + clk_put(clock);
> + }
> + sdev->clks[i] = clock;
> + }
of_clk_bulk_get_all() + clk_bulk_prepare_enable()?
There's also devm_clk_bulk_get_all(), but not for the OF variant.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 08/10] drm/simpledrm: Acquire clocks from DT device node
Make sure required hardware clocks are enabled while the firmware
framebuffer is in use.
The basic code has been taken from the simplefb driver and adapted
to DRM. Clocks are released automatically via devres helpers.
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
drivers/gpu/drm/tiny/simpledrm.c | 108 +++
1 file changed, 108 insertions(+)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index c9cef2b50de6..10ca3373b61f 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -1,5 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
+#include
+#include
#include
#include
@@ -190,6 +192,12 @@ struct simpledrm_device {
struct drm_device dev;
struct platform_device *pdev;
+ /* clocks */
+#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
+ unsigned int clk_count;
+ struct clk **clks;
+#endif
+
/* simplefb settings */
struct drm_display_mode mode;
const struct drm_format_info *format;
@@ -211,6 +219,103 @@ static struct simpledrm_device
*simpledrm_device_of_dev(struct drm_device *dev)
return container_of(dev, struct simpledrm_device, dev);
}
+/*
+ * Hardware
+ */
+
+#if defined CONFIG_OF && defined CONFIG_COMMON_CLK
+/*
+ * Clock handling code.
+ *
+ * Here we handle the clocks property of our "simple-framebuffer" dt node.
+ * This is necessary so that we can make sure that any clocks needed by
+ * the display engine that the bootloader set up for us (and for which it
+ * provided a simplefb dt node), stay up, for the life of the simplefb
+ * driver.
+ *
+ * When the driver unloads, we cleanly disable, and then release the clocks.
+ *
+ * We only complain about errors here, no action is taken as the most likely
+ * error can only happen due to a mismatch between the bootloader which set
+ * up simplefb, and the clock definitions in the device tree. Chances are
+ * that there are no adverse effects, and if there are, a clean teardown of
+ * the fb probe will not help us much either. So just complain and carry on,
+ * and hope that the user actually gets a working fb at the end of things.
+ */
+
+static void simpledrm_device_release_clocks(void *res)
+{
+ struct simpledrm_device *sdev = simpledrm_device_of_dev(res);
+ unsigned int i;
+
+ for (i = 0; i < sdev->clk_count; ++i) {
+ if (sdev->clks[i]) {
+ clk_disable_unprepare(sdev->clks[i]);
+ clk_put(sdev->clks[i]);
+ }
+ }
+}
+
+static int simpledrm_device_init_clocks(struct simpledrm_device *sdev)
+{
+ struct drm_device *dev = >dev;
+ struct platform_device *pdev = sdev->pdev;
+ struct device_node *of_node = pdev->dev.of_node;
+ struct clk *clock;
+ unsigned int i;
+ int ret;
+
+ if (dev_get_platdata(>dev) || !of_node)
+ return 0;
+
+ sdev->clk_count = of_clk_get_parent_count(of_node);
+ if (!sdev->clk_count)
+ return 0;
+
+ sdev->clks = drmm_kzalloc(dev, sdev->clk_count * sizeof(sdev->clks[0]),
+ GFP_KERNEL);
+ if (!sdev->clks)
+ return -ENOMEM;
+
+ for (i = 0; i < sdev->clk_count; ++i) {
+ clock = of_clk_get(of_node, i);
+ if (IS_ERR(clock)) {
+ ret = PTR_ERR(clock);
+ if (ret == -EPROBE_DEFER)
+ goto err;
+ drm_err(dev, "clock %u not found: %d\n", i, ret);
+ continue;
+ }
+ ret = clk_prepare_enable(clock);
+ if (ret) {
+ drm_err(dev, "failed to enable clock %u: %d\n",
+ i, ret);
+ clk_put(clock);
+ }
+ sdev->clks[i] = clock;
+ }
+
+ return devm_add_action_or_reset(>dev,
+ simpledrm_device_release_clocks,
+ sdev);
+
+err:
+ while (i) {
+ --i;
+ if (sdev->clks[i]) {
+ clk_disable_unprepare(sdev->clks[i]);
+ clk_put(sdev->clks[i]);
+ }
+ }
+ return ret;
+}
+#else
+static int simpledrm_device_init_clocks(struct simpledrm_device *sdev)
+{
+ return 0;
+}
+#endif
+
/*
* Simplefb settings
*/
@@ -534,6 +639,9 @@ simpledrm_device_create(struct drm_driver *drv, struct
platform_device *pdev)
return ERR_CAST(sdev);
sdev->pdev = pdev;
+ ret = simpledrm_device_init_clocks(sdev);
+ if (ret)
+ return ERR_PTR(ret);
ret = simpledrm_device_init_fb(sdev);
if (ret)
return ERR_PTR(ret);
--
2.30.1
___
Virtualization mailing list
[PATCH v2 05/10] drm: Add simpledrm driver
The simpledrm driver is a DRM driver for simplefb framebuffers as
provided by the kernel's boot code. This driver enables basic
graphical output on many different graphics devices that are provided
by the platform (e.g., EFI, VESA, embedded framebuffers).
With the kernel's simplefb infrastructure, the kernel receives a
pre-configured framebuffer from the system (i.e., firmware, boot
loader). It creates a platform device to which simpledrm attaches.
The system's framebuffer consists of a memory range, size and format.
Based on these values, simpledrm creates a DRM devices. No actual
modesetting is possible.
v2:
* rename driver to simpledrm
* add dri-devel to MAINTAINERS entry
* put native format first in primary-plane format list (Daniel)
* inline simplekms_device_cleanup() (Daniel)
* use helpers for shadow-buffered planes
* fix whitespace errors
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
MAINTAINERS | 7 +
drivers/gpu/drm/tiny/Kconfig | 16 +
drivers/gpu/drm/tiny/Makefile| 1 +
drivers/gpu/drm/tiny/simpledrm.c | 527 +++
4 files changed, 551 insertions(+)
create mode 100644 drivers/gpu/drm/tiny/simpledrm.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 3dc7b57be31d..e9d53daf8b58 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5744,6 +5744,13 @@ S: Orphan / Obsolete
F: drivers/gpu/drm/savage/
F: include/uapi/drm/savage_drm.h
+DRM DRIVER FOR SIMPLE FRAMEBUFFERS
+M: Thomas Zimmermann
+L: dri-de...@lists.freedesktop.org
+S: Maintained
+T: git git://anongit.freedesktop.org/drm/drm-misc
+F: drivers/gpu/drm/tiny/simplekms.c
+
DRM DRIVER FOR SIS VIDEO CARDS
S: Orphan / Obsolete
F: drivers/gpu/drm/sis/
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index 9bbaa1a69050..d46f95d9196d 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -38,6 +38,22 @@ config DRM_GM12U320
This is a KMS driver for projectors which use the GM12U320 chipset
for video transfer over USB2/3, such as the Acer C120 mini projector.
+config DRM_SIMPLEDRM
+ tristate "Simple framebuffer driver"
+ depends on DRM
+ select DRM_GEM_SHMEM_HELPER
+ select DRM_KMS_HELPER
+ help
+ DRM driver for simple platform-provided framebuffers.
+
+ This driver assumes that the display hardware has been initialized
+ by the firmware or bootloader before the kernel boots. Scanout
+ buffer, size, and display format must be provided via device tree,
+ UEFI, VESA, etc.
+
+ On x86 and compatible, you should also select CONFIG_X86_SYSFB to
+ use UEFI and VESA framebuffers.
+
config TINYDRM_HX8357D
tristate "DRM support for HX8357D display panels"
depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index bef6780bdd6f..9cc847e756da 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -3,6 +3,7 @@
obj-$(CONFIG_DRM_ARCPGU) += arcpgu.o
obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o
obj-$(CONFIG_DRM_GM12U320) += gm12u320.o
+obj-$(CONFIG_DRM_SIMPLEDRM)+= simpledrm.o
obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o
obj-$(CONFIG_TINYDRM_ILI9225) += ili9225.o
obj-$(CONFIG_TINYDRM_ILI9341) += ili9341.o
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
new file mode 100644
index ..0422c549b97a
--- /dev/null
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -0,0 +1,527 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRIVER_NAME"simpledrm"
+#define DRIVER_DESC"DRM driver for simple-framebuffer platform devices"
+#define DRIVER_DATE"20200625"
+#define DRIVER_MAJOR 1
+#define DRIVER_MINOR 0
+
+/*
+ * Assume a monitor resolution of 96 dpi to
+ * get a somewhat reasonable screen size.
+ */
+#define RES_MM(d) \
+ (((d) * 254ul) / (96ul * 10ul))
+
+#define SIMPLEDRM_MODE(hd, vd) \
+ DRM_SIMPLE_MODE(hd, vd, RES_MM(hd), RES_MM(vd))
+
+/*
+ * Helpers for simplefb
+ */
+
+static int
+simplefb_get_validated_int(struct drm_device *dev, const char *name,
+ uint32_t value)
+{
+ if (value > INT_MAX) {
+ drm_err(dev, "simplefb: invalid framebuffer %s of %u\n",
+ name, value);
+ return -EINVAL;
+ }
+ return (int)value;
+}
+
+static int
+simplefb_get_validated_int0(struct drm_device *dev, const char *name,
+ uint32_t value)
+{
+ if (!value) {
+ drm_err(dev, "simplefb: invalid framebuffer %s of %u\n",
+ name,
[PATCH v2 10/10] drm/simpledrm: Acquire memory aperture for framebuffer
We register the simplekms device with the DRM platform helpers. A
native driver for the graphics hardware will kick-out the simpledrm
driver before taking over the device.
v2:
* adapt to aperture changes
* use drm_dev_unplug() and drm_dev_enter/exit()
* don't split error string
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
drivers/gpu/drm/tiny/Kconfig | 1 +
drivers/gpu/drm/tiny/simpledrm.c | 83 ++--
2 files changed, 81 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig
index d46f95d9196d..5b72dd8e93f9 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -41,6 +41,7 @@ config DRM_GM12U320
config DRM_SIMPLEDRM
tristate "Simple framebuffer driver"
depends on DRM
+ select DRM_APERTURE
select DRM_GEM_SHMEM_HELPER
select DRM_KMS_HELPER
help
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 2e27eeb791a1..67d33af19086 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -5,7 +5,9 @@
#include
#include
#include
+#include
+#include
#include
#include
#include
@@ -37,6 +39,12 @@
#define SIMPLEDRM_MODE(hd, vd) \
DRM_SIMPLE_MODE(hd, vd, RES_MM(hd), RES_MM(vd))
+/*
+ * Protects the platform device's drvdata against
+ * concurrent manipulation.
+ */
+static DEFINE_SPINLOCK(simpledrm_drvdata_lock);
+
/*
* Helpers for simplefb
*/
@@ -515,16 +523,53 @@ static int simpledrm_device_init_fb(struct
simpledrm_device *sdev)
* Memory management
*/
+static void simpledrm_aperture_detach(struct drm_device *dev, resource_size_t
base,
+ resource_size_t size)
+{
+ struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
+ struct platform_device *pdev = sdev->pdev;
+
+ if (WARN_ON(drm_dev_is_unplugged(dev)))
+ return; /* BUG: driver already got detached */
+
+ /*
+* If simpledrm gets detached from the aperture, it's like unplugging
+* the device. So call drm_dev_unplug().
+*/
+ drm_dev_unplug(dev);
+
+ spin_lock(_drvdata_lock);
+ sdev = platform_get_drvdata(pdev);
+ platform_set_drvdata(pdev, NULL); /* required; see simpledrm_remove() */
+ spin_unlock(_drvdata_lock);
+}
+
+static const struct drm_aperture_funcs simpledrm_aperture_funcs = {
+ .detach = simpledrm_aperture_detach,
+};
+
static int simpledrm_device_init_mm(struct simpledrm_device *sdev)
{
+ struct drm_device *dev = >dev;
struct platform_device *pdev = sdev->pdev;
struct resource *mem;
+ struct drm_aperture *ap;
void __iomem *screen_base;
+ int ret;
mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!mem)
return -EINVAL;
+ ap = devm_aperture_acquire(dev, mem->start, resource_size(mem),
+ _aperture_funcs);
+ if (IS_ERR(ap)) {
+ ret = PTR_ERR(ap);
+ drm_err(dev, "could not acquire memory range [0x%llx:0x%llx]:
error %d\n",
+ mem->start, mem->end, ret);
+ return ret;
+ }
+
screen_base = devm_ioremap_wc(>dev, mem->start,
resource_size(mem));
if (!screen_base)
@@ -625,12 +670,18 @@ simpledrm_simple_display_pipe_enable(struct
drm_simple_display_pipe *pipe,
struct drm_shadow_plane_state *shadow_plane_state =
to_drm_shadow_plane_state(plane_state);
struct drm_framebuffer *fb = plane_state->fb;
void *vmap = shadow_plane_state->map[0].vaddr; /* TODO: Use mapping
abstraction properly */
+ struct drm_device *dev = >dev;
+ int idx;
if (!fb)
return;
+ if (!drm_dev_enter(dev, ))
+ return;
+
drm_fb_blit_dstclip(sdev->screen_base, sdev->pitch,
sdev->format->format, vmap, fb);
+ drm_dev_exit(idx);
}
static void
@@ -642,7 +693,9 @@ simpledrm_simple_display_pipe_update(struct
drm_simple_display_pipe *pipe,
struct drm_shadow_plane_state *shadow_plane_state =
to_drm_shadow_plane_state(plane_state);
void *vmap = shadow_plane_state->map[0].vaddr; /* TODO: Use mapping
abstraction properly */
struct drm_framebuffer *fb = plane_state->fb;
+ struct drm_device *dev = >dev;
struct drm_rect clip;
+ int idx;
if (!fb)
return;
@@ -650,8 +703,13 @@ simpledrm_simple_display_pipe_update(struct
drm_simple_display_pipe *pipe,
if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state,
))
return;
+ if (!drm_dev_enter(dev, ))
+ return;
+
drm_fb_blit_rect_dstclip(sdev->screen_base, sdev->pitch,
sdev->format->format, vmap,
[PATCH v2 07/10] drm/simpledrm: Initialize framebuffer data from device-tree node
A firmware framebuffer might also be specified via device-tree files. If
no device platform data is given, try the DT device node.
v2:
* add Device Tree match table
* clean-up parser wrappers
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
drivers/gpu/drm/tiny/simpledrm.c | 89
1 file changed, 89 insertions(+)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 4f0d4ec0b432..c9cef2b50de6 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -114,6 +114,74 @@ simplefb_get_format_pd(struct drm_device *dev,
return simplefb_get_validated_format(dev, pd->format);
}
+static int
+simplefb_read_u32_of(struct drm_device *dev, struct device_node *of_node,
+const char *name, u32 *value)
+{
+ int ret = of_property_read_u32(of_node, name, value);
+
+ if (ret)
+ drm_err(dev, "simplefb: cannot parse framebuffer %s: error
%d\n",
+ name, ret);
+ return ret;
+}
+
+static int
+simplefb_read_string_of(struct drm_device *dev, struct device_node *of_node,
+ const char *name, const char **value)
+{
+ int ret = of_property_read_string(of_node, name, value);
+
+ if (ret)
+ drm_err(dev, "simplefb: cannot parse framebuffer %s: error
%d\n",
+ name, ret);
+ return ret;
+}
+
+static int
+simplefb_get_width_of(struct drm_device *dev, struct device_node *of_node)
+{
+ u32 width;
+ int ret = simplefb_read_u32_of(dev, of_node, "width", );
+
+ if (ret)
+ return ret;
+ return simplefb_get_validated_int0(dev, "width", width);
+}
+
+static int
+simplefb_get_height_of(struct drm_device *dev, struct device_node *of_node)
+{
+ u32 height;
+ int ret = simplefb_read_u32_of(dev, of_node, "height", );
+
+ if (ret)
+ return ret;
+ return simplefb_get_validated_int0(dev, "height", height);
+}
+
+static int
+simplefb_get_stride_of(struct drm_device *dev, struct device_node *of_node)
+{
+ u32 stride;
+ int ret = simplefb_read_u32_of(dev, of_node, "stride", );
+
+ if (ret)
+ return ret;
+ return simplefb_get_validated_int(dev, "stride", stride);
+}
+
+static const struct drm_format_info *
+simplefb_get_format_of(struct drm_device *dev, struct device_node *of_node)
+{
+ const char *format;
+ int ret = simplefb_read_string_of(dev, of_node, "format", );
+
+ if (ret)
+ return ERR_PTR(ret);
+ return simplefb_get_validated_format(dev, format);
+}
+
/*
* Simple Framebuffer device
*/
@@ -166,6 +234,7 @@ static int simpledrm_device_init_fb(struct simpledrm_device
*sdev)
struct drm_device *dev = >dev;
struct platform_device *pdev = sdev->pdev;
const struct simplefb_platform_data *pd = dev_get_platdata(>dev);
+ struct device_node *of_node = pdev->dev.of_node;
if (pd) {
width = simplefb_get_width_pd(dev, pd);
@@ -180,6 +249,19 @@ static int simpledrm_device_init_fb(struct
simpledrm_device *sdev)
format = simplefb_get_format_pd(dev, pd);
if (IS_ERR(format))
return PTR_ERR(format);
+ } else if (of_node) {
+ width = simplefb_get_width_of(dev, of_node);
+ if (width < 0)
+ return width;
+ height = simplefb_get_height_of(dev, of_node);
+ if (height < 0)
+ return height;
+ stride = simplefb_get_stride_of(dev, of_node);
+ if (stride < 0)
+ return stride;
+ format = simplefb_get_format_of(dev, of_node);
+ if (IS_ERR(format))
+ return PTR_ERR(format);
} else {
drm_err(dev, "no simplefb configuration found\n");
return -ENODEV;
@@ -516,9 +598,16 @@ static int simpledrm_remove(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id simpledrm_of_match_table[] = {
+ { .compatible = "simple-framebuffer", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, simpledrm_of_match_table);
+
static struct platform_driver simpledrm_platform_driver = {
.driver = {
.name = "simple-framebuffer", /* connect to sysfb */
+ .of_match_table = simpledrm_of_match_table,
},
.probe = simpledrm_probe,
.remove = simpledrm_remove,
--
2.30.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 09/10] drm/simpledrm: Acquire regulators from DT device node
Make sure required hardware regulators are enabled while the firmware
framebuffer is in use.
The basic code has been taken from the simplefb driver and adapted
to DRM. Regulators are released automatically via devres helpers.
v2:
* use strscpy()
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
drivers/gpu/drm/tiny/simpledrm.c | 128 +++
1 file changed, 128 insertions(+)
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c
index 10ca3373b61f..2e27eeb791a1 100644
--- a/drivers/gpu/drm/tiny/simpledrm.c
+++ b/drivers/gpu/drm/tiny/simpledrm.c
@@ -4,6 +4,7 @@
#include
#include
#include
+#include
#include
#include
@@ -197,6 +198,11 @@ struct simpledrm_device {
unsigned int clk_count;
struct clk **clks;
#endif
+ /* regulators */
+#if defined CONFIG_OF && defined CONFIG_REGULATOR
+ unsigned int regulator_count;
+ struct regulator **regulators;
+#endif
/* simplefb settings */
struct drm_display_mode mode;
@@ -316,6 +322,125 @@ static int simpledrm_device_init_clocks(struct
simpledrm_device *sdev)
}
#endif
+#if defined CONFIG_OF && defined CONFIG_REGULATOR
+
+#define SUPPLY_SUFFIX "-supply"
+
+/*
+ * Regulator handling code.
+ *
+ * Here we handle the num-supplies and vin*-supply properties of our
+ * "simple-framebuffer" dt node. This is necessary so that we can make sure
+ * that any regulators needed by the display hardware that the bootloader
+ * set up for us (and for which it provided a simplefb dt node), stay up,
+ * for the life of the simplefb driver.
+ *
+ * When the driver unloads, we cleanly disable, and then release the
+ * regulators.
+ *
+ * We only complain about errors here, no action is taken as the most likely
+ * error can only happen due to a mismatch between the bootloader which set
+ * up simplefb, and the regulator definitions in the device tree. Chances are
+ * that there are no adverse effects, and if there are, a clean teardown of
+ * the fb probe will not help us much either. So just complain and carry on,
+ * and hope that the user actually gets a working fb at the end of things.
+ */
+
+static void simpledrm_device_release_regulators(void *res)
+{
+ struct simpledrm_device *sdev = simpledrm_device_of_dev(res);
+ unsigned int i;
+
+ for (i = 0; i < sdev->regulator_count; ++i) {
+ if (sdev->regulators[i]) {
+ regulator_disable(sdev->regulators[i]);
+ regulator_put(sdev->regulators[i]);
+ }
+ }
+}
+
+static int simpledrm_device_init_regulators(struct simpledrm_device *sdev)
+{
+ struct drm_device *dev = >dev;
+ struct platform_device *pdev = sdev->pdev;
+ struct device_node *of_node = pdev->dev.of_node;
+ struct property *prop;
+ struct regulator *regulator;
+ const char *p;
+ unsigned int count = 0, i = 0;
+ int ret;
+
+ if (dev_get_platdata(>dev) || !of_node)
+ return 0;
+
+ /* Count the number of regulator supplies */
+ for_each_property_of_node(of_node, prop) {
+ p = strstr(prop->name, SUPPLY_SUFFIX);
+ if (p && p != prop->name)
+ ++count;
+ }
+
+ if (!count)
+ return 0;
+
+ sdev->regulators = drmm_kzalloc(dev,
+ count * sizeof(sdev->regulators[0]),
+ GFP_KERNEL);
+ if (!sdev->regulators)
+ return -ENOMEM;
+
+ for_each_property_of_node(of_node, prop) {
+ char name[32]; /* 32 is max size of property name */
+ size_t len;
+
+ p = strstr(prop->name, SUPPLY_SUFFIX);
+ if (!p || p == prop->name)
+ continue;
+ len = strlen(prop->name) - strlen(SUPPLY_SUFFIX) + 1;
+ strscpy(name, prop->name, min(sizeof(name), len));
+
+ regulator = regulator_get_optional(>dev, name);
+ if (IS_ERR(regulator)) {
+ ret = PTR_ERR(regulator);
+ if (ret == -EPROBE_DEFER)
+ goto err;
+ drm_err(dev, "regulator %s not found: %d\n",
+ name, ret);
+ continue;
+ }
+
+ ret = regulator_enable(regulator);
+ if (ret) {
+ drm_err(dev, "failed to enable regulator %u: %d\n",
+ i, ret);
+ regulator_put(regulator);
+ }
+
+ sdev->regulators[i++] = regulator;
+ }
+ sdev->regulator_count = i;
+
+ return devm_add_action_or_reset(>dev,
+ simpledrm_device_release_regulators,
+ sdev);
+
+err:
+ while (i) {
+
[PATCH v2 04/10] drm/aperture: Add infrastructure for aperture ownership
Platform devices might operate on firmware framebuffers, such as VESA or
EFI. Before a native driver for the graphics hardware can take over the
device, it has to remove any platform driver that operates on the firmware
framebuffer. Aperture helpers provide the infrastructure for platform
drivers to acquire firmware framebuffers, and for native drivers to remove
them later on.
It works similar to the related fbdev mechanism. During initialization, the
platform driver acquires the firmware framebuffer's I/O memory and provides
a callback to be removed. The native driver later uses this information to
remove any platform driver for it's framebuffer I/O memory.
The aperture removal code is integrated into the existing code for removing
conflicting framebuffers, so native drivers use it automatically.
v2:
* rename plaform helpers to aperture helpers
* tie to device lifetime with devm_ functions
* removed unsued remove() callback
* rename kickout to detach
* make struct drm_aperture private
* rebase onto existing drm_aperture.h header file
* use MIT license only for simplicity
* documentation
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
Documentation/gpu/drm-internals.rst | 6 +
drivers/gpu/drm/Kconfig | 7 +
drivers/gpu/drm/Makefile| 1 +
drivers/gpu/drm/drm_aperture.c | 287
include/drm/drm_aperture.h | 38 +++-
5 files changed, 338 insertions(+), 1 deletion(-)
create mode 100644 drivers/gpu/drm/drm_aperture.c
diff --git a/Documentation/gpu/drm-internals.rst
b/Documentation/gpu/drm-internals.rst
index 4c7642d2ca34..06af044c882f 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -78,9 +78,15 @@ DRM_IOCTL_VERSION ioctl.
Managing Ownership of the Framebuffer Aperture
--
+.. kernel-doc:: drivers/gpu/drm/drm_aperture.c
+ :doc: overview
+
.. kernel-doc:: include/drm/drm_aperture.h
:internal:
+.. kernel-doc:: drivers/gpu/drm/drm_aperture.c
+ :export:
+
Device Instance and Driver Handling
---
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 1461652921be..b9d3fb91d22d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -221,6 +221,13 @@ config DRM_SCHED
tristate
depends on DRM
+config DRM_APERTURE
+ bool
+ depends on DRM
+ help
+ Controls ownership of graphics apertures. Required to
+ synchronize with firmware-based drivers.
+
source "drivers/gpu/drm/i2c/Kconfig"
source "drivers/gpu/drm/arm/Kconfig"
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 5eb5bf7c16e3..c9ecb02df0f3 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -32,6 +32,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
drm-$(CONFIG_PCI) += drm_pci.o
drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
+drm-$(CONFIG_DRM_APERTURE) += drm_aperture.o
drm_vram_helper-y := drm_gem_vram_helper.o
obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
new file mode 100644
index ..4b02b5fed0a1
--- /dev/null
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -0,0 +1,287 @@
+// SPDX-License-Identifier: MIT
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/**
+ * DOC: overview
+ *
+ * A graphics device might be supported by different drivers, but only one
+ * driver can be active at any given time. Many systems load a generic
+ * graphics drivers, such as EFI-GOP or VESA, early during the boot process.
+ * During later boot stages, they replace the generic driver with a dedicated,
+ * hardware-specific driver. To take over the device the dedicated driver
+ * first has to remove the generic driver. DRM aperture functions manage
+ * ownership of DRM framebuffer memory and hand-over between drivers.
+ *
+ * DRM drivers should call drm_fb_helper_remove_conflicting_framebuffers()
+ * at the top of their probe function. The function removes any generic
+ * driver that is currently associated with the given framebuffer memory.
+ * If the framebuffer is located at PCI BAR 0, the rsp code looks as in the
+ * example given below.
+ *
+ * .. code-block:: c
+ *
+ * static int remove_conflicting_framebuffers(struct pci_dev *pdev)
+ * {
+ * struct apertures_struct *ap;
+ * bool primary = false;
+ * int ret;
+ *
+ * ap = alloc_apertures(1);
+ * if (!ap)
+ * return -ENOMEM;
+ *
+ * ap->ranges[0].base = pci_resource_start(pdev, 0);
+ * ap->ranges[0].size = pci_resource_len(pdev, 0);
+ *
+ * #ifdef CONFIG_X86
+ * primary =
[PATCH v2 06/10] drm/simpledrm: Add fbdev emulation
This displays a console on simpledrm's framebuffer. The default framebuffer format is being used. Signed-off-by: Thomas Zimmermann Reviewed-by: Daniel Vetter Tested-by: nerdopolis --- drivers/gpu/drm/tiny/simpledrm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 0422c549b97a..4f0d4ec0b432 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -500,6 +501,8 @@ static int simpledrm_probe(struct platform_device *pdev) if (ret) return ret; + drm_fbdev_generic_setup(dev, 0); + return 0; } -- 2.30.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 03/10] drm/aperture: Move fbdev conflict helpers into drm_aperture.h
Fbdev's helpers for handling conflicting framebuffers are related to
framebuffer apertures, not console emulation. Therefore move them into a
drm_aperture.h, which will contain the interfaces for the new aperture
helpers.
Signed-off-by: Thomas Zimmermann
Tested-by: nerdopolis
---
Documentation/gpu/drm-internals.rst | 6 +++
include/drm/drm_aperture.h | 60 +
include/drm/drm_fb_helper.h | 56 ++-
3 files changed, 69 insertions(+), 53 deletions(-)
create mode 100644 include/drm/drm_aperture.h
diff --git a/Documentation/gpu/drm-internals.rst
b/Documentation/gpu/drm-internals.rst
index 12272b168580..4c7642d2ca34 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -75,6 +75,12 @@ update it, its value is mostly useless. The DRM core prints
it to the
kernel log at initialization time and passes it to userspace through the
DRM_IOCTL_VERSION ioctl.
+Managing Ownership of the Framebuffer Aperture
+--
+
+.. kernel-doc:: include/drm/drm_aperture.h
+ :internal:
+
Device Instance and Driver Handling
---
diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
new file mode 100644
index ..13766efe9517
--- /dev/null
+++ b/include/drm/drm_aperture.h
@@ -0,0 +1,60 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef _DRM_APERTURE_H_
+#define _DRM_APERTURE_H_
+
+#include
+#include
+
+/**
+ * drm_fb_helper_remove_conflicting_framebuffers - remove firmware-configured
framebuffers
+ * @a: memory range, users of which are to be removed
+ * @name: requesting driver name
+ * @primary: also kick vga16fb if present
+ *
+ * This function removes framebuffer devices (initialized by
firmware/bootloader)
+ * which use memory range described by @a. If @a is NULL all such devices are
+ * removed.
+ */
+static inline int
+drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
+ const char *name, bool primary)
+{
+#if IS_REACHABLE(CONFIG_FB)
+ return remove_conflicting_framebuffers(a, name, primary);
+#else
+ return 0;
+#endif
+}
+
+/**
+ * drm_fb_helper_remove_conflicting_pci_framebuffers - remove
firmware-configured
+ * framebuffers for PCI
devices
+ * @pdev: PCI device
+ * @name: requesting driver name
+ *
+ * This function removes framebuffer devices (eg. initialized by firmware)
+ * using memory range configured for any of @pdev's memory bars.
+ *
+ * The function assumes that PCI device with shadowed ROM drives a primary
+ * display and so kicks out vga16fb.
+ */
+static inline int
+drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
+ const char *name)
+{
+ int ret = 0;
+
+ /*
+* WARNING: Apparently we must kick fbdev drivers before vgacon,
+* otherwise the vga fbdev driver falls over.
+*/
+#if IS_REACHABLE(CONFIG_FB)
+ ret = remove_conflicting_pci_framebuffers(pdev, name);
+#endif
+ if (ret == 0)
+ ret = vga_remove_vgacon(pdev);
+ return ret;
+}
+
+#endif
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 3b273f9ca39a..d06a3942fddb 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -30,13 +30,13 @@
#ifndef DRM_FB_HELPER_H
#define DRM_FB_HELPER_H
-struct drm_fb_helper;
-
+#include
#include
#include
#include
#include
-#include
+
+struct drm_fb_helper;
enum mode_set_atomic {
LEAVE_ATOMIC_MODE_SET,
@@ -451,54 +451,4 @@ drm_fbdev_generic_setup(struct drm_device *dev, unsigned
int preferred_bpp)
#endif
-/**
- * drm_fb_helper_remove_conflicting_framebuffers - remove firmware-configured
framebuffers
- * @a: memory range, users of which are to be removed
- * @name: requesting driver name
- * @primary: also kick vga16fb if present
- *
- * This function removes framebuffer devices (initialized by
firmware/bootloader)
- * which use memory range described by @a. If @a is NULL all such devices are
- * removed.
- */
-static inline int
-drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
- const char *name, bool primary)
-{
-#if IS_REACHABLE(CONFIG_FB)
- return remove_conflicting_framebuffers(a, name, primary);
-#else
- return 0;
-#endif
-}
-
-/**
- * drm_fb_helper_remove_conflicting_pci_framebuffers - remove
firmware-configured framebuffers for PCI devices
- * @pdev: PCI device
- * @name: requesting driver name
- *
- * This function removes framebuffer devices (eg. initialized by firmware)
- * using memory range configured for any of @pdev's memory bars.
- *
- * The function assumes that PCI device with shadowed ROM drives a primary
- * display and so kicks out vga16fb.
- */
-static inline int
[PATCH v2 02/10] drm/format-helper: Add blitter functions
The blitter functions copy a framebuffer to I/O memory using one of
the existing conversion functions.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Daniel Vetter
Tested-by: nerdopolis
---
drivers/gpu/drm/drm_format_helper.c | 87 +
include/drm/drm_format_helper.h | 8 +++
2 files changed, 95 insertions(+)
diff --git a/drivers/gpu/drm/drm_format_helper.c
b/drivers/gpu/drm/drm_format_helper.c
index 8d5a683afea7..0e885cd34107 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -344,3 +344,90 @@ void drm_fb_xrgb_to_gray8(u8 *dst, void *vaddr, struct
drm_framebuffer *fb,
}
EXPORT_SYMBOL(drm_fb_xrgb_to_gray8);
+/**
+ * drm_fb_blit_rect_dstclip - Copy parts of a framebuffer to display memory
+ * @dst: The display memory to copy to
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
+ * @dst_format:FOURCC code of the display's color format
+ * @vmap: The framebuffer memory to copy from
+ * @fb:The framebuffer to copy from
+ * @clip: Clip rectangle area to copy
+ *
+ * This function copies parts of a framebuffer to display memory. If the
+ * formats of the display and the framebuffer mismatch, the blit function
+ * will attempt to convert between them.
+ *
+ * Use drm_fb_blit_dstclip() to copy the full framebuffer.
+ *
+ * Returns:
+ * 0 on success, or
+ * -EINVAL if the color-format conversion failed, or
+ * a negative error code otherwise.
+ */
+int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
+uint32_t dst_format, void *vmap,
+struct drm_framebuffer *fb,
+struct drm_rect *clip)
+{
+ uint32_t fb_format = fb->format->format;
+
+ /* treat alpha channel like filler bits */
+ if (fb_format == DRM_FORMAT_ARGB)
+ fb_format = DRM_FORMAT_XRGB;
+ if (dst_format == DRM_FORMAT_ARGB)
+ dst_format = DRM_FORMAT_XRGB;
+
+ if (dst_format == fb_format) {
+ drm_fb_memcpy_dstclip(dst, dst_pitch, vmap, fb, clip);
+ return 0;
+
+ } else if (dst_format == DRM_FORMAT_RGB565) {
+ if (fb_format == DRM_FORMAT_XRGB) {
+ drm_fb_xrgb_to_rgb565_dstclip(dst, dst_pitch,
+ vmap, fb, clip,
+ false);
+ return 0;
+ }
+ } else if (dst_format == DRM_FORMAT_RGB888) {
+ if (fb_format == DRM_FORMAT_XRGB) {
+ drm_fb_xrgb_to_rgb888_dstclip(dst, dst_pitch,
+ vmap, fb, clip);
+ return 0;
+ }
+ }
+
+ return -EINVAL;
+}
+EXPORT_SYMBOL(drm_fb_blit_rect_dstclip);
+
+/**
+ * drm_fb_blit_dstclip - Copy framebuffer to display memory
+ * @dst: The display memory to copy to
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
+ * @dst_format:FOURCC code of the display's color format
+ * @vmap: The framebuffer memory to copy from
+ * @fb:The framebuffer to copy from
+ *
+ * This function copies a full framebuffer to display memory. If the formats
+ * of the display and the framebuffer mismatch, the copy function will
+ * attempt to convert between them.
+ *
+ * See drm_fb_blit_rect_dstclip() for more inforamtion.
+ *
+ * Returns:
+ * 0 on success, or a negative error code otherwise.
+ */
+int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch,
+ uint32_t dst_format, void *vmap,
+ struct drm_framebuffer *fb)
+{
+ struct drm_rect fullscreen = {
+ .x1 = 0,
+ .x2 = fb->width,
+ .y1 = 0,
+ .y2 = fb->height,
+ };
+ return drm_fb_blit_rect_dstclip(dst, dst_pitch, dst_format, vmap, fb,
+ );
+}
+EXPORT_SYMBOL(drm_fb_blit_dstclip);
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 2b5036a5fbe7..4e0258a61311 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -28,4 +28,12 @@ void drm_fb_xrgb_to_rgb888_dstclip(void __iomem *dst,
unsigned int dst_pitch
void drm_fb_xrgb_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip);
+int drm_fb_blit_rect_dstclip(void __iomem *dst, unsigned int dst_pitch,
+uint32_t dst_format, void *vmap,
+struct drm_framebuffer *fb,
+struct drm_rect *rect);
+int drm_fb_blit_dstclip(void __iomem *dst, unsigned int dst_pitch,
+ uint32_t dst_format, void *vmap,
+
[PATCH v2 01/10] drm/format-helper: Pass destination pitch to drm_fb_memcpy_dstclip()
The memcpy's destination buffer might have a different pitch than the
source. Support different pitches as function argument.
Signed-off-by: Thomas Zimmermann
Reviewed-by: Daniel Vetter
Tested-by: nerdopolis
---
drivers/gpu/drm/drm_format_helper.c| 9 +
drivers/gpu/drm/mgag200/mgag200_mode.c | 2 +-
drivers/gpu/drm/tiny/cirrus.c | 2 +-
include/drm/drm_format_helper.h| 2 +-
4 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/drm_format_helper.c
b/drivers/gpu/drm/drm_format_helper.c
index c043ca364c86..8d5a683afea7 100644
--- a/drivers/gpu/drm/drm_format_helper.c
+++ b/drivers/gpu/drm/drm_format_helper.c
@@ -52,6 +52,7 @@ EXPORT_SYMBOL(drm_fb_memcpy);
/**
* drm_fb_memcpy_dstclip - Copy clip buffer
* @dst: Destination buffer (iomem)
+ * @dst_pitch: Number of bytes between two consecutive scanlines within dst
* @vaddr: Source buffer
* @fb: DRM framebuffer
* @clip: Clip rectangle area to copy
@@ -59,12 +60,12 @@ EXPORT_SYMBOL(drm_fb_memcpy);
* This function applies clipping on dst, i.e. the destination is a
* full (iomem) framebuffer but only the clip rect content is copied over.
*/
-void drm_fb_memcpy_dstclip(void __iomem *dst, void *vaddr,
- struct drm_framebuffer *fb,
+void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch,
+ void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip)
{
unsigned int cpp = fb->format->cpp[0];
- unsigned int offset = clip_offset(clip, fb->pitches[0], cpp);
+ unsigned int offset = clip_offset(clip, dst_pitch, cpp);
size_t len = (clip->x2 - clip->x1) * cpp;
unsigned int y, lines = clip->y2 - clip->y1;
@@ -73,7 +74,7 @@ void drm_fb_memcpy_dstclip(void __iomem *dst, void *vaddr,
for (y = 0; y < lines; y++) {
memcpy_toio(dst, vaddr, len);
vaddr += fb->pitches[0];
- dst += fb->pitches[0];
+ dst += dst_pitch;
}
}
EXPORT_SYMBOL(drm_fb_memcpy_dstclip);
diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c
b/drivers/gpu/drm/mgag200/mgag200_mode.c
index cece3e57fb27..9d576240faed 100644
--- a/drivers/gpu/drm/mgag200/mgag200_mode.c
+++ b/drivers/gpu/drm/mgag200/mgag200_mode.c
@@ -1554,7 +1554,7 @@ mgag200_handle_damage(struct mga_device *mdev, struct
drm_framebuffer *fb,
{
void *vmap = map->vaddr; /* TODO: Use mapping abstraction properly */
- drm_fb_memcpy_dstclip(mdev->vram, vmap, fb, clip);
+ drm_fb_memcpy_dstclip(mdev->vram, fb->pitches[0], vmap, fb, clip);
/* Always scanout image at VRAM offset 0 */
mgag200_set_startadd(mdev, (u32)0);
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index ad922c3ec681..ae5643b0a6f4 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -323,7 +323,7 @@ static int cirrus_fb_blit_rect(struct drm_framebuffer *fb,
const struct dma_buf_
return -ENODEV;
if (cirrus->cpp == fb->format->cpp[0])
- drm_fb_memcpy_dstclip(cirrus->vram,
+ drm_fb_memcpy_dstclip(cirrus->vram, fb->pitches[0],
vmap, fb, rect);
else if (fb->format->cpp[0] == 4 && cirrus->cpp == 2)
diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h
index 5f9e37032468..2b5036a5fbe7 100644
--- a/include/drm/drm_format_helper.h
+++ b/include/drm/drm_format_helper.h
@@ -11,7 +11,7 @@ struct drm_rect;
void drm_fb_memcpy(void *dst, void *vaddr, struct drm_framebuffer *fb,
struct drm_rect *clip);
-void drm_fb_memcpy_dstclip(void __iomem *dst, void *vaddr,
+void drm_fb_memcpy_dstclip(void __iomem *dst, unsigned int dst_pitch, void
*vaddr,
struct drm_framebuffer *fb,
struct drm_rect *clip);
void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb,
--
2.30.1
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
[PATCH v2 00/10] drm: Support simple-framebuffer devices and firmware fbs
This patchset adds support for simple-framebuffer platform devices and a handover mechanism for native drivers to take-over control of the hardware. The new driver, called simpledrm, binds to a simple-frambuffer platform device. The kernel's boot code creates such devices for firmware-provided framebuffers, such as EFI-GOP or VESA. Typically the BIOS, UEFI or boot loader sets up the framebuffers. Description via device tree is also an option. Simpledrm is small enough to be linked into the kernel. The driver's main purpose is to provide graphical output during the early phases of the boot process, before the native DRM drivers are available. Native drivers are typically loaded from an initrd ram disk. Occationally simpledrm can also serve as interim solution on graphics hardware without native DRM driver. So far distributions rely on fbdev drivers, such as efifb, vesafb or simplefb, for early-boot graphical output. However fbdev is deprecated and the drivers do not provide DRM interfaces for modern userspace. Patches 1 and 2 prepare the DRM format helpers for simpledrm. Patches 3 and 4 add a hand-over mechanism. Simpledrm acquires it's framebuffer's I/O-memory range and provides a callback function to be removed by a native driver. The native driver will remove simpledrm before taking over the hardware. The removal is integrated into existing helpers, so drivers use it automatically. Patches 5 to 10 add the simpledrm driver. It's build on simple DRM helpers and SHMEM. It supports 16-bit, 24-bit and 32-bit RGB framebuffers. During pageflips, SHMEM buffers are copied into the framebuffer memory, similar to cirrus or mgag200. The code in patches 8 and 9 handles clocks and regulators. It's based on the simplefb drivers, but has been modified for DRM. I've also been working on fastboot support (i.e., flicker-free booting). This requires state-readout from simpledrm via generic interfaces, as outlined in [1]. I do have some prototype code, but it will take a while to get this ready. Simpledrm will then support it. I've tested simpledrm with x86 EFI and VESA framebuffers, which both work reliably. The fbdev console and Weston work automatically. Xorg requires manual configuration of the device. Xorgs current modesetting driver does not work with both, platform and PCI device, for the same physical hardware. Once configured, X11 works. I looked into X11, but couldn't see an easy way of fixing the problem. With the push towards Wayland+Xwayland I expect the problem to become a non-issue soon. Additional testing has been reported at [2]. One cosmetical issue is that simpledrm's device file is card0 and the native driver's device file is card1. After simpledrm has been kicked out, only card1 is left. This does not seem to be a practical problem however. TODO/IDEAS: * provide deferred takeover * provide bootsplash DRM client * make simplekms usable with ARM-EFI fbs v2: * rename to simpledrm, aperture helpers * reorganized patches * use hotplug helpers for removal (Daniel) * added DT match tables (Rob) * use shadow-plane helpers * lots of minor cleanups [1] https://lore.kernel.org/dri-devel/CAKMK7uHtqHy_oz4W7F+hmp9iqp7W5Ra8CxPvJ=9bwmvfu-o...@mail.gmail.com/ [2] https://lore.kernel.org/dri-devel/1761762.3HQLrFs1K7@nerdopolis/ Thomas Zimmermann (10): drm/format-helper: Pass destination pitch to drm_fb_memcpy_dstclip() drm/format-helper: Add blitter functions drm/aperture: Move fbdev conflict helpers into drm_aperture.h drm/aperture: Add infrastructure for aperture ownership drm: Add simpledrm driver drm/simpledrm: Add fbdev emulation drm/simpledrm: Initialize framebuffer data from device-tree node drm/simpledrm: Acquire clocks from DT device node drm/simpledrm: Acquire regulators from DT device node drm/simpledrm: Acquire memory aperture for framebuffer Documentation/gpu/drm-internals.rst| 12 + MAINTAINERS| 7 + drivers/gpu/drm/Kconfig| 7 + drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/drm_aperture.c | 287 drivers/gpu/drm/drm_format_helper.c| 96 ++- drivers/gpu/drm/mgag200/mgag200_mode.c | 2 +- drivers/gpu/drm/tiny/Kconfig | 17 + drivers/gpu/drm/tiny/Makefile | 1 + drivers/gpu/drm/tiny/cirrus.c | 2 +- drivers/gpu/drm/tiny/simpledrm.c | 932 + include/drm/drm_aperture.h | 96 +++ include/drm/drm_fb_helper.h| 56 +- include/drm/drm_format_helper.h| 10 +- 14 files changed, 1466 insertions(+), 60 deletions(-) create mode 100644 drivers/gpu/drm/drm_aperture.c create mode 100644 drivers/gpu/drm/tiny/simpledrm.c create mode 100644 include/drm/drm_aperture.h -- 2.30.1 ___ Virtualization mailing list Virtualization@lists.linux-foundation.org
Re: [RFC v2 05/13] vhost: Route guest->host notification through shadow virtqueue
在 2021/3/18 下午5:18, Eugenio Perez Martin 写道:
On Thu, Mar 18, 2021 at 4:11 AM Jason Wang wrote:
在 2021/3/18 上午12:47, Eugenio Perez Martin 写道:
On Wed, Mar 17, 2021 at 3:05 AM Jason Wang wrote:
在 2021/3/16 下午6:31, Eugenio Perez Martin 写道:
On Tue, Mar 16, 2021 at 8:18 AM Jason Wang wrote:
在 2021/3/16 上午3:48, Eugenio Pérez 写道:
Shadow virtqueue notifications forwarding is disabled when vhost_dev
stops, so code flow follows usual cleanup.
Signed-off-by: Eugenio Pérez
---
hw/virtio/vhost-shadow-virtqueue.h | 7 ++
include/hw/virtio/vhost.h | 4 +
hw/virtio/vhost-shadow-virtqueue.c | 113 ++-
hw/virtio/vhost.c | 143 -
4 files changed, 265 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/vhost-shadow-virtqueue.h
b/hw/virtio/vhost-shadow-virtqueue.h
index 6cc18d6acb..c891c6510d 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -17,6 +17,13 @@
typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
+bool vhost_shadow_vq_start(struct vhost_dev *dev,
+ unsigned idx,
+ VhostShadowVirtqueue *svq);
+void vhost_shadow_vq_stop(struct vhost_dev *dev,
+ unsigned idx,
+ VhostShadowVirtqueue *svq);
+
VhostShadowVirtqueue *vhost_shadow_vq_new(struct vhost_dev *dev, int idx);
void vhost_shadow_vq_free(VhostShadowVirtqueue *vq);
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index ac963bf23d..7ffdf9aea0 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -55,6 +55,8 @@ struct vhost_iommu {
QLIST_ENTRY(vhost_iommu) iommu_next;
};
+typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
+
typedef struct VhostDevConfigOps {
/* Vhost device config space changed callback
*/
@@ -83,7 +85,9 @@ struct vhost_dev {
uint64_t backend_cap;
bool started;
bool log_enabled;
+bool shadow_vqs_enabled;
uint64_t log_size;
+VhostShadowVirtqueue **shadow_vqs;
Any reason that you don't embed the shadow virtqueue into
vhost_virtqueue structure?
Not really, it could be relatively big and I would prefer SVQ
members/methods to remain hidden from any other part that includes
vhost.h. But it could be changed, for sure.
(Note that there's a masked_notifier in struct vhost_virtqueue).
They are used differently: in SVQ the masked notifier is a pointer,
and if it's NULL the SVQ code knows that device is not masked. The
vhost_virtqueue is the real owner.
Yes, but it's an example for embedding auxciliary data structures in the
vhost_virtqueue.
It could be replaced by a boolean in SVQ or something like that, I
experimented with a tri-state too (UNMASKED, MASKED, MASKED_NOTIFIED)
and let vhost.c code to manage all the transitions. But I find clearer
the pointer use, since it's the more natural for the
vhost_virtqueue_mask, vhost_virtqueue_pending existing functions.
This masking/unmasking is the part I dislike the most from this
series, so I'm very open to alternatives.
See below. I think we don't even need to care about that.
Error *migration_blocker;
const VhostOps *vhost_ops;
void *opaque;
diff --git a/hw/virtio/vhost-shadow-virtqueue.c
b/hw/virtio/vhost-shadow-virtqueue.c
index 4512e5b058..3e43399e9c 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -8,9 +8,12 @@
*/
#include "hw/virtio/vhost-shadow-virtqueue.h"
+#include "hw/virtio/vhost.h"
+
+#include "standard-headers/linux/vhost_types.h"
#include "qemu/error-report.h"
-#include "qemu/event_notifier.h"
+#include "qemu/main-loop.h"
/* Shadow virtqueue to relay notifications */
typedef struct VhostShadowVirtqueue {
@@ -18,14 +21,121 @@ typedef struct VhostShadowVirtqueue {
EventNotifier kick_notifier;
/* Shadow call notifier, sent to vhost */
EventNotifier call_notifier;
+
+/*
+ * Borrowed virtqueue's guest to host notifier.
+ * To borrow it in this event notifier allows to register on the event
+ * loop and access the associated shadow virtqueue easily. If we use the
+ * VirtQueue, we don't have an easy way to retrieve it.
So this is something that worries me. It looks like a layer violation
that makes the codes harder to work correctly.
I don't follow you here.
The vhost code already depends on virtqueue in the same sense:
virtio_queue_get_host_notifier is called on vhost_virtqueue_start. So
if this behavior ever changes it is unlikely for vhost to keep working
without changes. vhost_virtqueue has a kick/call int where I think it
should be stored actually, but they are never used as far as I see.
Previous RFC did rely on vhost_dev_disable_notifiers. From its documentation:
/* Stop processing guest IO notifications in vhost.
* Start
Re: [PATCH] virtio_net: replace if (cond) BUG() with BUG_ON()
On Wed, Mar 17, 2021 at 01:57:15PM +0800, Jiapeng Chong wrote: > Fix the following coccicheck warnings: > > ./drivers/net/virtio_net.c:1551:2-5: WARNING: Use BUG_ON instead of if > condition followed by BUG. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/net/virtio_net.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index 82e520d..093530b 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -1545,10 +1545,8 @@ static int xmit_skb(struct send_queue *sq, struct > sk_buff *skb) > else > hdr = skb_vnet_hdr(skb); > > - if (virtio_net_hdr_from_skb(skb, >hdr, > - virtio_is_little_endian(vi->vdev), false, > - 0)) > - BUG(); > + BUG_ON(virtio_net_hdr_from_skb(skb, >hdr, > virtio_is_little_endian(vi->vdev), > +false, 0)); This BUG() in virtio isn't supposed to be in the first place. You should return -EINVAL instead of crashing system. Thanks > > if (vi->mergeable_rx_bufs) > hdr->num_buffers = 0; > -- > 1.8.3.1 > ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
