date:20200722

On Fri, May 29, 2020 at 03:33:17PM +0300, Andy Shevchenko wrote:
> acpi_dev_get_resources() does perform the NULL pointer check against
> ACPI companion device which is given as function parameter. Thus,
> there is no need to duplicate this check in the caller.

Any comment so far?

> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 24 
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c 
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index 574dcfec9577..6f9e08cda964 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -426,23 +426,19 @@ static void i2c_acpi_find_adapter(struct intel_dsi 
> *intel_dsi,
>  {
>   struct drm_device *drm_dev = intel_dsi->base.base.dev;
>   struct device *dev = &drm_dev->pdev->dev;
> - struct acpi_device *acpi_dev;
> + struct acpi_device *acpi_dev = ACPI_COMPANION(dev);
>   struct list_head resource_list;
>   struct i2c_adapter_lookup lookup;
>  
> - acpi_dev = ACPI_COMPANION(dev);
> - if (acpi_dev) {
> - memset(&lookup, 0, sizeof(lookup));
> - lookup.slave_addr = slave_addr;
> - lookup.intel_dsi = intel_dsi;
> - lookup.dev_handle = acpi_device_handle(acpi_dev);
> -
> - INIT_LIST_HEAD(&resource_list);
> - acpi_dev_get_resources(acpi_dev, &resource_list,
> -i2c_adapter_lookup,
> -&lookup);
> - acpi_dev_free_resource_list(&resource_list);
> - }
> + memset(&lookup, 0, sizeof(lookup));
> + lookup.slave_addr = slave_addr;
> + lookup.intel_dsi = intel_dsi;
> + lookup.dev_handle = acpi_device_handle(acpi_dev);
> +
> + INIT_LIST_HEAD(&resource_list);
> + acpi_dev_get_resources(acpi_dev, &resource_list,
> +i2c_adapter_lookup, &lookup);
> + acpi_dev_free_resource_list(&resource_list);
>  }
>  #else
>  static inline void i2c_acpi_find_adapter(struct intel_dsi *intel_dsi,
> -- 
> 2.26.2
> 

-- 
With Best Regards,
Andy Shevchenko


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

[PATCH -next] gpu: drm: Fix spinlock vblank_time_lock use error.

2020-07-22 Thread Xu Qiang

The drm_handle_vblank function is in the interrupt context.
Therefore, the spin lock vblank_time_lock is obtained
from the interrupt context.

Cc: 
Signed-off-by: Xu Qiang 
---
 drivers/gpu/drm/drm_vblank.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
index f402c75b9d34..4ca63ff33a43 100644
--- a/drivers/gpu/drm/drm_vblank.c
+++ b/drivers/gpu/drm/drm_vblank.c
@@ -229,10 +229,11 @@ static void drm_reset_vblank_timestamp(struct drm_device 
*dev, unsigned int pipe
 {
u32 cur_vblank;
bool rc;
+   unsigned long irqflags;
ktime_t t_vblank;
int count = DRM_TIMESTAMP_MAXRETRIES;
 
-   spin_lock(&dev->vblank_time_lock);
+   spin_lock_irqsave(&dev->vblank_time_lock, irqflags);
 
/*
 * sample the current counter to avoid random jumps
@@ -257,7 +258,7 @@ static void drm_reset_vblank_timestamp(struct drm_device 
*dev, unsigned int pipe
 */
store_vblank(dev, pipe, 1, t_vblank, cur_vblank);
 
-   spin_unlock(&dev->vblank_time_lock);
+   spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
 }
 
 /*
@@ -1106,11 +1107,12 @@ static int __enable_vblank(struct drm_device *dev, 
unsigned int pipe)
 static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe)
 {
struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
+   unsigned long irqflags;
int ret = 0;
 
assert_spin_locked(&dev->vbl_lock);
 
-   spin_lock(&dev->vblank_time_lock);
+   spin_lock_irqsave(&dev->vblank_time_lock, irqflags);
 
if (!vblank->enabled) {
/*
@@ -1136,7 +1138,7 @@ static int drm_vblank_enable(struct drm_device *dev, 
unsigned int pipe)
}
}
 
-   spin_unlock(&dev->vblank_time_lock);
+   spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
 
return ret;
 }
@@ -1917,6 +1919,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned 
int pipe)
 {
struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
unsigned long irqflags;
+   unsigned long irqflags_vblank;
bool disable_irq;
 
if (drm_WARN_ON_ONCE(dev, !drm_dev_has_vblank(dev)))
@@ -1931,18 +1934,18 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned 
int pipe)
 * vblank enable/disable, as this would cause inconsistent
 * or corrupted timestamps and vblank counts.
 */
-   spin_lock(&dev->vblank_time_lock);
+   spin_lock_irqsave(&dev->vblank_time_lock, irqflags_vblank);
 
/* Vblank irq handling disabled. Nothing to do. */
if (!vblank->enabled) {
-   spin_unlock(&dev->vblank_time_lock);
+   spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags_vblank);
spin_unlock_irqrestore(&dev->event_lock, irqflags);
return false;
}
 
drm_update_vblank_count(dev, pipe, true);
 
-   spin_unlock(&dev->vblank_time_lock);
+   spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags_vblank);
 
wake_up(&vblank->queue);
 
-- 
2.25.0

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

Re: nouveau regression with 5.7 caused by "PCI/PM: Assume ports without DLL Link Active train links in 100 ms"

2020-07-22 Thread Mika Westerberg

On Tue, Jul 21, 2020 at 11:01:55AM -0400, Lyude Paul wrote:
> Sure thing. Also, feel free to let me know if you'd like access to one of the
> systems we saw breaking with this patch - I'm fairly sure I've got one of them
> locally at my apartment and don't mind setting up AMT/KVM/SSH

Probably no need for remote access (thanks for the offer, though). I
attached a test patch to the bug report:

  https://bugzilla.kernel.org/show_bug.cgi?id=208597

that tries to work it around (based on the ->pm_cap == 0). I wonder if
anyone would have time to try it out.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: io-mapping: Indicate mapping failure

On Tue, Jul 21, 2020 at 11:34:25AM -0400, Michael J. Ruhl wrote:
> I found this when my system crashed long after the mapping failure.
> The expected behavior should have been driver exit.
> 
> Since this is almost exclusively used for drm, I am posting to
> the dri mailing list.  Should this go to another list as well?

Just drop this cover letter. For single patch is not needed. Use place
immediately after cutter '---' line for comments like above.

-- 
With Best Regards,
Andy Shevchenko


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

[PATCH] drm/virtio: fix memory leak in virtio_gpu_cleanup_object()

2020-07-22 Thread Xin He

Before setting shmem->pages to NULL, kfree() should
be called.

Signed-off-by: Xin He 
Reviewed-by: Qi Liu 
---
 drivers/gpu/drm/virtio/virtgpu_object.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c 
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 6ccbd01cd888..703b5cd51751 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -79,6 +79,7 @@ void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo)
}
 
sg_free_table(shmem->pages);
+   kfree(shmem->pages);
shmem->pages = NULL;
drm_gem_shmem_unpin(&bo->base.base);
}
-- 
2.21.1 (Apple Git-122.3)

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

Re: [PATCH v5 0/6] Add support for GPU DDR BW scaling

2020-07-22 Thread Viresh Kumar

On 21-07-20, 07:28, Rob Clark wrote:
> With your ack, I can add the patch the dev_pm_opp_set_bw patch to my
> tree and merge it via msm-next -> drm-next -> linus

I wanted to send it via my tree, but its okay. Pick this patch from
linux-next and add my Ack, I will drop it after that.

a8351c12c6c7 OPP: Add and export helper to set bandwidth

