[Intel-gfx] [RFC] drm/i915/bxt: Add pipe_src size property

[Ville Syrjälä]

On Wed, Jan 06, 2016 at 08:57:57AM +0100, Daniel Vetter wrote:
> On Tue, Jan 05, 2016 at 05:18:40PM +0200, Ville Syrjälä wrote:
> > On Tue, Jan 05, 2016 at 03:50:38PM +0100, Daniel Vetter wrote:
> > > On Mon, Jan 04, 2016 at 07:06:15PM +0200, Ville Syrjälä wrote:
> > > > On Mon, Jan 04, 2016 at 11:16:39AM +0100, Maarten Lankhorst wrote:
> > > > > Hey,
> > > > > 
> > > > > Op 23-12-15 om 12:05 schreef Nabendu Maiti:
> > > > > > This patch is adding pipesource size as property as intel property.User
> > > > > > application is allowed to change the pipe source size in runtime on BXT/SKL.
> > > > > > Added the property as inteli crtc property.
> > > > > >
> > > > > > Comments and suggestions are requested for whether we can keep the
> > > > > > property as intel crtc property or move to drm level.
> > > > > >
> > > > > This property would get lost on a modeset. But why do you need a pipe_src property?
> > > > 
> > > > It's needed if we want to use the panel fitter with non-eDP/LVDS/DSI
> > > > displays.
> > > > 
> > > > Last time this came up I decided that we want to expose this via a new
> > > > "fixed_mode" property. Ie. userspace can choose the actual display
> > > > timings by setting the "fixed_mode" property with a specific mode, and
> > > > then the normal mode property will basically just control just the pipe
> > > > src size just like it already does for eDP/LVDS/DSI where we already have
> > > > the fixed_mode internally (just not exposed to usersapce). If the
> > > > fixed_mode is not specified, things will work just as they do right
> > > > now. Obviously for eDP/LVDS/DSI we will reject any request to
> > > > change/unset the fixed mode.
> > > > 
> > > > The benefit of that approach is that things will work exactly the same
> > > > way as before unless the user explicitly sets the fixed_mode, and once
> > > > it's set thigns will work exactly the same way as they have worked so
> > > > far for eDP/LVDS/DSI. Also it keeps the policy of choosing the fixed
> > > > mode strictly is userspace for external displays.
> > > > 
> > > > And as a bonus we will also expose the eDP/LVDS/DSI fixed_mode to
> > > > userspace giving userspace more information on what the actual panel
> > > > timings are. Currently userspace has to more or less guess that one
> > > > of the modes the connector claims to support has the actual panel
> > > > timings.
> > > > 
> > > > So far I've not heard any opposing opinions to this plan.
> > > 
> > > Hm yeah, pipe_src would run into all kinds of fun in conjunction with the
> > > existing fixed mode stuff we have. Just exposing the fixed as a prop might
> > > be simpler. But then we need to figure out what to do wrt the clock in the
> > > mode passed in through userspace - currently the fixed mode always wins,
> > > but for manual DRRS it would be nice if userspace could control it, and
> > > doesn't have to know that there's a fixed_mode.
> > 
> > We could have the user mode vrefresh indicate the desired refresh rate
> > of the fixed mode as well. In fact I've been wanting to add a check to
> > make sure the user mode refresh rate isn't too far off from the
> > fixed/downlclock mode vrefresh, but didn't get around to it yet.
> 
> Yeah agreed, userspace vrefresh should control (or at least be checked
> against) the fixed mode vrefresh.
> 
> > > So semantically I think only exposing the pipe src to expose panel fitters
> > > would be cleaner. Then we'd need to deal with some internal troubles to
> > > make sure we combine everything correctly, but that shouldn't be too hard
> > > really.
> > 
> > I think it's quite a bit more work since we have to redo all the fb size
> > checks and whatnot to use the property when specified. I once outlined
> > a more detailed plan for his approach too (too lazy to dig up the mail),
> > but I think the fixed mode prop is a simpler approach and won't actually
> > require much in the way of userspace changes either. It'll be enough to
> > set the property once and then even the legacy modeset ioctls will work
> > exactly like they do now for eDP/LVDS/DSI.
> 
> One downside of the fixed mode against an explicit pipe src rectangle is
> that the explict rectangle allows us to letter/pillar/stretch/move too.
> With just a fixed_mode that's much harder to pull off.

Nope. It's the same. For both cases we still need some kind of border
property to get real control over the output size/position of the pfit.
The fixed_mode/pipe_src prop would just control the input size.

> 
> I think for i915 it should be fairly ok-ish to implement this. We just
> need to move pipe src rectangle to drm_crtc_state, and adjust all the pfit
> config logic to work on the pipe level. Panels would then only replace the
> output mode with the fixed panel mode, and leave scaling/pfit selection to
> the core crtc code.
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC