Re: [PATCH v10 67/69] drm/connector: Drop redundant hdmi vendor/product fields

[Cristian Ciocaltea <cristian.ciocaltea@collabora.com>]

On 8/20/26 1:10 PM, Maxime Ripard wrote:
> On Fri, Jul 31, 2026 at 07:20:14PM +0300, Cristian Ciocaltea wrote:
>> Now that all users migrated to the new drmm_connector_hdmi_init()
>> signature, vendor and product are provided through struct
>> drm_connector_hdmi_funcs, a reference to which is already stored in
>> drm_connector_hdmi.
>>
>> Drop the redundant fields from drm_connector_hdmi and point its users to
>> hdmi.funcs->vendor and hdmi.funcs->product instead.
>>
>> This allows simplifying the related connector registration tests by
>> getting rid of the now unnecessary KUNIT_EXPECT_MEMEQ() checks.
>>
>> Tested-by: Diederik de Haas <diederik@cknow-tech.com>  # NanoPC-T6 LTS, Rock 5B
>> Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
>> ---
>>  drivers/gpu/drm/display/drm_hdmi_state_helper.c |  4 +--
>>  drivers/gpu/drm/drm_connector.c                 |  4 ---
>>  drivers/gpu/drm/tests/drm_connector_test.c      | 41 +++----------------------
>>  include/drm/drm_connector.h                     | 14 ++-------
>>  4 files changed, 8 insertions(+), 55 deletions(-)
>>
[...]

>>  /*
>>   * Test that the registration of a connector with a vendor name at the
>> - * maximum length succeeds, and is stored padded without the trailing
>> - * zero.
>> + * maximum length succeeds.
>>   */
>>  static void drm_test_connector_hdmi_init_vendor_length_exact(struct kunit *test)
>>  {
>>  	struct drm_connector_init_priv *priv = test->priv;
>> -	const char expected_vendor[DRM_CONNECTOR_HDMI_VENDOR_LEN] = {
>> -		'V', 'e', 'n', 'd', 'o', 'r',
>> -		'V', 'e',
>> -	};
>>  	int ret;
>>  
>>  	priv->hdmi_funcs = dummy_hdmi_funcs;
>> @@ -911,10 +882,6 @@ static void drm_test_connector_hdmi_init_vendor_length_exact(struct kunit *test)
>>  				       DRM_MODE_CONNECTOR_HDMIA,
>>  				       &priv->ddc);
>>  	KUNIT_EXPECT_EQ(test, ret, 0);
>> -	KUNIT_EXPECT_MEMEQ(test,
>> -			   priv->connector.hdmi.vendor,
>> -			   expected_vendor,
>> -			   sizeof(priv->connector.hdmi.vendor));
>>  }
> 
> Unfortunately, these tests were useful, and are there to match what the
> spec asks for.

I've just added a new test to cover this, as well as a couple of prerequisites
to consolidate SPD InfoFrame handling:

* video/hdmi: Define SPD InfoFrame field lengths and use strtomem_pad()

  HDMI specification defines the SPD InfoFrame Vendor Name and Product
  Description as fixed-size fields, 8 and 16 bytes respectively, padded
  with zeros and left without any trailing NUL when a name spans the whole
  field.

  Give those lengths a name and mark the fields as non-strings, so that
  the copies can be handed over to strtomem_pad(), which implements
  precisely the required semantics.  This also bounds the reads from the
  source strings, whereas the open-coded strlen() could run past the end
  of the buffer in the hdmi_spd_infoframe_unpack() path, where the names
  come straight from the wire and are not NUL-terminated.

  While at it, replace the related magic numbers in the pack and unpack
  helpers with the new defines.

* drm/connector: Use the SPD InfoFrame field length defines

  DRM_CONNECTOR_HDMI_{VENDOR,PRODUCT}_LEN used to size the vendor and
  product arrays in struct drm_connector_hdmi.  Those arrays are gone and
  both names are now only validated before being copied into the SPD
  InfoFrame, hence the limits they have to be checked against are the ones
  of the SPD InfoFrame fields themselves.

  Switch the remaining users over to
  HDMI_SPD_INFOFRAME_{VENDOR,PRODUCT}_LEN and drop the DRM specific
  defines, so that the two cannot drift apart.

* drm/tests: hdmi: Add SPD InfoFrame vendor/product coverage

  The vendor and product strings provided through struct
  drm_connector_hdmi_funcs end up in the SPD InfoFrame, whose fields are
  defined by the HDMI specification as fixed-size: 8 bytes for the vendor
  name and 16 bytes for the product description, padded with zeros and
  left without any trailing NUL when a name spans the whole field.

  Nothing exercises that so far, since the SPD InfoFrame is only generated
  for connectors implementing the related hooks, which none of the
  existing test funcs provides.

  Add a connector variant supplying those hooks, along with parametrized
  tests covering both the padded and the exact length cases.

Thanks,
Cristian