Re: [PATCH v8 1/3] media: qcom: camss: Add common TPG support

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 14/01/2026 22:07, Vladimir Zapolskiy wrote:
> Hi Wenmeng.
> 
> On 1/14/26 14:18, Wenmeng Liu wrote:
>>
>> Hi Vladimir,
>>
>>
>> On 1/14/2026 1:05 PM, Vladimir Zapolskiy wrote:
>>> Hi Wenmeng.
>>>
>>> On 1/14/26 05:04, Wenmeng Liu wrote:
>>>> Hi Vladimir,
>>>>
>>>> On 1/14/2026 12:27 AM, Vladimir Zapolskiy wrote:
>>>>> Hello Wenmeng.
>>>>>
>>>>> On 1/13/26 11:03, Wenmeng Liu wrote:
>>>>>> Introduce a new common Test Pattern Generator (TPG) implementation 
>>>>>> for
>>>>>> Qualcomm CAMSS. This module provides a generic interface for pattern
>>>>>> generation that can be reused by multiple platforms.
>>>>>>
>>>>>> Unlike CSID-integrated TPG, this TPG acts as a standalone block
>>>>>> that emulates both CSIPHY and sensor behavior, enabling flexible test
>>>>>> patterns without external hardware.
>>>>>>
>>>>>> Signed-off-by: Wenmeng Liu <wenmeng.liu@oss.qualcomm.com>
>>>>>> ---
>>>>>>     drivers/media/platform/qcom/camss/Makefile    |   1 +
>>>>>>     drivers/media/platform/qcom/camss/camss-tpg.c | 710 ++++++++++ 
>>>>>> ++++++
>>>>>> ++++++++++
>>>>>>     drivers/media/platform/qcom/camss/camss-tpg.h | 127 +++++
>>>>>>     drivers/media/platform/qcom/camss/camss.h     |   5 +
>>>>>>     4 files changed, 843 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/media/platform/qcom/camss/Makefile b/drivers/
>>>>>> media/platform/qcom/camss/Makefile
>>>>>> index
>>>>>> 5e349b4915130c71dbff90e73102e46dfede1520..d355e67c25700ac061b878543c32ed8defc03ad0 100644
>>>>>> --- a/drivers/media/platform/qcom/camss/Makefile
>>>>>> +++ b/drivers/media/platform/qcom/camss/Makefile
>>>>>> @@ -27,5 +27,6 @@ qcom-camss-objs += \
>>>>>>             camss-vfe.o \
>>>>>>             camss-video.o \
>>>>>>             camss-format.o \
>>>>>> +        camss-tpg.o \
>>>>>
>>>>> While you're here, please sort and keep the lines in alphabetical 
>>>>> order.
>>>> ACK.
>>>>
>>>>>
>>>>>>     obj-$(CONFIG_VIDEO_QCOM_CAMSS) += qcom-camss.o
>>>>>> diff --git a/drivers/media/platform/qcom/camss/camss-tpg.c b/drivers/
>>>>>> media/platform/qcom/camss/camss-tpg.c
>>>>>> new file mode 100644
>>>>>> index
>>>>>> 0000000000000000000000000000000000000000..f4c015aafa202e5b64fafa3c543128fda6440b11
>>>>>> --- /dev/null
>>>>>> +++ b/drivers/media/platform/qcom/camss/camss-tpg.c
>>>>>> @@ -0,0 +1,710 @@
>>>>>> +// SPDX-License-Identifier: GPL-2.0
>>>>>> +/*
>>>>>> + *
>>>>>> + * Qualcomm MSM Camera Subsystem - TPG Module
>>>>>> + *
>>>>>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its 
>>>>>> subsidiaries.
>>>>>> + */
>>>>>> +#include <linux/clk.h>
>>>>>> +#include <linux/delay.h>
>>>>>> +#include <linux/io.h>
>>>>>> +#include <linux/kernel.h>
>>>>>> +#include <linux/of.h>
>>>>>> +#include <linux/platform_device.h>
>>>>>> +#include <linux/pm_runtime.h>
>>>>>> +#include <media/media-entity.h>
>>>>>> +#include <media/v4l2-device.h>
>>>>>> +#include <media/v4l2-subdev.h>
>>>>>> +
>>>>>> +#include "camss-tpg.h"
>>>>>> +#include "camss.h"
>>>>>> +
>>>>>> +const char * const testgen_payload_modes[] = {
>>>>>> +    "Disabled",
>>>>>> +    "Incrementing",
>>>>>> +    "Alternating 0x55/0xAA",
>>>>>> +    "Reserved",
>>>>>> +    "Reserved",
>>>>>> +    "Pseudo-random Data",
>>>>>> +    "User Specified",
>>>>>> +    "Reserved",
>>>>>> +    "Reserved",
>>>>>> +    "Color bars",
>>>>>> +    "Reserved"
>>>>>
>>>>> It makes little sense to mention the unsupported values, and then
>>>>> introduce enum tpg_testgen_mode to list the supported ones.
>>>>>
>>>> This is for ctrl menu, will do as follow:
>>>> static const char * const testgen_payload_modes[] = {
>>>>        [TPG_PAYLOAD_MODE_DISABLED]          = "Disabled",
>>>>        [TPG_PAYLOAD_MODE_INCREMENTING]      = "Incrementing",
>>>>        [TPG_PAYLOAD_MODE_ALTERNATING_55_AA]       = "Alternating
>>>> 0x55/0xAA",
>>>>        [TPG_PAYLOAD_MODE_RANDOM]      = "Pseudo-random Data",
>>>>        [TPG_PAYLOAD_MODE_USER_SPECIFIED]    = "User Specified",
>>>>        [TPG_PAYLOAD_MODE_COLOR_BARS]        = "Color bars",
>>>> };
>>>>
>>>
>>> This is also not perfect, still userspace is misinformed about a number
>>> of possible TPG modes vs. a number of actually supported TPG modes.
>>>
>> 0x0: INCREMENTING
>> 0x1: ALTERNATING_55_AA
>> 0x4: RANDOM
>> 0x5: USER_SPECIFIED
>> 0x8: COLOR_BARS
>>
>> These values come from the register configuration, these pattern values
>> are consistent with the CSID TPG.
> 
> Userspace should not be aware of such low level details as register values,
> there are many abstraction layers in-between to hide this type of 
> information.
> 
> Writing proper values to registers should be a concern on the driver level,
> it sounds improper to push this simple task and responsibility to 
> userspace.

I think we should stick to the same format as is already upstream for 
the CSID version of this - which is the same data.

---
bod