mbox series

[00/10] Improvements and fixes for mxsfb DRM driver

Message ID 1561555938-21595-1-git-send-email-robert.chiras@nxp.com (mailing list archive)
Headers show
Series Improvements and fixes for mxsfb DRM driver | expand

Message

Robert Chiras June 26, 2019, 1:32 p.m. UTC
This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like 8MQ, 8MM
and 8QXP). Following, are the new features added and fixes from this
patch-set:

1. Add support for drm_bridge
On 8MQ and 8MM, the LCDIF block is not directly connected to a parallel
display connector, where an LCD panel can be attached, but instead it is
connected to DSI controller. Since this DSI stands between the display
controller (eLCDIF) and the physical connector, the DSI can be implemented
as a DRM bridge. So, in order to be able to connect the mxsfb driver to
the DSI driver, the support for a drm_bridge was needed in mxsfb DRM
driver (the actual driver for the eLCDIF block).

2. Add support for additional pixel formats
Some of the pixel formats needed by Android were not implemented in this
driver, but they were actually supported. So, add support for them.

3. Add support for horizontal stride
Having support for horizontal stride allows the use of eLCDIF with a GPU
(for example) that can only output resolution sizes multiple of a power of
8. For example, 1080 is not a power of 16, so in order to support 1920x1080
output from GPUs that can produce linear buffers only in sizes multiple to 16,
this feature is needed.

3. Few minor features and bug-fixing
The addition of max-res DT property was actually needed in order to limit
the bandwidth usage of the eLCDIF block. This is need on systems where
multiple display controllers are presend and the memory bandwidth is not
enough to handle all of them at maximum capacity (like it is the case on
8MQ, where there are two display controllers: DCSS and eLCDIF).
The rest of the patches are bug-fixes.

Mirela Rabulea (1):
  drm/mxsfb: Signal mode changed when bpp changed

Robert Chiras (9):
  drm/mxsfb: Update mxsfb to support a bridge
  drm/mxsfb: Update mxsfb with additional pixel formats
  drm/mxsfb: Fix the vblank events
  dt-bindings: display: Add max-res property for mxsfb
  drm/mxsfb: Add max-res property for MXSFB
  drm/mxsfb: Update mxsfb to support LCD reset
  drm/mxsfb: Improve the axi clock usage
  drm/mxsfb: Clear OUTSTANDING_REQS bits
  drm/mxsfb: Add support for horizontal stride

 .../devicetree/bindings/display/mxsfb.txt          |   6 +
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c                 | 290 ++++++++++++++++++---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c                  | 189 +++++++++++---
 drivers/gpu/drm/mxsfb/mxsfb_drv.h                  |  10 +-
 drivers/gpu/drm/mxsfb/mxsfb_out.c                  |  26 +-
 drivers/gpu/drm/mxsfb/mxsfb_regs.h                 | 128 ++++++---
 6 files changed, 531 insertions(+), 118 deletions(-)

Comments

Guido Günther July 11, 2019, 3:04 p.m. UTC | #1
Hi Robert,
On Wed, Jun 26, 2019 at 04:32:08PM +0300, Robert Chiras wrote:
> This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like 8MQ, 8MM
> and 8QXP). Following, are the new features added and fixes from this
> patch-set:
> 
> 1. Add support for drm_bridge
> On 8MQ and 8MM, the LCDIF block is not directly connected to a parallel
> display connector, where an LCD panel can be attached, but instead it is
> connected to DSI controller. Since this DSI stands between the display
> controller (eLCDIF) and the physical connector, the DSI can be implemented
> as a DRM bridge. So, in order to be able to connect the mxsfb driver to
> the DSI driver, the support for a drm_bridge was needed in mxsfb DRM
> driver (the actual driver for the eLCDIF block).

So I wanted to test this but with both my somewhat cleaned up nwl
driver¹ and the nwl driver forward ported from the nxp vendor tree I'm
looking at a black screen with current mainline - while my dcss forward
port gives me nice output on mipi dsi. Do you have a tree that uses mipi
dsi on imx8mq where I could look at to check for differences?

Cheers,
 -- Guido

