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

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:   Mon, 18 May 2020 13:26:58 +0200
From:   Paul Cercueil <paul@...pouillou.net>
To:     Emil Velikov <emil.l.velikov@...il.com>
Cc:     David Airlie <airlied@...ux.ie>, Daniel Vetter <daniel@...ll.ch>,
        Rob Herring <robh+dt@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        devicetree <devicetree@...r.kernel.org>, od@...c.me,
        linux-kernel@...r.kernel.org,
        ML dri-devel <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH 11/12] gpu/drm: Ingenic: Add support for the IPU

Hi Emil,

Le lun. 18 mai 2020 à 11:48, Emil Velikov <emil.l.velikov@...il.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@...pouillou.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