[PATCH] drm: clarify adjusted_mode documentation for bridges

[PATCH] drm: clarify adjusted_mode documentation for bridges

[Philippe Cornu]

This patch clarifies the adjusted_mode documentation
for bridges.

Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
This patch follows discussions in:
- "drm: clarify adjusted_mode for a bridge connected to a crtc"
  https://patchwork.freedesktop.org/patch/206801/
- "drm: bridge: Constify mode arguments to bridge .mode_set() operation"
  https://patchwork.freedesktop.org/patch/215449/

 include/drm/drm_bridge.h | 16 ++++++++++++++++
 include/drm/drm_crtc.h   | 11 +++++++----
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 3270fec46979..7d632c6a9214 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -178,6 +178,22 @@ struct drm_bridge_funcs {
 	 * 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.
 	 */
 	void (*mode_set)(struct drm_bridge *bridge,
 			 struct drm_display_mode *mode,
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index a2d81d2907a9..65f749a9e9d3 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -134,10 +134,13 @@ struct drm_crtc_state {
 	 *
 	 * 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.
+	 * is actually programmed into the hardware.
+	 *
+	 * For drivers using drm_bridge, this stores 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.
 	 */
 	struct drm_display_mode adjusted_mode;
 
-- 
2.15.1

Re: [PATCH] drm: clarify adjusted_mode documentation for bridges

[Philippe CORNU]

Hi Archit & Andrzej,

May I ask you please a short review of this documentation update.
Many thanks
Philippe :-)

On 04/09/2018 05:24 PM, Philippe Cornu wrote:
> This patch clarifies the adjusted_mode documentation
> for bridges.
> 
> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> This patch follows discussions in:
> - "drm: clarify adjusted_mode for a bridge connected to a crtc"
>    https://patchwork.freedesktop.org/patch/206801/
> - "drm: bridge: Constify mode arguments to bridge .mode_set() operation"
>    https://patchwork.freedesktop.org/patch/215449/
> 
>   include/drm/drm_bridge.h | 16 ++++++++++++++++
>   include/drm/drm_crtc.h   | 11 +++++++----
>   2 files changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 3270fec46979..7d632c6a9214 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -178,6 +178,22 @@ struct drm_bridge_funcs {
>   	 * 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.
>   	 */
>   	void (*mode_set)(struct drm_bridge *bridge,
>   			 struct drm_display_mode *mode,
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index a2d81d2907a9..65f749a9e9d3 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -134,10 +134,13 @@ struct drm_crtc_state {
>   	 *
>   	 * 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.
> +	 * is actually programmed into the hardware.
> +	 *
> +	 * For drivers using drm_bridge, this stores 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.
>   	 */
>   	struct drm_display_mode adjusted_mode;
>   
> 

Re: [PATCH] drm: clarify adjusted_mode documentation for bridges

[Archit Taneja]

On Thursday 19 April 2018 09:20 PM, Philippe CORNU wrote:
> Hi Archit & Andrzej,
> 
> May I ask you please a short review of this documentation update.
> Many thanks
> Philippe :-)
> 
> On 04/09/2018 05:24 PM, Philippe Cornu wrote:
>> This patch clarifies the adjusted_mode documentation
>> for bridges.
>>

Reviewed-by: Archit Taneja <architt@codeaurora.org>

>> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>> ---
>> This patch follows discussions in:
>> - "drm: clarify adjusted_mode for a bridge connected to a crtc"
>>     https://patchwork.freedesktop.org/patch/206801/
>> - "drm: bridge: Constify mode arguments to bridge .mode_set() operation"
>>     https://patchwork.freedesktop.org/patch/215449/
>>
>>    include/drm/drm_bridge.h | 16 ++++++++++++++++
>>    include/drm/drm_crtc.h   | 11 +++++++----
>>    2 files changed, 23 insertions(+), 4 deletions(-)
>>
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index 3270fec46979..7d632c6a9214 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -178,6 +178,22 @@ struct drm_bridge_funcs {
>>    	 * 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.
>>    	 */
>>    	void (*mode_set)(struct drm_bridge *bridge,
>>    			 struct drm_display_mode *mode,
>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>> index a2d81d2907a9..65f749a9e9d3 100644
>> --- a/include/drm/drm_crtc.h
>> +++ b/include/drm/drm_crtc.h
>> @@ -134,10 +134,13 @@ struct drm_crtc_state {
>>    	 *
>>    	 * 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.
>> +	 * is actually programmed into the hardware.
>> +	 *
>> +	 * For drivers using drm_bridge, this stores 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.
>>    	 */
>>    	struct drm_display_mode adjusted_mode;
>>    

Re: [PATCH] drm: clarify adjusted_mode documentation for bridges

[Philippe CORNU]

Hi,

Applied on drm-misc-next.
Many thanks,
Philippe :-)

On 04/19/2018 07:00 PM, Archit Taneja wrote:
> 
> 
> On Thursday 19 April 2018 09:20 PM, Philippe CORNU wrote:
>> Hi Archit & Andrzej,
>>
>> May I ask you please a short review of this documentation update.
>> Many thanks
>> Philippe :-)
>>
>> On 04/09/2018 05:24 PM, Philippe Cornu wrote:
>>> This patch clarifies the adjusted_mode documentation
>>> for bridges.
>>>
> 
> Reviewed-by: Archit Taneja <architt@codeaurora.org>
> 
>>> Signed-off-by: Philippe Cornu <philippe.cornu@st.com>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
>>> ---
>>> This patch follows discussions in:
>>> - "drm: clarify adjusted_mode for a bridge connected to a crtc"
>>>      https://patchwork.freedesktop.org/patch/206801/
>>> - "drm: bridge: Constify mode arguments to bridge .mode_set() operation"
>>>      https://patchwork.freedesktop.org/patch/215449/
>>>
>>>     include/drm/drm_bridge.h | 16 ++++++++++++++++
>>>     include/drm/drm_crtc.h   | 11 +++++++----
>>>     2 files changed, 23 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>>> index 3270fec46979..7d632c6a9214 100644
>>> --- a/include/drm/drm_bridge.h
>>> +++ b/include/drm/drm_bridge.h
>>> @@ -178,6 +178,22 @@ struct drm_bridge_funcs {
>>>     	 * 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.
>>>     	 */
>>>     	void (*mode_set)(struct drm_bridge *bridge,
>>>     			 struct drm_display_mode *mode,
>>> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>>> index a2d81d2907a9..65f749a9e9d3 100644
>>> --- a/include/drm/drm_crtc.h
>>> +++ b/include/drm/drm_crtc.h
>>> @@ -134,10 +134,13 @@ struct drm_crtc_state {
>>>     	 *
>>>     	 * 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.
>>> +	 * is actually programmed into the hardware.
>>> +	 *
>>> +	 * For drivers using drm_bridge, this stores 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.
>>>     	 */
>>>     	struct drm_display_mode adjusted_mode;
>>>