> Otherwise I can send a second later pull req that adds the final patch
> after has rebased to 5.9-rc1 (by which point the opp next tree will
> have presumably been merged

The PM stuff gets pushed fairly early and so I was asking you to
rebase just on my tree, so you could have sent the pull request right
after the PM tree landed there instead of waiting for rc1.

But its fine now.

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

Re: [PATCH] drm/msm/dp: Add DP compliance tests on Snapdragon Chipsets

2020-07-22 Thread khsieh


On 2020-07-20 19:57, Rob Clark wrote:
On Mon, Jul 20, 2020 at 4:32 PM Stephen Boyd  
wrote:


Quoting khs...@codeaurora.org (2020-07-20 15:48:13)
> On 2020-07-20 13:18, Stephen Boyd wrote:
> > Quoting Kuogee Hsieh (2020-07-07 11:41:25)
> >>  drivers/gpu/drm/msm/dp/dp_power.c   |  32 +-
> >>  drivers/gpu/drm/msm/dp/dp_power.h   |   1 +
> >>  drivers/gpu/drm/msm/dp/dp_reg.h |   1 +
> >>  17 files changed, 861 insertions(+), 424 deletions(-)
> >
> > It seems to spread various changes throughout the DP bits and only has
> > a
> > short description about what's changing. Given that the series above
> > isn't merged it would be better to get rid of this change and make the
> > changes in the patches that introduce these files.
> >
>
> Yes, the base DP driver is not yet merged as its still in reviews and
> has been for a while.
> While it is being reviewed, different developers are working on
> different aspects of DP such as base DP driver, DP compliance, audio etc
> to keep things going in parallel.
> To maintain the authorship of the different developers, we prefer having
> them as separate changes and not merge them.
> We can make all these changes as part of the same series if that shall
> help to keep things together but would prefer the changes themselves to
> be separate.
> Please consider this and let us know if that works.
>

I'm not the maintainer here so it's not really up to me, but this is 
why
we have the Co-developed-by tag, to show that multiple people worked 
on

some patch. The patch is supposed to logically stand on its own
regardless of how many people worked on it. Authorship is a single
person but the Co-developed-by tag helps express that more than one
person is the actual author of the patch. Can you use that tag instead
and then squash this into the other DP patches?


The dpu mega-patches are hard enough to review already.. I'd really
appreciated it if the dpu dev's sort out some way to squash later
fixups into earlier patches

BR,
-R
as per discussion on IRC, I have separated the parts of this change 
which are
unrelated to compliance and we have merged it to the base DP driver and 
added
the Co-developed-by tag there. Since this change adds supports for DP 
compliance
on MSM chipsets which is a new feature and not fixes to the base driver, 
we will
prefer to have this as a separate change as it will make it easier for 
you to

review it instead of continuing to expand the base DP driver
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/panel: remove meaningless if(ret) check code.

2020-07-22 Thread Bernard Zhao

The function drm_panel_add always returns true.
So if(ret) check code will never run into error branch.
Remove these check make the code a bit readable.

Signed-off-by: Bernard Zhao 
---
 drivers/gpu/drm/panel/panel-feixin-k101-im2ba02.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-feixin-k101-im2ba02.c 
b/drivers/gpu/drm/panel/panel-feixin-k101-im2ba02.c
index fddbfddf6566..7f2eb54e4254 100644
--- a/drivers/gpu/drm/panel/panel-feixin-k101-im2ba02.c
+++ b/drivers/gpu/drm/panel/panel-feixin-k101-im2ba02.c
@@ -478,9 +478,7 @@ static int k101_im2ba02_dsi_probe(struct mipi_dsi_device 
*dsi)
if (ret)
return ret;
 
-   ret = drm_panel_add(&ctx->panel);
-   if (ret < 0)
-   return ret;
+   drm_panel_add(&ctx->panel);
 
dsi->mode_flags = MIPI_DSI_MODE_VIDEO;
dsi->format = MIPI_DSI_FMT_RGB888;
-- 
2.17.1

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

[PATCH -next] drm/nouveau/kms/nvd9-: Fix file release memory leak

2020-07-22 Thread Wei Yongjun

When using single_open() for opening, single_release() should be
used instead of seq_release(), otherwise there is a memory leak.

Fixes: 12885ecbfe62 ("drm/nouveau/kms/nvd9-: Add CRC support")
Reported-by: Hulk Robot 
Signed-off-by: Wei Yongjun 
---
 drivers/gpu/drm/nouveau/dispnv50/crc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/crc.c 
b/drivers/gpu/drm/nouveau/dispnv50/crc.c
index f17fb6d56757..4971a1042415 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/crc.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/crc.c
@@ -706,6 +706,7 @@ static const struct file_operations 
nv50_crc_flip_threshold_fops = {
.open = nv50_crc_debugfs_flip_threshold_open,
.read = seq_read,
.write = nv50_crc_debugfs_flip_threshold_set,
+   .release = single_release,
 };
 
 int nv50_head_crc_late_register(struct nv50_head *head)



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

[PATCH] drm/panel: remove meaningless if(ret) check code.

2020-07-22 Thread Bernard Zhao

The function drm_panel_add always returns true.
So if(ret) check code will never run into error branch.
Remove these check will make the code a bit readable.

Signed-off-by: Bernard Zhao 
---
 drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c 
b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
index 95b789ab9d29..2c168a405928 100644
--- a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
+++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
@@ -225,9 +225,7 @@ static int feiyang_dsi_probe(struct mipi_dsi_device *dsi)
if (ret)
return ret;
 
-   ret = drm_panel_add(&ctx->panel);
-   if (ret < 0)
-   return ret;
+   drm_panel_add(&ctx->panel);
 
dsi->mode_flags = MIPI_DSI_MODE_VIDEO_BURST;
dsi->format = MIPI_DSI_FMT_RGB888;
-- 
2.17.1

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

Re: [PATCH] io-mapping: Indicate mapping failure

2020-07-22 Thread Mike Rapoport

On Tue, Jul 21, 2020 at 03:00:41PM +, Ruhl, Michael J wrote:
> >-Original Message-
> >From: Andy Shevchenko 
> >Sent: Tuesday, July 21, 2020 10:47 AM
> >To: Ruhl, Michael J 
> >Cc: dri-devel@lists.freedesktop.org; Andrew Morton  >foundation.org>; Mike Rapoport ; Chris Wilson
> >; sta...@vger.kernel.org
> >Subject: Re: [PATCH] io-mapping: Indicate mapping failure
> >
> >On Tue, Jul 21, 2020 at 10:16:41AM -0400, Michael J. Ruhl wrote:
> >> Sometimes it is good to know when your mapping failed.

I was going to say it's always a good idea ;-)

> >Can you elaborate...
> 
> Sure, guess I was too glib. 😊
> 
> Currently  the io_mapping_init_wc (the !ATOMIC_IOMAP version), function will
> always return success.
> 
> If the setting of the iomem (from ioremap_wc) fails, the only way for the 
> caller to know is to check the value of iomap->iomem.
> 
> Since all of the callers expect a NULL return on error, and check for a NULL,
> I felt this needed a fixes (i.e. unexpected behavior).
> 
> >> Fixes: cafaf14a5d8f ("io-mapping: Always create a struct to hold metadata
> >about the io-mapping"
> >
> >...especially taking into account that Fixes implies regression / bug?
> 
> The failure (in my case a crash) is not revealed until the address is accessed
> long after the init.
> 
> I will update the commit.
> 
> Mike
> 
> >--
> >With Best Regards,
> >Andy Shevchenko
> >
> 

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

Re: [PATCH] io-mapping: Indicate mapping failure

On Tue, Jul 21, 2020 at 10:16:41AM -0400, Michael J. Ruhl wrote:
> Sometimes it is good to know when your mapping failed.

Can you elaborate...

> Fixes: cafaf14a5d8f ("io-mapping: Always create a struct to hold metadata 
> about the io-mapping"

...especially taking into account that Fixes implies regression / bug?

-- 
With Best Regards,
Andy Shevchenko


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

Re: [PATCH v1] io-mapping: Indicate mapping failure

On Tue, Jul 21, 2020 at 11:34:26AM -0400, Michael J. Ruhl wrote:

Thanks for an update, my comments below.

> The !ATOMIC_IOMAP version of io_maping_init_wc will always return
> success, even when the ioremap fails.
> 
> Since the ATOMIC_IOMAP version returns NULL when the init fails, and
> callers check for a NULL return on error this is unexpected.
> 
> Return NULL on ioremap failure.
> 
> Fixes: cafaf14a5d8f ("io-mapping: Always create a struct to hold metadata 
> about the io-mapping"

Missed parenthesis.

Still not visible why Fixes tag.
Provide also couple of lines of crash and add a paragraph about it.

-- 
With Best Regards,
Andy Shevchenko


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

Re: nouveau regression with 5.7 caused by "PCI/PM: Assume ports without DLL Link Active train links in 100 ms"

2020-07-22 Thread Patrick Volkerding

On 7/21/20 10:27 AM, Mika Westerberg wrote:
> On Tue, Jul 21, 2020 at 11:01:55AM -0400, Lyude Paul wrote:
>> Sure thing. Also, feel free to let me know if you'd like access to one of the
>> systems we saw breaking with this patch - I'm fairly sure I've got one of 
>> them
>> locally at my apartment and don't mind setting up AMT/KVM/SSH
> Probably no need for remote access (thanks for the offer, though). I
> attached a test patch to the bug report:
>
>   https://bugzilla.kernel.org/show_bug.cgi?id=208597
>
> that tries to work it around (based on the ->pm_cap == 0). I wonder if
> anyone would have time to try it out.


Hi Mika,

I can confirm that this patch applied to 5.4.52 fixes the issue with
hybrid graphics on the Thinkpad X1 Extreme gen2.

Thanks,

Pat

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

[PATCH -next] drm/amdgpu/vcn3.0: Remove set but not used variable 'direct_poll'

2020-07-22 Thread YueHaibing

drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c: In function vcn_v3_0_start_sriov:
drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c:1235:3:
 warning: variable direct_poll set but not used [-Wunused-but-set-variable]

It is never used, so can remove it.

Signed-off-by: YueHaibing 
---
 drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index 910a4a32ff78..53f680134c40 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -1231,8 +1231,6 @@ static int vcn_v3_0_start_sriov(struct amdgpu_device 
*adev)
direct_wt = { {0} };
struct mmsch_v3_0_cmd_direct_read_modify_write
direct_rd_mod_wt = { {0} };
-   struct mmsch_v3_0_cmd_direct_polling
-   direct_poll = { {0} };
struct mmsch_v3_0_cmd_end end = { {0} };
struct mmsch_v3_0_init_header header;
 
@@ -1240,8 +1238,6 @@ static int vcn_v3_0_start_sriov(struct amdgpu_device 
*adev)
MMSCH_COMMAND__DIRECT_REG_WRITE;
direct_rd_mod_wt.cmd_header.command_type =
MMSCH_COMMAND__DIRECT_REG_READ_MODIFY_WRITE;
-   direct_poll.cmd_header.command_type =
-   MMSCH_COMMAND__DIRECT_REG_POLLING;
end.cmd_header.command_type =
MMSCH_COMMAND__END;
 
-- 
2.17.1


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

[PULL] drm-misc-fixes

2020-07-22 Thread Thomas Zimmermann

Hi Dave and Daniel,

here's this week's PR for drm-misc-fixes. There are only 2 fixes. The sun4i
patch updates a bugfix that was merged via drm-misc-fixes a few weeks ago.

Best regards
Thomas

drm-misc-fixes-2020-07-22:
 * sun4i: Fix inverted HPD result; fixes an earlier fix
 * lima: fix timeout during reset

The following changes since commit 6348dd291e3653534a9e28e6917569bc9967b35b:

  dmabuf: use spinlock to access dmabuf->name (2020-07-10 15:39:29 +0530)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2020-07-22

for you to fetch changes up to f3f90c6db188d437add55aaffadd5ad5bcb8cda6:

  drm/lima: fix wait pp reset timeout (2020-07-20 08:46:06 +0800)


 * sun4i: Fix inverted HPD result; fixes an earlier fix
 * lima: fix timeout during reset


Chen-Yu Tsai (1):
  drm: sun4i: hdmi: Fix inverted HPD result

Qiang Yu (1):
  drm/lima: fix wait pp reset timeout

 drivers/gpu/drm/lima/lima_pp.c | 2 ++
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau

2020-07-22 Thread Chauhan, Madhav

[AMD Public Use]

-Original Message-
From: Christian König  
Sent: Tuesday, July 21, 2020 1:03 PM
To: dri-devel@lists.freedesktop.org
Cc: Chauhan, Madhav ; tzimmerm...@suse.de; 
michael.j.r...@intel.com
Subject: [PATCH 02/11] drm/ttm: cleanup io_mem interface with nouveau

Nouveau is the only user of this functionality and evicting io space on -EAGAIN 
is really a misuse of the return code.

Instead switch to using -ENOSPC here which makes much more sense and simplifies 
the code.

Signed-off-by: Christian König 

Complete remaining cleanup patches (Patch 2- 11) looks fine,
Patch 2-11: Reviewed-by: Madhav Chauhan 

Regards,
Madhav
---
 drivers/gpu/drm/nouveau/nouveau_bo.c | 2 --
 drivers/gpu/drm/ttm/ttm_bo_util.c| 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c 
b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 61355cfb7335..a48652826f67 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1505,8 +1505,6 @@ nouveau_ttm_io_mem_reserve(struct ttm_bo_device *bdev, 
struct ttm_mem_reg *reg)
if (ret != 1) {
if (WARN_ON(ret == 0))
return -EINVAL;
-   if (ret == -ENOSPC)
-   return -EAGAIN;
return ret;
}
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 5e0f3a9caedc..7d2c50fef456 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -116,7 +116,7 @@ static int ttm_mem_io_evict(struct ttm_mem_type_manager 
*man)
struct ttm_buffer_object *bo;
 
if (!man->use_io_reserve_lru || list_empty(&man->io_reserve_lru))
-   return -EAGAIN;
+   return -ENOSPC;
 
bo = list_first_entry(&man->io_reserve_lru,
  struct ttm_buffer_object,
@@ -143,7 +143,7 @@ int ttm_mem_io_reserve(struct ttm_bo_device *bdev,
mem->bus.io_reserved_count++ == 0) {
 retry:
ret = bdev->driver->io_mem_reserve(bdev, mem);
-   if (ret == -EAGAIN) {
+   if (ret == -ENOSPC) {
ret = ttm_mem_io_evict(man);
if (ret == 0)
goto retry;
--
2.17.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Linaro-mm-sig] [PATCH 1/2] dma-buf.rst: Document why indefinite fences are a bad idea

On Wed, Jul 22, 2020 at 8:45 AM Thomas Hellström (Intel)
 wrote:
>
>
> On 2020-07-22 00:45, Dave Airlie wrote:
> > On Tue, 21 Jul 2020 at 18:47, Thomas Hellström (Intel)
> >  wrote:
> >>
> >> On 7/21/20 9:45 AM, Christian König wrote:
> >>> Am 21.07.20 um 09:41 schrieb Daniel Vetter:
>  On Mon, Jul 20, 2020 at 01:15:17PM +0200, Thomas Hellström (Intel)
>  wrote:
> > Hi,
> >
> > On 7/9/20 2:33 PM, Daniel Vetter wrote:
> >> Comes up every few years, gets somewhat tedious to discuss, let's
> >> write this down once and for all.
> >>
> >> What I'm not sure about is whether the text should be more explicit in
> >> flat out mandating the amdkfd eviction fences for long running compute
> >> workloads or workloads where userspace fencing is allowed.
> > Although (in my humble opinion) it might be possible to completely
> > untangle
> > kernel-introduced fences for resource management and dma-fences used
> > for
> > completion- and dependency tracking and lift a lot of restrictions
> > for the
> > dma-fences, including prohibiting infinite ones, I think this makes
> > sense
> > describing the current state.
>  Yeah I think a future patch needs to type up how we want to make that
>  happen (for some cross driver consistency) and what needs to be
>  considered. Some of the necessary parts are already there (with like the
>  preemption fences amdkfd has as an example), but I think some clear docs
>  on what's required from both hw, drivers and userspace would be really
>  good.
> >>> I'm currently writing that up, but probably still need a few days for
> >>> this.
> >> Great! I put down some (very) initial thoughts a couple of weeks ago
> >> building on eviction fences for various hardware complexity levels here:
> >>
> >> https://gitlab.freedesktop.org/thomash/docs/-/blob/master/Untangling%20dma-fence%20and%20memory%20allocation.odt
> > We are seeing HW that has recoverable GPU page faults but only for
> > compute tasks, and scheduler without semaphores hw for graphics.
> >
> > So a single driver may have to expose both models to userspace and
> > also introduces the problem of how to interoperate between the two
> > models on one card.
> >
> > Dave.
>
> Hmm, yes to begin with it's important to note that this is not a
> replacement for new programming models or APIs, This is something that
> takes place internally in drivers to mitigate many of the restrictions
> that are currently imposed on dma-fence and documented in this and
> previous series. It's basically the driver-private narrow completions
> Jason suggested in the lockdep patches discussions implemented the same
> way as eviction-fences.
>
> The memory fence API would be local to helpers and middle-layers like
> TTM, and the corresponding drivers.  The only cross-driver-like
> visibility would be that the dma-buf move_notify() callback would not be
> allowed to wait on dma-fences or something that depends on a dma-fence.

Because we can't preempt (on some engines at least) we already have
the requirement that cross driver buffer management can get stuck on a
dma-fence. Not even taking into account the horrors we do with
userptr, which are cross driver no matter what. Limiting move_notify
to memory fences only doesn't work, since the pte clearing might need
to wait for a dma_fence first. Hence this becomes a full end-of-batch
fence, not just a limited kernel-internal memory fence.

That's kinda why I think only reasonable option is to toss in the
towel and declare dma-fence to be the memory fence (and suck up all
the consequences of that decision as uapi, which is kinda where we
are), and construct something new&entirely free-wheeling for userspace
fencing. But only for engines that allow enough preempt/gpu page
faulting to make that possible. Free wheeling userspace fences/gpu
semaphores or whatever you want to call them (on windows I think it's
monitored fence) only work if you can preempt to decouple the memory
fences from your gpu command execution.

There's the in-between step of just decoupling the batchbuffer
submission prep for hw without any preempt (but a scheduler), but that
seems kinda pointless. Modern execbuf should be O(1) fastpath, with
all the allocation/mapping work pulled out ahead. vk exposes that
model directly to clients, GL drivers could use it internally too, so
I see zero value in spending lots of time engineering very tricky
kernel code just for old userspace. Much more reasonable to do that in
userspace, where we have real debuggers and no panics about security
bugs (or well, a lot less, webgl is still a thing, but at least
browsers realized you need to container that completely).

Cheers, Daniel

> So with that in mind, I don't foresee engines with different
> capabilities on the same card being a problem.
>
> /Thomas
>
>

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
__

Re: pages pinned for BO lifetime and security


Am 22.07.20 um 02:22 schrieb Gurchetan Singh:

+Christian who added DMABUF_MOVE_NOTIFY which added the relevant blurb:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/dma-buf/Kconfig#n46 



Currently, the user seems to amdgpu for P2P dma-buf and it seems to 
plumb ttm (*move_notify) callback to dma-buf.  We're not sure if it's 
a security issue occurring across DRM drivers, or one more specific to 
the new amdgpu use case.


On Tue, Jul 21, 2020 at 1:03 PM Chia-I Wu > wrote:


Hi list,

virtio-gpu is moving in the direction where BO pages are pinned for
the lifetime for simplicity.  I am wondering if that is considered a
security issue in general, especially after running into the
description of the new DMABUF_MOVE_NOTIFY config option.



Yes, that is generally considered a deny of service possibility and so 
far Dave and Daniel have rejected all tries to upstream stuff like this 
as far as I know.


DMA-buf an pinning for scanout are the only exceptions since the 
implementation wouldn't have been possible otherwise.




Most drivers do not have a shrinker, or whether a BO is purgeable is
entirely controlled by the userspace (madvice).  They can be
categorized as "a security problem where userspace is able to pin
unrestricted amounts of memory".  But those drivers are normally found
on systems without swap.  I don't think the issue applies.



This is completely independent of the availability of swap or not.

Pinning of pages in large quantities can result in all kind of problems 
and needs to be prevented even without swap.


Otherwise you can ran into problems even with simple I/O operations for 
example.




Of the desktop GPU drivers, i915's shrinker certainly supports purging
to swap.  TTM is a bit hard to follow.  I can't really tell if amdgpu
or nouveau supports that.  virtio-gpu is more commonly found on
systems with swaps so I think it should follow the desktop practices?



What we do at least in the amdgpu, radeon, i915 and nouveau is to only 
allow it for scanout and that in turn is limited by the physical number 
of CRTCs on the board.




Truth is, the emulated virtio-gpu device always supports page moves
with VIRTIO_GPU_CMD_RESOURCE_{ATTACH,DETACH}_BACKING.  It is just that
the driver does not make use of them.  That makes this less of an
issue because the driver can be fixed anytime (finger crossed that the
emulator won't have bugs in these untested paths).  This issue becomes
more urgent because we are considering adding a new HW command[1]
where page moves will be disallowed.  We definitely don't want a HW
command that is inherently insecure, if BO pages pinned for the
lifetime is considered a security issue on desktops.



Yeah, that's probably not such a good idea :)

Regards,
Christian.



[1] VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB

https://gitlab.freedesktop.org/virgl/drm-misc-next/-/blob/virtio-gpu-next/include/uapi/linux/virtio_gpu.h#L396





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

Re: pages pinned for BO lifetime and security


Am 22.07.20 um 09:19 schrieb Christian König:

Am 22.07.20 um 02:22 schrieb Gurchetan Singh:

+Christian who added DMABUF_MOVE_NOTIFY which added the relevant blurb:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/dma-buf/Kconfig#n46 



Currently, the user seems to amdgpu for P2P dma-buf and it seems to 
plumb ttm (*move_notify) callback to dma-buf.  We're not sure if it's 
a security issue occurring across DRM drivers, or one more specific 
to the new amdgpu use case.


On Tue, Jul 21, 2020 at 1:03 PM Chia-I Wu > wrote:


Hi list,

virtio-gpu is moving in the direction where BO pages are pinned for
the lifetime for simplicity.  I am wondering if that is considered a
security issue in general, especially after running into the
description of the new DMABUF_MOVE_NOTIFY config option.



Yes, that is generally considered a deny of service possibility and so 
far Dave and Daniel have rejected all tries to upstream stuff like 
this as far as I know.


DMA-buf an pinning for scanout are the only exceptions since the 
implementation wouldn't have been possible otherwise.


Or better say for scanout pinning is a hardware requirement. For DMA-buf 
we obviously can have a better approach :)


Christian.





Most drivers do not have a shrinker, or whether a BO is purgeable is
entirely controlled by the userspace (madvice).  They can be
categorized as "a security problem where userspace is able to pin
unrestricted amounts of memory".  But those drivers are normally
found
on systems without swap.  I don't think the issue applies.



This is completely independent of the availability of swap or not.

Pinning of pages in large quantities can result in all kind of 
problems and needs to be prevented even without swap.


Otherwise you can ran into problems even with simple I/O operations 
for example.




Of the desktop GPU drivers, i915's shrinker certainly supports
purging
to swap.  TTM is a bit hard to follow.  I can't really tell if amdgpu
or nouveau supports that.  virtio-gpu is more commonly found on
systems with swaps so I think it should follow the desktop practices?



What we do at least in the amdgpu, radeon, i915 and nouveau is to only 
allow it for scanout and that in turn is limited by the physical 
number of CRTCs on the board.




Truth is, the emulated virtio-gpu device always supports page moves
with VIRTIO_GPU_CMD_RESOURCE_{ATTACH,DETACH}_BACKING.  It is just
that
the driver does not make use of them.  That makes this less of an
issue because the driver can be fixed anytime (finger crossed
that the
emulator won't have bugs in these untested paths).  This issue
becomes
more urgent because we are considering adding a new HW command[1]
where page moves will be disallowed.  We definitely don't want a HW
command that is inherently insecure, if BO pages pinned for the
lifetime is considered a security issue on desktops.



Yeah, that's probably not such a good idea :)

Regards,
Christian.



[1] VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB

https://gitlab.freedesktop.org/virgl/drm-misc-next/-/blob/virtio-gpu-next/include/uapi/linux/virtio_gpu.h#L396







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

[Bug 207383] [Regression] 5.7 amdgpu/polaris11 gpf: amdgpu_atomic_commit_tail

2020-07-22 Thread bugzilla-daemon

https://bugzilla.kernel.org/show_bug.cgi?id=207383

--- Comment #83 from Christian König (christian.koe...@amd.com) ---
Instead of working around the bug I think we should concentrate on nailing the
root cause.

I suggest to insert an use after free check into just that structure. In other
words add a field "magic_number" will it with 0xdeadbeef on allocation and set
it to zero before the kfree().

A simple BUG_ON(ptr->magic_number != 0xdeadbeef) should yield results rather
quickly.

Then just add printk()s before the kfree() to figure out why we have this use
after free race.

-- 
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: pages pinned for BO lifetime and security

On Wed, Jul 22, 2020 at 9:19 AM Christian König
 wrote:
>
> Am 22.07.20 um 02:22 schrieb Gurchetan Singh:
>
> +Christian who added DMABUF_MOVE_NOTIFY which added the relevant blurb:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/dma-buf/Kconfig#n46
>
> Currently, the user seems to amdgpu for P2P dma-buf and it seems to plumb ttm 
> (*move_notify) callback to dma-buf.  We're not sure if it's a security issue 
> occurring across DRM drivers, or one more specific to the new amdgpu use case.
>
> On Tue, Jul 21, 2020 at 1:03 PM Chia-I Wu  wrote:
>>
>> Hi list,
>>
>> virtio-gpu is moving in the direction where BO pages are pinned for
>> the lifetime for simplicity.  I am wondering if that is considered a
>> security issue in general, especially after running into the
>> description of the new DMABUF_MOVE_NOTIFY config option.
>
>
> Yes, that is generally considered a deny of service possibility and so far 
> Dave and Daniel have rejected all tries to upstream stuff like this as far as 
> I know.

Uh we have merged pretty much all arm-soc drivers without real
shrinkers. Whether that was a good idea or not is maybe different
question - now that we do have pretty good helpers maybe we should
poke this a bit more. But then SoCs Suck (tm).

But for real gpus they do indeed all have shrinkers, and not just "pin
everything forever" model. Real gpus = stuff you might run on servers
or multi-app and all that stuff, not with a simple "we just kill all
background jobs if memory gets low" model like on android and other
such things.

> DMA-buf an pinning for scanout are the only exceptions since the 
> implementation wouldn't have been possible otherwise.
>
>>
>> Most drivers do not have a shrinker, or whether a BO is purgeable is
>> entirely controlled by the userspace (madvice).  They can be
>> categorized as "a security problem where userspace is able to pin
>> unrestricted amounts of memory".  But those drivers are normally found
>> on systems without swap.  I don't think the issue applies.
>
>
> This is completely independent of the availability of swap or not.
>
> Pinning of pages in large quantities can result in all kind of problems and 
> needs to be prevented even without swap.

Yeah you don't just kill swap, you kill a ton of other kernel services
with mass pinning. I think even the pinning of scanout buffers for
i915 from system memory is somewhat questionable (but I guess small
enough to not matter in practice).

> Otherwise you can ran into problems even with simple I/O operations for 
> example.
>
>>
>> Of the desktop GPU drivers, i915's shrinker certainly supports purging
>> to swap.  TTM is a bit hard to follow.  I can't really tell if amdgpu
>> or nouveau supports that.  virtio-gpu is more commonly found on
>> systems with swaps so I think it should follow the desktop practices?
>
>
> What we do at least in the amdgpu, radeon, i915 and nouveau is to only allow 
> it for scanout and that in turn is limited by the physical number of CRTCs on 
> the board.
>
>>
>> Truth is, the emulated virtio-gpu device always supports page moves
>> with VIRTIO_GPU_CMD_RESOURCE_{ATTACH,DETACH}_BACKING.  It is just that
>> the driver does not make use of them.  That makes this less of an
>> issue because the driver can be fixed anytime (finger crossed that the
>> emulator won't have bugs in these untested paths).  This issue becomes
>> more urgent because we are considering adding a new HW command[1]
>> where page moves will be disallowed.  We definitely don't want a HW
>> command that is inherently insecure, if BO pages pinned for the
>> lifetime is considered a security issue on desktops.
>
>
> Yeah, that's probably not such a good idea :)

Well if the pinning is just for the duration of the hw command, it's
fine, just like batch buffers. But if it's long term pinning then that
doesn't sound like a good idea. RDMA has this as their inherit hw
programming model (except if your hw is really fancy and has hw page
fault handling on the rdma nic), and they hard limit such pins to what
you can mlock (or something similar within rdma).
-Daniel

>
> Regards,
> Christian.
>
>>
>> [1] VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB
>> https://gitlab.freedesktop.org/virgl/drm-misc-next/-/blob/virtio-gpu-next/include/uapi/linux/virtio_gpu.h#L396
>
>
> ___
> 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: pages pinned for BO lifetime and security

On Wed, Jul 22, 2020 at 9:19 AM Christian König
 wrote:
> Am 22.07.20 um 02:22 schrieb Gurchetan Singh:
>> Of the desktop GPU drivers, i915's shrinker certainly supports purging
>> to swap.  TTM is a bit hard to follow.  I can't really tell if amdgpu
>> or nouveau supports that.  virtio-gpu is more commonly found on
>> systems with swaps so I think it should follow the desktop practices?
>
>
> What we do at least in the amdgpu, radeon, i915 and nouveau is to only allow 
> it for scanout and that in turn is limited by the physical number of CRTCs on 
> the board.

Somewhat aside, but I'm not sure the ttm shrinker really works like
that. I think there's two parts:
1. kernel thread which takes buffers and unbinds them when we're over
the ttm global limit. This is the ttm_shrink_work stuff, and it only
shrinks if the zone is over a hard limit. Below that it just leaves
buffers pinned.

2. Actual core mm shrinker, which releases buffers held in cache by
ttm_page_alloc_dma.c. But that only happens when buffers have been
unbound by the first thread, so anything below those hard limits is
not shrinkable. And iirc those hard limits are like half of system
memory or so (last time I looked through this stuff at least).

No idea why exactly things are like they are, since the first thread
already does a dma_resv_trylock, and that's enough to avoid locking
inversions when being called from 2. Or well, should be at least, for
reasonable driver design.

The only other thing I'm seeing is the global lru, but that could be
fixed by having a per-device core mm shrinker instance which directly
shrinks the per-device lru. And then we just globally balance like
with all shrinkers through the core mm "shrink everyone equally"
approach. You can even keep the separate page alloc shrinker, since
core mm always loops over all shrinkers - we're not the only ones
where shrinking one cache makes more memory available for another
cache to shrink, e.g. you can't throw out an inode without first
throwing out all the dentry pointing at them.

Another problem would be allocating memory while holding per-device
lru locks (since trylock on such a global lock in shrinkers is a
really bad idea, we know that from all the dev->struct_mutex lolz in
i915). But for ttm that's not a problem since all lru are spinlock, so
only GFP_ATOMIC allowed anyway, hence no problem.

Adding Thomas for this ttm tangent.
-Daniel
-- 
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: [Linaro-mm-sig] [PATCH 1/2] dma-buf.rst: Document why indefinite fences are a bad idea

2020-07-22 Thread Intel

On 2020-07-22 09:11, Daniel Vetter wrote:

On Wed, Jul 22, 2020 at 8:45 AM Thomas Hellström (Intel)
wrote:

On 2020-07-22 00:45, Dave Airlie wrote:

On Tue, 21 Jul 2020 at 18:47, Thomas Hellström (Intel)
wrote:

On 7/21/20 9:45 AM, Christian König wrote:

Am 21.07.20 um 09:41 schrieb Daniel Vetter:

On Mon, Jul 20, 2020 at 01:15:17PM +0200, Thomas Hellström (Intel)
wrote:

Hi,

On 7/9/20 2:33 PM, Daniel Vetter wrote:

Comes up every few years, gets somewhat tedious to discuss, let's
write this down once and for all.

What I'm not sure about is whether the text should be more explicit in
flat out mandating the amdkfd eviction fences for long running compute
workloads or workloads where userspace fencing is allowed.

Although (in my humble opinion) it might be possible to completely
untangle
kernel-introduced fences for resource management and dma-fences used
for
completion- and dependency tracking and lift a lot of restrictions
for the
dma-fences, including prohibiting infinite ones, I think this makes
sense
describing the current state.

Yeah I think a future patch needs to type up how we want to make that
happen (for some cross driver consistency) and what needs to be
considered. Some of the necessary parts are already there (with like the
preemption fences amdkfd has as an example), but I think some clear docs
on what's required from both hw, drivers and userspace would be really
good.

I'm currently writing that up, but probably still need a few days for
this.

Great! I put down some (very) initial thoughts a couple of weeks ago
building on eviction fences for various hardware complexity levels here:

https://gitlab.freedesktop.org/thomash/docs/-/blob/master/Untangling%20dma-fence%20and%20memory%20allocation.odt

We are seeing HW that has recoverable GPU page faults but only for
compute tasks, and scheduler without semaphores hw for graphics.

So a single driver may have to expose both models to userspace and
also introduces the problem of how to interoperate between the two
models on one card.

Dave.

Hmm, yes to begin with it's important to note that this is not a
replacement for new programming models or APIs, This is something that
takes place internally in drivers to mitigate many of the restrictions
that are currently imposed on dma-fence and documented in this and
previous series. It's basically the driver-private narrow completions
Jason suggested in the lockdep patches discussions implemented the same
way as eviction-fences.

The memory fence API would be local to helpers and middle-layers like
TTM, and the corresponding drivers. The only cross-driver-like
visibility would be that the dma-buf move_notify() callback would not be
allowed to wait on dma-fences or something that depends on a dma-fence.

Because we can't preempt (on some engines at least) we already have
the requirement that cross driver buffer management can get stuck on a
dma-fence. Not even taking into account the horrors we do with
userptr, which are cross driver no matter what. Limiting move_notify
to memory fences only doesn't work, since the pte clearing might need
to wait for a dma_fence first. Hence this becomes a full end-of-batch
fence, not just a limited kernel-internal memory fence.

For non-preemptible hardware the memory fence typically *is* the
end-of-batch fence. (Unless, as documented, there is a scheduler
consuming sync-file dependencies in which case the memory fence wait
needs to be able to break out of that). The key thing is not that we can
break out of execution, but that we can break out of dependencies, since
when we're executing all dependecies (modulo semaphores) are already
fulfilled. That's what's eliminating the deadlocks.

That's kinda why I think only reasonable option is to toss in the
towel and declare dma-fence to be the memory fence (and suck up all
the consequences of that decision as uapi, which is kinda where we
are), and construct something new&entirely free-wheeling for userspace
fencing. But only for engines that allow enough preempt/gpu page
faulting to make that possible. Free wheeling userspace fences/gpu
semaphores or whatever you want to call them (on windows I think it's
monitored fence) only work if you can preempt to decouple the memory
fences from your gpu command execution.

There's the in-between step of just decoupling the batchbuffer
submission prep for hw without any preempt (but a scheduler), but that
seems kinda pointless. Modern execbuf should be O(1) fastpath, with
all the allocation/mapping work pulled out ahead. vk exposes that
model directly to clients, GL drivers could use it internally too, so
I see zero value in spending lots of time engineering very tricky
kernel code just for old userspace. Much more reasonable to do that in
userspace, where we have real debuggers and no panics about security
bugs (or well, a lot less, webgl is still a thing, but at least
browsers realized you need to container that completely).

Sure, it's definitely a big chunk o

Re: [PATCH v2] fbdev: Detect integer underflow at "struct fbcon_ops"->clear_margins.

On Tue, Jul 21, 2020 at 6:08 PM Greg Kroah-Hartman
 wrote:
>
> On Thu, Jul 16, 2020 at 08:27:21PM +0900, Tetsuo Handa wrote:
> > On 2020/07/16 19:00, Daniel Vetter wrote:
> > > On Thu, Jul 16, 2020 at 12:29:00AM +0900, Tetsuo Handa wrote:
> > >> On 2020/07/16 0:12, Dan Carpenter wrote:
> > >>> I've complained about integer overflows in fbdev for a long time...
> > >>>
> > >>> What I'd like to see is something like the following maybe.  I don't
> > >>> know how to get the vc_data in fbmem.c so it doesn't include your checks
> > >>> for negative.
> > >>
> > >> Yes. Like I said "Thus, I consider that we need more sanity/constraints 
> > >> checks." at
> > >> https://lore.kernel.org/lkml/b1e7dd6a-fc22-bba8-0abb-d3e779329...@i-love.sakura.ne.jp/
> > >>  ,
> > >> we want basic checks. That's a task for fbdev people who should be 
> > >> familiar with
> > >> necessary constraints.
> > >
> > > I think the worldwide supply of people who understand fbdev and willing to
> > > work on it is roughly 0. So if someone wants to fix this mess properly
> > > (which likely means adding tons of over/underflow checks at entry points,
> > > since you're never going to catch the driver bugs, there's too many and
> > > not enough people who care) they need to fix this themselves.
> >
> > But I think we can enforce reasonable constraint which is much stricter 
> > than Dan's basic_checks()
> > (which used INT_MAX). For example, do we need to accept var->{xres,yres} >= 
> > 1048576, for
> > "32768 rows or cols" * "32 pixels per character" = 1045876 and 
> > vc_do_resize() accepts only
> > rows and cols < 32768 ?
> >
> > >
> > > Just to avoid confusion here.
> > >
> > >> Anyway, my two patches are small and low cost; can we apply these 
> > >> patches regardless
> > >> of basic checks?
> > >
> > > Which two patches where?
> >
> > [PATCH v3] vt: Reject zero-sized screen buffer size.
> >  from 
> > https://lkml.kernel.org/r/20200712111013.11881-1-penguin-ker...@i-love.sakura.ne.jp
>
> This is now in my tree.
>
> > [PATCH v2] fbdev: Detect integer underflow at "struct 
> > fbcon_ops"->clear_margins.
> >  from 
> > https://lkml.kernel.org/r/20200715015102.3814-1-penguin-ker...@i-love.sakura.ne.jp
>
> That should be taken by the fbdev maintainer, but I can take it too if
> people want.

Just missed this weeks pull request train and feeling like not worth
making this an exception (it's been broken forever after all), so
maybe best if you just add this to vt.

Acked-by: Daniel Vetter 

Also this avoids the impression I know what's going on in fbdev code,
maybe with sufficient abandon from my side someone will pop up who
cares an fixes the bazillion of syzkaller issues we seem to have
around console/vt and everything related.
-Daniel
-- 
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: [Linaro-mm-sig] [PATCH 1/2] dma-buf.rst: Document why indefinite fences are a bad idea

On Wed, Jul 22, 2020 at 10:05 AM Thomas Hellström (Intel)
 wrote:
>
>
> On 2020-07-22 09:11, Daniel Vetter wrote:
> > On Wed, Jul 22, 2020 at 8:45 AM Thomas Hellström (Intel)
> >  wrote:
> >>
> >> On 2020-07-22 00:45, Dave Airlie wrote:
> >>> On Tue, 21 Jul 2020 at 18:47, Thomas Hellström (Intel)
> >>>  wrote:
>  On 7/21/20 9:45 AM, Christian König wrote:
> > Am 21.07.20 um 09:41 schrieb Daniel Vetter:
> >> On Mon, Jul 20, 2020 at 01:15:17PM +0200, Thomas Hellström (Intel)
> >> wrote:
> >>> Hi,
> >>>
> >>> On 7/9/20 2:33 PM, Daniel Vetter wrote:
>  Comes up every few years, gets somewhat tedious to discuss, let's
>  write this down once and for all.
> 
>  What I'm not sure about is whether the text should be more explicit 
>  in
>  flat out mandating the amdkfd eviction fences for long running 
>  compute
>  workloads or workloads where userspace fencing is allowed.
> >>> Although (in my humble opinion) it might be possible to completely
> >>> untangle
> >>> kernel-introduced fences for resource management and dma-fences used
> >>> for
> >>> completion- and dependency tracking and lift a lot of restrictions
> >>> for the
> >>> dma-fences, including prohibiting infinite ones, I think this makes
> >>> sense
> >>> describing the current state.
> >> Yeah I think a future patch needs to type up how we want to make that
> >> happen (for some cross driver consistency) and what needs to be
> >> considered. Some of the necessary parts are already there (with like 
> >> the
> >> preemption fences amdkfd has as an example), but I think some clear 
> >> docs
> >> on what's required from both hw, drivers and userspace would be really
> >> good.
> > I'm currently writing that up, but probably still need a few days for
> > this.
>  Great! I put down some (very) initial thoughts a couple of weeks ago
>  building on eviction fences for various hardware complexity levels here:
> 
>  https://gitlab.freedesktop.org/thomash/docs/-/blob/master/Untangling%20dma-fence%20and%20memory%20allocation.odt
> >>> We are seeing HW that has recoverable GPU page faults but only for
> >>> compute tasks, and scheduler without semaphores hw for graphics.
> >>>
> >>> So a single driver may have to expose both models to userspace and
> >>> also introduces the problem of how to interoperate between the two
> >>> models on one card.
> >>>
> >>> Dave.
> >> Hmm, yes to begin with it's important to note that this is not a
> >> replacement for new programming models or APIs, This is something that
> >> takes place internally in drivers to mitigate many of the restrictions
> >> that are currently imposed on dma-fence and documented in this and
> >> previous series. It's basically the driver-private narrow completions
> >> Jason suggested in the lockdep patches discussions implemented the same
> >> way as eviction-fences.
> >>
> >> The memory fence API would be local to helpers and middle-layers like
> >> TTM, and the corresponding drivers.  The only cross-driver-like
> >> visibility would be that the dma-buf move_notify() callback would not be
> >> allowed to wait on dma-fences or something that depends on a dma-fence.
> > Because we can't preempt (on some engines at least) we already have
> > the requirement that cross driver buffer management can get stuck on a
> > dma-fence. Not even taking into account the horrors we do with
> > userptr, which are cross driver no matter what. Limiting move_notify
> > to memory fences only doesn't work, since the pte clearing might need
> > to wait for a dma_fence first. Hence this becomes a full end-of-batch
> > fence, not just a limited kernel-internal memory fence.
>
> For non-preemptible hardware the memory fence typically *is* the
> end-of-batch fence. (Unless, as documented, there is a scheduler
> consuming sync-file dependencies in which case the memory fence wait
> needs to be able to break out of that). The key thing is not that we can
> break out of execution, but that we can break out of dependencies, since
> when we're executing all dependecies (modulo semaphores) are already
> fulfilled. That's what's eliminating the deadlocks.
>
> > That's kinda why I think only reasonable option is to toss in the
> > towel and declare dma-fence to be the memory fence (and suck up all
> > the consequences of that decision as uapi, which is kinda where we
> > are), and construct something new&entirely free-wheeling for userspace
> > fencing. But only for engines that allow enough preempt/gpu page
> > faulting to make that possible. Free wheeling userspace fences/gpu
> > semaphores or whatever you want to call them (on windows I think it's
> > monitored fence) only work if you can preempt to decouple the memory
> > fences from your gpu command execution.
> >
> > There's the in-between step of just decoupling the batchbuffer
> > submission p

Re: [Linaro-mm-sig] [PATCH 1/2] dma-buf.rst: Document why indefinite fences are a bad idea

2020-07-22 Thread Intel

On 2020-07-22 11:45, Daniel Vetter wrote:

On Wed, Jul 22, 2020 at 10:05 AM Thomas Hellström (Intel)
wrote:

On 2020-07-22 09:11, Daniel Vetter wrote:

On Wed, Jul 22, 2020 at 8:45 AM Thomas Hellström (Intel)
wrote:

On 2020-07-22 00:45, Dave Airlie wrote:

On Tue, 21 Jul 2020 at 18:47, Thomas Hellström (Intel)
wrote:

On 7/21/20 9:45 AM, Christian König wrote:

Am 21.07.20 um 09:41 schrieb Daniel Vetter:

On Mon, Jul 20, 2020 at 01:15:17PM +0200, Thomas Hellström (Intel)
wrote:

Hi,

On 7/9/20 2:33 PM, Daniel Vetter wrote:

Comes up every few years, gets somewhat tedious to discuss, let's
write this down once and for all.

I'm currently writing that up, but probably still need a few days for
this.

Great! I put down some (very) initial thoughts a couple of weeks ago
building on eviction fences for various hardware complexity levels here:

https://gitlab.freedesktop.org/thomash/docs/-/blob/master/Untangling%20dma-fence%20and%20memory%20allocation.odt

We are seeing HW that has recoverable GPU page faults but only for
compute tasks, and scheduler without semaphores hw for graphics.

So a single driver may have to expose both models to userspace and
also introduces the problem of how to interoperate between the two
models on one card.

Dave.

Re: pages pinned for BO lifetime and security


Am 22.07.20 um 09:46 schrieb Daniel Vetter:

On Wed, Jul 22, 2020 at 9:19 AM Christian König
 wrote:

Am 22.07.20 um 02:22 schrieb Gurchetan Singh:

Of the desktop GPU drivers, i915's shrinker certainly supports purging
to swap.  TTM is a bit hard to follow.  I can't really tell if amdgpu
or nouveau supports that.  virtio-gpu is more commonly found on
systems with swaps so I think it should follow the desktop practices?


What we do at least in the amdgpu, radeon, i915 and nouveau is to only allow it 
for scanout and that in turn is limited by the physical number of CRTCs on the 
board.

Somewhat aside, but I'm not sure the ttm shrinker really works like
that. I think there's two parts:
1. kernel thread which takes buffers and unbinds them when we're over
the ttm global limit. This is the ttm_shrink_work stuff, and it only
shrinks if the zone is over a hard limit. Below that it just leaves
buffers pinned.

2. Actual core mm shrinker, which releases buffers held in cache by
ttm_page_alloc_dma.c. But that only happens when buffers have been
unbound by the first thread, so anything below those hard limits is
not shrinkable. And iirc those hard limits are like half of system
memory or so (last time I looked through this stuff at least).

No idea why exactly things are like they are, since the first thread
already does a dma_resv_trylock, and that's enough to avoid locking
inversions when being called from 2. Or well, should be at least, for
reasonable driver design.


Yes, that's currently a bit messy in TTM and not such a good design over 
all.



The only other thing I'm seeing is the global lru, but that could be
fixed by having a per-device core mm shrinker instance which directly
shrinks the per-device lru. And then we just globally balance like
with all shrinkers through the core mm "shrink everyone equally"
approach. You can even keep the separate page alloc shrinker, since
core mm always loops over all shrinkers - we're not the only ones
where shrinking one cache makes more memory available for another
cache to shrink, e.g. you can't throw out an inode without first
throwing out all the dentry pointing at them.


My plan is to replace all this with an explicit SWAP domain for buffer 
objects.


One idea was to make the SYSTEM and SWAP domain global and express all 
this with transits between the different domains. But having one 
shrinker per device sounds like an even better idea now.



Another problem would be allocating memory while holding per-device
lru locks (since trylock on such a global lock in shrinkers is a
really bad idea, we know that from all the dev->struct_mutex lolz in
i915). But for ttm that's not a problem since all lru are spinlock, so
only GFP_ATOMIC allowed anyway, hence no problem.


Yes, exactly.

Christian.



Adding Thomas for this ttm tangent.
-Daniel


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

[PULL] drm-misc-next

2020-07-22 Thread Maarten Lankhorst

drm-misc-next-2020-07-22:
drm-misc-next for v5.9:

UAPI Changes:

Cross-subsystem Changes:
- Convert panel-dsi-cm and ingenic bindings to YAML.
- Add lockdep annotations for dma-fence. \o/
- Describe why indefinite fences are a bad idea
- Update binding for rocktech jh057n00900.

Core Changes:
- Add vblank workers.
- Use spin_(un)lock_irq instead of the irqsave/restore variants in crtc code.
- Add managed vram helpers.
- Convert more logging to drm functions.
- Replace more http links with https in core and drivers.
- Cleanup to ttm iomem functions and implementation.
- Remove TTM CMA memtype as it doesn't work correctly.
- Remove TTM_MEMTYPE_FLAG_MAPPABLE for many drivers that have no
  unmappable memory resources.

Driver Changes:
- Add CRC support to nouveau, using the new vblank workers.
- Dithering and atomic state fix for nouveau.
- Fixes for Frida FRD350H54004 panel.
- Add support for OSD mode (sprite planes), IPU (scaling) and multiple
  panels/bridges to ingenic.
- Use managed vram helpers in ast.
- Assorted small fixes to ingenic, i810, mxsfb.
- Remove optional unused ttm dummy functions.
The following changes since commit 947fcfeac3295ff0961bb50803e0c4ae63cff65b:

  drm: drm_rect.h: delete duplicated word in comment (2020-07-15 14:03:02 +0200)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2020-07-22

for you to fetch changes up to acc0c39a59ccd8161b9066265fb8798b4ee07dc9:

  dt-binding: display: Allow a single port node on rocktech, jh057n00900 
(2020-07-21 09:34:23 -0600)


drm-misc-next for v5.9:

UAPI Changes:

Cross-subsystem Changes:
- Convert panel-dsi-cm and ingenic bindings to YAML.
- Add lockdep annotations for dma-fence. \o/
- Describe why indefinite fences are a bad idea
- Update binding for rocktech jh057n00900.

Core Changes:
- Add vblank workers.
- Use spin_(un)lock_irq instead of the irqsave/restore variants in crtc code.
- Add managed vram helpers.
- Convert more logging to drm functions.
- Replace more http links with https in core and drivers.
- Cleanup to ttm iomem functions and implementation.
- Remove TTM CMA memtype as it doesn't work correctly.
- Remove TTM_MEMTYPE_FLAG_MAPPABLE for many drivers that have no
  unmappable memory resources.

Driver Changes:
- Add CRC support to nouveau, using the new vblank workers.
- Dithering and atomic state fix for nouveau.
- Fixes for Frida FRD350H54004 panel.
- Add support for OSD mode (sprite planes), IPU (scaling) and multiple
  panels/bridges to ingenic.
- Use managed vram helpers in ast.
- Assorted small fixes to ingenic, i810, mxsfb.
- Remove optional unused ttm dummy functions.


Alexander A. Klimov (4):
  drm/vboxvideo: Replace HTTP links with HTTPS ones
  drm/tidss: Replace HTTP links with HTTPS ones
  drm: Replace HTTP links with HTTPS ones
  video: fbdev: Replace HTTP links with HTTPS ones

Christian König (9):
  drm/vram-helper: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  drm: remove optional dummy function from drivers using TTM
  drm/ttm: cleanup io_mem interface with nouveau
  drm/ttm: remove io_reserve_fastpath flag
  drm/ttm: cleanup coding style and implementation.
  drm/ttm: remove TTM_MEMTYPE_FLAG_CMA
  drm/vmwgfx: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  drm/nouveau: stop using TTM_MEMTYPE_FLAG_MAPPABLE
  drm/qxl: stop using TTM_MEMTYPE_FLAG_MAPPABLE v2

Christophe JAILLET (1):
  drm/i810: switch from 'pci_' to 'dma_' API

Daniel Vetter (3):
  dma-fence: basic lockdep annotations
  dma-fence: prime lockdep annotations
  dma-buf.rst: Document why indefinite fences are a bad idea

Guido Günther (1):
  drm/mxsfb: Make supported modifiers explicit

Lyude Paul (15):
  drm/vblank: Register drmm cleanup action once per drm_vblank_crtc
  drm/vblank: Use spin_(un)lock_irq() in drm_crtc_vblank_off()
  drm/vblank: Add vblank works
  drm/nouveau/kms/nv140-: Don't modify depth in state during atomic commit
  drm/nouveau/kms/nv50-: Fix disabling dithering
  drm/nouveau/kms/nv140-: Track wndw mappings in nv50_head_atom
  drm/nouveau/kms/nv50-: Expose nv50_outp_atom in disp.h
  drm/nouveau/kms/nv50-: Move hard-coded object handles into header
  drm/nouveau/kms/nvd9-: Add CRC support
  drm/nouveau/kms/nvd9-: Fix disabling CRCs alongside OR reprogramming
  drm/vblank: Use spin_(un)lock_irq() in drm_crtc_vblank_reset()
  drm/vblank: Use spin_(un)lock_irq() in drm_crtc_vblank_on()
  drm/vblank: Use spin_(un)lock_irq() in drm_legacy_vblank_post_modeset()
  drm/vblank: Use spin_(un)lock_irq() in drm_queue_vblank_event()
  drm/vblank: Use spin_(un)lock_irq() in drm_crtc_queue_sequence_ioctl()

Ondrej Jirman (2):
  dt-bindings: display: Fix example in nwl-dsi.yaml
  dt-binding: display: Allow a single port node on rocktech,

Re: [PATCH 09/20] Documentation: i2c: eliminate duplicated word

2020-07-22 Thread Wolfram Sang

On Tue, Jul 07, 2020 at 11:04:03AM -0700, Randy Dunlap wrote:
> Drop doubled word "new".
> 
> Signed-off-by: Randy Dunlap 

For the record:

Acked-by: Wolfram Sang 



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

[PATCH] drm/vkms: add missing drm_crtc_vblank_put to the get/put pair on flush

2020-07-22 Thread Melissa Wen

This patch adds a missing drm_crtc_vblank_put op to the pair
drm_crtc_vblank_get/put (inc/decrement counter to guarantee vblanks).

It clears the execution of the following kms_cursor_crc subtests:
1. pipe-A-cursor-[size,alpha-opaque, NxN-(on-screen, off-screen, sliding,
   random, fast-moving])] - successful when running individually.
2. pipe-A-cursor-dpms passes again
3. pipe-A-cursor-suspend also passes

The issue was initially tracked in the sequential execution of IGT
kms_cursor_crc subtest: when running the test sequence or one of its
subtests twice, the odd execs complete and the pairs get stuck in an
endless wait. In the IGT code, calling a wait_for_vblank before the start
of CRC capture prevented the busy-wait. But the problem persisted in the
pipe-A-cursor-dpms and -suspend subtests.

Checking the history, the pipe-A-cursor-dpms subtest was successful when,
in vkms_atomic_commit_tail, instead of using the flip_done op, it used
wait_for_vblanks. Another way to prevent blocking was wait_one_vblank when
enabling crtc. However, in both cases, pipe-A-cursor-suspend persisted
blocking in the 2nd start of CRC capture, which may indicate that
something got stuck in the step of CRC setup. Indeed, wait_one_vblank in
the crc setup was able to sync things and free all kms_cursor_crc
subtests.

Tracing and comparing a clean run with a blocked one:
- in a clean one, vkms_crtc_atomic_flush enables vblanks;
- when blocked, only in next op, vkms_crtc_atomic_enable, the vblanks
started. Moreover, a series of vkms_vblank_simulate flow out until
disabling vblanks.
Also watching the steps of vkms_crtc_atomic_flush, when the very first
drm_crtc_vblank_get returned an error, the subtest crashed. On the other
hand, when vblank_get succeeded, the subtest completed. Finally, checking
the flush steps: it increases counter to hold a vblank reference (get),
but there isn't a op to decreased it and release vblanks (put).

Cc: Daniel Vetter 
Cc: Rodrigo Siqueira 
Cc: Haneen Mohammed 
Signed-off-by: Melissa Wen 
---
 drivers/gpu/drm/vkms/vkms_crtc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c b/drivers/gpu/drm/vkms/vkms_crtc.c
index ac85e17428f8..a99d6b4a92dd 100644
--- a/drivers/gpu/drm/vkms/vkms_crtc.c
+++ b/drivers/gpu/drm/vkms/vkms_crtc.c
@@ -246,6 +246,7 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
 
spin_unlock(&crtc->dev->event_lock);
 
+   drm_crtc_vblank_put(crtc);
crtc->state->event = NULL;
}
 
-- 
2.27.0

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

Re: pages pinned for BO lifetime and security

Am 22.07.20 um 09:32 schrieb Daniel Vetter:

On Wed, Jul 22, 2020 at 9:19 AM Christian König
wrote:

Am 22.07.20 um 02:22 schrieb Gurchetan Singh:

+Christian who added DMABUF_MOVE_NOTIFY which added the relevant blurb:

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fdma-buf%2FKconfig%23n46&data=02%7C01%7Cchristian.koenig%40amd.com%7C916f6a9b64884e21fd7f08d82e115c8b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63731122752271&sdata=WKs3KSr3K1DdVmDaIZ%2FnV8VEBPWMGMSGweeay0HIOxw%3D&reserved=0

Currently, the user seems to amdgpu for P2P dma-buf and it seems to plumb ttm
(*move_notify) callback to dma-buf. We're not sure if it's a security issue
occurring across DRM drivers, or one more specific to the new amdgpu use case.

On Tue, Jul 21, 2020 at 1:03 PM Chia-I Wu wrote:

Hi list,

virtio-gpu is moving in the direction where BO pages are pinned for
the lifetime for simplicity. I am wondering if that is considered a
security issue in general, especially after running into the
description of the new DMABUF_MOVE_NOTIFY config option.

Yes, that is generally considered a deny of service possibility and so far Dave
and Daniel have rejected all tries to upstream stuff like this as far as I know.

Uh we have merged pretty much all arm-soc drivers without real
shrinkers. Whether that was a good idea or not is maybe different
question - now that we do have pretty good helpers maybe we should
poke this a bit more. But then SoCs Suck (tm).

I was under the impression that those SoC drivers still use the GEM
helpers which unpinns stuff when it is not in use. But I might be wrong.

But for real gpus they do indeed all have shrinkers, and not just "pin
everything forever" model. Real gpus = stuff you might run on servers
or multi-app and all that stuff, not with a simple "we just kill all
background jobs if memory gets low" model like on android and other
such things.

DMA-buf an pinning for scanout are the only exceptions since the implementation
wouldn't have been possible otherwise.

Most drivers do not have a shrinker, or whether a BO is purgeable is
entirely controlled by the userspace (madvice). They can be
categorized as "a security problem where userspace is able to pin
unrestricted amounts of memory". But those drivers are normally found
on systems without swap. I don't think the issue applies.

This is completely independent of the availability of swap or not.

Pinning of pages in large quantities can result in all kind of problems and
needs to be prevented even without swap.

Yeah you don't just kill swap, you kill a ton of other kernel services
with mass pinning. I think even the pinning of scanout buffers for
i915 from system memory is somewhat questionable (but I guess small
enough to not matter in practice).

Yeah, we had a really hard time explaining that internally as well.

Christian.

Otherwise you can ran into problems even with simple I/O operations for example.

Of the desktop GPU drivers, i915's shrinker certainly supports purging
to swap. TTM is a bit hard to follow. I can't really tell if amdgpu
or nouveau supports that. virtio-gpu is more commonly found on
systems with swaps so I think it should follow the desktop practices?

What we do at least in the amdgpu, radeon, i915 and nouveau is to only allow it
for scanout and that in turn is limited by the physical number of CRTCs on the
board.

Truth is, the emulated virtio-gpu device always supports page moves
with VIRTIO_GPU_CMD_RESOURCE_{ATTACH,DETACH}_BACKING. It is just that
the driver does not make use of them. That makes this less of an
issue because the driver can be fixed anytime (finger crossed that the
emulator won't have bugs in these untested paths). This issue becomes
more urgent because we are considering adding a new HW command[1]
where page moves will be disallowed. We definitely don't want a HW
command that is inherently insecure, if BO pages pinned for the
lifetime is considered a security issue on desktops.

Yeah, that's probably not such a good idea :)

Well if the pinning is just for the duration of the hw command, it's
fine, just like batch buffers. But if it's long term pinning then that
doesn't sound like a good idea. RDMA has this as their inherit hw
programming model (except if your hw is really fancy and has hw page
fault handling on the rdma nic), and they hard limit such pins to what
you can mlock (or something similar within rdma).
-Daniel

Regards,
Christian.

[1] VIRTIO_GPU_CMD_RESOURCE_CREATE_BLOB
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fvirgl%2Fdrm-misc-next%2F-%2Fblob%2Fvirtio-gpu-next%2Finclude%2Fuapi%2Flinux%2Fvirtio_gpu.h%23L396&data=02%7C01%7Cchristian.koenig%40amd.com%7C916f6a9b64884e21fd7f08d82e115c8b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63731122752271&sdata=AWtwzk%2BP7P7031ibTumr2J%2FQB%2Fzssg1Imag

Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE


Am 22.07.20 um 07:34 schrieb Daniel Vetter:

On Tue, Jul 21, 2020 at 4:46 PM Christian König
 wrote:

Am 21.07.20 um 11:24 schrieb dan...@ffwll.ch:

On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:

The driver doesn't expose any not-mapable memory resources.

Signed-off-by: Christian König 
---
   drivers/gpu/drm/radeon/radeon_ttm.c | 13 -
   1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 54af06df865b..b474781a0920 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
  switch (type) {
  case TTM_PL_SYSTEM:
  /* System memory */
-man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+man->flags = 0;
  man->available_caching = TTM_PL_MASK_CACHING;
  man->default_caching = TTM_PL_FLAG_CACHED;
  break;
@@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
  man->func = &ttm_bo_manager_func;
  man->available_caching = TTM_PL_MASK_CACHING;
  man->default_caching = TTM_PL_FLAG_CACHED;
-man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+man->flags = 0;
   #if IS_ENABLED(CONFIG_AGP)
  if (rdev->flags & RADEON_IS_AGP) {
  if (!rdev->ddev->agp) {
@@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
(unsigned)type);
  return -EINVAL;
  }
-if (!rdev->ddev->agp->cant_use_aperture)
-man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;

There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
flag set. And radeon.ko did at least once work on these. And your patch to
disable agp only changes the default, it doesn't rip out the code.

The key pint is that the flags for AGP are the same as the one for the
PCIe path. So no functional change at all :)

I misread the code somehow, I didn't spot the unconditional setting of
FLAG_MAPPABLE for all TTM_PL_TT, irrespective of agp or not, somehow
thought that's another case.

Reviewed-by: Daniel Vetter 


And for the amdgpu patch? Otherwise I just ping Alex for an rb.

Thanks,
Christian.




The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().

Christian.


So not sure your assumption here is correct.
-Daniel


  man->available_caching = TTM_PL_FLAG_UNCACHED |
   TTM_PL_FLAG_WC;
  man->default_caching = TTM_PL_FLAG_WC;
@@ -103,8 +101,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
  case TTM_PL_VRAM:
  /* "On-card" video ram */
  man->func = &ttm_bo_manager_func;
-man->flags = TTM_MEMTYPE_FLAG_FIXED |
- TTM_MEMTYPE_FLAG_MAPPABLE;
+man->flags = TTM_MEMTYPE_FLAG_FIXED;
  man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
  man->default_caching = TTM_PL_FLAG_WC;
  break;
@@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, 
bool evict,

   static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct 
ttm_mem_reg *mem)
   {
-struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
  struct radeon_device *rdev = radeon_get_rdev(bdev);

  mem->bus.addr = NULL;
@@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device 
*bdev, struct ttm_mem_
  mem->bus.size = mem->num_pages << PAGE_SHIFT;
  mem->bus.base = 0;
  mem->bus.is_iomem = false;
-if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-return -EINVAL;
+
  switch (mem->mem_type) {
  case TTM_PL_SYSTEM:
  /* system memory */
--
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchristian.koenig%40amd.com%7C2fb0200ef32f4afcc3e208d82e00ef7d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637309928847998899&sdata=9VclRJ7e3xfohsaLsiVF6Y83c%2Bkncmbo5uqoeV6tT9M%3D&reserved=0




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

Re: [PATCH v1 1/1] drm/bridge: nwl-dsi: Drop DRM_BRIDGE_ATTACH_NO_CONNECTOR check.

Hi Guido,

Thank you for the patch.

On Sat, Jul 18, 2020 at 08:26:37PM +0200, Guido Günther wrote:
> We don't create a connector but let panel_bridge handle that so there's
> no point in rejecting DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> 
> Signed-off-by: Guido Günther 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/bridge/nwl-dsi.c | 5 -
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/nwl-dsi.c 
> b/drivers/gpu/drm/bridge/nwl-dsi.c
> index 77a79af70914..ce94f797d090 100644
> --- a/drivers/gpu/drm/bridge/nwl-dsi.c
> +++ b/drivers/gpu/drm/bridge/nwl-dsi.c
> @@ -918,11 +918,6 @@ static int nwl_dsi_bridge_attach(struct drm_bridge 
> *bridge,
>   struct drm_panel *panel;
>   int ret;
>  
> - if (flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR) {
> - DRM_ERROR("Fix bridge driver to make connector optional!");
> - return -EINVAL;
> - }
> -
>   ret = drm_of_find_panel_or_bridge(dsi->dev->of_node, 1, 0, &panel,
> &panel_bridge);
>   if (ret)

-- 
Regards,

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

Re: pages pinned for BO lifetime and security

On Wed, Jul 22, 2020 at 1:12 PM Christian König
 wrote:
>
> Am 22.07.20 um 09:32 schrieb Daniel Vetter:
> > On Wed, Jul 22, 2020 at 9:19 AM Christian König
> >  wrote:
> >> Am 22.07.20 um 02:22 schrieb Gurchetan Singh:
> >>
> >> +Christian who added DMABUF_MOVE_NOTIFY which added the relevant blurb:
> >>
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2Flinux.git%2Ftree%2Fdrivers%2Fdma-buf%2FKconfig%23n46&data=02%7C01%7Cchristian.koenig%40amd.com%7C916f6a9b64884e21fd7f08d82e115c8b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C63731122752271&sdata=WKs3KSr3K1DdVmDaIZ%2FnV8VEBPWMGMSGweeay0HIOxw%3D&reserved=0
> >>
> >> Currently, the user seems to amdgpu for P2P dma-buf and it seems to plumb 
> >> ttm (*move_notify) callback to dma-buf.  We're not sure if it's a security 
> >> issue occurring across DRM drivers, or one more specific to the new amdgpu 
> >> use case.
> >>
> >> On Tue, Jul 21, 2020 at 1:03 PM Chia-I Wu  wrote:
> >>> Hi list,
> >>>
> >>> virtio-gpu is moving in the direction where BO pages are pinned for
> >>> the lifetime for simplicity.  I am wondering if that is considered a
> >>> security issue in general, especially after running into the
> >>> description of the new DMABUF_MOVE_NOTIFY config option.
> >>
> >> Yes, that is generally considered a deny of service possibility and so far 
> >> Dave and Daniel have rejected all tries to upstream stuff like this as far 
> >> as I know.
> > Uh we have merged pretty much all arm-soc drivers without real
> > shrinkers. Whether that was a good idea or not is maybe different
> > question - now that we do have pretty good helpers maybe we should
> > poke this a bit more. But then SoCs Suck (tm).
>
> I was under the impression that those SoC drivers still use the GEM
> helpers which unpinns stuff when it is not in use. But I might be wrong.

It's kinda mostly there, even some helpers for shrinking but a)
helpers on, not all drivers use it b) for purgeable objects only, not
generally for inactive stuff - there's no active use tracking c) cma
helpers (ok that one is only for vc4 as the render driver) don't even
have that. I had some slow burner series to get us towards dma_resv
locking in shmem helpers and then maybe even a common shrinker helper
with some "actually kick it out now" callback, but yeah never got
there.

So maybe per-device object shrinker helper would be something neat we
could lift out of ttm (when it's happening), maybe with a simple
callback somewhere in it's lru tracking. Probably best if the shrinker
lru is outright separate from anything else or it just gets messy.
-Daniel

> > But for real gpus they do indeed all have shrinkers, and not just "pin
> > everything forever" model. Real gpus = stuff you might run on servers
> > or multi-app and all that stuff, not with a simple "we just kill all
> > background jobs if memory gets low" model like on android and other
> > such things.
> >
> >> DMA-buf an pinning for scanout are the only exceptions since the 
> >> implementation wouldn't have been possible otherwise.
> >>
> >>> Most drivers do not have a shrinker, or whether a BO is purgeable is
> >>> entirely controlled by the userspace (madvice).  They can be
> >>> categorized as "a security problem where userspace is able to pin
> >>> unrestricted amounts of memory".  But those drivers are normally found
> >>> on systems without swap.  I don't think the issue applies.
> >>
> >> This is completely independent of the availability of swap or not.
> >>
> >> Pinning of pages in large quantities can result in all kind of problems 
> >> and needs to be prevented even without swap.
> > Yeah you don't just kill swap, you kill a ton of other kernel services
> > with mass pinning. I think even the pinning of scanout buffers for
> > i915 from system memory is somewhat questionable (but I guess small
> > enough to not matter in practice).
>
> Yeah, we had a really hard time explaining that internally as well.
>
> Christian.
>
> >> Otherwise you can ran into problems even with simple I/O operations for 
> >> example.
> >>
> >>> Of the desktop GPU drivers, i915's shrinker certainly supports purging
> >>> to swap.  TTM is a bit hard to follow.  I can't really tell if amdgpu
> >>> or nouveau supports that.  virtio-gpu is more commonly found on
> >>> systems with swaps so I think it should follow the desktop practices?
> >>
> >> What we do at least in the amdgpu, radeon, i915 and nouveau is to only 
> >> allow it for scanout and that in turn is limited by the physical number of 
> >> CRTCs on the board.
> >>
> >>> Truth is, the emulated virtio-gpu device always supports page moves
> >>> with VIRTIO_GPU_CMD_RESOURCE_{ATTACH,DETACH}_BACKING.  It is just that
> >>> the driver does not make use of them.  That makes this less of an
> >>> issue because the driver can be fixed anytime (finger crossed that the
> >>> emulator won't have bugs in these untested paths).  This issue

Re: [Intel-gfx] [PATCH v1] drm/i915/dsi: Drop double check for ACPI companion device

2020-07-22 Thread Ville Syrjälä

On Fri, May 29, 2020 at 03:33:17PM +0300, Andy Shevchenko wrote:
> acpi_dev_get_resources() does perform the NULL pointer check against
> ACPI companion device which is given as function parameter. Thus,
> there is no need to duplicate this check in the caller.
> 
> Signed-off-by: Andy Shevchenko 

Sorry, I did look at this but apparently forgot to reply...

> ---
>  drivers/gpu/drm/i915/display/intel_dsi_vbt.c | 24 
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c 
> b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> index 574dcfec9577..6f9e08cda964 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi_vbt.c
> @@ -426,23 +426,19 @@ static void i2c_acpi_find_adapter(struct intel_dsi 
> *intel_dsi,
>  {
>   struct drm_device *drm_dev = intel_dsi->base.base.dev;
>   struct device *dev = &drm_dev->pdev->dev;
> - struct acpi_device *acpi_dev;
> + struct acpi_device *acpi_dev = ACPI_COMPANION(dev);
>   struct list_head resource_list;
>   struct i2c_adapter_lookup lookup;
>  
> - acpi_dev = ACPI_COMPANION(dev);
> - if (acpi_dev) {
> - memset(&lookup, 0, sizeof(lookup));
> - lookup.slave_addr = slave_addr;
> - lookup.intel_dsi = intel_dsi;
> - lookup.dev_handle = acpi_device_handle(acpi_dev);
> -
> - INIT_LIST_HEAD(&resource_list);
> - acpi_dev_get_resources(acpi_dev, &resource_list,
> -i2c_adapter_lookup,
> -&lookup);
> - acpi_dev_free_resource_list(&resource_list);
> - }
> + memset(&lookup, 0, sizeof(lookup));
> + lookup.slave_addr = slave_addr;
> + lookup.intel_dsi = intel_dsi;
> + lookup.dev_handle = acpi_device_handle(acpi_dev);

struct i2c_adapter_lookup lookup = {
.slave_addr = ...
};

?

> +
> + INIT_LIST_HEAD(&resource_list);

Declare as LIST_HEAD(resource_list); ?

> + acpi_dev_get_resources(acpi_dev, &resource_list,
> +i2c_adapter_lookup, &lookup);
> + acpi_dev_free_resource_list(&resource_list);

I was very confused by this code since on the first glance it appears to
absolutely nothing. After a deeper look it looks like
i2c_adapter_lookup() magically mutates intel_dsi->i2c_bus_num.
Did I mention I hate functions with side effects? IMO would be much
better if i2c_adapter_lookup() did what it says on the tin and just
returned the adapter number and let the caller deal with it. But
this is a pre-existing issue with the code and so not directly related
to your patch.

>  }
>  #else
>  static inline void i2c_acpi_find_adapter(struct intel_dsi *intel_dsi,
> -- 
> 2.26.2
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Linaro-mm-sig] [PATCH 1/2] dma-buf.rst: Document why indefinite fences are a bad idea

On Wed, Jul 22, 2020 at 12:31 PM Thomas Hellström (Intel)
 wrote:
>
>
> On 2020-07-22 11:45, Daniel Vetter wrote:
> > On Wed, Jul 22, 2020 at 10:05 AM Thomas Hellström (Intel)
> >  wrote:
> >>
> >> On 2020-07-22 09:11, Daniel Vetter wrote:
> >>> On Wed, Jul 22, 2020 at 8:45 AM Thomas Hellström (Intel)
> >>>  wrote:
>  On 2020-07-22 00:45, Dave Airlie wrote:
> > On Tue, 21 Jul 2020 at 18:47, Thomas Hellström (Intel)
> >  wrote:
> >> On 7/21/20 9:45 AM, Christian König wrote:
> >>> Am 21.07.20 um 09:41 schrieb Daniel Vetter:
>  On Mon, Jul 20, 2020 at 01:15:17PM +0200, Thomas Hellström (Intel)
>  wrote:
> > Hi,
> >
> > On 7/9/20 2:33 PM, Daniel Vetter wrote:
> >> Comes up every few years, gets somewhat tedious to discuss, let's
> >> write this down once and for all.
> >>
> >> What I'm not sure about is whether the text should be more 
> >> explicit in
> >> flat out mandating the amdkfd eviction fences for long running 
> >> compute
> >> workloads or workloads where userspace fencing is allowed.
> > Although (in my humble opinion) it might be possible to completely
> > untangle
> > kernel-introduced fences for resource management and dma-fences used
> > for
> > completion- and dependency tracking and lift a lot of restrictions
> > for the
> > dma-fences, including prohibiting infinite ones, I think this makes
> > sense
> > describing the current state.
>  Yeah I think a future patch needs to type up how we want to make that
>  happen (for some cross driver consistency) and what needs to be
>  considered. Some of the necessary parts are already there (with like 
>  the
>  preemption fences amdkfd has as an example), but I think some clear 
>  docs
>  on what's required from both hw, drivers and userspace would be 
>  really
>  good.
> >>> I'm currently writing that up, but probably still need a few days for
> >>> this.
> >> Great! I put down some (very) initial thoughts a couple of weeks ago
> >> building on eviction fences for various hardware complexity levels 
> >> here:
> >>
> >> https://gitlab.freedesktop.org/thomash/docs/-/blob/master/Untangling%20dma-fence%20and%20memory%20allocation.odt
> > We are seeing HW that has recoverable GPU page faults but only for
> > compute tasks, and scheduler without semaphores hw for graphics.
> >
> > So a single driver may have to expose both models to userspace and
> > also introduces the problem of how to interoperate between the two
> > models on one card.
> >
> > Dave.
>  Hmm, yes to begin with it's important to note that this is not a
>  replacement for new programming models or APIs, This is something that
>  takes place internally in drivers to mitigate many of the restrictions
>  that are currently imposed on dma-fence and documented in this and
>  previous series. It's basically the driver-private narrow completions
>  Jason suggested in the lockdep patches discussions implemented the same
>  way as eviction-fences.
> 
>  The memory fence API would be local to helpers and middle-layers like
>  TTM, and the corresponding drivers.  The only cross-driver-like
>  visibility would be that the dma-buf move_notify() callback would not be
>  allowed to wait on dma-fences or something that depends on a dma-fence.
> >>> Because we can't preempt (on some engines at least) we already have
> >>> the requirement that cross driver buffer management can get stuck on a
> >>> dma-fence. Not even taking into account the horrors we do with
> >>> userptr, which are cross driver no matter what. Limiting move_notify
> >>> to memory fences only doesn't work, since the pte clearing might need
> >>> to wait for a dma_fence first. Hence this becomes a full end-of-batch
> >>> fence, not just a limited kernel-internal memory fence.
> >> For non-preemptible hardware the memory fence typically *is* the
> >> end-of-batch fence. (Unless, as documented, there is a scheduler
> >> consuming sync-file dependencies in which case the memory fence wait
> >> needs to be able to break out of that). The key thing is not that we can
> >> break out of execution, but that we can break out of dependencies, since
> >> when we're executing all dependecies (modulo semaphores) are already
> >> fulfilled. That's what's eliminating the deadlocks.
> >>
> >>> That's kinda why I think only reasonable option is to toss in the
> >>> towel and declare dma-fence to be the memory fence (and suck up all
> >>> the consequences of that decision as uapi, which is kinda where we
> >>> are), and construct something new&entirely free-wheeling for userspace
> >>> fencing. But only for engines that allow enough preempt/gpu page
> >>> faulting to make that possib

Re: [PATCH -next] gpu: drm: Fix spinlock vblank_time_lock use error.

2020-07-22 Thread Ville Syrjälä

On Wed, Jul 22, 2020 at 01:05:27AM +, Xu Qiang wrote:
> The drm_handle_vblank function is in the interrupt context.
> Therefore, the spin lock vblank_time_lock is obtained
> from the interrupt context.
> 
> Cc: 
> Signed-off-by: Xu Qiang 
> ---
>  drivers/gpu/drm/drm_vblank.c | 17 ++---
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index f402c75b9d34..4ca63ff33a43 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -229,10 +229,11 @@ static void drm_reset_vblank_timestamp(struct 
> drm_device *dev, unsigned int pipe
>  {
>   u32 cur_vblank;
>   bool rc;
> + unsigned long irqflags;
>   ktime_t t_vblank;
>   int count = DRM_TIMESTAMP_MAXRETRIES;
>  
> - spin_lock(&dev->vblank_time_lock);
> + spin_lock_irqsave(&dev->vblank_time_lock, irqflags);

Nak. This is always called with interrupts off, so no point on wasting
time saving/restoring the flags. And it's the same situation for all the
other cases you have below.

>  
>   /*
>* sample the current counter to avoid random jumps
> @@ -257,7 +258,7 @@ static void drm_reset_vblank_timestamp(struct drm_device 
> *dev, unsigned int pipe
>*/
>   store_vblank(dev, pipe, 1, t_vblank, cur_vblank);
>  
> - spin_unlock(&dev->vblank_time_lock);
> + spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
>  }
>  
>  /*
> @@ -1106,11 +1107,12 @@ static int __enable_vblank(struct drm_device *dev, 
> unsigned int pipe)
>  static int drm_vblank_enable(struct drm_device *dev, unsigned int pipe)
>  {
>   struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
> + unsigned long irqflags;
>   int ret = 0;
>  
>   assert_spin_locked(&dev->vbl_lock);
>  
> - spin_lock(&dev->vblank_time_lock);
> + spin_lock_irqsave(&dev->vblank_time_lock, irqflags);
>  
>   if (!vblank->enabled) {
>   /*
> @@ -1136,7 +1138,7 @@ static int drm_vblank_enable(struct drm_device *dev, 
> unsigned int pipe)
>   }
>   }
>  
> - spin_unlock(&dev->vblank_time_lock);
> + spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
>  
>   return ret;
>  }
> @@ -1917,6 +1919,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned 
> int pipe)
>  {
>   struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>   unsigned long irqflags;
> + unsigned long irqflags_vblank;
>   bool disable_irq;
>  
>   if (drm_WARN_ON_ONCE(dev, !drm_dev_has_vblank(dev)))
> @@ -1931,18 +1934,18 @@ bool drm_handle_vblank(struct drm_device *dev, 
> unsigned int pipe)
>* vblank enable/disable, as this would cause inconsistent
>* or corrupted timestamps and vblank counts.
>*/
> - spin_lock(&dev->vblank_time_lock);
> + spin_lock_irqsave(&dev->vblank_time_lock, irqflags_vblank);
>  
>   /* Vblank irq handling disabled. Nothing to do. */
>   if (!vblank->enabled) {
> - spin_unlock(&dev->vblank_time_lock);
> + spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags_vblank);
>   spin_unlock_irqrestore(&dev->event_lock, irqflags);
>   return false;
>   }
>  
>   drm_update_vblank_count(dev, pipe, true);
>  
> - spin_unlock(&dev->vblank_time_lock);
> + spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags_vblank);
>  
>   wake_up(&vblank->queue);
>  
> -- 
> 2.25.0
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE

On Wed, Jul 22, 2020 at 1:13 PM Christian König
 wrote:
>
> Am 22.07.20 um 07:34 schrieb Daniel Vetter:
> > On Tue, Jul 21, 2020 at 4:46 PM Christian König
> >  wrote:
> >> Am 21.07.20 um 11:24 schrieb dan...@ffwll.ch:
> >>> On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:
>  The driver doesn't expose any not-mapable memory resources.
> 
>  Signed-off-by: Christian König 
>  ---
> drivers/gpu/drm/radeon/radeon_ttm.c | 13 -
> 1 file changed, 4 insertions(+), 9 deletions(-)
> 
>  diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
>  b/drivers/gpu/drm/radeon/radeon_ttm.c
>  index 54af06df865b..b474781a0920 100644
>  --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>  +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>  @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device 
>  *bdev, uint32_t type,
>    switch (type) {
>    case TTM_PL_SYSTEM:
>    /* System memory */
>  -man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>  +man->flags = 0;
>    man->available_caching = TTM_PL_MASK_CACHING;
>    man->default_caching = TTM_PL_FLAG_CACHED;
>    break;
>  @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device 
>  *bdev, uint32_t type,
>    man->func = &ttm_bo_manager_func;
>    man->available_caching = TTM_PL_MASK_CACHING;
>    man->default_caching = TTM_PL_FLAG_CACHED;
>  -man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
>  +man->flags = 0;
> #if IS_ENABLED(CONFIG_AGP)
>    if (rdev->flags & RADEON_IS_AGP) {
>    if (!rdev->ddev->agp) {
>  @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device 
>  *bdev, uint32_t type,
>  (unsigned)type);
>    return -EINVAL;
>    }
>  -if (!rdev->ddev->agp->cant_use_aperture)
>  -man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >>> There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
> >>> flag set. And radeon.ko did at least once work on these. And your patch to
> >>> disable agp only changes the default, it doesn't rip out the code.
> >> The key pint is that the flags for AGP are the same as the one for the
> >> PCIe path. So no functional change at all :)
> > I misread the code somehow, I didn't spot the unconditional setting of
> > FLAG_MAPPABLE for all TTM_PL_TT, irrespective of agp or not, somehow
> > thought that's another case.
> >
> > Reviewed-by: Daniel Vetter 
>
> And for the amdgpu patch? Otherwise I just ping Alex for an rb.

See my question over there, I'm not seeing how the code prevents mmap
for AMDGPU_PL_* domains after your patch. Once that's cleared up happy
to r-b that one and the final one too.
-Daniel

>
> Thanks,
> Christian.
>
> >
> >> The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().
> >>
> >> Christian.
> >>
> >>> So not sure your assumption here is correct.
> >>> -Daniel
> >>>
>    man->available_caching = TTM_PL_FLAG_UNCACHED |
> TTM_PL_FLAG_WC;
>    man->default_caching = TTM_PL_FLAG_WC;
>  @@ -103,8 +101,7 @@ static int radeon_init_mem_type(struct ttm_bo_device 
>  *bdev, uint32_t type,
>    case TTM_PL_VRAM:
>    /* "On-card" video ram */
>    man->func = &ttm_bo_manager_func;
>  -man->flags = TTM_MEMTYPE_FLAG_FIXED |
>  - TTM_MEMTYPE_FLAG_MAPPABLE;
>  +man->flags = TTM_MEMTYPE_FLAG_FIXED;
>    man->available_caching = TTM_PL_FLAG_UNCACHED | 
>  TTM_PL_FLAG_WC;
>    man->default_caching = TTM_PL_FLAG_WC;
>    break;
>  @@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object 
>  *bo, bool evict,
> 
> static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, 
>  struct ttm_mem_reg *mem)
> {
>  -struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
>    struct radeon_device *rdev = radeon_get_rdev(bdev);
> 
>    mem->bus.addr = NULL;
>  @@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct 
>  ttm_bo_device *bdev, struct ttm_mem_
>    mem->bus.size = mem->num_pages << PAGE_SHIFT;
>    mem->bus.base = 0;
>    mem->bus.is_iomem = false;
>  -if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
>  -return -EINVAL;
>  +
>    switch (mem->mem_type) {
>    case TTM_PL_SYSTEM:
>    /* system memory */
>  --
>  2.17.1
> 
>

Re: [PATCH v7 2/3] drm: bridge: Add support for Cadence MHDP DPI/DP bridge

2020-07-22 Thread kernel test robot

Hi Swapnil,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v5.8-rc6 next-20200721]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:
https://github.com/0day-ci/linux/commits/Swapnil-Jakhade/drm-Add-support-for-Cadence-MHDP-DPI-DP-bridge-and-J721E-wrapper/20200722-154322
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: x86_64-allmodconfig (attached as .config)
compiler: gcc-9 (Debian 9.3.0-14) 9.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/bridge/cdns-mhdp-core.c: In function 'cdns_mhdp_fw_activate':
>> drivers/gpu/drm/bridge/cdns-mhdp-core.c:765:10: warning: conversion from 
>> 'long unsigned int' to 'unsigned int' changes value from 
>> '18446744073709551613' to '4294967293' [-Woverflow]
 765 |   writel(~CDNS_APB_INT_MASK_SW_EVENT_INT,
   In file included from include/linux/bits.h:23,
from include/linux/bitops.h:5,
from include/linux/kernel.h:12,
from include/linux/clk.h:13,
from drivers/gpu/drm/bridge/cdns-mhdp-core.c:10:
   drivers/gpu/drm/bridge/cdns-mhdp-core.c: In function 
'cdns_mhdp_link_training_init':
   include/linux/bits.h:26:28: warning: comparison of unsigned expression < 0 
is always false [-Wtype-limits]
  26 |   __builtin_constant_p((l) > (h)), (l) > (h), 0)))
 |^
   include/linux/build_bug.h:16:62: note: in definition of macro 
'BUILD_BUG_ON_ZERO'
  16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); 
})))
 |  ^
   include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
  39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
 |   ^~~
   drivers/gpu/drm/bridge/cdns-mhdp-core.h:116:35: note: in expansion of macro 
'GENMASK'
 116 | #define CDNS_DP_LANE_EN_LANES(x)  GENMASK((x) - 1, 0)
 |   ^~~
   drivers/gpu/drm/bridge/cdns-mhdp-core.c:888:8: note: in expansion of macro 
'CDNS_DP_LANE_EN_LANES'
 888 |CDNS_DP_LANE_EN_LANES(mhdp->link.num_lanes));
 |^
   include/linux/bits.h:26:40: warning: comparison of unsigned expression < 0 
is always false [-Wtype-limits]
  26 |   __builtin_constant_p((l) > (h)), (l) > (h), 0)))
 |^
   include/linux/build_bug.h:16:62: note: in definition of macro 
'BUILD_BUG_ON_ZERO'
  16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); 
})))
 |  ^
   include/linux/bits.h:39:3: note: in expansion of macro 'GENMASK_INPUT_CHECK'
  39 |  (GENMASK_INPUT_CHECK(h, l) + __GENMASK(h, l))
 |   ^~~
   drivers/gpu/drm/bridge/cdns-mhdp-core.h:116:35: note: in expansion of macro 
