From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A59BB33F5A0 for ; Tue, 9 Jun 2026 14:59:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781017176; cv=none; b=iCL3IhcKgytwcgtm3kFSOAcwp4msXiKXtipjA3q0b8al6S5q0Q+WYlNlXOFM+cXBethnXLj2sBvS8J9vzbj1rAo6wufAlohTZ8Z4OmVp9rBpwt2h5t7e7Yi8LXoqci9lImZZg4corxvxqwiiEKuFRNAVG/GKlqFH4XAKug+FtmI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781017176; c=relaxed/simple; bh=RFAsydRRnH9kBK1HE9vkCKxVfrbNnjJ51L2OAwOgw8Q=; h=Date:From:To:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=DKJ8ZQfwpxyy6Ojo30Aiq+lpW8BvcCdFsYvMfNvnHpE01+zFOBJRXjc15CtbML68rWHF+GL35Ck0kkwdgIz7CyULKBE0YIHuQoREGCIvXpEtiK2VAFivqmmThx2GXNEWcfkz6GENb26J1aS3pND55WtQmTcHdcL2hNXy974aAkI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch; spf=none smtp.mailfrom=ffwll.ch; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b=lMFYYx22; arc=none smtp.client-ip=209.85.128.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ffwll.ch Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=ffwll.ch Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=ffwll.ch header.i=@ffwll.ch header.b="lMFYYx22" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-490abf12f0fso29774965e9.0 for ; Tue, 09 Jun 2026 07:59:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ffwll.ch; s=google; t=1781017170; x=1781621970; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:to:from:date:from:to:cc:subject :date:message-id:reply-to; bh=tUdHUMUik9TEHOW4E8Paa+t240ejFjgq+HfjWasiBuI=; b=lMFYYx22yzRTpK3gwNK7EfO0C/3ncfvht1JqW/uYHMcINqaJWSA6+hVJI7iQcmNJdK emdTU85QnDPfwOPwOHLP6wy8stOPwUHZCFNOFYi6da5BM/+0OciuHtf71EsxoS9Rd4gk 3/Mc3qwgiwoNRDyslN5B52hyF6suIknp69CKg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781017170; x=1781621970; h=in-reply-to:content-disposition:mime-version:references :mail-followup-to:message-id:subject:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=tUdHUMUik9TEHOW4E8Paa+t240ejFjgq+HfjWasiBuI=; b=WAGSJWEAlvvyWBPJIE8wOpbav2Xszmu6oIqgfNsqrN8m8NlPWGGjvwAqkjoCIz+fFD EiBJc22UcAJWV2OyViKJqEr9iQAiZfDEECVneQgYiQhq0+zdSIw2L0F5dZR20Yc1I+DO ewzKsk0fRJjywsNRS6MoiCWU0Qb5AGEgvW+qYQravvah3QbAHEeZglIETP+wHUSjK7Fy hQQcMgk71RFbleJ229bRQEiu2rlDYleLoOGfK92ZNXLENaB4WL77y6RMb9/dOwS2Qr1Y e1FbRD9Qk7jc5SPcJXe+o16wb7iltKJmXKkfUP75PY8rfhHH1mut9WqzTA999pS1Cqfu SEtQ== X-Forwarded-Encrypted: i=1; AFNElJ890TYBpen6+biNzQg4GIrj0Sthj0+wSWV++358RLjn+Qzp9X1y6M0nBEVfao1hXKmMCBN8SPG4VHUqKx8=@vger.kernel.org X-Gm-Message-State: AOJu0YzC4rB+lCb/l2+FJmmDuiZr0Ph1WDgCevoiCkjSA0K32reVfcg2 jCiIDjroDcQWwaO/QvFf001aODs7Yidxbk0pw0oDdb2VTKTvQOolDodHWAoGCSHaPa8= X-Gm-Gg: Acq92OH1cDFWxDR5sXdRx52jsGlTjcDIVZFhdm7aOUJ4V4Y6BZjDt7eBpxaz/ioIVT8 eXvPmd579Sj+KPgzIw1u9+SWTuKwEkDVOXfUi9tTFx8iyrEogCQMP7PZlUGKFxUKNxpAvO0dOM2 uIc/6+jbCVneMnS4VCN4mG9n1sa7l+bZ4zC7pKMcoat5z3lk17hW/2eOwt6613pbe0goyiAf/Zh 1a0Nj6i5bFj1OXlptx5gnAJyrjfkB81Zx86kQ65p9wjq1PlIcDNqtuCJi3/3bpPalDCFVsyG/wy Q5ehwJ4FE/t5LygcVo4GgrFxRvnqxHkYclo5BQoePe62jjsWi18cTYrjEfFzNxmOjfCKbrz6Oyp 9tZL3gS3mosgYy8wlZKqqE0UYIBrhcP1U35OzU+TM/xJy1ecqQDKjCJ7is3/gWymc0WPBpSvz5/ 8SSgnk0Uu/fpIo8tfT3dehumFKFefwUhsWU2DRXh69Be9tiA== X-Received: by 2002:a05:600c:8208:b0:490:ad1e:1846 with SMTP id 5b1f17b1804b1-490c2cf6718mr248198135e9.9.1781017169549; Tue, 09 Jun 2026 07:59:29 -0700 (PDT) Received: from phenom.ffwll.local ([2a02:168:57f4:0:5485:d4b2:c087:b497]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-490bc3b5b82sm518278635e9.1.2026.06.09.07.59.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 09 Jun 2026 07:59:28 -0700 (PDT) Date: Tue, 9 Jun 2026 16:59:27 +0200 From: Simona Vetter To: Maxime Ripard , Maarten Lankhorst , Thomas Zimmermann , David Airlie , Simona Vetter , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Jyri Sarha , Tomi Valkeinen , Devarsh Thakkar , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 11/28] drm/atomic_sro: Create kernel parameter to force or disable readout Message-ID: Mail-Followup-To: Maxime Ripard , Maarten Lankhorst , Thomas Zimmermann , David Airlie , Simona Vetter , Andrzej Hajda , Neil Armstrong , Robert Foss , Laurent Pinchart , Jonas Karlman , Jernej Skrabec , Jyri Sarha , Tomi Valkeinen , Devarsh Thakkar , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org References: <20260423-drm-state-readout-v2-0-8549f87cb978@kernel.org> <20260423-drm-state-readout-v2-11-8549f87cb978@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Operating-System: Linux phenom 6.19.10+deb14-amd64 On Thu, May 21, 2026 at 12:51:06PM +0200, Simona Vetter wrote: > On Thu, Apr 23, 2026 at 12:18:24PM +0200, Maxime Ripard wrote: > > The hardware state readout is useful, but might need to be disabled > > in case of bugs, or its checks relaxed during development when not > > all hooks are implemented yet. > > > > Add a module parameter to control the readout behavior: it can be > > disabled entirely, or the checks for missing compare or readout hooks > > can be skipped independently. > > > > Suggested-by: Simona Vetter > > Signed-off-by: Maxime Ripard > > --- > > drivers/gpu/drm/drm_atomic_sro.c | 36 ++++++++++++++++++++++++++++++++++++ > > include/drm/drm_atomic_sro.h | 2 ++ > > 2 files changed, 38 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_atomic_sro.c b/drivers/gpu/drm/drm_atomic_sro.c > > index 177b97d451f5..a46f06e75c4e 100644 > > --- a/drivers/gpu/drm/drm_atomic_sro.c > > +++ b/drivers/gpu/drm/drm_atomic_sro.c > > @@ -11,10 +11,46 @@ > > #include > > > > #include "drm_internal.h" > > #include "drm_crtc_internal.h" > > > > +enum drm_atomic_readout_status { > > + DRM_ATOMIC_READOUT_DISABLED = 0, > > + DRM_ATOMIC_READOUT_ENABLED, > > + DRM_ATOMIC_READOUT_SKIP_MISSING_COMPARE, > > + DRM_ATOMIC_READOUT_SKIP_MISSING_READOUT, > > +}; > > + > > +static unsigned int atomic_readout = DRM_ATOMIC_READOUT_ENABLED; > > +module_param_unsafe(atomic_readout, uint, 0); > > Default is actually 0 here. I agree with the docs that it should be 1, > since drivers have an explicit opt-in through setting the main entry point > in drm_mode_config_funcs. > > I was also pondering whether we should have a compare-only mode, but the > issue is that once you build a driver on readout being a thing, that could > blow up. So I think that should be left as a per-driver tunable. > > That's also why this must be a unsafe debug option, it might actually > break the driver. Maxime pointed out that this makes non sense because the default is already enabled, and yes I got confused. So strike that, but maybe think whether 0600 as permissions makes more sense, because that's the bit that confused me. Either way, this hunk here also looks good as-is. -Sima > > > +MODULE_PARM_DESC(atomic_readout, > > + "Enable Hardware State Readout (0 = disabled, 1 = enabled, 2 = ignore missing compares, 3 = ignore missing readouts and compares, default = 1)"); > > + > > +/** > > + * drm_atomic_sro_device_can_readout - check if a device supports hardware state readout > > + * @dev: DRM device to check > > + * > > + * Verifies that the device is an atomic driver, that readout is > > + * enabled, and that all KMS objects implement the relevant hooks. > > + * > > + * RETURNS: > > + * > > + * True if the device supports full hardware state readout, false > > + * otherwise. > > + */ > > +bool drm_atomic_sro_device_can_readout(struct drm_device *dev) > > +{ > > + if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) > > I think this should be drm_drv_uses_atomic_modeset() since it's an > internal check, not an uapi check. > > With the two issues addressed: > > Reviewed-by: Simona Vetter > > > + return false; > > + > > + if (atomic_readout == DRM_ATOMIC_READOUT_DISABLED) > > + return false; > > + > > + return true; > > +} > > +EXPORT_SYMBOL(drm_atomic_sro_device_can_readout); > > + > > struct __drm_atomic_sro_plane { > > struct drm_plane *ptr; > > struct drm_plane_state *state; > > }; > > > > diff --git a/include/drm/drm_atomic_sro.h b/include/drm/drm_atomic_sro.h > > index 5a9333a05796..6e5262384c71 100644 > > --- a/include/drm/drm_atomic_sro.h > > +++ b/include/drm/drm_atomic_sro.h > > @@ -13,10 +13,12 @@ struct drm_plane; > > struct drm_plane_state; > > struct drm_printer; > > struct drm_private_obj; > > struct drm_private_state; > > > > +bool drm_atomic_sro_device_can_readout(struct drm_device *dev); > > + > > struct drm_atomic_sro_state *drm_atomic_sro_state_alloc(struct drm_device *dev); > > void drm_atomic_sro_state_free(struct drm_atomic_sro_state *state); > > void drm_atomic_sro_state_print(const struct drm_atomic_sro_state *state, > > struct drm_printer *p); > > > > > > -- > > 2.53.0 > > > > -- > Simona Vetter > Software Engineer > http://blog.ffwll.ch -- Simona Vetter Software Engineer http://blog.ffwll.ch