| Message ID | 1454668600-22759-4-git-send-email-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2/5/2016 4:06 PM, Jani Nikula wrote: > Hide knowledge about VBT child devices in intel_bios.c. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/intel_bios.c | 33 ++++++++++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_dsi.c | 23 ++++++++++++++--------- > 3 files changed, 47 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 234f33708a31..cd2595c25f8d 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1516,7 +1516,6 @@ struct intel_vbt_data { > > /* MIPI DSI */ > struct { > - u16 port; > u16 panel_id; > struct mipi_config *config; > struct mipi_pps_data *pps; > @@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size); > bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv); > bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin); > bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port); > +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port); > > /* intel_opregion.c */ > #ifdef CONFIG_ACPI > diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c > index 1429d8214885..b0cf33234c33 100644 > --- a/drivers/gpu/drm/i915/intel_bios.c > +++ b/drivers/gpu/drm/i915/intel_bios.c > @@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv, > &&p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) { > DRM_DEBUG_KMS("Found MIPI as LFP\n"); > dev_priv->vbt.has_mipi = 1; > - dev_priv->vbt.dsi.port = p_child->common.dvo_port; > } > > child_dev_ptr = dev_priv->vbt.child_dev + count; > @@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port) > > return false; > } > + > +/** > + * intel_bios_is_dsi_present - is DSI present in VBT > + * @dev_priv: i915 device instance > + * @port: port for DSI if present > + * > + * Return true if DSI is present, and return the port in %port. > + */ > +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, > + enum port *port) > +{ > + union child_device_config *p_child; > + int i; > + > + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) { > + p_child = dev_priv->vbt.child_dev + i; > + > + if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT)) > + continue; > + > + switch (p_child->common.dvo_port) { > + case DVO_PORT_MIPIA: > + case DVO_PORT_MIPIB: > + case DVO_PORT_MIPIC: > + case DVO_PORT_MIPID: > + *port = p_child->common.dvo_port - DVO_PORT_MIPIA; > + return true; > + } > + } can we add platform check & vbt.has_mipi check ? it will avoid the need to loop through the array. Also since all platforms so far (VLV/CHV/BXT) has only MIPI A & MIPI C is B & D needed here ? we can directly fail here for unsupported ports rather than in dsi_init as special check. > + > + return false; > +} > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index 378f879f4015..5b6ec567e3ce 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -1112,9 +1112,15 @@ void intel_dsi_init(struct drm_device *dev) > DRM_DEBUG_KMS("\n"); > > /* There is no detection method for MIPI so rely on VBT */ > - if (!dev_priv->vbt.has_mipi) > + if (!intel_bios_is_dsi_present(dev_priv, &port)) > return; > > + if (port != PORT_A && port != PORT_C) { > + DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n", > + port_name(port)); > + return; > + } > + > if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { > dev_priv->mipi_mmio_base = VLV_MIPI_BASE; > } else { > @@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev) > intel_connector->unregister = intel_connector_unregister; > > /* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */ > - if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) { > - intel_encoder->crtc_mask = (1 << PIPE_A); > - intel_dsi->ports = (1 << PORT_A); > - } else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) { > - intel_encoder->crtc_mask = (1 << PIPE_B); > - intel_dsi->ports = (1 << PORT_C); > - } > + if (port == PORT_A) > + intel_encoder->crtc_mask = 1 << PIPE_A; > + else > + intel_encoder->crtc_mask = 1 << PIPE_B; > The above limitation applies only to CHT/VLV. > if (dev_priv->vbt.dsi.config->dual_link) > - intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C)); > + intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C); > + else > + intel_dsi->ports = 1 << port; > > /* Create a DSI host (and a device) for each port. */ > for_each_dsi_port(port, intel_dsi->ports) {
On Mon, 08 Feb 2016, "Thulasimani, Sivakumar" <sivakumar.thulasimani@intel.com> wrote: > On 2/5/2016 4:06 PM, Jani Nikula wrote: >> Hide knowledge about VBT child devices in intel_bios.c. >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/i915/i915_drv.h | 2 +- >> drivers/gpu/drm/i915/intel_bios.c | 33 ++++++++++++++++++++++++++++++++- >> drivers/gpu/drm/i915/intel_dsi.c | 23 ++++++++++++++--------- >> 3 files changed, 47 insertions(+), 11 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 234f33708a31..cd2595c25f8d 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -1516,7 +1516,6 @@ struct intel_vbt_data { >> >> /* MIPI DSI */ >> struct { >> - u16 port; >> u16 panel_id; >> struct mipi_config *config; >> struct mipi_pps_data *pps; >> @@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size); >> bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv); >> bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin); >> bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port); >> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port); >> >> /* intel_opregion.c */ >> #ifdef CONFIG_ACPI >> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c >> index 1429d8214885..b0cf33234c33 100644 >> --- a/drivers/gpu/drm/i915/intel_bios.c >> +++ b/drivers/gpu/drm/i915/intel_bios.c >> @@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv, >> &&p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) { >> DRM_DEBUG_KMS("Found MIPI as LFP\n"); >> dev_priv->vbt.has_mipi = 1; >> - dev_priv->vbt.dsi.port = p_child->common.dvo_port; >> } >> >> child_dev_ptr = dev_priv->vbt.child_dev + count; >> @@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port) >> >> return false; >> } >> + >> +/** >> + * intel_bios_is_dsi_present - is DSI present in VBT >> + * @dev_priv: i915 device instance >> + * @port: port for DSI if present >> + * >> + * Return true if DSI is present, and return the port in %port. >> + */ >> +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, >> + enum port *port) >> +{ >> + union child_device_config *p_child; >> + int i; >> + >> + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) { >> + p_child = dev_priv->vbt.child_dev + i; >> + >> + if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT)) >> + continue; >> + >> + switch (p_child->common.dvo_port) { >> + case DVO_PORT_MIPIA: >> + case DVO_PORT_MIPIB: >> + case DVO_PORT_MIPIC: >> + case DVO_PORT_MIPID: >> + *port = p_child->common.dvo_port - DVO_PORT_MIPIA; >> + return true; >> + } >> + } > can we add platform check & vbt.has_mipi check ? it will avoid the need > to loop through the array. I'm actually going to remove has_mipi altogether as unnecessary duplication of information. We run the loop very few times anyway. Also, I don't want to duplicate the information about platforms, that stuff is in intel_setup_outputs and in the encoder inits. Single point of truth. > Also since all platforms so far (VLV/CHV/BXT) has only MIPI A & MIPI C > is B & D needed here ? we can directly fail here for unsupported ports > rather than in dsi_init as special check. Agreed. >> + >> + return false; >> +} >> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c >> index 378f879f4015..5b6ec567e3ce 100644 >> --- a/drivers/gpu/drm/i915/intel_dsi.c >> +++ b/drivers/gpu/drm/i915/intel_dsi.c >> @@ -1112,9 +1112,15 @@ void intel_dsi_init(struct drm_device *dev) >> DRM_DEBUG_KMS("\n"); >> >> /* There is no detection method for MIPI so rely on VBT */ >> - if (!dev_priv->vbt.has_mipi) >> + if (!intel_bios_is_dsi_present(dev_priv, &port)) >> return; >> >> + if (port != PORT_A && port != PORT_C) { >> + DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n", >> + port_name(port)); >> + return; >> + } >> + >> if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { >> dev_priv->mipi_mmio_base = VLV_MIPI_BASE; >> } else { >> @@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev) >> intel_connector->unregister = intel_connector_unregister; >> >> /* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */ >> - if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) { >> - intel_encoder->crtc_mask = (1 << PIPE_A); >> - intel_dsi->ports = (1 << PORT_A); >> - } else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) { >> - intel_encoder->crtc_mask = (1 << PIPE_B); >> - intel_dsi->ports = (1 << PORT_C); >> - } >> + if (port == PORT_A) >> + intel_encoder->crtc_mask = 1 << PIPE_A; >> + else >> + intel_encoder->crtc_mask = 1 << PIPE_B; >> > The above limitation applies only to CHT/VLV. True, but I'm again reluctant to make the change at this point. It should be added as part of BXT DSI instead. BR, Jani. >> if (dev_priv->vbt.dsi.config->dual_link) >> - intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C)); >> + intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C); >> + else >> + intel_dsi->ports = 1 << port; >> >> /* Create a DSI host (and a device) for each port. */ >> for_each_dsi_port(port, intel_dsi->ports) { > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 234f33708a31..cd2595c25f8d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1516,7 +1516,6 @@ struct intel_vbt_data { /* MIPI DSI */ struct { - u16 port; u16 panel_id; struct mipi_config *config; struct mipi_pps_data *pps; @@ -3400,6 +3399,7 @@ bool intel_bios_is_valid_vbt(const void *buf, size_t size); bool intel_bios_is_tv_present(struct drm_i915_private *dev_priv); bool intel_bios_is_lvds_present(struct drm_i915_private *dev_priv, u8 *i2c_pin); bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port); +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, enum port *port); /* intel_opregion.c */ #ifdef CONFIG_ACPI diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index 1429d8214885..b0cf33234c33 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -1237,7 +1237,6 @@ parse_device_mapping(struct drm_i915_private *dev_priv, &&p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT) { DRM_DEBUG_KMS("Found MIPI as LFP\n"); dev_priv->vbt.has_mipi = 1; - dev_priv->vbt.dsi.port = p_child->common.dvo_port; } child_dev_ptr = dev_priv->vbt.child_dev + count; @@ -1552,3 +1551,35 @@ bool intel_bios_is_port_edp(struct drm_i915_private *dev_priv, enum port port) return false; } + +/** + * intel_bios_is_dsi_present - is DSI present in VBT + * @dev_priv: i915 device instance + * @port: port for DSI if present + * + * Return true if DSI is present, and return the port in %port. + */ +bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv, + enum port *port) +{ + union child_device_config *p_child; + int i; + + for (i = 0; i < dev_priv->vbt.child_dev_num; i++) { + p_child = dev_priv->vbt.child_dev + i; + + if (!(p_child->common.device_type & DEVICE_TYPE_MIPI_OUTPUT)) + continue; + + switch (p_child->common.dvo_port) { + case DVO_PORT_MIPIA: + case DVO_PORT_MIPIB: + case DVO_PORT_MIPIC: + case DVO_PORT_MIPID: + *port = p_child->common.dvo_port - DVO_PORT_MIPIA; + return true; + } + } + + return false; +} diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index 378f879f4015..5b6ec567e3ce 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -1112,9 +1112,15 @@ void intel_dsi_init(struct drm_device *dev) DRM_DEBUG_KMS("\n"); /* There is no detection method for MIPI so rely on VBT */ - if (!dev_priv->vbt.has_mipi) + if (!intel_bios_is_dsi_present(dev_priv, &port)) return; + if (port != PORT_A && port != PORT_C) { + DRM_DEBUG_KMS("VBT has unsupported DSI port %c\n", + port_name(port)); + return; + } + if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { dev_priv->mipi_mmio_base = VLV_MIPI_BASE; } else { @@ -1153,16 +1159,15 @@ void intel_dsi_init(struct drm_device *dev) intel_connector->unregister = intel_connector_unregister; /* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */ - if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) { - intel_encoder->crtc_mask = (1 << PIPE_A); - intel_dsi->ports = (1 << PORT_A); - } else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) { - intel_encoder->crtc_mask = (1 << PIPE_B); - intel_dsi->ports = (1 << PORT_C); - } + if (port == PORT_A) + intel_encoder->crtc_mask = 1 << PIPE_A; + else + intel_encoder->crtc_mask = 1 << PIPE_B; if (dev_priv->vbt.dsi.config->dual_link) - intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C)); + intel_dsi->ports = (1 << PORT_A) | (1 << PORT_C); + else + intel_dsi->ports = 1 << port; /* Create a DSI host (and a device) for each port. */ for_each_dsi_port(port, intel_dsi->ports) {
Hide knowledge about VBT child devices in intel_bios.c. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_bios.c | 33 ++++++++++++++++++++++++++++++++- drivers/gpu/drm/i915/intel_dsi.c | 23 ++++++++++++++--------- 3 files changed, 47 insertions(+), 11 deletions(-)