public inbox for drm-ai-reviews@public-inbox.freedesktop.org
 help / color / mirror / Atom feed
From: Chen-Yu Tsai <wenst@chromium.org>
To: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
	Maxime Ripard <mripard@kernel.org>,
	Thomas Zimmermann <tzimmermann@suse.de>,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] drm/xlnx/zynqmp-dpsub: Fix dependencies for COMPILE_TEST
Date: Fri, 10 Apr 2026 17:08:00 +0800	[thread overview]
Message-ID: <CAGXv+5HjX3FagZs5K+Z=PYQWt2a92My6wgG+ZXKh140w3Hz-QA@mail.gmail.com> (raw)
In-Reply-To: <386241e0-3d1b-43cc-8bc0-b6d35a97ba89@ideasonboard.com>

On Fri, Apr 10, 2026 at 4:49 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
>
> Hi,
>
> On 08/04/2026 11:14, Chen-Yu Tsai wrote:
> > The zynqmp-dpsub driver does not have build time dependencies on the PHY
> > or DMA drivers. These are runtime hardware restrictions.
> >
> > Group the two dependencies with ARCH_ZYNQMP so that the driver can be
> > compile tested without them.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> > IMO the two driver dependencies could be removed altogether, but that
> > would be up to the driver and platform maintainers.
> > ---
> >   drivers/gpu/drm/xlnx/Kconfig | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/Kconfig b/drivers/gpu/drm/xlnx/Kconfig
> > index cfabf5e2a0bb..4c6729459f40 100644
> > --- a/drivers/gpu/drm/xlnx/Kconfig
> > +++ b/drivers/gpu/drm/xlnx/Kconfig
> > @@ -1,10 +1,8 @@
> >   config DRM_ZYNQMP_DPSUB
> >       tristate "ZynqMP DisplayPort Controller Driver"
> > -     depends on ARCH_ZYNQMP || COMPILE_TEST
> > +     depends on (ARCH_ZYNQMP && PHY_XILINX_ZYNQMP && XILINX_ZYNQMP_DPDMA) || COMPILE_TEST
> >       depends on COMMON_CLK && DRM && OF
> >       depends on DMADEVICES
> > -     depends on PHY_XILINX_ZYNQMP
> > -     depends on XILINX_ZYNQMP_DPDMA
> >       select DMA_ENGINE
> >       select DRM_CLIENT_SELECTION
> >       select DRM_DISPLAY_DP_HELPER
>
> I think the above looks more difficult to understand than the current
> version. We should perhaps rather drop the dependencies. But if we go
> that way, then... we can also drop DMADEVICES, DMA_ENGINE, GENERIC_PHY
> at least.

Perhaps. All the APIs for these are properly stubbed, so I guess
they also count as runtime dependencies.

> What problem does this solve? Why are these two dependencies bad for
> compile testing, but the other dependencies/selects are ok?

I was build testing changes across multiple DRM drivers in addition to
the main platform I work on. Having to find and select each dependency
was annoying.

I would say DMADEVICES and GENERIC_PHY are much more common to embedded
devices than SoC specific drivers.

> I personally don't mind hard runtime dependencies expressed in the
> Kconfig, as searching for the correct dependency-drivers when your
> driver doesn't probe is always a PITA.

I don't mind it either. But the way they are described means that compile
testing is overly dependent on having some other platform specific
driver enabled.


I suppose another way to write this would be:

    depends on ARCH_ZYNQMP if !COMPILE_TEST
    depends on PHY_XILINX_ZYNQMP if !COMPILE_TEST
    depends on XILINX_ZYNQMP_DPDMA if !COMPILE_TEST
    ...

Does that work for you?


Thanks
ChenYu

  reply	other threads:[~2026-04-10  9:08 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-08  8:14 [PATCH] drm/xlnx/zynqmp-dpsub: Fix dependencies for COMPILE_TEST Chen-Yu Tsai
2026-04-10  8:49 ` Tomi Valkeinen
2026-04-10  9:08   ` Chen-Yu Tsai [this message]
2026-04-10  9:27     ` Ville Syrjälä
2026-04-10 10:43     ` Tomi Valkeinen
2026-04-12  2:56 ` Claude review: " Claude Code Review Bot
2026-04-12  2:56 ` Claude Code Review Bot

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='CAGXv+5HjX3FagZs5K+Z=PYQWt2a92My6wgG+ZXKh140w3Hz-QA@mail.gmail.com' \
    --to=wenst@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=tomi.valkeinen@ideasonboard.com \
    --cc=tzimmermann@suse.de \
    /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