It's mandatory and considered core state since ioctls rely on this working.
Thanks to Laurent for pointing out this gap.
v2: Clarify to "atomic drivers" only.
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Sean Paul sean@poorly.run Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- include/drm/drm_connector.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 02a131202add..f43f40d5888a 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -517,6 +517,10 @@ struct drm_connector_state { * Used by the atomic helpers to select the encoder, through the * &drm_connector_helper_funcs.atomic_best_encoder or * &drm_connector_helper_funcs.best_encoder callbacks. + * + * NOTE: Atomic drivers must fill this out (either themselves or through + * helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will + * not return correct data to userspace. */ struct drm_encoder *best_encoder;
Hi Daniel,
Thank you for the patch.
On Mon, May 06, 2019 at 04:46:29PM +0200, Daniel Vetter wrote:
It's mandatory and considered core state since ioctls rely on this working.
Thanks to Laurent for pointing out this gap.
v2: Clarify to "atomic drivers" only.
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Sean Paul sean@poorly.run Signed-off-by: Daniel Vetter daniel.vetter@intel.com
include/drm/drm_connector.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 02a131202add..f43f40d5888a 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -517,6 +517,10 @@ struct drm_connector_state { * Used by the atomic helpers to select the encoder, through the * &drm_connector_helper_funcs.atomic_best_encoder or * &drm_connector_helper_funcs.best_encoder callbacks.
How about updating this part as well ?
"Used by both the DRM core and the atomic helpers to select the encoder (through the &drm_connector_helper_funcs.atomic_best_encoder), access it and report it to userspace (through the GETCONNECTOR and GETENCODER ioctls). This field shall be set by all atomic drivers, either directly or through atomic helpers."
** NOTE: Atomic drivers must fill this out (either themselves or through* helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will* not return correct data to userspace.
On Mon, May 06, 2019 at 05:57:53PM +0300, Laurent Pinchart wrote:
Hi Daniel,
Thank you for the patch.
On Mon, May 06, 2019 at 04:46:29PM +0200, Daniel Vetter wrote:
It's mandatory and considered core state since ioctls rely on this working.
Thanks to Laurent for pointing out this gap.
v2: Clarify to "atomic drivers" only.
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Sean Paul sean@poorly.run Signed-off-by: Daniel Vetter daniel.vetter@intel.com
include/drm/drm_connector.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 02a131202add..f43f40d5888a 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -517,6 +517,10 @@ struct drm_connector_state { * Used by the atomic helpers to select the encoder, through the * &drm_connector_helper_funcs.atomic_best_encoder or * &drm_connector_helper_funcs.best_encoder callbacks.
How about updating this part as well ?
"Used by both the DRM core and the atomic helpers to select the encoder (through the &drm_connector_helper_funcs.atomic_best_encoder), access it and report it to userspace (through the GETCONNECTOR and GETENCODER ioctls). This field shall be set by all atomic drivers, either directly or through atomic helpers."
It's kinda two things, I think best to describe in two paragraphs. But I can move the core usage up, since arguable more important. Otoh most drivers won't care since helpers handle this, and they care more about how @best_encoder is used.
E.g. core never calls the helper callbacks @atomic_best_endoer/best_encoder, which isn't clear anymore with your wording. And I have a sticker for core/helper splits :-) -Daniel
** NOTE: Atomic drivers must fill this out (either themselves or through* helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will* not return correct data to userspace.-- Regards,
Laurent Pinchart
On Mon, May 06, 2019 at 05:27:24PM +0200, Daniel Vetter wrote:
On Mon, May 06, 2019 at 05:57:53PM +0300, Laurent Pinchart wrote:
Hi Daniel,
Thank you for the patch.
On Mon, May 06, 2019 at 04:46:29PM +0200, Daniel Vetter wrote:
It's mandatory and considered core state since ioctls rely on this working.
Thanks to Laurent for pointing out this gap.
v2: Clarify to "atomic drivers" only.
Cc: Laurent Pinchart laurent.pinchart@ideasonboard.com Cc: Sean Paul sean@poorly.run Signed-off-by: Daniel Vetter daniel.vetter@intel.com
include/drm/drm_connector.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 02a131202add..f43f40d5888a 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -517,6 +517,10 @@ struct drm_connector_state { * Used by the atomic helpers to select the encoder, through the * &drm_connector_helper_funcs.atomic_best_encoder or * &drm_connector_helper_funcs.best_encoder callbacks.
How about updating this part as well ?
"Used by both the DRM core and the atomic helpers to select the encoder (through the &drm_connector_helper_funcs.atomic_best_encoder), access it and report it to userspace (through the GETCONNECTOR and GETENCODER ioctls). This field shall be set by all atomic drivers, either directly or through atomic helpers."
It's kinda two things, I think best to describe in two paragraphs. But I can move the core usage up, since arguable more important. Otoh most drivers won't care since helpers handle this, and they care more about how @best_encoder is used.
E.g. core never calls the helper callbacks @atomic_best_endoer/best_encoder, which isn't clear anymore with your wording. And I have a sticker for core/helper splits :-)
Went ahead with Sean's irc ack and merged this. We can bikeshed more later on I guess with more patches. I think sprinkling a pile of cross references once Sean's and yours patches all land would be good. -Daniel
-Daniel
** NOTE: Atomic drivers must fill this out (either themselves or through* helpers), for otherwise the GETCONNECTOR and GETENCODER IOCTLs will* not return correct data to userspace.-- Regards,
Laurent Pinchart
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Daniel Vetter -
Laurent Pinchart