* Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @ 2019-06-13 20:13 Nathan Huckleberry 2019-06-13 20:49 ` Sean Paul ` (2 more replies) 0 siblings, 3 replies; 8+ messages in thread From: Nathan Huckleberry @ 2019-06-13 20:13 UTC (permalink / raw) To: jsanka, robdclark, sean, airlied, daniel Cc: linux-arm-msm, dri-devel, freedreno, clang-built-linux Hey all, I'm looking into cleaning up ignored warnings in the kernel so we can remove compiler flags to ignore warnings. There are several unused variables in dpu_formats.c ('dpu_format_map_tile', 'dpu_format_map_p010', 'dpu_format_map_p010_ubwc', 'dpu_format_map_tp10_ubwc'). They look like modifiers that were never implemented. I'd like to remove these variables if there are no plans moving forward to implement them. Otherwise I'll just leave them. https://github.com/ClangBuiltLinux/linux/issues/528 Thanks, Nathan Huckleberry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c 2019-06-13 20:13 Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c Nathan Huckleberry @ 2019-06-13 20:49 ` Sean Paul 2019-06-14 18:55 ` Nick Desaulniers 2019-06-13 20:52 ` Rob Clark 2019-06-14 9:46 ` Jani Nikula 2 siblings, 1 reply; 8+ messages in thread From: Sean Paul @ 2019-06-13 20:49 UTC (permalink / raw) To: Nathan Huckleberry Cc: Jeykumar Sankaran, Rob Clark, Dave Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno, clang-built-linux On Thu, Jun 13, 2019 at 4:13 PM Nathan Huckleberry <nhuck@google.com> wrote: > > Hey all, > > I'm looking into cleaning up ignored warnings in the kernel so we can > remove compiler flags to ignore warnings. > > There are several unused variables in dpu_formats.c > ('dpu_format_map_tile', 'dpu_format_map_p010', > 'dpu_format_map_p010_ubwc', 'dpu_format_map_tp10_ubwc'). > They look like modifiers that were never implemented. I'd like to > remove these variables if there are no plans moving forward to > implement them. Otherwise I'll just leave them. We can probably remove them for now and if someone wants to add support, they can dredge them back up. Sean > > https://github.com/ClangBuiltLinux/linux/issues/528 > > Thanks, > Nathan Huckleberry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c 2019-06-13 20:49 ` Sean Paul @ 2019-06-14 18:55 ` Nick Desaulniers 0 siblings, 0 replies; 8+ messages in thread From: Nick Desaulniers @ 2019-06-14 18:55 UTC (permalink / raw) To: Sean Paul Cc: Nathan Huckleberry, Jeykumar Sankaran, Rob Clark, Dave Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno, clang-built-linux On Thu, Jun 13, 2019 at 1:50 PM Sean Paul <sean@poorly.run> wrote: > > On Thu, Jun 13, 2019 at 4:13 PM Nathan Huckleberry <nhuck@google.com> wrote: > > > > Hey all, > > > > I'm looking into cleaning up ignored warnings in the kernel so we can > > remove compiler flags to ignore warnings. > > > > There are several unused variables in dpu_formats.c > > ('dpu_format_map_tile', 'dpu_format_map_p010', > > 'dpu_format_map_p010_ubwc', 'dpu_format_map_tp10_ubwc'). > > They look like modifiers that were never implemented. I'd like to > > remove these variables if there are no plans moving forward to > > implement them. Otherwise I'll just leave them. > > We can probably remove them for now and if someone wants to add > support, they can dredge them back up. Yep, this has been the feedback for other patches for this warning when the code was dead or not obviously some kind of bug/typo/copy-pasta. Nathan, please submit a patch removing the dead code; it may be reverted later when it's actually wired up. Nothing is truly lost w/ git*. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c 2019-06-13 20:13 Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c Nathan Huckleberry 2019-06-13 20:49 ` Sean Paul @ 2019-06-13 20:52 ` Rob Clark 2019-06-14 7:17 ` Daniel Vetter 2019-06-14 9:46 ` Jani Nikula 2 siblings, 1 reply; 8+ messages in thread From: Rob Clark @ 2019-06-13 20:52 UTC (permalink / raw) To: Nathan Huckleberry Cc: Jeykumar Sankaran, Sean Paul, David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno, clang-built-linux so, for dpu_format_map_tile, I'd like to define a fourcc modifier for tiled formats (we currently have a workaround in userspace w/ a private modifier in the gallium driver).. I think the problem is defining the layout of the tiled format(s) (there are at least two per generation and I can't guarantee they are the same across adreno generations). We've mostly avoided needing to know the exact layout by using gpu blits to go from tiled<->linear so far. For the others, those look like formats we haven't wired up yet. I'd say they are all things we want to support eventually, although not sure what the timeline will be.. but I'd ask if you remove them then split into at least a separate patch for dpu_format_map_tile vs others, so we can more easily revert/amend to bring them back. BR, -R On Thu, Jun 13, 2019 at 1:13 PM Nathan Huckleberry <nhuck@google.com> wrote: > > Hey all, > > I'm looking into cleaning up ignored warnings in the kernel so we can > remove compiler flags to ignore warnings. > > There are several unused variables in dpu_formats.c > ('dpu_format_map_tile', 'dpu_format_map_p010', > 'dpu_format_map_p010_ubwc', 'dpu_format_map_tp10_ubwc'). > They look like modifiers that were never implemented. I'd like to > remove these variables if there are no plans moving forward to > implement them. Otherwise I'll just leave them. > > https://github.com/ClangBuiltLinux/linux/issues/528 > > Thanks, > Nathan Huckleberry ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c 2019-06-13 20:52 ` Rob Clark @ 2019-06-14 7:17 ` Daniel Vetter 0 siblings, 0 replies; 8+ messages in thread From: Daniel Vetter @ 2019-06-14 7:17 UTC (permalink / raw) To: Rob Clark Cc: Nathan Huckleberry, Jeykumar Sankaran, Sean Paul, David Airlie, Daniel Vetter, linux-arm-msm, dri-devel, freedreno, clang-built-linux On Thu, Jun 13, 2019 at 01:52:02PM -0700, Rob Clark wrote: > so, for dpu_format_map_tile, I'd like to define a fourcc modifier for > tiled formats (we currently have a workaround in userspace w/ a > private modifier in the gallium driver).. I think the problem is > defining the layout of the tiled format(s) (there are at least two per > generation and I can't guarantee they are the same across adreno > generations). We've mostly avoided needing to know the exact layout > by using gpu blits to go from tiled<->linear so far. > > For the others, those look like formats we haven't wired up yet. > > I'd say they are all things we want to support eventually, although > not sure what the timeline will be.. but I'd ask if you remove them > then split into at least a separate patch for dpu_format_map_tile vs > others, so we can more easily revert/amend to bring them back. We've been kinda cheating on this with i915 modifiers too, X/Y tiled depend upon the chip you're running on :-) But on all modern chips it's a lot more well-defined, so probably not a huge problem. I think there's little chance right now for adreno to become a discrete gpu, so could do the same tricky. If it ever becomes discrete or we want to share more, then we'd need to bake in the layout properly I think. I guess the question is: Is the format shared with e.g. camera blocks, or purely a freedreno internal thing. -Daniel > > BR, > -R > > On Thu, Jun 13, 2019 at 1:13 PM Nathan Huckleberry <nhuck@google.com> wrote: > > > > Hey all, > > > > I'm looking into cleaning up ignored warnings in the kernel so we can > > remove compiler flags to ignore warnings. > > > > There are several unused variables in dpu_formats.c > > ('dpu_format_map_tile', 'dpu_format_map_p010', > > 'dpu_format_map_p010_ubwc', 'dpu_format_map_tp10_ubwc'). > > They look like modifiers that were never implemented. I'd like to > > remove these variables if there are no plans moving forward to > > implement them. Otherwise I'll just leave them. > > > > https://github.com/ClangBuiltLinux/linux/issues/528 > > > > Thanks, > > Nathan Huckleberry -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c 2019-06-13 20:13 Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c Nathan Huckleberry 2019-06-13 20:49 ` Sean Paul 2019-06-13 20:52 ` Rob Clark @ 2019-06-14 9:46 ` Jani Nikula 2019-06-14 18:59 ` Nick Desaulniers 2 siblings, 1 reply; 8+ messages in thread From: Jani Nikula @ 2019-06-14 9:46 UTC (permalink / raw) To: Nathan Huckleberry, jsanka, robdclark, sean, airlied, daniel Cc: linux-arm-msm, freedreno, dri-devel, clang-built-linux On Thu, 13 Jun 2019, Nathan Huckleberry <nhuck@google.com> wrote: > Hey all, > > I'm looking into cleaning up ignored warnings in the kernel so we can > remove compiler flags to ignore warnings. Wholeheartedly agreed on the goal. > There are several unused variables in dpu_formats.c > ('dpu_format_map_tile', 'dpu_format_map_p010', > 'dpu_format_map_p010_ubwc', 'dpu_format_map_tp10_ubwc'). > They look like modifiers that were never implemented. I'd like to > remove these variables if there are no plans moving forward to > implement them. Otherwise I'll just leave them. > > https://github.com/ClangBuiltLinux/linux/issues/528 No opinion on the said variables above, but, FWIW, personally I think it's fine to add the cflags to supress warnings locally where needed in order to be able to achieve the greater goal of removing the cflags globally. In drivers/gpu/drm/i915/Makefile we actually go for much stricter warnings than the kernel defaults, and disable a more limited and tailored set of warnings. You can do this both on a subdir and file level with subdir-ccflags-y and CFLAGS_filename.o, respectively. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c 2019-06-14 9:46 ` Jani Nikula @ 2019-06-14 18:59 ` Nick Desaulniers 2019-06-14 20:56 ` [PATCH] drm/msm/dpu: Fix Wunused-const-variable Nathan Huckleberry 0 siblings, 1 reply; 8+ messages in thread From: Nick Desaulniers @ 2019-06-14 18:59 UTC (permalink / raw) To: Jani Nikula Cc: Nathan Huckleberry, jsanka, robdclark, sean, David Airlie, Daniel Vetter, linux-arm-msm, freedreno, dri-devel, clang-built-linux On Fri, Jun 14, 2019 at 2:43 AM Jani Nikula <jani.nikula@linux.intel.com> wrote: > No opinion on the said variables above, but, FWIW, personally I think > it's fine to add the cflags to supress warnings locally where needed in > order to be able to achieve the greater goal of removing the cflags > globally. I think there's on the order of ~10 of these: https://github.com/ClangBuiltLinux/linux/issues?q=is%3Aissue+is%3Aopen+label%3A-Wunused-const-variable Nathan's got a pretty good handle on just fixing them. > In drivers/gpu/drm/i915/Makefile we actually go for much stricter > warnings than the kernel defaults, and disable a more limited and > tailored set of warnings. I like this. > > You can do this both on a subdir and file level with subdir-ccflags-y > and CFLAGS_filename.o, respectively. That said, I have used this trick before, but I feel like the fewer people that know about this trick, the less it can be [ab]used. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] drm/msm/dpu: Fix Wunused-const-variable 2019-06-14 18:59 ` Nick Desaulniers @ 2019-06-14 20:56 ` Nathan Huckleberry 0 siblings, 0 replies; 8+ messages in thread From: Nathan Huckleberry @ 2019-06-14 20:56 UTC (permalink / raw) To: robdclark, sean, airlied, daniel Cc: linux-arm-msm, dri-devel, freedreno, linux-kernel, Nathan Huckleberry, clang-built-linux Clang produces the following warning drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:477:32: warning: unused variable 'dpu_format_map_tile' [-Wunused-const-variable] static const struct dpu_format dpu_format_map_tile[] = { ^ drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:602:32: warning: unused variable 'dpu_format_map_p010' [-Wunused-const-variable] static const struct dpu_format dpu_format_map_p010[] = { ^ drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:610:32: warning: unused variable 'dpu_format_map_p010_ubwc' [-Wunused-const-variable] static const struct dpu_format dpu_format_map_p010_ubwc[] = { ^ drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c:619:32: warning: unused variable 'dpu_format_map_tp10_ubwc' [-Wunused-const-variable] static const struct dpu_format dpu_format_map_tp10_ubwc[] = { ^ Removing the unimplemented modifiers that cause the warning. Cc: clang-built-linux@googlegroups.com Link: https://github.com/ClangBuiltLinux/linux/issues/528 Signed-off-by: Nathan Huckleberry <nhuck@google.com> --- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 110 -------------------- 1 file changed, 110 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index 0440696b5bad..d28520faf157 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -470,90 +470,6 @@ static const struct dpu_format dpu_format_map[] = { DPU_FETCH_LINEAR, 3), }; -/* - * A5x tile formats tables: - * These tables hold the A5x tile formats supported. - */ -static const struct dpu_format dpu_format_map_tile[] = { - INTERLEAVED_RGB_FMT_TILED(BGR565, - 0, COLOR_5BIT, COLOR_6BIT, COLOR_5BIT, - C2_R_Cr, C0_G_Y, C1_B_Cb, 0, 3, - false, 2, 0, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(ARGB8888, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C3_ALPHA, C2_R_Cr, C0_G_Y, C1_B_Cb, 4, - true, 4, 0, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(ABGR8888, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C3_ALPHA, C1_B_Cb, C0_G_Y, C2_R_Cr, 4, - true, 4, 0, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(XBGR8888, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4, - false, 4, 0, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(RGBA8888, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4, - true, 4, 0, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(BGRA8888, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C1_B_Cb, C0_G_Y, C2_R_Cr, C3_ALPHA, 4, - true, 4, 0, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(BGRX8888, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C1_B_Cb, C0_G_Y, C2_R_Cr, C3_ALPHA, 4, - false, 4, 0, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(XRGB8888, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C3_ALPHA, C2_R_Cr, C0_G_Y, C1_B_Cb, 4, - false, 4, 0, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(RGBX8888, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4, - false, 4, 0, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(ABGR2101010, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4, - true, 4, DPU_FORMAT_FLAG_DX, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - INTERLEAVED_RGB_FMT_TILED(XBGR2101010, - COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C2_R_Cr, C0_G_Y, C1_B_Cb, C3_ALPHA, 4, - true, 4, DPU_FORMAT_FLAG_DX, - DPU_FETCH_UBWC, 1, DPU_TILE_HEIGHT_TILED), - - PSEUDO_YUV_FMT_TILED(NV12, - 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C1_B_Cb, C2_R_Cr, - DPU_CHROMA_420, DPU_FORMAT_FLAG_YUV, - DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_NV12), - - PSEUDO_YUV_FMT_TILED(NV21, - 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C2_R_Cr, C1_B_Cb, - DPU_CHROMA_420, DPU_FORMAT_FLAG_YUV, - DPU_FETCH_UBWC, 2, DPU_TILE_HEIGHT_NV12), -}; - /* * UBWC formats table: * This table holds the UBWC formats supported. @@ -599,32 +515,6 @@ static const struct dpu_format dpu_format_map_ubwc[] = { DPU_FETCH_UBWC, 4, DPU_TILE_HEIGHT_NV12), }; -static const struct dpu_format dpu_format_map_p010[] = { - PSEUDO_YUV_FMT_LOOSE(NV12, - 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C1_B_Cb, C2_R_Cr, - DPU_CHROMA_420, (DPU_FORMAT_FLAG_YUV | DPU_FORMAT_FLAG_DX), - DPU_FETCH_LINEAR, 2), -}; - -static const struct dpu_format dpu_format_map_p010_ubwc[] = { - PSEUDO_YUV_FMT_LOOSE_TILED(NV12, - 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C1_B_Cb, C2_R_Cr, - DPU_CHROMA_420, (DPU_FORMAT_FLAG_YUV | DPU_FORMAT_FLAG_DX | - DPU_FORMAT_FLAG_COMPRESSED), - DPU_FETCH_UBWC, 4, DPU_TILE_HEIGHT_NV12), -}; - -static const struct dpu_format dpu_format_map_tp10_ubwc[] = { - PSEUDO_YUV_FMT_TILED(NV12, - 0, COLOR_8BIT, COLOR_8BIT, COLOR_8BIT, - C1_B_Cb, C2_R_Cr, - DPU_CHROMA_420, (DPU_FORMAT_FLAG_YUV | DPU_FORMAT_FLAG_DX | - DPU_FORMAT_FLAG_COMPRESSED), - DPU_FETCH_UBWC, 4, DPU_TILE_HEIGHT_NV12), -}; - /* _dpu_get_v_h_subsample_rate - Get subsample rates for all formats we support * Note: Not using the drm_format_*_subsampling since we have formats */ -- 2.22.0.410.gd8fdbe21b5-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-14 20:56 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-13 20:13 Cleanup of -Wunused-const-variable in drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c Nathan Huckleberry 2019-06-13 20:49 ` Sean Paul 2019-06-14 18:55 ` Nick Desaulniers 2019-06-13 20:52 ` Rob Clark 2019-06-14 7:17 ` Daniel Vetter 2019-06-14 9:46 ` Jani Nikula 2019-06-14 18:59 ` Nick Desaulniers 2019-06-14 20:56 ` [PATCH] drm/msm/dpu: Fix Wunused-const-variable Nathan Huckleberry
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).