| Message ID | 1452007335-19621-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jan 05, 2016 at 04:22:15PM +0100, Daniel Vetter wrote: > Another pile of vfuncs from the old gpu.tmpl xml documentation that > I've forgotten to delete. I spotted a few more things to > clarify/extend in the new kerneldoc while going through this once > more. > > v2: Spelling fixes (Thierry). > > v3: More spelling fixes and use Thierry's proposal to clarify why > drivers need to validate modes both in ->mode_fixup and ->mode_valid. > > Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Cc: Thierry Reding <treding@nvidia.com> > Acked-by: Thierry Reding <treding@nvidia.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> ... and pulled into drm-misc. -Daniel > --- > Documentation/DocBook/gpu.tmpl | 188 ------------------------------- > include/drm/drm_modeset_helper_vtables.h | 44 +++++++- > 2 files changed, 41 insertions(+), 191 deletions(-) > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index 225a246c5f53..faa5e0d4208d 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -1579,194 +1579,6 @@ void intel_crt_init(struct drm_device *dev) > entities. > </para> > <sect2> > - <title>Legacy CRTC Helper Operations</title> > - <itemizedlist> > - <listitem id="drm-helper-crtc-mode-fixup"> > - <synopsis>bool (*mode_fixup)(struct drm_crtc *crtc, > - const struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode);</synopsis> > - <para> > - Let CRTCs adjust the requested mode or reject it completely. This > - operation returns true if the mode is accepted (possibly after being > - adjusted) or false if it is rejected. > - </para> > - <para> > - The <methodname>mode_fixup</methodname> operation should reject the > - mode if it can't reasonably use it. The definition of "reasonable" > - is currently fuzzy in this context. One possible behaviour would be > - to set the adjusted mode to the panel timings when a fixed-mode > - panel is used with hardware capable of scaling. Another behaviour > - would be to accept any input mode and adjust it to the closest mode > - supported by the hardware (FIXME: This needs to be clarified). > - </para> > - </listitem> > - <listitem> > - <synopsis>int (*mode_set_base)(struct drm_crtc *crtc, int x, int y, > - struct drm_framebuffer *old_fb)</synopsis> > - <para> > - Move the CRTC on the current frame buffer (stored in > - <literal>crtc->fb</literal>) to position (x,y). Any of the frame > - buffer, x position or y position may have been modified. > - </para> > - <para> > - This helper operation is optional. If not provided, the > - <function>drm_crtc_helper_set_config</function> function will fall > - back to the <methodname>mode_set</methodname> helper operation. > - </para> > - <note><para> > - FIXME: Why are x and y passed as arguments, as they can be accessed > - through <literal>crtc->x</literal> and > - <literal>crtc->y</literal>? > - </para></note> > - </listitem> > - <listitem> > - <synopsis>void (*prepare)(struct drm_crtc *crtc);</synopsis> > - <para> > - Prepare the CRTC for mode setting. This operation is called after > - validating the requested mode. Drivers use it to perform > - device-specific operations required before setting the new mode. > - </para> > - </listitem> > - <listitem> > - <synopsis>int (*mode_set)(struct drm_crtc *crtc, struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode, int x, int y, > - struct drm_framebuffer *old_fb);</synopsis> > - <para> > - Set a new mode, position and frame buffer. Depending on the device > - requirements, the mode can be stored internally by the driver and > - applied in the <methodname>commit</methodname> operation, or > - programmed to the hardware immediately. > - </para> > - <para> > - The <methodname>mode_set</methodname> operation returns 0 on success > - or a negative error code if an error occurs. > - </para> > - </listitem> > - <listitem> > - <synopsis>void (*commit)(struct drm_crtc *crtc);</synopsis> > - <para> > - Commit a mode. This operation is called after setting the new mode. > - Upon return the device must use the new mode and be fully > - operational. > - </para> > - </listitem> > - </itemizedlist> > - </sect2> > - <sect2> > - <title>Encoder Helper Operations</title> > - <itemizedlist> > - <listitem> > - <synopsis>bool (*mode_fixup)(struct drm_encoder *encoder, > - const struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode);</synopsis> > - <para> > - Let encoders adjust the requested mode or reject it completely. This > - operation returns true if the mode is accepted (possibly after being > - adjusted) or false if it is rejected. See the > - <link linkend="drm-helper-crtc-mode-fixup">mode_fixup CRTC helper > - operation</link> for an explanation of the allowed adjustments. > - </para> > - </listitem> > - <listitem> > - <synopsis>void (*prepare)(struct drm_encoder *encoder);</synopsis> > - <para> > - Prepare the encoder for mode setting. This operation is called after > - validating the requested mode. Drivers use it to perform > - device-specific operations required before setting the new mode. > - </para> > - </listitem> > - <listitem> > - <synopsis>void (*mode_set)(struct drm_encoder *encoder, > - struct drm_display_mode *mode, > - struct drm_display_mode *adjusted_mode);</synopsis> > - <para> > - Set a new mode. Depending on the device requirements, the mode can > - be stored internally by the driver and applied in the > - <methodname>commit</methodname> operation, or programmed to the > - hardware immediately. > - </para> > - </listitem> > - <listitem> > - <synopsis>void (*commit)(struct drm_encoder *encoder);</synopsis> > - <para> > - Commit a mode. This operation is called after setting the new mode. > - Upon return the device must use the new mode and be fully > - operational. > - </para> > - </listitem> > - </itemizedlist> > - </sect2> > - <sect2> > - <title>Connector Helper Operations</title> > - <itemizedlist> > - <listitem> > - <synopsis>struct drm_encoder *(*best_encoder)(struct drm_connector *connector);</synopsis> > - <para> > - Return a pointer to the best encoder for the connecter. Device that > - map connectors to encoders 1:1 simply return the pointer to the > - associated encoder. This operation is mandatory. > - </para> > - </listitem> > - <listitem> > - <synopsis>int (*get_modes)(struct drm_connector *connector);</synopsis> > - <para> > - Fill the connector's <structfield>probed_modes</structfield> list > - by parsing EDID data with <function>drm_add_edid_modes</function>, > - adding standard VESA DMT modes with <function>drm_add_modes_noedid</function>, > - or calling <function>drm_mode_probed_add</function> directly for every > - supported mode and return the number of modes it has detected. This > - operation is mandatory. > - </para> > - <para> > - Note that the caller function will automatically add standard VESA > - DMT modes up to 1024x768 if the <methodname>get_modes</methodname> > - helper operation returns no mode and if the connector status is > - connector_status_connected. There is no need to call > - <function>drm_add_edid_modes</function> manually in that case. > - </para> > - <para> > - The <structfield>vrefresh</structfield> value is computed by > - <function>drm_helper_probe_single_connector_modes</function>. > - </para> > - <para> > - When parsing EDID data, <function>drm_add_edid_modes</function> fills the > - connector <structfield>display_info</structfield> > - <structfield>width_mm</structfield> and > - <structfield>height_mm</structfield> fields. When creating modes > - manually the <methodname>get_modes</methodname> helper operation must > - set the <structfield>display_info</structfield> > - <structfield>width_mm</structfield> and > - <structfield>height_mm</structfield> fields if they haven't been set > - already (for instance at initialization time when a fixed-size panel is > - attached to the connector). The mode <structfield>width_mm</structfield> > - and <structfield>height_mm</structfield> fields are only used internally > - during EDID parsing and should not be set when creating modes manually. > - </para> > - </listitem> > - <listitem> > - <synopsis>int (*mode_valid)(struct drm_connector *connector, > - struct drm_display_mode *mode);</synopsis> > - <para> > - Verify whether a mode is valid for the connector. Return MODE_OK for > - supported modes and one of the enum drm_mode_status values (MODE_*) > - for unsupported modes. This operation is optional. > - </para> > - <para> > - As the mode rejection reason is currently not used beside for > - immediately removing the unsupported mode, an implementation can > - return MODE_BAD regardless of the exact reason why the mode is not > - valid. > - </para> > - <note><para> > - Note that the <methodname>mode_valid</methodname> helper operation is > - only called for modes detected by the device, and > - <emphasis>not</emphasis> for modes set by the user through the CRTC > - <methodname>set_config</methodname> operation. > - </para></note> > - </listitem> > - </itemizedlist> > - </sect2> > - <sect2> > <title>Atomic Modeset Helper Functions Reference</title> > <sect3> > <title>Overview</title> > diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h > index 29e0dc50031d..a126a0d7aed4 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -131,6 +131,20 @@ struct drm_crtc_helper_funcs { > * Atomic drivers which need to inspect and adjust more state should > * instead use the @atomic_check callback. > * > + * Also beware that neither core nor helpers filter modes before > + * passing them to the driver: While the list of modes that is > + * advertised to userspace is filtered using the connector's > + * ->mode_valid() callback, neither the core nor the helpers do any > + * filtering on modes passed in from userspace when setting a mode. It > + * is therefore possible for userspace to pass in a mode that was > + * previously filtered out using ->mode_valid() or add a custom mode > + * that wasn't probed from EDID or similar to begin with. Even though > + * this is an advanced feature and rarely used nowadays, some users rely > + * on being able to specify modes manually so drivers must be prepared > + * to deal with it. Specifically this means that all drivers need not > + * only validate modes in ->mode_valid() but also in ->mode_fixup() to > + * make sure invalid modes passed in from userspace are rejected. > + * > * RETURNS: > * > * True if an acceptable configuration is possible, false if the modeset > @@ -188,7 +202,9 @@ struct drm_crtc_helper_funcs { > * This callback is used by the legacy CRTC helpers to set a new > * framebuffer and scanout position. It is optional and used as an > * optimized fast-path instead of a full mode set operation with all the > - * resulting flickering. Since it can't update other planes it's > + * resulting flickering. If it is not present > + * drm_crtc_helper_set_config() will fall back to a full modeset, using > + * the ->mode_set() callback. Since it can't update other planes it's > * incompatible with atomic modeset support. > * > * This callback is only used by the CRTC helpers and deprecated. > @@ -439,6 +455,20 @@ struct drm_encoder_helper_funcs { > * Atomic drivers which need to inspect and adjust more state should > * instead use the @atomic_check callback. > * > + * Also beware that neither core nor helpers filter modes before > + * passing them to the driver: While the list of modes that is > + * advertised to userspace is filtered using the connector's > + * ->mode_valid() callback, neither the core nor the helpers do any > + * filtering on modes passed in from userspace when setting a mode. It > + * is therefore possible for userspace to pass in a mode that was > + * previously filtered out using ->mode_valid() or add a custom mode > + * that wasn't probed from EDID or similar to begin with. Even though > + * this is an advanced feature and rarely used nowadays, some users rely > + * on being able to specify modes manually so drivers must be prepared > + * to deal with it. Specifically this means that all drivers need not > + * only validate modes in ->mode_valid() but also in ->mode_fixup() to > + * make sure invalid modes passed in from userspace are rejected. > + * > * RETURNS: > * > * True if an acceptable configuration is possible, false if the modeset > @@ -640,8 +670,16 @@ struct drm_connector_helper_funcs { > * In this function drivers then parse the modes in the EDID and add > * them by calling drm_add_edid_modes(). But connectors that driver a > * fixed panel can also manually add specific modes using > - * drm_mode_probed_add(). Finally drivers that support audio probably > - * want to update the ELD data, too, using drm_edid_to_eld(). > + * drm_mode_probed_add(). Drivers which manually add modes should also > + * make sure that the @display_info, @width_mm and @height_mm fields of the > + * struct #drm_connector are filled in. > + * > + * Virtual drivers that just want some standard VESA mode with a given > + * resolution can call drm_add_modes_noedid(), and mark the preferred > + * one using drm_set_preferred_mode(). > + * > + * Finally drivers that support audio probably want to update the ELD > + * data, too, using drm_edid_to_eld(). > * > * This function is only called after the ->detect() hook has indicated > * that a sink is connected and when the EDID isn't overridden through > -- > 2.6.4 >
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 225a246c5f53..faa5e0d4208d 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -1579,194 +1579,6 @@ void intel_crt_init(struct drm_device *dev) entities. </para> <sect2> - <title>Legacy CRTC Helper Operations</title> - <itemizedlist> - <listitem id="drm-helper-crtc-mode-fixup"> - <synopsis>bool (*mode_fixup)(struct drm_crtc *crtc, - const struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode);</synopsis> - <para> - Let CRTCs adjust the requested mode or reject it completely. This - operation returns true if the mode is accepted (possibly after being - adjusted) or false if it is rejected. - </para> - <para> - The <methodname>mode_fixup</methodname> operation should reject the - mode if it can't reasonably use it. The definition of "reasonable" - is currently fuzzy in this context. One possible behaviour would be - to set the adjusted mode to the panel timings when a fixed-mode - panel is used with hardware capable of scaling. Another behaviour - would be to accept any input mode and adjust it to the closest mode - supported by the hardware (FIXME: This needs to be clarified). - </para> - </listitem> - <listitem> - <synopsis>int (*mode_set_base)(struct drm_crtc *crtc, int x, int y, - struct drm_framebuffer *old_fb)</synopsis> - <para> - Move the CRTC on the current frame buffer (stored in - <literal>crtc->fb</literal>) to position (x,y). Any of the frame - buffer, x position or y position may have been modified. - </para> - <para> - This helper operation is optional. If not provided, the - <function>drm_crtc_helper_set_config</function> function will fall - back to the <methodname>mode_set</methodname> helper operation. - </para> - <note><para> - FIXME: Why are x and y passed as arguments, as they can be accessed - through <literal>crtc->x</literal> and - <literal>crtc->y</literal>? - </para></note> - </listitem> - <listitem> - <synopsis>void (*prepare)(struct drm_crtc *crtc);</synopsis> - <para> - Prepare the CRTC for mode setting. This operation is called after - validating the requested mode. Drivers use it to perform - device-specific operations required before setting the new mode. - </para> - </listitem> - <listitem> - <synopsis>int (*mode_set)(struct drm_crtc *crtc, struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode, int x, int y, - struct drm_framebuffer *old_fb);</synopsis> - <para> - Set a new mode, position and frame buffer. Depending on the device - requirements, the mode can be stored internally by the driver and - applied in the <methodname>commit</methodname> operation, or - programmed to the hardware immediately. - </para> - <para> - The <methodname>mode_set</methodname> operation returns 0 on success - or a negative error code if an error occurs. - </para> - </listitem> - <listitem> - <synopsis>void (*commit)(struct drm_crtc *crtc);</synopsis> - <para> - Commit a mode. This operation is called after setting the new mode. - Upon return the device must use the new mode and be fully - operational. - </para> - </listitem> - </itemizedlist> - </sect2> - <sect2> - <title>Encoder Helper Operations</title> - <itemizedlist> - <listitem> - <synopsis>bool (*mode_fixup)(struct drm_encoder *encoder, - const struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode);</synopsis> - <para> - Let encoders adjust the requested mode or reject it completely. This - operation returns true if the mode is accepted (possibly after being - adjusted) or false if it is rejected. See the - <link linkend="drm-helper-crtc-mode-fixup">mode_fixup CRTC helper - operation</link> for an explanation of the allowed adjustments. - </para> - </listitem> - <listitem> - <synopsis>void (*prepare)(struct drm_encoder *encoder);</synopsis> - <para> - Prepare the encoder for mode setting. This operation is called after - validating the requested mode. Drivers use it to perform - device-specific operations required before setting the new mode. - </para> - </listitem> - <listitem> - <synopsis>void (*mode_set)(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode);</synopsis> - <para> - Set a new mode. Depending on the device requirements, the mode can - be stored internally by the driver and applied in the - <methodname>commit</methodname> operation, or programmed to the - hardware immediately. - </para> - </listitem> - <listitem> - <synopsis>void (*commit)(struct drm_encoder *encoder);</synopsis> - <para> - Commit a mode. This operation is called after setting the new mode. - Upon return the device must use the new mode and be fully - operational. - </para> - </listitem> - </itemizedlist> - </sect2> - <sect2> - <title>Connector Helper Operations</title> - <itemizedlist> - <listitem> - <synopsis>struct drm_encoder *(*best_encoder)(struct drm_connector *connector);</synopsis> - <para> - Return a pointer to the best encoder for the connecter. Device that - map connectors to encoders 1:1 simply return the pointer to the - associated encoder. This operation is mandatory. - </para> - </listitem> - <listitem> - <synopsis>int (*get_modes)(struct drm_connector *connector);</synopsis> - <para> - Fill the connector's <structfield>probed_modes</structfield> list - by parsing EDID data with <function>drm_add_edid_modes</function>, - adding standard VESA DMT modes with <function>drm_add_modes_noedid</function>, - or calling <function>drm_mode_probed_add</function> directly for every - supported mode and return the number of modes it has detected. This - operation is mandatory. - </para> - <para> - Note that the caller function will automatically add standard VESA - DMT modes up to 1024x768 if the <methodname>get_modes</methodname> - helper operation returns no mode and if the connector status is - connector_status_connected. There is no need to call - <function>drm_add_edid_modes</function> manually in that case. - </para> - <para> - The <structfield>vrefresh</structfield> value is computed by - <function>drm_helper_probe_single_connector_modes</function>. - </para> - <para> - When parsing EDID data, <function>drm_add_edid_modes</function> fills the - connector <structfield>display_info</structfield> - <structfield>width_mm</structfield> and - <structfield>height_mm</structfield> fields. When creating modes - manually the <methodname>get_modes</methodname> helper operation must - set the <structfield>display_info</structfield> - <structfield>width_mm</structfield> and - <structfield>height_mm</structfield> fields if they haven't been set - already (for instance at initialization time when a fixed-size panel is - attached to the connector). The mode <structfield>width_mm</structfield> - and <structfield>height_mm</structfield> fields are only used internally - during EDID parsing and should not be set when creating modes manually. - </para> - </listitem> - <listitem> - <synopsis>int (*mode_valid)(struct drm_connector *connector, - struct drm_display_mode *mode);</synopsis> - <para> - Verify whether a mode is valid for the connector. Return MODE_OK for - supported modes and one of the enum drm_mode_status values (MODE_*) - for unsupported modes. This operation is optional. - </para> - <para> - As the mode rejection reason is currently not used beside for - immediately removing the unsupported mode, an implementation can - return MODE_BAD regardless of the exact reason why the mode is not - valid. - </para> - <note><para> - Note that the <methodname>mode_valid</methodname> helper operation is - only called for modes detected by the device, and - <emphasis>not</emphasis> for modes set by the user through the CRTC - <methodname>set_config</methodname> operation. - </para></note> - </listitem> - </itemizedlist> - </sect2> - <sect2> <title>Atomic Modeset Helper Functions Reference</title> <sect3> <title>Overview</title> diff --git a/include/drm/drm_modeset_helper_vtables.h b/include/drm/drm_modeset_helper_vtables.h index 29e0dc50031d..a126a0d7aed4 100644 --- a/include/drm/drm_modeset_helper_vtables.h +++ b/include/drm/drm_modeset_helper_vtables.h @@ -131,6 +131,20 @@ struct drm_crtc_helper_funcs { * Atomic drivers which need to inspect and adjust more state should * instead use the @atomic_check callback. * + * Also beware that neither core nor helpers filter modes before + * passing them to the driver: While the list of modes that is + * advertised to userspace is filtered using the connector's + * ->mode_valid() callback, neither the core nor the helpers do any + * filtering on modes passed in from userspace when setting a mode. It + * is therefore possible for userspace to pass in a mode that was + * previously filtered out using ->mode_valid() or add a custom mode + * that wasn't probed from EDID or similar to begin with. Even though + * this is an advanced feature and rarely used nowadays, some users rely + * on being able to specify modes manually so drivers must be prepared + * to deal with it. Specifically this means that all drivers need not + * only validate modes in ->mode_valid() but also in ->mode_fixup() to + * make sure invalid modes passed in from userspace are rejected. + * * RETURNS: * * True if an acceptable configuration is possible, false if the modeset @@ -188,7 +202,9 @@ struct drm_crtc_helper_funcs { * This callback is used by the legacy CRTC helpers to set a new * framebuffer and scanout position. It is optional and used as an * optimized fast-path instead of a full mode set operation with all the - * resulting flickering. Since it can't update other planes it's + * resulting flickering. If it is not present + * drm_crtc_helper_set_config() will fall back to a full modeset, using + * the ->mode_set() callback. Since it can't update other planes it's * incompatible with atomic modeset support. * * This callback is only used by the CRTC helpers and deprecated. @@ -439,6 +455,20 @@ struct drm_encoder_helper_funcs { * Atomic drivers which need to inspect and adjust more state should * instead use the @atomic_check callback. * + * Also beware that neither core nor helpers filter modes before + * passing them to the driver: While the list of modes that is + * advertised to userspace is filtered using the connector's + * ->mode_valid() callback, neither the core nor the helpers do any + * filtering on modes passed in from userspace when setting a mode. It + * is therefore possible for userspace to pass in a mode that was + * previously filtered out using ->mode_valid() or add a custom mode + * that wasn't probed from EDID or similar to begin with. Even though + * this is an advanced feature and rarely used nowadays, some users rely + * on being able to specify modes manually so drivers must be prepared + * to deal with it. Specifically this means that all drivers need not + * only validate modes in ->mode_valid() but also in ->mode_fixup() to + * make sure invalid modes passed in from userspace are rejected. + * * RETURNS: * * True if an acceptable configuration is possible, false if the modeset @@ -640,8 +670,16 @@ struct drm_connector_helper_funcs { * In this function drivers then parse the modes in the EDID and add * them by calling drm_add_edid_modes(). But connectors that driver a * fixed panel can also manually add specific modes using - * drm_mode_probed_add(). Finally drivers that support audio probably - * want to update the ELD data, too, using drm_edid_to_eld(). + * drm_mode_probed_add(). Drivers which manually add modes should also + * make sure that the @display_info, @width_mm and @height_mm fields of the + * struct #drm_connector are filled in. + * + * Virtual drivers that just want some standard VESA mode with a given + * resolution can call drm_add_modes_noedid(), and mark the preferred + * one using drm_set_preferred_mode(). + * + * Finally drivers that support audio probably want to update the ELD + * data, too, using drm_edid_to_eld(). * * This function is only called after the ->detect() hook has indicated * that a sink is connected and when the EDID isn't overridden through