Re: [PATCH v1 1/3] drm: Add passive_vrr properties for passive/desktop VRR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Contradictory kerneldoc for `passive_vrr_disabled` in `struct drm_crtc_state` and multiple spelling errors in comments.
--

commit 870f0fcb955a90bd32f228b223d6193abfa63290
Author: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Subject: drm: Add passive_vrr properties for passive/desktop VRR

This commit adds atomic CRTC and immutable connector properties to support
passive VRR for desktop usage on HDMI sinks. This aims to avoid blanking or
flickering on VRR entry and exit by keeping the sink in a variable-refresh
state during fixed refresh use.

> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 9d820a2a87ce4..c429105172b0d 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
[ ... ]
> @@ -2386,6 +2396,17 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>   *
>   *	The driver may place further restrictions within these minimum
>   *	and maximum bounds.
> + *
> + * "PASSIVE_VRR_DISABLED":
> + *	Default &drm_crtc boolean property that notifies the driver that the
> + *	VRR singalling should be disabled in fixed refresh rate scenarios.
> + *	Functionally, psssive vrr works the same as VRR_ENABLED == false

[Severity: Low]
This isn't a bug, but there are a couple of spelling errors in the uAPI
documentation here: "singalling" should be "signalling", and "psssive"
should be "passive".

> + *	but works around displays blanking (mainly HDMI) that do not support
> + *	seamless VRR transitions. Also helps with brightness flickering during
> + *	VRR transitions.
> + *
> + *	Passive VRR mode is not that useful for DP/eDP sinks where seamless VRR
> + *	transitions are enforced by the standard.
>   */
[ ... ]
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c6dbe8b7db9ee..642a374f403e6 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -311,6 +311,15 @@ struct drm_crtc_state {
>  	 */
>  	bool vrr_enabled;
>  
> +	/**
> +	 * @passive_vrr_disabled:
> +	 *
> +	 * Indicates if variable refresh rate on desktop should be enabled for
> +	 * the CRTC. Support for the requested state will depend on driver and
> +	 * hardware capabiltiy - lacking support is not treated as failure.
> +	 */
> +	bool passive_vrr_disabled;
> +

[Severity: Low]
This isn't a bug, but the documentation states this field "Indicates if
variable refresh rate on desktop should be enabled", which contradicts the
variable name passive_vrr_disabled. Should this description read "disabled"
instead?

Also, there is a spelling error in "capabiltiy" (capability).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901191251.2653684-1-jerry.zuo@amd.com?part=1