Re: [PATCH] drm/exynos: Drop local dma_parms

2020-09-21 Thread Inki Dae
Hi Robin,

As Marek said, he posted same patch[1] before so I merged it instead of you.
I totally forgot to merge it. :( Sorry for confusing.

[1] 
https://lore.kernel.org/dri-devel/20200707110827.3760-1-m.szyprow...@samsung.com/

Thanks,
Inki Dae

20. 9. 4. 오전 5:51에 Robin Murphy 이(가) 쓴 글:
> Since commit 9495b7e92f71 ("driver core: platform: Initialize dma_parms
> for platform devices"), struct platform_device already provides a
> dma_parms structure, so we can save allocating another one.
> 
> Also the DMA segment size is simply a size, not a bitmask.
> 
> Signed-off-by: Robin Murphy 
> ---
>  drivers/gpu/drm/exynos/exynos_drm_dma.c | 26 +
>  1 file changed, 1 insertion(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dma.c 
> b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> index 58b89ec11b0e..9f25a5ebbf7d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dma.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dma.c
> @@ -31,23 +31,6 @@
>  #define EXYNOS_DEV_ADDR_START0x2000
>  #define EXYNOS_DEV_ADDR_SIZE 0x4000
>  
> -static inline int configure_dma_max_seg_size(struct device *dev)
> -{
> - if (!dev->dma_parms)
> - dev->dma_parms = kzalloc(sizeof(*dev->dma_parms), GFP_KERNEL);
> - if (!dev->dma_parms)
> - return -ENOMEM;
> -
> - dma_set_max_seg_size(dev, DMA_BIT_MASK(32));
> - return 0;
> -}
> -
> -static inline void clear_dma_max_seg_size(struct device *dev)
> -{
> - kfree(dev->dma_parms);
> - dev->dma_parms = NULL;
> -}
> -
>  /*
>   * drm_iommu_attach_device- attach device to iommu mapping
>   *
> @@ -69,9 +52,7 @@ static int drm_iommu_attach_device(struct drm_device 
> *drm_dev,
>   return -EINVAL;
>   }
>  
> - ret = configure_dma_max_seg_size(subdrv_dev);
> - if (ret)
> - return ret;
> + dma_set_max_seg_size(subdrv_dev, UINT_MAX);
>  
>   if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
>   /*
> @@ -89,9 +70,6 @@ static int drm_iommu_attach_device(struct drm_device 
> *drm_dev,
>   ret = iommu_attach_device(priv->mapping, subdrv_dev);
>   }
>  
> - if (ret)
> - clear_dma_max_seg_size(subdrv_dev);
> -
>   return ret;
>  }
>  
> @@ -114,8 +92,6 @@ static void drm_iommu_detach_device(struct drm_device 
> *drm_dev,
>   arm_iommu_attach_device(subdrv_dev, *dma_priv);
>   } else if (IS_ENABLED(CONFIG_IOMMU_DMA))
>   iommu_detach_device(priv->mapping, subdrv_dev);
> -
> - clear_dma_max_seg_size(subdrv_dev);
>  }
>  
>  int exynos_drm_register_dma(struct drm_device *drm, struct device *dev,
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)

2020-09-21 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=209345

--- Comment #6 from Alexander von Gluck (kallis...@unixzen.com) ---
Created attachment 292557
  --> https://bugzilla.kernel.org/attachment.cgi?id=292557=edit
tesla k80 patch

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)

2020-09-21 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=209345

--- Comment #5 from Alexander von Gluck (kallis...@unixzen.com) ---
[ 2208.130049] nouveau: version magic '5.8.10 SMP mod_unload ' should be
'5.8.10-200.fc32.x86_64 SMP mod_unload '
[ 2460.923164] ACPI Warning: \_SB.PCI0.GFX0._DSM: Argument #4 type mismatch -
Found [Buffer], ACPI requires [Package] (20200528/nsarguments-59)
[ 2460.923220] nouveau :08:00.0: can't change power state from D3hot to D0
(config space inaccessible)
[ 2460.923393] nouveau :08:00.0: GPU not supported on big-endian
[ 2460.923411] nouveau: probe of :08:00.0 failed with error -38
[ 2460.923424] nouveau :09:00.0: can't change power state from D3hot to D0
(config space inaccessible)
[ 2460.923504] nouveau :09:00.0: GPU not supported on big-endian
[ 2460.923507] nouveau: probe of :09:00.0 failed with error -38

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


linux-next: manual merge of the drm-misc tree with the drm-intel tree

2020-09-21 Thread Stephen Rothwell
Hi all,

Today's linux-next merge of the drm-misc tree got a conflict in:

  drivers/gpu/drm/i915/selftests/mock_gem_device.c

between commit:

  9f9f4101fc98 ("drm/i915/selftests: Push the fake iommu device from the stack 
to data")

from the drm-intel tree and commit:

  cd01269d11a3 ("drm/i915/selftests: align more to real device lifetimes")

from the drm-misc tree.

I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging.  You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.

-- 
Cheers,
Stephen Rothwell

diff --cc drivers/gpu/drm/i915/selftests/mock_gem_device.c
index 397c313a8b69,c207d2239791..
--- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
+++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
@@@ -118,12 -116,11 +116,11 @@@ static struct dev_pm_domain pm_domain 
  
  struct drm_i915_private *mock_gem_device(void)
  {
 -  struct drm_i915_private *i915;
 -  struct pci_dev *pdev;
  #if IS_ENABLED(CONFIG_IOMMU_API) && defined(CONFIG_INTEL_IOMMU)
 -  struct dev_iommu iommu;
 +  static struct dev_iommu fake_iommu = { .priv = (void *)-1 };
  #endif
 +  struct drm_i915_private *i915;
 +  struct pci_dev *pdev;
-   int err;
  
pdev = kzalloc(sizeof(*pdev), GFP_KERNEL);
if (!pdev)
@@@ -141,11 -132,28 +132,26 @@@
dma_coerce_mask_and_coherent(>dev, DMA_BIT_MASK(64));
  
  #if IS_ENABLED(CONFIG_IOMMU_API) && defined(CONFIG_INTEL_IOMMU)
 -  /* HACK HACK HACK to disable iommu for the fake device; force identity 
mapping */
 -  memset(, 0, sizeof(iommu));
 -  iommu.priv = (void *)-1;
 -  pdev->dev.iommu = 
 +  /* HACK to disable iommu for the fake device; force identity mapping */
 +  pdev->dev.iommu = _iommu;
  #endif
+   if (!devres_open_group(>dev, NULL, GFP_KERNEL)) {
+   put_device(>dev);
+   return NULL;
+   }
+ 
+   i915 = devm_drm_dev_alloc(>dev, _driver,
+ struct drm_i915_private, drm);
+   if (IS_ERR(i915)) {
+   pr_err("Failed to allocate mock GEM device: err=%ld\n", 
PTR_ERR(i915));
+   devres_release_group(>dev, NULL);
+   put_device(>dev);
+ 
+   return NULL;
+   }
  
pci_set_drvdata(pdev, i915);
+   i915->drm.pdev = pdev;
  
dev_pm_domain_set(>dev, _domain);
pm_runtime_enable(>dev);


pgpzYUS1WDHAb.pgp
Description: OpenPGP digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v17 00/20] iommu/arm-smmu + drm/msm: per-process GPU pgtables

2020-09-21 Thread Jordan Crouse
On Mon, Sep 21, 2020 at 10:30:57PM +0100, Will Deacon wrote:
> On Sat, Sep 05, 2020 at 01:04:06PM -0700, Rob Clark wrote:
> > From: Rob Clark 
> > 
> > NOTE: I have re-ordered the series, and propose that we could merge this
> >   series in the following order:
> > 
> >1) 01-11 - merge via drm / msm-next
> >2) 12-15 - merge via iommu, no dependency on msm-next pull req
> 
> Thanks, I've queued 12-15 in the smmu tree.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates
> 
> Will

Will, thanks for your help and patience in getting these merged. 

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)

2020-09-21 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=209345

--- Comment #4 from Alexander von Gluck (kallis...@unixzen.com) ---
Physical K80 board in my possession.  They go for cheap now-a-days on ebay :-)

Memory size (GDDR5): 24GB
CUDA cores: 4992
Number Of GPUs: 2x GK120 GPUs


I'll try adding the nvf2 and see what happens.   I have it in a TB3 enclosure
plugged into my Dell XPS 13, so it makes testing things pretty easy.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v17 00/20] iommu/arm-smmu + drm/msm: per-process GPU pgtables

2020-09-21 Thread Rob Clark
On Mon, Sep 21, 2020 at 2:31 PM Will Deacon  wrote:
>
> On Sat, Sep 05, 2020 at 01:04:06PM -0700, Rob Clark wrote:
> > From: Rob Clark 
> >
> > NOTE: I have re-ordered the series, and propose that we could merge this
> >   series in the following order:
> >
> >1) 01-11 - merge via drm / msm-next
> >2) 12-15 - merge via iommu, no dependency on msm-next pull req
>
> Thanks, I've queued 12-15 in the smmu tree.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates
>

Thx, let me know if you are up for trying a post-rc1 pull req for
16-18 (plus Bjorn's series if it is reposted in time).. I can
certainly help rebase/wrangle patches if that helps.. otherwise we can
try for those next cycle

BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)

2020-09-21 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=209345

Ilia Mirkin (imir...@alum.mit.edu) changed:

   What|Removed |Added

 CC||imir...@alum.mit.edu

--- Comment #3 from Ilia Mirkin (imir...@alum.mit.edu) ---
Is this a physical K80 board, or a virtual one via some sort of cloud provider?

We have nvf0 = GK110, nvf1 = what we call GK110B, but I'm not sure that's an
official name - basically the GTX 780 Ti and related Titans.

We don't have explicit support for nvf2 -- as I understand the K80 (GK210)
actually has some slight differences, e.g. more shared memory, etc... not sure
if that translates into some ctxsw fw differences or if it should just work --
you can check drivers/gpu/drm/nouveau/nvkm/engine/device/base.c, should be easy
to add in 0xf2 support based on the nvf1 if you want to play with it.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)

2020-09-21 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=209345

--- Comment #2 from Alexander von Gluck (kallis...@unixzen.com) ---
Created attachment 292555
  --> https://bugzilla.kernel.org/attachment.cgi?id=292555=edit
nvidia thunderbolt 3 attachment

logs from attachment

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 209345] [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)

2020-09-21 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=209345

--- Comment #1 from Alexander von Gluck (kallis...@unixzen.com) ---
lspci -vvn

08:00.0 0302: 10de:102d (rev ff) (prog-if ff)
!!! Unknown header type 7f
Kernel driver in use: nouveau
Kernel modules: nouveau

09:00.0 0302: 10de:102d (rev ff) (prog-if ff)
!!! Unknown header type 7f
Kernel modules: nouveau

08:00.0 3D controller: NVIDIA Corporation GK210GL [Tesla K80] (rev ff)
09:00.0 3D controller: NVIDIA Corporation GK210GL [Tesla K80] (rev ff)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 209345] New: [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla K80)

2020-09-21 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=209345

Bug ID: 209345
   Summary: [nouveau] unknown chipset (0f22d0a1) (nVidia Tesla
K80)
   Product: Drivers
   Version: 2.5
Kernel Version: 5.8.9
  Hardware: x86-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: kallis...@unixzen.com
Regression: No

Attaching an nVidia Tesla K80 compute card to a Fedora 32 machine.

Getting:
nouveau :08:00.0: unknown chipset (0f22d0a1)

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v5 05/38] drm: prime: use sgtable iterators in drm_prime_sg_to_page_addr_arrays()

2020-09-21 Thread Alex Goins
Tested-by: Alex Goins 

This change fixes a regression with drm_prime_sg_to_page_addr_arrays() and
AMDGPU in v5.9.

Commit 39913934 similarly revamped AMDGPU to use sgtable helper functions. When
it changed from dma_map_sg_attrs() to dma_map_sgtable(), as a side effect it
started correctly updating sgt->nents to the return value of dma_map_sg_attrs().
However, drm_prime_sg_to_page_addr_arrays() incorrectly uses sgt->nents to
iterate over pages, rather than sgt->orig_nents, resulting in it now returning
the incorrect number of pages on AMDGPU.

I had written a patch that changes drm_prime_sg_to_page_addr_arrays() to use
for_each_sgtable_sg() instead of for_each_sg(), iterating using sgt->orig_nents:

