[Bug 110702] segfault in radeonsi HEVC hardware decoding

[bugzilla-daemon]

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

--- Comment #1 from Pierre Ossman  ---
Created attachment 144290
  --> https://bugs.freedesktop.org/attachment.cgi?id=144290=edit
gdb bt full

A backtrace of the crash from gdb.

It looks like the allocation filed, but then either kodi or the driver went
ahead and tried to clear the buffer anyway.

-- 
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 110701] GPU faults in in Unigine Valley 1.0

[bugzilla-daemon]

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

--- Comment #5 from Yury Zhuravlev  ---
> can you try to build mesa for previous commit? Like
> 6b3343e5d80abf162b45f0d7e977449588824706 
> 
> I think we need to change the title of this bug.

sorry, it's also unstable, but I can't reproduce error easily. 
Anyway, try commit before Marek patches
d65b160e6a8712a33d72bea1a1b49587d483a18a

-- 
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 110701] GPU faults in in Unigine Valley 1.0

[bugzilla-daemon]

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

--- Comment #4 from Yury Zhuravlev  ---
I made bisect and I found bad commit it's here
https://cgit.freedesktop.org/mesa/mesa/commit/?id=4549c3678865236216952f649fa5ed0115fe81b9

can you try to build mesa for previous commit? Like
6b3343e5d80abf162b45f0d7e977449588824706 

I think we need to change the title of this bug.

-- 
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 110701] GPU faults in in Unigine Valley 1.0

[bugzilla-daemon]

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

--- Comment #3 from Yury Zhuravlev  ---
Similar, after the update today I have the same errors even for KDE KWin. 
Looks like Marek something did.

Error:
[  240.649210] amdgpu :1f:00.0: [gfxhub] no-retry page fault (src_id:0
ring:24 vmid:7 pasid:32769, for process systemsettings5 pid 12567 thread
systemsett:cs0 pid 12571)   
[  240.649211] amdgpu :1f:00.0:   in page starting at address
0x800100be6000 from 27
[  240.649212] amdgpu :1f:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x00701031
[  240.649215] amdgpu :1f:00.0: [gfxhub] no-retry page fault (src_id:0
ring:24 vmid:7 pasid:32769, for process systemsettings5 pid 12567 thread
systemsett:cs0 pid 12571)   
[  240.649216] amdgpu :1f:00.0:   in page starting at address
0x800100bf3000 from 27
[  240.649217] amdgpu :1f:00.0: VM_L2_PROTECTION_FAULT_STATUS:0x
[  240.649220] amdgpu :1f:00.0: [gfxhub] no-retry page fault (src_id:0
ring:24 vmid:7 pasid:32769, for process systemsettings5 pid 12567 thread
systemsett:cs0 pid 12571)

and etc. 

My spec:
OpenGL renderer string: Radeon RX Vega (VEGA10, DRM 3.30.0, 5.1.0-gentoo, LLVM
9.0.0)
OpenGL core profile version string: 4.5 (Core Profile) Mesa 19.2.0-devel
(git-28c2ce7105)

-- 
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 110701] GPU faults in in Unigine Valley 1.0

[bugzilla-daemon]

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

--- Comment #2 from Christian Widmer  ---
I should probably mention that I am using a Radeon RX580 on Gentoo with kernel
5.1.3 and a pretty recent git version of LLVM.

-- 
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 110701] GPU faults in in Unigine Valley 1.0

[bugzilla-daemon]

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

--- Comment #1 from Christian Widmer  ---
Having a similar issue where using OpenGL in certain applications (specifically
encountered the problem in Anki with hardware acceleration and mpv with the
x11egl context and vaapi hardware decoding) causes dmesg being filled with GPU
faults, I decided to bisect mesa and was able to identify commit [1] as being
the culprit. Reverting that one makes the issue disappear for me. Does this, by
any chance, solve the issue for you as well?

[1] [78e35df52aa2f7d770f929a0866a0faa89c261a9] radeonsi: update buffer
descriptors in all contexts after buffer invalidation

-- 
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: lima_bo memory leak after drm_sched job destruction rework

[Grodzovsky, Andrey]

Don't have the code in front of me now but as far as I remember it will only 
prematurely terminate in drm_sched_cleanup_jobs if there is timeout work in 
progress which would not be the case if nothing hangs.

Andrey


From: Erico Nunes 
Sent: 17 May 2019 17:42:48
To: Grodzovsky, Andrey
Cc: Deucher, Alexander; Koenig, Christian; Zhou, David(ChunMing); David Airlie; 
Daniel Vetter; Lucas Stach; Russell King; Christian Gmeiner; Qiang Yu; Rob 
Herring; Tomeu Vizoso; Eric Anholt; Rex Zhu; Huang, Ray; Deng, Emily; Nayan 
Deshmukh; Sharat Masetty; amd-...@lists.freedesktop.org; 
dri-devel@lists.freedesktop.org; l...@lists.freedesktop.org
Subject: Re: lima_bo memory leak after drm_sched job destruction rework

[CAUTION: External Email]

On Fri, May 17, 2019 at 10:43 PM Grodzovsky, Andrey
 wrote:
> On 5/17/19 3:35 PM, Erico Nunes wrote:
> > Lima currently defaults to an "infinite" timeout. Setting a 500ms
> > default timeout like most other drm_sched users do fixed the leak for
> > me.
>
> I am not very clear about the problem - so you basically never allow a
> time out handler to run ? And then when the job hangs for ever you get
> this memory leak ? How it worked for you before this refactoring ? As
> far as I remember  sched->ops->free_job before this change was called
> from drm_sched_job_finish which is the work scheduled from HW fence
> signaled callback - drm_sched_process_job so if your job hangs for ever
> anyway and this work is never scheduled  when your free_job callback was
> called ?

In this particular case, the jobs run successfully, nothing hangs.
Lima currently specifies an "infinite" timeout to the drm scheduler,
so if a job did did hang, it would hang forever, I suppose. But this
is not the issue.

If I understand correctly it worked well before the rework because the
cleanup was triggered at the end of drm_sched_process_job
independently on the timeout.

What I'm observing now is that even when jobs run successfully, they
are not cleaned by the drm scheduler because drm_sched_cleanup_jobs
seems to give up based on the status of a timeout worker.
I would expect the timeout value to only be relevant in error/hung job cases.

I will probably set the timeout to a reasonable value anyway, I just
posted here to report that this can possibly be a bug in the drm
scheduler after that rework.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v6 2/3] dt-bindings: backlight: add lm3630a bindings

[Brian Masney]

Hi Rob,

On Fri, May 17, 2019 at 04:11:48PM -0500, Rob Herring wrote:
> On Wed, Apr 24, 2019 at 4:25 AM Brian Masney  wrote:
> >
> > Add new backlight bindings for the TI LM3630A dual-string white LED.
> >
> > Signed-off-by: Brian Masney 
> > Reviewed-by: Rob Herring 
> > ---
> > Changes since v5:
> > - Change 'lm3630a_bl@38' in examples to 'led-controller@38'
> >
> > Changes since v4:
> > - Drop $ref from led-sources
> > - Drop description from reg of i2c address
> > - Expand description of reg for the control bank
> > - Drop status from examples
> >
> > Changes since v3:
> > - Add label. I didn't add a description for it since that'll come from
> >   the common properties once its converted.
> >
> > Changes since v2:
> > - Update description of max-brightness
> > - Add description for reg
> > - Correct typo: s/tranisiton/transition
> > - add reg to control banks
> > - add additionalProperties
> >
> >  .../leds/backlight/lm3630a-backlight.yaml | 129 ++
> >  1 file changed, 129 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml
> 
> I'm working on getting the examples to be validated by the schema (in
> addition to just building with dtc) and there's a couple of errors:
> 
> Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
> '#address-cells', '#size-cells' do not match any of the regexes:
> '^led@[01]$', 'pinctrl-[0-9]+'
> Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
> '#address-cells', '#size-cells' do not match any of the regexes:
> '^led@[01]$', 'pinctrl-[0-9]+'
> 
> You didn't list '#address-cells' and '#size-cells'.
> 
> Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
> led@0: 'ti,linear-mapping-mode' does not match any of the regexes:
> 'pinctrl-[0-9]+'
> Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
> led@1: 'ti,linear-mapping-mode' does not match any of the regexes:
> 'pinctrl-[0-9]+'
> 
> 'ti,linear-mapping-mode' is not defined in the child nodes.

I'm sorry about that. I'll send out a patch this weekend to correct
this. I have 'make dt_binding_check' in my local build script. Is there
something else that I should be running as well? Or do you have a
branch somewhere with your validation work that I can test my changes
against?

Brian

[PATCH 1/3] media: vsp1: drm: Split vsp1_du_setup_lif()

[Kieran Bingham]

Break vsp1_du_setup_lif() into components more suited to the DRM Atomic
API. The existing vsp1_du_setup_lif() API call is maintained as it is
still used from the DU.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 233 ++---
 include/media/vsp1.h   |  32 +++-
 2 files changed, 199 insertions(+), 66 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index a4a45d68a6ef..ce5c0780680f 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -616,18 +616,15 @@ int vsp1_du_init(struct device *dev)
 EXPORT_SYMBOL_GPL(vsp1_du_init);
 
 /**
- * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
+ * vsp1_du_atomic_modeset - Configure the mode as part of an atomic update
  * @dev: the VSP device
  * @pipe_index: the DRM pipeline index
- * @cfg: the LIF configuration
+ * @cfg: the mode configuration
  *
  * Configure the output part of VSP DRM pipeline for the given frame @cfg.width
  * and @cfg.height. This sets up formats on the BRx source pad, the WPF sink 
and
  * source pads, and the LIF sink pad.
  *
- * The @pipe_index argument selects which DRM pipeline to setup. The number of
- * available pipelines depend on the VSP instance.
- *
  * As the media bus code on the blend unit source pad is conditioned by the
  * configuration of its sink 0 pad, we also set up the formats on all blend 
unit
  * sinks, even if the configuration will be overwritten later by
@@ -635,15 +632,14 @@ EXPORT_SYMBOL_GPL(vsp1_du_init);
  * a well defined state.
  *
  * Return 0 on success or a negative error code on failure.
+ *
  */
-int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
- const struct vsp1_du_lif_config *cfg)
+int vsp1_du_atomic_modeset(struct device *dev, unsigned int pipe_index,
+  const struct vsp1_du_modeset_config *cfg)
 {
struct vsp1_device *vsp1 = dev_get_drvdata(dev);
struct vsp1_drm_pipeline *drm_pipe;
struct vsp1_pipeline *pipe;
-   unsigned long flags;
-   unsigned int i;
int ret;
 
if (pipe_index >= vsp1->info->lif_count)
@@ -652,60 +648,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
drm_pipe = >drm->pipe[pipe_index];
pipe = _pipe->pipe;
 
-   if (!cfg) {
-   struct vsp1_brx *brx;
-
-   mutex_lock(>drm->lock);
-
-   brx = to_brx(>brx->subdev);
-
-   /*
-* NULL configuration means the CRTC is being disabled, stop
-* the pipeline and turn the light off.
-*/
-   ret = vsp1_pipeline_stop(pipe);
-   if (ret == -ETIMEDOUT)
-   dev_err(vsp1->dev, "DRM pipeline stop timeout\n");
-
-   for (i = 0; i < ARRAY_SIZE(pipe->inputs); ++i) {
-   struct vsp1_rwpf *rpf = pipe->inputs[i];
-
-   if (!rpf)
-   continue;
-
-   /*
-* Remove the RPF from the pipe and the list of BRx
-* inputs.
-*/
-   WARN_ON(!rpf->entity.pipe);
-   rpf->entity.pipe = NULL;
-   list_del(>entity.list_pipe);
-   pipe->inputs[i] = NULL;
-
-   brx->inputs[rpf->brx_input].rpf = NULL;
-   }
-
-   drm_pipe->du_complete = NULL;
-   pipe->num_inputs = 0;
-
-   dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
-   __func__, pipe->lif->index,
-   BRX_NAME(pipe->brx));
-
-   list_del(>brx->list_pipe);
-   pipe->brx->pipe = NULL;
-   pipe->brx = NULL;
-
-   mutex_unlock(>drm->lock);
-
-   vsp1_dlm_reset(pipe->output->dlm);
-   vsp1_device_put(vsp1);
-
-   dev_dbg(vsp1->dev, "%s: pipeline disabled\n", __func__);
-
-   return 0;
-   }
-
drm_pipe->width = cfg->width;
drm_pipe->height = cfg->height;
pipe->interlaced = cfg->interlaced;
@@ -722,8 +664,43 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int 
pipe_index,
goto unlock;
 
ret = vsp1_du_pipeline_setup_output(vsp1, pipe);
-   if (ret < 0)
-   goto unlock;
+
+unlock:
+   mutex_unlock(>drm->lock);
+
+   return ret;
+}
+
+/**
+ * vsp1_du_atomic_enable - Enable and start a DU pipeline
+ * @dev: the VSP device
+ * @pipe_index: the DRM pipeline index
+ * @cfg: the enablement configuration
+ *
+ * The @pipe_index argument selects which DRM pipeline to enable. The number of
+ * available pipelines depend on the VSP instance.
+ *
+ * The configuration passes a callback function to register notification of
+ * frame completion 

[PATCH 2/3] drm: rcar-du: Convert to the new VSP atomic API

[Kieran Bingham]

The configuration API between the VSP and the DU has been updated to
provide finer grain control over modesetting, and enablement.

Split rcar_du_vsp_enable() into rcar_du_vsp_modeset() and
rcar_du_vsp_enable() accordingly, and update each function to use the
new VSP API.

There are no further users of the deprecated vsp1_du_setup_lif() which
can now be removed.

Signed-off-by: Kieran Bingham 
---
 drivers/gpu/drm/rcar-du/rcar_du_crtc.c |  4 +++-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 21 +++--
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h  |  2 ++
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c 
b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
index 2da46e3dc4ae..cccd6fe85749 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
@@ -492,8 +492,10 @@ static void rcar_du_crtc_setup(struct rcar_du_crtc *rcrtc)
rcar_du_group_write(rcrtc->group, rcrtc->index % 2 ? DS2PR : DS1PR, 0);
 
/* Enable the VSP compositor. */
-   if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE))
+   if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) {
+   rcar_du_vsp_modeset(rcrtc);
rcar_du_vsp_enable(rcrtc);
+   }
 
/* Turn vertical blanking interrupt reporting on. */
drm_crtc_vblank_on(>crtc);
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
index 5e4faf258c31..c170427fcad9 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
@@ -44,16 +44,14 @@ static void rcar_du_vsp_complete(void *private, unsigned 
int status, u32 crc)
drm_crtc_add_crc_entry(>crtc, false, 0, );
 }
 
-void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
+void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc)
 {
const struct drm_display_mode *mode = >crtc.state->adjusted_mode;
struct rcar_du_device *rcdu = crtc->dev;
-   struct vsp1_du_lif_config cfg = {
+   struct vsp1_du_modeset_config cfg = {
.width = mode->hdisplay,
.height = mode->vdisplay,
.interlaced = mode->flags & DRM_MODE_FLAG_INTERLACE,
-   .callback = rcar_du_vsp_complete,
-   .callback_data = crtc,
};
struct rcar_du_plane_state state = {
.state = {
@@ -90,12 +88,23 @@ void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
 */
crtc->group->need_restart = true;
 
-   vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, );
+
+   vsp1_du_atomic_modeset(crtc->vsp->vsp, crtc->vsp_pipe, );
+}
+
+void rcar_du_vsp_enable(struct rcar_du_crtc *crtc)
+{
+   struct vsp1_du_enable_config cfg = {
+   .callback = rcar_du_vsp_complete,
+   .callback_data = crtc,
+   };
+
+   vsp1_du_atomic_enable(crtc->vsp->vsp, crtc->vsp_pipe, );
 }
 
 void rcar_du_vsp_disable(struct rcar_du_crtc *crtc)
 {
-   vsp1_du_setup_lif(crtc->vsp->vsp, crtc->vsp_pipe, NULL);
+   vsp1_du_atomic_disable(crtc->vsp->vsp, crtc->vsp_pipe);
 }
 
 void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc)
diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h 
b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
index 9b4724159378..a6f6bb4690f2 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
+++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.h
@@ -58,6 +58,7 @@ to_rcar_vsp_plane_state(struct drm_plane_state *state)
 #ifdef CONFIG_DRM_RCAR_VSP
 int rcar_du_vsp_init(struct rcar_du_vsp *vsp, struct device_node *np,
 unsigned int crtcs);
+void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc);
 void rcar_du_vsp_enable(struct rcar_du_crtc *crtc);
 void rcar_du_vsp_disable(struct rcar_du_crtc *crtc);
 void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc);
@@ -73,6 +74,7 @@ static inline int rcar_du_vsp_init(struct rcar_du_vsp *vsp,
 {
return -ENXIO;
 }
+static inlinc void rcar_du_vsp_modeset(struct rcar_du_crtc *crtc) { };
 static inline void rcar_du_vsp_enable(struct rcar_du_crtc *crtc) { };
 static inline void rcar_du_vsp_disable(struct rcar_du_crtc *crtc) { };
 static inline void rcar_du_vsp_atomic_begin(struct rcar_du_crtc *crtc) { };
-- 
2.20.1

[PATCH 3/3] media: vsp1: drm: Remove vsp1_du_setup_lif()

[Kieran Bingham]

The vsp1_du_setup_lif() function is deprecated, and the users have been
removed. Remove the implementation and the associated configuration
structure.

Signed-off-by: Kieran Bingham 
---
 drivers/media/platform/vsp1/vsp1_drm.c | 47 --
 include/media/vsp1.h   | 22 
 2 files changed, 69 deletions(-)

diff --git a/drivers/media/platform/vsp1/vsp1_drm.c 
b/drivers/media/platform/vsp1/vsp1_drm.c
index ce5c0780680f..12f76344bdec 100644
--- a/drivers/media/platform/vsp1/vsp1_drm.c
+++ b/drivers/media/platform/vsp1/vsp1_drm.c
@@ -817,53 +817,6 @@ int vsp1_du_atomic_disable(struct device *dev, unsigned 
int pipe_index)
 }
 EXPORT_SYMBOL_GPL(vsp1_du_atomic_disable);
 
