[Intel-gfx] [PATCH] drm/i915/selftests: Memory mapping with IOMMU
From: hardikdx
When IOMMU is enabled, comparing CPU value with page failes as *CPU
does not match with page value. These values are comparible only
when IOMMU is disabled. Hence, remove comparision to run live selftest
without an issue.
Signed-off-by: hardikdx
Cc: Matthew Auld
---
.../drm/i915/gem/selftests/i915_gem_mman.c| 30 ++-
1 file changed, 2 insertions(+), 28 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 05a3b29f545e..64ecc4a32636 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -146,20 +146,7 @@ static int check_partial_mapping(struct
drm_i915_gem_object *obj,
p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
cpu = kmap(p) + offset_in_page(offset);
drm_clflush_virt_range(cpu, sizeof(*cpu));
- if (*cpu != (u32)page) {
- pr_err("Partial view for %lu [%u] (offset=%llu, size=%u [%llu,
row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected write to
page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n",
- page, n,
- view.partial.offset,
- view.partial.size,
- vma->size >> PAGE_SHIFT,
- tile->tiling ? tile_row_pages(obj) : 0,
- vma->fence ? vma->fence->id : -1, tile->tiling,
tile->stride,
- offset >> PAGE_SHIFT,
- (unsigned int)offset_in_page(offset),
- offset,
- (u32)page, *cpu);
- err = -EINVAL;
- }
+
*cpu = 0;
drm_clflush_virt_range(cpu, sizeof(*cpu));
kunmap(p);
@@ -239,20 +226,7 @@ static int check_partial_mappings(struct
drm_i915_gem_object *obj,
p = i915_gem_object_get_page(obj, offset >> PAGE_SHIFT);
cpu = kmap(p) + offset_in_page(offset);
drm_clflush_virt_range(cpu, sizeof(*cpu));
- if (*cpu != (u32)page) {
- pr_err("Partial view for %lu [%u] (offset=%llu, size=%u
[%llu, row size %u], fence=%d, tiling=%d, stride=%d) misalignment, expected
write to page (%llu + %u [0x%llx]) of 0x%x, found 0x%x\n",
- page, n,
- view.partial.offset,
- view.partial.size,
- vma->size >> PAGE_SHIFT,
- tile->tiling ? tile_row_pages(obj) : 0,
- vma->fence ? vma->fence->id : -1, tile->tiling,
tile->stride,
- offset >> PAGE_SHIFT,
- (unsigned int)offset_in_page(offset),
- offset,
- (u32)page, *cpu);
- err = -EINVAL;
- }
+
*cpu = 0;
drm_clflush_virt_range(cpu, sizeof(*cpu));
kunmap(p);
--
2.25.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3] drm/i915: Invoke another _DSM to enable MUX on HP Workstation laptops
On Mon, Apr 26, 2021 at 11:24 PM Kai-Heng Feng
wrote:
>
> On HP Fury G7 Workstations, graphics output is re-routed from Intel GFX
> to discrete GFX after S3. This is not desirable, because userspace will
> treat connected display as a new one, losing display settings.
>
> The expected behavior is to let discrete GFX drives all external
> displays.
>
> The platform in question uses ACPI method \_SB.PCI0.HGME to enable MUX.
> The method is inside the another _DSM, so add the _DSM and call it
> accordingly.
>
> I also tested some MUX-less and iGPU only laptops with that _DSM, no
> regression was found.
>
> v3:
> - Remove BXT from names.
> - Change the parameter type.
> - Fold the function into intel_modeset_init_hw().
>
> v2:
> - Forward declare struct pci_dev.
>
> Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/3113
> References:
> https://lore.kernel.org/intel-gfx/1460040732-31417-4-git-send-email-animesh.ma...@intel.com/
> Signed-off-by: Kai-Heng Feng
A gentle ping...
> ---
> drivers/gpu/drm/i915/display/intel_acpi.c| 18 ++
> drivers/gpu/drm/i915/display/intel_acpi.h| 3 +++
> drivers/gpu/drm/i915/display/intel_display.c | 2 ++
> 3 files changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_acpi.c
> b/drivers/gpu/drm/i915/display/intel_acpi.c
> index 833d0c1be4f1..d008d3976261 100644
> --- a/drivers/gpu/drm/i915/display/intel_acpi.c
> +++ b/drivers/gpu/drm/i915/display/intel_acpi.c
> @@ -13,12 +13,17 @@
> #include "intel_display_types.h"
>
> #define INTEL_DSM_REVISION_ID 1 /* For Calpella anyway... */
> +#define INTEL_DSM_FN_PLATFORM_MUX_ENABLE 0 /* No args */
> #define INTEL_DSM_FN_PLATFORM_MUX_INFO 1 /* No args */
>
> static const guid_t intel_dsm_guid =
> GUID_INIT(0x7ed873d3, 0xc2d0, 0x4e4f,
> 0xa8, 0x54, 0x0f, 0x13, 0x17, 0xb0, 0x1c, 0x2c);
>
> +static const guid_t intel_dsm_guid2 =
> + GUID_INIT(0x3e5b41c6, 0xeb1d, 0x4260,
> + 0x9d, 0x15, 0xc7, 0x1f, 0xba, 0xda, 0xe4, 0x14);
> +
> static char *intel_dsm_port_name(u8 id)
> {
> switch (id) {
> @@ -176,6 +181,19 @@ void intel_unregister_dsm_handler(void)
> {
> }
>
> +void intel_dsm_enable_mux(struct drm_i915_private *i915)
> +{
> + struct pci_dev *pdev = i915->drm.pdev;
> + acpi_handle dhandle;
> +
> + dhandle = ACPI_HANDLE(>dev);
> + if (!dhandle)
> + return;
> +
> + acpi_evaluate_dsm(dhandle, _dsm_guid2, INTEL_DSM_REVISION_ID,
> + INTEL_DSM_FN_PLATFORM_MUX_ENABLE, NULL);
> +}
> +
> /*
> * ACPI Specification, Revision 5.0, Appendix B.3.2 _DOD (Enumerate All
> Devices
> * Attached to the Display Adapter).
> diff --git a/drivers/gpu/drm/i915/display/intel_acpi.h
> b/drivers/gpu/drm/i915/display/intel_acpi.h
> index e8b068661d22..def013cf6308 100644
> --- a/drivers/gpu/drm/i915/display/intel_acpi.h
> +++ b/drivers/gpu/drm/i915/display/intel_acpi.h
> @@ -11,11 +11,14 @@ struct drm_i915_private;
> #ifdef CONFIG_ACPI
> void intel_register_dsm_handler(void);
> void intel_unregister_dsm_handler(void);
> +void intel_dsm_enable_mux(struct drm_i915_private *i915);
> void intel_acpi_device_id_update(struct drm_i915_private *i915);
> #else
> static inline void intel_register_dsm_handler(void) { return; }
> static inline void intel_unregister_dsm_handler(void) { return; }
> static inline
> +void intel_dsm_enable_mux(struct drm_i915_private *i915) { return; }
> +static inline
> void intel_acpi_device_id_update(struct drm_i915_private *i915) { return; }
> #endif /* CONFIG_ACPI */
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index a10e26380ef3..d79dae370b20 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -11472,6 +11472,8 @@ void intel_modeset_init_hw(struct drm_i915_private
> *i915)
> {
> struct intel_cdclk_state *cdclk_state;
>
> + intel_dsm_enable_mux(i915);
> +
> if (!HAS_DISPLAY(i915))
> return;
>
> --
> 2.30.2
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure
On Tue, May 11, 2021 at 2:06 AM Albert Astals Cid wrote:
>
> Yes, I also have the same.
>
> I git bisected that and found this to be the cause, i started a new email
> thread because i couldn't find this email ^_^
Should be fixed by
https://cgit.freedesktop.org/drm-tip/commit/?id=acca7762eb71bc05a8f28d29320d193150051f79
Kai-Heng
>
> Cheers,
> Albert
>
> El dilluns, 10 de maig de 2021, a les 10:07:33 (CEST), Emanuele Panigati va
> escriure:
> > Hi,
> > on my Dell XPS 15 9570 laptop I might have a regression with Arch Linux
> > (kernel 5.12.2-arch1-1: during boot the laptop monitor goes black while
> > external monitors still works...
> >
> >
> > Panich
> >
> >
> > Il giorno lun 11 gen 2021 alle ore 19:28 Ville Syrjälä <
> > ville.syrj...@linux.intel.com> ha scritto:
> >
> > > On Thu, Jan 07, 2021 at 08:20:25PM +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä
> > > >
> > > > Some new eDP panels don't like to operate at the max parameters, and
> > > > instead we need to go for an optimal confiugration. That unfortunately
> > > > doesn't work with older eDP panels which are generally only guaranteed
> > > > to work at the max parameters.
> > > >
> > > > To solve these two conflicting requirements let's start with the optimal
> > > > setup, and if that fails we start again with the max parameters. The
> > > > downside is probably an extra modeset when we switch strategies but
> > > > I don't see a good way to avoid that.
> > > >
> > > > For a bit of history we first tried to go for the fast+narrow in
> > > > commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config
> > > > fast and narrow"). but that had to be reverted due to regression
> > > > on older panels in commit f11cb1c19ad0 ("drm/i915/dp: revert back
> > > > to max link rate and lane count on eDP"). So now we try to get
> > > > the best of both worlds by using both strategies.
> > >
> > > Pushed. Fingers crossed for no regressions...
> > >
> > > --
> > > Ville Syrjälä
> > > Intel
> > >
> >
>
>
>
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 00/97] Basic GuC submission support in the i915
On Sun, 09 May 2021 16:11:43 -0700, Jason Ekstrand wrote: > > Yes, landing GuC support may be the first step in removing execlist > support. The inevitable reality is that GPU scheduling is coming and > likely to be there only path in the not-too-distant future. (See also > the ongoing thread with AMD about fences.) I'm not going to pass > judgement on whether or not this is a good thing. I'm just reading the > winds and, in my view, this is where things are headed for good or ill. > > In answer to the question above, the answer to "what do we gain from > GuC?" may soon be, "you get to use your GPU." We're not there yet and, > again, I'm not necessarily advocating for it, but that is likely where > things are headed. > > A firmware-based submission model isn't a bad design IMO and, aside from > the firmware freedom issues, I think there are actual advantages to the > model. Immediately, it'll unlock a few features like parallel submission > (more on that in a bit) and long-running compute because they're > implemented in GuC and the work to implement them properly in the > execlist scheduler is highly non-trivial. Longer term, it may (no > guarantees) unlock some performance by getting the kernel out of the way. I believe another main reason for GuC is support for HW based virtualization like SRIOV. The only way to support SRIOV with execlists would be to statically partition the GPU between VM's, any dynamic partitioning needs something in HW. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 5/5] drm/i915/display/xelpd: Implement Wa_14013475917
On Sat, Apr 17, 2021 at 05:21:26PM -0700, José Roberto de Souza wrote: > This workaround requires that VIDEO_DIP_ENABLE_VSC_HSW is never set > with PSR. > > BSpec: 54369 > BSpec: 54077 Reviewed-by: Radhakrishna Sripada > Cc: Matt Atwood > Cc: Gwan-gyeong Mun > Signed-off-by: José Roberto de Souza > --- > drivers/gpu/drm/i915/display/intel_hdmi.c | 5 + > 1 file changed, 5 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c > b/drivers/gpu/drm/i915/display/intel_hdmi.c > index de7328685d40..3876a52642a4 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > @@ -531,6 +531,11 @@ void hsw_write_infoframe(struct intel_encoder *encoder, > hsw_dip_data_reg(dev_priv, cpu_transcoder, type, > i >> 2), > 0); > > + /* Wa_14013475917 */ > + if (DISPLAY_VER(dev_priv) == 13 && crtc_state->has_psr && > + type == DP_SDP_VSC) > + return; > + > val |= hsw_infoframe_enable(type); > intel_de_write(dev_priv, ctl_reg, val); > intel_de_posting_read(dev_priv, ctl_reg); > -- > 2.31.1 > > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/5] drm/i915/display: Drop dead code from hsw_read_infoframe()
On Sat, Apr 17, 2021 at 05:21:25PM -0700, José Roberto de Souza wrote:
> HSW_TVIDEO_DIP_CTL is read but not used.
>
Reviewed-by: Radhakrishna Sripada
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
> b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 47a8f0a1c5e2..de7328685d40 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -542,11 +542,9 @@ void hsw_read_infoframe(struct intel_encoder *encoder,
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> - u32 val, *data = frame;
> + u32 *data = frame;
> int i;
>
> - val = intel_de_read(dev_priv, HSW_TVIDEO_DIP_CTL(cpu_transcoder));
> -
> for (i = 0; i < len; i += 4)
> *data++ = intel_de_read(dev_priv,
> hsw_dip_data_reg(dev_priv,
> cpu_transcoder, type, i >> 2));
> --
> 2.31.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/5] drm/i915/display: Drop duplicated code in intel_dp_set_infoframes()
On Sat, Apr 17, 2021 at 05:21:24PM -0700, José Roberto de Souza wrote:
> No functional changes in here.
>
> Cc: Matt Atwood
Reviewed-by: Radhakrishna Sripada
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 17 ++---
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 72bcc10cae4f..cf380f98d54c 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2865,24 +2865,19 @@ void intel_dp_set_infoframes(struct intel_encoder
> *encoder,
> u32 dip_enable = VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW |
>VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW |
>VIDEO_DIP_ENABLE_SPD_HSW | VIDEO_DIP_ENABLE_DRM_GLK;
> - u32 val = intel_de_read(dev_priv, reg);
> + u32 val = intel_de_read(dev_priv, reg) & ~dip_enable;
>
> /* TODO: Add DSC case (DIP_ENABLE_PPS) */
> /* When PSR is enabled, this routine doesn't disable VSC DIP */
> - if (crtc_state->has_psr)
> - val &= ~dip_enable;
> - else
> - val &= ~(dip_enable | VIDEO_DIP_ENABLE_VSC_HSW);
> -
> - if (!enable) {
> - intel_de_write(dev_priv, reg, val);
> - intel_de_posting_read(dev_priv, reg);
> - return;
> - }
> + if (!crtc_state->has_psr)
> + val &= ~VIDEO_DIP_ENABLE_VSC_HSW;
>
> intel_de_write(dev_priv, reg, val);
> intel_de_posting_read(dev_priv, reg);
>
> + if (!enable)
> + return;
> +
> /* When PSR is enabled, VSC SDP is handled by PSR routine */
> if (!crtc_state->has_psr)
> intel_write_dp_sdp(encoder, crtc_state, DP_SDP_VSC);
> --
> 2.31.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/5] drm/i915/display: Replace intel_psr_enabled() calls by intel_crtc_state check
On Sat, Apr 17, 2021 at 05:21:23PM -0700, José Roberto de Souza wrote:
> All of this places don't need to intel_psr_enabled() that will lock
> psr mutex, check state and unlock.
>
> Instead it can directly check PSR state in intel_crtc_state, the only
> place that was not possible was intel_read_dp_vsc_sdp() but since
> "drm/i915/display: Fill PSR state during hardware configuration read
> out" it is possible.
>
Reviewed-by: Radhakrishna Sripada
> Cc: Gwan-gyeong Mun
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 5ee953aaa00c..72bcc10cae4f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2861,7 +2861,6 @@ void intel_dp_set_infoframes(struct intel_encoder
> *encoder,
>const struct drm_connector_state *conn_state)
> {
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> i915_reg_t reg = HSW_TVIDEO_DIP_CTL(crtc_state->cpu_transcoder);
> u32 dip_enable = VIDEO_DIP_ENABLE_AVI_HSW | VIDEO_DIP_ENABLE_GCP_HSW |
>VIDEO_DIP_ENABLE_VS_HSW | VIDEO_DIP_ENABLE_GMP_HSW |
> @@ -2870,7 +2869,7 @@ void intel_dp_set_infoframes(struct intel_encoder
> *encoder,
>
> /* TODO: Add DSC case (DIP_ENABLE_PPS) */
> /* When PSR is enabled, this routine doesn't disable VSC DIP */
> - if (intel_psr_enabled(intel_dp))
> + if (crtc_state->has_psr)
> val &= ~dip_enable;
> else
> val &= ~(dip_enable | VIDEO_DIP_ENABLE_VSC_HSW);
> @@ -2885,7 +2884,7 @@ void intel_dp_set_infoframes(struct intel_encoder
> *encoder,
> intel_de_posting_read(dev_priv, reg);
>
> /* When PSR is enabled, VSC SDP is handled by PSR routine */
> - if (!intel_psr_enabled(intel_dp))
> + if (!crtc_state->has_psr)
> intel_write_dp_sdp(encoder, crtc_state, DP_SDP_VSC);
>
> intel_write_dp_sdp(encoder, crtc_state,
> HDMI_PACKET_TYPE_GAMUT_METADATA);
> @@ -3012,14 +3011,13 @@ static void intel_read_dp_vsc_sdp(struct
> intel_encoder *encoder,
> struct drm_dp_vsc_sdp *vsc)
> {
> struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> unsigned int type = DP_SDP_VSC;
> struct dp_sdp sdp = {};
> int ret;
>
> /* When PSR is enabled, VSC SDP is handled by PSR routine */
> - if (intel_psr_enabled(intel_dp))
> + if (crtc_state->has_psr)
> return;
>
> if ((crtc_state->infoframes.enable &
> --
> 2.31.1
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915/display: Fill PSR state during hardware configuration read out
On Sat, Apr 17, 2021 at 05:21:22PM -0700, José Roberto de Souza wrote:
> So far if we had a mismatch between the state asked and what was
> programmed in hardware for PSR, this mismatch would go unnoticed.
>
> So here adding the PSR to the hardware configuration readout,
> EDP_PSR_CTL and EDP_PSR2_CTL can't be directly read because its state
> flips due to other factors like frontbuffer modifications and CRC.
>
Minor nit-pick below with that fixed
Reviewed-by: Radhakrishna Sripada
> Cc: Gwan-gyeong Mun
> Signed-off-by: José Roberto de Souza
> ---
> drivers/gpu/drm/i915/display/intel_ddi.c | 2 +
> drivers/gpu/drm/i915/display/intel_display.c | 5 +++
> drivers/gpu/drm/i915/display/intel_psr.c | 47
> drivers/gpu/drm/i915/display/intel_psr.h | 3 ++
> 4 files changed, 57 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 4ef573883412..f69ed3c4c30a 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -3707,6 +3707,8 @@ static void intel_ddi_get_config(struct intel_encoder
> *encoder,
>
> intel_read_dp_sdp(encoder, pipe_config,
> HDMI_PACKET_TYPE_GAMUT_METADATA);
> intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC);
> +
> + intel_psr_get_config(encoder, pipe_config);
> }
>
> void intel_ddi_get_clock(struct intel_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 9c13d0ac022b..ecdca523e364 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8350,6 +8350,11 @@ intel_pipe_config_compare(const struct
> intel_crtc_state *current_config,
> PIPE_CONF_CHECK_I(vrr.flipline);
> PIPE_CONF_CHECK_I(vrr.pipeline_full);
>
> + PIPE_CONF_CHECK_BOOL(has_psr);
> + PIPE_CONF_CHECK_BOOL(has_psr2);
> + PIPE_CONF_CHECK_BOOL(enable_psr2_sel_fetch);
> + PIPE_CONF_CHECK_I(dc3co_exitline);
> +
> #undef PIPE_CONF_CHECK_X
> #undef PIPE_CONF_CHECK_I
> #undef PIPE_CONF_CHECK_BOOL
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 4ad756e238c5..bd7997a3ef7c 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -886,6 +886,53 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
> crtc_state->infoframes.enable |=
> intel_hdmi_infoframe_enable(DP_SDP_VSC);
> }
>
> +void intel_psr_get_config(struct intel_encoder *encoder,
> + struct intel_crtc_state *pipe_config)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> + struct intel_dp *intel_dp;
> + u32 val;
> +
> + if (!dig_port)
> + return;
> +
> + intel_dp = _port->dp;
> + if (!CAN_PSR(intel_dp))
> + return;
> +
> + mutex_lock(_dp->psr.lock);
> + if (!intel_dp->psr.enabled) {
goto unlock: should suffice here instead of unlock and return.
-RK
> + mutex_unlock(_dp->psr.lock);
> + return;
> + }
> +
> + /*
> + * Not possible to read EDP_PSR/PSR2_CTL registers as it is
> + * enabled/disabled because of frontbuffer tracking and others.
> + */
> + pipe_config->has_psr = true;
> + pipe_config->has_psr2 = intel_dp->psr.psr2_enabled;
> + pipe_config->infoframes.enable |=
> intel_hdmi_infoframe_enable(DP_SDP_VSC);
> +
> + if (!intel_dp->psr.psr2_enabled)
> + goto unlock;
> +
> + if (HAS_PSR2_SEL_FETCH(dev_priv)) {
> + val = intel_de_read(dev_priv,
> PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder));
> + if (val & PSR2_MAN_TRK_CTL_ENABLE)
> + pipe_config->enable_psr2_sel_fetch = true;
> + }
> +
> + if (DISPLAY_VER(dev_priv) >= 12) {
> + val = intel_de_read(dev_priv,
> EXITLINE(intel_dp->psr.transcoder));
> + val &= EXITLINE_MASK;
> + pipe_config->dc3co_exitline = val;
> + }
> +unlock:
> + mutex_unlock(_dp->psr.lock);
> +}
> +
> static void intel_psr_activate(struct intel_dp *intel_dp)
> {
> struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> b/drivers/gpu/drm/i915/display/intel_psr.h
> index 0491a49ffd50..e3db85e97f4c 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -17,6 +17,7 @@ struct intel_crtc;
> struct intel_atomic_state;
> struct intel_plane_state;
> struct intel_plane;
> +struct intel_encoder;
>
> void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> void intel_psr_enable(struct intel_dp *intel_dp,
> @@ -37,6 +38,8 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> void intel_psr_init(struct intel_dp
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
On Mon, May 10, 2021 at 02:49:44PM +0100, David Woodhouse wrote: > On Mon, 2021-05-10 at 13:55 +0200, Mauro Carvalho Chehab wrote: > > This patch series is doing conversion only when using ASCII makes > > more sense than using UTF-8. > > > > See, a number of converted documents ended with weird characters > > like ZERO WIDTH NO-BREAK SPACE (U+FEFF) character. This specific > > character doesn't do any good. > > > > Others use NO-BREAK SPACE (U+A0) instead of 0x20. Harmless, until > > someone tries to use grep[1]. > > Replacing those makes sense. But replacing emdashes — which are a > distinct character that has no direct replacement in ASCII and which > people do *deliberately* use instead of hyphen-minus — does not. I regularly use --- for em-dashes and -- for en-dashes. Markdown will automatically translate 3 ASCII hypens to em-dashes, and 2 ASCII hyphens to en-dashes. It's much, much easier for me to type 2 or 3 hypens into my text editor of choice than trying to enter the UTF-8 characters. If we can make sphinx do this translation, maybe that's the best way of dealing with these two characters? Cheers, - Ted ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/display: Try YCbCr420 color when RGB fails (rev2)
== Series Details == Series: drm/i915/display: Try YCbCr420 color when RGB fails (rev2) URL : https://patchwork.freedesktop.org/series/89842/ State : success == Summary == CI Bug Log - changes from CI_DRM_10063_full -> Patchwork_20094_full Summary --- **SUCCESS** No regressions found. New tests - New tests have been introduced between CI_DRM_10063_full and Patchwork_20094_full: ### New IGT tests (10) ### * igt@gem_exec_reloc@basic-wide-active: - Statuses : - Exec time: [None] s * igt@gem_wait@await: - Statuses : - Exec time: [None] s * igt@kms_flip@flip-vs-rmfb-interruptible@a-vga1: - Statuses : 1 pass(s) - Exec time: [15.17] s * igt@kms_flip@flip-vs-rmfb-interruptible@b-vga1: - Statuses : 1 pass(s) - Exec time: [15.16] s * igt@kms_flip@plain-flip-fb-recreate-interruptible@a-vga1: - Statuses : 1 pass(s) - Exec time: [15.45] s * igt@kms_flip@plain-flip-fb-recreate-interruptible@b-vga1: - Statuses : 1 pass(s) - Exec time: [15.45] s * igt@perf_pmu@busy-double-start: - Statuses : - Exec time: [None] s * igt@perf_pmu@busy-idle-no-semaphores: - Statuses : - Exec time: [None] s * igt@perf_pmu@idle: - Statuses : - Exec time: [None] s * igt@prime_busy@before: - Statuses : - Exec time: [None] s Known issues Here are the changes found in Patchwork_20094_full that come from known issues: ### IGT changes ### Issues hit * igt@gem_ctx_persistence@clone: - shard-snb: NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#1099]) +4 similar issues [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-snb7/igt@gem_ctx_persiste...@clone.html * igt@gem_ctx_ringsize@active@bcs0: - shard-skl: NOTRUN -> [INCOMPLETE][2] ([i915#3316]) [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-skl6/igt@gem_ctx_ringsize@act...@bcs0.html * igt@gem_ctx_shared@q-in-order: - shard-snb: NOTRUN -> [SKIP][3] ([fdo#109271]) +328 similar issues [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-snb2/igt@gem_ctx_sha...@q-in-order.html * igt@gem_exec_fair@basic-deadline: - shard-skl: NOTRUN -> [FAIL][4] ([i915#2846]) [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-skl10/igt@gem_exec_f...@basic-deadline.html * igt@gem_exec_fair@basic-flow@rcs0: - shard-tglb: [PASS][5] -> [FAIL][6] ([i915#2842]) [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10063/shard-tglb3/igt@gem_exec_fair@basic-f...@rcs0.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-tglb3/igt@gem_exec_fair@basic-f...@rcs0.html * igt@gem_exec_fair@basic-none@vcs0: - shard-kbl: NOTRUN -> [FAIL][7] ([i915#2842]) +2 similar issues [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-kbl2/igt@gem_exec_fair@basic-n...@vcs0.html * igt@gem_exec_fair@basic-pace@rcs0: - shard-kbl: [PASS][8] -> [FAIL][9] ([i915#2842]) [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10063/shard-kbl7/igt@gem_exec_fair@basic-p...@rcs0.html [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-kbl3/igt@gem_exec_fair@basic-p...@rcs0.html * igt@gem_exec_fair@basic-sync@rcs0: - shard-kbl: [PASS][10] -> [SKIP][11] ([fdo#109271]) [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10063/shard-kbl3/igt@gem_exec_fair@basic-s...@rcs0.html [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-kbl1/igt@gem_exec_fair@basic-s...@rcs0.html * igt@gem_exec_reloc@basic-wide-active@vcs1: - shard-iclb: NOTRUN -> [FAIL][12] ([i915#2389]) [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-iclb1/igt@gem_exec_reloc@basic-wide-act...@vcs1.html * igt@gem_exec_whisper@basic-queues-all: - shard-glk: [PASS][13] -> [DMESG-WARN][14] ([i915#118] / [i915#95]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10063/shard-glk4/igt@gem_exec_whis...@basic-queues-all.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-glk4/igt@gem_exec_whis...@basic-queues-all.html * igt@gem_userptr_blits@vma-merge: - shard-apl: NOTRUN -> [FAIL][15] ([i915#3318]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-apl7/igt@gem_userptr_bl...@vma-merge.html * igt@gen9_exec_parse@allowed-single: - shard-skl: [PASS][16] -> [DMESG-WARN][17] ([i915#1436] / [i915#716]) [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10063/shard-skl7/igt@gen9_exec_pa...@allowed-single.html [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/shard-skl4/igt@gen9_exec_pa...@allowed-single.html * igt@i915_suspend@fence-restore-tiled2untiled: - shard-kbl: NOTRUN -> [DMESG-WARN][18] ([i915#180])
Re: [Intel-gfx] [RFC PATCH 00/97] Basic GuC submission support in the i915
Daniel Vetter writes: > On Mon, May 10, 2021 at 3:55 PM Martin Peres wrote: >> >> On 10/05/2021 02:11, Jason Ekstrand wrote: >> > On May 9, 2021 12:12:36 Martin Peres wrote: >> > >> >> Hi, >> >> >> >> On 06/05/2021 22:13, Matthew Brost wrote: >> >>> Basic GuC submission support. This is the first bullet point in the >> >>> upstreaming plan covered in the following RFC [1]. >> >>> >> >>> At a very high level the GuC is a piece of firmware which sits between >> >>> the i915 and the GPU. It offloads some of the scheduling of contexts >> >>> from the i915 and programs the GPU to submit contexts. The i915 >> >>> communicates with the GuC and the GuC communicates with the GPU. >> >> >> >> May I ask what will GuC command submission do that execlist won't/can't >> >> do? And what would be the impact on users? Even forgetting the troubled >> >> history of GuC (instability, performance regression, poor level of user >> >> support, 6+ years of trying to upstream it...), adding this much code >> >> and doubling the amount of validation needed should come with a >> >> rationale making it feel worth it... and I am not seeing here. Would you >> >> mind providing the rationale behind this work? >> >> >> >>> >> >>> GuC submission will be disabled by default on all current upstream >> >>> platforms behind a module parameter - enable_guc. A value of 3 will >> >>> enable submission and HuC loading via the GuC. GuC submission should >> >>> work on all gen11+ platforms assuming the GuC firmware is present. >> >> >> >> What is the plan here when it comes to keeping support for execlist? I >> >> am afraid that landing GuC support in Linux is the first step towards >> >> killing the execlist, which would force users to use proprietary >> >> firmwares that even most Intel engineers have little influence over. >> >> Indeed, if "drm/i915/guc: Disable semaphores when using GuC scheduling" >> >> which states "Disable semaphores when using GuC scheduling as semaphores >> >> are broken in the current GuC firmware." is anything to go by, it means >> >> that even Intel developers seem to prefer working around the GuC >> >> firmware, rather than fixing it. >> > >> > Yes, landing GuC support may be the first step in removing execlist >> > support. The inevitable reality is that GPU scheduling is coming and >> > likely to be there only path in the not-too-distant future. (See also >> > the ongoing thread with AMD about fences.) I'm not going to pass >> > judgement on whether or not this is a good thing. I'm just reading the >> > winds and, in my view, this is where things are headed for good or ill. >> > >> > In answer to the question above, the answer to "what do we gain from >> > GuC?" may soon be, "you get to use your GPU." We're not there yet and, >> > again, I'm not necessarily advocating for it, but that is likely where >> > things are headed. >> >> This will be a sad day, especially since it seems fundamentally opposed >> with any long-term support, on top of taking away user freedom to >> fix/tweak their system when Intel won't. >> >> > A firmware-based submission model isn't a bad design IMO and, aside from >> > the firmware freedom issues, I think there are actual advantages to the >> > model. Immediately, it'll unlock a few features like parallel submission >> > (more on that in a bit) and long-running compute because they're >> > implemented in GuC and the work to implement them properly in the >> > execlist scheduler is highly non-trivial. Longer term, it may (no >> > guarantees) unlock some performance by getting the kernel out of the way. >> >> Oh, I definitely agree with firmware-based submission model not being a >> bad design. I was even cheering for it in 2015. Experience with it made >> me regret that deeply since :s >> >> But with the DRM scheduler being responsible for most things, I fail to >> see what we could offload in the GuC except context switching (like >> every other manufacturer). The problem is, the GuC does way more than >> just switching registers in bulk, and if the number of revisions of the >> GuC is anything to go by, it is way too complex for me to feel >> comfortable with it. > > We need to flesh out that part of the plan more, but we're not going > to use drm scheduler for everything. It's only to handle the dma-fence > legacy side of things, which means: > - timeout handling for batches that take too long > - dma_fence dependency sorting/handling > - boosting of context from display flips (currently missing, needs to > be ported from drm/i915) > > The actual round-robin/preempt/priority handling is still left to the > backend, in this case here the fw. So there's large chunks of > code/functionality where drm/scheduler wont be involved in, and like > Jason says: The hw direction winds definitely blow in the direction > that this is all handled in hw. > I agree with Martin on this. Given that using GuC currently involves making your open-source graphics stack rely on a closed-source
Re: [Intel-gfx] [RFC PATCH 00/97] Basic GuC submission support in the i915
On Mon, May 10, 2021 at 3:55 PM Martin Peres wrote: > > On 10/05/2021 02:11, Jason Ekstrand wrote: > > On May 9, 2021 12:12:36 Martin Peres wrote: > > > >> Hi, > >> > >> On 06/05/2021 22:13, Matthew Brost wrote: > >>> Basic GuC submission support. This is the first bullet point in the > >>> upstreaming plan covered in the following RFC [1]. > >>> > >>> At a very high level the GuC is a piece of firmware which sits between > >>> the i915 and the GPU. It offloads some of the scheduling of contexts > >>> from the i915 and programs the GPU to submit contexts. The i915 > >>> communicates with the GuC and the GuC communicates with the GPU. > >> > >> May I ask what will GuC command submission do that execlist won't/can't > >> do? And what would be the impact on users? Even forgetting the troubled > >> history of GuC (instability, performance regression, poor level of user > >> support, 6+ years of trying to upstream it...), adding this much code > >> and doubling the amount of validation needed should come with a > >> rationale making it feel worth it... and I am not seeing here. Would you > >> mind providing the rationale behind this work? > >> > >>> > >>> GuC submission will be disabled by default on all current upstream > >>> platforms behind a module parameter - enable_guc. A value of 3 will > >>> enable submission and HuC loading via the GuC. GuC submission should > >>> work on all gen11+ platforms assuming the GuC firmware is present. > >> > >> What is the plan here when it comes to keeping support for execlist? I > >> am afraid that landing GuC support in Linux is the first step towards > >> killing the execlist, which would force users to use proprietary > >> firmwares that even most Intel engineers have little influence over. > >> Indeed, if "drm/i915/guc: Disable semaphores when using GuC scheduling" > >> which states "Disable semaphores when using GuC scheduling as semaphores > >> are broken in the current GuC firmware." is anything to go by, it means > >> that even Intel developers seem to prefer working around the GuC > >> firmware, rather than fixing it. > > > > Yes, landing GuC support may be the first step in removing execlist > > support. The inevitable reality is that GPU scheduling is coming and > > likely to be there only path in the not-too-distant future. (See also > > the ongoing thread with AMD about fences.) I'm not going to pass > > judgement on whether or not this is a good thing. I'm just reading the > > winds and, in my view, this is where things are headed for good or ill. > > > > In answer to the question above, the answer to "what do we gain from > > GuC?" may soon be, "you get to use your GPU." We're not there yet and, > > again, I'm not necessarily advocating for it, but that is likely where > > things are headed. > > This will be a sad day, especially since it seems fundamentally opposed > with any long-term support, on top of taking away user freedom to > fix/tweak their system when Intel won't. > > > A firmware-based submission model isn't a bad design IMO and, aside from > > the firmware freedom issues, I think there are actual advantages to the > > model. Immediately, it'll unlock a few features like parallel submission > > (more on that in a bit) and long-running compute because they're > > implemented in GuC and the work to implement them properly in the > > execlist scheduler is highly non-trivial. Longer term, it may (no > > guarantees) unlock some performance by getting the kernel out of the way. > > Oh, I definitely agree with firmware-based submission model not being a > bad design. I was even cheering for it in 2015. Experience with it made > me regret that deeply since :s > > But with the DRM scheduler being responsible for most things, I fail to > see what we could offload in the GuC except context switching (like > every other manufacturer). The problem is, the GuC does way more than > just switching registers in bulk, and if the number of revisions of the > GuC is anything to go by, it is way too complex for me to feel > comfortable with it. We need to flesh out that part of the plan more, but we're not going to use drm scheduler for everything. It's only to handle the dma-fence legacy side of things, which means: - timeout handling for batches that take too long - dma_fence dependency sorting/handling - boosting of context from display flips (currently missing, needs to be ported from drm/i915) The actual round-robin/preempt/priority handling is still left to the backend, in this case here the fw. So there's large chunks of code/functionality where drm/scheduler wont be involved in, and like Jason says: The hw direction winds definitely blow in the direction that this is all handled in hw. > >> In the same vein, I have another concern related to the impact of GuC on > >> Linux's stable releases. Let's say that in 3 years, a new application > >> triggers a bug in command submission inside the firmware. Given that the > >> Linux community cannot patch the
Re: [Intel-gfx] [RFC PATCH 00/97] Basic GuC submission support in the i915
On May 10, 2021 08:55:55 Martin Peres wrote: On 10/05/2021 02:11, Jason Ekstrand wrote: On May 9, 2021 12:12:36 Martin Peres wrote: Hi, On 06/05/2021 22:13, Matthew Brost wrote: Basic GuC submission support. This is the first bullet point in the upstreaming plan covered in the following RFC [1]. At a very high level the GuC is a piece of firmware which sits between the i915 and the GPU. It offloads some of the scheduling of contexts from the i915 and programs the GPU to submit contexts. The i915 communicates with the GuC and the GuC communicates with the GPU. May I ask what will GuC command submission do that execlist won't/can't do? And what would be the impact on users? Even forgetting the troubled history of GuC (instability, performance regression, poor level of user support, 6+ years of trying to upstream it...), adding this much code and doubling the amount of validation needed should come with a rationale making it feel worth it... and I am not seeing here. Would you mind providing the rationale behind this work? GuC submission will be disabled by default on all current upstream platforms behind a module parameter - enable_guc. A value of 3 will enable submission and HuC loading via the GuC. GuC submission should work on all gen11+ platforms assuming the GuC firmware is present. What is the plan here when it comes to keeping support for execlist? I am afraid that landing GuC support in Linux is the first step towards killing the execlist, which would force users to use proprietary firmwares that even most Intel engineers have little influence over. Indeed, if "drm/i915/guc: Disable semaphores when using GuC scheduling" which states "Disable semaphores when using GuC scheduling as semaphores are broken in the current GuC firmware." is anything to go by, it means that even Intel developers seem to prefer working around the GuC firmware, rather than fixing it. Yes, landing GuC support may be the first step in removing execlist support. The inevitable reality is that GPU scheduling is coming and likely to be there only path in the not-too-distant future. (See also the ongoing thread with AMD about fences.) I'm not going to pass judgement on whether or not this is a good thing. I'm just reading the winds and, in my view, this is where things are headed for good or ill. In answer to the question above, the answer to "what do we gain from GuC?" may soon be, "you get to use your GPU." We're not there yet and, again, I'm not necessarily advocating for it, but that is likely where things are headed. This will be a sad day, especially since it seems fundamentally opposed with any long-term support, on top of taking away user freedom to fix/tweak their system when Intel won't. A firmware-based submission model isn't a bad design IMO and, aside from the firmware freedom issues, I think there are actual advantages to the model. Immediately, it'll unlock a few features like parallel submission (more on that in a bit) and long-running compute because they're implemented in GuC and the work to implement them properly in the execlist scheduler is highly non-trivial. Longer term, it may (no guarantees) unlock some performance by getting the kernel out of the way. Oh, I definitely agree with firmware-based submission model not being a bad design. I was even cheering for it in 2015. Experience with it made me regret that deeply since :s But with the DRM scheduler being responsible for most things, I fail to see what we could offload in the GuC except context switching (like every other manufacturer). The problem is, the GuC does way more than just switching registers in bulk, and if the number of revisions of the GuC is anything to go by, it is way too complex for me to feel comfortable with it. It's more than just bulk register writes. When it comes to load-balancing multiple GPU users, firmware can theoretically preempt and switch faster leading to more efficient time-slicing. All we really need the DRM scheduler for is handling implicit dma_fence dependencies between different applications. In the same vein, I have another concern related to the impact of GuC on Linux's stable releases. Let's say that in 3 years, a new application triggers a bug in command submission inside the firmware. Given that the Linux community cannot patch the GuC firmware, how likely is it that Intel would release a new GuC version? That would not be necessarily such a big problem if newer versions of the GuC could easily be backported to this potentially-decade-old Linux version, but given that the GuC seems to have ABI-breaking changes on a monthly cadence (we are at major version 60 *already*? :o), I would say that it is highly-unlikely that it would not require potentially-extensive changes to i915 to make it work, making the fix almost impossible to land in the stable tree... Do you have a plan to mitigate this problem? Patches like "drm/i915/guc: Disable bonding extension with GuC submission"
[Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/display: Try YCbCr420 color when RGB fails (rev2)
== Series Details == Series: drm/i915/display: Try YCbCr420 color when RGB fails (rev2) URL : https://patchwork.freedesktop.org/series/89842/ State : success == Summary == CI Bug Log - changes from CI_DRM_10063 -> Patchwork_20094 Summary --- **SUCCESS** No regressions found. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_20094/index.html New tests - New tests have been introduced between CI_DRM_10063 and Patchwork_20094: ### New IGT tests (50) ### * igt@kms_flip@basic-flip-vs-dpms@a-dp2: - Statuses : 4 pass(s) - Exec time: [0.58, 1.34] s * igt@kms_flip@basic-flip-vs-dpms@a-dsi1: - Statuses : 3 pass(s) - Exec time: [1.55, 2.00] s * igt@kms_flip@basic-flip-vs-dpms@a-dvi-d1: - Statuses : 1 pass(s) - Exec time: [1.16] s * igt@kms_flip@basic-flip-vs-dpms@a-lvds1: - Statuses : 1 pass(s) - Exec time: [1.69] s * igt@kms_flip@basic-flip-vs-dpms@b-dp2: - Statuses : 4 pass(s) - Exec time: [0.57, 1.37] s * igt@kms_flip@basic-flip-vs-dpms@b-dsi1: - Statuses : 3 pass(s) - Exec time: [1.34, 1.71] s * igt@kms_flip@basic-flip-vs-dpms@b-dvi-d1: - Statuses : 1 pass(s) - Exec time: [1.16] s * igt@kms_flip@basic-flip-vs-dpms@b-lvds1: - Statuses : 1 pass(s) - Exec time: [1.48] s * igt@kms_flip@basic-flip-vs-dpms@c-dp2: - Statuses : 4 pass(s) - Exec time: [0.58, 1.35] s * igt@kms_flip@basic-flip-vs-dpms@c-dsi1: - Statuses : 3 pass(s) - Exec time: [1.34, 1.70] s * igt@kms_flip@basic-flip-vs-dpms@c-vga1: - Statuses : 2 pass(s) - Exec time: [0.84, 0.85] s * igt@kms_flip@basic-flip-vs-dpms@d-dsi1: - Statuses : 1 pass(s) - Exec time: [1.65] s * igt@kms_flip@basic-flip-vs-modeset@a-dp2: - Statuses : 4 pass(s) - Exec time: [0.58, 1.35] s * igt@kms_flip@basic-flip-vs-modeset@a-dsi1: - Statuses : 3 pass(s) - Exec time: [1.55, 1.94] s * igt@kms_flip@basic-flip-vs-modeset@a-dvi-d1: - Statuses : 1 pass(s) - Exec time: [1.02] s * igt@kms_flip@basic-flip-vs-modeset@a-lvds1: - Statuses : 1 pass(s) - Exec time: [1.74] s * igt@kms_flip@basic-flip-vs-modeset@b-dp2: - Statuses : 4 pass(s) - Exec time: [0.58, 1.33] s * igt@kms_flip@basic-flip-vs-modeset@b-dsi1: - Statuses : 3 pass(s) - Exec time: [1.34, 1.51] s * igt@kms_flip@basic-flip-vs-modeset@b-dvi-d1: - Statuses : 1 pass(s) - Exec time: [1.16] s * igt@kms_flip@basic-flip-vs-modeset@b-lvds1: - Statuses : 1 pass(s) - Exec time: [1.41] s * igt@kms_flip@basic-flip-vs-modeset@c-dp2: - Statuses : 4 pass(s) - Exec time: [0.58, 1.35] s * igt@kms_flip@basic-flip-vs-modeset@c-dsi1: - Statuses : 3 pass(s) - Exec time: [1.35, 1.58] s * igt@kms_flip@basic-flip-vs-modeset@c-vga1: - Statuses : 2 pass(s) - Exec time: [0.85, 0.86] s * igt@kms_flip@basic-flip-vs-modeset@d-dsi1: - Statuses : 1 pass(s) - Exec time: [1.56] s * igt@kms_flip@basic-flip-vs-wf_vblank@a-dp2: - Statuses : 4 pass(s) - Exec time: [0.91, 1.27] s * igt@kms_flip@basic-flip-vs-wf_vblank@a-dsi1: - Statuses : 3 pass(s) - Exec time: [1.92, 2.00] s * igt@kms_flip@basic-flip-vs-wf_vblank@a-dvi-d1: - Statuses : 1 pass(s) - Exec time: [1.41] s * igt@kms_flip@basic-flip-vs-wf_vblank@a-lvds1: - Statuses : 1 pass(s) - Exec time: [1.92] s * igt@kms_flip@basic-flip-vs-wf_vblank@b-dp2: - Statuses : 4 pass(s) - Exec time: [0.89, 1.25] s * igt@kms_flip@basic-flip-vs-wf_vblank@b-dsi1: - Statuses : 3 pass(s) - Exec time: [1.81, 1.89] s * igt@kms_flip@basic-flip-vs-wf_vblank@b-dvi-d1: - Statuses : 1 pass(s) - Exec time: [1.39] s * igt@kms_flip@basic-flip-vs-wf_vblank@b-lvds1: - Statuses : 1 pass(s) - Exec time: [1.89] s * igt@kms_flip@basic-flip-vs-wf_vblank@c-dp2: - Statuses : 4 pass(s) - Exec time: [0.97, 1.27] s * igt@kms_flip@basic-flip-vs-wf_vblank@c-dsi1: - Statuses : 3 pass(s) - Exec time: [1.74, 1.89] s * igt@kms_flip@basic-flip-vs-wf_vblank@c-vga1: - Statuses : 2 pass(s) - Exec time: [1.10] s * igt@kms_flip@basic-flip-vs-wf_vblank@d-dsi1: - Statuses : 1 pass(s) - Exec time: [1.75] s * igt@kms_flip@basic-plain-flip@a-dp2: - Statuses : 4 pass(s) - Exec time: [0.60, 0.97] s * igt@kms_flip@basic-plain-flip@a-dsi1: - Statuses : 3 pass(s) - Exec time: [1.50, 1.74] s * igt@kms_flip@basic-plain-flip@a-dvi-d1: - Statuses : 1 pass(s) - Exec time: [0.94] s * igt@kms_flip@basic-plain-flip@a-lvds1: - Statuses : 1 pass(s) - Exec time: [1.43] s * igt@kms_flip@basic-plain-flip@a-vga1: - Statuses : 8 pass(s) - Exec time: [0.70, 1.44] s * igt@kms_flip@basic-plain-flip@b-dp2: - Statuses : 4 pass(s) - Exec time: [0.60, 1.01] s * igt@kms_flip@basic-plain-flip@b-dsi1: - Statuses : 3 pass(s) - Exec time:
[Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915: Only set bind_async_flags when concurrent access wa is not active.
== Series Details == Series: drm/i915: Only set bind_async_flags when concurrent access wa is not active. URL : https://patchwork.freedesktop.org/series/89961/ State : failure == Summary == Applying: drm/i915: Only set bind_async_flags when concurrent access wa is not active. error: sha1 information is lacking or useless (drivers/gpu/drm/i915/gt/gen8_ppgtt.c). error: could not build fake ancestor hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0001 drm/i915: Only set bind_async_flags when concurrent access wa is not active. When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 8/9] drm/gem: Associate GEM objects with drm cgroup
Hi, Mon, 2021-05-10 at 17:36 +0200, Daniel Vetter wrote: > ... > > If DRM allows user-space to exhaust all of system memory, this seems > > to be a gap in enforcement of MEMCG limits for system memory. > > I tried to look into it when this was discussed in the past > > My guess is that shmem_read_mapping_page_gfp() -> > > shmem_getpage_gfp() is not choosing the correct MM to charge against > > in the use case of drivers using shmemfs for backing gem buffers. > > Yeah we know about this one since forever. The bug report is roughly > as old as the gem/ttm memory managers :-/ So another problem might be > that if we now suddenly include gpu memory in the memcg accounting, we > start breaking a bunch of workloads that worked just fine beforehand. It's not the first time tightening security requires adapting settings for running workloads... Workload GPU memory usage needs to be significant portion of application's real memory usage, to cause workload to hit limits that have been set for it earlier. Therefore I think it to definitely be something that user setting such limits actually cares about. => I think the important thing is that reason for the failures is clear from the OOM message. This works much better if GPU related memory usage is specifically stated in that message, once that memory starts to be accounted for system memory. - Eero ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Stop propagating fence errors by default
On Fri, May 07, 2021 at 09:35:21AM +0100, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin
>
> This is an alternative proposed fix for the below references bug report
> where dma fence error propagation is causing undesirable change in
> behaviour post GPU hang/reset.
>
> Approach in this patch is to simply stop propagating all dma fence errors
> by default since that seems to be the upstream ask.
>
> To handle the case where i915 needs error propagation for security, I add
> a new dma fence flag DMA_FENCE_FLAG_PROPAGATE_ERROR and make use of it in
> the command parsing chain only.
>
> It sounds a plausible argument that fence propagation could be useful in
> which case a core flag to enable opt-in should be universally useful.
>
> Signed-off-by: Tvrtko Ursulin
> Reported-by: Marcin Slusarz
> Reported-by: Miroslav Bendik
> References: 9e31c1fe45d5 ("drm/i915: Propagate errors on awaiting already
> signaled fences")
> References: https://gitlab.freedesktop.org/drm/intel/-/issues/3080
> Cc: Jason Ekstrand
> Cc: Daniel Vetter
> ---
> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 ++
> drivers/gpu/drm/i915/i915_sw_fence.c | 8
> drivers/gpu/drm/i915/i915_sw_fence.h | 8
> include/linux/dma-fence.h | 1 +
I still don't like this, least because we still introduce the concept of
error propagation to dma-fence (but hey only in i915 code, which is
exactly the kind of not-really-upstream approach we got a major chiding
for).
The only thing this does is make it explicitly opt-in instead opt-out,
like the first fix. The right approach is imo still to just throw it out,
and instead make the one error propagation we really need very, very
explicit. Instead of hiding it behind lots of magic.
The one error propagation we need is when the cmd parser work fails, it
must cancel it's corresponding request to make sure the batchbuffer
doesn't run. This should require about 2 lines in total:
- one line to store the request so that the cmd parser work can access it.
No refcounting needed, because the the request cannot even start (much
less get freed) before the cmd parser has singalled its fence
- one line to kill the request if the parsing fails. Maybe 2 if you
include the if condition. I have no idea how that's done since I'm
honestly lost how the i915 scheduler decides whether to run a batch or
not. I'm guessing we have a version of this for the ringbuffer and the
execlist backend (if not maybe gen7 cmdparser is broken?)
I don't see any need for magic behind-the-scenes propagation of such a
security critical error. Especially when that error propagation thing
caused security bugs of its own, is an i915-only feature, and not
motivated by any userspace/uapi requirements at all.
Thanks, Daniel
> 4 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> index 297143511f99..6a516d1261d0 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> @@ -2522,6 +2522,8 @@ static int eb_parse_pipeline(struct i915_execbuffer *eb,
> }
>
> dma_fence_work_init(>base, _parse_ops);
> + /* Propagate errors for security. */
> + __set_bit(DMA_FENCE_FLAG_PROPAGATE_ERROR, >base.dma.flags);
>
> pw->engine = eb->engine;
> pw->batch = eb->batch->vma;
> diff --git a/drivers/gpu/drm/i915/i915_sw_fence.c
> b/drivers/gpu/drm/i915/i915_sw_fence.c
> index 2744558f3050..2ee917932ccf 100644
> --- a/drivers/gpu/drm/i915/i915_sw_fence.c
> +++ b/drivers/gpu/drm/i915/i915_sw_fence.c
> @@ -449,7 +449,7 @@ static void dma_i915_sw_fence_wake_timer(struct dma_fence
> *dma,
>
> fence = xchg(>base.fence, NULL);
> if (fence) {
> - i915_sw_fence_set_error_once(fence, dma->error);
> + i915_sw_fence_propagate_dma_fence_error(fence, dma);
> i915_sw_fence_complete(fence);
> }
>
> @@ -480,7 +480,7 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence
> *fence,
> might_sleep_if(gfpflags_allow_blocking(gfp));
>
> if (dma_fence_is_signaled(dma)) {
> - i915_sw_fence_set_error_once(fence, dma->error);
> + i915_sw_fence_propagate_dma_fence_error(fence, dma);
> return 0;
> }
>
> @@ -496,7 +496,7 @@ int i915_sw_fence_await_dma_fence(struct i915_sw_fence
> *fence,
> if (ret)
> return ret;
>
> - i915_sw_fence_set_error_once(fence, dma->error);
> + i915_sw_fence_propagate_dma_fence_error(fence, dma);
> return 0;
> }
>
> @@ -548,7 +548,7 @@ int __i915_sw_fence_await_dma_fence(struct i915_sw_fence
> *fence,
> debug_fence_assert(fence);
>
> if (dma_fence_is_signaled(dma)) {
> - i915_sw_fence_set_error_once(fence, dma->error);
> +
Re: [Intel-gfx] [RFC PATCH 8/9] drm/gem: Associate GEM objects with drm cgroup
The other thread about how to manage gpu compute resources reminded me of this one about gpu memory splitting. On Thu, Mar 18, 2021 at 8:20 PM Brian Welty wrote: > > > On 3/18/2021 3:16 AM, Daniel Vetter wrote: > > On Sat, Mar 6, 2021 at 1:44 AM Brian Welty wrote: > >> > >> > >> On 2/11/2021 7:34 AM, Daniel Vetter wrote: > >>> On Wed, Feb 10, 2021 at 02:00:57PM -0800, Brian Welty wrote: > > On 2/9/2021 2:54 AM, Daniel Vetter wrote: > > On Tue, Jan 26, 2021 at 01:46:25PM -0800, Brian Welty wrote: > >> This patch adds tracking of which cgroup to make charges against for a > >> given GEM object. We associate the current task's cgroup with GEM > >> objects > >> as they are created. First user of this is for charging DRM cgroup for > >> device memory allocations. The intended behavior is for device > >> drivers to > >> make the cgroup charging calls at the time that backing store is > >> allocated > >> or deallocated for the object. > >> > >> Exported functions are provided for charging memory allocations for a > >> GEM object to DRM cgroup. To aid in debugging, we store how many bytes > >> have been charged inside the GEM object. Add helpers for setting and > >> clearing the object's associated cgroup which will check that charges > >> are > >> not being leaked. > >> > >> For shared objects, this may make the charge against a cgroup that is > >> potentially not the same cgroup as the process using the memory. Based > >> on the memory cgroup's discussion of "memory ownership", this seems > >> acceptable [1]. > >> > >> [1] https://www.kernel.org/doc/Documentation/cgroup-v2.txt, "Memory > >> Ownership" > >> > >> Signed-off-by: Brian Welty > > > > Since for now we only have the generic gpu/xpu/bikeshed.memory bucket > > that > > counts everything, why don't we also charge in these gem functions? > > I'm not sure what you mean exactly. You want to merge/move the charging > logic > proposed in patch #5 (drm_cgroup_try_charge in kernel/cgroup/drm.c) into > drm_gem_object_charge_mem() ? > > Or reading below, I think you are okay keeping the logic separated as > is, but > you want much of the code in kernel/cgroup/drm.c moved to > drivers/gpu/cgroup ? > Yes, I see that should allow to reduce number of exported functions. > >>> > >>> Both. I mean we'd need to look at the code again when it's shuffled, but > >>> I'd say: > >>> > >>> - cgroup code and the charging for general gpu memory moves to > >>> drivers/gpu/cgroup, so dma-buf heaps can use it too. > >>> > >>> - the charging for gem buffers moves into core gem code, so it happens for > >>> all gpu drivers and all gem buffer allocations. > >> > >> Daniel, I'm not sure we're in sync on what 'charging for general gpu > >> memory' > >> means. Thus far, I have been proposing to charge/uncharge when backing > >> store is > >> allocated/freed. And thus, this would be done in DRM driver (so then also > >> in > >> the dma-buf exporter). > >> I can't see how we'd hoist this part into drm gem code. > >> The memory limit in this series is for VRAM usage/limit not GEM buffers... > > > > Yes this would be at gem buffer creation time. And just to get cgroups > > for drm up > > Okay, but it's not of the ones in Tejun's list to start with: >https://lists.freedesktop.org/archives/dri-devel/2020-April/262141.html > I hoped we would start by pursuing those (gpu.weight and gpu.memory.high) > as first step. > > Limiting GEM buffers is essentially controlling virtual memory size, which > tend to just always get set to unlimited. > Would be nice to get consensus from maintainers before proceeding to implement > this. Hm I missed this one from Tejun. The problem with just assuming that dma-buf in system memory are included as part of the overall memcg is that when we throw a buffer out of vram, it needs to go somewhere. So from that pov keeping all the dma-buf in their separate hierarchy, outside of what memcg sees, simplifies a lot of things. Unlike e.g. numa migration gpu vram migration is often required for correctness (some buffers _have_ to be in in VRAM on some gpus, or the workload just doesn't work), so if we allow admins to misconfigure stuff then there could be a lot of bad surprises. I'm also expecting more hierarchies like this, e.g. if you have a bunch of GPUs connected with a fast interconnect, then you might want to set an overall limit for VRAM across all gpus, and then maybe additional limits on each gpu node. This is entirely different if we manage the VRAM as ZONE_DEVICE memory, but in that case I expect memcg will be able to take care of all the managing needs. Another case to consider is CMA allocations, where it's a zone in normal memory we need to allocate from, so again strange issues with double counting are to be expected. Or am I worrying
[Intel-gfx] i915 and swiotlb_max_segment
Hi all, swiotlb_max_segment is a rather strange "API" export by swiotlb.c, and i915 is the only (remaining) user. swiotlb_max_segment returns 0 if swiotlb is not in use, 1 if SWIOTLB_FORCE is set or swiotlb-zen is set, and the swiotlb segment size when swiotlb is otherwise enabled. i915 then uses it to: a) decided on the max order in i915_gem_object_get_pages_internal b) decide on a max segment size in i915_sg_segment_size for a) it really seems i915 should switch to dma_alloc_noncoherent or dma_alloc_noncontigous ASAP instead of using alloc_page and streaming DMA mappings. Any chance I could trick one of the i915 maintaines into doing just that given that the callchain is not exactly trivial? For b) I'm not sure swiotlb and i915 really agree on the meaning of the value. swiotlb_set_max_segment basically returns the entire size of the swiotlb buffer, while i915 seems to use it to limit the size each scatterlist entry. It seems like dma_max_mapping_size might be the best value to use here. Once that is fixed I'd like to kill off swiotlb_max_segment as it is a horribly confusing API. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 08/15] swiotlb: Bounce data from/to restricted DMA pool if available
> +static inline bool is_dev_swiotlb_force(struct device *dev)
> +{
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> + if (dev->dma_io_tlb_mem)
> + return true;
> +#endif /* CONFIG_DMA_RESTRICTED_POOL */
> + return false;
> +}
> +
> /* If SWIOTLB is active, use its maximum mapping size */
> if (is_swiotlb_active(dev) &&
> - (dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE))
> + (dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE ||
> + is_dev_swiotlb_force(dev)))
This is a mess. I think the right way is to have an always_bounce flag
in the io_tlb_mem structure instead. Then the global swiotlb_force can
go away and be replace with this and the fact that having no
io_tlb_mem structure at all means forced no buffering (after a little
refactoring).
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 05/15] swiotlb: Add a new get_io_tlb_mem getter
> +static inline struct io_tlb_mem *get_io_tlb_mem(struct device *dev)
> +{
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> + if (dev && dev->dma_io_tlb_mem)
> + return dev->dma_io_tlb_mem;
> +#endif /* CONFIG_DMA_RESTRICTED_POOL */
> +
> + return io_tlb_default_mem;
Given that we're also looking into a not addressing restricted pool
I'd rather always assign the active pool to dev->dma_io_tlb_mem and
do away with this helper.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 04/15] swiotlb: Add restricted DMA pool initialization
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> +#include
> +#include
> +#include
> +#include
> +#include
> +#endif
I don't think any of this belongs into swiotlb.c. Marking
swiotlb_init_io_tlb_mem non-static and having all this code in a separate
file is probably a better idea.
> +#ifdef CONFIG_DMA_RESTRICTED_POOL
> +static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
> + struct device *dev)
> +{
> + struct io_tlb_mem *mem = rmem->priv;
> + unsigned long nslabs = rmem->size >> IO_TLB_SHIFT;
> +
> + if (dev->dma_io_tlb_mem)
> + return 0;
> +
> + /* Since multiple devices can share the same pool, the private data,
> + * io_tlb_mem struct, will be initialized by the first device attached
> + * to it.
> + */
This is not the normal kernel comment style.
> +#ifdef CONFIG_ARM
> + if (!PageHighMem(pfn_to_page(PHYS_PFN(rmem->base {
> + kfree(mem);
> + return -EINVAL;
> + }
> +#endif /* CONFIG_ARM */
And this is weird. Why would ARM have such a restriction? And if we have
such rstrictions it absolutely belongs into an arch helper.
> + swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false);
> +
> + rmem->priv = mem;
> +
> +#ifdef CONFIG_DEBUG_FS
> + if (!debugfs_dir)
> + debugfs_dir = debugfs_create_dir("swiotlb", NULL);
> +
> + swiotlb_create_debugfs(mem, rmem->name, debugfs_dir);
Doesn't the debugfs_create_dir belong into swiotlb_create_debugfs? Also
please use IS_ENABLEd or a stub to avoid ifdefs like this.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 02/15] swiotlb: Refactor swiotlb_create_debugfs
Looks good, Reviewed-by: Christoph Hellwig ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 01/15] swiotlb: Refactor swiotlb init functions
Looks good, Reviewed-by: Christoph Hellwig ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
Em Mon, 10 May 2021 13:19:50 +0200 Mauro Carvalho Chehab escreveu: > Em Mon, 10 May 2021 12:52:44 +0200 > Thorsten Leemhuis escreveu: > > > On 10.05.21 12:26, Mauro Carvalho Chehab wrote: > > > > > > As Linux developers are all around the globe, and not everybody has UTF-8 > > > as their default charset, better to use UTF-8 only on cases where it is > > > really > > > needed. > > > […] > > > The remaining patches on series address such cases on *.rst files and > > > inside the Documentation/ABI, using this perl map table in order to do the > > > charset conversion: > > > > > > my %char_map = ( > > > […] > > > 0x2013 => '-', # EN DASH > > > 0x2014 => '-', # EM DASH > > > > I might be performing bike shedding here, but wouldn't it be better to > > replace those two with "--", as explained in > > https://en.wikipedia.org/wiki/Dash#Approximating_the_em_dash_with_two_or_three_hyphens > > > > For EM DASH there seems to be even "---", but I'd say that is a bit too > > much. > > Yeah, we can do, instead: > > 0x2013 => '--', # EN DASH > 0x2014 => '---',# EM DASH > > I was actually in doubt about those ;-) On a quick test, I changed my script to use "--" and "---" for EN/EM DASH chars. The diff below is against both versions. There are a couple of places where it got mathematically wrong, like this one: -operation over a temperature range of -40°C to +125°C. +operation over a temperature range of --40°C to +125°C. On others, it is just a matter of personal taste. My personal opinion is that, on most cases, a single "-" would be better. Thanks, Mauro diff --git a/Documentation/ABI/testing/sysfs-class-net-cdc_ncm b/Documentation/ABI/testing/sysfs-class-net-cdc_ncm index 41a1eef0d0e7..469325255887 100644 --- a/Documentation/ABI/testing/sysfs-class-net-cdc_ncm +++ b/Documentation/ABI/testing/sysfs-class-net-cdc_ncm @@ -93,7 +93,7 @@ Contact: Bjørn Mork Description: - Bit 0: 16-bit NTB supported (set to 1) - Bit 1: 32-bit NTB supported - - Bits 2 - 15: reserved (reset to zero; must be ignored by host) + - Bits 2 -- 15: reserved (reset to zero; must be ignored by host) What: /sys/class/net//cdc_ncm/dwNtbInMaxSize Date: May 2014 diff --git a/Documentation/PCI/acpi-info.rst b/Documentation/PCI/acpi-info.rst index 9b4b04039982..7a75f1f6e73c 100644 --- a/Documentation/PCI/acpi-info.rst +++ b/Documentation/PCI/acpi-info.rst @@ -140,8 +140,8 @@ address always corresponds to bus 0, even if the bus range below the bridge Extended Address Space Descriptor (.4) General Flags: Bit [0] Consumer/Producer: -* 1 - This device consumes this resource -* 0 - This device produces and consumes this resource +* 1 -- This device consumes this resource +* 0 -- This device produces and consumes this resource [5] ACPI 6.2, sec 19.6.43: ResourceUsage specifies whether the Memory range is consumed by diff --git a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.rst b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.rst index d76c6bfdc659..34a12b12df51 100644 --- a/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.rst +++ b/Documentation/RCU/Design/Memory-Ordering/Tree-RCU-Memory-Ordering.rst @@ -215,7 +215,7 @@ newly arrived RCU callbacks against future grace periods: 43 } But the only part of ``rcu_prepare_for_idle()`` that really matters for -this discussion are lines 37-39. We will therefore abbreviate this +this discussion are lines 37--39. We will therefore abbreviate this function as follows: .. kernel-figure:: rcu_node-lock.svg diff --git a/Documentation/RCU/Design/Requirements/Requirements.rst b/Documentation/RCU/Design/Requirements/Requirements.rst index a3493b34f3dd..a42dc3cf26bd 100644 --- a/Documentation/RCU/Design/Requirements/Requirements.rst +++ b/Documentation/RCU/Design/Requirements/Requirements.rst @@ -2354,8 +2354,8 @@ which in practice also means that RCU must have an aggressive stress-test suite. This stress-test suite is called ``rcutorture``. Although the need for ``rcutorture`` was no surprise, the current -immense popularity of the Linux kernel is posing interesting-and perhaps -unprecedented-validation challenges. To see this, keep in mind that +immense popularity of the Linux kernel is posing interesting---and perhaps +unprecedented---validation challenges. To see this, keep in mind that there are well over one billion instances of the Linux kernel running today, given Android smartphones, Linux-powered televisions, and servers. This number can be expected to increase sharply with the advent diff --git a/Documentation/admin-guide/index.rst b/Documentation/admin-guide/index.rst index b1692643718d..1a6dbda71ad6 100644 --- a/Documentation/admin-guide/index.rst +++ b/Documentation/admin-guide/index.rst @@ -3,7 +3,7
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
Hi David,
Em Mon, 10 May 2021 11:54:02 +0100
David Woodhouse escreveu:
> On Mon, 2021-05-10 at 12:26 +0200, Mauro Carvalho Chehab wrote:
> > There are several UTF-8 characters at the Kernel's documentation.
> >
> > Several of them were due to the process of converting files from
> > DocBook, LaTeX, HTML and Markdown. They were probably introduced
> > by the conversion tools used on that time.
> >
> > Other UTF-8 characters were added along the time, but they're easily
> > replaceable by ASCII chars.
> >
> > As Linux developers are all around the globe, and not everybody has UTF-8
> > as their default charset, better to use UTF-8 only on cases where it is
> > really
> > needed.
>
> No, that is absolutely the wrong approach.
>
> If someone has a local setup which makes bogus assumptions about text
> encodings, that is their own mistake.
>
> We don't do them any favours by trying to *hide* it in the common case
> so that they don't notice it for longer.
>
> There really isn't much excuse for such brokenness, this far into the
> 21st century.
>
> Even *before* UTF-8 came along in the final decade of the last
> millennium, it was important to know which character set a given piece
> of text was encoded in.
>
> In fact it was even *more* important back then, we couldn't just assume
> UTF-8 everywhere like we can in modern times.
>
> Git can already do things like CRLF conversion on checking files out to
> match local conventions; if you want to teach it to do character set
> conversions too then I suppose that might be useful to a few developers
> who've fallen through a time warp and still need it. But nobody's ever
> bothered before because it just isn't necessary these days.
>
> Please *don't* attempt to address this anachronistic and esoteric
> "requirement" by dragging the kernel source back in time by three
> decades.
No. The idea is not to go back three decades ago.
The goal is just to avoid use UTF-8 where it is not needed. See, the vast
majority of UTF-8 chars are kept:
- Non-ASCII Latin and Greek chars;
- Box drawings;
- arrows;
- most symbols.
There, it makes perfect sense to keep using UTF-8.
We should keep using UTF-8 on Kernel. This is something that it shouldn't
be changed.
---
This patch series is doing conversion only when using ASCII makes
more sense than using UTF-8.
See, a number of converted documents ended with weird characters
like ZERO WIDTH NO-BREAK SPACE (U+FEFF) character. This specific
character doesn't do any good.
Others use NO-BREAK SPACE (U+A0) instead of 0x20. Harmless, until
someone tries to use grep[1].
[1] try to run:
$ git grep "CPU 0 has been" Documentation/RCU/
it will return nothing with current upstream.
But it will work fine after the series is applied:
$ git grep "CPU 0 has been" Documentation/RCU/
Documentation/RCU/Design/Data-Structures/Data-Structures.rst:| #. CPU 0
has been in dyntick-idle mode for quite some time. When it |
Documentation/RCU/Design/Data-Structures/Data-Structures.rst:|notices
that CPU 0 has been in dyntick idle mode, which qualifies |
The main point on this series is to replace just the occurrences
where ASCII represents the symbol equally well, e. g. it is limited
for those chars:
- U+2010 ('‐'): HYPHEN
- U+00ad (''): SOFT HYPHEN
- U+2013 ('–'): EN DASH
- U+2014 ('—'): EM DASH
- U+2018 ('‘'): LEFT SINGLE QUOTATION MARK
- U+2019 ('’'): RIGHT SINGLE QUOTATION MARK
- U+00b4 ('´'): ACUTE ACCENT
- U+201c ('“'): LEFT DOUBLE QUOTATION MARK
- U+201d ('”'): RIGHT DOUBLE QUOTATION MARK
- U+00d7 ('×'): MULTIPLICATION SIGN
- U+2212 ('−'): MINUS SIGN
- U+2217 ('∗'): ASTERISK OPERATOR
(this one used as a pointer reference like "*foo" on C code
example inside a document converted from LaTeX)
- U+00bb ('»'): RIGHT-POINTING DOUBLE ANGLE QUOTATION MARK
(this one also used wrongly on an ABI file, meaning '>')
- U+00a0 (' '): NO-BREAK SPACE
- U+feff (''): ZERO WIDTH NO-BREAK SPACE
Using the above symbols will just trick tools like grep for no good
reason.
Thanks,
Mauro
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
Em Mon, 10 May 2021 12:52:44 +0200 Thorsten Leemhuis escreveu: > On 10.05.21 12:26, Mauro Carvalho Chehab wrote: > > > > As Linux developers are all around the globe, and not everybody has UTF-8 > > as their default charset, better to use UTF-8 only on cases where it is > > really > > needed. > > […] > > The remaining patches on series address such cases on *.rst files and > > inside the Documentation/ABI, using this perl map table in order to do the > > charset conversion: > > > > my %char_map = ( > > […] > > 0x2013 => '-', # EN DASH > > 0x2014 => '-', # EM DASH > I might be performing bike shedding here, but wouldn't it be better to > replace those two with "--", as explained in > https://en.wikipedia.org/wiki/Dash#Approximating_the_em_dash_with_two_or_three_hyphens > > For EM DASH there seems to be even "---", but I'd say that is a bit too > much. Yeah, we can do, instead: 0x2013 => '--', # EN DASH 0x2014 => '---',# EM DASH I was actually in doubt about those ;-) Btw, when producing HTML documentation, Sphinx should convert: -- into EN DASH and: --- into EM DASH So, the resulting html will be identical. > Or do you fear the extra work as some lines then might break the > 80-character limit then? No, I suspect that the line size won't be an issue. Some care should taken when EN DASH and EM DASH are used inside tables. Thanks, Mauro ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
On 10/05/2021 14:38, Mauro Carvalho Chehab wrote: > Em Mon, 10 May 2021 14:16:16 +0100 > Edward Cree escreveu: >> But what kinds of things with × or — in are going to be grept for? > > Actually, on almost all places, those aren't used inside math formulae, but > instead, they describe video some resolutions: Ehh, those are also proper uses of ×. It's still a multiplication, after all. > it is a way more likely that, if someone wants to grep, they would be > doing something like this, in order to get video resolutions: Why would someone be grepping for "all video resolutions mentioned in the documentation"? That seems contrived to me. -ed ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
Em Mon, 10 May 2021 14:16:16 +0100
Edward Cree escreveu:
> On 10/05/2021 12:55, Mauro Carvalho Chehab wrote:
> > The main point on this series is to replace just the occurrences
> > where ASCII represents the symbol equally well
>
> > - U+2014 ('—'): EM DASH
> Em dash is not the same thing as hyphen-minus, and the latter does not
> serve 'equally well'. People use em dashes because — even in
> monospace fonts — they make text easier to read and comprehend, when
> used correctly.
True, but if you look at the diff, on several places, IMHO a single
hyphen would make more sensus. Maybe those places came from a converted
doc.
> I accept that some of the other distinctions — like en dashes — are
> needlessly pedantic (though I don't doubt there is someone out there
> who will gladly defend them with the same fervour with which I argue
> for the em dash) and I wouldn't take the trouble to use them myself;
> but I think there is a reasonable assumption that when someone goes
> to the effort of using a Unicode punctuation mark that is semantic
> (rather than merely typographical), they probably had a reason for
> doing so.
>
> > - U+2018 ('‘'): LEFT SINGLE QUOTATION MARK
> > - U+2019 ('’'): RIGHT SINGLE QUOTATION MARK
> > - U+201c ('“'): LEFT DOUBLE QUOTATION MARK
> > - U+201d ('”'): RIGHT DOUBLE QUOTATION MARK
> (These are purely typographic, I have no problem with dumping them.)
>
> > - U+00d7 ('×'): MULTIPLICATION SIGN
> Presumably this is appearing in mathematical formulae, in which case
> changing it to 'x' loses semantic information.
>
> > Using the above symbols will just trick tools like grep for no good
> > reason.
> NBSP, sure. That one's probably an artefact of some document format
> conversion somewhere along the line, anyway.
> But what kinds of things with × or — in are going to be grept for?
Actually, on almost all places, those aren't used inside math formulae, but
instead, they describe video some resolutions:
$ git grep × Documentation/
Documentation/devicetree/bindings/display/panel/asus,z00t-tm5p5-nt35596.yaml:title:
ASUS Z00T TM5P5 NT35596 5.5" 1080×1920 LCD Panel
Documentation/devicetree/bindings/display/panel/panel-simple-dsi.yaml:
# LG ACX467AKM-7 4.95" 1080×1920 LCD Panel
Documentation/devicetree/bindings/sound/tlv320adcx140.yaml: 1 -
Mic bias is set to VREF × 1.096
Documentation/userspace-api/media/v4l/crop.rst:of 16 × 16 pixels. The
source cropping rectangle is set to defaults,
Documentation/userspace-api/media/v4l/crop.rst:which are also the upper
limit in this example, of 640 × 400 pixels at
Documentation/userspace-api/media/v4l/crop.rst:offset 0, 0. An
application requests an image size of 300 × 225 pixels,
Documentation/userspace-api/media/v4l/crop.rst:The driver sets the
image size to the closest possible values 304 × 224,
Documentation/userspace-api/media/v4l/crop.rst:is 608 × 224 (224 × 2:1
would exceed the limit 400). The offset 0, 0 is
Documentation/userspace-api/media/v4l/crop.rst:rectangle of 608 × 456
pixels. The present scaling factors limit
Documentation/userspace-api/media/v4l/crop.rst:cropping to 640 × 384,
so the driver returns the cropping size 608 × 384
Documentation/userspace-api/media/v4l/crop.rst:and adjusts the image
size to closest possible 304 × 192.
Documentation/userspace-api/media/v4l/diff-v4l.rst:size bitmap of 1024
× 625 bits. Struct :c:type:`v4l2_window`
Documentation/userspace-api/media/v4l/vidioc-cropcap.rst:
Assuming pixel aspect 1/1 this could be for example a 640 × 480
Documentation/userspace-api/media/v4l/vidioc-cropcap.rst:
rectangle for NTSC, a 768 × 576 rectangle for PAL and SECAM
it is a way more likely that, if someone wants to grep, they would be
doing something like this, in order to get video resolutions:
$ git grep -E "\b[1-9][0-9]+\s*x\s*[0-9]+\b" Documentation/
Documentation/ABI/obsolete/sysfs-driver-hid-roccat-koneplus:Description:
When read the mouse returns a 30x30 pixel image of the
Documentation/ABI/obsolete/sysfs-driver-hid-roccat-konepure:Description:
When read the mouse returns a 30x30 pixel image of the
Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7:
Provides access to the binary "24x7 catalog" provided by the
Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7:
https://raw.githubusercontent.com/jmesmon/catalog-24x7/master/hv-24x7-
catalog.h
Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7:
Exposes the "version" field of the 24x7 catalog. This is also
Documentation/ABI/testing/sysfs-bus-event_source-devices-hv_24x7:
HCALLs to retrieve hv-24x7 pmu event counter data.
Documentation/ABI/testing/sysfs-bus-vfio-mdev: "2
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
On 10/05/2021 12:55, Mauro Carvalho Chehab wrote:
> The main point on this series is to replace just the occurrences
> where ASCII represents the symbol equally well
> - U+2014 ('—'): EM DASH
Em dash is not the same thing as hyphen-minus, and the latter does not
serve 'equally well'. People use em dashes because — even in
monospace fonts — they make text easier to read and comprehend, when
used correctly.
I accept that some of the other distinctions — like en dashes — are
needlessly pedantic (though I don't doubt there is someone out there
who will gladly defend them with the same fervour with which I argue
for the em dash) and I wouldn't take the trouble to use them myself;
but I think there is a reasonable assumption that when someone goes
to the effort of using a Unicode punctuation mark that is semantic
(rather than merely typographical), they probably had a reason for
doing so.
> - U+2018 ('‘'): LEFT SINGLE QUOTATION MARK
> - U+2019 ('’'): RIGHT SINGLE QUOTATION MARK
> - U+201c ('“'): LEFT DOUBLE QUOTATION MARK
> - U+201d ('”'): RIGHT DOUBLE QUOTATION MARK
(These are purely typographic, I have no problem with dumping them.)
> - U+00d7 ('×'): MULTIPLICATION SIGN
Presumably this is appearing in mathematical formulae, in which case
changing it to 'x' loses semantic information.
> Using the above symbols will just trick tools like grep for no good
> reason.
NBSP, sure. That one's probably an artefact of some document format
conversion somewhere along the line, anyway.
But what kinds of things with × or — in are going to be grept for?
If there are em dashes lying around that semantically _should_ be
hyphen-minus (one of your patches I've seen, for instance, fixes an
*en* dash moonlighting as the option character in an `ethtool`
command line), then sure, convert them.
But any time someone is using a Unicode character to *express
semantics*, even if you happen to think the semantic distinction
involved is a pedantic or unimportant one, I think you need an
explicit grep case to justify ASCIIfying it.
-ed
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
On Mon, May 10, 2021 at 02:16:16PM +0100, Edward Cree wrote:
> On 10/05/2021 12:55, Mauro Carvalho Chehab wrote:
> > The main point on this series is to replace just the occurrences
> > where ASCII represents the symbol equally well
>
> > - U+2014 ('—'): EM DASH
> Em dash is not the same thing as hyphen-minus, and the latter does not
> serve 'equally well'. People use em dashes because — even in
> monospace fonts — they make text easier to read and comprehend, when
> used correctly.
> I accept that some of the other distinctions — like en dashes — are
> needlessly pedantic (though I don't doubt there is someone out there
> who will gladly defend them with the same fervour with which I argue
> for the em dash) and I wouldn't take the trouble to use them myself;
> but I think there is a reasonable assumption that when someone goes
> to the effort of using a Unicode punctuation mark that is semantic
> (rather than merely typographical), they probably had a reason for
> doing so.
I think you're overestimating the amount of care and typographical
knowledge that your average kernel developer has. Most of these
UTF-8 characters come from latex conversions and really aren't
necessary (and are being used incorrectly).
You seem quite knowedgeable about the various differences. Perhaps
you'd be willing to write a document for Documentation/doc-guide/
that provides guidance for when to use which kinds of horizontal
line? https://www.punctuationmatters.com/hyphen-dash-n-dash-and-m-dash/
talks about it in the context of publications, but I think we need
something more suited to our needs for kernel documentation.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH RFC] Revert "drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure"
On 9.5.2021 7.43, Paul Zimmerman wrote: This reverts commit 2bbd6dba84d44219387df051a1c799b7bac46099. Since 5.12-rc2, my Dell XPS-15 laptop has had a blank screen on boot. The system seems to run fine other than having no display, I am able to ssh into the machine. I don't see anything interesting in the dmesg log. I bisected the problem down to this commit, and reverting it fixes the problem. Have you tried with drm-tip? It has acca7762eb71bc0 which hopefully helps here. -- t ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [RFC PATCH 00/97] Basic GuC submission support in the i915
On 10/05/2021 02:11, Jason Ekstrand wrote: On May 9, 2021 12:12:36 Martin Peres wrote: Hi, On 06/05/2021 22:13, Matthew Brost wrote: Basic GuC submission support. This is the first bullet point in the upstreaming plan covered in the following RFC [1]. At a very high level the GuC is a piece of firmware which sits between the i915 and the GPU. It offloads some of the scheduling of contexts from the i915 and programs the GPU to submit contexts. The i915 communicates with the GuC and the GuC communicates with the GPU. May I ask what will GuC command submission do that execlist won't/can't do? And what would be the impact on users? Even forgetting the troubled history of GuC (instability, performance regression, poor level of user support, 6+ years of trying to upstream it...), adding this much code and doubling the amount of validation needed should come with a rationale making it feel worth it... and I am not seeing here. Would you mind providing the rationale behind this work? GuC submission will be disabled by default on all current upstream platforms behind a module parameter - enable_guc. A value of 3 will enable submission and HuC loading via the GuC. GuC submission should work on all gen11+ platforms assuming the GuC firmware is present. What is the plan here when it comes to keeping support for execlist? I am afraid that landing GuC support in Linux is the first step towards killing the execlist, which would force users to use proprietary firmwares that even most Intel engineers have little influence over. Indeed, if "drm/i915/guc: Disable semaphores when using GuC scheduling" which states "Disable semaphores when using GuC scheduling as semaphores are broken in the current GuC firmware." is anything to go by, it means that even Intel developers seem to prefer working around the GuC firmware, rather than fixing it. Yes, landing GuC support may be the first step in removing execlist support. The inevitable reality is that GPU scheduling is coming and likely to be there only path in the not-too-distant future. (See also the ongoing thread with AMD about fences.) I'm not going to pass judgement on whether or not this is a good thing. I'm just reading the winds and, in my view, this is where things are headed for good or ill. In answer to the question above, the answer to "what do we gain from GuC?" may soon be, "you get to use your GPU." We're not there yet and, again, I'm not necessarily advocating for it, but that is likely where things are headed. This will be a sad day, especially since it seems fundamentally opposed with any long-term support, on top of taking away user freedom to fix/tweak their system when Intel won't. A firmware-based submission model isn't a bad design IMO and, aside from the firmware freedom issues, I think there are actual advantages to the model. Immediately, it'll unlock a few features like parallel submission (more on that in a bit) and long-running compute because they're implemented in GuC and the work to implement them properly in the execlist scheduler is highly non-trivial. Longer term, it may (no guarantees) unlock some performance by getting the kernel out of the way. Oh, I definitely agree with firmware-based submission model not being a bad design. I was even cheering for it in 2015. Experience with it made me regret that deeply since :s But with the DRM scheduler being responsible for most things, I fail to see what we could offload in the GuC except context switching (like every other manufacturer). The problem is, the GuC does way more than just switching registers in bulk, and if the number of revisions of the GuC is anything to go by, it is way too complex for me to feel comfortable with it. In the same vein, I have another concern related to the impact of GuC on Linux's stable releases. Let's say that in 3 years, a new application triggers a bug in command submission inside the firmware. Given that the Linux community cannot patch the GuC firmware, how likely is it that Intel would release a new GuC version? That would not be necessarily such a big problem if newer versions of the GuC could easily be backported to this potentially-decade-old Linux version, but given that the GuC seems to have ABI-breaking changes on a monthly cadence (we are at major version 60 *already*? :o), I would say that it is highly-unlikely that it would not require potentially-extensive changes to i915 to make it work, making the fix almost impossible to land in the stable tree... Do you have a plan to mitigate this problem? Patches like "drm/i915/guc: Disable bonding extension with GuC submission" also make me twitch, as this means the two command submission paths will not be functionally equivalent, and enabling GuC could thus introduce a user-visible regression (one app used to work, then stopped working). Could you add in the commit's message a proof that this would not end up being a user regression (in which case, why
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
On Mon, 2021-05-10 at 13:55 +0200, Mauro Carvalho Chehab wrote: > This patch series is doing conversion only when using ASCII makes > more sense than using UTF-8. > > See, a number of converted documents ended with weird characters > like ZERO WIDTH NO-BREAK SPACE (U+FEFF) character. This specific > character doesn't do any good. > > Others use NO-BREAK SPACE (U+A0) instead of 0x20. Harmless, until > someone tries to use grep[1]. Replacing those makes sense. But replacing emdashes — which are a distinct character that has no direct replacement in ASCII and which people do *deliberately* use instead of hyphen-minus — does not. Perhaps stick to those two, and any cases where an emdash or endash has been used where U+002D HYPHEN-MINUS *should* have been used. And please fix your cover letter which made no reference to 'grep', and only presented a completely bogus argument for the change instead. smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: Only set bind_async_flags when concurrent access wa is not active.
Signed-off-by: Maarten Lankhorst --- drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 4 +++- drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c index 1aee5e6b1b23..de3aa79b788e 100644 --- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c @@ -433,7 +433,9 @@ struct i915_ppgtt *gen6_ppgtt_create(struct intel_gt *gt) ppgtt->base.vm.pd_shift = ilog2(SZ_4K * SZ_4K / sizeof(gen6_pte_t)); ppgtt->base.vm.top = 1; - ppgtt->base.vm.bind_async_flags = I915_VMA_LOCAL_BIND; + if (!intel_vm_no_concurrent_access_wa(gt->i915)) + ppgtt->base.vm.bind_async_flags = I915_VMA_LOCAL_BIND; + ppgtt->base.vm.allocate_va_range = gen6_alloc_va_range; ppgtt->base.vm.clear_range = gen6_ppgtt_clear_range; ppgtt->base.vm.insert_entries = gen6_ppgtt_insert_entries; diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index dbb9364f0a65..d157db3c7c23 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -798,7 +798,9 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt) goto err_free_pd; } - ppgtt->vm.bind_async_flags = I915_VMA_LOCAL_BIND; + if (!intel_vm_no_concurrent_access_wa(gt->i915)) + ppgtt->vm.bind_async_flags = I915_VMA_LOCAL_BIND; + ppgtt->vm.insert_entries = gen8_ppgtt_insert; ppgtt->vm.insert_page = gen8_ppgtt_insert_entry; ppgtt->vm.allocate_va_range = gen8_ppgtt_alloc; -- 2.31.0 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 0/3] drm/i915/display: Try YCbCr420 color when RGB fails
When encoder validation of a display mode fails, retry with less bandwidth heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups to support 4k60Hz output, which previously failed silently. AMDGPU had nearly the exact same issue. This problem description is therefore copied from my commit message of the AMDGPU patch. On some setups, while the monitor and the gpu support display modes with pixel clocks of up to 600MHz, the link encoder might not. This prevents YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be possible. However, which color mode is used is decided before the link encoder capabilities are checked. This patch fixes the problem by retrying to find a display mode with YCbCr420 enforced and using it, if it is valid. This patchset is revision 7. Fixed a rebase issue in 1/3 and moved message from error output to debug output in 2/3. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 1/3] drm/i915/display: New function to avoid duplicate code in upcomming commits
Moves some checks that later will be performed 2 times to an own function.
This avoids duplicate code later on.
Signed-off-by: Werner Sembach
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 42 +++
1 file changed, 27 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 46de56af33db..2f1ca91387e4 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1861,6 +1861,32 @@ static int intel_hdmi_port_clock(int clock, int bpc)
return clock * bpc / 8;
}
+static enum drm_mode_status
+intel_hdmi_mode_clock_valid(struct intel_hdmi *hdmi, int clock, bool
has_hdmi_sink)
+{
+ struct drm_device *dev = intel_hdmi_to_dev(hdmi);
+ struct drm_i915_private *dev_priv = to_i915(dev);
+ enum drm_mode_status status;
+
+ /* check if we can do 8bpc */
+ status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
+ true, has_hdmi_sink);
+
+ if (has_hdmi_sink) {
+ /* if we can't do 8bpc we may still be able to do 12bpc */
+ if (status != MODE_OK && !HAS_GMCH(dev_priv))
+ status = hdmi_port_clock_valid(hdmi,
intel_hdmi_port_clock(clock, 12),
+ true, has_hdmi_sink);
+
+ /* if we can't do 8,12bpc we may still be able to do 10bpc */
+ if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
+ status = hdmi_port_clock_valid(hdmi,
intel_hdmi_port_clock(clock, 10),
+ true, has_hdmi_sink);
+ }
+
+ return status;
+}
+
static enum drm_mode_status
intel_hdmi_mode_valid(struct drm_connector *connector,
struct drm_display_mode *mode)
@@ -1891,21 +1917,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
if (drm_mode_is_420_only(>display_info, mode))
clock /= 2;
- /* check if we can do 8bpc */
- status = hdmi_port_clock_valid(hdmi, intel_hdmi_port_clock(clock, 8),
- true, has_hdmi_sink);
-
- if (has_hdmi_sink) {
- /* if we can't do 8bpc we may still be able to do 12bpc */
- if (status != MODE_OK && !HAS_GMCH(dev_priv))
- status = hdmi_port_clock_valid(hdmi,
intel_hdmi_port_clock(clock, 12),
- true, has_hdmi_sink);
-
- /* if we can't do 8,12bpc we may still be able to do 10bpc */
- if (status != MODE_OK && DISPLAY_VER(dev_priv) >= 11)
- status = hdmi_port_clock_valid(hdmi,
intel_hdmi_port_clock(clock, 10),
- true, has_hdmi_sink);
- }
+ status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
if (status != MODE_OK)
return status;
--
2.25.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 3/3] drm/i915/display: Use YCbCr420 as fallback when RGB fails
When encoder validation of a display mode fails, retry with less bandwidth
heavy YCbCr420 color mode, if available. This enables some HDMI 1.4 setups
to support 4k60Hz output, which previously failed silently.
AMDGPU had nearly the exact same issue. This problem description is
therefore copied from my commit message of the AMDGPU patch.
On some setups, while the monitor and the gpu support display modes with
pixel clocks of up to 600MHz, the link encoder might not. This prevents
YCbCr444 and RGB encoding for 4k60Hz, but YCbCr420 encoding might still be
possible. However, which color mode is used is decided before the link
encoder capabilities are checked. This patch fixes the problem by retrying
to find a display mode with YCbCr420 enforced and using it, if it is
valid.
Signed-off-by: Werner Sembach
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 25 ---
1 file changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index c411f1862286..6e135662da3e 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -1898,6 +1898,7 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
int clock = mode->clock;
int max_dotclk = to_i915(connector->dev)->max_dotclk_freq;
bool has_hdmi_sink = intel_has_hdmi_sink(hdmi, connector->state);
+ bool ycbcr_420_only;
if (mode->flags & DRM_MODE_FLAG_DBLSCAN)
return MODE_NO_DBLESCAN;
@@ -1914,12 +1915,22 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
clock *= 2;
}
- if (drm_mode_is_420_only(>display_info, mode))
+ ycbcr_420_only = drm_mode_is_420_only(>display_info, mode);
+ if (ycbcr_420_only)
clock /= 2;
status = intel_hdmi_mode_clock_valid(hdmi, clock, has_hdmi_sink);
- if (status != MODE_OK)
- return status;
+ if (status != MODE_OK) {
+ if (ycbcr_420_only ||
+ !connector->ycbcr_420_allowed ||
+ !drm_mode_is_420_also(>display_info, mode))
+ return status;
+
+ clock /= 2;
+ status = intel_hdmi_mode_clock_valid(hdmi, clock,
has_hdmi_sink);
+ if (status != MODE_OK)
+ return status;
+ }
return intel_mode_valid_max_plane_size(dev_priv, mode, false);
}
@@ -2127,6 +2138,14 @@ static int intel_hdmi_compute_output_format(struct
intel_encoder *encoder,
}
ret = intel_hdmi_compute_clock(encoder, crtc_state);
+ if (ret) {
+ if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_YCBCR420 &&
+ connector->ycbcr_420_allowed &&
+ drm_mode_is_420_also(>display_info,
adjusted_mode)) {
+ crtc_state->output_format =
INTEL_OUTPUT_FORMAT_YCBCR420;
+ ret = intel_hdmi_compute_clock(encoder, crtc_state);
+ }
+ }
return ret;
}
--
2.25.1
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v7 2/3] drm/i915/display: Restructure output format computation for better expandability
Couples the decission between RGB and YCbCr420 mode and the check if the
port clock can archive the required frequency. Other checks and
configuration steps that where previously done in between can also be done
before or after.
This allows for are cleaner implementation of retrying different color
encodings.
A slight change in behaviour occurs with this patch: If YCbCr420 is not
allowed but display is YCbCr420 only it no longer fails, but just prints
an error and tries to fallback on RGB.
Signed-off-by: Werner Sembach
---
drivers/gpu/drm/i915/display/intel_hdmi.c | 66 ---
1 file changed, 35 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c
b/drivers/gpu/drm/i915/display/intel_hdmi.c
index 2f1ca91387e4..c411f1862286 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -2000,29 +2000,6 @@ static bool hdmi_deep_color_possible(const struct
intel_crtc_state *crtc_state,
INTEL_OUTPUT_FORMAT_YCBCR420);
}
-static int
-intel_hdmi_ycbcr420_config(struct intel_crtc_state *crtc_state,
- const struct drm_connector_state *conn_state)
-{
- struct drm_connector *connector = conn_state->connector;
- struct drm_i915_private *i915 = to_i915(connector->dev);
- const struct drm_display_mode *adjusted_mode =
- _state->hw.adjusted_mode;
-
- if (!drm_mode_is_420_only(>display_info, adjusted_mode))
- return 0;
-
- if (!connector->ycbcr_420_allowed) {
- drm_err(>drm,
- "Platform doesn't support YCBCR420 output\n");
- return -EINVAL;
- }
-
- crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
-
- return intel_pch_panel_fitting(crtc_state, conn_state);
-}
-
static int intel_hdmi_compute_bpc(struct intel_encoder *encoder,
struct intel_crtc_state *crtc_state,
int clock)
@@ -2129,6 +2106,31 @@ static bool intel_hdmi_has_audio(struct intel_encoder
*encoder,
return intel_conn_state->force_audio == HDMI_AUDIO_ON;
}
+static int intel_hdmi_compute_output_format(struct intel_encoder *encoder,
+ struct intel_crtc_state *crtc_state,
+ const struct drm_connector_state
*conn_state)
+{
+ struct drm_connector *connector = conn_state->connector;
+ struct drm_i915_private *i915 = to_i915(connector->dev);
+ const struct drm_display_mode *adjusted_mode =
_state->hw.adjusted_mode;
+ int ret;
+ bool ycbcr_420_only;
+
+ ycbcr_420_only = drm_mode_is_420_only(>display_info,
adjusted_mode);
+ if (connector->ycbcr_420_allowed && ycbcr_420_only) {
+ crtc_state->output_format = INTEL_OUTPUT_FORMAT_YCBCR420;
+ } else {
+ if (!connector->ycbcr_420_allowed && ycbcr_420_only)
+ drm_dbg_kms(>drm,
+ "YCbCr 4:2:0 mode but YCbCr 4:2:0 output
not possible. Falling back to RGB.\n");
+ crtc_state->output_format = INTEL_OUTPUT_FORMAT_RGB;
+ }
+
+ ret = intel_hdmi_compute_clock(encoder, crtc_state);
+
+ return ret;
+}
+
int intel_hdmi_compute_config(struct intel_encoder *encoder,
struct intel_crtc_state *pipe_config,
struct drm_connector_state *conn_state)
@@ -2153,23 +2155,25 @@ int intel_hdmi_compute_config(struct intel_encoder
*encoder,
if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
pipe_config->pixel_multiplier = 2;
- ret = intel_hdmi_ycbcr420_config(pipe_config, conn_state);
- if (ret)
- return ret;
-
- pipe_config->limited_color_range =
- intel_hdmi_limited_color_range(pipe_config, conn_state);
-
if (HAS_PCH_SPLIT(dev_priv) && !HAS_DDI(dev_priv))
pipe_config->has_pch_encoder = true;
pipe_config->has_audio =
intel_hdmi_has_audio(encoder, pipe_config, conn_state);
- ret = intel_hdmi_compute_clock(encoder, pipe_config);
+ ret = intel_hdmi_compute_output_format(encoder, pipe_config,
conn_state);
if (ret)
return ret;
+ if (pipe_config->output_format == INTEL_OUTPUT_FORMAT_YCBCR420) {
+ ret = intel_pch_panel_fitting(pipe_config, conn_state);
+ if (ret)
+ return ret;
+ }
+
+ pipe_config->limited_color_range =
+ intel_hdmi_limited_color_range(pipe_config, conn_state);
+
if (conn_state->picture_aspect_ratio)
adjusted_mode->picture_aspect_ratio =
conn_state->picture_aspect_ratio;
--
2.25.1
___
Intel-gfx mailing list
Re: [Intel-gfx] [PATCH 44/53] docs: gpu: avoid using UTF-8 chars
On Mon, May 10, 2021 at 12:26:56PM +0200, Mauro Carvalho Chehab wrote:
> While UTF-8 characters can be used at the Linux documentation,
> the best is to use them only when ASCII doesn't offer a good replacement.
> So, replace the occurences of the following UTF-8 characters:
>
> - U+2019 ('’'): RIGHT SINGLE QUOTATION MARK
>
> Signed-off-by: Mauro Carvalho Chehab
> ---
> Documentation/gpu/i915.rst | 2 +-
> Documentation/gpu/komeda-kms.rst | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> index 486c720f3890..2cbf54460b48 100644
> --- a/Documentation/gpu/i915.rst
> +++ b/Documentation/gpu/i915.rst
> @@ -361,7 +361,7 @@ Locking Guidelines
> real bad.
>
> #. Do not nest different lru/memory manager locks within each other.
> - Take them in turn to update memory allocations, relying on the object’s
> + Take them in turn to update memory allocations, relying on the object's
> dma_resv ww_mutex to serialize against other operations.
>
> #. The suggestion for lru/memory managers locks is that they are small
> diff --git a/Documentation/gpu/komeda-kms.rst
> b/Documentation/gpu/komeda-kms.rst
> index eb693c857e2d..c2067678e92c 100644
> --- a/Documentation/gpu/komeda-kms.rst
> +++ b/Documentation/gpu/komeda-kms.rst
> @@ -324,7 +324,7 @@ the control-abilites of device.
>
> We have _dev, _pipeline, _component. Now fill devices
> with
> pipelines. Since komeda is not for D71 only but also intended for later
> products,
> -of course we’d better share as much as possible between different products.
> To
> +of course we'd better share as much as possible between different products.
> To
> achieve this, split the komeda device into two layers: CORE and CHIP.
>
> - CORE: for common features and capabilities handling.
Acked-by: Liviu Dudau
Best regards,
Liviu
> --
> 2.30.2
>
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
On 10.05.21 12:26, Mauro Carvalho Chehab wrote: > > As Linux developers are all around the globe, and not everybody has UTF-8 > as their default charset, better to use UTF-8 only on cases where it is really > needed. > […] > The remaining patches on series address such cases on *.rst files and > inside the Documentation/ABI, using this perl map table in order to do the > charset conversion: > > my %char_map = ( > […] > 0x2013 => '-', # EN DASH > 0x2014 => '-', # EM DASH I might be performing bike shedding here, but wouldn't it be better to replace those two with "--", as explained in https://en.wikipedia.org/wiki/Dash#Approximating_the_em_dash_with_two_or_three_hyphens For EM DASH there seems to be even "---", but I'd say that is a bit too much. Or do you fear the extra work as some lines then might break the 80-character limit then? Ciao, Thorsten ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 44/53] docs: gpu: avoid using UTF-8 chars
On Mon, 10 May 2021, Mauro Carvalho Chehab wrote:
> While UTF-8 characters can be used at the Linux documentation,
> the best is to use them only when ASCII doesn't offer a good replacement.
> So, replace the occurences of the following UTF-8 characters:
>
> - U+2019 ('’'): RIGHT SINGLE QUOTATION MARK
>
> Signed-off-by: Mauro Carvalho Chehab
Acked-by: Jani Nikula
> ---
> Documentation/gpu/i915.rst | 2 +-
> Documentation/gpu/komeda-kms.rst | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
> index 486c720f3890..2cbf54460b48 100644
> --- a/Documentation/gpu/i915.rst
> +++ b/Documentation/gpu/i915.rst
> @@ -361,7 +361,7 @@ Locking Guidelines
> real bad.
>
> #. Do not nest different lru/memory manager locks within each other.
> - Take them in turn to update memory allocations, relying on the object’s
> + Take them in turn to update memory allocations, relying on the object's
> dma_resv ww_mutex to serialize against other operations.
>
> #. The suggestion for lru/memory managers locks is that they are small
> diff --git a/Documentation/gpu/komeda-kms.rst
> b/Documentation/gpu/komeda-kms.rst
> index eb693c857e2d..c2067678e92c 100644
> --- a/Documentation/gpu/komeda-kms.rst
> +++ b/Documentation/gpu/komeda-kms.rst
> @@ -324,7 +324,7 @@ the control-abilites of device.
>
> We have _dev, _pipeline, _component. Now fill devices
> with
> pipelines. Since komeda is not for D71 only but also intended for later
> products,
> -of course we’d better share as much as possible between different products.
> To
> +of course we'd better share as much as possible between different products.
> To
> achieve this, split the komeda device into two layers: CORE and CHIP.
>
> - CORE: for common features and capabilities handling.
--
Jani Nikula, Intel Open Source Graphics Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
On Mon, 2021-05-10 at 12:26 +0200, Mauro Carvalho Chehab wrote: > There are several UTF-8 characters at the Kernel's documentation. > > Several of them were due to the process of converting files from > DocBook, LaTeX, HTML and Markdown. They were probably introduced > by the conversion tools used on that time. > > Other UTF-8 characters were added along the time, but they're easily > replaceable by ASCII chars. > > As Linux developers are all around the globe, and not everybody has UTF-8 > as their default charset, better to use UTF-8 only on cases where it is really > needed. No, that is absolutely the wrong approach. If someone has a local setup which makes bogus assumptions about text encodings, that is their own mistake. We don't do them any favours by trying to *hide* it in the common case so that they don't notice it for longer. There really isn't much excuse for such brokenness, this far into the 21st century. Even *before* UTF-8 came along in the final decade of the last millennium, it was important to know which character set a given piece of text was encoded in. In fact it was even *more* important back then, we couldn't just assume UTF-8 everywhere like we can in modern times. Git can already do things like CRLF conversion on checking files out to match local conventions; if you want to teach it to do character set conversions too then I suppose that might be useful to a few developers who've fallen through a time warp and still need it. But nobody's ever bothered before because it just isn't necessary these days. Please *don't* attempt to address this anachronistic and esoteric "requirement" by dragging the kernel source back in time by three decades. smime.p7s Description: S/MIME cryptographic signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BUILD: failure for Restricted DMA (rev2)
== Series Details == Series: Restricted DMA (rev2) URL : https://patchwork.freedesktop.org/series/89341/ State : failure == Summary == Applying: swiotlb: Refactor swiotlb init functions error: sha1 information is lacking or useless (kernel/dma/swiotlb.c). error: could not build fake ancestor hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0001 swiotlb: Refactor swiotlb init functions When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 00/53] Get rid of UTF-8 chars that can be mapped as ASCII
There are several UTF-8 characters at the Kernel's documentation.
Several of them were due to the process of converting files from
DocBook, LaTeX, HTML and Markdown. They were probably introduced
by the conversion tools used on that time.
Other UTF-8 characters were added along the time, but they're easily
replaceable by ASCII chars.
As Linux developers are all around the globe, and not everybody has UTF-8
as their default charset, better to use UTF-8 only on cases where it is really
needed.
The first 3 patches on this series were manually written, in order to solve
a few special cases.
The remaining patches on series address such cases on *.rst files and
inside the Documentation/ABI, using this perl map table in order to do the
charset conversion:
my %char_map = (
0x2010 => '-', # HYPHEN
0xad => '-', # SOFT HYPHEN
0x2013 => '-', # EN DASH
0x2014 => '-', # EM DASH
0x2018 => "'", # LEFT SINGLE QUOTATION MARK
0x2019 => "'", # RIGHT SINGLE QUOTATION MARK
0xb4 => "'", # ACUTE ACCENT
0x201c => '"', # LEFT DOUBLE QUOTATION MARK
0x201d => '"', # RIGHT DOUBLE QUOTATION MARK
0x2212 => '-', # MINUS SIGN
0x2217 => '*', # ASTERISK OPERATOR
0xd7 => 'x', # MULTIPLICATION SIGN
0xbb => '>', # RIGHT-POINTING DOUBLE ANGLE QUOTATION MARK
0xa0 => ' ', # NO-BREAK SPACE
0xfeff => '', # ZERO WIDTH NO-BREAK SPACE
);
After the conversion, those UTF-8 chars will be kept:
- U+00a9 ('©'): COPYRIGHT SIGN
- U+00ac ('¬'): NOT SIGN# only at
Documentation/powerpc/transactional_memory.rst
- U+00ae ('®'): REGISTERED SIGN
- U+00b0 ('°'): DEGREE SIGN
- U+00b1 ('±'): PLUS-MINUS SIGN
- U+00b2 ('²'): SUPERSCRIPT TWO
- U+00b5 ('µ'): MICRO SIGN
- U+00b7 ('·'): MIDDLE DOT # See below
- U+00bd ('½'): VULGAR FRACTION ONE HALF
- U+00c7 ('Ç'): LATIN CAPITAL LETTER C WITH CEDILLA
- U+00df ('ß'): LATIN SMALL LETTER SHARP S
- U+00e1 ('á'): LATIN SMALL LETTER A WITH ACUTE
- U+00e4 ('ä'): LATIN SMALL LETTER A WITH DIAERESIS
- U+00e6 ('æ'): LATIN SMALL LETTER AE
- U+00e7 ('ç'): LATIN SMALL LETTER C WITH CEDILLA
- U+00e9 ('é'): LATIN SMALL LETTER E WITH ACUTE
- U+00ea ('ê'): LATIN SMALL LETTER E WITH CIRCUMFLEX
- U+00eb ('ë'): LATIN SMALL LETTER E WITH DIAERESIS
- U+00f3 ('ó'): LATIN SMALL LETTER O WITH ACUTE
- U+00f4 ('ô'): LATIN SMALL LETTER O WITH CIRCUMFLEX
- U+00f6 ('ö'): LATIN SMALL LETTER O WITH DIAERESIS
- U+00f8 ('ø'): LATIN SMALL LETTER O WITH STROKE
- U+00fa ('ú'): LATIN SMALL LETTER U WITH ACUTE
- U+00fc ('ü'): LATIN SMALL LETTER U WITH DIAERESIS
- U+00fd ('ý'): LATIN SMALL LETTER Y WITH ACUTE
- U+011f ('ğ'): LATIN SMALL LETTER G WITH BREVE
- U+0142 ('ł'): LATIN SMALL LETTER L WITH STROKE
- U+03bc ('μ'): GREEK SMALL LETTER MU
- U+2026 ('…'): HORIZONTAL ELLIPSIS
- U+2122 ('™'): TRADE MARK SIGN
- U+2191 ('↑'): UPWARDS ARROW
- U+2192 ('→'): RIGHTWARDS ARROW
- U+2193 ('↓'): DOWNWARDS ARROW
- U+2264 ('≤'): LESS-THAN OR EQUAL TO
- U+2265 ('≥'): GREATER-THAN OR EQUAL TO
- U+2500 ('─'): BOX DRAWINGS LIGHT HORIZONTAL
- U+2502 ('│'): BOX DRAWINGS LIGHT VERTICAL
- U+2514 ('└'): BOX DRAWINGS LIGHT UP AND RIGHT
- U+251c ('├'): BOX DRAWINGS LIGHT VERTICAL AND RIGHT
- U+2b0d ('⬍'): UP DOWN BLACK ARROW
PS.: maintainers were bcc on patch 00/53, in order to reduce the
risk of patch 00 to be rejected by list servers.
-
For U+00b7 ('·'): MIDDLE DOT, I opted to keep it on a few places:
- Documentation/devicetree/bindings/clock/qcom,rpmcc.txt
As this file will be some day converted to yaml, where the
MIDDLE DOT will be removed, I guess it is not worth touching it.
- Documentation/scheduler/sched-deadline.rst
There, it is used on a math expressions. So, better to keep.
- Documentation/devicetree/bindings/media/video-interface-devices.yaml
There, it part of an ASCII artwork.
- translations/zh_CN
I prefer not touching it, as it might have some special meaning in Simplified
Chinese.
Mauro Carvalho Chehab (53):
docs: cdrom-standard.rst: get rid of uneeded UTF-8 chars
docs: ABI: remove a meaningless UTF-8 character
docs: ABI: remove some spurious characters
docs: index.rst: avoid using UTF-8 chars
docs: hwmon: avoid using UTF-8 chars
docs: admin-guide: avoid using UTF-8 chars
docs: admin-guide: media: ipu3.rst: avoid using UTF-8 chars
docs: admin-guide: sysctl: kernel.rst: avoid using UTF-8 chars
docs: admin-guide: perf: imx-ddr.rst: avoid using UTF-8 chars
docs: admin-guide: pm: avoid using UTF-8 chars
docs:
[Intel-gfx] [PATCH 44/53] docs: gpu: avoid using UTF-8 chars
While UTF-8 characters can be used at the Linux documentation,
the best is to use them only when ASCII doesn't offer a good replacement.
So, replace the occurences of the following UTF-8 characters:
- U+2019 ('’'): RIGHT SINGLE QUOTATION MARK
Signed-off-by: Mauro Carvalho Chehab
---
Documentation/gpu/i915.rst | 2 +-
Documentation/gpu/komeda-kms.rst | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index 486c720f3890..2cbf54460b48 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -361,7 +361,7 @@ Locking Guidelines
real bad.
#. Do not nest different lru/memory manager locks within each other.
- Take them in turn to update memory allocations, relying on the object’s
+ Take them in turn to update memory allocations, relying on the object's
dma_resv ww_mutex to serialize against other operations.
#. The suggestion for lru/memory managers locks is that they are small
diff --git a/Documentation/gpu/komeda-kms.rst b/Documentation/gpu/komeda-kms.rst
index eb693c857e2d..c2067678e92c 100644
--- a/Documentation/gpu/komeda-kms.rst
+++ b/Documentation/gpu/komeda-kms.rst
@@ -324,7 +324,7 @@ the control-abilites of device.
We have _dev, _pipeline, _component. Now fill devices with
pipelines. Since komeda is not for D71 only but also intended for later
products,
-of course we’d better share as much as possible between different products. To
+of course we'd better share as much as possible between different products. To
achieve this, split the komeda device into two layers: CORE and CHIP.
- CORE: for common features and capabilities handling.
--
2.30.2
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] ✗ Fi.CI.BUILD: failure for Revert "drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure"
== Series Details == Series: Revert "drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure" URL : https://patchwork.freedesktop.org/series/89941/ State : failure == Summary == Applying: Revert "drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure" Using index info to reconstruct a base tree... M drivers/gpu/drm/i915/display/intel_display_types.h M drivers/gpu/drm/i915/display/intel_dp.c Falling back to patching base and 3-way merge... Auto-merging drivers/gpu/drm/i915/display/intel_dp.c CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_dp.c Auto-merging drivers/gpu/drm/i915/display/intel_display_types.h error: Failed to merge in the changes. hint: Use 'git am --show-current-patch=diff' to see the failed patch Patch failed at 0001 Revert "drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure" When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v5 00/16] Restricted DMA
v6: https://lore.kernel.org/patchwork/cover/1423201/ ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 15/15] of: Add plumbing for restricted DMA pool
If a device is not behind an IOMMU, we look up the device node and set
up the restricted DMA when the restricted-dma-pool is presented.
Signed-off-by: Claire Chang
---
drivers/of/address.c| 25 +
drivers/of/device.c | 3 +++
drivers/of/of_private.h | 5 +
3 files changed, 33 insertions(+)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index aca94c348bd4..c562a9ff5f0b 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1112,6 +1113,30 @@ bool of_dma_is_coherent(struct device_node *np)
}
EXPORT_SYMBOL_GPL(of_dma_is_coherent);
+int of_dma_set_restricted_buffer(struct device *dev)
+{
+ struct device_node *node;
+ int count, i;
+
+ if (!dev->of_node)
+ return 0;
+
+ count = of_property_count_elems_of_size(dev->of_node, "memory-region",
+ sizeof(phandle));
+ for (i = 0; i < count; i++) {
+ node = of_parse_phandle(dev->of_node, "memory-region", i);
+ /* There might be multiple memory regions, but only one
+* restriced-dma-pool region is allowed.
+*/
+ if (of_device_is_compatible(node, "restricted-dma-pool") &&
+ of_device_is_available(node))
+ return of_reserved_mem_device_init_by_idx(
+ dev, dev->of_node, i);
+ }
+
+ return 0;
+}
+
/**
* of_mmio_is_nonposted - Check if device uses non-posted MMIO
* @np:device node
diff --git a/drivers/of/device.c b/drivers/of/device.c
index c5a9473a5fb1..d8d865223e51 100644
--- a/drivers/of/device.c
+++ b/drivers/of/device.c
@@ -165,6 +165,9 @@ int of_dma_configure_id(struct device *dev, struct
device_node *np,
arch_setup_dma_ops(dev, dma_start, size, iommu, coherent);
+ if (!iommu)
+ return of_dma_set_restricted_buffer(dev);
+
return 0;
}
EXPORT_SYMBOL_GPL(of_dma_configure_id);
diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
index d717efbd637d..e9237f5eff48 100644
--- a/drivers/of/of_private.h
+++ b/drivers/of/of_private.h
@@ -163,12 +163,17 @@ struct bus_dma_region;
#if defined(CONFIG_OF_ADDRESS) && defined(CONFIG_HAS_DMA)
int of_dma_get_range(struct device_node *np,
const struct bus_dma_region **map);
+int of_dma_set_restricted_buffer(struct device *dev);
#else
static inline int of_dma_get_range(struct device_node *np,
const struct bus_dma_region **map)
{
return -ENODEV;
}
+static inline int of_dma_get_restricted_buffer(struct device *dev)
+{
+ return -ENODEV;
+}
#endif
#endif /* _LINUX_OF_PRIVATE_H */
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 14/15] dt-bindings: of: Add restricted DMA pool
Introduce the new compatible string, restricted-dma-pool, for restricted
DMA. One can specify the address and length of the restricted DMA memory
region by restricted-dma-pool in the reserved-memory node.
Signed-off-by: Claire Chang
---
.../reserved-memory/reserved-memory.txt | 27 +++
1 file changed, 27 insertions(+)
diff --git
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index e8d3096d922c..284aea659015 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -51,6 +51,23 @@ compatible (optional) - standard definition
used as a shared pool of DMA buffers for a set of devices. It can
be used by an operating system to instantiate the necessary pool
management subsystem if necessary.
+- restricted-dma-pool: This indicates a region of memory meant to be
+ used as a pool of restricted DMA buffers for a set of devices. The
+ memory region would be the only region accessible to those devices.
+ When using this, the no-map and reusable properties must not be set,
+ so the operating system can create a virtual mapping that will be
used
+ for synchronization. The main purpose for restricted DMA is to
+ mitigate the lack of DMA access control on systems without an IOMMU,
+ which could result in the DMA accessing the system memory at
+ unexpected times and/or unexpected addresses, possibly leading to
data
+ leakage or corruption. The feature on its own provides a basic level
+ of protection against the DMA overwriting buffer contents at
+ unexpected times. However, to protect against general data leakage
and
+ system memory corruption, the system needs to provide way to lock
down
+ the memory access, e.g., MPU. Note that since coherent allocation
+ needs remapping, one must set up another device coherent pool by
+ shared-dma-pool and use dma_alloc_from_dev_coherent instead for
atomic
+ coherent allocation.
- vendor specific string in the form ,[-]
no-map (optional) - empty property
- Indicates the operating system must not create a virtual mapping
@@ -120,6 +137,11 @@ one for multimedia processing (named
multimedia-memory@7700, 64MiB).
compatible = "acme,multimedia-memory";
reg = <0x7700 0x400>;
};
+
+ restricted_dma_mem_reserved: restricted_dma_mem_reserved {
+ compatible = "restricted-dma-pool";
+ reg = <0x5000 0x40>;
+ };
};
/* ... */
@@ -138,4 +160,9 @@ one for multimedia processing (named
multimedia-memory@7700, 64MiB).
memory-region = <_reserved>;
/* ... */
};
+
+ pcie_device: pcie_device@0,0 {
+ memory-region = <_dma_mem_reserved>;
+ /* ... */
+ };
};
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 13/15] dma-direct: Allocate memory from restricted DMA pool if available
The restricted DMA pool is preferred if available.
The restricted DMA pools provide a basic level of protection against the
DMA overwriting buffer contents at unexpected times. However, to protect
against general data leakage and system memory corruption, the system
needs to provide a way to lock down the memory access, e.g., MPU.
Note that since coherent allocation needs remapping, one must set up
another device coherent pool by shared-dma-pool and use
dma_alloc_from_dev_coherent instead for atomic coherent allocation.
Signed-off-by: Claire Chang
---
kernel/dma/direct.c | 38 +-
1 file changed, 29 insertions(+), 9 deletions(-)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index eb4098323bbc..0d521f78c7b9 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -78,6 +78,10 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t
phys, size_t size)
static void __dma_direct_free_pages(struct device *dev, struct page *page,
size_t size)
{
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+ if (swiotlb_free(dev, page, size))
+ return;
+#endif
dma_free_contiguous(dev, page, size);
}
@@ -92,7 +96,17 @@ static struct page *__dma_direct_alloc_pages(struct device
*dev, size_t size,
gfp |= dma_direct_optimal_gfp_mask(dev, dev->coherent_dma_mask,
_limit);
- page = dma_alloc_contiguous(dev, size, gfp);
+
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+ page = swiotlb_alloc(dev, size);
+ if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
+ __dma_direct_free_pages(dev, page, size);
+ page = NULL;
+ }
+#endif
+
+ if (!page)
+ page = dma_alloc_contiguous(dev, size, gfp);
if (page && !dma_coherent_ok(dev, page_to_phys(page), size)) {
dma_free_contiguous(dev, page, size);
page = NULL;
@@ -148,7 +162,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
gfp |= __GFP_NOWARN;
if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
- !force_dma_unencrypted(dev)) {
+ !force_dma_unencrypted(dev) && !is_dev_swiotlb_force(dev)) {
page = __dma_direct_alloc_pages(dev, size, gfp & ~__GFP_ZERO);
if (!page)
return NULL;
@@ -161,18 +175,23 @@ void *dma_direct_alloc(struct device *dev, size_t size,
}
if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
- !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
- !dev_is_dma_coherent(dev))
+ !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && !dev_is_dma_coherent(dev) &&
+ !is_dev_swiotlb_force(dev))
return arch_dma_alloc(dev, size, dma_handle, gfp, attrs);
/*
* Remapping or decrypting memory may block. If either is required and
* we can't block, allocate the memory from the atomic pools.
+* If restricted DMA (i.e., is_dev_swiotlb_force) is required, one must
+* set up another device coherent pool by shared-dma-pool and use
+* dma_alloc_from_dev_coherent instead.
*/
if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
!gfpflags_allow_blocking(gfp) &&
(force_dma_unencrypted(dev) ||
-(IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
!dev_is_dma_coherent(dev
+(IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
+ !dev_is_dma_coherent(dev))) &&
+ !is_dev_swiotlb_force(dev))
return dma_direct_alloc_from_pool(dev, size, dma_handle, gfp);
/* we always manually zero the memory once we are done */
@@ -253,15 +272,15 @@ void dma_direct_free(struct device *dev, size_t size,
unsigned int page_order = get_order(size);
if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) &&
- !force_dma_unencrypted(dev)) {
+ !force_dma_unencrypted(dev) && !is_dev_swiotlb_force(dev)) {
/* cpu_addr is a struct page cookie, not a kernel address */
dma_free_contiguous(dev, cpu_addr, size);
return;
}
if (!IS_ENABLED(CONFIG_ARCH_HAS_DMA_SET_UNCACHED) &&
- !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) &&
- !dev_is_dma_coherent(dev)) {
+ !IS_ENABLED(CONFIG_DMA_DIRECT_REMAP) && !dev_is_dma_coherent(dev) &&
+ !is_dev_swiotlb_force(dev)) {
arch_dma_free(dev, size, cpu_addr, dma_addr, attrs);
return;
}
@@ -289,7 +308,8 @@ struct page *dma_direct_alloc_pages(struct device *dev,
size_t size,
void *ret;
if (IS_ENABLED(CONFIG_DMA_COHERENT_POOL) &&
- force_dma_unencrypted(dev) && !gfpflags_allow_blocking(gfp))
+ force_dma_unencrypted(dev) && !gfpflags_allow_blocking(gfp) &&
+ !is_dev_swiotlb_force(dev))
return dma_direct_alloc_from_pool(dev, size, dma_handle,
[Intel-gfx] [PATCH v6 12/15] swiotlb: Add restricted DMA alloc/free support.
Add the functions, swiotlb_{alloc,free} to support the memory allocation
from restricted DMA pool.
Signed-off-by: Claire Chang
---
include/linux/swiotlb.h | 4
kernel/dma/swiotlb.c| 35 +--
2 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 0c5a18d9cf89..e8cf49bd90c5 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -134,6 +134,10 @@ unsigned int swiotlb_max_segment(void);
size_t swiotlb_max_mapping_size(struct device *dev);
bool is_swiotlb_active(struct device *dev);
void __init swiotlb_adjust_size(unsigned long size);
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+struct page *swiotlb_alloc(struct device *dev, size_t size);
+bool swiotlb_free(struct device *dev, struct page *page, size_t size);
+#endif /* CONFIG_DMA_RESTRICTED_POOL */
#else
#define swiotlb_force SWIOTLB_NO_FORCE
static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index fa11787e4b95..d99d5f902259 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -457,8 +457,9 @@ static int find_slots(struct device *dev, phys_addr_t
orig_addr,
index = wrap = wrap_index(mem, ALIGN(mem->index, stride));
do {
- if ((slot_addr(tbl_dma_addr, index) & iotlb_align_mask) !=
- (orig_addr & iotlb_align_mask)) {
+ if (orig_addr &&
+ (slot_addr(tbl_dma_addr, index) & iotlb_align_mask) !=
+ (orig_addr & iotlb_align_mask)) {
index = wrap_index(mem, index + 1);
continue;
}
@@ -701,6 +702,36 @@ late_initcall(swiotlb_create_default_debugfs);
#endif
#ifdef CONFIG_DMA_RESTRICTED_POOL
+struct page *swiotlb_alloc(struct device *dev, size_t size)
+{
+ struct io_tlb_mem *mem = dev->dma_io_tlb_mem;
+ phys_addr_t tlb_addr;
+ int index;
+
+ if (!mem)
+ return NULL;
+
+ index = find_slots(dev, 0, size);
+ if (index == -1)
+ return NULL;
+
+ tlb_addr = slot_addr(mem->start, index);
+
+ return pfn_to_page(PFN_DOWN(tlb_addr));
+}
+
+bool swiotlb_free(struct device *dev, struct page *page, size_t size)
+{
+ phys_addr_t tlb_addr = page_to_phys(page);
+
+ if (!is_swiotlb_buffer(dev, tlb_addr))
+ return false;
+
+ release_slots(dev, tlb_addr);
+
+ return true;
+}
+
static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
struct device *dev)
{
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 10/15] swiotlb: Refactor swiotlb_tbl_unmap_single
Add a new function, release_slots, to make the code reusable for supporting
different bounce buffer pools, e.g. restricted DMA pool.
Signed-off-by: Claire Chang
---
kernel/dma/swiotlb.c | 35 ---
1 file changed, 20 insertions(+), 15 deletions(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index ef04d8f7708f..fa11787e4b95 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -550,27 +550,15 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev,
phys_addr_t orig_addr,
return tlb_addr;
}
-/*
- * tlb_addr is the physical address of the bounce buffer to unmap.
- */
-void swiotlb_tbl_unmap_single(struct device *hwdev, phys_addr_t tlb_addr,
- size_t mapping_size, enum dma_data_direction dir,
- unsigned long attrs)
+static void release_slots(struct device *dev, phys_addr_t tlb_addr)
{
- struct io_tlb_mem *mem = get_io_tlb_mem(hwdev);
+ struct io_tlb_mem *mem = get_io_tlb_mem(dev);
unsigned long flags;
- unsigned int offset = swiotlb_align_offset(hwdev, tlb_addr);
+ unsigned int offset = swiotlb_align_offset(dev, tlb_addr);
int index = (tlb_addr - offset - mem->start) >> IO_TLB_SHIFT;
int nslots = nr_slots(mem->slots[index].alloc_size + offset);
int count, i;
- /*
-* First, sync the memory before unmapping the entry
-*/
- if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
- (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL))
- swiotlb_bounce(hwdev, tlb_addr, mapping_size, DMA_FROM_DEVICE);
-
/*
* Return the buffer to the free list by setting the corresponding
* entries to indicate the number of contiguous entries available.
@@ -605,6 +593,23 @@ void swiotlb_tbl_unmap_single(struct device *hwdev,
phys_addr_t tlb_addr,
spin_unlock_irqrestore(>lock, flags);
}
+/*
+ * tlb_addr is the physical address of the bounce buffer to unmap.
+ */
+void swiotlb_tbl_unmap_single(struct device *dev, phys_addr_t tlb_addr,
+ size_t mapping_size, enum dma_data_direction dir,
+ unsigned long attrs)
+{
+ /*
+* First, sync the memory before unmapping the entry
+*/
+ if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
+ (dir == DMA_FROM_DEVICE || dir == DMA_BIDIRECTIONAL))
+ swiotlb_bounce(dev, tlb_addr, mapping_size, DMA_FROM_DEVICE);
+
+ release_slots(dev, tlb_addr);
+}
+
void swiotlb_sync_single_for_device(struct device *dev, phys_addr_t tlb_addr,
size_t size, enum dma_data_direction dir)
{
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 11/15] dma-direct: Add a new wrapper __dma_direct_free_pages()
Add a new wrapper __dma_direct_free_pages() that will be useful later
for swiotlb_free().
Signed-off-by: Claire Chang
---
kernel/dma/direct.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 078f7087e466..eb4098323bbc 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -75,6 +75,12 @@ static bool dma_coherent_ok(struct device *dev, phys_addr_t
phys, size_t size)
min_not_zero(dev->coherent_dma_mask, dev->bus_dma_limit);
}
+static void __dma_direct_free_pages(struct device *dev, struct page *page,
+ size_t size)
+{
+ dma_free_contiguous(dev, page, size);
+}
+
static struct page *__dma_direct_alloc_pages(struct device *dev, size_t size,
gfp_t gfp)
{
@@ -237,7 +243,7 @@ void *dma_direct_alloc(struct device *dev, size_t size,
return NULL;
}
out_free_pages:
- dma_free_contiguous(dev, page, size);
+ __dma_direct_free_pages(dev, page, size);
return NULL;
}
@@ -273,7 +279,7 @@ void dma_direct_free(struct device *dev, size_t size,
else if (IS_ENABLED(CONFIG_ARCH_HAS_DMA_CLEAR_UNCACHED))
arch_dma_clear_uncached(cpu_addr, size);
- dma_free_contiguous(dev, dma_direct_to_page(dev, dma_addr), size);
+ __dma_direct_free_pages(dev, dma_direct_to_page(dev, dma_addr), size);
}
struct page *dma_direct_alloc_pages(struct device *dev, size_t size,
@@ -310,7 +316,7 @@ struct page *dma_direct_alloc_pages(struct device *dev,
size_t size,
*dma_handle = phys_to_dma_direct(dev, page_to_phys(page));
return page;
out_free_pages:
- dma_free_contiguous(dev, page, size);
+ __dma_direct_free_pages(dev, page, size);
return NULL;
}
@@ -329,7 +335,7 @@ void dma_direct_free_pages(struct device *dev, size_t size,
if (force_dma_unencrypted(dev))
set_memory_encrypted((unsigned long)vaddr, 1 << page_order);
- dma_free_contiguous(dev, page, size);
+ __dma_direct_free_pages(dev, page, size);
}
#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 09/15] swiotlb: Move alloc_size to find_slots
Move the maintenance of alloc_size to find_slots for better code
reusability later.
Signed-off-by: Claire Chang
---
kernel/dma/swiotlb.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 3f1ad080a4bc..ef04d8f7708f 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -482,8 +482,11 @@ static int find_slots(struct device *dev, phys_addr_t
orig_addr,
return -1;
found:
- for (i = index; i < index + nslots; i++)
+ for (i = index; i < index + nslots; i++) {
mem->slots[i].list = 0;
+ mem->slots[i].alloc_size =
+ alloc_size - ((i - index) << IO_TLB_SHIFT);
+ }
for (i = index - 1;
io_tlb_offset(i) != IO_TLB_SEGSIZE - 1 &&
mem->slots[i].list; i--)
@@ -538,11 +541,8 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev,
phys_addr_t orig_addr,
* This is needed when we sync the memory. Then we sync the buffer if
* needed.
*/
- for (i = 0; i < nr_slots(alloc_size + offset); i++) {
+ for (i = 0; i < nr_slots(alloc_size + offset); i++)
mem->slots[index + i].orig_addr = slot_addr(orig_addr, i);
- mem->slots[index + i].alloc_size =
- alloc_size - (i << IO_TLB_SHIFT);
- }
tlb_addr = slot_addr(mem->start, index) + offset;
if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
(dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL))
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 08/15] swiotlb: Bounce data from/to restricted DMA pool if available
Regardless of swiotlb setting, the restricted DMA pool is preferred if
available.
The restricted DMA pools provide a basic level of protection against the
DMA overwriting buffer contents at unexpected times. However, to protect
against general data leakage and system memory corruption, the system
needs to provide a way to lock down the memory access, e.g., MPU.
Note that is_dev_swiotlb_force doesn't check if
swiotlb_force == SWIOTLB_FORCE. Otherwise the memory allocation behavior
with default swiotlb will be changed by the following patche
("dma-direct: Allocate memory from restricted DMA pool if available").
Signed-off-by: Claire Chang
---
include/linux/swiotlb.h | 13 +
kernel/dma/direct.c | 3 ++-
kernel/dma/direct.h | 3 ++-
kernel/dma/swiotlb.c| 8
4 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index c530c976d18b..0c5a18d9cf89 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -120,6 +120,15 @@ static inline bool is_swiotlb_buffer(struct device *dev,
phys_addr_t paddr)
return mem && paddr >= mem->start && paddr < mem->end;
}
+static inline bool is_dev_swiotlb_force(struct device *dev)
+{
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+ if (dev->dma_io_tlb_mem)
+ return true;
+#endif /* CONFIG_DMA_RESTRICTED_POOL */
+ return false;
+}
+
void __init swiotlb_exit(void);
unsigned int swiotlb_max_segment(void);
size_t swiotlb_max_mapping_size(struct device *dev);
@@ -131,6 +140,10 @@ static inline bool is_swiotlb_buffer(struct device *dev,
phys_addr_t paddr)
{
return false;
}
+static inline bool is_dev_swiotlb_force(struct device *dev)
+{
+ return false;
+}
static inline void swiotlb_exit(void)
{
}
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 7a88c34d0867..078f7087e466 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -496,7 +496,8 @@ size_t dma_direct_max_mapping_size(struct device *dev)
{
/* If SWIOTLB is active, use its maximum mapping size */
if (is_swiotlb_active(dev) &&
- (dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE))
+ (dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE ||
+is_dev_swiotlb_force(dev)))
return swiotlb_max_mapping_size(dev);
return SIZE_MAX;
}
diff --git a/kernel/dma/direct.h b/kernel/dma/direct.h
index 13e9e7158d94..f94813674e23 100644
--- a/kernel/dma/direct.h
+++ b/kernel/dma/direct.h
@@ -87,7 +87,8 @@ static inline dma_addr_t dma_direct_map_page(struct device
*dev,
phys_addr_t phys = page_to_phys(page) + offset;
dma_addr_t dma_addr = phys_to_dma(dev, phys);
- if (unlikely(swiotlb_force == SWIOTLB_FORCE))
+ if (unlikely(swiotlb_force == SWIOTLB_FORCE) ||
+ is_dev_swiotlb_force(dev))
return swiotlb_map(dev, phys, size, dir, attrs);
if (unlikely(!dma_capable(dev, dma_addr, size, true))) {
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 81bed3d2c771..3f1ad080a4bc 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -347,7 +347,7 @@ void __init swiotlb_exit(void)
static void swiotlb_bounce(struct device *dev, phys_addr_t tlb_addr, size_t
size,
enum dma_data_direction dir)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
+ struct io_tlb_mem *mem = get_io_tlb_mem(dev);
int index = (tlb_addr - mem->start) >> IO_TLB_SHIFT;
phys_addr_t orig_addr = mem->slots[index].orig_addr;
size_t alloc_size = mem->slots[index].alloc_size;
@@ -429,7 +429,7 @@ static unsigned int wrap_index(struct io_tlb_mem *mem,
unsigned int index)
static int find_slots(struct device *dev, phys_addr_t orig_addr,
size_t alloc_size)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
+ struct io_tlb_mem *mem = get_io_tlb_mem(dev);
unsigned long boundary_mask = dma_get_seg_boundary(dev);
dma_addr_t tbl_dma_addr =
phys_to_dma_unencrypted(dev, mem->start) & boundary_mask;
@@ -506,7 +506,7 @@ phys_addr_t swiotlb_tbl_map_single(struct device *dev,
phys_addr_t orig_addr,
size_t mapping_size, size_t alloc_size,
enum dma_data_direction dir, unsigned long attrs)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
+ struct io_tlb_mem *mem = get_io_tlb_mem(dev);
unsigned int offset = swiotlb_align_offset(dev, orig_addr);
unsigned int i;
int index;
@@ -557,7 +557,7 @@ void swiotlb_tbl_unmap_single(struct device *hwdev,
phys_addr_t tlb_addr,
size_t mapping_size, enum dma_data_direction dir,
unsigned long attrs)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
+ struct io_tlb_mem *mem = get_io_tlb_mem(hwdev);
unsigned long flags;
unsigned int offset =
[Intel-gfx] [PATCH v6 07/15] swiotlb: Update is_swiotlb_active to add a struct device argument
Update is_swiotlb_active to add a struct device argument. This will be
useful later to allow for restricted DMA pool.
Signed-off-by: Claire Chang
---
drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_ttm.c| 2 +-
drivers/pci/xen-pcifront.c | 2 +-
include/linux/swiotlb.h | 4 ++--
kernel/dma/direct.c | 2 +-
kernel/dma/swiotlb.c | 4 ++--
6 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
index ce6b664b10aa..7d48c433446b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_internal.c
@@ -42,7 +42,7 @@ static int i915_gem_object_get_pages_internal(struct
drm_i915_gem_object *obj)
max_order = MAX_ORDER;
#ifdef CONFIG_SWIOTLB
- if (is_swiotlb_active()) {
+ if (is_swiotlb_active(NULL)) {
unsigned int max_segment;
max_segment = swiotlb_max_segment();
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index e8b506a6685b..2a2ae6d6cf6d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -321,7 +321,7 @@ nouveau_ttm_init(struct nouveau_drm *drm)
}
#if IS_ENABLED(CONFIG_SWIOTLB) && IS_ENABLED(CONFIG_X86)
- need_swiotlb = is_swiotlb_active();
+ need_swiotlb = is_swiotlb_active(NULL);
#endif
ret = ttm_device_init(>ttm.bdev, _bo_driver, drm->dev->dev,
diff --git a/drivers/pci/xen-pcifront.c b/drivers/pci/xen-pcifront.c
index b7a8f3a1921f..6d548ce53ce7 100644
--- a/drivers/pci/xen-pcifront.c
+++ b/drivers/pci/xen-pcifront.c
@@ -693,7 +693,7 @@ static int pcifront_connect_and_init_dma(struct
pcifront_device *pdev)
spin_unlock(_dev_lock);
- if (!err && !is_swiotlb_active()) {
+ if (!err && !is_swiotlb_active(NULL)) {
err = pci_xen_swiotlb_init_late();
if (err)
dev_err(>xdev->dev, "Could not setup SWIOTLB!\n");
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 2a6cca07540b..c530c976d18b 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -123,7 +123,7 @@ static inline bool is_swiotlb_buffer(struct device *dev,
phys_addr_t paddr)
void __init swiotlb_exit(void);
unsigned int swiotlb_max_segment(void);
size_t swiotlb_max_mapping_size(struct device *dev);
-bool is_swiotlb_active(void);
+bool is_swiotlb_active(struct device *dev);
void __init swiotlb_adjust_size(unsigned long size);
#else
#define swiotlb_force SWIOTLB_NO_FORCE
@@ -143,7 +143,7 @@ static inline size_t swiotlb_max_mapping_size(struct device
*dev)
return SIZE_MAX;
}
-static inline bool is_swiotlb_active(void)
+static inline bool is_swiotlb_active(struct device *dev)
{
return false;
}
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index 84c9feb5474a..7a88c34d0867 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -495,7 +495,7 @@ int dma_direct_supported(struct device *dev, u64 mask)
size_t dma_direct_max_mapping_size(struct device *dev)
{
/* If SWIOTLB is active, use its maximum mapping size */
- if (is_swiotlb_active() &&
+ if (is_swiotlb_active(dev) &&
(dma_addressing_limited(dev) || swiotlb_force == SWIOTLB_FORCE))
return swiotlb_max_mapping_size(dev);
return SIZE_MAX;
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 4ea027b75013..81bed3d2c771 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -662,9 +662,9 @@ size_t swiotlb_max_mapping_size(struct device *dev)
return ((size_t)IO_TLB_SIZE) * IO_TLB_SEGSIZE;
}
-bool is_swiotlb_active(void)
+bool is_swiotlb_active(struct device *dev)
{
- return io_tlb_default_mem != NULL;
+ return get_io_tlb_mem(dev) != NULL;
}
EXPORT_SYMBOL_GPL(is_swiotlb_active);
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 06/15] swiotlb: Update is_swiotlb_buffer to add a struct device argument
Update is_swiotlb_buffer to add a struct device argument. This will be
useful later to allow for restricted DMA pool.
Signed-off-by: Claire Chang
---
drivers/iommu/dma-iommu.c | 12 ++--
drivers/xen/swiotlb-xen.c | 2 +-
include/linux/swiotlb.h | 6 +++---
kernel/dma/direct.c | 6 +++---
kernel/dma/direct.h | 6 +++---
5 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index 7bcdd1205535..a5df35bfd150 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -504,7 +504,7 @@ static void __iommu_dma_unmap_swiotlb(struct device *dev,
dma_addr_t dma_addr,
__iommu_dma_unmap(dev, dma_addr, size);
- if (unlikely(is_swiotlb_buffer(phys)))
+ if (unlikely(is_swiotlb_buffer(dev, phys)))
swiotlb_tbl_unmap_single(dev, phys, size, dir, attrs);
}
@@ -575,7 +575,7 @@ static dma_addr_t __iommu_dma_map_swiotlb(struct device
*dev, phys_addr_t phys,
}
iova = __iommu_dma_map(dev, phys, aligned_size, prot, dma_mask);
- if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(phys))
+ if (iova == DMA_MAPPING_ERROR && is_swiotlb_buffer(dev, phys))
swiotlb_tbl_unmap_single(dev, phys, org_size, dir, attrs);
return iova;
}
@@ -781,7 +781,7 @@ static void iommu_dma_sync_single_for_cpu(struct device
*dev,
if (!dev_is_dma_coherent(dev))
arch_sync_dma_for_cpu(phys, size, dir);
- if (is_swiotlb_buffer(phys))
+ if (is_swiotlb_buffer(dev, phys))
swiotlb_sync_single_for_cpu(dev, phys, size, dir);
}
@@ -794,7 +794,7 @@ static void iommu_dma_sync_single_for_device(struct device
*dev,
return;
phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
- if (is_swiotlb_buffer(phys))
+ if (is_swiotlb_buffer(dev, phys))
swiotlb_sync_single_for_device(dev, phys, size, dir);
if (!dev_is_dma_coherent(dev))
@@ -815,7 +815,7 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
if (!dev_is_dma_coherent(dev))
arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
- if (is_swiotlb_buffer(sg_phys(sg)))
+ if (is_swiotlb_buffer(dev, sg_phys(sg)))
swiotlb_sync_single_for_cpu(dev, sg_phys(sg),
sg->length, dir);
}
@@ -832,7 +832,7 @@ static void iommu_dma_sync_sg_for_device(struct device *dev,
return;
for_each_sg(sgl, sg, nelems, i) {
- if (is_swiotlb_buffer(sg_phys(sg)))
+ if (is_swiotlb_buffer(dev, sg_phys(sg)))
swiotlb_sync_single_for_device(dev, sg_phys(sg),
sg->length, dir);
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index 4c89afc0df62..0c6ed09f8513 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -100,7 +100,7 @@ static int is_xen_swiotlb_buffer(struct device *dev,
dma_addr_t dma_addr)
* in our domain. Therefore _only_ check address within our domain.
*/
if (pfn_valid(PFN_DOWN(paddr)))
- return is_swiotlb_buffer(paddr);
+ return is_swiotlb_buffer(dev, paddr);
return 0;
}
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index b469f04cca26..2a6cca07540b 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -113,9 +113,9 @@ static inline struct io_tlb_mem *get_io_tlb_mem(struct
device *dev)
return io_tlb_default_mem;
}
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
+static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
+ struct io_tlb_mem *mem = get_io_tlb_mem(dev);
return mem && paddr >= mem->start && paddr < mem->end;
}
@@ -127,7 +127,7 @@ bool is_swiotlb_active(void);
void __init swiotlb_adjust_size(unsigned long size);
#else
#define swiotlb_force SWIOTLB_NO_FORCE
-static inline bool is_swiotlb_buffer(phys_addr_t paddr)
+static inline bool is_swiotlb_buffer(struct device *dev, phys_addr_t paddr)
{
return false;
}
diff --git a/kernel/dma/direct.c b/kernel/dma/direct.c
index f737e3347059..84c9feb5474a 100644
--- a/kernel/dma/direct.c
+++ b/kernel/dma/direct.c
@@ -343,7 +343,7 @@ void dma_direct_sync_sg_for_device(struct device *dev,
for_each_sg(sgl, sg, nents, i) {
phys_addr_t paddr = dma_to_phys(dev, sg_dma_address(sg));
- if (unlikely(is_swiotlb_buffer(paddr)))
+ if (unlikely(is_swiotlb_buffer(dev, paddr)))
swiotlb_sync_single_for_device(dev, paddr, sg->length,
dir);
@@ -369,7 +369,7 @@ void dma_direct_sync_sg_for_cpu(struct device
[Intel-gfx] [PATCH v6 05/15] swiotlb: Add a new get_io_tlb_mem getter
Add a new getter, get_io_tlb_mem, to help select the io_tlb_mem struct.
The restricted DMA pool is preferred if available.
Signed-off-by: Claire Chang
---
include/linux/swiotlb.h | 11 +++
1 file changed, 11 insertions(+)
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 03ad6e3b4056..b469f04cca26 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -2,6 +2,7 @@
#ifndef __LINUX_SWIOTLB_H
#define __LINUX_SWIOTLB_H
+#include
#include
#include
#include
@@ -102,6 +103,16 @@ struct io_tlb_mem {
};
extern struct io_tlb_mem *io_tlb_default_mem;
+static inline struct io_tlb_mem *get_io_tlb_mem(struct device *dev)
+{
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+ if (dev && dev->dma_io_tlb_mem)
+ return dev->dma_io_tlb_mem;
+#endif /* CONFIG_DMA_RESTRICTED_POOL */
+
+ return io_tlb_default_mem;
+}
+
static inline bool is_swiotlb_buffer(phys_addr_t paddr)
{
struct io_tlb_mem *mem = io_tlb_default_mem;
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 04/15] swiotlb: Add restricted DMA pool initialization
Add the initialization function to create restricted DMA pools from
matching reserved-memory nodes.
Signed-off-by: Claire Chang
---
include/linux/device.h | 4 +++
include/linux/swiotlb.h | 3 +-
kernel/dma/swiotlb.c| 79 +
3 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/include/linux/device.h b/include/linux/device.h
index 38a2071cf776..4987608ea4ff 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -416,6 +416,7 @@ struct dev_links_info {
* @dma_pools: Dma pools (if dma'ble device).
* @dma_mem: Internal for coherent mem override.
* @cma_area: Contiguous memory area for dma allocations
+ * @dma_io_tlb_mem: Internal for swiotlb io_tlb_mem override.
* @archdata: For arch-specific additions.
* @of_node: Associated device tree node.
* @fwnode:Associated device node supplied by platform firmware.
@@ -521,6 +522,9 @@ struct device {
#ifdef CONFIG_DMA_CMA
struct cma *cma_area; /* contiguous memory area for dma
allocations */
+#endif
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+ struct io_tlb_mem *dma_io_tlb_mem;
#endif
/* arch specific additions */
struct dev_archdata archdata;
diff --git a/include/linux/swiotlb.h b/include/linux/swiotlb.h
index 216854a5e513..03ad6e3b4056 100644
--- a/include/linux/swiotlb.h
+++ b/include/linux/swiotlb.h
@@ -72,7 +72,8 @@ extern enum swiotlb_force swiotlb_force;
* range check to see if the memory was in fact allocated by this
* API.
* @nslabs:The number of IO TLB blocks (in groups of 64) between @start and
- * @end. This is command line adjustable via setup_io_tlb_npages.
+ * @end. For default swiotlb, this is command line adjustable via
+ * setup_io_tlb_npages.
* @used: The number of used IO TLB block.
* @list: The free list describing the number of free entries available
* from each index.
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 858475bd6923..4ea027b75013 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -39,6 +39,13 @@
#ifdef CONFIG_DEBUG_FS
#include
#endif
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+#include
+#include
+#include
+#include
+#include
+#endif
#include
#include
@@ -687,3 +694,75 @@ static int __init swiotlb_create_default_debugfs(void)
late_initcall(swiotlb_create_default_debugfs);
#endif
+
+#ifdef CONFIG_DMA_RESTRICTED_POOL
+static int rmem_swiotlb_device_init(struct reserved_mem *rmem,
+ struct device *dev)
+{
+ struct io_tlb_mem *mem = rmem->priv;
+ unsigned long nslabs = rmem->size >> IO_TLB_SHIFT;
+
+ if (dev->dma_io_tlb_mem)
+ return 0;
+
+ /* Since multiple devices can share the same pool, the private data,
+* io_tlb_mem struct, will be initialized by the first device attached
+* to it.
+*/
+ if (!mem) {
+ mem = kzalloc(struct_size(mem, slots, nslabs), GFP_KERNEL);
+ if (!mem)
+ return -ENOMEM;
+#ifdef CONFIG_ARM
+ if (!PageHighMem(pfn_to_page(PHYS_PFN(rmem->base {
+ kfree(mem);
+ return -EINVAL;
+ }
+#endif /* CONFIG_ARM */
+ swiotlb_init_io_tlb_mem(mem, rmem->base, nslabs, false);
+
+ rmem->priv = mem;
+
+#ifdef CONFIG_DEBUG_FS
+ if (!debugfs_dir)
+ debugfs_dir = debugfs_create_dir("swiotlb", NULL);
+
+ swiotlb_create_debugfs(mem, rmem->name, debugfs_dir);
+#endif /* CONFIG_DEBUG_FS */
+ }
+
+ dev->dma_io_tlb_mem = mem;
+
+ return 0;
+}
+
+static void rmem_swiotlb_device_release(struct reserved_mem *rmem,
+ struct device *dev)
+{
+ if (dev)
+ dev->dma_io_tlb_mem = NULL;
+}
+
+static const struct reserved_mem_ops rmem_swiotlb_ops = {
+ .device_init = rmem_swiotlb_device_init,
+ .device_release = rmem_swiotlb_device_release,
+};
+
+static int __init rmem_swiotlb_setup(struct reserved_mem *rmem)
+{
+ unsigned long node = rmem->fdt_node;
+
+ if (of_get_flat_dt_prop(node, "reusable", NULL) ||
+ of_get_flat_dt_prop(node, "linux,cma-default", NULL) ||
+ of_get_flat_dt_prop(node, "linux,dma-default", NULL) ||
+ of_get_flat_dt_prop(node, "no-map", NULL))
+ return -EINVAL;
+
+ rmem->ops = _swiotlb_ops;
+ pr_info("Reserved memory: created device swiotlb memory pool at %pa,
size %ld MiB\n",
+ >base, (unsigned long)rmem->size / SZ_1M);
+ return 0;
+}
+
+RESERVEDMEM_OF_DECLARE(dma, "restricted-dma-pool", rmem_swiotlb_setup);
+#endif /* CONFIG_DMA_RESTRICTED_POOL */
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
[Intel-gfx] [PATCH v6 03/15] swiotlb: Add DMA_RESTRICTED_POOL
Add a new kconfig symbol, DMA_RESTRICTED_POOL, for restricted DMA pool. Signed-off-by: Claire Chang --- kernel/dma/Kconfig | 14 ++ 1 file changed, 14 insertions(+) diff --git a/kernel/dma/Kconfig b/kernel/dma/Kconfig index 77b405508743..3e961dc39634 100644 --- a/kernel/dma/Kconfig +++ b/kernel/dma/Kconfig @@ -80,6 +80,20 @@ config SWIOTLB bool select NEED_DMA_MAP_STATE +config DMA_RESTRICTED_POOL + bool "DMA Restricted Pool" + depends on OF && OF_RESERVED_MEM + select SWIOTLB + help + This enables support for restricted DMA pools which provide a level of + DMA memory protection on systems with limited hardware protection + capabilities, such as those lacking an IOMMU. + + For more information see + + and . + If unsure, say "n". + # # Should be selected if we can mmap non-coherent mappings to userspace. # The only thing that is really required is a way to set an uncached bit -- 2.31.1.607.g51e8a6a459-goog ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 02/15] swiotlb: Refactor swiotlb_create_debugfs
Split the debugfs creation to make the code reusable for supporting
different bounce buffer pools, e.g. restricted DMA pool.
Signed-off-by: Claire Chang
---
kernel/dma/swiotlb.c | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index d3232fc19385..858475bd6923 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -64,6 +64,7 @@
enum swiotlb_force swiotlb_force;
struct io_tlb_mem *io_tlb_default_mem;
+static struct dentry *debugfs_dir;
/*
* Max segment that we can provide which (if pages are contingous) will
@@ -662,18 +663,27 @@ EXPORT_SYMBOL_GPL(is_swiotlb_active);
#ifdef CONFIG_DEBUG_FS
-static int __init swiotlb_create_debugfs(void)
+static void swiotlb_create_debugfs(struct io_tlb_mem *mem, const char *name,
+ struct dentry *node)
{
- struct io_tlb_mem *mem = io_tlb_default_mem;
-
if (!mem)
- return 0;
- mem->debugfs = debugfs_create_dir("swiotlb", NULL);
+ return;
+
+ mem->debugfs = debugfs_create_dir(name, node);
debugfs_create_ulong("io_tlb_nslabs", 0400, mem->debugfs, >nslabs);
debugfs_create_ulong("io_tlb_used", 0400, mem->debugfs, >used);
+}
+
+static int __init swiotlb_create_default_debugfs(void)
+{
+ struct io_tlb_mem *mem = io_tlb_default_mem;
+
+ swiotlb_create_debugfs(mem, "swiotlb", NULL);
+ debugfs_dir = mem->debugfs;
+
return 0;
}
-late_initcall(swiotlb_create_debugfs);
+late_initcall(swiotlb_create_default_debugfs);
#endif
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 01/15] swiotlb: Refactor swiotlb init functions
Add a new function, swiotlb_init_io_tlb_mem, for the io_tlb_mem struct
initialization to make the code reusable.
Note that we now also call set_memory_decrypted in swiotlb_init_with_tbl.
Signed-off-by: Claire Chang
---
kernel/dma/swiotlb.c | 51 ++--
1 file changed, 25 insertions(+), 26 deletions(-)
diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
index 8ca7d505d61c..d3232fc19385 100644
--- a/kernel/dma/swiotlb.c
+++ b/kernel/dma/swiotlb.c
@@ -168,9 +168,30 @@ void __init swiotlb_update_mem_attributes(void)
memset(vaddr, 0, bytes);
}
-int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
+static void swiotlb_init_io_tlb_mem(struct io_tlb_mem *mem, phys_addr_t start,
+ unsigned long nslabs, bool late_alloc)
{
+ void *vaddr = phys_to_virt(start);
unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
+
+ mem->nslabs = nslabs;
+ mem->start = start;
+ mem->end = mem->start + bytes;
+ mem->index = 0;
+ mem->late_alloc = late_alloc;
+ spin_lock_init(>lock);
+ for (i = 0; i < mem->nslabs; i++) {
+ mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
+ mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
+ mem->slots[i].alloc_size = 0;
+ }
+
+ set_memory_decrypted((unsigned long)vaddr, bytes >> PAGE_SHIFT);
+ memset(vaddr, 0, bytes);
+}
+
+int __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
+{
struct io_tlb_mem *mem;
size_t alloc_size;
@@ -186,16 +207,8 @@ int __init swiotlb_init_with_tbl(char *tlb, unsigned long
nslabs, int verbose)
if (!mem)
panic("%s: Failed to allocate %zu bytes align=0x%lx\n",
__func__, alloc_size, PAGE_SIZE);
- mem->nslabs = nslabs;
- mem->start = __pa(tlb);
- mem->end = mem->start + bytes;
- mem->index = 0;
- spin_lock_init(>lock);
- for (i = 0; i < mem->nslabs; i++) {
- mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
- mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
- mem->slots[i].alloc_size = 0;
- }
+
+ swiotlb_init_io_tlb_mem(mem, __pa(tlb), nslabs, false);
io_tlb_default_mem = mem;
if (verbose)
@@ -282,7 +295,6 @@ swiotlb_late_init_with_default_size(size_t default_size)
int
swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
{
- unsigned long bytes = nslabs << IO_TLB_SHIFT, i;
struct io_tlb_mem *mem;
if (swiotlb_force == SWIOTLB_NO_FORCE)
@@ -297,20 +309,7 @@ swiotlb_late_init_with_tbl(char *tlb, unsigned long nslabs)
if (!mem)
return -ENOMEM;
- mem->nslabs = nslabs;
- mem->start = virt_to_phys(tlb);
- mem->end = mem->start + bytes;
- mem->index = 0;
- mem->late_alloc = 1;
- spin_lock_init(>lock);
- for (i = 0; i < mem->nslabs; i++) {
- mem->slots[i].list = IO_TLB_SEGSIZE - io_tlb_offset(i);
- mem->slots[i].orig_addr = INVALID_PHYS_ADDR;
- mem->slots[i].alloc_size = 0;
- }
-
- set_memory_decrypted((unsigned long)tlb, bytes >> PAGE_SHIFT);
- memset(tlb, 0, bytes);
+ swiotlb_init_io_tlb_mem(mem, virt_to_phys(tlb), nslabs, true);
io_tlb_default_mem = mem;
swiotlb_print_info();
--
2.31.1.607.g51e8a6a459-goog
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v6 00/15] Restricted DMA
From: Claire Chang This series implements mitigations for lack of DMA access control on systems without an IOMMU, which could result in the DMA accessing the system memory at unexpected times and/or unexpected addresses, possibly leading to data leakage or corruption. For example, we plan to use the PCI-e bus for Wi-Fi and that PCI-e bus is not behind an IOMMU. As PCI-e, by design, gives the device full access to system memory, a vulnerability in the Wi-Fi firmware could easily escalate to a full system exploit (remote wifi exploits: [1a], [1b] that shows a full chain of exploits; [2], [3]). To mitigate the security concerns, we introduce restricted DMA. Restricted DMA utilizes the existing swiotlb to bounce streaming DMA in and out of a specially allocated region and does memory allocation from the same region. The feature on its own provides a basic level of protection against the DMA overwriting buffer contents at unexpected times. However, to protect against general data leakage and system memory corruption, the system needs to provide a way to restrict the DMA to a predefined memory region (this is usually done at firmware level, e.g. MPU in ATF on some ARM platforms [4]). [1a] https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_4.html [1b] https://googleprojectzero.blogspot.com/2017/04/over-air-exploiting-broadcoms-wi-fi_11.html [2] https://blade.tencent.com/en/advisories/qualpwn/ [3] https://www.bleepingcomputer.com/news/security/vulnerabilities-found-in-highly-popular-firmware-for-wifi-chips/ [4] https://github.com/ARM-software/arm-trusted-firmware/blob/master/plat/mediatek/mt8183/drivers/emi_mpu/emi_mpu.c#L132 v6: Address the comments in v5 v5: Rebase on latest linux-next https://lore.kernel.org/patchwork/cover/1416899/ v4: - Fix spinlock bad magic - Use rmem->name for debugfs entry - Address the comments in v3 https://lore.kernel.org/patchwork/cover/1378113/ v3: Using only one reserved memory region for both streaming DMA and memory allocation. https://lore.kernel.org/patchwork/cover/1360992/ v2: Building on top of swiotlb. https://lore.kernel.org/patchwork/cover/1280705/ v1: Using dma_map_ops. https://lore.kernel.org/patchwork/cover/1271660/ *** BLURB HERE *** Claire Chang (15): swiotlb: Refactor swiotlb init functions swiotlb: Refactor swiotlb_create_debugfs swiotlb: Add DMA_RESTRICTED_POOL swiotlb: Add restricted DMA pool initialization swiotlb: Add a new get_io_tlb_mem getter swiotlb: Update is_swiotlb_buffer to add a struct device argument swiotlb: Update is_swiotlb_active to add a struct device argument swiotlb: Bounce data from/to restricted DMA pool if available swiotlb: Move alloc_size to find_slots swiotlb: Refactor swiotlb_tbl_unmap_single dma-direct: Add a new wrapper __dma_direct_free_pages() swiotlb: Add restricted DMA alloc/free support. dma-direct: Allocate memory from restricted DMA pool if available dt-bindings: of: Add restricted DMA pool of: Add plumbing for restricted DMA pool .../reserved-memory/reserved-memory.txt | 27 ++ drivers/gpu/drm/i915/gem/i915_gem_internal.c | 2 +- drivers/gpu/drm/nouveau/nouveau_ttm.c | 2 +- drivers/iommu/dma-iommu.c | 12 +- drivers/of/address.c | 25 ++ drivers/of/device.c | 3 + drivers/of/of_private.h | 5 + drivers/pci/xen-pcifront.c| 2 +- drivers/xen/swiotlb-xen.c | 2 +- include/linux/device.h| 4 + include/linux/swiotlb.h | 41 ++- kernel/dma/Kconfig| 14 + kernel/dma/direct.c | 63 +++-- kernel/dma/direct.h | 9 +- kernel/dma/swiotlb.c | 242 +- 15 files changed, 356 insertions(+), 97 deletions(-) -- 2.31.1.607.g51e8a6a459-goog ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot
This patch cause "x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x064a2000-0x064a2fff], got write-back" problem. my 2GB ram Bay trail z3735f tablet runing on android-x86, "i915: fix remap_io_sg to verify the pgprot" cause this problem. 05-09 02:59:25.099 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x0640a000-0x0640dfff], got write-back 05-09 02:59:25.106 1440 1440 W hwc-gl-worker: EGL_ANDROID_native_fence_sync extension not supported 05-09 02:59:25.111 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x064a2000-0x064a2fff], got write-back 05-09 02:59:25.118 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x0640-0x06404fff], got write-back 05-09 02:59:25.125 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x06405000-0x06408fff], got write-back 05-09 02:59:25.148 1440 1440 W hwc-gl-worker: EGL_ANDROID_native_fence_sync extension not supported 05-09 02:59:25.158 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x06542000-0x06542fff], got write-back 05-09 02:59:25.165 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x06499000-0x0649dfff], got write-back 05-09 02:59:25.171 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x0649e000-0x064a1fff], got write-back 05-09 02:59:25.177 1440 1440 W hwc-gl-worker: EGL_ANDROID_native_fence_sync extension not supported 05-09 02:59:25.183 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x065fa000-0x065fafff], got write-back 05-09 02:59:25.192 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x06539000-0x0653dfff], got write-back 05-09 02:59:25.199 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x0653e000-0x06541fff], got write-back 05-09 02:59:25.204 1440 1440 W hwc-gl-worker: EGL_ANDROID_native_fence_sync extension not supported 05-09 02:59:25.212 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x066a2000-0x066a2fff], got write-back 05-09 02:59:25.218 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x065f1000-0x065f5fff], got write-back 05-09 02:59:25.226 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x065f6000-0x065f9fff], got write-back 05-09 02:59:27.101 0 0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x08a76000-0x08a76fff], got write-back 05-09 02:59:27.225 0 0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x08a77000-0x08a7afff], got write-back 05-09 02:59:27.242 0 0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x08bd-0x08bd0fff], got write-back 05-09 02:59:27.254 0 0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x08bd1000-0x08bf0fff], got write-back 05-09 02:59:27.310 1440 1440 E drm-fb : Failed to get handle from prime fd: 25 05-09 02:59:27.322 0 0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x080d5000-0x080d9fff], got write-back 05-09 02:59:27.322 0 0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x080da000-0x080ddfff], got write-back 05-09 02:59:27.338 0 0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x1b83-0x1b83], got write-back 05-09 02:59:27.338 0 0 W x86/PAT : BootAnimation:1665 map pfn RAM range req write-combining for [mem 0x1b76a000-0x1b76efff], got write-back 05-09 02:59:27.344 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x07e87000-0x07e8bfff], got write-back 05-09 02:59:27.349 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x07e8c000-0x07e90fff], got write-back 05-09 02:59:27.347 1440 1440 E drm-fb : Failed to get handle from prime fd: 25 05-09 02:59:27.361 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c123000-0x1c126fff], got write-back 05-09 02:59:27.361 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c127000-0x1c12afff], got write-back 05-09 02:59:27.362 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c12f000-0x1c13efff], got write-back 05-09 02:59:27.362 0 0 W x86/PAT : surfaceflinger:1440 map pfn RAM range req write-combining for [mem 0x1c12b000-0x1c12efff], got write-back 05-09 02:59:27.364 1440 1440 E drm-fb : Failed to get handle from prime fd: 25 05-09 02:59:27.377
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure
Hi,
on my Dell XPS 15 9570 laptop I might have a regression with Arch Linux
(kernel 5.12.2-arch1-1: during boot the laptop monitor goes black while
external monitors still works...
Panich
Il giorno lun 11 gen 2021 alle ore 19:28 Ville Syrjälä <
ville.syrj...@linux.intel.com> ha scritto:
> On Thu, Jan 07, 2021 at 08:20:25PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä
> >
> > Some new eDP panels don't like to operate at the max parameters, and
> > instead we need to go for an optimal confiugration. That unfortunately
> > doesn't work with older eDP panels which are generally only guaranteed
> > to work at the max parameters.
> >
> > To solve these two conflicting requirements let's start with the optimal
> > setup, and if that fails we start again with the max parameters. The
> > downside is probably an extra modeset when we switch strategies but
> > I don't see a good way to avoid that.
> >
> > For a bit of history we first tried to go for the fast+narrow in
> > commit 7769db588384 ("drm/i915/dp: optimize eDP 1.4+ link config
> > fast and narrow"). but that had to be reverted due to regression
> > on older panels in commit f11cb1c19ad0 ("drm/i915/dp: revert back
> > to max link rate and lane count on eDP"). So now we try to get
> > the best of both worlds by using both strategies.
>
> Pushed. Fingers crossed for no regressions...
>
> --
> Ville Syrjälä
> Intel
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] REGRESSION with 5.12: Suspend not working on Toshiba notebook - FIXED
On Mon, May 03, 2021 at 02:00:50PM +0200, Andreas Friedrich wrote: Hello Joonas, > ... > > It's very possible that it can be i915 bug. What you can try is to > > blacklist i915 module and operate the system with SSH and see if the > > latest kernel still freezes? > This is a good idea. I have disabled i915 in my kernel configuration: > grep I915 .config > # CONFIG_DRM_I915 is not set > This time the suspend works fine! So I think it is definitely an i915 > DRM bug. > > > > Also, please try drm-tip kernel and see if it fixed there. > I have tried: > uname -r > 5.12.0-rc8+ > but no changes. The system freezes after the first try. > ... > > Please do file a bug on the issue tracker as requested: > > > > https://gitlab.freedesktop.org/drm/intel/issues/ > Done. Fixed with 5.12.2 drm/i915: Disable runtime power management during shutdown, commit 7962893ecb853aa7c8925ce237ab6c4274cfc1c7 upstream. Best regards Andreas Friedrich ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] REGRESSION with 5.12: Suspend not working on Toshiba notebook - NOT FIXED
On Sat, May 08, 2021 at 01:52:54AM +0200, Andreas Friedrich wrote: Hello Joonas, ... > Fixed with 5.12.2 > drm/i915: Disable runtime power management during shutdown, > commit 7962893ecb853aa7c8925ce237ab6c4274cfc1c7 upstream. I was wrong. One of 10 suspend still hangs. Best regards Andreas Friedrich ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH RFC] Revert "drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure"
On Sat, May 8, 2021 at 9:44 PM Paul Zimmerman wrote:
>
> This reverts commit 2bbd6dba84d44219387df051a1c799b7bac46099.
>
> Since 5.12-rc2, my Dell XPS-15 laptop has had a blank screen on boot.
> The system seems to run fine other than having no display, I am able
> to ssh into the machine. I don't see anything interesting in the dmesg
> log. I bisected the problem down to this commit, and reverting it fixes
> the problem.
>
> Signed-off-by: Paul Zimmerman
Replying to myself, to say that I just tested kernel 5.13-rc1, and the
problem still exists there, and the same revert fixes the problem.
- Paul
> ---
> I am attaching the dmesg log from 5.12.0 when the problem occurs. Any
> other debugging info you want me to provide?
>
> .../drm/i915/display/intel_display_types.h| 1 -
> drivers/gpu/drm/i915/display/intel_dp.c | 75 +++
> 2 files changed, 9 insertions(+), 67 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 184ecbbcec99..196900100689 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1424,7 +1424,6 @@ struct intel_dp {
> bool has_hdmi_sink;
> bool has_audio;
> bool reset_link_params;
> - bool use_max_params;
> u8 dpcd[DP_RECEIVER_CAP_SIZE];
> u8 psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
> u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 775d89b6c3fc..238ae1099132 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -480,13 +480,6 @@ int intel_dp_get_link_train_fallback_values(struct
> intel_dp *intel_dp,
> return -1;
> }
>
> - if (intel_dp_is_edp(intel_dp) && !intel_dp->use_max_params) {
> - drm_dbg_kms(>drm,
> - "Retrying Link training for eDP with max
> parameters\n");
> - intel_dp->use_max_params = true;
> - return 0;
> - }
> -
> index = intel_dp_rate_index(intel_dp->common_rates,
> intel_dp->num_common_rates,
> link_rate);
> @@ -1174,44 +1167,6 @@ intel_dp_compute_link_config_wide(struct intel_dp
> *intel_dp,
> return -EINVAL;
> }
>
> -/* Optimize link config in order: max bpp, min lanes, min clock */
> -static int
> -intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
> - struct intel_crtc_state *pipe_config,
> - const struct link_config_limits *limits)
> -{
> - const struct drm_display_mode *adjusted_mode =
> _config->hw.adjusted_mode;
> - int bpp, clock, lane_count;
> - int mode_rate, link_clock, link_avail;
> -
> - for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
> - int output_bpp =
> intel_dp_output_bpp(pipe_config->output_format, bpp);
> -
> - mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
> - output_bpp);
> -
> - for (lane_count = limits->min_lane_count;
> -lane_count <= limits->max_lane_count;
> -lane_count <<= 1) {
> - for (clock = limits->min_clock; clock <=
> limits->max_clock; clock++) {
> - link_clock = intel_dp->common_rates[clock];
> - link_avail =
> intel_dp_max_data_rate(link_clock,
> -
> lane_count);
> -
> - if (mode_rate <= link_avail) {
> - pipe_config->lane_count = lane_count;
> - pipe_config->pipe_bpp = bpp;
> - pipe_config->port_clock = link_clock;
> -
> - return 0;
> - }
> - }
> - }
> - }
> -
> - return -EINVAL;
> -}
> -
> static int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8
> dsc_max_bpc)
> {
> int i, num_bpc;
> @@ -1435,14 +1390,13 @@ intel_dp_compute_link_config(struct intel_encoder
> *encoder,
> limits.min_bpp = intel_dp_min_bpp(pipe_config->output_format);
> limits.max_bpp = intel_dp_max_bpp(intel_dp, pipe_config);
>
> - if (intel_dp->use_max_params) {
> + if (intel_dp_is_edp(intel_dp)) {
> /*
> * Use the maximum clock and number of lanes the eDP panel
> -* advertizes being capable of in case the initial fast
> -* optimal params failed us. The panels are generally
> +*
[Intel-gfx] [PATCH RFC] Revert "drm/i915: Try to use fast+narrow link on eDP again and fall back to the old max strategy on failure"
This reverts commit 2bbd6dba84d44219387df051a1c799b7bac46099.
Since 5.12-rc2, my Dell XPS-15 laptop has had a blank screen on boot.
The system seems to run fine other than having no display, I am able
to ssh into the machine. I don't see anything interesting in the dmesg
log. I bisected the problem down to this commit, and reverting it fixes
the problem.
Signed-off-by: Paul Zimmerman
---
I am attaching the dmesg log from 5.12.0 when the problem occurs. Any
other debugging info you want me to provide?
.../drm/i915/display/intel_display_types.h| 1 -
drivers/gpu/drm/i915/display/intel_dp.c | 75 +++
2 files changed, 9 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
b/drivers/gpu/drm/i915/display/intel_display_types.h
index 184ecbbcec99..196900100689 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1424,7 +1424,6 @@ struct intel_dp {
bool has_hdmi_sink;
bool has_audio;
bool reset_link_params;
- bool use_max_params;
u8 dpcd[DP_RECEIVER_CAP_SIZE];
u8 psr_dpcd[EDP_PSR_RECEIVER_CAP_SIZE];
u8 downstream_ports[DP_MAX_DOWNSTREAM_PORTS];
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
b/drivers/gpu/drm/i915/display/intel_dp.c
index 775d89b6c3fc..238ae1099132 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -480,13 +480,6 @@ int intel_dp_get_link_train_fallback_values(struct
intel_dp *intel_dp,
return -1;
}
- if (intel_dp_is_edp(intel_dp) && !intel_dp->use_max_params) {
- drm_dbg_kms(>drm,
- "Retrying Link training for eDP with max
parameters\n");
- intel_dp->use_max_params = true;
- return 0;
- }
-
index = intel_dp_rate_index(intel_dp->common_rates,
intel_dp->num_common_rates,
link_rate);
@@ -1174,44 +1167,6 @@ intel_dp_compute_link_config_wide(struct intel_dp
*intel_dp,
return -EINVAL;
}
-/* Optimize link config in order: max bpp, min lanes, min clock */
-static int
-intel_dp_compute_link_config_fast(struct intel_dp *intel_dp,
- struct intel_crtc_state *pipe_config,
- const struct link_config_limits *limits)
-{
- const struct drm_display_mode *adjusted_mode =
_config->hw.adjusted_mode;
- int bpp, clock, lane_count;
- int mode_rate, link_clock, link_avail;
-
- for (bpp = limits->max_bpp; bpp >= limits->min_bpp; bpp -= 2 * 3) {
- int output_bpp =
intel_dp_output_bpp(pipe_config->output_format, bpp);
-
- mode_rate = intel_dp_link_required(adjusted_mode->crtc_clock,
- output_bpp);
-
- for (lane_count = limits->min_lane_count;
-lane_count <= limits->max_lane_count;
-lane_count <<= 1) {
- for (clock = limits->min_clock; clock <=
limits->max_clock; clock++) {
- link_clock = intel_dp->common_rates[clock];
- link_avail = intel_dp_max_data_rate(link_clock,
- lane_count);
-
- if (mode_rate <= link_avail) {
- pipe_config->lane_count = lane_count;
- pipe_config->pipe_bpp = bpp;
- pipe_config->port_clock = link_clock;
-
- return 0;
- }
- }
- }
- }
-
- return -EINVAL;
-}
-
static int intel_dp_dsc_compute_bpp(struct intel_dp *intel_dp, u8 dsc_max_bpc)
{
int i, num_bpc;
@@ -1435,14 +1390,13 @@ intel_dp_compute_link_config(struct intel_encoder
*encoder,
limits.min_bpp = intel_dp_min_bpp(pipe_config->output_format);
limits.max_bpp = intel_dp_max_bpp(intel_dp, pipe_config);
- if (intel_dp->use_max_params) {
+ if (intel_dp_is_edp(intel_dp)) {
/*
* Use the maximum clock and number of lanes the eDP panel
-* advertizes being capable of in case the initial fast
-* optimal params failed us. The panels are generally
+* advertizes being capable of. The panels are generally
* designed to support only a single clock and lane
-* configuration, and typically on older panels these
-* values correspond to the native resolution of the panel.
+* configuration, and typically these values correspond to the
+* native resolution of the panel.
*/
Re: [Intel-gfx] [PATCH] drm/i915/display: relax 2big checking around initial fb
On Fri, 7 May 2021, Auld, Matthew wrote: > >From: Chris Wilson > >The kernel prefers enabling fbc over the initial fb, since this leads to >actual runtime power savings, so if the initial fb is deemed too big >using some heuristic, then we simply skip allocating stolen for it. >However if the kernel is not configured with fbcon then it should be >possible to relax this, since unlike with fbcon the display server >shouldn't preserve it when later replacing it, and so we should be able >to re-use the stolen memory for fbc and friends. This patch is reported >to fix some flicker seen during boot splash on some devices. > >Signed-off-by: Chris Wilson >Signed-off-by: Matthew Auld >Cc: Lee Shawn C >Cc: Ville Syrjälä >Cc: Daniel Vetter No more flicker issue seen during boot splash with this patch on my DUT. Thanks! >--- > drivers/gpu/drm/i915/display/intel_display.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > >diff --git a/drivers/gpu/drm/i915/display/intel_display.c >b/drivers/gpu/drm/i915/display/intel_display.c >index ec2d3fa60003..0ee1f0213fd9 100644 >--- a/drivers/gpu/drm/i915/display/intel_display.c >+++ b/drivers/gpu/drm/i915/display/intel_display.c >@@ -1455,7 +1455,7 @@ initial_plane_vma(struct drm_i915_private *i915, >* important and we should probably use that space with FBC or other >* features. >*/ >- if (size * 2 > i915->stolen_usable_size) >+ if (IS_ENABLED(FRAMEBUFFER_CONSOLE) && size * 2 > >+i915->stolen_usable_size) > return NULL; > > obj = i915_gem_object_create_stolen_for_preallocated(i915, base, size); >-- >2.26.3 > ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 1/5] drm/connector: Create a helper to attach the hdr_output_metadata property
Hi, On Fri, Apr 30, 2021 at 11:44:47AM +0200, Maxime Ripard wrote: > All the drivers that implement HDR output call pretty much the same > function to initialise the hdr_output_metadata property, and while the > creation of that property is in a helper, every driver uses the same > code to attach it. > > Provide a helper for it as well > > Reviewed-by: Harry Wentland > Reviewed-by: Jernej Skrabec > Signed-off-by: Maxime Ripard I pushed all 5 patches on friday Maxime signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gt: Do release kernel context if breadcrumb measure fails
On 10/05/2021 10:15, Janusz Krzysztofik wrote:
Hi Tvrtko,
On poniedziałek, 10 maja 2021 11:14:46 CEST Tvrtko Ursulin wrote:
On 07/05/2021 15:42, Janusz Krzysztofik wrote:
Commit fb5970da1b42 ("drm/i915/gt: Use the kernel_context to measure the
breadcrumb size") reordered some operations inside engine_init_common()
and added an error unwind path to that function. In that path, a
reference to a kernel context candidate supposed to be released on error
was put, but the context, pinned when created, was not unpinned first.
Fix it by replacing intel_context_put() with destroy_pinned_context()
introduced later by commit b436a5f8b6c8 ("drm/i915/gt: Track all timelines
created using the HWSP").
Signed-off-by: Janusz Krzysztofik
Cc: Chris Wilson
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 6dbdbde00f14..eba2da9679a5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -898,7 +898,7 @@ static int engine_init_common(struct intel_engine_cs
*engine)
return 0;
err_context:
- intel_context_put(ce);
+ destroy_pinned_context(ce);
return ret;
}
Reviewed-by: Tvrtko Ursulin
Found by some test or code inspection?
Code inspection.
Cool. It was all green on the CI front so I am pushing it as we speak.
Thanks!
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/stolen: shuffle around init_memory_region
On 07/05/2021 10:59, Matthew Auld wrote: We generally want to first call i915_gem_object_init_memory_region() before calling into get_pages(), since this sets up various bits of state which might be needed there. Currently for stolen this doesn't matter much, but it might in the future, and at the very least this makes things consistent with the other backends. Signed-off-by: Matthew Auld Cc: Thomas Hellström --- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index 293f640faa0a..b5553fc3ac4d 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -657,9 +657,11 @@ static int __i915_gem_object_create_stolen(struct intel_memory_region *mem, if (WARN_ON(!i915_gem_object_trylock(obj))) return -EBUSY; + i915_gem_object_init_memory_region(obj, mem); + err = i915_gem_object_pin_pages(obj); - if (!err) - i915_gem_object_init_memory_region(obj, mem); + if (err) + i915_gem_object_release_memory_region(obj); i915_gem_object_unlock(obj); return err; Reviewed-by: Tvrtko Ursulin Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gt: Do release kernel context if breadcrumb measure fails
Hi Tvrtko,
On poniedziałek, 10 maja 2021 11:14:46 CEST Tvrtko Ursulin wrote:
>
> On 07/05/2021 15:42, Janusz Krzysztofik wrote:
> > Commit fb5970da1b42 ("drm/i915/gt: Use the kernel_context to measure the
> > breadcrumb size") reordered some operations inside engine_init_common()
> > and added an error unwind path to that function. In that path, a
> > reference to a kernel context candidate supposed to be released on error
> > was put, but the context, pinned when created, was not unpinned first.
> > Fix it by replacing intel_context_put() with destroy_pinned_context()
> > introduced later by commit b436a5f8b6c8 ("drm/i915/gt: Track all timelines
> > created using the HWSP").
> >
> > Signed-off-by: Janusz Krzysztofik
> > Cc: Chris Wilson
> > ---
> > drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > index 6dbdbde00f14..eba2da9679a5 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> > @@ -898,7 +898,7 @@ static int engine_init_common(struct intel_engine_cs
> > *engine)
> > return 0;
> >
> > err_context:
> > - intel_context_put(ce);
> > + destroy_pinned_context(ce);
> > return ret;
> > }
> >
> >
>
> Reviewed-by: Tvrtko Ursulin
>
> Found by some test or code inspection?
Code inspection.
Thanks,
Janusz
>
> Regards,
>
> Tvrtko
>
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/gt: Do release kernel context if breadcrumb measure fails
On 07/05/2021 15:42, Janusz Krzysztofik wrote:
Commit fb5970da1b42 ("drm/i915/gt: Use the kernel_context to measure the
breadcrumb size") reordered some operations inside engine_init_common()
and added an error unwind path to that function. In that path, a
reference to a kernel context candidate supposed to be released on error
was put, but the context, pinned when created, was not unpinned first.
Fix it by replacing intel_context_put() with destroy_pinned_context()
introduced later by commit b436a5f8b6c8 ("drm/i915/gt: Track all timelines
created using the HWSP").
Signed-off-by: Janusz Krzysztofik
Cc: Chris Wilson
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index 6dbdbde00f14..eba2da9679a5 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -898,7 +898,7 @@ static int engine_init_common(struct intel_engine_cs
*engine)
return 0;
err_context:
- intel_context_put(ce);
+ destroy_pinned_context(ce);
return ret;
}
Reviewed-by: Tvrtko Ursulin
Found by some test or code inspection?
Regards,
Tvrtko
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 4/4] i915: fix remap_io_sg to verify the pgprot
On Sun, May 09, 2021 at 03:33:29AM +0800, youling257 wrote: > This patch cause "x86/PAT : surfaceflinger:1440 map pfn RAM range req > write-combining for [mem 0x064a2000-0x064a2fff], got write-back" problem. > my 2GB ram Bay trail z3735f tablet runing on android-x86, "i915: fix > remap_io_sg to verify the pgprot" cause this problem. So this is the memtype verification added by the patch, meaning that the old code did in fact not call into track_pfn_remap with the right flags. Can the i915 maintainers take a look at making sure the page permissions here make sense? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
