[Intel-gfx] [PATCH v2 09/11] drm/i915: Turn intel_digital_port_connected() in a vfunc

[Ville Syrjälä]

On Tue, Jan 28, 2020 at 11:06:42AM +0200, Jani Nikula wrote:
> On Tue, 21 Jan 2020, Ville Syrjala <ville.syrjala at linux.intel.com> wrote:
> > diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> > index 3da166054788..cf0df6f18734 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dp.h
> > +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> > @@ -116,6 +116,12 @@ void intel_dp_vsc_enable(struct intel_dp *intel_dp,
> >  void intel_dp_hdr_metadata_enable(struct intel_dp *intel_dp,
> >  				  const struct intel_crtc_state *crtc_state,
> >  				  const struct drm_connector_state *conn_state);
> > +bool cpt_digital_port_connected(struct intel_encoder *encoder);
> > +bool spt_digital_port_connected(struct intel_encoder *encoder);
> > +bool ivb_digital_port_connected(struct intel_encoder *encoder);
> > +bool bdw_digital_port_connected(struct intel_encoder *encoder);
> > +bool bxt_digital_port_connected(struct intel_encoder *encoder);
> > +bool icp_digital_port_connected(struct intel_encoder *encoder);
> 
> Please find a way to not expose a plethora of platform specific
> functions from files. I want to take things to the completely opposite
> direction.

Many of these do disappear in the next patch. Still, I was letting
this idea simmer a bit in case I'd actually come up with a good
approach, alas no inspiration has occurred. I think we have three
options:

1) keep everyting hidden in intel_dp.c and expose some kind of
   intel_dig_port_init() thing that we call from the ddi code.
   The reason I don't really like this is I've been thinking of
   stuffing all the function pointers into some const vtables
   which would probably be mostly populated by stuff from
   intel_ddi.c. So we'd end up in the exact opposite situation
   where ddi code would have to exposes functions to intel_dp.c.

2) Store the hpd register in intel_encoder/dig_port/etc. so we could
   unify the ilk/ivb/bdw/pch functions into a single function. But
   since we still need special sauce for the TC ports we'd still want
   the vfunc. And since we do have the vfunc storing the hpd register
   offset feels rather redundant.

3) Duplicate two of the functions in intel_ddi.c:
   ivb->hsw and pch->lpt (or whatever we call it), and move
   the bdw function over entirely. The duplication seems a
   bit silly, but might actually be the least annoying approach.

-- 
Ville Syrjälä
Intel