[PATCH v3 2/5] dt-bindings: display: renesas: lvds: Document r8a7744 bindings

[Biju Das]

Document the RZ/G1N (R8A7744) LVDS bindings.

Signed-off-by: Biju Das 
---
 Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt 
b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
index 27a054e..900a884 100644
--- a/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
+++ b/Documentation/devicetree/bindings/display/bridge/renesas,lvds.txt
@@ -8,6 +8,7 @@ Required properties:
 
 - compatible : Shall contain one of
   - "renesas,r8a7743-lvds" for R8A7743 (RZ/G1M) compatible LVDS encoders
+  - "renesas,r8a7744-lvds" for R8A7744 (RZ/G1N) compatible LVDS encoders
   - "renesas,r8a774c0-lvds" for R8A774C0 (RZ/G2E) compatible LVDS encoders
   - "renesas,r8a7790-lvds" for R8A7790 (R-Car H2) compatible LVDS encoders
   - "renesas,r8a7791-lvds" for R8A7791 (R-Car M2-W) compatible LVDS encoders
-- 
2.7.4

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

[PATCH] drm/sun4i: hdmi: Improve compatibility with hpd-less HDMI displays

[Priit Laes]

From: Priit Laes 

Even though HDMI connector features hotplug detect pin (HPD), there
are older devices which do not support it. For these devices fall
back to additional check on I2C bus to probe for EDID data.

One known example is HDMI/DVI display with following edid:

$ xxd -p display.edid
000005a1e0030100150f0103800f05780a0f6ea05748
9a2610474f20010101010101010101010101010101012a08804520e0
0b10200040009536001800fd0034441a2403000a202020202020
001000310a202020202020202020202000102a4030701300
782d111e006b

$ edid-decode display.edid
EDID version: 1.3
Manufacturer: AMA Model 3e0 Serial Number 1
Digital display
Maximum image size: 15 cm x 5 cm
Gamma: 2.20
RGB color display
First detailed timing is preferred timing
Display x,y Chromaticity:
  Red:   0.6250, 0.3398
  Green: 0.2841, 0.6044
  Blue:  0.1494, 0.0644
  White: 0.2802, 0.3105

Established timings supported:
  640x480@60Hz 4:3 HorFreq: 31469 Hz Clock: 25.175 MHz
Standard timings supported:
Detailed mode: Clock 20.900 MHz, 149 mm x 54 mm
   640  672  672  709 hborder 0
   480  484  484  491 vborder 0
   -hsync -vsync
   VertFreq: 60 Hz, HorFreq: 29478 Hz
Monitor ranges (GTF): 52-68Hz V, 26-36kHz H, max dotclock 30MHz
Dummy block
Dummy block
Checksum: 0x6b (valid)

Now, current implementation is still flawed, as HDMI uses the
HPD signal to indicate that the source should re-read the EDID
due to change in device capabilities. With current HPD polling
implementation we would most certainly miss those notifications
as one can try just swapping two HDMI monitors really fast.

Proper fix would be skipping the HPD pin detection and relying
on just EDID fetching and acting on its changes.

CC: Russell King 
Signed-off-by: Priit Laes 
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c 
b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 25f4d676fd82..3b80380bc76e 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -242,14 +242,18 @@ sun4i_hdmi_connector_detect(struct drm_connector 
*connector, bool force)
struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector);
unsigned long reg;
 
-   if (readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,
-  reg & SUN4I_HDMI_HPD_HIGH,
-  0, 50)) {
-   cec_phys_addr_invalidate(hdmi->cec_adap);
-   return connector_status_disconnected;
-   }
-
-   return connector_status_connected;
+   /* TODO: Drop HPD polling and instead keep track of EDID changes */
+   if (!readl_poll_timeout(hdmi->base + SUN4I_HDMI_HPD_REG, reg,
+   reg & SUN4I_HDMI_HPD_HIGH,
+   0, 50))
+   return connector_status_connected;
+
+   /* Fall back to EDID in case display does not support HPD */
+   if (!IS_ERR(hdmi->i2c) && drm_probe_ddc(hdmi->i2c))
+   return connector_status_connected;
+
+   cec_phys_addr_invalidate(hdmi->cec_adap);
+   return connector_status_disconnected;
 }
 
 static const struct drm_connector_funcs sun4i_hdmi_connector_funcs = {
-- 
2.11.0

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

[PATCH] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.

[Ronald Tschalär]

commit d6abe6df706c66d803e6dd4fe98c1b6b7f125a56 (drm/bridge:
sil_sii8620: do not have a dependency of RC_CORE) added a dependency on
INPUT. However, this causes problems with other drivers, in particular
an input driver that depends on MFD_INTEL_LPSS_PCI (to be added in a
future commit):

  drivers/clk/Kconfig:9:error: recursive dependency detected!
  drivers/clk/Kconfig:9:symbol COMMON_CLK is selected by MFD_INTEL_LPSS
  drivers/mfd/Kconfig:566:  symbol MFD_INTEL_LPSS is selected by 
MFD_INTEL_LPSS_PCI
  drivers/mfd/Kconfig:580:  symbol MFD_INTEL_LPSS_PCI is implied by 
KEYBOARD_APPLESPI
  drivers/input/keyboard/Kconfig:73:symbol KEYBOARD_APPLESPI depends on 
INPUT
  drivers/input/Kconfig:8:  symbol INPUT is selected by DRM_SIL_SII8620
  drivers/gpu/drm/bridge/Kconfig:83:symbol DRM_SIL_SII8620 depends on 
DRM_BRIDGE
  drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by 
DRM_PL111
  drivers/gpu/drm/pl111/Kconfig:1:  symbol DRM_PL111 depends on COMMON_CLK

According to the docs, select should only be used for non-visible
symbols. Furthermore almost all other references to INPUT throughout the
kernel config are depends, not selects. Hence this change.

CC: Inki Dae 
CC: Andrzej Hajda 
Signed-off-by: Ronald Tschalär 
---
 drivers/gpu/drm/bridge/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 2fee47b0d50b..eabedc83f25c 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -83,9 +83,9 @@ config DRM_PARADE_PS8622
 config DRM_SIL_SII8620
tristate "Silicon Image SII8620 HDMI/MHL bridge"
depends on OF
+   depends on INPUT
select DRM_KMS_HELPER
imply EXTCON
-   select INPUT
select RC_CORE
help
  Silicon Image SII8620 HDMI/MHL bridge chip driver.
-- 
2.20.1

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

Re: [PATCH] fbdev: omap2: no need to check return value of debugfs_create functions

[Tony Lindgren]

* Greg Kroah-Hartman  [190122 15:28]:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.

Acked-by: Tony Lindgren 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 0/5] Add Display support

[Biju Das]

This patch series aims to add display support for iWave G20D-Q7 board based on 
RZ/G1N.

This patch series is tested against linux-next

V1--> V2
* Add DU support : Removed LVDS definition from r8a7743 DU node.
V2--> V3
* Added LVDS support for both r8a7743 and r8a7744 SoC.

Biju Das (5):
  ARM: dts: r8a7743: Convert to new LVDS DT bindings
  dt-bindings: display: renesas: lvds: Document r8a7744 bindings
  drm: rcar-du: lvds: Add r8a7744 support
  ARM: dts: r8a7744: Add DU support
  ARM: dts: r8a7744: Add LVDS support

 .../bindings/display/bridge/renesas,lvds.txt   |  1 +
 arch/arm/boot/dts/r8a7743.dtsi | 36 
 arch/arm/boot/dts/r8a7744.dtsi | 38 --
 drivers/gpu/drm/rcar-du/rcar_lvds.c|  1 +
 4 files changed, 67 insertions(+), 9 deletions(-)

-- 
2.7.4

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

Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap

[Liam Mark]

On Mon, 21 Jan 2019, Brian Starkey wrote:

> Hi,
> 
> Sorry for being a bit sporadic on this. I was out travelling last week
> with little time for email.
> 
> On Fri, Jan 18, 2019 at 11:16:31AM -0600, Andrew F. Davis wrote:
> > On 1/17/19 7:11 PM, Liam Mark wrote:
> > > On Thu, 17 Jan 2019, Andrew F. Davis wrote:
> > > 
> > >> On 1/16/19 4:54 PM, Liam Mark wrote:
> > >>> On Wed, 16 Jan 2019, Andrew F. Davis wrote:
> > >>>
> >  On 1/16/19 9:19 AM, Brian Starkey wrote:
> > > Hi :-)
> > >
> > > On Tue, Jan 15, 2019 at 12:40:16PM -0600, Andrew F. Davis wrote:
> > >> On 1/15/19 12:38 PM, Andrew F. Davis wrote:
> > >>> On 1/15/19 11:45 AM, Liam Mark wrote:
> >  On Tue, 15 Jan 2019, Andrew F. Davis wrote:
> > 
> > > On 1/14/19 11:13 AM, Liam Mark wrote:
> > >> On Fri, 11 Jan 2019, Andrew F. Davis wrote:
> > >>
> > >>> Buffers may not be mapped from the CPU so skip cache 
> > >>> maintenance here.
> > >>> Accesses from the CPU to a cached heap should be bracketed with
> > >>> {begin,end}_cpu_access calls so maintenance should not be 
> > >>> needed anyway.
> > >>>
> > >>> Signed-off-by: Andrew F. Davis 
> > >>> ---
> > >>>  drivers/staging/android/ion/ion.c | 7 ---
> > >>>  1 file changed, 4 insertions(+), 3 deletions(-)
> > >>>
> > >>> diff --git a/drivers/staging/android/ion/ion.c 
> > >>> b/drivers/staging/android/ion/ion.c
> > >>> index 14e48f6eb734..09cb5a8e2b09 100644
> > >>> --- a/drivers/staging/android/ion/ion.c
> > >>> +++ b/drivers/staging/android/ion/ion.c
> > >>> @@ -261,8 +261,8 @@ static struct sg_table 
> > >>> *ion_map_dma_buf(struct dma_buf_attachment *attachment,
> > >>>  
> > >>> table = a->table;
> > >>>  
> > >>> -   if (!dma_map_sg(attachment->dev, table->sgl, 
> > >>> table->nents,
> > >>> -   direction))
> > >>> +   if (!dma_map_sg_attrs(attachment->dev, table->sgl, 
> > >>> table->nents,
> > >>> + direction, 
> > >>> DMA_ATTR_SKIP_CPU_SYNC))
> > >>
> > >> Unfortunately I don't think you can do this for a couple reasons.
> > >> You can't rely on {begin,end}_cpu_access calls to do cache 
> > >> maintenance.
> > >> If the calls to {begin,end}_cpu_access were made before the call 
> > >> to 
> > >> dma_buf_attach then there won't have been a device attached so 
> > >> the calls 
> > >> to {begin,end}_cpu_access won't have done any cache maintenance.
> > >>
> > >
> > > That should be okay though, if you have no attachments (or all
> > > attachments are IO-coherent) then there is no need for cache
> > > maintenance. Unless you mean a sequence where a non-io-coherent 
> > > device
> > > is attached later after data has already been written. Does that
> > > sequence need supporting? 
> > 
> >  Yes, but also I think there are cases where CPU access can happen 
> >  before 
> >  in Android, but I will focus on later for now.
> > 
> > > DMA-BUF doesn't have to allocate the backing
> > > memory until map_dma_buf() time, and that should only happen 
> > > after all
> > > the devices have attached so it can know where to put the buffer. 
> > > So we
> > > shouldn't expect any CPU access to buffers before all the devices 
> > > are
> > > attached and mapped, right?
> > >
> > 
> >  Here is an example where CPU access can happen later in Android.
> > 
> >  Camera device records video -> software post processing -> video 
> >  device 
> >  (who does compression of raw data) and writes to a file
> > 
> >  In this example assume the buffer is cached and the devices are 
> >  not 
> >  IO-coherent (quite common).
> > 
> > >>>
> > >>> This is the start of the problem, having cached mappings of memory 
> > >>> that
> > >>> is also being accessed non-coherently is going to cause issues one 
> > >>> way
> > >>> or another. On top of the speculative cache fills that have to be
> > >>> constantly fought back against with CMOs like below; some coherent
> > >>> interconnects behave badly when you mix coherent and non-coherent 
> > >>> access
> > >>> (snoop filters get messed up).
> > >>>
> > >>> The solution is to either always have the addresses marked 
> > >>> non-coherent
> > >>> (like device memory, no-map carveouts), or if you really want to use
> > >>> regular system memory allocated at runtime, then all cached 
> > >>> mappings of
> > >>> it need to be dropped, ev

[PATCH v3 3/5] drm: rcar-du: lvds: Add r8a7744 support

[Biju Das]

The LVDS encoders on RZ/G1N SoC is similar to RZ/G1M. Add support for
RZ/G1N (R8A7744) SoC to the LVDS encoder driver.

Signed-off-by: Biju Das 
---
 drivers/gpu/drm/rcar-du/rcar_lvds.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/rcar-du/rcar_lvds.c 
b/drivers/gpu/drm/rcar-du/rcar_lvds.c
index 96d749a..caea2c5 100644
--- a/drivers/gpu/drm/rcar-du/rcar_lvds.c
+++ b/drivers/gpu/drm/rcar-du/rcar_lvds.c
@@ -785,6 +785,7 @@ static const struct rcar_lvds_device_info 
rcar_lvds_r8a77995_info = {
 
 static const struct of_device_id rcar_lvds_of_table[] = {
{ .compatible = "renesas,r8a7743-lvds", .data = &rcar_lvds_gen2_info },
+   { .compatible = "renesas,r8a7744-lvds", .data = &rcar_lvds_gen2_info },
{ .compatible = "renesas,r8a774c0-lvds", .data = 
&rcar_lvds_r8a77990_info },
{ .compatible = "renesas,r8a7790-lvds", .data = &rcar_lvds_r8a7790_info 
},
{ .compatible = "renesas,r8a7791-lvds", .data = &rcar_lvds_gen2_info },
-- 
2.7.4

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

Re: [PATCH 4/4] staging: android: ion: Support for mapping with dma mapping attributes

[Liam Mark]

On Mon, 21 Jan 2019, Brian Starkey wrote:

> Hi Liam,
> 
> On Fri, Jan 18, 2019 at 10:37:47AM -0800, Liam Mark wrote:
> > Add support for configuring dma mapping attributes when mapping
> > and unmapping memory through dma_buf_map_attachment and
> > dma_buf_unmap_attachment.
> > 
> > For example this will allow ION clients to skip cache maintenance, by
> > using DMA_ATTR_SKIP_CPU_SYNC, for buffers which are clean and haven't been
> > accessed by the CPU.
> 
> How can a client know that the buffer won't be accessed by the CPU in
> the future though?
> 
Yes, for use cases where you don't if it will be accessed in the future 
then you would only use it to optimize the dma map path, but as I 
mentioned in the other thread there are cases (such as in our Camera) 
where we have complete ownership of buffers and do know if it will be 
accessed in the future.

> I don't think we can push this decision to clients, because they are
> lacking information about what else is going on with the buffer. It
> needs to be done by the exporter, IMO.
> 

I do agree it would be better to handle in the exporter, but in a 
pipelining use case where there might not be any devices attached that 
doesn't seem very doable.

> Thanks,
> -Brian
> 
> > 
> > Signed-off-by: Liam Mark 
> > ---
> >  drivers/staging/android/ion/ion.c | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/staging/android/ion/ion.c 
> > b/drivers/staging/android/ion/ion.c
> > index 1fe633a7fdba..0aae845b20ba 100644
> > --- a/drivers/staging/android/ion/ion.c
> > +++ b/drivers/staging/android/ion/ion.c
> > @@ -268,8 +268,8 @@ static struct sg_table *ion_map_dma_buf(struct 
> > dma_buf_attachment *attachment,
> > table = a->table;
> >  
> > mutex_lock(&buffer->lock);
> > -   if (!dma_map_sg(attachment->dev, table->sgl, table->nents,
> > -   direction)) {
> > +   if (!dma_map_sg_attrs(attachment->dev, table->sgl, table->nents,
> > + direction, attachment->dma_map_attrs)) {
> > mutex_unlock(&buffer->lock);
> > return ERR_PTR(-ENOMEM);
> > }
> > @@ -287,7 +287,8 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment 
> > *attachment,
> > struct ion_buffer *buffer = attachment->dmabuf->priv;
> >  
> > mutex_lock(&buffer->lock);
> > -   dma_unmap_sg(attachment->dev, table->sgl, table->nents, direction);
> > +   dma_unmap_sg_attrs(attachment->dev, table->sgl, table->nents, direction,
> > +  attachment->dma_map_attrs);
> > a->dma_mapped = false;
> > mutex_unlock(&buffer->lock);
> >  }
> > -- 
> > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, 
> > a Linux Foundation Collaborative Project
> > 
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes

[Liam Mark]

On Tue, 22 Jan 2019, Andrew F. Davis wrote:

> On 1/21/19 4:18 PM, Liam Mark wrote:
> > On Mon, 21 Jan 2019, Andrew F. Davis wrote:
> > 
> >> On 1/21/19 2:20 PM, Liam Mark wrote:
> >>> On Mon, 21 Jan 2019, Andrew F. Davis wrote:
> >>>
>  On 1/21/19 1:44 PM, Liam Mark wrote:
> > On Mon, 21 Jan 2019, Christoph Hellwig wrote:
> >
> >> On Sat, Jan 19, 2019 at 08:50:41AM -0800, Laura Abbott wrote:
>  And who is going to decide which ones to pass?  And who documents
>  which ones are safe?
> 
>  I'd much rather have explicit, well documented dma-buf flags that
>  might get translated to the DMA API flags, which are not error 
>  checked,
>  not very well documented and way to easy to get wrong.
> 
> >>>
> >>> I'm not sure having flags in dma-buf really solves anything
> >>> given drivers can use the attributes directly with dma_map
> >>> anyway, which is what we're looking to do. The intention
> >>> is for the driver creating the dma_buf attachment to have
> >>> the knowledge of which flags to use.
> >>
> >> Well, there are very few flags that you can simply use for all calls of
> >> dma_map*.  And given how badly these flags are defined I just don't 
> >> want
> >> people to add more places where they indirectly use these flags, as
> >> it will be more than enough work to clean up the current mess.
> >>
> >> What flag(s) do you want to pass this way, btw?  Maybe that is where
> >> the problem is.
> >>
> >
> > The main use case is for allowing clients to pass in 
> > DMA_ATTR_SKIP_CPU_SYNC in order to skip the default cache maintenance 
> > which happens in dma_buf_map_attachment and dma_buf_unmap_attachment. 
> > In 
> > ION the buffers aren't usually accessed from the CPU so this allows 
> > clients to often avoid doing unnecessary cache maintenance.
> >
> 
>  How can a client know that no CPU access has occurred that needs to be
>  flushed out?
> 
> >>>
> >>> I have left this to clients, but if they own the buffer they can have the 
> >>> knowledge as to whether CPU access is needed in that use case (example 
> >>> for 
> >>> post-processing).
> >>>
> >>> For example with the previous version of ION we left all decisions of 
> >>> whether cache maintenance was required up to the client, they would use 
> >>> the ION cache maintenance IOCTL to force cache maintenance only when it 
> >>> was required.
> >>> In these cases almost all of the access was being done by the device and 
> >>> in the rare cases CPU access was required clients would initiate the 
> >>> required cache maintenance before and after the CPU access.
> >>>
> >>
> >> I think we have different definitions of "client", I'm talking about the
> >> DMA-BUF client (the importer), that is who can set this flag. It seems
> >> you mean the userspace application, which has no control over this flag.
> >>
> > 
> > I am also talking about dma-buf clients, I am referring to both the 
> > userspace and kernel component of the client. For example our Camera ION 
> > client has both a usersapce and kernel component and they have ION 
> > buffers, which they control the access to, which may or may not be 
> > accessed by the CPU in certain uses cases.
> > 
> 
> I know they often work together, but for this discussion it would be
> good to keep kernel clients and usperspace clients separate. There are
> three types of actors at play here, userspace clients, kernel clients,
> and exporters.
> 
> DMA-BUF only provides the basic sync primitive + mmap directly to
> userspace, 

Well dma-buf does provide dma_buf_kmap/dma_buf_begin_cpu_access which 
allows the same fucntionality in the kernel, but I don't think that changes
your argument.

> both operations are fulfilled by the exporter. This patch is
> about adding more control to the kernel side clients. The kernel side
> clients cannot know what userspace or other kernel side clients have
> done with the buffer, *only* the exporter has the whole picture.
> 
> Therefor neither type of client should be deciding if the CPU needs
> flushed or not, only the exporter, based on the type of buffer, the
> current set attachments, and previous actions (is this first attachment,
> CPU get access in-between, etc...) can make this decision.
> 
> You goal seems to be to avoid unneeded CPU side CMOs when a device
> detaches and another attaches with no CPU access in-between, right?
> That's reasonable to me, but it must be the exporter who keeps track and
> skips the CMO. This patch allows the client to tell the exporter the CMO
> is not needed and that is not safe.
> 

I agree it would be better have this logic in the exporter, but I just 
haven't heard an upstreamable way to make that work.
But maybe to explore that a bit more.

If we consider having CPU access with no devices attached a legitimate use 
case:

The pipelining use case I am

Re: [PATCH v9 00/22] Re-use nvram module

[Finn Thain]

On Tue, 22 Jan 2019, Greg Kroah-Hartman wrote:

> On Tue, Jan 15, 2019 at 03:18:56PM +1100, Finn Thain wrote:
> > The "generic" NVRAM module, drivers/char/generic_nvram.c, implements a
> > /dev/nvram misc device. This module is used only by 32-bit PowerPC
> > platforms.
> > 
> > The RTC "CMOS" NVRAM module, drivers/char/nvram.c, also implements a
> > /dev/nvram misc device. This module is now used only by x86 and m68k
> > thanks to commit 3ba9faedc180 ("char: nvram: disable on ARM").
> > 
> > The "generic" module cannot be used by x86 or m68k platforms because it
> > cannot co-exist with the "CMOS" module. One reason for that is the
> > CONFIG_GENERIC_NVRAM kludge in drivers/char/Makefile. Another reason is
> > that automatically loading the appropriate module would be impossible
> > because only one module can provide the char-major-10-144 alias.
> > 
> > A multi-platform kernel binary needs a single, generic module. With this
> > patch series, drivers/char/nvram.c becomes more generic and some of the
> > arch-specific code gets moved under arch/. The nvram module is then
> > usable by all m68k, powerpc and x86 platforms.
> > 
> > This allows for removal of drivers/char/generic_nvram.c as well as a
> > duplicate in arch/powerpc/kernel/nvram_64.c. By reducing the number of
> > /dev/nvram char misc device implementations, the number of bugs and
> > inconsistencies is also reduced.
> > 
> > This approach reduces inconsistencies between PPC32 and PPC64 and also
> > between PPC_PMAC and MAC. A uniform API has benefits for userspace.
> > 
> > For example, some error codes for some ioctl calls become consistent
> > across PowerPC platforms. The uniform API can potentially benefit any
> > bootloader that works across the various platforms having XPRAM
> > (e.g. Emile).
> > 
> > This patch series was tested on Atari, Mac, PowerMac (both 32-bit and
> > 64-bit) and ThinkPad hardware. AFAIK, it has not yet been tested on
> > pSeries or CHRP.
> > 
> > I think there are two possible merge strategies for this patch series.
> > The char misc maintainer could take the entire series. Alternatively,
> > the m68k maintainer could take patches 1 thru 16 (though some of these
> > have nothing to do with m68k) and after those patches reach mainline
> > the powerpc maintainer could take 17 thru 22.
> 
> I just took the whole series, thanks for doing this, looks good.
> 

Thanks, Greg.

I haven't seen any acks from powerpc maintainers yet...

-- 

> greg k-h
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Xen-devel] [PATCH v2] drm/xen-front: Make shmem backed display buffer coherent

[Oleksandr Andrushchenko]

Hello, Julien!

On 1/21/19 7:09 PM, Julien Grall wrote:
> Hello,
>
> On 21/01/2019 12:43, Oleksandr Andrushchenko wrote:
>> On 1/18/19 1:43 PM, Julien Grall wrote:
>>> On 18/01/2019 09:40, Oleksandr Andrushchenko wrote:
 On 1/17/19 11:18 AM, Christoph Hellwig wrote:
> On Wed, Jan 16, 2019 at 06:43:29AM +, Oleksandr Andrushchenko
> wrote:
>>> This whole issue keeps getting more and more confusing.
>> Well, I don't really do DMA here, but instead the buffers in
>> question are shared with other Xen domain, so effectively it
>> could be thought of some sort of DMA here, where the "device" is
>> that remote domain. If the buffers are not flushed then the
>> remote part sees some inconsistency which in my case results
>> in artifacts on screen while displaying the buffers.
>> When buffers are allocated via DMA API then there are no artifacts;
>> if buffers are allocated with shmem + DMA mapping then there are no
>> artifacts as well.
>> The only offending use-case is when I use shmem backed buffers,
>> but do not flush them
> The right answer would be to implement cache maintainance hooks for
> this case in the Xen arch code.  These would basically look the same
> as the low-level cache maintainance used by the DMA ops, but without
> going through the DMA mapping layer, in fact they should probably
> reuse the same low-level assembly routines.
>
> I don't think this is the first usage of such Xen buffer sharing, so
> what do the other users do?
 I'll have to get even deeper into it. Initially I
 looked at the code, but didn't find anything useful.
 Or maybe I have just overlooked obvious things there
>>>  From Xen on Arm ABI:
>>>
>>> "All memory which is shared with other entities in the system
>>> (including the hypervisor and other guests) must reside in memory
>>> which is mapped as Normal Inner Write-Back Outer Write-Back
>>> Inner-Shareable.
>>> This applies to:
>>>    - hypercall arguments passed via a pointer to guest memory.
>>>    - memory shared via the grant table mechanism (including PV I/O
>>>  rings etc).
>>>    - memory shared with the hypervisor (struct shared_info, struct
>>>  vcpu_info, the grant table, etc).
>>> "
>>>
>>> So you should not need any cache maintenance here. Can you provide
>>> more details on the memory attribute you use for memory shared in both
>>> the backend and frontend?
>>>
>> It takes quite some time to collect this (because many components are
>> involved in the
>> use-case), but for now the pages in the guest domain are:
>> !PTE_RDONLY + PTE_PXN + PTE_SHARED + PTE_AF + PTE_UXN +
>> PTE_ATTRINDX(MT_NORMAL)
>
> So that's the attribute for the page mapped in the frontend, right? 
> How about the backend side?
Please see below
>
> Also, could that page be handed to the graphic card correctly?
Yes, if we use zero-copying. But please see below
> If so, is your graphic card coherent?
Yes, it is
>
> If one of your components is mapping with non-cacheable attributes 
> then you are already not following the Xen Arm ABI. In that case, we 
> would need to discuss how to extend the ABI.
>
> Cheers,
>
Well, I didn't get the attributes of pages at the backend side, but IMO 
those
do not matter in my use-case (for simplicity I am not using zero-copying at
backend side):

1. Frontend device allocates display buffer pages which come from shmem
and have these attributes:
!PTE_RDONLY + PTE_PXN + PTE_SHARED + PTE_AF + PTE_UXN + 
PTE_ATTRINDX(MT_NORMAL)

2. Frontend grants references to these pages and shares those with the 
backend

3. Backend is a user-space application (Weston client), so it uses 
gntdev kernel
driver to mmap the pages. The pages, which are used by gntdev, are those 
coming
from the Xen balloon driver and I believe they are all normal memory and
shouldn't be non-cached.

4. Once the frontend starts displaying it flips the buffers and backend 
does *memcpy*
from the frontend-backend shared buffer into Weston's buffer. This means
no HW at the backend side touches the shared buffer.

5. I can see distorted picture.

Previously I used setup with zero-copying, so then the picture becomes 
more complicated
in terms of buffers and how those used by the backed, but anyways it 
seems that the
very basic scenario with memory copying doesn't work for me.

Using DMA API on frontend's side does help - no artifacts are seen.
This is why I'm thinking that this is related to frontend/kernel side 
rather then to
the backend side. This is why I'm thinking this is related to caches and 
trying to figure
out what can be done here instead of using DMA API.

Thank you,
Olkesandr
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Xen-devel] [PATCH v2] drm/xen-front: Make shmem backed display buffer coherent

[Julien Grall]

On 1/22/19 10:28 AM, Oleksandr Andrushchenko wrote:

Hello, Julien!


Hi,


On 1/21/19 7:09 PM, Julien Grall wrote:
Well, I didn't get the attributes of pages at the backend side, but IMO
those
do not matter in my use-case (for simplicity I am not using zero-copying at
backend side):


They are actually important no matter what is your use case. If you 
access the same physical page with different attributes, then you are 
asking for trouble.


This is why Xen imposes all the pages shared to have their memory 
attributes following some rules. Actually, speaking with Mark R., we may 
want to tight a bit more the attributes.




1. Frontend device allocates display buffer pages which come from shmem
and have these attributes:
!PTE_RDONLY + PTE_PXN + PTE_SHARED + PTE_AF + PTE_UXN +
PTE_ATTRINDX(MT_NORMAL)


My knowledge of Xen DRM is inexistent. However, looking at the code in 
5.0-rc2, I don't seem to find the same attributes. For instance 
xen_drm_front_gem_prime_vmap and gem_mmap_obj are using 
pgprot_writecombine. So it looks like, the mapping will be non-cacheable 
on Arm64.


Can you explain how you came up to these attributes?



2. Frontend grants references to these pages and shares those with the
backend

3. Backend is a user-space application (Weston client), so it uses
gntdev kernel
driver to mmap the pages. The pages, which are used by gntdev, are those
coming
from the Xen balloon driver and I believe they are all normal memory and
shouldn't be non-cached.

4. Once the frontend starts displaying it flips the buffers and backend
does *memcpy*
from the frontend-backend shared buffer into Weston's buffer. This means
no HW at the backend side touches the shared buffer.

5. I can see distorted picture.

Previously I used setup with zero-copying, so then the picture becomes
more complicated
in terms of buffers and how those used by the backed, but anyways it
seems that the
very basic scenario with memory copying doesn't work for me.

Using DMA API on frontend's side does help - no artifacts are seen.
This is why I'm thinking that this is related to frontend/kernel side
rather then to
the backend side. This is why I'm thinking this is related to caches and
trying to figure
out what can be done here instead of using DMA API.


We actually never required to use cache flush in other PV protocol, so I 
still don't understand why the PV DRM should be different here.


To me, it looks like that you are either missing some barriers or the 
memory attributes are not correct.


Cheers,

--
Julien Grall
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH] drm: disable WC optimization for cache coherent devices on non-x86

[Ard Biesheuvel]

On Tue, 22 Jan 2019 at 21:56, Alex Deucher  wrote:
>
> On Tue, Jan 22, 2019 at 4:19 AM Ard Biesheuvel
>  wrote:
> >
> > On Mon, 21 Jan 2019 at 20:04, Michel Dänzer  wrote:
> > >
> > > On 2019-01-21 7:28 p.m., Ard Biesheuvel wrote:
> > > > On Mon, 21 Jan 2019 at 19:24, Michel Dänzer  wrote:
> > > >> On 2019-01-21 7:20 p.m., Ard Biesheuvel wrote:
> > > >>> On Mon, 21 Jan 2019 at 19:04, Michel Dänzer  
> > > >>> wrote:
> > >  On 2019-01-21 6:59 p.m., Ard Biesheuvel wrote:
> > > > On Mon, 21 Jan 2019 at 18:55, Michel Dänzer  
> > > > wrote:
> > > >> On 2019-01-21 5:30 p.m., Ard Biesheuvel wrote:
> > > >>> On Mon, 21 Jan 2019 at 17:22, Christoph Hellwig 
> > > >>>  wrote:
> > > >>>
> > >  Until that happens we should just change the driver ifdefs to 
> > >  default
> > >  the hacks to off and only enable them on setups where we 100%
> > >  positively know that they actually work.  And document that fact
> > >  in big fat comments.
> > > >>>
> > > >>> Well, as I mentioned in my commit log as well, if we default to 
> > > >>> off
> > > >>> unless CONFIG_X86, we may break working setups on MIPS and Power 
> > > >>> where
> > > >>> the device is in fact non-cache coherent, and relies on this
> > > >>> 'optimization' to get things working.
> > > >>
> > > >> FWIW, the amdgpu driver doesn't rely on non-snooped transfers for
> > > >> correct basic operation (the scenario Christian brought up is a 
> > > >> very
> > > >> specialized use-case), so that shouldn't be an issue.
> > > >
> > > > The point is that this is only true for x86.
> > > >
> > > > On other architectures, the use of non-cached mappings on the CPU 
> > > > side
> > > > means that you /do/ rely on non-snooped transfers, since if those
> > > > transfers turn out not to snoop inadvertently, the accesses are
> > > > incoherent with the CPU's view of memory.
> > > 
> > >  The driver generally only uses non-cached mappings if
> > >  drm_arch/device_can_wc_memory returns true.
> > > >>>
> > > >>> Indeed. And so we should take care to only return 'true' from that
> > > >>> function if it is guaranteed that non-cached CPU mappings are coherent
> > > >>> with the mappings used by the GPU, either because that is always the
> > > >>> case (like on x86), or because we know that the platform in question
> > > >>> implements NoSnoop correctly throughout the interconnect.
> > > >>>
> > > >>> What seems to be complicating matters is that in some cases, the
> > > >>> device is non-cache coherent to begin with, so regardless of whether
> > > >>> the NoSnoop attribute is used or not, those accesses will not snoop in
> > > >>> the caches and be coherent with the non-cached mappings used by the
> > > >>> CPU. So if we restrict this optimization [on non-X86] to platforms
> > > >>> that are known to implement NoSnoop correctly, we may break platforms
> > > >>> that are implicitly NoSnoop all the time.
> > > >>
> > > >> Since the driver generally doesn't rely on non-snooped accesses for
> > > >> correctness, that couldn't "break" anything that hasn't always been 
> > > >> broken.
> > > >
> > > > Again, that is only true on x86.
> > > >
> > > > On other architectures, DMA writes from the device may allocate in the
> > > > caches, and be invisible to the CPU when it uses non-cached mappings.
> > >
> > > Let me try one last time:
> > >
> > > If drm_arch_can_wc_memory returns false, the driver falls back to the
> > > normal mode of operation, using a cacheable CPU mapping and snooped GPU
> > > transfers, even if userspace asks (as a performance optimization) for a
> > > write-combined CPU mapping and non-snooped GPU transfers via
> > > AMDGPU_GEM_CREATE_CPU_GTT_USWC.
> >
> > Another question: when userspace requests for such a mapping to be
> > created, does this involve pages that are mapped cacheable into the
> > userland process?
>
> AMDGPU_GEM_CREATE_CPU_GTT_USWC means the buffer should be uncached and
> write combined from the CPU's perspective (hence the 'CPU' in the flag
> name).  On the GPU side if that flag is set, we do an non-snooped GPU
> mapping for better performance if the buffer ends up getting mapped
> into the GPU's address space for GPU access.
>

