On Thu, Aug 3, 2017 at 12:11 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Wed, Aug 02, 2017 at 08:37:56PM -0500, David Lechner wrote:
On 08/02/2017 06:28 PM, Daniel Vetter wrote:
On Wed, Aug 2, 2017 at 6:37 PM, David Lechner david@lechnology.com wrote:
On 08/02/2017 04:46 AM, Daniel Vetter wrote:
On Tue, Aug 01, 2017 at 10:26:41AM -0500, David Lechner wrote:
The fbdev subsystem has a place for physical dimensions (width and height in mm) that is readable by userspace. Since DRM also knows these dimensions, pass this information to the fbdev device.
Signed-off-by: David Lechner david@lechnology.com
Still in the wrong function. Also please add some notation about what you changed when resubmitting a patch (it took me a while to remember that I replied to you already). That makes patch reviewing more efficient.
Sorry for being so dense. :-/
I did read your first reply at least 10 times. All of the terminology is foreign to me, but after sleeping on it a few days, I think it is slowly soaking into my brain.
No problem, the code is fairly convoluted. One more question on your v3: From reading fbdev code I don't see any place that overwrites the physical dimensions except the fill_var helper function (which is called deep down from register_framebuffer). If we entirely remove the var.width/height assignments from that (including the -1 default) and move all of it into setup_crtcs, would that work?
I tried that, but in my case, drm_setup_crtcs() is only called once before the fbdev is allocated. So, I got a kernel oops from dereferencing a null pointer, which is why there is a null check for fb_helper->fbdev as part of the if statement I added there. Since drm_setup_crtcs() is not called again, the var.width and height are never set.
fb_helper->fbdev is allocated in drm_fb_helper_alloc_fbi, which /should/ be called as the very first thing in setting up the entire fbdev stuff. If you're oopsing on that, something went wrong. We also set info->var in a few other places before calling register_framebuffer already (see the line that sets info->var.pixclock right before calling register_framebuffer).
I also noticed that there is another similar workaround/code duplication for the framebuffer not being allocated in the first call to drm_setup_crtcs() seen at the end of drm_fb_helper_single_fb_probe().
Ok I screwed up, alloc_fbi is called from fb_probe, which is called only from helper_single_probe(). Sorry about all my confusion, sounds like we indeed have to duplicate this. Since the duplication is necessary, can you pls extract an update_var static helper function, which is called from both places? I'd put the call in drm_fb_helper_hotplugt_event and right before the register_framebuffer for the load path, not trying to be clever like I suggested and hiding it somewhere in drm_setup_crtcs.
Sorry for going all over the place with this, but fbdev isn't really something I know all that well ... -Daniel