From bf6c484c136e66e86e80f59f60df170297b3b29b Mon Sep 17 00:00:00 2001 From: Alexander Boettcher Date: Tue, 20 Feb 2024 11:36:59 +0100 Subject: [PATCH] gpu/intel: use with(fn, fn_error) pattern Issue #5081 --- repos/os/src/drivers/gpu/intel/main.cc | 138 ++++++++++-------- .../src/drivers/gpu/intel/platform_session.h | 54 +++---- 2 files changed, 104 insertions(+), 88 deletions(-) diff --git a/repos/os/src/drivers/gpu/intel/main.cc b/repos/os/src/drivers/gpu/intel/main.cc index 4f7955d1b5..0a365d64fb 100644 --- a/repos/os/src/drivers/gpu/intel/main.cc +++ b/repos/os/src/drivers/gpu/intel/main.cc @@ -263,26 +263,26 @@ struct Igd::Device Constructible _ggtt { }; - __attribute__((warn_unused_result)) - bool with_ggtt(auto const &fn) + void with_ggtt(auto const &fn, auto const &fn_error) { - if (!_ggtt.constructed()) - return false; + if (!_ggtt.constructed()) { + fn_error(); + return; + } - return _resources.with_mmio([&](auto &mmio) { + _resources.with_mmio([&](auto &mmio) { fn(*_ggtt, mmio); + }, [&]() { + fn_error(); }); } - __attribute__((warn_unused_result)) - bool with_ggtt(auto const &fn) const + void with_ggtt(auto const &fn, auto const &fn_error) const { - if (!_ggtt.constructed()) - return false; - - fn(*_ggtt); - - return true; + if (_ggtt.constructed()) + fn(*_ggtt); + else + fn_error(); } /************ @@ -315,7 +315,7 @@ struct Igd::Device { Genode::Registered *mem = nullptr; - if (!with_ggtt([&](auto &ggtt, auto &mmio) { + with_ggtt([&](auto &ggtt, auto &mmio) { Genode::Dataspace_client client(cap); addr_t const phys_addr = _pci_backend_alloc.dma_addr(cap); size_t const size = client.size(); @@ -332,8 +332,9 @@ struct Igd::Device addr_t const pa = phys_addr + i; ggtt.insert_pte(mmio, pa, offset + (i / PAGE_SIZE)); } - })) + }, []() { throw Could_not_map_vram(); + }); if (!mem) throw Could_not_map_vram(); @@ -341,14 +342,15 @@ struct Igd::Device return *mem; } - void unmap_dataspace_ggtt(Genode::Allocator &alloc, Ggtt::Mapping &m) + void unmap_dataspace_ggtt(Genode::Allocator &alloc, Ggtt::Mapping *m) { - if (!with_ggtt([&](auto &ggtt, auto &mmio) { - size_t const num = m.vsize / PAGE_SIZE; - ggtt.remove_pte_range(mmio, m.offset, num); - Genode::destroy(&alloc, &m); - })) - error(__func__, " failed"); + with_ggtt([&](auto &ggtt, auto &mmio) { + size_t const num = m->vsize / PAGE_SIZE; + ggtt.remove_pte_range(mmio, m->offset, num); + Genode::destroy(&alloc, m); + }, []() { + error("unmap_dataspace_ggtt failed"); + }); } struct Invalid_ppgtt : Genode::Exception { }; @@ -418,7 +420,7 @@ struct Igd::Device map(device.map_dataspace_ggtt(alloc, gmm.ram_ds.ds, gmm.offset)) { } - ~Mapping_guard() { device.unmap_dataspace_ggtt(alloc, map); } + ~Mapping_guard() { device.unmap_dataspace_ggtt(alloc, &map); } }; Device const &device; @@ -432,10 +434,11 @@ struct Igd::Device { Ggtt::Offset offset { }; - if (!device.with_ggtt([&](auto &ggtt) { + device.with_ggtt([&](auto &ggtt) { offset = ggtt.find_free(pages, true); - })) + }, []() { error("Gtt::Offset setup failed"); + }); return offset; } @@ -444,10 +447,11 @@ struct Igd::Device { addr_t const offset = (map.map.offset + skip) * PAGE_SIZE; - if (!device._resources.with_gmadr(offset, [&](Byte_range_ptr const &range) { + device._resources.with_gmadr(offset, [&](Byte_range_ptr const &range) { fn(range); - })) + }, []() { error("Gmadr object unavailable"); + }); } addr_t gmaddr() const { /* graphics memory address */ @@ -1167,7 +1171,7 @@ struct Igd::Device Genode::error("watchdog triggered: engine stuck," " vGPU=", _active_vgpu->id()); - bool ok = _resources.with_mmio([&](auto &mmio) { + _resources.with_mmio([&](auto &mmio) { if (DEBUG) { mmio.dump(); mmio.error_dump(); @@ -1192,10 +1196,9 @@ struct Igd::Device /* the stuck vgpu */ _handle_vgpu_after_reset(*vgpu); - }); - - if (!ok) + }, []() { error("reset of vGPU failed"); + }); } Genode::Signal_handler _watchdog_timeout_sigh { @@ -1212,7 +1215,7 @@ struct Igd::Device if (state == "driver_reinit") { _resources.acquire_device(); - if (!_resources.with_mmio([&](auto &mmio) { + _resources.with_mmio([&](auto &mmio) { mmio.generation(_info.generation); reinit(mmio); @@ -1230,8 +1233,9 @@ struct Igd::Device } else warning("setup_ring_vram failed"); }); - })) + }, []() { error("reinit - failed"); + }); return; } @@ -1271,7 +1275,7 @@ struct Igd::Device { using namespace Genode; - if (!_resources.with_mmio_gmadr([&](auto &mmio, auto &gmadr) { + _resources.with_mmio_gmadr([&](auto &mmio, auto &gmadr) { _resources.with_platform([&](auto &plat_con) { auto const ggtt_base = mmio.base() + (mmio.size() / 2); @@ -1288,8 +1292,9 @@ struct Igd::Device reinit(mmio); }); - })) + }, []() { throw Unsupported_device(); + }); _timer.sigh(_watchdog_timeout_sigh); } @@ -2030,14 +2035,15 @@ class Gpu::Session_component : public Genode::Session_object if (vram.owner(_owner.cap) == false) return false; if (!vram.map.invalid()) { - _device.unmap_dataspace_ggtt(_heap, vram.map); + _device.unmap_dataspace_ggtt(_heap, &vram.map); } if (vram.fenced != Vram::INVALID_FENCE) { - if (!_device._resources.with_mmio([&](auto &mmio) { + _device._resources.with_mmio([&](auto &mmio) { _device.clear_tiling(mmio, vram.fenced); - })) + }, [&]() { warning("tiling could not be cleared"); + }); _vgpu.active_fences--; } @@ -2097,7 +2103,10 @@ class Gpu::Session_component : public Genode::Session_object bool ppgtt_va_valid = false; Gpu::addr_t ppgtt_va = 0; - bool const dev_offline = !_device._resources.with_mmio([](auto &){}); + bool dev_offline = true; + + _device._resources.with_mmio([&](auto &){ dev_offline = false; }, + [ ](){}); _apply_vram_local(id, [&] (Vram_local &vram_local) { @@ -2132,18 +2141,18 @@ class Gpu::Session_component : public Genode::Session_object /* remember current execute in vGPU */ _vgpu.delay_execute(ppgtt_va); - /* return seqno which will be used after device is re-aquired */ + /* return seqno which will be used after device is re-acquired */ return { .value = _vgpu.current_seqno() + 1 }; } if (!found) throw Gpu::Session::Invalid_state(); - if (!_device._resources.with_mmio([&](auto &mmio) { + _device._resources.with_mmio([&](auto &mmio) { _device.vgpu_activate(_vgpu, mmio); - })) { + }, []() { Genode::error("Device mmio not available"); - } + }); return { .value = _vgpu.current_seqno() }; } @@ -2345,9 +2354,9 @@ class Gpu::Session_component : public Genode::Session_object { bool result = false; - result |= _device._resources.with_mmio([&](auto &mmio) { + _device._resources.with_mmio([&](auto &mmio) { result = _set_tiling_gpu(mmio, id, offset, mode); - }); + }, []() { }); return result; } @@ -2450,10 +2459,17 @@ class Gpu::Root : public Gpu::Root_component void _destroy_session(Session_component *s) override { if (s->vgpu_active()) { - if (!_device || !_device->_resources.with_mmio([&](auto &mmio) { - Genode::warning("vGPU active, reset device and hope for the best"); - _device->reset(mmio); - })) + bool warn = true; + + if (_device) { + _device->_resources.with_mmio([&](auto &mmio) { + Genode::warning("vGPU active, reset device and hope for the best"); + _device->reset(mmio); + warn = false; + }, []() { }); + } + + if (warn) Genode::warning("vGPU active, reset of device failed"); } else { /* remove from scheduled list */ @@ -2598,10 +2614,11 @@ struct Main : Irq_ack_handler, Gpu_reset_handler { bool display_irq = false; if (_igd_device.constructed()) { - if (!_dev.with_mmio([&](auto &mmio) { + _dev.with_mmio([&](auto &mmio) { display_irq = _igd_device->handle_irq(mmio); - })) + }, []() { error("handle_irq with mmio not possible"); + }); /* GPU not present forward all IRQs to platform client */ } else { _platform_root.handle_irq(); @@ -2622,14 +2639,18 @@ struct Main : Irq_ack_handler, Gpu_reset_handler void ack_irq() override { if (_igd_device.constructed()) { - if (!_dev.with_mmio([&](auto &mmio) { + _dev.with_mmio([&](auto &mmio) { _igd_device->enable_master_irq(mmio); - })) - error(__func__, " with_mmio failed"); + }, []() { + error("ack_irq with_mmio failed"); + }); } - if (!_dev.with_irq([&](auto &irq) { irq.ack(); })) - error(__func__, " with_irq failed"); + _dev.with_irq([&](auto &irq) { + irq.ack(); + }, []() { + error("ack_irq with_irq failed"); + }); if (_igd_device.constructed()) _igd_device->device_release_if_stopped_and_idle(); @@ -2637,13 +2658,14 @@ struct Main : Irq_ack_handler, Gpu_reset_handler void reset() override { - if (!_dev.with_mmio([&](auto &mmio) { + _dev.with_mmio([&](auto &mmio) { _dev.with_platform([&](auto &plat_con) { auto const base = mmio.base() + (mmio.size() / 2); Igd::Ggtt(plat_con, mmio, base, Igd::GTT_RESERVED, 0, 0); }); - })) + }, []() { error("reset failed"); + }); } }; diff --git a/repos/os/src/drivers/gpu/intel/platform_session.h b/repos/os/src/drivers/gpu/intel/platform_session.h index d84e6e8355..5fd50aea25 100644 --- a/repos/os/src/drivers/gpu/intel/platform_session.h +++ b/repos/os/src/drivers/gpu/intel/platform_session.h @@ -300,7 +300,7 @@ class Platform::Resources : Noncopyable void _reinit() { - if (!with_mmio_gmadr([&](auto &mmio, auto &gmadr) { + with_mmio_gmadr([&](auto &mmio, auto &gmadr) { using namespace Igd; @@ -328,8 +328,9 @@ class Platform::Resources : Noncopyable _rm_gmadr.detach(0ul); _rm_gmadr.attach_at(gmadr.cap(), 0ul, APERTURE_RESERVED); - })) - error(__func__, " failed"); + }, []() { + error("reinit failed"); + }); } public: @@ -369,47 +370,40 @@ class Platform::Resources : Noncopyable } } - __attribute__((warn_unused_result)) - bool with_mmio_gmadr(auto const &fn) + void with_mmio_gmadr(auto const &fn, auto const &fn_error) { - if (!_mmio.constructed() || !_gmadr.constructed()) - return false; + if (!_mmio.constructed() || !_gmadr.constructed()) { + fn_error(); + return; + } fn(*_mmio, *_gmadr); - - return true; } - __attribute__((warn_unused_result)) - bool with_gmadr(auto const offset, auto const &fn) + void with_gmadr(auto const offset, auto const &fn, auto const &fn_error) { - if (!_gmadr.constructed() || !_gmadr_mem.constructed()) - return false; + if (!_gmadr.constructed() || !_gmadr_mem.constructed()) { + fn_error(); + return; + } fn({_gmadr_mem->local_addr() + offset, _gmadr->size() - offset }); - return true; } - __attribute__((warn_unused_result)) - bool with_irq(auto const &fn) + void with_irq(auto const &fn, auto const &fn_error) { - if (!_irq.constructed()) - return false; - - fn(*_irq); - - return true; + if (_irq.constructed()) + fn(*_irq); + else + fn_error(); } - __attribute__((warn_unused_result)) - bool with_mmio(auto const &fn) + void with_mmio(auto const &fn, auto const &fn_error) { - if (!_mmio.constructed()) - return false; - - fn(*_mmio); - - return true; + if (_mmio.constructed()) + fn(*_mmio); + else + fn_error(); } void with_gttm_gmadr(auto const &fn)