| Message ID | 20181018111902.31738-1-linus.walleij@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] drm: dsi: Add lane clock rate fields to DSI device | expand |
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
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 <<a href="mailto:linus.walleij@linaro.org">linus.walleij@linaro.org</a>> 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 <<a href="mailto:a.hajda@samsung.com" target="_blank" rel="noreferrer">a.hajda@samsung.com</a>><br> Signed-off-by: Linus Walleij <<a href="mailto:linus.walleij@linaro.org" target="_blank" rel="noreferrer">linus.walleij@linaro.org</a>><br> ---<br> ChangeLog v1->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'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 <<a href="mailto:a.hajda@samsung.com">a.hajda@samsung.com</a>></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 "mipi-dsi:"<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 --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:"
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(+)