diff mbox series

[v2,5/7] drm/panic: Convert to drm_fb_clip_offset()

Message ID 3121082eb4beb461773ebb6f656ed9b4286967ee.1718305355.git.geert+renesas@glider.be (mailing list archive)
State Awaiting Upstream
Headers show
Series drm/panic: Fixes and graphical logo | expand

Commit Message

Geert Uytterhoeven June 13, 2024, 7:18 p.m. UTC
Use the drm_fb_clip_offset() helper instead of open-coding the same
operation.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
DRM_PANIC already selects DRM_KMS_HELPER.

v2:
  - New.
---
 drivers/gpu/drm/drm_panic.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jocelyn Falempe June 14, 2024, 9:44 a.m. UTC | #1
On 13/06/2024 21:18, Geert Uytterhoeven wrote:
> Use the drm_fb_clip_offset() helper instead of open-coding the same
> operation.

Thanks, it's a nice cleanup.

Reviewed-by: Jocelyn Falempe <jfalempe@redhat.com>

> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> DRM_PANIC already selects DRM_KMS_HELPER.
> 
> v2:
>    - New.
> ---
>   drivers/gpu/drm/drm_panic.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
> index 814ef5c20c08ee42..5b0acf8c86e402a8 100644
> --- a/drivers/gpu/drm/drm_panic.c
> +++ b/drivers/gpu/drm/drm_panic.c
> @@ -285,7 +285,7 @@ static void drm_panic_blit(struct drm_scanout_buffer *sb, struct drm_rect *clip,
>   		return drm_panic_blit_pixel(sb, clip, sbuf8, spitch, fg_color);
>   
>   	map = sb->map[0];
> -	iosys_map_incr(&map, clip->y1 * sb->pitch[0] + clip->x1 * sb->format->cpp[0]);
> +	iosys_map_incr(&map, drm_fb_clip_offset(sb->pitch[0], sb->format, clip));
>   
>   	switch (sb->format->cpp[0]) {
>   	case 2:
> @@ -373,7 +373,7 @@ static void drm_panic_fill(struct drm_scanout_buffer *sb, struct drm_rect *clip,
>   		return drm_panic_fill_pixel(sb, clip, color);
>   
>   	map = sb->map[0];
> -	iosys_map_incr(&map, clip->y1 * sb->pitch[0] + clip->x1 * sb->format->cpp[0]);
> +	iosys_map_incr(&map, drm_fb_clip_offset(sb->pitch[0], sb->format, clip));
>   
>   	switch (sb->format->cpp[0]) {
>   	case 2:
kernel test robot June 15, 2024, 10:55 a.m. UTC | #2
Hi Geert,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[cannot apply to linus/master v6.10-rc3 next-20240613]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Geert-Uytterhoeven/drm-panic-Fix-uninitialized-drm_scanout_buffer-set_pixel-crash/20240614-032053
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/3121082eb4beb461773ebb6f656ed9b4286967ee.1718305355.git.geert%2Brenesas%40glider.be
patch subject: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
config: x86_64-randconfig-003-20240615 (https://download.01.org/0day-ci/archive/20240615/202406151811.yEIZ6203-lkp@intel.com/config)
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240615/202406151811.yEIZ6203-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202406151811.yEIZ6203-lkp@intel.com/

All errors (new ones prefixed by >>):

>> depmod: ERROR: Cycle detected: drm -> drm_kms_helper -> drm
   depmod: ERROR: Found 2 modules in dependency cycles!
   make[3]: *** [scripts/Makefile.modinst:128: depmod] Error 1 shuffle=844234264
   make[3]: Target '__modinst' not remade because of errors.
   make[2]: *** [Makefile:1842: modules_install] Error 2 shuffle=844234264
   make[1]: *** [Makefile:240: __sub-make] Error 2 shuffle=844234264
   make[1]: Target 'modules_install' not remade because of errors.
   make: *** [Makefile:240: __sub-make] Error 2 shuffle=844234264
   make: Target 'modules_install' not remade because of errors.
Geert Uytterhoeven June 16, 2024, 9:08 a.m. UTC | #3
On Sat, Jun 15, 2024 at 12:55 PM kernel test robot <lkp@intel.com> wrote:
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on drm-misc/drm-misc-next]
> [cannot apply to linus/master v6.10-rc3 next-20240613]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Geert-Uytterhoeven/drm-panic-Fix-uninitialized-drm_scanout_buffer-set_pixel-crash/20240614-032053
> base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> patch link:    https://lore.kernel.org/r/3121082eb4beb461773ebb6f656ed9b4286967ee.1718305355.git.geert%2Brenesas%40glider.be
> patch subject: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
> config: x86_64-randconfig-003-20240615 (https://download.01.org/0day-ci/archive/20240615/202406151811.yEIZ6203-lkp@intel.com/config)
> compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240615/202406151811.yEIZ6203-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202406151811.yEIZ6203-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> depmod: ERROR: Cycle detected: drm -> drm_kms_helper -> drm
>    depmod: ERROR: Found 2 modules in dependency cycles!

Oops, so DRM core cannot call any of the helpers, and DRM_PANIC
selecting DRM_KMS_HELPER was wrong in the first place?

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven June 16, 2024, 9:12 a.m. UTC | #4
On Sun, Jun 16, 2024 at 11:08 AM Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> On Sat, Jun 15, 2024 at 12:55 PM kernel test robot <lkp@intel.com> wrote:
> > kernel test robot noticed the following build errors:
> >
> > [auto build test ERROR on drm-misc/drm-misc-next]
> > [cannot apply to linus/master v6.10-rc3 next-20240613]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url:    https://github.com/intel-lab-lkp/linux/commits/Geert-Uytterhoeven/drm-panic-Fix-uninitialized-drm_scanout_buffer-set_pixel-crash/20240614-032053
> > base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
> > patch link:    https://lore.kernel.org/r/3121082eb4beb461773ebb6f656ed9b4286967ee.1718305355.git.geert%2Brenesas%40glider.be
> > patch subject: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
> > config: x86_64-randconfig-003-20240615 (https://download.01.org/0day-ci/archive/20240615/202406151811.yEIZ6203-lkp@intel.com/config)
> > compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240615/202406151811.yEIZ6203-lkp@intel.com/reproduce)
> >
> > If you fix the issue in a separate patch/commit (i.e. not just a new version of
> > the same patch/commit), kindly add following tags
> > | Reported-by: kernel test robot <lkp@intel.com>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/202406151811.yEIZ6203-lkp@intel.com/
> >
> > All errors (new ones prefixed by >>):
> >
> > >> depmod: ERROR: Cycle detected: drm -> drm_kms_helper -> drm
> >    depmod: ERROR: Found 2 modules in dependency cycles!
>
> Oops, so DRM core cannot call any of the helpers, and DRM_PANIC
> selecting DRM_KMS_HELPER was wrong in the first place?

Q: So how does this work with DRM_PANIC calling
   drm_fb_helper_emergency_disable()?
A: drm_fb_helper_emergency_disable() is a dummy if
   !CONFIG_DRM_FBDEV_EMULATION, so I guess no one tried to build
   a failing randconfig with CONFIG_DRM_FBDEV_EMULATION=y yet.

Gr{oetje,eeting}s,

                        Geert
Jocelyn Falempe June 17, 2024, 7:29 a.m. UTC | #5
On 16/06/2024 11:12, Geert Uytterhoeven wrote:
> On Sun, Jun 16, 2024 at 11:08 AM Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Sat, Jun 15, 2024 at 12:55 PM kernel test robot <lkp@intel.com> wrote:
>>> kernel test robot noticed the following build errors:
>>>
>>> [auto build test ERROR on drm-misc/drm-misc-next]
>>> [cannot apply to linus/master v6.10-rc3 next-20240613]
>>> [If your patch is applied to the wrong git tree, kindly drop us a note.
>>> And when submitting patch, we suggest to use '--base' as documented in
>>> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>>>
>>> url:    https://github.com/intel-lab-lkp/linux/commits/Geert-Uytterhoeven/drm-panic-Fix-uninitialized-drm_scanout_buffer-set_pixel-crash/20240614-032053
>>> base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
>>> patch link:    https://lore.kernel.org/r/3121082eb4beb461773ebb6f656ed9b4286967ee.1718305355.git.geert%2Brenesas%40glider.be
>>> patch subject: [PATCH v2 5/7] drm/panic: Convert to drm_fb_clip_offset()
>>> config: x86_64-randconfig-003-20240615 (https://download.01.org/0day-ci/archive/20240615/202406151811.yEIZ6203-lkp@intel.com/config)
>>> compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
>>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240615/202406151811.yEIZ6203-lkp@intel.com/reproduce)
>>>
>>> If you fix the issue in a separate patch/commit (i.e. not just a new version of
>>> the same patch/commit), kindly add following tags
>>> | Reported-by: kernel test robot <lkp@intel.com>
>>> | Closes: https://lore.kernel.org/oe-kbuild-all/202406151811.yEIZ6203-lkp@intel.com/
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>>> depmod: ERROR: Cycle detected: drm -> drm_kms_helper -> drm
>>>     depmod: ERROR: Found 2 modules in dependency cycles!
>>
>> Oops, so DRM core cannot call any of the helpers, and DRM_PANIC
>> selecting DRM_KMS_HELPER was wrong in the first place?
> 
> Q: So how does this work with DRM_PANIC calling
>     drm_fb_helper_emergency_disable()?
> A: drm_fb_helper_emergency_disable() is a dummy if
>     !CONFIG_DRM_FBDEV_EMULATION, so I guess no one tried to build
>     a failing randconfig with CONFIG_DRM_FBDEV_EMULATION=y yet.

drm_fb_helper_emergency_disable() is part of the series
https://patchwork.freedesktop.org/series/132720/
which, after discussing it on IRC with sima and Javier, is not a good 
solution, and is abandoned.

I think the "select DRM_KMS_HELPER" is a leftover from earlier version 
of drm_panic, that used the color conversion function from 
drm_kms_helper, but that has changed in v10 and later.

drm_panic is called from the drm_core code, so in fact it can't depends 
on the kms helper.

I don't see a good solution to workaround this circular dependency, 
maybe depends on DRM_KMS_HELPER being built-in ? (so that means drm_core 
will also be built-in). But that means platform that build drm core as 
module, won't be able to use drm_panic.

There are a few of them in the kernel tree:
rg -l CONFIG_DRM=m
arch/powerpc/configs/85xx/stx_gp3_defconfig
arch/powerpc/configs/ppc6xx_defconfig
arch/powerpc/configs/skiroot_defconfig
arch/powerpc/configs/pmac32_defconfig
arch/mips/configs/ci20_defconfig
arch/arc/configs/axs101_defconfig
arch/arc/configs/axs103_smp_defconfig
arch/riscv/configs/defconfig
arch/parisc/configs/generic-32bit_defconfig
arch/arm/configs/davinci_all_defconfig
arch/arm/configs/omap2plus_defconfig
arch/arm/configs/pxa_defconfig
arch/arm64/configs/defconfig

> 
> Gr{oetje,eeting}s,
> 
>                          Geert
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_panic.c b/drivers/gpu/drm/drm_panic.c
index 814ef5c20c08ee42..5b0acf8c86e402a8 100644
--- a/drivers/gpu/drm/drm_panic.c
+++ b/drivers/gpu/drm/drm_panic.c
@@ -285,7 +285,7 @@  static void drm_panic_blit(struct drm_scanout_buffer *sb, struct drm_rect *clip,
 		return drm_panic_blit_pixel(sb, clip, sbuf8, spitch, fg_color);
 
 	map = sb->map[0];
-	iosys_map_incr(&map, clip->y1 * sb->pitch[0] + clip->x1 * sb->format->cpp[0]);
+	iosys_map_incr(&map, drm_fb_clip_offset(sb->pitch[0], sb->format, clip));
 
 	switch (sb->format->cpp[0]) {
 	case 2:
@@ -373,7 +373,7 @@  static void drm_panic_fill(struct drm_scanout_buffer *sb, struct drm_rect *clip,
 		return drm_panic_fill_pixel(sb, clip, color);
 
 	map = sb->map[0];
-	iosys_map_incr(&map, clip->y1 * sb->pitch[0] + clip->x1 * sb->format->cpp[0]);
+	iosys_map_incr(&map, drm_fb_clip_offset(sb->pitch[0], sb->format, clip));
 
 	switch (sb->format->cpp[0]) {
 	case 2: