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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 11 Feb 2020, "Lisovskiy, Stanislav" <stanislav.lisovskiy@xxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux