On 2019-03-14 12:53 p.m., Nicholas Kazlauskas wrote:
> We want DRM planes to be initialized in the following order:
>
> - primary planes
> - overlay planes
> - cursor planes
>
> to support existing userspace expectations for plane z-ordering. This
> means that we also need to register CRTCs after all planes have been
> initialized since overlay planes can be placed on any CRTC.
>
> So the only reason why we have the mode_info->planes list is to
> remember the primary planes for use later when we need to register
> the CRTC.
>
> Overlay planes have no purpose being in this list. DRM will cleanup
> any planes that we've registered for us, so the only planes that need to
> be explicitly cleaned up are the ones that have failed to register.
>
> By dropping the explicit free on every plane in the mode_info->planes
> list this patch also fixes a double-free in the case where we fail to
> initialize only some of the planes.
>
> Cc: Leo Li <sunpeng.li@xxxxxxx>
> Cc: Harry Wentland <harry.wentland@xxxxxxx>
> Signed-off-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx>
Reviewed-by: Harry Wentland <harry.wentland@xxxxxxx>
Harry
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index cde0da3ae964..f770de36121f 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -1818,8 +1818,6 @@ static int initialize_plane(struct amdgpu_display_manager *dm,
> int ret = 0;
>
> plane = kzalloc(sizeof(struct drm_plane), GFP_KERNEL);
> - mode_info->planes[plane_id] = plane;
> -
> if (!plane) {
> DRM_ERROR("KMS: Failed to allocate plane\n");
> return -ENOMEM;
> @@ -1836,13 +1834,17 @@ static int initialize_plane(struct amdgpu_display_manager *dm,
> if (plane_id >= dm->dc->caps.max_streams)
> possible_crtcs = 0xff;
>
> - ret = amdgpu_dm_plane_init(dm, mode_info->planes[plane_id], possible_crtcs);
> + ret = amdgpu_dm_plane_init(dm, plane, possible_crtcs);
>
> if (ret) {
> DRM_ERROR("KMS: Failed to initialize plane\n");
> + kfree(plane);
> return ret;
> }
>
> + if (mode_info)
> + mode_info->planes[plane_id] = plane;
> +
> return ret;
> }
>
> @@ -1885,7 +1887,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
> struct amdgpu_encoder *aencoder = NULL;
> struct amdgpu_mode_info *mode_info = &adev->mode_info;
> uint32_t link_cnt;
> - int32_t overlay_planes, primary_planes, total_planes;
> + int32_t overlay_planes, primary_planes;
> enum dc_connection_type new_connection_type = dc_connection_none;
>
> link_cnt = dm->dc->caps.max_links;
> @@ -1914,9 +1916,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
>
> /* There is one primary plane per CRTC */
> primary_planes = dm->dc->caps.max_streams;
> -
> - total_planes = primary_planes + overlay_planes;
> - ASSERT(total_planes <= AMDGPU_MAX_PLANES);
> + ASSERT(primary_planes <= AMDGPU_MAX_PLANES);
>
> /*
> * Initialize primary planes, implicit planes for legacy IOCTLS.
> @@ -1937,7 +1937,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
> * Order is reversed to match iteration order in atomic check.
> */
> for (i = (overlay_planes - 1); i >= 0; i--) {
> - if (initialize_plane(dm, mode_info, primary_planes + i,
> + if (initialize_plane(dm, NULL, primary_planes + i,
> DRM_PLANE_TYPE_OVERLAY)) {
> DRM_ERROR("KMS: Failed to initialize overlay plane\n");
> goto fail;
> @@ -2041,8 +2041,7 @@ static int amdgpu_dm_initialize_drm_device(struct amdgpu_device *adev)
> fail:
> kfree(aencoder);
> kfree(aconnector);
> - for (i = 0; i < primary_planes; i++)
> - kfree(mode_info->planes[i]);
> +
> return -EINVAL;
> }
>
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
