| Message ID | 20170515091136.26307-1-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Daniel, On 15-05-2017 10:11, Daniel Vetter wrote: > Laurent started a massive discussion on IRC about this. Let's try to > document common usage a bit better. > > v2: Cross-links+typos. > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Jose Abreu <Jose.Abreu@synopsys.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Jose Abreu <joabreu@synopsys.com> Best regards, Jose Miguel Abreu > --- > include/drm/drm_bridge.h | 2 +- > include/drm/drm_crtc.h | 28 +++++++++++++++++++++++++--- > include/drm/drm_modeset_helper_vtables.h | 6 ++++-- > 3 files changed, 30 insertions(+), 6 deletions(-) > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index f694de756ecf..f3ad38d0d621 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -91,7 +91,7 @@ struct drm_bridge_funcs { > * the display chain, either the final &drm_connector or the next > * &drm_bridge. The parameter adjusted_mode is the input mode the bridge > * requires. It can be modified by this callback and does not need to > - * match mode. > + * match mode. See also &drm_crtc_state.adjusted_mode for more details. > * > * This is the only hook that allows a bridge to reject a modeset. If > * this function passes all other callbacks must succeed for this > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h > index 06236b002c22..5f5d53973ca5 100644 > --- a/include/drm/drm_crtc.h > +++ b/include/drm/drm_crtc.h > @@ -90,8 +90,6 @@ struct drm_plane_helper_funcs; > * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes > * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors > * @encoder_mask: bitmask of (1 << drm_encoder_index(encoder)) of attached encoders > - * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings > - * @mode: current mode timings > * @mode_blob: &drm_property_blob for @mode > * @state: backpointer to global drm_atomic_state > * > @@ -131,9 +129,33 @@ struct drm_crtc_state { > u32 connector_mask; > u32 encoder_mask; > > - /* adjusted_mode: for use by helpers and drivers */ > + /** > + * @adjusted_mode: > + * > + * Internal display timings which can be used by the driver to handle > + * differences between the mode requested by userspace in @mode and what > + * is actually programmed into the hardware. It is purely driver > + * implementation defined what exactly this adjusted mode means. Usually > + * it is used to store the hardware display timings used between the > + * CRTC and encoder blocks. > + */ > struct drm_display_mode adjusted_mode; > > + /** > + * @mode: > + * > + * Display timings requested by userspace. The driver should try to > + * match the refresh rate as close as possible (but note that it's > + * undefined what exactly is close enough, e.g. some of the HDMI modes > + * only differ in less than 1% of the refresh rate). The active width > + * and height as observed by userspace for positioning planes must match > + * exactly. > + * > + * For external connectors where the sink isn't fixed (like with a > + * built-in panel), this mode here should match the physical mode on the > + * wire to the last details (i.e. including sync polarities and > + * everything). > + */ > struct drm_display_mode mode; > > /* blob property to expose current mode to atomic userspace */ > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index 91d071ff1232..c72fca544a41 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -138,7 +138,8 @@ struct drm_crtc_helper_funcs { > * encoders need to be fed with. Note that this is the inverse semantics > * of the meaning for the &drm_encoder and &drm_bridge_funcs.mode_fixup > * vfunc. If the CRTC cannot support the requested conversion from mode > - * to adjusted_mode it should reject the modeset. > + * to adjusted_mode it should reject the modeset. See also > + * &drm_crtc_state.adjusted_mode for more details. > * > * This function is used by both legacy CRTC helpers and atomic helpers. > * With atomic helpers it is optional. > @@ -510,7 +511,8 @@ struct drm_encoder_helper_funcs { > * mode is the display mode that should be fed to the next element in > * the display chain, either the final &drm_connector or a &drm_bridge. > * The parameter adjusted_mode is the input mode the encoder requires. It > - * can be modified by this callback and does not need to match mode. > + * can be modified by this callback and does not need to match mode. See > + * also &drm_crtc_state.adjusted_mode for more details. > * > * This function is used by both legacy CRTC helpers and atomic helpers. > * This hook is optional.
On 15.05.2017 11:11, Daniel Vetter wrote: > Laurent started a massive discussion on IRC about this. Let's try to > document common usage a bit better. > > v2: Cross-links+typos. > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Jose Abreu <Jose.Abreu@synopsys.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> Reviewed-by: Andrzej Hajda <a.hajda@samsung.com> -- Regards Andrzej
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h index f694de756ecf..f3ad38d0d621 100644 --- a/include/drm/drm_bridge.h +++ b/include/drm/drm_bridge.h @@ -91,7 +91,7 @@ struct drm_bridge_funcs { * the display chain, either the final &drm_connector or the next * &drm_bridge. The parameter adjusted_mode is the input mode the bridge * requires. It can be modified by this callback and does not need to - * match mode. + * match mode. See also &drm_crtc_state.adjusted_mode for more details. * * This is the only hook that allows a bridge to reject a modeset. If * this function passes all other callbacks must succeed for this diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 06236b002c22..5f5d53973ca5 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -90,8 +90,6 @@ struct drm_plane_helper_funcs; * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors * @encoder_mask: bitmask of (1 << drm_encoder_index(encoder)) of attached encoders - * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings - * @mode: current mode timings * @mode_blob: &drm_property_blob for @mode * @state: backpointer to global drm_atomic_state * @@ -131,9 +129,33 @@ struct drm_crtc_state { u32 connector_mask; u32 encoder_mask; - /* adjusted_mode: for use by helpers and drivers */ + /** + * @adjusted_mode: + * + * Internal display timings which can be used by the driver to handle + * differences between the mode requested by userspace in @mode and what + * is actually programmed into the hardware. It is purely driver + * implementation defined what exactly this adjusted mode means. Usually + * it is used to store the hardware display timings used between the + * CRTC and encoder blocks. + */ struct drm_display_mode adjusted_mode; + /** + * @mode: + * + * Display timings requested by userspace. The driver should try to + * match the refresh rate as close as possible (but note that it's + * undefined what exactly is close enough, e.g. some of the HDMI modes + * only differ in less than 1% of the refresh rate). The active width + * and height as observed by userspace for positioning planes must match + * exactly. + * + * For external connectors where the sink isn't fixed (like with a + * built-in panel), this mode here should match the physical mode on the + * wire to the last details (i.e. including sync polarities and + * everything). + */ struct drm_display_mode mode; /* blob property to expose current mode to atomic userspace */ diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 91d071ff1232..c72fca544a41 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -138,7 +138,8 @@ struct drm_crtc_helper_funcs { * encoders need to be fed with. Note that this is the inverse semantics * of the meaning for the &drm_encoder and &drm_bridge_funcs.mode_fixup * vfunc. If the CRTC cannot support the requested conversion from mode - * to adjusted_mode it should reject the modeset. + * to adjusted_mode it should reject the modeset. See also + * &drm_crtc_state.adjusted_mode for more details. * * This function is used by both legacy CRTC helpers and atomic helpers. * With atomic helpers it is optional. @@ -510,7 +511,8 @@ struct drm_encoder_helper_funcs { * mode is the display mode that should be fed to the next element in * the display chain, either the final &drm_connector or a &drm_bridge. * The parameter adjusted_mode is the input mode the encoder requires. It - * can be modified by this callback and does not need to match mode. + * can be modified by this callback and does not need to match mode. See + * also &drm_crtc_state.adjusted_mode for more details. * * This function is used by both legacy CRTC helpers and atomic helpers. * This hook is optional.
Laurent started a massive discussion on IRC about this. Let's try to document common usage a bit better. v2: Cross-links+typos. Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Jose Abreu <Jose.Abreu@synopsys.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- include/drm/drm_bridge.h | 2 +- include/drm/drm_crtc.h | 28 +++++++++++++++++++++++++--- include/drm/drm_modeset_helper_vtables.h | 6 ++++-- 3 files changed, 30 insertions(+), 6 deletions(-)