-   for_each_sg(sgt->sgl, sg, sgt->nents, count) {
+   for_each_sgtable_sg(sgt, sg, count) {

This patch takes it further, but still has the effect of fixing the number of
pages that drm_prime_sg_to_page_addr_arrays() returns. Something like this
should be included in v5.9 to prevent a regression with AMDGPU.

Thanks,
Alex

On Wed, 13 May 2020, Marek Szyprowski wrote:

> Replace the current hand-crafted code for extracting pages and DMA
> addresses from the given scatterlist by the much more robust
> code based on the generic scatterlist iterators and recently
> introduced sg_table-based wrappers. The resulting code is simple and
> easy to understand, so the comment describing the old code is no
> longer needed.
> 
> Signed-off-by: Marek Szyprowski 
> ---
> For more information, see '[PATCH v5 00/38] DRM: fix struct sg_table nents
> vs. orig_nents misuse' thread:
> https://lore.kernel.org/linux-iommu/20200513132114.6046-1-m.szyprow...@samsung.com/T/
> ---
>  drivers/gpu/drm/drm_prime.c | 47 
> ++---
>  1 file changed, 14 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 1d2e5fe..dfdf4d4 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -985,45 +985,26 @@ struct drm_gem_object *drm_gem_prime_import(struct 
> drm_device *dev,
>  int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page 
> **pages,
>dma_addr_t *addrs, int max_entries)
>  {
> - unsigned count;
> - struct scatterlist *sg;
> - struct page *page;
> - u32 page_len, page_index;
> - dma_addr_t addr;
> - u32 dma_len, dma_index;
> + struct sg_dma_page_iter dma_iter;
> + struct sg_page_iter page_iter;
> + struct page **p = pages;
> + dma_addr_t *a = addrs;
>  
> - /*
> -  * Scatterlist elements contains both pages and DMA addresses, but
> -  * one shoud not assume 1:1 relation between them. The sg->length is
> -  * the size of the physical memory chunk described by the sg->page,
> -  * while sg_dma_len(sg) is the size of the DMA (IO virtual) chunk
> -  * described by the sg_dma_address(sg).
> -  */
> - page_index = 0;
> - dma_index = 0;
> - for_each_sg(sgt->sgl, sg, sgt->nents, count) {
> - page_len = sg->length;
> - page = sg_page(sg);
> - dma_len = sg_dma_len(sg);
> - addr = sg_dma_address(sg);
> -
> - while (pages && page_len > 0) {
> - if (WARN_ON(page_index >= max_entries))
> + if (pages) {
> + for_each_sgtable_page(sgt, _iter, 0) {
> + if (p - pages >= max_entries)
>   return -1;
> - pages[page_index] = page;
> - page++;
> - page_len -= PAGE_SIZE;
> - page_index++;
> + *p++ = sg_page_iter_page(_iter);
>   }
> - while (addrs && dma_len > 0) {
> - if (WARN_ON(dma_index >= max_entries))
> + }
> + if (addrs) {
> + for_each_sgtable_dma_page(sgt, _iter, 0) {
> + if (a - addrs >= max_entries)
>   return -1;
> - addrs[dma_index] = addr;
> - addr += PAGE_SIZE;
> - dma_len -= PAGE_SIZE;
> - dma_index++;
> + *a++ = sg_page_iter_dma_address(_iter);
>   }
>   }
> +
>   return 0;
>  }
>  EXPORT_SYMBOL(drm_prime_sg_to_page_addr_arrays);
> -- 
> 1.9.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/i915/dp: Tweak initial dpcd backlight.enabled value

2020-09-21 Thread Lyude Paul
So if I understand this correctly, it sounds like that some Pixelbooks boot up
with DP_EDP_BACKLIGHT_BRIGHTNESS_MSB set to a non-zero value, without the
panel actually having DPCD backlight controls enabled?

If I'm understanding that correctly, then this patch looks good to me:

Reviewed-by: Lyude Paul 

On Thu, 2020-09-17 at 20:28 -0400, Sean Paul wrote:
> From: Sean Paul 
> 
> In commit 79946723092b ("drm/i915: Assume 100% brightness when not in
> DPCD control mode"), we fixed the brightness level when DPCD control was
> not active to max brightness. This is as good as we can guess since most
> backlights go on full when uncontrolled.
> 
> However in doing so we changed the semantics of the initial
> 'backlight.enabled' value. At least on Pixelbooks, they  were relying
> on the brightness level in DP_EDP_BACKLIGHT_BRIGHTNESS_MSB to be 0 on
> boot such that enabled would be false. This causes the device to be
> enabled when the brightness is set. Without this, brightness control
> doesn't work. So by changing brightness to max, we also flipped enabled
> to be true on boot.
> 
> To fix this, make enabled a function of brightness and backlight control
> mechanism.
> 
> Fixes: 79946723092b ("drm/i915: Assume 100% brightness when not in DPCD
> control mode")
> Cc: Lyude Paul 
> Cc: Jani Nikula 
> Cc: Juha-Pekka Heikkila 
> Cc: "Ville Syrjälä" 
> Cc: Rodrigo Vivi 
> Cc: Kevin Chowski >
> Signed-off-by: Sean Paul 
> ---
>  .../drm/i915/display/intel_dp_aux_backlight.c | 31 ---
>  1 file changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> index acbd7eb66cbe..036f504ac7db 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
> @@ -52,17 +52,11 @@ static void set_aux_backlight_enable(struct intel_dp
> *intel_dp, bool enable)
>   }
>  }
>  
> -/*
> - * Read the current backlight value from DPCD register(s) based
> - * on if 8-bit(MSB) or 16-bit(MSB and LSB) values are supported
> - */
> -static u32 intel_dp_aux_get_backlight(struct intel_connector *connector)
> +static bool intel_dp_aux_backlight_dpcd_mode(struct intel_connector
> *connector)
>  {
>   struct intel_dp *intel_dp = intel_attached_dp(connector);
>   struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> - u8 read_val[2] = { 0x0 };
>   u8 mode_reg;
> - u16 level = 0;
>  
>   if (drm_dp_dpcd_readb(_dp->aux,
> DP_EDP_BACKLIGHT_MODE_SET_REGISTER,
> @@ -70,15 +64,29 @@ static u32 intel_dp_aux_get_backlight(struct
> intel_connector *connector)
>   drm_dbg_kms(>drm,
>   "Failed to read the DPCD register 0x%x\n",
>   DP_EDP_BACKLIGHT_MODE_SET_REGISTER);
> - return 0;
> + return false;
>   }
>  
> + return (mode_reg & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) ==
> +DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD;
> +}
> +
> +/*
> + * Read the current backlight value from DPCD register(s) based
> + * on if 8-bit(MSB) or 16-bit(MSB and LSB) values are supported
> + */
> +static u32 intel_dp_aux_get_backlight(struct intel_connector *connector)
> +{
> + struct intel_dp *intel_dp = intel_attached_dp(connector);
> + struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> + u8 read_val[2] = { 0x0 };
> + u16 level = 0;
> +
>   /*
>* If we're not in DPCD control mode yet, the programmed brightness
>* value is meaningless and we should assume max brightness
>*/
> - if ((mode_reg & DP_EDP_BACKLIGHT_CONTROL_MODE_MASK) !=
> - DP_EDP_BACKLIGHT_CONTROL_MODE_DPCD)
> + if (!intel_dp_aux_backlight_dpcd_mode(connector))
>   return connector->panel.backlight.max;
>  
>   if (drm_dp_dpcd_read(_dp->aux, DP_EDP_BACKLIGHT_BRIGHTNESS_MSB,
> @@ -319,7 +327,8 @@ static int intel_dp_aux_setup_backlight(struct
> intel_connector *connector,
>  
>   panel->backlight.min = 0;
>   panel->backlight.level = intel_dp_aux_get_backlight(connector);
> - panel->backlight.enabled = panel->backlight.level != 0;
> + panel->backlight.enabled = intel_dp_aux_backlight_dpcd_mode(connector)
> &&
> +panel->backlight.level != 0;
>  
>   return 0;
>  }
-- 
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/3] iommu/io-pgtable-arm: Support coherency for Mali LPAE

2020-09-21 Thread Will Deacon
On Mon, Sep 21, 2020 at 10:53:23PM +0100, Robin Murphy wrote:
> On 2020-09-21 18:57, Will Deacon wrote:
> > On Wed, Sep 16, 2020 at 12:51:05AM +0100, Robin Murphy wrote:
> > > Midgard GPUs have ACE-Lite master interfaces which allows systems to
> > > integrate them in an I/O-coherent manner. It seems that from the GPU's
> > > viewpoint, the rest of the system is its outer shareable domain, and so
> > > even when snoop signals are wired up, they are only emitted for outer
> > > shareable accesses. As such, setting the TTBR_SHARE_OUTER bit does
> > > indeed get coherent pagetable walks working nicely for the coherent
> > > T620 in the Arm Juno SoC.
> > 
> > I can't help but think some of this commentary deserves to be in the code
> > as well.
> 
> Sure, if you want.

Yes, please.

> > Do you know if this sort of thing is done for other SoCs too, or is this
> > just a Juno quirk?
> 
> Yup, this is a "Midgard working as designed" thing. Juno is the coherent
> example I have to hand, but off the top of my head I believe some of the
> Exynos SoCs can also use their GPUs coherently if a switch is flipped in the
> interconnect to change routing between the CCI and a direct-to-RAM path; I
> expect there are probably further Midgard examples that I'm not aware of.
> Then there are definitely coherent Bifrost GPUs like the Amlogic S922/A311
> that prompted me to revive this patch, which we currently drive in "Legacy"
> mode and thus behave the same way as Midgard (Bifrost's "AArch64" mode
> realigns Ish and Osh with the rest of the system, and instead invents a new
> "Internal Shareable" value in between Nsh and Ish to represent the
> shareability between cores within the GPU for which Midgard hijacked Ish).

That is more than I wanted to know :) "Internal Shareable", jeez...

Thanks,

Will
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCHv4 1/6] iommu/io-pgtable-arm: Add support to use system cache

2020-09-21 Thread Will Deacon
On Mon, Sep 21, 2020 at 11:03:49PM +0100, Robin Murphy wrote:
> On 2020-09-21 19:03, Will Deacon wrote:
> > On Fri, Sep 11, 2020 at 07:57:18PM +0530, Sai Prakash Ranjan wrote:
> > > Add a quirk IO_PGTABLE_QUIRK_SYS_CACHE to override the
> > > attributes set in TCR for the page table walker when
> > > using system cache.
> > 
> > I wonder if the panfrost folks can reuse this for the issue discussed
> > over at:
> > 
> > https://lore.kernel.org/r/cover.1600213517.git.robin.mur...@arm.com
> 
> Isn't this all hinged around the outer cacheability attribute, rather than
> shareability (since these are nominally NC mappings and thus already
> properly Osh)? The Panfrost issue is just about shareability domains being a
> bit wonky; the cacheability attributes there are actually reasonably normal
> (other than not having a non-cacheable type at all, only a choice of
> allocation policies...)

Hmm, yes, this quirk _also_ changes the cacheability settings which isn't
what we need. It's a bit grotty having two different ways to configure these
TCR bits (i.e. a quirk and a format), but at least the mali format rejects
all of the quirks so I suppose it's not the end of the world.

Will
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCHv4 1/6] iommu/io-pgtable-arm: Add support to use system cache

2020-09-21 Thread Robin Murphy

On 2020-09-21 19:03, Will Deacon wrote:

On Fri, Sep 11, 2020 at 07:57:18PM +0530, Sai Prakash Ranjan wrote:

Add a quirk IO_PGTABLE_QUIRK_SYS_CACHE to override the
attributes set in TCR for the page table walker when
using system cache.


I wonder if the panfrost folks can reuse this for the issue discussed
over at:

https://lore.kernel.org/r/cover.1600213517.git.robin.mur...@arm.com


Isn't this all hinged around the outer cacheability attribute, rather 
than shareability (since these are nominally NC mappings and thus 
already properly Osh)? The Panfrost issue is just about shareability 
domains being a bit wonky; the cacheability attributes there are 
actually reasonably normal (other than not having a non-cacheable type 
at all, only a choice of allocation policies...)


Robin.


However, Sai, your email setup went wrong when you posted this so you
probably need to repost now that you have that fixed.

Will
___
iommu mailing list
io...@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/3] iommu/io-pgtable-arm: Support coherency for Mali LPAE

2020-09-21 Thread Robin Murphy

On 2020-09-21 18:57, Will Deacon wrote:

On Wed, Sep 16, 2020 at 12:51:05AM +0100, Robin Murphy wrote:

Midgard GPUs have ACE-Lite master interfaces which allows systems to
integrate them in an I/O-coherent manner. It seems that from the GPU's
viewpoint, the rest of the system is its outer shareable domain, and so
even when snoop signals are wired up, they are only emitted for outer
shareable accesses. As such, setting the TTBR_SHARE_OUTER bit does
indeed get coherent pagetable walks working nicely for the coherent
T620 in the Arm Juno SoC.


I can't help but think some of this commentary deserves to be in the code
as well.


Sure, if you want.


Do you know if this sort of thing is done for other SoCs too, or is this
just a Juno quirk?


Yup, this is a "Midgard working as designed" thing. Juno is the coherent 
example I have to hand, but off the top of my head I believe some of the 
Exynos SoCs can also use their GPUs coherently if a switch is flipped in 
the interconnect to change routing between the CCI and a direct-to-RAM 
path; I expect there are probably further Midgard examples that I'm not 
aware of. Then there are definitely coherent Bifrost GPUs like the 
Amlogic S922/A311 that prompted me to revive this patch, which we 
currently drive in "Legacy" mode and thus behave the same way as Midgard 
(Bifrost's "AArch64" mode realigns Ish and Osh with the rest of the 
system, and instead invents a new "Internal Shareable" value in between 
Nsh and Ish to represent the shareability between cores within the GPU 
for which Midgard hijacked Ish).


Robin.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/4] video: fbdev: amba-clcd: Retire elder CLCD driver

2020-09-21 Thread Linus Walleij
On Tue, Sep 15, 2020 at 3:10 AM Peter Collingbourne  wrote:
> On Tue, Jun 9, 2020 at 1:08 PM Linus Walleij  wrote:
> >
> > All the functionality in this driver has been reimplemented
> > in the new DRM driver in drivers/gpu/drm/pl111/* and all
> > the boards using it have been migrated to use the DRM driver
> > with all configuration coming from the device tree.
>
> Android's FVP configuration still uses FBDEV.

But all DRM drivers support frame buffer emulation so I don't
see how this can be a problem? The new DRM driver provides
a framebuffer.

I'm also confused about how this can be in use.
fvp-base-revc.dts includes rtsm_ve-motherboard.dtsi
where the PL111 is defined.

commit f1fe12c8bf33241e04c57a0fc5b25b16ba77ba2d
"ARM: dts: Modernize the Vexpress PL111 integration"
changes the DTS for the FVP such that only the new DRM
driver really works with it: it removes the panel-dpi encoded
panel and defines the panel
"arm,rtsm-display" for FVP and that is *only* supported
by drivers/gpu/drm/panel/panel-simple.c from DRM.

The upstream FVP has been using the new DRM driver
by default since
commit 37ad688497785c9ad1471236e28efda1e2f39741
"arm64: defconfig: Switch to PL11x DRM driver"
in january 2019.

Are you using old or outoftree devicetrees with a newer
kernel?

> While it would be great
> to see it migrated to DRM, this first requires resolving a design
> incompatibility between Android's composer and DRM devices that only
> support software rendering. I proposed a patch that would help resolve
> this [1], but there was disagreement about the approach and I haven't
> had time to get back to this.
>
> Can this please be reverted until FVP on Android can be migrated to DRM?

We need a clear technical reason, I understand that using DRM
directly might be a problem, but DRM supports full framebuffer
emulation and from a userspace point of view, what the new
driver provides should be equivalent.

I can think of problems like Android seeing the new fancy
DRM userspace ABI but surely it can be instructed to
ignore that and just use the framebuffer emulation instead?

If there is anything else making DRMs framebuffer emulation
substandard I am sure the DRM developers would like to know,
especially if it makes Android unhappy.

Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v17 00/20] iommu/arm-smmu + drm/msm: per-process GPU pgtables

2020-09-21 Thread Will Deacon
On Sat, Sep 05, 2020 at 01:04:06PM -0700, Rob Clark wrote:
> From: Rob Clark 
> 
> NOTE: I have re-ordered the series, and propose that we could merge this
>   series in the following order:
> 
>1) 01-11 - merge via drm / msm-next
>2) 12-15 - merge via iommu, no dependency on msm-next pull req

Thanks, I've queued 12-15 in the smmu tree.

https://git.kernel.org/pub/scm/linux/kernel/git/will/linux.git/log/?h=for-joerg/arm-smmu/updates

Will
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] MAINTAINERS: move Milo Kim to credits

2020-09-21 Thread Pavel Machek
Hi!

> Milo Kim's email in TI bounces with permanent error (550: Invalid
> recipient).  Last email from him on LKML was in 2017.  Move Milo Kim to
> credits and remove the separate driver entries for:
> 
>  - TI LP855x backlight driver,
>  - TI LP8727 charger driver,
>  - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers.
> 
> Signed-off-by: Krzysztof Kozlowski 

I believe normal way would be to mark the entries "orphaned", not to
drop them altogether. Plus, I believe someone from TI is likely to
step up.

Best regards,
Pavel



> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5b9621ca2b31..031adeff29cc 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17471,29 +17471,6 @@ S:   Maintained
>  F:   sound/soc/codecs/isabelle*
>  F:   sound/soc/codecs/lm49453*
>  
> -TI LP855x BACKLIGHT DRIVER
> -M:   Milo Kim 
> -S:   Maintained
> -F:   Documentation/driver-api/backlight/lp855x-driver.rst
> -F:   drivers/video/backlight/lp855x_bl.c
> -F:   include/linux/platform_data/lp855x.h
> -
> -TI LP8727 CHARGER DRIVER
> -M:   Milo Kim 
> -S:   Maintained
> -F:   drivers/power/supply/lp8727_charger.c
> -F:   include/linux/platform_data/lp8727.h
> -
> -TI LP8788 MFD DRIVER
> -M:   Milo Kim 
> -S:   Maintained
> -F:   drivers/iio/adc/lp8788_adc.c
> -F:   drivers/leds/leds-lp8788.c
> -F:   drivers/mfd/lp8788*.c
> -F:   drivers/power/supply/lp8788-charger.c
> -F:   drivers/regulator/lp8788-*.c
> -F:   include/linux/mfd/lp8788*.h
> -
>  TI NETCP ETHERNET DRIVER
>  M:   Wingman Kwok 
>  M:   Murali Karicheri 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH] MAINTAINERS: move Milo Kim to credits

2020-09-21 Thread Krzysztof Kozlowski
Milo Kim's email in TI bounces with permanent error (550: Invalid
recipient).  Last email from him on LKML was in 2017.  Move Milo Kim to
credits and remove the separate driver entries for:

 - TI LP855x backlight driver,
 - TI LP8727 charger driver,
 - TI LP8788 MFD (ADC, LEDs, charger and regulator) drivers.

Signed-off-by: Krzysztof Kozlowski 
---
 CREDITS |  3 +++
 MAINTAINERS | 23 ---
 2 files changed, 3 insertions(+), 23 deletions(-)

diff --git a/CREDITS b/CREDITS
index 1df63cdf71df..46384b11f258 100644
--- a/CREDITS
+++ b/CREDITS
@@ -1910,6 +1910,9 @@ S: 660 Harvard Ave. #7
 S: Santa Clara, CA 95051
 S: USA
 
+N: Milo Kim
+D: TI LP855x, LP8727 and LP8788 drivers
+
 N: Russell King
 E: r...@arm.linux.org.uk
 D: Linux/arm integrator, maintainer & hacker
diff --git a/MAINTAINERS b/MAINTAINERS
index 5b9621ca2b31..031adeff29cc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17471,29 +17471,6 @@ S: Maintained
 F: sound/soc/codecs/isabelle*
 F: sound/soc/codecs/lm49453*
 
-TI LP855x BACKLIGHT DRIVER
-M: Milo Kim 
-S: Maintained
-F: Documentation/driver-api/backlight/lp855x-driver.rst
-F: drivers/video/backlight/lp855x_bl.c
-F: include/linux/platform_data/lp855x.h
-
-TI LP8727 CHARGER DRIVER
-M: Milo Kim 
-S: Maintained
-F: drivers/power/supply/lp8727_charger.c
-F: include/linux/platform_data/lp8727.h
-
-TI LP8788 MFD DRIVER
-M: Milo Kim 
-S: Maintained
-F: drivers/iio/adc/lp8788_adc.c
-F: drivers/leds/leds-lp8788.c
-F: drivers/mfd/lp8788*.c
-F: drivers/power/supply/lp8788-charger.c
-F: drivers/regulator/lp8788-*.c
-F: include/linux/mfd/lp8788*.h
-
 TI NETCP ETHERNET DRIVER
 M: Wingman Kwok 
 M: Murali Karicheri 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 6/6] x86/xen: open code alloc_vm_area in arch_gnttab_valloc

2020-09-21 Thread boris . ostrovsky

On 9/18/20 12:37 PM, Christoph Hellwig wrote:
>  
> +static int gnttab_apply(pte_t *pte, unsigned long addr, void *data)
> +{
> + pte_t ***p = data;
> +
> + **p = pte;
> + (*p)++;
> + return 0;
> +}
> +
>  static int arch_gnttab_valloc(struct gnttab_vm_area *area, unsigned 
> nr_frames)
>  {
>   area->ptes = kmalloc_array(nr_frames, sizeof(*area->ptes), GFP_KERNEL);
>   if (area->ptes == NULL)
>   return -ENOMEM;
> -
> - area->area = alloc_vm_area(PAGE_SIZE * nr_frames, area->ptes);
> - if (area->area == NULL) {
> - kfree(area->ptes);
> - return -ENOMEM;
> - }
> -
> + area->area = get_vm_area(PAGE_SIZE * nr_frames, VM_IOREMAP);
> + if (!area->area)
> + goto out_free_ptes;
> + if (apply_to_page_range(_mm, (unsigned long)area->area->addr,
> + PAGE_SIZE * nr_frames, gnttab_apply, >ptes))


This will end up incrementing area->ptes pointer. So perhaps something like


pte_t **ptes = area->ptes;

if (apply_to_page_range(_mm, (unsigned long)area->area->addr,
    PAGE_SIZE * nr_frames, gnttab_apply, )) {

   ...

}


-boris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3] drm/panel: samsung: make vint_table static const

2020-09-21 Thread Linus Walleij
On Wed, Sep 16, 2020 at 3:22 PM Jason Yan  wrote:

> This eliminates the following sparse warning:
>
> drivers/gpu/drm/panel/panel-samsung-s6e3ha2.c:217:15: warning: symbol
> 'vint_table' was not declared. Should it be static?
>
> While at it, make the table const as it is never modified.
>
> Reported-by: Hulk Robot 
> Signed-off-by: Jason Yan 
> Reviewed-by: Laurent Pinchart 

Patch applied to drm-misc-next.

Yours,
Linus Walleij
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends

2020-09-21 Thread Ira Weiny
On Sat, Sep 19, 2020 at 06:39:06PM +0100, Matthew Wilcox wrote:
> On Sat, Sep 19, 2020 at 10:18:54AM -0700, Linus Torvalds wrote:
> > On Sat, Sep 19, 2020 at 2:50 AM Thomas Gleixner  wrote:
> > >
> > > this provides a preemptible variant of kmap_atomic & related
> > > interfaces. This is achieved by:
> > 
> > Ack. This looks really nice, even apart from the new capability.
> > 
> > The only thing I really reacted to is that the name doesn't make sense
> > to me: "kmap_temporary()" seems a bit odd.
> > 
> > Particularly for an interface that really is basically meant as a
> > better replacement of "kmap_atomic()" (but is perhaps also a better
> > replacement for "kmap()").
> > 
> > I think I understand how the name came about: I think the "temporary"
> > is there as a distinction from the "longterm" regular kmap(). So I
> > think it makes some sense from an internal implementation angle, but I
> > don't think it makes a lot of sense from an interface name.
> > 
> > I don't know what might be a better name, but if we want to emphasize
> > that it's thread-private and a one-off, maybe "local" would be a
> > better naming, and make it distinct from the "global" nature of the
> > old kmap() interface?
> > 
> > However, another solution might be to just use this new preemptible
> > "local" kmap(), and remove the old global one entirely. Yes, the old
> > global one caches the page table mapping and that sounds really
> > efficient and nice. But it's actually horribly horribly bad, because
> > it means that we need to use locking for them. Your new "temporary"
> > implementation seems to be fundamentally better locking-wise, and only
> > need preemption disabling as locking (and is equally fast for the
> > non-highmem case).
> > 
> > So I wonder if the single-page TLB flush isn't a better model, and
> > whether it wouldn't be a lot simpler to just get rid of the old
> > complex kmap() entirely, and replace it with this?
> > 
> > I agree we can't replace the kmap_atomic() version, because maybe
> > people depend on the preemption disabling it also implied. But what
> > about replacing the non-atomic kmap()?
> 
> My concern with that is people might use kmap() and then pass the address
> to a different task.  So we need to audit the current users of kmap()
> and convert any that do that into using vmap() instead.
> 

I've done some of this work.[3]  PKS and pmem stray write protection[2] depend
on kmap to enable the correct PKS settings.  After working through the
exception handling we realized that some users of kmap() seem to be doing just
this; passing the address to a different task.

>From what I have found ~90% of kmap() callers are 'kmap_thread()' and the other
~10% are kmap().[3]  But of those 10% I'm not familiar with the code enough to
know if they really require a 'global' map.  What I do know is they save an
address which appears to be used in other threads.  But I could be wrong.

For PKS I added a 'global' implementation which could then be called by kmap()
and added a new kmap_thread() call which used the original 'local' version of
the PKS interface.  The PKS work is still being reviewed internally for the TIP
core code.  But I've pushed it all to git hub for purposes of this
discussion.[1]

> I like kmap_local().  Or kmap_thread().

I chose kmap_thread() so that makes sense to me.  I also thought about using
kmap_global() as an alternative interface which would change just ~10% of the
callers and make the series much smaller.  But internal discussions lead me to
chose kmap_thread() as the new interface so that we don't change the semantics
of kmap().

Ira


[1] https://github.com/weiny2/linux-kernel/tree/lm-pks-pmem-for-5.10-v3

[2] https://lore.kernel.org/lkml/20200717072056.73134-1-ira.we...@intel.com/

[3]
12:42:06 > git grep ' kmap(' *.c | grep -v '* ' | wc -l
22

12:43:32 > git grep ' kmap_thread(' *.c | grep -v '* ' | wc -l
204

Here are the callers which hand an address to another thread.

12:45:25 > git grep ' kmap(' *.c | grep -v '* '
arch/x86/mm/dump_pagetables.c:  [PKMAP_BASE_NR] = { 0UL, "Persistent 
kmap() Area" },
drivers/firewire/net.c: ptr = kmap(dev->broadcast_rcv_buffer.pages[u]);
drivers/gpu/drm/i915/gem/i915_gem_pages.c:  return 
kmap(sg_page(sgt->sgl));
drivers/gpu/drm/i915/selftests/i915_perf.c: scratch = 
kmap(ce->vm->scratch[0].base.page);
drivers/gpu/drm/ttm/ttm_bo_util.c:  map->virtual = kmap(map->page);
drivers/infiniband/hw/qib/qib_user_sdma.c:  mpage = kmap(page);
drivers/misc/vmw_vmci/vmci_host.c:  context->notify = 
kmap(context->notify_page) + (uva & (PAGE_SIZE - 1));
drivers/misc/xilinx_sdfec.c:addr = kmap(pages[i]);
drivers/mmc/host/usdhi6rol0.c:  host->pg.mapped = kmap(host->pg.page);
drivers/mmc/host/usdhi6rol0.c:  host->pg.mapped = kmap(host->pg.page);
drivers/mmc/host/usdhi6rol0.c:  host->pg.mapped = kmap(host->pg.page);
drivers/nvme/target/tcp.c:  iov->iov_base = 

Re: [PATCH v1 ] drm/amd/pm: Removed fixed clock in auto mode DPM

2020-09-21 Thread Alex Deucher
Applied with fixed up whitespace.

Thanks,

Alex

On Tue, Sep 15, 2020 at 3:45 AM Christian König
 wrote:
>
> Am 15.09.20 um 09:18 schrieb Sudheesh Mavila:
> >  SMU10_UMD_PSTATE_PEAK_FCLK value should not be used to set the DPM.
> >
> >  Change  suggested by evan.q...@amd.com
>
> Can't say much about the change itself, but the Commit message is
> indented and the indentation in the code doesn't look consistent either.
>
> Christian.
>
> >
> > Signed-off-by: Sudheesh Mavila 
> > ---
> >   drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c | 10 ++
> >   1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c 
> > b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> > index c9cfe90a2947..081cb9b1b7c8 100644
> > --- a/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> > +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/smu10_hwmgr.c
> > @@ -566,6 +566,8 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
> > *hwmgr,
> >   struct smu10_hwmgr *data = hwmgr->backend;
> >   uint32_t min_sclk = hwmgr->display_config->min_core_set_clock;
> >   uint32_t min_mclk = hwmgr->display_config->min_mem_set_clock/100;
> > + uint32_t index_fclk = data->clock_vol_info.vdd_dep_on_fclk->count - 1;
> > + uint32_t index_socclk = data->clock_vol_info.vdd_dep_on_socclk->count 
> > - 1;
> >
> >   if (hwmgr->smu_version < 0x1E3700) {
> >   pr_info("smu firmware version too old, can not set dpm 
> > level\n");
> > @@ -679,13 +681,13 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
> > *hwmgr,
> >   smum_send_msg_to_smc_with_parameter(hwmgr,
> >   
> > PPSMC_MSG_SetHardMinFclkByFreq,
> >   
> > hwmgr->display_config->num_display > 3 ?
> > - SMU10_UMD_PSTATE_PEAK_FCLK :
> > + 
> > data->clock_vol_info.vdd_dep_on_fclk->entries[0].clk :
> >   min_mclk,
> >   NULL);
> >
> >   smum_send_msg_to_smc_with_parameter(hwmgr,
> >   
> > PPSMC_MSG_SetHardMinSocclkByFreq,
> > - SMU10_UMD_PSTATE_MIN_SOCCLK,
> > + 
> > data->clock_vol_info.vdd_dep_on_socclk->entries[0].clk,
> >   NULL);
> >   smum_send_msg_to_smc_with_parameter(hwmgr,
> >   PPSMC_MSG_SetHardMinVcn,
> > @@ -698,11 +700,11 @@ static int smu10_dpm_force_dpm_level(struct pp_hwmgr 
> > *hwmgr,
> >   NULL);
> >   smum_send_msg_to_smc_with_parameter(hwmgr,
> >   
> > PPSMC_MSG_SetSoftMaxFclkByFreq,
> > - SMU10_UMD_PSTATE_PEAK_FCLK,
> > + 
> > data->clock_vol_info.vdd_dep_on_fclk->entries[index_fclk].clk,
> >   NULL);
> >   smum_send_msg_to_smc_with_parameter(hwmgr,
> >   
> > PPSMC_MSG_SetSoftMaxSocclkByFreq,
> > - SMU10_UMD_PSTATE_PEAK_SOCCLK,
> > + 
> > data->clock_vol_info.vdd_dep_on_socclk->entries[index_socclk].clk,
> >   NULL);
> >   smum_send_msg_to_smc_with_parameter(hwmgr,
> >   PPSMC_MSG_SetSoftMaxVcn,
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/amd/display: optimize code runtime a bit

2020-09-21 Thread Alex Deucher
On Mon, Sep 21, 2020 at 9:14 AM Bernard Zhao  wrote:
>
> Static function dal_ddc_i2c_payloads_destroy is only called
> in dal_ddc_service_query_ddc_data, the parameter is 
> , there is no point NULL risk, so no need to check.
> This change is to make the code run a bit fast.
>

How about just getting rid of dal_ddc_i2c_payloads_destroy() and just
call dal_vector_destruct() directly.

Alex


> Signed-off-by: Bernard Zhao 
> ---
>  drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c 
> b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> index b984eecca58b..6dcc666738fc 100644
> --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_ddc.c
> @@ -150,9 +150,6 @@ static uint32_t dal_ddc_i2c_payloads_get_count(struct 
> i2c_payloads *p)
>
>  static void dal_ddc_i2c_payloads_destroy(struct i2c_payloads *p)
>  {
> -   if (!p)
> -   return;
> -
> dal_vector_destruct(>payloads);
>  }
>
> --
> 2.28.0
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next] drm/amdgpu/gmc9: simplify the return expression of gmc_v9_0_suspend

2020-09-21 Thread Alex Deucher
Applied.  Thanks!

Alex

On Mon, Sep 21, 2020 at 9:14 AM Liu Shixin  wrote:
>
> Simplify the return expression.
>
> Signed-off-by: Liu Shixin 
> ---
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 5400cac02087..cb9e9e5afa5a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -1683,14 +1683,9 @@ static int gmc_v9_0_hw_fini(void *handle)
>
>  static int gmc_v9_0_suspend(void *handle)
>  {
> -   int r;
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> -   r = gmc_v9_0_hw_fini(adev);
> -   if (r)
> -   return r;
> -
> -   return 0;
> +   return gmc_v9_0_hw_fini(adev);
>  }
>
>  static int gmc_v9_0_resume(void *handle)
> --
> 2.25.1
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next] drm/amd/pm: simplify the return expression of smu_hw_fini

2020-09-21 Thread Alex Deucher
Applied.  Thanks!

Alex

On Mon, Sep 21, 2020 at 9:14 AM Liu Shixin  wrote:
>
> Simplify the return expression.
>
> Signed-off-by: Liu Shixin 
> ---
>  drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c 
> b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> index 5c4b74f964fc..3612841d40dc 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
> @@ -1214,7 +1214,6 @@ static int smu_hw_fini(void *handle)
>  {
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
> struct smu_context *smu = >smu;
> -   int ret = 0;
>
> if (amdgpu_sriov_vf(adev)&& !amdgpu_sriov_is_pp_one_vf(adev))
> return 0;
> @@ -1230,11 +1229,7 @@ static int smu_hw_fini(void *handle)
>
> adev->pm.dpm_enabled = false;
>
> -   ret = smu_smc_hw_cleanup(smu);
> -   if (ret)
> -   return ret;
> -
> -   return 0;
> +   return smu_smc_hw_cleanup(smu);
>  }
>
>  int smu_reset(struct smu_context *smu)
> --
> 2.25.1
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next] drm/amdgpu: simplify the return expression

2020-09-21 Thread Alex Deucher
Applied.  Thanks!

Alex

On Mon, Sep 21, 2020 at 9:14 AM Qinglang Miao  wrote:
>
> Simplify the return expression.
>
> Signed-off-by: Qinglang Miao 
> ---
>  drivers/gpu/drm/amd/amdgpu/cik_ih.c   |  7 +--
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 10 ++
>  2 files changed, 3 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/cik_ih.c 
> b/drivers/gpu/drm/amd/amdgpu/cik_ih.c
> index 401c99f0b..db953e95f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/cik_ih.c
> +++ b/drivers/gpu/drm/amd/amdgpu/cik_ih.c
> @@ -316,14 +316,9 @@ static int cik_ih_sw_fini(void *handle)
>
>  static int cik_ih_hw_init(void *handle)
>  {
> -   int r;
> struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>
> -   r = cik_ih_irq_init(adev);
> -   if (r)
> -   return r;
> -
> -   return 0;
> +   return cik_ih_irq_init(adev);
>  }
>
>  static int cik_ih_hw_fini(void *handle)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 20d8a03ca..56ed108b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -2198,7 +2198,6 @@ static int gfx_v9_0_gpu_early_init(struct amdgpu_device 
> *adev)
>  static int gfx_v9_0_compute_ring_init(struct amdgpu_device *adev, int 
> ring_id,
>   int mec, int pipe, int queue)
>  {
> -   int r;
> unsigned irq_type;
> struct amdgpu_ring *ring = >gfx.compute_ring[ring_id];
> unsigned int hw_prio;
> @@ -2223,13 +,8 @@ static int gfx_v9_0_compute_ring_init(struct 
> amdgpu_device *adev, int ring_id,
> hw_prio = amdgpu_gfx_is_high_priority_compute_queue(adev, 
> ring->queue) ?
> AMDGPU_GFX_PIPE_PRIO_HIGH : 
> AMDGPU_GFX_PIPE_PRIO_NORMAL;
> /* type-2 packets are deprecated on MEC, use type-3 instead */
> -   r = amdgpu_ring_init(adev, ring, 1024,
> ->gfx.eop_irq, irq_type, hw_prio);
> -   if (r)
> -   return r;
> -
> -
> -   return 0;
> +   return amdgpu_ring_init(adev, ring, 1024,
> +   >gfx.eop_irq, irq_type, hw_prio);
>  }
>
>  static int gfx_v9_0_sw_init(void *handle)
> --
> 2.23.0
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next] drm/amdgpu/mes: simplify the return expression of mes_v10_1_ring_init

2020-09-21 Thread Alex Deucher
Applied.  Thanks!

Alex

On Mon, Sep 21, 2020 at 9:14 AM Qinglang Miao  wrote:
>
> Simplify the return expression.
>
> Signed-off-by: Qinglang Miao 
> ---
>  drivers/gpu/drm/amd/amdgpu/mes_v10_1.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c 
> b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> index 4b746584a..1c22d8393 100644
> --- a/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/mes_v10_1.c
> @@ -832,7 +832,6 @@ static int mes_v10_1_queue_init(struct amdgpu_device 
> *adev)
>  static int mes_v10_1_ring_init(struct amdgpu_device *adev)
>  {
> struct amdgpu_ring *ring;
> -   int r;
>
> ring = >mes.ring;
>
> @@ -849,11 +848,7 @@ static int mes_v10_1_ring_init(struct amdgpu_device 
> *adev)
> ring->no_scheduler = true;
> sprintf(ring->name, "mes_%d.%d.%d", ring->me, ring->pipe, 
> ring->queue);
>
> -   r = amdgpu_ring_init(adev, ring, 1024, NULL, 0, 
> AMDGPU_RING_PRIO_DEFAULT);
> -   if (r)
> -   return r;
> -
> -   return 0;
> +   return amdgpu_ring_init(adev, ring, 1024, NULL, 0, 
> AMDGPU_RING_PRIO_DEFAULT);
>  }
>
>  static int mes_v10_1_mqd_sw_init(struct amdgpu_device *adev)
> --
> 2.23.0
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[Bug 207901] Nouveau: In a 4 monitor setup, 1-2 displays remains black after boot

2020-09-21 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=207901

--- Comment #30 from Lyude Paul (ly...@redhat.com) ---
Hi! Sorry for the long wait, I got distracted with some other work that needed
to be done. Also, sorry for making you buy that adapter since it looks like
things aren't working still. So I took a look at the logs that you gave me and
noticed some interesting things that didn't catch my eye before. In the last
log you gave me with the new DP adapter I can't see nouveau noticing any of the
monitors. On the log before that though, I actually see it detect all four
monitors. I also see it actually turn all four on successfully, huh.

The other thing I noticed is that it looks like you're using the nouveau DDX in
your X config. That makes me wonder, do the screens come up -at all- during the
boot process before the GUI comes up? As well, have you tried the modesetting
DDX to see if that makes any difference, or perhaps wayland?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/6] zsmalloc: switch from alloc_vm_area to get_vm_area

