From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 3/8] drm/i915: extract set_pipe_timings from ironlake_crtc_mode_set Date: Thu, 20 Sep 2012 09:32:41 +0200 Message-ID: <20120920073241.GA2844@bremse> References: <1347455196-5167-1-git-send-email-przanoni@gmail.com> <1347455196-5167-4-git-send-email-przanoni@gmail.com> <20120912141133.GI5533@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-qc0-f177.google.com (mail-qc0-f177.google.com [209.85.216.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 846DD9E7EF for ; Thu, 20 Sep 2012 00:32:50 -0700 (PDT) Received: by qcsu28 with SMTP id u28so1594251qcs.36 for ; Thu, 20 Sep 2012 00:32:49 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Paulo Zanoni Cc: intel-gfx@lists.freedesktop.org, Paulo Zanoni List-Id: intel-gfx@lists.freedesktop.org On Wed, Sep 19, 2012 at 03:11:33PM -0300, Paulo Zanoni wrote: > Hi > > 2012/9/12 Daniel Vetter : > > On Wed, Sep 12, 2012 at 10:06:31AM -0300, Paulo Zanoni wrote: > >> From: Paulo Zanoni > >> > >> Signed-off-by: Paulo Zanoni > > > > Hm, I think we should extract the same code from i9xx_crtc_set_mode, too > > and share it in a common intel_set_pipe_timings. Their almost identical > > safe for: > > - vsyncshift is only gen4+ > > This is easy to solve. > > > - source position handling is a bit different, but I think it'd be > > semantically clearer if we leave that out of set_pipe_timings. Imo that > > belongs to the panel fitter settings, which are currently splattered all > > over. Meh. > > Well, the PIPESRC register is described inside the "pipe timings" > documentation section, so I think it should be inside the > set_pipe_timings function. > > I actually implemented your suggestion locally and the only real > problem is that on i9xx_crtc_mode_set we currently write to DSPSIZE > and DSPPOS before writing to PIPESRC, so to make the code look good we > have 2 options: > 1 - Write to DSPPOS and DSPSIZE before writing all the timing registers > 2 - Write to DSPPOS and DSPSIZE after writing all the timing registers > > In both cases we are changing the writing order. I looked at the > documentation and it seems we should be writing to the plane registers > only after setting the pipe registers, so maybe solution 2 is the > correct. The problem is that yes, we are changing the behavior and I > don't even have such machines to test. > > So, how do we proceed here? Want the version, keep the old one, or do > something entirely different? I guess a quick patch to move around the DSP* regs down and run it on a few gen2/3 machines. Then move things around if it doesn't blow up. Since I'm travelling I think you need to volunteer Chris for the testing ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch