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

[Ville Syrjälä]

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.

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

-- 
Ville Syrjälä
Intel OTC