[igt-dev] [PATCH i-g-t] tools/amd_hdmi_compliance_yuv420: Test 4K video modes with YUV encoding
Kazlauskas, Nicholas
Nicholas.Kazlauskas at amd.com
Wed Sep 11 14:29:51 UTC 2019
On 2019-09-11 10:11 a.m., Wang, Chao-kai (Stylon) wrote:
> From: Stylon Wang <stylon.wang at amd.com>
>
> HDMI 2.0 compliance tests needs 4K modes with YUV encoding.
> This test selects 4K modes based on specified VIC.
It helps to note what changed in a patch series in both the patch title
and in the commit message here.
You can add a v2 to the patch header by appending a -v2 to your command, eg:
"git send-email -v2 ..."
And in the commit description you should do something like:
v2:
- example change 1
- example change 2
Other than that, I do have some general comments and nitpicks:
I think this tool might be better off named as just amd_hdmi_compliance
so we can expand on this later.
Nicholas Kazlauskas
>
> Signed-off-by: Stylon Wang <stylon.wang at amd.com>
> ---
> tools/Makefile.am | 3 +
> tools/Makefile.sources | 4 +
> tools/amd_hdmi_compliance_yuv420.c | 193 +++++++++++++++++++++++++++++
> tools/meson.build | 5 +
> 4 files changed, 205 insertions(+)
> create mode 100644 tools/amd_hdmi_compliance_yuv420.c
>
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 4f54720f..1a23e078 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -11,6 +11,9 @@ bin_PROGRAMS += intel_dp_compliance
> intel_dp_compliance_CFLAGS = $(AM_CFLAGS) $(GLIB_CFLAGS)
> intel_dp_compliance_LDADD = $(top_builddir)/lib/libintel_tools.la
>
> +bin_PROGRAMS += amd_hdmi_compliance_yuv420
> +amd_hdmi_compliance_yuv420_CFLAGS = $(AM_CFLAGS) $(GLIB_CFLAGS)
> +
> SUBDIRS = null_state_gen registers
>
> AM_CPPFLAGS = -I$(top_srcdir) -I$(top_srcdir)/include/drm-uapi -I$(top_srcdir)/lib \
> diff --git a/tools/Makefile.sources b/tools/Makefile.sources
> index 50706f41..8196cb59 100644
> --- a/tools/Makefile.sources
> +++ b/tools/Makefile.sources
> @@ -66,3 +66,7 @@ intel_dp_compliance_SOURCES = \
> intel_dp_compliance_hotplug.c \
> $(NULL)
>
> +amd_hdmi_compliance_yuv420_SOURCES = \
> + amd_hdmi_compliance_yuv420.c \
> + $(NULL)
> +
> diff --git a/tools/amd_hdmi_compliance_yuv420.c b/tools/amd_hdmi_compliance_yuv420.c
> new file mode 100644
> index 00000000..5f5eae2c
> --- /dev/null
> +++ b/tools/amd_hdmi_compliance_yuv420.c
> @@ -0,0 +1,193 @@
> +/*
> + * Copyright 2019 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "igt.h"
> +
> +/* Common test data */
> +typedef struct data {
> + struct igt_fb pattern_fb_info;
> + int fd;
> + igt_display_t display;
> + igt_plane_t *primary;
> + igt_output_t *output;
> + igt_pipe_t *pipe;
> + enum pipe pipe_id;
> + bool use_virtual_connector;
> +} data_t;
> +
> +// Video modes indexed by VIC
IGT uses kernel style formatting so this comment should be:
/* Video modes indexed by VIC */
> +static drmModeModeInfo test_modes[] = {
> + [1] = { 25175,
> + 640, 656, 752, 800, 0,
> + 480, 489, 492, 525, 0,
> + 60, 0xa, 0x40,
> + "640x480", /* VIC 1 */
> + },
> + [96] = { 594000,
> + 3840, 4896, 4984, 5280, 0,
> + 2160, 2168, 2178, 2250, 0,
> + 50, 0x5|DRM_MODE_FLAG_PIC_AR_16_9, 0x40,
> + "3840x2160", /* VIC 96 */
> + },
> + [97] = { 594000,
> + 3840, 4016, 4104, 4400, 0,
> + 2160, 2168, 2178, 2250, 0,
> + 60, 0x5|DRM_MODE_FLAG_PIC_AR_16_9, 0x40,
> + "3840x2160", /* VIC 97 */
> + },
> + [101] = { 594000,
> + 4096, 5064, 5152, 5280, 0,
> + 2160, 2168, 2178, 2250, 0,
> + 50, 0x5|DRM_MODE_FLAG_PIC_AR_256_135, 0x40,
> + "4096x2160", /* VIC 101 */
> + },
> + [102] = { 594000,
> + 4096, 4184, 4272, 4400, 0,
> + 2160, 2168, 2178, 2250, 0,
> + 60, 0x5|DRM_MODE_FLAG_PIC_AR_256_135, 0x40,
> + "4096x2160", /* VIC 102 */
> + },
> + [106] = { 594000,
> + 3840, 4896, 4984, 5280, 0,
> + 2160, 2168, 2178, 2250, 0,
> + 50, 0x5|DRM_MODE_FLAG_PIC_AR_64_27, 0x40,
> + "3840x2160", /* VIC 106 */
> + },
> + [107] = { 594000,
> + 3840, 4016, 4104, 4400, 0,
> + 2160, 2168, 2178, 2250, 0,
> + 60, 0x5|DRM_MODE_FLAG_PIC_AR_64_27, 0x40,
> + "3840x2160", /* VIC 107 */
> + },
> +};
> +
> +/* Common test setup. */
> +static void test_init(data_t *data)
> +{
> + igt_display_t *display = &data->display;
> +
> + data->pipe_id = PIPE_A;
> + data->pipe = &data->display.pipes[data->pipe_id];
> +
> + igt_display_reset(display);
> +
> + /* find a connected HDMI output */
> + data->output = NULL;
> + for (int i=0; i < data->display.n_outputs; ++i) {
> + drmModeConnector *connector = data->display.outputs[i].config.connector;
> + if (connector->connection == DRM_MODE_CONNECTED &&
> + (connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> + (data->use_virtual_connector &&
> + connector->connector_type == DRM_MODE_CONNECTOR_VIRTUAL))) {
> + data->output = &data->display.outputs[i];
> + }
> + }
> +
> + igt_require(data->output);
> +
> + data->primary =
> + igt_pipe_get_plane_type(data->pipe, DRM_PLANE_TYPE_PRIMARY);
> +
> + igt_output_set_pipe(data->output, data->pipe_id);
> +
> +}
> +
> +/* Common test cleanup. */
> +static void test_fini(data_t *data)
> +{
> + igt_display_reset(&data->display);
> +}
> +
> +static void test_vic_mode(data_t *data, int vic)
> +{
> + igt_display_t *display = &data->display;
> + drmModeModeInfo *mode;
> + igt_fb_t afb;
> +
> + test_init(data);
> +
> + mode = &test_modes[vic];
> +
> + igt_output_override_mode(data->output, mode);
> +
> + igt_create_pattern_fb(data->fd, mode->hdisplay, mode->vdisplay, DRM_FORMAT_XRGB8888, 0, &afb);
> +
> + igt_plane_set_fb(data->primary, &afb);
> +
> + igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +
> + fprintf(stderr, "Press [Enter] to finish\n");
I think igt_info(...) is preferred here over fprintf directly.
> + getchar();
I understand that you aren't using igt_debug_wait_for_keypress here, but
I think an improvement that could be made is to extract this out into a
sub function that waits explicitly for the newline:
static void wait_for_keypress(void)
{
while (getchar() != '\n')
;
}
> +
> + test_fini(data);
> +}
> +
> +const char *optstr = "hvt:";
> +static void usage(const char *name)
> +{
> + fprintf(stderr, "Usage: %s [-ht]\n", name);
> + fprintf(stderr, "-h: show help\n");
> + fprintf(stderr, "-t vic: select video mode based on VIC\n");
> + fprintf(stderr, "-v: test on 'Virtual' connector as well, for debugging.\n");
igt_info(...) if possible here
> +}
> +
> +int main(int argc, char **argv)
> +{
> + data_t data;
> + int c;
> + int vic = 1; /* default to VIC 1 (640x480) */
> +
> + memset(&data, 0, sizeof(data));
> +
> + while((c = getopt(argc, argv, optstr)) != -1) {
> + switch(c) {
> + case 't':
> + vic = atoi(optarg);
> + break;
> + case 'v':
> + data.use_virtual_connector = true;
> + break;
> + default:
> + case 'h':
> + usage(argv[0]);
> + exit(1);
> + }
> + }
> +
> + if (vic < 1 ||
> + vic > ARRAY_SIZE(test_modes) ||
> + !test_modes[vic].name[0]) {
> + fprintf(stderr, "VIC %d is not supported\n", vic);
igt_info(...) if possible here
> + exit(1);
> + }
> +
> + data.fd = drm_open_driver_master(DRIVER_ANY);
> + kmstest_set_vt_graphics_mode();
> +
> + igt_display_require(&data.display, data.fd);
> + igt_require(data.display.is_atomic);
> + igt_display_require_output(&data.display);
> +
> + test_vic_mode(&data, vic);
> +
> + igt_display_fini(&data.display);
> +}
> diff --git a/tools/meson.build b/tools/meson.build
> index 6e72b263..fa366ab2 100644
> --- a/tools/meson.build
> +++ b/tools/meson.build
> @@ -100,6 +100,11 @@ executable('intel_gpu_top', 'intel_gpu_top.c',
> install_rpath : bindir_rpathdir,
> dependencies : lib_igt_perf)
>
> +executable('amd_hdmi_compliance_yuv420', 'amd_hdmi_compliance_yuv420.c',
> + dependencies : [tool_deps],
> + install_rpath : bindir_rpathdir,
> + install : true)
> +
> conf_data = configuration_data()
> conf_data.set('prefix', prefix)
> conf_data.set('exec_prefix', '${prefix}')
>
More information about the igt-dev
mailing list