linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
To: David Laight <david.laight.linux@gmail.com>
Cc: "Sandy Huang" <hjc@rock-chips.com>,
	"Heiko Stübner" <heiko@sntech.de>,
	"Andy Yan" <andy.yan@rock-chips.com>,
	"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>,
	"Chaoyi Chen" <chaoyi.chen@rock-chips.com>,
	dri-devel@lists.freedesktop.org,
	linux-arm-kernel@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	"Daniel Stone" <daniels@collabora.com>,
	kernel@collabora.com
Subject: Re: [PATCH v4 1/8] drm/rockchip: vop2: Switch impossible format conditional to WARN_ON
Date: Fri, 12 Dec 2025 13:43:55 +0100	[thread overview]
Message-ID: <14738785.uLZWGnKmhe@workhorse> (raw)
In-Reply-To: <20251211223822.6eeabb4d@pumpkin>

On Thursday, 11 December 2025 23:38:22 Central European Standard Time David Laight wrote:
> On Thu, 11 Dec 2025 21:40:31 +0100
> Nicolas Frattaroli <nicolas.frattaroli@collabora.com> wrote:
> 
> > From: Daniel Stone <daniels@collabora.com>
> > 
> > We should never be able to create a framebuffer with an unsupported
> > format, so throw a warning if this ever happens, instead of attempting
> > to stagger on.
> 
> It doesn't look like you've changed the behaviour.

Yes, the commit message needs to be adjusted.

> Except that all the systems with PANIC_ON_WARN set will panic.
> I believe that is somewhere over 90% of systems.

I also like making up statistics. Warning here is the correct move
in my opinion because this warning being triggered indicates a bug
in the kernel code, and with PANIC_ON_WARN the user explicitly says
they would rather panic in such a case than treat it as an abnormal
condition that is recoverable.

The reason why this condition ever occurring should be treated as an
abnormal condition is because the DRM subsystem should guarantee we
don't get a framebuffer of a format we didn't explicitly declare
support for in the first place. So this condition being hit either
means drm_universal_plane_init is broken, or the array of formats
that's passed to it is out of sync with the conversion code, which
is also a bug. Or someone managed to thoroughly hose DRM's internal
kernel-side data structures, which is precisely the kind of thing
PANIC_ON_WARN users want to abort for.

> 
> 	David
> 
> > 
> > Signed-off-by: Daniel Stone <daniels@collabora.com>
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > index 498df0ce4680..20b49209ddcd 100644
> > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
> > @@ -1030,7 +1030,8 @@ static int vop2_plane_atomic_check(struct drm_plane *plane,
> >  		return 0;
> >  
> >  	format = vop2_convert_format(fb->format->format);
> > -	if (format < 0)
> > +	/* We shouldn't be able to create a fb for an unsupported format */
> > +	if (WARN_ON(format < 0))
> >  		return format;
> >  
> >  	/* Co-ordinates have now been clipped */
> > 
> 
> 






  reply	other threads:[~2025-12-12 12:46 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-11 20:40 [PATCH v4 0/8] drm/rockchip: No more post-atomic_check fixups Nicolas Frattaroli
2025-12-11 20:40 ` [PATCH v4 1/8] drm/rockchip: vop2: Switch impossible format conditional to WARN_ON Nicolas Frattaroli
2025-12-11 22:38   ` David Laight
2025-12-12 12:43     ` Nicolas Frattaroli [this message]
2025-12-15 17:55       ` Daniel Stone
2025-12-15 18:16         ` David Laight
2025-12-11 20:40 ` [PATCH v4 2/8] drm/rockchip: vop2: Switch impossible pos " Nicolas Frattaroli
2025-12-11 22:41   ` David Laight
2025-12-15 17:57     ` Daniel Stone
2025-12-11 20:40 ` [PATCH v4 3/8] drm/rockchip: vop2: Fix Esmart test condition Nicolas Frattaroli
2025-12-11 20:40 ` [PATCH v4 4/8] drm/rockchip: vop2: Enforce scaling workaround in plane_check Nicolas Frattaroli
2025-12-11 20:40 ` [PATCH v4 5/8] drm/rockchip: vop2: Enforce AFBC source alignment " Nicolas Frattaroli
2025-12-11 20:40 ` [PATCH v4 6/8] drm/rockchip: vop2: Enforce AFBC transform stride align " Nicolas Frattaroli
2025-12-11 20:40 ` [PATCH v4 7/8] drm/rockchip: vop2: Use drm_is_afbc helper function Nicolas Frattaroli
2025-12-11 20:40 ` [PATCH v4 8/8] drm/rockchip: vop2: Simplify format_mod_supported Nicolas Frattaroli

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=14738785.uLZWGnKmhe@workhorse \
    --to=nicolas.frattaroli@collabora.com \
    --cc=airlied@gmail.com \
    --cc=andy.yan@rock-chips.com \
    --cc=chaoyi.chen@rock-chips.com \
    --cc=daniels@collabora.com \
    --cc=david.laight.linux@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=heiko@sntech.de \
    --cc=hjc@rock-chips.com \
    --cc=kernel@collabora.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=maarten.lankhorst@linux.intel.com \
    --cc=mripard@kernel.org \
    --cc=simona@ffwll.ch \
    --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;
as well as URLs for NNTP newsgroup(s).