-/**
- * vsp1_du_setup_lif - Setup the output part of the VSP pipeline
- * @dev: the VSP device
- * @pipe_index: the DRM pipeline index
- * @cfg: the LIF configuration
- *
- * Configure the output part of VSP DRM pipeline for the given frame @cfg.width
- * and @cfg.height. This sets up formats on the BRx source pad, the WPF sink 
and
- * source pads, and the LIF sink pad.
- *
- * The @pipe_index argument selects which DRM pipeline to setup. The number of
- * available pipelines depend on the VSP instance.
- *
- * As the media bus code on the blend unit source pad is conditioned by the
- * configuration of its sink 0 pad, we also set up the formats on all blend 
unit
- * sinks, even if the configuration will be overwritten later by
- * vsp1_du_setup_rpf(). This ensures that the blend unit configuration is set 
to
- * a well defined state.
- *
- * Return 0 on success or a negative error code on failure.
- */
-int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
- const struct vsp1_du_lif_config *cfg)
-{
-   int ret;
-
-   struct vsp1_du_modeset_config modes = {
-   .width = cfg->width,
-   .height = cfg->height,
-   .interlaced = cfg->interlaced,
-   };
-   struct vsp1_du_enable_config enable = {
-   .callback = cfg->callback,
-   .callback_data = cfg->callback_data,
-   };
-
-   if (!cfg)
-   return vsp1_du_atomic_disable(dev, pipe_index);
-
-   ret = vsp1_du_atomic_modeset(dev, pipe_index, );
-   if (ret)
-   return ret;
-
-   return vsp1_du_atomic_enable(dev, pipe_index, );
-}
-EXPORT_SYMBOL_GPL(vsp1_du_setup_lif);
-
 /**
  * vsp1_du_atomic_begin - Prepare for an atomic update
  * @dev: the VSP device
diff --git a/include/media/vsp1.h b/include/media/vsp1.h
index 13f5a1c4d45a..bc0a26d33d9a 100644
--- a/include/media/vsp1.h
+++ b/include/media/vsp1.h
@@ -20,28 +20,6 @@ int vsp1_du_init(struct device *dev);
 #define VSP1_DU_STATUS_COMPLETEBIT(0)
 #define VSP1_DU_STATUS_WRITEBACK   BIT(1)
 
-/**
- * struct vsp1_du_lif_config - VSP LIF configuration - Deprecated
- * @width: output frame width
- * @height: output frame height
- * @interlaced: true for interlaced pipelines
- * @callback: frame completion callback function (optional). When a callback
- *   is provided, the VSP driver guarantees that it will be called once
- *   and only once for each vsp1_du_atomic_flush() call.
- * @callback_data: data to be passed to the frame completion callback
- */
-struct vsp1_du_lif_config {
-   unsigned int width;
-   unsigned int height;
-   bool interlaced;
-
-   void (*callback)(void *data, unsigned int status, u32 crc);
-   void *callback_data;
-};
-
-int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index,
- const struct vsp1_du_lif_config *cfg);
-
 /**
  * struct vsp1_du_modeset_config - VSP LIF Mode configuration
  * @width: output frame width
-- 
2.20.1

[RFC PATCH 0/3] VSP1/DU atomic interface changes

[Kieran Bingham]

As part of the ongoing DU group refactoring it became apparent that we need to
split the configuration of the VSP to allow fine grain control of setting the
VSP1 mode configuration and enabling/disabling of the pipeline.

To split the mode configuration and the pipeline enablement, we add three new
calls:

 - vsp1_du_atomic_modeset()
 - vsp1_du_atomic_enable()
 - vsp1_du_atomic_disable()

To support the cross-component API, the new interface is added in [patch 1/3],
including an implementation of vsp1_du_setup_lif() to support the transition.

The DRM usage is adapted in [patch 2/3], before the call is removed entirely in
[patch 3/3]

Whilst these patches are independent and could be reviewed separately, they are
not expected to be integrated until the associated group rework is completed.

Kieran Bingham (3):
  media: vsp1: drm: Split vsp1_du_setup_lif()
  drm: rcar-du: Convert to the new VSP atomic API
  media: vsp1: drm: Remove vsp1_du_setup_lif()

 drivers/gpu/drm/rcar-du/rcar_du_crtc.c |   4 +-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.c  |  21 ++-
 drivers/gpu/drm/rcar-du/rcar_du_vsp.h  |   2 +
 drivers/media/platform/vsp1/vsp1_drm.c | 188 -
 include/media/vsp1.h   |  26 ++--
 5 files changed, 159 insertions(+), 82 deletions(-)

-- 
2.20.1

Re: [PATCH v5 1/6] drm: panfrost: add optional bus_clock

[Rob Herring]

On Fri, May 17, 2019 at 5:08 PM Clément Péron  wrote:
>
> Hi Rob,
>
> On Fri, 17 May 2019 at 22:07, Rob Herring  wrote:
> >
> > On Fri, May 17, 2019 at 1:47 PM Clément Péron  wrote:
> > >
> > > Allwinner H6 has an ARM Mali-T720 MP2 which required a bus_clock.
> > >
> > > Add an optional bus_clock at the init of the panfrost driver.
> > >
> > > Signed-off-by: Clément Péron 
> > > ---
> > >  drivers/gpu/drm/panfrost/panfrost_device.c | 25 +-
> > >  drivers/gpu/drm/panfrost/panfrost_device.h |  1 +
> > >  2 files changed, 25 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c 
> > > b/drivers/gpu/drm/panfrost/panfrost_device.c
> > > index 3b2bced1b015..8da6e612d384 100644
> > > --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> > > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> > > @@ -44,7 +44,8 @@ static int panfrost_clk_init(struct panfrost_device 
> > > *pfdev)
> > >
> > > pfdev->clock = devm_clk_get(pfdev->dev, NULL);
> > > if (IS_ERR(pfdev->clock)) {
> > > -   dev_err(pfdev->dev, "get clock failed %ld\n", 
> > > PTR_ERR(pfdev->clock));
> > > +   dev_err(pfdev->dev, "get clock failed %ld\n",
> > > +   PTR_ERR(pfdev->clock));
> >
> > Please drop this whitespace change.
>
> Sorry, I thought it was only a mistake here, I will drop it.
> Why are they so many lines over 80 characters?

I'd guess most are prints and/or just slightly over.

> Is there a specific coding style to follow ?

Yes, but generally the 80 character thing is more a guidance. Not
having unrelated changes in a single commit is more of a hard rule.

Rob

Re: [PATCH v5 1/6] drm: panfrost: add optional bus_clock

[Clément Péron]

Hi Rob,

On Fri, 17 May 2019 at 22:07, Rob Herring  wrote:
>
> On Fri, May 17, 2019 at 1:47 PM Clément Péron  wrote:
> >
> > Allwinner H6 has an ARM Mali-T720 MP2 which required a bus_clock.
> >
> > Add an optional bus_clock at the init of the panfrost driver.
> >
> > Signed-off-by: Clément Péron 
> > ---
> >  drivers/gpu/drm/panfrost/panfrost_device.c | 25 +-
> >  drivers/gpu/drm/panfrost/panfrost_device.h |  1 +
> >  2 files changed, 25 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c 
> > b/drivers/gpu/drm/panfrost/panfrost_device.c
> > index 3b2bced1b015..8da6e612d384 100644
> > --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> > +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> > @@ -44,7 +44,8 @@ static int panfrost_clk_init(struct panfrost_device 
> > *pfdev)
> >
> > pfdev->clock = devm_clk_get(pfdev->dev, NULL);
> > if (IS_ERR(pfdev->clock)) {
> > -   dev_err(pfdev->dev, "get clock failed %ld\n", 
> > PTR_ERR(pfdev->clock));
> > +   dev_err(pfdev->dev, "get clock failed %ld\n",
> > +   PTR_ERR(pfdev->clock));
>
> Please drop this whitespace change.

Sorry, I thought it was only a mistake here, I will drop it.
Why are they so many lines over 80 characters?
Is there a specific coding style to follow ?

Thanks,
Clement

>
> > return PTR_ERR(pfdev->clock);
> > }
> >

Re: [PATCH] drm: rcar-du: writeback: include interface header

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Fri, May 17, 2019 at 10:20:49PM +0100, Kieran Bingham wrote:
> The new writeback feature is exports functions so that they can
> integrate into the rcar_du_kms module.
> 
> The interface functions are defined in the rcar_du_writeback header, but
> it is not included in the object file itself leading to compiler
> warnings for missing prototypes.
> 
> Include the header as appropriate.
> 
> Signed-off-by: Kieran Bingham 

Reviewed-by: Laurent Pinchart 

And applied to my tree.

> ---
>  drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c 
> b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
> index 989a0be94131..ae07290bba6a 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
> @@ -14,6 +14,7 @@
>  #include "rcar_du_crtc.h"
>  #include "rcar_du_drv.h"
>  #include "rcar_du_kms.h"
> +#include "rcar_du_writeback.h"
>  
>  /**
>   * struct rcar_du_wb_conn_state - Driver-specific writeback connector state

-- 
Regards,

Laurent Pinchart

[PATCH] drm: rcar-du: writeback: include interface header

[Kieran Bingham]

The new writeback feature is exports functions so that they can
integrate into the rcar_du_kms module.

The interface functions are defined in the rcar_du_writeback header, but
it is not included in the object file itself leading to compiler
warnings for missing prototypes.

Include the header as appropriate.

Signed-off-by: Kieran Bingham 
---
 drivers/gpu/drm/rcar-du/rcar_du_writeback.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c 
b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
index 989a0be94131..ae07290bba6a 100644
--- a/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
+++ b/drivers/gpu/drm/rcar-du/rcar_du_writeback.c
@@ -14,6 +14,7 @@
 #include "rcar_du_crtc.h"
 #include "rcar_du_drv.h"
 #include "rcar_du_kms.h"
+#include "rcar_du_writeback.h"
 
 /**
  * struct rcar_du_wb_conn_state - Driver-specific writeback connector state
-- 
2.20.1

Re: [PATCH v6 2/3] dt-bindings: backlight: add lm3630a bindings

[Rob Herring]

On Wed, Apr 24, 2019 at 4:25 AM Brian Masney  wrote:
>
> Add new backlight bindings for the TI LM3630A dual-string white LED.
>
> Signed-off-by: Brian Masney 
> Reviewed-by: Rob Herring 
> ---
> Changes since v5:
> - Change 'lm3630a_bl@38' in examples to 'led-controller@38'
>
> Changes since v4:
> - Drop $ref from led-sources
> - Drop description from reg of i2c address
> - Expand description of reg for the control bank
> - Drop status from examples
>
> Changes since v3:
> - Add label. I didn't add a description for it since that'll come from
>   the common properties once its converted.
>
> Changes since v2:
> - Update description of max-brightness
> - Add description for reg
> - Correct typo: s/tranisiton/transition
> - add reg to control banks
> - add additionalProperties
>
>  .../leds/backlight/lm3630a-backlight.yaml | 129 ++
>  1 file changed, 129 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.yaml

I'm working on getting the examples to be validated by the schema (in
addition to just building with dtc) and there's a couple of errors:

Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
'#address-cells', '#size-cells' do not match any of the regexes:
'^led@[01]$', 'pinctrl-[0-9]+'
Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
'#address-cells', '#size-cells' do not match any of the regexes:
'^led@[01]$', 'pinctrl-[0-9]+'

You didn't list '#address-cells' and '#size-cells'.

Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
led@0: 'ti,linear-mapping-mode' does not match any of the regexes:
'pinctrl-[0-9]+'
Documentation/devicetree/bindings/leds/backlight/lm3630a-backlight.example.dt.yaml:
led@1: 'ti,linear-mapping-mode' does not match any of the regexes:
'pinctrl-[0-9]+'

'ti,linear-mapping-mode' is not defined in the child nodes.

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

Re: lima_bo memory leak after drm_sched job destruction rework

[Grodzovsky, Andrey]

On 5/17/19 3:35 PM, Erico Nunes wrote:
> [CAUTION: External Email]
>
> Hello,
>
> I have recently observed a memory leak issue with lima using
> drm-misc-next, which I initially reported here:
> https://gitlab.freedesktop.org/lima/linux/issues/24
> It is an easily reproduceable memory leak which I was able to bisect to 
> commit:
>
> 5918045c4ed4 drm/scheduler: rework job destruction
>
> After some investigation, it seems that after the refactor,
> sched->ops->free_job (in lima: lima_sched_free_job) is no longer
> called.
> With some more debugging I found that it is not being called because
> the job freeing is now in drm_sched_cleanup_jobs, which for lima
> always aborts in the initial "Don't destroy jobs while the timeout
> worker is running" condition.
>
> Lima currently defaults to an "infinite" timeout. Setting a 500ms
> default timeout like most other drm_sched users do fixed the leak for
> me.


I am not very clear about the problem - so you basically never allow a 
time out handler to run ? And then when the job hangs for ever you get 
this memory leak ? How it worked for you before this refactoring ? As 
far as I remember  sched->ops->free_job before this change was called 
from drm_sched_job_finish which is the work scheduled from HW fence 
signaled callback - drm_sched_process_job so if your job hangs for ever 
anyway and this work is never scheduled  when your free_job callback was 
called ?


>
> I can send a patch to set a 500ms timeout and have it probably working
> again, but I am wondering now if this is expected behaviour for
> drm_sched after the refactor.
> In particular I also noticed that drm_sched_suspend_timeout is not
> called anywhere. Is it expected that we now rely on a timeout
> parameter to cleanup jobs that ran successfully?


AFAIK the drm_sched_suspend_timeout is used by a driver in a staging 
branch, Christian can give more detail.

Andrey


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

Re: [PATCH v5 1/6] drm: panfrost: add optional bus_clock

[Rob Herring]

On Fri, May 17, 2019 at 1:47 PM Clément Péron  wrote:
>
> Allwinner H6 has an ARM Mali-T720 MP2 which required a bus_clock.
>
> Add an optional bus_clock at the init of the panfrost driver.
>
> Signed-off-by: Clément Péron 
> ---
>  drivers/gpu/drm/panfrost/panfrost_device.c | 25 +-
>  drivers/gpu/drm/panfrost/panfrost_device.h |  1 +
>  2 files changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c 
> b/drivers/gpu/drm/panfrost/panfrost_device.c
> index 3b2bced1b015..8da6e612d384 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_device.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_device.c
> @@ -44,7 +44,8 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
>
> pfdev->clock = devm_clk_get(pfdev->dev, NULL);
> if (IS_ERR(pfdev->clock)) {
> -   dev_err(pfdev->dev, "get clock failed %ld\n", 
> PTR_ERR(pfdev->clock));
> +   dev_err(pfdev->dev, "get clock failed %ld\n",
> +   PTR_ERR(pfdev->clock));

Please drop this whitespace change.

> return PTR_ERR(pfdev->clock);
> }
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 110702] segfault in radeonsi HEVC hardware decoding

[bugzilla-daemon]

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

Bug ID: 110702
   Summary: segfault in radeonsi HEVC hardware decoding
   Product: Mesa
   Version: unspecified
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: pierre-bugzi...@ossman.eu
QA Contact: dri-devel@lists.freedesktop.org

A few HEVC files gives me a segfault in radeonsi_dri.so when trying to play
them via Kodi. They work fine when decoding in software, and other HEVC files
work fine being decoded in hardware. I do not know what is special about these
files.

> amdgpu: Failed to allocate a buffer:
> amdgpu:size  : 3221295104 bytes
> amdgpu:alignment : 4096 bytes
> amdgpu:domains   : 4
> amdgpu: Failed to allocate a buffer:
> amdgpu:size  : 3221295104 bytes
> amdgpu:alignment : 4096 bytes
> amdgpu:domains   : 4
> EE ../src/gallium/drivers/radeon/radeon_vcn_dec.c:880 rvcn_dec_message_decode 
> UVD - Can't allocated context buffer.
> /usr/bin/kodi: line 219:  1223 Segmentation fault  (core dumped) 
> ${KODI_BINARY} $SAVED_ARGS

Lines seen in ~/.xsession-errors

This is the details Kodi reports about the video:

> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:   
> Metadata:
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]: 
> ENCODER : Lavf58.27.102
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:   
> Duration: 01:29:45.38, start: 0.00, bitrate: 1598 kb/s
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]: 
> Stream #0:0(eng): Video: hevc (Main 10), yuv420p10le(tv), 1920x1080 [SAR 1:1 
> DAR 16:9], 23.98 fps, 23.98 tbr, 1k tbn, 23.98 tbc (default)
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]: 
> Metadata:
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  BPS-eng : 8246896
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  DURATION-eng: 01:29:45.38000
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  NUMBER_OF_FRAMES-eng: 129120
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  NUMBER_OF_BYTES-eng: 5551584033
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  _STATISTICS_WRITING_APP-eng: mkvmerge v33.1.0 ('Primrose') 64-bit
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  _STATISTICS_WRITING_DATE_UTC-eng: 2019-05-13 00:59:51
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  _STATISTICS_TAGS-eng: BPS DURATION NUMBER_OF_FRAMES NUMBER_OF_BYTES
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  ENCODER : Lavc58.51.100 libx265
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  DURATION: 01:29:45.38000
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]: 
> Stream #0:1(eng): Audio: eac3, 48000 Hz, 6 channels, fltp (default)
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]: 
> Metadata:
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  BPS-eng : 64
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  DURATION-eng: 01:29:45.37600
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  NUMBER_OF_FRAMES-eng: 168293
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  NUMBER_OF_BYTES-eng: 430830080
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  _STATISTICS_WRITING_APP-eng: mkvmerge v33.1.0 ('Primrose') 64-bit
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  _STATISTICS_WRITING_DATE_UTC-eng: 2019-05-13 00:59:51
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  _STATISTICS_TAGS-eng: BPS DURATION NUMBER_OF_FRAMES NUMBER_OF_BYTES
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  DURATION: 01:29:45.38400
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]: 
> Stream #0:2(eng): Subtitle: ass
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]: 
> Metadata:
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  BPS-eng : 38
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  DURATION-eng: 01:29:38.0
> 2019-05-17 19:51:31.904 T:140684663195392INFO: ffmpeg[7FF3B3600700]:  
>  NUMBER_OF_FRAMES-eng: 955
> 2019-05-17 19:51:31.905 

Re: [PATCH v4 01/18] kunit: test: add KUnit test runner core

[Stephen Boyd]

Quoting Brendan Higgins (2019-05-14 15:16:54)
> diff --git a/include/kunit/test.h b/include/kunit/test.h
> new file mode 100644
> index 0..e682ea0e1f9a5
> --- /dev/null
> +++ b/include/kunit/test.h
> @@ -0,0 +1,162 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Base unit test (KUnit) API.
> + *
> + * Copyright (C) 2019, Google LLC.
> + * Author: Brendan Higgins 
> + */
> +
> +#ifndef _KUNIT_TEST_H
> +#define _KUNIT_TEST_H
> +
> +#include 
> +#include 

Is this include used here?

> +
> +struct kunit;
> +
> +/**
> + * struct kunit_case - represents an individual test case.
> + * @run_case: the function representing the actual test case.
> + * @name: the name of the test case.
> + *
> + * A test case is a function with the signature, ``void (*)(struct kunit *)``
> + * that makes expectations (see KUNIT_EXPECT_TRUE()) about code under test. 
> Each
> + * test case is associated with a  kunit_module and will be run after 
> the
> + * module's init function and followed by the module's exit function.
> + *
> + * A test case should be static and should only be created with the 
> KUNIT_CASE()
> + * macro; additionally, every array of test cases should be terminated with 
> an
> + * empty test case.
> + *
> + * Example:
> + *
> + * .. code-block:: c
> + *
> + * void add_test_basic(struct kunit *test)
> + * {
> + * KUNIT_EXPECT_EQ(test, 1, add(1, 0));
> + * KUNIT_EXPECT_EQ(test, 2, add(1, 1));
> + * KUNIT_EXPECT_EQ(test, 0, add(-1, 1));
> + * KUNIT_EXPECT_EQ(test, INT_MAX, add(0, INT_MAX));
> + * KUNIT_EXPECT_EQ(test, -1, add(INT_MAX, INT_MIN));
> + * }
> + *
> + * static struct kunit_case example_test_cases[] = {
> + * KUNIT_CASE(add_test_basic),
> + * {},

Nitpick: Please drop the comma on the sentinel so nobody gets ideas to
add another entry after it.

> + * };
> + *
> + */
> +struct kunit_case {
> +   void (*run_case)(struct kunit *test);
> +   const char name[256];

Maybe 256 can be a #define KUNIT_NAME_MAX_LEN? Or it could just be a
const char pointer to a literal pool? Are unit tests making up names at
runtime?

> +
> +   /* private: internal use only. */
> +   bool success;
> +};
> +
> +/**
> + * KUNIT_CASE - A helper for creating a  kunit_case
> + * @test_name: a reference to a test case function.
> + *
> + * Takes a symbol for a function representing a test case and creates a
> + *  kunit_case object from it. See the documentation for
> + *  kunit_case for an example on how to use it.
> + */
> +#define KUNIT_CASE(test_name) { .run_case = test_name, .name = #test_name }
> +
> +/**
> + * struct kunit_module - describes a related collection of  
> kunit_case s.
> + * @name: the name of the test. Purely informational.
> + * @init: called before every test case.
> + * @exit: called after every test case.
> + * @test_cases: a null terminated array of test cases.
> + *
> + * A kunit_module is a collection of related  kunit_case s, such that
> + * @init is called before every test case and @exit is called after every 
> test
> + * case, similar to the notion of a *test fixture* or a *test class* in other
> + * unit testing frameworks like JUnit or Googletest.
> + *
> + * Every  kunit_case must be associated with a kunit_module for KUnit 
> to
> + * run it.
> + */
> +struct kunit_module {
> +   const char name[256];
> +   int (*init)(struct kunit *test);
> +   void (*exit)(struct kunit *test);
> +   struct kunit_case *test_cases;

Can this variable be const? Or we expect test modules to adjust test_cases after
the fact?

> +};
> +
> +/**
> + * struct kunit - represents a running instance of a test.
> + * @priv: for user to store arbitrary data. Commonly used to pass data 
> created
> + * in the init function (see  kunit_module).
> + *
> + * Used to store information about the current context under which the test 
> is
> + * running. Most of this data is private and should only be accessed 
> indirectly
> + * via public functions; the one exception is @priv which can be used by the
> + * test writer to store arbitrary data.
> + */
> +struct kunit {
> +   void *priv;
> +
> +   /* private: internal use only. */
> +   const char *name; /* Read only after initialization! */
> +   spinlock_t lock; /* Gaurds all mutable test state. */
> +   bool success; /* Protected by lock. */
> +};
> +
> +void kunit_init_test(struct kunit *test, const char *name);
> +
> +int kunit_run_tests(struct kunit_module *module);
> +
> +/**
> + * module_test() - used to register a  kunit_module with KUnit.
> + * @module: a statically allocated  kunit_module.
> + *
> + * Registers @module with the test framework. See  kunit_module for 
> more
> + * information.
> + */
> +#define module_test(module) \
> +   static int module_kunit_init##module(void) \
> +   { \
> +   return kunit_run_tests(); \
> +   } \
> +   

[PATCH v5 3/6] dt-bindings: gpu: add bus clock for Mali Midgard GPUs

[Clément Péron]

From: Icenowy Zheng 

Some SoCs adds a bus clock gate to the Mali Midgard GPU.

Add the binding for the bus clock.

Signed-off-by: Icenowy Zheng 
Signed-off-by: Clément Péron 
Reviewed-by: Rob Herring 
---
 Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt 
b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
index 1b1a74129141..2e8bbce35695 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
@@ -31,6 +31,12 @@ Optional properties:
 
 - clocks : Phandle to clock for the Mali Midgard device.
 
+- clock-names : Specify the names of the clocks specified in clocks
+  when multiple clocks are present.
+* core: clock driving the GPU itself (When only one clock is present,
+  assume it's this clock.)
+* bus: bus clock for the GPU
+
 - mali-supply : Phandle to regulator for the Mali device. Refer to
   Documentation/devicetree/bindings/regulator/regulator.txt for details.
 
-- 
2.17.1

[PATCH v5 1/6] drm: panfrost: add optional bus_clock

[Clément Péron]

Allwinner H6 has an ARM Mali-T720 MP2 which required a bus_clock.

Add an optional bus_clock at the init of the panfrost driver.

Signed-off-by: Clément Péron 
---
 drivers/gpu/drm/panfrost/panfrost_device.c | 25 +-
 drivers/gpu/drm/panfrost/panfrost_device.h |  1 +
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c 
b/drivers/gpu/drm/panfrost/panfrost_device.c
index 3b2bced1b015..8da6e612d384 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.c
+++ b/drivers/gpu/drm/panfrost/panfrost_device.c
@@ -44,7 +44,8 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
 
pfdev->clock = devm_clk_get(pfdev->dev, NULL);
if (IS_ERR(pfdev->clock)) {
-   dev_err(pfdev->dev, "get clock failed %ld\n", 
PTR_ERR(pfdev->clock));
+   dev_err(pfdev->dev, "get clock failed %ld\n",
+   PTR_ERR(pfdev->clock));
return PTR_ERR(pfdev->clock);
}
 
@@ -55,11 +56,33 @@ static int panfrost_clk_init(struct panfrost_device *pfdev)
if (err)
return err;
 
+   pfdev->bus_clock = devm_clk_get_optional(pfdev->dev, "bus");
+   if (IS_ERR(pfdev->bus_clock)) {
+   dev_err(pfdev->dev, "get bus_clock failed %ld\n",
+   PTR_ERR(pfdev->bus_clock));
+   return PTR_ERR(pfdev->bus_clock);
+   }
+
+   if (pfdev->bus_clock) {
+   rate = clk_get_rate(pfdev->bus_clock);
+   dev_info(pfdev->dev, "bus_clock rate = %lu\n", rate);
+
+   err = clk_prepare_enable(pfdev->bus_clock);
+   if (err)
+   goto disable_clock;
+   }
+
return 0;
+
+disable_clock:
+   clk_disable_unprepare(pfdev->clock);
+
+   return err;
 }
 
 static void panfrost_clk_fini(struct panfrost_device *pfdev)
 {
+   clk_disable_unprepare(pfdev->bus_clock);
clk_disable_unprepare(pfdev->clock);
 }
 
diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h 
b/drivers/gpu/drm/panfrost/panfrost_device.h
index 56f452dfb490..8074f221034b 100644
--- a/drivers/gpu/drm/panfrost/panfrost_device.h
+++ b/drivers/gpu/drm/panfrost/panfrost_device.h
@@ -66,6 +66,7 @@ struct panfrost_device {
 
void __iomem *iomem;
struct clk *clock;
+   struct clk *bus_clock;
struct regulator *regulator;
struct reset_control *rstc;
 
-- 
2.17.1

[PATCH v5 4/6] dt-bindings: gpu: mali-midgard: Add H6 mali gpu compatible

[Clément Péron]

This add the H6 mali compatible in the dt-bindings to later support
specific implementation.

Signed-off-by: Clément Péron 
Reviewed-by: Rob Herring 
---
 .../devicetree/bindings/gpu/arm,mali-midgard.txt | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt 
b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
index 2e8bbce35695..4bf17e1cf555 100644
--- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
@@ -15,6 +15,7 @@ Required properties:
 + "arm,mali-t860"
 + "arm,mali-t880"
   * which must be preceded by one of the following vendor specifics:
++ "allwinner,sun50i-h6-mali"
 + "amlogic,meson-gxm-mali"
 + "rockchip,rk3288-mali"
 + "rockchip,rk3399-mali"
@@ -49,9 +50,15 @@ Vendor-specific bindings
 
 
 The Mali GPU is integrated very differently from one SoC to
-another. In order to accomodate those differences, you have the option
+another. In order to accommodate those differences, you have the option
 to specify one more vendor-specific compatible, among:
 
+- "allwinner,sun50i-h6-mali"
+  Required properties:
+  - clocks : phandles to core and bus clocks
+  - clock-names : must contain "core" and "bus"
+  - resets: phandle to GPU reset line
+
 - "amlogic,meson-gxm-mali"
   Required properties:
   - resets : Should contain phandles of :
-- 
2.17.1

[PATCH v5 6/6] arm64: dts: allwinner: Add mali GPU supply for H6 boards

[Clément Péron]

Enable and add supply to the Mali GPU node on all the
H6 boards.

Regarding the datasheet the maximum time for supply to reach
its voltage is 32ms.

Signed-off-by: Clément Péron 
---
 arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts | 6 ++
 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts  | 6 ++
 arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi   | 6 ++
 arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts| 6 ++
 4 files changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
index 0dc33c90dd60..fe36c6588d8e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-beelink-gs1.dts
@@ -70,6 +70,11 @@
status = "okay";
 };
 
+ {
+   mali-supply = <_dcdcc>;
+   status = "okay";
+};
+
  {
status = "okay";
 };
@@ -206,6 +211,7 @@
};
 
reg_dcdcc: dcdcc {
+   regulator-enable-ramp-delay = <32000>;
regulator-min-microvolt = <81>;
regulator-max-microvolt = <108>;
regulator-name = "vdd-gpu";
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
index 17d496990108..ea4866b0fa7a 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi-3.dts
@@ -58,6 +58,11 @@
status = "okay";
 };
 
+ {
+   mali-supply = <_dcdcc>;
+   status = "okay";
+};
+
  {
vmmc-supply = <_cldo1>;
cd-gpios = < 5 6 GPIO_ACTIVE_LOW>; /* PF6 */
@@ -165,6 +170,7 @@
};
 
reg_dcdcc: dcdcc {
+   regulator-enable-ramp-delay = <32000>;
regulator-min-microvolt = <81>;
regulator-max-microvolt = <108>;
regulator-name = "vdd-gpu";
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi 
b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
index 62e27948a3fa..ec770f07aa82 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-orangepi.dtsi
@@ -55,6 +55,11 @@
status = "okay";
 };
 
+ {
+   mali-supply = <_dcdcc>;
+   status = "okay";
+};
+
  {
vmmc-supply = <_cldo1>;
cd-gpios = < 5 6 GPIO_ACTIVE_LOW>;
@@ -163,6 +168,7 @@
};
 
reg_dcdcc: dcdcc {
+   regulator-enable-ramp-delay = <32000>;
regulator-min-microvolt = <81>;
regulator-max-microvolt = <108>;
regulator-name = "vdd-gpu";
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
index 4802902e128f..625a29a25c52 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64.dts
@@ -85,6 +85,11 @@
status = "okay";
 };
 
+ {
+   mali-supply = <_dcdcc>;
+   status = "okay";
+};
+
  {
status = "okay";
 };
@@ -215,6 +220,7 @@
};
 
reg_dcdcc: dcdcc {
+   regulator-enable-ramp-delay = <32000>;
regulator-min-microvolt = <81>;
regulator-max-microvolt = <108>;
regulator-name = "vdd-gpu";
-- 
2.17.1

[PATCH v5 5/6] arm64: dts: allwinner: Add ARM Mali GPU node for H6

[Clément Péron]

Add the mali gpu node to the H6 device-tree.

Signed-off-by: Clément Péron 
---
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi 
b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
index 16c5c3d0fd81..6aad06095c40 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi
@@ -157,6 +157,20 @@
allwinner,sram = <_sram 1>;
};
 
