From patchwork Wed Jul 26 10:54:28 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Javier Martinez Canillas X-Patchwork-Id: 126318 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:a985:0:b0:3e4:2afc:c1 with SMTP id t5csp365465vqo; Wed, 26 Jul 2023 05:28:26 -0700 (PDT) X-Google-Smtp-Source: APBJJlGjF+tydvSaM4zSJKWIJOV4FgFytvDAvIlID1eBAW33Ws31D6DoeGq4UI77AdCBBMO4w9OH X-Received: by 2002:a17:906:9bdb:b0:994:5544:3aea with SMTP id de27-20020a1709069bdb00b0099455443aeamr1462058ejc.51.1690374506558; Wed, 26 Jul 2023 05:28:26 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690374506; cv=none; d=google.com; s=arc-20160816; b=jbWXlnWKwi0vpZNgfdzpK43P6nA61ERtgaHZ9IhbPtJO7WUDEW9iqCoz0Y5qti+kxe 6PAPKWBIs9kKW5BNvLV3FNOGllVBFWLFKoKJCCQFGtAexyLqnrP9JKKvWwqrm+/oMi87 flMGA/ytvgwFmqBoGdsjo3lBnwhpq1CY1mjdgc00EdUwMWy3HiS/GbVfSeJ2qtX+169y qAsn3GvXolHKubJuQzwAVsQKVAyIS0JMTudaShvmbvpWCOs+ApeVN9B+3CLH+QkvKvD1 U6TYZD6s0RMaTI18o6EOeMNa+mRiNaF/MWbTI2930Jl6iztfLbjprz9p5A0WwZyuC257 ft0Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=E3wvUr7a8sNlMjkQjrp9y08VnHFKDhRShbNPZvWniXw=; fh=UVCJSeUyPqiC7S2wQ7yZsmFOia5YngsxMZ/PCh/OjNk=; b=jEkU/Tp6iYxhs85LRRod6mArozKidlkf+44ix1jEq98F+pBqtM/cMbNDA+NjNdGRzj d+lY+ChMmSDd8RLdVWLXRhiMr5l7UT7veOL4C6w2l2mYaAZsUVc9DAYlr+xi/w9LAaXR /VXnSzQySPErpvYZpa44sAX0SMaQVz89x4iaebfeHvgmMqLga+BPMTPuWeMJdMZPaDst ej8lbCCQNIxehuQnMa1eRMu1YZeoYgwdh51iEbxB0eBF3cdcpvf61DMd0arT7BZY3M/W zhgGxyJv89ll2/klpqgfjL7ww1RoLdcEgoW8eHTeS1ea23FRp4Gyo33YUQ/VMskRB0hX yMMA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=RCCcqS88; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id kg8-20020a17090776e800b009888aa1da13si9794555ejc.752.2023.07.26.05.28.01; Wed, 26 Jul 2023 05:28:26 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=RCCcqS88; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232853AbjGZKz6 (ORCPT + 99 others); Wed, 26 Jul 2023 06:55:58 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41528 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233017AbjGZKz4 (ORCPT ); Wed, 26 Jul 2023 06:55:56 -0400 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CE19B132 for ; Wed, 26 Jul 2023 03:55:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1690368906; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=E3wvUr7a8sNlMjkQjrp9y08VnHFKDhRShbNPZvWniXw=; b=RCCcqS88sSa9+2VI6xMDCFhulrEnopkf30m9HjtmUzh2b5RkxACLrZNqi4rKSjOARDzW/p rRRYrlNcFIN68FjhDhjAbACNQpq/qq+jrR6nWo1V3bDk9KyuP/KxpPu9XwnIgeowZXMzrj BUmnmQbJ/3T/yGDVqHFWdO1AuQ/ZH60= Received: from mail-wr1-f70.google.com (mail-wr1-f70.google.com [209.85.221.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-169-S971_7b8OtGloJyvbhOTVQ-1; Wed, 26 Jul 2023 06:55:01 -0400 X-MC-Unique: S971_7b8OtGloJyvbhOTVQ-1 Received: by mail-wr1-f70.google.com with SMTP id ffacd0b85a97d-3176549261aso1320810f8f.3 for ; Wed, 26 Jul 2023 03:55:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1690368900; x=1690973700; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=E3wvUr7a8sNlMjkQjrp9y08VnHFKDhRShbNPZvWniXw=; b=fwyB4Q2aVVNvWnIDNIOynfGGoIUU1ZD7pd7JNYuYf4vEEHtqTm2vaeOmOEP92EbsSC TTt8ehMTM+YqTcWahhMsx3Pt/nu0xogw3jT4MOoqkfVwdBgIrbZZWGf+lRDx74SttqBQ 9D0pq2iTKyt1nprYZqCtxVcwbyZKM9Qnu9c5twPiu4bd1C7qsNOYSQr6zafKlOHt52yF aioAcmhIfKBhY5ZVIB1mdERPIF0Za9occCfDIxnM59eNKzQcthnawbS1M1+lDVMkk3kj l1jSsH79aj20mMp7KYSYy6DE+jBoGLUJb58me8UYk9VLLHC3XaeeWxcz2zgkSHcmRNab uJSQ== X-Gm-Message-State: ABy/qLYWrlymOpCCFpMkvpR7liKQZq9WKwm76DMiwtwWvfO8X9ArDbVL bBKWX4LC7zBLsX/z2dyScK3yXCjdRkOHnVIR4k98iX+szEYe23NxAdrjVpUK9wJDbQqQRgsyOEK 3qLZwzKMM4ZvYzFd/dISxQ2gWG4RQlckjLZE+sidbckH5hKPWBJeNWCOvv6qkJbv3eaT4DIxZGg HXZd5GdCA= X-Received: by 2002:a5d:6203:0:b0:317:6129:a972 with SMTP id y3-20020a5d6203000000b003176129a972mr1150660wru.37.1690368900391; Wed, 26 Jul 2023 03:55:00 -0700 (PDT) X-Received: by 2002:a5d:6203:0:b0:317:6129:a972 with SMTP id y3-20020a5d6203000000b003176129a972mr1150644wru.37.1690368900006; Wed, 26 Jul 2023 03:55:00 -0700 (PDT) Received: from minerva.home (205.pool92-176-231.dynamic.orange.es. [92.176.231.205]) by smtp.gmail.com with ESMTPSA id n7-20020adffe07000000b003176c6e87b1sm4894703wrr.81.2023.07.26.03.54.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Jul 2023 03:54:59 -0700 (PDT) From: Javier Martinez Canillas To: linux-kernel@vger.kernel.org Cc: Geert Uytterhoeven , Thomas Zimmermann , Daniel Vetter , Maxime Ripard , Javier Martinez Canillas , Daniel Vetter , David Airlie , dri-devel@lists.freedesktop.org Subject: [PATCH v5 2/2] drm/ssd130x: Allocate buffer in the plane's .atomic_check() callback Date: Wed, 26 Jul 2023 12:54:28 +0200 Message-ID: <20230726105433.389740-2-javierm@redhat.com> X-Mailer: git-send-email 2.41.0 In-Reply-To: <20230726105433.389740-1-javierm@redhat.com> References: <20230726105433.389740-1-javierm@redhat.com> MIME-Version: 1.0 X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1772486138692275649 X-GMAIL-MSGID: 1772486138692275649 Drivers are not allowed to fail after drm_atomic_helper_swap_state() has been called and the new atomic state is stored into the current sw state. Since the struct ssd130x_device .data_array is allocated in the encoder's .atomic_enable callback, the operation can fail and this is after the new state has been stored. So it can break an atomic mode settings assumption. Fix this by having custom helpers to allocate, duplicate and destroy the plane state, that will take care of allocating and freeing these buffers. Suggested-by: Maxime Ripard Signed-off-by: Javier Martinez Canillas Acked-by: Maxime Ripard Tested-by: Geert Uytterhoeven --- Changes in v5: - Add collected tags from Maxime and Geert. - Update commit message to not mention the kernel oops (Geert Uytterhoeven). - Drop Reported-by and Fixes tags (Geert Uytterhoeven). - Update comment about buffer and data_array fields (Geert Uytterhoeven). - Remove superfluous NULL check in ssd130x_fb_blit_rect() (Geert Uytterhoeven). - Reset .buffer to NULL if .data_array allocation fails (Geert Uytterhoeven). - Inline buffer alloc/free helper functions (Geert Uytterhoeven). Changes in v4: - Move buffers allocation/free to plane .reset/.destroy helpers (Maxime Ripard). Changes in v3: - Move the buffers allocation to the plane helper funcs .begin_fb_access and the free to .end_fb_access callbacks. - Always allocate intermediate buffer because is use in ssd130x_clear_screen(). - Don't allocate the buffers as device managed resources. Changes in v2: - Move the buffers allocation to .fb_create instead of preventing atomic updates for disable planes. - Don't allocate the intermediate buffer when using the native R1 format. - Allocate buffers as device managed resources. drivers/gpu/drm/solomon/ssd130x.c | 158 +++++++++++++++++++++++------- drivers/gpu/drm/solomon/ssd130x.h | 3 - 2 files changed, 121 insertions(+), 40 deletions(-) diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c index 51472a184ef9..d2f8dd6a6347 100644 --- a/drivers/gpu/drm/solomon/ssd130x.c +++ b/drivers/gpu/drm/solomon/ssd130x.c @@ -141,6 +141,19 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = { }; EXPORT_SYMBOL_NS_GPL(ssd130x_variants, DRM_SSD130X); +struct ssd130x_plane_state { + struct drm_plane_state base; + /* Intermediate buffer to convert pixels from XRGB8888 to HW format */ + u8 *buffer; + /* Buffer to store pixels in HW format and written to the panel */ + u8 *data_array; +}; + +static inline struct ssd130x_plane_state *to_ssd130x_plane_state(struct drm_plane_state *state) +{ + return container_of(state, struct ssd130x_plane_state, base); +} + static inline struct ssd130x_device *drm_to_ssd130x(struct drm_device *drm) { return container_of(drm, struct ssd130x_device, drm); @@ -434,12 +447,14 @@ static int ssd130x_init(struct ssd130x_device *ssd130x) SSD130X_SET_ADDRESS_MODE_HORIZONTAL); } -static int ssd130x_update_rect(struct ssd130x_device *ssd130x, struct drm_rect *rect) +static int ssd130x_update_rect(struct ssd130x_device *ssd130x, + struct ssd130x_plane_state *ssd130x_state, + struct drm_rect *rect) { unsigned int x = rect->x1; unsigned int y = rect->y1; - u8 *buf = ssd130x->buffer; - u8 *data_array = ssd130x->data_array; + u8 *buf = ssd130x_state->buffer; + u8 *data_array = ssd130x_state->data_array; unsigned int width = drm_rect_width(rect); unsigned int height = drm_rect_height(rect); unsigned int line_length = DIV_ROUND_UP(width, 8); @@ -535,7 +550,8 @@ static int ssd130x_update_rect(struct ssd130x_device *ssd130x, struct drm_rect * return ret; } -static void ssd130x_clear_screen(struct ssd130x_device *ssd130x) +static void ssd130x_clear_screen(struct ssd130x_device *ssd130x, + struct ssd130x_plane_state *ssd130x_state) { struct drm_rect fullscreen = { .x1 = 0, @@ -544,21 +560,21 @@ static void ssd130x_clear_screen(struct ssd130x_device *ssd130x) .y2 = ssd130x->height, }; - ssd130x_update_rect(ssd130x, &fullscreen); + ssd130x_update_rect(ssd130x, ssd130x_state, &fullscreen); } -static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_map *vmap, +static int ssd130x_fb_blit_rect(struct drm_plane_state *state, + const struct iosys_map *vmap, struct drm_rect *rect) { + struct drm_framebuffer *fb = state->fb; struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev); unsigned int page_height = ssd130x->device_info->page_height; + struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(state); + u8 *buf = ssd130x_state->buffer; struct iosys_map dst; unsigned int dst_pitch; int ret = 0; - u8 *buf = ssd130x->buffer; - - if (!buf) - return 0; /* Align y to display page boundaries */ rect->y1 = round_down(rect->y1, page_height); @@ -575,11 +591,49 @@ static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb, const struct iosys_m drm_gem_fb_end_cpu_access(fb, DMA_FROM_DEVICE); - ssd130x_update_rect(ssd130x, rect); + ssd130x_update_rect(ssd130x, ssd130x_state, rect); return ret; } +static int ssd130x_primary_plane_helper_atomic_check(struct drm_plane *plane, + struct drm_atomic_state *state) +{ + struct drm_device *drm = plane->dev; + struct ssd130x_device *ssd130x = drm_to_ssd130x(drm); + struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(state, plane); + struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(plane_state); + unsigned int page_height = ssd130x->device_info->page_height; + unsigned int pages = DIV_ROUND_UP(ssd130x->height, page_height); + const struct drm_format_info *fi; + unsigned int pitch; + int ret; + + ret = drm_plane_helper_atomic_check(plane, state); + if (ret) + return ret; + + fi = drm_format_info(DRM_FORMAT_R1); + if (!fi) + return -EINVAL; + + pitch = drm_format_info_min_pitch(fi, 0, ssd130x->width); + + ssd130x_state->buffer = kcalloc(pitch, ssd130x->height, GFP_KERNEL); + if (!ssd130x_state->buffer) + return -ENOMEM; + + ssd130x_state->data_array = kcalloc(ssd130x->width, pages, GFP_KERNEL); + if (!ssd130x_state->data_array) { + kfree(ssd130x_state->buffer); + /* Set to prevent a double free in .atomic_destroy_state() */ + ssd130x_state->buffer = NULL; + return -ENOMEM; + } + + return 0; +} + static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane, struct drm_atomic_state *state) { @@ -602,7 +656,7 @@ static void ssd130x_primary_plane_helper_atomic_update(struct drm_plane *plane, if (!drm_rect_intersect(&dst_clip, &damage)) continue; - ssd130x_fb_blit_rect(plane_state->fb, &shadow_plane_state->data[0], &dst_clip); + ssd130x_fb_blit_rect(plane_state, &shadow_plane_state->data[0], &dst_clip); } drm_dev_exit(idx); @@ -613,19 +667,69 @@ static void ssd130x_primary_plane_helper_atomic_disable(struct drm_plane *plane, { struct drm_device *drm = plane->dev; struct ssd130x_device *ssd130x = drm_to_ssd130x(drm); + struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(plane->state); int idx; if (!drm_dev_enter(drm, &idx)) return; - ssd130x_clear_screen(ssd130x); + ssd130x_clear_screen(ssd130x, ssd130x_state); drm_dev_exit(idx); } +/* Called during init to allocate the plane's atomic state. */ +static void ssd130x_primary_plane_reset(struct drm_plane *plane) +{ + struct ssd130x_plane_state *ssd130x_state; + + WARN_ON(plane->state); + + ssd130x_state = kzalloc(sizeof(*ssd130x_state), GFP_KERNEL); + if (!ssd130x_state) + return; + + __drm_atomic_helper_plane_reset(plane, &ssd130x_state->base); +} + +static struct drm_plane_state *ssd130x_primary_plane_duplicate_state(struct drm_plane *plane) +{ + struct ssd130x_plane_state *old_ssd130x_state; + struct ssd130x_plane_state *ssd130x_state; + + if (WARN_ON(!plane->state)) + return NULL; + + old_ssd130x_state = to_ssd130x_plane_state(plane->state); + ssd130x_state = kmemdup(old_ssd130x_state, sizeof(*ssd130x_state), GFP_KERNEL); + if (!ssd130x_state) + return NULL; + + /* The buffers are not duplicated and are allocated in .atomic_check */ + ssd130x_state->buffer = NULL; + ssd130x_state->data_array = NULL; + + __drm_atomic_helper_plane_duplicate_state(plane, &ssd130x_state->base); + + return &ssd130x_state->base; +} + +static void ssd130x_primary_plane_destroy_state(struct drm_plane *plane, + struct drm_plane_state *state) +{ + struct ssd130x_plane_state *ssd130x_state = to_ssd130x_plane_state(state); + + kfree(ssd130x_state->data_array); + kfree(ssd130x_state->buffer); + + __drm_atomic_helper_plane_destroy_state(&ssd130x_state->base); + + kfree(ssd130x_state); +} + static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs = { DRM_GEM_SHADOW_PLANE_HELPER_FUNCS, - .atomic_check = drm_plane_helper_atomic_check, + .atomic_check = ssd130x_primary_plane_helper_atomic_check, .atomic_update = ssd130x_primary_plane_helper_atomic_update, .atomic_disable = ssd130x_primary_plane_helper_atomic_disable, }; @@ -633,6 +737,9 @@ static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs = static const struct drm_plane_funcs ssd130x_primary_plane_funcs = { .update_plane = drm_atomic_helper_update_plane, .disable_plane = drm_atomic_helper_disable_plane, + .reset = ssd130x_primary_plane_reset, + .atomic_duplicate_state = ssd130x_primary_plane_duplicate_state, + .atomic_destroy_state = ssd130x_primary_plane_destroy_state, .destroy = drm_plane_cleanup, DRM_GEM_SHADOW_PLANE_FUNCS, }; @@ -677,10 +784,6 @@ static void ssd130x_encoder_helper_atomic_enable(struct drm_encoder *encoder, { struct drm_device *drm = encoder->dev; struct ssd130x_device *ssd130x = drm_to_ssd130x(drm); - unsigned int page_height = ssd130x->device_info->page_height; - unsigned int pages = DIV_ROUND_UP(ssd130x->height, page_height); - const struct drm_format_info *fi; - unsigned int pitch; int ret; ret = ssd130x_power_on(ssd130x); @@ -691,22 +794,6 @@ static void ssd130x_encoder_helper_atomic_enable(struct drm_encoder *encoder, if (ret) goto power_off; - fi = drm_format_info(DRM_FORMAT_R1); - if (!fi) - goto power_off; - - pitch = drm_format_info_min_pitch(fi, 0, ssd130x->width); - - ssd130x->buffer = kcalloc(pitch, ssd130x->height, GFP_KERNEL); - if (!ssd130x->buffer) - goto power_off; - - ssd130x->data_array = kcalloc(ssd130x->width, pages, GFP_KERNEL); - if (!ssd130x->data_array) { - kfree(ssd130x->buffer); - goto power_off; - } - ssd130x_write_cmd(ssd130x, 1, SSD130X_DISPLAY_ON); backlight_enable(ssd130x->bl_dev); @@ -728,9 +815,6 @@ static void ssd130x_encoder_helper_atomic_disable(struct drm_encoder *encoder, ssd130x_write_cmd(ssd130x, 1, SSD130X_DISPLAY_OFF); - kfree(ssd130x->data_array); - kfree(ssd130x->buffer); - ssd130x_power_off(ssd130x); } diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h index 161588b1cc4d..87968b3e7fb8 100644 --- a/drivers/gpu/drm/solomon/ssd130x.h +++ b/drivers/gpu/drm/solomon/ssd130x.h @@ -89,9 +89,6 @@ struct ssd130x_device { u8 col_end; u8 page_start; u8 page_end; - - u8 *buffer; - u8 *data_array; }; extern const struct ssd130x_deviceinfo ssd130x_variants[];