Re: [Intel-gfx] [PATCH v1] drm/i915: Call intel_edp_init_connector only for eDP.

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

On Tue, 11 Feb 2020, "Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com> wrote:
> Well, you can just take all those checks and put them into separate
> function. Something like:
>
> bool intel_dp_supports_mst(intel_dp) {
> 	if (HAS_DP_MST(i915) && !intel_dp_is_edp(intel_dp)) &&
>              !(INTEL_GEN(i915) < 12 && port == PORT_A) &&
>              !(INTEL_GEN(i915) <	11 && port == PORT_E))
> 			return true;
> 	return false;
> }
>
> so, then you would have it nicely looking and understandable:
>
> if (intel_dp_supports_mst(intel_dp))
> 	intel_dp_mst_encoder_init(intel_dig_port,
>                                   intel_connector->base.base.id);
>
> Anyway, I'm _not_ stating that this is _always_ the best way, but 
> I don't see at least any reasons currently why it couldn't be done so.

It's fine, until you realize you need to call a function with the
condition from more than one place, and you need to remember to have the
same conditions in all the places. So the condition is no longer in one
isolated place. It's not like we haven't thought about this before. ;)

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx