mbox series

[v3,00/15] Improvements and fixes for mxsfb DRM driver

Message ID 1566382555-12102-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 Aug. 21, 2019, 10:15 a.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.

v3:
- Removed the max-res property patches and added support for
  max-memory-bandwidth property, as it is also implemented in other drivers
- Removed unnecessary drm_vblank_off in probe

v2:
- Collected Tested-by from Guido
- Split the first patch, which added more than one feature into relevant
  patches, explaining each feature added
- Also split the second patch into more patches, to differentiate between
  the feature itself (additional pixel formats support) and the cleanup
  of the register definitions for a better representation (guido)
- Included a patch submitted by Guido, while he was testing my patch-set

Guido Günther (1):
  drm/mxsfb: Read bus flags from bridge if present

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

Robert Chiras (13):
  drm/mxsfb: Update mxsfb to support a bridge
  drm/mxsfb: Add defines for the rest of registers
  drm/mxsfb: Reset vital registers for a proper initialization
  drm/mxsfb: Update register definitions using bit manipulation defines
  drm/mxsfb: Update mxsfb with additional pixel formats
  drm/mxsfb: Fix the vblank events
  drm/mxsfb: Add max-memory-bandwidth property for MXSFB
  dt-bindings: display: Add max-memory-bandwidth 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
  drm/mxsfb: Add support for live pixel format change

 .../devicetree/bindings/display/mxsfb.txt          |   5 +
 drivers/gpu/drm/mxsfb/mxsfb_crtc.c                 | 287 ++++++++++++++++++---
 drivers/gpu/drm/mxsfb/mxsfb_drv.c                  | 203 +++++++++++++--
 drivers/gpu/drm/mxsfb/mxsfb_drv.h                  |  12 +-
 drivers/gpu/drm/mxsfb/mxsfb_out.c                  |  26 +-
 drivers/gpu/drm/mxsfb/mxsfb_regs.h                 | 193 +++++++++-----
 6 files changed, 589 insertions(+), 137 deletions(-)

Comments

Guido Günther Aug. 26, 2019, 12:05 p.m. UTC | #1
Hi,
On Wed, Aug 21, 2019 at 01:15:40PM +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:

