All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Boddie <paul@boddie.org.uk>
To: dri-devel@lists.freedesktop.org
Cc: Paul Cercueil <paul@crapouillou.net>,
	Emil Velikov <emil.l.velikov@gmail.com>,
	devicetree <devicetree@vger.kernel.org>,
	od@zcrc.me, "Rafael J . Wysocki" <rafael@kernel.org>,
	David Airlie <airlied@linux.ie>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>
Subject: Re: [PATCH 11/12] gpu/drm: Ingenic: Add support for the IPU
Date: Tue, 19 May 2020 19:10:32 +0200	[thread overview]
Message-ID: <3220152.ycyENPvHUQ@jeremy> (raw)
In-Reply-To: <YGYIAQ.7N9YGTBML0143@crapouillou.net>

On Monday 18. May 2020 13.26.58 Paul Cercueil wrote:
> >>  
> >>  @@ -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.

Since I noticed it in the above patch details, I think the mask when updating 
the burst setting in the LCD_CTRL register should - in general - involve 
multiple bits, since the BST field is 3 bits wide on the JZ4780 and 2 bits 
wide on earlier products. Just setting BURST_16 (0b10) could potentially 
enable BURST_32 (0b11) or other field values that are not explicitly defined.

Hope this is useful!

Paul

WARNING: multiple messages have this Message-ID (diff)
From: Paul Boddie <paul@boddie.org.uk>
To: dri-devel@lists.freedesktop.org
Cc: devicetree <devicetree@vger.kernel.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	David Airlie <airlied@linux.ie>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Emil Velikov <emil.l.velikov@gmail.com>,
	linux-kernel@vger.kernel.org, Rob Herring <robh+dt@kernel.org>,
	Paul Cercueil <paul@crapouillou.net>,
	od@zcrc.me
Subject: Re: [PATCH 11/12] gpu/drm: Ingenic: Add support for the IPU
Date: Tue, 19 May 2020 19:10:32 +0200	[thread overview]
Message-ID: <3220152.ycyENPvHUQ@jeremy> (raw)
In-Reply-To: <YGYIAQ.7N9YGTBML0143@crapouillou.net>

On Monday 18. May 2020 13.26.58 Paul Cercueil wrote:
> >>  
> >>  @@ -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.

Since I noticed it in the above patch details, I think the mask when updating 
the burst setting in the LCD_CTRL register should - in general - involve 
multiple bits, since the BST field is 3 bits wide on the JZ4780 and 2 bits 
wide on earlier products. Just setting BURST_16 (0b10) could potentially 
enable BURST_32 (0b11) or other field values that are not explicitly defined.

Hope this is useful!

Paul
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2020-05-19 17:20 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-16 21:50 [PATCH 01/12] dt-bindings: display: Convert ingenic,lcd.txt to YAML Paul Cercueil
2020-05-16 21:50 ` Paul Cercueil
2020-05-16 21:50 ` [PATCH 02/12] dt-bindings: display: Add ingenic,ipu.yaml Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-17  6:17   ` Sam Ravnborg
2020-05-17  6:17     ` Sam Ravnborg
2020-05-17 11:06     ` Paul Cercueil
2020-05-17 11:06       ` Paul Cercueil
2020-05-16 21:50 ` [PATCH 03/12] component: Support binding with no matches Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-16 21:50 ` [PATCH 04/12] gpu/drm: ingenic: Fix bogus crtc_atomic_check callback Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-17  6:17   ` Sam Ravnborg
2020-05-17  6:17     ` Sam Ravnborg
2020-05-17 12:19     ` Paul Cercueil
2020-05-17 12:19       ` Paul Cercueil
2020-05-16 21:50 ` [PATCH 05/12] gpu/drm: Ingenic: Fix opaque pointer casted to wrong type Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-17  6:21   ` Sam Ravnborg
2020-05-17  6:21     ` Sam Ravnborg
2020-05-17 12:19     ` Paul Cercueil
2020-05-17 12:19       ` Paul Cercueil
2020-05-16 21:50 ` [PATCH 06/12] gpu/drm: Ingenic: Fix incorrect assumption about plane->index Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-19 11:49   ` Sasha Levin
2020-05-19 11:49     ` Sasha Levin
2020-05-16 21:50 ` [PATCH 07/12] gpu/drm: ingenic: Set DMA descriptor chain address in probe Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-16 21:50 ` [PATCH 08/12] gpu/drm: Ingenic: Move register definitions to ingenic-drm.h Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-16 21:50 ` [PATCH 09/12] gpu/drm: Ingenic: Add support for OSD mode Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-16 21:50 ` [PATCH 10/12] gpu/drm: Ingenic: Register driver as component master Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-16 21:50 ` [PATCH 11/12] gpu/drm: Ingenic: Add support for the IPU Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-18 10:48   ` Emil Velikov
2020-05-18 10:48     ` Emil Velikov
2020-05-18 11:26     ` Paul Cercueil
2020-05-18 11:26       ` Paul Cercueil
2020-05-19 17:10       ` Paul Boddie [this message]
2020-05-19 17:10         ` Paul Boddie
2020-05-16 21:50 ` [PATCH 12/12] gpu/drm: Ingenic: Support multiple panels/bridges Paul Cercueil
2020-05-16 21:50   ` Paul Cercueil
2020-05-17  6:12 ` [PATCH 01/12] dt-bindings: display: Convert ingenic,lcd.txt to YAML Sam Ravnborg
2020-05-17  6:12   ` Sam Ravnborg
2020-05-28 20:17 ` Rob Herring
2020-05-28 20:17   ` Rob Herring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3220152.ycyENPvHUQ@jeremy \
    --to=paul@boddie.org.uk \
    --cc=airlied@linux.ie \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=emil.l.velikov@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=od@zcrc.me \
    --cc=paul@crapouillou.net \
    --cc=rafael@kernel.org \
    --cc=robh+dt@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.