[Intel-gfx] [PATCH 3/7] drm/i915: clear up the fdi dotclock semantics for M/N computation
Paulo Zanoni
przanoni at gmail.com
Mon Jun 3 17:59:29 CEST 2013
2013/6/1 Daniel Vetter <daniel.vetter at ffwll.ch>:
> We currently mutliply the link_bw of the fdi link with the pixel
> multiplier, which is wrong: The FDI link doesn't suddenly grow more
> bandwidth. In reality the pixel mutliplication only happens in the PCH,
> before the pixels are fed into the port.
>
> But since we our code treats the uses the target clock after pixels
> are doubled (tripled, ...) already, we need to correct this.
>
> Semantically it's clearer to divide the target clock to get the fdi
> dotclock instead of multiplying the bw, so do that instead.
>
> Note that the target clock is already multiplied by the same factor,
> so the division will never loose accuracy for the M/N computation.
>
> The lane computation otoh used the wrong value, we also need to feed
> the fdi dotclock to that.
>
> Split out on a request from Paulo Zanoni.
>
> v2: Also fix the lane computation.
>
> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
Do we ever test the pixel_multiplier case? Does it work at all?
Why is intel_ddi_get_config the only get_config function that has
"pipe_config->pixel_multiplier = 1"? If we assigned "1" to everybody
that don't need other values we would be able to remove many of those
"if (pixel_multiplier > 1)" checks.
Anyway, the patch seems to do what it says, so: Reviewed-by: Paulo
Zanoni <paulo.r.zanoni at intel.com>.
> ---
> drivers/gpu/drm/i915/intel_display.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a29295e..761254d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3992,7 +3992,7 @@ static int ironlake_fdi_compute_config(struct intel_crtc *intel_crtc,
> {
> struct drm_device *dev = intel_crtc->base.dev;
> struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> - int target_clock, lane, link_bw;
> + int target_clock, lane, link_bw, fdi_dotclock;
> bool setup_ok, needs_recompute = false;
>
> retry:
> @@ -4010,14 +4010,16 @@ retry:
> else
> target_clock = adjusted_mode->clock;
>
> - lane = ironlake_get_lanes_required(target_clock, link_bw,
> + fdi_dotclock = target_clock;
> + if (pipe_config->pixel_multiplier > 1)
> + fdi_dotclock /= pipe_config->pixel_multiplier;
> +
> + lane = ironlake_get_lanes_required(fdi_dotclock, link_bw,
> pipe_config->pipe_bpp);
>
> pipe_config->fdi_lanes = lane;
>
> - if (pipe_config->pixel_multiplier > 1)
> - link_bw *= pipe_config->pixel_multiplier;
> - intel_link_compute_m_n(pipe_config->pipe_bpp, lane, target_clock,
> + intel_link_compute_m_n(pipe_config->pipe_bpp, lane, fdi_dotclock,
> link_bw, &pipe_config->fdi_m_n);
>
> setup_ok = ironlake_check_fdi_lanes(intel_crtc->base.dev,
> --
> 1.7.11.7
>
--
Paulo Zanoni
More information about the Intel-gfx
mailing list