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");