| Message ID | 20190407165243.54043-4-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/fb-helper: Move modesetting code to drm_client | expand |
Den 07.04.2019 18.52, skrev Noralf Trønnes: > It is generic code and having it in the helper will let other drivers > benefit from it. > > One change was necessary assuming this to be true: > INTEL_INFO(dev_priv)->num_pipes == dev->mode_config.num_crtc > > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/drm_fb_helper.c | 194 ++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_fbdev.c | 218 ----------------------------- > include/drm/drm_fb_helper.h | 23 --- > 3 files changed, 190 insertions(+), 245 deletions(-) > Applied to drm-misc-next. Noralf.
Hi Am 07.04.19 um 18:52 schrieb Noralf Trønnes: > It is generic code and having it in the helper will let other drivers > benefit from it. > > One change was necessary assuming this to be true: > INTEL_INFO(dev_priv)->num_pipes == dev->mode_config.num_crtc > > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> > Cc: intel-gfx@lists.freedesktop.org > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > Reviewed-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/drm_fb_helper.c | 194 ++++++++++++++++++++++++- > drivers/gpu/drm/i915/intel_fbdev.c | 218 ----------------------------- > include/drm/drm_fb_helper.h | 23 --- > 3 files changed, 190 insertions(+), 245 deletions(-) > > diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c > index a6be09ae899b..eda8b63f350d 100644 > --- a/drivers/gpu/drm/drm_fb_helper.c > +++ b/drivers/gpu/drm/drm_fb_helper.c > @@ -2556,6 +2556,194 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper, > fb_helper->sw_rotations |= DRM_MODE_ROTATE_0; > } > > +static struct drm_fb_helper_crtc * > +drm_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc) > +{ > + int i; > + > + for (i = 0; i < fb_helper->crtc_count; i++) > + if (fb_helper->crtc_info[i].mode_set.crtc == crtc) > + return &fb_helper->crtc_info[i]; > + > + return NULL; > +} > + > +/* Try to read the BIOS display configuration and use it for the initial config */ > +static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper, > + struct drm_fb_helper_crtc **crtcs, > + struct drm_display_mode **modes, > + struct drm_fb_offset *offsets, > + bool *enabled, int width, int height) > +{ > + struct drm_device *dev = fb_helper->dev; > + unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG); > + unsigned long conn_configured, conn_seq; > + int i, j; > + bool *save_enabled; > + bool fallback = true, ret = true; > + int num_connectors_enabled = 0; > + int num_connectors_detected = 0; > + struct drm_modeset_acquire_ctx ctx; > + > + save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL); > + if (!save_enabled) > + return false; > + > + drm_modeset_acquire_init(&ctx, 0); > + > + while (drm_modeset_lock_all_ctx(dev, &ctx) != 0) > + drm_modeset_backoff(&ctx); > + > + memcpy(save_enabled, enabled, count); > + conn_seq = GENMASK(count - 1, 0); > + conn_configured = 0; > +retry: > + for (i = 0; i < count; i++) { > + struct drm_fb_helper_connector *fb_conn; > + struct drm_connector *connector; > + struct drm_encoder *encoder; > + struct drm_fb_helper_crtc *new_crtc; > + > + fb_conn = fb_helper->connector_info[i]; > + connector = fb_conn->connector; > + > + if (conn_configured & BIT(i)) > + continue; > + > + /* First pass, only consider tiled connectors */ > + if (conn_seq == GENMASK(count - 1, 0) && !connector->has_tile) > + continue; > + > + if (connector->status == connector_status_connected) > + num_connectors_detected++; > + > + if (!enabled[i]) { > + DRM_DEBUG_KMS("connector %s not enabled, skipping\n", > + connector->name); > + conn_configured |= BIT(i); > + continue; > + } > + > + if (connector->force == DRM_FORCE_OFF) { > + DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n", > + connector->name); > + enabled[i] = false; > + continue; > + } > + > + encoder = connector->state->best_encoder; This patch breaks ast right here. Ast is a non-atomic driver and has connector->state set to NULL. Reading best_encoder gives a NULL-pointer dereference and the display turns black. Stack trace is below. Best regards Thomas [ 29.583012] [drm:drm_fb_helper_firmware_config.isra.37 [drm_kms_helper]] *ERROR* drivers/gpu/drm/drm_fb_helper.c:2649 connector=00000000db73d0b1 [ 29.583048] [drm:drm_fb_helper_firmware_config.isra.37 [drm_kms_helper]] *ERROR* drivers/gpu/drm/drm_fb_helper.c:2650 connector->state= (null) 0m] Found device[ 29.609593] BUG: unable to handle kernel NULL pointer dereference at 0000000000000010 [ 29.609594] #PF error: [normal kernel read fault] [ 29.609595] PGD 0 P4D 0 [ 29.609597] Oops: 0000 [#1] SMP PTI [ 29.609599] CPU: 1 PID: 354 Comm: systemd-udevd Tainted: G E 5.1.0-rc5-1-default+ #11 [ 29.609600] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN FIRE X2270 M2, BIOS 2.05 07/01/2010 [ 29.609608] RIP: 0010:drm_fb_helper_firmware_config.isra.37+0x2d6/0x8a0 [drm_kms_helper] [ 29.609609] Code: 48 89 de ba 5a 0a 00 00 48 c7 c7 5b 0b 48 c0 e8 60 d1 17 00 48 8b 85 e0 03 00 00 48 89 de ba 5b 0a 00 00 48 c7 c7 90 ef 47 c0 <48> 8b 48 10 e8 41 d1 17 00 ba 5d 0a 00 00 48 89 de 4c 89 ef 4b [ 29.609610] RSP: 0018:ffffb6f800c3f898 EFLAGS: 00010296 [ 29.609612] RAX: 0000000000000000 RBX: ffffffffc047eec0 RCX: 0000000000000000 [ 29.609612] RDX: 0000000000000a5b RSI: ffffffffc047eec0 RDI: ffffffffc047ef90 [ 29.609613] RBP: ffff9e76b6e42800 R08: 0000000000000002 R09: 0000000000000000 [ 29.609614] R10: 00000000009053e2 R11: 0000000000002ae7 R12: 0000000000000000 [ 29.609615] R13: ffffffffc0480b40 R14: 0000000000000000 R15: ffff9e76b5bcac00 [ 29.609616] FS: 00007f147397ef80(0000) GS:ffff9e76bb640000(0000) knlGS:0000000000000000 [ 29.609617] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 29.609618] CR2: 0000000000000010 CR3: 0000000135bd2002 CR4: 00000000000206e0 [ 29.609619] Call Trace: [ 29.609630] ? drm_helper_probe_single_connector_modes+0x27f/0x680 [drm_kms_helper] [ 29.609640] drm_setup_crtcs+0x431/0xd80 [drm_kms_helper] [ 29.753065] __drm_fb_helper_initial_config_and_unlock+0x6f/0x6a0 [drm_kms_helper] [ 29.753160] ? drm_modeset_unlock_all+0x31/0x50 [drm] ST1000[ 29.765758] ast_fbdev_init+0xa8/0xc0 [ast] [ 29.765762] ast_driver_load.cold.7+0x2b3/0xe11 [ast] [ 29.765775] drm_dev_register+0x111/0x150 [drm] [ 29.765790] drm_get_pci_dev+0x95/0x190 [drm] [ 29.765794] local_pci_probe+0x42/0x80 [ 29.765796] pci_device_probe+0xf4/0x170 [ 29.765800] really_probe+0xf8/0x3b0 [ 29.765802] driver_probe_device+0xb3/0xf0 [ 29.765805] device_driver_attach+0x50/0x60 [ 29.805092] __driver_attach+0x86/0x140 [ 29.805095] ? device_driver_attach+0x60/0x60 [ 29.805138] bus_for_each_dev+0x63/0x90 DM010-2EP102 4 29.817250] bus_add_driver+0x131/0x1e0 [ 29.817252] ? 0xffffffffc03e6000 0m. [ 29.817254] driver_register+0x6b/0xb0 [ 29.817255] ? 0xffffffffc03e6000 [ 29.817258] do_one_initcall+0x46/0x1c8 [ 29.817262] ? __vunmap+0x89/0xc0 [ 29.817265] do_init_module+0x5a/0x210 [ 29.817266] load_module+0x1aff/0x1f70 [ 29.817270] ? __do_sys_finit_module+0x8f/0xd0 [ 29.817271] __do_sys_finit_module+0x8f/0xd0 [ 29.817274] do_syscall_64+0x60/0x110 [ 29.817279] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 29.817280] RIP: 0033:0x7f147428b2f9 [ 29.817282] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 6f 4b 0c 00 f7 d8 64 89 08 [ 29.817282] RSP: 002b:00007fffa8cd4478 EFLAGS: 00000246 ORIG_RAX: 0000000000000139 [ 29.817284] RAX: ffffffffffffffda RBX: 0000561801fe62e0 RCX: 00007f147428b2f9 [ 29.817285] RDX: 0000000000000000 RSI: 00007f147440089d RDI: 0000000000000016 [ 29.817285] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000561801ffcd50 [ 29.817286] R10: 0000000000000016 R11: 0000000000000246 R12: 00007f147440089d [ 29.817287] R13: 0000000000020000 R14: 0000561802006510 R15: 0000561801fe62e0 [ 29.817288] Modules linked in: hid_generic(E) btrfs(E) libcrc32c(E) usbhid(E) xor(E) ast(E+) i2c_algo_bit(E) drm_kms_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) ehci_pci(E) drm_vram_h) [ 29.817300] CR2: 0000000000000010 [ 29.817311] ---[ end trace 640b47541cd2e101 ]--- [ 29.827577] PM: Image not found (code -22) [ 29.830117] RIP: 0010:drm_fb_helper_firmware_config.isra.37+0x2d6/0x8a0 [drm_kms_helper] [ 29.985828] Code: 48 89 de ba 5a 0a 00 00 48 c7 c7 5b 0b 48 c0 e8 60 d1 17 00 48 8b 85 e0 03 00 00 48 89 de ba 5b 0a 00 00 48 c7 c7 90 ef 47 c0 <48> 8b 48 10 e8 41 d1 17 00 ba 5d 0a 00 00 48 89 de 4c 89 ef 4b [ 29.985829] RSP: 0018:ffffb6f800c3f898 EFLAGS: 00010296 [ 29.985830] RAX: 0000000000000000 RBX: ffffffffc047eec0 RCX: 0000000000000000 [ 29.985831] RDX: 0000000000000a5b RSI: ffffffffc047eec0 RDI: ffffffffc047ef90 [ 29.985832] RBP: ffff9e76b6e42800 R08: 0000000000000002 R09: 0000000000000000 [ 29.985834] R10: 00000000009053e2 R11: 0000000000002ae7 R12: 0000000000000000 [ 30.038589] R13: ffffffffc0480b40 R14: 0000000000000000 R15: ffff9e76b5bcac00 [ 30.038590] FS: 00007f147397ef80(0000) GS:ffff9e76bb640000(0000) knlGS:0000000000000000 [ 30.038591] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 30.038592] CR2: 0000000000000010 CR3: 0000000135bd2002 CR4: 00000000000206e0 > + if (!encoder || WARN_ON(!connector->state->crtc)) { > + if (connector->force > DRM_FORCE_OFF) > + goto bail; > + > + DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n", > + connector->name); > + enabled[i] = false; > + conn_configured |= BIT(i); > + continue; > + } > + > + num_connectors_enabled++; > + > + new_crtc = drm_fb_helper_crtc(fb_helper, connector->state->crtc); > + > + /* > + * Make sure we're not trying to drive multiple connectors > + * with a single CRTC, since our cloning support may not > + * match the BIOS. > + */ > + for (j = 0; j < count; j++) { > + if (crtcs[j] == new_crtc) { > + DRM_DEBUG_KMS("fallback: cloned configuration\n"); > + goto bail; > + } > + } > + > + DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n", > + connector->name); > + > + /* go for command line mode first */ > + modes[i] = drm_pick_cmdline_mode(fb_conn); > + > + /* try for preferred next */ > + if (!modes[i]) { > + DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n", > + connector->name, connector->has_tile); > + modes[i] = drm_has_preferred_mode(fb_conn, width, > + height); > + } > + > + /* No preferred mode marked by the EDID? Are there any modes? */ > + if (!modes[i] && !list_empty(&connector->modes)) { > + DRM_DEBUG_KMS("using first mode listed on connector %s\n", > + connector->name); > + modes[i] = list_first_entry(&connector->modes, > + struct drm_display_mode, > + head); > + } > + > + /* last resort: use current mode */ > + if (!modes[i]) { > + /* > + * IMPORTANT: We want to use the adjusted mode (i.e. > + * after the panel fitter upscaling) as the initial > + * config, not the input mode, which is what crtc->mode > + * usually contains. But since our current > + * code puts a mode derived from the post-pfit timings > + * into crtc->mode this works out correctly. > + * > + * This is crtc->mode and not crtc->state->mode for the > + * fastboot check to work correctly. > + */ > + DRM_DEBUG_KMS("looking for current mode on connector %s\n", > + connector->name); > + modes[i] = &connector->state->crtc->mode; > + } > + crtcs[i] = new_crtc; > + > + DRM_DEBUG_KMS("connector %s on [CRTC:%d:%s]: %dx%d%s\n", > + connector->name, > + connector->state->crtc->base.id, > + connector->state->crtc->name, > + modes[i]->hdisplay, modes[i]->vdisplay, > + modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" : ""); > + > + fallback = false; > + conn_configured |= BIT(i); > + } > + > + if (conn_configured != conn_seq) { /* repeat until no more are found */ > + conn_seq = conn_configured; > + goto retry; > + } > + > + /* > + * If the BIOS didn't enable everything it could, fall back to have the > + * same user experiencing of lighting up as much as possible like the > + * fbdev helper library. > + */ > + if (num_connectors_enabled != num_connectors_detected && > + num_connectors_enabled < dev->mode_config.num_crtc) { > + DRM_DEBUG_KMS("fallback: Not all outputs enabled\n"); > + DRM_DEBUG_KMS("Enabled: %i, detected: %i\n", num_connectors_enabled, > + num_connectors_detected); > + fallback = true; > + } > + > + if (fallback) { > +bail: > + DRM_DEBUG_KMS("Not using firmware configuration\n"); > + memcpy(enabled, save_enabled, count); > + ret = false; > + } > + > + drm_modeset_drop_locks(&ctx); > + drm_modeset_acquire_fini(&ctx); > + > + kfree(save_enabled); > + return ret; > +} > + > static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, > u32 width, u32 height) > { > @@ -2588,10 +2776,8 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, > DRM_DEBUG_KMS("No connectors reported connected with modes\n"); > drm_enable_connectors(fb_helper, enabled); > > - if (!(fb_helper->funcs->initial_config && > - fb_helper->funcs->initial_config(fb_helper, crtcs, modes, > - offsets, > - enabled, width, height))) { > + if (!drm_fb_helper_firmware_config(fb_helper, crtcs, modes, offsets, > + enabled, width, height)) { > memset(modes, 0, fb_helper->connector_count*sizeof(modes[0])); > memset(crtcs, 0, fb_helper->connector_count*sizeof(crtcs[0])); > memset(offsets, 0, fb_helper->connector_count*sizeof(offsets[0])); > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index ef93c27e60b4..c4d17dda3355 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -284,225 +284,7 @@ static int intelfb_create(struct drm_fb_helper *helper, > return ret; > } > > -static struct drm_fb_helper_crtc * > -intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc) > -{ > - int i; > - > - for (i = 0; i < fb_helper->crtc_count; i++) > - if (fb_helper->crtc_info[i].mode_set.crtc == crtc) > - return &fb_helper->crtc_info[i]; > - > - return NULL; > -} > - > -/* > - * Try to read the BIOS display configuration and use it for the initial > - * fb configuration. > - * > - * The BIOS or boot loader will generally create an initial display > - * configuration for us that includes some set of active pipes and displays. > - * This routine tries to figure out which pipes and connectors are active > - * and stuffs them into the crtcs and modes array given to us by the > - * drm_fb_helper code. > - * > - * The overall sequence is: > - * intel_fbdev_init - from driver load > - * intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data > - * drm_fb_helper_init - build fb helper structs > - * drm_fb_helper_single_add_all_connectors - more fb helper structs > - * intel_fbdev_initial_config - apply the config > - * drm_fb_helper_initial_config - call ->probe then register_framebuffer() > - * drm_setup_crtcs - build crtc config for fbdev > - * intel_fb_initial_config - find active connectors etc > - * drm_fb_helper_single_fb_probe - set up fbdev > - * intelfb_create - re-use or alloc fb, build out fbdev structs > - * > - * Note that we don't make special consideration whether we could actually > - * switch to the selected modes without a full modeset. E.g. when the display > - * is in VGA mode we need to recalculate watermarks and set a new high-res > - * framebuffer anyway. > - */ > -static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, > - struct drm_fb_helper_crtc **crtcs, > - struct drm_display_mode **modes, > - struct drm_fb_offset *offsets, > - bool *enabled, int width, int height) > -{ > - struct drm_i915_private *dev_priv = to_i915(fb_helper->dev); > - unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG); > - unsigned long conn_configured, conn_seq; > - int i, j; > - bool *save_enabled; > - bool fallback = true, ret = true; > - int num_connectors_enabled = 0; > - int num_connectors_detected = 0; > - struct drm_modeset_acquire_ctx ctx; > - > - save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL); > - if (!save_enabled) > - return false; > - > - drm_modeset_acquire_init(&ctx, 0); > - > - while (drm_modeset_lock_all_ctx(fb_helper->dev, &ctx) != 0) > - drm_modeset_backoff(&ctx); > - > - memcpy(save_enabled, enabled, count); > - conn_seq = GENMASK(count - 1, 0); > - conn_configured = 0; > -retry: > - for (i = 0; i < count; i++) { > - struct drm_fb_helper_connector *fb_conn; > - struct drm_connector *connector; > - struct drm_encoder *encoder; > - struct drm_fb_helper_crtc *new_crtc; > - > - fb_conn = fb_helper->connector_info[i]; > - connector = fb_conn->connector; > - > - if (conn_configured & BIT(i)) > - continue; > - > - /* First pass, only consider tiled connectors */ > - if (conn_seq == GENMASK(count - 1, 0) && !connector->has_tile) > - continue; > - > - if (connector->status == connector_status_connected) > - num_connectors_detected++; > - > - if (!enabled[i]) { > - DRM_DEBUG_KMS("connector %s not enabled, skipping\n", > - connector->name); > - conn_configured |= BIT(i); > - continue; > - } > - > - if (connector->force == DRM_FORCE_OFF) { > - DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n", > - connector->name); > - enabled[i] = false; > - continue; > - } > - > - encoder = connector->state->best_encoder; > - if (!encoder || WARN_ON(!connector->state->crtc)) { > - if (connector->force > DRM_FORCE_OFF) > - goto bail; > - > - DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n", > - connector->name); > - enabled[i] = false; > - conn_configured |= BIT(i); > - continue; > - } > - > - num_connectors_enabled++; > - > - new_crtc = intel_fb_helper_crtc(fb_helper, > - connector->state->crtc); > - > - /* > - * Make sure we're not trying to drive multiple connectors > - * with a single CRTC, since our cloning support may not > - * match the BIOS. > - */ > - for (j = 0; j < count; j++) { > - if (crtcs[j] == new_crtc) { > - DRM_DEBUG_KMS("fallback: cloned configuration\n"); > - goto bail; > - } > - } > - > - DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n", > - connector->name); > - > - /* go for command line mode first */ > - modes[i] = drm_pick_cmdline_mode(fb_conn); > - > - /* try for preferred next */ > - if (!modes[i]) { > - DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n", > - connector->name, connector->has_tile); > - modes[i] = drm_has_preferred_mode(fb_conn, width, > - height); > - } > - > - /* No preferred mode marked by the EDID? Are there any modes? */ > - if (!modes[i] && !list_empty(&connector->modes)) { > - DRM_DEBUG_KMS("using first mode listed on connector %s\n", > - connector->name); > - modes[i] = list_first_entry(&connector->modes, > - struct drm_display_mode, > - head); > - } > - > - /* last resort: use current mode */ > - if (!modes[i]) { > - /* > - * IMPORTANT: We want to use the adjusted mode (i.e. > - * after the panel fitter upscaling) as the initial > - * config, not the input mode, which is what crtc->mode > - * usually contains. But since our current > - * code puts a mode derived from the post-pfit timings > - * into crtc->mode this works out correctly. > - * > - * This is crtc->mode and not crtc->state->mode for the > - * fastboot check to work correctly. crtc_state->mode has > - * I915_MODE_FLAG_INHERITED, which we clear to force check > - * state. > - */ > - DRM_DEBUG_KMS("looking for current mode on connector %s\n", > - connector->name); > - modes[i] = &connector->state->crtc->mode; > - } > - crtcs[i] = new_crtc; > - > - DRM_DEBUG_KMS("connector %s on [CRTC:%d:%s]: %dx%d%s\n", > - connector->name, > - connector->state->crtc->base.id, > - connector->state->crtc->name, > - modes[i]->hdisplay, modes[i]->vdisplay, > - modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" :""); > - > - fallback = false; > - conn_configured |= BIT(i); > - } > - > - if (conn_configured != conn_seq) { /* repeat until no more are found */ > - conn_seq = conn_configured; > - goto retry; > - } > - > - /* > - * If the BIOS didn't enable everything it could, fall back to have the > - * same user experiencing of lighting up as much as possible like the > - * fbdev helper library. > - */ > - if (num_connectors_enabled != num_connectors_detected && > - num_connectors_enabled < INTEL_INFO(dev_priv)->num_pipes) { > - DRM_DEBUG_KMS("fallback: Not all outputs enabled\n"); > - DRM_DEBUG_KMS("Enabled: %i, detected: %i\n", num_connectors_enabled, > - num_connectors_detected); > - fallback = true; > - } > - > - if (fallback) { > -bail: > - DRM_DEBUG_KMS("Not using firmware configuration\n"); > - memcpy(enabled, save_enabled, count); > - ret = false; > - } > - > - drm_modeset_drop_locks(&ctx); > - drm_modeset_acquire_fini(&ctx); > - > - kfree(save_enabled); > - return ret; > -} > - > static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { > - .initial_config = intel_fb_initial_config, > .fb_probe = intelfb_create, > }; > > diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h > index 81ae48a0df48..4db7de19a491 100644 > --- a/include/drm/drm_fb_helper.h > +++ b/include/drm/drm_fb_helper.h > @@ -101,29 +101,6 @@ struct drm_fb_helper_funcs { > */ > int (*fb_probe)(struct drm_fb_helper *helper, > struct drm_fb_helper_surface_size *sizes); > - > - /** > - * @initial_config: > - * > - * Driver callback to setup an initial fbdev display configuration. > - * Drivers can use this callback to tell the fbdev emulation what the > - * preferred initial configuration is. This is useful to implement > - * smooth booting where the fbdev (and subsequently all userspace) never > - * changes the mode, but always inherits the existing configuration. > - * > - * This callback is optional. > - * > - * RETURNS: > - * > - * The driver should return true if a suitable initial configuration has > - * been filled out and false when the fbdev helper should fall back to > - * the default probing logic. > - */ > - bool (*initial_config)(struct drm_fb_helper *fb_helper, > - struct drm_fb_helper_crtc **crtcs, > - struct drm_display_mode **modes, > - struct drm_fb_offset *offsets, > - bool *enabled, int width, int height); > }; > > struct drm_fb_helper_connector { >
Den 23.04.2019 16.17, skrev Thomas Zimmermann: > Hi > > Am 07.04.19 um 18:52 schrieb Noralf Trønnes: >> It is generic code and having it in the helper will let other drivers >> benefit from it. >> >> One change was necessary assuming this to be true: >> INTEL_INFO(dev_priv)->num_pipes == dev->mode_config.num_crtc >> >> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com> >> Cc: intel-gfx@lists.freedesktop.org >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> >> Reviewed-by: Jani Nikula <jani.nikula@intel.com> >> --- >> drivers/gpu/drm/drm_fb_helper.c | 194 ++++++++++++++++++++++++- >> drivers/gpu/drm/i915/intel_fbdev.c | 218 ----------------------------- >> include/drm/drm_fb_helper.h | 23 --- >> 3 files changed, 190 insertions(+), 245 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c >> index a6be09ae899b..eda8b63f350d 100644 >> --- a/drivers/gpu/drm/drm_fb_helper.c >> +++ b/drivers/gpu/drm/drm_fb_helper.c >> @@ -2556,6 +2556,194 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper, >> fb_helper->sw_rotations |= DRM_MODE_ROTATE_0; >> } >> >> +static struct drm_fb_helper_crtc * >> +drm_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc) >> +{ >> + int i; >> + >> + for (i = 0; i < fb_helper->crtc_count; i++) >> + if (fb_helper->crtc_info[i].mode_set.crtc == crtc) >> + return &fb_helper->crtc_info[i]; >> + >> + return NULL; >> +} >> + >> +/* Try to read the BIOS display configuration and use it for the initial config */ >> +static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper, >> + struct drm_fb_helper_crtc **crtcs, >> + struct drm_display_mode **modes, >> + struct drm_fb_offset *offsets, >> + bool *enabled, int width, int height) >> +{ >> + struct drm_device *dev = fb_helper->dev; >> + unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG); >> + unsigned long conn_configured, conn_seq; >> + int i, j; >> + bool *save_enabled; >> + bool fallback = true, ret = true; >> + int num_connectors_enabled = 0; >> + int num_connectors_detected = 0; >> + struct drm_modeset_acquire_ctx ctx; >> + >> + save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL); >> + if (!save_enabled) >> + return false; >> + >> + drm_modeset_acquire_init(&ctx, 0); >> + >> + while (drm_modeset_lock_all_ctx(dev, &ctx) != 0) >> + drm_modeset_backoff(&ctx); >> + >> + memcpy(save_enabled, enabled, count); >> + conn_seq = GENMASK(count - 1, 0); >> + conn_configured = 0; >> +retry: >> + for (i = 0; i < count; i++) { >> + struct drm_fb_helper_connector *fb_conn; >> + struct drm_connector *connector; >> + struct drm_encoder *encoder; >> + struct drm_fb_helper_crtc *new_crtc; >> + >> + fb_conn = fb_helper->connector_info[i]; >> + connector = fb_conn->connector; >> + >> + if (conn_configured & BIT(i)) >> + continue; >> + >> + /* First pass, only consider tiled connectors */ >> + if (conn_seq == GENMASK(count - 1, 0) && !connector->has_tile) >> + continue; >> + >> + if (connector->status == connector_status_connected) >> + num_connectors_detected++; >> + >> + if (!enabled[i]) { >> + DRM_DEBUG_KMS("connector %s not enabled, skipping\n", >> + connector->name); >> + conn_configured |= BIT(i); >> + continue; >> + } >> + >> + if (connector->force == DRM_FORCE_OFF) { >> + DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n", >> + connector->name); >> + enabled[i] = false; >> + continue; >> + } >> + >> + encoder = connector->state->best_encoder; > > This patch breaks ast right here. Ast is a non-atomic driver and has > connector->state set to NULL. Reading best_encoder gives a NULL-pointer > dereference and the display turns black. Stack trace is below. > Thanks for the report, I've sent a fix. Noralf. > Best regards > Thomas > > > [ 29.583012] [drm:drm_fb_helper_firmware_config.isra.37 > [drm_kms_helper]] *ERROR* drivers/gpu/drm/drm_fb_helper.c:2649 > connector=00000000db73d0b1 > [ 29.583048] [drm:drm_fb_helper_firmware_config.isra.37 > [drm_kms_helper]] *ERROR* drivers/gpu/drm/drm_fb_helper.c:2650 > connector->state= (null) > 0m] Found device[ 29.609593] BUG: unable to handle kernel NULL pointer > dereference at 0000000000000010 > [ 29.609594] #PF error: [normal kernel read fault] > [ 29.609595] PGD 0 P4D 0 > [ 29.609597] Oops: 0000 [#1] SMP PTI > [ 29.609599] CPU: 1 PID: 354 Comm: systemd-udevd Tainted: G > E 5.1.0-rc5-1-default+ #11 > [ 29.609600] Hardware name: Sun Microsystems SUN FIRE X2270 M2/SUN > FIRE X2270 M2, BIOS 2.05 07/01/2010 > [ 29.609608] RIP: > 0010:drm_fb_helper_firmware_config.isra.37+0x2d6/0x8a0 [drm_kms_helper] > [ 29.609609] Code: 48 89 de ba 5a 0a 00 00 48 c7 c7 5b 0b 48 c0 e8 60 > d1 17 00 48 8b 85 e0 03 00 00 48 89 de ba 5b 0a 00 00 48 c7 c7 90 ef 47 > c0 <48> 8b 48 10 e8 41 d1 17 00 ba 5d 0a 00 00 48 89 de 4c 89 ef 4b > [ 29.609610] RSP: 0018:ffffb6f800c3f898 EFLAGS: 00010296 > [ 29.609612] RAX: 0000000000000000 RBX: ffffffffc047eec0 RCX: > 0000000000000000 > [ 29.609612] RDX: 0000000000000a5b RSI: ffffffffc047eec0 RDI: > ffffffffc047ef90 > [ 29.609613] RBP: ffff9e76b6e42800 R08: 0000000000000002 R09: > 0000000000000000 > [ 29.609614] R10: 00000000009053e2 R11: 0000000000002ae7 R12: > 0000000000000000 > [ 29.609615] R13: ffffffffc0480b40 R14: 0000000000000000 R15: > ffff9e76b5bcac00 > [ 29.609616] FS: 00007f147397ef80(0000) GS:ffff9e76bb640000(0000) > knlGS:0000000000000000 > [ 29.609617] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 29.609618] CR2: 0000000000000010 CR3: 0000000135bd2002 CR4: > 00000000000206e0 > [ 29.609619] Call Trace: > [ 29.609630] ? drm_helper_probe_single_connector_modes+0x27f/0x680 > [drm_kms_helper] > [ 29.609640] drm_setup_crtcs+0x431/0xd80 [drm_kms_helper] > [ 29.753065] __drm_fb_helper_initial_config_and_unlock+0x6f/0x6a0 > [drm_kms_helper] > [ 29.753160] ? drm_modeset_unlock_all+0x31/0x50 [drm] > ST1000[ 29.765758] ast_fbdev_init+0xa8/0xc0 [ast] > [ 29.765762] ast_driver_load.cold.7+0x2b3/0xe11 [ast] > [ 29.765775] drm_dev_register+0x111/0x150 [drm] > [ 29.765790] drm_get_pci_dev+0x95/0x190 [drm] > [ 29.765794] local_pci_probe+0x42/0x80 > [ 29.765796] pci_device_probe+0xf4/0x170 > [ 29.765800] really_probe+0xf8/0x3b0 > [ 29.765802] driver_probe_device+0xb3/0xf0 > [ 29.765805] device_driver_attach+0x50/0x60 > [ 29.805092] __driver_attach+0x86/0x140 > [ 29.805095] ? device_driver_attach+0x60/0x60 > [ 29.805138] bus_for_each_dev+0x63/0x90 > DM010-2EP102 4 29.817250] bus_add_driver+0x131/0x1e0 > [ 29.817252] ? 0xffffffffc03e6000 > 0m. > [ 29.817254] driver_register+0x6b/0xb0 > [ 29.817255] ? 0xffffffffc03e6000 > [ 29.817258] do_one_initcall+0x46/0x1c8 > [ 29.817262] ? __vunmap+0x89/0xc0 > [ 29.817265] do_init_module+0x5a/0x210 > [ 29.817266] load_module+0x1aff/0x1f70 > [ 29.817270] ? __do_sys_finit_module+0x8f/0xd0 > [ 29.817271] __do_sys_finit_module+0x8f/0xd0 > [ 29.817274] do_syscall_64+0x60/0x110 > [ 29.817279] entry_SYSCALL_64_after_hwframe+0x49/0xbe > [ 29.817280] RIP: 0033:0x7f147428b2f9 > [ 29.817282] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 > 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f > 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 6f 4b 0c 00 f7 d8 64 89 08 > [ 29.817282] RSP: 002b:00007fffa8cd4478 EFLAGS: 00000246 ORIG_RAX: > 0000000000000139 > [ 29.817284] RAX: ffffffffffffffda RBX: 0000561801fe62e0 RCX: > 00007f147428b2f9 > [ 29.817285] RDX: 0000000000000000 RSI: 00007f147440089d RDI: > 0000000000000016 > [ 29.817285] RBP: 0000000000000000 R08: 0000000000000000 R09: > 0000561801ffcd50 > [ 29.817286] R10: 0000000000000016 R11: 0000000000000246 R12: > 00007f147440089d > [ 29.817287] R13: 0000000000020000 R14: 0000561802006510 R15: > 0000561801fe62e0 > [ 29.817288] Modules linked in: hid_generic(E) btrfs(E) libcrc32c(E) > usbhid(E) xor(E) ast(E+) i2c_algo_bit(E) drm_kms_helper(E) > syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) ehci_pci(E) > drm_vram_h) > [ 29.817300] CR2: 0000000000000010 > [ 29.817311] ---[ end trace 640b47541cd2e101 ]--- > [ 29.827577] PM: Image not found (code -22) > [ 29.830117] RIP: > 0010:drm_fb_helper_firmware_config.isra.37+0x2d6/0x8a0 [drm_kms_helper] > [ 29.985828] Code: 48 89 de ba 5a 0a 00 00 48 c7 c7 5b 0b 48 c0 e8 60 > d1 17 00 48 8b 85 e0 03 00 00 48 89 de ba 5b 0a 00 00 48 c7 c7 90 ef 47 > c0 <48> 8b 48 10 e8 41 d1 17 00 ba 5d 0a 00 00 48 89 de 4c 89 ef 4b > [ 29.985829] RSP: 0018:ffffb6f800c3f898 EFLAGS: 00010296 > [ 29.985830] RAX: 0000000000000000 RBX: ffffffffc047eec0 RCX: > 0000000000000000 > [ 29.985831] RDX: 0000000000000a5b RSI: ffffffffc047eec0 RDI: > ffffffffc047ef90 > [ 29.985832] RBP: ffff9e76b6e42800 R08: 0000000000000002 R09: > 0000000000000000 > [ 29.985834] R10: 00000000009053e2 R11: 0000000000002ae7 R12: > 0000000000000000 > [ 30.038589] R13: ffffffffc0480b40 R14: 0000000000000000 R15: > ffff9e76b5bcac00 > [ 30.038590] FS: 00007f147397ef80(0000) GS:ffff9e76bb640000(0000) > knlGS:0000000000000000 > [ 30.038591] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 30.038592] CR2: 0000000000000010 CR3: 0000000135bd2002 CR4: > 00000000000206e0 > > > >> + if (!encoder || WARN_ON(!connector->state->crtc)) { >> + if (connector->force > DRM_FORCE_OFF) >> + goto bail; >> + >> + DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n", >> + connector->name); >> + enabled[i] = false; >> + conn_configured |= BIT(i); >> + continue; >> + } >> + >> + num_connectors_enabled++; >> + >> + new_crtc = drm_fb_helper_crtc(fb_helper, connector->state->crtc); >> + >> + /* >> + * Make sure we're not trying to drive multiple connectors >> + * with a single CRTC, since our cloning support may not >> + * match the BIOS. >> + */ >> + for (j = 0; j < count; j++) { >> + if (crtcs[j] == new_crtc) { >> + DRM_DEBUG_KMS("fallback: cloned configuration\n"); >> + goto bail; >> + } >> + } >> + >> + DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n", >> + connector->name); >> + >> + /* go for command line mode first */ >> + modes[i] = drm_pick_cmdline_mode(fb_conn); >> + >> + /* try for preferred next */ >> + if (!modes[i]) { >> + DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n", >> + connector->name, connector->has_tile); >> + modes[i] = drm_has_preferred_mode(fb_conn, width, >> + height); >> + } >> + >> + /* No preferred mode marked by the EDID? Are there any modes? */ >> + if (!modes[i] && !list_empty(&connector->modes)) { >> + DRM_DEBUG_KMS("using first mode listed on connector %s\n", >> + connector->name); >> + modes[i] = list_first_entry(&connector->modes, >> + struct drm_display_mode, >> + head); >> + } >> + >> + /* last resort: use current mode */ >> + if (!modes[i]) { >> + /* >> + * IMPORTANT: We want to use the adjusted mode (i.e. >> + * after the panel fitter upscaling) as the initial >> + * config, not the input mode, which is what crtc->mode >> + * usually contains. But since our current >> + * code puts a mode derived from the post-pfit timings >> + * into crtc->mode this works out correctly. >> + * >> + * This is crtc->mode and not crtc->state->mode for the >> + * fastboot check to work correctly. >> + */ >> + DRM_DEBUG_KMS("looking for current mode on connector %s\n", >> + connector->name); >> + modes[i] = &connector->state->crtc->mode; >> + } >> + crtcs[i] = new_crtc; >> + >> + DRM_DEBUG_KMS("connector %s on [CRTC:%d:%s]: %dx%d%s\n", >> + connector->name, >> + connector->state->crtc->base.id, >> + connector->state->crtc->name, >> + modes[i]->hdisplay, modes[i]->vdisplay, >> + modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" : ""); >> + >> + fallback = false; >> + conn_configured |= BIT(i); >> + } >> + >> + if (conn_configured != conn_seq) { /* repeat until no more are found */ >> + conn_seq = conn_configured; >> + goto retry; >> + } >> + >> + /* >> + * If the BIOS didn't enable everything it could, fall back to have the >> + * same user experiencing of lighting up as much as possible like the >> + * fbdev helper library. >> + */ >> + if (num_connectors_enabled != num_connectors_detected && >> + num_connectors_enabled < dev->mode_config.num_crtc) { >> + DRM_DEBUG_KMS("fallback: Not all outputs enabled\n"); >> + DRM_DEBUG_KMS("Enabled: %i, detected: %i\n", num_connectors_enabled, >> + num_connectors_detected); >> + fallback = true; >> + } >> + >> + if (fallback) { >> +bail: >> + DRM_DEBUG_KMS("Not using firmware configuration\n"); >> + memcpy(enabled, save_enabled, count); >> + ret = false; >> + } >> + >> + drm_modeset_drop_locks(&ctx); >> + drm_modeset_acquire_fini(&ctx); >> + >> + kfree(save_enabled); >> + return ret; >> +} >> + >> static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, >> u32 width, u32 height) >> { >> @@ -2588,10 +2776,8 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, >> DRM_DEBUG_KMS("No connectors reported connected with modes\n"); >> drm_enable_connectors(fb_helper, enabled); >> >> - if (!(fb_helper->funcs->initial_config && >> - fb_helper->funcs->initial_config(fb_helper, crtcs, modes, >> - offsets, >> - enabled, width, height))) { >> + if (!drm_fb_helper_firmware_config(fb_helper, crtcs, modes, offsets, >> + enabled, width, height)) { >> memset(modes, 0, fb_helper->connector_count*sizeof(modes[0])); >> memset(crtcs, 0, fb_helper->connector_count*sizeof(crtcs[0])); >> memset(offsets, 0, fb_helper->connector_count*sizeof(offsets[0])); >> diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c >> index ef93c27e60b4..c4d17dda3355 100644 >> --- a/drivers/gpu/drm/i915/intel_fbdev.c >> +++ b/drivers/gpu/drm/i915/intel_fbdev.c >> @@ -284,225 +284,7 @@ static int intelfb_create(struct drm_fb_helper *helper, >> return ret; >> } >> >> -static struct drm_fb_helper_crtc * >> -intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc) >> -{ >> - int i; >> - >> - for (i = 0; i < fb_helper->crtc_count; i++) >> - if (fb_helper->crtc_info[i].mode_set.crtc == crtc) >> - return &fb_helper->crtc_info[i]; >> - >> - return NULL; >> -} >> - >> -/* >> - * Try to read the BIOS display configuration and use it for the initial >> - * fb configuration. >> - * >> - * The BIOS or boot loader will generally create an initial display >> - * configuration for us that includes some set of active pipes and displays. >> - * This routine tries to figure out which pipes and connectors are active >> - * and stuffs them into the crtcs and modes array given to us by the >> - * drm_fb_helper code. >> - * >> - * The overall sequence is: >> - * intel_fbdev_init - from driver load >> - * intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data >> - * drm_fb_helper_init - build fb helper structs >> - * drm_fb_helper_single_add_all_connectors - more fb helper structs >> - * intel_fbdev_initial_config - apply the config >> - * drm_fb_helper_initial_config - call ->probe then register_framebuffer() >> - * drm_setup_crtcs - build crtc config for fbdev >> - * intel_fb_initial_config - find active connectors etc >> - * drm_fb_helper_single_fb_probe - set up fbdev >> - * intelfb_create - re-use or alloc fb, build out fbdev structs >> - * >> - * Note that we don't make special consideration whether we could actually >> - * switch to the selected modes without a full modeset. E.g. when the display >> - * is in VGA mode we need to recalculate watermarks and set a new high-res >> - * framebuffer anyway. >> - */ >> -static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, >> - struct drm_fb_helper_crtc **crtcs, >> - struct drm_display_mode **modes, >> - struct drm_fb_offset *offsets, >> - bool *enabled, int width, int height) >> -{ >> - struct drm_i915_private *dev_priv = to_i915(fb_helper->dev); >> - unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG); >> - unsigned long conn_configured, conn_seq; >> - int i, j; >> - bool *save_enabled; >> - bool fallback = true, ret = true; >> - int num_connectors_enabled = 0; >> - int num_connectors_detected = 0; >> - struct drm_modeset_acquire_ctx ctx; >> - >> - save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL); >> - if (!save_enabled) >> - return false; >> - >> - drm_modeset_acquire_init(&ctx, 0); >> - >> - while (drm_modeset_lock_all_ctx(fb_helper->dev, &ctx) != 0) >> - drm_modeset_backoff(&ctx); >> - >> - memcpy(save_enabled, enabled, count); >> - conn_seq = GENMASK(count - 1, 0); >> - conn_configured = 0; >> -retry: >> - for (i = 0; i < count; i++) { >> - struct drm_fb_helper_connector *fb_conn; >> - struct drm_connector *connector; >> - struct drm_encoder *encoder; >> - struct drm_fb_helper_crtc *new_crtc; >> - >> - fb_conn = fb_helper->connector_info[i]; >> - connector = fb_conn->connector; >> - >> - if (conn_configured & BIT(i)) >> - continue; >> - >> - /* First pass, only consider tiled connectors */ >> - if (conn_seq == GENMASK(count - 1, 0) && !connector->has_tile) >> - continue; >> - >> - if (connector->status == connector_status_connected) >> - num_connectors_detected++; >> - >> - if (!enabled[i]) { >> - DRM_DEBUG_KMS("connector %s not enabled, skipping\n", >> - connector->name); >> - conn_configured |= BIT(i); >> - continue; >> - } >> - >> - if (connector->force == DRM_FORCE_OFF) { >> - DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n", >> - connector->name); >> - enabled[i] = false; >> - continue; >> - } >> - >> - encoder = connector->state->best_encoder; >> - if (!encoder || WARN_ON(!connector->state->crtc)) { >> - if (connector->force > DRM_FORCE_OFF) >> - goto bail; >> - >> - DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n", >> - connector->name); >> - enabled[i] = false; >> - conn_configured |= BIT(i); >> - continue; >> - } >> - >> - num_connectors_enabled++; >> - >> - new_crtc = intel_fb_helper_crtc(fb_helper, >> - connector->state->crtc); >> - >> - /* >> - * Make sure we're not trying to drive multiple connectors >> - * with a single CRTC, since our cloning support may not >> - * match the BIOS. >> - */ >> - for (j = 0; j < count; j++) { >> - if (crtcs[j] == new_crtc) { >> - DRM_DEBUG_KMS("fallback: cloned configuration\n"); >> - goto bail; >> - } >> - } >> - >> - DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n", >> - connector->name); >> - >> - /* go for command line mode first */ >> - modes[i] = drm_pick_cmdline_mode(fb_conn); >> - >> - /* try for preferred next */ >> - if (!modes[i]) { >> - DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n", >> - connector->name, connector->has_tile); >> - modes[i] = drm_has_preferred_mode(fb_conn, width, >> - height); >> - } >> - >> - /* No preferred mode marked by the EDID? Are there any modes? */ >> - if (!modes[i] && !list_empty(&connector->modes)) { >> - DRM_DEBUG_KMS("using first mode listed on connector %s\n", >> - connector->name); >> - modes[i] = list_first_entry(&connector->modes, >> - struct drm_display_mode, >> - head); >> - } >> - >> - /* last resort: use current mode */ >> - if (!modes[i]) { >> - /* >> - * IMPORTANT: We want to use the adjusted mode (i.e. >> - * after the panel fitter upscaling) as the initial >> - * config, not the input mode, which is what crtc->mode >> - * usually contains. But since our current >> - * code puts a mode derived from the post-pfit timings >> - * into crtc->mode this works out correctly. >> - * >> - * This is crtc->mode and not crtc->state->mode for the >> - * fastboot check to work correctly. crtc_state->mode has >> - * I915_MODE_FLAG_INHERITED, which we clear to force check >> - * state. >> - */ >> - DRM_DEBUG_KMS("looking for current mode on connector %s\n", >> - connector->name); >> - modes[i] = &connector->state->crtc->mode; >> - } >> - crtcs[i] = new_crtc; >> - >> - DRM_DEBUG_KMS("connector %s on [CRTC:%d:%s]: %dx%d%s\n", >> - connector->name, >> - connector->state->crtc->base.id, >> - connector->state->crtc->name, >> - modes[i]->hdisplay, modes[i]->vdisplay, >> - modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" :""); >> - >> - fallback = false; >> - conn_configured |= BIT(i); >> - } >> - >> - if (conn_configured != conn_seq) { /* repeat until no more are found */ >> - conn_seq = conn_configured; >> - goto retry; >> - } >> - >> - /* >> - * If the BIOS didn't enable everything it could, fall back to have the >> - * same user experiencing of lighting up as much as possible like the >> - * fbdev helper library. >> - */ >> - if (num_connectors_enabled != num_connectors_detected && >> - num_connectors_enabled < INTEL_INFO(dev_priv)->num_pipes) { >> - DRM_DEBUG_KMS("fallback: Not all outputs enabled\n"); >> - DRM_DEBUG_KMS("Enabled: %i, detected: %i\n", num_connectors_enabled, >> - num_connectors_detected); >> - fallback = true; >> - } >> - >> - if (fallback) { >> -bail: >> - DRM_DEBUG_KMS("Not using firmware configuration\n"); >> - memcpy(enabled, save_enabled, count); >> - ret = false; >> - } >> - >> - drm_modeset_drop_locks(&ctx); >> - drm_modeset_acquire_fini(&ctx); >> - >> - kfree(save_enabled); >> - return ret; >> -} >> - >> static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { >> - .initial_config = intel_fb_initial_config, >> .fb_probe = intelfb_create, >> }; >> >> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h >> index 81ae48a0df48..4db7de19a491 100644 >> --- a/include/drm/drm_fb_helper.h >> +++ b/include/drm/drm_fb_helper.h >> @@ -101,29 +101,6 @@ struct drm_fb_helper_funcs { >> */ >> int (*fb_probe)(struct drm_fb_helper *helper, >> struct drm_fb_helper_surface_size *sizes); >> - >> - /** >> - * @initial_config: >> - * >> - * Driver callback to setup an initial fbdev display configuration. >> - * Drivers can use this callback to tell the fbdev emulation what the >> - * preferred initial configuration is. This is useful to implement >> - * smooth booting where the fbdev (and subsequently all userspace) never >> - * changes the mode, but always inherits the existing configuration. >> - * >> - * This callback is optional. >> - * >> - * RETURNS: >> - * >> - * The driver should return true if a suitable initial configuration has >> - * been filled out and false when the fbdev helper should fall back to >> - * the default probing logic. >> - */ >> - bool (*initial_config)(struct drm_fb_helper *fb_helper, >> - struct drm_fb_helper_crtc **crtcs, >> - struct drm_display_mode **modes, >> - struct drm_fb_offset *offsets, >> - bool *enabled, int width, int height); >> }; >> >> struct drm_fb_helper_connector { >> >
diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index a6be09ae899b..eda8b63f350d 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2556,6 +2556,194 @@ static void drm_setup_crtc_rotation(struct drm_fb_helper *fb_helper, fb_helper->sw_rotations |= DRM_MODE_ROTATE_0; } +static struct drm_fb_helper_crtc * +drm_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc) +{ + int i; + + for (i = 0; i < fb_helper->crtc_count; i++) + if (fb_helper->crtc_info[i].mode_set.crtc == crtc) + return &fb_helper->crtc_info[i]; + + return NULL; +} + +/* Try to read the BIOS display configuration and use it for the initial config */ +static bool drm_fb_helper_firmware_config(struct drm_fb_helper *fb_helper, + struct drm_fb_helper_crtc **crtcs, + struct drm_display_mode **modes, + struct drm_fb_offset *offsets, + bool *enabled, int width, int height) +{ + struct drm_device *dev = fb_helper->dev; + unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG); + unsigned long conn_configured, conn_seq; + int i, j; + bool *save_enabled; + bool fallback = true, ret = true; + int num_connectors_enabled = 0; + int num_connectors_detected = 0; + struct drm_modeset_acquire_ctx ctx; + + save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL); + if (!save_enabled) + return false; + + drm_modeset_acquire_init(&ctx, 0); + + while (drm_modeset_lock_all_ctx(dev, &ctx) != 0) + drm_modeset_backoff(&ctx); + + memcpy(save_enabled, enabled, count); + conn_seq = GENMASK(count - 1, 0); + conn_configured = 0; +retry: + for (i = 0; i < count; i++) { + struct drm_fb_helper_connector *fb_conn; + struct drm_connector *connector; + struct drm_encoder *encoder; + struct drm_fb_helper_crtc *new_crtc; + + fb_conn = fb_helper->connector_info[i]; + connector = fb_conn->connector; + + if (conn_configured & BIT(i)) + continue; + + /* First pass, only consider tiled connectors */ + if (conn_seq == GENMASK(count - 1, 0) && !connector->has_tile) + continue; + + if (connector->status == connector_status_connected) + num_connectors_detected++; + + if (!enabled[i]) { + DRM_DEBUG_KMS("connector %s not enabled, skipping\n", + connector->name); + conn_configured |= BIT(i); + continue; + } + + if (connector->force == DRM_FORCE_OFF) { + DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n", + connector->name); + enabled[i] = false; + continue; + } + + encoder = connector->state->best_encoder; + if (!encoder || WARN_ON(!connector->state->crtc)) { + if (connector->force > DRM_FORCE_OFF) + goto bail; + + DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n", + connector->name); + enabled[i] = false; + conn_configured |= BIT(i); + continue; + } + + num_connectors_enabled++; + + new_crtc = drm_fb_helper_crtc(fb_helper, connector->state->crtc); + + /* + * Make sure we're not trying to drive multiple connectors + * with a single CRTC, since our cloning support may not + * match the BIOS. + */ + for (j = 0; j < count; j++) { + if (crtcs[j] == new_crtc) { + DRM_DEBUG_KMS("fallback: cloned configuration\n"); + goto bail; + } + } + + DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n", + connector->name); + + /* go for command line mode first */ + modes[i] = drm_pick_cmdline_mode(fb_conn); + + /* try for preferred next */ + if (!modes[i]) { + DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n", + connector->name, connector->has_tile); + modes[i] = drm_has_preferred_mode(fb_conn, width, + height); + } + + /* No preferred mode marked by the EDID? Are there any modes? */ + if (!modes[i] && !list_empty(&connector->modes)) { + DRM_DEBUG_KMS("using first mode listed on connector %s\n", + connector->name); + modes[i] = list_first_entry(&connector->modes, + struct drm_display_mode, + head); + } + + /* last resort: use current mode */ + if (!modes[i]) { + /* + * IMPORTANT: We want to use the adjusted mode (i.e. + * after the panel fitter upscaling) as the initial + * config, not the input mode, which is what crtc->mode + * usually contains. But since our current + * code puts a mode derived from the post-pfit timings + * into crtc->mode this works out correctly. + * + * This is crtc->mode and not crtc->state->mode for the + * fastboot check to work correctly. + */ + DRM_DEBUG_KMS("looking for current mode on connector %s\n", + connector->name); + modes[i] = &connector->state->crtc->mode; + } + crtcs[i] = new_crtc; + + DRM_DEBUG_KMS("connector %s on [CRTC:%d:%s]: %dx%d%s\n", + connector->name, + connector->state->crtc->base.id, + connector->state->crtc->name, + modes[i]->hdisplay, modes[i]->vdisplay, + modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" : ""); + + fallback = false; + conn_configured |= BIT(i); + } + + if (conn_configured != conn_seq) { /* repeat until no more are found */ + conn_seq = conn_configured; + goto retry; + } + + /* + * If the BIOS didn't enable everything it could, fall back to have the + * same user experiencing of lighting up as much as possible like the + * fbdev helper library. + */ + if (num_connectors_enabled != num_connectors_detected && + num_connectors_enabled < dev->mode_config.num_crtc) { + DRM_DEBUG_KMS("fallback: Not all outputs enabled\n"); + DRM_DEBUG_KMS("Enabled: %i, detected: %i\n", num_connectors_enabled, + num_connectors_detected); + fallback = true; + } + + if (fallback) { +bail: + DRM_DEBUG_KMS("Not using firmware configuration\n"); + memcpy(enabled, save_enabled, count); + ret = false; + } + + drm_modeset_drop_locks(&ctx); + drm_modeset_acquire_fini(&ctx); + + kfree(save_enabled); + return ret; +} + static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, u32 width, u32 height) { @@ -2588,10 +2776,8 @@ static void drm_setup_crtcs(struct drm_fb_helper *fb_helper, DRM_DEBUG_KMS("No connectors reported connected with modes\n"); drm_enable_connectors(fb_helper, enabled); - if (!(fb_helper->funcs->initial_config && - fb_helper->funcs->initial_config(fb_helper, crtcs, modes, - offsets, - enabled, width, height))) { + if (!drm_fb_helper_firmware_config(fb_helper, crtcs, modes, offsets, + enabled, width, height)) { memset(modes, 0, fb_helper->connector_count*sizeof(modes[0])); memset(crtcs, 0, fb_helper->connector_count*sizeof(crtcs[0])); memset(offsets, 0, fb_helper->connector_count*sizeof(offsets[0])); diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index ef93c27e60b4..c4d17dda3355 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -284,225 +284,7 @@ static int intelfb_create(struct drm_fb_helper *helper, return ret; } -static struct drm_fb_helper_crtc * -intel_fb_helper_crtc(struct drm_fb_helper *fb_helper, struct drm_crtc *crtc) -{ - int i; - - for (i = 0; i < fb_helper->crtc_count; i++) - if (fb_helper->crtc_info[i].mode_set.crtc == crtc) - return &fb_helper->crtc_info[i]; - - return NULL; -} - -/* - * Try to read the BIOS display configuration and use it for the initial - * fb configuration. - * - * The BIOS or boot loader will generally create an initial display - * configuration for us that includes some set of active pipes and displays. - * This routine tries to figure out which pipes and connectors are active - * and stuffs them into the crtcs and modes array given to us by the - * drm_fb_helper code. - * - * The overall sequence is: - * intel_fbdev_init - from driver load - * intel_fbdev_init_bios - initialize the intel_fbdev using BIOS data - * drm_fb_helper_init - build fb helper structs - * drm_fb_helper_single_add_all_connectors - more fb helper structs - * intel_fbdev_initial_config - apply the config - * drm_fb_helper_initial_config - call ->probe then register_framebuffer() - * drm_setup_crtcs - build crtc config for fbdev - * intel_fb_initial_config - find active connectors etc - * drm_fb_helper_single_fb_probe - set up fbdev - * intelfb_create - re-use or alloc fb, build out fbdev structs - * - * Note that we don't make special consideration whether we could actually - * switch to the selected modes without a full modeset. E.g. when the display - * is in VGA mode we need to recalculate watermarks and set a new high-res - * framebuffer anyway. - */ -static bool intel_fb_initial_config(struct drm_fb_helper *fb_helper, - struct drm_fb_helper_crtc **crtcs, - struct drm_display_mode **modes, - struct drm_fb_offset *offsets, - bool *enabled, int width, int height) -{ - struct drm_i915_private *dev_priv = to_i915(fb_helper->dev); - unsigned int count = min(fb_helper->connector_count, BITS_PER_LONG); - unsigned long conn_configured, conn_seq; - int i, j; - bool *save_enabled; - bool fallback = true, ret = true; - int num_connectors_enabled = 0; - int num_connectors_detected = 0; - struct drm_modeset_acquire_ctx ctx; - - save_enabled = kcalloc(count, sizeof(bool), GFP_KERNEL); - if (!save_enabled) - return false; - - drm_modeset_acquire_init(&ctx, 0); - - while (drm_modeset_lock_all_ctx(fb_helper->dev, &ctx) != 0) - drm_modeset_backoff(&ctx); - - memcpy(save_enabled, enabled, count); - conn_seq = GENMASK(count - 1, 0); - conn_configured = 0; -retry: - for (i = 0; i < count; i++) { - struct drm_fb_helper_connector *fb_conn; - struct drm_connector *connector; - struct drm_encoder *encoder; - struct drm_fb_helper_crtc *new_crtc; - - fb_conn = fb_helper->connector_info[i]; - connector = fb_conn->connector; - - if (conn_configured & BIT(i)) - continue; - - /* First pass, only consider tiled connectors */ - if (conn_seq == GENMASK(count - 1, 0) && !connector->has_tile) - continue; - - if (connector->status == connector_status_connected) - num_connectors_detected++; - - if (!enabled[i]) { - DRM_DEBUG_KMS("connector %s not enabled, skipping\n", - connector->name); - conn_configured |= BIT(i); - continue; - } - - if (connector->force == DRM_FORCE_OFF) { - DRM_DEBUG_KMS("connector %s is disabled by user, skipping\n", - connector->name); - enabled[i] = false; - continue; - } - - encoder = connector->state->best_encoder; - if (!encoder || WARN_ON(!connector->state->crtc)) { - if (connector->force > DRM_FORCE_OFF) - goto bail; - - DRM_DEBUG_KMS("connector %s has no encoder or crtc, skipping\n", - connector->name); - enabled[i] = false; - conn_configured |= BIT(i); - continue; - } - - num_connectors_enabled++; - - new_crtc = intel_fb_helper_crtc(fb_helper, - connector->state->crtc); - - /* - * Make sure we're not trying to drive multiple connectors - * with a single CRTC, since our cloning support may not - * match the BIOS. - */ - for (j = 0; j < count; j++) { - if (crtcs[j] == new_crtc) { - DRM_DEBUG_KMS("fallback: cloned configuration\n"); - goto bail; - } - } - - DRM_DEBUG_KMS("looking for cmdline mode on connector %s\n", - connector->name); - - /* go for command line mode first */ - modes[i] = drm_pick_cmdline_mode(fb_conn); - - /* try for preferred next */ - if (!modes[i]) { - DRM_DEBUG_KMS("looking for preferred mode on connector %s %d\n", - connector->name, connector->has_tile); - modes[i] = drm_has_preferred_mode(fb_conn, width, - height); - } - - /* No preferred mode marked by the EDID? Are there any modes? */ - if (!modes[i] && !list_empty(&connector->modes)) { - DRM_DEBUG_KMS("using first mode listed on connector %s\n", - connector->name); - modes[i] = list_first_entry(&connector->modes, - struct drm_display_mode, - head); - } - - /* last resort: use current mode */ - if (!modes[i]) { - /* - * IMPORTANT: We want to use the adjusted mode (i.e. - * after the panel fitter upscaling) as the initial - * config, not the input mode, which is what crtc->mode - * usually contains. But since our current - * code puts a mode derived from the post-pfit timings - * into crtc->mode this works out correctly. - * - * This is crtc->mode and not crtc->state->mode for the - * fastboot check to work correctly. crtc_state->mode has - * I915_MODE_FLAG_INHERITED, which we clear to force check - * state. - */ - DRM_DEBUG_KMS("looking for current mode on connector %s\n", - connector->name); - modes[i] = &connector->state->crtc->mode; - } - crtcs[i] = new_crtc; - - DRM_DEBUG_KMS("connector %s on [CRTC:%d:%s]: %dx%d%s\n", - connector->name, - connector->state->crtc->base.id, - connector->state->crtc->name, - modes[i]->hdisplay, modes[i]->vdisplay, - modes[i]->flags & DRM_MODE_FLAG_INTERLACE ? "i" :""); - - fallback = false; - conn_configured |= BIT(i); - } - - if (conn_configured != conn_seq) { /* repeat until no more are found */ - conn_seq = conn_configured; - goto retry; - } - - /* - * If the BIOS didn't enable everything it could, fall back to have the - * same user experiencing of lighting up as much as possible like the - * fbdev helper library. - */ - if (num_connectors_enabled != num_connectors_detected && - num_connectors_enabled < INTEL_INFO(dev_priv)->num_pipes) { - DRM_DEBUG_KMS("fallback: Not all outputs enabled\n"); - DRM_DEBUG_KMS("Enabled: %i, detected: %i\n", num_connectors_enabled, - num_connectors_detected); - fallback = true; - } - - if (fallback) { -bail: - DRM_DEBUG_KMS("Not using firmware configuration\n"); - memcpy(enabled, save_enabled, count); - ret = false; - } - - drm_modeset_drop_locks(&ctx); - drm_modeset_acquire_fini(&ctx); - - kfree(save_enabled); - return ret; -} - static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { - .initial_config = intel_fb_initial_config, .fb_probe = intelfb_create, }; diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h index 81ae48a0df48..4db7de19a491 100644 --- a/include/drm/drm_fb_helper.h +++ b/include/drm/drm_fb_helper.h @@ -101,29 +101,6 @@ struct drm_fb_helper_funcs { */ int (*fb_probe)(struct drm_fb_helper *helper, struct drm_fb_helper_surface_size *sizes); - - /** - * @initial_config: - * - * Driver callback to setup an initial fbdev display configuration. - * Drivers can use this callback to tell the fbdev emulation what the - * preferred initial configuration is. This is useful to implement - * smooth booting where the fbdev (and subsequently all userspace) never - * changes the mode, but always inherits the existing configuration. - * - * This callback is optional. - * - * RETURNS: - * - * The driver should return true if a suitable initial configuration has - * been filled out and false when the fbdev helper should fall back to - * the default probing logic. - */ - bool (*initial_config)(struct drm_fb_helper *fb_helper, - struct drm_fb_helper_crtc **crtcs, - struct drm_display_mode **modes, - struct drm_fb_offset *offsets, - bool *enabled, int width, int height); }; struct drm_fb_helper_connector {