+   gpu: gpu@180 {
+   compatible = "allwinner,sun50i-h6-mali",
+"arm,mali-t720";
+   reg = <0x0180 0x4000>;
+   interrupts = ,
+,
+;
+   interrupt-names = "job", "mmu", "gpu";
+   clocks = < CLK_GPU>, < CLK_BUS_GPU>;
+   clock-names = "core", "bus";
+   resets = < RST_BUS_GPU>;
+   status = "disabled";
+   };
+
syscon: syscon@300 {
compatible = "allwinner,sun50i-h6-system-control",
 "allwinner,sun50i-a64-system-control";
-- 
2.17.1

[PATCH v5 0/6] Allwinner H6 Mali GPU support

[Clément Péron]

Hi,

The Allwinner H6 has a Mali-T720 MP2 which should be supported by
the new panfrost driver. This series fix two issues and introduce the
dt-bindings but a simple benchmark show that it's still NOT WORKING.

I'm pushing it in case someone want to continue the work.

This has been tested with Mesa3D 19.1.0-RC2 and a GPU bitness patch[1].

One patch is from Icenowy Zheng where I changed the order has required
by Rob Herring[2].

Thanks,
Clement

[1] https://gitlab.freedesktop.org/kszaq/mesa/tree/panfrost_64_32
[2] https://patchwork.kernel.org/patch/10699829/

[  345.204813] panfrost 180.gpu: mmu irq status=1
[  345.209617] panfrost 180.gpu: Unhandled Page fault in AS0 at VA
0x02400400
[  345.209617] Reason: TODO
[  345.209617] raw fault status: 0x82C1
[  345.209617] decoded fault status: SLAVE FAULT
[  345.209617] exception type 0xC1: TRANSLATION_FAULT_LEVEL1
[  345.209617] access type 0x2: READ
[  345.209617] source id 0x8000
[  345.729957] panfrost 180.gpu: gpu sched timeout, js=0,
status=0x8, head=0x2400400, tail=0x2400400, sched_job=9e204de9
[  346.055876] panfrost 180.gpu: mmu irq status=1
[  346.060680] panfrost 180.gpu: Unhandled Page fault in AS0 at VA
0x02C00A00
[  346.060680] Reason: TODO
[  346.060680] raw fault status: 0x810002C1
[  346.060680] decoded fault status: SLAVE FAULT
[  346.060680] exception type 0xC1: TRANSLATION_FAULT_LEVEL1
[  346.060680] access type 0x2: READ
[  346.060680] source id 0x8100
[  346.561955] panfrost 180.gpu: gpu sched timeout, js=1,
status=0x8, head=0x2c00a00, tail=0x2c00a00, sched_job=b55a9a85
[  346.573913] panfrost 180.gpu: mmu irq status=1
[  346.578707] panfrost 180.gpu: Unhandled Page fault in AS0 at VA
0x02C00B80

Changes in v4:
 - Add bus_clock probe
 - Fix sanity check in io-pgtable
 - Add vramp-delay
 - Merge all boards into one patch
 - Remove upstreamed Neil A. patch

Changes in v3 (Thanks to Maxime Ripard):
 - Reauthor Icenowy for her path

Changes in v2 (Thanks to Maxime Ripard):
 - Drop GPU OPP Table
 - Add clocks and clock-names in required

Clément Péron (5):
  drm: panfrost: add optional bus_clock
  iommu: io-pgtable: fix sanity check for non 48-bit mali iommu
  dt-bindings: gpu: mali-midgard: Add H6 mali gpu compatible
  arm64: dts: allwinner: Add ARM Mali GPU node for H6
  arm64: dts: allwinner: Add mali GPU supply for H6 boards

Icenowy Zheng (1):
  dt-bindings: gpu: add bus clock for Mali Midgard GPUs

 .../bindings/gpu/arm,mali-midgard.txt | 15 ++-
 .../dts/allwinner/sun50i-h6-beelink-gs1.dts   |  6 +
 .../dts/allwinner/sun50i-h6-orangepi-3.dts|  6 +
 .../dts/allwinner/sun50i-h6-orangepi.dtsi |  6 +
 .../boot/dts/allwinner/sun50i-h6-pine-h64.dts |  6 +
 arch/arm64/boot/dts/allwinner/sun50i-h6.dtsi  | 14 +++
 drivers/gpu/drm/panfrost/panfrost_device.c| 25 ++-
 drivers/gpu/drm/panfrost/panfrost_device.h|  1 +
 drivers/iommu/io-pgtable-arm.c|  2 +-
 9 files changed, 78 insertions(+), 3 deletions(-)

-- 
2.17.1

[PATCH v5 2/6] iommu: io-pgtable: fix sanity check for non 48-bit mali iommu

[Clément Péron]

Allwinner H6 SoC has a Mali T720MP2 with a 33-bit iommu which
trig the sanity check during the alloc of the pgtable.

Change the sanity check to allow non 48-bit configuration.

Suggested-by: Robin Murphy 
Signed-off-by: Clément Péron 
---
 drivers/iommu/io-pgtable-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 4e21efbc4459..74f2ce802e6f 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -1016,7 +1016,7 @@ arm_mali_lpae_alloc_pgtable(struct io_pgtable_cfg *cfg, 
void *cookie)
 {
struct io_pgtable *iop;
 
-   if (cfg->ias != 48 || cfg->oas > 40)
+   if (cfg->ias > 48 || cfg->oas > 40)
return NULL;
 
cfg->pgsize_bitmap &= (SZ_4K | SZ_2M | SZ_1G);
-- 
2.17.1

Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec()

[Stephen Boyd]

Quoting Brendan Higgins (2019-05-14 15:17:10)
> diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c
> new file mode 100644
> index 0..fe0f2bae66085
> --- /dev/null
> +++ b/kernel/sysctl-test.c
> @@ -0,0 +1,293 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * KUnit test of proc sysctl.
> + */
> +
> +#include 
> +#include 

Is this include used?

> +#include 
> +#include 

Is this include used?

> +
> +
> +static void sysctl_test_dointvec_happy_single_negative(struct kunit *test)
> +{
> +   struct ctl_table table = {
> +   .procname = "foo",
> +   .data   = _data.int_0001,
> +   .maxlen = sizeof(int),
> +   .mode   = 0644,
> +   .proc_handler   = proc_dointvec,
> +   .extra1 = _zero,
> +   .extra2 = _one_hundred,
> +   };
> +   char input[] = "-9";
> +   size_t len = sizeof(input) - 1;
> +   loff_t pos = 0;
> +
> +   table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> +   KUNIT_EXPECT_EQ(test, 0, proc_dointvec(, 1, input, , ));
> +   KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> +   KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos);
> +   KUNIT_EXPECT_EQ(test, -9, *(int *)table.data);

Is the casting necessary? Or can the macro do a type coercion of the
second parameter based on the first type?

> +}
> +
> +static void sysctl_test_dointvec_single_less_int_min(struct kunit *test)
> +{
> +   struct ctl_table table = {
> +   .procname = "foo",
> +   .data   = _data.int_0001,
> +   .maxlen = sizeof(int),
> +   .mode   = 0644,
> +   .proc_handler   = proc_dointvec,
> +   .extra1 = _zero,
> +   .extra2 = _one_hundred,
> +   };
> +   char input[32];
> +   size_t len = sizeof(input) - 1;
> +   loff_t pos = 0;
> +   unsigned long abs_of_less_than_min = (unsigned long)INT_MAX
> +- (INT_MAX + INT_MIN) + 1;
> +
> +   KUNIT_EXPECT_LT(test,
> +   snprintf(input, sizeof(input), "-%lu",
> +abs_of_less_than_min),
> +   sizeof(input));
> +
> +   table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> +   KUNIT_EXPECT_EQ(test, -EINVAL,
> +   proc_dointvec(, 1, input, , ));
> +   KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> +   KUNIT_EXPECT_EQ(test, 0, *(int *)table.data);
> +}
> +
> +static void sysctl_test_dointvec_single_greater_int_max(struct kunit *test)
> +{
> +   struct ctl_table table = {
> +   .procname = "foo",
> +   .data   = _data.int_0001,
> +   .maxlen = sizeof(int),
> +   .mode   = 0644,
> +   .proc_handler   = proc_dointvec,
> +   .extra1 = _zero,
> +   .extra2 = _one_hundred,
> +   };
> +   char input[32];
> +   size_t len = sizeof(input) - 1;
> +   loff_t pos = 0;
> +   unsigned long greater_than_max = (unsigned long)INT_MAX + 1;
> +
> +   KUNIT_EXPECT_GT(test, greater_than_max, INT_MAX);
> +   KUNIT_EXPECT_LT(test, snprintf(input, sizeof(input), "%lu",
> +  greater_than_max),
> +   sizeof(input));
> +   table.data = kunit_kzalloc(test, sizeof(int), GFP_USER);
> +   KUNIT_EXPECT_EQ(test, -EINVAL,
> +   proc_dointvec(, 1, input, , ));
> +   KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len);
> +   KUNIT_EXPECT_EQ(test, 0, *(int *)table.data);
> +}
> +
> +static int sysctl_test_init(struct kunit *test)
> +{
> +   return 0;
> +}
> +
> +/*
> + * This is run once after each test case, see the comment on 
> example_test_module
> + * for more information.
> + */
> +static void sysctl_test_exit(struct kunit *test)
> +{
> +}

Can the above two be omitted? If they can be empty sometimes it would be
nice to avoid the extra symbols and code by letting them be assigned to
NULL in the kunit_module.

