linux-toradex.git/drivers/gpu/drm/solomon, branch v6.7

drm/ssd130x: Fix possible uninitialized usage of crtc_state variable

2023-10-30T10:00:27+00:00

Avoid a possible uninitialized use of the crtc_state variable in function
ssd132x_primary_plane_atomic_check() and avoid the following Smatch warn:

    drivers/gpu/drm/solomon/ssd130x.c:921 ssd132x_primary_plane_atomic_check()
    error: uninitialized symbol 'crtc_state'.

Fixes: fdd591e00a9c ("drm/ssd130x: Add support for the SSD132x OLED controller family")
Reported-by: Dan Carpenter 
Closes: https://lore.kernel.org/dri-devel/7dd6ca45-8263-44fe-a318-2fd9d761425d@moroto.mountain/
Signed-off-by: Javier Martinez Canillas 
Acked-by: Jocelyn Falempe 
Link: https://patchwork.freedesktop.org/patch/msgid/20231020225338.1686974-1-javierm@redhat.com
(cherry picked from commit 9e4db199e66d427c50458f4d72734cc4f0b92948)
Signed-off-by: Javier Martinez Canillas

drm/ssd130x: Add support for the SSD132x OLED controller family

2023-10-18T07:53:29+00:00

The Solomon SSD132x controllers (such as the SSD1322, SSD1325 and SSD1327)
are used by 16 grayscale dot matrix OLED panels, extend the driver to also
support this chip family.

Instead adding an indirection level to allow the same modesetting pipeline
to be used by both controller families, add another pipeline for SSD132x.

This leads to some code duplication but it makes the driver easier to read
and reason about. Once other controller families are added (e.g: SSD133x),
some common code can be factored out in driver helpers to be shared by the
different families. But that can be done later once these patterns emerge.

Signed-off-by: Javier Martinez Canillas 
Acked-by: Thomas Zimmermann 
Link: https://patchwork.freedesktop.org/patch/msgid/20231014071520.1342189-5-javierm@redhat.com

drm/ssd130x: Rename commands that are shared across chip families

2023-10-18T07:53:28+00:00

There are some commands that are shared between the SSD130x and SSD132x
controller families, define these as a common SSD13XX set of commands.

Signed-off-by: Javier Martinez Canillas 
Acked-by: Thomas Zimmermann 
Link: https://patchwork.freedesktop.org/patch/msgid/20231014071520.1342189-4-javierm@redhat.com

drm/ssd130x: Add a controller family id to the device info data

2023-10-18T07:53:26+00:00

To allow the driver to have a per Solomon display controller modesetting
pipeline and support aother controller families besides SSD130x.

Signed-off-by: Javier Martinez Canillas 
Acked-by: Thomas Zimmermann 
Link: https://patchwork.freedesktop.org/patch/msgid/20231014071520.1342189-3-javierm@redhat.com

drm/ssd130x: Replace .page_height field in device info with a constant

2023-10-18T07:53:25+00:00

This deemed useful to avoid hardcoding a page height and allow to support
other Solomon controller families, but dividing the screen in pages seems
to be something that is specific to the SSD130x chip family.

For example, SSD132x chip family divides the screen in segments (columns)
and common outputs (rows), so the concept of screen pages does not exist
for the SSD132x family.

Let's drop this field from the device info struct and just use a constant
SSD130X_PAGE_HEIGHT macro to define the page height. While being there,
replace hardcoded 8 values in places where it is used as the page height.

Signed-off-by: Javier Martinez Canillas 
Reviewed-by: Geert Uytterhoeven 
Acked-by: Thomas Zimmermann 
Link: https://patchwork.freedesktop.org/patch/msgid/20231014071520.1342189-2-javierm@redhat.com

drm/ssd130x: Fix atomic_check for disabled planes

2023-10-13T15:53:32+00:00

The plane's atomic_check returns -EINVAL if the CRTC has not been
set. This is the case for disabled planes, for which atomic_check
should return 0. For disabled planes, it also omits the mandatory
call to drm_atomic_helper_check_plane_state().

Replace the test with the boiler-plate code that first invokes
drm_atomic_helper_check_plane_state() and then tests for the plane
to be visible. Return early for non-visible planes.

Signed-off-by: Thomas Zimmermann 
Fixes: d51f9fbd98b6 ("drm/ssd130x: Store the HW buffer in the driver-private CRTC state")
Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 
Cc: Geert Uytterhoeven 
Cc: Javier Martinez Canillas 
Cc: Maxime Ripard 
Signed-off-by: Javier Martinez Canillas 
Link: https://patchwork.freedesktop.org/patch/msgid/20231009141018.11291-7-tzimmermann@suse.de

drm/ssd130x: Drop _helper prefix from struct drm_*_helper_funcs callbacks

2023-09-22T09:48:08+00:00

The driver uses a naming convention where functions for struct drm_*_funcs
callbacks are named ssd130x_$object_$operation, while the callbacks for
struct drm_*_helper_funcs are named ssd130x_$object_helper_$operation.

The idea is that this helper_ prefix in the function names denote that are
for struct drm_*_helper_funcs callbacks. This convention was copied from
other drivers when ssd130x was written, but Maxime pointed out that is the
exception rather than the norm.

So let's get rid of the _helper prefixes from the function handlers names.

Suggested-by: Maxime Ripard 
Signed-off-by: Javier Martinez Canillas 
Reviewed-by: Maxime Ripard 
Link: https://patchwork.freedesktop.org/patch/msgid/20230914195138.1518065-1-javierm@redhat.com

drm/ssd130x: Call drm_atomic_helper_shutdown() at remove time

2023-09-21T17:50:03+00:00

Based on grepping through the source code, this driver appears to be
missing a call to drm_atomic_helper_shutdown() at remove time. Let's
add it.

The fact that we should call drm_atomic_helper_shutdown() in the case
of OS driver remove comes straight out of the kernel doc "driver
instance overview" in drm_drv.c.

Suggested-by: Maxime Ripard 
Acked-by: Maxime Ripard 
Signed-off-by: Douglas Anderson 
Link: https://patchwork.freedesktop.org/patch/msgid/20230901163944.RFT.4.I4752a39ad9f8fd08b32c2b78a8a3e40491bfb5eb@changeid

drm/ssd130x: Store the HW buffer in the driver-private CRTC state

2023-09-14T16:45:57+00:00

The commit 45b58669e532 ("drm/ssd130x: Allocate buffer in the plane's
.atomic_check() callback") moved the allocation of the intermediate and
HW buffers from the encoder's .atomic_enable callback, to the plane's
.atomic_check callback.

This was suggested by Maxime Ripard, because drivers aren't allowed to
fail after the drm_atomic_helper_swap_state() function has been called.

And the encoder's .atomic_enable happens after the new atomic state has
been swapped, so allocations (that can fail) shouldn't be done there.

But the HW buffer isn't really tied to the plane's state. It has a fixed
size that only depends on the (also fixed) display resolution defined in
the Device Tree Blob.

That buffer can be considered part of the CRTC state, and for this reason
makes more sense to do its allocation in the CRTC .atomic_check callback.

The other allocated buffer (used to store a conversion from the emulated
XR24 format to the native R1 format) is part of the plane's state, since
it will be optional once the driver supports R1 and allows user-space to
set that pixel format.

So let's keep the allocation for it in the plane's .atomic_check callback,
this can't be moved to the CRTC's .atomic_check because changing a format
does not trigger a CRTC mode set.

Reported-by: Geert Uytterhoeven
Closes: https://lore.kernel.org/dri-devel/CAMuHMdWv_QSatDgihr8=2SXHhvp=icNxumZcZOPwT9Q_QiogNQ@mail.gmail.com/
Signed-off-by: Javier Martinez Canillas
Acked-by: Maxime Ripard
Link: https://patchwork.freedesktop.org/patch/msgid/20230913052938.1114651-1-javierm@redhat.com

drm/ssd130x: Use bool for ssd130x_deviceinfo flags

2023-09-10T07:05:47+00:00

The .need_pwm and .need_chargepump fields in struct ssd130x_deviceinfo
are flags that can have only two possible values: 0 and 1.
Reduce kernel size by changing their types from int to bool.

Signed-off-by: Geert Uytterhoeven 
Reviewed-by: Javier Martinez Canillas 
Tested-by: Javier Martinez Canillas 
Signed-off-by: Javier Martinez Canillas 
Link: https://patchwork.freedesktop.org/patch/msgid/285005ff361969eff001386c5f97990f0e703838.1692888745.git.geert@linux-m68k.org