drm/docs: more leftovers from the big vtable documentation pile
diff mbox

Message ID 1452007335-19621-1-git-send-email-daniel.vetter@ffwll.ch
State New
Headers show

Commit Message

Daniel Vetter Jan. 5, 2016, 3:22 p.m. UTC
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>
---
 Documentation/DocBook/gpu.tmpl           | 188 -------------------------------
 include/drm/drm_modeset_helper_vtables.h |  44 +++++++-
 2 files changed, 41 insertions(+), 191 deletions(-)

Comments

Daniel Vetter Jan. 5, 2016, 3:30 p.m. UTC | #1
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-&gt;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-&gt;x</literal> and
> -            <literal>crtc-&gt;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
>

Patch
diff mbox

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-&gt;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-&gt;x</literal> and
-            <literal>crtc-&gt;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