> +
> +/*
> + * Here we make a list of all the test cases we want to add to the test 
> module
> + * below.
> + */
> +static struct kunit_case sysctl_test_cases[] = {
> +   /*
> +* This is a helper to create a test case object from a test case
> +* function; its exact function is not important to understand how to
> +* use KUnit, just know that this is how you associate test cases 
> with a
> +* test module.
> +*/
> +   KUNIT_CASE(sysctl_test_dointvec_null_tbl_data),
> +   KUNIT_CASE(sysctl_test_dointvec_table_maxlen_unset),
> +   KUNIT_CASE(sysctl_test_dointvec_table_len_is_zero),
> +   KUNIT_CASE(sysctl_test_dointvec_table_read_but_position_set),
> +   KUNIT_CASE(sysctl_test_dointvec_happy_single_positive),
> +   

Re: [PATCH v4 04/18] kunit: test: add kunit_stream a std::stream like logger

[Stephen Boyd]

Quoting Brendan Higgins (2019-05-14 15:16:57)
> diff --git a/kunit/kunit-stream.c b/kunit/kunit-stream.c
> new file mode 100644
> index 0..1884f1b550888
> --- /dev/null
> +++ b/kunit/kunit-stream.c
> @@ -0,0 +1,152 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * C++ stream style string formatter and printer used in KUnit for outputting
> + * KUnit messages.
> + *
> + * Copyright (C) 2019, Google LLC.
> + * Author: Brendan Higgins 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +static const char *kunit_stream_get_level(struct kunit_stream *this)
> +{
> +   unsigned long flags;
> +   const char *level;
> +
> +   spin_lock_irqsave(>lock, flags);
> +   level = this->level;
> +   spin_unlock_irqrestore(>lock, flags);
> +
> +   return level;

Please remove this whole function and inline it to the one call-site.

> +}
> +
> +void kunit_stream_set_level(struct kunit_stream *this, const char *level)
> +{
> +   unsigned long flags;
> +
> +   spin_lock_irqsave(>lock, flags);
> +   this->level = level;
> +   spin_unlock_irqrestore(>lock, flags);

I don't get the locking here. What are we protecting against? Are tests
running in parallel using the same kunit_stream? If so, why is the level
changeable in one call and then adding strings is done in a different
function call? It would make sense to combine the level setting and
string adding so that it's one atomic operation if it's truly a parallel
operation, or remove the locking entirely.

> +}
> +
> +void kunit_stream_add(struct kunit_stream *this, const char *fmt, ...)
> +{
> +   va_list args;
> +   struct string_stream *stream = this->internal_stream;
> +
> +   va_start(args, fmt);
> +
> +   if (string_stream_vadd(stream, fmt, args) < 0)
> +   kunit_err(this->test, "Failed to allocate fragment: %s\n", 
> fmt);
> +
> +   va_end(args);
> +}
> +
> +void kunit_stream_append(struct kunit_stream *this,
> +   struct kunit_stream *other)
> +{
> +   struct string_stream *other_stream = other->internal_stream;
> +   const char *other_content;
> +
> +   other_content = string_stream_get_string(other_stream);
> +
> +   if (!other_content) {
> +   kunit_err(this->test,
> + "Failed to get string from second argument for 
> appending.\n");
> +   return;
> +   }
> +
> +   kunit_stream_add(this, other_content);
> +}
> +
> +void kunit_stream_clear(struct kunit_stream *this)
> +{
> +   string_stream_clear(this->internal_stream);
> +}
> +
> +void kunit_stream_commit(struct kunit_stream *this)

Should this be rather called kunit_stream_flush()?

> +{
> +   struct string_stream *stream = this->internal_stream;
> +   struct string_stream_fragment *fragment;
> +   const char *level;
> +   char *buf;
> +
> +   level = kunit_stream_get_level(this);
> +   if (!level) {
> +   kunit_err(this->test,
> + "Stream was committed without a specified log 
> level.\n");

Drop the full-stop?

> +   level = KERN_ERR;
> +   kunit_stream_set_level(this, level);
> +   }
> +
> +   buf = string_stream_get_string(stream);
> +   if (!buf) {
> +   kunit_err(this->test,

Can you grow a local variable for 'this->test'? It's used many times.

Also, 'this' is not very kernel idiomatic. We usually name variables by
their type instead of 'this' which is a keyword in other languages.
Perhaps it could be named 'kstream'?

> +"Could not allocate buffer, dumping stream:\n");
> +   list_for_each_entry(fragment, >fragments, node) {
> +   kunit_err(this->test, fragment->fragment);
> +   }
> +   kunit_err(this->test, "\n");
> +   goto cleanup;
> +   }
> +
> +   kunit_printk(level, this->test, buf);
> +   kfree(buf);
> +
> +cleanup:
> +   kunit_stream_clear(this);
> +}
> +
> +static int kunit_stream_init(struct kunit_resource *res, void *context)
> +{
> +   struct kunit *test = context;
> +   struct kunit_stream *stream;
> +
> +   stream = kzalloc(sizeof(*stream), GFP_KERNEL);

Of course, here it's called 'stream', so maybe it should be 'kstream'
here too.

> +   if (!stream)
> +   return -ENOMEM;
> +
> +   res->allocation = stream;
> +   stream->test = test;
> +   spin_lock_init(>lock);
> +   stream->internal_stream = new_string_stream();

Can new_string_stream() be renamed to alloc_string_stream()? Sorry, I
just see so much C++ isms in here it's hard to read from the kernel
developer perspective.

> +
> +   if (!stream->internal_stream) {

Nitpick: Please join this to the "allocation" event above instead of
keeping it separated.

> +   kfree(stream);
> +   return -ENOMEM;
> +   }
> +
> +   return 0;
> +}
> +
> +static void 

Re: [PATCH v4 03/18] kunit: test: add string_stream a std::stream like string builder

[Stephen Boyd]

Quoting Brendan Higgins (2019-05-14 15:16:56)
> A number of test features need to do pretty complicated string printing
> where it may not be possible to rely on a single preallocated string
> with parameters.
> 
> So provide a library for constructing the string as you go similar to
> C++'s std::string.
> 
> Signed-off-by: Brendan Higgins 
> Reviewed-by: Greg Kroah-Hartman 
> Reviewed-by: Logan Gunthorpe 

Is there any reason why we can't use the seqfile API for this? These
both share a similar goal, formatting strings into a buffer to be read
later. Maybe some new APIs would be needed to extract the buffer
differently, but I hope we could share the code.

If it can't be used, can you please add the reasoning to the commit text
here?

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

Re: [PATCH] gpu: drm: use struct_size() in kmalloc()

[Pan, Xinhui]

I am going to put more members which are also array after this struct, not only 
obj[].  Looks like this struct_size did not help on multiple array case. Thanks 
anyway.

From: xiaolinkui 
Sent: Friday, May 17, 2019 4:46:00 PM
To: Deucher, Alexander; Koenig, Christian; Zhou, David(ChunMing); 
airl...@linux.ie; dan...@ffwll.ch; Pan, Xinhui; Quan, Evan
Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; 
linux-ker...@vger.kernel.org; xiaolin...@kylinos.cn
Subject: [PATCH] gpu: drm: use struct_size() in kmalloc()

[CAUTION: External Email]

Use struct_size() helper to keep code simple.

Signed-off-by: xiaolinkui 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
index 22bd21e..4717a64 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
@@ -1375,8 +1375,7 @@ int amdgpu_ras_init(struct amdgpu_device *adev)
if (con)
return 0;

-   con = kmalloc(sizeof(struct amdgpu_ras) +
-   sizeof(struct ras_manager) * AMDGPU_RAS_BLOCK_COUNT,
+   con = kmalloc(struct_size(con, objs, AMDGPU_RAS_BLOCK_COUNT),
GFP_KERNEL|__GFP_ZERO);
if (!con)
return -ENOMEM;
--
2.7.4



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

[PATCH 1/2] drm/arm/hdlcd: Actually validate CRTC modes

[Robin Murphy]

Rather than allowing any old mode through, then subsequently refusing
unmatchable clock rates in atomic_check when it's too late to back out
and pick a different mode, let's do that validation up-front where it
will cause unsupported modes to be correctly pruned in the first place.

This also eliminates an issue whereby a perceived clock rate of 0 would
cause atomic disable to fail and prevent the module from being unloaded.

Signed-off-by: Robin Murphy 
---

This supersedes my previous patch here:
https://patchwork.freedesktop.org/patch/288553/
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 0b2b62f8fa3c..ecac6fe0b213 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -186,20 +186,19 @@ static void hdlcd_crtc_atomic_disable(struct drm_crtc 
*crtc,
clk_disable_unprepare(hdlcd->clk);
 }
 
-static int hdlcd_crtc_atomic_check(struct drm_crtc *crtc,
-  struct drm_crtc_state *state)
+static enum drm_mode_status hdlcd_crtc_mode_valid(struct drm_crtc *crtc,
+   const struct drm_display_mode *mode)
 {
struct hdlcd_drm_private *hdlcd = crtc_to_hdlcd_priv(crtc);
-   struct drm_display_mode *mode = >adjusted_mode;
long rate, clk_rate = mode->clock * 1000;
 
rate = clk_round_rate(hdlcd->clk, clk_rate);
if (rate != clk_rate) {
/* clock required by mode not supported by hardware */
-   return -EINVAL;
+   return MODE_NOCLOCK;
}
 
-   return 0;
+   return MODE_OK;
 }
 
 static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc,
@@ -220,7 +219,7 @@ static void hdlcd_crtc_atomic_begin(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_helper_funcs hdlcd_crtc_helper_funcs = {
-   .atomic_check   = hdlcd_crtc_atomic_check,
+   .mode_valid = hdlcd_crtc_mode_valid,
.atomic_begin   = hdlcd_crtc_atomic_begin,
.atomic_enable  = hdlcd_crtc_atomic_enable,
.atomic_disable = hdlcd_crtc_atomic_disable,
-- 
2.21.0.dirty

[PATCH 2/2] drm/arm/hdlcd: Allow a bit of clock tolerance

[Robin Murphy]

On the Arm Juno platform, the HDLCD pixel clock is constrained to 250KHz
resolution in order to avoid the tiny System Control Processor spending
aeons trying to calculate exact PLL coefficients. This means that modes
like my oddball 1600x1200 with 130.89MHz clock get rejected since the
rate cannot be matched exactly. In practice, though, this mode works
quite happily with the clock at 131MHz, so let's relax the check to
allow a little bit of slop.

Signed-off-by: Robin Murphy 
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index ecac6fe0b213..a3efa28436ea 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -193,7 +193,8 @@ static enum drm_mode_status hdlcd_crtc_mode_valid(struct 
drm_crtc *crtc,
long rate, clk_rate = mode->clock * 1000;
 
rate = clk_round_rate(hdlcd->clk, clk_rate);
-   if (rate != clk_rate) {
+   /* 0.1% seems a close enough tolerance for the TDA19988 on Juno */
+   if (abs(rate - clk_rate) * 1000 > clk_rate) {
/* clock required by mode not supported by hardware */
return MODE_NOCLOCK;
}
-- 
2.21.0.dirty

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

Re: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required pixel clock rate

[liviu.du...@arm.com]

On Fri, May 17, 2019 at 10:38:00AM +, Wen He wrote:
> 
> 
> > -Original Message-
> > From: liviu.du...@arm.com [mailto:liviu.du...@arm.com]
> > Sent: 2019年5月16日 16:24
> > To: Wen He 
> > Cc: dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; Leo Li
> > 
> > Subject: Re: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required
> > pixel clock rate
> > 
> > Caution: EXT Email
> > 
> > On Thu, May 16, 2019 at 08:10:21AM +, Wen He wrote:
> > >
> > >
> > > > -Original Message-
> > > > From: liviu.du...@arm.com [mailto:liviu.du...@arm.com]
> > > > Sent: 2019年5月15日 23:46
> > > > To: Wen He 
> > > > Cc: dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org;
> > > > Leo Li 
> > > > Subject: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for
> > > > required pixel clock rate
> > > >
> > > >
> > > > Hi Wen,
> > >
> > > Hi Liviu,
> > >
> 
> Hi Liviu,
> 
> > > >
> > > > On Wed, May 15, 2019 at 02:42:08AM +, Wen He wrote:
> > > > > Disable checking for required pixel clock rate if ARCH_LAYERSCPAE
> > > > > is enable.
> > > > >
> > > > > Signed-off-by: Alison Wang 
> > > > > Signed-off-by: Wen He 
> > > > > ---
> > > > > change in description:
> > > > >   - This check that only supported one pixel clock required clock
> > rate
> > > > >   compare with dts node value. but we have supports 4 pixel clock
> > > > >   for ls1028a board.
> > > >
> > > > So, your DT says your pixel clock provider is a fixed clock? If you
> > > > support more than one rate, you should instead use a real provider
> > > > for it. How do you support the 4 pixel clocks?
> > > >
> > >
> > > Yes , the DT node only can provided one pixel clock by using a fixed 
> > > clock.
> > > But we Display Port controller support 4 or more resolutions, each of
> > > which requires a set of pixel clocks to drive, and we hope they can
> > > switch any resolution we want by some program every times.
> > 
> > That program can't be some userspace application, because it will have to
> > make changes to the hardware and the kernel will not know that things have
> > changed under its feet. That leaves the option of the bootloader or some 
> > other
> > kernel module doing the changes.
> > 
> > If you have another kernel module that knows how to change clocks, that
> > should be implemented using the common clocks infrastructure, at which time
> > you can put it in the DT as the clock provider for the pixelclock.
> > 
> 
> Hi Liviu,

Hi Wen,

> 
> Yes, I think you are right, and even though we didn't implement clocks prepare
> /enable/disable interface, but we can notification hardware to change pixel 
> clock
> by determining the required pixel clock in each mode once had modeset event
> in DP driver.
> 
> But the point is how do we meet the conditions for the clock rate check in 
> here? 

You don't need to change anything in this driver, your clock provider will only
set the values it supports. So in malidp_crtc_atomic_enable() we set the
desired pxlclk, but your provider will drop/refuse the change, so in
malidp_crtc_mode_valid() only the 4 supported resolutions will pass, all other
modes will fail.

> 
> As you know,

I don't, I still don't have a LS1028A platform ;)

Best regards,
Liviu


>  we LS1028A supports 4 modes, every resolution change will to
> determine whether the hardware supports the pixel clock required for the 
> resolution
> by calling this function malidp_crtc_mode_valid() . assume if we put 
> fixed-clock in DT
> node that will can't meet this checking.
> 
> Best Regards,
> Wen
> 
> > If the bootloader does the changes, then the bootloader should edit the DT
> > and set the correct value for the pixel clock. Regardless, with your change 
> > and
> > on your platform the user can request any resolution and the driver will
> > silently fail to set that resolution.
> > 
> > One other problem is the one Robin raised, where the kernel is compiled for
> > multiple platforms, like what various Linux distributions do. That kernel 
> > will
> > either work on other SoC or not, depending on what
> > CONFIG_ARCH_LAYERSCAPE is set to.
> > 
> > In summary, for this patch, it's a NAK. There are proper ways of achieving 
> > what
> > you need, but this patch is not.
> > 
> > Best regards,
> > Liviu
> > 
> > >
> > > For example, if we set that fixed pixel clock is 2700 (27Mhz), but
> > > user hope can see a group 1080p resolution penguins during startup ,
> > > and hope playing a 4k video once system boot up done.
> > > Btw, In our board, the 1080p resolution is driven by a 148.5Mhz pixel
> > > clock, 4k is driven by a 594Mhz. 27Mhz only can drive 480p resolution.
> > >
> > > To meet the above user requirements, I was to setup following steps,
> > > 1. Add the "video=1920x1080-32@60" to bootargs command line [specify
> > > penguins size] 2. Play a 4K video with 4k resolution when system boot up
> > done.
> > >
> > > > Also, not sure what the paragraph above is meant to be. Should it be
> > > > 

Re: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required pixel clock rate

[liviu.du...@arm.com]

On Fri, May 17, 2019 at 10:37:01AM +, Wen He wrote:
> 
> 
> > -Original Message-
> > From: Robin Murphy [mailto:robin.mur...@arm.com]
> > Sent: 2019年5月16日 18:45
> > To: Wen He ; dri-devel@lists.freedesktop.org;
> > linux-ker...@vger.kernel.org; liviu.du...@arm.com
> > Cc: Leo Li 
> > Subject: Re: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required
> > pixel clock rate
> > 
> > 
> > On 16/05/2019 10:42, Wen He wrote:
> > >
> > >
> > >> -Original Message-
> > >> From: Robin Murphy [mailto:robin.mur...@arm.com]
> > >> Sent: 2019年5月16日 1:14
> > >> To: Wen He ; dri-devel@lists.freedesktop.org;
> > >> linux-ker...@vger.kernel.org; liviu.du...@arm.com
> > >> Cc: Leo Li 
> > >> Subject: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for
> > >> required pixel clock rate
> > >>
> > >> Caution: EXT Email
> > >>
> > >> On 15/05/2019 03:42, Wen He wrote:
> > >>> Disable checking for required pixel clock rate if ARCH_LAYERSCPAE is
> > >>> enable.
> > >>>
> > >>> Signed-off-by: Alison Wang 
> > >>> Signed-off-by: Wen He 
> > >>> ---
> > >>> change in description:
> > >>>- This check that only supported one pixel clock required clock
> > rate
> > >>>compare with dts node value. but we have supports 4 pixel clock
> > >>>for ls1028a board.
> > >>>drivers/gpu/drm/arm/malidp_crtc.c | 2 ++
> > >>>1 file changed, 2 insertions(+)
> > >>>
> > >>> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> > >>> b/drivers/gpu/drm/arm/malidp_crtc.c
> > >>> index 56aad288666e..bb79223d9981 100644
> > >>> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> > >>> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> > >>> @@ -36,11 +36,13 @@ static enum drm_mode_status
> > >>> malidp_crtc_mode_valid(struct drm_crtc *crtc,
> > >>>
> > >>>if (req_rate) {
> > >>>rate = clk_round_rate(hwdev->pxlclk, req_rate);
> > >>> +#ifndef CONFIG_ARCH_LAYERSCAPE
> > >>
> > >> What about multiplatform builds? The kernel config doesn't tell you
> > >> what hardware you're actually running on.
> > >>
> > >
> > > Hi Robin,
> > >
> > > Thanks for your reply.
> > >
> > > In fact, Only one platform integrates this IP when
> > CONFIG_ARCH_LAYERSCAPE is set.
> > > Although this are not good ways, but I think it won't be a problem under
> > multiplatform builds.
> > 
> > My point is that ARCH_LAYERSCAPE is going to be enabled in distribution
> > kernels along with everything else, so you're effectively removing this 
> > check for
> > all other vendors' Mali-DP implementations as well, which is probably not 
> > OK.
> > 
> > Furthermore, if LS1028A really only supports 4 specific modes as the BSP
> > documentation I found claims, then surely you'd want a *more* specific check
> > here, rather than no check at all?
> 
> Hi Robin,
> 
> Thanks for your comments.
> 
> Yes, As you said, now LS1028A only supports 4 modes, and we use three clocks 
> to support
> all four modes. In fact, this was really the point.
> 
> However, we can only enable one mode to meet the check statement in this 
> place.
> 
> For example, If user has a 1080p monitor, we must be set the pixel 
> fixed-clock to 148.5MHz. 
> if user want to choice 4k monitor, we also to be change the pixel fixed-clock 
> to 594MHz in
> DT node. In reality, We have no way of knowing what kind of monitor the user 
> wants. Right?

How does your DT know which monitor the user is going to plug in? Like I've
said, if you expose the mechanism you use to set the fixed-clock to a certain
value via the clk provider then you will be able to switch automatically to
that frequency without your patch.

Best regards,
Liviu

> 
> Moreover, user cannot to change screen resolution in this case, I don't think 
> this place is
> reasonable. we need to supporting Ubuntu , Wayland and other embedded GU, so 
> we need
> to switch the resolutions.
> 
> Maybe it's that most android device used, and android system always only need 
> one
> resolution.
> 
> Best Regards,
> Wen
> 
> > 
> > Robin.

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯

Re: [PATCH] drm/panfrost: Select devfreq

[Neil Armstrong]

On 17/05/2019 17:00, Ezequiel Garcia wrote:
> Currently, there is some logic for the driver to work without devfreq.
> However, the driver actually fails to probe if !CONFIG_PM_DEVFREQ.
> 
> Fix this by selecting devfreq, and drop the additional checks
> for devfreq.
> 

Please add a Fixes tag.

Neil

> Signed-off-by: Ezequiel Garcia 
> ---
>  drivers/gpu/drm/panfrost/Kconfig|  1 +
>  drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 ++---
>  2 files changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panfrost/Kconfig 
> b/drivers/gpu/drm/panfrost/Kconfig
> index 591611dc4e34..81963e964b0f 100644
> --- a/drivers/gpu/drm/panfrost/Kconfig
> +++ b/drivers/gpu/drm/panfrost/Kconfig
> @@ -9,6 +9,7 @@ config DRM_PANFROST
>   select IOMMU_SUPPORT
>   select IOMMU_IO_PGTABLE_LPAE
>   select DRM_GEM_SHMEM_HELPER
> + select PM_DEVFREQ
>   help
> DRM driver for ARM Mali Midgard (T6xx, T7xx, T8xx) and
> Bifrost (G3x, G5x, G7x) GPUs.
> diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c 
> b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> index 238bd1d89d43..29fcffdf2d57 100644
> --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
> @@ -140,8 +140,8 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
>   return 0;
>  
>   ret = dev_pm_opp_of_add_table(>pdev->dev);
> - if (ret == -ENODEV) /* Optional, continue without devfreq */
> - return 0;
> + if (ret)
> + return ret;
>  
>   panfrost_devfreq_reset(pfdev);
>  
> @@ -170,9 +170,6 @@ void panfrost_devfreq_resume(struct panfrost_device 
> *pfdev)
>  {
>   int i;
>  
> - if (!pfdev->devfreq.devfreq)
> - return;
> -
>   panfrost_devfreq_reset(pfdev);
>   for (i = 0; i < NUM_JOB_SLOTS; i++)
>   pfdev->devfreq.slot[i].busy = false;
> @@ -182,9 +179,6 @@ void panfrost_devfreq_resume(struct panfrost_device 
> *pfdev)
>  
>  void panfrost_devfreq_suspend(struct panfrost_device *pfdev)
>  {
> - if (!pfdev->devfreq.devfreq)
> - return;
> -
>   devfreq_suspend_device(pfdev->devfreq.devfreq);
>  }
>  
> @@ -194,9 +188,6 @@ static void panfrost_devfreq_update_utilization(struct 
> panfrost_device *pfdev, i
>   ktime_t now;
>   ktime_t last;
>  
> - if (!pfdev->devfreq.devfreq)
> - return;
> -
>   now = ktime_get();
>   last = pfdev->devfreq.slot[slot].time_last_update;
>  
> 

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

Re: [PATCH 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I

[Vasily Khoruzhick]

On Fri, May 17, 2019 at 2:47 AM Torsten Duwe  wrote:
>
> On Fri, May 17, 2019 at 11:08:45AM +0200, Maxime Ripard wrote:
> > >
> > > So for all current practical purposes, we can assume the Teres-I panel
> > > to be powered properly and providing valid EDID; nothing to worry about
> > > in software.
> >
> > You're creating a generic binding for all the users of that bridge,
> > while considering only the specific case of the Teres-I.
>
> All I'm saying is that _this_ usage is also valid. Nothing keeps other
> users from defining the output panel; on the contrary: the driver at hand
> already considers an _optional_ panel and handles it, conditionally. So
> driver and binding spec are 100% in sync here.

Well, endpoint is not necessarily a panel. It can be another bridge or
connector - that's why panel can be optional in driver. But it don't
think that you can just omit an endpoint.

> This is much more straightforward than requiring an output and making up
> some dummy code and params because it cannot reasonably be handled.
> (Remember, if there is an output, the driver will make calls to the
> "attached device" driver.)

They aren't dummy. Moreover you have to attach backlight somewhere (to
panel) so it can be disabled when output is disabled.

Try 'xrandr --output eDP-1 --off' on teres with your current code and
see that backlight stays on.

>
> Torsten
>

[PATCH] backlight: gpio-backlight: Set power state instead of brightness at probe

[Paul Kocialkowski]

On a trivial gpio-backlight setup with a panel using the backlight but
no boot software to enable it beforehand, we fall in a case where the
backlight is disabled (not just blanked) and thus remains disabled when
the panel gets enabled.

Setting gbl->def_value via the device-tree prop allows enabling the
backlight in this situation, but it will be unblanked straight away,
in compliance with the binding. This does not work well when there was no
boot software to display something before, since we really need to unblank
by the time the panel is enabled, not before.

Resolve the situation by setting the brightness to 1 at probe and
managing the power state accordingly, a bit like it's done in
pwm-backlight.

Fixes: 8b770e3c9824 ("backlight: Add GPIO-based backlight driver")
Signed-off-by: Paul Kocialkowski 
---
 drivers/video/backlight/gpio_backlight.c | 19 ++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/video/backlight/gpio_backlight.c 
b/drivers/video/backlight/gpio_backlight.c
index e470da95d806..c9cb97fa13d0 100644
--- a/drivers/video/backlight/gpio_backlight.c
+++ b/drivers/video/backlight/gpio_backlight.c
@@ -57,6 +57,21 @@ static const struct backlight_ops gpio_backlight_ops = {
.check_fb   = gpio_backlight_check_fb,
 };
 
+static int gpio_backlight_initial_power_state(struct gpio_backlight *gbl)
+{
+   struct device_node *node = gbl->dev->of_node;
+
+   /* If we absolutely want the backlight enabled at boot. */
+   if (gbl->def_value)
+   return FB_BLANK_UNBLANK;
+
+   /* If there's no panel to unblank the backlight later. */
+   if (!node || !node->phandle)
+   return FB_BLANK_UNBLANK;
+
+   return FB_BLANK_POWERDOWN;
+}
+
 static int gpio_backlight_probe_dt(struct platform_device *pdev,
   struct gpio_backlight *gbl)
 {
@@ -142,7 +157,9 @@ static int gpio_backlight_probe(struct platform_device 
*pdev)
return PTR_ERR(bl);
}
 
-   bl->props.brightness = gbl->def_value;
+   bl->props.brightness = 1;
+   bl->props.power = gpio_backlight_initial_power_state(gbl);
+
backlight_update_status(bl);
 
platform_set_drvdata(pdev, bl);
-- 
2.21.0

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

[PATCH] drm/panfrost: Select devfreq

[Ezequiel Garcia]

Currently, there is some logic for the driver to work without devfreq.
However, the driver actually fails to probe if !CONFIG_PM_DEVFREQ.

Fix this by selecting devfreq, and drop the additional checks
for devfreq.

Signed-off-by: Ezequiel Garcia 
---
 drivers/gpu/drm/panfrost/Kconfig|  1 +
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 ++---
 2 files changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/Kconfig b/drivers/gpu/drm/panfrost/Kconfig
index 591611dc4e34..81963e964b0f 100644
--- a/drivers/gpu/drm/panfrost/Kconfig
+++ b/drivers/gpu/drm/panfrost/Kconfig
@@ -9,6 +9,7 @@ config DRM_PANFROST
select IOMMU_SUPPORT
select IOMMU_IO_PGTABLE_LPAE
select DRM_GEM_SHMEM_HELPER
+   select PM_DEVFREQ
help
  DRM driver for ARM Mali Midgard (T6xx, T7xx, T8xx) and
  Bifrost (G3x, G5x, G7x) GPUs.
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c 
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index 238bd1d89d43..29fcffdf2d57 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -140,8 +140,8 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev)
return 0;
 
ret = dev_pm_opp_of_add_table(>pdev->dev);
-   if (ret == -ENODEV) /* Optional, continue without devfreq */
-   return 0;
+   if (ret)
+   return ret;
 
panfrost_devfreq_reset(pfdev);
 
@@ -170,9 +170,6 @@ void panfrost_devfreq_resume(struct panfrost_device *pfdev)
 {
int i;
 
-   if (!pfdev->devfreq.devfreq)
-   return;
-
panfrost_devfreq_reset(pfdev);
for (i = 0; i < NUM_JOB_SLOTS; i++)
pfdev->devfreq.slot[i].busy = false;
@@ -182,9 +179,6 @@ void panfrost_devfreq_resume(struct panfrost_device *pfdev)
 
 void panfrost_devfreq_suspend(struct panfrost_device *pfdev)
 {
-   if (!pfdev->devfreq.devfreq)
-   return;
-
devfreq_suspend_device(pfdev->devfreq.devfreq);
 }
 
@@ -194,9 +188,6 @@ static void panfrost_devfreq_update_utilization(struct 
panfrost_device *pfdev, i
ktime_t now;
ktime_t last;
 
-   if (!pfdev->devfreq.devfreq)
-   return;
-
now = ktime_get();
last = pfdev->devfreq.slot[slot].time_last_update;
 
-- 
2.20.1

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

Re: [PATCH] drm/connector: Add doc for content_protection tri-state

[Sean Paul]

On Fri, May 17, 2019 at 10:12 AM Harry Wentland  wrote:
>
> It was there all along in the patch description when this change was
> introduced but it would be helpful to have the same documented in the
> code.
>
> Signed-off-by: Harry Wentland 
> Cc: Sean Paul 
> Cc: Sean Paul 
> Cc: Bhawanpreet Lakha 

Reviewed-by: Sean Paul 

> ---
>  drivers/gpu/drm/drm_connector.c | 9 +
>  1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b34c3d38bf15..c53e51ad3b59 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1519,6 +1519,15 @@ 
> EXPORT_SYMBOL(drm_connector_attach_scaling_mode_property);
>   * Content Protection is intentionally vague to allow for different 
> underlying
>   * technologies, however it is most implemented by HDCP.
>   *
> + * The property is a tri-state with the following values:
> + * - OFF: Self explanatory, no content protection
> + * - DESIRED: Userspace requests that the driver enable protection
> + * - ENABLED: Once the driver has authenticated the link, it sets this value
> + *
> + * The driver is responsible for downgrading ENABLED to DESIRED if the link 
> becomes
> + * unprotected. The driver should also maintain the desiredness of protection
> + * across hotplug/dpms/suspend.
> + *
>   * The content protection will be set to 
> _connector_state.content_protection
>   *
>   * Returns:
> --
> 2.21.0
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH] drm/i915: Tolerate file owned GEM contexts on hot unbind

[Chris Wilson]

Quoting Janusz Krzysztofik (2019-05-17 15:06:17)
> From: Janusz Krzysztofik 
> 
> During i915_driver_unload(), GEM contexts are verified restrictively
> inside i915_gem_fini() if they don't consume shared resources which
> should be cleaned up before the driver is released.  If those checks
> don't result in kernel panic, one more check is performed at the end of
> i915_gem_fini() which issues a WARN_ON() if GEM contexts still exist.

Just fix the underlying bug of this code being called too early. The
assumptions we made for unload are clearly invalid when applied to
unbind, and we need to split the phases.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm] enable syncobj test depending on capability

[Michel Dänzer]

On 2019-05-17 3:43 p.m., Koenig, Christian wrote:
> No, first of all I'm really busy with those TTM problems.
> 
> And second I'm actually not very familiar with this either.
> 
> Please just split the patch up into two, one updating the headers and one 
> fixing the test case.
> 
> Maybe that's enough for Michel,

This is the most important part of include/drm/README:


When and how to update these files
--
Note: One should not do _any_ changes to the files apart from the steps
below.

In order to update the files do the following:
 - Switch to a Linux kernel tree/branch which is not rebased.
   For example: drm-next (https://cgit.freedesktop.org/drm/drm)
 - Install the headers via `make headers_install' to a separate location.
 - Copy the drm header[s] + git add + git commit.
 - Note: Your commit message must include:
   a) Brief summary on the delta. If there's any change that looks like an
API/ABI break one _must_ explicitly state why it's safe to do so.
   b) "Generated using make headers_install."
   c) "Generated from $tree/branch commit $sha"


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

[PATCH] drm/connector: Add doc for content_protection tri-state

[Harry Wentland]

It was there all along in the patch description when this change was
introduced but it would be helpful to have the same documented in the
code.

Signed-off-by: Harry Wentland 
Cc: Sean Paul 
Cc: Sean Paul 
Cc: Bhawanpreet Lakha 
---
 drivers/gpu/drm/drm_connector.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b34c3d38bf15..c53e51ad3b59 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1519,6 +1519,15 @@ 
EXPORT_SYMBOL(drm_connector_attach_scaling_mode_property);
  * Content Protection is intentionally vague to allow for different underlying
  * technologies, however it is most implemented by HDCP.
  *
+ * The property is a tri-state with the following values:
+ * - OFF: Self explanatory, no content protection
+ * - DESIRED: Userspace requests that the driver enable protection
+ * - ENABLED: Once the driver has authenticated the link, it sets this value
+ *
+ * The driver is responsible for downgrading ENABLED to DESIRED if the link 
becomes
+ * unprotected. The driver should also maintain the desiredness of protection
+ * across hotplug/dpms/suspend.
+ *
  * The content protection will be set to 
_connector_state.content_protection
  *
  * Returns:
-- 
2.21.0

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

[RFC PATCH] drm/i915: Tolerate file owned GEM contexts on hot unbind

[Janusz Krzysztofik]

From: Janusz Krzysztofik 

During i915_driver_unload(), GEM contexts are verified restrictively
inside i915_gem_fini() if they don't consume shared resources which
should be cleaned up before the driver is released.  If those checks
don't result in kernel panic, one more check is performed at the end of
i915_gem_fini() which issues a WARN_ON() if GEM contexts still exist.

Some GEM contexts are allocated unconditionally on device file open,
one per each file descriptor, and are kept open until those file
descriptors are closed.  Since open file descriptors prevent the driver
module from being unloaded, that protects the driver from being
released while contexts are still open.  However, that's not the case
on driver unbind or device unplug sysfs operations which are executed
regardless of open file descriptors.

To protect kernel resources from being accessed by those open file
decriptors while driver unbind or device unplug operation is in
progress, the driver now calls drm_device_unplug() at the beginning of
that process and relies on the DRM layer to provide such protection.

Taking all above information into account, as soon as shared resources
not associated with specific file descriptors are cleaned up, it should
be safe to postpone completion of driver release until users of those
open file decriptors give up on errors and close them.

When device has been marked unplugged, use WARN_ON() conditionally so
the warning is displayed only if a GEM context not associated with a
file descriptor is still allocated.

Signed-off-by: Janusz Krzysztofik 
---
 drivers/gpu/drm/i915/i915_gem.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 54f27cabae2a..c00b6dbaf4f5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4670,7 +4670,17 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
 
i915_gem_drain_freed_objects(dev_priv);
 
-   WARN_ON(!list_empty(_priv->contexts.list));
+   if (drm_dev_is_unplugged(_priv->drm)) {
+   struct i915_gem_context *ctx, *cn;
+
+   list_for_each_entry_safe(ctx, cn, _priv->contexts.list,
+link) {
+   WARN_ON(IS_ERR_OR_NULL(ctx->file_priv));
+   break;
+   }
+   } else {
+   WARN_ON(!list_empty(_priv->contexts.list));
+   }
 }
 
 void i915_gem_init_mmio(struct drm_i915_private *i915)
-- 
2.21.0

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

Re: [v11 05/12] drm/i915: Attach HDR metadata property to connector

[Ville Syrjälä]

On Thu, May 16, 2019 at 07:40:10PM +0530, Uma Shankar wrote:
> Attach HDR metadata property to connector object.
> 
> v2: Rebase
> 
> v3: Updated the property name as per updated name
> while creating hdr metadata property
> 
> Signed-off-by: Uma Shankar 
> Reviewed-by: Shashank Sharma 
> ---
>  drivers/gpu/drm/i915/intel_hdmi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 2a4086c..92597d8 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2724,6 +2724,8 @@ static void intel_hdmi_destroy(struct drm_connector 
> *connector)
>  
>   drm_connector_attach_content_type_property(connector);
>   connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> + drm_object_attach_property(>base,
> +
> connector->dev->mode_config.hdr_output_metadata_property, 0);

I think this needs a platform check. We shouldn't expose this on
platforms that can't actually transmit the infoframe.

>  
>   if (!HAS_GMCH(dev_priv))
>   drm_connector_attach_max_bpc_property(connector, 8, 12);
> -- 
> 1.9.1

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

Re: [v11 06/12] drm/i915: Write HDR infoframe and send to panel

[Ville Syrjälä]

On Thu, May 16, 2019 at 07:40:11PM +0530, Uma Shankar wrote:
> Enable writing of HDR metadata infoframe to panel.
> The data will be provid by usersapace compositors, based
> on blending policies and passsed to driver through a blob
> property.
> 
> v2: Rebase
> 
> v3: Fixed a warning message
> 
> v4: Addressed Shashank's review comments
> 
> v5: Rebase. Added infoframe calculation in compute config.
> 
> v6: Addressed Shashank's review comment. Added HDR metadata
> support from GEN10 onwards as per Shashank's recommendation.
> 
> v7: Addressed Shashank's review comments
> 
> v8: Added Shashank's RB.
> 
> v9: Addressed Ville's review comments.
> 
> Signed-off-by: Uma Shankar 
> Reviewed-by: Shashank Sharma 
> ---
>  drivers/gpu/drm/i915/intel_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_hdmi.c | 44 
> +++
>  2 files changed, 45 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 5258abb..40e2c52 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -910,6 +910,7 @@ struct intel_crtc_state {
>   union hdmi_infoframe avi;
>   union hdmi_infoframe spd;
>   union hdmi_infoframe hdmi;
> + union hdmi_infoframe drm;
>   } infoframes;
>  
>   /* HDMI scrambling status */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 92597d8..d3b8e09 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -573,6 +573,7 @@ static u32 hsw_infoframes_enabled(struct intel_encoder 
> *encoder,
>   HDMI_INFOFRAME_TYPE_AVI,
>   HDMI_INFOFRAME_TYPE_SPD,
>   HDMI_INFOFRAME_TYPE_VENDOR,
> + HDMI_INFOFRAME_TYPE_DRM,
>  };
>  
>  u32 intel_hdmi_infoframe_enable(unsigned int type)
> @@ -795,6 +796,41 @@ void intel_read_infoframe(struct intel_encoder *encoder,
>   return true;
>  }
>  
> +static bool
> +intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
> +  struct intel_crtc_state *crtc_state,
> +  struct drm_connector_state *conn_state)
> +{
> + struct hdmi_drm_infoframe *frame = _state->infoframes.drm.drm;
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + int ret;
> +
> + if (!(INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)))
> + return true;
> +
> + if (!crtc_state->has_infoframe)
> + return true;
> +
> + if (!conn_state->hdr_output_metadata ||
> + conn_state->hdr_output_metadata->length == 0)

The core has already done the length check for us. So can be dropped
from here.

> + return true;
> +
> + crtc_state->infoframes.enable |=
> + intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM);
> +
> + ret = drm_hdmi_infoframe_set_hdr_metadata(frame, conn_state);
> + if (ret < 0) {
> + DRM_ERROR("couldn't set HDR metadata in infoframe\n");

Still a user triggreable ERROR.

> + return false;
> + }
> +
> + ret = hdmi_drm_infoframe_check(frame);
> + if (WARN_ON(ret))
> + return false;
> +
> + return true;
> +}
> +
>  static void g4x_set_infoframes(struct intel_encoder *encoder,
>  bool enable,
>  const struct intel_crtc_state *crtc_state,
> @@ -1180,6 +1216,9 @@ static void hsw_set_infoframes(struct intel_encoder 
> *encoder,
>   intel_write_infoframe(encoder, crtc_state,
> HDMI_INFOFRAME_TYPE_VENDOR,
> _state->infoframes.hdmi);
> + intel_write_infoframe(encoder, crtc_state,
> +   HDMI_INFOFRAME_TYPE_DRM,
> +   _state->infoframes.drm);
>  }
>  
>  void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable)
> @@ -2386,6 +2425,11 @@ int intel_hdmi_compute_config(struct intel_encoder 
> *encoder,
>   return -EINVAL;
>   }
>  
> + if (!intel_hdmi_compute_drm_infoframe(encoder, pipe_config, 
> conn_state)) {
> + DRM_DEBUG_KMS("bad DRM infoframe\n");
> + return -EINVAL;
> + }
> +
>   return 0;
>  }
>  
> -- 
> 1.9.1

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

RE: [PATCH 01/11] drm/ttm: Make LRU removal optional.

[Zhou, David(ChunMing)]

> -Original Message-
> From: Christian König 
> Sent: Tuesday, May 14, 2019 8:31 PM
> To: Olsak, Marek ; Zhou, David(ChunMing)
> ; Liang, Prike ; dri-
> de...@lists.freedesktop.org; amd-...@lists.freedesktop.org
> Subject: [PATCH 01/11] drm/ttm: Make LRU removal optional.
> 
> [CAUTION: External Email]
> 
> We are already doing this for DMA-buf imports and also for amdgpu VM BOs
> for quite a while now.
> 
> If this doesn't run into any problems we are probably going to stop removing
> BOs from the LRU altogether.
> 
> Signed-off-by: Christian König 
> ---
[snip]
> diff --git a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> index 0075eb9a0b52..957ec375a4ba 100644
> --- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> +++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
> @@ -69,7 +69,8 @@ void ttm_eu_backoff_reservation(struct
> ww_acquire_ctx *ticket,
> list_for_each_entry(entry, list, head) {
> struct ttm_buffer_object *bo = entry->bo;
> 
> -   ttm_bo_add_to_lru(bo);
> +   if (list_empty(>lru))
> +   ttm_bo_add_to_lru(bo);
> reservation_object_unlock(bo->resv);
> }
> spin_unlock(>lru_lock);
> @@ -93,7 +94,7 @@ EXPORT_SYMBOL(ttm_eu_backoff_reservation);
> 
>  int ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
>struct list_head *list, bool intr,
> -  struct list_head *dups)
> +  struct list_head *dups, bool del_lru)
>  {
> struct ttm_bo_global *glob;
> struct ttm_validate_buffer *entry; @@ -172,11 +173,11 @@ int
> ttm_eu_reserve_buffers(struct ww_acquire_ctx *ticket,
> list_add(>head, list);
> }
> 
> -   if (ticket)
> -   ww_acquire_done(ticket);
> -   spin_lock(>lru_lock);
> -   ttm_eu_del_from_lru_locked(list);
> -   spin_unlock(>lru_lock);
> +   if (del_lru) {
> +   spin_lock(>lru_lock);
> +   ttm_eu_del_from_lru_locked(list);
> +   spin_unlock(>lru_lock);
> +   }

Can you make bo to lru tail here when del_lru is false?

Busy iteration in evict_first will try other process Bos first, which could 
save loop time.

> return 0;
>  }
>  EXPORT_SYMBOL(ttm_eu_reserve_buffers);
> @@ -203,7 +204,10 @@ void ttm_eu_fence_buffer_objects(struct
> ww_acquire_ctx *ticket,
> reservation_object_add_shared_fence(bo->resv, fence);
> else
> reservation_object_add_excl_fence(bo->resv, fence);
> -   ttm_bo_add_to_lru(bo);
> +   if (list_empty(>lru))
> +   ttm_bo_add_to_lru(bo);
> +   else
> +   ttm_bo_move_to_lru_tail(bo, NULL);

If this line is done in above, then we don't need this here.

-David
> reservation_object_unlock(bo->resv);
> }
> spin_unlock(>lru_lock);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index 161b80fee492..5cffaa24259f 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -63,7 +63,7 @@ static int virtio_gpu_object_list_validate(struct
> ww_acquire_ctx *ticket,
> struct virtio_gpu_object *qobj;
> int ret;
> 
> -   ret = ttm_eu_reserve_buffers(ticket, head, true, NULL);
> +   ret = ttm_eu_reserve_buffers(ticket, head, true, NULL, true);
> if (ret != 0)
> return ret;
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> index a7c30e567f09..d28cbedba0b5 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> @@ -465,7 +465,8 @@ vmw_resource_check_buffer(struct ww_acquire_ctx
> *ticket,
> val_buf->bo = >backup->base;
> val_buf->num_shared = 0;
> list_add_tail(_buf->head, _list);
> -   ret = ttm_eu_reserve_buffers(ticket, _list, interruptible, NULL);
> +   ret = ttm_eu_reserve_buffers(ticket, _list, interruptible, NULL,
> +true);
> if (unlikely(ret != 0))
> goto out_no_reserve;
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
> b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
> index 3b396fea40d7..ac435b51f4eb 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_validation.h
> @@ -165,7 +165,7 @@ vmw_validation_bo_reserve(struct
> vmw_validation_context *ctx,
>   bool intr)
>  {
> return ttm_eu_reserve_buffers(>ticket, >bo_list, intr,
> - NULL);
> + NULL, true);
>  }
> 
>  /**
> diff --git a/include/drm/ttm/ttm_bo_driver.h
> b/include/drm/ttm/ttm_bo_driver.h index c008346c2401..fc0d995ac90d
> 100644
> --- 

Re: [v11 08/12] drm/i915: Enable infoframes on GLK+ for HDR

[Ville Syrjälä]

On Thu, May 16, 2019 at 07:40:13PM +0530, Uma Shankar wrote:
> From: Ville Syrjälä 
> 
> This patch enables infoframes on GLK+ to be
> used to send HDR metadata to HDMI sink.
> 
> v2: Addressed Shashank's review comment.
> 
> v3: Addressed Shashank's review comment.
> 
> v4: Added Shashank's RB.
> 
> v5: Dropped hdr_metadata_change check while modeset, as per
> Ville's suggestion.
> 
> Signed-off-by: Ville Syrjälä 
> Signed-off-by: Uma Shankar 
> Reviewed-by: Shashank Sharma 
> ---
>  drivers/gpu/drm/i915/i915_reg.h   |  4 
>  drivers/gpu/drm/i915/intel_hdmi.c | 19 +++
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index e97c47f..d3f5510 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4694,6 +4694,7 @@ enum {
>  #define   VIDEO_DIP_FREQ_MASK(3 << 16)
>  /* HSW and later: */
>  #define   DRM_DIP_ENABLE (1 << 28)

Just noticed this duplicate bit definition. The new name looks to
be more in line with the names of the other bits, so I would just
kill this old bogus defintion while at it.

> +#define   VIDEO_DIP_ENABLE_DRM_GLK   (1 << 28)
>  #define   PSR_VSC_BIT_7_SET  (1 << 27)
>  #define   VSC_SELECT_MASK(0x3 << 25)
>  #define   VSC_SELECT_SHIFT   25
> @@ -8146,6 +8147,7 @@ enum {
>  #define _HSW_VIDEO_DIP_SPD_DATA_A0x602A0
>  #define _HSW_VIDEO_DIP_GMP_DATA_A0x602E0
>  #define _HSW_VIDEO_DIP_VSC_DATA_A0x60320
> +#define _GLK_VIDEO_DIP_DRM_DATA_A0x60440
>  #define _HSW_VIDEO_DIP_AVI_ECC_A 0x60240
>  #define _HSW_VIDEO_DIP_VS_ECC_A  0x60280
>  #define _HSW_VIDEO_DIP_SPD_ECC_A 0x602C0
> @@ -8159,6 +8161,7 @@ enum {
>  #define _HSW_VIDEO_DIP_SPD_DATA_B0x612A0
>  #define _HSW_VIDEO_DIP_GMP_DATA_B0x612E0
>  #define _HSW_VIDEO_DIP_VSC_DATA_B0x61320
> +#define _GLK_VIDEO_DIP_DRM_DATA_B0x61440
>  #define _HSW_VIDEO_DIP_BVI_ECC_B 0x61240
>  #define _HSW_VIDEO_DIP_VS_ECC_B  0x61280
>  #define _HSW_VIDEO_DIP_SPD_ECC_B 0x612C0
> @@ -8184,6 +8187,7 @@ enum {
>  #define HSW_TVIDEO_DIP_SPD_DATA(trans, i)_MMIO_TRANS2(trans, 
> _HSW_VIDEO_DIP_SPD_DATA_A + (i) * 4)
>  #define HSW_TVIDEO_DIP_GMP_DATA(trans, i)_MMIO_TRANS2(trans, 
> _HSW_VIDEO_DIP_GMP_DATA_A + (i) * 4)
>  #define HSW_TVIDEO_DIP_VSC_DATA(trans, i)_MMIO_TRANS2(trans, 
> _HSW_VIDEO_DIP_VSC_DATA_A + (i) * 4)
> +#define GLK_TVIDEO_DIP_DRM_DATA(trans, i)_MMIO_TRANS2(trans, 
> _GLK_VIDEO_DIP_DRM_DATA_A + (i) * 4)
>  #define ICL_VIDEO_DIP_PPS_DATA(trans, i) _MMIO_TRANS2(trans, 
> _ICL_VIDEO_DIP_PPS_DATA_A + (i) * 4)
>  #define ICL_VIDEO_DIP_PPS_ECC(trans, i)  _MMIO_TRANS2(trans, 
> _ICL_VIDEO_DIP_PPS_ECC_A + (i) * 4)
>  
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index d3b8e09..8bd7c07 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -152,6 +152,8 @@ static u32 hsw_infoframe_enable(unsigned int type)
>   return VIDEO_DIP_ENABLE_SPD_HSW;
>   case HDMI_INFOFRAME_TYPE_VENDOR:
>   return VIDEO_DIP_ENABLE_VS_HSW;
> + case HDMI_INFOFRAME_TYPE_DRM:
> + return VIDEO_DIP_ENABLE_DRM_GLK;
>   default:
>   MISSING_CASE(type);
>   return 0;
> @@ -177,6 +179,8 @@ static u32 hsw_infoframe_enable(unsigned int type)
>   return HSW_TVIDEO_DIP_SPD_DATA(cpu_transcoder, i);
>   case HDMI_INFOFRAME_TYPE_VENDOR:
>   return HSW_TVIDEO_DIP_VS_DATA(cpu_transcoder, i);
> + case HDMI_INFOFRAME_TYPE_DRM:
> + return GLK_TVIDEO_DIP_DRM_DATA(cpu_transcoder, i);
>   default:
>   MISSING_CASE(type);
>   return INVALID_MMIO_REG;
> @@ -560,10 +564,16 @@ static u32 hsw_infoframes_enabled(struct intel_encoder 
> *encoder,
>  {
>   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>   u32 val = I915_READ(HSW_TVIDEO_DIP_CTL(pipe_config->cpu_transcoder));
> + u32 mask;
>  
> - return val & (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
> -   VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
> -   VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
> + mask = (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
> + VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
> + VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
> +
> + if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
> + mask |= VIDEO_DIP_ENABLE_DRM_GLK;
> +
> + return val & mask;
>  }
>  
>  static const u8 infoframe_type_to_idx[] = {
> @@ -1193,7 +1203,8 @@ static void hsw_set_infoframes(struct intel_encoder 
> *encoder,
>  
>   val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
>VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
> -   

Re: [PATCH libdrm] enable syncobj test depending on capability

[Koenig, Christian]

No, first of all I'm really busy with those TTM problems.

And second I'm actually not very familiar with this either.

Please just split the patch up into two, one updating the headers and one 
fixing the test case.

Maybe that's enough for Michel,
Christian.

Am 17.05.19 um 14:28 schrieb Zhou, David(ChunMing):
Can you guy do that? Otherwise if kernel driver doesn't set that cap, test 
could fail.

Thanks,
-David

 Original Message 
Subject: Re: [PATCH libdrm] enable syncobj test depending on capability
From: "Koenig, Christian"
To: Michel Dänzer ,"Zhou, David(ChunMing)" ,"Zhou, David(ChunMing)"
CC: dri-devel@lists.freedesktop.org

Am 17.05.19 um 11:55 schrieb Michel Dänzer:
> [CAUTION: External Email]
>
> On 2019-05-17 11:47 a.m., zhoucm1 wrote:
>> ping, Could you help check in patch to gitlab? My connection to gitlab
>> still has problem.
> Please follow the process documented in include/drm/README for
> include/drm/drm.h .

Yeah, the header should be updated separately to what is currently in
drm-next (or drm-misc-next).

And then we can update the fix on top of that,
Christian.

>
>
> --
> Earthling Michel Dänzer   |  https://www.amd.com
> Libre software enthusiast | Mesa and X developer


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

Re: [PATCH v7 3/6] drm/i915/dp: Program VSC Header and DB for Pixel Encoding/Colorimetry Format

[Maarten Lankhorst]

Op 10-05-2019 om 03:53 schreef Gwan-gyeong Mun:
> Function intel_pixel_encoding_setup_vsc handles vsc header and data block
> setup for pixel encoding / colorimetry format.
>
> Setup VSC header and data block in function intel_pixel_encoding_setup_vsc
> for pixel encoding / colorimetry format as per dp 1.4a spec,
> section 2.2.5.7.1, table 2-119: VSC SDP Header Bytes, section 2.2.5.7.5,
> table 2-120:VSC SDP Payload for DB16 through DB18.
>
> v2:
>   Minor style fix. [Maarten]
>   Refer to commit ids instead of patchwork. [Maarten]
>
> v6: Rebase
>
> v7:
>   Rebase and addressed review comments from Ville.
>   Use a structure initializer instead of memset().
>   Fix non-standard comment format.
>   Remove a referring to specific commit.
>   Add a setting of dynamic range bit to  vsc_sdp.DB17.
>   Add a setting of bpc which is based on pipe_bpp.
>   Remove duplicated checking of connector's ycbcr_420_allowed from
>   intel_pixel_encoding_setup_vsc(). It is already checked from
>   intel_dp_ycbcr420_config().
>   Remove comments for VSC_SDP_EXTENSION_FOR_COLORIMETRY_SUPPORTED. It is
>   already implemented on intel_dp_get_colorimetry_status().
>
> Cc: Maarten Lankhorst 
> Cc: Ville Syrjälä 
> Signed-off-by: Gwan-gyeong Mun 
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  1 +
>  drivers/gpu/drm/i915/intel_dp.c  | 90 
>  drivers/gpu/drm/i915/intel_drv.h |  2 +
>  3 files changed, 93 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index cd5277d98b03..2f1688ea5a2c 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3391,6 +3391,7 @@ static void intel_enable_ddi_dp(struct intel_encoder 
> *encoder,
>  
>   intel_edp_backlight_on(crtc_state, conn_state);
>   intel_psr_enable(intel_dp, crtc_state);
> + intel_dp_ycbcr_420_enable(intel_dp, crtc_state);
>   intel_edp_drrs_enable(intel_dp, crtc_state);
>  
>   if (crtc_state->has_audio)
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 9f219f8f0c71..a3fd279a5c52 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4407,6 +4407,96 @@ u8 intel_dp_dsc_get_slice_count(struct intel_dp 
> *intel_dp,
>   return 0;
>  }
>  
> +static void
> +intel_pixel_encoding_setup_vsc(struct intel_dp *intel_dp,
> +const struct intel_crtc_state *crtc_state)
> +{
> + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> + struct dp_sdp vsc_sdp = {};
> +
> + /* Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119 */
> + vsc_sdp.sdp_header.HB0 = 0;
> + vsc_sdp.sdp_header.HB1 = 0x7;
> +
> + /*
> +  * VSC SDP supporting 3D stereo, PSR2, and Pixel Encoding/
> +  * Colorimetry Format indication.
> +  */
> + vsc_sdp.sdp_header.HB2 = 0x5;
> +
> + /*
> +  * VSC SDP supporting 3D stereo, + PSR2, + Pixel Encoding/
> +  * Colorimetry Format indication (HB2 = 05h).
> +  */
> + vsc_sdp.sdp_header.HB3 = 0x13;
> +
> + /*
> +  * YCbCr 420 = 3h DB16[7:4] ITU-R BT.601 = 0h, ITU-R BT.709 = 1h
> +  * DB16[3:0] DP 1.4a spec, Table 2-120
> +  */
> + vsc_sdp.DB[16] = 0x3 << 4; /* 0x3 << 4 , YCbCr 420*/
> + /* RGB->YCBCR color conversion uses the BT.709 color space. */
> + vsc_sdp.DB[16] |= 0x1; /* 0x1, ITU-R BT.709 */
> +
> + /*
> +  * For pixel encoding formats YCbCr444, YCbCr422, YCbCr420, and Y Only,
> +  * the following Component Bit Depth values are defined:
> +  * 001b = 8bpc.
> +  * 010b = 10bpc.
> +  * 011b = 12bpc.
> +  * 100b = 16bpc.
> +  */
> + switch (crtc_state->pipe_bpp) {
> + case 24: /* 8bpc */
> + vsc_sdp.DB[17] = 0x1;
> + break;
> + case 30: /* 10bpc */
> + vsc_sdp.DB[17] = 0x2;
> + break;
> + case 36: /* 12bpc */
> + vsc_sdp.DB[17] = 0x3;
> + break;
> + case 48: /* 16bpc */
> + vsc_sdp.DB[17] = 0x4;
> + break;
> + default:
> + DRM_DEBUG_KMS("Invalid bpp value '%d'\n", crtc_state->pipe_bpp);

I would use MISSING_CASE(crtc_state->pipe_bpp); here, as it's pretty fatal to 
VSC setup. :)

Otherwise series looks good from an atomic point of view. I would try to get an 
ACK from the analogix bridge maintainer for
inclusion in the drm-intel-next-queued tree, since it seems harmless enough to 
include from there.

So for whole series:
Reviewed-by: Maarten Lankhorst 

> + break;
> + }
> +
> + /*
> +  * Dynamic Range (Bit 7)
> +  * 0 = VESA range, 1 = CTA range.
> +  * all YCbCr are always limited range
> +  */
> + vsc_sdp.DB[17] |= 0x80;
> +
> + /*
> +  * Content Type (Bits 2:0)
> +  * 000b = Not defined.
> +  * 001b = Graphics.
> +  * 010b = Photo.
> +  * 011b = Video.
> +  * 100b = Game
> +  * All other 

Re: [PATCH] drm/amd/display: Allow faking displays as VRR capable.

[Harry Wentland]

On 2019-04-30 3:56 p.m., Mario Kleiner wrote:
> [CAUTION: External Email]
> 
> On Tue, Apr 30, 2019 at 2:22 PM Kazlauskas, Nicholas
>  wrote:
>>
>> On 4/30/19 3:44 AM, Michel Dänzer wrote:
>>> [CAUTION: External Email]
>>>
>>> On 2019-04-30 9:37 a.m., Mario Kleiner wrote:
 Allow to detect any connected display to be marked as
 VRR capable. This is useful for testing the basics of
 VRR mode, e.g., scheduling and timestamping, BTR, and
 transition logic, on non-VRR capable displays, e.g.,
 to perform IGT test-suit kms_vrr test runs.

 This fake VRR display mode is enabled by setting the
 optional module parameter amdgpu.fakevrrdisplay=1.

 It will try to use VRR range info parsed from EDID on
 DisplayPort displays which have a compatible EDID,
 but not compatible DPCD caps for Adaptive Sync. E.g.,
 NVidia G-Sync compatible displays expose a proper EDID,
 but not proper DPCD caps.

 It will use a hard-coded VRR range of 30 Hz - 144 Hz on
 other displays without suitable EDID, e.g., standard
 DisplayPort, HDMI, DVI monitors.

 Signed-off-by: Mario Kleiner 

 [...]

   struct amdgpu_mgpu_info mgpu_info = {
.mutex = __MUTEX_INITIALIZER(mgpu_info.mutex),
 @@ -665,6 +666,16 @@ MODULE_PARM_DESC(halt_if_hws_hang, "Halt if HWS hang 
 is detected (0 = off (defau
   MODULE_PARM_DESC(dcfeaturemask, "all stable DC features enabled 
 (default))");
   module_param_named(dcfeaturemask, amdgpu_dc_feature_mask, uint, 0444);

 +/**
 + * DOC: fakevrrdisplay (int)
 + * Override detection of VRR displays to mark any display as VRR capable, 
 even
 + * if it is not. Useful for basic testing of VRR without need to attach 
 such a
 + * display, e.g., for igt tests.
 + * Setting 1 enables faking VRR. Default value, 0, does normal detection.
 + */
 +module_param_named(fakevrrdisplay, amdgpu_fake_vrr_display, int, 0644);
 +MODULE_PARM_DESC(fakevrrdisplay, "Detect any display as VRR capable (0 = 
 off (default), 1 = on)");
>>>
>>> amdgpu has too many module parameters already; IMHO this kind of niche
>>> use-case doesn't justify adding yet another one. For the vast majority
>>> of users, this would just be another knob to break things, resulting in
>>> support burden for us.
>>>
>>> How about e.g. making the vrr_capable property mutable, or adding
>>> another property for this?
>>>
>>>
>>> --
>>> Earthling Michel Dänzer   |  https://www.amd.com
>>> Libre software enthusiast | Mesa and X developer
>>>
>>
>> Since vrr_capable is already an optional property I think making it
>> mutable could potentially be an option. It would allow for userspace to
>> be able to disable capability as well that way.
> 
> Yes, that would have been useful for at least my case. In my own
> toolkit i will need to control vrr on/off on a run-by-run basis,
> depending on what the users experiment scripts want. So i'll add code
> to manipulate my fullscreen windows attached XAtom directly and
> override Mesa's choices. A bit of a hack, but should hopefully work.
> 
> At least for my special niche, more easily accessible (== RandR output
> props) info is always helpful. Other things that would probably make
> my case easier would be optional properties to report the "vrr_active"
> state back, so that the toolkit can know cheaply via a simple query
> without doubt at any point in time if vrr is active or not, because i
> need to use very different ways of scheduling swapbuffers and
> correctness checking the results for vrr vs. non-vrr.
> 
> Or some vrr_range property, so the toolkit can know if it is operating
> the hw in normal vrr range, or in BTR mode, and adapt its swapbuffers
> scheduling, e.g., to try to help the current vrr/btr code a bit to
> make the "right" decisions for stable timing.
> 
> Of course that makes userspace clients more dependent on current hw
> implementation details, so i can see why it's probably not a popular
> choice for a generic api. The best long-term solution is to have
> proper api for the client to just provide a target presentation
> timestamp and leave the rest to the magic little elves inside the
> machine.
> 
>>
>> It's a pretty niche usecase though. However, as Michel said, it would
>> probably just end up being another setting that allows users to break
>> their own setup.
>>
>> Nicholas Kazlauskas
> 
> Ok, fair enough, thank you two for the feedback. I assumed users
> wouldn't mess with this module parameter, given that it has zero
> practical end-user value and can essentially only be used for cheap
> IGT regression testing, but then one never knows if users think it is
> some magic "get Freesync for free" switch and poke the button anyway.
> I'll keep the patch locally for my own testing then.
> 

Users will absolutely set this, report varying levels of success and
then report bugs when they 

Re:[PATCH libdrm] enable syncobj test depending on capability

[Zhou, David(ChunMing)]

Can you guy do that? Otherwise if kernel driver doesn't set that cap, test 
could fail.

Thanks,
-David

 Original Message 
Subject: Re: [PATCH libdrm] enable syncobj test depending on capability
From: "Koenig, Christian"
To: Michel Dänzer ,"Zhou, David(ChunMing)" ,"Zhou, David(ChunMing)"
CC: dri-devel@lists.freedesktop.org

Am 17.05.19 um 11:55 schrieb Michel Dänzer:
> [CAUTION: External Email]
>
> On 2019-05-17 11:47 a.m., zhoucm1 wrote:
>> ping, Could you help check in patch to gitlab? My connection to gitlab
>> still has problem.
> Please follow the process documented in include/drm/README for
> include/drm/drm.h .

Yeah, the header should be updated separately to what is currently in
drm-next (or drm-misc-next).

And then we can update the fix on top of that,
Christian.

>
>
> --
> Earthling Michel Dänzer   |  https://www.amd.com
> Libre software enthusiast | Mesa and X developer

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

Re: [PATCH libdrm] enable syncobj test depending on capability

[Koenig, Christian]

Am 17.05.19 um 11:55 schrieb Michel Dänzer:
> [CAUTION: External Email]
>
> On 2019-05-17 11:47 a.m., zhoucm1 wrote:
>> ping, Could you help check in patch to gitlab? My connection to gitlab
>> still has problem.
> Please follow the process documented in include/drm/README for
> include/drm/drm.h .

Yeah, the header should be updated separately to what is currently in 
drm-next (or drm-misc-next).

And then we can update the fix on top of that,
Christian.

>
>
> --
> Earthling Michel Dänzer   |  https://www.amd.com
> Libre software enthusiast | Mesa and X developer

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

[Bug 110701] GPU faults in in Unigine Valley 1.0

[bugzilla-daemon]

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

Bug ID: 110701
   Summary: GPU faults in in Unigine Valley 1.0
   Product: Mesa
   Version: git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: eero.t.tammi...@intel.com
QA Contact: dri-devel@lists.freedesktop.org

Setup:
- FullHD monitor (through HDMI KVM)
- HadesCanyon KBL i7-8809G ([AMD/ATI] Vega [Radeon RX Vega M] (rev c0))
- Ubuntu 18.04
- drm-tip git kernel v5.1
- Last VegaM firmware from kernel git
- X server git version
- Unigine Valley 1.0
- Mesa:
OpenGL renderer string: AMD VEGAM (DRM 3.32.0, 5.1.0, LLVM 7.0.0)
OpenGL core profile version string: 4.5 (Core Profile) Mesa 19.2.0-devel
(git-d65b160e6a)

Test-cases:
* Several runs of:
  bin/valley_x64 -project_name Valley -data_path ../ -engine_config
../data/valley_1.0.cfg -system_script valley/unigine.cpp -video_app opengl
-sound_app null -video_mode -1 -video_fullscreen 1 -video_multisample 0
-video_width 1920 -video_height 1080 -extern_define
,BENCHMARK,RELEASE,LANGUAGE_EN,QUALITY_HIGH 

Expected outcome:
* No GPU issues (same as earlier, e.g. with yesterday's Mesa a9cef4f0e5)

Actual outcome:
* On last of 3 runs:
---
[  451.020091] amdgpu :01:00.0: GPU fault detected: 147 0x0b618802 for
process valley_x64 pid 2048 thread valley_x64:cs0 pid 2066
[  451.020093] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x00044F6C
[  451.020093] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x06188002
[  451.020095] amdgpu :01:00.0: VM fault (0x02, vmid 3, pasid 32772) at
page 282476, read from 'TC0' (0x54433000) (392)
[  451.020101] amdgpu :01:00.0: GPU fault detected: 147 0x0b610402 for
process valley_x64 pid 2048 thread valley_x64:cs0 pid 2066
[  451.020102] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x
[  451.020102] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x0608400C
[  451.020103] amdgpu :01:00.0: VM fault (0x0c, vmid 3, pasid 32772) at
page 0, read from 'TC5' (0x54433500) (132)
[  451.020109] amdgpu :01:00.0: GPU fault detected: 147 0x0b698402 for
process valley_x64 pid 2048 thread valley_x64:cs0 pid 2066
[  451.020110] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x0002
[  451.020110] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x0618800C
[  451.020111] amdgpu :01:00.0: VM fault (0x0c, vmid 3, pasid 32772) at
page 2, read from 'TC0' (0x54433000) (392)
[  451.020468] amdgpu :01:00.0: GPU fault detected: 147 0x0ac90802 for
process valley_x64 pid 2048 thread valley_x64:cs0 pid 2066
[  451.020469] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x
[  451.020470] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x06104002
[  451.020471] amdgpu :01:00.0: VM fault (0x02, vmid 3, pasid 32772) at
page 0, read from 'TC3' (0x54433300) (260)
[  451.020476] amdgpu :01:00.0: GPU fault detected: 147 0x00108402 for
process valley_x64 pid 2048 thread valley_x64:cs0 pid 2066
[  451.020477] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x00046DD2
[  451.020477] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x06088002
[  451.020478] amdgpu :01:00.0: VM fault (0x02, vmid 3, pasid 32772) at
page 290258, read from 'TC4' (0x54433400) (136)
[  451.020484] amdgpu :01:00.0: GPU fault detected: 147 0x0ad90402 for
process valley_x64 pid 2048 thread valley_x64:cs0 pid 2066
[  451.020484] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x000460A8
[  451.020485] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x06008002
[  451.020486] amdgpu :01:00.0: VM fault (0x02, vmid 3, pasid 32772) at
page 286888, read from 'TC6' (0x54433600) (8)
[  451.020491] amdgpu :01:00.0: GPU fault detected: 147 0x0c380402 for
process valley_x64 pid 2048 thread valley_x64:cs0 pid 2066
[  451.020492] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x00046A0F
[  451.020493] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x06104002
[  451.020494] amdgpu :01:00.0: VM fault (0x02, vmid 3, pasid 32772) at
page 289295, read from 'TC3' (0x54433300) (260)
[  451.020499] amdgpu :01:00.0: GPU fault detected: 147 0x0ac98402 for
process valley_x64 pid 2048 thread valley_x64:cs0 pid 2066
[  451.020500] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_ADDR  
0x00045EAA
[  451.020500] amdgpu :01:00.0:   VM_CONTEXT1_PROTECTION_FAULT_STATUS
0x06108002
[  451.020501] amdgpu :01:00.0: VM fault (0x02, vmid 3, pasid 32772) at
page 286378, read from 'TC2' (0x54433200) (264)
[  451.020507] amdgpu :01:00.0: GPU fault detected: 147 0x0ed88802 for
process valley_x64 pid 2048 thread valley_x64:cs0 pid 2066
[  451.020507] amdgpu 

[Bug 108898] (Recoverable) GPU hangs with GfxBench Manhattan GL tests

[bugzilla-daemon]

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

--- Comment #5 from Eero Tamminen  ---
Sometimes there's also another error message, about fences:
[ 5813.444709] [drm:amdgpu_dm_atomic_commit_tail [amdgpu]] *ERROR* Waiting for
fences timed out or interrupted!
[ 5818.564819] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, but
soft recovered

-- 
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 0/2] Add BO reservation to GEM VRAM pin/unpin/push_to_system

[Gerd Hoffmann]

  Hi,

> It turns out that the bochs and vbox drivers automatically reserved and
> unreserved the BO from within their pin and unpin functions. The other
> drivers; ast, hibmc and mgag200; performed reservation explicitly. With the
> GEM VRAM conversion, automatic BO reservation within pin and unpin functions
> accidentally got lost. So for bochs and vbox, ttm_bo_validate() worked on
> unlocked BOs.
> 
> This patch set fixes the problem by adding automatic reservation to the
> implementation of drm_gem_vram_{pin,unpin,push_to_system}() to fix bochs
> and vbox. It removes explicit BO reservation around the pin, unpin and
> push-to-system calls in the ast, hibmc and mgag200 drivers.
> 
> The only exception is the cursor handling of mgag200. In this case, the
> mgag200 driver now calls drm_gem_vram_{pin,unpin}_reserved(), which works
> with reserved BOs. The respective code should be refactored in a future
> patch to work with the regular pin and unpin functions.

Looks good, pushed to drm-misc-next.

thanks,
  Gerd

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

RE: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required pixel clock rate

[Wen He]

> -Original Message-
> From: liviu.du...@arm.com [mailto:liviu.du...@arm.com]
> Sent: 2019年5月16日 16:24
> To: Wen He 
> Cc: dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; Leo Li
> 
> Subject: Re: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required
> pixel clock rate
> 
> Caution: EXT Email
> 
> On Thu, May 16, 2019 at 08:10:21AM +, Wen He wrote:
> >
> >
> > > -Original Message-
> > > From: liviu.du...@arm.com [mailto:liviu.du...@arm.com]
> > > Sent: 2019年5月15日 23:46
> > > To: Wen He 
> > > Cc: dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org;
> > > Leo Li 
> > > Subject: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for
> > > required pixel clock rate
> > >
> > >
> > > Hi Wen,
> >
> > Hi Liviu,
> >

Hi Liviu,

> > >
> > > On Wed, May 15, 2019 at 02:42:08AM +, Wen He wrote:
> > > > Disable checking for required pixel clock rate if ARCH_LAYERSCPAE
> > > > is enable.
> > > >
> > > > Signed-off-by: Alison Wang 
> > > > Signed-off-by: Wen He 
> > > > ---
> > > > change in description:
> > > >   - This check that only supported one pixel clock required clock
> rate
> > > >   compare with dts node value. but we have supports 4 pixel clock
> > > >   for ls1028a board.
> > >
> > > So, your DT says your pixel clock provider is a fixed clock? If you
> > > support more than one rate, you should instead use a real provider
> > > for it. How do you support the 4 pixel clocks?
> > >
> >
> > Yes , the DT node only can provided one pixel clock by using a fixed clock.
> > But we Display Port controller support 4 or more resolutions, each of
> > which requires a set of pixel clocks to drive, and we hope they can
> > switch any resolution we want by some program every times.
> 
> That program can't be some userspace application, because it will have to
> make changes to the hardware and the kernel will not know that things have
> changed under its feet. That leaves the option of the bootloader or some other
> kernel module doing the changes.
> 
> If you have another kernel module that knows how to change clocks, that
> should be implemented using the common clocks infrastructure, at which time
> you can put it in the DT as the clock provider for the pixelclock.
> 

Hi Liviu,

Yes, I think you are right, and even though we didn't implement clocks prepare
/enable/disable interface, but we can notification hardware to change pixel 
clock
by determining the required pixel clock in each mode once had modeset event
in DP driver.

But the point is how do we meet the conditions for the clock rate check in 
here? 

As you know, we LS1028A supports 4 modes, every resolution change will to
determine whether the hardware supports the pixel clock required for the 
resolution
by calling this function malidp_crtc_mode_valid() . assume if we put 
fixed-clock in DT
node that will can't meet this checking.

Best Regards,
Wen

> If the bootloader does the changes, then the bootloader should edit the DT
> and set the correct value for the pixel clock. Regardless, with your change 
> and
> on your platform the user can request any resolution and the driver will
> silently fail to set that resolution.
> 
> One other problem is the one Robin raised, where the kernel is compiled for
> multiple platforms, like what various Linux distributions do. That kernel will
> either work on other SoC or not, depending on what
> CONFIG_ARCH_LAYERSCAPE is set to.
> 
> In summary, for this patch, it's a NAK. There are proper ways of achieving 
> what
> you need, but this patch is not.
> 
> Best regards,
> Liviu
> 
> >
> > For example, if we set that fixed pixel clock is 2700 (27Mhz), but
> > user hope can see a group 1080p resolution penguins during startup ,
> > and hope playing a 4k video once system boot up done.
> > Btw, In our board, the 1080p resolution is driven by a 148.5Mhz pixel
> > clock, 4k is driven by a 594Mhz. 27Mhz only can drive 480p resolution.
> >
> > To meet the above user requirements, I was to setup following steps,
> > 1. Add the "video=1920x1080-32@60" to bootargs command line [specify
> > penguins size] 2. Play a 4K video with 4k resolution when system boot up
> done.
> >
> > > Also, not sure what the paragraph above is meant to be. Should it be
> > > part of the commit message?
> > >
> >
> > These comments just want to let you know.
> >
> > > Best regards,
> > > Liviu
> > >
> > >
> > > >  drivers/gpu/drm/arm/malidp_crtc.c | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> > > > b/drivers/gpu/drm/arm/malidp_crtc.c
> > > > index 56aad288666e..bb79223d9981 100644
> > > > --- a/drivers/gpu/drm/arm/malidp_crtc.c
> > > > +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> > > > @@ -36,11 +36,13 @@ static enum drm_mode_status
> > > > malidp_crtc_mode_valid(struct drm_crtc *crtc,
> > > >
> >
> > According to our pixel configuration above, Now the variable req_rate
> > value is 14850 or 5940, another 

Re: [PATCH v2 1/3] dt-bindings: gpu: add #cooling-cells property to the ARM Mali Midgard GPU binding

[Heiko Stuebner]

Am Donnerstag, 16. Mai 2019, 19:40:38 CEST schrieb Doug Anderson:
> Hi,
> 
> On Thu, May 16, 2019 at 10:25 AM Matthias Kaehlcke  wrote:
> 
> > The GPU can be used as a thermal cooling device, add an optional
> > '#cooling-cells' property.
> >
> > Signed-off-by: Matthias Kaehlcke 
> > ---
> > Changes in v2:
> > - patch added to the series
> > ---
> >  Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt 
> > b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> > index 18a2cde2e5f3..61fd41a20f99 100644
> > --- a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
> > @@ -37,6 +37,8 @@ Optional properties:
> >  - operating-points-v2 : Refer to 
> > Documentation/devicetree/bindings/opp/opp.txt
> >for details.
> >
> > +- #cooling-cells: Refer to 
> > Documentation/devicetree/bindings/thermal/thermal.txt
> > +  for details.
> >
> >  Example for a Mali-T760:
> >
> > @@ -51,6 +53,7 @@ gpu@ffa3 {
> > mali-supply = <_gpu>;
> > operating-points-v2 = <_opp_table>;
> > power-domains = < RK3288_PD_GPU>;
> > +   #cooling-cells = <2>;
> >  };
> 
> You will conflict with d5ff1adb3809 ("dt-bindings: gpu: mali-midgard:
> Add resets property"), but it's easy to rebase.  I'll leave it to
> whoever is going to land this to decide if they would like you to
> re-post or if they can handle resolving the conflict themselves.
> +Kevin who appears to be the one who landed the conflicting commit.

No problem, I can update this comment when applying
(likely to drm-misc to not create more conflicts),
but will give Rob a bit more time to possibly object :-)

[somewhere in the recent past, he said to not wait on him on the tiny
property-additions, and cooling-cells is pretty well used one at that]


> With that:
> 
> Reviewed-by: Douglas Anderson 

RE: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required pixel clock rate

[Wen He]

> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: 2019年5月16日 18:45
> To: Wen He ; dri-devel@lists.freedesktop.org;
> linux-ker...@vger.kernel.org; liviu.du...@arm.com
> Cc: Leo Li 
> Subject: Re: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required
> pixel clock rate
> 
> 
> On 16/05/2019 10:42, Wen He wrote:
> >
> >
> >> -Original Message-
> >> From: Robin Murphy [mailto:robin.mur...@arm.com]
> >> Sent: 2019年5月16日 1:14
> >> To: Wen He ; dri-devel@lists.freedesktop.org;
> >> linux-ker...@vger.kernel.org; liviu.du...@arm.com
> >> Cc: Leo Li 
> >> Subject: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for
> >> required pixel clock rate
> >>
> >> Caution: EXT Email
> >>
> >> On 15/05/2019 03:42, Wen He wrote:
> >>> Disable checking for required pixel clock rate if ARCH_LAYERSCPAE is
> >>> enable.
> >>>
> >>> Signed-off-by: Alison Wang 
> >>> Signed-off-by: Wen He 
> >>> ---
> >>> change in description:
> >>>- This check that only supported one pixel clock required clock
> rate
> >>>compare with dts node value. but we have supports 4 pixel clock
> >>>for ls1028a board.
> >>>drivers/gpu/drm/arm/malidp_crtc.c | 2 ++
> >>>1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> >>> b/drivers/gpu/drm/arm/malidp_crtc.c
> >>> index 56aad288666e..bb79223d9981 100644
> >>> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> >>> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> >>> @@ -36,11 +36,13 @@ static enum drm_mode_status
> >>> malidp_crtc_mode_valid(struct drm_crtc *crtc,
> >>>
> >>>if (req_rate) {
> >>>rate = clk_round_rate(hwdev->pxlclk, req_rate);
> >>> +#ifndef CONFIG_ARCH_LAYERSCAPE
> >>
> >> What about multiplatform builds? The kernel config doesn't tell you
> >> what hardware you're actually running on.
> >>
> >
> > Hi Robin,
> >
> > Thanks for your reply.
> >
> > In fact, Only one platform integrates this IP when
> CONFIG_ARCH_LAYERSCAPE is set.
> > Although this are not good ways, but I think it won't be a problem under
> multiplatform builds.
> 
> My point is that ARCH_LAYERSCAPE is going to be enabled in distribution
> kernels along with everything else, so you're effectively removing this check 
> for
> all other vendors' Mali-DP implementations as well, which is probably not OK.
> 
> Furthermore, if LS1028A really only supports 4 specific modes as the BSP
> documentation I found claims, then surely you'd want a *more* specific check
> here, rather than no check at all?

Hi Robin,

Thanks for your comments.

Yes, As you said, now LS1028A only supports 4 modes, and we use three clocks to 
support
all four modes. In fact, this was really the point.

However, we can only enable one mode to meet the check statement in this place.

For example, If user has a 1080p monitor, we must be set the pixel fixed-clock 
to 148.5MHz. 
if user want to choice 4k monitor, we also to be change the pixel fixed-clock 
to 594MHz in
DT node. In reality, We have no way of knowing what kind of monitor the user 
wants. Right?

Moreover, user cannot to change screen resolution in this case, I don't think 
this place is
reasonable. we need to supporting Ubuntu , Wayland and other embedded GU, so we 
need
to switch the resolutions.

Maybe it's that most android device used, and android system always only need 
one
resolution.

Best Regards,
Wen

> 
> Robin.

Re: [PATCH v4 2/2] drm/stm: dsi: add regulator support

[Benjamin Gaignard]

Le mar. 14 mai 2019 à 11:36, Yannick Fertré  a écrit :
>
> Add support of regulator for the phy part of the DSI
> controller.
>
> Signed-off-by: Yannick Fertré 
> Acked-by: Philippe Cornu 
Applied on drm-misc-next,

Thanks,
Benjamin

> ---
>  drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 60 
> ---
>  1 file changed, 49 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c 
> b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> index 1bef73e..d8e4a14 100644
> --- a/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> +++ b/drivers/gpu/drm/stm/dw_mipi_dsi-stm.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -76,6 +77,7 @@ struct dw_mipi_dsi_stm {
> u32 hw_version;
> int lane_min_kbps;
> int lane_max_kbps;
> +   struct regulator *vdd_supply;
>  };
>
>  static inline void dsi_write(struct dw_mipi_dsi_stm *dsi, u32 reg, u32 val)
> @@ -314,21 +316,36 @@ static int dw_mipi_dsi_stm_probe(struct platform_device 
> *pdev)
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> dsi->base = devm_ioremap_resource(dev, res);
> if (IS_ERR(dsi->base)) {
> -   DRM_ERROR("Unable to get dsi registers\n");
> -   return PTR_ERR(dsi->base);
> +   ret = PTR_ERR(dsi->base);
> +   DRM_ERROR("Unable to get dsi registers %d\n", ret);
> +   return ret;
> +   }
> +
> +   dsi->vdd_supply = devm_regulator_get(dev, "phy-dsi");
> +   if (IS_ERR(dsi->vdd_supply)) {
> +   ret = PTR_ERR(dsi->vdd_supply);
> +   if (ret != -EPROBE_DEFER)
> +   DRM_ERROR("Failed to request regulator: %d\n", ret);
> +   return ret;
> +   }
> +
> +   ret = regulator_enable(dsi->vdd_supply);
> +   if (ret) {
> +   DRM_ERROR("Failed to enable regulator: %d\n", ret);
> +   return ret;
> }
>
> dsi->pllref_clk = devm_clk_get(dev, "ref");
> if (IS_ERR(dsi->pllref_clk)) {
> ret = PTR_ERR(dsi->pllref_clk);
> -   dev_err(dev, "Unable to get pll reference clock: %d\n", ret);
> -   return ret;
> +   DRM_ERROR("Unable to get pll reference clock: %d\n", ret);
> +   goto err_clk_get;
> }
>
> ret = clk_prepare_enable(dsi->pllref_clk);
> if (ret) {
> -   dev_err(dev, "%s: Failed to enable pllref_clk\n", __func__);
> -   return ret;
> +   DRM_ERROR("Failed to enable pllref_clk: %d\n", ret);
> +   goto err_clk_get;
> }
>
> dw_mipi_dsi_stm_plat_data.base = dsi->base;
> @@ -338,20 +355,28 @@ static int dw_mipi_dsi_stm_probe(struct platform_device 
> *pdev)
>
> dsi->dsi = dw_mipi_dsi_probe(pdev, _mipi_dsi_stm_plat_data);
> if (IS_ERR(dsi->dsi)) {
> -   DRM_ERROR("Failed to initialize mipi dsi host\n");
> -   clk_disable_unprepare(dsi->pllref_clk);
> -   return PTR_ERR(dsi->dsi);
> +   ret = PTR_ERR(dsi->dsi);
> +   DRM_ERROR("Failed to initialize mipi dsi host: %d\n", ret);
> +   goto err_dsi_probe;
> }
>
> return 0;
> +
> +err_dsi_probe:
> +   clk_disable_unprepare(dsi->pllref_clk);
> +err_clk_get:
> +   regulator_disable(dsi->vdd_supply);
> +
> +   return ret;
>  }
>
>  static int dw_mipi_dsi_stm_remove(struct platform_device *pdev)
>  {
> struct dw_mipi_dsi_stm *dsi = platform_get_drvdata(pdev);
>
> -   clk_disable_unprepare(dsi->pllref_clk);
> dw_mipi_dsi_remove(dsi->dsi);
> +   clk_disable_unprepare(dsi->pllref_clk);
> +   regulator_disable(dsi->vdd_supply);
>
> return 0;
>  }
> @@ -363,6 +388,7 @@ static int __maybe_unused dw_mipi_dsi_stm_suspend(struct 
> device *dev)
> DRM_DEBUG_DRIVER("\n");
>
> clk_disable_unprepare(dsi->pllref_clk);
> +   regulator_disable(dsi->vdd_supply);
>
> return 0;
>  }
> @@ -370,10 +396,22 @@ static int __maybe_unused 
> dw_mipi_dsi_stm_suspend(struct device *dev)
>  static int __maybe_unused dw_mipi_dsi_stm_resume(struct device *dev)
>  {
> struct dw_mipi_dsi_stm *dsi = dw_mipi_dsi_stm_plat_data.priv_data;
> +   int ret;
>
> DRM_DEBUG_DRIVER("\n");
>
> -   clk_prepare_enable(dsi->pllref_clk);
> +   ret = regulator_enable(dsi->vdd_supply);
> +   if (ret) {
> +   DRM_ERROR("Failed to enable regulator: %d\n", ret);
> +   return ret;
> +   }
> +
> +   ret = clk_prepare_enable(dsi->pllref_clk);
> +   if (ret) {
> +   regulator_disable(dsi->vdd_supply);
> +   DRM_ERROR("Failed to enable pllref_clk: %d\n", ret);
> +   return ret;
> +   }
>
> return 0;
>  }
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org

Re: [PATCH v4 1/2] dt-bindings: display: stm32: add supply property to DSI controller

[Benjamin Gaignard]

Le mar. 14 mai 2019 à 11:36, Yannick Fertré  a écrit :
>
> This patch adds documentation of a new property phy-dsi-supply to the
> STM32 DSI controller.
>
> Signed-off-by: Yannick Fertré 
> Reviewed-by: Rob Herring 
> Reviewed-by: Philippe Cornu 

Applied on drm-misc-next,

Thanks,
Benjamin

> ---
>  Documentation/devicetree/bindings/display/st,stm32-ltdc.txt | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt 
> b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> index 3eb1b48..60c54da 100644
> --- a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> +++ b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
> @@ -40,6 +40,8 @@ Mandatory nodes specific to STM32 DSI:
>  - panel or bridge node: A node containing the panel or bridge description as
>documented in [6].
>- port: panel or bridge port node, connected to the DSI output port 
> (port@1).
> +Optional properties:
> +- phy-dsi-supply: phandle of the regulator that provides the supply voltage.
>
>  Note: You can find more documentation in the following references
>  [1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> @@ -101,6 +103,7 @@ Example 2: DSI panel
> clock-names = "pclk", "ref";
> resets = < STM32F4_APB2_RESET(DSI)>;
> reset-names = "apb";
> +   phy-dsi-supply = <>;
>
> ports {
> #address-cells = <1>;
> --
> 2.7.4
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v7 09/11] drm: uevent for connector status change

[Pekka Paalanen]

On Thu, 16 May 2019 14:24:55 +0200
Daniel Vetter  wrote:

> On Thu, May 16, 2019 at 11:22:11AM +0300, Pekka Paalanen wrote:
> > On Wed, 15 May 2019 10:24:49 +0200
> > Daniel Vetter  wrote:
> >   
> > > On Wed, May 15, 2019 at 10:37:31AM +0300, Pekka Paalanen wrote:  
> > > > On Tue, 14 May 2019 16:34:01 +0200
> > > > Daniel Vetter  wrote:
> > > > 
> > > > > On Tue, May 14, 2019 at 3:36 PM Pekka Paalanen  
> > > > > wrote:
> > > > > >
> > > > > > On Tue, 14 May 2019 13:02:09 +0200
> > > > > > Daniel Vetter  wrote:
> > > > > >  
> > > > > > > On Tue, May 14, 2019 at 10:18 AM Ser, Simon  
> > > > > > > wrote:  
> > > > > > > >
> > > > > > > > On Tue, 2019-05-14 at 11:02 +0300, Pekka Paalanen wrote:  
> > > > 
> > > > ...
> > > > 
> > > > > > > > > Hi Daniel,
> > > > > > > > >
> > > > > > > > > just to clarify the first case, specific to one very 
> > > > > > > > > particular
> > > > > > > > > property:
> > > > > > > > >
> > > > > > > > > With HDCP, there is a property that may change dynamically at 
> > > > > > > > > runtime
> > > > > > > > > (the undesired/desired/enabled tristate). Userspace must be 
> > > > > > > > > notified
> > > > > > > > > when it changes, I do not want userspace have to poll that 
> > > > > > > > > property
> > > > > > > > > with a timer.
> > > > > > > > >
> > > > > > > > > When that property alone changes, and userspace is prepared 
> > > > > > > > > to handle
> > > > > > > > > that property changing alone, it must not trigger a reprobe 
> > > > > > > > > of the
> > > > > > > > > connector. There is no reason to reprobe at that point AFAIU.
> > > > > > > > >
> > > > > > > > > How do you ensure that userspace can avoid triggering a 
> > > > > > > > > reprobe with the
> > > > > > > > > epoch approach or with any alternate uevent design?
> > > > > > > > >
> > > > > > > > > We need an event to userspace that indicates that re-reading 
> > > > > > > > > the
> > > > > > > > > properties is enough and reprobe of the connector is not 
> > > > > > > > > necessary.
> > > > > > > > > This is complementary to indicating to userspace that only 
> > > > > > > > > some
> > > > > > > > > connectors need to be reprobed instead of everything.  
> > > > > > > >
> > > > > > > > Can't you use the PROPERTY hint? If PROPERTY is the HDCP one, 
> > > > > > > > skip the
> > > > > > > > reprobing. Would that work?  
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > yes, that would work, if it was acceptable to DRM upstream. The 
> > > > > > replies
> > > > > > to Paul seemed to be going south so fast that I thought we wouldn't 
> > > > > > get
> > > > > > any new uevent fields in favour of "epoch counters".
> > > > > >  
> > > > > > > Yes that's the idea, depending upon which property you get you 
> > > > > > > know
> > > > > > > it's a sink change (needs full reprobe) or something else like 
> > > > > > > hdcp
> > > > > > > state machinery update.  
> > > > > >
> > > > > > Right.
> > > > > >  
> > > > > > > Wrt avoiding the full reprobe for sink changes: I think we should
> > > > > > > indeed decouple that from the per-connector event for sink 
> > > > > > > changes.
> > > > > > > That along is a good win already, since you know for which 
> > > > > > > connector
> > > > > > > you need to call drmGetConnector (which forces the reprobe). It 
> > > > > > > would
> > > > > > > be nice to only call drmGetConnectorCurrent (avoids the reprobe), 
> > > > > > > but
> > > > > > > historically speaking every time we tried to rely on this we 
> > > > > > > ended up
> > > > > > > regretting things.  
> > > > > >
> > > > > > What changed? This sounds very much what Paul suggested. Looking at 
> > > > > > it
> > > > > > from userspace side:  
> > > > > 
> > > > > This sounds solid, some refinements below:
> > > > > 
> > > > > > HOTPLUG=1 CONNECTOR=xx PROPERTY=yy
> > > > > >
> > > > > > - If yy is "Content Protection", no need to drmModeGetConnector(), 
> > > > > > just
> > > > > >   re-get the connector properties.
> > > > > >
> > > > > > - Kernel probably shouldn't bother sending this for properties where
> > > > > >   re-probe could be necessary, and send the below instead.  
> > > > > 
> > > > > 
> > > > > I think we should assert that the kernel can get the new property
> > > > > values using drmModeGetConnectorCurrent for this case, i.e. the kernel
> > > > > does not expect a full reprobe. I.e. upgrade your idea from "should"
> > > > > to "must"
> > > > 
> > > > Hi Daniel,
> > > > 
> > > > ok, that's good.
> > > > 
> > > > > Furthermore different property can indicate different kind of updates,
> > > > > e.g. hdcp vs general sink change vs. whatever else might come in the
> > > > > future.
> > > > 
> > > > What do you mean by different kinds of updates?
> > > 
> > > Atm we're discussing two:
> > > 
> > > - "Content Protection"
> > > - "sink changed, but you don't need to reprobe" this would be quite a bit
> > >   a catch all from the output 

[PATCH 1/2] drm/komeda: Add komeda_fb_check_src_coords

[james qian wang (Arm Technology China)]

Add komeda_fb_check_src_coords and check if the layer configured src
rect can meet the requirement of fb and fb format.

Signed-off-by: James Qian Wang (Arm Technology China) 
---
 .../arm/display/komeda/komeda_framebuffer.c   | 21 
 .../arm/display/komeda/komeda_framebuffer.h   |  2 ++
 .../display/komeda/komeda_pipeline_state.c| 34 +--
 3 files changed, 46 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
index 360ab701a88b..dd4232d13b27 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
@@ -213,6 +213,27 @@ komeda_fb_create(struct drm_device *dev, struct drm_file 
*file,
return ERR_PTR(ret);
 }
 
+int komeda_fb_check_src_coords(const struct komeda_fb *kfb,
+  u32 src_x, u32 src_y, u32 src_w, u32 src_h)
+{
+   const struct drm_framebuffer *fb = >base;
+   const struct drm_format_info *info = fb->format;
+
+   if ((src_x + src_w > fb->width) || (src_y + src_h > fb->height)) {
+   DRM_DEBUG_ATOMIC("Invalid source coordinate.\n");
+   return -EINVAL;
+   }
+
+   if ((src_x % info->hsub) || (src_w % info->hsub) ||
+   (src_y % info->vsub) || (src_h % info->vsub)) {
+   DRM_DEBUG_ATOMIC("Wrong subsampling dimension x:%d, y:%d, w:%d, 
h:%d for format: %x.\n",
+src_x, src_y, src_w, src_h, info->format);
+   return -EINVAL;
+   }
+
+   return 0;
+}
+
 dma_addr_t
 komeda_fb_get_pixel_addr(struct komeda_fb *kfb, int x, int y, int plane)
 {
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h 
b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
index f4046e2e6f74..c61ca98a3a63 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.h
@@ -38,6 +38,8 @@ struct komeda_fb {
 struct drm_framebuffer *
 komeda_fb_create(struct drm_device *dev, struct drm_file *file,
const struct drm_mode_fb_cmd2 *mode_cmd);
+int komeda_fb_check_src_coords(const struct komeda_fb *kfb,
+  u32 src_x, u32 src_y, u32 src_w, u32 src_h);
 dma_addr_t
 komeda_fb_get_pixel_addr(struct komeda_fb *kfb, int x, int y, int plane);
 bool komeda_fb_is_layer_supported(struct komeda_fb *kfb, u32 layer_type,
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index d3f414fc3956..fcd34164b3c2 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -269,21 +269,33 @@ komeda_component_get_avail_scaler(struct komeda_component 
*c,
 
 static int
 komeda_layer_check_cfg(struct komeda_layer *layer,
-  struct komeda_plane_state *kplane_st,
+  struct komeda_fb *kfb,
   struct komeda_data_flow_cfg *dflow)
 {
-   struct komeda_fb *kfb = to_kfb(kplane_st->base.fb);
+   u32 hsize_in, vsize_in;
 
if (!komeda_fb_is_layer_supported(kfb, layer->layer_type, dflow->rot))
return -EINVAL;
 
-   if (!in_range(>hsize_in, dflow->in_w)) {
-   DRM_DEBUG_ATOMIC("src_w: %d is out of range.\n", dflow->in_w);
+   if (komeda_fb_check_src_coords(kfb, dflow->in_x, dflow->in_y,
+  dflow->in_w, dflow->in_h))
+   return -EINVAL;
+
+   if (layer->base.id == KOMEDA_COMPONENT_WB_LAYER) {
+   hsize_in = dflow->out_w;
+   vsize_in = dflow->out_h;
+   } else {
+   hsize_in = dflow->in_w;
+   vsize_in = dflow->in_h;
+   }
+
+   if (!in_range(>hsize_in, hsize_in)) {
+   DRM_DEBUG_ATOMIC("invalidate src_w %d.\n", hsize_in);
return -EINVAL;
}
 
-   if (!in_range(>vsize_in, dflow->in_h)) {
-   DRM_DEBUG_ATOMIC("src_h: %d is out of range.\n", dflow->in_h);
+   if (!in_range(>vsize_in, vsize_in)) {
+   DRM_DEBUG_ATOMIC("invalidate src_h %d.\n", vsize_in);
return -EINVAL;
}
 
@@ -302,7 +314,7 @@ komeda_layer_validate(struct komeda_layer *layer,
struct komeda_layer_state *st;
int i, err;
 
-   err = komeda_layer_check_cfg(layer, kplane_st, dflow);
+   err = komeda_layer_check_cfg(layer, kfb, dflow);
if (err)
return err;
 
@@ -360,11 +372,11 @@ komeda_wb_layer_validate(struct komeda_layer *wb_layer,
struct komeda_fb *kfb = to_kfb(conn_st->writeback_job->fb);
struct komeda_component_state *c_st;
struct komeda_layer_state *st;
-   int i;
+   int i, err;
 
-   if (!komeda_fb_is_layer_supported(kfb, wb_layer->layer_type,
- 

[PATCH 2/2] drm/komeda: Add format support for Y0L2, P010, YUV420_8/10BIT

[james qian wang (Arm Technology China)]

- Y0L2 and P010 are block (tiled) format, Update the kemeda logic to
compatible with such block format.
- Since DRM introduced a general block information to drm_format_info,
  the format_caps->tiled_size no long needed, delete it.
- Build some fb utils functions for code sharing.

Signed-off-by: James Qian Wang (Arm Technology China) 
---
 .../gpu/drm/arm/display/include/malidp_io.h   |  7 ++
 .../arm/display/komeda/d71/d71_component.c| 58 ---
 .../gpu/drm/arm/display/komeda/d71/d71_dev.c  | 72 +--
 .../arm/display/komeda/komeda_format_caps.h   |  2 -
 .../arm/display/komeda/komeda_framebuffer.c   | 57 ---
 5 files changed, 97 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/include/malidp_io.h 
b/drivers/gpu/drm/arm/display/include/malidp_io.h
index 4fb3caf864ce..9440dff94212 100644
--- a/drivers/gpu/drm/arm/display/include/malidp_io.h
+++ b/drivers/gpu/drm/arm/display/include/malidp_io.h
@@ -21,6 +21,13 @@ malidp_write32(u32 __iomem *base, u32 offset, u32 v)
writel(v, (base + (offset >> 2)));
 }
 
+static inline void
+malidp_write64(u32 __iomem *base, u32 offset, u64 v)
+{
+   writel(lower_32_bits(v), (base + (offset >> 2)));
+   writel(upper_32_bits(v), (base + (offset >> 2) + 1));
+}
+
 static inline void
 malidp_write32_mask(u32 __iomem *base, u32 offset, u32 m, u32 v)
 {
diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c 
b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
index b85514b599e1..ca4b2f7a8106 100644
--- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
+++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
@@ -163,6 +163,30 @@ static inline u32 to_d71_input_id(struct 
komeda_component_output *output)
return comp ? (comp->hw_id + output->output_port) : 0;
 }
 
+static void d71_layer_update_fb(struct komeda_component *c,
+   struct komeda_fb *kfb,
+   u64 *addr)
+{
+   struct drm_framebuffer *fb = >base;
+   const struct drm_format_info *info = fb->format;
+   u32 __iomem *reg = c->reg;
+   int block_h;
+
+   if (info->num_planes > 2)
+   malidp_write64(reg, BLK_P2_PTR_LOW, addr[2]);
+
+   if (info->num_planes > 1) {
+   block_h = drm_format_info_block_height(info, 1);
+   malidp_write32(reg, BLK_P1_STRIDE, fb->pitches[1] * block_h);
+   malidp_write64(reg, BLK_P1_PTR_LOW, addr[1]);
+   }
+
+   block_h = drm_format_info_block_height(info, 0);
+   malidp_write32(reg, BLK_P0_STRIDE, fb->pitches[0] * block_h);
+   malidp_write64(reg, BLK_P0_PTR_LOW, addr[0]);
+   malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
+}
+
 static void d71_layer_disable(struct komeda_component *c)
 {
malidp_write32_mask(c->reg, BLK_CONTROL, L_EN, 0);
@@ -178,22 +202,8 @@ static void d71_layer_update(struct komeda_component *c,
u32 __iomem *reg = c->reg;
u32 ctrl_mask = L_EN | L_ROT(L_ROT_R270) | L_HFLIP | L_VFLIP | L_TBU_EN;
u32 ctrl = L_EN | to_rot_ctrl(st->rot);
-   int i;
 
-   for (i = 0; i < fb->format->num_planes; i++) {
-   malidp_write32(reg,
-  BLK_P0_PTR_LOW + i * LAYER_PER_PLANE_REGS * 4,
-  lower_32_bits(st->addr[i]));
-   malidp_write32(reg,
-  BLK_P0_PTR_HIGH + i * LAYER_PER_PLANE_REGS * 4,
-  upper_32_bits(st->addr[i]));
-   if (i >= 2)
-   break;
-
-   malidp_write32(reg,
-  BLK_P0_STRIDE + i * LAYER_PER_PLANE_REGS * 4,
-  fb->pitches[i] & 0x);
-   }
+   d71_layer_update_fb(c, kfb, st->addr);
 
malidp_write32(reg, AD_CONTROL, to_ad_ctrl(fb->modifier));
if (fb->modifier) {
@@ -247,7 +257,6 @@ static void d71_layer_update(struct komeda_component *c,
plane_st->color_range));
}
 
-   malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
 
if (kfb->is_va)
@@ -369,26 +378,15 @@ static void d71_wb_layer_update(struct komeda_component 
*c,
 {
struct komeda_layer_state *st = to_layer_st(state);
struct drm_connector_state *conn_st = state->wb_conn->state;
-   struct drm_framebuffer *fb = conn_st->writeback_job->fb;
-   struct komeda_fb *kfb = to_kfb(fb);
-   u32 __iomem *reg = c->reg;
+   struct komeda_fb *kfb = to_kfb(conn_st->writeback_job->fb);
u32 ctrl = L_EN | LW_OFM, mask = L_EN | LW_OFM | LW_TBU_EN;
-   int i;
+   u32 __iomem *reg = c->reg;
 
-   for (i = 0; i < fb->format->num_planes; i++) {
-   malidp_write32(reg + i * LAYER_PER_PLANE_REGS, BLK_P0_PTR_LOW,
-  lower_32_bits(st->addr[i]));
-

[PATCH 0/2] drm/komeda: Add format: Y0L2, P010, YUV420_8/10BIT

[james qian wang (Arm Technology China)]

This patch series adds new formats:
- block format support for Y0L2, P010
- AFBC format YUV420_8_BIT, YUV420_10_BIT 

This patch series depends on:
- https://patchwork.freedesktop.org/series/58710/
- https://patchwork.freedesktop.org/series/59000/
- https://patchwork.freedesktop.org/series/59002/
- https://patchwork.freedesktop.org/series/60698/

James Qian Wang (Arm Technology China) (2):
  drm/komeda: Add komeda_fb_check_src_coords
  drm/komeda: Add format support for Y0L2, P010, YUV420_8/10BIT

 .../gpu/drm/arm/display/include/malidp_io.h   |  7 ++
 .../arm/display/komeda/d71/d71_component.c| 58 +++---
 .../gpu/drm/arm/display/komeda/d71/d71_dev.c  | 72 -
 .../arm/display/komeda/komeda_format_caps.h   |  2 -
 .../arm/display/komeda/komeda_framebuffer.c   | 78 ---
 .../arm/display/komeda/komeda_framebuffer.h   |  2 +
 .../display/komeda/komeda_pipeline_state.c| 34 +---
 7 files changed, 143 insertions(+), 110 deletions(-)

-- 
2.17.1

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

Re: [PATCH libdrm] enable syncobj test depending on capability

[Michel Dänzer]

On 2019-05-17 11:47 a.m., zhoucm1 wrote:
> ping, Could you help check in patch to gitlab? My connection to gitlab
> still has problem.

Please follow the process documented in include/drm/README for
include/drm/drm.h .


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

Re: [PATCH libdrm] enable syncobj test depending on capability

[zhoucm1]

ping, Could you help check in patch to gitlab? My connection to gitlab 
still has problem.



Thanks,

-David


On 2019年05月16日 19:03, Zhou, David(ChunMing) wrote:

could you help push this patch as well?

Thanks,
-David

 Original Message 
Subject: Re: [PATCH libdrm] enable syncobj test depending on capability
From: "Koenig, Christian"
To: "Zhou, David(ChunMing)" ,dri-devel@lists.freedesktop.org
CC:

Am 16.05.19 um 12:46 schrieb Chunming Zhou:
> Feature is controlled by DRM_CAP_SYNCOBJ_TIMELINE drm capability.
>
> Signed-off-by: Chunming Zhou 

Reviewed-by: Christian König 

> ---
>   include/drm/drm.h    | 1 +
>   tests/amdgpu/syncobj_tests.c | 8 
>   2 files changed, 9 insertions(+)
>
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index c893f3b4..532787bf 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -643,6 +643,7 @@ struct drm_gem_open {
>   #define DRM_CAP_PAGE_FLIP_TARGET    0x11
>   #define DRM_CAP_CRTC_IN_VBLANK_EVENT    0x12
>   #define DRM_CAP_SYNCOBJ 0x13
> +#define DRM_CAP_SYNCOBJ_TIMELINE 0x14
>
>   /** DRM_IOCTL_GET_CAP ioctl argument type */
>   struct drm_get_cap {
> diff --git a/tests/amdgpu/syncobj_tests.c b/tests/amdgpu/syncobj_tests.c
> index a0c627d7..869ed88e 100644
> --- a/tests/amdgpu/syncobj_tests.c
> +++ b/tests/amdgpu/syncobj_tests.c
> @@ -22,6 +22,7 @@
>   */
>
>   #include "CUnit/Basic.h"
> +#include "xf86drm.h"
>
>   #include "amdgpu_test.h"
>   #include "amdgpu_drm.h"
> @@ -36,6 +37,13 @@ static void amdgpu_syncobj_timeline_test(void);
>
>   CU_BOOL suite_syncobj_timeline_tests_enable(void)
>   {
> + int r;
> + uint64_t cap = 0;
> +
> + r = drmGetCap(drm_amdgpu[0], DRM_CAP_SYNCOBJ_TIMELINE, );
> + if (r || cap == 0)
> + return CU_FALSE;
> +
>    return CU_TRUE;
>   }
>



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

Re: [PATCH 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I

[Torsten Duwe]

On Fri, May 17, 2019 at 11:08:45AM +0200, Maxime Ripard wrote:
> >
> > So for all current practical purposes, we can assume the Teres-I panel
> > to be powered properly and providing valid EDID; nothing to worry about
> > in software.
> 
> You're creating a generic binding for all the users of that bridge,
> while considering only the specific case of the Teres-I.

All I'm saying is that _this_ usage is also valid. Nothing keeps other
users from defining the output panel; on the contrary: the driver at hand
already considers an _optional_ panel and handles it, conditionally. So
driver and binding spec are 100% in sync here.

This is much more straightforward than requiring an output and making up
some dummy code and params because it cannot reasonably be handled.
(Remember, if there is an output, the driver will make calls to the
"attached device" driver.)

Torsten

Re: [PATCH 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I

[Maxime Ripard]

On Fri, May 17, 2019 at 10:14:18AM +0200, Torsten Duwe wrote:
> On Fri, 17 May 2019 09:27:38 +0200
> Maxime Ripard  wrote:
>
> > On Thu, May 16, 2019 at 06:48:59PM +0200, Torsten Duwe wrote:
> > > On Thu, May 16, 2019 at 09:06:41AM -0700, Vasily Khoruzhick wrote:
> > > >
> > > > Driver can talk to the panel over AUX channel only after t1+t3,
> > > > t1 is up to 10ms, t3 is up to 200ms.
> > >
> > > This is after power-on. The boot loader needs to deal with this.
> >
> > The bootloader can deal with it, but the kernel will also need to. The
> > bootloader might not be doing this because it's not been updated, the
> > regulator might have been disabled between the time the kernel was
> > started and the time the bridge driver probes, etc.
>
> No, you cannot practically switch off this voltage. It supports _all_
> the devices I mentioned. In fact, the PMIC needs to enable it initially,
> and then it takes some time before the SoC can access the MMC and read
> the SPL from it, just because of exactly these 3.3V. Then the boot
> loader starts, and later the eDP bridge gets initialised.

All these devices can be unused, disabled, or compiled as modules.

> In *theory*, albeit a very daring one, I could imagine a very deep
> sleep mode that can only be ended by pressing the power button, which
> should still work without DCDC1. Only then, a description of the panel
> would be required. But I probably missed something and even this does
> not work.
>
> So for all current practical purposes, we can assume the Teres-I panel
> to be powered properly and providing valid EDID; nothing to worry about
> in software.

You're creating a generic binding for all the users of that bridge,
while considering only the specific case of the Teres-I.

Maxime

--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com


signature.asc
Description: PGP signature

Re: [PATCH v2 1/3] dt-bindings: add vendor prefix for Evervision Electronics

[Marco Felsch]

On 19-05-16 21:38, Rob Herring wrote:
> On Tue, Apr 23, 2019 at 7:26 AM Thierry Reding  
> wrote:
> >
> > On Tue, Apr 16, 2019 at 12:06:43PM +0200, Marco Felsch wrote:
> > > Evervision Electronics is a panel manufacturer from Taipei.
> > > http://www.evervisionlcd.com/index.php?lang=en
> > >
> > > Signed-off-by: Marco Felsch 
> > > Reviewed-by: Rob Herring 
> > > ---
> > >  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
> > >  1 file changed, 1 insertion(+)
> >
> > Applied, thanks.
> 
> I've converted this file to json-schema as of v5.2-rc1. See commit
> 8122de54602e. Applied, but doesn't seem to be in linux-next?
> 
> Rob

I tought this patch was already applied?

Regards,
  Marco

-- 
Pengutronix e.K.   | |
Industrial Linux Solutions | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 11/11] drm/amdgpu: stop removing BOs from the LRU during CS

[Liang, Prike]

Hi Christian，

With the series patch set , amdgpu_vm_validate_pt_bos occasionally evicted 
amdgpu BOs failed and can’t
find the valid first busy bo . Another problem is that  during the first BOs 
get lock period will run into deadlock .

/* check if other user occupy memory too long time */
if (!first_bo || !request_resv || !request_resv->lock.ctx) {
if (first_bo)
ttm_bo_put(first_bo);
return -EBUSY;
}
if (first_bo->resv == request_resv) {
ttm_bo_put(first_bo);
return -EBUSY;
}
if (ctx->interruptible)
ret = ww_mutex_lock_interruptible(_bo->resv->lock,
  
request_resv->lock.ctx);
else
ret = ww_mutex_lock(_bo->resv->lock, 
request_resv->lock.ctx);
if (ret) {
ttm_bo_put(first_bo);
if (ret == -EDEADLK) {
ret = -EAGAIN;
}

return ret;
}

Thanks
Prike

From: Christian König 
Sent: Wednesday, May 15, 2019 3:05 PM
To: Liang, Prike ; Marek Olšák 
Cc: Zhou, David(ChunMing) ; dri-devel 
; amd-gfx mailing list 

Subject: Re: [PATCH 11/11] drm/amdgpu: stop removing BOs from the LRU during CS

[CAUTION: External Email]
Hi Prike,

no, that can lead to massive problems in a real OOM situation and is not 
something we can do here.

Christian.

Am 15.05.19 um 04:00 schrieb Liang, Prike:
Hi Christian ,

I just wonder when encounter ENOMEM error during pin amdgpu BOs can we retry 
validate again as below.
With the following simply patch the Abaqus pinned issue not observed.

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
index 11cbf63..72a32f5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
@@ -902,11 +902,15 @@ int amdgpu_bo_pin_restricted(struct amdgpu_bo *bo, u32 
domain,
bo->placements[i].lpfn = lpfn;
bo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
}
-
+retry:
r = ttm_bo_validate(>tbo, >placement, );
if (unlikely(r)) {
-   dev_err(adev->dev, "%p pin failed\n", bo);
-   goto error;
+if (r == -ENOMEM){
+goto retry;
+} else {
+   dev_err(adev->dev, "%p pin failed\n", bo);
+   goto error;
+}
}

bo->pin_count = 1;


Thanks,
Prike

From: Marek Olšák 
Sent: Wednesday, May 15, 2019 3:33 AM
To: Christian König 

Cc: Zhou, David(ChunMing) ; 
Liang, Prike ; dri-devel 
; 
amd-gfx mailing list 

Subject: Re: [PATCH 11/11] drm/amdgpu: stop removing BOs from the LRU during CS

[CAUTION: External Email]
This series fixes the OOM errors. However, if I torture the kernel driver more, 
I can get it to deadlock and end up with unkillable processes. I can also get 
an OOM error. I just ran the test 5 times:

AMD_DEBUG=testgdsmm glxgears & AMD_DEBUG=testgdsmm glxgears & 
AMD_DEBUG=testgdsmm glxgears & AMD_DEBUG=testgdsmm glxgears & 
AMD_DEBUG=testgdsmm glxgears

Marek

On Tue, May 14, 2019 at 8:31 AM Christian König 
mailto:ckoenig.leichtzumer...@gmail.com>> 
wrote:
This avoids OOM situations when we have lots of threads
submitting at the same time.

Signed-off-by: Christian König 
mailto:christian.koe...@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index fff558cf385b..f9240a94217b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -648,7 +648,7 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
}

r = ttm_eu_reserve_buffers(>ticket, >validated, true,
-  , true);
+  , false);
if (unlikely(r != 0)) {
if (r != -ERESTARTSYS)
DRM_ERROR("ttm_eu_reserve_buffers failed.\n");
--
2.17.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 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I

[Torsten Duwe]

On Fri, 17 May 2019 09:27:38 +0200
Maxime Ripard  wrote:

> On Thu, May 16, 2019 at 06:48:59PM +0200, Torsten Duwe wrote:
> > On Thu, May 16, 2019 at 09:06:41AM -0700, Vasily Khoruzhick wrote:
> > >
> > > Driver can talk to the panel over AUX channel only after t1+t3,
> > > t1 is up to 10ms, t3 is up to 200ms.
> >
> > This is after power-on. The boot loader needs to deal with this.
> 
> The bootloader can deal with it, but the kernel will also need to. The
> bootloader might not be doing this because it's not been updated, the
> regulator might have been disabled between the time the kernel was
> started and the time the bridge driver probes, etc.

No, you cannot practically switch off this voltage. It supports _all_
the devices I mentioned. In fact, the PMIC needs to enable it initially,
and then it takes some time before the SoC can access the MMC and read
the SPL from it, just because of exactly these 3.3V. Then the boot
loader starts, and later the eDP bridge gets initialised.

In *theory*, albeit a very daring one, I could imagine a very deep
sleep mode that can only be ended by pressing the power button, which
should still work without DCDC1. Only then, a description of the panel
would be required. But I probably missed something and even this does
not work.

So for all current practical purposes, we can assume the Teres-I panel
to be powered properly and providing valid EDID; nothing to worry about
in software.

HTH,
Torsten

Re: drm/nouveau/core/memory: kmemleak 684 new suspected memory leaks

[Sergey Senozhatsky]

On (05/17/19 15:31), Sergey Senozhatsky wrote:
> >   backtrace:
> > [<81f2894f>] nvkm_memory_tags_get+0x8e/0x130
> > [<7cd7c0bc>] gf100_vmm_valid+0x196/0x2f0
> > [<70cc6d67>] nvkm_vmm_map+0xa8/0x360
> > [] nvkm_vram_map+0x48/0x50
> > [] nvkm_uvmm_mthd+0x658/0x770
> > [<463fca5a>] nvkm_ioctl+0xdf/0x177
> > [<0afc4996>] nvif_object_mthd+0xd4/0x100
> > [<2f7a7385>] nvif_vmm_map+0xeb/0x100
> > [] nouveau_mem_map+0x79/0xd0
> > [<14ddc0cf>] nouveau_vma_new+0x19d/0x1c0
> > [] nouveau_gem_object_open+0xd4/0x140
> > [<9cd25861>] drm_gem_handle_create_tail+0xe3/0x160
> > [<191784d9>] nouveau_gem_ioctl_new+0x6e/0xd0
> > [<159678df>] drm_ioctl_kernel+0x8c/0xd0
> > [] drm_ioctl+0x1c4/0x360
> > [<6833fe15>] nouveau_drm_ioctl+0x63/0xb0
[..]
>   backtrace:
> [<6933ed2b>] nouveau_conn_reset+0x20/0xb0
> [<572e2e30>] nouveau_connector_create+0x356/0x54c
> [<8a6a13cd>] nv50_display_create+0x2fb/0x917
> [<7fab0a58>] nouveau_display_create+0x3e6/0x600
> [<8b8644c8>] nouveau_drm_device_init+0x149/0x6b0
> [<4fd78a1f>] nouveau_drm_probe+0x263/0x2b0
> [<357716ef>] pci_device_probe+0xa3/0x110
> [<061d40e4>] really_probe+0xd3/0x240
> [<0ade44b6>] driver_probe_device+0x50/0xc0
> [<9cd0024c>] device_driver_attach+0x53/0x60
> [] __driver_attach+0x4c/0xb0
> [<16d8457f>] bus_for_each_dev+0x66/0x90
> [] bus_add_driver+0x171/0x1c0
> [<21c08fc1>] driver_register+0x6c/0xaf
> [<86357843>] do_one_initcall+0x36/0x1d4
> [] kernel_init_freeable+0x1bf/0x24f

And one more:

iccsense->rail = kmalloc_array(cnt, sizeof(struct pwr_rail_t), 
GFP_KERNEL);

unreferenced object 0x94e5ccdc7600 (size 96):
  comm "swapper/0", pid 1, jiffies 4294667774 (age 913.205s)
  hex dump (first 32 bytes):
00 00 00 cc e5 94 ff ff 00 00 00 00 00 00 00 00  
04 00 f1 ff 00 00 00 00 00 00 00 00 00 00 00 00  
  backtrace:
[<242abcb3>] nvbios_iccsense_parse+0xdc/0x250
[] nvkm_iccsense_oneinit+0x55/0x370
[<20e0a743>] nvkm_subdev_init+0x53/0xd0
[<4d8c6ef1>] nvkm_device_init+0x10d/0x190
[] nvkm_udevice_init+0x41/0x60
[<47effcfc>] nvkm_object_init+0x3e/0x100
[<6d6bad21>] nvkm_ioctl_new+0x145/0x1e0
[] nvkm_ioctl+0xdf/0x177
[<4cdc9cf8>] nvif_object_init+0xd6/0x130
[<1637584b>] nvif_device_init+0xe/0x50
[<830683d4>] nouveau_cli_init+0x17d/0x410
[] nouveau_drm_device_init+0x55/0x6b0
[<7bc74e3f>] nouveau_drm_probe+0x263/0x2b0
[<0f94f913>] pci_device_probe+0xa3/0x110
[] really_probe+0xd3/0x240
[] driver_probe_device+0x50/0xc0

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

RE: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required pixel clock rate

[Wen He]

> -Original Message-
> From: Robin Murphy [mailto:robin.mur...@arm.com]
> Sent: 2019年5月16日 1:14
> To: Wen He ; dri-devel@lists.freedesktop.org;
> linux-ker...@vger.kernel.org; liviu.du...@arm.com
> Cc: Leo Li 
> Subject: [EXT] Re: [v1] drm/arm/mali-dp: Disable checking for required pixel
> clock rate
> 
> Caution: EXT Email
> 
> On 15/05/2019 03:42, Wen He wrote:
> > Disable checking for required pixel clock rate if ARCH_LAYERSCPAE is
> > enable.
> >
> > Signed-off-by: Alison Wang 
> > Signed-off-by: Wen He 
> > ---
> > change in description:
> >   - This check that only supported one pixel clock required clock rate
> >   compare with dts node value. but we have supports 4 pixel clock
> >   for ls1028a board.
> >   drivers/gpu/drm/arm/malidp_crtc.c | 2 ++
> >   1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/arm/malidp_crtc.c
> > b/drivers/gpu/drm/arm/malidp_crtc.c
> > index 56aad288666e..bb79223d9981 100644
> > --- a/drivers/gpu/drm/arm/malidp_crtc.c
> > +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> > @@ -36,11 +36,13 @@ static enum drm_mode_status
> > malidp_crtc_mode_valid(struct drm_crtc *crtc,
> >
> >   if (req_rate) {
> >   rate = clk_round_rate(hwdev->pxlclk, req_rate);
> > +#ifndef CONFIG_ARCH_LAYERSCAPE
> 
> What about multiplatform builds? The kernel config doesn't tell you what
> hardware you're actually running on.
> 

Hi Robin,

Thanks for your reply.

In fact, Only one platform integrates this IP when CONFIG_ARCH_LAYERSCAPE is 
set.
Although this are not good ways, but I think it won't be a problem under 
multiplatform builds.

Best Regards,
Wen

> Robin.
> 
> >   if (rate != req_rate) {
> >   DRM_DEBUG_DRIVER("pxlclk doesn't support %ld
> Hz\n",
> >req_rate);
> >   return MODE_NOCLOCK;
> >   }
> > +#endif
> >   }
> >
> >   return MODE_OK;
> >
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I

[Torsten Duwe]

On Thu, May 16, 2019 at 09:06:41AM -0700, Vasily Khoruzhick wrote:
> 
> Driver can talk to the panel over AUX channel only after t1+t3, t1 is
> up to 10ms, t3 is up to 200ms.

This is after power-on. The boot loader needs to deal with this.

> It works with older version of driver
> that keeps panel always on because it takes a while between driver
> probe and pipeline start.

No lid switch, no USB, no WiFi, no MMC. If you disable DCDC1 you'll
run out of wakeup-sources ;-) IOW: I see no practical way any OS
driver can switch this panel voltage off and survive...

> All in all - you don't need panel timings since there's EDID but you
> still need panel delays. Anyway, it's up to you and maintainers.

Let's give it a try.

Torsten

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

Re: [Nouveau] [PATCH] drm/nouveau: fix duplication of nv50_head_atom struct

[Peteris Rudzusiks]

On Wed, May 15, 2019 at 11:29:40PM -0400, Ilia Mirkin wrote:
> On Tue, May 14, 2019 at 3:57 PM Peteris Rudzusiks
>  wrote:
> >
> > On Tue, May 14, 2019 at 04:55:05PM +1000, Ben Skeggs wrote:
> > > On Sun, 12 May 2019 at 04:23, Peteris Rudzusiks
> > >  wrote:
> > > >
> > > > nv50_head_atomic_duplicate_state() makes a copy of nv50_head_atom
> > > > struct. This patch adds copying of struct member named "or", which
> > > > previously was left uninitialized in the duplicated structure.
> > > >
> > > > Due to this bug, incorrect nhsync and nvsync values were sometimes used.
> > > > In my particular case, that lead to a mismatch between the output
> > > > resolution of the graphics device (GeForce GT 630 OEM) and the reported
> > > > input signal resolution on the display. xrandr reported 1680x1050, but
> > > > the display reported 1280x1024. As a result of this mismatch, the output
> > > > on the display looked like it was cropped (only part of the output was
> > > > actually visible on the display).
> > > >
> > > > git bisect pointed to commit 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle
> > > > SetControlOutputResource from head"), which added the member "or" to
> > > > nv50_head_atom structure, but forgot to copy it in
> > > > nv50_head_atomic_duplicate_state().
> > > >
> > > > Fixes: 2ca7fb5c1cc6 ("drm/nouveau/kms/nv50: handle 
> > > > SetControlOutputResource from head")
> > > > Signed-off-by: Peteris Rudzusiks 
> > > Oops, nice catch.  Thank you for this, I've merged it in my tree and
> > > will get it upstream ASAP.
> > >
> > > Thanks,
> > > Ben.
> > >
> > Hi Ben,
> >
> > Thank you for taking the time to review and merge this patch.
> >
> > I'm new to the Linux kernel development process, so I am not sure what
> > happens next. Does inclusion in your tree imply that this fix will end
> > up in some (most likely - next) mainline kernel? Will it also be
> > backported to 4.19 LTS branch?
> >
> > This bug affects all kernel versions starting from v4.18. Probably not
> > that many systems though.
> 
> Ben submits a pull request to Dave Airlie (drm maintainer), and Dave
> submits one to Linus for inclusion in the "official" upstream
> repository. Dave just sent a pull request to Linus, who usually picks
> these up within a few days (exceptions apply).
> 
> Once in the mainline tree, the "Fixes" tag is likely to cause it to
> get picked up for stable. You can also nominate it for stable kernel
> branch inclusion explicitly (there are instructions somewhere, but
> basically you send an email to some list saying "please include commit
> ABC in kernels XYZ").
> 
> What Ubuntu ships is, ultimately, up to Ubuntu. They will, however,
> frequently follow the stable kernel branches, and listen to the list
> above as well.
> 
> Hope this helps,
> 
>   -ilia

Thanks for explaing this. I'll wait and see if this patch gets included
in stable releases without explicitly asking for it.

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

Re: drm/nouveau/core/memory: kmemleak 684 new suspected memory leaks

[Sergey Senozhatsky]

On (05/17/19 16:27), Sergey Senozhatsky wrote:
> > ... but most likely it's utterly wrong.
> > 
> 
> JFI, I removed kmemleak annotation

meeehhh

kmemleak: 2046 new suspected memory leaks (see /sys/kernel/debug/kmemleak)

unreferenced object 0x95cbea4e6060 (size 16):
  comm "Web Content", pid 1191, jiffies 4294795669 (age 735.950s)
  hex dump (first 16 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
  backtrace:
[] nvkm_memory_tags_get+0x8e/0x130
[<061f3c89>] gf100_vmm_valid+0x196/0x2f0
[] nvkm_vmm_map+0xa8/0x360
[] nvkm_vram_map+0x48/0x50
[<006adddb>] nvkm_uvmm_mthd+0x658/0x770
[] nvkm_ioctl+0xdf/0x177
[<03acea2c>] nvif_object_mthd+0xd4/0x100
[<33824292>] nvif_vmm_map+0xeb/0x100
[<537f8629>] nouveau_mem_map+0x79/0xd0
[] nouveau_vma_new+0x19d/0x1c0
[] nouveau_gem_object_open+0xd4/0x140
[<5a53123b>] drm_gem_handle_create_tail+0xe3/0x160
[] nouveau_gem_ioctl_new+0x6e/0xd0
[] drm_ioctl_kernel+0x8c/0xd0
[<4f28d8a6>] drm_ioctl+0x1c4/0x360
[] nouveau_drm_ioctl+0x63/0xb0

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

Re: drm/nouveau/core/memory: kmemleak 684 new suspected memory leaks

[Sergey Senozhatsky]

On (05/17/19 15:13), Sergey Senozhatsky wrote:
> 5.1.0-next-20190517
> 
> I'm looking at quite a lot of kmemleak reports coming from
> drm/nouveau/core/memory, all of which are:
> 
> unreferenced object 0x8deec27c4ac0 (size 16):
>   comm "Web Content", pid 5309, jiffies 4309675011 (age 68.076s)
>   hex dump (first 16 bytes):
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
>   backtrace:
> [<81f2894f>] nvkm_memory_tags_get+0x8e/0x130
> [<7cd7c0bc>] gf100_vmm_valid+0x196/0x2f0
> [<70cc6d67>] nvkm_vmm_map+0xa8/0x360
> [<ab678644>] nvkm_vram_map+0x48/0x50
> [<d8176378>] nvkm_uvmm_mthd+0x658/0x770
> [<463fca5a>] nvkm_ioctl+0xdf/0x177
> [<0afc4996>] nvif_object_mthd+0xd4/0x100
> [<2f7a7385>] nvif_vmm_map+0xeb/0x100
> [<ef2537ed>] nouveau_mem_map+0x79/0xd0
> [<14ddc0cf>] nouveau_vma_new+0x19d/0x1c0
> [<f99888a1>] nouveau_gem_object_open+0xd4/0x140
> [<9cd25861>] drm_gem_handle_create_tail+0xe3/0x160
> [<191784d9>] nouveau_gem_ioctl_new+0x6e/0xd0
> [<159678df>] drm_ioctl_kernel+0x8c/0xd0
> [<fbaa6154>] drm_ioctl+0x1c4/0x360
> [<6833fe15>] nouveau_drm_ioctl+0x63/0xb0

Yet another one (4 leaks), but this looks more like a real leak.

unreferenced object 0x8f1e0cbbe840 (size 192):
  comm "swapper/0", pid 1, jiffies 4294668445 (age 742.639s)
  hex dump (first 32 bytes):
00 90 89 0c 1e 8f ff ff 00 00 00 00 00 00 00 00  
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
  backtrace:
[<6933ed2b>] nouveau_conn_reset+0x20/0xb0
[<572e2e30>] nouveau_connector_create+0x356/0x54c
[<8a6a13cd>] nv50_display_create+0x2fb/0x917
[<7fab0a58>] nouveau_display_create+0x3e6/0x600
[<8b8644c8>] nouveau_drm_device_init+0x149/0x6b0
[<4fd78a1f>] nouveau_drm_probe+0x263/0x2b0
[<357716ef>] pci_device_probe+0xa3/0x110
[<061d40e4>] really_probe+0xd3/0x240
[<0ade44b6>] driver_probe_device+0x50/0xc0
[<9cd0024c>] device_driver_attach+0x53/0x60
[<b11ab0bb>] __driver_attach+0x4c/0xb0
[<16d8457f>] bus_for_each_dev+0x66/0x90
[<f2855f5e>] bus_add_driver+0x171/0x1c0
[<21c08fc1>] driver_register+0x6c/0xaf
[<86357843>] do_one_initcall+0x36/0x1d4
[<a6be055a>] kernel_init_freeable+0x1bf/0x24f

Seems that connector ->state is not fully destroyed.

---
 drivers/gpu/drm/nouveau/nouveau_connector.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
b/drivers/gpu/drm/nouveau/nouveau_connector.c
index 4116ee62adaf..caec1737a7de 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -251,8 +251,10 @@ nouveau_conn_reset(struct drm_connector *connector)
if (WARN_ON(!(asyc = kzalloc(sizeof(*asyc), GFP_KERNEL
return;
 
-   if (connector->state)
+   if (connector->state) {
__drm_atomic_helper_connector_destroy_state(connector->state);
+   kfree(connector->state);
+   }
__drm_atomic_helper_connector_reset(connector, >state);
asyc->dither.mode = DITHERING_MODE_AUTO;
asyc->dither.depth = DITHERING_DEPTH_AUTO;
-- 
2.21.0

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

drm/nouveau/core/memory: kmemleak 684 new suspected memory leaks

[Sergey Senozhatsky]

Hello,

5.1.0-next-20190517

I'm looking at quite a lot of kmemleak reports coming from
drm/nouveau/core/memory, all of which are:

unreferenced object 0x8deec27c4ac0 (size 16):
  comm "Web Content", pid 5309, jiffies 4309675011 (age 68.076s)
  hex dump (first 16 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  
  backtrace:
[<81f2894f>] nvkm_memory_tags_get+0x8e/0x130
[<7cd7c0bc>] gf100_vmm_valid+0x196/0x2f0
[<70cc6d67>] nvkm_vmm_map+0xa8/0x360
[<ab678644>] nvkm_vram_map+0x48/0x50
[<d8176378>] nvkm_uvmm_mthd+0x658/0x770
[<463fca5a>] nvkm_ioctl+0xdf/0x177
[<0afc4996>] nvif_object_mthd+0xd4/0x100
[<2f7a7385>] nvif_vmm_map+0xeb/0x100
[<ef2537ed>] nouveau_mem_map+0x79/0xd0
[<14ddc0cf>] nouveau_vma_new+0x19d/0x1c0
[<f99888a1>] nouveau_gem_object_open+0xd4/0x140
[<9cd25861>] drm_gem_handle_create_tail+0xe3/0x160
[<191784d9>] nouveau_gem_ioctl_new+0x6e/0xd0
[<159678df>] drm_ioctl_kernel+0x8c/0xd0
[<fbaa6154>] drm_ioctl+0x1c4/0x360
[<6833fe15>] nouveau_drm_ioctl+0x63/0xb0

Wondering if those are real leaks or just false positives.

For now I marked `tags' as kmemleak_not_leak(); but most
likely it's utterly wrong.

Any thoughts?

---
 drivers/gpu/drm/nouveau/nvkm/core/memory.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/core/memory.c 
b/drivers/gpu/drm/nouveau/nvkm/core/memory.c
index e85a08ecd9da..cd46f54c5c32 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/memory.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/memory.c
@@ -25,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 
 void
 nvkm_memory_tags_put(struct nvkm_memory *memory, struct nvkm_device *device,
@@ -92,6 +93,7 @@ nvkm_memory_tags_get(struct nvkm_memory *memory, struct 
nvkm_device *device,
 
refcount_set(>refcount, 1);
mutex_unlock(>subdev.mutex);
+   kmemleak_not_leak(tags);
*ptags = tags;
return 0;
 }
-- 
2.21.0

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

[PATCH v2 3/3] drm/panfrost: Use drm_gem_shmem_map_offset()

[Steven Price]

panfrost_ioctl_mmap_bo() contains a reimplementation of
drm_gem_shmem_map_offset() but with a bug - it allows mapping imported
objects (without going through the exporter). Fix this by switching to
use the new drm_gem_shmem_map_offset() function instead which has
the bonus of simplifying the code.

CC: Alyssa Rosenzweig 
Signed-off-by: Steven Price 
---
 drivers/gpu/drm/panfrost/panfrost_drv.c | 16 ++--
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c 
b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 94b0819ad50b..a261b59208d0 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -254,26 +254,14 @@ static int panfrost_ioctl_mmap_bo(struct drm_device *dev, 
void *data,
  struct drm_file *file_priv)
 {
struct drm_panfrost_mmap_bo *args = data;
-   struct drm_gem_object *gem_obj;
-   int ret;
 
if (args->flags != 0) {
DRM_INFO("unknown mmap_bo flags: %d\n", args->flags);
return -EINVAL;
}
 
-   gem_obj = drm_gem_object_lookup(file_priv, args->handle);
-   if (!gem_obj) {
-   DRM_DEBUG("Failed to look up GEM BO %d\n", args->handle);
-   return -ENOENT;
-   }
-
-   ret = drm_gem_create_mmap_offset(gem_obj);
-   if (ret == 0)
-   args->offset = drm_vma_node_offset_addr(_obj->vma_node);
-   drm_gem_object_put_unlocked(gem_obj);
-
-   return ret;
+   return drm_gem_shmem_map_offset(file_priv, dev, args->handle,
+  >offset);
 }
 
 static int panfrost_ioctl_get_bo_offset(struct drm_device *dev, void *data,
-- 
2.20.1

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

Re: drm/nouveau/core/memory: kmemleak 684 new suspected memory leaks

[Sergey Senozhatsky]

On (05/17/19 15:13), Sergey Senozhatsky wrote:

> ... but most likely it's utterly wrong.
> 

JFI, I removed kmemleak annotation and added the following
thing:

@@ -360,6 +360,7 @@ gp100_vmm_valid(struct nvkm_vmm *vmm, void *argv, u32 argc,
return -EINVAL;
}

+   kfree(map->tags);
ret = nvkm_memory_tags_get(memory, device, tags,
   nvkm_ltc_tags_clear,
   >tags);

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

[PATCH v2 2/3] drm: shmem: Add drm_gem_shmem_map_offset() wrapper

[Steven Price]

Provide a wrapper for drm_gem_map_offset() for clients of shmem. This
wrapper provides the correct semantics for the drm_gem_shmem_mmap()
callback.

Signed-off-by: Steven Price 
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 20 
 include/drm/drm_gem_shmem_helper.h |  2 ++
 2 files changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c 
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 1ee208c2c85e..9dbebc4897d1 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -400,6 +400,26 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, 
struct drm_device *dev,
 }
 EXPORT_SYMBOL_GPL(drm_gem_shmem_dumb_create);
 
+/**
+ * drm_gem_map_offset - return the fake mmap offset for a gem object
+ * @file: drm file-private structure containing the gem object
+ * @dev: corresponding drm_device
+ * @handle: gem object handle
+ * @offset: return location for the fake mmap offset
+ *
+ * This provides an offset suitable for user space to return to the
+ * drm_gem_shmem_mmap() callback via an mmap() call.
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_map_offset(struct drm_file *file, struct drm_device *dev,
+u32 handle, u64 *offset)
+{
+   return drm_gem_map_offset(file, dev, handle, offset);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_map_offset);
+
 static vm_fault_t drm_gem_shmem_fault(struct vm_fault *vmf)
 {
struct vm_area_struct *vma = vmf->vma;
diff --git a/include/drm/drm_gem_shmem_helper.h 
b/include/drm/drm_gem_shmem_helper.h
index 038b6d313447..4239dd4f 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -128,6 +128,8 @@ drm_gem_shmem_create_with_handle(struct drm_file *file_priv,
 int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
  struct drm_mode_create_dumb *args);
 
+int drm_gem_shmem_map_offset(struct drm_file *file, struct drm_device *dev,
+u32 handle, u64 *offset);
 int drm_gem_shmem_mmap(struct file *filp, struct vm_area_struct *vma);
 
 extern const struct vm_operations_struct drm_gem_shmem_vm_ops;
-- 
2.20.1

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

Re: [RESEND PATCH v2 2/2] drm/panfrost: Expose perf counters through debugfs

[Steven Price]

On 14/05/2019 14:31, Rob Herring wrote:
> On Tue, May 14, 2019 at 5:48 AM Boris Brezillon
>  wrote:
>>
>> Add a way to dump perf counters through debugfs. The implementation is
>> kept simple and has a number of limitations:
>>
>> * it's not designed for multi-user usage as the counter values are
>>   reset after each dump and there's no per-user context
>> * only accessible to root users
>> * no counters naming/position abstraction. Things are dumped in a raw
>>   format that has to be parsed by the user who has to know where the
>>   relevant values are and what they mean
>>
>> This implementation is intended to be used by mesa developers to help
>> debug perf-related issues while we work on a more generic approach that
>> would allow all GPU drivers to expose their counters in a consistent
>> way. As a result, this debugfs interface is considered unstable and
>> might be deprecated in the future.
>>
>> Signed-off-by: Boris Brezillon 
>> ---
>> Changes in v2:
>> * Expose counters through debugfs and keep things simple for now (we'll
>>   work on a generic solution in parallel)
>> ---
>>  drivers/gpu/drm/panfrost/Makefile   |   3 +-
>>  drivers/gpu/drm/panfrost/panfrost_device.c  |   9 +
>>  drivers/gpu/drm/panfrost/panfrost_device.h  |   3 +
>>  drivers/gpu/drm/panfrost/panfrost_drv.c |   7 +
>>  drivers/gpu/drm/panfrost/panfrost_gpu.c |   7 +
>>  drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 339 
>>  drivers/gpu/drm/panfrost/panfrost_perfcnt.h |  15 +
>>  drivers/gpu/drm/panfrost/panfrost_regs.h|  19 ++
>>  8 files changed, 401 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_perfcnt.c
>>  create mode 100644 drivers/gpu/drm/panfrost/panfrost_perfcnt.h
>>
[...]
>> new file mode 100644
>> index ..8093783a5a26
>> --- /dev/null
>> +++ b/drivers/gpu/drm/panfrost/panfrost_perfcnt.c
>> @@ -0,0 +1,339 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright 2019 Collabora Ltd */
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#include "panfrost_device.h"
>> +#include "panfrost_features.h"
>> +#include "panfrost_gem.h"
>> +#include "panfrost_issues.h"
>> +#include "panfrost_job.h"
>> +#include "panfrost_mmu.h"
>> +#include "panfrost_regs.h"
>> +
>> +#define COUNTERS_PER_BLOCK 64
>> +#define BYTES_PER_COUNTER  4
>> +#define BLOCKS_PER_COREGROUP   8
>> +#define V4_SHADERS_PER_COREGROUP   4
>> +
>> +struct panfrost_perfcnt {
>> +   struct panfrost_gem_object *bo;
>> +   size_t bosize;
>> +   void *buf;
>> +   bool enabled;
>> +   struct mutex lock;
>> +   struct completion dump_comp;
>> +};
>> +
>> +void panfrost_perfcnt_clean_cache_done(struct panfrost_device *pfdev)
>> +{
>> +   complete(>perfcnt->dump_comp);
>> +}
>> +
>> +void panfrost_perfcnt_sample_done(struct panfrost_device *pfdev)
>> +{
>> +   gpu_write(pfdev, GPU_CMD, GPU_CMD_CLEAN_CACHES);
>> +}
>> +
>> +static void panfrost_perfcnt_setup(struct panfrost_device *pfdev)
>> +{
>> +   u32 cfg;
>> +
>> +   /*
>> +* Always use address space 0 for now.
>> +* FIXME: this needs to be updated when we start using different
>> +* address space.
>> +*/
>> +   cfg = GPU_PERFCNT_CFG_AS(0);
>> +   if (panfrost_model_cmp(pfdev, 0x1000) >= 0)
> 
> You've got a couple of these. Perhaps we should add either a
> model_is_bifrost() helper or an is_bifrost variable to use.
> 
>> +   cfg |= GPU_PERFCNT_CFG_SETSEL(1);

mali_kbase only sets this if CONFIG_MALI_PRFCNT_SET_SECONDARY is defined
- i.e. this selects a different selection of counters. At at the very
least this should be an optional flag that can be set, but I suspect the
primary set are the ones you are interested in.

>> +
>> +   gpu_write(pfdev, GPU_PERFCNT_CFG,
>> + cfg | GPU_PERFCNT_CFG_MODE(GPU_PERFCNT_CFG_MODE_OFF));
>> +
>> +   if (!pfdev->perfcnt->enabled)
>> +   return;
>> +
>> +   gpu_write(pfdev, GPU_PRFCNT_JM_EN, 0x);
>> +   gpu_write(pfdev, GPU_PRFCNT_SHADER_EN, 0x);
>> +   gpu_write(pfdev, GPU_PRFCNT_MMU_L2_EN, 0x);
>> +
>> +   /*
>> +* Due to PRLAM-8186 we need to disable the Tiler before we enable HW
>> +* counters.
> 
> Seems like you could just always apply the work-around? It doesn't
> look like it would be much different.

Technically the workaround causes the driver to perform the operation in
the wrong order below (write TILER_EN when the mode is MANUAL) - this is
a workaround for the hardware issue (and therefore works on that
hardware). But I wouldn't like to say it works on other hardware which
doesn't have the issue.

>> +*/
>> +   if (panfrost_has_hw_issue(pfdev, HW_ISSUE_8186))
>> +   gpu_write(pfdev, GPU_PRFCNT_TILER_EN, 0);
>> +   else
>> +   

Re: [PATCH 4/4] arm64: DTS: allwinner: a64: enable ANX6345 bridge on Teres-I

[Maxime Ripard]

On Thu, May 16, 2019 at 06:48:59PM +0200, Torsten Duwe wrote:
> On Thu, May 16, 2019 at 09:06:41AM -0700, Vasily Khoruzhick wrote:
> >
> > Driver can talk to the panel over AUX channel only after t1+t3, t1 is
> > up to 10ms, t3 is up to 200ms.
>
> This is after power-on. The boot loader needs to deal with this.

The bootloader can deal with it, but the kernel will also need to. The
bootloader might not be doing this because it's not been updated, the
regulator might have been disabled between the time the kernel was
started and the time the bridge driver probes, etc.

Maxime

--
Maxime Ripard, Bootlin
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