Re: [CI v5 2/2] drm/i915/vbt: Handle generic DTD block

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

On Fri, 15 Nov 2019, Matt Roper <matthew.d.roper@intel.com> wrote:
> VBT revision 229 adds a new "Generic DTD" block 58 and deprecates the
> old LFP panel mode data in block 42.  Let's start parsing this block to
> fill in the panel fixed mode on devices with a >=229 VBT.
>
> v2:
>  * Update according to the recent updates:
>     - DTD size is now 16 bits instead of 24
>     - polarity is now just a single bit for hsync and vsync and is
>       properly documented
>  * Minor checkpatch fix
>
> v3:
>  * Now that panel options are parsed separately from the previous patch,
>    move generic DTD parsing into a function parallel to
>    parse_lfp_panel_dtd.  We'll still fall back to looking at the legacy
>    LVDS timing block if the generic DTD fails.  (Jani)
>  * Don't forget to actually set lfp_lvds_vbt_mode!  (Jani)
>  * Drop "bdb_" prefix from dtd entry structure.  (Jani)
>  * Follow C99 standard for structure's flexible array member.  (Jani)
>
> v4:
>  * Add "positive" to polarity field names for clarity.  (Jani)
>  * Move VBT version check and fallback to legacy DTD parsing logic to a
>    helper to keep top-level VBT parsing uncluttered.  (Jani)
>  * Restructure reserved bit packing at end of generic_dtd_entry from
>    "u32 rsvd:24" to "u8 rsvd[3]" to prevent copy/paste mistakes in the
>    future.  (Jani)

Thanks, looks nice.

BR,
Jani.

>
> Bspec: 54751
> Bspec: 20148
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bios.c     | 96 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_vbt_defs.h | 31 ++++++
>  2 files changed, 125 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index d13ce0b7db8b..f6a9a5ccb556 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -296,7 +296,7 @@ parse_lfp_panel_dtd(struct drm_i915_private *dev_priv,
>  
>  	dev_priv->vbt.lfp_lvds_vbt_mode = panel_fixed_mode;
>  
> -	DRM_DEBUG_KMS("Found panel mode in BIOS VBT tables:\n");
> +	DRM_DEBUG_KMS("Found panel mode in BIOS VBT legacy lfp table:\n");
>  	drm_mode_debug_printmodeline(panel_fixed_mode);
>  
>  	fp_timing = get_lvds_fp_timing(bdb, lvds_lfp_data,
> @@ -313,6 +313,98 @@ parse_lfp_panel_dtd(struct drm_i915_private *dev_priv,
>  	}
>  }
>  
> +static void
> +parse_generic_dtd(struct drm_i915_private *dev_priv,
> +		  const struct bdb_header *bdb)
> +{
> +	const struct bdb_generic_dtd *generic_dtd;
> +	const struct generic_dtd_entry *dtd;
> +	struct drm_display_mode *panel_fixed_mode;
> +	int num_dtd;
> +
> +	generic_dtd = find_section(bdb, BDB_GENERIC_DTD);
> +	if (!generic_dtd)
> +		return;
> +
> +	if (generic_dtd->gdtd_size < sizeof(struct generic_dtd_entry)) {
> +		DRM_ERROR("GDTD size %u is too small.\n",
> +			  generic_dtd->gdtd_size);
> +		return;
> +	} else if (generic_dtd->gdtd_size !=
> +		   sizeof(struct generic_dtd_entry)) {
> +		DRM_ERROR("Unexpected GDTD size %u\n", generic_dtd->gdtd_size);
> +		/* DTD has unknown fields, but keep going */
> +	}
> +
> +	num_dtd = (get_blocksize(generic_dtd) -
> +		   sizeof(struct bdb_generic_dtd)) / generic_dtd->gdtd_size;
> +	if (dev_priv->vbt.panel_type > num_dtd) {
> +		DRM_ERROR("Panel type %d not found in table of %d DTD's\n",
> +			  dev_priv->vbt.panel_type, num_dtd);
> +		return;
> +	}
> +
> +	dtd = &generic_dtd->dtd[dev_priv->vbt.panel_type];
> +
> +	panel_fixed_mode = kzalloc(sizeof(*panel_fixed_mode), GFP_KERNEL);
> +	if (!panel_fixed_mode)
> +		return;
> +
> +	panel_fixed_mode->hdisplay = dtd->hactive;
> +	panel_fixed_mode->hsync_start =
> +		panel_fixed_mode->hdisplay + dtd->hfront_porch;
> +	panel_fixed_mode->hsync_end =
> +		panel_fixed_mode->hsync_start + dtd->hsync;
> +	panel_fixed_mode->htotal = panel_fixed_mode->hsync_end;
> +
> +	panel_fixed_mode->vdisplay = dtd->vactive;
> +	panel_fixed_mode->vsync_start =
> +		panel_fixed_mode->vdisplay + dtd->vfront_porch;
> +	panel_fixed_mode->vsync_end =
> +		panel_fixed_mode->vsync_start + dtd->vsync;
> +	panel_fixed_mode->vtotal = panel_fixed_mode->vsync_end;
> +
> +	panel_fixed_mode->clock = dtd->pixel_clock;
> +	panel_fixed_mode->width_mm = dtd->width_mm;
> +	panel_fixed_mode->height_mm = dtd->height_mm;
> +
> +	panel_fixed_mode->type = DRM_MODE_TYPE_PREFERRED;
> +	drm_mode_set_name(panel_fixed_mode);
> +
> +	if (dtd->hsync_positive_polarity)
> +		panel_fixed_mode->flags |= DRM_MODE_FLAG_PHSYNC;
> +	else
> +		panel_fixed_mode->flags |= DRM_MODE_FLAG_NHSYNC;
> +
> +	if (dtd->vsync_positive_polarity)
> +		panel_fixed_mode->flags |= DRM_MODE_FLAG_PVSYNC;
> +	else
> +		panel_fixed_mode->flags |= DRM_MODE_FLAG_NVSYNC;
> +
> +	DRM_DEBUG_KMS("Found panel mode in BIOS VBT generic dtd table:\n");
> +	drm_mode_debug_printmodeline(panel_fixed_mode);
> +
> +	dev_priv->vbt.lfp_lvds_vbt_mode = panel_fixed_mode;
> +}
> +
> +static void
> +parse_panel_dtd(struct drm_i915_private *dev_priv,
> +		const struct bdb_header *bdb)
> +{
> +	/*
> +	 * Older VBTs provided provided DTD information for internal displays
> +	 * through the "LFP panel DTD" block (42).  As of VBT revision 229,
> +	 * that block is now deprecated and DTD information should be provided
> +	 * via a newer "generic DTD" block (58).  Just to be safe, we'll
> +	 * try the new generic DTD block first on VBT >= 229, but still fall
> +	 * back to trying the old LFP block if that fails.
> +	 */
> +	if (bdb->version >= 229)
> +		parse_generic_dtd(dev_priv, bdb);
> +	if (!dev_priv->vbt.lfp_lvds_vbt_mode)
> +		parse_lfp_panel_dtd(dev_priv, bdb);
> +}
> +
>  static void
>  parse_lfp_backlight(struct drm_i915_private *dev_priv,
>  		    const struct bdb_header *bdb)
> @@ -1877,7 +1969,7 @@ void intel_bios_init(struct drm_i915_private *dev_priv)
>  	parse_general_features(dev_priv, bdb);
>  	parse_general_definitions(dev_priv, bdb);
>  	parse_panel_options(dev_priv, bdb);
> -	parse_lfp_panel_dtd(dev_priv, bdb);
> +	parse_panel_dtd(dev_priv, bdb);
>  	parse_lfp_backlight(dev_priv, bdb);
>  	parse_sdvo_panel_data(dev_priv, bdb);
>  	parse_driver_features(dev_priv, bdb);
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index 69a7cb1fa121..f0338da3a82a 100644
> --- a/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> +++ b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> @@ -115,6 +115,7 @@ enum bdb_block_id {
>  	BDB_MIPI_CONFIG			= 52,
>  	BDB_MIPI_SEQUENCE		= 53,
>  	BDB_COMPRESSION_PARAMETERS	= 56,
> +	BDB_GENERIC_DTD			= 58,
>  	BDB_SKIP			= 254, /* VBIOS private block, ignore */
>  };
>  
> @@ -863,4 +864,34 @@ struct bdb_compression_parameters {
>  	struct dsc_compression_parameters_entry data[16];
>  } __packed;
>  
> +/*
> + * Block 58 - Generic DTD Block
> + */
> +
> +struct generic_dtd_entry {
> +	u32 pixel_clock;
> +	u16 hactive;
> +	u16 hblank;
> +	u16 hfront_porch;
> +	u16 hsync;
> +	u16 vactive;
> +	u16 vblank;
> +	u16 vfront_porch;
> +	u16 vsync;
> +	u16 width_mm;
> +	u16 height_mm;
> +
> +	/* Flags */
> +	u8 rsvd_flags:6;
> +	u8 vsync_positive_polarity:1;
> +	u8 hsync_positive_polarity:1;
> +
> +	u8 rsvd[3];
> +} __packed;
> +
> +struct bdb_generic_dtd {
> +	u16 gdtd_size;
> +	struct generic_dtd_entry dtd[];	/* up to 24 DTD's */
> +} __packed;
> +
>  #endif /* _INTEL_VBT_DEFS_H_ */

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