<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> </head> <body bgcolor="#FFFFFF" text="#000000"> Attaching old discussion thread for easy reference. <meta http-equiv="Content-Type" content="text/html; charset=windows-1252"> <meta name="ProgId" content="Word.Document"> <meta name="Generator" content="Microsoft Word 15"> <meta name="Originator" content="Microsoft Word 15"> <link rel="File-List" href="file:///C:%5CUsers%5Cnmaiti%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_filelist.xml">  <link rel="themeData" href="file:///C:%5CUsers%5Cnmaiti%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_themedata.thmx"> <link rel="colorSchemeMapping" href="file:///C:%5CUsers%5Cnmaiti%5CAppData%5CLocal%5CTemp%5Cmsohtmlclip1%5C01%5Cclip_colorschememapping.xml">  <style>  </style> On Tue, Jan 05, 2016 at 05:18:40PM +0200, Ville Syrj�l� wrote:<o:p></o:p> > On Tue, Jan 05, 2016 at 03:50:38PM +0100, Daniel Vetter wrote:<o:p></o:p> > > On Mon, Jan 04, 2016 at 07:06:15PM +0200, Ville Syrj�l� wrote:<o:p></o:p> > > > On Mon, Jan 04, 2016 at 11:16:39AM +0100, Maarten Lankhorst wrote:<o:p></o:p> > > > > Hey,<o:p></o:p> > > > > <o:p></o:p> > > > > Op 23-12-15 om 12:05 schreef Nabendu Maiti:<o:p></o:p> > > > > > This patch is adding pipesource size as property as intel <o:p></o:p> > > > > > property.User application is allowed to change the pipe source size in runtime on BXT/SKL.<o:p></o:p> > > > > > Added the property as inteli crtc property.<o:p></o:p> > > > > ><o:p></o:p> > > > > > Comments and suggestions are requested for whether we can keep <o:p></o:p> > > > > > the property as intel crtc property or move to drm level.<o:p></o:p> > > > > ><o:p></o:p> > > > > This property would get lost on a modeset. But why do you need a pipe_src property?<o:p></o:p> > > > <o:p></o:p> > > > It's needed if we want to use the panel fitter with <o:p></o:p> > > > non-eDP/LVDS/DSI displays.<o:p></o:p> > > > <o:p></o:p> > > > Last time this came up I decided that we want to expose this via a <o:p></o:p> > > > new "fixed_mode" property. Ie. userspace can choose the actual <o:p></o:p> > > > display timings by setting the "fixed_mode" property with a <o:p></o:p> > > > specific mode, and then the normal mode property will basically <o:p></o:p> > > > just control just the pipe src size just like it already does for <o:p></o:p> > > > eDP/LVDS/DSI where we already have the fixed_mode internally (just <o:p></o:p> > > > not exposed to usersapce). If the fixed_mode is not specified, <o:p></o:p> > > > things will work just as they do right now. Obviously for <o:p></o:p> > > > eDP/LVDS/DSI we will reject any request to change/unset the fixed mode.<o:p></o:p> > > > <o:p></o:p> > > > The benefit of that approach is that things will work exactly the <o:p></o:p> > > > same way as before unless the user explicitly sets the fixed_mode, <o:p></o:p> > > > and once it's set thigns will work exactly the same way as they <o:p></o:p> > > > have worked so far for eDP/LVDS/DSI. Also it keeps the policy of <o:p></o:p> > > > choosing the fixed mode strictly is userspace for external displays.<o:p></o:p> > > > <o:p></o:p> > > > And as a bonus we will also expose the eDP/LVDS/DSI fixed_mode to <o:p></o:p> > > > userspace giving userspace more information on what the actual <o:p></o:p> > > > panel timings are. Currently userspace has to more or less guess <o:p></o:p> > > > that one of the modes the connector claims to support has the <o:p></o:p> > > > actual panel timings.<o:p></o:p> > > > <o:p></o:p> > > > So far I've not heard any opposing opinions to this plan.<o:p></o:p> > > <o:p></o:p> > > Hm yeah, pipe_src would run into all kinds of fun in conjunction <o:p></o:p> > > with the existing fixed mode stuff we have. Just exposing the fixed <o:p></o:p> > > as a prop might be simpler. But then we need to figure out what to <o:p></o:p> > > do wrt the clock in the mode passed in through userspace - currently <o:p></o:p> > > the fixed mode always wins, but for manual DRRS it would be nice if <o:p></o:p> > > userspace could control it, and doesn't have to know that there's a fixed_mode.<o:p></o:p> > <o:p></o:p> > We could have the user mode vrefresh indicate the desired refresh rate <o:p></o:p> > of the fixed mode as well. In fact I've been wanting to add a check to <o:p></o:p> > make sure the user mode refresh rate isn't too far off from the <o:p></o:p> > fixed/downlclock mode vrefresh, but didn't get around to it yet.<o:p></o:p> <o:p>�</o:p> Yeah agreed, userspace vrefresh should control (or at least be checked<o:p></o:p> against) the fixed mode vrefresh.<o:p></o:p> <o:p>�</o:p> > > So semantically I think only exposing the pipe src to expose panel <o:p></o:p> > > fitters would be cleaner. Then we'd need to deal with some internal <o:p></o:p> > > troubles to make sure we combine everything correctly, but that <o:p></o:p> > > shouldn't be too hard really.<o:p></o:p> > <o:p></o:p> > I think it's quite a bit more work since we have to redo all the fb <o:p></o:p> > size checks and whatnot to use the property when specified. I once <o:p></o:p> > outlined a more detailed plan for his approach too (too lazy to dig up <o:p></o:p> > the mail), but I think the fixed mode prop is a simpler approach and <o:p></o:p> > won't actually require much in the way of userspace changes either. <o:p></o:p> > It'll be enough to set the property once and then even the legacy <o:p></o:p> > modeset ioctls will work exactly like they do now for eDP/LVDS/DSI.<o:p></o:p> <o:p>�</o:p> 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.<o:p></o:p> With just a fixed_mode that's much harder to pull off.<o:p></o:p> <o:p>�</o:p> 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.<o:p></o:p> -Daniel<o:p></o:p> --<o:p></o:p> Daniel Vetter<o:p></o:p> Software Engineer, Intel Corporation<o:p></o:p> <a href="http://blog.ffwll.ch">http://blog.ffwll.ch</a><o:p></o:p> _______________________________________________<o:p></o:p> Intel-gfx mailing list<o:p></o:p> <a href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a><o:p></o:p> <a href="http://lists.freedesktop.org/mailman/listinfo/intel-gfx">http://lists.freedesktop.org/mailman/listinfo/intel-gfx</a><o:p></o:p> ..> <div class="moz-cite-prefix">On 10/03/2016 04:57 PM, Ville Syrj�l� wrote: </div> <blockquote cite="mid:20161003112713.GT4329@intel.com" type="cite"> <pre wrap="">On Wed, Sep 21, 2016 at 07:47:37PM +0530, Maiti, Nabendu Bikash wrote: </pre> <blockquote type="cite"> <pre wrap="">Hi, On 9/20/2016 1:55 PM, Ville Syrj�l� wrote: </pre> <blockquote type="cite"> <pre wrap="">On Tue, Aug 30, 2016 at 10:30:54AM +0530, Nabendu Maiti wrote: </pre> <blockquote type="cite"> <pre wrap="">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. </pre> </blockquote> <pre wrap="">Some quick observations: - missing any interaction with drm core, so all generic fb size checks and whatnot will not work out, I think </pre> </blockquote> <pre wrap="">Pipe scaler is not dependent on fp I think. We have fb size checks are done in plane check. </pre> </blockquote> <pre wrap="">You need to explain how this all interacts with the legacy pipe/plane size == mode hdisplay/vdisplay stuff. </pre> </blockquote> There is no direct interaction with the plane. This properties use current mode-set timings - horizontal and vertical timing as stored in pipe_src_w and pipe_src_h variables. They are filled just after pll and clock are set and adjusted modeset structures are filled up. This design will not interfere with current existing code of pipe/plane clock setting. PIpe scaler hardware will only use scaling of data not the clock. <blockquote cite="mid:20161003112713.GT4329@intel.com" type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">- 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. </pre> </blockquote> <pre wrap="">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. </pre> </blockquote> <pre wrap="">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. </pre> </blockquote> There will be no change in legacy code path. They will work as it is.� Only downside will be mixup of legacy and non-atomic use. But individually they should work as fine as before. The configuration of pipe_src is done as before. They will be changed if user explicitly want to change them. Internal checks are there if some odd unaccepted values are provided. Morever the current modeset values (hsize a vsize) are exposed to userspace through this design. If required vrefresh data can be feed to userspace to by adding another property. This way complexity in user space can be reduced by only exposing horizontal/vertical size, instead of full mode structure. <blockquote cite="mid:20161003112713.GT4329@intel.com" type="cite"> <pre wrap=""> </pre> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">- 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 </pre> </blockquote> <pre wrap="">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. </pre> </blockquote> <pre wrap="">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.</pre> </blockquote> There is no hardware pipe scaler before gen7 (VLV/BXT). VLV +CHT platform also can be supported on this design with little additional modification. Using this Letterbox/pillerbox modes in Gen7 can be programmatically selected and according to proper register write. But as far as I know old platforms has limited functionality of panel fitting either by changing clock/ doing nearby supported modeset. I understand fixed mode is the timing required by panel and adjusted mode is the nearest permissive timing available from gen platform. But using hardware scaler we don't change timing. We keep the timing intact, but modify the blended out pixel data from pipe.� simple use case I can think of: Display is showing some document blended with other planes. Double clicking should enlarge some portion of the screen and fir to the pannel without screen blanking due to modeset. .. <blockquote cite="mid:20161003112713.GT4329@intel.com" type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap="">- the pfiter_recalculate thing looks pretty wrong atomic wise </pre> </blockquote> <pre wrap="">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. </pre> </blockquote> <pre wrap="">It's going through encoder->crtc links and whanot. That's not going to fly.</pre> </blockquote> There is no use of encoder->crtc in the design, everything maintained in crtc. (Note Intial few patches using the legacy encoder->crtc iteration to find current scaling mode). Pfit_calculate just find out the pipe out size in panel if not provided by user. If target pixel area in crtc going out of panel clipping/clamming is used. Currently clamping is implemented. Regards, Nabendu Nabendu Maiti (7): <blockquote cite="mid:20161003112713.GT4329@intel.com" type="cite"> <blockquote type="cite"> <blockquote type="cite"> <blockquote type="cite"> <pre wrap=""> 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 <a class="moz-txt-link-abbreviated" href="mailto:Intel-gfx@lists.freedesktop.org">Intel-gfx@lists.freedesktop.org</a> <a class="moz-txt-link-freetext" href="https://lists.freedesktop.org/mailman/listinfo/intel-gfx">https://lists.freedesktop.org/mailman/listinfo/intel-gfx</a> </pre> </blockquote> </blockquote> <pre wrap="">-- Regards, Nabendu </pre> </blockquote> </blockquote> </body> </html>