[Intel-gfx] [PATCH v2] drm/i915/vbt: Handle generic DTD block
Jani Nikula
jani.nikula at intel.com
Tue Oct 29 07:10:26 UTC 2019
On Mon, 28 Oct 2019, Matt Roper <matthew.d.roper at 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
>
> Bspec: 54751
> Bspec: 20148
> Cc: Jani Nikula <jani.nikula at intel.com>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_bios.c | 87 ++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_vbt_defs.h | 31 +++++++
> 2 files changed, 115 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index 63c1bd4c2954..0d1504e23a49 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -202,6 +202,72 @@ get_lvds_fp_timing(const struct bdb_header *bdb,
> return (const struct lvds_fp_timing *)((const u8 *)bdb + ofs);
> }
>
> +static struct drm_display_mode *
> +parse_generic_dtd(struct drm_i915_private *dev_priv,
> + const struct bdb_generic_dtd *generic_dtd)
> +{
> + const struct bdb_generic_dtd_entry *dtd;
> + struct drm_display_mode *panel_fixed_mode;
> + int num_dtd;
> +
> + if (generic_dtd->gdtd_size < sizeof(struct bdb_generic_dtd_entry)) {
> + DRM_ERROR("GDTD size %u is too small.\n",
> + generic_dtd->gdtd_size);
> + return NULL;
I think in the future we may want to copy this to an allocated dtd entry
with zero padding so we don't have to care in the parser. But this can
be like this for now.
> + } else if (generic_dtd->gdtd_size !=
> + sizeof(struct bdb_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 NULL;
> + }
> +
> + dtd = &generic_dtd->dtd[dev_priv->vbt.panel_type];
> +
> + panel_fixed_mode = kzalloc(sizeof(*panel_fixed_mode), GFP_KERNEL);
> + if (!panel_fixed_mode)
> + return NULL;
> +
> + 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_polarity)
> + panel_fixed_mode->flags |= DRM_MODE_FLAG_PHSYNC;
> + else
> + panel_fixed_mode->flags |= DRM_MODE_FLAG_NHSYNC;
> +
> + if (dtd->vsync_polarity)
> + panel_fixed_mode->flags |= DRM_MODE_FLAG_PVSYNC;
> + else
> + panel_fixed_mode->flags |= DRM_MODE_FLAG_NVSYNC;
> +
> + return panel_fixed_mode;
> +}
> +
> /* Try to find integrated panel data */
> static void
> parse_lfp_panel_data(struct drm_i915_private *dev_priv,
> @@ -210,6 +276,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
> const struct bdb_lvds_options *lvds_options;
> const struct bdb_lvds_lfp_data *lvds_lfp_data;
> const struct bdb_lvds_lfp_data_ptrs *lvds_lfp_data_ptrs;
> + const struct bdb_generic_dtd *generic_dtd;
> const struct lvds_dvo_timing *panel_dvo_timing;
> const struct lvds_fp_timing *fp_timing;
> struct drm_display_mode *panel_fixed_mode;
> @@ -262,6 +329,18 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
> break;
> }
>
> + if (bdb->version >= 229) {
> + generic_dtd = find_section(bdb, BDB_GENERIC_DTD);
> + if (!generic_dtd)
> + return;
> +
> + panel_fixed_mode = parse_generic_dtd(dev_priv, generic_dtd);
> + if (!panel_fixed_mode)
> + return;
Knowing how it is with VBTs... maybe we should fall back to the old ways
in both error paths?
I'm also wondering about splitting this at a higher level for clarity
instead of shoving this here. Call the parser for each bdb block at the
top level? Handle the case for legacy conditional to the fixed mode
already being there?
> +
> + goto skip_legacy_lfp;
> + }
> +
> lvds_lfp_data = find_section(bdb, BDB_LVDS_LFP_DATA);
> if (!lvds_lfp_data)
> return;
> @@ -282,9 +361,6 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
>
> dev_priv->vbt.lfp_lvds_vbt_mode = panel_fixed_mode;
I think you'll want to set this for the generic dtd too. ;)
>
> - DRM_DEBUG_KMS("Found panel mode in BIOS VBT tables:\n");
> - drm_mode_debug_printmodeline(panel_fixed_mode);
> -
> fp_timing = get_lvds_fp_timing(bdb, lvds_lfp_data,
> lvds_lfp_data_ptrs,
> panel_type);
> @@ -297,6 +373,11 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv,
> dev_priv->vbt.bios_lvds_val);
> }
> }
> +
> +skip_legacy_lfp:
> + DRM_DEBUG_KMS("Found panel mode in BIOS VBT tables:\n");
> + drm_mode_debug_printmodeline(panel_fixed_mode);
> +
> }
>
> static void
> diff --git a/drivers/gpu/drm/i915/display/intel_vbt_defs.h b/drivers/gpu/drm/i915/display/intel_vbt_defs.h
> index 69a7cb1fa121..82fcab0b3752 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 bdb_generic_dtd_entry {
I think I wanted to restrict the bdb_ prefix to actual top level bdb
blocks.
> + 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_polarity:1;
> + u8 hsync_polarity:1;
> +
> + u32 rsvd:24;
> +} __packed;
> +
> +struct bdb_generic_dtd {
> + u16 gdtd_size;
> + struct bdb_generic_dtd_entry dtd[0]; /* up to 24 DTD's */
I think [] without the 0 is the standard way nowadays.
> +} __packed;
> +
> #endif /* _INTEL_VBT_DEFS_H_ */
--
Jani Nikula, Intel Open Source Graphics Center
More information about the Intel-gfx
mailing list