* Re: [PATCH v1 2/2] ASoC: codecs: add Qualcomm WSA885X I2C codec driver
From: Prasad Kumpatla @ 2026-06-23 9:20 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Srinivas Kandagatla, linux-arm-msm, linux-sound, devicetree,
linux-kernel, linux-gpio, Srinivas Kandagatla, Liam Girdwood,
Mark Brown, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jaroslav Kysela, Takashi Iwai, Linus Walleij
In-Reply-To: <CAMRc=Mf2oujn6MstGqKg1JCu3hbPD5zHhCB-Zke_hu8LYCz-Xg@mail.gmail.com>
On 6/11/2026 3:19 PM, Bartosz Golaszewski wrote:
> On Wed, 10 Jun 2026 17:57:08 +0200, Prasad Kumpatla
> <prasad.kumpatla@oss.qualcomm.com> said:
>> Add an ASoC codec driver for the Qualcomm WSA885X smart speaker
>> amplifier accessed over I2C.
>>
>> The driver provides the control-side support needed for playback
>> bring-up, including register programming, serial interface setup, clock
>> handling, mute and gain control, reset handling and interrupt support.
>>
>> Program the init table during codec initialization and reapply it only
>> after an explicit device reset so the static device configuration is
>> not rewritten on every playback start. Also program the TDM control
>> slot-count field from the runtime slot configuration so the same codec
>> path can be used with 2-slot, 4-slot, or 8-slot Audio IF backends.
>>
>> Keep the stream-time power-state sequencing in the DAI callbacks and
>> use normal regmap access for the control path.
>>
>> Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
>> ---
> ...
>
>> diff --git a/sound/soc/codecs/wsa885x-i2c.c b/sound/soc/codecs/wsa885x-i2c.c
>> new file mode 100644
>> index 000000000..a7d8f8d48
>> --- /dev/null
>> +++ b/sound/soc/codecs/wsa885x-i2c.c
>> @@ -0,0 +1,1643 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +/* WSA885X I2C codec driver */
>> +
>> +#include <linux/gpio/consumer.h>
>> +#include <linux/bitfield.h>
>> +#include <linux/i2c.h>
>> +#include <linux/module.h>
>> +#include <linux/regmap.h>
>> +#include <linux/property.h>
>> +#include <linux/regulator/consumer.h>
>> +#include <linux/slab.h>
>> +#include <sound/core.h>
>> +#include <sound/pcm.h>
>> +#include <sound/pcm_params.h>
>> +#include <sound/soc-dapm.h>
>> +#include <sound/soc.h>
>> +#include <sound/tlv.h>
>> +#include <linux/interrupt.h>
> Can you keep the headers in alphabetical order?
Hi Bart,
Thanks for review the patch and the feedback.
Ack, Will update
>
> ...
>
>> +
>> +#define WSA885X_FU21_VOL_STEPS 124
>> +#define WSA885X_USAGE_MODE_MAX 8
>> +#define WSA885X_INIT_TABLE_MAX_ITEMS 256
> Add newline.
Ack, Will update.
>
> ...
>
>> +
>> +static int wsa885x_apply_init_table(struct wsa885x_i2c_priv *wsa885x)
>> +{
>> + int i;
>> + int ret;
> I'd put it on the same line (elsewhere too) but that's personal preference.
Ack, I will make them to a single line.
>
>> +
>> + if (!wsa885x || !wsa885x->regmap)
>> + return -EINVAL;
>
> You have a lot of these checks but this can't really happen, can it?
Ack, I will cleanup and remove the all unnecessary checks and update in
next version
>
>> +
>> + if (!wsa885x->init_table_size)
>> + return 0;
>> +
>> + if (!wsa885x->init_table)
>> + return -EINVAL;
>> +
>> + for (i = 0; i < wsa885x->init_table_size / 2; i++) {
>> + u32 reg = wsa885x->init_table[2 * i];
>> + u32 val = wsa885x->init_table[2 * i + 1];
>> +
>> + if (wsa885x->batt_conf == WSA885X_BATT_2S && reg == WSA885X_SPK_TOP_LF_CH1_CTRL11)
>> + continue;
>> +
>> + if (wsa885x->batt_conf == WSA885X_BATT_2S && reg == WSA885X_SPK_TOP_LF_CH2_CTRL11)
>> + continue;
>> +
>> + ret = regmap_write(wsa885x->regmap, reg, val);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int wsa885x_hw_init(struct wsa885x_i2c_priv *wsa885x)
>> +{
>> + static const struct reg_sequence regs[] = {
>> + { WSA885X_DIG_CTRL1_SPMI_PAD_GPIO2_CTL, 0x2e },
>> + { WSA885X_DIG_CTRL1_INTR_MODE, 0x01 },
>> + { WSA885X_DIG_CTRL1_PIN_CT, 0x04 },
>> + };
>> + int ret;
>> +
>> + if (!wsa885x || !wsa885x->regmap)
>> + return -EINVAL;
>> +
>> + ret = wsa885x_apply_init_table(wsa885x);
>> + if (ret)
>> + return ret;
>> +
>> + if (wsa885x->batt_conf == WSA885X_BATT_2S) {
>> + ret = wsa885x_2s_conf(wsa885x);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + return regmap_multi_reg_write(wsa885x->regmap, regs, ARRAY_SIZE(regs));
>> +}
>> +
>> +static int wsa885x_unmask_interrupts(struct wsa885x_i2c_priv *wsa885x)
>> +{
>> + static const struct reg_sequence regs[] = {
>> + { WSA885X_INTR_MASK0, 0x00 },
>> + { WSA885X_INTR_MASK0 + 1, 0x00 },
>> + { WSA885X_INTR_MASK0 + 2, 0xf8 },
>> + };
>> +
>> + if (!wsa885x || !wsa885x->regmap)
>> + return -EINVAL;
>> +
>> + return regmap_multi_reg_write(wsa885x->regmap, regs, ARRAY_SIZE(regs));
>> +}
>> +
>> +static int wsa885x_wait_for_pde_state(struct wsa885x_i2c_priv *wsa885x, int ps)
>> +{
>> + int act_ps = -1, cnt = 0, clock_valid = -1;
>> + int rc = 0;
>> +
>> + if (!wsa885x || !wsa885x->regmap)
>> + return -EINVAL;
>> +
>> + if (ps < 0 || ps > 3)
>> + return -EINVAL;
>> +
>> + do {
>> + usleep_range(1000, 1500);
>> + rc = regmap_read(wsa885x->regmap,
>> + WSA885X_SMP_AMP_CTRL_STEREO_PDE23_ACT_PS,
>> + &act_ps);
>> + if (rc) {
>> + dev_err(wsa885x->dev, "PDE state read failed: %d\n", rc);
>> + return rc;
>> + }
>> + if (act_ps == ps)
>> + return 0;
>> + } while (++cnt < 5);
> Newline.
Ack.
>
>> + if (regmap_read(wsa885x->regmap,
>> + WSA885X_SMP_AMP_CTRL_STEREO_CS21_CLOCK_VALID,
>> + &clock_valid))
>> + dev_err(wsa885x->dev,
>> + "PDE power state %d request failed, actual_ps %d, clock_valid read failed\n",
>> + ps, act_ps);
>> + else
>> + dev_err(wsa885x->dev,
>> + "PDE power state %d request failed, actual_ps %d, clock_valid:%d\n",
>> + ps, act_ps, clock_valid);
>> +
>> + return -ETIMEDOUT;
>> +}
>> +
>> +static int wsa885x_codec_hw_params(struct snd_pcm_substream *substream,
>> + struct snd_pcm_hw_params *params,
>> + struct snd_soc_dai *dai)
>> +{
>> + struct wsa885x_i2c_priv *wsa885x;
>> + u8 pcm_rate, cs21_sample_rate_idx, cs24_sample_rate_idx;
>> +
>> + (void)substream;
> Do we warn about unused arguments in the kernel now?
Right, this is unnecessary. I'll drop the unused parameter cast.
Ack.
>
> ...
>
>> +
>> +static int wsa885x_stereo_gain_offset_get(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + struct snd_soc_component *component;
>> + struct wsa885x_i2c_priv *wsa885x;
>> + int val;
>> +
>> + if (!kcontrol || !ucontrol)
>> + return -EINVAL;
>> +
>> + component = snd_kcontrol_chip(kcontrol);
>> + if (!component)
>> + return -EINVAL;
>> +
>> + wsa885x = snd_soc_component_get_drvdata(component);
>> + if (!wsa885x)
>> + return -EINVAL;
>> +
>> + val = wsa885x->stereo_vol_db + 84;
>> + if (val < 0 || val > WSA885X_FU21_VOL_STEPS)
>> + return -ERANGE;
>> +
>> + ucontrol->value.integer.value[0] = val;
>> + return 0;
>> +}
>> +
>> +static int wsa885x_stereo_gain_offset_put(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + struct snd_soc_component *component;
>> + struct wsa885x_i2c_priv *wsa885x;
>> + long val;
>> +
>> + if (!kcontrol || !ucontrol)
>> + return -EINVAL;
>> +
>> + component = snd_kcontrol_chip(kcontrol);
>> + if (!component)
>> + return -EINVAL;
>> +
>> + wsa885x = snd_soc_component_get_drvdata(component);
>> + if (!wsa885x)
>> + return -EINVAL;
>> +
>> + val = ucontrol->value.integer.value[0];
>> +
>> + if (val < 0 || val > WSA885X_FU21_VOL_STEPS) {
>> + dev_err(component->dev, "%s: Invalid range, Val: %ld\n", __func__, val);
>> + return -EINVAL;
>> + }
>> + wsa885x->stereo_vol_db = (int)val - 84;
>> + return 0;
>> +}
>> +
>> +static int wsa885x_i2c_usage_modes_get(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + struct snd_soc_component *component;
>> + struct wsa885x_i2c_priv *wsa885x_i2c;
>> +
>> + if (!kcontrol || !ucontrol)
>> + return -EINVAL;
>> +
>> + component = snd_kcontrol_chip(kcontrol);
>> + if (!component)
>> + return -EINVAL;
>> +
>> + wsa885x_i2c = snd_soc_component_get_drvdata(component);
>> + if (!wsa885x_i2c)
>> + return -EINVAL;
>> +
>> + if (wsa885x_i2c->usage_mode > WSA885X_USAGE_MODE_MAX)
>> + return -ERANGE;
>> +
>> + ucontrol->value.integer.value[0] = wsa885x_i2c->usage_mode;
>> +
>> + return 0;
>> +}
>> +
>> +static int wsa885x_i2c_usage_modes_put(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + struct snd_soc_component *component;
>> + struct wsa885x_i2c_priv *wsa885x_i2c;
>> + long val;
>> +
>> + if (!kcontrol || !ucontrol)
>> + return -EINVAL;
>> +
>> + component = snd_kcontrol_chip(kcontrol);
>> + if (!component)
>> + return -EINVAL;
>> +
>> + wsa885x_i2c = snd_soc_component_get_drvdata(component);
>> + if (!wsa885x_i2c)
>> + return -EINVAL;
>> +
> You seem to be repeating the same sequence in multiple functions just to get
> the address of wsa885x_i2c. Can you factor it out into a separate helper and
> save some lines?
Ack.
>
>> + val = ucontrol->value.integer.value[0];
>> +
>> + if (val < 0 || val > WSA885X_USAGE_MODE_MAX)
>> + return -EINVAL;
>> +
>> + wsa885x_i2c->usage_mode = val;
>> +
>> + return 0;
>> +}
>> +
>> +static int wsa885x_i2c_rx_slot_mask_get(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + struct snd_soc_component *component;
>> + struct wsa885x_i2c_priv *wsa885x_i2c;
>> + u32 mask;
>> +
>> + if (!kcontrol || !ucontrol)
>> + return -EINVAL;
>> +
>> + component = snd_kcontrol_chip(kcontrol);
>> + if (!component)
>> + return -EINVAL;
>> +
>> + wsa885x_i2c = snd_soc_component_get_drvdata(component);
>> + if (!wsa885x_i2c)
>> + return -EINVAL;
>> +
>> + mask = wsa885x_i2c->rx_slot_mask;
>> + if (!wsa885x_is_valid_rx_slot_mask(mask))
>> + return -ERANGE;
>> +
>> + ucontrol->value.integer.value[0] = mask;
>> +
>> + return 0;
>> +}
>> +
>> +static int wsa885x_i2c_rx_slot_mask_put(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + struct snd_soc_component *component;
>> + struct wsa885x_i2c_priv *wsa885x_i2c;
>> + long mask;
>> +
>> + if (!kcontrol || !ucontrol)
>> + return -EINVAL;
>> +
>> + component = snd_kcontrol_chip(kcontrol);
>> + if (!component)
>> + return -EINVAL;
>> +
>> + wsa885x_i2c = snd_soc_component_get_drvdata(component);
>> + if (!wsa885x_i2c)
>> + return -EINVAL;
>> +
>> + mask = ucontrol->value.integer.value[0];
>> +
>> + if (!wsa885x_is_valid_rx_slot_mask(mask))
>> + return -EINVAL;
>> +
>> + wsa885x_i2c->rx_slot_mask = mask;
>> +
>> + return 0;
>> +}
>> +
> ...
>
>> + /* INTR_CLEAR registers are write-only; use regmap_write
>> + * instead of regmap_update_bits to avoid the read-modify-write
>> + * that regmap_update_bits performs on non-readable registers.
>> + */
> /*
> */
>
> style comments please
Ack. will update
>
> ...
>
>> + ret = devm_add_action_or_reset(dev, wsa885x_gpio_powerdown, wsa885x);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "devm_add_action_or_reset failed\n");
>> +
>> + i2c_set_clientdata(client, wsa885x);
> I don't see a corresponding i2c_get_clientdata(). Do you really need it?
It is currently not being used, so storing the client data is unnecessary.
I'll either remove it or add it together with the code that requires
i2c_get_clientdata() in a future versions.
Thanks,
Prasad
>
> ...
>
> Bart
^ permalink raw reply
* Re: [PATCH 3/6] drm/tiny: Add DRM driver for Solomon SSD16xx e-paper display controllers
From: Thomas Zimmermann @ 2026-06-23 9:17 UTC (permalink / raw)
To: Devarsh Thakkar, David Airlie, Simona Vetter, Maarten Lankhorst,
Maxime Ripard, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Bjorn Andersson, dri-devel, devicetree,
linux-kernel
Cc: praneeth, vigneshr, s-jain1, r-donadkar, r-sharma3, afd, Sen Wang,
LiangCheng Wang, Aldea, Andrei, Judith Mendez, D, Yashas
In-Reply-To: <e14de1da-5d1e-4086-a713-246ebc15603e@ti.com>
Hi,
sorry, I've lost track of what the most recent status is here.
Am 18.06.26 um 17:41 schrieb Devarsh Thakkar:
[...]
>>>>> +config DRM_PANEL_SSD16XX
>>>>
>>>> Just call it DRM_SSD16XX without the panel. In DRM, things named
>>>> 'panel' are usually built around struct drm_panel, which doesn't
>>>> seem the case here.
>>>>
>>>
>
> I see drivers/gpu/drm/tiny/panel-mipi-dbi.c [0] using
> CONFIG_DRM_PANEL_MIPI_DBI [1] in the same directory, and that driver
> does not use struct drm_panel either - the only drm_panel reference
> there is a call to of_get_drm_panel_display_mode(), which is a DT
> display-mode helper unrelated to the panel framework.
It's either a misnomer or there for historical reasons IMHO. Just
mipi-dbi would have been better. Or maybe it should use the framework
around drm_panel. As it is now, the naming is somwhat misleading.
>
> Given this existing precedent in drm/tiny/ itself and also for the
> reasons explained below as this driver houses both controller specific
> and panel specific logic, I would prefer to keep DRM_PANEL_SSD16XX.
No need to duplicate bad decisions. Also, can your driver use the panel
framework?
Best regards
Thomas
>
>
>
>>> Oh ok, I preferred DRM_PANEL_SSD16XX since it also enumerates and
>>> uses panel specific data/compatible such as this driver supporting
>>> gooddisplay,gdey042t81 and more can be added too (just like panel-
>>> ilitek* for e.g.) unlike controller only drivers which need to be
>>> linked to separate panel drivers.
>>>
>>> Do you prefer to change it to DRM_SSD16XX_PANEL to not conflict with
>>> DRM_PANEL* drivers and for better context or still prefer to keep it
>>> as DRM_SSD16XX ?
>>>
>>>>> +
>
> <snip>
>
>>>>> diff --git a/drivers/gpu/drm/tiny/panel-ssd16xx.c
>>>>> b/drivers/gpu/drm/ tiny/panel-ssd16xx.c
>>>>> new file mode 100644
>>>>> inde`x 000000000000..b232837c54ff
>>>>> --- /dev/null
>>>>> +++ b/drivers/gpu/drm/tiny/panel-ssd16xx.c
>>>>
>>>> Again, remove 'panel'.
>>>
>
> Also for the naming too, I'd prefer to keep panel-ssd16xx.c similar to
> panel-mipi-dbi.c [0] for the reasons mentioned below.
>
>>> Yes I can remove the panel, but I am just concerned if it won't
>>> mislead folks to understand ssd16xx as a controller only driver,
>>> requiring a separate panel driver to interface with ?
>>>
>>> Basically panel-ssd16xx naming was chosen since this driver houses
>>> both the ssd16xx controller context and also the panel being used
>>> along with that (similar to panel-ilitek-ili9881c.c) and i did not
>>> want to confuse it with a controller only driver (similar to
>>> tc358775.c), if it is overalapping a known pattern reserved for
>>> drm_panel drivers do you think we should rename it to
>>> ssd16xx-panel.c instead or you prefer ssd16xx.c as more appropriate
>>> one ?
>>>
>
> Kindly let me know if it sounds okay.
>
> [0] :
> https://gitlab.com/linux-kernel/linux-next/-/blob/next-20260618/drivers/gpu/drm/tiny/panel-mipi-dbi.c?ref_type=tags
> [1] :
> https://gitlab.com/linux-kernel/linux-next/-/blob/next-20260618/drivers/gpu/drm/tiny/Makefile?ref_type=tags#L8
>
> Regards
> Devarsh
>
>>>>
>>>>> @@ -0,0 +1,2548 @@
>>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>>> +/*
>>>>> + * DRM driver for e-paper display panels using Solomon SSD16xx
>>>>> family controllers
>>>>> + *
>>>>> + * Copyright (C) 2026 Texas Instruments Incorporated - https://
>>>>> www.ti.com/
>>>>> + *
>>>>> + * Author: Devarsh Thakkar <devarsht@ti.com>
>>>>> + *
>>>>> + * References: https://github.com/Lesords/epaper
>>>>> + */
>>>>> +
>>>>> +#include <linux/delay.h>
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/of.h>
>>>>> +#include <linux/property.h>
>>>>> +#include <linux/spi/spi.h>
>>>>> +
>>>>> +#include <drm/clients/drm_client_setup.h>
>>>>> +#include <drm/drm_atomic.h>
>>>>> +#include <drm/drm_atomic_helper.h>
>>>>> +#include <drm/drm_damage_helper.h>
>>>>> +#include <drm/drm_drv.h>
>>>>> +#include <drm/drm_fb_helper.h>
>>>>> +#include <drm/drm_fbdev_dma.h>
>>>>> +#include <drm/drm_fb_dma_helper.h>
>>>>> +#include <drm/drm_framebuffer.h>
>>>>> +#include <drm/drm_gem_dma_helper.h>
>>>>> +#include <drm/drm_gem_framebuffer_helper.h>
>>>>> +#include <drm/drm_probe_helper.h>
>>>>
>>>>> +#include <drm/drm_simple_kms_helper.h>
>>>>
>>>> Obsolete. Anything you use from this header should be open-coded in
>>>> the driver.
>>>>
>>>
>>> Agreed, will remove it in V2.
>>>
>>>>> +#include <drm/drm_print.h>
>>>>
>>>>
>>>> Please remove all of the parameters below. They might be nice for
>>>> your debugging, but they do not belong in the upstream driver.
>>>>
>>>
>>> As mentioned previously, had kept these params mainly for legacy
>>> non- drm fbdev based applications.
>>>
>>>>> +
>>>>> +static int rotation = -1;
>>>>> +module_param(rotation, int, 0644);
>>>>> +MODULE_PARM_DESC(rotation,
>>>>> + "Display rotation (-1=use DT, 0/180=landscape,
>>>>> 90/270=portrait)");
>>>>
>>>> Please remove this. There is a rotation property in struct
>>>> drm_connector, which stores the rotation. IIRC it can be overridden
>>>> on the kernel command line.
>>>>
>>>
>>> As I understand you are referring to below fields from drm_connector
>>> struct, please correct me if I am wrong here but I think the
>>> rotation/ orientation functionality supported by ssd16xx controller
>>> does not match much with below model but instead matches what is
>>> done in drivers/gpu/ drm/drm_mipi_dbi.c (although that does not
>>> support runtime rotation) as explained below :
>>>
>>> drm_connector (rotation specific members):
>>>
>>>
>>> 1. panel_orientation (display_info.panel_orientation):
>>> Readable from DT via of_drm_get_panel_orientation(), overridable from
>>> cmdline. However it is not writable by userspace at runtime (which
>>> we require). More importantly, when Weston reads panel_orientation
>>> it applies an output transform and then attempts to offload rotation
>>> to the plane via plane.rotation. This model assumes the plane can
>>> geometrically map a 300x400 source framebuffer to a 400x300 CRTC
>>> i.e. hardware scan- out rotation. Our driver has no such hardware as
>>> explained in detail below.
>>>
>>>
>>> 2. rotation_reflection (cmdline_mode.rotation_reflection):
>>> Cmdline-only (video=...:rotate=N), no DT path (we require both
>>> DT-path and runtime suport). Also I think this is strictly for
>>> in-kernel drm_clients and also It currently returns false for 90/270
>>> unless
>>> the plane has a hardware rotation property.
>>>
>>>
>>> Both paths therefore ultimately require hardware plane rotation that
>>> this driver does not have and both seem to be supported just
>>> statically i.e. cmdline or dt property.
>>>
>>> Our use-case needs to support runtime rotation configuration ours is
>>> not a mounted display but a portable hand-held device (https://
>>> www.beagleboard.org/boards/beaglebadge) and we have an accelerometer
>>> in our device which can detect panel orientation and based on
>>> accelerometer reading the drm app can runtime set the custom drm
>>> rotation property to switch to new orientation dynamically.
>>>
>>> Also our driver is fundamentally different from a GPU display
>>> pipeline or controllers supporting transpose function. The SSD16xx
>>> display controller has no transpose or rotation function but instead
>>> supports different scan-modes, so there is no hardware path that can
>>> take a 400x300 plane and transpose it to a 300x400 display output.
>>> The controller is a simple RAM writer: the CPU writes a byte stream
>>> over SPI, and the controller's internal cursor
>>> advances sequentially according to the data entry mode register
>>> (command
>>> 0x11), which selects between X++/Y++ and X--/Y-- scan directions with a
>>> configurable start position.
>>>
>>>
>>> For portrait orientation we therefore change the DRM mode itself to
>>> 300x400 from the original 400x300, so the application is asked to
>>> provide a 300x400 framebuffer.
>>> The driver then writes this buffer column-by-column over SPI to the
>>> display controller's RAM. Since the controller supports different scan
>>> start positions (cursor at origin vs cursor at maximum address)
>>> combined
>>> with the appropriate X/Y scan direction, we are able to correctly
>>> render
>>> the 300x400 buffer onto the panel when it is held in portrait
>>> orientation (90*, 270*).
>>>
>>>
>>> This means the CRTC mode must reflect the logical dimensions directly,
>>> exactly as drm_mipi_dbi_dev_init() does via mipi_dbi_rotate_mode() for
>>> MIPI DBI drivers. Accepting a 300x400 framebuffer onto a 400x300 CRTC
>>> (as the panel_orientation + plane.rotation model requires) is not
>>> possible: drm_atomic_helper_check_plane_state(DRM_PLANE_NO_SCALING)
>>> enforces src_w == crtc_w and src_h == crtc_h, and there is no hardware
>>> to perform the geometric remapping between the two sizes.
>>>
>>>
>>> For runtime rotation changes (which are required as the panel is not
>>> physically fixed), we therefor wanted to use a custom drm connector
>>> property. We can look to use the standard DRM_MODE_ROTATE_* bitmask
>>> (not a custom enum, that was used in v1), we can also look to check
>>> if driver can triggers a full modeset through the normal DRM path,
>>> connector_get_modes returns the correctly dimensioned mode for the
>>> new orientation, and userspace receives a mode-changed event with
>>> the new dimensions.
>>>
>>>
>>> This is semantically what MIPI DBI tiny drivers do at boot (fixed
>>> from DT), made runtime-changeable via the custom drm connector
>>> property in this driver.
>>>
>>> Maybe, I can try to use standard bitmask instead of custom enum to
>>> re- use standard macros :
>>>
>>> drm_property_create_bitmask(drm, 0, "rotation",
>>> rotation_props,
>>> ARRAY_SIZE(rotation_props),
>>> DRM_MODE_ROTATE_0 DRM_MODE_ROTATE_90 |
>>> DRM_MODE_ROTATE_180 |DRM_MODE_ROTATE_270);
>>>
>>> but keep it as connector property?
>>>
>>>>> +
>>>>> +static int refresh_mode = -1;
>>>>> +module_param(refresh_mode, int, 0644);
>>>>> +MODULE_PARM_DESC(refresh_mode,
>>>>> + "Refresh mode (-1=panel default, 0=partial ~300-500ms,
>>>>> 1=full ~1.5-2s, 2=fast ~1.0-1.5s)");
>>>>> +
>>>>> +static int border_waveform_init_lut = -1;
>>>>> +module_param(border_waveform_init_lut, int, 0644);
>>>>> +MODULE_PARM_DESC(border_waveform_init_lut,
>>>>> + "Border waveform index during clear/init (-1=panel
>>>>> default, 0-9=enum index)");
>>>>> +
>>>>> +static int border_waveform_lut = -1;
>>>>> +module_param(border_waveform_lut, int, 0644);
>>>>> +MODULE_PARM_DESC(border_waveform_lut,
>>>>> + "Border waveform index during display updates (-1=panel
>>>>> default, 0-9=enum index)");
>>>>> +
>>>>
>>>> Please remove it. Only the panel default. If you have panels where
>>>> the default is known to be incorrect, you can add specific
>>>> workarounds in the driver.
>>>>
>>>
>>> I think the most of these params are kept to sane defaults but they
>>> may change w.r.t use-cases and each panel can be used in context of
>>> multiple use-cases.
>>>
>>>>
>>>>> +static bool border_refresh_on_every_update;
>>>>> +module_param(border_refresh_on_every_update, bool, 0644);
>>>>> +MODULE_PARM_DESC(border_refresh_on_every_update,
>>>>> + "Re-send border waveform command before each display
>>>>> update (default: false)");
>>>>
>>>> Pick a sane default.
>>>>
>>>
>>> Yes driver is picking a sane default already for this (refresh
>>> border on init once with white border and keep it as floating in
>>> later updates), but just a back-door for the application in case it
>>> wants to avoid ghosting totally altogether or has specific needs
>>> w.r.t border handling.
>>>
>>>>> +
>>>>> +static int clear_on_init = -1;
>>>>> +module_param(clear_on_init, int, 0644);
>>>>> +MODULE_PARM_DESC(clear_on_init,
>>>>> + "Clear display on first app launch (-1=disabled,
>>>>> 0=partial, 1=full, 2=fast)");
>>>>> +
>>>>> +static int clear_on_close = -1;
>>>>> +module_param(clear_on_close, int, 0644);
>>>>> +MODULE_PARM_DESC(clear_on_close,
>>>>> + "Clear display on app close/CRTC disable (-1=disabled,
>>>>> 0=partial, 1=full, 2=fast)");
>>>>> +
>>>>> +static int clear_on_disable = -1;
>>>>> +module_param(clear_on_disable, int, 0644);
>>>>> +MODULE_PARM_DESC(clear_on_disable,
>>>>> + "Clear display on CRTC disable/DPMS off (-1=disabled,
>>>>> 0=partial, 1=full, 2=fast)");
>>>>> +
>>>>> +static int refresh_mode_init = -1;
>>>>> +module_param(refresh_mode_init, int, 0644);
>>>>> +MODULE_PARM_DESC(refresh_mode_init,
>>>>> + "Skip baseline establishment on first enable
>>>>> (-1=disabled, 0=partial, 1=full, 2=fast)");
>>>>
>>>> Use 'disabled' for all of them.
>>>>
>>>>> +
>>>>> +static int color_mode = -1;
>>>>> +module_param(color_mode, int, 0644);
>>>>> +MODULE_PARM_DESC(color_mode,
>>>>> + "Color mode (-1=panel default, 0=black-white, 1=3-color;
>>>>> 3- color only valid for panels with red plane support)");
>>>>
>>>> 'Panel default.' Colors should be controlled by DRM clients via
>>>> the framebuffer.
>>>>
>>>
>>> As mentioned previously, say user-space is only supporting and
>>> giving XR24 or XR32 format, from that we can't infer whether
>>> user-space want to drive display in B/W mode or color-mode.
>>>
>>>>> +
>>>>> +/*
>>>>> -----------------------------------------------------------------------
>>>>>
>>>>> + * SSD16xx family common: commands, data values, and bit
>>>>> definitions.
>>>>> + * These apply equally to SSD1673, SSD1680, and SSD1683.
>>>>> + *
>>>>> -----------------------------------------------------------------------
>>>>>
>>>>> + */
>>>>> +
>>>>> +/* SPI command codes (common) */
>>>>> +#define SSD16XX_CMD_DRIVER_OUTPUT_CONTROL 0x01
>>>>> +#define SSD16XX_CMD_DATA_ENTRY_MODE 0x11
>>>>> +#define SSD16XX_CMD_SW_RESET 0x12
>>>>> +#define SSD16XX_CMD_MASTER_ACTIVATION 0x20
>>>>> +#define SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1 0x21
>>>>> +#define SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2 0x22
>>>>> +#define SSD16XX_CMD_WRITE_RAM_BW 0x24
>>>>> +#define SSD16XX_CMD_BORDER_WAVEFORM_CONTROL 0x3C
>>>>> +#define SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END 0x44
>>>>> +#define SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END 0x45
>>>>> +#define SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER 0x4E
>>>>> +#define SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER 0x4F
>>>>> +
>>>>> +/*
>>>>> + * Data Entry Mode (command 0x11) AM/IDY/IDX bit encoding (common).
>>>>> + *
>>>>> + * Bit 2 (AM): Address update direction: 0 = X direction, 1 = Y
>>>>> direction
>>>>> + * ID[1:0] when AM=0 (X-direction modes, address counter advances
>>>>> in X):
>>>>> + * 00 = X decrement, Y decrement 01 = X increment, Y decrement
>>>>> + * 10 = X decrement, Y increment 11 = X increment, Y
>>>>> increment (default)
>>>>> + *
>>>>> + * Rotation to data entry mode mapping (actual implementation
>>>>> uses two modes,
>>>>> + * with scan direction controlled via RAM cursor positioning and
>>>>> manual tweaking):
>>>>> + * 0°/270° → 0x03 (X++, Y++) Landscape/Portrait-CW: cursor at
>>>>> (0, 0)
>>>>> + * 90°/180° → 0x00 (X--, Y--) Portrait-CCW/Upside-down: cursor
>>>>> at (max, max)
>>>>> + *
>>>>> + * The pixel packing in convert_fb_to_1bpp is grouped by physical
>>>>> layout:
>>>>> + * - Portrait (90°/270°): column-major packing, rightmost
>>>>> column first
>>>>> + * - Landscape (0°/180°): row-major packing, top to bottom,
>>>>> left to right
>>>>> + * Hardware cursor position and scan mode handle the final
>>>>> orientation.
>>>>> + */
>>>>> +#define SSD16XX_DATA_ENTRY_XDEC_YDEC 0x00 /* X--, Y-- (X-
>>>>> mode) */
>>>>> +#define SSD16XX_DATA_ENTRY_XINC_YINC 0x03 /* X++, Y++ (X-
>>>>> mode, default) */
>>>>> +
>>>>> +/* POR reset value: GD=0 (G0 first), SM=0 (interlaced), TB=0 (G0-
>>>>> >G299) */
>>>>> +#define SSD16XX_DRIVER_OUTPUT_CTRL_DEFAULT 0x00
>>>>> +
>>>>> +/* Display Update Control 1 (0x21) byte 2 default (common) */
>>>>> +#define SSD16XX_CTRL1_BYTE2_DEFAULT 0x00
>>>>> +
>>>>> +/*
>>>>> + * Display Update Control 2 (0x22) individual bit definitions
>>>>> (common).
>>>>> + * NOTE: BIT(3) is NOT common — see SSD1683_CTRL2_MODE2 in the
>>>>> SSD1683
>>>>> + * section below; it has a completely different meaning in SSD1673.
>>>>> + */
>>>>> +#define SSD16XX_CTRL2_ENABLE_CLK BIT(7)
>>>>> +#define SSD16XX_CTRL2_ENABLE_ANALOG BIT(6)
>>>>> +#define SSD16XX_CTRL2_LOAD_TEMPERATURE BIT(5)
>>>>> +#define SSD16XX_CTRL2_LOAD_LUT BIT(4)
>>>>> +#define SSD16XX_CTRL2_DISPLAY BIT(2)
>>>>> +#define SSD16XX_CTRL2_DISABLE_ANALOG BIT(1)
>>>>> +#define SSD16XX_CTRL2_DISABLE_CLK BIT(0)
>>>>> +
>>>>> +#define SSD16XX_SPI_BITS_PER_WORD 8
>>>>> +#define SSD16XX_SPI_SPEED_DEFAULT 1000000
>>>>> +
>>>>> +/* Maximum time to wait for the BUSY pin to deassert after a
>>>>> display update */
>>>>> +#define SSD16XX_BUSY_WAIT_TIMEOUT_MS 6000
>>>>> +
>>>>> +/*
>>>>> -----------------------------------------------------------------------
>>>>>
>>>>> + * SSD1683 / SSD1680 specific: commands, data values, and bit
>>>>> definitions.
>>>>> + *
>>>>> -----------------------------------------------------------------------
>>>>>
>>>>> + */
>>>>> +
>>>>> +/*
>>>>> + * Deep Sleep Mode values (command 0x10).
>>>>> + */
>>>>> +#define SSD1683_DEEP_SLEEP_MODE_1 0x01 /* RAM
>>>>> retained */
>>>>> +#define SSD1683_DEEP_SLEEP_MODE_2 0x03 /* RAM lost
>>>>> (max power) */
>>>>> +
>>>>> +/*
>>>>> + * Temperature Sensor Selection (command 0x18).
>>>>> + */
>>>>> +#define SSD1683_CMD_TEMPERATURE_SENSOR_CONTROL 0x18
>>>>> +#define SSD1683_TEMP_SENSOR_INTERNAL 0x80 /* Bit 7:
>>>>> use internal sensor */
>>>>> +
>>>>> +/*
>>>>> + * Write RED RAM (command 0x26).
>>>>> + */
>>>>> +#define SSD1683_CMD_WRITE_RAM_RED 0x26
>>>>> +
>>>>> +/*
>>>>> + * Border Waveform Control (command 0x3C) byte values.
>>>>> + */
>>>>> +#define SSD1683_BORDER_WAVEFORM_LUT0 0x00 /* GS
>>>>> Transition LUT0 (black) */
>>>>> +#define SSD1683_BORDER_WAVEFORM_LUT1 0x01 /* GS
>>>>> Transition LUT1 (white) */
>>>>> +#define SSD1683_BORDER_WAVEFORM_LUT2 0x02 /* GS
>>>>> Transition LUT2 (black) */
>>>>> +#define SSD1683_BORDER_WAVEFORM_LUT3 0x03 /* GS
>>>>> Transition LUT3 (gray) */
>>>>> +#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSS 0x40 /* Fix Level
>>>>> VSS (0V, black) */
>>>>> +#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSH1 0x50 /* Fix Level
>>>>> VSH1 (+15V, black) */
>>>>> +#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSL 0x60 /* Fix Level
>>>>> VSL (-15V, white) */
>>>>> +#define SSD1683_BORDER_WAVEFORM_FIXLVL_VSH2 0x70 /* Fix Level
>>>>> VSH2 (+15V alt, black) */
>>>>> +#define SSD1683_BORDER_WAVEFORM_VCOM 0x80 /* Follow VCOM
>>>>> (-2V~-3V, preserve) */
>>>>> +#define SSD1683_BORDER_WAVEFORM_HIZ 0xC0 /* HiZ
>>>>> (floating, default) */
>>>>> +
>>>>> +/*
>>>>> + * Display Update Control 1 (0x21) byte 1 — RED RAM control.
>>>>> + */
>>>>> +#define SSD1683_CTRL1_NORMAL 0x00 /* Both BW and RED
>>>>> RAMs enabled */
>>>>> +#define SSD1683_CTRL1_BYPASS_RED_RAM 0x40 /* Bypass RED
>>>>> RAM (force RED=0) */
>>>>> +
>>>>> +/*
>>>>> + * Display Update Control 2 (0x22) BIT(3) — "Display Mode 2"
>>>>> (partial/BW).
>>>>> + */
>>>>> +#define SSD1683_CTRL2_MODE2 BIT(3)
>>>>> +
>>>>> +/* Composite CTRL2 sequences for each refresh mode */
>>>>> +#define SSD1683_CTRL2_FULL_REFRESH (SSD16XX_CTRL2_ENABLE_CLK | \
>>>>> + SSD16XX_CTRL2_ENABLE_ANALOG | \
>>>>> + SSD16XX_CTRL2_LOAD_TEMPERATURE | \
>>>>> + SSD16XX_CTRL2_LOAD_LUT | \
>>>>> + SSD16XX_CTRL2_DISPLAY | \
>>>>> + SSD16XX_CTRL2_DISABLE_ANALOG | \
>>>>> + SSD16XX_CTRL2_DISABLE_CLK) /* 0xF7, ~1.5-2s */
>>>>> +
>>>>> +#define SSD1683_CTRL2_FAST_REFRESH (SSD16XX_CTRL2_ENABLE_CLK | \
>>>>> + SSD16XX_CTRL2_ENABLE_ANALOG | \
>>>>> + SSD16XX_CTRL2_DISPLAY | \
>>>>> + SSD16XX_CTRL2_DISABLE_ANALOG | \
>>>>> + SSD16XX_CTRL2_DISABLE_CLK) /* 0xC7,
>>>>> ~1.0-1.5s */
>>>>> +
>>>>> +#define SSD1683_CTRL2_PARTIAL_REFRESH (SSD16XX_CTRL2_ENABLE_CLK | \
>>>>> + SSD16XX_CTRL2_ENABLE_ANALOG | \
>>>>> + SSD16XX_CTRL2_LOAD_TEMPERATURE | \
>>>>> + SSD16XX_CTRL2_LOAD_LUT | \
>>>>> + SSD1683_CTRL2_MODE2 | \
>>>>> + SSD16XX_CTRL2_DISPLAY | \
>>>>> + SSD16XX_CTRL2_DISABLE_ANALOG | \
>>>>> + SSD16XX_CTRL2_DISABLE_CLK) /* 0xFF,
>>>>> ~300-500ms */
>>>>> +
>>>>> +/*
>>>>> + * Standalone LUT pre-load sequence (0x91 = ENABLE_CLK | LOAD_LUT
>>>>> | LOAD_TEMPERATURE |
>>>>> + * DISABLE_CLK).
>>>>> + * Pre-loads the OTP LUT without triggering a display update.
>>>>> Required for
>>>>> + * FAST refresh mode (0xC7) which omits LOAD_LUT from each update
>>>>> cycle.
>>>>> + */
>>>>> +#define SSD1683_CTRL2_LOAD_TEMP_LUT (SSD16XX_CTRL2_ENABLE_CLK | \
>>>>> + SSD16XX_CTRL2_LOAD_LUT | \
>>>>> + SSD16XX_CTRL2_LOAD_TEMPERATURE | \
>>>>> + SSD16XX_CTRL2_DISABLE_CLK) /* 0xB1 */
>>>>> +
>>>>> +MODULE_IMPORT_NS("DMA_BUF");
>>>>> +
>>>>> +enum ssd16xx_controller {
>>>>> + SSD1683 = 1,
>>>>> +};
>>>>> +
>>>>> +enum ssd16xx_model {
>>>>> + GDEY042T81 = 1,
>>>>> +};
>>>>> +
>>>>> +enum ssd16xx_refresh_mode {
>>>>> + SSD16XX_REFRESH_PARTIAL = 0, /* Partial refresh (~300-500ms) */
>>>>> + SSD16XX_REFRESH_FULL, /* Full refresh (~1.5-2s) */
>>>>> + SSD16XX_REFRESH_FAST, /* Fast refresh, skip temp load
>>>>> (~1.0-1.5s) */
>>>>> +};
>>>>> +
>>>>> +enum ssd16xx_color_mode {
>>>>> + SSD16XX_COLOR_MODE_BW = 0, /* Black/white only; RED RAM
>>>>> always bypassed */
>>>>> + SSD16XX_COLOR_MODE_3COLOR = 1, /* 3-colour BWR; RED RAM used
>>>>> for red pixels */
>>>>> +};
>>>>> +
>>>>> +/* Border waveform enum indices (0-9); mapped to HW bytes via
>>>>> + * controller_cfg->border_waveform_table[]
>>>>> + */
>>>>> +enum ssd16xx_border_waveform {
>>>>> + SSD16XX_BORDER_LUT0 = 0, /* GS Transition LUT0 (black) */
>>>>> + SSD16XX_BORDER_LUT1, /* GS Transition LUT1 (white) */
>>>>> + SSD16XX_BORDER_LUT2, /* GS Transition LUT2 (black) */
>>>>> + SSD16XX_BORDER_LUT3, /* GS Transition LUT3 (gray) */
>>>>> + SSD16XX_BORDER_VSS, /* Fix Level VSS (black) */
>>>>> + SSD16XX_BORDER_VSH1, /* Fix Level VSH1 (black) */
>>>>> + SSD16XX_BORDER_VSL, /* Fix Level VSL (white) */
>>>>> + SSD16XX_BORDER_VSH2, /* Fix Level VSH2 (black) */
>>>>> + SSD16XX_BORDER_VCOM, /* Follow VCOM (preserve) */
>>>>> + SSD16XX_BORDER_HIZ, /* HiZ (floating, default) */
>>>>> +};
>>>>> +
>>>>> +/* SSD1683/SSD1680 border waveform byte encoding for command 0x3C */
>>>>> +static const u8 ssd1683_border_waveform_table[] = {
>>>>> + [SSD16XX_BORDER_LUT0] = SSD1683_BORDER_WAVEFORM_LUT0,
>>>>> + [SSD16XX_BORDER_LUT1] = SSD1683_BORDER_WAVEFORM_LUT1,
>>>>> + [SSD16XX_BORDER_LUT2] = SSD1683_BORDER_WAVEFORM_LUT2,
>>>>> + [SSD16XX_BORDER_LUT3] = SSD1683_BORDER_WAVEFORM_LUT3,
>>>>> + [SSD16XX_BORDER_VSS] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSS,
>>>>> + [SSD16XX_BORDER_VSH1] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSH1,
>>>>> + [SSD16XX_BORDER_VSL] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSL,
>>>>> + [SSD16XX_BORDER_VSH2] = SSD1683_BORDER_WAVEFORM_FIXLVL_VSH2,
>>>>> + [SSD16XX_BORDER_VCOM] = SSD1683_BORDER_WAVEFORM_VCOM,
>>>>> + [SSD16XX_BORDER_HIZ] = SSD1683_BORDER_WAVEFORM_HIZ,
>>>>> +};
>>>>> +
>>>>> +struct ssd16xx_controller_config {
>>>>> + u16 max_width;
>>>>> + u16 max_height;
>>>>> + u8 ram_x_address_bits;
>>>>> + u8 ram_y_address_bits;
>>>>> +
>>>>> + /*
>>>>> + * has_temp_sensor_ctrl: controller supports command 0x18
>>>>> (Temperature
>>>>> + * Sensor Selection). Present in SSD1683/SSD1680; absent in
>>>>> SSD1673
>>>>> + * which uses command 0x1A (direct temperature write) instead.
>>>>> + */
>>>>> + bool has_temp_sensor_ctrl;
>>>>> +
>>>>> + /*
>>>>> + * Deep sleep mode byte values for command 0x10.
>>>>> + * deep_sleep_mode_level1: lower-power sleep, RAM content
>>>>> retained
>>>>> + * (MODE_1 on SSD1683/SSD1680; used for runtime idle /
>>>>> app- close).
>>>>> + * deep_sleep_mode_level2: maximum power savings, RAM may
>>>>> be lost
>>>>> + * (MODE_2 on SSD1683/SSD1680; used for system suspend).
>>>>> + * Chips with a single sleep mode set both fields to the same
>>>>> value.
>>>>> + */
>>>>> + u8 deep_sleep_mode_level1;
>>>>> + u8 deep_sleep_mode_level2;
>>>>> +
>>>>> + /*
>>>>> + * border_waveform_table: chip-specific byte values for the
>>>>> 10 logical
>>>>> + * border waveform modes (indexed by enum
>>>>> ssd16xx_border_waveform).
>>>>> + * The encoding of command 0x3C differs between
>>>>> SSD1683/SSD1680 and
>>>>> + * SSD1673, so each controller provides its own translation
>>>>> table.
>>>>> + */
>>>>> + const u8 *border_waveform_table;
>>>>> +
>>>>> + /*
>>>>> + * Display Update Control 1 (cmd 0x21) byte 1 values.
>>>>> + * ctrl1_normal: both BW and RED RAMs participate in
>>>>> the waveform.
>>>>> + * ctrl1_bypass_red_ram: RED RAM bypassed; waveform driven
>>>>> from BW RAM only.
>>>>> + * SSD1673 has no RED RAM so both fields carry the same value.
>>>>> + */
>>>>> + u8 ctrl1_normal;
>>>>> + u8 ctrl1_bypass_red_ram;
>>>>> +
>>>>> + /*
>>>>> + * Display Update Control 2 (cmd 0x22) composite sequences
>>>>> for each
>>>>> + * refresh mode (indexed by enum ssd16xx_refresh_mode) and the
>>>>> + * standalone LUT pre-load sequence used before fast refresh.
>>>>> + * Values differ between SSD1683/SSD1680 and SSD1673 (MODE2
>>>>> bit, etc.).
>>>>> + */
>>>>> + u8 ctrl2_refresh[3]; /* indexed by
>>>>> SSD16XX_REFRESH_PARTIAL/ FULL/FAST */
>>>>> + u8 ctrl2_load_temp_lut; /* standalone LUT pre-load (no
>>>>> display update) */
>>>>> +};
>>>>> +
>>>>> +struct ssd16xx_panel_config {
>>>>> + /* Data Entry Mode - controls X/Y increment direction for
>>>>> landscape (0°) */
>>>>> + u8 data_entry_mode;
>>>>> +
>>>>> + /* Driver Output Control - third byte (scan direction) */
>>>>> + u8 driver_output_ctrl_byte3;
>>>>> +
>>>>> + /* Default refresh mode for this panel */
>>>>> + enum ssd16xx_refresh_mode default_refresh_mode;
>>>>> +
>>>>> + /* Default border waveform during clear/init (enum index 0-9) */
>>>>> + enum ssd16xx_border_waveform default_border_waveform_init;
>>>>> +
>>>>> + /* Default border waveform during display updates (enum index
>>>>> 0-9) */
>>>>> + enum ssd16xx_border_waveform default_border_waveform_update;
>>>>> +
>>>>> + /* Whether to re-send border waveform command before each
>>>>> display update */
>>>>> + bool default_border_refresh_on_every_update;
>>>>> +
>>>>> + /*
>>>>> + * Default clear-on-init behaviour.
>>>>> + * -1=disabled, 0=partial, 1=full, 2=fast (matches enum
>>>>> ssd16xx_refresh_mode)
>>>>> + */
>>>>> + int default_clear_on_init;
>>>>> +
>>>>> + /* Default clear-on-close behaviour (-1=disabled, 0=partial,
>>>>> 1=full, 2=fast) */
>>>>> + int default_clear_on_close;
>>>>> +
>>>>> + /* Default clear-on-disable behaviour (-1=disabled,
>>>>> 0=partial, 1=full, 2=fast) */
>>>>> + int default_clear_on_disable;
>>>>> +
>>>>> + /*
>>>>> + * Default refresh-mode-init: -1=disabled, else skip baseline
>>>>> establishment
>>>>> + * and start directly in this refresh mode.
>>>>> + */
>>>>> + int default_refresh_mode_init;
>>>>> +
>>>>> + /*
>>>>> + * Whether this panel has a physical red colour plane
>>>>> (3-colour BWR).
>>>>> + * false: 2-colour black/white only; the RED RAM is always
>>>>> bypassed.
>>>>> + * true: 3-colour panel; full-refresh writes to the RED RAM
>>>>> so that
>>>>> + * red pixels are driven through the red waveform.
>>>>> + */
>>>>> + bool red_supported;
>>>>> +
>>>>> + /* Panel-specific display mode (resolution and physical
>>>>> dimensions) */
>>>>> + const struct drm_display_mode *mode;
>>>>> +};
>>>>> +
>>>>> +struct ssd16xx_panel {
>>>>
>>>> Better call this 'struct ssd16xx_device' and the rsp variables
>>>> 'ssd16xx'. As mentioned, the name 'panel' already has a specific
>>>> meaning in DRM.
>>>>
>>>
>>> Alright I can do that, I thought folks won't confuse it since this
>>> is not importing drm_panel struct.
>>>
>>>>
>>>>> + struct drm_device drm;
>>>>> +
>>>>> + struct drm_plane primary_plane;
>>>>> + struct drm_crtc crtc;
>>>>> + struct drm_encoder encoder;
>>>>> + struct drm_connector connector;
>>>>> +
>>>>> + struct spi_device *spi;
>>>>> + struct gpio_desc *reset;
>>>>> + struct gpio_desc *busy;
>>>>> + struct gpio_desc *dc;
>>>>> +
>>>>> + enum ssd16xx_model model;
>>>>> + enum ssd16xx_controller controller;
>>>>> + const struct ssd16xx_controller_config *controller_cfg;
>>>>> + const struct ssd16xx_panel_config *panel_cfg;
>>>>> + struct drm_display_mode *mode;
>>>>> + u32 width;
>>>>> + u32 height;
>>>>> +
>>>>> + bool initialized;
>>>>> + bool reinit_pending; /* HW re-init required after
>>>>> orientation change */
>>>>> + bool init_refresh_pending; /* First frame after
>>>>> refresh_mode_init enable */
>>>>> + bool first_clear_done; /* clear_on_init has already fired
>>>>> once */
>>>>> + bool display_cleared_on_deinit; /* Avoid redundant clear in
>>>>> atomic_disable/master_drop */
>>>>> +
>>>>> + int orientation; /* Display orientation in degrees:
>>>>> 0/90/180/270 */
>>>>> + enum ssd16xx_refresh_mode refresh_mode; /* Active refresh
>>>>> mode */
>>>>> + enum ssd16xx_color_mode color_mode; /* Active color mode
>>>>> (BW or 3-color) */
>>>>> + bool fast_lut_pending; /* LUT pre-load needed before next
>>>>> fast refresh */
>>>>> +
>>>>> + /* Border waveform (as enum indices) */
>>>>> + int border_waveform_init_idx; /* Border waveform during
>>>>> clear/ init */
>>>>> + int border_waveform_update_idx; /* Border waveform during
>>>>> display updates */
>>>>> + bool border_refresh_on_every_update; /* Re-send border cmd
>>>>> each display update */
>>>>> + bool border_waveform_pending; /* One-shot: send border cmd
>>>>> on next update */
>>>>> +
>>>>> + /* Display control */
>>>>> + int clear_on_init; /* -1=disabled, 0=partial, 1=full,
>>>>> 2=fast */
>>>>> + int clear_on_close; /* -1=disabled, 0=partial, 1=full,
>>>>> 2=fast */
>>>>> + int clear_on_disable; /* -1=disabled, 0=partial, 1=full,
>>>>> 2=fast */
>>>>> + int refresh_mode_init; /* -1=disabled, else use this mode for
>>>>> the first frame */
>>>>> +
>>>>> + u8 *tx_buf; /* 1bpp frame buffer (mono + white) */
>>>>> + u8 *tx_red_buf; /* 1bpp red-channel buffer (3-color panels
>>>>> only) */
>>>>> + u16 *tx_buf9; /* 9-bit SPI expansion buffer (3-wire mode
>>>>> only) */
>>>>> +
>>>>> + struct drm_framebuffer *last_fb; /* Last drawn FB for
>>>>> reinit redraws */
>>>>> + struct drm_property *rotation_property;
>>>>> + struct drm_property *refresh_mode_property;
>>>>> + struct drm_property *border_waveform_init_property;
>>>>> + struct drm_property *border_waveform_update_property;
>>>>> + struct drm_property *border_refresh_on_every_update_property;
>>>>> + struct drm_property *clear_on_init_property;
>>>>> + struct drm_property *clear_on_close_property;
>>>>> + struct drm_property *clear_on_disable_property;
>>>>> + struct drm_property *refresh_mode_init_property;
>>>>> + struct drm_property *color_mode_property;
>>>>> +};
>>>>> +
>>>>> +static inline struct ssd16xx_panel *to_ssd16xx_panel(struct
>>>>> drm_device *drm)
>>>>> +{
>>>>> + return container_of(drm, struct ssd16xx_panel, drm);
>>>>> +}
>>>>> +
>>>>> +static inline struct ssd16xx_panel *crtc_to_ssd16xx_panel(struct
>>>>> drm_crtc *crtc)
>>>>> +{
>>>>> + return container_of(crtc, struct ssd16xx_panel, crtc);
>>>>> +}
>>>>> +
>>>>> +static inline struct ssd16xx_panel *plane_to_ssd16xx_panel(struct
>>>>> drm_plane *plane)
>>>>> +{
>>>>> + return container_of(plane, struct ssd16xx_panel, primary_plane);
>>>>> +}
>>>>> +
>>>>> +static const struct ssd16xx_controller_config
>>>>> ssd16xx_controller_configs[] = {
>>>>> + [SSD1683] = {
>>>>> + .max_width = 400,
>>>>> + .max_height = 300,
>>>>> + .ram_x_address_bits = 8,
>>>>> + .ram_y_address_bits = 16,
>>>>> + .has_temp_sensor_ctrl = true,
>>>>> + .deep_sleep_mode_level1 = SSD1683_DEEP_SLEEP_MODE_1,
>>>>> + .deep_sleep_mode_level2 = SSD1683_DEEP_SLEEP_MODE_2,
>>>>> + .border_waveform_table = ssd1683_border_waveform_table,
>>>>> + .ctrl1_normal = SSD1683_CTRL1_NORMAL,
>>>>> + .ctrl1_bypass_red_ram = SSD1683_CTRL1_BYPASS_RED_RAM,
>>>>> + .ctrl2_refresh = {
>>>>> + [SSD16XX_REFRESH_PARTIAL] =
>>>>> SSD1683_CTRL2_PARTIAL_REFRESH,
>>>>> + [SSD16XX_REFRESH_FULL] = SSD1683_CTRL2_FULL_REFRESH,
>>>>> + [SSD16XX_REFRESH_FAST] = SSD1683_CTRL2_FAST_REFRESH,
>>>>> + },
>>>>> + .ctrl2_load_temp_lut = SSD1683_CTRL2_LOAD_TEMP_LUT,
>>>>> + },
>>>>> +};
>>>>> +
>>>>> +/* GDEY042T81: 4.2" 400x300 panel, 84.8x63.6mm active area */
>>>>> +static const struct drm_display_mode gdey042t81_mode = {
>>>>> + DRM_SIMPLE_MODE(400, 300, 85, 64),
>>>>> +};
>>>>> +
>>>>> +static const struct ssd16xx_panel_config ssd16xx_panel_configs[] = {
>>>>> + [GDEY042T81] = {
>>>>> + .data_entry_mode = SSD16XX_DATA_ENTRY_XINC_YINC,
>>>>> + .driver_output_ctrl_byte3 =
>>>>> SSD16XX_DRIVER_OUTPUT_CTRL_DEFAULT,
>>>>> + .default_refresh_mode = SSD16XX_REFRESH_PARTIAL,
>>>>> + .default_border_waveform_init = SSD16XX_BORDER_LUT1,
>>>>> /* white, clean clear */
>>>>> + .default_border_waveform_update = SSD16XX_BORDER_HIZ,
>>>>> /* floating, preserve */
>>>>> + .default_border_refresh_on_every_update = false,
>>>>> + .default_clear_on_init = -1,
>>>>> + .default_clear_on_close = -1,
>>>>> + .default_clear_on_disable = -1,
>>>>> + .default_refresh_mode_init = SSD16XX_REFRESH_FULL,
>>>>> + .red_supported = false, /* 2-colour black/white panel */
>>>>> + .mode = &gdey042t81_mode,
>>>>> + },
>>>>> +};
>>>>> +
>>>>> +static void ssd16xx_wait_for_panel(struct ssd16xx_panel *panel,
>>>>> + int *err)
>>>>> +{
>>>>> + unsigned long timeout_jiffies = jiffies +
>>>>> + msecs_to_jiffies(SSD16XX_BUSY_WAIT_TIMEOUT_MS);
>>>>> + unsigned long start_ms = jiffies_to_msecs(jiffies);
>>>>> + int busy_val;
>>>>> +
>>>>> + if (*err)
>>>>> + return;
>>>>
>>>> This is good. It'll simplify error handling in other places.
>>>>
>>>>> +
>>>>> + busy_val = gpiod_get_value_cansleep(panel->busy);
>>>>> + drm_dbg(&panel->drm, "BUSY initial value: %d\n", busy_val);
>>>>> +
>>>>> + while (gpiod_get_value_cansleep(panel->busy) == 1) {
>>>>> + if (time_after(jiffies, timeout_jiffies)) {
>>>>> + drm_err(&panel->drm, "Busy wait timed out after
>>>>> %lums\n",
>>>>> + jiffies_to_msecs(jiffies) - start_ms);
>>>>> + *err = -ETIMEDOUT;
>>>>> + return;
>>>>> + }
>>>>> + usleep_range(100, 200);
>>>>> + }
>>>>> +
>>>>> + drm_dbg(&panel->drm, "BUSY became ready after %lums\n",
>>>>> + jiffies_to_msecs(jiffies) - start_ms);
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_spi_sync(struct spi_device *spi, struct
>>>>> spi_message *msg,
>>>>> + int *err)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + if (*err)
>>>>> + return;
>>>>> +
>>>>> + ret = spi_sync(spi, msg);
>>>>> + if (ret < 0)
>>>>> + *err = ret;
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_send_cmd(struct ssd16xx_panel *panel, u8 cmd,
>>>>> + int *err)
>>>>> +{
>>>>> + u16 word;
>>>>> + struct spi_transfer xfer = {};
>>>>> + struct spi_message msg;
>>>>> +
>>>>> + if (*err)
>>>>> + return;
>>>>> +
>>>>> + spi_message_init(&msg);
>>>>> + spi_message_add_tail(&xfer, &msg);
>>>>> +
>>>>> + if (panel->dc) {
>>>>> + /* 4-wire SPI: D/C# GPIO low selects command mode */
>>>>> + xfer.tx_buf = &cmd;
>>>>> + xfer.len = 1;
>>>>> + gpiod_set_value_cansleep(panel->dc, 0);
>>>>> + } else {
>>>>> + /*
>>>>> + * 3-wire SPI (9-bit): bit 8 is the D/C# bit.
>>>>> + * D/C# = 0 means the following 8 bits are a command.
>>>>> + */
>>>>> + word = cmd; /* bit 8 = 0 for command */
>>>>> + xfer.tx_buf = &word;
>>>>> + xfer.len = sizeof(u16);
>>>>> + xfer.bits_per_word = 9;
>>>>> + }
>>>>> +
>>>>> + ssd16xx_spi_sync(panel->spi, &msg, err);
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_send_data(struct ssd16xx_panel *panel, u8 data,
>>>>> + int *err)
>>>>> +{
>>>>> + u16 word;
>>>>> + struct spi_transfer xfer = {};
>>>>> + struct spi_message msg;
>>>>> +
>>>>> + if (*err)
>>>>> + return;
>>>>> +
>>>>> + spi_message_init(&msg);
>>>>> + spi_message_add_tail(&xfer, &msg);
>>>>> +
>>>>> + if (panel->dc) {
>>>>> + /* 4-wire SPI: D/C# GPIO high selects data mode */
>>>>> + xfer.tx_buf = &data;
>>>>> + xfer.len = 1;
>>>>> + gpiod_set_value_cansleep(panel->dc, 1);
>>>>> + } else {
>>>>> + /*
>>>>> + * 3-wire SPI (9-bit): bit 8 is the D/C# bit.
>>>>> + * D/C# = 1 means the following 8 bits are data.
>>>>> + */
>>>>> + word = 0x100 | data;
>>>>> + xfer.tx_buf = &word;
>>>>> + xfer.len = sizeof(u16);
>>>>> + xfer.bits_per_word = 9;
>>>>> + }
>>>>> +
>>>>> + ssd16xx_spi_sync(panel->spi, &msg, err);
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_send_x_param(struct ssd16xx_panel *panel, u16 x,
>>>>> + int *err)
>>>>> +{
>>>>> + if (*err)
>>>>> + return;
>>>>> +
>>>>> + if (panel->controller_cfg->ram_x_address_bits == 8) {
>>>>> + ssd16xx_send_data(panel, (u8)x, err);
>>>>> + } else {
>>>>> + ssd16xx_send_data(panel, x & 0xFF, err);
>>>>> + ssd16xx_send_data(panel, (x >> 8) & 0xFF, err);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_send_y_param(struct ssd16xx_panel *panel, u16 y,
>>>>> + int *err)
>>>>> +{
>>>>> + if (*err)
>>>>> + return;
>>>>> +
>>>>> + if (panel->controller_cfg->ram_y_address_bits == 8) {
>>>>> + ssd16xx_send_data(panel, (u8)y, err);
>>>>> + } else {
>>>>> + ssd16xx_send_data(panel, y & 0xFF, err);
>>>>> + ssd16xx_send_data(panel, (y >> 8) & 0xFF, err);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_send_data_bulk(struct ssd16xx_panel *panel,
>>>>> + const u8 *data, size_t len,
>>>>> + int *err)
>>>>> +{
>>>>> + struct spi_transfer xfer = {};
>>>>> + struct spi_message msg;
>>>>> +
>>>>> + if (*err)
>>>>> + return;
>>>>> +
>>>>> + if (!data || !len)
>>>>> + return;
>>>>> +
>>>>> + spi_message_init(&msg);
>>>>> + spi_message_add_tail(&xfer, &msg);
>>>>> +
>>>>> + if (panel->dc) {
>>>>> + /* 4-wire SPI: D/C# GPIO high selects data mode */
>>>>> + xfer.tx_buf = data;
>>>>> + xfer.len = len;
>>>>> + gpiod_set_value_cansleep(panel->dc, 1);
>>>>> + ssd16xx_spi_sync(panel->spi, &msg, err);
>>>>> + } else {
>>>>> + /* 3-wire (9-bit): expand u8 → u16 with D/C#=1 in bit 8. */
>>>>> + size_t i;
>>>>> + u16 *buf = panel->tx_buf9;
>>>>> +
>>>>> + for (i = 0; i < len; i++)
>>>>> + buf[i] = 0x100 | data[i];
>>>>> +
>>>>> + xfer.tx_buf = buf;
>>>>> + xfer.len = len * sizeof(u16);
>>>>> + xfer.bits_per_word = 9;
>>>>> + ssd16xx_spi_sync(panel->spi, &msg, err);
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_display_update(struct ssd16xx_panel *panel,
>>>>> + u8 ctrl1_byte1, u8 ctrl1_byte2, u8 ctrl2_mode,
>>>>> + int *err)
>>>>> +{
>>>>> + if (*err)
>>>>> + return;
>>>>> +
>>>>> + drm_dbg(&panel->drm,
>>>>> + "display_update: Setting ctrl1=0x%02x,0x%02x mode=0x%02x\n",
>>>>> + ctrl1_byte1, ctrl1_byte2, ctrl2_mode);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1,
>>>>> err);
>>>>> + ssd16xx_send_data(panel, ctrl1_byte1, err);
>>>>> + ssd16xx_send_data(panel, ctrl1_byte2, err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2,
>>>>> err);
>>>>> + ssd16xx_send_data(panel, ctrl2_mode, err);
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_MASTER_ACTIVATION, err);
>>>>> +
>>>>> + drm_dbg(&panel->drm,
>>>>> + "display_update: Master activation sent, waiting...\n");
>>>>> +
>>>>> + ssd16xx_wait_for_panel(panel, err);
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_hw_reset(struct ssd16xx_panel *panel)
>>>>> +{
>>>>> + gpiod_set_value_cansleep(panel->reset, 1);
>>>>> + usleep_range(10000, 11000);
>>>>> + gpiod_set_value_cansleep(panel->reset, 0);
>>>>> + usleep_range(10000, 11000);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * ssd16xx_preload_fast_lut() - pre-load the OTP LUT for fast
>>>>> refresh mode.
>>>>> + *
>>>>> + * Fast refresh (CTRL2 = 0xC7) omits the LOAD_LUT step on every
>>>>> update to save
>>>>> + * time. It relies on the LUT being loaded upfront via this
>>>>> standalone sequence
>>>>> + * (CTRL2 = 0xB1: ENABLE_CLK | LOAD_LUT |
>>>>> SSD16XX_CTRL2_LOAD_TEMPERATURE | DISABLE_CLK,
>>>>> + * no display update).
>>>>> + *
>>>>> + * Must be called when:
>>>>> + * a) hw_init runs with refresh_mode == FAST, and
>>>>> + * b) switching to fast refresh from a mode that did not leave
>>>>> a valid Mode1
>>>>> + * LUT in the controller (i.e. previous mode was not FULL
>>>>> refresh, which
>>>>> + * carries LOAD_LUT in its own CTRL2 sequence).
>>>>> + */
>>>>> +static int ssd16xx_preload_fast_lut(struct ssd16xx_panel *panel)
>>>>> +{
>>>>> + int err = 0;
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1,
>>>>> &err);
>>>>> + ssd16xx_send_data(panel, panel->controller_cfg-
>>>>> >ctrl1_bypass_red_ram, &err);
>>>>> + ssd16xx_send_data(panel, SSD16XX_CTRL1_BYTE2_DEFAULT, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2,
>>>>> &err);
>>>>> + ssd16xx_send_data(panel, panel->controller_cfg-
>>>>> >ctrl2_load_temp_lut, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_MASTER_ACTIVATION, &err);
>>>>> + ssd16xx_wait_for_panel(panel, &err);
>>>>> +
>>>>> + return err;
>>>>> +}
>>>>> +
>>>>> +static int ssd16xx_hw_init(struct ssd16xx_panel *panel)
>>>>> +{
>>>>> + int err = 0;
>>>>> + u16 ram_height = panel->controller_cfg->max_height;
>>>>> + u8 data_entry_mode;
>>>>> +
>>>>> + ssd16xx_hw_reset(panel);
>>>>> +
>>>>> + /* Software reset */
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_SW_RESET, &err);
>>>>> + ssd16xx_wait_for_panel(panel, &err);
>>>>> +
>>>>> + /* Driver output control (0x01): MUX ratio and scan
>>>>> direction. */
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DRIVER_OUTPUT_CONTROL,
>>>>> &err);
>>>>> + ssd16xx_send_y_param(panel, ram_height - 1, &err);
>>>>> + ssd16xx_send_data(panel, panel->panel_cfg-
>>>>> >driver_output_ctrl_byte3, &err);
>>>>> +
>>>>> + /* Internal temperature sensor (SSD1683/SSD1680 only; not
>>>>> present in SSD1673) */
>>>>> + if (panel->controller_cfg->has_temp_sensor_ctrl) {
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD1683_CMD_TEMPERATURE_SENSOR_CONTROL, &err);
>>>>> + ssd16xx_send_data(panel, SSD1683_TEMP_SENSOR_INTERNAL,
>>>>> &err);
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * For FAST refresh mode, pre-load the LUT once here during
>>>>> initialization.
>>>>> + * FAST mode ctrl2 (0xC7) omits LOAD_LUT on every update for
>>>>> speed, so the
>>>>> + * LUT must be loaded upfront. FULL (0xF7) and PARTIAL (0xFF)
>>>>> load LUT on
>>>>> + * every update, so no preload is needed for those modes.
>>>>> + */
>>>>> + if (panel->refresh_mode == SSD16XX_REFRESH_FAST) {
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_DISPLAY_UPDATE_CONTROL1, &err);
>>>>> + ssd16xx_send_data(panel, panel->controller_cfg-
>>>>> >ctrl1_bypass_red_ram, &err);
>>>>> + ssd16xx_send_data(panel, SSD16XX_CTRL1_BYTE2_DEFAULT, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_DISPLAY_UPDATE_CONTROL2, &err);
>>>>> + ssd16xx_send_data(panel, panel->controller_cfg-
>>>>> >ctrl2_load_temp_lut, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_MASTER_ACTIVATION,
>>>>> &err);
>>>>> + ssd16xx_wait_for_panel(panel, &err);
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * Set Data Entry Mode (0x11) based on orientation. This
>>>>> controls
>>>>> + * how the RAM address counter auto-advances after each byte
>>>>> write.
>>>>> + *
>>>>> + * Implementation uses two data entry modes:
>>>>> + * - 90°/180° use XDEC_YDEC (0x00): X--, Y-- with cursor at
>>>>> (max, max)
>>>>> + * - 0°/270° use XINC_YINC (0x03): X++, Y++ with cursor at
>>>>> (0, 0)
>>>>> + *
>>>>> + * The convert_fb_to_1bpp packing is grouped by physical layout:
>>>>> + * - Portrait orientations (90°/270°): column-major packing
>>>>> + * - Landscape orientations (0°/180°): row-major packing
>>>>> + *
>>>>> + * Final scan direction and image orientation are controlled
>>>>> by the
>>>>> + * combination of data entry mode and RAM cursor position set
>>>>> in fb_dirty.
>>>>> + *
>>>>> + * The RAM address window and cursor are NOT set here; fb_dirty
>>>>> + * always programmes them (with the correct end-before-start
>>>>> order
>>>>> + * for decrement modes) immediately before writing frame data.
>>>>> + */
>>>>> + switch (panel->orientation) {
>>>>
>>>> As mentioned, use the connector property instead.
>>>>
>>>>> + case 90:
>>>>> + case 180:
>>>>> + data_entry_mode = SSD16XX_DATA_ENTRY_XDEC_YDEC;
>>>>> + break;
>>>>> + default: /* 0°/270° */
>>>>> + data_entry_mode = SSD16XX_DATA_ENTRY_XINC_YINC;
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_DATA_ENTRY_MODE, &err);
>>>>> + ssd16xx_send_data(panel, data_entry_mode, &err);
>>>>> + drm_dbg(&panel->drm, "hw_init: orientation=%u°
>>>>> data_entry=0x%02x\n",
>>>>> + panel->orientation, data_entry_mode);
>>>>> +
>>>>> + ssd16xx_wait_for_panel(panel, &err);
>>>>> +
>>>>> + if (err)
>>>>> + drm_err(&panel->drm, "Hardware initialization failed:
>>>>> %d\n", err);
>>>>> +
>>>>> + return err;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Clear display by writing all-white to both BW and RED RAM.
>>>>> + * The ctrl2 argument selects the waveform (full/partial/fast
>>>>> refresh).
>>>>> + * Border waveform is set to init value before clearing, then
>>>>> restored
>>>>> + * to the update value to preserve the border during subsequent
>>>>> updates.
>>>>> + */
>>>>> +static int ssd16xx_clear_display(struct ssd16xx_panel *panel, u8
>>>>> ctrl2)
>>>>> +{
>>>>> + const u8 *bw_tbl = panel->controller_cfg->border_waveform_table;
>>>>> + int err = 0;
>>>>> + unsigned int data_size = (panel->width * panel->height) / 8;
>>>>> + u8 *white_buffer = panel->tx_buf;
>>>>> +
>>>>> + memset(white_buffer, 0xFF, data_size);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
>>>>> + ssd16xx_send_x_param(panel, 0x00, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
>>>>> + ssd16xx_send_y_param(panel, 0x00, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_WRITE_RAM_BW, &err);
>>>>> + ssd16xx_send_data_bulk(panel, white_buffer, data_size, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD1683_CMD_WRITE_RAM_RED, &err);
>>>>> + ssd16xx_send_data_bulk(panel, white_buffer, data_size, &err);
>>>>> +
>>>>> + /* Set border waveform for the clear operation */
>>>>> + drm_dbg(&panel->drm, "clear_display: Set border init
>>>>> waveform: 0x%02x\n",
>>>>> + bw_tbl[panel->border_waveform_init_idx]);
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_BORDER_WAVEFORM_CONTROL,
>>>>> &err);
>>>>> + ssd16xx_send_data(panel,
>>>>> + bw_tbl[panel->border_waveform_init_idx],
>>>>> + &err);
>>>>> +
>>>>> + /* 3-colour mode: CTRL1_NORMAL (read both RAMs); BW mode:
>>>>> bypass RED. */
>>>>> + ssd16xx_display_update(panel,
>>>>> + panel->color_mode == SSD16XX_COLOR_MODE_3COLOR
>>>>> + ? panel->controller_cfg->ctrl1_normal
>>>>> + : panel->controller_cfg->ctrl1_bypass_red_ram,
>>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT, ctrl2, &err);
>>>>> +
>>>>> + /* Restore border waveform to update/preservation value */
>>>>> + drm_dbg(&panel->drm, "clear_display: Restored border update
>>>>> waveform: 0x%02x\n",
>>>>> + bw_tbl[panel->border_waveform_update_idx]);
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_BORDER_WAVEFORM_CONTROL,
>>>>> &err);
>>>>> + ssd16xx_send_data(panel,
>>>>> + bw_tbl[panel->border_waveform_update_idx],
>>>>> + &err);
>>>>> +
>>>>> + return err;
>>>>> +}
>>>>> +
>>>>> +static u8 ssd16xx_refresh_mode_to_ctrl2(struct ssd16xx_panel *panel,
>>>>> + enum ssd16xx_refresh_mode mode)
>>>>> +{
>>>>> + if (mode < ARRAY_SIZE(panel->controller_cfg->ctrl2_refresh))
>>>>> + return panel->controller_cfg->ctrl2_refresh[mode];
>>>>> + return
>>>>> panel->controller_cfg->ctrl2_refresh[SSD16XX_REFRESH_FULL];
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Clear display on new DRM master open (if clear_on_init >= 0).
>>>>> + * Guarded by panel->first_clear_done; master_drop resets it
>>>>> unconditionally
>>>>> + * so each new client session gets a fresh clear.
>>>>> + */
>>>>> +static int ssd16xx_clear_display_on_init(struct ssd16xx_panel
>>>>> *panel)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + if (panel->clear_on_init < 0 || panel->first_clear_done)
>>>>> + return 0;
>>>>> +
>>>>> + drm_dbg(&panel->drm, "clear_on_init: running, mode=%d\n",
>>>>> + panel->clear_on_init);
>>>>> + ret = ssd16xx_clear_display(panel,
>>>>> + ssd16xx_refresh_mode_to_ctrl2(panel, panel-
>>>>> >clear_on_init));
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + panel->first_clear_done = true;
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Clear display when the displaying client exits (if
>>>>> clear_on_close >= 0).
>>>>> + * Called from ssd16xx_drm_master_drop().
>>>>> + */
>>>>> +static int ssd16xx_clear_display_on_exit(struct ssd16xx_panel
>>>>> *panel)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + if (panel->clear_on_close < 0)
>>>>> + return 0;
>>>>> +
>>>>> + drm_dbg(&panel->drm, "clear_on_close: running, mode=%d\n",
>>>>> + panel->clear_on_close);
>>>>> + ret = ssd16xx_clear_display(panel,
>>>>> + ssd16xx_refresh_mode_to_ctrl2(panel, panel-
>>>>> >clear_on_close));
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * ssd16xx_pixel_luma() - return ITU-R BT.601 luminance (0-255)
>>>>> for one pixel.
>>>>> + *
>>>>> + * For colour formats the result is (299*R + 587*G + 114*B) / 1000;
>>>>> + * for luma-only formats the luma byte is returned directly.
>>>>> + *
>>>>> + * R1 is never passed here — it is already 1bpp and is handled
>>>>> directly by
>>>>> + * the callers.
>>>>> + */
>>>>> +static u8 ssd16xx_pixel_luma(struct iosys_map *src,
>>>>> + struct drm_framebuffer *fb,
>>>>> + unsigned int x, unsigned int y)
>>>>> +{
>>>>> + switch (fb->format->format) {
>>>>> + case DRM_FORMAT_XRGB8888: {
>>>>> + u32 *line = (u32 *)(src->vaddr + y * fb->pitches[0]);
>>>>> + u32 px = line[x];
>>>>> + u8 r = (px >> 16) & 0xFF, g = (px >> 8) & 0xFF, b = px &
>>>>> 0xFF;
>>>>> +
>>>>> + return (u8)((299u * r + 587u * g + 114u * b) / 1000u);
>>>>> + }
>>>>> + case DRM_FORMAT_RGB888: {
>>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>>> + u8 r = line[x * 3], g = line[x * 3 + 1], b = line[x * 3 +
>>>>> 2];
>>>>> +
>>>>> + return (u8)((299u * r + 587u * g + 114u * b) / 1000u);
>>>>> + }
>>>>> + case DRM_FORMAT_RGB565: {
>>>>> + u16 *line = (u16 *)(src->vaddr + y * fb->pitches[0]);
>>>>> + u16 px = line[x];
>>>>> + u8 r = ((px >> 11) & 0x1F) << 3;
>>>>> + u8 g = ((px >> 5) & 0x3F) << 2;
>>>>> + u8 b = (px & 0x1F) << 3;
>>>>> +
>>>>> + return (u8)((299u * r + 587u * g + 114u * b) / 1000u);
>>>>> + }
>>>>> + case DRM_FORMAT_R8: {
>>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>>> +
>>>>> + return line[x];
>>>>> + }
>>>>> + case DRM_FORMAT_NV12:
>>>>> + case DRM_FORMAT_NV16:
>>>>> + return ((u8 *)(src->vaddr))[y * fb->pitches[0] + x];
>>>>> + case DRM_FORMAT_YUYV: {
>>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>>> +
>>>>> + return line[x * 2];
>>>>> + }
>>>>> + case DRM_FORMAT_UYVY: {
>>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>>> +
>>>>> + return line[x * 2 + 1];
>>>>> + }
>>>>> + default:
>>>>> + return 0;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * ssd16xx_pixel_is_white() - test whether a pixel maps to white
>>>>> in 1bpp output.
>>>>> + *
>>>>> + * Uses fixed threshold of 127. Pixels with luma strictly greater
>>>>> than 127
>>>>> + * are rendered white.
>>>>> + */
>>>>> +static bool ssd16xx_pixel_is_white(struct iosys_map *src,
>>>>> + struct drm_framebuffer *fb,
>>>>> + unsigned int x, unsigned int y)
>>>>> +{
>>>>> + /* R1 is already binarised; avoid the luma computation
>>>>> entirely. */
>>>>> + if (fb->format->format == DRM_FORMAT_R1) {
>>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>>> +
>>>>> + return !!(line[x / 8] & (1 << (7 - (x % 8))));
>>>>> + }
>>>>> + return ssd16xx_pixel_luma(src, fb, x, y) > 127;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * ssd16xx_pixel_is_red() - test whether a pixel is dominated by
>>>>> the red channel.
>>>>> + *
>>>>> + * Only meaningful for formats that carry RGB information
>>>>> (XRGB8888, RGB888,
>>>>> + * RGB565). For luma-only and monochrome formats there is no red
>>>>> channel, so
>>>>> + * the function always returns false; callers should use
>>>>> ssd16xx_pixel_is_white()
>>>>> + * to obtain the BW value for those formats.
>>>>> + *
>>>>> + * Returns true when the red component exceeds 50% intensity AND
>>>>> is strictly
>>>>> + * greater than both green and blue (dominant red hue).
>>>>> + */
>>>>> +static bool ssd16xx_pixel_is_red(struct iosys_map *src,
>>>>> + struct drm_framebuffer *fb,
>>>>> + unsigned int x, unsigned int y)
>>>>> +{
>>>>> + u32 format = fb->format->format;
>>>>> +
>>>>> + switch (format) {
>>>>> + case DRM_FORMAT_XRGB8888: {
>>>>> + u32 *line = (u32 *)(src->vaddr + y * fb->pitches[0]);
>>>>> + u32 px = line[x];
>>>>> + u8 r = (px >> 16) & 0xFF;
>>>>> + u8 g = (px >> 8) & 0xFF;
>>>>> + u8 b = px & 0xFF;
>>>>> +
>>>>> + return r > 127 && r > g && r > b;
>>>>> + }
>>>>> + case DRM_FORMAT_RGB888: {
>>>>> + u8 *line = (u8 *)(src->vaddr + y * fb->pitches[0]);
>>>>> + u8 r = line[x * 3];
>>>>> + u8 g = line[x * 3 + 1];
>>>>> + u8 b = line[x * 3 + 2];
>>>>> +
>>>>> + return r > 127 && r > g && r > b;
>>>>> + }
>>>>> + case DRM_FORMAT_RGB565: {
>>>>> + u16 *line = (u16 *)(src->vaddr + y * fb->pitches[0]);
>>>>> + u16 px = line[x];
>>>>> + u8 r = ((px >> 11) & 0x1F) << 3;
>>>>> + u8 g = ((px >> 5) & 0x3F) << 2;
>>>>> + u8 b = (px & 0x1F) << 3;
>>>>> +
>>>>> + return r > 127 && r > g && r > b;
>>>>> + }
>>>>> + default:
>>>>> + return false; /* No colour channel information */
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * ssd16xx_convert_fb_to_3color() - split a framebuffer into BW
>>>>> and RED planes.
>>>>> + * @bw_dst: output buffer for the black/white RAM plane
>>>>> (1=white, 0=black)
>>>>> + * @red_dst: output buffer for the red RAM plane (1=red, 0=not red)
>>>>> + * @src: mapped framebuffer memory
>>>>> + * @fb: DRM framebuffer descriptor
>>>>> + * @rect: region to convert (must be aligned to 8-pixel
>>>>> boundaries)
>>>>> + *
>>>>> + * Each output buffer must be at least rect_width/8 * rect_height
>>>>> bytes.
>>>>> + * Pixels are classified as:
>>>>> + * - red: written to red_dst as 1, bw_dst as 0 (black)
>>>>> + * - white: written to bw_dst as 1, red_dst as 0
>>>>> + * - black: written to both as 0
>>>>> + *
>>>>> + * For monochrome formats (R1) where no colour information is
>>>>> available the
>>>>> + * source data is copied verbatim to bw_dst and red_dst is
>>>>> cleared to 0xFF
>>>>> + * (all-white = no red pixels).
>>>>> + */
>>>>> +static void ssd16xx_convert_fb_to_3color(u8 *bw_dst, u8 *red_dst,
>>>>> + struct iosys_map *src,
>>>>> + struct drm_framebuffer *fb,
>>>>> + struct drm_rect *rect)
>>>>> +{
>>>>> + unsigned int x, y;
>>>>> + u8 bw_byte = 0, red_byte = 0;
>>>>> + unsigned int bit_pos = 0;
>>>>> + unsigned int dst_idx = 0;
>>>>> +
>>>>> + drm_dbg(fb->dev,
>>>>> + "convert_3color: fmt=%p4cc rect=(%d,%d)-(%d,%d) path=%s\n",
>>>>> + &fb->format->format,
>>>>> + rect->x1, rect->y1, rect->x2, rect->y2,
>>>>> + fb->format->format == DRM_FORMAT_R1 ? "R1-direct" :
>>>>> "color- pixel");
>>>>> +
>>>>> + /*
>>>>> + * R1 is already monochrome — no colour channel exists.
>>>>> + * Copy BW data directly and leave the red plane all-white
>>>>> (transparent).
>>>>> + */
>>>>> + if (fb->format->format == DRM_FORMAT_R1) {
>>>>> + unsigned int src_pitch = fb->pitches[0];
>>>>> + unsigned int width_bytes = drm_rect_width(rect) / 8;
>>>>> + unsigned int data_size = width_bytes *
>>>>> drm_rect_height(rect);
>>>>> +
>>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>>> + u8 *line = src->vaddr + y * src_pitch + (rect->x1 / 8);
>>>>> +
>>>>> + memcpy(bw_dst + dst_idx, line, width_bytes);
>>>>> + dst_idx += width_bytes;
>>>>> + }
>>>>> + memset(red_dst, 0xFF, data_size); /* 0xFF = all white: no
>>>>> red pixels */
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + /* Use fixed threshold of 127 for grayscale to monochrome
>>>>> conversion. */
>>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>>> + for (x = rect->x1; x < rect->x2; x++) {
>>>>> + bool is_red = ssd16xx_pixel_is_red(src, fb, x, y);
>>>>> +
>>>>> + if (is_red)
>>>>> + red_byte |= (1 << (7 - bit_pos));
>>>>> + else if (ssd16xx_pixel_is_white(src, fb, x, y))
>>>>> + bw_byte |= (1 << (7 - bit_pos));
>>>>> + /* else: black pixel — both bits remain 0 */
>>>>> + if (++bit_pos == 8) {
>>>>> + bw_dst[dst_idx] = bw_byte;
>>>>> + red_dst[dst_idx] = red_byte;
>>>>> + dst_idx++;
>>>>> + bw_byte = 0;
>>>>> + red_byte = 0;
>>>>> + bit_pos = 0;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + /* Flush any partial byte at the end of each row */
>>>>> + if (bit_pos > 0) {
>>>>> + bw_dst[dst_idx] = bw_byte;
>>>>> + red_dst[dst_idx] = red_byte;
>>>>> + dst_idx++;
>>>>> + bw_byte = 0;
>>>>> + red_byte = 0;
>>>>> + bit_pos = 0;
>>>>> + }
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * ssd16xx_convert_r8_to_red_only() - map an R8 framebuffer to
>>>>> the RED RAM plane.
>>>>> + *
>>>>> + * Used when the panel has a physical red colour plane
>>>>> (red_supported == true)
>>>>> + * and the framebuffer format is DRM_FORMAT_R8. Pixels with
>>>>> value >= 128 are
>>>>> + * treated as red ink; the BW RAM is set to all-white so that
>>>>> only red ink
>>>>> + * appears on the white background.
>>>>> + *
>>>>> + * Hardware orientation is handled by the caller via RAM counter
>>>>> positioning;
>>>>> + * data is written in normal row-major order here (same as
>>>>> convert_fb_to_3color).
>>>>> + */
>>>>> +static void ssd16xx_convert_r8_to_red_only(u8 *bw_dst, u8 *red_dst,
>>>>> + struct iosys_map *src,
>>>>> + struct drm_framebuffer *fb,
>>>>> + struct drm_rect *rect)
>>>>> +{
>>>>> + unsigned int src_pitch = fb->pitches[0];
>>>>> + unsigned int width = drm_rect_width(rect);
>>>>> + unsigned int height = drm_rect_height(rect);
>>>>> + unsigned int data_size = DIV_ROUND_UP(width, 8) * height;
>>>>> + unsigned int dst_idx = 0;
>>>>> + unsigned int x, y;
>>>>> + u8 red_byte = 0;
>>>>> + unsigned int bit_pos = 0;
>>>>> +
>>>>> + /* BW RAM: all-white background - no black ink, only red ink
>>>>> shows */
>>>>> + memset(bw_dst, 0xFF, data_size);
>>>>> +
>>>>> + /* RED RAM: R8 >= 128 -> red ink (1-bit set) */
>>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>>> + u8 *line = src->vaddr + y * src_pitch;
>>>>> +
>>>>> + for (x = rect->x1; x < rect->x2; x++) {
>>>>> + if (line[x] >= 128)
>>>>> + red_byte |= (1 << (7 - bit_pos));
>>>>> + if (++bit_pos == 8) {
>>>>> + red_dst[dst_idx++] = red_byte;
>>>>> + red_byte = 0;
>>>>> + bit_pos = 0;
>>>>> + }
>>>>> + }
>>>>> + if (bit_pos > 0) {
>>>>> + red_dst[dst_idx++] = red_byte;
>>>>> + red_byte = 0;
>>>>> + bit_pos = 0;
>>>>> + }
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Convert framebuffer to 1-bit monochrome for e-paper display.
>>>>> + *
>>>>> + * Supported formats: XRGB8888, RGB888, RGB565, R8, NV12, NV16,
>>>>> YUYV, UYVY, R1.
>>>>> + * For colour and luma formats, Otsu's global binarisation method
>>>>> computes an
>>>>> + * optimal per-image threshold from the luminance histogram.
>>>>> + * R1 is the controller's native format and bypasses conversion
>>>>> entirely.
>>>>> + *
>>>>> + * Output layout:
>>>>> + * 0°/180° landscape: row-major, left-to-right, top-to-bottom
>>>>> + * 90°/270° CW portrait: column-major, rightmost column first
>>>>> + */
>>>>> +static void ssd16xx_convert_fb_to_1bpp(u8 *dst, struct iosys_map
>>>>> *src,
>>>>> + struct drm_framebuffer *fb,
>>>>> + struct drm_rect *rect,
>>>>> + unsigned int orientation)
>>>>> +{
>>>>> + u32 format = fb->format->format;
>>>>> + int x, y;
>>>>> + u8 byte = 0;
>>>>> + unsigned int bit_pos = 0;
>>>>> + unsigned int dst_idx = 0;
>>>>> +
>>>>> + /* Use fixed threshold of 127 for grayscale to monochrome
>>>>> conversion. */
>>>>> + drm_dbg(fb->dev,
>>>>> + "convert_1bpp: fmt=%p4cc rect=(%d,%d)-(%d,%d) orient=%u°
>>>>> path=%s\n",
>>>>> + &fb->format->format,
>>>>> + rect->x1, rect->y1, rect->x2, rect->y2,
>>>>> + orientation,
>>>>> + (format == DRM_FORMAT_R1 && orientation == 0 && rect->x1
>>>>> % 8 == 0) ? "R1-fast" :
>>>>> + (orientation == 90 || orientation == 270) ? "portrait" :
>>>>> "landscape");
>>>>> +
>>>>> + /*
>>>>> + * R1 fast path: 0° landscape with byte-aligned rect.
>>>>> + * R1 is already 1bpp so landscape rows map directly to
>>>>> output bytes via
>>>>> + * memcpy — no per-pixel computation needed. rect->x1 must be a
>>>>> + * multiple of 8 so that (rect->x1 / 8) gives the correct
>>>>> byte offset;
>>>>> + * if not, the generic pixel-by-pixel loop below handles non-
>>>>> aligned
>>>>> + * rects safely.
>>>>> + */
>>>>> + if (format == DRM_FORMAT_R1 && orientation == 0 && rect->x1 %
>>>>> 8 == 0) {
>>>>> + unsigned int src_pitch = fb->pitches[0];
>>>>> + unsigned int width_bytes = drm_rect_width(rect) / 8;
>>>>> +
>>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>>> + u8 *src_line = src->vaddr + y * src_pitch + (rect->x1
>>>>> / 8);
>>>>> +
>>>>> + memcpy(dst + dst_idx, src_line, width_bytes);
>>>>> + dst_idx += width_bytes;
>>>>> + }
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + switch (orientation) {
>>>>> + case 90:
>>>>> + case 270:
>>>>> + /*
>>>>> + * Portrait (90° or 270°): column-major packing.
>>>>> + * Each portrait source column becomes one physical RAM row.
>>>>> + * The data entry mode and cursor position control scan
>>>>> direction.
>>>>> + */
>>>>> + for (x = rect->x2 - 1; x >= (int)rect->x1; x--) {
>>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>>> + if (ssd16xx_pixel_is_white(src, fb, x, y))
>>>>> + byte |= (1 << (7 - bit_pos));
>>>>> + if (++bit_pos == 8) {
>>>>> + dst[dst_idx++] = byte;
>>>>> + byte = 0;
>>>>> + bit_pos = 0;
>>>>> + }
>>>>> + }
>>>>> + if (bit_pos > 0) {
>>>>> + dst[dst_idx++] = byte;
>>>>> + byte = 0;
>>>>> + bit_pos = 0;
>>>>> + }
>>>>> + }
>>>>> + break;
>>>>> +
>>>>> + case 0:
>>>>> + case 180:
>>>>> + default:
>>>>> + /*
>>>>> + * Landscape (0° or 180°): row-major packing.
>>>>> + * Each landscape source row becomes one physical RAM row.
>>>>> + * The data entry mode and cursor position control scan
>>>>> direction.
>>>>> + */
>>>>> + for (y = rect->y1; y < rect->y2; y++) {
>>>>> + for (x = rect->x1; x < rect->x2; x++) {
>>>>> + if (ssd16xx_pixel_is_white(src, fb, x, y))
>>>>> + byte |= (1 << (7 - bit_pos));
>>>>> + if (++bit_pos == 8) {
>>>>> + dst[dst_idx++] = byte;
>>>>> + byte = 0;
>>>>> + bit_pos = 0;
>>>>> + }
>>>>> + }
>>>>> + if (bit_pos > 0) {
>>>>> + dst[dst_idx++] = byte;
>>>>> + byte = 0;
>>>>> + bit_pos = 0;
>>>>> + }
>>>>> + }
>>>>> + break;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static int ssd16xx_fb_dirty(struct drm_framebuffer *fb, struct
>>>>> drm_rect *rect,
>>>>> + struct ssd16xx_panel *panel)
>>>>> +{
>>>>> + const u8 *ctrl2_tbl = panel->controller_cfg->ctrl2_refresh;
>>>>> + struct drm_gem_dma_object *dma_obj =
>>>>> drm_fb_dma_get_gem_obj(fb, 0);
>>>>> + struct iosys_map map;
>>>>> + int err = 0;
>>>>> + unsigned int data_size = (panel->width * panel->height) / 8;
>>>>> + u8 *mono_buffer = NULL;
>>>>> + u8 *red_buffer = NULL;
>>>>> + u16 ram_x_start, ram_x_end, ram_y_start, ram_y_end;
>>>>> +
>>>>> + /* Process full display area; convert handles orientation
>>>>> traversal. */
>>>>> + rect->x1 = 0;
>>>>> + rect->y1 = 0;
>>>>> + rect->x2 = panel->width;
>>>>> + rect->y2 = panel->height;
>>>>> +
>>>>> + drm_dbg(&panel->drm,
>>>>> + "fb_dirty: fb=%dx%d, refresh_mode=%d, orientation=%d\n",
>>>>> + fb->width, fb->height, panel->refresh_mode, panel-
>>>>> >orientation);
>>>>> +
>>>>> + mono_buffer = panel->tx_buf;
>>>>> + memset(mono_buffer, 0, data_size);
>>>>> +
>>>>> + /* 3-colour FULL/FAST: populate red channel. */
>>>>> + if (panel->color_mode == SSD16XX_COLOR_MODE_3COLOR &&
>>>>> + (panel->refresh_mode == SSD16XX_REFRESH_FULL ||
>>>>> + panel->refresh_mode == SSD16XX_REFRESH_FAST)) {
>>>>> + red_buffer = panel->tx_red_buf;
>>>>> + memset(red_buffer, 0, data_size);
>>>>> + }
>>>>> +
>>>>> + iosys_map_set_vaddr(&map, dma_obj->vaddr);
>>>>> +
>>>>> + if (red_buffer && fb->format->format == DRM_FORMAT_R8)
>>>>> + ssd16xx_convert_r8_to_red_only(mono_buffer, red_buffer,
>>>>> &map, fb, rect);
>>>>> + else if (red_buffer)
>>>>> + ssd16xx_convert_fb_to_3color(mono_buffer, red_buffer,
>>>>> &map, fb, rect);
>>>>> + else
>>>>> + ssd16xx_convert_fb_to_1bpp(mono_buffer, &map, fb, rect,
>>>>> panel->orientation);
>>>>> +
>>>>> + drm_dbg(&panel->drm,
>>>>> + "fb_dirty: mono[0..3]=0x%02x 0x%02x 0x%02x 0x%02x
>>>>> (data_size=%u)\n",
>>>>> + mono_buffer[0], mono_buffer[1], mono_buffer[2],
>>>>> mono_buffer[3],
>>>>> + data_size);
>>>>> +
>>>>> + /* Set RAM window and cursor for current orientation. */
>>>>> + ram_x_start = 0;
>>>>> + ram_x_end = (panel->controller_cfg->max_width / 8) - 1;
>>>>> + ram_y_start = 0;
>>>>> + ram_y_end = panel->controller_cfg->max_height - 1;
>>>>> +
>>>>> + switch (panel->orientation) {
>>>>> + case 90:
>>>>> + case 180:
>>>>> + /* 90°/180°: XDEC_YDEC mode, send end-before-start;
>>>>> cursor at (max, max). */
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
>>>>> + ssd16xx_send_x_param(panel, ram_x_end, &err);
>>>>> + ssd16xx_send_x_param(panel, ram_x_start, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END, &err);
>>>>> + ssd16xx_send_y_param(panel, ram_y_end, &err);
>>>>> + ssd16xx_send_y_param(panel, ram_y_start, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
>>>>> + ssd16xx_send_x_param(panel, ram_x_end, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
>>>>> + ssd16xx_send_y_param(panel, ram_y_end, &err);
>>>>> + break;
>>>>> +
>>>>> + default: /* 0°/270° */
>>>>> + /* 0°/270°: XINC_YINC mode, cursor at (0, 0). */
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_X_ADDRESS_START_END, &err);
>>>>> + ssd16xx_send_x_param(panel, ram_x_start, &err);
>>>>> + ssd16xx_send_x_param(panel, ram_x_end, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_Y_ADDRESS_START_END, &err);
>>>>> + ssd16xx_send_y_param(panel, ram_y_start, &err);
>>>>> + ssd16xx_send_y_param(panel, ram_y_end, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_X_ADDRESS_COUNTER, &err);
>>>>> + ssd16xx_send_x_param(panel, ram_x_start, &err);
>>>>> +
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_SET_RAM_Y_ADDRESS_COUNTER, &err);
>>>>> + ssd16xx_send_y_param(panel, ram_y_start, &err);
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD16XX_CMD_WRITE_RAM_BW, &err);
>>>>> + ssd16xx_send_data_bulk(panel, mono_buffer, data_size, &err);
>>>>> +
>>>>> + /* Re-send border waveform when: every-update mode, init
>>>>> frame, or
>>>>> + * the border_waveform_update property just changed (one-shot).
>>>>> + */
>>>>> + drm_dbg(&panel->drm,
>>>>> + "fb_dirty: border check: every_update=%d init_pending=%d
>>>>> border_pending=%d idx=%d hw=0x%02x\n",
>>>>> + panel->border_refresh_on_every_update, panel-
>>>>> >init_refresh_pending,
>>>>> + panel->border_waveform_pending, panel-
>>>>> >border_waveform_update_idx,
>>>>> + panel->controller_cfg->border_waveform_table[panel-
>>>>> >border_waveform_update_idx]);
>>>>> + if (panel->border_refresh_on_every_update || panel-
>>>>> >init_refresh_pending ||
>>>>> + panel->border_waveform_pending) {
>>>>> + u8 idx = panel->border_waveform_update_idx;
>>>>> + u8 border =
>>>>> panel->controller_cfg->border_waveform_table[idx];
>>>>> +
>>>>> + drm_dbg(&panel->drm, "fb_dirty: Sending border waveform:
>>>>> 0x%02x\n",
>>>>> + border);
>>>>> + ssd16xx_send_cmd(panel,
>>>>> SSD16XX_CMD_BORDER_WAVEFORM_CONTROL, &err);
>>>>> + ssd16xx_send_data(panel, border, &err);
>>>>> + panel->border_waveform_pending = false;
>>>>> + }
>>>>> +
>>>>> + switch (panel->refresh_mode) {
>>>>> + case SSD16XX_REFRESH_FULL:
>>>>> + /*
>>>>> + * BW full refresh: write RED RAM BEFORE display_update
>>>>> + * to avoid a post-BUSY write timing issue on some
>>>>> + * controller revisions that silently corrupts RED RAM.
>>>>> + * RED RAM is then bypassed (CTRL1_BYPASS_RED_RAM) so
>>>>> + * stale RED RAM content does not affect the output.
>>>>> + */
>>>>> + ssd16xx_send_cmd(panel, SSD1683_CMD_WRITE_RAM_RED, &err);
>>>>> + if (red_buffer) {
>>>>> + /* 3-colour: write red channel before activating */
>>>>> + ssd16xx_send_data_bulk(panel, red_buffer, data_size,
>>>>> &err);
>>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>>> >ctrl1_normal,
>>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>>> + ctrl2_tbl[SSD16XX_REFRESH_FULL], &err);
>>>>> + } else {
>>>>> + ssd16xx_send_data_bulk(panel, mono_buffer, data_size,
>>>>> &err);
>>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>>> >ctrl1_bypass_red_ram,
>>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>>> + ctrl2_tbl[SSD16XX_REFRESH_FULL], &err);
>>>>> + }
>>>>> + break;
>>>>> + case SSD16XX_REFRESH_FAST:
>>>>> + /*
>>>>> + * Fast refresh: LUT pre-loaded during hw_init;
>>>>> BYPASS_RED_RAM
>>>>> + * so RED RAM does not affect the current output.
>>>>> + * Write RED RAM BEFORE display_update (same reasoning as
>>>>> FULL)
>>>>> + * so it holds the just-displayed frame as a valid
>>>>> reference for
>>>>> + * any subsequent PARTIAL refresh.
>>>>> + */
>>>>> +
>>>>> + ssd16xx_send_cmd(panel, SSD1683_CMD_WRITE_RAM_RED, &err);
>>>>> + if (red_buffer) {
>>>>> + /* 3-colour: write red channel before activating */
>>>>> + ssd16xx_send_data_bulk(panel, red_buffer, data_size,
>>>>> &err);
>>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>>> >ctrl1_normal,
>>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>>> + ctrl2_tbl[SSD16XX_REFRESH_FAST], &err);
>>>>> + } else {
>>>>> + ssd16xx_send_data_bulk(panel, mono_buffer, data_size,
>>>>> &err);
>>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>>> >ctrl1_bypass_red_ram,
>>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>>> + ctrl2_tbl[SSD16XX_REFRESH_FAST], &err);
>>>>> + }
>>>>> + break;
>>>>> + case SSD16XX_REFRESH_PARTIAL:
>>>>> + default:
>>>>> + /*
>>>>> + * Partial refresh: both RAMs used for transition waveforms.
>>>>> + * RED RAM must hold the PREVIOUS frame (= current display
>>>>> + * content) so the controller can compute pixel transitions.
>>>>> + * Write RED RAM AFTER display_update so it captures the
>>>>> + * just-displayed frame as the reference for the next
>>>>> partial.
>>>>> + */
>>>>> + drm_dbg(&panel->drm,
>>>>> + "fb_dirty: partial pre-update: mono[0]=0x%02x
>>>>> (BW=new, RED=prev)\n",
>>>>> + mono_buffer[0]);
>>>>> + ssd16xx_display_update(panel, panel->controller_cfg-
>>>>> >ctrl1_normal,
>>>>> + SSD16XX_CTRL1_BYTE2_DEFAULT,
>>>>> + ctrl2_tbl[SSD16XX_REFRESH_PARTIAL], &err);
>>>>> + ssd16xx_send_cmd(panel, SSD1683_CMD_WRITE_RAM_RED, &err);
>>>>> + ssd16xx_send_data_bulk(panel, mono_buffer, data_size, &err);
>>>>> + drm_dbg(&panel->drm,
>>>>> + "fb_dirty: partial post-update: wrote RED baseline
>>>>> mono[0]=0x%02x\n",
>>>>> + mono_buffer[0]);
>>>>> + break;
>>>>> + }
>>>>> +
>>>>> + return err;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> -----------------------------------------------------------------------------
>>>>> + * Plane Functions
>>>>> + */
>>>>> +
>>>>> +static void ssd16xx_plane_destroy(struct drm_plane *plane)
>>>>> +{
>>>>> + drm_plane_cleanup(plane);
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_plane_reset(struct drm_plane *plane)
>>>>> +{
>>>>> + drm_atomic_helper_plane_reset(plane);
>>>>> +}
>>>>
>>>> Please avoid these wrappers.
>>>>
>>>
>>> Understood, will update in V2.
>>>
>>>>> +
>>>>> +static const struct drm_plane_funcs ssd16xx_plane_funcs = {
>>>>> + .update_plane = drm_atomic_helper_update_plane,
>>>>> + .disable_plane = drm_atomic_helper_disable_plane,
>>>>> + .destroy = ssd16xx_plane_destroy,
>>>>> + .reset = ssd16xx_plane_reset,
>>>>> + .atomic_duplicate_state =
>>>>> drm_atomic_helper_plane_duplicate_state,
>>>>> + .atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
>>>>> +};
>>>>> +
>>>>> +static int ssd16xx_plane_atomic_check(struct drm_plane *plane,
>>>>> + struct drm_atomic_state *state)
>>>>> +{
>>>>> + struct drm_plane_state *new_plane_state =
>>>>> + drm_atomic_get_new_plane_state(state, plane);
>>>>> + struct drm_crtc_state *crtc_state;
>>>>> +
>>>>> + if (!new_plane_state->crtc)
>>>>> + return 0;
>>>>> +
>>>>> + crtc_state = drm_atomic_get_new_crtc_state(state,
>>>>> new_plane_state->crtc);
>>>>> +
>>>>> + return drm_atomic_helper_check_plane_state(new_plane_state,
>>>>> crtc_state,
>>>>> + DRM_PLANE_NO_SCALING,
>>>>> + DRM_PLANE_NO_SCALING,
>>>>> + false, false);
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_plane_atomic_update(struct drm_plane *plane,
>>>>> + struct drm_atomic_state *state)
>>>>> +{
>>>>> + struct drm_plane_state *old_state =
>>>>> drm_atomic_get_old_plane_state(state, plane);
>>>>> + struct drm_plane_state *new_state =
>>>>> drm_atomic_get_new_plane_state(state, plane);
>>>>> + struct ssd16xx_panel *panel = plane_to_ssd16xx_panel(plane);
>>>>> + enum ssd16xx_refresh_mode saved_mode;
>>>>> + u8 saved_border_waveform_idx;
>>>>> + struct drm_framebuffer *fb = new_state->fb;
>>>>> + struct drm_rect rect;
>>>>> + int ret;
>>>>> +
>>>>> + drm_dbg(&panel->drm, "plane_atomic_update: fb=%p,
>>>>> initialized=%d\n",
>>>>> + fb, panel->initialized);
>>>>> +
>>>>> + if (!fb || !panel->initialized)
>>>>> + return;
>>>>> +
>>>>> + /*
>>>>> + * If a rotation change is pending, skip the update here —
>>>>> crtc_atomic_flush
>>>>> + * will re-init the hardware for the new orientation and redraw.
>>>>> + */
>>>>> + if (panel->reinit_pending) {
>>>>> + drm_dbg(&panel->drm, "plane_atomic_update: skipping
>>>>> (reinit pending)\n");
>>>>> + return;
>>>>> + }
>>>>> +
>>>>> + if (!drm_atomic_helper_damage_merged(old_state, new_state,
>>>>> &rect)) {
>>>>> + rect.x1 = 0;
>>>>> + rect.y1 = 0;
>>>>> + rect.x2 = fb->width;
>>>>> + rect.y2 = fb->height;
>>>>> + drm_dbg(&panel->drm, "plane_atomic_update: no damage,
>>>>> using full screen\n");
>>>>> + }
>>>>> +
>>>>> + drm_dbg(&panel->drm, "plane_atomic_update: calling fb_dirty
>>>>> rect=(%d,%d)-(%d,%d)\n",
>>>>> + rect.x1, rect.y1, rect.x2, rect.y2);
>>>>> + /*
>>>>> + * When refresh_mode_init was set, use the specified mode for
>>>>> this first
>>>>> + * frame only, then restore the user-configured refresh_mode so
>>>>> + * subsequent updates continue with the configured mode.
>>>>> + */
>>>>> + saved_mode = panel->refresh_mode;
>>>>> + saved_border_waveform_idx = panel->border_waveform_update_idx;
>>>>> + if (panel->init_refresh_pending) {
>>>>> + panel->refresh_mode = panel->refresh_mode_init;
>>>>> + panel->border_waveform_update_idx = panel-
>>>>> >border_waveform_init_idx;
>>>>> + }
>>>>> +
>>>>> + /*
>>>>> + * Fast refresh (0xC7) omits LOAD_LUT on every update cycle
>>>>> and relies
>>>>> + * on the LUT being pre-loaded upfront. The property setter
>>>>> arms
>>>>> + * fast_lut_pending whenever the user switches into fast
>>>>> mode. Consume
>>>>> + * the flag here (once) before the first fast-refresh frame
>>>>> so the
>>>>> + * controller's LUT is in the correct state.
>>>>> + */
>>>>> + if (panel->fast_lut_pending) {
>>>>> + ret = ssd16xx_preload_fast_lut(panel);
>>>>> + if (ret) {
>>>>> + drm_err(&panel->drm,
>>>>> + "plane_atomic_update: fast LUT preload failed:
>>>>> %d\n", ret);
>>>>> + }
>>>>> +
>>>>> + panel->fast_lut_pending = false;
>>>>> + }
>>>>> +
>>>>> + ret = ssd16xx_fb_dirty(fb, &rect, panel);
>>>>> + if (ret)
>>>>> + drm_err(&panel->drm, "plane_atomic_update: display update
>>>>> failed: %d\n", ret);
>>>>> + else
>>>>> + panel->last_fb = fb;
>>>>> +
>>>>> + panel->refresh_mode = saved_mode;
>>>>> + panel->border_waveform_update_idx = saved_border_waveform_idx;
>>>>> +
>>>>> + /*
>>>>> + * If this was the init frame (which used
>>>>> border_waveform_init_idx
>>>>> + * inside fb_dirty), arm border_waveform_pending so the normal
>>>>> + * (non-init) border value is sent at the start of the next
>>>>> update.
>>>>> + */
>>>>> + if (panel->init_refresh_pending) {
>>>>> + panel->init_refresh_pending = false;
>>>>> + panel->border_waveform_pending = true;
>>>>> + }
>>>>> +}
>>>>> +
>>>>> +static const struct drm_plane_helper_funcs
>>>>> ssd16xx_plane_helper_funcs = {
>>>>> + .atomic_check = ssd16xx_plane_atomic_check,
>>>>> + .atomic_update = ssd16xx_plane_atomic_update,
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> -----------------------------------------------------------------------------
>>>>> + * CRTC Functions
>>>>> + */
>>>>> +
>>>>> +static void ssd16xx_crtc_destroy(struct drm_crtc *crtc)
>>>>> +{
>>>>> + drm_crtc_cleanup(crtc);
>>>>> +}
>>>>> +
>>>>> +static const struct drm_crtc_funcs ssd16xx_crtc_funcs = {
>>>>> + .reset = drm_atomic_helper_crtc_reset,
>>>>> + .destroy = ssd16xx_crtc_destroy,
>>>>> + .set_config = drm_atomic_helper_set_config,
>>>>> + .page_flip = drm_atomic_helper_page_flip,
>>>>> + .atomic_duplicate_state =
>>>>> drm_atomic_helper_crtc_duplicate_state,
>>>>> + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
>>>>> +};
>>>>> +
>>>>> +static enum drm_mode_status ssd16xx_crtc_mode_valid(struct
>>>>> drm_crtc *crtc,
>>>>> + const struct drm_display_mode *mode)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = crtc_to_ssd16xx_panel(crtc);
>>>>> +
>>>>> + /* Accept only our panel's native mode (landscape or
>>>>> portrait) */
>>>>> + if ((mode->hdisplay == panel->mode->hdisplay &&
>>>>> + mode->vdisplay == panel->mode->vdisplay) ||
>>>>> + (mode->hdisplay == panel->mode->vdisplay &&
>>>>> + mode->vdisplay == panel->mode->hdisplay))
>>>>> + return MODE_OK;
>>>>> +
>>>>> + return MODE_BAD;
>>>>> +}
>>>>> +
>>>>> +static int ssd16xx_crtc_atomic_check(struct drm_crtc *crtc,
>>>>> + struct drm_atomic_state *state)
>>>>> +{
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_crtc_atomic_disable(struct drm_crtc *crtc,
>>>>> + struct drm_atomic_state *state)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = crtc_to_ssd16xx_panel(crtc);
>>>>> + int ret, idx;
>>>>> +
>>>>> + if (!drm_dev_enter(&panel->drm, &idx))
>>>>> + return;
>>>>> +
>>>>> + if (panel->clear_on_disable < 0 || panel-
>>>>> >display_cleared_on_deinit)
>>>>> + goto out;
>>>>> +
>>>>> + drm_dbg(&panel->drm, "clear_on_disable: running, mode=%d\n",
>>>>> + panel->clear_on_disable);
>>>>> + ret = ssd16xx_clear_display(panel,
>>>>> + ssd16xx_refresh_mode_to_ctrl2(panel,
>>>>> + panel->clear_on_disable));
>>>>> + if (ret) {
>>>>> + drm_err(&panel->drm, "atomic_disable: clear failed:
>>>>> %d\n", ret);
>>>>> + goto out;
>>>>> + }
>>>>> +
>>>>> + panel->display_cleared_on_deinit = true;
>>>>> +out:
>>>>> + drm_dev_exit(idx);
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_crtc_atomic_enable(struct drm_crtc *crtc,
>>>>> + struct drm_atomic_state *state)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = crtc_to_ssd16xx_panel(crtc);
>>>>> + struct drm_crtc_state *crtc_state =
>>>>> drm_atomic_get_new_crtc_state(state, crtc);
>>>>> + int ret, idx;
>>>>> +
>>>>> + if (!drm_dev_enter(&panel->drm, &idx))
>>>>> + return;
>>>>> +
>>>>> + panel->display_cleared_on_deinit = false;
>>>>> +
>>>>> + drm_dbg(&panel->drm, "atomic_enable: %dx%d\n",
>>>>> + crtc_state->mode.hdisplay, crtc_state->mode.vdisplay);
>>>>> +
>>>>> + panel->width = crtc_state->mode.hdisplay;
>>>>> + panel->height = crtc_state->mode.vdisplay;
>>>>> +
>>>>> + ret = ssd16xx_hw_init(panel);
>>>>> + if (ret) {
>>>>> + drm_err(&panel->drm, "crtc_atomic_enable: HW init failed:
>>>>> %d\n", ret);
>>>>> + goto out;
>>>>> + }
>>>>> + panel->initialized = true;
>>>>> +
>>>>> + /* Clear display on first app launch if configured */
>>>>> + ret = ssd16xx_clear_display_on_init(panel);
>>>>> + if (ret)
>>>>> + drm_err(&panel->drm, "crtc_atomic_enable: clear on init
>>>>> failed: %d\n", ret);
>>>>> +
>>>>> + /*
>>>>> + * If refresh_mode_init is set, arm init_refresh_pending so
>>>>> + * plane_atomic_update uses the specified mode for the first
>>>>> frame
>>>>> + * then restores the user-configured or panel default
>>>>> refresh_mode.
>>>>> + */
>>>>> + if (panel->refresh_mode_init >= 0) {
>>>>> + drm_dbg(&panel->drm,
>>>>> + "atomic_enable: refresh_mode_init=%d, using for first
>>>>> frame\n",
>>>>> + panel->refresh_mode_init);
>>>>> + panel->init_refresh_pending = true;
>>>>> + }
>>>>> +
>>>>> +out:
>>>>> + drm_dev_exit(idx);
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * Re-initialize hardware and redraw the current framebuffer when
>>>>> the
>>>>> + * display orientation changes at runtime via the rotation
>>>>> connector property.
>>>>> + * Called by the DRM atomic helper after atomic_enable/disable
>>>>> have run.
>>>>> + */
>>>>> +static void ssd16xx_crtc_atomic_flush(struct drm_crtc *crtc,
>>>>> + struct drm_atomic_state *state)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = crtc_to_ssd16xx_panel(crtc);
>>>>> + struct drm_framebuffer *fb;
>>>>> + struct drm_rect full;
>>>>> + int ret, idx;
>>>>> +
>>>>> + if (!panel->reinit_pending || !panel->initialized)
>>>>> + return;
>>>>> +
>>>>> + if (!drm_dev_enter(&panel->drm, &idx))
>>>>> + return;
>>>>> +
>>>>> + panel->reinit_pending = false;
>>>>> +
>>>>> + drm_dbg(&panel->drm, "atomic_flush: reinit, orientation=%u°\n",
>>>>> + panel->orientation);
>>>>> +
>>>>> + ret = ssd16xx_hw_init(panel);
>>>>> + if (ret) {
>>>>> + drm_err(&panel->drm, "Orientation re-init failed: %d\n",
>>>>> ret);
>>>>> + goto out;
>>>>> + }
>>>>> +
>>>>> + fb = panel->primary_plane.state ? panel->primary_plane.state->fb
>>>>> + : panel->last_fb;
>>>>> + if (fb) {
>>>>> + full.x1 = 0;
>>>>> + full.y1 = 0;
>>>>> + full.x2 = fb->width;
>>>>> + full.y2 = fb->height;
>>>>> + ret = ssd16xx_fb_dirty(fb, &full, panel);
>>>>> + if (ret)
>>>>> + drm_err(&panel->drm, "atomic_flush: display update
>>>>> failed: %d\n", ret);
>>>>> + else
>>>>> + panel->last_fb = fb;
>>>>> + }
>>>>> +
>>>>> +out:
>>>>> + drm_dev_exit(idx);
>>>>> +}
>>>>> +
>>>>> +static const struct drm_crtc_helper_funcs
>>>>> ssd16xx_crtc_helper_funcs = {
>>>>> + .mode_valid = ssd16xx_crtc_mode_valid,
>>>>> + .atomic_check = ssd16xx_crtc_atomic_check,
>>>>> + .atomic_disable = ssd16xx_crtc_atomic_disable,
>>>>> + .atomic_enable = ssd16xx_crtc_atomic_enable,
>>>>> + .atomic_flush = ssd16xx_crtc_atomic_flush,
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> -----------------------------------------------------------------------------
>>>>> + * Connector Functions
>>>>> + */
>>>>> +
>>>>> +static int ssd16xx_connector_get_modes(struct drm_connector
>>>>> *connector)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(connector->dev);
>>>>> + bool mode_is_portrait = (panel->mode->hdisplay < panel->mode-
>>>>> >vdisplay);
>>>>> + bool orient_is_portrait = (panel->orientation == 90 || panel-
>>>>> >orientation == 270);
>>>>> +
>>>>> + drm_dbg(&panel->drm, "connector_get_modes: orientation=%u°\n",
>>>>> + panel->orientation);
>>>>> +
>>>>> + /* For portrait, swap dimensions so clients see logical size. */
>>>>> + if (mode_is_portrait != orient_is_portrait) {
>>>>> + struct drm_display_mode *mode;
>>>>> +
>>>>> + mode = drm_mode_duplicate(&panel->drm, panel->mode);
>>>>> + if (!mode)
>>>>> + return 0;
>>>>> + swap(mode->hdisplay, mode->vdisplay);
>>>>> + swap(mode->hsync_start, mode->vsync_start);
>>>>> + swap(mode->hsync_end, mode->vsync_end);
>>>>> + swap(mode->htotal, mode->vtotal);
>>>>> + swap(mode->width_mm, mode->height_mm);
>>>>> + mode->type |= DRM_MODE_TYPE_PREFERRED;
>>>>> + drm_mode_set_name(mode);
>>>>> + drm_mode_probed_add(connector, mode);
>>>>> + return 1;
>>>>> + }
>>>>> +
>>>>> + return drm_connector_helper_get_modes_fixed(connector, panel-
>>>>> >mode);
>>>>> +}
>>>>> +
>>>>> +static const struct drm_connector_helper_funcs
>>>>> ssd16xx_connector_helper_funcs = {
>>>>> + .get_modes = ssd16xx_connector_get_modes,
>>>>> +};
>>>>> +
>>>>> +/* Enum values for the rotation connector property (degrees
>>>>> clockwise) */
>>>>> +static const struct drm_prop_enum_list ssd16xx_rotation_enum[] = {
>>>>> + { 0, "0" },
>>>>> + { 90, "90" },
>>>>> + { 180, "180" },
>>>>> + { 270, "270" },
>>>>> +};
>>>>> +
>>>>> +/* Enum values for the refresh_mode connector property */
>>>>> +static const struct drm_prop_enum_list
>>>>> ssd16xx_refresh_mode_enum[] = {
>>>>> + { SSD16XX_REFRESH_PARTIAL, "partial" },
>>>>> + { SSD16XX_REFRESH_FULL, "full" },
>>>>> + { SSD16XX_REFRESH_FAST, "fast" },
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> + * Enum for clear_on_init, clear_on_close, refresh_mode_init
>>>>> properties.
>>>>> + * Value 0 = disabled; values 1-3 = partial/full/fast (refresh
>>>>> mode + 1).
>>>>> + * The +1 offset allows a single enum to represent both
>>>>> "disabled" and the
>>>>> + * three refresh modes without sign-extending the DRM property
>>>>> value.
>>>>> + */
>>>>> +static const struct drm_prop_enum_list
>>>>> ssd16xx_init_refresh_enum[] = {
>>>>> + { 0, "disabled" },
>>>>> + { 1, "partial" },
>>>>> + { 2, "full" },
>>>>> + { 3, "fast" },
>>>>> +};
>>>>> +
>>>>> +/* Enum values for the color_mode connector property */
>>>>> +static const struct drm_prop_enum_list ssd16xx_color_mode_enum[] = {
>>>>> + { SSD16XX_COLOR_MODE_BW, "black-white" },
>>>>> + { SSD16XX_COLOR_MODE_3COLOR, "3-color" },
>>>>> +};
>>>>> +
>>>>> +/* Enum values for border_waveform connector properties (one per
>>>>> HW mode) */
>>>>> +static const struct drm_prop_enum_list
>>>>> ssd16xx_border_waveform_enum[] = {
>>>>> + { SSD16XX_BORDER_LUT0, "lut0_black" },
>>>>> + { SSD16XX_BORDER_LUT1, "lut1_white" },
>>>>> + { SSD16XX_BORDER_LUT2, "lut2_black" },
>>>>> + { SSD16XX_BORDER_LUT3, "lut3_gray" },
>>>>> + { SSD16XX_BORDER_VSS, "vss_black" },
>>>>> + { SSD16XX_BORDER_VSH1, "vsh1_black" },
>>>>> + { SSD16XX_BORDER_VSL, "vsl_white" },
>>>>> + { SSD16XX_BORDER_VSH2, "vsh2_black" },
>>>>> + { SSD16XX_BORDER_VCOM, "vcom_preserve" },
>>>>> + { SSD16XX_BORDER_HIZ, "hiz_float" },
>>>>> +};
>>>>> +
>>>>> +static int ssd16xx_connector_create_properties(struct
>>>>> ssd16xx_panel *panel)
>>>>> +{
>>>>> + struct drm_device *drm = &panel->drm;
>>>>> + struct drm_connector *connector = &panel->connector;
>>>>> +
>>>>> + panel->rotation_property =
>>>>> + drm_property_create_enum(drm, 0, "rotation",
>>>>> + ssd16xx_rotation_enum,
>>>>> + ARRAY_SIZE(ssd16xx_rotation_enum));
>>>>> + if (!panel->rotation_property)
>>>>> + return -ENOMEM;
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->rotation_property, panel->orientation);
>>>>> +
>>>>> + panel->refresh_mode_property =
>>>>> + drm_property_create_enum(drm, 0, "refresh_mode",
>>>>> + ssd16xx_refresh_mode_enum,
>>>>> + ARRAY_SIZE(ssd16xx_refresh_mode_enum));
>>>>> + if (!panel->refresh_mode_property)
>>>>> + return -ENOMEM;
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->refresh_mode_property,
>>>>> panel->refresh_mode);
>>>>> +
>>>>> + panel->border_waveform_init_property =
>>>>> + drm_property_create_enum(drm, 0, "border_waveform_init",
>>>>> + ssd16xx_border_waveform_enum,
>>>>> + ARRAY_SIZE(ssd16xx_border_waveform_enum));
>>>>> + if (!panel->border_waveform_init_property)
>>>>> + return -ENOMEM;
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->border_waveform_init_property,
>>>>> + panel->border_waveform_init_idx);
>>>>> +
>>>>> + panel->border_waveform_update_property =
>>>>> + drm_property_create_enum(drm, 0, "border_waveform_update",
>>>>> + ssd16xx_border_waveform_enum,
>>>>> + ARRAY_SIZE(ssd16xx_border_waveform_enum));
>>>>> + if (!panel->border_waveform_update_property)
>>>>> + return -ENOMEM;
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->border_waveform_update_property,
>>>>> + panel->border_waveform_update_idx);
>>>>> +
>>>>> + panel->border_refresh_on_every_update_property =
>>>>> + drm_property_create_bool(drm, 0,
>>>>> "border_refresh_on_every_update");
>>>>> + if (!panel->border_refresh_on_every_update_property)
>>>>> + return -ENOMEM;
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->border_refresh_on_every_update_property,
>>>>> + panel->border_refresh_on_every_update);
>>>>> +
>>>>> + panel->clear_on_init_property =
>>>>> + drm_property_create_enum(drm, 0, "clear_on_init",
>>>>> + ssd16xx_init_refresh_enum,
>>>>> + ARRAY_SIZE(ssd16xx_init_refresh_enum));
>>>>> + if (!panel->clear_on_init_property)
>>>>> + return -ENOMEM;
>>>>> + /* Property value 0=disabled, 1-3=mode; field is -1/0/1/2 →
>>>>> val = field+1 */
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->clear_on_init_property,
>>>>> + panel->clear_on_init + 1);
>>>>> +
>>>>> + panel->clear_on_close_property =
>>>>> + drm_property_create_enum(drm, 0, "clear_on_close",
>>>>> + ssd16xx_init_refresh_enum,
>>>>> + ARRAY_SIZE(ssd16xx_init_refresh_enum));
>>>>> + if (!panel->clear_on_close_property)
>>>>> + return -ENOMEM;
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->clear_on_close_property,
>>>>> + panel->clear_on_close + 1);
>>>>> +
>>>>> + panel->clear_on_disable_property =
>>>>> + drm_property_create_enum(drm, 0, "clear_on_disable",
>>>>> + ssd16xx_init_refresh_enum,
>>>>> + ARRAY_SIZE(ssd16xx_init_refresh_enum));
>>>>> + if (!panel->clear_on_disable_property)
>>>>> + return -ENOMEM;
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->clear_on_disable_property,
>>>>> + panel->clear_on_disable + 1);
>>>>> +
>>>>> + panel->refresh_mode_init_property =
>>>>> + drm_property_create_enum(drm, 0, "refresh_mode_init",
>>>>> + ssd16xx_init_refresh_enum,
>>>>> + ARRAY_SIZE(ssd16xx_init_refresh_enum));
>>>>> + if (!panel->refresh_mode_init_property)
>>>>> + return -ENOMEM;
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->refresh_mode_init_property,
>>>>> + panel->refresh_mode_init + 1);
>>>>> +
>>>>> + /*
>>>>> + * color_mode: only expose 3-color option on panels that
>>>>> physically have
>>>>> + * a red plane; on BW-only panels the property still exists for
>>>>> + * consistency but userspace can only set "black-white".
>>>>> + */
>>>>> + panel->color_mode_property =
>>>>> + drm_property_create_enum(drm, 0, "color_mode",
>>>>> + ssd16xx_color_mode_enum,
>>>>> + panel->panel_cfg->red_supported
>>>>> + ? ARRAY_SIZE(ssd16xx_color_mode_enum)
>>>>> + : 1);
>>>>> + if (!panel->color_mode_property)
>>>>> + return -ENOMEM;
>>>>> + drm_object_attach_property(&connector->base,
>>>>> + panel->color_mode_property,
>>>>> + panel->color_mode);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static int ssd16xx_connector_atomic_get_property(struct
>>>>> drm_connector *connector,
>>>>> + const struct drm_connector_state *state,
>>>>> + struct drm_property *property,
>>>>> + uint64_t *val)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(connector->dev);
>>>>> +
>>>>> + drm_dbg(&panel->drm, "get_property: %s\n", property->name);
>>>>> +
>>>>> + if (property == panel->rotation_property) {
>>>>> + *val = panel->orientation;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->refresh_mode_property) {
>>>>> + *val = panel->refresh_mode;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->border_waveform_init_property) {
>>>>> + *val = panel->border_waveform_init_idx;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->border_waveform_update_property) {
>>>>> + *val = panel->border_waveform_update_idx;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property ==
>>>>> panel->border_refresh_on_every_update_property) {
>>>>> + *val = panel->border_refresh_on_every_update;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->clear_on_init_property) {
>>>>> + *val = panel->clear_on_init + 1; /* field -1/0/1/2 → val
>>>>> 0/1/2/3 */
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->clear_on_close_property) {
>>>>> + *val = panel->clear_on_close + 1;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->clear_on_disable_property) {
>>>>> + *val = panel->clear_on_disable + 1;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->refresh_mode_init_property) {
>>>>> + *val = panel->refresh_mode_init + 1;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->color_mode_property) {
>>>>> + *val = panel->color_mode;
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>> + return -EINVAL;
>>>>> +}
>>>>> +
>>>>> +static int ssd16xx_connector_atomic_set_property(struct
>>>>> drm_connector *connector,
>>>>> + struct drm_connector_state *state,
>>>>> + struct drm_property *property,
>>>>> + uint64_t val)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(connector->dev);
>>>>> +
>>>>> + drm_dbg(&panel->drm, "set_property: %s = %llu\n", property-
>>>>> >name, val);
>>>>> +
>>>>> + if (property == panel->rotation_property) {
>>>>> + if (val != 0 && val != 90 && val != 180 && val != 270)
>>>>> + return -EINVAL;
>>>>> + panel->orientation = val;
>>>>> + /*
>>>>> + * Flag hardware re-init needed. crtc_atomic_flush will call
>>>>> + * ssd16xx_hw_init() with the new orientation and redraw.
>>>>> + */
>>>>> + panel->reinit_pending = true;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->refresh_mode_property) {
>>>>> + if (val > SSD16XX_REFRESH_FAST)
>>>>> + return -EINVAL;
>>>>> + /*
>>>>> + * Fast refresh (0xC7) omits LOAD_LUT on every update and
>>>>> relies
>>>>> + * on the LUT being pre-loaded upfront. Arm the one-shot
>>>>> flag
>>>>> + * when switching into fast mode so the next
>>>>> plane_atomic_update
>>>>> + * loads the LUT before the first fast-refresh cycle.
>>>>> Clear it
>>>>> + * when switching away so a fresh pre-load happens if the
>>>>> user
>>>>> + * returns to fast mode later.
>>>>> + */
>>>>> + if (val == SSD16XX_REFRESH_FAST &&
>>>>> + panel->refresh_mode != SSD16XX_REFRESH_FULL)
>>>>> + panel->fast_lut_pending = true;
>>>>> + else
>>>>> + panel->fast_lut_pending = false;
>>>>> + panel->refresh_mode = val;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->border_waveform_init_property) {
>>>>> + if (val >= ARRAY_SIZE(ssd1683_border_waveform_table))
>>>>> + return -EINVAL;
>>>>> + panel->border_waveform_init_idx = val;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->border_waveform_update_property) {
>>>>> + const u8 *bw_tbl = panel->controller_cfg-
>>>>> >border_waveform_table;
>>>>> + bool changed = (int)val !=
>>>>> panel->border_waveform_update_idx;
>>>>> +
>>>>> + if (val >= ARRAY_SIZE(ssd1683_border_waveform_table))
>>>>> + return -EINVAL;
>>>>> + drm_dbg(&panel->drm,
>>>>> + "set_property: border_waveform_update old=%d new=%llu
>>>>> hw=0x%02x -> 0x%02x %s\n",
>>>>> + panel->border_waveform_update_idx, val,
>>>>> + bw_tbl[panel->border_waveform_update_idx],
>>>>> + bw_tbl[val],
>>>>> + changed ? "(arming pending)" : "(no change)");
>>>>> + /* Arm one-shot flag so the new border value is sent on
>>>>> the very
>>>>> + * next display update, even if
>>>>> border_refresh_on_every_update is
>>>>> + * not set. Cleared in fb_dirty after the command is sent.
>>>>> + */
>>>>> + if ((int)val != panel->border_waveform_update_idx)
>>>>> + panel->border_waveform_pending = true;
>>>>> + panel->border_waveform_update_idx = val;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property ==
>>>>> panel->border_refresh_on_every_update_property) {
>>>>> + panel->border_refresh_on_every_update = !!val;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->clear_on_init_property) {
>>>>> + if (val > 3)
>>>>> + return -EINVAL;
>>>>> + panel->clear_on_init = (int)val - 1; /* val 0/1/2/3 →
>>>>> field -1/0/1/2 */
>>>>> + panel->first_clear_done = false; /* allow re-fire on
>>>>> next enable */
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->clear_on_close_property) {
>>>>> + if (val > 3)
>>>>> + return -EINVAL;
>>>>> + panel->clear_on_close = (int)val - 1;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->clear_on_disable_property) {
>>>>> + if (val > 3)
>>>>> + return -EINVAL;
>>>>> + panel->clear_on_disable = (int)val - 1;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->refresh_mode_init_property) {
>>>>> + if (val > 3)
>>>>> + return -EINVAL;
>>>>> + panel->refresh_mode_init = (int)val - 1;
>>>>> + return 0;
>>>>> + }
>>>>> + if (property == panel->color_mode_property) {
>>>>> + if (val > SSD16XX_COLOR_MODE_3COLOR)
>>>>> + return -EINVAL;
>>>>> + if (val == SSD16XX_COLOR_MODE_3COLOR &&
>>>>> !panel->panel_cfg- >red_supported) {
>>>>> + drm_dbg(&panel->drm,
>>>>> + "set_property: 3-color mode not supported by this
>>>>> panel\n");
>>>>> + return -EINVAL;
>>>>> + }
>>>>> + panel->color_mode = val;
>>>>> + return 0;
>>>>> + }
>>>>> +
>>>>> + return -EINVAL;
>>>>> +}
>>>>> +
>>>>> +static const struct drm_connector_funcs ssd16xx_connector_funcs = {
>>>>> + .reset = drm_atomic_helper_connector_reset,
>>>>> + .fill_modes = drm_helper_probe_single_connector_modes,
>>>>> + .destroy = drm_connector_cleanup,
>>>>> + .atomic_duplicate_state =
>>>>> drm_atomic_helper_connector_duplicate_state,
>>>>> + .atomic_destroy_state =
>>>>> drm_atomic_helper_connector_destroy_state,
>>>>> + .atomic_get_property = ssd16xx_connector_atomic_get_property,
>>>>> + .atomic_set_property = ssd16xx_connector_atomic_set_property,
>>>>> +};
>>>>> +
>>>>> +static const u32 ssd16xx_formats[] = {
>>>>> + DRM_FORMAT_XRGB8888, /* 32-bit RGB with padding (preferred) */
>>>>> + DRM_FORMAT_RGB888, /* 24-bit packed RGB */
>>>>> + DRM_FORMAT_RGB565, /* 16-bit RGB (5:6:5) */
>>>>> + DRM_FORMAT_R8, /* 8-bit grayscale */
>>>>> + DRM_FORMAT_NV12, /* YUV 4:2:0 planar */
>>>>> + DRM_FORMAT_NV16, /* YUV 4:2:2 planar */
>>>>> + DRM_FORMAT_YUYV, /* Packed YUV 4:2:2 (Y0 U0 Y1 V0) */
>>>>> + DRM_FORMAT_UYVY, /* Packed YUV 4:2:2 (U0 Y0 V0 Y1) */
>>>>> + DRM_FORMAT_R1, /* 1-bit monochrome (native, 8 pixels/
>>>>> byte) */
>>>>> +};
>>>>
>>>> Why do you have all these formats?
>>>>
>>>> Only export the modes your panel can do natively; plus maybe
>>>> XRGB8888 for compatibility.
>>>>
>>>
>>> I wanted to keep YUV formats too since some apps such as camera apps
>>> (in case we want to click a picture and display over on the e-paper
>>> badge directly) support only YUV formats but yeah if it's too much I
>>> can remove them from driver and instead have the conversion in the
>>> app itself.
>>>
>>>>> +
>>>>> +DEFINE_DRM_GEM_FOPS(ssd16xx_fops);
>>>>> +
>>>>> +/*
>>>>> + * ssd16xx_drm_master_set - arm init refresh when a new master
>>>>> takes control.
>>>>> + */
>>>>> +static void ssd16xx_drm_master_set(struct drm_device *drm,
>>>>> + struct drm_file *file, bool from_open)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(drm);
>>>>> +
>>>>> + panel->display_cleared_on_deinit = false;
>>>>> + panel->first_clear_done = false;
>>>>> +
>>>>> + if (panel->refresh_mode_init >= 0)
>>>>> + panel->init_refresh_pending = true;
>>>>> +}
>>>>> +
>>>>> +/*
>>>>> + * ssd16xx_drm_master_drop - clear display and disarm init
>>>>> refresh when the
>>>>> + * master client exits.
>>>>> + */
>>>>> +static void ssd16xx_drm_master_drop(struct drm_device *drm,
>>>>> + struct drm_file *file)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = to_ssd16xx_panel(drm);
>>>>> + int ret;
>>>>> +
>>>>> + panel->init_refresh_pending = false;
>>>>> + panel->first_clear_done = false;
>>>>> +
>>>>> + if (panel->clear_on_close < 0 ||
>>>>> panel->display_cleared_on_deinit)
>>>>> + return;
>>>>> +
>>>>> + ret = ssd16xx_clear_display_on_exit(panel);
>>>>> + if (ret)
>>>>> + drm_err(drm, "master_drop: clear on close failed: %d\n",
>>>>> ret);
>>>>> +
>>>>> + panel->display_cleared_on_deinit = true;
>>>>> +}
>>>>
>>>> No, don't overload these. Just remove all this. Clearing should be
>>>> left to the DRM client.
>>>>
>>>
>>> Yes, the choice to clear or not to clear is left to drm client
>>> depending on drm property setting done by drm client, the driver
>>> clears the display. It would be difficult to update all different
>>> apps to pass a blank white buffer to clear the screen and what if
>>> the app gets closed abruptly (as master drop callback will get
>>> triggered), then in that case the current driver logic ensures that
>>> screen gets cleared. In normal LCD displays if app gets closed
>>> abruptly, the display would have gone-off automatically as signals
>>> would stop getting transmitted but in e-paper panel the last display
>>> context would remain and I think it is driver responsibility to
>>> clear that if that was the policy communicated by application to the
>>> driver.
>>>
>>>>> +
>>>>> +static struct drm_driver ssd16xx_drm_driver = {
>>>>> + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
>>>>> + .fops = &ssd16xx_fops,
>>>>> + .name = "ssd16xx",
>>>>> + .desc = "DRM driver for SSD16xx e-paper controller family",
>>>>> + .major = 1,
>>>>> + .minor = 0,
>>>>> + .master_set = ssd16xx_drm_master_set,
>>>>> + .master_drop = ssd16xx_drm_master_drop,
>>>>> + DRM_GEM_DMA_DRIVER_OPS,
>>>>> + DRM_FBDEV_DMA_DRIVER_OPS,
>>>>> +};
>>>>> +
>>>>> +static const struct drm_mode_config_funcs
>>>>> ssd16xx_mode_config_funcs = {
>>>>> + .fb_create = drm_gem_fb_create_with_dirty,
>>>>> + .atomic_check = drm_atomic_helper_check,
>>>>> + .atomic_commit = drm_atomic_helper_commit,
>>>>> +};
>>>>> +
>>>>> +/*
>>>>> + * Use the RPM commit-tail variant so that
>>>>> drm_atomic_helper_commit_modeset_enables
>>>>> + * (which calls crtc_atomic_enable) runs before
>>>>> drm_atomic_helper_commit_planes.
>>>>> + * Without this, the standard commit_tail calls commit_planes before
>>>>> + * modeset_enables, so plane_atomic_update would see initialized
>>>>> == false on the
>>>>> + * first commit and silently drop the frame.
>>>>> + */
>>>>> +static const struct drm_mode_config_helper_funcs
>>>>> ssd16xx_mode_config_helper_funcs = {
>>>>> + .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
>>>>> +};
>>>>> +
>>>>> +static int ssd16xx_alloc_tx_bufs(struct ssd16xx_panel *panel)
>>>>> +{
>>>>> + struct device *dev = &panel->spi->dev;
>>>>> + size_t frame_size = (panel->controller_cfg->max_width *
>>>>> + panel->controller_cfg->max_height) / 8;
>>>>> +
>>>>> + panel->tx_buf = devm_kmalloc(dev, frame_size, GFP_KERNEL);
>>>>
>>>> drmm_kmalloc() here and for the other buffers.
>>>>
>>>
>>> Understood, thanks for pointing will fix it in V2.
>>>
>>> Best Regards
>>> Devarsh
>>>
>>>> Best regards
>>>> Thomas
>>>
>>>>
>>>>> + if (!panel->tx_buf)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + if (panel->panel_cfg->red_supported) {
>>>>> + panel->tx_red_buf = devm_kmalloc(dev, frame_size,
>>>>> GFP_KERNEL);
>>>>> + if (!panel->tx_red_buf)
>>>>> + return -ENOMEM;
>>>>> + }
>>>>> +
>>>>> + if (!panel->dc) {
>>>>> + panel->tx_buf9 = devm_kmalloc_array(dev, frame_size,
>>>>> + sizeof(u16), GFP_KERNEL);
>>>>> + if (!panel->tx_buf9)
>>>>> + return -ENOMEM;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static int ssd16xx_probe(struct spi_device *spi)
>>>>> +{
>>>>> + struct device *dev = &spi->dev;
>>>>> + struct ssd16xx_panel *panel;
>>>>> + struct drm_device *drm;
>>>>> + const struct spi_device_id *spi_id;
>>>>> + struct drm_display_mode *mode;
>>>>> + const void *match;
>>>>> + enum ssd16xx_model model;
>>>>> + u32 dt_rotation = 0;
>>>>> + int ret;
>>>>> +
>>>>> + match = device_get_match_data(dev);
>>>>> + if (match) {
>>>>> + model = (enum ssd16xx_model)(uintptr_t)match;
>>>>> + } else {
>>>>> + spi_id = spi_get_device_id(spi);
>>>>> + model = (enum ssd16xx_model)spi_id->driver_data;
>>>>> + }
>>>>> +
>>>>> + if (!dev->coherent_dma_mask) {
>>>>> + ret = dma_coerce_mask_and_coherent(dev, DMA_BIT_MASK(64));
>>>>> + if (ret) {
>>>>> + dev_warn(dev, "Failed to set DMA mask: %d\n", ret);
>>>>> + return ret;
>>>>> + }
>>>>> + }
>>>>> +
>>>>> + panel = devm_drm_dev_alloc(dev, &ssd16xx_drm_driver,
>>>>> + struct ssd16xx_panel, drm);
>>>>> + if (IS_ERR(panel))
>>>>> + return PTR_ERR(panel);
>>>>> +
>>>>> + drm = &panel->drm;
>>>>> + panel->spi = spi;
>>>>> + panel->model = model;
>>>>> + spi_set_drvdata(spi, panel);
>>>>> +
>>>>> + spi->mode = SPI_MODE_0;
>>>>> + spi->bits_per_word = SSD16XX_SPI_BITS_PER_WORD;
>>>>> +
>>>>> + if (!spi->max_speed_hz) {
>>>>> + drm_warn(drm, "spi-max-frequency not specified, using %u
>>>>> Hz\n",
>>>>> + SSD16XX_SPI_SPEED_DEFAULT);
>>>>> + spi->max_speed_hz = SSD16XX_SPI_SPEED_DEFAULT;
>>>>> + }
>>>>> +
>>>>> + ret = spi_setup(spi);
>>>>> + if (ret < 0) {
>>>>> + drm_err(drm, "SPI setup failed: %d\n", ret);
>>>>> + return ret;
>>>>> + }
>>>>> +
>>>>> + switch (model) {
>>>>> + case GDEY042T81:
>>>>> + panel->controller = SSD1683;
>>>>> + break;
>>>>> + default:
>>>>> + drm_err(drm, "Unknown panel model: %d\n", model);
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + if (panel->controller >=
>>>>> ARRAY_SIZE(ssd16xx_controller_configs) ||
>>>>> + !ssd16xx_controller_configs[panel->controller].max_width)
>>>>> + return -EINVAL;
>>>>> + panel->controller_cfg = &ssd16xx_controller_configs[panel-
>>>>> >controller];
>>>>> +
>>>>> + if (model >= ARRAY_SIZE(ssd16xx_panel_configs))
>>>>> + return -EINVAL;
>>>>> + panel->panel_cfg = &ssd16xx_panel_configs[model];
>>>>> +
>>>>> + mode = devm_kmemdup(dev, panel->panel_cfg->mode,
>>>>> + sizeof(*panel->panel_cfg->mode), GFP_KERNEL);
>>>>> + if (!mode)
>>>>> + return -ENOMEM;
>>>>> +
>>>>> + panel->refresh_mode = panel->panel_cfg->default_refresh_mode;
>>>>> + /* Default color mode: 3-color for panels with red plane, BW
>>>>> otherwise */
>>>>> + panel->color_mode = panel->panel_cfg->red_supported
>>>>> + ? SSD16XX_COLOR_MODE_3COLOR
>>>>> + : SSD16XX_COLOR_MODE_BW;
>>>>> + panel->border_waveform_init_idx = panel->panel_cfg-
>>>>> >default_border_waveform_init;
>>>>> + panel->border_waveform_update_idx = panel->panel_cfg-
>>>>> >default_border_waveform_update;
>>>>> + panel->border_refresh_on_every_update =
>>>>> + panel->panel_cfg->default_border_refresh_on_every_update;
>>>>> + panel->clear_on_init =
>>>>> panel->panel_cfg->default_clear_on_init;
>>>>> + panel->clear_on_close = panel->panel_cfg-
>>>>> >default_clear_on_close;
>>>>> + panel->clear_on_disable = panel->panel_cfg-
>>>>> >default_clear_on_disable;
>>>>> + panel->refresh_mode_init = panel->panel_cfg-
>>>>> >default_refresh_mode_init;
>>>>> +
>>>>> + /* Module parameter overrides for border/display control */
>>>>> + if (border_waveform_init_lut >= 0 &&
>>>>> + border_waveform_init_lut <
>>>>> (int)ARRAY_SIZE(ssd1683_border_waveform_table))
>>>>> + panel->border_waveform_init_idx = border_waveform_init_lut;
>>>>> + if (border_waveform_lut >= 0 &&
>>>>> + border_waveform_lut <
>>>>> (int)ARRAY_SIZE(ssd1683_border_waveform_table))
>>>>> + panel->border_waveform_update_idx = border_waveform_lut;
>>>>> + if (border_refresh_on_every_update)
>>>>> + panel->border_refresh_on_every_update = true;
>>>>> + if (clear_on_init >= 0 && clear_on_init <= 2)
>>>>> + panel->clear_on_init = clear_on_init;
>>>>> + if (clear_on_close >= 0 && clear_on_close <= 2)
>>>>> + panel->clear_on_close = clear_on_close;
>>>>> + if (clear_on_disable >= 0 && clear_on_disable <= 2)
>>>>> + panel->clear_on_disable = clear_on_disable;
>>>>> + if (refresh_mode_init >= 0 && refresh_mode_init <= 2)
>>>>> + panel->refresh_mode_init = refresh_mode_init;
>>>>> +
>>>>> + /* Module parameter overrides panel default refresh mode when
>>>>> set */
>>>>> + if (refresh_mode >= 0) {
>>>>> + if (refresh_mode > SSD16XX_REFRESH_FAST)
>>>>> + drm_warn(drm, "Invalid refresh_mode module param %d,
>>>>> ignored\n",
>>>>> + refresh_mode);
>>>>> + else
>>>>> + panel->refresh_mode = refresh_mode;
>>>>> + }
>>>>> +
>>>>> + /* Module parameter overrides panel default color mode when
>>>>> set */
>>>>> + if (color_mode >= 0) {
>>>>> + if (color_mode > SSD16XX_COLOR_MODE_3COLOR)
>>>>> + drm_warn(drm, "Invalid color_mode module param %d,
>>>>> ignored\n",
>>>>> + color_mode);
>>>>> + else if (color_mode == SSD16XX_COLOR_MODE_3COLOR &&
>>>>> + !panel->panel_cfg->red_supported)
>>>>> + drm_warn(drm,
>>>>> + "color_mode=3-color requested but panel has no
>>>>> red plane, ignored\n");
>>>>> + else
>>>>> + panel->color_mode = color_mode;
>>>>> + }
>>>>> +
>>>>> + /* Parse "rotation" DT property; swap mode dimensions for
>>>>> portrait. */
>>>>> + device_property_read_u32(dev, "rotation", &dt_rotation);
>>>>> + if (dt_rotation != 0 && dt_rotation != 90 && dt_rotation !=
>>>>> 180 && dt_rotation != 270) {
>>>>> + drm_warn(drm, "Invalid DT rotation %u, defaulting to
>>>>> 0°\n", dt_rotation);
>>>>> + dt_rotation = 0;
>>>>> + }
>>>>> + panel->orientation = dt_rotation;
>>>>> +
>>>>> + /* Module parameter overrides DT rotation when set */
>>>>> + if (rotation >= 0) {
>>>>> + if (rotation != 0 && rotation != 90 && rotation != 180 &&
>>>>> rotation != 270)
>>>>> + drm_warn(drm, "Invalid rotation module param %d,
>>>>> ignored\n",
>>>>> + rotation);
>>>>> + else
>>>>> + panel->orientation = rotation;
>>>>> + }
>>>>> +
>>>>> + drm_dbg(drm, "Using %s orientation (%u°, %ux%u logical)\n",
>>>>> + (panel->orientation == 90 || panel->orientation == 270) ?
>>>>> "portrait" : "landscape",
>>>>> + panel->orientation, mode->hdisplay, mode->vdisplay);
>>>>> +
>>>>> + /* Swap mode dimensions for portrait so clients see logical
>>>>> size. */
>>>>> + if (panel->orientation == 90 || panel->orientation == 270) {
>>>>> + swap(mode->hdisplay, mode->vdisplay);
>>>>> + swap(mode->hsync_start, mode->vsync_start);
>>>>> + swap(mode->hsync_end, mode->vsync_end);
>>>>> + swap(mode->htotal, mode->vtotal);
>>>>> + swap(mode->width_mm, mode->height_mm);
>>>>> + drm_dbg(drm, "Mode dimensions swapped for portrait:
>>>>> %ux%u\n",
>>>>> + mode->hdisplay, mode->vdisplay);
>>>>> + } else {
>>>>> + drm_dbg(drm, "Mode dimensions unchanged: %ux%u\n",
>>>>> + mode->hdisplay, mode->vdisplay);
>>>>> + }
>>>>> + panel->mode = mode;
>>>>> + panel->width = mode->hdisplay;
>>>>> + panel->height = mode->vdisplay;
>>>>> +
>>>>> + /* Acquire GPIOs. */
>>>>> + panel->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
>>>>> + if (IS_ERR(panel->reset))
>>>>> + return dev_err_probe(dev, PTR_ERR(panel->reset), "Failed
>>>>> to get RESET GPIO\n");
>>>>> +
>>>>> + panel->busy = devm_gpiod_get(dev, "busy", GPIOD_IN);
>>>>> + if (IS_ERR(panel->busy))
>>>>> + return dev_err_probe(dev, PTR_ERR(panel->busy), "Failed
>>>>> to get BUSY GPIO\n");
>>>>> +
>>>>> + panel->dc = devm_gpiod_get_optional(dev, "dc", GPIOD_OUT_LOW);
>>>>> + if (IS_ERR(panel->dc))
>>>>> + return dev_err_probe(dev, PTR_ERR(panel->dc), "Failed to
>>>>> get DC GPIO\n");
>>>>> + if (!panel->dc) {
>>>>> + if (!spi_is_bpw_supported(spi, 9))
>>>>> + return dev_err_probe(dev, -EINVAL,
>>>>> + "3-wire SPI mode requires 9-bit word
>>>>> support\n");
>>>>> + drm_dbg(drm, "dc-gpios not specified, using 3-wire
>>>>> (9-bit) SPI mode\n");
>>>>> + }
>>>>> +
>>>>> + ret = ssd16xx_alloc_tx_bufs(panel);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + ssd16xx_hw_reset(panel);
>>>>> +
>>>>> + ret = drmm_mode_config_init(drm);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + drm->mode_config.funcs = &ssd16xx_mode_config_funcs;
>>>>> + drm->mode_config.helper_private =
>>>>> &ssd16xx_mode_config_helper_funcs;
>>>>> + drm->mode_config.min_width = min(panel->width, panel->height);
>>>>> + drm->mode_config.max_width = max(panel->width, panel->height);
>>>>> + drm->mode_config.min_height = min(panel->width, panel->height);
>>>>> + drm->mode_config.max_height = max(panel->width, panel->height);
>>>>> +
>>>>> + drm_connector_helper_add(&panel->connector,
>>>>> &ssd16xx_connector_helper_funcs);
>>>>> + ret = drm_connector_init(drm, &panel->connector,
>>>>> &ssd16xx_connector_funcs,
>>>>> + DRM_MODE_CONNECTOR_SPI);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + ret = drm_universal_plane_init(drm, &panel->primary_plane, 0,
>>>>> + &ssd16xx_plane_funcs,
>>>>> + ssd16xx_formats, ARRAY_SIZE(ssd16xx_formats),
>>>>> + NULL, DRM_PLANE_TYPE_PRIMARY, NULL);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + drm_plane_helper_add(&panel->primary_plane,
>>>>> &ssd16xx_plane_helper_funcs);
>>>>> + drm_plane_enable_fb_damage_clips(&panel->primary_plane);
>>>>> +
>>>>> + ret = drm_crtc_init_with_planes(drm, &panel->crtc, &panel-
>>>>> >primary_plane,
>>>>> + NULL, &ssd16xx_crtc_funcs, NULL);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + drm_crtc_helper_add(&panel->crtc, &ssd16xx_crtc_helper_funcs);
>>>>> +
>>>>> + ret = drm_simple_encoder_init(drm, &panel->encoder,
>>>>> DRM_MODE_ENCODER_NONE);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> + panel->encoder.possible_crtcs = drm_crtc_mask(&panel->crtc);
>>>>> +
>>>>> + ret = drm_connector_attach_encoder(&panel->connector, &panel-
>>>>> >encoder);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + ret = ssd16xx_connector_create_properties(panel);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + drm_mode_config_reset(drm);
>>>>> +
>>>>> + ret = drm_dev_register(drm, 0);
>>>>> + if (ret)
>>>>> + return ret;
>>>>> +
>>>>> + drm_dbg(drm, "SSD16xx e-paper display initialized (%dx%d, %d°
>>>>> rotation)\n",
>>>>> + panel->width, panel->height, panel->orientation);
>>>>> +
>>>>> + drm_client_setup(drm, NULL);
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_remove(struct spi_device *spi)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = spi_get_drvdata(spi);
>>>>> +
>>>>> + drm_dev_unplug(&panel->drm);
>>>>> + drm_atomic_helper_shutdown(&panel->drm);
>>>>> +}
>>>>> +
>>>>> +static void ssd16xx_shutdown(struct spi_device *spi)
>>>>> +{
>>>>> + struct ssd16xx_panel *panel = spi_get_drvdata(spi);
>>>>> +
>>>>> + drm_atomic_helper_shutdown(&panel->drm);
>>>>> +}
>>>>> +
>>>>> +static const struct of_device_id ssd16xx_of_match[] = {
>>>>> + { .compatible = "gooddisplay,gdey042t81", .data = (void
>>>>> *)GDEY042T81 },
>>>>> + { }
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(of, ssd16xx_of_match);
>>>>> +
>>>>> +static const struct spi_device_id ssd16xx_id[] = {
>>>>> + { "gdey042t81", GDEY042T81 },
>>>>> + { }
>>>>> +};
>>>>> +MODULE_DEVICE_TABLE(spi, ssd16xx_id);
>>>>> +
>>>>> +static struct spi_driver ssd16xx_spi_driver = {
>>>>> + .driver = {
>>>>> + .name = "ssd16xx",
>>>>> + .of_match_table = ssd16xx_of_match,
>>>>> + },
>>>>> + .probe = ssd16xx_probe,
>>>>> + .remove = ssd16xx_remove,
>>>>> + .shutdown = ssd16xx_shutdown,
>>>>> + .id_table = ssd16xx_id,
>>>>> +};
>>>>> +module_spi_driver(ssd16xx_spi_driver);
>>>>> +
>>>>> +MODULE_AUTHOR("Devarsh Thakkar <devarsht@ti.com>");
>>>>> +MODULE_DESCRIPTION("DRM driver for Solomon SSD16xx e-paper
>>>>> display controller family");
>>>>> +MODULE_LICENSE("GPL");
>>>>
>>>
>>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v4 1/3] iio: adc: Fix incorrect reading when datarate changed in single mode
From: Andy Shevchenko @ 2026-06-23 9:16 UTC (permalink / raw)
To: Jakub Szczudlo
Cc: linux-iio, andy, antoniu.miclaus, conor+dt, devicetree, dlechner,
duje, jic23, jishnu.prakash, jorge.marques, joshua.crofts1,
krzk+dt, linusw, linux-kernel, marcelo.schmitt, mazziesaccount,
mike.looijmans, nuno.sa, robh, sakari.ailus, wens
In-Reply-To: <20260622221550.374235-2-jakubszczudlo40@gmail.com>
On Tue, Jun 23, 2026 at 12:15:48AM +0200, Jakub Szczudlo wrote:
> When device is suspended and it is in single mode then changing
> datarate doesn't make it actual wait for new measurement, so to
> be sure that read after change is correct functions that changes
> datarate and gain will wait for new data.
...
> +/* Timeout based on the minimum sample rate of 8 SPS (7.5s) */
> +#define ADS1100_MAX_DRDY_TIMEOUT_US 7500000
Not sure if the multiplier will look good here
#define ADS1100_MAX_DRDY_TIMEOUT_US (7500 * USEC_PER_MSEC)
(comment might need an update to use 7500 ms, but see above).
...
> +static bool ads1100_new_data_not_ready(struct ads1100_data *data)
> +{
> + int ret;
> + u8 buffer[3];
> +
> + ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
> + if (ret < 0) {
> + dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> + return true;
> + } else if (ret < 3) {
sizeof()
> + dev_err(&data->client->dev, "Short I2C read\n");
> + return true;
> + }
> +
> + return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
> +}
...
> +static int ads1100_poll_data_ready(struct ads1100_data *data)
> +{
> + int ret;
> + u8 buffer[3];
> + bool data_ready;
> + int datarate = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> + /* To be sure we wait 5 times more than datarate */
> + unsigned long wait_time = DIV_ROUND_CLOSEST(MICRO, 5 * datarate);
First of all, reversed xmas tree order can be better (especially
when something is assigned). Second, use units in the variable name,
wait_time_us. And at last, use USEC_PER_SEC instead of MICRO
(this will need time.h to be included if not yet).
> + /* To be sure that polled value will have value after config change */
> + ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
> + if (ret < 0) {
> + dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> + return ret;
> + }
> + return read_poll_timeout(ads1100_new_data_not_ready, data_ready,
> + !data_ready, wait_time,
> + ADS1100_MAX_DRDY_TIMEOUT_US, false, data);
Why not readx_poll_timeout()? It's a short cut for the one-argument "read"
function.
> +}
...
> ads1100_set_config_bits(data, ADS1100_PGA_MASK, ffs(gain) - 1);
>
> - return 0;
> + ret = ads1100_poll_data_ready(data);
> +
> + return ret;
Is it specifically done due to next patch? But this one is marked as Fix and
will go deep back in the releases. For them this will look unjustified. Just
use
return ads1100_...;
here.
> static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
> {
> unsigned int i;
> unsigned int size;
> + int ret;
>
> size = data->supports_data_rate ? ARRAY_SIZE(ads1100_data_rate) : 1;
> for (i = 0; i < size; i++) {
> - if (ads1100_data_rate[i] == rate)
> - return ads1100_set_config_bits(data, ADS1100_DR_MASK,
> - FIELD_PREP(ADS1100_DR_MASK, i));
> + if (ads1100_data_rate[i] != rate)
> + continue;
This will look better if you break here and add a check
if (i == size)
return -EINVAL;
... then your new code...
return ads1100_...;
> + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(&data->client->dev, pm);
> +
This blank line is not needed as they are coupled, but I don't know if we have
an agreed style in IIO for this.
> + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> + if (ret)
> + return ret;
> +
> + ret = ads1100_set_config_bits(data, ADS1100_DR_MASK,
> + FIELD_PREP(ADS1100_DR_MASK, i));
> + if (ret)
> + return ret;
> + ret = ads1100_poll_data_ready(data);
> +
> + return ret;
As per above.
> }
>
> return -EINVAL;
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v1 2/2] ASoC: codecs: add Qualcomm WSA885X I2C codec driver
From: Prasad Kumpatla @ 2026-06-23 9:13 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai,
Linus Walleij, Bartosz Golaszewski, Srinivas Kandagatla,
linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio
In-Reply-To: <20260611-straight-refined-beetle-e2c934@quoll>
On 6/11/2026 3:09 PM, Krzysztof Kozlowski wrote:
> On Wed, Jun 10, 2026 at 09:27:08PM +0530, Prasad Kumpatla wrote:
>> +};
>> +
>> +static void wsa885x_gpio_set(struct wsa885x_i2c_priv *wsa885x, bool val)
>> +{
>> + if (!wsa885x || !wsa885x->sd_n)
> How wsa885x can be NULL?
>
> This wrapper is pointless. Avoid creating abstraction layers over single
> call to standard kernel interfaces.
Hi Krzysztof,
Thanks for reviewing and comments on patch.
Agree. The NULL check is unnecessary, and the helper does not add any
meaningful abstraction.
I'll remove the wrapper and use the GPIO API directly in the next revision.
>
>> + return;
>> +
>> + gpiod_set_value_cansleep(wsa885x->sd_n, val);
>> +}
>> +
> ...
>
>> +
>> +static void wsa885x_gpio_powerdown(void *data)
>> +{
>> + struct wsa885x_i2c_priv *wsa885x = data;
>> +
>> + if (!wsa885x)
>> + return;
> How is this possible?
No, I will remove all the unnecessary checks in the next version of patch.
>
>> +
>> + wsa885x_gpio_set(wsa885x, true);
>> +}
>> +
> ...
>
>> + if (count > 0) {
>> + if (count % 2) {
>> + dev_err(dev, "%s: Invalid number of elements in %s (%d)\n",
>> + __func__, init_table_prop, count);
>> + return -EINVAL;
>> + }
>> + if (count > WSA885X_INIT_TABLE_MAX_ITEMS) {
>> + dev_err(dev, "%s: %s has too many elements (%d > %u)\n",
>> + __func__, init_table_prop, count,
>> + WSA885X_INIT_TABLE_MAX_ITEMS);
>> + return -EINVAL;
>> + }
>> + wsa885x->init_table_size = count;
>> +
>> + wsa885x->init_table = devm_kcalloc(dev, wsa885x->init_table_size,
>> + sizeof(*wsa885x->init_table), GFP_KERNEL);
>> + if (!wsa885x->init_table)
>> + return -ENOMEM;
>> +
>> + if (device_property_read_u32_array(dev, init_table_prop,
>> + wsa885x->init_table,
>> + wsa885x->init_table_size)) {
>> + dev_err(dev, "%s: Failed to read %s\n",
>> + __func__, init_table_prop);
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + ret = device_property_read_u32(dev, "qcom,battery-config",
>> + &wsa885x->batt_conf);
>> + if (ret) {
>> + wsa885x->batt_conf = WSA885X_BATT_1S;
>> + } else if (wsa885x->batt_conf != WSA885X_BATT_1S &&
>> + wsa885x->batt_conf != WSA885X_BATT_2S) {
>> + return dev_err_probe(dev, -EINVAL,
>> + "Invalid battery config %u (expected 1S or 2S)\n",
>> + wsa885x->batt_conf);
>> + }
>> +
>> + for (i = 0; i < WSA885X_SUPPLIES_NUM; i++)
>> + wsa885x->supplies[i].supply = wsa885x_supply_name[i];
>> +
>> + ret = devm_regulator_bulk_get(dev, WSA885X_SUPPLIES_NUM, wsa885x->supplies);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Failed to get regulators\n");
>> +
>> + ret = regulator_bulk_enable(WSA885X_SUPPLIES_NUM, wsa885x->supplies);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Failed to enable regulators\n");
>> +
>> + ret = devm_add_action_or_reset(dev, wsa885x_regulator_disable, wsa885x);
> Why you cannot simply use devm_regulator_get_enable?
Ack, will use.
>
>> + if (ret)
>> + return dev_err_probe(dev, ret, "devm_add_action_or_reset failed\n");
>> +
>> + wsa885x->sd_n = devm_gpiod_get(dev, "powerdown", GPIOD_OUT_HIGH);
>> + if (IS_ERR(wsa885x->sd_n))
>> + return dev_err_probe(dev, PTR_ERR(wsa885x->sd_n),
>> + "Shutdown Control GPIO not found\n");
> Messed/misaligned indentation.
Ack, Will update
>
>> +
>> + wsa885x_gpio_set(wsa885x, false);
>> +
>> + ret = devm_add_action_or_reset(dev, wsa885x_gpio_powerdown, wsa885x);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "devm_add_action_or_reset failed\n");
>> +
>> + i2c_set_clientdata(client, wsa885x);
>> +
>> + wsa885x->intr_pin = devm_gpiod_get(dev, "interrupt", GPIOD_IN);
>> + if (IS_ERR(wsa885x->intr_pin))
>> + return dev_err_probe(dev, PTR_ERR(wsa885x->intr_pin),
>> + "Interrupt GPIO not found\n");
>> +
>> + ret = wsa885x_register_irq(wsa885x);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "wsa885x irq registration failed\n");
>> +
>> + ret = devm_snd_soc_register_component(dev, component_driver,
>> + wsa885x_i2c_dai,
>> + ARRAY_SIZE(wsa885x_i2c_dai));
>> + if (ret)
>> + return dev_err_probe(dev, ret, "Codec component registration failed\n");
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id wsa885x_i2c_dt_match[] = {
>> + {
>> + .compatible = "qcom,wsa885x-i2c",
>> + },
>> + {}
>> +};
>> +
>> +static const struct i2c_device_id wsa885x_id_i2c[] = {
>> + {"wsa885x_i2c", 0},
> Used named initializers.
Ack, Will update
>
>> + {}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(i2c, wsa885x_id_i2c);
>> +MODULE_DEVICE_TABLE(of, wsa885x_i2c_dt_match);
> Don't come with own coding style. Each above goes IMMEDIATELY after the table.
Agreed. I'll place each MODULE_DEVICE_TABLE() immediately after its
associated table to match the existing kernel style.
Thanks,
Prasad
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH net] net: ethernet: qualcomm: ppe: Demote from supported and fix maintainer addresses
From: Jie Luo @ 2026-06-23 9:08 UTC (permalink / raw)
To: Andrew Lunn, Krzysztof Kozlowski
Cc: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Lei Wei, Suruchi Agarwal, Pavithra R, linux-kernel, linux-arm-msm,
linux-clk, devicetree, netdev
In-Reply-To: <0247dfba-1c14-4fea-aab3-5489a36f35f6@lunn.ch>
On 6/23/2026 4:10 PM, Andrew Lunn wrote:
> Emails to the maintainer of Qualcomm PPE Ethernet driver (Luo Jie
> <quic_luoj@quicinc.com>) bounce permanently (full mailbox), because the
> "quicinc.com" addresses were deprecated for public work. All Qualcomm
> contributors are aware of that and were asked to fix their addresses.
>
> Driver is not supported - in terms of how netdev understands supported
> commitment - if maintainer does not care to receive the patches for its
> code, so demote it to "maintained" to reflect true status.
>
> Fix all occurences of Luo Jie email address to preferred and working
> domain.
Thanks a lot for fixing my email address and for the help!
Acked-by: Luo Jie <jie.luo@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: sound: add qcom,wsa885x-i2c
From: Prasad Kumpatla @ 2026-06-23 9:07 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai,
Linus Walleij, Bartosz Golaszewski, Srinivas Kandagatla,
linux-arm-msm, linux-sound, devicetree, linux-kernel, linux-gpio
In-Reply-To: <20260611-debonair-barnacle-of-action-ee9d22@quoll>
On 6/11/2026 3:04 PM, Krzysztof Kozlowski wrote:
> On Wed, Jun 10, 2026 at 09:27:07PM +0530, Prasad Kumpatla wrote:
>> Document the Qualcomm WSA885X I2C smart amplifier binding.
>>
>> Describe the required supplies, powerdown and interrupt GPIOs, the
>> optional battery configuration, and the optional init-table property
>> used to program the device during codec initialization.
>>
>> This matches the driver programming model and documents the DT data
> Binding matches hardware, not driver. Please describe the hardware.
Hi Krzysztof,
Thanks for reviewing the patch and for the feedback.
Ack, Will add more HW details in next version.
>
>> needed to use the codec on platforms with Audio IF playback.
>>
>> Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
>> ---
>> .../bindings/sound/qcom,wsa885x-i2c.yaml | 89 +++++++++++++++++++
>> 1 file changed, 89 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml b/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml
>> new file mode 100644
>> index 000000000..1069f470d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml
> There is no I2C in device name.
Ack, Will remove.
>
>> @@ -0,0 +1,89 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/qcom,wsa885x-i2c.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm WSA885x I2C smart speaker amplifier
>> +
>> +maintainers:
>> + - Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
>> + - Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
>> +
>> +description: |
> Do not need '|' unless you need to preserve formatting.
Ack, Will Update.
>
>> + WSA885x is a Qualcomm Aqstic smart speaker amplifier with an I2C control
>> + interface and a digital audio interface exposed through ASoC DAI callbacks.
>> +
>> +allOf:
>> + - $ref: dai-common.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: qcom,wsa885x-i2c
> Same here
>
> Also, incorrect usage of wildcard. Look at other bindings how this is
> written, so you will not repeat the same comments:
> https://lore.kernel.org/all/20250522-rb2_audio_v3-v3-3-9eeb08cab9dc@linaro.org/
>
> Read writing bindings before posting next version.
>
> I also cannot find traces of internal review of this. Did it happen? Did
> you receive toolset comments?
Ack, Thanks for the reference link, will cross check and update the
bindings.
No, there is o internal review done for this patch due to timelines.
>
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + '#sound-dai-cells':
>> + const: 0
>> +
>> + powerdown-gpios:
>> + description: GPIO controlling the SD_N powerdown pin.
>> + maxItems: 1
>> +
>> + interrupt-gpios:
> No, interrupts are never written as GPIOs.
>
> Where is this binding coming from?
Agree, Will remove this and come up standard interrupt bindings in next
version.
>
>> + description: GPIO used for the codec interrupt output.
>> + maxItems: 1
>> +
>> + vdd-1p8-supply: true
>> +
>> + vdd-io-supply: true
>> +
>> + qcom,battery-config:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Speaker battery configuration, 1 for 1S and 2 for 2S.
> Use string
Ack.
>
>> + default: 1
>> + enum: [1, 2]
>> +
>> + qcom,wsa885x-init-table:
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + minItems: 2
>> + maxItems: 256
>> + description: |
>> + Sequence of register/value pairs applied during codec hardware
> No, we don't store register values usually.
Ack,I'll move them into the driver as a register table,
making them easier to maintain and avoiding opaque DT data.
>> + initialization. Entries are encoded as alternating register address and
>> + register value cells. The number of entries must be even (register/value
>> + pairs); maxItems is 256 (128 pairs).
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - '#sound-dai-cells'
>> + - powerdown-gpios
>> + - interrupt-gpios
>> + - vdd-1p8-supply
>> + - vdd-io-supply
>> +
>> +additionalProperties: false
> unevaluated instead. Again, OPEN other existing bindings. Why doing
> something completely different? Is there any WSA88xx binding with
> additionalProperties? No.
Thanks for pointing this out. I'll align the schema with the existing
WSA88xx
bindings and replace additionalProperties: false with
unevaluatedProperties: false
in the next revision.
Thanks,
Prasad
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH v2 3/4] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support
From: Markus Elfring @ 2026-06-23 9:05 UTC (permalink / raw)
To: Amit Barzilai, dri-devel, devicetree, linux-fbdev, linux-staging,
Andy Shevchenko, Conor Dooley, David Airlie, Greg Kroah-Hartman,
Helge Deller, Javier Martinez Canillas, Krzysztof Kozlowski,
Maarten Lankhorst, Maxime Ripard, Rob Herring, Simona Vetter,
Thomas Zimmermann
Cc: LKML, Adam Azuddin, Chintan Patel
In-Reply-To: <20260622152506.78627-4-amit.barzilai22@gmail.com>
…
> +++ b/drivers/gpu/drm/solomon/ssd130x.c
> @@ -146,6 +146,33 @@
> #define SSD133X_COLOR_DEPTH_256 0x0
> #define SSD133X_COLOR_DEPTH_65K 0x1
>
> +/* ssd135x commands */
> +#define SSD135X_SET_COL_RANGE 0x15
> +#define SSD135X_WRITE_RAM 0x5c
> +#define SSD135X_SET_ROW_RANGE 0x75
…
How do you think about to use an enumeration for such data?
https://en.wikipedia.org/wiki/Enumerated_type#C_and_syntactically_similar_languages
Regards,
Markus
^ permalink raw reply
* Re: [PATCH v2 2/4] drm/ssd130x: Add RGB565 support to SSD133X family
From: Andy Shevchenko @ 2026-06-23 9:03 UTC (permalink / raw)
To: Amit Barzilai
Cc: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam,
chintanlike, dri-devel, devicetree, linux-kernel, linux-fbdev,
linux-staging
In-Reply-To: <20260622152506.78627-3-amit.barzilai22@gmail.com>
On Mon, Jun 22, 2026 at 06:25:04PM +0300, Amit Barzilai wrote:
> SSD133X screens were getting 8bpp (RGB332) instead of the 16bpp
> (RGB565) that they support. This change adds a boolean to the
> deviceinfo struct selecting whether the variant is driven at
> DRM_FORMAT_RGB565.
>
> Changed SSD133X to now utilize 65k color (RGB565).
...
> - * Each Segment has a 8-bit pixel and each Common output has a
> - * row of pixels. When using the (default) horizontal address
> - * increment mode, each byte of data sent to the controller has
> - * a Segment (e.g: SEG0).
> + * Each Segment holds one pixel and each Common output has a row
> + * of pixels. A pixel is 8 bits (one byte) in the 256 color
> + * (RGB332) format or 16 bits (two bytes) in the 65k color
> + * (RGB565) format. When using the (default) horizontal address
> + * increment mode, the pixel data is sent Segment by Segment
> + * (e.g: SEG0 first).
> *
> * When using the 256 color depth format, each pixel contains 3
> * sub-pixels for color A, B and C. These have 3 bit, 3 bit and
> * 2 bits respectively.
Something wrong with the plural. There is a difference between "3-bit" and
"3 bits", but "3 bit" is odd.
> + *
> + * When using the 65k color depth format, each pixel contains 3
> + * sub-pixels for color A, B and C. These have 5 bit, 6 bit and
> + * 5 bits respectively.
Same mistake is repeated here.
...
> + /*
> + * Per-variant output format selector for the SSD133X data path. The
> + * hardware can drive the panel in RGB332 (1 byte/pixel) or RGB565
> + * (2 bytes/pixel); this is a policy choice per variant, not a
In other comments it was spelled fully, be consistent "1 byte per pixel",
"2 bytes per pixel".
> + * capability probe. When set, the variant is driven at RGB565.
> + */
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH V13 2/9] dt-bindings: iio: imu: icm42600: Add icm42607
From: Jean-Baptiste Maneyrol @ 2026-06-23 8:14 UTC (permalink / raw)
To: Chris Morgan
Cc: Jonathan Cameron, Chris Morgan, linux-iio@vger.kernel.org,
andy@kernel.org, nuno.sa@analog.com, dlechner@baylibre.com,
linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org,
heiko@sntech.de, conor+dt@kernel.org, krzk+dt@kernel.org,
robh@kernel.org, andriy.shevchenko@intel.com, Krzysztof Kozlowski
In-Reply-To: <PH0PR19MB997338ED05370F27B730FEE60CA5EE2@PH0PR19MB997338.namprd19.prod.outlook.com>
>From: Chris Morgan <macromorgan@hotmail.com>
>Sent: Tuesday, June 23, 2026 02:06
>To: Jean-Baptiste Maneyrol
>Cc: Jonathan Cameron; Chris Morgan; linux-iio@vger.kernel.org; andy@kernel.org; nuno.sa@analog.com; dlechner@baylibre.com; linux-rockchip@lists.infradead.org; devicetree@vger.kernel.org; heiko@sntech.de; conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org; andriy.shevchenko@intel.com; Krzysztof Kozlowski
>Subject: Re: [PATCH V13 2/9] dt-bindings: iio: imu: icm42600: Add icm42607
>
>On Mon, Jun 22, 2026 at 09: 23: 28AM +0000, Jean-Baptiste Maneyrol wrote: > Hello Chris and Jonathan, > > concerning dt bindings, my initial understanding was that we had a file per > driver. But here, Chris is doing a new driver for
>ZjQcmQRYFpfptBannerStart
>This Message Is From an External Sender
>This message came from outside your organization.
>
>ZjQcmQRYFpfptBannerEnd
>
>On Mon, Jun 22, 2026 at 09:23:28AM +0000, Jean-Baptiste Maneyrol wrote:
>> Hello Chris and Jonathan,
>>
>> concerning dt bindings, my initial understanding was that we had a file per
>> driver. But here, Chris is doing a new driver for icm42607 while adding new
>> bindings here.
>>
>> Does it means we don't have 1 binding file per driver, and there is no need
>> to create a new binding file for inv_icm42607 driver?
>>
>> Despite the naming, icm42607 chips are a complete new design very different
>> than all other icm42600 chips. It using similar IPs for things like the FIFO,
>> but all other parts are different. Especially, it doesn't use banks for
>> registers access but indirect access delegated to the chip internals for
>> accessing certain registers.
>
>For what it's worth I'm not using any of those registers in the driver
>currently; from what I see in the datasheets I was able to find on the
>web the 42607p doesn't do the indirect register access (again unless
>I'm misreading). To be fair I don't have any other icm42607 chips to
>test against. The 42607c does appear to do such register access.
>
>Thank you,
>Chris
Hello Chris,
here is a link to download ICM-42670-P datasheet, this chip is completely similar
to ICM-42607-P:
https://www.invensense.tdk.com/en-us/download-resource/ds-000451-icm-42670-p-datasheet
Indirect register access is required when you want to use the FIFO for configuring
which data is stored inside or when you want to update gyro/accel hardware
offsets (calibbias iio attribute usually). Also required for a lot of more
complex internal chip configuration.
I didn't had a chance to look at your driver currently. I hope to be able to
have a look soon.
I can you give the figures for the required maximum sleep time for accel and
gyro startups and stops. Usually, they are not provided in datasheet (only mean
values).
Thanks for your work,
JB
>
>>
>> Thanks,
>> JB
>>
>> >From: Chris Morgan <macromorgan@hotmail.com>
>> >
>> >Add the ICM42607 and ICM42607P inertial measurement unit.
>> >
>> >This device is functionally very similar to the icm42600 series with a
>> >very different register layout. The driver does not require an
>> >interrupt for these specific chip revisions.
>> >
>> >Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
>> >Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>> >---
>> > .../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-
>> > 1 file changed, 17 insertions(+), 1 deletion(-)
>> >
>> >diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
>> >index 9b2af104f186..81b6e85decd5 100644
>> >--- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
>> >+++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
>> >@@ -30,6 +30,8 @@ properties:
>> > - invensense,icm42600
>> > - invensense,icm42602
>> > - invensense,icm42605
>> >+ - invensense,icm42607
>> >+ - invensense,icm42607p
>> > - invensense,icm42622
>> > - invensense,icm42631
>> > - invensense,icm42686
>> >@@ -67,10 +69,24 @@ properties:
>> > required:
>> > - compatible
>> > - reg
>> >- - interrupts
>> >
>> > allOf:
>> > - $ref: /schemas/spi/spi-peripheral-props.yaml#
>> >+ - if:
>> >+ properties:
>> >+ compatible:
>> >+ contains:
>> >+ enum:
>> >+ - invensense,icm42600
>> >+ - invensense,icm42602
>> >+ - invensense,icm42605
>> >+ - invensense,icm42622
>> >+ - invensense,icm42631
>> >+ - invensense,icm42686
>> >+ - invensense,icm42688
>> >+ then:
>> >+ required:
>> >+ - interrupts
>> >
>> > unevaluatedProperties: false
>> >
>> >--
>> >2.43.0
>
>
________________________________________
From: Chris Morgan <macromorgan@hotmail.com>
Sent: Tuesday, June 23, 2026 02:06
To: Jean-Baptiste Maneyrol
Cc: Jonathan Cameron; Chris Morgan; linux-iio@vger.kernel.org; andy@kernel.org; nuno.sa@analog.com; dlechner@baylibre.com; linux-rockchip@lists.infradead.org; devicetree@vger.kernel.org; heiko@sntech.de; conor+dt@kernel.org; krzk+dt@kernel.org; robh@kernel.org; andriy.shevchenko@intel.com; Krzysztof Kozlowski
Subject: Re: [PATCH V13 2/9] dt-bindings: iio: imu: icm42600: Add icm42607
On Mon, Jun 22, 2026 at 09: 23: 28AM +0000, Jean-Baptiste Maneyrol wrote: > Hello Chris and Jonathan, > > concerning dt bindings, my initial understanding was that we had a file per > driver. But here, Chris is doing a new driver for
ZjQcmQRYFpfptBannerStart
This Message Is From an External Sender
This message came from outside your organization.
ZjQcmQRYFpfptBannerEnd
On Mon, Jun 22, 2026 at 09:23:28AM +0000, Jean-Baptiste Maneyrol wrote:
> Hello Chris and Jonathan,
>
> concerning dt bindings, my initial understanding was that we had a file per
> driver. But here, Chris is doing a new driver for icm42607 while adding new
> bindings here.
>
> Does it means we don't have 1 binding file per driver, and there is no need
> to create a new binding file for inv_icm42607 driver?
>
> Despite the naming, icm42607 chips are a complete new design very different
> than all other icm42600 chips. It using similar IPs for things like the FIFO,
> but all other parts are different. Especially, it doesn't use banks for
> registers access but indirect access delegated to the chip internals for
> accessing certain registers.
For what it's worth I'm not using any of those registers in the driver
currently; from what I see in the datasheets I was able to find on the
web the 42607p doesn't do the indirect register access (again unless
I'm misreading). To be fair I don't have any other icm42607 chips to
test against. The 42607c does appear to do such register access.
Thank you,
Chris
>
> Thanks,
> JB
>
> >From: Chris Morgan <macromorgan@hotmail.com>
> >
> >Add the ICM42607 and ICM42607P inertial measurement unit.
> >
> >This device is functionally very similar to the icm42600 series with a
> >very different register layout. The driver does not require an
> >interrupt for these specific chip revisions.
> >
> >Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> >Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> >---
> > .../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> >diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >index 9b2af104f186..81b6e85decd5 100644
> >--- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >+++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >@@ -30,6 +30,8 @@ properties:
> > - invensense,icm42600
> > - invensense,icm42602
> > - invensense,icm42605
> >+ - invensense,icm42607
> >+ - invensense,icm42607p
> > - invensense,icm42622
> > - invensense,icm42631
> > - invensense,icm42686
> >@@ -67,10 +69,24 @@ properties:
> > required:
> > - compatible
> > - reg
> >- - interrupts
> >
> > allOf:
> > - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >+ - if:
> >+ properties:
> >+ compatible:
> >+ contains:
> >+ enum:
> >+ - invensense,icm42600
> >+ - invensense,icm42602
> >+ - invensense,icm42605
> >+ - invensense,icm42622
> >+ - invensense,icm42631
> >+ - invensense,icm42686
> >+ - invensense,icm42688
> >+ then:
> >+ required:
> >+ - interrupts
> >
> > unevaluatedProperties: false
> >
> >--
> >2.43.0
^ permalink raw reply
* Re: [PATCH 1/3] arm64: dts: qcom: sm8450: Add IPA support
From: Krzysztof Kozlowski @ 2026-06-23 8:55 UTC (permalink / raw)
To: Esteban Urrutia
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alex Elder, linux-arm-msm,
devicetree, linux-kernel, netdev
In-Reply-To: <20260622-sm8450-ipa-v1-1-532f0299f96e@proton.me>
On Mon, Jun 22, 2026 at 09:44:17PM -0400, Esteban Urrutia wrote:
> Add support for IPA in DT while expanding the IMEM region just enough to
> accommodate the modem tables used by IPA.
> As reference, SM8450 uses IPA v5.1.
>
> Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
> ---
> arch/arm64/boot/dts/qcom/sm8450.dtsi | 55 ++++++++++++++++++++++++++++++++----
> 1 file changed, 50 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 56cb6e959e4e..c904720008fa 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -2639,6 +2639,47 @@ adreno_smmu: iommu@3da0000 {
> dma-coherent;
> };
>
> + ipa: ipa@3f40000 {
> + compatible = "qcom,sm8450-ipa";
> +
> + iommus = <&apps_smmu 0x5c0 0x0>,
> + <&apps_smmu 0x5c2 0x0>;
> + reg = <0 0x3f40000 0 0x10000>,
'reg' is always the second property, followed by reg-names.
> + <0 0x3f50000 0 0x5000>,
> + <0 0x3e04000 0 0xfc000>;
> + reg-names = "ipa-reg",
> + "ipa-shared",
> + "gsi";
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: net: qcom,ipa: Add SM8450 compatible string
From: Krzysztof Kozlowski @ 2026-06-23 8:54 UTC (permalink / raw)
To: Esteban Urrutia
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Alex Elder, linux-arm-msm,
devicetree, linux-kernel, netdev
In-Reply-To: <20260622-sm8450-ipa-v1-2-532f0299f96e@proton.me>
On Mon, Jun 22, 2026 at 09:44:18PM -0400, Esteban Urrutia wrote:
> Declare compatible string in ipa binding for SM8450,
> which uses IPA v5.1.
Please wrap commit message according to Linux coding style / submission
process (neither too early nor over the limit):
https://elixir.bootlin.com/linux/v6.4-rc1/source/Documentation/process/submitting-patches.rst#L597
Please organize the patch documenting the compatible (DT bindings)
before the patch using that compatible.
See also: https://elixir.bootlin.com/linux/v6.14-rc6/source/Documentation/devicetree/bindings/submitting-patches.rst#L46
With this fixed:
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v1 1/2] dt-bindings: sound: add qcom,wsa885x-i2c
From: Prasad Kumpatla @ 2026-06-23 8:54 UTC (permalink / raw)
To: Linus Walleij
Cc: Srinivas Kandagatla, Liam Girdwood, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jaroslav Kysela, Takashi Iwai,
Bartosz Golaszewski, Srinivas Kandagatla, linux-arm-msm,
linux-sound, devicetree, linux-kernel, linux-gpio
In-Reply-To: <CAD++jLkFPb4CZqTsAh_qX1Jt9pWxhwhgbhREe9uybL_S7-t60Q@mail.gmail.com>
On 6/11/2026 2:50 AM, Linus Walleij wrote:
> Hi Prasad,
>
> thanks for your patch!
Hi Linus Walleij,
Thanks for reviewing the patch and for the feedback.
>
> On Wed, Jun 10, 2026 at 5:57 PM Prasad Kumpatla
> <prasad.kumpatla@oss.qualcomm.com> wrote:
>
>> Document the Qualcomm WSA885X I2C smart amplifier binding.
> Skip I2C? We don't need to tell e.g. "PCI" in some device on PCI and
> there is no reason to mention I2C for this device, the fact that it sits
> on an I2C bus will be apparent later.
Ack, Will rename the file name.
>
>> Describe the required supplies, powerdown and interrupt GPIOs, the
>> optional battery configuration, and the optional init-table property
>> used to program the device during codec initialization.
>>
>> This matches the driver programming model and documents the DT data
>> needed to use the codec on platforms with Audio IF playback.
>>
>> Signed-off-by: Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
> Perhaps add
> Link: https://www.qualcomm.com/audio/applications/compute-and-mobile-audio/products/wsa8815
>
> (...)
Checking internal to get product Doc to publish.
>> ---
>> .../bindings/sound/qcom,wsa885x-i2c.yaml | 89 +++++++++++++++++++
>> 1 file changed, 89 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml b/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml
> Drop the -i2c suffix on the files.
Ack, Will update.
>
>> new file mode 100644
>> index 000000000..1069f470d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/qcom,wsa885x-i2c.yaml
>> @@ -0,0 +1,89 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/sound/qcom,wsa885x-i2c.yaml#
> Drop the -i2c suffix.
Ack, Will update.
>
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm WSA885x I2C smart speaker amplifier
> Drop I2C.
Ack, Will update.
>
>> +maintainers:
>> + - Srinivas Kandagatla <srinivas.kandagatla@oss.qualcomm.com>
>> + - Prasad Kumpatla <prasad.kumpatla@oss.qualcomm.com>
>> +
>> +description: |
>> + WSA885x is a Qualcomm Aqstic smart speaker amplifier with an I2C control
>> + interface and a digital audio interface exposed through ASoC DAI callbacks.
>> +
>> +allOf:
>> + - $ref: dai-common.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: qcom,wsa885x-i2c
> Drop -i2c
Ack, Will update.
>
>> + reg:
>> + maxItems: 1
>> +
>> + '#sound-dai-cells':
>> + const: 0
>> +
>> + powerdown-gpios:
>> + description: GPIO controlling the SD_N powerdown pin.
>> + maxItems: 1
>> +
>> + interrupt-gpios:
>> + description: GPIO used for the codec interrupt output.
>> + maxItems: 1
>> +
>> + vdd-1p8-supply: true
>> +
>> + vdd-io-supply: true
>> +
>> + qcom,battery-config:
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + description: Speaker battery configuration, 1 for 1S and 2 for 2S.
> What is a "1S" and a "2S"? Include description here.
Ack, Will Update more details for 1s and 2s in next version.
>
>> + default: 1
>> + enum: [1, 2]
>> +
>> + qcom,wsa885x-init-table:
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + minItems: 2
>> + maxItems: 256
>> + description: |
>> + Sequence of register/value pairs applied during codec hardware
>> + initialization. Entries are encoded as alternating register address and
>> + register value cells. The number of entries must be even (register/value
>> + pairs); maxItems is 256 (128 pairs).
> Can this just be a table inside the driver, if it will be the same
> array for every user? If this is board-unique then it needs to describe
> what each value is actually doing well enough so engineers can use this
> documentation right here to configure their board without looking through
> register maps and what not.
>
> Something more abstract using SI units etc is probably needed here.
These values are part of a fixed hardware initialization sequence and
are not board-specific.
I'll move them into the driver as a register table, making them easier
to maintain and
avoiding opaque DT data.
Thanks,
Prasad
>
> Yours,
> Linus Walleij
^ permalink raw reply
* Re: [PATCH] dt-bindings: watchdog: microchip,pic32mzda-wdt: Convert to DT schema
From: Krzysztof Kozlowski @ 2026-06-23 8:51 UTC (permalink / raw)
To: Udaya Kiran Challa
Cc: tsbogend, robh, krzk+dt, conor+dt, skhan, me, linux-rtc,
devicetree, linux-kernel
In-Reply-To: <20260620172354.155565-1-challauday369@gmail.com>
On Sat, Jun 20, 2026 at 10:53:54PM +0530, Udaya Kiran Challa wrote:
> + watchdog@1f800800 {
> + compatible = "microchip,pic32mzda-wdt";
> + reg = <0x1f800800 0x200>;
> + clocks = <&rootclk REF2CLK>;
> + };
Indentaion needs fixing.
With that:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 4/4] staging: fbtft: remove fb_ssd1351 driver
From: Javier Martinez Canillas @ 2026-06-23 8:50 UTC (permalink / raw)
To: Andy Shevchenko, Amit Barzilai
Cc: maarten.lankhorst, mripard, tzimmermann, airlied, simona, robh,
krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam, chintanlike,
dri-devel, devicetree, linux-kernel, linux-fbdev, linux-staging
In-Reply-To: <ajpGy7K4eM8oJIfD@ashevche-desk.local>
Andy Shevchenko <andriy.shevchenko@intel.com> writes:
> On Mon, Jun 22, 2026 at 06:25:06PM +0300, Amit Barzilai wrote:
>> The SSD1351 support was added to the ssd130x DRM driver. To avoid
>> confusion and irrelevant updates, the staging fb_ssd1351 driver is
>> removed.
>
> NAK, the fbtft has two drivers in one (SPI + parallel), plus as Maxime said,
> it has its own binding.
>
Yes, I agree that we don't want to delete the fbtft driver.
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
* Re: [PATCH RFC v7 0/9] firmware: arm_scmi: vendors: Qualcomm Generic Vendor Extensions
From: Sudeep Holla @ 2026-06-23 8:47 UTC (permalink / raw)
To: Pragnesh Papaniya
Cc: Cristian Marussi, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Sibi Sankar, MyungJoo Ham, Kyungmin Park, Chanwoo Choi,
Dmitry Osipenko, Thierry Reding, Jonathan Hunter, Bjorn Andersson,
Konrad Dybcio, Rajendra Nayak, Pankaj Patil, linux-arm-msm,
linux-kernel, arm-scmi, linux-arm-kernel, devicetree, linux-pm,
linux-tegra, Amir Vajid, Ramakrishna Gottimukkula
In-Reply-To: <8725caf9-cebb-49ce-b2c8-4960a6073322@oss.qualcomm.com>
On Fri, Jun 19, 2026 at 06:01:23PM +0530, Pragnesh Papaniya wrote:
>
> On 16-Jun-26 1:57 PM, Sudeep Holla wrote:
>
> > Not sure if it was discussed in the previous versions or not, it would be
> > good if you can capture why some of bus scaling doesn't work with the existing
> > SCMI performance protocol and the monitors don't fit the MPAM mode.
> >
> > Please capture them in 1/9 as a motivation for this vendor protocol. It will
> > then help to understand it better as I am still struggling to. Sorry for that.
>
> Thanks for the input!
>
> SCMI perf protocol exports perf domains to kernel where kernel can set
> the frequency but here the scaling governor runs on the SCP while kernel
> just observes frequency changes made by remote governor.
OK if it is sort of read-only w.r.t kernel, why not perf domain notifications
work to consume the change done by the SCMI platform.
And why do you have set operations in the vendor protocol being proposed then.
It all looks like something just cooked up to make things work. I need
detailed reasoning as why the existing perf protocol can't work considering
all the existing notifications in place.
> While MPAM is not enabled/supported on all hardware (Hamoa).
Fair enough but I still don't fully understand to rule that out yet.
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH 02/11] dt-bindings: Add the actual power domains on U8500
From: Krzysztof Kozlowski @ 2026-06-23 8:46 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Ulf Hansson,
Mark Brown, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
David Airlie, Simona Vetter, Vinod Koul, Frank Li, Lee Jones,
linux-arm-kernel, devicetree, linux-pm, dri-devel, dmaengine
In-Reply-To: <20260618-ux500-power-domains-v7-1-v1-2-eb5e50b1a588@kernel.org>
On Thu, Jun 18, 2026 at 07:00:48AM +0200, Linus Walleij wrote:
> This file has been left in an unfinished state just defining
> the root power domain for the U8500 SoC. Fix it up by adding
> the actual existing power domains in this SoC.
>
> The PRCMU code and old regulator driver is mentioning some
> *_RET domains, this means "retention" and is a state in the
> domain and not a domain of its own.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
From/SoB mismatch.
> ---
> include/dt-bindings/arm/ux500_pm_domains.h | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/include/dt-bindings/arm/ux500_pm_domains.h b/include/dt-bindings/arm/ux500_pm_domains.h
> index 9bd764f0c9e6..1c168e59ac90 100644
> --- a/include/dt-bindings/arm/ux500_pm_domains.h
> +++ b/include/dt-bindings/arm/ux500_pm_domains.h
> @@ -8,8 +8,23 @@
> #define _DT_BINDINGS_ARM_UX500_PM_DOMAINS_H
>
> #define DOMAIN_VAPE 0
> +#define DOMAIN_VARM 1
> +#define DOMAIN_VMODEM 2
> +#define DOMAIN_VPLL 3
> +#define DOMAIN_VSMPS1 4
> +#define DOMAIN_VSMPS2 5
> +#define DOMAIN_VSMPS3 6
> +#define DOMAIN_VRF1 7
> +#define DOMAIN_SVA_MMDSP 8
> +#define DOMAIN_SVA_PIPE 9
> +#define DOMAIN_SIA_MMDSP 10
> +#define DOMAIN_SIA_PIPE 11
> +#define DOMAIN_SGA 12
> +#define DOMAIN_B2R2_MCDE 13
> +#define DOMAIN_ESRAM_12 14
> +#define DOMAIN_ESRAM_34 15
>
> /* Number of PM domains. */
> -#define NR_DOMAINS (DOMAIN_VAPE + 1)
> +#define NR_DOMAINS (DOMAIN_ESRAM_34 + 1)
In a separate commit, instead you need to drop NR_DOMAINS and move them
to driver. If this changes, then it is not ABI. We did similarly for
many clock bindings/drivers.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: display: Add Solomon SSD1351 OLED controller
From: Krzysztof Kozlowski @ 2026-06-23 8:43 UTC (permalink / raw)
To: Amit Barzilai
Cc: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam,
chintanlike, dri-devel, devicetree, linux-kernel, linux-fbdev,
linux-staging
In-Reply-To: <20260622152506.78627-2-amit.barzilai22@gmail.com>
On Mon, Jun 22, 2026 at 06:25:03PM +0300, Amit Barzilai wrote:
> Add a device tree binding for the Solomon SSD1351, a 128x128 65k-color
> RGB OLED display controller driven over a 4-wire SPI bus. The binding
> builds on the shared solomon,ssd-common.yaml properties already used by
> the other Solomon display controllers.
>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> .../bindings/display/solomon,ssd1351.yaml | 42 +++++++++++++++++++
> 1 file changed, 42 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd1351.yaml
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 4/4] staging: fbtft: remove fb_ssd1351 driver
From: Andy Shevchenko @ 2026-06-23 8:41 UTC (permalink / raw)
To: Amit Barzilai
Cc: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller, azuddinadam,
chintanlike, dri-devel, devicetree, linux-kernel, linux-fbdev,
linux-staging
In-Reply-To: <20260622152506.78627-5-amit.barzilai22@gmail.com>
On Mon, Jun 22, 2026 at 06:25:06PM +0300, Amit Barzilai wrote:
> The SSD1351 support was added to the ssd130x DRM driver. To avoid
> confusion and irrelevant updates, the staging fb_ssd1351 driver is
> removed.
NAK, the fbtft has two drivers in one (SPI + parallel), plus as Maxime said,
it has its own binding.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH V2 3/8] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
From: Bartosz Golaszewski @ 2026-06-23 8:34 UTC (permalink / raw)
To: Sherry Sun (OSS)
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun, robh, krzk+dt, conor+dt,
Frank.Li, s.hauer, kernel, festevam, amitkumar.karwar,
neeraj.sanjaykale, marcel, luiz.dentz, hongxing.zhu, l.stach,
lpieralisi, kwilczynski, mani, bhelgaas, brgl
In-Reply-To: <20260623030736.1421537-4-sherry.sun@oss.nxp.com>
On Tue, 23 Jun 2026 05:07:30 +0200, "Sherry Sun (OSS)"
<sherry.sun@oss.nxp.com> said:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> Power supply to the M.2 Bluetooth device attached to the host using M.2
> connector is controlled using the 'uart' pwrseq device. So add support for
> getting the pwrseq device if the OF graph link is present. Once obtained,
> pwrseq_power_on() is called to power up the M.2 Bluetooth card. The power
> sequencer descriptor is obtained via devm_pwrseq_get(), so the power-off
> and cleanup are handled automatically when the device is unbound.
>
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> ---
Reviewed-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
^ permalink raw reply
* Re: [PATCH v4 2/4] iio: adc: mt6323-auxadc: add mt6323 PMIC AUXADC driver
From: sashiko-bot @ 2026-06-23 8:31 UTC (permalink / raw)
To: Roman Vivchar via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260623-mt6323-adc-v4-2-299680ad3194@protonmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] NULL pointer dereference when accessing `dev->parent->parent` if the device is instantiated directly via Device Tree.
- [High] The driver powers down the AUXADC clock during initialization by setting an active-high power-down bit.
- [High] Logic error: Polling a channel data register (`ADC19`) for a busy state using an inverted mask (`GENMASK(15, 1)`).
- [High] The driver disables the 1.8V Reference Voltage by setting an active-low Enable Bar (`_ENB`) bit.
- [Medium] Hardware resource leak: voltage buffer left permanently enabled if register operations fail during channel request or release.
--
commit 14abb8cfa2d6b144f1f3f039d3bbc230861286d0
Author: Roman Vivchar <rva333@protonmail.com>
Date: Tue Jun 23 08:16:14 2026 +0000
iio: adc: mt6323-auxadc: add mt6323 PMIC AUXADC driver
The mt6323 AUXADC is a 15-bit ADC used for system monitoring. This driver
provides support for reading various channels including battery and
charger voltages, battery and chip temperature, current sensing and
accessory detection.
Add a driver for the AUXADC found in the MediaTek mt6323 PMIC.
Tested-by: Ben Grisdale <bengris32@protonmail.ch> # Amazon Echo Dot (2nd Generation)
Signed-off-by: Roman Vivchar <rva333@protonmail.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> diff --git a/drivers/iio/adc/mt6323-auxadc.c b/drivers/iio/adc/mt6323-auxadc.c
> new file mode 100644
> index 0000000000000..c450fb6f09cb5
> --- /dev/null
> +++ b/drivers/iio/adc/mt6323-auxadc.c
[ ... ]
> +static int mt6323_auxadc_prepare_channel(struct mt6323_auxadc *auxadc)
> +{
> + struct regmap *map = auxadc->regmap;
> + u32 val;
> + int ret;
> +
> + ret = regmap_read(map, MT6323_AUXADC_CON19, &val);
> + if (ret)
> + return ret;
> +
> + /* The ADC is idle. */
> + if (!(val & AUXADC_CON19_DECI_GDLY_MASK))
> + return 0;
> +
> + ret = regmap_read_poll_timeout(map, MT6323_AUXADC_ADC19,
> + val, !(val & AUXADC_ADC19_BUSY_MASK),
> + 10, 500);
[Severity: High]
The polling here checks the data payload register MT6323_AUXADC_ADC19 instead
of a status register. Is this a typo? Waiting for both the READY bit and
payload bits to become zero on a data register might immediately succeed
with a garbage state or result in a timeout failure.
> + if (ret)
> + return ret;
> +
> + return regmap_clear_bits(map, MT6323_AUXADC_CON19,
> + AUXADC_CON19_DECI_GDLY_MASK);
> +}
> +
> +static int mt6323_auxadc_request(struct mt6323_auxadc *auxadc,
> + unsigned long channel)
> +{
> + struct regmap *map = auxadc->regmap;
> + int ret;
> +
> + ret = regmap_set_bits(map, MT6323_AUXADC_CON11, AUXADC_CON11_VBUF_EN);
> + if (ret)
> + return ret;
> +
> + return regmap_set_bits(map, MT6323_AUXADC_CON22, BIT(channel));
[Severity: Medium]
If this second register write fails, the function returns an error without
rolling back the AUXADC_CON11_VBUF_EN bit set just above. The caller
in mt6323_auxadc_read_raw() then bypasses mt6323_auxadc_release() on error.
Will this leave the voltage buffer permanently enabled, causing a silent
resource and power leak?
> +}
> +
> +static int mt6323_auxadc_release(struct mt6323_auxadc *auxadc,
> + unsigned long channel)
> +{
> + struct regmap *map = auxadc->regmap;
> + int ret;
> +
> + ret = regmap_clear_bits(map, MT6323_AUXADC_CON22, BIT(channel));
> + if (ret)
> + return ret;
[Severity: Medium]
Similarly here, if clearing the CON22 register fails, the function exits
early and skips clearing the AUXADC_CON11_VBUF_EN bit. Should the voltage
buffer be cleared even if the channel bits fail to clear?
> +
> + return regmap_clear_bits(map, MT6323_AUXADC_CON11, AUXADC_CON11_VBUF_EN);
> +}
[ ... ]
> +static int mt6323_auxadc_init(struct mt6323_auxadc *auxadc)
> +{
> + struct regmap *map = auxadc->regmap;
> + int ret;
> +
> + ret = regmap_set_bits(map, MT6323_STRUP_CON10,
> + AUXADC_STRUP_CON10_RSTB_SW |
> + AUXADC_STRUP_CON10_RSTB_SEL);
> + if (ret)
> + return ret;
> +
> + ret = regmap_set_bits(map, MT6323_TOP_CKPDN2, AUXADC_TOP_CKPDN2_CTL_CK);
[Severity: High]
In MediaTek PMICs, CKPDN usually stands for Clock Power Down. Does setting
this active-high bit power down the AUXADC clock instead of enabling it?
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(map, MT6323_AUXADC_CON10,
> + AUXADC_TRIM_CH2_MASK | AUXADC_TRIM_CH4_MASK |
> + AUXADC_TRIM_CH5_MASK | AUXADC_TRIM_CH6_MASK,
> + FIELD_PREP(AUXADC_TRIM_CH2_MASK, 1) |
> + FIELD_PREP(AUXADC_TRIM_CH4_MASK, 1) |
> + FIELD_PREP(AUXADC_TRIM_CH5_MASK, 1) |
> + FIELD_PREP(AUXADC_TRIM_CH6_MASK, 1));
> + if (ret)
> + return ret;
> +
> + ret = regmap_set_bits(map, MT6323_AUXADC_CON27,
> + AUXADC_CON27_VREF18_ENB_MD |
> + AUXADC_CON27_MD_STATUS);
[Severity: High]
The ENB suffix typically designates an active-low Enable Bar bit.
By using regmap_set_bits() to set AUXADC_CON27_VREF18_ENB_MD to 1, does this
explicitly power down the 1.8V reference voltage needed by the ADC?
> + if (ret)
> + return ret;
[ ... ]
> +static int mt6323_auxadc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mt6323_auxadc *auxadc;
> + struct regmap *regmap;
> + struct iio_dev *iio;
> + int ret;
> +
> + regmap = dev_get_regmap(dev->parent->parent, NULL);
[Severity: High]
The driver exports an of_match_table, which means it could be instantiated
directly by the Device Tree core instead of as an MFD sub-device. In that
scenario, could dev->parent or dev->parent->parent be NULL?
If so, passing a NULL pointer to dev_get_regmap() will cause a kernel crash.
> + if (!regmap)
> + return dev_err_probe(dev, -ENODEV, "failed to get regmap\n");
> +
> + iio = devm_iio_device_alloc(dev, sizeof(*auxadc));
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623-mt6323-adc-v4-0-299680ad3194@protonmail.com?part=2
^ permalink raw reply
* Re: [PATCH v3 2/2] iio: adc: add Axiado SARADC driver
From: Petar Stepanovic @ 2026-06-23 8:30 UTC (permalink / raw)
To: Joshua Crofts
Cc: Akhila Kavi, Prasad Bolisetty, Jonathan Cameron, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Harshit Shah, linux-iio, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260622115554.000036a9@gmail.com>
On 6/22/2026 11:55 AM, Joshua Crofts wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> On Mon, 22 Jun 2026 00:47:28 -0700
> Petar Stepanovic <pstepanovic@axiado.com> wrote:
>
>> Add support for the SARADC controller found on Axiado AX3000 and
>> AX3005 SoCs.
>>
>> The driver supports single-shot voltage reads through the IIO
>> subsystem. The number of available input channels is selected from
>> the SoC match data, allowing AX3000 and AX3005 variants to use the
>> same driver.
>>
>> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
>> ---
>> + info->clk_rate = clk_get_rate(info->clk);
>> + if (!info->clk_rate)
>> + return dev_err_probe(dev, -EINVAL, "invalid clock rate\n");
>> +
>> + ret = devm_regulator_get_enable_read_voltage(dev, "vref");
>> + if (ret < 0)
>> + return dev_err_probe(dev, info->vref_uV,
>> + "failed to get vref voltage\n");
> Sashiko raised an issue that I've missed on previous reads - why
> are you using info->vref_uV in dev_err_probe()? The info struct
> is not zeroed out on initialization, which means that dev_err_probe
> will return a different value each time when read_voltage() fails.
> It was designed to accept the retval from whatever function we're
> checking.
Thank you for catching this.
You are right, |dev_err_probe()| should use the return value from |devm_regulator_get_enable_read_voltage()|, not |info->vref_uV|.
I will fix this in the next version by passing |ret| to |dev_err_probe()| and assigning |info->vref_uV| only after the call succeeds.
Regards,
Petar
^ permalink raw reply
* Re: [PATCH v4 2/4] PCI: rzg3s-host: Use shared reset controls for power domain resets
From: Lad, Prabhakar @ 2026-06-23 8:29 UTC (permalink / raw)
To: Manivannan Sadhasivam
Cc: Philipp Zabel, Claudiu Beznea, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-pci, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <txpkke2xogecipyetascqajgaxamd3ualcuhsibxf75llzcym5@xgcn7efcbmp4>
Hi Manivanna,
On Tue, Jun 23, 2026 at 7:04 AM Manivannan Sadhasivam <mani@kernel.org> wrote:
>
> On Mon, Jun 22, 2026 at 03:53:57PM +0100, Lad, Prabhakar wrote:
> > Hi Manivannan,
> >
> > On Mon, Jun 22, 2026 at 3:30 PM Manivannan Sadhasivam <mani@kernel.org> wrote:
> > >
> > > On Fri, Jun 05, 2026 at 12:54:46PM +0100, Lad, Prabhakar wrote:
> > > > Hi Philipp,
> > > >
> > > > Thank you for the review.
> > > >
> > > > On Wed, Jun 3, 2026 at 9:16 AM Philipp Zabel <p.zabel@pengutronix.de> wrote:
> > > > >
> > > > > On Di, 2026-06-02 at 20:50 +0100, Prabhakar wrote:
> > > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > >
> > > > > > Switch to shared reset controls for PCIe power resets to prepare for
> > > > > > RZ/V2H(P) support. On this platform, multiple PCIe controllers share
> > > > > > the same reset line, requiring shared ownership of the reset control.
> > > > > >
> > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > > > > > Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > > > > > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > > > > > ---
> > > > > > v3->v4:
> > > > > > - Added RB/TB tags.
> > > > > >
> > > > > > v2->v3:
> > > > > > - No change.
> > > > > >
> > > > > > v1->v2:
> > > > > > - Updated commit message.
> > > > > > ---
> > > > > > drivers/pci/controller/pcie-rzg3s-host.c | 6 +++---
> > > > > > 1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> > > > > > index d86e7516dcc2..a5192e4b58df 100644
> > > > > > --- a/drivers/pci/controller/pcie-rzg3s-host.c
> > > > > > +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> > > > > > @@ -1276,9 +1276,9 @@ static int rzg3s_pcie_resets_prepare_and_get(struct rzg3s_pcie_host *host)
> > > > > > for (i = 0; i < data->num_cfg_resets; i++)
> > > > > > host->cfg_resets[i].id = data->cfg_resets[i];
> > > > > >
> > > > > > - ret = devm_reset_control_bulk_get_exclusive(host->dev,
> > > > > > - data->num_power_resets,
> > > > > > - host->power_resets);
> > > > > > + ret = devm_reset_control_bulk_get_shared(host->dev,
> > > > > > + data->num_power_resets,
> > > > > > + host->power_resets);
> > > > > > if (ret)
> > > > > > return ret;
> > > > > >
> > > > >
> > > > > I have a few questions about this.
> > > > >
> > > > > Can you move rzg3s_pcie_resets_prepare_and_get() and
> > > > > rzg3s_pcie_power_resets_deassert() up before setting
> > > > > RZG3S_SYSC_FUNC_ID_MODE and RZG3S_SYSC_FUNC_ID_RST_RSM_B in
> > > > > rzg3s_pcie_probe() without ill effect?
> > > > >
> > > > > Can you move rzg3s_pcie_power_resets_deassert() up before setting
> > > > > RZG3S_SYSC_FUNC_ID_MODE and RZG3S_SYSC_FUNC_ID_RST_RSM_B
> > > > > rzg3s_pcie_resume_noirq()?
> > > > >
> > > > > Those would have the same effect as the reset already being deasserted
> > > > > by the other controller.
> > > > >
> > > > Yes to both. I have reordered the sequences as suggested, and it works
> > > > perfectly without any ill effects.
> > > >
> > >
> > > Are you going to respin the patches incorporating the review comments?
> > >
> > If I have not mistaken, no code changes were requested; it was just
> > that Philipp wanted to ensure the shared reset worked correctly after
> > shuffling the code around.
> >
>
> Ah, I was mistaken.
>
> > I can respin the series if it fails to apply on top of pci/next.
> >
>
> Sure. Please respin once v7.2-rc1 is released.
>
Sure, will do.
Cheers,
Prabhakar
^ permalink raw reply
* Re: [PATCH 2/2] pwm: add Axiado AX3000 PWM driver
From: Petar Stepanovic @ 2026-06-23 8:25 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Akhila Kavi, Prasad Bolisetty, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Harshit Shah, linux-pwm, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <ajlf4t_tuuX-Eplc@monoceros>
On 6/22/2026 6:29 PM, Uwe Kleine-König wrote:
> Hello Petar,
>
> Just a very high-level review:
Thank you for the review.
I will address all your comments in the next version.
Regards,
Petar
^ permalink raw reply
* RE: [PATCH] dt-bindings: clock: Replace bouncing emails
From: Alim Akhtar @ 2026-06-23 6:00 UTC (permalink / raw)
To: 'Krzysztof Kozlowski', 'Bjorn Andersson',
'Michael Turquette', 'Stephen Boyd',
'Brian Masney', 'Rob Herring',
'Krzysztof Kozlowski', 'Conor Dooley',
'Sylwester Nawrocki', 'Chanwoo Choi',
'Peter Griffin', 'Barnabas Czeman',
'Tomasz Figa', linux-arm-msm, linux-clk, devicetree,
linux-kernel, linux-samsung-soc, linux-arm-kernel, cpgs
In-Reply-To: <20260623055626.23814-2-krzysztof.kozlowski@oss.qualcomm.com>
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Sent: Tuesday, June 23, 2026 11:26 AM
> To: Bjorn Andersson <andersson@kernel.org>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Brian
> Masney <bmasney@redhat.com>; Rob Herring <robh@kernel.org>;
> Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Sylwester Nawrocki <s.nawrocki@samsung.com>;
> Chanwoo Choi <cw00.choi@samsung.com>; Peter Griffin
> <peter.griffin@linaro.org>; Alim Akhtar <alim.akhtar@samsung.com>;
> Barnabas Czeman <barnabas.czeman@mainlining.org>; Tomasz Figa
> <tomasz.figa@gmail.com>; linux-arm-msm@vger.kernel.org; linux-
> clk@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-samsung-soc@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Subject: [PATCH] dt-bindings: clock: Replace bouncing emails
>
> Replace permanently bouncing email addresses (550 5.1.1 Recipient address
> rejected) of Adam Skladowski, Sireesh Kodali and Chanho Park. There are
no
> new messages from them via other email addresses, so drop them
> permanently. Add Alim Akhtar to Samsung ExynosAutov9 SoC clocks,
> because he looks at other Samsung clock hardware and drivers.
>
> Signed-off-by: Krzysztof Kozlowski
> <krzysztof.kozlowski@oss.qualcomm.com>
>
Acked-by: Alim Akhtar <alim.akhtar@samsung.com>
^ permalink raw reply
* [PATCH v4 4/4] ARM: dts: mediatek: mt6323: add AUXADC support
From: Roman Vivchar via B4 Relay @ 2026-06-23 8:16 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Lee Jones
Cc: linux-iio, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, Ben Grisdale, Roman Vivchar
In-Reply-To: <20260623-mt6323-adc-v4-0-299680ad3194@protonmail.com>
From: Roman Vivchar <rva333@protonmail.com>
Add the devicetree node for the mt6323 AUXADC.
Tested-by: Ben Grisdale <bengris32@protonmail.ch> # Amazon Echo Dot (2nd Generation)
Signed-off-by: Roman Vivchar <rva333@protonmail.com>
---
arch/arm/boot/dts/mediatek/mt6323.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/mediatek/mt6323.dtsi b/arch/arm/boot/dts/mediatek/mt6323.dtsi
index c230c865116d..c070f4b0936c 100644
--- a/arch/arm/boot/dts/mediatek/mt6323.dtsi
+++ b/arch/arm/boot/dts/mediatek/mt6323.dtsi
@@ -14,6 +14,11 @@ pmic: mt6323 {
interrupt-controller;
#interrupt-cells = <2>;
+ mt6323_adc: adc {
+ compatible = "mediatek,mt6323-auxadc";
+ #io-channel-cells = <1>;
+ };
+
mt6323_leds: leds {
compatible = "mediatek,mt6323-led";
#address-cells = <1>;
--
2.54.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox