Re: [PATCH 1/7] drm/i915: add lspcon vbt bit parsing

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 22 Mar 2016, Shashank Sharma <shashank.sharma@intel.com> wrote:
> LSPCON can be configured on a port using VBT entry.
> This patch adds code to parse VBT and detect presence of
> LSPCON for a ddi port.
>
> Signed-off-by: Akashdeep Sharma <akashdeep.sharma@intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>  drivers/gpu/drm/i915/intel_bios.c     | 42 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_vbt_defs.h |  1 +
>  3 files changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f330a53..cbd40de 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3373,6 +3373,7 @@ 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);
> +bool intel_bios_is_lspcon_preset(struct drm_device *dev, 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 083003b..a04ab5c 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -1588,3 +1588,45 @@ bool intel_bios_is_dsi_present(struct drm_i915_private *dev_priv,
>  
>  	return false;
>  }
> +

Please add kernel-doc comment like all the other intel_bios_is_*
functions have.

> +bool
> +intel_bios_is_lspcon_preset(struct drm_device *dev, enum port port)

You mean is *present*?

Please pass struct drm_i915_private *dev_priv instead of dev.

> +{
> +	unsigned char i;
> +	enum port dvo_port = 0;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	/* LSPCON is supported only for GEN 9 */
> +	if (!IS_GEN9(dev))
> +		return false;
> +
> +	/* Check if lspcon is supported in VBT */
> +	for (i = 0; i < dev_priv->vbt.child_dev_num; i++) {
> +		if (!dev_priv->vbt.child_dev[i].common.lspcon)

I think to be defensive we should also check for bdb version >=
192. That we could check before the loop along with the gen check.

> +			continue;
> +
> +		switch (dev_priv->vbt.child_dev[i].common.dvo_port) {
> +		case DVO_PORT_DPB:
> +			dvo_port = PORT_B;
> +			break;
> +
> +		case DVO_PORT_DPC:
> +			dvo_port = PORT_C;
> +			break;
> +
> +		case DVO_PORT_DPD:
> +			dvo_port = PORT_D;
> +			break;
> +
> +		default:
> +			continue;
> +		}
> +
> +		if (dvo_port == port) {
> +			DRM_DEBUG_DRIVER("LSPCON configured on port %c\n",
> +			port_name(port));
> +			return true;
> +		}
> +	}
> +	return false;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_vbt_defs.h b/drivers/gpu/drm/i915/intel_vbt_defs.h
> index 749dcea..0066b24 100644
> --- a/drivers/gpu/drm/i915/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/intel_vbt_defs.h
> @@ -276,6 +276,7 @@ struct common_child_dev_config {
>  	u8 flags_1;
>  	u8 not_common3[13];
>  	u8 iboost_level;
> +	u8 lspcon:1;

Huh? AFAICT from the spec, lspcon is bit 2 in flags_1. You could define
flags_1 in terms of bitfields, including IBOOST_ENABLE.

>  } __packed;

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx