Re: [PATCH v2 05/10] drm/simpledrm: Convert to atomic helpers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 25.07.22 um 17:46 schrieb Javier Martinez Canillas:
On 7/20/22 16:27, Thomas Zimmermann wrote:
Replace the simple-KMS helpers with the regular atomic helpers. The
regular helpers are better architectured and therefore allow for easier
code sharing among drivers. No functional changes.


Acked-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>

But I've a question below...
Signed-off-by: Thomas Zimmermann <tzimmermann@xxxxxxx>
---
  drivers/gpu/drm/tiny/simpledrm.c | 283 ++++++++++++++++++++-----------
  1 file changed, 180 insertions(+), 103 deletions(-)

[...]

+static void simpledrm_crtc_helper_atomic_enable(struct drm_crtc *crtc,
+						struct drm_atomic_state *old_state)
+{
+	/*
+	 * Always enabled; screen updates are performed by
+	 * the primary plane's atomic_update function.
+	 */
+}
+
+static void simpledrm_crtc_helper_atomic_disable(struct drm_crtc *crtc,
+						 struct drm_atomic_state *old_state)
+{
+	/*
+	 * Always enabled; disabling clears the screen in the
+	 * primary plane's atomic_disable function.
+	 */
+}

...do we really need to have these ? Can't we just not set them ?

+
+static const struct drm_crtc_helper_funcs simpledrm_crtc_helper_funcs = {
+	.mode_valid = simpledrm_crtc_helper_mode_valid,
+	.atomic_check = simpledrm_crtc_helper_atomic_check,
+	.atomic_enable = simpledrm_crtc_helper_atomic_enable,
+	.atomic_disable = simpledrm_crtc_helper_atomic_disable,
+};
+
looking at https://elixir.bootlin.com/linux/latest/source/include/drm/drm_modeset_helper_vtables.h#L703
that says the .atomic_{en,dis}able handlers are optional.


The code also looks like we don't need the helpers. I mostly added them for the comments they contain, but I can also add those next to simpledrm_crtc_helper_funcs.

Best regards
Thomas


--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux