| Message ID | 20190820191203.25807-10-David.Francis@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Display Stream Compression (DSC) for AMD Navi | expand |
[Added Leo Li here, since they did the auxdev work that introduced these
functions]
Since it seems we'll actually be doing remote DPCD read/writes in DRM drivers
and not just from auxdev, maybe we should combine drm_dp_dpcd_read() with
drm_dp_mst_dpcd_read() and do the same with the _write() variants? Based on
previous discussions with Leo Li I remember that we can't just combine the
dp_aux_dev->transfer() callbacks in struct drm_dp_aux_dev, but I don't see a
reason we can't just teach drm_dp_dpcd_read/write() to work with MST aux so
that we don't need two seperate functions. This should be pretty easy to do,
just:
/* Add a check like this at the start of drm_dp_dpcd_read(): */
ssize_t drm_dp_dpcd_read(...) {
if (aux->is_remote)
return drm_dp_mst_dpcd_read(...);
/* ... */
}
/* And in drm_dp_dpcd_write(): */
ssize_t drm_dp_dpcd_write(...) {
if (aux->is_remote)
return drm_dp_mst_dpcd_write(...);
/* ... */
}
Then just replace the manual calls to drm_dp_mst_dpcd_write() in
drivers/gpu/drm/drm_dp_aux_dev.c with normal
drm_dp_dpcd_read()/drm_dp_dpcd_write() calls. Thoughts?
On Tue, 2019-08-20 at 15:11 -0400, David Francis wrote:
> To use these functions in drm driver directories, they must be
> exported
>
> Signed-off-by: David Francis <David.Francis@amd.com>
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index b40d975aec76..5d5bd42da17c 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1512,6 +1512,7 @@ ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux,
> return drm_dp_send_dpcd_read(port->mgr, port,
> offset, size, buffer);
> }
> +EXPORT_SYMBOL(drm_dp_mst_dpcd_read);
>
> /**
> * drm_dp_mst_dpcd_write() - write a series of bytes to the DPCD via
> sideband
> @@ -1535,6 +1536,7 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux,
> return drm_dp_send_dpcd_write(port->mgr, port,
> offset, size, buffer);
> }
> +EXPORT_SYMBOL(drm_dp_mst_dpcd_write);
>
> static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8
> *guid)
> {
On 2019-08-20 5:02 p.m., Lyude Paul wrote: > [Added Leo Li here, since they did the auxdev work that introduced these > functions] > > Since it seems we'll actually be doing remote DPCD read/writes in DRM drivers > and not just from auxdev, maybe we should combine drm_dp_dpcd_read() with > drm_dp_mst_dpcd_read() and do the same with the _write() variants? Based on > previous discussions with Leo Li I remember that we can't just combine the > dp_aux_dev->transfer() callbacks in struct drm_dp_aux_dev, but I don't see a > reason we can't just teach drm_dp_dpcd_read/write() to work with MST aux so > that we don't need two seperate functions. This should be pretty easy to do, > just: > > /* Add a check like this at the start of drm_dp_dpcd_read(): */ > ssize_t drm_dp_dpcd_read(...) { > if (aux->is_remote) > return drm_dp_mst_dpcd_read(...); > > /* ... */ > } > > /* And in drm_dp_dpcd_write(): */ > ssize_t drm_dp_dpcd_write(...) { > if (aux->is_remote) > return drm_dp_mst_dpcd_write(...); > > /* ... */ > } > > Then just replace the manual calls to drm_dp_mst_dpcd_write() in > drivers/gpu/drm/drm_dp_aux_dev.c with normal > drm_dp_dpcd_read()/drm_dp_dpcd_write() calls. Thoughts? I think this would work well. drm_dp_mst_dpcd_read/write will eventually call drm_dp_dpcd_write, so doing so would cause a recursive call. That should be safe though. The mst manager's aux will be used, and that should always be a real (is_remote == false) aux, fwict. Essentially, it's just moving that conditional from auxdev_read/write_iter to drm_dp_dpcd_read/write. Leo > > On Tue, 2019-08-20 at 15:11 -0400, David Francis wrote: >> To use these functions in drm driver directories, they must be >> exported >> >> Signed-off-by: David Francis <David.Francis@amd.com> >> --- >> drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c >> b/drivers/gpu/drm/drm_dp_mst_topology.c >> index b40d975aec76..5d5bd42da17c 100644 >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c >> @@ -1512,6 +1512,7 @@ ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux, >> return drm_dp_send_dpcd_read(port->mgr, port, >> offset, size, buffer); >> } >> +EXPORT_SYMBOL(drm_dp_mst_dpcd_read); >> >> /** >> * drm_dp_mst_dpcd_write() - write a series of bytes to the DPCD via >> sideband >> @@ -1535,6 +1536,7 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux, >> return drm_dp_send_dpcd_write(port->mgr, port, >> offset, size, buffer); >> } >> +EXPORT_SYMBOL(drm_dp_mst_dpcd_write); >> >> static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 >> *guid) >> {
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index b40d975aec76..5d5bd42da17c 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -1512,6 +1512,7 @@ ssize_t drm_dp_mst_dpcd_read(struct drm_dp_aux *aux, return drm_dp_send_dpcd_read(port->mgr, port, offset, size, buffer); } +EXPORT_SYMBOL(drm_dp_mst_dpcd_read); /** * drm_dp_mst_dpcd_write() - write a series of bytes to the DPCD via sideband @@ -1535,6 +1536,7 @@ ssize_t drm_dp_mst_dpcd_write(struct drm_dp_aux *aux, return drm_dp_send_dpcd_write(port->mgr, port, offset, size, buffer); } +EXPORT_SYMBOL(drm_dp_mst_dpcd_write); static void drm_dp_check_mstb_guid(struct drm_dp_mst_branch *mstb, u8 *guid) {
To use these functions in drm driver directories, they must be exported Signed-off-by: David Francis <David.Francis@amd.com> --- drivers/gpu/drm/drm_dp_mst_topology.c | 2 ++ 1 file changed, 2 insertions(+)