Yes, so much was clear. And the reason this breaks on some arm64
systems is because
a) non-snooped PCIe TLP attributes may be ignored, and
b) non-x86 CPUs do not snoop the caches when accessing uncached mappings.

I don't think there is actually any disagreement on this part. And I
think my patch is reasonable, only Christoph is objecting to it on the
grounds that drivers should not go around the DMA API and create
vmap()s of DMA pages with self chosen attributes.

What I am trying to figure out is why the coherency problem exists:
- it could be that the device is reading using cached mappings and
sees stale clean cachelines that shadow data written by the CPU us

Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes

[Liam Mark]

On Tue, 22 Jan 2019, Andrew F. Davis wrote:

> On 1/21/19 4:12 PM, Liam Mark wrote:
> > On Mon, 21 Jan 2019, Christoph Hellwig wrote:
> > 
> >> On Mon, Jan 21, 2019 at 11:44:10AM -0800, Liam Mark wrote:
> >>> The main use case is for allowing clients to pass in 
> >>> DMA_ATTR_SKIP_CPU_SYNC in order to skip the default cache maintenance 
> >>> which happens in dma_buf_map_attachment and dma_buf_unmap_attachment. In 
> >>> ION the buffers aren't usually accessed from the CPU so this allows 
> >>> clients to often avoid doing unnecessary cache maintenance.
> >>
> >> This can't work.  The cpu can still easily speculate into this area.
> > 
> > Can you provide more detail on your concern here.
> > The use case I am thinking about here is a cached buffer which is accessed 
> > by a non IO-coherent device (quite a common use case for ION).
> > 
> > Guessing on your concern:
> > The speculative access can be an issue if you are going to access the 
> > buffer from the CPU after the device has written to it, however if you 
> > know you aren't going to do any CPU access before the buffer is again 
> > returned to the device then I don't think the speculative access is a 
> > concern.
> > 
> >> Moreover in general these operations should be cheap if the addresses
> >> aren't cached.
> >>
> > 
> > I am thinking of use cases with cached buffers here, so CMO isn't cheap.
> > 
> 
> These buffers are cacheable, not cached, if you haven't written anything
> the data wont actually be in cache. 

That's true

> And in the case of speculative cache
> filling the lines are marked clean. In either case the only cost is the
> little 7 instruction loop calling the clean/invalidate instruction (dc
> civac for ARMv8) for the cache-lines. Unless that is the cost you are
> trying to avoid?
> 

This is the cost I am trying to avoid and this comes back to our previous 
discussion.  We have a coherent system cache so if you are doing this for 
every cache line on a large buffer it adds up with this work and the going 
to the bus.
For example I believe 1080P buffers are 8MB, and 4K buffers are even 
larger.

I also still think you would want to solve this properly such that 
invalidates aren't being done unnecessarily.

> In that case if you are mapping and unmapping so much that the little
> CMO here is hurting performance then I would argue your usage is broken
> and needs to be re-worked a bit.
> 

I am not sure I would say it is broken, the large buffers (example 1080P 
buffers) are mapped and unmapped on every frame. I don't think there is 
any clean way to avoid that in a pipelining framework, you could ask 
clients to keep the buffers dma mapped but there isn't necessarily a good 
time to tell them to unmap. 

It would be unfortunate to not consider this something legitimate for 
usespace to do in a pipelining use case.
Requiring devices to stay attached doesn't seem very clean to me as there 
isn't necessarily a nice place to tell them when to detach.


Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 4/5] ARM: dts: r8a7744: Add DU support

[Biju Das]

Add du node to r8a7744 SoC DT. Boards that want to enable the DU
need to specify the output topology.

Signed-off-by: Biju Das 
---
 arch/arm/boot/dts/r8a7744.dtsi | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7744.dtsi b/arch/arm/boot/dts/r8a7744.dtsi
index 8d25a0a..83804aa 100644
--- a/arch/arm/boot/dts/r8a7744.dtsi
+++ b/arch/arm/boot/dts/r8a7744.dtsi
@@ -1645,8 +1645,14 @@
};
 
du: display@feb0 {
-   reg = <0 0xfeb0 0 0x4>,
- <0 0xfeb9 0 0x1c>;
+   compatible = "renesas,du-r8a7744";
+   reg = <0 0xfeb0 0 0x4>;
+   interrupts = ,
+;
+   clocks = <&cpg CPG_MOD 724>,
+<&cpg CPG_MOD 723>;
+   clock-names = "du.0", "du.1";
+   status = "disabled";
 
ports {
#address-cells = <1>;
@@ -1663,7 +1669,6 @@
};
};
};
-   /* placeholder */
};
 