'GENMASK'
 116 | #define CDNS_DP_LANE_EN_LANES(x)  GENMASK((x) - 1, 0)
 |   ^~~
   drivers/gpu/drm/bridge/cdns-mhdp-core.c:888:8: note: in expansion of macro 
'CDNS_DP_LANE_EN_LANES'
 888 |CDNS_DP_LANE_EN_LANES(mhdp->link.num_lanes));
 |^
   drivers/gpu/drm/bridge/cdns-mhdp-core.c: In function 
'cdns_mhdp_fill_host_caps':
   drivers/gpu/drm/bridge/cdns-mhdp-core.c:1378:2: error: implicit declaration 
of function 'phy_get_attrs'; did you mean 'vfs_getattr'? 
[-Werror=implicit-function-declaration]
1378 |  phy_get_attrs(mhdp->phy, &attrs);
 |  ^
 |  vfs_getattr
   drivers/gpu/drm/bridge/cdns-mhdp-core.c:1384:19: error: 'struct phy_attrs' 
has no member named 'max_link_rate'
1384 |  link_rate = attrs.max_link_rate;
 |   ^
   drivers/gpu/drm/bridge/cdns-mhdp-core.c: In function 'cdns_mhdp_attach':
   drivers/gpu/drm/bridge/cdns-mhdp-core.c:1709:10: warning: conversion from 
