diff mbox series

[v2] drm: dsi: Add lane clock rate fields to DSI device

Message ID 20181018111902.31738-1-linus.walleij@linaro.org (mailing list archive)
State New, archived
Headers show
Series [v2] drm: dsi: Add lane clock rate fields to DSI device | expand

Commit Message

Linus Walleij Oct. 18, 2018, 11:19 a.m. UTC
The DSI devices have a maximum operating frequency specified
in their data sheet per the MIPI specification, and DSI hosts
that can scale their frequency need this information to set
their clock dividers right.

As current panel drivers often lack this information, specify
that setting it to zero will make the DSI host use some
reasonable default.

Cc: Andrzej Hajda <a.hajda@samsung.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
ChangeLog v1->v2:
- s/*_rate_hz/*_rate/g
- s/operation/mode/g
- Clarify that zero is only allowed for legacy drivers
---
 include/drm/drm_mipi_dsi.h | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Linus Walleij Oct. 22, 2018, 6:50 a.m. UTC | #1
On Thu, Oct 18, 2018 at 1:19 PM Linus Walleij <linus.walleij@linaro.org> wrote:

> The DSI devices have a maximum operating frequency specified
> in their data sheet per the MIPI specification, and DSI hosts
> that can scale their frequency need this information to set
> their clock dividers right.
>
> As current panel drivers often lack this information, specify
> that setting it to zero will make the DSI host use some
> reasonable default.
>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - s/*_rate_hz/*_rate/g
> - s/operation/mode/g
> - Clarify that zero is only allowed for legacy drivers

Andrzej are you fine with this version (Acked-by) so I can apply it?

Yours,
Linus Walleij
Andrzej Hajda Oct. 23, 2018, 5:03 a.m. UTC | #2
czw., 18 paź 2018, 12:21 użytkownik Linus Walleij <linus.walleij@linaro.org>
napisał:

> The DSI devices have a maximum operating frequency specified
> in their data sheet per the MIPI specification, and DSI hosts
> that can scale their frequency need this information to set
> their clock dividers right.
>
> As current panel drivers often lack this information, specify
> that setting it to zero will make the DSI host use some
> reasonable default.
>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - s/*_rate_hz/*_rate/g
> - s/operation/mode/g
> - Clarify that zero is only allowed for legacy drivers
> ---
>  include/drm/drm_mipi_dsi.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 4fef19064b0f..da3499de2dc2 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -168,6 +168,12 @@ struct mipi_dsi_device_info {
>   * @format: pixel format for video mode
>   * @lanes: number of active data lanes
>   * @mode_flags: DSI operation mode related flags
> + * @hs_rate: Maximum lane frequency for high speed mode, this should
> + * be set to the real limits of the hardware, zero is only accepted for
> + * legacy drivers
> + * @lp_rate: Maximum lane frequency for low power mode, this should
> + * be set to the real limits of the hardware, zero is only accepted for
> + * legacy drivers
>

The convention here is no-capital-letters, moreover it would be good to put
units in the description.

Regarding zero value, as I looked into some random panel data sheets they
do not always have specified rates, so maybe calculation of hs_rate from
specified video mode wouldn't be so bad, but in such case it should be
described here, up to you, can be adjusted in the future when we will have
more input.

So with minor changes from previous sentence:
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>

I have no access to my corp mail atm, so if there is policy to send tags
from the same email, I can do it on Friday.

Regards
Andrzej

  */
