[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f77e0d03-ba4a-4722-b575-7aee8e93f04b@linaro.org>
Date: Thu, 15 Jan 2026 01:06:27 +0000
From: Bryan O'Donoghue <bryan.odonoghue@...aro.org>
To: Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
Wenmeng Liu <wenmeng.liu@....qualcomm.com>, Robert Foss <rfoss@...nel.org>,
Todor Tomov <todor.too@...il.com>, Mauro Carvalho Chehab <mchehab@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-media@...r.kernel.org,
linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v8 1/3] media: qcom: camss: Add common TPG support
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@....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
Powered by blists - more mailing lists