'long unsigned int' to 'unsigned int' changes value from '18446744073709551613' 
to '4294967293' [-Woverflow]
1709 |   writel(~CDNS_APB_INT_MASK_SW_EVENT_INT,
   drivers/gpu/drm/bridge/cdns-mhdp-core.c: In function 
'cdns_mhdp_validate_mode_params'

Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE


Am 22.07.20 um 13:42 schrieb Daniel Vetter:

On Wed, Jul 22, 2020 at 1:13 PM Christian König
 wrote:

Am 22.07.20 um 07:34 schrieb Daniel Vetter:

On Tue, Jul 21, 2020 at 4:46 PM Christian König
 wrote:

Am 21.07.20 um 11:24 schrieb dan...@ffwll.ch:

On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:

The driver doesn't expose any not-mapable memory resources.

Signed-off-by: Christian König 
---
drivers/gpu/drm/radeon/radeon_ttm.c | 13 -
1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
b/drivers/gpu/drm/radeon/radeon_ttm.c
index 54af06df865b..b474781a0920 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
   switch (type) {
   case TTM_PL_SYSTEM:
   /* System memory */
-man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+man->flags = 0;
   man->available_caching = TTM_PL_MASK_CACHING;
   man->default_caching = TTM_PL_FLAG_CACHED;
   break;
@@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
   man->func = &ttm_bo_manager_func;
   man->available_caching = TTM_PL_MASK_CACHING;
   man->default_caching = TTM_PL_FLAG_CACHED;
-man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
+man->flags = 0;
#if IS_ENABLED(CONFIG_AGP)
   if (rdev->flags & RADEON_IS_AGP) {
   if (!rdev->ddev->agp) {
@@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
 (unsigned)type);
   return -EINVAL;
   }
-if (!rdev->ddev->agp->cant_use_aperture)
-man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;

There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with this
flag set. And radeon.ko did at least once work on these. And your patch to
disable agp only changes the default, it doesn't rip out the code.

The key pint is that the flags for AGP are the same as the one for the
PCIe path. So no functional change at all :)

I misread the code somehow, I didn't spot the unconditional setting of
FLAG_MAPPABLE for all TTM_PL_TT, irrespective of agp or not, somehow
thought that's another case.

Reviewed-by: Daniel Vetter 

And for the amdgpu patch? Otherwise I just ping Alex for an rb.

See my question over there, I'm not seeing how the code prevents mmap
for AMDGPU_PL_* domains after your patch. Once that's cleared up happy
to r-b that one and the final one too.


I already replied, sounds like you never got that.

Anyway see the switch just below the two lines I removed:

    switch (mem->mem_type) {
    case TTM_PL_SYSTEM:



    case TTM_PL_TT:

...

    case TTM_PL_VRAM:

...

    default:
    return -EINVAL;
    }


So again, no functional change at all.

Cheers,
Christian.


-Daniel


Thanks,
Christian.


The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().

Christian.


So not sure your assumption here is correct.
-Daniel


   man->available_caching = TTM_PL_FLAG_UNCACHED |
TTM_PL_FLAG_WC;
   man->default_caching = TTM_PL_FLAG_WC;
@@ -103,8 +101,7 @@ static int radeon_init_mem_type(struct ttm_bo_device *bdev, 
uint32_t type,
   case TTM_PL_VRAM:
   /* "On-card" video ram */
   man->func = &ttm_bo_manager_func;
-man->flags = TTM_MEMTYPE_FLAG_FIXED |
- TTM_MEMTYPE_FLAG_MAPPABLE;
+man->flags = TTM_MEMTYPE_FLAG_FIXED;
   man->available_caching = TTM_PL_FLAG_UNCACHED | TTM_PL_FLAG_WC;
   man->default_caching = TTM_PL_FLAG_WC;
   break;
@@ -394,7 +391,6 @@ static int radeon_bo_move(struct ttm_buffer_object *bo, 
bool evict,

static int radeon_ttm_io_mem_reserve(struct ttm_bo_device *bdev, struct 
ttm_mem_reg *mem)
{
-struct ttm_mem_type_manager *man = &bdev->man[mem->mem_type];
   struct radeon_device *rdev = radeon_get_rdev(bdev);

   mem->bus.addr = NULL;
@@ -402,8 +398,7 @@ static int radeon_ttm_io_mem_reserve(struct ttm_bo_device 
*bdev, struct ttm_mem_
   mem->bus.size = mem->num_pages << PAGE_SHIFT;
   mem->bus.base = 0;
   mem->bus.is_iomem = false;
-if (!(man->flags & TTM_MEMTYPE_FLAG_MAPPABLE))
-return -EINVAL;
+
   switch (mem->mem_type) {
   case TTM_PL_SYSTEM:
   /* system memory */
--
2.17.1

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&data=02%7C01%7Cchrist

Re: [PATCH v7 2/3] drm: bridge: Add support for Cadence MHDP DPI/DP bridge

Hi Swapnil,

Thank you for the patch.

On Wed, Jul 22, 2020 at 09:40:39AM +0200, Swapnil Jakhade wrote:
> Add a new DRM bridge driver for Cadence MHDP DPTX IP used in TI J721e SoC.
> MHDP DPTX IP is the component that complies with VESA DisplayPort (DP) and
> embedded Display Port (eDP) standards. It integrates uCPU running the
> embedded Firmware (FW) interfaced over APB interface.
> 
> Basically, it takes a DPI stream as input and outputs it encoded in DP
> format. Currently, it supports only SST mode.
> 
> Co-developed-by: Tomi Valkeinen 
> Signed-off-by: Tomi Valkeinen 
> Co-developed-by: Jyri Sarha 
> Signed-off-by: Jyri Sarha 
> Signed-off-by: Quentin Schulz 
> Signed-off-by: Yuti Amonkar 
> Signed-off-by: Swapnil Jakhade 
> ---
>  drivers/gpu/drm/bridge/Kconfig  |   11 +
>  drivers/gpu/drm/bridge/Makefile |2 +
>  drivers/gpu/drm/bridge/cdns-mhdp-core.c | 2493 +++
>  drivers/gpu/drm/bridge/cdns-mhdp-core.h |  396 
>  4 files changed, 2902 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-core.c
>  create mode 100644 drivers/gpu/drm/bridge/cdns-mhdp-core.h
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 43271c21d3fc..6a4c324302a8 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -27,6 +27,17 @@ config DRM_CDNS_DSI
> Support Cadence DPI to DSI bridge. This is an internal
> bridge and is meant to be directly embedded in a SoC.
>  
> +config DRM_CDNS_MHDP
> + tristate "Cadence DPI/DP bridge"
> + select DRM_KMS_HELPER
> + select DRM_PANEL_BRIDGE
> + depends on OF
> + help
> +   Support Cadence DPI to DP bridge. This is an internal
> +   bridge and is meant to be directly embedded in a SoC.
> +   It takes a DPI stream as input and outputs it encoded
> +   in DP format.
> +
>  config DRM_CHRONTEL_CH7033
>   tristate "Chrontel CH7033 Video Encoder"
>   depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index d63d4b7e4347..0080a9f80f29 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -1,5 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o
> +obj-$(CONFIG_DRM_CDNS_MHDP) += cdns-mhdp.o
> +cdns-mhdp-objs := cdns-mhdp-core.o
>  obj-$(CONFIG_DRM_CHRONTEL_CH7033) += chrontel-ch7033.o
>  obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o
>  obj-$(CONFIG_DRM_LVDS_CODEC) += lvds-codec.o
> diff --git a/drivers/gpu/drm/bridge/cdns-mhdp-core.c 
> b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
> new file mode 100644
> index ..b16c5503cef1
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/cdns-mhdp-core.c
> @@ -0,0 +1,2493 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Cadence MHDP DP bridge driver.
> + *
> + * Copyright: 2019 Cadence Design Systems, Inc.
> + *
> + * Author: Quentin Schulz 
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +#include "cdns-mhdp-core.h"
> +
> +static DECLARE_WAIT_QUEUE_HEAD(fw_load_wq);
> +
> +static int cdns_mhdp_mailbox_read(struct cdns_mhdp_device *mhdp)
> +{
> + int ret, empty;
> +
> + WARN_ON(!mutex_is_locked(&mhdp->mbox_mutex));
> +
> + ret = readx_poll_timeout(readl, mhdp->regs + CDNS_MAILBOX_EMPTY,
> +  empty, !empty, MAILBOX_RETRY_US,
> +  MAILBOX_TIMEOUT_US);
> + if (ret < 0)
> + return ret;
> +
> + return readl(mhdp->regs + CDNS_MAILBOX_RX_DATA) & 0xff;
> +}
> +
> +static int cdns_mhdp_mailbox_write(struct cdns_mhdp_device *mhdp, u8 val)
> +{
> + int ret, full;
> +
> + WARN_ON(!mutex_is_locked(&mhdp->mbox_mutex));
> +
> + ret = readx_poll_timeout(readl, mhdp->regs + CDNS_MAILBOX_FULL,
> +  full, !full, MAILBOX_RETRY_US,
> +  MAILBOX_TIMEOUT_US);
> + if (ret < 0)
> + return ret;
> +
> + writel(val, mhdp->regs + CDNS_MAILBOX_TX_DATA);
> +
> + return 0;
> +}
> +
> +static int cdns_mhdp_mailbox_validate_receive(struct cdns_mhdp_device *mhdp,
> +   u8 module_id, u8 opcode,
> +   u16 req_size)
> +{
> + u32 mbox_size, i;
> + u8 header[4];
> + int ret;
> +
> + /* read the header of the message */
> + for (i = 0; i < 4; i++) {
> + ret = cdns_mhdp_mailbox_read(mhdp);
> + if (ret < 0)
> + return ret;
> +
> + header[i] = ret;
> + }
> +
> + mbox_size = get_unaligned_be16(header + 2);
> +
> + if (opco

RE: [PATCH v2] io-mapping: Indicate mapping failure

2020-07-22 Thread Ruhl, Michael J

>-Original Message-
>From: Andrew Morton 
>Sent: Tuesday, July 21, 2020 5:24 PM
>To: Ruhl, Michael J 
>Cc: dri-devel@lists.freedesktop.org; Mike Rapoport ;
>Andy Shevchenko ; Chris Wilson
>; sta...@vger.kernel.org
>Subject: Re: [PATCH v2] io-mapping: Indicate mapping failure
>
>On Tue, 21 Jul 2020 21:02:44 + "Ruhl, Michael J"
> wrote:
>
>> >--- a/include/linux/io-mapping.h~io-mapping-indicate-mapping-failure-fix
>> >+++ a/include/linux/io-mapping.h
>> >@@ -107,9 +107,12 @@ io_mapping_init_wc(struct io_mapping *io
>> >   resource_size_t base,
>> >   unsigned long size)
>> > {
>> >+   iomap->iomem = ioremap_wc(base, size);
>> >+   if (!iomap->iomem)
>> >+   return NULL;
>> >+
>>
>> This does make more sense.
>>
>> I am confused by the two follow up emails I just got.
>
>One was your original patch, the other is my suggested alteration.
>
>> Shall I resubmit, or is this path (if !iomap->iomem) return NULL)
>> now in the tree.
>
>All is OK.  If my alteration is acceptable (and, preferably, tested!)
>then when the time comes, I'll fold it into the base patch, add a
>note indicating this change and shall then send it to Linus.

I am good with the change and have tested it.

Instead of the system crashing I get:

i915 :01:00.0: [drm] *ERROR* Failed to setup region(-5) type=1
i915 :01:00.0: Device initialization failed (-5)
i915: probe of :01:00.0 failed with error -5

Which is the expected error.

If you would like this for the updated patch:

Tested-By: Michael J. Ruhl 

Thanks!

Mike

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

Re: [PATCH 06/11] drm/radeon: stop using TTM_MEMTYPE_FLAG_MAPPABLE

On Wed, Jul 22, 2020 at 1:50 PM Christian König
 wrote:
>
> Am 22.07.20 um 13:42 schrieb Daniel Vetter:
> > On Wed, Jul 22, 2020 at 1:13 PM Christian König
> >  wrote:
> >> Am 22.07.20 um 07:34 schrieb Daniel Vetter:
> >>> On Tue, Jul 21, 2020 at 4:46 PM Christian König
> >>>  wrote:
>  Am 21.07.20 um 11:24 schrieb dan...@ffwll.ch:
> > On Tue, Jul 21, 2020 at 09:32:40AM +0200, Christian König wrote:
> >> The driver doesn't expose any not-mapable memory resources.
> >>
> >> Signed-off-by: Christian König 
> >> ---
> >> drivers/gpu/drm/radeon/radeon_ttm.c | 13 -
> >> 1 file changed, 4 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c 
> >> b/drivers/gpu/drm/radeon/radeon_ttm.c
> >> index 54af06df865b..b474781a0920 100644
> >> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> >> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> >> @@ -76,7 +76,7 @@ static int radeon_init_mem_type(struct ttm_bo_device 
> >> *bdev, uint32_t type,
> >>switch (type) {
> >>case TTM_PL_SYSTEM:
> >>/* System memory */
> >> -man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >> +man->flags = 0;
> >>man->available_caching = TTM_PL_MASK_CACHING;
> >>man->default_caching = TTM_PL_FLAG_CACHED;
> >>break;
> >> @@ -84,7 +84,7 @@ static int radeon_init_mem_type(struct ttm_bo_device 
> >> *bdev, uint32_t type,
> >>man->func = &ttm_bo_manager_func;
> >>man->available_caching = TTM_PL_MASK_CACHING;
> >>man->default_caching = TTM_PL_FLAG_CACHED;
> >> -man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> >> +man->flags = 0;
> >> #if IS_ENABLED(CONFIG_AGP)
> >>if (rdev->flags & RADEON_IS_AGP) {
> >>if (!rdev->ddev->agp) {
> >> @@ -92,8 +92,6 @@ static int radeon_init_mem_type(struct ttm_bo_device 
> >> *bdev, uint32_t type,
> >>  (unsigned)type);
> >>return -EINVAL;
> >>}
> >> -if (!rdev->ddev->agp->cant_use_aperture)
> >> -man->flags = TTM_MEMTYPE_FLAG_MAPPABLE;
> > There is a bunch of agp drivers (alpha, ppc, that kind of stuff) with 
> > this
> > flag set. And radeon.ko did at least once work on these. And your patch 
> > to
> > disable agp only changes the default, it doesn't rip out the code.
>  The key pint is that the flags for AGP are the same as the one for the
>  PCIe path. So no functional change at all :)
> >>> I misread the code somehow, I didn't spot the unconditional setting of
> >>> FLAG_MAPPABLE for all TTM_PL_TT, irrespective of agp or not, somehow
> >>> thought that's another case.
> >>>
> >>> Reviewed-by: Daniel Vetter 
> >> And for the amdgpu patch? Otherwise I just ping Alex for an rb.
> > See my question over there, I'm not seeing how the code prevents mmap
> > for AMDGPU_PL_* domains after your patch. Once that's cleared up happy
> > to r-b that one and the final one too.
>
> I already replied, sounds like you never got that.

I got it, but I suck at reading mailing lists.

> Anyway see the switch just below the two lines I removed:
> > switch (mem->mem_type) {
> > case TTM_PL_SYSTEM:
> 
> > case TTM_PL_TT:
> ...
> > case TTM_PL_VRAM:
> ...
> > default:
> > return -EINVAL;
> > }
>
> So again, no functional change at all.

Indeed, I score another point for being blind. r-b: also on the amdgpu
and final cleanup patch.
-Daniel

>
> Cheers,
> Christian.
>
> > -Daniel
> >
> >> Thanks,
> >> Christian.
> >>
>  The real handling of cant_use_aperture is in radeon_ttm_io_mem_reserve().
> 
>  Christian.
> 
> > So not sure your assumption here is correct.
> > -Daniel
> >
> >>man->available_caching = TTM_PL_FLAG_UNCACHED |
> >> TTM_PL_FLAG_WC;
> >>man->default_caching = TTM_PL_FLAG_WC;
> >> @@ -103,8 +101,7 @@ static int radeon_init_mem_type(struct 
> >> ttm_bo_device *bdev, uint32_t type,
> >>case TTM_PL_VRAM:
> >>/* "On-card" video ram */
> >>man->func = &ttm_bo_manager_func;
> >> -man->flags = TTM_MEMTYPE_FLAG_FIXED |
> >> - TTM_MEMTYPE_FLAG_MAPPABLE;
> >> +man->flags = TTM_MEMTYPE_FLAG_FIXED;
> >>man->available_caching = TTM_PL_FLAG_UNCACHED | 
> >> TTM_PL_FLAG_WC;
> >>man->default_caching = TTM_PL_FLAG_WC;
> >>break;
> >> @@ -394,7 +391,6 @@ static int radeon_bo_move(struct

Re: [PATCH] drm/vkms: add missing drm_crtc_vblank_put to the get/put pair on flush

2020-07-22 Thread daniel

On Wed, Jul 22, 2020 at 08:04:11AM -0300, Melissa Wen wrote:
> This patch adds a missing drm_crtc_vblank_put op to the pair
> drm_crtc_vblank_get/put (inc/decrement counter to guarantee vblanks).
> 
> It clears the execution of the following kms_cursor_crc subtests:
> 1. pipe-A-cursor-[size,alpha-opaque, NxN-(on-screen, off-screen, sliding,
>random, fast-moving])] - successful when running individually.
> 2. pipe-A-cursor-dpms passes again
> 3. pipe-A-cursor-suspend also passes
> 
> The issue was initially tracked in the sequential execution of IGT
> kms_cursor_crc subtest: when running the test sequence or one of its
> subtests twice, the odd execs complete and the pairs get stuck in an
> endless wait. In the IGT code, calling a wait_for_vblank before the start
> of CRC capture prevented the busy-wait. But the problem persisted in the
> pipe-A-cursor-dpms and -suspend subtests.
> 
> Checking the history, the pipe-A-cursor-dpms subtest was successful when,
> in vkms_atomic_commit_tail, instead of using the flip_done op, it used
> wait_for_vblanks. Another way to prevent blocking was wait_one_vblank when
> enabling crtc. However, in both cases, pipe-A-cursor-suspend persisted
> blocking in the 2nd start of CRC capture, which may indicate that
> something got stuck in the step of CRC setup. Indeed, wait_one_vblank in
> the crc setup was able to sync things and free all kms_cursor_crc
> subtests.
> 
> Tracing and comparing a clean run with a blocked one:
> - in a clean one, vkms_crtc_atomic_flush enables vblanks;
> - when blocked, only in next op, vkms_crtc_atomic_enable, the vblanks
> started. Moreover, a series of vkms_vblank_simulate flow out until
> disabling vblanks.
> Also watching the steps of vkms_crtc_atomic_flush, when the very first
> drm_crtc_vblank_get returned an error, the subtest crashed. On the other
> hand, when vblank_get succeeded, the subtest completed. Finally, checking
> the flush steps: it increases counter to hold a vblank reference (get),
> but there isn't a op to decreased it and release vblanks (put).
> 
> Cc: Daniel Vetter 
> Cc: Rodrigo Siqueira 
> Cc: Haneen Mohammed 
> Signed-off-by: Melissa Wen 
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c 
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index ac85e17428f8..a99d6b4a92dd 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -246,6 +246,7 @@ static void vkms_crtc_atomic_flush(struct drm_crtc *crtc,
>  
>   spin_unlock(&crtc->dev->event_lock);
>  
> + drm_crtc_vblank_put(crtc);

Uh so I reviewed this a bit more carefully now, and I dont think this is
the correct bugfix. From the kerneldoc of drm_crtc_arm_vblank_event():

 * Caller must hold a vblank reference for the event @e acquired by a
 * drm_crtc_vblank_get(), which will be dropped when the next vblank arrives.

So when we call drm_crtc_arm_vblank_event then the vblank_put gets called
for us. And that's the only case where we successfully acquired a vblank
interrupt reference since on failure of drm_crtc_vblank_get (0 indicates
success for that function, failure negative error number) we directly send
out the event.

So something else fishy is going on, and now I'm totally confused why this
even happens.

We also have a pile of WARN_ON checks in drm_crtc_vblank_put to make sure
we don't underflow the refcount, so it's also not that I think (except if
this patch creates more WARNING backtraces).

But clearly it changes behaviour somehow ... can you try to figure out
what changes? Maybe print out the vblank->refcount at various points in
the driver, and maybe also trace when exactly the fake vkms vblank hrtimer
is enabled/disabled ...

I'm totally confused about what's going on here now.
-Daniel

>   crtc->state->event = NULL;
>   }
>  
> -- 
> 2.27.0
> 

-- 
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] drm/bridge/adv7511: set the bridge type properly

Hi Laurentiu,

Thank you for the patch.

On Mon, Jul 20, 2020 at 03:42:27PM +0300, Laurentiu Palcu wrote:
> From: Laurentiu Palcu 
> 
> After the drm_bridge_connector_init() helper function has been added, the ADV
> driver has been changed accordingly. However, the 'type' field of the bridge
> structure was left unset, which makes the helper function always return 
> -EINVAL.
> 
> Signed-off-by: Laurentiu Palcu 

Reviewed-by: Laurent Pinchart 

> ---
> Hi,
> 
> I've hit this while trying to use this helper in the new i.MX8MQ DCSS
> driver, as suggested by Sam, and I wanted to test it with NWL MIPI_DSI and
> ADV since support is already in mainline.
> 
>  drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c 
> b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> index f45cdca9cce5..a0d392c338da 100644
> --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
> @@ -1283,6 +1283,7 @@ static int adv7511_probe(struct i2c_client *i2c, const 
> struct i2c_device_id *id)
>   adv7511->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID
>   | DRM_BRIDGE_OP_HPD;
>   adv7511->bridge.of_node = dev->of_node;
> + adv7511->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>  
>   drm_bridge_add(&adv7511->bridge);
>  

-- 
Regards,

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

Re: [Linaro-mm-sig] [PATCH 1/2] dma-buf.rst: Document why indefinite fences are a bad idea

2020-07-22 Thread Intel

On 2020-07-22 13:39, Daniel Vetter wrote:

On Wed, Jul 22, 2020 at 12:31 PM Thomas Hellström (Intel)
wrote:

On 2020-07-22 11:45, Daniel Vetter wrote:

On Wed, Jul 22, 2020 at 10:05 AM Thomas Hellström (Intel)
wrote:

On 2020-07-22 09:11, Daniel Vetter wrote:

On Wed, Jul 22, 2020 at 8:45 AM Thomas Hellström (Intel)
wrote:

On 2020-07-22 00:45, Dave Airlie wrote:

On Tue, 21 Jul 2020 at 18:47, Thomas Hellström (Intel)
wrote:

On 7/21/20 9:45 AM, Christian König wrote:

Am 21.07.20 um 09:41 schrieb Daniel Vetter:

On Mon, Jul 20, 2020 at 01:15:17PM +0200, Thomas Hellström (Intel)
wrote:

Hi,

On 7/9/20 2:33 PM, Daniel Vetter wrote:

Comes up every few years, gets somewhat tedious to discuss, let's
write this down once and for all.

I'm currently writing that up, but probably still need a few days for
this.

Great! I put down some (very) initial thoughts a couple of weeks ago
building on eviction fences for various hardware complexity levels here:

https://gitlab.freedesktop.org/thomash/docs/-/blob/master/Untangling%20dma-fence%20and%20memory%20allocation.odt

We are seeing HW that has recoverable GPU page faults but only for
compute tasks, and scheduler without semaphores hw for graphics.

So a single driver may have to expose both models to userspace and
also introduces the problem of how to interoperate between the two
models on one card.

Dave.

[PATCH] drm/amdgpu/dc: Simplify drm_crtc_state::active checks

2020-07-22 Thread Michel Dänzer

From: Michel Dänzer 

drm_atomic_crtc_check enforces that ::active can only be true if
::enable is as well.

Signed-off-by: Michel Dänzer 
---
 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c| 16 +++-
 1 file changed, 3 insertions(+), 13 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 312c543b258f..dabef307a74f 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -3415,21 +3415,12 @@ static bool modeset_required(struct drm_crtc_state 
*crtc_state,
 struct dc_stream_state *new_stream,
 struct dc_stream_state *old_stream)
 {
-   if (!drm_atomic_crtc_needs_modeset(crtc_state))
-   return false;
-
-   if (!crtc_state->enable)
-   return false;
-
-   return crtc_state->active;
+   return crtc_state->active && drm_atomic_crtc_needs_modeset(crtc_state);
 }
 
 static bool modereset_required(struct drm_crtc_state *crtc_state)
 {
-   if (!drm_atomic_crtc_needs_modeset(crtc_state))
-   return false;
-
-   return !crtc_state->enable || !crtc_state->active;
+   return !crtc_state->active && drm_atomic_crtc_needs_modeset(crtc_state);
 }
 
 static void amdgpu_dm_encoder_destroy(struct drm_encoder *encoder)
@@ -8108,8 +8099,7 @@ static int dm_update_crtc_state(struct 
amdgpu_display_manager *dm,
 * We want to do dc stream updates that do not require a
 * full modeset below.
 */
-   if (!(enable && aconnector && new_crtc_state->enable &&
- new_crtc_state->active))
+   if (!(enable && aconnector && new_crtc_state->active))
return 0;
/*
 * Given above conditions, the dc state cannot be NULL because:
-- 
2.28.0.rc0

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

Re: [PATCH v3 06/21] drm/bridge: tc358767: add drm_panel_bridge support

Hi Sam,

On Sun, Jul 19, 2020 at 03:06:56PM +0200, Sam Ravnborg wrote:
> On Sat, Jul 11, 2020 at 01:19:35AM +0300, Laurent Pinchart wrote:
> > On Fri, Jul 03, 2020 at 09:24:02PM +0200, Sam Ravnborg wrote:
> > > Prepare the bridge driver for use in a chained setup by
> > > replacing direct use of drm_panel with drm_panel_bridge support.
> > > 
> > > The bridge driver assume the panel is optional.
> > > The relevant tests are migrated over to check for the
> > > pnale bridge to keep the same functionality.
> > 
> > s/pnale/panel/
> > 
> > > Note: the bridge panel will use the connector type from the panel.
> > > 
> > > Signed-off-by: Sam Ravnborg 
> > > Cc: Andrzej Hajda 
> > > Cc: Neil Armstrong 
> > > Cc: Laurent Pinchart 
> > > Cc: Jonas Karlman 
> > > Cc: Jernej Skrabec 
> > > ---
> > >  drivers/gpu/drm/bridge/tc358767.c | 57 +++
> > >  1 file changed, 27 insertions(+), 30 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/bridge/tc358767.c 
> > > b/drivers/gpu/drm/bridge/tc358767.c
> > > index c2777b226c75..08d483664258 100644
> > > --- a/drivers/gpu/drm/bridge/tc358767.c
> > > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > > @@ -244,8 +244,8 @@ struct tc_data {
> > >   struct drm_dp_aux   aux;
> > >  
> > >   struct drm_bridge   bridge;
> > > + struct drm_bridge   *panel_bridge;
> > >   struct drm_connectorconnector;
> > > - struct drm_panel*panel;
> > >  
> > >   /* link settings */
> > >   struct tc_edp_link  link;
> > > @@ -1236,13 +1236,6 @@ static int tc_stream_disable(struct tc_data *tc)
> > >   return 0;
> > >  }
> > >  
> > > -static void tc_bridge_pre_enable(struct drm_bridge *bridge)
> > > -{
> > > - struct tc_data *tc = bridge_to_tc(bridge);
> > > -
> > > - drm_panel_prepare(tc->panel);
> > > -}
> > > -
> > >  static void tc_bridge_enable(struct drm_bridge *bridge)
> > >  {
> > >   struct tc_data *tc = bridge_to_tc(bridge);
> > > @@ -1266,8 +1259,6 @@ static void tc_bridge_enable(struct drm_bridge 
> > > *bridge)
> > >   tc_main_link_disable(tc);
> > >   return;
> > >   }
> > > -
> > > - drm_panel_enable(tc->panel);
> > >  }
> > >  
> > >  static void tc_bridge_disable(struct drm_bridge *bridge)
> > > @@ -1275,8 +1266,6 @@ static void tc_bridge_disable(struct drm_bridge 
> > > *bridge)
> > >   struct tc_data *tc = bridge_to_tc(bridge);
> > >   int ret;
> > >  
> > > - drm_panel_disable(tc->panel);
> > > -
> > >   ret = tc_stream_disable(tc);
> > >   if (ret < 0)
> > >   dev_err(tc->dev, "main link stream stop error: %d\n", ret);
> > > @@ -1286,13 +1275,6 @@ static void tc_bridge_disable(struct drm_bridge 
> > > *bridge)
> > >   dev_err(tc->dev, "main link disable error: %d\n", ret);
> > >  }
> > >  
> > > -static void tc_bridge_post_disable(struct drm_bridge *bridge)
> > > -{
> > > - struct tc_data *tc = bridge_to_tc(bridge);
> > > -
> > > - drm_panel_unprepare(tc->panel);
> > > -}
> > > -
> > >  static bool tc_bridge_mode_fixup(struct drm_bridge *bridge,
> > >const struct drm_display_mode *mode,
> > >struct drm_display_mode *adj)
> > > @@ -1348,9 +1330,11 @@ static int tc_connector_get_modes(struct 
> > > drm_connector *connector)
> > >   return 0;
> > >   }
> > >  
> > > - count = drm_panel_get_modes(tc->panel, connector);
> > > - if (count > 0)
> > > - return count;
> > > + if (tc->panel_bridge) {
> > > + count = drm_bridge_get_modes(tc->panel_bridge, connector);
> > > + if (count > 0)
> > > + return count;
> > > + }
> > >  
> > >   edid = drm_get_edid(connector, &tc->aux.ddc);
> > >  
> > > @@ -1378,7 +1362,7 @@ static enum drm_connector_status 
> > > tc_connector_detect(struct drm_connector *conne
> > >   int ret;
> > >  
> > >   if (tc->hpd_pin < 0) {
> > > - if (tc->panel)
> > > + if (tc->panel_bridge)
> > >   return connector_status_connected;
> > >   else
> > >   return connector_status_unknown;
> > > @@ -1413,6 +1397,13 @@ static int tc_bridge_attach(struct drm_bridge 
> > > *bridge,
> > >   struct drm_device *drm = bridge->dev;
> > >   int ret;
> > >  
> > > + if (tc->panel_bridge) {
> > > + ret = drm_bridge_attach(tc->bridge.encoder, tc->panel_bridge,
> > > + &tc->bridge, flags);
> > > + if (ret < 0)
> > > + return ret;
> > > + }
> > 
> > With this both this driver and the panel bridge driver will create a
> > connector. The simplest way to handle that is probably to pass
> > flags & ~DRM_BRIDGE_ATTACH_NO_CONNECTOR to drm_bridge_attach(). It's a
> > bit of a hack, but should go away once all users are converted to
> > !DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> 
> I do not follow you here - sorry.
> 
> We have two situations:
> 
> display driver creates the connector - and passes 
> DRM_BRIDGE_ATTACH_NO_CONNECTOR.
> - bridge driver shall not create connector
> - bridge panel shall

Re: [Linaro-mm-sig] [PATCH 1/2] dma-buf.rst: Document why indefinite fences are a bad idea