- 09 Nov, 2021 6 commits
-
-
Dan Carpenter authored
The "ret" variable is checked on the previous line so we know it's zero. No need to check again. Signed-off-by:
Dan Carpenter <dan.carpenter@oracle.com> Reviewed-by:
Rodrigo Vivi <rodrigo.vivi@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Currently we're only calling intel_update_active_dpll() for the bigjoiner master pipe but not for the slave. With TC ports this leads to the two pipes end up trying to use different PLLs (TC vs. TBT). What's worse we're enabling the PLL that didn't get intel_update_active_dpll() called on it at the spot where we need the clocks turned on. So we turn on the wrong PLL and the DDI is now trying to source its clock from the other PLL which is still disabled. Naturally that doesn't end so well and the DDI fails to start up. The state checker also gets a bit unhappy (which is a good thing) when it notices that one of the pipes was using the wrong PLL. Let's fix this by remembering to call intel_update_active_dpll() for both pipes. That should get the correct PLL turned on when we need it, and the state checker should also be happy. Cc: Imre Deak <imre.deak@intel.com> Cc: Manasi Navare <manasi.d.navare@intel.com> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/4434 Fixes: e12d6218 ("drm/i915: Reduce bigjoiner special casing") Signed-off-by:
Ville Syrjälä <ville.syrjala@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20211105212156.5697-1-ville.syrjala@linux.intel.comReviewed-by:
Imre Deak <imre.deak@intel.com>
-
Ville Syrjälä authored
We have to bash in a lot of registers to load the higher precision LUT modes. The locking overhead is significant, especially as we have to get this done as quickly as possible during vblank. So let's switch to unlocked accesses for these. Fortunately the LUT registers are mostly spread around such that two pipes do not have any registers on the same cacheline. So as long as commits on the same pipe are serialized (which they are) we should get away with this without angering the hardware. The only exceptions are the PREC_PIPEGCMAX registers on ilk/snb which we don't use atm as they are only used in the 12bit gamma mode. If/when we add support for that we may need to remember to still serialize those registers, though I'm not sure ilk/snb are actually affected by the same cacheline issue. I think ivb/hsw at least were, but they use a different set of registers for the precision LUT. I have a test case which is updating the LUTs on two pipes from a single atomic commit. Running that in a loop for a minute I get the following worst case with the locks in place: intel_crtc_vblank_work_start: pipe B, frame=10037, scanline=1081 intel_crtc_vblank_work_start: pipe A, frame=12274, scanline=769 intel_crtc_vblank_work_end: pipe A, frame=12274, scanline=58 intel_crtc_vblank_work_end: pipe B, frame=10037, scanline=74 And here's the worst case with the locks removed: intel_crtc_vblank_work_start: pipe B, frame=5869, scanline=1081 intel_crtc_vblank_work_start: pipe A, frame=7616, scanline=769 intel_crtc_vblank_work_end: pipe B, frame=5869, scanline=1096 intel_crtc_vblank_work_end: pipe A, frame=7616, scanline=777 The test was done on a snb using the 10bit 1024 entry LUT mode. The vtotals for the two displays are 793 and 1125. So we can see that with the locks ripped out the LUT updates are pretty nicely confined within the vblank, whereas with the locks in place we're routinely blasting past the vblank end which causes visual artifacts near the top of the screen. Signed-off-by:
Uma Shankar <uma.shankar@intel.com>
-
Ville Syrjälä authored
The pipe gamma registers are single buffered so they should only be updated during the vblank to avoid screen tearing. In fact they really should only be updated between start of vblank and frame start because that is the only time the pipe is guaranteed to be empty. Already at frame start the pipe begins to fill up with data for the next frame. Unfortunately frame start happens ~1 scanline after the start of vblank which in practice doesn't always leave us enough time to finish the gamma update in time (gamma LUTs can be several KiB of data we have to bash into the registers). However we must try our best and so we'll add a vblank work for each pipe from where we can do the gamma update. Additionally we could consider pushing frame start forward to the max of ~4 scanlines after start of vblank. But not sure that's exactly a validated configuration. As it stands the ~100 first pixels tend to make it through with the old gamma values. Even though the vblank worker is running on a high prority thread we still have to contend with C-states. If the CPU happens be in a deep C-state when the vblank interrupt arrives even the irq handler gets delayed massively (I've observed dozens of scanlines worth of latency). To avoid that problem we'll use the qos mechanism to keep the CPU awake while the vblank work is scheduled. With all this hooked up we can finally enjoy near atomic gamma updates. It even works across several pipes from the same atomic commit which previously was a total fail because we did the gamma updates for each pipe serially after waiting for all pipes to have latched the double buffered registers. In the future the DSB should take over this responsibility which will hopefully avoid some of these issues. Kudos to Lyude for finishing the actual vblank workers. Works like the proverbial train toilet. v2: Add missing intel_atomic_state fwd declaration v3: Clean up properly when not scheduling the worker v4: Clean up the rest and add tracepoints v5: s/intel_wait_for_vblank_works/intel_wait_for_vblank_workers/ (Jani,Uma) CC: Lyude Paul <lyude@redhat.com> Signed-off-by:
-
Ville Syrjälä authored
Do the vrr push before we sample the frame counter to know when the commit has been latched. Doing these in the wrong order could lead us to complete the flip before it has actually happened. Cc: Manasi Navare <manasi.d.navare@intel.com> Signed-off-by:
-
William Tseng authored
Avoid setting LP_DATA_TRANSFER when enable_lpdt is false Cc: Ville Syrjala <ville.syrjala@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Vandita Kulkarni <vandita.kulkarni@intel.com> Cc: Lee Shawn C <shawn.c.lee@intel.com> Cc: Cooper Chiou <cooper.chiou@intel.com> Signed-off-by:
William Tseng <william.tseng@intel.com> Signed-off-by:
Jani Nikula <jani.nikula@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20211109034125.11291-1-william.tseng@intel.com
-
- 08 Nov, 2021 1 commit
-
-
Radhakrishna Sripada authored
The earlier update to BW formulae broke ADL-P. Include display 13 to use TGL path for BW parameters. Fixes: c64a9a7c ("drm/i915: Update memory bandwidth formulae") Cc: Matt Roper <matthew.d.roper@intel.com> Reported-by:
José Roberto de Souza <jose.souza@intel.com> Signed-off-by:
Radhakrishna Sripada <radhakrishna.sripada@intel.com> Reviewed-by:
Caz Yokoyama <caz.yokoyama@intel.com> Reviewed-by:
-
- 05 Nov, 2021 6 commits
-
-
José Roberto de Souza authored
It was also defeatured for ADL-P and other platforms. BSpec: 55424 Signed-off-by:
-
Jani Nikula authored
Follow the filename based prefix naming. Reviewed-by:
-
Jani Nikula authored
It's all internal to intel_audio.c. Cc: Dave Airlie <airlied@redhat.com> Reviewed-by:
Lucas De Marchi <lucas.demarchi@intel.com> Signed-off-by:
-
Jani Nikula authored
Add a standalone definition of struct intel_audio_private, and note that all of it is private to intel_audio.c. v2: Rebase Cc: Dave Airlie <airlied@redhat.com> Reviewed-by:
-
Jani Nikula authored
Add name to the audio sub-struct in drm_i915_private, and remove the tautologies and other inconsistencies in the member names. v2: Call the mutex member mutex, not lock. (Ville) Cc: Dave Airlie <airlied@redhat.com> Reviewed-by:
-
Jani Nikula authored
With an anonymous struct, this can be pure hierarchical organization without code changes. We'll follow up with adding a name to the sub-struct separately. Cc: Dave Airlie <airlied@redhat.com> Reviewed-by:
-
- 04 Nov, 2021 10 commits
-
-
Radhakrishna Sripada authored
The formulae has been updated to include more variables. Make sure the code carries the same. Bspec: 64631, 54023 v2: Make GEN11 follow the default route and fix calculation of maxdebw(RK) v3: Fix div by zero on default case Correct indent for fallthrough(Jani) v4: Fix div by zero on gen11. v5: Fix 0 max_numchannels case v6: - Split gen11/gen12 algorithms - Fix RKL deburst value - Fix difference b/ween ICL and TGL algorithms - Protect deinterleave from being 0 - Warn when numchannels exceeds max_numchannels - Fix scaling of clk_max from different units - s/deinterleave/channelwidth/ in calculating peakbw - Fix off by one for num_planes TGL+ - Fix SAGV check v7: Fix div by zero error on gen11 v8: Even though the algorithm for gen11 says that we need to return derated bw for a qgv point whose planes are less than no of active planes, we return 0 for deratedbw when only one plane is allowed. We modify the algorithm to accommodate the case where no of active planes are same as the min no of planes supported by a qgv point. v9: Fix dclk scaling for dg1 Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Suggested-by:Matt Roper <matthew.d.roper@intel.com> Signed-off-by:
Anusha Srivatsa <anusha.srivatsa@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Chop vlv_sprite_update() into two halves. Fist half becomes the _noarm() variant, second part the _arm() variant. Fortunately I have already previously grouped the register writes into roughtly the correct order, so the split looks surprisingly clean. Looks like most of the hardware logic was copied from the pre-ctg sprite C, so SPSTRIDE/POS/SIZE are armed by SPSURF, while the rest are self arming. SPCONSTALPHA is the one entirely new register that didn't exist in the old sprite C, and looks like that one is self arming. The CHV pipe B CSC is also self arming, like the rest of the CHV pipe B additions. I didn't have time to capture i915_update_info numbers for these, but since all the other platforms generally showed improvements, and crucially no regression, I am fairly confident this should behave similarly. Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by:
Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
-
Ville Syrjälä authored
Chop ivb_sprite_update() into two halves. Fist half becomes the _noarm() variant, second part the _arm() variant. Fortunately I have already previously grouped the register writes into roughtly the correct order, so the split looks surprisingly clean. Didn't bother with i915_update_info numbers for this one. I expect the results to be pretty much identical to the snb numbers from the corresponding g4x+ sprite modification. Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Chop g4x_sprite_update() into two halves. Fist half becomes the _noarm() variant, second part the _arm() variant. Fortunately I have already previously grouped the register writes into roughtly the correct order, so the split looks surprisingly clean. Not much of a change in i915_update_info on these older platforms that don't have so many planes or registers to begin with. Here are the numbers from snb (totally unpatched vs. both primary plane and sprite patched applied) running kms_atomic_transition --r plane-all-transition --extended: w/o patch w/ patch Updates: 5404 Updates: 5405 | | 1us |****** 1us |****** |********* |********* 4us |*********** 4us |*********** |********** |********** 16us |** 16us |** | | 66us | 66us | | | 262us | 262us | | | 1ms | 1ms | | | 4ms | 4ms | | | 17ms | 17ms | | | Min update: 1400ns Min update: 1307ns Max update: 19809ns Max update: 20194ns Average update: 6957ns Average update: 6432ns Overruns > 100us: 0 Overruns > 100us: 0 But there seems to be a slight improvement with lockdep enabled: w/o patch w/ patch Updates: 17612 Updates: 16364 | | 1us | 1us | |****** |****** 4us |********** 4us |********** |************ |************* 16us |************* 16us |************ |*** |* 66us | 66us | | | 262us | 262us | | | 1ms | 1ms | | | 4ms | 4ms | | | 17ms | 17ms | | | Min update: 3141ns Min update: 3562ns Max update: 126450ns Max update: 73354ns Average update: 16373ns Average update: 15153ns Overruns > 250us: 0 Overruns > 250us: 0 Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by: -
Ville Syrjälä authored
Chop i9xx_plane_update() into two halves. Fist half becomes the _noarm() variant, second part the _arm() variant. Fortunately I have already previously grouped the register writes into roughtly the correct order, so the split looks surprisingly clean. One slightly surprising fact was that the CHV pipe B PRIMPOS/SIZE registers are self arming unlike their pre-ctg DSPPOS/SIZE counterparts. In fact all the new CHV pipe B registers are self arming. Also we must remind ourselves that i830/i845 are a bit borked in that all of their plane registers are self-arming. I didn't do any i915_update_info measurements for this one alone. I'll get total numbers with the corrsponding sprite plane changes. v2: Don't break my precious i830/i845 Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Chop skl_program_plane() into two halves. Fist half becomes the _noarm() variant, second part the _arm() variant. Fortunately I have already previously grouped the register writes into roughtly the correct order, so the split looks surprisingly clean. A few notable oddities I did not realize were self arming are AUX_DIST and COLOR_CTL. i915_update_info doesn't look too terrible on my cfl running kms_atomic_transition --r plane-all-transition --extended: w/o patch w/ patch Updates: 2178 Updates: 2018 | | 1us | 1us | | | 4us | 4us |***** |********* |********** 16us |********** 16us |******* |*** | 66us | 66us | | | 262us | 262us | | | 1ms | 1ms | | | 4ms | 4ms | | | 17ms | 17ms | | | Min update: 8332ns Min update: 6164ns Max update: 48758ns Max update: 31808ns Average update: 19959ns Average update: 13159ns Overruns > 100us: 0 Overruns > 100us: 0 And with lockdep enabled: w/o patch w/ patch Updates: 2177 Updates: 2172 | | 1us | 1us | | | 4us | 4us | |******* |********* 16us |********** 16us |********** |******* |* 66us | 66us | | | 262us | 262us | | | 1ms | 1ms | | | 4ms | 4ms | | | 17ms | 17ms | | | Min update: 12645ns Min update: 9980ns Max update: 50153ns Max update: 33533ns Average update: 25337ns Average update: 18245ns Overruns > 250us: 0 Overruns > 250us: 0 TODO: On icl+ everything seems to be armed by PLANE_SURF, so we can optimize this even further on modern platforms. But I think there's a bit of refactoring to be done first to figure out the best way to go about it (eg. just reusing the current skl+ functions, or doing a lower level split). TODO: Split scaler programming as well, but IIRC the scaler has some oddball double buffering behaviour on some platforms, so needs proper reverse engineering Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by: -
Ville Syrjälä authored
The amount of plane registers we have to write has been steadily increasing, putting more pressure on the vblank evasion mechanism and forcing us to increase its time budget. Let's try to take some of the pressure off by splitting plane updates into two parts: 1) write all non-self arming plane registers, ie. the registers where the write actually does nothing until a separate arming register is also written which will cause the hardware to latch the new register values at the next start of vblank 2) write all self arming plane registers, ie. registers which always just latch at the next start of vblank, and registers which also arm other registers to do so Here we just provide the mechanism, but don't actually implement the split on any platform yet. so everything stays now in the _arm() hooks. Subsequently we can move a whole bunch of stuff into the _noarm() part, especially in more modern platforms where the number of registers we have to write is also the greatest. On older platforms this is less beneficial probably, but no real reason to deviate from a common behaviour. And let's sprinkle some TODOs around the areas that will need adapting. Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Give all sprite exclusive functions/etc. a proper namespace. Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com> Signed-off-by:
-
Ville Syrjälä authored
We're currently forgetting to set the PLANE_SURF_DECRYPT flag in the async flip path. So if the hardware were to latch that bit despite this being an async flip we'd start scanning out garbage. And if it doesn't latch it then I guess we'd just end up with a weird register value that doesn't actually match the hardware state, which isn't great for anyone staring at register dumps. Similarly the async flip path also forgets to call skl_surf_address() which means the DPT address space to GGTT address space downshift is not being applied to the offset. Which means we are pointing PLANE_SURF at some random location in GGTT instead of the correct DPT page. So let's fix two birds with one stone and extract the PLANE_SURF calculation from skl_program_plane() into a small helper and use it in the async flip path as well. Cc: Anshuman Gupta <anshuman.gupta@intel.com> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: Juston Li <juston.li@intel.com> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> Cc: Uma Shankar <uma.shankar@intel.com> Cc: Karthik B S <karthik.b.s@intel.com> Signed-off-by:
-
Ville Syrjälä authored
Async flips are only capable of changing PLANE_SURF, hence we they can't easily be used with planar formats. Older platforms could require updating AUX_DIST as well, which is not possible. We'd have to make sure AUX_DIST doesn't change before allowing the async flip through. If we could get async flips with CCS then that might be interesting, but since the hw doesn't allow async flips with CCS I don't see much point in allowing this for planar formats either. No one renders their game content in YUV anyway. icl+ could in theory do this I suppose since each color plane has its own PLANE_SURF register, but I don't know if there is some magic to guarantee that both the Y and UV plane would async flip synchronously if you will. Ie. beyond just a clean tear we'd potentially get some kind of weird tear with some random mix of luma and chroma from the old and new frames. So let's just say no to async flips when scanning out planar formats. Cc: Karthik B S <karthik.b.s@intel.com> Signed-off-by:
-
- 03 Nov, 2021 12 commits
-
-
Ville Syrjälä authored
Streamline the code by using intel_de_rmw(). Signed-off-by:
-
Ville Syrjälä authored
Streamline the code by using intel_de_rmw(). Signed-off-by:
-
Ville Syrjälä authored
Streamline the code by using intel_de_rmw(). Signed-off-by:
-
Ville Syrjälä authored
Now that the link buf_trans, link training, and the combo/mg/dkl/snps phy programming are all fixed up we can allow per-lane DP drive settings on icl+. Make it so. Signed-off-by:
-
Ville Syrjälä authored
Prepare for per-lane drive settings by querying the desired vswing level per-lane. Signed-off-by:
-
Ville Syrjälä authored
Prepare for per-lane drive settings by querying the desired vswing level per-lane. Note that the code only does two loops, with each one writing the levels for two TX lanes. The register offsets also look a bit funny because each time through the loop we write to the exact same register offsets. The crucial bit is the HIP_INDEX_REG write that steers the same mmio window into different places. Signed-off-by:
-
Ville Syrjälä authored
Prepare for per-lane drive settings by querying the desired vswing level per-lane. Note that the code only does two loops, with each one writing the levels for two TX lanes. Signed-off-by:
-
Ville Syrjälä authored
Prepare for per-lane drive settings by querying the desired vswing level per-lane. Signed-off-by:
-
Ville Syrjälä authored
Program each TX lane individually so that we can start to use per-lane drive settings. Signed-off-by:
-
Javier Martinez Canillas authored
The comment mentions that the KMS is enabled by default unless either the i915.modeset module parameter or vga_text_mode_force boot option are used. But the latter does not exist and instead the nomodeset option was meant. Signed-off-by:
Javier Martinez Canillas <javierm@redhat.com> Signed-off-by:
-
Imre Deak authored
At least during hibernation the DPT mappings are lost with all stolen memory content, so suspend/resume these mappings similarly to GGTT mappings. This fixes a problem where the restoring modeset during system resume fails with pipe faults if a tiled framebuffer was active before suspend. v2: Clarify the way restore works in intel_dpt_resume()'s Docbook entry. (Chris) Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Vunny Sodhi <vunny.sodhi@intel.com> Reported-and-tested-by:Vunny Sodhi <vunny.sodhi@intel.com> Signed-off-by:
Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by:
-
Imre Deak authored
Factor out functions that are needed by the next patch to suspend/resume the memory mappings for DPT FBs. No functional change, except reordering during suspend the ggtt->invalidate(ggtt) call wrt. atomic_set(&ggtt->vm.open, open) and mutex_unlock(&ggtt->vm.mutex). This shouldn't matter due to the i915 suspend sequence being single threaded. Cc: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by:
-
- 02 Nov, 2021 5 commits
-
-
Imre Deak authored
As opposed to other GEN12 platforms ADLP provides a way to program the stride of CCS surfaces independently of the main surface stride (within the corresponding limit of the preceding and succeeding power-of-two values of the main surface stride). Using this HW feature we can remove the POT stride restriction on CCS surfaces, making the ADLP CCS FB uAPI (FB modifiers) identical to that of TGL. The HW makes the CCS stride flexible programming possible by deriving the stride from the value programmed to the PLANE_STRIDE register. After that the HW rounds up this value to the next power-of-two value and uses this for walking the pages of the main surface mapped to GTT/DPT. To align with the above scheme, introduce a scanout_stride view parameter which will be programmed to the PLANE_STRIDE register and use the mapping_stride view param to store the POT aligned value of the same. By requiring userspace to pass in FBs with a CCS stride that aligns with the main surface stride (matching the requirement of all GEN12 platforms), the scanout_stride will be the userspace main surface stride and the mapping_stride will be the POT rounded value of the same. Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> Cc: Nanley G Chery <nanley.g.chery@intel.com> Cc: Sameer Lattannavar <sameer.lattannavar@intel.com> Signed-off-by:
Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> Link: https://patchwork.freedesktop.org/patch/msgid/20211026225105.2783797-8-imre.deak@intel.com
-
Imre Deak authored
Since the surfaces of tiled FBs on ADLP are remapped it's pointless to require an alignment in the allocated object. The necessary tile-row alignment (to be programmed to the surface start register) will be ensured later when flipping to the FB. Signed-off-by:
-
Imre Deak authored
The next patch needs to distinguish between a view's mapping and scanout stride. Rename the current stride parameter to mapping_stride with the script below. mapping_stride will keep the same meaning as stride had on all platforms so far, while the meaning of it will change on ADLP. No functional changes. @@ identifier intel_fb_view; identifier i915_color_plane_view; identifier color_plane; expression e; type T; @@ struct intel_fb_view { ... struct i915_color_plane_view { ... - T stride; + T mapping_stride; ... } color_plane[e]; ... }; @@ struct i915_color_plane_view pv; @@ pv. - stride + mapping_stride @@ struct i915_color_plane_view *pvp; @@ pvp-> - stride + mapping_stride Signed-off-by: -
Imre Deak authored
During remapping CCS FBs the CCS AUX surface mapped size and offset->x,y coordinate calculations assumed a tiled layout. This works as long as the CCS surface height is aligned to 64 lines (ensuring a 4k bytes CCS surface tile layout). However this alignment is not required by the HW (and the driver doesn't enforces it either). Add the remapping logic required to remap the pages of CCS surfaces without the above alignment, assuming the natural linear layout of the CCS surface (vs. tiled main surface layout). Cc: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Fixes: 3d1adc3d ("drm/i915/adlp: Add support for remapping CCS FBs") Signed-off-by:
-
Imre Deak authored
Factor out functions needed to map contiguous FB obj pages to a GTT/DPT VMA view in the next patch. While at it s/4096/I915_GTT_PAGE_SIZE/ in add_padding_pages(). No functional changes. v2: s/4096/I915_GTT_PAGE_SIZE/ (Matthew) Signed-off-by:
Matthew Auld <matthew.auld@intel.com> Reviewed-by:
-
