On Wed, Mar 25, 2015 at 06:45:58PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Recent BSW VBT has a VBT child device size 37 bytes instead of the 33
> bytes our code assumes. This means we fail to parse the VBT and thus
> fail to detect eDP ports properly and just register them as DP ports
> instead.
>
> Fix it up by using the reported child device size from the VBT instead
> of assuming it matches out struct defintions.
>
> The latest spec I have shows that the child device size should be 36
> bytes for rev >= 195, however on my BSW the size is actually 37 bytes.
> And our current struct definition is 33 bytes.
>
> Feels like the entire VBT parses would need to be rewritten to handle
> changes in the layout better, but for now I've decided to do just the
> bare minimum to get my eDP port back.
>
> Cc: Vijay Purushothaman <vijay.a.purushothaman@xxxxxxxxxxxxxxx>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
Two minor comments, but, in any case, this is:
Reviewed-by: Damien Lespiau <damien.lespiau@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_bios.c | 26 +++++++++++++-------------
> drivers/gpu/drm/i915/intel_bios.h | 4 ++--
> 2 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c
> index c684085..4cbd325 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -447,6 +447,12 @@ parse_general_definitions(struct drm_i915_private *dev_priv,
> }
> }
>
> +static union child_device_config *
> +child_device_ptr(struct bdb_general_definitions *p_defs, int i)
> +{
> + return (void *) &p_defs->devices[i * p_defs->child_dev_size];
> +}
We could use a child_device_num() helper as well.
> static void
> parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
> struct bdb_header *bdb)
> @@ -476,10 +482,10 @@ parse_sdvo_device_mapping(struct drm_i915_private *dev_priv,
> block_size = get_blocksize(p_defs);
> /* get the number of child device */
> child_device_num = (block_size - sizeof(*p_defs)) /
> - sizeof(*p_child);
> + p_defs->child_dev_size;
> count = 0;
> for (i = 0; i < child_device_num; i++) {
> - p_child = &(p_defs->devices[i]);
> + p_child = child_device_ptr(p_defs, i);
> if (!p_child->old.device_type) {
> /* skip the device block if device type is invalid */
> continue;
> @@ -1067,25 +1073,19 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> DRM_DEBUG_KMS("No general definition block is found, no devices defined.\n");
> return;
> }
> - /* judge whether the size of child device meets the requirements.
> - * If the child device size obtained from general definition block
> - * is different with sizeof(struct child_device_config), skip the
> - * parsing of sdvo device info
> - */
> - if (p_defs->child_dev_size != sizeof(*p_child)) {
> - /* different child dev size . Ignore it */
> - DRM_DEBUG_KMS("different child size is found. Invalid.\n");
> + if (p_defs->child_dev_size < sizeof(*p_child)) {
> + DRM_ERROR("General definiton block child device size is too small.\n");
definition
> return;
> }
> /* get the block size of general definitions */
> block_size = get_blocksize(p_defs);
> /* get the number of child device */
> child_device_num = (block_size - sizeof(*p_defs)) /
> - sizeof(*p_child);
> + p_defs->child_dev_size;
> count = 0;
> /* get the number of child device that is present */
> for (i = 0; i < child_device_num; i++) {
> - p_child = &(p_defs->devices[i]);
> + p_child = child_device_ptr(p_defs, i);
> if (!p_child->common.device_type) {
> /* skip the device block if device type is invalid */
> continue;
> @@ -1105,7 +1105,7 @@ parse_device_mapping(struct drm_i915_private *dev_priv,
> dev_priv->vbt.child_dev_num = count;
> count = 0;
> for (i = 0; i < child_device_num; i++) {
> - p_child = &(p_defs->devices[i]);
> + p_child = child_device_ptr(p_defs, i);
> if (!p_child->common.device_type) {
> /* skip the device block if device type is invalid */
> continue;
> diff --git a/drivers/gpu/drm/i915/intel_bios.h b/drivers/gpu/drm/i915/intel_bios.h
> index 6afd5be..af0b476 100644
> --- a/drivers/gpu/drm/i915/intel_bios.h
> +++ b/drivers/gpu/drm/i915/intel_bios.h
> @@ -277,9 +277,9 @@ struct bdb_general_definitions {
> * And the device num is related with the size of general definition
> * block. It is obtained by using the following formula:
> * number = (block_size - sizeof(bdb_general_definitions))/
> - * sizeof(child_device_config);
> + * defs->child_dev_size;
> */
> - union child_device_config devices[0];
> + uint8_t devices[0];
> } __packed;
>
> /* Mask for DRRS / Panel Channel / SSC / BLT control bits extraction */
> --
> 2.0.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
