Re: [PATCH] drm/i915/ddi: abstract figuring out encoder name

[Jani Nikula <jani.nikula@intel.com>]

On Wed, 03 Sep 2025, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
> On 9/3/2025 3:40 PM, Jani Nikula wrote:
>> The encoder name deduction has become a bit cumbersome within
>> intel_ddi_init(). Split it out to a separate function to declutter
>> intel_ddi_init(), even if that means having to use a temp seq buffer for
>> the name.
>>
>> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> LGTM.
>
> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

Thanks, pushed to din.

BR,
Jani.

>
>> ---
>>   drivers/gpu/drm/i915/display/intel_ddi.c | 69 +++++++++++++-----------
>>   1 file changed, 38 insertions(+), 31 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index 4e4ea3a0ff83..68e049ad042b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -26,6 +26,7 @@
>>    */
>>   
>>   #include <linux/iopoll.h>
>> +#include <linux/seq_buf.h>
>>   #include <linux/string_helpers.h>
>>   
>>   #include <drm/display/drm_dp_helper.h>
>> @@ -5077,11 +5078,45 @@ static bool port_in_use(struct intel_display *display, enum port port)
>>   	return false;
>>   }
>>   
>> +static const char *intel_ddi_encoder_name(struct intel_display *display,
>> +					  enum port port, enum phy phy,
>> +					  struct seq_buf *s)
>> +{
>> +	if (DISPLAY_VER(display) >= 13 && port >= PORT_D_XELPD) {
>> +		seq_buf_printf(s, "DDI %c/PHY %c",
>> +			       port_name(port - PORT_D_XELPD + PORT_D),
>> +			       phy_name(phy));
>> +	} else if (DISPLAY_VER(display) >= 12) {
>> +		enum tc_port tc_port = intel_port_to_tc(display, port);
>> +
>> +		seq_buf_printf(s, "DDI %s%c/PHY %s%c",
>> +			       port >= PORT_TC1 ? "TC" : "",
>> +			       port >= PORT_TC1 ? port_tc_name(port) : port_name(port),
>> +			       tc_port != TC_PORT_NONE ? "TC" : "",
>> +			       tc_port != TC_PORT_NONE ? tc_port_name(tc_port) : phy_name(phy));
>> +	} else if (DISPLAY_VER(display) >= 11) {
>> +		enum tc_port tc_port = intel_port_to_tc(display, port);
>> +
>> +		seq_buf_printf(s, "DDI %c%s/PHY %s%c",
>> +			       port_name(port),
>> +			       port >= PORT_C ? " (TC)" : "",
>> +			       tc_port != TC_PORT_NONE ? "TC" : "",
>> +			       tc_port != TC_PORT_NONE ? tc_port_name(tc_port) : phy_name(phy));
>> +	} else {
>> +		seq_buf_printf(s, "DDI %c/PHY %c", port_name(port),  phy_name(phy));
>> +	}
>> +
>> +	drm_WARN_ON(display->drm, seq_buf_has_overflowed(s));
>> +
>> +	return seq_buf_str(s);
>> +}
>> +
>>   void intel_ddi_init(struct intel_display *display,
>>   		    const struct intel_bios_encoder_data *devdata)
>>   {
>>   	struct intel_digital_port *dig_port;
>>   	struct intel_encoder *encoder;
>> +	DECLARE_SEQ_BUF(encoder_name, 20);
>>   	bool init_hdmi, init_dp;
>>   	enum port port;
>>   	enum phy phy;
>> @@ -5166,37 +5201,9 @@ void intel_ddi_init(struct intel_display *display,
>>   	encoder = &dig_port->base;
>>   	encoder->devdata = devdata;
>>   
>> -	if (DISPLAY_VER(display) >= 13 && port >= PORT_D_XELPD) {
>> -		drm_encoder_init(display->drm, &encoder->base, &intel_ddi_funcs,
>> -				 DRM_MODE_ENCODER_TMDS,
>> -				 "DDI %c/PHY %c",
>> -				 port_name(port - PORT_D_XELPD + PORT_D),
>> -				 phy_name(phy));
>> -	} else if (DISPLAY_VER(display) >= 12) {
>> -		enum tc_port tc_port = intel_port_to_tc(display, port);
>> -
>> -		drm_encoder_init(display->drm, &encoder->base, &intel_ddi_funcs,
>> -				 DRM_MODE_ENCODER_TMDS,
>> -				 "DDI %s%c/PHY %s%c",
>> -				 port >= PORT_TC1 ? "TC" : "",
>> -				 port >= PORT_TC1 ? port_tc_name(port) : port_name(port),
>> -				 tc_port != TC_PORT_NONE ? "TC" : "",
>> -				 tc_port != TC_PORT_NONE ? tc_port_name(tc_port) : phy_name(phy));
>> -	} else if (DISPLAY_VER(display) >= 11) {
>> -		enum tc_port tc_port = intel_port_to_tc(display, port);
>> -
>> -		drm_encoder_init(display->drm, &encoder->base, &intel_ddi_funcs,
>> -				 DRM_MODE_ENCODER_TMDS,
>> -				 "DDI %c%s/PHY %s%c",
>> -				 port_name(port),
>> -				 port >= PORT_C ? " (TC)" : "",
>> -				 tc_port != TC_PORT_NONE ? "TC" : "",
>> -				 tc_port != TC_PORT_NONE ? tc_port_name(tc_port) : phy_name(phy));
>> -	} else {
>> -		drm_encoder_init(display->drm, &encoder->base, &intel_ddi_funcs,
>> -				 DRM_MODE_ENCODER_TMDS,
>> -				 "DDI %c/PHY %c", port_name(port), phy_name(phy));
>> -	}
>> +	drm_encoder_init(display->drm, &encoder->base, &intel_ddi_funcs,
>> +			 DRM_MODE_ENCODER_TMDS, "%s",
>> +			 intel_ddi_encoder_name(display, port, phy, &encoder_name));
>>   
>>   	intel_encoder_link_check_init(encoder, intel_ddi_link_check);
>>   

-- 
Jani Nikula, Intel