VC4 fixes for -next
Subsystems (3)
Merging vc4 to drm-next got a few automatic sparse and coccinelle warning reports generated. The first 3 patches come from the maintainers of those systems (thanks!), and the last 3 are fixes I've come up with in the last few days.
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 1/6 drm/vc4: vc4_plane_duplicate_state() can be static
From: kbuild test robot <[email protected]>
--- drivers/gpu/drm/vc4/vc4_plane.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/gpu/drm/vc4/vc4_plane.c +4 -4
@@ -70,7 +70,7 @@ static bool plane_enabled(struct drm_plane_state *state)return state->fb && state->crtc; } -struct drm_plane_state *vc4_plane_duplicate_state (struct drm_plane *plane) +static struct drm_plane_state *vc4_plane_duplicate_state (struct drm_plane *plane) {struct vc4_plane_state *vc4_state;
@@ -97,8 +97,8 @@ struct drm_plane_state *vc4_plane_duplicate_state(struct drm_plane *plane)return &vc4_state->base; } -void vc4_plane_destroy_state (struct drm_plane *plane, -struct drm_plane_state *state) +static void vc4_plane_destroy_state (struct drm_plane *plane, +struct drm_plane_state *state) {struct vc4_plane_state *vc4_state =to_vc4_plane_state (state);
@@ -108,7 +108,7 @@ void vc4_plane_destroy_state(struct drm_plane *plane, }/* Called during init to allocate the plane's atomic state. */ -void vc4_plane_reset (struct drm_plane *plane) +static void vc4_plane_reset (struct drm_plane *plane) {struct vc4_plane_state *vc4_state;
Signature · 6 lines
2.6.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 2/6 drm/vc4: fix platform_no_drv_owner.cocci warnings
In reply to Eric AnholtFrom: Julia Lawall <[email protected]>
drivers/gpu/drm/vc4/vc4_drv.c:248:3-8: No need to set .owner here. The core will do it.
Remove .owner field if calls are used which set it automatically
Generated by: scripts/coccinelle/api/platform_no_drv_owner.cocci
--- drivers/gpu/drm/vc4/vc4_drv.c | 1 - 1 file changed, 1 deletion(-)
drivers/gpu/drm/vc4/vc4_drv.c +0 -1
@@ -259,7 +259,6 @@ static struct platform_driver vc4_platform_driver = { .remove = vc4_platform_drm_remove, .driver = { .name = "vc4-drm", - .owner = THIS_MODULE, .of_match_table = vc4_of_match, }, };
Signature · 6 lines
2.6.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 3/6 drm/vc4: fix itnull.cocci warnings
In reply to Eric AnholtFrom: Julia Lawall <[email protected]>
Connector cannot be null because it is a list entry, ie accessed at an offset from the positions of the list structure pointers themselves.
Generated by: scripts/coccinelle/iterators/itnull.cocci
--- drivers/gpu/drm/vc4/vc4_crtc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/gpu/drm/vc4/vc4_crtc.c +1 -1
@@ -168,7 +168,7 @@ static int vc4_get_clock_select(struct drm_crtc *crtc)struct drm_connector *connector;drm_for_each_connector (connector, crtc->dev) { -if (connector && connector->state->crtc == crtc) { +if (connector->state->crtc == crtc) {struct drm_encoder *encoder = connector->encoder;struct vc4_encoder *vc4_encoder =to_vc4_encoder (encoder);
Signature · 1 line
2.6.1
PATCH 4/6 drm/vc4: Fix some failure to track __iomem decorations on pointers.
In reply to Eric AnholtCaught by the kbuild test robot.
--- drivers/gpu/drm/vc4/vc4_crtc.c | 3 ++- drivers/gpu/drm/vc4/vc4_hvs.c | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-)
drivers/gpu/drm/vc4/vc4_crtc.c +2 -1
@@ -401,7 +401,8 @@ static void vc4_crtc_atomic_flush(struct drm_crtc *crtc, dlist_next++;HVS_WRITE (SCALER_DISPLISTX (vc4_crtc->channel), - (u32 *)vc4_crtc->dlist - (u32 *)vc4->hvs->dlist); + (u32 __iomem *)vc4_crtc->dlist - + (u32 __iomem *)vc4->hvs->dlist);/* Make the next display list start after ours. */ vc4_crtc->dlist_size -= (dlist_next - vc4_crtc->dlist);
drivers/gpu/drm/vc4/vc4_hvs.c +4 -4
@@ -75,10 +75,10 @@ void vc4_hvs_dump_state(struct drm_device *dev)for (i =0 ; i <64 ; i +=4 ) {DRM_INFO ("0x%08x (%s): 0x%08x 0x%08x 0x%08x 0x%08x\n" , i *4 , i <HVS_BOOTLOADER_DLIST_END ?"B" :"D" , - ((uint32_t *)vc4->hvs->dlist)[i +0 ], - ((uint32_t *)vc4->hvs->dlist)[i +1 ], - ((uint32_t *)vc4->hvs->dlist)[i +2 ], - ((uint32_t *)vc4->hvs->dlist)[i +3 ]); +readl ((u32 __iomem *)vc4->hvs->dlist +0 ), +readl ((u32 __iomem *)vc4->hvs->dlist +1 ), +readl ((u32 __iomem *)vc4->hvs->dlist +2 ), +readl ((u32 __iomem *)vc4->hvs->dlist +3 )); } }
Signature · 6 lines
2.6.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/6] drm/vc4: Fix some failure to track __iomem decorations on pointers.
Review
Quoted 10 lines · Eric Anholt
Caught by the kbuild test robot.
Signed-off-by Eric Anholt <[email protected]>--- drivers/gpu/drm/vc4/vc4_crtc.c | 3 ++- drivers/gpu/drm/vc4/vc4_hvs.c | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/gpu/drm/vc4/vc4_crtc.c
drivers/gpu/drm/vc4/vc4_crtc.c static void vc4_crtc_atomic_flush(struct drm_crtc
Quoted 4 lines
index a3a77dd..32c0348 100644
drivers/gpu/drm/vc4/vc4_crtc.c +0
-0@@ -401,7 +401,8 @@ static void vc4_crtc_atomic_flush(struct drm_crtc
*crtc,
drivers/gpu/drm/vc4/vc4_crtc.c static void vc4_crtc_atomic_flush(struct drm_crtc *crtc,
Quoted 4 lines
drivers/gpu/drm/vc4/vc4_crtc.c +0
-1dlist_next++; HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel), - (u32 *)vc4_crtc->dlist - (u32
*)vc4->hvs->dlist);
Quoted 11 lines
drivers/gpu/drm/vc4/vc4_crtc.c +2
-0+ (u32 __iomem *)vc4_crtc->dlist - + (u32 __iomem *)vc4->hvs->dlist);/* Make the next display list start after ours. */ vc4_crtc->dlist_size -= (dlist_next - vc4_crtc->dlist);drivers/gpu/drm/vc4/vc4_hvs.c +0
-0@@ -75,10 +75,10 @@ void vc4_hvs_dump_state(struct drm_device *dev)for (i =0 ; i <64 ; i +=4 ) {
drivers/gpu/drm/vc4/vc4_hvs.c void vc4_hvs_dump_state(struct drm_device *dev) · line 81
Quoted 10 lines
drivers/gpu/drm/vc4/vc4_hvs.c +4
-4DRM_INFO ("0x%08x (%s): 0x%08x 0x%08x 0x%08x 0x%08x\n" , i *4 , i <HVS_BOOTLOADER_DLIST_END ?"B" :"D" , - ((uint32_t *)vc4->hvs->dlist)[i +0 ], - ((uint32_t *)vc4->hvs->dlist)[i +1 ], - ((uint32_t *)vc4->hvs->dlist)[i +2 ], - ((uint32_t *)vc4->hvs->dlist)[i +3 ]); +readl ((u32 __iomem *)vc4->hvs->dlist +0 ), +readl ((u32 __iomem *)vc4->hvs->dlist +1 ), +readl ((u32 __iomem *)vc4->hvs->dlist +2 ), +readl ((u32 __iomem *)vc4->hvs->dlist +3 ));
Looks like you lost the +i here, no?
Quoted 10 lines
drivers/gpu/drm/vc4/vc4_hvs.c +0
-1} } --2.6.1
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/6] drm/vc4: Fix some failure to track __iomem decorations on pointers.
Quoted 37 lines · Ilia Mirkin
Review
Quoted 10 lines · Eric Anholt
Caught by the kbuild test robot.
Signed-off-by Eric Anholt <[email protected]>--- drivers/gpu/drm/vc4/vc4_crtc.c | 3 ++- drivers/gpu/drm/vc4/vc4_hvs.c | 8 ++++---- 2 files changed, 6 insertions(+), 5 deletions(-)(quoted diff) +0
-0
b/drivers/gpu/drm/vc4/vc4_crtc.cdrivers/gpu/drm/vc4/vc4_crtc.c static void vc4_crtc_atomic_flush(struct drm_crtc
Quoted 4 lines
index a3a77dd..32c0348 100644
drivers/gpu/drm/vc4/vc4_crtc.c +0
-0@@ -401,7 +401,8 @@ static void vc4_crtc_atomic_flush(struct drm_crtc*crtc,
drivers/gpu/drm/vc4/vc4_crtc.c static void vc4_crtc_atomic_flush(struct drm_crtc *crtc,
Quoted 4 lines
drivers/gpu/drm/vc4/vc4_crtc.c +0
-1dlist_next++; HVS_WRITE(SCALER_DISPLISTX(vc4_crtc->channel), - (u32 *)vc4_crtc->dlist - (u32*)vc4->hvs->dlist);
Quoted 15 lines
drivers/gpu/drm/vc4/vc4_crtc.c +2
-0+ (u32 __iomem *)vc4_crtc->dlist - + (u32 __iomem *)vc4->hvs->dlist);/* Make the next display list start after ours. */ vc4_crtc->dlist_size -= (dlist_next - vc4_crtc->dlist);drivers/gpu/drm/vc4/vc4_hvs.c +0
-2@@ -75,10 +75,10 @@ void vc4_hvs_dump_state(struct drm_device *dev)for (i =0 ; i <64 ; i +=4 ) { DRM_INFO("0x%08x (%s): 0x%08x 0x%08x 0x%08x 0x%08x\n", i * 4, i < HVS_BOOTLOADER_DLIST_END ? "B" : "D", - ((uint32_t *)vc4->hvs->dlist)[i + 0], - ((uint32_t *)vc4->hvs->dlist)[i + 1],
Quoted 8 lines
drivers/gpu/drm/vc4/vc4_hvs.c void vc4_hvs_dump_state(struct drm_device *dev) · line 81
Quoted 6 lines
drivers/gpu/drm/vc4/vc4_hvs.c +4
-2- ((uint32_t *)vc4->hvs->dlist)[i +2 ], - ((uint32_t *)vc4->hvs->dlist)[i + 3]); +readl ((u32 __iomem *)vc4->hvs->dlist +0 ), +readl ((u32 __iomem *)vc4->hvs->dlist +1 ), +readl ((u32 __iomem *)vc4->hvs->dlist +2 ), + readl((u32 __iomem *)vc4->hvs->dlist + 3));Looks like you lost the +i here, no?
Indeed. Thanks!
PATCH 5/6 drm/vc4: Make sure that planes aren't scaled.
In reply to Eric AnholtWe would scan out the memory around them if an upscale was attempted, and would just scan out incorrectly for downscaling.
---
It looks like, while modetest only wants to set scaling on overlay planes, one could do so on our primary/cursor planes if the fd was in universal planes mode.
drivers/gpu/drm/vc4/vc4_plane.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
drivers/gpu/drm/vc4/vc4_plane.c +10 -0
@@ -157,6 +157,16 @@ static int vc4_plane_mode_set(struct drm_plane *plane,int crtc_w = state->crtc_w;int crtc_h = state->crtc_h; +if (state->crtc_w <<16 != state->src_w || + state->crtc_h <<16 != state->src_h) { +/* We don't support scaling yet, which involves +* allocating the LBM memory for scaling temporary +* storage, and putting filter kernels in the HVS +* context. +*/ +return -EINVAL ; + } +if (crtc_x <0 ) { offset +=drm_format_plane_cpp (fb->pixel_format,0 ) * -crtc_x; crtc_w += crtc_x;
Signature · 6 lines
2.6.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/6] drm/vc4: Make sure that planes aren't scaled.
Quoted 20 lines · Eric Anholt
We would scan out the memory around them if an upscale was attempted, and would just scan out incorrectly for downscaling.
Signed-off-by Eric Anholt <[email protected]>---
It looks like, while modetest only wants to set scaling on overlay planes, one could do so on our primary/cursor planes if the fd was in universal planes mode.
drivers/gpu/drm/vc4/vc4_plane.c | 10 ++++++++++ 1 file changed, 10 insertions(+)drivers/gpu/drm/vc4/vc4_plane.c +0
-0@@ -157,6 +157,16 @@ static int vc4_plane_mode_set(struct drm_plane *plane,int crtc_w = state->crtc_w;int crtc_h = state->crtc_h;
drivers/gpu/drm/vc4/vc4_plane.c static int vc4_plane_mode_set(struct drm_plane *plane, · line 168
Quoted 10 lines
drivers/gpu/drm/vc4/vc4_plane.c +9
-0+if (state->crtc_w <<16 != state->src_w || + state->crtc_h <<16 != state->src_h) { +/* We don't support scaling yet, which involves +* allocating the LBM memory for scaling temporary +* storage, and putting filter kernels in the HVS +* context. +*/ +return -EINVAL ; + }
Just aside: There's drm_plane_helper_check_update for the common cases. Unfortunately it suffers a bit from a bikeshed since it wants struct drm_rect but atomic states don't give you that.
-Daniel
Quoted 11 lines
drivers/gpu/drm/vc4/vc4_plane.c +1
-0+if (crtc_x <0 ) { offset +=drm_format_plane_cpp (fb->pixel_format,0 ) * -crtc_x; crtc_w += crtc_x;Signature · 6 lines
2.6.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 6/6 drm/vc4: Add support for more a few more RGB display plane formats.
In reply to Eric AnholtThese were all touch-tested with modetest.
--- drivers/gpu/drm/vc4/vc4_plane.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
drivers/gpu/drm/vc4/vc4_plane.c +16 -0
@@ -51,6 +51,22 @@ static const struct hvs_format { .drm = DRM_FORMAT_ARGB8888, .hvs = HVS_PIXEL_FORMAT_RGBA8888, .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = true, }, + { + .drm = DRM_FORMAT_RGB565, .hvs = HVS_PIXEL_FORMAT_RGB565, + .pixel_order = HVS_PIXEL_ORDER_XRGB, .has_alpha = false, + }, + { + .drm = DRM_FORMAT_BGR565, .hvs = HVS_PIXEL_FORMAT_RGB565, + .pixel_order = HVS_PIXEL_ORDER_XBGR, .has_alpha = false, + }, + { + .drm = DRM_FORMAT_ARGB1555, .hvs = HVS_PIXEL_FORMAT_RGBA5551, + .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = true, + }, + { + .drm = DRM_FORMAT_XRGB1555, .hvs = HVS_PIXEL_FORMAT_RGBA5551, + .pixel_order = HVS_PIXEL_ORDER_ABGR, .has_alpha = false, + }, }; static const struct hvs_format *vc4_get_hvs_format(u32 drm_format)
Signature · 6 lines
2.6.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel