linux-kernel - Re: [PATCH v5 2/7] drm/ingenic: Add support for JZ4780 and HDMI output

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 18 Jan 2022 15:50:01 +0100
From:   "H. Nikolaus Schaller" <hns@...delico.com>
To:     Paul Cercueil <paul@...pouillou.net>
Cc:     Mark Rutland <mark.rutland@....com>,
        Paul Boddie <paul@...die.org.uk>,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Neil Armstrong <narmstrong@...libre.com>,
        David Airlie <airlied@...ux.ie>,
        dri-devel <dri-devel@...ts.freedesktop.org>,
        linux-mips <linux-mips@...r.kernel.org>,
        Andrzej Hajda <a.hajda@...sung.com>,
        Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Sam Ravnborg <sam@...nborg.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Harry Wentland <harry.wentland@....com>,
        OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS 
        <devicetree@...r.kernel.org>, Kees Cook <keescook@...omium.org>,
        Jonas Karlman <jonas@...boo.se>,
        Mark Brown <broonie@...nel.org>,
        Maxime Ripard <maxime@...no.tech>,
        Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
        Liam Girdwood <lgirdwood@...il.com>,
        Robert Foss <robert.foss@...aro.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Rob Herring <robh+dt@...nel.org>,
        "Eric W. Biederman" <ebiederm@...ssion.com>,
        Daniel Vetter <daniel@...ll.ch>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Discussions about the Letux Kernel 
        <letux-kernel@...nphoenux.org>
Subject: Re: [PATCH v5 2/7] drm/ingenic: Add support for JZ4780 and HDMI
 output

Hi Paul,
any insights on the JZ_REG_LCD_OSDC issue below?

There is a second, unrelated issue with the introduction of

"drm/bridge: display-connector: implement bus fmts callbacks"

which breaks bus format negotiations.

These are the two last unsolved issues to submit a fully working driver.

> Am 22.12.2021 um 15:03 schrieb H. Nikolaus Schaller <hns@...delico.com>:
> 
>> Am 08.11.2021 um 10:37 schrieb Paul Cercueil <paul@...pouillou.net>:
>> 
>> Hi Nikolaus,
>> 
>> Le dim., nov. 7 2021 at 21:25:38 +0100, H. Nikolaus Schaller <hns@...delico.com> a écrit :
>>> Hi Paul,
>>>>>>> 
>>>>>>> @@ -1274,7 +1319,7 @@ static int ingenic_drm_bind(struct device *dev, bool has_components)
>>>>>>> 	/* Enable OSD if available */
>>>>>>> 	if (soc_info->has_osd)
>>>>>>> -		regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
>>>>>>> +		regmap_set_bits(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
>>>>>> This change is unrelated to this patch, and I'm not even sure it's a valid change. The driver shouldn't rely on previous register values.
>>>>> I think I already commented that I think the driver should also not reset
>>>>> previous register values to zero.
>>>> You did comment this, yes, but I don't agree. The driver *should* reset the registers to zero. It should *not* have to rely on whatever was configured before.
>>>> Even if I did agree, this is a functional change unrelated to JZ4780 support, so it would have to be splitted to its own patch.
>>> Well it is in preparation of setting more bits that are only available for the jz4780.
>>> But it will be splitted into its own patch for other reasons - if we ever make osd working...
>>>>> If I counted correctly this register has 18 bits which seem to include
>>>>> some interrupt masks (which could be initialized somewhere else) and we
>>>>> write a constant 0x1.
>>>>> Of course most other bits are clearly OSD related (alpha blending),
>>>>> i.e. they can have any value (incl. 0) if OSD is disabled. But here we
>>>>> enable it. I think there may be missing some setting for the other bits.
>>>>> So are you sure, that we can unconditionally reset *all* bits
>>>>> except JZ_LCD_OSDC_OSDEN for the jz4780?
>>>>> Well I have no experience with OSD being enabled at all. I.e. I have no
>>>>> test scenario.
> 
> It turns out that the line
> 
> 		ret = clk_prepare_enable(priv->lcd_clk);
> 
> initializes JZ_REG_LCD_OSDC to 0x00010005 (i.e. printk tells 0x0 before).

more detailled test shows that it is the underlying 

	clk_enable(priv->lcd_clk)

(i.e. not the prepare).
> 
> and writing 
> 
> 		regmap_write(priv->map, JZ_REG_LCD_OSDC, JZ_LCD_OSDC_OSDEN);
> 
> overwrites it with 0x00000001.
> 
> This makes the HDMI monitor go/stay black until I write manually 0x5 to
> JZ_REG_LCD_OSDC.
> 
> This means that JZ_LCD_OSDC_ALPHAEN must be enabled on jz4780 as well.
> Hence overwriting just with JZ_LCD_OSDC_OSDEN breaks it.
> 
> Now the questions:
> a) 0x5 is understandable that it sets JZ_LCD_OSDC_ALPHAEN - but why is it needed?
>   Is this a not well documented difference between jz4740/60/70/80?
> b) how can clk_prepare_enable() write 0x00010005 to JZ_REG_LCD_OSDC? Bug or feature?
>   Something in cgu driver going wrong?

I now suspect that it is an undocumented SoC feature.

> c) what do your SoC or panels do if you write 0x5 to JZ_REG_LCD_OSDC?
> d) 0x00010005 also has bit 16 set which is undocumented... But this is a don't care
>   according to jz4780 PM.

BR and thanks,
Nikolaus