Re: [PATCH 0/7] drm/i915: Add pipe scaler for Gen9 in atomic path

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Sep 21, 2016 at 07:47:37PM +0530, Maiti, Nabendu Bikash wrote:
> Hi,
> 
> 
> On 9/20/2016 1:55 PM, Ville Syrjälä wrote:
> > On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote:
> >> Following patch series add pipe scaler functionality in atomic path.The pipe
> >> scaler can be changed dynamically without modeset.Apart from default panel
> >> fitter supported scaling modes -CENTER/ASPECT/FULLSCREEN custom scaling mode
> >> mode is added as there is no such restriction on GEn9 pipe scaler.
> >
> > Some quick observations:
> > - missing any interaction with drm core, so all generic fb size checks
> >   and whatnot will not work out, I think
> Pipe scaler is not dependent on fp I think. We have fb size checks are 
> done in plane check.

You need to explain how this all interacts with the legacy pipe/plane
size == mode hdisplay/vdisplay stuff.

> 
> > - the way it's done goes against what I've suggested in the past. My
> >   idea has been to add a "fixed mode" property to connectors instead,
> >   which I think would minimize the impact on the core, and it would
> >   follow closely the way eDP/LVDS/DSI already works today.
> yes using fixed mode we can do also but I wanted to be part of crtc 
> property instead of connector property. As fixed mode is basically 
> intended for fixed mode panels.But we may use pipe scaler for fixed mode 
> and dynamic mode panels.

That doesn't say much. The fixed mode apporach, I think, might be easier
to incorporate in a way that keeps the legacy apporach working. Adding a
totally different way to configure the pipe src size will mean more weird
interactions between the properties.

Also it culd be supported with non-atomic userspace reasonably easily.
We'll need some sort of userspace for this anyway, otherwise it's just
untested/unused code.

> 
> > - There's no need to restrict the feature to gen9+ only. It should work
> >   out just fine for at least all pch platforms. gmch platforms would be
> >   more challenging
> This code I designed to use gen9+, and properties like crtc destination 
> size and offsets also exposed.There is no restrictions on modes (eg. 
> pillerbox/letterbox) and down scaling ratios as previous platforms. 
> Currently scaling mode is part of connector property and implemented as 
> legacy property. I created new scaling mode as atomic property. I think 
> gen9+ onward platforms may have proper atomic pipe scaling properties 
> and user space may use it fully dynamically without modeset.

None of that tells me why it's gen9+ only. IIRC the panel fitter
configuration been very flexible ever since ILK, so the only real
difference should be which registers to write.

> 
> > - the pfiter_recalculate thing looks pretty wrong atomic wise
> Sorry, I couldn't get it. Are you referring pipe scaler registers are 
> not written together with other registers?  pfiter_calculate only 
> calculate and stores the data for later commit. Please provide more 
> details on it.

It's going through encoder->crtc links and whanot. That's not going
to fly.

> >
> >>
> >>
> >>
> >> Nabendu Maiti (7):
> >>   drm/i915: Add pipe scaler pipe source drm property
> >>   drm/i915: Add pipe_src size property calculations in atomic path.
> >>   drm/i915: Panel fitting calculation for GEN9
> >>   drm/i915: Adding atomic fitting mode property for GEN9
> >>   drm/i915: Add pipe scaler co-ordinate and size property for Gen9
> >>   drm/i915: Update pipe-scaler according to destination size
> >>   drm/i915: Pipescaler destination size limit check on Gen9
> >>
> >>  drivers/gpu/drm/drm_atomic.c         |  35 ++++++++++
> >>  drivers/gpu/drm/drm_crtc.c           |  56 +++++++++++++++
> >>  drivers/gpu/drm/i915/intel_display.c | 128 +++++++++++++++++++++++++++++++++--
> >>  drivers/gpu/drm/i915/intel_drv.h     |   3 +
> >>  drivers/gpu/drm/i915/intel_panel.c   |  34 +++++++++-
> >>  include/drm/drm_crtc.h               |  14 ++++
> >>  include/uapi/drm/drm_mode.h          |   1 +
> >>  7 files changed, 263 insertions(+), 8 deletions(-)
> >>
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> 
> -- 
> Regards,
> Nabendu

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx