Re: [PATCH v2 2/4] arm64: dts: qcom: shikra-cqm-evk: Enable display and add ili7807s panel

[Arpit Saini <arpit.saini@oss.qualcomm.com>]

Hi Dmitry,

Just following up on this, 
I've addressed all the review comments, Could you take another look when you 
have a chance ? 

Happy to make further changes if needed.
Thanks for your time!.

Thanks,
Arpit

 

On 7/28/2026 11:14 AM, Arpit Saini wrote:
> Hi Dmitry,
> 
> On 7/22/2026 3:57 AM, Dmitry Baryshkov wrote:
>> On Tue, Jul 21, 2026 at 02:35:42PM +0530, Arpit Saini wrote:
>>> Hi Dmitry,
>>>
>>> On 7/20/2026 6:35 PM, Dmitry Baryshkov wrote:
>>>> On Mon, Jul 20, 2026 at 05:37:25PM +0530, Arpit Saini wrote:
>>>>> Hi Konrad,
>>>>>
>>>>> On 7/16/2026 11:21 PM, Konrad Dybcio wrote:
>>>>>> On 7/14/26 9:21 AM, Arpit Saini wrote:
>>>>>>> Hi Dmitry,Konrad
>>>>>>>
>>>>>>> On 7/10/2026 6:09 PM, Dmitry Baryshkov wrote:
>>>>>>>> On Fri, Jul 10, 2026 at 05:28:04PM +0530, Arpit Saini wrote:
>>>>>>>>> Hi Konrad,
>>>>>>>>>
>>>>>>>>> On 7/10/2026 2:31 PM, Arpit Saini wrote:
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>>>>> Normally the backlight EN pin is controlled by a backlight driver.
>>>>>>>>>
>>>>>>>>> However, this panel uses MIPI DCS based backlight control with no
>>>>>>>>>
>>>>>>>>> separate backlight driver. Since there is no driver to assert gpio91,
>>>>>>>>>
>>>>>>>>> we control it via the panel's pinctrl default state..
>>>>>>>>>
>>>>>>>>> So this is platform specific pin , we need this to enable/disable
>>>>>>>>>
>>>>>>>>> the backlight.
>>>>>>>>
>>>>>>>> Handle it from the panel driver. Also please fix your editor to prevent
>>>>>>>> it from inserting extra empty lines.
>>>>>>>>
>>>>>>>
>>>>>>> Earlier I handled it through panel driver, but this GPIO is specific to our
>>>>>>> Qualcomm Daughter Card, it is not going to the panel connector.
>>>>>>
>>>>>> So is it an enable pin of a voltage regulator maybe? I'm having trouble
>>>>>> understanding how this is all tied together, given the panel driver
>>>>>> uses DCS commands to control the backlight status
>>>>>>
>>>>>> Konrad
>>>>>
>>>>> No, it is not a voltage regulator enable.
>>>>> GPIO91 (WLED_P1_EN) is the enable pin of the WLED driver IC on the Qualcomm daughter card.
>>>>> The WLED driver is a boost LED driver that powers the backlight LEDs.
>>>>> The backlight control works in two parts:
>>>>
>>>> _that powers the backlight LEDs_.
>>>>
>>>> So it is a power supply.
>>>>
>>>>>
>>>>> 1. GPIO91 must be asserted to enable the WLED driver IC itself.
>>>>> 2. The panel's CABC output (a PWM signal) is connected to the WLED driver's PWM input
>>>>> and controls the actual brightness.
>>>>> This CABC output is driven by the DCS set_display_brightness command sent from the panel driver.
>>>>>
>>>>> So the DCS commands control brightness via the panel's PWM/CABC output,
>>>>> but the WLED driver IC needs to be enabled first via GPIO91.
>>>>>
>>>>> Since GPIO91 is on the Qualcomm daughter card and not on the panel connector.
>>>>>
>>>>> Dmitry suggested , it is not appropriate to model it in the panel binding.
>>>>> The current approach of asserting it via pinctrl default state in the board DTS keeps it board-specific.
>>>>>
>>>>> If you have a better suggestion for modelling this, I'm happy to follow your guidance.
>>>>>
>>>>> SoC (ITP Baseband Card)
>>>>>  ├── GPIO91 (WLED_P1_EN) ─────────────────────────────────────┐
>>>>>  │                                                            │
>>>>>  └── DSI ──────────────────────────────────────────────────┐  │
>>>>>                                                            │  │
>>>>> ┌──────────────────────────────────────────────────────────┼──┼──────────────────────────────────┐
>>>>> │ Qualcomm Display Daughter Card                           │  │                                  │
>>>>> │                                                          │  │                                  │
>>>>> │  ┌─────────────────────────────────┐                     │  │   ┌──────────────────────┐       │
>>>>> │  │  DLC DLC0697 Panel (ILI7807S)   │◄────────────────────┘  │   │  WLED Driver IC      │       │
>>>>> │  │                                 │                        |   │                      │       │
>>>>> │  │  DCS set_display_brightness()   │                        └──►│  EN                  │       │
>>>>> │  │        │                        │                            │                      │       │
>>>>> │  │        ▼                        │                            │                      │       │
>>>>> │  │  CABC output ───────────────────┼───────────────────────────►│  PWM                 │       │
>>>>> │  │                                 │                            │                      │       │
>>>>> │  │                                 │                            │  LED outputs ────────┼──► Backlight LEDs
>>>>> │  └─────────────────────────────────┘                            └──────────────────────┘       │
>>>>> │                                                                                                │
>>>>> └────────────────────────────────────────────────────────────────────────────────────────────────┘
>>>>
>>>> This is a nice diagram, but it shows backlight LEDs as something
>>>> completely separate. Usually they are a part of the panel. Are they not
>>>> in this case?
>>>>
>>>> I'd be more interested in a different representation of the information.
>>>> What are the _actual_ panel inputs? Decribe those (and only those) in
>>>> the panel bindings.
>>>>
>>>
>>> No , Backlight LEDs are part of the panel, I will correct the diagram for that,
>>> and regards panel bindings, I modeled only the actual panel inputs.
>>>
>>> Hopefully, this diagram will clear this.
>>>
>>> ┌─────────────────┐         ┌───────────────────────────────────────────────────────────────────────────────────────────┐
>>> │    SOC Card     │         │                                  Display DC                                               │
>>> │                 │         │                                                        ┌──────────────────────────────┐   │
>>> │  DSI0           │         │                                                        │      DLC Display CONN        │   │
>>> │  ├─ DSICLK_P/M ─┼─────────┼────────────────────────────────────────────────────────┼──► DSICLK_P/M                │   │
>>> │  ├─ DSILN0_P/M ─┼─────────┼────────────────────────────────────────────────────────┼──► DSILN0_P/M                │   │
>>> │  ├─ DSILN1_P/M ─┼─────────┼────────────────────────────────────────────────────────┼──► DSILN1_P/M                │   │
>>> │  ├─ DSILN2_P/M ─┼─────────┼────────────────────────────────────────────────────────┼──► DSILN2_P/M                │   │
>>> │  └─ DSILN3_P/M ─┼─────────┼────────────────────────────────────────────────────────┼──► DSILN3_P/M                │   │
>>> │                 │         │                                                        │                              │   │
>>> │  1.8V ──────────┼─────────┼──────────────────────────────────── vddi ──────────────┼──► vddi                      │   │
>>> │                 │         │                                                        │                              │   │
>>> │  VSYS_PWR ──────┼─────────┼──────┐                                                 │                              │   │
>>> │                 │         │      │                                                 │                              │   │
>>> │  LCD_BIAS_EN ───┼─────────┼──────┼──► ┌──────────────────┐                         │                              │   │
>>> │                 │         │      ├───►│  LCD BIAS DRIVER  ├──── VREG_DISP_P (+5.5V)┼──► VREG_DISP_P (+5.5V)       │   │
>>> │                 │         │      │    └──────────────────┘──── VREG_DISP_N (-5.5V)─┼──► VREG_DISP_N (-5.5V)       │   │
>>> │                 │         │      │                                                 │                              │   │
>>> │  LCD_BKLT_EN ───┼─────────┼──────┼──► ┌──────────────────┐                         │                              │   │
>>> │                 |         |      |    │   WLED DRIVER    ├──── VREG_WLED ──────────┼──► VREG_WLED                 │   │
>>> │                 │         │      └───►│                   ├──── WLED_SIN1 ─────────┼──► WLED_SIN1                 │   │
>>> │                 │         │           │                   ├──── WLED_SIN2 ─────────┼──► WLED_SIN2                 │   │
>>> │                 │         │           │             ◄─PWM─┤◄───────────────────────┼◄── CABC                      │   │
>>> │                 │         │           └──────────────────┘                         │                              │   │
>>
>> Perfect. So this is LED device, it provides backlight to the panel.
>> Describe it correctly in the DT.
>>
>> Think of somebody using PM8150's WLED with the panel.
>>
>> For the panel bindings use only actual panel inputs. Think of it being
>> used on a completely different platform, without the DC card.
>>
> The panel driver already handles this generically. In ili7807s_probe():
> 	ret = drm_panel_of_backlight(&ctx->panel);
> 	if (ret)
> 		return ret;
> 
> 	if (!ctx->panel.backlight) {
> 		ctx->panel.backlight = ili7807s_create_backlight(dsi);
> 		if (IS_ERR(ctx->panel.backlight))
> 			return dev_err_probe(dev, PTR_ERR(ctx->panel.backlight),
> 					     "failed to create backlight\n");
> 	}
> 
> If a platform provides a `backlight` phandle in the panel DT node 
> (e.g., PM8150 WLED or any other backlight device), drm_panel_of_backlight()
> will use that external backlight device.
> 
> The DCS fallback is not created in that case.
> 
> For our Qualcomm display daughter card, the DCS fallback backlight is used.
> 
> So the panel driver is already platform-agnostic:
> 
> - On a platform with PM8150 WLED → provide backlight = <&pm8150_wled>
> in the panel DT node, and the panel driver uses it directly.
> 
> - On our daughter card → no backlight phandle, panel driver
> creates DCS backlight, GPIO91 is handled via board-level pinctrl.
> 
> Please let me know if there is another way to do this,
> or if I have misunderstood your comment.
> 
> Thanks,
> Arpit
> 
> 
>>
>>> │  MDP_VSYNC_P ───┼─────────┼────────────────────────────────────────────────────────┼──► MDP_VSYNC_P               │   │
>>> │                 │         │                                                        │                              │   │
>>> │  DISP_RST ──────┼─────────┼────────────────────────────────────────────────────────┼──► DISP_RST                  │   │
>>> │                 │         │                                                        │                              │   │
>>> │  Touch ─//──────┼─────────┼────────────────────────────────────────────────────────┼──► Touch signals             │   │
>>> │  signals        │         │                                                        └──────────────────────────────┘   │
>>> └─────────────────┘         └───────────────────────────────────────────────────────────────────────────────────────────┘
>>>
>>> Thanks,
>>> Arpit
>>
>