2020-09-21 Thread Minchan Kim
On Mon, Sep 21, 2020 at 08:17:08PM +0200, Christoph Hellwig wrote:
> On Mon, Sep 21, 2020 at 10:42:56AM -0700, Minchan Kim wrote:
> > IIRC, the problem was runtime pte popluating needs GFP_KERNEL but
> > zs_map_object API runs under non-preemtible section.
> 
> Make sense.
> 
> > > - area->vm = alloc_vm_area(PAGE_SIZE * 2, NULL);
> > > + area->vm = get_vm_area(PAGE_SIZE * 2, 0);
> > >   if (!area->vm)
> > >   return -ENOMEM;
> > >   return 0;
> > 
> > I think it shoud work.
> > 
> > diff --git a/mm/memory.c b/mm/memory.c
> > index 05789aa4af12..6a1e4d854593 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -2232,7 +2232,7 @@ static int apply_to_pte_range(struct mm_struct *mm, 
> > pmd_t *pmd,
> > arch_enter_lazy_mmu_mode();
> >  
> > do {
> > -   if (create || !pte_none(*pte)) {
> > +   if ((create || !pte_none(*pte)) && fn) {
> > err = fn(pte++, addr, data);
> > if (err)
> > break;
> > diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> > index 3e4fe3259612..9ef7daf3d279 100644
> > --- a/mm/zsmalloc.c
> > +++ b/mm/zsmalloc.c
> > @@ -1116,6 +1116,8 @@ static struct zspage *find_get_zspage(struct 
> > size_class *class)
> >  #ifdef CONFIG_ZSMALLOC_PGTABLE_MAPPING
> >  static inline int __zs_cpu_up(struct mapping_area *area)
> >  {
> > +   int ret;
> > +
> > /*
> >  * Make sure we don't leak memory if a cpu UP notification
> >  * and zs_init() race and both call zs_cpu_up() on the same cpu
> > @@ -1125,7 +1127,13 @@ static inline int __zs_cpu_up(struct mapping_area 
> > *area)
> > area->vm = get_vm_area(PAGE_SIZE * 2, 0);
> > if (!area->vm)
> > return -ENOMEM;
> > -   return 0;
> > +
> > +   /*
> > +* Populate ptes in advance to avoid pte allocation with GFP_KERNEL
> > +* in non-preemtible context of zs_map_object.
> > +*/
> > +   ret = apply_to_page_range(_mm, NULL, PAGE_SIZE * 2, NULL, NULL);
> > +   return ret;
> 
> I think this needs the addr from the vm area somewhere..

Yeah, let's assign the addres we got get_vm_area.

> 
> We probaby want to add a trivial helper to prefault an area instead of
> the open coded variant.

It seems zsmalloc is only customer the function so let's have the helper
when we see another customer.

If we don't have objection, I'd like to ask to Andrew fold this up.

---
 mm/memory.c   | 2 +-
 mm/zsmalloc.c | 8 +++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 05789aa4af12..6a1e4d854593 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2232,7 +2232,7 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t 
*pmd,
arch_enter_lazy_mmu_mode();
 
do {
-   if (create || !pte_none(*pte)) {
+   if ((create || !pte_none(*pte)) && fn) {
err = fn(pte++, addr, data);
if (err)
break;
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 3e4fe3259612..918c7b019b3d 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1125,7 +1125,13 @@ static inline int __zs_cpu_up(struct mapping_area *area)
area->vm = get_vm_area(PAGE_SIZE * 2, 0);
if (!area->vm)
return -ENOMEM;
-   return 0;
+
+   /*
+* Populate ptes in advance to avoid pte allocation with GFP_KERNEL
+* in non-preemtible context of zs_map_object.
+*/
+   return apply_to_page_range(_mm, (unsigned long)area->vm->addr,
+   PAGE_SIZE * 2, NULL, NULL);
 }
 
 static inline void __zs_cpu_down(struct mapping_area *area)
-- 
2.28.0.681.g6f77f65b4e-goog

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/dp: start using more of the extended receiver caps

2020-09-21 Thread Jani Nikula
On Mon, 21 Sep 2020, Lyude Paul  wrote:
> On Tue, 2020-09-01 at 21:01 +0300, Jani Nikula wrote:
>> I guess we can try without the change, and fix later if we hit issues.
>
> I'm fine with the change if it doesn't break things btw - just as long as
> we're making sure that we don't zero things out by accident

My conclusion was to go without, it's a trivial change to add
afterwards as needed.

BR,
Jani.




-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/dp: start using more of the extended receiver caps

2020-09-21 Thread Lyude Paul
On Tue, 2020-09-01 at 21:01 +0300, Jani Nikula wrote:
> On Tue, 01 Sep 2020, Lyude Paul  wrote:
> > On Tue, 2020-09-01 at 15:32 +0300, Jani Nikula wrote:
> > > In the future, we'll be needing more of the extended receiver capability
> > > field starting at DPCD address 0x2200. (Specifically, we'll need main
> > > link channel coding cap for DP 2.0.) Start using it now to not miss out
> > > later on.
> > > 
> > > Cc: Lyude Paul 
> > > Signed-off-by: Jani Nikula 
> > > 
> > > ---
> > > 
> > > I guess this can be merged after the topic branch to drm-misc-next or
> > > so, but I'd prefer to have this fairly early on to catch any potential
> > > issues.
> > > ---
> > >  drivers/gpu/drm/drm_dp_helper.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > > b/drivers/gpu/drm/drm_dp_helper.c
> > > index 1e7c638873c8..3a3c238452df 100644
> > > --- a/drivers/gpu/drm/drm_dp_helper.c
> > > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > > @@ -436,7 +436,7 @@ static u8 drm_dp_downstream_port_count(const u8
> > > dpcd[DP_RECEIVER_CAP_SIZE])
> > >  static int drm_dp_read_extended_dpcd_caps(struct drm_dp_aux *aux,
> > > u8 dpcd[DP_RECEIVER_CAP_SIZE])
> > >  {
> > > - u8 dpcd_ext[6];
> > > + u8 dpcd_ext[DP_RECEIVER_CAP_SIZE];
> > 
> > Not 100% sure this is right? It's not clear at first glance of the 2.0
> > spec, but
> > my assumption would be that on < DP2.0 devices that everything but those
> > first 6
> > bytes are zeroed out in the extended DPRX field. Since we memcpy()
> > dpcd_ext
> > using sizeof(dpcd_ext), we'd potentially end up zeroing out all of the
> > DPCD caps
> > that comes after those 6 bytes.
> 
> Re-reading stuff... AFAICT everything in 0x2200..0x220F should be
> valid. They should match what's in 0x..0x000F except for 0x,
> 0x0001, and 0x0005, for backwards compatibility.
> 
> Apparently there are no such backwards compatibility concerns with the
> other receiver cap fields then.
> 
> But it gives me an uneasy feeling that many places in the spec refer to
> 0x2200+ even though they should per spec be the same in 0x+.
> 
> I guess we can try without the change, and fix later if we hit issues.

I'm fine with the change if it doesn't break things btw - just as long as
we're making sure that we don't zero things out by accident
> 
> 
> BR,
> Jani.
> 
-- 
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3] drm/dp_mst: Retrieve extended DPCD caps for topology manager

2020-09-21 Thread Lyude Paul
Hi, sorry I lost track of this until just now. Comments down below:

On Fri, 2020-09-11 at 11:44 +0800, Koba Ko wrote:
> As per DP-1.3, First check DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT.
> If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 1, read the DP_DP13_DPCD_REV to
> get the faster capability.
> If DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT is 0, read DP_DPCD_REV.
> 
> Signed-off-by: Koba Ko 
> Reviewed-by: Lyude Paul 
> ---
> Changelog:
> 1. Adjust the commit message.
> 2. use drm_dbg_kms instead and print the return code.
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 14 +++---
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 7753c718ddf9..63f8809b9aa4 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3671,8 +3671,6 @@ EXPORT_SYMBOL(drm_dp_read_mst_cap);
>  int drm_dp_mst_topology_mgr_set_mst(struct drm_dp_mst_topology_mgr *mgr,
> bool mst_state)
>  {
>   int ret = 0;
> - u8 dpcd_ext = 0;
> - unsigned int dpcd_offset = 0;
>   struct drm_dp_mst_branch *mstb = NULL;
>  
>   mutex_lock(>payload_lock);
> @@ -3686,17 +3684,11 @@ int drm_dp_mst_topology_mgr_set_mst(struct
> drm_dp_mst_topology_mgr *mgr, bool ms
>   struct drm_dp_payload reset_pay;
>  
>   WARN_ON(mgr->mst_primary);
> - drm_dp_dpcd_read(mgr->aux,
> -  DP_TRAINING_AUX_RD_INTERVAL,
> -  _ext, sizeof(dpcd_ext));
> -
> - dpcd_offset =
> - ((dpcd_ext & DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT)
> ?  DP_DP13_DPCD_REV : DP_DPCD_REV);


Uh, are you sure you formatted this patch correctly? None of these hunks are 
present upstream, this looks like it's a diff for the previous version of this 
patch that you sent out
>  
>   /* get dpcd info */
> - ret = drm_dp_dpcd_read(mgr->aux, dpcd_offset, mgr->dpcd,
> DP_RECEIVER_CAP_SIZE);
> - if (ret != DP_RECEIVER_CAP_SIZE) {
> - DRM_DEBUG_KMS("failed to read DPCD\n");
> + ret = drm_dp_read_dpcd_caps(mgr->aux, mgr->dpcd);
> + if (ret < 0) {
> + drm_dbg_kms(mgr->dev, "failed to read DPCD, ret %d\n",
> ret);
>   goto out_unlock;
>   }
>  
-- 
Cheers,
Lyude Paul (she/her)
Software Engineer at Red Hat

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCHv4 1/6] iommu/io-pgtable-arm: Add support to use system cache

2020-09-21 Thread Will Deacon
On Fri, Sep 11, 2020 at 07:57:18PM +0530, Sai Prakash Ranjan wrote:
> Add a quirk IO_PGTABLE_QUIRK_SYS_CACHE to override the
> attributes set in TCR for the page table walker when
> using system cache.

I wonder if the panfrost folks can reuse this for the issue discussed
over at:

https://lore.kernel.org/r/cover.1600213517.git.robin.mur...@arm.com

However, Sai, your email setup went wrong when you posted this so you
probably need to repost now that you have that fixed.

Will
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/3] iommu/io-pgtable-arm: Support coherency for Mali LPAE

2020-09-21 Thread Will Deacon
On Wed, Sep 16, 2020 at 12:51:05AM +0100, Robin Murphy wrote:
> Midgard GPUs have ACE-Lite master interfaces which allows systems to
> integrate them in an I/O-coherent manner. It seems that from the GPU's
> viewpoint, the rest of the system is its outer shareable domain, and so
> even when snoop signals are wired up, they are only emitted for outer
> shareable accesses. As such, setting the TTBR_SHARE_OUTER bit does
> indeed get coherent pagetable walks working nicely for the coherent
> T620 in the Arm Juno SoC.

I can't help but think some of this commentary deserves to be in the code
as well.

Do you know if this sort of thing is done for other SoCs too, or is this
just a Juno quirk?

Will
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/6] zsmalloc: switch from alloc_vm_area to get_vm_area

2020-09-21 Thread Minchan Kim
On Fri, Sep 18, 2020 at 06:37:19PM +0200, Christoph Hellwig wrote:
> There is no obvious reason why zsmalloc needs to pre-fault the PTEs
> given that it later uses map_kernel_range to just like vmap().

IIRC, the problem was runtime pte popluating needs GFP_KERNEL but
zs_map_object API runs under non-preemtible section.

> 
> Signed-off-by: Christoph Hellwig 
> ---
>  mm/zsmalloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
> index c36fdff9a37131..3e4fe3259612fd 100644
> --- a/mm/zsmalloc.c
> +++ b/mm/zsmalloc.c
> @@ -1122,7 +1122,7 @@ static inline int __zs_cpu_up(struct mapping_area *area)
>*/
>   if (area->vm)
>   return 0;
> - area->vm = alloc_vm_area(PAGE_SIZE * 2, NULL);
> + area->vm = get_vm_area(PAGE_SIZE * 2, 0);
>   if (!area->vm)
>   return -ENOMEM;
>   return 0;

I think it shoud work.

diff --git a/mm/memory.c b/mm/memory.c
index 05789aa4af12..6a1e4d854593 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2232,7 +2232,7 @@ static int apply_to_pte_range(struct mm_struct *mm, pmd_t 
*pmd,
arch_enter_lazy_mmu_mode();
 
do {
-   if (create || !pte_none(*pte)) {
+   if ((create || !pte_none(*pte)) && fn) {
err = fn(pte++, addr, data);
if (err)
break;
diff --git a/mm/zsmalloc.c b/mm/zsmalloc.c
index 3e4fe3259612..9ef7daf3d279 100644
--- a/mm/zsmalloc.c
+++ b/mm/zsmalloc.c
@@ -1116,6 +1116,8 @@ static struct zspage *find_get_zspage(struct size_class 
*class)
 #ifdef CONFIG_ZSMALLOC_PGTABLE_MAPPING
 static inline int __zs_cpu_up(struct mapping_area *area)
 {
+   int ret;
+
/*
 * Make sure we don't leak memory if a cpu UP notification
 * and zs_init() race and both call zs_cpu_up() on the same cpu
@@ -1125,7 +1127,13 @@ static inline int __zs_cpu_up(struct mapping_area *area)
area->vm = get_vm_area(PAGE_SIZE * 2, 0);
if (!area->vm)
return -ENOMEM;
-   return 0;
+
+   /*
+* Populate ptes in advance to avoid pte allocation with GFP_KERNEL
+* in non-preemtible context of zs_map_object.
+*/
+   ret = apply_to_page_range(_mm, NULL, PAGE_SIZE * 2, NULL, NULL);
+   return ret;
 }
 
 static inline void __zs_cpu_down(struct mapping_area *area)

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next] drm: omapdrm: dss: simplify the return expression of hdmi_init_pll_data

2020-09-21 Thread Tomi Valkeinen
Hi,

On 21/09/2020 16:10, Qinglang Miao wrote:
> Simplify the return expression.
> 
> Signed-off-by: Qinglang Miao 
> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi_pll.c | 7 +--
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_pll.c 
> b/drivers/gpu/drm/omapdrm/dss/hdmi_pll.c
> index cf2b000f3..c3e85b636 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi_pll.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi_pll.c
> @@ -131,7 +131,6 @@ static int hdmi_init_pll_data(struct dss_device *dss,
>  {
>   struct dss_pll *pll = >pll;
>   struct clk *clk;
> - int r;
>  
>   clk = devm_clk_get(>dev, "sys_clk");
>   if (IS_ERR(clk)) {
> @@ -151,11 +150,7 @@ static int hdmi_init_pll_data(struct dss_device *dss,
>  
>   pll->ops = _pll_ops;
>  
> - r = dss_pll_register(dss, pll);
> - if (r)
> - return r;
> -
> - return 0;
> + return dss_pll_register(dss, pll);
>  }
>  
>  int hdmi_pll_init(struct dss_device *dss, struct platform_device *pdev,
> 

I like it more when there's a return 0 at the end of the function, especially 
in functions where
there are multiple cases of if(...) return r. It's more easily readable, at 
least to my eyes.

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel

2020-09-21 Thread Fabio Estevam
Hi Guido,

On Mon, Sep 21, 2020 at 1:56 PM Guido Günther  wrote:
>
> We need to reset both for the panel to show an image.

There is a typo in "resets" in the Subject line.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC PATCH v1 1/3] drm/panel: mantix: Don't dereference NULL mode

2020-09-21 Thread Guido Günther
Don't dereference mode which was just NULL checked.

Signed-off-by: Guido Günther 
Reported-by: Dan Carpenter 
---
 drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c 
b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
index 3482e28e30fc..4a7fbf64bb7a 100644
--- a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
+++ b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
@@ -204,7 +204,7 @@ static int mantix_get_modes(struct drm_panel *panel,
if (!mode) {
dev_err(ctx->dev, "Failed to add mode %ux%u@%u\n",
default_mode.hdisplay, default_mode.vdisplay,
-   drm_mode_vrefresh(mode));
+   drm_mode_vrefresh(_mode));
return -ENOMEM;
}
 
-- 
2.26.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC PATCH v1 3/3] dt-binding: display: Require two rests on mantix panel

2020-09-21 Thread Guido Günther
We need to reset both for the panel to show an image.

Signed-off-by: Guido Günther 
---
 .../bindings/display/panel/mantix,mlaf057we51-x.yaml   | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml 
b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
index 937323cc9aaa..ba5a18fac9f9 100644
--- a/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
+++ b/Documentation/devicetree/bindings/display/panel/mantix,mlaf057we51-x.yaml
@@ -35,7 +35,9 @@ properties:
   vddi-supply:
 description: 1.8V I/O voltage supply
 
-  reset-gpios: true
+  reset-gpios:
+minItems: 2
+maxItems: 2
 
   backlight: true
 
@@ -62,7 +64,8 @@ examples:
 avdd-supply = <_avdd>;
 avee-supply = <_avee>;
 vddi-supply = <_1v8_p>;
-reset-gpios = < 29 GPIO_ACTIVE_LOW>;
+reset-gpios = < 29 GPIO_ACTIVE_LOW>,
+  < 24 GPIO_ACTIVE_LOW>;
 backlight = <>;
 };
 };
-- 
2.26.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC PATCH v1 2/3] drm/panel: mantix: Fix panel reset

2020-09-21 Thread Guido Günther
The mantix panel needs two reset lines (RESX and TP_RSTN) deasserted to
output an image. Only deasserting RESX is not enough and the display
will stay blank. Deassert in prepare() and assert in unprepare() to keep
device held in reset when off.

Signed-off-by: Guido Günther 
---
 .../gpu/drm/panel/panel-mantix-mlaf057we51.c  | 37 ---
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c 
b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
index 4a7fbf64bb7a..29d28c63bb72 100644
--- a/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
+++ b/drivers/gpu/drm/panel/panel-mantix-mlaf057we51.c
@@ -23,10 +23,13 @@
 #define MANTIX_CMD_OTP_STOP_RELOAD_MIPI 0x41
 #define MANTIX_CMD_INT_CANCEL   0x4C
 
+#define MANTIX_NUM_RESETS 2
+
 struct mantix {
struct device *dev;
struct drm_panel panel;
-   struct gpio_desc *reset_gpio;
+   /* RESX and TP_RSTN */
+   struct gpio_descs *reset_gpios;
 
struct regulator *avdd;
struct regulator *avee;
@@ -122,8 +125,13 @@ static int mantix_disable(struct drm_panel *panel)
 
 static int mantix_unprepare(struct drm_panel *panel)
 {
+   DECLARE_BITMAP(asserted_values, BITS_PER_TYPE(1));
struct mantix *ctx = panel_to_mantix(panel);
 
+   gpiod_set_array_value_cansleep(ctx->reset_gpios->ndescs,
+  ctx->reset_gpios->desc,
+  ctx->reset_gpios->info, asserted_values);
+
regulator_disable(ctx->avee);
regulator_disable(ctx->avdd);
/* T11 */
@@ -137,6 +145,7 @@ static int mantix_unprepare(struct drm_panel *panel)
 
 static int mantix_prepare(struct drm_panel *panel)
 {
+   DECLARE_BITMAP(deasserted_values, BITS_PER_TYPE(0));
struct mantix *ctx = panel_to_mantix(panel);
int ret;
 
@@ -165,14 +174,11 @@ static int mantix_prepare(struct drm_panel *panel)
return ret;
}
 
-   /* T3+T5 */
-   usleep_range(1, 12000);
-
-   gpiod_set_value_cansleep(ctx->reset_gpio, 1);
-   usleep_range(5150, 7000);
-
-   gpiod_set_value_cansleep(ctx->reset_gpio, 0);
-
+   /* T3 + T4 + time for voltage to become stable: */
+   usleep_range(6000, 7000);
+   gpiod_set_array_value_cansleep(ctx->reset_gpios->ndescs,
+  ctx->reset_gpios->desc,
+  ctx->reset_gpios->info, 
deasserted_values);
/* T6 */
msleep(50);
 
@@ -236,10 +242,15 @@ static int mantix_probe(struct mipi_dsi_device *dsi)
if (!ctx)
return -ENOMEM;
 
-   ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
-   if (IS_ERR(ctx->reset_gpio)) {
-   dev_err(dev, "cannot get reset gpio\n");
-   return PTR_ERR(ctx->reset_gpio);
+   ctx->reset_gpios = devm_gpiod_get_array(dev, "reset", GPIOD_OUT_HIGH);
+   if (IS_ERR(ctx->reset_gpios)) {
+   dev_err(dev, "cannot get reset gpios\n");
+   return PTR_ERR(ctx->reset_gpios);
+   }
+
+   if (ctx->reset_gpios->ndescs != MANTIX_NUM_RESETS) {
+   dev_err(dev, "Need exactly %d reset-gpios\n", 
MANTIX_NUM_RESETS);
+   return -EINVAL;
}
 
mipi_dsi_set_drvdata(dsi, ctx);
-- 
2.26.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[RFC PATCH v1 0/3] drm/panel: mantix panel reset fixes

2020-09-21 Thread Guido Günther

Posting as RFC since I'm not sure how to handle the bindings, please see below.

The first patch in this series fixes dereferencing a NULL mode in the error
path. The second one extends the resets to not only reset RESX but also TP_RSTN
since otherwise the display will stay completely blank. I didn't spot that
before initial submission since the reset line was bound to the touch
controller and although that failed to probe it came far enough to deassert the
reset line (at an arbitrary point in time during boot) and hence we got a
picture. Since touch and panel are on the same IC they're not completely
independent and i might have to turn the whole thing into an MFD at some point
but this series gets the panel to a reliably working state on boot and on fb
blank/unblank.

Since the reset-gpios are active low we can deassert in prepare and assert in
unprepare simplifying the code making sure lines are kept low when the
panel is off.

As for the bindings it seems I can't override the maxItems of reset-gpios since
that is already set in bindings/display/panel/panel-common.yaml to maxItems:
1`.  Is there a way to do that instead of side stepping the issue and using
a different name or using enable-gpios for one of them or forcing everyone else
to set `maxItems: 1`?

The binding were not part of a stable kernel so I hope it's okay to not worry
about backward compatibility.

Guido Günther (3):
  drm/panel: mantix: Don't dereference NULL mode
  drm/panel: mantix: Fix panel reset
  dt-binding: display: Require two rests on mantix panel

 .../display/panel/mantix,mlaf057we51-x.yaml   |  7 +++-
 .../gpu/drm/panel/panel-mantix-mlaf057we51.c  | 39 ---
 2 files changed, 30 insertions(+), 16 deletions(-)

-- 
2.26.2

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 9/9] drm/mediatek: reduce clear event

2020-09-21 Thread Matthias Brugger



On 08/07/2020 01:01, Chun-Kuang Hu wrote:

Hi, Dennis:

Dennis YC Hsieh  於 2020年7月7日 週二 下午11:47寫道:


No need to clear event again since event always clear before wait.
This fix depend on patch:
   "soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api"


Acked-by: Chun-Kuang Hu 



Pushed now to v5.9-next/soc

Thanks!



Fixes: 2f965be7f9008 ("drm/mediatek: apply CMDQ control flow")
Signed-off-by: Dennis YC Hsieh 
---
  drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index c84e7a14d4a8..ba6cf956b239 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc 
*mtk_crtc)
 mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
 cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, 
PAGE_SIZE);
 cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
-   cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, true);
+   cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
 mtk_crtc_ddp_config(crtc, cmdq_handle);
 cmdq_pkt_finalize(cmdq_handle);
 cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
--
1.7.9.5
___
Linux-mediatek mailing list
linux-media...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 9/9] drm/mediatek: reduce clear event

2020-09-21 Thread Matthias Brugger




On 09/07/2020 04:39, Bibby Hsieh wrote:

Hi, Dennis,

Thanks for this patch.

It's better to send another tree for this patch.
Because this tree is only for soc/mediatek.

Please do not forget to add the dependency information.


Normally you are right. This time I took this patch as well into my tree. I 
verified with the merge tag from CKs DRM tree that we don't have any conflicts, 
so we should be Ok.


Regards,
Matthias



Bibby

On Tue, 2020-07-07 at 23:45 +0800, Dennis YC Hsieh wrote:

No need to clear event again since event always clear before wait.
This fix depend on patch:
   "soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api"

Fixes: 2f965be7f9008 ("drm/mediatek: apply CMDQ control flow")
Signed-off-by: Dennis YC Hsieh 


Reviewed-by: Bibby Hsieh 


---
  drivers/gpu/drm/mediatek/mtk_drm_crtc.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index c84e7a14d4a8..ba6cf956b239 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc 
*mtk_crtc)
mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
-   cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, true);
+   cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, false);
mtk_crtc_ddp_config(crtc, cmdq_handle);
cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [patch RFC 00/15] mm/highmem: Provide a preemptible variant of kmap_atomic & friends

2020-09-21 Thread Linus Torvalds
On Mon, Sep 21, 2020 at 12:39 AM Thomas Gleixner  wrote:
>
> If a task is migrated to a different CPU then the mapping address will
> change which will explode in colourful ways.

Heh.

Right you are.

Maybe we really *could* call this new kmap functionality something
like "kmap_percpu()" (or maybe "local" is good enough), and make it
act like your RT code does for spinlocks - not disable preemption, but
only disabling CPU migration.

That would probably be good enough for a lot of users that don't want
to expose excessive latencies, but where it's really not a huge deal
to say "stick to this CPU for a short while".

The crypto code certainly sounds like one such case.

 Linus
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 0/3] drm: commit_work scheduling

2020-09-21 Thread Rob Clark
On Mon, Sep 21, 2020 at 9:10 AM Qais Yousef  wrote:
>
> On 09/19/20 12:37, Rob Clark wrote:
> > From: Rob Clark 
> >
> > The android userspace treats the display pipeline as a realtime problem.
> > And arguably, if your goal is to not miss frame deadlines (ie. vblank),
> > it is.  (See https://lwn.net/Articles/809545/ for the best explaination
> > that I found.)
> >
> > But this presents a problem with using workqueues for non-blocking
> > atomic commit_work(), because the SCHED_FIFO userspace thread(s) can
> > preempt the worker.  Which is not really the outcome you want.. once
> > the required fences are scheduled, you want to push the atomic commit
> > down to hw ASAP.
> >
> > But the decision of whether commit_work should be RT or not really
> > depends on what userspace is doing.  For a pure CFS userspace display
> > pipeline, commit_work() should remain SCHED_NORMAL.
>
> Just a side note; this RT vs CFS inter-operatability is an issue that
> creeps up every now and again.
>
> https://lore.kernel.org/lkml/1567048502-6064-1-git-send-email-jing-ting...@mediatek.com/
>
> Does the UI thread in Android ever run as RT by the way? I always suspected it
> is one susceptible to such potential delays since it is part of the 
> application
> thread and thought it can't be trusted to become RT.

The application itself is not RT, since there is no good way to know
what other things the app may be doing.  Although that is mentioned in
the lwn article (ie. the hypothetical SCHED_DEADLINE + token passing
part)

But at least once the app / ui thread is done, then it is all (or at
least in theory "all" if atomic-helper doesn't gum up the works ;-))
SCHED_FIFO from there to the display.

> Those 120MHz displays will stress the pipeline :-)

indeed

BR,
-R

> >
> > To handle this, convert non-blocking commit_work() to use per-CRTC
> > kthread workers, instead of system_unbound_wq.  Per-CRTC workers are
> > used to avoid serializing commits when userspace is using a per-CRTC
> > update loop.
> >
> > A client-cap is introduced so that userspace can opt-in to SCHED_FIFO
> > priority commit work.
> >
> > A potential issue is that since 616d91b68cd ("sched: Remove
> > sched_setscheduler*() EXPORTs") we have limited RT priority levels,
> > meaning that commit_work() ends up running at the same priority level
> > as vblank-work.  This shouldn't be a big problem *yet*, due to limited
> > use of vblank-work at this point.  And if it could be arranged that
> > vblank-work is scheduled before signaling out-fences and/or sending
> > pageflip events, it could probably work ok to use a single priority
> > level for both commit-work and vblank-work.
>
> This is a function of num_cpus too. As long as nr_cpus > nr_running_rt_tasks
> you should be fine.
>
> Cheers
>
> --
> Qais Yousef
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 8/9] soc: mediatek: cmdq: add clear option in cmdq_pkt_wfe api

2020-09-21 Thread Matthias Brugger




On 07/07/2020 17:45, Dennis YC Hsieh wrote:

Add clear parameter to let client decide if
event should be clear to 0 after GCE receive it.

Change since v2:
- Keep behavior in drm crtc driver and
   separate bug fix code into another patch.


This, should go...



Signed-off-by: Dennis YC Hsieh 
---


...here :)

I fixed to commit message and pushed the patch to v5.9-next/soc

Thanks!


  drivers/gpu/drm/mediatek/mtk_drm_crtc.c  |2 +-
  drivers/soc/mediatek/mtk-cmdq-helper.c   |5 +++--
  include/linux/mailbox/mtk-cmdq-mailbox.h |3 +--
  include/linux/soc/mediatek/mtk-cmdq.h|5 +++--
  4 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index ec6c9ffbf35e..c84e7a14d4a8 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -490,7 +490,7 @@ static void mtk_drm_crtc_hw_config(struct mtk_drm_crtc 
*mtk_crtc)
mbox_flush(mtk_crtc->cmdq_client->chan, 2000);
cmdq_handle = cmdq_pkt_create(mtk_crtc->cmdq_client, PAGE_SIZE);
cmdq_pkt_clear_event(cmdq_handle, mtk_crtc->cmdq_event);
-   cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event);
+   cmdq_pkt_wfe(cmdq_handle, mtk_crtc->cmdq_event, true);
mtk_crtc_ddp_config(crtc, cmdq_handle);
cmdq_pkt_finalize(cmdq_handle);
cmdq_pkt_flush_async(cmdq_handle, ddp_cmdq_cb, cmdq_handle);
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c 
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index d55dc3296105..505651b0d715 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -316,15 +316,16 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 
high_addr_reg_idx,
  }
  EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
  
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)

+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear)
  {
struct cmdq_instruction inst = { {0} };
+   u32 clear_option = clear ? CMDQ_WFE_UPDATE : 0;
  
  	if (event >= CMDQ_MAX_EVENT)

return -EINVAL;
  
  	inst.op = CMDQ_CODE_WFE;

-   inst.value = CMDQ_WFE_OPTION;
+   inst.value = CMDQ_WFE_OPTION | clear_option;
inst.event = event;
  
  	return cmdq_pkt_append_command(pkt, inst);

diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h 
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index efbd8a9eb2d1..d5a983d65f05 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -28,8 +28,7 @@
   * bit 16-27: update value
   * bit 31: 1 - update, 0 - no update
   */
-#define CMDQ_WFE_OPTION(CMDQ_WFE_UPDATE | 
CMDQ_WFE_WAIT | \
-   CMDQ_WFE_WAIT_VALUE)
+#define CMDQ_WFE_OPTION(CMDQ_WFE_WAIT | 
CMDQ_WFE_WAIT_VALUE)
  
  /** cmdq event maximum */

  #define CMDQ_MAX_EVENT0x3ff
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h 
b/include/linux/soc/mediatek/mtk-cmdq.h
index 34354e952f60..960704d75994 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -182,11 +182,12 @@ int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 
high_addr_reg_idx,
  /**
   * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
   * @pkt:  the CMDQ packet
- * @event: the desired event type to "wait and CLEAR"
+ * @event: the desired event type to wait
+ * @clear: clear event or not after event arrive
   *
   * Return: 0 for success; else the error code is returned
   */
-int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event);
+int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event, bool clear);
  
  /**

   * cmdq_pkt_clear_event() - append clear event command to the CMDQ packet


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 0/3] drm: commit_work scheduling

2020-09-21 Thread Rob Clark
On Mon, Sep 21, 2020 at 8:16 AM Rob Clark  wrote:
>
> On Mon, Sep 21, 2020 at 2:21 AM Daniel Vetter  wrote:
> >
> > On Sat, Sep 19, 2020 at 12:37:23PM -0700, Rob Clark wrote:
> > > From: Rob Clark 
> > >
> > > The android userspace treats the display pipeline as a realtime problem.
> > > And arguably, if your goal is to not miss frame deadlines (ie. vblank),
> > > it is.  (See https://lwn.net/Articles/809545/ for the best explaination
> > > that I found.)
> > >
> > > But this presents a problem with using workqueues for non-blocking
> > > atomic commit_work(), because the SCHED_FIFO userspace thread(s) can
> > > preempt the worker.  Which is not really the outcome you want.. once
> > > the required fences are scheduled, you want to push the atomic commit
> > > down to hw ASAP.
> > >
> > > But the decision of whether commit_work should be RT or not really
> > > depends on what userspace is doing.  For a pure CFS userspace display
> > > pipeline, commit_work() should remain SCHED_NORMAL.
> > >
> > > To handle this, convert non-blocking commit_work() to use per-CRTC
> > > kthread workers, instead of system_unbound_wq.  Per-CRTC workers are
> > > used to avoid serializing commits when userspace is using a per-CRTC
> > > update loop.
> > >
> > > A client-cap is introduced so that userspace can opt-in to SCHED_FIFO
> > > priority commit work.
> > >
> > > A potential issue is that since 616d91b68cd ("sched: Remove
> > > sched_setscheduler*() EXPORTs") we have limited RT priority levels,
> > > meaning that commit_work() ends up running at the same priority level
> > > as vblank-work.  This shouldn't be a big problem *yet*, due to limited
> > > use of vblank-work at this point.  And if it could be arranged that
> > > vblank-work is scheduled before signaling out-fences and/or sending
> > > pageflip events, it could probably work ok to use a single priority
> > > level for both commit-work and vblank-work.
> >
> > The part I don't like about this is that it all feels rather hacked
> > together, and if we add more stuff (or there's some different thing in the
> > system that also needs rt scheduling) then it doesn't compose.
>
> The ideal thing would be that userspace is in control of the
> priorities.. the setclientcap approach seemed like a reasonable way to
> give the drm-master a way to opt in.
>
> I suppose instead userspace could use sched_setscheduler().. but that
> would require userspace to be root, and would require some way to find
> the tid.
>
> Is there some way we could arrange for the per-crtc kthread's to be
> owned by the drm master?  That would solve the "must be root" issue.
> And since the target audience is an atomic userspace, I suppose we
> could expose the tid as a read-only property on the crtc?

Looks like kthread goes out of it's way to *not* be owned by users (to
avoid fork, and such complications?)

But maybe we could modify the kthread_worker's task->real_cred?  I
didn't see any examples of anything else doing something similar, so
I'm not sure what sorts of dragons there lie..

> BR,
> -R
>
> > So question to rt/worker folks: What's the best way to let userspace set
> > the scheduling mode and priorities of things the kernel does on its
> > behalf? Surely we're not the first ones where if userspace runs with some
> > rt priority it'll starve out the kernel workers that it needs. Hardcoding
> > something behind a subsystem ioctl (which just means every time userspace
> > changes what it does, we need a new such flag or mode) can't be the right
> > thing.
> >
> > Peter, Tejun?
> >
> > Thanks, Daniel
> >
> > >
> > > Rob Clark (3):
> > >   drm/crtc: Introduce per-crtc kworker
> > >   drm/atomic: Use kthread worker for nonblocking commits
> > >   drm: Add a client-cap to set scheduling mode
> > >
> > >  drivers/gpu/drm/drm_atomic_helper.c | 13 ++
> > >  drivers/gpu/drm/drm_auth.c  |  4 
> > >  drivers/gpu/drm/drm_crtc.c  | 37 +
> > >  drivers/gpu/drm/drm_ioctl.c | 13 ++
> > >  include/drm/drm_atomic.h| 31 
> > >  include/drm/drm_crtc.h  | 10 
> > >  include/uapi/drm/drm.h  | 13 ++
> > >  7 files changed, 117 insertions(+), 4 deletions(-)
> > >
> > > --
> > > 2.26.2
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 5/9] soc: mediatek: cmdq: add write_s value function

2020-09-21 Thread Matthias Brugger




On 07/07/2020 17:45, Dennis YC Hsieh wrote:

add write_s function in cmdq helper functions which
writes a constant value to address with large dma
access support.

Signed-off-by: Dennis YC Hsieh 


Now pushed to v5.9-next/soc

Thanks!


---
  drivers/soc/mediatek/mtk-cmdq-helper.c |   14 ++
  include/linux/soc/mediatek/mtk-cmdq.h  |   13 +
  2 files changed, 27 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c 
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index ed9f5e63c195..4e86b65815fc 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -280,6 +280,20 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 
high_addr_reg_idx,
  }
  EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
  
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,

+  u16 addr_low, u32 value)
+{
+   struct cmdq_instruction inst = {};
+
+   inst.op = CMDQ_CODE_WRITE_S;
+   inst.sop = high_addr_reg_idx;
+   inst.offset = addr_low;
+   inst.value = value;
+
+   return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_value);
+
  int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
  {
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h 
b/include/linux/soc/mediatek/mtk-cmdq.h
index cd7ec714344e..ae73e10da274 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -152,6 +152,19 @@ int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 
high_addr_reg_idx,
  u16 addr_low, u16 src_reg_idx, u32 mask);
  
  /**

+ * cmdq_pkt_write_s_value() - append write_s command to the CMDQ packet which
+ *   write value to a physical address
+ * @pkt:   the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low:  low address of pa
+ * @value: the specified target value
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+  u16 addr_low, u32 value);
+
+/**
   * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
   * @pkt:  the CMDQ packet
   * @event:the desired event type to "wait and CLEAR"


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 3/9] soc: mediatek: cmdq: add write_s_mask function

2020-09-21 Thread Matthias Brugger




On 07/07/2020 17:45, Dennis YC Hsieh wrote:

add write_s_mask function in cmdq helper functions which
writes value contains in internal register to address
with mask and large dma access support.

Signed-off-by: Dennis YC Hsieh 


Now pushed to v5.9-next/soc

Thanks!


---
  drivers/soc/mediatek/mtk-cmdq-helper.c   |   23 +++
  include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
  include/linux/soc/mediatek/mtk-cmdq.h|   18 ++
  3 files changed, 42 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c 
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 880349b3f16c..550e9e7e3ff2 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -242,6 +242,29 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 
high_addr_reg_idx,
  }
  EXPORT_SYMBOL(cmdq_pkt_write_s);
  
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,

+ u16 addr_low, u16 src_reg_idx, u32 mask)
+{
+   struct cmdq_instruction inst = {};
+   int err;
+
+   inst.op = CMDQ_CODE_MASK;
+   inst.mask = ~mask;
+   err = cmdq_pkt_append_command(pkt, inst);
+   if (err < 0)
+   return err;
+
+   inst.mask = 0;
+   inst.op = CMDQ_CODE_WRITE_S_MASK;
+   inst.src_t = CMDQ_REG_TYPE;
+   inst.sop = high_addr_reg_idx;
+   inst.offset = addr_low;
+   inst.src_reg = src_reg_idx;
+
+   return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask);
+
  int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
  {
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h 
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 1f76cfedb16d..90d1d8e64412 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -61,6 +61,7 @@ enum cmdq_code {
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
CMDQ_CODE_WRITE_S = 0x90,
+   CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
  };
  
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h

index 9b0c57a0063d..53230341bf94 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -122,6 +122,24 @@ int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 
high_addr_reg_idx,
 u16 addr_low, u16 src_reg_idx);
  
  /**

+ * cmdq_pkt_write_s_mask() - append write_s with mask command to the CMDQ 
packet
+ * @pkt:   the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low:  low address of pa
+ * @src_reg_idx:   the CMDQ internal register ID which cache source value
+ * @mask:  the specified target address mask, use U32_MAX if no need
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s_mask(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+ u16 addr_low, u16 src_reg_idx, u32 mask);
+
+/**
   * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
   * @pkt:  the CMDQ packet
   * @event:the desired event type to "wait and CLEAR"


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 6/9] soc: mediatek: cmdq: add write_s_mask value function

2020-09-21 Thread Matthias Brugger




On 07/07/2020 17:45, Dennis YC Hsieh wrote:

add write_s_mask_value function in cmdq helper functions which
writes a constant value to address with mask and large dma
access support.

Signed-off-by: Dennis YC Hsieh 


Now pushed to v5.9-next/soc

Thanks!


---
  drivers/soc/mediatek/mtk-cmdq-helper.c |   21 +
  include/linux/soc/mediatek/mtk-cmdq.h  |   15 +++
  2 files changed, 36 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c 
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 4e86b65815fc..b6e25f216605 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -294,6 +294,27 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 
high_addr_reg_idx,
  }
  EXPORT_SYMBOL(cmdq_pkt_write_s_value);
  
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,

+   u16 addr_low, u32 value, u32 mask)
+{
+   struct cmdq_instruction inst = {};
+   int err;
+
+   inst.op = CMDQ_CODE_MASK;
+   inst.mask = ~mask;
+   err = cmdq_pkt_append_command(pkt, inst);
+   if (err < 0)
+   return err;
+
+   inst.op = CMDQ_CODE_WRITE_S_MASK;
+   inst.sop = high_addr_reg_idx;
+   inst.offset = addr_low;
+   inst.value = value;
+
+   return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s_mask_value);
+
  int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
  {
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h 
b/include/linux/soc/mediatek/mtk-cmdq.h
index ae73e10da274..d9390d76ee14 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -165,6 +165,21 @@ int cmdq_pkt_write_s_value(struct cmdq_pkt *pkt, u8 
high_addr_reg_idx,
   u16 addr_low, u32 value);
  
  /**

+ * cmdq_pkt_write_s_mask_value() - append write_s command with mask to the CMDQ
+ *packet which write value to a physical
+ *address
+ * @pkt:   the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low:  low address of pa
+ * @value: the specified target value
+ * @mask:  the specified target mask
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_write_s_mask_value(struct cmdq_pkt *pkt, u8 high_addr_reg_idx,
+   u16 addr_low, u32 value, u32 mask);
+
+/**
   * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
   * @pkt:  the CMDQ packet
   * @event:the desired event type to "wait and CLEAR"


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 4/9] soc: mediatek: cmdq: add read_s function

2020-09-21 Thread Matthias Brugger




On 07/07/2020 17:45, Dennis YC Hsieh wrote:

Add read_s function in cmdq helper functions which support read value from
register or dma physical address into gce internal register.

Signed-off-by: Dennis YC Hsieh 


Now pushed to v5.9-next/soc

Thanks!


---
  drivers/soc/mediatek/mtk-cmdq-helper.c   |   15 +++
  include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
  include/linux/soc/mediatek/mtk-cmdq.h|   12 
  3 files changed, 28 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c 
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 550e9e7e3ff2..ed9f5e63c195 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -227,6 +227,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
  }
  EXPORT_SYMBOL(cmdq_pkt_write_mask);
  
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,

+   u16 reg_idx)
+{
+   struct cmdq_instruction inst = {};
+
+   inst.op = CMDQ_CODE_READ_S;
+   inst.dst_t = CMDQ_REG_TYPE;
+   inst.sop = high_addr_reg_idx;
+   inst.reg_dst = reg_idx;
+   inst.src_reg = addr_low;
+
+   return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_read_s);
+
  int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
 u16 addr_low, u16 src_reg_idx)
  {
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h 
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 90d1d8e64412..efbd8a9eb2d1 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -60,6 +60,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+   CMDQ_CODE_READ_S = 0x80,
CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_WRITE_S_MASK = 0x91,
CMDQ_CODE_LOGIC = 0xa0,
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h 
b/include/linux/soc/mediatek/mtk-cmdq.h
index 53230341bf94..cd7ec714344e 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -104,6 +104,18 @@ struct cmdq_client *cmdq_mbox_create(struct device *dev, 
int index,
  int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
u16 offset, u32 value, u32 mask);
  
+/*

+ * cmdq_pkt_read_s() - append read_s command to the CMDQ packet
+ * @pkt:   the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low:  low address of pa
+ * @reg_idx:   the CMDQ internal register ID to cache read data
+ *
+ * Return: 0 for success; else the error code is returned
+ */
+int cmdq_pkt_read_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx, u16 addr_low,
+   u16 reg_idx);
+
  /**
   * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
   * @pkt:  the CMDQ packet


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 2/9] soc: mediatek: cmdq: add write_s function

2020-09-21 Thread Matthias Brugger




On 07/07/2020 17:45, Dennis YC Hsieh wrote:

add write_s function in cmdq helper functions which
writes value contains in internal register to address
with large dma access support.

Signed-off-by: Dennis YC Hsieh 


Now pushed to v5.9-next/soc

Thanks!

---
  drivers/soc/mediatek/mtk-cmdq-helper.c   |   19 +++
  include/linux/mailbox/mtk-cmdq-mailbox.h |1 +
  include/linux/soc/mediatek/mtk-cmdq.h|   19 +++
  3 files changed, 39 insertions(+)

diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c 
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index 9faf78fbed3a..880349b3f16c 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -18,6 +18,10 @@ struct cmdq_instruction {
union {
u32 value;
u32 mask;
+   struct {
+   u16 arg_c;
+   u16 src_reg;
+   };
};
union {
u16 offset;
@@ -223,6 +227,21 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,
  }
  EXPORT_SYMBOL(cmdq_pkt_write_mask);
  
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,

+u16 addr_low, u16 src_reg_idx)
+{
+   struct cmdq_instruction inst = {};
+
+   inst.op = CMDQ_CODE_WRITE_S;
+   inst.src_t = CMDQ_REG_TYPE;
+   inst.sop = high_addr_reg_idx;
+   inst.offset = addr_low;
+   inst.src_reg = src_reg_idx;
+
+   return cmdq_pkt_append_command(pkt, inst);
+}
+EXPORT_SYMBOL(cmdq_pkt_write_s);
+
  int cmdq_pkt_wfe(struct cmdq_pkt *pkt, u16 event)
  {
struct cmdq_instruction inst = { {0} };
diff --git a/include/linux/mailbox/mtk-cmdq-mailbox.h 
b/include/linux/mailbox/mtk-cmdq-mailbox.h
index 05eea1aef5aa..1f76cfedb16d 100644
--- a/include/linux/mailbox/mtk-cmdq-mailbox.h
+++ b/include/linux/mailbox/mtk-cmdq-mailbox.h
@@ -60,6 +60,7 @@ enum cmdq_code {
CMDQ_CODE_JUMP = 0x10,
CMDQ_CODE_WFE = 0x20,
CMDQ_CODE_EOC = 0x40,
+   CMDQ_CODE_WRITE_S = 0x90,
CMDQ_CODE_LOGIC = 0xa0,
  };
  
diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h

index 2249ecaf77e4..9b0c57a0063d 100644
--- a/include/linux/soc/mediatek/mtk-cmdq.h
+++ b/include/linux/soc/mediatek/mtk-cmdq.h
@@ -12,6 +12,8 @@
  #include 
  
  #define CMDQ_NO_TIMEOUT		0xu

+#define CMDQ_ADDR_HIGH(addr)   ((u32)(((addr) >> 16) & GENMASK(31, 0)))
+#define CMDQ_ADDR_LOW(addr)((u16)(addr) | BIT(1))
  
  struct cmdq_pkt;
  
@@ -103,6 +105,23 @@ int cmdq_pkt_write_mask(struct cmdq_pkt *pkt, u8 subsys,

u16 offset, u32 value, u32 mask);
  
  /**

+ * cmdq_pkt_write_s() - append write_s command to the CMDQ packet
+ * @pkt:   the CMDQ packet
+ * @high_addr_reg_idx: internal register ID which contains high address of pa
+ * @addr_low:  low address of pa
+ * @src_reg_idx:   the CMDQ internal register ID which cache source value
+ *
+ * Return: 0 for success; else the error code is returned
+ *
+ * Support write value to physical address without subsys. Use CMDQ_ADDR_HIGH()
+ * to get high address and call cmdq_pkt_assign() to assign value into internal
+ * reg. Also use CMDQ_ADDR_LOW() to get low address for addr_low parameter when
+ * call to this function.
+ */
+int cmdq_pkt_write_s(struct cmdq_pkt *pkt, u16 high_addr_reg_idx,
+u16 addr_low, u16 src_reg_idx);
+
+/**
   * cmdq_pkt_wfe() - append wait for event command to the CMDQ packet
   * @pkt:  the CMDQ packet
   * @event:the desired event type to "wait and CLEAR"


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v3 1/9] soc: mediatek: cmdq: add address shift in jump

2020-09-21 Thread Matthias Brugger




On 07/07/2020 17:45, Dennis YC Hsieh wrote:

Add address shift when compose jump instruction
to compatible with 35bit format.

Change since v1:
- Rename cmdq_mbox_shift() to cmdq_get_shift_pa().

Signed-off-by: Dennis YC Hsieh 


Now pushed to v5.9-next/soc

Thanks!


---
  drivers/soc/mediatek/mtk-cmdq-helper.c |3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c 
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index dc644cfb6419..9faf78fbed3a 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -329,7 +329,8 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt)
  
  	/* JUMP to end */

inst.op = CMDQ_CODE_JUMP;
-   inst.value = CMDQ_JUMP_PASS;
+   inst.value = CMDQ_JUMP_PASS >>
+   cmdq_get_shift_pa(((struct cmdq_client *)pkt->cl)->chan);
err = cmdq_pkt_append_command(pkt, inst);
  
  	return err;



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] soc: mediatek: cmdq: Remove timeout handler in helper function

2020-09-21 Thread Matthias Brugger



On 21/09/2020 17:32, Chun-Kuang Hu wrote:

Hi, Matthias:

Matthias Brugger  於 2020年9月21日 週一 下午4:53寫道:




On 21/09/2020 01:42, Chun-Kuang Hu wrote:

For each client driver, its timeout handler need to dump hardware register
or its state machine information, so remove timeout handler in helper
function and let client driver implement its own timeout handler.



I don't see the implementation of a client side handler. Did I miss something?
Would it make sense to instead add a callback to the handler in 
cmdq_mbox_create()?


According to the commit message, it make sense to add a callback to
the handler in comq_mbox_create().
But for DRM, I would like to check timeout in vblank irq because the
register should be applied in vblank. (I have not implement this patch
yet)
What I want to say is that different client may have different way to
detect timeout and different way to handle it.
If you want, I would add DRM timeout handle patch with this patch, and
modify commit message to include different way to detect timeout.



I think that would help me to understand the whole picture.

Thanks a lot!
Matthias


Regards,
Chun-Kuang.



Regards,
Matthias


Signed-off-by: Chun-Kuang Hu 
---
   drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  3 +-
   drivers/soc/mediatek/mtk-cmdq-helper.c  | 41 +
   include/linux/soc/mediatek/mtk-cmdq.h   | 11 +--
   3 files changed, 3 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 3fc5511330b9..cabeb7fea2be 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -824,8 +824,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
   #if IS_REACHABLE(CONFIG_MTK_CMDQ)
   mtk_crtc->cmdq_client =
   cmdq_mbox_create(mtk_crtc->mmsys_dev,
-  drm_crtc_index(_crtc->base),
-  2000);
+  drm_crtc_index(_crtc->base));
   if (IS_ERR(mtk_crtc->cmdq_client)) {
   dev_dbg(dev, "mtk_crtc %d failed to create mailbox client, writing 
register by CPU now\n",
   drm_crtc_index(_crtc->base));
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c 
b/drivers/soc/mediatek/mtk-cmdq-helper.c
index dc644cfb6419..4f311f035b59 100644
--- a/drivers/soc/mediatek/mtk-cmdq-helper.c
+++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
@@ -65,14 +65,7 @@ int cmdq_dev_get_client_reg(struct device *dev,
   }
   EXPORT_SYMBOL(cmdq_dev_get_client_reg);

-static void cmdq_client_timeout(struct timer_list *t)
-{
- struct cmdq_client *client = from_timer(client, t, timer);
-
- dev_err(client->client.dev, "cmdq timeout!\n");
-}
-
-struct cmdq_client *cmdq_mbox_create(struct device *dev, int index, u32 
timeout)
+struct cmdq_client *cmdq_mbox_create(struct device *dev, int index)
   {
   struct cmdq_client *client;

@@ -80,12 +73,6 @@ struct cmdq_client *cmdq_mbox_create(struct device *dev, int 
index, u32 timeout)
   if (!client)
   return (struct cmdq_client *)-ENOMEM;

- client->timeout_ms = timeout;
- if (timeout != CMDQ_NO_TIMEOUT) {
- spin_lock_init(>lock);
- timer_setup(>timer, cmdq_client_timeout, 0);
- }
- client->pkt_cnt = 0;
   client->client.dev = dev;
   client->client.tx_block = false;
   client->client.knows_txdone = true;
@@ -107,11 +94,6 @@ EXPORT_SYMBOL(cmdq_mbox_create);

   void cmdq_mbox_destroy(struct cmdq_client *client)
   {
- if (client->timeout_ms != CMDQ_NO_TIMEOUT) {
- spin_lock(>lock);
- del_timer_sync(>timer);
- spin_unlock(>lock);
- }
   mbox_free_channel(client->chan);
   kfree(client);
   }
@@ -342,18 +324,6 @@ static void cmdq_pkt_flush_async_cb(struct cmdq_cb_data 
data)
   struct cmdq_task_cb *cb = >cb;
   struct cmdq_client *client = (struct cmdq_client *)pkt->cl;

- if (client->timeout_ms != CMDQ_NO_TIMEOUT) {
- unsigned long flags = 0;
-
- spin_lock_irqsave(>lock, flags);
- if (--client->pkt_cnt == 0)
- del_timer(>timer);
- else
- mod_timer(>timer, jiffies +
-   msecs_to_jiffies(client->timeout_ms));
- spin_unlock_irqrestore(>lock, flags);
- }
-
   dma_sync_single_for_cpu(client->chan->mbox->dev, pkt->pa_base,
   pkt->cmd_buf_size, DMA_TO_DEVICE);
   if (cb->cb) {
@@ -366,7 +336,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, 
cmdq_async_flush_cb cb,
void *data)
   {
   int err;
- unsigned long flags = 0;
   struct cmdq_client *client = (struct cmdq_client *)pkt->cl;

   pkt->cb.cb = cb;
@@ -377,14 +346,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, 
cmdq_async_flush_cb cb,
   

Re: [PATCH 2/2] drm/msm: Leave inuse count intact on map failure

2020-09-21 Thread Rob Clark
On Mon, Sep 21, 2020 at 8:27 AM Akhil P Oommen  wrote:
>
> Leave the inuse count intact on map failure to keep the accounting
> accurate.
>
> Signed-off-by: Akhil P Oommen 
> ---
>  drivers/gpu/drm/msm/msm_gem_vma.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c 
> b/drivers/gpu/drm/msm/msm_gem_vma.c
> index 80a8a26..8367a1c 100644
> --- a/drivers/gpu/drm/msm/msm_gem_vma.c
> +++ b/drivers/gpu/drm/msm/msm_gem_vma.c
> @@ -88,8 +88,10 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
> ret = aspace->mmu->funcs->map(aspace->mmu, vma->iova, sgt,
> size, prot);
>
> -   if (ret)
> +   if (ret) {
> vma->mapped = false;
> +   vma->inuse++;

vma->inuse-- ?

BR,
-R

> +   }
>
> return ret;
>  }
> --
> 2.7.4
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 1/2] drm/msm: Replace active_list with refcount

2020-09-21 Thread Rob Clark
On Mon, Sep 21, 2020 at 8:27 AM Akhil P Oommen  wrote:
>
> In the case where we have a back-to-back submission that shares the same
> BO, this BO will be prematurely moved to inactive_list while retiring the
> first submit. But it will be still part of the second submit which is
> being processed by the GPU. Now, if the shrinker happens to be triggered at
> this point, it will result in premature purging of this BO.
>
> To fix this, we can replace the active_list with reference counting and
> move the BO to inactive list only when this count becomes zero.
>
> Signed-off-by: Akhil P Oommen 
> ---
>  drivers/gpu/drm/msm/msm_drv.h |  5 ++---
>  drivers/gpu/drm/msm/msm_gem.c | 30 --
>  drivers/gpu/drm/msm/msm_gem.h |  4 +++-
>  drivers/gpu/drm/msm/msm_gpu.c | 11 +++
>  4 files changed, 28 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index 3193274..28e3c8d 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -309,9 +309,8 @@ void msm_gem_put_vaddr(struct drm_gem_object *obj);
>  int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv);
>  int msm_gem_sync_object(struct drm_gem_object *obj,
> struct msm_fence_context *fctx, bool exclusive);
> -void msm_gem_move_to_active(struct drm_gem_object *obj,
> -   struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence);
> -void msm_gem_move_to_inactive(struct drm_gem_object *obj);
> +void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu);
> +void msm_gem_active_put(struct drm_gem_object *obj);
>  int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t 
> *timeout);
>  int msm_gem_cpu_fini(struct drm_gem_object *obj);
>  void msm_gem_free_object(struct drm_gem_object *obj);
> diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
> index 76a6c52..accc106 100644
> --- a/drivers/gpu/drm/msm/msm_gem.c
> +++ b/drivers/gpu/drm/msm/msm_gem.c
> @@ -743,33 +743,36 @@ int msm_gem_sync_object(struct drm_gem_object *obj,
> return 0;
>  }
>
> -void msm_gem_move_to_active(struct drm_gem_object *obj,
> -   struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence)
> +void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu)
>  {
> struct msm_gem_object *msm_obj = to_msm_bo(obj);
> +   WARN_ON(!mutex_is_locked(>dev->struct_mutex));
> WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED);
> +
> msm_obj->gpu = gpu;
> -   if (exclusive)
> -   dma_resv_add_excl_fence(obj->resv, fence);
> -   else
> -   dma_resv_add_shared_fence(obj->resv, fence);
> list_del_init(_obj->mm_list);
> -   list_add_tail(_obj->mm_list, >active_list);
> +   atomic_inc(_obj->active_count);

I don't think we want to get rid of active_list, but just supplement
it with a refcnt.

I suspect as-is, this breaks $debugfs/gem (which iterates the active
and inactive list)

>  }
>
> -void msm_gem_move_to_inactive(struct drm_gem_object *obj)
> +static void move_to_inactive(struct msm_gem_object *msm_obj)
>  {
> -   struct drm_device *dev = obj->dev;
> +   struct drm_device *dev = msm_obj->base.dev;
> struct msm_drm_private *priv = dev->dev_private;
> -   struct msm_gem_object *msm_obj = to_msm_bo(obj);
> -
> -   WARN_ON(!mutex_is_locked(>struct_mutex));
>
> msm_obj->gpu = NULL;
> -   list_del_init(_obj->mm_list);
> list_add_tail(_obj->mm_list, >inactive_list);
>  }
>
> +void msm_gem_active_put(struct drm_gem_object *obj)
> +{
> +   struct msm_gem_object *msm_obj = to_msm_bo(obj);
> +
> +   WARN_ON(!mutex_is_locked(>dev->struct_mutex));
> +
> +   if (atomic_dec_and_test(_obj->active_count))
> +   move_to_inactive(msm_obj);
> +}
> +
>  int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t 
> *timeout)
>  {
> bool write = !!(op & MSM_PREP_WRITE);
> @@ -1104,7 +1107,6 @@ static struct drm_gem_object *_msm_gem_new(struct 
> drm_device *dev,
> }
>
> if (struct_mutex_locked) {
> -   WARN_ON(!mutex_is_locked(>struct_mutex));

This looks unrelated (and I don't think we want to drop this WARN_ON())

BR,
-R

> list_add_tail(_obj->mm_list, >inactive_list);
> } else {
> mutex_lock(>struct_mutex);
> diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
> index 7b1c7a5..a1bf741 100644
> --- a/drivers/gpu/drm/msm/msm_gem.h
> +++ b/drivers/gpu/drm/msm/msm_gem.h
> @@ -88,12 +88,14 @@ struct msm_gem_object {
> struct mutex lock; /* Protects resources associated with bo */
>
> char name[32]; /* Identifier to print for the debugfs files */
> +
> +   atomic_t active_count;
>  };
>  #define to_msm_bo(x) container_of(x, struct msm_gem_object, base)
>
>  static inline bool is_active(struct msm_gem_object *msm_obj)
>  {
> - 

Re: [PATCH] soc: mediatek: cmdq: Remove timeout handler in helper function

2020-09-21 Thread Chun-Kuang Hu
Hi, Matthias:

Matthias Brugger  於 2020年9月21日 週一 下午4:53寫道:
>
>
>
> On 21/09/2020 01:42, Chun-Kuang Hu wrote:
> > For each client driver, its timeout handler need to dump hardware register
> > or its state machine information, so remove timeout handler in helper
> > function and let client driver implement its own timeout handler.
> >
>
> I don't see the implementation of a client side handler. Did I miss something?
> Would it make sense to instead add a callback to the handler in 
> cmdq_mbox_create()?

According to the commit message, it make sense to add a callback to
the handler in comq_mbox_create().
But for DRM, I would like to check timeout in vblank irq because the
register should be applied in vblank. (I have not implement this patch
yet)
What I want to say is that different client may have different way to
detect timeout and different way to handle it.
If you want, I would add DRM timeout handle patch with this patch, and
modify commit message to include different way to detect timeout.

Regards,
Chun-Kuang.

>
> Regards,
> Matthias
>
> > Signed-off-by: Chun-Kuang Hu 
> > ---
> >   drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  3 +-
> >   drivers/soc/mediatek/mtk-cmdq-helper.c  | 41 +
> >   include/linux/soc/mediatek/mtk-cmdq.h   | 11 +--
> >   3 files changed, 3 insertions(+), 52 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
> > b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > index 3fc5511330b9..cabeb7fea2be 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
> > @@ -824,8 +824,7 @@ int mtk_drm_crtc_create(struct drm_device *drm_dev,
> >   #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> >   mtk_crtc->cmdq_client =
> >   cmdq_mbox_create(mtk_crtc->mmsys_dev,
> > -  drm_crtc_index(_crtc->base),
> > -  2000);
> > +  drm_crtc_index(_crtc->base));
> >   if (IS_ERR(mtk_crtc->cmdq_client)) {
> >   dev_dbg(dev, "mtk_crtc %d failed to create mailbox client, 
> > writing register by CPU now\n",
> >   drm_crtc_index(_crtc->base));
> > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c 
> > b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > index dc644cfb6419..4f311f035b59 100644
> > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c
> > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c
> > @@ -65,14 +65,7 @@ int cmdq_dev_get_client_reg(struct device *dev,
> >   }
> >   EXPORT_SYMBOL(cmdq_dev_get_client_reg);
> >
> > -static void cmdq_client_timeout(struct timer_list *t)
> > -{
> > - struct cmdq_client *client = from_timer(client, t, timer);
> > -
> > - dev_err(client->client.dev, "cmdq timeout!\n");
> > -}
> > -
> > -struct cmdq_client *cmdq_mbox_create(struct device *dev, int index, u32 
> > timeout)
> > +struct cmdq_client *cmdq_mbox_create(struct device *dev, int index)
> >   {
> >   struct cmdq_client *client;
> >
> > @@ -80,12 +73,6 @@ struct cmdq_client *cmdq_mbox_create(struct device *dev, 
> > int index, u32 timeout)
> >   if (!client)
> >   return (struct cmdq_client *)-ENOMEM;
> >
> > - client->timeout_ms = timeout;
> > - if (timeout != CMDQ_NO_TIMEOUT) {
> > - spin_lock_init(>lock);
> > - timer_setup(>timer, cmdq_client_timeout, 0);
> > - }
> > - client->pkt_cnt = 0;
> >   client->client.dev = dev;
> >   client->client.tx_block = false;
> >   client->client.knows_txdone = true;
> > @@ -107,11 +94,6 @@ EXPORT_SYMBOL(cmdq_mbox_create);
> >
> >   void cmdq_mbox_destroy(struct cmdq_client *client)
> >   {
> > - if (client->timeout_ms != CMDQ_NO_TIMEOUT) {
> > - spin_lock(>lock);
> > - del_timer_sync(>timer);
> > - spin_unlock(>lock);
> > - }
> >   mbox_free_channel(client->chan);
> >   kfree(client);
> >   }
> > @@ -342,18 +324,6 @@ static void cmdq_pkt_flush_async_cb(struct 
> > cmdq_cb_data data)
> >   struct cmdq_task_cb *cb = >cb;
> >   struct cmdq_client *client = (struct cmdq_client *)pkt->cl;
> >
> > - if (client->timeout_ms != CMDQ_NO_TIMEOUT) {
> > - unsigned long flags = 0;
> > -
> > - spin_lock_irqsave(>lock, flags);
> > - if (--client->pkt_cnt == 0)
> > - del_timer(>timer);
> > - else
> > - mod_timer(>timer, jiffies +
> > -   msecs_to_jiffies(client->timeout_ms));
> > - spin_unlock_irqrestore(>lock, flags);
> > - }
> > -
> >   dma_sync_single_for_cpu(client->chan->mbox->dev, pkt->pa_base,
> >   pkt->cmd_buf_size, DMA_TO_DEVICE);
> >   if (cb->cb) {
> > @@ -366,7 +336,6 @@ int cmdq_pkt_flush_async(struct cmdq_pkt *pkt, 
> > cmdq_async_flush_cb cb,
> >void *data)
> >   {
> >   int err;
> > - unsigned long flags 

[PATCH 1/2] drm/msm: Replace active_list with refcount

2020-09-21 Thread Akhil P Oommen
In the case where we have a back-to-back submission that shares the same
BO, this BO will be prematurely moved to inactive_list while retiring the
first submit. But it will be still part of the second submit which is
being processed by the GPU. Now, if the shrinker happens to be triggered at
this point, it will result in premature purging of this BO.

To fix this, we can replace the active_list with reference counting and
move the BO to inactive list only when this count becomes zero.

Signed-off-by: Akhil P Oommen 
---
 drivers/gpu/drm/msm/msm_drv.h |  5 ++---
 drivers/gpu/drm/msm/msm_gem.c | 30 --
 drivers/gpu/drm/msm/msm_gem.h |  4 +++-
 drivers/gpu/drm/msm/msm_gpu.c | 11 +++
 4 files changed, 28 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 3193274..28e3c8d 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -309,9 +309,8 @@ void msm_gem_put_vaddr(struct drm_gem_object *obj);
 int msm_gem_madvise(struct drm_gem_object *obj, unsigned madv);
 int msm_gem_sync_object(struct drm_gem_object *obj,
struct msm_fence_context *fctx, bool exclusive);
-void msm_gem_move_to_active(struct drm_gem_object *obj,
-   struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence);
-void msm_gem_move_to_inactive(struct drm_gem_object *obj);
+void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu);
+void msm_gem_active_put(struct drm_gem_object *obj);
 int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t 
*timeout);
 int msm_gem_cpu_fini(struct drm_gem_object *obj);
 void msm_gem_free_object(struct drm_gem_object *obj);
diff --git a/drivers/gpu/drm/msm/msm_gem.c b/drivers/gpu/drm/msm/msm_gem.c
index 76a6c52..accc106 100644
--- a/drivers/gpu/drm/msm/msm_gem.c
+++ b/drivers/gpu/drm/msm/msm_gem.c
@@ -743,33 +743,36 @@ int msm_gem_sync_object(struct drm_gem_object *obj,
return 0;
 }
 
-void msm_gem_move_to_active(struct drm_gem_object *obj,
-   struct msm_gpu *gpu, bool exclusive, struct dma_fence *fence)
+void msm_gem_active_get(struct drm_gem_object *obj, struct msm_gpu *gpu)
 {
struct msm_gem_object *msm_obj = to_msm_bo(obj);
+   WARN_ON(!mutex_is_locked(>dev->struct_mutex));
WARN_ON(msm_obj->madv != MSM_MADV_WILLNEED);
+
msm_obj->gpu = gpu;
-   if (exclusive)
-   dma_resv_add_excl_fence(obj->resv, fence);
-   else
-   dma_resv_add_shared_fence(obj->resv, fence);
list_del_init(_obj->mm_list);
-   list_add_tail(_obj->mm_list, >active_list);
+   atomic_inc(_obj->active_count);
 }
 
-void msm_gem_move_to_inactive(struct drm_gem_object *obj)
+static void move_to_inactive(struct msm_gem_object *msm_obj)
 {
-   struct drm_device *dev = obj->dev;
+   struct drm_device *dev = msm_obj->base.dev;
struct msm_drm_private *priv = dev->dev_private;
-   struct msm_gem_object *msm_obj = to_msm_bo(obj);
-
-   WARN_ON(!mutex_is_locked(>struct_mutex));
 
msm_obj->gpu = NULL;
-   list_del_init(_obj->mm_list);
list_add_tail(_obj->mm_list, >inactive_list);
 }
 
+void msm_gem_active_put(struct drm_gem_object *obj)
+{
+   struct msm_gem_object *msm_obj = to_msm_bo(obj);
+
+   WARN_ON(!mutex_is_locked(>dev->struct_mutex));
+
+   if (atomic_dec_and_test(_obj->active_count))
+   move_to_inactive(msm_obj);
+}
+
 int msm_gem_cpu_prep(struct drm_gem_object *obj, uint32_t op, ktime_t *timeout)
 {
bool write = !!(op & MSM_PREP_WRITE);
@@ -1104,7 +1107,6 @@ static struct drm_gem_object *_msm_gem_new(struct 
drm_device *dev,
}
 
if (struct_mutex_locked) {
-   WARN_ON(!mutex_is_locked(>struct_mutex));
list_add_tail(_obj->mm_list, >inactive_list);
} else {
mutex_lock(>struct_mutex);
diff --git a/drivers/gpu/drm/msm/msm_gem.h b/drivers/gpu/drm/msm/msm_gem.h
index 7b1c7a5..a1bf741 100644
--- a/drivers/gpu/drm/msm/msm_gem.h
+++ b/drivers/gpu/drm/msm/msm_gem.h
@@ -88,12 +88,14 @@ struct msm_gem_object {
struct mutex lock; /* Protects resources associated with bo */
 
char name[32]; /* Identifier to print for the debugfs files */
+
+   atomic_t active_count;
 };
 #define to_msm_bo(x) container_of(x, struct msm_gem_object, base)
 
 static inline bool is_active(struct msm_gem_object *msm_obj)
 {
-   return msm_obj->gpu != NULL;
+   return atomic_read(_obj->active_count);
 }
 
 static inline bool is_purgeable(struct msm_gem_object *msm_obj)
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index 29c8d73c..55d1648 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -698,8 +698,8 @@ static void retire_submit(struct msm_gpu *gpu, struct 
msm_ringbuffer *ring,
 
for (i = 0; i < submit->nr_bos; i++) {
struct msm_gem_object *msm_obj = 

[PATCH 2/2] drm/msm: Leave inuse count intact on map failure

2020-09-21 Thread Akhil P Oommen
Leave the inuse count intact on map failure to keep the accounting
accurate.

Signed-off-by: Akhil P Oommen 
---
 drivers/gpu/drm/msm/msm_gem_vma.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/msm_gem_vma.c 
b/drivers/gpu/drm/msm/msm_gem_vma.c
index 80a8a26..8367a1c 100644
--- a/drivers/gpu/drm/msm/msm_gem_vma.c
+++ b/drivers/gpu/drm/msm/msm_gem_vma.c
@@ -88,8 +88,10 @@ msm_gem_map_vma(struct msm_gem_address_space *aspace,
ret = aspace->mmu->funcs->map(aspace->mmu, vma->iova, sgt,
size, prot);
 
-   if (ret)
+   if (ret) {
vma->mapped = false;
+   vma->inuse++;
+   }
 
return ret;
 }
-- 
2.7.4

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 0/3] drm: commit_work scheduling

2020-09-21 Thread Rob Clark
On Mon, Sep 21, 2020 at 8:16 AM Rob Clark  wrote:
>
> On Mon, Sep 21, 2020 at 2:21 AM Daniel Vetter  wrote:
> >
> > On Sat, Sep 19, 2020 at 12:37:23PM -0700, Rob Clark wrote:
> > > From: Rob Clark 
> > >
> > > The android userspace treats the display pipeline as a realtime problem.
> > > And arguably, if your goal is to not miss frame deadlines (ie. vblank),
> > > it is.  (See https://lwn.net/Articles/809545/ for the best explaination
> > > that I found.)
> > >
> > > But this presents a problem with using workqueues for non-blocking
> > > atomic commit_work(), because the SCHED_FIFO userspace thread(s) can
> > > preempt the worker.  Which is not really the outcome you want.. once
> > > the required fences are scheduled, you want to push the atomic commit
> > > down to hw ASAP.
> > >
> > > But the decision of whether commit_work should be RT or not really
> > > depends on what userspace is doing.  For a pure CFS userspace display
> > > pipeline, commit_work() should remain SCHED_NORMAL.
> > >
> > > To handle this, convert non-blocking commit_work() to use per-CRTC
> > > kthread workers, instead of system_unbound_wq.  Per-CRTC workers are
> > > used to avoid serializing commits when userspace is using a per-CRTC
> > > update loop.
> > >
> > > A client-cap is introduced so that userspace can opt-in to SCHED_FIFO
> > > priority commit work.
> > >
> > > A potential issue is that since 616d91b68cd ("sched: Remove
> > > sched_setscheduler*() EXPORTs") we have limited RT priority levels,
> > > meaning that commit_work() ends up running at the same priority level
> > > as vblank-work.  This shouldn't be a big problem *yet*, due to limited
> > > use of vblank-work at this point.  And if it could be arranged that
> > > vblank-work is scheduled before signaling out-fences and/or sending
> > > pageflip events, it could probably work ok to use a single priority
> > > level for both commit-work and vblank-work.
> >
> > The part I don't like about this is that it all feels rather hacked
> > together, and if we add more stuff (or there's some different thing in the
> > system that also needs rt scheduling) then it doesn't compose.
>
> The ideal thing would be that userspace is in control of the
> priorities.. the setclientcap approach seemed like a reasonable way to
> give the drm-master a way to opt in.
>
> I suppose instead userspace could use sched_setscheduler().. but that
> would require userspace to be root, and would require some way to find
> the tid.
>
> Is there some way we could arrange for the per-crtc kthread's to be
> owned by the drm master?  That would solve the "must be root" issue.
> And since the target audience is an atomic userspace, I suppose we
> could expose the tid as a read-only property on the crtc?

Side-note, we have the same issue with work scheduled when GPU
completes a batch/submit.. I'm less sure what to do with that, so
figured I'd start with the commit_work because that was the "easy"
part ;-)

The retire_work tends to complete quickly, so maybe sched_set_fifo()
is sufficient.  (That plus these days things that run android tend to
have 8 cores so you can kinda get away with multiple things at the
same RT priority level to some degree..)

> BR,
> -R
>
> > So question to rt/worker folks: What's the best way to let userspace set
> > the scheduling mode and priorities of things the kernel does on its
> > behalf? Surely we're not the first ones where if userspace runs with some
> > rt priority it'll starve out the kernel workers that it needs. Hardcoding
> > something behind a subsystem ioctl (which just means every time userspace
> > changes what it does, we need a new such flag or mode) can't be the right
> > thing.
> >
> > Peter, Tejun?
> >
> > Thanks, Daniel
> >
> > >
> > > Rob Clark (3):
> > >   drm/crtc: Introduce per-crtc kworker
> > >   drm/atomic: Use kthread worker for nonblocking commits
> > >   drm: Add a client-cap to set scheduling mode
> > >
> > >  drivers/gpu/drm/drm_atomic_helper.c | 13 ++
> > >  drivers/gpu/drm/drm_auth.c  |  4 
> > >  drivers/gpu/drm/drm_crtc.c  | 37 +
> > >  drivers/gpu/drm/drm_ioctl.c | 13 ++
> > >  include/drm/drm_atomic.h| 31 
> > >  include/drm/drm_crtc.h  | 10 
> > >  include/uapi/drm/drm.h  | 13 ++
> > >  7 files changed, 117 insertions(+), 4 deletions(-)
> > >
> > > --
> > > 2.26.2
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 0/3] drm: commit_work scheduling

2020-09-21 Thread Rob Clark
On Mon, Sep 21, 2020 at 2:21 AM Daniel Vetter  wrote:
>
> On Sat, Sep 19, 2020 at 12:37:23PM -0700, Rob Clark wrote:
> > From: Rob Clark 
> >
> > The android userspace treats the display pipeline as a realtime problem.
> > And arguably, if your goal is to not miss frame deadlines (ie. vblank),
> > it is.  (See https://lwn.net/Articles/809545/ for the best explaination
> > that I found.)
> >
> > But this presents a problem with using workqueues for non-blocking
> > atomic commit_work(), because the SCHED_FIFO userspace thread(s) can
> > preempt the worker.  Which is not really the outcome you want.. once
> > the required fences are scheduled, you want to push the atomic commit
> > down to hw ASAP.
> >
> > But the decision of whether commit_work should be RT or not really
> > depends on what userspace is doing.  For a pure CFS userspace display
> > pipeline, commit_work() should remain SCHED_NORMAL.
> >
> > To handle this, convert non-blocking commit_work() to use per-CRTC
> > kthread workers, instead of system_unbound_wq.  Per-CRTC workers are
> > used to avoid serializing commits when userspace is using a per-CRTC
> > update loop.
> >
> > A client-cap is introduced so that userspace can opt-in to SCHED_FIFO
> > priority commit work.
> >
> > A potential issue is that since 616d91b68cd ("sched: Remove
> > sched_setscheduler*() EXPORTs") we have limited RT priority levels,
> > meaning that commit_work() ends up running at the same priority level
> > as vblank-work.  This shouldn't be a big problem *yet*, due to limited
> > use of vblank-work at this point.  And if it could be arranged that
> > vblank-work is scheduled before signaling out-fences and/or sending
> > pageflip events, it could probably work ok to use a single priority
> > level for both commit-work and vblank-work.
>
> The part I don't like about this is that it all feels rather hacked
> together, and if we add more stuff (or there's some different thing in the
> system that also needs rt scheduling) then it doesn't compose.

The ideal thing would be that userspace is in control of the
priorities.. the setclientcap approach seemed like a reasonable way to
give the drm-master a way to opt in.

I suppose instead userspace could use sched_setscheduler().. but that
would require userspace to be root, and would require some way to find
the tid.

Is there some way we could arrange for the per-crtc kthread's to be
owned by the drm master?  That would solve the "must be root" issue.
And since the target audience is an atomic userspace, I suppose we
could expose the tid as a read-only property on the crtc?

BR,
-R

> So question to rt/worker folks: What's the best way to let userspace set
> the scheduling mode and priorities of things the kernel does on its
> behalf? Surely we're not the first ones where if userspace runs with some
> rt priority it'll starve out the kernel workers that it needs. Hardcoding
> something behind a subsystem ioctl (which just means every time userspace
> changes what it does, we need a new such flag or mode) can't be the right
> thing.
>
> Peter, Tejun?
>
> Thanks, Daniel
>
> >
> > Rob Clark (3):
> >   drm/crtc: Introduce per-crtc kworker
> >   drm/atomic: Use kthread worker for nonblocking commits
> >   drm: Add a client-cap to set scheduling mode
> >
> >  drivers/gpu/drm/drm_atomic_helper.c | 13 ++
> >  drivers/gpu/drm/drm_auth.c  |  4 
> >  drivers/gpu/drm/drm_crtc.c  | 37 +
> >  drivers/gpu/drm/drm_ioctl.c | 13 ++
> >  include/drm/drm_atomic.h| 31 
> >  include/drm/drm_crtc.h  | 10 
> >  include/uapi/drm/drm.h  | 13 ++
> >  7 files changed, 117 insertions(+), 4 deletions(-)
> >
> > --
> > 2.26.2
> >
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 4/7] drm/omap: Implement CTM property for CRTC using OVL managers CPR matrix

2020-09-21 Thread Tomi Valkeinen
Hi,

On 04/09/2019 23:20, Ilia Mirkin wrote:

>> Implement CTM color management property for OMAP CRTC using DSS
>> overlay manager's Color Phase Rotation matrix. The CPR matrix does not
>> exactly match the CTM property documentation. On DSS the CPR matrix is
>> applied after gamma table look up. However, it seems stupid to add a
>> custom property just for that.
>
> In that case the DRM documentation should be updated to mention that
> both options are allowed.

 Ok, if that is alright. But if we do that, then I guess all the drivers
 implementing CTM should document the point where it is applied in the
 pipeline.
>>>
>>> Whatever solution we end up picking, I think it should at least be
>>> discussed with a broader upstream audience and not just swept under the
>>> omapdrm carpet :-)
>>>
>>
>> I'll try to write something and send the next series to wider audience.
>> Let's see what jury says.
> 
> In case it's useful ... the pipeline normally goes degamma -> ctm ->
> gamma. If your ctm is applied after gamma, perhaps you can just rename
> "gamma" to "degamma" and be done? (There's the unfortunate case of
> legacy gamma which does end up in "GAMMA" when using atomic helpers.
> But in such a case, you won't have a CTM.)

Waking up old thread, as I started looking at these patches again. So the 
problem was that DRM
defines the output color transformations as:

degamma -> ctm -> gamma -> out

whereas OMAP DSS has

gamma -> ctm -> out

The omapdrm driver could declare the gamma table as degamma, as suggested by 
Ilia, and for the
legacy drmModeCrtcSetGamma, the omapdrm driver could implement its own version, 
and use the degamma
table internally (with no ctm).

For legacy, that would work fine and as expected, but I think the atomic 
version would be a bit odd,
with only degamma, and no gamma.

Quick grep for drm_crtc_enable_color_mgmt shows that there are other drivers 
that have CTM and
gamma, but no degamma. I wonder if all those have ctm -> gamma -> out, or are 
they similar to OMAP DSS.

Any thoughts on how to proceed with this?

Should we have a property that describes the order? Or new property name for 
gamma before ctm
(PREGAMMA)? Or just have it as degamma, and let the userspace deal with it 
(i.e. figure out there's
no gamma, but there's degamma, so use degamma)?

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 2/3] drm/atomic: Use kthread worker for nonblocking commits

2020-09-21 Thread Rob Clark
On Mon, Sep 21, 2020 at 2:23 AM Daniel Vetter  wrote:
>
> On Sat, Sep 19, 2020 at 12:37:25PM -0700, Rob Clark wrote:
> > From: Rob Clark 
> >
> > This will allow us to more easily switch scheduling rules based on what
> > userspace wants.
> >
> > Signed-off-by: Rob Clark 
>
> I still think switching to the highpriority systemwq as a start (like i915
> already does) would be a good first step no matter what we end up doing
> for the android thing.

highpri wq is probably better than the current state, but it doesn't
really address the problem.  You'll still end up with surfaceflinger
preempting commit_work..

And with non-RT priority, you'll still occasionally get lower priority
threads which haven't had a chance to run for a while preempting you.

BR,
-R


> -Daniel
>
> > ---
> >  drivers/gpu/drm/drm_atomic_helper.c | 13 
> >  include/drm/drm_atomic.h| 31 +
> >  2 files changed, 40 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > b/drivers/gpu/drm/drm_atomic_helper.c
> > index 9e1ad493e689..75eeec5e7b10 100644
> > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > @@ -1659,11 +1659,11 @@ static void commit_tail(struct drm_atomic_state 
> > *old_state)
> >   drm_atomic_state_put(old_state);
> >  }
> >
> > -static void commit_work(struct work_struct *work)
> > +static void commit_work(struct kthread_work *work)
> >  {
> >   struct drm_atomic_state *state = container_of(work,
> > struct drm_atomic_state,
> > -   commit_work);
> > +   commit_kwork);
> >   commit_tail(state);
> >  }
> >
> > @@ -1797,6 +1797,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
> >struct drm_atomic_state *state,
> >bool nonblock)
> >  {
> > + struct kthread_worker *worker = NULL;
> >   int ret;
> >
> >   if (state->async_update) {
> > @@ -1814,7 +1815,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
> >   if (ret)
> >   return ret;
> >
> > - INIT_WORK(>commit_work, commit_work);
> > + kthread_init_work(>commit_kwork, commit_work);
> >
> >   ret = drm_atomic_helper_prepare_planes(dev, state);
> >   if (ret)
> > @@ -1857,8 +1858,12 @@ int drm_atomic_helper_commit(struct drm_device *dev,
> >*/
> >
> >   drm_atomic_state_get(state);
> > +
> >   if (nonblock)
> > - queue_work(system_unbound_wq, >commit_work);
> > + worker = drm_atomic_pick_worker(state);
> > +
> > + if (worker)
> > + kthread_queue_work(worker, >commit_kwork);
> >   else
> >   commit_tail(state);
> >
> > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> > index d07c851d255b..8d0ee19953df 100644
> > --- a/include/drm/drm_atomic.h
> > +++ b/include/drm/drm_atomic.h
> > @@ -373,8 +373,18 @@ struct drm_atomic_state {
> >*
> >* Work item which can be used by the driver or helpers to execute the
> >* commit without blocking.
> > +  *
> > +  * This is deprecated, use commit_kwork.
> >*/
> >   struct work_struct commit_work;
> > +
> > + /**
> > +  * @commit_kwork:
> > +  *
> > +  * Work item which can be used by the driver or helpers to execute the
> > +  * commit without blocking.
> > +  */
> > + struct kthread_work commit_kwork;
> >  };
> >
> >  void __drm_crtc_commit_free(struct kref *kref);
> > @@ -954,6 +964,27 @@ void drm_state_dump(struct drm_device *dev, struct 
> > drm_printer *p);
> > (new_obj_state) = 
> > (__state)->private_objs[__i].new_state, 1); \
> >(__i)++)
> >
> > +/**
> > + * drm_atomic_pick_worker - helper to get kworker to use for nonblocking 
> > commit
> > + * @state: the _atomic_state for the commit
> > + *
> > + * Pick an appropriate worker for a given atomic update.  The first CRTC
> > + * invovled in the atomic update is used to pick the worker, to prevent
> > + * serializing multiple pageflips / atomic-updates on indenpendent CRTCs.
> > + */
> > +static inline struct kthread_worker *
> > +drm_atomic_pick_worker(const struct drm_atomic_state *state)
> > +{
> > + struct drm_crtc_state *crtc_state;
> > + struct drm_crtc *crtc;
> > + unsigned i;
> > +
> > + for_each_new_crtc_in_state(state, crtc, crtc_state, i)
> > + return crtc->worker;
> > +
> > + return NULL;
> > +}
> > +
> >  /**
> >   * drm_atomic_crtc_needs_modeset - compute combined modeset need
> >   * @state: _crtc_state for the CRTC
> > --
> > 2.26.2
> >
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org

[PATCH 03/11] drm/vmwgfx: remove unused placement combination

2020-09-21 Thread Christian König
Just some dead code cleanup.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h|  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 30 --
 2 files changed, 31 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 1523b51a7284..9ceee4eb0b13 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -1008,7 +1008,6 @@ extern struct ttm_placement vmw_vram_placement;
 extern struct ttm_placement vmw_vram_ne_placement;
 extern struct ttm_placement vmw_vram_sys_placement;
 extern struct ttm_placement vmw_vram_gmr_placement;
-extern struct ttm_placement vmw_vram_gmr_ne_placement;
 extern struct ttm_placement vmw_sys_placement;
 extern struct ttm_placement vmw_sys_ne_placement;
 extern struct ttm_placement vmw_evictable_placement;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 7454f797d37b..d7ea658e9910 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -65,13 +65,6 @@ static const struct ttm_place gmr_placement_flags = {
.flags = TTM_PL_FLAG_CACHED
 };
 
-static const struct ttm_place gmr_ne_placement_flags = {
-   .fpfn = 0,
-   .lpfn = 0,
-   .mem_type = VMW_PL_GMR,
-   .flags = TTM_PL_FLAG_CACHED | TTM_PL_FLAG_NO_EVICT
-};
-
 static const struct ttm_place mob_placement_flags = {
.fpfn = 0,
.lpfn = 0,
@@ -128,29 +121,6 @@ struct ttm_placement vmw_vram_gmr_placement = {
.busy_placement = _placement_flags
 };
 
-static const struct ttm_place vram_gmr_ne_placement_flags[] = {
-   {
-   .fpfn = 0,
-   .lpfn = 0,
-   .mem_type = TTM_PL_VRAM,
-   .flags = TTM_PL_FLAG_CACHED |
-TTM_PL_FLAG_NO_EVICT
-   }, {
-   .fpfn = 0,
-   .lpfn = 0,
-   .mem_type = VMW_PL_GMR,
-   .flags = TTM_PL_FLAG_CACHED |
-TTM_PL_FLAG_NO_EVICT
-   }
-};
-
-struct ttm_placement vmw_vram_gmr_ne_placement = {
-   .num_placement = 2,
-   .placement = vram_gmr_ne_placement_flags,
-   .num_busy_placement = 1,
-   .busy_placement = _ne_placement_flags
-};
-
 struct ttm_placement vmw_vram_sys_placement = {
.num_placement = 1,
.placement = _placement_flags,
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 04/11] drm/vmwgfx: stop using ttm_bo_create

2020-09-21 Thread Christian König
Implement in the driver instead since it is the only user of that function.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 42 ++
 drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c |  6 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h|  4 +++
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  8 ++---
 4 files changed, 52 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 813f1b148094..30d19b45b602 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -487,6 +487,48 @@ static void vmw_user_bo_destroy(struct ttm_buffer_object 
*bo)
ttm_prime_object_kfree(vmw_user_bo, prime);
 }
 
+/**
+ * vmw_bo_create_kernel - Create a pinned BO for internal kernel use.
+ *
+ * @dev_priv: Pointer to the device private struct
+ * @size: size of the BO we need
+ * @placement: where to put it
+ * @p_bo: resulting BO
+ *
+ * Creates and pin a simple BO for in kernel use.
+ */
+int vmw_bo_create_kernel(struct vmw_private *dev_priv, unsigned long size,
+struct ttm_placement *placement,
+struct ttm_buffer_object **p_bo)
+{
+   unsigned npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+   struct ttm_buffer_object *bo;
+   size_t acc_size;
+   int ret;
+
+   bo = kzalloc(sizeof(*bo), GFP_KERNEL);
+   if (unlikely(!bo))
+   return -ENOMEM;
+
+   acc_size = ttm_round_pot(sizeof(*bo));
+   acc_size += ttm_round_pot(npages * sizeof(void *));
+   acc_size += ttm_round_pot(sizeof(struct ttm_tt));
+   ret = ttm_bo_init_reserved(_priv->bdev, bo, size,
+  ttm_bo_type_device, placement, 0,
+  false, acc_size, NULL, NULL, NULL);
+   if (unlikely(ret))
+   goto error_free;
+
+   ttm_bo_pin(bo);
+   ttm_bo_unreserve(bo);
+   *p_bo = bo;
+
+   return 0;
+
+error_free:
+   kfree(bo);
+   return ret;
+}
 
 /**
  * vmw_bo_init - Initialize a vmw buffer object
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
index 3b41cf63110a..9a9fe10d829b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c
@@ -1245,9 +1245,9 @@ int vmw_cmdbuf_set_pool_size(struct vmw_cmdbuf_man *man,
!dev_priv->has_mob)
return -ENOMEM;
 
-   ret = ttm_bo_create(_priv->bdev, size, ttm_bo_type_device,
-   _mob_ne_placement, 0, false,
-   >cmd_space);
+   ret = vmw_bo_create_kernel(dev_priv, size,
+  _mob_placement,
+  >cmd_space);
if (ret)
return ret;
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h 
b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 9ceee4eb0b13..5d07de5183e1 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -845,6 +845,10 @@ extern void vmw_bo_get_guest_ptr(const struct 
ttm_buffer_object *buf,
 SVGAGuestPtr *ptr);
 extern void vmw_bo_pin_reserved(struct vmw_buffer_object *bo, bool pin);
 extern void vmw_bo_bo_free(struct ttm_buffer_object *bo);
+extern int vmw_bo_create_kernel(struct vmw_private *dev_priv,
+   unsigned long size,
+   struct ttm_placement *placement,
+   struct ttm_buffer_object **p_bo);
 extern int vmw_bo_init(struct vmw_private *dev_priv,
   struct vmw_buffer_object *vmw_bo,
   size_t size, struct ttm_placement *placement,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index d7ea658e9910..39a2f720f4ed 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -793,11 +793,9 @@ int vmw_bo_create_and_populate(struct vmw_private 
*dev_priv,
struct ttm_buffer_object *bo;
int ret;
 
-   ret = ttm_bo_create(_priv->bdev, bo_size,
-   ttm_bo_type_device,
-   _sys_ne_placement,
-   0, false, );
-
+   ret = vmw_bo_create_kernel(dev_priv, bo_size,
+  _sys_placement,
+  );
if (unlikely(ret != 0))
return ret;
 
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Nuke TTM_PL_FLAG_NO_EVICT

2020-09-21 Thread Christian König
Hi guys,

The TTM_PL_FLAG_NO_EVICT flag was never a placement flag to begin with. Instead 
it affects LRU and eviction handling.

So clean this up and provide the common logic of pinning/unpinning a buffer 
object instead.

Since this affects basically all the driver using TTM please comment and/or 
test the patches.

Regards,
Christian.


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 01/11] drm/ttm: add ttm_bo_pin()/ttm_bo_unpin()

2020-09-21 Thread Christian König
As an alternative to the placement flag add a
pin count to the ttm buffer object.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo.c  |  9 ++---
 drivers/gpu/drm/ttm/ttm_bo_util.c |  2 +-
 include/drm/ttm/ttm_bo_api.h  | 24 
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 70b3bee27850..b82b49d43942 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -115,7 +115,7 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object 
*bo,
struct ttm_bo_device *bdev = bo->bdev;
struct ttm_resource_manager *man;
 
-   if (!list_empty(>lru))
+   if (!list_empty(>lru) || bo->pin_count)
return;
 
if (mem->placement & TTM_PL_FLAG_NO_EVICT)
@@ -165,7 +165,8 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
ttm_bo_del_from_lru(bo);
ttm_bo_add_mem_to_lru(bo, >mem);
 
-   if (bulk && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT)) {
+   if (bulk && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT) &&
+   !bo->pin_count) {
switch (bo->mem.mem_type) {
case TTM_PL_TT:
ttm_bo_bulk_move_set_pos(>tt[bo->priority], bo);
@@ -544,8 +545,9 @@ static void ttm_bo_release(struct kref *kref)
 * shrinkers, now that they are queued for
 * destruction.
 */
-   if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) {
+   if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT || bo->pin_count) {
bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
+   bo->pin_count = 0;
ttm_bo_del_from_lru(bo);
ttm_bo_add_mem_to_lru(bo, >mem);
}
@@ -1172,6 +1174,7 @@ int ttm_bo_init_reserved(struct ttm_bo_device *bdev,
bo->moving = NULL;
bo->mem.placement = TTM_PL_FLAG_CACHED;
bo->acc_size = acc_size;
+   bo->pin_count = 0;
bo->sg = sg;
if (resv) {
bo->base.resv = resv;
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index fb2a25f8408f..1968df9743fc 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -352,7 +352,6 @@ static int ttm_buffer_object_transfer(struct 
ttm_buffer_object *bo,
return -ENOMEM;
 
fbo->base = *bo;
-   fbo->base.mem.placement |= TTM_PL_FLAG_NO_EVICT;
 
ttm_bo_get(bo);
fbo->bo = bo;
@@ -372,6 +371,7 @@ static int ttm_buffer_object_transfer(struct 
ttm_buffer_object *bo,
kref_init(>base.kref);
fbo->base.destroy = _transfered_destroy;
fbo->base.acc_size = 0;
+   fbo->base.pin_count = 1;
if (bo->type != ttm_bo_type_sg)
fbo->base.base.resv = >base.base._resv;
 
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 0f7cd21d6d74..83979500df1c 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -157,6 +157,7 @@ struct ttm_buffer_object {
 
struct dma_fence *moving;
unsigned priority;
+   unsigned pin_count;
 
/**
 * Special members that are protected by the reserve lock
@@ -606,6 +607,29 @@ static inline bool ttm_bo_uses_embedded_gem_object(struct 
ttm_buffer_object *bo)
return bo->base.dev != NULL;
 }
 
+/**
+ * ttm_bo_pin - Pin the buffer object.
+ * @bo: The buffer object to pin
+ *
+ * Make sure the buffer is not evicted any more during memory pressure.
+ */
+static inline void ttm_bo_pin(struct ttm_buffer_object *bo)
+{
+   ++bo->pin_count;
+}
+
+/**
+ * ttm_bo_unpin - Unpin the buffer object.
+ * @bo: The buffer object to unpin
+ *
+ * Allows the buffer object to be evicted again during memory pressure.
+ */
+static inline void ttm_bo_unpin(struct ttm_buffer_object *bo)
+{
+   WARN_ON_ONCE(!bo->pin_count);
+   --bo->pin_count;
+}
+
 int ttm_mem_evict_first(struct ttm_bo_device *bdev,
struct ttm_resource_manager *man,
const struct ttm_place *place,
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 07/11] drm/qxl: switch over to the new pin interface

2020-09-21 Thread Christian König
Stop using TTM_PL_FLAG_NO_EVICT.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/qxl/qxl_debugfs.c |  2 +-
 drivers/gpu/drm/qxl/qxl_drv.h |  1 -
 drivers/gpu/drm/qxl/qxl_ioctl.c   |  4 +--
 drivers/gpu/drm/qxl/qxl_object.c  | 44 +--
 drivers/gpu/drm/qxl/qxl_object.h  |  2 +-
 drivers/gpu/drm/qxl/qxl_release.c |  4 +--
 drivers/gpu/drm/qxl/qxl_ttm.c |  2 +-
 7 files changed, 20 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c 
b/drivers/gpu/drm/qxl/qxl_debugfs.c
index 524d35b648d8..183d15e2cf58 100644
--- a/drivers/gpu/drm/qxl/qxl_debugfs.c
+++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
@@ -67,7 +67,7 @@ qxl_debugfs_buffers_info(struct seq_file *m, void *data)
 
seq_printf(m, "size %ld, pc %d, num releases %d\n",
   (unsigned long)bo->tbo.base.size,
-  bo->pin_count, rel);
+  bo->tbo.pin_count, rel);
}
return 0;
 }
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index aae90a9ee1db..3602e8b34189 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -80,7 +80,6 @@ struct qxl_bo {
struct ttm_placeplacements[3];
struct ttm_placementplacement;
struct ttm_bo_kmap_obj  kmap;
-   unsigned int pin_count;
void*kptr;
unsigned intmap_count;
int type;
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index 5cea6eea72ab..0bab9ec6adc1 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -326,8 +326,8 @@ static int qxl_update_area_ioctl(struct drm_device *dev, 
void *data,
if (ret)
goto out;
 
-   if (!qobj->pin_count) {
-   qxl_ttm_placement_from_domain(qobj, qobj->type, false);
+   if (!qobj->tbo.pin_count) {
+   qxl_ttm_placement_from_domain(qobj, qobj->type);
ret = ttm_bo_validate(>tbo, >placement, );
if (unlikely(ret))
goto out;
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 2bc364412e8b..d3635e3e3267 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -51,14 +51,12 @@ bool qxl_ttm_bo_is_qxl_bo(struct ttm_buffer_object *bo)
return false;
 }
 
-void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 domain, bool pinned)
+void qxl_ttm_placement_from_domain(struct qxl_bo *qbo, u32 domain)
 {
u32 c = 0;
u32 pflag = 0;
unsigned int i;
 
-   if (pinned)
-   pflag |= TTM_PL_FLAG_NO_EVICT;
if (qbo->tbo.base.size <= PAGE_SIZE)
pflag |= TTM_PL_FLAG_TOPDOWN;
 
@@ -128,14 +126,13 @@ int qxl_bo_create(struct qxl_device *qdev,
}
bo->tbo.base.funcs = _object_funcs;
bo->type = domain;
-   bo->pin_count = pinned ? 1 : 0;
bo->surface_id = 0;
INIT_LIST_HEAD(>list);
 
if (surf)
bo->surf = *surf;
 
-   qxl_ttm_placement_from_domain(bo, domain, pinned);
+   qxl_ttm_placement_from_domain(bo, domain);
 
r = ttm_bo_init(>mman.bdev, >tbo, size, type,
>placement, 0, !kernel, size,
@@ -147,6 +144,8 @@ int qxl_bo_create(struct qxl_device *qdev,
size, domain);
return r;
}
+   if (pinned)
+   ttm_bo_pin(>tbo);
*bo_ptr = bo;
return 0;
 }
@@ -248,39 +247,22 @@ static int __qxl_bo_pin(struct qxl_bo *bo)
struct drm_device *ddev = bo->tbo.base.dev;
int r;
 
-   if (bo->pin_count) {
-   bo->pin_count++;
+   if (bo->tbo.pin_count) {
+   ttm_bo_pin(>tbo);
return 0;
}
-   qxl_ttm_placement_from_domain(bo, bo->type, true);
+   qxl_ttm_placement_from_domain(bo, bo->type);
r = ttm_bo_validate(>tbo, >placement, );
-   if (likely(r == 0)) {
-   bo->pin_count = 1;
-   }
+   if (likely(r == 0))
+   ttm_bo_pin(>tbo);
if (unlikely(r != 0))
dev_err(ddev->dev, "%p pin failed\n", bo);
return r;
 }
 
-static int __qxl_bo_unpin(struct qxl_bo *bo)
+static void __qxl_bo_unpin(struct qxl_bo *bo)
 {
-   struct ttm_operation_ctx ctx = { false, false };
-   struct drm_device *ddev = bo->tbo.base.dev;
-   int r, i;
-
-   if (!bo->pin_count) {
-   dev_warn(ddev->dev, "%p unpin not necessary\n", bo);
-   return 0;
-   }
-   bo->pin_count--;
-   if (bo->pin_count)
-   return 0;
-   for (i = 0; i < bo->placement.num_placement; i++)
-   bo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT;
-   r = ttm_bo_validate(>tbo, >placement, );
-   if (unlikely(r != 

[PATCH 09/11] drm/amdgpu: switch over to the new pin interface

2020-09-21 Thread Christian König
Stop using TTM_PL_FLAG_NO_EVICT.

Signed-off-by: Christian König 
---
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  5 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_display.c   |  8 +---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c   |  5 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.c| 41 +--
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c   |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c|  2 +-
 9 files changed, 24 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index b6b821500d30..64d4b5ff95d6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -1479,7 +1479,7 @@ int amdgpu_amdkfd_gpuvm_map_memory_to_gpu(
}
}
 
-   if (!amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) && !bo->pin_count)
+   if (!amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) && !bo->tbo.pin_count)
amdgpu_bo_fence(bo,
>process_info->eviction_fence->base,
true);
@@ -1558,7 +1558,8 @@ int amdgpu_amdkfd_gpuvm_unmap_memory_from_gpu(
 * required.
 */
if (mem->mapped_to_gpu_memory == 0 &&
-   !amdgpu_ttm_tt_get_usermm(mem->bo->tbo.ttm) && !mem->bo->pin_count)
+   !amdgpu_ttm_tt_get_usermm(mem->bo->tbo.ttm) &&
+   !mem->bo->tbo.pin_count)
amdgpu_amdkfd_remove_eviction_fence(mem->bo,
process_info->eviction_fence);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 12598a4b5c78..d50b63a93d37 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -410,7 +410,7 @@ static int amdgpu_cs_bo_validate(struct amdgpu_cs_parser *p,
uint32_t domain;
int r;
 
-   if (bo->pin_count)
+   if (bo->tbo.pin_count)
return 0;
 
/* Don't move this buffer if we have depleted our allowance
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
index c81206e6096f..4cba095b6c44 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
@@ -132,10 +132,7 @@ static void amdgpu_display_unpin_work_func(struct 
work_struct *__work)
/* unpin of the old buffer */
r = amdgpu_bo_reserve(work->old_abo, true);
if (likely(r == 0)) {
-   r = amdgpu_bo_unpin(work->old_abo);
-   if (unlikely(r != 0)) {
-   DRM_ERROR("failed to unpin buffer after flip\n");
-   }
+   amdgpu_bo_unpin(work->old_abo);
amdgpu_bo_unreserve(work->old_abo);
} else
DRM_ERROR("failed to reserve buffer after flip\n");
@@ -249,8 +246,7 @@ int amdgpu_display_crtc_page_flip_target(struct drm_crtc 
*crtc,
}
 unpin:
if (!adev->enable_virtual_display)
-   if (unlikely(amdgpu_bo_unpin(new_abo) != 0))
-   DRM_ERROR("failed to unpin new abo in error path\n");
+   amdgpu_bo_unpin(new_abo);
 
 unreserve:
amdgpu_bo_unreserve(new_abo);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 957934926b24..5b465ab774d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -281,7 +281,7 @@ static struct sg_table *amdgpu_dma_buf_map(struct 
dma_buf_attachment *attach,
struct sg_table *sgt;
long r;
 
-   if (!bo->pin_count) {
+   if (!bo->tbo.pin_count) {
/* move buffer into GTT or VRAM */
struct ttm_operation_ctx ctx = { false, false };
unsigned domains = AMDGPU_GEM_DOMAIN_GTT;
@@ -390,7 +390,8 @@ static int amdgpu_dma_buf_begin_cpu_access(struct dma_buf 
*dma_buf,
if (unlikely(ret != 0))
return ret;
 
-   if (!bo->pin_count && (bo->allowed_domains & AMDGPU_GEM_DOMAIN_GTT)) {
+   if (!bo->tbo.pin_count &&
+   (bo->allowed_domains & AMDGPU_GEM_DOMAIN_GTT)) {
amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_GTT);
ret = ttm_bo_validate(>tbo, >placement, );
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index aa7f230c71bf..59b52804622d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -860,7 +860,7 @@ static int amdgpu_debugfs_gem_bo_info(int id, void *ptr, 
void *data)
seq_printf(m, "\t0x%08x: %12ld byte %s",
   id, amdgpu_bo_size(bo), placement);
 
-   pin_count = 

[PATCH 10/11] drm/ttm: remove ttm_bo_create

2020-09-21 Thread Christian König
Not used any more.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo.c | 40 
 include/drm/ttm/ttm_bo_api.h | 24 --
 2 files changed, 64 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index b82b49d43942..1a4b25083326 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1254,19 +1254,6 @@ int ttm_bo_init(struct ttm_bo_device *bdev,
 }
 EXPORT_SYMBOL(ttm_bo_init);
 
-static size_t ttm_bo_acc_size(struct ttm_bo_device *bdev,
- unsigned long bo_size,
- unsigned struct_size)
-{
-   unsigned npages = (PAGE_ALIGN(bo_size)) >> PAGE_SHIFT;
-   size_t size = 0;
-
-   size += ttm_round_pot(struct_size);
-   size += ttm_round_pot(npages * sizeof(void *));
-   size += ttm_round_pot(sizeof(struct ttm_tt));
-   return size;
-}
-
 size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev,
   unsigned long bo_size,
   unsigned struct_size)
@@ -1281,33 +1268,6 @@ size_t ttm_bo_dma_acc_size(struct ttm_bo_device *bdev,
 }
 EXPORT_SYMBOL(ttm_bo_dma_acc_size);
 
-int ttm_bo_create(struct ttm_bo_device *bdev,
-   unsigned long size,
-   enum ttm_bo_type type,
-   struct ttm_placement *placement,
-   uint32_t page_alignment,
-   bool interruptible,
-   struct ttm_buffer_object **p_bo)
-{
-   struct ttm_buffer_object *bo;
-   size_t acc_size;
-   int ret;
-
-   bo = kzalloc(sizeof(*bo), GFP_KERNEL);
-   if (unlikely(bo == NULL))
-   return -ENOMEM;
-
-   acc_size = ttm_bo_acc_size(bdev, size, sizeof(struct 
ttm_buffer_object));
-   ret = ttm_bo_init(bdev, bo, size, type, placement, page_alignment,
- interruptible, acc_size,
- NULL, NULL, NULL);
-   if (likely(ret == 0))
-   *p_bo = bo;
-
-   return ret;
-}
-EXPORT_SYMBOL(ttm_bo_create);
-
 int ttm_bo_evict_mm(struct ttm_bo_device *bdev, unsigned mem_type)
 {
struct ttm_resource_manager *man = ttm_manager_type(bdev, mem_type);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 83979500df1c..ce592c9061e8 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -447,30 +447,6 @@ int ttm_bo_init(struct ttm_bo_device *bdev, struct 
ttm_buffer_object *bo,
struct sg_table *sg, struct dma_resv *resv,
void (*destroy) (struct ttm_buffer_object *));
 
-/**
- * ttm_bo_create
- *
- * @bdev: Pointer to a ttm_bo_device struct.
- * @size: Requested size of buffer object.
- * @type: Requested type of buffer object.
- * @placement: Initial placement.
- * @page_alignment: Data alignment in pages.
- * @interruptible: If needing to sleep while waiting for GPU resources,
- * sleep interruptible.
- * @p_bo: On successful completion *p_bo points to the created object.
- *
- * This function allocates a ttm_buffer_object, and then calls ttm_bo_init
- * on that object. The destroy function is set to kfree().
- * Returns
- * -ENOMEM: Out of memory.
- * -EINVAL: Invalid placement flags.
- * -ERESTARTSYS: Interrupted by signal while waiting for resources.
- */
-int ttm_bo_create(struct ttm_bo_device *bdev, unsigned long size,
- enum ttm_bo_type type, struct ttm_placement *placement,
- uint32_t page_alignment, bool interruptible,
- struct ttm_buffer_object **p_bo);
-
 /**
  * ttm_bo_evict_mm
  *
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 05/11] drm/vmwgfx: switch over to the new pin interface

2020-09-21 Thread Christian König
Stop using TTM_PL_FLAG_NO_EVICT.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_blit.c   |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_bo.c | 48 +++---
 drivers/gpu/drm/vmwgfx/vmwgfx_cotable.c|  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c|  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h|  7 +---
 drivers/gpu/drm/vmwgfx/vmwgfx_fb.c |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_resource.c   |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c   |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_shader.c |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 42 ---
 drivers/gpu/drm/vmwgfx/vmwgfx_validation.c |  2 +-
 11 files changed, 38 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
index e8d66182cd7b..ea2f2f937eb3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_blit.c
@@ -459,9 +459,9 @@ int vmw_bo_cpu_blit(struct ttm_buffer_object *dst,
int ret = 0;
 
/* Buffer objects need to be either pinned or reserved: */
-   if (!(dst->mem.placement & TTM_PL_FLAG_NO_EVICT))
+   if (!(dst->pin_count))
dma_resv_assert_held(dst->base.resv);
-   if (!(src->mem.placement & TTM_PL_FLAG_NO_EVICT))
+   if (!(src->pin_count))
dma_resv_assert_held(src->base.resv);
 
if (!ttm_tt_is_populated(dst->ttm)) {
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
index 30d19b45b602..236eaa163917 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_bo.c
@@ -106,7 +106,7 @@ int vmw_bo_pin_in_placement(struct vmw_private *dev_priv,
if (unlikely(ret != 0))
goto err;
 
-   if (buf->pin_count > 0)
+   if (buf->base.pin_count > 0)
ret = ttm_bo_mem_compat(placement, >mem,
_flags) == true ? 0 : -EINVAL;
else
@@ -155,7 +155,7 @@ int vmw_bo_pin_in_vram_or_gmr(struct vmw_private *dev_priv,
if (unlikely(ret != 0))
goto err;
 
-   if (buf->pin_count > 0) {
+   if (buf->base.pin_count > 0) {
ret = ttm_bo_mem_compat(_vram_gmr_placement, >mem,
_flags) == true ? 0 : -EINVAL;
goto out_unreserve;
@@ -246,12 +246,12 @@ int vmw_bo_pin_in_start_of_vram(struct vmw_private 
*dev_priv,
if (bo->mem.mem_type == TTM_PL_VRAM &&
bo->mem.start < bo->num_pages &&
bo->mem.start > 0 &&
-   buf->pin_count == 0) {
+   buf->base.pin_count == 0) {
ctx.interruptible = false;
(void) ttm_bo_validate(bo, _sys_placement, );
}
 
-   if (buf->pin_count > 0)
+   if (buf->base.pin_count > 0)
ret = ttm_bo_mem_compat(, >mem,
_flags) == true ? 0 : -EINVAL;
else
@@ -343,23 +343,13 @@ void vmw_bo_pin_reserved(struct vmw_buffer_object *vbo, 
bool pin)
 
dma_resv_assert_held(bo->base.resv);
 
-   if (pin) {
-   if (vbo->pin_count++ > 0)
-   return;
-   } else {
-   WARN_ON(vbo->pin_count <= 0);
-   if (--vbo->pin_count > 0)
-   return;
-   }
+   if (pin == !!bo->pin_count)
+   return;
 
pl.fpfn = 0;
pl.lpfn = 0;
pl.mem_type = bo->mem.mem_type;
pl.flags = bo->mem.placement;
-   if (pin)
-   pl.flags |= TTM_PL_FLAG_NO_EVICT;
-   else
-   pl.flags &= ~TTM_PL_FLAG_NO_EVICT;
 
memset(, 0, sizeof(placement));
placement.num_placement = 1;
@@ -368,8 +358,12 @@ void vmw_bo_pin_reserved(struct vmw_buffer_object *vbo, 
bool pin)
ret = ttm_bo_validate(bo, , );
 
BUG_ON(ret != 0 || bo->mem.mem_type != old_mem_type);
-}
 
+   if (pin)
+   ttm_bo_pin(bo);
+   else
+   ttm_bo_unpin(bo);
+}
 
 /**
  * vmw_bo_map_and_cache - Map a buffer object and cache the map
@@ -538,6 +532,7 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, 
unsigned long size,
  * @size: Buffer object size in bytes.
  * @placement: Initial placement.
  * @interruptible: Whether waits should be performed interruptible.
+ * @pin: If the BO should be created pinned at a fixed location.
  * @bo_free: The buffer object destructor.
  * Returns: Zero on success, negative error code on error.
  *
@@ -546,9 +541,10 @@ int vmw_bo_create_kernel(struct vmw_private *dev_priv, 
unsigned long size,
 int vmw_bo_init(struct vmw_private *dev_priv,
struct vmw_buffer_object *vmw_bo,
size_t size, struct ttm_placement *placement,
-   bool interruptible,
+   bool interruptible, bool pin,
void (*bo_free)(struct ttm_buffer_object *bo))
 {
+   struct ttm_operation_ctx ctx = { 

[PATCH 11/11] drm/ttm: remove TTM_PL_FLAG_NO_EVICT

2020-09-21 Thread Christian König
Not used any more.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo.c| 11 +++
 include/drm/ttm/ttm_placement.h |  1 -
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 1a4b25083326..5737b3fae1b3 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -118,9 +118,6 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object 
*bo,
if (!list_empty(>lru) || bo->pin_count)
return;
 
-   if (mem->placement & TTM_PL_FLAG_NO_EVICT)
-   return;
-
man = ttm_manager_type(bdev, mem->mem_type);
list_add_tail(>lru, >lru[bo->priority]);
 
@@ -165,8 +162,7 @@ void ttm_bo_move_to_lru_tail(struct ttm_buffer_object *bo,
ttm_bo_del_from_lru(bo);
ttm_bo_add_mem_to_lru(bo, >mem);
 
-   if (bulk && !(bo->mem.placement & TTM_PL_FLAG_NO_EVICT) &&
-   !bo->pin_count) {
+   if (bulk && !bo->pin_count) {
switch (bo->mem.mem_type) {
case TTM_PL_TT:
ttm_bo_bulk_move_set_pos(>tt[bo->priority], bo);
@@ -541,12 +537,11 @@ static void ttm_bo_release(struct kref *kref)
spin_lock(_bo_glob.lru_lock);
 
/*
-* Make NO_EVICT bos immediately available to
+* Make pinned bos immediately available to
 * shrinkers, now that they are queued for
 * destruction.
 */
-   if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT || bo->pin_count) {
-   bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT;
+   if (bo->pin_count) {
bo->pin_count = 0;
ttm_bo_del_from_lru(bo);
ttm_bo_add_mem_to_lru(bo, >mem);
diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
index d4022655eae4..50e72df48b8d 100644
--- a/include/drm/ttm/ttm_placement.h
+++ b/include/drm/ttm/ttm_placement.h
@@ -58,7 +58,6 @@
 #define TTM_PL_FLAG_UNCACHED(1 << 17)
 #define TTM_PL_FLAG_WC  (1 << 18)
 #define TTM_PL_FLAG_CONTIGUOUS  (1 << 19)
-#define TTM_PL_FLAG_NO_EVICT(1 << 21)
 #define TTM_PL_FLAG_TOPDOWN (1 << 22)
 
 #define TTM_PL_MASK_CACHING (TTM_PL_FLAG_CACHED | \
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 06/11] drm/vram-helper: switch over to the new pin interface

2020-09-21 Thread Christian König
Stop using TTM_PL_FLAG_NO_EVICT.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 37 +++
 include/drm/drm_gem_vram_helper.h |  3 ---
 2 files changed, 9 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 50cad0e4a92e..bd3dd17ad81d 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -301,7 +301,7 @@ static u64 drm_gem_vram_pg_offset(struct 
drm_gem_vram_object *gbo)
  */
 s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo)
 {
-   if (WARN_ON_ONCE(!gbo->pin_count))
+   if (WARN_ON_ONCE(!gbo->bo.pin_count))
return (s64)-ENODEV;
return drm_gem_vram_pg_offset(gbo) << PAGE_SHIFT;
 }
@@ -310,24 +310,21 @@ EXPORT_SYMBOL(drm_gem_vram_offset);
 static int drm_gem_vram_pin_locked(struct drm_gem_vram_object *gbo,
   unsigned long pl_flag)
 {
-   int i, ret;
struct ttm_operation_ctx ctx = { false, false };
+   int ret;
 
-   if (gbo->pin_count)
+   if (gbo->bo.pin_count)
goto out;
 
if (pl_flag)
drm_gem_vram_placement(gbo, pl_flag);
 
-   for (i = 0; i < gbo->placement.num_placement; ++i)
-   gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
-
ret = ttm_bo_validate(>bo, >placement, );
if (ret < 0)
return ret;
 
 out:
-   ++gbo->pin_count;
+   ttm_bo_pin(>bo);
 
return 0;
 }
@@ -369,26 +366,9 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, 
unsigned long pl_flag)
 }
 EXPORT_SYMBOL(drm_gem_vram_pin);
 
-static int drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo)
+static void drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo)
 {
-   int i, ret;
-   struct ttm_operation_ctx ctx = { false, false };
-
-   if (WARN_ON_ONCE(!gbo->pin_count))
-   return 0;
-
-   --gbo->pin_count;
-   if (gbo->pin_count)
-   return 0;
-
-   for (i = 0; i < gbo->placement.num_placement ; ++i)
-   gbo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT;
-
-   ret = ttm_bo_validate(>bo, >placement, );
-   if (ret < 0)
-   return ret;
-
-   return 0;
+   ttm_bo_unpin(>bo);
 }
 
 /**
@@ -406,10 +386,11 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
ret = ttm_bo_reserve(>bo, true, false, NULL);
if (ret)
return ret;
-   ret = drm_gem_vram_unpin_locked(gbo);
+
+   drm_gem_vram_unpin_locked(gbo);
ttm_bo_unreserve(>bo);
 
-   return ret;
+   return 0;
 }
 EXPORT_SYMBOL(drm_gem_vram_unpin);
 
diff --git a/include/drm/drm_gem_vram_helper.h 
b/include/drm/drm_gem_vram_helper.h
index 62cc6e6c3a4f..128f88174d32 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -35,7 +35,6 @@ struct vm_area_struct;
  * @placement: TTM placement information. Supported placements are \
%TTM_PL_VRAM and %TTM_PL_SYSTEM
  * @placements:TTM placement information.
- * @pin_count: Pin counter
  *
  * The type struct drm_gem_vram_object represents a GEM object that is
  * backed by VRAM. It can be used for simple framebuffer devices with
@@ -64,8 +63,6 @@ struct drm_gem_vram_object {
/* Supported placements are %TTM_PL_VRAM and %TTM_PL_SYSTEM */
struct ttm_placement placement;
struct ttm_place placements[2];
-
-   int pin_count;
 };
 
 /**
-- 
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH 02/11] drm/radeon: switch over to the new pin interface

2020-09-21 Thread Christian König
Stop using TTM_PL_FLAG_NO_EVICT.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/radeon/radeon.h |  1 -
 drivers/gpu/drm/radeon/radeon_display.c |  9 ++
 drivers/gpu/drm/radeon/radeon_object.c  | 37 ++---
 drivers/gpu/drm/radeon/radeon_object.h  |  2 +-
 drivers/gpu/drm/radeon/radeon_ttm.c |  2 +-
 5 files changed, 13 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index a6d8de01194a..5d54bccebd4d 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -497,7 +497,6 @@ struct radeon_bo {
struct ttm_buffer_objecttbo;
struct ttm_bo_kmap_obj  kmap;
u32 flags;
-   unsignedpin_count;
void*kptr;
u32 tiling_flags;
u32 pitch;
diff --git a/drivers/gpu/drm/radeon/radeon_display.c 
b/drivers/gpu/drm/radeon/radeon_display.c
index 7b69d6dfe44a..3eacf33bbe48 100644
--- a/drivers/gpu/drm/radeon/radeon_display.c
+++ b/drivers/gpu/drm/radeon/radeon_display.c
@@ -273,10 +273,7 @@ static void radeon_unpin_work_func(struct work_struct 
*__work)
/* unpin of the old buffer */
r = radeon_bo_reserve(work->old_rbo, false);
if (likely(r == 0)) {
-   r = radeon_bo_unpin(work->old_rbo);
-   if (unlikely(r != 0)) {
-   DRM_ERROR("failed to unpin buffer after flip\n");
-   }
+   radeon_bo_unpin(work->old_rbo);
radeon_bo_unreserve(work->old_rbo);
} else
DRM_ERROR("failed to reserve buffer after flip\n");
@@ -607,9 +604,7 @@ static int radeon_crtc_page_flip_target(struct drm_crtc 
*crtc,
DRM_ERROR("failed to reserve new rbo in error path\n");
goto cleanup;
}
-   if (unlikely(radeon_bo_unpin(new_rbo) != 0)) {
-   DRM_ERROR("failed to unpin new rbo in error path\n");
-   }
+   radeon_bo_unpin(new_rbo);
radeon_bo_unreserve(new_rbo);
 
 cleanup:
diff --git a/drivers/gpu/drm/radeon/radeon_object.c 
b/drivers/gpu/drm/radeon/radeon_object.c
index 316e35d3f8a9..0de267ab3913 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -334,8 +334,8 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 
domain, u64 max_offset,
if (radeon_ttm_tt_has_userptr(bo->rdev, bo->tbo.ttm))
return -EPERM;
 
-   if (bo->pin_count) {
-   bo->pin_count++;
+   if (bo->tbo.pin_count) {
+   ttm_bo_pin(>tbo);
if (gpu_addr)
*gpu_addr = radeon_bo_gpu_offset(bo);
 
@@ -367,13 +367,11 @@ int radeon_bo_pin_restricted(struct radeon_bo *bo, u32 
domain, u64 max_offset,
bo->rdev->mc.visible_vram_size >> PAGE_SHIFT;
else
bo->placements[i].lpfn = max_offset >> PAGE_SHIFT;
-
-   bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
}
 
r = ttm_bo_validate(>tbo, >placement, );
if (likely(r == 0)) {
-   bo->pin_count = 1;
+   ttm_bo_pin(>tbo);
if (gpu_addr != NULL)
*gpu_addr = radeon_bo_gpu_offset(bo);
if (domain == RADEON_GEM_DOMAIN_VRAM)
@@ -391,32 +389,15 @@ int radeon_bo_pin(struct radeon_bo *bo, u32 domain, u64 
*gpu_addr)
return radeon_bo_pin_restricted(bo, domain, 0, gpu_addr);
 }
 
-int radeon_bo_unpin(struct radeon_bo *bo)
+void radeon_bo_unpin(struct radeon_bo *bo)
 {
-   struct ttm_operation_ctx ctx = { false, false };
-   int r, i;
-
-   if (!bo->pin_count) {
-   dev_warn(bo->rdev->dev, "%p unpin not necessary\n", bo);
-   return 0;
-   }
-   bo->pin_count--;
-   if (bo->pin_count)
-   return 0;
-   for (i = 0; i < bo->placement.num_placement; i++) {
-   bo->placements[i].lpfn = 0;
-   bo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT;
-   }
-   r = ttm_bo_validate(>tbo, >placement, );
-   if (likely(r == 0)) {
+   ttm_bo_unpin(>tbo);
+   if (!bo->tbo.pin_count) {
if (bo->tbo.mem.mem_type == TTM_PL_VRAM)
bo->rdev->vram_pin_size -= radeon_bo_size(bo);
else
bo->rdev->gart_pin_size -= radeon_bo_size(bo);
-   } else {
-   dev_err(bo->rdev->dev, "%p validate failed for unpin\n", bo);
}
-   return r;
 }
 
 int radeon_bo_evict_vram(struct radeon_device *rdev)
@@ -549,7 +530,7 @@ int radeon_bo_list_validate(struct radeon_device *rdev,
 
list_for_each_entry(lobj, head, tv.head) {
struct radeon_bo *bo = lobj->robj;
-   if (!bo->pin_count) {
+   if 

[PATCH 08/11] drm/nouveau: switch over to the new pin interface

2020-09-21 Thread Christian König
Stop using TTM_PL_FLAG_NO_EVICT.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/nouveau/nouveau_bo.c   | 48 +++---
 drivers/gpu/drm/nouveau/nouveau_bo.h   |  3 --
 drivers/gpu/drm/nouveau/nouveau_chan.c |  2 +-
 3 files changed, 13 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 2ee75646ad6f..bcae4514952f 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -139,7 +139,7 @@ nouveau_bo_del_ttm(struct ttm_buffer_object *bo)
struct drm_device *dev = drm->dev;
struct nouveau_bo *nvbo = nouveau_bo(bo);
 
-   WARN_ON(nvbo->pin_refcnt > 0);
+   WARN_ON(nvbo->bo.pin_count > 0);
nouveau_bo_del_io_reserve_lru(bo);
nv10_bo_put_tile_region(dev, nvbo->tile, NULL);
 
@@ -417,9 +417,8 @@ nouveau_bo_placement_set(struct nouveau_bo *nvbo, uint32_t 
domain,
 {
struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev);
struct ttm_placement *pl = >placement;
-   uint32_t flags = (nvbo->force_coherent ? TTM_PL_FLAG_UNCACHED :
-TTM_PL_MASK_CACHING) |
-(nvbo->pin_refcnt ? TTM_PL_FLAG_NO_EVICT : 0);
+   uint32_t flags = nvbo->force_coherent ? TTM_PL_FLAG_UNCACHED :
+   TTM_PL_MASK_CACHING;
 
pl->placement = nvbo->placements;
set_placement_list(drm, nvbo->placements, >num_placement,
@@ -453,7 +452,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, 
bool contig)
}
}
 
-   if (nvbo->pin_refcnt) {
+   if (nvbo->bo.pin_count) {
bool error = evict;
 
switch (bo->mem.mem_type) {
@@ -472,7 +471,7 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, 
bool contig)
 bo->mem.mem_type, domain);
ret = -EBUSY;
}
-   nvbo->pin_refcnt++;
+   ttm_bo_pin(>bo);
goto out;
}
 
@@ -483,18 +482,12 @@ nouveau_bo_pin(struct nouveau_bo *nvbo, uint32_t domain, 
bool contig)
goto out;
}
 
-   nvbo->pin_refcnt++;
nouveau_bo_placement_set(nvbo, domain, 0);
-
-   /* drop pin_refcnt temporarily, so we don't trip the assertion
-* in nouveau_bo_move() that makes sure we're not trying to
-* move a pinned buffer
-*/
-   nvbo->pin_refcnt--;
ret = nouveau_bo_validate(nvbo, false, false);
if (ret)
goto out;
-   nvbo->pin_refcnt++;
+
+   ttm_bo_pin(>bo);
 
switch (bo->mem.mem_type) {
case TTM_PL_VRAM:
@@ -519,30 +512,14 @@ nouveau_bo_unpin(struct nouveau_bo *nvbo)
 {
struct nouveau_drm *drm = nouveau_bdev(nvbo->bo.bdev);
struct ttm_buffer_object *bo = >bo;
-   int ret, ref;
+   int ret;
 
ret = ttm_bo_reserve(bo, false, false, NULL);
if (ret)
return ret;
 
-   ref = --nvbo->pin_refcnt;
-   WARN_ON_ONCE(ref < 0);
-   if (ref)
-   goto out;
-
-   switch (bo->mem.mem_type) {
-   case TTM_PL_VRAM:
-   nouveau_bo_placement_set(nvbo, NOUVEAU_GEM_DOMAIN_VRAM, 0);
-   break;
-   case TTM_PL_TT:
-   nouveau_bo_placement_set(nvbo, NOUVEAU_GEM_DOMAIN_GART, 0);
-   break;
-   default:
-   break;
-   }
-
-   ret = nouveau_bo_validate(nvbo, false, false);
-   if (ret == 0) {
+   ttm_bo_unpin(>bo);
+   if (!nvbo->bo.pin_count) {
switch (bo->mem.mem_type) {
case TTM_PL_VRAM:
drm->gem.vram_available += bo->mem.size;
@@ -555,9 +532,8 @@ nouveau_bo_unpin(struct nouveau_bo *nvbo)
}
}
 
-out:
ttm_bo_unreserve(bo);
-   return ret;
+   return 0;
 }
 
 int
@@ -1065,7 +1041,7 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
if (ret)
return ret;
 
-   if (nvbo->pin_refcnt)
+   if (nvbo->bo.pin_count)
NV_WARN(drm, "Moving pinned object %p!\n", nvbo);
 
if (drm->client.device.info.family < NV_DEVICE_INFO_V0_TESLA) {
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.h 
b/drivers/gpu/drm/nouveau/nouveau_bo.h
index 2a23c8207436..ff68ded8d590 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.h
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.h
@@ -40,9 +40,6 @@ struct nouveau_bo {
 
struct nouveau_drm_tile *tile;
 
-   /* protect by the ttm reservation lock */
-   int pin_refcnt;
-
struct ttm_bo_kmap_obj dma_buf_vmap;
 };
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c 
b/drivers/gpu/drm/nouveau/nouveau_chan.c
index 8f099601d2f2..5d191e58edf1 100644
--- a/drivers/gpu/drm/nouveau/nouveau_chan.c
+++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
@@ -107,7 +107,7 @@ nouveau_channel_del(struct 

Re: [PATCH AUTOSEL 5.4 13/15] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is

2020-09-21 Thread Michel Dänzer

On 2020-09-21 4:40 p.m., Sasha Levin wrote:

From: Michel Dänzer 

[ Upstream commit 2f228aab21bbc74e90e267a721215ec8be51daf7 ]

Don't check drm_crtc_state::active for this either, per its
documentation in include/drm/drm_crtc.h:

  * Hence drivers must not consult @active in their various
  * _mode_config_funcs.atomic_check callback to reject an atomic
  * commit.

atomic_remove_fb disables the CRTC as needed for disabling the primary
plane.

This prevents at least the following problems if the primary plane gets
disabled (e.g. due to destroying the FB assigned to the primary plane,
as happens e.g. with mutter in Wayland mode):

* The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID
   (which enables the cursor plane).
* If the cursor plane was enabled, changing the legacy DPMS property
   value from off to on returned EINVAL.

v2:
* Minor changes to code comment and commit log, per review feedback.

GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108
GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165
GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344
Suggested-by: Daniel Vetter 
Acked-by: Daniel Vetter 
Reviewed-by: Nicholas Kazlauskas 
Signed-off-by: Michel Dänzer 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 


I'm a bit nervous about this getting backported so far back so quickly. 
I'd prefer waiting for 5.9 final first at least.



--
Earthling Michel Dänzer   |   https://redhat.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH AUTOSEL 5.4 12/15] drm/amd/display: update nv1x stutter latencies

2020-09-21 Thread Sasha Levin
From: Jun Lei 

[ Upstream commit c4790a8894232f39c25c7c546c06efe074e63384 ]

[why]
Recent characterization shows increased stutter latencies on some SKUs,
leading to underflow.

[how]
Update SOC params to account for this worst case latency.

Signed-off-by: Jun Lei 
Acked-by: Aurabindo Pillai 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 05b98eadc2899..46afd048c1ea3 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -340,8 +340,8 @@ struct _vcs_dpi_soc_bounding_box_st dcn2_0_nv14_soc = {
},
},
.num_states = 5,
-   .sr_exit_time_us = 8.6,
-   .sr_enter_plus_exit_time_us = 10.9,
+   .sr_exit_time_us = 11.6,
+   .sr_enter_plus_exit_time_us = 13.9,
.urgent_latency_us = 4.0,
.urgent_latency_pixel_data_only_us = 4.0,
.urgent_latency_pixel_mixed_with_vm_data_us = 4.0,
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH AUTOSEL 5.8 13/20] drm/amd/display: Don't use DRM_ERROR() for DTM add topology

2020-09-21 Thread Sasha Levin
From: Bhawanpreet Lakha 

[ Upstream commit 4cdd7b332ed139b1e37faeb82409a14490adb644 ]

[Why]
Previously we were only calling add_topology when hdcp was being enabled.
Now we call add_topology by default so the ERROR messages are printed if
the firmware is not loaded.

This error message is not relevant for normal display functionality so
no need to print a ERROR message.

[How]
Change DRM_ERROR to DRM_INFO

Signed-off-by: Bhawanpreet Lakha 
Acked-by: Aurabindo Pillai 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c 
b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
index fb1161dd7ea80..3a367a5968ae1 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.c
@@ -88,7 +88,7 @@ enum mod_hdcp_status mod_hdcp_add_display_to_topology(struct 
mod_hdcp *hdcp,
enum mod_hdcp_status status = MOD_HDCP_STATUS_SUCCESS;
 
if (!psp->dtm_context.dtm_initialized) {
-   DRM_ERROR("Failed to add display topology, DTM TA is not 
initialized.");
+   DRM_INFO("Failed to add display topology, DTM TA is not 
initialized.");
display->state = MOD_HDCP_DISPLAY_INACTIVE;
return MOD_HDCP_STATUS_FAILURE;
}
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH AUTOSEL 5.8 14/20] drm/amd/display: update nv1x stutter latencies

2020-09-21 Thread Sasha Levin
From: Jun Lei 

[ Upstream commit c4790a8894232f39c25c7c546c06efe074e63384 ]

[why]
Recent characterization shows increased stutter latencies on some SKUs,
leading to underflow.

[how]
Update SOC params to account for this worst case latency.

Signed-off-by: Jun Lei 
Acked-by: Aurabindo Pillai 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index 2d9055eb3ce92..20bdabebbc434 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -409,8 +409,8 @@ struct _vcs_dpi_soc_bounding_box_st dcn2_0_nv14_soc = {
},
},
.num_states = 5,
-   .sr_exit_time_us = 8.6,
-   .sr_enter_plus_exit_time_us = 10.9,
+   .sr_exit_time_us = 11.6,
+   .sr_enter_plus_exit_time_us = 13.9,
.urgent_latency_us = 4.0,
.urgent_latency_pixel_data_only_us = 4.0,
.urgent_latency_pixel_mixed_with_vm_data_us = 4.0,
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH AUTOSEL 4.19 7/9] drm/amdkfd: fix a memory leak issue

2020-09-21 Thread Sasha Levin
From: Dennis Li 

[ Upstream commit 087d764159996ae378b08c0fdd557537adfd6899 ]

In the resume stage of GPU recovery, start_cpsch will call pm_init
which set pm->allocated as false, cause the next pm_release_ib has
no chance to release ib memory.

Add pm_release_ib in stop_cpsch which will be called in the suspend
stage of GPU recovery.

Reviewed-by: Felix Kuehling 
Signed-off-by: Dennis Li 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index 189212cb35475..bff39f561264e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1101,6 +1101,8 @@ static int stop_cpsch(struct device_queue_manager *dqm)
unmap_queues_cpsch(dqm, KFD_UNMAP_QUEUES_FILTER_ALL_QUEUES, 0);
dqm_unlock(dqm);
 
+   pm_release_ib(>packets);
+
kfd_gtt_sa_free(dqm->dev, dqm->fence_mem);
pm_uninit(>packets);
 
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH AUTOSEL 5.8 12/20] drm/amdkfd: fix a memory leak issue

2020-09-21 Thread Sasha Levin
From: Dennis Li 

[ Upstream commit 087d764159996ae378b08c0fdd557537adfd6899 ]

In the resume stage of GPU recovery, start_cpsch will call pm_init
which set pm->allocated as false, cause the next pm_release_ib has
no chance to release ib memory.

Add pm_release_ib in stop_cpsch which will be called in the suspend
stage of GPU recovery.

Reviewed-by: Felix Kuehling 
Signed-off-by: Dennis Li 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index e9c4867abeffb..00b042a15373a 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1177,6 +1177,8 @@ static int stop_cpsch(struct device_queue_manager *dqm)
dqm->sched_running = false;
dqm_unlock(dqm);
 
+   pm_release_ib(>packets);
+
kfd_gtt_sa_free(dqm->dev, dqm->fence_mem);
pm_uninit(>packets, hanging);
 
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH AUTOSEL 5.8 16/20] drm/amd/display: Don't log hdcp module warnings in dmesg

2020-09-21 Thread Sasha Levin
From: Bhawanpreet Lakha 

[ Upstream commit 875d369d8f75275d30e59421602d9366426abff7 ]

[Why]
DTM topology updates happens by default now. This results in DTM
warnings when hdcp is not even being enabled. This spams the dmesg
and doesn't effect normal display functionality so it is better to log it
using DRM_DEBUG_KMS()

[How]
Change the DRM_WARN() to DRM_DEBUG_KMS()

Signed-off-by: Bhawanpreet Lakha 
Acked-by: Alex Deucher 
Reviewed-by: Rodrigo Siqueira 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h 
b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h
index d3192b9d0c3d8..47f8ee2832ff0 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.h
@@ -27,7 +27,7 @@
 #define MOD_HDCP_LOG_H_
 
 #ifdef CONFIG_DRM_AMD_DC_HDCP
-#define HDCP_LOG_ERR(hdcp, ...) DRM_WARN(__VA_ARGS__)
+#define HDCP_LOG_ERR(hdcp, ...) DRM_DEBUG_KMS(__VA_ARGS__)
 #define HDCP_LOG_VER(hdcp, ...) DRM_DEBUG_KMS(__VA_ARGS__)
 #define HDCP_LOG_FSM(hdcp, ...) DRM_DEBUG_KMS(__VA_ARGS__)
 #define HDCP_LOG_TOP(hdcp, ...) pr_debug("[HDCP_TOP]:"__VA_ARGS__)
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH AUTOSEL 5.8 15/20] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is

2020-09-21 Thread Sasha Levin
From: Michel Dänzer 

[ Upstream commit 2f228aab21bbc74e90e267a721215ec8be51daf7 ]

Don't check drm_crtc_state::active for this either, per its
documentation in include/drm/drm_crtc.h:

 * Hence drivers must not consult @active in their various
 * _mode_config_funcs.atomic_check callback to reject an atomic
 * commit.

atomic_remove_fb disables the CRTC as needed for disabling the primary
plane.

This prevents at least the following problems if the primary plane gets
disabled (e.g. due to destroying the FB assigned to the primary plane,
as happens e.g. with mutter in Wayland mode):

* The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID
  (which enables the cursor plane).
* If the cursor plane was enabled, changing the legacy DPMS property
  value from off to on returned EINVAL.

v2:
* Minor changes to code comment and commit log, per review feedback.

GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108
GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165
GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344
Suggested-by: Daniel Vetter 
Acked-by: Daniel Vetter 
Reviewed-by: Nicholas Kazlauskas 
Signed-off-by: Michel Dänzer 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++-
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 3f7eced92c0c8..7c1cc0ba30a55 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -5257,19 +5257,6 @@ static void dm_crtc_helper_disable(struct drm_crtc *crtc)
 {
 }
 
-static bool does_crtc_have_active_cursor(struct drm_crtc_state *new_crtc_state)
-{
-   struct drm_device *dev = new_crtc_state->crtc->dev;
-   struct drm_plane *plane;
-
-   drm_for_each_plane_mask(plane, dev, new_crtc_state->plane_mask) {
-   if (plane->type == DRM_PLANE_TYPE_CURSOR)
-   return true;
-   }
-
-   return false;
-}
-
 static int count_crtc_active_planes(struct drm_crtc_state *new_crtc_state)
 {
struct drm_atomic_state *state = new_crtc_state->state;
@@ -5349,19 +5336,20 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc 
*crtc,
return ret;
}
 
-   /* In some use cases, like reset, no stream is attached */
-   if (!dm_crtc_state->stream)
-   return 0;
-
/*
-* We want at least one hardware plane enabled to use
-* the stream with a cursor enabled.
+* We require the primary plane to be enabled whenever the CRTC is, 
otherwise
+* drm_mode_cursor_universal may end up trying to enable the cursor 
plane while all other
+* planes are disabled, which is not supported by the hardware. And 
there is legacy
+* userspace which stops using the HW cursor altogether in response to 
the resulting EINVAL.
 */
-   if (state->enable && state->active &&
-   does_crtc_have_active_cursor(state) &&
-   dm_crtc_state->active_planes == 0)
+   if (state->enable &&
+   !(state->plane_mask & drm_plane_mask(crtc->primary)))
return -EINVAL;
 
+   /* In some use cases, like reset, no stream is attached */
+   if (!dm_crtc_state->stream)
+   return 0;
+
if (dc_validate_stream(dc, dm_crtc_state->stream) == DC_OK)
return 0;
 
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH AUTOSEL 5.4 11/15] drm/amdkfd: fix a memory leak issue

2020-09-21 Thread Sasha Levin
From: Dennis Li 

[ Upstream commit 087d764159996ae378b08c0fdd557537adfd6899 ]

In the resume stage of GPU recovery, start_cpsch will call pm_init
which set pm->allocated as false, cause the next pm_release_ib has
no chance to release ib memory.

Add pm_release_ib in stop_cpsch which will be called in the suspend
stage of GPU recovery.

Reviewed-by: Felix Kuehling 
Signed-off-by: Dennis Li 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
index a2ed9c257cb0d..e9a2784400792 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
@@ -1075,6 +1075,8 @@ static int stop_cpsch(struct device_queue_manager *dqm)
unmap_queues_cpsch(dqm, KFD_UNMAP_QUEUES_FILTER_ALL_QUEUES, 0);
dqm_unlock(dqm);
 
+   pm_release_ib(>packets);
+
kfd_gtt_sa_free(dqm->dev, dqm->fence_mem);
pm_uninit(>packets);
 
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH AUTOSEL 5.4 13/15] drm/amdgpu/dc: Require primary plane to be enabled whenever the CRTC is

2020-09-21 Thread Sasha Levin
From: Michel Dänzer 

[ Upstream commit 2f228aab21bbc74e90e267a721215ec8be51daf7 ]

Don't check drm_crtc_state::active for this either, per its
documentation in include/drm/drm_crtc.h:

 * Hence drivers must not consult @active in their various
 * _mode_config_funcs.atomic_check callback to reject an atomic
 * commit.

atomic_remove_fb disables the CRTC as needed for disabling the primary
plane.

This prevents at least the following problems if the primary plane gets
disabled (e.g. due to destroying the FB assigned to the primary plane,
as happens e.g. with mutter in Wayland mode):

* The legacy cursor ioctl returned EINVAL for a non-0 cursor FB ID
  (which enables the cursor plane).
* If the cursor plane was enabled, changing the legacy DPMS property
  value from off to on returned EINVAL.

v2:
* Minor changes to code comment and commit log, per review feedback.

GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1108
GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1165
GitLab: https://gitlab.gnome.org/GNOME/mutter/-/issues/1344
Suggested-by: Daniel Vetter 
Acked-by: Daniel Vetter 
Reviewed-by: Nicholas Kazlauskas 
Signed-off-by: Michel Dänzer 
Signed-off-by: Alex Deucher 
Signed-off-by: Sasha Levin 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 32 ++-
 1 file changed, 10 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 60e50181f6d39..2384aa018993d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4299,19 +4299,6 @@ static void dm_crtc_helper_disable(struct drm_crtc *crtc)
 {
 }
 
-static bool does_crtc_have_active_cursor(struct drm_crtc_state *new_crtc_state)
-{
-   struct drm_device *dev = new_crtc_state->crtc->dev;
-   struct drm_plane *plane;
-
-   drm_for_each_plane_mask(plane, dev, new_crtc_state->plane_mask) {
-   if (plane->type == DRM_PLANE_TYPE_CURSOR)
-   return true;
-   }
-
-   return false;
-}
-
 static int count_crtc_active_planes(struct drm_crtc_state *new_crtc_state)
 {
struct drm_atomic_state *state = new_crtc_state->state;
@@ -4391,19 +4378,20 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc 
*crtc,
return ret;
}
 
-   /* In some use cases, like reset, no stream is attached */
-   if (!dm_crtc_state->stream)
-   return 0;
-
/*
-* We want at least one hardware plane enabled to use
-* the stream with a cursor enabled.
+* We require the primary plane to be enabled whenever the CRTC is, 
otherwise
+* drm_mode_cursor_universal may end up trying to enable the cursor 
plane while all other
+* planes are disabled, which is not supported by the hardware. And 
there is legacy
+* userspace which stops using the HW cursor altogether in response to 
the resulting EINVAL.
 */
-   if (state->enable && state->active &&
-   does_crtc_have_active_cursor(state) &&
-   dm_crtc_state->active_planes == 0)
+   if (state->enable &&
+   !(state->plane_mask & drm_plane_mask(crtc->primary)))
return -EINVAL;
 
+   /* In some use cases, like reset, no stream is attached */
+   if (!dm_crtc_state->stream)
+   return 0;
+
if (dc_validate_stream(dc, dm_crtc_state->stream) == DC_OK)
return 0;
 
-- 
2.25.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH] drm/vram-helper: Fix use of top-down placement

2020-09-21 Thread Christian König

Am 21.09.20 um 16:25 schrieb Thomas Zimmermann:

Commit 7053e0eab473 ("drm/vram-helper: stop using TTM placement flags")
cleared the BO placement flags if top-down placement had been selected.
Hence, BOs that were supposed to go into VRAM are now placed in a default
location in system memory.

Trying to scanout the incorrectly pinned BO results in displayed garbage
and an error message.

[  146.108127] [ cut here ]
[  146.1V08180] WARNING: CPU: 0 PID: 152 at 
drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60 
[drm_vram_helper]
...
[  146.108591]  ast_cursor_page_flip+0x3e/0x150 [ast]
[  146.108622]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
[  146.108654]  drm_atomic_helper_commit_planes+0x197/0x4c0
[  146.108699]  drm_atomic_helper_commit_tail_rpm+0x59/0xa0
[  146.108718]  commit_tail+0x103/0x1c0
...
[  146.109302] ---[ end trace d901a1ba1d949036 ]---

Fix the bug by keeping the placement flags. The top-down placement flag
is stored in a separate variable.

Signed-off-by: Thomas Zimmermann 
Fixes: 7053e0eab473 ("drm/vram-helper: stop using TTM placement flags")
Cc: Christian König 
Cc: Dave Airlie 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org


Reviewed-by: Christian König 

It's always the stupid typos, thanks for narrowing this down.

And please keep an eye open I have quite a number of more patches in the 
pipeline.


Thanks,
Christian.


---
  drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 50cad0e4a92e..375c79e23ca5 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -140,7 +140,7 @@ static void drm_gem_vram_placement(struct 
drm_gem_vram_object *gbo,
unsigned int c = 0;
  
  	if (pl_flag & DRM_GEM_VRAM_PL_FLAG_TOPDOWN)

-   pl_flag = TTM_PL_FLAG_TOPDOWN;
+   invariant_flags = TTM_PL_FLAG_TOPDOWN;
  
  	gbo->placement.placement = gbo->placements;

gbo->placement.busy_placement = gbo->placements;


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 0/3] drm: commit_work scheduling

2020-09-21 Thread Tejun Heo
Hello,

On Mon, Sep 21, 2020 at 11:21:54AM +0200, Daniel Vetter wrote:
> The part I don't like about this is that it all feels rather hacked
> together, and if we add more stuff (or there's some different thing in the
> system that also needs rt scheduling) then it doesn't compose.
> 
> So question to rt/worker folks: What's the best way to let userspace set
> the scheduling mode and priorities of things the kernel does on its
> behalf? Surely we're not the first ones where if userspace runs with some
> rt priority it'll starve out the kernel workers that it needs. Hardcoding
> something behind a subsystem ioctl (which just means every time userspace
> changes what it does, we need a new such flag or mode) can't be the right
> thing.

Maybe not first but there haven't been many. The main benefit of workqueue
is that the users get to pool the worker threads automatically. I don't
think the existing workqueue design is something suitable for actual RT use
cases. Furthermore, there are inherent conflicts between sharing resources
and RT as this this patchset is already showing w/ needing per-crtc worker
thread. Maybe we can further abstract it if there are more use cases but for
now kthread_worker based implementation sounds about right to me.

Thanks.

-- 
tejun
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Mesa-dev] XDC 2020 feedback and comments

2020-09-21 Thread Jason Ekstrand
First off, I think you all did a fantastic job.  I felt that things
ran very smoothly and, as far as the talks themselves go, I think it
went almost as smoothly as an in-person XDC.  I'm really quite
impressed.  I do have a couple pieces of more nuanced feedback:

 1. I think we were maybe a bit too scared of overloading jitsi.
Having more people in the instance for questions might have made that
portion go better.  As it was, there was only one or two talks that
had any live questions.  That said, there are a few advantages to
having things funneled through IRC, the most obvious of which being
that people can ask their question mid-talk and have it handled at the
end instead of having to remember it for 20 minutes.

 2. I really miss the hallway track.  On Thursday, after the
conference, Bas, Connor, and I used jitsi to have a chat about
ray-tracing.  That was really fun and I wish I'd done something like
that every day of XDC.  Maybe it's my own fault for not setting up
said chats but I think it could have been made more accessible (I had
no idea how to fork off a jitsi instance) and/or encouraged somehow.

--Jason

On Mon, Sep 21, 2020 at 3:07 AM Samuel Iglesias Gonsálvez
 wrote:
>
> Hi all,
>
> Huge thanks again to the entire team from Intel, for their great work
> organizing XDC 2020, our first virtual conference!
>
> As usual we're looking for feedback on both XDC itself, and the CFP
> process and program selection. Both about what was great and should be
> kept for next year's edition, and where there's room for improvement.
>
> The board does keep some notes, for those interested in what we have
> already:
>
> - XDC notes for prospective organizers:
> https://www.x.org/wiki/Events/RFP/
>
> - CFP notes: https://www.x.org/wiki/Events/PapersCommittee/
>
> If you want to send in your comments in private, please send them to
> the X.org Foundation board: bo...@foundation.x.org
>
> Cheers,
>
> Sam
> ___
> mesa-dev mailing list
> mesa-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 6/8] drm/vram-helper: don't use ttm bo->offset v4

2020-09-21 Thread Thomas Zimmermann
Hi

Am 17.09.20 um 14:32 schrieb Christian König:
> Am 17.09.20 um 14:29 schrieb Thomas Zimmermann:
>> Hi Christian
>>
>> Am 17.09.20 um 13:12 schrieb Christian König:
>>> Hi Thomas,
>>>
>>> Am 17.09.20 um 12:51 schrieb Thomas Zimmermann:
 Hi

 Am 24.06.20 um 20:26 schrieb Nirmoy Das:
> Calculate GEM VRAM bo's offset within vram-helper without depending on
> bo->offset.
>
> Signed-off-by: Nirmoy Das 
> Reviewed-by: Daniel Vetter 
> ---
>    drivers/gpu/drm/drm_gem_vram_helper.c | 11 ++-
>    1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 0023ce1d2cf7..ad096600d89f 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -281,6 +281,15 @@ u64 drm_gem_vram_mmap_offset(struct
> drm_gem_vram_object *gbo)
>    }
>    EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
>    +static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
> +{
> +    /* Keep TTM behavior for now, remove when drivers are audited */
> +    if (WARN_ON_ONCE(!gbo->bo.mem.mm_node))
 At this line I got
>>> Sounds like ast forgot to pin the cursor to VRAM.
>>>
>>> If you look at ast_cursor_page_flip(), you see:
  off = drm_gem_vram_offset(gbo);
  if (drm_WARN_ON_ONCE(dev, off < 0))
  return; /* Bug: we didn't pin the cursor HW BO to
 VRAM. */
>>> The drm_WARN_ON_ONCE() just never triggered before because it checks for
>>> the wrong condition (off < 0).
>> GEM VRAM BOs have a pin counter. drm_gem_vram_offset() returns -ENODEV
>> if the BO's pin count is 0 (i.e., the BO has not been pinned anywhere).
>> That's what we're testing here. Two cursor BOs are permanently pinned to
>> the top of VRAM memory by ast_cursor_init(). If perma-pinning in
>> ast_cursor_init() fails, driver initialization should fail entirely.
>>
>> These cursor BOs do some sort of double buffering, On successful
>> initialization, the actual cursor image is later blit-ed into one of the
>> BOs.
>>
>> All this used to work from what I can tell. Is there any chance that the
>> recent TTM refactoring broke this?
> 
> Yeah, certainly possible. If you have time please bisect.

Found it.

https://patchwork.freedesktop.org/patch/391418/

Best regards
Thomas

> 
> Thanks,
> Christian.
> 
>>
>> Best regards
>> Thomas
>>
>>> Regards,
>>> Christian.
>>>
 [  146.133821] [ cut here ]
 [  146.133872] WARNING: CPU: 6 PID: 7 at
 drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60
 [drm_vram_helper]
 [  146.133880] Modules linked in: fuse(E) af_packet(E)
 ebtable_filter(E)
 ebtables(E) ip6table_filter(E) ip6_tables(E) iptable_filter(E)
 ip_tables(E) x_tables(E) bpfilter(E) rfkill(E) dmi_sysfs(E) msr(E)
 intel_powerclamp(E) coretemp(E) kv)
 [  146.134051]  scsi_dh_emc(E) scsi_dh_alua(E)
 [  146.134074] CPU: 6 PID: 7 Comm: kworker/u48:0 Tainted:
 G    E
   5.9.0-rc4-1-default+ #492
 [  146.134083] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN
 FIRE X2270 M2, BIOS 2.05    07/01/2010
 [  146.134099] Workqueue: events_unbound commit_work
 [  146.134116] RIP: 0010:drm_gem_vram_offset+0x59/0x60
 [drm_vram_helper]
 [  146.134128] Code: 02 00 00 00 74 24 48 8d bb 80 02 00 00 e8 ef 27 17
 d7 48 8b 83 80 02 00 00 5b 48 c1 e0 0c c3 0f 0b 48 c7 c0 ed ff ff ff 5b
 c3 <0f> 0b 31 c0 5b c3 90 66 66 66 66 90 41 56 41 55 49 89 d5 41 54 49
 [  146.134137] RSP: 0018:c9107c38 EFLAGS: 00010246
 [  146.134149] RAX:  RBX: 88855000 RCX:
 c032323b
 [  146.134158] RDX: dc00 RSI: 88810e236300 RDI:
 88855278
 [  146.134168] RBP: 88810909 R08: c0323225 R09:
 0002
 [  146.134177] R10: ed1020675020 R11: 0001 R12:
 888109091050
 [  146.134187] R13: 88810e236300 R14: 88810909 R15:
 
 [  146.134197] FS:  () GS:88811600()
 knlGS:
 [  146.134206] CS:  0010 DS:  ES:  CR0: 80050033
 [  146.134215] CR2: 7f60547d9100 CR3: 29216002 CR4:
 000206e0
 [  146.134223] Call Trace:
 [  146.134245]  ast_cursor_page_flip+0x3e/0x150 [ast]
 [  146.134272]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
 [  146.134300]  drm_atomic_helper_commit_planes+0x197/0x4c0
 [  146.134341]  drm_atomic_helper_commit_tail_rpm+0x51/0x90
 [  146.134357]  commit_tail+0x103/0x1c0
 [  146.134390]  process_one_work+0x56a/0xa60
 [  146.134431]  ? __lock_acquired+0x1ca/0x3d0
 [  146.134447]  ? pwq_dec_nr_in_flight+0x110/0x110
 [  146.134460]  ? __lock_contended+0x4a0/0x4a0
 [  

[PATCH] drm/vram-helper: Fix use of top-down placement

2020-09-21 Thread Thomas Zimmermann
Commit 7053e0eab473 ("drm/vram-helper: stop using TTM placement flags")
cleared the BO placement flags if top-down placement had been selected.
Hence, BOs that were supposed to go into VRAM are now placed in a default
location in system memory.

Trying to scanout the incorrectly pinned BO results in displayed garbage
and an error message.

[  146.108127] [ cut here ]
[  146.1V08180] WARNING: CPU: 0 PID: 152 at 
drivers/gpu/drm/drm_gem_vram_helper.c:284 drm_gem_vram_offset+0x59/0x60 
[drm_vram_helper]
...
[  146.108591]  ast_cursor_page_flip+0x3e/0x150 [ast]
[  146.108622]  ast_cursor_plane_helper_atomic_update+0x8a/0xc0 [ast]
[  146.108654]  drm_atomic_helper_commit_planes+0x197/0x4c0
[  146.108699]  drm_atomic_helper_commit_tail_rpm+0x59/0xa0
[  146.108718]  commit_tail+0x103/0x1c0
...
[  146.109302] ---[ end trace d901a1ba1d949036 ]---

Fix the bug by keeping the placement flags. The top-down placement flag
is stored in a separate variable.

Signed-off-by: Thomas Zimmermann 
Fixes: 7053e0eab473 ("drm/vram-helper: stop using TTM placement flags")
Cc: Christian König 
Cc: Dave Airlie 
Cc: Maarten Lankhorst 
Cc: Maxime Ripard 
Cc: Thomas Zimmermann 
Cc: David Airlie 
Cc: Daniel Vetter 
Cc: dri-devel@lists.freedesktop.org
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 50cad0e4a92e..375c79e23ca5 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -140,7 +140,7 @@ static void drm_gem_vram_placement(struct 
drm_gem_vram_object *gbo,
unsigned int c = 0;
 
if (pl_flag & DRM_GEM_VRAM_PL_FLAG_TOPDOWN)
-   pl_flag = TTM_PL_FLAG_TOPDOWN;
+   invariant_flags = TTM_PL_FLAG_TOPDOWN;
 
gbo->placement.placement = gbo->placements;
gbo->placement.busy_placement = gbo->placements;
-- 
2.28.0

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next] drm/panfrost: simplify the return expression of cz_ih_hw_init()

2020-09-21 Thread Philipp Zabel
On Mon, 2020-09-21 at 21:10 +0800, Qinglang Miao wrote:
> Simplify the return expression.
> 
> Signed-off-by: Qinglang Miao 
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 8 +---
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c 
> b/drivers/gpu/drm/panfrost/panfrost_device.c
> index e68967338..ea8d31863 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -18,19 +18,13 @@
>  
>  static int panfrost_reset_init(struct panfrost_device *pfdev)
>  {
> - int err;
> -
>   pfdev->rstc = devm_reset_control_array_get(pfdev->dev, false, true);
>   if (IS_ERR(pfdev->rstc)) {
>   dev_err(pfdev->dev, "get reset failed %ld\n", 
> PTR_ERR(pfdev->rstc));
>   return PTR_ERR(pfdev->rstc);
>   }
>  
> - err = reset_control_deassert(pfdev->rstc);
> - if (err)
> - return err;
> -
> - return 0;
> + return reset_control_deassert(pfdev->rstc);
>  }
>  
>  static void panfrost_reset_fini(struct panfrost_device *pfdev)

Reviewed-by: Philipp Zabel 

regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [Re-send][PATCH] gpu/ipu-v3:reduce protected code area in ipu idmac get/put

2020-09-21 Thread Philipp Zabel
Hi Bernard,

On Mon, 2020-09-21 at 19:11 +0800, Bernard wrote:
> This change will speed-up a bit these ipu_idmac_get &
> ipu_idmac_put processing and there is no need to protect
> kzalloc & kfree.

I don't think that will be measurable, the channel lock is very unlikely
to be contended. It might make more sense to replace the list walk with
a bitfield.

> Signed-off-by: Bernard Zhao 
> ---
>  drivers/gpu/ipu-v3/ipu-common.c | 24 +---
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c
> index b3dae9ec1a38..8b3a57864c2e 100644
> --- a/drivers/gpu/ipu-v3/ipu-common.c
> +++ b/drivers/gpu/ipu-v3/ipu-common.c
> @@ -267,29 +267,30 @@ EXPORT_SYMBOL_GPL(ipu_rot_mode_to_degrees);
>  struct ipuv3_channel *ipu_idmac_get(struct ipu_soc *ipu, unsigned num)
>  {
>   struct ipuv3_channel *channel;
> + struct ipuv3_channel *entry;
>  
>   dev_dbg(ipu->dev, "%s %d\n", __func__, num);
>  
>   if (num > 63)
>   return ERR_PTR(-ENODEV);
>  
> + channel = kzalloc(sizeof(*channel), GFP_KERNEL);
> + if (!channel)
> + return ERR_PTR(-ENOMEM);
> +
> + channel->num = num;
> + channel->ipu = ipu;
> +
>   mutex_lock(>channel_lock);
>  
> - list_for_each_entry(channel, >channels, list) {
> - if (channel->num == num) {
> + list_for_each_entry(entry, >channels, list) {
> + if (entry->num == num) {
> + kfree(channel);
>   channel = ERR_PTR(-EBUSY);
>   goto out;

This leaks the channel memory allocated above.

>   }
>   }
> 
> - channel = kzalloc(sizeof(*channel), GFP_KERNEL);
> - if (!channel) {
> - channel = ERR_PTR(-ENOMEM);
> - goto out;
> - }
> -
> - channel->num = num;
> - channel->ipu = ipu;
>   list_add(>list, >channels);
>  
>  out:
> @@ -308,9 +309,10 @@ void ipu_idmac_put(struct ipuv3_channel *channel)
>   mutex_lock(>channel_lock);
>  
>   list_del(>list);
> - kfree(channel);
>  
>   mutex_unlock(>channel_lock);
> +
> + kfree(channel);
>  }
>  EXPORT_SYMBOL_GPL(ipu_idmac_put);

regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()

2020-09-21 Thread Thierry Reding
On Mon, Sep 21, 2020 at 04:12:20PM +0300, Mikko Perttunen wrote:
> On 9/21/20 4:10 PM, Qinglang Miao wrote:
> > Simplify the return expression.
> > 
> > Signed-off-by: Qinglang Miao 
> > ---
> >   drivers/gpu/host1x/cdma.c | 8 +---
> >   1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
> > index e8d3fda91..08a0f9e10 100644
> > --- a/drivers/gpu/host1x/cdma.c
> > +++ b/drivers/gpu/host1x/cdma.c
> > @@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma 
> > *cdma,
> >*/
> >   int host1x_cdma_init(struct host1x_cdma *cdma)
> >   {
> > -   int err;
> > -
> > mutex_init(>lock);
> > init_completion(>complete);
> > @@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma)
> > cdma->running = false;
> > cdma->torndown = false;
> > -   err = host1x_pushbuffer_init(>push_buffer);
> > -   if (err)
> > -   return err;
> > -
> > -   return 0;
> > +   return host1x_pushbuffer_init(>push_buffer);
> 
> IMHO, this makes it less readable since now it kind of looks like
> host1x_pushbuffer_init is returning some meaningful value, instead of the
> code just handling error values in a sequence.

Agreed. Collapsing the error handling like this also makes adding code
to the bottom of this function more complicated than necessary.

Thierry


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH -next] gpu: host1x: simplify the return expression of host1x_cdma_init()

2020-09-21 Thread Mikko Perttunen

On 9/21/20 4:10 PM, Qinglang Miao wrote:

Simplify the return expression.

Signed-off-by: Qinglang Miao 
---
  drivers/gpu/host1x/cdma.c | 8 +---
  1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
index e8d3fda91..08a0f9e10 100644
--- a/drivers/gpu/host1x/cdma.c
+++ b/drivers/gpu/host1x/cdma.c
@@ -448,8 +448,6 @@ void host1x_cdma_update_sync_queue(struct host1x_cdma *cdma,
   */
  int host1x_cdma_init(struct host1x_cdma *cdma)
  {
-   int err;
-
mutex_init(>lock);
init_completion(>complete);
  
@@ -459,11 +457,7 @@ int host1x_cdma_init(struct host1x_cdma *cdma)

cdma->running = false;
cdma->torndown = false;
  
-	err = host1x_pushbuffer_init(>push_buffer);

-   if (err)
-   return err;
-
-   return 0;
+   return host1x_pushbuffer_init(>push_buffer);


IMHO, this makes it less readable since now it kind of looks like 
host1x_pushbuffer_init is returning some meaningful value, instead of 
the code just handling error values in a sequence.


Mikko


  }
  
  /*



___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH 03/13] dt-bindings: arm: fsl: Fix matching Purism Librem5 phones

2020-09-21 Thread Shawn Guo
On Fri, Sep 04, 2020 at 04:53:02PM +0200, Krzysztof Kozlowski wrote:
> All Purism Librem5 phones have three compatibles so they need their own
> entry to fix dbts_check warnings like:
> 
>   arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml: /:
> compatible: ['purism,librem5r2', 'purism,librem5', 'fsl,imx8mq'] is not 
> valid under any of the given schemas
> 
>   arch/arm64/boot/dts/freescale/imx8mq-librem5-r2.dt.yaml: /:
> compatible: ['purism,librem5r2', 'purism,librem5', 'fsl,imx8mq'] is too 
> long
> 
> Signed-off-by: Krzysztof Kozlowski 

Applied, thanks.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


Re: [PATCH v2] Revert "drm/radeon: handle PCIe root ports with addressing limitations"

2020-09-21 Thread Sasha Levin
Hi

[This is an automated email]

This commit has been processed because it contains a "Fixes:" tag
fixing commit: 33b3ad3788ab ("drm/radeon: handle PCIe root ports with 
addressing limitations").

The bot has tested the following trees: v5.8.10, v5.4.66.

v5.8.10: Build OK!
v5.4.66: Failed to apply! Possible dependencies:
8b53e1cb2728 ("drm/radeon: switch to gem vma offset manager")
9d6f4484e81c ("drm/ttm: turn ttm_bo_device.vma_manager into a pointer")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[GIT PULL] drm/tegra: Changes for v5.10-rc1

2020-09-21 Thread Thierry Reding
Hi Dave,

The following changes since commit 9123e3a74ec7b934a4a099e98af6a61c2f80bbf5:

  Linux 5.9-rc1 (2020-08-16 13:04:57 -0700)

are available in the Git repository at:

  ssh://git.freedesktop.org/git/tegra/linux.git tags/drm/tegra/for-5.10-rc1

for you to fetch changes up to d9f980ebcd01d90a2a76ea2383c925abf9223cab:

  drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge 
(2020-09-17 12:31:18 +0200)

Thanks,
Thierry


drm/tegra: Changes for v5.10-rc1

This is a handful of patches that add bridge support for Tegra devices
and fix a couple of minor issues.


Dmitry Osipenko (4):
  drm/tegra: output: Don't leak OF node on error
  drm/tegra: output: Support DRM bridges
  drm/tegra: output: rgb: Support LVDS encoder bridge
  drm/tegra: output: rgb: Wrap directly-connected panel into DRM bridge

Thierry Reding (1):
  drm/tegra: Properly reference count the DDC I2C adapter

 drivers/gpu/drm/tegra/drm.h|   2 +
 drivers/gpu/drm/tegra/output.c |  24 +++---
 drivers/gpu/drm/tegra/rgb.c| 102 ++---
 drivers/gpu/drm/tegra/sor.c|   7 ++-
 4 files changed, 80 insertions(+), 55 deletions(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


  1   2   >