lkml.org 
[lkml]   [2018]   [Apr]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH] drm: clarify adjusted_mode for a bridge connected to a crtc
    Date
    Hi Laurent,

    On 04/06/2018 04:53 PM, Laurent Pinchart wrote:
    > Hi Philippe,
    >
    > Thank you for the patch.
    >
    > On Monday, 26 February 2018 14:16:04 EEST Philippe Cornu wrote:
    >> This patch clarifies the adjusted_mode documentation
    >> for a bridge directly connected to a crtc.
    >>
    >> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
    >> ---
    >> This patch is linked to the discussion https://lkml.org/lkml/2018/1/25/367
    >>
    >> include/drm/drm_bridge.h | 3 ++-
    >> 1 file changed, 2 insertions(+), 1 deletion(-)
    >>
    >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
    >> index 3270fec46979..b5f3c070467c 100644
    >> --- a/include/drm/drm_bridge.h
    >> +++ b/include/drm/drm_bridge.h
    >> @@ -177,7 +177,8 @@ struct drm_bridge_funcs {
    >> * pipeline has been called already. If the bridge is the first element
    >> * then this would be &drm_encoder_helper_funcs.mode_set. The display
    >> * pipe (i.e. clocks and timing signals) is off when this function is
    >> - * called.
    >> + * called. If the bridge is connected to the crtc, the adjusted_mode
    >> + * parameter is the one defined in &drm_crtc_state.adjusted_mode.
    >
    > Unless I'm mistaken this will always be the mode stored in
    > &drm_crtc_state.adjusted_mode (at least for atomic drivers), regardless of
    > whether the bridge is the first in the chain (connected to the CRTC) or not.
    > What is important to document is that we have a single adjusted_mode for the
    > whole chain of bridges, and that it corresponds to the mode output by the CRTC
    > for the first bridge. Bridges further in the chain can look at that mode,
    > although there will probably be nothing of interest to them there.
    >
    > How about the following text ?
    >
    > /**
    > * @mode_set:
    > *
    > * This callback should set the given mode on the bridge. It is called
    > * after the @mode_set callback for the preceding element in the display
    > * pipeline has been called already. If the bridge is the first element
    > * then this would be &drm_encoder_helper_funcs.mode_set. The display
    > * pipe (i.e. clocks and timing signals) is off when this function is
    > * called.
    > *
    > * The adjusted_mode parameter corresponds to the mode output by the CRTC
    > * for the first bridge in the chain. It can be different from the mode
    > * parameter that contains the desired mode for the connector at the end
    > * of the bridges chain, for instance when the first bridge in the chain
    > * performs scaling. The adjusted mode is mostly useful for the first
    > * bridge in the chain and is likely irrelevant for the other bridges.
    > *
    > * For atomic drivers the adjusted_mode is the mode stored in
    > * &drm_crtc_state.adjusted_mode.
    > *
    > * NOTE:
    > *
    > * If a need arises to store and access modes adjusted for other locations
    > * than the connection between the CRTC and the first bridge, the DRM
    > * framework will have to be extended with DRM bridge states.
    > */
    >
    > Then I think we should also update the documentation of
    > drm_crtc_state.adjusted_mode accordingly:
    >
    > /*
    > * @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.
    > *
    > * For drivers using drm_bridge, this store the hardware display timings
    > * used between the CRTC and the first bridge. For other drivers, the
    > * meaning of the adjusted_mode field is purely driver implementation
    > * defined information, and will usually be used to store the hardware
    > * display timings used between the CRTC and encoder blocks.
    > */
    >

    Your proposal is very clear and understandable. I will make a new patch
    version based on it.
    Big thanks
    Philippe :-)
    \
     
     \ /
      Last update: 2018-04-06 17:30    [W:2.469 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site