Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Daniel Vetter]

On Mon, Mar 26, 2018 at 6:36 PM, Noralf Trønnes  wrote:
> I you pick up the patch(es) and need to change something, you don't have
> to bother with retaining my authorship (but please cc me). Just claim it
> for your own and make it work.
> Less work for me when I get there (eventually).

Why can't be quite this flippant with copyright for upstream :-)

Either please put something like "Based on work by ..." into the
commit message (when it's a big rewrite) or just add your own s-o-b
line and a small note what you changed (if it's just a tiny change).

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Oleksandr Andrushchenko]

On 03/26/2018 07:36 PM, Noralf Trønnes wrote:


Den 26.03.2018 15.02, skrev Oleksandr Andrushchenko:

Hi, Noralf!

On 03/17/2018 04:40 PM, Noralf Trønnes wrote:


Den 16.03.2018 09.03, skrev Daniel Vetter:

On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter  wrote:

Hi Noralf,

On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:

This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
(fbdev) and tinydrm.

There are several changes in this version:

I've used Daniel's idea of protecting drm_device.unplugged with 
srcu to

provide race free drm_dev_unplug().

The fbdev helper unplug patch has become very small with Daniel's 
help.
Ref is now taken and dropped in the existing helpers, so I could 
drop

drm_fb_helper_simple_init().

I has annoyed me that fbdev is restored in drm_driver.last_close 
even if
fbdev isn't used. I've added a patch to fix that. This means I 
can drop
calling drm_atomic_helper_shutdown() in tinydrm_unregister(), 
since I
now can easily disable the pipeline from userspace by just 
closing the

users. Disabled pipeline means balanced regulator_enable/disable.

The 'Embed drm_device in tinydrm_device' patch has gained
drm_driver.release functions after a discussion with Laurent. My
previous version relied on obscure freeing in tinydrm_release().
This means that I didn't retain the ack's.

Laurent also caught an ugly devm_kmalloc() in
tinydrm_display_pipe_init() that I've fixed.
I'm practically packing for my 2 weeks of conference travel 
already, so
only very cursory read of the initial patches for core 
I think

this looks really splendid now.

But I won't have time for review for the next few week, would be 
good if
you could drag some others into this discussions. Iirc there's 
recently
been a few different people interested in udl (even some patches I 
think),

they might be able to help out with testing

Also, would be great if you can submit this to intel-gfx on the next
round, so that our CI can pick it up and give it a solid beating. 
Touching

critical core paths like in patch 1 is always a bit scary.

While reviewing xen-front's hotunplug handling I just realized this
never landed. Can you pls resend and nag me to review it properly? I'd
really like to get the drm_dev_unplug stuff sorted out better.


My plan was to pick this up after switching tinydrm over to vmalloc 
buffers,

but that work is now waiting for the generic fbdev emulation to land.

I'm currently wandering around inside drm_fb_helper looking for a 
way out,
I can feel the draft so there has to be an exit somewhere. I hope 
that in

a week or two I'm done with the next version of the RFC using the
drm_fb_helper mode setting code instead of the ioctl's.

At that point it will be a good thing to flush my "caches" of the
drm_fb_helper code, since after looking at it for a long time, I really
don't see the details anymore. So I'll pick up the unplug series 
then, at
least the core patches should be trivial to review and get merged if 
the CI

agrees.

Could you please estimate when unplug series can be on review?
I am doing some unplug work in my PV DRM driver and would like
to understand if it is feasible to wait for you or post patches as 
they are

and then plumb in drm_dev_{enter|exit} later after your work is merged



Ok, so I have looked at the patches and some work I have lying around.
As things stand now I see an opportunity to move some stuff from tinydrm
into drm_simple_kms_helper as part of adding unplug support to tinydrm.
This also depends on the generic fbdev emulation I'm working on.
This all means that it won't be some trivial tweaking to the unplug
patchset before resending it. It will take some time.

My suggestion is that you just add the core unplug patch as part of your
driver submission instead of waiting for me.

drm: Use srcu to protect drm_device.unplugged
https://patchwork.freedesktop.org/patch/175779/


This is exactly the patch I need. Daniel, could you please
review this single patch?

I believe this patch should be good to go as-is. Please CC
intel-...@lists.freedesktop.org if you do so to have the Intel CI 
verify it.



I will

As for drm_fb_helper unplug support:

drm/fb-helper: Support device unplug
https://patchwork.freedesktop.org/patch/175785/

I'm not as confident in this one since I'm not sure that those
drm_dev_is_unplugged() checks are really necessary. The driver has to do
the check anyways. But this patch isn't necessary for you to do most of
your driver side unplug protection though. You can do testing without
fbdev enabled and let me to care of this when I get around to it.

I you pick up the patch(es) and need to change something, you don't have
to bother with retaining my authorship 

I will retain

(but please cc me).

Ok

Just claim it
for your own and make it work.
Less work for me when I get there (eventually).

Noralf.


Thank you,
Oleksandr

Thank you,
Oleksandr


Noralf.


Thanks, Daniel


Thanks, Daniel


Noralf.

Noralf 

RE: [PATCH 3/5] drm/ttm: remove the backing store if no placement is given

[He, Roger]

Acked-by: Roger He 

-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Sunday, March 25, 2018 6:58 PM
To: linaro-mm-...@lists.linaro.org; linux-me...@vger.kernel.org; 
dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; 
sumit.sem...@linaro.org
Subject: [PATCH 3/5] drm/ttm: remove the backing store if no placement is given

Pipeline removal of the BOs backing store when the placement is given during 
validation.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 
98e06f8bf23b..17e821f01d0a 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1078,6 +1078,18 @@ int ttm_bo_validate(struct ttm_buffer_object *bo,
uint32_t new_flags;
 
reservation_object_assert_held(bo->resv);
+
+   /*
+* Remove the backing store if no placement is given.
+*/
+   if (!placement->num_placement && !placement->num_busy_placement) {
+   ret = ttm_bo_pipeline_gutting(bo);
+   if (ret)
+   return ret;
+
+   return ttm_tt_create(bo, false);
+   }
+
/*
 * Check whether we need to move buffer.
 */
--
2.14.1

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

RE: [PATCH 2/5] drm/ttm: keep a reference to transfer pipelined BOs

[He, Roger]

Reviewed-by: Roger He 

-Original Message-
From: amd-gfx [mailto:amd-gfx-boun...@lists.freedesktop.org] On Behalf Of 
Christian K?nig
Sent: Friday, March 16, 2018 9:21 PM
To: linaro-mm-...@lists.linaro.org; linux-me...@vger.kernel.org; 
dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org
Subject: [PATCH 2/5] drm/ttm: keep a reference to transfer pipelined BOs

Make sure the transfered BO is never destroy before the transfer BO.

Signed-off-by: Christian König 
---
 drivers/gpu/drm/ttm/ttm_bo_util.c | 50 +++
 1 file changed, 30 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c 
b/drivers/gpu/drm/ttm/ttm_bo_util.c
index 1f730b3f18e5..1ee20558ee31 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_util.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
@@ -39,6 +39,11 @@
 #include 
 #include 
 
+struct ttm_transfer_obj {
+   struct ttm_buffer_object base;
+   struct ttm_buffer_object *bo;
+};
+
 void ttm_bo_free_old_node(struct ttm_buffer_object *bo)  {
ttm_bo_mem_put(bo, >mem);
@@ -435,7 +440,11 @@ EXPORT_SYMBOL(ttm_bo_move_memcpy);
 
 static void ttm_transfered_destroy(struct ttm_buffer_object *bo)  {
-   kfree(bo);
+   struct ttm_transfer_obj *fbo;
+
+   fbo = container_of(bo, struct ttm_transfer_obj, base);
+   ttm_bo_unref(>bo);
+   kfree(fbo);
 }
 
 /**
@@ -456,14 +465,15 @@ static void ttm_transfered_destroy(struct 
ttm_buffer_object *bo)  static int ttm_buffer_object_transfer(struct 
ttm_buffer_object *bo,
  struct ttm_buffer_object **new_obj)  {
-   struct ttm_buffer_object *fbo;
+   struct ttm_transfer_obj *fbo;
int ret;
 
fbo = kmalloc(sizeof(*fbo), GFP_KERNEL);
if (!fbo)
return -ENOMEM;
 
-   *fbo = *bo;
+   fbo->base = *bo;
+   fbo->bo = ttm_bo_reference(bo);
 
/**
 * Fix up members that we shouldn't copy directly:
@@ -471,25 +481,25 @@ static int ttm_buffer_object_transfer(struct 
ttm_buffer_object *bo,
 */
 
atomic_inc(>bdev->glob->bo_count);
-   INIT_LIST_HEAD(>ddestroy);
-   INIT_LIST_HEAD(>lru);
-   INIT_LIST_HEAD(>swap);
-   INIT_LIST_HEAD(>io_reserve_lru);
-   mutex_init(>wu_mutex);
-   fbo->moving = NULL;
-   drm_vma_node_reset(>vma_node);
-   atomic_set(>cpu_writers, 0);
-
-   kref_init(>list_kref);
-   kref_init(>kref);
-   fbo->destroy = _transfered_destroy;
-   fbo->acc_size = 0;
-   fbo->resv = >ttm_resv;
-   reservation_object_init(fbo->resv);
-   ret = reservation_object_trylock(fbo->resv);
+   INIT_LIST_HEAD(>base.ddestroy);
+   INIT_LIST_HEAD(>base.lru);
+   INIT_LIST_HEAD(>base.swap);
+   INIT_LIST_HEAD(>base.io_reserve_lru);
+   mutex_init(>base.wu_mutex);
+   fbo->base.moving = NULL;
+   drm_vma_node_reset(>base.vma_node);
+   atomic_set(>base.cpu_writers, 0);
+
+   kref_init(>base.list_kref);
+   kref_init(>base.kref);
+   fbo->base.destroy = _transfered_destroy;
+   fbo->base.acc_size = 0;
+   fbo->base.resv = >base.ttm_resv;
+   reservation_object_init(fbo->base.resv);
+   ret = reservation_object_trylock(fbo->base.resv);
WARN_ON(!ret);
 
-   *new_obj = fbo;
+   *new_obj = >base;
return 0;
 }
 
--
2.14.1

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

[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105733

--- Comment #3 from Allan  ---
Updating, now this error appears in dmesg too :

```
[ 1502.683100] Chrome_~dThread[2218]: segfault at 0 ip 7f53a4452bd3 sp
7f53a0899ad0 error 6 in libxul.so[7f53a3f3e000+4e2a000]
[ 1502.689186] Chrome_~dThread[2694]: segfault at 0 ip 7f2ef4552bd3 sp
7f2ef0999ad0 error 6 in libxul.so[7f2ef403e000+4e2a000]
[ 1502.689275] Chrome_~dThread[2300]: segfault at 0 ip 7fc55ad52bd3 sp
7fc557199ad0 error 6 in libxul.so[7fc55a83e000+4e2a000]
[ 1502.689287] Chrome_~dThread[2781]: segfault at 0 ip 7f2ce4852bd3 sp
7f2ce0c99ad0 error 6 in libxul.so[7f2ce433e000+4e2a000]
```

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

[Bug 104412] RX 460 HDMI 4k 60fps not working, DisplayPort is.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=104412

--- Comment #12 from Alfe  ---
(In reply to S.H. from comment #6)
> Hello,
> Last night I was able to get my hands on a Gigabyte RX460 and did some
> testing.
> Booting and normal startup with the Gigabyte also only used 4k@30Hz but I
> was able to reach 4k@60Hz by adding a modeline via xrandr.
> With the Saphhire I am not able to get 4k@60Hz even with a custom modeline
> set via xrandr.
> Something seems broken here and I don't think both vendors messed up their
> cards.

Hi!  Since I'm also trying around with this, it would be really helpful if you
shared your modeline with which you were able to achieve 4k@60Hz ;-)

Cheers, Alfe

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

[Bug 100069] Dirt: Showdown bad performance with enabled advanced lightning

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100069

--- Comment #7 from Marek Olšák  ---
The low performance is a known issue. We don't know if it's expected or not.

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

[Bug 100069] Dirt: Showdown bad performance with enabled advanced lightning

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=100069

--- Comment #6 from Marek Olšák  ---
(In reply to Gregor Münch from comment #4)
> Created attachment 137129 [details]
> Ingame
> 
> Looks like this is one of the last games which have pretty garbled graphics
> with mesa.
> What is needed to fix this?

What graphics settings did you use in the game?

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

[PATCH v2 03/10] drm/i915/psr: Nuke aux frame sync

[José Roberto de Souza]

eDP spec states that aux frame is required to do PSR2 selective
update but i915 don't fully implement it. It sends the aux frame
sync messages but the value is always zero as the GTC is not enabled
in driver.

Through tests was findout that pannels can do selective update when
the y-coordinate is also included in SDP, that is why it is required
to run PSR2 in i915.

A dummy value is not useful at all to sink, so removing everything
related to aux frame sync, if GTC is enabled we can bring this back.

Cc: Vathsala Nagaraju 
Acked-by: Rodrigo Vivi 
Reviewed-by: Dhinakaran Pandiyan 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 -
 drivers/gpu/drm/i915/intel_psr.c | 24 +---
 2 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 800230ba1c3b..fade9029b6f5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -603,7 +603,6 @@ struct i915_psr {
struct delayed_work work;
unsigned busy_frontbuffer_bits;
bool psr2_support;
-   bool aux_frame_sync;
bool link_standby;
bool y_cord_support;
bool colorimetry_support;
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b8e083e10029..c0a6f63b586f 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -137,16 +137,9 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 
if (INTEL_GEN(dev_priv) >= 9 &&
(intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
-   uint8_t frame_sync_cap;
 
dev_priv->psr.sink_support = true;
-   if (drm_dp_dpcd_readb(_dp->aux,
- DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
- _sync_cap) != 1)
-   frame_sync_cap = 0;
-   dev_priv->psr.aux_frame_sync = frame_sync_cap & 
DP_AUX_FRAME_SYNC_CAP;
-   /* PSR2 needs frame sync as well */
-   dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
+   dev_priv->psr.psr2_support = true;
DRM_DEBUG_KMS("PSR2 %s on sink",
  dev_priv->psr.psr2_support ? "supported" : "not 
supported");
 
@@ -268,12 +261,6 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
struct drm_device *dev = dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
 
-
-   /* Enable AUX frame sync at sink */
-   if (dev_priv->psr.aux_frame_sync)
-   drm_dp_dpcd_writeb(_dp->aux,
-   DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
-   DP_AUX_FRAME_SYNC_ENABLE);
/* Enable ALPM at sink for psr2 */
if (dev_priv->psr.psr2_support && dev_priv->psr.alpm)
drm_dp_dpcd_writeb(_dp->aux,
@@ -712,11 +699,6 @@ static void hsw_psr_disable(struct intel_dp *intel_dp,
i915_reg_t psr_status;
u32 psr_status_mask;
 
-   if (dev_priv->psr.aux_frame_sync)
-   drm_dp_dpcd_writeb(_dp->aux,
-   DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
-   0);
-
if (dev_priv->psr.psr2_support) {
psr_status = EDP_PSR2_STATUS;
psr_status_mask = EDP_PSR2_STATUS_STATE_MASK;
@@ -860,10 +842,6 @@ static void intel_psr_exit(struct drm_i915_private 
*dev_priv)
return;
 
if (HAS_DDI(dev_priv)) {
-   if (dev_priv->psr.aux_frame_sync)
-   drm_dp_dpcd_writeb(_dp->aux,
-   DP_SINK_DEVICE_AUX_FRAME_SYNC_CONF,
-   0);
if (dev_priv->psr.psr2_support) {
val = I915_READ(EDP_PSR2_CTL);
WARN_ON(!(val & EDP_PSR2_ENABLE));
-- 
2.16.2

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

[PATCH v2 04/10] drm/i915/psr: Tie PSR2 support to Y coordinate requirement

[José Roberto de Souza]

Although i915 don't implement aux sync frame through tests was
findout that pannels can do selective update when the y-coordinate
is also included in SDP, that is why it is required to run PSR2 in
i915.

So moving to only one place the sink requirements that the actual
driver needs to enable PSR2.

Also intel_psr2_config_valid() is called every time the crtc config
is computed, wasting some time every time it was checking for
Y coordinate requirement.

This allow us to nuke y_cord_support and some of VSC setup code that
was handling a scenario that would never happen(PSR2 without Y
coordinate).

Also here renaming intel_dp_get_y_cord_status() to
intel_dp_get_y_coord_required() as it more accurate to the name and
function of bit according to eDP spec.

Cc: Rodrigo Vivi 
Reviewed-by: Dhinakaran Pandiyan 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 -
 drivers/gpu/drm/i915/intel_psr.c | 46 +---
 2 files changed, 19 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index fade9029b6f5..92cf6f4e9e00 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -604,7 +604,6 @@ struct i915_psr {
unsigned busy_frontbuffer_bits;
bool psr2_support;
bool link_standby;
-   bool y_cord_support;
bool colorimetry_support;
bool alpm;
bool has_hw_tracking;
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c0a6f63b586f..fb2d0fe7106b 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -93,7 +93,7 @@ static void psr_aux_io_power_put(struct intel_dp *intel_dp)
intel_display_power_put(dev_priv, psr_aux_domain(intel_dp));
 }
 
-static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
+static bool intel_dp_get_y_coord_required(struct intel_dp *intel_dp)
 {
uint8_t psr_caps = 0;
 
@@ -130,22 +130,29 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
drm_dp_dpcd_read(_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
 sizeof(intel_dp->psr_dpcd));
 
-   if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
+   if (intel_dp->psr_dpcd[0]) {
dev_priv->psr.sink_support = true;
DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
}
 
if (INTEL_GEN(dev_priv) >= 9 &&
-   (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
-
-   dev_priv->psr.sink_support = true;
-   dev_priv->psr.psr2_support = true;
+   (intel_dp->psr_dpcd[0] == DP_PSR2_WITH_Y_COORD_IS_SUPPORTED)) {
+   /*
+* All panels that supports PSR version 03h (PSR2 +
+* Y-coordinate) can handle Y-coordinates in VSC but we are
+* only sure that it is going to be used when required by the
+* panel. This way panel is capable to do selective update
+* without a aux frame sync.
+*
+* To support PSR version 02h and PSR version 03h without
+* Y-coordinate requirement panels we would need to enable
+* GTC first.
+*/
+   dev_priv->psr.psr2_support = 
intel_dp_get_y_coord_required(intel_dp);
DRM_DEBUG_KMS("PSR2 %s on sink",
  dev_priv->psr.psr2_support ? "supported" : "not 
supported");
 
if (dev_priv->psr.psr2_support) {
-   dev_priv->psr.y_cord_support =
-   intel_dp_get_y_cord_status(intel_dp);
dev_priv->psr.colorimetry_support =
intel_dp_get_colorimetry_status(intel_dp);
dev_priv->psr.alpm =
@@ -191,16 +198,12 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
memset(_vsc, 0, sizeof(psr_vsc));
psr_vsc.sdp_header.HB0 = 0;
psr_vsc.sdp_header.HB1 = 0x7;
-   if (dev_priv->psr.colorimetry_support &&
-   dev_priv->psr.y_cord_support) {
+   if (dev_priv->psr.colorimetry_support) {
psr_vsc.sdp_header.HB2 = 0x5;
psr_vsc.sdp_header.HB3 = 0x13;
-   } else if (dev_priv->psr.y_cord_support) {
+   } else {
psr_vsc.sdp_header.HB2 = 0x4;
psr_vsc.sdp_header.HB3 = 0xe;
-   } else {
-   psr_vsc.sdp_header.HB2 = 0x3;
-   psr_vsc.sdp_header.HB3 = 0xc;
}
} else {
/* Prepare VSC packet as per EDP 1.3 spec, Table 3.10 */
@@ -457,15 +460,6 @@ static bool intel_psr2_config_valid(struct intel_dp 
*intel_dp,
return false;
}
 
-   

[PATCH v2 06/10] drm/i915/psr: Do not override PSR2 sink support

[José Roberto de Souza]

Sink can support our PSR2 requirements but userspace can request
a resolution that PSR2 hardware do not support, in this case it
was overwritten the PSR2 sink support.
Adding another flag here, this way if requested resolution changed
to a value that PSR2 hardware can handle, PSR2 can be enabled.

Cc: Dhinakaran Pandiyan 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_debugfs.c |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h |  3 ++-
 drivers/gpu/drm/i915/intel_psr.c| 33 +
 3 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 7816cd53100a..16f9977995df 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2630,7 +2630,7 @@ static int i915_edp_psr_status(struct seq_file *m, void 
*data)
   yesno(work_busy(_priv->psr.work.work)));
 
if (HAS_DDI(dev_priv)) {
-   if (dev_priv->psr.psr2_support)
+   if (dev_priv->psr.psr2_enabled)
enabled = I915_READ(EDP_PSR2_CTL) & EDP_PSR2_ENABLE;
else
enabled = I915_READ(EDP_PSR_CTL) & EDP_PSR_ENABLE;
@@ -2678,7 +2678,7 @@ static int i915_edp_psr_status(struct seq_file *m, void 
*data)
 
seq_printf(m, "Performance_Counter: %u\n", psrperf);
}
-   if (dev_priv->psr.psr2_support) {
+   if (dev_priv->psr.psr2_enabled) {
u32 psr2 = I915_READ(EDP_PSR2_STATUS);
 
seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 92cf6f4e9e00..46cae097201c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -602,11 +602,12 @@ struct i915_psr {
bool active;
struct delayed_work work;
unsigned busy_frontbuffer_bits;
-   bool psr2_support;
+   bool sink_psr2_support;
bool link_standby;
bool colorimetry_support;
bool alpm;
bool has_hw_tracking;
+   bool psr2_enabled;
 
void (*enable_source)(struct intel_dp *,
  const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 84e1f8be5c48..5efddd920681 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -148,11 +148,12 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 * Y-coordinate requirement panels we would need to enable
 * GTC first.
 */
-   dev_priv->psr.psr2_support = 
intel_dp_get_y_coord_required(intel_dp);
-   DRM_DEBUG_KMS("PSR2 %s on sink",
- dev_priv->psr.psr2_support ? "supported" : "not 
supported");
+   dev_priv->psr.sink_psr2_support =
+   intel_dp_get_y_coord_required(intel_dp);
+   DRM_DEBUG_KMS("PSR2 %s on sink", dev_priv->psr.sink_psr2_support
+ ? "supported" : "not supported");
 
-   if (dev_priv->psr.psr2_support) {
+   if (dev_priv->psr.sink_psr2_support) {
dev_priv->psr.colorimetry_support =
intel_dp_get_colorimetry_status(intel_dp);
dev_priv->psr.alpm =
@@ -193,7 +194,7 @@ static void hsw_psr_setup_vsc(struct intel_dp *intel_dp,
struct drm_i915_private *dev_priv = 
to_i915(intel_dig_port->base.base.dev);
struct edp_vsc_psr psr_vsc;
 
-   if (dev_priv->psr.psr2_support) {
+   if (dev_priv->psr.psr2_enabled) {
/* Prepare VSC Header for SU as per EDP 1.4 spec, Table 6.11 */
memset(_vsc, 0, sizeof(psr_vsc));
psr_vsc.sdp_header.HB0 = 0;
@@ -265,7 +266,7 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
struct drm_i915_private *dev_priv = to_i915(dev);
 
/* Enable ALPM at sink for psr2 */
-   if (dev_priv->psr.psr2_support && dev_priv->psr.alpm)
+   if (dev_priv->psr.psr2_enabled && dev_priv->psr.alpm)
drm_dp_dpcd_writeb(_dp->aux,
DP_RECEIVER_ALPM_CONFIG,
DP_ALPM_ENABLE);
@@ -424,7 +425,7 @@ static void hsw_psr_activate(struct intel_dp *intel_dp)
 */
 
/* psr1 and psr2 are mutually exclusive.*/
-   if (dev_priv->psr.psr2_support)
+   if (dev_priv->psr.psr2_enabled)
hsw_activate_psr2(intel_dp);
else
hsw_activate_psr1(intel_dp);
@@ -444,7 +445,7 @@ static bool intel_psr2_config_valid(struct intel_dp 
*intel_dp,
 * dynamically during PSR enable, and extracted from sink
 * caps during eDP detection.
 */
-   if 

[PATCH v2 05/10] drm/i915/psr/cnl: Enable Y-coordinate support in source

[José Roberto de Souza]

From: "Souza, Jose" 

For Geminilake and Cannonlake+ the Y-coordinate support must be
enabled in PSR2_CTL too.

Spec: 7713 and 7720

Cc: Dhinakaran Pandiyan 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_reg.h  |  3 +++
 drivers/gpu/drm/i915/intel_psr.c | 16 
 2 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1bca695f404b..1c6f463bc919 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4052,6 +4052,8 @@ enum {
 #define EDP_PSR2_CTL   _MMIO(0x6f900)
 #define   EDP_PSR2_ENABLE  (1<<31)
 #define   EDP_SU_TRACK_ENABLE  (1<<30)
+#define   EDP_Y_COORDINATE_VALID   (1<<26) /* GLK and CNL+ */
+#define   EDP_Y_COORDINATE_ENABLE  (1<<25) /* GLK and CNL+ */
 #define   EDP_MAX_SU_DISABLE_TIME(t)   ((t)<<20)
 #define   EDP_MAX_SU_DISABLE_TIME_MASK (0x1f<<20)
 #define   EDP_PSR2_TP2_TIME_500(0<<8)
@@ -7058,6 +7060,7 @@ enum {
 #define CHICKEN_TRANS_A 0x420c0
 #define CHICKEN_TRANS_B 0x420c4
 #define CHICKEN_TRANS(trans) _MMIO_TRANS(trans, CHICKEN_TRANS_A, 
CHICKEN_TRANS_B)
+#define  VSC_DATA_SEL_SOFTWARE_CONTROL (1<<25) /* GLK and CNL+ */
 #define  DDI_TRAINING_OVERRIDE_ENABLE  (1<<19)
 #define  DDI_TRAINING_OVERRIDE_VALUE   (1<<18)
 #define  DDIE_TRAINING_OVERRIDE_ENABLE (1<<17) /* CHICKEN_TRANS_A only */
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index fb2d0fe7106b..84e1f8be5c48 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -386,8 +386,10 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
/* FIXME: selective update is probably totally broken because it doesn't
 * mesh at all with our frontbuffer tracking. And the hw alone isn't
 * good enough. */
-   val |= EDP_PSR2_ENABLE |
-   EDP_SU_TRACK_ENABLE;
+   val |= EDP_PSR2_ENABLE | EDP_SU_TRACK_ENABLE;
+   if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+   val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE;
+   }
 
if (drm_dp_dpcd_readb(_dp->aux,
DP_SYNCHRONIZATION_LATENCY_IN_SINK,
@@ -569,8 +571,14 @@ static void hsw_psr_enable_source(struct intel_dp 
*intel_dp,
hsw_psr_setup_aux(intel_dp);
 
if (dev_priv->psr.psr2_support) {
-   u32 chicken = PSR2_VSC_ENABLE_PROG_HEADER
- | PSR2_ADD_VERTICAL_LINE_COUNT;
+   u32 chicken = I915_READ(CHICKEN_TRANS(cpu_transcoder));
+
+   if (INTEL_GEN(dev_priv) == 9 && !IS_GEMINILAKE(dev_priv))
+   chicken |= (PSR2_VSC_ENABLE_PROG_HEADER
+  | PSR2_ADD_VERTICAL_LINE_COUNT);
+
+   else
+   chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
 
I915_WRITE(EDP_PSR_DEBUG,
-- 
2.16.2

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

[PATCH v2 09/10] drm/i915/psr: Set DPCD PSR2 enable bit when needed

[José Roberto de Souza]

In the 2 eDP1.4a pannels tested set or not set bit have no effect
but is better set it and comply with specification.

Signed-off-by: José Roberto de Souza 
Cc: Dhinakaran Pandiyan 
Reviewed-by: Rodrigo Vivi 
---
 drivers/gpu/drm/i915/intel_psr.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index d079cf0b034c..2d53f7398a6d 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -278,18 +278,19 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
struct intel_digital_port *dig_port = dp_to_dig_port(intel_dp);
struct drm_device *dev = dig_port->base.base.dev;
struct drm_i915_private *dev_priv = to_i915(dev);
+   u8 dpcd_val = DP_PSR_ENABLE;
 
/* Enable ALPM at sink for psr2 */
if (dev_priv->psr.psr2_enabled && dev_priv->psr.alpm)
drm_dp_dpcd_writeb(_dp->aux,
DP_RECEIVER_ALPM_CONFIG,
DP_ALPM_ENABLE);
+
+   if (dev_priv->psr.psr2_enabled)
+   dpcd_val |= DP_PSR_ENABLE_PSR2;
if (dev_priv->psr.link_standby)
-   drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG,
-  DP_PSR_ENABLE | DP_PSR_MAIN_LINK_ACTIVE);
-   else
-   drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG,
-  DP_PSR_ENABLE);
+   dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
+   drm_dp_dpcd_writeb(_dp->aux, DP_PSR_EN_CFG, dpcd_val);
 
drm_dp_dpcd_writeb(_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
 }
-- 
2.16.2

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

[PATCH v2 10/10] drm/i915/debugfs: Print sink PSR status

[José Roberto de Souza]

IGT tests could be improved with sink status, knowing for sure that
hardware have activate or exit PSR.

Cc: Dhinakaran Pandiyan 
Cc: Rodrigo Vivi 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_debugfs.c | 29 +
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 16f9977995df..91a8f70ffdd3 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2603,6 +2603,26 @@ static const char *psr2_live_status(u32 val)
return "unknown";
 }
 
+static const char *psr_sink_self_refresh_status(u8 val)
+{
+   static const char * const sink_status[] = {
+   "inactive",
+   "transitioning to active",
+   "active",
+   "active receiving selective update",
+   "transitioning to inactive",
+   "reserved",
+   "reserved",
+   "sink internal error"
+   };
+
+   val &= DP_PSR_SINK_STATE_MASK;
+   if (val < ARRAY_SIZE(sink_status))
+   return sink_status[val];
+
+   return "unknown";
+}
+
 static int i915_edp_psr_status(struct seq_file *m, void *data)
 {
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -2684,6 +2704,15 @@ static int i915_edp_psr_status(struct seq_file *m, void 
*data)
seq_printf(m, "EDP_PSR2_STATUS: %x [%s]\n",
   psr2, psr2_live_status(psr2));
}
+
+   if (dev_priv->psr.enabled) {
+   struct drm_dp_aux *aux = _priv->psr.enabled->aux;
+   u8 val;
+
+   if (drm_dp_dpcd_readb(aux, DP_PSR_STATUS, ) == 1)
+   seq_printf(m, "Sink self refresh status: 0x%x [%s]\n",
+  val, psr_sink_self_refresh_status(val));
+   }
mutex_unlock(_priv->psr.lock);
 
intel_runtime_pm_put(dev_priv);
-- 
2.16.2

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

[PATCH v2 01/10] drm: Add DP PSR2 sink enable bit

[José Roberto de Souza]

To comply with eDP1.4a this bit should be set when enabling PSR2.

Signed-off-by: José Roberto de Souza 
Reviewed-by: Rodrigo Vivi 
---
 include/drm/drm_dp_helper.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 62903bae0221..0bac0c7d0dec 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -478,6 +478,7 @@
 # define DP_PSR_FRAME_CAPTURE  (1 << 3)
 # define DP_PSR_SELECTIVE_UPDATE   (1 << 4)
 # define DP_PSR_IRQ_HPD_WITH_CRC_ERRORS (1 << 5)
+# define DP_PSR_ENABLE_PSR2(1 << 6) /* eDP 1.4a */
 
 #define DP_ADAPTER_CTRL0x1a0
 # define DP_ADAPTER_CTRL_FORCE_LOAD_SENSE   (1 << 0)
-- 
2.16.2

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

[PATCH v2 08/10] drm/i915/psr: Cache sink synchronization latency

[José Roberto de Souza]

This value do not change overtime so better cache it than
fetch it every PSR enable.

Cc: Dhinakaran Pandiyan 
Reviewed-by: Rodrigo Vivi 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 28 
 2 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 46cae097201c..5373b171bb96 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -608,6 +608,7 @@ struct i915_psr {
bool alpm;
bool has_hw_tracking;
bool psr2_enabled;
+   u8 sink_sync_latency;
 
void (*enable_source)(struct intel_dp *,
  const struct intel_crtc_state *);
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index bec455e28943..d079cf0b034c 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -122,6 +122,18 @@ static bool intel_dp_get_alpm_status(struct intel_dp 
*intel_dp)
return alpm_caps & DP_ALPM_CAP;
 }
 
+static u8 intel_dp_get_sink_sync_latency(struct intel_dp *intel_dp)
+{
+   u8 val = 0;
+
+   if (drm_dp_dpcd_readb(_dp->aux,
+ DP_SYNCHRONIZATION_LATENCY_IN_SINK, ) == 1)
+   val &= DP_MAX_RESYNC_FRAME_COUNT_MASK;
+   else
+   DRM_ERROR("Unable to get sink synchronization latency\n");
+   return val;
+}
+
 void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 {
struct drm_i915_private *dev_priv =
@@ -158,6 +170,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
intel_dp_get_colorimetry_status(intel_dp);
dev_priv->psr.alpm =
intel_dp_get_alpm_status(intel_dp);
+   dev_priv->psr.sink_sync_latency =
+   intel_dp_get_sink_sync_latency(intel_dp);
}
}
 }
@@ -379,10 +393,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 * with the 5 or 6 idle patterns.
 */
uint32_t idle_frames = max(6, dev_priv->vbt.psr.idle_frames);
-   uint32_t val;
-   uint8_t sink_latency;
-
-   val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
+   u32 val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
 
/* FIXME: selective update is probably totally broken because it doesn't
 * mesh at all with our frontbuffer tracking. And the hw alone isn't
@@ -392,14 +403,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
val |= EDP_Y_COORDINATE_VALID | EDP_Y_COORDINATE_ENABLE;
}
 
-   if (drm_dp_dpcd_readb(_dp->aux,
-   DP_SYNCHRONIZATION_LATENCY_IN_SINK,
-   _latency) == 1) {
-   sink_latency &= DP_MAX_RESYNC_FRAME_COUNT_MASK;
-   } else {
-   sink_latency = 0;
-   }
-   val |= EDP_PSR2_FRAME_BEFORE_SU(sink_latency + 1);
+   val |= EDP_PSR2_FRAME_BEFORE_SU(dev_priv->psr.sink_sync_latency + 1);
 
if (dev_priv->vbt.psr.tp2_tp3_wakeup_time > 5)
val |= EDP_PSR2_TP2_TIME_2500;
-- 
2.16.2

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

[PATCH v2 02/10] drm: Add DP last received PSR SDP VSC register and bits

[José Roberto de Souza]

This is a register to help debug what is in the last SDP VSC
packet revived by sink.

Signed-off-by: José Roberto de Souza 
Reviewed-by: Rodrigo Vivi 
---
 include/drm/drm_dp_helper.h | 9 +
 1 file changed, 9 insertions(+)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index 0bac0c7d0dec..91c9bcd4196f 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -795,6 +795,15 @@
 # define DP_LAST_ACTUAL_SYNCHRONIZATION_LATENCY_MASK   (0xf << 4)
 # define DP_LAST_ACTUAL_SYNCHRONIZATION_LATENCY_SHIFT  4
 
+#define DP_LAST_RECEIVED_PSR_SDP   0x200a /* eDP 1.2 */
+# define DP_PSR_STATE_BIT  (1 << 0) /* eDP 1.2 */
+# define DP_UPDATE_RFB_BIT (1 << 1) /* eDP 1.2 */
+# define DP_CRC_VALID_BIT  (1 << 2) /* eDP 1.2 */
+# define DP_SU_VALID   (1 << 3) /* eDP 1.4 */
+# define DP_FIRST_SCAN_LINE_SU_REGION  (1 << 4) /* eDP 1.4 */
+# define DP_LAST_SCAN_LINE_SU_REGION   (1 << 5) /* eDP 1.4 */
+# define DP_Y_COORDINATE_VALID (1 << 6) /* eDP 1.4a */
+
 #define DP_RECEIVER_ALPM_STATUS0x200b  /* eDP 1.4 */
 # define DP_ALPM_LOCK_TIMEOUT_ERROR(1 << 0)
 
-- 
2.16.2

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

[PATCH v2 07/10] drm/i915/psr: Use PSR2 macro for PSR2

[José Roberto de Souza]

Cosmetic change.

Reviewed-by: Rodrigo Vivi 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_reg.h  | 3 ++-
 drivers/gpu/drm/i915/intel_psr.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1c6f463bc919..219a4da284aa 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4063,8 +4063,9 @@ enum {
 #define   EDP_PSR2_TP2_TIME_MASK   (3<<8)
 #define   EDP_PSR2_FRAME_BEFORE_SU_SHIFT 4
 #define   EDP_PSR2_FRAME_BEFORE_SU_MASK(0xf<<4)
-#define   EDP_PSR2_IDLE_MASK   0xf
 #define   EDP_PSR2_FRAME_BEFORE_SU(a)  ((a)<<4)
+#define   EDP_PSR2_IDLE_FRAME_MASK 0xf
+#define   EDP_PSR2_IDLE_FRAME_SHIFT0
 
 #define EDP_PSR2_STATUS_MMIO(0x6f940)
 #define EDP_PSR2_STATUS_STATE_MASK (0xf<<28)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 5efddd920681..bec455e28943 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -382,7 +382,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
uint32_t val;
uint8_t sink_latency;
 
-   val = idle_frames << EDP_PSR_IDLE_FRAME_SHIFT;
+   val = idle_frames << EDP_PSR2_IDLE_FRAME_SHIFT;
 
/* FIXME: selective update is probably totally broken because it doesn't
 * mesh at all with our frontbuffer tracking. And the hw alone isn't
-- 
2.16.2

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

Re: [PATCH libdrm v2] libdrm: Use readdir instead of readdir_r to avoid build warnings

[John Stultz]

On Thu, Mar 22, 2018 at 9:51 AM, Emil Velikov  wrote:
> On 20 March 2018 at 18:17, Eric Engestrom  wrote:
>> On Tuesday, 2018-03-20 17:48:23 +, Emil Velikov wrote:
>>> From: John Stultz 
>>>
>>> Building libdrm under AOSP, we see the following build warning:
>>> external/libdrm/xf86drm.c:2861:12: warning: 'readdir_r' is deprecated: 
>>> readdir_r is deprecated; use readdir instead [-Wdeprecated-declarations]
>>> while (readdir_r(sysdir, pent, ) == 0 && ent != NULL) {
>>>^
>>>
>>> Building on Linux with glibc produces the same warning.
>>> Thus, this patch replaces readdir_r with readdir.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102031
>>> Cc: Robert Foss 
>>> Cc: Rob Herring 
>>> Cc: Stefan Schake 
>>> Cc: John Stultz 
>>> Cc: Eric Engestrom 
>>> Signed-off-by: John Stultz 
>>> Reviewed-by: Emil Velikov 
>>> [Emil Velikov: remove unused variables, Eric]
>>> Signed-off-by: Emil Velikov 
>>
>> I think that's pretty much exactly the patch I have at home :)
>> (and that I forgot to send out last night)
>>
>> Reviewed-by: Eric Engestrom 
>>
> Thanks pushed to master.

Sorry again for the slow response here, but an after-the-fact thanks
for sending out the much improved version!

I really appreciate it!
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 2/2] Revert "libdrm: intel/Android.mk: Filter libdrm_intel library requirements on x86/x86_64"

[John Stultz]

On Tue, Mar 20, 2018 at 10:36 AM, Emil Velikov  wrote:
> This reverts commit ed07718ae7bab596297abf210bb0c37c6dba58ed.
>
> The commit added a guard since libpciaccess may be missing on some
> setups. As of last commit there are no traces of the project, from
> Android POV.
>
> Hence, we can revert this workaround - which caused similar breakage to
> the one it's trying to fix. This time in Mesa.
>
> Cc: John Stultz 
> Cc: Rob Herring 
> Cc: John Stultz 

Acked-by: John Stultz 

Thanks so much for sending these out!
-john
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 1/2] intel: Do not use libpciaccess on Android

[John Stultz]

On Tue, Mar 20, 2018 at 10:36 AM, Emil Velikov  wrote:
> From: Tomasz Figa 
>
> This patch makes the code not rely anymore on libpciaccess when compiled
> for Android to eliminate ioperm() and iopl() syscalls required by that
> library. As a side effect, the mappable aperture size is hardcoded to 64
> MiB on Android, however nothing seems to rely on this value anyway, as
> checked be grepping relevant code in drm_gralloc and Mesa.
>
> Cc: John Stultz 
> Cc: Rob Herring 
> Cc: John Stultz 
> Cc: Tomasz Figa 
> Signed-off-by: Tomasz Figa 
> [Emil Velikov: rebase against master]
> Signed-off-by: Emil Velikov 
> ---
> Tomasz, I've taken the liberty of pulling the patch from the Android
> tree. Hope you don't mind.
> ---

Sorry, being abroad for a conference last week slowed me down and I
didn't get to validate this.

Many thanks for sending this out, I do agree its cleaner then my filtering fix.

I've got this patch (along with the __func__ fix EricE noticed) built
and tested for the HiKey boards and it works ok for me.

Feel free to add my:
Acked-by: John Stultz 

Emil: Do you mind respinning with the fix so Rob or someone can apply?

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

Re: [PATCH 1/7] dt-bindings: add cdtech vendor prefix

[Giulio Benetti]

Hi,

Il 27/03/2018 00:24, Rob Herring ha scritto:

On Wed, Mar 21, 2018 at 09:03:07PM +0100, Giulio Benetti wrote:

This adds a vendor prefix "cdtech" for CDTech(H.K.) Electronics Limited


Would be good to have website and/or info about what this company does.


Do you mean to have it in commit log?
I ask you because I can resubmit this patch with the others since I have 
to do v2 patchset.


Thanks

--
Giulio Benetti
CTO

MICRONOVA SRL
Sede: Via A. Niedda 3 - 35010 Vigonza (PD)
Tel. 049/8931563 - Fax 049/8931346
Cod.Fiscale - P.IVA 02663420285
Capitale Sociale € 26.000 i.v.
Iscritta al Reg. Imprese di Padova N. 02663420285
Numero R.E.A. 258642





Signed-off-by: Giulio Benetti 
---
  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
  1 file changed, 1 insertion(+)


In any case,

Reviewed-by: Rob Herring 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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

Re: [PATCHv2 1/3] dt-bindings: display: dw_hdmi.txt: add cec-disable property

[Rob Herring]

On Fri, Mar 23, 2018 at 01:59:13PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Some boards have both a DesignWare and their own CEC controller.
> The CEC pin is only hooked up to their own CEC controller and not
> to the DW controller.
> 
> Add the cec-disable property to disable the DW CEC controller.
> 
> This particular situation happens on Amlogic boards that have their
> own meson CEC controller.

Seems like we could avoid this by describing how the CEC line is hooked 
up which could be needed for other reasons.

> 
> Signed-off-by: Hans Verkuil 
> Acked-by: Neil Armstrong 
> ---
>  Documentation/devicetree/bindings/display/bridge/dw_hdmi.txt | 3 +++
>  1 file changed, 3 insertions(+)
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/7] dt-bindings: add micronova vendor prefix

[Rob Herring]

On Wed, Mar 21, 2018 at 09:03:11PM +0100, Giulio Benetti wrote:
> This adds a vendor prefix "micronova" for Micronova srl
> 
> Signed-off-by: Giulio Benetti 
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

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

Re: [PATCH 2/7] drm/panel: add panel CDTech S070_WV95_CT16 to panel-simple

[Rob Herring]

On Wed, Mar 21, 2018 at 09:03:08PM +0100, Giulio Benetti wrote:
> Signed-off-by: Giulio Benetti 
> ---
>  .../display/panel/cdtech,s070wv95-ct16.txt |  7 ++
>  drivers/gpu/drm/panel/panel-simple.c   | 27 
> ++
>  2 files changed, 34 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/cdtech,s070wv95-ct16.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/cdtech,s070wv95-ct16.txt 
> b/Documentation/devicetree/bindings/display/panel/cdtech,s070wv95-ct16.txt
> new file mode 100644
> index 000..b00d6c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/cdtech,s070wv95-ct16.txt
> @@ -0,0 +1,7 @@
> +CDTech(H.K.) Electronics Limited 7" 800x480 color TFT-LCD panel
> +
> +Required properties:
> +- compatible: should be "cdtech,s070wv95-ct16"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.

Please be specific as to what properties apply. In particular, are you 
not describing the power supply and it has multiple supplies or the 
panel has a single supply?

Rob

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

Re: [PATCH v2 1/2] dt-bindings: arm: omap: dmm: Document new compatible for DRA7xx family

[Rob Herring]

On Thu, Mar 22, 2018 at 03:42:05PM +0200, Peter Ujfalusi wrote:
> From: Tomi Valkeinen 
> 
> Define unique compatible string for the DMM in DRA7xx family.
> 
> The new compatible can be used to apply DRA7xx specific workarounds for
> ERRATAs, like i878 (MPU Lockup with concurrent DMM and EMIF accesses)
> 
> Signed-off-by: Tomi Valkeinen 
> ---
>  Documentation/devicetree/bindings/arm/omap/dmm.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Rob Herring 

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

Re: [PATCH 3/7] drm/panel: add panel CDTech S043WQ26H-CT7 to panel-simple

[Rob Herring]

On Wed, Mar 21, 2018 at 09:03:09PM +0100, Giulio Benetti wrote:
> Signed-off-by: Giulio Benetti 
> ---
>  .../display/panel/cdtech,s043wq26h-ct7.txt |  7 ++
>  drivers/gpu/drm/panel/panel-simple.c   | 28 
> ++
>  2 files changed, 35 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/cdtech,s043wq26h-ct7.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/cdtech,s043wq26h-ct7.txt 
> b/Documentation/devicetree/bindings/display/panel/cdtech,s043wq26h-ct7.txt
> new file mode 100644
> index 000..a22af85
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/panel/cdtech,s043wq26h-ct7.txt
> @@ -0,0 +1,7 @@
> +CDTech(H.K.) Electronics Limited 4.3" 480x272 color TFT-LCD panel
> +
> +Required properties:
> +- compatible: should be "cdtech,s043wq26h-ct7"
> +
> +This binding is compatible with the simple-panel binding, which is specified
> +in simple-panel.txt in this directory.

Same comment here.

Also, please split bindings to separate patches.

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

Re: [PATCH 1/7] dt-bindings: add cdtech vendor prefix

[Rob Herring]

On Wed, Mar 21, 2018 at 09:03:07PM +0100, Giulio Benetti wrote:
> This adds a vendor prefix "cdtech" for CDTech(H.K.) Electronics Limited

Would be good to have website and/or info about what this company does.

> 
> Signed-off-by: Giulio Benetti 
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

In any case,

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

Re: [PATCH 6/7] ARM: dts: sun7i: Add dts file for the A20-linova1-4_3 HMI

[Rob Herring]

On Wed, Mar 21, 2018 at 09:03:12PM +0100, Giulio Benetti wrote:
> The A20-Linova1-4_3 HMI, also called Q027_2_A which is printed on
> production label, is an industrial Human Machine Interface.
> It features:
> - 512MB DDR RAM
> - 1 Sd-card >= 4GB
> - 1 Usb otg(programmable via software) with A-Usb Connector
> - 1 Usb host
> - 1 Buzzer
> - 1 Input for LiPo
> - 1 Relay to signal absence of power supply
> - 1 External Rtc with 56 bytes of ram + CR2032 battery
> - 1 4.3" 24-bits Tft 480x272 with PCap on
> - 1 Mono audio 1-watt amplifier
> - 1 RS485 port
> - 1 Power On Line through +12Vdc reaching 57.600baud,
>   from where it can be supplied and placed in a network of 50 units
> - exposed jtag pins
> 
> HMI is supplied from +12Vdc.
> Ethernet is absent, so for debugging, need to enable rndis on Usb otg
> port through an A-A usb cable.
> It comes in different flavours for connector types and can be found with
> umounted features as requested by customers.
> 
> Signed-off-by: Giulio Benetti 
> ---
>  .../devicetree/bindings/arm/micronova.txt  |   6 +
>  arch/arm/boot/dts/Makefile |   1 +
>  arch/arm/boot/dts/sun7i-a20-linova1-ctp-4_3i.dts   | 192 
> +
>  3 files changed, 199 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/micronova.txt
>  create mode 100644 arch/arm/boot/dts/sun7i-a20-linova1-ctp-4_3i.dts
> 
> diff --git a/Documentation/devicetree/bindings/arm/micronova.txt 
> b/Documentation/devicetree/bindings/arm/micronova.txt
> new file mode 100644
> index 000..35c4127
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/micronova.txt
> @@ -0,0 +1,6 @@
> +Micronova Device Tree Bindings
> +---
> +
> +A20-LiNova1-4_3 HMI
> +Required root node properties:
> +- compatible = "micronova,a20-linova1-ctp-4_3i", "allwinner,sun7i-a20";

I'd prefer that board compatibles are documented where the SoC 
compatibles are documented, but it seems mostly allwinner platforms have 
not done this.

> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index ade7a38..c45a4f25 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -941,6 +941,7 @@ dtb-$(CONFIG_MACH_SUN7I) += \
>   sun7i-a20-lamobo-r1.dtb \
>   sun7i-a20-m3.dtb \
>   sun7i-a20-mk808c.dtb \
> + sun7i-a20-linova1-ctp-4_3i.dtb\
>   sun7i-a20-olimex-som-evb.dtb \
>   sun7i-a20-olinuxino-lime.dtb \
>   sun7i-a20-olinuxino-lime2.dtb \
> diff --git a/arch/arm/boot/dts/sun7i-a20-linova1-ctp-4_3i.dts 
> b/arch/arm/boot/dts/sun7i-a20-linova1-ctp-4_3i.dts
> new file mode 100644
> index 000..cd4ac73
> --- /dev/null
> +++ b/arch/arm/boot/dts/sun7i-a20-linova1-ctp-4_3i.dts
> @@ -0,0 +1,192 @@
> +/*
> + * This is based on sun7i-a20-linova1-ctp-4_3i.dts
> + *
> + * Copyright 2014 - Hans de Goede 
> + * Copyright (c) 2014 FUKAUMI Naoki 
> + * Copyright (c) 2018 Giulio Benetti 
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPL or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + *  a) This file is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This file is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + *  b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, 

Re: [PATCH 1/2] dt-bindings: Document qcom,adreno-gmu

[Rob Herring]

On Fri, Mar 16, 2018 at 12:51:51PM -0600, Jordan Crouse wrote:
> Document the device tree bindings for the Adreno GMU device
> available on Adreno a6xx targets.
> 
> Signed-off-by: Jordan Crouse 
> ---
>  .../devicetree/bindings/display/msm/gmu.txt| 54 
> ++
>  .../devicetree/bindings/display/msm/gpu.txt| 10 +++-
>  2 files changed, 62 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt

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

Re: [PATCH v6 1/3] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder

[Rob Herring]

On Tue, Mar 20, 2018 at 02:43:33PM +0200, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> (CC'ing Rob)
> 
> Thank you for the patch.
> 
> On Friday, 16 March 2018 17:16:37 EET Jacopo Mondi wrote:
> > Document Thine THC63LVD1024 LVDS decoder device tree bindings.
> > 
> > Signed-off-by: Jacopo Mondi 
> > Reviewed-by: Andrzej Hajda 
> > Reviewed-by: Niklas Söderlund 
> > ---
> >  .../bindings/display/bridge/thine,thc63lvd1024.txt | 66 +++
> >  1 file changed, 66 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > new file mode 100644
> > index 000..8225c6a
> > --- /dev/null
> > +++
> > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt
> > @@ -0,0 +1,66 @@
> > +Thine Electronics THC63LVD1024 LVDS decoder
> > +---
> > +
> > +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS
> > streams
> > +to parallel data outputs. The chip supports single/dual input/output modes,
> > +handling up to two two input LVDS stream and up to two digital CMOS/TTL
> > outputs.
> > +
> > +Single or dual operation modes, output data mapping and DDR output modes
> > are
> > +configured through input signals and the chip does not expose any control
> > bus.
> > +
> > +Required properties:
> > +- compatible: Shall be "thine,thc63lvd1024"
> > +
> > +Optional properties:
> > +- vcc-supply: Power supply for TTL output and digital circuitry
> > +- cvcc-supply: Power supply for TTL CLOCKOUT signal
> > +- lvcc-supply: Power supply for LVDS inputs
> > +- pvcc-supply: Power supply for PLL circuitry
> 
> As explained in a comment to one of the previous versions of this series, I'm 
> tempted to make vcc-supply mandatory and drop the three other power supplies 
> for now, as I believe there's very little chance they will be connected to 
> separately controllable regulators (all supplies use the same voltage). In 
> the 
> very unlikely event that this occurs in design we need to support in the 
> future, the cvcc, lvcc and pvcc supplies can be added later as optional 
> without breaking backward compatibility.

I'm okay with that.

> Apart from that,
> 
> Reviewed-by: Laurent Pinchart 
> 
> > +- pdwn-gpios: Power down GPIO signal. Active low

powerdown-gpios is the semi-standard name.

> > +- oe-gpios: Output enable GPIO signal. Active high
> > +
> > +The THC63LVD1024 video port connections are modeled according
> > +to OF graph bindings specified by
> > Documentation/devicetree/bindings/graph.txt
> > +
> > +Required video port nodes:
> > +- Port@0: First LVDS input port
> > +- Port@2: First digital CMOS/TTL parallel output

s/Port/port/

> > +
> > +Optional video port nodes:
> > +- Port@1: Second LVDS input port
> > +- Port@3: Second digital CMOS/TTL parallel output
> > +
> > +Example:
> > +
> > +
> > +   thc63lvd1024: lvds-decoder {
> > +   compatible = "thine,thc63lvd1024";
> > +
> > +   vcc-supply = <_lvds_vcc>;
> > +   lvcc-supply = <_lvds_lvcc>;
> > +
> > +   pdwn-gpio = < 15 GPIO_ACTIVE_LOW>;
> > +
> > +   ports {
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   port@0 {
> > +   reg = <0>;
> > +
> > +   lvds_dec_in_0: endpoint {
> > +   remote-endpoint = <_out>;
> > +   };
> > +   };
> > +
> > +   port@2{
> > +   reg = <2>;
> > +
> > +   lvds_dec_out_2: endpoint {
> > +   remote-endpoint = <_in>;
> > +   };
> > +
> > +   };
> > +
> > +   };
> > +   };
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 199123] kernel 4.16rc5 doesnt boot on ryzen 5 2400g due to an amdgpu change

[bugzilla-daemon]

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

--- Comment #13 from david becerra (davidbecerrapor...@gmail.com) ---
issue still present in 4.16rc7
4.15.13 still fine

-- 
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: [Intel-gfx] [PATCH 15/23] drm: Stop updating plane->crtc/fb/old_fb on atomic drivers

[Daniel Vetter]

On Mon, Mar 26, 2018 at 10:52:58PM +0200, Daniel Vetter wrote:
> On Thu, Mar 22, 2018 at 05:23:05PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > Stop playing around with plane->crtc/fb/old_fb with atomic
> > drivers. Make life a lot simpler when we don't have to do the
> > magic old_fb vs. fb dance around plane updates. That way we
> > can't risk plane->fb getting out of sync with plane->state->fb
> > and we're less likely to leak any refcounts as well.
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/drm_atomic.c| 55 
> > -
> >  drivers/gpu/drm/drm_atomic_helper.c | 15 +-
> >  drivers/gpu/drm/drm_crtc.c  |  8 --
> >  drivers/gpu/drm/drm_fb_helper.c |  7 -
> >  drivers/gpu/drm/drm_framebuffer.c   |  5 
> >  drivers/gpu/drm/drm_plane.c | 14 ++
> >  drivers/gpu/drm/drm_plane_helper.c  |  4 ++-
> >  include/drm/drm_atomic.h|  3 --
> >  8 files changed, 24 insertions(+), 87 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 7d25c42f22db..b16cc37e2adf 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -692,6 +692,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state 
> > *state,
> >  
> > WARN_ON(!state->acquire_ctx);
> >  
> > +   /* the legacy pointers should never be set */
> > +   WARN_ON(plane->fb);
> > +   WARN_ON(plane->old_fb);
> > +   WARN_ON(plane->crtc);
> 
> I did already review all the users for plane->crtc and found:
> 
> armada + shmob: not atomic, should be fine
> 
> But there's also exynos, msm/mdp5, sti and vc4 doing various silly things
> with setting plane->crtc. I think before you can add this WARN_ON we need
> to clean up that cruft (it looks like 100% cargo culting, so should be
> quit).

Ah, follow-up patches take care of most of this. But note that msm sets
plane->crtc in its _init function, so will trip over your WARN_ON here.

And you seem to have missed sti, which looks at plane->crtc instead of
plane->state->crtc (and appropriate locking) in its debugfs code.
-Daniel

> 
> Going to think about the other patches tomorrow.
> -Daniel
> 
> > +
> > plane_state = drm_atomic_get_existing_plane_state(state, plane);
> > if (plane_state)
> > return plane_state;
> > @@ -2021,45 +2026,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
> > *state,
> > return ret;
> >  }
> >  
> > -/**
> > - * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb 
> > pointers.
> > - *
> > - * @dev: drm device to check.
> > - * @plane_mask: plane mask for planes that were updated.
> > - * @ret: return value, can be -EDEADLK for a retry.
> > - *
> > - * Before doing an update _plane.old_fb is set to _plane.fb, but 
> > before
> > - * dropping the locks old_fb needs to be set to NULL and plane->fb 
> > updated. This
> > - * is a common operation for each atomic update, so this call is split off 
> > as a
> > - * helper.
> > - */
> > -void drm_atomic_clean_old_fb(struct drm_device *dev,
> > -unsigned plane_mask,
> > -int ret)
> > -{
> > -   struct drm_plane *plane;
> > -
> > -   /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
> > -* locks (ie. while it is still safe to deref plane->state).  We
> > -* need to do this here because the driver entry points cannot
> > -* distinguish between legacy and atomic ioctls.
> > -*/
> > -   drm_for_each_plane_mask(plane, dev, plane_mask) {
> > -   if (ret == 0) {
> > -   struct drm_framebuffer *new_fb = plane->state->fb;
> > -   if (new_fb)
> > -   drm_framebuffer_get(new_fb);
> > -   plane->fb = new_fb;
> > -   plane->crtc = plane->state->crtc;
> > -
> > -   if (plane->old_fb)
> > -   drm_framebuffer_put(plane->old_fb);
> > -   }
> > -   plane->old_fb = NULL;
> > -   }
> > -}
> > -EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> > -
> >  /**
> >   * DOC: explicit fencing properties
> >   *
> > @@ -2280,9 +2246,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > unsigned int copied_objs, copied_props;
> > struct drm_atomic_state *state;
> > struct drm_modeset_acquire_ctx ctx;
> > -   struct drm_plane *plane;
> > struct drm_out_fence_state *fence_state;
> > -   unsigned plane_mask;
> > int ret = 0;
> > unsigned int i, j, num_fences;
> >  
> > @@ -2322,7 +2286,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> > state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
> >  
> >  retry:
> > -   plane_mask = 0;
> > copied_objs = 0;
> > copied_props = 0;
> > fence_state = NULL;
> > @@ -2393,12 +2356,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> 

Re: [Intel-gfx] [PATCH 15/23] drm: Stop updating plane->crtc/fb/old_fb on atomic drivers

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:23:05PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Stop playing around with plane->crtc/fb/old_fb with atomic
> drivers. Make life a lot simpler when we don't have to do the
> magic old_fb vs. fb dance around plane updates. That way we
> can't risk plane->fb getting out of sync with plane->state->fb
> and we're less likely to leak any refcounts as well.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_atomic.c| 55 
> -
>  drivers/gpu/drm/drm_atomic_helper.c | 15 +-
>  drivers/gpu/drm/drm_crtc.c  |  8 --
>  drivers/gpu/drm/drm_fb_helper.c |  7 -
>  drivers/gpu/drm/drm_framebuffer.c   |  5 
>  drivers/gpu/drm/drm_plane.c | 14 ++
>  drivers/gpu/drm/drm_plane_helper.c  |  4 ++-
>  include/drm/drm_atomic.h|  3 --
>  8 files changed, 24 insertions(+), 87 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7d25c42f22db..b16cc37e2adf 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -692,6 +692,11 @@ drm_atomic_get_plane_state(struct drm_atomic_state 
> *state,
>  
>   WARN_ON(!state->acquire_ctx);
>  
> + /* the legacy pointers should never be set */
> + WARN_ON(plane->fb);
> + WARN_ON(plane->old_fb);
> + WARN_ON(plane->crtc);

I did already review all the users for plane->crtc and found:

armada + shmob: not atomic, should be fine

But there's also exynos, msm/mdp5, sti and vc4 doing various silly things
with setting plane->crtc. I think before you can add this WARN_ON we need
to clean up that cruft (it looks like 100% cargo culting, so should be
quit).

Going to think about the other patches tomorrow.
-Daniel

> +
>   plane_state = drm_atomic_get_existing_plane_state(state, plane);
>   if (plane_state)
>   return plane_state;
> @@ -2021,45 +2026,6 @@ int drm_atomic_set_property(struct drm_atomic_state 
> *state,
>   return ret;
>  }
>  
> -/**
> - * drm_atomic_clean_old_fb -- Unset old_fb pointers and set plane->fb 
> pointers.
> - *
> - * @dev: drm device to check.
> - * @plane_mask: plane mask for planes that were updated.
> - * @ret: return value, can be -EDEADLK for a retry.
> - *
> - * Before doing an update _plane.old_fb is set to _plane.fb, but 
> before
> - * dropping the locks old_fb needs to be set to NULL and plane->fb updated. 
> This
> - * is a common operation for each atomic update, so this call is split off 
> as a
> - * helper.
> - */
> -void drm_atomic_clean_old_fb(struct drm_device *dev,
> -  unsigned plane_mask,
> -  int ret)
> -{
> - struct drm_plane *plane;
> -
> - /* if succeeded, fixup legacy plane crtc/fb ptrs before dropping
> -  * locks (ie. while it is still safe to deref plane->state).  We
> -  * need to do this here because the driver entry points cannot
> -  * distinguish between legacy and atomic ioctls.
> -  */
> - drm_for_each_plane_mask(plane, dev, plane_mask) {
> - if (ret == 0) {
> - struct drm_framebuffer *new_fb = plane->state->fb;
> - if (new_fb)
> - drm_framebuffer_get(new_fb);
> - plane->fb = new_fb;
> - plane->crtc = plane->state->crtc;
> -
> - if (plane->old_fb)
> - drm_framebuffer_put(plane->old_fb);
> - }
> - plane->old_fb = NULL;
> - }
> -}
> -EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> -
>  /**
>   * DOC: explicit fencing properties
>   *
> @@ -2280,9 +2246,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   unsigned int copied_objs, copied_props;
>   struct drm_atomic_state *state;
>   struct drm_modeset_acquire_ctx ctx;
> - struct drm_plane *plane;
>   struct drm_out_fence_state *fence_state;
> - unsigned plane_mask;
>   int ret = 0;
>   unsigned int i, j, num_fences;
>  
> @@ -2322,7 +2286,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET);
>  
>  retry:
> - plane_mask = 0;
>   copied_objs = 0;
>   copied_props = 0;
>   fence_state = NULL;
> @@ -2393,12 +2356,6 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>   copied_props++;
>   }
>  
> - if (obj->type == DRM_MODE_OBJECT_PLANE && count_props &&
> - !(arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)) {
> - plane = obj_to_plane(obj);
> - plane_mask |= (1 << drm_plane_index(plane));
> - plane->old_fb = plane->fb;
> - }
>   drm_mode_object_put(obj);
>   }
>  
> @@ -2419,8 +2376,6 @@ int drm_mode_atomic_ioctl(struct drm_device 

Re: [PATCH 06/23] drm: Adjust whitespace for legibility

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:22:56PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Add a bit of whitespace here and there to make the code look a bit
> more structured.
> 
> Signed-off-by: Ville Syrjälä 

Too lazy to grow a real opinion on whitespace :-)

Acked-by: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_crtc.c  | 4 +++-
>  drivers/gpu/drm/drm_plane.c | 6 +-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 8552ed419056..537ffaab855c 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -434,13 +434,13 @@ int drm_mode_getcrtc(struct drm_device *dev,
>   if (crtc->state->enable) {
>   drm_mode_convert_to_umode(_resp->mode, 
> >state->mode);
>   crtc_resp->mode_valid = 1;
> -
>   } else {
>   crtc_resp->mode_valid = 0;
>   }
>   } else {
>   crtc_resp->x = crtc->x;
>   crtc_resp->y = crtc->y;
> +
>   if (crtc->enabled) {
>   drm_mode_convert_to_umode(_resp->mode, 
> >mode);
>   crtc_resp->mode_valid = 1;
> @@ -592,6 +592,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>   ret = drm_modeset_lock_all_ctx(crtc->dev, );
>   if (ret)
>   goto out;
> +
>   if (crtc_req->mode_valid) {
>   /* If we have a mode we need a framebuffer. */
>   /* If we pass -1, set the mode with the currently bound fb */
> @@ -601,6 +602,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>   ret = -EINVAL;
>   goto out;
>   }
> +
>   fb = plane->fb;
>   /* Make refcounting symmetric with the lookup path. */
>   drm_framebuffer_get(fb);
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 38e2a628bfa2..bedceca7dd06 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -785,6 +785,7 @@ static int drm_mode_cursor_universal(struct drm_crtc 
> *crtc,
>   DRM_DEBUG_KMS("failed to wrap cursor buffer in 
> drm framebuffer\n");
>   return PTR_ERR(fb);
>   }
> +
>   fb->hot_x = req->hot_x;
>   fb->hot_y = req->hot_y;
>   } else {
> @@ -1035,7 +1036,8 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>  state->src_h,
>  fb);
>   } else {
> - ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y, 
> >mode, fb);
> + ret = drm_crtc_check_viewport(crtc, crtc->x, crtc->y,
> +   >mode, fb);
>   }
>   if (ret)
>   goto out;
> @@ -1052,10 +1054,12 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>   ret = -ENOMEM;
>   goto out;
>   }
> +
>   e->event.base.type = DRM_EVENT_FLIP_COMPLETE;
>   e->event.base.length = sizeof(e->event);
>   e->event.vbl.user_data = page_flip->user_data;
>   e->event.vbl.crtc_id = crtc->base.id;
> +
>   ret = drm_event_reserve_init(dev, file_priv, >base, 
> >event.base);
>   if (ret) {
>   kfree(e);
> -- 
> 2.16.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 05/23] drm: Add local 'plane' variable for primary/cursor planes

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:22:55PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Make the code a bit more readable by storing the plane pointer in a
> local variable rather than having to do crtc->{primary,cursor} all the
> time.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_crtc.c  | 32 +++-
>  drivers/gpu/drm/drm_plane.c | 32 ++--
>  2 files changed, 37 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 7a973ada7195..8552ed419056 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -402,6 +402,7 @@ int drm_mode_getcrtc(struct drm_device *dev,
>  {
>   struct drm_mode_crtc *crtc_resp = data;
>   struct drm_crtc *crtc;
> + struct drm_plane *plane;
>  
>   if (!drm_core_check_feature(dev, DRIVER_MODESET))
>   return -EINVAL;
> @@ -410,21 +411,23 @@ int drm_mode_getcrtc(struct drm_device *dev,
>   if (!crtc)
>   return -ENOENT;
>  
> + plane = crtc->primary;
> +
>   crtc_resp->gamma_size = crtc->gamma_size;
>  
> - drm_modeset_lock(>primary->mutex, NULL);
> - if (crtc->primary->state && crtc->primary->state->fb)
> - crtc_resp->fb_id = crtc->primary->state->fb->base.id;
> - else if (!crtc->primary->state && crtc->primary->fb)
> - crtc_resp->fb_id = crtc->primary->fb->base.id;
> + drm_modeset_lock(>mutex, NULL);
> + if (plane->state && plane->state->fb)
> + crtc_resp->fb_id = plane->state->fb->base.id;
> + else if (!plane->state && plane->fb)
> + crtc_resp->fb_id = plane->fb->base.id;
>   else
>   crtc_resp->fb_id = 0;
>  
> - if (crtc->primary->state) {
> - crtc_resp->x = crtc->primary->state->src_x >> 16;
> - crtc_resp->y = crtc->primary->state->src_y >> 16;
> + if (plane->state) {
> + crtc_resp->x = plane->state->src_x >> 16;
> + crtc_resp->y = plane->state->src_y >> 16;
>   }
> - drm_modeset_unlock(>primary->mutex);
> + drm_modeset_unlock(>mutex);
>  
>   drm_modeset_lock(>mutex, NULL);
>   if (crtc->state) {
> @@ -554,6 +557,7 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>   struct drm_mode_config *config = >mode_config;
>   struct drm_mode_crtc *crtc_req = data;
>   struct drm_crtc *crtc;
> + struct drm_plane *plane;
>   struct drm_connector **connector_set = NULL, *connector;
>   struct drm_framebuffer *fb = NULL;
>   struct drm_display_mode *mode = NULL;
> @@ -580,6 +584,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,

Could do the same for setconfig_internal's tmp->primary, if you're bored.

>   }
>   DRM_DEBUG_KMS("[CRTC:%d:%s]\n", crtc->base.id, crtc->name);
>  
> + plane = crtc->primary;
> +
>   mutex_lock(>dev->mode_config.mutex);
>   drm_modeset_acquire_init(, DRM_MODESET_ACQUIRE_INTERRUPTIBLE);
>  retry:
> @@ -590,12 +596,12 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>   /* If we have a mode we need a framebuffer. */
>   /* If we pass -1, set the mode with the currently bound fb */
>   if (crtc_req->fb_id == -1) {
> - if (!crtc->primary->fb) {
> + if (!plane->fb) {
>   DRM_DEBUG_KMS("CRTC doesn't have current FB\n");
>   ret = -EINVAL;
>   goto out;
>   }
> - fb = crtc->primary->fb;
> + fb = plane->fb;
>   /* Make refcounting symmetric with the lookup path. */
>   drm_framebuffer_get(fb);
>   } else {
> @@ -627,8 +633,8 @@ int drm_mode_setcrtc(struct drm_device *dev, void *data,
>* match real hardware capabilities. Skip the check in that
>* case.
>*/
> - if (!crtc->primary->format_default) {
> - ret = drm_plane_check_pixel_format(crtc->primary,
> + if (!plane->format_default) {
> + ret = drm_plane_check_pixel_format(plane,
>  fb->format->format,
>  fb->modifier);
>   if (ret) {
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 6d2a6e428a3e..38e2a628bfa2 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -756,6 +756,7 @@ static int drm_mode_cursor_universal(struct drm_crtc 
> *crtc,
>struct drm_modeset_acquire_ctx *ctx)
>  {
>   struct drm_device *dev = crtc->dev;
> + struct drm_plane *plane = crtc->cursor;
>   struct drm_framebuffer *fb = 

Re: [PATCH 02/23] drm/atomic-helper: Make drm_atomic_helper_disable_all() update the plane->fb pointers

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:22:52PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> drm_atomic_helper_shutdown() needs to release the reference held by
> plane->fb, so we want to use drm_atomic_clean_old_fb() in
> drm_atomic_helper_disable_all(). However during suspend/resume, gpu
> reset and load detection we should probably leave that stuff alone,
> as otherwise we'd have to make sure we put them back again when
> we restore the duplicated state to the device. Seems simpler to me
> to not touch any of it anyway.
> 
> v2: Don't inflict the clean_old_fbs bool to drivers (Daniel)
> 
> Cc: martin.pe...@free.fr
> Cc: ch...@chris-wilson.co.uk
> Cc: Dave Airlie  (v1)

More funny (v1) I just noticed ...
-Daniel

> Cc: Maarten Lankhorst 
> Cc: Daniel Vetter 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 67 
> ++---
>  1 file changed, 40 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index c48f187d08de..39a69508d8c9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2881,31 +2881,9 @@ int __drm_atomic_helper_set_config(struct drm_mode_set 
> *set,
>   return 0;
>  }
>  
> -/**
> - * drm_atomic_helper_disable_all - disable all currently active outputs
> - * @dev: DRM device
> - * @ctx: lock acquisition context
> - *
> - * Loops through all connectors, finding those that aren't turned off and 
> then
> - * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
> - * that they are connected to.
> - *
> - * This is used for example in suspend/resume to disable all currently active
> - * functions when suspending. If you just want to shut down everything at 
> e.g.
> - * driver unload, look at drm_atomic_helper_shutdown().
> - *
> - * Note that if callers haven't already acquired all modeset locks this might
> - * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
> - *
> - * Returns:
> - * 0 on success or a negative error code on failure.
> - *
> - * See also:
> - * drm_atomic_helper_suspend(), drm_atomic_helper_resume() and
> - * drm_atomic_helper_shutdown().
> - */
> -int drm_atomic_helper_disable_all(struct drm_device *dev,
> -   struct drm_modeset_acquire_ctx *ctx)
> +static int __drm_atomic_helper_disable_all(struct drm_device *dev,
> +struct drm_modeset_acquire_ctx *ctx,
> +bool clean_old_fbs)
>  {
>   struct drm_atomic_state *state;
>   struct drm_connector_state *conn_state;
> @@ -2914,6 +2892,7 @@ int drm_atomic_helper_disable_all(struct drm_device 
> *dev,
>   struct drm_plane *plane;
>   struct drm_crtc_state *crtc_state;
>   struct drm_crtc *crtc;
> + unsigned int plane_mask = 0;
>   int ret, i;
>  
>   state = drm_atomic_state_alloc(dev);
> @@ -2956,14 +2935,48 @@ int drm_atomic_helper_disable_all(struct drm_device 
> *dev,
>   goto free;
>  
>   drm_atomic_set_fb_for_plane(plane_state, NULL);
> +
> + if (clean_old_fbs) {
> + plane->old_fb = plane->fb;
> + plane_mask |= BIT(drm_plane_index(plane));
> + }
>   }
>  
>   ret = drm_atomic_commit(state);
>  free:
> + drm_atomic_clean_old_fb(dev, plane_mask, ret);
> +
>   drm_atomic_state_put(state);
>   return ret;
>  }
> -
> +/**
> + * drm_atomic_helper_disable_all - disable all currently active outputs
> + * @dev: DRM device
> + * @ctx: lock acquisition context
> + *
> + * Loops through all connectors, finding those that aren't turned off and 
> then
> + * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
> + * that they are connected to.
> + *
> + * This is used for example in suspend/resume to disable all currently active
> + * functions when suspending. If you just want to shut down everything at 
> e.g.
> + * driver unload, look at drm_atomic_helper_shutdown().
> + *
> + * Note that if callers haven't already acquired all modeset locks this might
> + * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + *
> + * See also:
> + * drm_atomic_helper_suspend(), drm_atomic_helper_resume() and
> + * drm_atomic_helper_shutdown().
> + */
> +int drm_atomic_helper_disable_all(struct drm_device *dev,
> +   struct drm_modeset_acquire_ctx *ctx)
> +{
> + return __drm_atomic_helper_disable_all(dev, ctx, false);
> +}
>  EXPORT_SYMBOL(drm_atomic_helper_disable_all);
>  
>  /**
> @@ -2986,7 +2999,7 @@ void drm_atomic_helper_shutdown(struct drm_device *dev)
>   while (1) {
>

Re: [Intel-gfx] [PATCH 04/23] drm/atomic-helper: WARN if legacy plane fb pointers are bogus when committing duplicated state

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:22:54PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> drm_atomic_helper_commit_duplicated_state() should only be called
> resume/reset/load_detect paths where plane->old_fb should always be
> NULL and plane->fb should be equal to the new_plane_state->fb.
> Assert that is indeed the case.
> 
> Cc: martin.pe...@free.fr
> Cc: ch...@chris-wilson.co.uk
> Cc: Dave Airlie  (v1)

The (v1) here looks funny. dim add-missing-cc not working?

> Cc: Maarten Lankhorst 
> Cc: Daniel Vetter 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 39a69508d8c9..1b39ebf2be2e 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3106,8 +3106,13 @@ int drm_atomic_helper_commit_duplicated_state(struct 
> drm_atomic_state *state,
>  
>   state->acquire_ctx = ctx;
>  
> - for_each_new_plane_in_state(state, plane, new_plane_state, i)
> + for_each_new_plane_in_state(state, plane, new_plane_state, i) {
> + WARN_ON(plane->crtc != new_plane_state->crtc);
> + WARN_ON(plane->fb != new_plane_state->fb);
> + WARN_ON(plane->old_fb);

Yeah I think with your rework of how this works this holds true now.

Reviewed-by: Daniel Vetter 

> +
>   state->planes[i].old_state = plane->state;
> + }
>  
>   for_each_new_crtc_in_state(state, crtc, new_crtc_state, i)
>   state->crtcs[i].old_state = crtc->state;
> -- 
> 2.16.1
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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: [Intel-gfx] [PATCH 03/23] drm: Clear crtc->primary->crtc when disabling the crtc via setcrtc()

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:22:53PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Keep the primary->crtc in sync with the state->crtc (also with
> primary->fb and state->fb) when disabling the crtc (and thus also
> the primary) via setcrtc().
> 
> Signed-off-by: Ville Syrjälä 

Yeah seems more consistent, and we do the same for atomic already. Only 2
legacy drivers seem to look at this (armada and shmob), and I think both
should be fine.

Reviewed-by: Daniel Vetter 

> ---
>  drivers/gpu/drm/drm_crtc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 03583887cfec..7a973ada7195 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -471,7 +471,7 @@ static int __drm_mode_set_config_internal(struct 
> drm_mode_set *set,
>  
>   ret = crtc->funcs->set_config(set, ctx);
>   if (ret == 0) {
> - crtc->primary->crtc = crtc;
> + crtc->primary->crtc = fb ? crtc : NULL;
>   crtc->primary->fb = fb;
>   }
>  
> -- 
> 2.16.1
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 02/23] drm/atomic-helper: Make drm_atomic_helper_disable_all() update the plane->fb pointers

[Daniel Vetter]

On Thu, Mar 22, 2018 at 05:22:52PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> drm_atomic_helper_shutdown() needs to release the reference held by
> plane->fb, so we want to use drm_atomic_clean_old_fb() in
> drm_atomic_helper_disable_all(). However during suspend/resume, gpu
> reset and load detection we should probably leave that stuff alone,
> as otherwise we'd have to make sure we put them back again when
> we restore the duplicated state to the device. Seems simpler to me
> to not touch any of it anyway.
> 
> v2: Don't inflict the clean_old_fbs bool to drivers (Daniel)
> 
> Cc: martin.pe...@free.fr
> Cc: ch...@chris-wilson.co.uk
> Cc: Dave Airlie  (v1)
> Cc: Maarten Lankhorst 
> Cc: Daniel Vetter 
> Signed-off-by: Ville Syrjälä 

I think this would be cleaner diff to read if you squash the first 2
patches together. Also avoids the bisect fail. With that (and I trust you
to come up with a suitably merged commit message):

Reviewed-by: Daniel Vetter 

I reviewed this by re-reading the analysis from 49d70aeaeca8f62b72b77 and
trusting my former self :-)

Cheers, Daniel

> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 67 
> ++---
>  1 file changed, 40 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> b/drivers/gpu/drm/drm_atomic_helper.c
> index c48f187d08de..39a69508d8c9 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2881,31 +2881,9 @@ int __drm_atomic_helper_set_config(struct drm_mode_set 
> *set,
>   return 0;
>  }
>  
> -/**
> - * drm_atomic_helper_disable_all - disable all currently active outputs
> - * @dev: DRM device
> - * @ctx: lock acquisition context
> - *
> - * Loops through all connectors, finding those that aren't turned off and 
> then
> - * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
> - * that they are connected to.
> - *
> - * This is used for example in suspend/resume to disable all currently active
> - * functions when suspending. If you just want to shut down everything at 
> e.g.
> - * driver unload, look at drm_atomic_helper_shutdown().
> - *
> - * Note that if callers haven't already acquired all modeset locks this might
> - * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
> - *
> - * Returns:
> - * 0 on success or a negative error code on failure.
> - *
> - * See also:
> - * drm_atomic_helper_suspend(), drm_atomic_helper_resume() and
> - * drm_atomic_helper_shutdown().
> - */
> -int drm_atomic_helper_disable_all(struct drm_device *dev,
> -   struct drm_modeset_acquire_ctx *ctx)
> +static int __drm_atomic_helper_disable_all(struct drm_device *dev,
> +struct drm_modeset_acquire_ctx *ctx,
> +bool clean_old_fbs)
>  {
>   struct drm_atomic_state *state;
>   struct drm_connector_state *conn_state;
> @@ -2914,6 +2892,7 @@ int drm_atomic_helper_disable_all(struct drm_device 
> *dev,
>   struct drm_plane *plane;
>   struct drm_crtc_state *crtc_state;
>   struct drm_crtc *crtc;
> + unsigned int plane_mask = 0;
>   int ret, i;
>  
>   state = drm_atomic_state_alloc(dev);
> @@ -2956,14 +2935,48 @@ int drm_atomic_helper_disable_all(struct drm_device 
> *dev,
>   goto free;
>  
>   drm_atomic_set_fb_for_plane(plane_state, NULL);
> +
> + if (clean_old_fbs) {
> + plane->old_fb = plane->fb;
> + plane_mask |= BIT(drm_plane_index(plane));
> + }
>   }
>  
>   ret = drm_atomic_commit(state);
>  free:
> + drm_atomic_clean_old_fb(dev, plane_mask, ret);
> +
>   drm_atomic_state_put(state);
>   return ret;
>  }
> -
> +/**
> + * drm_atomic_helper_disable_all - disable all currently active outputs
> + * @dev: DRM device
> + * @ctx: lock acquisition context
> + *
> + * Loops through all connectors, finding those that aren't turned off and 
> then
> + * turns them off by setting their DPMS mode to OFF and deactivating the CRTC
> + * that they are connected to.
> + *
> + * This is used for example in suspend/resume to disable all currently active
> + * functions when suspending. If you just want to shut down everything at 
> e.g.
> + * driver unload, look at drm_atomic_helper_shutdown().
> + *
> + * Note that if callers haven't already acquired all modeset locks this might
> + * return -EDEADLK, which must be handled by calling drm_modeset_backoff().
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + *
> + * See also:
> + * drm_atomic_helper_suspend(), drm_atomic_helper_resume() and
> + * drm_atomic_helper_shutdown().
> + */
> +int drm_atomic_helper_disable_all(struct 

[PATCH 1/1] drm: add parameter explanation for some gem dmabuf_ops

[Samuel Li]

To reduce some warnings.

Signed-off-by: Samuel Li 
---
 drivers/gpu/drm/drm_prime.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 7856a9b..caf675e 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -331,6 +331,9 @@ EXPORT_SYMBOL(drm_gem_map_dma_buf);
 
 /**
  * drm_gem_unmap_dma_buf - unmap_dma_buf implementation for GEM
+ * @attach: attachment to unmap buffer from
+ * @sgt: scatterlist info of the buffer to unmap
+ * @dir: direction of DMA transfer
  *
  * Not implemented. The unmap is done at drm_gem_map_detach().  This can be
  * used as the _buf_ops.unmap_dma_buf callback.
@@ -429,6 +432,8 @@ EXPORT_SYMBOL(drm_gem_dmabuf_vunmap);
 
 /**
  * drm_gem_dmabuf_kmap_atomic - map_atomic implementation for GEM
+ * @dma_buf: buffer to be mapped
+ * @page_num: page number within the buffer
  *
  * Not implemented. This can be used as the _buf_ops.map_atomic callback.
  */
@@ -441,6 +446,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_kmap_atomic);
 
 /**
  * drm_gem_dmabuf_kunmap_atomic - unmap_atomic implementation for GEM
+ * @dma_buf: buffer to be unmapped
+ * @page_num: page number within the buffer
+ * @addr: virtual address of the buffer
  *
  * Not implemented. This can be used as the _buf_ops.unmap_atomic callback.
  */
@@ -453,6 +461,8 @@ EXPORT_SYMBOL(drm_gem_dmabuf_kunmap_atomic);
 
 /**
  * drm_gem_dmabuf_kmap - map implementation for GEM
+ * @dma_buf: buffer to be mapped
+ * @page_num: page number within the buffer
  *
  * Not implemented. This can be used as the _buf_ops.map callback.
  */
@@ -464,6 +474,9 @@ EXPORT_SYMBOL(drm_gem_dmabuf_kmap);
 
 /**
  * drm_gem_dmabuf_kunmap - unmap implementation for GEM
+ * @dma_buf: buffer to be unmapped
+ * @page_num: page number within the buffer
+ * @addr: virtual address of the buffer
  *
  * Not implemented. This can be used as the _buf_ops.unmap callback.
  */
-- 
2.7.4

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

Re: [RFC PATCH 2/3] drm: bridge: panel: allow override of the bus format

[Laurent Pinchart]

Hi Peter,

(CC'ing Jacopo Mondi)

On Sunday, 25 March 2018 15:01:11 EEST Peter Rosin wrote:
> On 2018-03-20 14:56, Laurent Pinchart wrote:
> > Hi Peter,
> > 
> > Thank you for the patch.
> > 
> > On Sunday, 18 March 2018 00:15:24 EET Peter Rosin wrote:
> >> Useful if the bridge does some kind of conversion of the bus format.
> >> 
> >> Signed-off-by: Peter Rosin 
> >> ---
> >> 
> >>  drivers/gpu/drm/bridge/panel.c | 22 +-
> >>  include/drm/drm_bridge.h   |  1 +
> >>  2 files changed, 22 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/panel.c
> >> b/drivers/gpu/drm/bridge/panel.c index 6d99d4a3beb3..ccef0283ff41 100644
> >> --- a/drivers/gpu/drm/bridge/panel.c
> >> +++ b/drivers/gpu/drm/bridge/panel.c
> >> @@ -22,6 +22,7 @@ struct panel_bridge {
> >>struct drm_connector connector;
> >>struct drm_panel *panel;
> >>u32 connector_type;
> >> +  u32 bus_format;
> >>  };
> >>  
> >>  static inline struct panel_bridge *
> >> @@ -40,8 +41,15 @@ static int panel_bridge_connector_get_modes(struct
> >> drm_connector *connector) {
> >>struct panel_bridge *panel_bridge =
> >>drm_connector_to_panel_bridge(connector);
> >> 
> >> +  int ret;
> >> +
> >> +  ret = drm_panel_get_modes(panel_bridge->panel);
> >> +
> >> +  if (panel_bridge->bus_format)
> >> +  drm_display_info_set_bus_formats(>display_info,
> >> +   _bridge->bus_format, 1);
> > 
> > While I agree with the problem statement and, to some extent, the DT
> > bindings, I don't think this is the right implementation. You've
> > correctly noted that display controller shouldn't blindly use the formats
> > reported by the panel through the connector formats, and that hacking the
> > panel driver to override the formats isn't a good idea, so I wouldn't
> > override the formats reported by the connector. I would instead extend
> > the drm_bridge API to report formats at bridge inputs. This would be more
> > generic and allow each bridge to configure itself according to the next
> > bridge in the chain.
> > 
> > I'm not sure whether this API extension should be in the form of a new
> > bridge function, or if the formats should be stored in the drm_bridge
> > structure directly as done for connectors in the display info structure.
> > I'm tempted by the former, but I'm open to discussions.
> 
> Ok, I can look into that, but let me check if I got this right. From the
> very little of the code that I have looked at, I have gathered that display
> controllers handle bridges explicitly, right?

That's correct, yes. Or, rather, they handle the first bridge in the chain, 
and then other bridges are handled recursively.

> If so, by extending the bridge (with either a new function or new data) you
> impose changes to all display controllers wanting to handle this new bridge
> input-format. If so, I assume I can leave out the changes to all display
> controllers that I do not care about. Correct?

That's correct.

> Also, don't hold your breath waiting for a v2, but I'll try to get to it :-)

I won't hold my breath, but Jacopo might :-) He has a similar issue to solve 
(reporting the LVDS modes supported by the bridge).

> >> -  return drm_panel_get_modes(panel_bridge->panel);
> >> +  return ret;
> >>  }
> >>  
> >>  static const struct drm_connector_helper_funcs
> >> @@ -203,6 +211,18 @@ void drm_panel_bridge_remove(struct drm_bridge
> >> *bridge)
> >>  }
> >>  EXPORT_SYMBOL(drm_panel_bridge_remove);
> >> 
> >> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> >> bus_format)
> >> +{
> >> +  struct panel_bridge *panel_bridge;
> >> +
> >> +  if (!bridge)
> >> +  return;
> >> +
> >> +  panel_bridge = drm_bridge_to_panel_bridge(bridge);
> >> +  panel_bridge->bus_format = bus_format;
> >> +}
> >> +EXPORT_SYMBOL(drm_panel_bridge_set_bus_format);
> >> +
> >>  static void devm_drm_panel_bridge_release(struct device *dev, void *res)
> >>  {
> >>struct drm_bridge **bridge = res;
> >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> >> index 682d01ba920c..81903b92f187 100644
> >> --- a/include/drm/drm_bridge.h
> >> +++ b/include/drm/drm_bridge.h
> >> @@ -268,6 +268,7 @@ void drm_bridge_enable(struct drm_bridge *bridge);
> >>  struct drm_bridge *drm_panel_bridge_add(struct drm_panel *panel,
> >>u32 connector_type);
> >>  void drm_panel_bridge_remove(struct drm_bridge *bridge);
> >> +void drm_panel_bridge_set_bus_format(struct drm_bridge *bridge, u32
> >> bus_format);
> >> struct drm_bridge *devm_drm_panel_bridge_add(struct device *dev,
> >> struct drm_panel *panel,
> >> u32 connector_type);

-- 
Regards,

Laurent Pinchart

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

[PATCH] drm/syncobj: Stop reusing the same struct file for all syncobj -> fd

[Jason Ekstrand]

From: Chris Wilson 

The vk cts test:
dEQP-VK.api.external.semaphore.opaque_fd.export_multiple_times_temporary

triggers a lot of
VFS: Close: file count is 0

Dave pointed out that clearing the syncobj->file from
drm_syncobj_file_release() was sufficient to silence the test, but that
opens a can of worm since we assumed that the syncobj->file was never
unset. Stop trying to reuse the same struct file for every fd pointing
to the drm_syncobj, and allocate one file for each fd instead.

v2: Fixup return handling of drm_syncobj_fd_to_handle
v2.1: [airlied: fix possible syncobj ref race]
v2.2: [jekstrand: back-port to 4.14]

Reported-by: Dave Airlie 
Signed-off-by: Chris Wilson 
Tested-by: Dave Airlie 
Reviewed-by: Daniel Vetter 
Signed-off-by: Dave Airlie 
Signed-off-by: Jason Ekstrand 
Tested-by: Clayton Craft 
---

The back-port from 4.15 to 4.14 was non-trivial.  It'd be good if Chris and
maybe Daniel could do a quick re-review.

 drivers/gpu/drm/drm_syncobj.c | 81 ---
 include/drm/drm_syncobj.h |  5 ---
 2 files changed, 30 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 0422b8c..7bcf570 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -328,28 +328,11 @@ static const struct file_operations drm_syncobj_file_fops 
= {
.release = drm_syncobj_file_release,
 };
 
-static int drm_syncobj_alloc_file(struct drm_syncobj *syncobj)
-{
-   struct file *file = anon_inode_getfile("syncobj_file",
-  _syncobj_file_fops,
-  syncobj, 0);
-   if (IS_ERR(file))
-   return PTR_ERR(file);
-
-   drm_syncobj_get(syncobj);
-   if (cmpxchg(>file, NULL, file)) {
-   /* lost the race */
-   fput(file);
-   }
-
-   return 0;
-}
-
 static int drm_syncobj_handle_to_fd(struct drm_file *file_private,
u32 handle, int *p_fd)
 {
struct drm_syncobj *syncobj = drm_syncobj_find(file_private, handle);
-   int ret;
+   struct file *file;
int fd;
 
if (!syncobj)
@@ -361,46 +344,40 @@ static int drm_syncobj_handle_to_fd(struct drm_file 
*file_private,
return fd;
}
 
-   if (!syncobj->file) {
-   ret = drm_syncobj_alloc_file(syncobj);
-   if (ret)
-   goto out_put_fd;
+   file = anon_inode_getfile("syncobj_file",
+ _syncobj_file_fops,
+ syncobj, 0);
+   if (IS_ERR(file)) {
+   put_unused_fd(fd);
+   drm_syncobj_put(syncobj);
+   return PTR_ERR(file);
}
-   fd_install(fd, syncobj->file);
-   drm_syncobj_put(syncobj);
+
+   drm_syncobj_get(syncobj);
+   fd_install(fd, file);
+
*p_fd = fd;
return 0;
-out_put_fd:
-   put_unused_fd(fd);
-   drm_syncobj_put(syncobj);
-   return ret;
 }
 
-static struct drm_syncobj *drm_syncobj_fdget(int fd)
-{
-   struct file *file = fget(fd);
-
-   if (!file)
-   return NULL;
-   if (file->f_op != _syncobj_file_fops)
-   goto err;
-
-   return file->private_data;
-err:
-   fput(file);
-   return NULL;
-};
-
 static int drm_syncobj_fd_to_handle(struct drm_file *file_private,
int fd, u32 *handle)
 {
-   struct drm_syncobj *syncobj = drm_syncobj_fdget(fd);
+   struct drm_syncobj *syncobj;
+   struct file *file;
int ret;
 
-   if (!syncobj)
+   file = fget(fd);
+   if (!file)
+   return -EINVAL;
+
+   if (file->f_op != _syncobj_file_fops) {
+   fput(file);
return -EINVAL;
+   }
 
/* take a reference to put in the idr */
+   syncobj = file->private_data;
drm_syncobj_get(syncobj);
 
idr_preload(GFP_KERNEL);
@@ -409,12 +386,14 @@ static int drm_syncobj_fd_to_handle(struct drm_file 
*file_private,
spin_unlock(_private->syncobj_table_lock);
idr_preload_end();
 
-   if (ret < 0) {
-   fput(syncobj->file);
-   return ret;
-   }
-   *handle = ret;
-   return 0;
+   if (ret > 0) {
+   *handle = ret;
+   ret = 0;
+   } else
+   drm_syncobj_put(syncobj);
+
+   fput(file);
+   return ret;
 }
 
 int drm_syncobj_import_sync_file_fence(struct drm_file *file_private,
diff --git a/include/drm/drm_syncobj.h b/include/drm/drm_syncobj.h
index c00fee5..6d45aae 100644
--- a/include/drm/drm_syncobj.h
+++ b/include/drm/drm_syncobj.h
@@ -60,11 +60,6 @@ struct drm_syncobj {
 * 

Re: [PATCH] drm/scdc-helper: Convert errors into debug messages

[Ville Syrjälä]

On Sat, Mar 24, 2018 at 08:35:43AM +0530, Sharma, Shashank wrote:
> Reviewed-by: Shashank Sharma 

Thanks. Pushed to drm-misc-next.

> 
> Regards
> Shashank
> On 3/23/2018 11:55 PM, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> >
> > Since we may attempt to reconfigure SCDC when the sink has already been
> > disconnected we probably shouldn't scare the user with errors in dmesg
> > that are 100% expected in that case. Just leave it up to the caller
> > whether to print an error message or not, and just output debug
> > messages from the helper itself.
> >
> > Cc: Shashank Sharma 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >   drivers/gpu/drm/drm_scdc_helper.c | 10 +-
> >   1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_scdc_helper.c 
> > b/drivers/gpu/drm/drm_scdc_helper.c
> > index 657ea5ab6c3f..870e25f1f788 100644
> > --- a/drivers/gpu/drm/drm_scdc_helper.c
> > +++ b/drivers/gpu/drm/drm_scdc_helper.c
> > @@ -141,7 +141,7 @@ bool drm_scdc_get_scrambling_status(struct i2c_adapter 
> > *adapter)
> >   
> > ret = drm_scdc_readb(adapter, SCDC_SCRAMBLER_STATUS, );
> > if (ret < 0) {
> > -   DRM_ERROR("Failed to read scrambling status: %d\n", ret);
> > +   DRM_DEBUG_KMS("Failed to read scrambling status: %d\n", ret);
> > return false;
> > }
> >   
> > @@ -168,7 +168,7 @@ bool drm_scdc_set_scrambling(struct i2c_adapter 
> > *adapter, bool enable)
> >   
> > ret = drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, );
> > if (ret < 0) {
> > -   DRM_ERROR("Failed to read TMDS config: %d\n", ret);
> > +   DRM_DEBUG_KMS("Failed to read TMDS config: %d\n", ret);
> > return false;
> > }
> >   
> > @@ -179,7 +179,7 @@ bool drm_scdc_set_scrambling(struct i2c_adapter 
> > *adapter, bool enable)
> >   
> > ret = drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config);
> > if (ret < 0) {
> > -   DRM_ERROR("Failed to enable scrambling: %d\n", ret);
> > +   DRM_DEBUG_KMS("Failed to enable scrambling: %d\n", ret);
> > return false;
> > }
> >   
> > @@ -223,7 +223,7 @@ bool drm_scdc_set_high_tmds_clock_ratio(struct 
> > i2c_adapter *adapter, bool set)
> >   
> > ret = drm_scdc_readb(adapter, SCDC_TMDS_CONFIG, );
> > if (ret < 0) {
> > -   DRM_ERROR("Failed to read TMDS config: %d\n", ret);
> > +   DRM_DEBUG_KMS("Failed to read TMDS config: %d\n", ret);
> > return false;
> > }
> >   
> > @@ -234,7 +234,7 @@ bool drm_scdc_set_high_tmds_clock_ratio(struct 
> > i2c_adapter *adapter, bool set)
> >   
> > ret = drm_scdc_writeb(adapter, SCDC_TMDS_CONFIG, config);
> > if (ret < 0) {
> > -   DRM_ERROR("Failed to set TMDS clock ratio: %d\n", ret);
> > +   DRM_DEBUG_KMS("Failed to set TMDS clock ratio: %d\n", ret);
> > return false;
> > }
> >   

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

[Bug 99349] Failed to build shader (translation from TGSI)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=99349

--- Comment #21 from mirh  ---
Specific issue was reported in bug 105371. Follows there. 
Sorry for the bother.

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

[Bug 199101] AMDGPU Fury X random screen flicker on Linux kernel 4.16rc5

[bugzilla-daemon]

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

--- Comment #7 from Kevin McCormack (wittyma...@yahoo.com) ---
It's not a problem on 4.15. It was still a problem on the last rc I tried.

On March 26, 2018 4:58:42 AM EDT, bugzilla-dae...@bugzilla.kernel.org wrote:
>https://bugzilla.kernel.org/show_bug.cgi?id=199101
>
>Thorsten Leemhuis (regressi...@leemhuis.info) changed:
>
>   What|Removed |Added
>
>  CC||regressi...@leemhuis.info
>
>--- Comment #6 from Thorsten Leemhuis (regressi...@leemhuis.info) ---
>(In reply to Kevin McCormack from comment #3)
>> Still happening with rc6
>
>Kevin: Is this still happening? And is this working with 4.15 (that's
>unclear
>from the initial report)? Just wondering, because I have this issue on
>the
>regression reports for 4.16
>
>-- 
>You are receiving this mail because:
>You reported the bug.

-- 
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

[PATCH] drm/scheduler: fix param documentation

[Nayan Deshmukh]

There is no @kernel parameter anymore and document the
@guilty parameter

Signed-off-by: Nayan Deshmukh 
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c 
b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 0d95888ccc3e..1d368bc66ac2 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -117,8 +117,9 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
  * @sched  The pointer to the scheduler
  * @entity The pointer to a valid drm_sched_entity
  * @rq The run queue this entity belongs
- * @kernel If this is an entity for the kernel
  * @jobs   The max number of jobs in the job queue
+ * @guilty  atomic_t set to 1 when a job on this queue
+ *  is found to be guilty causing a timeout
  *
  * return 0 if succeed. negative error code on failure
 */
-- 
2.14.3

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

Re: [PATCH v2 2/3] drm/msm/dsi: add implementation for helper functions

[Sibi S]

Hi Jordan,
Thanks for the review.

On 03/20/2018 01:53 AM, Jordan Crouse wrote:

On Tue, Mar 20, 2018 at 01:11:02AM +0530, Sibi S wrote:

Add dsi host helper function implementation for DSI v2
DSI 6G 1.x and DSI 6G v2.0+ controllers

Signed-off-by: Sibi S 
---
  drivers/gpu/drm/msm/dsi/dsi.h  |  15 +++
  drivers/gpu/drm/msm/dsi/dsi_cfg.c  |  56 +++--
  drivers/gpu/drm/msm/dsi/dsi_host.c | 236 -
  3 files changed, 296 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 80be83e8fdec..dfa049d876bd 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -183,6 +183,21 @@ int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
  int msm_dsi_host_init(struct msm_dsi *msm_dsi);
  int msm_dsi_runtime_suspend(struct device *dev);
  int msm_dsi_runtime_resume(struct device *dev);
+int dsi_link_clk_enable_6g(struct msm_dsi_host *msm_host);
+int dsi_link_clk_enable_v2(struct msm_dsi_host *msm_host);
+void dsi_link_clk_disable_6g(struct msm_dsi_host *msm_host);
+void dsi_link_clk_disable_v2(struct msm_dsi_host *msm_host);
+int dsi_tx_buf_alloc_6g(struct msm_dsi_host *msm_host, int size);
+int dsi_tx_buf_alloc_v2(struct msm_dsi_host *msm_host, int size);
+void *dsi_tx_buf_get_6g(struct msm_dsi_host *msm_host);
+void *dsi_tx_buf_get_v2(struct msm_dsi_host *msm_host);
+void dsi_tx_buf_put_6g(struct msm_dsi_host *msm_host);
+int dsi_dma_base_get_6g(struct msm_dsi_host *msm_host, uint64_t *iova);
+int dsi_dma_base_get_v2(struct msm_dsi_host *msm_host, uint64_t *iova);
+int dsi_clk_init_v2(struct msm_dsi_host *msm_host);
+int dsi_clk_init_6g_v2(struct msm_dsi_host *msm_host);
+int dsi_calc_clk_rate_v2(struct msm_dsi_host *msm_host);
+int dsi_calc_clk_rate_6g(struct msm_dsi_host *msm_host);
  
  /* dsi phy */

  struct msm_dsi_phy;
diff --git a/drivers/gpu/drm/msm/dsi/dsi_cfg.c 
b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
index 0327bb54b01b..dcdfb1bb54f9 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_cfg.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_cfg.c
@@ -136,20 +136,58 @@ static const struct msm_dsi_config sdm845_dsi_cfg = {
.num_dsi = 2,
  };
  
+const static struct msm_dsi_host_cfg_ops msm_dsi_v2_host_ops = {

+   .link_clk_enable = dsi_link_clk_enable_v2,
+   .link_clk_disable = dsi_link_clk_disable_v2,
+   .clk_init_ver = dsi_clk_init_v2,
+   .tx_buf_alloc = dsi_tx_buf_alloc_v2,
+   .tx_buf_get = dsi_tx_buf_get_v2,
+   .tx_buf_put = NULL,
+   .dma_base_get = dsi_dma_base_get_v2,
+   .calc_clk_rate = dsi_calc_clk_rate_v2,
+};
+
+const static struct msm_dsi_host_cfg_ops msm_dsi_6g_host_ops = {
+   .link_clk_enable = dsi_link_clk_enable_6g,
+   .link_clk_disable = dsi_link_clk_disable_6g,
+   .clk_init_ver = NULL,
+   .tx_buf_alloc = dsi_tx_buf_alloc_6g,
+   .tx_buf_get = dsi_tx_buf_get_6g,
+   .tx_buf_put = dsi_tx_buf_put_6g,
+   .dma_base_get = dsi_dma_base_get_6g,
+   .calc_clk_rate = dsi_calc_clk_rate_6g,
+};
+
+const static struct msm_dsi_host_cfg_ops msm_dsi_6g_v2_host_ops = {
+   .link_clk_enable = dsi_link_clk_enable_6g,
+   .link_clk_disable = dsi_link_clk_disable_6g,
+   .clk_init_ver = dsi_clk_init_6g_v2,
+   .tx_buf_alloc = dsi_tx_buf_alloc_6g,
+   .tx_buf_get = dsi_tx_buf_get_6g,
+   .tx_buf_put = dsi_tx_buf_put_6g,
+   .dma_base_get = dsi_dma_base_get_6g,
+   .calc_clk_rate = dsi_calc_clk_rate_6g,
+};
+
  static const struct msm_dsi_cfg_handler dsi_cfg_handlers[] = {
-   {MSM_DSI_VER_MAJOR_V2, MSM_DSI_V2_VER_MINOR_8064, _dsi_cfg},
+   {MSM_DSI_VER_MAJOR_V2, MSM_DSI_V2_VER_MINOR_8064,
+   _dsi_cfg, _dsi_v2_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_0,
-   _apq8084_dsi_cfg},
+   _apq8084_dsi_cfg, _dsi_6g_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_1,
-   _apq8084_dsi_cfg},
+   _apq8084_dsi_cfg, _dsi_6g_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_1_1,
-   _apq8084_dsi_cfg},
+   _apq8084_dsi_cfg, _dsi_6g_host_ops},
{MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_2,
-   _apq8084_dsi_cfg},
-   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_3, _dsi_cfg},
-   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_3_1, _dsi_cfg},
-   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_4_1, _dsi_cfg},
-   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V2_2_1, _dsi_cfg},
+   _apq8084_dsi_cfg, _dsi_6g_host_ops},
+   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_3,
+   _dsi_cfg, _dsi_6g_host_ops},
+   {MSM_DSI_VER_MAJOR_6G, MSM_DSI_6G_VER_MINOR_V1_3_1,
+   _dsi_cfg, _dsi_6g_host_ops},
+   {MSM_DSI_VER_MAJOR_6G, 

[PATCH 6/8] drm/arm/malidp: Enable/disable the scaling engine interrupts with memory writeback

[Ayan Kumar Halder]

Scaling engine interrupts need to be enabled/disabled as and when memwrite
is enabled and disabled. The reason being scaling engine interrupts are
used only by the memory writeout layer.

This patch depends on:

"[Patch v5,1/3] drm: mali-dp: Add support for writeback on DP550/DP650"

Change-Id: Ic78aa5cd7b53998a1947067c4a15c19de239583b
Signed-off-by: Ayan Kumar Halder 
---
 drivers/gpu/drm/arm/malidp_hw.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index f5633bc..90d76e4 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -621,12 +621,14 @@ static int malidp550_enable_memwrite(struct 
malidp_hw_device *hwdev,
malidp_hw_setbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | 
MALIDP_SE_MEMWRITE_EN,
  MALIDP550_SE_CONTROL);
 
+   malidp_se_irq_hw_init(hwdev);
return 0;
 }
 
 static void malidp550_disable_memwrite(struct malidp_hw_device *hwdev)
 {
u32 base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK);
+   malidp_se_irq_fini(hwdev);
malidp_hw_clearbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | 
MALIDP_SE_MEMWRITE_EN,
MALIDP550_SE_CONTROL);
malidp_hw_clearbits(hwdev, MALIDP_SCALE_ENGINE_EN, base + 
MALIDP_DE_DISPLAY_FUNC);
-- 
2.7.4

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

[PATCH 8/8] drm/arm/malidp: Added the late system pm functions

[Ayan Kumar Halder]

malidp_pm_suspend_late checks if the runtime status is not suspended
and if so, invokes malidp_runtime_pm_suspend which disables the
display engine/core interrupts and the clocks. It sets the runtime status
as suspended. Subsequently, malidp_pm_resume_early will invoke
malidp_runtime_pm_resume which enables the clocks and the interrupts
(previously disabled) and sets the runtime status as active.

Signed-off-by: Ayan Kumar Halder 
Change-Id: I5f8c3d28f076314a1c9da2a46760a9c37039ccda
---
 drivers/gpu/drm/arm/malidp_drv.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index bd44a6d..f6124d8 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -766,8 +766,25 @@ static int __maybe_unused malidp_pm_resume(struct device 
*dev)
return 0;
 }
 
+static int __maybe_unused malidp_pm_suspend_late(struct device *dev)
+{
+   if (!pm_runtime_status_suspended(dev)) {
+   malidp_runtime_pm_suspend(dev);
+   pm_runtime_set_suspended(dev);
+   }
+   return 0;
+}
+
+static int __maybe_unused malidp_pm_resume_early(struct device *dev)
+{
+   malidp_runtime_pm_resume(dev);
+   pm_runtime_set_active(dev);
+   return 0;
+}
+
 static const struct dev_pm_ops malidp_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend, malidp_pm_resume) \
+   SET_LATE_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend_late, 
malidp_pm_resume_early) \
SET_RUNTIME_PM_OPS(malidp_runtime_pm_suspend, malidp_runtime_pm_resume, 
NULL)
 };
 
-- 
2.7.4

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

[PATCH 5/8] drm/arm/malidp: Enable/disable interrupts in runtime pm

[Ayan Kumar Halder]

Display engine and core interrupts need to be disabled when the
system invokes malidp_runtime_pm_suspend. Consequently, they
need to be enabled in malidp_runtime_pm_resume.

Signed-off-by: Ayan Kumar Halder 
Change-Id: Ib8e5e8319fdd768f8a97d9b5960fcfa8ba90eba3
---
 drivers/gpu/drm/arm/malidp_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index f7a8beb..e5a1fa0 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -470,6 +470,7 @@ static int malidp_runtime_pm_suspend(struct device *dev)
/* we can only suspend if the hardware is in config mode */
WARN_ON(!hwdev->hw->in_config_mode(hwdev));
 
+   malidp_de_irq_fini(hwdev);
hwdev->pm_suspended = true;
clk_disable_unprepare(hwdev->mclk);
clk_disable_unprepare(hwdev->aclk);
@@ -488,6 +489,7 @@ static int malidp_runtime_pm_resume(struct device *dev)
clk_prepare_enable(hwdev->aclk);
clk_prepare_enable(hwdev->mclk);
hwdev->pm_suspended = false;
+   malidp_de_irq_hw_init(hwdev);
 
return 0;
 }
-- 
2.7.4

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

[PATCH 3/8] drm/arm/malidp: Split malidp_de_irq_init

[Ayan Kumar Halder]

Extract the hardware initialisation part from malidp_de_irq_init() into the
malidp_de_irq_hw_init() which will be later invoked from runtime_pm_resume
function when it needs to re-enable the interrupts.

Change-Id: If8bdb0e246653cb7d7b7d6d63919c45b01350c10
Signed-off-by: Ayan Kumar Halder 
---
 drivers/gpu/drm/arm/malidp_hw.c | 25 ++---
 drivers/gpu/drm/arm/malidp_hw.h |  1 +
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 8fb02f3..3e73370 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -869,6 +869,23 @@ static irqreturn_t malidp_de_irq_thread_handler(int irq, 
void *arg)
return IRQ_HANDLED;
 }
 
+void malidp_de_irq_hw_init(struct malidp_hw_device *hwdev)
+{
+   /* ensure interrupts are disabled */
+   malidp_hw_disable_irq(hwdev, MALIDP_DE_BLOCK, 0x);
+   malidp_hw_clear_irq(hwdev, MALIDP_DE_BLOCK, 0x);
+   malidp_hw_disable_irq(hwdev, MALIDP_DC_BLOCK, 0x);
+   malidp_hw_clear_irq(hwdev, MALIDP_DC_BLOCK, 0x);
+
+   /* first enable the DC block IRQs */
+   malidp_hw_enable_irq(hwdev, MALIDP_DC_BLOCK,
+hwdev->hw->map.dc_irq_map.irq_mask);
+
+   /* now enable the DE block IRQs */
+   malidp_hw_enable_irq(hwdev, MALIDP_DE_BLOCK,
+hwdev->hw->map.de_irq_map.irq_mask);
+}
+
 int malidp_de_irq_init(struct drm_device *drm, int irq)
 {
struct malidp_drm *malidp = drm->dev_private;
@@ -889,13 +906,7 @@ int malidp_de_irq_init(struct drm_device *drm, int irq)
return ret;
}
 
-   /* first enable the DC block IRQs */
-   malidp_hw_enable_irq(hwdev, MALIDP_DC_BLOCK,
-hwdev->hw->map.dc_irq_map.irq_mask);
-
-   /* now enable the DE block IRQs */
-   malidp_hw_enable_irq(hwdev, MALIDP_DE_BLOCK,
-hwdev->hw->map.de_irq_map.irq_mask);
+   malidp_de_irq_hw_init(hwdev);
 
return 0;
 }
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 6607aba..3b049d0 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -297,6 +297,7 @@ static inline void malidp_hw_enable_irq(struct 
malidp_hw_device *hwdev,
 }
 
 int malidp_de_irq_init(struct drm_device *drm, int irq);
+void malidp_de_irq_hw_init(struct malidp_hw_device *hwdev);
 void malidp_de_irq_fini(struct malidp_hw_device *hwdev);
 int malidp_se_irq_init(struct drm_device *drm, int irq);
 void malidp_se_irq_fini(struct malidp_hw_device *hwdev);
-- 
2.7.4

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

Re: [PATCH] drm/scheduler: remove incorrect param documentation

[Nayan Deshmukh]

I will send in the patches.

On Mon, Mar 26, 2018 at 10:29 PM, Christian König
 wrote:
> Yeah, probably good idea to have that separate.
>
> But would still be a nice cleanup and it is merged upstream after the
> etnaviv patches anyway.
>
> Regards,
> Christian.
>
>
> Am 26.03.2018 um 17:29 schrieb Nayan Deshmukh:
>>
>> I am not removing jobs parameters as of now as it will lead to changes
>> in all the calls to this function and will affect the etnaviv patches.
>>
>> Regards,
>> Nayan.
>>
>> On Sun, Mar 25, 2018 at 4:59 PM, Christian König
>>  wrote:
>>>
>>> Am 25.03.2018 um 13:21 schrieb Nayan Deshmukh:

 On Sun, Mar 25, 2018 at 4:44 PM, Christian König
  wrote:
>
> Am 25.03.2018 um 13:09 schrieb Nayan Deshmukh:
>>
>> Signed-off-by: Nayan Deshmukh 
>> ---
>> drivers/gpu/drm/scheduler/gpu_scheduler.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
>> b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>> index 0d95888ccc3e..27fdda1264f7 100644
>> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
>> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
>> @@ -117,7 +117,6 @@ drm_sched_rq_select_entity(struct drm_sched_rq
>> *rq)
>>  * @sched The pointer to the scheduler
>>  * @entityThe pointer to a valid drm_sched_entity
>>  * @rqThe run queue this entity belongs
>> - * @kernel If this is an entity for the kernel
>>  * @jobs  The max number of jobs in the job queue
>
>
> If I'm not completely mistaken the jobs parameter is unused and we also
> have
> a @guilty parameter as well.
>
 Yes. But I am not sure what should be done with the jobs parameter.
 Should I remove it entirely?
>>>
>>>
>>> Yes, probably a leftover from when we used a kfifo for the queue.
>>>
>>> Regards,
>>> Christian.
>>>
>>>
> The description for the @guilty should be something like "atomic_t set
> to
> 1
> when a job on this queue is found to be guilty causing a timeout".
>
> Regards,
> Christian.
>
>
>>  *
>>  * return 0 if succeed. negative error code on failure
>
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/8] drm/arm/malidp: Split malidp_se_irq_init

[Ayan Kumar Halder]

Extract the hardware initialisation part from malidp_se_irq_init() into the
malidp_se_irq_hw_init() which will be later invoked from
malidpxxx_enable_memwrite() when it needs to re-enable the interrupts.

Signed-off-by: Ayan Kumar Halder 
Change-Id: Ibb26e86b38141993539307705695e3f6a9e32caa
---
 drivers/gpu/drm/arm/malidp_hw.c | 14 --
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 3e73370..f5633bc 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -163,6 +163,7 @@ static const u16 
dp500_se_scaling_coeffs[][SE_N_SCALING_COEFFS] = {
 };
 
 #define MALIDP_DE_DEFAULT_PREFETCH_START   5
+static void malidp_se_irq_hw_init(struct malidp_hw_device *hwdev);
 
 static int malidp500_query_hw(struct malidp_hw_device *hwdev)
 {
@@ -952,6 +953,16 @@ static irqreturn_t malidp_se_irq(int irq, void *arg)
return IRQ_HANDLED;
 }
 
+static void malidp_se_irq_hw_init(struct malidp_hw_device *hwdev)
+{
+   /* ensure interrupts are disabled */
+   malidp_hw_disable_irq(hwdev, MALIDP_SE_BLOCK, 0x);
+   malidp_hw_clear_irq(hwdev, MALIDP_SE_BLOCK, 0x);
+
+   malidp_hw_enable_irq(hwdev, MALIDP_SE_BLOCK,
+hwdev->hw->map.se_irq_map.irq_mask);
+}
+
 static irqreturn_t malidp_se_irq_thread_handler(int irq, void *arg)
 {
return IRQ_HANDLED;
@@ -975,8 +986,7 @@ int malidp_se_irq_init(struct drm_device *drm, int irq)
return ret;
}
 
-   malidp_hw_enable_irq(hwdev, MALIDP_SE_BLOCK,
-hwdev->hw->map.se_irq_map.irq_mask);
+   malidp_se_irq_hw_init(hwdev);
 
return 0;
 }
-- 
2.7.4

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

[PATCH 7/8] drm/arm/malidp: Set the output_depth register in modeset

[Ayan Kumar Halder]

One needs to store the value of the OUTPUT_DEPTH that one has parsed from
device tree, so that it can be restored on system resume. This value is
set in the modeset function as this gets reset when the system suspends.

Signed-off-by: Ayan Kumar Halder 
Change-Id: I361b1214cd4e5005d21eef3ca6bf39ca90be2506
---
 drivers/gpu/drm/arm/malidp_drv.c | 1 +
 drivers/gpu/drm/arm/malidp_hw.c  | 4 
 drivers/gpu/drm/arm/malidp_hw.h  | 1 +
 3 files changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index e5a1fa0..bd44a6d 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -601,6 +601,7 @@ static int malidp_bind(struct device *dev)
for (i = 0; i < MAX_OUTPUT_CHANNELS; i++)
out_depth = (out_depth << 8) | (output_width[i] & 0xf);
malidp_hw_write(hwdev, out_depth, hwdev->hw->map.out_depth_base);
+   hwdev->output_color_depth = out_depth;
 
atomic_set(>config_valid, 0);
init_waitqueue_head(>wq);
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index 90d76e4..1bf10fb 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -234,6 +234,8 @@ static void malidp500_modeset(struct malidp_hw_device 
*hwdev, struct videomode *
 {
u32 val = 0;
 
+   malidp_hw_write(hwdev, hwdev->output_color_depth,
+   hwdev->hw->map.out_depth_base);
malidp_hw_clearbits(hwdev, MALIDP500_DC_CLEAR_MASK, 
MALIDP500_DC_CONTROL);
if (mode->flags & DISPLAY_FLAGS_HSYNC_HIGH)
val |= MALIDP500_HSYNCPOL;
@@ -458,6 +460,8 @@ static void malidp550_modeset(struct malidp_hw_device 
*hwdev, struct videomode *
 {
u32 val = MALIDP_DE_DEFAULT_PREFETCH_START;
 
+   malidp_hw_write(hwdev, hwdev->output_color_depth,
+   hwdev->hw->map.out_depth_base);
malidp_hw_write(hwdev, val, MALIDP550_DE_CONTROL);
/*
 * Mali-DP550 and Mali-DP650 encode the background color like this:
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 3b049d0..844732d 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -228,6 +228,7 @@ struct malidp_hw_device {
 
u8 min_line_size;
u16 max_line_size;
+   u32 output_color_depth;
 
/* track the device PM state */
bool pm_suspended;
-- 
2.7.4

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

[PATCH 0/8] drm/arm/malidp: Enhance support for system and runtime power management on malidp.

[Ayan Kumar Halder]

This patch series enhances and fixes certain issues relevant to system and
runtime power management on malidp.

Ayan Kumar Halder (8):
  drm/arm/malidp: Modified the prototype of malidp_de_irq_fini
  drm/arm/malidp: Modified the prototype of malidp_se_irq_fini
  drm/arm/malidp: Split malidp_de_irq_init
  drm/arm/malidp: Split malidp_se_irq_init
  drm/arm/malidp: Enable/disable interrupts in runtime pm
  drm/arm/malidp: Enable/disable the scaling engine interrupts with
memory writeback
  drm/arm/malidp: Set the output_depth register in modeset
  drm/arm/malidp: Added the late system pm functions

 drivers/gpu/drm/arm/malidp_drv.c | 33 
 drivers/gpu/drm/arm/malidp_hw.c  | 55 +++-
 drivers/gpu/drm/arm/malidp_hw.h  |  6 +++--
 3 files changed, 70 insertions(+), 24 deletions(-)

-- 
2.7.4

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

[PATCH 2/8] drm/arm/malidp: Modified the prototype of malidp_se_irq_fini

[Ayan Kumar Halder]

'struct drm_device' is being replaced with 'struct malidp_hw_device'
as the function argument.The reason being the dependency of
malidp_se_irq_fini on 'struct drm_device' needs to be removed so as to
enable it to call from functions which receives 'struct malidp_hw_device'
as argument. Furthermore, there is no way to retrieve 'struct drm_device'
from 'struct malidp_hw_device'

Change-Id: Iab7ac99917f0faf739aee97b00e1758ad1ae787b
Signed-off-by: Ayan Kumar Halder 
---
 drivers/gpu/drm/arm/malidp_drv.c | 4 ++--
 drivers/gpu/drm/arm/malidp_hw.c  | 5 +
 drivers/gpu/drm/arm/malidp_hw.h  | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index ed38ba9..f7a8beb 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -653,7 +653,7 @@ static int malidp_bind(struct device *dev)
 fbdev_fail:
pm_runtime_get_sync(dev);
 vblank_fail:
-   malidp_se_irq_fini(drm);
+   malidp_se_irq_fini(hwdev);
malidp_de_irq_fini(hwdev);
drm->irq_enabled = false;
 irq_init_fail:
@@ -690,7 +690,7 @@ static void malidp_unbind(struct device *dev)
drm_kms_helper_poll_fini(drm);
pm_runtime_get_sync(dev);
drm_crtc_vblank_off(>crtc);
-   malidp_se_irq_fini(drm);
+   malidp_se_irq_fini(hwdev);
malidp_de_irq_fini(hwdev);
drm->irq_enabled = false;
component_unbind_all(dev, drm);
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index b13dfac..8fb02f3 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -970,11 +970,8 @@ int malidp_se_irq_init(struct drm_device *drm, int irq)
return 0;
 }
 
-void malidp_se_irq_fini(struct drm_device *drm)
+void malidp_se_irq_fini(struct malidp_hw_device *hwdev)
 {
-   struct malidp_drm *malidp = drm->dev_private;
-   struct malidp_hw_device *hwdev = malidp->dev;
-
malidp_hw_disable_irq(hwdev, MALIDP_SE_BLOCK,
  hwdev->hw->map.se_irq_map.irq_mask);
 }
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index 6e2a2f6..6607aba 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -299,7 +299,7 @@ static inline void malidp_hw_enable_irq(struct 
malidp_hw_device *hwdev,
 int malidp_de_irq_init(struct drm_device *drm, int irq);
 void malidp_de_irq_fini(struct malidp_hw_device *hwdev);
 int malidp_se_irq_init(struct drm_device *drm, int irq);
-void malidp_se_irq_fini(struct drm_device *drm);
+void malidp_se_irq_fini(struct malidp_hw_device *hwdev);
 
 u8 malidp_hw_get_format_id(const struct malidp_hw_regmap *map,
   u8 layer_id, u32 format);
-- 
2.7.4

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

[PATCH 1/8] drm/arm/malidp: Modified the prototype of malidp_de_irq_fini

[Ayan Kumar Halder]

'struct drm_device' is being replaced with 'struct malidp_hw_device'
as the function argument. The reason being the dependency of
malidp_de_irq_fini on 'struct drm_device' needs to be removed so as to
enable it to call from functions which receives 'struct malidp_hw_device'
as argument. Furthermore, there is no way to retrieve 'struct drm_device'
from 'struct malidp_hw_device'.

Change-Id: I39c38cc4c0c9dd951777fbcb13e2ee3168ea0141
Signed-off-by: Ayan Kumar Halder 
---
 drivers/gpu/drm/arm/malidp_drv.c | 9 ++---
 drivers/gpu/drm/arm/malidp_hw.c  | 5 +
 drivers/gpu/drm/arm/malidp_hw.h  | 2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index 4b0c4b4..ed38ba9 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -295,6 +295,8 @@ static int malidp_irq_init(struct platform_device *pdev)
 {
int irq_de, irq_se, ret = 0;
struct drm_device *drm = dev_get_drvdata(>dev);
+   struct malidp_drm *malidp = drm->dev_private;
+   struct malidp_hw_device *hwdev = malidp->dev;
 
/* fetch the interrupts from DT */
irq_de = platform_get_irq_byname(pdev, "DE");
@@ -314,7 +316,7 @@ static int malidp_irq_init(struct platform_device *pdev)
 
ret = malidp_se_irq_init(drm, irq_se);
if (ret) {
-   malidp_de_irq_fini(drm);
+   malidp_de_irq_fini(hwdev);
return ret;
}
 
@@ -652,7 +654,7 @@ static int malidp_bind(struct device *dev)
pm_runtime_get_sync(dev);
 vblank_fail:
malidp_se_irq_fini(drm);
-   malidp_de_irq_fini(drm);
+   malidp_de_irq_fini(hwdev);
drm->irq_enabled = false;
 irq_init_fail:
component_unbind_all(dev, drm);
@@ -681,6 +683,7 @@ static void malidp_unbind(struct device *dev)
 {
struct drm_device *drm = dev_get_drvdata(dev);
struct malidp_drm *malidp = drm->dev_private;
+   struct malidp_hw_device *hwdev = malidp->dev;
 
drm_dev_unregister(drm);
drm_fb_cma_fbdev_fini(drm);
@@ -688,7 +691,7 @@ static void malidp_unbind(struct device *dev)
pm_runtime_get_sync(dev);
drm_crtc_vblank_off(>crtc);
malidp_se_irq_fini(drm);
-   malidp_de_irq_fini(drm);
+   malidp_de_irq_fini(hwdev);
drm->irq_enabled = false;
component_unbind_all(dev, drm);
of_node_put(malidp->crtc.port);
diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index e4d9ebc..b13dfac 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -900,11 +900,8 @@ int malidp_de_irq_init(struct drm_device *drm, int irq)
return 0;
 }
 
-void malidp_de_irq_fini(struct drm_device *drm)
+void malidp_de_irq_fini(struct malidp_hw_device *hwdev)
 {
-   struct malidp_drm *malidp = drm->dev_private;
-   struct malidp_hw_device *hwdev = malidp->dev;
-
malidp_hw_disable_irq(hwdev, MALIDP_DE_BLOCK,
  hwdev->hw->map.de_irq_map.irq_mask);
malidp_hw_disable_irq(hwdev, MALIDP_DC_BLOCK,
diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h
index a242e97..6e2a2f6 100644
--- a/drivers/gpu/drm/arm/malidp_hw.h
+++ b/drivers/gpu/drm/arm/malidp_hw.h
@@ -297,7 +297,7 @@ static inline void malidp_hw_enable_irq(struct 
malidp_hw_device *hwdev,
 }
 
 int malidp_de_irq_init(struct drm_device *drm, int irq);
-void malidp_de_irq_fini(struct drm_device *drm);
+void malidp_de_irq_fini(struct malidp_hw_device *hwdev);
 int malidp_se_irq_init(struct drm_device *drm, int irq);
 void malidp_se_irq_fini(struct drm_device *drm);
 
-- 
2.7.4

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

[Bug 105747] TOPAZ: ring buffers not getting initialized with the amd-staging-drm-next branch

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105747

Alex Deucher  changed:

   What|Removed |Added

 Resolution|--- |NOTABUG
 Status|NEW |RESOLVED

--- Comment #4 from Alex Deucher  ---
The driver will print an error if any of the ring or IB tests fail.  It just
doesn't print anything if they succeed. You can adjust the debug level by
adjusting the debug module parameter for the drm.ko module.

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

[Bug 91808] trine1 misrender r600g

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=91808

--- Comment #8 from i...@yahoo.com ---
This bug is still present with current Mesa-18.0.0.rc5.

Since the old trace is not available anymore I've made a new one(70MB):

https://drive.google.com/open?id=16N0m--IKupnWvnuvu27uebBjbDHUP3C4

I used "llvmpipe" during capture and the result also plays perfectly with
"llvmpipe" (aka Mesa3D software render).

The bug is visible in the rotating gears/cogs. I tried "R600_DEBUG=nosb", but
this does not seem to fix it.

I tried to find the draw operations. Frame #175 seems to show the artifact
quite clearly. The draw operations at call #8053345 seems to draw a small cog
that looks normal.
I believe that the smeared gear is drawn by call #8053560.


My hardware is Radeon HD5670 Redwood Evergreen (R600 driver).

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

Re: [PATCH] drm/scheduler: remove incorrect param documentation

[Christian König]

Yeah, probably good idea to have that separate.

But would still be a nice cleanup and it is merged upstream after the 
etnaviv patches anyway.


Regards,
Christian.

Am 26.03.2018 um 17:29 schrieb Nayan Deshmukh:

I am not removing jobs parameters as of now as it will lead to changes
in all the calls to this function and will affect the etnaviv patches.

Regards,
Nayan.

On Sun, Mar 25, 2018 at 4:59 PM, Christian König
 wrote:

Am 25.03.2018 um 13:21 schrieb Nayan Deshmukh:

On Sun, Mar 25, 2018 at 4:44 PM, Christian König
 wrote:

Am 25.03.2018 um 13:09 schrieb Nayan Deshmukh:

Signed-off-by: Nayan Deshmukh 
---
drivers/gpu/drm/scheduler/gpu_scheduler.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 0d95888ccc3e..27fdda1264f7 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -117,7 +117,6 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
 * @sched The pointer to the scheduler
 * @entityThe pointer to a valid drm_sched_entity
 * @rqThe run queue this entity belongs
- * @kernel If this is an entity for the kernel
 * @jobs  The max number of jobs in the job queue


If I'm not completely mistaken the jobs parameter is unused and we also
have
a @guilty parameter as well.


Yes. But I am not sure what should be done with the jobs parameter.
Should I remove it entirely?


Yes, probably a leftover from when we used a kfifo for the queue.

Regards,
Christian.



The description for the @guilty should be something like "atomic_t set to
1
when a job on this queue is found to be guilty causing a timeout".

Regards,
Christian.



 *
 * return 0 if succeed. negative error code on failure




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

Re: [PATCH] drm/scheduler: fix param documentation

[Christian König]

A bit more commit message would be nice to have, like "There is no more 
@kernel parameter and document the new @guilty parameter".


Am 26.03.2018 um 17:21 schrieb Nayan Deshmukh:

Signed-off-by: Nayan Deshmukh 


With the commit message fixed the patch is Reviewed-by: Christian König 
.


Christian.


---
  drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c 
b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 0d95888ccc3e..1d368bc66ac2 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -117,8 +117,9 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
   * @sched The pointer to the scheduler
   * @entityThe pointer to a valid drm_sched_entity
   * @rqThe run queue this entity belongs
- * @kernel If this is an entity for the kernel
   * @jobs  The max number of jobs in the job queue
+ * @guilty  atomic_t set to 1 when a job on this queue
+ *  is found to be guilty causing a timeout
   *
   * return 0 if succeed. negative error code on failure
  */


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

[Bug 105747] TOPAZ: ring buffers not getting initialized with the amd-staging-drm-next branch

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105747

--- Comment #3 from Nayan Deshmukh  ---
Oh..I am seeing any bug as such, I thought the ring buffers are getting
initialized. How can I get the verbose output to confirm that everything is
working fine?

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

Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Noralf Trønnes]

Den 26.03.2018 15.02, skrev Oleksandr Andrushchenko:

Hi, Noralf!

On 03/17/2018 04:40 PM, Noralf Trønnes wrote:


Den 16.03.2018 09.03, skrev Daniel Vetter:

On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter  wrote:

Hi Noralf,

On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:

This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
(fbdev) and tinydrm.

There are several changes in this version:

I've used Daniel's idea of protecting drm_device.unplugged with 
srcu to

provide race free drm_dev_unplug().

The fbdev helper unplug patch has become very small with Daniel's 
help.

Ref is now taken and dropped in the existing helpers, so I could drop
drm_fb_helper_simple_init().

I has annoyed me that fbdev is restored in drm_driver.last_close 
even if
fbdev isn't used. I've added a patch to fix that. This means I can 
drop

calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
now can easily disable the pipeline from userspace by just closing 
the

users. Disabled pipeline means balanced regulator_enable/disable.

The 'Embed drm_device in tinydrm_device' patch has gained
drm_driver.release functions after a discussion with Laurent. My
previous version relied on obscure freeing in tinydrm_release().
This means that I didn't retain the ack's.

Laurent also caught an ugly devm_kmalloc() in
tinydrm_display_pipe_init() that I've fixed.
I'm practically packing for my 2 weeks of conference travel 
already, so
only very cursory read of the initial patches for core 
I think

this looks really splendid now.

But I won't have time for review for the next few week, would be 
good if
you could drag some others into this discussions. Iirc there's 
recently
been a few different people interested in udl (even some patches I 
think),

they might be able to help out with testing

Also, would be great if you can submit this to intel-gfx on the next
round, so that our CI can pick it up and give it a solid beating. 
Touching

critical core paths like in patch 1 is always a bit scary.

While reviewing xen-front's hotunplug handling I just realized this
never landed. Can you pls resend and nag me to review it properly? I'd
really like to get the drm_dev_unplug stuff sorted out better.


My plan was to pick this up after switching tinydrm over to vmalloc 
buffers,

but that work is now waiting for the generic fbdev emulation to land.

I'm currently wandering around inside drm_fb_helper looking for a way 
out,
I can feel the draft so there has to be an exit somewhere. I hope 
that in

a week or two I'm done with the next version of the RFC using the
drm_fb_helper mode setting code instead of the ioctl's.

At that point it will be a good thing to flush my "caches" of the
drm_fb_helper code, since after looking at it for a long time, I really
don't see the details anymore. So I'll pick up the unplug series 
then, at
least the core patches should be trivial to review and get merged if 
the CI

agrees.

Could you please estimate when unplug series can be on review?
I am doing some unplug work in my PV DRM driver and would like
to understand if it is feasible to wait for you or post patches as 
they are

and then plumb in drm_dev_{enter|exit} later after your work is merged



Ok, so I have looked at the patches and some work I have lying around.
As things stand now I see an opportunity to move some stuff from tinydrm
into drm_simple_kms_helper as part of adding unplug support to tinydrm.
This also depends on the generic fbdev emulation I'm working on.
This all means that it won't be some trivial tweaking to the unplug
patchset before resending it. It will take some time.

My suggestion is that you just add the core unplug patch as part of your
driver submission instead of waiting for me.

drm: Use srcu to protect drm_device.unplugged
https://patchwork.freedesktop.org/patch/175779/

I believe this patch should be good to go as-is. Please CC
intel-...@lists.freedesktop.org if you do so to have the Intel CI verify it.

As for drm_fb_helper unplug support:

drm/fb-helper: Support device unplug
https://patchwork.freedesktop.org/patch/175785/

I'm not as confident in this one since I'm not sure that those
drm_dev_is_unplugged() checks are really necessary. The driver has to do
the check anyways. But this patch isn't necessary for you to do most of
your driver side unplug protection though. You can do testing without
fbdev enabled and let me to care of this when I get around to it.

I you pick up the patch(es) and need to change something, you don't have
to bother with retaining my authorship (but please cc me). Just claim it
for your own and make it work.
Less work for me when I get there (eventually).

Noralf.


Thank you,
Oleksandr


Noralf.


Thanks, Daniel


Thanks, Daniel


Noralf.

Noralf Trønnes (11):
   drm: Use srcu to protect drm_device.unplugged
   drm/fb-helper: Support device unplug
   drm/fb-helper: Ensure driver module is pinned in fb_open()
   drm/fb-helper: Don't 

[Patch v2 5/6] drm/omap: Add virtual plane support to omap_plane

[Benoit Parrot]

Add virtual wide plane support by adding an secondary
plane_id so that an "omap_plane" can be composed of up to
two physical planes.

When at least one 'plane' child node is present in DT then
omap_plane_init will only use the plane described in DT.
Some of these nodes may be a virtual wide plane if they are defined
as two physical planes.
Planes can also be associated with various crtcs independently.
Therefore we can restrict the use of virtual plane to specific
CRTC/video port if need be, if crtc_mask is not specified then
the plane will be available to all available crtcs.
Physical planes which are not described will essentially be hidden
from the driver.

If no 'plane' child nodes exist then the normal plane
allocation will take place.

Signed-off-by: Benoit Parrot 
---
 drivers/gpu/drm/omapdrm/omap_drv.c   | 127 +++--
 drivers/gpu/drm/omapdrm/omap_fb.c|  66 ++-
 drivers/gpu/drm/omapdrm/omap_fb.h|   4 +-
 drivers/gpu/drm/omapdrm/omap_plane.c | 151 ++-
 drivers/gpu/drm/omapdrm/omap_plane.h |   4 +-
 5 files changed, 283 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c 
b/drivers/gpu/drm/omapdrm/omap_drv.c
index 37ee20c773c7..4c43ef239136 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -16,6 +16,7 @@
  */
 
 #include 
+#include 
 
 #include 
 #include 
@@ -116,6 +117,112 @@ static void omap_atomic_commit_tail(struct 
drm_atomic_state *old_state)
priv->dispc_ops->runtime_put();
 }
 
+static int omap_atomic_state_zpos_cmp(const void *a, const void *b)
+{
+   const struct drm_plane_state *sa = *(struct drm_plane_state **)a;
+   const struct drm_plane_state *sb = *(struct drm_plane_state **)b;
+
+   if (sa->zpos != sb->zpos)
+   return sa->zpos - sb->zpos;
+   else
+   return sa->plane->base.id - sb->plane->base.id;
+}
+
+static int omap_atomic_crtc_normalize_zpos(struct drm_crtc *crtc,
+  struct drm_crtc_state *crtc_state)
+{
+   struct drm_atomic_state *state = crtc_state->state;
+   struct drm_device *dev = crtc->dev;
+   int total_planes = dev->mode_config.num_total_plane;
+   struct drm_plane_state **states;
+   struct drm_plane *plane;
+   int i, inc, n = 0;
+   int ret = 0;
+
+   DRM_DEBUG_ATOMIC("[CRTC:%d:%s] calculating normalized zpos values\n",
+crtc->base.id, crtc->name);
+
+   states = kmalloc_array(total_planes, sizeof(*states), GFP_KERNEL);
+   if (!states)
+   return -ENOMEM;
+
+   /*
+* Normalization process might create new states for planes which
+* normalized_zpos has to be recalculated.
+*/
+   drm_for_each_plane_mask(plane, dev, crtc_state->plane_mask) {
+   struct drm_plane_state *plane_state =
+   drm_atomic_get_plane_state(state, plane);
+   if (IS_ERR(plane_state)) {
+   ret = PTR_ERR(plane_state);
+   goto done;
+   }
+   states[n++] = plane_state;
+   DRM_DEBUG_ATOMIC("[PLANE:%d:%s] processing zpos value %d\n",
+plane->base.id, plane->name,
+plane_state->zpos);
+   }
+
+   sort(states, n, sizeof(*states), omap_atomic_state_zpos_cmp, NULL);
+
+   for (inc = 0, i = 0; i < n; i++) {
+   plane = states[i]->plane;
+
+   states[i]->normalized_zpos = i + inc;
+   DRM_DEBUG_ATOMIC("[PLANE:%d:%s] normalized zpos value %d\n",
+plane->base.id, plane->name, i + inc);
+   /*
+* If the current plane is virtual it uses 2 hw planes
+* therefore the very next zpos is used by the secondary/aux
+* plane so we need to skip one zpos from this point on.
+*/
+   if (is_omap_plane_virtual(plane))
+   inc++;
+   }
+   crtc_state->zpos_changed = true;
+
+done:
+   kfree(states);
+   return ret;
+}
+
+static int omap_atomic_normalize_zpos(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+   struct drm_crtc *crtc;
+   struct drm_crtc_state *old_crtc_state, *new_crtc_state;
+   int i, ret = 0;
+
+   for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, 
new_crtc_state, i) {
+   if (old_crtc_state->plane_mask != new_crtc_state->plane_mask ||
+   new_crtc_state->zpos_changed) {
+   ret = omap_atomic_crtc_normalize_zpos(crtc,
+ new_crtc_state);
+   if (ret)
+   return ret;
+   }
+   }
+   return 0;
+}
+
+static int 

[Patch v2 3/6] dt-bindings: display/ti: Add plane binding to dispc node

[Benoit Parrot]

Currently all available display pipelines (i.e. plane) and output port
resources are exposed to user-space. In some cases it is needed to be
able to restrict which resources are actually visible from user-space.
Also in cases where a display wider than 2048 pixels is to be supported
more than one video pipeline is needed. In this case the 2nd hardware
pipeline needed is not visible to user space applications.

These video pipeline definitions must be statically defined so that
the number of visible pipelines does not change from the user-space
perspective.

In order to allow this we are adding an optional 'plane' sub-node to
the generic DISPC node.

Signed-off-by: Benoit Parrot 
---
 .../devicetree/bindings/display/ti/ti,omap-dss.txt | 65 ++
 1 file changed, 65 insertions(+)

diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt 
b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
index 249e588d7865..2dd411cb5a83 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
@@ -28,6 +28,36 @@ Optional properties:
 - max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth 
limit
in bytes per second
 
+Optional Subnode:
+- plane: Child node(s) which defines which video planes are available to
+   the system. If at least one plane child node is defined then
+   only planes defined by these nodes will be available to the system.
+   Plane nodes must be sequential starting with reg = <0> as DT parsing
+   will stop on the first missing numbered node.
+   This means if plane #1 is defined but plane #0 is not then it will
+   be as if none of the plane nodes were defined.
+
+   Each plane node contains the following properties:
+   Required properties:
+   - reg:   Used to identify the plane
+   - video-pipelines: One or two HW pipeline number(s).
+   When 2 numbers are present this indicates a virtual wide
+   plane composed of two physical planes intended to be used
+   when the display is larger then the capacity of a
+   single plane i.e. wider than 2048 pixels.
+   The first number in the pair will dictate the capabilities
+   of the plane. This means that for proper
+   operation the virtual plane should be composed of HW
+   planes of the same capabilities.
+   If GFX pipeline is used in a virtual plane it should be
+   specified first, otherwise unexpected behavior would
+   be encountered.
+   Optional property:
+   - video-outputs:  One or more HW output number(s).
+   Describe the list of video output on which this plane
+   is available. If this node is not present then the
+   plane will be available on all available video output.
+
 Video Ports
 ---
 
@@ -216,3 +246,38 @@ OMAP HDMI --(HDMI)--> TPD12S015 --(HDMI)--> HDMI Connector
};
};
 };
+
+A short example on how to define a virtual plane configuration
+to enable wide display support.
+Here we define:
+- plane#0 to be the HW pipeline #0 (i.e. GFX pipeline)
+ only available on video output #0
+- plane#1 to be a virtual wide plane composed of HW pipeline #1 and #2
+ (i.e. VID1 & VID2) available on video output #0 & #1
+- plane#2 to be the HW pipeline #3 (i.e. VID3 pipeline)
+ only available on video output #0
+
+ {
+dispc@58001000 {
+#address-cells = <1>;
+#size-cells = <0>;
+
+plane@0 {
+reg = <0>;
+video-pipelines = <0>;
+video-outputs = <0>;
+};
+
+plane@1 {
+reg = <1>;
+video-pipelines = <1 2>;
+video-outputs = <0 1>;
+};
+
+plane@2 {
+reg = <2>;
+video-pipelines = <3>;
+video-outputs = <0>;
+};
+};
+};
-- 
2.9.0

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

[Patch v2 4/6] drm/omap: Add virtual plane DT parsing support

[Benoit Parrot]

Virtual planes are used to extend display size capability for display
larger than 2048 pixels by splitting the frame buffer equally between
two physical video-pipelines.

Here we are adding DT support to parse 'plane' child nodes which
describe how logical planes are mapped to physical video-pipeline(s)
and which video-outputs they are available on.

Signed-off-by: Benoit Parrot 
---
 drivers/gpu/drm/omapdrm/dss/dispc.c   | 110 ++
 drivers/gpu/drm/omapdrm/dss/omapdss.h |  11 
 2 files changed, 121 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 35541d4441df..06a2e894175e 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -4360,6 +4360,115 @@ static u32 dispc_get_memory_bandwidth_limit(void)
return limit;
 }
 
+static struct device_node *dispc_of_get_plane_by_id(struct device_node *node,
+   u32 id)
+{
+   struct device_node *plane;
+
+   for_each_child_of_node(node, plane) {
+   u32 plane_id = 0;
+
+   if (of_node_cmp(plane->name, "plane") != 0)
+   continue;
+   of_property_read_u32(plane, "reg", _id);
+   if (id == plane_id)
+   return plane;
+   }
+
+   return NULL;
+}
+
+static int dispc_parse_dt_plane_data(struct dispc_plane_mappings *map)
+{
+   struct platform_device *pdev = dispc.pdev;
+   struct device_node *np = pdev->dev.of_node;
+   struct device_node *ep;
+   struct property *prop;
+   const __be32 *cur;
+   u32 v;
+   u32 num_ovls = dispc_get_num_ovls();
+   u32 hw_plane_mask = 0;
+   u32 num_planes;
+   int i, index;
+
+   if (!np)
+   return 0;
+
+   for (i = 0; i < num_ovls; i++) {
+   ep = dispc_of_get_plane_by_id(np, i);
+   if (!ep)
+   break;
+   if (!of_property_read_bool(ep, "video-pipelines")) {
+   dev_err(>dev,
+   "malformed plane node: video-pipelines 
missing.\n");
+   return -EINVAL;
+   }
+
+   index = 0;
+   of_property_for_each_u32(ep, "video-pipelines", prop, cur, v) {
+   if (v >= num_ovls) {
+   dev_err(>dev,
+   "video-pipelines property: '%d' 
out-of-range.\n",
+   v);
+   return -EINVAL;
+   }
+   if (hw_plane_mask & BIT_MASK(v)) {
+   dev_err(>dev,
+   "video-pipelines property: '%d' used 
more than once.\n",
+   v);
+   return -EINVAL;
+   }
+   hw_plane_mask |= BIT(v);
+
+   if (index == 0) {
+   map->plane[i].main_id = v;
+   } else if (index == 1) {
+   map->plane[i].aux_id = v;
+   map->plane[i].is_virtual = true;
+   } else if (index > 1) {
+   dev_err(>dev,
+   "video-pipelines property: more than 2 
values specified.\n");
+   return -EINVAL;
+   }
+   index++;
+   }
+
+   of_property_for_each_u32(ep, "video-outputs", prop, cur, v) {
+   if (v >= dispc_get_num_mgrs()) {
+   dev_err(>dev,
+   "video-outputs property: '%d' 
out-of-range.\n",
+   v);
+   return -EINVAL;
+   }
+   map->plane[i].crtc_mask |= BIT(v);
+   }
+   }
+
+   num_planes = i;
+
+   if (num_planes) {
+   dev_dbg(>dev, "Plane definitions found from DT:");
+   for (i = 0; i < num_planes; i++) {
+   if (map->plane[i].is_virtual) {
+   dev_dbg(>dev,
+   "plane%d: virtual video-pipelines: %d, 
%d video-output mask: 0x%04x",
+   i, map->plane[i].main_id,
+   map->plane[i].aux_id,
+   map->plane[i].crtc_mask);
+   } else {
+   dev_dbg(>dev,
+   "plane%d: video-pipelines: %d 
video-output mask: 0x%04x",
+   i, map->plane[i].main_id,
+   

[Patch v2 2/6] dt-bindings: display/ti: Move common dispc bindings to omap-dss.txt

[Benoit Parrot]

Add common DISPC bindings into the top level bindings file.
Move common bindings here instead of having multiple copies of
the same information in all of the variant specific files.

Signed-off-by: Benoit Parrot 
Reviewed-by: Rob Herring 
---
 Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt  | 5 -
 Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt  | 7 +++
 Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt | 4 
 Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt | 4 
 Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt | 4 
 Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt | 4 
 6 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt 
b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
index 91279f1060fe..c30f9ec189ed 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,dra7-dss.txt
@@ -47,11 +47,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth 
limit
-   in bytes per second
-
-
 HDMI
 
 
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt 
b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
index e1ef29569338..249e588d7865 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap-dss.txt
@@ -21,6 +21,13 @@ a RGB pixel stream to encoders.
 The encoder modules encode the received RGB pixel stream to a video output like
 HDMI, MIPI DPI, etc.
 
+DISPC
+-
+
+Optional properties:
+- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth 
limit
+   in bytes per second
+
 Video Ports
 ---
 
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt 
b/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt
index ee867c4d1152..afcd5a86c6a4 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap2-dss.txt
@@ -28,10 +28,6 @@ Required properties:
 - ti,hwmods: "dss_dispc"
 - interrupts: the DISPC interrupt
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth 
limit
-   in bytes per second
-
 
 RFBI
 
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt 
b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
index cd02516a40b6..dc66e1447c31 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap3-dss.txt
@@ -37,10 +37,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth 
limit
-   in bytes per second
-
 
 RFBI
 
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt 
b/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt
index 0f85f6b3a5a8..bc624dbd 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap4-dss.txt
@@ -36,10 +36,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth 
limit
-   in bytes per second
-
 
 RFBI
 
diff --git a/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt 
b/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt
index 20861218649f..118a486c47bb 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt
+++ b/Documentation/devicetree/bindings/display/ti/ti,omap5-dss.txt
@@ -36,10 +36,6 @@ Required properties:
 - clocks: handle to fclk
 - clock-names: "fck"
 
-Optional properties:
-- max-memory-bandwidth: Input memory (from main memory to dispc) bandwidth 
limit
-   in bytes per second
-
 
 RFBI
 
-- 
2.9.0

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

[Patch v2 6/6] drm/omap: Allow wider display when a virtual plane is available

[Benoit Parrot]

Add an exception case when filtering out display mode so that if
a virtual wide plane is available then display wider than 2048 can be
supported as long as the required timing parameters can also be met.

Signed-off-by: Benoit Parrot 
---
 drivers/gpu/drm/omapdrm/omap_connector.c |  3 ++-
 drivers/gpu/drm/omapdrm/omap_plane.c | 12 
 drivers/gpu/drm/omapdrm/omap_plane.h |  1 +
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c 
b/drivers/gpu/drm/omapdrm/omap_connector.c
index d5e059abb555..517f7fa80ce1 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -203,7 +203,8 @@ static int omap_connector_mode_valid(struct drm_connector 
*connector,
u16 width, height;
 
priv->dispc_ops->ovl_get_max_size(, );
-   if (mode->hdisplay > width)
+   if (mode->hdisplay > width &&
+   !omap_have_any_virtual_plane(dev))
r = -EINVAL;
}
 
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c 
b/drivers/gpu/drm/omapdrm/omap_plane.c
index 21c927bbf5a7..8529abdcdeb8 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -209,6 +209,18 @@ bool is_omap_plane_virtual(struct drm_plane *plane)
return omap_plane->virtual_plane;
 }
 
+bool omap_have_any_virtual_plane(struct drm_device *dev)
+{
+   struct drm_plane *plane;
+
+   drm_for_each_plane(plane, dev) {
+   if (is_omap_plane_virtual(plane))
+   return true;
+   }
+
+   return false;
+}
+
 /* helper to install properties which are common to planes and crtcs */
 void omap_plane_install_properties(struct drm_plane *plane,
struct drm_mode_object *obj)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.h 
b/drivers/gpu/drm/omapdrm/omap_plane.h
index 48815a05f4fe..86b1c2022231 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.h
+++ b/drivers/gpu/drm/omapdrm/omap_plane.h
@@ -35,5 +35,6 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 void omap_plane_install_properties(struct drm_plane *plane,
struct drm_mode_object *obj);
 bool is_omap_plane_virtual(struct drm_plane *plane);
+bool omap_have_any_virtual_plane(struct drm_device *dev);
 
 #endif /* __OMAPDRM_PLANE_H__ */
-- 
2.9.0

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

[Patch v2 1/6] drm/omap: Add ability to filter out modes which can't be supported

[Benoit Parrot]

Currently available display mode from a connector are filtered out
based only on pixel clock capability. However we also need to filter
out wider mode if we cannot handle them based on available pipeline
capabilities.

Signed-off-by: Benoit Parrot 
---
 drivers/gpu/drm/omapdrm/dss/dispc.c  | 27 +++
 drivers/gpu/drm/omapdrm/dss/omapdss.h|  1 +
 drivers/gpu/drm/omapdrm/omap_connector.c | 10 ++
 3 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 624dee22f46b..35541d4441df 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -100,6 +100,8 @@ struct dispc_features {
u8 mgr_height_start;
u16 mgr_width_max;
u16 mgr_height_max;
+   u16 ovl_width_max;
+   u16 ovl_height_max;
unsigned long max_lcd_pclk;
unsigned long max_tv_pclk;
unsigned int max_downscale;
@@ -2465,6 +2467,12 @@ static int dispc_ovl_calc_scaling(unsigned long pclk, 
unsigned long lclk,
return 0;
 }
 
+static void dispc_ovl_get_max_size(u16 *width, u16 *height)
+{
+   *width = dispc.feat->ovl_width_max;
+   *height = dispc.feat->ovl_height_max;
+}
+
 static int dispc_ovl_setup_common(enum omap_plane_id plane,
enum omap_overlay_caps caps, u32 paddr, u32 p_uv_addr,
u16 screen_width, int pos_x, int pos_y, u16 width, u16 height,
@@ -2500,6 +2508,10 @@ static int dispc_ovl_setup_common(enum omap_plane_id 
plane,
out_width = out_width == 0 ? width : out_width;
out_height = out_height == 0 ? height : out_height;
 
+   WARN(out_width > dispc.feat->ovl_width_max,
+"Requested OVL width (%d) is larger than can be supported (%d).\n",
+out_width, dispc.feat->ovl_width_max);
+
if (ilace && height == out_height)
fieldmode = true;
 
@@ -4043,6 +4055,8 @@ static const struct dispc_features omap24xx_dispc_feats = 
{
.mgr_height_start   =   26,
.mgr_width_max  =   2048,
.mgr_height_max =   2048,
+   .ovl_width_max  =   2048,
+   .ovl_height_max =   2048,
.max_lcd_pclk   =   6650,
.max_downscale  =   2,
/*
@@ -4080,6 +4094,8 @@ static const struct dispc_features 
omap34xx_rev1_0_dispc_feats = {
.mgr_height_start   =   26,
.mgr_width_max  =   2048,
.mgr_height_max =   2048,
+   .ovl_width_max  =   2048,
+   .ovl_height_max =   2048,
.max_lcd_pclk   =   17300,
.max_tv_pclk=   5900,
.max_downscale  =   4,
@@ -4114,6 +4130,8 @@ static const struct dispc_features 
omap34xx_rev3_0_dispc_feats = {
.mgr_height_start   =   26,
.mgr_width_max  =   2048,
.mgr_height_max =   2048,
+   .ovl_width_max  =   2048,
+   .ovl_height_max =   2048,
.max_lcd_pclk   =   17300,
.max_tv_pclk=   5900,
.max_downscale  =   4,
@@ -4148,6 +4166,8 @@ static const struct dispc_features omap36xx_dispc_feats = 
{
.mgr_height_start   =   26,
.mgr_width_max  =   2048,
.mgr_height_max =   2048,
+   .ovl_width_max  =   2048,
+   .ovl_height_max =   2048,
.max_lcd_pclk   =   17300,
.max_tv_pclk=   5900,
.max_downscale  =   4,
@@ -4182,6 +4202,8 @@ static const struct dispc_features am43xx_dispc_feats = {
.mgr_height_start   =   26,
.mgr_width_max  =   2048,
.mgr_height_max =   2048,
+   .ovl_width_max  =   2048,
+   .ovl_height_max =   2048,
.max_lcd_pclk   =   17300,
.max_tv_pclk=   5900,
.max_downscale  =   4,
@@ -4216,6 +4238,8 @@ static const struct dispc_features omap44xx_dispc_feats = 
{
.mgr_height_start   =   26,
.mgr_width_max  =   2048,
.mgr_height_max =   2048,
+   .ovl_width_max  =   2048,
+   .ovl_height_max =   2048,
.max_lcd_pclk   =   17000,
.max_tv_pclk=   185625000,
.max_downscale  =   4,
@@ -4255,6 +4279,8 @@ static const struct dispc_features omap54xx_dispc_feats = 
{
.mgr_height_start   =   27,
.mgr_width_max  =   4096,
.mgr_height_max =   4096,
+   .ovl_width_max  =   2048,
+   .ovl_height_max =   4096,
.max_lcd_pclk   = 

[Patch v2 0/6] drm/omap: Add virtual-planes support

[Benoit Parrot]

This patch series adds virtual-plane support to omapdrm driver
to allow the use of display wider than 2048 pixels.

The DT bindings are also cleaned up to remove duplication when
properties are common to all implementations.

This patch series depends on Peter Ujfalusi's normalized zpos patch
set which can be found starting here:
  https://patchwork.kernel.org/patch/10299041/

Changes since v1:
- Added a check to filter out unsupportable display mode based on size
- Added Rob reviewed-by for patch #2
- Improve description of patch #3 replace term to remove DRM references
  and use terminology consistent with the Technical Reference Manual 
- Fix the reported zpos issue by reusing and locally extending the
  normalized zpos handling.
- Addressed Tomi's review comments

Benoit Parrot (6):
  drm/omap: Add ability to filter out modes which can't be supported
  dt-bindings: display/ti: Move common dispc bindings to omap-dss.txt
  dt-bindings: display/ti: Add plane binding to dispc node
  drm/omap: Add virtual plane DT parsing support
  drm/omap: Add virtual plane support to omap_plane
  drm/omap: Allow wider display when a virtual plane is available

 .../devicetree/bindings/display/ti/ti,dra7-dss.txt |   5 -
 .../devicetree/bindings/display/ti/ti,omap-dss.txt |  72 +
 .../bindings/display/ti/ti,omap2-dss.txt   |   4 -
 .../bindings/display/ti/ti,omap3-dss.txt   |   4 -
 .../bindings/display/ti/ti,omap4-dss.txt   |   4 -
 .../bindings/display/ti/ti,omap5-dss.txt   |   4 -
 drivers/gpu/drm/omapdrm/dss/dispc.c| 137 +
 drivers/gpu/drm/omapdrm/dss/omapdss.h  |  12 ++
 drivers/gpu/drm/omapdrm/omap_connector.c   |  11 ++
 drivers/gpu/drm/omapdrm/omap_drv.c | 127 +++-
 drivers/gpu/drm/omapdrm/omap_fb.c  |  66 ++---
 drivers/gpu/drm/omapdrm/omap_fb.h  |   4 +-
 drivers/gpu/drm/omapdrm/omap_plane.c   | 163 -
 drivers/gpu/drm/omapdrm/omap_plane.h   |   5 +-
 14 files changed, 528 insertions(+), 90 deletions(-)

-- 
2.9.0

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

Re: [PATCH DRM] drm: Remove drm_property_{un/reference}_blob aliases

[Julia Lawall]

On Tue, 20 Mar 2018, Haneen Mohammed wrote:

> This patch remove the compatibility aliases
> drm_property_{reference/unreference}_blob of
> drm_property_blob_{get/put} since all callers have been converted to the
> prefered _{get/put}.
>
> Remove the helpers from the semantic patch drm-get-put-cocci.
>
> Signed-off-by: Haneen Mohammed 

Acked-by: Julia Lawall 

For the Coccinelle part.

> ---
>  include/drm/drm_property.h   | 26 --
>  scripts/coccinelle/api/drm-get-put.cocci | 10 --
>  2 files changed, 36 deletions(-)
>
> diff --git a/include/drm/drm_property.h b/include/drm/drm_property.h
> index 8a522b4..08d5dbb 100644
> --- a/include/drm/drm_property.h
> +++ b/include/drm/drm_property.h
> @@ -279,32 +279,6 @@ struct drm_property_blob *drm_property_blob_get(struct 
> drm_property_blob *blob);
>  void drm_property_blob_put(struct drm_property_blob *blob);
>
>  /**
> - * drm_property_reference_blob - acquire a blob property reference
> - * @blob: DRM blob property
> - *
> - * This is a compatibility alias for drm_property_blob_get() and should not 
> be
> - * used by new code.
> - */
> -static inline struct drm_property_blob *
> -drm_property_reference_blob(struct drm_property_blob *blob)
> -{
> - return drm_property_blob_get(blob);
> -}
> -
> -/**
> - * drm_property_unreference_blob - release a blob property reference
> - * @blob: DRM blob property
> - *
> - * This is a compatibility alias for drm_property_blob_put() and should not 
> be
> - * used by new code.
> - */
> -static inline void
> -drm_property_unreference_blob(struct drm_property_blob *blob)
> -{
> - drm_property_blob_put(blob);
> -}
> -
> -/**
>   * drm_property_find - find property object
>   * @dev: DRM device
>   * @file_priv: drm file to check for lease against.
> diff --git a/scripts/coccinelle/api/drm-get-put.cocci 
> b/scripts/coccinelle/api/drm-get-put.cocci
> index ceb71ea..3a09c97 100644
> --- a/scripts/coccinelle/api/drm-get-put.cocci
> +++ b/scripts/coccinelle/api/drm-get-put.cocci
> @@ -40,12 +40,6 @@ expression object;
>  - drm_gem_object_unreference_unlocked(object)
>  + drm_gem_object_put_unlocked(object)
>  |
> -- drm_property_reference_blob(object)
> -+ drm_property_blob_get(object)
> -|
> -- drm_property_unreference_blob(object)
> -+ drm_property_blob_put(object)
> -|
>  - drm_dev_unref(object)
>  + drm_dev_put(object)
>  )
> @@ -72,10 +66,6 @@ __drm_gem_object_unreference(object)
>  |
>  drm_gem_object_unreference_unlocked(object)
>  |
> -drm_property_unreference_blob@p(object)
> -|
> -drm_property_reference_blob@p(object)
> -|
>  drm_dev_unref@p(object)
>  )
>
> --
> 2.7.4
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 105747] TOPAZ: ring buffers not getting initialized with the amd-staging-drm-next branch

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105747

--- Comment #2 from Alex Deucher  ---
Is there an actual bug you are seeing?  We no longer print the ring and IB test
info in the log unless you enable verbose logging to make the driver less
chatty.

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

[PATCH 1/6] drm/tinydrm: Use gem_free_object_unlocked

[Daniel Vetter]

tinydrm doesn't use dev->struct_mutex and therefore has no need to use
gem_free_object.

Signed-off-by: Daniel Vetter 
Cc: "Noralf Trønnes" 
---
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 2 +-
 include/drm/tinydrm/tinydrm.h   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c 
b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
index 4c6616278c48..24a33bf862fa 100644
--- a/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
+++ b/drivers/gpu/drm/tinydrm/core/tinydrm-core.c
@@ -91,7 +91,7 @@ EXPORT_SYMBOL(tinydrm_gem_cma_prime_import_sg_table);
  * GEM object state and frees the memory used to store the object itself using
  * drm_gem_cma_free_object(). It also handles PRIME buffers which has the 
kernel
  * virtual address set by tinydrm_gem_cma_prime_import_sg_table(). Drivers
- * can use this as their _driver->gem_free_object callback.
+ * can use this as their _driver->gem_free_object_unlocked callback.
  */
 void tinydrm_gem_cma_free_object(struct drm_gem_object *gem_obj)
 {
diff --git a/include/drm/tinydrm/tinydrm.h b/include/drm/tinydrm/tinydrm.h
index 07a9a11fe19d..77a93ec577fd 100644
--- a/include/drm/tinydrm/tinydrm.h
+++ b/include/drm/tinydrm/tinydrm.h
@@ -41,7 +41,7 @@ pipe_to_tinydrm(struct drm_simple_display_pipe *pipe)
  * the _driver structure.
  */
 #define TINYDRM_GEM_DRIVER_OPS \
-   .gem_free_object= tinydrm_gem_cma_free_object, \
+   .gem_free_object_unlocked = tinydrm_gem_cma_free_object, \
.gem_print_info = drm_gem_cma_print_info, \
.gem_vm_ops = _gem_cma_vm_ops, \
.prime_handle_to_fd = drm_gem_prime_handle_to_fd, \
-- 
2.16.2

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

[PATCH 0/6] move more drivers to gem_free_object_unlocked

[Daniel Vetter]

Hi all,

Random drive-by crusade against dev->struct_mutex usage. The only two
leftover drivers still relying on gem_free_object and the magic lifetime
rule that hodling dev->struct_mutex will prevent gem object destruction
are msm and gma500. Both actually need this, and don't look like they can
be fixed with an easy patch.

But the less this pattern is in the tree the more we can hopefully avoid
copypasta-spreading it more.

Cheers, Daniel

Daniel Vetter (6):
  drm/tinydrm: Use gem_free_object_unlocked
  staging/vboxvideo: Use gem_free_object_unlocked
  drm/rockchip: fixup comment for gem_free_object_unlocked
  drm/udl: Get rid of dev->struct_mutex usage
  drm/omapdrm: Fix mm_list locking
  drm/omapdrm: Switch to gem_free_object_unlocked

 drivers/gpu/drm/omapdrm/omap_debugfs.c  |  2 ++
 drivers/gpu/drm/omapdrm/omap_drv.c  |  4 ++--
 drivers/gpu/drm/omapdrm/omap_drv.h  |  2 +-
 drivers/gpu/drm/omapdrm/omap_gem.c  | 11 +++
 drivers/gpu/drm/rockchip/rockchip_drm_gem.c |  4 ++--
 drivers/gpu/drm/tinydrm/core/tinydrm-core.c |  2 +-
 drivers/gpu/drm/udl/udl_dmabuf.c|  5 +++--
 drivers/gpu/drm/udl/udl_drv.c   |  2 +-
 drivers/gpu/drm/udl/udl_drv.h   |  2 ++
 drivers/gpu/drm/udl/udl_gem.c   |  5 +++--
 drivers/gpu/drm/udl/udl_main.c  |  2 ++
 drivers/staging/vboxvideo/vbox_drv.c|  2 +-
 include/drm/tinydrm/tinydrm.h   |  2 +-
 13 files changed, 28 insertions(+), 17 deletions(-)

-- 
2.16.2

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

Re: [PATCH] drm/gem: Document that handle_create must be the last step

[Daniel Vetter]

On Thu, Mar 22, 2018 at 10:12:03AM +0200, Oleksandr Andrushchenko wrote:
> On 03/22/2018 10:02 AM, Daniel Vetter wrote:
> > It published
> s/It/If

Doesn't make much sense to me, It = drm_gem_handle_create.

> >   the gem object to userspace, by that point other threads
> > can guess the id and start using it. And gem IDs are _very_ easy to
> > guess (it's just an idr).
> > 
> > Since gem objects is the only thing we allow drivers to create
> > themselves (all the kms/prime/syncobj stuff is handled by the core) no
> > other functions seem to be in need of this clarification.
> > 
> > Motivated by reviewing the xen-front kms driver.
> > 
> > Cc: Oleksandr Andrushchenko 
> > Signed-off-by: Daniel Vetter 
> > ---
> >   drivers/gpu/drm/drm_gem.c | 9 ++---
> >   1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> > index 4975ba9a7bc8..4a16d7b26c89 100644
> > --- a/drivers/gpu/drm/drm_gem.c
> > +++ b/drivers/gpu/drm/drm_gem.c
> > @@ -436,9 +436,12 @@ drm_gem_handle_create_tail(struct drm_file *file_priv,
> >* @obj: object to register
> >* @handlep: pionter to return the created handle to the caller
> >*
> > - * Create a handle for this object. This adds a handle reference
> > - * to the object, which includes a regular reference count. Callers
> > - * will likely want to dereference the object afterwards.
> > + * Create a handle for this object. This adds a handle reference to the 
> > object,
> > + * which includes a regular reference count. Callers will likely want to
> > + * dereference the object afterwards.
> > + *
> > + * Since this publishes @obj to userspace it must be fully set up by this 
> > point,
> > + * drivers must call this last in their buffer object creation callbacks.
> >*/
> >   int drm_gem_handle_create(struct drm_file *file_priv,
> >   struct drm_gem_object *obj,
> 
> Reviewed-by: Oleksandr Andrushchenko 

Applied, thx for review.
-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/5] dma-buf: add optional invalidate_mappings callback v2

[Jerome Glisse]

On Mon, Mar 26, 2018 at 10:01:21AM +0200, Daniel Vetter wrote:
> On Thu, Mar 22, 2018 at 10:58:55AM +0100, Christian König wrote:
> > Am 22.03.2018 um 08:18 schrieb Daniel Vetter:
> > > On Wed, Mar 21, 2018 at 12:54:20PM +0100, Christian König wrote:
> > > > Am 21.03.2018 um 09:28 schrieb Daniel Vetter:
> > > > > On Tue, Mar 20, 2018 at 06:47:57PM +0100, Christian König wrote:
> > > > > > Am 20.03.2018 um 15:08 schrieb Daniel Vetter:
> > > > > > > [SNIP]
> > > > > > > For the in-driver reservation path (CS) having a slow-path that 
> > > > > > > grabs a
> > > > > > > temporary reference, drops the vram lock and then locks the 
> > > > > > > reservation
> > > > > > > normally (using the acquire context used already for the entire 
> > > > > > > CS) is a
> > > > > > > bit tricky, but totally feasible. Ttm doesn't do that though.
> > > > > > That is exactly what we do in amdgpu as well, it's just not very 
> > > > > > efficient
> > > > > > nor reliable to retry getting the right pages for a submission over 
> > > > > > and over
> > > > > > again.
> > > > > Out of curiosity, where's that code? I did read the ttm eviction code 
> > > > > way
> > > > > back, and that one definitely didn't do that. Would be interesting to
> > > > > update my understanding.
> > > > That is in amdgpu_cs.c. amdgpu_cs_parser_bos() does a horrible dance 
> > > > with
> > > > grabbing, releasing and regrabbing locks in a loop.
> > > > 
> > > > Then in amdgpu_cs_submit() we grab an lock preventing page table 
> > > > updates and
> > > > check if all pages are still the one we want to have:
> > > > >      amdgpu_mn_lock(p->mn);
> > > > >      if (p->bo_list) {
> > > > >      for (i = p->bo_list->first_userptr;
> > > > >   i < p->bo_list->num_entries; ++i) {
> > > > >      struct amdgpu_bo *bo = 
> > > > > p->bo_list->array[i].robj;
> > > > > 
> > > > >      if
> > > > > (amdgpu_ttm_tt_userptr_needs_pages(bo->tbo.ttm)) {
> > > > >      amdgpu_mn_unlock(p->mn);
> > > > >      return -ERESTARTSYS;
> > > > >      }
> > > > >      }
> > > > >      }
> > > > If anything changed on the page tables we restart the whole IOCTL using
> > > > -ERESTARTSYS and try again.
> > > I'm not talking about userptr here, but general bo eviction. Sorry for the
> > > confusion.
> > > 
> > > The reason I'm dragging all the general bo management into this
> > > discussions is because we do seem to have fairly fundamental difference in
> > > how that's done, with resulting consequences for the locking hierarchy.
> > > 
> > > And if this invalidate_mapping stuff should work, together with userptr
> > > and everything else, I think we're required to agree on how this is all
> > > supposed to nest, and how exactly we should back off for the other side
> > > that needs to break the locking circle.
> > > 
> > > That aside, I don't entirely understand why you need to restart so much. I
> > > figured that get_user_pages is ordered correctly against mmu
> > > invalidations, but I get the impression you think that's not the case. How
> > > does that happen?
> > 
> > Correct. I've had the same assumption, but both Jerome as well as our
> > internal tests proved me wrong on that.
> > 
> > Key take away from that was that you can't take any locks from neither the
> > MMU notifier nor the shrinker you also take while calling kmalloc (or
> > simpler speaking get_user_pages()).
> > 
> > Additional to that in the MMU or shrinker callback all different kinds of
> > locks might be held, so you basically can't assume that you do thinks like
> > recursive page table walks or call dma_unmap_anything.
> 
> That sounds like a design bug in mmu_notifiers, since it would render them
> useless for KVM. And they were developed for that originally. I think I'll
> chat with Jerome to understand this, since it's all rather confusing.

Doing dma_unmap() during mmu_notifier callback should be fine, it was last
time i check. However there is no formal contract that it is ok to do so.
But i want to revisit DMA API to something where device driver get control
rather than delegate. High level ideas are:
  - device driver allocate DMA space ie range of physical bus address
  - device driver completely manage the range in whichever ways it wants
under its own locking.
  - DMA provide API to flush tlb/cache of a range of physical bus address
with the explicit contract that it can only take short lived spinlock
to do so and not allocate or free memory while doing so (ie it should
only be a bunch of registers writes)

I haven't had time to prototype this but i floated the idea a while back
and i am hoping to discuss it some more sooner rather than latter (LSF/MM
is coming soon).


So from a safety point of view i would argue that you must be doing dma_unmap()
from invalidate_range_start() callback. But it all depends 

Re: [PATCH] drm/scheduler: remove incorrect param documentation

[Nayan Deshmukh]

I am not removing jobs parameters as of now as it will lead to changes
in all the calls to this function and will affect the etnaviv patches.

Regards,
Nayan.

On Sun, Mar 25, 2018 at 4:59 PM, Christian König
 wrote:
> Am 25.03.2018 um 13:21 schrieb Nayan Deshmukh:
>>
>> On Sun, Mar 25, 2018 at 4:44 PM, Christian König
>>  wrote:
>>>
>>> Am 25.03.2018 um 13:09 schrieb Nayan Deshmukh:

 Signed-off-by: Nayan Deshmukh 
 ---
drivers/gpu/drm/scheduler/gpu_scheduler.c | 1 -
1 file changed, 1 deletion(-)

 diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c
 b/drivers/gpu/drm/scheduler/gpu_scheduler.c
 index 0d95888ccc3e..27fdda1264f7 100644
 --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
 +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
 @@ -117,7 +117,6 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
 * @sched The pointer to the scheduler
 * @entityThe pointer to a valid drm_sched_entity
 * @rqThe run queue this entity belongs
 - * @kernel If this is an entity for the kernel
 * @jobs  The max number of jobs in the job queue
>>>
>>>
>>> If I'm not completely mistaken the jobs parameter is unused and we also
>>> have
>>> a @guilty parameter as well.
>>>
>> Yes. But I am not sure what should be done with the jobs parameter.
>> Should I remove it entirely?
>
>
> Yes, probably a leftover from when we used a kfifo for the queue.
>
> Regards,
> Christian.
>
>
>>
>>> The description for the @guilty should be something like "atomic_t set to
>>> 1
>>> when a job on this queue is found to be guilty causing a timeout".
>>>
>>> Regards,
>>> Christian.
>>>
>>>
 *
 * return 0 if succeed. negative error code on failure
>>>
>>>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/scheduler: fix param documentation

[Nayan Deshmukh]

Signed-off-by: Nayan Deshmukh 
---
 drivers/gpu/drm/scheduler/gpu_scheduler.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c 
b/drivers/gpu/drm/scheduler/gpu_scheduler.c
index 0d95888ccc3e..1d368bc66ac2 100644
--- a/drivers/gpu/drm/scheduler/gpu_scheduler.c
+++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c
@@ -117,8 +117,9 @@ drm_sched_rq_select_entity(struct drm_sched_rq *rq)
  * @sched  The pointer to the scheduler
  * @entity The pointer to a valid drm_sched_entity
  * @rq The run queue this entity belongs
- * @kernel If this is an entity for the kernel
  * @jobs   The max number of jobs in the job queue
+ * @guilty  atomic_t set to 1 when a job on this queue
+ *  is found to be guilty causing a timeout
  *
  * return 0 if succeed. negative error code on failure
 */
-- 
2.14.3

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

Re: [PATCH] dma-buf: use parameter structure for dma_buf_attach

[Daniel Vetter]

On Mon, Mar 26, 2018 at 12:47:01PM +0200, Christian König wrote:
> Am 26.03.2018 um 10:36 schrieb Daniel Vetter:
> > On Sun, Mar 25, 2018 at 01:34:51PM +0200, Christian König wrote:
> [SNIP]
> > > - attach->dev = dev;
> > > + attach->dev = info->dev;
> > >   attach->dmabuf = dmabuf;
> > > + attach->priv = info->priv;
> > The ->priv field is for the exporter, not the importer. See e.g.
> > drm_gem_map_attach. You can't let the importer set this now too, so needs
> > to be removed from the info struct.
> 
> Crap, in this case I need to add an importer_priv field because we now need
> to map from the attachment to it's importer object as well.
> 
> Thanks for noticing this.

Maybe add the importer_priv field only in the series that actually adds
it, not in this prep patch. You can mention all the fields you need here
in the commit message for justification.
-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: [linux-sunxi] Preferring cursor plane over overlay plane

[Chen-Yu Tsai]

On Mon, Mar 26, 2018 at 10:45 PM, Maxime Ripard
 wrote:
> On Mon, Mar 26, 2018 at 10:22:45PM +0800, Chen-Yu Tsai wrote:
>> On Mon, Mar 26, 2018 at 10:14 PM, Joonas Kylmälä  
>> wrote:
>> > Hi DRM subsystem developers,
>> >
>> > I ran into this patch where overlay plane was switched to cursor plane
>> > because there was no proper cursor plane available on the display
>> > hardware: . Can we discuss whether
>> > to have a policy of using a normal plane for cursor plane in case a
>> > dedicated HW cursor plane is missing?
>> >
>> > Daniel Vetter suggests that it might be fine to use normal plane for
>> > cursor plane because how to use the plane would be only "a hint to
>> > userspace" (see the email linked).
>> >
>> > My motivation for having this discussion is that the newer Allwinner
>> > SoCs don't have dedicated HW cursor plane and the sun4i DRM driver
>> > currently uses the extra planes as overlay planes which makes moving the
>> > cursor on Xfce4 DE a terrible experience. To have better cursor moving
>> > experience one overlay plane would need to be sacrificed.
>>
>> If you look at the development history, we've never supported cursor planes.
>
> X can use an overlay to put the cursor though.
>
>> At the beginning we supported one main plane and one overlay plane. That was
>> it. The Display Engine 1.0 does have support for an extra hardware cursor,
>> but we haven't done the work to support it yet. I don't know about the
>> Display Engine 2.0 though.
>
> An issue with supporting the hardware cursor we have is that as far as
> I understood, the cursor plane in DRM has the assumption that it would
> be an ARGB format. In the first display engine, the format is actually
> an 8-bit palette with 1 bit of alpha iirc.

Looks like it's 32x32 pixels with an 8-bit (max) palette, with full RGBA
for the colors in the palette. I don't see the 1 bit alpha you mentioned.
Looks like this needs some extra work for building the palette and copying
the cursor image.

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

[Bug 104037] [skl] GPU HANG: ecode 9:0:0x87f9bffb, in Xorg [1688], reason: Hang on rcs0, action: reset using kicad

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=104037

Elizabeth  changed:

   What|Removed |Added

   Assignee|dri-devel@lists.freedesktop |intel-3d-bugs@lists.freedes
   |.org|ktop.org
 QA Contact|dri-devel@lists.freedesktop |intel-3d-bugs@lists.freedes
   |.org|ktop.org
  Component|Drivers/DRI/i915|Drivers/DRI/i965

--- Comment #3 from Elizabeth  ---
Moving to i965 since is skl.

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

Re: [PATCH] gpu: host1x: Fix compiler errors

[Thierry Reding]

On Mon, Mar 26, 2018 at 04:44:14PM +0200, Emil Goode wrote:
> The compiler is complaining with the following errors:
> 
> drivers/gpu/host1x/cdma.c:94:48: error:
>   passing argument 3 of ‘dma_alloc_wc’ from incompatible pointer type
>   [-Werror=incompatible-pointer-types]
> 
> drivers/gpu/host1x/cdma.c:113:48: error:
>   passing argument 3 of ‘dma_alloc_wc’ from incompatible pointer type
>   [-Werror=incompatible-pointer-types]
> 
> The expected pointer type of the third argument to dma_alloc_wc() is
> dma_addr_t but phys_addr_t is passed. Fix this by adding casts to the
> expected pointer type.
> 
> Signed-off-by: Emil Goode 
> ---
>  drivers/gpu/host1x/cdma.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

What compiler do you use? I do regular builds and check for warnings and
errors, and this one is new to me.

Thierry

> 
> diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c
> index 28541b280739..5e8b321a751e 100644
> --- a/drivers/gpu/host1x/cdma.c
> +++ b/drivers/gpu/host1x/cdma.c
> @@ -91,8 +91,8 @@ static int host1x_pushbuffer_init(struct push_buffer *pb)
>  
>   size = iova_align(>iova, size);
>  
> - pb->mapped = dma_alloc_wc(host1x->dev, size, >phys,
> -   GFP_KERNEL);
> + pb->mapped = dma_alloc_wc(host1x->dev, size,
> +   (dma_addr_t *)>phys, GFP_KERNEL);
>   if (!pb->mapped)
>   return -ENOMEM;
>  
> @@ -110,8 +110,8 @@ static int host1x_pushbuffer_init(struct push_buffer *pb)
>   if (err)
>   goto iommu_free_iova;
>   } else {
> - pb->mapped = dma_alloc_wc(host1x->dev, size, >phys,
> -   GFP_KERNEL);
> + pb->mapped = dma_alloc_wc(host1x->dev, size,
> +   (dma_addr_t *)>phys, GFP_KERNEL);
>   if (!pb->mapped)
>   return -ENOMEM;
>  
> -- 
> 2.11.0
> 


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

[Bug 104037] [skl] GPU HANG: ecode 9:0:0x87f9bffb, in Xorg [1688], reason: Hang on rcs0, action: reset using kicad

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=104037

Elizabeth  changed:

   What|Removed |Added

 Status|NEW |NEEDINFO
Summary|[drm] GPU HANG: ecode   |[skl] GPU HANG: ecode
   |9:0:0x87f9bffb, in Xorg |9:0:0x87f9bffb, in Xorg
   |[1688], reason: Hang on |[1688], reason: Hang on
   |rcs0, action: reset |rcs0, action: reset using
   ||kicad

--- Comment #2 from Elizabeth  ---
Hello, sorry for the delay. Philip, is this still happening with mesa 17.3.6?
This could be a dup of https://bugs.freedesktop.org/show_bug.cgi?id=103555.

Shane, your hang seems to be a different issue. If still reproducible could 
you open a new bug for this case. Thank you.

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

Re: [linux-sunxi] Preferring cursor plane over overlay plane

[Maxime Ripard]

On Mon, Mar 26, 2018 at 10:22:45PM +0800, Chen-Yu Tsai wrote:
> On Mon, Mar 26, 2018 at 10:14 PM, Joonas Kylmälä  
> wrote:
> > Hi DRM subsystem developers,
> >
> > I ran into this patch where overlay plane was switched to cursor plane
> > because there was no proper cursor plane available on the display
> > hardware: . Can we discuss whether
> > to have a policy of using a normal plane for cursor plane in case a
> > dedicated HW cursor plane is missing?
> >
> > Daniel Vetter suggests that it might be fine to use normal plane for
> > cursor plane because how to use the plane would be only "a hint to
> > userspace" (see the email linked).
> >
> > My motivation for having this discussion is that the newer Allwinner
> > SoCs don't have dedicated HW cursor plane and the sun4i DRM driver
> > currently uses the extra planes as overlay planes which makes moving the
> > cursor on Xfce4 DE a terrible experience. To have better cursor moving
> > experience one overlay plane would need to be sacrificed.
> 
> If you look at the development history, we've never supported cursor planes.

X can use an overlay to put the cursor though.

> At the beginning we supported one main plane and one overlay plane. That was
> it. The Display Engine 1.0 does have support for an extra hardware cursor,
> but we haven't done the work to support it yet. I don't know about the
> Display Engine 2.0 though.

An issue with supporting the hardware cursor we have is that as far as
I understood, the cursor plane in DRM has the assumption that it would
be an ARGB format. In the first display engine, the format is actually
an 8-bit palette with 1 bit of alpha iirc.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


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

[Bug 105725] WARNING: CPU: 0 PID: 487 at drivers/gpu/drm/amd/amdgpu/../display /dc/gpio/gpio_base.c:64 dal_gpio_open_ex+0xc/0x30 [amdgpu]

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105725

--- Comment #2 from hjpries...@gmail.com ---
Is there a guide a build the 4.15.x kernel using the drm-4.17-wip?
I can try to compile/test it next weekend.

The problem I have is that when I display a certain png with "imagemagick
display" X11 hangs.
The screen is not longer updated and only a reboot helps.

When I run the program using strace the last system call displayed is:

 ioctl(6, DRM_IOCTL_AMDGPU_WAIT_CS. 

That is why I am thinking it might have to do with the amdgpu.

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

Re: [RFC] Per file OOM badness

[Lucas Stach]

Hi all,

Am Dienstag, den 30.01.2018, 11:28 +0100 schrieb Michal Hocko:
> On Tue 30-01-18 10:29:10, Michel Dänzer wrote:
> > On 2018-01-24 12:50 PM, Michal Hocko wrote:
> > > On Wed 24-01-18 12:23:10, Michel Dänzer wrote:
> > > > On 2018-01-24 12:01 PM, Michal Hocko wrote:
> > > > > On Wed 24-01-18 11:27:15, Michel Dänzer wrote:
> > > 
> > > [...]
> > > > > > 2. If the OOM killer kills a process which is sharing BOs
> > > > > > with another
> > > > > > process, this should result in the other process dropping
> > > > > > its references
> > > > > > to the BOs as well, at which point the memory is released.
> > > > > 
> > > > > OK. How exactly are those BOs mapped to the userspace?
> > > > 
> > > > I'm not sure what you're asking. Userspace mostly uses a GEM
> > > > handle to
> > > > refer to a BO. There can also be userspace CPU mappings of the
> > > > BO's
> > > > memory, but userspace doesn't need CPU mappings for all BOs and
> > > > only
> > > > creates them as needed.
> > > 
> > > OK, I guess you have to bear with me some more. This whole stack
> > > is a
> > > complete uknonwn. I am mostly after finding a boundary where you
> > > can
> > > charge the allocated memory to the process so that the oom killer
> > > can
> > > consider it. Is there anything like that? Except for the proposed
> > > file
> > > handle hack?
> > 
> > How about the other way around: what APIs can we use to charge /
> > "uncharge" memory to a process? If we have those, we can experiment
> > with
> > different places to call them.
> 
> add_mm_counter() and I would add a new counter e.g. MM_KERNEL_PAGES.

So is anyone still working on this? This is hurting us bad enough that
I don't want to keep this topic rotting for another year.

If no one is currently working on this I would volunteer to give the
simple "just account private, non-shared buffers in process RSS" a
spin.

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

Re: [linux-sunxi] Preferring cursor plane over overlay plane

[Chen-Yu Tsai]

On Mon, Mar 26, 2018 at 10:14 PM, Joonas Kylmälä  wrote:
> Hi DRM subsystem developers,
>
> I ran into this patch where overlay plane was switched to cursor plane
> because there was no proper cursor plane available on the display
> hardware: . Can we discuss whether
> to have a policy of using a normal plane for cursor plane in case a
> dedicated HW cursor plane is missing?
>
> Daniel Vetter suggests that it might be fine to use normal plane for
> cursor plane because how to use the plane would be only "a hint to
> userspace" (see the email linked).
>
> My motivation for having this discussion is that the newer Allwinner
> SoCs don't have dedicated HW cursor plane and the sun4i DRM driver
> currently uses the extra planes as overlay planes which makes moving the
> cursor on Xfce4 DE a terrible experience. To have better cursor moving
> experience one overlay plane would need to be sacrificed.

If you look at the development history, we've never supported cursor planes.
At the beginning we supported one main plane and one overlay plane. That was
it. The Display Engine 1.0 does have support for an extra hardware cursor,
but we haven't done the work to support it yet. I don't know about the
Display Engine 2.0 though.

ChenYu

> Also, I probably missed some recipients for this email so please forward
> it to the recipients you think might be interested about this.
>
> Best regards,
> Joonas
>
> --
> You received this message because you are subscribed to the Google Groups 
> "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to linux-sunxi+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 105725] WARNING: CPU: 0 PID: 487 at drivers/gpu/drm/amd/amdgpu/../display /dc/gpio/gpio_base.c:64 dal_gpio_open_ex+0xc/0x30 [amdgpu]

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105725

--- Comment #1 from Harry Wentland  ---
I'm not sure, but I think they could be related to a hang.

A couple questions:
 * Do you see a hang a boot, or in a different scenario?
 * What display are you using?
 * What's your distribution?
 * Are you able to try the drm-next-4.17-wip branch from
https://cgit.freedesktop.org/~agd5f/linux/ ?

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

[PATCH][next] drm/amd/pp: fix logical or'ing of garbage in result by initializing result to zero

[Colin King]

From: Colin Ian King 

Currently result is not initialized, so it contains a garbage value and
this is or'd with data from vega10_program_didt_config_registers() which
can end up with a non-zero value when the result should be zero.  Fix
this by ensuring result is initialized to zero.

Detected by CoverityScan, CID#1466088 ("Uninitialized scalar variable")

Fixes: 9b7b8154cdb8 ("drm/amd/powerplay: added didt support for vega10")
Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c 
b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
index b1f74c7f0943..49822ea3ddd5 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
@@ -1091,7 +1091,7 @@ static int vega10_disable_se_edc_config(struct pp_hwmgr 
*hwmgr)
 
 static int vega10_enable_psm_gc_edc_config(struct pp_hwmgr *hwmgr)
 {
-   int result;
+   int result = 0;
uint32_t num_se = 0;
uint32_t count, data;
struct amdgpu_device *adev = hwmgr->adev;
-- 
2.15.1

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

Re: [PATCH v2 2/2] gpu: drm: nouveau: Use list_{next/prev}_entry instead of list_entry

[Daniel Vetter]

On Mon, Mar 26, 2018 at 01:43:30PM +0100, Chris Wilson wrote:
> Quoting Ben Skeggs (2018-03-26 13:34:54)
> > On Mon, Mar 26, 2018 at 4:01 AM, Arushi Singhal
> >  wrote:
> > > It's better to use list_entry instead of list_{next/prev}_entry
> > > as it makes the code more clear to read.
> > > This patch replace list_entry with list_{next/prev}_entry.
> > >
> > > Signed-off-by: Arushi Singhal 
> > Acked-by: Ben Skeggs 

Applied, thanks for ack

> > 
> > > ---
> > >  drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c 
> > > b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > > index e4c8d31..81c3567 100644
> > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
> > > @@ -134,7 +134,7 @@ nvkm_cstate_find_best(struct nvkm_clk *clk, struct 
> > > nvkm_pstate *pstate,
> > >nvkm_volt_map(volt, volt->max2_id, 
> > > clk->temp));
> > >
> > > for (cstate = start; >head != >list;
> > > -cstate = list_entry(cstate->head.prev, typeof(*cstate), 
> > > head)) {
> > > +cstate = list_prev_entry(cstate, head)) {
> 
> This loop could be written as:
> 
>   cstate = start; /* cstate looks redundant here, just use start? */
>   list_for_each_entry_from_reverse(cstate, >list, head)

Yeah, sounds like a good follow-up patch.
-Daniel

> 
> > > if (nvkm_cstate_valid(clk, cstate, max_volt, clk->temp))
> > > break;
> > > }
> > > --
> > > 2.7.4
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 3/3] xf86drm: replace stat() with access() to verify file existence

[Emil Velikov]

On 26 March 2018 at 11:26, Eric Engestrom  wrote:
> Signed-off-by: Eric Engestrom 
> ---
>  xf86drm.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 5701952ae83634b47628..47a82407df82d37a59b2 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3767,10 +3767,8 @@ int drmGetDevice2(int fd, uint32_t flags, drmDevicePtr 
> *device)
>  return -EINVAL;
>
>  n = snprintf(node, PATH_MAX, dev_name, DRM_DIR_NAME, min - base);
> -if (n == -1 || n >= PATH_MAX)
> +if (n == -1 || n >= PATH_MAX || access(node, F_OK))
>return -errno;
> -if (stat(node, ))
> -return -EINVAL;
>
Above all - there's a beefy RATIONALE section in man 3p access and I'm
wondering if 2) isn't applicable in some odd corner case.
If things are safe - please a) document why - perf./other reasons and
b) update the other instances.

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

Re: [PATCH libdrm 2/3] xf86drm: add buffer size safety to sprintf()

[Emil Velikov]

On 26 March 2018 at 11:26, Eric Engestrom  wrote:
> Signed-off-by: Eric Engestrom 
> ---
>  xf86drm.c | 6 +++---
>  xf86drmMode.c | 6 --
>  2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index b6e5d8cc1bb50ffe75a2..5701952ae83634b47628 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -349,7 +349,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
>  return -EINVAL;
>  };
>
> -sprintf(buf, dev_name, DRM_DIR_NAME, minor);
> +snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, minor);
The patch feels a big meh, for two reasons:
 - buffer contents are controlled and will never overflow
 - s{n,}printf return value is not checked

If there's a particular tool that should be silenced up sure, let's
land it. Otherwise I might as well list my 'grand master plan', we can
start deleting all this code ;-)

What do you think?
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 1/3] xf86drm: replace sprintf()+strdup() with asprintf()

[Emil Velikov]

On 26 March 2018 at 14:57, Jani Nikula  wrote:
> On Mon, 26 Mar 2018, Eric Engestrom  wrote:
>> Signed-off-by: Eric Engestrom 
>> ---
>>  xf86drm.c | 28 ++--
>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index 3a9d0ed2cc9b196ae7d1..b6e5d8cc1bb50ffe75a2 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -2823,7 +2823,7 @@ static char *drmGetMinorNameForFD(int fd, int type)
>>  struct stat sbuf;
>>  const char *name = drmGetMinorName(type);
>>  int len;
>> -char dev_name[64], buf[64];
>> +char *dev_name, buf[64];
>>  int maj, min;
>>
>>  if (!name)
>> @@ -2848,20 +2848,22 @@ static char *drmGetMinorNameForFD(int fd, int type)
>>
>>  while ((ent = readdir(sysdir))) {
>>  if (strncmp(ent->d_name, name, len) == 0) {
>> -snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s",
>> - ent->d_name);
>> +if (asprintf(_name, DRM_DIR_NAME "/%s",
>
> Just noting in passing that asprintf is a GNU extension, is that okay?
>
Was going to mention the same thing. Also POSIX please add it to the
next version ;-)
It doesn't seem to make the code shorter, so I'd go with let's drop this?

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

Re: [PATCH libdrm 1/3] xf86drm: replace sprintf()+strdup() with asprintf()

[Jani Nikula]

On Mon, 26 Mar 2018, Eric Engestrom  wrote:
> Signed-off-by: Eric Engestrom 
> ---
>  xf86drm.c | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 3a9d0ed2cc9b196ae7d1..b6e5d8cc1bb50ffe75a2 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2823,7 +2823,7 @@ static char *drmGetMinorNameForFD(int fd, int type)
>  struct stat sbuf;
>  const char *name = drmGetMinorName(type);
>  int len;
> -char dev_name[64], buf[64];
> +char *dev_name, buf[64];
>  int maj, min;
>  
>  if (!name)
> @@ -2848,20 +2848,22 @@ static char *drmGetMinorNameForFD(int fd, int type)
>  
>  while ((ent = readdir(sysdir))) {
>  if (strncmp(ent->d_name, name, len) == 0) {
> -snprintf(dev_name, sizeof(dev_name), DRM_DIR_NAME "/%s",
> - ent->d_name);
> +if (asprintf(_name, DRM_DIR_NAME "/%s",

Just noting in passing that asprintf is a GNU extension, is that okay?

BR,
Jani.


> + ent->d_name) < 0) {
> +dev_name = NULL;
> +}
>  
>  closedir(sysdir);
> -return strdup(dev_name);
> +return dev_name;
>  }
>  }
>  return NULL;
>  #else
>  struct stat sbuf;
> -char buf[PATH_MAX + 1];
> +char *buf;
>  const char *dev_name;
>  unsigned int maj, min;
> -int n, base;
> +int base;
>  
>  if (fstat(fd, ))
>  return NULL;
> @@ -2890,11 +2892,10 @@ static char *drmGetMinorNameForFD(int fd, int type)
>  if (base < 0)
>  return NULL;
>  
> -n = snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, min - base);
> -if (n == -1 || n >= sizeof(buf))
> +if (asprintf(, dev_name, DRM_DIR_NAME, min - base) < 0)
>  return NULL;
>  
> -return strdup(buf);
> +return buf;
>  #endif
>  }
>  
> @@ -4119,10 +4120,10 @@ char *drmGetDeviceNameFromFd2(int fd)
>  return strdup(path);
>  #else
>  struct stat  sbuf;
> -char node[PATH_MAX + 1];
> +char*node;
>  const char  *dev_name;
>  int  node_type;
> -int  maj, min, n, base;
> +int  maj, min, base;
>  
>  if (fstat(fd, ))
>  return NULL;
> @@ -4155,11 +4156,10 @@ char *drmGetDeviceNameFromFd2(int fd)
>  if (base < 0)
>  return NULL;
>  
> -n = snprintf(node, PATH_MAX, dev_name, DRM_DIR_NAME, min - base);
> -if (n == -1 || n >= PATH_MAX)
> +if (asprintf(, dev_name, DRM_DIR_NAME, min - base) < 0)
>return NULL;
>  
> -return strdup(node);
> +return node;
>  #endif
>  }

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

[Bug 105617] [CI] [CNL only] igt@* - incomplete - Build timed out (after 18 minutes). Marking the build as aborted.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105617

--- Comment #7 from Marta Löfstedt  ---
This should help:
https://patchwork.freedesktop.org/series/40646/

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

[Bug 105617] [CI] [CNL only] igt@* - incomplete - Build timed out (after 18 minutes). Marking the build as aborted.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105617

--- Comment #6 from Marta Löfstedt  ---
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_7/fi-cnl-y3/igt@kms_frontbuffer_track...@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html

running: igt/kms_frontbuffer_tracking/fbcpsr-1p-primscrn-pri-indfb-draw-blt

[96/98] skip: 25, pass: 69, fail: 2 |  
Build timed out (after 18 minutes). Marking the build as aborted.

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

[Bug 99349] Failed to build shader (translation from TGSI)

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=99349

mirh  changed:

   What|Removed |Added

 CC||m...@protonmail.ch

--- Comment #20 from mirh  ---
http://www.graphicsfuzz.com/benchmark/android-v1.html
I'm still having the same problem with mesa-git and firefox nightly (HD 6310)

r600_shader_from_tgsi - GPR limit exceeded - shader requires 130 registers
EE r600_shader.c:183 r600_pipe_shader_create - translation from TGSI failed !
EE r600_state_common.c:872 r600_shader_select - Failed to build shader variant
(type=1) -12

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

Re: [PATCH v2 00/11] drm/tinydrm: Support device unplug

[Oleksandr Andrushchenko]

Hi, Noralf!

On 03/17/2018 04:40 PM, Noralf Trønnes wrote:


Den 16.03.2018 09.03, skrev Daniel Vetter:

On Fri, Sep 8, 2017 at 6:33 PM, Daniel Vetter  wrote:

Hi Noralf,

On Fri, Sep 08, 2017 at 05:07:19PM +0200, Noralf Trønnes wrote:

This adds device unplug support to drm_fb_helper, drm_fb_cma_helper
(fbdev) and tinydrm.

There are several changes in this version:

I've used Daniel's idea of protecting drm_device.unplugged with 
srcu to

provide race free drm_dev_unplug().

The fbdev helper unplug patch has become very small with Daniel's 
help.

Ref is now taken and dropped in the existing helpers, so I could drop
drm_fb_helper_simple_init().

I has annoyed me that fbdev is restored in drm_driver.last_close 
even if
fbdev isn't used. I've added a patch to fix that. This means I can 
drop

calling drm_atomic_helper_shutdown() in tinydrm_unregister(), since I
now can easily disable the pipeline from userspace by just closing the
users. Disabled pipeline means balanced regulator_enable/disable.

The 'Embed drm_device in tinydrm_device' patch has gained
drm_driver.release functions after a discussion with Laurent. My
previous version relied on obscure freeing in tinydrm_release().
This means that I didn't retain the ack's.

Laurent also caught an ugly devm_kmalloc() in
tinydrm_display_pipe_init() that I've fixed.

I'm practically packing for my 2 weeks of conference travel already, so
only very cursory read of the initial patches for core I 
think

this looks really splendid now.

But I won't have time for review for the next few week, would be 
good if

you could drag some others into this discussions. Iirc there's recently
been a few different people interested in udl (even some patches I 
think),

they might be able to help out with testing

Also, would be great if you can submit this to intel-gfx on the next
round, so that our CI can pick it up and give it a solid beating. 
Touching

critical core paths like in patch 1 is always a bit scary.

While reviewing xen-front's hotunplug handling I just realized this
never landed. Can you pls resend and nag me to review it properly? I'd
really like to get the drm_dev_unplug stuff sorted out better.


My plan was to pick this up after switching tinydrm over to vmalloc 
buffers,

but that work is now waiting for the generic fbdev emulation to land.

I'm currently wandering around inside drm_fb_helper looking for a way 
out,

I can feel the draft so there has to be an exit somewhere. I hope that in
a week or two I'm done with the next version of the RFC using the
drm_fb_helper mode setting code instead of the ioctl's.

At that point it will be a good thing to flush my "caches" of the
drm_fb_helper code, since after looking at it for a long time, I really
don't see the details anymore. So I'll pick up the unplug series then, at
least the core patches should be trivial to review and get merged if 
the CI

agrees.

Could you please estimate when unplug series can be on review?
I am doing some unplug work in my PV DRM driver and would like
to understand if it is feasible to wait for you or post patches as they are
and then plumb in drm_dev_{enter|exit} later after your work is merged

Thank you,
Oleksandr


Noralf.


Thanks, Daniel


Thanks, Daniel


Noralf.

Noralf Trønnes (11):
   drm: Use srcu to protect drm_device.unplugged
   drm/fb-helper: Support device unplug
   drm/fb-helper: Ensure driver module is pinned in fb_open()
   drm/fb-helper: Don't restore if fbdev is not in use
   drm/fb-cma-helper: Make struct drm_fbdev_cma public
   drm/fb-cma-helper: Support device unplug
   drm/tinydrm: Drop driver registered message
   drm/tinydrm: Embed drm_device in tinydrm_device
   drm/tinydrm: Support device unplug in core
   drm/tinydrm/mi0283qt: Let the display pipe handle power
   drm/tinydrm: Support device unplug in drivers

  drivers/gpu/drm/drm_drv.c   |  54 +--
  drivers/gpu/drm/drm_fb_cma_helper.c |  13 +--
  drivers/gpu/drm/drm_fb_helper.c | 108 
--
  drivers/gpu/drm/tinydrm/core/tinydrm-core.c | 135 
+++-

  drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c |  28 +++---
  drivers/gpu/drm/tinydrm/mi0283qt.c  |  81 +
  drivers/gpu/drm/tinydrm/mipi-dbi.c  |  58 +---
  drivers/gpu/drm/tinydrm/repaper.c   |  62 -
  drivers/gpu/drm/tinydrm/st7586.c    |  54 ++-
  include/drm/drm_device.h    |   9 +-
  include/drm/drm_drv.h   |  15 +++-
  include/drm/drm_fb_cma_helper.h |  11 ++-
  include/drm/drm_fb_helper.h |  28 ++
  include/drm/tinydrm/mipi-dbi.h  |   1 +
  include/drm/tinydrm/tinydrm.h   |  10 ++-
  15 files changed, 469 insertions(+), 198 deletions(-)

--
2.7.4


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


-- Daniel Vetter Software Engineer, Intel Corporation +41 

[Bug 104090] Reduced colors on RX580 through eDP on Asus GL702ZC laptop

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=104090

--- Comment #16 from Hein-Pieter van Braam  ---
It appears that 4.16.0-rc7 fixed the issue on this laptop. I'll do a bit more
testing but it seems on par with a system without dc now. Great stuff.

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