Re: [Intel-gfx] [PATCH 09/10] drm/i915: Iterate all child devs in intel_bios_is_port_present()

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Feb 13, 2023 at 06:41:18PM +0200, Jani Nikula wrote:
> On Mon, 13 Feb 2023, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Mon, Feb 13, 2023 at 06:08:50PM +0200, Jani Nikula wrote:
> >> On Wed, 08 Feb 2023, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> >> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> >
> >> > Instead of consulting vbt.ports[] lets just go through the
> >> > whole child device list to check whether a specific port
> >> > was declared by the VBT or not.
> >> 
> >> Might want to mention that this does not impact the dupe checking even
> >> if we look at display_devices instead of vbt.ports[].
> >
> > Hmm, except for the !is_port_valid() check.
> 
> D'oh!
> 
> > Should we
> > maybe do that here too, or should we just think about 
> > fully exorcising invalid ports from the child device list?
> 
> Maybe the first step that fits in this series is just including the
> check here too?
> 
> Otherwise, I'm divided. I kind of always wanted the VBT parsing part to
> be just that, parsing what's in the VBT, and only include checks/filters
> that ensure it's internally consistent. And the consumers of the data
> would cross check against platforms etc.
> 
> But I guess the VBT data itself has been filled with platform specific
> details, so *shrug*. I could be persuaded either way.
> 
> I guess there could be a separate filtering step. I'd hate to add
> filtering to parse_general_definitions().

Yeah, parse->filter->consume might be the right option eventually.

I've been also pondering about deleting the child device from
the list if we fail to probe it (eg. in the case of these 
phantom eDPs). But haven't yet really thought through the
implications of keep vs. delete.

> 
> BR,
> Jani.
> 
> 
> >
> >> 
> >> >
> >> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> > ---
> >> >  drivers/gpu/drm/i915/display/intel_bios.c | 11 ++++++++++-
> >> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> >> > index efe33af2259b..1af175b48ae6 100644
> >> > --- a/drivers/gpu/drm/i915/display/intel_bios.c
> >> > +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> >> > @@ -3398,10 +3398,19 @@ bool intel_bios_is_lvds_present(struct drm_i915_private *i915, u8 *i2c_pin)
> >> >   */
> >> >  bool intel_bios_is_port_present(struct drm_i915_private *i915, enum port port)
> >> >  {
> >> > +	const struct intel_bios_encoder_data *devdata;
> >> > +
> >> >  	if (WARN_ON(!has_ddi_port_info(i915)))
> >> >  		return true;
> >> >  
> >> > -	return i915->display.vbt.ports[port];
> >> > +	list_for_each_entry(devdata, &i915->display.vbt.display_devices, node) {
> >> > +		const struct child_device_config *child = &devdata->child;
> >> > +
> >> > +		if (dvo_port_to_port(i915, child->dvo_port) == port)
> >> > +			return true;
> >> > +	}
> >> > +
> >> > +	return false;
> >> >  }
> >> >  
> >> >  /**
> >> 
> >> -- 
> >> Jani Nikula, Intel Open Source Graphics Center
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

-- 
Ville Syrjälä
Intel