2018-04-09 15:31:30

by Philippe Cornu

[permalink] [raw]
Subject: [PATCH] drm: clarify adjusted_mode documentation for bridges

This patch clarifies the adjusted_mode documentation
for bridges.

Signed-off-by: Philippe Cornu <[email protected]>
Signed-off-by: Laurent Pinchart <[email protected]>
---
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



2018-04-19 15:52:54

by Philippe Cornu

[permalink] [raw]
Subject: Re: [PATCH] drm: clarify adjusted_mode documentation for bridges

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 <[email protected]>
> Signed-off-by: Laurent Pinchart <[email protected]>
> ---
> 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;
>
>

2018-04-19 17:03:39

by Archit Taneja

[permalink] [raw]
Subject: Re: [PATCH] drm: clarify adjusted_mode documentation for bridges



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 <[email protected]>

>> Signed-off-by: Philippe Cornu <[email protected]>
>> Signed-off-by: Laurent Pinchart <[email protected]>
>> ---
>> 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;
>>

2018-05-15 15:57:07

by Philippe Cornu

[permalink] [raw]
Subject: Re: [PATCH] drm: clarify adjusted_mode documentation for bridges

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 <[email protected]>
>
>>> Signed-off-by: Philippe Cornu <[email protected]>
>>> Signed-off-by: Laurent Pinchart <[email protected]>
>>> ---
>>> 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;
>>>