[PATCH 3/4] drm/i915/gtt/xehpsdv: move scratch page to system memory
From: Matthew Auld On some platforms the hw has dropped support for 4K GTT pages when dealing with LMEM, and due to the design of 64K GTT pages in the hw, we can only mark the *entire* page-table as operating in 64K GTT mode, since the enable bit is still on the pde, and not the pte. And since we we still need to allow 4K GTT pages for SMEM objects, we can't have a "normal" 4K page-table with scratch pointing to LMEM, since that's undefined from the hw pov. The simplest solution is to just move the 64K scratch page to SMEM on such platforms and call it a day, since that should work for all configurations. Signed-off-by: Matthew Auld Signed-off-by: Ramalingam C --- drivers/gpu/drm/i915/gt/gen6_ppgtt.c | 1 + drivers/gpu/drm/i915/gt/gen8_ppgtt.c | 23 +-- drivers/gpu/drm/i915/gt/intel_ggtt.c | 3 +++ drivers/gpu/drm/i915/gt/intel_gtt.c | 2 +- drivers/gpu/drm/i915/gt/intel_gtt.h | 2 ++ drivers/gpu/drm/i915/selftests/mock_gtt.c | 2 ++ 6 files changed, 30 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c index 4a166d25fe60..c0d149f04949 100644 --- a/drivers/gpu/drm/i915/gt/gen6_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen6_ppgtt.c @@ -454,6 +454,7 @@ struct i915_ppgtt *gen6_ppgtt_create(struct intel_gt *gt) ppgtt->base.vm.cleanup = gen6_ppgtt_cleanup; ppgtt->base.vm.alloc_pt_dma = alloc_pt_dma; + ppgtt->base.vm.alloc_scratch_dma = alloc_pt_dma; ppgtt->base.vm.pte_encode = ggtt->vm.pte_encode; err = gen6_ppgtt_init_scratch(ppgtt); diff --git a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c index 95c02096a61b..b012c50f7ce7 100644 --- a/drivers/gpu/drm/i915/gt/gen8_ppgtt.c +++ b/drivers/gpu/drm/i915/gt/gen8_ppgtt.c @@ -776,10 +776,29 @@ struct i915_ppgtt *gen8_ppgtt_create(struct intel_gt *gt, */ ppgtt->vm.has_read_only = !IS_GRAPHICS_VER(gt->i915, 11, 12); - if (HAS_LMEM(gt->i915)) + if (HAS_LMEM(gt->i915)) { ppgtt->vm.alloc_pt_dma = alloc_pt_lmem; - else + + /* +* On some platforms the hw has dropped support for 4K GTT pages +* when dealing with LMEM, and due to the design of 64K GTT +* pages in the hw, we can only mark the *entire* page-table as +* operating in 64K GTT mode, since the enable bit is still on +* the pde, and not the pte. And since we still need to allow +* 4K GTT pages for SMEM objects, we can't have a "normal" 4K +* page-table with scratch pointing to LMEM, since that's +* undefined from the hw pov. The simplest solution is to just +* move the 64K scratch page to SMEM on such platforms and call +* it a day, since that should work for all configurations. +*/ + if (HAS_64K_PAGES(gt->i915)) + ppgtt->vm.alloc_scratch_dma = alloc_pt_dma; + else + ppgtt->vm.alloc_scratch_dma = alloc_pt_lmem; + } else { ppgtt->vm.alloc_pt_dma = alloc_pt_dma; + ppgtt->vm.alloc_scratch_dma = alloc_pt_dma; + } err = gen8_init_scratch(&ppgtt->vm); if (err) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index cbc6d2b1fd9e..d85a1050f4a8 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -941,6 +941,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) size = gen8_get_total_gtt_size(snb_gmch_ctl); ggtt->vm.alloc_pt_dma = alloc_pt_dma; + ggtt->vm.alloc_scratch_dma = alloc_pt_dma; ggtt->vm.lmem_pt_obj_flags = I915_BO_ALLOC_PM_EARLY; ggtt->vm.total = (size / sizeof(gen8_pte_t)) * I915_GTT_PAGE_SIZE; @@ -1094,6 +1095,7 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt) ggtt->vm.total = (size / sizeof(gen6_pte_t)) * I915_GTT_PAGE_SIZE; ggtt->vm.alloc_pt_dma = alloc_pt_dma; + ggtt->vm.alloc_scratch_dma = alloc_pt_dma; ggtt->vm.clear_range = nop_clear_range; if (!HAS_FULL_PPGTT(i915) || intel_scanout_needs_vtd_wa(i915)) @@ -1146,6 +1148,7 @@ static int i915_gmch_probe(struct i915_ggtt *ggtt) (struct resource)DEFINE_RES_MEM(gmadr_base, ggtt->mappable_end); ggtt->vm.alloc_pt_dma = alloc_pt_dma; + ggtt->vm.alloc_scratch_dma = alloc_pt_dma; if (needs_idle_maps(i915)) { drm_notice(&i915->drm, diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c index 0dd254cb1f69..1428e2b9075a 100644 --- a/drivers/gpu/drm/i915/gt/intel_gtt.c +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c @@ -301,7 +301,7 @@ int setup_scratch_page(struct i915_address_space *vm) do { struct drm_i915_gem_object *obj;
[PATCH 2/4] drm/i915/xehpsdv: set min page-size to 64K
From: Matthew Auld LMEM should be allocated at 64K granularity, since 4K page support will eventually be dropped for LMEM when using the PPGTT. Signed-off-by: Matthew Auld Signed-off-by: Stuart Summers Signed-off-by: Ramalingam C Cc: Joonas Lahtinen Cc: Rodrigo Vivi Reviewed-by: Lucas De Marchi --- drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 6 +- drivers/gpu/drm/i915/gt/intel_region_lmem.c | 5 - 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c index bce03d74a0b4..ba90ab47d838 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c @@ -780,6 +780,7 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type, struct intel_uncore *uncore = &i915->uncore; struct pci_dev *pdev = to_pci_dev(i915->drm.dev); struct intel_memory_region *mem; + resource_size_t min_page_size; resource_size_t io_start; resource_size_t lmem_size; u64 lmem_base; @@ -791,8 +792,11 @@ i915_gem_stolen_lmem_setup(struct drm_i915_private *i915, u16 type, lmem_size = pci_resource_len(pdev, 2) - lmem_base; io_start = pci_resource_start(pdev, 2) + lmem_base; + min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K : + I915_GTT_PAGE_SIZE_4K; + mem = intel_memory_region_create(i915, lmem_base, lmem_size, -I915_GTT_PAGE_SIZE_4K, io_start, +min_page_size, io_start, type, instance, &i915_region_stolen_lmem_ops); if (IS_ERR(mem)) diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c b/drivers/gpu/drm/i915/gt/intel_region_lmem.c index 9ea49e0a27c0..fde2dcb59809 100644 --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c @@ -197,6 +197,7 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt) struct intel_uncore *uncore = gt->uncore; struct pci_dev *pdev = to_pci_dev(i915->drm.dev); struct intel_memory_region *mem; + resource_size_t min_page_size; resource_size_t io_start; resource_size_t lmem_size; int err; @@ -211,10 +212,12 @@ static struct intel_memory_region *setup_lmem(struct intel_gt *gt) if (GEM_WARN_ON(lmem_size > pci_resource_len(pdev, 2))) return ERR_PTR(-ENODEV); + min_page_size = HAS_64K_PAGES(i915) ? I915_GTT_PAGE_SIZE_64K : + I915_GTT_PAGE_SIZE_4K; mem = intel_memory_region_create(i915, 0, lmem_size, -I915_GTT_PAGE_SIZE_4K, +min_page_size, io_start, INTEL_MEMORY_LOCAL, 0, -- 2.20.1
[PATCH 1/4] drm/i915: Add has_64k_pages flag
From: Stuart Summers Add a new platform flag, has_64k_pages, for platforms supporting base page sizes of 64k. Signed-off-by: Stuart Summers Signed-off-by: Ramalingam C Reviewed-by: Lucas De Marchi --- drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_pci.c | 2 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + 3 files changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 85bb8d3107f0..6132163e1cb3 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1528,6 +1528,8 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, #define HAS_MSLICES(dev_priv) \ (INTEL_INFO(dev_priv)->has_mslices) +#define HAS_64K_PAGES(dev_priv) (INTEL_INFO(dev_priv)->has_64k_pages) + #define HAS_IPC(dev_priv) (INTEL_INFO(dev_priv)->display.has_ipc) #define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 6aaa7c644c9b..634282edadb7 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1029,6 +1029,7 @@ static const struct intel_device_info xehpsdv_info = { DGFX_FEATURES, PLATFORM(INTEL_XEHPSDV), .display = { }, + .has_64k_pages = 1, .pipe_mask = 0, .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | @@ -1047,6 +1048,7 @@ static const struct intel_device_info dg2_info = { .graphics.rel = 55, .media.rel = 55, PLATFORM(INTEL_DG2), + .has_64k_pages = 1, .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VECS1) | diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 669f0d26c3c3..f38ac5bd837b 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -123,6 +123,7 @@ enum intel_ppgtt_type { func(is_dgfx); \ /* Keep has_* in alphabetical order */ \ func(has_64bit_reloc); \ + func(has_64k_pages); \ func(gpu_reset_clobbers_display); \ func(has_reset_engine); \ func(has_global_mocs); \ -- 2.20.1
[PATCH 0/4] Basic enabling of 64k page support
Preparational patches for 64k page support. Matthew Auld (3): drm/i915/xehpsdv: set min page-size to 64K drm/i915/gtt/xehpsdv: move scratch page to system memory drm/i915: enforce min page size for scratch Stuart Summers (1): drm/i915: Add has_64k_pages flag drivers/gpu/drm/i915/gem/i915_gem_stolen.c | 6 +- drivers/gpu/drm/i915/gt/gen6_ppgtt.c| 1 + drivers/gpu/drm/i915/gt/gen8_ppgtt.c| 23 +++-- drivers/gpu/drm/i915/gt/intel_ggtt.c| 3 +++ drivers/gpu/drm/i915/gt/intel_gtt.c | 14 - drivers/gpu/drm/i915/gt/intel_gtt.h | 2 ++ drivers/gpu/drm/i915/gt/intel_region_lmem.c | 5 - drivers/gpu/drm/i915/i915_drv.h | 2 ++ drivers/gpu/drm/i915/i915_pci.c | 2 ++ drivers/gpu/drm/i915/intel_device_info.h| 1 + drivers/gpu/drm/i915/selftests/mock_gtt.c | 2 ++ 11 files changed, 56 insertions(+), 5 deletions(-) -- 2.20.1
Re: [PATCH 1/4] dt-bindings: display: bridge: tc358867: Document DPI output support
On 12/7/21 17:43, Laurent Pinchart wrote: [...] diff --git a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml index f1541cc05297..5cfda6f2ba69 100644 --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml @@ -61,8 +61,8 @@ properties: port@1: $ref: /schemas/graph.yaml#/properties/port description: | -DPI input port. The remote endpoint phandle should be a -reference to a valid DPI output endpoint node +DPI input/output port. The remote endpoint phandle should be a +reference to a valid DPI output or input endpoint node. I assume the mode of operation (input or output) will be fixed for a given hardware design. Isn't this something that should be recorded in DT ? It would simplify configuration of the device in the driver. Currently the configuration (DSI-to-DPI / DPI-to-eDP) is inferred from the presence of DPI panel. If DPI panel present, DSI-to-DPI, else, DPI-to-eDP.
Re: [PATCH 1/4] dt-bindings: display: bridge: tc358867: Document DPI output support
Hi Marek, Thank you for the patch. On Sat, Nov 27, 2021 at 04:24:02AM +0100, Marek Vasut wrote: > The TC358767/TC358867/TC9595 are all capable of operating in multiple > modes, DPI-to-(e)DP, DSI-to-(e)DP, DSI-to-DPI. Document support for the > DPI output port, which can now be connected both as input and output. > > Signed-off-by: Marek Vasut > Cc: Andrzej Hajda > Cc: Jernej Skrabec > Cc: Jonas Karlman > Cc: Laurent Pinchart > Cc: Neil Armstrong > Cc: Rob Herring > Cc: Sam Ravnborg > Cc: devicet...@vger.kernel.org > To: dri-devel@lists.freedesktop.org > --- > .../devicetree/bindings/display/bridge/toshiba,tc358767.yaml | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git > a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml > b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml > index f1541cc05297..5cfda6f2ba69 100644 > --- a/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml > +++ b/Documentation/devicetree/bindings/display/bridge/toshiba,tc358767.yaml > @@ -61,8 +61,8 @@ properties: >port@1: > $ref: /schemas/graph.yaml#/properties/port > description: | > -DPI input port. The remote endpoint phandle should be a > -reference to a valid DPI output endpoint node > +DPI input/output port. The remote endpoint phandle should be a > +reference to a valid DPI output or input endpoint node. I assume the mode of operation (input or output) will be fixed for a given hardware design. Isn't this something that should be recorded in DT ? It would simplify configuration of the device in the driver. > >port@2: > $ref: /schemas/graph.yaml#/properties/port > -- Regards, Laurent Pinchart
Re: [PATCH 2/4] drm/bridge: tc358767: Move hardware init to enable callback
On 12/7/21 17:28, Dave Stevenson wrote: Hi, [...] Secondly the datasheet states that the DSI Reference Clock Source Division Selection is done by the MODE1 pin, and switches between HSCKBY2 divided by 7 and HSCKBY2 divided by 9. There is a stated assumption that HSCK is either 364MHz or 468MHz, thereby ending up with 26MHz. To do this we have to be running DSI in burst mode, but the support for that in DRM is near zero. Yes, you have to run the DSI clock lane at either of the two clock frequencies, that is rather limiting. The DSI has to be running in continuous clock mode, this has nothing to do with burst mode, the burst mode is a DSI host setting which is supported by most DSI hosts. OK burst mode is technically dropping the lane to LP mode, and you don't need that state transition. To quote the Toshiba datasheet: "As the clock derived from the DSICLK has to be fixed at 13, 26, 19.2 or 38.4 MHz, the DSI Host may need to run DSI clock lane at higher frequency than needed by frame rate and may have to send the DSI video mode transmissions in burst mode (explained in DSI section of this specification) " So where are you configuring the DSI clock lane to be running at one of those frequencies? Or are you requiring your panel to be running with a matching pixel clock? This is a preparatory patch, so nowhere yet. I forced the clock lane to the required clock frequency on the DSI host side thus far. You can still configure the chip to derive clock from Xtal, even with DSI as data input. Ah, I'd read too much into the commit text and read it as already fully implemented with a DSI derived clock instead of refclk. Sorry. Are you already working on the infrastructure changes, or are they just vaguely planned? I plan to implement those changes, when I have time to do that.
Re: [PATCH 2/4] drm/bridge: tc358767: Move hardware init to enable callback
On Tue, 7 Dec 2021 at 13:59, Marek Vasut wrote: > > On 12/7/21 14:34, Dave Stevenson wrote: > > Hi, > > The TC358767/TC358867/TC9595 are all capable of operating either from > attached Xtal or from DSI clock lane clock. In case the later is used, > all I2C accesses will fail until the DSI clock lane is running and > supplying clock to the chip. > > Move all hardware initialization to enable callback to guarantee the > DSI clock lane is running before accessing the hardware. In case Xtal > is attached to the chip, this change has no effect. > >>> > >>> This puzzles me as there seem to be a couple of ambiguities over how > >>> it would be used. > >>> > >>> Firstly the DT binding lists the clock as a required property, but > >>> there isn't one present if deriving the clock from the DSI clock lane. > >> > >> That's correct, the clock for the internal PLLs are derived from the DSI > >> clock lane. > > > > Does that mean you're creating a dummy clock device? > > If it's a required property then presumably yes, but it seems a little > > odd as that reflck does not exist. > > No. The refclk will become optional, but for that I need some more > infrastructure work, i.e. some way to communicate the requirements of > the DSI clock lane to the DSI host. > > >>> Secondly the datasheet states that the DSI Reference Clock Source > >>> Division Selection is done by the MODE1 pin, and switches between > >>> HSCKBY2 divided by 7 and HSCKBY2 divided by 9. There is a stated > >>> assumption that HSCK is either 364MHz or 468MHz, thereby ending up > >>> with 26MHz. To do this we have to be running DSI in burst mode, but > >>> the support for that in DRM is near zero. > >> > >> Yes, you have to run the DSI clock lane at either of the two clock > >> frequencies, that is rather limiting. The DSI has to be running in > >> continuous clock mode, this has nothing to do with burst mode, the burst > >> mode is a DSI host setting which is supported by most DSI hosts. > > > > OK burst mode is technically dropping the lane to LP mode, and you > > don't need that state transition. > > > > To quote the Toshiba datasheet: > > "As the clock derived from the > > DSICLK has to be fixed at 13, 26, 19.2 or 38.4 MHz, the DSI Host may > > need to run DSI clock lane at > > higher frequency than needed by frame rate and may have to send the > > DSI video mode transmissions in > > burst mode (explained in DSI section of this specification) " > > > > So where are you configuring the DSI clock lane to be running at one > > of those frequencies? Or are you requiring your panel to be running > > with a matching pixel clock? > > This is a preparatory patch, so nowhere yet. I forced the clock lane to > the required clock frequency on the DSI host side thus far. You can > still configure the chip to derive clock from Xtal, even with DSI as > data input. Ah, I'd read too much into the commit text and read it as already fully implemented with a DSI derived clock instead of refclk. Sorry. Are you already working on the infrastructure changes, or are they just vaguely planned? > >>> Can I ask how the chip has actually been configured and run in this mode? > >> > >> REFCLK Xtal disconnected and HSCKBY2/7 MODE0=H MODE1=L , that should be > >> all you need. Do you plan to develop a board with this bridge ? > > > > Yes, I have a board with this chip in DSI to DPI mode that I'm trying > > to get to work. The intent was to configure it via DSI LP commands > > rather than I2C, but currently it's refusing to do anything. > > I see.
Re: [PATCH] drm/bridge: parade-ps8640: Add backpointer to drm_device in drm_dp_aux
Hi, On Mon, Dec 6, 2021 at 5:44 PM Philip Chen wrote: > > Hi > > On Mon, Dec 6, 2021 at 4:29 PM Douglas Anderson wrote: > > > > When we added the support for the AUX channel in commit 13afcdd7277e > > ("drm/bridge: parade-ps8640: Add support for AUX channel") we forgot > > to set "drm_dev" to avoid the warning splat at the beginning of > > drm_dp_aux_register(). Since everything was working I guess I never > > noticed the splat when testing against mainline. In any case, it's > > easy to fix. This is basically just like commit 6cba3fe43341 ("drm/dp: > > Add backpointer to drm_device in drm_dp_aux") but just for the > > parade-ps8640. > > > > Fixes: 13afcdd7277e ("drm/bridge: parade-ps8640: Add support for AUX > > channel") > > Signed-off-by: Douglas Anderson > > --- > > > > drivers/gpu/drm/bridge/parade-ps8640.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c > > b/drivers/gpu/drm/bridge/parade-ps8640.c > > index 26898042ba3d..818704bf5e86 100644 > > --- a/drivers/gpu/drm/bridge/parade-ps8640.c > > +++ b/drivers/gpu/drm/bridge/parade-ps8640.c > > @@ -449,6 +449,7 @@ static int ps8640_bridge_attach(struct drm_bridge > > *bridge, > > if (!(flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR)) > > return -EINVAL; > > > > + ps_bridge->aux.drm_dev = bridge->dev; > > ret = drm_dp_aux_register(&ps_bridge->aux); > > if (ret) { > > dev_err(dev, "failed to register DP AUX channel: %d\n", > > ret); > > -- > > 2.34.1.400.ga245620fadb-goog > > > > Signed-off-by: Philip Chen That's probably not quite the right tag. I'm going to assume you meant Reviewed-by? Usually Signed-off-by is added if you were one of the authors of the patch or you were a maintainer that "touched" the patch on its way to landing upstream... -Doug
[PATCH v3 6/6] Documentation/gpu: Add amdgpu and dc glossary
In the DC driver, we have multiple acronyms that are not obvious most of the time; the same idea is valid for amdgpu. This commit introduces a DC and amdgpu glossary in order to make it easier to navigate through our driver. Changes since V2: - Add MMHUB Changes since V1: - Yann: Divide glossary based on driver context. - Alex: Make terms more consistent and update CPLIB - Add new acronyms to the glossary Signed-off-by: Rodrigo Siqueira --- Documentation/gpu/amdgpu/amdgpu-glossary.rst | 50 .../gpu/amdgpu/display/dc-glossary.rst| 243 ++ Documentation/gpu/amdgpu/display/index.rst| 1 + Documentation/gpu/amdgpu/index.rst| 7 + 4 files changed, 301 insertions(+) create mode 100644 Documentation/gpu/amdgpu/amdgpu-glossary.rst create mode 100644 Documentation/gpu/amdgpu/display/dc-glossary.rst diff --git a/Documentation/gpu/amdgpu/amdgpu-glossary.rst b/Documentation/gpu/amdgpu/amdgpu-glossary.rst new file mode 100644 index ..65803bfac776 --- /dev/null +++ b/Documentation/gpu/amdgpu/amdgpu-glossary.rst @@ -0,0 +1,50 @@ +=== +AMDGPU Glossary +=== + +Here you can find some generic acronyms used in the amdgpu driver. Notice that +we have a dedicated glossary for Display Core. + +.. glossary:: + +CPLIB + Content Protection Library + +DFS + Digital Frequency Synthesizer + +ECP + Enhanced Content Protection + +EOP + End Of Pipe/Pipeline + +HQD + Hardware Queue Descriptor + +KCQ + Kernel Compute Queue + +KGQ + Kernel Graphics Queue + +KIQ + Kernel Interface Queue + +MMHUB + Multi-Media HUB + +MQD + Memory Queue Descriptor + +PPLib + PowerPlay Library - PowerPlay is the power management component. + +SMU + System Management Unit + +VCE + Video Compression Engine + +VCN + Video Codec Next diff --git a/Documentation/gpu/amdgpu/display/dc-glossary.rst b/Documentation/gpu/amdgpu/display/dc-glossary.rst new file mode 100644 index ..547c0bfbb3e2 --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dc-glossary.rst @@ -0,0 +1,243 @@ +=== +DC Glossary +=== + +On this page, we try to keep track of acronyms related to the display +component. If you do not find what you are looking for, look at the amdgpu +glossary; if you cannot find it anywhere, consider asking in the amdgfx and +update this page. + +.. glossary:: + +ABM + Adaptive Backlight Modulation + +APU + Accelerated Processing Unit + +ASIC + Application-Specific Integrated Circuit + +ASSR + Alternate Scrambler Seed Reset + +AZ + Azalia (HD audio DMA engine) + +BPC + Bits Per Colour/Component + +BPP + Bits Per Pixel + +Clocks + * PCLK: Pixel Clock + * SYMCLK: Symbol Clock + * SOCCLK: GPU Engine Clock + * DISPCLK: Display Clock + * DPPCLK: DPP Clock + * DCFCLK: Display Controller Fabric Clock + * REFCLK: Real Time Reference Clock + * PPLL: Pixel PLL + * FCLK: Fabric Clock + * MCLK: Memory Clock + +CRC + Cyclic Redundancy Check + +CRTC + Cathode Ray Tube Controller - commonly called "Controller" - Generates + raw stream of pixels, clocked at pixel clock + +CVT + Coordinated Video Timings + +DAL + Display Abstraction layer + +DC (Software) + Display Core + +DC (Hardware) + Display Controller + +DCC + Delta Colour Compression + +DCE + Display Controller Engine + +DCHUB + Display Controller HUB + +ARB + Arbiter + +VTG + Vertical Timing Generator + +DCN + Display Core Next + +DCCG + Display Clock Generator block + +DDC + Display Data Channel + +DIO + Display IO + +DPP + Display Pipes and Planes + +DSC + Display Stream Compression (Reduce the amount of bits to represent pixel + count while at the same pixel clock) + +dGPU + discrete GPU + +DMIF + Display Memory Interface + +DML + Display Mode Library + +DMCU + Display Micro-Controller Unit + +DMCUB + Display Micro-Controller Unit, version B + +DPCD + DisplayPort Configuration Data + +DPM(S) + Display Power Management (Signaling) + +DRR + Dynamic Refresh Rate + +DWB + Display Writeback + +FB + Frame Buffer + +FBC + Frame Buffer Compression + +FEC + Forward Error Correction + +FRL + Fixed Rate Link + +GCO + Graphical Controller Object + +GMC + Graphic Memory Controller + +GSL + Global Swap Lock + +iGPU + integrated GPU + +IH + Interrupt Handler + +ISR + Interrupt Service Request + +ISV + Independent Software Vendor + +KMD + Kernel Mode Driver + +LB + Line Buffer + +LFC + Low Framerate Compensation + +
[PATCH v3 5/6] Documentation/gpu: Add basic overview of DC pipeline
This commit describes how DCN works by providing high-level diagrams with an explanation of each component. In particular, it details the Global Sync signals. Change since V2: - Add a comment about MMHUBBUB. Signed-off-by: Rodrigo Siqueira --- .../gpu/amdgpu/display/config_example.svg | 414 ++ .../amdgpu/display/dc_pipeline_overview.svg | 1125 + .../gpu/amdgpu/display/dcn-overview.rst | 171 +++ .../gpu/amdgpu/display/global_sync_vblank.svg | 485 +++ Documentation/gpu/amdgpu/display/index.rst| 23 +- 5 files changed, 2206 insertions(+), 12 deletions(-) create mode 100644 Documentation/gpu/amdgpu/display/config_example.svg create mode 100644 Documentation/gpu/amdgpu/display/dc_pipeline_overview.svg create mode 100644 Documentation/gpu/amdgpu/display/dcn-overview.rst create mode 100644 Documentation/gpu/amdgpu/display/global_sync_vblank.svg diff --git a/Documentation/gpu/amdgpu/display/config_example.svg b/Documentation/gpu/amdgpu/display/config_example.svg new file mode 100644 index ..cdac9858601c --- /dev/null +++ b/Documentation/gpu/amdgpu/display/config_example.svg @@ -0,0 +1,414 @@ + + + +http://purl.org/dc/elements/1.1/"; + xmlns:cc="http://creativecommons.org/ns#"; + xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"; + xmlns:svg="http://www.w3.org/2000/svg"; + xmlns="http://www.w3.org/2000/svg"; + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"; + xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"; + width="144.63406mm" + height="66.596054mm" + viewBox="0 0 144.15195 66.596054" + version="1.1" + id="svg8" + inkscape:version="0.92.5 (2060ec1f9f, 2020-04-08)" + sodipodi:docname="config_example.svg"> + + + + + + + + + + + + + + + + + + + + + + + +image/svg+xml +http://purl.org/dc/dcmitype/StillImage"; /> + + + + + + + + + + + + + + + +Configurations +A +B +C + + + + + +A +B +C +C +Old config +Old config + + +VUpdate +UpdateLock +Register updatePending Status +Buf 0 +Buf 1 + + diff --git a/Documentation/gpu/amdgpu/display/dc_pipeline_overview.svg b/Documentation/gpu/amdgpu/display/dc_pipeline_overview.svg new file mode 100644 index ..9adecebfe65b --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dc_pipeline_overview.svg @@ -0,0 +1,1125 @@ + + + +http://purl.org/dc/elements/1.1/"; + xmlns:cc="http://creativecommons.org/ns#"; + xmlns:rdf="http://www.w3.org/1999/02/22-rdf-syntax-ns#"; + xmlns:svg="http://www.w3.org/2000/svg"; + xmlns="http://www.w3.org/2000/svg"; + xmlns:sodipodi="http://sodipodi.sourceforge.net/DTD/sodipodi-0.dtd"; + xmlns:inkscape="http://www.inkscape.org/namespaces/inkscape"; + width="1296.7491" + height="741.97845" + viewBox="0 0 343.0982 196.31514" + version="1.1" + id="svg8" + inkscape:version="0.92.5 (2060ec1f9f, 2020-04-08)" + sodipodi:docname="dc_pipeline_overview.svg"> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +image/svg+xml +http://purl.org/dc/dcmitype/StillImage"; /> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + DCHUB + HUBP(n) + + + DPP(n) + + + + MPC + + + + OPTC + + + + DIO + + + + DCCG + + + + DMU + + + + + AZ + + + + MMHUBBUB + + + + DWB(n) + + + + + + + + + + + + +Global sync +Pixel data +Sideband signal +Config. Bus + +SDP + +Monitor + + OPP + + + + + + + + + + + + +dc_plane +dc_stream + + + +dc_state + + + +Code struct + + + +dc_link + + + +Floating pointcalculation + + + +bit-depthreduction/dither +} +Notes + + diff --git a/Documentation/gpu/amdgpu/display/dcn-overview.rst b/Documentation/gpu/amdgpu/display/dcn-overview.rst new file mode 100644 index ..f98624d7828e --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dcn-overview.rst @@ -0,0 +1,171 @@ +
[PATCH v3 1/6] Documentation/gpu: Reorganize DC documentation
Display core documentation is not well organized, and it is hard to find information due to the lack of sections. This commit reorganizes the documentation layout, and it is preparation work for future changes. Changes since V1: - Christian: Group amdgpu documentation together. - Daniel: Drop redundant amdgpu prefix. - Jani: Create index pages. - Yann: Mirror display folder in the documentation. Signed-off-by: Rodrigo Siqueira --- Documentation/gpu/amdgpu-dc.rst | 74 --- Documentation/gpu/amdgpu/display/dc-debug.rst | 4 + .../gpu/amdgpu/display/display-manager.rst| 42 +++ Documentation/gpu/amdgpu/display/index.rst| 29 .../gpu/{amdgpu.rst => amdgpu/index.rst} | 18 - Documentation/gpu/drivers.rst | 3 +- 6 files changed, 91 insertions(+), 79 deletions(-) delete mode 100644 Documentation/gpu/amdgpu-dc.rst create mode 100644 Documentation/gpu/amdgpu/display/dc-debug.rst create mode 100644 Documentation/gpu/amdgpu/display/display-manager.rst create mode 100644 Documentation/gpu/amdgpu/display/index.rst rename Documentation/gpu/{amdgpu.rst => amdgpu/index.rst} (96%) diff --git a/Documentation/gpu/amdgpu-dc.rst b/Documentation/gpu/amdgpu-dc.rst deleted file mode 100644 index f7ff7e1309de.. --- a/Documentation/gpu/amdgpu-dc.rst +++ /dev/null @@ -1,74 +0,0 @@ -=== -drm/amd/display - Display Core (DC) -=== - -*placeholder - general description of supported platforms, what dc is, etc.* - -Because it is partially shared with other operating systems, the Display Core -Driver is divided in two pieces. - -1. **Display Core (DC)** contains the OS-agnostic components. Things like - hardware programming and resource management are handled here. -2. **Display Manager (DM)** contains the OS-dependent components. Hooks to the - amdgpu base driver and DRM are implemented here. - -It doesn't help that the entire package is frequently referred to as DC. But -with the context in mind, it should be clear. - -When CONFIG_DRM_AMD_DC is enabled, DC will be initialized by default for -supported ASICs. To force disable, set `amdgpu.dc=0` on kernel command line. -Likewise, to force enable on unsupported ASICs, set `amdgpu.dc=1`. - -To determine if DC is loaded, search dmesg for the following entry: - -``Display Core initialized with `` - -AMDgpu Display Manager -== - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: overview - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h - :internal: - -Lifecycle -- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: DM Lifecycle - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :functions: dm_hw_init dm_hw_fini - -Interrupts --- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c - :doc: overview - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c - :internal: - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :functions: register_hpd_handlers dm_crtc_high_irq dm_pflip_high_irq - -Atomic Implementation -- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: atomic - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :functions: amdgpu_dm_atomic_check amdgpu_dm_atomic_commit_tail - -Display Core - - -**WIP** - -FreeSync Video --- - -.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c - :doc: FreeSync Video diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst b/Documentation/gpu/amdgpu/display/dc-debug.rst new file mode 100644 index ..bbb8c3fc8eee --- /dev/null +++ b/Documentation/gpu/amdgpu/display/dc-debug.rst @@ -0,0 +1,4 @@ +Display Core Debug tools + + +TODO diff --git a/Documentation/gpu/amdgpu/display/display-manager.rst b/Documentation/gpu/amdgpu/display/display-manager.rst new file mode 100644 index ..7ce31f89d9a0 --- /dev/null +++ b/Documentation/gpu/amdgpu/display/display-manager.rst @@ -0,0 +1,42 @@ +== +AMDgpu Display Manager +== + +.. contents:: Table of Contents +:depth: 3 + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c + :doc: overview + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h + :internal: + +Lifecycle += + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c + :doc: DM Lifecycle + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c + :functions: dm_hw_init dm_hw_fini + +Interrupts +== + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c + :doc: overview + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_irq.c + :internal: + +.. kernel-doc:: drivers/gpu/drm/amd/display/amdgp
[PATCH v3 2/6] Documentation/gpu: Document amdgpu_dm_visual_confirm debugfs entry
Display core provides a feature that makes it easy for users to debug Multiple planes by enabling a visual notification at the bottom of each plane. This commit introduces how to use such a feature. Signed-off-by: Rodrigo Siqueira --- Documentation/gpu/amdgpu/display/dc-debug.rst | 34 ++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst b/Documentation/gpu/amdgpu/display/dc-debug.rst index bbb8c3fc8eee..532cbbd64863 100644 --- a/Documentation/gpu/amdgpu/display/dc-debug.rst +++ b/Documentation/gpu/amdgpu/display/dc-debug.rst @@ -1,4 +1,36 @@ + Display Core Debug tools -TODO +DC Debugfs +== + +Multiple Planes Debug +- + +If you want to enable or debug multiple planes in a specific user-space +application, you can leverage a debug feature named visual confirm. For +enabling it, you will need:: + + echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_visual_confirm + +You need to reload your GUI to see the visual confirmation. When the plane +configuration changes or a full update occurs there will be a colored bar at +the bottom of each hardware plane being drawn on the screen. + +* The color indicates the format - For example, red is AR24 and green is NV12 +* The height of the bar indicates the index of the plane +* Pipe split can be observed if there are two bars with a difference in height + covering the same plane + +Consider the video playback case in which a video is played in a specific +plane, and the desktop is drawn in another plane. The video plane should +feature one or two green bars at the bottom of the video depending on pipe +split configuration. + +* There should **not** be any visual corruption +* There should **not** be any underflow or screen flashes +* There should **not** be any black screens +* There should **not** be any cursor corruption +* Multiple plane **may** be briefly disabled during window transitions or + resizing but should come back after the action has finished -- 2.25.1
[PATCH v3 4/6] Documentation/gpu: How to collect DTN log
Introduce how to collect DTN log from debugfs. Signed-off-by: Rodrigo Siqueira --- Documentation/gpu/amdgpu/display/dc-debug.rst | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst b/Documentation/gpu/amdgpu/display/dc-debug.rst index 6dbd21f7f59e..40c55a618918 100644 --- a/Documentation/gpu/amdgpu/display/dc-debug.rst +++ b/Documentation/gpu/amdgpu/display/dc-debug.rst @@ -58,3 +58,20 @@ In this case, if you have a pipe split, you will see one small red bar at the bottom of the display covering the entire display width and another bar covering the second pipe. In other words, you will see a bit high bar in the second pipe. + +DTN Debug += + +DC (DCN) provides an extensive log that dumps multiple details from our +hardware configuration. Via debugfs, you can capture those status values by +using Display Test Next (DTN) log, which can be captured via debugfs by using:: + + cat /sys/kernel/debug/dri/0/amdgpu_dm_dtn_log + +Since this log is updated accordingly with DCN status, you can also follow the +change in real-time by using something like:: + + sudo watch -d cat /sys/kernel/debug/dri/0/amdgpu_dm_dtn_log + +When reporting a bug related to DC, consider attaching this log before and +after you reproduce the bug. -- 2.25.1
[PATCH v3 3/6] Documentation/gpu: Document pipe split visual confirmation
Display core provides a feature that makes it easy for users to debug Pipe Split. This commit introduces how to use such a debug option. Signed-off-by: Rodrigo Siqueira --- Documentation/gpu/amdgpu/display/dc-debug.rst | 28 +-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/Documentation/gpu/amdgpu/display/dc-debug.rst b/Documentation/gpu/amdgpu/display/dc-debug.rst index 532cbbd64863..6dbd21f7f59e 100644 --- a/Documentation/gpu/amdgpu/display/dc-debug.rst +++ b/Documentation/gpu/amdgpu/display/dc-debug.rst @@ -2,8 +2,18 @@ Display Core Debug tools -DC Debugfs -== +DC Visual Confirmation +== + +Display core provides a feature named visual confirmation, which is a set of +bars added at the scanout time by the driver to convey some specific +information. In general, you can enable this debug option by using:: + + echo > /sys/kernel/debug/dri/0/amdgpu_dm_visual_confirm + +Where `N` is an integer number for some specific scenarios that the developer +wants to enable, you will see some of these debug cases in the following +subsection. Multiple Planes Debug - @@ -34,3 +44,17 @@ split configuration. * There should **not** be any cursor corruption * Multiple plane **may** be briefly disabled during window transitions or resizing but should come back after the action has finished + +Pipe Split Debug + + +Sometimes we need to debug if DCN is splitting pipes correctly, and visual +confirmation is also handy for this case. Similar to the MPO case, you can use +the below command to enable visual confirmation:: + + echo 1 > /sys/kernel/debug/dri/0/amdgpu_dm_visual_confirm + +In this case, if you have a pipe split, you will see one small red bar at the +bottom of the display covering the entire display width and another bar +covering the second pipe. In other words, you will see a bit high bar in the +second pipe. -- 2.25.1
[PATCH v3 0/6] Expand display core documentation
Display Core (DC) is one of the components under amdgpu, and it has multiple features directly related to the KMS API. Unfortunately, we don't have enough documentation about DC in the upstream, which makes the life of some external contributors a little bit more challenging. For these reasons, this patchset reworks part of the DC documentation and introduces a new set of details on how the display core works on DCN IP. Another improvement that this documentation effort tries to bring is making explicit some of our hardware-specific details to guide user-space developers better. In my view, it is easier to review this series if you apply it in your local kernel and build the HTML version (make htmldocs). I'm suggesting this approach because I added a few SVG diagrams that will be easier to see in the HTML version. If you cannot build the documentation, try to open the SVG images while reviewing the content. In summary, in this series, you will find: 1. Patch 1: Re-arrange of display core documentation. This is preparation work for the other patches, but it is also a way to expand this documentation. 2. Patch 2 to 4: Document some common debug options related to display. 3. Patch 5: This patch provides an overview of how our display core next works and a brief explanation of each component. 4. Patch 6: We use a lot of acronyms in our driver; for this reason, we exposed a glossary with common terms used by display core. Please let us know what you think we can improve this series and what kind of content you want to see for the next series. Changes since V2: - Add a comment about MMHUBBUB Changes since V1: - Group amdgpu documentation together. - Create index pages. - Mirror display folder in the documentation. - Divide glossary based on driver context. - Make terms more consistent and update CPLIB - Add new acronyms to the glossary Thanks Siqueira Rodrigo Siqueira (6): Documentation/gpu: Reorganize DC documentation Documentation/gpu: Document amdgpu_dm_visual_confirm debugfs entry Documentation/gpu: Document pipe split visual confirmation Documentation/gpu: How to collect DTN log Documentation/gpu: Add basic overview of DC pipeline Documentation/gpu: Add amdgpu and dc glossary Documentation/gpu/amdgpu-dc.rst | 74 -- Documentation/gpu/amdgpu/amdgpu-glossary.rst | 50 + .../gpu/amdgpu/display/config_example.svg | 414 ++ Documentation/gpu/amdgpu/display/dc-debug.rst | 77 ++ .../gpu/amdgpu/display/dc-glossary.rst| 243 .../amdgpu/display/dc_pipeline_overview.svg | 1125 + .../gpu/amdgpu/display/dcn-overview.rst | 171 +++ .../gpu/amdgpu/display/display-manager.rst| 42 + .../gpu/amdgpu/display/global_sync_vblank.svg | 485 +++ Documentation/gpu/amdgpu/display/index.rst| 29 + .../gpu/{amdgpu.rst => amdgpu/index.rst} | 25 +- Documentation/gpu/drivers.rst |3 +- 12 files changed, 2659 insertions(+), 79 deletions(-) delete mode 100644 Documentation/gpu/amdgpu-dc.rst create mode 100644 Documentation/gpu/amdgpu/amdgpu-glossary.rst create mode 100644 Documentation/gpu/amdgpu/display/config_example.svg create mode 100644 Documentation/gpu/amdgpu/display/dc-debug.rst create mode 100644 Documentation/gpu/amdgpu/display/dc-glossary.rst create mode 100644 Documentation/gpu/amdgpu/display/dc_pipeline_overview.svg create mode 100644 Documentation/gpu/amdgpu/display/dcn-overview.rst create mode 100644 Documentation/gpu/amdgpu/display/display-manager.rst create mode 100644 Documentation/gpu/amdgpu/display/global_sync_vblank.svg create mode 100644 Documentation/gpu/amdgpu/display/index.rst rename Documentation/gpu/{amdgpu.rst => amdgpu/index.rst} (95%) -- 2.25.1
[PATCH v8 6/6] drm/sprd: add Unisoc's drm mipi dsi&dphy driver
Adds dsi host controller support for the Unisoc's display subsystem. Adds dsi phy support for the Unisoc's display subsystem. Only MIPI DSI Displays supported, DP/TV/HMDI will be support in the feature. v1: - Remove dphy and dsi graph binding, merge the dphy driver into the dsi. v2: - Use drm_xxx to replace all DRM_XXX. - Use kzalloc to replace devm_kzalloc for sprd_dsi structure init. v4: - Use drmm_helpers to allocate encoder. - Move allocate encoder and connector to bind function. v5: - Drop the dsi ip file prefix. - Fix the checkpatch warnings. - Add Signed-off-by for dsi&dphy patch. - Use the mode_flags of mipi_dsi_device to setup crtc DPI and EDPI mode. v6: - Redesign the way to access the dsi register. - Reduce the dsi_context member variables. v7: - Fix codeing style issue by checkpatch. - Drop the pll registers structure define. - Use bridge API instead of drm panel API. - Register mipi_dsi_host on probe phase; - Remove some unused function. v8: - Fix missing signed-off-by. - Move component_add to dsi_host.attach callback. Cc: Orson Zhai Cc: Chunyan Zhang Signed-off-by: Kevin Tang --- drivers/gpu/drm/sprd/Kconfig |1 + drivers/gpu/drm/sprd/Makefile|8 +- drivers/gpu/drm/sprd/megacores_pll.c | 305 drivers/gpu/drm/sprd/sprd_dpu.c | 13 + drivers/gpu/drm/sprd/sprd_drm.c |1 + drivers/gpu/drm/sprd/sprd_drm.h |1 + drivers/gpu/drm/sprd/sprd_dsi.c | 1073 ++ drivers/gpu/drm/sprd/sprd_dsi.h | 126 +++ 8 files changed, 1526 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/sprd/megacores_pll.c create mode 100644 drivers/gpu/drm/sprd/sprd_dsi.c create mode 100644 drivers/gpu/drm/sprd/sprd_dsi.h diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig index 37762c333..3edeaeca0 100644 --- a/drivers/gpu/drm/sprd/Kconfig +++ b/drivers/gpu/drm/sprd/Kconfig @@ -5,6 +5,7 @@ config DRM_SPRD select DRM_GEM_CMA_HELPER select DRM_KMS_CMA_HELPER select DRM_KMS_HELPER + select DRM_MIPI_DSI select VIDEOMODE_HELPERS help Choose this option if you have a Unisoc chipset. diff --git a/drivers/gpu/drm/sprd/Makefile b/drivers/gpu/drm/sprd/Makefile index ab12b95e6..e82e6a6f8 100644 --- a/drivers/gpu/drm/sprd/Makefile +++ b/drivers/gpu/drm/sprd/Makefile @@ -1,4 +1,8 @@ # SPDX-License-Identifier: GPL-2.0 -obj-y := sprd_drm.o \ - sprd_dpu.o +sprd-drm-y := sprd_drm.o \ + sprd_dpu.o \ + sprd_dsi.o \ + megacores_pll.o + +obj-$(CONFIG_DRM_SPRD) += sprd-drm.o \ No newline at end of file diff --git a/drivers/gpu/drm/sprd/megacores_pll.c b/drivers/gpu/drm/sprd/megacores_pll.c new file mode 100644 index 0..3091dfdc1 --- /dev/null +++ b/drivers/gpu/drm/sprd/megacores_pll.c @@ -0,0 +1,305 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2020 Unisoc Inc. + */ + +#include +#include +#include +#include +#include +#include + +#include "sprd_dsi.h" + +#define L 0 +#define H 1 +#define CLK0 +#define DATA 1 +#define INFINITY 0x +#define MIN_OUTPUT_FREQ(100) + +#define AVERAGE(a, b) (min(a, b) + abs((b) - (a)) / 2) + +/* sharkle */ +#define VCO_BAND_LOW 750 +#define VCO_BAND_MID 1100 +#define VCO_BAND_HIGH 1500 +#define PHY_REF_CLK26000 + +static int dphy_calc_pll_param(struct dphy_pll *pll) +{ + const u32 khz = 1000; + const u32 mhz = 100; + const unsigned long long factor = 100; + unsigned long long tmp; + int i; + + pll->potential_fvco = pll->freq / khz; + pll->ref_clk = PHY_REF_CLK / khz; + + for (i = 0; i < 4; ++i) { + if (pll->potential_fvco >= VCO_BAND_LOW && + pll->potential_fvco <= VCO_BAND_HIGH) { + pll->fvco = pll->potential_fvco; + pll->out_sel = BIT(i); + break; + } + pll->potential_fvco <<= 1; + } + if (pll->fvco == 0) + return -EINVAL; + + if (pll->fvco >= VCO_BAND_LOW && pll->fvco <= VCO_BAND_MID) { + /* vco band control */ + pll->vco_band = 0x0; + /* low pass filter control */ + pll->lpf_sel = 1; + } else if (pll->fvco > VCO_BAND_MID && pll->fvco <= VCO_BAND_HIGH) { + pll->vco_band = 0x1; + pll->lpf_sel = 0; + } else { + return -EINVAL; + } + + pll->nint = pll->fvco / pll->ref_clk; + tmp = pll->fvco * factor * mhz; + do_div(tmp, pll->ref_clk); + tmp = tmp - pll->nint * factor * mhz; + tmp *= BIT(20); + do_div(tmp, 1); + pll->kint = (u32)tmp; +
[PATCH v8 5/6] dt-bindings: display: add Unisoc's mipi dsi controller bindings
From: Kevin Tang Adds MIPI DSI Controller support for Unisoc's display subsystem. v5: - Remove panel_in port for dsi node. Cc: Orson Zhai Cc: Chunyan Zhang Signed-off-by: Kevin Tang Reviewed-by: Rob Herring --- .../display/sprd/sprd,sharkl3-dsi-host.yaml | 88 +++ 1 file changed, 88 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml diff --git a/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml new file mode 100644 index 0..bc5594d18 --- /dev/null +++ b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dsi-host.yaml @@ -0,0 +1,88 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/sprd/sprd,sharkl3-dsi-host.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Unisoc MIPI DSI Controller + +maintainers: + - Kevin Tang + +properties: + compatible: +const: sprd,sharkl3-dsi-host + + reg: +maxItems: 1 + + interrupts: +maxItems: 2 + + clocks: +minItems: 1 + + clock-names: +items: + - const: clk_src_96m + + power-domains: +maxItems: 1 + + ports: +type: object + +properties: + "#address-cells": +const: 1 + + "#size-cells": +const: 0 + + port@0: +type: object +description: + A port node with endpoint definitions as defined in + Documentation/devicetree/bindings/media/video-interfaces.txt. + That port should be the input endpoint, usually coming from + the associated DPU. + +required: + - "#address-cells" + - "#size-cells" + - port@0 + +additionalProperties: false + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + - ports + +additionalProperties: false + +examples: + - | +#include +#include +dsi: dsi@6310 { +compatible = "sprd,sharkl3-dsi-host"; +reg = <0x6310 0x1000>; +interrupts = , + ; +clock-names = "clk_src_96m"; +clocks = <&pll CLK_TWPLL_96M>; +ports { +#address-cells = <1>; +#size-cells = <0>; +port@0 { +reg = <0>; +dsi_in: endpoint { +remote-endpoint = <&dpu_out>; +}; +}; +}; +}; -- 2.29.0
[PATCH v8 4/6] drm/sprd: add Unisoc's drm display controller driver
Adds DPU(Display Processor Unit) support for the Unisoc's display subsystem. It's support multi planes, scaler, rotation, PQ(Picture Quality) and more. v2: - Use drm_xxx to replace all DRM_XXX. - Use kzalloc to replace devm_kzalloc for sprd_dpu structure init. v3: - Remove dpu_layer stuff layer and commit layers by aotmic_update v4: - Use drmm_helpers to allocate crtc and planes. - Move rotation enum definitions to crtc layer reg bitfields. - Move allocate crtc and planes to bind function. v5: - Fix the checkpatch warnings. - Use mode_set_nofb instead of mode_valid callback. - Follow the OF-Graph bindings, use of_graph_get_port_by_id instead of of_parse_phandle. - Use zpos to represent the layer position. - Rebase to last drm misc branch. v6: - Disable and clear interrupts before register dpu IRQ - Init dpi config used by crtc_state->adjusted_mode on mode_set_nofb - Remove enable_irq and disable_irq function call. - Remove drm_format_info function call. v7: - Remove iommu error interrupt handling function. Cc: Orson Zhai Cc: Chunyan Zhang Signed-off-by: Kevin Tang Reviewed-by: Maxime Ripard --- drivers/gpu/drm/sprd/Kconfig| 1 + drivers/gpu/drm/sprd/Makefile | 3 +- drivers/gpu/drm/sprd/sprd_dpu.c | 867 drivers/gpu/drm/sprd/sprd_dpu.h | 109 drivers/gpu/drm/sprd/sprd_drm.c | 1 + drivers/gpu/drm/sprd/sprd_drm.h | 2 + 6 files changed, 982 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/sprd/sprd_dpu.c create mode 100644 drivers/gpu/drm/sprd/sprd_dpu.h diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig index 726c3e76d..37762c333 100644 --- a/drivers/gpu/drm/sprd/Kconfig +++ b/drivers/gpu/drm/sprd/Kconfig @@ -5,6 +5,7 @@ config DRM_SPRD select DRM_GEM_CMA_HELPER select DRM_KMS_CMA_HELPER select DRM_KMS_HELPER + select VIDEOMODE_HELPERS help Choose this option if you have a Unisoc chipset. If M is selected the module will be called sprd_drm. diff --git a/drivers/gpu/drm/sprd/Makefile b/drivers/gpu/drm/sprd/Makefile index 9850f00b8..ab12b95e6 100644 --- a/drivers/gpu/drm/sprd/Makefile +++ b/drivers/gpu/drm/sprd/Makefile @@ -1,3 +1,4 @@ # SPDX-License-Identifier: GPL-2.0 -obj-y := sprd_drm.o +obj-y := sprd_drm.o \ + sprd_dpu.o diff --git a/drivers/gpu/drm/sprd/sprd_dpu.c b/drivers/gpu/drm/sprd/sprd_dpu.c new file mode 100644 index 0..1d10d0998 --- /dev/null +++ b/drivers/gpu/drm/sprd/sprd_dpu.c @@ -0,0 +1,867 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2020 Unisoc Inc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "sprd_drm.h" +#include "sprd_dpu.h" + +/* Global control registers */ +#define REG_DPU_CTRL 0x04 +#define REG_DPU_CFG0 0x08 +#define REG_PANEL_SIZE 0x20 +#define REG_BLEND_SIZE 0x24 +#define REG_BG_COLOR 0x2C + +/* Layer0 control registers */ +#define REG_LAY_BASE_ADDR0 0x30 +#define REG_LAY_BASE_ADDR1 0x34 +#define REG_LAY_BASE_ADDR2 0x38 +#define REG_LAY_CTRL 0x40 +#define REG_LAY_SIZE 0x44 +#define REG_LAY_PITCH 0x48 +#define REG_LAY_POS0x4C +#define REG_LAY_ALPHA 0x50 +#define REG_LAY_CROP_START 0x5C + +/* Interrupt control registers */ +#define REG_DPU_INT_EN 0x1E0 +#define REG_DPU_INT_CLR0x1E4 +#define REG_DPU_INT_STS0x1E8 + +/* DPI control registers */ +#define REG_DPI_CTRL 0x1F0 +#define REG_DPI_H_TIMING 0x1F4 +#define REG_DPI_V_TIMING 0x1F8 + +/* MMU control registers */ +#define REG_MMU_EN 0x800 +#define REG_MMU_VPN_RANGE 0x80C +#define REG_MMU_PPN1 0x83C +#define REG_MMU_RANGE1 0x840 +#define REG_MMU_PPN2 0x844 +#define REG_MMU_RANGE2 0x848 + +/* Global control bits */ +#define BIT_DPU_RUNBIT(0) +#define BIT_DPU_STOP BIT(1) +#define BIT_DPU_REG_UPDATE BIT(2) +#define BIT_DPU_IF_EDPIBIT(0) + +/* Layer control bits */ +#define BIT_DPU_LAY_EN BIT(0) +#define BIT_DPU_LAY_LAYER_ALPHA(0x01 << 2) +#define BIT_DPU_LAY_COMBO_ALPHA(0x02 << 2) +#define BIT_DPU_LAY_FORMAT_YUV422_2PLANE (0x00 << 4) +#define BIT_DPU_LAY_FORMAT_YUV420_2PLANE (0x01 << 4) +#define BIT_DPU_LAY_FORMAT_YUV420_3PLANE (0x02 << 4) +#define BIT_DPU_LAY_FORMAT_ARGB(0x03 << 4) +#define BIT_DPU_LAY_FORMAT_RGB565 (0x04 << 4) +#define BIT_DPU_LAY_DATA_ENDIAN_B0B1B2B3 (0x00 << 8) +#define BIT_DPU_LAY_DATA_ENDIAN_B3B2B1B0 (0x01 << 8) +#define BIT_DPU_
[PATCH v8 3/6] dt-bindings: display: add Unisoc's dpu bindings
From: Kevin Tang DPU (Display Processor Unit) is the Display Controller for the Unisoc SoCs which transfers the image data from a video memory buffer to an internal LCD interface. Cc: Orson Zhai Cc: Chunyan Zhang Signed-off-by: Kevin Tang Reviewed-by: Rob Herring --- .../display/sprd/sprd,sharkl3-dpu.yaml| 77 +++ 1 file changed, 77 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dpu.yaml diff --git a/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dpu.yaml b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dpu.yaml new file mode 100644 index 0..4ebea60b8 --- /dev/null +++ b/Documentation/devicetree/bindings/display/sprd/sprd,sharkl3-dpu.yaml @@ -0,0 +1,77 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/sprd/sprd,sharkl3-dpu.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Unisoc Sharkl3 Display Processor Unit (DPU) + +maintainers: + - Kevin Tang + +description: | + DPU (Display Processor Unit) is the Display Controller for the Unisoc SoCs + which transfers the image data from a video memory buffer to an internal + LCD interface. + +properties: + compatible: +const: sprd,sharkl3-dpu + + reg: +maxItems: 1 + + interrupts: +maxItems: 1 + + clocks: +minItems: 2 + + clock-names: +items: + - const: clk_src_128m + - const: clk_src_384m + + power-domains: +maxItems: 1 + + iommus: +maxItems: 1 + + port: +type: object +description: + A port node with endpoint definitions as defined in + Documentation/devicetree/bindings/media/video-interfaces.txt. + That port should be the output endpoint, usually output to + the associated DSI. + +required: + - compatible + - reg + - interrupts + - clocks + - clock-names + - port + +additionalProperties: false + +examples: + - | +#include +#include +dpu: dpu@6300 { +compatible = "sprd,sharkl3-dpu"; +reg = <0x6300 0x1000>; +interrupts = ; +clock-names = "clk_src_128m", "clk_src_384m"; + +clocks = <&pll CLK_TWPLL_128M>, + <&pll CLK_TWPLL_384M>; + +dpu_port: port { +dpu_out: endpoint { +remote-endpoint = <&dsi_in>; +}; +}; +}; -- 2.29.0
[PATCH v8 2/6] drm/sprd: add Unisoc's drm kms master
Adds drm support for the Unisoc's display subsystem. This is drm kms driver, this driver provides support for the application framework in Android, Yocto and more. Application framework can access Unisoc's display internal peripherals through libdrm or libkms, it's test ok by modetest (DRM/KMS test tool) and Android HWComposer. v4: - Move the devm_drm_dev_alloc to master_ops->bind function. - The managed drmm_mode_config_init() it is no longer necessary for drivers to explicitly call drm_mode_config_cleanup, so delete it. v5: - Remove subdir-ccflgas-y for Makefile. - Keep the selects sorted by alphabet for Kconfig. Cc: Orson Zhai Cc: Chunyan Zhang Signed-off-by: Kevin Tang --- drivers/gpu/drm/Kconfig | 2 + drivers/gpu/drm/Makefile| 1 + drivers/gpu/drm/sprd/Kconfig| 11 ++ drivers/gpu/drm/sprd/Makefile | 3 + drivers/gpu/drm/sprd/sprd_drm.c | 203 drivers/gpu/drm/sprd/sprd_drm.h | 16 +++ 6 files changed, 236 insertions(+) create mode 100644 drivers/gpu/drm/sprd/Kconfig create mode 100644 drivers/gpu/drm/sprd/Makefile create mode 100644 drivers/gpu/drm/sprd/sprd_drm.c create mode 100644 drivers/gpu/drm/sprd/sprd_drm.h diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index fb1446170..f50abc84f 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -395,6 +395,8 @@ source "drivers/gpu/drm/xlnx/Kconfig" source "drivers/gpu/drm/gud/Kconfig" +source "drivers/gpu/drm/sprd/Kconfig" + config DRM_HYPERV tristate "DRM Support for Hyper-V synthetic video device" depends on DRM && PCI && MMU && HYPERV diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 1c41156de..e6bab8df7 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -132,3 +132,4 @@ obj-$(CONFIG_DRM_TIDSS) += tidss/ obj-y += xlnx/ obj-y += gud/ obj-$(CONFIG_DRM_HYPERV) += hyperv/ +obj-$(CONFIG_DRM_SPRD) += sprd/ diff --git a/drivers/gpu/drm/sprd/Kconfig b/drivers/gpu/drm/sprd/Kconfig new file mode 100644 index 0..726c3e76d --- /dev/null +++ b/drivers/gpu/drm/sprd/Kconfig @@ -0,0 +1,11 @@ +config DRM_SPRD + tristate "DRM Support for Unisoc SoCs Platform" + depends on ARCH_SPRD || COMPILE_TEST + depends on DRM && OF + select DRM_GEM_CMA_HELPER + select DRM_KMS_CMA_HELPER + select DRM_KMS_HELPER + help + Choose this option if you have a Unisoc chipset. + If M is selected the module will be called sprd_drm. + diff --git a/drivers/gpu/drm/sprd/Makefile b/drivers/gpu/drm/sprd/Makefile new file mode 100644 index 0..9850f00b8 --- /dev/null +++ b/drivers/gpu/drm/sprd/Makefile @@ -0,0 +1,3 @@ +# SPDX-License-Identifier: GPL-2.0 + +obj-y := sprd_drm.o diff --git a/drivers/gpu/drm/sprd/sprd_drm.c b/drivers/gpu/drm/sprd/sprd_drm.c new file mode 100644 index 0..bb87f28f2 --- /dev/null +++ b/drivers/gpu/drm/sprd/sprd_drm.c @@ -0,0 +1,203 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2020 Unisoc Inc. + */ + +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "sprd_drm.h" + +#define DRIVER_NAME"sprd" +#define DRIVER_DESC"Spreadtrum SoCs' DRM Driver" +#define DRIVER_DATE"20200201" +#define DRIVER_MAJOR 1 +#define DRIVER_MINOR 0 + +static const struct drm_mode_config_helper_funcs sprd_drm_mode_config_helper = { + .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm, +}; + +static const struct drm_mode_config_funcs sprd_drm_mode_config_funcs = { + .fb_create = drm_gem_fb_create, + .atomic_check = drm_atomic_helper_check, + .atomic_commit = drm_atomic_helper_commit, +}; + +static void sprd_drm_mode_config_init(struct drm_device *drm) +{ + drm->mode_config.min_width = 0; + drm->mode_config.min_height = 0; + drm->mode_config.max_width = 8192; + drm->mode_config.max_height = 8192; + drm->mode_config.allow_fb_modifiers = true; + + drm->mode_config.funcs = &sprd_drm_mode_config_funcs; + drm->mode_config.helper_private = &sprd_drm_mode_config_helper; +} + +DEFINE_DRM_GEM_CMA_FOPS(sprd_drm_fops); + +static struct drm_driver sprd_drm_drv = { + .driver_features= DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC, + .fops = &sprd_drm_fops, + + /* GEM Operations */ + DRM_GEM_CMA_DRIVER_OPS, + + .name = DRIVER_NAME, + .desc = DRIVER_DESC, + .date = DRIVER_DATE, + .major = DRIVER_MAJOR, + .minor = DRIVER_MINOR, +}; + +static int sprd_drm_bind(struct device *dev) +{ + struct platform_device *pdev = to_platform_device(dev); + struct drm_device *drm; + struct sprd_drm *sprd; + int ret; + + sprd = devm_drm
[PATCH v8 1/6] dt-bindings: display: add Unisoc's drm master bindings
From: Kevin Tang The Unisoc DRM master device is a virtual device needed to list all DPU devices or other display interface nodes that comprise the graphics subsystem Unisoc's display pipeline have several components as below description, multi display controllers and corresponding physical interfaces. For different display scenarios, dpu0 and dpu1 maybe binding to different encoder. E.g: dpu0 and dpu1 both binding to DSI for dual mipi-dsi display; dpu0 binding to DSI for primary display, and dpu1 binding to DP for external display; Cc: Orson Zhai Cc: Chunyan Zhang Signed-off-by: Kevin Tang Reviewed-by: Rob Herring --- .../display/sprd/sprd,display-subsystem.yaml | 64 +++ 1 file changed, 64 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml diff --git a/Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml b/Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml new file mode 100644 index 0..3d107e943 --- /dev/null +++ b/Documentation/devicetree/bindings/display/sprd/sprd,display-subsystem.yaml @@ -0,0 +1,64 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/sprd/sprd,display-subsystem.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Unisoc DRM master device + +maintainers: + - Kevin Tang + +description: | + The Unisoc DRM master device is a virtual device needed to list all + DPU devices or other display interface nodes that comprise the + graphics subsystem. + + Unisoc's display pipeline have several components as below description, + multi display controllers and corresponding physical interfaces. + For different display scenarios, dpu0 and dpu1 maybe binding to different + encoder. + + E.g: + dpu0 and dpu1 both binding to DSI for dual mipi-dsi display; + dpu0 binding to DSI for primary display, and dpu1 binding to DP for external display; + + +-+ + | | + |+-+ | + ++ | +++-+|DPHY/CPHY| | +--+ + |+->+dpu0+--->+MIPI|DSI +--->+Combo+->+Panel0| + |AXI | | +++-++-+ | +--+ + || | ^ | + || | | | + || | +---+ | + || | | | + |APB | | +--+-++---++---+ | +--+ + |+->+dpu1+--->+DisplayPort+--->+PHY+->+Panel1| + || | +++---++---+ | +--+ + ++ | | + +-+ + +properties: + compatible: +const: sprd,display-subsystem + + ports: +$ref: /schemas/types.yaml#/definitions/phandle-array +description: + Should contain a list of phandles pointing to display interface port + of DPU devices. + +required: + - compatible + - ports + +additionalProperties: false + +examples: + - | +display-subsystem { +compatible = "sprd,display-subsystem"; +ports = <&dpu_out>; +}; + -- 2.29.0
[PATCH v8 0/6] Add Unisoc's drm kms module
ChangeList: RFC v1: 1. only upstream modeset and atomic at first commit. 2. remove some unused code; 3. use alpha and blend_mode properties; 3. add yaml support; 4. remove auto-adaptive panel driver; 5. bugfix RFC v2: 1. add sprd crtc and plane module for KMS, preparing for multi crtc&encoder 2. remove gem drivers, use generic CMA handlers 3. remove redundant "module_init", all the sub modules loading by KMS RFC v3: 1. multi crtc&encoder design have problem, so rollback to v1 RFC v4: 1. update to gcc-linaro-7.5.0 2. update to Linux 5.6-rc3 3. remove pm_runtime support 4. add COMPILE_TEST, remove unused kconfig 5. "drm_dev_put" on drm_unbind 6. fix some naming convention issue 7. remove semaphore lock for crtc flip 8. remove static variables RFC v5: 1. optimize encoder and connector code implementation 2. use "platform_get_irq" and "platform_get_resource" 3. drop useless function return type, drop unless debug log 4. custom properties should be separate, so drop it 5. use DRM_XXX replase pr_xxx 6. drop dsi&dphy hal callback ops 7. drop unless callback ops checking 8. add comments for sprd dpu structure RFC v6: 1. Access registers via readl/writel 2. Checking for unsupported KMS properties (format, rotation, blend_mode, etc) on plane_check ops 3. Remove always true checks for dpu core ops RFC v7: 1. Fix DTC unit name warnings 2. Fix the problem of maintainers 3. Call drmm_mode_config_init to mode config init 4. Embed drm_device in sprd_drm and use devm_drm_dev_alloc 5. Replace DRM_XXX with drm_xxx on KMS module, but not suitable for other subsystems 6. Remove plane_update stuff, dpu handles all the HW update in crtc->atomic_flush 7. Dsi&Dphy Code structure adjustment, all move to "sprd/" v0: 1. Remove dpu_core_ops stuff layer for sprd drtc driver, but dpu_layer need to keeping. Because all the HW update in crtc->atomic_flush, we need temporary storage all layers for the dpu pageflip of atomic_flush. 2. Add ports subnode with port@X. v1: 1. Remove dphy and dsi graph binding, merge the dphy driver into the dsi. 2. Add commit messages for Unisoc's virtual nodes. v2: 1. Use drm_xxx to replace all DRM_XXX. 2. Use kzalloc to replace devm_kzalloc for sprd_dsi/sprd_dpu structure init. 3. Remove dpu_core_ops midlayer. v3: 1. Remove dpu_layer midlayer and commit layers by aotmic_update v4: 1. Move the devm_drm_dev_alloc to master_ops->bind function. 2. The managed drmm_mode_config_init() it is no longer necessary for drivers to explicitly call drm_mode_config_cleanup, so delete it. 3. Use drmm_helpers to allocate crtc ,planes and encoder. 4. Move allocate crtc ,planes, encoder to bind funtion. 5. Move rotation enum definitions to crtc layer reg bitfields. v5: 1. Remove subdir-ccflgas-y for Makefile. 2. Keep the selects sorted by alphabet for Kconfig. 3. Fix the checkpatch warnings. 4. Use mode_set_nofb instead of mode_valid callback. 5. Follow the OF-Graph bindings, use of_graph_get_port_by_id instead of of_parse_phandle. 6. Use zpos to represent the layer position. 7. Rebase to last drm misc branch. 8. Remove panel_in port for dsi node. 9. Drop the dsi ip file prefix. 10. Add Signed-off-by for dsi&dphy patch. 11. Use the mode_flags of mipi_dsi_device to setup crtc DPI and EDPI mode. v6: 1. Disable and clear interrupts before register dpu IRQ 2. Init dpi config used by crtc_state->adjusted_mode on mode_set_nofb 3. Remove enable_irq and disable_irq function call. 4. Remove drm_format_info function call. 5. Redesign the way to access the dsi register. 6. Reduce the dsi_context member variables. v7: 1. Fix codeing style issue by checkpatch. 2. Drop the pll registers structure define. 3. Use bridge API instead of drm panel API. 4. Register mipi_dsi_host on probe phase; 5. Remove iommu error interrupt handling function. 6. Remove some unused function. v8: 1. Fix missing signed-off-by. 2. Move component_add to dsi_host.attach callback. Kevin Tang (6): dt-bindings: display: add Unisoc's drm master bindings drm/sprd: add Unisoc's drm kms master dt-bindings: display: add Unisoc's dpu bindings drm/sprd: add Unisoc's drm display controller driver dt-bindings: display: add Unisoc's mipi dsi controller bindings drm/sprd: add Unisoc's drm mipi dsi&dphy driver .../display/sprd/sprd,display-subsystem.yaml | 64 + .../display/sprd/sprd,sharkl3-dpu.yaml| 77 ++ .../display/sprd/sprd,sharkl3-dsi-host.yaml | 88 ++ drivers/gpu/drm/Kconfig |2 + drivers/gpu/drm/Makefile |1 + drivers/gpu/drm/sprd/Kconfig | 13 + drivers/gpu/drm/sprd/Makefile |8 + drivers/gpu/drm/sprd/megacores_pll.c | 305 + drivers/gpu/drm/sprd/sprd_dpu.c | 880 ++ drivers/gpu/drm/sprd/sprd_dpu.h | 109 ++ drivers/gpu/drm/sprd/sprd_drm.c | 205 drivers/gpu/drm/sprd/sprd_drm.h | 19 + drivers/gpu/drm/sprd/sprd_dsi.c | 1
Re: [PATCH v2 08/16] drm/i915: Pass trylock context to callers
On Mon, 29 Nov 2021 at 13:58, Maarten Lankhorst wrote: > > We are moving away from short term pinning, and need a way to evict > objects locked by the current context. Pass the ww context to all > eviction functions, so that they can evict objects that are already > locked by the current ww context. > > On top of that, this slightly improves ww handling because the locked > objects are marked as locked by the correct ww. > > Signed-off-by: Maarten Lankhorst Reviewed-by: Matthew Auld
Re: [Intel-gfx] [PATCH v9 2/8] drm/i915/ttm: add tt shmem backend
On 07/12/2021 13:10, Tvrtko Ursulin wrote: On 18/10/2021 10:10, Matthew Auld wrote: For cached objects we can allocate our pages directly in shmem. This should make it possible(in a later patch) to utilise the existing i915-gem shrinker code for such objects. For now this is still disabled. v2(Thomas): - Add optional try_to_writeback hook for objects. Importantly we need to check if the object is even still shrinkable; in between us dropping the shrinker LRU lock and acquiring the object lock it could for example have been moved. Also we need to differentiate between "lazy" shrinking and the immediate writeback mode. Also later we need to handle objects which don't even have mm.pages, so bundling this into put_pages() would require somehow handling that edge case, hence just letting the ttm backend handle everything in try_to_writeback doesn't seem too bad. v3(Thomas): - Likely a bad idea to touch the object from the unpopulate hook, since it's not possible to hold a reference, without also creating circular dependency, so likely this is too fragile. For now just ensure we at least mark the pages as dirty/accessed when called from the shrinker on WILLNEED objects. - s/try_to_writeback/shrinker_release_pages, since this can do more than just writeback. - Get rid of do_backup boolean and just set the SWAPPED flag prior to calling unpopulate. - Keep shmem_tt as lowest priority for the TTM LRU bo_swapout walk, since these just get skipped anyway. We can try to come up with something better later. v4(Thomas): - s/PCI_DMA/DMA/. Also drop NO_KERNEL_MAPPING and NO_WARN, which apparently doesn't do anything with streaming mappings. - Just pass along the error for ->truncate, and assume nothing. Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Christian König Cc: Oak Zeng Reviewed-by: Thomas Hellström Acked-by: Oak Zeng [snip] -static void try_to_writeback(struct drm_i915_gem_object *obj, - unsigned int flags) +static int try_to_writeback(struct drm_i915_gem_object *obj, unsigned int flags) { + if (obj->ops->shrinker_release_pages) + return obj->ops->shrinker_release_pages(obj, + flags & I915_SHRINK_WRITEBACK); I have a high level question about how does this new vfunc fit with the existing code. Because I notice in the implementation (i915_ttm_shrinker_release_pages) it ends up doing: ... switch (obj->mm.madv) { case I915_MADV_DONTNEED: return i915_ttm_purge(obj); case __I915_MADV_PURGED: return 0; } ... and then... if (should_writeback) __shmem_writeback(obj->base.size, i915_tt->filp->f_mapping); Which on a glance looks like a possible conceptual duplication of the concepts in this very function (try_to_writeback): + switch (obj->mm.madv) { case I915_MADV_DONTNEED: i915_gem_object_truncate(obj); - return; + return 0; case __I915_MADV_PURGED: - return; + return 0; } if (flags & I915_SHRINK_WRITEBACK) i915_gem_object_writeback(obj); So question is this the final design or some futher tidy is possible/planned? It seems ok to me, all things considered. The TTM version needs to check if the object is still shrinkable at the start(plus some other stuff), upon acquiring the object lock. If that succeeds we can do the above madv checks and potentially just call truncate. Otherwise we can proceed with shrinking, but again TTM is special here, and we have to call ttm_bo_validate() underneath(we might not even have mm.pages here). And then if that all works we might be able to also perform the writeback, if needed. So I suppose we could add all that directly in try_to_writeback(), and make it conditional for TTM devices, or I guess we need two separate hooks, one to do some pre-checking and another do the actual swap step. Not sure if that is better or worse though. Background to my question is that I will float a patch which proposes to remove writeback altogether. There are reports from the fields that it causes deadlocks and looking at 2d6692e642e7 ("drm/i915: Start writeback from the shrinker") and its history it seems like it was a known risk. Apart from the code organisation questions, on the practical level - do you need writeback from the TTM backend or while I am proposing to remove it from the "legacy" paths, I can propose removing it from the TTM flow as well? Yeah, if that is somehow busted then we should remove from TTM backend also. Regards, Tvrtko
Re: [PATCH 2/4] drm/bridge: tc358767: Move hardware init to enable callback
On 12/7/21 14:34, Dave Stevenson wrote: Hi, The TC358767/TC358867/TC9595 are all capable of operating either from attached Xtal or from DSI clock lane clock. In case the later is used, all I2C accesses will fail until the DSI clock lane is running and supplying clock to the chip. Move all hardware initialization to enable callback to guarantee the DSI clock lane is running before accessing the hardware. In case Xtal is attached to the chip, this change has no effect. This puzzles me as there seem to be a couple of ambiguities over how it would be used. Firstly the DT binding lists the clock as a required property, but there isn't one present if deriving the clock from the DSI clock lane. That's correct, the clock for the internal PLLs are derived from the DSI clock lane. Does that mean you're creating a dummy clock device? If it's a required property then presumably yes, but it seems a little odd as that reflck does not exist. No. The refclk will become optional, but for that I need some more infrastructure work, i.e. some way to communicate the requirements of the DSI clock lane to the DSI host. Secondly the datasheet states that the DSI Reference Clock Source Division Selection is done by the MODE1 pin, and switches between HSCKBY2 divided by 7 and HSCKBY2 divided by 9. There is a stated assumption that HSCK is either 364MHz or 468MHz, thereby ending up with 26MHz. To do this we have to be running DSI in burst mode, but the support for that in DRM is near zero. Yes, you have to run the DSI clock lane at either of the two clock frequencies, that is rather limiting. The DSI has to be running in continuous clock mode, this has nothing to do with burst mode, the burst mode is a DSI host setting which is supported by most DSI hosts. OK burst mode is technically dropping the lane to LP mode, and you don't need that state transition. To quote the Toshiba datasheet: "As the clock derived from the DSICLK has to be fixed at 13, 26, 19.2 or 38.4 MHz, the DSI Host may need to run DSI clock lane at higher frequency than needed by frame rate and may have to send the DSI video mode transmissions in burst mode (explained in DSI section of this specification) " So where are you configuring the DSI clock lane to be running at one of those frequencies? Or are you requiring your panel to be running with a matching pixel clock? This is a preparatory patch, so nowhere yet. I forced the clock lane to the required clock frequency on the DSI host side thus far. You can still configure the chip to derive clock from Xtal, even with DSI as data input. Can I ask how the chip has actually been configured and run in this mode? REFCLK Xtal disconnected and HSCKBY2/7 MODE0=H MODE1=L , that should be all you need. Do you plan to develop a board with this bridge ? Yes, I have a board with this chip in DSI to DPI mode that I'm trying to get to work. The intent was to configure it via DSI LP commands rather than I2C, but currently it's refusing to do anything. I see.
Re: [PATCH 2/4] drm/bridge: tc358767: Move hardware init to enable callback
On Mon, 6 Dec 2021 at 20:24, Marek Vasut wrote: > > On 12/6/21 19:01, Dave Stevenson wrote: > > Hi Marek > > Hi, > > >> The TC358767/TC358867/TC9595 are all capable of operating either from > >> attached Xtal or from DSI clock lane clock. In case the later is used, > >> all I2C accesses will fail until the DSI clock lane is running and > >> supplying clock to the chip. > >> > >> Move all hardware initialization to enable callback to guarantee the > >> DSI clock lane is running before accessing the hardware. In case Xtal > >> is attached to the chip, this change has no effect. > > > > This puzzles me as there seem to be a couple of ambiguities over how > > it would be used. > > > > Firstly the DT binding lists the clock as a required property, but > > there isn't one present if deriving the clock from the DSI clock lane. > > That's correct, the clock for the internal PLLs are derived from the DSI > clock lane. Does that mean you're creating a dummy clock device? If it's a required property then presumably yes, but it seems a little odd as that reflck does not exist. > > Secondly the datasheet states that the DSI Reference Clock Source > > Division Selection is done by the MODE1 pin, and switches between > > HSCKBY2 divided by 7 and HSCKBY2 divided by 9. There is a stated > > assumption that HSCK is either 364MHz or 468MHz, thereby ending up > > with 26MHz. To do this we have to be running DSI in burst mode, but > > the support for that in DRM is near zero. > > Yes, you have to run the DSI clock lane at either of the two clock > frequencies, that is rather limiting. The DSI has to be running in > continuous clock mode, this has nothing to do with burst mode, the burst > mode is a DSI host setting which is supported by most DSI hosts. OK burst mode is technically dropping the lane to LP mode, and you don't need that state transition. To quote the Toshiba datasheet: "As the clock derived from the DSICLK has to be fixed at 13, 26, 19.2 or 38.4 MHz, the DSI Host may need to run DSI clock lane at higher frequency than needed by frame rate and may have to send the DSI video mode transmissions in burst mode (explained in DSI section of this specification) " So where are you configuring the DSI clock lane to be running at one of those frequencies? Or are you requiring your panel to be running with a matching pixel clock? > > Can I ask how the chip has actually been configured and run in this mode? > > REFCLK Xtal disconnected and HSCKBY2/7 MODE0=H MODE1=L , that should be > all you need. Do you plan to develop a board with this bridge ? Yes, I have a board with this chip in DSI to DPI mode that I'm trying to get to work. The intent was to configure it via DSI LP commands rather than I2C, but currently it's refusing to do anything. Dave
Re: [Intel-gfx] [PATCH v9 2/8] drm/i915/ttm: add tt shmem backend
On 18/10/2021 10:10, Matthew Auld wrote: For cached objects we can allocate our pages directly in shmem. This should make it possible(in a later patch) to utilise the existing i915-gem shrinker code for such objects. For now this is still disabled. v2(Thomas): - Add optional try_to_writeback hook for objects. Importantly we need to check if the object is even still shrinkable; in between us dropping the shrinker LRU lock and acquiring the object lock it could for example have been moved. Also we need to differentiate between "lazy" shrinking and the immediate writeback mode. Also later we need to handle objects which don't even have mm.pages, so bundling this into put_pages() would require somehow handling that edge case, hence just letting the ttm backend handle everything in try_to_writeback doesn't seem too bad. v3(Thomas): - Likely a bad idea to touch the object from the unpopulate hook, since it's not possible to hold a reference, without also creating circular dependency, so likely this is too fragile. For now just ensure we at least mark the pages as dirty/accessed when called from the shrinker on WILLNEED objects. - s/try_to_writeback/shrinker_release_pages, since this can do more than just writeback. - Get rid of do_backup boolean and just set the SWAPPED flag prior to calling unpopulate. - Keep shmem_tt as lowest priority for the TTM LRU bo_swapout walk, since these just get skipped anyway. We can try to come up with something better later. v4(Thomas): - s/PCI_DMA/DMA/. Also drop NO_KERNEL_MAPPING and NO_WARN, which apparently doesn't do anything with streaming mappings. - Just pass along the error for ->truncate, and assume nothing. Signed-off-by: Matthew Auld Cc: Thomas Hellström Cc: Christian König Cc: Oak Zeng Reviewed-by: Thomas Hellström Acked-by: Oak Zeng [snip] -static void try_to_writeback(struct drm_i915_gem_object *obj, -unsigned int flags) +static int try_to_writeback(struct drm_i915_gem_object *obj, unsigned int flags) { + if (obj->ops->shrinker_release_pages) + return obj->ops->shrinker_release_pages(obj, + flags & I915_SHRINK_WRITEBACK); I have a high level question about how does this new vfunc fit with the existing code. Because I notice in the implementation (i915_ttm_shrinker_release_pages) it ends up doing: ... switch (obj->mm.madv) { case I915_MADV_DONTNEED: return i915_ttm_purge(obj); case __I915_MADV_PURGED: return 0; } ... and then... if (should_writeback) __shmem_writeback(obj->base.size, i915_tt->filp->f_mapping); Which on a glance looks like a possible conceptual duplication of the concepts in this very function (try_to_writeback): + switch (obj->mm.madv) { case I915_MADV_DONTNEED: i915_gem_object_truncate(obj); - return; + return 0; case __I915_MADV_PURGED: - return; + return 0; } if (flags & I915_SHRINK_WRITEBACK) i915_gem_object_writeback(obj); So question is this the final design or some futher tidy is possible/planned? Background to my question is that I will float a patch which proposes to remove writeback altogether. There are reports from the fields that it causes deadlocks and looking at 2d6692e642e7 ("drm/i915: Start writeback from the shrinker") and its history it seems like it was a known risk. Apart from the code organisation questions, on the practical level - do you need writeback from the TTM backend or while I am proposing to remove it from the "legacy" paths, I can propose removing it from the TTM flow as well? Regards, Tvrtko
Re: [PATCH] drm/rockchip: use generic fbdev setup
Hi Am 07.12.21 um 12:54 schrieb John Keeping: Are you able to pick this up (and [1])? Otherwise what is needed here and who should pick this up? [1] https://lore.kernel.org/dri-devel/20211101114622.813536-1-j...@metanate.com/ I added both patches to drm-misc-next. Best regards Thomas -- 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
[PATCH 24/24] drm/ttm: remove bo->moving
This is now handled by the DMA-buf framework in the dma_resv obj. Signed-off-by: Christian König --- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 13 --- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 7 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c| 11 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c | 11 -- drivers/gpu/drm/ttm/ttm_bo.c | 10 ++ drivers/gpu/drm/ttm/ttm_bo_util.c | 7 drivers/gpu/drm/ttm/ttm_bo_vm.c | 34 +++ drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 6 include/drm/ttm/ttm_bo_api.h | 2 -- 9 files changed, 40 insertions(+), 61 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index bbfd7a1e42e8..7bd39e5d36dd 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -2330,6 +2330,8 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef) struct amdgpu_bo *bo = mem->bo; uint32_t domain = mem->domain; struct kfd_mem_attachment *attachment; + struct dma_resv_iter cursor; + struct dma_fence *fence; total_size += amdgpu_bo_size(bo); @@ -2344,10 +2346,13 @@ int amdgpu_amdkfd_gpuvm_restore_process_bos(void *info, struct dma_fence **ef) goto validate_map_fail; } } - ret = amdgpu_sync_fence(&sync_obj, bo->tbo.moving); - if (ret) { - pr_debug("Memory eviction: Sync BO fence failed. Try again\n"); - goto validate_map_fail; + dma_resv_for_each_fence(&cursor, bo->tbo.base.resv, + DMA_RESV_USAGE_KERNEL, fence) { + ret = amdgpu_sync_fence(&sync_obj, fence); + if (ret) { + pr_debug("Memory eviction: Sync BO fence failed. Try again\n"); + goto validate_map_fail; + } } list_for_each_entry(attachment, &mem->attachments, list) { if (!attachment->is_mapped) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index a40ede9bccd0..3881a503a7bf 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -608,9 +608,8 @@ int amdgpu_bo_create(struct amdgpu_device *adev, if (unlikely(r)) goto fail_unreserve; - amdgpu_bo_fence(bo, fence, false); - dma_fence_put(bo->tbo.moving); - bo->tbo.moving = dma_fence_get(fence); + dma_resv_add_fence(bo->tbo.base.resv, fence, + DMA_RESV_USAGE_KERNEL); dma_fence_put(fence); } if (!bp->resv) @@ -1290,7 +1289,7 @@ void amdgpu_bo_release_notify(struct ttm_buffer_object *bo) r = amdgpu_fill_buffer(abo, AMDGPU_POISON, bo->base.resv, &fence); if (!WARN_ON(r)) { - amdgpu_bo_fence(abo, fence, false); + dma_resv_add_fence(bo->base.resv, fence, DMA_RESV_USAGE_KERNEL); dma_fence_put(fence); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c index e3fbf0f10add..31913ae86de6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_cpu.c @@ -74,13 +74,12 @@ static int amdgpu_vm_cpu_update(struct amdgpu_vm_update_params *p, { unsigned int i; uint64_t value; - int r; + long r; - if (vmbo->bo.tbo.moving) { - r = dma_fence_wait(vmbo->bo.tbo.moving, true); - if (r) - return r; - } + r = dma_resv_wait_timeout(vmbo->bo.tbo.base.resv, DMA_RESV_USAGE_KERNEL, + true, MAX_SCHEDULE_TIMEOUT); + if (r < 0) + return r; pe += (unsigned long)amdgpu_bo_kptr(&vmbo->bo); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c index dbb551762805..bdb44cee19d3 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_sdma.c @@ -204,14 +204,19 @@ static int amdgpu_vm_sdma_update(struct amdgpu_vm_update_params *p, struct amdgpu_bo *bo = &vmbo->bo; enum amdgpu_ib_pool_type pool = p->immediate ? AMDGPU_IB_POOL_IMMEDIATE : AMDGPU_IB_POOL_DELAYED; + struct dma_resv_iter cursor; unsigned int i, ndw, nptes; + struct dma_fence *fence; uint64_t *pte; int r; /* Wait for PD/PT moves to be completed */ - r = amdgpu_sync_f
[PATCH 21/24] dma-buf: add DMA_RESV_USAGE_BOOKKEEP
Add an usage for submissions independent of implicit sync but still interesting for memory management. Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c | 2 +- drivers/dma-buf/st-dma-resv.c | 2 +- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c| 4 ++-- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 6 +++--- drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_userptr.c| 2 +- drivers/gpu/drm/qxl/qxl_debugfs.c | 2 +- drivers/gpu/drm/radeon/radeon_gem.c| 2 +- drivers/gpu/drm/radeon/radeon_mn.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 14 +++--- include/linux/dma-resv.h | 18 +- 15 files changed, 40 insertions(+), 24 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index a2a0b5b6c107..a058a3e805ab 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -548,7 +548,7 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src) list = NULL; - dma_resv_iter_begin(&cursor, src, DMA_RESV_USAGE_READ); + dma_resv_iter_begin(&cursor, src, DMA_RESV_USAGE_BOOKKEEP); dma_resv_for_each_fence_unlocked(&cursor, f) { if (dma_resv_iter_is_restarted(&cursor)) { diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c index 062b57d63fa6..8ace9e84c845 100644 --- a/drivers/dma-buf/st-dma-resv.c +++ b/drivers/dma-buf/st-dma-resv.c @@ -296,7 +296,7 @@ int dma_resv(void) int r; spin_lock_init(&fence_lock); - for (usage = DMA_RESV_USAGE_KERNEL; usage <= DMA_RESV_USAGE_READ; + for (usage = DMA_RESV_USAGE_KERNEL; usage <= DMA_RESV_USAGE_BOOKKEEP; ++usage) { r = subtests(tests, (void *)(unsigned long)usage); if (r) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 4a469831afe3..bbfd7a1e42e8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -246,7 +246,7 @@ static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, */ replacement = dma_fence_get_stub(); dma_resv_replace_fences(bo->tbo.base.resv, ef->base.context, - replacement, DMA_RESV_USAGE_READ); + replacement, DMA_RESV_USAGE_BOOKKEEP); dma_fence_put(replacement); return 0; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c index 490d2a7a3e2b..ddf46802b1ff 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c @@ -111,7 +111,7 @@ void amdgpu_pasid_free_delayed(struct dma_resv *resv, struct dma_fence *fence; int r; - r = dma_resv_get_singleton(resv, DMA_RESV_USAGE_READ, &fence); + r = dma_resv_get_singleton(resv, DMA_RESV_USAGE_BOOKKEEP, &fence); if (r) goto fallback; @@ -139,7 +139,7 @@ void amdgpu_pasid_free_delayed(struct dma_resv *resv, /* Not enough memory for the delayed delete, as last resort * block for all the fences to complete. */ - dma_resv_wait_timeout(resv, DMA_RESV_USAGE_READ, + dma_resv_wait_timeout(resv, DMA_RESV_USAGE_BOOKKEEP, false, MAX_SCHEDULE_TIMEOUT); amdgpu_pasid_free(pasid); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c index 86f5248676b0..b86c0b8252a5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c @@ -75,7 +75,7 @@ static bool amdgpu_mn_invalidate_gfx(struct mmu_interval_notifier *mni, mmu_interval_set_seq(mni, cur_seq); - r = dma_resv_wait_timeout(bo->tbo.base.resv, DMA_RESV_USAGE_READ, + r = dma_resv_wait_timeout(bo->tbo.base.resv, DMA_RESV_USAGE_BOOKKEEP, false, MAX_SCHEDULE_TIMEOUT); mutex_unlock(&adev->notifier_lock); if (r <= 0) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c index 183623806056..1447f009a957 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c @@ -260,7 +260,7 @@ int amdgpu_sync_resv(struct amdgpu_device *adev, struct amdgpu_sync *sync, return -EINVAL; /* TODO: Use DMA_RESV_USAGE_READ here */ - dma_resv_for_each_fence(&cursor, resv, DMA_RESV_USAGE_READ, f) { + dma_resv_for_each_fence(&cursor, resv, DMA_RESV_USAGE_BOOKKEEP, f) {
[PATCH 22/24] dma-buf: wait for map to complete for static attachments
We have previously done that in the individual drivers but it is more defensive to move that into the common code. Dynamic attachments should wait for map operations to complete by themselves. Signed-off-by: Christian König --- drivers/dma-buf/dma-buf.c | 18 +++--- drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 14 +- drivers/gpu/drm/nouveau/nouveau_prime.c | 17 + drivers/gpu/drm/radeon/radeon_prime.c | 16 +++- 4 files changed, 20 insertions(+), 45 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 528983d3ba64..d3dd602c4753 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -660,12 +660,24 @@ static struct sg_table * __map_dma_buf(struct dma_buf_attachment *attach, enum dma_data_direction direction) { struct sg_table *sg_table; + signed long ret; sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction); + if (IS_ERR_OR_NULL(sg_table)) + return sg_table; + + if (!dma_buf_attachment_is_dynamic(attach)) { + ret = dma_resv_wait_timeout(attach->dmabuf->resv, + DMA_RESV_USAGE_KERNEL, true, + MAX_SCHEDULE_TIMEOUT); + if (ret < 0) { + attach->dmabuf->ops->unmap_dma_buf(attach, sg_table, + direction); + return ERR_PTR(ret); + } + } - if (!IS_ERR_OR_NULL(sg_table)) - mangle_sg_table(sg_table); - + mangle_sg_table(sg_table); return sg_table; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c index 4896c876ffec..33127bd56c64 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c @@ -102,21 +102,9 @@ static int amdgpu_dma_buf_pin(struct dma_buf_attachment *attach) { struct drm_gem_object *obj = attach->dmabuf->priv; struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj); - int r; /* pin buffer into GTT */ - r = amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT); - if (r) - return r; - - if (bo->tbo.moving) { - r = dma_fence_wait(bo->tbo.moving, true); - if (r) { - amdgpu_bo_unpin(bo); - return r; - } - } - return 0; + return amdgpu_bo_pin(bo, AMDGPU_GEM_DOMAIN_GTT); } /** diff --git a/drivers/gpu/drm/nouveau/nouveau_prime.c b/drivers/gpu/drm/nouveau/nouveau_prime.c index 60019d0532fc..347488685f74 100644 --- a/drivers/gpu/drm/nouveau/nouveau_prime.c +++ b/drivers/gpu/drm/nouveau/nouveau_prime.c @@ -93,22 +93,7 @@ int nouveau_gem_prime_pin(struct drm_gem_object *obj) if (ret) return -EINVAL; - ret = ttm_bo_reserve(&nvbo->bo, false, false, NULL); - if (ret) - goto error; - - if (nvbo->bo.moving) - ret = dma_fence_wait(nvbo->bo.moving, true); - - ttm_bo_unreserve(&nvbo->bo); - if (ret) - goto error; - - return ret; - -error: - nouveau_bo_unpin(nvbo); - return ret; + return 0; } void nouveau_gem_prime_unpin(struct drm_gem_object *obj) diff --git a/drivers/gpu/drm/radeon/radeon_prime.c b/drivers/gpu/drm/radeon/radeon_prime.c index 4a90807351e7..42a87948e28c 100644 --- a/drivers/gpu/drm/radeon/radeon_prime.c +++ b/drivers/gpu/drm/radeon/radeon_prime.c @@ -77,19 +77,9 @@ int radeon_gem_prime_pin(struct drm_gem_object *obj) /* pin buffer into GTT */ ret = radeon_bo_pin(bo, RADEON_GEM_DOMAIN_GTT, NULL); - if (unlikely(ret)) - goto error; - - if (bo->tbo.moving) { - ret = dma_fence_wait(bo->tbo.moving, false); - if (unlikely(ret)) { - radeon_bo_unpin(bo); - goto error; - } - } - - bo->prime_shared_count++; -error: + if (likely(ret == 0)) + bo->prime_shared_count++; + radeon_bo_unreserve(bo); return ret; } -- 2.25.1
[PATCH 17/24] drm/amdgpu: use dma_resv_get_singleton in amdgpu_pasid_free_cb
Makes the code a bit more simpler. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 23 +++ 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c index be48487e2ca7..888d97143177 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c @@ -107,36 +107,19 @@ static void amdgpu_pasid_free_cb(struct dma_fence *fence, void amdgpu_pasid_free_delayed(struct dma_resv *resv, u32 pasid) { - struct dma_fence *fence, **fences; struct amdgpu_pasid_cb *cb; - unsigned count; + struct dma_fence *fence; int r; - r = dma_resv_get_fences(resv, true, &count, &fences); + r = dma_resv_get_singleton(resv, true, &fence); if (r) goto fallback; - if (count == 0) { + if (!fence) { amdgpu_pasid_free(pasid); return; } - if (count == 1) { - fence = fences[0]; - kfree(fences); - } else { - uint64_t context = dma_fence_context_alloc(1); - struct dma_fence_array *array; - - array = dma_fence_array_create(count, fences, context, - 1, false); - if (!array) { - kfree(fences); - goto fallback; - } - fence = &array->base; - } - cb = kmalloc(sizeof(*cb), GFP_KERNEL); if (!cb) { /* Last resort when we are OOM */ -- 2.25.1
[PATCH 20/24] dma-buf: add DMA_RESV_USAGE_KERNEL
Add an usage for kernel submissions. Waiting for those are mandatory for dynamic DMA-bufs. Signed-off-by: Christian König --- drivers/dma-buf/st-dma-resv.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 6 -- drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 2 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 4 ++-- drivers/gpu/drm/radeon/radeon_uvd.c | 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 2 +- drivers/gpu/drm/ttm/ttm_bo_util.c| 4 ++-- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 2 +- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 2 +- drivers/infiniband/core/umem_dmabuf.c| 2 +- include/linux/dma-resv.h | 22 13 files changed, 39 insertions(+), 15 deletions(-) diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c index d0f7c2bfd4f0..062b57d63fa6 100644 --- a/drivers/dma-buf/st-dma-resv.c +++ b/drivers/dma-buf/st-dma-resv.c @@ -296,7 +296,7 @@ int dma_resv(void) int r; spin_lock_init(&fence_lock); - for (usage = DMA_RESV_USAGE_WRITE; usage <= DMA_RESV_USAGE_READ; + for (usage = DMA_RESV_USAGE_KERNEL; usage <= DMA_RESV_USAGE_READ; ++usage) { r = subtests(tests, (void *)(unsigned long)usage); if (r) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index eaa19154551c..a40ede9bccd0 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c @@ -764,7 +764,7 @@ int amdgpu_bo_kmap(struct amdgpu_bo *bo, void **ptr) return 0; } - r = dma_resv_wait_timeout(bo->tbo.base.resv, DMA_RESV_USAGE_WRITE, + r = dma_resv_wait_timeout(bo->tbo.base.resv, DMA_RESV_USAGE_KERNEL, false, MAX_SCHEDULE_TIMEOUT); if (r < 0) return r; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c index 33deb0df62fd..9e102080dad9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c @@ -1163,7 +1163,7 @@ static int amdgpu_uvd_send_msg(struct amdgpu_ring *ring, struct amdgpu_bo *bo, if (direct) { r = dma_resv_wait_timeout(bo->tbo.base.resv, - DMA_RESV_USAGE_WRITE, false, + DMA_RESV_USAGE_KERNEL, false, msecs_to_jiffies(10)); if (r == 0) r = -ETIMEDOUT; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 2d77e469ef3c..a2f627af3ce2 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -185,9 +185,11 @@ static int submit_fence_sync(struct etnaviv_gem_submit *submit) return ret; if (submit->flags & ETNA_SUBMIT_NO_IMPLICIT) - continue; + usage = DMA_RESV_USAGE_KERNEL; + else + usage = dma_resv_usage_rw(bo->flags & + ETNA_SUBMIT_BO_WRITE); - usage = dma_resv_usage_rw(bo->flags & ETNA_SUBMIT_BO_WRITE); ret = dma_resv_get_fences(robj, usage, &bo->nr_shared, &bo->shared); if (ret) diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c index e70fb65bb54f..b9281ca96ece 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c @@ -109,7 +109,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, i915_fence_timeout(to_i915(obj->base.dev)), I915_FENCE_GFP); dma_resv_add_fence(obj->base.resv, &clflush->base.dma, - DMA_RESV_USAGE_WRITE); + DMA_RESV_USAGE_KERNEL); dma_fence_work_commit(&clflush->base); } else if (obj->mm.pages) { __do_clflush(obj); diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index 05076e530e7d..13deb6c70ba6 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -962,10 +962,10 @@ nouveau_bo_vm_cleanup(struct ttm_buffer_object *bo, struct dma_fence *fence; int ret; - ret = dma_resv_get_singleton(bo->base.resv, DMA_RESV_USAGE_WRITE, + ret = dma_resv_get_singleton(bo->base.resv, DMA_RESV_USAGE_KERNEL, &fence);
[PATCH 23/24] amdgpu: remove DMA-buf fence workaround
Not needed any more now we have that inside the framework. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 52 +++-- 2 files changed, 6 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h index 044b41f0bfd9..529d52a204cf 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.h @@ -34,7 +34,6 @@ struct amdgpu_fpriv; struct amdgpu_bo_list_entry { struct ttm_validate_buffer tv; struct amdgpu_bo_va *bo_va; - struct dma_fence_chain *chain; uint32_tpriority; struct page **user_pages; booluser_invalidated; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 92091e800022..413606d10080 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -576,14 +576,6 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, struct amdgpu_bo *bo = ttm_to_amdgpu_bo(e->tv.bo); e->bo_va = amdgpu_vm_bo_find(vm, bo); - - if (bo->tbo.base.dma_buf && !amdgpu_bo_explicit_sync(bo)) { - e->chain = dma_fence_chain_alloc(); - if (!e->chain) { - r = -ENOMEM; - goto error_validate; - } - } } amdgpu_cs_get_threshold_for_moves(p->adev, &p->bytes_moved_threshold, @@ -634,13 +626,8 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p, } error_validate: - if (r) { - amdgpu_bo_list_for_each_entry(e, p->bo_list) { - dma_fence_chain_free(e->chain); - e->chain = NULL; - } + if (r) ttm_eu_backoff_reservation(&p->ticket, &p->validated); - } out: return r; } @@ -680,17 +667,9 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error, { unsigned i; - if (error && backoff) { - struct amdgpu_bo_list_entry *e; - - amdgpu_bo_list_for_each_entry(e, parser->bo_list) { - dma_fence_chain_free(e->chain); - e->chain = NULL; - } - + if (error && backoff) ttm_eu_backoff_reservation(&parser->ticket, &parser->validated); - } for (i = 0; i < parser->num_post_deps; i++) { drm_syncobj_put(parser->post_deps[i].syncobj); @@ -1265,29 +1244,10 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, amdgpu_vm_move_to_lru_tail(p->adev, &fpriv->vm); - amdgpu_bo_list_for_each_entry(e, p->bo_list) { - struct dma_resv *resv = e->tv.bo->base.resv; - struct dma_fence_chain *chain = e->chain; - struct dma_resv_iter cursor; - struct dma_fence *fence; - - if (!chain) - continue; - - /* -* Work around dma_resv shortcommings by wrapping up the -* submission in a dma_fence_chain and add it as exclusive -* fence. -*/ - dma_resv_for_each_fence(&cursor, resv, - DMA_RESV_USAGE_WRITE, - fence) { - break; - } - dma_fence_chain_init(chain, fence, dma_fence_get(p->fence), 1); - dma_resv_add_fence(resv, &chain->base, DMA_RESV_USAGE_WRITE); - e->chain = NULL; - } + /* For now manually add the resulting fence as writer as well */ + amdgpu_bo_list_for_each_entry(e, p->bo_list) + dma_resv_add_fence(e->tv.bo->base.resv, p->fence, + DMA_RESV_USAGE_WRITE); ttm_eu_fence_buffer_objects(&p->ticket, &p->validated, p->fence); mutex_unlock(&p->adev->notifier_lock); -- 2.25.1
[PATCH 18/24] dma-buf: add enum dma_resv_usage v3
This change adds the dma_resv_usage enum and allows us to specify why a dma_resv object is queried for its containing fences. Additional to that a dma_resv_usage_rw() helper function is added to aid retrieving the fences for a read or write userspace submission. This is then deployed to the different query functions of the dma_resv object and all of their users. When the write paratermer was previously true we now use DMA_RESV_USAGE_WRITE and DMA_RESV_USAGE_READ otherwise. v2: add KERNEL/OTHER in separate patch v3: some kerneldoc suggestions by Daniel Signed-off-by: Christian König --- drivers/dma-buf/dma-buf.c | 3 +- drivers/dma-buf/dma-resv.c| 33 + drivers/dma-buf/st-dma-resv.c | 48 ++--- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 5 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 4 +- drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 3 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 7 +- .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 3 +- drivers/gpu/drm/drm_gem.c | 6 +- drivers/gpu/drm/drm_gem_atomic_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem.c | 6 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 7 +- .../gpu/drm/i915/display/intel_atomic_plane.c | 3 +- drivers/gpu/drm/i915/gem/i915_gem_busy.c | 4 +- drivers/gpu/drm/i915/gem/i915_gem_lmem.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_userptr.c | 2 +- drivers/gpu/drm/i915/gem/i915_gem_wait.c | 6 +- .../drm/i915/gem/selftests/i915_gem_dmabuf.c | 3 +- drivers/gpu/drm/i915/i915_request.c | 3 +- drivers/gpu/drm/i915/i915_sw_fence.c | 2 +- drivers/gpu/drm/msm/msm_gem.c | 3 +- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 3 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 8 +-- drivers/gpu/drm/nouveau/nouveau_fence.c | 3 +- drivers/gpu/drm/nouveau/nouveau_gem.c | 3 +- drivers/gpu/drm/panfrost/panfrost_drv.c | 3 +- drivers/gpu/drm/qxl/qxl_debugfs.c | 3 +- drivers/gpu/drm/radeon/radeon_display.c | 3 +- drivers/gpu/drm/radeon/radeon_gem.c | 9 ++- drivers/gpu/drm/radeon/radeon_mn.c| 4 +- drivers/gpu/drm/radeon/radeon_sync.c | 2 +- drivers/gpu/drm/radeon/radeon_uvd.c | 4 +- drivers/gpu/drm/scheduler/sched_main.c| 3 +- drivers/gpu/drm/ttm/ttm_bo.c | 18 ++--- drivers/gpu/drm/vgem/vgem_fence.c | 4 +- drivers/gpu/drm/virtio/virtgpu_ioctl.c| 5 +- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c| 4 +- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 4 +- drivers/infiniband/core/umem_dmabuf.c | 3 +- include/linux/dma-resv.h | 69 +++ 46 files changed, 205 insertions(+), 125 deletions(-) diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c index 602b12d7470d..528983d3ba64 100644 --- a/drivers/dma-buf/dma-buf.c +++ b/drivers/dma-buf/dma-buf.c @@ -1124,7 +1124,8 @@ static int __dma_buf_begin_cpu_access(struct dma_buf *dmabuf, long ret; /* Wait on any implicit rendering fences */ - ret = dma_resv_wait_timeout(resv, write, true, MAX_SCHEDULE_TIMEOUT); + ret = dma_resv_wait_timeout(resv, dma_resv_usage_rw(write), + true, MAX_SCHEDULE_TIMEOUT); if (ret < 0) return ret; diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index ecb2ff606bac..33a17db89fb4 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -408,7 +408,7 @@ static void dma_resv_iter_restart_unlocked(struct dma_resv_iter *cursor) cursor->seq = read_seqcount_begin(&cursor->obj->seq); cursor->index = -1; cursor->shared_count = 0; - if (cursor->all_fences) { + if (cursor->usage >= DMA_RESV_USAGE_READ) { cursor->fences = dma_resv_shared_list(cursor->obj); if (cursor->fences) cursor->shared_count = cursor->fences->shared_count; @@ -515,7 +515,7 @@ struct dma_fence *dma_resv_iter_first(struct dma_resv_iter *cursor) dma_resv_assert_held(cursor->obj); cursor->index = 0; - if (cursor->all_fences) + if (cursor->usage >= DMA_RESV_USAGE_READ) cursor->fences = dma_resv_shared_list(cursor->obj); else cursor->fences = NULL; @@ -570,7 +570,7 @@ int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src) list = NULL; excl = NULL; - dma_resv_iter_begin(
[PATCH 19/24] dma-buf: specify usage while adding fences to dma_resv obj v2
Instead of distingting between shared and exclusive fences specify the fence usage while adding fences. Rework all drivers to use this interface instead and deprecate the old one. v2: some kerneldoc comments suggested by Daniel Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c| 374 -- drivers/dma-buf/st-dma-resv.c | 109 ++--- .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c| 6 +- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 8 +- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 4 +- drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 12 +- drivers/gpu/drm/i915/gem/i915_gem_busy.c | 13 +- drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 5 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 4 +- .../drm/i915/gem/selftests/i915_gem_migrate.c | 2 +- drivers/gpu/drm/i915/i915_vma.c | 10 +- .../drm/i915/selftests/intel_memory_region.c | 2 +- drivers/gpu/drm/lima/lima_gem.c | 4 +- drivers/gpu/drm/msm/msm_gem_submit.c | 4 +- drivers/gpu/drm/nouveau/nouveau_bo.c | 9 +- drivers/gpu/drm/nouveau/nouveau_fence.c | 2 +- drivers/gpu/drm/panfrost/panfrost_job.c | 2 +- drivers/gpu/drm/qxl/qxl_release.c | 5 +- drivers/gpu/drm/radeon/radeon_object.c| 6 +- drivers/gpu/drm/radeon/radeon_vm.c| 2 +- drivers/gpu/drm/ttm/ttm_bo.c | 6 +- drivers/gpu/drm/ttm/ttm_bo_util.c | 7 +- drivers/gpu/drm/ttm/ttm_execbuf_util.c| 10 +- drivers/gpu/drm/v3d/v3d_gem.c | 6 +- drivers/gpu/drm/vc4/vc4_gem.c | 4 +- drivers/gpu/drm/vgem/vgem_fence.c | 11 +- drivers/gpu/drm/virtio/virtgpu_gem.c | 5 +- drivers/gpu/drm/vmwgfx/vmwgfx_bo.c| 5 +- include/linux/dma-resv.h | 88 ++--- 31 files changed, 307 insertions(+), 426 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 33a17db89fb4..a2a0b5b6c107 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -44,12 +44,12 @@ /** * DOC: Reservation Object Overview * - * The reservation object provides a mechanism to manage shared and - * exclusive fences associated with a buffer. A reservation object - * can have attached one exclusive fence (normally associated with - * write operations) or N shared fences (read operations). The RCU - * mechanism is used to protect read access to fences from locked - * write-side updates. + * The reservation object provides a mechanism to manage a container of + * dma_fence object associated with a resource. A reservation object + * can have any number of fences attaches to it. Each fence carring an usage + * parameter determining how the operation represented by the fence is using the + * resource. The RCU mechanism is used to protect read access to fences from + * locked write-side updates. * * See struct dma_resv for more details. */ @@ -57,36 +57,80 @@ DEFINE_WD_CLASS(reservation_ww_class); EXPORT_SYMBOL(reservation_ww_class); +/* Mask for the lower fence pointer bits */ +#define DMA_RESV_LIST_MASK 0x3 + /** - * struct dma_resv_list - a list of shared fences + * struct dma_resv_list - an array of fences * @rcu: for internal use - * @shared_count: table of shared fences - * @shared_max: for growing shared fence table - * @shared: shared fence table + * @num_fences: table of fences + * @max_fences: for growing fence table + * @table: fence table */ struct dma_resv_list { struct rcu_head rcu; - u32 shared_count, shared_max; - struct dma_fence __rcu *shared[]; + u32 num_fences, max_fences; + struct dma_fence __rcu *table[]; }; +/** + * dma_resv_list_entry - extract fence and usage from a list entry + * @list: the list to extract and entry from + * @index: which entry we want + * @check: lockdep check that the access is allowed + * @fence: the resulting fence + * @usage: the resulting usage + * + * Extract the fence and usage flags from an RCU protected entry in the list. + */ +static void dma_resv_list_entry(struct dma_resv_list *list, unsigned int index, + bool check, struct dma_fence **fence, + enum dma_resv_usage *usage) +{ + long tmp; + + tmp = (long)rcu_dereference_check(list->table[index], check); + *fence = (struct dma_fence *)(tmp & ~DMA_RESV_LIST_MASK); + if (usage) + *usage = tmp & DMA_RESV_LIST_MASK; +} + +/** + * dma_resv_list_set - set fence and usage at a specific index + * @list: the list to modify + * @index: where to make the change + * @fence: the fence to set + * @usage: the usage to set + * + * Set the fence and usage flags at the specific index in the list. + */ +static void dma_resv
[PATCH 14/24] dma-buf/drivers: make reserving a shared slot mandatory v2
Audit all the users of dma_resv_add_excl_fence() and make sure they reserve a shared slot also when only trying to add an exclusive fence. This is the next step towards handling the exclusive fence like a shared one. v2: fix missed case in amdgpu Signed-off-by: Christian König --- drivers/dma-buf/st-dma-resv.c | 64 +-- drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 8 +++ drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 8 +-- drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 3 +- .../gpu/drm/i915/gem/i915_gem_execbuffer.c| 8 +-- .../drm/i915/gem/selftests/i915_gem_migrate.c | 5 +- drivers/gpu/drm/i915/i915_vma.c | 6 ++ .../drm/i915/selftests/intel_memory_region.c | 7 ++ drivers/gpu/drm/lima/lima_gem.c | 10 ++- drivers/gpu/drm/msm/msm_gem_submit.c | 18 +++--- drivers/gpu/drm/nouveau/nouveau_fence.c | 9 +-- drivers/gpu/drm/panfrost/panfrost_job.c | 4 ++ drivers/gpu/drm/ttm/ttm_bo_util.c | 12 +++- drivers/gpu/drm/ttm/ttm_execbuf_util.c| 11 ++-- drivers/gpu/drm/v3d/v3d_gem.c | 15 +++-- drivers/gpu/drm/vgem/vgem_fence.c | 12 ++-- drivers/gpu/drm/virtio/virtgpu_gem.c | 9 +++ drivers/gpu/drm/vmwgfx/vmwgfx_bo.c| 16 +++-- 18 files changed, 133 insertions(+), 92 deletions(-) diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c index cbe999c6e7a6..f33bafc78693 100644 --- a/drivers/dma-buf/st-dma-resv.c +++ b/drivers/dma-buf/st-dma-resv.c @@ -75,17 +75,16 @@ static int test_signaling(void *arg, bool shared) goto err_free; } - if (shared) { - r = dma_resv_reserve_shared(&resv, 1); - if (r) { - pr_err("Resv shared slot allocation failed\n"); - goto err_unlock; - } + r = dma_resv_reserve_shared(&resv, 1); + if (r) { + pr_err("Resv shared slot allocation failed\n"); + goto err_unlock; + } + if (shared) dma_resv_add_shared_fence(&resv, f); - } else { + else dma_resv_add_excl_fence(&resv, f); - } if (dma_resv_test_signaled(&resv, shared)) { pr_err("Resv unexpectedly signaled\n"); @@ -134,17 +133,16 @@ static int test_for_each(void *arg, bool shared) goto err_free; } - if (shared) { - r = dma_resv_reserve_shared(&resv, 1); - if (r) { - pr_err("Resv shared slot allocation failed\n"); - goto err_unlock; - } + r = dma_resv_reserve_shared(&resv, 1); + if (r) { + pr_err("Resv shared slot allocation failed\n"); + goto err_unlock; + } + if (shared) dma_resv_add_shared_fence(&resv, f); - } else { + else dma_resv_add_excl_fence(&resv, f); - } r = -ENOENT; dma_resv_for_each_fence(&cursor, &resv, shared, fence) { @@ -206,18 +204,17 @@ static int test_for_each_unlocked(void *arg, bool shared) goto err_free; } - if (shared) { - r = dma_resv_reserve_shared(&resv, 1); - if (r) { - pr_err("Resv shared slot allocation failed\n"); - dma_resv_unlock(&resv); - goto err_free; - } + r = dma_resv_reserve_shared(&resv, 1); + if (r) { + pr_err("Resv shared slot allocation failed\n"); + dma_resv_unlock(&resv); + goto err_free; + } + if (shared) dma_resv_add_shared_fence(&resv, f); - } else { + else dma_resv_add_excl_fence(&resv, f); - } dma_resv_unlock(&resv); r = -ENOENT; @@ -290,18 +287,17 @@ static int test_get_fences(void *arg, bool shared) goto err_resv; } - if (shared) { - r = dma_resv_reserve_shared(&resv, 1); - if (r) { - pr_err("Resv shared slot allocation failed\n"); - dma_resv_unlock(&resv); - goto err_resv; - } + r = dma_resv_reserve_shared(&resv, 1); + if (r) { + pr_err("Resv shared slot allocation failed\n"); + dma_resv_unlock(&resv); + goto err_resv; + } + if (shared) dma_resv_add_shared_fence(&resv, f); - } else { + else dma_resv_add_excl_fence(&resv, f); - } dma_resv_unlock(&resv); r = dma_resv_get_fences(&resv, shared, &i, &fences); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c index 4fcfc2313b8c..24a6b88afcca 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdg
[PATCH 16/24] drm/nouveau: support more than one write fence in fenv50_wndw_prepare_fb
Use dma_resv_get_singleton() here to eventually get more than one write fence as single fence. Signed-off-by: Christian König --- drivers/gpu/drm/nouveau/dispnv50/wndw.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/nouveau/dispnv50/wndw.c b/drivers/gpu/drm/nouveau/dispnv50/wndw.c index 133c8736426a..b55a8a723581 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/wndw.c +++ b/drivers/gpu/drm/nouveau/dispnv50/wndw.c @@ -536,8 +536,6 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) struct nouveau_bo *nvbo; struct nv50_head_atom *asyh; struct nv50_wndw_ctxdma *ctxdma; - struct dma_resv_iter cursor; - struct dma_fence *fence; int ret; NV_ATOMIC(drm, "%s prepare: %p\n", plane->name, fb); @@ -560,13 +558,11 @@ nv50_wndw_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) asyw->image.handle[0] = ctxdma->object.handle; } - dma_resv_iter_begin(&cursor, nvbo->bo.base.resv, false); - dma_resv_for_each_fence_unlocked(&cursor, fence) { - /* TODO: We only use the first writer here */ - asyw->state.fence = dma_fence_get(fence); - break; - } - dma_resv_iter_end(&cursor); + ret = dma_resv_get_singleton(nvbo->bo.base.resv, false, +&asyw->state.fence); + if (ret) + return ret; + asyw->image.offset[0] = nvbo->offset; if (wndw->func->prepare) { -- 2.25.1
[PATCH 15/24] drm: support more than one write fence in drm_gem_plane_helper_prepare_fb
Use dma_resv_get_singleton() here to eventually get more than one write fence as single fence. Signed-off-by: Christian König --- drivers/gpu/drm/drm_gem_atomic_helper.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_atomic_helper.c b/drivers/gpu/drm/drm_gem_atomic_helper.c index c3189afe10cb..9338ddb7edff 100644 --- a/drivers/gpu/drm/drm_gem_atomic_helper.c +++ b/drivers/gpu/drm/drm_gem_atomic_helper.c @@ -143,25 +143,21 @@ */ int drm_gem_plane_helper_prepare_fb(struct drm_plane *plane, struct drm_plane_state *state) { - struct dma_resv_iter cursor; struct drm_gem_object *obj; struct dma_fence *fence; + int ret; if (!state->fb) return 0; obj = drm_gem_fb_get_obj(state->fb, 0); - dma_resv_iter_begin(&cursor, obj->resv, false); - dma_resv_for_each_fence_unlocked(&cursor, fence) { - /* TODO: Currently there should be only one write fence, so this -* here works fine. But drm_atomic_set_fence_for_plane() should -* be changed to be able to handle more fences in general for -* multiple BOs per fb anyway. */ - dma_fence_get(fence); - break; - } - dma_resv_iter_end(&cursor); + ret = dma_resv_get_singleton(obj->resv, false, &fence); + if (ret) + return ret; + /* TODO: drm_atomic_set_fence_for_plane() should be changed to be able +* to handle more fences in general for multiple BOs per fb. +*/ drm_atomic_set_fence_for_plane(state, fence); return 0; } -- 2.25.1
[PATCH 13/24] dma-buf: drop the DAG approach for the dma_resv object
So far we had the approach of using a directed acyclic graph with the dma_resv obj. This turned out to have many downsides, especially it means that every single driver and user of this interface needs to be aware of this restriction when adding fences. If the rules for the DAG are not followed then we end up with potential hard to debug memory corruption, information leaks or even elephant big security holes because we allow userspace to access freed up memory. Since we already took a step back from that by always looking at all fences we now go a step further and stop dropping the shared fences when a new exclusive one is added. Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c | 13 - 1 file changed, 13 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 9acceabc9399..ecb2ff606bac 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -383,29 +383,16 @@ EXPORT_SYMBOL(dma_resv_replace_fences); void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence) { struct dma_fence *old_fence = dma_resv_excl_fence(obj); - struct dma_resv_list *old; - u32 i = 0; dma_resv_assert_held(obj); - old = dma_resv_shared_list(obj); - if (old) - i = old->shared_count; - dma_fence_get(fence); write_seqcount_begin(&obj->seq); /* write_seqcount_begin provides the necessary memory barrier */ RCU_INIT_POINTER(obj->fence_excl, fence); - if (old) - old->shared_count = 0; write_seqcount_end(&obj->seq); - /* inplace update, no shared fences */ - while (i--) - dma_fence_put(rcu_dereference_protected(old->shared[i], - dma_resv_held(obj))); - dma_fence_put(old_fence); } EXPORT_SYMBOL(dma_resv_add_excl_fence); -- 2.25.1
[PATCH 10/24] drm/amdgpu: remove excl as shared workarounds
This was added because of the now dropped shared on excl dependency. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 5 + drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 -- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 0311d799a010..53e407ea4c89 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1275,14 +1275,11 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, /* * Work around dma_resv shortcommings by wrapping up the * submission in a dma_fence_chain and add it as exclusive -* fence, but first add the submission as shared fence to make -* sure that shared fences never signal before the exclusive -* one. +* fence. */ dma_fence_chain_init(chain, dma_resv_excl_fence(resv), dma_fence_get(p->fence), 1); - dma_resv_add_shared_fence(resv, p->fence); rcu_assign_pointer(resv->fence_excl, &chain->base); e->chain = NULL; } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c index a1e63ba4c54a..85d31d85c384 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c @@ -226,12 +226,6 @@ static void amdgpu_gem_object_close(struct drm_gem_object *obj, if (!amdgpu_vm_ready(vm)) goto out_unlock; - fence = dma_resv_excl_fence(bo->tbo.base.resv); - if (fence) { - amdgpu_bo_fence(bo, fence, true); - fence = NULL; - } - r = amdgpu_vm_clear_freed(adev, vm, &fence); if (r || !fence) goto out_unlock; -- 2.25.1
[PATCH 12/24] dma-buf: finally make dma_resv_excl_fence private
Drivers should never touch this directly. Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c | 17 + include/linux/dma-resv.h | 17 - 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 694716a3d66d..9acceabc9399 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -147,6 +147,23 @@ void dma_resv_fini(struct dma_resv *obj) } EXPORT_SYMBOL(dma_resv_fini); +/** + * dma_resv_excl_fence - return the object's exclusive fence + * @obj: the reservation object + * + * Returns the exclusive fence (if any). Caller must either hold the objects + * through dma_resv_lock() or the RCU read side lock through rcu_read_lock(), + * or one of the variants of each + * + * RETURNS + * The exclusive fence or NULL + */ +static inline struct dma_fence * +dma_resv_excl_fence(struct dma_resv *obj) +{ + return rcu_dereference_check(obj->fence_excl, dma_resv_held(obj)); +} + /** * dma_resv_shared_list - get the reservation object's shared fence list * @obj: the reservation object diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index cdfbbda6f600..40ac9d486f8f 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -412,23 +412,6 @@ static inline void dma_resv_unlock(struct dma_resv *obj) ww_mutex_unlock(&obj->lock); } -/** - * dma_resv_excl_fence - return the object's exclusive fence - * @obj: the reservation object - * - * Returns the exclusive fence (if any). Caller must either hold the objects - * through dma_resv_lock() or the RCU read side lock through rcu_read_lock(), - * or one of the variants of each - * - * RETURNS - * The exclusive fence or NULL - */ -static inline struct dma_fence * -dma_resv_excl_fence(struct dma_resv *obj) -{ - return rcu_dereference_check(obj->fence_excl, dma_resv_held(obj)); -} - void dma_resv_init(struct dma_resv *obj); void dma_resv_fini(struct dma_resv *obj); int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences); -- 2.25.1
[PATCH 11/24] drm/amdgpu: use dma_resv_for_each_fence for CS workaround
Get the write fence using dma_resv_for_each_fence instead of accessing it manually. Signed-off-by: Christian König --- drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c index 53e407ea4c89..7facd614e50a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c @@ -1268,6 +1268,8 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, amdgpu_bo_list_for_each_entry(e, p->bo_list) { struct dma_resv *resv = e->tv.bo->base.resv; struct dma_fence_chain *chain = e->chain; + struct dma_resv_iter cursor; + struct dma_fence *fence; if (!chain) continue; @@ -1277,9 +1279,10 @@ static int amdgpu_cs_submit(struct amdgpu_cs_parser *p, * submission in a dma_fence_chain and add it as exclusive * fence. */ - dma_fence_chain_init(chain, dma_resv_excl_fence(resv), -dma_fence_get(p->fence), 1); - + dma_resv_for_each_fence(&cursor, resv, false, fence) { + break; + } + dma_fence_chain_init(chain, fence, dma_fence_get(p->fence), 1); rcu_assign_pointer(resv->fence_excl, &chain->base); e->chain = NULL; } -- 2.25.1
[PATCH 07/24] drm/nouveau: stop using dma_resv_excl_fence
Instead use the new dma_resv_get_singleton function. Signed-off-by: Christian König --- drivers/gpu/drm/nouveau/nouveau_bo.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c index fa73fe57f97b..74f8652d2bd3 100644 --- a/drivers/gpu/drm/nouveau/nouveau_bo.c +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c @@ -959,7 +959,14 @@ nouveau_bo_vm_cleanup(struct ttm_buffer_object *bo, { struct nouveau_drm *drm = nouveau_bdev(bo->bdev); struct drm_device *dev = drm->dev; - struct dma_fence *fence = dma_resv_excl_fence(bo->base.resv); + struct dma_fence *fence; + int ret; + + /* TODO: This is actually a memory management dependency */ + ret = dma_resv_get_singleton(bo->base.resv, false, &fence); + if (ret) + dma_resv_wait_timeout(bo->base.resv, false, false, + MAX_SCHEDULE_TIMEOUT); nv10_bo_put_tile_region(dev, *old_tile, fence); *old_tile = new_tile; -- 2.25.1
[PATCH 08/24] drm/vmwgfx: stop using dma_resv_excl_fence
Instead use the new dma_resv_get_singleton function. Signed-off-by: Christian König --- drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c index 8d1e869cc196..23c3fc2cbf10 100644 --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c @@ -1168,8 +1168,10 @@ int vmw_resources_clean(struct vmw_buffer_object *vbo, pgoff_t start, vmw_bo_fence_single(bo, NULL); if (bo->moving) dma_fence_put(bo->moving); - bo->moving = dma_fence_get - (dma_resv_excl_fence(bo->base.resv)); + + /* TODO: This is actually a memory management dependency */ + return dma_resv_get_singleton(bo->base.resv, false, + &bo->moving); } return 0; -- 2.25.1
[PATCH 09/24] drm/radeon: stop using dma_resv_excl_fence
Instead use the new dma_resv_get_singleton function. Signed-off-by: Christian König --- drivers/gpu/drm/radeon/radeon_display.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index 573154268d43..a6f875118f01 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -533,7 +533,12 @@ static int radeon_crtc_page_flip_target(struct drm_crtc *crtc, DRM_ERROR("failed to pin new rbo buffer before flip\n"); goto cleanup; } - work->fence = dma_fence_get(dma_resv_excl_fence(new_rbo->tbo.base.resv)); + r = dma_resv_get_singleton(new_rbo->tbo.base.resv, false, &work->fence); + if (r) { + radeon_bo_unreserve(new_rbo); + DRM_ERROR("failed to get new rbo buffer fences\n"); + goto cleanup; + } radeon_bo_get_tiling_flags(new_rbo, &tiling_flags, NULL); radeon_bo_unreserve(new_rbo); -- 2.25.1
[PATCH 06/24] drm/etnaviv: stop using dma_resv_excl_fence
We can get the excl fence together with the shared ones as well. Signed-off-by: Christian König --- drivers/gpu/drm/etnaviv/etnaviv_gem.h| 1 - drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 14 +- drivers/gpu/drm/etnaviv/etnaviv_sched.c | 10 -- 3 files changed, 5 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h index 98e60df882b6..f596d743baa3 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h @@ -80,7 +80,6 @@ struct etnaviv_gem_submit_bo { u64 va; struct etnaviv_gem_object *obj; struct etnaviv_vram_mapping *mapping; - struct dma_fence *excl; unsigned int nr_shared; struct dma_fence **shared; }; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c index 64c90ff348f2..4286dc93fdaa 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c @@ -188,15 +188,11 @@ static int submit_fence_sync(struct etnaviv_gem_submit *submit) if (submit->flags & ETNA_SUBMIT_NO_IMPLICIT) continue; - if (bo->flags & ETNA_SUBMIT_BO_WRITE) { - ret = dma_resv_get_fences(robj, true, &bo->nr_shared, - &bo->shared); - if (ret) - return ret; - } else { - bo->excl = dma_fence_get(dma_resv_excl_fence(robj)); - } - + ret = dma_resv_get_fences(robj, + !!(bo->flags & ETNA_SUBMIT_BO_WRITE), + &bo->nr_shared, &bo->shared); + if (ret) + return ret; } return ret; diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c index 180bb633d5c5..8c038a363d15 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c @@ -39,16 +39,6 @@ etnaviv_sched_dependency(struct drm_sched_job *sched_job, struct etnaviv_gem_submit_bo *bo = &submit->bos[i]; int j; - if (bo->excl) { - fence = bo->excl; - bo->excl = NULL; - - if (!dma_fence_is_signaled(fence)) - return fence; - - dma_fence_put(fence); - } - for (j = 0; j < bo->nr_shared; j++) { if (!bo->shared[j]) continue; -- 2.25.1
[PATCH 03/24] dma-buf: drop excl_fence parameter from dma_resv_get_fences
Returning the exclusive fence separately is no longer used. Instead add a write parameter to indicate the use case. Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c | 48 drivers/dma-buf/st-dma-resv.c| 26 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 6 ++- drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 3 +- include/linux/dma-resv.h | 4 +- 6 files changed, 31 insertions(+), 58 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index a12a3a39f280..480c305554a1 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -611,57 +611,45 @@ EXPORT_SYMBOL(dma_resv_copy_fences); * dma_resv_get_fences - Get an object's shared and exclusive * fences without update side lock held * @obj: the reservation object - * @fence_excl: the returned exclusive fence (or NULL) - * @shared_count: the number of shared fences returned - * @shared: the array of shared fence ptrs returned (array is krealloc'd to - * the required size, and must be freed by caller) - * - * Retrieve all fences from the reservation object. If the pointer for the - * exclusive fence is not specified the fence is put into the array of the - * shared fences as well. Returns either zero or -ENOMEM. + * @write: true if we should return all fences + * @num_fences: the number of fences returned + * @fences: the array of fence ptrs returned (array is krealloc'd to the + * required size, and must be freed by caller) + * + * Retrieve all fences from the reservation object. + * Returns either zero or -ENOMEM. */ -int dma_resv_get_fences(struct dma_resv *obj, struct dma_fence **fence_excl, - unsigned int *shared_count, struct dma_fence ***shared) +int dma_resv_get_fences(struct dma_resv *obj, bool write, + unsigned int *num_fences, struct dma_fence ***fences) { struct dma_resv_iter cursor; struct dma_fence *fence; - *shared_count = 0; - *shared = NULL; - - if (fence_excl) - *fence_excl = NULL; + *num_fences = 0; + *fences = NULL; - dma_resv_iter_begin(&cursor, obj, true); + dma_resv_iter_begin(&cursor, obj, write); dma_resv_for_each_fence_unlocked(&cursor, fence) { if (dma_resv_iter_is_restarted(&cursor)) { unsigned int count; - while (*shared_count) - dma_fence_put((*shared)[--(*shared_count)]); + while (*num_fences) + dma_fence_put((*fences)[--(*num_fences)]); - if (fence_excl) - dma_fence_put(*fence_excl); - - count = cursor.shared_count; - count += fence_excl ? 0 : 1; + count = cursor.shared_count + 1; /* Eventually re-allocate the array */ - *shared = krealloc_array(*shared, count, + *fences = krealloc_array(*fences, count, sizeof(void *), GFP_KERNEL); - if (count && !*shared) { + if (count && !*fences) { dma_resv_iter_end(&cursor); return -ENOMEM; } } - dma_fence_get(fence); - if (dma_resv_iter_is_exclusive(&cursor) && fence_excl) - *fence_excl = fence; - else - (*shared)[(*shared_count)++] = fence; + (*fences)[(*num_fences)++] = dma_fence_get(fence); } dma_resv_iter_end(&cursor); diff --git a/drivers/dma-buf/st-dma-resv.c b/drivers/dma-buf/st-dma-resv.c index bc32b3eedcb6..cbe999c6e7a6 100644 --- a/drivers/dma-buf/st-dma-resv.c +++ b/drivers/dma-buf/st-dma-resv.c @@ -275,7 +275,7 @@ static int test_shared_for_each_unlocked(void *arg) static int test_get_fences(void *arg, bool shared) { - struct dma_fence *f, *excl = NULL, **fences = NULL; + struct dma_fence *f, **fences = NULL; struct dma_resv resv; int r, i; @@ -304,35 +304,19 @@ static int test_get_fences(void *arg, bool shared) } dma_resv_unlock(&resv); - r = dma_resv_get_fences(&resv, &excl, &i, &fences); + r = dma_resv_get_fences(&resv, shared, &i, &fences); if (r) { pr_err("get_fences failed\n"); goto err_free; } - if (shared) { - if (excl != NULL) { - pr_err("get_fences returned unexpected excl fence\n"); - goto err_free; - } - if (i != 1 || fences[0] != f) { -
[PATCH 04/24] dma-buf: add dma_resv_get_singleton v2
Add a function to simplify getting a single fence for all the fences in the dma_resv object. v2: fix ref leak in error handling Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c | 52 ++ include/linux/dma-resv.h | 2 ++ 2 files changed, 54 insertions(+) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 480c305554a1..694716a3d66d 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -34,6 +34,7 @@ */ #include +#include #include #include #include @@ -657,6 +658,57 @@ int dma_resv_get_fences(struct dma_resv *obj, bool write, } EXPORT_SYMBOL_GPL(dma_resv_get_fences); +/** + * dma_resv_get_singleton - Get a single fence for all the fences + * @obj: the reservation object + * @write: true if we should return all fences + * @fence: the resulting fence + * + * Get a single fence representing all the fences inside the resv object. + * Returns either 0 for success or -ENOMEM. + * + * Warning: This can't be used like this when adding the fence back to the resv + * object since that can lead to stack corruption when finalizing the + * dma_fence_array. + */ +int dma_resv_get_singleton(struct dma_resv *obj, bool write, + struct dma_fence **fence) +{ + struct dma_fence_array *array; + struct dma_fence **fences; + unsigned count; + int r; + + r = dma_resv_get_fences(obj, write, &count, &fences); +if (r) + return r; + + if (count == 0) { + *fence = NULL; + return 0; + } + + if (count == 1) { + *fence = fences[0]; + kfree(fences); + return 0; + } + + array = dma_fence_array_create(count, fences, + dma_fence_context_alloc(1), + 1, false); + if (!array) { + while (count--) + dma_fence_put(fences[count]); + kfree(fences); + return -ENOMEM; + } + + *fence = &array->base; + return 0; +} +EXPORT_SYMBOL_GPL(dma_resv_get_singleton); + /** * dma_resv_wait_timeout - Wait on reservation's objects * shared and/or exclusive fences. diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index fa2002939b19..cdfbbda6f600 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -438,6 +438,8 @@ void dma_resv_replace_fences(struct dma_resv *obj, uint64_t context, void dma_resv_add_excl_fence(struct dma_resv *obj, struct dma_fence *fence); int dma_resv_get_fences(struct dma_resv *obj, bool write, unsigned int *num_fences, struct dma_fence ***fences); +int dma_resv_get_singleton(struct dma_resv *obj, bool write, + struct dma_fence **fence); int dma_resv_copy_fences(struct dma_resv *dst, struct dma_resv *src); long dma_resv_wait_timeout(struct dma_resv *obj, bool wait_all, bool intr, unsigned long timeout); -- 2.25.1
[PATCH 05/24] RDMA: use dma_resv_wait() instead of extracting the fence
Use dma_resv_wait() instead of extracting the exclusive fence and waiting on it manually. Signed-off-by: Christian König --- drivers/infiniband/core/umem_dmabuf.c | 8 ++-- 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/infiniband/core/umem_dmabuf.c b/drivers/infiniband/core/umem_dmabuf.c index f0760741f281..d32cd7538835 100644 --- a/drivers/infiniband/core/umem_dmabuf.c +++ b/drivers/infiniband/core/umem_dmabuf.c @@ -16,7 +16,6 @@ int ib_umem_dmabuf_map_pages(struct ib_umem_dmabuf *umem_dmabuf) { struct sg_table *sgt; struct scatterlist *sg; - struct dma_fence *fence; unsigned long start, end, cur = 0; unsigned int nmap = 0; int i; @@ -68,11 +67,8 @@ int ib_umem_dmabuf_map_pages(struct ib_umem_dmabuf *umem_dmabuf) * may be not up-to-date. Wait for the exporter to finish * the migration. */ - fence = dma_resv_excl_fence(umem_dmabuf->attach->dmabuf->resv); - if (fence) - return dma_fence_wait(fence, false); - - return 0; + return dma_resv_wait_timeout(umem_dmabuf->attach->dmabuf->resv, false, +false, MAX_SCHEDULE_TIMEOUT); } EXPORT_SYMBOL(ib_umem_dmabuf_map_pages); -- 2.25.1
[PATCH 02/24] dma-buf: finally make the dma_resv_list private
Drivers should never touch this directly. Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c | 26 ++ include/linux/dma-resv.h | 26 +- 2 files changed, 27 insertions(+), 25 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index a688dbded3d3..a12a3a39f280 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -56,6 +56,19 @@ DEFINE_WD_CLASS(reservation_ww_class); EXPORT_SYMBOL(reservation_ww_class); +/** + * struct dma_resv_list - a list of shared fences + * @rcu: for internal use + * @shared_count: table of shared fences + * @shared_max: for growing shared fence table + * @shared: shared fence table + */ +struct dma_resv_list { + struct rcu_head rcu; + u32 shared_count, shared_max; + struct dma_fence __rcu *shared[]; +}; + /** * dma_resv_list_alloc - allocate fence list * @shared_max: number of fences we need space for @@ -133,6 +146,19 @@ void dma_resv_fini(struct dma_resv *obj) } EXPORT_SYMBOL(dma_resv_fini); +/** + * dma_resv_shared_list - get the reservation object's shared fence list + * @obj: the reservation object + * + * Returns the shared fence list. Caller must either hold the objects + * through dma_resv_lock() or the RCU read side lock through rcu_read_lock(), + * or one of the variants of each + */ +static inline struct dma_resv_list *dma_resv_shared_list(struct dma_resv *obj) +{ + return rcu_dereference_check(obj->fence, dma_resv_held(obj)); +} + /** * dma_resv_reserve_shared - Reserve space to add shared fences to * a dma_resv. diff --git a/include/linux/dma-resv.h b/include/linux/dma-resv.h index e0be34265eae..3baf2a4a9a0d 100644 --- a/include/linux/dma-resv.h +++ b/include/linux/dma-resv.h @@ -47,18 +47,7 @@ extern struct ww_class reservation_ww_class; -/** - * struct dma_resv_list - a list of shared fences - * @rcu: for internal use - * @shared_count: table of shared fences - * @shared_max: for growing shared fence table - * @shared: shared fence table - */ -struct dma_resv_list { - struct rcu_head rcu; - u32 shared_count, shared_max; - struct dma_fence __rcu *shared[]; -}; +struct dma_resv_list; /** * struct dma_resv - a reservation object manages fences for a buffer @@ -440,19 +429,6 @@ dma_resv_excl_fence(struct dma_resv *obj) return rcu_dereference_check(obj->fence_excl, dma_resv_held(obj)); } -/** - * dma_resv_shared_list - get the reservation object's shared fence list - * @obj: the reservation object - * - * Returns the shared fence list. Caller must either hold the objects - * through dma_resv_lock() or the RCU read side lock through rcu_read_lock(), - * or one of the variants of each - */ -static inline struct dma_resv_list *dma_resv_shared_list(struct dma_resv *obj) -{ - return rcu_dereference_check(obj->fence, dma_resv_held(obj)); -} - void dma_resv_init(struct dma_resv *obj); void dma_resv_fini(struct dma_resv *obj); int dma_resv_reserve_shared(struct dma_resv *obj, unsigned int num_fences); -- 2.25.1
[PATCH 01/24] dma-buf: add dma_resv_replace_fences
This function allows to replace fences from the shared fence list when we can gurantee that the operation represented by the original fence has finished or no accesses to the resources protected by the dma_resv object any more when the new fence finishes. Then use this function in the amdkfd code when BOs are unmapped from the process. Signed-off-by: Christian König --- drivers/dma-buf/dma-resv.c| 43 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 49 +++ include/linux/dma-resv.h | 2 + 3 files changed, 52 insertions(+), 42 deletions(-) diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c index 4deea75c0b9c..a688dbded3d3 100644 --- a/drivers/dma-buf/dma-resv.c +++ b/drivers/dma-buf/dma-resv.c @@ -284,6 +284,49 @@ void dma_resv_add_shared_fence(struct dma_resv *obj, struct dma_fence *fence) } EXPORT_SYMBOL(dma_resv_add_shared_fence); +/** + * dma_resv_replace_fences - replace fences in the dma_resv obj + * @obj: the reservation object + * @context: the context of the fences to replace + * @replacement: the new fence to use instead + * + * Replace fences with a specified context with a new fence. Only valid if the + * operation represented by the original fences is completed or has no longer + * access to the resources protected by the dma_resv object when the new fence + * completes. + */ +void dma_resv_replace_fences(struct dma_resv *obj, uint64_t context, +struct dma_fence *replacement) +{ + struct dma_resv_list *list; + struct dma_fence *old; + unsigned int i; + + dma_resv_assert_held(obj); + + write_seqcount_begin(&obj->seq); + + old = dma_resv_excl_fence(obj); + if (old->context == context) { + RCU_INIT_POINTER(obj->fence_excl, dma_fence_get(replacement)); + dma_fence_put(old); + } + + list = dma_resv_shared_list(obj); + for (i = 0; list && i < list->shared_count; ++i) { + old = rcu_dereference_protected(list->shared[i], + dma_resv_held(obj)); + if (old->context != context) + continue; + + rcu_assign_pointer(list->shared[i], dma_fence_get(replacement)); + dma_fence_put(old); + } + + write_seqcount_end(&obj->seq); +} +EXPORT_SYMBOL(dma_resv_replace_fences); + /** * dma_resv_add_excl_fence - Add an exclusive fence. * @obj: the reservation object diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c index 71acd577803e..b558ef0f8c4a 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c @@ -236,53 +236,18 @@ void amdgpu_amdkfd_release_notify(struct amdgpu_bo *bo) static int amdgpu_amdkfd_remove_eviction_fence(struct amdgpu_bo *bo, struct amdgpu_amdkfd_fence *ef) { - struct dma_resv *resv = bo->tbo.base.resv; - struct dma_resv_list *old, *new; - unsigned int i, j, k; + struct dma_fence *replacement; if (!ef) return -EINVAL; - old = dma_resv_shared_list(resv); - if (!old) - return 0; - - new = kmalloc(struct_size(new, shared, old->shared_max), GFP_KERNEL); - if (!new) - return -ENOMEM; - - /* Go through all the shared fences in the resevation object and sort -* the interesting ones to the end of the list. + /* TODO: Instead of block before we should use the fence of the page +* table update and TLB flush here directly. */ - for (i = 0, j = old->shared_count, k = 0; i < old->shared_count; ++i) { - struct dma_fence *f; - - f = rcu_dereference_protected(old->shared[i], - dma_resv_held(resv)); - - if (f->context == ef->base.context) - RCU_INIT_POINTER(new->shared[--j], f); - else - RCU_INIT_POINTER(new->shared[k++], f); - } - new->shared_max = old->shared_max; - new->shared_count = k; - - /* Install the new fence list, seqcount provides the barriers */ - write_seqcount_begin(&resv->seq); - RCU_INIT_POINTER(resv->fence, new); - write_seqcount_end(&resv->seq); - - /* Drop the references to the removed fences or move them to ef_list */ - for (i = j; i < old->shared_count; ++i) { - struct dma_fence *f; - - f = rcu_dereference_protected(new->shared[i], - dma_resv_held(resv)); - dma_fence_put(f); - } - kfree_rcu(old, rcu); - + replacement = dma_fence_get_stub(); + dma_resv_replace_fences(bo->tbo.base.resv, ef->base.context, + rep
completely rework the dma_resv semantic
Hi Daniel, just a gentle ping that you wanted to take a look at this. Not much changed compared to the last version, only a minor bugfix in the dma_resv_get_singleton error handling. Regards, Christian.
Re: [RFC] drm/msm/dp: Allow attaching a drm_panel
+Hans and Imre On Mon, Dec 06, 2021 at 02:31:40PM -0800, Bjorn Andersson wrote: > On Thu 07 Oct 03:17 PDT 2021, Heikki Krogerus wrote: > > On Wed, Oct 06, 2021 at 01:26:35PM -0700, Prashant Malani wrote: > > > (CC+ Heikki) > [..] > > > On Wed, Oct 6, 2021 at 8:19 AM Doug Anderson > > > wrote: > [..] > > void drm_connector_oob_hotplug_event(struct fwnode_handle > > *connector_fwnode); > > > > If your USB Type-C controller/port driver does not yet register the DP > > alt mode, the it's responsible of handling HPD separately by calling > > drm_connector_oob_hotplug_event() on its own. > > > > Finally found my way back to this topic and it doesn't look like I can > reuse the existing altmode code with the firmware interface provided by > Qualcomm, so I just hacked something up that invokes > drm_connector_oob_hotplug_event(). > > But I'm not able to make sense of what the expected usage is. Reading > altmode/displayport.c, it seems that I should only invoke > drm_connector_oob_hotplug_event() as HPD state toggles. > > I made a trial implementation of this, where my firmware interface > driver calls drm_connector_oob_hotplug_event() every time HPD state > changes and then in my oob_hotplug_event callback I flip the DP > controller between on and off. > > Unfortunately when I then connect my HDMI dongle, I get HPD state HIGH, > call the oob_hotplug_event, the DP driver powers up and concludes that > there's nothing connected to the dongle and goes to idle. I then connect > the HDMI cable to the dongle, the firmware sends me another message with > HPD irq and state HIGH, which I ignore because it's not a change in > state. > > In the end I hacked up drm_connector_oob_hotplug_event() to allow me to > pass the HPD state and this solves my problem. I can now distinguish > between connect, disconnect and attention. > > Can you please help shed some light on what I might be missing? Originally I wanted an API that we could use to pass all the details that we have in the USB Type-C drivers (that would be the configuration and status) to the GPU drivers, but Hans was against that because, if I remember correctly, the OOB hotplug event may need to be delivered to the GPU drivers in some cases even when the connector is not USB Type-C connector, and he wanted a common API. Hans, please correct me if I got it wrong. I think that the GPU drivers need to handle USB Type-C connectors separately one way or the other, but maybe the notification from the connector can continue to be generic - not USB Type-C specific. Imre proposed that the GPU drivers should be able to query the DisplayPort configuration and status from the USB Type-C drivers instead of the USB Type-C drivers just dumping the information together with the notification about some event (so connection, disconnection or attention) like I originally proposed. Imre, please correct me if I misunderstood you :-). I'm fine with anything, but we do need improvement here as you guys can see. thanks, -- heikki
Re: [PATCH] drm/syncobj: Deal with signalled fences in transfer.
Am 07.12.21 um 12:35 schrieb Bas Nieuwenhuizen: On Tue, Dec 7, 2021 at 12:28 PM Lionel Landwerlin wrote: On 07/12/2021 13:00, Christian König wrote: Am 07.12.21 um 11:40 schrieb Bas Nieuwenhuizen: On Tue, Dec 7, 2021 at 8:21 AM Christian König wrote: Am 07.12.21 um 08:10 schrieb Lionel Landwerlin: On 07/12/2021 03:32, Bas Nieuwenhuizen wrote: See the comments in the code. Basically if the seqno is already signalled then we get a NULL fence. If we then put the NULL fence in a binary syncobj it counts as unsignalled, making that syncobj pretty much useless for all expected uses. Not 100% sure about the transfer to a timeline syncobj but I believe it is needed there too, as AFAICT the add_point function assumes the fence isn't NULL. Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between binary and timeline v2") Cc: sta...@vger.kernel.org Signed-off-by: Bas Nieuwenhuizen --- drivers/gpu/drm/drm_syncobj.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index fdd2ec87cdd1..eb28a40400d2 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -861,6 +861,19 @@ static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private, &fence); if (ret) goto err; + +/* If the requested seqno is already signaled drm_syncobj_find_fence may + * return a NULL fence. To make sure the recipient gets signalled, use + * a new fence instead. + */ +if (!fence) { +fence = dma_fence_allocate_private_stub(); +if (!fence) { +ret = -ENOMEM; +goto err; +} +} + Shouldn't we fix drm_syncobj_find_fence() instead? Mhm, now that you mention it. Bas, why do you think that dma_fence_chain_find_seqno() may return NULL when the fence is already signaled? Double checking the code that should never ever happen. Well, I tested the patch with https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fmerge_requests%2F14097%2Fdiffs%3Fcommit_id%3Dd4c5c840f4e3839f9f5c1747a9034eb2b565f5c0&data=04%7C01%7Cchristian.koenig%40amd.com%7C342b0cbdff7d487630ae08d9b975abd9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637744738372115823%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=5SiZYL1TgLq3ldGy1COOkSasklZWQN%2BxWGXJ1j%2BHSOQ%3D&reserved=0 so I'm pretty sure it happens, and this patch fixes it, though I may have misidentified what the code should do. My reading is that the dma_fence_chain_for_each in dma_fence_chain_find_seqno will never visit a signalled fence (unless the top one is signalled), as dma_fence_chain_walk will never return a signalled fence (it only returns on NULL or !signalled). Ah, yes that suddenly makes more sense. Happy to move this to drm_syncobj_find_fence. No, I think that your current patch is fine. That drm_syncobj_find_fence() only returns NULL when it can't find anything !signaled is correct behavior I think. We should probably update the docs then : * Returns 0 on success or a negative error value on failure. On success @fence * contains a reference to the fence, which must be released by calling * dma_fence_put(). Looking at some of the kernel drivers, it looks like they don't all protect themselves against NULL pointers : https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fgpu%2Fdrm%2Fvc4%2Fvc4_gem.c%23L1195&data=04%7C01%7Cchristian.koenig%40amd.com%7C342b0cbdff7d487630ae08d9b975abd9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637744738372115823%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=LjyWQpIUqqAGgR7ak3CJOJTqf%2FG8QB9BZX542vL25RA%3D&reserved=0 https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fgpu%2Fdrm%2Famd%2Famdgpu%2Famdgpu_cs.c%23L1020&data=04%7C01%7Cchristian.koenig%40amd.com%7C342b0cbdff7d487630ae08d9b975abd9%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637744738372115823%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=kk9k8sDNLiOFTIyul79FbhfQ4Y2MdwFA6rT0h46xM40%3D&reserved=0 amdgpu handles it here (amdgpu_sync_fence checks for a NULL fence). But yeah I think it is a bit treacherous, especially as this only occurs with timeline semaphores. Mhm, that's a good point. While I still think it makes more sense from the design perspective to return NULL to distinct that there is nothing to wait for, I see as well that this is it not the most defensive approach. Ok in that case let's move it into drm_syncobj_find_fence() instead. Just one more thing, the timestamp is now busted anyway (cause the original fence is already garbage collected). So we can probably us
Re: [PATCH] drm/rockchip: use generic fbdev setup
Hi Thomas, On Mon, Nov 01, 2021 at 11:34:15AM +, John Keeping wrote: > On Sun, 31 Oct 2021 19:09:39 +0100 > Thomas Zimmermann wrote: > > Am 30.10.21 um 14:05 schrieb John Keeping: > > > On Fri, Oct 29, 2021 at 09:00:08PM +0200, Thomas Zimmermann wrote: > > >> Am 29.10.21 um 13:50 schrieb John Keeping: > > >>> The Rockchip fbdev code does not add anything compared to > > >>> drm_fbdev_generic_setup(); the one custom function for .fb_mmap does the > > >>> same thing as gem_prime_mmap which is called by the helper. > > >>> > > >>> Signed-off-by: John Keeping > > >>> --- > > >>>drivers/gpu/drm/rockchip/Makefile | 1 - > > >>>drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 10 +- > > >>>drivers/gpu/drm/rockchip/rockchip_drm_drv.h | 2 - > > >>>drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c | 164 > > >>> -- > > >>>drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h | 24 --- > > >>>5 files changed, 2 insertions(+), 199 deletions(-) > > >>>delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c > > >>>delete mode 100644 drivers/gpu/drm/rockchip/rockchip_drm_fbdev.h > > >>> > > >>> diff --git a/drivers/gpu/drm/rockchip/Makefile > > >>> b/drivers/gpu/drm/rockchip/Makefile > > >>> index 17a9e7eb2130..1a56f696558c 100644 > > >>> --- a/drivers/gpu/drm/rockchip/Makefile > > >>> +++ b/drivers/gpu/drm/rockchip/Makefile > > >>> @@ -5,7 +5,6 @@ > > >>>rockchipdrm-y := rockchip_drm_drv.o rockchip_drm_fb.o \ > > >>> rockchip_drm_gem.o rockchip_drm_vop.o rockchip_vop_reg.o > > >>> -rockchipdrm-$(CONFIG_DRM_FBDEV_EMULATION) += rockchip_drm_fbdev.o > > >>>rockchipdrm-$(CONFIG_ROCKCHIP_ANALOGIX_DP) += analogix_dp-rockchip.o > > >>>rockchipdrm-$(CONFIG_ROCKCHIP_CDN_DP) += cdn-dp-core.o cdn-dp-reg.o > > >>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > >>> b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > >>> index 69c699459dce..20d81ae69828 100644 > > >>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > >>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c > > >>> @@ -26,7 +26,6 @@ > > >>>#include "rockchip_drm_drv.h" > > >>>#include "rockchip_drm_fb.h" > > >>> -#include "rockchip_drm_fbdev.h" > > >>>#include "rockchip_drm_gem.h" > > >>>#define DRIVER_NAME "rockchip" > > >>> @@ -159,10 +158,6 @@ static int rockchip_drm_bind(struct device *dev) > > >>> drm_mode_config_reset(drm_dev); > > >>> - ret = rockchip_drm_fbdev_init(drm_dev); > > >>> - if (ret) > > >>> - goto err_unbind_all; > > >>> - > > >>> /* init kms poll for handling hpd */ > > >>> drm_kms_helper_poll_init(drm_dev); > > >>> @@ -170,10 +165,11 @@ static int rockchip_drm_bind(struct device *dev) > > >>> if (ret) > > >>> goto err_kms_helper_poll_fini; > > >>> + drm_fbdev_generic_setup(drm_dev, 32); > > >> > > >> Please pass 0 for the final argument. The fbdev helpers pick 32 by > > >> default. > > >> Maybe leave a comment if you require 32 here. > > > > > > I wanted to minimise the changes introduced here - passing 32 matches > > > the value passed to drm_fb_helper_initial_config() in the deleted code > > > from rockchip_drm_fbdev.c. > > > > In that case > > > > Acked-by: Thomas Zimmermann > > Thanks! Are you able to pick this up (and [1])? Otherwise what is needed here and who should pick this up? [1] https://lore.kernel.org/dri-devel/20211101114622.813536-1-j...@metanate.com/
Re: [PATCH] drm/syncobj: Deal with signalled fences in transfer.
On Tue, Dec 7, 2021 at 12:28 PM Lionel Landwerlin wrote: > > On 07/12/2021 13:00, Christian König wrote: > > Am 07.12.21 um 11:40 schrieb Bas Nieuwenhuizen: > >> On Tue, Dec 7, 2021 at 8:21 AM Christian König > >> wrote: > >>> Am 07.12.21 um 08:10 schrieb Lionel Landwerlin: > On 07/12/2021 03:32, Bas Nieuwenhuizen wrote: > > See the comments in the code. Basically if the seqno is already > > signalled then we get a NULL fence. If we then put the NULL fence > > in a binary syncobj it counts as unsignalled, making that syncobj > > pretty much useless for all expected uses. > > > > Not 100% sure about the transfer to a timeline syncobj but I > > believe it is needed there too, as AFAICT the add_point function > > assumes the fence isn't NULL. > > > > Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between > > binary and timeline v2") > > Cc: sta...@vger.kernel.org > > Signed-off-by: Bas Nieuwenhuizen > > --- > >drivers/gpu/drm/drm_syncobj.c | 26 ++ > >1 file changed, 26 insertions(+) > > > > diff --git a/drivers/gpu/drm/drm_syncobj.c > > b/drivers/gpu/drm/drm_syncobj.c > > index fdd2ec87cdd1..eb28a40400d2 100644 > > --- a/drivers/gpu/drm/drm_syncobj.c > > +++ b/drivers/gpu/drm/drm_syncobj.c > > @@ -861,6 +861,19 @@ static int > > drm_syncobj_transfer_to_timeline(struct drm_file *file_private, > > &fence); > >if (ret) > >goto err; > > + > > +/* If the requested seqno is already signaled > > drm_syncobj_find_fence may > > + * return a NULL fence. To make sure the recipient gets > > signalled, use > > + * a new fence instead. > > + */ > > +if (!fence) { > > +fence = dma_fence_allocate_private_stub(); > > +if (!fence) { > > +ret = -ENOMEM; > > +goto err; > > +} > > +} > > + > > Shouldn't we fix drm_syncobj_find_fence() instead? > >>> Mhm, now that you mention it. Bas, why do you think that > >>> dma_fence_chain_find_seqno() may return NULL when the fence is already > >>> signaled? > >>> > >>> Double checking the code that should never ever happen. > >> Well, I tested the patch with > >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fmerge_requests%2F14097%2Fdiffs%3Fcommit_id%3Dd4c5c840f4e3839f9f5c1747a9034eb2b565f5c0&data=04%7C01%7Cchristian.koenig%40amd.com%7Cc1ab29fc100842826f5d08d9b96e102a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637744705383763833%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=sXkTJWm%2FWm2xwgLGdepVWAOlqj%2FeArnvmMvnJpQ9YEs%3D&reserved=0 > >> > >> so I'm pretty sure it happens, and this patch fixes it, though I may > >> have misidentified what the code should do. > >> > >> My reading is that the dma_fence_chain_for_each in > >> dma_fence_chain_find_seqno will never visit a signalled fence (unless > >> the top one is signalled), as dma_fence_chain_walk will never return a > >> signalled fence (it only returns on NULL or !signalled). > > > > Ah, yes that suddenly makes more sense. > > > >> Happy to move this to drm_syncobj_find_fence. > > > > No, I think that your current patch is fine. > > > > That drm_syncobj_find_fence() only returns NULL when it can't find > > anything !signaled is correct behavior I think. > > > We should probably update the docs then : > > > * Returns 0 on success or a negative error value on failure. On > success @fence > * contains a reference to the fence, which must be released by calling > * dma_fence_put(). > > > Looking at some of the kernel drivers, it looks like they don't all > protect themselves against NULL pointers : > > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vc4/vc4_gem.c#L1195 > > https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c#L1020 amdgpu handles it here (amdgpu_sync_fence checks for a NULL fence). But yeah I think it is a bit treacherous, especially as this only occurs with timeline semaphores. > > > -Lionel > > > > > > Going to push your original patch if nobody has any more objections. > > > > But somebody might want to take care of the IGT as well. > > > > Regards, > > Christian. > > > >>> Regards, > >>> Christian. > >>> > By returning a stub fence for the timeline case if there isn't one. > > > Because the same NULL fence check appears missing in amdgpu (and > probably other drivers). > > > Also we should have tests for this in IGT. > > AMD contributed some tests when this code was written but they never > got reviewed :( > > > -Lionel > > > >chain = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL); > >if (!chain) {
Re: [PATCH] drm/syncobj: Deal with signalled fences in transfer.
On 07/12/2021 13:00, Christian König wrote: Am 07.12.21 um 11:40 schrieb Bas Nieuwenhuizen: On Tue, Dec 7, 2021 at 8:21 AM Christian König wrote: Am 07.12.21 um 08:10 schrieb Lionel Landwerlin: On 07/12/2021 03:32, Bas Nieuwenhuizen wrote: See the comments in the code. Basically if the seqno is already signalled then we get a NULL fence. If we then put the NULL fence in a binary syncobj it counts as unsignalled, making that syncobj pretty much useless for all expected uses. Not 100% sure about the transfer to a timeline syncobj but I believe it is needed there too, as AFAICT the add_point function assumes the fence isn't NULL. Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between binary and timeline v2") Cc: sta...@vger.kernel.org Signed-off-by: Bas Nieuwenhuizen --- drivers/gpu/drm/drm_syncobj.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index fdd2ec87cdd1..eb28a40400d2 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -861,6 +861,19 @@ static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private, &fence); if (ret) goto err; + + /* If the requested seqno is already signaled drm_syncobj_find_fence may + * return a NULL fence. To make sure the recipient gets signalled, use + * a new fence instead. + */ + if (!fence) { + fence = dma_fence_allocate_private_stub(); + if (!fence) { + ret = -ENOMEM; + goto err; + } + } + Shouldn't we fix drm_syncobj_find_fence() instead? Mhm, now that you mention it. Bas, why do you think that dma_fence_chain_find_seqno() may return NULL when the fence is already signaled? Double checking the code that should never ever happen. Well, I tested the patch with https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fmerge_requests%2F14097%2Fdiffs%3Fcommit_id%3Dd4c5c840f4e3839f9f5c1747a9034eb2b565f5c0&data=04%7C01%7Cchristian.koenig%40amd.com%7Cc1ab29fc100842826f5d08d9b96e102a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637744705383763833%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=sXkTJWm%2FWm2xwgLGdepVWAOlqj%2FeArnvmMvnJpQ9YEs%3D&reserved=0 so I'm pretty sure it happens, and this patch fixes it, though I may have misidentified what the code should do. My reading is that the dma_fence_chain_for_each in dma_fence_chain_find_seqno will never visit a signalled fence (unless the top one is signalled), as dma_fence_chain_walk will never return a signalled fence (it only returns on NULL or !signalled). Ah, yes that suddenly makes more sense. Happy to move this to drm_syncobj_find_fence. No, I think that your current patch is fine. That drm_syncobj_find_fence() only returns NULL when it can't find anything !signaled is correct behavior I think. We should probably update the docs then : * Returns 0 on success or a negative error value on failure. On success @fence * contains a reference to the fence, which must be released by calling * dma_fence_put(). Looking at some of the kernel drivers, it looks like they don't all protect themselves against NULL pointers : https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/vc4/vc4_gem.c#L1195 https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c#L1020 -Lionel Going to push your original patch if nobody has any more objections. But somebody might want to take care of the IGT as well. Regards, Christian. Regards, Christian. By returning a stub fence for the timeline case if there isn't one. Because the same NULL fence check appears missing in amdgpu (and probably other drivers). Also we should have tests for this in IGT. AMD contributed some tests when this code was written but they never got reviewed :( -Lionel chain = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL); if (!chain) { ret = -ENOMEM; @@ -890,6 +903,19 @@ drm_syncobj_transfer_to_binary(struct drm_file *file_private, args->src_point, args->flags, &fence); if (ret) goto err; + + /* If the requested seqno is already signaled drm_syncobj_find_fence may + * return a NULL fence. To make sure the recipient gets signalled, use + * a new fence instead. + */ + if (!fence) { + fence = dma_fence_allocate_private_stub(); + if (!fence) { + ret = -ENOMEM; + goto err; + } + } + drm_syncobj_replace_fence(binary_syncobj, fence); dma_fence_put(fence); err:
Re: [Intel-gfx] [PATCH v2 07/16] drm/i915: Take trylock during eviction, v2.
On Mon, 29 Nov 2021 at 13:57, Maarten Lankhorst wrote: > > Now that freeing objects takes the object lock when destroying the > backing pages, we can confidently take the object lock even for dead > objects. That looks to be a future patch, at least with non-TTM backend? Does something need to be re-ordered in the series? > > Use this fact to take the object lock in the shrinker, without requiring > a reference to the object, so all calls to unbind take the object lock. Hmm, the previous patch was talking about "we don't evict when refcount = 0", but this looks to be doing something else? > > This is the last step to requiring the object lock for vma_unbind. > > Changes since v1: > - No longer require the refcount, as every freed object now holds the lock > when unbinding VMA's. > > Signed-off-by: Maarten Lankhorst > --- > drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 6 > drivers/gpu/drm/i915/i915_gem_evict.c| 34 +--- > 2 files changed, 35 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > index eebff4735781..ad2123369e0d 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > @@ -405,12 +405,18 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, > unsigned long event, void *ptr > list_for_each_entry_safe(vma, next, > &i915->ggtt.vm.bound_list, vm_link) { > unsigned long count = vma->node.size >> PAGE_SHIFT; > + struct drm_i915_gem_object *obj = vma->obj; > > if (!vma->iomap || i915_vma_is_active(vma)) > continue; > > + if (!i915_gem_object_trylock(obj)) > + continue; > + > if (__i915_vma_unbind(vma) == 0) > freed_pages += count; > + > + i915_gem_object_unlock(obj); > } > mutex_unlock(&i915->ggtt.vm.mutex); > > diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c > b/drivers/gpu/drm/i915/i915_gem_evict.c > index 2b73ddb11c66..286efa462eca 100644 > --- a/drivers/gpu/drm/i915/i915_gem_evict.c > +++ b/drivers/gpu/drm/i915/i915_gem_evict.c > @@ -58,6 +58,9 @@ mark_free(struct drm_mm_scan *scan, > if (i915_vma_is_pinned(vma)) > return false; > > + if (!i915_gem_object_trylock(vma->obj)) > + return false; > + > list_add(&vma->evict_link, unwind); > return drm_mm_scan_add_block(scan, &vma->node); > } > @@ -178,6 +181,7 @@ i915_gem_evict_something(struct i915_address_space *vm, > list_for_each_entry_safe(vma, next, &eviction_list, evict_link) { > ret = drm_mm_scan_remove_block(&scan, &vma->node); > BUG_ON(ret); > + i915_gem_object_unlock(vma->obj); > } > > /* > @@ -222,10 +226,12 @@ i915_gem_evict_something(struct i915_address_space *vm, > * of any of our objects, thus corrupting the list). > */ > list_for_each_entry_safe(vma, next, &eviction_list, evict_link) { > - if (drm_mm_scan_remove_block(&scan, &vma->node)) > + if (drm_mm_scan_remove_block(&scan, &vma->node)) { > __i915_vma_pin(vma); > - else > + } else { > list_del(&vma->evict_link); > + i915_gem_object_unlock(vma->obj); > + } > } > > /* Unbinding will emit any required flushes */ > @@ -234,16 +240,22 @@ i915_gem_evict_something(struct i915_address_space *vm, > __i915_vma_unpin(vma); > if (ret == 0) > ret = __i915_vma_unbind(vma); > + > + i915_gem_object_unlock(vma->obj); > } > > while (ret == 0 && (node = drm_mm_scan_color_evict(&scan))) { > vma = container_of(node, struct i915_vma, node); > > + > /* If we find any non-objects (!vma), we cannot evict them */ > - if (vma->node.color != I915_COLOR_UNEVICTABLE) > + if (vma->node.color != I915_COLOR_UNEVICTABLE && > + i915_gem_object_trylock(vma->obj)) { > ret = __i915_vma_unbind(vma); > - else > - ret = -ENOSPC; /* XXX search failed, try again? */ > + i915_gem_object_unlock(vma->obj); > + } else { > + ret = -ENOSPC; > + } > } > > return ret; > @@ -333,6 +345,11 @@ int i915_gem_evict_for_node(struct i915_address_space > *vm, > break; > } > > + if (!i915_gem_object_trylock(vma->obj)) { > + ret = -ENOSPC; > + break; > + } > + > /* > * Never show fear
Re: [PATCH] drm/syncobj: Deal with signalled fences in transfer.
Am 07.12.21 um 11:40 schrieb Bas Nieuwenhuizen: On Tue, Dec 7, 2021 at 8:21 AM Christian König wrote: Am 07.12.21 um 08:10 schrieb Lionel Landwerlin: On 07/12/2021 03:32, Bas Nieuwenhuizen wrote: See the comments in the code. Basically if the seqno is already signalled then we get a NULL fence. If we then put the NULL fence in a binary syncobj it counts as unsignalled, making that syncobj pretty much useless for all expected uses. Not 100% sure about the transfer to a timeline syncobj but I believe it is needed there too, as AFAICT the add_point function assumes the fence isn't NULL. Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between binary and timeline v2") Cc: sta...@vger.kernel.org Signed-off-by: Bas Nieuwenhuizen --- drivers/gpu/drm/drm_syncobj.c | 26 ++ 1 file changed, 26 insertions(+) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index fdd2ec87cdd1..eb28a40400d2 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -861,6 +861,19 @@ static int drm_syncobj_transfer_to_timeline(struct drm_file *file_private, &fence); if (ret) goto err; + +/* If the requested seqno is already signaled drm_syncobj_find_fence may + * return a NULL fence. To make sure the recipient gets signalled, use + * a new fence instead. + */ +if (!fence) { +fence = dma_fence_allocate_private_stub(); +if (!fence) { +ret = -ENOMEM; +goto err; +} +} + Shouldn't we fix drm_syncobj_find_fence() instead? Mhm, now that you mention it. Bas, why do you think that dma_fence_chain_find_seqno() may return NULL when the fence is already signaled? Double checking the code that should never ever happen. Well, I tested the patch with https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fmesa%2Fmesa%2F-%2Fmerge_requests%2F14097%2Fdiffs%3Fcommit_id%3Dd4c5c840f4e3839f9f5c1747a9034eb2b565f5c0&data=04%7C01%7Cchristian.koenig%40amd.com%7Cc1ab29fc100842826f5d08d9b96e102a%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637744705383763833%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=sXkTJWm%2FWm2xwgLGdepVWAOlqj%2FeArnvmMvnJpQ9YEs%3D&reserved=0 so I'm pretty sure it happens, and this patch fixes it, though I may have misidentified what the code should do. My reading is that the dma_fence_chain_for_each in dma_fence_chain_find_seqno will never visit a signalled fence (unless the top one is signalled), as dma_fence_chain_walk will never return a signalled fence (it only returns on NULL or !signalled). Ah, yes that suddenly makes more sense. Happy to move this to drm_syncobj_find_fence. No, I think that your current patch is fine. That drm_syncobj_find_fence() only returns NULL when it can't find anything !signaled is correct behavior I think. Going to push your original patch if nobody has any more objections. But somebody might want to take care of the IGT as well. Regards, Christian. Regards, Christian. By returning a stub fence for the timeline case if there isn't one. Because the same NULL fence check appears missing in amdgpu (and probably other drivers). Also we should have tests for this in IGT. AMD contributed some tests when this code was written but they never got reviewed :( -Lionel chain = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL); if (!chain) { ret = -ENOMEM; @@ -890,6 +903,19 @@ drm_syncobj_transfer_to_binary(struct drm_file *file_private, args->src_point, args->flags, &fence); if (ret) goto err; + +/* If the requested seqno is already signaled drm_syncobj_find_fence may + * return a NULL fence. To make sure the recipient gets signalled, use + * a new fence instead. + */ +if (!fence) { +fence = dma_fence_allocate_private_stub(); +if (!fence) { +ret = -ENOMEM; +goto err; +} +} + drm_syncobj_replace_fence(binary_syncobj, fence); dma_fence_put(fence); err:
Re: [PATCH] fbdev: savagefb: make a variable local
Hi Dan, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.16-rc4 next-20211207] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Dan-Carpenter/fbdev-savagefb-make-a-variable-local/20211203-175849 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 5f58da2befa58edf3a70b91ed87ed9bf77f1e70e config: mips-buildonly-randconfig-r004-20211207 (https://download.01.org/0day-ci/archive/20211207/202112071822.ry2xayan-...@intel.com/config) compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project f50be8eb0a12a61d23db6cda452c693001d76898) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install mips cross compiling tool for clang build # apt-get install binutils-mips-linux-gnu # https://github.com/0day-ci/linux/commit/9ea7012b220fc1bd8aa2f0a65b97403cea046343 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Dan-Carpenter/fbdev-savagefb-make-a-variable-local/20211203-175849 git checkout 9ea7012b220fc1bd8aa2f0a65b97403cea046343 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash drivers/video/fbdev/savage/ If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): >> drivers/video/fbdev/savage/savagefb_driver.c:2173:17: warning: unused >> variable 'edid' unsigned char ^ fatal error: error in backend: Nested variants found in inline asm string: '.if ( 0x00 ) != -1)) 0x00 ) != -1)) : ($( static struct ftrace_branch_data __attribute__((__aligned__(4))) __attribute__((__section__("_ftrace_branch"))) __if_trace = $( .func = __func__, .file = "arch/mips/include/asm/barrier.h", .line = 16, $); 0x00 ) != -1)) : $))) ) && ( (1 << 0) ); .set push; .set mips64r2; .rept 1; sync 0x00; .endr; .set pop; .else; ; .endif' PLEASE submit a bug report to https://bugs.llvm.org/ and include the crash backtrace, preprocessed source, and associated run script. Stack dump: 0. Program arguments: clang -Wp,-MMD,drivers/video/fbdev/savage/.savagefb_driver.o.d -nostdinc -Iarch/mips/include -I./arch/mips/include/generated -Iinclude -I./include -Iarch/mips/include/uapi -I./arch/mips/include/generated/uapi -Iinclude/uapi -I./include/generated/uapi -include include/linux/compiler-version.h -include include/linux/kconfig.h -include include/linux/compiler_types.h -D__KERNEL__ -DVMLINUX_LOAD_ADDRESS=0x8010 -DLINKER_LOAD_ADDRESS=0x8010 -DDATAOFFSET=0 -Qunused-arguments -fmacro-prefix-map== -DKBUILD_EXTRA_WARN1 -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Werror=return-type -Wno-format-security -std=gnu89 --target=mips64el-linux -fintegrated-as -Werror=unknown-warning-option -Werror=ignored-optimization-argument -mabi=64 -G 0 -mno-abicalls -fno-pic -pipe -msoft-float -DGAS_HAS_SET_HARDFLOAT -Wa,-msoft-float -ffreestanding -EL -fno-stack-check -march=mips64 -Wa,--trap -DTOOLCHAIN_SUPPORTS_VIRT -Iarch/mips/include/asm/mach-malta -Iarch/mips/include/asm/mach-generic -fno-asynchronous-unwind-tables -fno-delete-null-pointer-checks -Wno-frame-address -Wno-address-of-packed-member -O2 -Wframe-larger-than=2048 -fstack-protector -Wimplicit-fallthrough -Wno-gnu -mno-global-merge -Wno-unused-but-set-variable -Wno-unused-const-variable -ftrivial-auto-var-init=zero -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang -fno-stack-clash-protection -pg -falign-functions=64 -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-array-bounds -fno-strict-overflow -fno-stack-check -Werror=date-time -Werror=incompatible-pointer-types -Wextra -Wunused -Wno-unused-parameter -Wmissing-declarations -Wmissing-format-attribute -Wmissing-prototypes -Wold-style-definition -Wmissing-include-dirs -Wunused-but-set-variable -Wunused-const-variable -Wno-missing-field-initializers -Wno-sign-compare -Wno-type-limits -I drivers/video/fbdev/savage -I ./drivers/video/fbdev/savage -ffunction-sections -fdata-sections -DKBUILD_MODFILE="drivers/video/fbdev/savage/savagefb" -DKBUILD_BASENAME="savagefb_driver" -DKBUILD_MODNAME="savagefb" -D__KBUILD_MODNAME=kmod_savagefb -c -o drivers/video/fbdev/savage/savagefb_driver.o drivers/video/fbd
Re: [Intel-gfx] [PATCH v2 03/16] drm/i915: Remove pages_mutex and intel_gtt->vma_ops.set/clear_pages members, v2.
On Tue, 7 Dec 2021 at 10:06, Maarten Lankhorst wrote: > > On 06-12-2021 18:10, Matthew Auld wrote: > > On Mon, 29 Nov 2021 at 13:57, Maarten Lankhorst > > wrote: > >> Big delta, but boils down to moving set_pages to i915_vma.c, and removing > >> the special handling, all callers use the defaults anyway. We only remap > >> in ggtt, so default case will fall through. > >> > >> Because we still don't require locking in i915_vma_unpin(), handle this by > >> using xchg in get_pages(), as it's locked with obj->mutex, and cmpxchg in > >> unpin, which only fails if we race a against a new pin. > >> > >> Changes since v1: > >> - aliasing gtt sets ZERO_SIZE_PTR, not -ENODEV, remove special case > >> from __i915_vma_get_pages(). (Matt) > >> > >> Signed-off-by: Maarten Lankhorst > > > > > >> +static int > >> +__i915_vma_get_pages(struct i915_vma *vma) > >> +{ > >> + struct sg_table *pages; > >> + int ret; > >> + > >> + /* > >> +* The vma->pages are only valid within the lifespan of the > >> borrowed > >> +* obj->mm.pages. When the obj->mm.pages sg_table is regenerated, > >> so > >> +* must be the vma->pages. A simple rule is that vma->pages must > >> only > >> +* be accessed when the obj->mm.pages are pinned. > >> +*/ > >> + GEM_BUG_ON(!i915_gem_object_has_pinned_pages(vma->obj)); > >> + > >> + switch (vma->ggtt_view.type) { > >> + default: > >> + GEM_BUG_ON(vma->ggtt_view.type); > >> + fallthrough; > >> + case I915_GGTT_VIEW_NORMAL: > >> + pages = vma->obj->mm.pages; > >> + break; > >> + > >> + case I915_GGTT_VIEW_ROTATED: > >> + pages = > >> + intel_rotate_pages(&vma->ggtt_view.rotated, > >> vma->obj); > >> + break; > >> + > >> + case I915_GGTT_VIEW_REMAPPED: > >> + pages = > >> + intel_remap_pages(&vma->ggtt_view.remapped, > >> vma->obj); > >> + break; > >> + > >> + case I915_GGTT_VIEW_PARTIAL: > >> + pages = intel_partial_pages(&vma->ggtt_view, vma->obj); > >> + break; > >> + } > >> + > >> + ret = 0; > >> + if (IS_ERR(pages)) { > >> + ret = PTR_ERR(pages); > >> + pages = NULL; > >> + drm_err(&vma->vm->i915->drm, > >> + "Failed to get pages for VMA view type %u (%d)!\n", > >> + vma->ggtt_view.type, ret); > >> + } > >> + > >> + pages = xchg(&vma->pages, pages); > >> + > >> + /* did we race against a put_pages? */ > >> + if (pages && pages != vma->obj->mm.pages) { > >> + sg_free_table(vma->pages); > >> + kfree(vma->pages); > > So should this one rather be: > > > > sg_free_table(pages); > > kfree(pages); > > > > Or am I missing something? > > Correct! I missed it. Will fix it up when committing, or if a new version is > needed. > r-b with that.
Re: [PATCH v2 06/16] drm/i915: Ensure gem_contexts selftests work with unbind changes.
On Mon, 29 Nov 2021 at 13:57, Maarten Lankhorst wrote: > > In the next commit, we don't evict when refcount = 0. > > igt_vm_isolation() continuously tries to pin/unpin at same address, > but also calls put() on the object, which means the object may not > be unpinned in time. > > Instead of this, re-use the same object over and over, so they can > be unbound as required. > > Signed-off-by: Maarten Lankhorst Is this something to be worried about in the real world, outside of the selftests? > --- > .../drm/i915/gem/selftests/i915_gem_context.c | 54 +++ > 1 file changed, 32 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > index b32f7fed2d9c..3fc595b57cf4 100644 > --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_context.c > @@ -1481,10 +1481,10 @@ static int check_scratch(struct i915_address_space > *vm, u64 offset) > > static int write_to_scratch(struct i915_gem_context *ctx, > struct intel_engine_cs *engine, > + struct drm_i915_gem_object *obj, > u64 offset, u32 value) > { > struct drm_i915_private *i915 = ctx->i915; > - struct drm_i915_gem_object *obj; > struct i915_address_space *vm; > struct i915_request *rq; > struct i915_vma *vma; > @@ -1497,15 +1497,9 @@ static int write_to_scratch(struct i915_gem_context > *ctx, > if (err) > return err; > > - obj = i915_gem_object_create_internal(i915, PAGE_SIZE); > - if (IS_ERR(obj)) > - return PTR_ERR(obj); > - > cmd = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); > - if (IS_ERR(cmd)) { > - err = PTR_ERR(cmd); > - goto out; > - } > + if (IS_ERR(cmd)) > + return PTR_ERR(cmd); > > *cmd++ = MI_STORE_DWORD_IMM_GEN4; > if (GRAPHICS_VER(i915) >= 8) { > @@ -1569,17 +1563,19 @@ static int write_to_scratch(struct i915_gem_context > *ctx, > i915_vma_unpin(vma); > out_vm: > i915_vm_put(vm); > -out: > - i915_gem_object_put(obj); > + > + if (!err) > + err = i915_gem_object_wait(obj, 0, MAX_SCHEDULE_TIMEOUT); > + > return err; > } > > static int read_from_scratch(struct i915_gem_context *ctx, > struct intel_engine_cs *engine, > +struct drm_i915_gem_object *obj, > u64 offset, u32 *value) > { > struct drm_i915_private *i915 = ctx->i915; > - struct drm_i915_gem_object *obj; > struct i915_address_space *vm; > const u32 result = 0x100; > struct i915_request *rq; > @@ -1594,10 +1590,6 @@ static int read_from_scratch(struct i915_gem_context > *ctx, > if (err) > return err; > > - obj = i915_gem_object_create_internal(i915, PAGE_SIZE); > - if (IS_ERR(obj)) > - return PTR_ERR(obj); > - > if (GRAPHICS_VER(i915) >= 8) { > const u32 GPR0 = engine->mmio_base + 0x600; > > @@ -1615,7 +1607,7 @@ static int read_from_scratch(struct i915_gem_context > *ctx, > cmd = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); > if (IS_ERR(cmd)) { > err = PTR_ERR(cmd); > - goto out; > + goto err_unpin; > } > > memset(cmd, POISON_INUSE, PAGE_SIZE); > @@ -1651,7 +1643,7 @@ static int read_from_scratch(struct i915_gem_context > *ctx, > cmd = i915_gem_object_pin_map_unlocked(obj, I915_MAP_WB); > if (IS_ERR(cmd)) { > err = PTR_ERR(cmd); > - goto out; > + goto err_unpin; > } > > memset(cmd, POISON_INUSE, PAGE_SIZE); > @@ -1722,8 +1714,10 @@ static int read_from_scratch(struct i915_gem_context > *ctx, > i915_vma_unpin(vma); > out_vm: > i915_vm_put(vm); > -out: > - i915_gem_object_put(obj); > + > + if (!err) > + err = i915_gem_object_wait(obj, 0, MAX_SCHEDULE_TIMEOUT); > + > return err; > } > > @@ -1765,6 +1759,7 @@ static int igt_vm_isolation(void *arg) > u64 vm_total; > u32 expected; > int err; > + struct drm_i915_gem_object *obj_a, *obj_b; Nit: Christmas tree-ish > > if (GRAPHICS_VER(i915) < 7) > return 0; > @@ -1810,6 +1805,18 @@ static int igt_vm_isolation(void *arg) > vm_total = ctx_a->vm->total; > GEM_BUG_ON(ctx_b->vm->total != vm_total); > > + obj_a = i915_gem_object_create_internal(i915, PAGE_SIZE); > + if (IS_ERR(obj_a)) { > + err = PTR_ERR(obj_a); > + goto out_file; > + } >
Re: [PATCH] drm/syncobj: Deal with signalled fences in transfer.
On Tue, Dec 7, 2021 at 8:21 AM Christian König wrote: > > Am 07.12.21 um 08:10 schrieb Lionel Landwerlin: > > On 07/12/2021 03:32, Bas Nieuwenhuizen wrote: > >> See the comments in the code. Basically if the seqno is already > >> signalled then we get a NULL fence. If we then put the NULL fence > >> in a binary syncobj it counts as unsignalled, making that syncobj > >> pretty much useless for all expected uses. > >> > >> Not 100% sure about the transfer to a timeline syncobj but I > >> believe it is needed there too, as AFAICT the add_point function > >> assumes the fence isn't NULL. > >> > >> Fixes: ea569910cbab ("drm/syncobj: add transition iotcls between > >> binary and timeline v2") > >> Cc: sta...@vger.kernel.org > >> Signed-off-by: Bas Nieuwenhuizen > >> --- > >> drivers/gpu/drm/drm_syncobj.c | 26 ++ > >> 1 file changed, 26 insertions(+) > >> > >> diff --git a/drivers/gpu/drm/drm_syncobj.c > >> b/drivers/gpu/drm/drm_syncobj.c > >> index fdd2ec87cdd1..eb28a40400d2 100644 > >> --- a/drivers/gpu/drm/drm_syncobj.c > >> +++ b/drivers/gpu/drm/drm_syncobj.c > >> @@ -861,6 +861,19 @@ static int > >> drm_syncobj_transfer_to_timeline(struct drm_file *file_private, > >>&fence); > >> if (ret) > >> goto err; > >> + > >> +/* If the requested seqno is already signaled > >> drm_syncobj_find_fence may > >> + * return a NULL fence. To make sure the recipient gets > >> signalled, use > >> + * a new fence instead. > >> + */ > >> +if (!fence) { > >> +fence = dma_fence_allocate_private_stub(); > >> +if (!fence) { > >> +ret = -ENOMEM; > >> +goto err; > >> +} > >> +} > >> + > > > > > > Shouldn't we fix drm_syncobj_find_fence() instead? > > Mhm, now that you mention it. Bas, why do you think that > dma_fence_chain_find_seqno() may return NULL when the fence is already > signaled? > > Double checking the code that should never ever happen. Well, I tested the patch with https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/14097/diffs?commit_id=d4c5c840f4e3839f9f5c1747a9034eb2b565f5c0 so I'm pretty sure it happens, and this patch fixes it, though I may have misidentified what the code should do. My reading is that the dma_fence_chain_for_each in dma_fence_chain_find_seqno will never visit a signalled fence (unless the top one is signalled), as dma_fence_chain_walk will never return a signalled fence (it only returns on NULL or !signalled). Happy to move this to drm_syncobj_find_fence. > > Regards, > Christian. > > > > > By returning a stub fence for the timeline case if there isn't one. > > > > > > Because the same NULL fence check appears missing in amdgpu (and > > probably other drivers). > > > > > > Also we should have tests for this in IGT. > > > > AMD contributed some tests when this code was written but they never > > got reviewed :( > > > > > > -Lionel > > > > > >> chain = kzalloc(sizeof(struct dma_fence_chain), GFP_KERNEL); > >> if (!chain) { > >> ret = -ENOMEM; > >> @@ -890,6 +903,19 @@ drm_syncobj_transfer_to_binary(struct drm_file > >> *file_private, > >>args->src_point, args->flags, &fence); > >> if (ret) > >> goto err; > >> + > >> +/* If the requested seqno is already signaled > >> drm_syncobj_find_fence may > >> + * return a NULL fence. To make sure the recipient gets > >> signalled, use > >> + * a new fence instead. > >> + */ > >> +if (!fence) { > >> +fence = dma_fence_allocate_private_stub(); > >> +if (!fence) { > >> +ret = -ENOMEM; > >> +goto err; > >> +} > >> +} > >> + > >> drm_syncobj_replace_fence(binary_syncobj, fence); > >> dma_fence_put(fence); > >> err: > > > > >
[PATCH v4 3/6] drm/aspeed: Update INTR_STS handling
From: tommy-huang Add interrupt clear register define for further chip support. Signed-off-by: tommy-huang --- drivers/gpu/drm/aspeed/aspeed_gfx.h | 1 + drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 6 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h index 96501152bafa..4e6a442c3886 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h @@ -12,6 +12,7 @@ struct aspeed_gfx { struct regmap *scu; u32 dac_reg; + u32 int_clr_reg; u32 vga_scratch_reg; u32 throd_val; u32 scan_line_max; diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index b53fee6f1c17..d4b56b3c7597 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -60,6 +60,7 @@ struct aspeed_gfx_config { u32 dac_reg;/* DAC register in SCU */ + u32 int_clear_reg; /* Interrupt clear register */ u32 vga_scratch_reg;/* VGA scratch register in SCU */ u32 throd_val; /* Default Threshold Seting */ u32 scan_line_max; /* Max memory size of one scan line */ @@ -67,6 +68,7 @@ struct aspeed_gfx_config { static const struct aspeed_gfx_config ast2400_config = { .dac_reg = 0x2c, + .int_clear_reg = 0x60, .vga_scratch_reg = 0x50, .throd_val = CRT_THROD_LOW(0x1e) | CRT_THROD_HIGH(0x12), .scan_line_max = 64, @@ -74,6 +76,7 @@ static const struct aspeed_gfx_config ast2400_config = { static const struct aspeed_gfx_config ast2500_config = { .dac_reg = 0x2c, + .int_clear_reg = 0x60, .vga_scratch_reg = 0x50, .throd_val = CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3c), .scan_line_max = 128, @@ -119,7 +122,7 @@ static irqreturn_t aspeed_gfx_irq_handler(int irq, void *data) if (reg & CRT_CTRL_VERTICAL_INTR_STS) { drm_crtc_handle_vblank(&priv->pipe.crtc); - writel(reg, priv->base + CRT_CTRL1); + writel(reg, priv->base + priv->int_clr_reg); return IRQ_HANDLED; } @@ -147,6 +150,7 @@ static int aspeed_gfx_load(struct drm_device *drm) config = match->data; priv->dac_reg = config->dac_reg; + priv->int_clr_reg = config->int_clear_reg; priv->vga_scratch_reg = config->vga_scratch_reg; priv->throd_val = config->throd_val; priv->scan_line_max = config->scan_line_max; -- 2.17.1
[PATCH v4 2/6] ARM: dts: aspeed: ast2600-evb: Enable GFX device
From: Joel Stanley Enable the GFX device with a framebuffer memory region. Signed-off-by: Joel Stanley Signed-off-by: tommy-huang --- arch/arm/boot/dts/aspeed-ast2600-evb.dts | 18 ++ 1 file changed, 18 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-ast2600-evb.dts b/arch/arm/boot/dts/aspeed-ast2600-evb.dts index b7eb552640cb..e223dad2abd0 100644 --- a/arch/arm/boot/dts/aspeed-ast2600-evb.dts +++ b/arch/arm/boot/dts/aspeed-ast2600-evb.dts @@ -23,6 +23,19 @@ reg = <0x8000 0x8000>; }; + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + gfx_memory: framebuffer { + size = <0x0100>; + alignment = <0x0100>; + compatible = "shared-dma-pool"; + reusable; + }; + }; + vcc_sdhci0: regulator-vcc-sdhci0 { compatible = "regulator-fixed"; regulator-name = "SDHCI0 Vcc"; @@ -300,3 +313,8 @@ vqmmc-supply = <&vccq_sdhci1>; clk-phase-sd-hs = <7>, <200>; }; + +&gfx { + status = "okay"; + memory-region = <&gfx_memory>; +}; -- 2.17.1
[PATCH v4 4/6] drm/aspeed: Add AST2600 chip support
From: tommy-huang Add AST2600 chip support and setting. Signed-off-by: tommy-huang --- drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 9 + 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index d4b56b3c7597..d10246b1d1c2 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -82,9 +82,18 @@ static const struct aspeed_gfx_config ast2500_config = { .scan_line_max = 128, }; +static const struct aspeed_gfx_config ast2600_config = { + .dac_reg = 0xc0, + .int_clear_reg = 0x68, + .vga_scratch_reg = 0x50, + .throd_val = CRT_THROD_LOW(0x50) | CRT_THROD_HIGH(0x70), + .scan_line_max = 128, +}; + static const struct of_device_id aspeed_gfx_match[] = { { .compatible = "aspeed,ast2400-gfx", .data = &ast2400_config }, { .compatible = "aspeed,ast2500-gfx", .data = &ast2500_config }, + { .compatible = "aspeed,ast2600-gfx", .data = &ast2600_config }, { }, }; MODULE_DEVICE_TABLE(of, aspeed_gfx_match); -- 2.17.1
[PATCH v4 6/6] arm:boot:dts:aspeed-g6 Add more gfx reset control
From: tommy-huang Add more gfx reset control for ast2600. Signed-off-by: tommy-huang --- arch/arm/boot/dts/aspeed-g6.dtsi | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi index e38c3742761b..b92b24609660 100644 --- a/arch/arm/boot/dts/aspeed-g6.dtsi +++ b/arch/arm/boot/dts/aspeed-g6.dtsi @@ -356,7 +356,9 @@ reg = <0x1e6e6000 0x1000>; reg-io-width = <4>; clocks = <&syscon ASPEED_CLK_GATE_D1CLK>; - resets = <&syscon ASPEED_RESET_GRAPHICS>; + resets = <&syscon ASPEED_RESET_CRT>, +<&syscon ASPEED_RESET_GRAPHICS>; + reset-names = "crt", "engine"; syscon = <&syscon>; status = "disabled"; interrupts = ; -- 2.17.1
[PATCH v4 5/6] drm/aspeed: Add reset and clock for AST2600
From: tommy-huang Add more reset and clock select code for AST2600. The gfx_flags parameter was added for chip caps idenified. Signed-off-by: tommy-huang --- drivers/gpu/drm/aspeed/aspeed_gfx.h | 16 +++- drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 16 drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 50 ++-- 3 files changed, 77 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx.h b/drivers/gpu/drm/aspeed/aspeed_gfx.h index 4e6a442c3886..2c733225d3c7 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx.h +++ b/drivers/gpu/drm/aspeed/aspeed_gfx.h @@ -8,7 +8,8 @@ struct aspeed_gfx { struct drm_device drm; void __iomem*base; struct clk *clk; - struct reset_control*rst; + struct reset_control*rst_crt; + struct reset_control*rst_engine; struct regmap *scu; u32 dac_reg; @@ -16,6 +17,7 @@ struct aspeed_gfx { u32 vga_scratch_reg; u32 throd_val; u32 scan_line_max; + u32 flags; struct drm_simple_display_pipe pipe; struct drm_connectorconnector; @@ -106,3 +108,15 @@ int aspeed_gfx_create_output(struct drm_device *drm); /* CRT_THROD */ #define CRT_THROD_LOW(x) (x) #define CRT_THROD_HIGH(x) ((x) << 8) + +/* SCU control */ +#define SCU_G6_CLK_COURCE 0x300 + +/* GFX FLAGS */ +#define RESET_MASK BIT(0) +#define RESET_G6 BIT(0) +#define CLK_MASK BIT(4) +#define CLK_G6 BIT(4) + +#define G6_CLK_MASK(BIT(8) | BIT(9) | BIT(10)) +#define G6_USB_40_CLK BIT(9) diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c index 827e62c1daba..e0975ecda92d 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c @@ -77,6 +77,18 @@ static void aspeed_gfx_disable_controller(struct aspeed_gfx *priv) regmap_update_bits(priv->scu, priv->dac_reg, BIT(16), 0); } +static void aspeed_gfx_set_clk(struct aspeed_gfx *priv) +{ + switch (priv->flags & CLK_MASK) { + case CLK_G6: + regmap_update_bits(priv->scu, SCU_G6_CLK_COURCE, G6_CLK_MASK, 0x0); + regmap_update_bits(priv->scu, SCU_G6_CLK_COURCE, G6_CLK_MASK, G6_USB_40_CLK); + break; + default: + break; + } +} + static void aspeed_gfx_crtc_mode_set_nofb(struct aspeed_gfx *priv) { struct drm_display_mode *m = &priv->pipe.crtc.state->adjusted_mode; @@ -87,6 +99,8 @@ static void aspeed_gfx_crtc_mode_set_nofb(struct aspeed_gfx *priv) if (err) return; + aspeed_gfx_set_clk(priv); + #if 0 /* TODO: we have only been able to test with the 40MHz USB clock. The * clock is fixed, so we cannot adjust it here. */ @@ -193,6 +207,7 @@ static void aspeed_gfx_pipe_update(struct drm_simple_display_pipe *pipe, static int aspeed_gfx_enable_vblank(struct drm_simple_display_pipe *pipe) { struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe); + u32 reg = readl(priv->base + CRT_CTRL1); /* Clear pending VBLANK IRQ */ @@ -207,6 +222,7 @@ static int aspeed_gfx_enable_vblank(struct drm_simple_display_pipe *pipe) static void aspeed_gfx_disable_vblank(struct drm_simple_display_pipe *pipe) { struct aspeed_gfx *priv = drm_pipe_to_aspeed_gfx(pipe); + u32 reg = readl(priv->base + CRT_CTRL1); reg &= ~CRT_CTRL_VERTICAL_INTR_EN; diff --git a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c index d10246b1d1c2..59a0de92650f 100644 --- a/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c +++ b/drivers/gpu/drm/aspeed/aspeed_gfx_drv.c @@ -64,6 +64,7 @@ struct aspeed_gfx_config { u32 vga_scratch_reg;/* VGA scratch register in SCU */ u32 throd_val; /* Default Threshold Seting */ u32 scan_line_max; /* Max memory size of one scan line */ + u32 gfx_flags; /* Flags for gfx chip caps */ }; static const struct aspeed_gfx_config ast2400_config = { @@ -72,6 +73,7 @@ static const struct aspeed_gfx_config ast2400_config = { .vga_scratch_reg = 0x50, .throd_val = CRT_THROD_LOW(0x1e) | CRT_THROD_HIGH(0x12), .scan_line_max = 64, + .gfx_flags = 0, }; static const struct aspeed_gfx_config ast2500_config = { @@ -80,6 +82,7 @@ static const struct aspeed_gfx_config ast2500_config = { .vga_scratch_reg = 0x50, .throd_val = CRT_THROD_LOW(0x24) | CRT_THROD_HIGH(0x3c), .scan_line_max = 128, + .gfx_flags = 0, }; static const struct aspeed_gfx_
[PATCH v4 1/1] arm:boot:dts:aspeed-g6 Add more gfx reset control
From: tommy-huang Add more gfx reset control for ast2600. Signed-off-by: tommy-huang --- arch/arm/boot/dts/aspeed-g6.dtsi | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi index a730c7706ecf..ae7a18b27701 100644 --- a/arch/arm/boot/dts/aspeed-g6.dtsi +++ b/arch/arm/boot/dts/aspeed-g6.dtsi @@ -356,7 +356,9 @@ reg = <0x1e6e6000 0x1000>; reg-io-width = <4>; clocks = <&syscon ASPEED_CLK_GATE_D1CLK>; - resets = <&syscon ASPEED_RESET_GRAPHICS>; + resets = <&syscon ASPEED_RESET_CRT>, +<&syscon ASPEED_RESET_GRAPHICS>; + reset-names = "crt", "engine"; syscon = <&syscon>; status = "disabled"; interrupts = ; -- 2.17.1
[PATCH v4 1/6] ARM: dts: aspeed: Add GFX node to AST2600
From: Joel Stanley The GFX device is present in the AST2600 SoC. Signed-off-by: Joel Stanley Signed-off-by: tommy-huang --- arch/arm/boot/dts/aspeed-g6.dtsi | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/arm/boot/dts/aspeed-g6.dtsi b/arch/arm/boot/dts/aspeed-g6.dtsi index 1b47be1704f8..e38c3742761b 100644 --- a/arch/arm/boot/dts/aspeed-g6.dtsi +++ b/arch/arm/boot/dts/aspeed-g6.dtsi @@ -351,6 +351,17 @@ quality = <100>; }; + gfx: display@1e6e6000 { + compatible = "aspeed,ast2600-gfx", "aspeed,ast2500-gfx", "syscon"; + reg = <0x1e6e6000 0x1000>; + reg-io-width = <4>; + clocks = <&syscon ASPEED_CLK_GATE_D1CLK>; + resets = <&syscon ASPEED_RESET_GRAPHICS>; + syscon = <&syscon>; + status = "disabled"; + interrupts = ; + }; + xdma: xdma@1e6e7000 { compatible = "aspeed,ast2600-xdma"; reg = <0x1e6e7000 0x100>; -- 2.17.1
[PATCH v4 0/6] Add Aspeed AST2600 soc display support
From: tommy-huang v4: Add necessary reset control for ast2600. Add chip caps for futher use. These code are test on AST2500 and AST2600 by below steps. 1. Add below config to turn VT and LOGO on. CONFIG_TTY=y CONFIG_VT=y CONFIG_CONSOLE_TRANSLATIONS=y CONFIG_VT_CONSOLE=y CONFIG_VT_CONSOLE_SLEEP=y CONFIG_HW_CONSOLE=y CONFIG_VT_HW_CONSOLE_BINDING=y CONFIG_UNIX98_PTYS=y CONFIG_LDISC_AUTOLOAD=y CONFIG_DEVMEM=y CONFIG_DUMMY_CONSOLE=y CONFIG_FRAMEBUFFER_CONSOLE=y CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y CONFIG_LOGO=y CONFIG_LOGO_LINUX_CLUT224=y 2. The Linux logo will be shown on the screen, when the BMC boot in Linux. v3: Refine the patch for clear separate purpose. Skip to send devicetree patch v2: Remove some unnecessary patch. Refine for reviwer request. v1: First add patch. Joel Stanley (2): ARM: dts: aspeed: Add GFX node to AST2600 ARM: dts: aspeed: ast2600-evb: Enable GFX device tommy-huang (4): drm/aspeed: Update INTR_STS handling drm/aspeed: Add AST2600 chip support drm/aspeed: Add reset and clock for AST2600 arm:boot:dts:aspeed-g6 Add more gfx reset control arch/arm/boot/dts/aspeed-ast2600-evb.dts | 18 +++ arch/arm/boot/dts/aspeed-g6.dtsi | 13 + drivers/gpu/drm/aspeed/aspeed_gfx.h | 17 ++- drivers/gpu/drm/aspeed/aspeed_gfx_crtc.c | 16 ++ drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 65 ++-- 5 files changed, 123 insertions(+), 6 deletions(-) -- 2.17.1
Re: [PATCH v2 2/3] drm/format-helper: Add drm_fb_xrgb8888_to_xrgb2101010_toio()
Am 07.12.21 um 11:20 schrieb Thomas Zimmermann: Hi Am 07.12.21 um 10:54 schrieb Hector Martin: Hi, thanks for the review! On 07/12/2021 18.40, Thomas Zimmermann wrote: Hi Am 07.12.21 um 08:29 schrieb Hector Martin: Add XRGB emulation support for devices that can only do XRGB2101010. This is chiefly useful for simpledrm on Apple devices where the bootloader-provided framebuffer is 10-bit. Signed-off-by: Hector Martin --- drivers/gpu/drm/drm_format_helper.c | 62 + include/drm/drm_format_helper.h | 3 ++ 2 files changed, 65 insertions(+) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index dbe3e830096e..edd611d3ab6a 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -409,6 +409,59 @@ void drm_fb_xrgb_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch, } EXPORT_SYMBOL(drm_fb_xrgb_to_rgb888_toio); +static void drm_fb_xrgb_to_xrgb2101010_line(u32 *dbuf, const u32 *sbuf, + unsigned int pixels) +{ + unsigned int x; + + for (x = 0; x < pixels; x++) { + *dbuf++ = ((sbuf[x] & 0x00FF) << 2) | + ((sbuf[x] & 0xFF00) << 4) | + ((sbuf[x] & 0x00FF) << 6); This isn't quite right. The lowest two destination bits in each component will always be zero. You have to do the shifting as above and for each component the two highest source bits have to be OR'ed into the two lowest destination bits. For example the source bits in a component are numbered 7 to 0 | 7 6 5 4 3 2 1 0 | then the destination bits should be | 7 6 5 4 3 2 1 0 7 6 | I think both approaches have pros and cons. Leaving the two LSBs always at 0 yields a fully linear transfer curve with no discontinuities, but means the maximum brightness is slightly less than full. Setting them fully maps the brightness range, but creates 4 double wide steps in the transfer curve (also it's potentially slightly slower CPU-wise). If you prefer the latter I'll do that for v2. We don't give guarantees for color output unless color spaces are involved. But the lack of LSB bits can be more visible than larger steps in the curve. With the current formats here, it's probably a non-issue. But there can be conversions, such as RGB444 to RGB88, where these missing LSBs make a visible difference. Therefore, please change the algorithm. It produces more consistent results over a variety of format conversion. It's better to have the same (default) algorithm for all of them. FTR, I just tested this in a painting program. I can see a difference between ff and fcfcfc iff both are next to each other. f8f8f8 is obviously gray. Best regards Thomas -- 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 v2 2/3] drm/format-helper: Add drm_fb_xrgb8888_to_xrgb2101010_toio()
Hi Am 07.12.21 um 10:54 schrieb Hector Martin: Hi, thanks for the review! On 07/12/2021 18.40, Thomas Zimmermann wrote: Hi Am 07.12.21 um 08:29 schrieb Hector Martin: Add XRGB emulation support for devices that can only do XRGB2101010. This is chiefly useful for simpledrm on Apple devices where the bootloader-provided framebuffer is 10-bit. Signed-off-by: Hector Martin --- drivers/gpu/drm/drm_format_helper.c | 62 + include/drm/drm_format_helper.h | 3 ++ 2 files changed, 65 insertions(+) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index dbe3e830096e..edd611d3ab6a 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -409,6 +409,59 @@ void drm_fb_xrgb_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch, } EXPORT_SYMBOL(drm_fb_xrgb_to_rgb888_toio); +static void drm_fb_xrgb_to_xrgb2101010_line(u32 *dbuf, const u32 *sbuf, + unsigned int pixels) +{ + unsigned int x; + + for (x = 0; x < pixels; x++) { + *dbuf++ = ((sbuf[x] & 0x00FF) << 2) | + ((sbuf[x] & 0xFF00) << 4) | + ((sbuf[x] & 0x00FF) << 6); This isn't quite right. The lowest two destination bits in each component will always be zero. You have to do the shifting as above and for each component the two highest source bits have to be OR'ed into the two lowest destination bits. For example the source bits in a component are numbered 7 to 0 | 7 6 5 4 3 2 1 0 | then the destination bits should be | 7 6 5 4 3 2 1 0 7 6 | I think both approaches have pros and cons. Leaving the two LSBs always at 0 yields a fully linear transfer curve with no discontinuities, but means the maximum brightness is slightly less than full. Setting them fully maps the brightness range, but creates 4 double wide steps in the transfer curve (also it's potentially slightly slower CPU-wise). If you prefer the latter I'll do that for v2. We don't give guarantees for color output unless color spaces are involved. But the lack of LSB bits can be more visible than larger steps in the curve. With the current formats here, it's probably a non-issue. But there can be conversions, such as RGB444 to RGB88, where these missing LSBs make a visible difference. Therefore, please change the algorithm. It produces more consistent results over a variety of format conversion. It's better to have the same (default) algorithm for all of them. Best regards Thomas -- 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 v2 0/6] drm/vc4: kms: Misc fixes for HVS commits
Maxime Ripard 於 2021年12月3日 週五 下午10:03寫道: > > On Mon, Nov 29, 2021 at 04:31:57PM +0800, Jian-Hong Pan wrote: > > Maxime Ripard 於 2021年11月26日 週五 下午11:45寫道: > > > > > > On Fri, Nov 19, 2021 at 06:24:34PM +0800, Jian-Hong Pan wrote: > > > > Maxime Ripard 於 2021年11月18日 週四 下午6:40寫道: > > > > > > > > > > On Thu, Nov 18, 2021 at 02:42:58PM +0800, Jian-Hong Pan wrote: > > > > > > Maxime Ripard 於 2021年11月17日 週三 下午5:45寫道: > > > > > > > The conversion to DRM commit helpers (f3c420fe19f8, "drm/vc4: > > > > > > > kms: Convert to > > > > > > > atomic helpers") introduced a number of issues in corner cases, > > > > > > > most of them > > > > > > > showing themselves in the form of either a vblank timeout or > > > > > > > use-after-free > > > > > > > error. > > > > > > > > > > > > > > These patches should fix most of them, some of them still being > > > > > > > debugged. > > > > > > > > > > > > > > Maxime > > > > > > > > > > > > > > Changes from v1: > > > > > > > - Prevent a null pointer dereference > > > > > > > > > > > > > > Maxime Ripard (6): > > > > > > > drm/vc4: kms: Wait for the commit before increasing our clock > > > > > > > rate > > > > > > > drm/vc4: kms: Fix return code check > > > > > > > drm/vc4: kms: Add missing drm_crtc_commit_put > > > > > > > drm/vc4: kms: Clear the HVS FIFO commit pointer once done > > > > > > > drm/vc4: kms: Don't duplicate pending commit > > > > > > > drm/vc4: kms: Fix previous HVS commit wait > > > > > > > > > > > > > > drivers/gpu/drm/vc4/vc4_kms.c | 42 > > > > > > > --- > > > > > > > 1 file changed, 19 insertions(+), 23 deletions(-) > > > > > > > > > > > > I tested the v2 patches based on latest mainline kernel with RPi 4B. > > > > > > System can boot up into desktop environment. > > > > > > > > > > So the thing that was broken initially isn't anymore? > > > > > > > > No. It does not appear anymore. > > > > > > > > > > Although it still hit the bug [1], which might be under debugging, I > > > > > > think these patches LGTM. > > > > > > > > > > The vblank timeout stuff is a symptom of various different bugs. Can > > > > > you > > > > > share your setup, your config.txt, and what you're doing to trigger > > > > > it? > > > > > > > > I get the RPi boot firmware files from raspberrypi's repository at tag > > > > 1.20211029 [1] > > > > And, make system boots: > > > > RPi firmware -> U-Boot -> Linux kernel (aarch64) with corresponding DTB > > > > > > > > The config.txt only has: > > > > enable_uart=1 > > > > arm_64bit=1 > > > > kernel=uboot.bin > > > > > > > > This bug can be reproduced with es2gears command easily. May need to > > > > wait it running a while. > > > > > > > > Mesa: 21.2.2 > > > > libdrm: 2.4.107 > > > > xserver/wayland: 1.20.11 Using wayland > > > > > > > > This bug happens with direct boot path as well: > > > > RPi firmware -> Linux kernel (aarch64) with corresponding DTB > > > > > > > > I tried with Endless OS and Ubuntu's RPi 4B images. > > > > An easy setup is using Ubuntu 21.10 RPi 4B image [2]. Then, replace > > > > the kernel & device tree blob and edit the config.txt. > > > > > > Does it still appear if you raise the core clock in the config.txt file > > > using: core_freq_min=600 ? > > > > It still appears when I raise the core clock in the config.txt file: > > core_freq_min=600 > > That's weird, we had some issues like that already but could never point > exactly what was going on. > > Is testing the official raspberrypi kernel an option for you? If so, > trying the same workload with fkms would certainly help I tested the raspberrypi kernel on rpi-5.16.y branch at commit bcb52df6df52 ("xhci: add a quirk to work around a suspected cache bug on VLI controllers"). Also, enabled the fkms. So, vc4 and v3d are loaded. However, the "flip_done timed out" error does not appear like mainline kernel. Jian-Hong Pan
Re: [Intel-gfx] [PATCH v2 03/16] drm/i915: Remove pages_mutex and intel_gtt->vma_ops.set/clear_pages members, v2.
On 06-12-2021 18:10, Matthew Auld wrote: > On Mon, 29 Nov 2021 at 13:57, Maarten Lankhorst > wrote: >> Big delta, but boils down to moving set_pages to i915_vma.c, and removing >> the special handling, all callers use the defaults anyway. We only remap >> in ggtt, so default case will fall through. >> >> Because we still don't require locking in i915_vma_unpin(), handle this by >> using xchg in get_pages(), as it's locked with obj->mutex, and cmpxchg in >> unpin, which only fails if we race a against a new pin. >> >> Changes since v1: >> - aliasing gtt sets ZERO_SIZE_PTR, not -ENODEV, remove special case >> from __i915_vma_get_pages(). (Matt) >> >> Signed-off-by: Maarten Lankhorst > > >> +static int >> +__i915_vma_get_pages(struct i915_vma *vma) >> +{ >> + struct sg_table *pages; >> + int ret; >> + >> + /* >> +* The vma->pages are only valid within the lifespan of the borrowed >> +* obj->mm.pages. When the obj->mm.pages sg_table is regenerated, so >> +* must be the vma->pages. A simple rule is that vma->pages must only >> +* be accessed when the obj->mm.pages are pinned. >> +*/ >> + GEM_BUG_ON(!i915_gem_object_has_pinned_pages(vma->obj)); >> + >> + switch (vma->ggtt_view.type) { >> + default: >> + GEM_BUG_ON(vma->ggtt_view.type); >> + fallthrough; >> + case I915_GGTT_VIEW_NORMAL: >> + pages = vma->obj->mm.pages; >> + break; >> + >> + case I915_GGTT_VIEW_ROTATED: >> + pages = >> + intel_rotate_pages(&vma->ggtt_view.rotated, >> vma->obj); >> + break; >> + >> + case I915_GGTT_VIEW_REMAPPED: >> + pages = >> + intel_remap_pages(&vma->ggtt_view.remapped, >> vma->obj); >> + break; >> + >> + case I915_GGTT_VIEW_PARTIAL: >> + pages = intel_partial_pages(&vma->ggtt_view, vma->obj); >> + break; >> + } >> + >> + ret = 0; >> + if (IS_ERR(pages)) { >> + ret = PTR_ERR(pages); >> + pages = NULL; >> + drm_err(&vma->vm->i915->drm, >> + "Failed to get pages for VMA view type %u (%d)!\n", >> + vma->ggtt_view.type, ret); >> + } >> + >> + pages = xchg(&vma->pages, pages); >> + >> + /* did we race against a put_pages? */ >> + if (pages && pages != vma->obj->mm.pages) { >> + sg_free_table(vma->pages); >> + kfree(vma->pages); > So should this one rather be: > > sg_free_table(pages); > kfree(pages); > > Or am I missing something? Correct! I missed it. Will fix it up when committing, or if a new version is needed.
Re: [PATCH v2 2/3] drm/format-helper: Add drm_fb_xrgb8888_to_xrgb2101010_toio()
Hi, thanks for the review! On 07/12/2021 18.40, Thomas Zimmermann wrote: Hi Am 07.12.21 um 08:29 schrieb Hector Martin: Add XRGB emulation support for devices that can only do XRGB2101010. This is chiefly useful for simpledrm on Apple devices where the bootloader-provided framebuffer is 10-bit. Signed-off-by: Hector Martin --- drivers/gpu/drm/drm_format_helper.c | 62 + include/drm/drm_format_helper.h | 3 ++ 2 files changed, 65 insertions(+) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index dbe3e830096e..edd611d3ab6a 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -409,6 +409,59 @@ void drm_fb_xrgb_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch, } EXPORT_SYMBOL(drm_fb_xrgb_to_rgb888_toio); +static void drm_fb_xrgb_to_xrgb2101010_line(u32 *dbuf, const u32 *sbuf, + unsigned int pixels) +{ + unsigned int x; + + for (x = 0; x < pixels; x++) { + *dbuf++ = ((sbuf[x] & 0x00FF) << 2) | + ((sbuf[x] & 0xFF00) << 4) | + ((sbuf[x] & 0x00FF) << 6); This isn't quite right. The lowest two destination bits in each component will always be zero. You have to do the shifting as above and for each component the two highest source bits have to be OR'ed into the two lowest destination bits. For example the source bits in a component are numbered 7 to 0 | 7 6 5 4 3 2 1 0 | then the destination bits should be | 7 6 5 4 3 2 1 0 7 6 | I think both approaches have pros and cons. Leaving the two LSBs always at 0 yields a fully linear transfer curve with no discontinuities, but means the maximum brightness is slightly less than full. Setting them fully maps the brightness range, but creates 4 double wide steps in the transfer curve (also it's potentially slightly slower CPU-wise). If you prefer the latter I'll do that for v2. -- Hector Martin (mar...@marcan.st) Public Key: https://mrcn.st/pub
RE: [v3 0/3] Introduce Raptor Lake S
> -Original Message- > From: Srivatsa, Anusha > Sent: Monday, December 6, 2021 9:59 AM > To: 'Tvrtko Ursulin' ; intel- > g...@lists.freedesktop.org > Cc: x...@kernel.org; dri-devel@lists.freedesktop.org; Ingo Molnar > ; Borislav Petkov ; Dave Hansen > ; Joonas Lahtinen > ; Nikula, Jani > Subject: RE: [v3 0/3] Introduce Raptor Lake S > > > > > -Original Message- > > From: Tvrtko Ursulin > > Sent: Friday, December 3, 2021 2:57 PM > > To: Srivatsa, Anusha ; intel- > > g...@lists.freedesktop.org > > Cc: x...@kernel.org; dri-devel@lists.freedesktop.org; Ingo Molnar > > ; Borislav Petkov ; Dave Hansen > > ; Joonas Lahtinen > > ; Nikula, Jani > > > > Subject: Re: [v3 0/3] Introduce Raptor Lake S > > > > > > On 03/12/2021 06:35, Anusha Srivatsa wrote: > > > Raptor Lake S(RPL-S) is a version 12 Display, Media and Render. For > > > all i915 purposes it is the same as Alder Lake S (ADL-S). > > > > > > The series introduces it as a subplatform of ADL-S. The one > > > difference is the GuC submission which is default on RPL-S but was > > > not the case with ADL-S. > > > > As a side note, not a blocker of any kind, I am slightly disheartened > > but the confusion of ADL_P and ADL_S being separate platforms, but > > then RPL_S is subplatform of ADL_S. Maybe it is just me not being able > > to keep track of things. > > > > > All patches are reviewed. Jani has acked the series. > > > Looking for other acks in order to merge these to respective branches. > > > > Which branches would that be for this series? First two to > > drm-intel-next and last one to drm-intel-gt-next? Is that complication > > needed and/or worth the effort? > > Tvrtko, > All three have to land to drm-intel-next. The last one has dependency on the > first patch and is a trivial change. @Ursulin, Tvrtko @Joonas Lahtinen can I get an ack to merge these into drm-intel-next branch? Anusha > Anusha > > Regards, > > > > Tvrtko > > > > > Cc: x...@kernel.org > > > Cc: dri-devel@lists.freedesktop.org > > > Cc: Ingo Molnar > > > Cc: Borislav Petkov > > > Cc: Dave Hansen > > > Cc: Joonas Lahtinen > > > Cc: Tvrtko Ursulin > > > Acked-by: Jani Nikula > > > > > > Anusha Srivatsa (3): > > >drm/i915/rpl-s: Add PCI IDS for Raptor Lake S > > >drm/i915/rpl-s: Add PCH Support for Raptor Lake S > > >drm/i915/rpl-s: Enable guc submission by default > > > > > > arch/x86/kernel/early-quirks.c | 1 + > > > drivers/gpu/drm/i915/gt/uc/intel_uc.c| 2 +- > > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > > drivers/gpu/drm/i915/i915_pci.c | 1 + > > > drivers/gpu/drm/i915/intel_device_info.c | 7 +++ > > > drivers/gpu/drm/i915/intel_device_info.h | 3 +++ > > > drivers/gpu/drm/i915/intel_pch.c | 1 + > > > drivers/gpu/drm/i915/intel_pch.h | 1 + > > > include/drm/i915_pciids.h| 9 + > > > 9 files changed, 26 insertions(+), 1 deletion(-) > > >
Re: [Nouveau] Regression in 5.15 in nouveau
Am 06.12.21 um 19:37 schrieb Dan Moulding: On 04.12.21 17:40, Stefan Fritsch wrote: Hi, when updating from 5.14 to 5.15 on a system with NVIDIA GP108 [GeForce GT 1030] (NV138) and Ryzen 9 3900XT using kde/plasma on X (not wayland), there is a regression: There is now some annoying black flickering in some applications, for example thunderbird, firefox, or mpv. It mostly happens when scrolling or when playing video. Only the window of the application flickers, not the whole screen. But the flickering is not limited to the scrolled area: for example in firefox the url and bookmark bars flicker, too, not only the web site. I have bisected the issue to this commit: commit 3e1ad79bf66165bdb2baca3989f9227939241f11 (HEAD) I have been experiencing this same issue since switching to 5.15. I can confirm that reverting the above mentioned commit fixes the issue for me. I'm on GP104 hardware (GeForce GTX 1070), also running KDE Plasma on X. I'm still scratching my head what's going wrong here. Either we trigger some performance problem because we now wait twice for submissions or nouveau is doing something very nasty and not syncing it's memory accesses correctly. Attached is an only compile tested patch which might mitigate the first problem. But if it's the second then nouveau has a really nasty design issue here and somebody with more background on that driver design needs to take a look. Please test if that patch changes anything. Thanks, Christian. Cheers, -- Dan >From bcb86d62569c0131288c8b032f848f28f0178648 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20K=C3=B6nig?= Date: Tue, 7 Dec 2021 10:10:15 +0100 Subject: [PATCH] drm/nouveau: wait for the exclusive fence after the shared ones MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Always waiting for the exclusive fence resulted on some performance regressions. So try to wait for the shared fences first, then the exclusive fence should always be signaled already. Signed-off-by: Christian König --- drivers/gpu/drm/nouveau/nouveau_fence.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c index 05d0b3eb3690..0947e332371b 100644 --- a/drivers/gpu/drm/nouveau/nouveau_fence.c +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c @@ -353,15 +353,19 @@ nouveau_fence_sync(struct nouveau_bo *nvbo, struct nouveau_channel *chan, bool e if (ret) return ret; - } - fobj = dma_resv_shared_list(resv); - fence = dma_resv_excl_fence(resv); + fobj = NULL; + } else { + fobj = dma_resv_shared_list(resv); + } - if (fence) { + for (i = 0; (i < fobj ? fobj->shared_count : 0) && !ret; ++i) { struct nouveau_channel *prev = NULL; bool must_wait = true; + fence = rcu_dereference_protected(fobj->shared[i], + dma_resv_held(resv)); + f = nouveau_local_fence(fence, chan->drm); if (f) { rcu_read_lock(); @@ -373,20 +377,13 @@ nouveau_fence_sync(struct nouveau_bo *nvbo, struct nouveau_channel *chan, bool e if (must_wait) ret = dma_fence_wait(fence, intr); - - return ret; } - if (!exclusive || !fobj) - return ret; - - for (i = 0; i < fobj->shared_count && !ret; ++i) { + fence = dma_resv_excl_fence(resv); + if (fence) { struct nouveau_channel *prev = NULL; bool must_wait = true; - fence = rcu_dereference_protected(fobj->shared[i], - dma_resv_held(resv)); - f = nouveau_local_fence(fence, chan->drm); if (f) { rcu_read_lock(); @@ -398,6 +395,8 @@ nouveau_fence_sync(struct nouveau_bo *nvbo, struct nouveau_channel *chan, bool e if (must_wait) ret = dma_fence_wait(fence, intr); + + return ret; } return ret; -- 2.25.1
Re: [PATCH v2 2/3] drm/format-helper: Add drm_fb_xrgb8888_to_xrgb2101010_toio()
Hi Am 07.12.21 um 08:29 schrieb Hector Martin: Add XRGB emulation support for devices that can only do XRGB2101010. This is chiefly useful for simpledrm on Apple devices where the bootloader-provided framebuffer is 10-bit. Signed-off-by: Hector Martin --- drivers/gpu/drm/drm_format_helper.c | 62 + include/drm/drm_format_helper.h | 3 ++ 2 files changed, 65 insertions(+) diff --git a/drivers/gpu/drm/drm_format_helper.c b/drivers/gpu/drm/drm_format_helper.c index dbe3e830096e..edd611d3ab6a 100644 --- a/drivers/gpu/drm/drm_format_helper.c +++ b/drivers/gpu/drm/drm_format_helper.c @@ -409,6 +409,59 @@ void drm_fb_xrgb_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch, } EXPORT_SYMBOL(drm_fb_xrgb_to_rgb888_toio); +static void drm_fb_xrgb_to_xrgb2101010_line(u32 *dbuf, const u32 *sbuf, + unsigned int pixels) +{ + unsigned int x; + + for (x = 0; x < pixels; x++) { + *dbuf++ = ((sbuf[x] & 0x00FF) << 2) | + ((sbuf[x] & 0xFF00) << 4) | + ((sbuf[x] & 0x00FF) << 6); This isn't quite right. The lowest two destination bits in each component will always be zero. You have to do the shifting as above and for each component the two highest source bits have to be OR'ed into the two lowest destination bits. For example the source bits in a component are numbered 7 to 0 | 7 6 5 4 3 2 1 0 | then the destination bits should be | 7 6 5 4 3 2 1 0 7 6 | Best regards Thomas + } +} + +/** + * drm_fb_xrgb_to_xrgb2101010_toio - Convert XRGB to XRGB2101010 clip + * buffer + * @dst: XRGB2101010 destination buffer (iomem) + * @dst_pitch: Number of bytes between two consecutive scanlines within dst + * @vaddr: XRGB source buffer + * @fb: DRM framebuffer + * @clip: Clip rectangle area to copy + * + * Drivers can use this function for XRGB2101010 devices that don't natively + * support XRGB. + */ +void drm_fb_xrgb_to_xrgb2101010_toio(void __iomem *dst, +unsigned int dst_pitch, const void *vaddr, +const struct drm_framebuffer *fb, +const struct drm_rect *clip) +{ + size_t linepixels = clip->x2 - clip->x1; + size_t dst_len = linepixels * sizeof(u32); + unsigned y, lines = clip->y2 - clip->y1; + void *dbuf; + + if (!dst_pitch) + dst_pitch = dst_len; + + dbuf = kmalloc(dst_len, GFP_KERNEL); + if (!dbuf) + return; + + vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32)); + for (y = 0; y < lines; y++) { + drm_fb_xrgb_to_xrgb2101010_line(dbuf, vaddr, linepixels); + memcpy_toio(dst, dbuf, dst_len); + vaddr += fb->pitches[0]; + dst += dst_pitch; + } + + kfree(dbuf); +} +EXPORT_SYMBOL(drm_fb_xrgb_to_xrgb2101010_toio); + /** * drm_fb_xrgb_to_gray8 - Convert XRGB to grayscale * @dst: 8-bit grayscale destination buffer @@ -500,6 +553,10 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for fb_format = DRM_FORMAT_XRGB; if (dst_format == DRM_FORMAT_ARGB) dst_format = DRM_FORMAT_XRGB; + if (fb_format == DRM_FORMAT_ARGB2101010) + fb_format = DRM_FORMAT_XRGB2101010; + if (dst_format == DRM_FORMAT_ARGB2101010) + dst_format = DRM_FORMAT_XRGB2101010; if (dst_format == fb_format) { drm_fb_memcpy_toio(dst, dst_pitch, vmap, fb, clip); @@ -515,6 +572,11 @@ int drm_fb_blit_toio(void __iomem *dst, unsigned int dst_pitch, uint32_t dst_for drm_fb_xrgb_to_rgb888_toio(dst, dst_pitch, vmap, fb, clip); return 0; } + } else if (dst_format == DRM_FORMAT_XRGB2101010) { + if (fb_format == DRM_FORMAT_XRGB) { + drm_fb_xrgb_to_xrgb2101010_toio(dst, dst_pitch, vmap, fb, clip); + return 0; + } } return -EINVAL; diff --git a/include/drm/drm_format_helper.h b/include/drm/drm_format_helper.h index 97e4c3223af3..b30ed5de0a33 100644 --- a/include/drm/drm_format_helper.h +++ b/include/drm/drm_format_helper.h @@ -33,6 +33,9 @@ void drm_fb_xrgb_to_rgb888(void *dst, unsigned int dst_pitch, const void *sr void drm_fb_xrgb_to_rgb888_toio(void __iomem *dst, unsigned int dst_pitch, const void *vaddr, const struct drm_framebuffer *fb, const struct drm_rect *clip); +void drm_fb_xrgb_to_xrgb2101010_toio(void __iomem *dst, unsigned int dst_pitch, +const void *vaddr, const struct drm_framebuffer *fb, +
Re: [PATCH v2 3/3] drm/simpledrm: Add XRGB2101010 format
Hi Am 07.12.21 um 08:29 schrieb Hector Martin: This is the format used by the bootloader framebuffer on Apple ARM64 platforms. Reviewed-by: Thomas Zimmermann Signed-off-by: Hector Martin --- drivers/gpu/drm/tiny/simpledrm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index 2f15b9aa..edadfd9ee882 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -571,7 +571,7 @@ static const uint32_t simpledrm_default_formats[] = { //DRM_FORMAT_XRGB1555, //DRM_FORMAT_ARGB1555, DRM_FORMAT_RGB888, - //DRM_FORMAT_XRGB2101010, + DRM_FORMAT_XRGB2101010, //DRM_FORMAT_ARGB2101010, You should also enable DRM_FORMAT_ARGB2101010 here. You added the conversion function, so DRM can deal with it. Having an alpha channel isn't typically supported for primary planes, but the format is listed in SIMPLEFB_FORMATS. [1] With this change: Reviewed-by: Thomas Zimmermann Best regards [1] https://elixir.bootlin.com/linux/latest/source/include/linux/platform_data/simplefb.h#L16 }; -- 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: Return error codes from struct drm_driver.gem_create_object
Hi Am 07.12.21 um 09:55 schrieb Dan Carpenter: I appologize. This thread has been really frustrating. I got mixed up because I recently sent patches for ingenic and vc4. Also we are working against different trees so maybe that is part of the problem? I'm looking at today's linux-next. Your patch has been applied. Yes. You updated all the drivers. But somehow the vc4 chunk from your patch was dropped. It was *NOT* dropped by Stephen Rothwell. It got dropped earlier. I am including the `git format-patch -1 ` output from the commit. My vc4 change is in drm-misc-next, [1] but not in drm-tip. [2] Your vc4 patch went through drm-misc-fixes. drm-tip is build automatically by our DRM tools from the various trees. The tools took my patch from drm-misc-next and your patch from drm-misc-fixes and the result was broken. Thanks for bringing this up. I'll see what I can do about it. Best regards Thomas [1] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/vc4/vc4_bo.c#n394 [2] https://cgit.freedesktop.org/drm/drm-tip/tree/drivers/gpu/drm/vc4/vc4_bo.c -- 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 v2 1/3] of: Move simple-framebuffer device handling from simplefb to of
Am 07.12.21 um 10:02 schrieb Thomas Zimmermann: Hi Am 07.12.21 um 08:29 schrieb Hector Martin: This code is required for both simplefb and simpledrm, so let's move it into the OF core instead of having it as an ad-hoc initcall in the drivers. Signed-off-by: Hector Martin Acked-by: Thomas Zimmermann Well, please don't take this as a review. :) This looks much better than before. Thank you. --- drivers/of/platform.c | 5 + drivers/video/fbdev/simplefb.c | 21 + 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b3faf89744aa..e097f40b03c0 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -540,6 +540,11 @@ static int __init of_platform_default_populate_init(void) of_node_put(node); } + for_each_child_of_node(of_chosen, node) { + if (of_device_is_compatible(node, "simple-framebuffer")) + of_platform_device_create(node, NULL, NULL); + } + /* Populate everything else. */ of_platform_default_populate(NULL, NULL, NULL); diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c index b63074fd892e..57541887188b 100644 --- a/drivers/video/fbdev/simplefb.c +++ b/drivers/video/fbdev/simplefb.c @@ -541,26 +541,7 @@ static struct platform_driver simplefb_driver = { .remove = simplefb_remove, }; -static int __init simplefb_init(void) -{ - int ret; - struct device_node *np; - - ret = platform_driver_register(&simplefb_driver); - if (ret) - return ret; - - if (IS_ENABLED(CONFIG_OF_ADDRESS) && of_chosen) { - for_each_child_of_node(of_chosen, np) { - if (of_device_is_compatible(np, "simple-framebuffer")) - of_platform_device_create(np, NULL, NULL); - } - } - - return 0; -} - -fs_initcall(simplefb_init); +module_platform_driver(simplefb_driver); MODULE_AUTHOR("Stephen Warren "); MODULE_DESCRIPTION("Simple framebuffer driver"); -- 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 v2 1/3] of: Move simple-framebuffer device handling from simplefb to of
Hi Am 07.12.21 um 08:29 schrieb Hector Martin: This code is required for both simplefb and simpledrm, so let's move it into the OF core instead of having it as an ad-hoc initcall in the drivers. Signed-off-by: Hector Martin Acked-by: Thomas Zimmermann This looks much better than before. Thank you. --- drivers/of/platform.c | 5 + drivers/video/fbdev/simplefb.c | 21 + 2 files changed, 6 insertions(+), 20 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b3faf89744aa..e097f40b03c0 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -540,6 +540,11 @@ static int __init of_platform_default_populate_init(void) of_node_put(node); } + for_each_child_of_node(of_chosen, node) { + if (of_device_is_compatible(node, "simple-framebuffer")) + of_platform_device_create(node, NULL, NULL); + } + /* Populate everything else. */ of_platform_default_populate(NULL, NULL, NULL); diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c index b63074fd892e..57541887188b 100644 --- a/drivers/video/fbdev/simplefb.c +++ b/drivers/video/fbdev/simplefb.c @@ -541,26 +541,7 @@ static struct platform_driver simplefb_driver = { .remove = simplefb_remove, }; -static int __init simplefb_init(void) -{ - int ret; - struct device_node *np; - - ret = platform_driver_register(&simplefb_driver); - if (ret) - return ret; - - if (IS_ENABLED(CONFIG_OF_ADDRESS) && of_chosen) { - for_each_child_of_node(of_chosen, np) { - if (of_device_is_compatible(np, "simple-framebuffer")) - of_platform_device_create(np, NULL, NULL); - } - } - - return 0; -} - -fs_initcall(simplefb_init); +module_platform_driver(simplefb_driver); MODULE_AUTHOR("Stephen Warren "); MODULE_DESCRIPTION("Simple framebuffer driver"); -- 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: [Bug Report] Desktop monitor sleep regression
Add CCs On Tue, Dec 7, 2021 at 7:37 AM Brandon Nielsen wrote: > Monitors no longer sleep properly on my system (dual monitor connected > via DP->DVI, amdgpu, x86_64). The monitors slept properly on 5.14, but > stopped during the 5.15 series. I have also filed this bug on the kernel > bugzilla[0] and downstream[1]. > > I have performed a bisect, first "bad" commit to master is > 55285e21f04517939480966164a33898c34b2af2[1], the same change made it > into the 5.15 branch as e3b39825ed0813f787cb3ebdc5ecaa5131623647. I have > verified the issue exists in latest master > (a51e3ac43ddbad891c2b1a4f3aa52371d6939570). > > > Steps to reproduce: > >1. Boot system (Fedora Workstation 35 in this case) >2. Log in >3. Lock screen (after a few seconds, monitors will enter power save > "sleep" state with backlight off) >4. Wait (usually no more than 30 seconds, sometimes up to a few minutes) >5. Observe monitor leaving "sleep" state (backlight comes back on), > but nothing is displayed > > > [0] - https://bugzilla.kernel.org/show_bug.cgi?id=215203 > [1] - https://bugzilla.redhat.com/show_bug.cgi?id=2028613
Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
I appologize. This thread has been really frustrating. I got mixed up because I recently sent patches for ingenic and vc4. Also we are working against different trees so maybe that is part of the problem? I'm looking at today's linux-next. Your patch has been applied. Yes. You updated all the drivers. But somehow the vc4 chunk from your patch was dropped. It was *NOT* dropped by Stephen Rothwell. It got dropped earlier. I am including the `git format-patch -1 ` output from the commit. regards, dan carpenter >From 4ff22f487f8c26b99cbe1678344595734c001a39 Mon Sep 17 00:00:00 2001 From: Thomas Zimmermann Date: Tue, 30 Nov 2021 10:52:55 +0100 Subject: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object GEM helper libraries use struct drm_driver.gem_create_object to let drivers override GEM object allocation. On failure, the call returns NULL. Change the semantics to make the calls return a pointer-encoded error. This aligns the callback with its callers. Fixes the ingenic driver, which already returns an error pointer. Also update the callers to handle the involved types more strictly. Signed-off-by: Thomas Zimmermann Reviewed-by: Steven Price Acked-by: Maxime Ripard Link: https://patchwork.freedesktop.org/patch/msgid/20211130095255.26710-1-tzimmerm...@suse.de --- drivers/gpu/drm/drm_gem_cma_helper.c| 17 ++--- drivers/gpu/drm/drm_gem_shmem_helper.c | 17 ++--- drivers/gpu/drm/drm_gem_vram_helper.c | 4 ++-- drivers/gpu/drm/lima/lima_gem.c | 2 +- drivers/gpu/drm/panfrost/panfrost_gem.c | 2 +- drivers/gpu/drm/v3d/v3d_bo.c| 4 ++-- drivers/gpu/drm/vgem/vgem_drv.c | 2 +- drivers/gpu/drm/virtio/virtgpu_object.c | 2 +- include/drm/drm_drv.h | 5 +++-- 9 files changed, 31 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index 5f42e44b2ab3..6f18f143dd30 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -67,18 +67,21 @@ __drm_gem_cma_create(struct drm_device *drm, size_t size, bool private) struct drm_gem_object *gem_obj; int ret = 0; - if (drm->driver->gem_create_object) + if (drm->driver->gem_create_object) { gem_obj = drm->driver->gem_create_object(drm, size); - else - gem_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL); - if (!gem_obj) - return ERR_PTR(-ENOMEM); + if (IS_ERR(gem_obj)) + return ERR_CAST(gem_obj); + cma_obj = to_drm_gem_cma_obj(gem_obj); + } else { + cma_obj = kzalloc(sizeof(*cma_obj), GFP_KERNEL); + if (!cma_obj) + return ERR_PTR(-ENOMEM); + gem_obj = &cma_obj->base; + } if (!gem_obj->funcs) gem_obj->funcs = &drm_gem_cma_default_funcs; - cma_obj = container_of(gem_obj, struct drm_gem_cma_object, base); - if (private) { drm_gem_private_object_init(drm, gem_obj, size); diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 0eeda1012364..7915047cb041 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -56,14 +56,17 @@ __drm_gem_shmem_create(struct drm_device *dev, size_t size, bool private) size = PAGE_ALIGN(size); - if (dev->driver->gem_create_object) + if (dev->driver->gem_create_object) { obj = dev->driver->gem_create_object(dev, size); - else - obj = kzalloc(sizeof(*shmem), GFP_KERNEL); - if (!obj) - return ERR_PTR(-ENOMEM); - - shmem = to_drm_gem_shmem_obj(obj); + if (IS_ERR(obj)) + return ERR_CAST(obj); + shmem = to_drm_gem_shmem_obj(obj); + } else { + shmem = kzalloc(sizeof(*shmem), GFP_KERNEL); + if (!shmem) + return ERR_PTR(-ENOMEM); + obj = &shmem->base; + } if (!obj->funcs) obj->funcs = &drm_gem_shmem_funcs; diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c index bfa386b98134..3f00192215d1 100644 --- a/drivers/gpu/drm/drm_gem_vram_helper.c +++ b/drivers/gpu/drm/drm_gem_vram_helper.c @@ -197,8 +197,8 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev, if (dev->driver->gem_create_object) { gem = dev->driver->gem_create_object(dev, size); - if (!gem) - return ERR_PTR(-ENOMEM); + if (IS_ERR(gem)) + return ERR_CAST(gem); gbo = drm_gem_vram_of_gem(gem); } else { gbo = kzalloc(sizeof(*gbo), GFP_KERNEL); diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gp
[PATCH] drm/shmem-helper: Remove duplicated include
Remove duplicated include in drm_gem_shmem_helper.c Signed-off-by: Yihao Han --- drivers/gpu/drm/drm_gem_shmem_helper.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c index 621924116eb4..7915047cb041 100644 --- a/drivers/gpu/drm/drm_gem_shmem_helper.c +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c @@ -10,7 +10,6 @@ #include #include #include -#include #ifdef CONFIG_X86 #include -- 2.17.1
WARNING: CPU: 1 PID: 722 at drivers/gpu/drm/i915/display/intel_tc.c:761
Hello, I found warnings in the stable tree. Commit: a2547651bc896f95a3680a6a0a27401e7c7a1080 ("Linux 5.15.6") There are two unique warn locations: ammarfaizi2@integral2:~$ sudo dmesg -Sr | grep -oiE 'WARNING:.+' | sort | uniq [sudo] password for ammarfaizi2: WARNING: CPU: 1 PID: 722 at drivers/gpu/drm/i915/display/intel_tc.c:531 intel_tc_port_sanitize+0x323/0x380 [i915] WARNING: CPU: 1 PID: 722 at drivers/gpu/drm/i915/display/intel_tc.c:761 intel_tc_port_init+0x1a9/0x1b0 [i915] Full log can be found here: https://gist.githubusercontent.com/ammarfaizi2/d588af19f7bb9eb40494626ecc041654/raw/b98d3e1ee5f5ed20b79b0a6cabce06dce8abcd97/kernel_log_bug_linux_5.15.6_stable.txt Warning: <4>[6.629829][ T722] [ cut here ] <4>[6.629830][ T722] i915 :00:02.0: drm_WARN_ON(val == 0x) <4>[6.629842][ T722] WARNING: CPU: 1 PID: 722 at drivers/gpu/drm/i915/display/intel_tc.c:761 intel_tc_port_init+0x1a9/0x1b0 [i915] <4>[6.629919][ T722] Modules linked in: i915(+) snd_soc_dmic snd_sof_pci_intel_tgl snd_sof_intel_hda_common snd_soc_hdac_hda soundwire_intel soundwire_generic_allocation soundwire_cadence snd_sof_intel_hda snd_sof_pci snd_sof_xtensa_dsp snd_sof snd_hda_ext_core snd_soc_acpi_intel_match snd_soc_acpi soundwire_bus ledtrig_audio snd_soc_core rtw88_8822ce snd_compress ac97_bus snd_pcm_dmaengine rtw88_8822c snd_hda_intel snd_intel_dspcfg rtw88_pci snd_intel_sdw_acpi rtw88_core snd_hda_codec snd_hda_core snd_hwdep intel_tcc_cooling mac80211 nls_iso8859_1 snd_pcm x86_pkg_temp_thermal intel_powerclamp coretemp snd_seq_midi kvm_intel snd_seq_midi_event snd_rawmidi mei_hdcp intel_rapl_msr ttm kvm cfg80211 drm_kms_helper snd_seq btusb btrtl btbcm uvcvideo btintel bluetooth videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 videobuf2_common videodev cec processor_thermal_device_pci_legacy processor_thermal_device snd_seq_device rc_core processor_thermal_rfim snd_timer processor_thermal_mbox <4>[6.629951][ T722] crct10dif_pclmul ecdh_generic i2c_algo_bit mc joydev input_leds processor_thermal_rapl snd ghash_clmulni_intel ecc fb_sys_fops mei_me aesni_intel hp_wmi syscopyarea intel_rapl_common crypto_simd sysfillrect platform_profile mei libarc4 sysimgblt serio_raw sparse_keymap efi_pstore hid_multitouch cryptd ee1004 soundcore wmi_bmof intel_soc_dts_iosf mac_hid int3400_thermal int3403_thermal int340x_thermal_zone acpi_thermal_rel acpi_pad dptf_pch_fivr sch_fq_codel zram drm msr parport_pc ppdev lp parport ip_tables x_tables autofs4 btrfs blake2b_generic xor usbhid raid6_pq libcrc32c hid_generic nvme nvme_core intel_lpss_pci xhci_pci crc32_pclmul xhci_pci_renesas intel_lpss i2c_i801 i2c_hid_acpi vmd i2c_smbus idma64 i2c_hid hid wmi video pinctrl_tigerlake <4>[6.629984][ T722] CPU: 1 PID: 722 Comm: modprobe Not tainted 5.15.6-icetea2-stable-00459-ga2547651bc89 #1 d738e98f796accca080303b93ac2eee924880c33 <4>[6.629986][ T722] Hardware name: HP HP Laptop 14s-dq2xxx/87FD, BIOS F.15 09/15/2021 <4>[6.629987][ T722] RIP: 0010:intel_tc_port_init+0x1a9/0x1b0 [i915] <4>[6.630045][ T722] Code: 4c 8b 67 50 4d 85 e4 75 03 4c 8b 27 e8 b0 a9 17 e0 48 c7 c1 c8 22 6b a1 4c 89 e2 48 c7 c7 5e 0f 6d a1 48 89 c6 e8 57 ee 58 e0 <0f> 0b e9 61 ff ff ff 0f 1f 44 00 00 48 8b 17 80 ba d3 0d 00 00 0b <4>[6.630046][ T722] RSP: 0018:c900017f7b08 EFLAGS: 00010296 <4>[6.630048][ T722] RAX: 0031 RBX: 88810403a000 RCX: 0027 <4>[6.630049][ T722] RDX: 88846fa60c28 RSI: 0001 RDI: 88846fa60c20 <4>[6.630050][ T722] RBP: R08: 82760528 R09: dfff <4>[6.630051][ T722] R10: 82680540 R11: 82680540 R12: 888102046410 <4>[6.630052][ T722] R13: R14: 88810403b940 R15: <4>[6.630054][ T722] FS: 7f4fd979e580() GS:88846fa4() knlGS: <4>[6.630055][ T722] CS: 0010 DS: ES: CR0: 80050033 <4>[6.630056][ T722] CR2: 7fe05f19fea0 CR3: 000119098004 CR4: 00770ee0 <4>[6.630058][ T722] PKRU: 5554 <4>[6.630059][ T722] Call Trace: <4>[6.630062][ T722] <4>[6.630066][ T722] intel_ddi_init+0x663/0xba0 [i915 91e0a10445cc74861446c203b02c9291e0680a4b] <4>[6.630125][ T722] intel_modeset_init_nogem+0x982/0x1230 [i915 91e0a10445cc74861446c203b02c9291e0680a4b] <4>[6.630180][ T722] ? gen12_fwtable_read32+0x96/0x2a0 [i915 91e0a10445cc74861446c203b02c9291e0680a4b] <4>[6.630230][ T722] i915_driver_probe+0x6dc/0xd10 [i915 91e0a10445cc74861446c203b02c9291e0680a4b] <4>[6.630276][ T722] ? vga_switcheroo_client_probe_defer+0x1f/0x40 <4>[6.630279][ T722] ? i915_pci_probe+0x31/0x110 [i915 91e0a10445cc74861446c203b02c9291e0680a4b] <4>[6.630322][ T722] local_pci_probe+0x40/0x80 <4>[6.630325][ T722]
Re: [PATCH v2 1/2] drm/input_helper: Add new input-handling helper
On Tue, Nov 30, 2021 at 12:35:45PM -0800, Brian Norris wrote: > Hi Pekka, > > On Fri, Nov 19, 2021 at 12:38:41PM +0200, Pekka Paalanen wrote: > > On Thu, 18 Nov 2021 17:46:10 -0800 > > Brian Norris wrote: > > > On Thu, Nov 18, 2021 at 12:39:28PM +0200, Pekka Paalanen wrote: > > > > On Wed, 17 Nov 2021 14:48:40 -0800 > > > > Brian Norris wrote: > > > > If KMS gets a pageflip or modeset in no time after an input event, then > > > > what's the gain. OTOH, if the display server is locking on to vblank, > > > > there might be a delay worth avoiding. But then, is it worth > > > > short-circuiting the wake-up in kernel vs. adding a new ioctl that > > > > userspace could hit to start the warming up process? > > > > > > Rob responded to the first part to some extent (there is definitely gain > > > to be had). > > > > > > To the last part: I wrote a simple debugfs hook to allow user space to > > > force a PSR exit, and then a simple user space program to read input > > > events and smash that debugfs file whenever it sees one. Testing in the > > > same scenarios, this appears to lose less than 100 microseconds versus > > > the in-kernel approach, which is negligible for this use case. (I'm not > > > sure about the other use cases.) > > > > > > So, this is technically doable in user space. > > > > This is crucial information I would like you to include in some commit > > message. I think it is very interesting for the reviewers. Maybe also > > copy that in the cover letter. > > > > In my opinion there is a clear and obvious decision due that > > measurement: Add the new ioctl for userspace to hit, do not try to > > hardcode or upload the wake-up policy into the kernel. > > Perhaps. > > I'll admit, I'm not eager to go write the fd-passing solutions that > others are designing on the fly. I'm currently torn on whether I'll just > live with this current patch set out-of-tree (or, y'all can decide that > a simple, 99% working solution is better than no solution), because it's > simple; or possibly figuring out how to utilize such an ioctl cleanly > within our display manager. I'm not super hopeful on the latter. > > IOW, I'm approximately in line with Doug's thoughts: > https://lore.kernel.org/all/CAD=FV=XARhZoj+0p-doxcbC=4K+NuMc=ur6wqg6kwk-mkpk...@mail.gmail.com/ > But then, we're obviously biased. > > > > I can't speak to the ease of _actually_ integrating this into even our > > > own Chrome display manager, but I highly doubt it will get integrated > > > into others. I'd posit this should weigh into the relative worth, but > > > otherwise can't really give you an answer there. > > > > I think such a thing would be very simple to add to any display server. > > They already have hooks for things like resetting idle timeout timers on > > any relevant input event. > > > > > I'd also note, software-directed PSR is so far designed to be completely > > > opaque to user space. There's no way to disable it; no way to know it's > > > active; and no way to know anything about the parameters it's computing > > > (like average entry/exit delay). Would you suggest a whole set of new > > > IOCTLs for this? > > > > Just one ioctl on the DRM device: "Hey, wake up!". Because that's what > > your patch does in-kernel, right? > > Well, we'd at least want something to advertise that the feature does > something ("is supported") I think, otherwise we're just asking user > space to do useless work. > > > If there are use case specific parameters, then how did you intend to > > allow adjusting those in your proposal? > > Another commenter mentioned the latency tradeoff -- it's possible that > there are panels/eDP-links that resume fast enough that one doesn't care > to use this ioctl. For an in-kernel solution, one has all the data > available and could use hardware information to make decisions, if > needed. For a user space solution, we won't have any of that, and we'd > have to work to expose that information. > > I suppose we could ignore that problem and only expose a minimal UAPI > until we need something more, but it feels like exposing a UAPI for > something is a critical point where one should make sure it's reasonably > descriptive and useful. > > > > > How do you know userspace is using this input device at all? If > > > > userspace is not using the input device, then DRM should not be opening > > > > it either, as it must have no effect on anything. > > > > > > > > If you open an input device that userspace does not use, you also cause > > > > a power consumption regression, because now the input device itself is > > > > active and possibly flooding the kernel with events (e.g. an > > > > accelerometer). > > > > > > Well, I don't think accelerometers show up as input devices, but I > > > suppose your point could apply to actual input devices. fwiw, filtering INPUT_PROP_ACCELEROMETER would go a long way towards ignoring accelerometers. > > My understanding is that accelerometers are evdev (input) device
Re: [PATCH] drm: Return error codes from struct drm_driver.gem_create_object
Hi Am 06.12.21 um 15:40 schrieb Dan Carpenter: On Mon, Dec 06, 2021 at 12:16:24PM +0100, Thomas Zimmermann wrote: Hi Am 06.12.21 um 11:42 schrieb Dan Carpenter: On Tue, Nov 30, 2021 at 10:52:55AM +0100, Thomas Zimmermann wrote: GEM helper libraries use struct drm_driver.gem_create_object to let drivers override GEM object allocation. On failure, the call returns NULL. Change the semantics to make the calls return a pointer-encoded error. This aligns the callback with its callers. Fixes the ingenic driver, which already returns an error pointer. Also update the callers to handle the involved types more strictly. Signed-off-by: Thomas Zimmermann --- There is an alternative patch at [1] that updates the value returned by ingenics' gem_create_object to NULL. Fixing the interface to return an errno code is more consistent with the rest of the GEM functions. [1] https://lore.kernel.org/dri-devel/2028111522.GD1147@kili/ My fix was already applied and backported to -stable etc... Your patch is not developed against a current tree so you broke it. Do you have a specific link? I just checked the stable tree at [1] and there no trace of your patch. It's in 5.15.6 and probably all the other supported -stable trees. https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/gpu/drm/vc4/vc4_bo.c?h=v5.15.6#n387 I'm not sure that I understand the problem. The URL points to vc4, but my link was to your patch for ingenic. vc4 is being updated here as well to ERR_PTR. The ingenic patch never made it into any tree. It actually was the reason to fix the interface. When the current patch makes it through the trees, it should fix all the affected drivers. Best regards Thomas Patches for DRM should go through through DRM trees; drm-misc-fixes in this case. Exceptions should at least be announce on dri-devel. Neither is the case here. Yeah. That's a good question. I don't know, because I just work against linux-next... regards, dan carpenter -- 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