[Intel-gfx] [PATCH 12/13] drm/display/dp_mst: convert to struct drm_edid

[Nautiyal, Ankit K]

On 4/21/2023 5:17 PM, Jani Nikula wrote:
> Convert the topology manager to use struct drm_edid, add
> drm_dp_mst_edid_read() that returns drm_edid, and rewrite the old
> drm_dp_mst_get_edid() to use it.
>
> Note that the old drm_get_edid() ended up calling
> drm_connector_update_edid_property(). This responsibility is now
> deferred to drivers, which all do it anyway after calling
> drm_dp_mst_edid_read() or drm_dp_mst_get_edid().
>
> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
> ---
>   drivers/gpu/drm/display/drm_dp_mst_topology.c | 53 +++++++++++++++----
>   include/drm/display/drm_dp_mst_helper.h       |  9 +++-
>   2 files changed, 49 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index a2b8732db0c8..be71be95b706 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -1823,7 +1823,7 @@ static void drm_dp_destroy_port(struct kref *kref)
>   		return;
>   	}
>   
> -	kfree(port->cached_edid);
> +	drm_edid_free(port->cached_edid);
>   
>   	/*
>   	 * we can't destroy the connector here, as we might be holding the
> @@ -2272,8 +2272,8 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch *mstb,
>   	if (port->pdt != DP_PEER_DEVICE_NONE &&
>   	    drm_dp_mst_is_end_device(port->pdt, port->mcs) &&
>   	    port->port_num >= DP_MST_LOGICAL_PORT_0)
> -		port->cached_edid = drm_get_edid(port->connector,
> -						 &port->aux.ddc);
> +		port->cached_edid = drm_edid_read_ddc(port->connector,
> +						      &port->aux.ddc);
>   
>   	drm_connector_register(port->connector);
>   	return;
> @@ -4133,7 +4133,7 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
>   		ret = connector_status_connected;
>   		/* for logical ports - cache the EDID */
>   		if (port->port_num >= DP_MST_LOGICAL_PORT_0 && !port->cached_edid)
> -			port->cached_edid = drm_get_edid(connector, &port->aux.ddc);
> +			port->cached_edid = drm_edid_read_ddc(connector, &port->aux.ddc);
>   		break;
>   	case DP_PEER_DEVICE_DP_LEGACY_CONV:
>   		if (port->ldps)
> @@ -4147,7 +4147,7 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
>   EXPORT_SYMBOL(drm_dp_mst_detect_port);
>   
>   /**
> - * drm_dp_mst_get_edid() - get EDID for an MST port
> + * drm_dp_mst_edid_read() - get EDID for an MST port

Perhaps mention drm_edid container here and also in return documentation?

In any case the change looks good to me.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>

>    * @connector: toplevel connector to get EDID for
>    * @mgr: manager for this port
>    * @port: unverified pointer to a port.
> @@ -4156,9 +4156,11 @@ EXPORT_SYMBOL(drm_dp_mst_detect_port);
>    * It validates the pointer still exists so the caller doesn't require a
>    * reference.
>    */
> -struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port)
> +const struct drm_edid *drm_dp_mst_edid_read(struct drm_connector *connector,
> +					    struct drm_dp_mst_topology_mgr *mgr,
> +					    struct drm_dp_mst_port *port)
>   {
> -	struct edid *edid = NULL;
> +	const struct drm_edid *drm_edid;
>   
>   	/* we need to search for the port in the mgr in case it's gone */
>   	port = drm_dp_mst_topology_get_port_validated(mgr, port);
> @@ -4166,12 +4168,41 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_
>   		return NULL;
>   
>   	if (port->cached_edid)
> -		edid = drm_edid_duplicate(port->cached_edid);
> -	else {
> -		edid = drm_get_edid(connector, &port->aux.ddc);
> -	}
> +		drm_edid = drm_edid_dup(port->cached_edid);
> +	else
> +		drm_edid = drm_edid_read_ddc(connector, &port->aux.ddc);
>   
>   	drm_dp_mst_topology_put_port(port);
> +
> +	return drm_edid;
> +}
> +EXPORT_SYMBOL(drm_dp_mst_edid_read);
> +
> +/**
> + * drm_dp_mst_get_edid() - get EDID for an MST port
> + * @connector: toplevel connector to get EDID for
> + * @mgr: manager for this port
> + * @port: unverified pointer to a port.
> + *
> + * This function is deprecated; please use drm_dp_mst_edid_read() instead.
> + *
> + * This returns an EDID for the port connected to a connector,
> + * It validates the pointer still exists so the caller doesn't require a
> + * reference.
> + */
> +struct edid *drm_dp_mst_get_edid(struct drm_connector *connector,
> +				 struct drm_dp_mst_topology_mgr *mgr,
> +				 struct drm_dp_mst_port *port)
> +{
> +	const struct drm_edid *drm_edid;
> +	struct edid *edid;
> +
> +	drm_edid = drm_dp_mst_edid_read(connector, mgr, port);
> +
> +	edid = drm_edid_duplicate(drm_edid_raw(drm_edid));
> +
> +	drm_edid_free(drm_edid);
> +
>   	return edid;
>   }
>   EXPORT_SYMBOL(drm_dp_mst_get_edid);
> diff --git a/include/drm/display/drm_dp_mst_helper.h b/include/drm/display/drm_dp_mst_helper.h
> index 5be96a158ab2..f962e97880b4 100644
> --- a/include/drm/display/drm_dp_mst_helper.h
> +++ b/include/drm/display/drm_dp_mst_helper.h
> @@ -138,7 +138,7 @@ struct drm_dp_mst_port {
>   	 * @cached_edid: for DP logical ports - make tiling work by ensuring
>   	 * that the EDID for all connectors is read immediately.
>   	 */
> -	struct edid *cached_edid;
> +	const struct drm_edid *cached_edid;
>   
>   	/**
>   	 * @fec_capable: bool indicating if FEC can be supported up to that
> @@ -819,7 +819,12 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
>   		       struct drm_dp_mst_topology_mgr *mgr,
>   		       struct drm_dp_mst_port *port);
>   
> -struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
> +const struct drm_edid *drm_dp_mst_edid_read(struct drm_connector *connector,
> +					    struct drm_dp_mst_topology_mgr *mgr,
> +					    struct drm_dp_mst_port *port);
> +struct edid *drm_dp_mst_get_edid(struct drm_connector *connector,
> +				 struct drm_dp_mst_topology_mgr *mgr,
> +				 struct drm_dp_mst_port *port);
>   
>   int drm_dp_get_vc_payload_bw(const struct drm_dp_mst_topology_mgr *mgr,
>   			     int link_rate, int link_lane_count);