On Mon, Sep 30, 2024 at 01:28:59PM GMT, Liu Ying wrote:
> Multiple display modes could be read from a display device's EDID.
> Use clk_round_rate() to validate the "ldb" clock rate for each mode
> in drm_bridge_funcs::mode_valid() to filter unsupported modes out.
>
> Also, if the "ldb" clock and the pixel clock are sibling in clock
> tree, use clk_round_rate() to validate the pixel clock rate against
> the "ldb" clock. This is not done in display controller driver
> because drm_crtc_helper_funcs::mode_valid() may not decide to do
> the validation or not if multiple encoders are connected to the CRTC,
> e.g., i.MX93 LCDIF may connect with MIPI DSI controller, LDB and
> parallel display output simultaneously.
>
> Signed-off-by: Liu Ying <victor.liu@xxxxxxx>
> ---
> drivers/gpu/drm/bridge/fsl-ldb.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/bridge/fsl-ldb.c b/drivers/gpu/drm/bridge/fsl-ldb.c
> index b559f3e0bef6..ee8471c86617 100644
> --- a/drivers/gpu/drm/bridge/fsl-ldb.c
> +++ b/drivers/gpu/drm/bridge/fsl-ldb.c
> @@ -11,6 +11,7 @@
> #include <linux/of_graph.h>
> #include <linux/platform_device.h>
> #include <linux/regmap.h>
> +#include <linux/units.h>
>
> #include <drm/drm_atomic_helper.h>
> #include <drm/drm_bridge.h>
> @@ -64,6 +65,7 @@ struct fsl_ldb_devdata {
> u32 lvds_ctrl;
> bool lvds_en_bit;
> bool single_ctrl_reg;
> + bool ldb_clk_pixel_clk_sibling;
> };
>
> static const struct fsl_ldb_devdata fsl_ldb_devdata[] = {
> @@ -74,11 +76,13 @@ static const struct fsl_ldb_devdata fsl_ldb_devdata[] = {
> [IMX8MP_LDB] = {
> .ldb_ctrl = 0x5c,
> .lvds_ctrl = 0x128,
> + .ldb_clk_pixel_clk_sibling = true,
> },
> [IMX93_LDB] = {
> .ldb_ctrl = 0x20,
> .lvds_ctrl = 0x24,
> .lvds_en_bit = true,
> + .ldb_clk_pixel_clk_sibling = true,
> },
> };
>
> @@ -269,11 +273,29 @@ fsl_ldb_mode_valid(struct drm_bridge *bridge,
> const struct drm_display_info *info,
> const struct drm_display_mode *mode)
> {
> + unsigned long link_freq, pclk_rate, rounded_pclk_rate;
> struct fsl_ldb *fsl_ldb = to_fsl_ldb(bridge);
>
> if (mode->clock > (fsl_ldb_is_dual(fsl_ldb) ? 160000 : 80000))
> return MODE_CLOCK_HIGH;
>
> + /* Validate "ldb" clock rate. */
> + link_freq = fsl_ldb_link_frequency(fsl_ldb, mode->clock);
> + if (link_freq != clk_round_rate(fsl_ldb->clk, link_freq))
> + return MODE_NOCLOCK;
> +
> + /*
> + * Use "ldb" clock to validate pixel clock rate,
> + * if the two clocks are sibling.
> + */
> + if (fsl_ldb->devdata->ldb_clk_pixel_clk_sibling) {
> + pclk_rate = mode->clock * HZ_PER_KHZ;
> +
> + rounded_pclk_rate = clk_round_rate(fsl_ldb->clk, pclk_rate);
> + if (rounded_pclk_rate != pclk_rate)
> + return MODE_NOCLOCK;
> + }
> +
I guess this is to workaround the fact that the parent rate would be
changed, and thus the sibling rate as well? This should be documented in
a comment if so.
Maxime
Attachment:
signature.asc
Description: PGP signature
