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. - 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. - 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. 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
