On Mon, 21 Oct 2013, Shobhit Kumar <shobhit.kumar@xxxxxxxxx> wrote:
> Minor modification to m_n_p calculations as well
That should probably be a separate patch, unless it's a requirement for
what the main subject of this patch is. The commit message does not say.
> Signed-off-by: Shobhit Kumar <shobhit.kumar@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dsi_pll.c | 75 ++++++++++++++++++++++++++++------
> 1 file changed, 63 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c
> index 44279b2..bf12335 100644
> --- a/drivers/gpu/drm/i915/intel_dsi_pll.c
> +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c
> @@ -50,6 +50,8 @@ static const u32 lfsr_converts[] = {
> 71, 35 /* 91 - 92 */
> };
>
> +#ifdef DSI_CLK_FROM_RR
> +
> static u32 dsi_rr_formula(const struct drm_display_mode *mode,
> int pixel_format, int video_mode_format,
> int lane_count, bool eotp)
> @@ -129,6 +131,40 @@ static u32 dsi_rr_formula(const struct drm_display_mode *mode,
> return dsi_clk;
> }
>
> +#else
> +
> +/* Get DSI clock from pixel clock */
> +static u32 dsi_clk_from_pclk(const struct drm_display_mode *mode,
> + int pixel_format, int lane_count)
> +{
> + u32 dsi_bit_clock_hz, dsi_clk;
> + u32 bpp;
> +
> + switch (pixel_format) {
> + default:
> + case VID_MODE_FORMAT_RGB888:
> + case VID_MODE_FORMAT_RGB666_LOOSE:
> + bpp = 24;
> + break;
> + case VID_MODE_FORMAT_RGB666:
> + bpp = 18;
> + break;
> + case VID_MODE_FORMAT_RGB565:
> + bpp = 16;
> + break;
> + }
> +
> + /* DSI data rate = pixel clock * bits per pixel / lane count
> + pixel clock is converted from KHz to Hz */
> + dsi_bit_clock_hz = (((mode->clock * 1000) * bpp) / lane_count);
> +
> + /* DSI clock rate */
> + dsi_clk = dsi_bit_clock_hz / (1000 * 1000);
> + return dsi_clk;
> +}
> +
> +#endif
> +
> #ifdef MNP_FROM_TABLE
>
> struct dsi_clock_table {
> @@ -208,29 +244,42 @@ static int dsi_calc_mnp(u32 dsi_clk, struct dsi_mnp *dsi_mnp)
> ref_clk = 25000;
> target_dsi_clk = dsi_clk * 1000;
> error = 0xFFFFFFFF;
> + tmp_error = 0xFFFFFFFF;
> calc_m = 0;
> calc_p = 0;
>
> for (m = 62; m <= 92; m++) {
> for (p = 2; p <= 6; p++) {
> -
> + /* Find the optimal m and p divisors
> + with minimal error +/- the required clock */
> calc_dsi_clk = (m * ref_clk) / p;
> - if (calc_dsi_clk >= target_dsi_clk) {
> + if (calc_dsi_clk == target_dsi_clk) {
> + calc_m = m;
> + calc_p = p;
> + error = 0;
> + break;
> + } else if (calc_dsi_clk > target_dsi_clk)
> tmp_error = calc_dsi_clk - target_dsi_clk;
> - if (tmp_error < error) {
> - error = tmp_error;
> - calc_m = m;
> - calc_p = p;
> - }
> + else
> + tmp_error = target_dsi_clk - calc_dsi_clk;
> +
Using abs() might clean this up a bit.
> + if (tmp_error < error) {
> + error = tmp_error;
> + calc_m = m;
> + calc_p = p;
> }
> }
> +
> + if (error == 0)
> + break;
The above changes should be a separate patch, with rationale in the
commit message.
> }
>
> m_seed = lfsr_converts[calc_m - 62];
> n = 1;
> +
> dsi_mnp->dsi_pll_ctrl = 1 << (DSI_PLL_P1_POST_DIV_SHIFT + calc_p - 2);
> dsi_mnp->dsi_pll_div = (n - 1) << DSI_PLL_N1_DIV_SHIFT |
> - m_seed << DSI_PLL_M1_DIV_SHIFT;
> + m_seed << DSI_PLL_M1_DIV_SHIFT;
If really needed, style/whitespace changes should also be separated.
>
> return 0;
> }
> @@ -249,11 +298,13 @@ static void vlv_configure_dsi_pll(struct intel_encoder *encoder)
> struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
> int ret;
> struct dsi_mnp dsi_mnp;
> - u32 dsi_clk;
> + u32 dsi_clk = 0;
>
> - dsi_clk = dsi_rr_formula(mode, intel_dsi->pixel_format,
> - intel_dsi->video_mode_format,
> - intel_dsi->lane_count, !intel_dsi->eot_disable);
> + if (intel_dsi->dsi_clock_freq)
> + dsi_clk = intel_dsi->dsi_clock_freq;
This is the third major change in the patch. Should be separate, with
rationale, or possibly bundled with a different change which starts
using it. Who is responsible for setting and clearing this?
> + else
> + dsi_clk = dsi_clk_from_pclk(mode, intel_dsi->pixel_format,
> + intel_dsi->lane_count);
>
> ret = dsi_calc_mnp(dsi_clk, &dsi_mnp);
> if (ret) {
> --
> 1.7.9.5
>
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