>  struct mipi_dsi_device {
>         struct mipi_dsi_host *host;
> @@ -178,6 +184,8 @@ struct mipi_dsi_device {
>         unsigned int lanes;
>         enum mipi_dsi_pixel_format format;
>         unsigned long mode_flags;
> +       unsigned long hs_rate;
> +       unsigned long lp_rate;
>  };
>
>  #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"
> --
> 2.17.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
<div dir="auto"><div><br><div class="gmail_quote"><div dir="ltr">czw., 18 paź 2018, 12:21 użytkownik Linus Walleij &lt;<a href="mailto:linus.walleij@linaro.org">linus.walleij@linaro.org</a>&gt; napisał:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The DSI devices have a maximum operating frequency specified<br>
in their data sheet per the MIPI specification, and DSI hosts<br>
that can scale their frequency need this information to set<br>
their clock dividers right.<br>
<br>
As current panel drivers often lack this information, specify<br>
that setting it to zero will make the DSI host use some<br>
reasonable default.<br>
<br>
Cc: Andrzej Hajda &lt;<a href="mailto:a.hajda@samsung.com" target="_blank" rel="noreferrer">a.hajda@samsung.com</a>&gt;<br>
Signed-off-by: Linus Walleij &lt;<a href="mailto:linus.walleij@linaro.org" target="_blank" rel="noreferrer">linus.walleij@linaro.org</a>&gt;<br>
---<br>
ChangeLog v1-&gt;v2:<br>
- s/*_rate_hz/*_rate/g<br>
- s/operation/mode/g<br>
- Clarify that zero is only allowed for legacy drivers<br>
---<br>
 include/drm/drm_mipi_dsi.h | 8 ++++++++<br>
 1 file changed, 8 insertions(+)<br>
<br>
diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h<br>
index 4fef19064b0f..da3499de2dc2 100644<br>
--- a/include/drm/drm_mipi_dsi.h<br>
+++ b/include/drm/drm_mipi_dsi.h<br>
@@ -168,6 +168,12 @@ struct mipi_dsi_device_info {<br>
  * @format: pixel format for video mode<br>
  * @lanes: number of active data lanes<br>
  * @mode_flags: DSI operation mode related flags<br>
+ * @hs_rate: Maximum lane frequency for high speed mode, this should<br>
+ * be set to the real limits of the hardware, zero is only accepted for<br>
+ * legacy drivers<br>
+ * @lp_rate: Maximum lane frequency for low power mode, this should<br>
+ * be set to the real limits of the hardware, zero is only accepted for<br>
+ * legacy drivers<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto"><div dir="auto" style="font-family:sans-serif">The convention here is no-capital-letters, moreover it would be good to put units in the description.</div><div dir="auto" style="font-family:sans-serif"><br></div><div dir="auto" style="font-family:sans-serif">Regarding zero value, as I looked into some random panel data sheets they do not always have specified rates, so maybe calculation of hs_rate from specified video mode wouldn&#39;t be so bad, but in such case it should be described here, up to you, can be adjusted in the future when we will have more input.</div><div dir="auto" style="font-family:sans-serif"><br></div><div dir="auto" style="font-family:sans-serif">So with minor changes from previous sentence:</div><div dir="auto" style="font-family:sans-serif">Reviewed-by: Andrzej Hajda &lt;<a href="mailto:a.hajda@samsung.com">a.hajda@samsung.com</a>&gt;</div><div dir="auto" style="font-family:sans-serif"><br></div><div dir="auto" style="font-family:sans-serif">I have no access to my corp mail atm, so if there is policy to send tags from the same email, I can do it on Friday.</div><div dir="auto" style="font-family:sans-serif"><br></div><div dir="auto" style="font-family:sans-serif">Regards</div><div dir="auto" style="font-family:sans-serif">Andrzej</div><div dir="auto" style="font-family:sans-serif"><br></div></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  */<br>
 struct mipi_dsi_device {<br>
        struct mipi_dsi_host *host;<br>
@@ -178,6 +184,8 @@ struct mipi_dsi_device {<br>
        unsigned int lanes;<br>
        enum mipi_dsi_pixel_format format;<br>
        unsigned long mode_flags;<br>
+       unsigned long hs_rate;<br>
+       unsigned long lp_rate;<br>
 };<br>
<br>
 #define MIPI_DSI_MODULE_PREFIX &quot;mipi-dsi:&quot;<br>
-- <br>
2.17.2<br>
<br>
_______________________________________________<br>
dri-devel mailing list<br>
<a href="mailto:dri-devel@lists.freedesktop.org" target="_blank" rel="noreferrer">dri-devel@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/dri-devel" rel="noreferrer noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/dri-devel</a><br>
</blockquote></div></div></div>
diff mbox series

Patch

diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
index 4fef19064b0f..da3499de2dc2 100644
--- a/include/drm/drm_mipi_dsi.h
+++ b/include/drm/drm_mipi_dsi.h
@@ -168,6 +168,12 @@  struct mipi_dsi_device_info {
  * @format: pixel format for video mode
  * @lanes: number of active data lanes
  * @mode_flags: DSI operation mode related flags
+ * @hs_rate: Maximum lane frequency for high speed mode, this should
+ * be set to the real limits of the hardware, zero is only accepted for
+ * legacy drivers
+ * @lp_rate: Maximum lane frequency for low power mode, this should
+ * be set to the real limits of the hardware, zero is only accepted for
+ * legacy drivers
  */
 struct mipi_dsi_device {
 	struct mipi_dsi_host *host;
@@ -178,6 +184,8 @@  struct mipi_dsi_device {
 	unsigned int lanes;
 	enum mipi_dsi_pixel_format format;
 	unsigned long mode_flags;
+	unsigned long hs_rate;
+	unsigned long lp_rate;
 };
 
 #define MIPI_DSI_MODULE_PREFIX "mipi-dsi:"