[Bug 110850] Vlc fails to decode video using vaapi
https://bugs.freedesktop.org/show_bug.cgi?id=110850 Bug ID: 110850 Summary: Vlc fails to decode video using vaapi Product: Mesa Version: 19.1 Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: akarshanbis...@fedoraproject.org QA Contact: dri-devel@lists.freedesktop.org Tested Mesa version: 19.1.0 rc4, 19.0.5 Operating System: Fedora 30 with kernel 5.1.6 VLC version: 3.0.7 Vetinari (revision 3.0.6-223-g5fc4822ace) GPU's tested: Both on Radeon RX550 and AMD APU a9-9410(stoney series) Testing on both Xorg and Wayland This bug is present since long time, selecting either vaapi video decoder or vaapi video decoder via drm in vlc--> toos --> preferences--> Input/Codecs --> hardware accelerated decoding fails and vlc switches to either vdpau or software decoding. (vdpau driver can't decode some hevc videos, vdpau is not supported on wayland) Steps to reproduce: 1) After selecting vaapi video decoder in vlc, run vlc on a terminal with the -v flag (for verbose) 2) play any media 3) Check the terminal ( Meanwhile vlc will try to switch to vdpau) Actual results: vlc's vaapi decoder fails with: ``` libva info: VA-API version 1.4.1 libva info: va_getDriverName() returns 0 libva info: User requested driver 'radeonsi' libva info: Trying to open /usr/lib64/dri/radeonsi_drv_video.so libva info: Found init function __vaDriverInit_1_4 libva info: va_openDriver() returns 0 [7f4c78001f60] glconv_vaapi_x11 gl error: vaDeriveImage: operation failed [7f4c7c062b30] main video output error: video output creation failed [7f4c88056060] main decoder error: failed to create video output ``` Expected result: Vlc should play vaapi as it does in Intel GPUs. I have a vlc bug ticket which says that the driver is buggy here: https://trac.videolan.org/vlc/ticket/21194 -- 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
[git pull] drm fixes for v5.2-rc4 (v2)
Hey Linus, A small bit more lively this week but not majorly so. I'm away in Japan next week for family holiday, so I'll be pretty disconnected, I've asked Daniel to do fixes for the week while I'm out. I sent this out earlier, but I forgot the subject, and then Ben asked about some nouveau firmware fixes. The nouveau firmware changes are a bit large, but they address a big problem where a whole set of boards don't load with the driver, and the new firmware fixes that, so I think it's worth trying to land it now. core: - Allow fb changes in async commits (drivers as well) udmabuf: - Unmap scatterlist when unmapping udmabuf nouveau: - firmware loading fixes for secboot firmware on new GPU revision. komeda: - oops, dma mapping and warning fixes arm-hdlcd: - clock fixes, - mode validation fix i915: - Add a missing Icelake workaround - GVT - DMA map fault fix and enforcement fixes amdgpu: - DCE resume fix - New raven variation updates Dave. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm/bridge: sii902x: fix a signedness bug in sii902x_audio_codec_init()
The "num_lanes" variable needs to be signed for the error handling to
work. The "i" variable can be a regular int as well.
Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/bridge/sii902x.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index 6b03616d6bc3..dd7aa466b280 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -719,7 +719,7 @@ static int sii902x_audio_codec_init(struct sii902x *sii902x,
.max_i2s_channels = 0,
};
u8 lanes[4];
- u32 num_lanes, i;
+ int num_lanes, i;
if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
dev_dbg(dev, "%s: No \"#sound-dai-cells\", no audio\n",
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[bug report] drm/bridge: sii902x: Implement HDMI audio support
Hello Jyri Sarha,
The patch ff5781634c41: "drm/bridge: sii902x: Implement HDMI audio
support" from May 27, 2019, leads to the following static checker
warning:
drivers/gpu/drm/bridge/sii902x.c:753 sii902x_audio_codec_init()
warn: 'sii902x->audio.mclk' isn't an ERR_PTR
drivers/gpu/drm/bridge/sii902x.c
723
724 if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
725 dev_dbg(dev, "%s: No \"#sound-dai-cells\", no audio\n",
726 __func__);
727 return 0;
728 }
729
730 num_lanes = of_property_read_variable_u8_array(dev->of_node,
731
"sil,i2s-data-lanes",
732 lanes, 1,
733
ARRAY_SIZE(lanes));
734
735 if (num_lanes == -EINVAL) {
736 dev_dbg(dev,
737 "%s: No \"sil,i2s-data-lanes\", use default
<0>\n",
738 __func__);
739 num_lanes = 1;
740 lanes[0] = 0;
741 } else if (num_lanes < 0) {
742 dev_err(dev,
743 "%s: Error gettin \"sil,i2s-data-lanes\": %d\n",
744 __func__, num_lanes);
745 return num_lanes;
746 }
747 codec_data.max_i2s_channels = 2 * num_lanes;
748
749 for (i = 0; i < num_lanes; i++)
750 sii902x->audio.i2s_fifo_sequence[i] |= audio_fifo_id[i]
|
751 i2s_lane_id[lanes[i]] |
SII902X_TPI_I2S_FIFO_ENABLE;
752
753 if (IS_ERR(sii902x->audio.mclk)) {
^^^
The "sii902x->audio.mclk" variable is never initialized.
754 dev_err(dev, "%s: No clock (audio mclk) found: %ld\n",
755 __func__, PTR_ERR(sii902x->audio.mclk));
756 return 0;
757 }
758
759 sii902x->audio.pdev = platform_device_register_data(
regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2] drm/bridge: sii902x: re-order conditions to prevent out of bounds read
This should check that "i" is within bounds before checking reading from
the array.
Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
Signed-off-by: Dan Carpenter
---
drivers/gpu/drm/bridge/sii902x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index d6f98d388ac2..6b03616d6bc3 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev, void
*data,
if (ret)
goto out;
- for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
-i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
+ for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
+ sii902x->audio.i2s_fifo_sequence[i]; i++)
regmap_write(sii902x->regmap,
SII902X_TPI_I2S_ENABLE_MAPPING_REG,
sii902x->audio.i2s_fifo_sequence[i]);
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/i915: remove irrelevant DRM_UNLOCKED flag
On Wed, May 22, 2019 at 04:47:01PM +0100, Emil Velikov wrote:
> From: Emil Velikov
>
> DRM_UNLOCKED doesn't do anything for non-legacy drivers. Remove it.
>
> Cc: intel-...@lists.freedesktop.org
> Cc: Daniel Vetter
> Signed-off-by: Emil Velikov
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/i915/i915_drv.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 1ad88e6d7c04..a18c155cff88 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -3145,9 +3145,9 @@ static const struct drm_ioctl_desc i915_ioctls[] = {
> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM,
> i915_gem_context_getparam_ioctl, DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM,
> i915_gem_context_setparam_ioctl, DRM_RENDER_ALLOW),
> DRM_IOCTL_DEF_DRV(I915_PERF_OPEN, i915_perf_open_ioctl,
> DRM_RENDER_ALLOW),
> - DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl,
> DRM_UNLOCKED|DRM_RENDER_ALLOW),
> - DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG,
> i915_perf_remove_config_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> - DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl,
> DRM_UNLOCKED|DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_PERF_ADD_CONFIG, i915_perf_add_config_ioctl,
> DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_PERF_REMOVE_CONFIG,
> i915_perf_remove_config_ioctl, DRM_RENDER_ALLOW),
> + DRM_IOCTL_DEF_DRV(I915_QUERY, i915_query_ioctl, DRM_RENDER_ALLOW),
> };
>
> static struct drm_driver driver = {
> --
> 2.21.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/vkms: Use index instead of 0 in possible crtc
On Thu, Jun 06, 2019 at 07:40:38PM -0300, Rodrigo Siqueira wrote:
> When vkms calls drm_universal_plane_init(), it sets 0 for the
> possible_crtcs parameter which works well for a single encoder and
> connector; however, this approach is not flexible and does not fit well
> for vkms. This commit adds an index parameter for vkms_plane_init()
> which makes code flexible and enables vkms to support other DRM features.
>
> Signed-off-by: Rodrigo Siqueira
I think a core patch to WARN_ON if this is NULL would be good. Since
that's indeed a bit broken ... We'd need to check all callers to make sure
there's not other such bugs anywhere ofc.
-Daniel
> ---
> drivers/gpu/drm/vkms/vkms_drv.c| 2 +-
> drivers/gpu/drm/vkms/vkms_drv.h| 4 ++--
> drivers/gpu/drm/vkms/vkms_output.c | 6 +++---
> drivers/gpu/drm/vkms/vkms_plane.c | 4 ++--
> 4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 738dd6206d85..92296bd8f623 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -92,7 +92,7 @@ static int vkms_modeset_init(struct vkms_device *vkmsdev)
> dev->mode_config.max_height = YRES_MAX;
> dev->mode_config.preferred_depth = 24;
>
> - return vkms_output_init(vkmsdev);
> + return vkms_output_init(vkmsdev, 0);
> }
>
> static int __init vkms_init(void)
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 81f1cfbeb936..e81073dea154 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -113,10 +113,10 @@ bool vkms_get_vblank_timestamp(struct drm_device *dev,
> unsigned int pipe,
> int *max_error, ktime_t *vblank_time,
> bool in_vblank_irq);
>
> -int vkms_output_init(struct vkms_device *vkmsdev);
> +int vkms_output_init(struct vkms_device *vkmsdev, int index);
>
> struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> - enum drm_plane_type type);
> + enum drm_plane_type type, int index);
>
> /* Gem stuff */
> struct drm_gem_object *vkms_gem_create(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c
> b/drivers/gpu/drm/vkms/vkms_output.c
> index 3b162b25312e..1442b447c707 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -36,7 +36,7 @@ static const struct drm_connector_helper_funcs
> vkms_conn_helper_funcs = {
> .get_modes= vkms_conn_get_modes,
> };
>
> -int vkms_output_init(struct vkms_device *vkmsdev)
> +int vkms_output_init(struct vkms_device *vkmsdev, int index)
> {
> struct vkms_output *output = &vkmsdev->output;
> struct drm_device *dev = &vkmsdev->drm;
> @@ -46,12 +46,12 @@ int vkms_output_init(struct vkms_device *vkmsdev)
> struct drm_plane *primary, *cursor = NULL;
> int ret;
>
> - primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY);
> + primary = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_PRIMARY, index);
> if (IS_ERR(primary))
> return PTR_ERR(primary);
>
> if (enable_cursor) {
> - cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR);
> + cursor = vkms_plane_init(vkmsdev, DRM_PLANE_TYPE_CURSOR, index);
> if (IS_ERR(cursor)) {
> ret = PTR_ERR(cursor);
> goto err_cursor;
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c
> b/drivers/gpu/drm/vkms/vkms_plane.c
> index 0e67d2d42f0c..20ffc52f9194 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -168,7 +168,7 @@ static const struct drm_plane_helper_funcs
> vkms_primary_helper_funcs = {
> };
>
> struct drm_plane *vkms_plane_init(struct vkms_device *vkmsdev,
> - enum drm_plane_type type)
> + enum drm_plane_type type, int index)
> {
> struct drm_device *dev = &vkmsdev->drm;
> const struct drm_plane_helper_funcs *funcs;
> @@ -190,7 +190,7 @@ struct drm_plane *vkms_plane_init(struct vkms_device
> *vkmsdev,
> funcs = &vkms_primary_helper_funcs;
> }
>
> - ret = drm_universal_plane_init(dev, plane, 0,
> + ret = drm_universal_plane_init(dev, plane, 1 << index,
> &vkms_plane_funcs,
> formats, nformats,
> NULL, type, NULL);
> --
> 2.21.0
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
[PATCH v2 hmm 07/11] mm/hmm: Use lockdep instead of comments
From: Jason Gunthorpe
So we can check locking at runtime.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
---
v2
- Fix missing & in lockdeps (Jason)
---
mm/hmm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index f67ba32983d9f1..c702cd72651b53 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -254,11 +254,11 @@ static const struct mmu_notifier_ops hmm_mmu_notifier_ops
= {
*
* To start mirroring a process address space, the device driver must register
* an HMM mirror struct.
- *
- * THE mm->mmap_sem MUST BE HELD IN WRITE MODE !
*/
int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm)
{
+ lockdep_assert_held_exclusive(&mm->mmap_sem);
+
/* Sanity check */
if (!mm || !mirror || !mirror->ops)
return -EINVAL;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
From: Jason Gunthorpe
Ralph observes that hmm_range_register() can only be called by a driver
while a mirror is registered. Make this clear in the API by passing in the
mirror structure as a parameter.
This also simplifies understanding the lifetime model for struct hmm, as
the hmm pointer must be valid as part of a registered mirror so all we
need in hmm_register_range() is a simple kref_get.
Suggested-by: Ralph Campbell
Signed-off-by: Jason Gunthorpe
---
v2
- Include the oneline patch to nouveau_svm.c
---
drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
include/linux/hmm.h | 7 ---
mm/hmm.c | 15 ++-
3 files changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
b/drivers/gpu/drm/nouveau/nouveau_svm.c
index 93ed43c413f0bb..8c92374afcf227 100644
--- a/drivers/gpu/drm/nouveau/nouveau_svm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
@@ -649,7 +649,7 @@ nouveau_svm_fault(struct nvif_notify *notify)
range.values = nouveau_svm_pfn_values;
range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT;
again:
- ret = hmm_vma_fault(&range, true);
+ ret = hmm_vma_fault(&svmm->mirror, &range, true);
if (ret == 0) {
mutex_lock(&svmm->mutex);
if (!hmm_vma_range_done(&range)) {
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 688c5ca7068795..2d519797cb134a 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -505,7 +505,7 @@ static inline bool hmm_mirror_mm_is_alive(struct hmm_mirror
*mirror)
* Please see Documentation/vm/hmm.rst for how to use the range API.
*/
int hmm_range_register(struct hmm_range *range,
- struct mm_struct *mm,
+ struct hmm_mirror *mirror,
unsigned long start,
unsigned long end,
unsigned page_shift);
@@ -541,7 +541,8 @@ static inline bool hmm_vma_range_done(struct hmm_range
*range)
}
/* This is a temporary helper to avoid merge conflict between trees. */
-static inline int hmm_vma_fault(struct hmm_range *range, bool block)
+static inline int hmm_vma_fault(struct hmm_mirror *mirror,
+ struct hmm_range *range, bool block)
{
long ret;
@@ -554,7 +555,7 @@ static inline int hmm_vma_fault(struct hmm_range *range,
bool block)
range->default_flags = 0;
range->pfn_flags_mask = -1UL;
- ret = hmm_range_register(range, range->vma->vm_mm,
+ ret = hmm_range_register(range, mirror,
range->start, range->end,
PAGE_SHIFT);
if (ret)
diff --git a/mm/hmm.c b/mm/hmm.c
index 547002f56a163d..8796447299023c 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -925,13 +925,13 @@ static void hmm_pfns_clear(struct hmm_range *range,
* Track updates to the CPU page table see include/linux/hmm.h
*/
int hmm_range_register(struct hmm_range *range,
- struct mm_struct *mm,
+ struct hmm_mirror *mirror,
unsigned long start,
unsigned long end,
unsigned page_shift)
{
unsigned long mask = ((1UL << page_shift) - 1UL);
- struct hmm *hmm;
+ struct hmm *hmm = mirror->hmm;
range->valid = false;
range->hmm = NULL;
@@ -945,15 +945,12 @@ int hmm_range_register(struct hmm_range *range,
range->start = start;
range->end = end;
- hmm = hmm_get_or_create(mm);
- if (!hmm)
- return -EFAULT;
-
/* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead) {
- hmm_put(hmm);
+ if (hmm->mm == NULL || hmm->dead)
return -EFAULT;
- }
+
+ range->hmm = hmm;
+ kref_get(&hmm->kref);
/* Initialize range to track CPU page table updates. */
mutex_lock(&hmm->lock);
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 09/11] mm/hmm: Poison hmm_range during unregister
On 6/6/19 11:44 AM, Jason Gunthorpe wrote: > From: Jason Gunthorpe > > Trying to misuse a range outside its lifetime is a kernel bug. Use WARN_ON > and poison bytes to detect this condition. > > Signed-off-by: Jason Gunthorpe > Reviewed-by: Jérôme Glisse > --- > v2 > - Keep range start/end valid after unregistration (Jerome) > --- > mm/hmm.c | 7 +-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/mm/hmm.c b/mm/hmm.c > index 6802de7080d172..c2fecb3ecb11e1 100644 > --- a/mm/hmm.c > +++ b/mm/hmm.c > @@ -937,7 +937,7 @@ void hmm_range_unregister(struct hmm_range *range) > struct hmm *hmm = range->hmm; > > /* Sanity check this really should not happen. */ That comment can also be deleted, as it has the same meaning as the WARN_ON() that you just added. > - if (hmm == NULL || range->end <= range->start) > + if (WARN_ON(range->end <= range->start)) > return; > > mutex_lock(&hmm->lock); > @@ -948,7 +948,10 @@ void hmm_range_unregister(struct hmm_range *range) > range->valid = false; > mmput(hmm->mm); > hmm_put(hmm); > - range->hmm = NULL; > + > + /* The range is now invalid, leave it poisoned. */ To be precise, we are poisoning the range's back pointer to it's owning hmm instance. Maybe this is clearer: /* * The range is now invalid, so poison it's hmm pointer. * Leave other range-> fields in place, for the caller's use. */ ...or something like that? > + range->valid = false; > + memset(&range->hmm, POISON_INUSE, sizeof(range->hmm)); > } > EXPORT_SYMBOL(hmm_range_unregister); > > The above are very minor documentation points, so: Reviewed-by: John Hubbard thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 03/11] mm/hmm: Hold a mmgrab from hmm to mm
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> So long a a struct hmm pointer exists, so should the struct mm it is
> linked too. Hold the mmgrab() as soon as a hmm is created, and mmdrop() it
> once the hmm refcount goes to zero.
>
> Since mmdrop() (ie a 0 kref on struct mm) is now impossible with a !NULL
> mm->hmm delete the hmm_hmm_destroy().
>
> Signed-off-by: Jason Gunthorpe
> Reviewed-by: Jérôme Glisse
> ---
> v2:
> - Fix error unwind paths in hmm_get_or_create (Jerome/Jason)
> ---
> include/linux/hmm.h | 3 ---
> kernel/fork.c | 1 -
> mm/hmm.c| 22 --
> 3 files changed, 4 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 2d519797cb134a..4ee3acabe5ed22 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -586,14 +586,11 @@ static inline int hmm_vma_fault(struct hmm_mirror
> *mirror,
> }
>
> /* Below are for HMM internal use only! Not to be used by device driver! */
> -void hmm_mm_destroy(struct mm_struct *mm);
> -
> static inline void hmm_mm_init(struct mm_struct *mm)
> {
> mm->hmm = NULL;
> }
> #else /* IS_ENABLED(CONFIG_HMM_MIRROR) */
> -static inline void hmm_mm_destroy(struct mm_struct *mm) {}
> static inline void hmm_mm_init(struct mm_struct *mm) {}
> #endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
>
> diff --git a/kernel/fork.c b/kernel/fork.c
> index b2b87d450b80b5..588c768ae72451 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -673,7 +673,6 @@ void __mmdrop(struct mm_struct *mm)
> WARN_ON_ONCE(mm == current->active_mm);
> mm_free_pgd(mm);
> destroy_context(mm);
> - hmm_mm_destroy(mm);
This is particularly welcome, not to have an "HMM is special" case
in such a core part of process/mm code.
> mmu_notifier_mm_destroy(mm);
> check_mm(mm);
> put_user_ns(mm->user_ns);
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 8796447299023c..cc7c26fda3300e 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -29,6 +29,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -82,6 +83,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> hmm->notifiers = 0;
> hmm->dead = false;
> hmm->mm = mm;
> + mmgrab(hmm->mm);
>
> spin_lock(&mm->page_table_lock);
> if (!mm->hmm)
> @@ -109,6 +111,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> mm->hmm = NULL;
> spin_unlock(&mm->page_table_lock);
> error:
> + mmdrop(hmm->mm);
> kfree(hmm);
> return NULL;
> }
> @@ -130,6 +133,7 @@ static void hmm_free(struct kref *kref)
> mm->hmm = NULL;
> spin_unlock(&mm->page_table_lock);
>
> + mmdrop(hmm->mm);
> mmu_notifier_call_srcu(&hmm->rcu, hmm_free_rcu);
> }
>
> @@ -138,24 +142,6 @@ static inline void hmm_put(struct hmm *hmm)
> kref_put(&hmm->kref, hmm_free);
> }
>
> -void hmm_mm_destroy(struct mm_struct *mm)
> -{
> - struct hmm *hmm;
> -
> - spin_lock(&mm->page_table_lock);
> - hmm = mm_get_hmm(mm);
> - mm->hmm = NULL;
> - if (hmm) {
> - hmm->mm = NULL;
> - hmm->dead = true;
> - spin_unlock(&mm->page_table_lock);
> - hmm_put(hmm);
> - return;
> - }
> -
> - spin_unlock(&mm->page_table_lock);
> -}
> -
> static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> {
> struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
>
Failed to find any problems with this. :)
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 03/11] mm/hmm: Hold a mmgrab from hmm to mm
From: Jason Gunthorpe
So long a a struct hmm pointer exists, so should the struct mm it is
linked too. Hold the mmgrab() as soon as a hmm is created, and mmdrop() it
once the hmm refcount goes to zero.
Since mmdrop() (ie a 0 kref on struct mm) is now impossible with a !NULL
mm->hmm delete the hmm_hmm_destroy().
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
---
v2:
- Fix error unwind paths in hmm_get_or_create (Jerome/Jason)
---
include/linux/hmm.h | 3 ---
kernel/fork.c | 1 -
mm/hmm.c| 22 --
3 files changed, 4 insertions(+), 22 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 2d519797cb134a..4ee3acabe5ed22 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -586,14 +586,11 @@ static inline int hmm_vma_fault(struct hmm_mirror *mirror,
}
/* Below are for HMM internal use only! Not to be used by device driver! */
-void hmm_mm_destroy(struct mm_struct *mm);
-
static inline void hmm_mm_init(struct mm_struct *mm)
{
mm->hmm = NULL;
}
#else /* IS_ENABLED(CONFIG_HMM_MIRROR) */
-static inline void hmm_mm_destroy(struct mm_struct *mm) {}
static inline void hmm_mm_init(struct mm_struct *mm) {}
#endif /* IS_ENABLED(CONFIG_HMM_MIRROR) */
diff --git a/kernel/fork.c b/kernel/fork.c
index b2b87d450b80b5..588c768ae72451 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -673,7 +673,6 @@ void __mmdrop(struct mm_struct *mm)
WARN_ON_ONCE(mm == current->active_mm);
mm_free_pgd(mm);
destroy_context(mm);
- hmm_mm_destroy(mm);
mmu_notifier_mm_destroy(mm);
check_mm(mm);
put_user_ns(mm->user_ns);
diff --git a/mm/hmm.c b/mm/hmm.c
index 8796447299023c..cc7c26fda3300e 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -82,6 +83,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
hmm->notifiers = 0;
hmm->dead = false;
hmm->mm = mm;
+ mmgrab(hmm->mm);
spin_lock(&mm->page_table_lock);
if (!mm->hmm)
@@ -109,6 +111,7 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
mm->hmm = NULL;
spin_unlock(&mm->page_table_lock);
error:
+ mmdrop(hmm->mm);
kfree(hmm);
return NULL;
}
@@ -130,6 +133,7 @@ static void hmm_free(struct kref *kref)
mm->hmm = NULL;
spin_unlock(&mm->page_table_lock);
+ mmdrop(hmm->mm);
mmu_notifier_call_srcu(&hmm->rcu, hmm_free_rcu);
}
@@ -138,24 +142,6 @@ static inline void hmm_put(struct hmm *hmm)
kref_put(&hmm->kref, hmm_free);
}
-void hmm_mm_destroy(struct mm_struct *mm)
-{
- struct hmm *hmm;
-
- spin_lock(&mm->page_table_lock);
- hmm = mm_get_hmm(mm);
- mm->hmm = NULL;
- if (hmm) {
- hmm->mm = NULL;
- hmm->dead = true;
- spin_unlock(&mm->page_table_lock);
- hmm_put(hmm);
- return;
- }
-
- spin_unlock(&mm->page_table_lock);
-}
-
static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
{
struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 08/11] mm/hmm: Remove racy protection against double-unregistration
From: Jason Gunthorpe
No other register/unregister kernel API attempts to provide this kind of
protection as it is inherently racy, so just drop it.
Callers should provide their own protection, it appears nouveau already
does, but just in case drop a debugging POISON.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
---
mm/hmm.c | 9 ++---
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index c702cd72651b53..6802de7080d172 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -284,18 +284,13 @@ EXPORT_SYMBOL(hmm_mirror_register);
*/
void hmm_mirror_unregister(struct hmm_mirror *mirror)
{
- struct hmm *hmm = READ_ONCE(mirror->hmm);
-
- if (hmm == NULL)
- return;
+ struct hmm *hmm = mirror->hmm;
down_write(&hmm->mirrors_sem);
list_del_init(&mirror->list);
- /* To protect us against double unregister ... */
- mirror->hmm = NULL;
up_write(&hmm->mirrors_sem);
-
hmm_put(hmm);
+ memset(&mirror->hmm, POISON_INUSE, sizeof(mirror->hmm));
}
EXPORT_SYMBOL(hmm_mirror_unregister);
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] Two bug-fixes for HMM
On Thu, Jun 06, 2019 at 07:04:46PM +, Kuehling, Felix wrote: > On 2019-06-06 11:11 a.m., Jason Gunthorpe wrote: > > On Fri, May 10, 2019 at 07:53:21PM +, Kuehling, Felix wrote: > >> These problems were found in AMD-internal testing as we're working on > >> adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get > >> them applied to a mainline Linux kernel as well as drm-next and > >> amd-staging-drm-next sooner rather than later. > >> > >> Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3, > >> but the driver changes for HMM are expected to land in 5.2 and will need to > >> be rebased on those HMM changes. > >> > >> I'd like to work out a flow between Jerome, Dave, Alex and myself that > >> allows us to test the latest version of HMM on amd-staging-drm-next so > >> that ideally everything comes together in master without much need for > >> rebasing and retesting. > > I think we have that now, I'm running a hmm.git that is clean and can > > be used for merging into DRM related trees (and RDMA). I've commited > > to send this tree to Linus at the start of the merge window. > > > > See here: > > > > https://lore.kernel.org/lkml/20190524124455.gb16...@ziepe.ca/ > > > > The tree is here: > > > > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm > > > > However please consult with me before making a merge commit to be > > co-ordinated. Thanks > > > > I see in this thread that AMDGPU missed 5.2 beacause of the > > co-ordination problems this tree is intended to solve, so I'm very > > hopeful this will help your work move into 5.3! > > Thanks Jason. Our two patches below were already included in the MM tree > (https://ozlabs.org/~akpm/mmots/broken-out/). With your new hmm.git, > does that mean HMM fixes and changes will no longer go through Andrew > Morton but directly through your tree to Linus? I belive so, that is what we agreed to in the RFC. At least for this cycle. I already noticed the duplication and sent Andrew a separate note.. It will be best if most of the things touching mm/hmm.c go to hmm.git to avoid conflicts for Linus. > We have also applied the two patches to our internal tree which is > currently based on 5.2-rc1 so we can make progress. Makes sense, this is is also why this shared tree should be very helpful.. I intend to run it as a clean and stable non-rebasing tree, ah probably starting tomorrow since I see there is still another fixup. :) > Alex, I think merging hmm would be an extra step every time you rebase > amd-staging-drm-next. We could probably also merge hmm at other times as > needed. Do you think this would cause trouble or confusion for > upstreaming through drm-next? I'm not sure what the workflow the amd tree uses, but.. Broadly: If the AMD tree is rebasing then likely you can simply rebase your AMD tree on top of hmm.git (or maybe hmm.git merge'd into DRM). Most likely we will want to send a PR for hmm.git to main DRM tree prior to merging AMD's tree, but I'm also rather relying on DRM folks to help build the workflow they want in their world.. There are quite a few options depending on people's preferences and workflow. Thanks, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v16 14/16] tee, arm64: untag user pointers in tee_shm_register
On Mon, Jun 3, 2019 at 6:56 PM Andrey Konovalov wrote: > > This patch is a part of a series that extends arm64 kernel ABI to allow to > pass tagged user pointers (with the top byte set to something else other > than 0x00) as syscall arguments. > > tee_shm_register()->optee_shm_unregister()->check_mem_type() uses provided > user pointers for vma lookups (via __check_mem_type()), which can only by > done with untagged pointers. > > Untag user pointers in this function. > > Signed-off-by: Andrey Konovalov Acked-by: Jens Wiklander > --- > drivers/tee/tee_shm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > index 49fd7312e2aa..96945f4cefb8 100644 > --- a/drivers/tee/tee_shm.c > +++ b/drivers/tee/tee_shm.c > @@ -263,6 +263,7 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, > unsigned long addr, > shm->teedev = teedev; > shm->ctx = ctx; > shm->id = -1; > + addr = untagged_addr(addr); > start = rounddown(addr, PAGE_SIZE); > shm->offset = addr - start; > shm->size = length; > -- > 2.22.0.rc1.311.g5d7573a151-goog > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 11/11] mm/hmm: Remove confusing comment and logic from hmm_release
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> hmm_release() is called exactly once per hmm. ops->release() cannot
> accidentally trigger any action that would recurse back onto
> hmm->mirrors_sem.
>
> This fixes a use after-free race of the form:
>
>CPU0 CPU1
>hmm_release()
> up_write(&hmm->mirrors_sem);
> hmm_mirror_unregister(mirror)
> down_write(&hmm->mirrors_sem);
> up_write(&hmm->mirrors_sem);
> kfree(mirror)
> mirror->ops->release(mirror)
>
> The only user we have today for ops->release is an empty function, so this
> is unambiguously safe.
>
> As a consequence of plugging this race drivers are not allowed to
> register/unregister mirrors from within a release op.
>
> Signed-off-by: Jason Gunthorpe
> ---
> mm/hmm.c | 28 +---
> 1 file changed, 9 insertions(+), 19 deletions(-)
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 709d138dd49027..3a45dd3d778248 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -136,26 +136,16 @@ static void hmm_release(struct mmu_notifier *mn, struct
> mm_struct *mm)
> WARN_ON(!list_empty(&hmm->ranges));
> mutex_unlock(&hmm->lock);
>
> - down_write(&hmm->mirrors_sem);
> - mirror = list_first_entry_or_null(&hmm->mirrors, struct hmm_mirror,
> - list);
> - while (mirror) {
> - list_del_init(&mirror->list);
> - if (mirror->ops->release) {
> - /*
> - * Drop mirrors_sem so the release callback can wait
> - * on any pending work that might itself trigger a
> - * mmu_notifier callback and thus would deadlock with
> - * us.
> - */
> - up_write(&hmm->mirrors_sem);
> + down_read(&hmm->mirrors_sem);
This is cleaner and simpler, but I suspect it is leading to the deadlock
that Ralph Campbell is seeing in his driver testing. (And in general, holding
a lock during a driver callback usually leads to deadlocks.)
Ralph, is this the one? It's the only place in this patchset where I can
see a lock around a callback to driver code, that wasn't there before. So
I'm pretty sure it is the one...
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
drm/imx: 'flip_done timed out' on i.MX6D+ and kernel v4.19.48
Hi, We observed the following error on an i.MX6D+ CPU during start of X. As a result, the screen goes blank. --- [ 3599.200886] [drm:drm_atomic_helper_wait_for_flip_done] *ERROR* [CRTC:30:crtc-0] flip_done timed out [ 3610.080885] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:30:crtc-0] flip_done timed out [ 3620.320849] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CONNECTOR:45:LVDS-1] flip_done timed out [ 3630.560864] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:28:plane-0] flip_done timed out --- The error happens quite randomly but it can be reliably reproduced by repeatedly restarting X . On our system, the error occurs somewhere between 80 and 300 restarts of X. We first observed the issue using kernel 4.14.123. Updating to 4.19.48 did not solve the issue unfortunately. Cheers, Hannes Moesl ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 04/11] mm/hmm: Simplify hmm_get_or_create and make it reliable
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> As coded this function can false-fail in various racy situations. Make it
> reliable by running only under the write side of the mmap_sem and avoiding
> the false-failing compare/exchange pattern.
>
> Also make the locking very easy to understand by only ever reading or
> writing mm->hmm while holding the write side of the mmap_sem.
>
> Signed-off-by: Jason Gunthorpe
> ---
> v2:
> - Fix error unwind of mmgrab (Jerome)
> - Use hmm local instead of 2nd container_of (Jerome)
> ---
> mm/hmm.c | 80
> 1 file changed, 29 insertions(+), 51 deletions(-)
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index cc7c26fda3300e..dc30edad9a8a02 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -40,16 +40,6 @@
> #if IS_ENABLED(CONFIG_HMM_MIRROR)
> static const struct mmu_notifier_ops hmm_mmu_notifier_ops;
>
> -static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
> -{
> - struct hmm *hmm = READ_ONCE(mm->hmm);
> -
> - if (hmm && kref_get_unless_zero(&hmm->kref))
> - return hmm;
> -
> - return NULL;
> -}
> -
> /**
> * hmm_get_or_create - register HMM against an mm (HMM internal)
> *
> @@ -64,11 +54,20 @@ static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
> */
> static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> {
> - struct hmm *hmm = mm_get_hmm(mm);
> - bool cleanup = false;
> + struct hmm *hmm;
>
> - if (hmm)
> - return hmm;
> + lockdep_assert_held_exclusive(&mm->mmap_sem);
> +
> + if (mm->hmm) {
> + if (kref_get_unless_zero(&mm->hmm->kref))
> + return mm->hmm;
> + /*
> + * The hmm is being freed by some other CPU and is pending a
> + * RCU grace period, but this CPU can NULL now it since we
> + * have the mmap_sem.
> + */
> + mm->hmm = NULL;
> + }
>
> hmm = kmalloc(sizeof(*hmm), GFP_KERNEL);
> if (!hmm)
> @@ -83,57 +82,36 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> hmm->notifiers = 0;
> hmm->dead = false;
> hmm->mm = mm;
> - mmgrab(hmm->mm);
> -
> - spin_lock(&mm->page_table_lock);
> - if (!mm->hmm)
> - mm->hmm = hmm;
> - else
> - cleanup = true;
> - spin_unlock(&mm->page_table_lock);
>
> - if (cleanup)
> - goto error;
> -
> - /*
> - * We should only get here if hold the mmap_sem in write mode ie on
> - * registration of first mirror through hmm_mirror_register()
> - */
> hmm->mmu_notifier.ops = &hmm_mmu_notifier_ops;
> - if (__mmu_notifier_register(&hmm->mmu_notifier, mm))
> - goto error_mm;
> + if (__mmu_notifier_register(&hmm->mmu_notifier, mm)) {
> + kfree(hmm);
> + return NULL;
> + }
>
> + mmgrab(hmm->mm);
> + mm->hmm = hmm;
> return hmm;
> -
> -error_mm:
> - spin_lock(&mm->page_table_lock);
> - if (mm->hmm == hmm)
> - mm->hmm = NULL;
> - spin_unlock(&mm->page_table_lock);
> -error:
> - mmdrop(hmm->mm);
> - kfree(hmm);
> - return NULL;
> }
>
> static void hmm_free_rcu(struct rcu_head *rcu)
> {
> - kfree(container_of(rcu, struct hmm, rcu));
> + struct hmm *hmm = container_of(rcu, struct hmm, rcu);
> +
> + down_write(&hmm->mm->mmap_sem);
> + if (hmm->mm->hmm == hmm)
> + hmm->mm->hmm = NULL;
> + up_write(&hmm->mm->mmap_sem);
> + mmdrop(hmm->mm);
> +
> + kfree(hmm);
> }
>
> static void hmm_free(struct kref *kref)
> {
> struct hmm *hmm = container_of(kref, struct hmm, kref);
> - struct mm_struct *mm = hmm->mm;
> -
> - mmu_notifier_unregister_no_release(&hmm->mmu_notifier, mm);
>
> - spin_lock(&mm->page_table_lock);
> - if (mm->hmm == hmm)
> - mm->hmm = NULL;
> - spin_unlock(&mm->page_table_lock);
> -
> - mmdrop(hmm->mm);
> + mmu_notifier_unregister_no_release(&hmm->mmu_notifier, hmm->mm);
> mmu_notifier_call_srcu(&hmm->rcu, hmm_free_rcu);
> }
>
>
Yes.
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 06/11] mm/hmm: Hold on to the mmget for the lifetime of the range
From: Jason Gunthorpe
Range functions like hmm_range_snapshot() and hmm_range_fault() call
find_vma, which requires hodling the mmget() and the mmap_sem for the mm.
Make this simpler for the callers by holding the mmget() inside the range
for the lifetime of the range. Other functions that accept a range should
only be called if the range is registered.
This has the side effect of directly preventing hmm_release() from
happening while a range is registered. That means range->dead cannot be
false during the lifetime of the range, so remove dead and
hmm_mirror_mm_is_alive() entirely.
Signed-off-by: Jason Gunthorpe
---
v2:
- Use Jerome's idea of just holding the mmget() for the range lifetime,
rework the patch to use that as as simplification to remove dead in
one step
---
include/linux/hmm.h | 26 --
mm/hmm.c| 28 ++--
2 files changed, 10 insertions(+), 44 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 2ab35b40992b24..0e20566802967a 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -91,7 +91,6 @@
* @mirrors_sem: read/write semaphore protecting the mirrors list
* @wq: wait queue for user waiting on a range invalidation
* @notifiers: count of active mmu notifiers
- * @dead: is the mm dead ?
*/
struct hmm {
struct mm_struct*mm;
@@ -104,7 +103,6 @@ struct hmm {
wait_queue_head_t wq;
struct rcu_head rcu;
longnotifiers;
- booldead;
};
/*
@@ -469,30 +467,6 @@ struct hmm_mirror {
int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm);
void hmm_mirror_unregister(struct hmm_mirror *mirror);
-/*
- * hmm_mirror_mm_is_alive() - test if mm is still alive
- * @mirror: the HMM mm mirror for which we want to lock the mmap_sem
- * Return: false if the mm is dead, true otherwise
- *
- * This is an optimization, it will not always accurately return false if the
- * mm is dead; i.e., there can be false negatives (process is being killed but
- * HMM is not yet informed of that). It is only intended to be used to optimize
- * out cases where the driver is about to do something time consuming and it
- * would be better to skip it if the mm is dead.
- */
-static inline bool hmm_mirror_mm_is_alive(struct hmm_mirror *mirror)
-{
- struct mm_struct *mm;
-
- if (!mirror || !mirror->hmm)
- return false;
- mm = READ_ONCE(mirror->hmm->mm);
- if (mirror->hmm->dead || !mm)
- return false;
-
- return true;
-}
-
/*
* Please see Documentation/vm/hmm.rst for how to use the range API.
*/
diff --git a/mm/hmm.c b/mm/hmm.c
index dc30edad9a8a02..f67ba32983d9f1 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -80,7 +80,6 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
mutex_init(&hmm->lock);
kref_init(&hmm->kref);
hmm->notifiers = 0;
- hmm->dead = false;
hmm->mm = mm;
hmm->mmu_notifier.ops = &hmm_mmu_notifier_ops;
@@ -124,20 +123,17 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
{
struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
- struct hmm_range *range;
/* hmm is in progress to free */
if (!kref_get_unless_zero(&hmm->kref))
return;
- /* Report this HMM as dying. */
- hmm->dead = true;
-
- /* Wake-up everyone waiting on any range. */
mutex_lock(&hmm->lock);
- list_for_each_entry(range, &hmm->ranges, list)
- range->valid = false;
- wake_up_all(&hmm->wq);
+ /*
+* Since hmm_range_register() holds the mmget() lock hmm_release() is
+* prevented as long as a range exists.
+*/
+ WARN_ON(!list_empty(&hmm->ranges));
mutex_unlock(&hmm->lock);
down_write(&hmm->mirrors_sem);
@@ -909,8 +905,8 @@ int hmm_range_register(struct hmm_range *range,
range->start = start;
range->end = end;
- /* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead)
+ /* Prevent hmm_release() from running while the range is valid */
+ if (!mmget_not_zero(hmm->mm))
return -EFAULT;
range->hmm = hmm;
@@ -955,6 +951,7 @@ void hmm_range_unregister(struct hmm_range *range)
/* Drop reference taken by hmm_range_register() */
range->valid = false;
+ mmput(hmm->mm);
hmm_put(hmm);
range->hmm = NULL;
}
@@ -982,10 +979,7 @@ long hmm_range_snapshot(struct hmm_range *range)
struct vm_area_struct *vma;
struct mm_walk mm_walk;
- /* Check if hmm_mm_destroy() was call. */
- if (hmm->mm == NULL || hmm->dead)
- return -EFAULT;
-
+ lockdep_assert_held(&hmm->mm->mmap_sem);
do {
/* If range is no longer valid force retry. */
Re: [PATCH v2 hmm 10/11] mm/hmm: Do not use list*_rcu() for hmm->ranges
On 6/6/19 11:44 AM, Jason Gunthorpe wrote: > From: Jason Gunthorpe > > This list is always read and written while holding hmm->lock so there is > no need for the confusing _rcu annotations. > > Signed-off-by: Jason Gunthorpe > Reviewed-by: Jérôme Glisse > --- > mm/hmm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/mm/hmm.c b/mm/hmm.c > index c2fecb3ecb11e1..709d138dd49027 100644 > --- a/mm/hmm.c > +++ b/mm/hmm.c > @@ -911,7 +911,7 @@ int hmm_range_register(struct hmm_range *range, > mutex_lock(&hmm->lock); > > range->hmm = hmm; > - list_add_rcu(&range->list, &hmm->ranges); > + list_add(&range->list, &hmm->ranges); > > /* >* If there are any concurrent notifiers we have to wait for them for > @@ -941,7 +941,7 @@ void hmm_range_unregister(struct hmm_range *range) > return; > > mutex_lock(&hmm->lock); > - list_del_rcu(&range->list); > + list_del(&range->list); > mutex_unlock(&hmm->lock); > > /* Drop reference taken by hmm_range_register() */ > Good point. I'd overlooked this many times. Reviewed-by: John Hubbard thanks, -- John Hubbard NVIDIA ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 11/11] mm/hmm: Remove confusing comment and logic from hmm_release
From: Jason Gunthorpe
hmm_release() is called exactly once per hmm. ops->release() cannot
accidentally trigger any action that would recurse back onto
hmm->mirrors_sem.
This fixes a use after-free race of the form:
CPU0 CPU1
hmm_release()
up_write(&hmm->mirrors_sem);
hmm_mirror_unregister(mirror)
down_write(&hmm->mirrors_sem);
up_write(&hmm->mirrors_sem);
kfree(mirror)
mirror->ops->release(mirror)
The only user we have today for ops->release is an empty function, so this
is unambiguously safe.
As a consequence of plugging this race drivers are not allowed to
register/unregister mirrors from within a release op.
Signed-off-by: Jason Gunthorpe
---
mm/hmm.c | 28 +---
1 file changed, 9 insertions(+), 19 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index 709d138dd49027..3a45dd3d778248 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -136,26 +136,16 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
WARN_ON(!list_empty(&hmm->ranges));
mutex_unlock(&hmm->lock);
- down_write(&hmm->mirrors_sem);
- mirror = list_first_entry_or_null(&hmm->mirrors, struct hmm_mirror,
- list);
- while (mirror) {
- list_del_init(&mirror->list);
- if (mirror->ops->release) {
- /*
-* Drop mirrors_sem so the release callback can wait
-* on any pending work that might itself trigger a
-* mmu_notifier callback and thus would deadlock with
-* us.
-*/
- up_write(&hmm->mirrors_sem);
+ down_read(&hmm->mirrors_sem);
+ list_for_each_entry(mirror, &hmm->mirrors, list) {
+ /*
+* Note: The driver is not allowed to trigger
+* hmm_mirror_unregister() from this thread.
+*/
+ if (mirror->ops->release)
mirror->ops->release(mirror);
- down_write(&hmm->mirrors_sem);
- }
- mirror = list_first_entry_or_null(&hmm->mirrors,
- struct hmm_mirror, list);
}
- up_write(&hmm->mirrors_sem);
+ up_read(&hmm->mirrors_sem);
hmm_put(hmm);
}
@@ -287,7 +277,7 @@ void hmm_mirror_unregister(struct hmm_mirror *mirror)
struct hmm *hmm = mirror->hmm;
down_write(&hmm->mirrors_sem);
- list_del_init(&mirror->list);
+ list_del(&mirror->list);
up_write(&hmm->mirrors_sem);
hmm_put(hmm);
memset(&mirror->hmm, POISON_INUSE, sizeof(mirror->hmm));
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 06/15] drm/bridge: tc358767: Simplify AUX data read
Simplify AUX data read by removing index arithmetic and shifting with
a helper function that does two things:
1. Fetch data from up to 4 32-bit registers from the chip
2. Copy read data into user provided array.
Signed-off-by: Andrey Smirnov
Reviewed-by: Andrzej Hajda
Cc: Andrzej Hajda
Cc: Laurent Pinchart
Cc: Tomi Valkeinen
Cc: Andrey Gusakov
Cc: Philipp Zabel
Cc: Cory Tusar
Cc: Chris Healy
Cc: Lucas Stach
Cc: dri-devel@lists.freedesktop.org
Cc: linux-ker...@vger.kernel.org
---
drivers/gpu/drm/bridge/tc358767.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/tc358767.c
b/drivers/gpu/drm/bridge/tc358767.c
index 7b15caec2ce5..7152b44db8a3 100644
--- a/drivers/gpu/drm/bridge/tc358767.c
+++ b/drivers/gpu/drm/bridge/tc358767.c
@@ -321,6 +321,20 @@ static int tc_aux_get_status(struct tc_data *tc, u8 *reply)
return 0;
}
+static int tc_aux_read_data(struct tc_data *tc, void *data, size_t size)
+{
+ u32 auxrdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)];
+ int ret, count = ALIGN(size, sizeof(u32));
+
+ ret = regmap_raw_read(tc->regmap, DP0_AUXRDATA(0), auxrdata, count);
+ if (ret)
+ return ret;
+
+ memcpy(data, auxrdata, size);
+
+ return size;
+}
+
static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
struct drm_dp_aux_msg *msg)
{
@@ -379,19 +393,10 @@ static ssize_t tc_aux_transfer(struct drm_dp_aux *aux,
if (ret)
return ret;
- if (request == DP_AUX_I2C_READ || request == DP_AUX_NATIVE_READ) {
- /* Read data */
- while (i < size) {
- if ((i % 4) == 0) {
- ret = regmap_read(tc->regmap,
- DP0_AUXRDATA(i >> 2), &tmp);
- if (ret)
- return ret;
- }
- buf[i] = tmp & 0xff;
- tmp = tmp >> 8;
- i++;
- }
+ switch (request) {
+ case DP_AUX_NATIVE_READ:
+ case DP_AUX_I2C_READ:
+ return tc_aux_read_data(tc, msg->buffer, size);
}
return size;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] Two bug-fixes for HMM
On Fri, May 10, 2019 at 07:53:21PM +, Kuehling, Felix wrote: > These problems were found in AMD-internal testing as we're working on > adopting HMM. They are rebased against glisse/hmm-5.2-v3. We'd like to get > them applied to a mainline Linux kernel as well as drm-next and > amd-staging-drm-next sooner rather than later. > > Currently the HMM in amd-staging-drm-next is quite far behind hmm-5.2-v3, > but the driver changes for HMM are expected to land in 5.2 and will need to > be rebased on those HMM changes. > > I'd like to work out a flow between Jerome, Dave, Alex and myself that > allows us to test the latest version of HMM on amd-staging-drm-next so > that ideally everything comes together in master without much need for > rebasing and retesting. I think we have that now, I'm running a hmm.git that is clean and can be used for merging into DRM related trees (and RDMA). I've commited to send this tree to Linus at the start of the merge window. See here: https://lore.kernel.org/lkml/20190524124455.gb16...@ziepe.ca/ The tree is here: https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/log/?h=hmm However please consult with me before making a merge commit to be co-ordinated. Thanks I see in this thread that AMDGPU missed 5.2 beacause of the co-ordination problems this tree is intended to solve, so I'm very hopeful this will help your work move into 5.3! > Maybe having Jerome's latest HMM changes in drm-next. However, that may > create dependencies where Jerome and Dave need to coordinate their pull- > requests for master. > > Felix Kuehling (1): > mm/hmm: Only set FAULT_FLAG_ALLOW_RETRY for non-blocking > > Philip Yang (1): > mm/hmm: support automatic NUMA balancing I've applied both of these patches with Jerome's Reviewed-by to hmm.git and added the missed Signed-off-by If you test and confirm I think this branch would be ready for merging toward the AMD tree. Regards, Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 07/11] mm/hmm: Use lockdep instead of comments
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> So we can check locking at runtime.
>
> Signed-off-by: Jason Gunthorpe
> Reviewed-by: Jérôme Glisse
> ---
> v2
> - Fix missing & in lockdeps (Jason)
> ---
> mm/hmm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index f67ba32983d9f1..c702cd72651b53 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -254,11 +254,11 @@ static const struct mmu_notifier_ops
> hmm_mmu_notifier_ops = {
> *
> * To start mirroring a process address space, the device driver must
> register
> * an HMM mirror struct.
> - *
> - * THE mm->mmap_sem MUST BE HELD IN WRITE MODE !
> */
> int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm)
> {
> + lockdep_assert_held_exclusive(&mm->mmap_sem);
> +
> /* Sanity check */
> if (!mm || !mirror || !mirror->ops)
> return -EINVAL;
>
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
drm/imx: 'flip_done timed out' on i.MX6D+ and kernel v4.19.48
Hi, We observed the following error on an i.MX6D+ CPU during start of X. As a result, the screen goes blank. --- [ 3599.200886] [drm:drm_atomic_helper_wait_for_flip_done] *ERROR* [CRTC:30:crtc-0] flip_done timed out [ 3610.080885] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CRTC:30:crtc-0] flip_done timed out [ 3620.320849] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [CONNECTOR:45:LVDS-1] flip_done timed out [ 3630.560864] [drm:drm_atomic_helper_wait_for_dependencies] *ERROR* [PLANE:28:plane-0] flip_done timed out --- The error happens quite randomly but it can be reliably reproduced by repeatedly restarting X . On our system, the error occurs somewhere between 80 and 300 restarts of X. We first observed the issue using kernel 4.14.123. Updating to 4.19.48 did not solve the issue unfortunately. Cheers, Hannes Moesl ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
From: Jason Gunthorpe
The wait_event_timeout macro already tests the condition as its first
action, so there is no reason to open code another version of this, all
that does is skip the might_sleep() debugging in common cases, which is
not helpful.
Further, based on prior patches, we can no simplify the required condition
test:
- If range is valid memory then so is range->hmm
- If hmm_release() has run then range->valid is set to false
at the same time as dead, so no reason to check both.
- A valid hmm has a valid hmm->mm.
Also, add the READ_ONCE for range->valid as there is no lock held here.
Signed-off-by: Jason Gunthorpe
Reviewed-by: Jérôme Glisse
---
include/linux/hmm.h | 12 ++--
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 4ee3acabe5ed22..2ab35b40992b24 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -218,17 +218,9 @@ static inline unsigned long hmm_range_page_size(const
struct hmm_range *range)
static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
unsigned long timeout)
{
- /* Check if mm is dead ? */
- if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
- range->valid = false;
- return false;
- }
- if (range->valid)
- return true;
- wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
+ wait_event_timeout(range->hmm->wq, range->valid,
msecs_to_jiffies(timeout));
- /* Return current valid status just in case we get lucky */
- return range->valid;
+ return READ_ONCE(range->valid);
}
/*
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 02/11] mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> Ralph observes that hmm_range_register() can only be called by a driver
> while a mirror is registered. Make this clear in the API by passing in the
> mirror structure as a parameter.
>
> This also simplifies understanding the lifetime model for struct hmm, as
> the hmm pointer must be valid as part of a registered mirror so all we
> need in hmm_register_range() is a simple kref_get.
>
> Suggested-by: Ralph Campbell
> Signed-off-by: Jason Gunthorpe
> ---
> v2
> - Include the oneline patch to nouveau_svm.c
> ---
> drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +-
> include/linux/hmm.h | 7 ---
> mm/hmm.c | 15 ++-
> 3 files changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_svm.c
> b/drivers/gpu/drm/nouveau/nouveau_svm.c
> index 93ed43c413f0bb..8c92374afcf227 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_svm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_svm.c
> @@ -649,7 +649,7 @@ nouveau_svm_fault(struct nvif_notify *notify)
> range.values = nouveau_svm_pfn_values;
> range.pfn_shift = NVIF_VMM_PFNMAP_V0_ADDR_SHIFT;
> again:
> - ret = hmm_vma_fault(&range, true);
> + ret = hmm_vma_fault(&svmm->mirror, &range, true);
> if (ret == 0) {
> mutex_lock(&svmm->mutex);
> if (!hmm_vma_range_done(&range)) {
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 688c5ca7068795..2d519797cb134a 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -505,7 +505,7 @@ static inline bool hmm_mirror_mm_is_alive(struct
> hmm_mirror *mirror)
> * Please see Documentation/vm/hmm.rst for how to use the range API.
> */
> int hmm_range_register(struct hmm_range *range,
> -struct mm_struct *mm,
> +struct hmm_mirror *mirror,
> unsigned long start,
> unsigned long end,
> unsigned page_shift);
> @@ -541,7 +541,8 @@ static inline bool hmm_vma_range_done(struct hmm_range
> *range)
> }
>
> /* This is a temporary helper to avoid merge conflict between trees. */
> -static inline int hmm_vma_fault(struct hmm_range *range, bool block)
> +static inline int hmm_vma_fault(struct hmm_mirror *mirror,
> + struct hmm_range *range, bool block)
> {
> long ret;
>
> @@ -554,7 +555,7 @@ static inline int hmm_vma_fault(struct hmm_range *range,
> bool block)
> range->default_flags = 0;
> range->pfn_flags_mask = -1UL;
>
> - ret = hmm_range_register(range, range->vma->vm_mm,
> + ret = hmm_range_register(range, mirror,
>range->start, range->end,
>PAGE_SHIFT);
> if (ret)
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 547002f56a163d..8796447299023c 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -925,13 +925,13 @@ static void hmm_pfns_clear(struct hmm_range *range,
> * Track updates to the CPU page table see include/linux/hmm.h
> */
> int hmm_range_register(struct hmm_range *range,
> -struct mm_struct *mm,
> +struct hmm_mirror *mirror,
> unsigned long start,
> unsigned long end,
> unsigned page_shift)
> {
> unsigned long mask = ((1UL << page_shift) - 1UL);
> - struct hmm *hmm;
> + struct hmm *hmm = mirror->hmm;
>
> range->valid = false;
> range->hmm = NULL;
> @@ -945,15 +945,12 @@ int hmm_range_register(struct hmm_range *range,
> range->start = start;
> range->end = end;
>
> - hmm = hmm_get_or_create(mm);
> - if (!hmm)
> - return -EFAULT;
> -
> /* Check if hmm_mm_destroy() was call. */
> - if (hmm->mm == NULL || hmm->dead) {
> - hmm_put(hmm);
> + if (hmm->mm == NULL || hmm->dead)
> return -EFAULT;
> - }
> +
> + range->hmm = hmm;
> + kref_get(&hmm->kref);
>
> /* Initialize range to track CPU page table updates. */
> mutex_lock(&hmm->lock);
>
I'm not a qualified Nouveau reviewer, but this looks obviously correct,
so:
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 10/11] mm/hmm: Do not use list*_rcu() for hmm->ranges
From: Jason Gunthorpe This list is always read and written while holding hmm->lock so there is no need for the confusing _rcu annotations. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse --- mm/hmm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index c2fecb3ecb11e1..709d138dd49027 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -911,7 +911,7 @@ int hmm_range_register(struct hmm_range *range, mutex_lock(&hmm->lock); range->hmm = hmm; - list_add_rcu(&range->list, &hmm->ranges); + list_add(&range->list, &hmm->ranges); /* * If there are any concurrent notifiers we have to wait for them for @@ -941,7 +941,7 @@ void hmm_range_unregister(struct hmm_range *range) return; mutex_lock(&hmm->lock); - list_del_rcu(&range->list); + list_del(&range->list); mutex_unlock(&hmm->lock); /* Drop reference taken by hmm_range_register() */ -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 15/15] drm/bridge: tc358767: Replace magic number in tc_main_link_enable()
We don't need 8 byte array, DP_LINK_STATUS_SIZE (6) should be enough. This also gets rid of a magic number as a bonus. Signed-off-by: Andrey Smirnov Cc: Andrzej Hajda Cc: Laurent Pinchart Cc: Tomi Valkeinen Cc: Andrey Gusakov Cc: Philipp Zabel Cc: Cory Tusar Cc: Chris Healy Cc: Lucas Stach Cc: dri-devel@lists.freedesktop.org Cc: linux-ker...@vger.kernel.org --- drivers/gpu/drm/bridge/tc358767.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 4a245144aa83..05c5fab011f8 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -874,7 +874,7 @@ static int tc_main_link_enable(struct tc_data *tc) u32 dp_phy_ctrl; u32 value; int ret; - u8 tmp[8]; + u8 tmp[DP_LINK_STATUS_SIZE]; dev_dbg(tc->dev, "link enable\n"); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 09/11] mm/hmm: Poison hmm_range during unregister
From: Jason Gunthorpe Trying to misuse a range outside its lifetime is a kernel bug. Use WARN_ON and poison bytes to detect this condition. Signed-off-by: Jason Gunthorpe Reviewed-by: Jérôme Glisse --- v2 - Keep range start/end valid after unregistration (Jerome) --- mm/hmm.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/mm/hmm.c b/mm/hmm.c index 6802de7080d172..c2fecb3ecb11e1 100644 --- a/mm/hmm.c +++ b/mm/hmm.c @@ -937,7 +937,7 @@ void hmm_range_unregister(struct hmm_range *range) struct hmm *hmm = range->hmm; /* Sanity check this really should not happen. */ - if (hmm == NULL || range->end <= range->start) + if (WARN_ON(range->end <= range->start)) return; mutex_lock(&hmm->lock); @@ -948,7 +948,10 @@ void hmm_range_unregister(struct hmm_range *range) range->valid = false; mmput(hmm->mm); hmm_put(hmm); - range->hmm = NULL; + + /* The range is now invalid, leave it poisoned. */ + range->valid = false; + memset(&range->hmm, POISON_INUSE, sizeof(range->hmm)); } EXPORT_SYMBOL(hmm_range_unregister); -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 05/11] mm/hmm: Remove duplicate condition test before wait_event_timeout
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> The wait_event_timeout macro already tests the condition as its first
> action, so there is no reason to open code another version of this, all
> that does is skip the might_sleep() debugging in common cases, which is
> not helpful.
>
> Further, based on prior patches, we can no simplify the required condition
"now simplify"
> test:
> - If range is valid memory then so is range->hmm
> - If hmm_release() has run then range->valid is set to false
>at the same time as dead, so no reason to check both.
> - A valid hmm has a valid hmm->mm.
>
> Also, add the READ_ONCE for range->valid as there is no lock held here.
>
> Signed-off-by: Jason Gunthorpe
> Reviewed-by: Jérôme Glisse
> ---
> include/linux/hmm.h | 12 ++--
> 1 file changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 4ee3acabe5ed22..2ab35b40992b24 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -218,17 +218,9 @@ static inline unsigned long hmm_range_page_size(const
> struct hmm_range *range)
> static inline bool hmm_range_wait_until_valid(struct hmm_range *range,
> unsigned long timeout)
> {
> - /* Check if mm is dead ? */
> - if (range->hmm == NULL || range->hmm->dead || range->hmm->mm == NULL) {
> - range->valid = false;
> - return false;
> - }
> - if (range->valid)
> - return true;
> - wait_event_timeout(range->hmm->wq, range->valid || range->hmm->dead,
> + wait_event_timeout(range->hmm->wq, range->valid,
> msecs_to_jiffies(timeout));
> - /* Return current valid status just in case we get lucky */
> - return range->valid;
> + return READ_ONCE(range->valid);
Just to ensure that I actually understand the model: I'm assuming that the
READ_ONCE is there solely to ensure that range->valid is read *after* the
wait_event_timeout() returns. Is that correct?
> }
>
> /*
>
In any case, it looks good, so:
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
From: Jason Gunthorpe
mmu_notifier_unregister_no_release() is not a fence and the mmu_notifier
system will continue to reference hmm->mn until the srcu grace period
expires.
Resulting in use after free races like this:
CPU0 CPU1
__mmu_notifier_invalidate_range_start()
srcu_read_lock
hlist_for_each ()
// mn == hmm->mn
hmm_mirror_unregister()
hmm_put()
hmm_free()
mmu_notifier_unregister_no_release()
hlist_del_init_rcu(hmm-mn->list)
mn->ops->invalidate_range_start(mn, range);
mm_get_hmm()
mm->hmm = NULL;
kfree(hmm)
mutex_lock(&hmm->lock);
Use SRCU to kfree the hmm memory so that the notifiers can rely on hmm
existing. Get the now-safe hmm struct through container_of and directly
check kref_get_unless_zero to lock it against free.
Signed-off-by: Jason Gunthorpe
---
v2:
- Spell 'free' properly (Jerome/Ralph)
---
include/linux/hmm.h | 1 +
mm/hmm.c| 25 +++--
2 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/include/linux/hmm.h b/include/linux/hmm.h
index 092f0234bfe917..688c5ca7068795 100644
--- a/include/linux/hmm.h
+++ b/include/linux/hmm.h
@@ -102,6 +102,7 @@ struct hmm {
struct mmu_notifier mmu_notifier;
struct rw_semaphore mirrors_sem;
wait_queue_head_t wq;
+ struct rcu_head rcu;
longnotifiers;
booldead;
};
diff --git a/mm/hmm.c b/mm/hmm.c
index 8e7403f081f44a..547002f56a163d 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -113,6 +113,11 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
return NULL;
}
+static void hmm_free_rcu(struct rcu_head *rcu)
+{
+ kfree(container_of(rcu, struct hmm, rcu));
+}
+
static void hmm_free(struct kref *kref)
{
struct hmm *hmm = container_of(kref, struct hmm, kref);
@@ -125,7 +130,7 @@ static void hmm_free(struct kref *kref)
mm->hmm = NULL;
spin_unlock(&mm->page_table_lock);
- kfree(hmm);
+ mmu_notifier_call_srcu(&hmm->rcu, hmm_free_rcu);
}
static inline void hmm_put(struct hmm *hmm)
@@ -153,10 +158,14 @@ void hmm_mm_destroy(struct mm_struct *mm)
static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
{
- struct hmm *hmm = mm_get_hmm(mm);
+ struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
struct hmm_range *range;
+ /* hmm is in progress to free */
+ if (!kref_get_unless_zero(&hmm->kref))
+ return;
+
/* Report this HMM as dying. */
hmm->dead = true;
@@ -194,13 +203,15 @@ static void hmm_release(struct mmu_notifier *mn, struct
mm_struct *mm)
static int hmm_invalidate_range_start(struct mmu_notifier *mn,
const struct mmu_notifier_range *nrange)
{
- struct hmm *hmm = mm_get_hmm(nrange->mm);
+ struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
struct hmm_mirror *mirror;
struct hmm_update update;
struct hmm_range *range;
int ret = 0;
- VM_BUG_ON(!hmm);
+ /* hmm is in progress to free */
+ if (!kref_get_unless_zero(&hmm->kref))
+ return 0;
update.start = nrange->start;
update.end = nrange->end;
@@ -245,9 +256,11 @@ static int hmm_invalidate_range_start(struct mmu_notifier
*mn,
static void hmm_invalidate_range_end(struct mmu_notifier *mn,
const struct mmu_notifier_range *nrange)
{
- struct hmm *hmm = mm_get_hmm(nrange->mm);
+ struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
- VM_BUG_ON(!hmm);
+ /* hmm is in progress to free */
+ if (!kref_get_unless_zero(&hmm->kref))
+ return;
mutex_lock(&hmm->lock);
hmm->notifiers--;
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 01/11] mm/hmm: fix use after free with struct hmm in the mmu notifiers
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
...
> diff --git a/mm/hmm.c b/mm/hmm.c
> index 8e7403f081f44a..547002f56a163d 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
...
> @@ -125,7 +130,7 @@ static void hmm_free(struct kref *kref)
> mm->hmm = NULL;
> spin_unlock(&mm->page_table_lock);
>
> - kfree(hmm);
> + mmu_notifier_call_srcu(&hmm->rcu, hmm_free_rcu);
It occurred to me to wonder if it is best to use the MMU notifier's
instance of srcu, instead of creating a separate instance for HMM.
But this really does seem appropriate, since we are after all using
this to synchronize with MMU notifier callbacks. So, fine.
> }
>
> static inline void hmm_put(struct hmm *hmm)
> @@ -153,10 +158,14 @@ void hmm_mm_destroy(struct mm_struct *mm)
>
> static void hmm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> {
> - struct hmm *hmm = mm_get_hmm(mm);
> + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> struct hmm_mirror *mirror;
> struct hmm_range *range;
>
> + /* hmm is in progress to free */
Well, sometimes, yes. :)
Maybe this wording is clearer (if we need any comment at all):
/* Bail out if hmm is in the process of being freed */
> + if (!kref_get_unless_zero(&hmm->kref))
> + return;
> +
> /* Report this HMM as dying. */
> hmm->dead = true;
>
> @@ -194,13 +203,15 @@ static void hmm_release(struct mmu_notifier *mn, struct
> mm_struct *mm)
> static int hmm_invalidate_range_start(struct mmu_notifier *mn,
> const struct mmu_notifier_range *nrange)
> {
> - struct hmm *hmm = mm_get_hmm(nrange->mm);
> + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> struct hmm_mirror *mirror;
> struct hmm_update update;
> struct hmm_range *range;
> int ret = 0;
>
> - VM_BUG_ON(!hmm);
> + /* hmm is in progress to free */
Same here.
> + if (!kref_get_unless_zero(&hmm->kref))
> + return 0;
>
> update.start = nrange->start;
> update.end = nrange->end;
> @@ -245,9 +256,11 @@ static int hmm_invalidate_range_start(struct
> mmu_notifier *mn,
> static void hmm_invalidate_range_end(struct mmu_notifier *mn,
> const struct mmu_notifier_range *nrange)
> {
> - struct hmm *hmm = mm_get_hmm(nrange->mm);
> + struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
>
> - VM_BUG_ON(!hmm);
> + /* hmm is in progress to free */
And here.
> + if (!kref_get_unless_zero(&hmm->kref))
> + return;
>
> mutex_lock(&hmm->lock);
> hmm->notifiers--;
>
Elegant fix. Regardless of the above chatter I added, you can add:
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 06/11] mm/hmm: Hold on to the mmget for the lifetime of the range
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> Range functions like hmm_range_snapshot() and hmm_range_fault() call
> find_vma, which requires hodling the mmget() and the mmap_sem for the mm.
>
> Make this simpler for the callers by holding the mmget() inside the range
> for the lifetime of the range. Other functions that accept a range should
> only be called if the range is registered.
>
> This has the side effect of directly preventing hmm_release() from
> happening while a range is registered. That means range->dead cannot be
> false during the lifetime of the range, so remove dead and
> hmm_mirror_mm_is_alive() entirely.
>
> Signed-off-by: Jason Gunthorpe
> ---
> v2:
> - Use Jerome's idea of just holding the mmget() for the range lifetime,
>rework the patch to use that as as simplification to remove dead in
>one step
> ---
> include/linux/hmm.h | 26 --
> mm/hmm.c| 28 ++--
> 2 files changed, 10 insertions(+), 44 deletions(-)
>
> diff --git a/include/linux/hmm.h b/include/linux/hmm.h
> index 2ab35b40992b24..0e20566802967a 100644
> --- a/include/linux/hmm.h
> +++ b/include/linux/hmm.h
> @@ -91,7 +91,6 @@
> * @mirrors_sem: read/write semaphore protecting the mirrors list
> * @wq: wait queue for user waiting on a range invalidation
> * @notifiers: count of active mmu notifiers
> - * @dead: is the mm dead ?
> */
> struct hmm {
> struct mm_struct*mm;
> @@ -104,7 +103,6 @@ struct hmm {
> wait_queue_head_t wq;
> struct rcu_head rcu;
> longnotifiers;
> - booldead;
> };
>
> /*
> @@ -469,30 +467,6 @@ struct hmm_mirror {
> int hmm_mirror_register(struct hmm_mirror *mirror, struct mm_struct *mm);
> void hmm_mirror_unregister(struct hmm_mirror *mirror);
>
> -/*
> - * hmm_mirror_mm_is_alive() - test if mm is still alive
> - * @mirror: the HMM mm mirror for which we want to lock the mmap_sem
> - * Return: false if the mm is dead, true otherwise
> - *
> - * This is an optimization, it will not always accurately return false if the
> - * mm is dead; i.e., there can be false negatives (process is being killed
> but
> - * HMM is not yet informed of that). It is only intended to be used to
> optimize
> - * out cases where the driver is about to do something time consuming and it
> - * would be better to skip it if the mm is dead.
> - */
> -static inline bool hmm_mirror_mm_is_alive(struct hmm_mirror *mirror)
> -{
> - struct mm_struct *mm;
> -
> - if (!mirror || !mirror->hmm)
> - return false;
> - mm = READ_ONCE(mirror->hmm->mm);
> - if (mirror->hmm->dead || !mm)
> - return false;
> -
> - return true;
> -}
> -
> /*
> * Please see Documentation/vm/hmm.rst for how to use the range API.
> */
> diff --git a/mm/hmm.c b/mm/hmm.c
> index dc30edad9a8a02..f67ba32983d9f1 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -80,7 +80,6 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
> mutex_init(&hmm->lock);
> kref_init(&hmm->kref);
> hmm->notifiers = 0;
> - hmm->dead = false;
> hmm->mm = mm;
>
> hmm->mmu_notifier.ops = &hmm_mmu_notifier_ops;
> @@ -124,20 +123,17 @@ static void hmm_release(struct mmu_notifier *mn, struct
> mm_struct *mm)
> {
> struct hmm *hmm = container_of(mn, struct hmm, mmu_notifier);
> struct hmm_mirror *mirror;
> - struct hmm_range *range;
>
> /* hmm is in progress to free */
> if (!kref_get_unless_zero(&hmm->kref))
> return;
>
> - /* Report this HMM as dying. */
> - hmm->dead = true;
> -
> - /* Wake-up everyone waiting on any range. */
> mutex_lock(&hmm->lock);
> - list_for_each_entry(range, &hmm->ranges, list)
> - range->valid = false;
> - wake_up_all(&hmm->wq);
> + /*
> + * Since hmm_range_register() holds the mmget() lock hmm_release() is
> + * prevented as long as a range exists.
> + */
> + WARN_ON(!list_empty(&hmm->ranges));
> mutex_unlock(&hmm->lock);
>
> down_write(&hmm->mirrors_sem);
> @@ -909,8 +905,8 @@ int hmm_range_register(struct hmm_range *range,
> range->start = start;
> range->end = end;
>
> - /* Check if hmm_mm_destroy() was call. */
> - if (hmm->mm == NULL || hmm->dead)
> + /* Prevent hmm_release() from running while the range is valid */
> + if (!mmget_not_zero(hmm->mm))
> return -EFAULT;
>
> range->hmm = hmm;
> @@ -955,6 +951,7 @@ void hmm_range_unregister(struct hmm_range *range)
>
> /* Drop reference taken by hmm_range_register() */
> range->valid = false;
> + mmput(hmm->mm);
> hmm_put(hmm);
> range->hmm = NULL;
> }
> @@ -982,10 +979,7 @@ long hmm_range_snapshot(struct hmm_range *range)
> struct vm_area_struct *vma;
> struct mm_walk mm_walk;
>
> - /* Check if hmm_mm_destroy
[PATCH v2 hmm 00/11] Various revisions from a locking/code review
From: Jason Gunthorpe For hmm.git: This patch series arised out of discussions with Jerome when looking at the ODP changes, particularly informed by use after free races we have already found and fixed in the ODP code (thanks to syzkaller) working with mmu notifiers, and the discussion with Ralph on how to resolve the lifetime model. Overall this brings in a simplified locking scheme and easy to explain lifetime model: If a hmm_range is valid, then the hmm is valid, if a hmm is valid then the mm is allocated memory. If the mm needs to still be alive (ie to lock the mmap_sem, find a vma, etc) then the mmget must be obtained via mmget_not_zero(). Locking of mm->hmm is shifted to use the mmap_sem consistently for all read/write and unlocked accesses are removed. The use unlocked reads on 'hmm->dead' are also eliminated in favour of using standard mmget() locking to prevent the mm from being released. Many of the debugging checks of !range->hmm and !hmm->mm are dropped in favour of poison - which is much clearer as to the lifetime intent. The trailing patches are just some random cleanups I noticed when reviewing this code. This v2 incorporates alot of the good off list changes & feedback Jerome had, and all the on-list comments too. However, now that we have the shared git I have kept the one line change to nouveau_svm.c rather than the compat funtions. I believe we can resolve this merge in the DRM tree now and keep the core mm/hmm.c clean. DRM maintainers, please correct me if I'm wrong. It is on top of hmm.git, and I have a git tree of this series to ease testing here: https://github.com/jgunthorpe/linux/tree/hmm There are still some open locking issues, as I think this remains unaddressed: https://lore.kernel.org/linux-mm/20190527195829.gb18...@mellanox.com/ I'm looking for some more acks, reviews and tests so this can move ahead to hmm.git. Detailed notes on the v2 changes are in each patch. The big changes: - mmget is held so long as the range is registered - the last patch 'Remove confusing comment and logic from hmm_release' is new Thanks everyone, Jason Jason Gunthorpe (11): mm/hmm: fix use after free with struct hmm in the mmu notifiers mm/hmm: Use hmm_mirror not mm as an argument for hmm_range_register mm/hmm: Hold a mmgrab from hmm to mm mm/hmm: Simplify hmm_get_or_create and make it reliable mm/hmm: Remove duplicate condition test before wait_event_timeout mm/hmm: Hold on to the mmget for the lifetime of the range mm/hmm: Use lockdep instead of comments mm/hmm: Remove racy protection against double-unregistration mm/hmm: Poison hmm_range during unregister mm/hmm: Do not use list*_rcu() for hmm->ranges mm/hmm: Remove confusing comment and logic from hmm_release drivers/gpu/drm/nouveau/nouveau_svm.c | 2 +- include/linux/hmm.h | 49 +-- kernel/fork.c | 1 - mm/hmm.c | 204 ++ 4 files changed, 87 insertions(+), 169 deletions(-) -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 06/15] drm/bridge: tc358767: Simplify AUX data read
On Thu, Jun 6, 2019 at 3:59 AM Andrzej Hajda wrote:
>
> On 05.06.2019 09:04, Andrey Smirnov wrote:
> > Simplify AUX data read by removing index arithmetic and shifting with
> > a helper functions that does three things:
> >
> > 1. Fetch data from up to 4 32-bit registers from the chip
> > 2. Optionally fix data endianness (not needed on LE hosts)
> > 3. Copy read data into user provided array.
> >
> > Signed-off-by: Andrey Smirnov
> > Cc: Archit Taneja
> > Cc: Andrzej Hajda
> > Cc: Laurent Pinchart
> > Cc: Tomi Valkeinen
> > Cc: Andrey Gusakov
> > Cc: Philipp Zabel
> > Cc: Cory Tusar
> > Cc: Chris Healy
> > Cc: Lucas Stach
> > Cc: dri-devel@lists.freedesktop.org
> > Cc: linux-ker...@vger.kernel.org
> > ---
> > drivers/gpu/drm/bridge/tc358767.c | 40 +--
> > 1 file changed, 27 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/tc358767.c
> > b/drivers/gpu/drm/bridge/tc358767.c
> > index e197ce0fb166..da47d81e7109 100644
> > --- a/drivers/gpu/drm/bridge/tc358767.c
> > +++ b/drivers/gpu/drm/bridge/tc358767.c
> > @@ -321,6 +321,29 @@ static int tc_aux_get_status(struct tc_data *tc, u8
> > *reply)
> > return 0;
> > }
> >
> > +static int tc_aux_read_data(struct tc_data *tc, void *data, size_t size)
> > +{
> > + u32 auxrdata[DP_AUX_MAX_PAYLOAD_BYTES / sizeof(u32)];
> > + int ret, i, count = DIV_ROUND_UP(size, sizeof(u32));
> > +
> > + ret = regmap_bulk_read(tc->regmap, DP0_AUXRDATA(0), auxrdata, count);
> > + if (ret)
> > + return ret;
> > +
> > + for (i = 0; i < count; i++) {
> > + /*
> > + * Our regmap is configured as LE for register data,
> > + * so we need undo any byte swapping that might have
> > + * happened to preserve original byte order.
> > + */
> > + le32_to_cpus(&auxrdata[i]);
> > + }
> > +
> > + memcpy(data, auxrdata, size);
> > +
> > + return size;
> > +}
> > +
>
>
> Hmm, cannot we just use regmap_raw_read?
I'll give it a try in v4.
Thanks,
Andrey Smirnov
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 hmm 04/11] mm/hmm: Simplify hmm_get_or_create and make it reliable
From: Jason Gunthorpe
As coded this function can false-fail in various racy situations. Make it
reliable by running only under the write side of the mmap_sem and avoiding
the false-failing compare/exchange pattern.
Also make the locking very easy to understand by only ever reading or
writing mm->hmm while holding the write side of the mmap_sem.
Signed-off-by: Jason Gunthorpe
---
v2:
- Fix error unwind of mmgrab (Jerome)
- Use hmm local instead of 2nd container_of (Jerome)
---
mm/hmm.c | 80
1 file changed, 29 insertions(+), 51 deletions(-)
diff --git a/mm/hmm.c b/mm/hmm.c
index cc7c26fda3300e..dc30edad9a8a02 100644
--- a/mm/hmm.c
+++ b/mm/hmm.c
@@ -40,16 +40,6 @@
#if IS_ENABLED(CONFIG_HMM_MIRROR)
static const struct mmu_notifier_ops hmm_mmu_notifier_ops;
-static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
-{
- struct hmm *hmm = READ_ONCE(mm->hmm);
-
- if (hmm && kref_get_unless_zero(&hmm->kref))
- return hmm;
-
- return NULL;
-}
-
/**
* hmm_get_or_create - register HMM against an mm (HMM internal)
*
@@ -64,11 +54,20 @@ static inline struct hmm *mm_get_hmm(struct mm_struct *mm)
*/
static struct hmm *hmm_get_or_create(struct mm_struct *mm)
{
- struct hmm *hmm = mm_get_hmm(mm);
- bool cleanup = false;
+ struct hmm *hmm;
- if (hmm)
- return hmm;
+ lockdep_assert_held_exclusive(&mm->mmap_sem);
+
+ if (mm->hmm) {
+ if (kref_get_unless_zero(&mm->hmm->kref))
+ return mm->hmm;
+ /*
+* The hmm is being freed by some other CPU and is pending a
+* RCU grace period, but this CPU can NULL now it since we
+* have the mmap_sem.
+*/
+ mm->hmm = NULL;
+ }
hmm = kmalloc(sizeof(*hmm), GFP_KERNEL);
if (!hmm)
@@ -83,57 +82,36 @@ static struct hmm *hmm_get_or_create(struct mm_struct *mm)
hmm->notifiers = 0;
hmm->dead = false;
hmm->mm = mm;
- mmgrab(hmm->mm);
-
- spin_lock(&mm->page_table_lock);
- if (!mm->hmm)
- mm->hmm = hmm;
- else
- cleanup = true;
- spin_unlock(&mm->page_table_lock);
- if (cleanup)
- goto error;
-
- /*
-* We should only get here if hold the mmap_sem in write mode ie on
-* registration of first mirror through hmm_mirror_register()
-*/
hmm->mmu_notifier.ops = &hmm_mmu_notifier_ops;
- if (__mmu_notifier_register(&hmm->mmu_notifier, mm))
- goto error_mm;
+ if (__mmu_notifier_register(&hmm->mmu_notifier, mm)) {
+ kfree(hmm);
+ return NULL;
+ }
+ mmgrab(hmm->mm);
+ mm->hmm = hmm;
return hmm;
-
-error_mm:
- spin_lock(&mm->page_table_lock);
- if (mm->hmm == hmm)
- mm->hmm = NULL;
- spin_unlock(&mm->page_table_lock);
-error:
- mmdrop(hmm->mm);
- kfree(hmm);
- return NULL;
}
static void hmm_free_rcu(struct rcu_head *rcu)
{
- kfree(container_of(rcu, struct hmm, rcu));
+ struct hmm *hmm = container_of(rcu, struct hmm, rcu);
+
+ down_write(&hmm->mm->mmap_sem);
+ if (hmm->mm->hmm == hmm)
+ hmm->mm->hmm = NULL;
+ up_write(&hmm->mm->mmap_sem);
+ mmdrop(hmm->mm);
+
+ kfree(hmm);
}
static void hmm_free(struct kref *kref)
{
struct hmm *hmm = container_of(kref, struct hmm, kref);
- struct mm_struct *mm = hmm->mm;
-
- mmu_notifier_unregister_no_release(&hmm->mmu_notifier, mm);
- spin_lock(&mm->page_table_lock);
- if (mm->hmm == hmm)
- mm->hmm = NULL;
- spin_unlock(&mm->page_table_lock);
-
- mmdrop(hmm->mm);
+ mmu_notifier_unregister_no_release(&hmm->mmu_notifier, hmm->mm);
mmu_notifier_call_srcu(&hmm->rcu, hmm_free_rcu);
}
--
2.21.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 hmm 08/11] mm/hmm: Remove racy protection against double-unregistration
On 6/6/19 11:44 AM, Jason Gunthorpe wrote:
> From: Jason Gunthorpe
>
> No other register/unregister kernel API attempts to provide this kind of
> protection as it is inherently racy, so just drop it.
>
> Callers should provide their own protection, it appears nouveau already
> does, but just in case drop a debugging POISON.
>
> Signed-off-by: Jason Gunthorpe
> Reviewed-by: Jérôme Glisse
> ---
> mm/hmm.c | 9 ++---
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
> diff --git a/mm/hmm.c b/mm/hmm.c
> index c702cd72651b53..6802de7080d172 100644
> --- a/mm/hmm.c
> +++ b/mm/hmm.c
> @@ -284,18 +284,13 @@ EXPORT_SYMBOL(hmm_mirror_register);
> */
> void hmm_mirror_unregister(struct hmm_mirror *mirror)
> {
> - struct hmm *hmm = READ_ONCE(mirror->hmm);
> -
> - if (hmm == NULL)
> - return;
> + struct hmm *hmm = mirror->hmm;
>
> down_write(&hmm->mirrors_sem);
> list_del_init(&mirror->list);
> - /* To protect us against double unregister ... */
> - mirror->hmm = NULL;
> up_write(&hmm->mirrors_sem);
> -
> hmm_put(hmm);
> + memset(&mirror->hmm, POISON_INUSE, sizeof(mirror->hmm));
I hadn't thought of POISON_* for these types of cases, it's a
good technique to remember.
I noticed that this is now done outside of the lock, but that
follows directly from your commit description, so that all looks
correct.
> }
> EXPORT_SYMBOL(hmm_mirror_unregister);
>
>
Reviewed-by: John Hubbard
thanks,
--
John Hubbard
NVIDIA
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/bridge: sii902x: re-order conditions to prevent out of bounds read
Am 07.06.2019 09:27, schrieb Dan Carpenter:
> This should check that "i" is within bounds before checking reading from
> the array.
>
> Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
> Signed-off-by: Dan Carpenter
> ---
> drivers/gpu/drm/bridge/sii902x.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c
> b/drivers/gpu/drm/bridge/sii902x.c
> index d6f98d388ac2..6b03616d6bc3 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev,
> void *data,
> if (ret)
> goto out;
>
> - for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
> - i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
> + for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
> + sii902x->audio.i2s_fifo_sequence[i]; i++)
> regmap_write(sii902x->regmap,
>SII902X_TPI_I2S_ENABLE_MAPPING_REG,
>sii902x->audio.i2s_fifo_sequence[i]);
mmmh, i am a big fan of KISS and in this case i would check
sii902x->audio.i2s_fifo_sequence[i]
outside for(). like:
for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++) {
if (!sii902x->audio.i2s_fifo_sequence[i])
break;
regmap_write(sii902x->regmap,
SII902X_TPI_I2S_ENABLE_MAPPING_REG,
sii902x->audio.i2s_fifo_sequence[i]);
}
just my 2 cents,
re,
wh
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/vkms: Add support for writeback
On Thu, Jun 06, 2019 at 07:41:01PM -0300, Rodrigo Siqueira wrote:
> This patch implements the necessary functions to add writeback support
> for vkms. This feature is useful for testing compositors if you don’t
> have hardware with writeback support.
>
> Signed-off-by: Rodrigo Siqueira
> ---
> drivers/gpu/drm/vkms/Makefile | 9 +-
> drivers/gpu/drm/vkms/vkms_crtc.c | 5 +
> drivers/gpu/drm/vkms/vkms_drv.c | 10 ++
> drivers/gpu/drm/vkms/vkms_drv.h | 12 ++
> drivers/gpu/drm/vkms/vkms_output.c| 6 +
> drivers/gpu/drm/vkms/vkms_writeback.c | 165 ++
> 6 files changed, 206 insertions(+), 1 deletion(-)
> create mode 100644 drivers/gpu/drm/vkms/vkms_writeback.c
>
> diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> index 89f09bec7b23..90eb7acd618d 100644
> --- a/drivers/gpu/drm/vkms/Makefile
> +++ b/drivers/gpu/drm/vkms/Makefile
> @@ -1,4 +1,11 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> vkms_crc.o
> +vkms-y := \
> + vkms_drv.o \
> + vkms_plane.o \
> + vkms_output.o \
> + vkms_crtc.o \
> + vkms_gem.o \
> + vkms_crc.o \
> + vkms_writeback.o
>
> obj-$(CONFIG_DRM_VKMS) += vkms.o
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 1bbe099b7db8..ce797e265b1b 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -23,6 +23,11 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> hrtimer *timer)
> if (!ret)
> DRM_ERROR("vkms failure on handling vblank");
>
> + if (output->writeback_status == WB_START) {
> + drm_writeback_signal_completion(&output->wb_connector, 0);
> + output->writeback_status = WB_STOP;
> + }
> +
> if (state && output->crc_enabled) {
> u64 frame = drm_crtc_accurate_vblank_count(crtc);
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.c b/drivers/gpu/drm/vkms/vkms_drv.c
> index 92296bd8f623..d5917d5a45e3 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.c
> +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> @@ -29,6 +29,10 @@ bool enable_cursor;
> module_param_named(enable_cursor, enable_cursor, bool, 0444);
> MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
>
> +int enable_writeback;
> +module_param_named(enable_writeback, enable_writeback, int, 0444);
> +MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector");
> +
> static const struct file_operations vkms_driver_fops = {
> .owner = THIS_MODULE,
> .open = drm_open,
> @@ -123,6 +127,12 @@ static int __init vkms_init(void)
> goto out_fini;
> }
>
> + vkms_device->output.writeback_status = WB_DISABLED;
> + if (enable_writeback) {
> + vkms_device->output.writeback_status = WB_STOP;
> + DRM_INFO("Writeback connector enabled");
> + }
> +
> ret = vkms_modeset_init(vkms_device);
> if (ret)
> goto out_fini;
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index e81073dea154..ca1f9ee63ec8 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -7,6 +7,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #define XRES_MIN20
> @@ -60,14 +61,22 @@ struct vkms_crtc_state {
> u64 frame_end;
> };
>
> +enum wb_status {
> + WB_DISABLED,
> + WB_START,
> + WB_STOP,
> +};
> +
> struct vkms_output {
> struct drm_crtc crtc;
> struct drm_encoder encoder;
> struct drm_connector connector;
> + struct drm_writeback_connector wb_connector;
> struct hrtimer vblank_hrtimer;
> ktime_t period_ns;
> struct drm_pending_vblank_event *event;
> bool crc_enabled;
> + enum wb_status writeback_status;
> /* ordered wq for crc_work */
> struct workqueue_struct *crc_workq;
> /* protects concurrent access to crc_data */
> @@ -141,4 +150,7 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const
> char *source_name,
> size_t *values_cnt);
> void vkms_crc_work_handle(struct work_struct *work);
>
> +/* Writeback */
> +int enable_writeback_connector(struct vkms_device *vkmsdev);
> +
> #endif /* _VKMS_DRV_H_ */
> diff --git a/drivers/gpu/drm/vkms/vkms_output.c
> b/drivers/gpu/drm/vkms/vkms_output.c
> index 1442b447c707..1fc1d4e9585c 100644
> --- a/drivers/gpu/drm/vkms/vkms_output.c
> +++ b/drivers/gpu/drm/vkms/vkms_output.c
> @@ -91,6 +91,12 @@ int vkms_output_init(struct vkms_device *vkmsdev, int
> index)
> goto err_attach;
> }
>
> + if (vkmsdev->output.writeback_status != WB_DISABLED) {
> + ret = enable_writeback_connector(vkmsdev);
> + if (ret)
> + DRM_ERROR("Failed to init writeback connector\n");
> + }
> +
>
Re: [PATCH 2/2] drm/vkms: Add support for writeback
On Fri, Jun 07, 2019 at 09:48:08AM +0200, Daniel Vetter wrote:
> On Thu, Jun 06, 2019 at 07:41:01PM -0300, Rodrigo Siqueira wrote:
> > This patch implements the necessary functions to add writeback support
> > for vkms. This feature is useful for testing compositors if you don’t
> > have hardware with writeback support.
> >
> > Signed-off-by: Rodrigo Siqueira
> > ---
> > drivers/gpu/drm/vkms/Makefile | 9 +-
> > drivers/gpu/drm/vkms/vkms_crtc.c | 5 +
> > drivers/gpu/drm/vkms/vkms_drv.c | 10 ++
> > drivers/gpu/drm/vkms/vkms_drv.h | 12 ++
> > drivers/gpu/drm/vkms/vkms_output.c| 6 +
> > drivers/gpu/drm/vkms/vkms_writeback.c | 165 ++
> > 6 files changed, 206 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/gpu/drm/vkms/vkms_writeback.c
> >
> > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > index 89f09bec7b23..90eb7acd618d 100644
> > --- a/drivers/gpu/drm/vkms/Makefile
> > +++ b/drivers/gpu/drm/vkms/Makefile
> > @@ -1,4 +1,11 @@
> > # SPDX-License-Identifier: GPL-2.0-only
> > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> > vkms_crc.o
> > +vkms-y := \
> > + vkms_drv.o \
> > + vkms_plane.o \
> > + vkms_output.o \
> > + vkms_crtc.o \
> > + vkms_gem.o \
> > + vkms_crc.o \
> > + vkms_writeback.o
> >
> > obj-$(CONFIG_DRM_VKMS) += vkms.o
> > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c
> > b/drivers/gpu/drm/vkms/vkms_crtc.c
> > index 1bbe099b7db8..ce797e265b1b 100644
> > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > @@ -23,6 +23,11 @@ static enum hrtimer_restart vkms_vblank_simulate(struct
> > hrtimer *timer)
> > if (!ret)
> > DRM_ERROR("vkms failure on handling vblank");
> >
> > + if (output->writeback_status == WB_START) {
> > + drm_writeback_signal_completion(&output->wb_connector, 0);
> > + output->writeback_status = WB_STOP;
> > + }
> > +
> > if (state && output->crc_enabled) {
> > u64 frame = drm_crtc_accurate_vblank_count(crtc);
> >
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.c
> > b/drivers/gpu/drm/vkms/vkms_drv.c
> > index 92296bd8f623..d5917d5a45e3 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.c
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.c
> > @@ -29,6 +29,10 @@ bool enable_cursor;
> > module_param_named(enable_cursor, enable_cursor, bool, 0444);
> > MODULE_PARM_DESC(enable_cursor, "Enable/Disable cursor support");
> >
> > +int enable_writeback;
> > +module_param_named(enable_writeback, enable_writeback, int, 0444);
> > +MODULE_PARM_DESC(enable_writeback, "Enable/Disable writeback connector");
> > +
> > static const struct file_operations vkms_driver_fops = {
> > .owner = THIS_MODULE,
> > .open = drm_open,
> > @@ -123,6 +127,12 @@ static int __init vkms_init(void)
> > goto out_fini;
> > }
> >
> > + vkms_device->output.writeback_status = WB_DISABLED;
> > + if (enable_writeback) {
> > + vkms_device->output.writeback_status = WB_STOP;
> > + DRM_INFO("Writeback connector enabled");
> > + }
> > +
> > ret = vkms_modeset_init(vkms_device);
> > if (ret)
> > goto out_fini;
> > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h
> > b/drivers/gpu/drm/vkms/vkms_drv.h
> > index e81073dea154..ca1f9ee63ec8 100644
> > --- a/drivers/gpu/drm/vkms/vkms_drv.h
> > +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> > @@ -7,6 +7,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> >
> > #define XRES_MIN20
> > @@ -60,14 +61,22 @@ struct vkms_crtc_state {
> > u64 frame_end;
> > };
> >
> > +enum wb_status {
> > + WB_DISABLED,
> > + WB_START,
> > + WB_STOP,
> > +};
> > +
> > struct vkms_output {
> > struct drm_crtc crtc;
> > struct drm_encoder encoder;
> > struct drm_connector connector;
> > + struct drm_writeback_connector wb_connector;
> > struct hrtimer vblank_hrtimer;
> > ktime_t period_ns;
> > struct drm_pending_vblank_event *event;
> > bool crc_enabled;
> > + enum wb_status writeback_status;
> > /* ordered wq for crc_work */
> > struct workqueue_struct *crc_workq;
> > /* protects concurrent access to crc_data */
> > @@ -141,4 +150,7 @@ int vkms_verify_crc_source(struct drm_crtc *crtc, const
> > char *source_name,
> >size_t *values_cnt);
> > void vkms_crc_work_handle(struct work_struct *work);
> >
> > +/* Writeback */
> > +int enable_writeback_connector(struct vkms_device *vkmsdev);
> > +
> > #endif /* _VKMS_DRV_H_ */
> > diff --git a/drivers/gpu/drm/vkms/vkms_output.c
> > b/drivers/gpu/drm/vkms/vkms_output.c
> > index 1442b447c707..1fc1d4e9585c 100644
> > --- a/drivers/gpu/drm/vkms/vkms_output.c
> > +++ b/drivers/gpu/drm/vkms/vkms_output.c
> > @@ -91,6 +91,12 @@ int vkms_output_init(struct vkms_device *vkmsdev, int
> > index)
> > goto err_attach;
> >
Re: [PATCH 3/8] drivers: (video|gpu): fix warning same module names
On Thu, Jun 06, 2019 at 11:47:12AM +0200, Anders Roxell wrote: > When building with CONFIG_DRM_MXSFB and CONFIG_FB_MXS enabled as > loadable modules, we see the following warning: > > warning: same module names found: > drivers/video/fbdev/mxsfb.ko > drivers/gpu/drm/mxsfb/mxsfb.ko > > Rework so the names matches the config fragment. > > Signed-off-by: Anders Roxell Reviewed-by: Daniel Vetter I'm assuming Bart will pick this one up for fbdev. -Daniel > --- > drivers/gpu/drm/mxsfb/Makefile | 4 ++-- > drivers/video/fbdev/Makefile | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile > index ff6e358088fa..5d49d7548e66 100644 > --- a/drivers/gpu/drm/mxsfb/Makefile > +++ b/drivers/gpu/drm/mxsfb/Makefile > @@ -1,3 +1,3 @@ > # SPDX-License-Identifier: GPL-2.0-only > -mxsfb-y := mxsfb_drv.o mxsfb_crtc.o mxsfb_out.o > -obj-$(CONFIG_DRM_MXSFB) += mxsfb.o > +drm-mxsfb-y := mxsfb_drv.o mxsfb_crtc.o mxsfb_out.o > +obj-$(CONFIG_DRM_MXSFB) += drm-mxsfb.o > diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile > index 655f2537cac1..7ee967525af2 100644 > --- a/drivers/video/fbdev/Makefile > +++ b/drivers/video/fbdev/Makefile > @@ -131,7 +131,8 @@ obj-$(CONFIG_FB_VGA16)+= vga16fb.o > obj-$(CONFIG_FB_OF) += offb.o > obj-$(CONFIG_FB_MX3) += mx3fb.o > obj-$(CONFIG_FB_DA8XX) += da8xx-fb.o > -obj-$(CONFIG_FB_MXS) += mxsfb.o > +obj-$(CONFIG_FB_MXS) += fb-mxs.o > +fb-mxs-objs:= mxsfb.o > obj-$(CONFIG_FB_SSD1307) += ssd1307fb.o > obj-$(CONFIG_FB_SIMPLE) += simplefb.o > > -- > 2.20.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/bridge: sii902x: re-order conditions to prevent out of bounds read
On Fri, Jun 07, 2019 at 09:39:57AM +0200, walter harms wrote:
>
>
> Am 07.06.2019 09:27, schrieb Dan Carpenter:
> > This should check that "i" is within bounds before checking reading from
> > the array.
> >
> > Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
> > Signed-off-by: Dan Carpenter
> > ---
> > drivers/gpu/drm/bridge/sii902x.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bridge/sii902x.c
> > b/drivers/gpu/drm/bridge/sii902x.c
> > index d6f98d388ac2..6b03616d6bc3 100644
> > --- a/drivers/gpu/drm/bridge/sii902x.c
> > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > @@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev,
> > void *data,
> > if (ret)
> > goto out;
> >
> > - for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
> > -i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
> > + for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
> > + sii902x->audio.i2s_fifo_sequence[i]; i++)
> > regmap_write(sii902x->regmap,
> > SII902X_TPI_I2S_ENABLE_MAPPING_REG,
> > sii902x->audio.i2s_fifo_sequence[i]);
>
>
> mmmh, i am a big fan of KISS and in this case i would check
> sii902x->audio.i2s_fifo_sequence[i]
> outside for(). like:
>
>for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++) {
> if (!sii902x->audio.i2s_fifo_sequence[i])
> break;
That's probably a better way, but either way is acceptable and I left it
how the author wrote it.
regards,
dan carpenter
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #47 from Rui Salvaterra --- (In reply to Marek Olšák from comment #45) > Yeah that looks good. Great! In that case, I'm going to do a proper version and send it. I don't like the fact that I'm doing chip_families[r300screen->caps.family] directly in r300_disk_cache_create, when we already have r300_get_name(struct pipe_screen* pscreen), so I'm going to factor that out to something like r300_get_family_name(struct r300_screen* r300screen). Thanks for the review, Marek! -- 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 2/2] drm/meson: fix G12A primary plane disabling
On 06/06/2019 19:30, Kevin Hilman wrote:
> Neil Armstrong writes:
>
>> The G12A Primary plane was disabled by writing in the OSD1 configuration
>> registers, but this caused the plane blender to stall instead of continuing
>> blended only the overlay plane.
>
> grammar nit: "...instead of continuing to blend only the overlay plane."
Fixed while applying on drm-misc-fixes
>
>> Fix this by disabling the OSD1 plane in the blender registers, and also
>> enabling it back using the same register.
>>
>> Fixes: 490f50c109d1 ("drm/meson: Add G12A support for OSD1 Plane")
>> Signed-off-by: Neil Armstrong
>
> Reviewed-by: Kevin Hilman
>
> As noted elsewhere, this driver is also full of magic constants used in
> register writes which makes reviewing this kind of change for
> correctness that much more difficult, but since that's already been
> pointed out elsewhere, and it's already on your TODO list, it should not
> block this important fix.
Yep, it's the top priority now.
Thanks,
Neil
>
> Kevin
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/meson: fix primary plane disabling
On 06/06/2019 19:25, Kevin Hilman wrote:
> Neil Armstrong writes:
>
>> The primary plane disable logic is flawed, when the primary plane is
>> disabled, it is re-enabled in the vsync irq when another plane is updated.
>>
>> Handle the plane disabling correctly by handling the primary plane
>> enable flag in the primary plane update & disable callbacks.
>>
>> Fixes: 490f50c109d1 ("drm/meson: Add G12A support for OSD1 Plane")
>> Signed-off-by: Neil Armstrong
>
> Reviewed-by: Kevin Hilman
>
Applying to drm-misc-fixes
Thanks,
Neil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/meson: fix G12A HDMI PLL settings for 4K60 1000/1001 variations
On 06/06/2019 18:30, Kevin Hilman wrote:
> Neil Armstrong writes:
>
>> The Amlogic G12A HDMI PLL needs some specific settings to lock with
>> different fractional values for the 5,4GHz mode.
>>
>> Handle the 1000/1001 variation fractional case here to avoid having
>> the PLL in an non lockable state.
>>
>> Fixes: 202b9808f8ed ("drm/meson: Add G12A Video Clock setup")
>> Signed-off-by: Neil Armstrong
>> ---
>> drivers/gpu/drm/meson/meson_vclk.c | 13 +++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_vclk.c
>> b/drivers/gpu/drm/meson/meson_vclk.c
>> index 44250eff8a3f..83fc2fc82001 100644
>> --- a/drivers/gpu/drm/meson/meson_vclk.c
>> +++ b/drivers/gpu/drm/meson/meson_vclk.c
>> @@ -553,8 +553,17 @@ void meson_hdmi_pll_set_params(struct meson_drm *priv,
>> unsigned int m,
>>
>> /* G12A HDMI PLL Needs specific parameters for 5.4GHz */
>> if (m >= 0xf7) {
>> -regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL4, 0xea68dc00);
>> -regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL5, 0x65771290);
>> +if (frac < 0x1) {
>> +regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL4,
>> +0x6a685c00);
>> +regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL5,
>> +0x11551293);
>> +} else {
>> +regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL4,
>> +0xea68dc00);
>> +regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL5,
>> +0x65771290);
>> +}
>> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL6, 0x39272000);
>> regmap_write(priv->hhi, HHI_HDMI_PLL_CNTL7, 0x5554);
>> } else {
>
> Reviewed-by: Kevin Hilman
Applied to drm-misc-fixes
>
> nit: this is continuing with more magic constants, and it would be nice
> to have them converted to #define'd bitfields. But since that isn't a
> new problem in this patch, it's fine to cleanup later.
Yep, it's on our to priority.
Neil
>
> Kevin
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #48 from Rui Salvaterra --- (In reply to Rui Salvaterra from comment #43) > Compiz feels much snappier, indeed. To be 100 % honest, the difference is noticeable, but my laptop has one big problem: as if the Xpress 200M wasn't slow enough, the screen has a resolution of 1680x1050 when the typical resolution, at the time, was 1280x800. That's 72 % more pixels to toss around on a GPU with shared memory. :( -- 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 108381] amdgpu_bo_import and amdgpu_bo_free are not paired which leaks amdgpu_bo
https://bugs.freedesktop.org/show_bug.cgi?id=108381 Pierre-Eric Pelloux-Prayer changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from Pierre-Eric Pelloux-Prayer --- Was fixed by https://gitlab.freedesktop.org/mesa/mesa/commit/82aa07f81fcc5ed696eea16f48cec7e39c3cd3d1 -- 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 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #49 from Richard Thier --- > as if the Xpress 200M wasn't slow enough, the screen has > a resolution of > 1680x1050 when the typical resolution, > at the time, was 1280x800. I don't get it. Is there no supported resolution below that big one? That is quite hard to believe. Mine is 1024x768 when maxed, but sometimes I reverse engineer game binaries that do not let me use 640x480 in case they lag haha. Fill rate really drops a lot when the resolution is high because of the mentioned square-relation of the screen area to pixel count... -- 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 110781] Radeon: heavy r300 performance drop regression between 11.x and 19.x
https://bugs.freedesktop.org/show_bug.cgi?id=110781 --- Comment #50 from Rui Salvaterra --- (In reply to Richard Thier from comment #49) > I don't get it. Is there no supported resolution below that big one? That is > quite hard to believe. Mine is 1024x768 when maxed, but sometimes I reverse > engineer game binaries that do not let me use 640x480 in case they lag haha. Heh… :) I only used Compiz at the native resolution on the LCD. I do have an old 1024x768 monitor with VGA input, I might give it a spin this weekend. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/stm: ltdc: No message if probe
Hi Yannick,
Thank you for your patch
Acked-by: Philippe Cornu
Philippe :-)
On 6/3/19 10:31 AM, Yannick Fertré wrote:
> Print display controller hardware version in debug mode only.
>
> Signed-off-by: Yannick Fertré
> ---
> drivers/gpu/drm/stm/ltdc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index a40870b..2fe6c4a 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -1229,7 +1229,7 @@ int ltdc_load(struct drm_device *ddev)
> goto err;
> }
>
> - DRM_INFO("ltdc hw version 0x%08x - ready\n", ldev->caps.hw_version);
> + DRM_DEBUG_DRIVER("ltdc hw version 0x%08x\n", ldev->caps.hw_version);
>
> /* Add endpoints panels or bridges if any */
> for (i = 0; i < MAX_ENDPOINTS; i++) {
>
[Bug 110659] pageflipping seems to cause jittering on mouse input when running Hitman 2 in Wine/DXVK with amdgpu.dc=1
https://bugs.freedesktop.org/show_bug.cgi?id=110659 --- Comment #21 from tempel.jul...@gmail.com --- I'm open to trying out other patches, e.g. concerning double buffering for the cursor. :) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/2] drm/komeda: Adds SMMU support
Hi Lowry,
On Thu, Jun 06, 2019 at 10:53:05AM +0100, Lowry Li (Arm Technology China) wrote:
> From: "Lowry Li (Arm Technology China)"
>
> Adds iommu_connect and disconnect for SMMU support, and configures
> TBU translation once SMMU has been attached to the display device.
>
> Signed-off-by: Lowry Li (Arm Technology China)
> ---
> .../gpu/drm/arm/display/komeda/d71/d71_component.c | 5 +++
> drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c | 49
> ++
> drivers/gpu/drm/arm/display/komeda/komeda_dev.c| 18
> drivers/gpu/drm/arm/display/komeda/komeda_dev.h| 7
> .../drm/arm/display/komeda/komeda_framebuffer.c| 2 +
> .../drm/arm/display/komeda/komeda_framebuffer.h| 2 +
> 6 files changed, 83 insertions(+)
>
> 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 4e26a80..4a48dd6 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -215,6 +215,8 @@ static void d71_layer_update(struct komeda_component *c,
> 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)
> + ctrl |= L_TBU_EN;
> malidp_write32_mask(reg, BLK_CONTROL, ctrl_mask, ctrl);
> }
>
> @@ -348,6 +350,9 @@ static void d71_wb_layer_update(struct komeda_component
> *c,
> fb->pitches[i] & 0x);
> }
>
> + if (kfb->is_va)
> + ctrl |= LW_TBU_EN;
> +
> malidp_write32(reg, LAYER_FMT, kfb->format_caps->hw_id);
> malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize, st->vsize));
> malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
> diff --git a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> index 8e682c7..1b9e734 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_dev.c
> @@ -517,6 +517,53 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> table->n_formats = ARRAY_SIZE(d71_format_caps_table);
> }
>
> +static int d71_connect_iommu(struct komeda_dev *mdev)
> +{
> + struct d71_dev *d71 = mdev->chip_data;
> + u32 __iomem *reg = d71->gcu_addr;
> + u32 check_bits = (d71->num_pipelines == 2) ?
> + GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> + int i, ret;
> +
> + if (!d71->integrates_tbu)
> + return -1;
> +
> + malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_CONNECT_MODE);
> +
> + ret = dp_wait_cond(has_bits(check_bits, malidp_read32(reg, BLK_STATUS)),
> + 100, 1000, 1000);
> + if (ret <= 0) {
You don't want to return -ETIMEDOUT if dp_wait_cond() returns zero. Maybe
follow the same flow as in d71_disconnect_iommu() ?
> + DRM_ERROR("connect to TCU timeout!\n");
> + malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> + return -ETIMEDOUT;
> + }
> +
> + for (i = 0; i < d71->num_pipelines; i++)
> + malidp_write32_mask(d71->pipes[i]->lpu_addr, LPU_TBU_CONTROL,
> + LPU_TBU_CTRL_TLBPEN, LPU_TBU_CTRL_TLBPEN);
> + return 0;
> +}
> +
> +static int d71_disconnect_iommu(struct komeda_dev *mdev)
> +{
> + struct d71_dev *d71 = mdev->chip_data;
> + u32 __iomem *reg = d71->gcu_addr;
> + u32 check_bits = (d71->num_pipelines == 2) ?
> + GCU_STATUS_TCS0 | GCU_STATUS_TCS1 : GCU_STATUS_TCS0;
> + int ret;
> +
> + malidp_write32_mask(reg, BLK_CONTROL, 0x7, TBU_DISCONNECT_MODE);
> +
> + ret = dp_wait_cond(((malidp_read32(reg, BLK_STATUS) & check_bits) == 0),
> + 100, 1000, 1000);
> + if (ret < 0) {
> + DRM_ERROR("disconnect from TCU timeout!\n");
> + malidp_write32_mask(reg, BLK_CONTROL, 0x7, INACTIVE_MODE);
> + }
> +
> + return ret;
> +}
> +
> static struct komeda_dev_funcs d71_chip_funcs = {
> .init_format_table = d71_init_fmt_tbl,
> .enum_resources = d71_enum_resources,
> @@ -527,6 +574,8 @@ static void d71_init_fmt_tbl(struct komeda_dev *mdev)
> .on_off_vblank = d71_on_off_vblank,
> .change_opmode = d71_change_opmode,
> .flush = d71_flush,
> + .connect_iommu = d71_connect_iommu,
> + .disconnect_iommu = d71_disconnect_iommu,
> };
>
> struct komeda_dev_funcs *
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index c92e161..e80e673 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -253,6 +253,19 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
> dev->dma_parms = &mdev->dma_parms;
> dma_set_max_seg_si
[Bug 109206] Kernel 4.20 amdgpu fails to load firmware on Ryzen 2500U
https://bugs.freedesktop.org/show_bug.cgi?id=109206 --- Comment #47 from Ondrej Lang --- The patch is not part of the kernel package itself, it is part of the linux-firmware package, there was no new release of linux-firmware since I tested this. I think the reason why you did not have to rebuld the initramfs is that there was no linux-firmware update, which would again put the raven_dmcu.bin file in /lib/firmware/amdgpu/ folder and also in the initramfs, so you would need to rename/move it again and run dracut to rebuild the initramfs. once a new version of linux-firmware comes out (with the patch) I will re-test and report results here. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 7/7] arm64: dts: allwinner: a64: enable ANX6345 bridge on Teres-I
On Fri, Jun 07, 2019 at 08:28:02AM +0200, Maxime Ripard wrote: > On Thu, Jun 06, 2019 at 03:59:27PM +0200, Harald Geyer wrote: > > > > If think valid compatible properties would be: > > compatible = "innolux,n116bge", "simple-panel"; > > compatible = "edp-connector", "simple-panel"; > > A connector isn't a panel. > > > compatible = "innolux,n116bge", "edp-connector", "simple-panel"; > > And the innolux,n116bge is certainly not a connector either. > > > compatible = "edp-connector", "innolux,n116bge", "simple-panel"; > > > > I can't make up my mind which one I prefere. However neither of these > > variants requires actually implmenting an edp-connector driver. > > No-one asked to do an edp-connector driver. You should use it in your > DT, but if you want to have some code in your driver that parses the > DT directly, I'm totally fine with that. I must admit I fail to understand what that extra node would be good for. Logically, the eDP far side is connected to the well-known n116bge. Inside the laptop case it might as well be a flat ribbon cable or soldered directly. In good intention, that's all I wanted to express in the DT. I don't know whether the relevant mechanical dimensions of the panel and the connector are standardised, so whether one could in theory assemble it with a different panel than the one it came with. OTOH, as I checked during the discussion with anarsoul, the panel's supply voltage is permanently connected to the main 3.3V rail. We already agreed that the eDP output port must not neccessarily be specified, this setup is a good example why: because the panel is always powered, the anx6345 can always pull valid EDID data from it so at this stage there's no need for any OS driver to reach beyond the bridge. IIRC even the backlight got switched off for the blank screen without. All I wanted to say is that "there's usually an n116bge behind it"; but this is mostly redundant. So, shall we just drop the output port specification (along with the panel node) in order to get one step further? > I guess you should describe why do you think it's "clear", because I'm > not sure this is obvious for everyone here. eDP allows to discover > which device is on the other side and its supported timings, just like > HDMI for example (or regular DP, for that matter). Would you think > it's clearly preferable to ship a DT with the DP/HDMI monitor > connected on the other side exposed as a panel as well? Well, as I wrote: "in good intention". That's the panel that comes with the kit but it is very well detected automatically, just like you describe. So, just leave it out? Torsten
Re: [PATCH][next] drm/bridge: sii902x: fix comparision of u32 with less than zero
On 03.06.2019 16:21, Colin King wrote:
> From: Colin Ian King
>
> The less than check for the variable num_lanes is always going to be
> false because the variable is a u32. Fix this by making num_lanes an
> int and also make loop index i an int too.
>
> Addresses-Coverity: ("Unsigned compared against 0")
Is there a rule in Kernel of adding such tags?
I have spotted only: Addresses-Coverity-ID?
Beside this:
Reviewed-by: Andrzej Hajda
--
Regards
Andrzej
> Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
> Signed-off-by: Colin Ian King
> ---
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c
> b/drivers/gpu/drm/bridge/sii902x.c
> index d6f98d388ac2..21a947603c88 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -719,7 +719,7 @@ static int sii902x_audio_codec_init(struct sii902x
> *sii902x,
> .max_i2s_channels = 0,
> };
> u8 lanes[4];
> - u32 num_lanes, i;
> + int num_lanes, i;
>
> if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
> dev_dbg(dev, "%s: No \"#sound-dai-cells\", no audio\n",
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/bridge: sii902x: fix a signedness bug in sii902x_audio_codec_init()
On 07.06.2019 09:27, Dan Carpenter wrote:
> The "num_lanes" variable needs to be signed for the error handling to
> work. The "i" variable can be a regular int as well.
>
> Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
> Signed-off-by: Dan Carpenter
Colin send already fix for this.
Regards
Andrzej
> ---
> drivers/gpu/drm/bridge/sii902x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c
> b/drivers/gpu/drm/bridge/sii902x.c
> index 6b03616d6bc3..dd7aa466b280 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -719,7 +719,7 @@ static int sii902x_audio_codec_init(struct sii902x
> *sii902x,
> .max_i2s_channels = 0,
> };
> u8 lanes[4];
> - u32 num_lanes, i;
> + int num_lanes, i;
>
> if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
> dev_dbg(dev, "%s: No \"#sound-dai-cells\", no audio\n",
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/bridge: sii902x: re-order conditions to prevent out of bounds read
On 07.06.2019 09:27, Dan Carpenter wrote:
> This should check that "i" is within bounds before checking reading from
> the array.
>
> Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
> Signed-off-by: Dan Carpenter
Reviewed-by: Andrzej Hajda
--
Regards
Andrzej
> ---
> drivers/gpu/drm/bridge/sii902x.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/sii902x.c
> b/drivers/gpu/drm/bridge/sii902x.c
> index d6f98d388ac2..6b03616d6bc3 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -589,8 +589,8 @@ static int sii902x_audio_hw_params(struct device *dev,
> void *data,
> if (ret)
> goto out;
>
> - for (i = 0; sii902x->audio.i2s_fifo_sequence[i] &&
> - i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence); i++)
> + for (i = 0; i < ARRAY_SIZE(sii902x->audio.i2s_fifo_sequence) &&
> + sii902x->audio.i2s_fifo_sequence[i]; i++)
> regmap_write(sii902x->regmap,
>SII902X_TPI_I2S_ENABLE_MAPPING_REG,
>sii902x->audio.i2s_fifo_sequence[i]);
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/bridge: sii902x: fix comparision of u32 with less than zero
On 07/06/2019 10:41, Andrzej Hajda wrote:
> On 03.06.2019 16:21, Colin King wrote:
>> From: Colin Ian King
>>
>> The less than check for the variable num_lanes is always going to be
>> false because the variable is a u32. Fix this by making num_lanes an
>> int and also make loop index i an int too.
>>
>> Addresses-Coverity: ("Unsigned compared against 0")
>
>
> Is there a rule in Kernel of adding such tags?
>
> I have spotted only: Addresses-Coverity-ID?
>
Unfortunately I'm running a Coverity in-house and so the Coverity ID is
not public, so it does not make sense for me to report the ID.
Colin
>
> Beside this:
>
> Reviewed-by: Andrzej Hajda
>
> --
> Regards
> Andrzej
>
>
>> Fixes: ff5781634c41 ("drm/bridge: sii902x: Implement HDMI audio support")
>> Signed-off-by: Colin Ian King
>> ---
>> drivers/gpu/drm/bridge/sii902x.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/sii902x.c
>> b/drivers/gpu/drm/bridge/sii902x.c
>> index d6f98d388ac2..21a947603c88 100644
>> --- a/drivers/gpu/drm/bridge/sii902x.c
>> +++ b/drivers/gpu/drm/bridge/sii902x.c
>> @@ -719,7 +719,7 @@ static int sii902x_audio_codec_init(struct sii902x
>> *sii902x,
>> .max_i2s_channels = 0,
>> };
>> u8 lanes[4];
>> -u32 num_lanes, i;
>> +int num_lanes, i;
>>
>> if (!of_property_read_bool(dev->of_node, "#sound-dai-cells")) {
>> dev_dbg(dev, "%s: No \"#sound-dai-cells\", no audio\n",
>
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] drm/komeda: Add split support for scaler
Hi James,
On Mon, May 20, 2019 at 11:44:47AM +0100, james qian wang (Arm Technology
China) wrote:
> To achieve same caling effect compare with none split, the texel
> calculation need to use the same scaling ratio before split, so add
> "total_xxx" to pipeline to describe the hsize/vsize before split.
> Update pipeline and d71_scaler_update accordingly.
>
> Signed-off-by: James Qian Wang (Arm Technology China)
>
> ---
> .../arm/display/komeda/d71/d71_component.c| 47 +--
> .../drm/arm/display/komeda/komeda_pipeline.h | 19 ++--
> .../display/komeda/komeda_pipeline_state.c| 21 -
> .../gpu/drm/arm/display/komeda/komeda_plane.c | 8 ++--
> .../arm/display/komeda/komeda_wb_connector.c | 2 +-
> 5 files changed, 81 insertions(+), 16 deletions(-)
>
> 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 3266bd54c936..d101a5cc2766 100644
> --- a/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> +++ b/drivers/gpu/drm/arm/display/komeda/d71/d71_component.c
> @@ -642,23 +642,58 @@ static void d71_scaler_update(struct komeda_component
> *c,
>
> malidp_write32(reg, BLK_IN_SIZE, HV_SIZE(st->hsize_in, st->vsize_in));
> malidp_write32(reg, SC_OUT_SIZE, HV_SIZE(st->hsize_out, st->vsize_out));
> + malidp_write32(reg, SC_H_CROP, HV_CROP(st->left_crop, st->right_crop));
> +
> + /* for right part, HW only sample the valid pixel which means the pixels
> + * in left_crop will be jumpped, and the first sample pixel is:
> + *
> + * dst_a = st->total_hsize_out - st->hsize_out + st->left_crop + 0.5;
> + *
> + * Then the corresponding texel in src is:
> + *
> + * h_delta_phase = st->total_hsize_in / st->total_hsize_out;
> + * src_a = dst_A * h_delta_phase;
> + *
> + * and h_init_phase is src_a deduct the real source start src_S;
> + *
> + * src_S = st->total_hsize_in - st->hsize_in;
> + * h_init_phase = src_a - src_S;
> + *
> + * And HW precision for the initial/delta_phase is 16:16 fixed point,
> + * the following is the simplified formula
> + */
> + if (st->right_part) {
> + u32 dst_a = st->total_hsize_out - st->hsize_out + st->left_crop;
> +
> + if (st->en_img_enhancement)
> + dst_a -= 1;
> +
> + init_ph = ((st->total_hsize_in * (2 * dst_a + 1) -
> + 2 * st->total_hsize_out * (st->total_hsize_in -
> + st->hsize_in)) << 15) / st->total_hsize_out;
> + } else {
> + init_ph = (st->total_hsize_in << 15) / st->total_hsize_out;
> + }
>
> - init_ph = (st->hsize_in << 15) / st->hsize_out;
> malidp_write32(reg, SC_H_INIT_PH, init_ph);
>
> - delta_ph = (st->hsize_in << 16) / st->hsize_out;
> + delta_ph = (st->total_hsize_in << 16) / st->total_hsize_out;
> malidp_write32(reg, SC_H_DELTA_PH, delta_ph);
>
> - init_ph = (st->vsize_in << 15) / st->vsize_out;
> + init_ph = (st->total_vsize_in << 15) / st->vsize_out;
> malidp_write32(reg, SC_V_INIT_PH, init_ph);
>
> - delta_ph = (st->vsize_in << 16) / st->vsize_out;
> + delta_ph = (st->total_vsize_in << 16) / st->vsize_out;
> malidp_write32(reg, SC_V_DELTA_PH, delta_ph);
>
> ctrl = 0;
> ctrl |= st->en_scaling ? SC_CTRL_SCL : 0;
> ctrl |= st->en_alpha ? SC_CTRL_AP : 0;
> ctrl |= st->en_img_enhancement ? SC_CTRL_IENH : 0;
> + /* If we use the hardware splitter we shouldn't set SC_CTRL_LS */
> + if (st->en_split &&
> + state->inputs[0].component->id != KOMEDA_COMPONENT_SPLITTER)
> + ctrl |= SC_CTRL_LS;
>
> malidp_write32(reg, BLK_CONTROL, ctrl);
> malidp_write32(reg, BLK_INPUT_ID0, to_d71_input_id(&state->inputs[0]));
> @@ -716,10 +751,12 @@ static int d71_scaler_init(struct d71_dev *d71,
> }
>
> scaler = to_scaler(c);
> - set_range(&scaler->hsize, 4, d71->max_line_size);
> + set_range(&scaler->hsize, 4, 2048);
> set_range(&scaler->vsize, 4, 4096);
> scaler->max_downscaling = 6;
> scaler->max_upscaling = 64;
> + scaler->scaling_split_overlap = 8;
> + scaler->enh_split_overlap = 1;
>
> malidp_write32(c->reg, BLK_CONTROL, 0);
>
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> index c92733736799..4e1cf8fd89bf 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline.h
> @@ -247,15 +247,22 @@ struct komeda_scaler {
> struct malidp_range hsize, vsize;
> u32 max_upscaling;
> u32 max_downscaling;
> + u8 scaling_split_overlap; /* split overlap for scaling */
> + u8 enh_split_overlap; /* split overlap for image enhancement */
> };
>
> struct komeda_scaler_state {
> struct komeda_c
Re: [PATCH 3/4] drm: Increase DRM_OBJECT_MAX_PROPERTY to 32
On Tue, May 21, 2019 at 10:34:54AM +0100, james qian wang (Arm Technology
China) wrote:
> DRM_OBJECT_MAX_PROPERTY number 24 is not enough for komeda usage, increase
> it to 32 to fit komeda's requirement.
>
> Signed-off-by: James Qian Wang (Arm Technology China)
>
Reviewed-by: Liviu Dudau
Best regards,
Liviu
> ---
> include/drm/drm_mode_object.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/drm/drm_mode_object.h b/include/drm/drm_mode_object.h
> index c34a3e8030e1..fd7666048197 100644
> --- a/include/drm/drm_mode_object.h
> +++ b/include/drm/drm_mode_object.h
> @@ -60,7 +60,7 @@ struct drm_mode_object {
> void (*free_cb)(struct kref *kref);
> };
>
> -#define DRM_OBJECT_MAX_PROPERTY 24
> +#define DRM_OBJECT_MAX_PROPERTY 32
> /**
> * struct drm_object_properties - property tracking for &drm_mode_object
> */
> --
> 2.17.1
--
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---
¯\_(ツ)_/¯
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/33] fbcon notifier begone v3!
On 6/6/19 9:38 AM, Daniel Vetter wrote: > Hi Bart, Hi Daniel, > On Tue, May 28, 2019 at 11:02:31AM +0200, Daniel Vetter wrote: >> Hi all, >> >> I think we're slowly getting there. Previous cover letters with more >> context: >> >> https://lists.freedesktop.org/archives/dri-devel/2019-May/218362.html >> >> tldr; I have a multi-year plan to improve fbcon locking, because the >> current thing is a bit a mess. >> >> Cover letter of this version, where I detail a bit more the details >> fixed in this one here: >> >> https://lists.freedesktop.org/archives/dri-devel/2019-May/218984.html >> >> Note that the locking plan in this one is already outdated, I overlooked a >> few fun issues around any printk() going back to console_lock. >> >> I think remaining bits: >> >> - Ack from Daniel Thompson for the backlight bits, he wanted to check the >> big picture. > > I think Daniel is still on vacation until next week or so. > >> - Hash out actual merge plan. > > I'd like to stuff this into drm.git somehow, I guess topic branch works > too. I would like to have topic branch for this patchset. > Long term I think we need to reconsider how we handle fbdev, at least the > core/fbcon pieces. Since a few years all the work in that area has been > motivated by drm, and pushed by drm contributors. Having that maintained > in a separate tree that doesn't regularly integrate imo doesn't make much > sense, and we ended up merging almost everything through some drm tree. > That one time we didn't (for some panel rotation stuff) it resulted in > some good suprises. > > I think best solution is if we put the core and fbcon bits into drm-misc, > as group maintained infrastructure piece. All the other gfx infra pieces > are maintained in there already too. You'd obviously get commit rights. > I think that would include > - drivers/video/fbdev > - drivers/video/*c > - drivers/video/console Sounds fine to me. > I don't really care about what happens with the actual fbdev drivers > (aside from the drm one in drm_fb_helper.c, but that's already maintained > as part of drm). I guess we could also put those into drm-misc, or as a > separate tree, depending what you want. > > Thoughts? I would like to handle fbdev changes for v5.3 merge window using fbdev tree but after that everything (including changes to fbdev drivers) can go through drm-misc tree. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > Cheers, Daniel > > >> >> I'm also cc'ing the entire pile to a lot more people on request. >> >> Thanks, Daniel >> >> Daniel Vetter (33): >> dummycon: Sprinkle locking checks >> fbdev: locking check for fb_set_suspend >> vt: might_sleep() annotation for do_blank_screen >> vt: More locking checks >> fbdev/sa1100fb: Remove dead code >> fbdev/cyber2000: Remove struct display >> fbdev/aty128fb: Remove dead code >> fbcon: s/struct display/struct fbcon_display/ >> fbcon: Remove fbcon_has_exited >> fbcon: call fbcon_fb_(un)registered directly >> fbdev/sh_mobile: remove sh_mobile_lcdc_display_notify >> fbdev/omap: sysfs files can't disappear before the device is gone >> fbdev: sysfs files can't disappear before the device is gone >> staging/olpc: lock_fb_info can't fail >> fbdev/atyfb: lock_fb_info can't fail >> fbdev: lock_fb_info cannot fail >> fbcon: call fbcon_fb_bind directly >> fbdev: make unregister/unlink functions not fail >> fbdev: unify unlink_framebuffer paths >> fbdev/sh_mob: Remove fb notifier callback >> fbdev: directly call fbcon_suspended/resumed >> fbcon: Call fbcon_mode_deleted/new_modelist directly >> fbdev: Call fbcon_get_requirement directly >> Revert "backlight/fbcon: Add FB_EVENT_CONBLANK" >> fbmem: pull fbcon_fb_blanked out of fb_blank >> fbdev: remove FBINFO_MISC_USEREVENT around fb_blank >> fb: Flatten control flow in fb_set_var >> fbcon: replace FB_EVENT_MODE_CHANGE/_ALL with direct calls >> vgaswitcheroo: call fbcon_remap_all directly >> fbcon: Call con2fb_map functions directly >> fbcon: Document what I learned about fbcon locking >> staging/olpc_dcon: Add drm conversion to TODO >> backlight: simplify lcd notifier >> >> arch/arm/mach-pxa/am200epd.c | 13 +- >> drivers/gpu/vga/vga_switcheroo.c | 11 +- >> drivers/media/pci/ivtv/ivtvfb.c | 6 +- >> drivers/staging/fbtft/fbtft-core.c| 4 +- >> drivers/staging/olpc_dcon/TODO| 7 + >> drivers/staging/olpc_dcon/olpc_dcon.c | 6 +- >> drivers/tty/vt/vt.c | 18 + >> drivers/video/backlight/backlight.c | 2 +- >> drivers/video/backlight/lcd.c | 12 - >> drivers/video/console/dummycon.c | 6 + >> drivers/video/fbdev/aty/aty128fb.c| 64 --- >> drivers/video/fbdev/aty/atyfb_base.c | 3 +- >> drivers/video/fbdev/core/fbcmap.c | 6 +- >> drivers/video/fbdev/core/fbco
Re: [Intel-gfx] [PATCH] drm/crc-debugfs: Also sprinkle irqrestore over early exits
On Thu, 6 Jun 2019 at 22:15, Daniel Vetter wrote:
>
> I. was. blind.
>
> Caught with vkms, which has some really slow crc computation function.
>
> Fixes: 1882018a70e0 ("drm/crc-debugfs: User irqsafe spinlock in
> drm_crtc_add_crc_entry")
> Cc: Rodrigo Siqueira
> Cc: Tomeu Vizoso
> Cc: Emil Velikov
> Cc: Benjamin Gaignard
> Cc: Ville Syrjälä
> Signed-off-by: Daniel Vetter
Reviewed-by: Emil Velikov
-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ioctl: Ditch DRM_UNLOCKED except for the legacy vblank ioctl
On Wed, 5 Jun 2019 at 13:08, Daniel Vetter wrote: > > This completes Emil's series of removing DRM_UNLOCKED from modern > drivers. It's entirely cargo-culted since we ignore it on > non-DRIVER_LEGACY drivers since: > > commit ea487835e8876abf7ad909636e308c801a2bcda6 > Author: Daniel Vetter > Date: Mon Sep 28 21:42:40 2015 +0200 > > drm: Enforce unlocked ioctl operation for kms driver ioctls > > Now justifying why we can do this for legacy drives too (and hence > close the source of all the bogus copypasting) is a bit more involved. > DRM_UNLOCKED was introduced in: > > commit ed8b67040965e4fe695db333d5914e18ea5f146f > Author: Arnd Bergmann > Date: Wed Dec 16 22:17:09 2009 + > > drm: convert drm_ioctl to unlocked_ioctl > > As a immediate hack to keep i810 happy, which would have deadlocked > without this trickery. The old BKL is automatically dropped in > schedule(), and hence the i810 vs. mmap_sem deadlock didn't actually > cause a real deadlock. But with a mutex it would. The solution was to > annotate these as DRM_UNLOCKED and mark i810 unsafe on SMP machines. > > This conversion caused a regression, because unlike the BKL a mutex > isn't dropped over schedule (that thing again), which caused a vblank > wait in one thread to block the entire desktop and all its apps. Back > then we did vblank scheduling by blocking in the client, awesome isn't > it. This was fixed quickly in (ok not so quickly, took 2 years): > > commit 8f4ff2b06afcd6f151868474a432c603057eaf56 > Author: Ilija Hadzic > Date: Mon Oct 31 17:46:18 2011 -0400 > > drm: do not sleep on vblank while holding a mutex > > All the other DRM_UNLOCKED annotations for all the core ioctls was > work to reach finer-grained locking for modern drivers. This took > years, and culminated in: > > commit fdd5b877e9ebc2029e1373b4a3cd057329a9ab7a > Author: Daniel Vetter > Date: Sat Dec 10 22:52:54 2016 +0100 > > drm: Enforce BKL-less ioctls for modern drivers > > DRM_UNLOCKED was never required by any legacy drivers, except for the > vblank_wait IOCTL. Therefore we will not regress these old drivers by > going back to where we've been in 2011. For all modern drivers nothing > will change. > > To make this perfectly clear, also add a comment to DRM_UNLOCKED. > > v2: Don't forget about drm_ioc32.c (Michel). Not a source of copypasta > mistakes when creating driver ioctl tables, but better to be > consistent. > Personally I would omit the "Not a source of copupasta..." note. > Cc: Michel Dänzer > Cc: Emil Velikov > Signed-off-by: Daniel Vetter Double-checked that only the VBLACK ioctl retained its annotation and all other core ioctls are DRM_UNLOCKED free. Technically with this change UMS drivers will start using a lock on the listed ioctls. I do not expect this to be a problem and admittedly I did not audit existing userspace. That said, the patch looks reasonable: Acked-by: Emil Velikov Daniel, can you apply any outstanding DRM_UNLOCKED patches from my series? Thanks Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 110725] [CI][SHARDS] igt@kms_big_fb@(linear|x-tiled|y-tiled)-(16|32|64)bpp-rotate-(90|270) - skip - unsupported configuration, SKIP
https://bugs.freedesktop.org/show_bug.cgi?id=110725
--- Comment #10 from CI Bug Log ---
A CI Bug Log filter associated to this bug has been updated:
{- CML ICL:
igt@kms_big_fb@(linear|x-tiled|y-tiled)-(16|32|64)bpp-rotate-(90|270) - skip -
unsupported configuration, SKIP -}
{+ CML ICL:
igt@kms_big_fb@(linear|x-tiled|y-tiled)-(8|16|32|64)bpp-rotate-(90|270) - skip
- unsupported configuration, SKIP +}
New failures caught by the filter:
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u/igt@kms_big...@y-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u/igt@kms_big...@y-tiled-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u/igt@kms_big...@linear-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u/igt@kms_big...@linear-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u/igt@kms_big...@x-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u2/igt@kms_big...@y-tiled-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u2/igt@kms_big...@linear-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u2/igt@kms_big...@x-tiled-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u2/igt@kms_big...@linear-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-cml-u2/igt@kms_big...@x-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-dsi/igt@kms_big...@linear-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-dsi/igt@kms_big...@x-tiled-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-dsi/igt@kms_big...@x-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-guc/igt@kms_big...@x-tiled-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-guc/igt@kms_big...@x-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-u2/igt@kms_big...@y-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-u2/igt@kms_big...@y-tiled-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-u2/igt@kms_big...@linear-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-u2/igt@kms_big...@linear-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-u2/igt@kms_big...@x-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-u3/igt@kms_big...@y-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-u3/igt@kms_big...@linear-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-u3/igt@kms_big...@linear-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-y/igt@kms_big...@y-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-y/igt@kms_big...@y-tiled-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-y/igt@kms_big...@linear-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_302/fi-icl-y/igt@kms_big...@x-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5041/shard-iclb2/igt@kms_big...@x-tiled-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5041/shard-iclb3/igt@kms_big...@linear-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5041/shard-iclb3/igt@kms_big...@x-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5041/shard-iclb4/igt@kms_big...@y-tiled-8bpp-rotate-90.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5041/shard-iclb8/igt@kms_big...@y-tiled-8bpp-rotate-270.html
*
https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5041/shard-iclb8/igt@kms_big...@linear-8bpp-rotate-270.html
--
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/komeda: Avoid using DRIVER_IRQ_SHARED
With reference to mainline commit (1ff494813bafa127ecba1160262ba39b2fdde7ba), DRIVER_IRQ_SHARED is to be used only by legacy drivers. Further, drm_irq_install() ignores this flag altogether. One needs to use devm_request_irq() instead, with IRQF_SHARED to create a shared interrupt handler. Signed-off-by: Ayan Kumar halder --- drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c index 86f6542afb40..7b5cde14e3ba 100644 --- a/drivers/gpu/drm/arm/display/komeda/komeda_kms.c +++ b/drivers/gpu/drm/arm/display/komeda/komeda_kms.c @@ -194,7 +194,9 @@ struct komeda_kms_dev *komeda_kms_attach(struct komeda_dev *mdev) drm_mode_config_reset(drm); - err = drm_irq_install(drm, mdev->irq); + err = devm_request_irq(drm->dev, mdev->irq, + komeda_kms_driver.irq_handler, IRQF_SHARED, + drm->driver->name, drm); if (err) goto cleanup_mode_config; -- 2.21.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/5] dma-buf: Add dma-buf heaps framework
Hi John, I think it's looking good. I spotted a couple of error paths which I think are missing cleanup, no complaints about the API though. On Fri, Jun 07, 2019 at 03:07:15AM +, John Stultz wrote: > From: "Andrew F. Davis" > > This framework allows a unified userspace interface for dma-buf > exporters, allowing userland to allocate specific types of memory > for use in dma-buf sharing. > > Each heap is given its own device node, which a user can allocate > a dma-buf fd from using the DMA_HEAP_IOC_ALLOC. > > This code is an evoluiton of the Android ION implementation, > and a big thanks is due to its authors/maintainers over time > for their effort: > Rebecca Schultz Zavin, Colin Cross, Benjamin Gaignard, > Laura Abbott, and many other contributors! > > Cc: Laura Abbott > Cc: Benjamin Gaignard > Cc: Sumit Semwal > Cc: Liam Mark > Cc: Pratik Patel > Cc: Brian Starkey > Cc: Vincent Donnefort > Cc: Sudipto Paul > Cc: Andrew F. Davis > Cc: Christoph Hellwig > Cc: Chenbo Feng > Cc: Alistair Strachan > Cc: dri-devel@lists.freedesktop.org > Reviewed-by: Benjamin Gaignard > Signed-off-by: Andrew F. Davis > Signed-off-by: John Stultz > Change-Id: I4af43a137ad34ff6f7da4d6b2864f3cd86fb7652 > --- > v2: > * Folded down fixes I had previously shared in implementing > heaps > * Make flags a u64 (Suggested by Laura) > * Add PAGE_ALIGN() fix to the core alloc funciton > * IOCTL fixups suggested by Brian > * Added fixes suggested by Benjamin > * Removed core stats mgmt, as that should be implemented by > per-heap code > * Changed alloc to return a dma-buf fd, rather then a buffer nit:s/then/than/ > (as it simplifies error handling) > v3: > * Removed scare-quotes in MAINTAINERS email address > * Get rid of .release function as it didn't do anything (from > Christoph) > * Renamed filp to file (suggested by Christoph) > * Split out ioctl handling to separate function (suggested by > Christoph) > * Add comment documenting PAGE_ALIGN usage (suggested by Brian) > * Switch from idr to Xarray (suggested by Brian) > * Fixup cdev creation (suggested by Brian) > * Avoid EXPORT_SYMBOL until we finalize modules (suggested by > Brian) > * Make struct dma_heap internal only (folded in from Andrew) > * Small cleanups suggested by GregKH > * Provide class->devnode callback to get consistent /dev/ > subdirectory naming (Suggested by Bjorn) > v4: > * Folded down dma-heap.h change that was in a following patch > * Added fd_flags entry to allocation structure and pass it > through to heap code for use on dma-buf fd creation (suggested > by Benjamin) > v5: > * Minor cleanups > --- > MAINTAINERS | 18 +++ > drivers/dma-buf/Kconfig | 8 ++ > drivers/dma-buf/Makefile | 1 + > drivers/dma-buf/dma-heap.c| 237 ++ > include/linux/dma-heap.h | 59 + > include/uapi/linux/dma-heap.h | 56 > 6 files changed, 379 insertions(+) > create mode 100644 drivers/dma-buf/dma-heap.c > create mode 100644 include/linux/dma-heap.h > create mode 100644 include/uapi/linux/dma-heap.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index a6954776a37e..5aded7e9a062 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4813,6 +4813,24 @@ F: include/linux/*fence.h > F: Documentation/driver-api/dma-buf.rst > T: git git://anongit.freedesktop.org/drm/drm-misc > > +DMA-BUF HEAPS FRAMEWORK > +M: Sumit Semwal > +R: Andrew F. Davis > +R: Benjamin Gaignard > +R: Liam Mark > +R: Laura Abbott > +R: Brian Starkey > +R: John Stultz > +S: Maintained > +L: linux-me...@vger.kernel.org > +L: dri-devel@lists.freedesktop.org > +L: linaro-mm-...@lists.linaro.org (moderated for non-subscribers) > +F: include/uapi/linux/dma-heap.h > +F: include/linux/dma-heap.h > +F: drivers/dma-buf/dma-heap.c > +F: drivers/dma-buf/heaps/* > +T: git git://anongit.freedesktop.org/drm/drm-misc > + > DMA GENERIC OFFLOAD ENGINE SUBSYSTEM > M: Vinod Koul > L: dmaeng...@vger.kernel.org > diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig > index d5f915830b68..9b93f86f597c 100644 > --- a/drivers/dma-buf/Kconfig > +++ b/drivers/dma-buf/Kconfig > @@ -39,4 +39,12 @@ config UDMABUF > A driver to let userspace turn memfd regions into dma-bufs. > Qemu can use this to create host dmabufs for guest framebuffers. > > +menuconfig DMABUF_HEAPS > + bool "DMA-BUF Userland Memory Heaps" > + select DMA_SHARED_BUFFER > + help > + Choose this option to enable the DMA-BUF userland memory heaps, > + this allows userspace to allocate dma-bufs that can be shared between > + drivers. > + > endmenu > diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile > index e8c7310cb800..1cb3dd104825 100644 > --- a/drivers/dma-buf/Makefile > +++ b/drivers/dma-buf/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0-only > obj-y := dma-buf.o dma-fence.o dma-fence-array.o
[PATCH 1/2] drm/edid: abstract override/firmware EDID retrieval
Abstract the debugfs override and the firmware EDID retrieval
function. We'll be needing it in the follow-up. No functional changes.
Cc: Daniel Vetter
Cc: Harish Chegondi
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index c3296a72fff9..c59a1e8c5ada 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1573,6 +1573,20 @@ static void connector_bad_edid(struct drm_connector
*connector,
}
}
+/* Get override or firmware EDID */
+static struct edid *drm_get_override_edid(struct drm_connector *connector)
+{
+ struct edid *override = NULL;
+
+ if (connector->override_edid)
+ override = drm_edid_duplicate(connector->edid_blob_ptr->data);
+
+ if (!override)
+ override = drm_load_edid_firmware(connector);
+
+ return IS_ERR(override) ? NULL : override;
+}
+
/**
* drm_do_get_edid - get EDID data using a custom EDID block read function
* @connector: connector we're probing
@@ -1600,15 +1614,10 @@ struct edid *drm_do_get_edid(struct drm_connector
*connector,
{
int i, j = 0, valid_extensions = 0;
u8 *edid, *new;
- struct edid *override = NULL;
-
- if (connector->override_edid)
- override = drm_edid_duplicate(connector->edid_blob_ptr->data);
-
- if (!override)
- override = drm_load_edid_firmware(connector);
+ struct edid *override;
- if (!IS_ERR_OR_NULL(override))
+ override = drm_get_override_edid(connector);
+ if (override)
return override;
if ((edid = kmalloc(EDID_LENGTH, GFP_KERNEL)) == NULL)
--
2.20.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] drm: add fallback override/firmware EDID modes workaround
We've moved the override and firmware EDID (simply "override EDID" from
now on) handling to the low level drm_do_get_edid() function in order to
transparently use the override throughout the stack. The idea is that
you get the override EDID via the ->get_modes() hook.
Unfortunately, there are scenarios where the DDC probe in drm_get_edid()
called via ->get_modes() fails, although the preceding ->detect()
succeeds.
In the case reported by Paul Wise, the ->detect() hook,
intel_crt_detect(), relies on hotplug detect, bypassing the DDC. In the
case reported by Ilpo Järvinen, there is no ->detect() hook, which is
interpreted as connected. The subsequent DDC probe reached via
->get_modes() fails, and we don't even look at the override EDID,
resulting in no modes being added.
Because drm_get_edid() is used via ->detect() all over the place, we
can't trivially remove the DDC probe, as it leads to override EDID
effectively meaning connector forcing. The goal is that connector
forcing and override EDID remain orthogonal.
Generally, the underlying problem here is the conflation of ->detect()
and ->get_modes() via drm_get_edid(). The former should just detect, and
the latter should just get the modes, typically via reading the EDID. As
long as drm_get_edid() is used in ->detect(), it needs to retain the DDC
probe. Or such users need to have a separate DDC probe step first.
Work around the regression by falling back to a separate attempt at
getting the override EDID at drm_helper_probe_single_connector_modes()
level. With a working DDC and override EDID, it'll never be called; the
override EDID will come via ->get_modes(). There will still be a failing
DDC probe attempt in the cases that require the fallback.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107583
Reported-by: Paul Wise
Cc: Paul Wise
References:
alpine.DEB.2.20.1905262211270.24390@whs-18.cs.helsinki.fi">http://mid.mail-archive.com/alpine.DEB.2.20.1905262211270.24390@whs-18.cs.helsinki.fi
Reported-by: Ilpo Järvinen
Cc: Ilpo Järvinen
References: 15f080f08d48 ("drm/edid: respect connector force for drm_get_edid
ddc probe")
Fixes: 53fd40a90f3c ("drm: handle override and firmware EDID at
drm_do_get_edid() level")
Cc: # v4.15+
Cc: Daniel Vetter
Cc: Ville Syrjälä
Cc: Harish Chegondi
Signed-off-by: Jani Nikula
---
drivers/gpu/drm/drm_edid.c | 29 +
drivers/gpu/drm/drm_probe_helper.c | 7 +++
include/drm/drm_edid.h | 1 +
3 files changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index c59a1e8c5ada..780146bfc225 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1587,6 +1587,35 @@ static struct edid *drm_get_override_edid(struct
drm_connector *connector)
return IS_ERR(override) ? NULL : override;
}
+/**
+ * drm_add_override_edid_modes - add modes from override/firmware EDID
+ * @connector: connector we're probing
+ *
+ * Add modes from the override/firmware EDID, if available. Only to be used
from
+ * drm_helper_probe_single_connector_modes() as a fallback for when DDC probe
+ * failed during drm_get_edid() and caused the override/firmware EDID to be
+ * skipped.
+ *
+ * Return: The number of modes added or 0 if we couldn't find any.
+ */
+int drm_add_override_edid_modes(struct drm_connector *connector)
+{
+ struct edid *override;
+ int num_modes = 0;
+
+ override = drm_get_override_edid(connector);
+ if (override) {
+ num_modes = drm_add_edid_modes(connector, override);
+ kfree(override);
+
+ DRM_DEBUG_KMS("[CONNECTOR:%d:%s] adding %d modes via fallback
override/firmware EDID\n",
+ connector->base.id, connector->name, num_modes);
+ }
+
+ return num_modes;
+}
+EXPORT_SYMBOL(drm_add_override_edid_modes);
+
/**
* drm_do_get_edid - get EDID data using a custom EDID block read function
* @connector: connector we're probing
diff --git a/drivers/gpu/drm/drm_probe_helper.c
b/drivers/gpu/drm/drm_probe_helper.c
index 01e243f1ea94..ef2c468205a2 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -480,6 +480,13 @@ int drm_helper_probe_single_connector_modes(struct
drm_connector *connector,
count = (*connector_funcs->get_modes)(connector);
+ /*
+* Fallback for when DDC probe failed in drm_get_edid() and thus skipped
+* override/firmware EDID.
+*/
+ if (count == 0 && connector->status == connector_status_connected)
+ count = drm_add_override_edid_modes(connector);
+
if (count == 0 && connector->status == connector_status_connected)
count = drm_add_modes_noedid(connector, 1024, 768);
count += drm_helper_probe_add_cmdline_mode(connector);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 88b63801f9db..b9719418c3d2 100644
--- a/include/drm/drm_edid.h
+++ b/
[Bug 110313] [CI][SHARDS] igt@kms_lease@lease-uevent - fail - Failed assertion: igt_lease_change_detected(uevent_monitor, 1)
https://bugs.freedesktop.org/show_bug.cgi?id=110313 --- Comment #5 from Mika Kahola --- It seems that we haven't seen this bug for a while now. Can we close this one? -- 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: DRM/AST regression (likely 4.14 -> 4.19+), providing EDID manually fails
On Tue, 04 Jun 2019, Ilpo Järvinen wrote: > Yes, if it applies fine to 5.1. ...Also, it will take a week or so. Please try these two patches instead: https://patchwork.freedesktop.org/series/61764/ BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3 0/5] drm/panel-simple: Add panel parameters for ortustech-com37h3m05dtc/99dtc and sharp-lq070y3dg3b
V3: * add bindings documentation (suggested by s...@ravnborg.org) V2 2019-06-05 07:07:05: * fix typo in 99dtc panel compatible string (reported by imir...@alum.mit.edu) V1 2019-06-04 14:53:00: Since v5.2-rc1 OMAP is no longer using a special display driver architecture for DPI panels, but uses the general drm/panel/panel-simple. So we finally can add SoC independent panel definitions for two panel models which we already had worked on quite a while ago (before device tree was introduced): https://patchwork.kernel.org/patch/2851295/ H. Nikolaus Schaller (5): drm/panel: simple: Add Sharp LQ070Y3DG3B panel support drm/panel: simple: Add Ortustech COM37H3M panel support dt-bindings: drm/panel: simple: add ortustech,com37h3m05dtc panel dt-bindings: drm/panel: simple: add ortustech,com37h3m99dtc panel dt-bindings: drm/panel: simple: add sharp,lq070y3dg3b panel .../display/panel/ortustech,com37h3m05dtc.txt | 12 .../display/panel/ortustech,com37h3m99dtc.txt | 12 .../display/panel/sharp,lq070y3dg3b.txt | 12 drivers/gpu/drm/panel/panel-simple.c | 63 +++ 4 files changed, 99 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt create mode 100644 Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt -- 2.19.1
[PATCH v3 4/5] dt-bindings: drm/panel: simple: add ortustech,com37h3m99dtc panel
Signed-off-by: H. Nikolaus Schaller --- .../display/panel/ortustech,com37h3m99dtc.txt| 12 1 file changed, 12 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt diff --git a/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt b/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt new file mode 100644 index ..06a73c3f46b5 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m99dtc.txt @@ -0,0 +1,12 @@ +OrtusTech COM37H3M99DTC Blanview 3.7" VGA portrait TFT-LCD panel + +Required properties: +- compatible: should be "ortustech,com37h3m99dtc" + +Optional properties: +- enable-gpios: GPIO pin to enable or disable the panel +- backlight: phandle of the backlight device attached to the panel +- power-supply: phandle of the regulator that provides the supply voltage + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. -- 2.19.1
[PATCH v3 3/5] dt-bindings: drm/panel: simple: add ortustech,com37h3m05dtc panel
Signed-off-by: H. Nikolaus Schaller --- .../display/panel/ortustech,com37h3m05dtc.txt| 12 1 file changed, 12 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt diff --git a/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt b/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt new file mode 100644 index ..c16907c02f80 --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/ortustech,com37h3m05dtc.txt @@ -0,0 +1,12 @@ +OrtusTech COM37H3M05DTC Blanview 3.7" VGA portrait TFT-LCD panel + +Required properties: +- compatible: should be "ortustech,com37h3m05dtc" + +Optional properties: +- enable-gpios: GPIO pin to enable or disable the panel +- backlight: phandle of the backlight device attached to the panel +- power-supply: phandle of the regulator that provides the supply voltage + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. -- 2.19.1
[PATCH v3 5/5] dt-bindings: drm/panel: simple: add sharp,lq070y3dg3b panel
Signed-off-by: H. Nikolaus Schaller --- .../bindings/display/panel/sharp,lq070y3dg3b.txt | 12 1 file changed, 12 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt diff --git a/Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt b/Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt new file mode 100644 index ..95534b55ee5f --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/sharp,lq070y3dg3b.txt @@ -0,0 +1,12 @@ +Sharp LQ070Y3DG3B 7.0" WVGA landscape TFT LCD panel + +Required properties: +- compatible: should be "sharp,lq070y3dg3b" + +Optional properties: +- enable-gpios: GPIO pin to enable or disable the panel +- backlight: phandle of the backlight device attached to the panel +- power-supply: phandle of the regulator that provides the supply voltage + +This binding is compatible with the simple-panel binding, which is specified +in simple-panel.txt in this directory. -- 2.19.1
[PATCH v3 2/5] drm/panel: simple: Add Ortustech COM37H3M panel support
The change adds support for the Ortustech COM37H3M05DTC/99DTC 3.7" TFT LCD
panel.
Tested on Letux3704.
Signed-off-by: H. Nikolaus Schaller
---
drivers/gpu/drm/panel/panel-simple.c | 33
1 file changed, 33 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 5b27829c5a78..1fb74908a269 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2007,6 +2007,33 @@ static const struct panel_desc ontat_yx700wv03 = {
.bus_format = MEDIA_BUS_FMT_RGB666_1X18,
};
+static const struct drm_display_mode ortustech_com37h3m_mode = {
+ .clock = 22153,
+ .hdisplay = 480,
+ .hsync_start = 480 + 8,
+ .hsync_end = 480 + 8 + 10,
+ .htotal = 480 + 8 + 10 + 10,
+ .vdisplay = 640,
+ .vsync_start = 640 + 4,
+ .vsync_end = 640 + 4 + 3,
+ .vtotal = 640 + 4 + 3 + 4,
+ .vrefresh = 60,
+ .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC,
+};
+
+static const struct panel_desc ortustech_com37h3m = {
+ .modes = &ortustech_com37h3m_mode,
+ .num_modes = 1,
+ .bpc = 8,
+ .size = {
+ .width = 56,/* 56.16mm */
+ .height = 75, /* 74.88mm */
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+ .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE |
+DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
+};
+
static const struct drm_display_mode ortustech_com43h4m85ulc_mode = {
.clock = 25000,
.hdisplay = 480,
@@ -2786,6 +2813,12 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "ontat,yx700wv03",
.data = &ontat_yx700wv03,
+ }, {
+ .compatible = "ortustech,com37h3m05dtc",
+ .data = &ortustech_com37h3m,
+ }, {
+ .compatible = "ortustech,com37h3m99dtc",
+ .data = &ortustech_com37h3m,
}, {
.compatible = "ortustech,com43h4m85ulc",
.data = &ortustech_com43h4m85ulc,
--
2.19.1
[PATCH v3 1/5] drm/panel: simple: Add Sharp LQ070Y3DG3B panel support
The change adds support for the Sharp LQ070Y3DG3B 7.0" TFT LCD panel.
Tested on Letux7004.
Signed-off-by: H. Nikolaus Schaller
---
drivers/gpu/drm/panel/panel-simple.c | 30
1 file changed, 30 insertions(+)
diff --git a/drivers/gpu/drm/panel/panel-simple.c
b/drivers/gpu/drm/panel/panel-simple.c
index 569be4efd8d1..5b27829c5a78 100644
--- a/drivers/gpu/drm/panel/panel-simple.c
+++ b/drivers/gpu/drm/panel/panel-simple.c
@@ -2204,6 +2204,33 @@ static const struct panel_desc samsung_ltn140at29_301 = {
},
};
+static const struct drm_display_mode sharp_lq070y3dg3b_mode = {
+ .clock = 33260,
+ .hdisplay = 800,
+ .hsync_start = 800 + 64,
+ .hsync_end = 800 + 64 + 128,
+ .htotal = 800 + 64 + 128 + 64,
+ .vdisplay = 480,
+ .vsync_start = 480 + 8,
+ .vsync_end = 480 + 8 + 2,
+ .vtotal = 480 + 8 + 2 + 35,
+ .vrefresh = 60,
+ .flags = DISPLAY_FLAGS_PIXDATA_POSEDGE,
+};
+
+static const struct panel_desc sharp_lq070y3dg3b = {
+ .modes = &sharp_lq070y3dg3b_mode,
+ .num_modes = 1,
+ .bpc = 8,
+ .size = {
+ .width = 152, /* 152.4mm */
+ .height = 91, /* 91.4mm */
+ },
+ .bus_format = MEDIA_BUS_FMT_RGB888_1X24,
+ .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_POSEDGE |
+DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE,
+};
+
static const struct drm_display_mode sharp_lq035q7db03_mode = {
.clock = 5500,
.hdisplay = 240,
@@ -2786,6 +2813,9 @@ static const struct of_device_id platform_of_match[] = {
}, {
.compatible = "sharp,lq035q7db03",
.data = &sharp_lq035q7db03,
+ }, {
+ .compatible = "sharp,lq070y3dg3b",
+ .data = &sharp_lq070y3dg3b,
}, {
.compatible = "sharp,lq101k1ly04",
.data = &sharp_lq101k1ly04,
--
2.19.1
Re: [Intel-gfx] [PATCH] drm/crc-debugfs: Also sprinkle irqrestore over early exits
Le ven. 7 juin 2019 à 12:07, Emil Velikov a écrit :
>
> On Thu, 6 Jun 2019 at 22:15, Daniel Vetter wrote:
> >
> > I. was. blind.
> >
> > Caught with vkms, which has some really slow crc computation function.
> >
> > Fixes: 1882018a70e0 ("drm/crc-debugfs: User irqsafe spinlock in
> > drm_crtc_add_crc_entry")
> > Cc: Rodrigo Siqueira
> > Cc: Tomeu Vizoso
> > Cc: Emil Velikov
> > Cc: Benjamin Gaignard
> > Cc: Ville Syrjälä
> > Signed-off-by: Daniel Vetter
>
> Reviewed-by: Emil Velikov
Reviewed-by: Benjamin Gaignard
>
> -Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/5] dma-buf: heaps: Add heap helpers
Hi John,
On Fri, Jun 07, 2019 at 03:07:16AM +, John Stultz wrote:
> Add generic helper dmabuf ops for dma heaps, so we can reduce
> the amount of duplicative code for the exported dmabufs.
>
> This code is an evolution of the Android ION implementation, so
> thanks to its original authors and maintainters:
> Rebecca Schultz Zavin, Colin Cross, Laura Abbott, and others!
>
> Cc: Laura Abbott
> Cc: Benjamin Gaignard
> Cc: Sumit Semwal
> Cc: Liam Mark
> Cc: Pratik Patel
> Cc: Brian Starkey
> Cc: Vincent Donnefort
> Cc: Sudipto Paul
> Cc: Andrew F. Davis
> Cc: Christoph Hellwig
> Cc: Chenbo Feng
> Cc: Alistair Strachan
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Benjamin Gaignard
> Signed-off-by: John Stultz
> Change-Id: I48d43656e7783f266d877e363116b5187639f996
> ---
> v2:
> * Removed cache management performance hack that I had
> accidentally folded in.
> * Removed stats code that was in helpers
> * Lots of checkpatch cleanups
> v3:
> * Uninline INIT_HEAP_HELPER_BUFFER (suggested by Christoph)
> * Switch to WARN on buffer destroy failure (suggested by Brian)
> * buffer->kmap_cnt decrementing cleanup (suggested by Christoph)
> * Extra buffer->vaddr checking in dma_heap_dma_buf_kmap
> (suggested by Brian)
> * Switch to_helper_buffer from macro to inline function
> (suggested by Benjamin)
> * Rename kmap->vmap (folded in from Andrew)
> * Use vmap for vmapping - not begin_cpu_access (folded in from
> Andrew)
> * Drop kmap for now, as its optional (folded in from Andrew)
> * Fold dma_heap_map_user into the single caller (foled in from
> Andrew)
> * Folded in patch from Andrew to track page list per heap not
> sglist, which simplifies the tracking logic
> v4:
> * Moved dma-heap.h change out to previous patch
> ---
> drivers/dma-buf/Makefile | 1 +
> drivers/dma-buf/heaps/Makefile | 2 +
> drivers/dma-buf/heaps/heap-helpers.c | 261 +++
> drivers/dma-buf/heaps/heap-helpers.h | 55 ++
> 4 files changed, 319 insertions(+)
> create mode 100644 drivers/dma-buf/heaps/Makefile
> create mode 100644 drivers/dma-buf/heaps/heap-helpers.c
> create mode 100644 drivers/dma-buf/heaps/heap-helpers.h
>
> diff --git a/drivers/dma-buf/Makefile b/drivers/dma-buf/Makefile
> index 1cb3dd104825..e3e3dca29e46 100644
> --- a/drivers/dma-buf/Makefile
> +++ b/drivers/dma-buf/Makefile
> @@ -1,6 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0-only
> obj-y := dma-buf.o dma-fence.o dma-fence-array.o dma-fence-chain.o \
>reservation.o seqno-fence.o
> +obj-$(CONFIG_DMABUF_HEAPS) += heaps/
> obj-$(CONFIG_DMABUF_HEAPS) += dma-heap.o
> obj-$(CONFIG_SYNC_FILE) += sync_file.o
> obj-$(CONFIG_SW_SYNC)+= sw_sync.o sync_debug.o
> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> new file mode 100644
> index ..de49898112db
> --- /dev/null
> +++ b/drivers/dma-buf/heaps/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-y+= heap-helpers.o
> diff --git a/drivers/dma-buf/heaps/heap-helpers.c
> b/drivers/dma-buf/heaps/heap-helpers.c
> new file mode 100644
> index ..00cbdbbb97e5
> --- /dev/null
> +++ b/drivers/dma-buf/heaps/heap-helpers.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "heap-helpers.h"
> +
> +void INIT_HEAP_HELPER_BUFFER(struct heap_helper_buffer *buffer,
> + void (*free)(struct heap_helper_buffer *))
> +{
> + buffer->private_flags = 0;
> + buffer->priv_virt = NULL;
> + mutex_init(&buffer->lock);
> + buffer->vmap_cnt = 0;
> + buffer->vaddr = NULL;
> + INIT_LIST_HEAD(&buffer->attachments);
> + buffer->free = free;
Should pagecount and pages be initialised here too? I'm not sure what
the metric for picking which members are/aren't initialised is.
> +}
> +
extra newline
> +
> +static void *dma_heap_map_kernel(struct heap_helper_buffer *buffer)
> +{
> + void *vaddr;
> +
> + vaddr = vmap(buffer->pages, buffer->pagecount, VM_MAP, PAGE_KERNEL);
> + if (!vaddr)
> + return ERR_PTR(-ENOMEM);
> +
> + return vaddr;
> +}
> +
> +void dma_heap_buffer_destroy(struct dma_heap_buffer *heap_buffer)
can be static
> +{
> + struct heap_helper_buffer *buffer = to_helper_buffer(heap_buffer);
> +
> + if (buffer->vmap_cnt > 0) {
> + WARN("%s: buffer still mapped in the kernel\n",
> + __func__);
> + vunmap(buffer->vaddr);
> + }
> +
> + buffer->free(buffer);
> +}
> +
> +static void *dma_heap_buffer_vmap_get(struct dma_heap_buffer *heap_buffer)
> +{
> + struct heap_helper_buffer *buffer = to_helper_buffer(heap_buffer);
> + void *vaddr;
> +
> + if (buffer->vmap_cnt) {
> + buffer->vmap_cnt++;
> + return buffer-
Re: [PATCH] drm/stm: support runtime power management
Hi Yannick,
Thank you for your patch
Acked-by: Philippe Cornu
Philippe :-)
On 6/3/19 10:32 AM, Yannick Fertré wrote:
> This patch enables runtime power management (runtime PM) support for
> the display controller. pm_runtime_enable() and pm_runtime_disable()
> are added during ltdc load and unload respectively.
> pm_runtime_get_sync() and pm_runtime_put_sync() are added for ltdc
> register access.
>
> Signed-off-by: Yannick Fertré
> ---
> drivers/gpu/drm/stm/drv.c | 43 +++--
> drivers/gpu/drm/stm/ltdc.c | 67
> +++---
> 2 files changed, 86 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> index 5834ef5..5659572 100644
> --- a/drivers/gpu/drm/stm/drv.c
> +++ b/drivers/gpu/drm/stm/drv.c
> @@ -12,6 +12,7 @@
> #include
> #include
> #include
> +#include
>
> #include
> #include
> @@ -135,14 +136,15 @@ static __maybe_unused int drv_suspend(struct device
> *dev)
> struct ltdc_device *ldev = ddev->dev_private;
> struct drm_atomic_state *state;
>
> - drm_kms_helper_poll_disable(ddev);
> + if (WARN_ON(!ldev->suspend_state))
> + return -ENOENT;
> +
> state = drm_atomic_helper_suspend(ddev);
> - if (IS_ERR(state)) {
> - drm_kms_helper_poll_enable(ddev);
> + if (IS_ERR(state))
> return PTR_ERR(state);
> - }
> +
> ldev->suspend_state = state;
> - ltdc_suspend(ddev);
> + pm_runtime_force_suspend(dev);
>
> return 0;
> }
> @@ -151,16 +153,41 @@ static __maybe_unused int drv_resume(struct device *dev)
> {
> struct drm_device *ddev = dev_get_drvdata(dev);
> struct ltdc_device *ldev = ddev->dev_private;
> + int ret;
>
> - ltdc_resume(ddev);
> - drm_atomic_helper_resume(ddev, ldev->suspend_state);
> - drm_kms_helper_poll_enable(ddev);
> + pm_runtime_force_resume(dev);
> + ret = drm_atomic_helper_resume(ddev, ldev->suspend_state);
> + if (ret) {
> + pm_runtime_force_suspend(dev);
> + ldev->suspend_state = NULL;
> + return ret;
> + }
>
> return 0;
> }
>
> +static __maybe_unused int drv_runtime_suspend(struct device *dev)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> +
> + DRM_DEBUG_DRIVER("\n");
> + ltdc_suspend(ddev);
> +
> + return 0;
> +}
> +
> +static __maybe_unused int drv_runtime_resume(struct device *dev)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> +
> + DRM_DEBUG_DRIVER("\n");
> + return ltdc_resume(ddev);
> +}
> +
> static const struct dev_pm_ops drv_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(drv_suspend, drv_resume)
> + SET_RUNTIME_PM_OPS(drv_runtime_suspend,
> +drv_runtime_resume, NULL)
> };
>
> static int stm_drm_platform_probe(struct platform_device *pdev)
> diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
> index ac29890..a40870b 100644
> --- a/drivers/gpu/drm/stm/ltdc.c
> +++ b/drivers/gpu/drm/stm/ltdc.c
> @@ -16,6 +16,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #include
> @@ -444,6 +445,7 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc
> *crtc,
>struct drm_crtc_state *old_state)
> {
> struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> + struct drm_device *ddev = crtc->dev;
>
> DRM_DEBUG_DRIVER("\n");
>
> @@ -457,6 +459,8 @@ static void ltdc_crtc_atomic_disable(struct drm_crtc
> *crtc,
>
> /* immediately commit disable of layers before switching off LTDC */
> reg_set(ldev->regs, LTDC_SRCR, SRCR_IMR);
> +
> + pm_runtime_put_sync(ddev->dev);
> }
>
> #define CLK_TOLERANCE_HZ 50
> @@ -505,17 +509,31 @@ static bool ltdc_crtc_mode_fixup(struct drm_crtc *crtc,
>struct drm_display_mode *adjusted_mode)
> {
> struct ltdc_device *ldev = crtc_to_ltdc(crtc);
> + struct drm_device *ddev = crtc->dev;
> int rate = mode->clock * 1000;
> + bool runtime_active;
> + int ret;
> +
> + runtime_active = pm_runtime_active(ddev->dev);
> +
> + if (runtime_active)
> + pm_runtime_put_sync(ddev->dev);
>
> - clk_disable(ldev->pixel_clk);
> if (clk_set_rate(ldev->pixel_clk, rate) < 0) {
> DRM_ERROR("Cannot set rate (%dHz) for pixel clk\n", rate);
> return false;
> }
> - clk_enable(ldev->pixel_clk);
>
> adjusted_mode->clock = clk_get_rate(ldev->pixel_clk) / 1000;
>
> + if (runtime_active) {
> + ret = pm_runtime_get_sync(ddev->dev);
> + if (ret) {
> + DRM_ERROR("Failed to fixup mode, cannot get sync\n");
> + return false;
> + }
> + }
> +
> DRM_DEBUG_DRIVER("requested clock %dkHz, adjusted clock %dkHz\n",
>m
Re: [PATCH v5 3/5] dma-buf: heaps: Add system heap to dmabuf heaps
Hi,
On Fri, Jun 07, 2019 at 03:07:17AM +, John Stultz wrote:
> This patch adds system heap to the dma-buf heaps framework.
>
> This allows applications to get a page-allocator backed dma-buf
> for non-contiguous memory.
>
> This code is an evolution of the Android ION implementation, so
> thanks to its original authors and maintainters:
> Rebecca Schultz Zavin, Colin Cross, Laura Abbott, and others!
>
> Cc: Laura Abbott
> Cc: Benjamin Gaignard
> Cc: Sumit Semwal
> Cc: Liam Mark
> Cc: Pratik Patel
> Cc: Brian Starkey
> Cc: Vincent Donnefort
> Cc: Sudipto Paul
> Cc: Andrew F. Davis
> Cc: Christoph Hellwig
> Cc: Chenbo Feng
> Cc: Alistair Strachan
> Cc: dri-devel@lists.freedesktop.org
> Reviewed-by: Benjamin Gaignard
> Signed-off-by: John Stultz
> Change-Id: I4dc5ff54ccb1f7ca3ac8675661114ca33813654b
> ---
> v2:
> * Switch allocate to return dmabuf fd
> * Simplify init code
> * Checkpatch fixups
> * Droped dead system-contig code
> v3:
> * Whitespace fixups from Benjamin
> * Make sure we're zeroing the allocated pages (from Liam)
> * Use PAGE_ALIGN() consistently (suggested by Brian)
> * Fold in new registration style from Andrew
> * Avoid needless dynamic allocation of sys_heap (suggested by
> Christoph)
> * Minor cleanups
> * Folded in changes from Andrew to use simplified page list
> from the heap helpers
> v4:
> * Optimization to allocate pages in chunks, similar to old
> pagepool code
> * Use fd_flags when creating dmabuf fd (Suggested by Benjamin)
> v5:
> * Back out large order page allocations (was leaking memory,
> as the page array didn't properly track order size)
> ---
> drivers/dma-buf/Kconfig | 2 +
> drivers/dma-buf/heaps/Kconfig | 6 ++
> drivers/dma-buf/heaps/Makefile | 1 +
> drivers/dma-buf/heaps/system_heap.c | 123
> 4 files changed, 132 insertions(+)
> create mode 100644 drivers/dma-buf/heaps/Kconfig
> create mode 100644 drivers/dma-buf/heaps/system_heap.c
>
> diff --git a/drivers/dma-buf/Kconfig b/drivers/dma-buf/Kconfig
> index 9b93f86f597c..434cfe646dad 100644
> --- a/drivers/dma-buf/Kconfig
> +++ b/drivers/dma-buf/Kconfig
> @@ -47,4 +47,6 @@ menuconfig DMABUF_HEAPS
> this allows userspace to allocate dma-bufs that can be shared between
> drivers.
>
> +source "drivers/dma-buf/heaps/Kconfig"
> +
> endmenu
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> new file mode 100644
> index ..205052744169
> --- /dev/null
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -0,0 +1,6 @@
> +config DMABUF_HEAPS_SYSTEM
> + bool "DMA-BUF System Heap"
> + depends on DMABUF_HEAPS
> + help
> + Choose this option to enable the system dmabuf heap. The system heap
> + is backed by pages from the buddy allocator. If in doubt, say Y.
> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> index de49898112db..d1808eca2581 100644
> --- a/drivers/dma-buf/heaps/Makefile
> +++ b/drivers/dma-buf/heaps/Makefile
> @@ -1,2 +1,3 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-y+= heap-helpers.o
> +obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)+= system_heap.o
> diff --git a/drivers/dma-buf/heaps/system_heap.c
> b/drivers/dma-buf/heaps/system_heap.c
> new file mode 100644
> index ..863834499ce1
> --- /dev/null
> +++ b/drivers/dma-buf/heaps/system_heap.c
> @@ -0,0 +1,123 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DMABUF System heap exporter
> + *
> + * Copyright (C) 2011 Google, Inc.
> + * Copyright (C) 2019 Linaro Ltd.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "heap-helpers.h"
> +
> +struct system_heap {
> + struct dma_heap *heap;
> +} sys_heap;
> +
> +
extra newline. Otherwise, LGTM:
Reviewed-by: Brian Starkey
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH trivial] drm/i915: Grammar s/the its/its/
Signed-off-by: Geert Uytterhoeven
---
drivers/gpu/drm/i915/intel_dpll_mgr.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dpll_mgr.h
b/drivers/gpu/drm/i915/intel_dpll_mgr.h
index 8835dd20f1d27e05..79a53c5439a8e6db 100644
--- a/drivers/gpu/drm/i915/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/intel_dpll_mgr.h
@@ -293,7 +293,7 @@ struct intel_shared_dpll {
/**
* @state:
*
-* Store the state for the pll, including the its hw state
+* Store the state for the pll, including its hw state
* and CRTCs using it.
*/
struct intel_shared_dpll_state state;
--
2.17.1
Re: [PATCH 14/20] drm: rcar-du: Add support for CMM
Hi Jacopo,
Thank you for the patch.
On Thu, Jun 06, 2019 at 04:22:14PM +0200, Jacopo Mondi wrote:
> Add a driver for the R-Car Display Unit Color Correction Module.
>
> Each DU output channel is provided with a CMM unit to perform image
> enhancement and color correction.
I would say "On most Gen3 SoCs, each DU ..." as V3* SoCs have no CMM.
>
> Add support for CMM through a driver that supports configuration of
> the 1-dimensional LUT table. More advanced CMM feature could be
> implemented on top of this basic one.
s/could be/will be/ ? :-)
>
> Signed-off-by: Jacopo Mondi
> ---
> drivers/gpu/drm/rcar-du/Kconfig| 7 +
> drivers/gpu/drm/rcar-du/Makefile | 1 +
> drivers/gpu/drm/rcar-du/rcar_cmm.c | 197 +
> drivers/gpu/drm/rcar-du/rcar_cmm.h | 38 ++
> 4 files changed, 243 insertions(+)
> create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
> create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
>
> diff --git a/drivers/gpu/drm/rcar-du/Kconfig b/drivers/gpu/drm/rcar-du/Kconfig
> index 1529849e217e..539d232790d1 100644
> --- a/drivers/gpu/drm/rcar-du/Kconfig
> +++ b/drivers/gpu/drm/rcar-du/Kconfig
> @@ -13,6 +13,13 @@ config DRM_RCAR_DU
> Choose this option if you have an R-Car chipset.
> If M is selected the module will be called rcar-du-drm.
>
> +config DRM_RCAR_CMM
> + bool "R-Car DU Color Management Module (CMM) Support"
> + depends on DRM && OF
> + depends on DRM_RCAR_DU
> + help
> + Enable support for R-Car Color Management Module (CMM).
> +
> config DRM_RCAR_DW_HDMI
> tristate "R-Car DU Gen3 HDMI Encoder Support"
> depends on DRM && OF
> diff --git a/drivers/gpu/drm/rcar-du/Makefile
> b/drivers/gpu/drm/rcar-du/Makefile
> index 6c2ed9c46467..4d1187ccc3e5 100644
> --- a/drivers/gpu/drm/rcar-du/Makefile
> +++ b/drivers/gpu/drm/rcar-du/Makefile
> @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS) += rcar_du_of.o \
> rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
> rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
>
> +obj-$(CONFIG_DRM_RCAR_CMM) += rcar_cmm.o
> obj-$(CONFIG_DRM_RCAR_DU)+= rcar-du-drm.o
> obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o
> obj-$(CONFIG_DRM_RCAR_LVDS) += rcar_lvds.o
> diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c
> b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> new file mode 100644
> index ..5d9d917b91f4
> --- /dev/null
> +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> + *
> + * Copyright (C) 2019 Jacopo Mondi
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +#include "rcar_cmm.h"
> +
> +#define CM2_LUT_CTRL 0x00
I would write all register addresses with 3 (or 4) digits.
> +#define CM2_LUT_CTRL_EN BIT(0)
> +#define CM2_LUT_TBLA 0x600
I would define this as
#define CM2_LUT_TBLA(n) (0x600 + (n) * 4)
> +
> +struct rcar_cmm {
> + struct clk *clk;
> + void __iomem *base;
> + bool enabled;
> +
> + /* LUT table scratch buffer. */
> + struct {
> + bool restore;
> + unsigned int size;
> + uint32_t table[CMM_GAMMA_LUT_SIZE];
> + } lut;
> +};
> +
> +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> +{
> + return ioread32(rcmm->base + reg);
> +}
> +
> +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> +{
> + iowrite32(data, rcmm->base + reg);
> +}
> +
> +int rcar_cmm_setup(struct platform_device *pdev, struct rcar_cmm_config
> *config)
Please document the functions exposed to the DU driver. It's hard to
understand the setup vs. enable/disable split by just reading this
driver.
> +{
> + struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> + unsigned int i;
> +
> + if (config->lut.size > CMM_GAMMA_LUT_SIZE)
> + return -EINVAL;
> +
> + /*
> + * As cmm_setup is called by atomic commit tail helper, it might be
> + * called before the enabling the CRTC (which calls cmm_enable()).
> + *
> + * Store the LUT table entries in the scratch buffer to be later
> + * programmed at enable time.
> + */
> + if (!rcmm->enabled) {
> + if (!config->lut.enable)
> + return 0;
> +
> + for (i = 0; i < config->lut.size; ++i) {
> + struct drm_color_lut *lut = &config->lut.table[i];
> +
> + rcmm->lut.table[i] = (lut->red & 0xff) << 16 |
> + (lut->green & 0xff) << 8 |
> + (lut->blue & 0xff);
> + }
> +
> + rcmm->lut.restore = true;
> + rcmm->lut.size = config->lut.size;
> +
> + return 0;
> + }
> +
> + if (rcar_cmm_read(rcm
Re: [PATCH 03/20] dt-bindings: display, renesas, du: Update 'vsps' in example
Hi Geert,
On Thu, Jun 06, 2019 at 10:00:23PM +0200, Geert Uytterhoeven wrote:
> On Thu, Jun 6, 2019 at 8:50 PM Laurent Pinchart wrote:
> > On Thu, Jun 06, 2019 at 04:22:03PM +0200, Jacopo Mondi wrote:
> >> Update the 'vsps' property structure in the documentation example to
> >> reflect what's actually implemented in the device tree sources.
> >>
> >> Signed-off-by: Jacopo Mondi
>
> >> --- a/Documentation/devicetree/bindings/display/renesas,du.txt
> >> +++ b/Documentation/devicetree/bindings/display/renesas,du.txt
> >> @@ -92,7 +92,7 @@ Example: R8A7795 (R-Car H3) ES2.0 DU
> >><&cpg CPG_MOD 722>,
> >><&cpg CPG_MOD 721>;
> >> clock-names = "du.0", "du.1", "du.2", "du.3";
> >> - vsps = <&vspd0 0>, <&vspd1 0>, <&vspd2 0>, <&vspd0 1>;
> >> + vsps = <&vspd0 0 &vspd1 0 &vspd2 &vspd0 1>;
> >
> > The former is simpler to read than the latter in my opinion. Shouldn't
> > we update the .dtsi files instead ?
>
> Yes, it is easier to read (for humans).
>
> >> cmms = <&cmm0 &cmm1 &cmm2 &cmm3>;
>
> Perhaps we want grouping here, too?
As there's a single entry per CMM it matters less in my opinion. I'm
fine with either options.
>
> >>
> >> ports {
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 15/20] drm: rcar-du: Claim CMM support for Gen3 SoCs
Hi Jacopo,
Thank you for the patch.
On Thu, Jun 06, 2019 at 04:22:15PM +0200, Jacopo Mondi wrote:
> Add CMM to the list of supported features for Gen3 SoCs that provide it:
> - R8A7795
> - R8A7796
> - R8A77965
> - R8A7799x
>
> Leave R8A77970 out as V3M and V3H are the only Gen3 SoCs that do not
> support CMM.
>
> Signed-off-by: Jacopo Mondi
Do we actually need this ? Could we just skip CMM handling if the cmms
property isn't set in DT ?
> ---
> drivers/gpu/drm/rcar-du/rcar_du_drv.c | 12
> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 1 +
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> index 75ab17af13a9..1e69cfa11798 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.c
> @@ -247,7 +247,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a7795_info = {
> .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> - | RCAR_DU_FEATURE_TVM_SYNC,
> + | RCAR_DU_FEATURE_TVM_SYNC
> + | RCAR_DU_FEATURE_CMM,
> .channels_mask = BIT(3) | BIT(2) | BIT(1) | BIT(0),
> .routes = {
> /*
> @@ -280,7 +281,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a7796_info = {
> .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> - | RCAR_DU_FEATURE_TVM_SYNC,
> + | RCAR_DU_FEATURE_TVM_SYNC
> + | RCAR_DU_FEATURE_CMM,
> .channels_mask = BIT(2) | BIT(1) | BIT(0),
> .routes = {
> /*
> @@ -309,7 +311,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a77965_info = {
> .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> | RCAR_DU_FEATURE_VSP1_SOURCE
> | RCAR_DU_FEATURE_INTERLACED
> - | RCAR_DU_FEATURE_TVM_SYNC,
> + | RCAR_DU_FEATURE_TVM_SYNC
> + | RCAR_DU_FEATURE_CMM,
> .channels_mask = BIT(3) | BIT(1) | BIT(0),
> .routes = {
> /*
> @@ -357,7 +360,8 @@ static const struct rcar_du_device_info
> rcar_du_r8a77970_info = {
> static const struct rcar_du_device_info rcar_du_r8a7799x_info = {
> .gen = 3,
> .features = RCAR_DU_FEATURE_CRTC_IRQ_CLOCK
> - | RCAR_DU_FEATURE_VSP1_SOURCE,
> + | RCAR_DU_FEATURE_VSP1_SOURCE
> + | RCAR_DU_FEATURE_CMM,
> .channels_mask = BIT(1) | BIT(0),
> .routes = {
> /*
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index 1327cd0df90a..a00dccc447aa 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -28,6 +28,7 @@ struct rcar_du_encoder;
> #define RCAR_DU_FEATURE_VSP1_SOURCE BIT(1) /* Has inputs from VSP1 */
> #define RCAR_DU_FEATURE_INTERLACED BIT(2) /* HW supports interlaced */
> #define RCAR_DU_FEATURE_TVM_SYNC BIT(3) /* Has TV switch/sync modes */
> +#define RCAR_DU_FEATURE_CMM BIT(4) /* Has CMM */
>
> #define RCAR_DU_QUIRK_ALIGN_128B BIT(0) /* Align pitches to 128 bytes */
>
--
Regards,
Laurent Pinchart
Re: [PATCH v2] linux-firmware: update firmware for mhdp8546
On Mon, Jun 3, 2019 at 7:06 AM Damian Kos wrote: > > This patch updates mhdp8546 firmware from v1.2.12 to v1.2.15. > > Added support for performing arbitrary I2C-over-AUX transactions. > Corrected handling of erroneous requests for reading/writing 0 bytes over > DPCD. > Add mailbox message to force uCPU fatal error. > > Signed-off-by: Damian Kos > --- > WHENCE | 2 +- > cadence/mhdp8546.bin | Bin 131072 -> 131072 bytes > 2 files changed, 1 insertion(+), 1 deletion(-) Applied and pushed out. josh ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/20] drm: rcar-du: Add support for CMM
Hi Jacopo,
On Fri, Jun 07, 2019 at 02:35:40PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Thu, Jun 06, 2019 at 04:22:14PM +0200, Jacopo Mondi wrote:
> > Add a driver for the R-Car Display Unit Color Correction Module.
> >
> > Each DU output channel is provided with a CMM unit to perform image
> > enhancement and color correction.
>
> I would say "On most Gen3 SoCs, each DU ..." as V3* SoCs have no CMM.
>
> >
> > Add support for CMM through a driver that supports configuration of
> > the 1-dimensional LUT table. More advanced CMM feature could be
> > implemented on top of this basic one.
>
> s/could be/will be/ ? :-)
>
> >
> > Signed-off-by: Jacopo Mondi
> > ---
> > drivers/gpu/drm/rcar-du/Kconfig| 7 +
> > drivers/gpu/drm/rcar-du/Makefile | 1 +
> > drivers/gpu/drm/rcar-du/rcar_cmm.c | 197 +
> > drivers/gpu/drm/rcar-du/rcar_cmm.h | 38 ++
> > 4 files changed, 243 insertions(+)
> > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
> > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> >
> > diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> > b/drivers/gpu/drm/rcar-du/Kconfig
> > index 1529849e217e..539d232790d1 100644
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -13,6 +13,13 @@ config DRM_RCAR_DU
> > Choose this option if you have an R-Car chipset.
> > If M is selected the module will be called rcar-du-drm.
> >
> > +config DRM_RCAR_CMM
> > + bool "R-Car DU Color Management Module (CMM) Support"
> > + depends on DRM && OF
> > + depends on DRM_RCAR_DU
> > + help
> > + Enable support for R-Car Color Management Module (CMM).
> > +
> > config DRM_RCAR_DW_HDMI
> > tristate "R-Car DU Gen3 HDMI Encoder Support"
> > depends on DRM && OF
> > diff --git a/drivers/gpu/drm/rcar-du/Makefile
> > b/drivers/gpu/drm/rcar-du/Makefile
> > index 6c2ed9c46467..4d1187ccc3e5 100644
> > --- a/drivers/gpu/drm/rcar-du/Makefile
> > +++ b/drivers/gpu/drm/rcar-du/Makefile
> > @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS) += rcar_du_of.o
> > \
> > rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
> > rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
> >
> > +obj-$(CONFIG_DRM_RCAR_CMM) += rcar_cmm.o
> > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du-drm.o
> > obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o
> > obj-$(CONFIG_DRM_RCAR_LVDS)+= rcar_lvds.o
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > new file mode 100644
> > index ..5d9d917b91f4
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > @@ -0,0 +1,197 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> > + *
> > + * Copyright (C) 2019 Jacopo Mondi
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +
> > +#include "rcar_cmm.h"
> > +
> > +#define CM2_LUT_CTRL 0x00
>
> I would write all register addresses with 3 (or 4) digits.
>
> > +#define CM2_LUT_CTRL_ENBIT(0)
> > +#define CM2_LUT_TBLA 0x600
>
> I would define this as
>
> #define CM2_LUT_TBLA(n) (0x600 + (n) * 4)
>
> > +
> > +struct rcar_cmm {
> > + struct clk *clk;
> > + void __iomem *base;
> > + bool enabled;
> > +
> > + /* LUT table scratch buffer. */
> > + struct {
> > + bool restore;
> > + unsigned int size;
> > + uint32_t table[CMM_GAMMA_LUT_SIZE];
> > + } lut;
> > +};
> > +
> > +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> > +{
> > + return ioread32(rcmm->base + reg);
> > +}
> > +
> > +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> > +{
> > + iowrite32(data, rcmm->base + reg);
> > +}
> > +
> > +int rcar_cmm_setup(struct platform_device *pdev, struct rcar_cmm_config
> > *config)
>
> Please document the functions exposed to the DU driver. It's hard to
> understand the setup vs. enable/disable split by just reading this
> driver.
>
> > +{
> > + struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > + unsigned int i;
> > +
> > + if (config->lut.size > CMM_GAMMA_LUT_SIZE)
> > + return -EINVAL;
> > +
> > + /*
> > +* As cmm_setup is called by atomic commit tail helper, it might be
> > +* called before the enabling the CRTC (which calls cmm_enable()).
> > +*
> > +* Store the LUT table entries in the scratch buffer to be later
> > +* programmed at enable time.
> > +*/
> > + if (!rcmm->enabled) {
> > + if (!config->lut.enable)
> > + return 0;
> > +
> > + for (i = 0; i < config->lut.size; ++i) {
> > + struct drm_color_lut *lut = &config->lut.table[i];
> > +
> > + rcmm->lut.table[i] = (lut->red & 0xff) <
Re: [PATCH 16/20] drm: rcar-du: kms: Collect CMM instances
Hi Jacopo,
Thank you for the patch.
On Thu, Jun 06, 2019 at 04:22:16PM +0200, Jacopo Mondi wrote:
> Implement device tree parsing to collect the available CMM instances
> described by the 'cmms' property. Associate CMMs with CRTCs and store a
> mask of active CMMs in the DU group for later enablement.
>
> Also define a new feature to let each SoC claim support for CMM.
The feature is added in the previous patch.
> Signed-off-by: Jacopo Mondi
> ---
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 7
> drivers/gpu/drm/rcar-du/rcar_du_crtc.h | 2 +
> drivers/gpu/drm/rcar-du/rcar_du_drv.h | 3 ++
> drivers/gpu/drm/rcar-du/rcar_du_group.h | 2 +
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 50 +
> 5 files changed, 64 insertions(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index 2da46e3dc4ae..9f270a54b164 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -1194,6 +1194,13 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int swindex,
> if (ret < 0)
> return ret;
>
> + /* CMM might be disabled for this CRTC. */
> + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CMM) &&
> + rcdu->cmms[swindex]) {
You can skip testing RCAR_DU_FEATURE_CMM here as rcdu->cmms[swindex]
can't be set if the feature isn't available.
> + rcrtc->cmm = rcdu->cmms[swindex];
> + rgrp->cmms_mask |= BIT(hwindex % 2);
> + }
> +
> drm_crtc_helper_add(crtc, &crtc_helper_funcs);
>
> /* Start with vertical blanking interrupt reporting disabled. */
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> index 3b7fc668996f..5f2940c42225 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.h
> @@ -39,6 +39,7 @@ struct rcar_du_vsp;
> * @vblank_wait: wait queue used to signal vertical blanking
> * @vblank_count: number of vertical blanking interrupts to wait for
> * @group: CRTC group this CRTC belongs to
> + * @cmm: CMM associated with this CRTC
> * @vsp: VSP feeding video to this CRTC
> * @vsp_pipe: index of the VSP pipeline feeding video to this CRTC
> * @writeback: the writeback connector
> @@ -64,6 +65,7 @@ struct rcar_du_crtc {
> unsigned int vblank_count;
>
> struct rcar_du_group *group;
> + struct platform_device *cmm;
> struct rcar_du_vsp *vsp;
> unsigned int vsp_pipe;
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> index a00dccc447aa..300ec60ba31b 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_drv.h
> @@ -13,6 +13,7 @@
> #include
> #include
>
> +#include "rcar_cmm.h"
> #include "rcar_du_crtc.h"
> #include "rcar_du_group.h"
> #include "rcar_du_vsp.h"
> @@ -70,6 +71,7 @@ struct rcar_du_device_info {
>
> #define RCAR_DU_MAX_CRTCS4
> #define RCAR_DU_MAX_GROUPS DIV_ROUND_UP(RCAR_DU_MAX_CRTCS, 2)
> +#define RCAR_DU_MAX_CMMS 4
> #define RCAR_DU_MAX_VSPS 4
>
> struct rcar_du_device {
> @@ -86,6 +88,7 @@ struct rcar_du_device {
> struct rcar_du_encoder *encoders[RCAR_DU_OUTPUT_MAX];
>
> struct rcar_du_group groups[RCAR_DU_MAX_GROUPS];
> + struct platform_device *cmms[RCAR_DU_MAX_CMMS];
> struct rcar_du_vsp vsps[RCAR_DU_MAX_VSPS];
>
> struct {
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.h
> b/drivers/gpu/drm/rcar-du/rcar_du_group.h
> index 87950c1f6a52..b0c1466593a3 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.h
> @@ -22,6 +22,7 @@ struct rcar_du_device;
> * @mmio_offset: registers offset in the device memory map
> * @index: group index
> * @channels_mask: bitmask of populated DU channels in this group
> + * @cmms_mask: bitmask of enabled CMMs in this group
> * @num_crtcs: number of CRTCs in this group (1 or 2)
> * @use_count: number of users of the group (rcar_du_group_(get|put))
> * @used_crtcs: number of CRTCs currently in use
> @@ -37,6 +38,7 @@ struct rcar_du_group {
> unsigned int index;
>
> unsigned int channels_mask;
> + unsigned int cmms_mask;
> unsigned int num_crtcs;
> unsigned int use_count;
> unsigned int used_crtcs;
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index adbc4f5d8fc5..5a910a04e1d9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -18,6 +18,7 @@
> #include
>
> #include
> +#include
> #include
>
> #include "rcar_du_crtc.h"
> @@ -614,6 +615,48 @@ static int rcar_du_vsps_init(struct rcar_du_device *rcdu)
> return ret;
> }
>
> +static int rcar_du_cmm_init(struct rcar_du_device *rcdu)
> +{
> + const struct device_node *np = rcdu->dev->o
Re: [PATCH 17/20] drm: rcar-du: crtc: Enable and disable CMMs
Hi Jacopo, Thank you for the patch. On Thu, Jun 06, 2019 at 04:22:17PM +0200, Jacopo Mondi wrote: > Enable/disable the CMM associated with a CRTC at > atomic_enable()/atomic_disable() time. I would squash this patch with the next 3. It's hard to review them independently. > Signed-off-by: Jacopo Mondi > --- > drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > index 9f270a54b164..e6d3df37c827 100644 > --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c > @@ -21,6 +21,7 @@ > #include > #include > > +#include "rcar_cmm.h" > #include "rcar_du_crtc.h" > #include "rcar_du_drv.h" > #include "rcar_du_encoder.h" > @@ -523,6 +524,7 @@ static int rcar_du_crtc_get(struct rcar_du_crtc *rcrtc) > goto error_group; > > rcar_du_crtc_setup(rcrtc); > + Unrelated change ? > rcrtc->initialized = true; > > return 0; > @@ -619,6 +621,9 @@ static void rcar_du_crtc_stop(struct rcar_du_crtc *rcrtc) > if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_VSP1_SOURCE)) > rcar_du_vsp_disable(rcrtc); > > + if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_CMM) && rcrtc->cmm) You can just test rcrtc->cmm. Same below. > + rcar_cmm_disable(rcrtc->cmm); > + > /* >* Select switch sync mode. This stops display operation and configures >* the HSYNC and VSYNC signals as inputs. > @@ -686,6 +691,9 @@ static void rcar_du_crtc_atomic_enable(struct drm_crtc > *crtc, > } > > rcar_du_crtc_start(rcrtc); > + > + if (rcar_du_has(rcrtc->dev, RCAR_DU_FEATURE_CMM) && rcrtc->cmm) > + rcar_cmm_enable(rcrtc->cmm); > } > > static void rcar_du_crtc_atomic_disable(struct drm_crtc *crtc, -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] video: fbdev: mxsfb: Remove driver
On 5/20/19 3:43 PM, Marek Vasut wrote: > On 5/20/19 3:06 PM, Fabio Estevam wrote: >> There is a DRM version of the mxsfb driver for quite some time >> at drivers/gpu/drm/mxsfb/, so there is no need to keep maintaining >> the fbdev version any longer. >> >> Remove the fbdev mxsfb driver in favour of the DRM version. >> >> Signed-off-by: Fabio Estevam > > Acked-by: Marek Vasut Patch queued for v5.3, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 18/20] drm: rcar-du: group: Enable DU's CMM extension
Hi Jacopo,
Thank you for the patch.
On Thu, Jun 06, 2019 at 04:22:18PM +0200, Jacopo Mondi wrote:
> Enable the CMM units through the display unit extensional function control
> group register DEFR7.
>
> Signed-off-by: Jacopo Mondi
> ---
> drivers/gpu/drm/rcar-du/rcar_du_group.c | 8
> drivers/gpu/drm/rcar-du/rcar_du_regs.h | 5 +
> 2 files changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> index 9eee47969e77..d252c9bb9809 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_group.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_group.c
> @@ -147,6 +147,14 @@ static void rcar_du_group_setup(struct rcar_du_group
> *rgrp)
>
> rcar_du_group_setup_pins(rgrp);
>
> + if (rcar_du_has(rcdu, RCAR_DU_FEATURE_CMM)) {
This is a good enough reason to keep RCAR_DU_FEATURE_CMM :-/
> + u32 defr7 = DEFR7_CODE |
> + (rgrp->cmms_mask & BIT(1) ? DEFR7_CMME1 : 0) |
> + (rgrp->cmms_mask & BIT(0) ? DEFR7_CMME0 : 0);
> +
> + rcar_du_group_write(rgrp, DEFR7, defr7);
It would be nice to disable the CMM when the LUT isn't used, but that
would be difficult at the moment. We can revisit this when Kieran's DU
group handling series will land.
Reviewed-by: Laurent Pinchart
> + }
> +
> if (rcdu->info->gen >= 2) {
> rcar_du_group_setup_defr8(rgrp);
> rcar_du_group_setup_didsr(rgrp);
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> index bc87f080b170..fb9964949368 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_regs.h
> @@ -197,6 +197,11 @@
> #define DEFR6_MLOS1 (1 << 2)
> #define DEFR6_DEFAULT(DEFR6_CODE | DEFR6_TCNE1)
>
> +#define DEFR70x000ec
> +#define DEFR7_CODE (0x7779 << 16)
> +#define DEFR7_CMME1 BIT(6)
> +#define DEFR7_CMME0 BIT(4)
> +
> /*
> -
> * R8A7790-only Control Registers
> */
--
Regards,
Laurent Pinchart
Re: [PATCH 19/20] drm: rcar-du: crtc: Register GAMMA_LUT properties
Hi Jacopo,
Thank you for the patch.
On Thu, Jun 06, 2019 at 04:22:19PM +0200, Jacopo Mondi wrote:
> Enable the GAMMA_LUT KMS property using the framework helpers to
> register the proeprty and the associated gamma table size maximum size.
>
> Signed-off-by: Jacopo Mondi
> ---
> drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> index e6d3df37c827..c920fb5dba65 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> @@ -1207,6 +1207,9 @@ int rcar_du_crtc_create(struct rcar_du_group *rgrp,
> unsigned int swindex,
> rcdu->cmms[swindex]) {
> rcrtc->cmm = rcdu->cmms[swindex];
> rgrp->cmms_mask |= BIT(hwindex % 2);
> +
> + drm_mode_crtc_set_gamma_size(crtc, CMM_GAMMA_LUT_SIZE);
> + drm_crtc_enable_color_mgmt(crtc, 0, false, CMM_GAMMA_LUT_SIZE);
This change looks good, but you also need to add support for legacy API.
According to the function's documentation,
* Drivers should use drm_atomic_helper_legacy_gamma_set() to implement the
* legacy &drm_crtc_funcs.gamma_set callback.
> }
>
> drm_crtc_helper_add(crtc, &crtc_helper_funcs);
>
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/8] drivers: (video|gpu): fix warning same module names
On 6/7/19 9:57 AM, Daniel Vetter wrote: > On Thu, Jun 06, 2019 at 11:47:12AM +0200, Anders Roxell wrote: >> When building with CONFIG_DRM_MXSFB and CONFIG_FB_MXS enabled as >> loadable modules, we see the following warning: >> >> warning: same module names found: >> drivers/video/fbdev/mxsfb.ko >> drivers/gpu/drm/mxsfb/mxsfb.ko >> >> Rework so the names matches the config fragment. >> >> Signed-off-by: Anders Roxell > > Reviewed-by: Daniel Vetter > > I'm assuming Bart will pick this one up for fbdev. The DRM mxsfb has been a default for almost a year (since July 2018) and I've just applied "[PATCH] video: fbdev: mxsfb: Remove driver" (https://marc.info/?l=dri-devel&m=155835758115686&w=2) so it seems that this patch is not needed any longer (sorry!). > -Daniel > >> --- >> drivers/gpu/drm/mxsfb/Makefile | 4 ++-- >> drivers/video/fbdev/Makefile | 3 ++- >> 2 files changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile >> index ff6e358088fa..5d49d7548e66 100644 >> --- a/drivers/gpu/drm/mxsfb/Makefile >> +++ b/drivers/gpu/drm/mxsfb/Makefile >> @@ -1,3 +1,3 @@ >> # SPDX-License-Identifier: GPL-2.0-only >> -mxsfb-y := mxsfb_drv.o mxsfb_crtc.o mxsfb_out.o >> -obj-$(CONFIG_DRM_MXSFB) += mxsfb.o >> +drm-mxsfb-y := mxsfb_drv.o mxsfb_crtc.o mxsfb_out.o >> +obj-$(CONFIG_DRM_MXSFB) += drm-mxsfb.o >> diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile >> index 655f2537cac1..7ee967525af2 100644 >> --- a/drivers/video/fbdev/Makefile >> +++ b/drivers/video/fbdev/Makefile >> @@ -131,7 +131,8 @@ obj-$(CONFIG_FB_VGA16)+= vga16fb.o >> obj-$(CONFIG_FB_OF) += offb.o >> obj-$(CONFIG_FB_MX3) += mx3fb.o >> obj-$(CONFIG_FB_DA8XX)+= da8xx-fb.o >> -obj-$(CONFIG_FB_MXS) += mxsfb.o >> +obj-$(CONFIG_FB_MXS) += fb-mxs.o >> +fb-mxs-objs := mxsfb.o >> obj-$(CONFIG_FB_SSD1307) += ssd1307fb.o >> obj-$(CONFIG_FB_SIMPLE) += simplefb.o >> >> -- >> 2.20.1 Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Re: [PATCH 20/20] drm: rcar-du: kms: Update CMM in atomic commit tail
Hi Jacopo,
Thank you for the patch.
On Thu, Jun 06, 2019 at 04:22:20PM +0200, Jacopo Mondi wrote:
> Update CMM settings at in the atomic commit tail helper method.
>
> The CMM is updated with new gamma values provided to the driver
> in the GAMMA_LUT blob property.
>
> Signed-off-by: Jacopo Mondi
> ---
> drivers/gpu/drm/rcar-du/rcar_du_kms.c | 36 +++
> 1 file changed, 36 insertions(+)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> index 5a910a04e1d9..29a2020a46b5 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_kms.c
> @@ -21,6 +21,7 @@
> #include
> #include
>
> +#include "rcar_cmm.h"
> #include "rcar_du_crtc.h"
> #include "rcar_du_drv.h"
> #include "rcar_du_encoder.h"
> @@ -367,6 +368,38 @@ rcar_du_fb_create(struct drm_device *dev, struct
> drm_file *file_priv,
> * Atomic Check and Update
> */
>
> +static void rcar_du_atomic_commit_update_cmm(struct drm_crtc *crtc,
> + struct drm_crtc_state *old_state)
> +{
> + struct rcar_du_crtc *rcrtc = to_rcar_crtc(crtc);
> + struct rcar_cmm_config cmm_config = {};
> +
> + if (!rcrtc->cmm || !crtc->state->color_mgmt_changed)
> + return;
> +
> + if (!crtc->state->gamma_lut) {
> + cmm_config.lut.enable = false;
> + rcar_cmm_setup(rcrtc->cmm, &cmm_config);
> +
> + return;
> + }
> +
> + cmm_config.lut.enable = true;
> + cmm_config.lut.table = (struct drm_color_lut *)
> +crtc->state->gamma_lut->data;
> +
> + /* Set LUT table size to 0 if entries should not be updated. */
> + if (!old_state->gamma_lut ||
> + (old_state->gamma_lut->base.id !=
> + crtc->state->gamma_lut->base.id))
> + cmm_config.lut.size = crtc->state->gamma_lut->length
> + / sizeof(cmm_config.lut.table[0]);
Do you need to call rcar_cmm_setup() at all in this case ?
> + else
> + cmm_config.lut.size = 0;
> +
> + rcar_cmm_setup(rcrtc->cmm, &cmm_config);
> +}
> +
> static int rcar_du_atomic_check(struct drm_device *dev,
> struct drm_atomic_state *state)
> {
> @@ -409,6 +442,9 @@ static void rcar_du_atomic_commit_tail(struct
> drm_atomic_state *old_state)
> rcdu->dpad1_source = rcrtc->index;
> }
>
> + for_each_old_crtc_in_state(old_state, crtc, crtc_state, i)
> + rcar_du_atomic_commit_update_cmm(crtc, crtc_state);
> +
> /* Apply the atomic update. */
> drm_atomic_helper_commit_modeset_disables(dev, old_state);
> drm_atomic_helper_commit_planes(dev, old_state,
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 14/20] drm: rcar-du: Add support for CMM
Hi Jacopo,
Now that I've read the whole series, here are a few comments.
On Fri, Jun 07, 2019 at 02:35:40PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Thu, Jun 06, 2019 at 04:22:14PM +0200, Jacopo Mondi wrote:
> > Add a driver for the R-Car Display Unit Color Correction Module.
> >
> > Each DU output channel is provided with a CMM unit to perform image
> > enhancement and color correction.
>
> I would say "On most Gen3 SoCs, each DU ..." as V3* SoCs have no CMM.
>
> >
> > Add support for CMM through a driver that supports configuration of
> > the 1-dimensional LUT table. More advanced CMM feature could be
> > implemented on top of this basic one.
>
> s/could be/will be/ ? :-)
>
> >
> > Signed-off-by: Jacopo Mondi
> > ---
> > drivers/gpu/drm/rcar-du/Kconfig| 7 +
> > drivers/gpu/drm/rcar-du/Makefile | 1 +
> > drivers/gpu/drm/rcar-du/rcar_cmm.c | 197 +
> > drivers/gpu/drm/rcar-du/rcar_cmm.h | 38 ++
> > 4 files changed, 243 insertions(+)
> > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.c
> > create mode 100644 drivers/gpu/drm/rcar-du/rcar_cmm.h
> >
> > diff --git a/drivers/gpu/drm/rcar-du/Kconfig
> > b/drivers/gpu/drm/rcar-du/Kconfig
> > index 1529849e217e..539d232790d1 100644
> > --- a/drivers/gpu/drm/rcar-du/Kconfig
> > +++ b/drivers/gpu/drm/rcar-du/Kconfig
> > @@ -13,6 +13,13 @@ config DRM_RCAR_DU
> > Choose this option if you have an R-Car chipset.
> > If M is selected the module will be called rcar-du-drm.
> >
> > +config DRM_RCAR_CMM
> > + bool "R-Car DU Color Management Module (CMM) Support"
> > + depends on DRM && OF
> > + depends on DRM_RCAR_DU
> > + help
> > + Enable support for R-Car Color Management Module (CMM).
> > +
> > config DRM_RCAR_DW_HDMI
> > tristate "R-Car DU Gen3 HDMI Encoder Support"
> > depends on DRM && OF
> > diff --git a/drivers/gpu/drm/rcar-du/Makefile
> > b/drivers/gpu/drm/rcar-du/Makefile
> > index 6c2ed9c46467..4d1187ccc3e5 100644
> > --- a/drivers/gpu/drm/rcar-du/Makefile
> > +++ b/drivers/gpu/drm/rcar-du/Makefile
> > @@ -15,6 +15,7 @@ rcar-du-drm-$(CONFIG_DRM_RCAR_LVDS) += rcar_du_of.o
> > \
> > rcar-du-drm-$(CONFIG_DRM_RCAR_VSP) += rcar_du_vsp.o
> > rcar-du-drm-$(CONFIG_DRM_RCAR_WRITEBACK) += rcar_du_writeback.o
> >
> > +obj-$(CONFIG_DRM_RCAR_CMM) += rcar_cmm.o
> > obj-$(CONFIG_DRM_RCAR_DU) += rcar-du-drm.o
> > obj-$(CONFIG_DRM_RCAR_DW_HDMI) += rcar_dw_hdmi.o
> > obj-$(CONFIG_DRM_RCAR_LVDS)+= rcar_lvds.o
> > diff --git a/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > new file mode 100644
> > index ..5d9d917b91f4
> > --- /dev/null
> > +++ b/drivers/gpu/drm/rcar-du/rcar_cmm.c
> > @@ -0,0 +1,197 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * rcar_cmm.c -- R-Car Display Unit Color Management Module
> > + *
> > + * Copyright (C) 2019 Jacopo Mondi
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +
> > +#include "rcar_cmm.h"
> > +
> > +#define CM2_LUT_CTRL 0x00
>
> I would write all register addresses with 3 (or 4) digits.
>
> > +#define CM2_LUT_CTRL_ENBIT(0)
> > +#define CM2_LUT_TBLA 0x600
>
> I would define this as
>
> #define CM2_LUT_TBLA(n) (0x600 + (n) * 4)
>
> > +
> > +struct rcar_cmm {
> > + struct clk *clk;
> > + void __iomem *base;
> > + bool enabled;
> > +
> > + /* LUT table scratch buffer. */
> > + struct {
> > + bool restore;
> > + unsigned int size;
> > + uint32_t table[CMM_GAMMA_LUT_SIZE];
> > + } lut;
> > +};
> > +
> > +static inline int rcar_cmm_read(struct rcar_cmm *rcmm, u32 reg)
> > +{
> > + return ioread32(rcmm->base + reg);
> > +}
> > +
> > +static inline void rcar_cmm_write(struct rcar_cmm *rcmm, u32 reg, u32 data)
> > +{
> > + iowrite32(data, rcmm->base + reg);
> > +}
> > +
> > +int rcar_cmm_setup(struct platform_device *pdev, struct rcar_cmm_config
> > *config)
>
> Please document the functions exposed to the DU driver. It's hard to
> understand the setup vs. enable/disable split by just reading this
> driver.
>
> > +{
> > + struct rcar_cmm *rcmm = platform_get_drvdata(pdev);
> > + unsigned int i;
> > +
> > + if (config->lut.size > CMM_GAMMA_LUT_SIZE)
> > + return -EINVAL;
> > +
> > + /*
> > +* As cmm_setup is called by atomic commit tail helper, it might be
> > +* called before the enabling the CRTC (which calls cmm_enable()).
> > +*
> > +* Store the LUT table entries in the scratch buffer to be later
> > +* programmed at enable time.
> > +*/
I think we'll be able to simplify this once Kieran's DU group handling
patches land. Let's see which series gets merged first :-)
> > + if (!rcmm->enabled) {
> > + if (!config->lut.enable)
> > + return 0;
> > +
> > +
Re: [PATCH] video: fbdev: atafb: remove superfluous function prototypes
On 5/21/19 4:12 PM, Geert Uytterhoeven wrote: > On Tue, May 21, 2019 at 4:02 PM Bartlomiej Zolnierkiewicz > wrote: >> No need for them. >> >> Cc: Geert Uytterhoeven >> Signed-off-by: Bartlomiej Zolnierkiewicz > > Reviewed-by: Geert Uytterhoeven Thanks, I queued the patch for v5.3. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Re: [PATCH v3] video: fbdev: atmel_lcdfb: add COMPILE_TEST support
On 6/1/19 11:06 PM, Alexandre Belloni wrote: > On 30/05/2019 14:30:19+0200, Bartlomiej Zolnierkiewicz wrote: >> Add COMPILE_TEST support to atmel_lcdfb driver for better compile >> testing coverage. >> >> While at it fix improper use of UL (to silence build warnings on >> x86_64). >> >> Cc: Alexandre Belloni > Acked-by: Alexandre Belloni Thanks, I queued the patch for v5.3. >> Cc: Ludovic Desroches >> Signed-off-by: Bartlomiej Zolnierkiewicz >> --- >> v3: fix build warnings on x86_64 > > Hopefully, no building errors anymore ;) I hope so. :) >> v2: add missing HAVE_CLK && HAS IOMEM dependencies >> >> drivers/video/fbdev/Kconfig |3 ++- >> drivers/video/fbdev/atmel_lcdfb.c |4 ++-- >> 2 files changed, 4 insertions(+), 3 deletions(-) Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] video: fbdev: imxfb: add COMPILE_TEST support
On 5/21/19 12:47 PM, Bartlomiej Zolnierkiewicz wrote: > Add COMPILE_TEST support to imxfb driver for better compile > testing coverage. > > Signed-off-by: Bartlomiej Zolnierkiewicz I queued the patch for v5.3. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] video: fbdev: pxa168fb: add COMPILE_TEST support
On 5/21/19 12:49 PM, Bartlomiej Zolnierkiewicz wrote: > Add COMPILE_TEST support to pxa168fb driver for better compile > testing coverage. > > Signed-off-by: Bartlomiej Zolnierkiewicz I queued the patch for v5.3. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
