Re: [PATCH 11/12] gpu/drm: Ingenic: Add support for the IPU

[Paul Cercueil <paul@crapouillou.net>]

Hi Emil,

Le lun. 18 mai 2020 à 11:48, Emil Velikov <emil.l.velikov@gmail.com> a 
écrit :
> Hi Paul,
> 
> Disclaimer: I don't know much about the hardware :-P
> 
> On Sun, 17 May 2020 at 00:31, Paul Cercueil <paul@crapouillou.net> 
> wrote:
>> 
>>  Add support for the Image Processing Unit (IPU) found in all Ingenic
>>  SoCs.
>> 
> Since the IPU is present on all devices supported, does it make sense
> to have it as separate module?
> Didn't check closely although I suspect doing that will remove the
> need for the component patch.

It makes sense to me; you may not want to enable the IPU on platforms 
where it doesn't add much, e.g. on the JZ4725B where it only handles 
YUV and not RGB.

Besides, while the older JZ4740 SoC has a IPU, it's mem-to-mem and 
cannot output directly to the CRTC, so it cannot be represented as a 
plane. On this SoC the current IPU code cannot be used. So the Ingenic 
DRM driver must still be able to probe with zero components.

>>  --- a/drivers/gpu/drm/ingenic/ingenic-drm.c
>>  +++ b/drivers/gpu/drm/ingenic/ingenic-drm.c
>>  @@ -50,7 +50,7 @@ struct jz_soc_info {
>> 
>>   struct ingenic_drm {
>>          struct drm_device drm;
>>  -       struct drm_plane f0, f1;
>>  +       struct drm_plane f0, f1, *ipu_plane;
>>          struct drm_crtc crtc;
>>          struct drm_encoder encoder;
>> 
>>  @@ -186,13 +186,16 @@ static void 
>> ingenic_drm_crtc_update_timings(struct ingenic_drm *priv,
>>          regmap_update_bits(priv->map, JZ_REG_LCD_CTRL,
>>                             JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16,
>>                             JZ_LCD_CTRL_OFUP | JZ_LCD_CTRL_BURST_16);
>>  +
>>  +       regmap_write(priv->map, JZ_REG_LCD_IPUR, JZ_LCD_IPUR_IPUREN 
>> |
>>  +                    (ht * vpe / 3) << JZ_LCD_IPUR_IPUR_LSB);
> 
> This hunk also indicates that it may be better to merge the IPU within
> the existing driver.

This writes the IPUR register of the CRTC, nothing wrong here.

-Paul