Re: [RFC PATCH v5 10/10] drm/tiny: add driver for Apple Touch Bars in x86 Macs
From: Thomas Zimmermann <tzimmermann@suse.de>
Date: 2025-02-17 08:04:31
Also in:
dri-devel, lkml
Hi Aditya, the code looks correct overall. There's one place where I think it fails. See below. Am 15.02.25 um 14:43 schrieb Aditya Garg: [...]
quoted hunk ↗ jump to hunk
DEFINE_DRM_GEM_FOPS(appletbdrm_drm_fops);@@ -484,10 +537,38 @@ static const struct drm_driver appletbdrm_drm_driver = {static int appletbdrm_setup_mode_config(struct appletbdrm_device *adev) { struct drm_connector *connector = &adev->connector; + struct drm_plane *primary_plane; + struct drm_crtc *crtc; + struct drm_encoder *encoder; struct drm_device *drm = &adev->drm; struct device *dev = adev->dev; int ret; + primary_plane = &adev->primary_plane; + ret = drm_universal_plane_init(drm, primary_plane, 0, + &appletbdrm_primary_plane_funcs, + appletbdrm_primary_plane_formats, + ARRAY_SIZE(appletbdrm_primary_plane_formats), + NULL, + DRM_PLANE_TYPE_PRIMARY, NULL); + if (ret) + return ret; + drm_plane_helper_add(primary_plane, &appletbdrm_primary_plane_helper_funcs); + + crtc = &adev->crtc; + ret = drm_crtc_init_with_planes(drm, crtc, primary_plane, NULL, + &appletbdrm_crtc_funcs, NULL); + if (ret) + return ret; + drm_crtc_helper_add(crtc, &appletbdrm_crtc_helper_funcs); + + encoder = &adev->encoder; + ret = drm_encoder_init(drm, encoder, &appletbdrm_encoder_funcs, + DRM_MODE_ENCODER_DAC, NULL); + if (ret) + return ret; + encoder->possible_crtcs = drm_crtc_mask(crtc); + ret = drmm_mode_config_init(drm);
Try to do drmm_mode_config_init() first. The initialization of planes, crtcs and encoders requires it. See [1] for how other drivers order these calls.
quoted hunk ↗ jump to hunk
if (ret) return dev_err_probe(dev, ret, "Failed to initialize mode configuration\n");@@ -530,13 +611,13 @@ static int appletbdrm_setup_mode_config(struct appletbdrm_device *adev)if (ret) return dev_err_probe(dev, ret, "Failed to set non-desktop property\n"); - ret = drm_simple_display_pipe_init(drm, &adev->pipe, &appletbdrm_pipe_funcs, - appletbdrm_formats, ARRAY_SIZE(appletbdrm_formats), - NULL, &adev->connector); + ret = drm_connector_attach_encoder(connector, encoder); + if (ret) return dev_err_probe(dev, ret, "Failed to initialize simple display pipe\n"); - drm_plane_enable_fb_damage_clips(&adev->pipe.plane);
+ drm_plane_helper_add(primary_plane, &appletbdrm_primary_plane_helper_funcs);
This line can be removed. You've already set the plane helpers a few lines above.
+ drm_plane_enable_fb_damage_clips(&adev->primary_plane);
And this call should better be done next to the plane init. The code at [1] again gives you an example of the preferable order. Best regards Thomas [1] https://elixir.bootlin.com/linux/v6.13.2/source/drivers/gpu/drm/tiny/bochs.c#L606
drm_mode_config_reset(drm); The commit history having both old and new revisions of the driver is here: https://github.com/AdityaGarg8/apple-touchbar-drv/blob/atomic/usr/src/apple-touchbar-advanced-0.1/appletbdrm.c Thanks Aditya
-- -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Frankenstrasse 146, 90461 Nuernberg, Germany GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman HRB 36809 (AG Nuernberg)