[igt-dev] [PATCH i-g-t 5/5] lib/igt_kms: use igt_edid to generate a 4K EDID
Ser, Simon
simon.ser at intel.com
Wed Jul 3 17:00:25 UTC 2019
On Wed, 2019-07-03 at 15:58 +0300, Ville Syrjälä wrote:
> On Wed, Jul 03, 2019 at 08:24:02AM +0000, Ser, Simon wrote:
> > Thanks for your reviews, these are good points!
> >
> > Replies inline.
> >
> > On Tue, 2019-07-02 at 18:51 +0300, Ville Syrjälä wrote:
> > > On Tue, Jul 02, 2019 at 03:50:38PM +0300, Simon Ser wrote:
> > > > The new EDID has been byte-by-byte checked to be exactly the same as before.
> > > >
> > > > Signed-off-by: Simon Ser <simon.ser at intel.com>
> > > > ---
> > > > lib/igt_kms.c | 107 ++++++++++++++++++++----------------
> > > > lib/igt_kms.h | 2 +-
> > > > lib/tests/igt_edid.c | 1 +
> > > > lib/tests/igt_hdmi_inject.c | 1 -
> > > > tests/kms_hdmi_inject.c | 9 +--
> > > > 5 files changed, 64 insertions(+), 56 deletions(-)
> > > >
> > > > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > > > index 45c79a380daf..444605e4b110 100644
> > > > --- a/lib/igt_kms.c
> > > > +++ b/lib/igt_kms.c
> > > > @@ -284,6 +284,66 @@ const unsigned char *igt_kms_get_dp_audio_edid(void)
> > > > return raw_edid;
> > > > }
> > > >
> > > > +static const uint8_t edid_4k_svds[] = {
> > > > + 32 | CEA_SVD_NATIVE, /* 1080p @ 24Hz (native) */
> > > > + 5, /* 1080i @ 60Hz */
> > > > + 20, /* 1080i @ 50Hz */
> > > > + 4, /* 720p @ 60Hz */
> > > > + 19, /* 720p @ 50Hz */
> > > > +};
> > > > +
> > > > +const unsigned char *igt_kms_get_4k_edid(void)
> > > > +{
> > > > + static unsigned char raw_edid[256];
> > > > + struct edid *edid;
> > > > + struct edid_ext *edid_ext;
> > > > + struct edid_cea *edid_cea;
> > > > + char *cea_data;
> > > > + struct edid_cea_data_block *block;
> > > > + char raw_hdmi[HDMI_VSD_MIN_SIZE + 6] = {0};
> > > > + struct hdmi_vsd *hdmi;
> > > > + size_t cea_data_size = 0;
> > > > + size_t svds_len;
> > > > +
> > > > + /* Create a new EDID from the base IGT EDID, and add an
> > > > + * extension that advertises 4K support. */
> > > > + edid = (struct edid *) raw_edid;
> > > > + memcpy(edid, igt_kms_get_base_edid(), sizeof(struct edid));
> > >
> > > That only contains the base block. Are we leaving stack garbage in the
> > > extension block?
> >
> > raw_edid is zero-initialized. edid is just a pointer to raw_edid.
>
> Ah, I failed to notice the static.
Hmm, I guess I should add {0} just for good measure.
> > > > + edid->extensions_len = 1;
> > > > + edid_ext = &edid->extensions[0];
> > > > + edid_cea = &edid_ext->data.cea;
> > > > + cea_data = edid_cea->data;
> > > > +
> > > > + /* Short Video Descriptor */
> > > > + block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> > > > + svds_len = sizeof(edid_4k_svds) / sizeof(edid_4k_svds[0]);
> > >
> > > I think we have an ARRAY_SIZE(). Although since we know these are bytes
> > > maybe just a sizeof() would be good enough.
> >
> > Fair
> >
> > > > + cea_data_size += edid_cea_data_block_set_svd(block, edid_4k_svds,
> > > > + svds_len);
> > > > +
> > > > + /* Vendor Specific Data block */
> > > > + hdmi = (struct hdmi_vsd *) raw_hdmi;
> > > > + hdmi->src_phy_addr[0] = 0x10;
> > > > + hdmi->src_phy_addr[1] = 0x00;
> > > > + hdmi->flags1 = 0;
> > > > + hdmi->max_tdms_clock = 0;
> > > > + hdmi->flags2 = HDMI_VSD_VIDEO_PRESENT;
> > > > + hdmi->data[0] = 0x00; /* HDMI video flags */
> > > > + hdmi->data[1] = 1 << 5; /* 1 VIC entry, 0 3D entries */
> > > > + hdmi->data[2] = 0x01; /* 2160p, specified as short descriptor */
> > > > +
> > > > + block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> > > > + cea_data_size += edid_cea_data_block_set_hdmi_vsd(block, hdmi,
> > > > + sizeof(raw_hdmi));
> > > > +
> > > > + assert(cea_data_size <= sizeof(edid_cea->data));
> > > > +
> > > > + edid_ext_set_cea(edid_ext, cea_data_size, 0, 0);
> > > > +
> > > > + edid_update_checksum(edid);
> > > > + edid_ext_update_cea_checksum(edid_ext);
> > > > + return raw_edid;
> > > > +}
> > > > +
> > > > const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
> > > > [IGT_PLANE_SRC_X] = "SRC_X",
> > > > [IGT_PLANE_SRC_Y] = "SRC_Y",
> > > > @@ -1348,8 +1408,6 @@ struct edid_block {
> > > > unsigned char *data;
> > > > };
> > > >
> > > > -#define DTD_SUPPORTS_AUDIO 1<<6
> > > > -
> > > > static struct edid_block
> > > > init_cea_block(const unsigned char *edid, size_t length,
> > > > unsigned char *new_edid_ptr[], size_t *new_length,
> > > > @@ -1437,51 +1495,6 @@ void kmstest_edid_add_3d(const unsigned char *edid, size_t length,
> > > > update_edid_csum(new_edid.data, length);
> > > > }
> > > >
> > > > -/**
> > > > - * kmstest_edid_add_4k:
> > > > - * @edid: an existing valid edid block
> > > > - * @length: length of @edid
> > > > - * @new_edid_ptr: pointer to where the new edid will be placed
> > > > - * @new_length: pointer to the size of the new edid
> > > > - *
> > > > - * Makes a copy of an existing edid block and adds an extension indicating
> > > > - * a HDMI 4K mode in vsdb.
> > > > - */
> > > > -void kmstest_edid_add_4k(const unsigned char *edid, size_t length,
> > > > - unsigned char *new_edid_ptr[], size_t *new_length)
> > > > -{
> > > > - char vsdb_block_len = 12;
> > > > - struct edid_block new_edid = init_cea_block(edid, length, new_edid_ptr,
> > > > - new_length, vsdb_block_len,
> > > > - 0);
> > > > - int pos = new_edid.pos;
> > > > -
> > > > - /* vsdb block ( id | length ) */
> > > > - new_edid.data[pos++] = 3 << 5 | (vsdb_block_len - 1);
> > > > - /* registration id */
> > > > - new_edid.data[pos++] = 0x3;
> > > > - new_edid.data[pos++] = 0xc;
> > > > - new_edid.data[pos++] = 0x0;
> > > > - /* source physical address */
> > > > - new_edid.data[pos++] = 0x10;
> > > > - new_edid.data[pos++] = 0x00;
> > > > - /* Supports_AI ... etc */
> > > > - new_edid.data[pos++] = 0x00;
> > > > - /* Max TMDS Clock */
> > > > - new_edid.data[pos++] = 0x00;
> > > > - /* Latency present, HDMI Video Present */
> > > > - new_edid.data[pos++] = 0x20;
> > > > - /* HDMI Video */
> > > > - new_edid.data[pos++] = 0x00; /* 3D present */
> > > > -
> > > > - /* HDMI MODE LEN -- how many entries */
> > > > - new_edid.data[pos++] = 0x20;
> > > > - /* 2160p, specified as short descriptor */
> > > > - new_edid.data[pos++] = 0x01;
> > > > -
> > > > - update_edid_csum(new_edid.data, length);
> > > > -}
> > > > -
> > > > /**
> > > > * kmstest_unset_all_crtcs:
> > > > * @drm_fd: the DRM fd
> > > > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > > > index f72508640712..203719878d86 100644
> > > > --- a/lib/igt_kms.h
> > > > +++ b/lib/igt_kms.h
> > > > @@ -195,7 +195,6 @@ enum intel_broadcast_rgb_mode {
> > > > bool kmstest_force_connector(int fd, drmModeConnector *connector,
> > > > enum kmstest_force_connector_state state);
> > > > void kmstest_edid_add_3d(const unsigned char *edid, size_t length, unsigned char *new_edid_ptr[], size_t *new_length);
> > > > -void kmstest_edid_add_4k(const unsigned char *edid, size_t length, unsigned char *new_edid_ptr[], size_t *new_length);
> > > > void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
> > > > const unsigned char *edid);
> > > >
> > > > @@ -763,6 +762,7 @@ const unsigned char *igt_kms_get_base_edid(void);
> > > > const unsigned char *igt_kms_get_alt_edid(void);
> > > > const unsigned char *igt_kms_get_hdmi_audio_edid(void);
> > > > const unsigned char *igt_kms_get_dp_audio_edid(void);
> > > > +const unsigned char *igt_kms_get_4k_edid(void);
> > > >
> > > > struct udev_monitor *igt_watch_hotplug(void);
> > > > bool igt_hotplug_detected(struct udev_monitor *mon,
> > > > diff --git a/lib/tests/igt_edid.c b/lib/tests/igt_edid.c
> > > > index a847df272525..1a78b38a945b 100644
> > > > --- a/lib/tests/igt_edid.c
> > > > +++ b/lib/tests/igt_edid.c
> > > > @@ -76,6 +76,7 @@ igt_simple_main
> > > > { "base", igt_kms_get_base_edid, 0 },
> > > > { "alt", igt_kms_get_alt_edid, 0 },
> > > > { "hdmi_audio", igt_kms_get_hdmi_audio_edid, 1 },
> > > > + { "4k", igt_kms_get_4k_edid, 1 },
> > > > {0},
> > > > }, *f;
> > > > const unsigned char *edid;
> > > > diff --git a/lib/tests/igt_hdmi_inject.c b/lib/tests/igt_hdmi_inject.c
> > > > index 2534b1a23acb..c70c3195cb1d 100644
> > > > --- a/lib/tests/igt_hdmi_inject.c
> > > > +++ b/lib/tests/igt_hdmi_inject.c
> > > > @@ -72,7 +72,6 @@ igt_simple_main
> > > > hdmi_inject_func inject;
> > > > } funcs[] = {
> > > > { "3D", kmstest_edid_add_3d },
> > > > - { "4k", kmstest_edid_add_4k },
> > > > { NULL, NULL },
> > > > }, *f;
> > > >
> > > > diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> > > > index 9a968fa9e50e..60198f529817 100644
> > > > --- a/tests/kms_hdmi_inject.c
> > > > +++ b/tests/kms_hdmi_inject.c
> > > > @@ -76,8 +76,7 @@ get_connector(int drm_fd, drmModeRes *res)
> > > > static void
> > > > hdmi_inject_4k(int drm_fd, drmModeConnector *connector)
> > > > {
> > > > - unsigned char *edid;
> > > > - size_t length;
> > > > + const unsigned char *edid;
> > > > struct kmstest_connector_config config;
> > > > int ret, cid, i, crtc_mask = -1;
> > > > int fb_id;
> > > > @@ -90,9 +89,7 @@ hdmi_inject_4k(int drm_fd, drmModeConnector *connector)
> > > > /* 4K requires at least HSW */
> > > > igt_require(IS_HASWELL(devid) || intel_gen(devid) >= 8);
> > > >
> > > > - kmstest_edid_add_4k(igt_kms_get_base_edid(), EDID_LENGTH, &edid,
> > > > - &length);
> > > > -
> > > > + edid = igt_kms_get_4k_edid();
> > > > kmstest_force_edid(drm_fd, connector, edid);
> > > >
> > > > if (!kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_ON))
> > > > @@ -135,8 +132,6 @@ hdmi_inject_4k(int drm_fd, drmModeConnector *connector)
> > > >
> > > > kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_UNSPECIFIED);
> > > > kmstest_force_edid(drm_fd, connector, NULL);
> > > > -
> > > > - free(edid);
> > > > }
> > > >
> > > > static void
> > > > --
> > > > 2.22.0
> > > >
> > > > _______________________________________________
> > > > igt-dev mailing list
> > > > igt-dev at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
More information about the igt-dev
mailing list