I've applied this whole series on top of my NWL work and it looks good
with a DSI panel. Applying the whole series also fixes an issue where
after unblank the output was sometimes shifted about half a screen width
to the right (which didn't happen with DCSS). So at least from the parts
I could test:

  Tested-by: Guido Günther <agx@sigxcpu.org> 

for the whole thing.
Cheers,
 -- Guido
> 
> 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.
> 
> v3:
> - Removed the max-res property patches and added support for
>   max-memory-bandwidth property, as it is also implemented in other drivers
> - Removed unnecessary drm_vblank_off in probe
> 
> v2:
> - Collected Tested-by from Guido
> - Split the first patch, which added more than one feature into relevant
>   patches, explaining each feature added
> - Also split the second patch into more patches, to differentiate between
>   the feature itself (additional pixel formats support) and the cleanup
>   of the register definitions for a better representation (guido)
> - Included a patch submitted by Guido, while he was testing my patch-set
> 
> Guido Günther (1):
>   drm/mxsfb: Read bus flags from bridge if present
> 
> Mirela Rabulea (1):
>   drm/mxsfb: Signal mode changed when bpp changed
> 
> Robert Chiras (13):
>   drm/mxsfb: Update mxsfb to support a bridge
>   drm/mxsfb: Add defines for the rest of registers
>   drm/mxsfb: Reset vital registers for a proper initialization
>   drm/mxsfb: Update register definitions using bit manipulation defines
>   drm/mxsfb: Update mxsfb with additional pixel formats
>   drm/mxsfb: Fix the vblank events
>   drm/mxsfb: Add max-memory-bandwidth property for MXSFB
>   dt-bindings: display: Add max-memory-bandwidth 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
>   drm/mxsfb: Add support for live pixel format change
> 
>  .../devicetree/bindings/display/mxsfb.txt          |   5 +
>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c                 | 287 ++++++++++++++++++---
>  drivers/gpu/drm/mxsfb/mxsfb_drv.c                  | 203 +++++++++++++--
>  drivers/gpu/drm/mxsfb/mxsfb_drv.h                  |  12 +-
>  drivers/gpu/drm/mxsfb/mxsfb_out.c                  |  26 +-
>  drivers/gpu/drm/mxsfb/mxsfb_regs.h                 | 193 +++++++++-----
>  6 files changed, 589 insertions(+), 137 deletions(-)
> 
> -- 
> 2.7.4
>
Stefan Agner Aug. 26, 2019, 2:35 p.m. UTC | #2
On 2019-08-26 14:05, Guido Günther wrote:
> Hi,
> On Wed, Aug 21, 2019 at 01:15:40PM +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:
> 
> I've applied this whole series on top of my NWL work and it looks good
> with a DSI panel. Applying the whole series also fixes an issue where
> after unblank the output was sometimes shifted about half a screen width
> to the right (which didn't happen with DCSS). So at least from the parts
> I could test:
> 
>   Tested-by: Guido Günther <agx@sigxcpu.org> 
> 
> for the whole thing.

Thanks for testing! What SoC did you use? I think it would be good to
also give this a try on i.MX 7 or i.MX 6ULL before merging.

--
Stefan


> Cheers,
>  -- Guido
>>
>> 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.
>>
>> v3:
>> - Removed the max-res property patches and added support for
>>   max-memory-bandwidth property, as it is also implemented in other drivers
>> - Removed unnecessary drm_vblank_off in probe
>>
>> v2:
>> - Collected Tested-by from Guido
>> - Split the first patch, which added more than one feature into relevant
>>   patches, explaining each feature added
>> - Also split the second patch into more patches, to differentiate between
>>   the feature itself (additional pixel formats support) and the cleanup
>>   of the register definitions for a better representation (guido)
>> - Included a patch submitted by Guido, while he was testing my patch-set
>>
>> Guido Günther (1):
>>   drm/mxsfb: Read bus flags from bridge if present
>>
>> Mirela Rabulea (1):
>>   drm/mxsfb: Signal mode changed when bpp changed
>>
>> Robert Chiras (13):
>>   drm/mxsfb: Update mxsfb to support a bridge
>>   drm/mxsfb: Add defines for the rest of registers
>>   drm/mxsfb: Reset vital registers for a proper initialization
>>   drm/mxsfb: Update register definitions using bit manipulation defines
>>   drm/mxsfb: Update mxsfb with additional pixel formats
>>   drm/mxsfb: Fix the vblank events
>>   drm/mxsfb: Add max-memory-bandwidth property for MXSFB
>>   dt-bindings: display: Add max-memory-bandwidth 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
>>   drm/mxsfb: Add support for live pixel format change
>>
>>  .../devicetree/bindings/display/mxsfb.txt          |   5 +
>>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c                 | 287 ++++++++++++++++++---
>>  drivers/gpu/drm/mxsfb/mxsfb_drv.c                  | 203 +++++++++++++--
>>  drivers/gpu/drm/mxsfb/mxsfb_drv.h                  |  12 +-
>>  drivers/gpu/drm/mxsfb/mxsfb_out.c                  |  26 +-
>>  drivers/gpu/drm/mxsfb/mxsfb_regs.h                 | 193 +++++++++-----
>>  6 files changed, 589 insertions(+), 137 deletions(-)
>>
>> --
>> 2.7.4
>>
Guido Günther Aug. 26, 2019, 4:34 p.m. UTC | #3
Hi,
On Mon, Aug 26, 2019 at 04:35:10PM +0200, Stefan Agner wrote:
> On 2019-08-26 14:05, Guido Günther wrote:
> > Hi,
> > On Wed, Aug 21, 2019 at 01:15:40PM +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:
> > 
> > I've applied this whole series on top of my NWL work and it looks good
> > with a DSI panel. Applying the whole series also fixes an issue where
> > after unblank the output was sometimes shifted about half a screen width
> > to the right (which didn't happen with DCSS). So at least from the parts
> > I could test:
> > 
> >   Tested-by: Guido Günther <agx@sigxcpu.org> 
> > 
> > for the whole thing.
> 
> Thanks for testing! What SoC did you use? I think it would be good to
> also give this a try on i.MX 7 or i.MX 6ULL before merging.

This was on i.MX8MQ. I don't have hardware to test mxsfb on anything
else over here atm.
Cheers,
 -- Guido

> 
> --
> Stefan
> 
> 
> > Cheers,
> >  -- Guido
> >>
> >> 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.
> >>
> >> v3:
> >> - Removed the max-res property patches and added support for
> >>   max-memory-bandwidth property, as it is also implemented in other drivers
> >> - Removed unnecessary drm_vblank_off in probe
> >>
> >> v2:
> >> - Collected Tested-by from Guido
> >> - Split the first patch, which added more than one feature into relevant
> >>   patches, explaining each feature added
> >> - Also split the second patch into more patches, to differentiate between
> >>   the feature itself (additional pixel formats support) and the cleanup
> >>   of the register definitions for a better representation (guido)
> >> - Included a patch submitted by Guido, while he was testing my patch-set
> >>
> >> Guido Günther (1):
> >>   drm/mxsfb: Read bus flags from bridge if present
> >>
> >> Mirela Rabulea (1):
> >>   drm/mxsfb: Signal mode changed when bpp changed
> >>
> >> Robert Chiras (13):
> >>   drm/mxsfb: Update mxsfb to support a bridge
> >>   drm/mxsfb: Add defines for the rest of registers
> >>   drm/mxsfb: Reset vital registers for a proper initialization
> >>   drm/mxsfb: Update register definitions using bit manipulation defines
> >>   drm/mxsfb: Update mxsfb with additional pixel formats
> >>   drm/mxsfb: Fix the vblank events
> >>   drm/mxsfb: Add max-memory-bandwidth property for MXSFB
> >>   dt-bindings: display: Add max-memory-bandwidth 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
> >>   drm/mxsfb: Add support for live pixel format change
> >>
> >>  .../devicetree/bindings/display/mxsfb.txt          |   5 +
> >>  drivers/gpu/drm/mxsfb/mxsfb_crtc.c                 | 287 ++++++++++++++++++---
> >>  drivers/gpu/drm/mxsfb/mxsfb_drv.c                  | 203 +++++++++++++--
> >>  drivers/gpu/drm/mxsfb/mxsfb_drv.h                  |  12 +-
> >>  drivers/gpu/drm/mxsfb/mxsfb_out.c                  |  26 +-
> >>  drivers/gpu/drm/mxsfb/mxsfb_regs.h                 | 193 +++++++++-----
> >>  6 files changed, 589 insertions(+), 137 deletions(-)
> >>
> >> --
> >> 2.7.4
> >>
>
Leonard Crestez Aug. 26, 2019, 7:19 p.m. UTC | #4
On 26.08.2019 17:35, Stefan Agner wrote:
> On 2019-08-26 14:05, Guido Günther wrote:
>> Hi,
>> On Wed, Aug 21, 2019 at 01:15:40PM +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:
>>
>> I've applied this whole series on top of my NWL work and it looks good
>> with a DSI panel. Applying the whole series also fixes an issue where
>> after unblank the output was sometimes shifted about half a screen width
>> to the right (which didn't happen with DCSS). So at least from the parts
>> I could test:
>>
>>    Tested-by: Guido Günther <agx@sigxcpu.org>
>>
>> for the whole thing.
> 
> Thanks for testing! What SoC did you use? I think it would be good to
> also give this a try on i.MX 7 or i.MX 6ULL before merging.

I did a quick test on imx6sx-sdb and it seems that [PATCH 07/15] 
"drm/mxsfb: Fix the vblank events" causes a hang on boot, even without a 
panel.

If I revert that particular patch it seems to be fine: the new pixel 
formats seem to work in modetest (checked with sii,43wvf1g panel).

--
Regards,
Leonard
Robert Chiras Aug. 28, 2019, 6:49 a.m. UTC | #5
Hi Leonard,

On Lu, 2019-08-26 at 19:19 +0000, Leonard Crestez wrote:
> On 26.08.2019 17:35, Stefan Agner wrote:
> > 
> > On 2019-08-26 14:05, Guido Günther wrote:
> > > 
> > > Hi,
> > > On Wed, Aug 21, 2019 at 01:15:40PM +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:
> > > I've applied this whole series on top of my NWL work and it looks
> > > good
> > > with a DSI panel. Applying the whole series also fixes an issue
> > > where
> > > after unblank the output was sometimes shifted about half a
> > > screen width
> > > to the right (which didn't happen with DCSS). So at least from
> > > the parts
> > > I could test:
> > > 
> > >    Tested-by: Guido Günther <agx@sigxcpu.org>
> > > 
> > > for the whole thing.
> > Thanks for testing! What SoC did you use? I think it would be good
> > to
> > also give this a try on i.MX 7 or i.MX 6ULL before merging.
> I did a quick test on imx6sx-sdb and it seems that [PATCH 07/15] 
> "drm/mxsfb: Fix the vblank events" causes a hang on boot, even
> without a 
> panel.
> 
> If I revert that particular patch it seems to be fine: the new pixel 
> formats seem to work in modetest (checked with sii,43wvf1g panel).
Thanks for feedback. I tested this and, indeed there are issues on 6SX
with this particular patch. It seems that there is a race-condition
caused by the vblank_on call in enable and IRQ routine. Since this is
not happening on any of i.MX8 SoC, I suspect the axi clock usage.
I think I will just remove this patch from the patch-set and handle
this case separately.
> 
> --
> Regards,
> Leonard

Regards,
Robert