prr: chipid@ff44 {
-- 
2.7.4

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

[PATCH] drm/vmwgfx: Replace PTR_RET with PTR_ERR_OR_ZERO

[Gustavo A. R. Silva]

PTR_RET is deprecated and will be removed soon.

Use PTR_ERR_OR_ZERO instead.

Notice that these are the last instances of PTR_RET in the
whole codebase.

Signed-off-by: Gustavo A. R. Silva 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
index f2d13a72c05d..137cb1a4d6b0 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c
@@ -2521,7 +2521,7 @@ static int vmw_cmd_dx_clear_rendertarget_view(struct 
vmw_private *dev_priv,
SVGA3dCmdDXClearRenderTargetView body;
} *cmd = container_of(header, typeof(*cmd), header);
 
-   return PTR_RET(vmw_view_id_val_add(sw_context, vmw_view_rt,
+   return PTR_ERR_OR_ZERO(vmw_view_id_val_add(sw_context, vmw_view_rt,
   cmd->body.renderTargetViewId));
 }
 
@@ -2542,7 +2542,7 @@ static int vmw_cmd_dx_clear_depthstencil_view(struct 
vmw_private *dev_priv,
SVGA3dCmdDXClearDepthStencilView body;
} *cmd = container_of(header, typeof(*cmd), header);
 
-   return PTR_RET(vmw_view_id_val_add(sw_context, vmw_view_ds,
+   return PTR_ERR_OR_ZERO(vmw_view_id_val_add(sw_context, vmw_view_ds,
   cmd->body.depthStencilViewId));
 }
 
@@ -2916,7 +2916,7 @@ static int vmw_cmd_dx_genmips(struct vmw_private 
*dev_priv,
SVGA3dCmdDXGenMips body;
} *cmd = container_of(header, typeof(*cmd), header);
 
-   return PTR_RET(vmw_view_id_val_add(sw_context, vmw_view_sr,
+   return PTR_ERR_OR_ZERO(vmw_view_id_val_add(sw_context, vmw_view_sr,
   cmd->body.shaderResourceViewId));
 }
 
-- 
2.20.1

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

Re: [PATCH] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.

[Lukas Wunner]

On Tue, Jan 22, 2019 at 06:13:11AM -0800, Ronald Tschalär wrote:
> commit d6abe6df706c66d803e6dd4fe98c1b6b7f125a56 (drm/bridge:
> sil_sii8620: do not have a dependency of RC_CORE) added a dependency on
> INPUT. However, this causes problems with other drivers, in particular
> an input driver that depends on MFD_INTEL_LPSS_PCI (to be added in a
> future commit):
> 
>   drivers/clk/Kconfig:9:error: recursive dependency detected!
>   drivers/clk/Kconfig:9:symbol COMMON_CLK is selected by 
> MFD_INTEL_LPSS
>   drivers/mfd/Kconfig:566:  symbol MFD_INTEL_LPSS is selected by 
> MFD_INTEL_LPSS_PCI
>   drivers/mfd/Kconfig:580:  symbol MFD_INTEL_LPSS_PCI is implied by 
> KEYBOARD_APPLESPI
>   drivers/input/keyboard/Kconfig:73:symbol KEYBOARD_APPLESPI depends on 
> INPUT
>   drivers/input/Kconfig:8:  symbol INPUT is selected by DRM_SIL_SII8620
>   drivers/gpu/drm/bridge/Kconfig:83:symbol DRM_SIL_SII8620 depends on 
> DRM_BRIDGE
>   drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by 
> DRM_PL111
>   drivers/gpu/drm/pl111/Kconfig:1:  symbol DRM_PL111 depends on COMMON_CLK
> 
> According to the docs, select should only be used for non-visible
> symbols. Furthermore almost all other references to INPUT throughout the
> kernel config are depends, not selects. Hence this change.
> 
> CC: Inki Dae 
> CC: Andrzej Hajda 
> Signed-off-by: Ronald Tschalär 

Reviewed-by: Lukas Wunner 

I think this needs to be merged through the input tree as a prerequisite
for the applespi.c driver (keyboard + touchpad driver for 2015+ MacBook,
MacBook Air and MacBook Pro which uses SPI instead of USB) to avoid
breaking the build.  Adding Dmitry.

Thanks,

Lukas


> ---
>  drivers/gpu/drm/bridge/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 2fee47b0d50b..eabedc83f25c 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -83,9 +83,9 @@ config DRM_PARADE_PS8622
>  config DRM_SIL_SII8620
>   tristate "Silicon Image SII8620 HDMI/MHL bridge"
>   depends on OF
> + depends on INPUT
>   select DRM_KMS_HELPER
>   imply EXTCON
> - select INPUT
>   select RC_CORE
>   help
> Silicon Image SII8620 HDMI/MHL bridge chip driver.
> -- 
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 201273] Fatal error during GPU init amdgpu RX560

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=201273

--- Comment #31 from quirin.blae...@freenet.de ---
Bug is still alive. amd-staging-drm-next
9846725b3f1c54bdc072b42c9b67b8dd6fdf9a90

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] video/fbdev: refactor video= cmdline parsing

[Jani Nikula]

Make the video_setup() function slightly easier to read by removing the
repeated checks for !global. Remove the misleading return value comment
while at it.

I'm slightly hesitant to change any of this, but here goes anyway, with
hopes that the next person to have to look at this has it a wee bit
easier.

Signed-off-by: Jani Nikula 
---
 drivers/video/fbdev/core/fb_cmdline.c | 23 ++-
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/video/fbdev/core/fb_cmdline.c 
b/drivers/video/fbdev/core/fb_cmdline.c
index 39509ccd92f1..3b5bd666b952 100644
--- a/drivers/video/fbdev/core/fb_cmdline.c
+++ b/drivers/video/fbdev/core/fb_cmdline.c
@@ -75,36 +75,33 @@ EXPORT_SYMBOL(fb_get_options);
  * NOTE: This function is a __setup and __init function.
  *It only stores the options.  Drivers have to call
  *fb_get_options() as necessary.
- *
- * Returns zero.
- *
  */
 static int __init video_setup(char *options)
 {
-   int i, global = 0;
-
if (!options || !*options)
-   global = 1;
+   goto out;
 
-   if (!global && !strncmp(options, "ofonly", 6)) {
+   if (!strncmp(options, "ofonly", 6)) {
ofonly = 1;
-   global = 1;
+   goto out;
}
 
-   if (!global && !strchr(options, ':')) {
-   fb_mode_option = options;
-   global = 1;
-   }
+   if (strchr(options, ':')) {
+   /* named */
+   int i;
 
-   if (!global) {
for (i = 0; i < FB_MAX; i++) {
if (video_options[i] == NULL) {
video_options[i] = options;
break;
}
}
+   } else {
+   /* global */
+   fb_mode_option = options;
}
 
+out:
return 1;
 }
 __setup("video=", video_setup);
-- 
2.20.1

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

[PATCH] drm/komeda: Off by one in komeda_fb_get_pixel_addr()

[Dan Carpenter]

The > should be >= to avoid an off by one bug.

Fixes: c46c24bb6b11 ("drm/komeda: Add komeda_framebuffer")
Signed-off-by: Dan Carpenter 
---

I'm 98% sure this is correct, but please review it carefully because I'm
not 100% positive.

 drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
index 4ddd5314ca23..23ee74d42239 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_framebuffer.c
@@ -144,7 +144,7 @@ komeda_fb_get_pixel_addr(struct komeda_fb *kfb, int x, int 
y, int plane)
const struct drm_gem_cma_object *obj;
u32 plane_x, plane_y, cpp, pitch, offset;
 
-   if (plane > fb->format->num_planes) {
+   if (plane >= fb->format->num_planes) {
DRM_DEBUG_KMS("Out of max plane num.\n");
return -EINVAL;
}
-- 
2.17.1

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

[PATCH 1/5] drm/tegra: Store parent pointer in Tegra DRM clients

[Thierry Reding]

From: Thierry Reding 

Tegra DRM clients need access to their parent, so store a pointer to it
upon registration.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/drm.c | 2 ++
 drivers/gpu/drm/tegra/drm.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 4b70ce664c41..61dcbd218ffc 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1041,6 +1041,7 @@ int tegra_drm_register_client(struct tegra_drm *tegra,
 {
mutex_lock(&tegra->clients_lock);
list_add_tail(&client->list, &tegra->clients);
+   client->drm = tegra;
mutex_unlock(&tegra->clients_lock);
 
return 0;
@@ -1051,6 +1052,7 @@ int tegra_drm_unregister_client(struct tegra_drm *tegra,
 {
mutex_lock(&tegra->clients_lock);
list_del_init(&client->list);
+   client->drm = NULL;
mutex_unlock(&tegra->clients_lock);
 
return 0;
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index f1763b4d5b5f..2c809755bca7 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -88,6 +88,7 @@ int tegra_drm_submit(struct tegra_drm_context *context,
 struct tegra_drm_client {
struct host1x_client base;
struct list_head list;
+   struct tegra_drm *drm;
 
unsigned int version;
const struct tegra_drm_client_ops *ops;
-- 
2.19.1

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

[PATCH 2/5] drm/tegra: vic: Load firmware on demand

[Thierry Reding]

From: Thierry Reding 

Loading the firmware requires an allocation of IOVA space to make sure
that the VIC's Falcon microcontroller can read the firmware if address
translation via the SMMU is enabled.

However, the allocation currently happens at a time where the geometry
of an IOMMU domain may not have been initialized yet. This happens for
example on Tegra186 and later where an ARM SMMU is used. Domains which
are created by the ARM SMMU driver postpone the geometry setup until a
device is attached to the domain. This is because IOMMU domains aren't
attached to a specific IOMMU instance at allocation time and hence the
input address space, which defines the geometry, is not known yet.

Work around this by postponing the firmware load until it is needed at
the time where a channel is opened to the VIC. At this time the shared
IOMMU domain's geometry has been properly initialized.

As a byproduct this allows the Tegra DRM to be created in the absence
of VIC firmware, since the VIC initialization no longer fails if the
firmware can't be found.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/vic.c | 17 ++---
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
index d47983deb1cf..afbdc33f49bc 100644
--- a/drivers/gpu/drm/tegra/vic.c
+++ b/drivers/gpu/drm/tegra/vic.c
@@ -181,13 +181,6 @@ static int vic_init(struct host1x_client *client)
vic->domain = tegra->domain;
}
 
-   if (!vic->falcon.data) {
-   vic->falcon.data = tegra;
-   err = falcon_load_firmware(&vic->falcon);
-   if (err < 0)
-   goto detach;
-   }
-
vic->channel = host1x_channel_request(client->dev);
if (!vic->channel) {
err = -ENOMEM;
@@ -256,6 +249,16 @@ static int vic_open_channel(struct tegra_drm_client 
*client,
if (err < 0)
return err;
 
+   if (!vic->falcon.data) {
+   vic->falcon.data = client->drm;
+
+   err = falcon_load_firmware(&vic->falcon);
+   if (err < 0) {
+   pm_runtime_put(vic->dev);
+   return err;
+   }
+   }
+
err = vic_boot(vic);
if (err < 0) {
pm_runtime_put(vic->dev);
-- 
2.19.1

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

[PATCH 3/5] drm/tegra: Setup shared IOMMU domain after initialization

[Thierry Reding]

From: Thierry Reding 

Move initialization of the shared IOMMU domain after the host1x device
has been initialized. At this point all the Tegra DRM clients have been
attached to the shared IOMMU domain.

This is important because Tegra186 and later use an ARM SMMU, for which
the driver defers setting up the geometry for a domain until a device is
attached to it. This is to ensure that the domain is properly set up for
a specific ARM SMMU instance, which is unknown at allocation time.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/drm.c | 54 -
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 61dcbd218ffc..271c7a5fc954 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -92,10 +92,6 @@ static int tegra_drm_load(struct drm_device *drm, unsigned 
long flags)
return -ENOMEM;
 
if (iommu_present(&platform_bus_type)) {
-   u64 carveout_start, carveout_end, gem_start, gem_end;
-   struct iommu_domain_geometry *geometry;
-   unsigned long order;
-
tegra->domain = iommu_domain_alloc(&platform_bus_type);
if (!tegra->domain) {
err = -ENOMEM;
@@ -105,27 +101,6 @@ static int tegra_drm_load(struct drm_device *drm, unsigned 
long flags)
err = iova_cache_get();
if (err < 0)
goto domain;
-
-   geometry = &tegra->domain->geometry;
-   gem_start = geometry->aperture_start;
-   gem_end = geometry->aperture_end - CARVEOUT_SZ;
-   carveout_start = gem_end + 1;
-   carveout_end = geometry->aperture_end;
-
-   order = __ffs(tegra->domain->pgsize_bitmap);
-   init_iova_domain(&tegra->carveout.domain, 1UL << order,
-carveout_start >> order);
-
-   tegra->carveout.shift = iova_shift(&tegra->carveout.domain);
-   tegra->carveout.limit = carveout_end >> tegra->carveout.shift;
-
-   drm_mm_init(&tegra->mm, gem_start, gem_end - gem_start + 1);
-   mutex_init(&tegra->mm_lock);
-
-   DRM_DEBUG("IOMMU apertures:\n");
-   DRM_DEBUG("  GEM: %#llx-%#llx\n", gem_start, gem_end);
-   DRM_DEBUG("  Carveout: %#llx-%#llx\n", carveout_start,
- carveout_end);
}
 
mutex_init(&tegra->clients_lock);
@@ -159,6 +134,35 @@ static int tegra_drm_load(struct drm_device *drm, unsigned 
long flags)
if (err < 0)
goto fbdev;
 
+   if (tegra->domain) {
+   u64 carveout_start, carveout_end, gem_start, gem_end;
+   dma_addr_t start, end;
+   unsigned long order;
+
+   start = tegra->domain->geometry.aperture_start;
+   end = tegra->domain->geometry.aperture_end;
+
+   gem_start = start;
+   gem_end = end - CARVEOUT_SZ;
+   carveout_start = gem_end + 1;
+   carveout_end = end;
+
+   order = __ffs(tegra->domain->pgsize_bitmap);
+   init_iova_domain(&tegra->carveout.domain, 1UL << order,
+carveout_start >> order);
+
+   tegra->carveout.shift = iova_shift(&tegra->carveout.domain);
+   tegra->carveout.limit = carveout_end >> tegra->carveout.shift;
+
+   drm_mm_init(&tegra->mm, gem_start, gem_end - gem_start + 1);
+   mutex_init(&tegra->mm_lock);
+
+   DRM_DEBUG("IOMMU apertures:\n");
+   DRM_DEBUG("  GEM: %#llx-%#llx\n", gem_start, gem_end);
+   DRM_DEBUG("  Carveout: %#llx-%#llx\n", carveout_start,
+ carveout_end);
+   }
+
if (tegra->hub) {
err = tegra_display_hub_prepare(tegra->hub);
if (err < 0)
-- 
2.19.1

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

[PATCH 4/5] drm/tegra: Restrict IOVA space to DMA mask

[Thierry Reding]

From: Thierry Reding 

On Tegra186 and later, the ARM SMMU provides an input address space that
is 48 bits wide. However, memory clients can only address up to 40 bits.
If the geometry is used as-is, allocations of IOVA space can end up in a
region that cannot be addressed by the memory clients.

To fix this, restrict the IOVA space to the DMA mask of the host1x
device. Note that, technically, the IOVA space needs to be restricted to
the intersection of the DMA masks for all clients that are attached to
the IOMMU domain. In practice using the DMA mask of the host1x device is
sufficient because all host1x clients share the same DMA mask.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/drm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 271c7a5fc954..0c5f1e6a0446 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -136,11 +136,12 @@ static int tegra_drm_load(struct drm_device *drm, 
unsigned long flags)
 
if (tegra->domain) {
u64 carveout_start, carveout_end, gem_start, gem_end;
+   u64 dma_mask = dma_get_mask(&device->dev);
dma_addr_t start, end;
unsigned long order;
 
-   start = tegra->domain->geometry.aperture_start;
-   end = tegra->domain->geometry.aperture_end;
+   start = tegra->domain->geometry.aperture_start & dma_mask;
+   end = tegra->domain->geometry.aperture_end & dma_mask;
 
gem_start = start;
gem_end = end - CARVEOUT_SZ;
-- 
2.19.1

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

[PATCH 0/5] drm/tegra: Fix IOVA space on Tegra186 and later

[Thierry Reding]

From: Thierry Reding 

Tegra186 and later are different than earlier generations in that they
use an ARM SMMU rather than the Tegra SMMU. The ARM SMMU driver behaves
slightly differently in that the geometry for IOMMU domains is set only
after a device was attached to it. This is to make sure that the SMMU
instance that the domain belongs to is known, because each instance can
have a different input address space (i.e. geometry).

Work around this by moving all IOVA allocations to a point where the
geometry of the domain is properly initialized.

This supersedes the following patch:

https://patchwork.kernel.org/patch/10775579/

Thierry

Thierry Reding (5):
  drm/tegra: Store parent pointer in Tegra DRM clients
  drm/tegra: vic: Load firmware on demand
  drm/tegra: Setup shared IOMMU domain after initialization
  drm/tegra: Restrict IOVA space to DMA mask
  gpu: host1x: Supports 40-bit addressing on Tegra186

 drivers/gpu/drm/tegra/drm.c | 57 +
 drivers/gpu/drm/tegra/drm.h |  1 +
 drivers/gpu/drm/tegra/vic.c | 17 ++-
 drivers/gpu/host1x/dev.c|  2 +-
 4 files changed, 44 insertions(+), 33 deletions(-)

-- 
2.19.1

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

[PATCH 5/5] gpu: host1x: Supports 40-bit addressing on Tegra186

[Thierry Reding]

From: Thierry Reding 

The host1x and clients instantiated on Tegra186 support addressing 40
bits of memory.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/host1x/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index 419d8929a98f..32fad4da7936 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -127,7 +127,7 @@ static const struct host1x_info host1x06_info = {
.nb_bases = 16,
.init = host1x06_init,
.sync_offset = 0x0,
-   .dma_mask = DMA_BIT_MASK(34),
+   .dma_mask = DMA_BIT_MASK(40),
.has_hypervisor = true,
 };
 
-- 
2.19.1

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

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Brian Starkey]

On Tue, Jan 22, 2019 at 08:19:10PM +0100, Daniel Vetter wrote:
> On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry  
> wrote:
> > On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > forward a lot:
> > >
> > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > >   and a sysroot build (should address all the build/cross platform
> > >   concerns raised in the RFC discussions).
> > >
> > > - tests reorganized into subdirectories so that the i915-gem tests
> > >   don't clog the main/shared tests directory anymore
> > >
> > > - quite a few more non-intel people contributing/reviewing/committing
> > >   igt tests patches.
> > >
> > > I think this addresses all the concerns raised in the RFC discussions,
> > > and assuming there's enough Acks and no new issues that pop up, we can
> > > go ahead with this.
> > >
> > > 1: https://patchwork.kernel.org/patch/10648851/
> > > Cc: Petri Latvala 
> > > Cc: Arkadiusz Hiler 
> > > Cc: Liviu Dudau 
> > > Cc: Sean Paul 
> > > Cc: Eric Anholt 
> > > Cc: Alex Deucher 
> > > Cc: Dave Airlie 
> > > Signed-off-by: Daniel Vetter 
> >
> > I'm all for anything resembling TDD and standardizing on one test 
> > framework. IGT works quite well for us for testing display stuff. We still 
> > have a way to go but have been trying to adopt this requirement lately 
> > anyways for the DC driver. Can't really comment on anything beyond display, 
> > though, for AMD.
> >
> > No matter how much I want this to be mandatory, seeing the discussions with 
> > ARM and the comment about lack of CRC on Nouveau makes me think that we 
> > might not be quite ready to go there. Implementing DWB is non-trivial. VKMS 
> > knows how to compute a CRC from a framebuffer, but that's the trivial part. 
> > Setting up the HW and SW to do DWB is the hard part.
> 
> We also discussed a bit writeback implementations on irc, and it looks
> like you can't really use writeback to accurately test that your
> compositing engine is programmed correctly, since on at least vc4,
> malidp and msm (not yet merged upstream) the writeback engine can't be
> shared with any other outputs, often it even needs a
> dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> That means if you botch your programming and e.g. cause an underrun
> scanning out continous-update outputs, then the writeback won't show
> that to you, since it's composited separately. I guess we could teach
> igt to run these tests on the special crtc->writeback pipeline only,
> but essentially that's a new testcase, and not really testing the
> actual display: It tests writeback, not hdmi/dp/panels/whatever real
> outputs you have.

That doesn't sound right for mali-dp at least. Our writeback is
synchronous with the normal output. If the normal output underruns
then the writeback shows that, and if the writeback overruns, then the
normal output dies too.

Our writeback encoder/connector is a "possible_clone" of the normal
one. So, to integrate it in igt I was intending to set up a cloned
output on the pipe being tested.

Cheers,
-Brian

> 
> I'd say we'll shrug these cases off as "can't be reasonable tested,
> won't have an igt". First driver team with hw that can be validated
> gets to fill the gaps :-) In practice still going to be a lot better
> than no tests at all, just exercising the feature will be useful, and
> will make it a lot easier for the next team to add the crc based tests
> on top.
> 
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Brian Starkey]

On Tue, Jan 22, 2019 at 02:53:37PM -0500, Sean Paul wrote:
> On Tue, Jan 22, 2019 at 07:42:30PM +, Wentland, Harry wrote:
> > 
> > 
> > On 2019-01-22 2:19 p.m., Daniel Vetter wrote:
> > > On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry  
> > > wrote:
> > >> On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> > >>> Compared to the RFC[1] no changes to the patch itself, but igt moved
> > >>> forward a lot:
> > >>>
> > >>> - gitlab CI builds with: reduced configs/libraries, arm cross build
> > >>>   and a sysroot build (should address all the build/cross platform
> > >>>   concerns raised in the RFC discussions).
> > >>>
> > >>> - tests reorganized into subdirectories so that the i915-gem tests
> > >>>   don't clog the main/shared tests directory anymore
> > >>>
> > >>> - quite a few more non-intel people contributing/reviewing/committing
> > >>>   igt tests patches.
> > >>>
> > >>> I think this addresses all the concerns raised in the RFC discussions,
> > >>> and assuming there's enough Acks and no new issues that pop up, we can
> > >>> go ahead with this.
> > >>>
> > >>> 1: https://patchwork.kernel.org/patch/10648851/
> > >>> Cc: Petri Latvala 
> > >>> Cc: Arkadiusz Hiler 
> > >>> Cc: Liviu Dudau 
> > >>> Cc: Sean Paul 
> > >>> Cc: Eric Anholt 
> > >>> Cc: Alex Deucher 
> > >>> Cc: Dave Airlie 
> > >>> Signed-off-by: Daniel Vetter 
> > >>
> > >> I'm all for anything resembling TDD and standardizing on one test 
> > >> framework. IGT works quite well for us for testing display stuff. We 
> > >> still have a way to go but have been trying to adopt this requirement 
> > >> lately anyways for the DC driver. Can't really comment on anything 
> > >> beyond display, though, for AMD.
> > >>
> > >> No matter how much I want this to be mandatory, seeing the discussions 
> > >> with ARM and the comment about lack of CRC on Nouveau makes me think 
> > >> that we might not be quite ready to go there. Implementing DWB is 
> > >> non-trivial. VKMS knows how to compute a CRC from a framebuffer, but 
> > >> that's the trivial part. Setting up the HW and SW to do DWB is the hard 
> > >> part.
> > > 
> > > We also discussed a bit writeback implementations on irc, and it looks
> > > like you can't really use writeback to accurately test that your
> > > compositing engine is programmed correctly, since on at least vc4,
> > > malidp and msm (not yet merged upstream) the writeback engine can't be
> > > shared with any other outputs, often it even needs a
> > > dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> > > That means if you botch your programming and e.g. cause an underrun
> > > scanning out continous-update outputs, then the writeback won't show
> > > that to you, since it's composited separately. I guess we could teach
> > > igt to run these tests on the special crtc->writeback pipeline only,
> > > but essentially that's a new testcase, and not really testing the
> > > actual display: It tests writeback, not hdmi/dp/panels/whatever real
> > > outputs you have.
> > > 
> > > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > > won't have an igt". First driver team with hw that can be validated
> > > gets to fill the gaps :-) In practice still going to be a lot better
> > > than no tests at all, just exercising the feature will be useful, and
> > > will make it a lot easier for the next team to add the crc based tests
> > > on top.
> > > 
> > 
> > I think that's reasonable. After all, we want to start somewhere.
> > 
> > Would it make sense to append something like ", if such a test can be 
> > reasonably made using IGT for the target HW." to make it clear to 
> > contributors that in cases like the one discussed this is at the reviewers 
> > discretion?
> > 
> > With that change (or anything else that clarifies your intentions as 
> > described above) I'd be happy to give my AB.
> 
> Me too
> 
> Sean

That sounds good to me too.

Thanks,
-Brian

> 
> > 
> > Harry
> > 
> > > -Daniel
> > > --
> > > Daniel Vetter
> > > Software Engineer, Intel Corporation
> > > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > > 
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 5/6] arm64: dts: renesas: r8a77990: ebisu: Enable LVDS1 encoder

[Laurent Pinchart]

Hi Simon,

On Wed, Jan 23, 2019 at 09:56:57AM +0100, Simon Horman wrote:
> On Wed, Jan 23, 2019 at 12:54:04AM +0200, Laurent Pinchart wrote:
> > The LVDS1 encoder must supply a pixel clock to the DU for the DPAD
> > output when the LVDS0 encoder is used. Enable it despite its output not
> > being connected.
> > 
> > Signed-off-by: Laurent Pinchart 
> > ---
> > Changes since v1:
> > 
> > - Add a comment in the DT to explain why the LVDS1 encoder needs to be
> >   enabled.
> 
> Thanks,
> 
> This looks fine to me but I will wait to see if there are other reviews
> before applying.
> 
> Reviewed-by: Simon Horman 

Please note that this will likely cause probe failures if applied
without the driver patches from this series. Would it be OK to merge the
DT changes through the DRM tree ?

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Jani Nikula]

On Tue, 22 Jan 2019, "Wentland, Harry"  wrote:
> Would it make sense to append something like ", if such a test can be
> reasonably made using IGT for the target HW." to make it clear to
> contributors that in cases like the one discussed this is at the
> reviewers discretion?

I think the simplest change would be to say API changes SHOULD have
driver-agnostic testcases, with the RFC 2119 meaning of SHOULD:

   SHOULD   This word, or the adjective "RECOMMENDED", mean that there
   may exist valid reasons in particular circumstances to ignore a
   particular item, but the full implications must be understood and
   carefully weighed before choosing a different course.

I.e. s/need/should/. I think it also catches the spirit of the
discussion here; seems like everyone agrees having tests is a good goal.

You'll have to allow for reviewer/maintainer/community discretion no
matter what. Judging by the discussion, CRC based tests don't currently
meet the driver-agnostic requirement. Playing devil's advocate, you
could argue any new APIs couldn't be tested with CRC either, even if it
were the most reasonable approach for i915.


BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Stone]

Hi,

On Tue, 22 Jan 2019 at 19:42, Wentland, Harry  wrote:
> On 2019-01-22 2:19 p.m., Daniel Vetter wrote:
> > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > won't have an igt". First driver team with hw that can be validated
> > gets to fill the gaps :-) In practice still going to be a lot better
> > than no tests at all, just exercising the feature will be useful, and
> > will make it a lot easier for the next team to add the crc based tests
> > on top.
>
> I think that's reasonable. After all, we want to start somewhere.
>
> Would it make sense to append something like ", if such a test can be 
> reasonably made using IGT for the target HW." to make it clear to 
> contributors that in cases like the one discussed this is at the reviewers 
> discretion?
>
> With that change (or anything else that clarifies your intentions as 
> described above) I'd be happy to give my AB.

I could definitely get behind that as well. For what it's worth, we're
(after a couple of stalls due to upstream rewrites) pushing forward
execution of igt within KernelCI. The results are not yet visible
within kernelci.org, but the tests are at least executing and we're
hoping they'll be tracked and visible soon.

If it helps, maybe we could draw up a list somewhere (README in igt?
wiki?) of which tests seem to pass generically across a few drivers,
which ones are expected to pass, which ones don't but really should,
etc. I'm currently running on imx-drm (complicated by hitting a
page-cache BUG!), and a couple of others are getting results from
rockchip, vc4, and msm. Those are pretty well supported and actively
maintained, so should give us a decent first cut at such a list.

Cheers,
Daniel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Jani Nikula]

On Wed, 23 Jan 2019, Daniel Stone  wrote:
> If it helps, maybe we could draw up a list somewhere (README in igt?
> wiki?) of which tests seem to pass generically across a few drivers,
> which ones are expected to pass, which ones don't but really should,
> etc. I'm currently running on imx-drm (complicated by hitting a
> page-cache BUG!), and a couple of others are getting results from
> rockchip, vc4, and msm. Those are pretty well supported and actively
> maintained, so should give us a decent first cut at such a list.

See tests/intel-ci/README. igt supports test lists, we could maintain
similar lists for different needs.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm/rockchip: rgb: update SPDX license identifier

[Sandy Huang]

update SPDX License Identifier from GPL-2.0+ to GPL-2.0
and drop some GPL text.

Signed-off-by: Sandy Huang 
---
 drivers/gpu/drm/rockchip/rockchip_rgb.c | 11 +--
 drivers/gpu/drm/rockchip/rockchip_rgb.h | 11 +--
 2 files changed, 2 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.c 
b/drivers/gpu/drm/rockchip/rockchip_rgb.c
index 96ac145..ad9358f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.c
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.c
@@ -1,17 +1,8 @@
-//SPDX-License-Identifier: GPL-2.0+
+// SPDX-License-Identifier: GPL-2.0
 /*
  * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
  * Author:
  *  Sandy Huang 
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
 #include 
diff --git a/drivers/gpu/drm/rockchip/rockchip_rgb.h 
b/drivers/gpu/drm/rockchip/rockchip_rgb.h
index 38b52e6..27b9635 100644
--- a/drivers/gpu/drm/rockchip/rockchip_rgb.h
+++ b/drivers/gpu/drm/rockchip/rockchip_rgb.h
@@ -1,17 +1,8 @@
-//SPDX-License-Identifier: GPL-2.0+
+/* SPDX-License-Identifier: GPL-2.0 */
 /*
  * Copyright (C) Fuzhou Rockchip Electronics Co.Ltd
  * Author:
  *  Sandy Huang 
- *
- * This software is licensed under the terms of the GNU General Public
- * License version 2, as published by the Free Software Foundation, and
- * may be copied, distributed, and modified under those terms.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
  */
 
 #ifdef CONFIG_ROCKCHIP_RGB
-- 
2.7.4



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

Re: [PATCH 13/15] drm/i915/tv: Generate better pipe timings for TV encoder

[Imre Deak]

On Tue, Jan 22, 2019 at 07:34:55PM +0200, Ville Syrjälä wrote:
> On Tue, Jan 22, 2019 at 07:22:24PM +0200, Imre Deak wrote:
> > On Mon, Nov 12, 2018 at 06:59:58PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä 
> > > 
> > > To make vblank timestamps work better with the TV encoder let's
> > > scale the pipe timings such that the relationship between the
> > > TV active and TV blanking periods is mirrored in the
> > > corresponding pipe timings.
> > > 
> > > Note that in reality the pipe runs at a faster speed during the
> > > TV vblank, and correspondigly there are periods when the pipe
> > > is enitrely stopped. We pretend that this isn't the case and
> > > as such we incur some error in the vblank timestamps during
> > > the TV vblank. Further explanation of the issues in a big
> > > comment in the code.
> > > 
> > > This makes the vblank timestamps good enough to make
> > > i965gm (which doesn't have a working frame counter with
> > > the TV encoder) report correct frame numbers. Previously
> > > you could get all kinds of nonsense which resulted in
> > > eg. glxgears reporting that it's running at twice the
> > > actual framerate in most cases.
> > > 
> > > Signed-off-by: Ville Syrjälä 
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h |   1 +
> > >  drivers/gpu/drm/i915/intel_tv.c | 322 +++-
> > >  2 files changed, 278 insertions(+), 45 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h 
> > > b/drivers/gpu/drm/i915/i915_reg.h
> > > index fe4b913e46ac..10813ae7bee7 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4848,6 +4848,7 @@ enum {
> > >  # define TV_OVERSAMPLE_NONE  (2 << 18)
> > >  /* Selects 8x oversampling */
> > >  # define TV_OVERSAMPLE_8X(3 << 18)
> > > +# define TV_OVERSAMPLE_MASK  (3 << 18)
> > >  /* Selects progressive mode rather than interlaced */
> > >  # define TV_PROGRESSIVE  (1 << 17)
> > >  /* Sets the colorburst to PAL mode.  Required for non-M PAL modes. */
> > > diff --git a/drivers/gpu/drm/i915/intel_tv.c 
> > > b/drivers/gpu/drm/i915/intel_tv.c
> > > index 216525dd144a..75126fce655d 100644
> > > --- a/drivers/gpu/drm/i915/intel_tv.c
> > > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > > @@ -340,7 +340,6 @@ struct tv_mode {
> > >   const struct video_levels *composite_levels, *svideo_levels;
> > >   const struct color_conversion *composite_color, *svideo_color;
> > >   const u32 *filter_table;
> > > - u16 max_srcw;
> > >  };
> > >  
> > >  
> > > @@ -729,7 +728,6 @@ static const struct tv_mode tv_modes[] = {
> > >   .burst_ena  = false,
> > >  
> > >   .filter_table = filter_table,
> > > - .max_srcw = 800
> > >   },
> > >   {
> > >   .name   = "1080i@50Hz",
> > > @@ -947,13 +945,183 @@ intel_tv_mode_vdisplay(const struct tv_mode 
> > > *tv_mode)
> > >   return 2 * (tv_mode->nbr_end + 1);
> > >  }
> > >  
> > > +static void
> > > +intel_tv_mode_to_mode(struct drm_display_mode *mode,
> > > +   const struct tv_mode *tv_mode)
> > > +{
> > > + mode->clock = tv_mode->clock /
> > > + (tv_mode->oversample >> !tv_mode->progressive);
> > > +
> > > + /*
> > > +  * tv_mode horizontal timings:
> > > +  *
> > > +  * hsync_end
> > > +  *| hblank_end
> > > +  *|| hblank_start
> > > +  *||   | htotal
> > > +  *| ___|
> > > +  * /   \___
> > > +  * \__/\
> > > +  */
> > > + mode->hdisplay =
> > > + tv_mode->hblank_start - tv_mode->hblank_end;
> > > + mode->hsync_start = mode->hdisplay +
> > > + tv_mode->htotal - tv_mode->hblank_start;
> > > + mode->hsync_end = mode->hsync_start +
> > > + tv_mode->hsync_end;
> > > + mode->htotal = tv_mode->htotal + 1;
> > > +
> > > + /*
> > > +  * tv_mode vertical timings:
> > > +  *
> > > +  * vsync_start
> > > +  *| vsync_end
> > > +  *|  | vi_end nbr_end
> > > +  *|  ||   |
> > > +  *|  | ___
> > > +  * \__/   \
> > > +  *\__/
> > > +  */
> > > + mode->vdisplay = intel_tv_mode_vdisplay(tv_mode);
> > > + if (tv_mode->progressive) {
> > > + mode->vsync_start = mode->vdisplay +
> > > + tv_mode->vsync_start_f1 + 1;
> > > + mode->vsync_end = mode->vsync_start +
> > > + tv_mode->vsync_len;
> > > + mode->vtotal = mode->vdisplay +
> > > + tv_mode->vi_end_f1 + 1;
> > > + } else {
> > > + mode->vsync_start = mode->vdisplay +
> > > + tv_mode->vsync_start_f1 + 1 +
> > > + tv_mode->vsync_start_f2 + 1;
> > > + mode->vsync_end = mode->vsync_start +
> > > + 2 * tv_mode->vsync_len;
> > > + mode->vtotal = mode->vdisplay +
> > > + tv_mode->vi_end_f1 + 1 +
> > > + tv_mode->vi_end_f2 + 1;
> > > + }
> > > +
> > > + /* TV has it's own notion of 

Re: [PATCH] gpu: ipu-v3: pre: don't trigger update if buffer address doesn't change

[Philipp Zabel]

On Mon, 2019-01-07 at 12:58 +0100, Lucas Stach wrote:
> Am Freitag, den 21.12.2018, 12:11 +0100 schrieb Philipp Zabel:
> > Hi Lucas,
> > 
> > > On Wed, Dec 19, 2018 at 4:40 PM Lucas Stach  
> > > wrote:
> > > 
> > > On a NOP double buffer update where current buffer address is the same
> > > as the next buffer address, the SDW_UPDATE bit clears too late.
> > 
> > What does this mean, exactly? Does the hardware behave differently
> > if the writel to IPU_PRE_NEXT_BUF doesn't change the value?
> 
> To me it seems like that. When the address changes, the SDW_UPDATE bit
> is cleared by the time we handle the EOF IRQ. When the address doesn't
> change, it seems the SDW_UPDATE bit clears much later (I've tried the
> new frame IRQ without any success), maybe only at the next EOF,
> effectively doubling the update period if a plane is flipped with the
> same buffer.

Alright, in that case I think it is correct to carry this workaround in
the PRE driver.

> > > As we
> > > are now using this bit to determine when it is safe to signal flip
> > > completion to userspace this will delay completion of atomic commits
> > > where one plane doesn't change the buffer by a whole frame period.
> > 
> > This sounds like the problem is not the way PRE behaves, but that we are
> > setting the SDW_UPDATE flag again and then later use it to check whether
> > the previous buffer was released, resulting in a false negative.
> > 
> > > Fix this by remembering the last buffer address and just skip the
> > > double buffer update if it would not change the buffer address.
> > 
> > It looks to me like ipu_plane_atomic_update does not have to call
> > ipu_prg_channel_configure (which then just relays to ipu_pre_update)
> > at all in this case. Should we just skip this in ipuv3-plane.c already?
> 
> While we could handle it in ipuv3-plane, this would require more of the
> PRE/PRG state tracking to be moved there. Currently a lot of this is
> hidden behind the simple ipu_prg_channel_configure() call.
> 
> > > Signed-off-by: Lucas Stach 
> > > ---
> > >  drivers/gpu/ipu-v3/ipu-pre.c | 5 +
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/ipu-v3/ipu-pre.c b/drivers/gpu/ipu-v3/ipu-pre.c
> > > index f933c1911e65..d383e8dbd6cc 100644
> > > --- a/drivers/gpu/ipu-v3/ipu-pre.c
> > > +++ b/drivers/gpu/ipu-v3/ipu-pre.c
> > > @@ -106,6 +106,7 @@ struct ipu_pre {
> > > void*buffer_virt;
> > > boolin_use;
> > > unsigned intsafe_window_end;
> > > +   unsigned intlast_bufaddr;
> > 
> > This looks a lot like plane state.
> > 
> > >  };
> > > 
> > >  static DEFINE_MUTEX(ipu_pre_list_mutex);
> > > @@ -242,7 +243,11 @@ void ipu_pre_update(struct ipu_pre *pre, unsigned 
> > > int bufaddr)
> > > unsigned short current_yblock;
> > > u32 val;
> > > 
> > > +   if (bufaddr == pre->last_bufaddr)
> > > +   return;
> > 
> > Hypothetical question, could this wrongly return if the channel is
> > first disabled, leaving
> > last_buffaddr set to X, and then the channel is reenabled, which
> > doesn't touch last_bufaddr,
> > and then the first update contains a buffer at the same address X?
> 
> Nope, this code path is only used when doing no-modeset updates of an
> active plane. When the plane gets disabled the PRE goes through a
> complete reinitialization via ipu_pre_configure() when the channel is
> reenabled.

Let's initialize last_bufaddr in ipu_pre_configure then. I'll amend the
patch as follows:

--8<--
--- a/drivers/gpu/ipu-v3/ipu-pre.c
+++ b/drivers/gpu/ipu-v3/ipu-pre.c
@@ -186,6 +186,7 @@ void ipu_pre_configure(struct ipu_pre *pre, unsigned int 
width,
 
writel(bufaddr, pre->regs + IPU_PRE_CUR_BUF);
writel(bufaddr, pre->regs + IPU_PRE_NEXT_BUF);
+   pre->last_bufaddr = bufaddr;
 
val = IPU_PRE_PREF_ENG_CTRL_INPUT_PIXEL_FORMAT(0) |
  IPU_PRE_PREF_ENG_CTRL_INPUT_ACTIVE_BPP(active_bpp) |
-->8--

regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH libdrm 2/2] xf85drm: de-duplicate drmParse{Platform.Host1x}{Bus, Device}Info

[Emil Velikov]

From: Emil Velikov 

The functions are virtually identical, fold them up.

Signed-off-by: Emil Velikov 
---
 xf86drm.c | 98 +--
 1 file changed, 15 insertions(+), 83 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 374734eb..18c9693a 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3508,7 +3508,7 @@ free_device:
 return ret;
 }
 
-static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr 
info)
+static int drmParseOFBusInfo(int maj, int min, char *fullname)
 {
 #ifdef __linux__
 char path[PATH_MAX + 1], *name, *foo;
@@ -3532,19 +3532,18 @@ static int drmParsePlatformBusInfo(int maj, int min, 
drmPlatformBusInfoPtr info)
 foo++;
 }
 
-strncpy(info->fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
-info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
+strncpy(fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
+fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
 free(name);
 
 return 0;
 #else
-#warning "Missing implementation of drmParsePlatformBusInfo"
+#warning "Missing implementation of drmParseOFBusInfo"
 return -EINVAL;
 #endif
 }
 
-static int drmParsePlatformDeviceInfo(int maj, int min,
-  drmPlatformDeviceInfoPtr info)
+static int drmParseOFDeviceInfo(int maj, int min, char ***compatible)
 {
 #ifdef __linux__
 char path[PATH_MAX + 1], *value, *foo;
@@ -3562,8 +3561,8 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
 count = 1;
 }
 
-info->compatible = calloc(count + 1, sizeof(*info->compatible));
-if (!info->compatible)
+*compatible = calloc(count + 1, sizeof(char *));
+if (!*compatible)
 return -ENOMEM;
 
 for (i = 0; i < count; i++) {
@@ -3587,19 +3586,19 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
 free(value);
 }
 
-info->compatible[i] = foo;
+*compatible[i] = foo;
 }
 
 return 0;
 
 free:
 while (i--)
-free(info->compatible[i]);
+free(*compatible[i]);
 
-free(info->compatible);
+free(*compatible);
 return err;
 #else
-#warning "Missing implementation of drmParsePlatformDeviceInfo"
+#warning "Missing implementation of drmParseOFDeviceInfo"
 return -EINVAL;
 #endif
 }
@@ -3622,7 +3621,7 @@ static int drmProcessPlatformDevice(drmDevicePtr *device,
 
 dev->businfo.platform = (drmPlatformBusInfoPtr)ptr;
 
-ret = drmParsePlatformBusInfo(maj, min, dev->businfo.platform);
+ret = drmParseOFBusInfo(maj, min, dev->businfo.platform->fullname);
 if (ret < 0)
 goto free_device;
 
@@ -3630,7 +3629,7 @@ static int drmProcessPlatformDevice(drmDevicePtr *device,
 ptr += sizeof(drmPlatformBusInfo);
 dev->deviceinfo.platform = (drmPlatformDeviceInfoPtr)ptr;
 
-ret = drmParsePlatformDeviceInfo(maj, min, dev->deviceinfo.platform);
+ret = drmParseOFDeviceInfo(maj, min, 
&dev->deviceinfo.platform->compatible);
 if (ret < 0)
 goto free_device;
 }
@@ -3644,73 +3643,6 @@ free_device:
 return ret;
 }
 
-static int drmParseHost1xBusInfo(int maj, int min, drmHost1xBusInfoPtr info)
-{
-#ifdef __linux__
-char path[PATH_MAX + 1], *name;
-
-snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
-
-name = sysfs_uevent_get(path, "OF_FULLNAME");
-if (!name)
-return -ENOENT;
-
-strncpy(info->fullname, name, DRM_HOST1X_DEVICE_NAME_LEN);
-info->fullname[DRM_HOST1X_DEVICE_NAME_LEN - 1] = '\0';
-free(name);
-
-return 0;
-#else
-#warning "Missing implementation of drmParseHost1xBusInfo"
-return -EINVAL;
-#endif
-}
-
-static int drmParseHost1xDeviceInfo(int maj, int min,
-drmHost1xDeviceInfoPtr info)
-{
-#ifdef __linux__
-char path[PATH_MAX + 1], *value;
-unsigned int count, i;
-int err;
-
-snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
-
-value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
-if (!value)
-return -ENOENT;
-
-sscanf(value, "%u", &count);
-free(value);
-
-info->compatible = calloc(count + 1, sizeof(*info->compatible));
-if (!info->compatible)
-return -ENOMEM;
-
-for (i = 0; i < count; i++) {
-value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
-if (!value) {
-err = -ENOENT;
-goto free;
-}
-
-info->compatible[i] = value;
-}
-
-return 0;
-
-free:
-while (i--)
-free(info->compatible[i]);
-
-free(info->compatible);
-return err;
-#else
-#warning "Missing implementation of drmParseHost1xDeviceInfo"
-return -EINVAL;
-#endif
-}
-
 static int drmProcessHost1xDevice(drmDevicePtr *device,
   const char *node, int node_type,
   int maj, int min, bool fetch_deviceinfo,
@@ -3729,7 +3661,7 @@ static int drmProcessHost1xDevice(drmDevicePtr *

[PATCH libdrm 1/2] xf86drm: fallback to MODALIAS for OF less platform devices

[Emil Velikov]

From: Emil Velikov 

Some devices can lack OF data or it may not be available in the uevent
file. Fallback to the MODALIAS data in those cases.

We strip any leading "MODALIAS=.*:" thus the resulting information is
compatible with existing code in Mesa.

Signed-off-by: Emil Velikov 
---
 xf86drm.c | 55 ++-
 1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/xf86drm.c b/xf86drm.c
index 10df682b..374734eb 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -3511,15 +3511,28 @@ free_device:
 static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr 
info)
 {
 #ifdef __linux__
-char path[PATH_MAX + 1], *name;
+char path[PATH_MAX + 1], *name, *foo;
 
 snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
 
 name = sysfs_uevent_get(path, "OF_FULLNAME");
-if (!name)
-return -ENOENT;
+foo = name;
+if (!name) {
+/* If the device lacks OF data, pick the MODALIAS info */
+name = sysfs_uevent_get(path, "MODALIAS");
+if (!name)
+return -ENOENT;
+
+/* .. and strip the MODALIAS=[platform,usb...]: part. */
+foo = strrchr(name, ':');
+if (!foo) {
+free(name);
+return -ENOENT;
+}
+foo++;
+}
 
-strncpy(info->fullname, name, DRM_PLATFORM_DEVICE_NAME_LEN);
+strncpy(info->fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
 info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
 free(name);
 
@@ -3534,18 +3547,20 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
   drmPlatformDeviceInfoPtr info)
 {
 #ifdef __linux__
-char path[PATH_MAX + 1], *value;
+char path[PATH_MAX + 1], *value, *foo;
 unsigned int count, i;
 int err;
 
 snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
 
 value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
-if (!value)
-return -ENOENT;
-
-sscanf(value, "%u", &count);
-free(value);
+if (value) {
+sscanf(value, "%u", &count);
+free(value);
+} else {
+/* Assume one entry if the device lack OF data */
+count = 1;
+}
 
 info->compatible = calloc(count + 1, sizeof(*info->compatible));
 if (!info->compatible)
@@ -3553,12 +3568,26 @@ static int drmParsePlatformDeviceInfo(int maj, int min,
 
 for (i = 0; i < count; i++) {
 value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
+foo = value;
 if (!value) {
-err = -ENOENT;
-goto free;
+/* If the device lacks OF data, pick the MODALIAS info */
+value = sysfs_uevent_get(path, "MODALIAS");
+if (!value) {
+err = -ENOENT;
+goto free;
+}
+
+/* .. and strip the MODALIAS=[platform,usb...]: part. */
+foo = strrchr(value, ':');
+if (!foo) {
+free(value);
+return -ENOENT;
+}
+foo = strdup(foo + 1);
+free(value);
 }
 
-info->compatible[i] = value;
+info->compatible[i] = foo;
 }
 
 return 0;
-- 
2.20.1

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

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 23, 2019 at 09:40:34AM +, Brian Starkey wrote:
> On Tue, Jan 22, 2019 at 08:19:10PM +0100, Daniel Vetter wrote:
> > On Tue, Jan 22, 2019 at 8:00 PM Wentland, Harry  
> > wrote:
> > > On 2019-01-16 11:39 a.m., Daniel Vetter wrote:
> > > > Compared to the RFC[1] no changes to the patch itself, but igt moved
> > > > forward a lot:
> > > >
> > > > - gitlab CI builds with: reduced configs/libraries, arm cross build
> > > >   and a sysroot build (should address all the build/cross platform
> > > >   concerns raised in the RFC discussions).
> > > >
> > > > - tests reorganized into subdirectories so that the i915-gem tests
> > > >   don't clog the main/shared tests directory anymore
> > > >
> > > > - quite a few more non-intel people contributing/reviewing/committing
> > > >   igt tests patches.
> > > >
> > > > I think this addresses all the concerns raised in the RFC discussions,
> > > > and assuming there's enough Acks and no new issues that pop up, we can
> > > > go ahead with this.
> > > >
> > > > 1: https://patchwork.kernel.org/patch/10648851/
> > > > Cc: Petri Latvala 
> > > > Cc: Arkadiusz Hiler 
> > > > Cc: Liviu Dudau 
> > > > Cc: Sean Paul 
> > > > Cc: Eric Anholt 
> > > > Cc: Alex Deucher 
> > > > Cc: Dave Airlie 
> > > > Signed-off-by: Daniel Vetter 
> > >
> > > I'm all for anything resembling TDD and standardizing on one test 
> > > framework. IGT works quite well for us for testing display stuff. We 
> > > still have a way to go but have been trying to adopt this requirement 
> > > lately anyways for the DC driver. Can't really comment on anything beyond 
> > > display, though, for AMD.
> > >
> > > No matter how much I want this to be mandatory, seeing the discussions 
> > > with ARM and the comment about lack of CRC on Nouveau makes me think that 
> > > we might not be quite ready to go there. Implementing DWB is non-trivial. 
> > > VKMS knows how to compute a CRC from a framebuffer, but that's the 
> > > trivial part. Setting up the HW and SW to do DWB is the hard part.
> > 
> > We also discussed a bit writeback implementations on irc, and it looks
> > like you can't really use writeback to accurately test that your
> > compositing engine is programmed correctly, since on at least vc4,
> > malidp and msm (not yet merged upstream) the writeback engine can't be
> > shared with any other outputs, often it even needs a
> > dedicated/special-purpose CRTC (at least vc4 from what I can tell).
> > That means if you botch your programming and e.g. cause an underrun
> > scanning out continous-update outputs, then the writeback won't show
> > that to you, since it's composited separately. I guess we could teach
> > igt to run these tests on the special crtc->writeback pipeline only,
> > but essentially that's a new testcase, and not really testing the
> > actual display: It tests writeback, not hdmi/dp/panels/whatever real
> > outputs you have.
> 
> That doesn't sound right for mali-dp at least. Our writeback is
> synchronous with the normal output. If the normal output underruns
> then the writeback shows that, and if the writeback overruns, then the
> normal output dies too.
> 
> Our writeback encoder/connector is a "possible_clone" of the normal
> one. So, to integrate it in igt I was intending to set up a cloned
> output on the pipe being tested.

Hm, I grepped for that, didn't find the assignment anywhere. If you can do
possible_clones writeback, then I think doing the writeback-as-fake CRC in
the kernel is probably the best, since it's closest to testing the real
thing. Of course if the writeback connector is already used by igt (for a
writeback test) then the magic "auto" crc tap point would need to fail.
But I don't think that would be a problem for igt, since if you're using
both writeback _and_ crc I have no idea what you're trying to do :-)

I also wouldn't tie this crc writeback into atomic (except with a special
"crc writeback flag" to make sure atomic_check rejects any other use of
the writeback connector while it's used for crc generation), but just a
vblank driven worker that flips between 2 buffers and checksums the other
one.
-Daniel

> 
> Cheers,
> -Brian
> 
> > 
> > I'd say we'll shrug these cases off as "can't be reasonable tested,
> > won't have an igt". First driver team with hw that can be validated
> > gets to fill the gaps :-) In practice still going to be a lot better
> > than no tests at all, just exercising the feature will be useful, and
> > will make it a lot easier for the next team to add the crc based tests
> > on top.
> > 
> > -Daniel
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 23, 2019 at 12:11:36PM +0200, Jani Nikula wrote:
> On Wed, 23 Jan 2019, Daniel Stone  wrote:
> > If it helps, maybe we could draw up a list somewhere (README in igt?
> > wiki?) of which tests seem to pass generically across a few drivers,
> > which ones are expected to pass, which ones don't but really should,
> > etc. I'm currently running on imx-drm (complicated by hitting a
> > page-cache BUG!), and a couple of others are getting results from
> > rockchip, vc4, and msm. Those are pretty well supported and actively
> > maintained, so should give us a decent first cut at such a list.
> 
> See tests/intel-ci/README. igt supports test lists, we could maintain
> similar lists for different needs.

Anything with DRIVER_ANY in it should work. I just noticed that we have a
few generic tests with only DRIVER_INTEL|DRIVER_AMDGPU, those should be
DRIVER_ANY.

I think the best way to get there is to integrate vkms into our CI (I
think that's something we can justify, we'll benefit from the better
overall testsuite ecosystem too), then we'd get test results for every
commit&patch. And would have at least some way to make sure the DRIVER_ANY
tests are somewhat reasonable. Quick discussion with Martin and Arek on
irc says this is doable.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [igt-dev] [PATCH] drm/doc: Make igts for cross-driver stuff mandatory

[Daniel Vetter]

On Wed, Jan 23, 2019 at 12:03:40PM +0200, Jani Nikula wrote:
> On Tue, 22 Jan 2019, "Wentland, Harry"  wrote:
> > Would it make sense to append something like ", if such a test can be
> > reasonably made using IGT for the target HW." to make it clear to
> > contributors that in cases like the one discussed this is at the
> > reviewers discretion?
> 
> I think the simplest change would be to say API changes SHOULD have
> driver-agnostic testcases, with the RFC 2119 meaning of SHOULD:
> 
>SHOULD   This word, or the adjective "RECOMMENDED", mean that there
>may exist valid reasons in particular circumstances to ignore a
>particular item, but the full implications must be understood and
>carefully weighed before choosing a different course.
> 
> I.e. s/need/should/. I think it also catches the spirit of the
> discussion here; seems like everyone agrees having tests is a good goal.
> 
> You'll have to allow for reviewer/maintainer/community discretion no
> matter what. Judging by the discussion, CRC based tests don't currently
> meet the driver-agnostic requirement. Playing devil's advocate, you
> could argue any new APIs couldn't be tested with CRC either, even if it
> were the most reasonable approach for i915.

I think I'll combine both for v3, I wanted to do something like that
anyway to address Eric Anholt's similar concern.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/11] drm: Add devm_drm_dev_init/register

[Noralf Trønnes]

Den 22.01.2019 20.30, skrev Daniel Vetter:
> On Tue, Jan 22, 2019 at 8:07 PM Noralf Trønnes  wrote:
>>
>>
>>
>> Den 22.01.2019 10.32, skrev Daniel Vetter:
>>> On Mon, Jan 21, 2019 at 01:21:46PM +0100, Noralf Trønnes wrote:


 Den 21.01.2019 10.55, skrev Daniel Vetter:
> On Mon, Jan 21, 2019 at 10:10:14AM +0100, Daniel Vetter wrote:
>> On Sun, Jan 20, 2019 at 12:43:08PM +0100, Noralf Trønnes wrote:
>>> This adds resource managed (devres) versions of drm_dev_init() and
>>> drm_dev_register().
>>>
>>> Also added is devm_drm_dev_register_with_fbdev() which sets up generic
>>> fbdev emulation as well.
>>>
>>> devm_drm_dev_register() isn't exported since there are no users.
>>>
>>> Signed-off-by: Noralf Trønnes 
>>

 

>>> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
>>> index 381581b01d48..12129772be45 100644
>>> --- a/drivers/gpu/drm/drm_drv.c
>>> +++ b/drivers/gpu/drm/drm_drv.c
>>> @@ -36,6 +36,7 @@
>>>
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>
>>>  #include "drm_crtc_internal.h"
>>> @@ -871,6 +872,111 @@ void drm_dev_unregister(struct drm_device *dev)
>>>  }
>>>  EXPORT_SYMBOL(drm_dev_unregister);
>>>
>>> +static void devm_drm_dev_init_release(void *data)
>>> +{
>>> + drm_dev_put(data);
>
> We need drm_dev_unplug() here, or this isn't safe.

 This function is only used to cover the error path if probe fails before
 devm_drm_dev_register() is called. devm_drm_dev_register_release() is
 the one that calls unplug. There are comments about this in the functions.
>>>
>>> I think I get a prize for being ignorant and blind :-/
>>>

>
>>> +}
>>> +
>>> +/**
>>> + * devm_drm_dev_init - Resource managed drm_dev_init()
>>> + * @parent: Parent device object
>>> + * @dev: DRM device
>>> + * @driver: DRM driver
>>> + *
>>> + * Managed drm_dev_init(). The DRM device initialized with this 
>>> function is
>>> + * automatically released on driver detach. You must supply a
>
> I think a bit more clarity here would be good:
>
> "... automatically released on driver unbind by callind drm_dev_unplug()."
>
>>> + * &drm_driver.release callback to control the finalization explicitly.
>
> I think a loud warning for these is in order:
>
> "WARNING:
>
> "In generally it is unsafe to use devm functions for drm structures
> because the lifetimes of &drm_device and the underlying &device do not
> match. This here works because it doesn't immediately free anything, but
> only calls drm_dev_unplug(), which internally decrements the &drm_device
> refcount through drm_dev_put().
>
> "All other drm structures must still be explicitly released in the
> &drm_driver.release callback."
>
> While thinking about this I just realized that with this design we have no
> good place to call drm_atomic_helper_shutdown(). Which we need to, or all
> kinds of things will leak badly (connectors, fb, ...), but there's no
> place to call it:
> - unbind is too early, since we haven't yet called drm_dev_unplug, and the
>   drm_dev_unregister in there must be called _before_ we start to shut
>   down anything.
> - drm_driver.release is way too late.
>
> Ofc for a real hotunplug there's no point in shutting down the hw (it's
> already gone), but for a driver unload/unbind it would be nice if this
> happens automatically and in the right order.
>
> So not sure what to do here really.

 How about this change: (it breaks the rule of pulling helpers into the
 core, so maybe we should put the devm_ functions into the simple KMS
 helper instead?)
>>>
>>> Yeah smells a bit much like midlayer ... What would work is having a pile
>>> more devm_ helper functions, so that we onion-unwrap everything correctly,
>>> and in the right order. So:
>>>
>>> - devm_drm_dev_init (always does a drm_dev_put())
>>>
>>> - devm_drm_poll_enable (shuts down the poll helper with a devm action)
>>>
>>> - devm_drm_mode_config_reset (does an atomic_helper_shutdown() as it's 
>>> cleanup action)
>>>
>>> - devm_drm_dev_register (grabs an additional drm_dev_get() reference so it
>>>   can call drm_dev_unplug() unconditionally).
>>>
>>
>> Beautiful! I really like this, it's very flexible.
>>
>> Where should devm_drm_mode_config_reset() live? It will pull in the
>> atomic helper...
> 
> I think a new drm_devm.c helper would be nice for all this stuff.
> Especially since you can't freely mix devm-based setup/cleanup with
> normal cleanup I think it'd be good to have it all together in one
> place. And perhaps even a code example in the DOC: overview.
> 
>>> We'd need to make sure some of the cleanup actions dtrt when the device is
>>> gone, but I think we can achieve that by liberally spri

Re: [PATCH] drm/modes: Prevent division by zero htotal

[Daniel Vetter]

On Wed, Jan 23, 2019 at 03:28:59PM +0800, Tina Zhang wrote:
> This patch prevents division by zero htotal.

How did you manage to get here with htotal == 0? This needs backtraces
(or if this is just about static checkers, a mention of that).
-Daniel

> 
> Signed-off-by: Tina Zhang 
> Cc: Adam Jackson 
> Cc: Dave Airlie 
> Cc: Daniel Vetter 
> ---
>  drivers/gpu/drm/drm_modes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c
> index adce9a2..59b92b1 100644
> --- a/drivers/gpu/drm/drm_modes.c
> +++ b/drivers/gpu/drm/drm_modes.c
> @@ -751,7 +751,7 @@ int drm_mode_hsync(const struct drm_display_mode *mode)
>   if (mode->hsync)
>   return mode->hsync;
>  
> - if (mode->htotal < 0)
> + if (mode->htotal <= 0)
>   return 0;
>  
>   calc_val = (mode->clock * 1000) / mode->htotal; /* hsync in Hz */
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] video/fbdev: refactor video= cmdline parsing

[Daniel Vetter]

On Wed, Jan 23, 2019 at 11:38:17AM +0200, Jani Nikula wrote:
> Make the video_setup() function slightly easier to read by removing the
> repeated checks for !global. Remove the misleading return value comment
> while at it.
> 
> I'm slightly hesitant to change any of this, but here goes anyway, with
> hopes that the next person to have to look at this has it a wee bit
> easier.
> 
> Signed-off-by: Jani Nikula 

Reviewed-by: Daniel Vetter 

> ---
>  drivers/video/fbdev/core/fb_cmdline.c | 23 ++-
>  1 file changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/video/fbdev/core/fb_cmdline.c 
> b/drivers/video/fbdev/core/fb_cmdline.c
> index 39509ccd92f1..3b5bd666b952 100644
> --- a/drivers/video/fbdev/core/fb_cmdline.c
> +++ b/drivers/video/fbdev/core/fb_cmdline.c
> @@ -75,36 +75,33 @@ EXPORT_SYMBOL(fb_get_options);
>   *   NOTE: This function is a __setup and __init function.
>   *It only stores the options.  Drivers have to call
>   *fb_get_options() as necessary.
> - *
> - *   Returns zero.
> - *
>   */
>  static int __init video_setup(char *options)
>  {
> - int i, global = 0;
> -
>   if (!options || !*options)
> - global = 1;
> + goto out;
>  
> - if (!global && !strncmp(options, "ofonly", 6)) {
> + if (!strncmp(options, "ofonly", 6)) {
>   ofonly = 1;
> - global = 1;
> + goto out;
>   }
>  
> - if (!global && !strchr(options, ':')) {
> - fb_mode_option = options;
> - global = 1;
> - }
> + if (strchr(options, ':')) {
> + /* named */
> + int i;
>  
> - if (!global) {
>   for (i = 0; i < FB_MAX; i++) {
>   if (video_options[i] == NULL) {
>   video_options[i] = options;
>   break;
>   }
>   }
> + } else {
> + /* global */
> + fb_mode_option = options;
>   }
>  
> +out:
>   return 1;
>  }
>  __setup("video=", video_setup);
> -- 
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] xf86drm: Add drmIsMaster()

[Daniel Vetter]

On Wed, Jan 23, 2019 at 03:38:45PM +1100, Christopher James Halse Rogers wrote:
> We can't use drmSetMaster to query whether or not a drm fd is master
> because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd.
> 
> Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is
> DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect
> whether or not the fd is master.
> 
> This is useful for code that might get master by open()ing the drm device
> while no other master exists, but can't call drmSetMaster itself because
> it's not running as root or is in a container, where container-root isn't
> real-root.
> 
> v2: Use the AUTH_MAGIC request rather than MODE_ATTACHMODE, as it's more
> clearly related to master status.
> 
> Signed-off-by: Christopher James Halse Rogers 
> 

Reviewed-by: Daniel Vetter 

Can I also motivate you for an igt, to make sure this uapi never breaks
again? I think adding a new subtest to core_auth.c would fit well, which
does:

1. open drm fd, check that his IsMaster function returns true (to avoid
dependencies with unreleased libdrm just make a localdrmIsMaster copy,
we'll collect it eventually).

2. keep the first fd open, open a 2nd fd, check that we're _not_ master
anymore.

3. close both fd.

Also, do you have libdrm commit rights to push this and make a release?

Cheers, Daniel

> ---
>  xf86drm.c | 15 +++
>  xf86drm.h |  2 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 10df682b..adee5bd9 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2741,6 +2741,21 @@ drm_public int drmDropMaster(int fd)
>  return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
>  }
>  
> +drm_public bool drmIsMaster(int fd)
> +{
> +/* Detect master by attempting something that requires master.
> + *
> + * Authenticating magic tokens requires master and 0 is
> + * guaranteed to be an invalid magic number. Attempting this on
> + * a master fd will fail therefore fail with EINVAL because 0 is
> + * invalid.
> + *
> + * A non-master fd will fail with EACCESS, as the kernel checks for
> + * master before attempting to do anything else.
> + */
> +return drmAuthMagic(fd, 0) == EINVAL;
> +}
> +
>  drm_public char *drmGetDeviceNameFromFd(int fd)
>  {
>  char name[128];
> diff --git a/xf86drm.h b/xf86drm.h
> index 7773d71a..9e920db9 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -37,6 +37,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #if defined(__cplusplus)
> @@ -733,6 +734,7 @@ extern void drmMsg(const char *format, ...) 
> DRM_PRINTFLIKE(1, 2);
>  
>  extern int drmSetMaster(int fd);
>  extern int drmDropMaster(int fd);
> +extern bool drmIsMaster(int fd);
>  
>  #define DRM_EVENT_CONTEXT_VERSION 4
>  
> -- 
> 2.19.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PULL] drm-misc-next

[Maxime Ripard]

Hi Dave, Daniel,

Here is the PR for drm-misc-next for this week.

Let me know if there's anything wrong.

Thanks!
Maxime

drm-misc-next-2019-01-23:
drm-misc-next for 5.1:

UAPI Changes:
 - Addition of the Allwinner tiled format modifier

Cross-subsystem Changes:

Core Changes:
 - dma-buf documentation improvements
 - Removal of now unused fbdev helpers
 - Addition of new drm fbdev helpers
 - Improvements to tinydrm
 - Addition of new drm_fourcc helpers
 - Impromevents to i2c-over-aux to handle I2C_M_STOP

Driver Changes:
 - Add support for the TI DS90C185 LVDS bridge
 - Improvements to the thc63lvdm83d bridge
 - Improvements to sun4i YUV and scaler support
 - Fix to the powerdown sequence of panel-innolux
The following changes since commit 94520db52fc0e931327bb77fe79a952a0e9dd2b0:

  drm: fix alpha build after drm_util.h change (2019-01-16 11:16:17 +0100)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2019-01-23

for you to fetch changes up to 46f3ceaffa81e846677bca8668e0ad40e643cffd:

  drm/panel: panel-innolux: set display off in innolux_panel_unprepare 
(2019-01-22 16:49:15 -0500)


drm-misc-next for 5.1:

UAPI Changes:
 - Addition of the Allwinner tiled format modifier

Cross-subsystem Changes:

Core Changes:
 - dma-buf documentation improvements
 - Removal of now unused fbdev helpers
 - Addition of new drm fbdev helpers
 - Improvements to tinydrm
 - Addition of new drm_fourcc helpers
 - Impromevents to i2c-over-aux to handle I2C_M_STOP

Driver Changes:
 - Add support for the TI DS90C185 LVDS bridge
 - Improvements to the thc63lvdm83d bridge
 - Improvements to sun4i YUV and scaler support
 - Fix to the powerdown sequence of panel-innolux


Hsin-Yi, Wang (1):
  drm/panel: panel-innolux: set display off in innolux_panel_unprepare

Jani Nikula (1):
  drm/dp: use DRM_DEBUG_DP() instead of drm_dbg for logging

Jonathan Corbet (1):
  dma-buf: Fix kerneldoc comment for struct dma_fence_array

Maxime Ripard (5):
  drm/sun4i: Move access control before setting the register as documented
  drm/sun4i: frontend: Add a quirk structure
  drm/sun4i: Set the coef_rdy bit right after the coef have been set
  drm/sun4i: Make COEF_RDY conditional
  drm/sun4i: frontend: Move the FIR filter phases to our quirks

Noralf Trønnes (6):
  drm/cma-helper: Remove unused fbdev code
  drm/gem-fb-helper: Add drm_gem_fb_create_with_dirty()
  drm/damage-helper: Add drm_atomic_helper_damage_merged()
  drm/tinydrm: Use struct drm_rect
  drm/tinydrm: Use damage helper for dirtyfb
  drm/todo: Tick off some tinydrm entries

Paul Kocialkowski (18):
  drm/fourcc: Add format info helpers for checking YUV planes disposition
  drm/fourcc: Add format info helpers for checking YUV sub-sampling
  drm/sun4i: backend: Use explicit fourcc helpers for packed YUV422 check
  drm/sun4i: frontend: Pass DRM format info to input format helpers
  drm/sun4i: frontend: Determine input format based on colorspace
  drm/sun4i: Move the BT.601 CSC coefficients to the frontend
  drm/sun4i: frontend: Configure and enable YUV to RGB CSC when needed
  drm/sun4i: frontend: Add support for packed YUV422 input formats
  drm/sun4i: frontend: Add support for semi-planar YUV input formats
  drm/sun4i: frontend: Add support for planar YUV input formats
  drm/fourcc: Add definitions for Allwinner vendor and VPU tiled format
  drm/sun4i: frontend: Add support for tiled YUV input mode configuration
  drm/sun4i: Add buffer stride and offset configuration for tiling mode
  drm/sun4i: frontend: Add and use helper for checking tiling support
  drm/sun4i: layer: Add tiled modifier support and helper
  drm/sun4i: drv: Allow framebuffer modifiers in mode config
  drm/sun4i: frontend: Hook-in support for the A10, with specific quirks
  drm/sun4i: frontend: Hook-in support for the A20

Peter Rosin (5):
  dt-bindings: display: bridge: fork out ti, ds90c185 from lvds-transmitter
  dt-bindings: display: bridge: lvds-transmitter: cleanup example
  dt-bindings: display: bridge: thc63lvdm83d: use standard powerdown-gpios
  drm/bridge: lvds-encoder: add dev helper variable in .probe()
  drm/bridge: lvds-encoder: add powerdown-gpios support

Sam Ravnborg (1):
  drm: fix drm_can_sleep() comment

Ville Syrjälä (1):
  drm/dp: Implement I2C_M_STOP for i2c-over-aux

YueHaibing (1):
  drm/stm: ltdc: remove set but not used variable 'src_h'

 .../bindings/display/bridge/lvds-transmitter.txt   |  12 +-
 .../bindings/display/bridge/thine,thc63lvdm83d.txt |   2 +-
 .../bindings/display/bridge/ti,ds90c185.txt|  55 
 Documentation/gpu/todo.rst |  35 --
 drivers/gpu/drm/Kconfig|   4 -
 drivers/gpu/drm/b

Re: [PATCH libdrm 1/2] xf86drm: fallback to MODALIAS for OF less platform devices

[Eric Engestrom]

On Wednesday, 2019-01-23 10:45:17 +, Emil Velikov wrote:
> From: Emil Velikov 
> 
> Some devices can lack OF data or it may not be available in the uevent
> file. Fallback to the MODALIAS data in those cases.
> 
> We strip any leading "MODALIAS=.*:" thus the resulting information is
> compatible with existing code in Mesa.
> 
> Signed-off-by: Emil Velikov 
> ---
>  xf86drm.c | 55 ++-
>  1 file changed, 42 insertions(+), 13 deletions(-)
> 
> diff --git a/xf86drm.c b/xf86drm.c
> index 10df682b..374734eb 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -3511,15 +3511,28 @@ free_device:
>  static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr 
> info)
>  {
>  #ifdef __linux__
> -char path[PATH_MAX + 1], *name;
> +char path[PATH_MAX + 1], *name, *foo;

I assume you didn't mean to send this patch yet? :P

>  
>  snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
>  
>  name = sysfs_uevent_get(path, "OF_FULLNAME");
> -if (!name)
> -return -ENOENT;
> +foo = name;
> +if (!name) {
> +/* If the device lacks OF data, pick the MODALIAS info */
> +name = sysfs_uevent_get(path, "MODALIAS");
> +if (!name)
> +return -ENOENT;
> +
> +/* .. and strip the MODALIAS=[platform,usb...]: part. */
> +foo = strrchr(name, ':');
> +if (!foo) {
> +free(name);
> +return -ENOENT;
> +}
> +foo++;
> +}
>  
> -strncpy(info->fullname, name, DRM_PLATFORM_DEVICE_NAME_LEN);
> +strncpy(info->fullname, foo, DRM_PLATFORM_DEVICE_NAME_LEN);
>  info->fullname[DRM_PLATFORM_DEVICE_NAME_LEN - 1] = '\0';
>  free(name);
>  
> @@ -3534,18 +3547,20 @@ static int drmParsePlatformDeviceInfo(int maj, int 
> min,
>drmPlatformDeviceInfoPtr info)
>  {
>  #ifdef __linux__
> -char path[PATH_MAX + 1], *value;
> +char path[PATH_MAX + 1], *value, *foo;
>  unsigned int count, i;
>  int err;
>  
>  snprintf(path, sizeof(path), "/sys/dev/char/%d:%d/device", maj, min);
>  
>  value = sysfs_uevent_get(path, "OF_COMPATIBLE_N");
> -if (!value)
> -return -ENOENT;
> -
> -sscanf(value, "%u", &count);
> -free(value);
> +if (value) {
> +sscanf(value, "%u", &count);
> +free(value);
> +} else {
> +/* Assume one entry if the device lack OF data */
> +count = 1;
> +}
>  
>  info->compatible = calloc(count + 1, sizeof(*info->compatible));
>  if (!info->compatible)
> @@ -3553,12 +3568,26 @@ static int drmParsePlatformDeviceInfo(int maj, int 
> min,
>  
>  for (i = 0; i < count; i++) {
>  value = sysfs_uevent_get(path, "OF_COMPATIBLE_%u", i);
> +foo = value;
>  if (!value) {
> -err = -ENOENT;
> -goto free;
> +/* If the device lacks OF data, pick the MODALIAS info */
> +value = sysfs_uevent_get(path, "MODALIAS");
> +if (!value) {
> +err = -ENOENT;
> +goto free;
> +}
> +
> +/* .. and strip the MODALIAS=[platform,usb...]: part. */
> +foo = strrchr(value, ':');
> +if (!foo) {
> +free(value);
> +return -ENOENT;
> +}
> +foo = strdup(foo + 1);
> +free(value);
>  }
>  
> -info->compatible[i] = value;
> +info->compatible[i] = foo;
>  }
>  
>  return 0;
> -- 
> 2.20.1
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] video/fbdev: refactor video= cmdline parsing

[Jani Nikula]

On Wed, 23 Jan 2019, Daniel Vetter  wrote:
> On Wed, Jan 23, 2019 at 11:38:17AM +0200, Jani Nikula wrote:
>> Make the video_setup() function slightly easier to read by removing the
>> repeated checks for !global. Remove the misleading return value comment
>> while at it.
>> 
>> I'm slightly hesitant to change any of this, but here goes anyway, with
>> hopes that the next person to have to look at this has it a wee bit
>> easier.
>> 
>> Signed-off-by: Jani Nikula 
>
> Reviewed-by: Daniel Vetter 

Thanks.

Just to be clear, I expect Bartlomiej to queue this via the fb tree
(provided he agrees with the change, of course).

BR,
Jani.


>
>> ---
>>  drivers/video/fbdev/core/fb_cmdline.c | 23 ++-
>>  1 file changed, 10 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/video/fbdev/core/fb_cmdline.c 
>> b/drivers/video/fbdev/core/fb_cmdline.c
>> index 39509ccd92f1..3b5bd666b952 100644
>> --- a/drivers/video/fbdev/core/fb_cmdline.c
>> +++ b/drivers/video/fbdev/core/fb_cmdline.c
>> @@ -75,36 +75,33 @@ EXPORT_SYMBOL(fb_get_options);
>>   *  NOTE: This function is a __setup and __init function.
>>   *It only stores the options.  Drivers have to call
>>   *fb_get_options() as necessary.
>> - *
>> - *  Returns zero.
>> - *
>>   */
>>  static int __init video_setup(char *options)
>>  {
>> -int i, global = 0;
>> -
>>  if (!options || !*options)
>> -global = 1;
>> +goto out;
>>  
>> -if (!global && !strncmp(options, "ofonly", 6)) {
>> +if (!strncmp(options, "ofonly", 6)) {
>>  ofonly = 1;
>> -global = 1;
>> +goto out;
>>  }
>>  
>> -if (!global && !strchr(options, ':')) {
>> -fb_mode_option = options;
>> -global = 1;
>> -}
>> +if (strchr(options, ':')) {
>> +/* named */
>> +int i;
>>  
>> -if (!global) {
>>  for (i = 0; i < FB_MAX; i++) {
>>  if (video_options[i] == NULL) {
>>  video_options[i] = options;
>>  break;
>>  }
>>  }
>> +} else {
>> +/* global */
>> +fb_mode_option = options;
>>  }
>>  
>> +out:
>>  return 1;
>>  }
>>  __setup("video=", video_setup);
>> -- 
>> 2.20.1
>> 
>> ___
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[v1] drm/msm: Remove clock and bandwidth votes in mdss pm suspend

[Jayant Shekhar]

MDSS PM suspend is dependent on runtime suspend for disabling
clocks and removing bandwidth votes. In case of pm_suspend
triggered, dpm_prepare hold a refcount on power usage of device
and hence runtime suspend is never triggered during pm_suspend.
As runtime suspend is not triggered, clocks and bandwidth votes
remain. Hence explicitly trigger mdss disable in msm_pm_suspend
to disable clocks and remove the votes.

Signed-off-by: Jayant Shekhar 
---
 drivers/gpu/drm/msm/msm_drv.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 5c60bb3..ffe3a25 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -1068,12 +1068,16 @@ static int msm_pm_suspend(struct device *dev)
 {
struct drm_device *ddev = dev_get_drvdata(dev);
struct msm_drm_private *priv = ddev->dev_private;
+   struct msm_mdss *mdss = priv->mdss;
 
if (!IS_ERR_OR_NULL(priv->pm_state))
return 0;
 
priv->pm_state = drm_atomic_helper_suspend(ddev);
 
+   if (mdss && mdss->funcs)
+   mdss->funcs->disable(mdss);
+
return IS_ERR(priv->pm_state) ? PTR_ERR(priv->pm_state) : 0;
 }
 
@@ -1081,11 +1085,15 @@ static int msm_pm_resume(struct device *dev)
 {
struct drm_device *ddev = dev_get_drvdata(dev);
struct msm_drm_private *priv = ddev->dev_private;
+   struct msm_mdss *mdss = priv->mdss;
int ret;
 
if (IS_ERR_OR_NULL(priv->pm_state))
return 0;
 
+   if (mdss && mdss->funcs)
+   mdss->funcs->enable(mdss);
+
ret = drm_atomic_helper_resume(ddev, priv->pm_state);
if (ret == 0)
priv->pm_state = NULL;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[Bug 201795] [Regression] Wrong 4k resolution detected with DisplayPort to HDMI adapter on amdgpu

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=201795

tempel.jul...@gmail.com changed:

   What|Removed |Added

 CC||tempel.jul...@gmail.com

--- Comment #13 from tempel.jul...@gmail.com ---
As a workaround, I'd give forcing the correct edid for each display a try:
https://wiki.archlinux.org/index.php/kernel_mode_setting#Forcing_modes_and_EDID
And maybe also edit the edid to offer only one correct native mode you want to
use.
Editing edids on Linux might be painful, so perhaps do it on Windows.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH libdrm 1/2] xf86drm: fallback to MODALIAS for OF less platform devices

[Emil Velikov]

On Wed, 23 Jan 2019 at 11:04, Eric Engestrom  wrote:
>
> On Wednesday, 2019-01-23 10:45:17 +, Emil Velikov wrote:
> > From: Emil Velikov 
> >
> > Some devices can lack OF data or it may not be available in the uevent
> > file. Fallback to the MODALIAS data in those cases.
> >
> > We strip any leading "MODALIAS=.*:" thus the resulting information is
> > compatible with existing code in Mesa.
> >
> > Signed-off-by: Emil Velikov 
> > ---
> >  xf86drm.c | 55 ++-
> >  1 file changed, 42 insertions(+), 13 deletions(-)
> >
> > diff --git a/xf86drm.c b/xf86drm.c
> > index 10df682b..374734eb 100644
> > --- a/xf86drm.c
> > +++ b/xf86drm.c
> > @@ -3511,15 +3511,28 @@ free_device:
> >  static int drmParsePlatformBusInfo(int maj, int min, drmPlatformBusInfoPtr 
> > info)
> >  {
> >  #ifdef __linux__
> > -char path[PATH_MAX + 1], *name;
> > +char path[PATH_MAX + 1], *name, *foo;
>
> I assume you didn't mean to send this patch yet? :P
>
Thanks Eric, I intentionally sent it out. Mind was blank thinking for
a reasonable variable name :-\
Suggestions are more than welcome.

For reference with this patch drmdevice and other drmDevice API users list:
 - VGEM, needs "drm/vgem: Fix vgem_init to get drm device available."
- in v5.0 only :'-(
 - etnaviv, after "drm/etnaviv: remove the need for a gpu-subsystem DT
node" landed in v4.17/18

HTH
Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] gpu: host1x: Represent host1x bus devices in debugfs

[Thierry Reding]

From: Thierry Reding 

This new debugfs file represents the state of host1x bus devices,
specifying the list of subdevices and marking which ones have
successfully registered.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/host1x/bus.c | 35 +++
 1 file changed, 35 insertions(+)

diff --git a/drivers/gpu/host1x/bus.c b/drivers/gpu/host1x/bus.c
index b4c385d4a6af..103fffc1904b 100644
--- a/drivers/gpu/host1x/bus.c
+++ b/drivers/gpu/host1x/bus.c
@@ -15,8 +15,10 @@
  * along with this program.  If not, see .
  */
 
+#include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -500,6 +502,36 @@ static void host1x_detach_driver(struct host1x *host1x,
mutex_unlock(&host1x->devices_lock);
 }
 
+static int host1x_devices_show(struct seq_file *s, void *data)
+{
+   struct host1x *host1x = s->private;
+   struct host1x_device *device;
+
+   mutex_lock(&host1x->devices_lock);
+
+   list_for_each_entry(device, &host1x->devices, list) {
+   struct host1x_subdev *subdev;
+
+   seq_printf(s, "%s\n", dev_name(&device->dev));
+
+   mutex_lock(&device->subdevs_lock);
+
+   list_for_each_entry(subdev, &device->active, list)
+   seq_printf(s, "  %pOFf: %s\n", subdev->np,
+  dev_name(subdev->client->dev));
+
+   list_for_each_entry(subdev, &device->subdevs, list)
+   seq_printf(s, "  %pOFf:\n", subdev->np);
+
+   mutex_unlock(&device->subdevs_lock);
+   }
+
+   mutex_unlock(&host1x->devices_lock);
+
+   return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(host1x_devices);
+
 /**
  * host1x_register() - register a host1x controller
  * @host1x: host1x controller
@@ -523,6 +555,9 @@ int host1x_register(struct host1x *host1x)
 
mutex_unlock(&drivers_lock);
 
+   debugfs_create_file("devices", S_IRUGO, host1x->debugfs, host1x,
+   &host1x_devices_fops);
+
return 0;
 }
 
-- 
2.19.1

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

Re: [PATCH 4/4] drm/imx: only send commit done event when all state has been applied

[Philipp Zabel]

On Fri, 2018-10-05 at 17:11 +0200, Philipp Zabel wrote:
> On Fri, 2018-09-14 at 18:59 +0200, Lucas Stach wrote:
> > Currently there is a small race window where we could manage to arm the
> > vblank event from atomic flush, but programming the hardware was too close
> > to the frame end, so the hardware will only apply the current state on the
> > next vblank. In this case we will send out the commit done event too early
> > causing userspace to reuse framebuffes that are still in use.
> > 
> > Instead of using the event arming mechnism, just remember the pending event
> > and send it from the vblank IRQ handler, once we are sure that all state
> > has been applied sucessfully.
> > 
> > Signed-off-by: Lucas Stach 
> > ---
> >  drivers/gpu/drm/imx/ipuv3-crtc.c | 32 
> >  1 file changed, 28 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c 
> > b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > index 7d4b710b837a..b0c95565a28d 100644
> > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > @@ -42,6 +42,7 @@ struct ipu_crtc {
> > struct ipu_dc   *dc;
> > struct ipu_di   *di;
> > int irq;
> > +   struct drm_pending_vblank_event *event;
> >  };
> >  
> >  static inline struct ipu_crtc *to_ipu_crtc(struct drm_crtc *crtc)
> > @@ -181,8 +182,31 @@ static const struct drm_crtc_funcs ipu_crtc_funcs = {
> >  static irqreturn_t ipu_irq_handler(int irq, void *dev_id)
> >  {
> > struct ipu_crtc *ipu_crtc = dev_id;
> > +   struct drm_crtc *crtc = &ipu_crtc->base;
> > +   unsigned long flags;
> > +   int i;
> > +
> > +   drm_crtc_handle_vblank(crtc);
> > +
> > +   if (ipu_crtc->event) {
> > +   for (i = 0; i < ARRAY_SIZE(ipu_crtc->plane); i++) {
> > +   struct ipu_plane *plane = ipu_crtc->plane[i];
> > +
> > +   if (!plane)
> > +   continue;
> > +
> > +   if (!ipu_plane_atomic_update_done(&plane->base))
> 
>   if (ipu_plane_atomic_update_pending(&plane->base))
> 
> > +   break;
> > +   }
> >  
> > -   drm_crtc_handle_vblank(&ipu_crtc->base);
> > +   if (i == ARRAY_SIZE(ipu_crtc->plane)) {
> > +   spin_lock_irqsave(&crtc->dev->event_lock, flags);
> > +   drm_crtc_send_vblank_event(crtc, ipu_crtc->event);
> > +   ipu_crtc->event = NULL;
> 
> These two happen under the event spinlock, but where event is set in
> ipu_crtc_atomic_flush, the locking is removed.
> 
> > +   drm_crtc_vblank_put(crtc);
> > +   spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> > +   }
> > +   }
> >  
> > return IRQ_HANDLED;
> >  }
> > @@ -229,13 +253,13 @@ static void ipu_crtc_atomic_begin(struct drm_crtc 
> > *crtc,
> >  static void ipu_crtc_atomic_flush(struct drm_crtc *crtc,
> >   struct drm_crtc_state *old_crtc_state)
> >  {
> > -   spin_lock_irq(&crtc->dev->event_lock);
> > +   struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
> > +
> > if (crtc->state->event) {
> > WARN_ON(drm_crtc_vblank_get(crtc));
> > -   drm_crtc_arm_vblank_event(crtc, crtc->state->event);
> > +   ipu_crtc->event = crtc->state->event;
> 
> We assume here that ipu_crtc->event is NULL and the irq handler is never
> running at the same time, otherwise we would drop an event. This is non-
> obvious to me, and I think it warrants a comment.
> 
> My understanding is the following:
> 
> - It is virtually impossible for atomic_flush to race against a delayed
>   previous ipu_irq_handler because the previous commit's commit_tail
>   would still be waiting for the vblank event to release it from
>   drm_atomic_helper_wait_for_flip_done.
> 
>   However, if the last commit's tail finishes after the irq_handler
>   calls drm_crtc_send_vblank_event(), and the new commit is issued, and
>   its tail work scheduled, all before the next line in the irq_handler,
>   ipu_crtc->event = NULL, then the new commit's tail could call
>   drm_atomic_helper_commit_planes and therefore ipu_crtc_atomic_flush
>   and ipu_crtc->event would be overwritten.
> 
> - It is unproblematic for a delayed atomic_flush to race against the
>   next ipu_irq_handler because ipu_crtc->event will just not be set
>   when the irq handler checks it, and the vblank event will be deferred
>   to the next interrupt.

How do we proceed with this? Keep the spin lock?

regards
Philipp
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/3] treewide: Lift switch variables out of switches

[Greg KH]

On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
> 
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed 
> [-Wswitch-unreachable]
>siginfo_t si;
>  ^~

That's a pain, so this means we can't have any new variables in { }
scope except for at the top of a function?

That's going to be a hard thing to keep from happening over time, as
this is valid C :(

greg k-h
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2 0/7] drm/bridge: tc358767: small fixes

[Tomi Valkeinen]

Hi Andrzej,

On 09/01/19 12:12, Andrzej Hajda wrote:
> On 09.01.2019 10:51, Lucas Stach wrote:
>> Am Mittwoch, den 09.01.2019, 11:12 +0200 schrieb Tomi Valkeinen:
>>> Hi Andrzej,
>>>
>>> On 09/01/19 10:22, Andrzej Hajda wrote:
 Hi Tomi,

 On 03.01.2019 12:59, Tomi Valkeinen wrote:
> Hi,
>
> We have TC358867 on our board, which I believe is almost identical to
> TC358767. We're using it with a DP connector instead of eDP with a fixed
> panel.
>
> I have tested these patches only on TI's 4.14 based kernel, as
> unfortunately we don't have all the necessary support in mainline yet.
> These patches fix various bugs, but I'm still seeing at least two
> issues:
>
> * Sync with some videomodes is not correct, resulting in a jumping and
>   skewed display
> * Link training fails sometimes
>
> I would appreciate if someone is able to verify these patches with
> TC358767.

 Do you want to wait for testers or shall I queue this patchset?
>>> I haven't heard from anyone, so I'm ok with pushing these.
>> For the series:
>>
>> Tested-by: Lucas Stach 
>>
>> on a device with TC358767 and a 4.20 based kernel.
> 
> 
> Already queued :)

Did you push these somewhere? What's the route for these patches, drm-misc?

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/4] drm/imx: only send commit done event when all state has been applied

[Daniel Vetter]

On Wed, Jan 23, 2019 at 12:35:02PM +0100, Philipp Zabel wrote:
> On Fri, 2018-10-05 at 17:11 +0200, Philipp Zabel wrote:
> > On Fri, 2018-09-14 at 18:59 +0200, Lucas Stach wrote:
> > > Currently there is a small race window where we could manage to arm the
> > > vblank event from atomic flush, but programming the hardware was too close
> > > to the frame end, so the hardware will only apply the current state on the
> > > next vblank. In this case we will send out the commit done event too early
> > > causing userspace to reuse framebuffes that are still in use.
> > > 
> > > Instead of using the event arming mechnism, just remember the pending 
> > > event
> > > and send it from the vblank IRQ handler, once we are sure that all state
> > > has been applied sucessfully.
> > > 
> > > Signed-off-by: Lucas Stach 
> > > ---
> > >  drivers/gpu/drm/imx/ipuv3-crtc.c | 32 
> > >  1 file changed, 28 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/imx/ipuv3-crtc.c 
> > > b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > index 7d4b710b837a..b0c95565a28d 100644
> > > --- a/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > +++ b/drivers/gpu/drm/imx/ipuv3-crtc.c
> > > @@ -42,6 +42,7 @@ struct ipu_crtc {
> > >   struct ipu_dc   *dc;
> > >   struct ipu_di   *di;
> > >   int irq;
> > > + struct drm_pending_vblank_event *event;
> > >  };
> > >  
> > >  static inline struct ipu_crtc *to_ipu_crtc(struct drm_crtc *crtc)
> > > @@ -181,8 +182,31 @@ static const struct drm_crtc_funcs ipu_crtc_funcs = {
> > >  static irqreturn_t ipu_irq_handler(int irq, void *dev_id)
> > >  {
> > >   struct ipu_crtc *ipu_crtc = dev_id;
> > > + struct drm_crtc *crtc = &ipu_crtc->base;
> > > + unsigned long flags;
> > > + int i;
> > > +
> > > + drm_crtc_handle_vblank(crtc);
> > > +
> > > + if (ipu_crtc->event) {
> > > + for (i = 0; i < ARRAY_SIZE(ipu_crtc->plane); i++) {
> > > + struct ipu_plane *plane = ipu_crtc->plane[i];
> > > +
> > > + if (!plane)
> > > + continue;
> > > +
> > > + if (!ipu_plane_atomic_update_done(&plane->base))
> > 
> > if (ipu_plane_atomic_update_pending(&plane->base))
> > 
> > > + break;
> > > + }
> > >  
> > > - drm_crtc_handle_vblank(&ipu_crtc->base);
> > > + if (i == ARRAY_SIZE(ipu_crtc->plane)) {
> > > + spin_lock_irqsave(&crtc->dev->event_lock, flags);
> > > + drm_crtc_send_vblank_event(crtc, ipu_crtc->event);
> > > + ipu_crtc->event = NULL;
> > 
> > These two happen under the event spinlock, but where event is set in
> > ipu_crtc_atomic_flush, the locking is removed.
> > 
> > > + drm_crtc_vblank_put(crtc);
> > > + spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
> > > + }
> > > + }
> > >  
> > >   return IRQ_HANDLED;
> > >  }
> > > @@ -229,13 +253,13 @@ static void ipu_crtc_atomic_begin(struct drm_crtc 
> > > *crtc,
> > >  static void ipu_crtc_atomic_flush(struct drm_crtc *crtc,
> > > struct drm_crtc_state *old_crtc_state)
> > >  {
> > > - spin_lock_irq(&crtc->dev->event_lock);
> > > + struct ipu_crtc *ipu_crtc = to_ipu_crtc(crtc);
> > > +
> > >   if (crtc->state->event) {
> > >   WARN_ON(drm_crtc_vblank_get(crtc));
> > > - drm_crtc_arm_vblank_event(crtc, crtc->state->event);
> > > + ipu_crtc->event = crtc->state->event;
> > 
> > We assume here that ipu_crtc->event is NULL and the irq handler is never
> > running at the same time, otherwise we would drop an event. This is non-
> > obvious to me, and I think it warrants a comment.
> > 
> > My understanding is the following:
> > 
> > - It is virtually impossible for atomic_flush to race against a delayed
> >   previous ipu_irq_handler because the previous commit's commit_tail
> >   would still be waiting for the vblank event to release it from
> >   drm_atomic_helper_wait_for_flip_done.
> > 
> >   However, if the last commit's tail finishes after the irq_handler
> >   calls drm_crtc_send_vblank_event(), and the new commit is issued, and
> >   its tail work scheduled, all before the next line in the irq_handler,
> >   ipu_crtc->event = NULL, then the new commit's tail could call
> >   drm_atomic_helper_commit_planes and therefore ipu_crtc_atomic_flush
> >   and ipu_crtc->event would be overwritten.
> > 
> > - It is unproblematic for a delayed atomic_flush to race against the
> >   next ipu_irq_handler because ipu_crtc->event will just not be set
> >   when the irq handler checks it, and the vblank event will be deferred
> >   to the next interrupt.
> 
> How do we proceed with this? Keep the spin lock?

Yeah, standard practice is to protect these things with a spinlock,
usually the drm->event_lock. Then the flip_done wait should make sure
overall ordering is correct, too.

Might be good to improve the kerneldocs that this is

[PATCH] drm/tegra: vic: Do not clear driver data

[Thierry Reding]

From: Thierry Reding 

Upon driver failure, the driver core will take care of clearing the
driver data, so there's no need to do so explicitly in the driver.

Signed-off-by: Thierry Reding 
---
 drivers/gpu/drm/tegra/vic.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
index 9cb10d1e923a..d20fcbf6196d 100644
--- a/drivers/gpu/drm/tegra/vic.c
+++ b/drivers/gpu/drm/tegra/vic.c
@@ -412,7 +412,6 @@ static int vic_probe(struct platform_device *pdev)
err = host1x_client_register(&vic->client.base);
if (err < 0) {
dev_err(dev, "failed to register host1x client: %d\n", err);
-   platform_set_drvdata(pdev, NULL);
goto exit_falcon;
}
 
-- 
2.19.1

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

Re: [Intel-gfx] [PATCH 14/15] drm/i915/tv: Fix >1024 modes on gen3

[Imre Deak]

On Mon, Nov 12, 2018 at 06:59:59PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> On gen3 we must disable the TV encoder vertical filter for >1024
> pixel wide sources. Once that's done all we can is try to center
> the image on the screen. Naturally the TV mode vertical resolution
> must be equal or larger than the user mode vertical resolution
> or else we'd have to cut off part of the user mode.
> 
> And while we may not be able to respect the user's choice of
> top and bottom borders exactly (or we'd have to reject he mode
> most likely), we can try to maintain the relative sizes of the
> top and bottom border with respect to each orher.
> 
> Additionally we must configure the pipe as interlaced if the
> TV mode is interlaced.
> 
> Signed-off-by: Ville Syrjälä 
> ---
>  drivers/gpu/drm/i915/intel_tv.c | 100 +---
>  1 file changed, 92 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 75126fce655d..7099d837e31a 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -861,6 +861,44 @@ static const struct tv_mode tv_modes[] = {
>   },
>  };
>  
> +struct intel_tv_connector_state {
> + struct drm_connector_state base;
> +
> + /*
> +  * May need to override the user margins for
> +  * gen3 >1024 wide source vertical centering.
> +  */
> + struct {
> + u16 top, bottom;
> + } margins;
> +
> + bool bypass_vfilter;
> +};
> +
> +#define to_intel_tv_connector_state(x) container_of(x, struct 
> intel_tv_connector_state, base)
> +
> +/**
> + * intel_digital_connector_duplicate_state - duplicate connector state
  ^intel_tv_connector_duplicate_state
> + * @connector: digital connector
  ^tv connector?
> + *
> + * Allocates and returns a copy of the connector state (both common and
> + * digital connector specific) for the specified connector.
> + *
> + * Returns: The newly allocated connector state, or NULL on failure.
> + */
> +struct drm_connector_state *
> +intel_tv_connector_duplicate_state(struct drm_connector *connector)
> +{
> + struct intel_tv_connector_state *state;
> +
> + state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
> + if (!state)
> + return NULL;
> +
> + __drm_atomic_helper_connector_duplicate_state(connector, &state->base);
> + return &state->base;
> +}

You didn't add the corresponding checks for the new
intel_tv_connector_state fields to intel_tv_atomic_check(). I suppose
that's ok since something resulting in a change in those will force a
modeset anyway:

Reviewed-by: Imre Deak 

> +
>  static struct intel_tv *enc_to_tv(struct intel_encoder *encoder)
>  {
>   return container_of(encoder, struct intel_tv, base);
> @@ -1129,6 +1167,9 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>   struct intel_crtc_state *pipe_config,
>   struct drm_connector_state *conn_state)
>  {
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_tv_connector_state *tv_conn_state =
> + to_intel_tv_connector_state(conn_state);
>   const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state);
>   struct drm_display_mode *adjusted_mode =
>   &pipe_config->base.adjusted_mode;
> @@ -1149,6 +1190,43 @@ intel_tv_compute_config(struct intel_encoder *encoder,
>   pipe_config->port_clock = tv_mode->clock;
>  
>   intel_tv_mode_to_mode(adjusted_mode, tv_mode);
> + drm_mode_set_crtcinfo(adjusted_mode, 0);
> +
> + if (IS_GEN3(dev_priv) && hdisplay > 1024) {
> + int extra, top, bottom;
> +
> + extra = adjusted_mode->crtc_vdisplay - vdisplay;
> +
> + if (extra < 0) {
> + DRM_DEBUG_KMS("No vertical scaling for >1024 pixel wide 
> modes\n");
> + return false;
> + }
> +
> + /* Need to turn off the vertical filter and center the image */
> +
> + /* Attempt to maintain the relative sizes of the margins */
> + top = conn_state->tv.margins.top;
> + bottom = conn_state->tv.margins.bottom;
> +
> + if (top + bottom)
> + top = extra * top / (top + bottom);
> + else
> + top = extra / 2;
> + bottom = extra - top;
> +
> + tv_conn_state->margins.top = top;
> + tv_conn_state->margins.bottom = bottom;
> +
> + tv_conn_state->bypass_vfilter = true;
> +
> + if (!tv_mode->progressive)
> + adjusted_mode->flags |= DRM_MODE_FLAG_INTERLACE;
> + } else {
> + tv_conn_state->margins.top = conn_state->tv.margins.top;
> + tv_conn_state->margins.bottom = conn_state->tv.margins.bottom;
> +
> + tv_conn_state->bypass_vfilter = false;
> + }
>  
>   DRM_DE

Re: [Intel-gfx] [PATCH 15/15] drm/i915/tv: Filter out >1024 wide modes that would need vertical scaling on gen3

[Imre Deak]

On Mon, Nov 12, 2018 at 07:00:00PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä 
> 
> Since gen3 can't handle >1024 wide sources with vertical scaling
> let's not advertize such modes in the mode list. Less tempetation
> to the user to try out things that won't work.
> 
> Signed-off-by: Ville Syrjälä 

Reviewed-by: Imre Deak 

> ---
>  drivers/gpu/drm/i915/intel_tv.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
> index 7099d837e31a..89c537839273 100644
> --- a/drivers/gpu/drm/i915/intel_tv.c
> +++ b/drivers/gpu/drm/i915/intel_tv.c
> @@ -1738,6 +1738,7 @@ intel_tv_set_mode_type(struct drm_display_mode *mode,
>  static int
>  intel_tv_get_modes(struct drm_connector *connector)
>  {
> + struct drm_i915_private *dev_priv = to_i915(connector->dev);
>   const struct tv_mode *tv_mode = intel_tv_mode_find(connector->state);
>   int i, count = 0;
>  
> @@ -1750,6 +1751,11 @@ intel_tv_get_modes(struct drm_connector *connector)
>   !tv_mode->component_only)
>   continue;
>  
> + /* no vertical scaling with wide sources on gen3 */
> + if (IS_GEN3(dev_priv) && input->w > 1024 &&
> + input->h > intel_tv_mode_vdisplay(tv_mode))
> + continue;
> +
>   mode = drm_mode_create(connector->dev);
>   if (!mode)
>   continue;
> -- 
> 2.18.1
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] drm/tegra: Restrict IOVA space to DMA mask

[Thierry Reding]

On Wed, Jan 23, 2019 at 04:41:44PM +0300, Dmitry Osipenko wrote:
> 23.01.2019 12:39, Thierry Reding пишет:
> > From: Thierry Reding 
> > 
> > On Tegra186 and later, the ARM SMMU provides an input address space that
> > is 48 bits wide. However, memory clients can only address up to 40 bits.
> > If the geometry is used as-is, allocations of IOVA space can end up in a
> > region that cannot be addressed by the memory clients.
> > 
> > To fix this, restrict the IOVA space to the DMA mask of the host1x
> > device. Note that, technically, the IOVA space needs to be restricted to
> > the intersection of the DMA masks for all clients that are attached to
> > the IOMMU domain. In practice using the DMA mask of the host1x device is
> > sufficient because all host1x clients share the same DMA mask.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> >  drivers/gpu/drm/tegra/drm.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> > index 271c7a5fc954..0c5f1e6a0446 100644
> > --- a/drivers/gpu/drm/tegra/drm.c
> > +++ b/drivers/gpu/drm/tegra/drm.c
> > @@ -136,11 +136,12 @@ static int tegra_drm_load(struct drm_device *drm, 
> > unsigned long flags)
> >  
> > if (tegra->domain) {
> > u64 carveout_start, carveout_end, gem_start, gem_end;
> > +   u64 dma_mask = dma_get_mask(&device->dev);
> > dma_addr_t start, end;
> > unsigned long order;
> >  
> > -   start = tegra->domain->geometry.aperture_start;
> > -   end = tegra->domain->geometry.aperture_end;
> > +   start = tegra->domain->geometry.aperture_start & dma_mask;
> > +   end = tegra->domain->geometry.aperture_end & dma_mask;
> >  
> > gem_start = start;
> > gem_end = end - CARVEOUT_SZ;
> > 
> 
> Wow, so IOVA could address >32bits on later Tegra's. AFAIK, currently
> there is no support for a proper programming of the 64bit addresses in
> the drivers code, hence.. won't it make sense to force IOVA mask to
> 32bit for now and hope that the second halve of address registers
> happen to be 0x in HW?

I think this restriction only applies to display at this point. In
practice you'd be hard put to trigger that case because IOVA memory is
allocated from the bottom, so you'd actually need to use up to 4 GiB of
IOVA space before hitting that.

That said, I vaguely remember typing up the patch to support writing the
WINBUF_START_ADDR_HI register and friends, but it looks as if that was
never merged.

I'll try to dig out that patch (or rewrite it, shouldn't be very
difficult) and make it part of this series. I'd rather fix that issue
than arbitrarily restrict the IOVA space, because that's likely to come
back and bite us at some point.

Thierry


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH AUTOSEL 4.20 034/117] drm/amdgpu: Correct get_crtc_scanoutpos behavior when vpos >= vtotal

[Sasha Levin]

On Wed, Jan 09, 2019 at 10:20:49AM +0100, Michel Dänzer wrote:

On 2019-01-08 8:25 p.m., Sasha Levin wrote:

From: Nicholas Kazlauskas 

[ Upstream commit 520f08df45fbe300ed650da786a74093d658b7e1 ]

When variable refresh rate is active [...]


Variable refresh rate (FreeSync) support is only landing in 5.0,
therefore this fix isn't needed in older kernels.


I'll drop it, thank you.

--
Thanks,
Sasha
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm/tegra: vic: Load firmware on demand

[Thierry Reding]

On Wed, Jan 23, 2019 at 03:47:45PM +0300, Dmitry Osipenko wrote:
> 23.01.2019 12:39, Thierry Reding пишет:
> > From: Thierry Reding 
> > 
> > Loading the firmware requires an allocation of IOVA space to make sure
> > that the VIC's Falcon microcontroller can read the firmware if address
> > translation via the SMMU is enabled.
> > 
> > However, the allocation currently happens at a time where the geometry
> > of an IOMMU domain may not have been initialized yet. This happens for
> > example on Tegra186 and later where an ARM SMMU is used. Domains which
> > are created by the ARM SMMU driver postpone the geometry setup until a
> > device is attached to the domain. This is because IOMMU domains aren't
> > attached to a specific IOMMU instance at allocation time and hence the
> > input address space, which defines the geometry, is not known yet.
> > 
> > Work around this by postponing the firmware load until it is needed at
> > the time where a channel is opened to the VIC. At this time the shared
> > IOMMU domain's geometry has been properly initialized.
> > 
> > As a byproduct this allows the Tegra DRM to be created in the absence
> > of VIC firmware, since the VIC initialization no longer fails if the
> > firmware can't be found.
> > 
> > Signed-off-by: Thierry Reding 
> > ---
> >  drivers/gpu/drm/tegra/vic.c | 17 ++---
> >  1 file changed, 10 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c
> > index d47983deb1cf..afbdc33f49bc 100644
> > --- a/drivers/gpu/drm/tegra/vic.c
> > +++ b/drivers/gpu/drm/tegra/vic.c
> > @@ -181,13 +181,6 @@ static int vic_init(struct host1x_client *client)
> > vic->domain = tegra->domain;
> > }
> >  
> > -   if (!vic->falcon.data) {
> > -   vic->falcon.data = tegra;
> > -   err = falcon_load_firmware(&vic->falcon);
> > -   if (err < 0)
> > -   goto detach;
> > -   }
> > -
> > vic->channel = host1x_channel_request(client->dev);
> > if (!vic->channel) {
> > err = -ENOMEM;
> > @@ -256,6 +249,16 @@ static int vic_open_channel(struct tegra_drm_client 
> > *client,
> > if (err < 0)
> > return err;
> >  
> > +   if (!vic->falcon.data) {
> > +   vic->falcon.data = client->drm;
> > +
> > +   err = falcon_load_firmware(&vic->falcon);
> > +   if (err < 0) {
> > +   pm_runtime_put(vic->dev);
> > +   return err;
> > +   }
> > +   }
> > +
> > err = vic_boot(vic);
> > if (err < 0) {
> > pm_runtime_put(vic->dev);
> > 
> 
> This only moves the firmware data-copying to a later stage and doesn't
> touch reading out of the firmware file, hence the claim about the
> "byproduct" is invalid. Please take a look at the patch I posted
> sometime ago [0] and feel free to use it as a reference.

You're right, that hunk ended up in some other patch. And indeed this
patch looks pretty much like yours, so I've merged both together (mine
hadn't moved things out to a separate function, so I did that now, and
mine still reuses the client->drm pointer introduced in an earlier patch
to make it easier to pass that around).

Will send out v2 of this patch.

Thierry


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Greg KH  wrote:
> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>> Variables declared in a switch statement before any case statements
>> cannot be initialized, so move all instances out of the switches.
>> After this, future always-initialized stack variables will work
>> and not throw warnings like this:
>> 
>> fs/fcntl.c: In function ‘send_sigio_to_task’:
>> fs/fcntl.c:738:13: warning: statement will never be executed 
>> [-Wswitch-unreachable]
>>siginfo_t si;
>>  ^~
>
> That's a pain, so this means we can't have any new variables in { }
> scope except for at the top of a function?
>
> That's going to be a hard thing to keep from happening over time, as
> this is valid C :(

Not all valid C is meant to be used! ;)

Anyway, I think you're mistaking the limitation to arbitrary blocks
while it's only about the switch block IIUC.

Can't have:

switch (i) {
int j;
case 0:
/* ... */
}

because it can't be turned into:

switch (i) {
int j = 0; /* not valid C */
case 0:
/* ... */
}

but can have e.g.:

switch (i) {
case 0:
{
int j = 0;
/* ... */
}
}

I think Kees' approach of moving such variable declarations to the
enclosing block scope is better than adding another nesting block.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Jani Nikula  wrote:
> On Wed, 23 Jan 2019, Greg KH  wrote:
>> On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>>> Variables declared in a switch statement before any case statements
>>> cannot be initialized, so move all instances out of the switches.
>>> After this, future always-initialized stack variables will work
>>> and not throw warnings like this:
>>> 
>>> fs/fcntl.c: In function ‘send_sigio_to_task’:
>>> fs/fcntl.c:738:13: warning: statement will never be executed 
>>> [-Wswitch-unreachable]
>>>siginfo_t si;
>>>  ^~
>>
>> That's a pain, so this means we can't have any new variables in { }
>> scope except for at the top of a function?
>>
>> That's going to be a hard thing to keep from happening over time, as
>> this is valid C :(
>
> Not all valid C is meant to be used! ;)
>
> Anyway, I think you're mistaking the limitation to arbitrary blocks
> while it's only about the switch block IIUC.
>
> Can't have:
>
>   switch (i) {
>   int j;
>   case 0:
>   /* ... */
>   }
>
> because it can't be turned into:
>
>   switch (i) {
>   int j = 0; /* not valid C */
>   case 0:
>   /* ... */
>   }
>
> but can have e.g.:
>
>   switch (i) {
>   case 0:
>   {
>   int j = 0;
>   /* ... */
>   }
>   }
>
> I think Kees' approach of moving such variable declarations to the
> enclosing block scope is better than adding another nesting block.

PS. The patch is

Reviewed-by: Jani Nikula 

and the drivers/gpu/drm/i915/* parts are

Acked-by: Jani Nikula 

for merging via whichever tree is appropriate. (There'll be minor
conflicts with in-flight work in our -next tree, but no biggie.)


-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2 0/7] drm/bridge: tc358767: small fixes

[A H]

Hi Tomi,

śr., 23 sty 2019, 13:52: Tomi Valkeinen  napisał(a):

> Hi Andrzej,
>
> On 09/01/19 12:12, Andrzej Hajda wrote:
> > On 09.01.2019 10:51, Lucas Stach wrote:
> >> Am Mittwoch, den 09.01.2019, 11:12 +0200 schrieb Tomi Valkeinen:
> >>> Hi Andrzej,
> >>>
> >>> On 09/01/19 10:22, Andrzej Hajda wrote:
>  Hi Tomi,
> 
>  On 03.01.2019 12:59, Tomi Valkeinen wrote:
> > Hi,
> >
> > We have TC358867 on our board, which I believe is almost identical to
> > TC358767. We're using it with a DP connector instead of eDP with a
> fixed
> > panel.
> >
> > I have tested these patches only on TI's 4.14 based kernel, as
> > unfortunately we don't have all the necessary support in mainline
> yet.
> > These patches fix various bugs, but I'm still seeing at least two
> > issues:
> >
> > * Sync with some videomodes is not correct, resulting in a jumping
> and
> >   skewed display
> > * Link training fails sometimes
> >
> > I would appreciate if someone is able to verify these patches with
> > TC358767.
> 
>  Do you want to wait for testers or shall I queue this patchset?
> >>> I haven't heard from anyone, so I'm ok with pushing these.
> >> For the series:
> >>
> >> Tested-by: Lucas Stach 
> >>
> >> on a device with TC358767 and a 4.20 based kernel.
> >
> >
> > Already queued :)
>
> Did you push these somewhere? What's the route for these patches, drm-misc?
>

drm-misc-fixes:

https://github.com/freedesktop/drm-misc/commits/drm-misc-fixes/drivers/gpu/drm/bridge/tc358767.c

Andrzej


>  Tomi
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] iommu/intel: quirk to disable DMAR for QM57 igfx

[Joonas Lahtinen]

Quoting Joerg Roedel (2019-01-22 18:51:35)
> On Tue, Jan 22, 2019 at 04:48:26PM +0200, Joonas Lahtinen wrote:
> > According to our IOMMU folks there exists some desire to be able to assign
> > the iGFX device aka have intel_iommu=on instead of intel_iommu=igfx_off
> > due to how the devices might be grouped in IOMMU groups. Even when you
> > would not be using the iGFX device.
> 
> You can force the igfx device into a SI domain, or does that also
> trigger the iommu issues on the chipset?

To be honest, we've had a mixture different issues on different SKUs
that have not been hit in the past when intel_iommu was just disabled by
default.

I know that in one group of the problems, the issue has been debugged
into the GPU having its own set of virtualization mapping translation
hardware with caching and it fails to track changes to the mapping. So
if a identity mapping was established and never changed, I'd assume that
to fix at least that class of problems.

Would just passing intel_iommu=on already cause a non-identity mapping to
possibly be used for the integrated GPU? If it did, then it would
explain quite few of the issues.

We have many reports where just having intel_iommu=on (and using the
system normally, without any virtualization stuff going on) will cause
unexplained GPU hangs. For those users, simply switching to
intel_iommu=igfx_off solves the problems, and the debug often ends
there.

Regards, Joonas

> In any case, if iommu=on breaks these systems I want to make them work
> again with opt-out, even at the cost of breaking assignability.
> 
> Regards,
> 
> Joerg
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [Intel-gfx] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jani Nikula]

On Wed, 23 Jan 2019, Edwin Zimmerman  wrote:
> On Wed, 23 Jan 2019, Jani Nikula  wrote:
>> On Wed, 23 Jan 2019, Greg KH  wrote:
>> > On Wed, Jan 23, 2019 at 03:03:47AM -0800, Kees Cook wrote:
>> >> Variables declared in a switch statement before any case statements
>> >> cannot be initialized, so move all instances out of the switches.
>> >> After this, future always-initialized stack variables will work
>> >> and not throw warnings like this:
>> >>
>> >> fs/fcntl.c: In function ‘send_sigio_to_task’:
>> >> fs/fcntl.c:738:13: warning: statement will never be executed 
>> >> [-Wswitch-unreachable]
>> >>siginfo_t si;
>> >>  ^~
>> >
>> > That's a pain, so this means we can't have any new variables in { }
>> > scope except for at the top of a function?
>> >
>> > That's going to be a hard thing to keep from happening over time, as
>> > this is valid C :(
>> 
>> Not all valid C is meant to be used! ;)
>
> Very true.  The other thing to keep in mind is the burden of enforcing
> a prohibition on a valid C construct like this.  It seems to me that
> patch reviewers and maintainers have enough to do without forcing them
> to watch for variable declarations in switch statements.  Automating
> this prohibition, should it be accepted, seems like a good idea to me.

Considering that the treewide diffstat to fix this is:

 18 files changed, 45 insertions(+), 46 deletions(-)

and using the gcc plugin in question will trigger the switch-unreachable
warning, I think we're good. There'll probably be the occasional
declarations that pass through, and will get fixed afterwards.

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/sun4i: dsi: Restrict DSI tcon clock divider

[Maxime Ripard]

The current code allows the TCON clock divider to have a range between 4
and 127 when feeding the DSI controller.

The only display supported so far had a display clock rate that ended up
using a divider of 4, but testing with other displays show that only 4
seems to be functional.

This also aligns with what Allwinner is doing in their BSP, so let's just
hardcode that we want a divider of 4 when using the DSI output.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++--
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c 
b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 0420f5c978b9..bee73ead732a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -341,8 +341,8 @@ static void sun4i_tcon0_mode_set_cpu(struct sun4i_tcon 
*tcon,
u32 block_space, start_delay;
u32 tcon_div;
 
-   tcon->dclk_min_div = 4;
-   tcon->dclk_max_div = 127;
+   tcon->dclk_min_div = SUN6I_DSI_TCON_DIV;
+   tcon->dclk_max_div = SUN6I_DSI_TCON_DIV;
 
sun4i_tcon0_mode_set_common(tcon, mode);
 
diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h 
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
index dbbc5b3ecbda..6d4a3c0fd9b5 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h
@@ -13,6 +13,8 @@
 #include 
 #include 
 
+#define SUN6I_DSI_TCON_DIV 4
+
 struct sun6i_dphy {
struct clk  *bus_clk;
struct clk  *mod_clk;
-- 
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/sun4i: dsi: Change the start delay calculation

[Maxime Ripard]

The current calculation for the video start delay in the current DSI driver
is that it is the total vertical size, minus the backporch and sync length,
plus 1.

However, the Allwinner code has it as the active vertical size, plus the
back porch and the sync length. This doesn't make any difference on the
only panel it has been tested with so far, since in that particular case
the front porch is equal to the sum of the back porch and sync length.

This is not the case for all panels, obviously, so we need to fix it. Since
the Allwinner code has a bunch of extra code to deal with out of bounds
values, so let's add them as well.

Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c 
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index 380fc527a707..e3e4ba90c059 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -357,7 +357,12 @@ static void sun6i_dsi_inst_init(struct sun6i_dsi *dsi,
 static u16 sun6i_dsi_get_video_start_delay(struct sun6i_dsi *dsi,
   struct drm_display_mode *mode)
 {
-   return mode->vtotal - (mode->vsync_end - mode->vdisplay) + 1;
+   u16 delay = (mode->vsync_end + 1) % mode->vtotal;
+
+   if (!delay)
+   delay = 1;
+
+   return delay;
 }
 
 static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
-- 
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 0/4] drm/sun4i: dsi: Add burst mode support

[Maxime Ripard]

Hi,

Here is a series implementing the burst mode support for DSI.

It's been tested on an A33 board with the panel supported on the 4th patch,
which should remove all quirks due to a different SoC from the equation.

Let me know what you think,
Maxime

Konstantin Sudakov (2):
  drm/sun4i: dsi: Add burst support
  drm/panel: Add Rondo RB070D30 panel

Maxime Ripard (2):
  drm/sun4i: dsi: Restrict DSI tcon clock divider
  drm/sun4i: dsi: Change the start delay calculation

 drivers/gpu/drm/panel/Kconfig|   9 +-
 drivers/gpu/drm/panel/Makefile   |   1 +-
 drivers/gpu/drm/panel/panel-rondo-rb070d30.c | 258 -
 drivers/gpu/drm/sun4i/sun4i_tcon.c   |   4 +-
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c   | 185 ++
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.h   |   2 +-
 6 files changed, 414 insertions(+), 45 deletions(-)
 create mode 100644 drivers/gpu/drm/panel/panel-rondo-rb070d30.c

base-commit: f6028e7e48b0f3865b0837d400ca783d3d200b62
-- 
git-series 0.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/4] drm/panel: Add Rondo RB070D30 panel

[Maxime Ripard]

From: Konstantin Sudakov 

The Rondo RB070D30 panel is a MIPI-DSI panel based on a Fitipower EK79007
controller and a 1024x600 panel.

Signed-off-by: Konstantin Sudakov 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/panel/Kconfig|   9 +-
 drivers/gpu/drm/panel/Makefile   |   1 +-
 drivers/gpu/drm/panel/panel-rondo-rb070d30.c | 258 -
 3 files changed, 268 insertions(+)
 create mode 100644 drivers/gpu/drm/panel/panel-rondo-rb070d30.c

diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
index 3f3537719beb..3164fa824a63 100644
--- a/drivers/gpu/drm/panel/Kconfig
+++ b/drivers/gpu/drm/panel/Kconfig
@@ -138,6 +138,15 @@ config DRM_PANEL_RAYDIUM_RM68200
  Say Y here if you want to enable support for Raydium RM68200
  720x1280 DSI video mode panel.
 
+config DRM_PANEL_RONDO_RB070D30
+   tristate "Rondo Electronics RB070D30 panel"
+   depends on OF
+   depends on DRM_MIPI_DSI
+   depends on BACKLIGHT_CLASS_DEVICE
+   help
+ Say Y here if you want to enable support for Rondo Electronics
+ RB070D30 1024x600 DSI panel.
+
 config DRM_PANEL_SAMSUNG_S6D16D0
tristate "Samsung S6D16D0 DSI video mode panel"
depends on OF
diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
index 4396658a7996..4fe4cf1bfdb5 100644
--- a/drivers/gpu/drm/panel/Makefile
+++ b/drivers/gpu/drm/panel/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_DRM_PANEL_ORISETECH_OTM8009A) += 
panel-orisetech-otm8009a.o
 obj-$(CONFIG_DRM_PANEL_PANASONIC_VVX10F034N00) += 
panel-panasonic-vvx10f034n00.o
 obj-$(CONFIG_DRM_PANEL_RASPBERRYPI_TOUCHSCREEN) += 
panel-raspberrypi-touchscreen.o
 obj-$(CONFIG_DRM_PANEL_RAYDIUM_RM68200) += panel-raydium-rm68200.o
+obj-$(CONFIG_DRM_PANEL_RONDO_RB070D30) += panel-rondo-rb070d30.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_LD9040) += panel-samsung-ld9040.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6D16D0) += panel-samsung-s6d16d0.o
 obj-$(CONFIG_DRM_PANEL_SAMSUNG_S6E3HA2) += panel-samsung-s6e3ha2.o
diff --git a/drivers/gpu/drm/panel/panel-rondo-rb070d30.c 
b/drivers/gpu/drm/panel/panel-rondo-rb070d30.c
new file mode 100644
index ..4f8e63f367b1
--- /dev/null
+++ b/drivers/gpu/drm/panel/panel-rondo-rb070d30.c
@@ -0,0 +1,258 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018, Bridge Systems BV
+ * Copyright (C) 2018, Bootlin
+ * Copyright (C) 2017, Free Electrons
+ *
+ * This file based on panel-ilitek-ili9881c.c
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+struct rb070d30_panel {
+   struct drm_panel panel;
+   struct mipi_dsi_device *dsi;
+   struct backlight_device *backlight;
+   struct regulator *supply;
+
+   struct {
+   struct gpio_desc *power;
+   struct gpio_desc *reset;
+   struct gpio_desc *updn;
+   struct gpio_desc *shlr;
+   } gpios;
+};
+
+static inline struct rb070d30_panel *panel_to_rb070d30_panel(struct drm_panel 
*panel)
+{
+   return container_of(panel, struct rb070d30_panel, panel);
+}
+
+static int rb070d30_panel_prepare(struct drm_panel *panel)
+{
+   struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+   int ret;
+
+   ret = regulator_enable(ctx->supply);
+   if (ret < 0) {
+   dev_err(&ctx->dsi->dev, "Failed to enable supply: %d\n", ret);
+   return ret;
+   }
+
+   /* Reset */
+   msleep(20);
+   gpiod_set_value(ctx->gpios.power, 1);
+   msleep(20);
+   gpiod_set_value(ctx->gpios.reset, 1);
+   msleep(20);
+   return 0;
+}
+
+static int rb070d30_panel_unprepare(struct drm_panel *panel)
+{
+   struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+
+   gpiod_set_value(ctx->gpios.power, 0);
+   gpiod_set_value(ctx->gpios.reset, 0);
+   regulator_disable(ctx->supply);
+
+   return 0;
+}
+
+static int rb070d30_panel_enable(struct drm_panel *panel)
+{
+   struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+   int ret;
+
+   ret = mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
+   if (ret)
+   return ret;
+
+   ret = backlight_enable(ctx->backlight);
+   if (ret)
+   goto out;
+
+   return 0;
+
+out:
+   mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+   return ret;
+}
+
+static int rb070d30_panel_disable(struct drm_panel *panel)
+{
+   struct rb070d30_panel *ctx = panel_to_rb070d30_panel(panel);
+
+   backlight_disable(ctx->backlight);
+   return mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
+}
+
+/* Default timings */
+static const struct drm_display_mode default_mode = {
+   .clock  = 51206,
+   .hdisplay   = 1024,
+   .hsync_start= 1024 + 160,
+   .hsync_end  = 1024 + 160 + 80,
+   .htotal = 1024 + 160 

Re: [Freedreno] [PATCH v4 3/3] drm/msm/dpu: add display port support in DPU

[Sean Paul]

On Mon, Dec 17, 2018 at 02:35:05PM -0800, Jeykumar Sankaran wrote:
> Add display port support in DPU by creating hooks
> for DP encoder enumeration and encoder mode
> initialization.
> 
> This change is based on the SDM845 Display port
> driver changes[1].
> 
> changes in v2:
> - rebase on [2] (Sean Paul)
> - remove unwanted error checks and
>   switch cases (Jordan Crouse)
> changes in v3:
> - add dp support after fixing
>   the current code base for error logging (Sean Paul)
> changes in v4:
> -  avoid duplicate returns (Jordan Crouse)
> -  get rid of duplicate error logs (Jordan Crouse)
> 
> [1] https://lwn.net/Articles/768265/
> [2] https://lkml.org/lkml/2018/11/17/87
> 
> Signed-off-by: Jeykumar Sankaran 
> Reviewed-by: Jordan Crouse 
> Reviewed-by: Sean Paul 
> ---
>  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c |  8 ++--
>  drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 58 
> +
>  2 files changed, 54 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 0dda4a6..371d17d 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -2031,7 +2031,7 @@ static int dpu_encoder_setup_display(struct 
> dpu_encoder_virt *dpu_enc,
>  {
>   int ret = 0;
>   int i = 0;
> - enum dpu_intf_type intf_type;
> + enum dpu_intf_type intf_type = INTF_NONE;
>   struct dpu_enc_phys_init_params phys_params;
>  
>   if (!dpu_enc || !dpu_kms) {
> @@ -2054,9 +2054,9 @@ static int dpu_encoder_setup_display(struct 
> dpu_encoder_virt *dpu_enc,
>   case DRM_MODE_ENCODER_DSI:
>   intf_type = INTF_DSI;
>   break;
> - default:
> - DPU_ERROR_ENC(dpu_enc, "unsupported display interface type\n");
> - return -EINVAL;
> + case DRM_MODE_ENCODER_TMDS:
> + intf_type = INTF_DP;
> + break;
>   }
>  
>   WARN_ON(disp_info->num_of_h_tiles < 1);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c 
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 885bf88..62b400c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -439,6 +439,31 @@ static int _dpu_kms_initialize_dsi(struct drm_device 
> *dev,
>   return rc;
>  }
>  
> +static int _dpu_kms_initialize_displayport(struct drm_device *dev,
> + struct msm_drm_private *priv,
> + struct dpu_kms *dpu_kms)
> +{
> + struct drm_encoder *encoder = NULL;
> + int rc;
> +
> + if (!priv->dp)
> + return 0;
> +
> + encoder = dpu_encoder_init(dev, DRM_MODE_ENCODER_TMDS);
> + if (IS_ERR(encoder)) {
> + DPU_ERROR("encoder init failed for dsi display\n");
> + return PTR_ERR(encoder);
> + }
> +
> + rc = msm_dp_modeset_init(priv->dp, dev, encoder);
> + if (rc) {
> + DPU_ERROR("modeset_init failed for DP, rc = %d\n", rc);
> + drm_encoder_cleanup(encoder);
> + }
> +
> + return rc;
> +}
> +
>  /**
>   * _dpu_kms_setup_displays - create encoders, bridges and connectors
>   *   for underlying displays
> @@ -451,12 +476,22 @@ static int _dpu_kms_setup_displays(struct drm_device 
> *dev,
>   struct msm_drm_private *priv,
>   struct dpu_kms *dpu_kms)
>  {
> + int rc;
> +
> + rc = _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
> + if (rc)
> + goto fail;
> +
> + rc = _dpu_kms_initialize_displayport(dev, priv, dpu_kms);
> + if (rc)
> + goto fail;
> +
>   /**
>* Extend this function to initialize other
>* types of displays
>*/
> -
> - return _dpu_kms_initialize_dsi(dev, priv, dpu_kms);
> +fail:

One more thing, remove this label and just return directly above.

> + return rc;

Then this becomes return 0

>  }
>  
>  static void _dpu_kms_drm_obj_destroy(struct dpu_kms *dpu_kms)
> @@ -669,13 +704,20 @@ static void _dpu_kms_set_encoder_mode(struct msm_kms 
> *kms,
>   info.capabilities = cmd_mode ? MSM_DISPLAY_CAP_CMD_MODE :
>   MSM_DISPLAY_CAP_VID_MODE;
>  
> - /* TODO: No support for DSI swap */
> - for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> - if (priv->dsi[i]) {
> - info.h_tile_instance[info.num_of_h_tiles] = i;
> - info.num_of_h_tiles++;
> + switch (info.intf_type) {
> + case DRM_MODE_ENCODER_DSI:
> + /* TODO: No support for DSI swap */
> + for (i = 0; i < ARRAY_SIZE(priv->dsi); i++) {
> + if (priv->dsi[i]) {
> + info.h_tile_instance[info.num_of_h_tiles] = i;
> + info.num_of_h_tiles++;
> + }
>  

[PATCH 3/4] drm/sun4i: dsi: Add burst support

[Maxime Ripard]

From: Konstantin Sudakov 

The current driver doesn't support the DSI burst operation mode.

Let's add the needed quirks to make it work.

Signed-off-by: Konstantin Sudakov 
Signed-off-by: Maxime Ripard 
---
 drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c | 178 +++---
 1 file changed, 136 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c 
b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
index e3e4ba90c059..6840b3512a45 100644
--- a/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
+++ b/drivers/gpu/drm/sun4i/sun6i_mipi_dsi.c
@@ -23,7 +23,9 @@
 #include 
 #include 
 
+#include "sun4i_crtc.h"
 #include "sun4i_drv.h"
+#include "sun4i_tcon.h"
 #include "sun6i_mipi_dsi.h"
 
 #include 
@@ -32,6 +34,8 @@
 #define SUN6I_DSI_CTL_EN   BIT(0)
 
 #define SUN6I_DSI_BASIC_CTL_REG0x00c
+#define SUN6I_DSI_BASIC_CTL_TRAIL_INV(n)   (((n) & 0xf) << 4)
+#define SUN6I_DSI_BASIC_CTL_TRAIL_FILL BIT(3)
 #define SUN6I_DSI_BASIC_CTL_HBP_DISBIT(2)
 #define SUN6I_DSI_BASIC_CTL_HSA_HSE_DISBIT(1)
 #define SUN6I_DSI_BASIC_CTL_VIDEO_BURSTBIT(0)
@@ -152,6 +156,8 @@
 
 #define SUN6I_DSI_CMD_TX_REG(n)(0x300 + (n) * 0x04)
 
+#define SUN6I_DSI_SYNC_POINT   40
+
 enum sun6i_dsi_start_inst {
DSI_START_LPRX,
DSI_START_LPTX,
@@ -365,31 +371,101 @@ static u16 sun6i_dsi_get_video_start_delay(struct 
sun6i_dsi *dsi,
return delay;
 }
 
+static u16 sun6i_dsi_get_line_num(struct sun6i_dsi *dsi,
+ struct drm_display_mode *mode)
+{
+   struct mipi_dsi_device *device = dsi->device;
+   unsigned Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
+
+   return mode->htotal * Bpp / device->lanes;
+}
+
+static u16 sun6i_dsi_get_drq_edge0(struct sun6i_dsi *dsi,
+  struct drm_display_mode *mode,
+  u16 line_num, u16 edge1)
+{
+   u16 edge0 = edge1;
+
+   edge0 += (mode->hdisplay + 40) * SUN6I_DSI_TCON_DIV / 8;
+
+   if (edge0 > line_num)
+   return edge0 - line_num;
+
+   return 1;
+}
+
+static u16 sun6i_dsi_get_drq_edge1(struct sun6i_dsi *dsi,
+  struct drm_display_mode *mode,
+  u16 line_num)
+{
+   struct mipi_dsi_device *device = dsi->device;
+   unsigned Bpp = mipi_dsi_pixel_format_to_bpp(device->format) / 8;
+   unsigned hbp = mode->htotal - mode->hsync_end;
+   u16 edge1;
+
+
+   edge1 = SUN6I_DSI_SYNC_POINT;
+   edge1 += mode->hdisplay + hbp + 20;
+   edge1 = edge1 * Bpp / device->lanes;
+
+   if (edge1 > line_num)
+   return line_num;
+
+   return edge1;
+}
+
 static void sun6i_dsi_setup_burst(struct sun6i_dsi *dsi,
  struct drm_display_mode *mode)
 {
struct mipi_dsi_device *device = dsi->device;
-   u32 val = 0;
+   u32 bpp = mipi_dsi_pixel_format_to_bpp(device->format);
+   u32 tcon0_drq = 0;
+
+   if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
+   u16 line_num = sun6i_dsi_get_line_num(dsi, mode);
+   u16 edge0, edge1;
+
+   edge1 = sun6i_dsi_get_drq_edge1(dsi, mode, line_num);
+   edge0 = sun6i_dsi_get_drq_edge0(dsi, mode, line_num, edge1);
 
-   if ((mode->hsync_end - mode->hdisplay) > 20) {
+   regmap_write(dsi->regs, SUN6I_DSI_BURST_DRQ_REG,
+SUN6I_DSI_BURST_DRQ_EDGE0(edge0) |
+SUN6I_DSI_BURST_DRQ_EDGE1(edge1));
+
+   regmap_write(dsi->regs, SUN6I_DSI_BURST_LINE_REG,
+SUN6I_DSI_BURST_LINE_NUM(line_num) |
+
SUN6I_DSI_BURST_LINE_SYNC_POINT(SUN6I_DSI_SYNC_POINT));
+
+   tcon0_drq = SUN6I_DSI_TCON_DRQ_ENABLE_MODE;
+   } else if ((mode->hsync_end - mode->hdisplay) > 20) {
/* Maagic */
u16 drq = (mode->hsync_end - mode->hdisplay) - 20;
-
-   drq *= mipi_dsi_pixel_format_to_bpp(device->format);
+   drq *= bpp;
drq /= 32;
 
-   val = (SUN6I_DSI_TCON_DRQ_ENABLE_MODE |
-  SUN6I_DSI_TCON_DRQ_SET(drq));
+   tcon0_drq = SUN6I_DSI_TCON_DRQ_ENABLE_MODE |
+   SUN6I_DSI_TCON_DRQ_SET(drq);
}
 
-   regmap_write(dsi->regs, SUN6I_DSI_TCON_DRQ_REG, val);
+   regmap_write(dsi->regs, SUN6I_DSI_TCON_DRQ_REG, tcon0_drq);
 }
 
 static void sun6i_dsi_setup_inst_loop(struct sun6i_dsi *dsi,
  struct drm_display_mode *mode)
 {
+   struct mipi_dsi_device *device = dsi->device;
u16 delay = 50 - 1;
 
+   if (device->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) {
+   delay = (mode->htotal - mode->hdisplay) * 150;
+   delay /= (mode->clock / 1000) * 8;
+   delay -= 5

[Bug 108031] [Regression] amdgpu_device_ip_init failed on Bonaire XT

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=108031

--- Comment #2 from Alexander Schlarb  ---
Created attachment 143205
  --> https://bugs.freedesktop.org/attachment.cgi?id=143205&action=edit
4.19.0 Debian kernal boot log (external screen working)

I can reproduce this with

00:01.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI]
Carrizo (rev c4)
03:00.0 Display controller: Advanced Micro Devices, Inc. [AMD/ATI] Bonaire
XT [Radeon R9 M280X] (rev 80)

on a “Lenovo ideapad Y700-ACZ”.

When I boot into the released Linux 4.19.0 from Debian I get a blank screen,
however today I discovered that if you have an external displayed plugged
during boot-up that display will be successfully enabled and you can then log
in as usual. The internal laptop display will not ever be enabled however and
checking under *Systemsettings* → *Monitor Settings* (KDE) will not show it
either. Also the entire system will feel very sluggish (not CPU or disk
related, I checked) and is only usable with lots of patience. On shutdown the
system will then hang (nothing about this in system logs unfortunately).

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 14/15] drm/i915/tv: Fix >1024 modes on gen3

[Ville Syrjälä]

On Wed, Jan 23, 2019 at 03:49:02PM +0200, Imre Deak wrote:
> On Mon, Nov 12, 2018 at 06:59:59PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä 
> > 
> > On gen3 we must disable the TV encoder vertical filter for >1024
> > pixel wide sources. Once that's done all we can is try to center
> > the image on the screen. Naturally the TV mode vertical resolution
> > must be equal or larger than the user mode vertical resolution
> > or else we'd have to cut off part of the user mode.
> > 
> > And while we may not be able to respect the user's choice of
> > top and bottom borders exactly (or we'd have to reject he mode
> > most likely), we can try to maintain the relative sizes of the
> > top and bottom border with respect to each orher.
> > 
> > Additionally we must configure the pipe as interlaced if the
> > TV mode is interlaced.
> > 
> > Signed-off-by: Ville Syrjälä 
> > ---
> >  drivers/gpu/drm/i915/intel_tv.c | 100 +---
> >  1 file changed, 92 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_tv.c 
> > b/drivers/gpu/drm/i915/intel_tv.c
> > index 75126fce655d..7099d837e31a 100644
> > --- a/drivers/gpu/drm/i915/intel_tv.c
> > +++ b/drivers/gpu/drm/i915/intel_tv.c
> > @@ -861,6 +861,44 @@ static const struct tv_mode tv_modes[] = {
> > },
> >  };
> >  
> > +struct intel_tv_connector_state {
> > +   struct drm_connector_state base;
> > +
> > +   /*
> > +* May need to override the user margins for
> > +* gen3 >1024 wide source vertical centering.
> > +*/
> > +   struct {
> > +   u16 top, bottom;
> > +   } margins;
> > +
> > +   bool bypass_vfilter;
> > +};
> > +
> > +#define to_intel_tv_connector_state(x) container_of(x, struct 
> > intel_tv_connector_state, base)
> > +
> > +/**
> > + * intel_digital_connector_duplicate_state - duplicate connector state
>   ^intel_tv_connector_duplicate_state
> > + * @connector: digital connector
>   ^tv connector?
> > + *
> > + * Allocates and returns a copy of the connector state (both common and
> > + * digital connector specific) for the specified connector.
> > + *
> > + * Returns: The newly allocated connector state, or NULL on failure.
> > + */
> > +struct drm_connector_state *
> > +intel_tv_connector_duplicate_state(struct drm_connector *connector)
> > +{
> > +   struct intel_tv_connector_state *state;
> > +
> > +   state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL);
> > +   if (!state)
> > +   return NULL;
> > +
> > +   __drm_atomic_helper_connector_duplicate_state(connector, &state->base);
> > +   return &state->base;
> > +}
> 
> You didn't add the corresponding checks for the new
> intel_tv_connector_state fields to intel_tv_atomic_check(). I suppose
> that's ok since something resulting in a change in those will force a
> modeset anyway:

The new fields are not visible to the user so nothing external
will change them. intel_tv_compute_config() (which is executed
after .atomic_check()) will just recompute them based on other
user visible state.

> 
> Reviewed-by: Imre Deak 
> 
> > +
> >  static struct intel_tv *enc_to_tv(struct intel_encoder *encoder)
> >  {
> > return container_of(encoder, struct intel_tv, base);
> > @@ -1129,6 +1167,9 @@ intel_tv_compute_config(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config,
> > struct drm_connector_state *conn_state)
> >  {
> > +   struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > +   struct intel_tv_connector_state *tv_conn_state =
> > +   to_intel_tv_connector_state(conn_state);
> > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state);
> > struct drm_display_mode *adjusted_mode =
> > &pipe_config->base.adjusted_mode;
> > @@ -1149,6 +1190,43 @@ intel_tv_compute_config(struct intel_encoder 
> > *encoder,
> > pipe_config->port_clock = tv_mode->clock;
> >  
> > intel_tv_mode_to_mode(adjusted_mode, tv_mode);
> > +   drm_mode_set_crtcinfo(adjusted_mode, 0);
> > +
> > +   if (IS_GEN3(dev_priv) && hdisplay > 1024) {
> > +   int extra, top, bottom;
> > +
> > +   extra = adjusted_mode->crtc_vdisplay - vdisplay;
> > +
> > +   if (extra < 0) {
> > +   DRM_DEBUG_KMS("No vertical scaling for >1024 pixel wide 
> > modes\n");
> > +   return false;
> > +   }
> > +
> > +   /* Need to turn off the vertical filter and center the image */
> > +
> > +   /* Attempt to maintain the relative sizes of the margins */
> > +   top = conn_state->tv.margins.top;
> > +   bottom = conn_state->tv.margins.bottom;
> > +
> > +   if (top + bottom)
> > +   top = extra * top / (top + bottom);
> > +   else
> > +   top = extra / 2;
> > +   bottom = extra - top;
> > +
> > +   tv_conn_state->margins.top = top;
> > +   tv_conn_state->margins.bottom = bottom;

Re: [PATCH] iommu/intel: quirk to disable DMAR for QM57 igfx

[Joerg Roedel]

On Wed, Jan 23, 2019 at 05:02:38PM +0200, Joonas Lahtinen wrote:
> We have many reports where just having intel_iommu=on (and using the
> system normally, without any virtualization stuff going on) will cause
> unexplained GPU hangs. For those users, simply switching to
> intel_iommu=igfx_off solves the problems, and the debug often ends
> there.

If you can reproduce problems on your side, then you can try to enable
CONFIG_INTEL_IOMMU_BROKEN_GFX_WA to force the GFX devices into the
identity mapping. We can also add a boot-parameter and workarounds if it
turns out that this is sufficient to make the GFX devices work with
IOMMU enabled.


Regards,

Joerg
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-wired-lan] [PATCH 1/3] treewide: Lift switch variables out of switches

[Jeff Kirsher]

On Wed, 2019-01-23 at 03:03 -0800, Kees Cook wrote:
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
> 
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed [-
> Wswitch-unreachable]
>siginfo_t si;
>  ^~
> 
> Signed-off-by: Kees Cook 

Acked-by: Jeff Kirsher 

For the e1000 changes.

> ---
>  arch/x86/xen/enlighten_pv.c   |  7 ---
>  drivers/char/pcmcia/cm4000_cs.c   |  2 +-
>  drivers/char/ppdev.c  | 20 -
> --
>  drivers/gpu/drm/drm_edid.c|  4 ++--
>  drivers/gpu/drm/i915/intel_display.c  |  2 +-
>  drivers/gpu/drm/i915/intel_pm.c   |  4 ++--
>  drivers/net/ethernet/intel/e1000/e1000_main.c |  3 ++-
>  drivers/tty/n_tty.c   |  3 +--
>  drivers/usb/gadget/udc/net2280.c  |  5 ++---
>  fs/fcntl.c|  3 ++-
>  mm/shmem.c|  5 +++--
>  net/core/skbuff.c |  4 ++--
>  net/ipv6/ip6_gre.c|  4 ++--
>  net/ipv6/ip6_tunnel.c |  4 ++--
>  net/openvswitch/flow_netlink.c|  7 +++
>  security/tomoyo/common.c  |  3 ++-
>  security/tomoyo/condition.c   |  7 ---
>  security/tomoyo/util.c|  4 ++--
>  18 files changed, 45 insertions(+), 46 deletions(-)



signature.asc
Description: This is a digitally signed message part
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap

[Andrew F. Davis]

On 1/22/19 11:33 AM, Sumit Semwal wrote:
> Hello everyone,
> 
> Sincere apologies for chiming in a bit late here, but was off due to
> some health issues.
> 

Hope you are feeling better friend :)

Looks like this email was a bit broken and you replied again, the
responses are a little different in each email, so I'd like to respond
to bits of both, I'll fix up the formatting.

> Also, adding Daniel Vetter to the mix, since he has been one of the
> core guys who shaped up dma-buf as it is today.
> 
> On Tue, 22 Jan 2019 at 02:51, Andrew F. Davis  wrote:
>>
>> On 1/21/19 5:22 AM, Brian Starkey wrote:

[snip]

>>>
>>> Actually I meant in the kernel, in exporters. I haven't seen anyone
>>> using the API as it was intended (defer allocation until first map,
>>> migrate between different attachments, etc.). Mostly, backing storage
>>> seems to get allocated at the point of export, and device mappings are
>>> often held persistently (e.g. the DRM prime code maps the buffer at
>>> import time, and keeps it mapped: drm_gem_prime_import_dev).
>>>
>>
> 
> So I suppose some clarification on the 'intended use' part of dma-buf
> about deferred allocation is due, so here it is: (Daniel, please feel
> free to chime in with your opinion here)
> 
>  - dma-buf was of course designed as a framework to help intelligent
> exporters to defer allocation until first map, and be able to migrate
> backing storage if required etc. At the same time, it is not a
> _requirement_ from any exporter, so exporters so far have just used it
> as a convenient mechanism for zero-copy.
> - ION is one of the few dma-buf exporters in kernel, which satisfies a
> certain set of expectations from its users.
> 

The issue here is that Ion is blocking the ability to late allocate, it
expects its heaps to have the memory ready at allocation time. My point
being if the DMA-BUFs intended design was to allow this then Ion should
respect that and also allow the same from its heap exporters.

>> I haven't either, which is a shame as it allows for some really useful
>> management strategies for shared memory resources. I'm working on one
>> such case right now, maybe I'll get to be the first to upstream one :)
>>
> That will be a really good thing! Though perhaps we ought to think if
> for what you're trying to do, is ION the right place, or should you
> have a device-specific exporter, available to users via dma-buf apis?
> 

I'm starting to question if Ion is the right place myself..

At a conceptual level I don't believe userspace should be picking the
backing memory type. This is because the right type of backing memory
for a task will change from system to system. The kernel should abstract
away these hardware differences from userspace as much as it can to
allow portable code.

For instance a device may need a contiguous buffer on one system but the
same device on another may have some IOMMU. So which type of memory do
we allocate? Same issue for cacheability and other properties.

What we need is a central allocator with full system knowledge to do the
choosing for us. It seems many agree with the above and I take
inspiration from your cenalloc patchset. The thing I'm not sure about is
letting the device drivers set their constraints, because they also
don't have the full system integration details. For cases where devices
are behind an IOMMU it is easy enough for the device to know, but what
about when we have external MMUs out on the bus for anyone to use (I'm
guessing you remember TILER..).

I would propose the central allocator keep per-system knowledge (or
fetch it from DT, or if this is considered policy then userspace) which
it can use to directly check the attached devices and pick the right memory.

Anyway the central system allocator could handle 90% of cases I can
think of, and this is where Ion comes back in, the other cases would
still require the program to manually pick the right memory (maybe for
performance reasons, etc.).

So my vision is to have Ion as the the main front-end for DMA-BUF
allocations, and expose the central allocator through it (maybe as a
default heap type that can be populated on a per-system basis), but also
have other individual heap types exported for the edge cases where
manual selection is needed like we do now.

Hence why Ion should allow direct control of the dma_buf_ops from the
heaps, so we can build central allocators as Ion heaps.

If I'm off into the weeds here and you have some other ideas I'm all ears.

Andrew

>>> I wasn't aware that CPU access before first device access was
>>> considered an abuse of the API - it seems like a valid thing to want
>>> to do.
>>>
>>
>> That's just it, I don't know if it is an abuse of API, I'm trying to get
>> some clarity on that. If we do want to allow early CPU access then that
>> seems to be in contrast to the idea of deferred allocation until first
>> device map, what is supposed to backing the buffer if no devices have
>> attached or mapped yet? Just some system m

Re: [PATCH] drm: Split out drm_probe_helper.h

[Sam Ravnborg]

Hi Daniel.

On Thu, Jan 17, 2019 at 10:03:34PM +0100, Daniel Vetter wrote:
> Having the probe helper stuff (which pretty much everyone needs) in
> the drm_crtc_helper.h file (which atomic drivers should never need) is
> confusing. Split them out.
> 
> To make sure I actually achieved the goal here I went through all
> drivers. And indeed, all atomic drivers are now free of
> drm_crtc_helper.h includes.

How are the plans to get this patchset merged?
There were dependencies on onging drmP.h removal in i915 IIRC?
I guess my "Minimize drmP.h dependencies" patch-set also have a role in this.

This does not hold up any work of mine, mainly wanted to make
sure this was not lost between all the other patches.

Sam
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap

[Andrew F. Davis]

On 1/22/19 9:23 PM, Sumit Semwal wrote:
> Hello everyone,
> 
> (Thanks to Dan for letting me know my last email got corrupted :/ -
> resending it here)
> 

Hmm, this one seems a bit messed up also (Thunderbird doesn't seem to
like it at least).

[snip]

> - from dma-buf PoV, ION is an exporter of dma-buf buffers, for its users
> that have specific requirements.
> 

This is what I'm hoping to change up a little bit, Ion shouldn't be the
exporter, its heaps should be the exporters (manage the dma_buf_ops),
Ion would only do advertising of available heaps and allow allocating
DMA-BUFs from them.

IMO that would clear up the other discussions going on right now about
how Ion should handle different dma-buf syncing tasks, it simply
wouldn't :). Plus Ion core gets slimmed down, maybe even enough for
destaging..

>> I haven't either, which is a shame as it allows for some really useful
>> management strategies for shared memory resources. I'm working on one
>> such case right now, maybe I'll get to be the first to upstream one :)
>>
> Yes, it would, and great that you're looking to be the first one to do it :)
> 
>> > I wasn't aware that CPU access before first device access was
>> > considered an abuse of the API - it seems like a valid thing to want
>> > to do.
>> >
>>
>> That's just it, I don't know if it is an abuse of API, I'm trying to get
>> some clarity on that. If we do want to allow early CPU access then that
>> seems to be in contrast to the idea of deferred allocation until first
>> device map, what is supposed to backing the buffer if no devices have
>> attached or mapped yet? Just some system memory followed by migration on
>> the first attach to the proper backing? Seems too time wasteful to be
>> have a valid use.
>>
>> Maybe it should be up to the exporter if early CPU access is allowed?
>>
>> I'm hoping someone with authority over the DMA-BUF framework can clarify
>> original intentions here.
>>
> 
> I suppose dma-buf as a framework can't know or decide what the exporter
> wants or can do - whether the exporter wants to use it for 'only
> zero-copy', or do some intelligent things behind the scene, I think
> should be best left to the exporter.
> 
> Hope this helps,

Yes, these inputs are very helpful, thanks,
Andrew

> Sumit.
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/14] staging: android: ion: Do not sync CPU cache on map/unmap

[Brian Starkey]

Hi Andrew,

On Wed, Jan 23, 2019 at 10:51:24AM -0600, Andrew F. Davis wrote:
> On 1/22/19 11:33 AM, Sumit Semwal wrote:
> > Hello everyone,
> > 
> > Sincere apologies for chiming in a bit late here, but was off due to
> > some health issues.
> > 
> 
> Hope you are feeling better friend :)
> 
> Looks like this email was a bit broken and you replied again, the
> responses are a little different in each email, so I'd like to respond
> to bits of both, I'll fix up the formatting.
> 
> > Also, adding Daniel Vetter to the mix, since he has been one of the
> > core guys who shaped up dma-buf as it is today.
> > 
> > On Tue, 22 Jan 2019 at 02:51, Andrew F. Davis  wrote:
> >>
> >> On 1/21/19 5:22 AM, Brian Starkey wrote:
> 
> [snip]
> 
> >>>
> >>> Actually I meant in the kernel, in exporters. I haven't seen anyone
> >>> using the API as it was intended (defer allocation until first map,
> >>> migrate between different attachments, etc.). Mostly, backing storage
> >>> seems to get allocated at the point of export, and device mappings are
> >>> often held persistently (e.g. the DRM prime code maps the buffer at
> >>> import time, and keeps it mapped: drm_gem_prime_import_dev).
> >>>
> >>
> > 
> > So I suppose some clarification on the 'intended use' part of dma-buf
> > about deferred allocation is due, so here it is: (Daniel, please feel
> > free to chime in with your opinion here)
> > 
> >  - dma-buf was of course designed as a framework to help intelligent
> > exporters to defer allocation until first map, and be able to migrate
> > backing storage if required etc. At the same time, it is not a
> > _requirement_ from any exporter, so exporters so far have just used it
> > as a convenient mechanism for zero-copy.
> > - ION is one of the few dma-buf exporters in kernel, which satisfies a
> > certain set of expectations from its users.
> > 
> 
> The issue here is that Ion is blocking the ability to late allocate, it
> expects its heaps to have the memory ready at allocation time. My point
> being if the DMA-BUFs intended design was to allow this then Ion should
> respect that and also allow the same from its heap exporters.
> 
> >> I haven't either, which is a shame as it allows for some really useful
> >> management strategies for shared memory resources. I'm working on one
> >> such case right now, maybe I'll get to be the first to upstream one :)
> >>
> > That will be a really good thing! Though perhaps we ought to think if
> > for what you're trying to do, is ION the right place, or should you
> > have a device-specific exporter, available to users via dma-buf apis?
> > 
> 
> I'm starting to question if Ion is the right place myself..
> 
> At a conceptual level I don't believe userspace should be picking the
> backing memory type. This is because the right type of backing memory
> for a task will change from system to system. The kernel should abstract
> away these hardware differences from userspace as much as it can to
> allow portable code.
> 
> For instance a device may need a contiguous buffer on one system but the
> same device on another may have some IOMMU. So which type of memory do
> we allocate? Same issue for cacheability and other properties.
> 
> What we need is a central allocator with full system knowledge to do the
> choosing for us. It seems many agree with the above and I take
> inspiration from your cenalloc patchset. The thing I'm not sure about is
> letting the device drivers set their constraints, because they also
> don't have the full system integration details. For cases where devices
> are behind an IOMMU it is easy enough for the device to know, but what
> about when we have external MMUs out on the bus for anyone to use (I'm
> guessing you remember TILER..).
> 
> I would propose the central allocator keep per-system knowledge (or
> fetch it from DT, or if this is considered policy then userspace) which
> it can use to directly check the attached devices and pick the right memory.
> 
> Anyway the central system allocator could handle 90% of cases I can
> think of, and this is where Ion comes back in, the other cases would
> still require the program to manually pick the right memory (maybe for
> performance reasons, etc.).
> 
> So my vision is to have Ion as the the main front-end for DMA-BUF
> allocations, and expose the central allocator through it (maybe as a
> default heap type that can be populated on a per-system basis), but also
> have other individual heap types exported for the edge cases where
> manual selection is needed like we do now.
> 
> Hence why Ion should allow direct control of the dma_buf_ops from the
> heaps, so we can build central allocators as Ion heaps.
> 
> If I'm off into the weeds here and you have some other ideas I'm all ears.
> 

This is a topic I've gone around a few times. The crux of it is, as
you know, a central allocator is Really Hard. I don't know what you've
seen/done so far in this area, so please forgive me if this is old hat
to you.

Android's p

Re: [PATCH] drm/vkms: Fix flush_work() without INIT_WORK().

[Shayenne Moura]

> syzbot is hitting a lockdep warning [1] because flush_work() is called
> without INIT_WORK() after kzalloc() at vkms_atomic_crtc_reset().
>
> Commit 6c234fe37c57627a ("drm/vkms: Implement CRC debugfs API") added
> INIT_WORK() to only vkms_atomic_crtc_duplicate_state() side. Assuming
> that lifecycle of crc_work is appropriately managed, fix this problem
> by adding INIT_WORK() to vkms_atomic_crtc_reset() side.
>
> [1] 
> https://syzkaller.appspot.com/bug?id=a5954455fcfa51c29ca2ab55b203076337e1c770
>
> Reported-and-tested-by: syzbot 
> 
> Signed-off-by: Tetsuo Handa 
> ---
>  drivers/gpu/drm/vkms/vkms_crtc.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c 
> b/drivers/gpu/drm/vkms/vkms_crtc.c
> index 177bbcb..3c37d8c 100644
> --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> @@ -104,6 +104,7 @@ static void vkms_atomic_crtc_reset(struct drm_crtc *crtc)
> vkms_state = kzalloc(sizeof(*vkms_state), GFP_KERNEL);
> if (!vkms_state)
> return;
> +   INIT_WORK(&vkms_state->crc_work, vkms_crc_work_handle);
>
> crtc->state = &vkms_state->base;
> crtc->state->crtc = crtc;
> --
> 1.8.3.1
>

Thank you for your patch!

Reviewed-by: Shayenne Moura 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] xf86drm: Add drmIsMaster()

[Emil Velikov]

On Wed, 23 Jan 2019 at 04:39, Christopher James Halse Rogers
 wrote:
>
> We can't use drmSetMaster to query whether or not a drm fd is master
> because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd.
>
> Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is
> DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect
> whether or not the fd is master.
>
> This is useful for code that might get master by open()ing the drm device
> while no other master exists, but can't call drmSetMaster itself because
> it's not running as root or is in a container, where container-root isn't
> real-root.
>
> v2: Use the AUTH_MAGIC request rather than MODE_ATTACHMODE, as it's more
> clearly related to master status.
>
> Signed-off-by: Christopher James Halse Rogers 
> 
> ---
>  xf86drm.c | 15 +++
>  xf86drm.h |  2 ++
>  2 files changed, 17 insertions(+)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index 10df682b..adee5bd9 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -2741,6 +2741,21 @@ drm_public int drmDropMaster(int fd)
>  return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
>  }
>
> +drm_public bool drmIsMaster(int fd)
> +{
> +/* Detect master by attempting something that requires master.
> + *
> + * Authenticating magic tokens requires master and 0 is
> + * guaranteed to be an invalid magic number. Attempting this on
> + * a master fd will fail therefore fail with EINVAL because 0 is
> + * invalid.
> + *
> + * A non-master fd will fail with EACCESS, as the kernel checks for
> + * master before attempting to do anything else.
> + */
> +return drmAuthMagic(fd, 0) == EINVAL;
What magic value is valid, is a DRM implementation detail, which we
don't need to depend upon.

Instead we can check for EACCES, since we care if we have permissions
- aka are we master.
The function returns a negative errno, so I'd make this a:

return drmAuthMagic(fd, 0) != -EACCES;

If you and Daniel agree, I'll squash this locally and push.

-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 1/4] drm/vc4: Wait for display list synchronization when completing commit

[Eric Anholt]

Paul Kocialkowski  writes:

> During an atomic commit, the HVS is configured with a display list
> for the channel matching the associated CRTC. The Pixel Valve (CRTC)
> and encoder are also configured for the new setup at that time.
> While the Pixel Valve and encoder are reconfigured synchronously, the
> HVS is only reconfigured after the display list address (DISPLIST) has
> been updated to the current display list address (DISPLACTX), which is
> the responsibility of the hardware.
>
> The time frame during which the HVS is still running on its previous
> configuration but the CRTC and encoder have been reconfigured already
> can lead to a number of synchronization issues. They will eventually
> cause errors reported on the FIFOs, such as underruns.
>
> With underrun detection enabled (from Boris Brezillon's series), this
> leads to unreliable underrun detection with random false positives.
>
> To ensure a coherent state, wait for each enabled channel of the HVS
> to synchronize its current display list address. This fixes the issue
> of random underrun reporting on commits.
>
> Signed-off-by: Paul Kocialkowski 
> ---
>  drivers/gpu/drm/vc4/vc4_drv.h  |  1 +
>  drivers/gpu/drm/vc4/vc4_hvs.c  | 17 +
>  drivers/gpu/drm/vc4/vc4_kms.c  |  2 ++
>  drivers/gpu/drm/vc4/vc4_regs.h |  2 ++
>  4 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index c24b078f0593..955f157f5ad0 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -772,6 +772,7 @@ void vc4_irq_reset(struct drm_device *dev);
>  extern struct platform_driver vc4_hvs_driver;
>  void vc4_hvs_dump_state(struct drm_device *dev);
>  int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused);
> +void vc4_hvs_sync_dlist(struct drm_device *dev);
>  
>  /* vc4_kms.c */
>  int vc4_kms_load(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 5d8c749c9749..1ba60b8e0c2d 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -166,6 +166,23 @@ static int vc4_hvs_upload_linear_kernel(struct vc4_hvs 
> *hvs,
>   return 0;
>  }
>  
> +void vc4_hvs_sync_dlist(struct drm_device *dev)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + unsigned int i;
> + int ret;
> +
> + for (i = 0; i < SCALER_CHANNELS_COUNT; i++) {
> + if (!(HVS_READ(SCALER_DISPCTRLX(i)) & SCALER_DISPCTRLX_ENABLE))
> + continue;
> +
> + ret = wait_for(HVS_READ(SCALER_DISPLACTX(i)) ==
> +HVS_READ(SCALER_DISPLISTX(i)), 1000);
> + WARN(ret, "Timeout waiting for channel %d display list sync\n",
> +  i);
> + }
> +}
> +
>  static int vc4_hvs_bind(struct device *dev, struct device *master, void 
> *data)
>  {
>   struct platform_device *pdev = to_platform_device(dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c
> index 0490edb192a1..2d66a2b57a91 100644
> --- a/drivers/gpu/drm/vc4/vc4_kms.c
> +++ b/drivers/gpu/drm/vc4/vc4_kms.c
> @@ -155,6 +155,8 @@ vc4_atomic_complete_commit(struct drm_atomic_state *state)
>  
>   drm_atomic_helper_commit_hw_done(state);
>  
> + vc4_hvs_sync_dlist(dev);
> +
>   drm_atomic_helper_wait_for_flip_done(dev, state);

wait_for_flip_done should already be waiting for DISPLACTX to match our
crtc's display list, though, right (see vc4_crtc_handle_page_flip())?
This seems like a no-op to me.


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 2/4] drm/vc4: Report underrun errors

[Eric Anholt]

Paul Kocialkowski  writes:

> From: Boris Brezillon 
>
> The DRM framework provides a generic way to report underrun errors.
> Let's implement the necessary hooks to support it in the VC4 driver.
>
> Signed-off-by: Boris Brezillon 
> ---
> Changes in v3:
> - Generic underrun report function has been dropped, adjust the
>   code accordingly

Update commit message for DRM framework not providing a generic way any
more?

>
> Changes in v2:
> - New patch
> ---
>  drivers/gpu/drm/vc4/vc4_debugfs.c |  1 +
>  drivers/gpu/drm/vc4/vc4_drv.h | 10 
>  drivers/gpu/drm/vc4/vc4_hvs.c | 87 +++
>  drivers/gpu/drm/vc4/vc4_kms.c |  4 ++
>  drivers/gpu/drm/vc4/vc4_regs.h| 46 
>  5 files changed, 113 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c 
> b/drivers/gpu/drm/vc4/vc4_debugfs.c
> index 7a0003de71ab..64021bcba041 100644
> --- a/drivers/gpu/drm/vc4/vc4_debugfs.c
> +++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
> @@ -23,6 +23,7 @@ static const struct drm_info_list vc4_debugfs_list[] = {
>   {"vec_regs", vc4_vec_debugfs_regs, 0},
>   {"txp_regs", vc4_txp_debugfs_regs, 0},
>   {"hvs_regs", vc4_hvs_debugfs_regs, 0},
> + {"hvs_underrun",  vc4_hvs_debugfs_underrun, 0},
>   {"crtc0_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)0},
>   {"crtc1_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)1},
>   {"crtc2_regs", vc4_crtc_debugfs_regs, 0, (void *)(uintptr_t)2},
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 955f157f5ad0..619fb8bec428 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -184,6 +184,13 @@ struct vc4_dev {
>   /* Bitmask of the current bin_alloc used for overflow memory. */
>   uint32_t bin_alloc_overflow;
>  
> + /* Incremented when an underrun error happened after an atomic commit.
> +  * This is particularly useful to detect when a specific modeset is too
> +  * demanding in term of memory or HVS bandwidth which is hard to guess
> +  * at atomic check time.
> +  */
> + atomic_t underrun;
> +
>   struct work_struct overflow_mem_work;
>  
>   int power_refcount;
> @@ -773,6 +780,9 @@ extern struct platform_driver vc4_hvs_driver;
>  void vc4_hvs_dump_state(struct drm_device *dev);
>  int vc4_hvs_debugfs_regs(struct seq_file *m, void *unused);
>  void vc4_hvs_sync_dlist(struct drm_device *dev);
> +int vc4_hvs_debugfs_underrun(struct seq_file *m, void *unused);
> +void vc4_hvs_unmask_underrun(struct drm_device *dev);
> +void vc4_hvs_mask_underrun(struct drm_device *dev);
>  
>  /* vc4_kms.c */
>  int vc4_kms_load(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/vc4/vc4_hvs.c b/drivers/gpu/drm/vc4/vc4_hvs.c
> index 1ba60b8e0c2d..3095fad2ff8b 100644
> --- a/drivers/gpu/drm/vc4/vc4_hvs.c
> +++ b/drivers/gpu/drm/vc4/vc4_hvs.c
> @@ -22,6 +22,7 @@
>   * each CRTC.
>   */
>  
> +#include 
>  #include 
>  #include "vc4_drv.h"
>  #include "vc4_regs.h"
> @@ -102,6 +103,18 @@ int vc4_hvs_debugfs_regs(struct seq_file *m, void 
> *unused)
>  
>   return 0;
>  }
> +
> +int vc4_hvs_debugfs_underrun(struct seq_file *m, void *data)
> +{
> + struct drm_info_node *node = m->private;
> + struct drm_device *dev = node->minor->dev;
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + struct drm_printer p = drm_seq_file_printer(m);
> +
> + drm_printf(&p, "%d\n", atomic_read(&vc4->underrun));
> +
> + return 0;
> +}
>  #endif
>  
>  /* The filter kernel is composed of dwords each containing 3 9-bit
> @@ -183,6 +196,62 @@ void vc4_hvs_sync_dlist(struct drm_device *dev)
>   }
>  }
>  
> +void vc4_hvs_mask_underrun(struct drm_device *dev)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + u32 dispctrl = HVS_READ(SCALER_DISPCTRL);
> +
> + dispctrl &= ~(SCALER_DISPCTRL_DSPEISLUR(0) |
> +   SCALER_DISPCTRL_DSPEISLUR(1) |
> +   SCALER_DISPCTRL_DSPEISLUR(2));
> +
> + HVS_WRITE(SCALER_DISPCTRL, dispctrl);
> +}
> +
> +void vc4_hvs_unmask_underrun(struct drm_device *dev)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + u32 dispctrl = HVS_READ(SCALER_DISPCTRL);
> +
> + dispctrl |= SCALER_DISPCTRL_DSPEISLUR(0) |
> + SCALER_DISPCTRL_DSPEISLUR(1) |
> + SCALER_DISPCTRL_DSPEISLUR(2);
> +
> + HVS_WRITE(SCALER_DISPSTAT,
> +   SCALER_DISPSTAT_EUFLOW(0) |
> +   SCALER_DISPSTAT_EUFLOW(1) |
> +   SCALER_DISPSTAT_EUFLOW(2));
> + HVS_WRITE(SCALER_DISPCTRL, dispctrl);
> +}
> +
> +static void vc4_hvs_report_underrun(struct drm_device *dev)
> +{
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> +
> + atomic_inc(&vc4->underrun);
> + DRM_DEV_ERROR(dev->dev, "HVS underrun\n");
> +}
> +
> +static irqreturn_t vc4_hvs_irq_handler(int irq, void *data)
> +{
> + struct drm_device *dev = data;
> + struct vc4_dev *vc4 = to_vc4_dev(dev);
> + u32 status;

[Bug 108031] [Regression] amdgpu_device_ip_init failed on Bonaire XT

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=108031

--- Comment #3 from Alex Deucher  ---
Remove amdgpu.dpm=1 from your kernel command line.  The default dpm
implementation changed in 4.19.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 108031] [Regression] amdgpu_device_ip_init failed on Bonaire XT

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=108031

--- Comment #4 from Alex Deucher  ---
(In reply to Alex Deucher from comment #3)
> Remove amdgpu.dpm=1 from your kernel command line.  The default dpm
> implementation changed in 4.19.

and setting dpm=1 overrides that resulting in different behavior.  power
management is always enabled by default.  the dpm option is only for debugging
and switching between implementations for asics that contain multiple
implementations.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] staging: android: ion: Allocate from heap ID directly without mask

[Andrew F. Davis]

Previously the heap to allocate from was selected by a mask of allowed
heap types. This may have been done as a primitive form of constraint
solving, the first heap type that matched any set bit of the heap mask
was allocated from, unless that heap was excluded by having its heap
ID bit not set in the separate passed in heap ID mask.

The heap type does not really represent constraints that should be
matched against to begin with. So the patch [0] removed the the heap type
mask matching but unfortunately left the heap ID mask check (possibly by
mistake or to preserve API). Therefor we now only have a mask of heap
IDs, but heap IDs are unique identifiers and have nothing to do with the
underling heap, so mask matching is not useful here. This also limits us
to 32 heaps total in a system.

With the heap query API users can find the right heap based on type or
name themselves then just supply the ID for that heap. Remove heap ID
mask and allow users to specify heap ID directly by its number.

I know this is an ABI break, but we are in staging so lets get this over
with now rather than limit ourselves later.

[0] 2bb9f5034ec7 ("gpu: ion: Remove heapmask from client")

Signed-off-by: Andrew F. Davis 
---

This also means we don't need to store the available heaps in a plist,
we only operation we care about is lookup, so a better data structure
should be chosen at some point, regular list or xarray maybe?

This is base on -next as to be on top of the other taken Ion patches.

 drivers/staging/android/ion/ion.c  | 21 ++---
 drivers/staging/android/uapi/ion.h |  6 ++
 2 files changed, 12 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c 
b/drivers/staging/android/ion/ion.c
index 92c2914239e3..06dd5bb10ecb 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -387,7 +387,7 @@ static const struct dma_buf_ops dma_buf_ops = {
.unmap = ion_dma_buf_kunmap,
 };
 
-static int ion_alloc(size_t len, unsigned int heap_id_mask, unsigned int flags)
+static int ion_alloc(size_t len, unsigned int heap_id, unsigned int flags)
 {
struct ion_device *dev = internal_dev;
struct ion_buffer *buffer = NULL;
@@ -396,23 +396,22 @@ static int ion_alloc(size_t len, unsigned int 
heap_id_mask, unsigned int flags)
int fd;
struct dma_buf *dmabuf;
 
-   pr_debug("%s: len %zu heap_id_mask %u flags %x\n", __func__,
-len, heap_id_mask, flags);
-   /*
-* traverse the list of heaps available in this system in priority
-* order.  If the heap type is supported by the client, and matches the
-* request of the caller allocate from it.  Repeat until allocate has
-* succeeded or all heaps have been tried
-*/
+   pr_debug("%s: len %zu heap_id %u flags %x\n", __func__,
+len, heap_id, flags);
+
len = PAGE_ALIGN(len);
 
if (!len)
return -EINVAL;
 
+   /*
+* Traverse the list of heaps available in this system.  If the
+* heap id matches the request of the caller allocate from it.
+*/
down_read(&dev->lock);
plist_for_each_entry(heap, &dev->heaps, node) {
/* if the caller didn't specify this heap id */
-   if (!((1 << heap->id) & heap_id_mask))
+   if (heap->id != heap_id)
continue;
buffer = ion_buffer_create(heap, dev, len, flags);
if (!IS_ERR(buffer))
@@ -541,7 +540,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, 
unsigned long arg)
int fd;
 
fd = ion_alloc(data.allocation.len,
-  data.allocation.heap_id_mask,
+  data.allocation.heap_id,
   data.allocation.flags);
if (fd < 0)
return fd;
diff --git a/drivers/staging/android/uapi/ion.h 
b/drivers/staging/android/uapi/ion.h
index 5d7009884c13..6a78a1e23251 100644
--- a/drivers/staging/android/uapi/ion.h
+++ b/drivers/staging/android/uapi/ion.h
@@ -35,8 +35,6 @@ enum ion_heap_type {
   */
 };
 
-#define ION_NUM_HEAP_IDS   (sizeof(unsigned int) * 8)
-
 /**
  * allocation flags - the lower 16 bits are used by core ion, the upper 16
  * bits are reserved for use by the heaps themselves.
@@ -59,7 +57,7 @@ enum ion_heap_type {
 /**
  * struct ion_allocation_data - metadata passed from userspace for allocations
  * @len:   size of the allocation
- * @heap_id_mask:  mask of heap ids to allocate from
+ * @heap_id:   heap id to allocate from
  * @flags: flags passed to heap
  * @handle:pointer that will be populated with a cookie to use to
  * refer to this allocation
@@ -68,7 +66,7 @@ enum ion_heap_type {
  */
 struct ion_allocation_data {
__u64 len;
-   __u32 h

[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105733

l...@protonmail.ch changed:

   What|Removed |Added

 CC||l...@protonmail.ch

--- Comment #57 from l...@protonmail.ch ---
Created attachment 143206
  --> https://bugs.freedesktop.org/attachment.cgi?id=143206&action=edit
I get these errors when attempting to boot after a normal GPU hang and KMS
happens

Recently I've been getting another type of hang somehow. After a normal hang
happens, where my screen gets garbled output, I can't even get past KMS in the
next couple of boots. I can fix this by flipping my VBIOS switch, which heavily
leads me to believe that amdgpu somehow corrupts the GPU's firmware. I have
attached the error I get when KMS happens at boot, which happens after I get a
hang while using the system normally. The monitor doesn't display anything when
this happens, but I can still control caps lock, etc., however I can not shut
it off normally. I have not tested whether SSH and such still work.

This honestly makes me doubt whether what I am experiencing is the same bug; is
it simply a faulty GPU? I am using a Sapphire RX 580 4GB, which I bought used
from a windows user. It *did* work for him, so obviously it isn't entirely
broken at least.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105733

--- Comment #58 from krutoiles...@gmail.com ---
The corruption suggestion is interesting. My RX580 does this and now it
won't even boot on windows anymore, just crashes.

On Wed, Jan 23, 2019, 12:44 PM  l...@protonmail.ch changed bug 105733
> 
> What Removed Added
> CC   l...@protonmail.ch
>
> *Comment # 57  on
> bug 105733  from
> l...@protonmail.ch  *
>
> Created attachment 143206 
>  [details] 
> 
> I get these errors when attempting to boot after a normal GPU hang and KMS
> happens
>
> Recently I've been getting another type of hang somehow. After a normal hang
> happens, where my screen gets garbled output, I can't even get past KMS in the
> next couple of boots. I can fix this by flipping my VBIOS switch, which 
> heavily
> leads me to believe that amdgpu somehow corrupts the GPU's firmware. I have
> attached the error I get when KMS happens at boot, which happens after I get a
> hang while using the system normally. The monitor doesn't display anything 
> when
> this happens, but I can still control caps lock, etc., however I can not shut
> it off normally. I have not tested whether SSH and such still work.
>
> This honestly makes me doubt whether what I am experiencing is the same bug; 
> is
> it simply a faulty GPU? I am using a Sapphire RX 580 4GB, which I bought used
> from a windows user. It *did* work for him, so obviously it isn't entirely
> broken at least.
>
> --
> You are receiving this mail because:
>
>- You are on the CC list for the bug.
>
>

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105733

--- Comment #59 from dwagner  ---
I don't think your observations indicate a hardware defect.

I have also a reproducible "hysteresis"-effect with regards to my RX460 GPU:
When I experience the bug scenario I reported in
https://bugs.freedesktop.org/show_bug.cgi?id=102322 and then reboot by pressing
the reset-button, the BIOS greeting and the GRUB loader are consistently not
shown (just a black screen, but with the connected TV indicating a valid HDMI
signal), only once Linux sets the console video mode during boot, then the
screen lights up again. If at that point, or at any time thereafter I reboot
either by typing "reboot" or by pressing the RESET button, then the BIOS
greeting and GRUB menu are shown as normal.

I think this is just due to some lack of thorough initialization upon reset,
because if I power down the machine by switching off the power supply, and then
reboot, the BIOS and GRUB menu always come up. Seems to me that pressing the
RESET button just isn't resetting as much as an actual power down does.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 108916] polaris11 d3d9 rendering issue

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=108916

--- Comment #1 from Axel Davy  ---
Hi,

I just wanted to confirm the trace reproduces the issue, but it may not be a
nine issue but a radeonsi llvm issue. The trace has no glitches on llvmpipe.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] xf86drm: Add drmIsMaster()

[Christopher James Halse Rogers]

On 24 January 2019 6:18:32 am NZDT, Emil Velikov  
wrote:
>On Wed, 23 Jan 2019 at 04:39, Christopher James Halse Rogers
> wrote:
>>
>> We can't use drmSetMaster to query whether or not a drm fd is master
>> because it requires CAP_SYS_ADMIN, even if the fd *is* a master fd.
>>
>> Pick DRM_IOCTL_MODE_ATTACHMODE as a long-deprecated ioctl that is
>> DRM_MASTER but not DRM_ROOT_ONLY as the probe by which we can detect
>> whether or not the fd is master.
>>
>> This is useful for code that might get master by open()ing the drm
>device
>> while no other master exists, but can't call drmSetMaster itself
>because
>> it's not running as root or is in a container, where container-root
>isn't
>> real-root.
>>
>> v2: Use the AUTH_MAGIC request rather than MODE_ATTACHMODE, as it's
>more
>> clearly related to master status.
>>
>> Signed-off-by: Christopher James Halse Rogers
>
>> ---
>>  xf86drm.c | 15 +++
>>  xf86drm.h |  2 ++
>>  2 files changed, 17 insertions(+)
>>
>> diff --git a/xf86drm.c b/xf86drm.c
>> index 10df682b..adee5bd9 100644
>> --- a/xf86drm.c
>> +++ b/xf86drm.c
>> @@ -2741,6 +2741,21 @@ drm_public int drmDropMaster(int fd)
>>  return drmIoctl(fd, DRM_IOCTL_DROP_MASTER, NULL);
>>  }
>>
>> +drm_public bool drmIsMaster(int fd)
>> +{
>> +/* Detect master by attempting something that requires
>master.
>> + *
>> + * Authenticating magic tokens requires master and 0 is
>> + * guaranteed to be an invalid magic number. Attempting this
>on
>> + * a master fd will fail therefore fail with EINVAL because
>0 is
>> + * invalid.
>> + *
>> + * A non-master fd will fail with EACCESS, as the kernel
>checks for
>> + * master before attempting to do anything else.
>> + */
>> +return drmAuthMagic(fd, 0) == EINVAL;
>What magic value is valid, is a DRM implementation detail, which we
>don't need to depend upon.
>
>Instead we can check for EACCES, since we care if we have permissions
>- aka are we master.
>The function returns a negative errno, so I'd make this a:
>
>return drmAuthMagic(fd, 0) != -EACCES;
>
>If you and Daniel agree, I'll squash this locally and push.

That's a much better idea, thanks!

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

[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=105733

--- Comment #60 from l...@protonmail.ch ---
dwagner, my problem persists even if I completely power the system down after
shutting it down by holding down the power button and then turning the PSU
completely off. I have not tried shutting it off only using the PSU fearing
damage to my hardware, although I will try that the next time at least once.

Also, it is reassuring to see that I am not the only one experiencing such odd
behavior, but could it not be that we simply all use faulty hardware?

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 201497] [amdgpu]: '*ERROR* No EDID read' is back in 4.19

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=201497

Daniel Andersson (engyw...@gmail.com) changed:

   What|Removed |Added

 Kernel Version|4.19|4.19 4.20 5.0-rc1 5.0-rc3
 Regression|No  |Yes

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 201497] [amdgpu]: '*ERROR* No EDID read' is back in 4.19

[bugzilla-daemon]

https://bugzilla.kernel.org/show_bug.cgi?id=201497

--- Comment #6 from Daniel Andersson (engyw...@gmail.com) ---
Created attachment 280701
  --> https://bugzilla.kernel.org/attachment.cgi?id=280701&action=edit
5.0-rc3 drm.debug=0x4

This is still an issue on 5.0-rc3.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109444] Graveyard Keeper: Lockup in under 5min of play.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109444

Bug ID: 109444
   Summary: Graveyard Keeper: Lockup in under 5min of play.
   Product: DRI
   Version: unspecified
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: cheako+bugs_freedesktop_...@mikemestnik.net

Created attachment 143209
  --> https://bugs.freedesktop.org/attachment.cgi?id=143209&action=edit
ar file with glxinfo and Xorg.log

When I play graveyard keeper, native or via wine, my system locks-up(does not
respond to ping or ssh, but dose continue to spin fan) after under 5min of
play. 
 I can't even get to a point where i can save my progress. 

I'm able to load it under renderdoc, but am unable to create a capture. 
Playing under renderdoc results in lower framerate, but little else seems
different(still locks-up).

bnieuwen2uizen, on IRC, asked me to open a bug report.

ThinkPad E585
8GB RAM, 8GB Swap.
AMD Ryzen 7 2700U with Radeon Vega Mobile Gfx
Manufacturer: LENOVO
Product Name: 20KV000YUS

Debian sid multi-arch
Graveyard Keeper is a 32bit Windows app, can't confirm native architecture.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109445] Graveyard Keeper: Lockup in under 5min of play.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109445

Bug ID: 109445
   Summary: Graveyard Keeper: Lockup in under 5min of play.
   Product: DRI
   Version: unspecified
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: cheako+bugs_freedesktop_...@mikemestnik.net

When I play graveyard keeper, native or via wine, my system locks-up(does not
respond to ping or ssh, but dose continue to spin fan) after under 5min of
play. 
 I can't even get to a point where i can save my progress. 

I'm able to load it under renderdoc, but am unable to create a capture. 
Playing under renderdoc results in lower framerate, but little else seems
different(still locks-up).

bnieuwen2uizen, on IRC, asked me to open a bug report.

ThinkPad E585
8GB RAM, 8GB Swap.
AMD Ryzen 7 2700U with Radeon Vega Mobile Gfx
Manufacturer: LENOVO
Product Name: 20KV000YUS

Debian sid multi-arch
Graveyard Keeper is a 32bit Windows app, can't confirm native architecture.

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109445] Graveyard Keeper: Lockup in under 5min of play.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109445

Mike Mestnik  changed:

   What|Removed |Added

URL||https://store.steampowered.
   ||com/app/599140/Graveyard_Ke
   ||eper/

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 109445] Graveyard Keeper: Lockup in under 5min of play.

[bugzilla-daemon]

https://bugs.freedesktop.org/show_bug.cgi?id=109445

--- Comment #1 from Mike Mestnik  
---
Created attachment 143210
  --> https://bugs.freedesktop.org/attachment.cgi?id=143210&action=edit
ar file containing glxinfo and Xorg.log

-- 
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/bridge: sil_sii8620: depend on INPUT instead of selecting it.

[Laurent Pinchart]

Hello Dmity,

On Wed, Jan 23, 2019 at 02:03:42PM -0800, Dmitry Torokhov wrote:
> On Wed, Jan 23, 2019 at 09:45:56AM +0100, Lukas Wunner wrote:
> > On Tue, Jan 22, 2019 at 06:13:11AM -0800, Ronald Tschalär wrote:
> >> commit d6abe6df706c66d803e6dd4fe98c1b6b7f125a56 (drm/bridge:
> >> sil_sii8620: do not have a dependency of RC_CORE) added a dependency on
> >> INPUT. However, this causes problems with other drivers, in particular
> >> an input driver that depends on MFD_INTEL_LPSS_PCI (to be added in a
> >> future commit):
> >> 
> >>   drivers/clk/Kconfig:9:error: recursive dependency detected!
> >>   drivers/clk/Kconfig:9:symbol COMMON_CLK is selected by 
> >> MFD_INTEL_LPSS
> >>   drivers/mfd/Kconfig:566:  symbol MFD_INTEL_LPSS is selected by 
> >> MFD_INTEL_LPSS_PCI
> >>   drivers/mfd/Kconfig:580:  symbol MFD_INTEL_LPSS_PCI is implied by 
> >> KEYBOARD_APPLESPI
> >>   drivers/input/keyboard/Kconfig:73:symbol KEYBOARD_APPLESPI depends 
> >> on INPUT
> >>   drivers/input/Kconfig:8:  symbol INPUT is selected by DRM_SIL_SII8620
> >>   drivers/gpu/drm/bridge/Kconfig:83:symbol DRM_SIL_SII8620 depends on 
> >> DRM_BRIDGE
> >>   drivers/gpu/drm/bridge/Kconfig:1: symbol DRM_BRIDGE is selected by 
> >> DRM_PL111
> >>   drivers/gpu/drm/pl111/Kconfig:1:  symbol DRM_PL111 depends on 
> >> COMMON_CLK
> >> 
> >> According to the docs, select should only be used for non-visible
> >> symbols. Furthermore almost all other references to INPUT throughout the
> >> kernel config are depends, not selects. Hence this change.
> 
> I think this is not as cut and dry. We should be able to select needed
> subsystems (such as INPUT, USB, etc) even if they are user visible.

Semantically, maybe, but given the current state of Kconfig this results
in a recursive dependencies nightmare. It's a no-go.

> User, when enabling a piece of hardware, does not need to know ultimate
> details of all subsystems the driver might need ti function.
> 
> It looks like one of the drivers implies MFD_INTEL_LPSS_PCI, maybe
> treating imply the same as select when detecting circular dependency is
> wrong as we are allowed to deselect implied dependencies?
> 
> >> 
> >> CC: Inki Dae 
> >> CC: Andrzej Hajda 
> >> Signed-off-by: Ronald Tschalär 
> > 
> > Reviewed-by: Lukas Wunner 
> > 
> > I think this needs to be merged through the input tree as a prerequisite
> > for the applespi.c driver (keyboard + touchpad driver for 2015+ MacBook,
> > MacBook Air and MacBook Pro which uses SPI instead of USB) to avoid
> > breaking the build.  Adding Dmitry.
> 
> I have no idea what applespi.c is (it is definitely not in my tree), so
> I think it should be merged through the same tree that the original
> commit was introduced through.
> 
> >> ---
> >>  drivers/gpu/drm/bridge/Kconfig | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/bridge/Kconfig 
> >> b/drivers/gpu/drm/bridge/Kconfig
> >> index 2fee47b0d50b..eabedc83f25c 100644
> >> --- a/drivers/gpu/drm/bridge/Kconfig
> >> +++ b/drivers/gpu/drm/bridge/Kconfig
> >> @@ -83,9 +83,9 @@ config DRM_PARADE_PS8622
> >>  config DRM_SIL_SII8620
> >>tristate "Silicon Image SII8620 HDMI/MHL bridge"
> >>depends on OF
> >> +  depends on INPUT
> >>select DRM_KMS_HELPER
> >>imply EXTCON
> >> -  select INPUT
> >>select RC_CORE
> 
> Keeping "select RC_CORE" is wrong though, as the driver appears to be
> working find without RC. Maybe it should be stubbed out?

It should definitely not be select'ed as it's a user-visible symbol. My
preference would be to simply revert d6abe6df706c. If we want (and can)
work without RC core then it should be stubbed out.

Commit d6abe6df706c states

And some boards not using remote controller device don't really
need to know that RC_CORE config should be enabled to use sil_sii8620
driver only for HDMI.

The same reasoning applies to INPUT, if we agree that depending on
RC_CORE is confusing for users, then depending on INPUT is confusing as
well. There's not reason to apply different standards to INPUT and
RC_CORE, depending on one and selecting the other doesn't make much
sense.

-- 
Regards,

Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel