[Bug 107153] 4.18-rc3 crash on hdmi (0010:dm_update_crtcs_state+0x41e/0x4a0)

[bugzilla-daemon]

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

--- Comment #16 from Peter  ---
Created attachment 140681
  --> https://bugs.freedesktop.org/attachment.cgi?id=140681=edit
Dmesg with patch minus freesync

The kernel wouldn't build with that patch. It doesn't seem to recognize the
freesync_enable part. Is that in 4.18?

I removed the three Freesync lines from the patch and built 4.18-rc5. I've
attached its dmesg output.


Build error:

drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c: In function
‘dm_update_crtcs_state’:
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4832:29: error:
‘struct dm_crtc_state’ has no member named ‘freesync_enabled’; did you mean
‘crc_enabled’?
  dm_old_crtc_state->freesync_enabled,
 ^~~~
 crc_enabled
drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm.c:4833:29: error:
‘struct dm_crtc_state’ has no member named ‘freesync_enabled’; did you mean
‘crc_enabled’?
  dm_new_crtc_state->freesync_enabled);
 ^~~~
 crc_enabled

-- 
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 200531] amdgpu: *ERROR* REG_WAIT timeout when a display is put to sleep

[bugzilla-daemon]

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

--- Comment #1 from Alexander Mezin (mezin.alexan...@gmail.com) ---
I was wrong, it's not a regression. it's happening since the moment I upgraded
from rx 580 to vega

-- 
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 200531] amdgpu: *ERROR* REG_WAIT timeout when a display is put to sleep

[bugzilla-daemon]

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

Alexander Mezin (mezin.alexan...@gmail.com) changed:

   What|Removed |Added

 Regression|Yes |No

-- 
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 200531] amdgpu: *ERROR* REG_WAIT timeout when a display is put to sleep

[bugzilla-daemon]

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

Alexander Mezin (mezin.alexan...@gmail.com) changed:

   What|Removed |Added

 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 200531] New: amdgpu: *ERROR* REG_WAIT timeout when a display is put to sleep

[bugzilla-daemon]

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

Bug ID: 200531
   Summary: amdgpu: *ERROR* REG_WAIT timeout when a display is put
to sleep
   Product: Drivers
   Version: 2.5
Kernel Version: 4.17.6
  Hardware: x86-64
OS: Linux
  Tree: Mainline
Status: NEW
  Severity: normal
  Priority: P1
 Component: Video(DRI - non Intel)
  Assignee: drivers_video-...@kernel-bugs.osdl.org
  Reporter: mezin.alexan...@gmail.com
Regression: No

Created attachment 277391
  --> https://bugzilla.kernel.org/attachment.cgi?id=277391=edit
dmesg

When a display is turned off (because of inactivity), sometimes I see "*ERROR*
REG_WAIT timeout" from amdgpu in the kernel log. And sometimes when the display
wakes up, GUI doesn't respond anymore (even mouse pointer isn't moving).

full dmesg output attached

card: RX Vega 64 (Sapphire Nitro+)

I think it started with 4.17.5 or 4.17.6, i'll try to find the exact commit

-- 
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

vmwgfx: refuse_hibernation

[Dave Airlie]

There is a member of a struct that doesn't look to be used anywhere.

(just noticed in passing).

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

Re: [Intel-gfx] [PATCH 2/2] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT

[Dhinakaran Pandiyan]

On Tue, 2018-07-17 at 15:34 -0700, Dhinakaran Pandiyan wrote:
> On Tue, 2018-07-17 at 14:49 -0700, matthew.s.atw...@intel.com wrote:
> > 
> > From: Matt Atwood 
> > 
> > According to DP spec (2.9.3.1 of DP 1.4) if
> > EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in
> > DPCD
> > 02200h through 0220Fh shall contain the DPRX's true capability.
> > These
> > values will match 0h through Fh, except for DPCD_REV,
> > MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.
> > 
> > Read from DPCD once for all 3 values as this is an expensive
> > operation.
> > Spec mentions that all of address space 02200h through 0220Fh
> > should
> > contain the right information however currently only 3 values can
> > differ.
> > 
> > There is no address space in the intel_dp->dpcd struct for
> > addresses
> > 02200h through 0220Fh, and since so much of the data is a
> > identical,
> > simply overwrite the values stored in 0h through Fh with
> > the
> > values that can be overwritten from addresses 02200h through
> > 0220Fh.
> > 
> > This patch helps with backward compatibility for devices pre DP1.3.
> > 
> > v2: read only dpcd values which can be affected,
> I still see 6 bytes read and 3 copied.
Ignore this, the original patch was reading 16B. Thanks for clarifying
Matt.

> 
> > 
> >  remove incorrect check,
> > split into drm include changes into separate patch, commit message,
> > verbose debugging statements during overwrite.
> > 
> > Signed-off-by: Matt Atwood 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 37
> > +
> >  1 file changed, 37 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index dde92e4af5d3..364cf41a8b89 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3738,6 +3738,43 @@ intel_dp_read_dpcd(struct intel_dp
> > *intel_dp)
> >      sizeof(intel_dp->dpcd)) < 0)
> >     return false; /* aux transfer failed */
> >  
> > +   if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > +   DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {
> > +   uint8_t dpcd_ext[6];
> > +
> > +   DRM_DEBUG_KMS("DPCD: Extended Receiver Capability
> > Field Present, accessing 02200h through 022FFh\n");
> > +
> > +   if (drm_dp_dpcd_read(_dp->aux,
> > DP_DP13_DPCD_REV,
> > +   _ext, sizeof(dpcd_ext)) < 0)
> > +   return false; /* aux transfer failed */
> > +
> > +   if (memcmp(_dp->dpcd[DP_DPCD_REV],
> > _ext[DP_DPCD_REV],
> > +      sizeof(u8))) 
> Why use memcmp and memcmpy if it's just one byte? You could just use
> "=="
> 

I believe this is what Jani suggested.
if (memcmp(old_dpcd, new_dpcd, sizeof(new_dpcd)) {
DRM_DEBUG_KMS();
memcpy(old_dpcd, new_dpcd, sizeof(new_dpcd);
}

We lose the information about which specific fields in the 6 bytes
changed, but that's okay IMO.

> > 
> > {
> > +   DRM_DEBUG_KMS("DPCD: new value for DPCD
> > Revision previous value %2x new value %2x\n",
> > +     intel_dp->dpcd[DP_DPCD_REV],
> > +     dpcd_ext[DP_DPCD_REV]);
> > +   memcpy(_dp->dpcd[DP_DPCD_REV],
> > +      _ext[DP_DPCD_REV],
> > +      sizeof(u8));
> > +   }
> > +   if (memcmp(_dp->dpcd[DP_MAX_LINK_RATE],
> > +      _ext[DP_MAX_LINK_RATE],
> > sizeof(u8)))
> > {
> > +   DRM_DEBUG_KMS("DPCD: new value for DPCD
> > Max
> > Link Rate previous value %2x new value %2x\n",
> > +     intel_dp-
> > > 
> > > dpcd[DP_MAX_LINK_RATE],
> > +     dpcd_ext[DP_MAX_LINK_RATE]);
> > +   memcpy(_dp->dpcd[DP_MAX_LINK_RATE],
> > +      _ext[DP_MAX_LINK_RATE],
> > sizeof(u8));
> > +   }
> > +   if (memcmp(_dp-
> > > 
> > > dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +      _ext[DP_DOWNSTREAMPORT_PRESENT],
> > sizeof(u8))) {
> > +   DRM_DEBUG_KMS("DPCD: new value for DPCD
> > Downstream Port Present  previous value %2x new value %2x\n",
> > +     intel_dp-
> > > 
> > > dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +     dpcd_ext[DP_DOWNSTREAMPORT_P
> > RE
> > SENT]);
> > +   memcpy(_dp-
> > > 
> > > dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +      _ext[DP_DOWNSTREAMPORT_PRESENT
> > ],
> > +      sizeof(u8));
> > +   }
> > +   }
> >     DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp-
> > >dpcd),
> > intel_dp->dpcd);
> >  
> >     return intel_dp->dpcd[DP_DPCD_REV] != 0;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 2/2] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT

[Rodrigo Vivi]

On Tue, Jul 17, 2018 at 03:34:35PM -0700, Dhinakaran Pandiyan wrote:
> On Tue, 2018-07-17 at 14:49 -0700, matthew.s.atw...@intel.com wrote:
> > From: Matt Atwood 
> > 
> > According to DP spec (2.9.3.1 of DP 1.4) if
> > EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in
> > DPCD
> > 02200h through 0220Fh shall contain the DPRX's true capability. These
> > values will match 0h through Fh, except for DPCD_REV,
> > MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.
> > 
> > Read from DPCD once for all 3 values as this is an expensive
> > operation.
> > Spec mentions that all of address space 02200h through 0220Fh should
> > contain the right information however currently only 3 values can
> > differ.
> > 
> > There is no address space in the intel_dp->dpcd struct for addresses
> > 02200h through 0220Fh, and since so much of the data is a identical,
> > simply overwrite the values stored in 0h through Fh with the
> > values that can be overwritten from addresses 02200h through 0220Fh.
> > 
> > This patch helps with backward compatibility for devices pre DP1.3.
> > 
> > v2: read only dpcd values which can be affected,
> 
> I still see 6 bytes read and 3 copied.

+1

> 
> >  remove incorrect check,
> > split into drm include changes into separate patch, commit message,
> > verbose debugging statements during overwrite.
> > 
> > Signed-off-by: Matt Atwood 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 37
> > +
> >  1 file changed, 37 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index dde92e4af5d3..364cf41a8b89 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3738,6 +3738,43 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
> >      sizeof(intel_dp->dpcd)) < 0)
> >     return false; /* aux transfer failed */
> >  
> > +   if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> > +   DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {
> > +   uint8_t dpcd_ext[6];
> > +
> > +   DRM_DEBUG_KMS("DPCD: Extended Receiver Capability
> > Field Present, accessing 02200h through 022FFh\n");
> > +
> > +   if (drm_dp_dpcd_read(_dp->aux,
> > DP_DP13_DPCD_REV,
> > +   _ext, sizeof(dpcd_ext)) < 0)
> > +   return false; /* aux transfer failed */
> > +
> > +   if (memcmp(_dp->dpcd[DP_DPCD_REV],
> > _ext[DP_DPCD_REV],
> > +      sizeof(u8))) 
> 
> Why use memcmp and memcmpy if it's just one byte? You could just use
> "=="

I think == should work here, but why not memcmp anyways?!

> 
> > {
> > +   DRM_DEBUG_KMS("DPCD: new value for DPCD
> > Revision previous value %2x new value %2x\n",
> > +     intel_dp->dpcd[DP_DPCD_REV],
> > +     dpcd_ext[DP_DPCD_REV]);
> > +   memcpy(_dp->dpcd[DP_DPCD_REV],
> > +      _ext[DP_DPCD_REV],
> > +      sizeof(u8));
> > +   }
> > +   if (memcmp(_dp->dpcd[DP_MAX_LINK_RATE],
> > +      _ext[DP_MAX_LINK_RATE], sizeof(u8)))
> > {
> > +   DRM_DEBUG_KMS("DPCD: new value for DPCD Max
> > Link Rate previous value %2x new value %2x\n",
> > +     intel_dp-
> > >dpcd[DP_MAX_LINK_RATE],
> > +     dpcd_ext[DP_MAX_LINK_RATE]);
> > +   memcpy(_dp->dpcd[DP_MAX_LINK_RATE],
> > +      _ext[DP_MAX_LINK_RATE],
> > sizeof(u8));
> > +   }
> > +   if (memcmp(_dp-
> > >dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +      _ext[DP_DOWNSTREAMPORT_PRESENT],
> > sizeof(u8))) {
> > +   DRM_DEBUG_KMS("DPCD: new value for DPCD
> > Downstream Port Present  previous value %2x new value %2x\n",
> > +     intel_dp-
> > >dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +     dpcd_ext[DP_DOWNSTREAMPORT_PRE
> > SENT]);
> > +   memcpy(_dp-
> > >dpcd[DP_DOWNSTREAMPORT_PRESENT],
> > +      _ext[DP_DOWNSTREAMPORT_PRESENT],
> > +      sizeof(u8));
> > +   }
> > +   }
> >     DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd),
> > intel_dp->dpcd);
> >  
> >     return intel_dp->dpcd[DP_DPCD_REV] != 0;
> ___
> 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: [Intel-gfx] [PATCH 2/2] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT

[Dhinakaran Pandiyan]

On Tue, 2018-07-17 at 14:49 -0700, matthew.s.atw...@intel.com wrote:
> From: Matt Atwood 
> 
> According to DP spec (2.9.3.1 of DP 1.4) if
> EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in
> DPCD
> 02200h through 0220Fh shall contain the DPRX's true capability. These
> values will match 0h through Fh, except for DPCD_REV,
> MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.
> 
> Read from DPCD once for all 3 values as this is an expensive
> operation.
> Spec mentions that all of address space 02200h through 0220Fh should
> contain the right information however currently only 3 values can
> differ.
> 
> There is no address space in the intel_dp->dpcd struct for addresses
> 02200h through 0220Fh, and since so much of the data is a identical,
> simply overwrite the values stored in 0h through Fh with the
> values that can be overwritten from addresses 02200h through 0220Fh.
> 
> This patch helps with backward compatibility for devices pre DP1.3.
> 
> v2: read only dpcd values which can be affected,

I still see 6 bytes read and 3 copied.

>  remove incorrect check,
> split into drm include changes into separate patch, commit message,
> verbose debugging statements during overwrite.
> 
> Signed-off-by: Matt Atwood 
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 37
> +
>  1 file changed, 37 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index dde92e4af5d3..364cf41a8b89 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3738,6 +3738,43 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
>    sizeof(intel_dp->dpcd)) < 0)
>   return false; /* aux transfer failed */
>  
> + if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
> + DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {
> + uint8_t dpcd_ext[6];
> +
> + DRM_DEBUG_KMS("DPCD: Extended Receiver Capability
> Field Present, accessing 02200h through 022FFh\n");
> +
> + if (drm_dp_dpcd_read(_dp->aux,
> DP_DP13_DPCD_REV,
> + _ext, sizeof(dpcd_ext)) < 0)
> + return false; /* aux transfer failed */
> +
> + if (memcmp(_dp->dpcd[DP_DPCD_REV],
> _ext[DP_DPCD_REV],
> +    sizeof(u8))) 

Why use memcmp and memcmpy if it's just one byte? You could just use
"=="

> {
> + DRM_DEBUG_KMS("DPCD: new value for DPCD
> Revision previous value %2x new value %2x\n",
> +   intel_dp->dpcd[DP_DPCD_REV],
> +   dpcd_ext[DP_DPCD_REV]);
> + memcpy(_dp->dpcd[DP_DPCD_REV],
> +    _ext[DP_DPCD_REV],
> +    sizeof(u8));
> + }
> + if (memcmp(_dp->dpcd[DP_MAX_LINK_RATE],
> +    _ext[DP_MAX_LINK_RATE], sizeof(u8)))
> {
> + DRM_DEBUG_KMS("DPCD: new value for DPCD Max
> Link Rate previous value %2x new value %2x\n",
> +   intel_dp-
> >dpcd[DP_MAX_LINK_RATE],
> +   dpcd_ext[DP_MAX_LINK_RATE]);
> + memcpy(_dp->dpcd[DP_MAX_LINK_RATE],
> +    _ext[DP_MAX_LINK_RATE],
> sizeof(u8));
> + }
> + if (memcmp(_dp-
> >dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +    _ext[DP_DOWNSTREAMPORT_PRESENT],
> sizeof(u8))) {
> + DRM_DEBUG_KMS("DPCD: new value for DPCD
> Downstream Port Present  previous value %2x new value %2x\n",
> +   intel_dp-
> >dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +   dpcd_ext[DP_DOWNSTREAMPORT_PRE
> SENT]);
> + memcpy(_dp-
> >dpcd[DP_DOWNSTREAMPORT_PRESENT],
> +    _ext[DP_DOWNSTREAMPORT_PRESENT],
> +    sizeof(u8));
> + }
> + }
>   DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd),
> intel_dp->dpcd);
>  
>   return intel_dp->dpcd[DP_DPCD_REV] != 0;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[resend PATCH v4 5/5] drm: mediatek: Omit warning on probe defers

[matthias . bgg]

From: Matthias Brugger 

It can happen that the clock drivers wasn't probed before the
ddp driver gets invoked. The driver used to omit a warning
that the driver failed to get the clocks. Omit this error on
the defered probe path.

Signed-off-by: Matthias Brugger 
Acked-by: CK Hu 
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c 
b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index bafc5c77c4fb..6b399348a2dc 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -374,7 +374,8 @@ static int mtk_ddp_probe(struct platform_device *pdev)
 
ddp->clk = devm_clk_get(dev, NULL);
if (IS_ERR(ddp->clk)) {
-   dev_err(dev, "Failed to get clock\n");
+   if (PTR_ERR(ddp->clk) != -EPROBE_DEFER)
+   dev_err(dev, "Failed to get clock\n");
return PTR_ERR(ddp->clk);
}
 
-- 
2.17.1

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

[resend PATCH v4 0/5] arm/arm64: mediatek: Fix mmsys device probing

[matthias . bgg]

Changes since v3:
- use platform device to probe clock driver
- add Acked-by CK Hu for the probe deferred patch

Changes since v2:
- fix kconfig typo (shame on me)
- delete __initconst from mm_clocks as converted to a platform driver
  
Changes since v1:
- add binding documentation
- ddp: use regmap_update_bits
- ddp: ignore EPROBE_DEFER on clock probing
- mfd: delete mmsys_private
- add Reviewed-by and Acked-by tags
 
MMSYS in Mediatek SoCs has some registers to control clock gates (which is 
used in the clk driver) and some registers to set the routing and enable
the differnet blocks of the display subsystem.

Up to now both drivers, clock and drm are probed with the same device tree
compatible. But only the first driver get probed, which in effect breaks
graphics on mt8173 and mt2701.

This patch uses a platform device registration in the DRM driver, which
will trigger the probe of the corresponding clock driver. It was tested on the
bananapi-r2 and the Acer R13 Chromebook.


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

[resend PATCH v4 1/5] drm/mediatek: Use regmap for register access

[matthias . bgg]

From: Matthias Brugger 

The mmsys memory space is shared between the drm and the
clk driver. Use regmap to access it.

Signed-off-by: Matthias Brugger 
Reviewed-by: Philipp Zabel 
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  4 +--
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c  | 38 ++---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.h  |  4 +--
 drivers/gpu/drm/mediatek/mtk_drm_drv.c  | 13 +++--
 drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  2 +-
 5 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 658b8dd45b83..4c65873b4867 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -33,7 +33,7 @@
  * @enabled: records whether crtc_enable succeeded
  * @planes: array of 4 drm_plane structures, one for each overlay plane
  * @pending_planes: whether any plane has pending changes to be applied
- * @config_regs: memory mapped mmsys configuration register space
+ * @config_regs: regmap mapped mmsys configuration register space
  * @mutex: handle to one of the ten disp_mutex streams
  * @ddp_comp_nr: number of components in ddp_comp
  * @ddp_comp: array of pointers the mtk_ddp_comp structures used by this crtc
@@ -48,7 +48,7 @@ struct mtk_drm_crtc {
struct drm_planeplanes[OVL_LAYER_NR];
boolpending_planes;
 
-   void __iomem*config_regs;
+   struct regmap   *config_regs;
struct mtk_disp_mutex   *mutex;
unsigned intddp_comp_nr;
struct mtk_ddp_comp **ddp_comp;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c 
b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 8130f3dab661..bafc5c77c4fb 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -185,53 +185,45 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id 
cur,
return value;
 }
 
-static void mtk_ddp_sout_sel(void __iomem *config_regs,
+static void mtk_ddp_sout_sel(struct regmap *config_regs,
 enum mtk_ddp_comp_id cur,
 enum mtk_ddp_comp_id next)
 {
if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0)
-   writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
-  config_regs + DISP_REG_CONFIG_OUT_SEL);
+   regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
+   BLS_TO_DSI_RDMA1_TO_DPI1);
 }
 
-void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
+void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
  enum mtk_ddp_comp_id cur,
  enum mtk_ddp_comp_id next)
 {
-   unsigned int addr, value, reg;
+   unsigned int addr, value;
 
value = mtk_ddp_mout_en(cur, next, );
-   if (value) {
-   reg = readl_relaxed(config_regs + addr) | value;
-   writel_relaxed(reg, config_regs + addr);
-   }
+   if (value)
+   regmap_update_bits(config_regs, addr, value, value);
 
mtk_ddp_sout_sel(config_regs, cur, next);
 
value = mtk_ddp_sel_in(cur, next, );
-   if (value) {
-   reg = readl_relaxed(config_regs + addr) | value;
-   writel_relaxed(reg, config_regs + addr);
-   }
+   if (value)
+   regmap_update_bits(config_regs, addr, value, value);
 }
 
-void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
+void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
   enum mtk_ddp_comp_id cur,
   enum mtk_ddp_comp_id next)
 {
-   unsigned int addr, value, reg;
+   unsigned int addr, value;
 
value = mtk_ddp_mout_en(cur, next, );
-   if (value) {
-   reg = readl_relaxed(config_regs + addr) & ~value;
-   writel_relaxed(reg, config_regs + addr);
-   }
+   if (value)
+   regmap_update_bits(config_regs, addr, value, 0);
 
value = mtk_ddp_sel_in(cur, next, );
-   if (value) {
-   reg = readl_relaxed(config_regs + addr) & ~value;
-   writel_relaxed(reg, config_regs + addr);
-   }
+   if (value)
+   regmap_update_bits(config_regs, addr, value, 0);
 }
 
 struct mtk_disp_mutex *mtk_disp_mutex_get(struct device *dev, unsigned int id)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h 
b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
index f9a799168077..32e12f33b76a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
@@ -20,10 +20,10 @@ struct regmap;
 struct device;
 struct mtk_disp_mutex;
 
-void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
+void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
  enum mtk_ddp_comp_id cur,

[resend PATCH v4 4/5] drm/mediatek: Add support for mmsys through a pdev

[matthias . bgg]

From: Matthias Brugger 

The MMSYS subsystem includes clocks and drm components.
This patch adds an initailization path through a platform device
for the clock part, so that both drivers get probed from the same
device tree compatible.

Signed-off-by: Matthias Brugger 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 18 ++
 drivers/gpu/drm/mediatek/mtk_drm_drv.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index dd249cf5121e..c946aea722e5 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -173,6 +173,7 @@ static const struct mtk_mmsys_driver_data 
mt2701_mmsys_driver_data = {
.ext_path = mt2701_mtk_ddp_ext,
.ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext),
.shadow_register = true,
+   .clk_drv_name = "clk-mt2701-mm",
 };
 
 static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
@@ -180,6 +181,7 @@ static const struct mtk_mmsys_driver_data 
mt8173_mmsys_driver_data = {
.main_len = ARRAY_SIZE(mt8173_mtk_ddp_main),
.ext_path = mt8173_mtk_ddp_ext,
.ext_len = ARRAY_SIZE(mt8173_mtk_ddp_ext),
+   .clk_drv_name = "clk-mt8173-mm",
 };
 
 static int mtk_drm_kms_init(struct drm_device *drm)
@@ -411,6 +413,19 @@ static int mtk_drm_probe(struct platform_device *pdev)
if (IS_ERR(private->config_regs))
return PTR_ERR(private->config_regs);
 
+   if (private->data->clk_drv_name) {
+   private->clk_dev = platform_device_register_data(dev,
+   private->data->clk_drv_name, -1,
+   NULL, 0);
+
+   if (IS_ERR(private->clk_dev)) {
+   pr_err("failed to register %s platform device\n",
+   private->data->clk_drv_name);
+
+   return PTR_ERR(private->clk_dev);
+   }
+   }
+
/* Iterate over sibling DISP function blocks */
for_each_child_of_node(dev->of_node->parent, node) {
const struct of_device_id *of_id;
@@ -515,6 +530,9 @@ static int mtk_drm_remove(struct platform_device *pdev)
for (i = 0; i < DDP_COMPONENT_ID_MAX; i++)
of_node_put(private->comp_node[i]);
 
+   if (private->clk_dev)
+   platform_device_unregister(private->clk_dev);
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 86cec19193c4..200eee5de419 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -34,11 +34,13 @@ struct mtk_mmsys_driver_data {
const enum mtk_ddp_comp_id *ext_path;
unsigned int ext_len;
bool shadow_register;
+   const char *clk_drv_name;
 };
 
 struct mtk_drm_private {
struct drm_device *drm;
struct device *dma_dev;
+   struct platform_device *clk_dev;
 
unsigned int num_pipes;
 
-- 
2.17.1

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

[resend PATCH v4 3/5] clk: mediatek: mt8173: switch mmsys to platform device probing

[matthias . bgg]

From: Matthias Brugger 

Switch probing for the MMSYS to support invocation to a
plain paltform device. The driver will be probed by the DRM subsystem.

Signed-off-by: Matthias Brugger 
---
 drivers/clk/mediatek/clk-mt8173.c | 51 ++-
 1 file changed, 44 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8173.c 
b/drivers/clk/mediatek/clk-mt8173.c
index 96c292c3e440..10b6a0251123 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -13,8 +13,11 @@
  */
 
 #include 
+#include 
 #include 
 #include 
+#include 
+#include 
 
 #include "clk-mtk.h"
 #include "clk-gate.h"
@@ -791,7 +794,7 @@ static const struct mtk_gate_regs mm1_cg_regs __initconst = 
{
.ops = _clk_gate_ops_setclr,\
}
 
-static const struct mtk_gate mm_clks[] __initconst = {
+static const struct mtk_gate mm_clks[] = {
/* MM0 */
GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
@@ -1152,22 +1155,56 @@ static void __init mtk_imgsys_init(struct device_node 
*node)
 }
 CLK_OF_DECLARE(mtk_imgsys, "mediatek,mt8173-imgsys", mtk_imgsys_init);
 
-static void __init mtk_mmsys_init(struct device_node *node)
-{
+struct mtk_mmsys_priv {
struct clk_onecell_data *clk_data;
+};
+
+static int mtk_mmsys_probe(struct platform_device *pdev)
+{
int r;
+   struct device_node *node;
+   struct mtk_mmsys_priv *private;
+
+   node = pdev->dev.parent->of_node;
 
-   clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
+   private = devm_kzalloc(>dev, sizeof(*private), GFP_KERNEL);
+   if (!private)
+   return -ENOMEM;
+
+   private->clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
+
+   platform_set_drvdata(pdev, private);
 
mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
-   clk_data);
+   private->clk_data);
 
-   r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+   r = of_clk_add_provider(node, of_clk_src_onecell_get,
+   private->clk_data);
if (r)
pr_err("%s(): could not register clock provider: %d\n",
__func__, r);
+
+   return r;
+}
+
+static int mtk_mmsys_remove(struct platform_device *pdev)
+{
+   struct mtk_mmsys_priv *private = platform_get_drvdata(pdev);
+
+   kfree(private->clk_data);
+   kfree(private);
+
+   return 0;
 }
-CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys", mtk_mmsys_init);
+
+static struct platform_driver clk_mt8173_mm_drv = {
+   .probe = mtk_mmsys_probe,
+   .probe = mtk_mmsys_remove,
+   .driver = {
+   .name = "clk-mt8173-mm",
+   },
+};
+module_platform_driver(clk_mt8173_mm_drv);
 
 static void __init mtk_vdecsys_init(struct device_node *node)
 {
-- 
2.17.1

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

[resend PATCH v4 2/5] clk: mediatek: mt2701-mmsys: switch to platform device probing

[matthias . bgg]

From: Matthias Brugger 

Switch probing for the MMSYS to support invocation to a plain
paltform device. The driver will be probed by the DRM subsystem.

Signed-off-by: Matthias Brugger 
---
 drivers/clk/mediatek/clk-mt2701-mm.c | 42 
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt2701-mm.c 
b/drivers/clk/mediatek/clk-mt2701-mm.c
index fe1f85072fc5..200b1842b94b 100644
--- a/drivers/clk/mediatek/clk-mt2701-mm.c
+++ b/drivers/clk/mediatek/clk-mt2701-mm.c
@@ -12,14 +12,20 @@
  * GNU General Public License for more details.
  */
 
+#include 
 #include 
 #include 
+#include 
 
 #include "clk-mtk.h"
 #include "clk-gate.h"
 
 #include 
 
+struct clk_mt2701_mm_priv {
+   struct clk_onecell_data *clk_data;
+};
+
 static const struct mtk_gate_regs disp0_cg_regs = {
.set_ofs = 0x0104,
.clr_ofs = 0x0108,
@@ -87,23 +93,25 @@ static const struct mtk_gate mm_clks[] = {
GATE_DISP1(CLK_MM_TVE_FMM, "mm_tve_fmm", "mm_sel", 14),
 };
 
-static const struct of_device_id of_match_clk_mt2701_mm[] = {
-   { .compatible = "mediatek,mt2701-mmsys", },
-   {}
-};
-
 static int clk_mt2701_mm_probe(struct platform_device *pdev)
 {
-   struct clk_onecell_data *clk_data;
int r;
-   struct device_node *node = pdev->dev.of_node;
+   struct device_node *node = pdev->dev.parent->of_node;
+   struct clk_mt2701_mm_priv *private;
+
+   private = devm_kzalloc(>dev, sizeof(*private), GFP_KERNEL);
+   if (!private)
+   return -ENOMEM;
 
-   clk_data = mtk_alloc_clk_data(CLK_MM_NR);
+   private->clk_data = mtk_alloc_clk_data(CLK_MM_NR);
+
+   platform_set_drvdata(pdev, private);
 
mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
-   clk_data);
+   private->clk_data);
 
-   r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+   r = of_clk_add_provider(node, of_clk_src_onecell_get,
+   private->clk_data);
if (r)
dev_err(>dev,
"could not register clock provider: %s: %d\n",
@@ -112,12 +120,22 @@ static int clk_mt2701_mm_probe(struct platform_device 
*pdev)
return r;
 }
 
+static int clk_mt2701_mm_remove(struct platform_device *pdev)
+{
+   struct clk_mt2701_mm_priv *private = platform_get_drvdata(pdev);
+
+   kfree(private->clk_data);
+   kfree(private);
+
+   return 0;
+}
+
 static struct platform_driver clk_mt2701_mm_drv = {
.probe = clk_mt2701_mm_probe,
+   .remove = clk_mt2701_mm_remove,
.driver = {
.name = "clk-mt2701-mm",
-   .of_match_table = of_match_clk_mt2701_mm,
},
 };
 
-builtin_platform_driver(clk_mt2701_mm_drv);
+module_platform_driver(clk_mt2701_mm_drv);
-- 
2.17.1

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

[PATCH v4--to=ulrich.hecht+rene...@gmail.com 5/5] drm: mediatek: Omit warning on probe defers

[matthias . bgg]

From: Matthias Brugger 

It can happen that the clock drivers wasn't probed before the
ddp driver gets invoked. The driver used to omit a warning
that the driver failed to get the clocks. Omit this error on
the defered probe path.

Signed-off-by: Matthias Brugger 
Acked-by: CK Hu 
---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c 
b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index bafc5c77c4fb..6b399348a2dc 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -374,7 +374,8 @@ static int mtk_ddp_probe(struct platform_device *pdev)
 
ddp->clk = devm_clk_get(dev, NULL);
if (IS_ERR(ddp->clk)) {
-   dev_err(dev, "Failed to get clock\n");
+   if (PTR_ERR(ddp->clk) != -EPROBE_DEFER)
+   dev_err(dev, "Failed to get clock\n");
return PTR_ERR(ddp->clk);
}
 
-- 
2.17.1

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

[PATCH v4--to=ulrich.hecht+rene...@gmail.com 2/5] clk: mediatek: mt2701-mmsys: switch to platform device probing

[matthias . bgg]

From: Matthias Brugger 

Switch probing for the MMSYS to support invocation to a plain
paltform device. The driver will be probed by the DRM subsystem.

Signed-off-by: Matthias Brugger 
---
 drivers/clk/mediatek/clk-mt2701-mm.c | 42 
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt2701-mm.c 
b/drivers/clk/mediatek/clk-mt2701-mm.c
index fe1f85072fc5..200b1842b94b 100644
--- a/drivers/clk/mediatek/clk-mt2701-mm.c
+++ b/drivers/clk/mediatek/clk-mt2701-mm.c
@@ -12,14 +12,20 @@
  * GNU General Public License for more details.
  */
 
+#include 
 #include 
 #include 
+#include 
 
 #include "clk-mtk.h"
 #include "clk-gate.h"
 
 #include 
 
+struct clk_mt2701_mm_priv {
+   struct clk_onecell_data *clk_data;
+};
+
 static const struct mtk_gate_regs disp0_cg_regs = {
.set_ofs = 0x0104,
.clr_ofs = 0x0108,
@@ -87,23 +93,25 @@ static const struct mtk_gate mm_clks[] = {
GATE_DISP1(CLK_MM_TVE_FMM, "mm_tve_fmm", "mm_sel", 14),
 };
 
-static const struct of_device_id of_match_clk_mt2701_mm[] = {
-   { .compatible = "mediatek,mt2701-mmsys", },
-   {}
-};
-
 static int clk_mt2701_mm_probe(struct platform_device *pdev)
 {
-   struct clk_onecell_data *clk_data;
int r;
-   struct device_node *node = pdev->dev.of_node;
+   struct device_node *node = pdev->dev.parent->of_node;
+   struct clk_mt2701_mm_priv *private;
+
+   private = devm_kzalloc(>dev, sizeof(*private), GFP_KERNEL);
+   if (!private)
+   return -ENOMEM;
 
-   clk_data = mtk_alloc_clk_data(CLK_MM_NR);
+   private->clk_data = mtk_alloc_clk_data(CLK_MM_NR);
+
+   platform_set_drvdata(pdev, private);
 
mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
-   clk_data);
+   private->clk_data);
 
-   r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+   r = of_clk_add_provider(node, of_clk_src_onecell_get,
+   private->clk_data);
if (r)
dev_err(>dev,
"could not register clock provider: %s: %d\n",
@@ -112,12 +120,22 @@ static int clk_mt2701_mm_probe(struct platform_device 
*pdev)
return r;
 }
 
+static int clk_mt2701_mm_remove(struct platform_device *pdev)
+{
+   struct clk_mt2701_mm_priv *private = platform_get_drvdata(pdev);
+
+   kfree(private->clk_data);
+   kfree(private);
+
+   return 0;
+}
+
 static struct platform_driver clk_mt2701_mm_drv = {
.probe = clk_mt2701_mm_probe,
+   .remove = clk_mt2701_mm_remove,
.driver = {
.name = "clk-mt2701-mm",
-   .of_match_table = of_match_clk_mt2701_mm,
},
 };
 
-builtin_platform_driver(clk_mt2701_mm_drv);
+module_platform_driver(clk_mt2701_mm_drv);
-- 
2.17.1

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

[PATCH v4--to=ulrich.hecht+rene...@gmail.com 3/5] clk: mediatek: mt8173: switch mmsys to platform device probing

[matthias . bgg]

From: Matthias Brugger 

Switch probing for the MMSYS to support invocation to a
plain paltform device. The driver will be probed by the DRM subsystem.

Signed-off-by: Matthias Brugger 
---
 drivers/clk/mediatek/clk-mt8173.c | 51 ++-
 1 file changed, 44 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8173.c 
b/drivers/clk/mediatek/clk-mt8173.c
index 96c292c3e440..10b6a0251123 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -13,8 +13,11 @@
  */
 
 #include 
+#include 
 #include 
 #include 
+#include 
+#include 
 
 #include "clk-mtk.h"
 #include "clk-gate.h"
@@ -791,7 +794,7 @@ static const struct mtk_gate_regs mm1_cg_regs __initconst = 
{
.ops = _clk_gate_ops_setclr,\
}
 
-static const struct mtk_gate mm_clks[] __initconst = {
+static const struct mtk_gate mm_clks[] = {
/* MM0 */
GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
@@ -1152,22 +1155,56 @@ static void __init mtk_imgsys_init(struct device_node 
*node)
 }
 CLK_OF_DECLARE(mtk_imgsys, "mediatek,mt8173-imgsys", mtk_imgsys_init);
 
-static void __init mtk_mmsys_init(struct device_node *node)
-{
+struct mtk_mmsys_priv {
struct clk_onecell_data *clk_data;
+};
+
+static int mtk_mmsys_probe(struct platform_device *pdev)
+{
int r;
+   struct device_node *node;
+   struct mtk_mmsys_priv *private;
+
+   node = pdev->dev.parent->of_node;
 
-   clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
+   private = devm_kzalloc(>dev, sizeof(*private), GFP_KERNEL);
+   if (!private)
+   return -ENOMEM;
+
+   private->clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
+
+   platform_set_drvdata(pdev, private);
 
mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
-   clk_data);
+   private->clk_data);
 
-   r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+   r = of_clk_add_provider(node, of_clk_src_onecell_get,
+   private->clk_data);
if (r)
pr_err("%s(): could not register clock provider: %d\n",
__func__, r);
+
+   return r;
+}
+
+static int mtk_mmsys_remove(struct platform_device *pdev)
+{
+   struct mtk_mmsys_priv *private = platform_get_drvdata(pdev);
+
+   kfree(private->clk_data);
+   kfree(private);
+
+   return 0;
 }
-CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys", mtk_mmsys_init);
+
+static struct platform_driver clk_mt8173_mm_drv = {
+   .probe = mtk_mmsys_probe,
+   .probe = mtk_mmsys_remove,
+   .driver = {
+   .name = "clk-mt8173-mm",
+   },
+};
+module_platform_driver(clk_mt8173_mm_drv);
 
 static void __init mtk_vdecsys_init(struct device_node *node)
 {
-- 
2.17.1

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

[PATCH v4--to=ulrich.hecht+rene...@gmail.com 4/5] drm/mediatek: Add support for mmsys through a pdev

[matthias . bgg]

From: Matthias Brugger 

The MMSYS subsystem includes clocks and drm components.
This patch adds an initailization path through a platform device
for the clock part, so that both drivers get probed from the same
device tree compatible.

Signed-off-by: Matthias Brugger 
---
 drivers/gpu/drm/mediatek/mtk_drm_drv.c | 18 ++
 drivers/gpu/drm/mediatek/mtk_drm_drv.h |  2 ++
 2 files changed, 20 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
index dd249cf5121e..c946aea722e5 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
@@ -173,6 +173,7 @@ static const struct mtk_mmsys_driver_data 
mt2701_mmsys_driver_data = {
.ext_path = mt2701_mtk_ddp_ext,
.ext_len = ARRAY_SIZE(mt2701_mtk_ddp_ext),
.shadow_register = true,
+   .clk_drv_name = "clk-mt2701-mm",
 };
 
 static const struct mtk_mmsys_driver_data mt8173_mmsys_driver_data = {
@@ -180,6 +181,7 @@ static const struct mtk_mmsys_driver_data 
mt8173_mmsys_driver_data = {
.main_len = ARRAY_SIZE(mt8173_mtk_ddp_main),
.ext_path = mt8173_mtk_ddp_ext,
.ext_len = ARRAY_SIZE(mt8173_mtk_ddp_ext),
+   .clk_drv_name = "clk-mt8173-mm",
 };
 
 static int mtk_drm_kms_init(struct drm_device *drm)
@@ -411,6 +413,19 @@ static int mtk_drm_probe(struct platform_device *pdev)
if (IS_ERR(private->config_regs))
return PTR_ERR(private->config_regs);
 
+   if (private->data->clk_drv_name) {
+   private->clk_dev = platform_device_register_data(dev,
+   private->data->clk_drv_name, -1,
+   NULL, 0);
+
+   if (IS_ERR(private->clk_dev)) {
+   pr_err("failed to register %s platform device\n",
+   private->data->clk_drv_name);
+
+   return PTR_ERR(private->clk_dev);
+   }
+   }
+
/* Iterate over sibling DISP function blocks */
for_each_child_of_node(dev->of_node->parent, node) {
const struct of_device_id *of_id;
@@ -515,6 +530,9 @@ static int mtk_drm_remove(struct platform_device *pdev)
for (i = 0; i < DDP_COMPONENT_ID_MAX; i++)
of_node_put(private->comp_node[i]);
 
+   if (private->clk_dev)
+   platform_device_unregister(private->clk_dev);
+
return 0;
 }
 
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h 
b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
index 86cec19193c4..200eee5de419 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h
@@ -34,11 +34,13 @@ struct mtk_mmsys_driver_data {
const enum mtk_ddp_comp_id *ext_path;
unsigned int ext_len;
bool shadow_register;
+   const char *clk_drv_name;
 };
 
 struct mtk_drm_private {
struct drm_device *drm;
struct device *dma_dev;
+   struct platform_device *clk_dev;
 
unsigned int num_pipes;
 
-- 
2.17.1

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

[PATCH v4--to=ulrich.hecht+rene...@gmail.com 1/5] drm/mediatek: Use regmap for register access

[matthias . bgg]

From: Matthias Brugger 

The mmsys memory space is shared between the drm and the
clk driver. Use regmap to access it.

Signed-off-by: Matthias Brugger 
Reviewed-by: Philipp Zabel 
---
 drivers/gpu/drm/mediatek/mtk_drm_crtc.c |  4 +--
 drivers/gpu/drm/mediatek/mtk_drm_ddp.c  | 38 ++---
 drivers/gpu/drm/mediatek/mtk_drm_ddp.h  |  4 +--
 drivers/gpu/drm/mediatek/mtk_drm_drv.c  | 13 +++--
 drivers/gpu/drm/mediatek/mtk_drm_drv.h  |  2 +-
 5 files changed, 24 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c 
b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
index 658b8dd45b83..4c65873b4867 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_crtc.c
@@ -33,7 +33,7 @@
  * @enabled: records whether crtc_enable succeeded
  * @planes: array of 4 drm_plane structures, one for each overlay plane
  * @pending_planes: whether any plane has pending changes to be applied
- * @config_regs: memory mapped mmsys configuration register space
+ * @config_regs: regmap mapped mmsys configuration register space
  * @mutex: handle to one of the ten disp_mutex streams
  * @ddp_comp_nr: number of components in ddp_comp
  * @ddp_comp: array of pointers the mtk_ddp_comp structures used by this crtc
@@ -48,7 +48,7 @@ struct mtk_drm_crtc {
struct drm_planeplanes[OVL_LAYER_NR];
boolpending_planes;
 
-   void __iomem*config_regs;
+   struct regmap   *config_regs;
struct mtk_disp_mutex   *mutex;
unsigned intddp_comp_nr;
struct mtk_ddp_comp **ddp_comp;
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c 
b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
index 8130f3dab661..bafc5c77c4fb 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.c
@@ -185,53 +185,45 @@ static unsigned int mtk_ddp_sel_in(enum mtk_ddp_comp_id 
cur,
return value;
 }
 
-static void mtk_ddp_sout_sel(void __iomem *config_regs,
+static void mtk_ddp_sout_sel(struct regmap *config_regs,
 enum mtk_ddp_comp_id cur,
 enum mtk_ddp_comp_id next)
 {
if (cur == DDP_COMPONENT_BLS && next == DDP_COMPONENT_DSI0)
-   writel_relaxed(BLS_TO_DSI_RDMA1_TO_DPI1,
-  config_regs + DISP_REG_CONFIG_OUT_SEL);
+   regmap_write(config_regs, DISP_REG_CONFIG_OUT_SEL,
+   BLS_TO_DSI_RDMA1_TO_DPI1);
 }
 
-void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
+void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
  enum mtk_ddp_comp_id cur,
  enum mtk_ddp_comp_id next)
 {
-   unsigned int addr, value, reg;
+   unsigned int addr, value;
 
value = mtk_ddp_mout_en(cur, next, );
-   if (value) {
-   reg = readl_relaxed(config_regs + addr) | value;
-   writel_relaxed(reg, config_regs + addr);
-   }
+   if (value)
+   regmap_update_bits(config_regs, addr, value, value);
 
mtk_ddp_sout_sel(config_regs, cur, next);
 
value = mtk_ddp_sel_in(cur, next, );
-   if (value) {
-   reg = readl_relaxed(config_regs + addr) | value;
-   writel_relaxed(reg, config_regs + addr);
-   }
+   if (value)
+   regmap_update_bits(config_regs, addr, value, value);
 }
 
-void mtk_ddp_remove_comp_from_path(void __iomem *config_regs,
+void mtk_ddp_remove_comp_from_path(struct regmap *config_regs,
   enum mtk_ddp_comp_id cur,
   enum mtk_ddp_comp_id next)
 {
-   unsigned int addr, value, reg;
+   unsigned int addr, value;
 
value = mtk_ddp_mout_en(cur, next, );
-   if (value) {
-   reg = readl_relaxed(config_regs + addr) & ~value;
-   writel_relaxed(reg, config_regs + addr);
-   }
+   if (value)
+   regmap_update_bits(config_regs, addr, value, 0);
 
value = mtk_ddp_sel_in(cur, next, );
-   if (value) {
-   reg = readl_relaxed(config_regs + addr) & ~value;
-   writel_relaxed(reg, config_regs + addr);
-   }
+   if (value)
+   regmap_update_bits(config_regs, addr, value, 0);
 }
 
 struct mtk_disp_mutex *mtk_disp_mutex_get(struct device *dev, unsigned int id)
diff --git a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h 
b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
index f9a799168077..32e12f33b76a 100644
--- a/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
+++ b/drivers/gpu/drm/mediatek/mtk_drm_ddp.h
@@ -20,10 +20,10 @@ struct regmap;
 struct device;
 struct mtk_disp_mutex;
 
-void mtk_ddp_add_comp_to_path(void __iomem *config_regs,
+void mtk_ddp_add_comp_to_path(struct regmap *config_regs,
  enum mtk_ddp_comp_id cur,

arm/arm64: mediatek: Fix mmsys device probing

[matthias . bgg]

Changes since v3:
- use platform device to probe clock driver
- add Acked-by CK Hu for the probe deferred patch

Changes since v2:
- fix kconfig typo (shame on me)
- delete __initconst from mm_clocks as converted to a platform driver
  
Changes since v1:
- add binding documentation
- ddp: use regmap_update_bits
- ddp: ignore EPROBE_DEFER on clock probing
- mfd: delete mmsys_private
- add Reviewed-by and Acked-by tags
 
MMSYS in Mediatek SoCs has some registers to control clock gates (which is 
used in the clk driver) and some registers to set the routing and enable
the differnet blocks of the display subsystem.

Up to now both drivers, clock and drm are probed with the same device tree
compatible. But only the first driver get probed, which in effect breaks
graphics on mt8173 and mt2701.

This patch uses a platform device registration in the DRM driver, which
will trigger the probe of the corresponding clock driver. It was tested on
the bananapi-r2 and the Acer R13 Chromebook.



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

[PATCH 2/2] drm/i915: implement EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT

[matthew . s . atwood]

From: Matt Atwood 

According to DP spec (2.9.3.1 of DP 1.4) if
EXTENDED_RECEIVER_CAPABILITY_FIELD_PRESENT is set the addresses in DPCD
02200h through 0220Fh shall contain the DPRX's true capability. These
values will match 0h through Fh, except for DPCD_REV,
MAX_LINK_RATE, DOWN_STREAM_PORT_PRESENT.

Read from DPCD once for all 3 values as this is an expensive operation.
Spec mentions that all of address space 02200h through 0220Fh should
contain the right information however currently only 3 values can
differ.

There is no address space in the intel_dp->dpcd struct for addresses
02200h through 0220Fh, and since so much of the data is a identical,
simply overwrite the values stored in 0h through Fh with the
values that can be overwritten from addresses 02200h through 0220Fh.

This patch helps with backward compatibility for devices pre DP1.3.

v2: read only dpcd values which can be affected, remove incorrect check,
split into drm include changes into separate patch, commit message,
verbose debugging statements during overwrite.

Signed-off-by: Matt Atwood 
---
 drivers/gpu/drm/i915/intel_dp.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index dde92e4af5d3..364cf41a8b89 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3738,6 +3738,43 @@ intel_dp_read_dpcd(struct intel_dp *intel_dp)
 sizeof(intel_dp->dpcd)) < 0)
return false; /* aux transfer failed */
 
+   if (intel_dp->dpcd[DP_TRAINING_AUX_RD_INTERVAL] &
+   DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT) {
+   uint8_t dpcd_ext[6];
+
+   DRM_DEBUG_KMS("DPCD: Extended Receiver Capability Field 
Present, accessing 02200h through 022FFh\n");
+
+   if (drm_dp_dpcd_read(_dp->aux, DP_DP13_DPCD_REV,
+   _ext, sizeof(dpcd_ext)) < 0)
+   return false; /* aux transfer failed */
+
+   if (memcmp(_dp->dpcd[DP_DPCD_REV], _ext[DP_DPCD_REV],
+  sizeof(u8))) {
+   DRM_DEBUG_KMS("DPCD: new value for DPCD Revision 
previous value %2x new value %2x\n",
+ intel_dp->dpcd[DP_DPCD_REV],
+ dpcd_ext[DP_DPCD_REV]);
+   memcpy(_dp->dpcd[DP_DPCD_REV],
+  _ext[DP_DPCD_REV],
+  sizeof(u8));
+   }
+   if (memcmp(_dp->dpcd[DP_MAX_LINK_RATE],
+  _ext[DP_MAX_LINK_RATE], sizeof(u8))) {
+   DRM_DEBUG_KMS("DPCD: new value for DPCD Max Link Rate 
previous value %2x new value %2x\n",
+ intel_dp->dpcd[DP_MAX_LINK_RATE],
+ dpcd_ext[DP_MAX_LINK_RATE]);
+   memcpy(_dp->dpcd[DP_MAX_LINK_RATE],
+  _ext[DP_MAX_LINK_RATE], sizeof(u8));
+   }
+   if (memcmp(_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
+  _ext[DP_DOWNSTREAMPORT_PRESENT], sizeof(u8))) {
+   DRM_DEBUG_KMS("DPCD: new value for DPCD Downstream Port 
Present  previous value %2x new value %2x\n",
+ intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
+ dpcd_ext[DP_DOWNSTREAMPORT_PRESENT]);
+   memcpy(_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT],
+  _ext[DP_DOWNSTREAMPORT_PRESENT],
+  sizeof(u8));
+   }
+   }
DRM_DEBUG_KMS("DPCD: %*ph\n", (int) sizeof(intel_dp->dpcd), 
intel_dp->dpcd);
 
return intel_dp->dpcd[DP_DPCD_REV] != 0;
-- 
2.17.1

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

[PATCH 1/2] drm/dp: add extended receiver capability field present bit

[matthew . s . atwood]

From: Matt Atwood 

This bit was added to DP Training Aux RD interval sometime between DP
1.2 and DP 1.3. Via description of the spec this field indicates the
panels true capabilities are described in DPCD address space 02200h
through 022FFh.

Signed-off-by: Matt Atwood 
---
 include/drm/drm_dp_helper.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index c01564991a9f..757bd5913f3d 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -123,8 +123,9 @@
 # define DP_FRAMING_CHANGE_CAP (1 << 1)
 # define DP_DPCD_DISPLAY_CONTROL_CAPABLE (1 << 3) /* edp v1.2 or higher */
 
-#define DP_TRAINING_AUX_RD_INTERVAL 0x00e   /* XXX 1.2? */
-# define DP_TRAINING_AUX_RD_MASK0x7F/* XXX 1.2? */
+#define DP_TRAINING_AUX_RD_INTERVAL 0x00e   /* XXX 1.2? */
+# define DP_TRAINING_AUX_RD_MASK0x7F/* XXX 1.2? */
+# define DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT (1 << 7)/* XXX 1.2? */
 
 #define DP_ADAPTER_CAP 0x00f   /* 1.2 */
 # define DP_FORCE_LOAD_SENSE_CAP   (1 << 0)
-- 
2.17.1

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

[Bug 107153] 4.18-rc3 crash on hdmi (0010:dm_update_crtcs_state+0x41e/0x4a0)

[bugzilla-daemon]

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

Leo Li  changed:

   What|Removed |Added

 Attachment #140677|0   |1
is obsolete||

--- Comment #15 from Leo Li  ---
Created attachment 140678
  --> https://bugs.freedesktop.org/attachment.cgi?id=140678=edit
[Patch] BUG_ON debug prints

-- 
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 107153] 4.18-rc3 crash on hdmi (0010:dm_update_crtcs_state+0x41e/0x4a0)

[bugzilla-daemon]

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

Leo Li  changed:

   What|Removed |Added

 Attachment #140613|0   |1
is obsolete||
 Attachment #140614|0   |1
is obsolete||

--- Comment #14 from Leo Li  ---
Created attachment 140677
  --> https://bugs.freedesktop.org/attachment.cgi?id=140677=edit
[Patch] BUG_ON debug prints

I'm having trouble reproducing this issue, probably because I don't have access
to a newer Onkyo receiver. It seems that's the common factor thus far.

Could you help me gather some debug info? I've attached a patch that dumps some
data to dmesg. Once it reproduces, please upload the entire dmesg 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 v2 5/5] drm/vkms: Implement CRC debugfs API

[Sean Paul]

On Tue, Jul 17, 2018 at 04:43:16PM -0400, Sean Paul wrote:
> On Tue, Jul 17, 2018 at 02:52:20AM +0300, Haneen Mohammed wrote:
> > On Mon, Jul 16, 2018 at 02:22:56PM -0400, Sean Paul wrote:
> > > On Sat, Jul 14, 2018 at 03:23:32PM +0300, Haneen Mohammed wrote:
> > > > Implement the set_crc_source() callback.
> > > > Compute CRC using crc32 on the visible part of the framebuffer.
> > > > Use ordered workqueue to compute and add CRC at the end of a vblank.
> > > > 
> > > > Use appropriate synchronization methods since the crc computation must
> > > > be atomic wrt the generated vblank event for a given atomic update,
> > > > 
> > > > Signed-off-by: Haneen Mohammed 
> > > 
> > > Hey Haneen,
> > > Thanks for revising this patch set. Things are looking good across the 
> > > series,
> > > just a few more comments :-)
> > > 
> > 
> > Thank you so much for the review! 
> > 
> > > > ---
> > > > Changes in v2:
> > > > - Include this patch in this patchset.
> > > > 
> > > >  drivers/gpu/drm/vkms/Makefile |  2 +-
> > > >  drivers/gpu/drm/vkms/vkms_crtc.c  | 60 ++-
> > > >  drivers/gpu/drm/vkms/vkms_drv.c   |  1 +
> > > >  drivers/gpu/drm/vkms/vkms_drv.h   | 21 +++
> > > >  drivers/gpu/drm/vkms/vkms_plane.c | 10 ++
> > > >  5 files changed, 85 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/vkms/Makefile 
> > > > b/drivers/gpu/drm/vkms/Makefile
> > > > index 986297da51bf..37966914f70b 100644
> > > > --- a/drivers/gpu/drm/vkms/Makefile
> > > > +++ b/drivers/gpu/drm/vkms/Makefile
> > > > @@ -1,3 +1,3 @@
> > > > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> > > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o 
> > > > vkms_crc.o
> > > >  
> > > >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c 
> > > > b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > index 26babb85ca77..f3a674db33b8 100644
> > > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > > @@ -10,18 +10,36 @@
> > > >  #include 
> > > >  #include 
> > > >  
> > > > -static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
> > > > +static void _vblank_handle(struct vkms_output *output)
> > > >  {
> > > > -   struct vkms_output *output = container_of(timer, struct 
> > > > vkms_output,
> > > > - vblank_hrtimer);
> > > > struct drm_crtc *crtc = >crtc;
> > > > -   int ret_overrun;
> > > > +   struct vkms_crtc_state *state = to_vkms_crtc_state(crtc->state);
> > > > bool ret;
> > > >  
> > > > +   int crc_enabled = 0;
> > > > +
> > > > +   spin_lock(>lock);
> > > > +   crc_enabled = output->crc_enabled;
> > > 
> > > Aside from the implicit bool -> int cast, I don't think you need this 
> > > local var,
> > > just use output->crc_enabled directly below.
> > > 
> > > > ret = drm_crtc_handle_vblank(crtc);
> > > > if (!ret)
> > > > DRM_ERROR("vkms failure on handling vblank");
> > > 
> > > This would be more useful with the error code printed.
> > > 
> > 
> > I think this only returns false on failure. Also I've noticed most of the 
> > usage of
> > drm_crtc_handle_vblank don't check the return value, should I do the
> > same as well and drop ret and error message?
> 
> Ahh, I didn't see that ret was bool. In that case, the error message is fine.
> It's always good practice to assume that if a function returns a status, it's
> worth checking.
> 
> > 
> > > >  
> > > > +   if (state && crc_enabled) {
> > > > +   state->n_frame = drm_crtc_accurate_vblank_count(crtc);
> > > > +   queue_work(output->crc_workq, >crc_work);
> > > > +   }
> > > > +
> > > > +   spin_unlock(>lock);
> > > > +}
> > > > +
> > > > +static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
> > > > +{
> > > > +   struct vkms_output *output = container_of(timer, struct 
> > > > vkms_output,
> > > > + vblank_hrtimer);
> > > > +   int ret_overrun;
> > > > +
> > > > +   _vblank_handle(output);
> > > > +
> > > > ret_overrun = hrtimer_forward_now(>vblank_hrtimer,
> > > >   output->period_ns);
> > > >  
> > > > @@ -97,6 +115,8 @@ vkms_atomic_crtc_duplicate_state(struct drm_crtc 
> > > > *crtc)
> > > >  
> > > > __drm_atomic_helper_crtc_duplicate_state(crtc, 
> > > > _state->base);
> > > >  
> > > > +   INIT_WORK(_state->crc_work, vkms_crc_work_handle);
> > > > +
> > > > return _state->base;
> > > >  }
> > > >  
> > > > @@ -104,11 +124,18 @@ static void vkms_atomic_crtc_destroy_state(struct 
> > > > drm_crtc *crtc,
> > > >struct drm_crtc_state *state)
> > > >  {
> > > > struct vkms_crtc_state *vkms_state;
> > > > +   struct vkms_output *vkms_out = 

[Bug 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #11 from bakarichar...@gmail.com ---
I played Vietcong but the GPU hasn't switched, is that okay? Is there any way
to monitor frequencies? However temperature seems to be OK now.

-- 
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 107266] Radeon Pro Duo (Polaris) - ring sdma0 timeout

[bugzilla-daemon]

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

--- Comment #1 from Alex Deucher  ---
Please attach your full dmesg output.  Are you attempting to use ROCm over the
drm-next-4.19-wip kernel?

-- 
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 5/5] drm/vkms: Implement CRC debugfs API

[Sean Paul]

On Tue, Jul 17, 2018 at 02:52:20AM +0300, Haneen Mohammed wrote:
> On Mon, Jul 16, 2018 at 02:22:56PM -0400, Sean Paul wrote:
> > On Sat, Jul 14, 2018 at 03:23:32PM +0300, Haneen Mohammed wrote:
> > > Implement the set_crc_source() callback.
> > > Compute CRC using crc32 on the visible part of the framebuffer.
> > > Use ordered workqueue to compute and add CRC at the end of a vblank.
> > > 
> > > Use appropriate synchronization methods since the crc computation must
> > > be atomic wrt the generated vblank event for a given atomic update,
> > > 
> > > Signed-off-by: Haneen Mohammed 
> > 
> > Hey Haneen,
> > Thanks for revising this patch set. Things are looking good across the 
> > series,
> > just a few more comments :-)
> > 
> 
> Thank you so much for the review! 
> 
> > > ---
> > > Changes in v2:
> > > - Include this patch in this patchset.
> > > 
> > >  drivers/gpu/drm/vkms/Makefile |  2 +-
> > >  drivers/gpu/drm/vkms/vkms_crtc.c  | 60 ++-
> > >  drivers/gpu/drm/vkms/vkms_drv.c   |  1 +
> > >  drivers/gpu/drm/vkms/vkms_drv.h   | 21 +++
> > >  drivers/gpu/drm/vkms/vkms_plane.c | 10 ++
> > >  5 files changed, 85 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> > > index 986297da51bf..37966914f70b 100644
> > > --- a/drivers/gpu/drm/vkms/Makefile
> > > +++ b/drivers/gpu/drm/vkms/Makefile
> > > @@ -1,3 +1,3 @@
> > > -vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o
> > > +vkms-y := vkms_drv.o vkms_plane.o vkms_output.o vkms_crtc.o vkms_gem.o 
> > > vkms_crc.o
> > >  
> > >  obj-$(CONFIG_DRM_VKMS) += vkms.o
> > > diff --git a/drivers/gpu/drm/vkms/vkms_crtc.c 
> > > b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > index 26babb85ca77..f3a674db33b8 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_crtc.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_crtc.c
> > > @@ -10,18 +10,36 @@
> > >  #include 
> > >  #include 
> > >  
> > > -static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
> > > +static void _vblank_handle(struct vkms_output *output)
> > >  {
> > > - struct vkms_output *output = container_of(timer, struct vkms_output,
> > > -   vblank_hrtimer);
> > >   struct drm_crtc *crtc = >crtc;
> > > - int ret_overrun;
> > > + struct vkms_crtc_state *state = to_vkms_crtc_state(crtc->state);
> > >   bool ret;
> > >  
> > > + int crc_enabled = 0;
> > > +
> > > + spin_lock(>lock);
> > > + crc_enabled = output->crc_enabled;
> > 
> > Aside from the implicit bool -> int cast, I don't think you need this local 
> > var,
> > just use output->crc_enabled directly below.
> > 
> > >   ret = drm_crtc_handle_vblank(crtc);
> > >   if (!ret)
> > >   DRM_ERROR("vkms failure on handling vblank");
> > 
> > This would be more useful with the error code printed.
> > 
> 
> I think this only returns false on failure. Also I've noticed most of the 
> usage of
> drm_crtc_handle_vblank don't check the return value, should I do the
> same as well and drop ret and error message?

Ahh, I didn't see that ret was bool. In that case, the error message is fine.
It's always good practice to assume that if a function returns a status, it's
worth checking.

> 
> > >  
> > > + if (state && crc_enabled) {
> > > + state->n_frame = drm_crtc_accurate_vblank_count(crtc);
> > > + queue_work(output->crc_workq, >crc_work);
> > > + }
> > > +
> > > + spin_unlock(>lock);
> > > +}
> > > +
> > > +static enum hrtimer_restart vkms_vblank_simulate(struct hrtimer *timer)
> > > +{
> > > + struct vkms_output *output = container_of(timer, struct vkms_output,
> > > +   vblank_hrtimer);
> > > + int ret_overrun;
> > > +
> > > + _vblank_handle(output);
> > > +
> > >   ret_overrun = hrtimer_forward_now(>vblank_hrtimer,
> > > output->period_ns);
> > >  
> > > @@ -97,6 +115,8 @@ vkms_atomic_crtc_duplicate_state(struct drm_crtc *crtc)
> > >  
> > >   __drm_atomic_helper_crtc_duplicate_state(crtc, _state->base);
> > >  
> > > + INIT_WORK(_state->crc_work, vkms_crc_work_handle);
> > > +
> > >   return _state->base;
> > >  }
> > >  
> > > @@ -104,11 +124,18 @@ static void vkms_atomic_crtc_destroy_state(struct 
> > > drm_crtc *crtc,
> > >  struct drm_crtc_state *state)
> > >  {
> > >   struct vkms_crtc_state *vkms_state;
> > > + struct vkms_output *vkms_out = drm_crtc_to_vkms_output(crtc);
> > >  
> > >   vkms_state = to_vkms_crtc_state(state);
> > >  
> > >   __drm_atomic_helper_crtc_destroy_state(state);
> > > - kfree(vkms_state);
> > > +
> > > + if (vkms_state) {
> > > + flush_workqueue(vkms_out->crc_workq);
> > 
> > I'm a little worried about this bit. Since the workqueue is per-output, is 
> > it
> > possible you'll be waiting for more frames to complete than you need to be?
> > 
> 
> I see, maybe I should flush per work_struct instead?

Yeah, that would make 

[Bug 107266] Radeon Pro Duo (Polaris) - ring sdma0 timeout

[bugzilla-daemon]

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

Bug ID: 107266
   Summary: Radeon Pro Duo (Polaris) - ring sdma0 timeout
   Product: DRI
   Version: unspecified
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu-pro
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: rh...@hotmail.com

Running a polaris Pro Duo on Ubuntu 18.04, Kernel from ROCm branch
4.17.0-rc2-180424-fkxamd and everything works great.

In testing latest drm-next-4.19-wip kernel, I get the following errors on boot,
and have no working opencl (ie clinfo hangs indefinitely)


[   58.913281] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma0 timeout,
signaled seq=573, emitted seq=574
[   58.913284] [drm] GPU recovery disabled.
[   58.914276] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma1 timeout,
signaled seq=331, emitted seq=333
[   58.914280] [drm] GPU recovery disabled.
[   58.914312] [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring sdma1 timeout,
signaled seq=331, emitted seq=333
[   58.914313] [drm] GPU recovery disabled.



Please let me know if you need any specifics.

-- 
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 v4 11/11] drm: rcar-du: Support interlaced video output through vsp1

[Kieran Bingham]

Hi Laurent,

On 17/07/18 14:51, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Monday, 16 July 2018 20:20:30 EEST Kieran Bingham wrote:
>> On 24/05/18 12:50, Laurent Pinchart wrote:
>>> On Thursday, 3 May 2018 16:36:22 EEST Kieran Bingham wrote:
 Use the newly exposed VSP1 interface to enable interlaced frame support
 through the VSP1 lif pipelines.
>>>
>>> s/lif/LIF/
>>
>> Fixed.
>>
 Signed-off-by: Kieran Bingham 
 ---

  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 +
  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 3 +++
  2 files changed, 4 insertions(+)

 diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
 b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index d71d709fe3d9..206532959ec9
 100644
 --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
 +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
 @@ -289,6 +289,7 @@ static void rcar_du_crtc_set_display_timing(struct
 rcar_du_crtc *rcrtc)
/* Signal polarities */
value = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
  | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DSMR_HSL : 0)
 +| ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? DSMR_ODEV : 0)
>>>
>>> How will this affect Gen2 ?.
>>
>> The bit is documented identically for Gen2. Potentially / Probably it
>> 'might' reverse the fields... I'm not certain yet. I don't have access
>> to a Gen2 platform to test.
>>
>> I'll see if this change can be dropped, but I think it is playing a role
>> in ensuring that the field detection occurs in VSP1 through the
>> VI6_STATUS_FLD_STD() field. (see vsp1_dlm_irq_frame_end())
>>
>>> Could you document what this change does in the
>>> commit message ?
>>
>> This sets the position in the buffer of the ODDF. With this set, the ODD
>> field is located in the second half (BOTTOM) of the same frame of the
>> interlace display.
>>
>> Otherwise, it's in the first half (TOP)
>>
>> I faced some issues as to the ordering when testing, so I suspect this
>> might actually be related to that. (re VI6_STATUS_FLD_STD in
>> vsp1_dlm_irq_frame_end()).
>>
>> As you mention - this may have a negative effect on the Gen2
>> implementation - so it needs considering with that.
>>
>>
>> /me to investigate further.
> 
> Thank you. I don't object to this change, but I'd like to know what its 
> implications are on Gen2. It might even fix a bug :-) Let me know if you'd 
> like me to run tests on a Lager board.

I've done some testing with this (removing the DSMR change, and
inverting the VI6_STATUS_FLD_STD handling) and had some odd results.
Perhaps my testing needs refinement.

So, yes please - I think I'd really like to know what the effects are on
a Lager platform.

Would you (or anyone with a Gen2 and interest in vsp1/du) be able to
test my latest vsp1/du/interlaced branch/tag on your local Gen2 platform
please?

I'm testing interlaced with:

kmstest # sanity test.
kmstest -c 1 -r 1920x1080i --flip

Any (easy) other methods for testing interlaced pipelines are welcome.

Is it possible to set the mode for kmscube? (--help doesn't look promising)

I have various test streams of interlaced media content in my media
library, but not an easy way of decoding and presenting these on the
screen on the Gen3.

I believe GStreamer now has a drm/kms sink ... Perhaps I should get that
recompiled. (That would help me with other tasks too actually)


> 
  | DSMR_DIPM_DISP | DSMR_CSPM;

rcar_du_crtc_write(rcrtc, DSMR, value);

 diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
 b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index af7822a66dee..c7b37232ee91
 100644
 --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
 +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
 @@ -186,6 +186,9 @@ static void rcar_du_vsp_plane_setup(struct
 rcar_du_vsp_plane *plane)
};
unsigned int i;

 +  cfg.interlaced = !!(plane->plane.state->crtc->mode.flags
 +  & DRM_MODE_FLAG_INTERLACE);
 +
cfg.src.left = state->state.src.x1 >> 16;
cfg.src.top = state->state.src.y1 >> 16;
cfg.src.width = drm_rect_width(>state.src) >> 16;
> 

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

Re: [PATCH 04/16] iommu: sva: Add support for private PASIDs

[Jordan Crouse]

On Tue, Jul 17, 2018 at 12:21:03PM +0100, Jean-Philippe Brucker wrote:
> Hi Jordan,
> 
> Thanks for the patches, I finally got around testing them with SMMUv3.
> It's an important feature, arguably more than SVA itself. I could pick
> this one as part of the SVA series, what do you think?

I'm good with whatever is the easiest.

> Although I probably would have done the same, I dislike the interface
> because it forces us to duplicate functions and IOMMU ops. The list is
> small but growing:
> 
> iommu_map
> iommu_map_sg
> iommu_unmap
> iommu_unmap_fast
> iommu_iova_to_phys
> iommu_tlb_range_add
> iommu_flush_tlb_all
> 
> Each of these and their associated IOMMU op will have an iommu_sva_X
> counterpart that takes one different argument. Modifying these functions
> to take both a domain and a PASID argument would be more elegant. Or as
> an intermediate solution, perhaps we could only change the IOMMU ops to
> take an additional argument, like you did for map_sg?
> 
> In any case it requires invasive changes in lots of drivers and we can
> always tidy up later, so unless Joerg has a preference I'd keep the
> duplicates for now.

I agree.

> However, having to lookup pasid-to-io_mm on every map/unmap call is
> cumbersome, especially since map/unmap are supposed to be as fast as
> possible. iommu_sva_alloc_pasid should return a structure representing
> the PASID instead of the value alone. The io_mm structure seems like a
> good fit, and the device driver can access io_mm->pasid directly or via
> an io_mm_get_pasid() function.
> 
> The new functions would then be:
> 
> struct io_mm *iommu_sva_alloc_pasid(domain, dev)
> void iommu_sva_free_pasid(domain, io_mm)
> 
> int iommu_sva_map(io_mm, iova, paddr, size, prot)
> size_t iommu_map_sg(io_mm, iova, sg, nents, prot)
> size_t iommu_sva_unmap(io_mm, iova, size)
> size_t iommu_sva_unmap_fast(io_mm, iova, size)
> phys_addr_t iommu_sva_iova_to_phys(io_mm, iova)
> void iommu_sva_flush_tlb_all(io_mm)
> void iommu_sva_tlb_range_add(io_mm, iova, size)

Okay - this sounds reasonable - a simplification like that could
even lead to making all the new functions static inlines which
would cut down on the exported symbols.

> A few more comments inline

All those sound like good ideas to me. I'll take a bit of time to bash on this
and send out an updated revision soonish.

Jordan



-- 
The 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 V2] drm: handle error values properly

[Sean Paul]

On Tue, Jul 17, 2018 at 03:28:21PM +0200, Nicholas Mc Guire wrote:
> drm_legacy_ctxbitmap_next() returns idr_alloc() which can return
> -ENOMEM, -EINVAL or -ENOSPC none of which are -1. since drm_context_t
> is an unsigned int an intermediate variable is used to handle the
> error cases, and then cast to drm_context_t after ensuring that the
> value is >= 0. The explicit cast is to mark the type conversion as
> intentional.
> 
> Signed-off-by: Nicholas Mc Guire 
> Reported-by: kbuild test robot 
> Fixes: d530b5f1ca0b ("drm: re-enable error handling")
> Fixes: 62968144e673 ("drm: convert drm context code to use Linux idr")
> ---
> 
> kbuild test robot reported:
> 
> tree:   git://anongit.freedesktop.org/drm/drm-misc for-linux-next-fixes
> head:   d530b5f1ca0bb66958a2b714bebe40a1248b9c15
> commit: d530b5f1ca0bb66958a2b714bebe40a1248b9c15 [2/2] drm: re-enable error
> +handling
> 
> smatch warnings:
> drivers/gpu/drm/drm_context.c:375 drm_legacy_addctx() warn: unsigned
> +'ctx->handle' is never less than zero.
> 
> 
> V2: The proposed fix in d530b5f1ca0b ("drm: re-enable error handling")
> actually was ineffective as the negative return value check was 
> against a unsigned int and thus always false as reported by
> kbuild test robot . The below patch removes that
> warning and fixes the original problem of missed error handling.
> 
> drm_context_t is actually just used in a few placed so the type could be
> changed but it is also exported via tools/include/uapi/drm/drm.h so
> changing the typedef of drm_context_t could break applications and thus
> this is not an option.
> 
> Patch was compile tested with: x86_64_defconfig
> 
> Patch is against 4.18-rc4 (localversion-next is next-20180717)
> 
>  drivers/gpu/drm/drm_context.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
> index 3c4000f..78f32a3 100644
> --- a/drivers/gpu/drm/drm_context.c
> +++ b/drivers/gpu/drm/drm_context.c
> @@ -361,22 +361,26 @@ int drm_legacy_addctx(struct drm_device *dev, void 
> *data,
>  {
>   struct drm_ctx_list *ctx_entry;
>   struct drm_ctx *ctx = data;
> + int ret;
>  
>   if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
>   !drm_core_check_feature(dev, DRIVER_LEGACY))
>   return -EINVAL;
>  
>   ctx->handle = drm_legacy_ctxbitmap_next(dev);

Remove this call?

Sean

> - if (ctx->handle == DRM_KERNEL_CONTEXT) {
> + ret = drm_legacy_ctxbitmap_next(dev);
> + if (ret == DRM_KERNEL_CONTEXT) {
>   /* Skip kernel's context and get a new one. */
> - ctx->handle = drm_legacy_ctxbitmap_next(dev);
> + ret = drm_legacy_ctxbitmap_next(dev);
>   }
> - DRM_DEBUG("%d\n", ctx->handle);
> - if (ctx->handle < 0) {
> + DRM_DEBUG("ctxbitmap is error code %d\n", ret);
> + if (ret < 0) {
>   DRM_DEBUG("Not enough free contexts.\n");
>   /* Should this return -EBUSY instead? */
>   return -ENOMEM;
>   }
> + /* valid context is >= 0 */
> + ctx->handle = (drm_context_t)ret;
>  
>   ctx_entry = kmalloc(sizeof(*ctx_entry), GFP_KERNEL);
>   if (!ctx_entry) {
> -- 
> 2.1.4
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 1/3] drm/i915: Split audio component to a generic type

[Rodrigo Vivi]

On Tue, Jul 17, 2018 at 11:26:50AM +0200, Takashi Iwai wrote:
> For allowing other drivers to use the DRM audio component, rename the
> i915_audio_component_* with drm_audio_component_*, and split the
> generic part into drm_audio_component.h.  The i915 specific stuff
> remains in struct i915_audio_component, which contains
> drm_audio_component as the base.
> 
> The license of drm_audio_component.h is kept to MIT as same as the the
> original i915_component.h.
> 
> This is a preliminary change for further development, and no
> functional changes by this patch itself, merely code-split and
> renames.
> 
> v1->v2: Use SPDX for drm_audio_component.h, fix remaining i915
> argument in drm_audio_component.h
> 
> Signed-off-by: Takashi Iwai 

Reviewed-by: Rodrigo Vivi 

> ---
>  drivers/gpu/drm/i915/intel_audio.c | 22 +++
>  include/drm/drm_audio_component.h  | 95 ++
>  include/drm/i915_component.h   | 85 ++
>  include/sound/hda_i915.h   |  6 +-
>  include/sound/hdaudio.h|  6 +-
>  sound/hda/hdac_i915.c  | 40 +++--
>  sound/pci/hda/patch_hdmi.c |  8 +--
>  sound/soc/codecs/hdac_hdmi.c   |  2 +-
>  8 files changed, 144 insertions(+), 120 deletions(-)
>  create mode 100644 include/drm/drm_audio_component.h
> 
> diff --git a/drivers/gpu/drm/i915/intel_audio.c 
> b/drivers/gpu/drm/i915/intel_audio.c
> index 3ea566f99450..7dd5605d94ae 100644
> --- a/drivers/gpu/drm/i915/intel_audio.c
> +++ b/drivers/gpu/drm/i915/intel_audio.c
> @@ -639,11 +639,12 @@ void intel_audio_codec_enable(struct intel_encoder 
> *encoder,
>   dev_priv->av_enc_map[pipe] = encoder;
>   mutex_unlock(_priv->av_mutex);
>  
> - if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
> + if (acomp && acomp->base.audio_ops &&
> + acomp->base.audio_ops->pin_eld_notify) {
>   /* audio drivers expect pipe = -1 to indicate Non-MST cases */
>   if (!intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST))
>   pipe = -1;
> - acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> + 
> acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr,
>(int) port, (int) pipe);
>   }
>  
> @@ -681,11 +682,12 @@ void intel_audio_codec_disable(struct intel_encoder 
> *encoder,
>   dev_priv->av_enc_map[pipe] = NULL;
>   mutex_unlock(_priv->av_mutex);
>  
> - if (acomp && acomp->audio_ops && acomp->audio_ops->pin_eld_notify) {
> + if (acomp && acomp->base.audio_ops &&
> + acomp->base.audio_ops->pin_eld_notify) {
>   /* audio drivers expect pipe = -1 to indicate Non-MST cases */
>   if (!intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST))
>   pipe = -1;
> - acomp->audio_ops->pin_eld_notify(acomp->audio_ops->audio_ptr,
> + 
> acomp->base.audio_ops->pin_eld_notify(acomp->base.audio_ops->audio_ptr,
>(int) port, (int) pipe);
>   }
>  
> @@ -880,7 +882,7 @@ static int i915_audio_component_get_eld(struct device 
> *kdev, int port,
>   return ret;
>  }
>  
> -static const struct i915_audio_component_ops i915_audio_component_ops = {
> +static const struct drm_audio_component_ops i915_audio_component_ops = {
>   .owner  = THIS_MODULE,
>   .get_power  = i915_audio_component_get_power,
>   .put_power  = i915_audio_component_put_power,
> @@ -897,12 +899,12 @@ static int i915_audio_component_bind(struct device 
> *i915_kdev,
>   struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
>   int i;
>  
> - if (WARN_ON(acomp->ops || acomp->dev))
> + if (WARN_ON(acomp->base.ops || acomp->base.dev))
>   return -EEXIST;
>  
>   drm_modeset_lock_all(_priv->drm);
> - acomp->ops = _audio_component_ops;
> - acomp->dev = i915_kdev;
> + acomp->base.ops = _audio_component_ops;
> + acomp->base.dev = i915_kdev;
>   BUILD_BUG_ON(MAX_PORTS != I915_MAX_PORTS);
>   for (i = 0; i < ARRAY_SIZE(acomp->aud_sample_rate); i++)
>   acomp->aud_sample_rate[i] = 0;
> @@ -919,8 +921,8 @@ static void i915_audio_component_unbind(struct device 
> *i915_kdev,
>   struct drm_i915_private *dev_priv = kdev_to_i915(i915_kdev);
>  
>   drm_modeset_lock_all(_priv->drm);
> - acomp->ops = NULL;
> - acomp->dev = NULL;
> + acomp->base.ops = NULL;
> + acomp->base.dev = NULL;
>   dev_priv->audio_component = NULL;
>   drm_modeset_unlock_all(_priv->drm);
>  }
> diff --git a/include/drm/drm_audio_component.h 
> b/include/drm/drm_audio_component.h
> new file mode 100644
> index ..e85689f212c2
> --- /dev/null
> +++ b/include/drm/drm_audio_component.h
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: MIT
> +// Copyright © 2014 Intel Corporation
> +
> 

Re: [Intel-gfx] [PATCH 1/6] drm: Let userspace check if driver supports modeset

[Chris Wilson]

Quoting Souza, Jose (2018-07-17 18:02:17)
> On Tue, 2018-07-17 at 08:28 +0100, Chris Wilson wrote:
> > Quoting José Roberto de Souza (2018-07-16 23:38:36)
> > > GPU accelerators usually don't have display block or the display
> > > driver part can be disable when building driver(for servers it save
> > > some resources) so it is important let userspace check this
> > > capability too.
> > 
> > We currently communicate that by having no modeset resources. What
> > does
> > another cap bit accomplish that we don't already know?
> 
> This is a hackish way to check if driver support modeset,
> drmModeGetResources()/drm_mode_getresources() can fail and return null
> by other reasons and just check for the errno value can be misleading
> too.

Do not confuse libdrm with the ioctl. You do not need to allocate
anything for such a check, and it is being used directly without
allocations as a has-kms check.

More to the point existing userspace determines modeset capability
through the reported resources. Your changelog needs to explain why they
are inadequate and how you plan to coordinate your fix with userspace.
As it stands you are introducing uABI with no user...
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #10 from Alex Deucher (alexdeuc...@gmail.com) ---
(In reply to Alex Deucher from comment #2)
> Duplicate of:
> https://bugs.freedesktop.org/show_bug.cgi?id=105760

This is apparently not a duplicate of that bug.  Similar symptoms, but
different root cause.

-- 
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 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #9 from Alex Deucher (alexdeuc...@gmail.com) ---
(In reply to bakarichard91 from comment #8)
> Hi, I've patched it and it loads perfectly without any additional
> parameters. I'm checking the temperature now.
> 
> Will this ever be merged to the "master"?

Assuming it fixes the issue, I'll go ahead and apply it to upstream and stable
kernels.

-- 
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 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #8 from bakarichar...@gmail.com ---
Hi, I've patched it and it loads perfectly without any additional parameters.
I'm checking the temperature now.

Will this ever be merged to the "master"?

Thanks for the help. The support of linux is better than than support of any
commerical product.

-- 
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] drm/vc4: Replace drm_dev_unref with drm_dev_put

[Eric Anholt]

Thomas Zimmermann  writes:

> This patch unifies the naming of DRM functions for reference counting
> of struct drm_device. The resulting code is more aligned with the rest
> of the Linux kernel interfaces.

Pushed to drm-misc-next.  Thanks!


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

Re: [PATCH] drm/pl111: Replace drm_dev_unref with drm_dev_put

[Eric Anholt]

Thomas Zimmermann  writes:

> This patch unifies the naming of DRM functions for reference counting
> of struct drm_device. The resulting code is more aligned with the rest
> of the Linux kernel interfaces.
>
> Signed-off-by: Thomas Zimmermann 

Pushed to drm-misc-next.  Thanks!


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

Re: [PATCH v2] drm/pl111: Use 64-bit arithmetic instead of 32-bit

[Eric Anholt]

"Gustavo A. R. Silva"  writes:

> Add suffix ULL to constant 1000 in order to give the compiler complete
> information about the proper arithmetic to use.
>
> Notice that such constant is used in a context that expects an
> expression of type u64 (64 bits, unsigned) and the following
> expression is currently being evaluated using 32-bit arithmetic:
>
> mode->clock * 1000
>
> Addresses-Coverity-ID: 1466139 ("Unintentional integer overflow")
> Signed-off-by: Gustavo A. R. Silva 

This is silly.  The clock won't be over 4ghz -- we haven't seen anything
over 1024x768 on this hardware as far as I know.  The u64 is for the
multiplication by width/height below.

I've still applied the patch to shut up the tool.


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

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Lyude Paul]

On Tue, 2018-07-17 at 20:32 +0200, Lukas Wunner wrote:
> On Tue, Jul 17, 2018 at 02:24:31PM -0400, Lyude Paul wrote:
> > On Tue, 2018-07-17 at 20:20 +0200, Lukas Wunner wrote:
> > > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire()
> > > wants it in resumed state, so is waiting forever for the device to
> > > runtime suspend in order to resume it again immediately afterwards.
> > > 
> > > The deadlock in the stack trace you've posted could be resolved using
> > > the technique I used in d61a5c106351 by adding the following to
> > > include/linux/pm_runtime.h:
> > > 
> > > static inline bool pm_runtime_status_suspending(struct device *dev)
> > > {
> > >   return dev->power.runtime_status == RPM_SUSPENDING;
> > > }
> > > 
> > > static inline bool is_pm_work(struct device *dev)
> > > {
> > >   struct work_struct *work = current_work();
> > > 
> > >   return work && work->func == dev->power.work;
> > > }
> > > 
> > > Then adding this to nvkm_i2c_aux_acquire():
> > > 
> > >   struct device *dev = pad->i2c->subdev.device->dev;
> > > 
> > >   if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) {
> > >   ret = pm_runtime_get_sync(dev);
> > >   if (ret < 0 && ret != -EACCES)
> > >   return ret;
> > >   }
> > > 
> > > But here's the catch:  This only works for an *async* runtime suspend.
> > > It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc,
> > > because then the runtime suspend is executed in the context of the caller,
> > > not in the context of dev->power.work.
> > > 
> > > So it's not a full solution, but hopefully something that gets you
> > > going.  I'm not really familiar with the code paths leading to
> > > nvkm_i2c_aux_acquire() to come up with a full solution off the top
> > > of my head I'm afraid.
> > 
> > OK-I was considering doing something similar to that commit beforehand but I
> > wasn't sure if I was going to just be hacking around an actual issue. That
> > doesn't seem to be the case. This is very helpful and hopefully I should be
> > able
> > to figure something out from this, thanks!
> 
> In some cases, the function acquiring the runtime PM ref is only called
> from a couple of places and then it would be feasible and appropriate
> to add a bool parameter to the function telling it to acquire the ref
> or not.  So the function is told using a parameter which context it's
> running in:  In the runtime_suspend code path or some other code path.
> 
> The technique to use current_work() is an alternative approach to figure
> out the context if passing in an additional parameter is not feasible
> for some reason.  That was the case with d61a5c106351.  That approach
> only works for work items though.

Something I'm curious about. This isn't the first time I've hit a situation like
this (see: the improper disable_depth fix I added into amdgpu I now need to go
and fix), which makes me wonder: is there actually any reason Linux's runtime PM
core doesn't just turn get/puts() in the context of s/r callbacks into no-ops by
default? 

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

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Lukas Wunner]

On Tue, Jul 17, 2018 at 02:24:31PM -0400, Lyude Paul wrote:
> On Tue, 2018-07-17 at 20:20 +0200, Lukas Wunner wrote:
> > Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire()
> > wants it in resumed state, so is waiting forever for the device to
> > runtime suspend in order to resume it again immediately afterwards.
> > 
> > The deadlock in the stack trace you've posted could be resolved using
> > the technique I used in d61a5c106351 by adding the following to
> > include/linux/pm_runtime.h:
> > 
> > static inline bool pm_runtime_status_suspending(struct device *dev)
> > {
> > return dev->power.runtime_status == RPM_SUSPENDING;
> > }
> > 
> > static inline bool is_pm_work(struct device *dev)
> > {
> > struct work_struct *work = current_work();
> > 
> > return work && work->func == dev->power.work;
> > }
> > 
> > Then adding this to nvkm_i2c_aux_acquire():
> > 
> > struct device *dev = pad->i2c->subdev.device->dev;
> > 
> > if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) {
> > ret = pm_runtime_get_sync(dev);
> > if (ret < 0 && ret != -EACCES)
> > return ret;
> > }
> > 
> > But here's the catch:  This only works for an *async* runtime suspend.
> > It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc,
> > because then the runtime suspend is executed in the context of the caller,
> > not in the context of dev->power.work.
> > 
> > So it's not a full solution, but hopefully something that gets you
> > going.  I'm not really familiar with the code paths leading to
> > nvkm_i2c_aux_acquire() to come up with a full solution off the top
> > of my head I'm afraid.
> 
> OK-I was considering doing something similar to that commit beforehand but I
> wasn't sure if I was going to just be hacking around an actual issue. That
> doesn't seem to be the case. This is very helpful and hopefully I should be 
> able
> to figure something out from this, thanks!

In some cases, the function acquiring the runtime PM ref is only called
from a couple of places and then it would be feasible and appropriate
to add a bool parameter to the function telling it to acquire the ref
or not.  So the function is told using a parameter which context it's
running in:  In the runtime_suspend code path or some other code path.

The technique to use current_work() is an alternative approach to figure
out the context if passing in an additional parameter is not feasible
for some reason.  That was the case with d61a5c106351.  That approach
only works for work items though.

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

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Lyude Paul]

On Tue, 2018-07-17 at 20:20 +0200, Lukas Wunner wrote:
> On Tue, Jul 17, 2018 at 12:53:11PM -0400, Lyude Paul wrote:
> > On Tue, 2018-07-17 at 09:16 +0200, Lukas Wunner wrote:
> > > On Mon, Jul 16, 2018 at 07:59:25PM -0400, Lyude Paul wrote:
> > > > In order to fix all of the spots that need to have runtime PM get/puts()
> > > > added, we need to ensure that it's possible for us to call
> > > > pm_runtime_get/put() in any context, regardless of how deep, since
> > > > almost all of the spots that are currently missing refs can potentially
> > > > get called in the runtime suspend/resume path. Otherwise, we'll try to
> > > > resume the GPU as we're trying to resume the GPU (and vice-versa) and
> > > > cause the kernel to deadlock.
> > > > 
> > > > With this, it should be safe to call the pm runtime functions in any
> > > > context in nouveau with one condition: any point in the driver that
> > > > calls pm_runtime_get*() cannot hold any locks owned by nouveau that
> > > > would be acquired anywhere inside nouveau_pmops_runtime_resume().
> > > > This includes modesetting locks, i2c bus locks, etc.
> > > 
> > > [snip]
> > > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > > > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *dev)
> > > > return -EBUSY;
> > > > }
> > > >  
> > > > +   dev->power.disable_depth++;
> > > > +
> > > 
> > > Anyway, if I understand the commit message correctly, you're hitting a
> > > pm_runtime_get_sync() in a code path that itself is called during a
> > > pm_runtime_get_sync().  Could you include stack traces in the commit
> > > message?  My gut feeling is that this patch masks a deeper issue,
> > > e.g. if the runtime_resume code path does in fact directly poll outputs,
> > > that would seem wrong.  Runtime resume should merely make the card
> > > accessible, i.e. reinstate power if necessary, put into PCI_D0,
> > > restore registers, etc.  Output polling should be scheduled
> > > asynchronously.
> > 
> > So: the reason that patch was added was mainly for the patches later in the
> > series that add guards around the i2c bus and aux bus, since both of those
> > require that the device be awake for it to work. Currently, the spot where
> > it
> > would recurse is:
> 
> Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire()
> wants it in resumed state, so is waiting forever for the device to
> runtime suspend in order to resume it again immediately afterwards.
> 
> The deadlock in the stack trace you've posted could be resolved using
> the technique I used in d61a5c106351 by adding the following to
> include/linux/pm_runtime.h:
> 
> static inline bool pm_runtime_status_suspending(struct device *dev)
> {
>   return dev->power.runtime_status == RPM_SUSPENDING;
> }
> 
> static inline bool is_pm_work(struct device *dev)
> {
>   struct work_struct *work = current_work();
> 
>   return work && work->func == dev->power.work;
> }
> 
> Then adding this to nvkm_i2c_aux_acquire():
> 
>   struct device *dev = pad->i2c->subdev.device->dev;
> 
>   if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) {
>   ret = pm_runtime_get_sync(dev);
>   if (ret < 0 && ret != -EACCES)
>   return ret;
>   }
> 
> But here's the catch:  This only works for an *async* runtime suspend.
> It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc,
> because then the runtime suspend is executed in the context of the caller,
> not in the context of dev->power.work.
> 
> So it's not a full solution, but hopefully something that gets you
> going.  I'm not really familiar with the code paths leading to
> nvkm_i2c_aux_acquire() to come up with a full solution off the top
> of my head I'm afraid.
OK-I was considering doing something similar to that commit beforehand but I
wasn't sure if I was going to just be hacking around an actual issue. That
doesn't seem to be the case. This is very helpful and hopefully I should be able
to figure something out from this, thanks!

> 
> Note, it's not sufficient to just check pm_runtime_status_suspending(dev)
> because if the runtime_suspend is carried out concurrently by something
> else, this will return true but it's not guaranteed that the device is
> actually kept awake until the i2c communication has been fully performed.
> 
> HTH,
> 
> Lukas
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Lukas Wunner]

