| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGb2v64bp0BYYdCbaS+wg0H+MD27Bk-n5i8t9X5nVGTG3_hX_Q@mail.gmail.com>
Date: Fri, 26 Dec 2025 03:30:23 +0800
From: Chen-Yu Tsai <wens@...nel.org>
To: Jernej Škrabec <jernej.skrabec@...il.com>
Cc: samuel@...lland.org, mripard@...nel.org, maarten.lankhorst@...ux.intel.com,
tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch, robh@...nel.org,
krzk+dt@...nel.org, conor+dt@...nel.org, mturquette@...libre.com,
sboyd@...nel.org, dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-sunxi@...ts.linux.dev,
linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org
Subject: Re: [PATCH 5/7] drm/sun4i: Add planes driver
On Fri, Dec 26, 2025 at 3:17 AM Jernej Škrabec <jernej.skrabec@...il.com> wrote:
>
> Dne četrtek, 25. december 2025 ob 10:37:06 Srednjeevropski standardni čas je Chen-Yu Tsai napisal(a):
> > On Thu, Dec 25, 2025 at 5:29 PM Chen-Yu Tsai <wens@...nel.org> wrote:
> > >
> > > On Sat, Nov 15, 2025 at 10:14 PM Jernej Skrabec
> > > <jernej.skrabec@...il.com> wrote:
> > > >
> > > > This driver serves just as planes sharing manager, needed for Display
> > > > Engine 3.3 and newer.
> > > >
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@...il.com>
> > > > ---
> > > > drivers/gpu/drm/sun4i/Kconfig | 8 +
> > > > drivers/gpu/drm/sun4i/Makefile | 1 +
> > > > drivers/gpu/drm/sun4i/sun50i_planes.c | 205 ++++++++++++++++++++++++++
> > > > drivers/gpu/drm/sun4i/sun50i_planes.h | 43 ++++++
> > > > 4 files changed, 257 insertions(+)
> > > > create mode 100644 drivers/gpu/drm/sun4i/sun50i_planes.c
> > > > create mode 100644 drivers/gpu/drm/sun4i/sun50i_planes.h
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig
> > > > index b56ba00aabca..946dd7606094 100644
> > > > --- a/drivers/gpu/drm/sun4i/Kconfig
> > > > +++ b/drivers/gpu/drm/sun4i/Kconfig
> > > > @@ -85,4 +85,12 @@ config DRM_SUN8I_TCON_TOP
> > > > TCON TOP is responsible for configuring display pipeline for
> > > > HDMI, TVE and LCD.
> > > >
> > > > +config DRM_SUN50I_PLANES
> > > > + tristate
> > > > + default DRM_SUN4I if DRM_SUN8I_MIXER!=n
> > > > + help
> > > > + Chose this option if you have an Allwinner Soc with the
> > > > + Display Engine 3.3 or newer. Planes are shared resource
> > > > + between multiple mixers.
> > > > +
> > > > endif
> > > > diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile
> > > > index bad7497a0d11..03f002abef15 100644
> > > > --- a/drivers/gpu/drm/sun4i/Makefile
> > > > +++ b/drivers/gpu/drm/sun4i/Makefile
> > > > @@ -38,3 +38,4 @@ obj-$(CONFIG_DRM_SUN6I_DSI) += sun6i_mipi_dsi.o
> > > > obj-$(CONFIG_DRM_SUN8I_DW_HDMI) += sun8i-drm-hdmi.o
> > > > obj-$(CONFIG_DRM_SUN8I_MIXER) += sun8i-mixer.o
> > > > obj-$(CONFIG_DRM_SUN8I_TCON_TOP) += sun8i_tcon_top.o
> > > > +obj-$(CONFIG_DRM_SUN50I_PLANES) += sun50i_planes.o
> > >
> > > I don't think you can have this as a separate module:
> > >
> > > a. You are using sun8i_vi_layer_init_one() and sun8i_ui_layer_init_one()
> > > from the sun8i-mixer module, and neither of them are exported symbols.
> > >
> > > b. You export sun50i_planes_setup() for sun8i-mixer to call, which ends
> > > up becoming a circular dependency.
> > >
> > > The easiest solution would be to just fold this into the sun8i-mixer module.
>
> I mimicked tcon-top module, but yeah, it's much less of a hassle to fold it
> into sun8i-mixer.
>
> > >
> > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun50i_planes.c b/drivers/gpu/drm/sun4i/sun50i_planes.c
> > > > new file mode 100644
> > > > index 000000000000..a99c01122990
> > > > --- /dev/null
> > > > +++ b/drivers/gpu/drm/sun4i/sun50i_planes.c
> > > > @@ -0,0 +1,205 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/* Copyright (c) 2025 Jernej Skrabec <jernej.skrabec@...il.com> */
> > > > +
> > > > +#include <linux/device.h>
> > > > +#include <linux/io.h>
> > > > +#include <linux/mfd/syscon.h>
> > > > +#include <linux/module.h>
> > > > +#include <linux/of.h>
> > > > +#include <linux/of_graph.h>
> > > > +#include <linux/platform_device.h>
> > > > +
> > > > +#include "sun50i_planes.h"
> > > > +#include "sun8i_ui_layer.h"
> > > > +#include "sun8i_vi_layer.h"
> > > > +
> > > > +static bool sun50i_planes_node_is_planes(struct device_node *node)
> > > > +{
> > > > + return !!of_match_node(sun50i_planes_of_table, node);
> > > > +}
> > > > +
> > > > +struct drm_plane **
> > > > +sun50i_planes_setup(struct device *dev, struct drm_device *drm,
> > > > + unsigned int mixer)
> > > > +{
> > > > + struct sun50i_planes *planes = dev_get_drvdata(dev);
> > > > + const struct sun50i_planes_quirks *quirks;
> > > > + struct drm_plane **drm_planes;
> > > > + const struct default_map *map;
> > > > + unsigned int i;
> > > > +
> > > > + if (!sun50i_planes_node_is_planes(dev->of_node)) {
> > > > + dev_err(dev, "Device is not planes driver!\n");
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + if (!planes) {
> > > > + dev_err(dev, "Planes driver is not loaded yet!\n");
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + if (mixer > 1) {
> > > > + dev_err(dev, "Mixer index is too high!\n");
> > > > + return NULL;
> > > > + }
> > > > +
> > > > + quirks = planes->quirks;
> > > > + map = &quirks->def_map[mixer];
> > > > +
> > > > + drm_planes = devm_kcalloc(drm->dev, map->num_ch + 1,
> > >
> > > Just a note: it seems we are missing the sentinel in sun8i_layers_init().
>
> Why do you think so? Current mainline code has mixer->cfg->vi_num +
> mixer->cfg->ui_num + 1.
I believe this was changed in your previous cleanups:
https://lore.kernel.org/all/20251104180942.61538-16-jernej.skrabec@gmail.com/
ChenYu
Powered by blists - more mailing lists