| Message ID | 20220905163300.391692-4-thierry.reding@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/simpledrm: Support system memory framebuffers | expand |
Hi Thierry,
I love your patch! Perhaps something to improve:
[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm/drm-next drm-intel/for-linux-next drm-tip/drm-tip next-20220923]
[cannot apply to robh/for-next linus/master v6.0-rc6]
[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/Thierry-Reding/drm-simpledrm-Support-system-memory-framebuffers/20220906-101223
base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
config: i386-randconfig-s001-20220919
compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# https://github.com/intel-lab-lkp/linux/commit/9b049b0aabd39ea7ccac7d39974aa0c90f7f8d33
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Thierry-Reding/drm-simpledrm-Support-system-memory-framebuffers/20220906-101223
git checkout 9b049b0aabd39ea7ccac7d39974aa0c90f7f8d33
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/tiny/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/tiny/simpledrm.c:469:27: sparse: sparse: incorrect type in assignment (different address spaces) @@ expected void [noderef] __iomem *screen_base @@ got void * @@
drivers/gpu/drm/tiny/simpledrm.c:469:27: sparse: expected void [noderef] __iomem *screen_base
drivers/gpu/drm/tiny/simpledrm.c:469:27: sparse: got void *
>> drivers/gpu/drm/tiny/simpledrm.c:621:47: sparse: sparse: incorrect type in argument 2 (different address spaces) @@ expected void *vaddr @@ got void [noderef] __iomem *screen_base @@
drivers/gpu/drm/tiny/simpledrm.c:621:47: sparse: expected void *vaddr
drivers/gpu/drm/tiny/simpledrm.c:621:47: sparse: got void [noderef] __iomem *screen_base
vim +469 drivers/gpu/drm/tiny/simpledrm.c
456
457 /*
458 * Memory management
459 */
460
461 static int simpledrm_device_init_mm_sysmem(struct simpledrm_device *sdev, phys_addr_t start,
462 size_t size)
463 {
464 struct drm_device *dev = &sdev->dev;
465 phys_addr_t end = start + size - 1;
466
467 drm_info(dev, "using system memory framebuffer at [%pa-%pa]\n", &start, &end);
468
> 469 sdev->screen_base = devm_memremap(dev->dev, start, size, MEMREMAP_WC);
470 if (!sdev->screen_base)
471 return -ENOMEM;
472
473 return 0;
474 }
475
476 static int simpledrm_device_init_mm_io(struct simpledrm_device *sdev, phys_addr_t start,
477 size_t size)
478 {
479 struct drm_device *dev = &sdev->dev;
480 phys_addr_t end = start + size - 1;
481 struct resource *mem;
482
483 drm_info(dev, "using I/O memory framebuffer at [%pa-%pa]\n", &start, &end);
484
485 mem = devm_request_mem_region(dev->dev, start, size, sdev->dev.driver->name);
486 if (!mem) {
487 /*
488 * We cannot make this fatal. Sometimes this comes from magic
489 * spaces our resource handlers simply don't know about. Use
490 * the I/O-memory resource as-is and try to map that instead.
491 */
492 drm_warn(dev, "could not acquire memory region [%pa-%pa]\n", &start, &end);
493 } else {
494 size = resource_size(mem);
495 start = mem->start;
496 }
497
498 sdev->screen_base = devm_ioremap_wc(dev->dev, start, size);
499 if (!sdev->screen_base)
500 return -ENOMEM;
501
502 return 0;
503 }
504
505 static void simpledrm_device_exit_mm(void *data)
506 {
507 struct simpledrm_device *sdev = data;
508 struct drm_device *dev = &sdev->dev;
509
510 of_reserved_mem_device_release(dev->dev);
511 }
512
513 static int simpledrm_device_init_mm(struct simpledrm_device *sdev)
514 {
515 int (*init)(struct simpledrm_device *sdev, phys_addr_t start, size_t size);
516 struct drm_device *dev = &sdev->dev;
517 struct platform_device *pdev = to_platform_device(dev->dev);
518 phys_addr_t start, end;
519 size_t size;
520 int ret;
521
522 ret = of_reserved_mem_device_init_by_idx(dev->dev, dev->dev->of_node, 0);
523 if (ret) {
524 struct resource *res;
525
526 res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
527 if (!res)
528 return -EINVAL;
529
530 init = simpledrm_device_init_mm_io;
531 size = resource_size(res);
532 start = res->start;
533 } else {
534 devm_add_action_or_reset(dev->dev, simpledrm_device_exit_mm, sdev);
535 init = simpledrm_device_init_mm_sysmem;
536 start = sdev->sysmem_start;
537 size = sdev->sysmem_size;
538 }
539
540 end = start + size - 1;
541
542 ret = devm_aperture_acquire_from_firmware(dev, start, size);
543 if (ret) {
544 drm_err(dev, "could not acquire memory range [%pa-%pa]: %d\n", &start, &end, ret);
545 return ret;
546 }
547
548 return init(sdev, start, size);
549 }
550
551 /*
552 * Modesetting
553 */
554
555 /*
556 * Support all formats of simplefb and maybe more; in order
557 * of preference. The display's update function will do any
558 * conversion necessary.
559 *
560 * TODO: Add blit helpers for remaining formats and uncomment
561 * constants.
562 */
563 static const uint32_t simpledrm_primary_plane_formats[] = {
564 DRM_FORMAT_XRGB8888,
565 DRM_FORMAT_ARGB8888,
566 DRM_FORMAT_RGB565,
567 //DRM_FORMAT_XRGB1555,
568 //DRM_FORMAT_ARGB1555,
569 DRM_FORMAT_RGB888,
570 DRM_FORMAT_XRGB2101010,
571 DRM_FORMAT_ARGB2101010,
572 };
573
574 static const uint64_t simpledrm_primary_plane_format_modifiers[] = {
575 DRM_FORMAT_MOD_LINEAR,
576 DRM_FORMAT_MOD_INVALID
577 };
578
579 static int simpledrm_primary_plane_helper_atomic_check(struct drm_plane *plane,
580 struct drm_atomic_state *new_state)
581 {
582 struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(new_state, plane);
583 struct drm_crtc *new_crtc = new_plane_state->crtc;
584 struct drm_crtc_state *new_crtc_state = NULL;
585 int ret;
586
587 if (new_crtc)
588 new_crtc_state = drm_atomic_get_new_crtc_state(new_state, new_crtc);
589
590 ret = drm_atomic_helper_check_plane_state(new_plane_state, new_crtc_state,
591 DRM_PLANE_NO_SCALING,
592 DRM_PLANE_NO_SCALING,
593 false, false);
594 if (ret)
595 return ret;
596 else if (!new_plane_state->visible)
597 return 0;
598
599 return 0;
600 }
601
602 static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane,
603 struct drm_atomic_state *old_state)
604 {
605 struct drm_plane_state *plane_state = plane->state;
606 struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(old_state, plane);
607 struct drm_shadow_plane_state *shadow_plane_state = to_drm_shadow_plane_state(plane_state);
608 struct drm_framebuffer *fb = plane_state->fb;
609 struct drm_device *dev = plane->dev;
610 struct simpledrm_device *sdev = simpledrm_device_of_dev(dev);
611 struct drm_rect src_clip, dst_clip;
612 struct iosys_map dst;
613 int idx;
614
615 if (!fb)
616 return;
617
618 if (sdev->sysmem_size == 0)
619 iosys_map_set_vaddr_iomem(&dst, sdev->screen_base);
620 else
> 621 iosys_map_set_vaddr(&dst, sdev->screen_base);
622
623 if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &src_clip))
624 return;
625
626 dst_clip = plane_state->dst;
627 if (!drm_rect_intersect(&dst_clip, &src_clip))
628 return;
629
630 if (!drm_dev_enter(dev, &idx))
631 return;
632
633 iosys_map_incr(&dst, drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip));
634 drm_fb_blit(&dst, &sdev->pitch, sdev->format->format, shadow_plane_state->data, fb,
635 &src_clip);
636
637 drm_dev_exit(idx);
638 }
639
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index a81f91814595..16377b39f012 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -2,6 +2,7 @@ #include <linux/clk.h> #include <linux/of_clk.h> +#include <linux/of_reserved_mem.h> #include <linux/minmax.h> #include <linux/platform_data/simplefb.h> #include <linux/platform_device.h> @@ -218,6 +219,8 @@ struct simpledrm_device { /* memory management */ void __iomem *screen_base; + phys_addr_t sysmem_start; + size_t sysmem_size; /* modesetting */ uint32_t formats[8]; @@ -451,6 +454,100 @@ static int simpledrm_device_init_regulators(struct simpledrm_device *sdev) } #endif +/* + * Memory management + */ + +static int simpledrm_device_init_mm_sysmem(struct simpledrm_device *sdev, phys_addr_t start, + size_t size) +{ + struct drm_device *dev = &sdev->dev; + phys_addr_t end = start + size - 1; + + drm_info(dev, "using system memory framebuffer at [%pa-%pa]\n", &start, &end); + + sdev->screen_base = devm_memremap(dev->dev, start, size, MEMREMAP_WC); + if (!sdev->screen_base) + return -ENOMEM; + + return 0; +} + +static int simpledrm_device_init_mm_io(struct simpledrm_device *sdev, phys_addr_t start, + size_t size) +{ + struct drm_device *dev = &sdev->dev; + phys_addr_t end = start + size - 1; + struct resource *mem; + + drm_info(dev, "using I/O memory framebuffer at [%pa-%pa]\n", &start, &end); + + mem = devm_request_mem_region(dev->dev, start, size, sdev->dev.driver->name); + if (!mem) { + /* + * We cannot make this fatal. Sometimes this comes from magic + * spaces our resource handlers simply don't know about. Use + * the I/O-memory resource as-is and try to map that instead. + */ + drm_warn(dev, "could not acquire memory region [%pa-%pa]\n", &start, &end); + } else { + size = resource_size(mem); + start = mem->start; + } + + sdev->screen_base = devm_ioremap_wc(dev->dev, start, size); + if (!sdev->screen_base) + return -ENOMEM; + + return 0; +} + +static void simpledrm_device_exit_mm(void *data) +{ + struct simpledrm_device *sdev = data; + struct drm_device *dev = &sdev->dev; + + of_reserved_mem_device_release(dev->dev); +} + +static int simpledrm_device_init_mm(struct simpledrm_device *sdev) +{ + int (*init)(struct simpledrm_device *sdev, phys_addr_t start, size_t size); + struct drm_device *dev = &sdev->dev; + struct platform_device *pdev = to_platform_device(dev->dev); + phys_addr_t start, end; + size_t size; + int ret; + + ret = of_reserved_mem_device_init_by_idx(dev->dev, dev->dev->of_node, 0); + if (ret) { + struct resource *res; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (!res) + return -EINVAL; + + init = simpledrm_device_init_mm_io; + size = resource_size(res); + start = res->start; + } else { + devm_add_action_or_reset(dev->dev, simpledrm_device_exit_mm, sdev); + init = simpledrm_device_init_mm_sysmem; + start = sdev->sysmem_start; + size = sdev->sysmem_size; + } + + end = start + size - 1; + + ret = devm_aperture_acquire_from_firmware(dev, start, size); + if (ret) { + drm_err(dev, "could not acquire memory range [%pa-%pa]: %d\n", &start, &end, ret); + return ret; + } + + return init(sdev, start, size); +} + /* * Modesetting */ @@ -511,13 +608,18 @@ static void simpledrm_primary_plane_helper_atomic_update(struct drm_plane *plane struct drm_framebuffer *fb = plane_state->fb; struct drm_device *dev = plane->dev; struct simpledrm_device *sdev = simpledrm_device_of_dev(dev); - struct iosys_map dst = IOSYS_MAP_INIT_VADDR(sdev->screen_base); struct drm_rect src_clip, dst_clip; + struct iosys_map dst; int idx; if (!fb) return; + if (sdev->sysmem_size == 0) + iosys_map_set_vaddr_iomem(&dst, sdev->screen_base); + else + iosys_map_set_vaddr(&dst, sdev->screen_base); + if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, &src_clip)) return; @@ -721,8 +823,6 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv, struct drm_device *dev; int width, height, stride; const struct drm_format_info *format; - struct resource *res, *mem; - void __iomem *screen_base; struct drm_plane *primary_plane; struct drm_crtc *crtc; struct drm_encoder *encoder; @@ -790,35 +890,9 @@ static struct simpledrm_device *simpledrm_device_create(struct drm_driver *drv, drm_dbg(dev, "framebuffer format=%p4cc, size=%dx%d, stride=%d byte\n", &format->format, width, height, stride); - /* - * Memory management - */ - - res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (!res) - return ERR_PTR(-EINVAL); - - ret = devm_aperture_acquire_from_firmware(dev, res->start, resource_size(res)); - if (ret) { - drm_err(dev, "could not acquire memory range %pr: error %d\n", res, ret); + ret = simpledrm_device_init_mm(sdev); + if (ret) return ERR_PTR(ret); - } - - mem = devm_request_mem_region(&pdev->dev, res->start, resource_size(res), drv->name); - if (!mem) { - /* - * We cannot make this fatal. Sometimes this comes from magic - * spaces our resource handlers simply don't know about. Use - * the I/O-memory resource as-is and try to map that instead. - */ - drm_warn(dev, "could not acquire memory region %pr\n", res); - mem = res; - } - - screen_base = devm_ioremap_wc(&pdev->dev, mem->start, resource_size(mem)); - if (!screen_base) - return ERR_PTR(-ENOMEM); - sdev->screen_base = screen_base; /* * Modesetting @@ -959,5 +1033,35 @@ static struct platform_driver simpledrm_platform_driver = { module_platform_driver(simpledrm_platform_driver); +static int simple_framebuffer_device_init(struct reserved_mem *rmem, struct device *dev) +{ + struct simpledrm_device *sdev = dev_get_drvdata(dev); + + sdev->sysmem_start = rmem->base; + sdev->sysmem_size = rmem->size; + + return 0; +} + +static void simple_framebuffer_device_release(struct reserved_mem *rmem, struct device *dev) +{ +} + +static const struct reserved_mem_ops simple_framebuffer_ops = { + .device_init = simple_framebuffer_device_init, + .device_release = simple_framebuffer_device_release, +}; + +static int simple_framebuffer_init(struct reserved_mem *rmem) +{ + pr_info("framebuffer memory at %pa, size %lu bytes\n", &rmem->base, + (unsigned long)rmem->size); + + rmem->ops = &simple_framebuffer_ops; + + return 0; +} +RESERVEDMEM_OF_DECLARE(simple_framebuffer, "framebuffer", simple_framebuffer_init); + MODULE_DESCRIPTION(DRIVER_DESC); MODULE_LICENSE("GPL v2");