On Tue, Jul 17, 2018 at 12:53:11PM -0400, Lyude Paul wrote:
> On Tue, 2018-07-17 at 09:16 +0200, Lukas Wunner wrote:
> > On Mon, Jul 16, 2018 at 07:59:25PM -0400, Lyude Paul wrote:
> > > In order to fix all of the spots that need to have runtime PM get/puts()
> > > added, we need to ensure that it's possible for us to call
> > > pm_runtime_get/put() in any context, regardless of how deep, since
> > > almost all of the spots that are currently missing refs can potentially
> > > get called in the runtime suspend/resume path. Otherwise, we'll try to
> > > resume the GPU as we're trying to resume the GPU (and vice-versa) and
> > > cause the kernel to deadlock.
> > > 
> > > With this, it should be safe to call the pm runtime functions in any
> > > context in nouveau with one condition: any point in the driver that
> > > calls pm_runtime_get*() cannot hold any locks owned by nouveau that
> > > would be acquired anywhere inside nouveau_pmops_runtime_resume().
> > > This includes modesetting locks, i2c bus locks, etc.
> > 
> > [snip]
> > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *dev)
> > >   return -EBUSY;
> > >   }
> > >  
> > > + dev->power.disable_depth++;
> > > +
> > 
> > Anyway, if I understand the commit message correctly, you're hitting a
> > pm_runtime_get_sync() in a code path that itself is called during a
> > pm_runtime_get_sync().  Could you include stack traces in the commit
> > message?  My gut feeling is that this patch masks a deeper issue,
> > e.g. if the runtime_resume code path does in fact directly poll outputs,
> > that would seem wrong.  Runtime resume should merely make the card
> > accessible, i.e. reinstate power if necessary, put into PCI_D0,
> > restore registers, etc.  Output polling should be scheduled
> > asynchronously.
> 
> So: the reason that patch was added was mainly for the patches later in the
> series that add guards around the i2c bus and aux bus, since both of those
> require that the device be awake for it to work. Currently, the spot where it
> would recurse is:

Okay, the PCI device is suspending and the nvkm_i2c_aux_acquire()
wants it in resumed state, so is waiting forever for the device to
runtime suspend in order to resume it again immediately afterwards.

The deadlock in the stack trace you've posted could be resolved using
the technique I used in d61a5c106351 by adding the following to
include/linux/pm_runtime.h:

static inline bool pm_runtime_status_suspending(struct device *dev)
{
return dev->power.runtime_status == RPM_SUSPENDING;
}

static inline bool is_pm_work(struct device *dev)
{
struct work_struct *work = current_work();

return work && work->func == dev->power.work;
}

Then adding this to nvkm_i2c_aux_acquire():

struct device *dev = pad->i2c->subdev.device->dev;

if (!(is_pm_work(dev) && pm_runtime_status_suspending(dev))) {
ret = pm_runtime_get_sync(dev);
if (ret < 0 && ret != -EACCES)
return ret;
}

But here's the catch:  This only works for an *async* runtime suspend.
It doesn't work for pm_runtime_put_sync(), pm_runtime_suspend() etc,
because then the runtime suspend is executed in the context of the caller,
not in the context of dev->power.work.

So it's not a full solution, but hopefully something that gets you
going.  I'm not really familiar with the code paths leading to
nvkm_i2c_aux_acquire() to come up with a full solution off the top
of my head I'm afraid.

Note, it's not sufficient to just check pm_runtime_status_suspending(dev)
because if the runtime_suspend is carried out concurrently by something
else, this will return true but it's not guaranteed that the device is
actually kept awake until the i2c communication has been fully performed.

HTH,

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

[PATCH] meson.build: fix intel atomics detection

[Peter Seiderer]

Use the stronger compiler.link() test (instead of the weaker
compiler.compile()) to fix the intel atomics detection.

Fixes false positive in case of sparc compile (buildroot toolchain).

Signed-off-by: Peter Seiderer 
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 9b443a50..e47ad264 100644
--- a/meson.build
+++ b/meson.build
@@ -49,7 +49,7 @@ intel_atomics = false
 lib_atomics = false
 
 dep_atomic_ops = dependency('atomic_ops', required : false)
-if cc.compiles('''
+if cc.links('''
 int atomic_add(int *i) { return __sync_add_and_fetch (i, 1); }
 int atomic_cmpxchg(int *i, int j, int k) { return 
__sync_val_compare_and_swap (i, j, k); }
 ''',
-- 
2.18.0

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

[Bug 107263] Decrease start-up time of amdgpu_init from 390 ms to under 100 ms

[bugzilla-daemon]

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

--- Comment #1 from Christian König  ---
We can certainly reduce the time amdgpu needs to load, but I'm not sure that we
can get under 100ms.

See on the hardware side when switching between power states we need some delay
before the clocks/voltages are stable. And we need to do this multiple times
during driver startup.

On top of that comes the display code which has some timeouts for probing the
connectors.

-- 
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 v4 10/11] media: vsp1: Support Interlaced display pipelines

[Laurent Pinchart]

Hi Kieran,

On Tuesday, 17 July 2018 19:08:44 EEST Kieran Bingham wrote:
> On 17/07/18 13:52, Laurent Pinchart wrote:
> > On Monday, 16 July 2018 21:21:00 EEST Kieran Bingham wrote:
> >> On 24/05/18 13:51, Laurent Pinchart wrote:
> >>> On Thursday, 3 May 2018 16:36:21 EEST Kieran Bingham wrote:
>  Calculate the top and bottom fields for the interlaced frames and
>  utilise the extended display list command feature to implement the
>  auto-field operations. This allows the DU to update the VSP2 registers
>  dynamically based upon the currently processing field.
>  
>  Signed-off-by: Kieran Bingham 
>  
>  ---
>  
>  v3:
>   - Pass DL through partition calls to allow autocmd's to be retrieved
>   - Document interlaced field in struct vsp1_du_atomic_config
>  
>  v2:
>   - fix erroneous BIT value which enabled interlaced
>   - fix field handling at frame_end interrupt
>  
>  ---
>  
>   drivers/media/platform/vsp1/vsp1_dl.c   | 10 -
>   drivers/media/platform/vsp1/vsp1_drm.c  | 11 -
>   drivers/media/platform/vsp1/vsp1_regs.h |  1 +-
>   drivers/media/platform/vsp1/vsp1_rpf.c  | 71 ++--
>   drivers/media/platform/vsp1/vsp1_rwpf.h |  1 +-
>   include/media/vsp1.h|  2 +-
>   6 files changed, 93 insertions(+), 3 deletions(-)
> > 
> > [snip]
> > 
>  diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
>  b/drivers/media/platform/vsp1/vsp1_drm.c index
>  2c3db8b8adce..cc29c9d96bb7
>  100644
>  --- a/drivers/media/platform/vsp1/vsp1_drm.c
>  +++ b/drivers/media/platform/vsp1/vsp1_drm.c
>  @@ -811,6 +811,17 @@ int vsp1_du_atomic_update(struct device *dev,
>  unsigned int pipe_index,
>   return -EINVAL;
>   }
>  
>  +if (!(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) && cfg->interlaced) {
> >>> 
> >>> Nitpicking, writing the condition as
> >>> 
> >>>   if (cfg->interlaced && !(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)))
> >> 
> >> Done.
> >> 
> >>> would match the comment better. You can also drop the parentheses around
> >>> the vsp1_feature() call.
> >>> 
>  +/*
>  + * Interlaced support requires extended display lists to
>  + * provide the auto-fld feature with the DU.
>  + */
>  +dev_dbg(vsp1->dev, "Interlaced unsupported on this 
>  output\n");
> >>> 
> >>> Could we catch this in the DU driver to fail atomic test ?
> >> 
> >> Ugh - I thought moving the configuration to vsp1_du_setup_lif() would
> >> give us this, but that return value is not checked in the DU.
> >> 
> >> How can we interogate the VSP1 to ask it if it supports interlaced from
> >> rcar_du_vsp_plane_atomic_check()?
> >> 
> >> Some dummy call to vsp1_du_setup_lif() to check the return value ? Or
> >> should we implement a hook to call through to perform checks in the VSP1
> >> DRM API?
> > 
> > Would it be possible to just infer that from the DU compatible string,
> > without querying the VSP driver ? Of course that's a bit of a layering
> > violation, but as we know what type of VSP instance is present in each
> > SoC, such a small hack wouldn't hurt in my opinion. If the need arises
> > later we can introduce an API to query the information from the VSP
> > driver.
> 
> I'm not sure what there is to match on currently.
> 
> I thought that we had restrictions on which display pipelines supported
> interlaced. (i.e. D3/E3 might not) - but they seem to support extended
> display lists ...
> 
> So isn't it the case that any pipeline which we connect to DRM supports
> interlaced? (currently) - we can't / don't physically connect other VSP
> entities to the DRM pipes...

I haven't checked, but if you think that's the case, I'll trust you :-)

>  +return -EINVAL;
>  +}
>  +
>  +rpf->interlaced = cfg->interlaced;
>  +
>   rpf->fmtinfo = fmtinfo;
>   rpf->format.num_planes = fmtinfo->planes;
>   rpf->format.plane_fmt[0].bytesperline = cfg->pitch;
> > 
> > [snip]

-- 
Regards,

Laurent Pinchart



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

[PATCH 4/5] drm/omapdrm: Substitute format_is_yuv() with format->is_yuv

[Ayan Kumar Halder]

drm_format_info table has a field 'is_yuv' to denote if the format
is yuv or not. The driver is expected to use this instead of
having a function for the same purpose.

Signed-off-by: Ayan Kumar halder 
---
 drivers/gpu/drm/omapdrm/dss/dispc.c | 26 ++
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
b/drivers/gpu/drm/omapdrm/dss/dispc.c
index 84f274c..8d2d7a4 100644
--- a/drivers/gpu/drm/omapdrm/dss/dispc.c
+++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
@@ -1140,18 +1140,6 @@ static void dispc_ovl_set_color_mode(struct dispc_device 
*dispc,
REG_FLD_MOD(dispc, DISPC_OVL_ATTRIBUTES(plane), m, 4, 1);
 }
 
-static bool format_is_yuv(u32 fourcc)
-{
-   switch (fourcc) {
-   case DRM_FORMAT_YUYV:
-   case DRM_FORMAT_UYVY:
-   case DRM_FORMAT_NV12:
-   return true;
-   default:
-   return false;
-   }
-}
-
 static void dispc_ovl_configure_burst_type(struct dispc_device *dispc,
   enum omap_plane_id plane,
   enum omap_dss_rotation_type rotation)
@@ -1910,11 +1898,14 @@ static void dispc_ovl_set_scaling_uv(struct 
dispc_device *dispc,
int scale_x = out_width != orig_width;
int scale_y = out_height != orig_height;
bool chroma_upscale = plane != OMAP_DSS_WB;
+   const struct drm_format_info *info;
+
+   info = drm_format_info(fourcc);
 
if (!dispc_has_feature(dispc, FEAT_HANDLE_UV_SEPARATE))
return;
 
-   if (!format_is_yuv(fourcc)) {
+   if (!info->is_yuv) {
/* reset chroma resampling for RGB formats  */
if (plane != OMAP_DSS_WB)
REG_FLD_MOD(dispc, DISPC_OVL_ATTRIBUTES2(plane),
@@ -2632,6 +2623,9 @@ static int dispc_ovl_setup_common(struct dispc_device 
*dispc,
bool ilace = !!(vm->flags & DISPLAY_FLAGS_INTERLACED);
unsigned long pclk = dispc_plane_pclk_rate(dispc, plane);
unsigned long lclk = dispc_plane_lclk_rate(dispc, plane);
+   const struct drm_format_info *info;
+
+   info = drm_format_info(fourcc);
 
/* when setting up WB, dispc_plane_pclk_rate() returns 0 */
if (plane == OMAP_DSS_WB)
@@ -2640,7 +2634,7 @@ static int dispc_ovl_setup_common(struct dispc_device 
*dispc,
if (paddr == 0 && rotation_type != OMAP_DSS_ROT_TILER)
return -EINVAL;
 
-   if (format_is_yuv(fourcc) && (in_width & 1)) {
+   if (info->is_yuv && (in_width & 1)) {
DSSERR("input width %d is not even for YUV format\n", in_width);
return -EINVAL;
}
@@ -2680,7 +2674,7 @@ static int dispc_ovl_setup_common(struct dispc_device 
*dispc,
DSSDBG("predecimation %d x %x, new input size %d x %d\n",
x_predecim, y_predecim, in_width, in_height);
 
-   if (format_is_yuv(fourcc) && (in_width & 1)) {
+   if (info->is_yuv && (in_width & 1)) {
DSSDBG("predecimated input width is not even for YUV format\n");
DSSDBG("adjusting input width %d -> %d\n",
in_width, in_width & ~1);
@@ -2688,7 +2682,7 @@ static int dispc_ovl_setup_common(struct dispc_device 
*dispc,
in_width &= ~1;
}
 
-   if (format_is_yuv(fourcc))
+   if (info->is_yuv)
cconv = 1;
 
if (ilace && !fieldmode) {
-- 
2.7.4

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

[PATCH 5/5] drm/sun4i: Substitute sun4i_backend_format_is_yuv() with format->is_yuv

[Ayan Kumar Halder]

drm_format_info table has a field 'is_yuv' to denote if the format
is yuv or not. The driver is expected to use this instead of
having a function for the same purpose.

Signed-off-by: Ayan Kumar halder 
---
 drivers/gpu/drm/sun4i/sun4i_backend.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
b/drivers/gpu/drm/sun4i/sun4i_backend.c
index de0a76d..d7950b5 100644
--- a/drivers/gpu/drm/sun4i/sun4i_backend.c
+++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
@@ -86,12 +86,6 @@ static inline bool 
sun4i_backend_format_is_packed_yuv422(uint32_t format)
}
 }
 
-static inline bool sun4i_backend_format_is_yuv(uint32_t format)
-{
-   return sun4i_backend_format_is_planar_yuv(format) ||
-   sun4i_backend_format_is_packed_yuv422(format);
-}
-
 static void sun4i_backend_apply_color_correction(struct sunxi_engine *engine)
 {
int i;
@@ -304,7 +298,7 @@ int sun4i_backend_update_layer_formats(struct sun4i_backend 
*backend,
   SUN4I_BACKEND_ATTCTL_REG0_LAY_GLBALPHA_EN,
   val);
 
-   if (sun4i_backend_format_is_yuv(fb->format->format))
+   if (fb->format->is_yuv)
return sun4i_backend_update_yuv_format(backend, layer, plane);
 
ret = sun4i_backend_drm_format_to_layer(fb->format->format, );
@@ -384,7 +378,7 @@ int sun4i_backend_update_layer_buffer(struct sun4i_backend 
*backend,
 */
paddr -= PHYS_OFFSET;
 
-   if (sun4i_backend_format_is_yuv(fb->format->format))
+   if (fb->format->is_yuv)
return sun4i_backend_update_yuv_buffer(backend, fb, paddr);
 
/* Write the 32 lower bits of the address (in bits) */
@@ -502,7 +496,7 @@ static int sun4i_backend_atomic_check(struct sunxi_engine 
*engine,
if (fb->format->has_alpha || (plane_state->alpha != 
DRM_BLEND_ALPHA_OPAQUE))
num_alpha_planes++;
 
-   if (sun4i_backend_format_is_yuv(fb->format->format)) {
+   if (fb->format->is_yuv) {
DRM_DEBUG_DRIVER("Plane FB format is YUV\n");
num_yuv_planes++;
}
-- 
2.7.4

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

[PATCH 2/5] drm/i915: Substitute intel_format_is_yuv() with format->is_yuv

[Ayan Kumar Halder]

drm_format_info table has a field 'is_yuv' to denote if the format
is yuv or not. The driver is expected to use this instead of
having a function for the same purpose.

Signed-off-by: Ayan Kumar halder 
---
 drivers/gpu/drm/i915/intel_display.c |  2 +-
 drivers/gpu/drm/i915/intel_drv.h |  2 --
 drivers/gpu/drm/i915/intel_sprite.c  | 20 +++-
 3 files changed, 4 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index fbe5a65..cf09012 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3657,7 +3657,7 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state 
*crtc_state,
plane_color_ctl |= PLANE_COLOR_PLANE_GAMMA_DISABLE;
plane_color_ctl |= glk_plane_color_ctl_alpha(fb->format->format);
 
-   if (intel_format_is_yuv(fb->format->format)) {
+   if (fb->format->is_yuv) {
if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
plane_color_ctl |= 
PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
else
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0c3ac0e..64111ea 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2069,7 +2069,6 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
 
 
 /* intel_sprite.c */
-bool intel_format_is_yuv(u32 format);
 int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
 int usecs);
 struct intel_plane *intel_sprite_plane_create(struct drm_i915_private 
*dev_priv,
@@ -2085,7 +2084,6 @@ void skl_disable_plane(struct intel_plane *plane, struct 
intel_crtc *crtc);
 bool skl_plane_get_hw_state(struct intel_plane *plane, enum pipe *pipe);
 bool skl_plane_has_ccs(struct drm_i915_private *dev_priv,
   enum pipe pipe, enum plane_id plane_id);
-bool intel_format_is_yuv(uint32_t format);
 bool skl_plane_has_planar(struct drm_i915_private *dev_priv,
  enum pipe pipe, enum plane_id plane_id);
 
diff --git a/drivers/gpu/drm/i915/intel_sprite.c 
b/drivers/gpu/drm/i915/intel_sprite.c
index 344c0e7..1bb7bc3 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -41,20 +41,6 @@
 #include 
 #include "i915_drv.h"
 
-bool intel_format_is_yuv(u32 format)
-{
-   switch (format) {
-   case DRM_FORMAT_YUYV:
-   case DRM_FORMAT_UYVY:
-   case DRM_FORMAT_VYUY:
-   case DRM_FORMAT_YVYU:
-   case DRM_FORMAT_NV12:
-   return true;
-   default:
-   return false;
-   }
-}
-
 int intel_usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
 int usecs)
 {
@@ -404,7 +390,7 @@ chv_update_csc(const struct intel_plane_state *plane_state)
const s16 *csc = csc_matrix[plane_state->base.color_encoding];
 
/* Seems RGB data bypasses the CSC always */
-   if (!intel_format_is_yuv(fb->format->format))
+   if (!fb->format->is_yuv)
return;
 
I915_WRITE_FW(SPCSCYGOFF(plane_id), SPCSC_OOFF(0) | SPCSC_IOFF(0));
@@ -439,7 +425,7 @@ vlv_update_clrc(const struct intel_plane_state *plane_state)
enum plane_id plane_id = plane->id;
int contrast, brightness, sh_scale, sh_sin, sh_cos;
 
-   if (intel_format_is_yuv(fb->format->format) &&
+   if (fb->format->is_yuv &&
plane_state->base.color_range == DRM_COLOR_YCBCR_LIMITED_RANGE) {
/*
 * Expand limited range to full range:
@@ -1040,7 +1026,7 @@ intel_check_sprite_plane(struct intel_plane *plane,
src->y1 = src_y << 16;
src->y2 = (src_y + src_h) << 16;
 
-   if (intel_format_is_yuv(fb->format->format) &&
+   if (fb->format->is_yuv &&
fb->format->format != DRM_FORMAT_NV12 &&
(src_x % 2 || src_w % 2)) {
DRM_DEBUG_KMS("src x/w (%u, %u) must be a multiple of 2 
for YUV planes\n",
-- 
2.7.4

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

[PATCH 3/5] drm/rockchip: Substitute is_yuv_support() with format->is_yuv

[Ayan Kumar Halder]

drm_format_info table has a field 'is_yuv' to denote if the format
is yuv or not. The driver is expected to use this instead of
having a function for the same purpose.

Signed-off-by: Ayan Kumar halder 
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 24 +---
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index effecbe..1359e5c 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -243,18 +243,6 @@ static enum vop_data_format vop_convert_format(uint32_t 
format)
}
 }
 
-static bool is_yuv_support(uint32_t format)
-{
-   switch (format) {
-   case DRM_FORMAT_NV12:
-   case DRM_FORMAT_NV16:
-   case DRM_FORMAT_NV24:
-   return true;
-   default:
-   return false;
-   }
-}
-
 static uint16_t scl_vop_cal_scale(enum scale_mode mode, uint32_t src,
  uint32_t dst, bool is_horizontal,
  int vsu_mode, int *vskiplines)
@@ -298,7 +286,8 @@ static void scl_vop_cal_scl_fac(struct vop *vop, const 
struct vop_win_data *win,
uint16_t cbcr_ver_scl_mode = SCALE_NONE;
int hsub = drm_format_horz_chroma_subsampling(pixel_format);
int vsub = drm_format_vert_chroma_subsampling(pixel_format);
-   bool is_yuv = is_yuv_support(pixel_format);
+   const struct drm_format_info *info;
+   bool is_yuv = false;
uint16_t cbcr_src_w = src_w / hsub;
uint16_t cbcr_src_h = src_h / vsub;
uint16_t vsu_mode;
@@ -306,6 +295,11 @@ static void scl_vop_cal_scl_fac(struct vop *vop, const 
struct vop_win_data *win,
uint32_t val;
int vskiplines;
 
+   info = drm_format_info(pixel_format);
+
+   if (info->is_yuv)
+   is_yuv = true;
+
if (dst_w > 3840) {
DRM_DEV_ERROR(vop->dev, "Maximum dst width (3840) exceeded\n");
return;
@@ -680,7 +674,7 @@ static int vop_plane_atomic_check(struct drm_plane *plane,
 * Src.x1 can be odd when do clip, but yuv plane start point
 * need align with 2 pixel.
 */
-   if (is_yuv_support(fb->format->format) && ((state->src.x1 >> 16) % 2)) {
+   if (fb->format->is_yuv && ((state->src.x1 >> 16) % 2)) {
DRM_ERROR("Invalid Source: Yuv format not support odd xpos\n");
return -EINVAL;
}
@@ -767,7 +761,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
VOP_WIN_SET(vop, win, format, format);
VOP_WIN_SET(vop, win, yrgb_vir, DIV_ROUND_UP(fb->pitches[0], 4));
VOP_WIN_SET(vop, win, yrgb_mst, dma_addr);
-   if (is_yuv_support(fb->format->format)) {
+   if (fb->format->is_yuv) {
int hsub = 
drm_format_horz_chroma_subsampling(fb->format->format);
int vsub = 
drm_format_vert_chroma_subsampling(fb->format->format);
int bpp = fb->format->cpp[1];
-- 
2.7.4

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

[PATCH 1/5] drm/fourcc: Add is_yuv field to drm_format_info to denote if the format is yuv

[Ayan Kumar Halder]

A lot of drivers duplicate the function to check if a format is yuv or not.
If we add a field (to denote whether the format is yuv or not) in the
drm_format_info table, all the drivers can use this field and it will
prevent duplication of similar logic.

Signed-off-by: Ayan Kumar halder 
---
 drivers/gpu/drm/drm_fourcc.c | 42 +-
 include/drm/drm_fourcc.h |  2 ++
 2 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 5ca6395..35c1e27 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -152,27 +152,27 @@ const struct drm_format_info *__drm_format_info(u32 
format)
{ .format = DRM_FORMAT_XBGR_A8, .depth = 32, 
.num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_RGBX_A8, .depth = 32, 
.num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
{ .format = DRM_FORMAT_BGRX_A8, .depth = 32, 
.num_planes = 2, .cpp = { 4, 1, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
-   { .format = DRM_FORMAT_YUV410,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
-   { .format = DRM_FORMAT_YVU410,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4 },
-   { .format = DRM_FORMAT_YUV411,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
-   { .format = DRM_FORMAT_YVU411,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1 },
-   { .format = DRM_FORMAT_YUV420,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
-   { .format = DRM_FORMAT_YVU420,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2 },
-   { .format = DRM_FORMAT_YUV422,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_YVU422,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_YUV444,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
-   { .format = DRM_FORMAT_YVU444,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 1, .vsub = 1 },
-   { .format = DRM_FORMAT_NV12,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
-   { .format = DRM_FORMAT_NV21,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 2 },
-   { .format = DRM_FORMAT_NV16,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_NV61,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_NV24,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
-   { .format = DRM_FORMAT_NV42,.depth = 0,  
.num_planes = 2, .cpp = { 1, 2, 0 }, .hsub = 1, .vsub = 1 },
-   { .format = DRM_FORMAT_YUYV,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_YVYU,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_UYVY,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_VYUY,.depth = 0,  
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1 },
-   { .format = DRM_FORMAT_AYUV,.depth = 0,  
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true },
+   { .format = DRM_FORMAT_YUV410,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4, .is_yuv = true },
+   { .format = DRM_FORMAT_YVU410,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 4, .is_yuv = true },
+   { .format = DRM_FORMAT_YUV411,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1, .is_yuv = true },
+   { .format = DRM_FORMAT_YVU411,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 4, .vsub = 1, .is_yuv = true },
+   { .format = DRM_FORMAT_YUV420,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2, .is_yuv = true },
+   { .format = DRM_FORMAT_YVU420,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 2, .is_yuv = true },
+   { .format = DRM_FORMAT_YUV422,  .depth = 0,  
.num_planes = 3, .cpp = { 1, 1, 1 }, .hsub = 2, .vsub = 1, .is_yuv = true },
+   { .format = 

[RESEND 1/2] backlight: Remove s6e63m0 driver

[Krzysztof Kozlowski]

The driver for S6E63M0 AMOLED LCD panel is not used.  It does not
support DeviceTree and respective possible users (S5Pv210 Aquila and
Goni boards) are DeviceTree-only.

Suggested-by: Marek Szyprowski 
Cc: Marek Szyprowski 
Cc: Inki Dae 
Signed-off-by: Krzysztof Kozlowski 
Acked-by: Jingoo Han 
Acked-by: Daniel Thompson 

---

Resending with Jingoo's and Daniel's acks.
---
 drivers/video/backlight/Kconfig |   8 -
 drivers/video/backlight/Makefile|   1 -
 drivers/video/backlight/s6e63m0.c   | 857 
 drivers/video/backlight/s6e63m0_gamma.h | 266 --
 4 files changed, 1132 deletions(-)
 delete mode 100644 drivers/video/backlight/s6e63m0.c
 delete mode 100644 drivers/video/backlight/s6e63m0_gamma.h

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 2919e2334052..2373c3cec0c3 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -111,14 +111,6 @@ config LCD_HP700
  If you have an HP Jornada 700 series handheld (710/720/728)
  say Y to enable LCD control driver.
 
-config LCD_S6E63M0
-   tristate "S6E63M0 AMOLED LCD Driver"
-   depends on SPI && BACKLIGHT_CLASS_DEVICE
-   default n
-   help
- If you have an S6E63M0 LCD Panel, say Y to enable its
- LCD control driver.
-
 config LCD_LD9040
tristate "LD9040 AMOLED LCD Driver"
depends on SPI && BACKLIGHT_CLASS_DEVICE
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index 0dcc2c745c03..c7a46392a76f 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -15,7 +15,6 @@ obj-$(CONFIG_LCD_LMS501KF03)  += lms501kf03.o
 obj-$(CONFIG_LCD_LTV350QV) += ltv350qv.o
 obj-$(CONFIG_LCD_OTM3225A) += otm3225a.o
 obj-$(CONFIG_LCD_PLATFORM) += platform_lcd.o
-obj-$(CONFIG_LCD_S6E63M0)  += s6e63m0.o
 obj-$(CONFIG_LCD_TDO24M)   += tdo24m.o
 obj-$(CONFIG_LCD_TOSA) += tosa_lcd.o
 obj-$(CONFIG_LCD_VGG2432A4)+= vgg2432a4.o
diff --git a/drivers/video/backlight/s6e63m0.c 
b/drivers/video/backlight/s6e63m0.c
deleted file mode 100644
index 3c4a22a3063a..
--- a/drivers/video/backlight/s6e63m0.c
+++ /dev/null
@@ -1,857 +0,0 @@
-/*
- * S6E63M0 AMOLED LCD panel driver.
- *
- * Author: InKi Dae  
- *
- * Derived from drivers/video/omap/lcd-apollon.c
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "s6e63m0_gamma.h"
-
-#define SLEEPMSEC  0x1000
-#define ENDDEF 0x2000
-#defineDEFMASK 0xFF00
-#define COMMAND_ONLY   0xFE
-#define DATA_ONLY  0xFF
-
-#define MIN_BRIGHTNESS 0
-#define MAX_BRIGHTNESS 10
-
-struct s6e63m0 {
-   struct device   *dev;
-   struct spi_device   *spi;
-   unsigned intpower;
-   unsigned intcurrent_brightness;
-   unsigned intgamma_mode;
-   unsigned intgamma_table_count;
-   struct lcd_device   *ld;
-   struct backlight_device *bd;
-   struct lcd_platform_data*lcd_pd;
-};
-
-static const unsigned short seq_panel_condition_set[] = {
-   0xF8, 0x01,
-   DATA_ONLY, 0x27,
-   DATA_ONLY, 0x27,
-   DATA_ONLY, 0x07,
-   DATA_ONLY, 0x07,
-   DATA_ONLY, 0x54,
-   DATA_ONLY, 0x9f,
-   DATA_ONLY, 0x63,
-   DATA_ONLY, 0x86,
-   DATA_ONLY, 0x1a,
-   DATA_ONLY, 0x33,
-   DATA_ONLY, 0x0d,
-   DATA_ONLY, 0x00,
-   DATA_ONLY, 0x00,
-
-   ENDDEF, 0x
-};
-
-static const unsigned short seq_display_condition_set[] = {
-   0xf2, 0x02,
-   DATA_ONLY, 0x03,
-   DATA_ONLY, 0x1c,
-   DATA_ONLY, 0x10,
-   DATA_ONLY, 0x10,
-
-   0xf7, 0x03,
-   DATA_ONLY, 0x00,
-   DATA_ONLY, 0x00,
-
-   ENDDEF, 0x
-};
-
-static const unsigned short seq_gamma_setting[] = {
-   0xfa, 0x00,
-   DATA_ONLY, 0x18,
-   DATA_ONLY, 0x08,
-   DATA_ONLY, 0x24,
-   DATA_ONLY, 0x64,
-   DATA_ONLY, 0x56,
-   DATA_ONLY, 0x33,
-   DATA_ONLY, 0xb6,
-   DATA_ONLY, 0xba,
-   DATA_ONLY, 0xa8,
-   DATA_ONLY, 0xac,
-   DATA_ONLY, 0xb1,
-   DATA_ONLY, 0x9d,
-   DATA_ONLY, 0xc1,
-   DATA_ONLY, 0xc1,
-   DATA_ONLY, 0xb7,
-   DATA_ONLY, 0x00,
-   DATA_ONLY, 0x9c,
-   DATA_ONLY, 0x00,
-   DATA_ONLY, 0x9f,
-   DATA_ONLY, 0x00,
-   DATA_ONLY, 0xd6,
-
-   0xfa, 0x01,
-
-   ENDDEF, 0x
-};
-
-static 

[RESEND 2/2] backlight: Remove ld9040 driver

[Krzysztof Kozlowski]

The driver for LD9040 AMOLED LCD panel was superseded with DRM driver
panel-samsung-ld9040.c.  It does not support DeviceTree and respective
possible user (Exynos4210 Universal C210) is DeviceTree-only and uses
DRM version of driver.

Suggested-by: Marek Szyprowski 
Cc: Marek Szyprowski 
Cc: Inki Dae 
Signed-off-by: Krzysztof Kozlowski 
Acked-by: Jingoo Han 
Acked-by: Daniel Thompson 

---

Resending with Jingoo's and Daniel's acks.
---
 drivers/video/backlight/Kconfig|   8 -
 drivers/video/backlight/Makefile   |   1 -
 drivers/video/backlight/ld9040.c   | 811 -
 drivers/video/backlight/ld9040_gamma.h | 202 
 4 files changed, 1022 deletions(-)
 delete mode 100644 drivers/video/backlight/ld9040.c
 delete mode 100644 drivers/video/backlight/ld9040_gamma.h

diff --git a/drivers/video/backlight/Kconfig b/drivers/video/backlight/Kconfig
index 2373c3cec0c3..71ee978c848f 100644
--- a/drivers/video/backlight/Kconfig
+++ b/drivers/video/backlight/Kconfig
@@ -111,14 +111,6 @@ config LCD_HP700
  If you have an HP Jornada 700 series handheld (710/720/728)
  say Y to enable LCD control driver.
 
-config LCD_LD9040
-   tristate "LD9040 AMOLED LCD Driver"
-   depends on SPI && BACKLIGHT_CLASS_DEVICE
-   default n
-   help
- If you have an LD9040 Panel, say Y to enable its
- control driver.
-
 config LCD_AMS369FG06
tristate "AMS369FG06 AMOLED LCD Driver"
depends on SPI && BACKLIGHT_CLASS_DEVICE
diff --git a/drivers/video/backlight/Makefile b/drivers/video/backlight/Makefile
index c7a46392a76f..63c507c07437 100644
--- a/drivers/video/backlight/Makefile
+++ b/drivers/video/backlight/Makefile
@@ -9,7 +9,6 @@ obj-$(CONFIG_LCD_HX8357)+= hx8357.o
 obj-$(CONFIG_LCD_ILI922X)  += ili922x.o
 obj-$(CONFIG_LCD_ILI9320)  += ili9320.o
 obj-$(CONFIG_LCD_L4F00242T03)  += l4f00242t03.o
-obj-$(CONFIG_LCD_LD9040)   += ld9040.o
 obj-$(CONFIG_LCD_LMS283GF05)   += lms283gf05.o
 obj-$(CONFIG_LCD_LMS501KF03)   += lms501kf03.o
 obj-$(CONFIG_LCD_LTV350QV) += ltv350qv.o
diff --git a/drivers/video/backlight/ld9040.c b/drivers/video/backlight/ld9040.c
deleted file mode 100644
index 677f8abba27c..
--- a/drivers/video/backlight/ld9040.c
+++ /dev/null
@@ -1,811 +0,0 @@
-/*
- * ld9040 AMOLED LCD panel driver.
- *
- * Copyright (c) 2011 Samsung Electronics
- * Author: Donghwa Lee  
- * Derived from drivers/video/backlight/s6e63m0.c
- *
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
- */
-
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "ld9040_gamma.h"
-
-#define SLEEPMSEC  0x1000
-#define ENDDEF 0x2000
-#defineDEFMASK 0xFF00
-#define COMMAND_ONLY   0xFE
-#define DATA_ONLY  0xFF
-
-#define MIN_BRIGHTNESS 0
-#define MAX_BRIGHTNESS 24
-
-struct ld9040 {
-   struct device   *dev;
-   struct spi_device   *spi;
-   unsigned intpower;
-   unsigned intcurrent_brightness;
-
-   struct lcd_device   *ld;
-   struct backlight_device *bd;
-   struct lcd_platform_data*lcd_pd;
-
-   struct mutexlock;
-   bool  enabled;
-};
-
-static struct regulator_bulk_data supplies[] = {
-   { .supply = "vdd3", },
-   { .supply = "vci", },
-};
-
-static void ld9040_regulator_enable(struct ld9040 *lcd)
-{
-   int ret = 0;
-   struct lcd_platform_data *pd = NULL;
-
-   pd = lcd->lcd_pd;
-   mutex_lock(>lock);
-   if (!lcd->enabled) {
-   ret = regulator_bulk_enable(ARRAY_SIZE(supplies), supplies);
-   if (ret)
-   goto out;
-
-   lcd->enabled = true;
-   }
-   msleep(pd->power_on_delay);
-out:
-   mutex_unlock(>lock);
-}
-
-static void ld9040_regulator_disable(struct ld9040 *lcd)
-{
-   int ret = 0;
-
-   mutex_lock(>lock);
-   if (lcd->enabled) {
-   ret = regulator_bulk_disable(ARRAY_SIZE(supplies), supplies);
-   if (ret)
-   goto out;
-
-   lcd->enabled = false;
-   }
-out:
-   mutex_unlock(>lock);
-}
-
-static const unsigned short seq_swreset[] = {
-   0x01, COMMAND_ONLY,
-   ENDDEF, 0x00
-};
-
-static const unsigned short seq_user_setting[] = {
-   0xF0, 0x5A,
-
-   DATA_ONLY, 0x5A,
-   ENDDEF, 0x00
-};
-
-static const unsigned short seq_elvss_on[] = {
-   0xB1, 0x0D,
-
-   DATA_ONLY, 0x00,
-   DATA_ONLY, 0x16,
-   ENDDEF, 

Re: [Intel-gfx] [PATCH 1/6] drm: Let userspace check if driver supports modeset

[Souza, Jose]

On Tue, 2018-07-17 at 08:28 +0100, Chris Wilson wrote:
> Quoting José Roberto de Souza (2018-07-16 23:38:36)
> > GPU accelerators usually don't have display block or the display
> > driver part can be disable when building driver(for servers it save
> > some resources) so it is important let userspace check this
> > capability too.
> 
> We currently communicate that by having no modeset resources. What
> does
> another cap bit accomplish that we don't already know?

This is a hackish way to check if driver support modeset,
drmModeGetResources()/drm_mode_getresources() can fail and return null
by other reasons and just check for the errno value can be misleading
too.

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

Re: [Nouveau] [PATCH 1/5] drm/nouveau: Prevent RPM callback recursion in suspend/resume paths

[Lyude Paul]

On Tue, 2018-07-17 at 09:16 +0200, Lukas Wunner wrote:
> [cc += linux-pm]
> 
> Hi Lyude,
> 
> First of all, thanks a lot for looking into this. 
> 
> On Mon, Jul 16, 2018 at 07:59:25PM -0400, Lyude Paul wrote:
> > In order to fix all of the spots that need to have runtime PM get/puts()
> > added, we need to ensure that it's possible for us to call
> > pm_runtime_get/put() in any context, regardless of how deep, since
> > almost all of the spots that are currently missing refs can potentially
> > get called in the runtime suspend/resume path. Otherwise, we'll try to
> > resume the GPU as we're trying to resume the GPU (and vice-versa) and
> > cause the kernel to deadlock.
> > 
> > With this, it should be safe to call the pm runtime functions in any
> > context in nouveau with one condition: any point in the driver that
> > calls pm_runtime_get*() cannot hold any locks owned by nouveau that
> > would be acquired anywhere inside nouveau_pmops_runtime_resume().
> > This includes modesetting locks, i2c bus locks, etc.
> 
> [snip]
> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *dev)
> > return -EBUSY;
> > }
> >  
> > +   dev->power.disable_depth++;
> > +
> 
> I'm not sure if that variable is actually private to the PM core.
> Grepping through the tree I only find a single occurrence where it's
> accessed outside the PM core and that's in amdgpu.  So this looks
> a little fishy TBH.  It may make sense to cc such patches to linux-pm
> to get Rafael & other folks involved with the PM core to comment.
> 
> Also, the disable_depth variable only exists if the kernel was
> compiled with CONFIG_PM enabled, but I can't find a "depends on PM"
> or something like that in nouveau's Kconfig.  Actually, if PM is
> not selected, all the nouveau_pmops_*() functions should be #ifdef'ed
> away, but oddly there's no #ifdef CONFIG_PM anywhere in nouveau_drm.c.
> 
> Anywayn, if I understand the commit message correctly, you're hitting a
> pm_runtime_get_sync() in a code path that itself is called during a
> pm_runtime_get_sync().  Could you include stack traces in the commit
> message?  My gut feeling is that this patch masks a deeper issue,
> e.g. if the runtime_resume code path does in fact directly poll outputs,
> that would seem wrong.  Runtime resume should merely make the card
> accessible, i.e. reinstate power if necessary, put into PCI_D0,
> restore registers, etc.  Output polling should be scheduled
> asynchronously.

Since it is apparently internal to the RPM core (I should go fix the references
to that which I added in amdgpu as well then, whoops...) I will have to figure
out another way to do this.

So: the reason that patch was added was mainly for the patches later in the
series that add guards around the i2c bus and aux bus, since both of those
require that the device be awake for it to work. Currently, the spot where it
would recurse is:

[   72.126859] nouveau :01:00.0: DRM: suspending console...
[   72.127161] nouveau :01:00.0: DRM: suspending display...
[  246.718589] INFO: task kworker/0:1:60 blocked for more than 120 seconds.
[  246.719254]   Tainted: G   O  4.18.0-rc5Lyude-Test+ #3
[  246.719411] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this
message.
[  246.719527] kworker/0:1 D060  2 0x8000
[  246.719636] Workqueue: pm pm_runtime_work
[  246.719772] Call Trace:
[  246.719874]  __schedule+0x322/0xaf0
[  246.722800]  schedule+0x33/0x90
[  246.724269]  rpm_resume+0x19c/0x850
[  246.725128]  ? finish_wait+0x90/0x90
[  246.725990]  __pm_runtime_resume+0x4e/0x90
[  246.726876]  nvkm_i2c_aux_acquire+0x39/0xc0 [nouveau]
[  246.727713]  nouveau_connector_aux_xfer+0x5c/0xd0 [nouveau]
[  246.728546]  drm_dp_dpcd_access+0x77/0x110 [drm_kms_helper]
[  246.729349]  drm_dp_dpcd_write+0x2b/0xb0 [drm_kms_helper]
[  246.730085]  drm_dp_mst_topology_mgr_suspend+0x4e/0x90 [drm_kms_helper]
[  246.730828]  nv50_display_fini+0xa5/0xc0 [nouveau]
[  246.731606]  nouveau_display_fini+0xc8/0x100 [nouveau]
[  246.732375]  nouveau_display_suspend+0x62/0x110 [nouveau]
[  246.733106]  nouveau_do_suspend+0x5e/0x2d0 [nouveau]
[  246.733839]  nouveau_pmops_runtime_suspend+0x4f/0xb0 [nouveau]
[  246.734585]  pci_pm_runtime_suspend+0x6b/0x190
[  246.735297]  ? pci_has_legacy_pm_support+0x70/0x70
[  246.736044]  __rpm_callback+0x7a/0x1d0
[  246.736742]  ? pci_has_legacy_pm_support+0x70/0x70
[  246.737467]  rpm_callback+0x24/0x80
[  246.738165]  ? pci_has_legacy_pm_support+0x70/0x70
[  246.738864]  rpm_suspend+0x142/0x6b0
[  246.739593]  pm_runtime_work+0x97/0xc0
[  246.740312]  process_one_work+0x231/0x620
[  246.741028]  worker_thread+0x44/0x3a0
[  246.741731]  kthread+0x12b/0x150
[  246.742439]  ? wq_pool_ids_show+0x140/0x140
[  246.743149]  ? kthread_create_worker_on_cpu+0x70/0x70
[  246.743846]  ret_from_fork+0x3a/0x50
[  246.744601] 
   

[Bug 107153] 4.18-rc3 crash on hdmi (0010:dm_update_crtcs_state+0x41e/0x4a0)

[bugzilla-daemon]

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

--- Comment #13 from Patrik Kullman  ---
Interesting, thanks Peter!
I don't quite have the same combos, booting with receiver and TV on still
crashes the driver. I'll try some with this as well.

I have now tried rc5 and bug is still present.

Another detail is that after booting and connecting the HDMI, the driver
crashes. Subsequent disconnect/connects gives:

 [drm] amdgpu_dm_irq_schedule_work FAILED src 6

-- 
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/amdgpu: fix spelling mistake "successed" -> "succeeded"

[Alex Deucher]

On Tue, Jul 17, 2018 at 5:29 AM, Colin King  wrote:
> From: Colin Ian King 
>
> Trivial fix to spelling mistake in dev_err error message.
>
> Signed-off-by: Colin Ian King 

Applied.  thanks!

Alex


> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 9883fa9bb41b..e9feb3c58389 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2004,7 +2004,7 @@ static int amdgpu_device_ip_reinit_early_sriov(struct 
> amdgpu_device *adev)
> continue;
>
> r = block->version->funcs->hw_init(adev);
> -   DRM_INFO("RE-INIT: %s %s\n", 
> block->version->funcs->name, r?"failed":"successed");
> +   DRM_INFO("RE-INIT: %s %s\n", 
> block->version->funcs->name, r?"failed":"succeeded");
> if (r)
> return r;
> }
> @@ -2039,7 +2039,7 @@ static int amdgpu_device_ip_reinit_late_sriov(struct 
> amdgpu_device *adev)
> continue;
>
> r = block->version->funcs->hw_init(adev);
> -   DRM_INFO("RE-INIT: %s %s\n", 
> block->version->funcs->name, r?"failed":"successed");
> +   DRM_INFO("RE-INIT: %s %s\n", 
> block->version->funcs->name, r?"failed":"succeeded");
> if (r)
> return r;
> }
> @@ -3092,7 +3092,7 @@ static int amdgpu_device_handle_vram_lost(struct 
> amdgpu_device *adev)
>   * @adev: amdgpu device pointer
>   *
>   * attempt to do soft-reset or full-reset and reinitialize Asic
> - * return 0 means successed otherwise failed
> + * return 0 means succeeded otherwise failed
>   */
>  static int amdgpu_device_reset(struct amdgpu_device *adev)
>  {
> @@ -3170,7 +3170,7 @@ static int amdgpu_device_reset(struct amdgpu_device 
> *adev)
>   * @from_hypervisor: request from hypervisor
>   *
>   * do VF FLR and reinitialize Asic
> - * return 0 means successed otherwise failed
> + * return 0 means succeeded otherwise failed
>   */
>  static int amdgpu_device_reset_sriov(struct amdgpu_device *adev,
>  bool from_hypervisor)
> @@ -3295,7 +3295,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device 
> *adev,
> dev_info(adev->dev, "GPU reset(%d) failed\n", 
> atomic_read(>gpu_reset_counter));
> amdgpu_vf_error_put(adev, AMDGIM_ERROR_VF_GPU_RESET_FAIL, 0, 
> r);
> } else {
> -   dev_info(adev->dev, "GPU reset(%d) 
> successed!\n",atomic_read(>gpu_reset_counter));
> +   dev_info(adev->dev, "GPU reset(%d) 
> succeeded!\n",atomic_read(>gpu_reset_counter));
> }
>
> amdgpu_vf_error_trans_all(adev);
> --
> 2.17.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 v4 10/11] media: vsp1: Support Interlaced display pipelines

[Kieran Bingham]

Hi Laurent,

On 17/07/18 13:52, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Monday, 16 July 2018 21:21:00 EEST Kieran Bingham wrote:
>> On 24/05/18 13:51, Laurent Pinchart wrote:
>>> On Thursday, 3 May 2018 16:36:21 EEST Kieran Bingham wrote:
 Calculate the top and bottom fields for the interlaced frames and
 utilise the extended display list command feature to implement the
 auto-field operations. This allows the DU to update the VSP2 registers
 dynamically based upon the currently processing field.

 Signed-off-by: Kieran Bingham 

 ---

 v3:
  - Pass DL through partition calls to allow autocmd's to be retrieved
  - Document interlaced field in struct vsp1_du_atomic_config

 v2:
  - fix erroneous BIT value which enabled interlaced
  - fix field handling at frame_end interrupt

 ---

  drivers/media/platform/vsp1/vsp1_dl.c   | 10 -
  drivers/media/platform/vsp1/vsp1_drm.c  | 11 -
  drivers/media/platform/vsp1/vsp1_regs.h |  1 +-
  drivers/media/platform/vsp1/vsp1_rpf.c  | 71 --
  drivers/media/platform/vsp1/vsp1_rwpf.h |  1 +-
  include/media/vsp1.h|  2 +-
  6 files changed, 93 insertions(+), 3 deletions(-)
> 
> [snip]
> 
 diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
 b/drivers/media/platform/vsp1/vsp1_drm.c index 2c3db8b8adce..cc29c9d96bb7
 100644
 --- a/drivers/media/platform/vsp1/vsp1_drm.c
 +++ b/drivers/media/platform/vsp1/vsp1_drm.c
 @@ -811,6 +811,17 @@ int vsp1_du_atomic_update(struct device *dev,
 unsigned
 int pipe_index, return -EINVAL;

}

 +  if (!(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) && cfg->interlaced) {
>>>
>>> Nitpicking, writing the condition as
>>>
>>> if (cfg->interlaced && !(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)))
>>
>> Done.
>>
>>> would match the comment better. You can also drop the parentheses around
>>> the vsp1_feature() call.
>>>
 +  /*
 +   * Interlaced support requires extended display lists to
 +   * provide the auto-fld feature with the DU.
 +   */
 +  dev_dbg(vsp1->dev, "Interlaced unsupported on this output\n");
>>>
>>> Could we catch this in the DU driver to fail atomic test ?
>>
>> Ugh - I thought moving the configuration to vsp1_du_setup_lif() would
>> give us this, but that return value is not checked in the DU.
>>
>> How can we interogate the VSP1 to ask it if it supports interlaced from
>> rcar_du_vsp_plane_atomic_check()?
>>
>>
>> Some dummy call to vsp1_du_setup_lif() to check the return value ? Or
>> should we implement a hook to call through to perform checks in the VSP1
>> DRM API?
> 
> Would it be possible to just infer that from the DU compatible string, 
> without 
> querying the VSP driver ? Of course that's a bit of a layering violation, but 
> as we know what type of VSP instance is present in each SoC, such a small 
> hack 
> wouldn't hurt in my opinion. If the need arises later we can introduce an API 
> to query the information from the VSP driver.

I'm not sure what there is to match on currently.

I thought that we had restrictions on which display pipelines supported
interlaced. (i.e. D3/E3 might not) - but they seem to support extended
display lists ...

So isn't it the case that any pipeline which we connect to DRM supports
interlaced? (currently) - we can't / don't physically connect other VSP
entities to the DRM pipes...


> 
 +  return -EINVAL;
 +  }
 +
 +  rpf->interlaced = cfg->interlaced;
 +
rpf->fmtinfo = fmtinfo;
rpf->format.num_planes = fmtinfo->planes;
rpf->format.plane_fmt[0].bytesperline = cfg->pitch;
> 
> [snip]


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

[Bug 107263] Decrease start-up time of amdgpu_init from 390 ms to under 100 ms

[bugzilla-daemon]

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

Alex Deucher  changed:

   What|Removed |Added

   Severity|normal  |enhancement

-- 
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 v4 09/11] media: vsp1: Provide support for extended command pools

[Kieran Bingham]

Hi Laurent,

Thanks for your review comments.

These were easier to go through than patch 8 :D

On 24/05/18 13:12, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Thursday, 3 May 2018 16:36:20 EEST Kieran Bingham wrote:
>> VSPD and VSP-DL devices can provide extended display lists supporting
>> extended command display list objects.
>>
>> These extended commands require their own dma memory areas for a header
>> and body specific to the command type.
>>
>> Implement a command pool to allocate all necessary memory in a single
>> DMA allocation to reduce pressure on the TLB, and provide convenient
>> re-usable command objects for the entities to utilise.
>>
>> Signed-off-by: Kieran Bingham 
>>
>> ---
>>
>> v2:
>>  - Fix spelling typo in commit message
>>  - constify, and staticify the instantiation of vsp1_extended_commands
>>  - s/autfld_cmds/autofld_cmds/
>>  - staticify cmd pool functions (Thanks kbuild-bot)
>> ---
>>  drivers/media/platform/vsp1/vsp1_dl.c | 191 +++-
>>  drivers/media/platform/vsp1/vsp1_dl.h |   3 +-
>>  2 files changed, 194 insertions(+)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
>> b/drivers/media/platform/vsp1/vsp1_dl.c index b64d32535edc..d33ae5f125bd
>> 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
>> @@ -117,6 +117,30 @@ struct vsp1_dl_body_pool {
>>  };
>>
>>  /**
>> + * struct vsp1_cmd_pool - display list body pool
> 
> The structure is called vsp1_dl_cmd_pool. I would document it as "Display 
> list 
> commands pool", not "body pool".
> 
>> + * @dma: DMA address of the entries
>> + * @size: size of the full DMA memory pool in bytes
>> + * @mem: CPU memory pointer for the pool
>> + * @bodies: Array of DLB structures for the pool
> 
> The field is called cmds.
> 

Fixed

>> + * @free: List of free DLB entries
> 
> "free pool entries" or "free command entries" ?
> 

Fixed

>> + * @lock: Protects the pool and free list
> 
> The pool is the whole structure, does the lock really protects all fields ?

Perhaps in theory, but not in practice. Only the free-list is protected
by this lock. Adjusting the comment.


> 
>> + * @vsp1: the VSP1 device
>> + */
>> +struct vsp1_dl_cmd_pool {
>> +/* DMA allocation */
>> +dma_addr_t dma;
>> +size_t size;
>> +void *mem;
>> +
>> +struct vsp1_dl_ext_cmd *cmds;
>> +struct list_head free;
>> +
>> +spinlock_t lock;
>> +
>> +struct vsp1_device *vsp1;
>> +};
>> +
>> +/**
>>   * struct vsp1_dl_list - Display list
>>   * @list: entry in the display list manager lists
>>   * @dlm: the display list manager
>> @@ -162,6 +186,7 @@ struct vsp1_dl_list {
>>   * @queued: list queued to the hardware (written to the DL registers)
>>   * @pending: list waiting to be queued to the hardware
>>   * @pool: body pool for the display list bodies
>> + * @autofld_cmds: command pool to support auto-fld interlaced mode
> 
> This could also be used for auto-disp. How about calling it cmdpool ?

Hrm ... I think originally I wanted to keep this specific to auto-fld,
as that is the 'type' which gets instatiated. But we can not have both
auto-fld and auto-disp at the same time, so cmdpool is fine. If we ever
find we need a pre_cmdpool, and a post_cmdpool, we can rename as
appropriate.

> 
>>   */
>>  struct vsp1_dl_manager {
>>  unsigned int index;
>> @@ -175,6 +200,7 @@ struct vsp1_dl_manager {
>>  struct vsp1_dl_list *pending;
>>
>>  struct vsp1_dl_body_pool *pool;
>> +struct vsp1_dl_cmd_pool *autofld_cmds;
>>  };
>>
>>  /* 
>> @@ -338,6 +364,140 @@ void vsp1_dl_body_write(struct vsp1_dl_body *dlb,
>> u32 reg, u32 data) }
>>
>>  /* 
>> + * Display List Extended Command Management
>> + */
>> +
>> +enum vsp1_extcmd_type {
>> +VSP1_EXTCMD_AUTODISP,
>> +VSP1_EXTCMD_AUTOFLD,
>> +};
>> +
>> +struct vsp1_extended_command_info {
>> +u16 opcode;
>> +size_t body_size;
>> +} static const vsp1_extended_commands[] = {
> 
> The location of the static const keywords is strange. I would move them 
> before 
> struct, or split the structure definition and variable declaration.

I'll split them.


> 
>> +[VSP1_EXTCMD_AUTODISP] = { 0x02, 96 },
>> +[VSP1_EXTCMD_AUTOFLD]  = { 0x03, 160 },
>> +};
>> +
>> +/**
>> + * vsp1_dl_cmd_pool_create - Create a pool of commands from a single
>> allocation
>> + * @vsp1: The VSP1 device
>> + * @type: The command pool type
>> + * @num_commands: The quantity of commands to allocate
> 
> s/quantity/number/ ?

Sure ... also s/num_commands/num_cmds/

>> + *
>> + * Allocate a pool of commands each with enough memory to contain the
>> private
>> + * data of each command. The allocation sizes are dependent upon the
>> command
>> + * type.
>> + *
>> + * Return a pointer to a pool on success or NULL if memory can't be
> 
> s/a pool/the pool/
> 

[Bug 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #7 from Alex Deucher (alexdeuc...@gmail.com) ---
Created attachment 277375
  --> https://bugzilla.kernel.org/attachment.cgi?id=277375=edit
possible fix

Does this patch fix the issue?

-- 
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 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #6 from bakarichar...@gmail.com ---
Created attachment 277373
  --> https://bugzilla.kernel.org/attachment.cgi?id=277373=edit
dmesg full

-- 
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 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #5 from bakarichar...@gmail.com ---
Created attachment 277371
  --> https://bugzilla.kernel.org/attachment.cgi?id=277371=edit
lspci -nn

-- 
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 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #4 from bakarichar...@gmail.com ---
Hi,

Sorry for the duplication. It's worth to mention that APIC problems occured too
which have been fixed using kernel parameters. (manual pci addressing)

-- 
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 v2] drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit

[Maxime Ripard]

On Tue, Jul 17, 2018 at 02:25:22PM +0200, Paul Kocialkowski wrote:
> Blending order is set based on the z position of each DRM plane. The
> blending order register is currently cleared at each atomic DRM commit,
> with the intent that each committed plane will set the appropriate
> bits (based on its z-pos) when enabling the plane.
> 
> However, it sometimes happens that a particular plane is left unchanged
> by an atomic commit and thus will not be configured again. In that
> scenario, blending order is cleared and only the bits relevant for the
> planes affected by the commit are set. This leaves the planes that did
> not change without their blending order set in the register, leading
> to that plane not being displayed.
> 
> Instead of clearing the blending order register at every atomic commit,
> this change moves the register's initial clear at bind time and only
> clears the bits for a specific plane when disabling it or changing its
> zpos.
> 
> This way, planes that are left untouched by a DRM atomic commit are
> no longer disabled.
> 
> Signed-off-by: Paul Kocialkowski 

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


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

Re: [PATCH 09/12] nubus: use for_each_if

[Geert Uytterhoeven]

Hi Daniel,

On Mon, Jul 9, 2018 at 10:44 AM Daniel Vetter  wrote:
> Avoids the inverted check compared to the open-coded version.
>
> Signed-off-by: Daniel Vetter 
> Cc: Finn Thain 
> Cc: linux-m...@lists.linux-m68k.org

Thanks for your patch!

> --- a/include/linux/nubus.h
> +++ b/include/linux/nubus.h
> @@ -127,7 +127,7 @@ struct nubus_rsrc *nubus_next_rsrc_or_null(struct 
> nubus_rsrc *from);
> for (f = nubus_first_rsrc_or_null(); f; f = 
> nubus_next_rsrc_or_null(f))
>
>  #define for_each_board_func_rsrc(b, f) \
> -   for_each_func_rsrc(f) if (f->board != b) {} else
> +   for_each_func_rsrc(f) for_each_if (f->board == b)

drivers/net/ethernet/8390/mac8390.c: In function ‘mac8390_device_probe’:
drivers/net/ethernet/8390/mac8390.c:402: error: implicit declaration
of function ‘for_each_if’
drivers/net/ethernet/8390/mac8390.c:402: error: expected ‘;’ before ‘{’ token

Apparently this depends on patch [01/12], which wasn't CCed to
linux-m...@lists.linux-m68k.org?

Please don't split CCs if all patches in the a series are not independent.
Thanks!

Gr{oetje,eeting}s,

Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

--- Comment #3 from Alex Deucher (alexdeuc...@gmail.com) ---
Please attach your full dmesg output from boot and `lspci -nn` output.

-- 
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 200517] Vega 8/Radeon 535 hybrid graphics - amdgpu crash on modesetting

[bugzilla-daemon]

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

Alex Deucher (alexdeuc...@gmail.com) changed:

   What|Removed |Added

 CC||alexdeuc...@gmail.com

--- Comment #2 from Alex Deucher (alexdeuc...@gmail.com) ---
Duplicate of:
https://bugs.freedesktop.org/show_bug.cgi?id=105760

-- 
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 107262] [drm] BIOS signature incorrect 0 0 with Ryzen 3 2200g

[bugzilla-daemon]

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

--- Comment #3 from Paul Menzel  ---
Thank you for the explanation. Could the message then please be improved to not
cause confusion?

-- 
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 v4 08/11] media: vsp1: Add support for extended display list headers

[Kieran Bingham]

Hi Laurent,

On 17/07/18 11:53, Laurent Pinchart wrote:
> Hi Kieran,
> 
> On Monday, 16 July 2018 20:14:55 EEST Kieran Bingham wrote:
>> On 24/05/18 12:44, Laurent Pinchart wrote:
>>> On Thursday, 3 May 2018 16:36:19 EEST Kieran Bingham wrote:
 Extended display list headers allow pre and post command lists to be
 executed by the VSP pipeline. This provides the base support for
 features such as AUTO_FLD (for interlaced support) and AUTO_DISP (for
 supporting continuous camera preview pipelines.

 Signed-off-by: Kieran Bingham 

 ---

 v2:
  - remove __packed attributes

 ---

  drivers/media/platform/vsp1/vsp1.h  |  1 +-
  drivers/media/platform/vsp1/vsp1_dl.c   | 83 +-
  drivers/media/platform/vsp1/vsp1_dl.h   | 29 -
  drivers/media/platform/vsp1/vsp1_drv.c  |  7 +-
  drivers/media/platform/vsp1/vsp1_regs.h |  5 +-
  5 files changed, 116 insertions(+), 9 deletions(-)
> 
> [snip]
> 
 diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
 b/drivers/media/platform/vsp1/vsp1_dl.c index 56514cd51c51..b64d32535edc
 100644
 --- a/drivers/media/platform/vsp1/vsp1_dl.c
 +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> 
> [snip]
> 
 +struct vsp1_dl_ext_header {
 +  u32 reserved0;  /* alignment padding */
 +
 +  u16 pre_ext_cmd_qty;
>>>
>>> Should this be called pre_ext_dl_num_cmd to match the datasheet ?
>>
>> Yes, renamed.
>>
 +  u16 flags;
>>>
>>> Aren't the flags supposed to come before the pre_ext_dl_num_cmd field ?
>>
>> These are out-of-order to account for the fact that they are 16bit values.
> 
> Ah OK. It makes sense, but is a bit confusing when reading the datasheet.

Yes, I agree. Realising the byte-ordering was off was a bit of a pain
point when I was testing too :D

> 
>> I felt that keeping them described in the struct was cleaner than shifts
>> and masks - but clearly this stands out, due to the byte-ordering.
>>
>> Would you prefer I re-write this as 32 bit accesses (or even 64bit),
>> with shifts? Or is a comment sufficient here ?
> 
> If it doesn't make the code too ugly, using larger accesses would be better I 
> think. Otherwise a comment would do I suppose.
> 
>> If we rewrite to be 32 bit accesses, would you be happy with the
>> following naming:
>>
>>  u32 reserved0;
>>  u32 pre_ext_dl_num_cmd; /* Also stores command flags. */
>>  u32 pre_ext_dl_plist;
>>  u32 post_ext_dl_num_cmd;
>>  u32 post_ext_dl_plist;
>>
>> (Technically the flags are for the whole header, not the just the
>> pre_ext, which is why I wanted it separated)
>>
>>
>> Actually - now I write that - the 'number of commands' is sort of a flag
>> or a parameter? so maybe the following is just as appropriate?:
>>
>>  u32 reserved0;
> 
> Maybe "u32 zero;" or "u32 padding;" ? The datasheet states this is padding 
> for 
> alignment purpose.

I've used "padding".


> 
>>  u32 pre_ext_dl_flags;
>>  u32 pre_ext_dl_plist;
>>  u32 post_ext_dl_flags;
>>  u32 post_ext_dl_plist;
>>
>> Or they could be named 'options', or parameters?
>>
>> Any comments before I hack that in?
>>
>> The annoying part is that the 'flags' aren't part of the pre_ext cmds,
>> they declare whether the pre or post cmd should be executed (or both I
>> presume, we are yet to see post-cmd usage)
> 
> I agree with you, having a separate flag field would be nicer, as the flags 
> are shared. I'll chose the easy option of letting you decide what you like 
> best :-) All the above options are equally good to me, provided you add a 
> comment explaining why the flag comes after the num_cmd field if you decide 
> to 
> keep it as a separate field.

I've added a comment to explain why the flags must be after num_cmd. I
feel it's better to keep the flags separated as they are not specific to
the pre_cmd.


> 
 +  u32 pre_ext_cmd_plist;
>>>
>>> And pre_ext_dl_plist ?
>>>
 +
 +  u32 post_ext_cmd_qty;
 +  u32 post_ext_cmd_plist;
>>>
>>> Similar comments for these variables.
>>
>> Renamed.
>>
 +};
 +
 +struct vsp1_dl_header_extended {
 +  struct vsp1_dl_header header;
 +  struct vsp1_dl_ext_header ext;
 +};
 +
  struct vsp1_dl_entry {
u32 addr;
u32 data;
  };

 +struct vsp1_dl_ext_cmd_header {
>>>
>>> Isn't this referred to in the datasheet as a body entry, not a header ?
>>> How about naming it vsp1_dl_ext_cmd_entry ? Or just vsp1_dl_ext_cmd (in
>>> which case the other structure that goes by the same name would need to be
>>> renamed) ?
>>
>> I think I was getting too creative. The reality is this part is really a
>> 'header' describing the data in the body, but yes - it should be named
>> to match a "Pre Extended Display List Body"
>>
>>   s/vsp1_dl_ext_cmd_header/vsp1_pre_ext_dl_body/
> 
> Sounds good to me.
> 
>> This will then leave "struct vsp1_dl_ext_cmd" which represents the
>> object instance within 

[Bug 107262] [drm] BIOS signature incorrect 0 0 with Ryzen 3 2200g

[bugzilla-daemon]

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

Alex Deucher  changed:

   What|Removed |Added

 Resolution|--- |NOTABUG
 Status|NEW |RESOLVED

--- Comment #2 from Alex Deucher  ---
The driver tries multiple methods to fetch the vbios depending on the platform.
 One of the methods fails and this message is the result.  The driver is
ultimately able to find a proper vbios, otherwise the driver would fail to
load.

-- 
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 v4 10/11] media: vsp1: Support Interlaced display pipelines

[Kieran Bingham]

Hi Laurent,



>>> +static void vsp1_rpf_configure_autofld(struct vsp1_rwpf *rpf,
>>> +  struct vsp1_dl_ext_cmd *cmd)
>>> +{
>>> +   const struct v4l2_pix_format_mplane *format = >format;
>>> +   struct vsp1_extcmd_auto_fld_body *auto_fld = cmd->data;
>>> +   u32 offset_y, offset_c;
>>> +
>>> +   /* Re-index our auto_fld to match the current RPF */
>>
>> s/RPF/RPF./
> 
> Fixed.
> 
>>
>>> +   auto_fld = _fld[rpf->entity.index];
>>> +
>>> +   auto_fld->top_y0 = rpf->mem.addr[0];
>>> +   auto_fld->top_c0 = rpf->mem.addr[1];
>>> +   auto_fld->top_c1 = rpf->mem.addr[2];
>>> +
>>> +   offset_y = format->plane_fmt[0].bytesperline;
>>> +   offset_c = format->plane_fmt[1].bytesperline;
>>> +
>>> +   auto_fld->bottom_y0 = rpf->mem.addr[0] + offset_y;
>>> +   auto_fld->bottom_c0 = rpf->mem.addr[1] + offset_c;
>>> +   auto_fld->bottom_c1 = rpf->mem.addr[2] + offset_c;
>>> +
>>> +   cmd->flags |= VSP1_DL_EXT_AUTOFLD_INT;
>>> +   cmd->flags |= BIT(16 + rpf->entity.index);
>>
>> Do you expect some flags to already be set ? If not, couldn't we assign the 
>> value to the field instead of OR'ing it ?

> No, I think you are correct. Moved to a single expression setting the
> cmd->flags in one line.

Ahem no - of course these flags have to be OR-ed in. Because it
potentially updates a single command object for multiple RPFs.

The flags get reset to 0 when the command object is discarded in
vsp1_dl_ext_cmd_put()


> 
>>
>>> +}



--
Kieran

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

Re: More drm-panel help volunteers?

[Philipp Zabel]

Hi Daniel,

On Fri, 2018-07-13 at 10:38 +0200, Daniel Vetter wrote:
> On Fri, Jul 13, 2018 at 10:09 AM, Lucas Stach  wrote:
> > Hi Daniel,
> > 
> > Am Mittwoch, den 11.07.2018, 20:07 +0200 schrieb Daniel Vetter:
> > > Hi all,
> > > 
> > > I chatted a bit with Thierry about drm-panel, and state of things
> > > still seems to be that when Thierry's on vacations/leave/busy patches
> > > don't move. So I looked at people with a few patches who could be
> > > volunteered

Heh :)

> > >  to sometimes help out with review:
> > > 
> > > 31  Thierry Reding
> > > 16  Philipp Zabel

Note that all these are just additions to the panel-simple driver ...

> > > 11  Lucas Stach
> > >  9  Ajay Kumar
> > >  9  Alexandre Courbot
> > >  8  Philippe CORNU
> > >  6  Boris Brezillon
> > >  6  Maxime Ripard
> > >  6  Stefan Agner
> > >  5  Arnd Bergmann
> > >  5  Dave Airlie
> > >  5  Yakir Yang
> > > 
> > > There's fairly little patch activity going on in drm/panel, main goal
> > > would be just to reduce the occasional long waits (recently a
> > > contributor pinged 3 times or so every few weeks for feedback).
> > > 
> > > Philipp Z and Lucas, either of you interested in drm-misc commit
> > > rights and occasionally scanning for drm-panel patches?
[...]
> > I don't feel fully qualified to look at the more complex panel stuff,
> > but at least making sure that simple panel patches don't get stuck
> > should be okay.

... so this also applies to me.

> Yes I think just taking care of the simpler panel patches (after they
> have the DT ack from Rob Herring or some other DT expert) would be
> great. And if Philipp wants to help to too when he's back we can fix
> him up with commit rights. I'll be away for the next 3 weeks, but
> Sean/Maarten/Gustavo should be around ...

I'd be fine with serving as a fallback.

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

Re: [PATCH] drm/panel: simple: Support simple VGA panels

[Rob Herring]

On Tue, Jul 17, 2018 at 1:47 AM Linus Walleij  wrote:
>
> On Tue, Jul 17, 2018 at 12:50 AM Rob Herring  wrote:
> > On Mon, Jul 16, 2018 at 3:23 AM Linus Walleij  
> > wrote:
>
> > > +Video Graphics Array
> >
> > VGA is more a controller interface than a panel...
>
> I don't know what else to call it though, other than formulating someting
> bureaucratic like
> "Video Graphics Array Display Resolutions"
>
> Or should I use the abbreviation:
> "VGA Display Resolutions" rather?
>
> The Wikipedia article "display resolution"
> https://en.wikipedia.org/wiki/Display_resolution
> just calls these three resolutions "VGA", "SVGA " and "XGA".
>
> > > +static const struct drm_display_mode video_graphics_array_modes[] = {
> > > +   {
> > > +   /*
> > > +* This is the most standardized "vanilla" VGA mode there 
> > > is:
> > > +* 640x480 @ 60 Hz
> >
> > Don't we have standard VESA timings already defined as well as timings
> > that can be calculated based on resolution and refresh rate (called
> > CVT IIRC). Why duplicate here?
>
> They are inside drivers/gpu/drm/drm_edid.c
> all in static const arrays and enumerated by the index in the
> DMT spec taken out of XFree86. (drm_dmt_modes[])
>
> I don't know. It is quite encapsulated into that EDID driver and
> doesn't seem very reusable. To pick out a few from inside EDID
> seems pretty daunting. I'd like to hear what the DRM folks think
> about that.
>
> > Why don't you just define a 'vga-connector' node
>
> Because this is not a VGA connector, it is just the VGA resolution.
> In other circumstances I do use it.

It's not a panel either. A connector is something with variable
resolution. A panel is fixed resolution. Which do you want?

> I think you most often have to use that connector with the dumb
> VGA DAC bridge though, as the VGA connector is analog and
> what comes out of a display controller is digital. So it needs to
> make some kind of sense here.
>
> In a way (as we discussed before) the whole panel-simple.c thing
> is a bit bogus, as it is probably in most cases hiding a bridge or
> a dumb DAC or at least a VGA connector or similar that should've
> been properly modeled, but in this case I am more certain that it is
> actually the right choice.
>
> I guess I could try to use the dumb VGA bridge and the VGA
> connector, and it indeed falls back to VGA resolutions if no DDC
> channel is available but if I go in and say there is a DAC bridge
> and VGA connector I am essentially lying.
>
> > and IIRC, you can
> > just force the standard modes from userspace with DRM.
>
> I guess you mean the kernel command line arguments, lest
> there will be no boot console.

Yes, the kernel command line is another way. But if you aren't using
fbcon, then userspace is the way.

> Apart from being a user experience horror story, that requires
> a VGA connector bridge, as per above. (It's the EDID code that
> does that command line parsing.) And that requires lying about
> having a VGA connector bridge.

Because you have to set the resolution on the kernel command line? I'm
open to having a default resolution for the connector in DT.

> But I can surely lie if everyone thinks that is the best idea :D
>
> > Maybe you need
> > some flag to force a connection in the emulator and perhaps some fake
> > EDID data to set a default mode.
>
> This device tree I'm creating is for ARM RTSM which is a proprietary
> ARM tool.
>
> Sudeep at ARM says it does not emulate any DAC bridge such
> as found in the Versatile Express machine family. It just expects
> to have the right resolution parameters written into the registers
> of the PL111, which in turn of course can only get it from a
> panel or bridge driver of some sort.

So putting *anything* in DT is a lie...

> The way those emulators work (AFAICT) is that they simply monitor
> register writes to the resolutions parameters to scale the emulator
> display window and then they read out the RGB data into that
> window, pixel by pixel, from the indicated memory area.
> It works for them.
>
> In QEMU, we had the same problem. It didn't support proper reporting
> of fake EDID on these platforms either, because of not properly
> modeling the hardware it was emulating, instead relying on the register
> reads as above. Since it is open source I could
> simply fix it:
> https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04651.html
> https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04652.html
> https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04653.html

Having a way to set the default display resolution in QEMU would be
nice. I've hacked around that when using virgl.

> After this, the QEMU for Versatile Express can properly use the
> bridge.
>
> I do try to be gritty and thorough! :D
>
> I don't really know what RTSM is and I can't run it myself. I just
> have to support it in the refactoring to use DRM for the ARM
> Versatile Express machines. I cannot change its source code.
>

Re: [PATCH v2] drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit

[Paul Kocialkowski]

On Tue, 2018-07-17 at 14:25 +0200, Paul Kocialkowski wrote:
> Blending order is set based on the z position of each DRM plane. The
> blending order register is currently cleared at each atomic DRM commit,
> with the intent that each committed plane will set the appropriate
> bits (based on its z-pos) when enabling the plane.
> 
> However, it sometimes happens that a particular plane is left unchanged
> by an atomic commit and thus will not be configured again. In that
> scenario, blending order is cleared and only the bits relevant for the
> planes affected by the commit are set. This leaves the planes that did
> not change without their blending order set in the register, leading
> to that plane not being displayed.
> 
> Instead of clearing the blending order register at every atomic commit,
> this change moves the register's initial clear at bind time and only
> clears the bits for a specific plane when disabling it or changing its
> zpos.
> 
> This way, planes that are left untouched by a DRM atomic commit are
> no longer disabled.

This patch was rebased to apply on top of DRM misc. V1 had been based on
the first revision of the DE2 z-pos support patch, while a subsequent
revision of the patch made it to the kernel tree.

Cheers!

Paul

> Signed-off-by: Paul Kocialkowski 
> ---
>  drivers/gpu/drm/sun4i/sun8i_mixer.c| 15 +++
>  drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 24 
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 24 
>  3 files changed, 43 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
> b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> index 8e81c24d736e..12cb7183ce51 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
> @@ -260,17 +260,6 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 
> format)
>   return NULL;
>  }
>  
> -static void sun8i_mixer_atomic_begin(struct sunxi_engine *engine,
> -  struct drm_crtc_state *old_state)
> -{
> - /*
> -  * Disable all pipes at the beginning. They will be enabled
> -  * again if needed in plane update callback.
> -  */
> - regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_PIPE_CTL,
> -SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
> -}
> -
>  static void sun8i_mixer_commit(struct sunxi_engine *engine)
>  {
>   DRM_DEBUG_DRIVER("Committing changes\n");
> @@ -322,7 +311,6 @@ static struct drm_plane **sun8i_layers_init(struct 
> drm_device *drm,
>  }
>  
>  static const struct sunxi_engine_ops sun8i_engine_ops = {
> - .atomic_begin   = sun8i_mixer_atomic_begin,
>   .commit = sun8i_mixer_commit,
>   .layers_init= sun8i_layers_init,
>  };
> @@ -449,6 +437,9 @@ static int sun8i_mixer_bind(struct device *dev, struct 
> device *master,
>   regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_MODE(i),
>SUN8I_MIXER_BLEND_MODE_DEF);
>  
> + regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL,
> +SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
> +
>   return 0;
>  
>  err_disable_bus_clk:
> diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
> b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> index 518e1921f47e..28c15c6ef1ef 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
> @@ -27,7 +27,8 @@
>  #include "sun8i_ui_scaler.h"
>  
>  static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
> -   int overlay, bool enable, unsigned int zpos)
> +   int overlay, bool enable, unsigned int zpos,
> +   unsigned int old_zpos)
>  {
>   u32 val;
>  
> @@ -43,6 +44,18 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer 
> *mixer, int channel,
>  SUN8I_MIXER_CHAN_UI_LAYER_ATTR(channel, overlay),
>  SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val);
>  
> + if (!enable || zpos != old_zpos) {
> + regmap_update_bits(mixer->engine.regs,
> +SUN8I_MIXER_BLEND_PIPE_CTL,
> +SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
> +0);
> +
> + regmap_update_bits(mixer->engine.regs,
> +SUN8I_MIXER_BLEND_ROUTE,
> +SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
> +0);
> + }
> +
>   if (enable) {
>   val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
>  
> @@ -242,9 +255,11 @@ static void sun8i_ui_layer_atomic_disable(struct 
> drm_plane *plane,
> struct drm_plane_state *old_state)
>  {
>   struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
> + unsigned int old_zpos = old_state->normalized_zpos;
>   struct sun8i_mixer 

Re: [PATCH v4 11/11] drm: rcar-du: Support interlaced video output through vsp1

[Laurent Pinchart]

Hi Kieran,

On Monday, 16 July 2018 20:20:30 EEST Kieran Bingham wrote:
> On 24/05/18 12:50, Laurent Pinchart wrote:
> > On Thursday, 3 May 2018 16:36:22 EEST Kieran Bingham wrote:
> >> Use the newly exposed VSP1 interface to enable interlaced frame support
> >> through the VSP1 lif pipelines.
> > 
> > s/lif/LIF/
> 
> Fixed.
> 
> >> Signed-off-by: Kieran Bingham 
> >> ---
> >> 
> >>  drivers/gpu/drm/rcar-du/rcar_du_crtc.c | 1 +
> >>  drivers/gpu/drm/rcar-du/rcar_du_vsp.c  | 3 +++
> >>  2 files changed, 4 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> >> b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c index d71d709fe3d9..206532959ec9
> >> 100644
> >> --- a/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_crtc.c
> >> @@ -289,6 +289,7 @@ static void rcar_du_crtc_set_display_timing(struct
> >> rcar_du_crtc *rcrtc)
> >>/* Signal polarities */
> >>value = ((mode->flags & DRM_MODE_FLAG_PVSYNC) ? DSMR_VSL : 0)
> >>  | ((mode->flags & DRM_MODE_FLAG_PHSYNC) ? DSMR_HSL : 0)
> >> +| ((mode->flags & DRM_MODE_FLAG_INTERLACE) ? DSMR_ODEV : 0)
> > 
> > How will this affect Gen2 ?.
> 
> The bit is documented identically for Gen2. Potentially / Probably it
> 'might' reverse the fields... I'm not certain yet. I don't have access
> to a Gen2 platform to test.
> 
> I'll see if this change can be dropped, but I think it is playing a role
> in ensuring that the field detection occurs in VSP1 through the
> VI6_STATUS_FLD_STD() field. (see vsp1_dlm_irq_frame_end())
> 
> > Could you document what this change does in the
> > commit message ?
> 
> This sets the position in the buffer of the ODDF. With this set, the ODD
> field is located in the second half (BOTTOM) of the same frame of the
> interlace display.
> 
> Otherwise, it's in the first half (TOP)
> 
> I faced some issues as to the ordering when testing, so I suspect this
> might actually be related to that. (re VI6_STATUS_FLD_STD in
> vsp1_dlm_irq_frame_end()).
> 
> As you mention - this may have a negative effect on the Gen2
> implementation - so it needs considering with that.
> 
> 
> /me to investigate further.

Thank you. I don't object to this change, but I'd like to know what its 
implications are on Gen2. It might even fix a bug :-) Let me know if you'd 
like me to run tests on a Lager board.

> >>  | DSMR_DIPM_DISP | DSMR_CSPM;
> >>
> >>rcar_du_crtc_write(rcrtc, DSMR, value);
> >> 
> >> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> >> b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c index af7822a66dee..c7b37232ee91
> >> 100644
> >> --- a/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> >> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vsp.c
> >> @@ -186,6 +186,9 @@ static void rcar_du_vsp_plane_setup(struct
> >> rcar_du_vsp_plane *plane)
> >>};
> >>unsigned int i;
> >> 
> >> +  cfg.interlaced = !!(plane->plane.state->crtc->mode.flags
> >> +  & DRM_MODE_FLAG_INTERLACE);
> >> +
> >>cfg.src.left = state->state.src.x1 >> 16;
> >>cfg.src.top = state->state.src.y1 >> 16;
> >>cfg.src.width = drm_rect_width(>state.src) >> 16;

-- 
Regards,

Laurent Pinchart



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

[PATCH V2] drm: handle error values properly

[Nicholas Mc Guire]

drm_legacy_ctxbitmap_next() returns idr_alloc() which can return
-ENOMEM, -EINVAL or -ENOSPC none of which are -1. since drm_context_t
is an unsigned int an intermediate variable is used to handle the
error cases, and then cast to drm_context_t after ensuring that the
value is >= 0. The explicit cast is to mark the type conversion as
intentional.

Signed-off-by: Nicholas Mc Guire 
Reported-by: kbuild test robot 
Fixes: d530b5f1ca0b ("drm: re-enable error handling")
Fixes: 62968144e673 ("drm: convert drm context code to use Linux idr")
---

kbuild test robot reported:

tree:   git://anongit.freedesktop.org/drm/drm-misc for-linux-next-fixes
head:   d530b5f1ca0bb66958a2b714bebe40a1248b9c15
commit: d530b5f1ca0bb66958a2b714bebe40a1248b9c15 [2/2] drm: re-enable error
+handling

smatch warnings:
drivers/gpu/drm/drm_context.c:375 drm_legacy_addctx() warn: unsigned
+'ctx->handle' is never less than zero.


V2: The proposed fix in d530b5f1ca0b ("drm: re-enable error handling")
actually was ineffective as the negative return value check was 
against a unsigned int and thus always false as reported by
kbuild test robot . The below patch removes that
warning and fixes the original problem of missed error handling.

drm_context_t is actually just used in a few placed so the type could be
changed but it is also exported via tools/include/uapi/drm/drm.h so
changing the typedef of drm_context_t could break applications and thus
this is not an option.

Patch was compile tested with: x86_64_defconfig

Patch is against 4.18-rc4 (localversion-next is next-20180717)

 drivers/gpu/drm/drm_context.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_context.c b/drivers/gpu/drm/drm_context.c
index 3c4000f..78f32a3 100644
--- a/drivers/gpu/drm/drm_context.c
+++ b/drivers/gpu/drm/drm_context.c
@@ -361,22 +361,26 @@ int drm_legacy_addctx(struct drm_device *dev, void *data,
 {
struct drm_ctx_list *ctx_entry;
struct drm_ctx *ctx = data;
+   int ret;
 
if (!drm_core_check_feature(dev, DRIVER_KMS_LEGACY_CONTEXT) &&
!drm_core_check_feature(dev, DRIVER_LEGACY))
return -EINVAL;
 
ctx->handle = drm_legacy_ctxbitmap_next(dev);
-   if (ctx->handle == DRM_KERNEL_CONTEXT) {
+   ret = drm_legacy_ctxbitmap_next(dev);
+   if (ret == DRM_KERNEL_CONTEXT) {
/* Skip kernel's context and get a new one. */
-   ctx->handle = drm_legacy_ctxbitmap_next(dev);
+   ret = drm_legacy_ctxbitmap_next(dev);
}
-   DRM_DEBUG("%d\n", ctx->handle);
-   if (ctx->handle < 0) {
+   DRM_DEBUG("ctxbitmap is error code %d\n", ret);
+   if (ret < 0) {
DRM_DEBUG("Not enough free contexts.\n");
/* Should this return -EBUSY instead? */
return -ENOMEM;
}
+   /* valid context is >= 0 */
+   ctx->handle = (drm_context_t)ret;
 
ctx_entry = kmalloc(sizeof(*ctx_entry), GFP_KERNEL);
if (!ctx_entry) {
-- 
2.1.4

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

Re: [PATCH] drm/imx: Remove unused field imx_drm_device.pipes

[Philipp Zabel]

On Tue, 2018-07-17 at 15:11 +0300, Leonard Crestez wrote:
> This has been unused since commit 44b460cfe554 ("drm: imx: remove struct
> imx_drm_crtc and imx_drm_crtc_helper_funcs")
> 
> Signed-off-by: Leonard Crestez 

Applied to imx-drm/next, thank you.

> ---
>  drivers/gpu/drm/imx/imx-drm-core.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> Looking at the imx_drm_device struct it is only used to save the
> drm_atomic_helper_suspend state. It seems like this could be replaced
> with drm_mode_config_helper_suspend/resume and the entire struct
> removed.
> 
> The only difference between imx_drm_suspend/resume and
> drm_mode_config_helper_suspend/resume is that the latter also suspends
> the fb_helper. This would be an improvement, right?

This looks about right to me. We currently don't call the
fbdev suspend/resume notifiers at all. The correct call
would be drm_fbdev_cma_set_suspend_unlocked instead of
drm_fb_helper_set_suspend_unlocked, though.

Since drm_fb_cma_fbdev_init registers its fb_helper with the
drm_device via drm_fb_helper_init, both (currently) do exactly
the same thing.

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

Re: [PATCH v4 10/11] media: vsp1: Support Interlaced display pipelines

[Laurent Pinchart]

Hi Kieran,

On Monday, 16 July 2018 21:21:00 EEST Kieran Bingham wrote:
> On 24/05/18 13:51, Laurent Pinchart wrote:
> > On Thursday, 3 May 2018 16:36:21 EEST Kieran Bingham wrote:
> >> Calculate the top and bottom fields for the interlaced frames and
> >> utilise the extended display list command feature to implement the
> >> auto-field operations. This allows the DU to update the VSP2 registers
> >> dynamically based upon the currently processing field.
> >> 
> >> Signed-off-by: Kieran Bingham 
> >> 
> >> ---
> >> 
> >> v3:
> >>  - Pass DL through partition calls to allow autocmd's to be retrieved
> >>  - Document interlaced field in struct vsp1_du_atomic_config
> >> 
> >> v2:
> >>  - fix erroneous BIT value which enabled interlaced
> >>  - fix field handling at frame_end interrupt
> >> 
> >> ---
> >> 
> >>  drivers/media/platform/vsp1/vsp1_dl.c   | 10 -
> >>  drivers/media/platform/vsp1/vsp1_drm.c  | 11 -
> >>  drivers/media/platform/vsp1/vsp1_regs.h |  1 +-
> >>  drivers/media/platform/vsp1/vsp1_rpf.c  | 71 --
> >>  drivers/media/platform/vsp1/vsp1_rwpf.h |  1 +-
> >>  include/media/vsp1.h|  2 +-
> >>  6 files changed, 93 insertions(+), 3 deletions(-)

[snip]

> >> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> >> b/drivers/media/platform/vsp1/vsp1_drm.c index 2c3db8b8adce..cc29c9d96bb7
> >> 100644
> >> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> >> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> >> @@ -811,6 +811,17 @@ int vsp1_du_atomic_update(struct device *dev,
> >> unsigned
> >> int pipe_index, return -EINVAL;
> >> 
> >>}
> >> 
> >> +  if (!(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)) && cfg->interlaced) {
> > 
> > Nitpicking, writing the condition as
> > 
> > if (cfg->interlaced && !(vsp1_feature(vsp1, VSP1_HAS_EXT_DL)))
> 
> Done.
> 
> > would match the comment better. You can also drop the parentheses around
> > the vsp1_feature() call.
> > 
> >> +  /*
> >> +   * Interlaced support requires extended display lists to
> >> +   * provide the auto-fld feature with the DU.
> >> +   */
> >> +  dev_dbg(vsp1->dev, "Interlaced unsupported on this output\n");
> > 
> > Could we catch this in the DU driver to fail atomic test ?
> 
> Ugh - I thought moving the configuration to vsp1_du_setup_lif() would
> give us this, but that return value is not checked in the DU.
> 
> How can we interogate the VSP1 to ask it if it supports interlaced from
> rcar_du_vsp_plane_atomic_check()?
> 
> 
> Some dummy call to vsp1_du_setup_lif() to check the return value ? Or
> should we implement a hook to call through to perform checks in the VSP1
> DRM API?

Would it be possible to just infer that from the DU compatible string, without 
querying the VSP driver ? Of course that's a bit of a layering violation, but 
as we know what type of VSP instance is present in each SoC, such a small hack 
wouldn't hurt in my opinion. If the need arises later we can introduce an API 
to query the information from the VSP driver.

> >> +  return -EINVAL;
> >> +  }
> >> +
> >> +  rpf->interlaced = cfg->interlaced;
> >> +
> >>rpf->fmtinfo = fmtinfo;
> >>rpf->format.num_planes = fmtinfo->planes;
> >>rpf->format.plane_fmt[0].bytesperline = cfg->pitch;

[snip]

-- 
Regards,

Laurent Pinchart



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

[Bug 107263] Decrease start-up time of amdgpu_init from 390 ms to under 100 ms

[bugzilla-daemon]

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

Bug ID: 107263
   Summary: Decrease start-up time of amdgpu_init from 390 ms to
under 100 ms
   Product: DRI
   Version: DRI git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: pmenzel+bugs.freedesk...@molgen.mpg.de

On a MSI B350M MORTAR with Linux 4.18-rc5+ and Debian Sid/unstable the time
noted by initcall_debug for the module amdgpu is 390 ms. The goal is to start
the graphical login manager as quickly as possible and having the graphics
device initialized quickly is therefore required.

Here is a commented excerpt:

[   15.674171] calling  amdgpu_init+0x0/0x86 [amdgpu] @ 375
[   15.674185] [drm] amdgpu kernel modesetting enabled.
[   15.676261] calling  hwrng_modinit+0x0/0x1000 [rng_core] @ 374
[   15.676858] initcall hwrng_modinit+0x0/0x1000 [rng_core] returned 0 after
578 usecs
[   15.680079] calling  sp_mod_init+0x0/0x1000 [ccp] @ 374
[   15.680103] initcall sp_mod_init+0x0/0x1000 [ccp] returned 0 after 19 usecs
[   15.681202] calling  kfd_module_init+0x0/0x1000 [amdkfd] @ 403
[   15.681461] Parsing CRAT table with 1 nodes
[   15.681465] Creating topology SYSFS entries
[   15.681510] Topology: Add APU node [0x0:0x0]
[   15.681510] Finished initializing topology
[   15.681531] kfd kfd: Initialized module
[   15.681538] initcall kfd_module_init+0x0/0x1000 [amdkfd] returned 0 after
321 usecs
[   15.683076] calling  svm_init+0x0/0xb8e [kvm_amd] @ 373
[   15.683078] kvm: disabled by bios
[   15.683096] initcall svm_init+0x0/0xb8e [kvm_amd] returned -95 after 16
usecs
[   15.685698] calling  mce_amd_init+0x0/0x1000 [edac_mce_amd] @ 374
[   15.685700] MCE: In-kernel MCE decoding enabled.
[   15.685703] initcall mce_amd_init+0x0/0x1000 [edac_mce_amd] returned 0 after
0 usecs
[   15.686596] calling  amd64_edac_init+0x0/0x2000 [amd64_edac_mod] @ 374
[   15.686606] EDAC amd64: Node 0: DRAM ECC disabled.
[   15.686607] EDAC amd64: ECC disabled in the BIOS or no ECC capability,
module will not load.
Either enable ECC checking or force module loading by setting
'ecc_enable_override'.
(Note that use of the override may cause unknown side effects.)
[   15.686610] initcall amd64_edac_init+0x0/0x2000 [amd64_edac_mod] returned
-19 after 11 usecs
[   15.688214] checking generic (e000 30) vs hw (e000 1000)
[   15.688216] fb: switching to amdgpudrmfb from EFI VGA
[   15.688231] Console: switching to colour dummy device 80x25
[   15.688894] [drm] initializing kernel modesetting (RAVEN 0x1002:0x15DD
0x1002:0x15DD 0xC8).
[   15.688939] [drm] register mmio base: 0xFE60
[   15.688940] [drm] register mmio size: 524288
[   15.688953] [drm] probing gen 2 caps for device 1022:15db = 700d03/e
[   15.688954] [drm] probing mlw for device 1022:15db = 700d03
[   15.688956] [drm] add ip block number 0 
[   15.688957] [drm] add ip block number 1 
[   15.688957] [drm] add ip block number 2 
[   15.688958] [drm] add ip block number 3 
[   15.688959] [drm] add ip block number 4 
[   15.688960] [drm] add ip block number 5 
[   15.688960] [drm] add ip block number 6 
[   15.688961] [drm] add ip block number 7 
[   15.688962] [drm] add ip block number 8 
[   15.690070] calling  init_fat_fs+0x0/0xfcb [fat] @ 405
[   15.690235] initcall init_fat_fs+0x0/0xfcb [fat] returned 0 after 157 usecs
[   15.690876] calling  init_vfat_fs+0x0/0x1000 [vfat] @ 405
[   15.690881] initcall init_vfat_fs+0x0/0x1000 [vfat] returned 0 after 2 usecs
[   15.692164] kfd kfd: DID 15dd is missing in supported_devices
[   15.692166] kfd kfd: kgd2kfd_probe failed
[   15.692176] [drm] VCN decode is enabled in VM mode
[   15.692177] [drm] VCN encode is enabled in VM mode
[   15.693015] calling  init_nls_cp437+0x0/0x1000 [nls_cp437] @ 406
[   15.693017] initcall init_nls_cp437+0x0/0x1000 [nls_cp437] returned 0 after
0 usecs

20 ms spent here.

[   15.713799] [drm] BIOS signature incorrect 0 0
[   15.713832] ATOM BIOS: 113-RAVEN-106
[   15.713864] [drm] vm size is 262144 GB, 4 levels, block size is 9-bit,
fragment size is 9-bit
[   15.713878] amdgpu :38:00.0: VRAM: 1024M 0x00F4 -
0x00F43FFF (1024M used)
[   15.713879] amdgpu :38:00.0: GTT: 1024M 0x00F5 -
0x00F53FFF
[   15.713882] [drm] Detected VRAM RAM=1024M, BAR=1024M
[   15.713883] [drm] RAM width 128bits DDR4
[   15.715449] r8169 :18:00.0 enp24s0: renamed from eth0
[   15.715481] [TTM] Zone  kernel: Available graphics memory: 7703284 kiB
[   15.715482] [TTM] Zone   dma32: Available graphics memory: 2097152 kiB
[   15.715482] [TTM] Initializing pool allocator
[   15.715485] [TTM] Initializing DMA pool allocator
[   15.715543] [drm] amdgpu: 1024M of VRAM memory ready
[   15.715544] [drm] amdgpu: 3072M of GTT memory ready.
[   

Re: [PATCH 1/2] drm/panel: Add support for Armadeus ST0700 Adapt

[Fabio Estevam]

Hi Sébastien,

On Tue, Jul 17, 2018 at 4:23 AM, Sébastien Szymanski
 wrote:
> This patch adds support for the Armadeus ST0700 Adapt. It comes with a
> Santek ST0700I5Y-RBSLW 7.0" WVGA (800x480) TFT and an adapter board so
> that it can be connected on the TFT header of Armadeus Dev boards.
>
> Signed-off-by: Sébastien Szymanski 
> ---
>  .../display/panel/armadeus,st0700-adapt.txt|  9 +++
>  drivers/gpu/drm/panel/panel-simple.c   | 29 
> ++
>  2 files changed, 38 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/armadeus,st0700-adapt.txt
>
> diff --git 
> a/Documentation/devicetree/bindings/display/panel/armadeus,st0700-adapt.txt 
> b/Documentation/devicetree/bindings/display/panel/armadeus,st0700-adapt.txt
> new file mode 100644
> index ..a30d63db3c8f
> --- /dev/null
> +++ 
> b/Documentation/devicetree/bindings/display/panel/armadeus,st0700-adapt.txt
> @@ -0,0 +1,9 @@
> +Armadeus ST0700 Adapt. A Santek ST0700I5Y-RBSLW 7.0" WVGA (800x480) TFT with
> +an adapter board.
> +
> +Required properties:
> +- compatible: "armadeus,st0700-adapt"

Shouldn't this be named "santek,st0700i5y" instead?

Santek is the vendor of the panel and st0700i5y is the model.

Then you could add a "santek" entry in
Documentation/devicetree/bindings/vendor-prefixes.txt.

Regards,

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

[PATCH v2] drm/sun4i: sun8i: Avoid clearing blending order at each atomic commit

[Paul Kocialkowski]

Blending order is set based on the z position of each DRM plane. The
blending order register is currently cleared at each atomic DRM commit,
with the intent that each committed plane will set the appropriate
bits (based on its z-pos) when enabling the plane.

However, it sometimes happens that a particular plane is left unchanged
by an atomic commit and thus will not be configured again. In that
scenario, blending order is cleared and only the bits relevant for the
planes affected by the commit are set. This leaves the planes that did
not change without their blending order set in the register, leading
to that plane not being displayed.

Instead of clearing the blending order register at every atomic commit,
this change moves the register's initial clear at bind time and only
clears the bits for a specific plane when disabling it or changing its
zpos.

This way, planes that are left untouched by a DRM atomic commit are
no longer disabled.

Signed-off-by: Paul Kocialkowski 
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c| 15 +++
 drivers/gpu/drm/sun4i/sun8i_ui_layer.c | 24 
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 24 
 3 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c 
b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 8e81c24d736e..12cb7183ce51 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -260,17 +260,6 @@ const struct de2_fmt_info *sun8i_mixer_format_info(u32 
format)
return NULL;
 }
 
-static void sun8i_mixer_atomic_begin(struct sunxi_engine *engine,
-struct drm_crtc_state *old_state)
-{
-   /*
-* Disable all pipes at the beginning. They will be enabled
-* again if needed in plane update callback.
-*/
-   regmap_update_bits(engine->regs, SUN8I_MIXER_BLEND_PIPE_CTL,
-  SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
-}
-
 static void sun8i_mixer_commit(struct sunxi_engine *engine)
 {
DRM_DEBUG_DRIVER("Committing changes\n");
@@ -322,7 +311,6 @@ static struct drm_plane **sun8i_layers_init(struct 
drm_device *drm,
 }
 
 static const struct sunxi_engine_ops sun8i_engine_ops = {
-   .atomic_begin   = sun8i_mixer_atomic_begin,
.commit = sun8i_mixer_commit,
.layers_init= sun8i_layers_init,
 };
@@ -449,6 +437,9 @@ static int sun8i_mixer_bind(struct device *dev, struct 
device *master,
regmap_write(mixer->engine.regs, SUN8I_MIXER_BLEND_MODE(i),
 SUN8I_MIXER_BLEND_MODE_DEF);
 
+   regmap_update_bits(mixer->engine.regs, SUN8I_MIXER_BLEND_PIPE_CTL,
+  SUN8I_MIXER_BLEND_PIPE_CTL_EN_MSK, 0);
+
return 0;
 
 err_disable_bus_clk:
diff --git a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c 
b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
index 518e1921f47e..28c15c6ef1ef 100644
--- a/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_ui_layer.c
@@ -27,7 +27,8 @@
 #include "sun8i_ui_scaler.h"
 
 static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, int channel,
- int overlay, bool enable, unsigned int zpos)
+ int overlay, bool enable, unsigned int zpos,
+ unsigned int old_zpos)
 {
u32 val;
 
@@ -43,6 +44,18 @@ static void sun8i_ui_layer_enable(struct sun8i_mixer *mixer, 
int channel,
   SUN8I_MIXER_CHAN_UI_LAYER_ATTR(channel, overlay),
   SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val);
 
+   if (!enable || zpos != old_zpos) {
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_BLEND_PIPE_CTL,
+  SUN8I_MIXER_BLEND_PIPE_CTL_EN(old_zpos),
+  0);
+
+   regmap_update_bits(mixer->engine.regs,
+  SUN8I_MIXER_BLEND_ROUTE,
+  SUN8I_MIXER_BLEND_ROUTE_PIPE_MSK(old_zpos),
+  0);
+   }
+
if (enable) {
val = SUN8I_MIXER_BLEND_PIPE_CTL_EN(zpos);
 
@@ -242,9 +255,11 @@ static void sun8i_ui_layer_atomic_disable(struct drm_plane 
*plane,
  struct drm_plane_state *old_state)
 {
struct sun8i_ui_layer *layer = plane_to_sun8i_ui_layer(plane);
+   unsigned int old_zpos = old_state->normalized_zpos;
struct sun8i_mixer *mixer = layer->mixer;
 
-   sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0);
+   sun8i_ui_layer_enable(mixer, layer->channel, layer->overlay, false, 0,
+ old_zpos);
 }
 
 static void sun8i_ui_layer_atomic_update(struct drm_plane *plane,
@@ -252,11 +267,12 @@ static void sun8i_ui_layer_atomic_update(struct drm_plane 
*plane,
 {
struct 

Re: [PATCH 2/2] drm/sun4i: sun4i: Introduce a quirk for lowest plane alpha support

[Maxime Ripard]

On Tue, Jul 17, 2018 at 10:52:30AM +0200, Paul Kocialkowski wrote:
> Not all sunxi platforms with the first version of the Display Engine
> support an alpha component on the plane with the lowest z position
> (as in: lowest z-pos), that gets blended with the background color.
> 
> In particular, the A13 is known to have this limitation. However, it was
> recently discovered that the A20 and A33 are capable of having alpha on
> their lowest plane.
> 
> Thus, this introduces a specific quirk to indicate such support,
> per-platform. Since this was not tested on sun4i and sun6i platforms, a
> conservative approach is kept and this feature is not supported.
> 
> Signed-off-by: Paul Kocialkowski 
> ---
>  drivers/gpu/drm/sun4i/sun4i_backend.c | 10 --
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c 
> b/drivers/gpu/drm/sun4i/sun4i_backend.c
> index a3cc398d4d80..cdc4a8a91ea2 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_backend.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c
> @@ -35,6 +35,8 @@
>  struct sun4i_backend_quirks {
>   /* backend <-> TCON muxing selection done in backend */
>   bool needs_output_muxing;
> + /* alpha at the lowest z position is not always supported */
> + bool supports_lowest_plane_alpha;
>  };
>  
>  static void sun4i_backend_apply_color_correction(struct sunxi_engine *engine)
> @@ -484,6 +486,7 @@ static void sun4i_backend_atomic_begin(struct 
> sunxi_engine *engine,
>  static int sun4i_backend_atomic_check(struct sunxi_engine *engine,
> struct drm_crtc_state *crtc_state)
>  {
> + struct sun4i_backend *backend = engine_to_sun4i_backend(engine);
>   struct drm_plane_state *plane_states[SUN4I_BACKEND_NUM_LAYERS] = { 0 };

Your new variable should be here.

>   struct drm_atomic_state *state = crtc_state->state;
>   struct drm_device *drm = state->dev;
> @@ -584,8 +587,9 @@ static int sun4i_backend_atomic_check(struct sunxi_engine 
> *engine,
>   }
>  
>   /* We can't have an alpha plane at the lowest position */
> - if (plane_states[0]->fb->format->has_alpha ||
> - (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE))
> + if ((plane_states[0]->fb->format->has_alpha ||
> + (plane_states[0]->alpha != DRM_BLEND_ALPHA_OPAQUE)) &&
> + !backend->quirks->supports_lowest_plane_alpha)
>   return -EINVAL;

This only partially does the job. This only allows to have an alpha
plane at the lowest position, but the fact that the alpha works at the
lowest position also means you can have two alpha planes now, and you
didn't change that check.

The pipe allocation algorithm would also need to be checked.

Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


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

Re: [Nouveau] [PATCH 4/5] drm/nouveau: Grab RPM ref when i2c bus is in use

[Karol Herbst]

On Tue, Jul 17, 2018 at 1:54 PM, Ben Skeggs  wrote:
> On Tue, 17 Jul 2018 at 20:18, Karol Herbst  wrote:
>>
>> mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd
>> the Linux glue code to the i2c stuff instead, but this is all done
>> from inside of nvkm. I think we should move it out into
>> drm/nouveau/nouveau_i2c.c and do the handling there.
> Huh?  No, this is completely fine.
>

okay, then the the two patches adding that guard code is reviewed-by me

>>
>> On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul  wrote:
>> > The i2c bus can be both accessed by DRM itself, along with any of it's
>> > devnodes (/sys/class/i2c). So, we need to make sure that all codepaths
>> > using the i2c bus keep the GPU resumed.
>> >
>> > Signed-off-by: Lyude Paul 
>> > Cc: Karol Herbst 
>> > Cc: sta...@vger.kernel.org
>> > ---
>> >  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++-
>> >  1 file changed, 11 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c 
>> > b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
>> > index 807a2b67bd64..1de48c990b80 100644
>> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
>> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
>> > @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus)
>> > BUS_TRACE(bus, "release");
>> > nvkm_i2c_pad_release(pad);
>> > mutex_unlock(>mutex);
>> > +   pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev);
>> >  }
>> >
>> >  int
>> >  nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus)
>> >  {
>> > struct nvkm_i2c_pad *pad = bus->pad;
>> > +   struct device *dev = pad->i2c->subdev.device->dev;
>> > int ret;
>> > +
>> > BUS_TRACE(bus, "acquire");
>> > +
>> > +   ret = pm_runtime_get_sync(dev);
>> > +   if (ret < 0 && ret != -EACCES)
>> > +   return ret;
>> > +
>> > mutex_lock(>mutex);
>> > ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C);
>> > -   if (ret)
>> > +   if (ret) {
>> > mutex_unlock(>mutex);
>> > +   pm_runtime_put_autosuspend(dev);
>> > +   }
>> > return ret;
>> >  }
>> >
>> > --
>> > 2.17.1
>> >
>> > ___
>> > Nouveau mailing list
>> > nouv...@lists.freedesktop.org
>> > https://lists.freedesktop.org/mailman/listinfo/nouveau
>> ___
>> Nouveau mailing list
>> nouv...@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107262] [drm] BIOS signature incorrect 0 0 with Ryzen 3 2200g

[bugzilla-daemon]

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

Paul Menzel  changed:

   What|Removed |Added

 CC||pmenzel+bugs.freedesktop@mo
   ||lgen.mpg.de

--- Comment #1 from Paul Menzel  ---
Created attachment 140666
  --> https://bugs.freedesktop.org/attachment.cgi?id=140666=edit
Extracted firmware

Video BIOS extracted with writing `1` to `/rom`, and then copying that.

-- 
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 107262] [drm] BIOS signature incorrect 0 0 with Ryzen 3 2200g

[bugzilla-daemon]

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

Bug ID: 107262
   Summary: [drm] BIOS signature incorrect 0 0 with Ryzen 3 2200g
   Product: DRI
   Version: DRI git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: pmenzel+bugs.freedesk...@molgen.mpg.de

Created attachment 140665
  --> https://bugs.freedesktop.org/attachment.cgi?id=140665=edit
Linux 4.18-rc5+ messages

On a MSI B350M MORTAR with Linux 4.18-rc5+ and Debian Sid/unstable Linux logs
that the Video BIOS signature is incorrect.

```
$ git log --oneline -1
30b06abfb92b (HEAD -> master, origin/master, origin/HEAD) Merge tag
'pinctrl-v4.18-3' of
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
$ git describe --dirty
v4.18-rc5-36-g30b06abfb92b
$ sudo dmesg
[…]
[   15.713799] [drm] BIOS signature incorrect 0 0
[   15.713832] ATOM BIOS: 113-RAVEN-106
[…]
```

-- 
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/suni4: Replace drm_dev_unref with drm_dev_put

[Maxime Ripard]

On Tue, Jul 17, 2018 at 10:48:14AM +0200, Thomas Zimmermann wrote:
> This patch unifies the naming of DRM functions for reference counting
> of struct drm_device. The resulting code is more aligned with the rest
> of the Linux kernel interfaces.
> 
> Signed-off-by: Thomas Zimmermann 

Applied, thanks!
Maxime

-- 
Maxime Ripard, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com


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

Re: [Nouveau] [PATCH 4/5] drm/nouveau: Grab RPM ref when i2c bus is in use

[Ben Skeggs]

On Tue, 17 Jul 2018 at 20:18, Karol Herbst  wrote:
>
> mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd
> the Linux glue code to the i2c stuff instead, but this is all done
> from inside of nvkm. I think we should move it out into
> drm/nouveau/nouveau_i2c.c and do the handling there.
Huh?  No, this is completely fine.

>
> On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul  wrote:
> > The i2c bus can be both accessed by DRM itself, along with any of it's
> > devnodes (/sys/class/i2c). So, we need to make sure that all codepaths
> > using the i2c bus keep the GPU resumed.
> >
> > Signed-off-by: Lyude Paul 
> > Cc: Karol Herbst 
> > Cc: sta...@vger.kernel.org
> > ---
> >  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c 
> > b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> > index 807a2b67bd64..1de48c990b80 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> > @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus)
> > BUS_TRACE(bus, "release");
> > nvkm_i2c_pad_release(pad);
> > mutex_unlock(>mutex);
> > +   pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev);
> >  }
> >
> >  int
> >  nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus)
> >  {
> > struct nvkm_i2c_pad *pad = bus->pad;
> > +   struct device *dev = pad->i2c->subdev.device->dev;
> > int ret;
> > +
> > BUS_TRACE(bus, "acquire");
> > +
> > +   ret = pm_runtime_get_sync(dev);
> > +   if (ret < 0 && ret != -EACCES)
> > +   return ret;
> > +
> > mutex_lock(>mutex);
> > ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C);
> > -   if (ret)
> > +   if (ret) {
> > mutex_unlock(>mutex);
> > +   pm_runtime_put_autosuspend(dev);
> > +   }
> > return ret;
> >  }
> >
> > --
> > 2.17.1
> >
> > ___
> > Nouveau mailing list
> > nouv...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/nouveau
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 107261] [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 1us * 10 tries - optc1_lock line:628

[bugzilla-daemon]

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

Bug ID: 107261
   Summary: [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT
timeout 1us * 10 tries - optc1_lock line:628
   Product: DRI
   Version: DRI git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: pmenzel+bugs.freedesk...@molgen.mpg.de

Created attachment 140664
  --> https://bugs.freedesktop.org/attachment.cgi?id=140664=edit
Linux 4.18-rc5+ messages

On a MSI B350M MORTAR with Linux 4.18-rc5+ and Debian Sid/unstable with GNOME I
get the error below.

```
$ git log --oneline -1
30b06abfb92b (HEAD -> master, origin/master, origin/HEAD) Merge tag
'pinctrl-v4.18-3' of
git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl
$ git describe --dirty
v4.18-rc5-36-g30b06abfb92b
$ sudo dmesg
[…]
[  171.571430] [drm:generic_reg_wait [amdgpu]] *ERROR* REG_WAIT timeout 1us *
10 tries - optc1_lock line:628
[  171.571485] WARNING: CPU: 2 PID: 917 at
drivers/gpu/drm/amd/amdgpu/../display/dc/dc_helper.c:254
generic_reg_wait+0xe7/0x160 [amdgpu]
[  171.571486] Modules linked in: fuse nls_ascii amdkfd nls_cp437 vfat fat
wmi_bmof ppdev amdgpu edac_mce_amd ccp rng_core chash snd_hda_codec_realtek
efi_pstore gpu_sched kvm ttm irqbypass snd_hda_codec_generic snd_hda_codec_hdmi
crct10dif_pclmul sp5100_tco crc32_pclmul pcspkr snd_hda_intel i2c_piix4
ghash_clmulni_intel drm_kms_helper snd_hda_codec snd_hda_core efivars sg
snd_hwdep k10temp r8169 snd_pcm drm mii snd_timer snd i2c_algo_bit soundcore
parport_pc wmi parport video button acpi_cpufreq efivarfs ip_tables x_tables
autofs4 ext4 crc16 mbcache jbd2 fscrypto dm_crypt dm_mod raid10 raid456
libcrc32c crc32c_generic async_raid6_recov async_memcpy async_pq async_xor xor
async_tx raid6_pq raid1 raid0 multipath linear md_mod sd_mod evdev hid_generic
usbhid hid crc32c_intel ahci libahci xhci_pci aesni_intel
[  171.571521]  aes_x86_64 xhci_hcd crypto_simd cryptd glue_helper libata
usbcore scsi_mod gpio_amdpt gpio_generic
[  171.571527] CPU: 2 PID: 917 Comm: gnome-shell Not tainted 4.18.0-rc5+ #1
[  171.571528] Hardware name: MSI MS-7A37/B350M MORTAR (MS-7A37), BIOS 1.F0
04/27/2018
[  171.571569] RIP: 0010:generic_reg_wait+0xe7/0x160 [amdgpu]
[  171.571569] Code: 44 24 58 8b 54 24 48 89 de 44 89 4c 24 08 48 8b 4c 24 50
48 c7 c7 10 2e b5 c0 e8 f4 e1 ac ff 83 7d 20 01 44 8b 4c 24 08 74 02 <0f> 0b 48
83 c4 10 44 89 c8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 41 0f 
[  171.571585] RSP: 0018:b2f003d1f920 EFLAGS: 00010297
[  171.571587] RAX:  RBX: 0001 RCX:

[  171.571587] RDX:  RSI: 9d185ec96738 RDI:
9d185ec96738
[  171.571588] RBP: 9d1843a49800 R08: 0392 R09:
0001
[  171.571588] R10:  R11: b4fcaf4d R12:
000b
[  171.571589] R13: 504d R14: 0100 R15:
0001
[  171.571590] FS:  7f7488a34ac0() GS:9d185ec8()
knlGS:
[  171.571591] CS:  0010 DS:  ES:  CR0: 80050033
[  171.571592] CR2: 01b1baab4488 CR3: 0003dc648000 CR4:
003406e0
[  171.571592] Call Trace:
[  171.571640]  optc1_lock+0xa0/0xb0 [amdgpu]
[  171.571684]  dcn10_pipe_control_lock.part.28+0x4a/0x70 [amdgpu]
[  171.571728]  dcn10_apply_ctx_for_surface+0xee/0x1210 [amdgpu]
[  171.571731]  ? _cond_resched+0x15/0x30
[  171.571774]  ? hubbub1_verify_allow_pstate_change_high+0xdd/0x180 [amdgpu]
[  171.571817]  ? dcn10_verify_allow_pstate_change_high+0x1d/0x240 [amdgpu]
[  171.571859]  ? dcn10_set_bandwidth+0x275/0x2d0 [amdgpu]
[  171.571900]  dc_commit_state+0x253/0x550 [amdgpu]
[  171.571940]  ? set_freesync_on_streams.part.6+0x4d/0x250 [amdgpu]
[  171.571979]  ? mod_freesync_set_user_enable+0x11f/0x150 [amdgpu]
[  171.572023]  amdgpu_dm_atomic_commit_tail+0x37c/0xd70 [amdgpu]
[  171.572058]  ? amdgpu_bo_pin_restricted+0xd6/0x300 [amdgpu]
[  171.572060]  ? _cond_resched+0x15/0x30
[  171.572061]  ? wait_for_completion_timeout+0x3b/0x1a0
[  171.572063]  ? refcount_inc+0x5/0x30
[  171.572070]  commit_tail+0x3d/0x70 [drm_kms_helper]
[  171.572075]  drm_atomic_helper_commit+0xb4/0x120 [drm_kms_helper]
[  171.572087]  drm_atomic_connector_commit_dpms+0xdb/0x100 [drm]
[  171.572097]  drm_mode_obj_set_property_ioctl+0x174/0x270 [drm]
[  171.572107]  ? drm_mode_obj_find_prop_id+0x40/0x40 [drm]
[  171.572116]  drm_ioctl_kernel+0xa1/0xf0 [drm]
[  171.572126]  drm_ioctl+0x1fc/0x390 [drm]
[  171.572135]  ? drm_mode_obj_find_prop_id+0x40/0x40 [drm]
[  171.572138]  ? eventfd_read+0xed/0x2a0
[  171.572171]  amdgpu_drm_ioctl+0x49/0x80 [amdgpu]
[  171.572174]  do_vfs_ioctl+0xa4/0x620
[  171.572176]  ksys_ioctl+0x60/0x90
[  171.572178]  ? ksys_read+0x9c/0xb0
[  171.572179]  __x64_sys_ioctl+0x16/0x20
[  171.572182]  

[PATCH] drm/zte: Replace drm_dev_unref with drm_dev_put

[Thomas Zimmermann]

This patch unifies the naming of DRM functions for reference counting
of struct drm_device. The resulting code is more aligned with the rest
of the Linux kernel interfaces.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/zte/zx_drm_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/zte/zx_drm_drv.c b/drivers/gpu/drm/zte/zx_drm_drv.c
index 6f4205e80378..02ae1caf6e8a 100644
--- a/drivers/gpu/drm/zte/zx_drm_drv.c
+++ b/drivers/gpu/drm/zte/zx_drm_drv.c
@@ -122,7 +122,7 @@ static int zx_drm_bind(struct device *dev)
component_unbind_all(dev, drm);
 out_unregister:
dev_set_drvdata(dev, NULL);
-   drm_dev_unref(drm);
+   drm_dev_put(drm);
return ret;
 }
 
@@ -136,7 +136,7 @@ static void zx_drm_unbind(struct device *dev)
drm_mode_config_cleanup(drm);
component_unbind_all(dev, drm);
dev_set_drvdata(dev, NULL);
-   drm_dev_unref(drm);
+   drm_dev_put(drm);
 }
 
 static const struct component_master_ops zx_drm_master_ops = {
-- 
2.18.0

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

[PATCH] drm/rockchip: Replace drm_dev_unref with drm_dev_put

[Thomas Zimmermann]

This patch unifies the naming of DRM functions for reference counting
of struct drm_device. The resulting code is more aligned with the rest
of the Linux kernel interfaces.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index f814d37b1db2..9c846be8fc64 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -184,7 +184,7 @@ static int rockchip_drm_bind(struct device *dev)
 err_free:
drm_dev->dev_private = NULL;
dev_set_drvdata(dev, NULL);
-   drm_dev_unref(drm_dev);
+   drm_dev_put(drm_dev);
return ret;
 }
 
@@ -204,7 +204,7 @@ static void rockchip_drm_unbind(struct device *dev)
 
drm_dev->dev_private = NULL;
dev_set_drvdata(dev, NULL);
-   drm_dev_unref(drm_dev);
+   drm_dev_put(drm_dev);
 }
 
 static const struct file_operations rockchip_drm_driver_fops = {
-- 
2.18.0

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

Re: [PATCH v4 08/11] media: vsp1: Add support for extended display list headers

[Laurent Pinchart]

Hi Kieran,

On Monday, 16 July 2018 20:14:55 EEST Kieran Bingham wrote:
> On 24/05/18 12:44, Laurent Pinchart wrote:
> > On Thursday, 3 May 2018 16:36:19 EEST Kieran Bingham wrote:
> >> Extended display list headers allow pre and post command lists to be
> >> executed by the VSP pipeline. This provides the base support for
> >> features such as AUTO_FLD (for interlaced support) and AUTO_DISP (for
> >> supporting continuous camera preview pipelines.
> >> 
> >> Signed-off-by: Kieran Bingham 
> >> 
> >> ---
> >> 
> >> v2:
> >>  - remove __packed attributes
> >> 
> >> ---
> >> 
> >>  drivers/media/platform/vsp1/vsp1.h  |  1 +-
> >>  drivers/media/platform/vsp1/vsp1_dl.c   | 83 +-
> >>  drivers/media/platform/vsp1/vsp1_dl.h   | 29 -
> >>  drivers/media/platform/vsp1/vsp1_drv.c  |  7 +-
> >>  drivers/media/platform/vsp1/vsp1_regs.h |  5 +-
> >>  5 files changed, 116 insertions(+), 9 deletions(-)

[snip]

> >> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> >> b/drivers/media/platform/vsp1/vsp1_dl.c index 56514cd51c51..b64d32535edc
> >> 100644
> >> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> >> +++ b/drivers/media/platform/vsp1/vsp1_dl.c

[snip]

> >> +struct vsp1_dl_ext_header {
> >> +  u32 reserved0;  /* alignment padding */
> >> +
> >> +  u16 pre_ext_cmd_qty;
> > 
> > Should this be called pre_ext_dl_num_cmd to match the datasheet ?
> 
> Yes, renamed.
> 
> >> +  u16 flags;
> > 
> > Aren't the flags supposed to come before the pre_ext_dl_num_cmd field ?
> 
> These are out-of-order to account for the fact that they are 16bit values.

Ah OK. It makes sense, but is a bit confusing when reading the datasheet.

> I felt that keeping them described in the struct was cleaner than shifts
> and masks - but clearly this stands out, due to the byte-ordering.
> 
> Would you prefer I re-write this as 32 bit accesses (or even 64bit),
> with shifts? Or is a comment sufficient here ?

If it doesn't make the code too ugly, using larger accesses would be better I 
think. Otherwise a comment would do I suppose.

> If we rewrite to be 32 bit accesses, would you be happy with the
> following naming:
> 
>   u32 reserved0;
>   u32 pre_ext_dl_num_cmd; /* Also stores command flags. */
>   u32 pre_ext_dl_plist;
>   u32 post_ext_dl_num_cmd;
>   u32 post_ext_dl_plist;
> 
> (Technically the flags are for the whole header, not the just the
> pre_ext, which is why I wanted it separated)
> 
> 
> Actually - now I write that - the 'number of commands' is sort of a flag
> or a parameter? so maybe the following is just as appropriate?:
> 
>   u32 reserved0;

Maybe "u32 zero;" or "u32 padding;" ? The datasheet states this is padding for 
alignment purpose.

>   u32 pre_ext_dl_flags;
>   u32 pre_ext_dl_plist;
>   u32 post_ext_dl_flags;
>   u32 post_ext_dl_plist;
> 
> Or they could be named 'options', or parameters?
> 
> Any comments before I hack that in?
> 
> The annoying part is that the 'flags' aren't part of the pre_ext cmds,
> they declare whether the pre or post cmd should be executed (or both I
> presume, we are yet to see post-cmd usage)

I agree with you, having a separate flag field would be nicer, as the flags 
are shared. I'll chose the easy option of letting you decide what you like 
best :-) All the above options are equally good to me, provided you add a 
comment explaining why the flag comes after the num_cmd field if you decide to 
keep it as a separate field.

> >> +  u32 pre_ext_cmd_plist;
> > 
> > And pre_ext_dl_plist ?
> > 
> >> +
> >> +  u32 post_ext_cmd_qty;
> >> +  u32 post_ext_cmd_plist;
> > 
> > Similar comments for these variables.
> 
> Renamed.
> 
> >> +};
> >> +
> >> +struct vsp1_dl_header_extended {
> >> +  struct vsp1_dl_header header;
> >> +  struct vsp1_dl_ext_header ext;
> >> +};
> >> +
> >>  struct vsp1_dl_entry {
> >>u32 addr;
> >>u32 data;
> >>  };
> >> 
> >> +struct vsp1_dl_ext_cmd_header {
> > 
> > Isn't this referred to in the datasheet as a body entry, not a header ?
> > How about naming it vsp1_dl_ext_cmd_entry ? Or just vsp1_dl_ext_cmd (in
> > which case the other structure that goes by the same name would need to be
> > renamed) ?
> 
> I think I was getting too creative. The reality is this part is really a
> 'header' describing the data in the body, but yes - it should be named
> to match a "Pre Extended Display List Body"
> 
>   s/vsp1_dl_ext_cmd_header/vsp1_pre_ext_dl_body/

Sounds good to me.

> This will then leave "struct vsp1_dl_ext_cmd" which represents the
> object instance within the VSP1 driver only.
> 
> >> +  u32 cmd;
> 
> This should really have been opcode then too :)

Good point.

> >> +  u32 flags;
> >> +  u32 data;
> >> +  u32 reserved;
> > 
> > The datasheet documents this as two 64-bit fields, shouldn't we handle the
> > structure the same way ?
> 
> I was trying to separate out the fields for clarity.
> 
> In this instance (unlike the 16bit handling above), the byte ordering of
> a 64 

Re: [Nouveau] [PATCH v2 2/3] drm/nouveau: Fix runtime PM leak in nv50_disp_atomic_commit()

[Ville Syrjälä]

On Tue, Jul 17, 2018 at 09:33:52AM +0200, Lukas Wunner wrote:
> On Thu, Jul 12, 2018 at 01:02:53PM -0400, Lyude Paul wrote:
> > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> > @@ -1878,7 +1878,7 @@ nv50_disp_atomic_commit(struct drm_device *dev,
> > nv50_disp_atomic_commit_tail(state);
> >  
> > drm_for_each_crtc(crtc, dev) {
> > -   if (crtc->state->enable) {
> > +   if (crtc->state->active) {
> > if (!drm->have_disp_power_ref) {
> > drm->have_disp_power_ref = true;
> > return 0;
> 
> Somewhat tangential comment on this older patch, since you
> continue to dig around in the runtime PM area:
> 
> Whenever a crtc is activated or deactivated in nouveau, we iterate
> over all crtcs and acquire a runtime PM if a crtc is active and
> previously there was no active one, or we drop a ref if none is
> active and previously there was an active one.
> 
> For a while now I've been thinking that it would be more straightforward
> to acquire a ref whenever a crtc is activated and drop one when a crtc
> is deactivated, i.e. hold one ref for every active crtc.  That way the
> have_disp_power_ref variable as well as the iteration logic could be
> removed, leading to a simplification.  Just a suggestion anyway.

The current code looks somewhat busted anyway. First problem is that
it's accessing crtc->state without the appropriate locks held (unless
something always pulls in all crtcs to every commit?). Second issue
is that the rpm_put() is called without waiting for nonblocking commits
to have finished so it looks like you can currentlly remove the power
before the hardware has been properly shut down.

-- 
Ville Syrjälä
Intel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH] drm/nouveau: Don't forget to label dp_aux devices

[Karol Herbst]

Reviewed-by: Karol Herbst 

2018-07-12 19:13 GMT+02:00 Lyude Paul :
> This makes debugging with DP tracing a lot harder to interpret, so name
> each i2c based off the name of the encoder that it's for
>
> Signed-off-by: Lyude Paul 
> ---
>  drivers/gpu/drm/nouveau/dispnv04/disp.c |  2 +-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c |  2 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c | 12 ++--
>  drivers/gpu/drm/nouveau/nouveau_connector.h |  3 ++-
>  4 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv04/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> index 501d2d290e9c..45ff1872d894 100644
> --- a/drivers/gpu/drm/nouveau/dispnv04/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv04/disp.c
> @@ -64,7 +64,7 @@ nv04_display_create(struct drm_device *dev)
> for (i = 0; i < dcb->entries; i++) {
> struct dcb_output *dcbent = >entry[i];
>
> -   connector = nouveau_connector_create(dev, dcbent->connector);
> +   connector = nouveau_connector_create(dev, dcbent);
> if (IS_ERR(connector))
> continue;
>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c 
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 9382e99a0bc7..4f8d51590bbb 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -2198,7 +2198,7 @@ nv50_display_create(struct drm_device *dev)
>
> /* create encoder/connector objects based on VBIOS DCB table */
> for (i = 0, dcbe = >entry[0]; i < dcb->entries; i++, dcbe++) {
> -   connector = nouveau_connector_create(dev, dcbe->connector);
> +   connector = nouveau_connector_create(dev, dcbe);
> if (IS_ERR(connector))
> continue;
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c 
> b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index 7b557c354307..0c5cc600c973 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -408,8 +408,10 @@ nouveau_connector_destroy(struct drm_connector 
> *connector)
> kfree(nv_connector->edid);
> drm_connector_unregister(connector);
> drm_connector_cleanup(connector);
> -   if (nv_connector->aux.transfer)
> +   if (nv_connector->aux.transfer) {
> drm_dp_aux_unregister(_connector->aux);
> +   kfree(nv_connector->aux.name);
> +   }
> kfree(connector);
>  }
>
> @@ -1201,13 +1203,16 @@ drm_conntype_from_dcb(enum dcb_connector_type dcb)
>  }
>
>  struct drm_connector *
> -nouveau_connector_create(struct drm_device *dev, int index)
> +nouveau_connector_create(struct drm_device *dev,
> +const struct dcb_output *dcbe)
>  {
> const struct drm_connector_funcs *funcs = _connector_funcs;
> struct nouveau_drm *drm = nouveau_drm(dev);
> struct nouveau_display *disp = nouveau_display(dev);
> struct nouveau_connector *nv_connector = NULL;
> struct drm_connector *connector;
> +   char aux_name[48] = {0};
> +   int index = dcbe->connector;
> int type, ret = 0;
> bool dummy;
>
> @@ -1306,6 +1311,9 @@ nouveau_connector_create(struct drm_device *dev, int 
> index)
> case DRM_MODE_CONNECTOR_eDP:
> nv_connector->aux.dev = dev->dev;
> nv_connector->aux.transfer = nouveau_connector_aux_xfer;
> +   snprintf(aux_name, sizeof(aux_name), "sor-%04x-%04x",
> +dcbe->hasht, dcbe->hashm);
> +   nv_connector->aux.name = kstrdup(aux_name, GFP_KERNEL);
> ret = drm_dp_aux_register(_connector->aux);
> if (ret) {
> NV_ERROR(drm, "failed to register aux channel\n");
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.h 
> b/drivers/gpu/drm/nouveau/nouveau_connector.h
> index a4d1a059bd3d..2c5cb51c7c33 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.h
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.h
> @@ -35,6 +35,7 @@
>  #include "nouveau_crtc.h"
>
>  struct nvkm_i2c_port;
> +struct dcb_output;
>
>  struct nouveau_connector {
> struct drm_connector base;
> @@ -76,7 +77,7 @@ nouveau_crtc_connector_get(struct nouveau_crtc *nv_crtc)
>  }
>
>  struct drm_connector *
> -nouveau_connector_create(struct drm_device *, int index);
> +nouveau_connector_create(struct drm_device *, const struct dcb_output *);
>
>  extern int nouveau_tv_disable;
>  extern int nouveau_ignorelid;
> --
> 2.17.1
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Nouveau] [PATCH] drm/nouveau: Fix bogus indenting in nouveau_hwmon.c

[Karol Herbst]

isn't there like 1 space missing for each change? Or maybe my client
is messed up, but please align it with the first letter of the
parameters not the "(".

With that fixed: Reviewed-by: Karol Herbst 

On Tue, Jul 17, 2018 at 2:07 AM, Lyude Paul  wrote:
> Signed-off-by: Lyude Paul 
> ---
>  drivers/gpu/drm/nouveau/nouveau_hwmon.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c 
> b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> index 44178b4c3599..d556f24c6c48 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
> @@ -655,7 +655,7 @@ nouveau_read_string(struct device *dev, enum 
> hwmon_sensor_types type, u32 attr,
>
>  static int
>  nouveau_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> -   int channel, long 
> *val)
> +int channel, long *val)
>  {
> switch (type) {
> case hwmon_chip:
> @@ -677,7 +677,7 @@ nouveau_read(struct device *dev, enum hwmon_sensor_types 
> type, u32 attr,
>
>  static int
>  nouveau_write(struct device *dev, enum hwmon_sensor_types type, u32 attr,
> -   int channel, long val)
> + int channel, long val)
>  {
> switch (type) {
> case hwmon_temp:
> --
> 2.17.1
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/mediatek: Replace drm_dev_unref with drm_dev_put

[Philipp Zabel]

On Tue, 2018-07-17 at 10:35 +0200, Thomas Zimmermann wrote:
> This patch unifies the naming of DRM functions for reference counting
> of struct drm_device. The resulting code is more aligned with the rest
> of the Linux kernel interfaces.
> 
> Signed-off-by: Thomas Zimmermann 
> ---
>  drivers/gpu/drm/mediatek/mtk_drm_drv.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c 
> b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> index a2ca90fc403c..5d024a154e1a 100644
> --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c
> @@ -341,7 +341,7 @@ static int mtk_drm_bind(struct device *dev)
>  err_deinit:
>   mtk_drm_kms_deinit(drm);
>  err_free:
> - drm_dev_unref(drm);
> + drm_dev_put(drm);
>   return ret;
>  }
>  
> @@ -350,7 +350,7 @@ static void mtk_drm_unbind(struct device *dev)
>   struct mtk_drm_private *private = dev_get_drvdata(dev);
>  
>   drm_dev_unregister(private->drm);
> - drm_dev_unref(private->drm);
> + drm_dev_put(private->drm);
>   private->drm = NULL;
>  }
>  
> @@ -504,7 +504,7 @@ static int mtk_drm_remove(struct platform_device *pdev)
>  
>   drm_dev_unregister(drm);
>   mtk_drm_kms_deinit(drm);
> - drm_dev_unref(drm);
> + drm_dev_put(drm);
>  
>   component_master_del(>dev, _drm_ops);
>   pm_runtime_disable(>dev);

Reviewed-by: Philipp Zabel 

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

Re: [Nouveau] [PATCH 4/5] drm/nouveau: Grab RPM ref when i2c bus is in use

[Karol Herbst]

mhh, we shouldn't call to Linux APIs from within of nvkm. Rather gaurd
the Linux glue code to the i2c stuff instead, but this is all done
from inside of nvkm. I think we should move it out into
drm/nouveau/nouveau_i2c.c and do the handling there.

On Tue, Jul 17, 2018 at 1:59 AM, Lyude Paul  wrote:
> The i2c bus can be both accessed by DRM itself, along with any of it's
> devnodes (/sys/class/i2c). So, we need to make sure that all codepaths
> using the i2c bus keep the GPU resumed.
>
> Signed-off-by: Lyude Paul 
> Cc: Karol Herbst 
> Cc: sta...@vger.kernel.org
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c 
> b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> index 807a2b67bd64..1de48c990b80 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/i2c/bus.c
> @@ -119,18 +119,28 @@ nvkm_i2c_bus_release(struct nvkm_i2c_bus *bus)
> BUS_TRACE(bus, "release");
> nvkm_i2c_pad_release(pad);
> mutex_unlock(>mutex);
> +   pm_runtime_put_autosuspend(pad->i2c->subdev.device->dev);
>  }
>
>  int
>  nvkm_i2c_bus_acquire(struct nvkm_i2c_bus *bus)
>  {
> struct nvkm_i2c_pad *pad = bus->pad;
> +   struct device *dev = pad->i2c->subdev.device->dev;
> int ret;
> +
> BUS_TRACE(bus, "acquire");
> +
> +   ret = pm_runtime_get_sync(dev);
> +   if (ret < 0 && ret != -EACCES)
> +   return ret;
> +
> mutex_lock(>mutex);
> ret = nvkm_i2c_pad_acquire(pad, NVKM_I2C_PAD_I2C);
> -   if (ret)
> +   if (ret) {
> mutex_unlock(>mutex);
> +   pm_runtime_put_autosuspend(dev);
> +   }
> return ret;
>  }
>
> --
> 2.17.1
>
> ___
> Nouveau mailing list
> nouv...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/imx: Replace drm_dev_unref with drm_dev_put

[Philipp Zabel]

On Tue, 2018-07-17 at 10:33 +0200, Thomas Zimmermann wrote:
> This patch unifies the naming of DRM functions for reference counting
> of struct drm_device. The resulting code is more aligned with the rest
> of the Linux kernel interfaces.
> 
> Signed-off-by: Thomas Zimmermann 

Thank you, applied to imx-drm/next.

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