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@xxxxxxxxxxxxx>: > >> Am 08.11.2021 um 10:37 schrieb Paul Cercueil <paul@xxxxxxxxxxxxxxx>: >> >> Hi Nikolaus, >> >> Le dim., nov. 7 2021 at 21:25:38 +0100, H. Nikolaus Schaller <hns@xxxxxxxxxxxxx> 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
