Re: [PATCHv3 05/30] drm/omap: improve DPI clock selection on DRA7xx

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomi,

Thank you for the patch.

On Tuesday 28 Mar 2017 16:07:51 Tomi Valkeinen wrote:
> The clock source selection for the LCD outputs is too hardcoded at the
> moment. For example, LCD3 is set to use PLL2_1, and PLL2 doesn't exist
> on DRA72x SoCs.
> 
> There are quite many ways to configure the clocks, even using HDMI PLL
> for LCD outputs, but enabling full configuration of the clocks is rather
> tricky.
> 
> This patch improves the situation a bit by checking if the PLL about to
> be used exists, and if not, tries another one.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/dss/dpi.c | 47 +++++++++++++++++++++++++++---------
>  1 file changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c
> b/drivers/gpu/drm/omapdrm/dss/dpi.c index e0b0c5c24c55..0f32d5d078c6 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
> @@ -67,6 +67,42 @@ static struct dpi_data *dpi_get_data_from_pdev(struct
> platform_device *pdev) return dev_get_drvdata(&pdev->dev);
>  }
> 
> +static enum dss_clk_source dpi_get_clk_src_dra7xx(enum omap_channel
> channel)
> +{
> +	/*
> +	 * Possible clock sources:
> +	 * LCD1: FCK/PLL1_1/HDMI_PLL
> +	 * LCD2: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_3)
> +	 * LCD3: FCK/PLL1_3/HDMI_PLL (DRA74x: PLL2_1)
> +	 */
> +
> +	switch (channel) {
> +	case OMAP_DSS_CHANNEL_LCD:
> +	{
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_1))
> +			return DSS_CLK_SRC_PLL1_1;
> +	}

Aren't you missing break statements ?

> +	case OMAP_DSS_CHANNEL_LCD2:
> +	{
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3))
> +			return DSS_CLK_SRC_PLL1_3;
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_3))
> +			return DSS_CLK_SRC_PLL2_3;
> +	}
> +	case OMAP_DSS_CHANNEL_LCD3:
> +	{
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL2_1))
> +			return DSS_CLK_SRC_PLL2_1;
> +		if (dss_pll_find_by_src(DSS_CLK_SRC_PLL1_3))
> +			return DSS_CLK_SRC_PLL1_3;

What happens if LCD2 is already using PLL1_3 ?

> +	}
> +	default:
> +		break;
> +	}
> +
> +	return DSS_CLK_SRC_FCK;
> +}
> +
>  static enum dss_clk_source dpi_get_clk_src(enum omap_channel channel)
>  {
>  	/*
> @@ -107,16 +143,7 @@ static enum dss_clk_source dpi_get_clk_src(enum
> omap_channel channel) }
> 
>  	case OMAPDSS_VER_DRA7xx:
> -		switch (channel) {
> -		case OMAP_DSS_CHANNEL_LCD:
> -			return DSS_CLK_SRC_PLL1_1;
> -		case OMAP_DSS_CHANNEL_LCD2:
> -			return DSS_CLK_SRC_PLL1_3;
> -		case OMAP_DSS_CHANNEL_LCD3:
> -			return DSS_CLK_SRC_PLL2_1;
> -		default:
> -			return DSS_CLK_SRC_FCK;
> -		}
> +		return dpi_get_clk_src_dra7xx(channel);
> 
>  	default:
>  		return DSS_CLK_SRC_FCK;

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel