From 558cc8e00c363b9b1e492e863bb69ace4dd01bcd Mon Sep 17 00:00:00 2001 From: Stefan Kalkowski Date: Thu, 16 Jun 2016 14:41:40 +0200 Subject: [PATCH] intel_fb_drv: make buffering mandatory Instead of providing a buffer to the client and blitting from that to the "real" framebuffer as an option, with this commit we always do so. Thereby its possible to immediately destroy the old framebuffer used by hardware when a configuration change is done, and a new framebuffer is used. This also simplifies the modesetting. Moreover, this commit fixes an issue when not using the connector reporting. Until now the initial mode detection of connectors was only done when the report was created. this is a regression that entered the driver when upgrading to the recent Linux kernel version. Ref #1997 --- repos/dde_linux/run/intel_fb.run | 2 +- .../src/drivers/framebuffer/intel/README | 9 -- .../framebuffer/intel/include/component.h | 137 ++++++++---------- .../src/drivers/framebuffer/intel/lx_emul.cc | 113 +++++---------- .../src/drivers/framebuffer/intel/lx_emul_c.c | 4 +- .../src/drivers/framebuffer/intel/main.cc | 3 +- .../src/test/framebuffer/intel/main.cc | 1 - 7 files changed, 103 insertions(+), 166 deletions(-) diff --git a/repos/dde_linux/run/intel_fb.run b/repos/dde_linux/run/intel_fb.run index 508de4c10..04378fc42 100644 --- a/repos/dde_linux/run/intel_fb.run +++ b/repos/dde_linux/run/intel_fb.run @@ -63,7 +63,7 @@ append config { - + diff --git a/repos/dde_linux/src/drivers/framebuffer/intel/README b/repos/dde_linux/src/drivers/framebuffer/intel/README index 4f15246e4..05b6c9ab1 100644 --- a/repos/dde_linux/src/drivers/framebuffer/intel/README +++ b/repos/dde_linux/src/drivers/framebuffer/intel/README @@ -26,15 +26,6 @@ this case it will also change the current virtual resolution to the maximum of the configured resolutions in width and height, and it will inform its client about the change in resolution. -It is possible to run the driver in 'buffered' mode, which means it does not -export the framebuffer memory directly to the client, but provides a simple RAM -dataspace instead. The copying from the RAM dataspace to the framebuffer memory -is done by the framebuffer driver when triggered by a refresh operation. This -option can alleviate tearing effects, and has to be enabled via the -configuration like this: - -! - If you experience problems like hotplugging of connectors does not work, you can force the driver to poll frequently for hotplug events by defining a period in milliseconds like this: diff --git a/repos/dde_linux/src/drivers/framebuffer/intel/include/component.h b/repos/dde_linux/src/drivers/framebuffer/intel/include/component.h index d3e935a52..5c0bb2fd6 100644 --- a/repos/dde_linux/src/drivers/framebuffer/intel/include/component.h +++ b/repos/dde_linux/src/drivers/framebuffer/intel/include/component.h @@ -42,18 +42,20 @@ class Framebuffer::Driver { private: + struct Configuration { + int height = 16; + int width = 64; + unsigned bpp = 2; + Genode::Dataspace_capability cap; + void * addr = nullptr; + Genode::size_t size = 0; + drm_framebuffer * lx_obj = nullptr; + } _config; + Session_component &_session; Timer::Connection _timer; Genode::Signal_handler _poll_handler; - int _height = 0; - int _width = 0; - static constexpr unsigned _bytes_per_pixel = 2; - void *_new_fb_ds_base = nullptr; - void *_cur_fb_ds_base = nullptr; - Genode::uint64_t _cur_fb_ds_size = 0; - drm_framebuffer *_new_fb = nullptr; - drm_framebuffer *_cur_fb = nullptr; - unsigned long _poll_ms = false; + unsigned long _poll_ms = 0; drm_display_mode * _preferred_mode(drm_connector *connector); @@ -65,19 +67,16 @@ class Framebuffer::Driver : _session(session), _timer(env), _poll_handler(env.ep(), *this, &Driver::_poll) {} - int width() const { return _width; } - int height() const { return _height; } - unsigned bpp() const { return _bytes_per_pixel; } - - Genode::size_t size() const { - return _width * _height * _bytes_per_pixel; } + int width() const { return _config.width; } + int height() const { return _config.height; } + int bpp() const { return _config.bpp; } + Genode::Dataspace_capability dataspace() const { + return _config.cap; } void finish_initialization(); void set_polling(unsigned long poll); - bool mode_changed(); + void update_mode(); void generate_report(); - void free_framebuffer(); - Genode::Dataspace_capability dataspace(); }; @@ -91,45 +90,20 @@ class Framebuffer::Session_component : public Genode::Rpc_object Genode::Attached_rom_dataspace &_config; Genode::Signal_context_capability _mode_sigh; Timer::Connection _timer; - bool _buffered; Lazy _fb_ds; - Lazy _bb_ds; - bool _in_update = false; - - bool _buffered_from_config() { - return _config.xml().attribute_value("buffered", false); } + Genode::Ram_session &_ram; + Genode::Attached_ram_dataspace _bb_ds; + bool _in_mode_change = true; unsigned long _polling_from_config() { return _config.xml().attribute_value("poll", 0); } - void _refresh_buffered(int x, int y, int w, int h) - { - using namespace Genode; - - int width = _driver.width(), height = _driver.height(); - unsigned bpp = _driver.bpp(); - - /* clip specified coordinates against screen boundaries */ - int x2 = min(x + w - 1, width - 1), - y2 = min(y + h - 1, height - 1); - int x1 = max(x, 0), - y1 = max(y, 0); - if (x1 > x2 || y1 > y2) return; - - /* copy pixels from back buffer to physical frame buffer */ - char *src = _bb_ds->local_addr() + bpp*(width*y1 + x1), - *dst = _fb_ds->local_addr() + bpp*(width*y1 + x1); - - blit(src, bpp*width, dst, bpp*width, - bpp*(x2 - x1 + 1), y2 - y1 + 1); - } - public: Session_component(Genode::Env &env, Genode::Attached_rom_dataspace &config) : _driver(env, *this), _config(config), _timer(env), - _buffered(_buffered_from_config()) {} + _ram(env.ram()), _bb_ds(env.ram(), env.rm(), 0) {} Driver & driver() { return _driver; } @@ -138,13 +112,19 @@ class Framebuffer::Session_component : public Genode::Rpc_object _config.update(); if (!_config.valid()) return; - _in_update = true; - _buffered = _buffered_from_config(); _driver.set_polling(_polling_from_config()); - if (_driver.mode_changed() && _mode_sigh.valid()) - Genode::Signal_transmitter(_mode_sigh).submit(); + + _in_mode_change = true; + + _driver.update_mode(); + + if (_driver.dataspace().valid()) + _fb_ds.construct(_driver.dataspace()); else - _in_update = false; + _fb_ds.destruct(); + + if (_mode_sigh.valid()) + Genode::Signal_transmitter(_mode_sigh).submit(); } Genode::Xml_node config() { return _config.xml(); } @@ -156,28 +136,9 @@ class Framebuffer::Session_component : public Genode::Rpc_object Genode::Dataspace_capability dataspace() override { - _in_update = false; - - if (_fb_ds.constructed()) - _fb_ds.destruct(); - - _fb_ds.construct(_driver.dataspace()); - if (!_fb_ds.is_constructed()) - PERR("framebuffer dataspace not initialized"); - - if (_buffered) { - if (_bb_ds.is_constructed()) - _bb_ds.destruct(); - - _bb_ds.construct(Genode::env()->ram_session(), _driver.size()); - if (!_bb_ds.is_constructed()) { - PERR("buffered mode enabled, but buffer not initialized"); - return Genode::Dataspace_capability(); - } - return _bb_ds->cap(); - } - - return _fb_ds->cap(); + _bb_ds.realloc(&_ram, _driver.width()*_driver.height()*_driver.bpp()); + _in_mode_change = false; + return _bb_ds.cap(); } Mode mode() const override { @@ -192,8 +153,32 @@ class Framebuffer::Session_component : public Genode::Rpc_object _timer.trigger_periodic(10*1000); } - void refresh(int x, int y, int w, int h) override { - if (_buffered && !_in_update) _refresh_buffered(x, y, w, h); } + void refresh(int x, int y, int w, int h) override + { + using namespace Genode; + + if (!_fb_ds.constructed() || + !_bb_ds.local_addr() || + _in_mode_change) return; + + int width = _driver.width(); + int height = _driver.height(); + unsigned bpp = _driver.bpp(); + + /* clip specified coordinates against screen boundaries */ + int x2 = min(x + w - 1, width - 1), + y2 = min(y + h - 1, height - 1); + int x1 = max(x, 0), + y1 = max(y, 0); + if (x1 > x2 || y1 > y2) return; + + /* copy pixels from back buffer to physical frame buffer */ + char *src = _bb_ds.local_addr() + bpp*(width*y1 + x1), + *dst = _fb_ds->local_addr() + bpp*(width*y1 + x1); + + blit(src, bpp*width, dst, bpp*width, + bpp*(x2 - x1 + 1), y2 - y1 + 1); + } }; diff --git a/repos/dde_linux/src/drivers/framebuffer/intel/lx_emul.cc b/repos/dde_linux/src/drivers/framebuffer/intel/lx_emul.cc index dc3410b04..a723b2609 100644 --- a/repos/dde_linux/src/drivers/framebuffer/intel/lx_emul.cc +++ b/repos/dde_linux/src/drivers/framebuffer/intel/lx_emul.cc @@ -165,120 +165,82 @@ void Framebuffer::Driver::set_polling(unsigned long poll) } -bool Framebuffer::Driver::mode_changed() +void Framebuffer::Driver::update_mode() { using namespace Genode; - int width = 0, height = 0; + Configuration old = _config; + _config = Configuration(); dde_for_each_connector(dde_drm_device, [&] (drm_connector *c) { drm_display_mode * mode = _preferred_mode(c); if (!mode) return; - if (mode->hdisplay > width) width = mode->hdisplay; - if (mode->vdisplay > height) height = mode->vdisplay; + if (mode->hdisplay > _config.width) _config.width = mode->hdisplay; + if (mode->vdisplay > _config.height) _config.height = mode->vdisplay; }); - if (width == _width && height == _height) return false; + _config.lx_obj = + dde_c_allocate_framebuffer(_config.width, _config.height, &_config.addr, + (uint64_t*)&_config.size, dde_drm_device); + _config.cap = _config.addr ? Lx::ioremap_lookup((addr_t)_config.addr, _config.size) + : Genode::Dataspace_capability(); - drm_framebuffer *fb = - dde_c_allocate_framebuffer(width, height, &_new_fb_ds_base, - &_cur_fb_ds_size, dde_drm_device); - if (!fb) { - Genode::error("failed to allocate framebuffer ", width, "x", height); - return false; + { + Drm_guard guard(dde_drm_device); + dde_for_each_connector(dde_drm_device, [&] (drm_connector *c) { + dde_c_set_mode(dde_drm_device, c, _config.lx_obj, + _preferred_mode(c)); }); } - Drm_guard guard(dde_drm_device); - dde_for_each_connector(dde_drm_device, [&] (drm_connector *c) { - dde_c_set_mode(dde_drm_device, c, fb, - _preferred_mode(c)); }); - - _new_fb = fb; - _width = width; - _height = height; - return true; -} - - -void Framebuffer::Driver::free_framebuffer() -{ - Lx::iounmap(_cur_fb_ds_base); - _cur_fb->funcs->destroy(_cur_fb); - _cur_fb_ds_base = nullptr; -} - - -void dde_run_fb_destroy(void * data) -{ - Framebuffer::Driver * drv = (Framebuffer::Driver*) data; - drv->free_framebuffer(); - while (true) Lx::scheduler().current()->block_and_schedule(); -} - - -Genode::Dataspace_capability Framebuffer::Driver::dataspace() -{ - using namespace Genode; - - if (_cur_fb_ds_base) { - Lx::Task task(dde_run_fb_destroy, this, "fb_destroy", - Lx::Task::PRIORITY_3, Lx::scheduler()); - while (_cur_fb_ds_base) Lx::scheduler().schedule(); - Lx::scheduler().remove(&task); - } - - _cur_fb = _new_fb; - _cur_fb_ds_base = _new_fb_ds_base; - return Lx::ioremap_lookup((addr_t)_cur_fb_ds_base, - (size_t)_cur_fb_ds_size); + if (old.addr) Lx::iounmap(old.addr); + if (old.lx_obj) old.lx_obj->funcs->destroy(old.lx_obj); } void Framebuffer::Driver::generate_report() { - static Genode::Reporter reporter("connectors"); + Drm_guard guard(dde_drm_device); + /* detect mode information per connector */ + { + struct drm_connector *c; + list_for_each_entry(c, &dde_drm_device->mode_config.connector_list, + head) + if (list_empty(&c->modes)) c->funcs->fill_modes(c, 0, 0); + } + + /* check for report configuration option */ + static Genode::Reporter reporter("connectors"); try { reporter.enabled(_session.config().sub_node("report") .attribute_value(reporter.name().string(), false)); } catch (...) { reporter.enabled(false); } - if (!reporter.is_enabled()) return; + /* write new report */ try { Genode::Reporter::Xml_generator xml(reporter, [&] () { - struct drm_device *dev = dde_drm_device; - struct drm_connector *connector; - struct list_head panel_list; - - Drm_guard guard(dev); - - INIT_LIST_HEAD(&panel_list); - - list_for_each_entry(connector, &dev->mode_config.connector_list, head) { + struct drm_connector *c; + list_for_each_entry(c, &dde_drm_device->mode_config.connector_list, + head) { xml.node("connector", [&] () { - if (list_empty(&connector->modes)) - connector->funcs->fill_modes(connector, 0, 0); - - bool connected = connector->status == connector_status_connected; - xml.attribute("name", connector->name); + bool connected = c->status == connector_status_connected; + xml.attribute("name", c->name); xml.attribute("connected", connected); if (!connected) return; struct drm_display_mode *mode; - struct list_head mode_list; - INIT_LIST_HEAD(&mode_list); - list_for_each_entry(mode, &connector->modes, head) { + list_for_each_entry(mode, &c->modes, head) { xml.node("mode", [&] () { - xml.attribute("width", mode->hdisplay); + xml.attribute("width", mode->hdisplay); xml.attribute("height", mode->vdisplay); - xml.attribute("hz", mode->vrefresh); + xml.attribute("hz", mode->vrefresh); }); } }); @@ -289,6 +251,7 @@ void Framebuffer::Driver::generate_report() } } + extern "C" { /********************** diff --git a/repos/dde_linux/src/drivers/framebuffer/intel/lx_emul_c.c b/repos/dde_linux/src/drivers/framebuffer/intel/lx_emul_c.c index e79182167..4878b0896 100644 --- a/repos/dde_linux/src/drivers/framebuffer/intel/lx_emul_c.c +++ b/repos/dde_linux/src/drivers/framebuffer/intel/lx_emul_c.c @@ -43,7 +43,7 @@ dde_c_allocate_framebuffer(int width, int height, void ** base, r->pixel_format = DRM_FORMAT_RGB565; r->pitches[0] = width * 2; fb = dde_c_intel_framebuffer_create(dev, r, obj); - if (!fb) goto err2; + if (IS_ERR(fb)) goto err2; if (intel_pin_and_fence_fb_obj(NULL, fb, NULL, NULL, NULL)) goto err1; @@ -56,8 +56,8 @@ dde_c_allocate_framebuffer(int width, int height, void ** base, err1: drm_framebuffer_remove(fb); - fb = NULL; err2: + fb = NULL; drm_gem_object_unreference(&obj->base); out1: kfree(r); diff --git a/repos/dde_linux/src/drivers/framebuffer/intel/main.cc b/repos/dde_linux/src/drivers/framebuffer/intel/main.cc index 1ac034b08..0ffa3e497 100644 --- a/repos/dde_linux/src/drivers/framebuffer/intel/main.cc +++ b/repos/dde_linux/src/drivers/framebuffer/intel/main.cc @@ -68,8 +68,7 @@ struct Main Lx::scheduler().schedule(); } - void announce() { - Genode::env()->parent()->announce(ep.manage(root)); } + void announce() { env.parent().announce(ep.manage(root)); } }; diff --git a/repos/dde_linux/src/test/framebuffer/intel/main.cc b/repos/dde_linux/src/test/framebuffer/intel/main.cc index 9dde78c97..93aa79b18 100644 --- a/repos/dde_linux/src/test/framebuffer/intel/main.cc +++ b/repos/dde_linux/src/test/framebuffer/intel/main.cc @@ -84,7 +84,6 @@ void Framebuffer_controller::update_fb_config(Xml_node & report) static char buf[4096]; Xml_generator xml(buf, sizeof(buf), "config", [&] { - xml.attribute("buffered", "yes"); xml.node("report", [&] { xml.attribute("connectors", "yes"); });