All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Simon Ser <simon.ser@intel.com>
Cc: igt-dev@lists.freedesktop.org, martin.peres@intel.com
Subject: Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_kms: remove length parameter from kmstest_force_edid
Date: Tue, 2 Jul 2019 16:46:35 +0300	[thread overview]
Message-ID: <20190702134635.GQ5942@intel.com> (raw)
In-Reply-To: <20190702125038.21621-1-simon.ser@intel.com>

On Tue, Jul 02, 2019 at 03:50:34PM +0300, Simon Ser wrote:
> Given an EDID, computing the size is trivial. Instead of having one size
> constant per EDID and hope the callers use the right one (ie. *not* EDID_LENGTH
> when there's an extension), we can make functions that take EDIDs compute the
> size if they need it.
> 
> We have tests in lib/tests/igt_edid.c which assert the number of extensions
> present in the EDID anyway.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  lib/igt_kms.c                     |  9 ++++++---
>  lib/igt_kms.h                     |  3 +--
>  tests/kms_3d.c                    |  4 ++--
>  tests/kms_force_connector_basic.c | 12 ++++++------
>  tests/kms_hdmi_inject.c           | 11 ++++-------
>  5 files changed, 19 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index dc8992cb043b..f8185edf6e8b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -182,6 +182,8 @@ const unsigned char *igt_kms_get_alt_edid(void)
>  	return (unsigned char *) &edid;
>  }
> 
> +#define AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
> +
>  static void
>  generate_audio_edid(unsigned char raw_edid[static AUDIO_EDID_LENGTH],
>  		    bool with_vsd, struct cea_sad *sad,
> @@ -998,7 +1000,7 @@ bool kmstest_force_connector(int drm_fd, drmModeConnector *connector,
>   * If @length is zero, the forced EDID will be removed.
        ^^^^^^^^
Please fix the docs too.

With that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>   */
>  void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
> -			const unsigned char *edid, size_t length)
> +			const unsigned char *edid)
>  {
>  	char *path;
>  	int debugfs_fd, ret;
> @@ -1011,10 +1013,11 @@ void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
> 
>  	igt_require(debugfs_fd != -1);
> 
> -	if (length == 0)
> +	if (edid == NULL)
>  		ret = write(debugfs_fd, "reset", 5);
>  	else
> -		ret = write(debugfs_fd, edid, length);
> +		ret = write(debugfs_fd, edid,
> +			    edid_get_size((struct edid *) edid));
>  	close(debugfs_fd);
> 
>  	/* To allow callers to always use GetConnectorCurrent we need to force a
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index a448a003ae56..f72508640712 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -197,7 +197,7 @@ bool kmstest_force_connector(int fd, drmModeConnector *connector,
>  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, size_t length);
> +			const unsigned char *edid);
> 
>  bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
>  					drmModeModeInfo *mode);
> @@ -759,7 +759,6 @@ struct cea_sad;
>  struct cea_speaker_alloc;
> 
>  #define EDID_LENGTH 128
> -#define AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
>  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);
> diff --git a/tests/kms_3d.c b/tests/kms_3d.c
> index df8185abebc4..a170814f6356 100644
> --- a/tests/kms_3d.c
> +++ b/tests/kms_3d.c
> @@ -60,7 +60,7 @@ igt_simple_main
>  	kmstest_edid_add_3d(igt_kms_get_base_edid(), EDID_LENGTH, &edid,
>  			    &length);
> 
> -	kmstest_force_edid(drm_fd, connector, edid, length);
> +	kmstest_force_edid(drm_fd, connector, edid);
>  	if (!kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_ON))
>  		igt_skip("Could not force connector on\n");
> 
> @@ -113,7 +113,7 @@ igt_simple_main
>  	}
> 
>  	kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_UNSPECIFIED);
> -	kmstest_force_edid(drm_fd, connector, NULL, 0);
> +	kmstest_force_edid(drm_fd, connector, NULL);
> 
>  	drmModeFreeConnector(connector);
>  	free(edid);
> diff --git a/tests/kms_force_connector_basic.c b/tests/kms_force_connector_basic.c
> index 20812d5e3189..f1533e5415c0 100644
> --- a/tests/kms_force_connector_basic.c
> +++ b/tests/kms_force_connector_basic.c
> @@ -48,7 +48,7 @@ static void reset_connectors(void)
>  		kmstest_force_connector(drm_fd, connector,
>  					FORCE_CONNECTOR_UNSPECIFIED);
> 
> -		kmstest_force_edid(drm_fd, connector, NULL, 0);
> +		kmstest_force_edid(drm_fd, connector, NULL);
> 
>  		drmModeFreeConnector(connector);
>  	}
> @@ -247,7 +247,7 @@ igt_main_args("", long_opts, help_str, opt_handler, NULL)
> 
>  		/* test edid forcing */
>  		kmstest_force_edid(drm_fd, vga_connector,
> -				   igt_kms_get_base_edid(), EDID_LENGTH);
> +				   igt_kms_get_base_edid());
>  		temp = drmModeGetConnectorCurrent(drm_fd,
>  						  vga_connector->connector_id);
> 
> @@ -260,7 +260,7 @@ igt_main_args("", long_opts, help_str, opt_handler, NULL)
>  		drmModeFreeConnector(temp);
> 
>  		/* remove edid */
> -		kmstest_force_edid(drm_fd, vga_connector, NULL, 0);
> +		kmstest_force_edid(drm_fd, vga_connector, NULL);
>  		kmstest_force_connector(drm_fd, vga_connector,
>  					FORCE_CONNECTOR_UNSPECIFIED);
>  		temp = drmModeGetConnectorCurrent(drm_fd,
> @@ -280,7 +280,7 @@ igt_main_args("", long_opts, help_str, opt_handler, NULL)
> 
>  		/* test pruning of stale modes */
>  		kmstest_force_edid(drm_fd, vga_connector,
> -				   igt_kms_get_alt_edid(), EDID_LENGTH);
> +				   igt_kms_get_alt_edid());
>  		temp = drmModeGetConnectorCurrent(drm_fd,
>  						  vga_connector->connector_id);
> 
> @@ -294,7 +294,7 @@ igt_main_args("", long_opts, help_str, opt_handler, NULL)
>  		drmModeFreeConnector(temp);
> 
>  		kmstest_force_edid(drm_fd, vga_connector,
> -				   igt_kms_get_base_edid(), EDID_LENGTH);
> +				   igt_kms_get_base_edid());
>  		temp = drmModeGetConnectorCurrent(drm_fd,
>  						  vga_connector->connector_id);
> 
> @@ -307,7 +307,7 @@ igt_main_args("", long_opts, help_str, opt_handler, NULL)
> 
>  		drmModeFreeConnector(temp);
> 
> -		kmstest_force_edid(drm_fd, vga_connector, NULL, 0);
> +		kmstest_force_edid(drm_fd, vga_connector, NULL);
>  		kmstest_force_connector(drm_fd, vga_connector,
>  					FORCE_CONNECTOR_UNSPECIFIED);
>  	}
> diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> index 8c0d1333db19..9a968fa9e50e 100644
> --- a/tests/kms_hdmi_inject.c
> +++ b/tests/kms_hdmi_inject.c
> @@ -93,7 +93,7 @@ hdmi_inject_4k(int drm_fd, drmModeConnector *connector)
>  	kmstest_edid_add_4k(igt_kms_get_base_edid(), EDID_LENGTH, &edid,
>  			    &length);
> 
> -	kmstest_force_edid(drm_fd, connector, edid, length);
> +	kmstest_force_edid(drm_fd, connector, edid);
> 
>  	if (!kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_ON))
>  		igt_skip("Could not force connector on\n");
> @@ -134,7 +134,7 @@ hdmi_inject_4k(int drm_fd, drmModeConnector *connector)
>  	igt_remove_fb(drm_fd, &fb);
> 
>  	kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_UNSPECIFIED);
> -	kmstest_force_edid(drm_fd, connector, NULL, 0);
> +	kmstest_force_edid(drm_fd, connector, NULL);
> 
>  	free(edid);
>  }
> @@ -143,15 +143,12 @@ static void
>  hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
>  {
>  	const unsigned char *edid;
> -	size_t length;
>  	int fb_id, cid, ret, crtc_mask = -1;
>  	struct igt_fb fb;
>  	struct kmstest_connector_config config;
> 
>  	edid = igt_kms_get_hdmi_audio_edid();
> -	length = AUDIO_EDID_LENGTH;
> -
> -	kmstest_force_edid(drm_fd, connector, edid, length);
> +	kmstest_force_edid(drm_fd, connector, edid);
> 
>  	if (!kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_ON))
>  		igt_skip("Could not force connector on\n");
> @@ -191,7 +188,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
>  	kmstest_dump_mode(&connector->modes[0]);
> 
>  	kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_UNSPECIFIED);
> -	kmstest_force_edid(drm_fd, connector, NULL, 0);
> +	kmstest_force_edid(drm_fd, connector, NULL);
>  }
> 
>  igt_main
> --
> 2.22.0
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

  parent reply	other threads:[~2019-07-02 13:46 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-02 12:50 [igt-dev] [PATCH i-g-t 1/5] lib/igt_kms: remove length parameter from kmstest_force_edid Simon Ser
2019-07-02 12:50 ` [igt-dev] [PATCH i-g-t 2/5] lib/igt_edid: add hdmi_vsd Simon Ser
2019-07-02 14:05   ` Ville Syrjälä
2019-07-03 12:10     ` Ser, Simon
2019-07-02 12:50 ` [igt-dev] [PATCH i-g-t 3/5] lib/igt_edid: add support for native DTDs in CEA extension blocks Simon Ser
2019-07-02 15:38   ` Ville Syrjälä
2019-07-02 12:50 ` [igt-dev] [PATCH i-g-t 4/5] lib/igt_edid: add support for Short Video Descriptors Simon Ser
2019-07-02 15:41   ` Ville Syrjälä
2019-07-02 12:50 ` [igt-dev] [PATCH i-g-t 5/5] lib/igt_kms: use igt_edid to generate a 4K EDID Simon Ser
2019-07-02 15:51   ` Ville Syrjälä
2019-07-03  8:24     ` Ser, Simon
2019-07-03 12:58       ` Ville Syrjälä
2019-07-03 17:00         ` Ser, Simon
2019-07-02 13:46 ` Ville Syrjälä [this message]
2019-07-02 14:12 ` [igt-dev] ✗ GitLab.Pipeline: warning for series starting with [i-g-t,1/5] lib/igt_kms: remove length parameter from kmstest_force_edid Patchwork
2019-07-02 14:28 ` [igt-dev] ✓ Fi.CI.BAT: success " Patchwork
2019-07-03 10:50 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190702134635.GQ5942@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=martin.peres@intel.com \
    --cc=simon.ser@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.