> 
> 2. Add support for additional pixel formats
> Some of the pixel formats needed by Android were not implemented in this
> driver, but they were actually supported. So, add support for them.
> 
> 3. Add support for horizontal stride
> Having support for horizontal stride allows the use of eLCDIF with a GPU
> (for example) that can only output resolution sizes multiple of a power of
> 8. For example, 1080 is not a power of 16, so in order to support 1920x1080
> output from GPUs that can produce linear buffers only in sizes multiple to 16,
> this feature is needed.
> 
> 3. Few minor features and bug-fixing
> The addition of max-res DT property was actually needed in order to limit
> the bandwidth usage of the eLCDIF block. This is need on systems where
> multiple display controllers are presend and the memory bandwidth is not
> enough to handle all of them at maximum capacity (like it is the case on
> 8MQ, where there are two display controllers: DCSS and eLCDIF).
> The rest of the patches are bug-fixes.
> 
> Mirela Rabulea (1):
>   drm/mxsfb: Signal mode changed when bpp changed
> 
> Robert Chiras (9):
>   drm/mxsfb: Update mxsfb to support a bridge
>   drm/mxsfb: Update mxsfb with additional pixel formats
>   drm/mxsfb: Fix the vblank events
>   dt-bindings: display: Add max-res property for mxsfb
>   drm/mxsfb: Add max-res property for MXSFB
>   drm/mxsfb: Update mxsfb to support LCD reset
>   drm/mxsfb: Improve the axi clock usage
>   drm/mxsfb: Clear OUTSTANDING_REQS bits
>   drm/mxsfb: Add support for horizontal stride
> 
>  .../devicetree/bindings/display/mxsfb.txt          |   6 +
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c                 | 290 ++++++++++++++++++---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c                  | 189 +++++++++++---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.h                  |  10 +-
>  drivers/gpu/drm/mxsfb/mxsfb_out.c                  |  26 +-
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h                 | 128 ++++++---
>  6 files changed, 531 insertions(+), 118 deletions(-)
> 
> -- 
> 2.7.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

¹ https://lists.freedesktop.org/archives/dri-devel/2019-March/209685.html
Robert Chiras July 12, 2019, 8:15 a.m. UTC | #2
Hi Guido,

On Jo, 2019-07-11 at 17:04 +0200, Guido Günther wrote:
> Hi Robert,
> On Wed, Jun 26, 2019 at 04:32:08PM +0300, Robert Chiras wrote:
> > 
> > This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like
> > 8MQ, 8MM
> > and 8QXP). Following, are the new features added and fixes from
> > this
> > patch-set:
> > 
> > 1. Add support for drm_bridge
> > On 8MQ and 8MM, the LCDIF block is not directly connected to a
> > parallel
> > display connector, where an LCD panel can be attached, but instead
> > it is
> > connected to DSI controller. Since this DSI stands between the
> > display
> > controller (eLCDIF) and the physical connector, the DSI can be
> > implemented
> > as a DRM bridge. So, in order to be able to connect the mxsfb
> > driver to
> > the DSI driver, the support for a drm_bridge was needed in mxsfb
> > DRM
> > driver (the actual driver for the eLCDIF block).
> So I wanted to test this but with both my somewhat cleaned up nwl
> driver¹ and the nwl driver forward ported from the nxp vendor tree
> I'm
> looking at a black screen with current mainline - while my dcss
> forward
> port gives me nice output on mipi dsi. Do you have a tree that uses
> mipi
> dsi on imx8mq where I could look at to check for differences?
Somewhere on the pixel path (between the display controller and the
DSI) there is a block that inverts the polarity. I can't remember
exactly what was the role of this block, but the polarity is inverted
when eLCDIF is used in combination with the DSI.
If you take a look at my DSI driver from NXP releases (I guess you have
them), you will see there is a hack in mode_fixup:

