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

["Lisovskiy, Stanislav" <stanislav.lisovskiy@intel.com>]

On Tue, 2020-02-11 at 16:12 +0200, Jani Nikula wrote:
> 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.
> ;)

Well, that is why I didn't say that this is always a best approach. :)

Sure, if I had to do lots of calls of this function(even though this
should be reviewed then, why we have to do it in a multiple places),
I would then just put this check and init together into some helper
function.

Stan

> 
> BR,
> Jani.
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx