[PATCH RFC] drm: Add a new connector atomic property for link status
On Tue, Nov 29, 2016 at 9:45 AM, Sean Paul wrote:
> On Mon, Nov 28, 2016 at 7:59 PM, Manasi Navare
> wrote:
>> On Wed, Nov 23, 2016 at 09:09:28AM +0100, Daniel Vetter wrote:
>>> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
>>> > On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
>>> > wrote:
>>> > > This is RFC patch for adding a connector link-status property
>>> > > and making it atomic by adding it to the drm_connector_state.
>>> > > This is to make sure its wired properly in
>>> > > drm_atomic_connector_set_property
>>> > > and drm_atomic_connector_get_property functions.
>>> > >
>>> >
>>> > Thanks for sending this. We ran into some re-training awkwardness
>>> > recently, and I
>>> > was hoping for such a patch.
>>> >
>>> > > Cc: Jani Nikula
>>> > > Cc: Daniel Vetter
>>> > > Cc: Ville Syrjala
>>> > > Cc: Chris Wilson
>>> > > Signed-off-by: Manasi Navare
>>> > > ---
>>> > > drivers/gpu/drm/drm_atomic.c| 5
>>> > > drivers/gpu/drm/drm_connector.c | 65
>>> > > +++--
>>> > > include/drm/drm_connector.h | 12 +++-
>>> > > include/drm/drm_mode_config.h | 5
>>> > > include/uapi/drm/drm_mode.h | 4 +++
>>> > > 5 files changed, 88 insertions(+), 3 deletions(-)
>>> > >
>>> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>> > > index 89737e4..644d19f 100644
>>> > > --- a/drivers/gpu/drm/drm_atomic.c
>>> > > +++ b/drivers/gpu/drm/drm_atomic.c
>>> > > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
>>> > > drm_connector *connector,
>>> > > * now?) atomic writes to DPMS property:
>>> > > */
>>> > > return -EINVAL;
>>> > > + } else if (property == config->link_status_property) {
>>> > > + state->link_status = val;
>>>
>>> I think we should have a check here to make sure userspace never tries to
>>> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
>>> atomic userspace is supposed to be able to just restore everything to what
>>> it was.
>>>
>>> I don't think trying to set it to "BAD" should cause an error though, just
>>> silently keep the link status at "GOOG". So:
>>>
>>> /* Never downgrade from GOOD to BAD on userspace's request here,
>>>* only hw issues can do that. */
>>> if (state->link_status == GOOD)
>>> return 0;
>>> state->link_status = val;
>>> return 0;
>>>
>>> > > + return 0;
>>> > > } else if (connector->funcs->atomic_set_property) {
>>> > > return connector->funcs->atomic_set_property(connector,
>>> > > state, property, val);
>>> > > @@ -1135,6 +1138,8 @@ static void
>>> > > drm_atomic_connector_print_state(struct drm_printer *p,
>>> > > *val = (state->crtc) ? state->crtc->base.id : 0;
>>> > > } else if (property == config->dpms_property) {
>>> > > *val = connector->dpms;
>>> > > + } else if (property == config->link_status_property) {
>>> > > + *val = state->link_status;
>>> > > } else if (connector->funcs->atomic_get_property) {
>>> > > return connector->funcs->atomic_get_property(connector,
>>> > > state, property, val);
>>> > > diff --git a/drivers/gpu/drm/drm_connector.c
>>> > > b/drivers/gpu/drm/drm_connector.c
>>> > > index 5a45262..4576c9f 100644
>>> > > --- a/drivers/gpu/drm/drm_connector.c
>>> > > +++ b/drivers/gpu/drm/drm_connector.c
>>> > > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
>>> > > drm_object_attach_property(>base,
>>> > > config->dpms_property, 0);
>>> > >
>>> > > + drm_object_attach_property(>base,
>>> > > + config->link_status_property,
>>> > > + 0);
>>> > > +
>>> > > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>>> > > drm_object_attach_property(>base,
>>> > > config->prop_crtc_id, 0);
>>> > > }
>>> > > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
>>> > > subpixel_order order)
>>> > > };
>>> > > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>>> > >
>>> > > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>>> > > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
>>> > > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
>>> > > +};
>>> > > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>>> > > +
>>> > > /**
>>> > > * drm_display_info_set_bus_formats - set the supported bus formats
>>> > > * @info: display info to store bus formats in
>>> > > @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
>>> > > drm_display_info *info,
>>> > > * path property the MST manager created. Userspace cannot change
>>> > > this
>>> > > * property.
>>> > > *
[PATCH RFC] drm: Add a new connector atomic property for link status
On Tue, Nov 29, 2016 at 02:43:55PM -0500, Sean Paul wrote:
> On Tue, Nov 29, 2016 at 9:45 AM, Sean Paul wrote:
> > On Mon, Nov 28, 2016 at 7:59 PM, Manasi Navare
> > wrote:
> >> On Wed, Nov 23, 2016 at 09:09:28AM +0100, Daniel Vetter wrote:
> >>> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
> >>> > On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
> >>> > wrote:
> >>> > > This is RFC patch for adding a connector link-status property
> >>> > > and making it atomic by adding it to the drm_connector_state.
> >>> > > This is to make sure its wired properly in
> >>> > > drm_atomic_connector_set_property
> >>> > > and drm_atomic_connector_get_property functions.
> >>> > >
> >>> >
> >>> > Thanks for sending this. We ran into some re-training awkwardness
> >>> > recently, and I
> >>> > was hoping for such a patch.
> >>> >
> >>> > > Cc: Jani Nikula
> >>> > > Cc: Daniel Vetter
> >>> > > Cc: Ville Syrjala
> >>> > > Cc: Chris Wilson
> >>> > > Signed-off-by: Manasi Navare
> >>> > > ---
> >>> > > drivers/gpu/drm/drm_atomic.c| 5
> >>> > > drivers/gpu/drm/drm_connector.c | 65
> >>> > > +++--
> >>> > > include/drm/drm_connector.h | 12 +++-
> >>> > > include/drm/drm_mode_config.h | 5
> >>> > > include/uapi/drm/drm_mode.h | 4 +++
> >>> > > 5 files changed, 88 insertions(+), 3 deletions(-)
> >>> > >
> >>> > > diff --git a/drivers/gpu/drm/drm_atomic.c
> >>> > > b/drivers/gpu/drm/drm_atomic.c
> >>> > > index 89737e4..644d19f 100644
> >>> > > --- a/drivers/gpu/drm/drm_atomic.c
> >>> > > +++ b/drivers/gpu/drm/drm_atomic.c
> >>> > > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
> >>> > > drm_connector *connector,
> >>> > > * now?) atomic writes to DPMS property:
> >>> > > */
> >>> > > return -EINVAL;
> >>> > > + } else if (property == config->link_status_property) {
> >>> > > + state->link_status = val;
> >>>
> >>> I think we should have a check here to make sure userspace never tries to
> >>> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
> >>> atomic userspace is supposed to be able to just restore everything to what
> >>> it was.
> >>>
> >>> I don't think trying to set it to "BAD" should cause an error though, just
> >>> silently keep the link status at "GOOG". So:
> >>>
> >>> /* Never downgrade from GOOD to BAD on userspace's request here,
> >>>* only hw issues can do that. */
> >>> if (state->link_status == GOOD)
> >>> return 0;
> >>> state->link_status = val;
> >>> return 0;
> >>>
> >>> > > + return 0;
> >>> > > } else if (connector->funcs->atomic_set_property) {
> >>> > > return
> >>> > > connector->funcs->atomic_set_property(connector,
> >>> > > state, property, val);
> >>> > > @@ -1135,6 +1138,8 @@ static void
> >>> > > drm_atomic_connector_print_state(struct drm_printer *p,
> >>> > > *val = (state->crtc) ? state->crtc->base.id : 0;
> >>> > > } else if (property == config->dpms_property) {
> >>> > > *val = connector->dpms;
> >>> > > + } else if (property == config->link_status_property) {
> >>> > > + *val = state->link_status;
> >>> > > } else if (connector->funcs->atomic_get_property) {
> >>> > > return
> >>> > > connector->funcs->atomic_get_property(connector,
> >>> > > state, property, val);
> >>> > > diff --git a/drivers/gpu/drm/drm_connector.c
> >>> > > b/drivers/gpu/drm/drm_connector.c
> >>> > > index 5a45262..4576c9f 100644
> >>> > > --- a/drivers/gpu/drm/drm_connector.c
> >>> > > +++ b/drivers/gpu/drm/drm_connector.c
> >>> > > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
> >>> > > drm_object_attach_property(>base,
> >>> > > config->dpms_property, 0);
> >>> > >
> >>> > > + drm_object_attach_property(>base,
> >>> > > + config->link_status_property,
> >>> > > + 0);
> >>> > > +
> >>> > > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> >>> > > drm_object_attach_property(>base,
> >>> > > config->prop_crtc_id, 0);
> >>> > > }
> >>> > > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
> >>> > > subpixel_order order)
> >>> > > };
> >>> > > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
> >>> > >
> >>> > > +static const struct drm_prop_enum_list drm_link_status_enum_list[] =
> >>> > > {
> >>> > > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
> >>> > > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
> >>> > > +};
> >>> > > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> >>> > > +
> >>> > > /**
> >>> > > * drm_display_info_set_bus_formats - set the
[PATCH RFC] drm: Add a new connector atomic property for link status
On Mon, Nov 28, 2016 at 7:59 PM, Manasi Navare
wrote:
> On Wed, Nov 23, 2016 at 09:09:28AM +0100, Daniel Vetter wrote:
>> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
>> > On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
>> > wrote:
>> > > This is RFC patch for adding a connector link-status property
>> > > and making it atomic by adding it to the drm_connector_state.
>> > > This is to make sure its wired properly in
>> > > drm_atomic_connector_set_property
>> > > and drm_atomic_connector_get_property functions.
>> > >
>> >
>> > Thanks for sending this. We ran into some re-training awkwardness
>> > recently, and I
>> > was hoping for such a patch.
>> >
>> > > Cc: Jani Nikula
>> > > Cc: Daniel Vetter
>> > > Cc: Ville Syrjala
>> > > Cc: Chris Wilson
>> > > Signed-off-by: Manasi Navare
>> > > ---
>> > > drivers/gpu/drm/drm_atomic.c| 5
>> > > drivers/gpu/drm/drm_connector.c | 65
>> > > +++--
>> > > include/drm/drm_connector.h | 12 +++-
>> > > include/drm/drm_mode_config.h | 5
>> > > include/uapi/drm/drm_mode.h | 4 +++
>> > > 5 files changed, 88 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> > > index 89737e4..644d19f 100644
>> > > --- a/drivers/gpu/drm/drm_atomic.c
>> > > +++ b/drivers/gpu/drm/drm_atomic.c
>> > > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
>> > > drm_connector *connector,
>> > > * now?) atomic writes to DPMS property:
>> > > */
>> > > return -EINVAL;
>> > > + } else if (property == config->link_status_property) {
>> > > + state->link_status = val;
>>
>> I think we should have a check here to make sure userspace never tries to
>> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
>> atomic userspace is supposed to be able to just restore everything to what
>> it was.
>>
>> I don't think trying to set it to "BAD" should cause an error though, just
>> silently keep the link status at "GOOG". So:
>>
>> /* Never downgrade from GOOD to BAD on userspace's request here,
>>* only hw issues can do that. */
>> if (state->link_status == GOOD)
>> return 0;
>> state->link_status = val;
>> return 0;
>>
>> > > + return 0;
>> > > } else if (connector->funcs->atomic_set_property) {
>> > > return connector->funcs->atomic_set_property(connector,
>> > > state, property, val);
>> > > @@ -1135,6 +1138,8 @@ static void
>> > > drm_atomic_connector_print_state(struct drm_printer *p,
>> > > *val = (state->crtc) ? state->crtc->base.id : 0;
>> > > } else if (property == config->dpms_property) {
>> > > *val = connector->dpms;
>> > > + } else if (property == config->link_status_property) {
>> > > + *val = state->link_status;
>> > > } else if (connector->funcs->atomic_get_property) {
>> > > return connector->funcs->atomic_get_property(connector,
>> > > state, property, val);
>> > > diff --git a/drivers/gpu/drm/drm_connector.c
>> > > b/drivers/gpu/drm/drm_connector.c
>> > > index 5a45262..4576c9f 100644
>> > > --- a/drivers/gpu/drm/drm_connector.c
>> > > +++ b/drivers/gpu/drm/drm_connector.c
>> > > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
>> > > drm_object_attach_property(>base,
>> > > config->dpms_property, 0);
>> > >
>> > > + drm_object_attach_property(>base,
>> > > + config->link_status_property,
>> > > + 0);
>> > > +
>> > > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>> > > drm_object_attach_property(>base,
>> > > config->prop_crtc_id, 0);
>> > > }
>> > > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
>> > > subpixel_order order)
>> > > };
>> > > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>> > >
>> > > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>> > > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
>> > > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
>> > > +};
>> > > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>> > > +
>> > > /**
>> > > * drm_display_info_set_bus_formats - set the supported bus formats
>> > > * @info: display info to store bus formats in
>> > > @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
>> > > drm_display_info *info,
>> > > * path property the MST manager created. Userspace cannot change
>> > > this
>> > > * property.
>> > > * TILE:
>> > > - * Connector tile group property to indicate how a set of DRM
>> > > connector
>> > > + * Connector tile group property to indicate how a
[PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, Nov 23, 2016 at 09:09:28AM +0100, Daniel Vetter wrote:
> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
> > On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
> > wrote:
> > > This is RFC patch for adding a connector link-status property
> > > and making it atomic by adding it to the drm_connector_state.
> > > This is to make sure its wired properly in
> > > drm_atomic_connector_set_property
> > > and drm_atomic_connector_get_property functions.
> > >
> >
> > Thanks for sending this. We ran into some re-training awkwardness
> > recently, and I
> > was hoping for such a patch.
> >
> > > Cc: Jani Nikula
> > > Cc: Daniel Vetter
> > > Cc: Ville Syrjala
> > > Cc: Chris Wilson
> > > Signed-off-by: Manasi Navare
> > > ---
> > > drivers/gpu/drm/drm_atomic.c| 5
> > > drivers/gpu/drm/drm_connector.c | 65
> > > +++--
> > > include/drm/drm_connector.h | 12 +++-
> > > include/drm/drm_mode_config.h | 5
> > > include/uapi/drm/drm_mode.h | 4 +++
> > > 5 files changed, 88 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > > index 89737e4..644d19f 100644
> > > --- a/drivers/gpu/drm/drm_atomic.c
> > > +++ b/drivers/gpu/drm/drm_atomic.c
> > > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
> > > drm_connector *connector,
> > > * now?) atomic writes to DPMS property:
> > > */
> > > return -EINVAL;
> > > + } else if (property == config->link_status_property) {
> > > + state->link_status = val;
>
> I think we should have a check here to make sure userspace never tries to
> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
> atomic userspace is supposed to be able to just restore everything to what
> it was.
>
> I don't think trying to set it to "BAD" should cause an error though, just
> silently keep the link status at "GOOG". So:
>
> /* Never downgrade from GOOD to BAD on userspace's request here,
>* only hw issues can do that. */
> if (state->link_status == GOOD)
> return 0;
> state->link_status = val;
> return 0;
>
> > > + return 0;
> > > } else if (connector->funcs->atomic_set_property) {
> > > return connector->funcs->atomic_set_property(connector,
> > > state, property, val);
> > > @@ -1135,6 +1138,8 @@ static void drm_atomic_connector_print_state(struct
> > > drm_printer *p,
> > > *val = (state->crtc) ? state->crtc->base.id : 0;
> > > } else if (property == config->dpms_property) {
> > > *val = connector->dpms;
> > > + } else if (property == config->link_status_property) {
> > > + *val = state->link_status;
> > > } else if (connector->funcs->atomic_get_property) {
> > > return connector->funcs->atomic_get_property(connector,
> > > state, property, val);
> > > diff --git a/drivers/gpu/drm/drm_connector.c
> > > b/drivers/gpu/drm/drm_connector.c
> > > index 5a45262..4576c9f 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
> > > drm_object_attach_property(>base,
> > > config->dpms_property, 0);
> > >
> > > + drm_object_attach_property(>base,
> > > + config->link_status_property,
> > > + 0);
> > > +
> > > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> > > drm_object_attach_property(>base,
> > > config->prop_crtc_id, 0);
> > > }
> > > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
> > > subpixel_order order)
> > > };
> > > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
> > >
> > > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> > > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
> > > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
> > > +};
> > > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> > > +
> > > /**
> > > * drm_display_info_set_bus_formats - set the supported bus formats
> > > * @info: display info to store bus formats in
> > > @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
> > > drm_display_info *info,
> > > * path property the MST manager created. Userspace cannot change
> > > this
> > > * property.
> > > * TILE:
> > > - * Connector tile group property to indicate how a set of DRM
> > > connector
> > > + * Connector tile group property to indicate how a set of DRM
> > > connector
> >
> > keyboard slip here
> >
> > > * compose together into one logical screen. This is used by both
> > > high-res
> > >
[PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, 23 Nov 2016, Sean Paul wrote:
> On Wed, Nov 23, 2016 at 10:15 AM, Jani Nikula
> wrote:
>> On Wed, 23 Nov 2016, Sean Paul wrote:
>>> On Wed, Nov 23, 2016 at 3:09 AM, Daniel Vetter wrote:
On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
> On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
> wrote:
> > This is RFC patch for adding a connector link-status property
> > and making it atomic by adding it to the drm_connector_state.
> > This is to make sure its wired properly in
> > drm_atomic_connector_set_property
> > and drm_atomic_connector_get_property functions.
> >
>
> Thanks for sending this. We ran into some re-training awkwardness
> recently, and I
> was hoping for such a patch.
>
> > Cc: Jani Nikula
> > Cc: Daniel Vetter
> > Cc: Ville Syrjala
> > Cc: Chris Wilson
> > Signed-off-by: Manasi Navare
> > ---
> > drivers/gpu/drm/drm_atomic.c| 5
> > drivers/gpu/drm/drm_connector.c | 65
> > +++--
> > include/drm/drm_connector.h | 12 +++-
> > include/drm/drm_mode_config.h | 5
> > include/uapi/drm/drm_mode.h | 4 +++
> > 5 files changed, 88 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 89737e4..644d19f 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
> > drm_connector *connector,
> > * now?) atomic writes to DPMS property:
> > */
> > return -EINVAL;
> > + } else if (property == config->link_status_property) {
> > + state->link_status = val;
I think we should have a check here to make sure userspace never tries to
set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
atomic userspace is supposed to be able to just restore everything to what
it was.
I don't think trying to set it to "BAD" should cause an error though, just
silently keep the link status at "GOOG". So:
/* Never downgrade from GOOD to BAD on userspace's request here,
* only hw issues can do that. */
if (state->link_status == GOOD)
return 0;
>>>
>>> Can we return an error if the transition is invalid?
>>>
state->link_status = val;
return 0;
> > + return 0;
> > } else if (connector->funcs->atomic_set_property) {
> > return connector->funcs->atomic_set_property(connector,
> > state, property, val);
> > @@ -1135,6 +1138,8 @@ static void
> > drm_atomic_connector_print_state(struct drm_printer *p,
> > *val = (state->crtc) ? state->crtc->base.id : 0;
> > } else if (property == config->dpms_property) {
> > *val = connector->dpms;
> > + } else if (property == config->link_status_property) {
> > + *val = state->link_status;
> > } else if (connector->funcs->atomic_get_property) {
> > return connector->funcs->atomic_get_property(connector,
> > state, property, val);
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index 5a45262..4576c9f 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
> > drm_object_attach_property(>base,
> > config->dpms_property, 0);
> >
> > + drm_object_attach_property(>base,
> > + config->link_status_property,
> > + 0);
> > +
> > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> > drm_object_attach_property(>base,
> > config->prop_crtc_id, 0);
> > }
> > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
> > subpixel_order order)
> > };
> > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
> >
> > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
> > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
> > +};
> > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> > +
> > /**
> > * drm_display_info_set_bus_formats - set the supported bus formats
> > * @info: display info to store bus formats in
> > @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
> > drm_display_info *info,
> > *
[Intel-gfx] [PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, Nov 23, 2016 at 10:32:44AM -0500, Sean Paul wrote:
> On Wed, Nov 23, 2016 at 10:15 AM, Jani Nikula
> wrote:
> > On Wed, 23 Nov 2016, Sean Paul wrote:
> >> On Wed, Nov 23, 2016 at 3:09 AM, Daniel Vetter wrote:
> >>> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
> On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
> wrote:
> > This is RFC patch for adding a connector link-status property
> > and making it atomic by adding it to the drm_connector_state.
> > This is to make sure its wired properly in
> > drm_atomic_connector_set_property
> > and drm_atomic_connector_get_property functions.
> >
>
> Thanks for sending this. We ran into some re-training awkwardness
> recently, and I
> was hoping for such a patch.
>
> > Cc: Jani Nikula
> > Cc: Daniel Vetter
> > Cc: Ville Syrjala
> > Cc: Chris Wilson
> > Signed-off-by: Manasi Navare
> > ---
> > drivers/gpu/drm/drm_atomic.c| 5
> > drivers/gpu/drm/drm_connector.c | 65
> > +++--
> > include/drm/drm_connector.h | 12 +++-
> > include/drm/drm_mode_config.h | 5
> > include/uapi/drm/drm_mode.h | 4 +++
> > 5 files changed, 88 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c
> > b/drivers/gpu/drm/drm_atomic.c
> > index 89737e4..644d19f 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
> > drm_connector *connector,
> > * now?) atomic writes to DPMS property:
> > */
> > return -EINVAL;
> > + } else if (property == config->link_status_property) {
> > + state->link_status = val;
> >>>
> >>> I think we should have a check here to make sure userspace never tries to
> >>> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
> >>> atomic userspace is supposed to be able to just restore everything to what
> >>> it was.
> >>>
> >>> I don't think trying to set it to "BAD" should cause an error though, just
> >>> silently keep the link status at "GOOG". So:
> >>>
> >>> /* Never downgrade from GOOD to BAD on userspace's request here,
> >>> * only hw issues can do that. */
> >>> if (state->link_status == GOOD)
> >>> return 0;
> >>
> >> Can we return an error if the transition is invalid?
> >>
> >>> state->link_status = val;
> >>> return 0;
> >>>
> > + return 0;
> > } else if (connector->funcs->atomic_set_property) {
> > return connector->funcs->atomic_set_property(connector,
> > state, property, val);
> > @@ -1135,6 +1138,8 @@ static void
> > drm_atomic_connector_print_state(struct drm_printer *p,
> > *val = (state->crtc) ? state->crtc->base.id : 0;
> > } else if (property == config->dpms_property) {
> > *val = connector->dpms;
> > + } else if (property == config->link_status_property) {
> > + *val = state->link_status;
> > } else if (connector->funcs->atomic_get_property) {
> > return connector->funcs->atomic_get_property(connector,
> > state, property, val);
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index 5a45262..4576c9f 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
> > drm_object_attach_property(>base,
> > config->dpms_property, 0);
> >
> > + drm_object_attach_property(>base,
> > + config->link_status_property,
> > + 0);
> > +
> > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> > drm_object_attach_property(>base,
> > config->prop_crtc_id, 0);
> > }
> > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
> > subpixel_order order)
> > };
> > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
> >
> > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
> > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
> > +};
> > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> > +
> > /**
> > * drm_display_info_set_bus_formats - set the supported bus formats
> > * @info: display info to store
[PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, 23 Nov 2016, Sean Paul wrote:
> On Wed, Nov 23, 2016 at 3:09 AM, Daniel Vetter wrote:
>> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
>>> On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
>>> wrote:
>>> > This is RFC patch for adding a connector link-status property
>>> > and making it atomic by adding it to the drm_connector_state.
>>> > This is to make sure its wired properly in
>>> > drm_atomic_connector_set_property
>>> > and drm_atomic_connector_get_property functions.
>>> >
>>>
>>> Thanks for sending this. We ran into some re-training awkwardness
>>> recently, and I
>>> was hoping for such a patch.
>>>
>>> > Cc: Jani Nikula
>>> > Cc: Daniel Vetter
>>> > Cc: Ville Syrjala
>>> > Cc: Chris Wilson
>>> > Signed-off-by: Manasi Navare
>>> > ---
>>> > drivers/gpu/drm/drm_atomic.c| 5
>>> > drivers/gpu/drm/drm_connector.c | 65
>>> > +++--
>>> > include/drm/drm_connector.h | 12 +++-
>>> > include/drm/drm_mode_config.h | 5
>>> > include/uapi/drm/drm_mode.h | 4 +++
>>> > 5 files changed, 88 insertions(+), 3 deletions(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>> > index 89737e4..644d19f 100644
>>> > --- a/drivers/gpu/drm/drm_atomic.c
>>> > +++ b/drivers/gpu/drm/drm_atomic.c
>>> > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
>>> > drm_connector *connector,
>>> > * now?) atomic writes to DPMS property:
>>> > */
>>> > return -EINVAL;
>>> > + } else if (property == config->link_status_property) {
>>> > + state->link_status = val;
>>
>> I think we should have a check here to make sure userspace never tries to
>> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
>> atomic userspace is supposed to be able to just restore everything to what
>> it was.
>>
>> I don't think trying to set it to "BAD" should cause an error though, just
>> silently keep the link status at "GOOG". So:
>>
>> /* Never downgrade from GOOD to BAD on userspace's request here,
>> * only hw issues can do that. */
>> if (state->link_status == GOOD)
>> return 0;
>
> Can we return an error if the transition is invalid?
>
>> state->link_status = val;
>> return 0;
>>
>>> > + return 0;
>>> > } else if (connector->funcs->atomic_set_property) {
>>> > return connector->funcs->atomic_set_property(connector,
>>> > state, property, val);
>>> > @@ -1135,6 +1138,8 @@ static void drm_atomic_connector_print_state(struct
>>> > drm_printer *p,
>>> > *val = (state->crtc) ? state->crtc->base.id : 0;
>>> > } else if (property == config->dpms_property) {
>>> > *val = connector->dpms;
>>> > + } else if (property == config->link_status_property) {
>>> > + *val = state->link_status;
>>> > } else if (connector->funcs->atomic_get_property) {
>>> > return connector->funcs->atomic_get_property(connector,
>>> > state, property, val);
>>> > diff --git a/drivers/gpu/drm/drm_connector.c
>>> > b/drivers/gpu/drm/drm_connector.c
>>> > index 5a45262..4576c9f 100644
>>> > --- a/drivers/gpu/drm/drm_connector.c
>>> > +++ b/drivers/gpu/drm/drm_connector.c
>>> > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
>>> > drm_object_attach_property(>base,
>>> > config->dpms_property, 0);
>>> >
>>> > + drm_object_attach_property(>base,
>>> > + config->link_status_property,
>>> > + 0);
>>> > +
>>> > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>>> > drm_object_attach_property(>base,
>>> > config->prop_crtc_id, 0);
>>> > }
>>> > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
>>> > subpixel_order order)
>>> > };
>>> > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>>> >
>>> > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>>> > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
>>> > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
>>> > +};
>>> > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>>> > +
>>> > /**
>>> > * drm_display_info_set_bus_formats - set the supported bus formats
>>> > * @info: display info to store bus formats in
>>> > @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
>>> > drm_display_info *info,
>>> > * path property the MST manager created. Userspace cannot change
>>> > this
>>> > * property.
>>> > * TILE:
>>> > - * Connector tile group property to indicate how a set of DRM
>>> > connector
>>> > + * Connector tile group property to indicate how a set of DRM
>>> > connector
>>>
>>> keyboard
[PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, Nov 23, 2016 at 4:52 PM, Jani Nikula wrote: >> Take the CDN DP driver in rockchip for example (posted yesterday). >> There's a worker in the driver that is responsible for handling >> hpd/extcon events from the USB-C port. Ideally, this worker should >> just be a thin shell around a kms uevent that lets userspace know >> there's been a change. Unfortunately since we need to make re-training >> work (/me grumbles about productization) seamlessly, we need to add a >> bunch of goo into the worker to handle re-training. Since we need to >> handle re-training there and in modeset, we need to properly >> synchronize things. So we end up with a bunch of code that doesn't >> *really* need to be there. >> >> So is the correct path forward to add GOOD/BAD connection handling to >> Chrome/drm_hwc > > I think so. > >> and rip re-training out of the various kernel drivers? > > IMO if it works, don't change it, at least not in a rush. It also > depends on the hardware and the driver; the amount of code required may > be vastly different for various drivers. > > Personally, I think in the long run letting userspace help here leads to > a simpler design and more efficient happy day scenario handling, and > userspace knows what's going on. We can't rip out re-training from existing drivers since it would break existing userspace in certain cases (like cable re-plug into same screen). But we could try to not implement so much for new drivers. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, Nov 23, 2016 at 10:52 AM, Jani Nikula
wrote:
> On Wed, 23 Nov 2016, Sean Paul wrote:
>> On Wed, Nov 23, 2016 at 10:15 AM, Jani Nikula
>> wrote:
>>> On Wed, 23 Nov 2016, Sean Paul wrote:
On Wed, Nov 23, 2016 at 3:09 AM, Daniel Vetter wrote:
> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
>> On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
>> wrote:
>> > This is RFC patch for adding a connector link-status property
>> > and making it atomic by adding it to the drm_connector_state.
>> > This is to make sure its wired properly in
>> > drm_atomic_connector_set_property
>> > and drm_atomic_connector_get_property functions.
>> >
>>
>> Thanks for sending this. We ran into some re-training awkwardness
>> recently, and I
>> was hoping for such a patch.
>>
>> > Cc: Jani Nikula
>> > Cc: Daniel Vetter
>> > Cc: Ville Syrjala
>> > Cc: Chris Wilson
>> > Signed-off-by: Manasi Navare
>> > ---
>> > drivers/gpu/drm/drm_atomic.c| 5
>> > drivers/gpu/drm/drm_connector.c | 65
>> > +++--
>> > include/drm/drm_connector.h | 12 +++-
>> > include/drm/drm_mode_config.h | 5
>> > include/uapi/drm/drm_mode.h | 4 +++
>> > 5 files changed, 88 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_atomic.c
>> > b/drivers/gpu/drm/drm_atomic.c
>> > index 89737e4..644d19f 100644
>> > --- a/drivers/gpu/drm/drm_atomic.c
>> > +++ b/drivers/gpu/drm/drm_atomic.c
>> > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
>> > drm_connector *connector,
>> > * now?) atomic writes to DPMS property:
>> > */
>> > return -EINVAL;
>> > + } else if (property == config->link_status_property) {
>> > + state->link_status = val;
>
> I think we should have a check here to make sure userspace never tries to
> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
> atomic userspace is supposed to be able to just restore everything to what
> it was.
>
> I don't think trying to set it to "BAD" should cause an error though, just
> silently keep the link status at "GOOG". So:
>
> /* Never downgrade from GOOD to BAD on userspace's request here,
> * only hw issues can do that. */
> if (state->link_status == GOOD)
> return 0;
Can we return an error if the transition is invalid?
> state->link_status = val;
> return 0;
>
>> > + return 0;
>> > } else if (connector->funcs->atomic_set_property) {
>> > return connector->funcs->atomic_set_property(connector,
>> > state, property, val);
>> > @@ -1135,6 +1138,8 @@ static void
>> > drm_atomic_connector_print_state(struct drm_printer *p,
>> > *val = (state->crtc) ? state->crtc->base.id : 0;
>> > } else if (property == config->dpms_property) {
>> > *val = connector->dpms;
>> > + } else if (property == config->link_status_property) {
>> > + *val = state->link_status;
>> > } else if (connector->funcs->atomic_get_property) {
>> > return connector->funcs->atomic_get_property(connector,
>> > state, property, val);
>> > diff --git a/drivers/gpu/drm/drm_connector.c
>> > b/drivers/gpu/drm/drm_connector.c
>> > index 5a45262..4576c9f 100644
>> > --- a/drivers/gpu/drm/drm_connector.c
>> > +++ b/drivers/gpu/drm/drm_connector.c
>> > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
>> > drm_object_attach_property(>base,
>> > config->dpms_property, 0);
>> >
>> > + drm_object_attach_property(>base,
>> > + config->link_status_property,
>> > + 0);
>> > +
>> > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>> > drm_object_attach_property(>base,
>> > config->prop_crtc_id, 0);
>> > }
>> > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
>> > subpixel_order order)
>> > };
>> > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>> >
>> > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>> > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
>> > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
>> > +};
>> > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>> > +
>> > /**
>> > * drm_display_info_set_bus_formats - set the supported bus formats
[PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, Nov 23, 2016 at 11:00:59AM +0100, Daniel Vetter wrote:
> On Tue, Nov 22, 2016 at 05:30:21PM -0800, Manasi Navare wrote:
> > This is RFC patch for adding a connector link-status property
> > and making it atomic by adding it to the drm_connector_state.
> > This is to make sure its wired properly in drm_atomic_connector_set_property
> > and drm_atomic_connector_get_property functions.
> >
> > Cc: Jani Nikula
> > Cc: Daniel Vetter
> > Cc: Ville Syrjala
> > Cc: Chris Wilson
> > Signed-off-by: Manasi Navare
> > ---
> > drivers/gpu/drm/drm_atomic.c| 5
> > drivers/gpu/drm/drm_connector.c | 65
> > +++--
> > include/drm/drm_connector.h | 12 +++-
> > include/drm/drm_mode_config.h | 5
> > include/uapi/drm/drm_mode.h | 4 +++
> > 5 files changed, 88 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 89737e4..644d19f 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
> > drm_connector *connector,
> > * now?) atomic writes to DPMS property:
> > */
> > return -EINVAL;
> > + } else if (property == config->link_status_property) {
> > + state->link_status = val;
> > + return 0;
> > } else if (connector->funcs->atomic_set_property) {
> > return connector->funcs->atomic_set_property(connector,
> > state, property, val);
> > @@ -1135,6 +1138,8 @@ static void drm_atomic_connector_print_state(struct
> > drm_printer *p,
> > *val = (state->crtc) ? state->crtc->base.id : 0;
> > } else if (property == config->dpms_property) {
> > *val = connector->dpms;
> > + } else if (property == config->link_status_property) {
> > + *val = state->link_status;
> > } else if (connector->funcs->atomic_get_property) {
> > return connector->funcs->atomic_get_property(connector,
> > state, property, val);
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index 5a45262..4576c9f 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
> > drm_object_attach_property(>base,
> > config->dpms_property, 0);
> >
> > + drm_object_attach_property(>base,
> > + config->link_status_property,
> > + 0);
> > +
> > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> > drm_object_attach_property(>base,
> > config->prop_crtc_id, 0);
> > }
> > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
> > subpixel_order order)
> > };
> > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
> >
> > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
> > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
> > +};
> > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> > +
> > /**
> > * drm_display_info_set_bus_formats - set the supported bus formats
> > * @info: display info to store bus formats in
> > @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
> > drm_display_info *info,
> > * path property the MST manager created. Userspace cannot change
> > this
> > * property.
> > * TILE:
> > - * Connector tile group property to indicate how a set of DRM
> > connector
> > + * Connector tile group property to indicate how a set of DRM
> > connector
> > * compose together into one logical screen. This is used by both
> > high-res
> > * external screens (often only using a single cable, but exposing
> > multiple
> > * DP MST sinks), or high-res integrated panels (like dual-link
> > DSI) which
> > @@ -625,7 +635,14 @@ int drm_display_info_set_bus_formats(struct
> > drm_display_info *info,
> > * tiling and virtualize both _crtc and _plane if needed.
> > Drivers
> > * should update this value using
> > drm_mode_connector_set_tile_property().
> > * Userspace cannot change this property.
> > - *
> > + * link-status:
> > + * Connector link-status property to indicate the status of link
> > during
> > + * the modeset. The default value of link-status is "GOOD". If
> > something
> > + * fails during modeset, the kernel driver can set this to "BAD",
> > prune
> > + * the mode list based on new link parameters and send a hotplug
> > uevent
> > + * to notify userspace to re-check the valid modes through
> > GET_CONNECTOR
> > + * IOCTL and redo a modeset. Drivers should update this value using
> > + *
[Intel-gfx] [PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, Nov 23, 2016 at 10:43 AM, Ville Syrjälä
wrote:
> On Wed, Nov 23, 2016 at 10:32:44AM -0500, Sean Paul wrote:
>> On Wed, Nov 23, 2016 at 10:15 AM, Jani Nikula
>> wrote:
>> > On Wed, 23 Nov 2016, Sean Paul wrote:
>> >> On Wed, Nov 23, 2016 at 3:09 AM, Daniel Vetter wrote:
>> >>> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
>> On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
>> wrote:
>> > This is RFC patch for adding a connector link-status property
>> > and making it atomic by adding it to the drm_connector_state.
>> > This is to make sure its wired properly in
>> > drm_atomic_connector_set_property
>> > and drm_atomic_connector_get_property functions.
>> >
>>
>> Thanks for sending this. We ran into some re-training awkwardness
>> recently, and I
>> was hoping for such a patch.
>>
>> > Cc: Jani Nikula
>> > Cc: Daniel Vetter
>> > Cc: Ville Syrjala
>> > Cc: Chris Wilson
>> > Signed-off-by: Manasi Navare
>> > ---
>> > drivers/gpu/drm/drm_atomic.c| 5
>> > drivers/gpu/drm/drm_connector.c | 65
>> > +++--
>> > include/drm/drm_connector.h | 12 +++-
>> > include/drm/drm_mode_config.h | 5
>> > include/uapi/drm/drm_mode.h | 4 +++
>> > 5 files changed, 88 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_atomic.c
>> > b/drivers/gpu/drm/drm_atomic.c
>> > index 89737e4..644d19f 100644
>> > --- a/drivers/gpu/drm/drm_atomic.c
>> > +++ b/drivers/gpu/drm/drm_atomic.c
>> > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
>> > drm_connector *connector,
>> > * now?) atomic writes to DPMS property:
>> > */
>> > return -EINVAL;
>> > + } else if (property == config->link_status_property) {
>> > + state->link_status = val;
>> >>>
>> >>> I think we should have a check here to make sure userspace never tries to
>> >>> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since
>> >>> with
>> >>> atomic userspace is supposed to be able to just restore everything to
>> >>> what
>> >>> it was.
>> >>>
>> >>> I don't think trying to set it to "BAD" should cause an error though,
>> >>> just
>> >>> silently keep the link status at "GOOG". So:
>> >>>
>> >>> /* Never downgrade from GOOD to BAD on userspace's request here,
>> >>> * only hw issues can do that. */
>> >>> if (state->link_status == GOOD)
>> >>> return 0;
>> >>
>> >> Can we return an error if the transition is invalid?
>> >>
>> >>> state->link_status = val;
>> >>> return 0;
>> >>>
>> > + return 0;
>> > } else if (connector->funcs->atomic_set_property) {
>> > return
>> > connector->funcs->atomic_set_property(connector,
>> > state, property, val);
>> > @@ -1135,6 +1138,8 @@ static void
>> > drm_atomic_connector_print_state(struct drm_printer *p,
>> > *val = (state->crtc) ? state->crtc->base.id : 0;
>> > } else if (property == config->dpms_property) {
>> > *val = connector->dpms;
>> > + } else if (property == config->link_status_property) {
>> > + *val = state->link_status;
>> > } else if (connector->funcs->atomic_get_property) {
>> > return
>> > connector->funcs->atomic_get_property(connector,
>> > state, property, val);
>> > diff --git a/drivers/gpu/drm/drm_connector.c
>> > b/drivers/gpu/drm/drm_connector.c
>> > index 5a45262..4576c9f 100644
>> > --- a/drivers/gpu/drm/drm_connector.c
>> > +++ b/drivers/gpu/drm/drm_connector.c
>> > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
>> > drm_object_attach_property(>base,
>> > config->dpms_property, 0);
>> >
>> > + drm_object_attach_property(>base,
>> > + config->link_status_property,
>> > + 0);
>> > +
>> > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>> > drm_object_attach_property(>base,
>> > config->prop_crtc_id, 0);
>> > }
>> > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
>> > subpixel_order order)
>> > };
>> > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>> >
>> > +static const struct drm_prop_enum_list drm_link_status_enum_list[] =
>> > {
>> > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
>> > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
>> > +};
>>
[PATCH RFC] drm: Add a new connector atomic property for link status
On Tue, Nov 22, 2016 at 05:30:21PM -0800, Manasi Navare wrote:
> This is RFC patch for adding a connector link-status property
> and making it atomic by adding it to the drm_connector_state.
> This is to make sure its wired properly in drm_atomic_connector_set_property
> and drm_atomic_connector_get_property functions.
>
> Cc: Jani Nikula
> Cc: Daniel Vetter
> Cc: Ville Syrjala
> Cc: Chris Wilson
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/drm_atomic.c| 5
> drivers/gpu/drm/drm_connector.c | 65
> +++--
> include/drm/drm_connector.h | 12 +++-
> include/drm/drm_mode_config.h | 5
> include/uapi/drm/drm_mode.h | 4 +++
> 5 files changed, 88 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 89737e4..644d19f 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector,
>* now?) atomic writes to DPMS property:
>*/
> return -EINVAL;
> + } else if (property == config->link_status_property) {
> + state->link_status = val;
> + return 0;
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -1135,6 +1138,8 @@ static void drm_atomic_connector_print_state(struct
> drm_printer *p,
> *val = (state->crtc) ? state->crtc->base.id : 0;
> } else if (property == config->dpms_property) {
> *val = connector->dpms;
> + } else if (property == config->link_status_property) {
> + *val = state->link_status;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 5a45262..4576c9f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
> drm_object_attach_property(>base,
> config->dpms_property, 0);
>
> + drm_object_attach_property(>base,
> +config->link_status_property,
> +0);
> +
> if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> drm_object_attach_property(>base,
> config->prop_crtc_id, 0);
> }
> @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
> subpixel_order order)
> };
> DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>
> +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
> + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> +
> /**
> * drm_display_info_set_bus_formats - set the supported bus formats
> * @info: display info to store bus formats in
> @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
> drm_display_info *info,
> * path property the MST manager created. Userspace cannot change this
> * property.
> * TILE:
> - * Connector tile group property to indicate how a set of DRM connector
> + * Connector tile group property to indicate how a set of DRM connector
> * compose together into one logical screen. This is used by both high-res
> * external screens (often only using a single cable, but exposing multiple
> * DP MST sinks), or high-res integrated panels (like dual-link DSI) which
> @@ -625,7 +635,14 @@ int drm_display_info_set_bus_formats(struct
> drm_display_info *info,
> * tiling and virtualize both _crtc and _plane if needed. Drivers
> * should update this value using drm_mode_connector_set_tile_property().
> * Userspace cannot change this property.
> - *
> + * link-status:
> + * Connector link-status property to indicate the status of link during
> + * the modeset. The default value of link-status is "GOOD". If something
> + * fails during modeset, the kernel driver can set this to "BAD", prune
> + * the mode list based on new link parameters and send a hotplug uevent
> + * to notify userspace to re-check the valid modes through GET_CONNECTOR
> + * IOCTL and redo a modeset. Drivers should update this value using
> + * drm_mode_connector_set_link_status_property().
> * Connectors also have one standardized atomic property:
> *
> * CRTC_ID:
> @@ -666,6 +683,13 @@ int drm_connector_create_standard_properties(struct
> drm_device *dev)
> return -ENOMEM;
> dev->mode_config.tile_property = prop;
>
> + prop = drm_property_create_enum(dev,
[PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, Nov 23, 2016 at 10:15 AM, Jani Nikula
wrote:
> On Wed, 23 Nov 2016, Sean Paul wrote:
>> On Wed, Nov 23, 2016 at 3:09 AM, Daniel Vetter wrote:
>>> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
wrote:
> This is RFC patch for adding a connector link-status property
> and making it atomic by adding it to the drm_connector_state.
> This is to make sure its wired properly in
> drm_atomic_connector_set_property
> and drm_atomic_connector_get_property functions.
>
Thanks for sending this. We ran into some re-training awkwardness
recently, and I
was hoping for such a patch.
> Cc: Jani Nikula
> Cc: Daniel Vetter
> Cc: Ville Syrjala
> Cc: Chris Wilson
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/drm_atomic.c| 5
> drivers/gpu/drm/drm_connector.c | 65
> +++--
> include/drm/drm_connector.h | 12 +++-
> include/drm/drm_mode_config.h | 5
> include/uapi/drm/drm_mode.h | 4 +++
> 5 files changed, 88 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 89737e4..644d19f 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> * now?) atomic writes to DPMS property:
> */
> return -EINVAL;
> + } else if (property == config->link_status_property) {
> + state->link_status = val;
>>>
>>> I think we should have a check here to make sure userspace never tries to
>>> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
>>> atomic userspace is supposed to be able to just restore everything to what
>>> it was.
>>>
>>> I don't think trying to set it to "BAD" should cause an error though, just
>>> silently keep the link status at "GOOG". So:
>>>
>>> /* Never downgrade from GOOD to BAD on userspace's request here,
>>> * only hw issues can do that. */
>>> if (state->link_status == GOOD)
>>> return 0;
>>
>> Can we return an error if the transition is invalid?
>>
>>> state->link_status = val;
>>> return 0;
>>>
> + return 0;
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -1135,6 +1138,8 @@ static void
> drm_atomic_connector_print_state(struct drm_printer *p,
> *val = (state->crtc) ? state->crtc->base.id : 0;
> } else if (property == config->dpms_property) {
> *val = connector->dpms;
> + } else if (property == config->link_status_property) {
> + *val = state->link_status;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c
> b/drivers/gpu/drm/drm_connector.c
> index 5a45262..4576c9f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
> drm_object_attach_property(>base,
> config->dpms_property, 0);
>
> + drm_object_attach_property(>base,
> + config->link_status_property,
> + 0);
> +
> if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> drm_object_attach_property(>base,
> config->prop_crtc_id, 0);
> }
> @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
> subpixel_order order)
> };
> DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>
> +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
> + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> +
> /**
> * drm_display_info_set_bus_formats - set the supported bus formats
> * @info: display info to store bus formats in
> @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
> drm_display_info *info,
> * path property the MST manager created. Userspace cannot change
> this
> * property.
> * TILE:
> - * Connector tile
[PATCH RFC] drm: Add a new connector atomic property for link status
On Wed, Nov 23, 2016 at 3:09 AM, Daniel Vetter wrote:
> On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
>> On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
>> wrote:
>> > This is RFC patch for adding a connector link-status property
>> > and making it atomic by adding it to the drm_connector_state.
>> > This is to make sure its wired properly in
>> > drm_atomic_connector_set_property
>> > and drm_atomic_connector_get_property functions.
>> >
>>
>> Thanks for sending this. We ran into some re-training awkwardness
>> recently, and I
>> was hoping for such a patch.
>>
>> > Cc: Jani Nikula
>> > Cc: Daniel Vetter
>> > Cc: Ville Syrjala
>> > Cc: Chris Wilson
>> > Signed-off-by: Manasi Navare
>> > ---
>> > drivers/gpu/drm/drm_atomic.c| 5
>> > drivers/gpu/drm/drm_connector.c | 65
>> > +++--
>> > include/drm/drm_connector.h | 12 +++-
>> > include/drm/drm_mode_config.h | 5
>> > include/uapi/drm/drm_mode.h | 4 +++
>> > 5 files changed, 88 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> > index 89737e4..644d19f 100644
>> > --- a/drivers/gpu/drm/drm_atomic.c
>> > +++ b/drivers/gpu/drm/drm_atomic.c
>> > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
>> > drm_connector *connector,
>> > * now?) atomic writes to DPMS property:
>> > */
>> > return -EINVAL;
>> > + } else if (property == config->link_status_property) {
>> > + state->link_status = val;
>
> I think we should have a check here to make sure userspace never tries to
> set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
> atomic userspace is supposed to be able to just restore everything to what
> it was.
>
> I don't think trying to set it to "BAD" should cause an error though, just
> silently keep the link status at "GOOG". So:
>
> /* Never downgrade from GOOD to BAD on userspace's request here,
> * only hw issues can do that. */
> if (state->link_status == GOOD)
> return 0;
Can we return an error if the transition is invalid?
> state->link_status = val;
> return 0;
>
>> > + return 0;
>> > } else if (connector->funcs->atomic_set_property) {
>> > return connector->funcs->atomic_set_property(connector,
>> > state, property, val);
>> > @@ -1135,6 +1138,8 @@ static void drm_atomic_connector_print_state(struct
>> > drm_printer *p,
>> > *val = (state->crtc) ? state->crtc->base.id : 0;
>> > } else if (property == config->dpms_property) {
>> > *val = connector->dpms;
>> > + } else if (property == config->link_status_property) {
>> > + *val = state->link_status;
>> > } else if (connector->funcs->atomic_get_property) {
>> > return connector->funcs->atomic_get_property(connector,
>> > state, property, val);
>> > diff --git a/drivers/gpu/drm/drm_connector.c
>> > b/drivers/gpu/drm/drm_connector.c
>> > index 5a45262..4576c9f 100644
>> > --- a/drivers/gpu/drm/drm_connector.c
>> > +++ b/drivers/gpu/drm/drm_connector.c
>> > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
>> > drm_object_attach_property(>base,
>> > config->dpms_property, 0);
>> >
>> > + drm_object_attach_property(>base,
>> > + config->link_status_property,
>> > + 0);
>> > +
>> > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
>> > drm_object_attach_property(>base,
>> > config->prop_crtc_id, 0);
>> > }
>> > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
>> > subpixel_order order)
>> > };
>> > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>> >
>> > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
>> > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
>> > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
>> > +};
>> > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
>> > +
>> > /**
>> > * drm_display_info_set_bus_formats - set the supported bus formats
>> > * @info: display info to store bus formats in
>> > @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
>> > drm_display_info *info,
>> > * path property the MST manager created. Userspace cannot change this
>> > * property.
>> > * TILE:
>> > - * Connector tile group property to indicate how a set of DRM
>> > connector
>> > + * Connector tile group property to indicate how a set of DRM
>> > connector
>>
>> keyboard slip here
>>
>> > * compose together into one logical screen. This is used by both
>> > high-res
>> > * external screens (often only using a single
[PATCH RFC] drm: Add a new connector atomic property for link status
On Tue, Nov 22, 2016 at 09:27:35PM -0500, Sean Paul wrote:
> On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
> wrote:
> > This is RFC patch for adding a connector link-status property
> > and making it atomic by adding it to the drm_connector_state.
> > This is to make sure its wired properly in drm_atomic_connector_set_property
> > and drm_atomic_connector_get_property functions.
> >
>
> Thanks for sending this. We ran into some re-training awkwardness
> recently, and I
> was hoping for such a patch.
>
> > Cc: Jani Nikula
> > Cc: Daniel Vetter
> > Cc: Ville Syrjala
> > Cc: Chris Wilson
> > Signed-off-by: Manasi Navare
> > ---
> > drivers/gpu/drm/drm_atomic.c| 5
> > drivers/gpu/drm/drm_connector.c | 65
> > +++--
> > include/drm/drm_connector.h | 12 +++-
> > include/drm/drm_mode_config.h | 5
> > include/uapi/drm/drm_mode.h | 4 +++
> > 5 files changed, 88 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 89737e4..644d19f 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
> > drm_connector *connector,
> > * now?) atomic writes to DPMS property:
> > */
> > return -EINVAL;
> > + } else if (property == config->link_status_property) {
> > + state->link_status = val;
I think we should have a check here to make sure userspace never tries to
set this from "GOOD" to "BAD". "BAD" to "BAD" we need to allow, since with
atomic userspace is supposed to be able to just restore everything to what
it was.
I don't think trying to set it to "BAD" should cause an error though, just
silently keep the link status at "GOOG". So:
/* Never downgrade from GOOD to BAD on userspace's request here,
* only hw issues can do that. */
if (state->link_status == GOOD)
return 0;
state->link_status = val;
return 0;
> > + return 0;
> > } else if (connector->funcs->atomic_set_property) {
> > return connector->funcs->atomic_set_property(connector,
> > state, property, val);
> > @@ -1135,6 +1138,8 @@ static void drm_atomic_connector_print_state(struct
> > drm_printer *p,
> > *val = (state->crtc) ? state->crtc->base.id : 0;
> > } else if (property == config->dpms_property) {
> > *val = connector->dpms;
> > + } else if (property == config->link_status_property) {
> > + *val = state->link_status;
> > } else if (connector->funcs->atomic_get_property) {
> > return connector->funcs->atomic_get_property(connector,
> > state, property, val);
> > diff --git a/drivers/gpu/drm/drm_connector.c
> > b/drivers/gpu/drm/drm_connector.c
> > index 5a45262..4576c9f 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
> > drm_object_attach_property(>base,
> > config->dpms_property, 0);
> >
> > + drm_object_attach_property(>base,
> > + config->link_status_property,
> > + 0);
> > +
> > if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> > drm_object_attach_property(>base,
> > config->prop_crtc_id, 0);
> > }
> > @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
> > subpixel_order order)
> > };
> > DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
> >
> > +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> > + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
> > + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
> > +};
> > +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> > +
> > /**
> > * drm_display_info_set_bus_formats - set the supported bus formats
> > * @info: display info to store bus formats in
> > @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
> > drm_display_info *info,
> > * path property the MST manager created. Userspace cannot change this
> > * property.
> > * TILE:
> > - * Connector tile group property to indicate how a set of DRM connector
> > + * Connector tile group property to indicate how a set of DRM
> > connector
>
> keyboard slip here
>
> > * compose together into one logical screen. This is used by both
> > high-res
> > * external screens (often only using a single cable, but exposing
> > multiple
> > * DP MST sinks), or high-res integrated panels (like dual-link DSI)
> > which
> > @@ -625,7 +635,14 @@ int drm_display_info_set_bus_formats(struct
> > drm_display_info *info,
> > * tiling and
[PATCH RFC] drm: Add a new connector atomic property for link status
On Tue, Nov 22, 2016 at 8:30 PM, Manasi Navare
wrote:
> This is RFC patch for adding a connector link-status property
> and making it atomic by adding it to the drm_connector_state.
> This is to make sure its wired properly in drm_atomic_connector_set_property
> and drm_atomic_connector_get_property functions.
>
Thanks for sending this. We ran into some re-training awkwardness
recently, and I
was hoping for such a patch.
> Cc: Jani Nikula
> Cc: Daniel Vetter
> Cc: Ville Syrjala
> Cc: Chris Wilson
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/drm_atomic.c| 5
> drivers/gpu/drm/drm_connector.c | 65
> +++--
> include/drm/drm_connector.h | 12 +++-
> include/drm/drm_mode_config.h | 5
> include/uapi/drm/drm_mode.h | 4 +++
> 5 files changed, 88 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 89737e4..644d19f 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> * now?) atomic writes to DPMS property:
> */
> return -EINVAL;
> + } else if (property == config->link_status_property) {
> + state->link_status = val;
> + return 0;
> } else if (connector->funcs->atomic_set_property) {
> return connector->funcs->atomic_set_property(connector,
> state, property, val);
> @@ -1135,6 +1138,8 @@ static void drm_atomic_connector_print_state(struct
> drm_printer *p,
> *val = (state->crtc) ? state->crtc->base.id : 0;
> } else if (property == config->dpms_property) {
> *val = connector->dpms;
> + } else if (property == config->link_status_property) {
> + *val = state->link_status;
> } else if (connector->funcs->atomic_get_property) {
> return connector->funcs->atomic_get_property(connector,
> state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 5a45262..4576c9f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
> drm_object_attach_property(>base,
> config->dpms_property, 0);
>
> + drm_object_attach_property(>base,
> + config->link_status_property,
> + 0);
> +
> if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> drm_object_attach_property(>base,
> config->prop_crtc_id, 0);
> }
> @@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
> subpixel_order order)
> };
> DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
>
> +static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
> + { DRM_MODE_LINK_STATUS_GOOD, "Good" },
> + { DRM_MODE_LINK_STATUS_BAD, "Bad" },
> +};
> +DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
> +
> /**
> * drm_display_info_set_bus_formats - set the supported bus formats
> * @info: display info to store bus formats in
> @@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
> drm_display_info *info,
> * path property the MST manager created. Userspace cannot change this
> * property.
> * TILE:
> - * Connector tile group property to indicate how a set of DRM connector
> + * Connector tile group property to indicate how a set of DRM connector
keyboard slip here
> * compose together into one logical screen. This is used by both
> high-res
> * external screens (often only using a single cable, but exposing
> multiple
> * DP MST sinks), or high-res integrated panels (like dual-link DSI)
> which
> @@ -625,7 +635,14 @@ int drm_display_info_set_bus_formats(struct
> drm_display_info *info,
> * tiling and virtualize both _crtc and _plane if needed. Drivers
> * should update this value using drm_mode_connector_set_tile_property().
> * Userspace cannot change this property.
> - *
> + * link-status:
> + * Connector link-status property to indicate the status of link during
> + * the modeset. The default value of link-status is "GOOD". If something
> + * fails during modeset, the kernel driver can set this to "BAD", prune
> + * the mode list based on new link parameters and send a hotplug uevent
> + * to notify userspace to re-check the valid modes through GET_CONNECTOR
> + * IOCTL and redo a modeset. Drivers should update this value using
> + * drm_mode_connector_set_link_status_property().
> * Connectors also have one standardized atomic property:
> *
> * CRTC_ID:
> @@ -666,6 +683,13 @@ int
[PATCH RFC] drm: Add a new connector atomic property for link status
This is RFC patch for adding a connector link-status property
and making it atomic by adding it to the drm_connector_state.
This is to make sure its wired properly in drm_atomic_connector_set_property
and drm_atomic_connector_get_property functions.
Cc: Jani Nikula
Cc: Daniel Vetter
Cc: Ville Syrjala
Cc: Chris Wilson
Signed-off-by: Manasi Navare
---
drivers/gpu/drm/drm_atomic.c| 5
drivers/gpu/drm/drm_connector.c | 65 +++--
include/drm/drm_connector.h | 12 +++-
include/drm/drm_mode_config.h | 5
include/uapi/drm/drm_mode.h | 4 +++
5 files changed, 88 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 89737e4..644d19f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1087,6 +1087,9 @@ int drm_atomic_connector_set_property(struct
drm_connector *connector,
* now?) atomic writes to DPMS property:
*/
return -EINVAL;
+ } else if (property == config->link_status_property) {
+ state->link_status = val;
+ return 0;
} else if (connector->funcs->atomic_set_property) {
return connector->funcs->atomic_set_property(connector,
state, property, val);
@@ -1135,6 +1138,8 @@ static void drm_atomic_connector_print_state(struct
drm_printer *p,
*val = (state->crtc) ? state->crtc->base.id : 0;
} else if (property == config->dpms_property) {
*val = connector->dpms;
+ } else if (property == config->link_status_property) {
+ *val = state->link_status;
} else if (connector->funcs->atomic_get_property) {
return connector->funcs->atomic_get_property(connector,
state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 5a45262..4576c9f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -243,6 +243,10 @@ int drm_connector_init(struct drm_device *dev,
drm_object_attach_property(>base,
config->dpms_property, 0);
+ drm_object_attach_property(>base,
+ config->link_status_property,
+ 0);
+
if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
drm_object_attach_property(>base,
config->prop_crtc_id, 0);
}
@@ -506,6 +510,12 @@ const char *drm_get_subpixel_order_name(enum
subpixel_order order)
};
DRM_ENUM_NAME_FN(drm_get_dpms_name, drm_dpms_enum_list)
+static const struct drm_prop_enum_list drm_link_status_enum_list[] = {
+ { DRM_MODE_LINK_STATUS_GOOD, "Good" },
+ { DRM_MODE_LINK_STATUS_BAD, "Bad" },
+};
+DRM_ENUM_NAME_FN(drm_get_link_status_name, drm_link_status_enum_list)
+
/**
* drm_display_info_set_bus_formats - set the supported bus formats
* @info: display info to store bus formats in
@@ -616,7 +626,7 @@ int drm_display_info_set_bus_formats(struct
drm_display_info *info,
* path property the MST manager created. Userspace cannot change this
* property.
* TILE:
- * Connector tile group property to indicate how a set of DRM connector
+ * Connector tile group property to indicate how a set of DRM connector
* compose together into one logical screen. This is used by both high-res
* external screens (often only using a single cable, but exposing multiple
* DP MST sinks), or high-res integrated panels (like dual-link DSI) which
@@ -625,7 +635,14 @@ int drm_display_info_set_bus_formats(struct
drm_display_info *info,
* tiling and virtualize both _crtc and _plane if needed. Drivers
* should update this value using drm_mode_connector_set_tile_property().
* Userspace cannot change this property.
- *
+ * link-status:
+ * Connector link-status property to indicate the status of link during
+ * the modeset. The default value of link-status is "GOOD". If something
+ * fails during modeset, the kernel driver can set this to "BAD", prune
+ * the mode list based on new link parameters and send a hotplug uevent
+ * to notify userspace to re-check the valid modes through GET_CONNECTOR
+ * IOCTL and redo a modeset. Drivers should update this value using
+ * drm_mode_connector_set_link_status_property().
* Connectors also have one standardized atomic property:
*
* CRTC_ID:
@@ -666,6 +683,13 @@ int drm_connector_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.tile_property = prop;
+ prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC,
"link-status",
+ drm_link_status_enum_list,
+ ARRAY_SIZE(drm_link_status_enum_list));
+ if (!prop)
+
