> -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel Vetter > Sent: Tuesday, March 17, 2015 7:33 AM > To: Konduru, Chandra > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Conselvan De Oliveira, Ander; Vetter, Daniel > Subject: Re: [PATCH 00/21] skylake display scalers > > On Sat, Mar 14, 2015 at 10:55:25PM -0700, Chandra Konduru wrote: > > This patch enables skylake display scalers in atomic framework. > > Skylake scalers are sharable within a pipe and can be used as a panel > > fitter or plane scaler. Two scalers cannot be ganged to a single plane > > to get higher scale factor but simultaneous use of one as plane scaler > > and one scaler as panel fitter is allowed. Reformatted previous patch > > series into smaller patches and addressed previous feedback inputs. > > Performed some initial testing and more testing is in works. > > Testing is done applying these patches on top of Ander's v2 atomic > > crtc patches. > > As several atomic crtc is in flight, will revisit scalers and perform > > any additional testing after atomic crtc is in place. > > Ok sprinkled a few smaller comments over a few patches, looks good overall I > think. And we discussed a bunch of the more tricky bits already in a 1:1 code > walkthrough. > > A few high-level bits: > - When splitting up patches pls dont make patches which just add > functions/structs without using them. That makes it a lot harder to > understand the big picture. The only exception is register #defines > since reviewing those against Bspec can be done mostly independently of > the code. So for functions make sure you have at least 1 use-site so > that it's clear what it does. For structures just start out with the > basic structure, then add members as you start using them. > > Since this amounts to a full rewrite of the patch series and this one > here isn't too tricky I think it's ok to not do that here. But please > follow this bkm for the next one. Sure. > > - Testcase: We definitely don't have testcases for primary plane scaling > (and panning fwiw), and iirc we also don't have any function testcases > for sprite scaling. > > - Testcases 2: I think we're also missing tests for the crtc pfit stuff. > I think it'd be good to exercise the various resolutions and different > boxing modes supported. I don't think we can do a full functional > testcase here with crc. I have written kms tests to cover, crtc pfit, primary plane scaling and sprite scaling and some combinations. Will be sending out once I take care of the review feedback and test them one more time. > > - Unfortunately we can't yet test the sharing logic fully due to lack of > atomic. But that's an open that we need to address once atomic has > landed. Especially to make sure that extreme cases (e.g. all scalers > used by planes <-> pfit enabled) work correctly. I'm covering test cases but not using atomic. Agree that once atomic is landed, scalers can be tested in atomic fashion. > > Overall I think this is ready for detailed review. Since we lack igts I recommend > that the reviewer also does the igt coverage, that's a good way to make sure all > issues are caught. > > Thanks, Daniel > > > > > > Thanks, > > Chandra > > > > Chandra Konduru (21): > > drm/i915: Adding drm helper function drm_plane_from_index(). > > drm/i915: Register definitions for skylake scalers > > drm/i915: Enable get_colorkey functions for primary plane. > > drm/i915: skylake scaler structure definitions > > drm/i915: Initialize skylake scalers > > drm/i915: Dump scaler_state too as part of dumping crtc_state > > drm/i915: Helper function to update skylake scaling ratio. > > drm/i915: Add helper function to update scaler_users in crtc_state > > drm/i915: Add atomic function to setup scalers scalers for a crtc. > > drm/i915: Helper function to detach a scaler from a plane or crtc > > drm/i915: Ensure planes begin with no scaler. > > drm/i915: Ensure colorkey and scaling aren't enabled at same time > > drm/i915: Preserve scaler state when clearing crtc_state > > drm/i915: use current scaler state during readout_hw_state. > > drm/i915: Update scaling ratio as part of crtc_compute_config > > drm/i915: Ensure setting up scalers into staged crtc_state > > drm/i915: copy staged scaler state from drm state to crtc->config. > > drm/i915: stage panel fitting scaler request for fixed mode panel > > drm/i915: Enable skylake panel fitting using skylake shared scalers > > drm/i915: Enable skylake primary plane scaling using shared scalers > > drm/i915: Enable skylake sprite plane scaling using shared scalers > > > > drivers/gpu/drm/drm_crtc.c | 20 ++ > > drivers/gpu/drm/i915/i915_reg.h | 114 +++++++++ > > drivers/gpu/drm/i915/intel_atomic.c | 157 ++++++++++++ > > drivers/gpu/drm/i915/intel_display.c | 442 > +++++++++++++++++++++++++++++++--- > > drivers/gpu/drm/i915/intel_dp.c | 7 + > > drivers/gpu/drm/i915/intel_drv.h | 109 +++++++++ > > drivers/gpu/drm/i915/intel_sprite.c | 95 ++++++-- > > include/drm/drm_crtc.h | 1 + > > 8 files changed, 895 insertions(+), 50 deletions(-) > > > > -- > > 1.7.9.5 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