unsigned int *flags = &mode->flags;
if (dsi->sync_pol {
	*flags |= DRM_MODE_FLAG_PHSYNC;
	*flags |= DRM_MODE_FLAG_PVSYNC;
	*flags &= ~DRM_MODE_FLAG_NHSYNC;
	*flags &= ~DRM_MODE_FLAG_NVSYNC;
} else {
	*flags &= ~DRM_MODE_FLAG_PHSYNC;
	*flags &= ~DRM_MODE_FLAG_PVSYNC;
	*flags |= DRM_MODE_FLAG_NHSYNC;
	*flags |= DRM_MODE_FLAG_NVSYNC;
}

I know it's not clean, but it works for now. You can try this in your
driver and see if it helps.
These days I will also take your nwl-dsi driver and test it, and also
add support for bridge and eLCDIF to see if I can make it work.

Best regards,
Robert
> 
> Cheers,
>  -- Guido
> 
> > 
> > 
> > 2. Add support for additional pixel formats
> > Some of the pixel formats needed by Android were not implemented in
> > this
> > driver, but they were actually supported. So, add support for them.
> > 
> > 3. Add support for horizontal stride
> > Having support for horizontal stride allows the use of eLCDIF with
> > a GPU
> > (for example) that can only output resolution sizes multiple of a
> > power of
> > 8. For example, 1080 is not a power of 16, so in order to support
> > 1920x1080
> > output from GPUs that can produce linear buffers only in sizes
> > multiple to 16,
> > this feature is needed.
> > 
> > 3. Few minor features and bug-fixing
> > The addition of max-res DT property was actually needed in order to
> > limit
> > the bandwidth usage of the eLCDIF block. This is need on systems
> > where
> > multiple display controllers are presend and the memory bandwidth
> > is not
> > enough to handle all of them at maximum capacity (like it is the
> > case on
> > 8MQ, where there are two display controllers: DCSS and eLCDIF).
> > The rest of the patches are bug-fixes.
> > 
> > Mirela Rabulea (1):
> >   drm/mxsfb: Signal mode changed when bpp changed
> > 
> > Robert Chiras (9):
> >   drm/mxsfb: Update mxsfb to support a bridge
> >   drm/mxsfb: Update mxsfb with additional pixel formats
> >   drm/mxsfb: Fix the vblank events
> >   dt-bindings: display: Add max-res property for mxsfb
> >   drm/mxsfb: Add max-res property for MXSFB
> >   drm/mxsfb: Update mxsfb to support LCD reset
> >   drm/mxsfb: Improve the axi clock usage
> >   drm/mxsfb: Clear OUTSTANDING_REQS bits
> >   drm/mxsfb: Add support for horizontal stride
> > 
> >  .../devicetree/bindings/display/mxsfb.txt          |   6 +
> >  drivers/gpu/drm/mxsfb/mxsfb_crtc.c                 | 290
> > ++++++++++++++++++---
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.c                  | 189
> > +++++++++++---
> >  drivers/gpu/drm/mxsfb/mxsfb_drv.h                  |  10 +-
> >  drivers/gpu/drm/mxsfb/mxsfb_out.c                  |  26 +-
> >  drivers/gpu/drm/mxsfb/mxsfb_regs.h                 | 128 ++++++---
> >  6 files changed, 531 insertions(+), 118 deletions(-)
> > 
> > --
> > 2.7.4
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fli
> > sts.infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-
> > kernel&data=02%7C01%7Crobert.chiras%40nxp.com%7C7dc01a0bdf9245b
> > 8d87008d70611055b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6369
> > 84542481903425&sdata=ySInO6H1B4kJtJUwRs2uTIUve0SSNZF0s%2Bv%2FDU
> > 0Vy1E%3D&reserved=0
> > 
> ¹ https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fl
> ists.freedesktop.org%2Farchives%2Fdri-devel%2F2019-
> March%2F209685.html&data=02%7C01%7Crobert.chiras%40nxp.com%7C7dc0
> 1a0bdf9245b8d87008d70611055b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C636984542481913416&sdata=ucYDQLiK7RalRF%2B5MeB3%2F76cFLGWa7C
> mxCFLEg4Wvqc%3D&reserved=0
Guido Günther July 16, 2019, 2:54 p.m. UTC | #3
Hi Robert,
On Fri, Jul 12, 2019 at 08:15:32AM +0000, Robert Chiras wrote:
> Hi Guido,
> 
> On Jo, 2019-07-11 at 17:04 +0200, Guido Günther wrote:
> > Hi Robert,
> > On Wed, Jun 26, 2019 at 04:32:08PM +0300, Robert Chiras wrote:
> > > 
> > > This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like
> > > 8MQ, 8MM
> > > and 8QXP). Following, are the new features added and fixes from
> > > this
> > > patch-set:
> > > 
> > > 1. Add support for drm_bridge
> > > On 8MQ and 8MM, the LCDIF block is not directly connected to a
> > > parallel
> > > display connector, where an LCD panel can be attached, but instead
> > > it is
> > > connected to DSI controller. Since this DSI stands between the
> > > display
> > > controller (eLCDIF) and the physical connector, the DSI can be
> > > implemented
> > > as a DRM bridge. So, in order to be able to connect the mxsfb
> > > driver to
> > > the DSI driver, the support for a drm_bridge was needed in mxsfb
> > > DRM
> > > driver (the actual driver for the eLCDIF block).
> > So I wanted to test this but with both my somewhat cleaned up nwl
> > driver¹ and the nwl driver forward ported from the nxp vendor tree
> > I'm
> > looking at a black screen with current mainline - while my dcss
> > forward
> > port gives me nice output on mipi dsi. Do you have a tree that uses
> > mipi
> > dsi on imx8mq where I could look at to check for differences?
> Somewhere on the pixel path (between the display controller and the
> DSI) there is a block that inverts the polarity. I can't remember
> exactly what was the role of this block, but the polarity is inverted
> when eLCDIF is used in combination with the DSI.
> If you take a look at my DSI driver from NXP releases (I guess you have
> them), you will see there is a hack in mode_fixup:
> 
> unsigned int *flags = &mode->flags;
> if (dsi->sync_pol {
> 	*flags |= DRM_MODE_FLAG_PHSYNC;
> 	*flags |= DRM_MODE_FLAG_PVSYNC;
> 	*flags &= ~DRM_MODE_FLAG_NHSYNC;
> 	*flags &= ~DRM_MODE_FLAG_NVSYNC;
> } else {
> 	*flags &= ~DRM_MODE_FLAG_PHSYNC;
> 	*flags &= ~DRM_MODE_FLAG_PVSYNC;
> 	*flags |= DRM_MODE_FLAG_NHSYNC;
> 	*flags |= DRM_MODE_FLAG_NVSYNC;
> }

Thanks for the suggestion! I'll try that.

> 
> I know it's not clean, but it works for now. You can try this in your
> driver and see if it helps.
> These days I will also take your nwl-dsi driver and test it, and also
> add support for bridge and eLCDIF to see if I can make it work.

I have hacky bridge support over here already. Give me some days to
clean it up and it might safe you some work.
Cheers,
 -- Guido
Guido Günther July 20, 2019, 9:09 p.m. UTC | #4
Hi Robert,
On Tue, Jul 16, 2019 at 04:54:50PM +0200, Guido Günther wrote:
> Hi Robert,
> On Fri, Jul 12, 2019 at 08:15:32AM +0000, Robert Chiras wrote:
> > Hi Guido,
> > 
> > On Jo, 2019-07-11 at 17:04 +0200, Guido Günther wrote:
> > > Hi Robert,
> > > On Wed, Jun 26, 2019 at 04:32:08PM +0300, Robert Chiras wrote:
> > > > 
> > > > This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like
> > > > 8MQ, 8MM
> > > > and 8QXP). Following, are the new features added and fixes from
> > > > this
> > > > patch-set:
> > > > 
> > > > 1. Add support for drm_bridge
> > > > On 8MQ and 8MM, the LCDIF block is not directly connected to a
> > > > parallel
> > > > display connector, where an LCD panel can be attached, but instead
> > > > it is
> > > > connected to DSI controller. Since this DSI stands between the
> > > > display
> > > > controller (eLCDIF) and the physical connector, the DSI can be
> > > > implemented
> > > > as a DRM bridge. So, in order to be able to connect the mxsfb
> > > > driver to
> > > > the DSI driver, the support for a drm_bridge was needed in mxsfb
> > > > DRM
> > > > driver (the actual driver for the eLCDIF block).
> > > So I wanted to test this but with both my somewhat cleaned up nwl
> > > driver¹ and the nwl driver forward ported from the nxp vendor tree
> > > I'm
> > > looking at a black screen with current mainline - while my dcss
> > > forward
> > > port gives me nice output on mipi dsi. Do you have a tree that uses
> > > mipi
> > > dsi on imx8mq where I could look at to check for differences?
> > Somewhere on the pixel path (between the display controller and the
> > DSI) there is a block that inverts the polarity. I can't remember
> > exactly what was the role of this block, but the polarity is inverted
> > when eLCDIF is used in combination with the DSI.
> > If you take a look at my DSI driver from NXP releases (I guess you have
> > them), you will see there is a hack in mode_fixup:
> > 
> > unsigned int *flags = &mode->flags;
> > if (dsi->sync_pol {
> > 	*flags |= DRM_MODE_FLAG_PHSYNC;
> > 	*flags |= DRM_MODE_FLAG_PVSYNC;
> > 	*flags &= ~DRM_MODE_FLAG_NHSYNC;
> > 	*flags &= ~DRM_MODE_FLAG_NVSYNC;
> > } else {
> > 	*flags &= ~DRM_MODE_FLAG_PHSYNC;
> > 	*flags &= ~DRM_MODE_FLAG_PVSYNC;
> > 	*flags |= DRM_MODE_FLAG_NHSYNC;
> > 	*flags |= DRM_MODE_FLAG_NVSYNC;
> > }
> 
> Thanks for the suggestion! I'll try that.
>
> > 
> > I know it's not clean, but it works for now. You can try this in your
> > driver and see if it helps.
> > These days I will also take your nwl-dsi driver and test it, and also
> > add support for bridge and eLCDIF to see if I can make it work.
> 
> I have hacky bridge support over here already. Give me some days to
> clean it up and it might safe you some work.

Your suggestion above (plus some other fixes) worked and
mxsfb+nwl+mixel-dphy works over here. I'll try to send a v1 of the nwl
driver out during the week.
Cheers,
 -- Guido

> Cheers,
>  -- Guido
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>
Guido Günther Aug. 13, 2019, 10:23 a.m. UTC | #5
Hi Robert,
On Wed, Jun 26, 2019 at 04:32:08PM +0300, Robert Chiras wrote:
> This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like 8MQ, 8MM
> and 8QXP). Following, are the new features added and fixes from this
> patch-set:

