Re: [PATCH 10/22] drm/mediatek: Use simple encoder
Hi, Thomas: On Thu, 2020-03-05 at 16:59 +0100, Thomas Zimmermann wrote: > The mediatak driver uses empty implementations for its encoders. Replace > the code with the generic simple encoder. > Acked-by: CK Hu > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/mediatek/mtk_dpi.c | 14 +++--- > drivers/gpu/drm/mediatek/mtk_dsi.c | 14 +++--- > 2 files changed, 6 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dpi.c > b/drivers/gpu/drm/mediatek/mtk_dpi.c > index 14fbe1c09ce9..9c90c58e5acd 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dpi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dpi.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > > #include "mtk_dpi_regs.h" > #include "mtk_drm_ddp_comp.h" > @@ -509,15 +510,6 @@ static int mtk_dpi_set_display_mode(struct mtk_dpi *dpi, > return 0; > } > > -static void mtk_dpi_encoder_destroy(struct drm_encoder *encoder) > -{ > - drm_encoder_cleanup(encoder); > -} > - > -static const struct drm_encoder_funcs mtk_dpi_encoder_funcs = { > - .destroy = mtk_dpi_encoder_destroy, > -}; > - > static bool mtk_dpi_encoder_mode_fixup(struct drm_encoder *encoder, > const struct drm_display_mode *mode, > struct drm_display_mode *adjusted_mode) > @@ -596,8 +588,8 @@ static int mtk_dpi_bind(struct device *dev, struct device > *master, void *data) > return ret; > } > > - ret = drm_encoder_init(drm_dev, >encoder, _dpi_encoder_funcs, > -DRM_MODE_ENCODER_TMDS, NULL); > + ret = drm_simple_encoder_init(drm_dev, >encoder, > + DRM_MODE_ENCODER_TMDS); > if (ret) { > dev_err(dev, "Failed to initialize decoder: %d\n", ret); > goto err_unregister; > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 0ede69830a9d..a9a25087112f 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > > #include "mtk_drm_ddp_comp.h" > > @@ -787,15 +788,6 @@ static void mtk_output_dsi_disable(struct mtk_dsi *dsi) > dsi->enabled = false; > } > > -static void mtk_dsi_encoder_destroy(struct drm_encoder *encoder) > -{ > - drm_encoder_cleanup(encoder); > -} > - > -static const struct drm_encoder_funcs mtk_dsi_encoder_funcs = { > - .destroy = mtk_dsi_encoder_destroy, > -}; > - > static bool mtk_dsi_encoder_mode_fixup(struct drm_encoder *encoder, > const struct drm_display_mode *mode, > struct drm_display_mode *adjusted_mode) > @@ -888,8 +880,8 @@ static int mtk_dsi_create_conn_enc(struct drm_device > *drm, struct mtk_dsi *dsi) > { > int ret; > > - ret = drm_encoder_init(drm, >encoder, _dsi_encoder_funcs, > -DRM_MODE_ENCODER_DSI, NULL); > + ret = drm_simple_encoder_init(drm, >encoder, > + DRM_MODE_ENCODER_DSI); > if (ret) { > DRM_ERROR("Failed to encoder init to drm\n"); > return ret; ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: fix false positive assert
On Mon, Mar 09, 2020 at 07:49:15PM +0800, Christian König wrote: > Pierre-eric, just a gentle ping on this? Could I get a tested-by? > > Ray can you ack or even review this? > > Thanks, > Christian. > > Am 06.03.20 um 13:41 schrieb Christian König: > > The assert sometimes incorrectly triggers when pinned BOs are destroyed. > > > > Signed-off-by: Christian König Reviewed-by: Huang Rui > > --- > > drivers/gpu/drm/ttm/ttm_bo.c | 5 ++--- > > 1 file changed, 2 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c > > index 2445e2bd6267..ca5a8d01ff1f 100644 > > --- a/drivers/gpu/drm/ttm/ttm_bo.c > > +++ b/drivers/gpu/drm/ttm/ttm_bo.c > > @@ -151,8 +151,6 @@ static void ttm_bo_add_mem_to_lru(struct > > ttm_buffer_object *bo, > > struct ttm_bo_device *bdev = bo->bdev; > > struct ttm_mem_type_manager *man; > > > > - dma_resv_assert_held(bo->base.resv); > > - > > if (!list_empty(>lru)) > > return; > > > > @@ -611,7 +609,8 @@ static void ttm_bo_release(struct kref *kref) > > */ > > if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) { > > bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT; > > - ttm_bo_move_to_lru_tail(bo, NULL); > > + ttm_bo_del_from_lru(bo); > > + ttm_bo_add_mem_to_lru(bo, >mem); > > } > > > > kref_init(>kref); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 8/8] drm/virtio: enable per context fencing
If there's a 3D context available, initialize the fence using the 3D context's fence context. We can't process just the largest fence id anymore, since it's not the same as the sequence number now. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_fence.c | 15 --- drivers/gpu/drm/virtio/virtgpu_vq.c| 4 +--- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index a6c6f498e79e..bfcb41bcaa68 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -30,6 +30,8 @@ #define to_virtio_fence(x) \ container_of(x, struct virtio_gpu_fence, f) +static atomic_t fence_seq = ATOMIC_INIT(0); + static const char *virtio_get_driver_name(struct dma_fence *f) { return "virtio_gpu"; @@ -52,7 +54,7 @@ static bool virtio_fence_signaled(struct dma_fence *f) static void virtio_fence_value_str(struct dma_fence *f, char *str, int size) { - snprintf(str, size, "%llu", f->seqno); + snprintf(str, size, "[%llu, %llu]", f->context, f->seqno); } static void virtio_timeline_value_str(struct dma_fence *f, char *str, int size) @@ -74,9 +76,11 @@ static const struct dma_fence_ops virtio_fence_ops = { struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev, struct virtio_gpu_fpriv *fpriv) { + uint64_t context; struct virtio_gpu_fence_driver *drv = >fence_drv; struct virtio_gpu_fence *fence = kzalloc(sizeof(struct virtio_gpu_fence), GFP_KERNEL); + if (!fence) return fence; @@ -86,7 +90,13 @@ struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev, * unknown yet. The fence must not be used outside of the driver * until virtio_gpu_fence_emit is called. */ - dma_fence_init(>f, _fence_ops, >lock, drv->context, 0); + if (fpriv) + context = fpriv->fence_context; + else + context = drv->context; + + dma_fence_init(>f, _fence_ops, >lock, context, + atomic_inc_return(_seq)); return fence; } @@ -100,7 +110,6 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, spin_lock_irqsave(>lock, irq_flags); fence->fence_id = ++drv->current_fence_id; - fence->f.seqno = fence->fence_id; dma_fence_get(>f); list_add_tail(>node, >fences); spin_unlock_irqrestore(>lock, irq_flags); diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c index 73854915ec34..e7d8b2398628 100644 --- a/drivers/gpu/drm/virtio/virtgpu_vq.c +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c @@ -239,6 +239,7 @@ void virtio_gpu_dequeue_ctrl_func(struct work_struct *work) __func__, fence_id, f); } else { fence_id = f; + virtio_gpu_fence_event_process(vgdev, fence_id); } } if (entry->resp_cb) @@ -246,9 +247,6 @@ void virtio_gpu_dequeue_ctrl_func(struct work_struct *work) } wake_up(>ctrlq.ack_queue); - if (fence_id) - virtio_gpu_fence_event_process(vgdev, fence_id); - list_for_each_entry_safe(entry, tmp, _list, list) { if (entry->objs) virtio_gpu_array_put_free_delayed(vgdev, entry->objs); -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 0/8] *** Per context fencing ***
We don't want fences from different 3D contexts/processes (GL, VK) to be on the same timeline. Sending this out as a RFC to solicit feedback on the general approach. Gurchetan Singh (8): drm/virtio: use fence_id when processing fences drm/virtio: allocate a fence context for every 3D context drm/virtio: plumb virtio_gpu_fpriv to virtio_gpu_fence_alloc drm/virtio: rename sync_seq and last_seq drm/virtio: track fence_id in virtio_gpu_fence virtio/drm: rework virtio_fence_signaled drm/virtio: check context when signaling drm/virtio: enable per context fencing drivers/gpu/drm/virtio/virtgpu_debugfs.c | 4 +- drivers/gpu/drm/virtio/virtgpu_drv.h | 12 +++-- drivers/gpu/drm/virtio/virtgpu_fence.c | 66 drivers/gpu/drm/virtio/virtgpu_ioctl.c | 9 ++-- drivers/gpu/drm/virtio/virtgpu_kms.c | 1 + drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +- drivers/gpu/drm/virtio/virtgpu_vq.c | 4 +- 7 files changed, 62 insertions(+), 36 deletions(-) -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 2/8] drm/virtio: allocate a fence context for every 3D context
We don't want fences from different 3D contexts (GL, VK) to be on the same timeline. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + drivers/gpu/drm/virtio/virtgpu_kms.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 9627cd08f5be..b4f85c5fedb9 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -215,6 +215,7 @@ struct virtio_gpu_device { struct virtio_gpu_fpriv { uint32_t ctx_id; bool context_created; + uint64_t fence_context; struct mutex context_lock; }; diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index 023a030ca7b9..76b0f18e6691 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -267,6 +267,7 @@ int virtio_gpu_driver_open(struct drm_device *dev, struct drm_file *file) } vfpriv->ctx_id = handle + 1; + vfpriv->fence_context = dma_fence_context_alloc(1); file->driver_priv = vfpriv; return 0; } -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 6/8] virtio/drm: rework virtio_fence_signaled
We signal the fences ourselves in virtio_gpu_fence_event_process, shortly after we set last_fence_id. The window of time is small enough such that it may be possible to return false in this optional callback and rely on DMA_FENCE_FLAG_SIGNALED_BIT. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_fence.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index d63848178a58..a63a383347c4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -43,13 +43,10 @@ static const char *virtio_get_timeline_name(struct dma_fence *f) static bool virtio_fence_signaled(struct dma_fence *f) { struct virtio_gpu_fence *fence = to_virtio_fence(f); - - if (WARN_ON_ONCE(fence->f.seqno == 0)) - /* leaked fence outside driver before completing -* initialization with virtio_gpu_fence_emit */ - return false; - if (atomic64_read(>drv->last_fence_id) >= fence->f.seqno) - return true; + /* leaked fence outside driver before completing +* initialization with virtio_gpu_fence_emit +*/ + WARN_ON_ONCE(fence->fence_id == 0); return false; } -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 5/8] drm/virtio: track fence_id in virtio_gpu_fence
Each fence should be associated with a [fence ID, context ID, seqno]. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_drv.h | 1 + drivers/gpu/drm/virtio/virtgpu_fence.c | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 7d96d0fdcbac..e98d1a4cbda9 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -107,6 +107,7 @@ struct virtio_gpu_fence_driver { struct virtio_gpu_fence { struct dma_fence f; + uint64_t fence_id; struct virtio_gpu_fence_driver *drv; struct list_head node; }; diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index 0c32f3f72737..d63848178a58 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -102,7 +102,8 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, unsigned long irq_flags; spin_lock_irqsave(>lock, irq_flags); - fence->f.seqno = ++drv->current_fence_id; + fence->fence_id = ++drv->current_fence_id; + fence->f.seqno = fence->fence_id; dma_fence_get(>f); list_add_tail(>node, >fences); spin_unlock_irqrestore(>lock, irq_flags); @@ -110,7 +111,7 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, trace_dma_fence_emit(>f); cmd_hdr->flags |= cpu_to_le32(VIRTIO_GPU_FLAG_FENCE); - cmd_hdr->fence_id = cpu_to_le64(fence->f.seqno); + cmd_hdr->fence_id = cpu_to_le64(fence->fence_id); } void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev, -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 4/8] drm/virtio: rename sync_seq and last_seq
To be clearer about our intentions to differentiate per-context sequence numbers and fence IDs, let's rename these variables. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_debugfs.c | 4 ++-- drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++-- drivers/gpu/drm/virtio/virtgpu_fence.c | 9 + 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_debugfs.c b/drivers/gpu/drm/virtio/virtgpu_debugfs.c index e27120d512b0..807fe7e9e4d9 100644 --- a/drivers/gpu/drm/virtio/virtgpu_debugfs.c +++ b/drivers/gpu/drm/virtio/virtgpu_debugfs.c @@ -60,8 +60,8 @@ virtio_gpu_debugfs_irq_info(struct seq_file *m, void *data) struct virtio_gpu_device *vgdev = node->minor->dev->dev_private; seq_printf(m, "fence %llu %lld\n", - (u64)atomic64_read(>fence_drv.last_seq), - vgdev->fence_drv.sync_seq); + (u64)atomic64_read(>fence_drv.last_fence_id), + vgdev->fence_drv.current_fence_id); return 0; } diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index 76223e2aa68d..7d96d0fdcbac 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -98,8 +98,8 @@ typedef void (*virtio_gpu_resp_cb)(struct virtio_gpu_device *vgdev, struct virtio_gpu_vbuffer *vbuf); struct virtio_gpu_fence_driver { - atomic64_t last_seq; - uint64_t sync_seq; + atomic64_t last_fence_id; + uint64_t current_fence_id; uint64_t context; struct list_head fences; spinlock_t lock; diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index f0a7ef80e484..0c32f3f72737 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -48,7 +48,7 @@ static bool virtio_fence_signaled(struct dma_fence *f) /* leaked fence outside driver before completing * initialization with virtio_gpu_fence_emit */ return false; - if (atomic64_read(>drv->last_seq) >= fence->f.seqno) + if (atomic64_read(>drv->last_fence_id) >= fence->f.seqno) return true; return false; } @@ -62,7 +62,8 @@ static void virtio_timeline_value_str(struct dma_fence *f, char *str, int size) { struct virtio_gpu_fence *fence = to_virtio_fence(f); - snprintf(str, size, "%llu", (u64)atomic64_read(>drv->last_seq)); + snprintf(str, size, "%llu", + (u64)atomic64_read(>drv->last_fence_id)); } static const struct dma_fence_ops virtio_fence_ops = { @@ -101,7 +102,7 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, unsigned long irq_flags; spin_lock_irqsave(>lock, irq_flags); - fence->f.seqno = ++drv->sync_seq; + fence->f.seqno = ++drv->current_fence_id; dma_fence_get(>f); list_add_tail(>node, >fences); spin_unlock_irqrestore(>lock, irq_flags); @@ -120,7 +121,7 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev, unsigned long irq_flags; spin_lock_irqsave(>lock, irq_flags); - atomic64_set(>fence_drv.last_seq, fence_id); + atomic64_set(>fence_drv.last_fence_id, fence_id); list_for_each_entry_safe(fence, tmp, >fences, node) { if (fence_id < fence->f.seqno) continue; -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 1/8] drm/virtio: use fence_id when processing fences
Currently, the fence ID, which can be used to identify a virtgpu fence, is the same as the fence sequence number. They could be the same, but not necessarily so. Let's differentiate them. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +- drivers/gpu/drm/virtio/virtgpu_fence.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index c1824bdf2418..9627cd08f5be 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -360,7 +360,7 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, struct virtio_gpu_ctrl_hdr *cmd_hdr, struct virtio_gpu_fence *fence); void virtio_gpu_fence_event_process(struct virtio_gpu_device *vdev, - u64 last_seq); + u64 fence_id); /* virtio_gpu_object */ void virtio_gpu_cleanup_object(struct virtio_gpu_object *bo); diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index 5b2a4146c5bd..2fe9c7ebcbd4 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -112,16 +112,16 @@ void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, } void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev, - u64 last_seq) + u64 fence_id) { struct virtio_gpu_fence_driver *drv = >fence_drv; struct virtio_gpu_fence *fence, *tmp; unsigned long irq_flags; spin_lock_irqsave(>lock, irq_flags); - atomic64_set(>fence_drv.last_seq, last_seq); + atomic64_set(>fence_drv.last_seq, fence_id); list_for_each_entry_safe(fence, tmp, >fences, node) { - if (last_seq < fence->f.seqno) + if (fence_id < fence->f.seqno) continue; dma_fence_signal_locked(>f); list_del(>node); -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 3/8] drm/virtio: plumb virtio_gpu_fpriv to virtio_gpu_fence_alloc
We'll need it when initiating a dma-fence. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_drv.h | 4 ++-- drivers/gpu/drm/virtio/virtgpu_fence.c | 3 ++- drivers/gpu/drm/virtio/virtgpu_ioctl.c | 9 + drivers/gpu/drm/virtio/virtgpu_plane.c | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h index b4f85c5fedb9..76223e2aa68d 100644 --- a/drivers/gpu/drm/virtio/virtgpu_drv.h +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h @@ -355,8 +355,8 @@ struct drm_plane *virtio_gpu_plane_init(struct virtio_gpu_device *vgdev, int index); /* virtio_gpu_fence.c */ -struct virtio_gpu_fence *virtio_gpu_fence_alloc( - struct virtio_gpu_device *vgdev); +struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev, + struct virtio_gpu_fpriv *fpriv); void virtio_gpu_fence_emit(struct virtio_gpu_device *vgdev, struct virtio_gpu_ctrl_hdr *cmd_hdr, struct virtio_gpu_fence *fence); diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index 2fe9c7ebcbd4..f0a7ef80e484 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -73,7 +73,8 @@ static const struct dma_fence_ops virtio_fence_ops = { .timeline_value_str = virtio_timeline_value_str, }; -struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev) +struct virtio_gpu_fence *virtio_gpu_fence_alloc(struct virtio_gpu_device *vgdev, + struct virtio_gpu_fpriv *fpriv) { struct virtio_gpu_fence_driver *drv = >fence_drv; struct virtio_gpu_fence *fence = kzalloc(sizeof(struct virtio_gpu_fence), diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c index 336cc9143205..4860dcb3b3bb 100644 --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c @@ -160,7 +160,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data, goto out_memdup; } - out_fence = virtio_gpu_fence_alloc(vgdev); + out_fence = virtio_gpu_fence_alloc(vgdev, vfpriv); if(!out_fence) { ret = -ENOMEM; goto out_unresv; @@ -226,6 +226,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file) { struct virtio_gpu_device *vgdev = dev->dev_private; + struct virtio_gpu_fpriv *vfpriv = file->driver_priv; struct drm_virtgpu_resource_create *rc = data; struct virtio_gpu_fence *fence; int ret; @@ -265,7 +266,7 @@ static int virtio_gpu_resource_create_ioctl(struct drm_device *dev, void *data, if (params.size == 0) params.size = PAGE_SIZE; - fence = virtio_gpu_fence_alloc(vgdev); + fence = virtio_gpu_fence_alloc(vgdev, vfpriv); if (!fence) return -ENOMEM; ret = virtio_gpu_object_create(vgdev, , , fence); @@ -329,7 +330,7 @@ static int virtio_gpu_transfer_from_host_ioctl(struct drm_device *dev, if (ret != 0) goto err_put_free; - fence = virtio_gpu_fence_alloc(vgdev); + fence = virtio_gpu_fence_alloc(vgdev, vfpriv); if (!fence) { ret = -ENOMEM; goto err_unlock; @@ -375,7 +376,7 @@ static int virtio_gpu_transfer_to_host_ioctl(struct drm_device *dev, void *data, goto err_put_free; ret = -ENOMEM; - fence = virtio_gpu_fence_alloc(vgdev); + fence = virtio_gpu_fence_alloc(vgdev, vfpriv); if (!fence) goto err_unlock; diff --git a/drivers/gpu/drm/virtio/virtgpu_plane.c b/drivers/gpu/drm/virtio/virtgpu_plane.c index 52d24179bcec..0cf43936abb3 100644 --- a/drivers/gpu/drm/virtio/virtgpu_plane.c +++ b/drivers/gpu/drm/virtio/virtgpu_plane.c @@ -202,7 +202,7 @@ static int virtio_gpu_cursor_prepare_fb(struct drm_plane *plane, vgfb = to_virtio_gpu_framebuffer(new_state->fb); bo = gem_to_virtio_gpu_obj(vgfb->base.obj[0]); if (bo && bo->dumb && (plane->state->fb != new_state->fb)) { - vgfb->fence = virtio_gpu_fence_alloc(vgdev); + vgfb->fence = virtio_gpu_fence_alloc(vgdev, NULL); if (!vgfb->fence) return -ENOMEM; } -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 7/8] drm/virtio: check context when signaling
This change: - Lookups virtgpu_fence given a fence_id - Signals all prior fences in a given context - Signals current fence No functional changes yet, since all fences are initialized from context 0. Signed-off-by: Gurchetan Singh --- drivers/gpu/drm/virtio/virtgpu_fence.c | 27 -- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/virtio/virtgpu_fence.c b/drivers/gpu/drm/virtio/virtgpu_fence.c index a63a383347c4..a6c6f498e79e 100644 --- a/drivers/gpu/drm/virtio/virtgpu_fence.c +++ b/drivers/gpu/drm/virtio/virtgpu_fence.c @@ -115,17 +115,32 @@ void virtio_gpu_fence_event_process(struct virtio_gpu_device *vgdev, u64 fence_id) { struct virtio_gpu_fence_driver *drv = >fence_drv; - struct virtio_gpu_fence *fence, *tmp; + struct virtio_gpu_fence *signaled, *curr, *tmp; unsigned long irq_flags; spin_lock_irqsave(>lock, irq_flags); atomic64_set(>fence_drv.last_fence_id, fence_id); - list_for_each_entry_safe(fence, tmp, >fences, node) { - if (fence_id < fence->f.seqno) + list_for_each_entry_safe(curr, tmp, >fences, node) { + if (fence_id != curr->fence_id) continue; - dma_fence_signal_locked(>f); - list_del(>node); - dma_fence_put(>f); + + signaled = curr; + list_for_each_entry_safe(curr, tmp, >fences, node) { + if (signaled->f.context != curr->f.context) + continue; + + if (!dma_fence_is_later(>f, >f)) + continue; + + dma_fence_signal_locked(>f); + list_del(>node); + dma_fence_put(>f); + } + + dma_fence_signal_locked(>f); + list_del(>node); + dma_fence_put(>f); + break; } spin_unlock_irqrestore(>lock, irq_flags); } -- 2.25.1.481.gfbce0eb801-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Clean up unused code, to improve a system!
In Pete Goodliffe words, "You can improve a system by adding new code. You can also improve a system by removing code" - In this case, commit "202b52b7fbf70" added new code to initialize end of the node. So, there is no need for duplicated initialization, and this patch simply removes it. Signed-off-by: Akeem G Abodunrin CC: Chris Wilson --- drivers/gpu/drm/drm_mm.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c index 47d5de9ca0a8..7cd9023e61fb 100644 --- a/drivers/gpu/drm/drm_mm.c +++ b/drivers/gpu/drm/drm_mm.c @@ -410,7 +410,6 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct drm_mm_node *node) u64 hole_start, hole_end; u64 adj_start, adj_end; - end = node->start + node->size; if (unlikely(end <= node->start)) return -ENOSPC; -- 2.21.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v11 3/5] soc: mediatek: Move mt8173 MMSYS to platform driver
Quoting Enric Balletbo Serra (2020-03-06 14:09:50) > Missatge de Stephen Boyd del dia dv., 6 de mar > 2020 a les 22:37: > > > > Quoting Enric Balletbo i Serra (2020-03-06 08:30:16) > > > On 5/3/20 22:01, Stephen Boyd wrote: > > > > Quoting Enric Balletbo i Serra (2020-03-02 03:01:26) > > > >> diff --git a/drivers/soc/mediatek/mtk-mmsys.c > > > >> b/drivers/soc/mediatek/mtk-mmsys.c > > > >> new file mode 100644 > > > >> index ..473cdf732fb5 > > > >> --- /dev/null > > > >> +++ b/drivers/soc/mediatek/mtk-mmsys.c > > > >> @@ -0,0 +1,154 @@ > > > >> +// SPDX-License-Identifier: GPL-2.0-only > > > >> +/* > > > >> + * Copyright (c) 2014 MediaTek Inc. > > > >> + * Author: James Liao > > > >> + */ > > > >> + > > > >> +#include > > > >> +#include > > > >> +#include > > > >> + > > > >> +#include "../../clk/mediatek/clk-gate.h" > > > >> +#include "../../clk/mediatek/clk-mtk.h" > > > > > > > > Why not use include/linux/clk/? > > > > > > > > > > I can move these files to include, this will impact a lot more of drivers > > > but, > > > yes, I think is the right way. > > > > > > > But I also don't understand why the clk driver is moved outside of > > > > drivers/clk/ into drivers/soc/. Commit text saying that it has shared > > > > registers doesn't mean it can't still keep the clk driver part in the > > > > drivers/clk/ area. > > > > > > > > > > Actually moving this to the soc directory has been requested by CK > > > (mediatek) as > > > a change in v8. You can see the discussion in [1] > > > > > > > I can reply there in that thread if necessary, but we shouldn't need to > > force simple-mfd into DT bindings to support this. Match the compatible > > string in drivers/soc/ and register devices in software for the > > different pieces of this overall hardware block. If necessary, pass down > > the ioremapped addresss down through device data to each logical driver > > in the respective subsystem. > > > > So yes, it looks like an MFD, but that doesn't mean we have to change > > the DT binding or put it in drivers/mfd to support that. And we don't > > have to fix any problems with allowing two drivers to probe the same > > compatible string. > > > > That thread maybe has too much information and things evolved since > then. Note that the final solution is not an MFD neither we change the > bindings. I pointed to that thread just because CK (CK please correct > me if I'm wrong) thought that the driver is not a pure clock driver > and he preferred to move to drivers/soc/mediatek (in that thread, he > exposes his opinion on that). Sorry to introduce more confusion. > > You seem to be fine with the approach (just minor changes), so it > looks to me that the only problem is if this should be in drivers/clk > or drivers/soc. Honestly, this is not something I can't decide and > I'll let you (the soc and clk maintainers) decide. I don't really have > a strong opinion here. I don't mind move again to drivers/clk if that > is what we want but let's come to an agreement. > It's already in drivers/clk, so leave the clk part there and register the clk device and any other devices by matching the compatible in drivers/soc. That is my preferred solution. Can that be done? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND PATCH v7] dt-bindings: display: Add idk-2121wr binding
Hi Prabhakar On Mon, Mar 09, 2020 at 09:23:24PM +, Lad, Prabhakar wrote: > Hi Rob, > > On Mon, Mar 9, 2020 at 8:32 PM Rob Herring wrote: > > > > On Fri, 6 Mar 2020 15:20:31 +, Lad Prabhakar wrote: > > > From: Fabrizio Castro > > > > > > Add binding for the idk-2121wr LVDS panel from Advantech. > > > > > > Some panel-specific documentation can be found here: > > > https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm > > > > > > Signed-off-by: Fabrizio Castro > > > Signed-off-by: Lad Prabhakar > > > --- > > > Apologies for flooding in I missed to add the ML email-ids for the earlier > > > version so resending it. > > > > > > Hi All, > > > > > > This patch is part of series [1] ("Add dual-LVDS panel support to EK874), > > > all the patches have been accepted from it except this one. I have fixed > > > Rob's comments in this version of the patch. > > > > > > [1] https://patchwork.kernel.org/cover/11297589/ > > > > > > v6->7 > > > * Added reference to lvds.yaml > > > * Changed maintainer to myself > > > * Switched to dual license > > > * Dropped required properties except for ports as rest are already listed > > >in lvds.panel > > > * Dropped Reviewed-by tag of Laurent, due to the changes made it might > > > not > > >be valid. > > > > > > v5->v6: > > > * No change > > > > > > v4->v5: > > > * No change > > > > > > v3->v4: > > > * Absorbed patch "dt-bindings: display: Add bindings for LVDS > > > bus-timings" > > > * Big restructuring after Rob's and Laurent's comments > > > > > > v2->v3: > > > * New patch > > > > > > .../display/panel/advantech,idk-2121wr.yaml| 120 > > > + > > > 1 file changed, 120 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml > > > > > > > My bot found errors running 'make dt_binding_check' on your patch: > > > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: > > panel-lvds: 'port' is a required property > > /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: > > panel-lvds: 'port' is a required property > > > This panel is a dual channel LVDS, as a result the root port is called as > ports instead of port and the child node port@0 and port@1 are used for > even and odd pixels, hence binding has required property as ports instead > of port. What goes wrong is that you have a ref to lvds.yaml - and thus you get also required from that file. So basically - I think this binding should not have a ref to lvds.yaml - as the binding needs to be different. Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: panel: Set connector type for OrtusTech COM43H4M85ULC panel
Hi Ville, On Mon, Mar 09, 2020 at 11:22:51PM +0200, Ville Syrjälä wrote: > On Mon, Mar 09, 2020 at 10:29:42PM +0200, Laurent Pinchart wrote: > > On Mon, Mar 09, 2020 at 08:45:41PM +0100, Sam Ravnborg wrote: > > > On Mon, Mar 09, 2020 at 09:01:27PM +0200, Laurent Pinchart wrote: > > > > On Mon, Mar 09, 2020 at 08:00:47PM +0100, Sam Ravnborg wrote: > > > > > On Mon, Mar 09, 2020 at 08:42:10PM +0200, Laurent Pinchart wrote: > > > > > > The OrtusTech COM43H4M85ULC is a DPI panel, set the connector type > > > > > > accordingly. > > > > > > > > > > > > Signed-off-by: Laurent Pinchart > > > > > > > > > > Reviewed-by: Sam Ravnborg > > > > > > > > > > I assume you will apply to drm-misc-next - OK? > > > > > > > > I still haven't got around to using dim :-) > > > > > > I can manage - so the entry level is pretty low. > > > > > > My lame and simple workflow > > > > > > dim update-branches > > > # save patch from mutt > > > cat mbox | dim apply > > Why don't you just pipe the thing into dim straight from mutt? > That's what I do. Followed by some amount of dim extract-tag > also piped in from mutt. > > > > git rebase etc. > > > dim checkpatch <= if I make changes while applying > > > #build testing > > > dim push > > > > > > > > > And when I do my own stuff: > > > dim update-branches > > > git checkout -b sam-my-stuff > > > hacking-hacking > > > commit, commit > > > git rebase --exec "dim add-missing-cc" HEAD~5 > > > > > > > > > dim can do much more than that - but the above is > > > the few dim commands I use. > > > This help me to do things remotely correct. > > > > > > So maybe this is as good as any time to try out dim? > > > > As good as any, and as bad as any I suppose :-) > > > > There are a few things I don't like with dim, and I haven't found time > > yet to see how to fix (how live with :-) them yet. Among those issues > > are > > > > - dim requires the kernel tree to be under $DIM_PREFIX. This is my main > > issue, as I have one kernel tree per project, with and develop for > > different subsystems in each. I would like dim to instead handle any > > kernel tree regardless of where it is located on the disk, without > > requiring me to add another DRM-specific tree to my workflow. > > > > - The script auto-updates itself, and I find that to be a security issue > > that I'm not comfortable with. > > What do you mean it auto updates? Never seen anything like that. Unless I'm mistaken, function dim_uptodate { local using using="${BASH_SOURCE[0]}" if [[ ! -e "$using" ]]; then echoerr "could not figure out the version being used ($using)." return 1 fi if [[ ! -e "$DIM_PREFIX/maintainer-tools/.git" ]]; then echoerr "could not find the upstream repo for $dim." return 1 fi git --git-dir=$DIM_PREFIX/maintainer-tools/.git fetch -q if ! git --git-dir=$DIM_PREFIX/maintainer-tools/.git show "@{upstream}:dim" |\ diff "$using" - >& /dev/null; then echoerr "not running upstream version of the script." return 1 fi } function check_for_updates { local stamp stampfile stampfile=$HOME/.dim-update-check-timestamp # daily check for updates based on file timestamp stamp=$(stat --printf=%Y $stampfile 2>/dev/null || echo -n 0) if [[ $((stamp + 24*60*60)) -lt $(date +%s) ]]; then dim_uptodate || true touch $stampfile fi } And check_for_updates is called for all commands not listed by list_developer_commands. > > - The dim script makes a special case of intel repositories internally, > > which I don't find very fair. Maybe that can be considered as a > > compensation for Intel's efforts in DRM development, but a model where > > the community maintaining drm-misc has to resolve conflicts with > > drm-intel before it reaches drm-next bothers me. > > It doesn't special case Intel repos. It just merges all the repos listed > in the config file to create a new drm-tip. There are Intel repos, > AMD repos, and various other repos. drm-amd has indeed been added and is taken into accout in dim_push_branch, with drm-misc and drm-intel, but there's still lots of Intel-specific code in other places. > The point is to keep drm-tip always up to date and working (*). And if > you manage to create a conflict you can't solve you can always ping > someone who can. Also hoefully no one should be seeing all that many > conflicts due to rerere (unless you actually created a new conflict > that is). What happens is that drm-misc is competing with drm-intel and drm-amd to get in drm-next, with everything merged in drm-tip. It effectively conveys a message that there's Intel, AMD, and the community, which means that Intel and AMD are considered higher priority than any other single vendor, when we came from a situation where, *in
[PATCH v5 1/2] drm/edid: Name the detailed monitor range flags
This patch adds defines for the detailed monitor range flags as per the EDID specification. v2: * Rename the flags with DRM_EDID_ (Jani N) Suggested-by: Ville Syrjälä Cc: Ville Syrjälä Cc: Harry Wentland Cc: Clinton A Taylor Cc: Kazlauskas Nicholas Cc: Jani Nikula Signed-off-by: Manasi Navare Reviewed-by: Nicholas Kazlauskas --- include/drm/drm_edid.h | 5 + 1 file changed, 5 insertions(+) diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h index f0b03d401c27..34b15e3d070c 100644 --- a/include/drm/drm_edid.h +++ b/include/drm/drm_edid.h @@ -91,6 +91,11 @@ struct detailed_data_string { u8 str[13]; } __attribute__((packed)); +#define DRM_EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00 +#define DRM_EDID_RANGE_LIMITS_ONLY_FLAG 0x01 +#define DRM_EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02 +#define DRM_EDID_CVT_SUPPORT_FLAG 0x04 + struct detailed_data_monitor_range { u8 min_vfreq; u8 max_vfreq; -- 2.19.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/2] drm/edid: Add function to parse EDID descriptors for adaptive sync limits
Adaptive Sync is a VESA feature so add a DRM core helper to parse the EDID's detailed descritors to obtain the adaptive sync monitor range. Store this info as part fo drm_display_info so it can be used across all drivers. This part of the code is stripped out of amdgpu's function amdgpu_dm_update_freesync_caps() to make it generic and be used across all DRM drivers v5: * Use the renamed flags v4: * Use is_display_descriptor() (Ville) * Name the monitor range flags (Ville) v3: * Remove the edid parsing restriction for just DP (Nicholas) * Use drm_for_each_detailed_block (Ville) * Make the drm_get_adaptive_sync_range function static (Harry, Jani) v2: * Change vmin and vmax to use u8 (Ville) * Dont store pixel clock since that is just a max dotclock and not related to VRR mode (Manasi) Cc: Ville Syrjälä Cc: Harry Wentland Cc: Clinton A Taylor Cc: Kazlauskas Nicholas Signed-off-by: Manasi Navare Reviewed-by: Nicholas Kazlauskas --- drivers/gpu/drm/drm_edid.c | 44 + include/drm/drm_connector.h | 22 +++ 2 files changed, 66 insertions(+) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index ad41764a4ebe..24b76ae58fdd 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4938,6 +4938,47 @@ static void drm_parse_cea_ext(struct drm_connector *connector, } } +static +void get_adaptive_sync_range(struct detailed_timing *timing, +void *info_adaptive_sync) +{ + struct drm_adaptive_sync_info *adaptive_sync = info_adaptive_sync; + const struct detailed_non_pixel *data = >data.other_data; + const struct detailed_data_monitor_range *range = >data.range; + + if (!is_display_descriptor((const u8 *)timing, EDID_DETAIL_MONITOR_RANGE)) + return; + + /* +* Check for flag range limits only. If flag == 1 then +* no additional timing information provided. +* Default GTF, GTF Secondary curve and CVT are not +* supported +*/ + if (range->flags != DRM_EDID_RANGE_LIMITS_ONLY_FLAG) + return; + + adaptive_sync->min_vfreq = range->min_vfreq; + adaptive_sync->max_vfreq = range->max_vfreq; +} + +static +void drm_get_adaptive_sync_range(struct drm_connector *connector, +const struct edid *edid) +{ + struct drm_display_info *info = >display_info; + + if (!version_greater(edid, 1, 1)) + return; + + drm_for_each_detailed_block((u8 *)edid, get_adaptive_sync_range, + >adaptive_sync); + + DRM_DEBUG_KMS("Adaptive Sync refresh rate range is %d Hz - %d Hz\n", + info->adaptive_sync.min_vfreq, + info->adaptive_sync.max_vfreq); +} + /* A connector has no EDID information, so we've got no EDID to compute quirks from. Reset * all of the values which would have been set from EDID */ @@ -4960,6 +5001,7 @@ drm_reset_display_info(struct drm_connector *connector) memset(>hdmi, 0, sizeof(info->hdmi)); info->non_desktop = 0; + memset(>adaptive_sync, 0, sizeof(info->adaptive_sync)); } u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edid) @@ -4975,6 +5017,8 @@ u32 drm_add_display_info(struct drm_connector *connector, const struct edid *edi info->non_desktop = !!(quirks & EDID_QUIRK_NON_DESKTOP); + drm_get_adaptive_sync_range(connector, edid); + DRM_DEBUG_KMS("non_desktop set to %d\n", info->non_desktop); if (edid->revision < 3) diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h index 0df7a95ca5d9..2b22c0fa42c4 100644 --- a/include/drm/drm_connector.h +++ b/include/drm/drm_connector.h @@ -254,6 +254,23 @@ enum drm_panel_orientation { DRM_MODE_PANEL_ORIENTATION_RIGHT_UP, }; +/** + * struct drm_adaptive_sync_info - Panel's Adaptive Sync capabilities for + * _display_info + * + * This struct is used to store a Panel's Adaptive Sync capabilities + * as parsed from EDID's detailed monitor range descriptor block. + * + * @min_vfreq: This is the min supported refresh rate in Hz from + * EDID's detailed monitor range. + * @max_vfreq: This is the max supported refresh rate in Hz from + * EDID's detailed monitor range + */ +struct drm_adaptive_sync_info { + u8 min_vfreq; + u8 max_vfreq; +}; + /* * This is a consolidated colorimetry list supported by HDMI and * DP protocol standard. The respective connectors will register @@ -473,6 +490,11 @@ struct drm_display_info { * @non_desktop: Non desktop display (HMD). */ bool non_desktop; + + /** +* @adaptive_sync: Adaptive Sync capabilities of the DP/eDP sink +*/ + struct drm_adaptive_sync_info adaptive_sync; }; int drm_display_info_set_bus_formats(struct drm_display_info *info, -- 2.19.1
Re: [PATCH] drm: panel: Set connector type for OrtusTech COM43H4M85ULC panel
On Mon, Mar 09, 2020 at 10:29:42PM +0200, Laurent Pinchart wrote: > Hi Sam, > > On Mon, Mar 09, 2020 at 08:45:41PM +0100, Sam Ravnborg wrote: > > On Mon, Mar 09, 2020 at 09:01:27PM +0200, Laurent Pinchart wrote: > > > On Mon, Mar 09, 2020 at 08:00:47PM +0100, Sam Ravnborg wrote: > > > > On Mon, Mar 09, 2020 at 08:42:10PM +0200, Laurent Pinchart wrote: > > > > > The OrtusTech COM43H4M85ULC is a DPI panel, set the connector type > > > > > accordingly. > > > > > > > > > > Signed-off-by: Laurent Pinchart > > > > > > > > Reviewed-by: Sam Ravnborg > > > > > > > > I assume you will apply to drm-misc-next - OK? > > > > > > I still haven't got around to using dim :-) > > > > I can manage - so the entry level is pretty low. > > > > My lame and simple workflow > > > > dim update-branches > > # save patch from mutt > > cat mbox | dim apply Why don't you just pipe the thing into dim straight from mutt? That's what I do. Followed by some amount of dim extract-tag also piped in from mutt. > > git rebase etc. > > dim checkpatch <= if I make changes while applying > > #build testing > > dim push > > > > > > And when I do my own stuff: > > dim update-branches > > git checkout -b sam-my-stuff > > hacking-hacking > > commit, commit > > git rebase --exec "dim add-missing-cc" HEAD~5 > > > > > > dim can do much more than that - but the above is > > the few dim commands I use. > > This help me to do things remotely correct. > > > > So maybe this is as good as any time to try out dim? > > As good as any, and as bad as any I suppose :-) > > There are a few things I don't like with dim, and I haven't found time > yet to see how to fix (how live with :-) them yet. Among those issues > are > > - dim requires the kernel tree to be under $DIM_PREFIX. This is my main > issue, as I have one kernel tree per project, with and develop for > different subsystems in each. I would like dim to instead handle any > kernel tree regardless of where it is located on the disk, without > requiring me to add another DRM-specific tree to my workflow. > > - The script auto-updates itself, and I find that to be a security issue > that I'm not comfortable with. What do you mean it auto updates? Never seen anything like that. > - The dim script makes a special case of intel repositories internally, > which I don't find very fair. Maybe that can be considered as a > compensation for Intel's efforts in DRM development, but a model where > the community maintaining drm-misc has to resolve conflicts with > drm-intel before it reaches drm-next bothers me. It doesn't special case Intel repos. It just merges all the repos listed in the config file to create a new drm-tip. There are Intel repos, AMD repos, and various other repos. The point is to keep drm-tip always up to date and working (*). And if you manage to create a conflict you can't solve you can always ping someone who can. Also hoefully no one should be seeing all that many conflicts due to rerere (unless you actually created a new conflict that is). * why would anyone run anything else but drm-tip anyway? ;) -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/dp_mst: Rewrite and fix bandwidth limit checks
On Mon, Mar 9, 2020 at 5:01 PM Lyude Paul wrote: > > Sigh, this is mostly my fault for not giving commit cd82d82cbc04 > ("drm/dp_mst: Add branch bandwidth validation to MST atomic check") > enough scrutiny during review. The way we're checking bandwidth > limitations here is mostly wrong: > > For starters, drm_dp_mst_atomic_check_bw_limit() determines the > pbn_limit of a branch by simply scanning each port on the current branch > device, then uses the last non-zero full_pbn value that it finds. It > then counts the sum of the PBN used on each branch device for that > level, and compares against the full_pbn value it found before. > > This is wrong because ports can and will have different PBN limitations > on many hubs, especially since a number of DisplayPort hubs out there > will be clever and only use the smallest link rate required for each > downstream sink - potentially giving every port a different full_pbn > value depending on what link rate it's trained at. This means with our > current code, which max PBN value we end up with is not well defined. > > Additionally, we also need to remember when checking bandwidth > limitations that the top-most device in any MST topology is a branch > device, not a port. This means that the first level of a topology > doesn't technically have a full_pbn value that needs to be checked. > Instead, we should assume that so long as our VCPI allocations fit we're > within the bandwidth limitations of the primary MSTB. > > We do however, want to check full_pbn on every port including those of > the primary MSTB. However, it's important to keep in mind that this > value represents the minimum link rate /between a port's sink or mstb, > and the mstb itself/. A quick diagram to explain: > > MSTB #1 >/ \ > / \ >Port #1Port #2 >full_pbn for Port #1 → | | ← full_pbn for Port #2 >Sink #1MSTB #2 > | >etc... > > Note that in the above diagram, the combined PBN from all VCPI > allocations on said hub should not exceed the full_pbn value of port #2, > and the display configuration on sink #1 should not exceed the full_pbn > value of port #1. However, port #1 and port #2 can otherwise consume as > much bandwidth as they want so long as their VCPI allocations still fit. > > And finally - our current bandwidth checking code also makes the mistake > of not checking whether something is an end device or not before trying > to traverse down it. > > So, let's fix it by rewriting our bandwidth checking helpers. We split > the function into one part for handling branches which simply adds up > the total PBN on each branch and returns it, and one for checking each > port to ensure we're not going over its PBN limit. Phew. > > This should fix regressions seen, where we erroneously reject display > configurations due to thinking they're going over our bandwidth limits > when they're not. > > Changes since v1: > * Took an even closer look at how PBN limitations are supposed to be > handled, and did some experimenting with Sean Paul. Ended up rewriting > these helpers again, but this time they should actually be correct! > Changes since v2: > * Small indenting fix > * Fix pbn_used check in drm_dp_mst_atomic_check_port_bw_limit() > > Signed-off-by: Lyude Paul > Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to MST > atomic check") > Cc: Mikita Lipski > Cc: Sean Paul > Cc: Hans de Goede Thanks for the detailed descriptions. The changes make sense to me, but I don't know the DP MST code that well, so patches 2-4 are: Acked-by: Alex Deucher > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 119 -- > 1 file changed, 93 insertions(+), 26 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index b81ad444c24f..d2f464bdcfff 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -4841,41 +4841,102 @@ static bool > drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port, > return false; > } > > -static inline > -int drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch, > -struct drm_dp_mst_topology_state > *mst_state) > +static int > +drm_dp_mst_atomic_check_port_bw_limit(struct drm_dp_mst_port *port, > + struct drm_dp_mst_topology_state > *state); > + > +static int > +drm_dp_mst_atomic_check_mstb_bw_limit(struct drm_dp_mst_branch *mstb, > + struct drm_dp_mst_topology_state *state) > { > - struct drm_dp_mst_port *port; > struct drm_dp_vcpi_allocation *vcpi; > - int pbn_limit = 0, pbn_used = 0; > + struct
Re: [PATCH v2 1/4] drm/dp_mst: Rename drm_dp_mst_is_dp_mst_end_device() to be less redundant
On Fri, Mar 6, 2020 at 6:46 PM Lyude Paul wrote: > > It's already prefixed by dp_mst, so we don't really need to repeat > ourselves here. One of the changes I should have picked up originally > when reviewing MST DSC support. > > There should be no functional changes here > > Cc: Mikita Lipski > Cc: Sean Paul > Cc: Hans de Goede > Signed-off-by: Lyude Paul Reviewed-by: Alex Deucher > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 6c62ad8f4414..6714d8a5c558 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -1937,7 +1937,7 @@ static u8 drm_dp_calculate_rad(struct drm_dp_mst_port > *port, > return parent_lct + 1; > } > > -static bool drm_dp_mst_is_dp_mst_end_device(u8 pdt, bool mcs) > +static bool drm_dp_mst_is_end_device(u8 pdt, bool mcs) > { > switch (pdt) { > case DP_PEER_DEVICE_DP_LEGACY_CONV: > @@ -1967,13 +1967,13 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 > new_pdt, > > /* Teardown the old pdt, if there is one */ > if (port->pdt != DP_PEER_DEVICE_NONE) { > - if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) { > + if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) { > /* > * If the new PDT would also have an i2c bus, > * don't bother with reregistering it > */ > if (new_pdt != DP_PEER_DEVICE_NONE && > - drm_dp_mst_is_dp_mst_end_device(new_pdt, > new_mcs)) { > + drm_dp_mst_is_end_device(new_pdt, new_mcs)) { > port->pdt = new_pdt; > port->mcs = new_mcs; > return 0; > @@ -1993,7 +1993,7 @@ drm_dp_port_set_pdt(struct drm_dp_mst_port *port, u8 > new_pdt, > port->mcs = new_mcs; > > if (port->pdt != DP_PEER_DEVICE_NONE) { > - if (drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) { > + if (drm_dp_mst_is_end_device(port->pdt, port->mcs)) { > /* add i2c over sideband */ > ret = drm_dp_mst_register_i2c_bus(>aux); > } else { > @@ -2169,7 +2169,7 @@ drm_dp_mst_port_add_connector(struct drm_dp_mst_branch > *mstb, > } > > if (port->pdt != DP_PEER_DEVICE_NONE && > - drm_dp_mst_is_dp_mst_end_device(port->pdt, port->mcs)) { > + drm_dp_mst_is_end_device(port->pdt, port->mcs)) { > port->cached_edid = drm_get_edid(port->connector, > >aux.ddc); > drm_connector_set_tile_property(port->connector); > -- > 2.24.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/2] drm/dp_mst: Fix link address probing regressions
On Fri, Mar 6, 2020 at 6:49 PM Lyude Paul wrote: > > While fixing some regressions caused by introducing bandwidth checking > into the DP MST atomic helpers, I realized there was another much more > subtle regression that got introduced by a seemingly harmless patch to > fix unused variable errors while compiling with W=1 (mentioned in patch > 2). Basically, this regression makes it so sometimes link address > appears to "hang". This patch series fixes it. Series is: Reviewed-by: Alex Deucher > > Lyude Paul (2): > drm/dp_mst: Make drm_dp_mst_dpcd_write() consistent with > drm_dp_dpcd_write() > drm/dp_mst: Fix drm_dp_check_mstb_guid() return code > > drivers/gpu/drm/drm_dp_mst_topology.c | 24 +++- > 1 file changed, 15 insertions(+), 9 deletions(-) > > -- > 2.24.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v3] drm/dp_mst: Rewrite and fix bandwidth limit checks
Sigh, this is mostly my fault for not giving commit cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to MST atomic check") enough scrutiny during review. The way we're checking bandwidth limitations here is mostly wrong: For starters, drm_dp_mst_atomic_check_bw_limit() determines the pbn_limit of a branch by simply scanning each port on the current branch device, then uses the last non-zero full_pbn value that it finds. It then counts the sum of the PBN used on each branch device for that level, and compares against the full_pbn value it found before. This is wrong because ports can and will have different PBN limitations on many hubs, especially since a number of DisplayPort hubs out there will be clever and only use the smallest link rate required for each downstream sink - potentially giving every port a different full_pbn value depending on what link rate it's trained at. This means with our current code, which max PBN value we end up with is not well defined. Additionally, we also need to remember when checking bandwidth limitations that the top-most device in any MST topology is a branch device, not a port. This means that the first level of a topology doesn't technically have a full_pbn value that needs to be checked. Instead, we should assume that so long as our VCPI allocations fit we're within the bandwidth limitations of the primary MSTB. We do however, want to check full_pbn on every port including those of the primary MSTB. However, it's important to keep in mind that this value represents the minimum link rate /between a port's sink or mstb, and the mstb itself/. A quick diagram to explain: MSTB #1 / \ / \ Port #1Port #2 full_pbn for Port #1 → | | ← full_pbn for Port #2 Sink #1MSTB #2 | etc... Note that in the above diagram, the combined PBN from all VCPI allocations on said hub should not exceed the full_pbn value of port #2, and the display configuration on sink #1 should not exceed the full_pbn value of port #1. However, port #1 and port #2 can otherwise consume as much bandwidth as they want so long as their VCPI allocations still fit. And finally - our current bandwidth checking code also makes the mistake of not checking whether something is an end device or not before trying to traverse down it. So, let's fix it by rewriting our bandwidth checking helpers. We split the function into one part for handling branches which simply adds up the total PBN on each branch and returns it, and one for checking each port to ensure we're not going over its PBN limit. Phew. This should fix regressions seen, where we erroneously reject display configurations due to thinking they're going over our bandwidth limits when they're not. Changes since v1: * Took an even closer look at how PBN limitations are supposed to be handled, and did some experimenting with Sean Paul. Ended up rewriting these helpers again, but this time they should actually be correct! Changes since v2: * Small indenting fix * Fix pbn_used check in drm_dp_mst_atomic_check_port_bw_limit() Signed-off-by: Lyude Paul Fixes: cd82d82cbc04 ("drm/dp_mst: Add branch bandwidth validation to MST atomic check") Cc: Mikita Lipski Cc: Sean Paul Cc: Hans de Goede --- drivers/gpu/drm/drm_dp_mst_topology.c | 119 -- 1 file changed, 93 insertions(+), 26 deletions(-) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index b81ad444c24f..d2f464bdcfff 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4841,41 +4841,102 @@ static bool drm_dp_mst_port_downstream_of_branch(struct drm_dp_mst_port *port, return false; } -static inline -int drm_dp_mst_atomic_check_bw_limit(struct drm_dp_mst_branch *branch, -struct drm_dp_mst_topology_state *mst_state) +static int +drm_dp_mst_atomic_check_port_bw_limit(struct drm_dp_mst_port *port, + struct drm_dp_mst_topology_state *state); + +static int +drm_dp_mst_atomic_check_mstb_bw_limit(struct drm_dp_mst_branch *mstb, + struct drm_dp_mst_topology_state *state) { - struct drm_dp_mst_port *port; struct drm_dp_vcpi_allocation *vcpi; - int pbn_limit = 0, pbn_used = 0; + struct drm_dp_mst_port *port; + int pbn_used = 0, ret; + bool found = false; - list_for_each_entry(port, >ports, next) { - if (port->mstb) - if (drm_dp_mst_atomic_check_bw_limit(port->mstb, mst_state)) - return -ENOSPC; + /* Check that we have at least one port in our state that's downstream +* of this branch,
Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling
Hi Boris, On Mon, Mar 09, 2020 at 09:42:44PM +0100, Boris Brezillon wrote: > On Mon, 9 Mar 2020 22:32:11 +0200 Laurent Pinchart wrote: > > On Mon, Mar 09, 2020 at 09:22:18PM +0100, Boris Brezillon wrote: > >> On Mon, 9 Mar 2020 21:59:26 +0200 Laurent Pinchart wrote: > >>> On Mon, Mar 09, 2020 at 08:55:59PM +0100, Boris Brezillon wrote: > On Mon, 9 Mar 2020 21:23:06 +0200 Laurent Pinchart wrote: > > On Mon, Mar 09, 2020 at 11:50:59AM +0100, Philipp Zabel wrote: > >> On Thu, 2019-11-14 at 14:17 +0100, Marek Vasut wrote: > >>> The bus_flags and bus_format handling logic does not seem to cover > >>> all potential usecases. Specifically, this seems to fail with an > >>> "edt,etm0700g0edh6" display attached to an 24bit display interface, > >>> with interface-pix-fmt = "rgb24" set in DT. > >> > >> interface-pix-fmt is a legacy property that was never intended to be > >> used as an override for the panel bus format. The bus flags were > >> supposed to be set from the display-timings node, back when there was > >> no > >> of-graph connected panel at all. > >> > >> That being said, there isn't really a proper alternative that allows to > >> override the bus format requested by the panel driver in the device > >> tree > >> to account for weird wiring. We could reuse the bus-width endpoint > >> property documented in [1], but that wouldn't completely specify how > >> the > >> RGB components are to be mapped onto the parallel bus. > >> > >> [1] Documentation/devicetree/bindings/media/video-interfaces.txt > > > > Things are funny sometimes, I've run into the exact same problem with a > > different display controller today. > > > > Shouldn't we use the data-shift property from [1] to specify this ? > > Combined with Boris' bus format negotiation for bridges, I think we > > would have all the components in place to solve this problem properly. > > > > I wonder if we shouldn't take more complex pin mappings into account > now and go directly for a data-mapping property describing those > mappings using a string. This way we'd have a single property that > would work for both fully parallel buses (DPI/RGB) and serial (or > partially parallel) ones (LVDS). > >>> > >>> I'm all for standardization, but I'm not sure data-mapping is the right > >>> property, at least with its current definition. It's really meant to > >>> describe how individual bits are mapped to the LVDS time slots. I'm fine > >>> extending it, but we need to define it clearly. How would you envision > >>> it being used in this case ? > >> > >> Well, clearly the data-width/data-shift approach does not solve all > >> problems: what do you do if the source R pins are connected to the sink > >> B pins? Well, the first answer would probably be 'have a serious > >> discussion with the HW designer responsible for this insanity' :-), but > >> once you've passed this 'WTF' stage, you'll have to find a way to tell > >> the source component it should use RGBxyx while the sink should use > >> BGRxyx (or vice-versa). This is something you can't extract that from > >> those width/shift props though. My suggestion would be to have one > >> string per MEDIA_BUS_FMT definition, so we can force things at the DT > >> level if we really have to. That's basically what the interface-pix-fmt > >> property was doing, except we would standardize the prop and values and > >> probably provide helpers so bridge elements don't have to parse this > >> prop manually. > > > > I don't think that would work in the general case though. We may want to > > use different formats and pick one of them at runtime based on external > > information (for instance when the sink can accept both RGB and YUV), > > hardcoding formats in DT isn't a good option. We instead need to add > > information to DT to specify how lines are connected, and deduce formats > > based on that. > > If we start describing the role of each pin, we're not that far from a > pinmux definition, the only difference being that we want pin configs > to match between the source and sink, where actual pinmux configs are > only controlled by one element (the HW block requesting exclusive > access to those pins). The trick here will be to find an appropriate middle-ground. I don't think we need to describe the role of each pin, but only to take into account the parallel bus routing configurations that are likely to happen in practice. Connecting MSBs to LSBs when decreasing the bus width (or the other way around when increasing it) is a common issue. Flipping R and B should be less common, but I suppose it can happen in practice if the display controller supports both RGB and BGR formats (it will just need to adjust the format internally if there's no dedicated R<->B flipping hardware option). What else do we have ? > Note
Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling
On Mon, 9 Mar 2020 22:32:11 +0200 Laurent Pinchart wrote: > Hi Boris, > > On Mon, Mar 09, 2020 at 09:22:18PM +0100, Boris Brezillon wrote: > > On Mon, 9 Mar 2020 21:59:26 +0200 Laurent Pinchart wrote: > > > On Mon, Mar 09, 2020 at 08:55:59PM +0100, Boris Brezillon wrote: > > > > On Mon, 9 Mar 2020 21:23:06 +0200 Laurent Pinchart wrote: > > > > > On Mon, Mar 09, 2020 at 11:50:59AM +0100, Philipp Zabel wrote: > > > > > > On Thu, 2019-11-14 at 14:17 +0100, Marek Vasut wrote: > > > > > > > The bus_flags and bus_format handling logic does not seem to cover > > > > > > > all potential usecases. Specifically, this seems to fail with an > > > > > > > "edt,etm0700g0edh6" display attached to an 24bit display > > > > > > > interface, > > > > > > > with interface-pix-fmt = "rgb24" set in DT. > > > > > > > > > > > > interface-pix-fmt is a legacy property that was never intended to be > > > > > > used as an override for the panel bus format. The bus flags were > > > > > > supposed to be set from the display-timings node, back when there > > > > > > was no > > > > > > of-graph connected panel at all. > > > > > > > > > > > > That being said, there isn't really a proper alternative that > > > > > > allows to > > > > > > override the bus format requested by the panel driver in the device > > > > > > tree > > > > > > to account for weird wiring. We could reuse the bus-width endpoint > > > > > > property documented in [1], but that wouldn't completely specify > > > > > > how the > > > > > > RGB components are to be mapped onto the parallel bus. > > > > > > > > > > > > [1] Documentation/devicetree/bindings/media/video-interfaces.txt > > > > > > > > > > > > > > > > Things are funny sometimes, I've run into the exact same problem with > > > > > a > > > > > different display controller today. > > > > > > > > > > Shouldn't we use the data-shift property from [1] to specify this ? > > > > > Combined with Boris' bus format negotiation for bridges, I think we > > > > > would have all the components in place to solve this problem > > > > > properly. > > > > > > > > I wonder if we shouldn't take more complex pin mappings into account > > > > now and go directly for a data-mapping property describing those > > > > mappings using a string. This way we'd have a single property that > > > > would work for both fully parallel buses (DPI/RGB) and serial (or > > > > partially parallel) ones (LVDS). > > > > > > I'm all for standardization, but I'm not sure data-mapping is the right > > > property, at least with its current definition. It's really meant to > > > describe how individual bits are mapped to the LVDS time slots. I'm fine > > > extending it, but we need to define it clearly. How would you envision > > > it being used in this case ? > > > > Well, clearly the data-width/data-shift approach does not solve all > > problems: what do you do if the source R pins are connected to the sink > > B pins? Well, the first answer would probably be 'have a serious > > discussion with the HW designer responsible for this insanity' :-), but > > once you've passed this 'WTF' stage, you'll have to find a way to tell > > the source component it should use RGBxyx while the sink should use > > BGRxyx (or vice-versa). This is something you can't extract that from > > those width/shift props though. My suggestion would be to have one > > string per MEDIA_BUS_FMT definition, so we can force things at the DT > > level if we really have to. That's basically what the interface-pix-fmt > > property was doing, except we would standardize the prop and values and > > probably provide helpers so bridge elements don't have to parse this > > prop manually. > > I don't think that would work in the general case though. We may want to > use different formats and pick one of them at runtime based on external > information (for instance when the sink can accept both RGB and YUV), > hardcoding formats in DT isn't a good option. We instead need to add > information to DT to specify how lines are connected, and deduce formats > based on that. > If we start describing the role of each pin, we're not that far from a pinmux definition, the only difference being that we want pin configs to match between the source and sink, where actual pinmux configs are only controlled by one element (the HW block requesting exclusive access to those pins). Note that none of those things actually solve Marek's issue, which was related to bus-flags, not bus-format. But I'm glad we have this discussion, since that's something I need to solve for an imx setup with a lvds-codec encoder connected to the imx-pd block. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] Cleanup drm_dp_mst_topology_cbs hooks
On Mon, Mar 9, 2020 at 4:27 PM Lyude Paul wrote: > > On Sat, 2020-03-07 at 14:00 +0530, Pankaj Bharadiya wrote: > > drm_dp_mst_topology_mgr_cbs.register_connector callbacks are identical > > amongst every driver and don't do anything other than calling > > drm_connector_register(). > > drm_dp_mst_topology_mgr_cbs.destroy_connector callbacks are identical > > amongst every driver and don't do anything other than cleaning up the > > connector((drm_connector_unregister()/drm_connector_put())) except for > > amdgpu_dm driver where some amdgpu_dm specific code in there. > > Yeah that amdgpu destruction code kinda stinks a little bit :\. I think we can > just drop some of it and move the rest into their connector destruction > callbacks. > > For the whole series: > Reviewed-by: Lyude Paul > > I'm going to go ahead and let the maintainers know I'm going to push this > (since there's some minor changes here outside of drm-misc), and push this to > drm-misc-next. Then I'll go and write some patches to remove the leftover amd > bits and drop the callback for good (I'll cc it to you as well). Series is: Reviewed-by: Alex Deucher > > > > > This series aims to cleaup these drm_dp_mst_topology_mgr_cbs hooks. > > > > Pankaj Bharadiya (5): > > drm: Register connector instead of calling register_connector callback > > drm: Remove dp mst register connector callbacks > > drm/dp_mst: Remove register_connector callback > > drm: Add drm_dp_destroy_connector helper and use it > > drm: Remove drm dp mst destroy_connector callbacks > > > > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 6 -- > > drivers/gpu/drm/drm_dp_mst_topology.c | 18 +++--- > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 > > drivers/gpu/drm/nouveau/dispnv50/disp.c | 19 --- > > drivers/gpu/drm/radeon/radeon_dp_mst.c| 17 - > > include/drm/drm_dp_mst_helper.h | 1 - > > 6 files changed, 15 insertions(+), 62 deletions(-) > > > -- > Cheers, > Lyude Paul (she/her) > Associate Software Engineer at Red Hat > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm: bridge: tfp410: Check device ID for I2C-connected TFP410
The TFP410 supports configuration through I2C (in which case the corresponding DT node is a child of an I2C controller) or through pins (in which case the DT node creates a platform device). When I2C access to the device is available, read and validate the device ID at probe time to ensure that the device is present. While at it, sort headers alphabetically. Signed-off-by: Laurent Pinchart --- This time based on drm-misc-next instead of v5.6-rc5, with conflicts resolved. drivers/gpu/drm/bridge/ti-tfp410.c | 134 + 1 file changed, 115 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index 40c4d4a5517b..be8d74ff632b 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -26,6 +27,7 @@ struct tfp410 { u32 bus_format; struct delayed_work hpd_work; struct gpio_desc*powerdown; + struct regmap *regmap; struct drm_bridge_timings timings; struct drm_bridge *next_bridge; @@ -213,7 +215,7 @@ static const struct drm_bridge_timings tfp410_default_timings = { .hold_time_ps = 1300, }; -static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) +static int tfp410_parse_timings(struct tfp410 *dvi) { struct drm_bridge_timings *timings = >timings; struct device_node *ep; @@ -224,7 +226,7 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) /* Start with defaults. */ *timings = tfp410_default_timings; - if (i2c) + if (dvi->regmap) /* * In I2C mode timings are configured through the I2C interface. * As the driver doesn't support I2C configuration yet, we just @@ -283,10 +285,10 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) return 0; } -static int tfp410_init(struct device *dev, bool i2c) +static int tfp410_init(struct tfp410 *dvi) { struct device_node *node; - struct tfp410 *dvi; + struct device *dev = dvi->dev; int ret; if (!dev->of_node) { @@ -294,11 +296,6 @@ static int tfp410_init(struct device *dev, bool i2c) return -ENXIO; } - dvi = devm_kzalloc(dev, sizeof(*dvi), GFP_KERNEL); - if (!dvi) - return -ENOMEM; - - dvi->dev = dev; dev_set_drvdata(dev, dvi); dvi->bridge.funcs = _bridge_funcs; @@ -306,7 +303,7 @@ static int tfp410_init(struct device *dev, bool i2c) dvi->bridge.timings = >timings; dvi->bridge.type = DRM_MODE_CONNECTOR_DVID; - ret = tfp410_parse_timings(dvi, i2c); + ret = tfp410_parse_timings(dvi); if (ret) return ret; @@ -346,7 +343,15 @@ static int tfp410_fini(struct device *dev) static int tfp410_probe(struct platform_device *pdev) { - return tfp410_init(>dev, false); + struct tfp410 *dvi; + + dvi = devm_kzalloc(>dev, sizeof(*dvi), GFP_KERNEL); + if (!dvi) + return -ENOMEM; + + dvi->dev = >dev; + + return tfp410_init(dvi); } static int tfp410_remove(struct platform_device *pdev) @@ -370,20 +375,111 @@ static struct platform_driver tfp410_platform_driver = { }; #if IS_ENABLED(CONFIG_I2C) -/* There is currently no i2c functionality. */ + +#define TFP410_VEN_ID_LO 0x00 +#define TFP410_VEN_ID_HI 0x01 +#define TFP410_DEV_ID_LO 0x02 +#define TFP410_DEV_ID_HI 0x03 +#define TFP410_REV_ID 0x04 +#define TFP410_CTL_1_MODE 0x08 +#define TFP410_CTL_2_MODE 0x09 +#define TFP410_CTL_3_MODE 0x0a +#define TFP410_CFG 0x0b +#define TFP410_DE_DLY 0x32 +#define TFP410_DE_CTL 0x33 +#define TFP410_DE_TOP 0x34 +#define TFP410_DE_CNT_LO 0x36 +#define TFP410_DE_CNT_HI 0x37 +#define TFP410_DE_LIN_LO 0x38 +#define TFP410_DE_LIN_HI 0x39 +#define TFP410_H_RES_LO0x3a +#define TFP410_H_RES_HI0x3b +#define TFP410_V_RES_LO0x3c +#define TFP410_V_RES_HI0x3d + +static const struct regmap_config tfp410_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + + .max_register = 0x3d, + .wr_table = &(const struct regmap_access_table) { + .yes_ranges = (const struct regmap_range[]) { + { + .range_min = TFP410_CTL_1_MODE, + .range_max = TFP410_DE_LIN_HI, + }, + }, + .n_yes_ranges = 1, + }, +}; + +static int tfp410_check_version(struct tfp410 *dvi) +{ +
Re: [Intel-gfx] [PATCH v4 1/2] drm/edid: Name the detailed monitor range flags
On Mon, Mar 09, 2020 at 10:35:52AM +0200, Jani Nikula wrote: > On Fri, 06 Mar 2020, Manasi Navare wrote: > > On Fri, Mar 06, 2020 at 12:30:46PM +0200, Jani Nikula wrote: > >> On Thu, 05 Mar 2020, Manasi Navare wrote: > >> > This patch adds defines for the detailed monitor > >> > range flags as per the EDID specification. > >> > > >> > Suggested-by: Ville Syrjälä > >> > Cc: Ville Syrjälä > >> > Cc: Harry Wentland > >> > Cc: Clinton A Taylor > >> > Cc: Kazlauskas Nicholas > >> > Signed-off-by: Manasi Navare > >> > --- > >> > include/drm/drm_edid.h | 5 + > >> > 1 file changed, 5 insertions(+) > >> > > >> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h > >> > index f0b03d401c27..f89c97623845 100644 > >> > --- a/include/drm/drm_edid.h > >> > +++ b/include/drm/drm_edid.h > >> > @@ -91,6 +91,11 @@ struct detailed_data_string { > >> > u8 str[13]; > >> > } __attribute__((packed)); > >> > > >> > +#define EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00 > >> > +#define EDID_RANGE_LIMITS_ONLY_FLAG 0x01 > >> > +#define EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02 > >> > +#define EDID_CVT_SUPPORT_FLAG 0x04 > >> > >> Bikeshed for consideration: > >> > >> drm_edid.h has some macros with DRM_EDID_ prefix, some with EDID_ > >> prefix, and then some with no prefix at all really. Should we start > >> consolidating on something when we add more? > >> > > > > Yes Jani I did notice the same thing and didnt know which convention > > to continue to follow but I noticed that majority of the defines were > > just EDID_ so just used that for these new defines. > > Ah, look again, DRM_EDID_ trumps EDID_ 51 to 15. > > > Is there a particular way you wish to consolidate this and use a specific > > convention for #defines? > > > > I can atleast change these new defines based on a preferred convention and > > then > > separate patches to change the rest in .h and corresponding usages in .c > > files. > > I'd suggest DRM_EDID_ for new ones, perhaps eventually rename old ones. Okay cool, I will rename this to be DRM_EDID_ and then work on renaming others later Thanks for pointing this out. Regards Manasi > > BR, > Jani. > > > > > > Regards > > Manasi > > > >> BR, > >> Jani. > >> > >> > >> > + > >> > struct detailed_data_monitor_range { > >> > u8 min_vfreq; > >> > u8 max_vfreq; > >> > >> -- > >> Jani Nikula, Intel Open Source Graphics Center > > -- > Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RESEND PATCH v7] dt-bindings: display: Add idk-2121wr binding
On Fri, 6 Mar 2020 15:20:31 +, Lad Prabhakar wrote: > From: Fabrizio Castro > > Add binding for the idk-2121wr LVDS panel from Advantech. > > Some panel-specific documentation can be found here: > https://buy.advantech.eu/Displays/Embedded-LCD-Kits-High-Brightness/model-IDK-2121WR-K2FHA2E.htm > > Signed-off-by: Fabrizio Castro > Signed-off-by: Lad Prabhakar > --- > Apologies for flooding in I missed to add the ML email-ids for the earlier > version so resending it. > > Hi All, > > This patch is part of series [1] ("Add dual-LVDS panel support to EK874), > all the patches have been accepted from it except this one. I have fixed > Rob's comments in this version of the patch. > > [1] https://patchwork.kernel.org/cover/11297589/ > > v6->7 > * Added reference to lvds.yaml > * Changed maintainer to myself > * Switched to dual license > * Dropped required properties except for ports as rest are already listed >in lvds.panel > * Dropped Reviewed-by tag of Laurent, due to the changes made it might not >be valid. > > v5->v6: > * No change > > v4->v5: > * No change > > v3->v4: > * Absorbed patch "dt-bindings: display: Add bindings for LVDS > bus-timings" > * Big restructuring after Rob's and Laurent's comments > > v2->v3: > * New patch > > .../display/panel/advantech,idk-2121wr.yaml| 120 > + > 1 file changed, 120 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.yaml > My bot found errors running 'make dt_binding_check' on your patch: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/display/panel/advantech,idk-2121wr.example.dt.yaml: panel-lvds: 'port' is a required property See https://patchwork.ozlabs.org/patch/1250384 Please check and re-submit. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling
Hi Boris, On Mon, Mar 09, 2020 at 09:22:18PM +0100, Boris Brezillon wrote: > On Mon, 9 Mar 2020 21:59:26 +0200 Laurent Pinchart wrote: > > On Mon, Mar 09, 2020 at 08:55:59PM +0100, Boris Brezillon wrote: > > > On Mon, 9 Mar 2020 21:23:06 +0200 Laurent Pinchart wrote: > > > > On Mon, Mar 09, 2020 at 11:50:59AM +0100, Philipp Zabel wrote: > > > > > On Thu, 2019-11-14 at 14:17 +0100, Marek Vasut wrote: > > > > > > The bus_flags and bus_format handling logic does not seem to cover > > > > > > all potential usecases. Specifically, this seems to fail with an > > > > > > "edt,etm0700g0edh6" display attached to an 24bit display interface, > > > > > > with interface-pix-fmt = "rgb24" set in DT. > > > > > > > > > > interface-pix-fmt is a legacy property that was never intended to be > > > > > used as an override for the panel bus format. The bus flags were > > > > > supposed to be set from the display-timings node, back when there was > > > > > no > > > > > of-graph connected panel at all. > > > > > > > > > > That being said, there isn't really a proper alternative that allows > > > > > to > > > > > override the bus format requested by the panel driver in the device > > > > > tree > > > > > to account for weird wiring. We could reuse the bus-width endpoint > > > > > property documented in [1], but that wouldn't completely specify how > > > > > the > > > > > RGB components are to be mapped onto the parallel bus. > > > > > > > > > > [1] Documentation/devicetree/bindings/media/video-interfaces.txt > > > > > > > > Things are funny sometimes, I've run into the exact same problem with a > > > > different display controller today. > > > > > > > > Shouldn't we use the data-shift property from [1] to specify this ? > > > > Combined with Boris' bus format negotiation for bridges, I think we > > > > would have all the components in place to solve this problem properly. > > > > > > I wonder if we shouldn't take more complex pin mappings into account > > > now and go directly for a data-mapping property describing those > > > mappings using a string. This way we'd have a single property that > > > would work for both fully parallel buses (DPI/RGB) and serial (or > > > partially parallel) ones (LVDS). > > > > I'm all for standardization, but I'm not sure data-mapping is the right > > property, at least with its current definition. It's really meant to > > describe how individual bits are mapped to the LVDS time slots. I'm fine > > extending it, but we need to define it clearly. How would you envision > > it being used in this case ? > > Well, clearly the data-width/data-shift approach does not solve all > problems: what do you do if the source R pins are connected to the sink > B pins? Well, the first answer would probably be 'have a serious > discussion with the HW designer responsible for this insanity' :-), but > once you've passed this 'WTF' stage, you'll have to find a way to tell > the source component it should use RGBxyx while the sink should use > BGRxyx (or vice-versa). This is something you can't extract that from > those width/shift props though. My suggestion would be to have one > string per MEDIA_BUS_FMT definition, so we can force things at the DT > level if we really have to. That's basically what the interface-pix-fmt > property was doing, except we would standardize the prop and values and > probably provide helpers so bridge elements don't have to parse this > prop manually. I don't think that would work in the general case though. We may want to use different formats and pick one of them at runtime based on external information (for instance when the sink can accept both RGB and YUV), hardcoding formats in DT isn't a good option. We instead need to add information to DT to specify how lines are connected, and deduce formats based on that. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: panel: Set connector type for OrtusTech COM43H4M85ULC panel
Hi Sam, On Mon, Mar 09, 2020 at 08:45:41PM +0100, Sam Ravnborg wrote: > On Mon, Mar 09, 2020 at 09:01:27PM +0200, Laurent Pinchart wrote: > > On Mon, Mar 09, 2020 at 08:00:47PM +0100, Sam Ravnborg wrote: > > > On Mon, Mar 09, 2020 at 08:42:10PM +0200, Laurent Pinchart wrote: > > > > The OrtusTech COM43H4M85ULC is a DPI panel, set the connector type > > > > accordingly. > > > > > > > > Signed-off-by: Laurent Pinchart > > > > > > Reviewed-by: Sam Ravnborg > > > > > > I assume you will apply to drm-misc-next - OK? > > > > I still haven't got around to using dim :-) > > I can manage - so the entry level is pretty low. > > My lame and simple workflow > > dim update-branches > # save patch from mutt > cat mbox | dim apply > git rebase etc. > dim checkpatch <= if I make changes while applying > #build testing > dim push > > > And when I do my own stuff: > dim update-branches > git checkout -b sam-my-stuff > hacking-hacking > commit, commit > git rebase --exec "dim add-missing-cc" HEAD~5 > > > dim can do much more than that - but the above is > the few dim commands I use. > This help me to do things remotely correct. > > So maybe this is as good as any time to try out dim? As good as any, and as bad as any I suppose :-) There are a few things I don't like with dim, and I haven't found time yet to see how to fix (how live with :-) them yet. Among those issues are - dim requires the kernel tree to be under $DIM_PREFIX. This is my main issue, as I have one kernel tree per project, with and develop for different subsystems in each. I would like dim to instead handle any kernel tree regardless of where it is located on the disk, without requiring me to add another DRM-specific tree to my workflow. - The script auto-updates itself, and I find that to be a security issue that I'm not comfortable with. - The dim script makes a special case of intel repositories internally, which I don't find very fair. Maybe that can be considered as a compensation for Intel's efforts in DRM development, but a model where the community maintaining drm-misc has to resolve conflicts with drm-intel before it reaches drm-next bothers me. The second issue is easy to solve by commenting out auto-update (not sure if Daniel will like that though :-)), and the third one isn't really a blocker, but the first one currently prevents me from using dim. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] Cleanup drm_dp_mst_topology_cbs hooks
On Sat, 2020-03-07 at 14:00 +0530, Pankaj Bharadiya wrote: > drm_dp_mst_topology_mgr_cbs.register_connector callbacks are identical > amongst every driver and don't do anything other than calling > drm_connector_register(). > drm_dp_mst_topology_mgr_cbs.destroy_connector callbacks are identical > amongst every driver and don't do anything other than cleaning up the > connector((drm_connector_unregister()/drm_connector_put())) except for > amdgpu_dm driver where some amdgpu_dm specific code in there. Yeah that amdgpu destruction code kinda stinks a little bit :\. I think we can just drop some of it and move the rest into their connector destruction callbacks. For the whole series: Reviewed-by: Lyude Paul I'm going to go ahead and let the maintainers know I'm going to push this (since there's some minor changes here outside of drm-misc), and push this to drm-misc-next. Then I'll go and write some patches to remove the leftover amd bits and drop the callback for good (I'll cc it to you as well). > > This series aims to cleaup these drm_dp_mst_topology_mgr_cbs hooks. > > Pankaj Bharadiya (5): > drm: Register connector instead of calling register_connector callback > drm: Remove dp mst register connector callbacks > drm/dp_mst: Remove register_connector callback > drm: Add drm_dp_destroy_connector helper and use it > drm: Remove drm dp mst destroy_connector callbacks > > .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 6 -- > drivers/gpu/drm/drm_dp_mst_topology.c | 18 +++--- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 16 > drivers/gpu/drm/nouveau/dispnv50/disp.c | 19 --- > drivers/gpu/drm/radeon/radeon_dp_mst.c| 17 - > include/drm/drm_dp_mst_helper.h | 1 - > 6 files changed, 15 insertions(+), 62 deletions(-) > -- Cheers, Lyude Paul (she/her) Associate Software Engineer at Red Hat ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling
On Mon, 9 Mar 2020 21:59:26 +0200 Laurent Pinchart wrote: > Hi Boris, > > On Mon, Mar 09, 2020 at 08:55:59PM +0100, Boris Brezillon wrote: > > On Mon, 9 Mar 2020 21:23:06 +0200 Laurent Pinchart wrote: > > > On Mon, Mar 09, 2020 at 11:50:59AM +0100, Philipp Zabel wrote: > > > > On Thu, 2019-11-14 at 14:17 +0100, Marek Vasut wrote: > > > > > The bus_flags and bus_format handling logic does not seem to cover > > > > > all potential usecases. Specifically, this seems to fail with an > > > > > "edt,etm0700g0edh6" display attached to an 24bit display interface, > > > > > with interface-pix-fmt = "rgb24" set in DT. > > > > > > > > interface-pix-fmt is a legacy property that was never intended to be > > > > used as an override for the panel bus format. The bus flags were > > > > supposed to be set from the display-timings node, back when there was no > > > > of-graph connected panel at all. > > > > > > > > That being said, there isn't really a proper alternative that allows to > > > > override the bus format requested by the panel driver in the device tree > > > > to account for weird wiring. We could reuse the bus-width endpoint > > > > property documented in [1], but that wouldn't completely specify how the > > > > RGB components are to be mapped onto the parallel bus. > > > > > > > > [1] Documentation/devicetree/bindings/media/video-interfaces.txt > > > > > > Things are funny sometimes, I've run into the exact same problem with a > > > different display controller today. > > > > > > Shouldn't we use the data-shift property from [1] to specify this ? > > > Combined with Boris' bus format negotiation for bridges, I think we > > > would have all the components in place to solve this problem properly. > > > > I wonder if we shouldn't take more complex pin mappings into account > > now and go directly for a data-mapping property describing those > > mappings using a string. This way we'd have a single property that > > would work for both fully parallel buses (DPI/RGB) and serial (or > > partially parallel) ones (LVDS). > > I'm all for standardization, but I'm not sure data-mapping is the right > property, at least with its current definition. It's really meant to > describe how individual bits are mapped to the LVDS time slots. I'm fine > extending it, but we need to define it clearly. How would you envision > it being used in this case ? > Well, clearly the data-width/data-shift approach does not solve all problems: what do you do if the source R pins are connected to the sink B pins? Well, the first answer would probably be 'have a serious discussion with the HW designer responsible for this insanity' :-), but once you've passed this 'WTF' stage, you'll have to find a way to tell the source component it should use RGBxyx while the sink should use BGRxyx (or vice-versa). This is something you can't extract that from those width/shift props though. My suggestion would be to have one string per MEDIA_BUS_FMT definition, so we can force things at the DT level if we really have to. That's basically what the interface-pix-fmt property was doing, except we would standardize the prop and values and probably provide helpers so bridge elements don't have to parse this prop manually. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling
Hi Boris, On Mon, Mar 09, 2020 at 08:55:59PM +0100, Boris Brezillon wrote: > On Mon, 9 Mar 2020 21:23:06 +0200 Laurent Pinchart wrote: > > On Mon, Mar 09, 2020 at 11:50:59AM +0100, Philipp Zabel wrote: > > > On Thu, 2019-11-14 at 14:17 +0100, Marek Vasut wrote: > > > > The bus_flags and bus_format handling logic does not seem to cover > > > > all potential usecases. Specifically, this seems to fail with an > > > > "edt,etm0700g0edh6" display attached to an 24bit display interface, > > > > with interface-pix-fmt = "rgb24" set in DT. > > > > > > interface-pix-fmt is a legacy property that was never intended to be > > > used as an override for the panel bus format. The bus flags were > > > supposed to be set from the display-timings node, back when there was no > > > of-graph connected panel at all. > > > > > > That being said, there isn't really a proper alternative that allows to > > > override the bus format requested by the panel driver in the device tree > > > to account for weird wiring. We could reuse the bus-width endpoint > > > property documented in [1], but that wouldn't completely specify how the > > > RGB components are to be mapped onto the parallel bus. > > > > > > [1] Documentation/devicetree/bindings/media/video-interfaces.txt > > > > Things are funny sometimes, I've run into the exact same problem with a > > different display controller today. > > > > Shouldn't we use the data-shift property from [1] to specify this ? > > Combined with Boris' bus format negotiation for bridges, I think we > > would have all the components in place to solve this problem properly. > > I wonder if we shouldn't take more complex pin mappings into account > now and go directly for a data-mapping property describing those > mappings using a string. This way we'd have a single property that > would work for both fully parallel buses (DPI/RGB) and serial (or > partially parallel) ones (LVDS). I'm all for standardization, but I'm not sure data-mapping is the right property, at least with its current definition. It's really meant to describe how individual bits are mapped to the LVDS time slots. I'm fine extending it, but we need to define it clearly. How would you envision it being used in this case ? -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling
On Mon, 9 Mar 2020 21:23:06 +0200 Laurent Pinchart wrote: > On Mon, Mar 09, 2020 at 11:50:59AM +0100, Philipp Zabel wrote: > > On Thu, 2019-11-14 at 14:17 +0100, Marek Vasut wrote: > > > The bus_flags and bus_format handling logic does not seem to cover > > > all potential usecases. Specifically, this seems to fail with an > > > "edt,etm0700g0edh6" display attached to an 24bit display interface, > > > with interface-pix-fmt = "rgb24" set in DT. > > > > interface-pix-fmt is a legacy property that was never intended to be > > used as an override for the panel bus format. The bus flags were > > supposed to be set from the display-timings node, back when there was no > > of-graph connected panel at all. > > > > That being said, there isn't really a proper alternative that allows to > > override the bus format requested by the panel driver in the device tree > > to account for weird wiring. We could reuse the bus-width endpoint > > property documented in [1], but that wouldn't completely specify how the > > RGB components are to be mapped onto the parallel bus. > > > > [1] Documentation/devicetree/bindings/media/video-interfaces.txt > > Things are funny sometimes, I've run into the exact same problem with a > different display controller today. > > Shouldn't we use the data-shift property from [1] to specify this ? > Combined with Boris' bus format negotiation for bridges, I think we > would have all the components in place to solve this problem properly. I wonder if we shouldn't take more complex pin mappings into account now and go directly for a data-mapping property describing those mappings using a string. This way we'd have a single property that would work for both fully parallel buses (DPI/RGB) and serial (or partially parallel) ones (LVDS). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 09/21] drm: mxsfb: Remove unneeded includes
A fair number of includes are not needed. Drop them, and add a couple of required includes that were included indirectly. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 12 +++- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 5 - 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index aef72adabf41..c4f1575b4210 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -10,19 +10,13 @@ #include #include -#include -#include +#include -#include - -#include +#include #include #include -#include +#include #include -#include -#include -#include #include #include diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index cffc70257bd3..204c1e52e9aa 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -9,15 +9,10 @@ */ #include -#include #include -#include #include #include -#include -#include #include -#include #include #include -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 04/21] drm: mxsfb: Remove unused macros from mxsfb_regs.h
mxsfb_regs.h defines macros related to register bits. Some of them are not used and don't clearly map to any particular register, so their purpose isn't known. Remove them. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_regs.h | 8 1 file changed, 8 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h index 713d8f830135..78e6cb754712 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h +++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h @@ -91,17 +91,9 @@ #define MXSFB_MAX_XRES 0x #define MXSFB_MAX_YRES 0x -#define RED 0 -#define GREEN 1 -#define BLUE 2 -#define TRANSP 3 - #define STMLCDIF_8BIT 1 /* pixel data bus to the display is of 8 bit width */ #define STMLCDIF_16BIT 0 /* pixel data bus to the display is of 16 bit width */ #define STMLCDIF_18BIT 2 /* pixel data bus to the display is of 18 bit width */ #define STMLCDIF_24BIT 3 /* pixel data bus to the display is of 24 bit width */ -#define MXSFB_SYNC_DATA_ENABLE_HIGH_ACT(1 << 6) -#define MXSFB_SYNC_DOTCLK_FALLING_ACT (1 << 7) /* negative edge sampling */ - #endif /* __MXSFB_REGS_H__ */ -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 14/21] drm: mxsfb: Enable vblank handling
Enable vblank handling when the CRTC is turned on and disable it when it is turned off. This requires moving vblank init after the KMS pipeline initialisation, otherwise drm_vblank_init() gets called with 0 CRTCs. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 15 +-- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 6 +- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index e324bd2a63a5..72b4f6a947a4 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -160,12 +160,6 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags) pm_runtime_enable(drm->dev); - ret = drm_vblank_init(drm, drm->mode_config.num_crtc); - if (ret < 0) { - dev_err(drm->dev, "Failed to initialise vblank\n"); - goto err_vblank; - } - /* Modeset init */ drm_mode_config_init(drm); @@ -175,6 +169,15 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags) goto err_vblank; } + ret = drm_vblank_init(drm, drm->mode_config.num_crtc); + if (ret < 0) { + dev_err(drm->dev, "Failed to initialise vblank\n"); + goto err_vblank; + } + + /* Start with vertical blanking interrupt reporting disabled. */ + drm_crtc_vblank_off(>crtc); + ret = mxsfb_attach_bridge(mxsfb); if (ret) { dev_err(drm->dev, "Cannot connect bridge: %d\n", ret); diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index ac2696c8483d..640305fb1068 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -322,8 +322,10 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc, dma_addr_t paddr; pm_runtime_get_sync(drm->dev); - mxsfb_enable_axi_clk(mxsfb); + + drm_crtc_vblank_on(crtc); + mxsfb_crtc_mode_set_nofb(mxsfb); /* Write cur_buf as well to avoid an initial corrupt frame */ @@ -353,6 +355,8 @@ static void mxsfb_crtc_atomic_disable(struct drm_crtc *crtc, } spin_unlock_irq(>event_lock); + drm_crtc_vblank_off(crtc); + mxsfb_disable_axi_clk(mxsfb); pm_runtime_put_sync(drm->dev); } -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 19/21] drm: mxsfb: Turn mxsfb_set_pixel_fmt() into a void function
The mxsfb_set_pixel_fmt() function returns an error when the selected pixel format is unsupported. This can never happen, as such errors are caught by the DRM core. Remove the error check. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index 640305fb1068..19b937b383cf 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -43,7 +43,7 @@ static u32 set_hsync_pulse_width(struct mxsfb_drm_private *mxsfb, u32 val) } /* Setup the MXSFB registers for decoding the pixels out of the framebuffer */ -static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb) +static void mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb) { struct drm_device *drm = mxsfb->drm; const u32 format = mxsfb->crtc.primary->state->fb->format->format; @@ -67,15 +67,10 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb) /* Do not use packed pixels = one pixel per word instead. */ ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7); break; - default: - dev_err(drm->dev, "Unhandled pixel format %08x\n", format); - return -EINVAL; } writel(ctrl1, mxsfb->base + LCDC_CTRL1); writel(ctrl, mxsfb->base + LCDC_CTRL); - - return 0; } static void mxsfb_set_bus_fmt(struct mxsfb_drm_private *mxsfb) @@ -218,9 +213,7 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) /* Clear the FIFOs */ writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_SET); - err = mxsfb_set_pixel_fmt(mxsfb); - if (err) - return; + mxsfb_set_pixel_fmt(mxsfb); clk_set_rate(mxsfb->clk, m->crtc_clock * 1000); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 16/21] drm: mxsfb: Add i.MX7 to the list of supported SoCs in Kconfig
Extend the Kconfig option description by listing the i.MX7 SoCs, as they are supported by the same driver. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/Kconfig b/drivers/gpu/drm/mxsfb/Kconfig index 0dca8f27169e..e91841f8f8a2 100644 --- a/drivers/gpu/drm/mxsfb/Kconfig +++ b/drivers/gpu/drm/mxsfb/Kconfig @@ -5,7 +5,7 @@ config DRM_MXS Choose this option to select drivers for MXS FB devices config DRM_MXSFB - tristate "i.MX23/i.MX28/i.MX6SX MXSFB LCD controller" + tristate "i.MX23/i.MX28/i.MX6SX/i.MX7 MXSFB LCD controller" depends on DRM && OF depends on COMMON_CLK select DRM_MXS @@ -14,7 +14,7 @@ config DRM_MXSFB select DRM_KMS_CMA_HELPER select DRM_PANEL help - Choose this option if you have an i.MX23/i.MX28/i.MX6SX MXSFB + Choose this option if you have an i.MX23/i.MX28/i.MX6SX/i.MX7 MXSFB LCD controller. If M is selected the module will be called mxsfb. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 00/21] drm: mxsfb: Add i.MX7 support
Hello, This patch series adds i.MX7 support to the mxsfb driver. The eLCDIF instance found in the i.MX7 is backward-compatible with the already supported LCDC v4, but has extended features amongst which the most notable one is a second plane. The first 9 patches (01/21 to 09/21) contain miscellaneous cleanups and refactoring to prepare for what is to come. Patch 10/21 starts the real work with removal of the DRM simple display pipeline helper, as it doesn't support multiple planes. The next patch (11/21) is an additional cleanup. Patches 12/21 to 14/21 fix vblank handling that I found to be broken when testing on my device. Patch 15/21 then performs an additional small cleanup, and patch 16/21 starts official support for i.MX7 by mentioning it in Kconfig. Patch 17/21 adds a new device model for the i.MX6SX and i.MX7 eLCDIF. After three additional cleanups in patches 18/21 to 20/21, patch 21/21 finally adds support for the second plane. The code is based on drm-misc-next and has been tested on an i.MX7D platform with a DPI panel. Laurent Pinchart (21): drm: mxsfb: Remove fbdev leftovers drm: mxsfb: Use drm_panel_bridge drm: mxsfb: Use BIT() macro to define register bitfields drm: mxsfb: Remove unused macros from mxsfb_regs.h drm: mxsfb: Clarify format and bus width configuration drm: mxsfb: Pass mxsfb_drm_private pointer to mxsfb_reset_block() drm: mxsfb: Use LCDC_CTRL register name explicitly drm: mxsfb: Remove register definitions from mxsfb_crtc.c drm: mxsfb: Remove unneeded includes drm: mxsfb: Stop using DRM simple display pipeline helper drm: mxsfb: Rename mxsfb_crtc.c to mxsfb_kms.c drm: mxsfb: Move vblank event arm to CRTC .atomic_flush() drm: mxsfb: Don't touch AXI clock in IRQ context drm: mxsfb: Enable vblank handling drm: mxsfb: Remove mxsfb_devdata unused fields drm: mxsfb: Add i.MX7 to the list of supported SoCs in Kconfig drm: mxsfb: Update internal IP version number for i.MX6SX drm: mxsfb: Drop non-OF support drm: mxsfb: Turn mxsfb_set_pixel_fmt() into a void function drm: mxsfb: Merge mxsfb_set_pixel_fmt() and mxsfb_set_bus_fmt() drm: mxsfb: Support the alpha plane drivers/gpu/drm/mxsfb/Kconfig | 4 +- drivers/gpu/drm/mxsfb/Makefile | 2 +- drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 343 - drivers/gpu/drm/mxsfb/mxsfb_drv.c | 246 - drivers/gpu/drm/mxsfb/mxsfb_drv.h | 42 ++- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 565 + drivers/gpu/drm/mxsfb/mxsfb_out.c | 99 - drivers/gpu/drm/mxsfb/mxsfb_regs.h | 107 +++--- 8 files changed, 730 insertions(+), 678 deletions(-) delete mode 100644 drivers/gpu/drm/mxsfb/mxsfb_crtc.c create mode 100644 drivers/gpu/drm/mxsfb/mxsfb_kms.c delete mode 100644 drivers/gpu/drm/mxsfb/mxsfb_out.c -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 15/21] drm: mxsfb: Remove mxsfb_devdata unused fields
The debug0 and ipversion fields of the mxsfb_devdata structure are unused. Remove them. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 4 drivers/gpu/drm/mxsfb/mxsfb_drv.h | 2 -- 2 files changed, 6 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 72b4f6a947a4..7c9a041f5f6d 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -42,19 +42,15 @@ static const struct mxsfb_devdata mxsfb_devdata[] = { .transfer_count = LCDC_V3_TRANSFER_COUNT, .cur_buf= LCDC_V3_CUR_BUF, .next_buf = LCDC_V3_NEXT_BUF, - .debug0 = LCDC_V3_DEBUG0, .hs_wdth_mask = 0xff, .hs_wdth_shift = 24, - .ipversion = 3, }, [MXSFB_V4] = { .transfer_count = LCDC_V4_TRANSFER_COUNT, .cur_buf= LCDC_V4_CUR_BUF, .next_buf = LCDC_V4_NEXT_BUF, - .debug0 = LCDC_V4_DEBUG0, .hs_wdth_mask = 0x3fff, .hs_wdth_shift = 18, - .ipversion = 4, }, }; diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h index edd766ad254f..607a6a5e6be3 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h @@ -19,10 +19,8 @@ struct mxsfb_devdata { unsigned int transfer_count; unsigned int cur_buf; unsigned int next_buf; - unsigned int debug0; unsigned int hs_wdth_mask; unsigned int hs_wdth_shift; - unsigned int ipversion; }; struct mxsfb_drm_private { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 05/21] drm: mxsfb: Clarify format and bus width configuration
Replace the convoluted way to set the format and bus width through difficult to read macros with more explicit ones. Also remove the outdated comment related to the limitations on bus width setting as it doesn't apply anymore (the bus width can be specified through the display_info bus format). Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 17 + drivers/gpu/drm/mxsfb/mxsfb_regs.h | 17 - 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index b69ace8bf526..8b6339316929 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -52,13 +52,6 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb) ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER; - /* -* WARNING: The bus width, CTRL_SET_BUS_WIDTH(), is configured to -* match the selected mode here. This differs from the original -* MXSFB driver, which had the option to configure the bus width -* to arbitrary value. This limitation should not pose an issue. -*/ - /* CTRL1 contains IRQ config and status bits, preserve those. */ ctrl1 = readl(mxsfb->base + LCDC_CTRL1); ctrl1 &= CTRL1_CUR_FRAME_DONE_IRQ_EN | CTRL1_CUR_FRAME_DONE_IRQ; @@ -66,12 +59,12 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb) switch (format) { case DRM_FORMAT_RGB565: dev_dbg(drm->dev, "Setting up RGB565 mode\n"); - ctrl |= CTRL_SET_WORD_LENGTH(0); + ctrl |= CTRL_WORD_LENGTH_16; ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0xf); break; case DRM_FORMAT_XRGB: dev_dbg(drm->dev, "Setting up XRGB mode\n"); - ctrl |= CTRL_SET_WORD_LENGTH(3); + ctrl |= CTRL_WORD_LENGTH_24; /* Do not use packed pixels = one pixel per word instead. */ ctrl1 |= CTRL1_SET_BYTE_PACKAGING(0x7); break; @@ -104,13 +97,13 @@ static void mxsfb_set_bus_fmt(struct mxsfb_drm_private *mxsfb) reg &= ~CTRL_BUS_WIDTH_MASK; switch (bus_format) { case MEDIA_BUS_FMT_RGB565_1X16: - reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_16BIT); + reg |= CTRL_BUS_WIDTH_16; break; case MEDIA_BUS_FMT_RGB666_1X18: - reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_18BIT); + reg |= CTRL_BUS_WIDTH_18; break; case MEDIA_BUS_FMT_RGB888_1X24: - reg |= CTRL_SET_BUS_WIDTH(STMLCDIF_24BIT); + reg |= CTRL_BUS_WIDTH_24; break; default: dev_err(drm->dev, "Unknown media bus format %d\n", bus_format); diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h index 78e6cb754712..8ebb52bb1b46 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h +++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h @@ -34,11 +34,15 @@ #define CTRL_VSYNC_MODEBIT(18) #define CTRL_DOTCLK_MODE BIT(17) #define CTRL_DATA_SELECT BIT(16) -#define CTRL_SET_BUS_WIDTH(x) (((x) & 0x3) << 10) -#define CTRL_GET_BUS_WIDTH(x) (((x) >> 10) & 0x3) +#define CTRL_BUS_WIDTH_16 (0 << 10) +#define CTRL_BUS_WIDTH_8 (1 << 10) +#define CTRL_BUS_WIDTH_18 (2 << 10) +#define CTRL_BUS_WIDTH_24 (3 << 10) #define CTRL_BUS_WIDTH_MASK(0x3 << 10) -#define CTRL_SET_WORD_LENGTH(x)(((x) & 0x3) << 8) -#define CTRL_GET_WORD_LENGTH(x)(((x) >> 8) & 0x3) +#define CTRL_WORD_LENGTH_16(0 << 8) +#define CTRL_WORD_LENGTH_8 (1 << 8) +#define CTRL_WORD_LENGTH_18(2 << 8) +#define CTRL_WORD_LENGTH_24(3 << 8) #define CTRL_MASTERBIT(5) #define CTRL_DF16 BIT(3) #define CTRL_DF18 BIT(2) @@ -91,9 +95,4 @@ #define MXSFB_MAX_XRES 0x #define MXSFB_MAX_YRES 0x -#define STMLCDIF_8BIT 1 /* pixel data bus to the display is of 8 bit width */ -#define STMLCDIF_16BIT 0 /* pixel data bus to the display is of 16 bit width */ -#define STMLCDIF_18BIT 2 /* pixel data bus to the display is of 18 bit width */ -#define STMLCDIF_24BIT 3 /* pixel data bus to the display is of 24 bit width */ - #endif /* __MXSFB_REGS_H__ */ -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 10/21] drm: mxsfb: Stop using DRM simple display pipeline helper
The DRM simple display pipeline helper only supports a single plane. In order to prepare for support of the alpha plane on i.MX6SX and i.MX7, move away from the helper. No new feature is added. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 184 ++--- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 108 ++--- drivers/gpu/drm/mxsfb/mxsfb_drv.h | 23 ++-- 3 files changed, 197 insertions(+), 118 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index c4f1575b4210..8f339adb8d04 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -9,15 +9,21 @@ */ #include +#include #include +#include #include +#include +#include #include #include +#include #include #include #include -#include +#include +#include #include #include "mxsfb_drv.h" @@ -26,6 +32,10 @@ /* 1 second delay should be plenty of time for block reset */ #define RESET_TIMEOUT 100 +/* - + * CRTC + */ + static u32 set_hsync_pulse_width(struct mxsfb_drm_private *mxsfb, u32 val) { return (val & mxsfb->devdata->hs_wdth_mask) << @@ -35,9 +45,8 @@ static u32 set_hsync_pulse_width(struct mxsfb_drm_private *mxsfb, u32 val) /* Setup the MXSFB registers for decoding the pixels out of the framebuffer */ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb) { - struct drm_crtc *crtc = >pipe.crtc; - struct drm_device *drm = crtc->dev; - const u32 format = crtc->primary->state->fb->format->format; + struct drm_device *drm = mxsfb->drm; + const u32 format = mxsfb->crtc.primary->state->fb->format->format; u32 ctrl, ctrl1; ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER; @@ -71,8 +80,7 @@ static int mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb) static void mxsfb_set_bus_fmt(struct mxsfb_drm_private *mxsfb) { - struct drm_crtc *crtc = >pipe.crtc; - struct drm_device *drm = crtc->dev; + struct drm_device *drm = mxsfb->drm; u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; u32 reg; @@ -175,7 +183,7 @@ static int mxsfb_reset_block(struct mxsfb_drm_private *mxsfb) static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb) { - struct drm_framebuffer *fb = mxsfb->pipe.plane.state->fb; + struct drm_framebuffer *fb = mxsfb->plane.state->fb; struct drm_gem_cma_object *gem; if (!fb) @@ -190,8 +198,8 @@ static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb) static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) { - struct drm_device *drm = mxsfb->pipe.crtc.dev; - struct drm_display_mode *m = >pipe.crtc.state->adjusted_mode; + struct drm_device *drm = mxsfb->crtc.dev; + struct drm_display_mode *m = >crtc.state->adjusted_mode; u32 bus_flags = mxsfb->connector->display_info.bus_flags; u32 vdctrl0, vsync_pulse_len, hsync_pulse_len; int err; @@ -273,10 +281,29 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) mxsfb->base + LCDC_VDCTRL4); } -void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb) +static int mxsfb_crtc_atomic_check(struct drm_crtc *crtc, + struct drm_crtc_state *state) { + bool has_primary = state->plane_mask & + drm_plane_mask(crtc->primary); + + /* The primary plane has to be enabled when the CRTC is active. */ + if (has_primary != state->active) + return -EINVAL; + + /* TODO: Is this needed ? */ + return drm_atomic_add_affected_planes(state->state, crtc); +} + +static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc, +struct drm_crtc_state *old_state) +{ + struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(crtc->dev); + struct drm_device *drm = mxsfb->drm; dma_addr_t paddr; + pm_runtime_get_sync(drm->dev); + mxsfb_enable_axi_clk(mxsfb); mxsfb_crtc_mode_set_nofb(mxsfb); @@ -290,17 +317,100 @@ void mxsfb_crtc_enable(struct mxsfb_drm_private *mxsfb) mxsfb_enable_controller(mxsfb); } -void mxsfb_crtc_disable(struct mxsfb_drm_private *mxsfb) +static void mxsfb_crtc_atomic_disable(struct drm_crtc *crtc, + struct drm_crtc_state *old_state) { + struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(crtc->dev); + struct drm_device *drm = mxsfb->drm; + struct drm_pending_vblank_event *event; + mxsfb_disable_controller(mxsfb); mxsfb_disable_axi_clk(mxsfb); + + pm_runtime_put_sync(drm->dev); + + spin_lock_irq(>event_lock); + event = crtc->state->event; + if (event) { + crtc->state->event = NULL; +
[PATCH 11/21] drm: mxsfb: Rename mxsfb_crtc.c to mxsfb_kms.c
The mxsfb_crtc.c file doesn't handle just the CRTC, but also the other KMS objects. Rename it accordingly. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/Makefile | 2 +- drivers/gpu/drm/mxsfb/{mxsfb_crtc.c => mxsfb_kms.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename drivers/gpu/drm/mxsfb/{mxsfb_crtc.c => mxsfb_kms.c} (100%) diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile index 811584e54ad1..26d153896d72 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-y := mxsfb_drv.o mxsfb_kms.o obj-$(CONFIG_DRM_MXSFB)+= mxsfb.o diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c similarity index 100% rename from drivers/gpu/drm/mxsfb/mxsfb_crtc.c rename to drivers/gpu/drm/mxsfb/mxsfb_kms.c -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 07/21] drm: mxsfb: Use LCDC_CTRL register name explicitly
The LCDC_CTRL register is located at address 0x. Some of the accesses to the register simply use the mxsfb->base address. Reference the LCDC_CTRL register explicitly instead to clarify the code. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index be60c4021e2f..722bd9b4f5f9 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -170,17 +170,17 @@ static int mxsfb_reset_block(struct mxsfb_drm_private *mxsfb) { int ret; - ret = clear_poll_bit(mxsfb->base, MODULE_SFTRST); + ret = clear_poll_bit(mxsfb->base + LCDC_CTRL, MODULE_SFTRST); if (ret) return ret; - writel(MODULE_CLKGATE, mxsfb->base + MXS_CLR_ADDR); + writel(MODULE_CLKGATE, mxsfb->base + LCDC_CTRL + MXS_CLR_ADDR); - ret = clear_poll_bit(mxsfb->base, MODULE_SFTRST); + ret = clear_poll_bit(mxsfb->base + LCDC_CTRL, MODULE_SFTRST); if (ret) return ret; - return clear_poll_bit(mxsfb->base, MODULE_CLKGATE); + return clear_poll_bit(mxsfb->base + LCDC_CTRL, MODULE_CLKGATE); } static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 18/21] drm: mxsfb: Drop non-OF support
The mxsfb driver is only used by OF platforms. Drop non-OF support. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 25 +++-- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 2316c12c5c42..ed8e3f7bc27c 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -130,7 +130,8 @@ static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb) return 0; } -static int mxsfb_load(struct drm_device *drm, unsigned long flags) +static int mxsfb_load(struct drm_device *drm, + const struct mxsfb_devdata *devdata) { struct platform_device *pdev = to_platform_device(drm->dev); struct mxsfb_drm_private *mxsfb; @@ -143,7 +144,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long flags) mxsfb->drm = drm; drm->dev_private = mxsfb; - mxsfb->devdata = _devdata[pdev->id_entry->driver_data]; + mxsfb->devdata = devdata; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); mxsfb->base = devm_ioremap_resource(drm->dev, res); @@ -288,18 +289,10 @@ static struct drm_driver mxsfb_driver = { .minor = 0, }; -static const struct platform_device_id mxsfb_devtype[] = { - { .name = "imx23-fb", .driver_data = MXSFB_V3, }, - { .name = "imx28-fb", .driver_data = MXSFB_V4, }, - { .name = "imx6sx-fb", .driver_data = MXSFB_V6, }, - { /* sentinel */ } -}; -MODULE_DEVICE_TABLE(platform, mxsfb_devtype); - static const struct of_device_id mxsfb_dt_ids[] = { - { .compatible = "fsl,imx23-lcdif", .data = _devtype[0], }, - { .compatible = "fsl,imx28-lcdif", .data = _devtype[1], }, - { .compatible = "fsl,imx6sx-lcdif", .data = _devtype[2], }, + { .compatible = "fsl,imx23-lcdif", .data = _devdata[MXSFB_V3], }, + { .compatible = "fsl,imx28-lcdif", .data = _devdata[MXSFB_V4], }, + { .compatible = "fsl,imx6sx-lcdif", .data = _devdata[MXSFB_V6], }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, mxsfb_dt_ids); @@ -314,14 +307,11 @@ static int mxsfb_probe(struct platform_device *pdev) if (!pdev->dev.of_node) return -ENODEV; - if (of_id) - pdev->id_entry = of_id->data; - drm = drm_dev_alloc(_driver, >dev); if (IS_ERR(drm)) return PTR_ERR(drm); - ret = mxsfb_load(drm, 0); + ret = mxsfb_load(drm, of_id->data); if (ret) goto err_free; @@ -375,7 +365,6 @@ static const struct dev_pm_ops mxsfb_pm_ops = { static struct platform_driver mxsfb_platform_driver = { .probe = mxsfb_probe, .remove = mxsfb_remove, - .id_table = mxsfb_devtype, .driver = { .name = "mxsfb", .of_match_table = mxsfb_dt_ids, -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 12/21] drm: mxsfb: Move vblank event arm to CRTC .atomic_flush()
The vblank event is armed in the plane .atomic_update(). This works fine as we have a single plane, but will break as soon as multiple planes are supported (not to mention it's logically the wrong place to perform the operation). Move it to CRTC .atomic_flush(). Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 35 ++- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index 8f339adb8d04..ebe0785694cb 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -295,6 +295,25 @@ static int mxsfb_crtc_atomic_check(struct drm_crtc *crtc, return drm_atomic_add_affected_planes(state->state, crtc); } +static void mxsfb_crtc_atomic_flush(struct drm_crtc *crtc, + struct drm_crtc_state *old_state) +{ + struct drm_pending_vblank_event *event; + + event = crtc->state->event; + crtc->state->event = NULL; + + if (!event) + return; + + spin_lock_irq(>dev->event_lock); + if (drm_crtc_vblank_get(crtc) == 0) + drm_crtc_arm_vblank_event(crtc, event); + else + drm_crtc_send_vblank_event(crtc, event); + spin_unlock_irq(>dev->event_lock); +} + static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc, struct drm_crtc_state *old_state) { @@ -364,6 +383,7 @@ static void mxsfb_crtc_disable_vblank(struct drm_crtc *crtc) static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = { .atomic_check = mxsfb_crtc_atomic_check, + .atomic_flush = mxsfb_crtc_atomic_flush, .atomic_enable = mxsfb_crtc_atomic_enable, .atomic_disable = mxsfb_crtc_atomic_disable, }; @@ -410,23 +430,8 @@ static void mxsfb_plane_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_pstate) { struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev); - struct drm_crtc *crtc = >crtc; - struct drm_pending_vblank_event *event; dma_addr_t paddr; - spin_lock_irq(>dev->event_lock); - event = crtc->state->event; - if (event) { - crtc->state->event = NULL; - - if (drm_crtc_vblank_get(crtc) == 0) { - drm_crtc_arm_vblank_event(crtc, event); - } else { - drm_crtc_send_vblank_event(crtc, event); - } - } - spin_unlock_irq(>dev->event_lock); - paddr = mxsfb_get_fb_paddr(mxsfb); if (paddr) { mxsfb_enable_axi_clk(mxsfb); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 02/21] drm: mxsfb: Use drm_panel_bridge
Replace the manual connector implementation based on drm_panel with the drm_panel_bridge helper. This simplifies the mxsfb driver by removing connector-related code, and standardizing all pipeline control operations on bridges. A hack is needed to get hold of the connector, as that's our only source of bus flags and formats for now. As soon as the bridge API provides us with that information this can be fixed. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/Makefile| 2 +- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 105 ++ drivers/gpu/drm/mxsfb/mxsfb_drv.h | 5 +- drivers/gpu/drm/mxsfb/mxsfb_out.c | 99 4 files changed, 53 insertions(+), 158 deletions(-) delete mode 100644 drivers/gpu/drm/mxsfb/mxsfb_out.c diff --git a/drivers/gpu/drm/mxsfb/Makefile b/drivers/gpu/drm/mxsfb/Makefile index ff6e358088fa..811584e54ad1 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 +mxsfb-y := mxsfb_drv.o mxsfb_crtc.o obj-$(CONFIG_DRM_MXSFB)+= mxsfb.o diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 2e6068d96034..cffc70257bd3 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -29,7 +29,6 @@ #include #include #include -#include #include #include #include @@ -100,29 +99,11 @@ static void mxsfb_pipe_enable(struct drm_simple_display_pipe *pipe, struct drm_crtc_state *crtc_state, struct drm_plane_state *plane_state) { - struct drm_connector *connector; struct mxsfb_drm_private *mxsfb = drm_pipe_to_mxsfb_drm_private(pipe); struct drm_device *drm = pipe->plane.dev; - if (!mxsfb->connector) { - list_for_each_entry(connector, - >mode_config.connector_list, - head) - if (connector->encoder == >pipe.encoder) { - mxsfb->connector = connector; - break; - } - } - - if (!mxsfb->connector) { - dev_warn(drm->dev, "No connector attached, using default\n"); - mxsfb->connector = >panel_connector; - } - pm_runtime_get_sync(drm->dev); - drm_panel_prepare(mxsfb->panel); mxsfb_crtc_enable(mxsfb); - drm_panel_enable(mxsfb->panel); } static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe) @@ -132,9 +113,7 @@ static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe) struct drm_crtc *crtc = >crtc; struct drm_pending_vblank_event *event; - drm_panel_disable(mxsfb->panel); mxsfb_crtc_disable(mxsfb); - drm_panel_unprepare(mxsfb->panel); pm_runtime_put_sync(drm->dev); spin_lock_irq(>event_lock); @@ -144,9 +123,6 @@ static void mxsfb_pipe_disable(struct drm_simple_display_pipe *pipe) drm_crtc_send_vblank_event(crtc, event); } spin_unlock_irq(>event_lock); - - if (mxsfb->connector != >panel_connector) - mxsfb->connector = NULL; } static void mxsfb_pipe_update(struct drm_simple_display_pipe *pipe, @@ -190,6 +166,48 @@ static struct drm_simple_display_pipe_funcs mxsfb_funcs = { .disable_vblank = mxsfb_pipe_disable_vblank, }; +static int mxsfb_attach_bridge(struct mxsfb_drm_private *mxsfb) +{ + struct drm_device *drm = mxsfb->drm; + struct drm_connector_list_iter iter; + struct drm_panel *panel; + struct drm_bridge *bridge; + int ret; + + ret = drm_of_find_panel_or_bridge(drm->dev->of_node, 0, 0, , + ); + if (ret) + return ret; + + if (panel) { + bridge = devm_drm_panel_bridge_add(drm->dev, panel); + if (IS_ERR(bridge)) + return PTR_ERR(bridge); + } + + if (!bridge) + return -ENODEV; + + ret = drm_simple_display_pipe_attach_bridge(>pipe, bridge); + if (ret) { + DRM_DEV_ERROR(drm->dev, + "failed to attach bridge: %d\n", ret); + return ret; + } + + mxsfb->bridge = bridge; + + /* +* Get hold of the connector. This is a bit of a hack, until the bridge +* API gives us bus flags and formats. +*/ + drm_connector_list_iter_begin(drm, ); + mxsfb->connector = drm_connector_list_iter_next(); + drm_connector_list_iter_end(); + + return 0; +} + static int mxsfb_load(struct drm_device *drm, unsigned long flags) { struct platform_device *pdev = to_platform_device(drm->dev); @@ -201,6 +219,7 @@ static int mxsfb_load(struct drm_device *drm, unsigned long
[PATCH 13/21] drm: mxsfb: Don't touch AXI clock in IRQ context
The driver attempts agressive power management by enabling and disabling the AXI clock around register accesses. This results in attempts to enable and disable the clock in the IRQ handler, which is a no-go as preparing or unpreparing the clock may sleep. On the other hand, the driver enables the AXI clock when enabling the CRTC and keeps it enabled until the CRTC is disabled. This is correct, and renders the power management attempt pointless, as interrupts are not supposed to occur when the CRTC is off. The same reasoning can be applied to the CRTC .enable_vblank() and .disable_vblank() that are not supposed to be called when the CRTC off and thus don't require manual handling of the AXI clock. Furthermore, vblank handling is never enabled, which results in the vblank enable and disable handlers never being called. To fix this, remove the manual clock handling in the IRQ, the CRTC .enable_vblank() and .disable_vblank() handlers and the plane .atomic_update() handler. We however need to handle the clock manually in mxsfb_irq_disable() as is calls .disable_vblank() manually and is used both at probe and remove time. The clock disabling is also moved to the last step of the mxsfb_crtc_atomic_disable() function, to prepare for enabling vblank handling. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 6 ++ drivers/gpu/drm/mxsfb/mxsfb_kms.c | 15 --- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index a8da92976d13..e324bd2a63a5 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -231,7 +231,9 @@ static void mxsfb_irq_disable(struct drm_device *drm) { struct mxsfb_drm_private *mxsfb = drm->dev_private; + mxsfb_enable_axi_clk(mxsfb); mxsfb->crtc.funcs->disable_vblank(>crtc); + mxsfb_disable_axi_clk(mxsfb); } static irqreturn_t mxsfb_irq_handler(int irq, void *data) @@ -240,8 +242,6 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data) struct mxsfb_drm_private *mxsfb = drm->dev_private; u32 reg; - mxsfb_enable_axi_clk(mxsfb); - reg = readl(mxsfb->base + LCDC_CTRL1); if (reg & CTRL1_CUR_FRAME_DONE_IRQ) @@ -249,8 +249,6 @@ static irqreturn_t mxsfb_irq_handler(int irq, void *data) writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR); - mxsfb_disable_axi_clk(mxsfb); - return IRQ_HANDLED; } diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index ebe0785694cb..ac2696c8483d 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -344,9 +344,6 @@ static void mxsfb_crtc_atomic_disable(struct drm_crtc *crtc, struct drm_pending_vblank_event *event; mxsfb_disable_controller(mxsfb); - mxsfb_disable_axi_clk(mxsfb); - - pm_runtime_put_sync(drm->dev); spin_lock_irq(>event_lock); event = crtc->state->event; @@ -355,6 +352,9 @@ static void mxsfb_crtc_atomic_disable(struct drm_crtc *crtc, drm_crtc_send_vblank_event(crtc, event); } spin_unlock_irq(>event_lock); + + mxsfb_disable_axi_clk(mxsfb); + pm_runtime_put_sync(drm->dev); } static int mxsfb_crtc_enable_vblank(struct drm_crtc *crtc) @@ -362,10 +362,8 @@ static int mxsfb_crtc_enable_vblank(struct drm_crtc *crtc) struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(crtc->dev); /* Clear and enable VBLANK IRQ */ - mxsfb_enable_axi_clk(mxsfb); writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR); writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_SET); - mxsfb_disable_axi_clk(mxsfb); return 0; } @@ -375,10 +373,8 @@ static void mxsfb_crtc_disable_vblank(struct drm_crtc *crtc) struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(crtc->dev); /* Disable and clear VBLANK IRQ */ - mxsfb_enable_axi_clk(mxsfb); writel(CTRL1_CUR_FRAME_DONE_IRQ_EN, mxsfb->base + LCDC_CTRL1 + REG_CLR); writel(CTRL1_CUR_FRAME_DONE_IRQ, mxsfb->base + LCDC_CTRL1 + REG_CLR); - mxsfb_disable_axi_clk(mxsfb); } static const struct drm_crtc_helper_funcs mxsfb_crtc_helper_funcs = { @@ -433,11 +429,8 @@ static void mxsfb_plane_atomic_update(struct drm_plane *plane, dma_addr_t paddr; paddr = mxsfb_get_fb_paddr(mxsfb); - if (paddr) { - mxsfb_enable_axi_clk(mxsfb); + if (paddr) writel(paddr, mxsfb->base + mxsfb->devdata->next_buf); - mxsfb_disable_axi_clk(mxsfb); - } } static const struct drm_plane_helper_funcs mxsfb_plane_helper_funcs = { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 20/21] drm: mxsfb: Merge mxsfb_set_pixel_fmt() and mxsfb_set_bus_fmt()
The mxsfb_set_pixel_fmt() and mxsfb_set_bus_fmt() functions both deal with format configuration, are always called in a row from mxsfb_crtc_mode_set_nofb(), and set fields from the LCDC_CTRL register. This requires a read-modify-update cycle in mxsfb_set_bus_fmt(). Make this more efficient by merging them together. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 47 +-- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index 19b937b383cf..f81f8c222c13 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -42,13 +42,23 @@ static u32 set_hsync_pulse_width(struct mxsfb_drm_private *mxsfb, u32 val) mxsfb->devdata->hs_wdth_shift; } -/* Setup the MXSFB registers for decoding the pixels out of the framebuffer */ -static void mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb) +/* + * Setup the MXSFB registers for decoding the pixels out of the framebuffer and + * outputting them on the bus. + */ +static void mxsfb_set_formats(struct mxsfb_drm_private *mxsfb) { struct drm_device *drm = mxsfb->drm; const u32 format = mxsfb->crtc.primary->state->fb->format->format; + u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; u32 ctrl, ctrl1; + if (mxsfb->connector->display_info.num_bus_formats) + bus_format = mxsfb->connector->display_info.bus_formats[0]; + + DRM_DEV_DEBUG_DRIVER(drm->dev, "Using bus_format: 0x%08X\n", +bus_format); + ctrl = CTRL_BYPASS_COUNT | CTRL_MASTER; /* CTRL1 contains IRQ config and status bits, preserve those. */ @@ -69,40 +79,23 @@ static void mxsfb_set_pixel_fmt(struct mxsfb_drm_private *mxsfb) break; } - writel(ctrl1, mxsfb->base + LCDC_CTRL1); - writel(ctrl, mxsfb->base + LCDC_CTRL); -} - -static void mxsfb_set_bus_fmt(struct mxsfb_drm_private *mxsfb) -{ - struct drm_device *drm = mxsfb->drm; - u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24; - u32 reg; - - reg = readl(mxsfb->base + LCDC_CTRL); - - if (mxsfb->connector->display_info.num_bus_formats) - bus_format = mxsfb->connector->display_info.bus_formats[0]; - - DRM_DEV_DEBUG_DRIVER(drm->dev, "Using bus_format: 0x%08X\n", -bus_format); - - reg &= ~CTRL_BUS_WIDTH_MASK; switch (bus_format) { case MEDIA_BUS_FMT_RGB565_1X16: - reg |= CTRL_BUS_WIDTH_16; + ctrl |= CTRL_BUS_WIDTH_16; break; case MEDIA_BUS_FMT_RGB666_1X18: - reg |= CTRL_BUS_WIDTH_18; + ctrl |= CTRL_BUS_WIDTH_18; break; case MEDIA_BUS_FMT_RGB888_1X24: - reg |= CTRL_BUS_WIDTH_24; + ctrl |= CTRL_BUS_WIDTH_24; break; default: dev_err(drm->dev, "Unknown media bus format %d\n", bus_format); break; } - writel(reg, mxsfb->base + LCDC_CTRL); + + writel(ctrl1, mxsfb->base + LCDC_CTRL1); + writel(ctrl, mxsfb->base + LCDC_CTRL); } static void mxsfb_enable_controller(struct mxsfb_drm_private *mxsfb) @@ -213,7 +206,7 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) /* Clear the FIFOs */ writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_SET); - mxsfb_set_pixel_fmt(mxsfb); + mxsfb_set_formats(mxsfb); clk_set_rate(mxsfb->clk, m->crtc_clock * 1000); @@ -255,8 +248,6 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) writel(vdctrl0, mxsfb->base + LCDC_VDCTRL0); - mxsfb_set_bus_fmt(mxsfb); - /* Frame length in lines. */ writel(m->crtc_vtotal, mxsfb->base + LCDC_VDCTRL1); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 06/21] drm: mxsfb: Pass mxsfb_drm_private pointer to mxsfb_reset_block()
The mxsfb_reset_block() function isn't special, pass it the mxsfb_drm_private pointer instead of a pointer to the base address. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index 8b6339316929..be60c4021e2f 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -166,21 +166,21 @@ static int clear_poll_bit(void __iomem *addr, u32 mask) return readl_poll_timeout(addr, reg, !(reg & mask), 0, RESET_TIMEOUT); } -static int mxsfb_reset_block(void __iomem *reset_addr) +static int mxsfb_reset_block(struct mxsfb_drm_private *mxsfb) { int ret; - ret = clear_poll_bit(reset_addr, MODULE_SFTRST); + ret = clear_poll_bit(mxsfb->base, MODULE_SFTRST); if (ret) return ret; - writel(MODULE_CLKGATE, reset_addr + MXS_CLR_ADDR); + writel(MODULE_CLKGATE, mxsfb->base + MXS_CLR_ADDR); - ret = clear_poll_bit(reset_addr, MODULE_SFTRST); + ret = clear_poll_bit(mxsfb->base, MODULE_SFTRST); if (ret) return ret; - return clear_poll_bit(reset_addr, MODULE_CLKGATE); + return clear_poll_bit(mxsfb->base, MODULE_CLKGATE); } static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb) @@ -213,7 +213,7 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) */ /* Mandatory eLCDIF reset as per the Reference Manual */ - err = mxsfb_reset_block(mxsfb->base); + err = mxsfb_reset_block(mxsfb); if (err) return; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 03/21] drm: mxsfb: Use BIT() macro to define register bitfields
Using BIT() is preferred over manual shifts as it's more readable, handles the 1 << 31 case properly, and avoids other mistakes as shown by the DEBUG0_HSYNC and DEBUG0_VSYNC bits (that are currently unused). Use it. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_regs.h | 56 +++--- 1 file changed, 28 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_regs.h b/drivers/gpu/drm/mxsfb/mxsfb_regs.h index 932d7ea08fd5..713d8f830135 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_regs.h +++ b/drivers/gpu/drm/mxsfb/mxsfb_regs.h @@ -28,51 +28,51 @@ #define LCDC_V4_DEBUG0 0x1d0 #define LCDC_V3_DEBUG0 0x1f0 -#define CTRL_SFTRST(1 << 31) -#define CTRL_CLKGATE (1 << 30) -#define CTRL_BYPASS_COUNT (1 << 19) -#define CTRL_VSYNC_MODE(1 << 18) -#define CTRL_DOTCLK_MODE (1 << 17) -#define CTRL_DATA_SELECT (1 << 16) +#define CTRL_SFTRSTBIT(31) +#define CTRL_CLKGATE BIT(30) +#define CTRL_BYPASS_COUNT BIT(19) +#define CTRL_VSYNC_MODEBIT(18) +#define CTRL_DOTCLK_MODE BIT(17) +#define CTRL_DATA_SELECT BIT(16) #define CTRL_SET_BUS_WIDTH(x) (((x) & 0x3) << 10) #define CTRL_GET_BUS_WIDTH(x) (((x) >> 10) & 0x3) #define CTRL_BUS_WIDTH_MASK(0x3 << 10) #define CTRL_SET_WORD_LENGTH(x)(((x) & 0x3) << 8) #define CTRL_GET_WORD_LENGTH(x)(((x) >> 8) & 0x3) -#define CTRL_MASTER(1 << 5) -#define CTRL_DF16 (1 << 3) -#define CTRL_DF18 (1 << 2) -#define CTRL_DF24 (1 << 1) -#define CTRL_RUN (1 << 0) +#define CTRL_MASTERBIT(5) +#define CTRL_DF16 BIT(3) +#define CTRL_DF18 BIT(2) +#define CTRL_DF24 BIT(1) +#define CTRL_RUN BIT(0) -#define CTRL1_FIFO_CLEAR (1 << 21) +#define CTRL1_FIFO_CLEAR BIT(21) #define CTRL1_SET_BYTE_PACKAGING(x)(((x) & 0xf) << 16) #define CTRL1_GET_BYTE_PACKAGING(x)(((x) >> 16) & 0xf) -#define CTRL1_CUR_FRAME_DONE_IRQ_EN(1 << 13) -#define CTRL1_CUR_FRAME_DONE_IRQ (1 << 9) +#define CTRL1_CUR_FRAME_DONE_IRQ_ENBIT(13) +#define CTRL1_CUR_FRAME_DONE_IRQ BIT(9) #define TRANSFER_COUNT_SET_VCOUNT(x) (((x) & 0x) << 16) #define TRANSFER_COUNT_GET_VCOUNT(x) (((x) >> 16) & 0x) #define TRANSFER_COUNT_SET_HCOUNT(x) ((x) & 0x) #define TRANSFER_COUNT_GET_HCOUNT(x) ((x) & 0x) -#define VDCTRL0_ENABLE_PRESENT (1 << 28) -#define VDCTRL0_VSYNC_ACT_HIGH (1 << 27) -#define VDCTRL0_HSYNC_ACT_HIGH (1 << 26) -#define VDCTRL0_DOTCLK_ACT_FALLING (1 << 25) -#define VDCTRL0_ENABLE_ACT_HIGH(1 << 24) -#define VDCTRL0_VSYNC_PERIOD_UNIT (1 << 21) -#define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT (1 << 20) -#define VDCTRL0_HALF_LINE (1 << 19) -#define VDCTRL0_HALF_LINE_MODE (1 << 18) +#define VDCTRL0_ENABLE_PRESENT BIT(28) +#define VDCTRL0_VSYNC_ACT_HIGH BIT(27) +#define VDCTRL0_HSYNC_ACT_HIGH BIT(26) +#define VDCTRL0_DOTCLK_ACT_FALLING BIT(25) +#define VDCTRL0_ENABLE_ACT_HIGHBIT(24) +#define VDCTRL0_VSYNC_PERIOD_UNIT BIT(21) +#define VDCTRL0_VSYNC_PULSE_WIDTH_UNIT BIT(20) +#define VDCTRL0_HALF_LINE BIT(19) +#define VDCTRL0_HALF_LINE_MODE BIT(18) #define VDCTRL0_SET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3) #define VDCTRL0_GET_VSYNC_PULSE_WIDTH(x) ((x) & 0x3) #define VDCTRL2_SET_HSYNC_PERIOD(x)((x) & 0x3) #define VDCTRL2_GET_HSYNC_PERIOD(x)((x) & 0x3) -#define VDCTRL3_MUX_SYNC_SIGNALS (1 << 29) -#define VDCTRL3_VSYNC_ONLY (1 << 28) +#define VDCTRL3_MUX_SYNC_SIGNALS BIT(29) +#define VDCTRL3_VSYNC_ONLY BIT(28) #define SET_HOR_WAIT_CNT(x)(((x) & 0xfff) << 16) #define GET_HOR_WAIT_CNT(x)(((x) >> 16) & 0xfff) #define SET_VERT_WAIT_CNT(x) ((x) & 0x) @@ -80,11 +80,11 @@ #define VDCTRL4_SET_DOTCLK_DLY(x) (((x) & 0x7) << 29) /* v4 only */ #define VDCTRL4_GET_DOTCLK_DLY(x) (((x) >> 29) & 0x7) /* v4 only */ -#define VDCTRL4_SYNC_SIGNALS_ON(1 << 18) +#define VDCTRL4_SYNC_SIGNALS_ONBIT(18) #define SET_DOTCLK_H_VALID_DATA_CNT(x) ((x) & 0x3) -#define DEBUG0_HSYNC (1 < 26) -#define DEBUG0_VSYNC (1 < 25) +#define DEBUG0_HSYNC BIT(26) +#define DEBUG0_VSYNC BIT(25) #define MXSFB_MIN_XRES 120 #define MXSFB_MIN_YRES 120 -- Regards, Laurent Pinchart ___ dri-devel mailing list
[PATCH 21/21] drm: mxsfb: Support the alpha plane
The LCDIF in the i.MX6SX and i.MX7 have a second plane called the alpha plane. Support it. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 + drivers/gpu/drm/mxsfb/mxsfb_drv.h | 16 ++-- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 129 + drivers/gpu/drm/mxsfb/mxsfb_regs.h | 22 + 4 files changed, 149 insertions(+), 21 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index ed8e3f7bc27c..ab3a212375f1 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -49,6 +49,7 @@ static const struct mxsfb_devdata mxsfb_devdata[] = { .next_buf = LCDC_V3_NEXT_BUF, .hs_wdth_mask = 0xff, .hs_wdth_shift = 24, + .has_overlay= false, }, [MXSFB_V4] = { .transfer_count = LCDC_V4_TRANSFER_COUNT, @@ -56,6 +57,7 @@ static const struct mxsfb_devdata mxsfb_devdata[] = { .next_buf = LCDC_V4_NEXT_BUF, .hs_wdth_mask = 0x3fff, .hs_wdth_shift = 18, + .has_overlay= false, }, [MXSFB_V6] = { .transfer_count = LCDC_V4_TRANSFER_COUNT, @@ -63,6 +65,7 @@ static const struct mxsfb_devdata mxsfb_devdata[] = { .next_buf = LCDC_V4_NEXT_BUF, .hs_wdth_mask = 0x3fff, .hs_wdth_shift = 18, + .has_overlay= true, }, }; diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.h b/drivers/gpu/drm/mxsfb/mxsfb_drv.h index 607a6a5e6be3..399d23e91ed1 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.h +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.h @@ -16,11 +16,12 @@ struct clk; struct mxsfb_devdata { - unsigned int transfer_count; - unsigned int cur_buf; - unsigned int next_buf; - unsigned int hs_wdth_mask; - unsigned int hs_wdth_shift; + unsigned inttransfer_count; + unsigned intcur_buf; + unsigned intnext_buf; + unsigned inths_wdth_mask; + unsigned inths_wdth_shift; + boolhas_overlay; }; struct mxsfb_drm_private { @@ -32,7 +33,10 @@ struct mxsfb_drm_private { struct clk *clk_disp_axi; struct drm_device *drm; - struct drm_planeplane; + struct { + struct drm_planeprimary; + struct drm_planeoverlay; + } planes; struct drm_crtc crtc; struct drm_encoder encoder; struct drm_connector*connector; diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index f81f8c222c13..c9c394f7cbe2 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -169,9 +169,9 @@ static int mxsfb_reset_block(struct mxsfb_drm_private *mxsfb) return clear_poll_bit(mxsfb->base + LCDC_CTRL, CTRL_CLKGATE); } -static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb) +static dma_addr_t mxsfb_get_fb_paddr(struct drm_plane *plane) { - struct drm_framebuffer *fb = mxsfb->plane.state->fb; + struct drm_framebuffer *fb = plane->state->fb; struct drm_gem_cma_object *gem; if (!fb) @@ -206,6 +206,9 @@ static void mxsfb_crtc_mode_set_nofb(struct mxsfb_drm_private *mxsfb) /* Clear the FIFOs */ writel(CTRL1_FIFO_CLEAR, mxsfb->base + LCDC_CTRL1 + REG_SET); + if (mxsfb->devdata->has_overlay) + writel(0, mxsfb->base + LCDC_AS_CTRL); + mxsfb_set_formats(mxsfb); clk_set_rate(mxsfb->clk, m->crtc_clock * 1000); @@ -313,7 +316,7 @@ static void mxsfb_crtc_atomic_enable(struct drm_crtc *crtc, mxsfb_crtc_mode_set_nofb(mxsfb); /* Write cur_buf as well to avoid an initial corrupt frame */ - paddr = mxsfb_get_fb_paddr(mxsfb); + paddr = mxsfb_get_fb_paddr(crtc->primary); if (paddr) { writel(paddr, mxsfb->base + mxsfb->devdata->cur_buf); writel(paddr, mxsfb->base + mxsfb->devdata->next_buf); @@ -410,20 +413,85 @@ static int mxsfb_plane_atomic_check(struct drm_plane *plane, false, true); } -static void mxsfb_plane_atomic_update(struct drm_plane *plane, - struct drm_plane_state *old_pstate) +static void mxsfb_plane_primary_atomic_update(struct drm_plane *plane, + struct drm_plane_state *old_pstate) { struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev); dma_addr_t paddr; - paddr = mxsfb_get_fb_paddr(mxsfb); + paddr = mxsfb_get_fb_paddr(plane); if (paddr) writel(paddr, mxsfb->base + mxsfb->devdata->next_buf); } -static const struct drm_plane_helper_funcs
[PATCH 08/21] drm: mxsfb: Remove register definitions from mxsfb_crtc.c
mxsfb_crtc.c defines several macros related to register addresses and bit, which duplicates macros from mxsfb_regs.h. Use the macros from mxsfb_regs.h instead and remove them. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_crtc.c | 14 +- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c index 722bd9b4f5f9..aef72adabf41 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_crtc.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_crtc.c @@ -29,10 +29,6 @@ #include "mxsfb_drv.h" #include "mxsfb_regs.h" -#define MXS_SET_ADDR 0x4 -#define MXS_CLR_ADDR 0x8 -#define MODULE_CLKGATE BIT(30) -#define MODULE_SFTRST BIT(31) /* 1 second delay should be plenty of time for block reset */ #define RESET_TIMEOUT 100 @@ -162,7 +158,7 @@ static int clear_poll_bit(void __iomem *addr, u32 mask) { u32 reg; - writel(mask, addr + MXS_CLR_ADDR); + writel(mask, addr + REG_CLR); return readl_poll_timeout(addr, reg, !(reg & mask), 0, RESET_TIMEOUT); } @@ -170,17 +166,17 @@ static int mxsfb_reset_block(struct mxsfb_drm_private *mxsfb) { int ret; - ret = clear_poll_bit(mxsfb->base + LCDC_CTRL, MODULE_SFTRST); + ret = clear_poll_bit(mxsfb->base + LCDC_CTRL, CTRL_SFTRST); if (ret) return ret; - writel(MODULE_CLKGATE, mxsfb->base + LCDC_CTRL + MXS_CLR_ADDR); + writel(CTRL_CLKGATE, mxsfb->base + LCDC_CTRL + REG_CLR); - ret = clear_poll_bit(mxsfb->base + LCDC_CTRL, MODULE_SFTRST); + ret = clear_poll_bit(mxsfb->base + LCDC_CTRL, CTRL_SFTRST); if (ret) return ret; - return clear_poll_bit(mxsfb->base + LCDC_CTRL, MODULE_CLKGATE); + return clear_poll_bit(mxsfb->base + LCDC_CTRL, CTRL_CLKGATE); } static dma_addr_t mxsfb_get_fb_paddr(struct mxsfb_drm_private *mxsfb) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 01/21] drm: mxsfb: Remove fbdev leftovers
Commit 8e93f1028d74 ("drm/mxsfb: Use drm_fbdev_generic_setup()") replaced fbdev handling with drm_fbdev_generic_setup() but left inclusion of the drm/drm_fb_cma_helper.h header. Remove it. Fixes: 8e93f1028d74 ("drm/mxsfb: Use drm_fbdev_generic_setup()") Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 497cf443a9af..2e6068d96034 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -24,7 +24,6 @@ #include #include #include -#include #include #include #include -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 17/21] drm: mxsfb: Update internal IP version number for i.MX6SX
The LCDIF present in the i.MX6SX has extra features compared to the i.MX28. It has however lost its IP version register, so no official version number is known. Bump the version to MXSFB_V6 following the i.MX version, in preparation for support for the additional features. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 7c9a041f5f6d..2316c12c5c42 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -35,6 +35,11 @@ enum mxsfb_devtype { MXSFB_V3, MXSFB_V4, + /* +* Starting at i.MX6 the hardware version register is gone, use the +* i.MX family number as the version. +*/ + MXSFB_V6, }; static const struct mxsfb_devdata mxsfb_devdata[] = { @@ -52,6 +57,13 @@ static const struct mxsfb_devdata mxsfb_devdata[] = { .hs_wdth_mask = 0x3fff, .hs_wdth_shift = 18, }, + [MXSFB_V6] = { + .transfer_count = LCDC_V4_TRANSFER_COUNT, + .cur_buf= LCDC_V4_CUR_BUF, + .next_buf = LCDC_V4_NEXT_BUF, + .hs_wdth_mask = 0x3fff, + .hs_wdth_shift = 18, + }, }; void mxsfb_enable_axi_clk(struct mxsfb_drm_private *mxsfb) @@ -279,7 +291,7 @@ static struct drm_driver mxsfb_driver = { static const struct platform_device_id mxsfb_devtype[] = { { .name = "imx23-fb", .driver_data = MXSFB_V3, }, { .name = "imx28-fb", .driver_data = MXSFB_V4, }, - { .name = "imx6sx-fb", .driver_data = MXSFB_V4, }, + { .name = "imx6sx-fb", .driver_data = MXSFB_V6, }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(platform, mxsfb_devtype); -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] dt-bindings: display: msm: gmu: move sram property to gpu bindings
On Mon, Mar 9, 2020 at 4:18 AM Brian Masney wrote: > > The sram property was incorrectly added to the GMU binding when it > really belongs with the GPU binding instead. Let's go ahead and > move it. > > While changes are being made here, let's update the sram property > description to mention that this property is only valid for a3xx and > a4xx GPUs. The a3xx/a4xx example in the GPU is replaced with what was > in the GMU. > > Signed-off-by: Brian Masney > Fixes: 198a72c8f9ee ("dt-bindings: display: msm: gmu: add optional ocmem > property") > --- > Background thread: > https://lore.kernel.org/lkml/20200303170159.ga13...@jcrouse1-lnx.qualcomm.com/ > > I started to look at what it would take to convert the GPU bindings to > YAML, however I am unsure of the complete list of "qcom,adreno-XYZ.W" > compatibles that are valid. heh, I'm not sure anyone is ;-) That said, adreno_device.c should give a complete list of XYZ (and *usually* the .W doesn't matter too much) BR, -R > > .../devicetree/bindings/display/msm/gmu.txt | 51 - > .../devicetree/bindings/display/msm/gpu.txt | 55 ++- > 2 files changed, 42 insertions(+), 64 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt > b/Documentation/devicetree/bindings/display/msm/gmu.txt > index bf9c7a2a495c..90af5b0a56a9 100644 > --- a/Documentation/devicetree/bindings/display/msm/gmu.txt > +++ b/Documentation/devicetree/bindings/display/msm/gmu.txt > @@ -31,10 +31,6 @@ Required properties: > - iommus: phandle to the adreno iommu > - operating-points-v2: phandle to the OPP operating points > > -Optional properties: > -- sram: phandle to the On Chip Memory (OCMEM) that's present on some > Snapdragon > -SoCs. See Documentation/devicetree/bindings/sram/qcom,ocmem.yaml. > - > Example: > > / { > @@ -67,50 +63,3 @@ Example: > operating-points-v2 = <_opp_table>; > }; > }; > - > -a3xx example with OCMEM support: > - > -/ { > - ... > - > - gpu: adreno@fdb0 { > - compatible = "qcom,adreno-330.2", > -"qcom,adreno"; > - reg = <0xfdb0 0x1>; > - reg-names = "kgsl_3d0_reg_memory"; > - interrupts = ; > - interrupt-names = "kgsl_3d0_irq"; > - clock-names = "core", > - "iface", > - "mem_iface"; > - clocks = < OXILI_GFX3D_CLK>, > -< OXILICX_AHB_CLK>, > -< OXILICX_AXI_CLK>; > - sram = <_sram>; > - power-domains = < OXILICX_GDSC>; > - operating-points-v2 = <_opp_table>; > - iommus = <_iommu 0>; > - }; > - > - ocmem@fdd0 { > - compatible = "qcom,msm8974-ocmem"; > - > - reg = <0xfdd0 0x2000>, > - <0xfec0 0x18>; > - reg-names = "ctrl", > -"mem"; > - > - clocks = < RPM_SMD_OCMEMGX_CLK>, > -< OCMEMCX_OCMEMNOC_CLK>; > - clock-names = "core", > - "iface"; > - > - #address-cells = <1>; > - #size-cells = <1>; > - > - gmu_sram: gmu-sram@0 { > - reg = <0x0 0x10>; > - ranges = <0 0 0xfec0 0x10>; > - }; > - }; > -}; > diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt > b/Documentation/devicetree/bindings/display/msm/gpu.txt > index 7edc298a15f2..fd779cd6994d 100644 > --- a/Documentation/devicetree/bindings/display/msm/gpu.txt > +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt > @@ -35,25 +35,54 @@ Required properties: >bring the GPU out of secure mode. > - firmware-name: optional property of the 'zap-shader' node, listing the >relative path of the device specific zap firmware. > +- sram: phandle to the On Chip Memory (OCMEM) that's present on some a3xx and > +a4xx Snapdragon SoCs. See > +Documentation/devicetree/bindings/sram/qcom,ocmem.yaml. > > -Example 3xx/4xx/a5xx: > +Example 3xx/4xx: > > / { > ... > > - gpu: qcom,kgsl-3d0@430 { > - compatible = "qcom,adreno-320.2", "qcom,adreno"; > - reg = <0x0430 0x2>; > + gpu: adreno@fdb0 { > + compatible = "qcom,adreno-330.2", > +"qcom,adreno"; > + reg = <0xfdb0 0x1>; > reg-names = "kgsl_3d0_reg_memory"; > - interrupts = ; > - clock-names = > - "core", > - "iface", > - "mem_iface"; > - clocks = > - < GFX3D_CLK>, > - < GFX3D_AHB_CLK>, > - < MMSS_IMEM_AHB_CLK>; > + interrupts = ; >
Re: [PATCH] drm: panel: Set connector type for OrtusTech COM43H4M85ULC panel
On Mon, Mar 09, 2020 at 09:01:27PM +0200, Laurent Pinchart wrote: > Hi Sam, > > On Mon, Mar 09, 2020 at 08:00:47PM +0100, Sam Ravnborg wrote: > > On Mon, Mar 09, 2020 at 08:42:10PM +0200, Laurent Pinchart wrote: > > > The OrtusTech COM43H4M85ULC is a DPI panel, set the connector type > > > accordingly. > > > > > > Signed-off-by: Laurent Pinchart > > > > Reviewed-by: Sam Ravnborg > > > > I assume you will apply to drm-misc-next - OK? > > I still haven't got around to using dim :-) I can manage - so the entry level is pretty low. My lame and simple workflow dim update-branches # save patch from mutt cat mbox | dim apply git rebase etc. dim checkpatch <= if I make changes while applying #build testing dim push And when I do my own stuff: dim update-branches git checkout -b sam-my-stuff hacking-hacking commit, commit git rebase --exec "dim add-missing-cc" HEAD~5 dim can do much more than that - but the above is the few dim commands I use. This help me to do things remotely correct. So maybe this is as good as any time to try out dim? Sam ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling
Hi Philipp, (CC'ing Boris) On Mon, Mar 09, 2020 at 11:50:59AM +0100, Philipp Zabel wrote: > On Thu, 2019-11-14 at 14:17 +0100, Marek Vasut wrote: > > The bus_flags and bus_format handling logic does not seem to cover > > all potential usecases. Specifically, this seems to fail with an > > "edt,etm0700g0edh6" display attached to an 24bit display interface, > > with interface-pix-fmt = "rgb24" set in DT. > > interface-pix-fmt is a legacy property that was never intended to be > used as an override for the panel bus format. The bus flags were > supposed to be set from the display-timings node, back when there was no > of-graph connected panel at all. > > That being said, there isn't really a proper alternative that allows to > override the bus format requested by the panel driver in the device tree > to account for weird wiring. We could reuse the bus-width endpoint > property documented in [1], but that wouldn't completely specify how the > RGB components are to be mapped onto the parallel bus. > > [1] Documentation/devicetree/bindings/media/video-interfaces.txt Things are funny sometimes, I've run into the exact same problem with a different display controller today. Shouldn't we use the data-shift property from [1] to specify this ? Combined with Boris' bus format negotiation for bridges, I think we would have all the components in place to solve this problem properly. Bonus points if we can get a helper function that CRTC code can call to get the bus format they should use without having to care about the details (and just to be clear, no yak shaving here, I'm not asking Marek to implement such a helper, it's not a blocking issue). > I do wonder whether for your case it would be better to implement a > MEDIA_BUS_FMT_RGB666_1X24_CPADLO though, to leave the LSBs untouched > instead of risking to dump them into accidental PCB antennae. That's a good point I haven't thought about, and I agree it makes sense. It means that display controller drivers will have to explicitly support MEDIA_BUS_FMT_RGB666_1X24_CPADLO and map it to MEDIA_BUS_FMT_RGB888_1X24 if the hardware doesn't support that feature, but I don't think that's a big issue. > > In this specific setup, the panel-simple.c driver entry for the display > > sets .bus_flags to non-zero value. However, as imxpd->bus_format is set > > from the DT property "interface-pix-fmt", imx_pd_encoder_atomic_check() > > will set imx_crtc_state->bus_flags = imxpd->bus_flags even though the > > imxpd->bus_flags is zero, while the di->bus_flags is correctly set by > > the panel-simple.c and non-zero. > > > > The result is incorrect flags being > > used for the display configuration and thus an image corruption. > > (Specifically, DRM_BUS_FLAG_PIXDATA_POSEDGE is not propagated and thus > > the ipuv3 clocks pixels on the wrong edge). > > > > This patch fixes the problem by overriding the imx_crtc_state->bus_format > > from the imxpd->bus_format only if the DT property "interface-pix-fmt" is > > present or if the DI provides no formats. Similarly for bus_flags, which > > are set from imxpd->bus_flags only if the DI provides no formats. > > > > Signed-off-by: Marek Vasut > > Cc: Daniel Vetter > > Cc: David Airlie > > Cc: Fabio Estevam > > Cc: NXP Linux Team > > Cc: Philipp Zabel > > Cc: Sascha Hauer > > Cc: Shawn Guo > > Cc: linux-arm-ker...@lists.infradead.org > > To: dri-devel@lists.freedesktop.org > > --- > > drivers/gpu/drm/imx/parallel-display.c | 13 - > > 1 file changed, 8 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/gpu/drm/imx/parallel-display.c > > b/drivers/gpu/drm/imx/parallel-display.c > > index 35518e5de356..92f00b12c068 100644 > > --- a/drivers/gpu/drm/imx/parallel-display.c > > +++ b/drivers/gpu/drm/imx/parallel-display.c > > @@ -113,13 +113,16 @@ static int imx_pd_encoder_atomic_check(struct > > drm_encoder *encoder, > > struct drm_display_info *di = _state->connector->display_info; > > struct imx_parallel_display *imxpd = enc_to_imxpd(encoder); > > > > - if (!imxpd->bus_format && di->num_bus_formats) { > > - imx_crtc_state->bus_flags = di->bus_flags; > > + if (imxpd->bus_format || !di->num_bus_formats) > > I see no reason to invert the logic here. Let's keep the common case > first. > > > + imx_crtc_state->bus_format = imxpd->bus_format; > > + else > > imx_crtc_state->bus_format = di->bus_formats[0]; > > - } else { > > + > > + if (di->num_bus_formats) > > + imx_crtc_state->bus_flags = di->bus_flags; > > + else > > imx_crtc_state->bus_flags = imxpd->bus_flags; > > - imx_crtc_state->bus_format = imxpd->bus_format; > > - } > > + > > imx_crtc_state->di_hsync_pin = 2; > > imx_crtc_state->di_vsync_pin = 3; > > So while I don't like this being used at all, I think the patch improves > consistency, as imx_pd_connector_get_modes doesn't allow to override the > panel's modes with DT display-timings either. -- Regards,
Re: [PATCH] drm: panel: Set connector type for OrtusTech COM43H4M85ULC panel
Hi Sam, On Mon, Mar 09, 2020 at 08:00:47PM +0100, Sam Ravnborg wrote: > On Mon, Mar 09, 2020 at 08:42:10PM +0200, Laurent Pinchart wrote: > > The OrtusTech COM43H4M85ULC is a DPI panel, set the connector type > > accordingly. > > > > Signed-off-by: Laurent Pinchart > > Reviewed-by: Sam Ravnborg > > I assume you will apply to drm-misc-next - OK? I still haven't got around to using dim :-) > > --- > > drivers/gpu/drm/panel/panel-simple.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > > b/drivers/gpu/drm/panel/panel-simple.c > > index e14c14ac62b5..145ee05dcacd 100644 > > --- a/drivers/gpu/drm/panel/panel-simple.c > > +++ b/drivers/gpu/drm/panel/panel-simple.c > > @@ -2439,6 +2439,7 @@ static const struct panel_desc > > ortustech_com43h4m85ulc = { > > }, > > .bus_format = MEDIA_BUS_FMT_RGB888_1X24, > > .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > > + .connector_type = DRM_MODE_CONNECTOR_DPI, > > }; > > > > static const struct drm_display_mode osddisplays_osd070t1718_19ts_mode = { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: panel: Set connector type for OrtusTech COM43H4M85ULC panel
Hi Laurent. On Mon, Mar 09, 2020 at 08:42:10PM +0200, Laurent Pinchart wrote: > The OrtusTech COM43H4M85ULC is a DPI panel, set the connector type > accordingly. > > Signed-off-by: Laurent Pinchart Reviewed-by: Sam Ravnborg I assume you will apply to drm-misc-next - OK? Sam > --- > drivers/gpu/drm/panel/panel-simple.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/panel/panel-simple.c > b/drivers/gpu/drm/panel/panel-simple.c > index e14c14ac62b5..145ee05dcacd 100644 > --- a/drivers/gpu/drm/panel/panel-simple.c > +++ b/drivers/gpu/drm/panel/panel-simple.c > @@ -2439,6 +2439,7 @@ static const struct panel_desc ortustech_com43h4m85ulc > = { > }, > .bus_format = MEDIA_BUS_FMT_RGB888_1X24, > .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, > + .connector_type = DRM_MODE_CONNECTOR_DPI, > }; > > static const struct drm_display_mode osddisplays_osd070t1718_19ts_mode = { > -- > Regards, > > Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm: bridge: add it66121 driver
Hi Phong. On Mon, Mar 09, 2020 at 04:36:54PM +0100, Phong LE wrote: > This commit is a simple driver for bridge HMDI it66121. > The input format is RBG and there is no color conversion. > Audio, HDCP and CEC are not supported yet. Nice driver. Some few comments below. Patch fails to apply/build on top of drm-misc-next. Please patch next version so it applies and works on top of drm-misc-next, as this is where this patch will go in. Who will maintain the driver? Maybe update MAINTAINERS? Sam > @@ -0,0 +1,983 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2020 BayLibre, SAS > + * Author: Phong LE > + * Copyright (C) 2018-2019, Artem Mygaiev > + * Copyright (C) 2017, Fresco Logic, Incorporated. > + * > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Sort alphabetically (almost done), and remove duplicate. > + > +static int ite66121_power_off(struct it66121_ctx *ctx) > +{ > + return regulator_bulk_disable(ARRAY_SIZE(ctx->supplies), ctx->supplies); > +} > + > +static inline int it66121_preamble_ddc(struct it66121_ctx *ctx) Let the compiler do the inline decision - no need to specify it here. > +{ > + return regmap_write(ctx->regmap, IT66121_MASTER_SEL_REG, > + IT66121_MASTER_SEL_HOST); > +} > + > +static inline int it66121_fire_afe(struct it66121_ctx *ctx) > +{ > + return regmap_write(ctx->regmap, IT66121_AFE_DRV_REG, 0); > +} > + > +static int it66121_configure_input(struct it66121_ctx *ctx) > +{ > + int ret; > + > + ret = regmap_write(ctx->regmap, IT66121_INPUT_MODE_REG, > +ctx->conf->input_mode_reg); > + if (ret) > + return ret; > + > + return regmap_write(ctx->regmap, IT66121_INPUT_CSC_REG, > + ctx->conf->input_conversion_reg); > +} > + Maybe a small comment: /* Configure analog front end */ So readers do not have to second guess the "afe" abbreavation. Or spell it out. > +static int it66121_configure_afe(struct it66121_ctx *ctx, > + const struct drm_display_mode *mode) > +{ > + int ret; > + > + ret = regmap_write(ctx->regmap, IT66121_AFE_DRV_REG, > +IT66121_AFE_DRV_RST); > + if (ret) > + return ret; > + > + if (mode->clock > IT66121_AFE_CLK_HIGH) { > + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_REG, > + IT66121_AFE_XP_GAINBIT | > + IT66121_AFE_XP_ENO, > + IT66121_AFE_XP_GAINBIT); > + if (ret) > + return ret; > + > + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG, > + IT66121_AFE_IP_GAINBIT | > + IT66121_AFE_IP_ER0 | > + IT66121_AFE_IP_EC1, > + IT66121_AFE_IP_GAINBIT); > + if (ret) > + return ret; > + > + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG, > + IT66121_AFE_XP_EC1_LOWCLK, 0x80); > + if (ret) > + return ret; > + } else { > + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_REG, > + IT66121_AFE_XP_GAINBIT | > + IT66121_AFE_XP_ENO, > + IT66121_AFE_XP_ENO); > + if (ret) > + return ret; > + > + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_IP_REG, > + IT66121_AFE_IP_GAINBIT | > + IT66121_AFE_IP_ER0 | > + IT66121_AFE_IP_EC1, IT66121_AFE_IP_ER0 | > + IT66121_AFE_IP_EC1); > + if (ret) > + return ret; > + > + ret = regmap_write_bits(ctx->regmap, IT66121_AFE_XP_EC1_REG, > + IT66121_AFE_XP_EC1_LOWCLK, > + IT66121_AFE_XP_EC1_LOWCLK); > + if (ret) > + return ret; > + } > + > + /* Clear reset flags */ > + ret = regmap_write_bits(ctx->regmap, IT66121_SW_RST_REG, > + IT66121_SW_RST_REF | IT66121_SW_RST_VID, > + ~(IT66121_SW_RST_REF | IT66121_SW_RST_VID) & > + 0xFF); > + if (ret) > + return ret; > + > + return it66121_fire_afe(ctx); > +} > + > +static inline int it66121_wait_ddc_ready(struct it66121_ctx *ctx) Drop inline > +{ > + int ret, val; > + > + ret = regmap_read_poll_timeout(ctx->regmap, IT66121_DDC_STATUS_REG, > +
[PATCH] drm: bridge: tfp410: Check device ID for I2C-connected TFP410
The TFP410 supports configuration through I2C (in which case the corresponding DT node is a child of an I2C controller) or through pins (in which case the DT node creates a platform device). When I2C access to the device is available, read and validate the device ID at probe time to ensure that the device is present. While at it, sort headers alphabetically. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/bridge/ti-tfp410.c | 133 + 1 file changed, 115 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c index f195a4732e0b..1404d1f0b1d2 100644 --- a/drivers/gpu/drm/bridge/ti-tfp410.c +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -32,6 +33,7 @@ struct tfp410 { int hpd_irq; struct delayed_work hpd_work; struct gpio_desc*powerdown; + struct regmap *regmap; struct drm_bridge_timings timings; @@ -201,7 +203,7 @@ static const struct drm_bridge_timings tfp410_default_timings = { .hold_time_ps = 1300, }; -static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) +static int tfp410_parse_timings(struct tfp410 *dvi) { struct drm_bridge_timings *timings = >timings; struct device_node *ep; @@ -212,7 +214,7 @@ static int tfp410_parse_timings(struct tfp410 *dvi, bool i2c) /* Start with defaults. */ *timings = tfp410_default_timings; - if (i2c) + if (dvi->regmap) /* * In I2C mode timings are configured through the I2C interface. * As the driver doesn't support I2C configuration yet, we just @@ -314,9 +316,9 @@ static int tfp410_get_connector_properties(struct tfp410 *dvi) return ret; } -static int tfp410_init(struct device *dev, bool i2c) +static int tfp410_init(struct tfp410 *dvi) { - struct tfp410 *dvi; + struct device *dev = dvi->dev; int ret; if (!dev->of_node) { @@ -324,17 +326,13 @@ static int tfp410_init(struct device *dev, bool i2c) return -ENXIO; } - dvi = devm_kzalloc(dev, sizeof(*dvi), GFP_KERNEL); - if (!dvi) - return -ENOMEM; dev_set_drvdata(dev, dvi); dvi->bridge.funcs = _bridge_funcs; dvi->bridge.of_node = dev->of_node; dvi->bridge.timings = >timings; - dvi->dev = dev; - ret = tfp410_parse_timings(dvi, i2c); + ret = tfp410_parse_timings(dvi); if (ret) goto fail; @@ -396,7 +394,15 @@ static int tfp410_fini(struct device *dev) static int tfp410_probe(struct platform_device *pdev) { - return tfp410_init(>dev, false); + struct tfp410 *dvi; + + dvi = devm_kzalloc(>dev, sizeof(*dvi), GFP_KERNEL); + if (!dvi) + return -ENOMEM; + + dvi->dev = >dev; + + return tfp410_init(dvi); } static int tfp410_remove(struct platform_device *pdev) @@ -420,20 +426,111 @@ static struct platform_driver tfp410_platform_driver = { }; #if IS_ENABLED(CONFIG_I2C) -/* There is currently no i2c functionality. */ + +#define TFP410_VEN_ID_LO 0x00 +#define TFP410_VEN_ID_HI 0x01 +#define TFP410_DEV_ID_LO 0x02 +#define TFP410_DEV_ID_HI 0x03 +#define TFP410_REV_ID 0x04 +#define TFP410_CTL_1_MODE 0x08 +#define TFP410_CTL_2_MODE 0x09 +#define TFP410_CTL_3_MODE 0x0a +#define TFP410_CFG 0x0b +#define TFP410_DE_DLY 0x32 +#define TFP410_DE_CTL 0x33 +#define TFP410_DE_TOP 0x34 +#define TFP410_DE_CNT_LO 0x36 +#define TFP410_DE_CNT_HI 0x37 +#define TFP410_DE_LIN_LO 0x38 +#define TFP410_DE_LIN_HI 0x39 +#define TFP410_H_RES_LO0x3a +#define TFP410_H_RES_HI0x3b +#define TFP410_V_RES_LO0x3c +#define TFP410_V_RES_HI0x3d + +static const struct regmap_config tfp410_regmap_config = { + .reg_bits = 8, + .val_bits = 8, + + .max_register = 0x3d, + .wr_table = &(const struct regmap_access_table) { + .yes_ranges = (const struct regmap_range[]) { + { + .range_min = TFP410_CTL_1_MODE, + .range_max = TFP410_DE_LIN_HI, + }, + }, + .n_yes_ranges = 1, + }, +}; + +static int tfp410_check_version(struct tfp410 *dvi) +{ + unsigned int value; + u16 vendor_id; + u16 device_id; + u8 revision_id; + int ret; + + ret = regmap_read(dvi->regmap, TFP410_VEN_ID_LO, ); + if (ret < 0) + return ret; + vendor_id =
[PATCH] drm: panel: Set connector type for OrtusTech COM43H4M85ULC panel
The OrtusTech COM43H4M85ULC is a DPI panel, set the connector type accordingly. Signed-off-by: Laurent Pinchart --- drivers/gpu/drm/panel/panel-simple.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/panel/panel-simple.c b/drivers/gpu/drm/panel/panel-simple.c index e14c14ac62b5..145ee05dcacd 100644 --- a/drivers/gpu/drm/panel/panel-simple.c +++ b/drivers/gpu/drm/panel/panel-simple.c @@ -2439,6 +2439,7 @@ static const struct panel_desc ortustech_com43h4m85ulc = { }, .bus_format = MEDIA_BUS_FMT_RGB888_1X24, .bus_flags = DRM_BUS_FLAG_DE_HIGH | DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE, + .connector_type = DRM_MODE_CONNECTOR_DPI, }; static const struct drm_display_mode osddisplays_osd070t1718_19ts_mode = { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: rework SET_MASTER and DROP_MASTER perm handling
On Mon, 9 Mar 2020 at 13:13, Emil Velikov wrote: > > OTOH, if applications exist that rely on drop-master failing in this > > specific case, making drop-master succeed would break them. That might > > include a buggy set-master path that was written, but does not actually > > work because it was never tested since drop-master never worked. > > > > So I do not fully buy this argument yet, but I also cannot name any > > explicit examples that would break. > > > > > I've ventured for a while in the X (Xorg + drivers), Weston, > sway/wlroots and Mesa's codebase. > There were zero instances of such misuse. If other projects come to > mind - I'll gladly take a look. > Just checked a few other projects with git pickaxe* - zero cases of mentioned (mis)use. In particular: - qtbase + qtwayland + gtk Never used the wrappers or ioctls - kwin + plasmashell Never used the wrappers or ioctls - mutter + gnome-shell Briefly used the wrappers. Sane codepath - igt-gpu-tools ... just because I had it open Sane use both wrappers and ioctls. Any other projects I should check? -Emil * Both via libdrm and directly calling the ioctl git log -p -S DROP_MASTER git log -p -S drmDropMaster ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v1] dt-bindings: msm: disp: add yaml schemas for DPU and DSI bindings
On Fri, 6 Mar 2020 17:06:00 +0530, Krishna Manikandan wrote: > MSM Mobile Display Subsytem(MDSS) encapsulates sub-blocks > like DPU display controller, DSI etc. Add YAML schema > for the device tree bindings for the same. > > Signed-off-by: Krishna Manikandan > --- > .../bindings/display/msm/dpu-sc7180.yaml | 269 +++ > .../bindings/display/msm/dpu-sdm845.yaml | 265 +++ > .../bindings/display/msm/dsi-sc7180.yaml | 369 > + > .../bindings/display/msm/dsi-sdm845.yaml | 369 > + > 4 files changed, 1272 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-sc7180.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/dpu-sdm845.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/dsi-sc7180.yaml > create mode 100644 > Documentation/devicetree/bindings/display/msm/dsi-sdm845.yaml > My bot found errors running 'make dt_binding_check' on your patch: Documentation/devicetree/bindings/display/simple-framebuffer.example.dts:21.16-37.11: Warning (chosen_node_is_root): /example-0/chosen: chosen node must be at root node Documentation/devicetree/bindings/display/msm/dpu-sc7180.example.dts:17:10: fatal error: dt-bindings/clock/qcom,dispcc-sc7180.h: No such file or directory #include ^~~~ compilation terminated. scripts/Makefile.lib:300: recipe for target 'Documentation/devicetree/bindings/display/msm/dpu-sc7180.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/display/msm/dpu-sc7180.example.dt.yaml] Error 1 Makefile:1263: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1250230 Please check and re-submit. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/3] dt-bindings: display: bridge: add it66121 bindings
Hi Phong. On Mon, Mar 09, 2020 at 04:36:53PM +0100, Phong LE wrote: > Add the ITE bridge HDMI it66121 bindings. Good to see that you used DT Schema. > > Signed-off-by: Phong LE > --- > .../bindings/display/bridge/ite,it66121.yaml | 95 +++ > 1 file changed, 95 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml > b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml > new file mode 100644 > index ..f546c0b5a465 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/ite,it66121.yaml > @@ -0,0 +1,95 @@ > +# SPDX-License-Identifier: GPL-2.0 For new schemas please use: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/ite,it66121.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: ITE it66121 HDMI bridge Device Tree Bindings > + > +maintainers: > + - Phong LE > + > +description: | > + The IT66121 is a high-performance and low-power single channel HDMI > + transmitter, fully compliant with HDMI 1.3a, HDCP 1.2 and backward > compatible > + to DVI 1.0 specifications. > + > +properties: > + compatible: > +const: ite,it66121 > + > + reg: > +maxItems: 1 > +description: base I2C address of the device > + > + reset-gpios: > +maxItems: 1 > +description: GPIO connected to active low reset > + > + vrf12-supply: > +maxItems: 1 > +description: Regulator for 1.2V analog core power. > + > + vcn33-supply: > +maxItems: 1 > +description: Regulator for 3.3V digital core power. > + > + vcn18-supply: > +maxItems: 1 > +description: Regulator for 1.8V IO core power. > + > + interrupts: > +maxItems: 1 > + > + pclk-dual-edge: > +maxItems: 1 > +description: enable pclk dual edge mode. > + > + port: > +type: object > + > +properties: > + endpoint: > +type: object > +description: | > + Input endpoints of the bridge. > + > +required: > + - endpoint > + Are we missing an additionalProperties: false? So we do not have other properties than the ones listed here. Sam > +required: > + - compatible > + - reg > + - reset-gpios > + - vrf12-supply > + - vcn33-supply > + - vcn18-supply > + - interrupts > + - port > + > +examples: > + - | > +i2c6 { > + #address-cells = <1>; > + #size-cells = <0>; > + > + it66121hdmitx: it66121hdmitx@4c { > +compatible = "ite,it66121"; > +pinctrl-names = "default"; > +pinctrl-0 = <_pins_default>; > +vcn33-supply = <_vcn33_wifi_reg>; > +vcn18-supply = <_vcn18_reg>; > +vrf12-supply = <_vrf12_reg>; > +reset-gpios = < 160 1 /* GPIO_ACTIVE_LOW */>; > +interrupt-parent = <>; > +interrupts = <4 8 /* IRQ_TYPE_LEVEL_LOW */>; > +reg = <0x4c>; > +pclk-dual-edge; > + > +port { > + it66121_in: endpoint { > +remote-endpoint = <_out>; > + }; > +}; > + }; > +}; > -- > 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] dt-bindings: add ITE vendor
Hi Phong. On Mon, Mar 09, 2020 at 04:36:52PM +0100, Phong LE wrote: > Add ITE Tech Inc. prefix "ite" in vendor-prefixes Maybe add to the changelog that their domain is http://www.ite.com.tw/? > > Signed-off-by: Phong LE Acked-by: Sam Ravnborg > --- > Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml > b/Documentation/devicetree/bindings/vendor-prefixes.yaml > index 9e67944bec9c..16d4c776fdc7 100644 > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml > @@ -469,6 +469,8 @@ patternProperties: > description: Intersil >"^issi,.*": > description: Integrated Silicon Solutions Inc. > + "^ite,.*": > +description: ITE Tech Inc. >"^itead,.*": > description: ITEAD Intelligent Systems Co.Ltd >"^iwave,.*": > -- > 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 2/2] drm/panel: add support for rm69299 visionox panel driver
Hi, On Mon, Mar 09, 2020 at 10:53:04AM +0530, Harigovindan P wrote: > Add support for Visionox panel driver. > > Signed-off-by: Harigovindan P > --- > > Changes in v2: > - Dropping redundant space in Kconfig(Sam Ravnborg). > - Changing structure for include files(Sam Ravnborg). > - Removing backlight related code and functions(Sam Ravnborg). > - Removing repeated printing of error message(Sam Ravnborg). > - Adding drm_connector as an argument for get_modes function. > Changes in v3: > - Adding arguments for drm_panel_init to support against mainline. > Changes in v4: > - Removing error messages from regulator_set_load. > - Removing dev struct entry. > - Removing checks. > - Dropping empty comment lines. > Changes in v5: > - Removing unused struct member variables. > - Removing blank lines. > - Fixed indentation. > - Invoking dsi_detach and panel_remove while early exiting from probe. > > drivers/gpu/drm/panel/Kconfig | 8 + > drivers/gpu/drm/panel/Makefile| 1 + > .../gpu/drm/panel/panel-visionox-rm69299.c| 315 ++ > 3 files changed, 324 insertions(+) > create mode 100644 drivers/gpu/drm/panel/panel-visionox-rm69299.c > > ... > > diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c > b/drivers/gpu/drm/panel/panel-visionox-rm69299.c > new file mode 100644 > index ..2bd3af46d933 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c > > ... > > +static int visionox_35597_power_on(struct visionox_rm69299 *ctx) > +{ s/35597/rm69299/ ? > +static const struct rm69299_config rm69299_dir = { > + .width_mm = 74, > + .height_mm = 131, > + .dm = _rm69299_1080x2248_60hz, > +}; Are there actually variants of the panel with different sizes? So far the driver supports a single type of panel, so I would say struct rm69299_config is not needed. It can be added later if the driver ever gets support for other panel variants. For now just assign the values directly in 'visionox_rm69299_get_modes'. > +static int visionox_rm69299_probe(struct mipi_dsi_device *dsi) > +{ > + struct device *dev = >dev; > + struct visionox_rm69299 *ctx; > + int ret; > + > + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL); > + if (!ctx) > + return -ENOMEM; > + > + mipi_dsi_set_drvdata(dsi, ctx); > + > + ctx->supplies[0].supply = "vdda"; > + ctx->supplies[1].supply = "vdd3p3"; > + > + ret = devm_regulator_bulk_get(ctx->panel.dev, ARRAY_SIZE(ctx->supplies), > + ctx->supplies); nit: alignment is odd, either align with a tab after 'devm_regulator_bulk_get' or with 'ctx->panel.dev'. > + if (ret < 0) > + return ret; > + > + ctx->reset_gpio = devm_gpiod_get(ctx->panel.dev, "reset", > GPIOD_OUT_LOW); > + if (IS_ERR(ctx->reset_gpio)) { > + DRM_DEV_ERROR(dev, "cannot get reset gpio %ld\n", > + PTR_ERR(ctx->reset_gpio)); > + return PTR_ERR(ctx->reset_gpio); > + } > + > + drm_panel_init(>panel, dev, _rm69299_drm_funcs, > +DRM_MODE_CONNECTOR_DSI); > + ctx->panel.dev = dev; > + ctx->panel.funcs = _rm69299_drm_funcs; > + drm_panel_add(>panel); > + > + dsi->lanes = 4; > + dsi->format = MIPI_DSI_FMT_RGB888; > + dsi->mode_flags = MIPI_DSI_MODE_VIDEO | MIPI_DSI_MODE_LPM | > + MIPI_DSI_CLOCK_NON_CONTINUOUS; > + ret = mipi_dsi_attach(dsi); > + if (ret < 0) { > + DRM_DEV_ERROR(dev, "dsi attach failed ret = %d\n", ret); > + goto err_dsi_attach; > + } > + > + ret = regulator_set_load(ctx->supplies[0].consumer, 32000); > + if (ret) { > + mipi_dsi_detach(dsi); > + drm_panel_remove(>panel); that's technically correct, but since you have the 'goto' above and do the same unwinding for the other 'regulator_set_load' call below it would be better to use a 'goto' here (and below) too. Actually the 'goto' above only makes sense if 'goto' is also used for the other cases. > + return ret; > + } > + > + ret = regulator_set_load(ctx->supplies[1].consumer, 13200); > + if (ret) { > + mipi_dsi_detach(dsi); > + drm_panel_remove(>panel); > + return ret; > + } > + > + return 0; > + > +err_dsi_attach: > + drm_panel_remove(>panel); > + return ret; > +} > + > +static int visionox_rm69299_remove(struct mipi_dsi_device *dsi) > +{ > + struct visionox_rm69299 *ctx = mipi_dsi_get_drvdata(dsi); > + > + mipi_dsi_detach(ctx->dsi); > + mipi_dsi_device_unregister(ctx->dsi); > + > + drm_panel_remove(>panel); > + return 0; > +} > + > +static const struct of_device_id visionox_rm69299_of_match[] = { > + { > + .compatible = "visionox,rm69299-1080p-display", > + .data = _dir, > +
Re: [PATCH] RFC: dma-buf: Add an API for importing and exporting sync files
Am 05.03.20 um 16:54 schrieb Jason Ekstrand: On Thu, Mar 5, 2020 at 7:06 AM Christian König wrote: [SNIP] Well as far as I can see this won't work because it would break the semantics of the timeline sync. I'm not 100% convinced it has to. We already have support for the seqno regressing and we ensure that we still wait for all the fences. I thought maybe we could use that but I haven't spent enough time looking at the details to be sure. I may be missing something. That won't work. The seqno regression works by punishing userspace for doing something stupid and undefined. Be we can't do that under normal circumstances. I can prototype that if you want, shouldn't be more than a few hours of hacking anyway. If you'd like to, go for it. I'd be happy to give it a go as well but if you already know what you want, it may be easier for you to just write the patch for the cursor. Send you two patches for that a few minutes ago. But keep in mind that those are completely untested. Two more questions: 1. Do you want this collapsing to happen every time we create a dma_fence_array or should it be a special entrypoint? Collapsing all the time likely means doing extra array calculations instead of the dma_fence_array taking ownership of the array that's passed in. My gut says that cost is ok; but my gut doesn't spend much time in kernel space. In my prototype implementation that is a dma_resv function you call and get either a single fence or a dma_fence_array with the collapsed fences in return. But I wouldn't add that to the general dma_fence_array_init function since this is still a rather special case. Well see the patches, they should be pretty self explaining. 2. When we do the collapsing, should we call dma_fence_is_signaled() to avoid adding signaled fences to the array? It seems like avoiding adding references to fences that are already signaled would let the kernel clean them up faster and reduce the likelihood that a fence will hang around forever because it keeps getting added to arrays with other unsignaled fences. I think so. Can't think of a good reason why we would want to add already signaled fences to the array. Christian. --Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 04/33] drm/panel-ilitek-ili9322: Fix dotclocks
On Mon, Mar 9, 2020 at 2:38 PM Ville Syrjala wrote: > From: Ville Syrjälä > > The listed dotclocks are two orders of mangnitude out. > Fix them. > > v2: Just divide everything by 100 (Linus) > > Cc: Linus Walleij > Cc: Thierry Reding > Signed-off-by: Ville Syrjälä Reviewed-by: Linus Walleij Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 01/33] drm/panel-novatek-nt35510: Fix dotclock
On Mon, Mar 9, 2020 at 2:36 PM Ville Syrjala wrote: > From: Ville Syrjälä > > The dotclock is three orders of magnitude out. Fix it. > > v2: Just set it to 20MHz (Linus) > > Cc: Linus Walleij > Cc: Sam Ravnborg > Signed-off-by: Ville Syrjälä Reviewed-by: Linus Walleij Yours, Linus Walleij ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 2/2] drm/msm/a6xx: Use the DMA API for GMU memory objects
The GMU has very few memory allocations and uses a flat memory space so there is no good reason to go out of our way to bypass the DMA APIs which were basically designed for this exact scenario. v4: Use dma_alloc_wc() v3: Set the dma mask correctly and use dma_addr_t for the iova type v2: Pass force_dma false to of_dma_configure to require that the DMA region be set up and return error from of_dma_configure to fail probe. Reviewed-by: Michael J. Ruhl Signed-off-by: Jordan Crouse --- drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 113 -- drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 6 +- 2 files changed, 12 insertions(+), 107 deletions(-) diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c index 748cd37..dd51dd0 100644 --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c @@ -2,6 +2,7 @@ /* Copyright (c) 2017-2019 The Linux Foundation. All rights reserved. */ #include +#include #include #include #include @@ -920,21 +921,10 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu) static void a6xx_gmu_memory_free(struct a6xx_gmu *gmu, struct a6xx_gmu_bo *bo) { - int count, i; - u64 iova; - if (IS_ERR_OR_NULL(bo)) return; - count = bo->size >> PAGE_SHIFT; - iova = bo->iova; - - for (i = 0; i < count; i++, iova += PAGE_SIZE) { - iommu_unmap(gmu->domain, iova, PAGE_SIZE); - __free_pages(bo->pages[i], 0); - } - - kfree(bo->pages); + dma_free_wc(gmu->dev, bo->size, bo->virt, bo->iova); kfree(bo); } @@ -942,7 +932,6 @@ static struct a6xx_gmu_bo *a6xx_gmu_memory_alloc(struct a6xx_gmu *gmu, size_t size) { struct a6xx_gmu_bo *bo; - int ret, count, i; bo = kzalloc(sizeof(*bo), GFP_KERNEL); if (!bo) @@ -950,86 +939,14 @@ static struct a6xx_gmu_bo *a6xx_gmu_memory_alloc(struct a6xx_gmu *gmu, bo->size = PAGE_ALIGN(size); - count = bo->size >> PAGE_SHIFT; + bo->virt = dma_alloc_wc(gmu->dev, bo->size, >iova, GFP_KERNEL); - bo->pages = kcalloc(count, sizeof(struct page *), GFP_KERNEL); - if (!bo->pages) { + if (!bo->virt) { kfree(bo); return ERR_PTR(-ENOMEM); } - for (i = 0; i < count; i++) { - bo->pages[i] = alloc_page(GFP_KERNEL); - if (!bo->pages[i]) - goto err; - } - - bo->iova = gmu->uncached_iova_base; - - for (i = 0; i < count; i++) { - ret = iommu_map(gmu->domain, - bo->iova + (PAGE_SIZE * i), - page_to_phys(bo->pages[i]), PAGE_SIZE, - IOMMU_READ | IOMMU_WRITE); - - if (ret) { - DRM_DEV_ERROR(gmu->dev, "Unable to map GMU buffer object\n"); - - for (i = i - 1 ; i >= 0; i--) - iommu_unmap(gmu->domain, - bo->iova + (PAGE_SIZE * i), - PAGE_SIZE); - - goto err; - } - } - - bo->virt = vmap(bo->pages, count, VM_IOREMAP, - pgprot_writecombine(PAGE_KERNEL)); - if (!bo->virt) - goto err; - - /* Align future IOVA addresses on 1MB boundaries */ - gmu->uncached_iova_base += ALIGN(size, SZ_1M); - return bo; - -err: - for (i = 0; i < count; i++) { - if (bo->pages[i]) - __free_pages(bo->pages[i], 0); - } - - kfree(bo->pages); - kfree(bo); - - return ERR_PTR(-ENOMEM); -} - -static int a6xx_gmu_memory_probe(struct a6xx_gmu *gmu) -{ - int ret; - - /* -* The GMU address space is hardcoded to treat the range -* 0x6000 - 0x8000 as un-cached memory. All buffers shared -* between the GMU and the CPU will live in this space -*/ - gmu->uncached_iova_base = 0x6000; - - - gmu->domain = iommu_domain_alloc(_bus_type); - if (!gmu->domain) - return -ENODEV; - - ret = iommu_attach_device(gmu->domain, gmu->dev); - - if (ret) { - iommu_domain_free(gmu->domain); - gmu->domain = NULL; - } - - return ret; } /* Return the 'arc-level' for the given frequency */ @@ -1289,10 +1206,6 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu) a6xx_gmu_memory_free(gmu, gmu->hfi); - iommu_detach_device(gmu->domain, gmu->dev); - - iommu_domain_free(gmu->domain); - free_irq(gmu->gmu_irq, gmu); free_irq(gmu->hfi_irq, gmu); @@ -1313,7 +1226,13 @@ int a6xx_gmu_init(struct a6xx_gpu *a6xx_gpu, struct device_node *node) gmu->dev = >dev; - of_dma_configure(gmu->dev, node, true); + /* Pass force_dma false to require the DT to set the dma
[PATCH v5 1/2] dt-bindings: display: msm: Convert GMU bindings to YAML
Convert display/msm/gmu.txt to display/msm/gmu.yaml and remove the old text bindings. Acked-by: Sam Ravnborg Reviewed-by: Rob Herring Signed-off-by: Jordan Crouse --- .../devicetree/bindings/display/msm/gmu.txt| 65 --- .../devicetree/bindings/display/msm/gmu.yaml | 123 + 2 files changed, 123 insertions(+), 65 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.yaml diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt b/Documentation/devicetree/bindings/display/msm/gmu.txt deleted file mode 100644 index 90af5b0..000 --- a/Documentation/devicetree/bindings/display/msm/gmu.txt +++ /dev/null @@ -1,65 +0,0 @@ -Qualcomm adreno/snapdragon GMU (Graphics management unit) - -The GMU is a programmable power controller for the GPU. the CPU controls the -GMU which in turn handles power controls for the GPU. - -Required properties: -- compatible: "qcom,adreno-gmu-XYZ.W", "qcom,adreno-gmu" -for example: "qcom,adreno-gmu-630.2", "qcom,adreno-gmu" - Note that you need to list the less specific "qcom,adreno-gmu" - for generic matches and the more specific identifier to identify - the specific device. -- reg: Physical base address and length of the GMU registers. -- reg-names: Matching names for the register regions - * "gmu" - * "gmu_pdc" - * "gmu_pdc_seg" -- interrupts: The interrupt signals from the GMU. -- interrupt-names: Matching names for the interrupts - * "hfi" - * "gmu" -- clocks: phandles to the device clocks -- clock-names: Matching names for the clocks - * "gmu" - * "cxo" - * "axi" - * "mnoc" -- power-domains: should be: - <_gpucc GPU_CX_GDSC> - <_gpucc GPU_GX_GDSC> -- power-domain-names: Matching names for the power domains -- iommus: phandle to the adreno iommu -- operating-points-v2: phandle to the OPP operating points - -Example: - -/ { - ... - - gmu: gmu@506a000 { - compatible="qcom,adreno-gmu-630.2", "qcom,adreno-gmu"; - - reg = <0x506a000 0x3>, - <0xb28 0x1>, - <0xb48 0x1>; - reg-names = "gmu", "gmu_pdc", "gmu_pdc_seq"; - - interrupts = , -; - interrupt-names = "hfi", "gmu"; - - clocks = < GPU_CC_CX_GMU_CLK>, - < GPU_CC_CXO_CLK>, - < GCC_DDRSS_GPU_AXI_CLK>, - < GCC_GPU_MEMNOC_GFX_CLK>; - clock-names = "gmu", "cxo", "axi", "memnoc"; - - power-domains = < GPU_CX_GDSC>, - < GPU_GX_GDSC>; - power-domain-names = "cx", "gx"; - - iommus = <_smmu 5>; - - operating-points-v2 = <_opp_table>; - }; -}; diff --git a/Documentation/devicetree/bindings/display/msm/gmu.yaml b/Documentation/devicetree/bindings/display/msm/gmu.yaml new file mode 100644 index 000..0b8736a --- /dev/null +++ b/Documentation/devicetree/bindings/display/msm/gmu.yaml @@ -0,0 +1,123 @@ +# SPDX-License-Identifier: GPL-2.0-only +# Copyright 2019-2020, The Linux Foundation, All Rights Reserved +%YAML 1.2 +--- + +$id: "http://devicetree.org/schemas/display/msm/gmu.yaml#; +$schema: "http://devicetree.org/meta-schemas/core.yaml#; + +title: Devicetree bindings for the GMU attached to certain Adreno GPUs + +maintainers: + - Rob Clark + +description: | + These bindings describe the Graphics Management Unit (GMU) that is attached + to members of the Adreno A6xx GPU family. The GMU provides on-device power + management and support to improve power efficiency and reduce the load on + the CPU. + +properties: + compatible: +items: + - enum: + - qcom,adreno-gmu-630.2 + - const: qcom,adreno-gmu + + reg: +items: + - description: Core GMU registers + - description: GMU PDC registers + - description: GMU PDC sequence registers + + reg-names: +items: + - const: gmu + - const: gmu_pdc + - const: gmu_pdc_seq + + clocks: +items: + - description: GMU clock + - description: GPU CX clock + - description: GPU AXI clock + - description: GPU MEMNOC clock + + clock-names: +items: + - const: gmu + - const: cxo + - const: axi + - const: memnoc + + interrupts: +items: + - description: GMU HFI interrupt + - description: GMU interrupt + + + interrupt-names: +items: + - const: hfi + - const: gmu + + power-domains: + items: + - description: CX power domain + - description: GX power domain + + power-domain-names: + items: + - const: cx + - const: gx + + iommus: +maxItems: 1 + + operating-points-v2: true + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - interrupts + - interrupt-names + - power-domains + -
[PATCH v5 0/2] msm/gpu/a6xx: use the DMA-API for GMU memory allocations
When CONFIG_INIT_ON_ALLOC_DEFAULT_ON the GMU memory allocator runs afoul of cache coherency issues because it is mapped as write-combine without clearing the cache after it was zeroed. Rather than duplicate the hacky workaround we use in the GEM allocator for the same reason it turns out that we don't need to have a bespoke memory allocator for the GMU anyway. It uses a flat, global address space and there are only two relatively minor allocations anyway. In short, this is essentially what the DMA API was created for so replace a bunch of memory management code with two calls to allocate and free DMA memory and we're fine. In a previous version of this series I added the dma-ranges property to the device tree file for the GMU and updated the bindings to YAML. Rob correctly pointed out that we didn't need dma-ranges any more but I'm still pushing the YAML conversion because it is good and we'll eventually need it anyway so why not. v5: Refresh on Brian Masney's patch that removes sram from gmu.txt [1] v4: Use dma_alloc_wc() wrappers per Michael Ruhl. v3: Fix YAML description per RobH and remove dma-ranges and replace it with the correct DMA mask in the GMU device. Convert the iova type to a dma_attr_t to make it 32 bit friendly. v2: Fix the example bindings for dma-ranges - the third item is the size Pass false to of_dma_configure so that it fails probe if the DMA region is not set up. [1] https://patchwork.freedesktop.org/patch/356869/?series=74446=1 Jordan Crouse (2): dt-bindings: display: msm: Convert GMU bindings to YAML drm/msm/a6xx: Use the DMA API for GMU memory objects .../devicetree/bindings/display/msm/gmu.txt| 65 --- .../devicetree/bindings/display/msm/gmu.yaml | 123 + drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 113 ++- drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 6 +- 4 files changed, 135 insertions(+), 172 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/msm/gmu.txt create mode 100644 Documentation/devicetree/bindings/display/msm/gmu.yaml -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH] dt-bindings: display: msm: gmu: move sram property to gpu bindings
On Mon, Mar 09, 2020 at 07:18:04AM -0400, Brian Masney wrote: > The sram property was incorrectly added to the GMU binding when it > really belongs with the GPU binding instead. Let's go ahead and > move it. > > While changes are being made here, let's update the sram property > description to mention that this property is only valid for a3xx and > a4xx GPUs. The a3xx/a4xx example in the GPU is replaced with what was > in the GMU. Thank you kindly! I'll re-submit my stack on top of this. Acked-by: Jordan Crouse > Signed-off-by: Brian Masney > Fixes: 198a72c8f9ee ("dt-bindings: display: msm: gmu: add optional ocmem > property") > --- > Background thread: > https://lore.kernel.org/lkml/20200303170159.ga13...@jcrouse1-lnx.qualcomm.com/ > > I started to look at what it would take to convert the GPU bindings to > YAML, however I am unsure of the complete list of "qcom,adreno-XYZ.W" > compatibles that are valid. > > .../devicetree/bindings/display/msm/gmu.txt | 51 - > .../devicetree/bindings/display/msm/gpu.txt | 55 ++- > 2 files changed, 42 insertions(+), 64 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt > b/Documentation/devicetree/bindings/display/msm/gmu.txt > index bf9c7a2a495c..90af5b0a56a9 100644 > --- a/Documentation/devicetree/bindings/display/msm/gmu.txt > +++ b/Documentation/devicetree/bindings/display/msm/gmu.txt > @@ -31,10 +31,6 @@ Required properties: > - iommus: phandle to the adreno iommu > - operating-points-v2: phandle to the OPP operating points > > -Optional properties: > -- sram: phandle to the On Chip Memory (OCMEM) that's present on some > Snapdragon > -SoCs. See Documentation/devicetree/bindings/sram/qcom,ocmem.yaml. > - > Example: > > / { > @@ -67,50 +63,3 @@ Example: > operating-points-v2 = <_opp_table>; > }; > }; > - > -a3xx example with OCMEM support: > - > -/ { > - ... > - > - gpu: adreno@fdb0 { > - compatible = "qcom,adreno-330.2", > - "qcom,adreno"; > - reg = <0xfdb0 0x1>; > - reg-names = "kgsl_3d0_reg_memory"; > - interrupts = ; > - interrupt-names = "kgsl_3d0_irq"; > - clock-names = "core", > - "iface", > - "mem_iface"; > - clocks = < OXILI_GFX3D_CLK>, > - < OXILICX_AHB_CLK>, > - < OXILICX_AXI_CLK>; > - sram = <_sram>; > - power-domains = < OXILICX_GDSC>; > - operating-points-v2 = <_opp_table>; > - iommus = <_iommu 0>; > - }; > - > - ocmem@fdd0 { > - compatible = "qcom,msm8974-ocmem"; > - > - reg = <0xfdd0 0x2000>, > - <0xfec0 0x18>; > - reg-names = "ctrl", > - "mem"; > - > - clocks = < RPM_SMD_OCMEMGX_CLK>, > - < OCMEMCX_OCMEMNOC_CLK>; > - clock-names = "core", > - "iface"; > - > - #address-cells = <1>; > - #size-cells = <1>; > - > - gmu_sram: gmu-sram@0 { > - reg = <0x0 0x10>; > - ranges = <0 0 0xfec0 0x10>; > - }; > - }; > -}; > diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt > b/Documentation/devicetree/bindings/display/msm/gpu.txt > index 7edc298a15f2..fd779cd6994d 100644 > --- a/Documentation/devicetree/bindings/display/msm/gpu.txt > +++ b/Documentation/devicetree/bindings/display/msm/gpu.txt > @@ -35,25 +35,54 @@ Required properties: >bring the GPU out of secure mode. > - firmware-name: optional property of the 'zap-shader' node, listing the >relative path of the device specific zap firmware. > +- sram: phandle to the On Chip Memory (OCMEM) that's present on some a3xx and > +a4xx Snapdragon SoCs. See > +Documentation/devicetree/bindings/sram/qcom,ocmem.yaml. > > -Example 3xx/4xx/a5xx: > +Example 3xx/4xx: > > / { > ... > > - gpu: qcom,kgsl-3d0@430 { > - compatible = "qcom,adreno-320.2", "qcom,adreno"; > - reg = <0x0430 0x2>; > + gpu: adreno@fdb0 { > + compatible = "qcom,adreno-330.2", > + "qcom,adreno"; > + reg = <0xfdb0 0x1>; > reg-names = "kgsl_3d0_reg_memory"; > - interrupts = ; > - clock-names = > - "core", > - "iface", > - "mem_iface"; > - clocks = > - < GFX3D_CLK>, > - < GFX3D_AHB_CLK>, > - < MMSS_IMEM_AHB_CLK>; > + interrupts = ; > + interrupt-names = "kgsl_3d0_irq"; > + clock-names = "core", > + "iface", > +
Re: [PATCH v4 4/4] drm/panfrost: Register devfreq cooling and attempt to add Energy Model
On 09/03/2020 13:41, Lukasz Luba wrote: > Register devfreq cooling device and attempt to register Energy Model. This > will add the devfreq device to the Energy Model framework. It will create > a dedicated and unified data structures used i.e. in thermal framework. > The last NULL parameter indicates that the power model is simplified and > created based on DT 'dynamic-power-coefficient', voltage and frequency. > > Signed-off-by: Lukasz Luba LGTM! Reviewed-by: Steven Price > --- > drivers/gpu/drm/panfrost/panfrost_devfreq.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > index 413987038fbf..8759a73db153 100644 > --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c > +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c > @@ -105,7 +105,7 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) > } > pfdev->devfreq.devfreq = devfreq; > > - cooling = of_devfreq_cooling_register(dev->of_node, devfreq); > + cooling = devfreq_cooling_em_register(devfreq, NULL); > if (IS_ERR(cooling)) > DRM_DEV_INFO(dev, "Failed to register cooling device\n"); > else > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 04/33] drm/panel-ilitek-ili9322: Fix dotclocks
From: Ville Syrjälä The listed dotclocks are two orders of mangnitude out. Fix them. v2: Just divide everything by 100 (Linus) Cc: Linus Walleij Cc: Thierry Reding Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c index f394d53a7da4..09935520e606 100644 --- a/drivers/gpu/drm/panel/panel-ilitek-ili9322.c +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9322.c @@ -540,7 +540,7 @@ static int ili9322_enable(struct drm_panel *panel) /* Serial RGB modes */ static const struct drm_display_mode srgb_320x240_mode = { - .clock = 2453500, + .clock = 24535, .hdisplay = 320, .hsync_start = 320 + 359, .hsync_end = 320 + 359 + 1, @@ -554,7 +554,7 @@ static const struct drm_display_mode srgb_320x240_mode = { }; static const struct drm_display_mode srgb_360x240_mode = { - .clock = 270, + .clock = 27000, .hdisplay = 360, .hsync_start = 360 + 35, .hsync_end = 360 + 35 + 1, @@ -569,7 +569,7 @@ static const struct drm_display_mode srgb_360x240_mode = { /* This is the only mode listed for parallel RGB in the datasheet */ static const struct drm_display_mode prgb_320x240_mode = { - .clock = 640, + .clock = 64000, .hdisplay = 320, .hsync_start = 320 + 38, .hsync_end = 320 + 38 + 1, @@ -584,7 +584,7 @@ static const struct drm_display_mode prgb_320x240_mode = { /* YUV modes */ static const struct drm_display_mode yuv_640x320_mode = { - .clock = 2454000, + .clock = 24540, .hdisplay = 640, .hsync_start = 640 + 252, .hsync_end = 640 + 252 + 1, @@ -598,7 +598,7 @@ static const struct drm_display_mode yuv_640x320_mode = { }; static const struct drm_display_mode yuv_720x360_mode = { - .clock = 270, + .clock = 27000, .hdisplay = 720, .hsync_start = 720 + 252, .hsync_end = 720 + 252 + 1, @@ -613,7 +613,7 @@ static const struct drm_display_mode yuv_720x360_mode = { /* BT.656 VGA mode, 640x480 */ static const struct drm_display_mode itu_r_bt_656_640_mode = { - .clock = 2454000, + .clock = 24540, .hdisplay = 640, .hsync_start = 640 + 3, .hsync_end = 640 + 3 + 1, @@ -628,7 +628,7 @@ static const struct drm_display_mode itu_r_bt_656_640_mode = { /* BT.656 D1 mode 720x480 */ static const struct drm_display_mode itu_r_bt_656_720_mode = { - .clock = 270, + .clock = 27000, .hdisplay = 720, .hsync_start = 720 + 3, .hsync_end = 720 + 3 + 1, -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 01/33] drm/panel-novatek-nt35510: Fix dotclock
From: Ville Syrjälä The dotclock is three orders of magnitude out. Fix it. v2: Just set it to 20MHz (Linus) Cc: Linus Walleij Cc: Sam Ravnborg Signed-off-by: Ville Syrjälä --- drivers/gpu/drm/panel/panel-novatek-nt35510.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/panel/panel-novatek-nt35510.c b/drivers/gpu/drm/panel/panel-novatek-nt35510.c index b4c014126781..4a8fa908a2cf 100644 --- a/drivers/gpu/drm/panel/panel-novatek-nt35510.c +++ b/drivers/gpu/drm/panel/panel-novatek-nt35510.c @@ -1019,7 +1019,7 @@ static const struct nt35510_config nt35510_hydis_hva40wv1 = { */ .mode = { /* The internal pixel clock of the NT35510 is 20 MHz */ - .clock = 2000, + .clock = 2, .hdisplay = 480, .hsync_start = 480 + 2, /* HFP = 2 */ .hsync_end = 480 + 2 + 0, /* HSync = 0 */ -- 2.24.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 15/33] drm/panel-simple: Fix dotclock for EDT ET035012DM6
On Fri, Mar 06, 2020 at 09:02:57AM +0100, Marco Felsch wrote: > On 20-03-03 16:52, Ville Syrjälä wrote: > > On Tue, Mar 03, 2020 at 08:33:20AM +0100, Marco Felsch wrote: > > > Hi Ville, > > > > > > On 20-03-02 22:34, Ville Syrjala wrote: > > > > From: Ville Syrjälä > > > > > > > > The currently listed dotclock disagrees with the currently > > > > listed vrefresh rate. Change the dotclock to match the vrefresh. > > > > > > > > Someone tell me which (if either) of the dotclock or vreresh is > > > > correct? > > > > > > Pls, check the datasheet which is linked within the comment. We hit the > > > vrefresh exactly if we are in SYNC MODE. > > > > It's too much work to start hunting datasheets for all these > > and figuring out what's going on in each case. Pls just > > inform me which way is correct if you know the details. > > How do you know that the clock is wrong if it is to much work? As I said > the clock is completely fine. htotal*vtotal*vrefresh != clock, so one or both are incorrect. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: rework SET_MASTER and DROP_MASTER perm handling
On Mon, 9 Mar 2020 at 08:38, Pekka Paalanen wrote: > > On Fri, 6 Mar 2020 18:51:22 + > Emil Velikov wrote: > > > On Fri, 6 Mar 2020 at 14:00, Pekka Paalanen wrote: > > > > > > On Wed, 19 Feb 2020 13:27:28 + > > > Emil Velikov wrote: > > > > > > > From: Emil Velikov > > > > > > > > > > ... > > > > > > > +/* > > > > + * In the olden days the SET/DROP_MASTER ioctls used to return EACCES > > > > when > > > > + * CAP_SYS_ADMIN was not set. This was used to prevent rogue > > > > applications > > > > + * from becoming master and/or failing to release it. > > > > + * > > > > + * At the same time, the first client (for a given VT) is _always_ > > > > master. > > > > + * Thus in order for the ioctls to succeed, one had to _explicitly_ > > > > run the > > > > + * application as root or flip the setuid bit. > > > > + * > > > > + * If the CAP_SYS_ADMIN was missing, no other client could become > > > > master... > > > > + * EVER :-( Leading to a) the graphics session dying badly or b) a > > > > completely > > > > + * locked session. > > > > + * > > > > > > Hi, > > > > > > sorry I had to trim this email harshly, but Google did not want to > > > deliver it otherwise. > > > > > > I agree that being able to drop master without CAP_SYS_ADMIN sounds > > > like a good thing. > > > > > > > + * > > > > + * As some point systemd-logind was introduced to orchestrate and > > > > delegate > > > > + * master as applicable. It does so by opening the fd and passing it > > > > to users > > > > + * while in itself logind a) does the set/drop master per users' > > > > request and > > > > + * b) * implicitly drops master on VT switch. > > > > + * > > > > + * Even though logind looks like the future, there are a few issues: > > > > + * - using it is not possible on some platforms > > > > + * - applications may not be updated to use it, > > > > + * - any client which fails to drop master* can DoS the application > > > > using > > > > + * logind, to a varying degree. > > > > + * > > > > + * * Either due missing CAP_SYS_ADMIN or simply not calling > > > > DROP_MASTER. > > > > + * > > > > + * > > > > + * Here we implement the next best thing: > > > > + * - ensure the logind style of fd passing works unchanged, and > > > > + * - allow a client to drop/set master, iff it is/was master at a > > > > given point > > > > + * in time. > > > > > > I understand the drop master part, because it is needed to get rid of > > > apps that accidentally gain DRM master because they were the first one > > > to open the DRM device (on a particular VT?). It could happen e.g. if a > > > Wayland client is inspecting DRM devices to figure what it wants to > > > lease while the user has VT-switched to a text-mode VT, I guess. E.g. > > > starting a Wayland VR compositor from a VT for whatever reason. > > > > > > The set master without CAP_SYS_ADMIN part I don't understand. > > > > > As you point out application can drop master for various reasons. One > > of which may be to say spawn another program which requires master for > > _non_ modeset reasons. For example: > > - amdgpu: create a renderer and use the context/process priority override > > IOCTL > > - vmwgfx: stream claim/unref (amongst others). > > Hi, > > if none of that is about KMS resources specifically, then to me it > seems like a mis-design that should be avoided if still possible. DRM > master is all about modeset, and in my option should be about nothing > else. > > Are those needed to keep existing userspace working? > In a perfect world sure. In practical terms - fixing these is less likely. We have limited time and thou shalt not regress userspace. About specifics on the individual drivers, I'll refer you to the respective teams. > > > Another case to consider is classic X or Wayland compositor. With > > CAP_SYS_ADMIN for DROP_MASTER removed, yet retained in SET_MASTER, the > > IOCTL will fail. Thus: > > - weston results in frozen session and session switching (have to > > force kill weston or sudo loginctl kill-session) > > - depending on the driver, X will work or crash > > > > To make this clearer I'll include //comment sections in the code. > > > > // comment > > To ensure the application can reclaim its master status, the tweaked > > CAP_SYS_ADMIN handling is needed for both IOCTLs. Otherwise X or > > Wayland compositors may freeze or crash as SET_MASTER fails. > > // comment > > A Wayland compositor or Xorg that got DRM master by first-opener up to > now has not been able to drop DRM master, which means VT switching away > does not work - does it? Correct - it does not. Yet the return code of the drmDropMaster is ignored so the compositor continues working... Somewhat. Weston got some checks recently IIRC. > If drop-master succeeded, then VT-switch back > doesn't work, which in my opinion is VT-switching failing again just in > a different way. > If the drmSetMaster fails (while Drop was successful), you'll get user-facing issues. The crash or freeze as
Re: [PATCH 24/33] drm/panel-simple: Fix dotclock for Ortustech COM37H3M
On Thu, Mar 05, 2020 at 08:41:43PM +0100, H. Nikolaus Schaller wrote: > > > Am 03.03.2020 um 16:49 schrieb H. Nikolaus Schaller : > > > > Hi, > > > >> Am 03.03.2020 um 16:03 schrieb Ville Syrjälä > >> : > >> > >>> I haven't looked into the driver code, but would it be > >>> possible to specify .clock = 0 (or leave it out) to > >>> calculate it bottom up? This would avoid such inconsistencies. > >> > >> I'm going to remove .vrefresh entirely from the struct. > >> It'll just be calculated from the other timings as needed. > > > > Ok! > > > > Anyways we should fix the panel timings so that it is compatible to > > .vrefresh = 60. > > > > I'll give it a try and let you know. > > Ok, here is a new parameter set within data sheet limits for both > panel variants: > > static const struct drm_display_mode ortustech_com37h3m_mode = { > .clock = 22153, > .hdisplay = 480, > .hsync_start = 480 + 40, > .hsync_end = 480 + 40 + 10, > .htotal = 480 + 40 + 10 + 40, > .vdisplay = 640, > .vsync_start = 640 + 4, > .vsync_end = 640 + 4 + 2, > .vtotal = 640 + 4 + 2 + 4, > .vrefresh = 60, > .flags = DRM_MODE_FLAG_NVSYNC | DRM_MODE_FLAG_NHSYNC, > }; > > I have tested on our omap3 based board and didn't find an issue > so you can insert into your patch. Migth be better if you send that so we get proper attribution and you can explain the change properly in the commit message. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/mm: Allow drm_mm_initialized() to be used outside of the locks
Mark up the potential racy read in drm_mm_initialized(), as we want a cheap and cheerful check: [ 121.098731] BUG: KCSAN: data-race in _i915_gem_object_create_stolen [i915] / rm_hole [ 121.098766] [ 121.098789] write (marked) to 0x8881f01ed330 of 8 bytes by task 3568 on cpu 3: [ 121.098831] rm_hole+0x64/0x140 [ 121.098860] drm_mm_insert_node_in_range+0x3d3/0x6c0 [ 121.099254] i915_gem_stolen_insert_node_in_range+0x91/0xe0 [i915] [ 121.099646] _i915_gem_object_create_stolen+0x9d/0x100 [i915] [ 121.100047] i915_gem_object_create_region+0x7a/0xa0 [i915] [ 121.100451] i915_gem_object_create_stolen+0x33/0x50 [i915] [ 121.100849] intel_engine_create_ring+0x1af/0x280 [i915] [ 121.101242] __execlists_context_alloc+0xce/0x3d0 [i915] [ 121.101635] execlists_context_alloc+0x25/0x40 [i915] [ 121.102030] intel_context_alloc_state+0xb6/0xf0 [i915] [ 121.102420] __intel_context_do_pin+0x1ff/0x220 [i915] [ 121.102815] i915_gem_do_execbuffer+0x46b4/0x4c20 [i915] [ 121.103211] i915_gem_execbuffer2_ioctl+0x2c3/0x580 [i915] [ 121.103244] drm_ioctl_kernel+0xe4/0x120 [ 121.103269] drm_ioctl+0x297/0x4c7 [ 121.103296] ksys_ioctl+0x89/0xb0 [ 121.103321] __x64_sys_ioctl+0x42/0x60 [ 121.103349] do_syscall_64+0x6e/0x2c0 [ 121.103377] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 121.103403] [ 121.103426] read to 0x8881f01ed330 of 8 bytes by task 3109 on cpu 1: [ 121.103819] _i915_gem_object_create_stolen+0x30/0x100 [i915] [ 121.104228] i915_gem_object_create_region+0x7a/0xa0 [i915] [ 121.104631] i915_gem_object_create_stolen+0x33/0x50 [i915] [ 121.105025] intel_engine_create_ring+0x1af/0x280 [i915] [ 121.105420] __execlists_context_alloc+0xce/0x3d0 [i915] [ 121.105818] execlists_context_alloc+0x25/0x40 [i915] [ 121.106202] intel_context_alloc_state+0xb6/0xf0 [i915] [ 121.106595] __intel_context_do_pin+0x1ff/0x220 [i915] [ 121.106985] i915_gem_do_execbuffer+0x46b4/0x4c20 [i915] [ 121.107375] i915_gem_execbuffer2_ioctl+0x2c3/0x580 [i915] [ 121.107409] drm_ioctl_kernel+0xe4/0x120 [ 121.107437] drm_ioctl+0x297/0x4c7 [ 121.107464] ksys_ioctl+0x89/0xb0 [ 121.107489] __x64_sys_ioctl+0x42/0x60 [ 121.107511] do_syscall_64+0x6e/0x2c0 [ 121.107535] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Signed-off-by: Chris Wilson --- include/drm/drm_mm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_mm.h b/include/drm/drm_mm.h index d7939c054259..ee8b0e80ca90 100644 --- a/include/drm/drm_mm.h +++ b/include/drm/drm_mm.h @@ -272,7 +272,7 @@ static inline bool drm_mm_node_allocated(const struct drm_mm_node *node) */ static inline bool drm_mm_initialized(const struct drm_mm *mm) { - return mm->hole_stack.next; + return READ_ONCE(mm->hole_stack.next); } /** -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Mark up racy check of drm_gem_object.handle_count
[ 1715.899800] BUG: KCSAN: data-race in drm_gem_handle_create_tail / drm_gem_object_handle_put_unlocked [ 1715.899838] [ 1715.899861] write to 0x8881830f3604 of 4 bytes by task 7834 on cpu 1: [ 1715.899896] drm_gem_handle_create_tail+0x62/0x250 [ 1715.899927] drm_gem_open_ioctl+0xc1/0x160 [ 1715.899956] drm_ioctl_kernel+0xe4/0x120 [ 1715.899981] drm_ioctl+0x297/0x4c7 [ 1715.93] ksys_ioctl+0x89/0xb0 [ 1715.900027] __x64_sys_ioctl+0x42/0x60 [ 1715.900052] do_syscall_64+0x6e/0x2c0 [ 1715.900079] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 1715.900100] [ 1715.900119] read to 0x8881830f3604 of 4 bytes by task 8137 on cpu 0: [ 1715.900149] drm_gem_object_handle_put_unlocked+0x31/0x130 [ 1715.900180] drm_gem_object_release_handle+0x93/0xe0 [ 1715.900208] drm_gem_handle_delete+0x7b/0xe0 [ 1715.900235] drm_gem_close_ioctl+0x61/0x80 [ 1715.900264] drm_ioctl_kernel+0xe4/0x120 [ 1715.900291] drm_ioctl+0x297/0x4c7 [ 1715.900316] ksys_ioctl+0x89/0xb0 [ 1715.900340] __x64_sys_ioctl+0x42/0x60 [ 1715.900363] do_syscall_64+0x6e/0x2c0 [ 1715.900388] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Signed-off-by: Chris Wilson --- drivers/gpu/drm/drm_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index a9e4a610445a..37627d06fb06 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -218,7 +218,7 @@ drm_gem_object_handle_put_unlocked(struct drm_gem_object *obj) struct drm_device *dev = obj->dev; bool final = false; - if (WARN_ON(obj->handle_count == 0)) + if (WARN_ON(READ_ONCE(obj->handle_count) == 0)) return; /* -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/ttm: fix false positive assert
Pierre-eric, just a gentle ping on this? Could I get a tested-by? Ray can you ack or even review this? Thanks, Christian. Am 06.03.20 um 13:41 schrieb Christian König: The assert sometimes incorrectly triggers when pinned BOs are destroyed. Signed-off-by: Christian König --- drivers/gpu/drm/ttm/ttm_bo.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c index 2445e2bd6267..ca5a8d01ff1f 100644 --- a/drivers/gpu/drm/ttm/ttm_bo.c +++ b/drivers/gpu/drm/ttm/ttm_bo.c @@ -151,8 +151,6 @@ static void ttm_bo_add_mem_to_lru(struct ttm_buffer_object *bo, struct ttm_bo_device *bdev = bo->bdev; struct ttm_mem_type_manager *man; - dma_resv_assert_held(bo->base.resv); - if (!list_empty(>lru)) return; @@ -611,7 +609,8 @@ static void ttm_bo_release(struct kref *kref) */ if (bo->mem.placement & TTM_PL_FLAG_NO_EVICT) { bo->mem.placement &= ~TTM_PL_FLAG_NO_EVICT; - ttm_bo_move_to_lru_tail(bo, NULL); + ttm_bo_del_from_lru(bo); + ttm_bo_add_mem_to_lru(bo, >mem); } kref_init(>kref); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/exynos: Fix cleanup of IOMMU related objects
Store the IOMMU mapping created by the device core of each Exynos DRM sub-device and restore it when the Exynos DRM driver is unbound. This fixes IOMMU initialization failure for the second time when a deferred probe is triggered from the bind() callback of master's compound DRM driver. This also fixes the following issue found using kmemleak detector: unreferenced object 0xc2137640 (size 64): comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s) hex dump (first 32 bytes): 50 a3 14 c2 80 a2 14 c2 01 00 00 00 20 00 00 00 P... ... 00 10 00 00 00 80 00 00 00 00 00 00 00 00 00 00 backtrace: [<3acd268d>] arch_setup_dma_ops+0x4c/0x104 [<9f7d2cce>] of_dma_configure+0x19c/0x3a4 [] really_probe+0xb0/0x47c [<4f510e4f>] driver_probe_device+0x78/0x1c4 [<7481a0cf>] device_driver_attach+0x58/0x60 [<0ff8f5c1>] __driver_attach+0xb8/0x158 [<86006144>] bus_for_each_dev+0x74/0xb4 [<10159dca>] bus_add_driver+0x1c0/0x200 [<8a265265>] driver_register+0x74/0x108 [] exynos_drm_init+0xb0/0x134 [] do_one_initcall+0x90/0x458 [<6da35917>] kernel_init_freeable+0x188/0x200 [] kernel_init+0x8/0x110 [<1f3cddf9>] ret_from_fork+0x14/0x20 [<8cd12507>] 0x0 unreferenced object 0xc214a280 (size 128): comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s) hex dump (first 32 bytes): 00 a0 ec ed 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 backtrace: [<3acd268d>] arch_setup_dma_ops+0x4c/0x104 [<9f7d2cce>] of_dma_configure+0x19c/0x3a4 [] really_probe+0xb0/0x47c [<4f510e4f>] driver_probe_device+0x78/0x1c4 [<7481a0cf>] device_driver_attach+0x58/0x60 [<0ff8f5c1>] __driver_attach+0xb8/0x158 [<86006144>] bus_for_each_dev+0x74/0xb4 [<10159dca>] bus_add_driver+0x1c0/0x200 [<8a265265>] driver_register+0x74/0x108 [] exynos_drm_init+0xb0/0x134 [] do_one_initcall+0x90/0x458 [<6da35917>] kernel_init_freeable+0x188/0x200 [] kernel_init+0x8/0x110 [<1f3cddf9>] ret_from_fork+0x14/0x20 [<8cd12507>] 0x0 unreferenced object 0xedeca000 (size 4096): comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 backtrace: [<3acd268d>] arch_setup_dma_ops+0x4c/0x104 [<9f7d2cce>] of_dma_configure+0x19c/0x3a4 [] really_probe+0xb0/0x47c [<4f510e4f>] driver_probe_device+0x78/0x1c4 [<7481a0cf>] device_driver_attach+0x58/0x60 [<0ff8f5c1>] __driver_attach+0xb8/0x158 [<86006144>] bus_for_each_dev+0x74/0xb4 [<10159dca>] bus_add_driver+0x1c0/0x200 [<8a265265>] driver_register+0x74/0x108 [] exynos_drm_init+0xb0/0x134 [] do_one_initcall+0x90/0x458 [<6da35917>] kernel_init_freeable+0x188/0x200 [] kernel_init+0x8/0x110 [<1f3cddf9>] ret_from_fork+0x14/0x20 [<8cd12507>] 0x0 unreferenced object 0xc214a300 (size 128): comm "swapper/0", pid 1, jiffies 4294937900 (age 3127.400s) hex dump (first 32 bytes): 00 a3 14 c2 00 a3 14 c2 00 40 18 c2 00 80 18 c2 .@.. 02 00 02 00 ad 4e ad de ff ff ff ff ff ff ff ff .N.. backtrace: [<08cbd8bc>] iommu_domain_alloc+0x24/0x50 [] arm_iommu_create_mapping+0xe4/0x134 [<3acd268d>] arch_setup_dma_ops+0x4c/0x104 [<9f7d2cce>] of_dma_configure+0x19c/0x3a4 [] really_probe+0xb0/0x47c [<4f510e4f>] driver_probe_device+0x78/0x1c4 [<7481a0cf>] device_driver_attach+0x58/0x60 [<0ff8f5c1>] __driver_attach+0xb8/0x158 [<86006144>] bus_for_each_dev+0x74/0xb4 [<10159dca>] bus_add_driver+0x1c0/0x200 [<8a265265>] driver_register+0x74/0x108 [] exynos_drm_init+0xb0/0x134 [] do_one_initcall+0x90/0x458 [<6da35917>] kernel_init_freeable+0x188/0x200 [] kernel_init+0x8/0x110 [<1f3cddf9>] ret_from_fork+0x14/0x20 Signed-off-by: Marek Szyprowski Reviewed-by: Lukasz Luba --- v2: - added kmemlead detector log from Lukasz Luba - added comment in the code as requested by Inki Dae --- drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 5 ++-- drivers/gpu/drm/exynos/exynos7_drm_decon.c| 5 ++-- drivers/gpu/drm/exynos/exynos_drm_dma.c | 28 +-- drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 ++-- drivers/gpu/drm/exynos/exynos_drm_fimc.c | 5 ++-- drivers/gpu/drm/exynos/exynos_drm_fimd.c | 5 ++-- drivers/gpu/drm/exynos/exynos_drm_g2d.c | 5 ++-- drivers/gpu/drm/exynos/exynos_drm_gsc.c | 5 ++-- drivers/gpu/drm/exynos/exynos_drm_rotator.c | 5 ++-- drivers/gpu/drm/exynos/exynos_drm_scaler.c| 6 ++-- drivers/gpu/drm/exynos/exynos_mixer.c | 7 +++-- 11 files changed, 53 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c index
Re: [PATCH] drm/imx: parallel-display: Adjust bus_flags and bus_format handling
Hi Marek, On Thu, 2019-11-14 at 14:17 +0100, Marek Vasut wrote: > The bus_flags and bus_format handling logic does not seem to cover > all potential usecases. Specifically, this seems to fail with an > "edt,etm0700g0edh6" display attached to an 24bit display interface, > with interface-pix-fmt = "rgb24" set in DT. interface-pix-fmt is a legacy property that was never intended to be used as an override for the panel bus format. The bus flags were supposed to be set from the display-timings node, back when there was no of-graph connected panel at all. That being said, there isn't really a proper alternative that allows to override the bus format requested by the panel driver in the device tree to account for weird wiring. We could reuse the bus-width endpoint property documented in [1], but that wouldn't completely specify how the RGB components are to be mapped onto the parallel bus. [1] Documentation/devicetree/bindings/media/video-interfaces.txt I do wonder whether for your case it would be better to implement a MEDIA_BUS_FMT_RGB666_1X24_CPADLO though, to leave the LSBs untouched instead of risking to dump them into accidental PCB antennae. > In this specific setup, the panel-simple.c driver entry for the display > sets .bus_flags to non-zero value. However, as imxpd->bus_format is set > from the DT property "interface-pix-fmt", imx_pd_encoder_atomic_check() > will set imx_crtc_state->bus_flags = imxpd->bus_flags even though the > imxpd->bus_flags is zero, while the di->bus_flags is correctly set by > the panel-simple.c and non-zero. > > The result is incorrect flags being > used for the display configuration and thus an image corruption. > (Specifically, DRM_BUS_FLAG_PIXDATA_POSEDGE is not propagated and thus > the ipuv3 clocks pixels on the wrong edge). > > This patch fixes the problem by overriding the imx_crtc_state->bus_format > from the imxpd->bus_format only if the DT property "interface-pix-fmt" is > present or if the DI provides no formats. Similarly for bus_flags, which > are set from imxpd->bus_flags only if the DI provides no formats. > > Signed-off-by: Marek Vasut > Cc: Daniel Vetter > Cc: David Airlie > Cc: Fabio Estevam > Cc: NXP Linux Team > Cc: Philipp Zabel > Cc: Sascha Hauer > Cc: Shawn Guo > Cc: linux-arm-ker...@lists.infradead.org > To: dri-devel@lists.freedesktop.org > --- > drivers/gpu/drm/imx/parallel-display.c | 13 - > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/imx/parallel-display.c > b/drivers/gpu/drm/imx/parallel-display.c > index 35518e5de356..92f00b12c068 100644 > --- a/drivers/gpu/drm/imx/parallel-display.c > +++ b/drivers/gpu/drm/imx/parallel-display.c > @@ -113,13 +113,16 @@ static int imx_pd_encoder_atomic_check(struct > drm_encoder *encoder, > struct drm_display_info *di = _state->connector->display_info; > struct imx_parallel_display *imxpd = enc_to_imxpd(encoder); > > - if (!imxpd->bus_format && di->num_bus_formats) { > - imx_crtc_state->bus_flags = di->bus_flags; > + if (imxpd->bus_format || !di->num_bus_formats) I see no reason to invert the logic here. Let's keep the common case first. > + imx_crtc_state->bus_format = imxpd->bus_format; > + else > imx_crtc_state->bus_format = di->bus_formats[0]; > - } else { > + > + if (di->num_bus_formats) > + imx_crtc_state->bus_flags = di->bus_flags; > + else > imx_crtc_state->bus_flags = imxpd->bus_flags; > - imx_crtc_state->bus_format = imxpd->bus_format; > - } > + > imx_crtc_state->di_hsync_pin = 2; > imx_crtc_state->di_vsync_pin = 3; So while I don't like this being used at all, I think the patch improves consistency, as imx_pd_connector_get_modes doesn't allow to override the panel's modes with DT display-timings either. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 09/12] dt-bindings: display: bridge: lvds-codec: Add new bus-width prop
Hi Laurent, On Tue, 25 Feb 2020 10:13:54 +0100 Boris Brezillon wrote: > > > diff --git > > > a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > > b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > > index 8f373029f5d2..7c4e42f4de61 100644 > > > --- a/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > > +++ b/Documentation/devicetree/bindings/display/bridge/lvds-codec.yaml > > > @@ -55,6 +55,14 @@ properties: > > > description: | > > >For LVDS encoders, port 0 is the parallel input > > >For LVDS decoders, port 0 is the LVDS input > > > +properties: > > > + bus-width: > > > +allOf: > > > + - $ref: /schemas/types.yaml#/definitions/uint32 > > > + - enum: [18, 24] > > > + - default: 24 > > > + description: > > > +Number of data lines used to transmit the RGB data. > > > > This is a bit unclear. First of all, depending on whether the node is an > > LVDS encoder or decoder, port@0 is either a parallel input or an LVDS > > input. The property mentiones RGB data, does it mean it apply to LVDS > > encoders only ? Or should it be in port@1 for LVDS decoders ? > > Right, I only considered the encoder case here. For the decoder case, we > don't need a bus-width prop yet, as the bus format output is currently > enforced by the bus format input of the next component in the chain > (panel/next-bridge), but that might change if we start dealing with > panel/bridges supporting several input formats and expecting the LVDS > encoder/decoder to select one. What we do need for the decoder case > though, is a data-mapping prop, otherwise this LVDS bridge exposes a > FIXED in-format and the previous element in the chain has to use its > 'default' output format (which might not be appropriate). > > Maybe we should go for Sam's approach and expose a data-mapping prop > on both ends of the bridge (that implies describing RGB/DPI bus width > using the data-mapping prop), this way we wouldn't have to distinguish > the encoder/decoder case. Gentle ping. Regards, Boris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH] drm/stm: repair runtime power management
Hello Marek, Thank for your patch. Pm_runtime_put_sync is also done into function ltdc_crtc_mode_fixup. To avoid several call of Pm_runtime_put_sync, it could be better to check pm_runtime activity: + int ret; DRM_DEBUG_DRIVER("\n"); + if (!pm_runtime_active(ddev->dev)) { + ret = pm_runtime_get_sync(ddev->dev); + if (ret) { + DRM_ERROR("Failed to enable crtc, cannot get sync\n"); + return; + } + } + Best regards Yannick Fertré -Original Message- From: Marek Vasut Sent: samedi 29 février 2020 23:17 To: dri-devel@lists.freedesktop.org Cc: Marek Vasut ; Yannick FERTRE ; Philippe CORNU ; Benjamin Gaignard ; Vincent ABRIOU ; Maxime Coquelin ; Alexandre TORGUE ; linux-st...@st-md-mailman.stormreply.com; linux-arm-ker...@lists.infradead.org Subject: [PATCH] drm/stm: repair runtime power management Add missing pm_runtime_get_sync() into ltdc_crtc_atomic_enable() to match pm_runtime_put_sync() in ltdc_crtc_atomic_disable(), otherwise the LTDC might suspend via runtime PM, disable clock, and then fail to resume later on. The test which triggers it is roughly -- run qt5 application which uses eglfs platform and etnaviv, stop the application, sleep for 15 minutes, run the application again. This leads to a timeout waiting for vsync, because the LTDC has suspended, but did not resume. Fixes: 35ab6cfbf211 ("drm/stm: support runtime power management") Signed-off-by: Marek Vasut Cc: Yannick Fertré Cc: Philippe Cornu Cc: Benjamin Gaignard Cc: Vincent Abriou Cc: Maxime Coquelin Cc: Alexandre Torgue To: dri-devel@lists.freedesktop.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-arm-ker...@lists.infradead.org --- [ cut here ] WARNING: CPU: 0 PID: 297 at drivers/gpu/drm/drm_atomic_helper.c:1494 drm_atomic_helper_wait_for_vblanks+0x1dc/0x200 [CRTC:35:crtc-0] vblank wait timed out Modules linked in: CPU: 0 PID: 297 Comm: QSGRenderThread Not tainted 5.6.0-rc3-next-20200228-00010-g318bf0fc08ef #2 Hardware name: STM32 (Device Tree Support) [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [] (show_stack) from [] (dump_stack+0xb4/0xd0) [] (dump_stack) from [] (__warn+0xd4/0xf0) [] (__warn) from [] (warn_slowpath_fmt+0x78/0xa8) [] (warn_slowpath_fmt) from [] (drm_atomic_helper_wait_for_vblanks+0x1dc/0x200) [] (drm_atomic_helper_wait_for_vblanks) from [] (drm_atomic_helper_commit_tail+0 x50/0x60) [] (drm_atomic_helper_commit_tail) from [] (commit_tail+0x12c/0x13c) [] (commit_tail) from [] (drm_atomic_helper_commit+0xf4/0x100) [] (drm_atomic_helper_commit) from [] (drm_atomic_helper_set_config+0x58/0x6c) [] (drm_atomic_helper_set_config) from [] (drm_mode_setcrtc+0x450/0x550) [] (drm_mode_setcrtc) from [] (drm_ioctl_kernel+0x90/0xe8) [] (drm_ioctl_kernel) from [] (drm_ioctl+0x2e4/0x32c) [] (drm_ioctl) from [] (vfs_ioctl+0x20/0x38) [] (vfs_ioctl) from [] (ksys_ioctl+0xbc/0x7b0) [] (ksys_ioctl) from [] (ret_fast_syscall+0x0/0x54) Exception stack(0xee8f3fa8 to 0xee8f3ff0) 3fa0: 0005 adcbeb18 0005 c06864a2 adcbeb18 0001 3fc0: 0005 adcbeb18 c06864a2 0036 0029 0023 0023 0007 3fe0: b113b098 adcbeafc b1125413 b6155cf8 ---[ end trace 2ad5ba954ceb767a ]--- --- drivers/gpu/drm/stm/ltdc.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c index 99bf93e8b36f..301de0498078 100644 --- a/drivers/gpu/drm/stm/ltdc.c +++ b/drivers/gpu/drm/stm/ltdc.c @@ -425,9 +425,12 @@ static void ltdc_crtc_atomic_enable(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"); + pm_runtime_get_sync(ddev->dev); + /* Sets the background color value */ reg_write(ldev->regs, LTDC_BCCR, BCCR_BCBLACK); -- 2.25.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 1/2] drm/virtio: factor out the sg_table from virtio_gpu_object
On Wed, Mar 04, 2020 at 05:32:11PM -0800, Gurchetan Singh wrote: > A resource will be a shmem based resource or a (planned) > vram based resource, so it makes sense to factor out common fields > (resource handle, dumb). > > v2: move mapped field to shmem object Pushed to drm-misc-next. thanks, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: rework SET_MASTER and DROP_MASTER perm handling
On Fri, 6 Mar 2020 18:51:22 + Emil Velikov wrote: > On Fri, 6 Mar 2020 at 14:00, Pekka Paalanen wrote: > > > > On Wed, 19 Feb 2020 13:27:28 + > > Emil Velikov wrote: > > > > > From: Emil Velikov > > > > > > > ... > > > > > +/* > > > + * In the olden days the SET/DROP_MASTER ioctls used to return EACCES > > > when > > > + * CAP_SYS_ADMIN was not set. This was used to prevent rogue applications > > > + * from becoming master and/or failing to release it. > > > + * > > > + * At the same time, the first client (for a given VT) is _always_ > > > master. > > > + * Thus in order for the ioctls to succeed, one had to _explicitly_ run > > > the > > > + * application as root or flip the setuid bit. > > > + * > > > + * If the CAP_SYS_ADMIN was missing, no other client could become > > > master... > > > + * EVER :-( Leading to a) the graphics session dying badly or b) a > > > completely > > > + * locked session. > > > + * > > > > Hi, > > > > sorry I had to trim this email harshly, but Google did not want to > > deliver it otherwise. > > > > I agree that being able to drop master without CAP_SYS_ADMIN sounds > > like a good thing. > > > > > + * > > > + * As some point systemd-logind was introduced to orchestrate and > > > delegate > > > + * master as applicable. It does so by opening the fd and passing it to > > > users > > > + * while in itself logind a) does the set/drop master per users' request > > > and > > > + * b) * implicitly drops master on VT switch. > > > + * > > > + * Even though logind looks like the future, there are a few issues: > > > + * - using it is not possible on some platforms > > > + * - applications may not be updated to use it, > > > + * - any client which fails to drop master* can DoS the application > > > using > > > + * logind, to a varying degree. > > > + * > > > + * * Either due missing CAP_SYS_ADMIN or simply not calling DROP_MASTER. > > > + * > > > + * > > > + * Here we implement the next best thing: > > > + * - ensure the logind style of fd passing works unchanged, and > > > + * - allow a client to drop/set master, iff it is/was master at a given > > > point > > > + * in time. > > > > I understand the drop master part, because it is needed to get rid of > > apps that accidentally gain DRM master because they were the first one > > to open the DRM device (on a particular VT?). It could happen e.g. if a > > Wayland client is inspecting DRM devices to figure what it wants to > > lease while the user has VT-switched to a text-mode VT, I guess. E.g. > > starting a Wayland VR compositor from a VT for whatever reason. > > > > The set master without CAP_SYS_ADMIN part I don't understand. > > > As you point out application can drop master for various reasons. One > of which may be to say spawn another program which requires master for > _non_ modeset reasons. For example: > - amdgpu: create a renderer and use the context/process priority override > IOCTL > - vmwgfx: stream claim/unref (amongst others). Hi, if none of that is about KMS resources specifically, then to me it seems like a mis-design that should be avoided if still possible. DRM master is all about modeset, and in my option should be about nothing else. Are those needed to keep existing userspace working? > Another case to consider is classic X or Wayland compositor. With > CAP_SYS_ADMIN for DROP_MASTER removed, yet retained in SET_MASTER, the > IOCTL will fail. Thus: > - weston results in frozen session and session switching (have to > force kill weston or sudo loginctl kill-session) > - depending on the driver, X will work or crash > > To make this clearer I'll include //comment sections in the code. > > // comment > To ensure the application can reclaim its master status, the tweaked > CAP_SYS_ADMIN handling is needed for both IOCTLs. Otherwise X or > Wayland compositors may freeze or crash as SET_MASTER fails. > // comment A Wayland compositor or Xorg that got DRM master by first-opener up to now has not been able to drop DRM master, which means VT switching away does not work - does it? If drop-master succeeded, then VT-switch back doesn't work, which in my opinion is VT-switching failing again just in a different way. OTOH, if applications exist that rely on drop-master failing in this specific case, making drop-master succeed would break them. That might include a buggy set-master path that was written, but does not actually work because it was never tested since drop-master never worked. So I do not fully buy this argument yet, but I also cannot name any explicit examples that would break. > > > + * > > > + * As a result this fixes, the following when using root-less build w/o > > > logind > > > > Why is non-root without any logind-equivalent a use case that should > > work? > > > // comment > Some platforms don't have equivalent (Android, CrOS, some BSDs), yet > root is required _solely_ for DROP/SET MASTER. So tweaking the > requirement sounds perfectly
Re: [Intel-gfx] [PATCH v4 1/2] drm/edid: Name the detailed monitor range flags
On Fri, 06 Mar 2020, Manasi Navare wrote: > On Fri, Mar 06, 2020 at 12:30:46PM +0200, Jani Nikula wrote: >> On Thu, 05 Mar 2020, Manasi Navare wrote: >> > This patch adds defines for the detailed monitor >> > range flags as per the EDID specification. >> > >> > Suggested-by: Ville Syrjälä >> > Cc: Ville Syrjälä >> > Cc: Harry Wentland >> > Cc: Clinton A Taylor >> > Cc: Kazlauskas Nicholas >> > Signed-off-by: Manasi Navare >> > --- >> > include/drm/drm_edid.h | 5 + >> > 1 file changed, 5 insertions(+) >> > >> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h >> > index f0b03d401c27..f89c97623845 100644 >> > --- a/include/drm/drm_edid.h >> > +++ b/include/drm/drm_edid.h >> > @@ -91,6 +91,11 @@ struct detailed_data_string { >> >u8 str[13]; >> > } __attribute__((packed)); >> > >> > +#define EDID_DEFAULT_GTF_SUPPORT_FLAG 0x00 >> > +#define EDID_RANGE_LIMITS_ONLY_FLAG 0x01 >> > +#define EDID_SECONDARY_GTF_SUPPORT_FLAG 0x02 >> > +#define EDID_CVT_SUPPORT_FLAG 0x04 >> >> Bikeshed for consideration: >> >> drm_edid.h has some macros with DRM_EDID_ prefix, some with EDID_ >> prefix, and then some with no prefix at all really. Should we start >> consolidating on something when we add more? >> > > Yes Jani I did notice the same thing and didnt know which convention > to continue to follow but I noticed that majority of the defines were > just EDID_ so just used that for these new defines. Ah, look again, DRM_EDID_ trumps EDID_ 51 to 15. > Is there a particular way you wish to consolidate this and use a specific > convention for #defines? > > I can atleast change these new defines based on a preferred convention and > then > separate patches to change the rest in .h and corresponding usages in .c > files. I'd suggest DRM_EDID_ for new ones, perhaps eventually rename old ones. BR, Jani. > > Regards > Manasi > >> BR, >> Jani. >> >> >> > + >> > struct detailed_data_monitor_range { >> >u8 min_vfreq; >> >u8 max_vfreq; >> >> -- >> Jani Nikula, Intel Open Source Graphics Center -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
AW: [PATCH] drm/amdgpu/display: Fix an error handling path in 'dm_update_crtc_state()'
Von: kernel-janitors-ow...@vger.kernel.org im Auftrag von Christophe JAILLET Gesendet: Sonntag, 8. März 2020 10:26 An: harry.wentl...@amd.com; sunpeng...@amd.com; alexander.deuc...@amd.com; christian.koe...@amd.com; david1.z...@amd.com; airl...@linux.ie; dan...@ffwll.ch; nicholas.kazlaus...@amd.com; bhawanpreet.la...@amd.com; mario.kleiner...@gmail.com; david.fran...@amd.com Cc: amd-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-ker...@vger.kernel.org; kernel-janit...@vger.kernel.org; Christophe JAILLET Betreff: [PATCH] drm/amdgpu/display: Fix an error handling path in 'dm_update_crtc_state()' 'dc_stream_release()' may be called twice. Once here, and once below in the error handling path if we branch to the 'fail' label. Set 'new_stream' to NULL, once released to avoid the duplicated release function call. Signed-off-by: Christophe JAILLET --- Maybe the 'goto fail' at line 7745 should be turned into a 'return ret' instead. Could be clearer. No Fixes tag provided because I've not been able to dig deep enough in the git history. --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c index 97c1b01c0fc1..9d7773a77c4f 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -7704,8 +7704,10 @@ static int dm_update_crtc_state(struct amdgpu_display_manager *dm, skip_modeset: /* Release extra reference */ - if (new_stream) -dc_stream_release(new_stream); + if (new_stream) { + dc_stream_release(new_stream); + new_stream = NULL; + } dc_stream_release() is NULL-checked, so the if can be dropped. re, wh /* * We want to do dc stream updates that do not require a -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
kernel BUG at drivers/dma-buf/dma-buf.c:LINE!
Hello, syzbot found the following crash on: HEAD commit:63623fd4 Merge tag 'for-linus' of git://git.kernel.org/pub.. git tree: upstream console output: https://syzkaller.appspot.com/x/log.txt?x=177bf331e0 kernel config: https://syzkaller.appspot.com/x/.config?x=9833e26bab355358 dashboard link: https://syzkaller.appspot.com/bug?extid=d6734079f30f7fc39021 compiler: gcc (GCC) 9.0.0 20181231 (experimental) syz repro: https://syzkaller.appspot.com/x/repro.syz?x=109d3ac3e0 IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+d6734079f30f7fc39...@syzkaller.appspotmail.com [ cut here ] kernel BUG at drivers/dma-buf/dma-buf.c:99! invalid opcode: [#1] PREEMPT SMP KASAN CPU: 1 PID: 18500 Comm: syz-executor.0 Not tainted 5.6.0-rc3-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 RIP: 0010:dma_buf_release+0x35b/0x420 drivers/dma-buf/dma-buf.c:99 Code: 00 00 e8 f8 d3 ec fc 4c 89 e7 45 31 e4 e8 ed 7a 35 fd e8 38 25 f7 fc 44 89 e0 5b 41 5c 41 5d 41 5e 41 5f 5d c3 e8 25 25 f7 fc <0f> 0b e8 1e 25 f7 fc 4c 89 ef e8 46 eb 00 00 eb 97 e8 0f 25 f7 fc RSP: 0018:c9000296fdc8 EFLAGS: 00010293 RAX: 888094086300 RBX: 0004 RCX: 847e6fc5 RDX: RSI: 847e722b RDI: 0005 RBP: c9000296fdf0 R08: 888094086300 R09: R10: R11: R12: 88808c6ec800 R13: 88808e6ecd7c R14: 88808e6ecd28 R15: 888097011940 FS: 02b7b940() GS:8880ae90() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 008a9e80 CR3: 916f CR4: 001406e0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 Call Trace: __fput+0x2ff/0x890 fs/file_table.c:280 fput+0x16/0x20 fs/file_table.c:313 task_work_run+0x145/0x1c0 kernel/task_work.c:113 tracehook_notify_resume include/linux/tracehook.h:188 [inline] exit_to_usermode_loop+0x316/0x380 arch/x86/entry/common.c:164 prepare_exit_to_usermode arch/x86/entry/common.c:195 [inline] syscall_return_slowpath arch/x86/entry/common.c:278 [inline] do_syscall_64+0x676/0x790 arch/x86/entry/common.c:304 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x416011 Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 04 1b 00 00 c3 48 83 ec 08 e8 0a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 53 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01 RSP: 002b:7fff23e39030 EFLAGS: 0293 ORIG_RAX: 0003 RAX: RBX: 0009 RCX: 00416011 RDX: RSI: 0081 RDI: 0008 RBP: R08: R09: 01ff R10: 00770938 R11: 0293 R12: 0076bf20 R13: 00770948 R14: R15: 0076bf2c Modules linked in: ---[ end trace 432bc190b75d ]--- RIP: 0010:dma_buf_release+0x35b/0x420 drivers/dma-buf/dma-buf.c:99 Code: 00 00 e8 f8 d3 ec fc 4c 89 e7 45 31 e4 e8 ed 7a 35 fd e8 38 25 f7 fc 44 89 e0 5b 41 5c 41 5d 41 5e 41 5f 5d c3 e8 25 25 f7 fc <0f> 0b e8 1e 25 f7 fc 4c 89 ef e8 46 eb 00 00 eb 97 e8 0f 25 f7 fc RSP: 0018:c9000296fdc8 EFLAGS: 00010293 RAX: 888094086300 RBX: 0004 RCX: 847e6fc5 RDX: RSI: 847e722b RDI: 0005 RBP: c9000296fdf0 R08: 888094086300 R09: R10: R11: R12: 88808c6ec800 R13: 88808e6ecd7c R14: 88808e6ecd28 R15: 888097011940 FS: 02b7b940() GS:8880ae90() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 008a9e80 CR3: 916f CR4: 001406e0 DR0: DR1: DR2: DR3: DR6: fffe0ff0 DR7: 0400 --- This bug is generated by a bot. It may contain errors. See https://goo.gl/tpsmEJ for more information about syzbot. syzbot engineers can be reached at syzkal...@googlegroups.com. syzbot will keep track of this bug report. See: https://goo.gl/tpsmEJ#status for how to communicate with syzbot. syzbot can test patches for this bug, for details see: https://goo.gl/tpsmEJ#testing-patches ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 0/2] Add support for rm69299 Visionox panel driver and add devicetree bindings for visionox panel
Adding support for visionox rm69299 panel driver and adding bindings for the same panel. Harigovindan P (2): dt-bindings: display: add visionox rm69299 panel variant drm/panel: add support for rm69299 visionox panel driver .../display/panel/visionox,rm69299.yaml | 85 + drivers/gpu/drm/panel/Kconfig | 8 + drivers/gpu/drm/panel/Makefile| 1 + .../gpu/drm/panel/panel-visionox-rm69299.c| 315 ++ 4 files changed, 409 insertions(+) create mode 100755 Documentation/devicetree/bindings/display/panel/visionox,rm69299.yaml create mode 100644 drivers/gpu/drm/panel/panel-visionox-rm69299.c -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 11/13] soc: mediatek: cmdq: add jump function
Add jump function so that client can jump to any address which contains instruction. Signed-off-by: Dennis YC Hsieh --- drivers/soc/mediatek/mtk-cmdq-helper.c | 13 + include/linux/soc/mediatek/mtk-cmdq.h | 11 +++ 2 files changed, 24 insertions(+) diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index 59bc1164b411..bb5be20fc70a 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -13,6 +13,7 @@ #define CMDQ_POLL_ENABLE_MASK BIT(0) #define CMDQ_EOC_IRQ_ENBIT(0) #define CMDQ_REG_TYPE 1 +#define CMDQ_JUMP_RELATIVE 1 struct cmdq_instruction { union { @@ -372,6 +373,18 @@ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value) } EXPORT_SYMBOL(cmdq_pkt_assign); +int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr) +{ + struct cmdq_instruction inst = { {0} }; + + inst.op = CMDQ_CODE_JUMP; + inst.offset = CMDQ_JUMP_RELATIVE; + inst.value = addr >> + cmdq_mbox_shift(((struct cmdq_client *)pkt->cl)->chan); + return cmdq_pkt_append_command(pkt, inst); +} +EXPORT_SYMBOL(cmdq_pkt_jump); + int cmdq_pkt_finalize(struct cmdq_pkt *pkt) { struct cmdq_instruction inst = { {0} }; diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h index 99e77155f967..1a6c56f3bec1 100644 --- a/include/linux/soc/mediatek/mtk-cmdq.h +++ b/include/linux/soc/mediatek/mtk-cmdq.h @@ -213,6 +213,17 @@ int cmdq_pkt_poll_mask(struct cmdq_pkt *pkt, u8 subsys, */ int cmdq_pkt_assign(struct cmdq_pkt *pkt, u16 reg_idx, u32 value); +/** + * cmdq_pkt_jump() - Append jump command to the CMDQ packet, ask GCE + * to execute an instruction that change current thread PC to + * a physical address which should contains more instruction. + * @pkt:the CMDQ packet + * @addr: physical address of target instruction buffer + * + * Return: 0 for success; else the error code is returned + */ +int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr); + /** * cmdq_pkt_finalize() - Append EOC and jump command to pkt. * @pkt: the CMDQ packet -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 1/2] dt-bindings: display: add visionox rm69299 panel variant
Add bindings for visionox rm69299 panel. Signed-off-by: Harigovindan P --- Changes in v2: - Removed unwanted properties from description. - Creating source files without execute permissions(Rob Herring). Changes in v3: - Changing txt file into yaml Changes in v4: - Updating license identifier. - Moving yaml file inside panel directory. - Removing pinctrl entries. - Adding documentation for reset-gpios. Changes in v5: - No changes. Updated 2/2 Patch. .../display/panel/visionox,rm69299.yaml | 85 +++ 1 file changed, 85 insertions(+) create mode 100755 Documentation/devicetree/bindings/display/panel/visionox,rm69299.yaml diff --git a/Documentation/devicetree/bindings/display/panel/visionox,rm69299.yaml b/Documentation/devicetree/bindings/display/panel/visionox,rm69299.yaml new file mode 100755 index ..052d87f8fadd --- /dev/null +++ b/Documentation/devicetree/bindings/display/panel/visionox,rm69299.yaml @@ -0,0 +1,85 @@ +# SPDX-License-Identifier: GPL-2.0-only or BSD-2-Clause +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/panel/visionox,rm69299.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Visionox model RM69299 Panels Device Tree Bindings. + +maintainers: + - Harigovindan P + +description: | + This binding is for display panels using a Visionox RM692999 panel. + +allOf: + - $ref: panel-common.yaml# + +patternProperties: + "^(panel|panel-dsi)@[0-9]$": +type: object +properties: + compatible: +const: visionox,rm69299-1080p-display + + reg: +maxItems: 1 + + vdda-supply: +description: + Phandle of the regulator that provides the vdda supply voltage. + + vdd3p3-supply: +description: + Phandle of the regulator that provides the vdd3p3 supply voltage. + + ports: +type: object +description: + A node containing DSI input & output port nodes with endpoint + definitions as documented in + Documentation/devicetree/bindings/media/video-interfaces.txt + Documentation/devicetree/bindings/graph.txt +properties: + port@0: +type: object +description: + DSI input port node. + + reset-gpios: +description: + a GPIO spec for the reset pin. + +required: + - compatible + - reg + - vdda-supply + - vdd3p3-supply + - reset-gpios + +additionalProperties: false + +examples: +- | +dsi@ae94000 { +panel@0 { +compatible = "visionox,rm69299-1080p-display"; + +vdda-supply = <_pp1800_l8c>; +vdd3p3-supply = <_pp2800_l18a>; + +reset-gpios = <_gpio 3 0>; +ports { +#address-cells = <1>; +#size-cells = <0>; +port@0 { +reg = <0>; +panel0_in: endpoint { +remote-endpoint = <_out>; +}; +}; +}; +}; +}; +... + -- 2.25.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RESEND PATCH v2 0/3] Add support for Chrontel CH7033 VGA/DVI Encoder
Hi, chained to this message is a driver for CH7033 along with device tree binding docs. I'm hoping that it could perhaps make it into 5.7. Please take a look. Previous submission [1] contained the exact same patches as this one, but at that time they relied on Laurent's omapdrm/bridge/devel branch that has been merged since. [1] https://lore.kernel.org/lkml/20200221162743.14141-1-lkund...@v3.sk/ Tested to work well on MMP3-based Dell Wyse 3020. There's no datasheet or programming manual available. Thanks, Lubo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 0/3] IT6505 cover letter
The IT6505 is a high-performance DisplayPort 1.1a transmitter, fully compliant with DisplayPort 1.1a, HDCP 1.3 specifications. The IT6505 supports color depth of up to 36 bits (12 bits/color) and ensures robust transmission of high-quality uncompressed video content, along with uncompressed and compressed digital audio content. This series contains document bindings, revert commit, add vendor prefix, Kconfig to control the function enable or not. Allen Chen (1): WIP: drm/bridge: add it6505 driver allen (2): dt-bindings: Add vendor prefix for ITE Tech. Inc. WIP: dt-bindings: Add binding for IT6505. .../bindings/display/bridge/ite,it6505.yaml| 96 + .../devicetree/bindings/vendor-prefixes.yaml |2 +- drivers/gpu/drm/bridge/Kconfig | 11 +- drivers/gpu/drm/bridge/Makefile|6 +- drivers/gpu/drm/bridge/ite-it6505.c| 3022 5 files changed, 3132 insertions(+), 5 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/ite,it6505.yaml create mode 100644 drivers/gpu/drm/bridge/ite-it6505.c -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 04/13] mailbox: mediatek: cmdq: clear task in channel before shutdown
Do success callback in channel when shutdown. For those task not finish, callback with error code thus client has chance to cleanup or reset. Signed-off-by: Dennis YC Hsieh Reviewed-by: CK Hu --- drivers/mailbox/mtk-cmdq-mailbox.c | 38 ++ 1 file changed, 38 insertions(+) diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c index 9994ac9426d6..b56d340c8982 100644 --- a/drivers/mailbox/mtk-cmdq-mailbox.c +++ b/drivers/mailbox/mtk-cmdq-mailbox.c @@ -387,6 +387,12 @@ static int cmdq_mbox_send_data(struct mbox_chan *chan, void *data) if (list_empty(>task_busy_list)) { WARN_ON(clk_enable(cmdq->clock) < 0); + /* +* The thread reset will clear thread related register to 0, +* including pc, end, priority, irq, suspend and enable. Thus +* set CMDQ_THR_ENABLED to CMDQ_THR_ENABLE_TASK will enable +* thread and make it running. +*/ WARN_ON(cmdq_thread_reset(cmdq, thread) < 0); writel(task->pa_base >> cmdq->shift_pa, @@ -450,6 +456,38 @@ static int cmdq_mbox_startup(struct mbox_chan *chan) static void cmdq_mbox_shutdown(struct mbox_chan *chan) { + struct cmdq_thread *thread = (struct cmdq_thread *)chan->con_priv; + struct cmdq *cmdq = dev_get_drvdata(chan->mbox->dev); + struct cmdq_task *task, *tmp; + unsigned long flags; + + spin_lock_irqsave(>chan->lock, flags); + if (list_empty(>task_busy_list)) + goto done; + + WARN_ON(cmdq_thread_suspend(cmdq, thread) < 0); + + /* make sure executed tasks have success callback */ + cmdq_thread_irq_handler(cmdq, thread); + if (list_empty(>task_busy_list)) + goto done; + + list_for_each_entry_safe(task, tmp, >task_busy_list, +list_entry) { + cmdq_task_exec_done(task, CMDQ_CB_ERROR); + kfree(task); + } + + cmdq_thread_disable(cmdq, thread); + clk_disable(cmdq->clock); +done: + /* +* The thread->task_busy_list empty means thread already disable. The +* cmdq_mbox_send_data() always reset thread which clear disable and +* suspend statue when first pkt send to channel, so there is no need +* to do any operation here, only unlock and leave. +*/ + spin_unlock_irqrestore(>chan->lock, flags); } static const struct mbox_chan_ops cmdq_mbox_chan_ops = { -- 2.18.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel