Re: [PATCH v4 5/5] drm/i915/display: log eDP TC support and DBuf overlap detection state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] drm/i915: commit message claims alphabetical order but implementation does not
--

commit a46ad637c40a9e4f034c8da54ce1e88270a8428d
Author: Michał Grzelak <michal.grzelak@linux.intel.com>

drm/i915/display: log eDP TC support and DBuf overlap detection state

This commit adds logging for eDP Type-C support and DBuf overlap detection
state in the display runtime info. It also attempts to reorder the fields
in the structure to maintain alphabetical order.

>     edp_tc_support and dbuf_overlap_detection are only fields of display's
>     runtime info which state is not logged. Show it along other fields.

[Severity: Low]
This isn't a bug, but there's a typo in the commit message. The commit message
refers to edp_tc_support, but the actual field in the code is named
edp_typec_support.

>     While at it, move dbuf_overlap_detection to have display runtime info's
>     fields in alphabetical order.

[Severity: Low]
The commit message states the fields are moved to be in alphabetical order,
but the resulting order in both the struct definition and the print statements
is not alphabetical.

> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> @@ -2005,6 +2005,9 @@ void intel_display_device_info_print(const struct intel_display_device_info *inf
>  	DEV_INFO_DISPLAY_FOR_EACH_FLAG(PRINT_FLAG);
>  #undef PRINT_FLAG
>  
> +	drm_printf(p, "edp_typec_support: %s\n", str_yes_no(runtime->edp_typec_support));
> +	drm_printf(p, "has_dbuf_overlap_detection: %s\n", str_yes_no(runtime->has_dbuf_overlap_detection));
> +
>  	drm_printf(p, "has_3dlut: %s\n", str_yes_no(runtime->has_3dlut));

[Severity: Low]
Should these print statements be sorted alphabetically to match the claim in
the commit message? Currently, edp_typec_support and 
has_dbuf_overlap_detection are placed before has_3dlut and has_hdcp, which 
breaks alphabetical ordering in intel_display_device_info_print().

[ ... ]

> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -307,10 +307,10 @@ struct intel_display_runtime_info {
>  
>  	bool has_3dlut;
>  	bool has_hdcp;
> +	bool has_dbuf_overlap_detection;
>  	bool has_dmc;
>  	bool has_dsc;
>  	bool edp_typec_support;
> -	bool has_dbuf_overlap_detection;
>  };

[Severity: Low]
Does this achieve the alphabetical ordering mentioned in the commit message?
The sequence in intel_display_runtime_info is now:

has_3dlut
has_hdcp
has_dbuf_overlap_detection
has_dmc
has_dsc
edp_typec_support

The has_dbuf_overlap_detection field is placed after has_hdcp, and
edp_typec_support is placed at the end.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013204705.1574475-1-michal.grzelak@linux.intel.com?part=5