Re: [PATCH 2/2] drm/dp_mst: Register aux-dev nodes for MST ports

["Ville Syrjälä" <ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]

On Tue, Apr 16, 2019 at 06:22:33PM -0400, Lyude Paul wrote:
> On Fri, 2019-04-12 at 12:05 -0400, sunpeng.li@amd.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Expose AUX devices for MST ports, similar to how they are exposed for
> > SST.
> > 
> > The registered device will have it's MST port path appended in order to
> > identify it. i.e. /dev/drm_dp_aux4_mst:0-2-1
> > 
> > So for a MST topology like so:
> > 
> >                +---------+
> >                |  ASIC   |
> >                +---------+
> >               Conn-0|
> >                     |
> >                +----v----+
> >           +----| MST HUB |----+
> >           |    +---------+    |
> >           |                   |
> >           |Port-1       Port-2|
> >     +-----v-----+       +-----v-----+
> >     |  MST      |       |  SST      |
> >     |  Display  |       |  Display  |
> >     +-----------+       +-----------+
> >           |Port-1
> >           x
> > 
> > The list of AUX device names will look like:
> > 
> > AUX Device Name       | MST Device
> > ----------------------+----------------------------------
> > drm_dp_aux0           | MST Hub
> > drm_dp_aux1_mst:0-1-1 | MST Display's disconnected DP out
> > drm_dp_aux2_mst:0-1   | MST Display
> > drm_dp_aux3_mst:0-2   | SST Display
> > 
> > Note that aux devices are only created for Physical Ports. Logical Ports
> > are left out, since they are internally connected within the MST device
> > (not connected to a DP RX or TX).
> > 
> > Leo Li:
> > * Add missing drm_crtc_helper_internal.h include
> > * Fix hard-coded offset and size in drm_dp_send_dpcd_read()
> > * Only create aux devices for physical ports.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: Leo Li <sunpeng.li@amd.com>
> > ---
> >  drivers/gpu/drm/drm_dp_aux_dev.c      |  13 +++-
> >  drivers/gpu/drm/drm_dp_mst_topology.c | 109 ++++++++++++++++++++++++++++++-
> > ---
> >  include/drm/drm_dp_helper.h           |   4 ++
> >  include/drm/drm_dp_mst_helper.h       |   6 ++
> >  4 files changed, 117 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_dp_aux_dev.c
> > b/drivers/gpu/drm/drm_dp_aux_dev.c
> > index 2310a67..f1241d1 100644
> > --- a/drivers/gpu/drm/drm_dp_aux_dev.c
> > +++ b/drivers/gpu/drm/drm_dp_aux_dev.c
> > @@ -34,6 +34,7 @@
> >  #include <linux/uaccess.h>
> >  #include <linux/uio.h>
> >  #include <drm/drm_dp_helper.h>
> > +#include <drm/drm_dp_mst_helper.h>
> >  #include <drm/drm_crtc.h>
> >  #include <drm/drmP.h>
> >  
> > @@ -160,7 +161,11 @@ static ssize_t auxdev_read_iter(struct kiocb *iocb,
> > struct iov_iter *to)
> >  			break;
> >  		}
> >  
> > -		res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
> > +		if (aux_dev->aux->is_remote)
> > +			res = drm_dp_mst_dpcd_read(aux_dev->aux, pos, buf,
> > todo);
> > +		else
> > +			res = drm_dp_dpcd_read(aux_dev->aux, pos, buf, todo);
> > +
> 
> mhhhh, is there a reason we can't just use the normal ->read and ->write
> auxdev callbacks for this instead of mixing layers like this?

IIRC I did think about that when I initially did this, but decided 
there was actual work involved so didn't bother with it, especially
since the code wasn't even working yet. Sadly I've forgotten what
the "acutal work" part was supposed to be.

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