There was some feedback on various patches, do you intend to pick that
up again? That would be cool since there's some overlapping work popping
up already e.g. in

    https://patchwork.freedesktop.org/series/64595/

showing up and it's the base for the tiny

    https://patchwork.freedesktop.org/series/64300/    

Cheers,
 -- Guido
Robert Chiras Aug. 13, 2019, 10:36 a.m. UTC | #6
On Ma, 2019-08-13 at 12:23 +0200, Guido Günther wrote:
> Hi Robert,
> On Wed, Jun 26, 2019 at 04:32:08PM +0300, Robert Chiras wrote:
> > 
> > This patch-set improves the use of eLCDIF block on iMX 8 SoCs (like
> > 8MQ, 8MM
> > and 8QXP). Following, are the new features added and fixes from
> > this
> > patch-set:
> There was some feedback on various patches, do you intend to pick
> that
> up again? That would be cool since there's some overlapping work
> popping
> up already e.g. in
> 
>     https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> Fpatchwork.freedesktop.org%2Fseries%2F64595%2F&data=02%7C01%7Crob
> ert.chiras%40nxp.com%7C6ca6724a656b41912f0408d71fd83da0%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C637012885918631196&sdata=b3CrbNu%
> 2FcsWBOA%2BcaQLX%2BrlrK7%2Fhf2%2F1vZS3eQGN7aM%3D&reserved=0
> 
> showing up and it's the base for the tiny
> 
>     https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2
> Fpatchwork.freedesktop.org%2Fseries%2F64300%2F&data=02%7C01%7Crob
> ert.chiras%40nxp.com%7C6ca6724a656b41912f0408d71fd83da0%7C686ea1d3bc2
> b4c6fa92cd99c5c301635%7C0%7C0%7C637012885918641196&sdata=h6KLVnSx
> xBwvK%2FvPF9zt4DQR6WnF1pyQSwKBTO4rQTg%3D&reserved=0
> 
> Cheers,
>  -- Guido

Hi Guido,

Yes, I plan to submit a next revision, but first I wanted to try it with your patch-set for the nwl-dsi driver.
Thanks for the heads-up.

Best regards,
Robert