On Tue, 12 Aug 2025, Dibin Moolakadan Subrahmanian 
<dibin.moolakadan.subrahman...@intel.com> wrote:
> On 29-07-2025 16:47, Jani Nikula wrote:
>> Add a common allocator function for struct intel_digital_port, with some
>> member default initialization to deduplicate them from everywhere
>> else. This is similar to intel_connector_alloc().
>>
>> At least for now, place this in intel_encoder.[ch]. We don't have a
>> dedicated file for dig port stuff, and there wouldn't be much to add
>> there anyway. A digital port is a sort of subclass of encoder, so the
>> location isn't far off the mark.
>>
>> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
>
> Looks good to me.
> Reviewed-by: Dibin Moolakadan Subrahmanian 
> <dibin.moolakadan.subrahman...@intel.com>

Thanks for the review, pushed to drm-intel-next.

BR,
Jani.

>
>> ---
>>   drivers/gpu/drm/i915/display/g4x_dp.c        |  7 +------
>>   drivers/gpu/drm/i915/display/g4x_hdmi.c      |  9 ++-------
>>   drivers/gpu/drm/i915/display/intel_ddi.c     |  8 +-------
>>   drivers/gpu/drm/i915/display/intel_encoder.c | 18 ++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_encoder.h |  3 +++
>>   5 files changed, 25 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/g4x_dp.c 
>> b/drivers/gpu/drm/i915/display/g4x_dp.c
>> index 846dbd8ae931..aa159f9ce12f 100644
>> --- a/drivers/gpu/drm/i915/display/g4x_dp.c
>> +++ b/drivers/gpu/drm/i915/display/g4x_dp.c
>> @@ -1300,12 +1300,10 @@ bool g4x_dp_init(struct intel_display *display,
>>              drm_dbg_kms(display->drm, "No VBT child device for DP-%c\n",
>>                          port_name(port));
>>   
>> -    dig_port = kzalloc(sizeof(*dig_port), GFP_KERNEL);
>> +    dig_port = intel_dig_port_alloc();
>>      if (!dig_port)
>>              return false;
>>   
>> -    dig_port->aux_ch = AUX_CH_NONE;
>> -
>>      intel_connector = intel_connector_alloc();
>>      if (!intel_connector)
>>              goto err_connector_alloc;
>> @@ -1315,8 +1313,6 @@ bool g4x_dp_init(struct intel_display *display,
>>   
>>      intel_encoder->devdata = devdata;
>>   
>> -    mutex_init(&dig_port->hdcp.mutex);
>> -
>>      if (drm_encoder_init(display->drm, &intel_encoder->base,
>>                           &intel_dp_enc_funcs, DRM_MODE_ENCODER_TMDS,
>>                           "DP %c", port_name(port)))
>> @@ -1386,7 +1382,6 @@ bool g4x_dp_init(struct intel_display *display,
>>      }
>>   
>>      dig_port->dp.output_reg = output_reg;
>> -    dig_port->max_lanes = 4;
>>   
>>      intel_encoder->type = INTEL_OUTPUT_DP;
>>      intel_encoder->power_domain = 
>> intel_display_power_ddi_lanes_domain(display, port);
>> diff --git a/drivers/gpu/drm/i915/display/g4x_hdmi.c 
>> b/drivers/gpu/drm/i915/display/g4x_hdmi.c
>> index 2610f5702fb9..108ebd97f9e4 100644
>> --- a/drivers/gpu/drm/i915/display/g4x_hdmi.c
>> +++ b/drivers/gpu/drm/i915/display/g4x_hdmi.c
>> @@ -19,6 +19,7 @@
>>   #include "intel_display_types.h"
>>   #include "intel_dp_aux.h"
>>   #include "intel_dpio_phy.h"
>> +#include "intel_encoder.h"
>>   #include "intel_fdi.h"
>>   #include "intel_fifo_underrun.h"
>>   #include "intel_hdmi.h"
>> @@ -690,12 +691,10 @@ bool g4x_hdmi_init(struct intel_display *display,
>>              drm_dbg_kms(display->drm, "No VBT child device for HDMI-%c\n",
>>                          port_name(port));
>>   
>> -    dig_port = kzalloc(sizeof(*dig_port), GFP_KERNEL);
>> +    dig_port = intel_dig_port_alloc();
>>      if (!dig_port)
>>              return false;
>>   
>> -    dig_port->aux_ch = AUX_CH_NONE;
>> -
>>      intel_connector = intel_connector_alloc();
>>      if (!intel_connector)
>>              goto err_connector_alloc;
>> @@ -704,8 +703,6 @@ bool g4x_hdmi_init(struct intel_display *display,
>>   
>>      intel_encoder->devdata = devdata;
>>   
>> -    mutex_init(&dig_port->hdcp.mutex);
>> -
>>      if (drm_encoder_init(display->drm, &intel_encoder->base,
>>                           &intel_hdmi_enc_funcs, DRM_MODE_ENCODER_TMDS,
>>                           "HDMI %c", port_name(port)))
>> @@ -767,8 +764,6 @@ bool g4x_hdmi_init(struct intel_display *display,
>>              intel_encoder->cloneable |= BIT(INTEL_OUTPUT_HDMI);
>>   
>>      dig_port->hdmi.hdmi_reg = hdmi_reg;
>> -    dig_port->dp.output_reg = INVALID_MMIO_REG;
>> -    dig_port->max_lanes = 4;
>>   
>>      intel_infoframe_init(dig_port);
>>   
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c 
>> b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index 0405396c7750..403249345f12 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -5148,12 +5148,10 @@ void intel_ddi_init(struct intel_display *display,
>>                          phy_name(phy));
>>      }
>>   
>> -    dig_port = kzalloc(sizeof(*dig_port), GFP_KERNEL);
>> +    dig_port = intel_dig_port_alloc();
>>      if (!dig_port)
>>              return;
>>   
>> -    dig_port->aux_ch = AUX_CH_NONE;
>> -
>>      encoder = &dig_port->base;
>>      encoder->devdata = devdata;
>>   
>> @@ -5191,9 +5189,6 @@ void intel_ddi_init(struct intel_display *display,
>>   
>>      intel_encoder_link_check_init(encoder, intel_ddi_link_check);
>>   
>> -    mutex_init(&dig_port->hdcp.mutex);
>> -    dig_port->hdcp.num_streams = 0;
>> -
>>      encoder->hotplug = intel_ddi_hotplug;
>>      encoder->compute_output_type = intel_ddi_compute_output_type;
>>      encoder->compute_config = intel_ddi_compute_config;
>> @@ -5331,7 +5326,6 @@ void intel_ddi_init(struct intel_display *display,
>>   
>>      dig_port->ddi_a_4_lanes = DISPLAY_VER(display) < 11 && ddi_buf_ctl & 
>> DDI_A_4_LANES;
>>   
>> -    dig_port->dp.output_reg = INVALID_MMIO_REG;
>>      dig_port->max_lanes = intel_ddi_max_lanes(dig_port);
>>   
>>      if (need_aux_ch(encoder, init_dp)) {
>> diff --git a/drivers/gpu/drm/i915/display/intel_encoder.c 
>> b/drivers/gpu/drm/i915/display/intel_encoder.c
>> index 0b7bd26f4339..f9ce81320d68 100644
>> --- a/drivers/gpu/drm/i915/display/intel_encoder.c
>> +++ b/drivers/gpu/drm/i915/display/intel_encoder.c
>> @@ -80,3 +80,21 @@ void intel_encoder_shutdown_all(struct intel_display 
>> *display)
>>              if (encoder->shutdown_complete)
>>                      encoder->shutdown_complete(encoder);
>>   }
>> +
>> +struct intel_digital_port *intel_dig_port_alloc(void)
>> +{
>> +    struct intel_digital_port *dig_port;
>> +
>> +    dig_port = kzalloc(sizeof(*dig_port), GFP_KERNEL);
>> +    if (!dig_port)
>> +            return NULL;
>> +
>> +    dig_port->hdmi.hdmi_reg = INVALID_MMIO_REG;
>> +    dig_port->dp.output_reg = INVALID_MMIO_REG;
>> +    dig_port->aux_ch = AUX_CH_NONE;
>> +    dig_port->max_lanes = 4;
>> +
>> +    mutex_init(&dig_port->hdcp.mutex);
>> +
>> +    return dig_port;
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_encoder.h 
>> b/drivers/gpu/drm/i915/display/intel_encoder.h
>> index 3fa5589f0b1c..65a9c521f8a6 100644
>> --- a/drivers/gpu/drm/i915/display/intel_encoder.h
>> +++ b/drivers/gpu/drm/i915/display/intel_encoder.h
>> @@ -6,6 +6,7 @@
>>   #ifndef __INTEL_ENCODER_H__
>>   #define __INTEL_ENCODER_H__
>>   
>> +struct intel_digital_port;
>>   struct intel_display;
>>   struct intel_encoder;
>>   
>> @@ -17,4 +18,6 @@ void intel_encoder_link_check_flush_work(struct 
>> intel_encoder *encoder);
>>   void intel_encoder_suspend_all(struct intel_display *display);
>>   void intel_encoder_shutdown_all(struct intel_display *display);
>>   
>> +struct intel_digital_port *intel_dig_port_alloc(void);
>> +
>>   #endif /* __INTEL_ENCODER_H__ */

-- 
Jani Nikula, Intel

Reply via email to