Re: [PATCH 17/20] drm/i915: Use adjusted_mode in the fastboot hack to disable pfit

Damien Lespiau
2013-09-20T15:33:47Z

On Fri, Sep 20, 2013 at 05:54:55PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 19, 2013 at 05:40:32PM +0100, Damien Lespiau wrote:
> > When booting with i915.fastboot=1, we always take tha code path and end
> > up undoing what we're trying to do with adjusted_mode.
> >
> > Hopefully, as the fastboot hardware readout code is using adjusted_mode
> > as well, it should be equivalent.
> >
> > Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> > ---
> > drivers/gpu/drm/i915/intel_display.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f868266..2b9f80b 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2288,9 +2288,12 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y,
> >
> > /* Update pipe size and adjust fitter if needed */
> > if (i915_fastboot) {
> > + const struct drm_display_mode *adjusted_mode =
> > + &intel_crtc->config.adjusted_mode;
> > +
> > I915_WRITE(PIPESRC(intel_crtc->pipe),
> > - ((crtc->mode.hdisplay - 1) << 16) |
> > - (crtc->mode.vdisplay - 1));
> > + ((adjusted_mode->crtc_hdisplay - 1) << 16) |
> > + (adjusted_mode->crtc_vdisplay - 1));
> > if (!intel_crtc->config.pch_pfit.enabled &&
> > (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) ||
> > intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) {
>
> OK, I'm offically confused by this thing. Maybe it got a bit broken
> by the pfit.enabled change?
>
> I must assume that the original intention of this was to turn off the
> panel fitter in case the BIOS had left it enabled w/ 0x0 size, but
> I'm not sure how that would even work. Anyways, now it will turn it
> off if it's already off, which doesn't make much sense.
>
> And I guess the PIPESRC write is there because we assume the BIOS left
> it wrong for the non-pfit case. We have explicit readout for it now,
> so we could actually check if that's the case.

Well, I didn't even read beyond the PIPESRC write, but yes, now that you
mention it, it looks dodgy.

Jesse, do you remember what was the original intention? neither the
commit introducing the change nor the comment are very verbose.

--
Damien