Re: [igt-dev] [PATCH i-g-t 3/3] lib/igt_chamelium: replace EDID IDs with opaque structs

[Martin Peres <martin.peres@linux.intel.com>]

On 14/06/2019 17:22, Simon Ser wrote:
> There are two reasons for this:
> 
> * An opaque struct is more type-safe than an int
> * In the future we'll want to tag EDIDs with the Chamelium port ID, to be able
>   to automagically infer the DRM connector <-> Chamelium port mapping. This is
>   necessary for DP MST (connector names are not stable).
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

> ---
>  lib/igt_chamelium.c   | 25 ++++++++++++-------------
>  lib/igt_chamelium.h   |  8 ++++++--
>  tests/kms_chamelium.c |  2 +-
>  3 files changed, 19 insertions(+), 16 deletions(-)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 0c44e56cad86..b83ff395d44b 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -530,13 +530,13 @@ void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
>   * Uploads and registers a new EDID with the chamelium. The EDID will be
>   * destroyed automatically when #chamelium_deinit is called.
>   *
> - * Returns: The ID of the EDID uploaded to the chamelium.
> + * Returns: An opaque pointer to the Chamelium EDID
>   */
> -int chamelium_new_edid(struct chamelium *chamelium,
> -		       const unsigned char *raw_edid)
> +struct chamelium_edid *chamelium_new_edid(struct chamelium *chamelium,
> +					  const unsigned char *raw_edid)
>  {
>  	xmlrpc_value *res;
> -	struct chamelium_edid *allocated_edid;
> +	struct chamelium_edid *chamelium_edid;
>  	int edid_id;
>  	struct edid *edid = (struct edid *) raw_edid;
>  	size_t edid_size = sizeof(struct edid) +
> @@ -548,14 +548,12 @@ int chamelium_new_edid(struct chamelium *chamelium,
>  	xmlrpc_read_int(&chamelium->env, res, &edid_id);
>  	xmlrpc_DECREF(res);
>  
> -	allocated_edid = malloc(sizeof(struct chamelium_edid));
> -	memset(allocated_edid, 0, sizeof(*allocated_edid));
> +	chamelium_edid = calloc(1, sizeof(struct chamelium_edid));
> +	chamelium_edid->id = edid_id;
>  
> -	allocated_edid->id = edid_id;
> +	igt_list_add(&chamelium_edid->link, &chamelium->edids);
>  
> -	igt_list_add(&allocated_edid->link, &chamelium->edids);
> -
> -	return edid_id;
> +	return chamelium_edid;
>  }
>  
>  static void chamelium_destroy_edid(struct chamelium *chamelium, int edid_id)
> @@ -568,7 +566,7 @@ static void chamelium_destroy_edid(struct chamelium *chamelium, int edid_id)
>   * chamelium_port_set_edid:
>   * @chamelium: The Chamelium instance to use
>   * @port: The port on the Chamelium to set the EDID on
> - * @edid_id: The ID of an EDID on the chamelium created with
> + * @edid: The Chamelium EDID to set
>   * #chamelium_new_edid, or 0 to disable the EDID on the port
>   *
>   * Sets a port on the chamelium to use the specified EDID. This does not fire a
> @@ -578,10 +576,11 @@ static void chamelium_destroy_edid(struct chamelium *chamelium, int edid_id)
>   * change.
>   */
>  void chamelium_port_set_edid(struct chamelium *chamelium,
> -			     struct chamelium_port *port, int edid_id)
> +			     struct chamelium_port *port,
> +			     struct chamelium_edid *edid)
>  {
>  	xmlrpc_DECREF(chamelium_rpc(chamelium, NULL, "ApplyEdid", "(ii)",
> -				    port->id, edid_id));
> +				    port->id, edid->id));
>  }
>  
>  /**
> diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
> index 28f726c91e83..ce9e9ced75d9 100644
> --- a/lib/igt_chamelium.h
> +++ b/lib/igt_chamelium.h
> @@ -59,6 +59,8 @@ struct chamelium_audio_file {
>  	int channels;
>  };
>  
> +struct chamelium_edid;
> +
>  /**
>   * CHAMELIUM_DEFAULT_EDID: provide this ID to #chamelium_port_set_edid to use
>   * the default EDID.
> @@ -98,9 +100,11 @@ void chamelium_fire_hpd_pulses(struct chamelium *chamelium,
>  void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
>  				   struct chamelium_port *port, int delay_ms,
>  				   bool rising_edge);
> -int chamelium_new_edid(struct chamelium *chamelium, const unsigned char *edid);
> +struct chamelium_edid *chamelium_new_edid(struct chamelium *chamelium,
> +					  const unsigned char *edid);
>  void chamelium_port_set_edid(struct chamelium *chamelium,
> -			     struct chamelium_port *port, int edid_id);
> +			     struct chamelium_port *port,
> +			     struct chamelium_edid *edid);
>  bool chamelium_port_get_ddc_state(struct chamelium *chamelium,
>  				  struct chamelium_port *port);
>  void chamelium_port_set_ddc_state(struct chamelium *chamelium,
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 03ebbd9c4b20..18f504f3bcad 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -51,7 +51,7 @@ typedef struct {
>  
>  	int drm_fd;
>  
> -	int edids[TEST_EDID_COUNT];
> +	struct chamelium_edid *edids[TEST_EDID_COUNT];
>  } data_t;
>  
>  #define HOTPLUG_TIMEOUT 20 /* seconds */
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev