On Thu, 2020-01-16 at 17:58 -0800, José Roberto de Souza wrote:
> This is a eDP function and it will always returns true for non-eDP
> ports.
>
> Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 4074d83b1a5f..a50b5b6dd009 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -7537,7 +7537,6 @@ intel_dp_init_connector(struct
> intel_digital_port *intel_dig_port,
>
> if (!intel_edp_init_connector(intel_dp, intel_connector)) {
> intel_dp_aux_fini(intel_dp);
> - intel_dp_mst_encoder_cleanup(intel_dig_port);
> goto fail;
> }
>
Change looks fine for me(anyway better than now).
But:
This whole thing looks kind of confusing to me. Why we are even calling
intel_edp_init_connector for
non-eDP ports, just to immediately get true returned? So returning
success means either success or that this is non-eDP..
This confuses the caller, that we have actually successfully
initialized eDP, while actually this also means here that it is not
eDP.
Why we can't just do it like:
if (intel_dp_is_edp(intel_dp)) {
if (!intel_edp_init_connector(intel_dp, intel_connector)) {
intel_dp_aux_fini(intel_dp);
goto fail;
}
}
it looks much more understandable and less confusing, i.e eDP functions
are only called for eDP and no return value hacks are needed.
Reviewed-by: Stanislav Lisovskiy <stanislav.lisovskiy@xxxxxxxxx>
Stan
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
