On 01/02/2021 23:13, Ville Syrjälä wrote: > On Wed, Sep 23, 2020 at 12:13:53PM +1000, Sam McNally wrote: >> From: Hans Verkuil <hans.verkuil@xxxxxxxxx> >> >> For adapters behind an MST hub use the correct AUX channel. >> >> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> >> [sammc@xxxxxxxxxxxx: rebased, removing redundant changes] >> Signed-off-by: Sam McNally <sammc@xxxxxxxxxxxx> >> --- >> >> (no changes since v1) >> >> drivers/gpu/drm/drm_dp_mst_topology.c | 36 +++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c >> index 15b6cc39a754..0d753201adbd 100644 >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c >> @@ -2255,6 +2255,9 @@ drm_dp_mst_topology_unlink_port(struct drm_dp_mst_topology_mgr *mgr, >> drm_dp_mst_topology_put_port(port); >> } >> >> +static ssize_t >> +drm_dp_mst_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg); >> + >> static struct drm_dp_mst_port * >> drm_dp_mst_add_port(struct drm_device *dev, >> struct drm_dp_mst_topology_mgr *mgr, >> @@ -2271,9 +2274,13 @@ drm_dp_mst_add_port(struct drm_device *dev, >> port->port_num = port_number; >> port->mgr = mgr; >> port->aux.name = "DPMST"; >> + mutex_init(&port->aux.hw_mutex); >> + mutex_init(&port->aux.cec.lock); >> port->aux.dev = dev->dev; >> port->aux.is_remote = true; >> >> + port->aux.transfer = drm_dp_mst_aux_transfer; >> + > > This was supposed to be handled via higher levels checking for > is_remote==true. Ah, I suspect this patch can be dropped entirely: it predates commit 2f221a5efed4 ("drm/dp_mst: Add MST support to DP DPCD R/W functions"). It looks like that commit basically solved what this older patch attempts to do as well. Sam, can you test if it works without this patch? Regards, Hans > >> /* initialize the MST downstream port's AUX crc work queue */ >> drm_dp_remote_aux_init(&port->aux); >> >> @@ -3503,6 +3510,35 @@ static int drm_dp_send_up_ack_reply(struct drm_dp_mst_topology_mgr *mgr, >> return 0; >> } >> >> +static ssize_t >> +drm_dp_mst_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) >> +{ >> + struct drm_dp_mst_port *port = >> + container_of(aux, struct drm_dp_mst_port, aux); >> + int ret; >> + >> + switch (msg->request & ~DP_AUX_I2C_MOT) { >> + case DP_AUX_NATIVE_WRITE: >> + case DP_AUX_I2C_WRITE: >> + case DP_AUX_I2C_WRITE_STATUS_UPDATE: >> + ret = drm_dp_send_dpcd_write(port->mgr, port, msg->address, >> + msg->size, msg->buffer); > > That doesn't make sense to me. I2c writes and DPCD writes > are definitely not the same thing. > > aux->transfer is a very low level thing. I don't think it's the > correct level of abstraction for sideband. > >> + break; >> + >> + case DP_AUX_NATIVE_READ: >> + case DP_AUX_I2C_READ: >> + ret = drm_dp_send_dpcd_read(port->mgr, port, msg->address, >> + msg->size, msg->buffer); >> + break; >> + >> + default: >> + ret = -EINVAL; >> + break; >> + } >> + >> + return ret; >> +} >> + >> static int drm_dp_get_vc_payload_bw(u8 dp_link_bw, u8 dp_link_count) >> { >> if (dp_link_bw == 0 || dp_link_count == 0) >> -- >> 2.28.0.681.g6f77f65b4e-goog >> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@xxxxxxxxxxxxxxxxxxxxx >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel