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
>
>>> Signed-off-by: Philippe Cornu
>>> Signed-off-by: Laurent Pinchart
>>> ---
>>> 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;
>>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
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
Signed-off-by: Philippe Cornu
Signed-off-by: Laurent Pinchart
---
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;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
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
> Signed-off-by: Laurent Pinchart
> ---
> 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;
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: clarify adjusted_mode documentation for bridges
This patch clarifies the adjusted_mode documentation
for bridges.
Signed-off-by: Philippe Cornu
Signed-off-by: Laurent Pinchart
---
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
