| Message ID | 20230208015508.24824-10-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Prep work for vbt.ports[] nukage | expand |
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[]. > > 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; > } > > /**
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. Should we maybe do that here too, or should we just think about fully exorcising invalid ports from the child device list? > > > > > 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
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(). 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
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
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; } /**