Re: [PATCH 2/3] drm/dp_mst: Expose build_mst_prop_path()
On Wed, 2019-04-24 at 23:52 +0300, Ville Syrjälä wrote: > On Wed, Apr 24, 2019 at 08:40:30PM +, Li, Sun peng (Leo) wrote: > > > > > > On 2019-04-24 1:26 p.m., Lyude Paul wrote: > > > Closer, but are we sure we want to use the MST prop path for this? Why > > > not add > > > a sysfs attribute with the corresponding DRM connector name instead > > > since the > > > connector itself will have a path property. That way we can associate > > > aux > > > devices for eDP and DP devices with their corresponding connectors as > > > well > > > > I thought about that as well, but I hit a wall when trying to get the > > SST connector from the aux device. Perhaps there's a simpler way that > > I'm overlooking? > > > > It's easier for MST, since the mst_port can be obtained via container_of > > dp_aux. port->connector would then give what we want. > > > > For SST though, each driver calls drm_aux_register() with an aux struct > > that they've initialized. I'm not sure how I can reliably get the > > drm_connector from that. > > On i915 the aux is a child of the connector, so no extra > attributes/links needed. Maybe other drivers should/could > follow that apporach as well? ooo, good point. Yeah that seems like it would be worth a shot since it'd be a little nicer then just adding more sysfs attributes. But otherwise if that doesn't work, adding a connector parameter to drm_dp_aux_register() should be fine. > -- Cheers, Lyude Paul ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/dp_mst: Expose build_mst_prop_path()
On Wed, Apr 24, 2019 at 08:40:30PM +, Li, Sun peng (Leo) wrote: > > > > On 2019-04-24 1:26 p.m., Lyude Paul wrote: > > Closer, but are we sure we want to use the MST prop path for this? Why not > > add > > a sysfs attribute with the corresponding DRM connector name instead since > > the > > connector itself will have a path property. That way we can associate aux > > devices for eDP and DP devices with their corresponding connectors as well > > I thought about that as well, but I hit a wall when trying to get the > SST connector from the aux device. Perhaps there's a simpler way that > I'm overlooking? > > It's easier for MST, since the mst_port can be obtained via container_of > dp_aux. port->connector would then give what we want. > > For SST though, each driver calls drm_aux_register() with an aux struct > that they've initialized. I'm not sure how I can reliably get the > drm_connector from that. On i915 the aux is a child of the connector, so no extra attributes/links needed. Maybe other drivers should/could follow that apporach as well? -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/dp_mst: Expose build_mst_prop_path()
On 2019-04-24 1:26 p.m., Lyude Paul wrote:
> Closer, but are we sure we want to use the MST prop path for this? Why not add
> a sysfs attribute with the corresponding DRM connector name instead since the
> connector itself will have a path property. That way we can associate aux
> devices for eDP and DP devices with their corresponding connectors as well
I thought about that as well, but I hit a wall when trying to get the
SST connector from the aux device. Perhaps there's a simpler way that
I'm overlooking?
It's easier for MST, since the mst_port can be obtained via container_of
dp_aux. port->connector would then give what we want.
For SST though, each driver calls drm_aux_register() with an aux struct
that they've initialized. I'm not sure how I can reliably get the
drm_connector from that.
Maybe drm_dp_aux should have a back reference to the connector? FWICT
all drivers using drm_aux_register() should be able to provide the
associated connector when calling it.
Leo
>
> On Mon, 2019-04-22 at 19:56 -0400, sunpeng...@amd.com wrote:
>> From: Leo Li
>>
>> To give identifiable attributes to MST DP aux devices, we can use the
>> MST relative address. Expose this function for later use.
>>
>> Signed-off-by: Leo Li
>> ---
>> drivers/gpu/drm/drm_dp_mst_topology.c | 4 ++--
>> include/drm/drm_dp_mst_helper.h | 4
>> 2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
>> b/drivers/gpu/drm/drm_dp_mst_topology.c
>> index 2ab16c9..86ff8e2 100644
>> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
>> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
>> @@ -1120,7 +1120,7 @@ static void drm_dp_check_mstb_guid(struct
>> drm_dp_mst_branch *mstb, u8 *guid)
>> }
>> }
>>
>> -static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
>> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
>> int pnum,
>> char *proppath,
>> size_t proppath_size)
>> @@ -1202,7 +1202,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
>> *mstb,
>> if (created && !port->input) {
>> char proppath[255];
>>
>> -build_mst_prop_path(mstb, port->port_num, proppath,
>> sizeof(proppath));
>> +drm_dp_build_mst_prop_path(mstb, port->port_num, proppath,
>> sizeof(proppath));
>> port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr,
>> port, proppath);
>> if (!port->connector) {
>> /* remove it from the port list */
>> diff --git a/include/drm/drm_dp_mst_helper.h
>> b/include/drm/drm_dp_mst_helper.h
>> index 371cc28..81c8d79 100644
>> --- a/include/drm/drm_dp_mst_helper.h
>> +++ b/include/drm/drm_dp_mst_helper.h
>> @@ -602,6 +602,10 @@ void drm_dp_mst_deallocate_vcpi(struct
>> drm_dp_mst_topology_mgr *mgr,
>> int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
>> int pbn);
>>
>> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
>> + int pnum,
>> + char *proppath,
>> + size_t proppath_size);
>>
>> int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/dp_mst: Expose build_mst_prop_path()
Closer, but are we sure we want to use the MST prop path for this? Why not add
a sysfs attribute with the corresponding DRM connector name instead since the
connector itself will have a path property. That way we can associate aux
devices for eDP and DP devices with their corresponding connectors as well
On Mon, 2019-04-22 at 19:56 -0400, sunpeng...@amd.com wrote:
> From: Leo Li
>
> To give identifiable attributes to MST DP aux devices, we can use the
> MST relative address. Expose this function for later use.
>
> Signed-off-by: Leo Li
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 4 ++--
> include/drm/drm_dp_mst_helper.h | 4
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 2ab16c9..86ff8e2 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1120,7 +1120,7 @@ static void drm_dp_check_mstb_guid(struct
> drm_dp_mst_branch *mstb, u8 *guid)
> }
> }
>
> -static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
> int pnum,
> char *proppath,
> size_t proppath_size)
> @@ -1202,7 +1202,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
> *mstb,
> if (created && !port->input) {
> char proppath[255];
>
> - build_mst_prop_path(mstb, port->port_num, proppath,
> sizeof(proppath));
> + drm_dp_build_mst_prop_path(mstb, port->port_num, proppath,
> sizeof(proppath));
> port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr,
> port, proppath);
> if (!port->connector) {
> /* remove it from the port list */
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> index 371cc28..81c8d79 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -602,6 +602,10 @@ void drm_dp_mst_deallocate_vcpi(struct
> drm_dp_mst_topology_mgr *mgr,
> int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> int pbn);
>
> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
> +int pnum,
> +char *proppath,
> +size_t proppath_size);
>
> int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>
--
Cheers,
Lyude Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/dp_mst: Expose build_mst_prop_path()
On Mon, Apr 22, 2019 at 07:56:27PM -0400, sunpeng...@amd.com wrote:
> From: Leo Li
>
> To give identifiable attributes to MST DP aux devices, we can use the
> MST relative address. Expose this function for later use.
>
> Signed-off-by: Leo Li
> ---
> drivers/gpu/drm/drm_dp_mst_topology.c | 4 ++--
> include/drm/drm_dp_mst_helper.h | 4
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 2ab16c9..86ff8e2 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -1120,7 +1120,7 @@ static void drm_dp_check_mstb_guid(struct
> drm_dp_mst_branch *mstb, u8 *guid)
> }
> }
>
> -static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
> int pnum,
> char *proppath,
> size_t proppath_size)
I was wondering if we need to export this but looks like both
drm_dp_mst_topology.o and drm_dp_aux_dev.o both end up in
drm_kms_helper.ko, so the answer is no.
Reviewed-by: Ville Syrjälä
> @@ -1202,7 +1202,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
> *mstb,
> if (created && !port->input) {
> char proppath[255];
>
> - build_mst_prop_path(mstb, port->port_num, proppath,
> sizeof(proppath));
> + drm_dp_build_mst_prop_path(mstb, port->port_num, proppath,
> sizeof(proppath));
> port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr,
> port, proppath);
> if (!port->connector) {
> /* remove it from the port list */
> diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
> index 371cc28..81c8d79 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -602,6 +602,10 @@ void drm_dp_mst_deallocate_vcpi(struct
> drm_dp_mst_topology_mgr *mgr,
> int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> int pbn);
>
> +void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
> +int pnum,
> +char *proppath,
> +size_t proppath_size);
>
> int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
>
> --
> 2.7.4
--
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] drm/dp_mst: Expose build_mst_prop_path()
From: Leo Li
To give identifiable attributes to MST DP aux devices, we can use the
MST relative address. Expose this function for later use.
Signed-off-by: Leo Li
---
drivers/gpu/drm/drm_dp_mst_topology.c | 4 ++--
include/drm/drm_dp_mst_helper.h | 4
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 2ab16c9..86ff8e2 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -1120,7 +1120,7 @@ static void drm_dp_check_mstb_guid(struct
drm_dp_mst_branch *mstb, u8 *guid)
}
}
-static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
+void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
int pnum,
char *proppath,
size_t proppath_size)
@@ -1202,7 +1202,7 @@ static void drm_dp_add_port(struct drm_dp_mst_branch
*mstb,
if (created && !port->input) {
char proppath[255];
- build_mst_prop_path(mstb, port->port_num, proppath,
sizeof(proppath));
+ drm_dp_build_mst_prop_path(mstb, port->port_num, proppath,
sizeof(proppath));
port->connector = (*mstb->mgr->cbs->add_connector)(mstb->mgr,
port, proppath);
if (!port->connector) {
/* remove it from the port list */
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index 371cc28..81c8d79 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -602,6 +602,10 @@ void drm_dp_mst_deallocate_vcpi(struct
drm_dp_mst_topology_mgr *mgr,
int drm_dp_find_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
int pbn);
+void drm_dp_build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
+ int pnum,
+ char *proppath,
+ size_t proppath_size);
int drm_dp_update_payload_part1(struct drm_dp_mst_topology_mgr *mgr);
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
