RE: [PATCH v3 1/6] drm/i915/ddi: change intel_ddi_init_{dp,hdmi}_connector() return type

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

On Fri, 13 Dec 2024, "Kandpal, Suraj" <suraj.kandpal@intel.com> wrote:
>> -----Original Message-----
>> From: Nikula, Jani <jani.nikula@intel.com>
>> Sent: Friday, December 13, 2024 7:46 PM
>> To: intel-gfx@lists.freedesktop.org; intel-xe@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; Sergey Senozhatsky
>> <senozhatsky@chromium.org>; Ville Syrjala <ville.syrjala@linux.intel.com>;
>> Kandpal, Suraj <suraj.kandpal@intel.com>
>> Subject: [PATCH v3 1/6] drm/i915/ddi: change
>> intel_ddi_init_{dp,hdmi}_connector() return type
>>
>> The caller doesn't actually need the returned struct intel_connector; it's
>> stored in the ->attached_connector of intel_dp ad intel_hdmi. Switch to
>
> Typo : *and
> Otherwise LGTM,
> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

Thanks, care to look at patch 4 as well, it's changed slightly?

BR,
Jani.

>
>> returning an int with 0 for success and negative errors codes to be able to
>> indicate success even when we don't have a connector.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_ddi.c | 20 +++++++++-----------
>>  1 file changed, 9 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>> b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index 4f9c50996446..21277cf8afef 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -4542,8 +4542,7 @@ static const struct drm_encoder_funcs
>> intel_ddi_funcs = {
>>       .late_register = intel_ddi_encoder_late_register,  };
>>
>> -static struct intel_connector *
>> -intel_ddi_init_dp_connector(struct intel_digital_port *dig_port)
>> +static int intel_ddi_init_dp_connector(struct intel_digital_port
>> +*dig_port)
>>  {
>>       struct drm_i915_private *i915 = to_i915(dig_port->base.base.dev);
>>       struct intel_connector *connector;
>> @@ -4551,7 +4550,7 @@ intel_ddi_init_dp_connector(struct
>> intel_digital_port *dig_port)
>>
>>       connector = intel_connector_alloc();
>>       if (!connector)
>> -             return NULL;
>> +             return -ENOMEM;
>>
>>       dig_port->dp.output_reg = DDI_BUF_CTL(port);
>>       if (DISPLAY_VER(i915) >= 14)
>> @@ -4566,7 +4565,7 @@ intel_ddi_init_dp_connector(struct
>> intel_digital_port *dig_port)
>>
>>       if (!intel_dp_init_connector(dig_port, connector)) {
>>               kfree(connector);
>> -             return NULL;
>> +             return -EINVAL;
>>       }
>>
>>       if (dig_port->base.type == INTEL_OUTPUT_EDP) { @@ -4582,7
>> +4581,7 @@ intel_ddi_init_dp_connector(struct intel_digital_port *dig_port)
>>               }
>>       }
>>
>> -     return connector;
>> +     return 0;
>>  }
>>
>>  static int intel_hdmi_reset_link(struct intel_encoder *encoder, @@ -4748,20
>> +4747,19 @@ static bool bdw_digital_port_connected(struct intel_encoder
>> *encoder)
>>       return intel_de_read(dev_priv, GEN8_DE_PORT_ISR) & bit;  }
>>
>> -static struct intel_connector *
>> -intel_ddi_init_hdmi_connector(struct intel_digital_port *dig_port)
>> +static int intel_ddi_init_hdmi_connector(struct intel_digital_port
>> +*dig_port)
>>  {
>>       struct intel_connector *connector;
>>       enum port port = dig_port->base.port;
>>
>>       connector = intel_connector_alloc();
>>       if (!connector)
>> -             return NULL;
>> +             return -ENOMEM;
>>
>>       dig_port->hdmi.hdmi_reg = DDI_BUF_CTL(port);
>>       intel_hdmi_init_connector(dig_port, connector);
>>
>> -     return connector;
>> +     return 0;
>>  }
>>
>>  static bool intel_ddi_a_force_4_lanes(struct intel_digital_port *dig_port) @@
>> -5306,7 +5304,7 @@ void intel_ddi_init(struct intel_display *display,
>>       intel_infoframe_init(dig_port);
>>
>>       if (init_dp) {
>> -             if (!intel_ddi_init_dp_connector(dig_port))
>> +             if (intel_ddi_init_dp_connector(dig_port))
>>                       goto err;
>>
>>               dig_port->hpd_pulse = intel_dp_hpd_pulse; @@ -5320,7
>> +5318,7 @@ void intel_ddi_init(struct intel_display *display,
>>        * but leave it just in case we have some really bad VBTs...
>>        */
>>       if (encoder->type != INTEL_OUTPUT_EDP && init_hdmi) {
>> -             if (!intel_ddi_init_hdmi_connector(dig_port))
>> +             if (intel_ddi_init_hdmi_connector(dig_port))
>>                       goto err;
>>       }
>>
>> --
>> 2.39.5
>

-- 
Jani Nikula, Intel