Re: [PATCH 18/48] ARM: pxa: hx4700: use gpio descriptors for audio
On Sun, May 1, 2022 at 11:41 PM Linus Walleij wrote: > > (...) > > +static struct gpiod_lookup_table hx4700_audio_gpio_table = { > > + .dev_id = "hx4700-audio", > > + .table = { > > + GPIO_LOOKUP("gpio-pxa", GPIO75_HX4700_EARPHONE_nDET, > > + "earphone-ndet", GPIO_ACTIVE_HIGH), > > This looks wrong. The n in nDET in the end of the name of the GPIO line > means active low does it not? > > What I usually do when I see this is to properly set it to > GPIO_ACTIVE_LOW in the descriptor table, then invert the logic > where it's getting used. > > Also rename to earphone-det instead of -ndet Thanks for taking a look! I changed it now, but I don't know if I got the correct number of inversions in the end. How does this look? Arnd commit 9d452c9cbee59fc58c940b5f7ae5a40579aab0d2 Author: Arnd Bergmann Date: Wed Sep 11 14:27:13 2019 +0200 ARM: pxa: hx4700: use gpio descriptors for audio The audio driver should not use a hardwired gpio number from the header. Change it to use a lookup table. Cc: Philipp Zabel Cc: Paul Parsons Acked-by: Mark Brown Acked-by: Robert Jarzmik Cc: alsa-de...@alsa-project.org Signed-off-by: Arnd Bergmann diff --git a/arch/arm/mach-pxa/hx4700-pcmcia.c b/arch/arm/mach-pxa/hx4700-pcmcia.c index e8acbfc9ef6c..e2331dfe427d 100644 --- a/arch/arm/mach-pxa/hx4700-pcmcia.c +++ b/arch/arm/mach-pxa/hx4700-pcmcia.c @@ -10,7 +10,7 @@ #include #include -#include +#include "hx4700.h" #include diff --git a/arch/arm/mach-pxa/hx4700.c b/arch/arm/mach-pxa/hx4700.c index 140a44cb2989..2ae06edf413c 100644 --- a/arch/arm/mach-pxa/hx4700.c +++ b/arch/arm/mach-pxa/hx4700.c @@ -41,7 +41,7 @@ #include "pxa27x.h" #include "addr-map.h" -#include +#include "hx4700.h" #include #include @@ -834,6 +834,19 @@ static struct i2c_board_info i2c_board_info[] __initdata = { }, }; +static struct gpiod_lookup_table hx4700_audio_gpio_table = { + .dev_id = "hx4700-audio", + .table = { + GPIO_LOOKUP("gpio-pxa", GPIO75_HX4700_EARPHONE_nDET, + "earphone-det", GPIO_ACTIVE_LOW), + GPIO_LOOKUP("gpio-pxa", GPIO92_HX4700_HP_DRIVER, + "hp-driver", GPIO_ACTIVE_HIGH), + GPIO_LOOKUP("gpio-pxa", GPIO107_HX4700_SPK_nSD, + "spk-sd", GPIO_ACTIVE_LOW), + { }, + }, +}; + static struct platform_device audio = { .name = "hx4700-audio", .id = -1, @@ -895,6 +908,7 @@ static void __init hx4700_init(void) gpiod_add_lookup_table(&bq24022_gpiod_table); gpiod_add_lookup_table(&gpio_vbus_gpiod_table); + gpiod_add_lookup_table(&hx4700_audio_gpio_table); platform_add_devices(devices, ARRAY_SIZE(devices)); pwm_add_table(hx4700_pwm_lookup, ARRAY_SIZE(hx4700_pwm_lookup)); diff --git a/arch/arm/mach-pxa/include/mach/hx4700.h b/arch/arm/mach-pxa/hx4700.h similarity index 99% rename from arch/arm/mach-pxa/include/mach/hx4700.h rename to arch/arm/mach-pxa/hx4700.h index 0c30e6d9c660..ce2db33989e1 100644 --- a/arch/arm/mach-pxa/include/mach/hx4700.h +++ b/arch/arm/mach-pxa/hx4700.h @@ -10,7 +10,7 @@ #include #include -#include "irqs.h" /* PXA_NR_BUILTIN_GPIO */ +#include /* PXA_NR_BUILTIN_GPIO */ #define HX4700_ASIC3_GPIO_BASE PXA_NR_BUILTIN_GPIO #define HX4700_EGPIO_BASE (HX4700_ASIC3_GPIO_BASE + ASIC3_NUM_GPIOS) diff --git a/sound/soc/pxa/hx4700.c b/sound/soc/pxa/hx4700.c index 7334fac758de..a0734b742322 100644 --- a/sound/soc/pxa/hx4700.c +++ b/sound/soc/pxa/hx4700.c @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include #include @@ -18,10 +18,10 @@ #include #include -#include #include #include "pxa2xx-i2s.h" +static struct gpio_desc *gpiod_hp_driver, *gpiod_spk_sd; static struct snd_soc_jack hs_jack; /* Headphones jack detection DAPM pin */ @@ -40,9 +40,7 @@ static struct snd_soc_jack_pin hs_jack_pin[] = { /* Headphones jack detection GPIO */ static struct snd_soc_jack_gpio hs_jack_gpio = { - .gpio = GPIO75_HX4700_EARPHONE_nDET, - .invert = true, - .name = "hp-gpio", + .name = "earphone-det", .report = SND_JACK_HEADPHONE, .debounce_time = 200, }; @@ -81,14 +79,14 @@ static const struct snd_soc_ops hx4700_ops = { static int hx4700_spk_power(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event) { - gpio_set_value(GPIO107_HX4700_SPK_nSD, !!SND_SOC_DAPM_EVENT_ON(event)); + gpiod_set_value(gpiod_spk_sd, !!SND_SOC_DAPM_EVENT_ON(event)); return 0; } static int hx4700_hp_power(struct snd_soc_dapm_widget *w, struct snd_kcontrol *k, int event) { - gpio_set_value(GPIO92_HX4700_HP_DRIVER, !!SND_SOC_DAPM_EVENT_ON(event)); + gpiod_set_value(gpiod_hp_driver, !!SND_SOC_DAPM_EVENT_ON
Re: [PATCH v2] drm/doc: add rfc section for small BAR uapi
On 20/04/2022 20:13, Matthew Auld wrote: Add an entry for the new uapi needed for small BAR on DG2+. v2: - Some spelling fixes and other small tweaks. (Akeem & Thomas) - Rework error capture interactions, including no longer needing NEEDS_CPU_ACCESS for objects marked for capture. (Thomas) - Add probed_cpu_visible_size. (Lionel) Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Lionel Landwerlin Cc: Jon Bloomfield Cc: Daniel Vetter Cc: Jordan Justen Cc: Kenneth Graunke Cc: Akeem G Abodunrin Cc: mesa-...@lists.freedesktop.org --- Documentation/gpu/rfc/i915_small_bar.h | 190 +++ Documentation/gpu/rfc/i915_small_bar.rst | 58 +++ Documentation/gpu/rfc/index.rst | 4 + 3 files changed, 252 insertions(+) create mode 100644 Documentation/gpu/rfc/i915_small_bar.h create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst diff --git a/Documentation/gpu/rfc/i915_small_bar.h b/Documentation/gpu/rfc/i915_small_bar.h new file mode 100644 index ..7bfd0cf44d35 --- /dev/null +++ b/Documentation/gpu/rfc/i915_small_bar.h @@ -0,0 +1,190 @@ +/** + * struct __drm_i915_memory_region_info - Describes one region as known to the + * driver. + * + * Note this is using both struct drm_i915_query_item and struct drm_i915_query. + * For this new query we are adding the new query id DRM_I915_QUERY_MEMORY_REGIONS + * at &drm_i915_query_item.query_id. + */ +struct __drm_i915_memory_region_info { + /** @region: The class:instance pair encoding */ + struct drm_i915_gem_memory_class_instance region; + + /** @rsvd0: MBZ */ + __u32 rsvd0; + + /** @probed_size: Memory probed by the driver (-1 = unknown) */ + __u64 probed_size; + + /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */ + __u64 unallocated_size; + + union { + /** @rsvd1: MBZ */ + __u64 rsvd1[8]; + struct { + /** +* @probed_cpu_visible_size: Memory probed by the driver +* that is CPU accessible. (-1 = unknown). +* +* This will be always be <= @probed_size, and the +* remainder(if there is any) will not be CPU +* accessible. +*/ + __u64 probed_cpu_visible_size; + }; Trying to implement userspace support in Vulkan for this, I have an additional question about the value of probed_cpu_visible_size. When is it set to -1? I'm guessing before there is support for this value it'll be 0 (MBZ). After after it should either be the entire lmem or something smaller. -Lionel + }; +}; + +/** + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour, with added + * extension support using struct i915_user_extension. + * + * Note that new buffer flags should be added here, at least for the stuff that + * is immutable. Previously we would have two ioctls, one to create the object + * with gem_create, and another to apply various parameters, however this + * creates some ambiguity for the params which are considered immutable. Also in + * general we're phasing out the various SET/GET ioctls. + */ +struct __drm_i915_gem_create_ext { + /** +* @size: Requested size for the object. +* +* The (page-aligned) allocated size for the object will be returned. +* +* Note that for some devices we have might have further minimum +* page-size restrictions(larger than 4K), like for device local-memory. +* However in general the final size here should always reflect any +* rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS +* extension to place the object in device local-memory. +*/ + __u64 size; + /** +* @handle: Returned handle for the object. +* +* Object handles are nonzero. +*/ + __u32 handle; + /** +* @flags: Optional flags. +* +* Supported values: +* +* I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the kernel that +* the object will need to be accessed via the CPU. +* +* Only valid when placing objects in I915_MEMORY_CLASS_DEVICE, and +* only strictly required on platforms where only some of the device +* memory is directly visible or mappable through the CPU, like on DG2+. +* +* One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM, to +* ensure we can always spill the allocation to system memory, if we +* can't place the object in the mappable part of +* I915_MEMORY_CLASS_DEVICE. +* +* Note that since the kernel only supports flat-CCS on objects that can +* *only* be placed in I915_MEMORY_CLASS_DEVICE, we therefore don't +* su
Re: [Freedreno] [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing
On Mon, 2 May 2022 at 02:56, Abhinav Kumar wrote: > > > > On 5/1/2022 1:06 PM, Marijn Suijten wrote: > > On 2022-04-30 12:25:57, Abhinav Kumar wrote: > >> > >> > >> On 4/30/2022 11:58 AM, Marijn Suijten wrote: > >>> On 2022-04-30 20:55:33, Dmitry Baryshkov wrote: > The downstream uses read-modify-write for updating command mode > compression registers. Let's follow this approach. This also fixes the > following warning: > > drivers/gpu/drm/msm/dsi/dsi_host.c:918:23: warning: variable 'reg_ctrl' > set but not used [-Wunused-but-set-variable] > > Reported-by: kernel test robot > Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration") > Signed-off-by: Dmitry Baryshkov > >>> > >>> I pointed this out in review multiple times, so you'll obviously get my: > >>> > >>> Reviewed-by: Marijn Suijten > >>> > >>> (But are you sure there's nothing else to clear in the 1st CTRL > >>> register, only the lowest 16 bits? That should mean `reg` never > >>> contains anything in 0x) > >> > >> The top 16 bits contain information for stream 1. > >> > >> Stream 1 is unused. And whatever is the reset value we should retain that. > >> > >> So this patch is correct. > > > > I was not debating the correctness of this patch, quite the contrary: > > it's already much better than it was before. > > > > I'm simply asking whether we should prevent `reg` (not `reg_ctrl`!) > > from having anything in the top 16 bits, which would overwrite the reset > > value for stream 1 which you correctly say should be retained as it is. > > It seems unlikely that this happens, but better be safe than sorry? > > Wouln't this macro already make sure that 'reg' doesnt have anything in > the top 16 bits? Its doing a & with 0x3f00 > > #define DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK > 0x3f00 > #define DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT 8 > static inline uint32_t > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(uint32_t val) > { > return ((val) << > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT) & > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK; > } > > Did you have anything else in mind? If so, please suggest. > > > > > Looking at the DSI register definition for DSC [1] again, I wonder if > > there's support for defining a common bitfield layout and reusing it > > thrice: the two channels for CMD mode and a single use for VIDEO. Don't > > think that'd make the kernel code better though if even possible at all. > > > > We can have a common bitfield layout for the two channels for command mode. > > So we can do something like below for common fields: > > -static inline uint32_t > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(uint32_t val) > +static inline uint32_t > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM_DATATYPE(uint32_t val, uint32_t > stream_id) > { > - return ((val) << > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT) & > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK; > + return ((val) << > (DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT + (stream_id > * 16)) & DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK; > } NAK. Please express this in the xml source. This file is autogenerated. > > Video mode can also use all of these except for WC as that field is not > present for cmd modes. > > This can go as a separate change . > > I can push this and perhaps get a Tested-by from Vinod as I dont have a > setup to re-validate this. > > Thanks > > Abhinav > > [1]: https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14967/diffs > > > > - Marijn > > > >>> > >>> However, this seems to indicate that the DSC patch series has been > >>> approved and merged somehow?? > >>> > --- > > Changes since v1: > - Fix c&p error and apply mask clear to reg_ctrl2 instead of reg_ctrl > (Abhinav) > > --- > drivers/gpu/drm/msm/dsi/dsi_host.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c > b/drivers/gpu/drm/msm/dsi/dsi_host.c > index c983698d1384..a95d5df52653 100644 > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c > @@ -961,10 +961,13 @@ static void dsi_update_dsc_timing(struct > msm_dsi_host *msm_host, bool is_cmd_mod > reg_ctrl = dsi_read(msm_host, > REG_DSI_COMMAND_COMPRESSION_MODE_CTRL); > reg_ctrl2 = dsi_read(msm_host, > REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2); > > + reg_ctrl &= ~0x; > reg_ctrl |= reg; > + > + reg_ctrl2 &= > ~DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH__MASK; > reg_ctrl2 |= > DSI_COMMAND_COMPRESSION_MODE_CTRL2_STREAM0_SLICE_WIDTH(bytes_in_slice); > > -
Re: [Freedreno] [PATCH] drm/msm/disp/dpu1: avoid clearing hw interrupts if hw_intr is null during drm uninit
On Mon, 2 May 2022 at 04:38, Abhinav Kumar wrote: > > Looks like our new CI has given all the answers we need :) which is a > great win for the CI in my opinion. > > Take a look at this report : > https://gitlab.freedesktop.org/drm/msm/-/jobs/22015361 > > This issue seems to be because this change > https://github.com/torvalds/linux/commit/169466d4e59ca204683998b7f45673ebf0eb2de6 > is missing in our tree. > > Without this change, what happens is that we are not hitting the return > 0 because we check for ENODEV. > > >/* > * External bridges are mandatory for eDP interfaces: one has to > * provide at least an eDP panel (which gets wrapped into > panel-bridge). > * > * For DisplayPort interfaces external bridges are optional, so > * silently ignore an error if one is not present (-ENODEV). > */ > rc = dp_parser_find_next_bridge(dp_priv->parser); > if (!dp->is_edp && rc == -ENODEV) > return 0; > > So, I think we should do both: > > 1) Since we are running CI on the tree, backport this change so that > this error path doesnt hit? > > 2) Add this protection as well because this shows that we can indeed hit > this path in EDEFER cases causing this crash. I have been waiting for v2 for the last week or so. It should include a fixed Fixes tag and an updated description (which should note that this happens in the error path, etc) as requested by Stephen. > > Thanks > > Abhinav > > On 4/27/2022 3:53 AM, Dmitry Baryshkov wrote: > > On 27/04/2022 00:50, Stephen Boyd wrote: > >> Quoting Vinod Polimera (2022-04-25 23:02:11) > >>> Avoid clearing irqs and derefernce hw_intr when hw_intr is null. > >> > >> Presumably this is only the case when the display driver doesn't fully > >> probe and something probe defers? Can you clarify how this situation > >> happens? > >> > >>> > >>> BUG: Unable to handle kernel NULL pointer dereference at virtual > >>> address > >>> > >>> Call trace: > >>> dpu_core_irq_uninstall+0x50/0xb0 > >>> dpu_irq_uninstall+0x18/0x24 > >>> msm_drm_uninit+0xd8/0x16c > >>> msm_drm_bind+0x580/0x5fc > >>> try_to_bring_up_master+0x168/0x1c0 > >>> __component_add+0xb4/0x178 > >>> component_add+0x1c/0x28 > >>> dp_display_probe+0x38c/0x400 > >>> platform_probe+0xb0/0xd0 > >>> really_probe+0xcc/0x2c8 > >>> __driver_probe_device+0xbc/0xe8 > >>> driver_probe_device+0x48/0xf0 > >>> __device_attach_driver+0xa0/0xc8 > >>> bus_for_each_drv+0x8c/0xd8 > >>> __device_attach+0xc4/0x150 > >>> device_initial_probe+0x1c/0x28 > >>> > >>> Fixes: a73033619ea ("drm/msm/dpu: squash dpu_core_irq into > >>> dpu_hw_interrupts") > >> > >> The fixes tag looks odd. In dpu_core_irq_uninstall() at that commit it > >> is dealing with 'irq_obj' which isn't a pointer. After commit > >> f25f656608e3 ("drm/msm/dpu: merge struct dpu_irq into struct > >> dpu_hw_intr") dpu_core_irq_uninstall() starts using 'hw_intr' which is > >> allocated on the heap. If we backported this patch to a place that had > >> a73033619ea without f25f656608e3 it wouldn't make any sense. > > > > I'd agree here. The following tag would be correct: > > > > Fixes: f25f656608e3 ("drm/msm/dpu: merge struct dpu_irq into struct > > dpu_hw_intr") > > > > > >> > >>> Signed-off-by: Vinod Polimera > >>> --- > >>> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > >>> b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > >>> index c515b7c..ab28577 100644 > >>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > >>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c > >>> @@ -599,6 +599,9 @@ void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms) > >>> { > >>> int i; > >>> > >>> + if (!dpu_kms->hw_intr) > >>> + return; > >>> + > >>> pm_runtime_get_sync(&dpu_kms->pdev->dev); > >>> for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++) > > > > -- With best wishes Dmitry
Re: [PATCH] drm/msm/dpu: add missing break statement for update_pending_flush_wb()
On Mon, 2 May 2022 at 08:39, Abhinav Kumar wrote: > > Add missing break statement for dpu_hw_ctl_update_pending_flush_wb(). > Otherwise this leads to below build warning. > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c:273:2: > warning: unannotated fall-through between switch labels >default: >^ > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c:273:2: > note: insert 'break;' to avoid fall-through >default: >^ >break; > 1 warning generated. > > Fixes: 2e0086d8c61d ("drm/msm/dpu: add changes to support writeback in > hw_ctl") > Reported-by: kernel test robot > Signed-off-by: Abhinav Kumar Reviewed-by: Dmitry Baryshkov > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > index 254fdf06bb42..c33e7ef611a6 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c > @@ -270,6 +270,7 @@ static void dpu_hw_ctl_update_pending_flush_wb(struct > dpu_hw_ctl *ctx, > case WB_1: > case WB_2: > ctx->pending_flush_mask |= BIT(WB_IDX); > + break; > default: > break; > } > -- > 2.7.4 > -- With best wishes Dmitry
[PATCH] drm/msm/dpu: don't access mode pointer before it is set
Move the initializer for the mode variable to the declaration point to remove unitialized variable access from the DEBUG_DPU macro. This fixes the following warning: drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c:250:37: note: initialize the variable 'mode' to silence this warning Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for writeback") Reported-by: kernel test robot Signed-off-by: Dmitry Baryshkov --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index f4a79715a02e..4829d1ce0cf8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -247,7 +247,7 @@ static int dpu_encoder_phys_wb_atomic_check( struct drm_connector_state *conn_state) { struct drm_framebuffer *fb; - const struct drm_display_mode *mode; + const struct drm_display_mode *mode = &crtc_state->mode; DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n", phys_enc->wb_idx, mode->name, mode->hdisplay, mode->vdisplay); @@ -256,7 +256,6 @@ static int dpu_encoder_phys_wb_atomic_check( return 0; fb = conn_state->writeback_job->fb; - mode = &crtc_state->mode; if (!conn_state || !conn_state->connector) { DPU_ERROR("invalid connector state\n"); -- 2.35.1
Re: [PATCH] drm/i915: use IOMEM_ERR_PTR() directly
On Mon, 02 May 2022, Kefeng Wang wrote: > Use IOMEM_ERR_PTR() instead of self defined IO_ERR_PTR(). > > Signed-off-by: Kefeng Wang Reviewed-by: Jani Nikula > --- > drivers/gpu/drm/i915/i915_vma.c | 4 ++-- > drivers/gpu/drm/i915/i915_vma.h | 1 - > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c > index 94fcdb7bd21d..639605c89b7b 100644 > --- a/drivers/gpu/drm/i915/i915_vma.c > +++ b/drivers/gpu/drm/i915/i915_vma.c > @@ -541,7 +541,7 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma) > int err; > > if (WARN_ON_ONCE(vma->obj->flags & I915_BO_ALLOC_GPU_ONLY)) > - return IO_ERR_PTR(-EINVAL); > + return IOMEM_ERR_PTR(-EINVAL); > > if (!i915_gem_object_is_lmem(vma->obj)) { > if (GEM_WARN_ON(!i915_vma_is_map_and_fenceable(vma))) { > @@ -594,7 +594,7 @@ void __iomem *i915_vma_pin_iomap(struct i915_vma *vma) > err_unpin: > __i915_vma_unpin(vma); > err: > - return IO_ERR_PTR(err); > + return IOMEM_ERR_PTR(err); > } > > void i915_vma_flush_writes(struct i915_vma *vma) > diff --git a/drivers/gpu/drm/i915/i915_vma.h b/drivers/gpu/drm/i915/i915_vma.h > index 67ae7341c7e0..8e74972fdca3 100644 > --- a/drivers/gpu/drm/i915/i915_vma.h > +++ b/drivers/gpu/drm/i915/i915_vma.h > @@ -331,7 +331,6 @@ static inline bool i915_node_color_differs(const struct > drm_mm_node *node, > * Returns a valid iomapped pointer or ERR_PTR. > */ > void __iomem *i915_vma_pin_iomap(struct i915_vma *vma); > -#define IO_ERR_PTR(x) ((void __iomem *)ERR_PTR(x)) > > /** > * i915_vma_unpin_iomap - unpins the mapping returned from i915_vma_iomap -- Jani Nikula, Intel Open Source Graphics Center
Re: [Freedreno] [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing
On 2022-05-01 16:56:45, Abhinav Kumar wrote: > [snip] > Wouln't this macro already make sure that 'reg' doesnt have anything in > the top 16 bits? Its doing a & with 0x3f00 Like I said, it is unlikely that this happens, only if someone starts changing the code that assigns to `reg` which is unlikely to pass review anyway. > [snip] > We can have a common bitfield layout for the two channels for command mode. > > So we can do something like below for common fields: > > -static inline uint32_t > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(uint32_t val) > +static inline uint32_t > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM_DATATYPE(uint32_t val, uint32_t > stream_id) > { > - return ((val) << > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT) & > DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK; > + return ((val) << > (DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT + (stream_id > * 16)) & DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK; > } > > Video mode can also use all of these except for WC as that field is not > present for cmd modes. > > This can go as a separate change . > > I can push this and perhaps get a Tested-by from Vinod as I dont have a > setup to re-validate this. How would you represent this in XML? I was hoping for a method that allows to construct the value in a generic way, without register names, and then simply have a "register macro" that moves (and perhaps masks) the preconstructed value into the right place. A bit like how `enum`s are currently set up in XML, but with bit ranges for the values and macros to set a value. I think I've _partially_ found what I was looking for: a ``. However, I don't know if we can utilize this multiple times within a single `reg32`, once with an offset for stream1. Alas, it's just bikeshedding at this point. - Marijn
Re: [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing
On 2022-05-02 01:44:20, Dmitry Baryshkov wrote: > [sni[ > > In any case, given that you've already sent this patch and another three > > patches [2] fixing/cleaning up the series tells me it's far from ready. > > Most of this should just be handled - or have been handled - in review > > and amended? > > During the review time we agreed that [2] would come as a separate > change It is an API change that would make using panel-bridge easier, > but isn't otherwise required. > > I have been working towards more logical drm_bridge/drm_bridge_connector > chains employing panel-bridge and display-connector where required, [2] > is a part of that effort (as well as few other patches that hit > dri-devel in the last few days). I understand what is going on now. Since the DSC patches have already been queued up in the 5.19 pull I won't hurry to review them; rather will go over them when time allows me to play with the many phones here that require DSC for the screen to work. I've been told the series didn't result in positive screen output way back in its infancy, but I'll re-evaluate and send fixes or improvements if/when necessary. - Marijn
[PATCH 0/3] drm: Allow simpledrm to setup its emulated FB as firmware provided
Hello, This series contain patches suggested by Thomas Zimmermannas a feedback for "[RFC PATCH v4 00/11] Fix some race between sysfb device registration and drivers probe" [0]. Since other changes in [0] were more controversial, I decided to just split this part in a new patch-set and revisit the rest of the patches later. Patch #1 is just a cleanup since when working on this noticed that some DRM drivers were passing as preferred bits per pixel to drm_fbdev_generic_setup() the value that is the default anyways. Patch #2 renames the 'preferred_bpp' drm_fbdev_generic_setup() parameter to 'options', and make this a multi field parameter so that it can be extended later to pass other options as well. Patch #3 finally adds the new DRM_FB_FW option and makes simpledrm to use it so that the registered framebuffer device is also marked as firmware provided. [0]: https://lore.kernel.org/lkml/20220429084253.1085911-1-javi...@redhat.com/ Javier Martinez Canillas (3): drm: Remove superfluous arg when calling to drm_fbdev_generic_setup() drm/fb-helper: Rename preferred_bpp drm_fbdev_generic_setup() parameter drm: Allow simpledrm to setup its emulated FB as firmware provided drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 +++-- drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- drivers/gpu/drm/arm/malidp_drv.c | 2 +- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- drivers/gpu/drm/ast/ast_drv.c | 2 +- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +- drivers/gpu/drm/drm_drv.c | 2 +- drivers/gpu/drm/drm_fb_helper.c | 25 --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +- .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +- drivers/gpu/drm/imx/dcss/dcss-kms.c | 2 +- drivers/gpu/drm/imx/imx-drm-core.c| 2 +- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- drivers/gpu/drm/mcde/mcde_drv.c | 2 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c| 2 +- drivers/gpu/drm/meson/meson_drv.c | 2 +- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +- drivers/gpu/drm/pl111/pl111_drv.c | 2 +- drivers/gpu/drm/qxl/qxl_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/sti/sti_drv.c | 2 +- drivers/gpu/drm/stm/drv.c | 2 +- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- drivers/gpu/drm/tidss/tidss_drv.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- drivers/gpu/drm/tiny/arcpgu.c | 2 +- drivers/gpu/drm/tiny/bochs.c | 2 +- drivers/gpu/drm/tiny/cirrus.c | 2 +- drivers/gpu/drm/tiny/simpledrm.c | 2 +- drivers/gpu/drm/tve200/tve200_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 2 +- drivers/gpu/drm/vc4/vc4_drv.c | 2 +- drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 +- include/drm/drm_fb_helper.h | 22 36 files changed, 80 insertions(+), 39 deletions(-) -- 2.35.1
[PATCH 1/3] drm: Remove superfluous arg when calling to drm_fbdev_generic_setup()
The drm_fbdev_generic_setup() function already sets the preferred bits per pixel for the device to dev->mode_config.preferred_depth, if preferred_bpp value is zero. Passing the same value to the function is unnecessary. Let's cleanup that in the two drivers that do it. Signed-off-by: Javier Martinez Canillas --- drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +- drivers/gpu/drm/tiny/cirrus.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index fe4269c5aa0a..ace92459e462 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -349,7 +349,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, goto err_unload; } - drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth); + drm_fbdev_generic_setup(dev, 0); return 0; diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c index c8e791840862..ed5a2e14894a 100644 --- a/drivers/gpu/drm/tiny/cirrus.c +++ b/drivers/gpu/drm/tiny/cirrus.c @@ -601,7 +601,7 @@ static int cirrus_pci_probe(struct pci_dev *pdev, if (ret) return ret; - drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth); + drm_fbdev_generic_setup(dev, 0); return 0; } -- 2.35.1
[PATCH 2/3] drm/fb-helper: Rename preferred_bpp drm_fbdev_generic_setup() parameter
By default the bits per pixel for the emulated framebuffer device is set to dev->mode_config.preferred_depth, but some devices need another value. Since this second parameter is only used by a few drivers, and to allow drivers to use it for passing other configurations when registering the fbdev, rename @preferred_bpp to @options and make it a multi-field param. The DRM_FB_SET_OPTION() and DRM_FB_GET_OPTION() macros are provided for drivers to set and get an option respectively. For now, only DRM_FB_BPP option exists but other options would be added later. Suggested-by: Thomas Zimmermann Signed-off-by: Javier Martinez Canillas --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 -- drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- drivers/gpu/drm/arm/malidp_drv.c| 2 +- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- drivers/gpu/drm/ast/ast_drv.c | 2 +- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 2 +- drivers/gpu/drm/drm_drv.c | 2 +- drivers/gpu/drm/drm_fb_helper.c | 16 drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +- drivers/gpu/drm/imx/dcss/dcss-kms.c | 2 +- drivers/gpu/drm/imx/imx-drm-core.c | 2 +- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- drivers/gpu/drm/mcde/mcde_drv.c | 2 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +- drivers/gpu/drm/meson/meson_drv.c | 2 +- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +- drivers/gpu/drm/pl111/pl111_drv.c | 2 +- drivers/gpu/drm/qxl/qxl_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/sti/sti_drv.c | 2 +- drivers/gpu/drm/stm/drv.c | 2 +- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- drivers/gpu/drm/tidss/tidss_drv.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- drivers/gpu/drm/tiny/arcpgu.c | 2 +- drivers/gpu/drm/tiny/bochs.c| 2 +- drivers/gpu/drm/tve200/tve200_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c| 2 +- drivers/gpu/drm/vc4/vc4_drv.c | 2 +- drivers/gpu/drm/virtio/virtgpu_drv.c| 2 +- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 +- include/drm/drm_fb_helper.h | 12 33 files changed, 58 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index b03663f42cc9..f5fae3838cdc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2118,9 +2118,11 @@ static int amdgpu_pci_probe(struct pci_dev *pdev, !list_empty(&adev_to_drm(adev)->mode_config.connector_list)) { /* select 8 bpp console on low vram cards */ if (adev->gmc.real_vram_size <= (32*1024*1024)) - drm_fbdev_generic_setup(adev_to_drm(adev), 8); + drm_fbdev_generic_setup(adev_to_drm(adev), + DRM_FB_SET_OPTION(DRM_FB_BPP, 8)); else - drm_fbdev_generic_setup(adev_to_drm(adev), 32); + drm_fbdev_generic_setup(adev_to_drm(adev), + DRM_FB_SET_OPTION(DRM_FB_BPP, 32)); } ret = amdgpu_debugfs_init(adev); diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c index e89ae0ec60eb..636b0e989398 100644 --- a/drivers/gpu/drm/arm/hdlcd_drv.c +++ b/drivers/gpu/drm/arm/hdlcd_drv.c @@ -321,7 +321,7 @@ static int hdlcd_drm_bind(struct device *dev) if (ret) goto err_register; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_generic_setup(drm, DRM_FB_SET_OPTION(DRM_FB_BPP, 32)); return 0; diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index d5aef21426cf..8f7c4f3136d5 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -863,7 +863,7 @@ static int malidp_bind(struct device *dev) if (ret) goto register_fail; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_generic_setup(drm, DRM_FB_SET_OPTION(DRM_FB_BPP, 32)); return 0; diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index 7780b72de9e8..c16837fd1f8d 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -343,7 +343,7 @@ static int aspeed_gfx_probe(struct platform_device *pdev) if (ret) goto err_unload; - drm_fbdev_generic_setup(&priv->drm, 32); + drm_fbdev_generic_setup(&priv->drm, DRM_FB_SET_OP
[PATCH 3/3] drm: Allow simpledrm to setup its emulated FB as firmware provided
Indicate to fbdev subsystem that the registered framebuffer is provided by the system firmware, so that it can handle accordingly. For example, would unregister the FB devices if asked to remove the conflicting framebuffers. Add a new DRM_FB_FW field to drm_fbdev_generic_setup() options parameter. Drivers can use this to indicate the FB helper initialization that the FB registered is provided by the firmware, so it can be configured as such. Suggested-by: Thomas Zimmermann Signed-off-by: Javier Martinez Canillas --- drivers/gpu/drm/drm_fb_helper.c | 9 + drivers/gpu/drm/tiny/simpledrm.c | 2 +- include/drm/drm_fb_helper.h | 10 ++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index f626947bb9b9..c2ff986f064d 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1891,6 +1891,10 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper, /* don't leak any physical addresses to userspace */ info->flags |= FBINFO_HIDE_SMEM_START; + /* Indicate that the framebuffer is provided by the firmware */ + if (fb_helper->firmware) + info->flags |= FBINFO_MISC_FIRMWARE; + /* Need to drop locks to avoid recursive deadlock in * register_framebuffer. This is ok because the only thing left to do is * register the fbdev emulation instance in kernel_fb_helper_list. */ @@ -2511,6 +2515,8 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = { * * * DRM_FB_BPP: bits per pixel for the device. If the field is not set, * @dev->mode_config.preferred_depth is used instead. + * * DRM_FB_FW: if the framebuffer for the device is provided by the + * system firmware. * * This function sets up generic fbdev emulation for drivers that supports * dumb buffers with a virtual address and that can be mmap'ed. @@ -2537,6 +2543,7 @@ void drm_fbdev_generic_setup(struct drm_device *dev, unsigned int options) { struct drm_fb_helper *fb_helper; unsigned int preferred_bpp = DRM_FB_GET_OPTION(DRM_FB_BPP, options); + bool firmware = DRM_FB_GET_OPTION(DRM_FB_FW, options); int ret; drm_WARN(dev, !dev->registered, "Device has not been registered.\n"); @@ -2569,6 +2576,8 @@ void drm_fbdev_generic_setup(struct drm_device *dev, unsigned int options) preferred_bpp = 32; fb_helper->preferred_bpp = preferred_bpp; + fb_helper->firmware = firmware; + ret = drm_fbdev_client_hotplug(&fb_helper->client); if (ret) drm_dbg_kms(dev, "client hotplug ret=%d\n", ret); diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index f5b8e864a5cd..5dcc21ea6180 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -901,7 +901,7 @@ static int simpledrm_probe(struct platform_device *pdev) if (ret) return ret; - drm_fbdev_generic_setup(dev, 0); + drm_fbdev_generic_setup(dev, DRM_FB_SET_OPTION(DRM_FB_FW, 1)); return 0; } diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 1da3ef76f499..0eec500e0784 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -44,6 +44,7 @@ enum mode_set_atomic { }; #define DRM_FB_BPP_MASK GENMASK(7, 0) +#define DRM_FB_FW_MASK GENMASK(8, 8) /* Using the GNU statement expression extension */ #define DRM_FB_SET_OPTION(option, value) \ @@ -197,6 +198,15 @@ struct drm_fb_helper { * See also: @deferred_setup */ int preferred_bpp; + + /** +* @firmware: +* +* Set if the driver indicates to the FB helper initialization that the +* framebuffer for the device being registered is provided by firmware, +* so that it can pass this on when registering the framebuffer device. +*/ + bool firmware; }; static inline struct drm_fb_helper * -- 2.35.1
Re: [PATCH v2] drm/doc: add rfc section for small BAR uapi
On 02/05/2022 10:54, Lionel Landwerlin wrote: On 20/04/2022 20:13, Matthew Auld wrote: Add an entry for the new uapi needed for small BAR on DG2+. v2: - Some spelling fixes and other small tweaks. (Akeem & Thomas) - Rework error capture interactions, including no longer needing NEEDS_CPU_ACCESS for objects marked for capture. (Thomas) - Add probed_cpu_visible_size. (Lionel) Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Lionel Landwerlin Cc: Jon Bloomfield Cc: Daniel Vetter Cc: Jordan Justen Cc: Kenneth Graunke Cc: Akeem G Abodunrin Cc: mesa-...@lists.freedesktop.org --- Documentation/gpu/rfc/i915_small_bar.h | 190 +++ Documentation/gpu/rfc/i915_small_bar.rst | 58 +++ Documentation/gpu/rfc/index.rst | 4 + 3 files changed, 252 insertions(+) create mode 100644 Documentation/gpu/rfc/i915_small_bar.h create mode 100644 Documentation/gpu/rfc/i915_small_bar.rst diff --git a/Documentation/gpu/rfc/i915_small_bar.h b/Documentation/gpu/rfc/i915_small_bar.h new file mode 100644 index ..7bfd0cf44d35 --- /dev/null +++ b/Documentation/gpu/rfc/i915_small_bar.h @@ -0,0 +1,190 @@ +/** + * struct __drm_i915_memory_region_info - Describes one region as known to the + * driver. + * + * Note this is using both struct drm_i915_query_item and struct drm_i915_query. + * For this new query we are adding the new query id DRM_I915_QUERY_MEMORY_REGIONS + * at &drm_i915_query_item.query_id. + */ +struct __drm_i915_memory_region_info { + /** @region: The class:instance pair encoding */ + struct drm_i915_gem_memory_class_instance region; + + /** @rsvd0: MBZ */ + __u32 rsvd0; + + /** @probed_size: Memory probed by the driver (-1 = unknown) */ + __u64 probed_size; + + /** @unallocated_size: Estimate of memory remaining (-1 = unknown) */ + __u64 unallocated_size; + + union { + /** @rsvd1: MBZ */ + __u64 rsvd1[8]; + struct { + /** + * @probed_cpu_visible_size: Memory probed by the driver + * that is CPU accessible. (-1 = unknown). + * + * This will be always be <= @probed_size, and the + * remainder(if there is any) will not be CPU + * accessible. + */ + __u64 probed_cpu_visible_size; + }; Trying to implement userspace support in Vulkan for this, I have an additional question about the value of probed_cpu_visible_size. When is it set to -1? I'm guessing before there is support for this value it'll be 0 (MBZ). After after it should either be the entire lmem or something smaller. -Lionel Other pain point of this new uAPI, previously we could query the unallocated size for each heap. Now lmem is effectively divided into 2 heaps, but unallocated_size is tracking allocation from both parts of lmem. Is adding new I915_MEMORY_CLASS_DEVICE_NON_MAPPABLE out of question? -Lionel + }; +}; + +/** + * struct __drm_i915_gem_create_ext - Existing gem_create behaviour, with added + * extension support using struct i915_user_extension. + * + * Note that new buffer flags should be added here, at least for the stuff that + * is immutable. Previously we would have two ioctls, one to create the object + * with gem_create, and another to apply various parameters, however this + * creates some ambiguity for the params which are considered immutable. Also in + * general we're phasing out the various SET/GET ioctls. + */ +struct __drm_i915_gem_create_ext { + /** + * @size: Requested size for the object. + * + * The (page-aligned) allocated size for the object will be returned. + * + * Note that for some devices we have might have further minimum + * page-size restrictions(larger than 4K), like for device local-memory. + * However in general the final size here should always reflect any + * rounding up, if for example using the I915_GEM_CREATE_EXT_MEMORY_REGIONS + * extension to place the object in device local-memory. + */ + __u64 size; + /** + * @handle: Returned handle for the object. + * + * Object handles are nonzero. + */ + __u32 handle; + /** + * @flags: Optional flags. + * + * Supported values: + * + * I915_GEM_CREATE_EXT_FLAG_NEEDS_CPU_ACCESS - Signal to the kernel that + * the object will need to be accessed via the CPU. + * + * Only valid when placing objects in I915_MEMORY_CLASS_DEVICE, and + * only strictly required on platforms where only some of the device + * memory is directly visible or mappable through the CPU, like on DG2+. + * + * One of the placements MUST also be I915_MEMORY_CLASS_SYSTEM, to + * ensure we can always spill the allocation to system memory, if we + * can't place the object in the mappable part of + * I915_MEMORY_CLASS_DEVICE. + * + * Note that since the kernel only supports flat-CCS
Re: [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing
On 02/05/2022 11:43, Marijn Suijten wrote: On 2022-05-02 01:44:20, Dmitry Baryshkov wrote: [sni[ In any case, given that you've already sent this patch and another three patches [2] fixing/cleaning up the series tells me it's far from ready. Most of this should just be handled - or have been handled - in review and amended? During the review time we agreed that [2] would come as a separate change It is an API change that would make using panel-bridge easier, but isn't otherwise required. I have been working towards more logical drm_bridge/drm_bridge_connector chains employing panel-bridge and display-connector where required, [2] is a part of that effort (as well as few other patches that hit dri-devel in the last few days). I understand what is going on now. Since the DSC patches have already been queued up in the 5.19 pull I won't hurry to review them; rather will go over them when time allows me to play with the many phones here that require DSC for the screen to work. I've been told the series didn't result in positive screen output way back in its infancy, but I'll re-evaluate and send fixes or improvements if/when necessary. Sure, thank you! They work on Pixel3 (sdm845, non-active CTLs, no ping-pong binding to intf). I still didn't have time to test them on P4 (sm8150, active CTLs, PPs bound to the intf in runtime). -- With best wishes Dmitry
Re: [Freedreno] [PATCH v2] drm/msm/dsi: use RMW cycles in dsi_update_dsc_timing
On 02/05/2022 11:34, Marijn Suijten wrote: On 2022-05-01 16:56:45, Abhinav Kumar wrote: [snip] Wouln't this macro already make sure that 'reg' doesnt have anything in the top 16 bits? Its doing a & with 0x3f00 Like I said, it is unlikely that this happens, only if someone starts changing the code that assigns to `reg` which is unlikely to pass review anyway. [snip] We can have a common bitfield layout for the two channels for command mode. So we can do something like below for common fields: -static inline uint32_t DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE(uint32_t val) +static inline uint32_t DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM_DATATYPE(uint32_t val, uint32_t stream_id) { - return ((val) << DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT) & DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK; + return ((val) << (DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__SHIFT + (stream_id * 16)) & DSI_COMMAND_COMPRESSION_MODE_CTRL_STREAM0_DATATYPE__MASK; } Video mode can also use all of these except for WC as that field is not present for cmd modes. This can go as a separate change . I can push this and perhaps get a Tested-by from Vinod as I dont have a setup to re-validate this. How would you represent this in XML? I was hoping for a method that allows to construct the value in a generic way, without register names, and then simply have a "register macro" that moves (and perhaps masks) the preconstructed value into the right place. A bit like how `enum`s are currently set up in XML, but with bit ranges for the values and macros to set a value. I think I've _partially_ found what I was looking for: a ``. However, I don't know if we can utilize this multiple times within a single `reg32`, once with an offset for stream1. Alas, it's just bikeshedding at this point. Unfortunately the following naïve patch doesn't work, stream1 bits are still defined in the 0:15 bit space. One would have to modify rnn tool to make sure that it takes into account the low/high parts of the bitfield when generating offsets/masks. diff --git a/src/freedreno/registers/dsi/dsi.xml b/src/freedreno/registers/dsi/dsi.xml index f2eef4ff41ae..b0166628ad0a 100644 --- a/src/freedreno/registers/dsi/dsi.xml +++ b/src/freedreno/registers/dsi/dsi.xml @@ -361,22 +361,19 @@ xsi:schemaLocation="http://nouveau.freedesktop.org/ rules-ng.xsd"> - - + type="uint"/> type="uint"/> + + + + type="COMPRESSION_MODE_CTRL"/> - type="uint"/> - type="uint"/> - type="uint"/> - - type="uint"/> - type="uint"/> - type="uint"/> - + type="COMPRESSION_MODE_CTRL"/> + type="COMPRESSION_MODE_CTRL"/> type="uint"/> -- With best wishes Dmitry
Re: [PATCH 0/3] drm: Allow simpledrm to setup its emulated FB as firmware provided
Hi Javier Am 02.05.22 um 10:48 schrieb Javier Martinez Canillas: Hello, This series contain patches suggested by Thomas Zimmermannas a feedback for "[RFC PATCH v4 00/11] Fix some race between sysfb device registration and drivers probe" [0]. Since other changes in [0] were more controversial, I decided to just split this part in a new patch-set and revisit the rest of the patches later. Patch #1 is just a cleanup since when working on this noticed that some DRM drivers were passing as preferred bits per pixel to drm_fbdev_generic_setup() the value that is the default anyways. Patch #2 renames the 'preferred_bpp' drm_fbdev_generic_setup() parameter to 'options', and make this a multi field parameter so that it can be extended later to pass other options as well. Patch #3 finally adds the new DRM_FB_FW option and makes simpledrm to use it so that the registered framebuffer device is also marked as firmware provided. For the whole patchset: Reviewed-by: Thomas Zimmermann Thanks a lot! [0]: https://lore.kernel.org/lkml/20220429084253.1085911-1-javi...@redhat.com/ Javier Martinez Canillas (3): drm: Remove superfluous arg when calling to drm_fbdev_generic_setup() drm/fb-helper: Rename preferred_bpp drm_fbdev_generic_setup() parameter drm: Allow simpledrm to setup its emulated FB as firmware provided drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 +++-- drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- drivers/gpu/drm/arm/malidp_drv.c | 2 +- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- drivers/gpu/drm/ast/ast_drv.c | 2 +- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +- drivers/gpu/drm/drm_drv.c | 2 +- drivers/gpu/drm/drm_fb_helper.c | 25 --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +- .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +- drivers/gpu/drm/imx/dcss/dcss-kms.c | 2 +- drivers/gpu/drm/imx/imx-drm-core.c| 2 +- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- drivers/gpu/drm/mcde/mcde_drv.c | 2 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c| 2 +- drivers/gpu/drm/meson/meson_drv.c | 2 +- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +- drivers/gpu/drm/pl111/pl111_drv.c | 2 +- drivers/gpu/drm/qxl/qxl_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/sti/sti_drv.c | 2 +- drivers/gpu/drm/stm/drv.c | 2 +- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- drivers/gpu/drm/tidss/tidss_drv.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- drivers/gpu/drm/tiny/arcpgu.c | 2 +- drivers/gpu/drm/tiny/bochs.c | 2 +- drivers/gpu/drm/tiny/cirrus.c | 2 +- drivers/gpu/drm/tiny/simpledrm.c | 2 +- drivers/gpu/drm/tve200/tve200_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 2 +- drivers/gpu/drm/vc4/vc4_drv.c | 2 +- drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 +- include/drm/drm_fb_helper.h | 22 36 files changed, 80 insertions(+), 39 deletions(-) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 0/3] drm: Allow simpledrm to setup its emulated FB as firmware provided
Hello Thomas, On 5/2/22 12:35, Thomas Zimmermann wrote: > Hi Javier > > Am 02.05.22 um 10:48 schrieb Javier Martinez Canillas: >> Hello, >> >> This series contain patches suggested by Thomas Zimmermannas a feedback for Ups, I missed a space here. I meant to write "Zimmermann as a feedback..." >> "[RFC PATCH v4 00/11] Fix some race between sysfb device registration and >> drivers probe" [0]. >> >> Since other changes in [0] were more controversial, I decided to just split >> this part in a new patch-set and revisit the rest of the patches later. >> >> Patch #1 is just a cleanup since when working on this noticed that some DRM >> drivers were passing as preferred bits per pixel to drm_fbdev_generic_setup() >> the value that is the default anyways. >> >> Patch #2 renames the 'preferred_bpp' drm_fbdev_generic_setup() parameter to >> 'options', and make this a multi field parameter so that it can be extended >> later to pass other options as well. >> >> Patch #3 finally adds the new DRM_FB_FW option and makes simpledrm to use it >> so that the registered framebuffer device is also marked as firmware >> provided. > > For the whole patchset: > > Reviewed-by: Thomas Zimmermann > > Thanks a lot! > Thanks for the prompt review! -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
[PATCH v4 0/4] lrc selftest fixes
Few bug fixes for lrc selftest. v4: Gen8 don't have per engine recording of BB_OFFSET [Chris] Chris Wilson (4): drm/i915/gt: Explicitly clear BB_OFFSET for new contexts drm/i915/selftests: Check for incomplete LRI from the context image drm/i915/selftest: Always cancel semaphore on error drm/i915/selftest: Clear the output buffers before GPU writes drivers/gpu/drm/i915/gt/intel_engine_regs.h | 1 + drivers/gpu/drm/i915/gt/intel_lrc.c | 20 drivers/gpu/drm/i915/gt/selftest_lrc.c | 115 3 files changed, 116 insertions(+), 20 deletions(-) -- 2.20.1
[PATCH v4 1/4] drm/i915/gt: Explicitly clear BB_OFFSET for new contexts
From: Chris Wilson Even though the initial protocontext we load onto HW has the register cleared, by the time we save it into the default image, BB_OFFSET has had the enable bit set. Reclear BB_OFFSET for each new context. Testcase: igt/i915_selftests/gt_lrc v2: Extend it for gen8. v3: BB_OFFSET is recorded per engine from Gen9 onwards Signed-off-by: Chris Wilson Cc: Mika Kuoppala Signed-off-by: Ramalingam C Reviewed-by: Thomas Hellstrom --- drivers/gpu/drm/i915/gt/intel_engine_regs.h | 1 + drivers/gpu/drm/i915/gt/intel_lrc.c | 20 drivers/gpu/drm/i915/gt/selftest_lrc.c | 5 + 3 files changed, 26 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_regs.h b/drivers/gpu/drm/i915/gt/intel_engine_regs.h index 75a0c55c5aa5..8c65f3a7acfb 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_regs.h @@ -109,6 +109,7 @@ #define RING_SBBSTATE(base)_MMIO((base) + 0x118) /* hsw+ */ #define RING_SBBADDR_UDW(base) _MMIO((base) + 0x11c) /* gen8+ */ #define RING_BBADDR(base) _MMIO((base) + 0x140) +#define RING_BB_OFFSET(base) _MMIO((base) + 0x158) #define RING_BBADDR_UDW(base) _MMIO((base) + 0x168) /* gen8+ */ #define CCID(base) _MMIO((base) + 0x180) #define CCID_EN BIT(0) diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c index eec73c66406c..ee8ab7470a62 100644 --- a/drivers/gpu/drm/i915/gt/intel_lrc.c +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c @@ -662,6 +662,21 @@ static int lrc_ring_mi_mode(const struct intel_engine_cs *engine) return -1; } +static int lrc_ring_bb_offset(const struct intel_engine_cs *engine) +{ + if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50)) + return 0x80; + else if (GRAPHICS_VER(engine->i915) >= 12) + return 0x70; + else if (GRAPHICS_VER(engine->i915) >= 9) + return 0x64; + else if (GRAPHICS_VER(engine->i915) >= 8 && +engine->class == RENDER_CLASS) + return 0xc4; + else + return -1; +} + static int lrc_ring_gpr0(const struct intel_engine_cs *engine) { if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50)) @@ -768,6 +783,7 @@ static void init_common_regs(u32 * const regs, bool inhibit) { u32 ctl; + int loc; ctl = _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH); ctl |= _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT); @@ -779,6 +795,10 @@ static void init_common_regs(u32 * const regs, regs[CTX_CONTEXT_CONTROL] = ctl; regs[CTX_TIMESTAMP] = ce->stats.runtime.last; + + loc = lrc_ring_bb_offset(engine); + if (loc != -1) + regs[loc + 1] = 0; } static void init_wa_bb_regs(u32 * const regs, diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c index 8b2c11dbe354..c4bd4e1ac5ef 100644 --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c @@ -357,6 +357,11 @@ static int live_lrc_fixed(void *arg) lrc_ring_cmd_buf_cctl(engine), "RING_CMD_BUF_CCTL" }, + { + i915_mmio_reg_offset(RING_BB_OFFSET(engine->mmio_base)), + lrc_ring_bb_offset(engine), + "RING_BB_OFFSET" + }, { }, }, *t; u32 *hw; -- 2.20.1
[PATCH v4 4/4] drm/i915/selftest: Clear the output buffers before GPU writes
From: Chris Wilson When testing whether we can get the GPU to leak information about non-privileged state, we first need to ensure that the output buffer is set to a known value as the HW may opt to skip the write into memory for a non-privileged read of a sensitive register. We chose POISON_INUSE (0x5a) so that is both non-zero and distinct from the poison values used during the test. v2: Use i915_gem_object_pin_map_unlocked Reported-by: CQ Tang Signed-off-by: Chris Wilson Cc: CQ Tang cc: Joonas Lahtinen Signed-off-by: Ramalingam C Reviewed-by: Thomas Hellstrom --- drivers/gpu/drm/i915/gt/selftest_lrc.c | 32 ++ 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c index e4d5d74489bf..d04d08d9d92e 100644 --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c @@ -1395,6 +1395,30 @@ static int compare_isolation(struct intel_engine_cs *engine, return err; } +static struct i915_vma * +create_result_vma(struct i915_address_space *vm, unsigned long sz) +{ + struct i915_vma *vma; + void *ptr; + + vma = create_user_vma(vm, sz); + if (IS_ERR(vma)) + return vma; + + /* Set the results to a known value distinct from the poison */ + ptr = i915_gem_object_pin_map_unlocked(vma->obj, I915_MAP_WC); + if (IS_ERR(ptr)) { + i915_vma_put(vma); + return ERR_CAST(ptr); + } + + memset(ptr, POISON_INUSE, vma->size); + i915_gem_object_flush_map(vma->obj); + i915_gem_object_unpin_map(vma->obj); + + return vma; +} + static int __lrc_isolation(struct intel_engine_cs *engine, u32 poison) { u32 *sema = memset32(engine->status_page.addr + 1000, 0, 1); @@ -1413,13 +1437,13 @@ static int __lrc_isolation(struct intel_engine_cs *engine, u32 poison) goto err_A; } - ref[0] = create_user_vma(A->vm, SZ_64K); + ref[0] = create_result_vma(A->vm, SZ_64K); if (IS_ERR(ref[0])) { err = PTR_ERR(ref[0]); goto err_B; } - ref[1] = create_user_vma(A->vm, SZ_64K); + ref[1] = create_result_vma(A->vm, SZ_64K); if (IS_ERR(ref[1])) { err = PTR_ERR(ref[1]); goto err_ref0; @@ -1441,13 +1465,13 @@ static int __lrc_isolation(struct intel_engine_cs *engine, u32 poison) } i915_request_put(rq); - result[0] = create_user_vma(A->vm, SZ_64K); + result[0] = create_result_vma(A->vm, SZ_64K); if (IS_ERR(result[0])) { err = PTR_ERR(result[0]); goto err_ref1; } - result[1] = create_user_vma(A->vm, SZ_64K); + result[1] = create_result_vma(A->vm, SZ_64K); if (IS_ERR(result[1])) { err = PTR_ERR(result[1]); goto err_result0; -- 2.20.1
[PATCH v4 2/4] drm/i915/selftests: Check for incomplete LRI from the context image
From: Chris Wilson In order to keep the context image parser simple, we assume that all commands follow a similar format. A few, especially not MI commands on the render engines, have fixed lengths not encoded in a length field. This caused us to incorrectly skip over 3D state commands, and start interpreting context data as instructions. Eventually, as Daniele discovered, this would lead us to find addition LRI as part of the data and mistakenly add invalid LRI commands to the context probes. Stop parsing after we see the first !MI command, as we know we will have seen all the context registers by that point. (Mostly true for all gen so far, though the render context does have LRI after the first page that we have been ignoring so far. It would be useful to extract those as well so that we have the full list of user accessible registers.) Similarly, emit a warning if we do try to emit an invalid zero-length LRI. Reported-by: Daniele Ceraolo Spurio Signed-off-by: Chris Wilson Cc: Daniele Ceraolo Spurio Signed-off-by: Ramalingam C Acked-by: Thomas Hellstrom --- drivers/gpu/drm/i915/gt/selftest_lrc.c | 61 +++--- 1 file changed, 54 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c index c4bd4e1ac5ef..3271f01fe7db 100644 --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c @@ -27,6 +27,9 @@ #define NUM_GPR 16 #define NUM_GPR_DW (NUM_GPR * 2) /* each GPR is 2 dwords */ +#define LRI_HEADER MI_INSTR(0x22, 0) +#define LRI_LENGTH_MASK GENMASK(7, 0) + static struct i915_vma *create_scratch(struct intel_gt *gt) { return __vm_create_scratch_for_read_pinned(>->ggtt->vm, PAGE_SIZE); @@ -202,7 +205,7 @@ static int live_lrc_layout(void *arg) continue; } - if ((lri & GENMASK(31, 23)) != MI_INSTR(0x22, 0)) { + if ((lri & GENMASK(31, 23)) != LRI_HEADER) { pr_err("%s: Expected LRI command at dword %d, found %08x\n", engine->name, dw, lri); err = -EINVAL; @@ -992,18 +995,40 @@ store_context(struct intel_context *ce, struct i915_vma *scratch) hw = defaults; hw += LRC_STATE_OFFSET / sizeof(*hw); do { - u32 len = hw[dw] & 0x7f; + u32 len = hw[dw] & LRI_LENGTH_MASK; + + /* +* Keep it simple, skip parsing complex commands +* +* At present, there are no more MI_LOAD_REGISTER_IMM +* commands after the first 3D state command. Rather +* than include a table (see i915_cmd_parser.c) of all +* the possible commands and their instruction lengths +* (or mask for variable length instructions), assume +* we have gathered the complete list of registers and +* bail out. +*/ + if ((hw[dw] >> INSTR_CLIENT_SHIFT) != INSTR_MI_CLIENT) + break; if (hw[dw] == 0) { dw++; continue; } - if ((hw[dw] & GENMASK(31, 23)) != MI_INSTR(0x22, 0)) { + if ((hw[dw] & GENMASK(31, 23)) != LRI_HEADER) { + /* Assume all other MI commands match LRI length mask */ dw += len + 2; continue; } + if (!len) { + pr_err("%s: invalid LRI found in context image\n", + ce->engine->name); + igt_hexdump(defaults, PAGE_SIZE); + break; + } + dw++; len = (len + 1) / 2; while (len--) { @@ -1155,18 +1180,29 @@ static struct i915_vma *load_context(struct intel_context *ce, u32 poison) hw = defaults; hw += LRC_STATE_OFFSET / sizeof(*hw); do { - u32 len = hw[dw] & 0x7f; + u32 len = hw[dw] & LRI_LENGTH_MASK; + + /* For simplicity, break parsing at the first complex command */ + if ((hw[dw] >> INSTR_CLIENT_SHIFT) != INSTR_MI_CLIENT) + break; if (hw[dw] == 0) { dw++; continue; } - if ((hw[dw] & GENMASK(31, 23)) != MI_INSTR(0x22, 0)) { + if ((hw[dw] & GENMASK(31, 23)) != LRI_HEADER) { dw += len + 2; continue; } + if (!len) { + pr_err("%s: invalid LRI found in context image\n", + ce->engine->name); + igt_hexdump(defaults, PAGE_SIZE); +
[PATCH v4 3/4] drm/i915/selftest: Always cancel semaphore on error
From: Chris Wilson Ensure that we always signal the semaphore when timing out, so that if it happens to be stuck waiting for the semaphore we will quickly recover without having to wait for a reset. Reported-by: CQ Tang Signed-off-by: Chris Wilson Cc: CQ Tang cc: Joonas Lahtinen Signed-off-by: Ramalingam C Reviewed-by: Thomas Hellstrom --- drivers/gpu/drm/i915/gt/selftest_lrc.c | 17 - 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c b/drivers/gpu/drm/i915/gt/selftest_lrc.c index 3271f01fe7db..e4d5d74489bf 100644 --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c @@ -1460,18 +1460,17 @@ static int __lrc_isolation(struct intel_engine_cs *engine, u32 poison) } err = poison_registers(B, poison, sema); - if (err) { - WRITE_ONCE(*sema, -1); - i915_request_put(rq); - goto err_result1; - } - - if (i915_request_wait(rq, 0, HZ / 2) < 0) { - i915_request_put(rq); + if (err == 0 && i915_request_wait(rq, 0, HZ / 2) < 0) { + pr_err("%s(%s): wait for results timed out\n", + __func__, engine->name); err = -ETIME; - goto err_result1; } + + /* Always cancel the semaphore wait, just in case the GPU gets stuck */ + WRITE_ONCE(*sema, -1); i915_request_put(rq); + if (err) + goto err_result1; err = compare_isolation(engine, ref, result, A, poison); -- 2.20.1
Re: [PATCH 2/3] drm/fb-helper: Rename preferred_bpp drm_fbdev_generic_setup() parameter
Hi Javier, Thank you for the patch. On Mon, May 02, 2022 at 10:48:29AM +0200, Javier Martinez Canillas wrote: > By default the bits per pixel for the emulated framebuffer device is set > to dev->mode_config.preferred_depth, but some devices need another value. > > Since this second parameter is only used by a few drivers, and to allow > drivers to use it for passing other configurations when registering the > fbdev, rename @preferred_bpp to @options and make it a multi-field param. > > The DRM_FB_SET_OPTION() and DRM_FB_GET_OPTION() macros are provided for > drivers to set and get an option respectively. For now, only DRM_FB_BPP > option exists but other options would be added later. > > Suggested-by: Thomas Zimmermann > Signed-off-by: Javier Martinez Canillas > --- > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 -- > drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- > drivers/gpu/drm/arm/malidp_drv.c| 2 +- > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- > drivers/gpu/drm/ast/ast_drv.c | 2 +- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 2 +- > drivers/gpu/drm/drm_drv.c | 2 +- > drivers/gpu/drm/drm_fb_helper.c | 16 > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +- > drivers/gpu/drm/imx/dcss/dcss-kms.c | 2 +- > drivers/gpu/drm/imx/imx-drm-core.c | 2 +- > drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- > drivers/gpu/drm/mcde/mcde_drv.c | 2 +- > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +- > drivers/gpu/drm/meson/meson_drv.c | 2 +- > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +- > drivers/gpu/drm/pl111/pl111_drv.c | 2 +- > drivers/gpu/drm/qxl/qxl_drv.c | 2 +- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- > drivers/gpu/drm/sti/sti_drv.c | 2 +- > drivers/gpu/drm/stm/drv.c | 2 +- > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- > drivers/gpu/drm/tidss/tidss_drv.c | 2 +- > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- > drivers/gpu/drm/tiny/arcpgu.c | 2 +- > drivers/gpu/drm/tiny/bochs.c| 2 +- > drivers/gpu/drm/tve200/tve200_drv.c | 2 +- > drivers/gpu/drm/vboxvideo/vbox_drv.c| 2 +- > drivers/gpu/drm/vc4/vc4_drv.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_drv.c| 2 +- > drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 +- > include/drm/drm_fb_helper.h | 12 > 33 files changed, 58 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index b03663f42cc9..f5fae3838cdc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -2118,9 +2118,11 @@ static int amdgpu_pci_probe(struct pci_dev *pdev, > !list_empty(&adev_to_drm(adev)->mode_config.connector_list)) { > /* select 8 bpp console on low vram cards */ > if (adev->gmc.real_vram_size <= (32*1024*1024)) > - drm_fbdev_generic_setup(adev_to_drm(adev), 8); > + drm_fbdev_generic_setup(adev_to_drm(adev), > + DRM_FB_SET_OPTION(DRM_FB_BPP, > 8)); > else > - drm_fbdev_generic_setup(adev_to_drm(adev), 32); > + drm_fbdev_generic_setup(adev_to_drm(adev), > + DRM_FB_SET_OPTION(DRM_FB_BPP, > 32)); > } > > ret = amdgpu_debugfs_init(adev); > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c > index e89ae0ec60eb..636b0e989398 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > @@ -321,7 +321,7 @@ static int hdlcd_drm_bind(struct device *dev) > if (ret) > goto err_register; > > - drm_fbdev_generic_setup(drm, 32); > + drm_fbdev_generic_setup(drm, DRM_FB_SET_OPTION(DRM_FB_BPP, 32)); > > return 0; > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > b/drivers/gpu/drm/arm/malidp_drv.c > index d5aef21426cf..8f7c4f3136d5 100644 > --- a/drivers/gpu/drm/arm/malidp_drv.c > +++ b/drivers/gpu/drm/arm/malidp_drv.c > @@ -863,7 +863,7 @@ static int malidp_bind(struct device *dev) > if (ret) > goto register_fail; > > - drm_fbdev_generic_setup(drm, 32); > + drm_fbdev_generic_setup(drm, DRM_FB_SET_OPTION(DRM_FB_BPP, 32)); > > return 0; > > diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > index 7780b72de9e8..c16837fd1f8d 100644 > --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c > +++ b/d
RE: [Intel-gfx] [V2 1/3] drm/debug: Expose connector's max supported bpc via debugfs
On Fri, 29 Apr 2022, "Murthy, Arun R" wrote: >> +static int output_bpc_show(struct seq_file *m, void *data) { > > Can we have a meaningful name instead of 'm' ? > Upon changing this parameter name, you can have my > Reviewed-By: Arun R Murthy Please keep 'm'. It's by far the most common name for struct seq_file * in the kernel: $ git grep -o "struct seq_file \*[a-zA-Z0-9_]\+" | sed 's/^.*:struct seq_file \*//' | sort | uniq -c | sort -rn | head -5 2212 m 1219 seq 1126 s 135 sf 121 file BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v4 1/4] drm/i915/gt: Explicitly clear BB_OFFSET for new contexts
On 2022-05-02 at 16:40:00 +0530, Ramalingam C wrote: > From: Chris Wilson > > Even though the initial protocontext we load onto HW has the register > cleared, by the time we save it into the default image, BB_OFFSET has > had the enable bit set. Reclear BB_OFFSET for each new context. > > Testcase: igt/i915_selftests/gt_lrc > > v2: > Extend it for gen8. > v3: > BB_OFFSET is recorded per engine from Gen9 onwards > > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Signed-off-by: Ramalingam C > Reviewed-by: Thomas Hellstrom Thomas, Could you please reconfirm your R-b for v3? This R-b was given for v1. Ram > --- > drivers/gpu/drm/i915/gt/intel_engine_regs.h | 1 + > drivers/gpu/drm/i915/gt/intel_lrc.c | 20 > drivers/gpu/drm/i915/gt/selftest_lrc.c | 5 + > 3 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_regs.h > b/drivers/gpu/drm/i915/gt/intel_engine_regs.h > index 75a0c55c5aa5..8c65f3a7acfb 100644 > --- a/drivers/gpu/drm/i915/gt/intel_engine_regs.h > +++ b/drivers/gpu/drm/i915/gt/intel_engine_regs.h > @@ -109,6 +109,7 @@ > #define RING_SBBSTATE(base) _MMIO((base) + 0x118) /* hsw+ */ > #define RING_SBBADDR_UDW(base) _MMIO((base) + 0x11c) > /* gen8+ */ > #define RING_BBADDR(base)_MMIO((base) + 0x140) > +#define RING_BB_OFFSET(base) _MMIO((base) + 0x158) > #define RING_BBADDR_UDW(base)_MMIO((base) + 0x168) > /* gen8+ */ > #define CCID(base) _MMIO((base) + 0x180) > #define CCID_ENBIT(0) > diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c > b/drivers/gpu/drm/i915/gt/intel_lrc.c > index eec73c66406c..ee8ab7470a62 100644 > --- a/drivers/gpu/drm/i915/gt/intel_lrc.c > +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c > @@ -662,6 +662,21 @@ static int lrc_ring_mi_mode(const struct intel_engine_cs > *engine) > return -1; > } > > +static int lrc_ring_bb_offset(const struct intel_engine_cs *engine) > +{ > + if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50)) > + return 0x80; > + else if (GRAPHICS_VER(engine->i915) >= 12) > + return 0x70; > + else if (GRAPHICS_VER(engine->i915) >= 9) > + return 0x64; > + else if (GRAPHICS_VER(engine->i915) >= 8 && > + engine->class == RENDER_CLASS) > + return 0xc4; > + else > + return -1; > +} > + > static int lrc_ring_gpr0(const struct intel_engine_cs *engine) > { > if (GRAPHICS_VER_FULL(engine->i915) >= IP_VER(12, 50)) > @@ -768,6 +783,7 @@ static void init_common_regs(u32 * const regs, >bool inhibit) > { > u32 ctl; > + int loc; > > ctl = _MASKED_BIT_ENABLE(CTX_CTRL_INHIBIT_SYN_CTX_SWITCH); > ctl |= _MASKED_BIT_DISABLE(CTX_CTRL_ENGINE_CTX_RESTORE_INHIBIT); > @@ -779,6 +795,10 @@ static void init_common_regs(u32 * const regs, > regs[CTX_CONTEXT_CONTROL] = ctl; > > regs[CTX_TIMESTAMP] = ce->stats.runtime.last; > + > + loc = lrc_ring_bb_offset(engine); > + if (loc != -1) > + regs[loc + 1] = 0; > } > > static void init_wa_bb_regs(u32 * const regs, > diff --git a/drivers/gpu/drm/i915/gt/selftest_lrc.c > b/drivers/gpu/drm/i915/gt/selftest_lrc.c > index 8b2c11dbe354..c4bd4e1ac5ef 100644 > --- a/drivers/gpu/drm/i915/gt/selftest_lrc.c > +++ b/drivers/gpu/drm/i915/gt/selftest_lrc.c > @@ -357,6 +357,11 @@ static int live_lrc_fixed(void *arg) > lrc_ring_cmd_buf_cctl(engine), > "RING_CMD_BUF_CCTL" > }, > + { > + > i915_mmio_reg_offset(RING_BB_OFFSET(engine->mmio_base)), > + lrc_ring_bb_offset(engine), > + "RING_BB_OFFSET" > + }, > { }, > }, *t; > u32 *hw; > -- > 2.20.1 >
[PATCH 0/2] fbdev: Fix a NULL pointer dereference in fb_release()
Hello, This small series contains fixes for two bugs I found in fbmem.c, that may explain a bug reported in the rpi4 [0] when using simplefb and vc4 drivers. I was not able to reproduce the mentioned bug, but looking at the code I think that it might explain the issue. I've tested these patches in a rpi4 with both simplefb and vc4 drivers built-in and did not find any regression. [0]: https://github.com/raspberrypi/linux/issues/5011 Best regards, Javier Javier Martinez Canillas (2): fbdev: Check in file_fb_info() if the fb_info was already been freed fbdev: Make fb_release() return -ENODEV if fbdev was unregistered drivers/video/fbdev/core/fbmem.c | 14 +++--- 1 file changed, 11 insertions(+), 3 deletions(-) -- 2.35.1
[PATCH 1/2] fbdev: Check in file_fb_info() if the fb_info was already been freed
If real driver probes, the fbdev core kicks out all drivers that are using a framebuffer that were provided by the system firmware. But it could be a user-space process still has a file descriptor for the fbdev device node. This can lead to a NULL pointer dereference, if the framebuffer device is unregistered and associated data freed, but later in the .release callback is attempted to access its struct fb_info. To prevent this, make file_fb_info() to also check the fb_info reference counter and just return NULL if this equals zero. Since that means it has already been freed. Signed-off-by: Javier Martinez Canillas --- drivers/video/fbdev/core/fbmem.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 84427470367b..20d8929df79f 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -751,8 +751,13 @@ static struct fb_info *file_fb_info(struct file *file) int fbidx = iminor(inode); struct fb_info *info = registered_fb[fbidx]; - if (info != file->private_data) - info = NULL; + if (!info) + return NULL; + + /* check that the fb_info has not changed or was already freed */ + if (info != file->private_data || refcount_read(&info->count) == 0) + return NULL; + return info; } -- 2.35.1
[PATCH 2/2] fbdev: Make fb_release() return -ENODEV if fbdev was unregistered
A reference to the framebuffer device struct fb_info is stored in the file private data, but this reference could no longer be valid and must not be accessed directly. Instead, the file_fb_info() accessor function must be used since it does sanity checking to make sure that the fb_info is valid. This can happen for example if the fbdev driver was one that is using a framebuffer provided by the system firmware. In that case, the fbdev core could unregister the framebuffer device if a real video driver is probed. Reported-by: Maxime Ripard Signed-off-by: Javier Martinez Canillas --- drivers/video/fbdev/core/fbmem.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 20d8929df79f..d68097105f93 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1439,7 +1439,10 @@ fb_release(struct inode *inode, struct file *file) __acquires(&info->lock) __releases(&info->lock) { - struct fb_info * const info = file->private_data; + struct fb_info * const info = file_fb_info(file); + + if (!info) + return -ENODEV; lock_fb_info(info); if (info->fbops->fb_release) -- 2.35.1
Re: [PATCH 2/2] fbdev: Make fb_release() return -ENODEV if fbdev was unregistered
Am 02.05.22 um 15:09 schrieb Javier Martinez Canillas: A reference to the framebuffer device struct fb_info is stored in the file private data, but this reference could no longer be valid and must not be accessed directly. Instead, the file_fb_info() accessor function must be used since it does sanity checking to make sure that the fb_info is valid. This can happen for example if the fbdev driver was one that is using a framebuffer provided by the system firmware. In that case, the fbdev core could unregister the framebuffer device if a real video driver is probed. Reported-by: Maxime Ripard Signed-off-by: Javier Martinez Canillas Reviewed-by: Thomas Zimmermann This seems like the correct thing to do in any case. Thanks for the patch. Before merging, you should also add Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal") Reported-by: Junxiao Chang Best regards Thomas --- drivers/video/fbdev/core/fbmem.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 20d8929df79f..d68097105f93 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1439,7 +1439,10 @@ fb_release(struct inode *inode, struct file *file) __acquires(&info->lock) __releases(&info->lock) { - struct fb_info * const info = file->private_data; + struct fb_info * const info = file_fb_info(file); + + if (!info) + return -ENODEV; lock_fb_info(info); if (info->fbops->fb_release) -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 1/2] fbdev: Check in file_fb_info() if the fb_info was already been freed
Hi Am 02.05.22 um 15:09 schrieb Javier Martinez Canillas: If real driver probes, the fbdev core kicks out all drivers that are using a framebuffer that were provided by the system firmware. But it could be a user-space process still has a file descriptor for the fbdev device node. This can lead to a NULL pointer dereference, if the framebuffer device is unregistered and associated data freed, but later in the .release callback is attempted to access its struct fb_info. To prevent this, make file_fb_info() to also check the fb_info reference counter and just return NULL if this equals zero. Since that means it has already been freed. Signed-off-by: Javier Martinez Canillas --- drivers/video/fbdev/core/fbmem.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 84427470367b..20d8929df79f 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -751,8 +751,13 @@ static struct fb_info *file_fb_info(struct file *file) int fbidx = iminor(inode); struct fb_info *info = registered_fb[fbidx]; - if (info != file->private_data) - info = NULL; + if (!info) + return NULL; + + /* check that the fb_info has not changed or was already freed */ + if (info != file->private_data || refcount_read(&info->count) == 0) + return NULL; + Acked-by: Thomas Zimmermann However, I'm having problems with the semantics of these variables: if we have an info from registered_fb[fbinx] and the refcount in info->count is still 0, isn't that a consistency problem? If so, we should print a WARN_ON(). Best regards Thomas return info; } -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH 2/3] drm/fb-helper: Rename preferred_bpp drm_fbdev_generic_setup() parameter
Hello Laurent, On 5/2/22 13:34, Laurent Pinchart wrote: > Hi Javier, > > Thank you for the patch. > Thanks a lot for your feedback. [snip] >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -2501,8 +2501,16 @@ static const struct drm_client_funcs >> drm_fbdev_client_funcs = { >> /** >> * drm_fbdev_generic_setup() - Setup generic fbdev emulation >> * @dev: DRM device >> - * @preferred_bpp: Preferred bits per pixel for the device. >> - * @dev->mode_config.preferred_depth is used if this is >> zero. >> + * @options: options for the registered framebuffer. >> + * >> + * The @options parameter is a multi-field parameter that can contain >> + * different options for the emulated framebuffer device registered. >> + * >> + * The options must be set using DRM_FB_SET_OPTION() and obtained using >> + * DRM_FB_GET_OPTION(). The options field are the following: >> + * >> + * * DRM_FB_BPP: bits per pixel for the device. If the field is not set, >> + * @dev->mode_config.preferred_depth is used instead. > > Do I assume correctly that a driver that would need to set multiple > options would do something like > > drm_fbdev_generic_setup(dev, DRM_FB_SET_OPTION(DRM_FB_BPP, 32) | > DRM_FB_SET_OPTION(DRM_FB_FW, 1)); > That's correct, yes. > ? If so, I would rename DRM_FB_SET_OPTION() to DRM_FB_OPTION() as it's > computing the value of the option bitfield, it doesn't actually set it. > Apart from that, > Right. I'll rename it. > Reviewed-by: Laurent Pinchart > Thanks! -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [Intel-gfx] [V2 3/3] drm/amd/display: Move connector debugfs to drm
On Fri-29-04-2022 08:02 pm, Murthy, Arun R wrote: -Original Message- From: Intel-gfx On Behalf Of Bhanuprakash Modem Sent: Monday, April 11, 2022 3:21 PM To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; amd- g...@lists.freedesktop.org; jani.nik...@linux.intel.com; ville.syrj...@linux.intel.com; harry.wentl...@amd.com; Sharma, Swati2 Cc: Rodrigo Siqueira Subject: [Intel-gfx] [V2 3/3] drm/amd/display: Move connector debugfs to drm As drm_connector already have the display_info, instead of creating "output_bpc" debugfs in vendor specific driver, move the logic to the drm layer. This patch will also move "Current" bpc to the crtc debugfs from connector debugfs, since we are getting this info from crtc_state. Cc: Harry Wentland Cc: Rodrigo Siqueira Signed-off-by: Bhanuprakash Modem Reported-by: kernel test robot --- Reviewed-by: Arun R Murthy Thanks Arun, @Harry/@Rodrigo, If this change sounds good to you, can you please help to push it? - Bhanu Thanks and Regards, Arun R Murthy
Re: [PATCH 1/2] fbdev: Check in file_fb_info() if the fb_info was already been freed
Hello Thomas, On 5/2/22 15:26, Thomas Zimmermann wrote: > Hi > > Am 02.05.22 um 15:09 schrieb Javier Martinez Canillas: >> If real driver probes, the fbdev core kicks out all drivers that are using >> a framebuffer that were provided by the system firmware. But it could be a >> user-space process still has a file descriptor for the fbdev device node. >> >> This can lead to a NULL pointer dereference, if the framebuffer device is >> unregistered and associated data freed, but later in the .release callback >> is attempted to access its struct fb_info. >> >> To prevent this, make file_fb_info() to also check the fb_info reference >> counter and just return NULL if this equals zero. Since that means it has >> already been freed. >> >> Signed-off-by: Javier Martinez Canillas >> --- >> >> drivers/video/fbdev/core/fbmem.c | 9 +++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/video/fbdev/core/fbmem.c >> b/drivers/video/fbdev/core/fbmem.c >> index 84427470367b..20d8929df79f 100644 >> --- a/drivers/video/fbdev/core/fbmem.c >> +++ b/drivers/video/fbdev/core/fbmem.c >> @@ -751,8 +751,13 @@ static struct fb_info *file_fb_info(struct file *file) >> int fbidx = iminor(inode); >> struct fb_info *info = registered_fb[fbidx]; >> >> -if (info != file->private_data) >> -info = NULL; >> +if (!info) >> +return NULL; >> + >> +/* check that the fb_info has not changed or was already freed */ >> +if (info != file->private_data || refcount_read(&info->count) == 0) >> +return NULL; >> + > > Acked-by: Thomas Zimmermann > > However, I'm having problems with the semantics of these variables: if > we have an info from registered_fb[fbinx] and the refcount in > info->count is still 0, isn't that a consistency problem? If so, we > should print a WARN_ON(). > That's a good point. Maybe we are being too paranoid here? If the fb_info was set to NULL then the existing if (info != file->private_data) check will already catch that issue. In other words, now that fb_release() is getting the fb_info with the file_fb_info() function instead of file->private_data directly, the NULL pointer dereference should not happen anymore. I think that will just drop this patch, the less we touch the fbdev code the better IMO. -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
Re: [Intel-gfx] [V2 3/3] drm/amd/display: Move connector debugfs to drm
On 2022-05-02 09:28, Modem, Bhanuprakash wrote: > On Fri-29-04-2022 08:02 pm, Murthy, Arun R wrote: >> >> >>> -Original Message- >>> From: Intel-gfx On Behalf Of >>> Bhanuprakash Modem >>> Sent: Monday, April 11, 2022 3:21 PM >>> To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; >>> amd- >>> g...@lists.freedesktop.org; jani.nik...@linux.intel.com; >>> ville.syrj...@linux.intel.com; harry.wentl...@amd.com; Sharma, Swati2 >>> >>> Cc: Rodrigo Siqueira >>> Subject: [Intel-gfx] [V2 3/3] drm/amd/display: Move connector debugfs to >>> drm >>> >>> As drm_connector already have the display_info, instead of creating >>> "output_bpc" debugfs in vendor specific driver, move the logic to the >>> drm >>> layer. >>> >>> This patch will also move "Current" bpc to the crtc debugfs from >>> connector >>> debugfs, since we are getting this info from crtc_state. >>> >>> Cc: Harry Wentland >>> Cc: Rodrigo Siqueira >>> Signed-off-by: Bhanuprakash Modem >>> Reported-by: kernel test robot >>> --- >> Reviewed-by: Arun R Murthy > > Thanks Arun, > > @Harry/@Rodrigo, If this change sounds good to you, can you please help > to push it? > This changes the output_bpc debugfs behavior on amdgpu and breaks the amd_max_bpc IGT test. I don't think we should merge this as-is. This patch also seems dependent on patch 1 of the series. Shouldn't they be merged together (please don't merge them as-is, though)? Harry > - Bhanu > >> >> Thanks and Regards, >> Arun R Murthy >> >
Re: [PATCH 2/2] fbdev: Make fb_release() return -ENODEV if fbdev was unregistered
Hello Thomas, On 5/2/22 15:20, Thomas Zimmermann wrote: > > > Am 02.05.22 um 15:09 schrieb Javier Martinez Canillas: >> A reference to the framebuffer device struct fb_info is stored in the file >> private data, but this reference could no longer be valid and must not be >> accessed directly. Instead, the file_fb_info() accessor function must be >> used since it does sanity checking to make sure that the fb_info is valid. >> >> This can happen for example if the fbdev driver was one that is using a >> framebuffer provided by the system firmware. In that case, the fbdev core >> could unregister the framebuffer device if a real video driver is probed. >> >> Reported-by: Maxime Ripard >> Signed-off-by: Javier Martinez Canillas > > Reviewed-by: Thomas Zimmermann > Thanks. > This seems like the correct thing to do in any case. Thanks for the Agreed, it's certainly a bug if not the same that was already reported. > patch. Before merging, you should also add > > Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced > removal") I thought about that but I don't think that's accurate since the bug is not related to that commit. That might make easier to reproduce it but is something that would happen anyway if for example someone attempted to remove a module or unbind the device using the sysfs entries. Maybe I can comment in the commit message that this change made it more likely to occur and for that reason I'm adding a fixes tag. > Reported-by: Junxiao Chang > Indeed. -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
[PATCH v2] fbdev: Make fb_release() return -ENODEV if fbdev was unregistered
A reference to the framebuffer device struct fb_info is stored in the file private data, but this reference could no longer be valid and must not be accessed directly. Instead, the file_fb_info() accessor function must be used since it does sanity checking to make sure that the fb_info is valid. This can happen for example if the registered framebuffer device is for a driver that just uses a framebuffer provided by the system firmware. In that case, the fbdev core would unregister the framebuffer device when a real video driver is probed and ask to remove conflicting framebuffers. The bug has been present for a long time but commit 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal") unmasked it since the fbdev core started unregistering the framebuffers' devices associated. Fixes: 27599aacbaef ("fbdev: Hot-unplug firmware fb devices on forced removal") Reported-by: Maxime Ripard Reported-by: Junxiao Chang Signed-off-by: Javier Martinez Canillas Reviewed-by: Thomas Zimmermann --- Changes in v2: - Drop patch 1/2 since patch 2/2 should be enough to fix the issue. - Add missing Fixes and Reported-by tags (Thomas Zimmermann). - Add Thomas Zimmermann's Reviewed-by tag. drivers/video/fbdev/core/fbmem.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c index 84427470367b..82d4318ba8f7 100644 --- a/drivers/video/fbdev/core/fbmem.c +++ b/drivers/video/fbdev/core/fbmem.c @@ -1434,7 +1434,10 @@ fb_release(struct inode *inode, struct file *file) __acquires(&info->lock) __releases(&info->lock) { - struct fb_info * const info = file->private_data; + struct fb_info * const info = file_fb_info(file); + + if (!info) + return -ENODEV; lock_fb_info(info); if (info->fbops->fb_release) -- 2.35.1
[PATCH 25/37] video: handle HAS_IOPORT dependencies
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends not being declared. We thus need to add HAS_IOPORT as dependency for those drivers using them and guard inline code in headers. Co-developed-by: Arnd Bergmann Signed-off-by: Niklas Schnelle --- drivers/video/fbdev/Kconfig | 25 + include/video/vga.h | 8 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 93b8d84c34cf..e7d27c0602d5 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -343,7 +343,7 @@ config FB_IMX config FB_CYBER2000 tristate "CyberPro 2000/2010/5000 support" - depends on FB && PCI && (BROKEN || !SPARC64) + depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64) select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -436,6 +436,7 @@ config FB_FM2 config FB_ARC tristate "Arc Monochrome LCD board support" depends on FB && (X86 || COMPILE_TEST) + depends on HAS_IOPORT select FB_SYS_FILLRECT select FB_SYS_COPYAREA select FB_SYS_IMAGEBLIT @@ -1242,7 +1243,7 @@ config FB_RADEON_DEBUG config FB_ATY128 tristate "ATI Rage128 display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1265,7 +1266,7 @@ config FB_ATY128_BACKLIGHT config FB_ATY tristate "ATI Mach64 display support" if PCI || ATARI - depends on FB && !SPARC32 + depends on FB && HAS_IOPORT && !SPARC32 select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1315,7 +1316,7 @@ config FB_ATY_BACKLIGHT config FB_S3 tristate "S3 Trio/Virge support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1374,7 +1375,7 @@ config FB_SAVAGE_ACCEL config FB_SIS tristate "SiS/XGI display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1404,7 +1405,7 @@ config FB_SIS_315 config FB_VIA tristate "VIA UniChrome (Pro) and Chrome9 display support" - depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST) + depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || COMPILE_TEST) select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1442,7 +1443,7 @@ endif config FB_NEOMAGIC tristate "NeoMagic display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_MODE_HELPERS select FB_CFB_FILLRECT select FB_CFB_COPYAREA @@ -1470,7 +1471,7 @@ config FB_KYRO config FB_3DFX tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_IMAGEBLIT select FB_CFB_FILLRECT select FB_CFB_COPYAREA @@ -1518,7 +1519,7 @@ config FB_VOODOO1 config FB_VT8623 tristate "VIA VT8623 support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1532,7 +1533,7 @@ config FB_VT8623 config FB_TRIDENT tristate "Trident/CyberXXX/CyberBlade support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1554,7 +1555,7 @@ config FB_TRIDENT config FB_ARK tristate "ARK 2000PV support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -2226,7 +2227,7 @@ config FB_SSD1307 config FB_SM712 tristate "Silicon Motion SM712 framebuffer support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT diff --git a/include/video/vga.h b/include/video/vga.h index d334e64c1c19..53cb52c0fddb 100644 --- a/include/video/vga.h +++ b/include/video/vga.h @@ -201,18 +201,26 @@ extern int restore_vga(struct vgastate *state); static inline unsigned char vga_io_r (unsigned short port) { +#ifdef CONFIG_HAS_IOPORT return inb_p(port); +#else + return 0xff; +#endif } static inline void vga_io_w (unsigned short port, unsigned char val) { +#ifdef CONFIG_HAS_IOPORT outb_p(val, port); +#endif } static inline void vga_io_w_fast (unsigned short port, unsigned char reg, unsigned char val) { +#ifdef CONFIG_HAS_I
Re: [PATCH v2 14/19] xen/drmfront: use xenbus_setup_ring() and xenbus_teardown_ring()
On 28.04.22 11:27, Juergen Gross wrote: Hello Juergen, all Simplify drmfront's ring creation and removal via xenbus_setup_ring() and xenbus_teardown_ring(). Signed-off-by: Juergen Gross I am not familiar with DRM bits of this driver, but a little bit familiar with Xen bits this patch only touches and I have environment to test. Xen specific changes looks good to me. Also I didn't see any specific to this series issues when testing virtulized display driver except one I have already pointed out in PATCH v2 08/19. root@salvator-x-h3-4x2g-xt-domu:~# dmesg | grep drm [ 0.158190] [drm] Registering XEN PV vdispl [ 0.159620] [drm] Connector device/vdispl/0/0: resolution 1920x1080 [ 0.159888] [drm] Have 1 connector(s) [ 0.289069] [drm] Creating Xen PV DRM Display Unit [ 0.289873] [drm] Initialized xendrm-du 1.0.0 20180221 for vdispl-0 on minor 0 [ 0.289918] [drm] Initialized xendrm-du 1.0.0 20180221 on minor 0 root@generic-armv8-xt-dom0:~# xenstore-ls -f | grep vdispl /local/domain/1/backend/vdispl = "" /local/domain/1/backend/vdispl/2 = "" /local/domain/1/backend/vdispl/2/0 = "" /local/domain/1/backend/vdispl/2/0/frontend = "/local/domain/2/device/vdispl/0" /local/domain/1/backend/vdispl/2/0/frontend-id = "2" /local/domain/1/backend/vdispl/2/0/online = "1" /local/domain/1/backend/vdispl/2/0/state = "4" /local/domain/2/device/vdispl = "" /local/domain/2/device/vdispl/0 = "" /local/domain/2/device/vdispl/0/backend = "/local/domain/1/backend/vdispl/2/0" /local/domain/2/device/vdispl/0/backend-id = "1" /local/domain/2/device/vdispl/0/state = "4" /local/domain/2/device/vdispl/0/be-alloc = "0" /local/domain/2/device/vdispl/0/0 = "" /local/domain/2/device/vdispl/0/0/resolution = "1920x1080" /local/domain/2/device/vdispl/0/0/unique-id = "HDMI-A-1" /local/domain/2/device/vdispl/0/0/req-ring-ref = "8" /local/domain/2/device/vdispl/0/0/req-event-channel = "7" /local/domain/2/device/vdispl/0/0/evt-ring-ref = "9" /local/domain/2/device/vdispl/0/0/evt-event-channel = "8" /libxl/2/device/vdispl = "" /libxl/2/device/vdispl/0 = "" /libxl/2/device/vdispl/0/frontend = "/local/domain/2/device/vdispl/0" /libxl/2/device/vdispl/0/backend = "/local/domain/1/backend/vdispl/2/0" /libxl/2/device/vdispl/0/frontend-id = "2" /libxl/2/device/vdispl/0/online = "1" /libxl/2/device/vdispl/0/state = "1" It worth mentioning I noticed one issue, but I believe it is not related to your series. root@salvator-x-h3-4x2g-xt-domu:~# modetest -M xendrm-du -s 31:1920x1080 [ 62.431887] [ cut here ] [ 62.431940] WARNING: CPU: 0 PID: 244 at drivers/gpu/drm/drm_gem.c:1055 drm_gem_mmap_obj+0x16c/0x180 [ 62.432000] Modules linked in: [ 62.432025] CPU: 0 PID: 244 Comm: modetest Tainted: G W 5.18.0-rc4-yocto-standard-00096-g936342d8fae2 #1 [ 62.432067] Hardware name: XENVM-4.17 (DT) [ 62.432089] pstate: 6005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 62.432126] pc : drm_gem_mmap_obj+0x16c/0x180 [ 62.432153] lr : drm_gem_mmap_obj+0x78/0x180 [ 62.432178] sp : 89da3bb0 [ 62.432196] x29: 89da3bb0 x28: 0008 x27: 0001c3a56780 [ 62.432237] x26: 0001c3a56f00 x25: 07e9 x24: 0001c23dec00 [ 62.432279] x23: 0001c0c98000 x22: 0001c2162b80 x21: [ 62.432320] x20: 0001c3a56780 x19: 0001c23dea00 x18: 0001 [ 62.432355] x17: x16: x15: 0003603c [ 62.432394] x14: x13: x12: [ 62.432430] x11: 0010 x10: 88071000 x9 : 0001c0f17e70 [ 62.432470] x8 : 8001f65ce000 x7 : 0001 x6 : 0001c388a000 [ 62.432505] x5 : 89da3a10 x4 : 0090 x3 : 10046400 [ 62.432539] x2 : 07e9 x1 : 9b0023a536f4f400 x0 : 10fb [ 62.432579] Call trace: [ 62.432593] drm_gem_mmap_obj+0x16c/0x180 [ 62.432617] drm_gem_mmap+0x128/0x228 [ 62.432641] mmap_region+0x384/0x5a0 [ 62.432667] do_mmap+0x354/0x4f0 [ 62.432687] vm_mmap_pgoff+0xdc/0x108 [ 62.432710] ksys_mmap_pgoff+0x1b8/0x208 [ 62.432734] __arm64_sys_mmap+0x30/0x48 [ 62.432760] invoke_syscall+0x44/0x108 [ 62.432783] el0_svc_common.constprop.0+0xcc/0xf0 [ 62.432811] do_el0_svc+0x24/0x88 [ 62.432831] el0_svc+0x2c/0x88 [ 62.432855] el0t_64_sync_handler+0xb0/0xb8 [ 62.432875] el0t_64_sync+0x18c/0x190 [ 62.432898] ---[ end trace ]--- setting mode 1920x1080-60.00Hz@XR24 on connectors 31, crtc 34 Although we see that WARNING, the application still works. Looking into the code, I assume that problem is that frontend doesn't set the VM_DONTEXPAND flag in mmap callback. This diff fixes an issue in my environment: index 5a5bf4e..e31554d 100644 --- a/drivers/gpu/drm/xen/xen_drm_front_gem.c +++ b/drivers/gpu/drm/xen/xen_drm_front_gem.c @@ -71,7 +71,7 @@ static int xen_drm_front_gem_object_mmap(st
[RFC v2 08/39] drm: handle HAS_IOPORT dependencies
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends not being declared. We thus need to add HAS_IOPORT as dependency for those drivers using them. In the bochs driver there is optional MMIO support detected at runtime, warn if this isn't taken when HAS_IOPORT is not defined. There is also a direct and hard coded use in cirrus.c which according to the comment is only necessary during resume. Let's just skip this as for example s390 which doesn't have I/O port support also doesen't support suspend/resume. Co-developed-by: Arnd Bergmann Signed-off-by: Niklas Schnelle --- drivers/gpu/drm/qxl/Kconfig | 1 + drivers/gpu/drm/tiny/bochs.c | 19 +++ drivers/gpu/drm/tiny/cirrus.c | 2 ++ 3 files changed, 22 insertions(+) diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig index ca3f51c2a8fe..d0e0d440c8d9 100644 --- a/drivers/gpu/drm/qxl/Kconfig +++ b/drivers/gpu/drm/qxl/Kconfig @@ -2,6 +2,7 @@ config DRM_QXL tristate "QXL virtual GPU" depends on DRM && PCI && MMU + depends on HAS_IOPORT select DRM_KMS_HELPER select DRM_TTM select DRM_TTM_HELPER diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c index ed971c8bb446..9acc726d99ec 100644 --- a/drivers/gpu/drm/tiny/bochs.c +++ b/drivers/gpu/drm/tiny/bochs.c @@ -1,5 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-or-later +#include #include #include @@ -102,7 +103,11 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val) writeb(val, bochs->mmio + offset); } else { +#ifdef HAS_IOPORT outb(val, ioport); +#else + WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT"); +#endif } } @@ -116,7 +121,12 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 ioport) return readb(bochs->mmio + offset); } else { +#ifdef HAS_IOPORT return inb(ioport); +#else + WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT"); + return 0xff; +#endif } } @@ -129,8 +139,13 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, u16 reg) ret = readw(bochs->mmio + offset); } else { +#ifdef HAS_IOPORT outw(reg, VBE_DISPI_IOPORT_INDEX); ret = inw(VBE_DISPI_IOPORT_DATA); +#else + WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT"); + ret = 0x; +#endif } return ret; } @@ -142,8 +157,12 @@ static void bochs_dispi_write(struct bochs_device *bochs, u16 reg, u16 val) writew(val, bochs->mmio + offset); } else { +#ifdef HAS_IOPORT outw(reg, VBE_DISPI_IOPORT_INDEX); outw(val, VBE_DISPI_IOPORT_DATA); +#else + WARN_ONCE(1, "Non-MMIO bochs device needs HAS_IOPORT"); +#endif } } diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c index c8e791840862..0dc4788c5399 100644 --- a/drivers/gpu/drm/tiny/cirrus.c +++ b/drivers/gpu/drm/tiny/cirrus.c @@ -306,8 +306,10 @@ static int cirrus_mode_set(struct cirrus_device *cirrus, cirrus_set_start_address(cirrus, 0); +#ifdef CONFIG_HAS_IOPORT /* Unblank (needed on S3 resume, vgabios doesn't do it then) */ outb(0x20, 0x3c0); +#endif drm_dev_exit(idx); return 0; -- 2.32.0
[PATCH] gpu/drm/radeon: Fix spelling typo in comments
Fix spelling typo in comments. Signed-off-by: pengfuyuan --- drivers/gpu/drm/radeon/atombios.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/radeon/atombios.h b/drivers/gpu/drm/radeon/atombios.h index bd5dc09e860f..da35a970fcc0 100644 --- a/drivers/gpu/drm/radeon/atombios.h +++ b/drivers/gpu/drm/radeon/atombios.h @@ -3599,7 +3599,7 @@ typedef struct _ATOM_LCD_RTS_RECORD UCHAR ucRTSValue; }ATOM_LCD_RTS_RECORD; -//!! If the record below exits, it shoud always be the first record for easy use in command table!!! +//!! If the record below exists, it should always be the first record for easy use in command table!!! // The record below is only used when LVDS_Info is present. From ATOM_LVDS_INFO_V12, use ucLCDPanel_SpecialHandlingCap instead. typedef struct _ATOM_LCD_MODE_CONTROL_CAP { @@ -3823,7 +3823,7 @@ typedef struct _ATOM_DPCD_INFO // Note1: This table is filled by SetBiosReservationStartInFB in CoreCommSubs.asm //at running time. // note2: From RV770, the memory is more than 32bit addressable, so we will change -//ucTableFormatRevision=1,ucTableContentRevision=4, the strcuture remains +//ucTableFormatRevision=1,ucTableContentRevision=4, the structure remains //exactly same as 1.1 and 1.2 (1.3 is never in use), but ulStartAddrUsedByFirmware //(in offset to start of memory address) is KB aligned instead of byte aligend. /***/ @@ -3858,7 +3858,7 @@ typedef struct _ATOM_VRAM_USAGE_BY_FIRMWARE ATOM_FIRMWARE_VRAM_RESERVE_INFO asFirmwareVramReserveInfo[ATOM_MAX_FIRMWARE_VRAM_USAGE_INFO]; }ATOM_VRAM_USAGE_BY_FIRMWARE; -// change verion to 1.5, when allow driver to allocate the vram area for command table access. +// change version to 1.5, when allow driver to allocate the vram area for command table access. typedef struct _ATOM_FIRMWARE_VRAM_RESERVE_INFO_V1_5 { ULONG ulStartAddrUsedByFirmware; @@ -5973,7 +5973,7 @@ typedef struct _ATOM_ASIC_INTERNAL_SS_INFO_V3 #define CLEAR_ATOM_S7_DOS_8BIT_DAC_EN ((ATOM_DOS_MODE_INFO_DEF << 8 )|ATOM_S7_DOS_8BIT_DAC_EN_SHIFT | ATOM_FLAG_CLEAR ) // -//Portion II: Definitinos only used in Driver +//Portion II: Definitions only used in Driver // // Macros used by driver @@ -7162,7 +7162,7 @@ typedef struct _DP_ENCODER_SERVICE_PARAMETERS // ucAction #define ATOM_DP_ACTION_GET_SINK_TYPE 0x01 -/* obselete */ +/* obsolete */ #define ATOM_DP_ACTION_TRAINING_START 0x02 #define ATOM_DP_ACTION_TRAINING_COMPLETE 0x03 #define ATOM_DP_ACTION_TRAINING_PATTERN_SEL0x04 -- 2.25.1
Re: [PATCH] dma-buf: add the name field to the table header
Hi Christian, On Thu, 28 Apr 2022 at 13:33, Christian König wrote: > > Am 28.04.22 um 08:39 schrieb Yuanzheng Song: > > 'cat /sys/kernel/debug/dma_buf/bufinfo' will print the Dma-buf > > Objects' information when the CONFIG_DEBUG_FS=y. > > However, the printed table header information does not contain > > the name field. So we need to add the name field to the table > > header and use the '' to replace the empty buf_obj->name. > > > > Signed-off-by: Yuanzheng Song > > Reviewed-by: Christian König > > Sumit do you want to push this or should I go ahead? No worries, I can push it out. Thanks :) > > > --- > > drivers/dma-buf/dma-buf.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c > > index 79795857be3e..a2f9a1815e38 100644 > > --- a/drivers/dma-buf/dma-buf.c > > +++ b/drivers/dma-buf/dma-buf.c > > @@ -1351,7 +1351,7 @@ static int dma_buf_debug_show(struct seq_file *s, > > void *unused) > > return ret; > > > > seq_puts(s, "\nDma-buf Objects:\n"); > > - seq_printf(s, "%-8s\t%-8s\t%-8s\t%-8s\texp_name\t%-8s\n", > > + seq_printf(s, "%-8s\t%-8s\t%-8s\t%-8s\texp_name\t%-8s\tname\n", > > "size", "flags", "mode", "count", "ino"); > > > > list_for_each_entry(buf_obj, &db_list.head, list_node) { > > @@ -1368,7 +1368,7 @@ static int dma_buf_debug_show(struct seq_file *s, > > void *unused) > > file_count(buf_obj->file), > > buf_obj->exp_name, > > file_inode(buf_obj->file)->i_ino, > > - buf_obj->name ?: ""); > > + buf_obj->name ?: ""); > > spin_unlock(&buf_obj->name_lock); > > > > dma_resv_describe(buf_obj->resv, s); > Best, Sumit.
Re: [PATCH 31/37] drm: handle HAS_IOPORT dependencies
On Fri, 2022-04-29 at 15:50 +0200, Niklas Schnelle wrote: > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > not being declared. We thus need to add HAS_IOPORT as dependency for > those drivers using them. There is also a direct and hard coded use in > cirrus.c which according to the comment is only necessary during resume. > Let's just skip this as for example s390 which doesn't have I/O port > support also doesen't support suspend/resume. > > Co-developed-by: Arnd Bergmann > Signed-off-by: Niklas Schnelle > --- Sorry everyone. I sent this as PATCH in error while preparing to sent the same series as RFC. Since e-mail has no remote delete and I lack a time machine let's just all pretend you only got the RFC.
Re: [PATCH 25/37] video: handle HAS_IOPORT dependencies
On Fri, 2022-04-29 at 15:50 +0200, Niklas Schnelle wrote: > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > not being declared. We thus need to add HAS_IOPORT as dependency for > those drivers using them and guard inline code in headers. > > Co-developed-by: Arnd Bergmann > Signed-off-by: Niklas Schnelle > --- Sorry everyone. I sent this as PATCH in error while preparing to sent the same series as RFC. Since e-mail has no remote delete and I lack a time machine let's just all pretend you only got the RFC.
[RFC v2 36/39] video: handle HAS_IOPORT dependencies
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends not being declared. We thus need to add HAS_IOPORT as dependency for those drivers using them and guard inline code in headers. Co-developed-by: Arnd Bergmann Signed-off-by: Niklas Schnelle --- drivers/video/fbdev/Kconfig | 25 + include/video/vga.h | 8 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig index 93b8d84c34cf..e7d27c0602d5 100644 --- a/drivers/video/fbdev/Kconfig +++ b/drivers/video/fbdev/Kconfig @@ -343,7 +343,7 @@ config FB_IMX config FB_CYBER2000 tristate "CyberPro 2000/2010/5000 support" - depends on FB && PCI && (BROKEN || !SPARC64) + depends on FB && PCI && HAS_IOPORT && (BROKEN || !SPARC64) select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -436,6 +436,7 @@ config FB_FM2 config FB_ARC tristate "Arc Monochrome LCD board support" depends on FB && (X86 || COMPILE_TEST) + depends on HAS_IOPORT select FB_SYS_FILLRECT select FB_SYS_COPYAREA select FB_SYS_IMAGEBLIT @@ -1242,7 +1243,7 @@ config FB_RADEON_DEBUG config FB_ATY128 tristate "ATI Rage128 display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1265,7 +1266,7 @@ config FB_ATY128_BACKLIGHT config FB_ATY tristate "ATI Mach64 display support" if PCI || ATARI - depends on FB && !SPARC32 + depends on FB && HAS_IOPORT && !SPARC32 select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1315,7 +1316,7 @@ config FB_ATY_BACKLIGHT config FB_S3 tristate "S3 Trio/Virge support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1374,7 +1375,7 @@ config FB_SAVAGE_ACCEL config FB_SIS tristate "SiS/XGI display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1404,7 +1405,7 @@ config FB_SIS_315 config FB_VIA tristate "VIA UniChrome (Pro) and Chrome9 display support" - depends on FB && PCI && GPIOLIB && I2C && (X86 || COMPILE_TEST) + depends on FB && PCI && GPIOLIB && I2C && HAS_IOPORT && (X86 || COMPILE_TEST) select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1442,7 +1443,7 @@ endif config FB_NEOMAGIC tristate "NeoMagic display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_MODE_HELPERS select FB_CFB_FILLRECT select FB_CFB_COPYAREA @@ -1470,7 +1471,7 @@ config FB_KYRO config FB_3DFX tristate "3Dfx Banshee/Voodoo3/Voodoo5 display support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_IMAGEBLIT select FB_CFB_FILLRECT select FB_CFB_COPYAREA @@ -1518,7 +1519,7 @@ config FB_VOODOO1 config FB_VT8623 tristate "VIA VT8623 support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1532,7 +1533,7 @@ config FB_VT8623 config FB_TRIDENT tristate "Trident/CyberXXX/CyberBlade support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -1554,7 +1555,7 @@ config FB_TRIDENT config FB_ARK tristate "ARK 2000PV support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT @@ -2226,7 +2227,7 @@ config FB_SSD1307 config FB_SM712 tristate "Silicon Motion SM712 framebuffer support" - depends on FB && PCI + depends on FB && PCI && HAS_IOPORT select FB_CFB_FILLRECT select FB_CFB_COPYAREA select FB_CFB_IMAGEBLIT diff --git a/include/video/vga.h b/include/video/vga.h index d334e64c1c19..53cb52c0fddb 100644 --- a/include/video/vga.h +++ b/include/video/vga.h @@ -201,18 +201,26 @@ extern int restore_vga(struct vgastate *state); static inline unsigned char vga_io_r (unsigned short port) { +#ifdef CONFIG_HAS_IOPORT return inb_p(port); +#else + return 0xff; +#endif } static inline void vga_io_w (unsigned short port, unsigned char val) { +#ifdef CONFIG_HAS_IOPORT outb_p(val, port); +#endif } static inline void vga_io_w_fast (unsigned short port, unsigned char reg, unsigned char val) { +#ifdef CONFIG_HAS_I
Re: [PATCH] MAINTAINERS: add Melissa to V3D maintainers
On Fri, 2022-04-29 at 18:33 -0100, Melissa Wen wrote: > I've been contributing to v3d through improvements, reviews, testing, > debugging, etc. So, I'm adding myself as a co-maintainer of the V3D > driver. > Acked-by: Juan A. Suarez
[PATCH 31/37] drm: handle HAS_IOPORT dependencies
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends not being declared. We thus need to add HAS_IOPORT as dependency for those drivers using them. There is also a direct and hard coded use in cirrus.c which according to the comment is only necessary during resume. Let's just skip this as for example s390 which doesn't have I/O port support also doesen't support suspend/resume. Co-developed-by: Arnd Bergmann Signed-off-by: Niklas Schnelle --- drivers/gpu/drm/qxl/Kconfig | 1 + drivers/gpu/drm/tiny/Kconfig | 1 + drivers/gpu/drm/tiny/cirrus.c | 2 ++ 3 files changed, 4 insertions(+) diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig index ca3f51c2a8fe..d0e0d440c8d9 100644 --- a/drivers/gpu/drm/qxl/Kconfig +++ b/drivers/gpu/drm/qxl/Kconfig @@ -2,6 +2,7 @@ config DRM_QXL tristate "QXL virtual GPU" depends on DRM && PCI && MMU + depends on HAS_IOPORT select DRM_KMS_HELPER select DRM_TTM select DRM_TTM_HELPER diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig index 627d637a1e7e..7102783809c3 100644 --- a/drivers/gpu/drm/tiny/Kconfig +++ b/drivers/gpu/drm/tiny/Kconfig @@ -13,6 +13,7 @@ config DRM_ARCPGU config DRM_BOCHS tristate "DRM Support for bochs dispi vga interface (qemu stdvga)" depends on DRM && PCI && MMU + depends on HAS_IOPORT select DRM_KMS_HELPER select DRM_VRAM_HELPER select DRM_TTM diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c index c8e791840862..0dc4788c5399 100644 --- a/drivers/gpu/drm/tiny/cirrus.c +++ b/drivers/gpu/drm/tiny/cirrus.c @@ -306,8 +306,10 @@ static int cirrus_mode_set(struct cirrus_device *cirrus, cirrus_set_start_address(cirrus, 0); +#ifdef CONFIG_HAS_IOPORT /* Unblank (needed on S3 resume, vgabios doesn't do it then) */ outb(0x20, 0x3c0); +#endif drm_dev_exit(idx); return 0; -- 2.32.0
[PATCH v3] uapi/drm/i915: Document memory residency and Flat-CCS capability of obj
Capture the impact of memory region preference list of the objects, on their memory residency and Flat-CCS capability. v2: Fix the Flat-CCS capability of an obj with {lmem, smem} preference list [Thomas] v3: Reworded the doc [Matt] Signed-off-by: Ramalingam C cc: Matthew Auld cc: Thomas Hellstrom cc: Daniel Vetter cc: Jon Bloomfield cc: Lionel Landwerlin cc: Kenneth Graunke cc: mesa-...@lists.freedesktop.org cc: Jordan Justen cc: Tony Ye Reviewed-by: Matthew Auld --- include/uapi/drm/i915_drm.h | 16 1 file changed, 16 insertions(+) diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index a2def7b27009..b7e1c2fe08dc 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -3443,6 +3443,22 @@ struct drm_i915_gem_create_ext { * At which point we get the object handle in &drm_i915_gem_create_ext.handle, * along with the final object size in &drm_i915_gem_create_ext.size, which * should account for any rounding up, if required. + * + * Note that userspace has no means of knowing the current backing region + * for objects where @num_regions is larger than one. The kernel will only + * ensure that the priority order of the @regions array is honoured, either + * when initially placing the object, or when moving memory around due to + * memory pressure + * + * On Flat-CCS capable HW, compression is supported for the objects residing + * in I915_MEMORY_CLASS_DEVICE. When such objects (compressed) has other + * memory class in @regions and migrated (by I915, due to memory + * constrain) to the non I915_MEMORY_CLASS_DEVICE region, then I915 needs to + * decompress the content. But I915 dosen't have the required information to + * decompress the userspace compressed objects. + * + * So I915 supports Flat-CCS, only on the objects which can reside only on + * I915_MEMORY_CLASS_DEVICE regions. */ struct drm_i915_gem_create_ext_memory_regions { /** @base: Extension link. See struct i915_user_extension. */ -- 2.20.1
[PATCH v3 0/3] Flat-CCS eviction enhancements
Flat-CCS eviction enhancements v3: Incorporated the review suggestions [Matt] Ramalingam C (3): drm/i915/gt: BUG_ON unexpected NULL at scatterlist walking drm/i915/gt: optimize the ccs_sz calculation per chunk drm/i915/gt: Document the eviction of the Flat-CCS objects drivers/gpu/drm/i915/gt/intel_migrate.c | 73 ++--- 1 file changed, 40 insertions(+), 33 deletions(-) -- 2.20.1
[PATCH v3 2/3] drm/i915/gt: optimize the ccs_sz calculation per chunk
Calculate the ccs_sz that needs to be emitted based on the src and dst pages emitted per chunk. And handle the return value of emit_pte for the ccs pages. v2: ccs_sz moved to the reduced scope [Matt] Signed-off-by: Ramalingam C Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/gt/intel_migrate.c | 36 + 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c index 168d17b6f48a..fc6975e55fae 100644 --- a/drivers/gpu/drm/i915/gt/intel_migrate.c +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c @@ -647,17 +647,9 @@ static int scatter_list_length(struct scatterlist *sg) static void calculate_chunk_sz(struct drm_i915_private *i915, bool src_is_lmem, - int *src_sz, int *ccs_sz, u32 bytes_to_cpy, - u32 ccs_bytes_to_cpy) + int *src_sz, u32 bytes_to_cpy, u32 ccs_bytes_to_cpy) { if (ccs_bytes_to_cpy) { - /* -* We can only copy the ccs data corresponding to -* the CHUNK_SZ of lmem which is -* GET_CCS_BYTES(i915, CHUNK_SZ)) -*/ - *ccs_sz = min_t(int, ccs_bytes_to_cpy, GET_CCS_BYTES(i915, CHUNK_SZ)); - if (!src_is_lmem) /* * When CHUNK_SZ is passed all the pages upto CHUNK_SZ @@ -717,10 +709,10 @@ intel_context_migrate_copy(struct intel_context *ce, struct drm_i915_private *i915 = ce->engine->i915; u32 ccs_bytes_to_cpy = 0, bytes_to_cpy; enum i915_cache_level ccs_cache_level; - int src_sz, dst_sz, ccs_sz; u32 src_offset, dst_offset; u8 src_access, dst_access; struct i915_request *rq; + int src_sz, dst_sz; bool ccs_is_src; int err; @@ -803,7 +795,7 @@ intel_context_migrate_copy(struct intel_context *ce, if (err) goto out_rq; - calculate_chunk_sz(i915, src_is_lmem, &src_sz, &ccs_sz, + calculate_chunk_sz(i915, src_is_lmem, &src_sz, bytes_to_cpy, ccs_bytes_to_cpy); len = emit_pte(rq, &it_src, src_cache_level, src_is_lmem, @@ -837,37 +829,35 @@ intel_context_migrate_copy(struct intel_context *ce, bytes_to_cpy -= len; if (ccs_bytes_to_cpy) { + int ccs_sz; + err = rq->engine->emit_flush(rq, EMIT_INVALIDATE); if (err) goto out_rq; + ccs_sz = GET_CCS_BYTES(i915, len); err = emit_pte(rq, &it_ccs, ccs_cache_level, false, ccs_is_src ? src_offset : dst_offset, ccs_sz); + if (err < 0) + goto out_rq; + if (err < ccs_sz) { + err = -EINVAL; + goto out_rq; + } err = rq->engine->emit_flush(rq, EMIT_INVALIDATE); if (err) goto out_rq; - /* -* Using max of src_sz and dst_sz, as we need to -* pass the lmem size corresponding to the ccs -* blocks we need to handle. -*/ - ccs_sz = max_t(int, ccs_is_src ? ccs_sz : src_sz, - ccs_is_src ? dst_sz : ccs_sz); - err = emit_copy_ccs(rq, dst_offset, dst_access, - src_offset, src_access, ccs_sz); + src_offset, src_access, len); if (err) goto out_rq; err = rq->engine->emit_flush(rq, EMIT_INVALIDATE); if (err) goto out_rq; - - /* Converting back to ccs bytes */ - ccs_sz = GET_CCS_BYTES(rq->engine->i915, ccs_sz); ccs_bytes_to_cpy -= ccs_sz; } -- 2.20.1
[PATCH v3 1/3] drm/i915/gt: BUG_ON unexpected NULL at scatterlist walking
While locating the start of ccs scatterlist in smem scatterlist, that has to be the size of lmem obj size + corresponding ccs data size, report bug if scatterlist terminate before that length. v2: s/GEM_BUG_ON/BUG_ON with more commenting [Matt] v3: Converted GEM_BUG_ON into BUG_ON with more documentation [Matt] Signed-off-by: Ramalingam C Reviewed-by: Matthew Auld (v1) --- drivers/gpu/drm/i915/gt/intel_migrate.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c index 9d552f30b627..168d17b6f48a 100644 --- a/drivers/gpu/drm/i915/gt/intel_migrate.c +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c @@ -687,6 +687,16 @@ static void get_ccs_sg_sgt(struct sgt_dma *it, u32 bytes_to_cpy) bytes_to_cpy -= len; it->sg = __sg_next(it->sg); + + /* +* On Flat-CCS capable platform when we back the lmem pages with +* smem pages we add extra pages at the end of the smem +* scatterlist, to store the ccs data corresponding to the lmem +* pages. get_ccs_sg_sgt() is called to get the pointer for the +* start of the extra pages added at the end of smem scatterlist. +* So scatterlist can't end at or before bytes_to_cpy. +*/ + BUG_ON(!it->sg); it->dma = sg_dma_address(it->sg); it->max = it->dma + sg_dma_len(it->sg); } while (bytes_to_cpy); @@ -748,8 +758,10 @@ intel_context_migrate_copy(struct intel_context *ce, * Need to fix it. */ ccs_bytes_to_cpy = src_sz != dst_sz ? GET_CCS_BYTES(i915, bytes_to_cpy) : 0; - if (ccs_bytes_to_cpy) + if (ccs_bytes_to_cpy) { + WARN_ON(abs(src_sz - dst_sz) < ccs_bytes_to_cpy); get_ccs_sg_sgt(&it_ccs, bytes_to_cpy); + } } src_offset = 0; -- 2.20.1
[PATCH v3 3/3] drm/i915/gt: Document the eviction of the Flat-CCS objects
Capture the eviction details for Flat-CCS capable, lmem objects. v2: Fix the Flat-ccs capbility of lmem obj with smem residency possibility [Thomas] v3: Fixed the suggestions [Matt] Signed-off-by: Ramalingam C cc: Thomas Hellstrom cc: Matthew Auld Reviewed-by: Matthew Auld --- drivers/gpu/drm/i915/gt/intel_migrate.c | 23 ++- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_migrate.c b/drivers/gpu/drm/i915/gt/intel_migrate.c index fc6975e55fae..509955885b93 100644 --- a/drivers/gpu/drm/i915/gt/intel_migrate.c +++ b/drivers/gpu/drm/i915/gt/intel_migrate.c @@ -485,16 +485,21 @@ static bool wa_1209644611_applies(int ver, u32 size) * And CCS data can be copied in and out of CCS region through * XY_CTRL_SURF_COPY_BLT. CPU can't access the CCS data directly. * - * When we exhaust the lmem, if the object's placements support smem, then we can - * directly decompress the compressed lmem object into smem and start using it - * from smem itself. + * I915 supports Flat-CCS on lmem only objects. When an objects has smem in + * its preference list, on memory pressure, i915 needs to migrate the lmem + * content into smem. If the lmem object is Flat-CCS compressed by userspace, + * then i915 needs to decompress it. But I915 lack the required information + * for such decompression. Hence I915 supports Flat-CCS only on lmem only objects. * - * But when we need to swapout the compressed lmem object into a smem region - * though objects' placement doesn't support smem, then we copy the lmem content - * as it is into smem region along with ccs data (using XY_CTRL_SURF_COPY_BLT). - * When the object is referred, lmem content will be swaped in along with - * restoration of the CCS data (using XY_CTRL_SURF_COPY_BLT) at corresponding - * location. + * When we exhaust the lmem, Flat-CCS capable objects' lmem backing memory can + * be temporarily evicted to smem, along with the auxiliary CCS state, where + * it can be potentially swapped-out at a later point, if required. + * If userspace later touches the evicted pages, then we always move + * the backing memory back to lmem, which includes restoring the saved CCS state, + * and potentially performing any required swap-in. + * + * For the migration of the lmem objects with smem in placement list, such as + * {lmem, smem}, objects are treated as non Flat-CCS capable objects. */ static inline u32 *i915_flush_dw(u32 *cmd, u32 flags) -- 2.20.1
[PATCH 3/3] drm/mgag200: Protect concurrent access to I/O registers with lock
Add a mutex lock to protect concurrent access to I/O registers against each other. This happens between invokataion of commit- tail functions and get-mode operations. Both with use the CRTC index registers MGA1064_GEN_IO_DATA and MGA1064_GEN_IO_CTL. Concurrent access can lead to failed mode-setting operations. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/mgag200/mgag200_drv.c | 6 ++ drivers/gpu/drm/mgag200/mgag200_drv.h | 1 + drivers/gpu/drm/mgag200/mgag200_mode.c | 14 ++ 3 files changed, 21 insertions(+) diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c index 217844d71ab5c..08839460606fe 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.c +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -65,6 +66,11 @@ static int mgag200_regs_init(struct mga_device *mdev) struct pci_dev *pdev = to_pci_dev(dev->dev); u32 option, option2; u8 crtcext3; + int ret; + + ret = drmm_mutex_init(dev, &mdev->rmmio_lock); + if (ret) + return ret; switch (mdev->type) { case G200_PCI: diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h b/drivers/gpu/drm/mgag200/mgag200_drv.h index 4368112023f7c..18829eb8398a0 100644 --- a/drivers/gpu/drm/mgag200/mgag200_drv.h +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h @@ -213,6 +213,7 @@ struct mga_device { struct drm_device base; unsigned long flags; + struct mutexrmmio_lock; resource_size_t rmmio_base; resource_size_t rmmio_size; void __iomem*rmmio; diff --git a/drivers/gpu/drm/mgag200/mgag200_mode.c b/drivers/gpu/drm/mgag200/mgag200_mode.c index 6e18d3bbd7207..abde7655477db 100644 --- a/drivers/gpu/drm/mgag200/mgag200_mode.c +++ b/drivers/gpu/drm/mgag200/mgag200_mode.c @@ -881,6 +881,14 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, .y2 = fb->height, }; + /* +* Concurrent operations could possibly trigger a call to +* drm_connector_helper_funcs.get_modes by trying to read the +* display modes. Protect access to I/O registers by acquiring +* the I/O-register lock. +*/ + mutex_lock(&mdev->rmmio_lock); + if (mdev->type == G200_WB || mdev->type == G200_EW3) mgag200_g200wb_hold_bmc(mdev); @@ -904,6 +912,8 @@ mgag200_simple_display_pipe_enable(struct drm_simple_display_pipe *pipe, mgag200_enable_display(mdev); mgag200_handle_damage(mdev, fb, &fullscreen, &shadow_plane_state->data[0]); + + mutex_unlock(&mdev->rmmio_lock); } static void @@ -963,8 +973,12 @@ mgag200_simple_display_pipe_update(struct drm_simple_display_pipe *pipe, if (!fb) return; + mutex_lock(&mdev->rmmio_lock); + if (drm_atomic_helper_damage_merged(old_state, state, &damage)) mgag200_handle_damage(mdev, fb, &damage, &shadow_plane_state->data[0]); + + mutex_unlock(&mdev->rmmio_lock); } static struct drm_crtc_state * -- 2.36.0
[PATCH 2/3] drm/ast: Protect concurrent access to I/O registers with lock
Add a mutex lock to protect concurrent access to I/O registers against each other. This happens between invokataion of commit- tail functions and get-mode operations. Both with use the CRTC index register AST_IO_CRTC_PORT. Concurrent access can lead to failed mode-setting operations. Signed-off-by: Thomas Zimmermann Reported-by: KuoHsiang Chou --- drivers/gpu/drm/ast/ast_drv.h | 1 + drivers/gpu/drm/ast/ast_main.c | 4 +++ drivers/gpu/drm/ast/ast_mode.c | 48 +++--- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_drv.h b/drivers/gpu/drm/ast/ast_drv.h index a19315b2f7e56..43cedd767f8f1 100644 --- a/drivers/gpu/drm/ast/ast_drv.h +++ b/drivers/gpu/drm/ast/ast_drv.h @@ -158,6 +158,7 @@ to_ast_sil164_connector(struct drm_connector *connector) struct ast_private { struct drm_device base; + struct mutex ioregs_lock; /* Protects access to I/O registers in ioregs */ void __iomem *regs; void __iomem *ioregs; void __iomem *dp501_fw_buf; diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 22e9e2d3c71ab..5ad473aeaea2b 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -420,6 +420,10 @@ struct ast_private *ast_device_create(const struct drm_driver *drv, pci_set_drvdata(pdev, dev); + ret = drmm_mutex_init(dev, &ast->ioregs_lock); + if (ret) + return ERR_PTR(ret); + ast->regs = pcim_iomap(pdev, 1, 0); if (!ast->regs) return ERR_PTR(-EIO); diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c index 45b56b39ad473..f7849638fcb7e 100644 --- a/drivers/gpu/drm/ast/ast_mode.c +++ b/drivers/gpu/drm/ast/ast_mode.c @@ -1099,6 +1099,20 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc, return 0; } +static void ast_crtc_helper_atomic_begin(struct drm_crtc *crtc, struct drm_atomic_state *state) +{ + struct drm_device *dev = crtc->dev; + struct ast_private *ast = to_ast_private(dev); + + /* +* Concurrent operations could possibly trigger a call to +* drm_connector_helper_funcs.get_modes by trying to read the +* display modes. Protect access to I/O registers by acquiring +* the I/O-register lock. Released in atomic_flush(). +*/ + mutex_lock(&ast->ioregs_lock); +} + static void ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_atomic_state *state) @@ -1107,7 +1121,8 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, crtc); struct drm_crtc_state *old_crtc_state = drm_atomic_get_old_crtc_state(state, crtc); - struct ast_private *ast = to_ast_private(crtc->dev); + struct drm_device *dev = crtc->dev; + struct ast_private *ast = to_ast_private(dev); struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc_state); struct ast_crtc_state *old_ast_crtc_state = to_ast_crtc_state(old_crtc_state); @@ -1117,6 +1132,8 @@ ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, */ if (old_ast_crtc_state->format != ast_crtc_state->format) ast_crtc_load_lut(ast, crtc); + + mutex_unlock(&ast->ioregs_lock); } static void @@ -1175,6 +1192,7 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc, static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = { .mode_valid = ast_crtc_helper_mode_valid, .atomic_check = ast_crtc_helper_atomic_check, + .atomic_begin = ast_crtc_helper_atomic_begin, .atomic_flush = ast_crtc_helper_atomic_flush, .atomic_enable = ast_crtc_helper_atomic_enable, .atomic_disable = ast_crtc_helper_atomic_disable, @@ -1260,21 +1278,33 @@ static int ast_crtc_init(struct drm_device *dev) static int ast_vga_connector_helper_get_modes(struct drm_connector *connector) { struct ast_vga_connector *ast_vga_connector = to_ast_vga_connector(connector); + struct drm_device *dev = connector->dev; + struct ast_private *ast = to_ast_private(dev); struct edid *edid; int count; if (!ast_vga_connector->i2c) goto err_drm_connector_update_edid_property; + /* +* Protect access to I/O registers from concurrent modesetting +* by acquiring the I/O-register lock. +*/ + mutex_lock(&ast->ioregs_lock); + edid = drm_get_edid(connector, &ast_vga_connector->i2c->adapter); if (!edid) - goto err_drm_connector_update_edid_property; + goto err_mutex_unlock; + + mutex_unlock(&ast->ioregs_lock); count = drm_add_edid_modes(connector, edid); kfree(edid); return count; +err_mutex_unlock: + mutex
[PATCH 0/3] drm/{ast, mgag200}: Protect I/O regs against concurrent access
Protect access to I/O registers in ast and mgag200 via lock. Commit- tail functions and get-modes operations use the same registers and can interfere with each other. This can result in failed mode-setting operations. As both drivers use fully managed cleanup, the patchset adds a new helper that initializes a mutex with auto-cleanup. Thomas Zimmermann (3): drm: Add DRM-managed mutex_init() drm/ast: Protect concurrent access to I/O registers with lock drm/mgag200: Protect concurrent access to I/O registers with lock drivers/gpu/drm/ast/ast_drv.h | 1 + drivers/gpu/drm/ast/ast_main.c | 4 +++ drivers/gpu/drm/ast/ast_mode.c | 48 -- drivers/gpu/drm/drm_managed.c | 27 +++ drivers/gpu/drm/mgag200/mgag200_drv.c | 6 drivers/gpu/drm/mgag200/mgag200_drv.h | 1 + drivers/gpu/drm/mgag200/mgag200_mode.c | 14 include/drm/drm_managed.h | 3 ++ 8 files changed, 101 insertions(+), 3 deletions(-) base-commit: 3ae2e00290c290713e21118220a817a24b44d39f prerequisite-patch-id: c2b2f08f0eccc9f5df0c0da49fa1d36267deb11d prerequisite-patch-id: c67e5d886a47b7d0266d81100837557fda34cb24 -- 2.36.0
[PATCH 1/3] drm: Add DRM-managed mutex_init()
Add drmm_mutex_init(), a helper that provides managed mutex cleanup. The mutex will be destroyed with the final reference of the DRM device. Signed-off-by: Thomas Zimmermann --- drivers/gpu/drm/drm_managed.c | 27 +++ include/drm/drm_managed.h | 3 +++ 2 files changed, 30 insertions(+) diff --git a/drivers/gpu/drm/drm_managed.c b/drivers/gpu/drm/drm_managed.c index 37d7db6223be6..4cf214de50c40 100644 --- a/drivers/gpu/drm/drm_managed.c +++ b/drivers/gpu/drm/drm_managed.c @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -262,3 +263,29 @@ void drmm_kfree(struct drm_device *dev, void *data) free_dr(dr_match); } EXPORT_SYMBOL(drmm_kfree); + +static void drmm_mutex_release(struct drm_device *dev, void *res) +{ + struct mutex *lock = res; + + mutex_destroy(lock); +} + +/** + * drmm_mutex_init - &drm_device-managed mutex_init() + * @dev: DRM device + * @lock: lock to be initialized + * + * Returns: + * 0 on success, or a negative errno code otherwise. + * + * This is a &drm_device-managed version of mutex_init(). The initialized + * lock is automatically destroyed on the final drm_dev_put(). + */ +int drmm_mutex_init(struct drm_device *dev, struct mutex *lock) +{ + mutex_init(lock); + + return drmm_add_action_or_reset(dev, drmm_mutex_release, lock); +} +EXPORT_SYMBOL(drmm_mutex_init); diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h index b45c6fbf53ac4..359883942612e 100644 --- a/include/drm/drm_managed.h +++ b/include/drm/drm_managed.h @@ -8,6 +8,7 @@ #include struct drm_device; +struct mutex; typedef void (*drmres_release_t)(struct drm_device *dev, void *res); @@ -104,4 +105,6 @@ char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp); void drmm_kfree(struct drm_device *dev, void *data); +int drmm_mutex_init(struct drm_device *dev, struct mutex *lock); + #endif -- 2.36.0
Re: [Intel-gfx] [V2 3/3] drm/amd/display: Move connector debugfs to drm
On Mon-02-05-2022 07:08 pm, Harry Wentland wrote: On 2022-05-02 09:28, Modem, Bhanuprakash wrote: On Fri-29-04-2022 08:02 pm, Murthy, Arun R wrote: -Original Message- From: Intel-gfx On Behalf Of Bhanuprakash Modem Sent: Monday, April 11, 2022 3:21 PM To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; amd- g...@lists.freedesktop.org; jani.nik...@linux.intel.com; ville.syrj...@linux.intel.com; harry.wentl...@amd.com; Sharma, Swati2 Cc: Rodrigo Siqueira Subject: [Intel-gfx] [V2 3/3] drm/amd/display: Move connector debugfs to drm As drm_connector already have the display_info, instead of creating "output_bpc" debugfs in vendor specific driver, move the logic to the drm layer. This patch will also move "Current" bpc to the crtc debugfs from connector debugfs, since we are getting this info from crtc_state. Cc: Harry Wentland Cc: Rodrigo Siqueira Signed-off-by: Bhanuprakash Modem Reported-by: kernel test robot --- Reviewed-by: Arun R Murthy Thanks Arun, @Harry/@Rodrigo, If this change sounds good to you, can you please help to push it? This changes the output_bpc debugfs behavior on amdgpu and breaks the amd_max_bpc IGT test. I don't think we should merge this as-is. Yeah, I have floated the IGT changes to support this series: https://patchwork.freedesktop.org/series/102387/ With this IGT change, we can merge this series as-is. I would like to request you to review IGT patches too. This patch also seems dependent on patch 1 of the series. Shouldn't they be merged together (please don't merge them as-is, though)? Yes, as other patches in this series are already reviewed, I think we need to plan to merge all patches in this series together (If above IGT & this patch looks good to you). - Bhanu Harry - Bhanu Thanks and Regards, Arun R Murthy
Re: [Intel-gfx] [V2 3/3] drm/amd/display: Move connector debugfs to drm
On 2022-05-02 10:27, Modem, Bhanuprakash wrote: > On Mon-02-05-2022 07:08 pm, Harry Wentland wrote: >> >> >> On 2022-05-02 09:28, Modem, Bhanuprakash wrote: >>> On Fri-29-04-2022 08:02 pm, Murthy, Arun R wrote: > -Original Message- > From: Intel-gfx On Behalf Of > Bhanuprakash Modem > Sent: Monday, April 11, 2022 3:21 PM > To: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; > amd- > g...@lists.freedesktop.org; jani.nik...@linux.intel.com; > ville.syrj...@linux.intel.com; harry.wentl...@amd.com; Sharma, Swati2 > > Cc: Rodrigo Siqueira > Subject: [Intel-gfx] [V2 3/3] drm/amd/display: Move connector > debugfs to > drm > > As drm_connector already have the display_info, instead of creating > "output_bpc" debugfs in vendor specific driver, move the logic to the > drm > layer. > > This patch will also move "Current" bpc to the crtc debugfs from > connector > debugfs, since we are getting this info from crtc_state. > > Cc: Harry Wentland > Cc: Rodrigo Siqueira > Signed-off-by: Bhanuprakash Modem > Reported-by: kernel test robot > --- Reviewed-by: Arun R Murthy >>> >>> Thanks Arun, >>> >>> @Harry/@Rodrigo, If this change sounds good to you, can you please help >>> to push it? >>> >> >> This changes the output_bpc debugfs behavior on amdgpu and breaks >> the amd_max_bpc IGT test. I don't think we should merge this as-is. > > Yeah, I have floated the IGT changes to support this series: > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F102387%2F&data=05%7C01%7Charry.wentland%40amd.com%7C61d4e4a755a5449ec58308da2c47dd89%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637870984414230229%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=kge5PgzzX81hsFLBKyPfyv7vQpb1Gse72FWuiGtyoAQ%3D&reserved=0 > > > With this IGT change, we can merge this series as-is. I would like to > request you to review IGT patches too. > >> >> This patch also seems dependent on patch 1 of the series. Shouldn't >> they be merged together (please don't merge them as-is, though)? > > Yes, as other patches in this series are already reviewed, I think we > need to plan to merge all patches in this series together (If above IGT > & this patch looks good to you). > Thanks for the context again and apologies I haven't had the time to have a closer look so far. I'll go over these and the IGT patches today and get back to you. Harry > - Bhanu > >> >> Harry >> >>> - Bhanu >>> Thanks and Regards, Arun R Murthy >>> >
Re: [PATCH] drm/panel: panel-simple: Fix proper bpc for AM-1280800N3TZQW-T00H
On Fri, 29 Apr 2022 at 11:49, Jagan Teki wrote: > > Hi Robert, > > Can you apply this? > Thanks for the reminder. Applied to drm-misc-next. > On Thu, Mar 31, 2022 at 8:22 PM Robert Foss wrote: > > > > On Thu, 31 Mar 2022 at 16:50, Jagan Teki wrote: > > > > > > + Robert > > > > > > On Tue, Feb 22, 2022 at 12:17 PM Jagan Teki > > > wrote: > > > > > > > > On Mon, Feb 7, 2022 at 6:34 PM Jagan Teki > > > > wrote: > > > > > > > > > > Hi Sam, > > > > > > > > > > On Mon, Dec 20, 2021 at 1:45 PM Sam Ravnborg > > > > > wrote: > > > > > > > > > > > > Hi Jagan, > > > > > > > > > > > > On Sun, Dec 19, 2021 at 10:10:10PM +0530, Jagan Teki wrote: > > > > > > > Hi Sam, > > > > > > > > > > > > > > On Thu, Nov 11, 2021 at 3:11 PM Jagan Teki > > > > > > > wrote: > > > > > > > > > > > > > > > > AM-1280800N3TZQW-T00H panel support 8 bpc not 6 bpc as per > > > > > > > > recent testing in i.MX8MM platform. > > > > > > > > > > > > > > > > Fix it. > > > > > > > > > > > > > > > > Fixes: bca684e69c4c ("drm/panel: simple: Add > > > > > > > > AM-1280800N3TZQW-T00H") > > > > > > > > Signed-off-by: Jagan Teki > > > > > > > > --- > > > > > > > > drivers/gpu/drm/panel/panel-simple.c | 2 +- > > > > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > index eb475a3a774b..cf3f21f649cb 100644 > > > > > > > > --- a/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > > > > > > > @@ -719,7 +719,7 @@ static const struct drm_display_mode > > > > > > > > ampire_am_1280800n3tzqw_t00h_mode = { > > > > > > > > static const struct panel_desc ampire_am_1280800n3tzqw_t00h = { > > > > > > > > .modes = &ire_am_1280800n3tzqw_t00h_mode, > > > > > > > > .num_modes = 1, > > > > > > > > - .bpc = 6, > > > > > > > > + .bpc = 8, > > > > > > > > > > > > > > Any response on this? > > > > Reviewed-by: Robert Foss > > > > -- > Jagan Teki, > Amarula Solutions India Pvt. Ltd. > Co-Founder & Embedded Linux Architect > 405/E-Block, Sri Lakshmi Shubham Arcade, Chandanagar, Hyderabad - 500050, > India > M. (+91) 910 009 0959 > [`as] http://www.amarulasolutions.com
Re: [PATCH] drm/ast: Atomic CR/SR reg R/W
Hi Am 06.12.21 um 02:38 schrieb Kuo-Hsiang Chou: I'm not going to merge this patch. As I said, I don't think it fixes the problem. Mouse movement and resolution switching should not interfere with each other. The DRM framework should guarantee that. OK, thanks for your confirmation. I cannot reproduce the issue, but there's most likely something else happening here. How can the system switch resolution and change the mouse at the same time? Sure, we will check if there is a 100 percent method to reproduce the issue. Thanks for your responses. I've been away for a while; sorry for all this taking so long. I've meanwhile been able to reproduce the problem. It happens when GNOME concurrently tries to set the video mode and read the available video modes from EDID. Reading EDID is not protected against concurrent mode setting or cursor movement. I've posted a patchset that should fix the problem. See [1]. Best regards Thomas [1] https://lore.kernel.org/dri-devel/20220502142514.2174-1-tzimmerm...@suse.de/T/#t Regards, Kuo-Hsiang Chou Best regards Thomas Hi Tomas, Good day! May I understand the review status, or is there anything I can do to improve it? Thanks! Best Regards, Kuo-Hsiang Chou Best Regards, Kuo-Hsiang Chou Best regards Thomas Signed-off-by: KuoHsiang Chou --- drivers/gpu/drm/ast/ast_main.c | 48 +- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/ast/ast_main.c b/drivers/gpu/drm/ast/ast_main.c index 79a361867..1d8fa70c5 100644 --- a/drivers/gpu/drm/ast/ast_main.c +++ b/drivers/gpu/drm/ast/ast_main.c @@ -41,28 +41,52 @@ void ast_set_index_reg_mask(struct ast_private *ast, uint32_t base, uint8_t index, uint8_t mask, uint8_t val) { - u8 tmp; - ast_io_write8(ast, base, index); - tmp = (ast_io_read8(ast, base + 1) & mask) | val; - ast_set_index_reg(ast, base, index, tmp); + uint16_t volatile usData; + uint8_t volatile jData; + + do { + ast_io_write8(ast, base, index); + usData = ast_io_read16(ast, base); + } while ((uint8_t)(usData) != index); + + jData = (uint8_t)(usData >> 8); + jData &= mask; + jData |= val; + usData = ((uint16_t) jData << 8) | (uint16_t) index; + ast_io_write16(ast, base, usData); } uint8_t ast_get_index_reg(struct ast_private *ast, uint32_t base, uint8_t index) { - uint8_t ret; - ast_io_write8(ast, base, index); - ret = ast_io_read8(ast, base + 1); - return ret; + uint16_t volatile usData; + uint8_t volatile jData; + + do { + ast_io_write8(ast, base, index); + usData = ast_io_read16(ast, base); + } while ((uint8_t)(usData) != index); + + jData = (uint8_t)(usData >> 8); + + return jData; } uint8_t ast_get_index_reg_mask(struct ast_private *ast, uint32_t base, uint8_t index, uint8_t mask) { - uint8_t ret; - ast_io_write8(ast, base, index); - ret = ast_io_read8(ast, base + 1) & mask; - return ret; + uint16_t volatile usData; + uint8_t volatile jData; + + do { + ast_io_write8(ast, base, index); + usData = ast_io_read16(ast, base); + } while ((uint8_t)(usData) != index); + + jData = (uint8_t)(usData >> 8); + jData &= mask; + + return jData; } static void ast_detect_config_mode(struct drm_device *dev, u32 *scu_rev) -- 2.18.4 -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev OpenPGP_signature Description: OpenPGP digital signature
Re: [PATCH] drm/bridge: it6505: Send DPCD SET_POWER to downstream
On Mon, 25 Apr 2022 at 15:44, Pin-Yen Lin wrote: > > Send DPCD SET_POWER command to downstream in .atomic_disable to make the > downstream monitor enter the power down mode, so the device suspend won't > be affected. > > Fixes: b5c84a9edcd418 ("drm/bridge: add it6505 driver") > Signed-off-by: Pin-Yen Lin > --- > > drivers/gpu/drm/bridge/ite-it6505.c | 29 ++--- > 1 file changed, 18 insertions(+), 11 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ite-it6505.c > b/drivers/gpu/drm/bridge/ite-it6505.c > index 8fed30df08b0..4b673c4792d7 100644 > --- a/drivers/gpu/drm/bridge/ite-it6505.c > +++ b/drivers/gpu/drm/bridge/ite-it6505.c > @@ -737,8 +737,9 @@ static int it6505_drm_dp_link_probe(struct drm_dp_aux > *aux, > return 0; > } > > -static int it6505_drm_dp_link_power_up(struct drm_dp_aux *aux, > - struct it6505_drm_dp_link *link) > +static int it6505_drm_dp_link_set_power(struct drm_dp_aux *aux, > + struct it6505_drm_dp_link *link, > + u8 mode) > { > u8 value; > int err; > @@ -752,18 +753,20 @@ static int it6505_drm_dp_link_power_up(struct > drm_dp_aux *aux, > return err; > > value &= ~DP_SET_POWER_MASK; > - value |= DP_SET_POWER_D0; > + value |= mode; > > err = drm_dp_dpcd_writeb(aux, DP_SET_POWER, value); > if (err < 0) > return err; > > - /* > -* According to the DP 1.1 specification, a "Sink Device must exit the > -* power saving state within 1 ms" (Section 2.5.3.1, Table 5-52, "Sink > -* Control Field" (register 0x600). > -*/ > - usleep_range(1000, 2000); > + if (mode == DP_SET_POWER_D0) { > + /* > +* According to the DP 1.1 specification, a "Sink Device must > +* exit the power saving state within 1 ms" (Section 2.5.3.1, > +* Table 5-52, "Sink Control Field" (register 0x600). > +*/ > + usleep_range(1000, 2000); > + } > > return 0; > } > @@ -2624,7 +2627,8 @@ static enum drm_connector_status it6505_detect(struct > it6505 *it6505) > if (it6505_get_sink_hpd_status(it6505)) { > it6505_aux_on(it6505); > it6505_drm_dp_link_probe(&it6505->aux, &it6505->link); > - it6505_drm_dp_link_power_up(&it6505->aux, &it6505->link); > + it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link, > +DP_SET_POWER_D0); > it6505->auto_train_retry = AUTO_TRAIN_RETRY; > > if (it6505->dpcd[0] == 0) { > @@ -2960,8 +2964,11 @@ static void it6505_bridge_atomic_disable(struct > drm_bridge *bridge, > > DRM_DEV_DEBUG_DRIVER(dev, "start"); > > - if (it6505->powered) > + if (it6505->powered) { > it6505_video_disable(it6505); > + it6505_drm_dp_link_set_power(&it6505->aux, &it6505->link, > +DP_SET_POWER_D3); > + } > } > > static enum drm_connector_status > -- > 2.36.0.rc2.479.g8af0fa9b8e-goog > Reviewed-by: Robert Foss Applied to drm-misc-next.
Re: [PATCH v2] drm/i915: Fix race in __i915_vma_remove_closed
friendly ping. I am not even myself completely convinced that this is the correct patch and it might just workaround some issues, but list_debug.c does check if a list was already deleted and throws an error if it was and this patch indeed fixes this one issue as multiple threads could enter __i915_vma_remove_closed on the same vma. On Wed, Apr 20, 2022 at 11:57 AM Karol Herbst wrote: > > i915_vma_reopen checked if the vma is closed before without taking the > lock. So multiple threads could attempt removing the vma. > > Instead the lock needs to be taken before actually checking. > > v2: move struct declaration > > Cc: Chris Wilson > Cc: intel-...@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/5732 > Signed-off-by: Karol Herbst > --- > drivers/gpu/drm/i915/i915_vma.c | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c > index 162e8d83691b..2efdad2b43fa 100644 > --- a/drivers/gpu/drm/i915/i915_vma.c > +++ b/drivers/gpu/drm/i915/i915_vma.c > @@ -1615,17 +1615,17 @@ void i915_vma_close(struct i915_vma *vma) > > static void __i915_vma_remove_closed(struct i915_vma *vma) > { > - struct intel_gt *gt = vma->vm->gt; > - > - spin_lock_irq(>->closed_lock); > list_del_init(&vma->closed_link); > - spin_unlock_irq(>->closed_lock); > } > > void i915_vma_reopen(struct i915_vma *vma) > { > + struct intel_gt *gt = vma->vm->gt; > + > + spin_lock_irq(>->closed_lock); > if (i915_vma_is_closed(vma)) > __i915_vma_remove_closed(vma); > + spin_unlock_irq(>->closed_lock); > } > > static void force_unbind(struct i915_vma *vma) > @@ -1641,6 +1641,7 @@ static void force_unbind(struct i915_vma *vma) > static void release_references(struct i915_vma *vma, bool vm_ddestroy) > { > struct drm_i915_gem_object *obj = vma->obj; > + struct intel_gt *gt = vma->vm->gt; > > GEM_BUG_ON(i915_vma_is_active(vma)); > > @@ -1651,7 +1652,9 @@ static void release_references(struct i915_vma *vma, > bool vm_ddestroy) > > spin_unlock(&obj->vma.lock); > > + spin_lock_irq(>->closed_lock); > __i915_vma_remove_closed(vma); > + spin_unlock_irq(>->closed_lock); > > if (vm_ddestroy) > i915_vm_resv_put(vma->vm); > -- > 2.35.1 >
Re: [PATCH v2 0/2] drm: bridge: adv7511: CEC support for ADV7535
On Sat, 23 Apr 2022 at 14:09, Alvin Šipraga wrote: > > From: Alvin Šipraga > > Changes: > > v1->v2: > - add Robert's r-b > - fix up 'case XXX...YYY+14' statements to read nicer in the 2nd patch > > > We have an ADV7535 which is nominally supported by this driver. These > two patches fix up the driver to get CEC working too. > > The first adds the basic support by correcting some register offsets. > > The second addresses an issue we saw with CEC RX on the ADV7535. It > hasn't been tested with the other chips (e.g. ADV7533), although it > should be compatible. I'm sending it against drm-misc-next because the > issue wasn't reported for other chips, and ADV7535 didn't have CEC > support before. But feel free to take it into -fixes instead. > > > Alvin Šipraga (2): > drm: bridge: adv7511: enable CEC support for ADV7535 > drm: bridge: adv7511: use non-legacy mode for CEC RX > > drivers/gpu/drm/bridge/adv7511/adv7511.h | 27 - > drivers/gpu/drm/bridge/adv7511/adv7511_cec.c | 116 +-- > drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 19 ++- > 3 files changed, 116 insertions(+), 46 deletions(-) Applied to drm-misc-next.
Re: [PATCH v4 1/2] dt-bindings: display: bridge: ldb: Implement simple Freescale i.MX8MP LDB bridge
On Tue, 26 Apr 2022 at 21:37, Marek Vasut wrote: > > The i.MX8MP contains two syscon registers which are responsible > for configuring the on-SoC DPI-to-LVDS serializer. Add DT binding > which represents this serializer as a bridge. > > Acked-by: Sam Ravnborg > Signed-off-by: Marek Vasut > Cc: Laurent Pinchart > Cc: Lucas Stach > Cc: Maxime Ripard > Cc: Peng Fan > Cc: Rob Herring > Cc: Robby Cai > Cc: Robert Foss > Cc: Sam Ravnborg > Cc: Thomas Zimmermann > Cc: devicet...@vger.kernel.org > To: dri-devel@lists.freedesktop.org > --- > V2: - Consistently use fsl,imx8mp-ldb as compatible > - Drop items: from compatible: > - Replace minItems with maxItems in clocks: > - Drop quotes from clock-names const: ldb > - Rename syscon to fsl,syscon > - Use generic name of ldb-lvds in example > V3: - Add AB from Sam > - Consistently use MX8MP > V4: - Rename to fsl-ldb all over the place > - Put the LDB node under media block controller in the example > --- > .../bindings/display/bridge/fsl,ldb.yaml | 92 +++ > 1 file changed, 92 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml > > diff --git a/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml > b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml > new file mode 100644 > index ..77f174eee424 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/fsl,ldb.yaml > @@ -0,0 +1,92 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/fsl,ldb.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Freescale i.MX8MP DPI to LVDS bridge chip > + > +maintainers: > + - Marek Vasut > + > +description: | > + The i.MX8MP mediamix contains two registers which are responsible > + for configuring the on-SoC DPI-to-LVDS serializer. This describes > + those registers as bridge within the DT. > + > +properties: > + compatible: > +const: fsl,imx8mp-ldb > + > + clocks: > +maxItems: 1 > + > + clock-names: > +const: ldb > + > + ports: > +$ref: /schemas/graph.yaml#/properties/ports > + > +properties: > + port@0: > +$ref: /schemas/graph.yaml#/properties/port > +description: Video port for DPI input. > + > + port@1: > +$ref: /schemas/graph.yaml#/properties/port > +description: Video port for LVDS Channel-A output (panel or bridge). > + > + port@2: > +$ref: /schemas/graph.yaml#/properties/port > +description: Video port for LVDS Channel-B output (panel or bridge). > + > +required: > + - port@0 > + - port@1 > + > +required: > + - compatible > + - clocks > + - ports > + > +additionalProperties: false > + > +examples: > + - | > +#include > + > +blk-ctrl { > +bridge { > +compatible = "fsl,imx8mp-ldb"; > +clocks = <&clk IMX8MP_CLK_MEDIA_LDB>; > +clock-names = "ldb"; > + > +ports { > +#address-cells = <1>; > +#size-cells = <0>; > + > +port@0 { > +reg = <0>; > + > +ldb_from_lcdif2: endpoint { > +remote-endpoint = <&lcdif2_to_ldb>; > +}; > +}; > + > +port@1 { > +reg = <1>; > + > +ldb_lvds_ch0: endpoint { > +remote-endpoint = <&ldb_to_lvdsx4panel>; > +}; > +}; > + > +port@2 { > +reg = <2>; > + > +ldb_lvds_ch1: endpoint { > +}; > +}; > +}; > +}; > +}; > -- Applied series to drm-misc-next.
Re: [PATCH] drm/bridge: tfp410: Make tfp410_fini() return void
On Thu, 28 Apr 2022 at 20:08, Laurent Pinchart wrote: > > Hi Uwe, > > Thank you for the patch. > > On Thu, Apr 28, 2022 at 06:28:03PM +0200, Uwe Kleine-König wrote: > > tfp410_fini() always returns zero. Make it return no value which makes it > > easier to see in the callers that there is no error to handle. > > > > Also the return value of i2c and platform driver remove callbacks is > > ignored anyway. This prepares making i2c and platform remove callbacks > > return void, too. > > > > Signed-off-by: Uwe Kleine-König > > Reviewed-by: Laurent Pinchart > > > --- > > drivers/gpu/drm/bridge/ti-tfp410.c | 12 +++- > > 1 file changed, 7 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c > > b/drivers/gpu/drm/bridge/ti-tfp410.c > > index ba3fa2a9b8a4..756b3e6e776b 100644 > > --- a/drivers/gpu/drm/bridge/ti-tfp410.c > > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > > @@ -341,13 +341,11 @@ static int tfp410_init(struct device *dev, bool i2c) > > return 0; > > } > > > > -static int tfp410_fini(struct device *dev) > > +static void tfp410_fini(struct device *dev) > > { > > struct tfp410 *dvi = dev_get_drvdata(dev); > > > > drm_bridge_remove(&dvi->bridge); > > - > > - return 0; > > } > > > > static int tfp410_probe(struct platform_device *pdev) > > @@ -357,7 +355,9 @@ static int tfp410_probe(struct platform_device *pdev) > > > > static int tfp410_remove(struct platform_device *pdev) > > { > > - return tfp410_fini(&pdev->dev); > > + tfp410_fini(&pdev->dev); > > + > > + return 0; > > } > > > > static const struct of_device_id tfp410_match[] = { > > @@ -394,7 +394,9 @@ static int tfp410_i2c_probe(struct i2c_client *client, > > > > static int tfp410_i2c_remove(struct i2c_client *client) > > { > > - return tfp410_fini(&client->dev); > > + tfp410_fini(&client->dev); > > + > > + return 0; > > } > > > > static const struct i2c_device_id tfp410_i2c_ids[] = { > > > > base-commit: 3123109284176b1532874591f7c81f3837bbdc17 > > -- > Regards, > > Laurent Pinchart Applied to drm-misc-next.
Re: [PATCHv4] drm/amdgpu: disable ASPM on Intel Alder Lake based systems
Hi Paul, On 5/1/2022 2:08 AM, Paul Menzel wrote: Dear Richard, Sorry for the late reply. Am 26.04.22 um 15:53 schrieb Gong, Richard: On 4/21/2022 12:35 AM, Paul Menzel wrote: Am 21.04.22 um 03:12 schrieb Gong, Richard: On 4/20/2022 3:29 PM, Paul Menzel wrote: Am 19.04.22 um 23:46 schrieb Gong, Richard: On 4/14/2022 2:52 AM, Paul Menzel wrote: [Cc: -kernel test robot ] […] Am 13.04.22 um 15:00 schrieb Alex Deucher: On Wed, Apr 13, 2022 at 3:43 AM Paul Menzel wrote: Thank you for sending out v4. Am 12.04.22 um 23:50 schrieb Richard Gong: […] I am still not clear, what “hang during suspend/resume” means. I guess suspending works fine? During resume (S3 or S0ix?), where does it hang? The system is functional, but there are only display problems? System freeze after suspend/resume. But you see certain messages still? At what point does it freeze exactly? In the bug report you posted Linux messages. No, the system freeze then users have to recycle power to recover. Then I misread the issue? Did you capture the messages over serial log then? I think so. We captured dmesg log. make a correction, the previous 'dmesg log' description was not accurate. I referred that to the kernel log captured via 'journalctl' after recycling the power. I should use kernel log rather than 'dmesg log' to avoid the confusion. Then the (whole) system did *not* freeze, if you could still log in (maybe over network) and execute `dmesg`. Please also paste the amdgpu(?) error logs in the commit message. As mentioned in my "previous previous" reply, the user have to recycle power to reset the system. When issue occurred, keyboard/mouse didn't work. 'demsg' and ssh didn't work either. As mentioned early we need support from Intel on how to get ASPM working for VI generation on Intel Alder Lake, but we don't know where things currently stand. Who is working on this, and knows? I have no idea. Kind regards, Paul Regards, Richard
Re: [PATCH v12] drm/amdgpu: add drm buddy support to amdgpu
On Wed, 27 Apr 2022 at 12:55, Mike Lothian wrote: > > On Tue, 26 Apr 2022 at 17:36, Christian König > wrote: > > > > Hi Mike, > > > > sounds like somehow stitching together the SG table for PRIME doesn't > > work any more with this patch. > > > > Can you try with P2P DMA disabled? > > -CONFIG_PCI_P2PDMA=y > +# CONFIG_PCI_P2PDMA is not set > > If that's what you're meaning, then there's no difference, I'll upload > my dmesg to the gitlab issue > > > > > Apart from that can you take a look Arun? > > > > Thanks, > > Christian. Hi Have you had any success in replicating this? Cheers Mike
Re: [PATCHv5] drm/amdgpu: vi: disable ASPM on Intel Alder Lake based systems
Hi Paul, On 5/1/2022 2:14 AM, Paul Menzel wrote: Dear Richard, Am 29.04.22 um 18:06 schrieb Richard Gong: Active State Power Management (ASPM) feature is enabled since kernel 5.14. There are some AMD Volcanic Islands (VI) GFX cards, such as the WX3200 and RX640, that do not work with ASPM-enabled Intel Alder Lake based systems. Using these GFX cards as video/display output, Intel Alder Lake based systems will freeze after suspend/resume. As replied in v4 just now, “freeze” is misleading if you can still run `dmesg` after resume. As my comments in v4, we can't run 'dmesg' when issue occurred. User have to recycle power to reset the system. Kind regards, Paul Regards, Richard The issue was originally reported on one system (Dell Precision 3660 with BIOS version 0.14.81), but was later confirmed to affect at least 4 pre-production Alder Lake based systems. Add an extra check to disable ASPM on Intel Alder Lake based systems with the problematic AMD Volcanic Islands GFX cards. Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default") Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1885&data=05%7C01%7Crichard.gong%40amd.com%7C78173acb0fe3463fead808da2b423e81%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637869860787352219%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=TK3Ur99Ro4OczgUlCpdod6CrvgGJvNZAyUfpzKEqExw%3D&reserved=0 Reported-by: kernel test robot Signed-off-by: Richard Gong --- v5: added vi to commit header and updated commit message rolled back guard with the preprocessor as did in v2 to correct build error on non-x86 systems v4: s/CONFIG_X86_64/CONFIG_X86 enhanced check logic v3: s/intel_core_aspm_chk/aspm_support_quirk_check correct build error with W=1 option v2: correct commit description move the check from chip family to problematic platform --- drivers/gpu/drm/amd/amdgpu/vi.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c index 039b90cdc3bc..45f0188c4273 100644 --- a/drivers/gpu/drm/amd/amdgpu/vi.c +++ b/drivers/gpu/drm/amd/amdgpu/vi.c @@ -81,6 +81,10 @@ #include "mxgpu_vi.h" #include "amdgpu_dm.h" +#if IS_ENABLED(CONFIG_X86) +#include +#endif + #define ixPCIE_LC_L1_PM_SUBSTATE 0x100100C6 #define PCIE_LC_L1_PM_SUBSTATE__LC_L1_SUBSTATES_OVERRIDE_EN_MASK 0x0001L #define PCIE_LC_L1_PM_SUBSTATE__LC_PCI_PM_L1_2_OVERRIDE_MASK 0x0002L @@ -1134,13 +1138,24 @@ static void vi_enable_aspm(struct amdgpu_device *adev) WREG32_PCIE(ixPCIE_LC_CNTL, data); } +static bool aspm_support_quirk_check(void) +{ +#if IS_ENABLED(CONFIG_X86) + struct cpuinfo_x86 *c = &cpu_data(0); + + return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE); +#else + return true; +#endif +} + static void vi_program_aspm(struct amdgpu_device *adev) { u32 data, data1, orig; bool bL1SS = false; bool bClkReqSupport = true; - if (!amdgpu_device_should_use_aspm(adev)) + if (!amdgpu_device_should_use_aspm(adev) || !aspm_support_quirk_check()) return; if (adev->flags & AMD_IS_APU ||
[PATCH v2 2/3] drm/fb-helper: Rename preferred_bpp drm_fbdev_generic_setup() parameter
By default the bits per pixel for the emulated framebuffer device is set to dev->mode_config.preferred_depth, but some devices need another value. Since this second parameter is only used by a few drivers, and to allow drivers to use it for passing other configurations when registering the fbdev, rename @preferred_bpp to @options and make it a multi-field param. The DRM_FB_OPTION() and DRM_FB_GET_OPTION() macros are provided to drivers for computing options bitfield values and getting the values respectively For now, only the DRM_FB_BPP option exists but other options can be added. Suggested-by: Thomas Zimmermann Signed-off-by: Javier Martinez Canillas Reviewed-by: Thomas Zimmermann Reviewed-by: Laurent Pinchart --- Changes in v2: - Rename DRM_FB_SET_OPTION() to DRM_FB_SET() and make more clear in the kernel-doc what this macro does (Laurent Pinchart). - Fix some kernel-doc issues I didn't notice in v1. - Add Reviewed-by tags from Thomas and Laurent. drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 -- drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- drivers/gpu/drm/arm/malidp_drv.c| 2 +- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- drivers/gpu/drm/ast/ast_drv.c | 2 +- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 2 +- drivers/gpu/drm/drm_drv.c | 2 +- drivers/gpu/drm/drm_fb_helper.c | 17 + drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +- drivers/gpu/drm/imx/dcss/dcss-kms.c | 2 +- drivers/gpu/drm/imx/imx-drm-core.c | 2 +- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- drivers/gpu/drm/mcde/mcde_drv.c | 2 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +- drivers/gpu/drm/meson/meson_drv.c | 2 +- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +- drivers/gpu/drm/pl111/pl111_drv.c | 2 +- drivers/gpu/drm/qxl/qxl_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/sti/sti_drv.c | 2 +- drivers/gpu/drm/stm/drv.c | 2 +- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- drivers/gpu/drm/tidss/tidss_drv.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- drivers/gpu/drm/tiny/arcpgu.c | 2 +- drivers/gpu/drm/tiny/bochs.c| 2 +- drivers/gpu/drm/tve200/tve200_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c| 2 +- drivers/gpu/drm/vc4/vc4_drv.c | 2 +- drivers/gpu/drm/virtio/virtgpu_drv.c| 2 +- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 +- include/drm/drm_fb_helper.h | 12 33 files changed, 59 insertions(+), 36 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index b03663f42cc9..0c54470975e3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -2118,9 +2118,11 @@ static int amdgpu_pci_probe(struct pci_dev *pdev, !list_empty(&adev_to_drm(adev)->mode_config.connector_list)) { /* select 8 bpp console on low vram cards */ if (adev->gmc.real_vram_size <= (32*1024*1024)) - drm_fbdev_generic_setup(adev_to_drm(adev), 8); + drm_fbdev_generic_setup(adev_to_drm(adev), + DRM_FB_OPTION(DRM_FB_BPP, 8)); else - drm_fbdev_generic_setup(adev_to_drm(adev), 32); + drm_fbdev_generic_setup(adev_to_drm(adev), + DRM_FB_OPTION(DRM_FB_BPP, 32)); } ret = amdgpu_debugfs_init(adev); diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c index e89ae0ec60eb..b69b1e5be379 100644 --- a/drivers/gpu/drm/arm/hdlcd_drv.c +++ b/drivers/gpu/drm/arm/hdlcd_drv.c @@ -321,7 +321,7 @@ static int hdlcd_drm_bind(struct device *dev) if (ret) goto err_register; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_generic_setup(drm, DRM_FB_OPTION(DRM_FB_BPP, 32)); return 0; diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index d5aef21426cf..25685b579a05 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -863,7 +863,7 @@ static int malidp_bind(struct device *dev) if (ret) goto register_fail; - drm_fbdev_generic_setup(drm, 32); + drm_fbdev_generic_setup(drm, DRM_FB_OPTION(DRM_FB_BPP, 32)); return 0; diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index 7780b72de9e8..d2e93aea 100644 --- a/drivers
[PATCH v2 0/3] drm: Allow simpledrm to setup its emulated FB as firmware provided
Hello, This series contain patches suggested by Thomas Zimmermann as a feedback for "[RFC PATCH v4 00/11] Fix some race between sysfb device registration and drivers probe" [0]. Since other changes in [0] were more controversial, I decided to just split this part in a new patch-set and revisit the rest of the patches later. This is a v2 that addresses issues pointed out in v1. Patch #1 is just a cleanup since when working on this noticed that some DRM drivers were passing as preferred bits per pixel to drm_fbdev_generic_setup() the value that is the default anyways. Patch #2 renames the 'preferred_bpp' drm_fbdev_generic_setup() parameter to 'options', and make this a multi field parameter so that it can be extended later to pass other options as well. Patch #3 finally adds the new DRM_FB_FW option and makes simpledrm to use it so that the registered framebuffer device is also marked as firmware provided. [0]: https://lore.kernel.org/lkml/20220429084253.1085911-1-javi...@redhat.com/ Changes in v2: - Rename DRM_FB_SET_OPTION() to DRM_FB_SET() and make more clear in the kernel-doc what this macro does (Laurent Pinchart). - Fix some kernel-doc issues I didn't notice in v1. - Add Reviewed-by tags from Thomas and Laurent. Javier Martinez Canillas (3): drm: Remove superfluous arg when calling to drm_fbdev_generic_setup() drm/fb-helper: Rename preferred_bpp drm_fbdev_generic_setup() parameter drm: Allow simpledrm to setup its emulated FB as firmware provided drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 +++-- drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- drivers/gpu/drm/arm/malidp_drv.c | 2 +- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- drivers/gpu/drm/ast/ast_drv.c | 2 +- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +- drivers/gpu/drm/drm_drv.c | 2 +- drivers/gpu/drm/drm_fb_helper.c | 26 --- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +- .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +- drivers/gpu/drm/imx/dcss/dcss-kms.c | 2 +- drivers/gpu/drm/imx/imx-drm-core.c| 2 +- drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- drivers/gpu/drm/mcde/mcde_drv.c | 2 +- drivers/gpu/drm/mediatek/mtk_drm_drv.c| 2 +- drivers/gpu/drm/meson/meson_drv.c | 2 +- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +- drivers/gpu/drm/pl111/pl111_drv.c | 2 +- drivers/gpu/drm/qxl/qxl_drv.c | 2 +- drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- drivers/gpu/drm/sti/sti_drv.c | 2 +- drivers/gpu/drm/stm/drv.c | 2 +- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- drivers/gpu/drm/tidss/tidss_drv.c | 2 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- drivers/gpu/drm/tiny/arcpgu.c | 2 +- drivers/gpu/drm/tiny/bochs.c | 2 +- drivers/gpu/drm/tiny/cirrus.c | 2 +- drivers/gpu/drm/tiny/simpledrm.c | 2 +- drivers/gpu/drm/tve200/tve200_drv.c | 2 +- drivers/gpu/drm/vboxvideo/vbox_drv.c | 2 +- drivers/gpu/drm/vc4/vc4_drv.c | 2 +- drivers/gpu/drm/virtio/virtgpu_drv.c | 2 +- drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 +- include/drm/drm_fb_helper.h | 22 36 files changed, 81 insertions(+), 39 deletions(-) -- 2.35.1
[PATCH v2 3/3] drm: Allow simpledrm to setup its emulated FB as firmware provided
Indicate to fbdev subsystem that the registered framebuffer is provided by the system firmware, so that it can handle accordingly. For example, would unregister the FB devices if asked to remove the conflicting framebuffers. Add a new DRM_FB_FW field to drm_fbdev_generic_setup() options parameter. Drivers can use this to indicate the FB helper initialization that the FB registered is provided by the firmware, so it can be configured as such. Suggested-by: Thomas Zimmermann Signed-off-by: Javier Martinez Canillas Reviewed-by: Thomas Zimmermann --- (no changes since v1) drivers/gpu/drm/drm_fb_helper.c | 9 + drivers/gpu/drm/tiny/simpledrm.c | 2 +- include/drm/drm_fb_helper.h | 10 ++ 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index fd0084ad77c3..775e47c5de1f 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -1891,6 +1891,10 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper, /* don't leak any physical addresses to userspace */ info->flags |= FBINFO_HIDE_SMEM_START; + /* Indicate that the framebuffer is provided by the firmware */ + if (fb_helper->firmware) + info->flags |= FBINFO_MISC_FIRMWARE; + /* Need to drop locks to avoid recursive deadlock in * register_framebuffer. This is ok because the only thing left to do is * register the fbdev emulation instance in kernel_fb_helper_list. */ @@ -2512,6 +2516,8 @@ static const struct drm_client_funcs drm_fbdev_client_funcs = { * * * DRM_FB_BPP: bits per pixel for the device. If the field is not set, * @dev->mode_config.preferred_depth is used instead. + * * DRM_FB_FW: if the framebuffer for the device is provided by the + * system firmware. * * This function sets up generic fbdev emulation for drivers that supports * dumb buffers with a virtual address and that can be mmap'ed. @@ -2538,6 +2544,7 @@ void drm_fbdev_generic_setup(struct drm_device *dev, unsigned int options) { struct drm_fb_helper *fb_helper; unsigned int preferred_bpp = DRM_FB_GET_OPTION(DRM_FB_BPP, options); + bool firmware = DRM_FB_GET_OPTION(DRM_FB_FW, options); int ret; drm_WARN(dev, !dev->registered, "Device has not been registered.\n"); @@ -2570,6 +2577,8 @@ void drm_fbdev_generic_setup(struct drm_device *dev, unsigned int options) preferred_bpp = 32; fb_helper->preferred_bpp = preferred_bpp; + fb_helper->firmware = firmware; + ret = drm_fbdev_client_hotplug(&fb_helper->client); if (ret) drm_dbg_kms(dev, "client hotplug ret=%d\n", ret); diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index f5b8e864a5cd..5dcc21ea6180 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -901,7 +901,7 @@ static int simpledrm_probe(struct platform_device *pdev) if (ret) return ret; - drm_fbdev_generic_setup(dev, 0); + drm_fbdev_generic_setup(dev, DRM_FB_SET_OPTION(DRM_FB_FW, 1)); return 0; } diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 740f87560102..77a72d55308d 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -44,6 +44,7 @@ enum mode_set_atomic { }; #define DRM_FB_BPP_MASK GENMASK(7, 0) +#define DRM_FB_FW_MASK GENMASK(8, 8) /* Using the GNU statement expression extension */ #define DRM_FB_OPTION(option, value) \ @@ -197,6 +198,15 @@ struct drm_fb_helper { * See also: @deferred_setup */ int preferred_bpp; + + /** +* @firmware: +* +* Set if the driver indicates to the FB helper initialization that the +* framebuffer for the device being registered is provided by firmware, +* so that it can pass this on when registering the framebuffer device. +*/ + bool firmware; }; static inline struct drm_fb_helper * -- 2.35.1
[PATCH v2 1/3] drm: Remove superfluous arg when calling to drm_fbdev_generic_setup()
The drm_fbdev_generic_setup() function already sets the preferred bits per pixel for the device to dev->mode_config.preferred_depth, if preferred_bpp value is zero. Passing the same value to the function is unnecessary. Let's cleanup that in the two drivers that do it. Signed-off-by: Javier Martinez Canillas Reviewed-by: Thomas Zimmermann --- (no changes since v1) drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +- drivers/gpu/drm/tiny/cirrus.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c index fe4269c5aa0a..ace92459e462 100644 --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c @@ -349,7 +349,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, goto err_unload; } - drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth); + drm_fbdev_generic_setup(dev, 0); return 0; diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c index c8e791840862..ed5a2e14894a 100644 --- a/drivers/gpu/drm/tiny/cirrus.c +++ b/drivers/gpu/drm/tiny/cirrus.c @@ -601,7 +601,7 @@ static int cirrus_pci_probe(struct pci_dev *pdev, if (ret) return ret; - drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth); + drm_fbdev_generic_setup(dev, 0); return 0; } -- 2.35.1
Re: [PATCH] drm: drm_gem.h: Add explicit includes for DEFINE_DRM_GEM_FOPS
On Fri, 29 Apr 2022, Jeffrey Hugo wrote: > DEFINE_DRM_GEM_FOPS() references drm functions from other headers. For > example drm_open() is defined in drm_file.h and drm_ioctl() is defined > in drm_ioctl.h. Since drm_gem.h doesn't include these headers, it > relies on an implicit include from the .c file to have included these > required headers before DEFINE_DRM_GEM_FOPS() gets used. Relying on > these implicit includes can cause build failures for new code that > doesn't know about these requirements, and can lead to future problems > if the headers ever get restructured as there will be a need to update > every downstream file that includes drm_gem.h. > > Lets fix this explicitly including the required headers in drm_gem.h so > that code that includes drm_gem.h does not need to worry about these > implicit dependencies. In the general case, I tend to agree, but in this specific instance I think I'd err on the side of fewer includes. I think the more likely outcome here is accumulating implicit dependencies on symbols from drm_file.h and drm_ioctl.h by including drm_gem.h only! I do think headers need to be self-contained, and we actually enforce this in i915 (see HDRTEST in drivers/gpu/drm/i915/Makefile), but not to the point of macro expansions. BR, Jani. > > Signed-off-by: Jeffrey Hugo > --- > include/drm/drm_gem.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index 9d7c61a..1cbe3d8 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -37,6 +37,8 @@ > #include > #include > > +#include > +#include > #include > > struct iosys_map; -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH v12] drm/amdgpu: add drm buddy support to amdgpu
On 5/2/2022 8:41 PM, Mike Lothian wrote: On Wed, 27 Apr 2022 at 12:55, Mike Lothian wrote: On Tue, 26 Apr 2022 at 17:36, Christian König wrote: Hi Mike, sounds like somehow stitching together the SG table for PRIME doesn't work any more with this patch. Can you try with P2P DMA disabled? -CONFIG_PCI_P2PDMA=y +# CONFIG_PCI_P2PDMA is not set If that's what you're meaning, then there's no difference, I'll upload my dmesg to the gitlab issue Apart from that can you take a look Arun? Thanks, Christian. Hi Have you had any success in replicating this? Hi Mike, I couldn't replicate on my Raven APU machine. I see you have 2 cards initialized, one is Renoir and the other is Navy Flounder. Could you give some more details, are you running Gravity Mark on Renoir and what is your system RAM configuration? Cheers Mike
Re: [PATCH v2 1/3] drm: Remove superfluous arg when calling to drm_fbdev_generic_setup()
Hi Javier, Thank you for the patch. On Mon, May 02, 2022 at 05:38:58PM +0200, Javier Martinez Canillas wrote: > The drm_fbdev_generic_setup() function already sets the preferred bits per > pixel for the device to dev->mode_config.preferred_depth, if preferred_bpp > value is zero. > > Passing the same value to the function is unnecessary. Let's cleanup that > in the two drivers that do it. This looks fine, so Reviewed-by: Laurent Pinchart but why do we have two different mechanisms to set the preferred depth ? Could we get all drivers to set dev->mode_config.preferred_depth and drop the depth option to drm_fbdev_generic_setup() ? There's a FIXME comment in drm_fbdev_generic_setup() that could be related. > Signed-off-by: Javier Martinez Canillas > Reviewed-by: Thomas Zimmermann > --- > > (no changes since v1) > > drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +- > drivers/gpu/drm/tiny/cirrus.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > index fe4269c5aa0a..ace92459e462 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > @@ -349,7 +349,7 @@ static int hibmc_pci_probe(struct pci_dev *pdev, > goto err_unload; > } > > - drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth); > + drm_fbdev_generic_setup(dev, 0); > > return 0; > > diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c > index c8e791840862..ed5a2e14894a 100644 > --- a/drivers/gpu/drm/tiny/cirrus.c > +++ b/drivers/gpu/drm/tiny/cirrus.c > @@ -601,7 +601,7 @@ static int cirrus_pci_probe(struct pci_dev *pdev, > if (ret) > return ret; > > - drm_fbdev_generic_setup(dev, dev->mode_config.preferred_depth); > + drm_fbdev_generic_setup(dev, 0); > return 0; > } > -- Regards, Laurent Pinchart
Re: [PATCH v2 2/3] drm/fb-helper: Rename preferred_bpp drm_fbdev_generic_setup() parameter
Hi Javier, Thank you for the patch. On Mon, May 02, 2022 at 05:38:59PM +0200, Javier Martinez Canillas wrote: > By default the bits per pixel for the emulated framebuffer device is set > to dev->mode_config.preferred_depth, but some devices need another value. > > Since this second parameter is only used by a few drivers, and to allow > drivers to use it for passing other configurations when registering the > fbdev, rename @preferred_bpp to @options and make it a multi-field param. > > The DRM_FB_OPTION() and DRM_FB_GET_OPTION() macros are provided to drivers > for computing options bitfield values and getting the values respectively > > For now, only the DRM_FB_BPP option exists but other options can be added. > > Suggested-by: Thomas Zimmermann > Signed-off-by: Javier Martinez Canillas > Reviewed-by: Thomas Zimmermann > Reviewed-by: Laurent Pinchart > --- > > Changes in v2: > - Rename DRM_FB_SET_OPTION() to DRM_FB_SET() and make more clear in the I assume you meant DRM_FB_OPTION() here, not DRM_FB_SET(). > kernel-doc what this macro does (Laurent Pinchart). > - Fix some kernel-doc issues I didn't notice in v1. > - Add Reviewed-by tags from Thomas and Laurent. > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 6 -- > drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- > drivers/gpu/drm/arm/malidp_drv.c| 2 +- > drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 2 +- > drivers/gpu/drm/ast/ast_drv.c | 2 +- > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 2 +- > drivers/gpu/drm/drm_drv.c | 2 +- > drivers/gpu/drm/drm_fb_helper.c | 17 + > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- > drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +- > drivers/gpu/drm/imx/dcss/dcss-kms.c | 2 +- > drivers/gpu/drm/imx/imx-drm-core.c | 2 +- > drivers/gpu/drm/ingenic/ingenic-drm-drv.c | 2 +- > drivers/gpu/drm/mcde/mcde_drv.c | 2 +- > drivers/gpu/drm/mediatek/mtk_drm_drv.c | 2 +- > drivers/gpu/drm/meson/meson_drv.c | 2 +- > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 2 +- > drivers/gpu/drm/pl111/pl111_drv.c | 2 +- > drivers/gpu/drm/qxl/qxl_drv.c | 2 +- > drivers/gpu/drm/rcar-du/rcar_du_drv.c | 2 +- > drivers/gpu/drm/sti/sti_drv.c | 2 +- > drivers/gpu/drm/stm/drv.c | 2 +- > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- > drivers/gpu/drm/tidss/tidss_drv.c | 2 +- > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +- > drivers/gpu/drm/tiny/arcpgu.c | 2 +- > drivers/gpu/drm/tiny/bochs.c| 2 +- > drivers/gpu/drm/tve200/tve200_drv.c | 2 +- > drivers/gpu/drm/vboxvideo/vbox_drv.c| 2 +- > drivers/gpu/drm/vc4/vc4_drv.c | 2 +- > drivers/gpu/drm/virtio/virtgpu_drv.c| 2 +- > drivers/gpu/drm/xlnx/zynqmp_dpsub.c | 2 +- > include/drm/drm_fb_helper.h | 12 > 33 files changed, 59 insertions(+), 36 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > index b03663f42cc9..0c54470975e3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c > @@ -2118,9 +2118,11 @@ static int amdgpu_pci_probe(struct pci_dev *pdev, > !list_empty(&adev_to_drm(adev)->mode_config.connector_list)) { > /* select 8 bpp console on low vram cards */ > if (adev->gmc.real_vram_size <= (32*1024*1024)) > - drm_fbdev_generic_setup(adev_to_drm(adev), 8); > + drm_fbdev_generic_setup(adev_to_drm(adev), > + DRM_FB_OPTION(DRM_FB_BPP, 8)); > else > - drm_fbdev_generic_setup(adev_to_drm(adev), 32); > + drm_fbdev_generic_setup(adev_to_drm(adev), > + DRM_FB_OPTION(DRM_FB_BPP, 32)); > } > > ret = amdgpu_debugfs_init(adev); > diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c > index e89ae0ec60eb..b69b1e5be379 100644 > --- a/drivers/gpu/drm/arm/hdlcd_drv.c > +++ b/drivers/gpu/drm/arm/hdlcd_drv.c > @@ -321,7 +321,7 @@ static int hdlcd_drm_bind(struct device *dev) > if (ret) > goto err_register; > > - drm_fbdev_generic_setup(drm, 32); > + drm_fbdev_generic_setup(drm, DRM_FB_OPTION(DRM_FB_BPP, 32)); > > return 0; > > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > b/drivers/gpu/drm/arm/malidp_drv.c > index d5aef21426cf..25685b579a05 100644 > --- a/drivers/gpu/drm/arm/malidp_drv.c > +++ b/drivers/gpu/drm/arm/malidp_drv.c > @@ -863,7 +863,7 @@ static int malidp_bind(struct device *dev) >
Re: [Freedreno] [PATCH] drm/msm/disp/dpu1: avoid clearing hw interrupts if hw_intr is null during drm uninit
On 5/2/2022 1:05 AM, Dmitry Baryshkov wrote: On Mon, 2 May 2022 at 04:38, Abhinav Kumar wrote: Looks like our new CI has given all the answers we need :) which is a great win for the CI in my opinion. Take a look at this report : https://gitlab.freedesktop.org/drm/msm/-/jobs/22015361 This issue seems to be because this change https://github.com/torvalds/linux/commit/169466d4e59ca204683998b7f45673ebf0eb2de6 is missing in our tree. Without this change, what happens is that we are not hitting the return 0 because we check for ENODEV. /* * External bridges are mandatory for eDP interfaces: one has to * provide at least an eDP panel (which gets wrapped into panel-bridge). * * For DisplayPort interfaces external bridges are optional, so * silently ignore an error if one is not present (-ENODEV). */ rc = dp_parser_find_next_bridge(dp_priv->parser); if (!dp->is_edp && rc == -ENODEV) return 0; So, I think we should do both: 1) Since we are running CI on the tree, backport this change so that this error path doesnt hit? 2) Add this protection as well because this shows that we can indeed hit this path in EDEFER cases causing this crash. I have been waiting for v2 for the last week or so. It should include a fixed Fixes tag and an updated description (which should note that this happens in the error path, etc) as requested by Stephen. Prior to the above CI report, we did not know what is the error path in which this happens and why. Till then we were just speculating. Now that we do, we can certainly add it and post a v2. Thanks Abhinav On 4/27/2022 3:53 AM, Dmitry Baryshkov wrote: On 27/04/2022 00:50, Stephen Boyd wrote: Quoting Vinod Polimera (2022-04-25 23:02:11) Avoid clearing irqs and derefernce hw_intr when hw_intr is null. Presumably this is only the case when the display driver doesn't fully probe and something probe defers? Can you clarify how this situation happens? BUG: Unable to handle kernel NULL pointer dereference at virtual address Call trace: dpu_core_irq_uninstall+0x50/0xb0 dpu_irq_uninstall+0x18/0x24 msm_drm_uninit+0xd8/0x16c msm_drm_bind+0x580/0x5fc try_to_bring_up_master+0x168/0x1c0 __component_add+0xb4/0x178 component_add+0x1c/0x28 dp_display_probe+0x38c/0x400 platform_probe+0xb0/0xd0 really_probe+0xcc/0x2c8 __driver_probe_device+0xbc/0xe8 driver_probe_device+0x48/0xf0 __device_attach_driver+0xa0/0xc8 bus_for_each_drv+0x8c/0xd8 __device_attach+0xc4/0x150 device_initial_probe+0x1c/0x28 Fixes: a73033619ea ("drm/msm/dpu: squash dpu_core_irq into dpu_hw_interrupts") The fixes tag looks odd. In dpu_core_irq_uninstall() at that commit it is dealing with 'irq_obj' which isn't a pointer. After commit f25f656608e3 ("drm/msm/dpu: merge struct dpu_irq into struct dpu_hw_intr") dpu_core_irq_uninstall() starts using 'hw_intr' which is allocated on the heap. If we backported this patch to a place that had a73033619ea without f25f656608e3 it wouldn't make any sense. I'd agree here. The following tag would be correct: Fixes: f25f656608e3 ("drm/msm/dpu: merge struct dpu_irq into struct dpu_hw_intr") Signed-off-by: Vinod Polimera --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c index c515b7c..ab28577 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_interrupts.c @@ -599,6 +599,9 @@ void dpu_core_irq_uninstall(struct dpu_kms *dpu_kms) { int i; + if (!dpu_kms->hw_intr) + return; + pm_runtime_get_sync(&dpu_kms->pdev->dev); for (i = 0; i < dpu_kms->hw_intr->total_irqs; i++)
Re: [PATCH v2 3/3] drm: Allow simpledrm to setup its emulated FB as firmware provided
Hi Javier, Thank you for the patch. On Mon, May 02, 2022 at 05:39:00PM +0200, Javier Martinez Canillas wrote: > Indicate to fbdev subsystem that the registered framebuffer is provided by > the system firmware, so that it can handle accordingly. For example, would > unregister the FB devices if asked to remove the conflicting framebuffers. > > Add a new DRM_FB_FW field to drm_fbdev_generic_setup() options parameter. > Drivers can use this to indicate the FB helper initialization that the FB > registered is provided by the firmware, so it can be configured as such. > > Suggested-by: Thomas Zimmermann > Signed-off-by: Javier Martinez Canillas > Reviewed-by: Thomas Zimmermann > --- > > (no changes since v1) > > drivers/gpu/drm/drm_fb_helper.c | 9 + > drivers/gpu/drm/tiny/simpledrm.c | 2 +- > include/drm/drm_fb_helper.h | 10 ++ > 3 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index fd0084ad77c3..775e47c5de1f 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -1891,6 +1891,10 @@ __drm_fb_helper_initial_config_and_unlock(struct > drm_fb_helper *fb_helper, > /* don't leak any physical addresses to userspace */ > info->flags |= FBINFO_HIDE_SMEM_START; > > + /* Indicate that the framebuffer is provided by the firmware */ > + if (fb_helper->firmware) > + info->flags |= FBINFO_MISC_FIRMWARE; > + > /* Need to drop locks to avoid recursive deadlock in >* register_framebuffer. This is ok because the only thing left to do is >* register the fbdev emulation instance in kernel_fb_helper_list. */ > @@ -2512,6 +2516,8 @@ static const struct drm_client_funcs > drm_fbdev_client_funcs = { > * > * * DRM_FB_BPP: bits per pixel for the device. If the field is not set, > * @dev->mode_config.preferred_depth is used instead. > + * * DRM_FB_FW: if the framebuffer for the device is provided by the > + * system firmware. > * > * This function sets up generic fbdev emulation for drivers that supports > * dumb buffers with a virtual address and that can be mmap'ed. > @@ -2538,6 +2544,7 @@ void drm_fbdev_generic_setup(struct drm_device *dev, > unsigned int options) > { > struct drm_fb_helper *fb_helper; > unsigned int preferred_bpp = DRM_FB_GET_OPTION(DRM_FB_BPP, options); > + bool firmware = DRM_FB_GET_OPTION(DRM_FB_FW, options); > int ret; > > drm_WARN(dev, !dev->registered, "Device has not been registered.\n"); > @@ -2570,6 +2577,8 @@ void drm_fbdev_generic_setup(struct drm_device *dev, > unsigned int options) > preferred_bpp = 32; > fb_helper->preferred_bpp = preferred_bpp; > > + fb_helper->firmware = firmware; I'd get rid of the local variable and write fb_helper->firmware = DRM_FB_GET_OPTION(DRM_FB_FW, options); Reviewed-by: Laurent Pinchart > + > ret = drm_fbdev_client_hotplug(&fb_helper->client); > if (ret) > drm_dbg_kms(dev, "client hotplug ret=%d\n", ret); > diff --git a/drivers/gpu/drm/tiny/simpledrm.c > b/drivers/gpu/drm/tiny/simpledrm.c > index f5b8e864a5cd..5dcc21ea6180 100644 > --- a/drivers/gpu/drm/tiny/simpledrm.c > +++ b/drivers/gpu/drm/tiny/simpledrm.c > @@ -901,7 +901,7 @@ static int simpledrm_probe(struct platform_device *pdev) > if (ret) > return ret; > > - drm_fbdev_generic_setup(dev, 0); > + drm_fbdev_generic_setup(dev, DRM_FB_SET_OPTION(DRM_FB_FW, 1)); > > return 0; > } > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > index 740f87560102..77a72d55308d 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -44,6 +44,7 @@ enum mode_set_atomic { > }; > > #define DRM_FB_BPP_MASK GENMASK(7, 0) > +#define DRM_FB_FW_MASK GENMASK(8, 8) > > /* Using the GNU statement expression extension */ > #define DRM_FB_OPTION(option, value) \ > @@ -197,6 +198,15 @@ struct drm_fb_helper { >* See also: @deferred_setup >*/ > int preferred_bpp; > + > + /** > + * @firmware: > + * > + * Set if the driver indicates to the FB helper initialization that the > + * framebuffer for the device being registered is provided by firmware, > + * so that it can pass this on when registering the framebuffer device. > + */ > + bool firmware; > }; > > static inline struct drm_fb_helper * -- Regards, Laurent Pinchart
Re: [PATCH] drm/msm/dpu: don't access mode pointer before it is set
On 5/2/2022 1:24 AM, Dmitry Baryshkov wrote: Move the initializer for the mode variable to the declaration point to remove unitialized variable access from the DEBUG_DPU macro. This fixes the following warning: drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c:250:37: note: initialize the variable 'mode' to silence this warning Fixes: d7d0e73f7de3 ("drm/msm/dpu: introduce the dpu_encoder_phys_* for writeback") Reported-by: kernel test robot Signed-off-by: Dmitry Baryshkov Thanks, Reviewed-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c index f4a79715a02e..4829d1ce0cf8 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c @@ -247,7 +247,7 @@ static int dpu_encoder_phys_wb_atomic_check( struct drm_connector_state *conn_state) { struct drm_framebuffer *fb; - const struct drm_display_mode *mode; + const struct drm_display_mode *mode = &crtc_state->mode; DPU_DEBUG("[atomic_check:%d, \"%s\",%d,%d]\n", phys_enc->wb_idx, mode->name, mode->hdisplay, mode->vdisplay); @@ -256,7 +256,6 @@ static int dpu_encoder_phys_wb_atomic_check( return 0; fb = conn_state->writeback_job->fb; - mode = &crtc_state->mode; if (!conn_state || !conn_state->connector) { DPU_ERROR("invalid connector state\n");
Re: [PATCH v2 7/7] vfio: Remove calls to vfio_group_add_container_user()
On Fri, Apr 29, 2022 at 02:28:20PM -0600, Alex Williamson wrote: > On Thu, 21 Apr 2022 13:28:38 -0300 > Jason Gunthorpe wrote: > > > When the open_device() op is called the container_users is incremented and > > held incremented until close_device(). Thus, so long as drivers call > > functions within their open_device()/close_device() region they do not > > need to worry about the container_users. > > > > These functions can all only be called between open_device() and > > close_device(): > > > > vfio_pin_pages() > > vfio_unpin_pages() > > vfio_dma_rw() > > vfio_register_notifier() > > vfio_unregister_notifier() > > > > Eliminate the calls to vfio_group_add_container_user() and add > > vfio_assert_device_open() to detect driver mis-use. > > A comment here explaining that decrementing open_count is pushed until > after close_device to support this feature would help to explain the > somewhat subtle change in vfio_group_get_device_fd(). I changed it like this: Eliminate the calls to vfio_group_add_container_user() and add vfio_assert_device_open() to detect driver mis-use. This causes the close_device() op to check device->open_count so always leave it elevated while calling the op. Thanks, Jason
Re: [PATCH v2 00/48] ARM: PXA multiplatform support
On 4/30/22 07:23, Arnd Bergmann wrote: On Sat, Apr 30, 2022 at 3:32 PM Arnd Bergmann wrote: On Sat, Apr 30, 2022 at 2:41 PM Guenter Roeck wrote: On 4/30/22 01:04, Arnd Bergmann wrote: and concluded that it must have done this for a long time. In my own qemu instance, I see a crash from iWMMXt, but that works fine on your machine. OTOH, your failed instances all look like they either time out or failed to find a rootfs. I tried passing an MMC device as root, and that works here. Booting from mmc works for me as well. Booting from pcmcia worked before, so I assume that there must be some regression. Ok, got it, and managed to reproduce the hang now. My "ARM: pxa/sa1100: move I/O space to PCI_IOBASE" patch managed to get it to the point of detecting the pcmcia device instead of crashing, so I assumed it was enough when it clearly was not. Before that patch, it still works, afterwards it hangs with "pata_pcmcia: probe of 0.0 failed with error -12" as mentioned above. I'll have another look. Got it: as the PCMCIA bus on this machine is the only thing with an I/O space, I assigned it port number range 0-0x1000, with an io_offset of 0, but this was apparently unexpected and triggered this sanity check: static int static_find_io(struct pcmcia_socket *s, unsigned int attr, unsigned int *base, unsigned int num, unsigned int align, struct resource **parent) { if (!s->io_offset) return -EINVAL; ... return 0; } I moved the devices around now, giving zeus/viper I/O space an offset of zero, and moving PCMCIA to offset 0x1 and 0x11000 for the two slots, which now works because the io_offset is nonzero. I've regenerated the branches again, and confirmed the for-next branch still boots from pcmcia. With v5.18-rc1-49-gcb813018b5c1, I still get: [0.797668] RAMDISK: Couldn't find valid RAM disk image starting at 0. [0.805262] /dev/root: Can't open blockdev [0.805487] VFS: Cannot open root device "(null)" or unknown-block(0,0): error -6 [0.805674] Please append a correct "root=" boot option; here are the available partitions: when trying to boot z2 from initrd. The other problems are gone. Guenter
Re: [PATCH v12] drm/amdgpu: add drm buddy support to amdgpu
On Mon, 2 May 2022 at 16:54, Arunpravin Paneer Selvam wrote: > > > > On 5/2/2022 8:41 PM, Mike Lothian wrote: > > On Wed, 27 Apr 2022 at 12:55, Mike Lothian wrote: > >> On Tue, 26 Apr 2022 at 17:36, Christian König > >> wrote: > >>> Hi Mike, > >>> > >>> sounds like somehow stitching together the SG table for PRIME doesn't > >>> work any more with this patch. > >>> > >>> Can you try with P2P DMA disabled? > >> -CONFIG_PCI_P2PDMA=y > >> +# CONFIG_PCI_P2PDMA is not set > >> > >> If that's what you're meaning, then there's no difference, I'll upload > >> my dmesg to the gitlab issue > >> > >>> Apart from that can you take a look Arun? > >>> > >>> Thanks, > >>> Christian. > > Hi > > > > Have you had any success in replicating this? > Hi Mike, > I couldn't replicate on my Raven APU machine. I see you have 2 cards > initialized, one is Renoir > and the other is Navy Flounder. Could you give some more details, are > you running Gravity Mark > on Renoir and what is your system RAM configuration? > > > > Cheers > > > > Mike > Hi It's a PRIME laptop, it failed on the RENOIR too, it caused a lockup, but systemd managed to capture it, I'll attach it to the issue I've got 64GB RAM, the 6800M has 12GB VRAM Cheers Mike
[PATCH 00/11] i915: Introduce Ponte Vecchio
Ponte Vecchio (PVC) is a new GPU based on the Xe_HPC architecture. As a compute-focused platform, PVC has compute engines and enhanced copy engines, but no render engine (there is no geometry pipeline) and no display. This is just a handful of early enablement patches, including some initial support for the new copy engines (although we're not yet adding those to the platform's engine list or exposing them to userspace just yet). Ayaz A Siddiqui (1): drm/i915/pvc: Define MOCS table for PVC John Harrison (1): drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter engine Lucas De Marchi (2): drm/i915/pvc: skip all copy engines from aux table invalidate drm/i915/pvc: read fuses for link copy engines Matt Roper (5): drm/i915/pvc: Add forcewake support drm/i915/pvc: Read correct RP_STATE_CAP register drm/i915/pvc: Engines definitions for new copy engines drm/i915/pvc: Interrupt support for new copy engines drm/i915/pvc: Reset support for new copy engines Stuart Summers (2): drm/i915/pvc: add initial Ponte Vecchio definitions drm/i915/pvc: Remove additional 3D flags from PIPE_CONTROL drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 20 ++- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 92 +++ drivers/gpu/drm/i915/gt/intel_engine_types.h | 10 +- drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 12 +- drivers/gpu/drm/i915/gt/intel_gt_irq.c| 16 ++ drivers/gpu/drm/i915/gt/intel_gt_regs.h | 56 --- drivers/gpu/drm/i915/gt/intel_gt_types.h | 1 + drivers/gpu/drm/i915/gt/intel_mocs.c | 24 ++- drivers/gpu/drm/i915/gt/intel_rps.c | 4 +- drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 +- drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 9 +- drivers/gpu/drm/i915/gvt/cmd_parser.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 6 + drivers/gpu/drm/i915/i915_pci.c | 23 +++ drivers/gpu/drm/i915/i915_reg.h | 9 ++ drivers/gpu/drm/i915/intel_device_info.c | 1 + drivers/gpu/drm/i915/intel_device_info.h | 5 +- drivers/gpu/drm/i915/intel_uncore.c | 150 +- drivers/gpu/drm/i915/selftests/intel_uncore.c | 2 + 19 files changed, 417 insertions(+), 38 deletions(-) -- 2.35.1
[PATCH 06/11] drm/i915/pvc: Reduce stack usage in reset selftest with extra blitter engine
From: John Harrison PVC adds extra blitter engines (in the following patch). The reset selftest has a local array on the stack which is sized by the number of engines. The increase pushes the size of this array to the point where it trips the 'stack too large' compile warning. This patch takes the allocation of the stack and makes it dynamic instead. Signed-off-by: John Harrison Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/selftest_hangcheck.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c index 83ff4c2e57c5..3b9d82276db2 100644 --- a/drivers/gpu/drm/i915/gt/selftest_hangcheck.c +++ b/drivers/gpu/drm/i915/gt/selftest_hangcheck.c @@ -979,6 +979,7 @@ static int __igt_reset_engines(struct intel_gt *gt, enum intel_engine_id id, tmp; struct hang h; int err = 0; + struct active_engine *threads; /* Check that issuing a reset on one engine does not interfere * with any other engine. @@ -996,8 +997,11 @@ static int __igt_reset_engines(struct intel_gt *gt, h.ctx->sched.priority = 1024; } + threads = kzalloc(sizeof(*threads) * I915_NUM_ENGINES, GFP_KERNEL); + if (!threads) + return -ENOMEM; + for_each_engine(engine, gt, id) { - struct active_engine threads[I915_NUM_ENGINES] = {}; unsigned long device = i915_reset_count(global); unsigned long count = 0, reported; bool using_guc = intel_engine_uses_guc(engine); @@ -1016,7 +1020,7 @@ static int __igt_reset_engines(struct intel_gt *gt, break; } - memset(threads, 0, sizeof(threads)); + memset(threads, 0, sizeof(*threads) * I915_NUM_ENGINES); for_each_engine(other, gt, tmp) { struct task_struct *tsk; @@ -1236,6 +1240,7 @@ static int __igt_reset_engines(struct intel_gt *gt, break; } } + kfree(threads); if (intel_gt_is_wedged(gt)) err = -EIO; -- 2.35.1
[PATCH 03/11] drm/i915/pvc: Define MOCS table for PVC
From: Ayaz A Siddiqui Bspec: 45101, 72161 Signed-off-by: Ayaz A Siddiqui Signed-off-by: Fei Yang Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_gt_types.h| 1 + drivers/gpu/drm/i915/gt/intel_mocs.c| 24 - drivers/gpu/drm/i915/gt/intel_workarounds.c | 13 --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_pci.c | 3 ++- drivers/gpu/drm/i915/intel_device_info.h| 1 + 6 files changed, 39 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index b06611c1d4ad..7853ea194ea6 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -221,6 +221,7 @@ struct intel_gt { struct { u8 uc_index; + u8 wb_index; /* Only for platforms listed in Bspec: 72161 */ } mocs; struct intel_pxp pxp; diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c index c4c37585ae8c..265812589f87 100644 --- a/drivers/gpu/drm/i915/gt/intel_mocs.c +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c @@ -23,6 +23,7 @@ struct drm_i915_mocs_table { unsigned int n_entries; const struct drm_i915_mocs_entry *table; u8 uc_index; + u8 wb_index; /* Only for platforms listed in Bspec: 72161 */ u8 unused_entries_index; }; @@ -47,6 +48,7 @@ struct drm_i915_mocs_table { /* Helper defines */ #define GEN9_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but configured. */ +#define PVC_NUM_MOCS_ENTRIES 3 /* (e)LLC caching options */ /* @@ -394,6 +396,17 @@ static const struct drm_i915_mocs_entry dg2_mocs_table_g10_ax[] = { MOCS_ENTRY(3, 0, L3_3_WB | L3_LKUP(1)), }; +static const struct drm_i915_mocs_entry pvc_mocs_table[] = { + /* Error */ + MOCS_ENTRY(0, 0, L3_3_WB), + + /* UC */ + MOCS_ENTRY(1, 0, L3_1_UC), + + /* WB */ + MOCS_ENTRY(2, 0, L3_3_WB), +}; + enum { HAS_GLOBAL_MOCS = BIT(0), HAS_ENGINE_MOCS = BIT(1), @@ -423,7 +436,14 @@ static unsigned int get_mocs_settings(const struct drm_i915_private *i915, memset(table, 0, sizeof(struct drm_i915_mocs_table)); table->unused_entries_index = I915_MOCS_PTE; - if (IS_DG2(i915)) { + if (IS_PONTEVECCHIO(i915)) { + table->size = ARRAY_SIZE(pvc_mocs_table); + table->table = pvc_mocs_table; + table->n_entries = PVC_NUM_MOCS_ENTRIES; + table->uc_index = 1; + table->wb_index = 2; + table->unused_entries_index = 2; + } else if (IS_DG2(i915)) { if (IS_DG2_GRAPHICS_STEP(i915, G10, STEP_A0, STEP_B0)) { table->size = ARRAY_SIZE(dg2_mocs_table_g10_ax); table->table = dg2_mocs_table_g10_ax; @@ -622,6 +642,8 @@ void intel_set_mocs_index(struct intel_gt *gt) get_mocs_settings(gt->i915, &table); gt->mocs.uc_index = table.uc_index; + if (HAS_L3_CCS_READ(gt->i915)) + gt->mocs.wb_index = table.wb_index; } void intel_mocs_init(struct intel_gt *gt) diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c index a05c4b99b3fb..a656d9c2ca2b 100644 --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c @@ -1994,7 +1994,7 @@ void intel_engine_apply_whitelist(struct intel_engine_cs *engine) static void engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) { - u8 mocs; + u8 mocs_w, mocs_r; /* * RING_CMD_CCTL are need to be programed to un-cached @@ -2002,11 +2002,18 @@ engine_fake_wa_init(struct intel_engine_cs *engine, struct i915_wa_list *wal) * Streamers on Gen12 onward platforms. */ if (GRAPHICS_VER(engine->i915) >= 12) { - mocs = engine->gt->mocs.uc_index; + if (HAS_L3_CCS_READ(engine->i915) && + engine->class == COMPUTE_CLASS) + mocs_r = engine->gt->mocs.wb_index; + else + mocs_r = engine->gt->mocs.uc_index; + + mocs_w = engine->gt->mocs.uc_index; + wa_masked_field_set(wal, RING_CMD_CCTL(engine->mmio_base), CMD_CCTL_MOCS_MASK, - CMD_CCTL_MOCS_OVERRIDE(mocs, mocs)); + CMD_CCTL_MOCS_OVERRIDE(mocs_w, mocs_r)); } } diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 2dddc27a1b0e..8c8e7308502b 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1369,6 +1369,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define HAS_LSPCON(dev_priv) (IS_DISPLAY_VER(dev_priv, 9, 10))
[PATCH 04/11] drm/i915/pvc: Read correct RP_STATE_CAP register
The SoC registers, including RP_STATE_CAP, have moved to a new location in GTTMMADR on Ponte Vecchio. We need to update the register offset accordingly. Cc: Rodrigo Vivi Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_rps.c | 4 +++- drivers/gpu/drm/i915/i915_reg.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c index 3476a11f294c..3bd8415a0f1b 100644 --- a/drivers/gpu/drm/i915/gt/intel_rps.c +++ b/drivers/gpu/drm/i915/gt/intel_rps.c @@ -1075,7 +1075,9 @@ static u32 intel_rps_read_state_cap(struct intel_rps *rps) struct drm_i915_private *i915 = rps_to_i915(rps); struct intel_uncore *uncore = rps_to_uncore(rps); - if (IS_XEHPSDV(i915)) + if (IS_PONTEVECCHIO(i915)) + return intel_uncore_read(uncore, PVC_RP_STATE_CAP); + else if (IS_XEHPSDV(i915)) return intel_uncore_read(uncore, XEHPSDV_RP_STATE_CAP); else if (IS_GEN9_LP(i915)) return intel_uncore_read(uncore, BXT_RP_STATE_CAP); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9ccb67eec1bd..4a3d7b96ef43 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1846,6 +1846,7 @@ #define BXT_RP_STATE_CAP_MMIO(0x138170) #define GEN9_RP_STATE_LIMITS _MMIO(0x138148) #define XEHPSDV_RP_STATE_CAP _MMIO(0x250014) +#define PVC_RP_STATE_CAP _MMIO(0x281014) #define GT0_PERF_LIMIT_REASONS _MMIO(0x1381a8) #define GT0_PERF_LIMIT_REASONS_MASK 0xde3 -- 2.35.1
[PATCH 01/11] drm/i915/pvc: add initial Ponte Vecchio definitions
From: Stuart Summers Additional blitter and media engines will be enabled later. Bspec: 44481, 44482 Signed-off-by: Stuart Summers Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_pci.c | 21 + drivers/gpu/drm/i915/intel_device_info.c | 1 + drivers/gpu/drm/i915/intel_device_info.h | 1 + 4 files changed, 25 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 24111bf42ce0..2dddc27a1b0e 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1062,6 +1062,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define IS_ALDERLAKE_P(dev_priv) IS_PLATFORM(dev_priv, INTEL_ALDERLAKE_P) #define IS_XEHPSDV(dev_priv) IS_PLATFORM(dev_priv, INTEL_XEHPSDV) #define IS_DG2(dev_priv) IS_PLATFORM(dev_priv, INTEL_DG2) +#define IS_PONTEVECCHIO(dev_priv) IS_PLATFORM(dev_priv, INTEL_PONTEVECCHIO) + #define IS_DG2_G10(dev_priv) \ IS_SUBPLATFORM(dev_priv, INTEL_DG2, INTEL_SUBPLATFORM_G10) #define IS_DG2_G11(dev_priv) \ diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 7739d6c33481..498708b33924 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1074,6 +1074,27 @@ static const struct intel_device_info ats_m_info = { .require_force_probe = 1, }; +#define XE_HPC_FEATURES \ + XE_HP_FEATURES, \ + .dma_mask_size = 52 + +__maybe_unused +static const struct intel_device_info pvc_info = { + XE_HPC_FEATURES, + XE_HPM_FEATURES, + DGFX_FEATURES, + .graphics.rel = 60, + .media.rel = 60, + PLATFORM(INTEL_PONTEVECCHIO), + .display = { 0 }, + .has_flat_ccs = 0, + .platform_engine_mask = + BIT(BCS0) | + BIT(VCS0) | + BIT(CCS0) | BIT(CCS1) | BIT(CCS2) | BIT(CCS3), + .require_force_probe = 1, +}; + #undef PLATFORM /* diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c index 63e05cd15a90..f0bf23726ed8 100644 --- a/drivers/gpu/drm/i915/intel_device_info.c +++ b/drivers/gpu/drm/i915/intel_device_info.c @@ -72,6 +72,7 @@ static const char * const platform_names[] = { PLATFORM_NAME(ALDERLAKE_P), PLATFORM_NAME(XEHPSDV), PLATFORM_NAME(DG2), + PLATFORM_NAME(PONTEVECCHIO), }; #undef PLATFORM_NAME diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 20c351c8d5bd..e7d2cf7d65c8 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -88,6 +88,7 @@ enum intel_platform { INTEL_ALDERLAKE_P, INTEL_XEHPSDV, INTEL_DG2, + INTEL_PONTEVECCHIO, INTEL_MAX_PLATFORMS }; -- 2.35.1
[PATCH 02/11] drm/i915/pvc: Add forcewake support
Add PVC's forcewake ranges. Bspec: 67609 Cc: Daniele Ceraolo Spurio Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/intel_uncore.c | 150 +- drivers/gpu/drm/i915/selftests/intel_uncore.c | 2 + 2 files changed, 151 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 83517a703eb6..3352065635e8 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1080,6 +1080,45 @@ static const struct i915_range dg2_shadowed_regs[] = { { .start = 0x1F8510, .end = 0x1F8550 }, }; +static const struct i915_range pvc_shadowed_regs[] = { + { .start = 0x2030, .end = 0x2030 }, + { .start = 0x2510, .end = 0x2550 }, + { .start = 0xA008, .end = 0xA00C }, + { .start = 0xA188, .end = 0xA188 }, + { .start = 0xA278, .end = 0xA278 }, + { .start = 0xA540, .end = 0xA56C }, + { .start = 0xC4C8, .end = 0xC4C8 }, + { .start = 0xC4E0, .end = 0xC4E0 }, + { .start = 0xC600, .end = 0xC600 }, + { .start = 0xC658, .end = 0xC658 }, + { .start = 0x22030, .end = 0x22030 }, + { .start = 0x22510, .end = 0x22550 }, + { .start = 0x1C0030, .end = 0x1C0030 }, + { .start = 0x1C0510, .end = 0x1C0550 }, + { .start = 0x1C4030, .end = 0x1C4030 }, + { .start = 0x1C4510, .end = 0x1C4550 }, + { .start = 0x1C8030, .end = 0x1C8030 }, + { .start = 0x1C8510, .end = 0x1C8550 }, + { .start = 0x1D0030, .end = 0x1D0030 }, + { .start = 0x1D0510, .end = 0x1D0550 }, + { .start = 0x1D4030, .end = 0x1D4030 }, + { .start = 0x1D4510, .end = 0x1D4550 }, + { .start = 0x1D8030, .end = 0x1D8030 }, + { .start = 0x1D8510, .end = 0x1D8550 }, + { .start = 0x1E0030, .end = 0x1E0030 }, + { .start = 0x1E0510, .end = 0x1E0550 }, + { .start = 0x1E4030, .end = 0x1E4030 }, + { .start = 0x1E4510, .end = 0x1E4550 }, + { .start = 0x1E8030, .end = 0x1E8030 }, + { .start = 0x1E8510, .end = 0x1E8550 }, + { .start = 0x1F0030, .end = 0x1F0030 }, + { .start = 0x1F0510, .end = 0x1F0550 }, + { .start = 0x1F4030, .end = 0x1F4030 }, + { .start = 0x1F4510, .end = 0x1F4550 }, + { .start = 0x1F8030, .end = 0x1F8030 }, + { .start = 0x1F8510, .end = 0x1F8550 }, +}; + static int mmio_range_cmp(u32 key, const struct i915_range *range) { if (key < range->start) @@ -1490,6 +1529,111 @@ static const struct intel_forcewake_range __dg2_fw_ranges[] = { XEHP_FWRANGES(FORCEWAKE_RENDER) }; +/* + * *Must* be sorted by offset ranges! See intel_fw_table_check(). + * + * Note that the spec lists several reserved/unused ranges that don't actually + * contain any registers. In the table below we'll combine those reserved + * ranges with either the preceding or following range to keep the table small + * and lookups fast. + */ +static const struct intel_forcewake_range __pvc_fw_ranges[] = { + GEN_FW_RANGE(0x0, 0xaff, 0), + GEN_FW_RANGE(0xb00, 0xbff, FORCEWAKE_GT), + GEN_FW_RANGE(0xc00, 0xfff, 0), + GEN_FW_RANGE(0x1000, 0x1fff, FORCEWAKE_GT), + GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER), + GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_GT), + GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER), + GEN_FW_RANGE(0x4000, 0x813f, FORCEWAKE_GT), /* + 0x4000 - 0x4aff: gt + 0x4b00 - 0x4fff: reserved + 0x5000 - 0x51ff: gt + 0x5200 - 0x52ff: reserved + 0x5300 - 0x53ff: gt + 0x5400 - 0x7fff: reserved + 0x8000 - 0x813f: gt */ + GEN_FW_RANGE(0x8140, 0x817f, FORCEWAKE_RENDER), + GEN_FW_RANGE(0x8180, 0x81ff, 0), + GEN_FW_RANGE(0x8200, 0x94cf, FORCEWAKE_GT), /* + 0x8200 - 0x82ff: gt + 0x8300 - 0x84ff: reserved + 0x8500 - 0x887f: gt + 0x8880 - 0x8a7f: reserved + 0x8a80 - 0x8aff: gt + 0x8b00 - 0x8fff: reserved + 0x9000 - 0x947f: gt + 0x9480 - 0x94cf: reserved */ + GEN_FW_RANGE(0x94d0, 0x955f, FORCEWAKE_RENDER), + GEN_FW_RANGE(0x9560, 0x967f, 0), /* + 0x9560 - 0x95ff: always on + 0x9600 - 0x967f: reserved */ + GEN_FW_RANGE(0x9680, 0x97ff, FORCEWAKE_RENDER), /* + 0x9680 - 0x96ff: render + 0x9700 - 0x97ff: reserved */ + GEN_FW_RANGE(0x9800, 0xcfff, FORCEWAKE_GT), /* + 0x9800 - 0xb4ff: gt + 0xb500 - 0xbfff: reserved + 0xc000 - 0xcfff: gt */ + GEN_FW_RANGE(0xd000, 0xd3ff, 0), + GEN_FW_RANGE(0xd400, 0xdbff, FORCEWAKE_GT), + GEN_FW_RANGE(0xdc00, 0xdcff, FORCEWAKE_RENDER), + GEN_FW_RANGE(0xdd00, 0xde7f, FORCEWAKE_GT), /* + 0xdd00 - 0xddff: gt + 0xde00 - 0xde7f: reserved */ + GEN_FW_RAN
[PATCH 08/11] drm/i915/pvc: Interrupt support for new copy engines
This patch adds the interrupt handler support for new copy engines. Bspec: 54030 Original-author: CQ Tang Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_gt_irq.c | 16 drivers/gpu/drm/i915/gt/intel_gt_regs.h | 4 2 files changed, 20 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_irq.c b/drivers/gpu/drm/i915/gt/intel_gt_irq.c index 88b4becfcb17..3a72d4fd0214 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_irq.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_irq.c @@ -193,6 +193,14 @@ void gen11_gt_irq_reset(struct intel_gt *gt) /* Restore masks irqs on RCS, BCS, VCS and VECS engines. */ intel_uncore_write(uncore, GEN11_RCS0_RSVD_INTR_MASK, ~0); intel_uncore_write(uncore, GEN11_BCS_RSVD_INTR_MASK,~0); + if (HAS_ENGINE(gt, BCS1) || HAS_ENGINE(gt, BCS2)) + intel_uncore_write(uncore, XEHPC_BCS1_BCS2_INTR_MASK, ~0); + if (HAS_ENGINE(gt, BCS3) || HAS_ENGINE(gt, BCS4)) + intel_uncore_write(uncore, XEHPC_BCS3_BCS4_INTR_MASK, ~0); + if (HAS_ENGINE(gt, BCS5) || HAS_ENGINE(gt, BCS6)) + intel_uncore_write(uncore, XEHPC_BCS5_BCS6_INTR_MASK, ~0); + if (HAS_ENGINE(gt, BCS7) || HAS_ENGINE(gt, BCS8)) + intel_uncore_write(uncore, XEHPC_BCS7_BCS8_INTR_MASK, ~0); intel_uncore_write(uncore, GEN11_VCS0_VCS1_INTR_MASK, ~0); intel_uncore_write(uncore, GEN11_VCS2_VCS3_INTR_MASK, ~0); if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5)) @@ -248,6 +256,14 @@ void gen11_gt_irq_postinstall(struct intel_gt *gt) /* Unmask irqs on RCS, BCS, VCS and VECS engines. */ intel_uncore_write(uncore, GEN11_RCS0_RSVD_INTR_MASK, ~smask); intel_uncore_write(uncore, GEN11_BCS_RSVD_INTR_MASK, ~smask); + if (HAS_ENGINE(gt, BCS1) || HAS_ENGINE(gt, BCS2)) + intel_uncore_write(uncore, XEHPC_BCS1_BCS2_INTR_MASK, ~dmask); + if (HAS_ENGINE(gt, BCS3) || HAS_ENGINE(gt, BCS4)) + intel_uncore_write(uncore, XEHPC_BCS3_BCS4_INTR_MASK, ~dmask); + if (HAS_ENGINE(gt, BCS5) || HAS_ENGINE(gt, BCS6)) + intel_uncore_write(uncore, XEHPC_BCS5_BCS6_INTR_MASK, ~dmask); + if (HAS_ENGINE(gt, BCS7) || HAS_ENGINE(gt, BCS8)) + intel_uncore_write(uncore, XEHPC_BCS7_BCS8_INTR_MASK, ~dmask); intel_uncore_write(uncore, GEN11_VCS0_VCS1_INTR_MASK, ~dmask); intel_uncore_write(uncore, GEN11_VCS2_VCS3_INTR_MASK, ~dmask); if (HAS_ENGINE(gt, VCS4) || HAS_ENGINE(gt, VCS5)) diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index aa2c0974b02c..fe09288a3145 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -1529,6 +1529,10 @@ #define GEN11_GUNIT_CSME_INTR_MASK _MMIO(0x1900f4) #define GEN12_CCS0_CCS1_INTR_MASK _MMIO(0x190100) #define GEN12_CCS2_CCS3_INTR_MASK _MMIO(0x190104) +#define XEHPC_BCS1_BCS2_INTR_MASK _MMIO(0x190110) +#define XEHPC_BCS3_BCS4_INTR_MASK _MMIO(0x190114) +#define XEHPC_BCS5_BCS6_INTR_MASK _MMIO(0x190118) +#define XEHPC_BCS7_BCS8_INTR_MASK _MMIO(0x19011c) #define GEN12_SFC_DONE(n) _MMIO(0x1cc000 + (n) * 0x1000) -- 2.35.1
[PATCH 05/11] drm/i915/pvc: Remove additional 3D flags from PIPE_CONTROL
From: Stuart Summers Although we already strip 3D-specific flags from PIPE_CONTROL instructions when submitting to a compute engine, there are some additional flags that need to be removed when the platform as a whole lacks a 3D pipeline. Add those restrictions here. Bspec: 47112 Signed-off-by: Stuart Summers Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 18 -- drivers/gpu/drm/i915/gt/intel_gpu_commands.h | 12 ++-- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_pci.c | 3 ++- drivers/gpu/drm/i915/intel_device_info.h | 3 ++- 5 files changed, 28 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 9529c5455bc3..0de17b568b41 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -196,8 +196,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) flags |= PIPE_CONTROL_CS_STALL; - if (engine->class == COMPUTE_CLASS) - flags &= ~PIPE_CONTROL_3D_FLAGS; + if (LACKS_3D_PIPELINE(engine->i915)) + flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS; + else if (engine->class == COMPUTE_CLASS) + flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS; cs = intel_ring_begin(rq, 6); if (IS_ERR(cs)) @@ -226,8 +228,10 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode) flags |= PIPE_CONTROL_CS_STALL; - if (engine->class == COMPUTE_CLASS) - flags &= ~PIPE_CONTROL_3D_FLAGS; + if (LACKS_3D_PIPELINE(engine->i915)) + flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS; + else if (engine->class == COMPUTE_CLASS) + flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS; if (!HAS_FLAT_CCS(rq->engine->i915)) count = 8 + 4; @@ -662,8 +666,10 @@ u32 *gen12_emit_fini_breadcrumb_rcs(struct i915_request *rq, u32 *cs) /* Wa_1409600907 */ flags |= PIPE_CONTROL_DEPTH_STALL; - if (rq->engine->class == COMPUTE_CLASS) - flags &= ~PIPE_CONTROL_3D_FLAGS; + if (LACKS_3D_PIPELINE(rq->engine->i915)) + flags &= ~PIPE_CONTROL_3D_ARCH_FLAGS; + else if (rq->engine->class == COMPUTE_CLASS) + flags &= ~PIPE_CONTROL_3D_ENGINE_FLAGS; cs = gen12_emit_ggtt_write_rcs(cs, rq->fence.seqno, diff --git a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h index e52718a87f14..5eaf54e72635 100644 --- a/drivers/gpu/drm/i915/gt/intel_gpu_commands.h +++ b/drivers/gpu/drm/i915/gt/intel_gpu_commands.h @@ -286,8 +286,8 @@ #define PIPE_CONTROL_DEPTH_CACHE_FLUSH (1<<0) #define PIPE_CONTROL_GLOBAL_GTT (1<<2) /* in addr dword */ -/* 3D-related flags can't be set on compute engine */ -#define PIPE_CONTROL_3D_FLAGS (\ +/* 3D-related flags that can't be set on _engines_ that lack a 3D pipeline */ +#define PIPE_CONTROL_3D_ENGINE_FLAGS (\ PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH | \ PIPE_CONTROL_DEPTH_CACHE_FLUSH | \ PIPE_CONTROL_TILE_CACHE_FLUSH | \ @@ -298,6 +298,14 @@ PIPE_CONTROL_VF_CACHE_INVALIDATE | \ PIPE_CONTROL_GLOBAL_SNAPSHOT_RESET) +/* 3D-related flags that can't be set on _platforms_ that lack a 3D pipeline */ +#define PIPE_CONTROL_3D_ARCH_FLAGS ( \ + PIPE_CONTROL_3D_ENGINE_FLAGS | \ + PIPE_CONTROL_INDIRECT_STATE_DISABLE | \ + PIPE_CONTROL_FLUSH_ENABLE | \ + PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE | \ + PIPE_CONTROL_DC_FLUSH_ENABLE) + #define MI_MATH(x) MI_INSTR(0x1a, (x) - 1) #define MI_MATH_INSTR(opcode, op1, op2) ((opcode) << 20 | (op1) << 10 | (op2)) /* Opcodes for MI_MATH_INSTR */ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8c8e7308502b..55c6b9c4e476 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1402,6 +1402,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define HAS_MBUS_JOINING(i915) (IS_ALDERLAKE_P(i915)) +#define LACKS_3D_PIPELINE(i915)(INTEL_INFO(i915)->lacks_3d_pipeline) + /* i915_gem.c */ void i915_gem_init_early(struct drm_i915_private *dev_priv); void i915_gem_cleanup_early(struct drm_i915_private *dev_priv); diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 07722cdf63ac..14e0e8225324 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1077,7 +1077,8 @@ static const struct intel_device_info ats_m_info = { #define XE_HPC_FEATURES \ XE_HP_FEATURES, \ .dma_mask_size =
[PATCH 09/11] drm/i915/pvc: Reset support for new copy engines
This patch adds the reset support for new copy engines in PVC. Bspec: 52549 Original-author: CQ Tang Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 8 + drivers/gpu/drm/i915/gt/intel_gt_regs.h | 44 +-- 2 files changed, 34 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 4532c3ea9ace..c6e93db134b1 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -390,6 +390,14 @@ static u32 get_reset_domain(u8 ver, enum intel_engine_id id) static const u32 engine_reset_domains[] = { [RCS0] = GEN11_GRDOM_RENDER, [BCS0] = GEN11_GRDOM_BLT, + [BCS1] = XEHPC_GRDOM_BLT1, + [BCS2] = XEHPC_GRDOM_BLT2, + [BCS3] = XEHPC_GRDOM_BLT3, + [BCS4] = XEHPC_GRDOM_BLT4, + [BCS5] = XEHPC_GRDOM_BLT5, + [BCS6] = XEHPC_GRDOM_BLT6, + [BCS7] = XEHPC_GRDOM_BLT7, + [BCS8] = XEHPC_GRDOM_BLT8, [VCS0] = GEN11_GRDOM_MEDIA, [VCS1] = GEN11_GRDOM_MEDIA2, [VCS2] = GEN11_GRDOM_MEDIA3, diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index fe09288a3145..98ede9c93f00 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -597,24 +597,32 @@ /* GEN11 changed all bit defs except for FULL & RENDER */ #define GEN11_GRDOM_FULL GEN6_GRDOM_FULL #define GEN11_GRDOM_RENDER GEN6_GRDOM_RENDER -#define GEN11_GRDOM_BLT (1 << 2) -#define GEN11_GRDOM_GUC (1 << 3) -#define GEN11_GRDOM_MEDIA(1 << 5) -#define GEN11_GRDOM_MEDIA2 (1 << 6) -#define GEN11_GRDOM_MEDIA3 (1 << 7) -#define GEN11_GRDOM_MEDIA4 (1 << 8) -#define GEN11_GRDOM_MEDIA5 (1 << 9) -#define GEN11_GRDOM_MEDIA6 (1 << 10) -#define GEN11_GRDOM_MEDIA7 (1 << 11) -#define GEN11_GRDOM_MEDIA8 (1 << 12) -#define GEN11_GRDOM_VECS (1 << 13) -#define GEN11_GRDOM_VECS2(1 << 14) -#define GEN11_GRDOM_VECS3(1 << 15) -#define GEN11_GRDOM_VECS4(1 << 16) -#define GEN11_GRDOM_SFC0 (1 << 17) -#define GEN11_GRDOM_SFC1 (1 << 18) -#define GEN11_GRDOM_SFC2 (1 << 19) -#define GEN11_GRDOM_SFC3 (1 << 20) +#define XEHPC_GRDOM_BLT8 REG_BIT(31) +#define XEHPC_GRDOM_BLT7 REG_BIT(30) +#define XEHPC_GRDOM_BLT6 REG_BIT(29) +#define XEHPC_GRDOM_BLT5 REG_BIT(28) +#define XEHPC_GRDOM_BLT4 REG_BIT(27) +#define XEHPC_GRDOM_BLT3 REG_BIT(26) +#define XEHPC_GRDOM_BLT2 REG_BIT(25) +#define XEHPC_GRDOM_BLT1 REG_BIT(24) +#define GEN11_GRDOM_SFC3 REG_BIT(20) +#define GEN11_GRDOM_SFC2 REG_BIT(19) +#define GEN11_GRDOM_SFC1 REG_BIT(18) +#define GEN11_GRDOM_SFC0 REG_BIT(17) +#define GEN11_GRDOM_VECS4REG_BIT(16) +#define GEN11_GRDOM_VECS3REG_BIT(15) +#define GEN11_GRDOM_VECS2REG_BIT(14) +#define GEN11_GRDOM_VECS REG_BIT(13) +#define GEN11_GRDOM_MEDIA8 REG_BIT(12) +#define GEN11_GRDOM_MEDIA7 REG_BIT(11) +#define GEN11_GRDOM_MEDIA6 REG_BIT(10) +#define GEN11_GRDOM_MEDIA5 REG_BIT(9) +#define GEN11_GRDOM_MEDIA4 REG_BIT(8) +#define GEN11_GRDOM_MEDIA3 REG_BIT(7) +#define GEN11_GRDOM_MEDIA2 REG_BIT(6) +#define GEN11_GRDOM_MEDIAREG_BIT(5) +#define GEN11_GRDOM_GUC REG_BIT(3) +#define GEN11_GRDOM_BLT REG_BIT(2) #define GEN11_VCS_SFC_RESET_BIT(instance)(GEN11_GRDOM_SFC0 << ((instance) >> 1)) #define GEN11_VECS_SFC_RESET_BIT(instance) (GEN11_GRDOM_SFC0 << (instance)) -- 2.35.1
[PATCH 07/11] drm/i915/pvc: Engines definitions for new copy engines
This patch adds the basic definitions needed to support new copy engines. Also updating the cmd_info to accommodate new engines, as the engine id's of legacy engines have been changed. Original-author: CQ Tang Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_engine_cs.c| 56 drivers/gpu/drm/i915/gt/intel_engine_types.h | 10 +++- drivers/gpu/drm/i915/gt/intel_gt_regs.h | 8 +++ drivers/gpu/drm/i915/gvt/cmd_parser.c| 2 +- drivers/gpu/drm/i915/i915_reg.h | 8 +++ 5 files changed, 82 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 14c6ddbbfde8..4532c3ea9ace 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -71,6 +71,62 @@ static const struct engine_info intel_engines[] = { { .graphics_ver = 6, .base = BLT_RING_BASE } }, }, + [BCS1] = { + .class = COPY_ENGINE_CLASS, + .instance = 1, + .mmio_bases = { + { .graphics_ver = 12, .base = XEHPC_BCS1_RING_BASE } + }, + }, + [BCS2] = { + .class = COPY_ENGINE_CLASS, + .instance = 2, + .mmio_bases = { + { .graphics_ver = 12, .base = XEHPC_BCS2_RING_BASE } + }, + }, + [BCS3] = { + .class = COPY_ENGINE_CLASS, + .instance = 3, + .mmio_bases = { + { .graphics_ver = 12, .base = XEHPC_BCS3_RING_BASE } + }, + }, + [BCS4] = { + .class = COPY_ENGINE_CLASS, + .instance = 4, + .mmio_bases = { + { .graphics_ver = 12, .base = XEHPC_BCS4_RING_BASE } + }, + }, + [BCS5] = { + .class = COPY_ENGINE_CLASS, + .instance = 5, + .mmio_bases = { + { .graphics_ver = 12, .base = XEHPC_BCS5_RING_BASE } + }, + }, + [BCS6] = { + .class = COPY_ENGINE_CLASS, + .instance = 6, + .mmio_bases = { + { .graphics_ver = 12, .base = XEHPC_BCS6_RING_BASE } + }, + }, + [BCS7] = { + .class = COPY_ENGINE_CLASS, + .instance = 7, + .mmio_bases = { + { .graphics_ver = 12, .base = XEHPC_BCS7_RING_BASE } + }, + }, + [BCS8] = { + .class = COPY_ENGINE_CLASS, + .instance = 8, + .mmio_bases = { + { .graphics_ver = 12, .base = XEHPC_BCS8_RING_BASE } + }, + }, [VCS0] = { .class = VIDEO_DECODE_CLASS, .instance = 0, diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 298f2cc7a879..356c15cdccf0 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -35,7 +35,7 @@ #define OTHER_CLASS4 #define COMPUTE_CLASS 5 #define MAX_ENGINE_CLASS 5 -#define MAX_ENGINE_INSTANCE7 +#define MAX_ENGINE_INSTANCE8 #define I915_MAX_SLICES3 #define I915_MAX_SUBSLICES 8 @@ -107,6 +107,14 @@ struct i915_ctx_workarounds { enum intel_engine_id { RCS0 = 0, BCS0, + BCS1, + BCS2, + BCS3, + BCS4, + BCS5, + BCS6, + BCS7, + BCS8, VCS0, VCS1, VCS2, diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h index a0a49c16babd..aa2c0974b02c 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h @@ -1476,6 +1476,14 @@ #define GEN11_KCR(19) #define GEN11_GTPM (16) #define GEN11_BCS(15) +#define XEHPC_BCS1 (14) +#define XEHPC_BCS2 (13) +#define XEHPC_BCS3 (12) +#define XEHPC_BCS4 (11) +#define XEHPC_BCS5 (10) +#define XEHPC_BCS6 (9) +#define XEHPC_BCS7 (8) +#define XEHPC_BCS8 (23) #define GEN12_CCS3 (7) #define GEN12_CCS2 (6) #define GEN12_CCS1 (5) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index b9eb75a2b400..0ba2a3455d99 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -428,7 +428,7 @@ struct cmd_info { #define R_VECS BIT(VECS0) #define R_A
[PATCH 10/11] drm/i915/pvc: skip all copy engines from aux table invalidate
From: Lucas De Marchi As we have more copy engines now, mask all of them from aux table invalidate. Cc: Prathap Kumar Valsan Signed-off-by: Lucas De Marchi Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/gen8_engine_cs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c index 0de17b568b41..f262aed94ef3 100644 --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c @@ -275,7 +275,7 @@ int gen12_emit_flush_xcs(struct i915_request *rq, u32 mode) if (!HAS_FLAT_CCS(rq->engine->i915) && (rq->engine->class == VIDEO_DECODE_CLASS || rq->engine->class == VIDEO_ENHANCEMENT_CLASS)) { - aux_inv = rq->engine->mask & ~BIT(BCS0); + aux_inv = rq->engine->mask & ~GENMASK(BCS8, BCS0); if (aux_inv) cmd += 4; } -- 2.35.1
[PATCH 11/11] drm/i915/pvc: read fuses for link copy engines
From: Lucas De Marchi The new Link Copy engines in PVC may be fused off according to the mslice_mask. Each bit of the MEML3_EN_MASK we read from the GEN10_MIRROR_FUSE3 register disables a pair of link copy engines. Bspec: 44483 Cc: Matt Roper Signed-off-by: Lucas De Marchi Signed-off-by: Matt Roper --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 28 +++ 1 file changed, 28 insertions(+) diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index c6e93db134b1..d10cdeff5072 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -686,6 +686,33 @@ static void engine_mask_apply_compute_fuses(struct intel_gt *gt) } } +static void engine_mask_apply_copy_fuses(struct intel_gt *gt) +{ + struct drm_i915_private *i915 = gt->i915; + struct intel_gt_info *info = >->info; + unsigned long meml3_mask; + u8 quad; + + meml3_mask = intel_uncore_read(gt->uncore, GEN10_MIRROR_FUSE3); + meml3_mask = REG_FIELD_GET(GEN12_MEML3_EN_MASK, meml3_mask); + + /* +* Link Copy engines may be fused off according to meml3_mask. Each +* bit is a quad that houses 2 Link Copy and two Sub Copy engines. +*/ + for_each_clear_bit(quad, &meml3_mask, GEN12_MAX_MSLICES) { + intel_engine_mask_t mask = GENMASK(BCS1 + quad * 2 + 1, + BCS1 + quad * 2); + + if (mask & info->engine_mask) { + drm_dbg(&i915->drm, "bcs%u fused off\n", quad * 2 + 1); + drm_dbg(&i915->drm, "bcs%u fused off\n", quad * 2 + 2); + + info->engine_mask &= ~mask; + } + } +} + /* * Determine which engines are fused off in our particular hardware. * Note that we have a catch-22 situation where we need to be able to access @@ -768,6 +795,7 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt) GEM_BUG_ON(vebox_mask != VEBOX_MASK(gt)); engine_mask_apply_compute_fuses(gt); + engine_mask_apply_copy_fuses(gt); return info->engine_mask; } -- 2.35.1
Tackling the indefinite/user DMA fence problem
Hello everyone, it's a well known problem that the DMA-buf subsystem mixed synchronization and memory management requirements into the same dma_fence and dma_resv objects. Because of this dma_fence objects need to guarantee that they complete within a finite amount of time or otherwise the system can easily deadlock. One of the few good things about this problem is that it is really good understood by now. Daniel and others came up with some documentation: https://dri.freedesktop.org/docs/drm/driver-api/dma-buf.html?highlight=dma_buf#indefinite-dma-fences And Jason did an excellent presentation about that problem on last years LPC: https://lpc.events/event/11/contributions/1115/ Based on that we had been able to reject new implementations of infinite/user DMA fences and mitigate the effect of the few existing ones. The still remaining down side is that we don't have a way of using user fences as dependency in both the explicit (sync_file, drm_syncobj) as well as the implicit (dma_resv) synchronization objects, resulting in numerous problems and limitations for things like HMM, user queues etc This patch set here now tries to tackle this problem by untangling the synchronization from the memory management. What it does *not* try to do is to fix the existing kernel fences, because I think we now can all agree on that this isn't really possible. To archive this goal what I do in this patch set is to add some parallel infrastructure to cleanly separate normal kernel dma_fence objects from indefinite/user fences: 1. It introduce a DMA_FENCE_FLAG_USER define (after renaming some existing driver defines). To note that a certain dma_fence is an user fence and *must* be ignore by memory management and never used as dependency for normal none user dma_fence objects. 2. The dma_fence_array and dma_fence_chain containers are modified so that they are marked as user fences whenever any of their contained fences are an user fence. 3. The dma_resv object gets a new DMA_RESV_USAGE_USER flag which must be used with indefinite/user fences and separates those into it's own synchronization domain. 4. The existing dma_buf_poll_add_cb() function is modified so that indefinite/user fences are included in the polling. 5. The sync_file synchronization object is modified so that we essentially have two fence streams instead of just one. 6. The drm_syncobj is modified in a similar way. User fences are just ignored unless the driver explicitly states support to wait for them. 7. The DRM subsystem gains a new DRIVER_USER_FENCE flag which drivers can use to indicate the need for user fences. If user fences are used the atomic mode setting starts to support user fences as IN/OUT fences. 8. Lockdep is used at various critical locations to ensure that nobody ever tries to mix user fences with non user fences. The general approach is to just ignore user fences unless a driver stated explicitely support for them. On top of all of this I've hacked amdgpu so that we add the resulting CS fence only as kernel dependency to the dma_resv object and an additional wrapped up with a dma_fence_array and a stub user fence. The result is that the newly added atomic modeset functions now correctly wait for the user fence to complete before doing the flip. And dependent CS don't pipeline any more, but rather block on the CPU before submitting work. After tons of debugging and testing everything now seems to not go up in flames immediately and even lockdep is happy with the annotations. I'm perfectly aware that this is probably by far the most controversial patch set I've ever created and I really wish we wouldn't need it. But we certainly have the requirement for this and I don't see much other chance to get that working in an UAPI compatible way. Thoughts/comments? Regards, Christian.
[PATCH 02/15] dma-buf: introduce user fence support
Start introducing a new part of the framework for handling user fences. In strict opposition to normal fences user fences don't reliable finish in a fixed amount of time and therefore can't be used as dependency in memory management. Because of this user fences are marked with DMA_FENCE_FLAG_USER. Lockdep is checked that we can at least fault user pages when we check them for signaling. This patch also adds a flag to dma_fence_get_stub() so that we can retrieve a signaled user fence. This can be used together with lockdep to test the handling in code path supporting user fences. Signed-off-by: Christian König --- drivers/dma-buf/dma-fence-unwrap.c| 4 +-- drivers/dma-buf/dma-fence.c | 31 --- drivers/dma-buf/st-dma-fence.c| 2 +- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ring.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 4 +-- drivers/gpu/drm/drm_syncobj.c | 10 +++--- include/linux/dma-fence.h | 17 +- 9 files changed, 49 insertions(+), 25 deletions(-) diff --git a/drivers/dma-buf/dma-fence-unwrap.c b/drivers/dma-buf/dma-fence-unwrap.c index c9becc74896d..87ee2efced10 100644 --- a/drivers/dma-buf/dma-fence-unwrap.c +++ b/drivers/dma-buf/dma-fence-unwrap.c @@ -76,7 +76,7 @@ struct dma_fence *__dma_fence_merge(unsigned int num_fences, } if (count == 0) - return dma_fence_get_stub(); + return dma_fence_get_stub(false); if (count > INT_MAX) return NULL; @@ -129,7 +129,7 @@ struct dma_fence *__dma_fence_merge(unsigned int num_fences, } while (tmp); if (count == 0) { - tmp = dma_fence_get_stub(); + tmp = dma_fence_get_stub(false); goto return_tmp; } diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c index 066400ed8841..52873f5eaeba 100644 --- a/drivers/dma-buf/dma-fence.c +++ b/drivers/dma-buf/dma-fence.c @@ -26,6 +26,7 @@ EXPORT_TRACEPOINT_SYMBOL(dma_fence_signaled); static DEFINE_SPINLOCK(dma_fence_stub_lock); static struct dma_fence dma_fence_stub; +static struct dma_fence dma_fence_user_stub; /* * fence context counter: each execution context should have its own @@ -123,24 +124,28 @@ static const struct dma_fence_ops dma_fence_stub_ops = { /** * dma_fence_get_stub - return a signaled fence + * @user: if true the returned fence is an user fence * - * Return a stub fence which is already signaled. The fence's - * timestamp corresponds to the first time after boot this - * function is called. + * Return a stub fence which is already signaled. The fence's timestamp + * corresponds to the first time after boot this function is called. If @user is + * true an user fence is returned which can be used with lockdep to test user + * fence saveness in a code path. */ -struct dma_fence *dma_fence_get_stub(void) +struct dma_fence *dma_fence_get_stub(bool user) { + struct dma_fence *fence = user ? &dma_fence_stub : &dma_fence_user_stub; + spin_lock(&dma_fence_stub_lock); - if (!dma_fence_stub.ops) { - dma_fence_init(&dma_fence_stub, - &dma_fence_stub_ops, - &dma_fence_stub_lock, + if (!fence->ops) { + dma_fence_init(fence, &dma_fence_stub_ops, &dma_fence_stub_lock, 0, 0); - dma_fence_signal_locked(&dma_fence_stub); + if (user) + set_bit(DMA_FENCE_FLAG_USER, &fence->flags); + dma_fence_signal_locked(fence); } spin_unlock(&dma_fence_stub_lock); - return dma_fence_get(&dma_fence_stub); + return dma_fence_get(fence); } EXPORT_SYMBOL(dma_fence_get_stub); @@ -497,8 +502,9 @@ dma_fence_wait_timeout(struct dma_fence *fence, bool intr, signed long timeout) return -EINVAL; might_sleep(); - __dma_fence_might_wait(); + if (test_bit(DMA_FENCE_FLAG_USER, &fence->flags)) + might_fault(); trace_dma_fence_wait_start(fence); if (fence->ops->wait) @@ -870,6 +876,9 @@ dma_fence_wait_any_timeout(struct dma_fence **fences, uint32_t count, for (i = 0; i < count; ++i) { struct dma_fence *fence = fences[i]; + if (test_bit(DMA_FENCE_FLAG_USER, &fence->flags)) + might_fault(); + cb[i].task = current; if (dma_fence_add_callback(fence, &cb[i].base, dma_fence_default_wait_cb)) { diff --git a/drivers/dma-buf/st-dma-fence.c b/drivers/dma-buf/st-dma-fence.c index c8a12d7ad71a..50f757f75645 100644 --- a/drivers/dma-buf/st-dma-fence.c +++ b/drivers/dma-buf/st-dma-fence.c @@ -412,7 +412,7 @@ static
[PATCH 01/15] dma-buf: rename DMA_FENCE_FLAG_USER_BITS to _DEVICE
This is supposed to be used by device drivers. Signed-off-by: Christian König --- drivers/gpu/drm/i915/i915_request.h | 2 +- drivers/gpu/drm/i915/i915_sw_fence_work.h | 2 +- drivers/gpu/drm/nouveau/nouveau_fence.c | 4 ++-- include/linux/dma-fence.h | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_request.h b/drivers/gpu/drm/i915/i915_request.h index 28b1f9db5487..716c8c413cc4 100644 --- a/drivers/gpu/drm/i915/i915_request.h +++ b/drivers/gpu/drm/i915/i915_request.h @@ -80,7 +80,7 @@ enum { * * See i915_request_is_active() */ - I915_FENCE_FLAG_ACTIVE = DMA_FENCE_FLAG_USER_BITS, + I915_FENCE_FLAG_ACTIVE = DMA_FENCE_FLAG_DRIVER, /* * I915_FENCE_FLAG_PQUEUE - this request is ready for execution diff --git a/drivers/gpu/drm/i915/i915_sw_fence_work.h b/drivers/gpu/drm/i915/i915_sw_fence_work.h index d56806918d13..ece0a06e598c 100644 --- a/drivers/gpu/drm/i915/i915_sw_fence_work.h +++ b/drivers/gpu/drm/i915/i915_sw_fence_work.h @@ -33,7 +33,7 @@ struct dma_fence_work { }; enum { - DMA_FENCE_WORK_IMM = DMA_FENCE_FLAG_USER_BITS, + DMA_FENCE_WORK_IMM = DMA_FENCE_FLAG_DRIVER, }; void dma_fence_work_init(struct dma_fence_work *f, diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c index 7f01dcf81fab..e2f61b34cc1e 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fence.c +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c @@ -61,7 +61,7 @@ nouveau_fence_signal(struct nouveau_fence *fence) list_del(&fence->head); rcu_assign_pointer(fence->channel, NULL); - if (test_bit(DMA_FENCE_FLAG_USER_BITS, &fence->base.flags)) { + if (test_bit(DMA_FENCE_FLAG_DRIVER, &fence->base.flags)) { struct nouveau_fence_chan *fctx = nouveau_fctx(fence); if (!--fctx->notify_ref) @@ -510,7 +510,7 @@ static bool nouveau_fence_enable_signaling(struct dma_fence *f) ret = nouveau_fence_no_signaling(f); if (ret) - set_bit(DMA_FENCE_FLAG_USER_BITS, &fence->base.flags); + set_bit(DMA_FENCE_FLAG_DRIVER, &fence->base.flags); else if (!--fctx->notify_ref) nvif_notify_put(&fctx->notify); diff --git a/include/linux/dma-fence.h b/include/linux/dma-fence.h index 775cdc0b4f24..afea82ec5946 100644 --- a/include/linux/dma-fence.h +++ b/include/linux/dma-fence.h @@ -49,7 +49,7 @@ struct dma_fence_cb; * DMA_FENCE_FLAG_SIGNALED_BIT - fence is already signaled * DMA_FENCE_FLAG_TIMESTAMP_BIT - timestamp recorded for fence signaling * DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT - enable_signaling might have been called - * DMA_FENCE_FLAG_USER_BITS - start of the unused bits, can be used by the + * DMA_FENCE_FLAG_DRIVER - start of the unused bits, can be used by the * implementer of the fence for its own purposes. Can be used in different * ways by different fence implementers, so do not rely on this. * @@ -99,7 +99,7 @@ enum dma_fence_flag_bits { DMA_FENCE_FLAG_SIGNALED_BIT, DMA_FENCE_FLAG_TIMESTAMP_BIT, DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT, - DMA_FENCE_FLAG_USER_BITS, /* must always be last member */ + DMA_FENCE_FLAG_DRIVER, /* must always be last member */ }; typedef void (*dma_fence_func_t)(struct dma_fence *fence, -- 2.25.1
[PATCH 03/15] dma-buf: add user fence support to dma_fence_array
When any of the fences inside the array is an user fence the whole array is considered to be an user fence as well. Signed-off-by: Christian König --- drivers/dma-buf/dma-fence-array.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/dma-buf/dma-fence-array.c b/drivers/dma-buf/dma-fence-array.c index 5c8a7084577b..de196b07d36d 100644 --- a/drivers/dma-buf/dma-fence-array.c +++ b/drivers/dma-buf/dma-fence-array.c @@ -189,8 +189,13 @@ struct dma_fence_array *dma_fence_array_create(int num_fences, * Enforce this here by checking that we don't create a dma_fence_array * with any container inside. */ - while (num_fences--) - WARN_ON(dma_fence_is_container(fences[num_fences])); + while (num_fences--) { + struct dma_fence *f = fences[num_fences]; + + WARN_ON(dma_fence_is_container(f)); + if (test_bit(DMA_FENCE_FLAG_USER, &f->flags)) + set_bit(DMA_FENCE_FLAG_USER, &array->base.flags); + } return array; } -- 2.25.1
[PATCH 04/15] dma-buf: add user fence support to dma_fence_chain
If either the previous fence or the newly chained on is an user fence the dma_fence chain node is considered an user fence as well. This way the user fence status propagates through the chain. Signed-off-by: Christian König --- drivers/dma-buf/dma-fence-chain.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/dma-buf/dma-fence-chain.c b/drivers/dma-buf/dma-fence-chain.c index 06f8ef97c6e8..421241607ac2 100644 --- a/drivers/dma-buf/dma-fence-chain.c +++ b/drivers/dma-buf/dma-fence-chain.c @@ -253,6 +253,10 @@ void dma_fence_chain_init(struct dma_fence_chain *chain, dma_fence_init(&chain->base, &dma_fence_chain_ops, &chain->lock, context, seqno); + if ((prev && test_bit(DMA_FENCE_FLAG_USER, &prev->flags)) || + test_bit(DMA_FENCE_FLAG_USER, &fence->flags)) + set_bit(DMA_FENCE_FLAG_USER, &chain->base.flags); + /* * Chaining dma_fence_chain container together is only allowed through * the prev fence and not through the contained fence. -- 2.25.1
[PATCH 05/15] dma-buf: add user fence support to dma_resv
This patch adds the new DMA_RESV_USAGE_USER flag to the dma_resv object which must be used with user fence objects. In opposite to the other usage flags this one doesn't automatically return other lower classes. So when user fences are requested from the dma_resv object only user fences are returned. Lockdep is used to enforce that user fences can only be queried while the dma_resv object is not locked. Additional to that waiting for the user fences inside a dma_resv object requires not other lock to be held. Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c | 62 +++--- include/linux/dma-resv.h | 23 -- 2 files changed, 58 insertions(+), 27 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 0cce6e4ec946..da667c21ad55 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -58,7 +58,7 @@ DEFINE_WD_CLASS(reservation_ww_class); EXPORT_SYMBOL(reservation_ww_class); /* Mask for the lower fence pointer bits */ -#define DMA_RESV_LIST_MASK 0x3 +#define DMA_RESV_LIST_MASK 0x7 struct dma_resv_list { struct rcu_head rcu; @@ -288,6 +288,10 @@ void dma_resv_add_fence(struct dma_resv *obj, struct dma_fence *fence, */ WARN_ON(dma_fence_is_container(fence)); + /* User fences must be added using DMA_RESV_USAGE_USER */ + WARN_ON(test_bit(DMA_FENCE_FLAG_USER, &fence->flags) != + (usage == DMA_RESV_USAGE_USER)); + fobj = dma_resv_fences_list(obj); count = fobj->num_fences; @@ -349,6 +353,15 @@ void dma_resv_replace_fences(struct dma_resv *obj, uint64_t context, } EXPORT_SYMBOL(dma_resv_replace_fences); +/* Matches requested usage with the fence usage for iterators */ +static bool dma_resv_iter_match_usage(struct dma_resv_iter *cursor) +{ + if (cursor->usage == DMA_RESV_USAGE_USER) + return cursor->fence_usage == DMA_RESV_USAGE_USER; + + return cursor->usage >= cursor->fence_usage; +} + /* Restart the unlocked iteration by initializing the cursor object. */ static void dma_resv_iter_restart_unlocked(struct dma_resv_iter *cursor) { @@ -385,8 +398,7 @@ static void dma_resv_iter_walk_unlocked(struct dma_resv_iter *cursor) continue; } - if (!dma_fence_is_signaled(cursor->fence) && - cursor->usage >= cursor->fence_usage) + if (dma_resv_iter_match_usage(cursor)) break; } while (true); } @@ -405,14 +417,9 @@ static void dma_resv_iter_walk_unlocked(struct dma_resv_iter *cursor) */ struct dma_fence *dma_resv_iter_first_unlocked(struct dma_resv_iter *cursor) { - rcu_read_lock(); - do { - dma_resv_iter_restart_unlocked(cursor); - dma_resv_iter_walk_unlocked(cursor); - } while (dma_resv_fences_list(cursor->obj) != cursor->fences); - rcu_read_unlock(); - - return cursor->fence; + /* Force a restart */ + cursor->fences = NULL; + return dma_resv_iter_next_unlocked(cursor); } EXPORT_SYMBOL(dma_resv_iter_first_unlocked); @@ -428,18 +435,21 @@ EXPORT_SYMBOL(dma_resv_iter_first_unlocked); */ struct dma_fence *dma_resv_iter_next_unlocked(struct dma_resv_iter *cursor) { - bool restart; - - rcu_read_lock(); cursor->is_restarted = false; - restart = dma_resv_fences_list(cursor->obj) != cursor->fences; do { - if (restart) - dma_resv_iter_restart_unlocked(cursor); - dma_resv_iter_walk_unlocked(cursor); - restart = true; - } while (dma_resv_fences_list(cursor->obj) != cursor->fences); - rcu_read_unlock(); + bool restart; + + rcu_read_lock(); + restart = dma_resv_fences_list(cursor->obj) != cursor->fences; + do { + if (restart) + dma_resv_iter_restart_unlocked(cursor); + dma_resv_iter_walk_unlocked(cursor); + restart = true; + } while (dma_resv_fences_list(cursor->obj) != cursor->fences); + rcu_read_unlock(); + + } while (cursor->fence && dma_fence_is_signaled(cursor->fence)); return cursor->fence; } @@ -491,7 +501,7 @@ struct dma_fence *dma_resv_iter_next(struct dma_resv_iter *cursor) dma_resv_list_entry(cursor->fences, cursor->index++, cursor->obj, &fence, &cursor->fence_usage); - } while (cursor->fence_usage > cursor->usage); + } while (!dma_resv_iter_match_usage(cursor)); return fence; } @@ -663,6 +673,9 @@ long dma_resv_wait_timeout(struct dma_resv *obj, enum dma_resv_usage usage, struct dma_resv_iter cursor; struct dma_fence *fence; + if (usage == DMA_RESV_USAGE_USER) + lockdep_assert_n