date:20180418

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

Marta Löfstedt  changed:

   What|Removed |Added

   See Also||https://bugs.freedesktop.or
   ||g/show_bug.cgi?id=106127

-- 
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 105597] [CI] igt@kms_cursor_legacy@cursora-vs-flipa-atomic-transition - incomplete - Build timed out (after 18 minutes)

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

Marta Löfstedt  changed:

   What|Removed |Added

   See Also||https://bugs.freedesktop.or
   ||g/show_bug.cgi?id=106127

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

[radeon-alex:amd-staging-drm-next 170/201] drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu_helper.c:663 smu_set_watermarks_for_clocks_ranges() error: we previously assumed 'wm_with_clock_ranges' cou

2018-04-18 Thread Dan Carpenter

tree:   git://people.freedesktop.org/~agd5f/linux.git amd-staging-drm-next
head:   d64547a1cfa860e241b27723c88f86fa3d88d3d7
commit: d6c9a7dc86cd39146afb0f47c06b6f95d7dd4997 [170/201] drm/amd/pp: Move 
common code to smu_helper.c

smatch warnings:
drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu_helper.c:663 
smu_set_watermarks_for_clocks_ranges() error: we previously assumed 
'wm_with_clock_ranges' could be null (see line 660)

git remote add radeon-alex git://people.freedesktop.org/~agd5f/linux.git
git remote update radeon-alex
git checkout d6c9a7dc86cd39146afb0f47c06b6f95d7dd4997
vim +/wm_with_clock_ranges +663 
drivers/gpu/drm/amd/amdgpu/../powerplay/hwmgr/smu_helper.c

d6c9a7dc Rex Zhu 2018-04-08  653  
d6c9a7dc Rex Zhu 2018-04-08  654  int smu_set_watermarks_for_clocks_ranges(void 
*wt_table,
d6c9a7dc Rex Zhu 2018-04-08  655struct 
pp_wm_sets_with_clock_ranges_soc15 *wm_with_clock_ranges)
d6c9a7dc Rex Zhu 2018-04-08  656  {
d6c9a7dc Rex Zhu 2018-04-08  657uint32_t i;
d6c9a7dc Rex Zhu 2018-04-08  658struct watermarks *table = wt_table;
d6c9a7dc Rex Zhu 2018-04-08  659  
d6c9a7dc Rex Zhu 2018-04-08 @660if (!table || wm_with_clock_ranges)
  
This test is inverted.  We should be checking if it's NULL not non-NULL.

d6c9a7dc Rex Zhu 2018-04-08  661return -EINVAL;
d6c9a7dc Rex Zhu 2018-04-08  662  
d6c9a7dc Rex Zhu 2018-04-08 @663if 
(wm_with_clock_ranges->num_wm_sets_dmif > 4 || 
wm_with_clock_ranges->num_wm_sets_mcif > 4)

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Freedreno] [DPU PATCH v2 2/2] drm/panel: add backlight control support for truly panel

2018-04-18 Thread abhinavk

Hi Bjorn

Thanks very much for the detailed response.

Yes, we decided that userspace hardcoding this node name is not a strong 
enough

reason to register as another backlight device.

Had one follow up question though.

The QC WLED driver, drivers/leds/leds-qpnp-wled.c is not registering 
itself as a backlight device.

It only registers as a led device.

In that case, we cannot invoke of_find_backlight_by_node to get a handle 
on it.

One question we have is , is it required that every WLED driver register 
itself as a backlight device too?

In this case since it is not doing so, but we need to trigger it for the 
backlight.

Is there any way we can do this?

OR shall we just abandon the backlight control out of this driver 
entirely.

Thanks

Abhinav

On 2018-04-18 21:00, Bjorn Andersson wrote:

On Tue 17 Apr 17:04 PDT 2018, abhin...@codeaurora.org wrote:

Hi Bjorn

Apologies if the prev reply wasnt clear.

Hope this one is.

Much better, now we can discuss the actual issues :)

reply inline.

On 2018-04-17 14:29, Bjorn Andersson wrote:
> On Tue 17 Apr 11:21 PDT 2018, abhin...@codeaurora.org wrote:
> > On 2018-04-16 23:13, Bjorn Andersson wrote:
> [..]
> > > If the panel isn't actually a piece of backlight hardware then it should
> > > not register a backlight device. Why do you need your own sysfs?
> > >
> > > Regards,
> > > Bjorn
> > [Abhinav] This particular panel isnt a piece of backlight hardware.
> > But, we want to have our own sysfs to give flexibility to our
> > userspace
> > to write and read stuff for its proprietary uses.
>
> Please be more specific in your replies, no one will accept code that
> "does stuff" and expecting a reviewer to spend time guessing or pulling
> the information out of you is a sure way to get your patches ignored.
>
> Exactly what kind of stuff are you talking about here and exactly which
> problem are you solving.
>
> > Thats how our downstream has been working so far and hence to maintain
> > the compatibility would like to have it.
>
> Make your proprietary code work with the upstream kernel and you
> shouldn't ever have to modify it.
>
> Regards,
> Bjorn

[Abhinav] We have a few userspace clients today for the backlight 
sysfs node

which read/write directly to
"/sys/class/backlight/panel0-backlight/brightness"
When I said "stuff" I was only referring to the brightness value.
This separate sysfs node allows us to validate those userspace
features of ours which directly edit the backlight value on our
reference platform.

That's nice, but you're enforcing that either all panel drivers must
implement this backlight wrapper or that your customers must modify
their user space to match their backlight implementation.

Since our reference platform uses this panel,hence having our own
sysfs alias helps.  Otherwise, whenever we try to use this panel with
upstream code we will have to end up changing all those places in our
userspace/framework to change the sysfs path.

The actual problem comes down to "how does user space know the name of
the backlight instance associated with the panel" and this is a valid
question to pursue.

But given your current design you could just scan for the one and only
backlight device available in the system; as your use of the static 
name

"panel0-backlight" doesn't allow multiple backlights anyway.

If your goal is simply to ship your reference with something that you
can show work, then just replace the hard coded panel0-backlight with
the name of the wled backlight device. Customers can change panels as
they wish, but in the event that they plug in a different backlight
controller they would need to modify the code.

Hence we wanted to preserve that sysfs node name.
The wled device is not created under /sys/class/backlight but under
/sys/class/leds/wled.
So we will have to change the name of this node across all our 
userspace.

Hard coding /sys/class/backlight/panel0-backlight in your user space
instead of /sys/class/leds/wled is hardly a gain, in particular since
the cost is 94 insertions - per panel driver.

Regards,
Bjorn
___
Freedreno mailing list
freedr...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

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

Re: [PATCH v1 2/4] dt-bindings: clock: mediatek: add g3dsys bindings

2018-04-18 Thread Stephen Boyd

Quoting sean.w...@mediatek.com (2018-04-18 03:24:54)
> From: Sean Wang 
> 
> Add bindings to g3dsys providing necessary clock and reset control to
> Mali-450.
> 
> Signed-off-by: Sean Wang 
> ---
>  .../bindings/arm/mediatek/mediatek,g3dsys.txt  | 30 
> ++

Why isn't this under bindings/clock/ ?

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

[Bug 105425] 3D & games produce periodic GPU crashes (Radeon R7 370)

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

--- Comment #36 from MirceaKitsune  ---
(In reply to iive from comment #35)

I will be busy tomorrow, and also wanted to look into how to do the other more
complicated tests. I'll be trying this out sometime in the next days.

-- 
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: [Freedreno] [DPU PATCH v2 2/2] drm/panel: add backlight control support for truly panel

2018-04-18 Thread abhinavk


Thanks Daniel and Sean for your comments.

Yes, the magic and algorithm is in userspace.

After this discussion, it seems like the complexity of the userspace 
having to
figure out which device node to use and to scale the backlight 
accordingly is not
a strong enough reason to handle this in the driver as it seems like 
there are other

opensource userspace clients doing the same thing.

I will submit another patchset to use the method suggested by Bjorne.

Thanks

Abhinav
On 2018-04-18 06:42, Sean Paul wrote:

On Wed, Apr 18, 2018 at 11:52:18AM +0100, Daniel Thompson wrote:
On Tue, Apr 17, 2018 at 05:42:04PM -0700, abhin...@codeaurora.org 
wrote:

> Adding another point.
>
> On 2018-04-17 17:04, abhin...@codeaurora.org wrote:
> > Hi Bjorn
> >
> > Apologies if the prev reply wasnt clear.
> >
> > Hope this one is.
> >
> > reply inline.
> >
> > On 2018-04-17 14:29, Bjorn Andersson wrote:
> > > On Tue 17 Apr 11:21 PDT 2018, abhin...@codeaurora.org wrote:
> > > > On 2018-04-16 23:13, Bjorn Andersson wrote:
> > > [..]
> > > > > If the panel isn't actually a piece of backlight hardware then it 
should
> > > > > not register a backlight device. Why do you need your own sysfs?
> > > > >
> > > > > Regards,
> > > > > Bjorn
> > > > [Abhinav] This particular panel isnt a piece of backlight hardware.
> > > > But, we want to have our own sysfs to give flexibility to our
> > > > userspace
> > > > to write and read stuff for its proprietary uses.
> > >
> > > Please be more specific in your replies, no one will accept code that
> > > "does stuff" and expecting a reviewer to spend time guessing or
> > > pulling
> > > the information out of you is a sure way to get your patches ignored.
> > >
> > > Exactly what kind of stuff are you talking about here and exactly
> > > which
> > > problem are you solving.
> > >
> > > > Thats how our downstream has been working so far and hence to
> > > > maintain
> > > > the compatibility would like to have it.
> > >
> > > Make your proprietary code work with the upstream kernel and you
> > > shouldn't ever have to modify it.
> > >
> > > Regards,
> > > Bjorn
> >
> > [Abhinav] We have a few userspace clients today for the backlight sysfs
> > node
> > which read/write directly to
> > "/sys/class/backlight/panel0-backlight/brightness"
> > When I said "stuff" I was only referring to the brightness value.
> > This separate sysfs node allows us to validate those userspace features
> > of ours
> > which directly edit the backlight value on our reference platform.
> > Since our reference platform uses this panel,hence having our own
> > sysfs alias helps.
> > Otherwise, whenever we try to use this panel with upstream code we
> > will have to end up
> > changing all those places in our userspace/framework to change the sysfs
> > path.
> > Hence we wanted to preserve that sysfs node name.
> > The wled device is not created under /sys/class/backlight but under
> > /sys/class/leds/wled.
> > So we will have to change the name of this node across all our
> > userspace.
> >
> > If this isnt a convincing reason enough to have its own sysfs node
> > path, I will use
> > your approach.
> >
> > Let me know your comments or if this is still not clear.
> >
> [Abhinav] We also might not be using the brightness value "as-it-is".
>
> We will potentially scale it up/down based on some requirements.
>
> If we do not have our own sysfs alias for this, how do we account for
> providing this interface for our chipset specific backlight dependent
> feature.
>
> Can you please comment on this?

Not easily. It's rather unclear what this chipset specific backlight
dependent feature you have alluded to is so how can we suggest how to
control or model it in the upstream kernel?



The code is here:

https://gitlab.freedesktop.org/seanpaul/dpu-staging/blob/mtp-squashed/drivers/gpu/drm/msm/dsi-staging/dsi_display.c#L76

AFAICT, there's nothing fancy in the kernel aside from scaling the 
brightness
level down twice. I assume the magic is in userspace. My initial 
reaction was
that the scaling factor should just be applied in userspace. Especially 
since
the scaling factor reduces the resolution of the backlight, and that's 
not

immediately obvious by looking at "brightness".

Sean


I can make a guess that is might be to do with brightness curves... 
but

I'd really prefer not to have to guess.

There are some problems with the current interface because it is not
well defined with the brightness control is linear or
logarithmic/perceptual (patches welcome) but for other common embedded
backlights (pwm_bl particularly) we expect calibration of the
brightness curve to be a job for the device tree (because it is a
property of the hardware it can be described in the DT) and there are
patches pending to improve this.


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

__

[Bug 105425] 3D & games produce periodic GPU crashes (Radeon R7 370)

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

--- Comment #35 from i...@yahoo.com ---
Any results?

Enable SysRq, start Xonotic, set r_shadow 2, play until it crashes, use SysRq
to sync, umonut, reboot.

After reboot, check if the crash has been captured by syslog/journald.

If there is nothing, then you'd have to use `netconsole`. OpenSUSE has it
compiled as module, so the description that involves `insmod` or `modprobe`
applies to you.

If you have the crash in the logs, then it is more likely that apitrace file
will remain whole after the hang&reboot.

-- 
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: [DPU PATCH v2 1/2] drm/msm/dsi: adjust dsi timing for dual dsi mode

2018-04-18 Thread chandanu


On 2018-04-17 13:28, Sean Paul wrote:

On Mon, Apr 16, 2018 at 05:40:13PM -0700, Chandan Uddaraju wrote:

For dual dsi mode, the horizontal timing needs
to be divided by half since both the dsi controllers
will be driving this panel. Adjust the pixel clock and
DSI timing accordingly.

Changes in V2:
--Removed Change-Id from the commit text tags.


You ignored my feedback on v1



Sorry Sean, I missed fixing your comment for this patch. I have uploaded 
V3 patch series for review.


thanks
Chandan


Signed-off-by: Chandan Uddaraju 
---
 drivers/gpu/drm/msm/dsi/dsi.h |  1 +
 drivers/gpu/drm/msm/dsi/dsi_host.c| 17 +
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 15 +++
 3 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h 
b/drivers/gpu/drm/msm/dsi/dsi.h

index 70d9a9a..4131b47 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -161,6 +161,7 @@ void msm_dsi_host_cmd_xfer_commit(struct 
mipi_dsi_host *host,

u32 dma_base, u32 len);
 int msm_dsi_host_enable(struct mipi_dsi_host *host);
 int msm_dsi_host_disable(struct mipi_dsi_host *host);
+void msm_dsi_host_adjust_timing_config(struct mipi_dsi_host *host);
 int msm_dsi_host_power_on(struct mipi_dsi_host *host,
struct msm_dsi_phy_shared_timings *phy_shared_timings);
 int msm_dsi_host_power_off(struct mipi_dsi_host *host);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c

index 7a03a94..66a21cb 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -2237,6 +2237,23 @@ static void msm_dsi_sfpb_config(struct 
msm_dsi_host *msm_host, bool enable)

SFPB_GPREG_MASTER_PORT_EN(en));
 }

+void msm_dsi_host_adjust_timing_config(struct mipi_dsi_host *host)
+{
+   struct msm_dsi_host *msm_host = to_msm_dsi_host(host);
+   struct drm_display_mode *mode = NULL;
+
+   mode = msm_host->mode;
+
+   mutex_lock(&msm_host->dev_mutex);
+   mode->htotal >>= 1;
+   mode->hdisplay >>= 1;
+   mode->hsync_start >>= 1;
+   mode->hsync_end >>= 1;
+
+   mode->clock >>= 1;
+   mutex_unlock(&msm_host->dev_mutex);
+}
+
 int msm_dsi_host_power_on(struct mipi_dsi_host *host,
struct msm_dsi_phy_shared_timings *phy_shared_timings)
 {
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c 
b/drivers/gpu/drm/msm/dsi/dsi_manager.c

index 4cb1cb6..8ef1c3d 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -627,6 +627,21 @@ static void dsi_mgr_bridge_mode_set(struct 
drm_bridge *bridge,

msm_dsi_host_set_display_mode(host, adjusted_mode);
if (is_dual_dsi && other_dsi)
msm_dsi_host_set_display_mode(other_dsi->host, adjusted_mode);
+
+   /*
+* For dual DSI mode, the current DRM mode has
+* the complete width of the panel. Since, the complete
+* panel is driven by two DSI controllers, the
+* horizontal timings and the pixel clock have to be
+* split between the two dsi controllers. Adjust the
+* DSI host timing structures accordingly.
+*/
+   if (is_dual_dsi) {
+   msm_dsi_host_adjust_timing_config(host);
+   if (other_dsi)
+   msm_dsi_host_adjust_timing_config(other_dsi->host);
+   }
+
 }

 static const struct drm_connector_funcs dsi_mgr_connector_funcs = {
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,

a Linux Foundation Collaborative Project


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

[DPU PATCH v3 2/2] drm/msm/dsi: Use one connector for dual DSI mode

2018-04-18 Thread Chandan Uddaraju

Current DSI driver uses two connectors for dual DSI case even
though we only have one panel. Fix this by implementing one
connector/bridge for dual DSI use case. Use master DSI
controllers to register one connector/bridge.

Changes in V2:
-Removed Change-Id from the commit text tags.
-Remove extra parentheses

Changes in V3:
-None

Signed-off-by: Chandan Uddaraju 
---
 drivers/gpu/drm/msm/dsi/dsi.c |   3 +
 drivers/gpu/drm/msm/dsi/dsi.h |   1 +
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 110 --
 3 files changed, 29 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.c b/drivers/gpu/drm/msm/dsi/dsi.c
index b744bcc..ff8164c 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.c
+++ b/drivers/gpu/drm/msm/dsi/dsi.c
@@ -208,6 +208,9 @@ int msm_dsi_modeset_init(struct msm_dsi *msm_dsi, struct 
drm_device *dev,
goto fail;
}
 
+   if (!msm_dsi_manager_validate_current_config(msm_dsi->id))
+   goto fail;
+
msm_dsi->encoder = encoder;
 
msm_dsi->bridge = msm_dsi_manager_bridge_init(msm_dsi->id);
diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 01c38f6..c858e8e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -100,6 +100,7 @@ struct msm_dsi {
 void msm_dsi_manager_attach_dsi_device(int id, u32 device_flags);
 int msm_dsi_manager_register(struct msm_dsi *msm_dsi);
 void msm_dsi_manager_unregister(struct msm_dsi *msm_dsi);
+bool msm_dsi_manager_validate_current_config(u8 id);
 
 /* msm dsi */
 static inline bool msm_dsi_device_connected(struct msm_dsi *msm_dsi)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_manager.c 
b/drivers/gpu/drm/msm/dsi/dsi_manager.c
index 3bb506b..2a11f82 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_manager.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_manager.c
@@ -306,67 +306,6 @@ static void dsi_mgr_connector_destroy(struct drm_connector 
*connector)
kfree(dsi_connector);
 }
 
-static void dsi_dual_connector_fix_modes(struct drm_connector *connector)
-{
-   struct drm_display_mode *mode, *m;
-
-   /* Only support left-right mode */
-   list_for_each_entry_safe(mode, m, &connector->probed_modes, head) {
-   mode->clock >>= 1;
-   mode->hdisplay >>= 1;
-   mode->hsync_start >>= 1;
-   mode->hsync_end >>= 1;
-   mode->htotal >>= 1;
-   drm_mode_set_name(mode);
-   }
-}
-
-static int dsi_dual_connector_tile_init(
-   struct drm_connector *connector, int id)
-{
-   struct drm_display_mode *mode;
-   /* Fake topology id */
-   char topo_id[8] = {'M', 'S', 'M', 'D', 'U', 'D', 'S', 'I'};
-
-   if (connector->tile_group) {
-   DBG("Tile property has been initialized");
-   return 0;
-   }
-
-   /* Use the first mode only for now */
-   mode = list_first_entry(&connector->probed_modes,
-   struct drm_display_mode,
-   head);
-   if (!mode)
-   return -EINVAL;
-
-   connector->tile_group = drm_mode_get_tile_group(
-   connector->dev, topo_id);
-   if (!connector->tile_group)
-   connector->tile_group = drm_mode_create_tile_group(
-   connector->dev, topo_id);
-   if (!connector->tile_group) {
-   pr_err("%s: failed to create tile group\n", __func__);
-   return -ENOMEM;
-   }
-
-   connector->has_tile = true;
-   connector->tile_is_single_monitor = true;
-
-   /* mode has been fixed */
-   connector->tile_h_size = mode->hdisplay;
-   connector->tile_v_size = mode->vdisplay;
-
-   /* Only support left-right mode */
-   connector->num_h_tile = 2;
-   connector->num_v_tile = 1;
-
-   connector->tile_v_loc = 0;
-   connector->tile_h_loc = (id == DSI_RIGHT) ? 1 : 0;
-
-   return 0;
-}
-
 static int dsi_mgr_connector_get_modes(struct drm_connector *connector)
 {
int id = dsi_mgr_connector_get_id(connector);
@@ -377,31 +316,15 @@ static int dsi_mgr_connector_get_modes(struct 
drm_connector *connector)
if (!panel)
return 0;
 
-   /* Since we have 2 connectors, but only 1 drm_panel in dual DSI mode,
-* panel should not attach to any connector.
-* Only temporarily attach panel to the current connector here,
-* to let panel set mode to this connector.
+   /*
+* In dual DSI mode, we have one connector that can be
+* attached to the drm_panel.
 */
drm_panel_attach(panel, connector);
num = drm_panel_get_modes(panel);
-   drm_panel_detach(panel);
if (!num)
return 0;
 
-   if (IS_DUAL_DSI()) {
-   /* report half resolution to user */
-   dsi_dual_connector_fix_modes(connector);
-   ret

[DPU PATCH v3 1/2] drm/msm/dsi: adjust dsi timing for dual dsi mode

2018-04-18 Thread Chandan Uddaraju

For dual dsi mode, the horizontal timing needs
to be divided by half since both the dsi controllers
will be driving this panel. Adjust the pixel clock and
DSI timing accordingly.

Changes in V2:
--Removed Change-Id from the commit text tags.

Changes in V3:
--Instead of adjusting the DRM mode structure, divide
  the clocks and horizontal timings in DSI host just
  before configuring the values.

Signed-off-by: Chandan Uddaraju 
---
 drivers/gpu/drm/msm/dsi/dsi.h |  6 ++--
 drivers/gpu/drm/msm/dsi/dsi_host.c| 55 ---
 drivers/gpu/drm/msm/dsi/dsi_manager.c |  7 +++--
 3 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi.h b/drivers/gpu/drm/msm/dsi/dsi.h
index 70d9a9a..01c38f6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi.h
+++ b/drivers/gpu/drm/msm/dsi/dsi.h
@@ -162,7 +162,8 @@ void msm_dsi_host_cmd_xfer_commit(struct mipi_dsi_host 
*host,
 int msm_dsi_host_enable(struct mipi_dsi_host *host);
 int msm_dsi_host_disable(struct mipi_dsi_host *host);
 int msm_dsi_host_power_on(struct mipi_dsi_host *host,
-   struct msm_dsi_phy_shared_timings *phy_shared_timings);
+   struct msm_dsi_phy_shared_timings *phy_shared_timings,
+   bool is_dual_dsi);
 int msm_dsi_host_power_off(struct mipi_dsi_host *host);
 int msm_dsi_host_set_display_mode(struct mipi_dsi_host *host,
struct drm_display_mode *mode);
@@ -175,7 +176,8 @@ int msm_dsi_host_set_src_pll(struct mipi_dsi_host *host,
struct msm_dsi_pll *src_pll);
 void msm_dsi_host_reset_phy(struct mipi_dsi_host *host);
 void msm_dsi_host_get_phy_clk_req(struct mipi_dsi_host *host,
-   struct msm_dsi_phy_clk_request *clk_req);
+   struct msm_dsi_phy_clk_request *clk_req,
+   bool is_dual_dsi);
 void msm_dsi_host_destroy(struct mipi_dsi_host *host);
 int msm_dsi_host_modeset_init(struct mipi_dsi_host *host,
struct drm_device *dev);
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 7a03a94..75d527e 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -118,6 +118,7 @@ struct msm_dsi_host {
struct clk *byte_intf_clk;
 
u32 byte_clk_rate;
+   u32 pixel_clk_rate;
u32 esc_clk_rate;
 
/* DSI v2 specific clocks */
@@ -510,7 +511,7 @@ static int dsi_link_clk_enable_6g(struct msm_dsi_host 
*msm_host)
goto error;
}
 
-   ret = clk_set_rate(msm_host->pixel_clk, msm_host->mode->clock * 1000);
+   ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate);
if (ret) {
pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret);
goto error;
@@ -591,7 +592,7 @@ static int dsi_link_clk_enable_v2(struct msm_dsi_host 
*msm_host)
goto error;
}
 
-   ret = clk_set_rate(msm_host->pixel_clk, msm_host->mode->clock * 1000);
+   ret = clk_set_rate(msm_host->pixel_clk, msm_host->pixel_clk_rate);
if (ret) {
pr_err("%s: Failed to set rate pixel clk, %d\n", __func__, ret);
goto error;
@@ -661,7 +662,7 @@ static void dsi_link_clk_disable(struct msm_dsi_host 
*msm_host)
}
 }
 
-static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host)
+static int dsi_calc_clk_rate(struct msm_dsi_host *msm_host, bool is_dual_dsi)
 {
struct drm_display_mode *mode = msm_host->mode;
const struct msm_dsi_cfg_handler *cfg_hnd = msm_host->cfg_hnd;
@@ -675,14 +676,28 @@ static int dsi_calc_clk_rate(struct msm_dsi_host 
*msm_host)
}
 
pclk_rate = mode->clock * 1000;
+
+   /*
+* For dual DSI mode, the current DRM mode has
+* the complete width of the panel. Since, the complete
+* panel is driven by two DSI controllers, the
+* the clock rates have to be split between
+* the two dsi controllers. Adjust the byte and
+* pixel clock rates for each dsi host accordingly.
+*/
+   if (is_dual_dsi)
+   pclk_rate /= 2;
+
if (lanes > 0) {
msm_host->byte_clk_rate = (pclk_rate * bpp) / (8 * lanes);
} else {
pr_err("%s: forcing mdss_dsi lanes to 1\n", __func__);
msm_host->byte_clk_rate = (pclk_rate * bpp) / 8;
}
+   msm_host->pixel_clk_rate = pclk_rate;
 
-   DBG("pclk=%d, bclk=%d", pclk_rate, msm_host->byte_clk_rate);
+   DBG("pclk=%d, bclk=%d", msm_host->pixel_clk_rate,
+   msm_host->byte_clk_rate);
 
msm_host->esc_clk_rate = clk_get_rate(msm_host->esc_clk);
 
@@ -884,7 +899,7 @@ static void dsi_ctrl_config(struct msm_dsi_host *msm_host, 
bool enable,
dsi_write(msm_host, REG_DSI_CTRL, data);
 }
 
-static void dsi_timing_setup(struct msm_dsi_host *msm_host)
+static void dsi_timing_setup(struct

[DPU PATCH v3 0/2] Connector virtualization for Dual-DSI

2018-04-18 Thread Chandan Uddaraju

This patch series adds support to DSI connector
virtualization for Dual-DSI configuration.

These changes have been tested using dual-dsi truly panel on sdm845 platform.

Additional changes that will be needed to have end-to-end functionality:
 --> DSI6G-v2 changes: https://patchwork.kernel.org/patch/10294605/
 --> truly panel patches: https://patchwork.kernel.org/patch/10327749/
 --> DPU changes that will be uploaded soon.

Changes in V2:
  Addressed Sean's review comments:
-Removed Change-Id from the commit text tags.
-Remove extra parentheses

Changes in V3:
  Addressed Sean's review comments:
--Instead of updating the DRM mode structure, divide
  the clocks and horizontal timings in DSI host just
  before configuring the values.

Chandan Uddaraju (2):
  drm/msm/dsi: adjust dsi timing for dual dsi mode
  drm/msm/dsi: Use one connector for dual DSI mode

 drivers/gpu/drm/msm/dsi/dsi.c |   3 +
 drivers/gpu/drm/msm/dsi/dsi.h |   7 +-
 drivers/gpu/drm/msm/dsi/dsi_host.c|  55 
 drivers/gpu/drm/msm/dsi/dsi_manager.c | 117 +-
 4 files changed, 81 insertions(+), 101 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

[PULL] drm-misc-fixes


Hi Dave,
Here's the first misc-fixes pull. It unfortunately contains a backmerge due to
Eric committing Daniel's patch before I had a chance to fast forward. Anyways,
it's not too convoluted, so I'll try to win the race this week :-). In general,
this is going to become more common as the number of committers grows. If it
bugs you, let me know and we'll figure out a process to avoid it.


drm-misc-fixes-2018-04-18-1:
drm-misc-fixes:

stable: vc4: Fix memory leak during BO teardown (Daniel)
dp: Add i2c retry for LSPCON adapters (Imre)
hdcp: Fix device count mask (Ramalingam)

Cc: Daniel J Blueman 
Cc: Ramalingam C 

Cheers, Sean


The following changes since commit a10beabba213924d876f2d10ca9351aeab93f58a:

  Merge branch 'drm-next-4.17' of git://people.freedesktop.org/~agd5f/linux 
into drm-next (2018-04-13 09:25:21 +1000)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2018-04-18-1

for you to fetch changes up to 7eb2c4dd54ff841f2fe509a84973eb25fa20bda2:

  drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state 
(2018-04-18 16:33:14 +0300)


drm-misc-fixes:

stable: vc4: Fix memory leak during BO teardown (Daniel)
dp: Add i2c retry for LSPCON adapters (Imre)
hdcp: Fix device count mask (Ramalingam)

Cc: Daniel J Blueman 
Cc: Ramalingam C 


Daniel J Blueman (1):
  drm/vc4: Fix memory leak during BO teardown

Imre Deak (1):
  drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state

Ramalingam C (1):
  drm: Fix HDCP downstream dev count read

Sean Paul (1):
  Merge airlied/drm-next into drm-misc-fixes

 drivers/gpu/drm/drm_dp_dual_mode_helper.c  | 39 --
 drivers/gpu/drm/vc4/vc4_bo.c   |  2 ++
 drivers/gpu/drm/vc4/vc4_validate_shaders.c |  1 +
 include/drm/drm_hdcp.h |  2 +-
 4 files changed, 36 insertions(+), 8 deletions(-)

-- 
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] drm/rockchip: Disable blending for win0

On Wed, Apr 18, 2018 at 10:31:52AM -0700, Kristian H. Kristensen wrote:
> Blending win0 with the background color doesn't seem to work
> correctly. 

Did you come to a conclusion about whether this applies across all Rk vops, or
just 3399 big?

Sean

> We only get the background color, no matter the contents of
> the win0 framebuffer.  However, blending pre-multiplied color with the
> default opaque black default background color is a no-op, so we can
> just disable blending to get the correct result.
> 
> Signed-off-by: Kristian H. Kristensen 
> Cc: Sandy Huang 
> Cc: Sean Paul 
> ---
> 
> v2: Drop CHROMIUM: prefix, rebase on Linus' master
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
> b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index 53d4afe15278..753a7548da84 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> @@ -76,6 +76,9 @@
>  #define VOP_WIN_GET_YRGBADDR(vop, win) \
>   vop_readl(vop, win->base + win->phy->yrgb_mst.offset)
>  
> +#define VOP_WIN_TO_INDEX(vop_win) \
> + ((vop_win) - (vop_win)->vop->win)
> +
>  #define to_vop(x) container_of(x, struct vop, crtc)
>  #define to_vop_win(x) container_of(x, struct vop_win, base)
>  
> @@ -708,6 +711,7 @@ static void vop_plane_atomic_update(struct drm_plane 
> *plane,
>   dma_addr_t dma_addr;
>   uint32_t val;
>   bool rb_swap;
> + int win_index = VOP_WIN_TO_INDEX(vop_win);
>   int format;
>  
>   /*
> @@ -777,7 +781,14 @@ static void vop_plane_atomic_update(struct drm_plane 
> *plane,
>   rb_swap = has_rb_swapped(fb->format->format);
>   VOP_WIN_SET(vop, win, rb_swap, rb_swap);
>  
> - if (fb->format->has_alpha) {
> + /*
> +  * Blending win0 with the background color doesn't seem to work
> +  * correctly. We only get the background color, no matter the contents
> +  * of the win0 framebuffer.  However, blending pre-multiplied color
> +  * with the default opaque black default background color is a no-op,
> +  * so we can just disable blending to get the correct result.
> +  */
> + if (fb->format->has_alpha && win_index > 0) {
>   VOP_WIN_SET(vop, win, dst_alpha_ctl,
>   DST_FACTOR_M0(ALPHA_SRC_INVERSE));
>   val = SRC_ALPHA_EN(1) | SRC_COLOR_M0(ALPHA_SRC_PRE_MUL) |
> -- 
> 2.17.0.484.g0c8726318c-goog
> 

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

[Bug 199319] Flickering screen on AMDGPU and DC with Linux 4.16-2

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

--- Comment #19 from Harry Wentland (harry.wentl...@amd.com) ---
@haro41, I recommend opening a new bug if you're seeing a different issue,
unless you have strong reason to believe it's caused by the fixes for the
flickering screen, or otherwise related to it.

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

[PATCH v2] drm/rockchip: Disable blending for win0

2018-04-18 Thread Kristian H. Kristensen

Blending win0 with the background color doesn't seem to work
correctly. We only get the background color, no matter the contents of
the win0 framebuffer.  However, blending pre-multiplied color with the
default opaque black default background color is a no-op, so we can
just disable blending to get the correct result.

Signed-off-by: Kristian H. Kristensen 
Cc: Sandy Huang 
Cc: Sean Paul 
---

v2: Drop CHROMIUM: prefix, rebase on Linus' master

 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c 
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index 53d4afe15278..753a7548da84 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -76,6 +76,9 @@
 #define VOP_WIN_GET_YRGBADDR(vop, win) \
vop_readl(vop, win->base + win->phy->yrgb_mst.offset)
 
+#define VOP_WIN_TO_INDEX(vop_win) \
+   ((vop_win) - (vop_win)->vop->win)
+
 #define to_vop(x) container_of(x, struct vop, crtc)
 #define to_vop_win(x) container_of(x, struct vop_win, base)
 
@@ -708,6 +711,7 @@ static void vop_plane_atomic_update(struct drm_plane *plane,
dma_addr_t dma_addr;
uint32_t val;
bool rb_swap;
+   int win_index = VOP_WIN_TO_INDEX(vop_win);
int format;
 
/*
@@ -777,7 +781,14 @@ static void vop_plane_atomic_update(struct drm_plane 
*plane,
rb_swap = has_rb_swapped(fb->format->format);
VOP_WIN_SET(vop, win, rb_swap, rb_swap);
 
-   if (fb->format->has_alpha) {
+   /*
+* Blending win0 with the background color doesn't seem to work
+* correctly. We only get the background color, no matter the contents
+* of the win0 framebuffer.  However, blending pre-multiplied color
+* with the default opaque black default background color is a no-op,
+* so we can just disable blending to get the correct result.
+*/
+   if (fb->format->has_alpha && win_index > 0) {
VOP_WIN_SET(vop, win, dst_alpha_ctl,
DST_FACTOR_M0(ALPHA_SRC_INVERSE));
val = SRC_ALPHA_EN(1) | SRC_COLOR_M0(ALPHA_SRC_PRE_MUL) |
-- 
2.17.0.484.g0c8726318c-goog

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

Re: [PATCH hwc v2 00/18] Add scene flattening support

2018-04-18 Thread John Stultz

On Fri, Apr 13, 2018 at 5:48 AM, Alexandru-Cosmin Gheorghe
 wrote:
> On second thought, I pushed this patchset here:
> https://github.com/ARM-software/drm-hwcomposer/tree/scene_flattening_support

Thanks for doing this!  For what its worth, I've spun these up on the
HiKey960 (which doesn't have the mali display processor), and they
don't seem to cause any trouble.

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

Re: [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver

2018-04-18 Thread Dongwon Kim

On Wed, Apr 18, 2018 at 09:38:39AM +0300, Oleksandr Andrushchenko wrote:
> On 04/17/2018 11:57 PM, Dongwon Kim wrote:
> >On Tue, Apr 17, 2018 at 09:59:28AM +0200, Daniel Vetter wrote:
> >>On Mon, Apr 16, 2018 at 12:29:05PM -0700, Dongwon Kim wrote:
> >>>Yeah, I definitely agree on the idea of expanding the use case to the
> >>>general domain where dmabuf sharing is used. However, what you are
> >>>targetting with proposed changes is identical to the core design of
> >>>hyper_dmabuf.
> >>>
> >>>On top of this basic functionalities, hyper_dmabuf has driver level
> >>>inter-domain communication, that is needed for dma-buf remote tracking
> >>>(no fence forwarding though), event triggering and event handling, extra
> >>>meta data exchange and hyper_dmabuf_id that represents grefs
> >>>(grefs are shared implicitly on driver level)
> >>This really isn't a positive design aspect of hyperdmabuf imo. The core
> >>code in xen-zcopy (ignoring the ioctl side, which will be cleaned up) is
> >>very simple & clean.
> >>
> >>If there's a clear need later on we can extend that. But for now xen-zcopy
> >>seems to cover the basic use-case needs, so gets the job done.
> >>
> >>>Also it is designed with frontend (common core framework) + backend
> >>>(hyper visor specific comm and memory sharing) structure for portability.
> >>>We just can't limit this feature to Xen because we want to use the same
> >>>uapis not only for Xen but also other applicable hypervisor, like ACORN.
> >>See the discussion around udmabuf and the needs for kvm. I think trying to
> >>make an ioctl/uapi that works for multiple hypervisors is misguided - it
> >>likely won't work.
> >>
> >>On top of that the 2nd hypervisor you're aiming to support is ACRN. That's
> >>not even upstream yet, nor have I seen any patches proposing to land linux
> >>support for ACRN. Since it's not upstream, it doesn't really matter for
> >>upstream consideration. I'm doubting that ACRN will use the same grant
> >>references as xen, so the same uapi won't work on ACRN as on Xen anyway.
> >Yeah, ACRN doesn't have grant-table. Only Xen supports it. But that is why
> >hyper_dmabuf has been architectured with the concept of backend.
> >If you look at the structure of backend, you will find that
> >backend is just a set of standard function calls as shown here:
> >
> >struct hyper_dmabuf_bknd_ops {
> > /* backend initialization routine (optional) */
> > int (*init)(void);
> >
> > /* backend cleanup routine (optional) */
> > int (*cleanup)(void);
> >
> > /* retreiving id of current virtual machine */
> > int (*get_vm_id)(void);
> >
> > /* get pages shared via hypervisor-specific method */
> > int (*share_pages)(struct page **pages, int vm_id,
> >int nents, void **refs_info);
> >
> > /* make shared pages unshared via hypervisor specific method */
> > int (*unshare_pages)(void **refs_info, int nents);
> >
> > /* map remotely shared pages on importer's side via
> >  * hypervisor-specific method
> >  */
> > struct page ** (*map_shared_pages)(unsigned long ref, int vm_id,
> >int nents, void **refs_info);
> >
> > /* unmap and free shared pages on importer's side via
> >  * hypervisor-specific method
> >  */
> > int (*unmap_shared_pages)(void **refs_info, int nents);
> >
> > /* initialize communication environment */
> > int (*init_comm_env)(void);
> >
> > void (*destroy_comm)(void);
> >
> > /* upstream ch setup (receiving and responding) */
> > int (*init_rx_ch)(int vm_id);
> >
> > /* downstream ch setup (transmitting and parsing responses) */
> > int (*init_tx_ch)(int vm_id);
> >
> > int (*send_req)(int vm_id, struct hyper_dmabuf_req *req, int wait);
> >};
> >
> >All of these can be mapped with any hypervisor specific implementation.
> >We designed backend implementation for Xen using grant-table, Xen event
> >and ring buffer communication. For ACRN, we have another backend using 
> >Virt-IO
> >for both memory sharing and communication.
> >
> >We tried to define this structure of backend to make it general enough (or
> >it can be even modified or extended to support more cases.) so that it can
> >fit to other hypervisor cases. Only requirements/expectation on the 
> >hypervisor
> >are page-level memory sharing and inter-domain communication, which I think
> >are standard features of modern hypervisor.
> >
> >And please review common UAPIs that hyper_dmabuf and xen-zcopy supports. They
> >are very general. One is getting FD (dmabuf) and get those shared. The other
> >is generating dmabuf from global handle (secure handle hiding gref behind 
> >it).
> >On top of this, hyper_dmabuf has "unshare" and "query" which are also useful
> >for any cases.
> >
> >So I don't know why we wouldn't want to try to make these standar

[Bug 199319] Flickering screen on AMDGPU and DC with Linux 4.16-2

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

--- Comment #18 from har...@gmx.de ---
@Harry Wentland,
i am trying kernel 4.17.0-rc1 (patches already merged by linus):

With 'amdgpu.dc=1', i get a black screen now at login (perhaps related to this
dc issue). I found 2 new warnings and call traces in kernel log, like:

Apr 18 18:28:41 debian kernel: WARNING: CPU: 10 PID: 345 at
drivers/gpu/drm/amd/amdgpu/../display/dc/dm_services.h:132
generic_reg_update_ex+0x102/0x150 [amdgpu]

...

-- 
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 1/2 v3] drm/pl111: Support the Versatile Express

2018-04-18 Thread Eric Anholt

Linus Walleij  writes:

> The Versatile Express uses a special configuration controller
> deeply embedded in the system motherboard FPGA to multiplex the
> two to three (!) display controller instances out to the single
> SiI9022 bridge.

With Robin's little fixes merged in, this is:

Acked-by: Eric Anholt 


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

Re: [PATCH 2/2 v3] drm/pl111: Enable device-specific assigned memory

2018-04-18 Thread Eric Anholt

Linus Walleij  writes:

> The Versatile Express has 8 MB of dedicated video RAM (VRAM)
> on the motherboard, which is what we should be using for the
> PL111 if available. On this platform, the memory backplane
> is constructed so that only this memory will work properly
> with the CLCD on the motherboard, using any other memory
> area just gives random snow on the display.
>
> The CA9 Versatile Express also has a PL111 instance on its
> core tile that can address all memory, and this does not
> have the restriction.

Reviewed-by: Eric Anholt 


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

[Bug 106111] [GPU Passthrough]GPU (Polaris) not reinitialized with Linux VM (Reset bug)

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

--- Comment #5 from Max  ---
(In reply to Alex Williamson from comment #4)
> There is a difference, now we have:
> 
> [   84.997634] vfio_ecap_init: :0a:00.0 hiding ecap 0x19@0x270
> [   84.997645] vfio_ecap_init: :0a:00.0 hiding ecap 0x1b@0x2d0
> [   84.997653] vfio_ecap_init: :0a:00.0 hiding ecap 0x1e@0x370
> [  145.518307] vfio_ecap_init: :0a:00.0 hiding ecap 0x19@0x270
> [  145.518313] vfio_ecap_init: :0a:00.0 hiding ecap 0x1b@0x2d0
> [  145.518318] vfio_ecap_init: :0a:00.0 hiding ecap 0x1e@0x370
> 
> So prior to time 145.5 the VM was shutdown and started again and we could
> still read config space of the device.  Previously we were already getting
> IOMMU faults before the second startup.  But shortly after:
> 
> [  193.328586] AMD-Vi: Completion-Wait loop timed out
> [  193.488711] AMD-Vi: Completion-Wait loop timed out
> [  194.169913] iommu ivhd0: AMD-Vi: Event logged [
> [  194.169921] iommu ivhd0: IOTLB_INV_TIMEOUT device=0a:00.0
> address=0x00043e8aaca0]
> [  194.169924] iommu ivhd0: AMD-Vi: Event logged [
> [  194.169928] iommu ivhd0: IOTLB_INV_TIMEOUT device=0a:00.0
> address=0x00043e8aacc0]
> 
> And the stuck in D3 state is evidence that the device is no longer
> accessible on the bus.  So that only delayed the issue, some interaction
> between the IOMMU and GPU is still failing.

Thanks for the explaination Alex.
Something could be done ? 
By AMD or VFIO mainteners ?

-- 
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 106111] [GPU Passthrough]GPU (Polaris) not reinitialized with Linux VM (Reset bug)

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

--- Comment #4 from Alex Williamson  ---
There is a difference, now we have:

[   84.997634] vfio_ecap_init: :0a:00.0 hiding ecap 0x19@0x270
[   84.997645] vfio_ecap_init: :0a:00.0 hiding ecap 0x1b@0x2d0
[   84.997653] vfio_ecap_init: :0a:00.0 hiding ecap 0x1e@0x370
[  145.518307] vfio_ecap_init: :0a:00.0 hiding ecap 0x19@0x270
[  145.518313] vfio_ecap_init: :0a:00.0 hiding ecap 0x1b@0x2d0
[  145.518318] vfio_ecap_init: :0a:00.0 hiding ecap 0x1e@0x370

So prior to time 145.5 the VM was shutdown and started again and we could still
read config space of the device.  Previously we were already getting IOMMU
faults before the second startup.  But shortly after:

[  193.328586] AMD-Vi: Completion-Wait loop timed out
[  193.488711] AMD-Vi: Completion-Wait loop timed out
[  194.169913] iommu ivhd0: AMD-Vi: Event logged [
[  194.169921] iommu ivhd0: IOTLB_INV_TIMEOUT device=0a:00.0
address=0x00043e8aaca0]
[  194.169924] iommu ivhd0: AMD-Vi: Event logged [
[  194.169928] iommu ivhd0: IOTLB_INV_TIMEOUT device=0a:00.0
address=0x00043e8aacc0]

And the stuck in D3 state is evidence that the device is no longer accessible
on the bus.  So that only delayed the issue, some interaction between the IOMMU
and GPU is still failing.

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

Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer

2018-04-18 Thread Nautiyal, Ankit K




On 4/17/2018 11:17 PM, Ville Syrjälä wrote:

On Tue, Apr 17, 2018 at 10:45:07AM +0530, Nautiyal, Ankit K wrote:

On 4/6/2018 11:14 PM, Ville Syrjälä wrote:

On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote:

This patch is causing failure of IGT test kms_3d. The kms_3d test
expects the no. of 3d modes to be 13.

(The test has hard-coded value for expected no. of 3d modes as 13)

But due to the addition of "matching aspect_ratio" in drm_mode_equal in
this patch, the total no. of

modes in the connector modelist is increased by 2, resulting in failure
of assertion 'mode_count==13'.

If kms_3d isn't setting the aspect ratio cap how is it affected by these
changes?

In drm_mode.c, the drm_mode_connector_list_update() uses drm_mode_equal,
to remove duplicate modes in connector_modes from the
connector->probe_modes.
Earlier, it wasn't matching for aspect-ratio and was discarding two of
the modes with aspect ratio,
as duplicates of other modes in the list.

Later, when we are pruning the modes in drm_mode_get_connector, the
logic there assumes,
that the modes are in a sorted order so that we just match with the last
valid mode for uniqueness.
This isn't the case with the spoofed edid in kms_3d.
Earlier, I was thinking if we should change the no. of expected modes in
kms_3d,
but that's not correct approach.

So finally, The pruning logic needs to be changed, to do away with any
assumption and check
all the modes in the list for duplicates. This however will take more
time to remove duplicates.

Any other suggestions on this?

What are all the modes this EDID gives us? The order in which the
modes are listed in the EDID should not be relevant as we sort
the mode list ourselves, and thus similar modes should appear back to
back on the list. So I don't really understand how we fail to
discard these two modes.


As we know the kms_3d test does not set the aspect ratio cap, the modes 
with aspect-ratios must be pruned,

unless they are unique.
Now the list of mode in kms_3d is something like this:
...
1280x720@60 flags 0x1c005 [3D mode - top and bottom] ->1
1280x720@60 flags 0x4005 [3D mode -frame packing] -> 2
1280x720@60 flags 0x1c005 [3D mode -top and bottom with aspect ratio]->3
1280x720@60 flags 0x4005 [3D mode frame packing with aspect ratio] ->4
...

The first two 3D modes are able to pass and are added in the 
drm_mode_get_connector mode list.
For third mode , the 3d mode with aspect ratio, must be pruned if not 
unique. So we check with the last valid mode in the list (1280x720@60 
flags 0x4005).

Everything matches except the 3d mode flags. So it gets added to the list.
For the fourth again, the mode with aspect ratio must be pruned, (not 
unique), but when we check with the last valid mode,

the 3d flags doesn't match, and we add that also in the list.

had the list been sorted by flag also:
1280x720@60 flags 0x1c005 [3D mode top and bottom]
1280x720@60 flags 0x1c005 [3D mode top and bottom with aspect ratio]
1280x720@60 flags 0x4005 [3D mode frame packing]
1280x720@60 flags 0x4005 [3D mode frame packing aspect ratio]

the aspect-ratio modes would have been pruned.

To avoid these scenarios, the best way is to modify the pruning logic to 
check against the whole list of already added modes.

I will send a patch for that shortly.

Regards,
Ankit




Regards
-Ankit


Perhaps this need to be handled in the test.

-Regards,

Ankit


On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote:

From: "Sharma, Shashank" 

Current DRM layer functions don't parse aspect ratio information
while converting a user mode->kernel mode or vice versa. This
causes modeset to pick mode with wrong aspect ratio, eventually
causing failures in HDMI compliance test cases, due to wrong VIC.

This patch adds aspect ratio information in DRM's mode conversion
and mode comparision functions, to make sure kernel picks mode
with right aspect ratio (as per the VIC).

Background:
This patch was once reviewed and merged, and later reverted due to
lack of DRM cap protection. This is a re-spin of this patch, this
time with DRM cap protection, to avoid aspect ratio information, when
the client doesn't request for it.

Review link: https://pw-emeril.freedesktop.org/patch/104068/
Background discussion: https://patchwork.kernel.org/patch/9379057/

Signed-off-by: Shashank Sharma 
Signed-off-by: Lin, Jia 
Signed-off-by: Akashdeep Sharma 
Reviewed-by: Jim Bride  (V2)
Reviewed-by: Jose Abreu  (V4)

Cc: Ville Syrjala 
Cc: Jim Bride 
Cc: Jose Abreu 
Cc: Ankit Nautiyal 

V3: modified the aspect-ratio check in drm_mode_equal as per new flags
   provided by Ville. https://patchwork.freedesktop.org/patch/188043/
V4: rebase
V5: rebase
V6: As recommended by Ville, avoided matching of aspect-ratio in
   drm_fb_helper, while trying to find a common mode among connectors
   for the target clone mode.
V7: rebase
V8: rebase
V9: rebase
V10: rebase
---
drivers/gpu/drm/drm_fb_helper.c | 12 ++--
drivers/gpu/drm/drm_modes.c | 35 +

Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver

2018-04-18 Thread Dongwon Kim

On Wed, Apr 18, 2018 at 03:42:29PM +0300, Oleksandr Andrushchenko wrote:
> On 04/18/2018 01:55 PM, Roger Pau Monné wrote:
> >On Wed, Apr 18, 2018 at 01:39:35PM +0300, Oleksandr Andrushchenko wrote:
> >>On 04/18/2018 01:18 PM, Paul Durrant wrote:
> -Original Message-
> From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
> Of Roger Pau Monné
> Sent: 18 April 2018 11:11
> To: Oleksandr Andrushchenko 
> Cc: jgr...@suse.com; Artem Mygaiev ;
> Dongwon Kim ; airl...@linux.ie;
> oleksandr_andrushche...@epam.com; linux-ker...@vger.kernel.org; dri-
> de...@lists.freedesktop.org; Potrola, MateuszX
> ; xen-de...@lists.xenproject.org;
> daniel.vet...@intel.com; boris.ostrov...@oracle.com; Matt Roper
> 
> Subject: Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy
> helper DRM driver
> 
> On Wed, Apr 18, 2018 at 11:01:12AM +0300, Oleksandr Andrushchenko
> wrote:
> >On 04/18/2018 10:35 AM, Roger Pau Monné wrote:
> After speaking with Oleksandr on IRC, I think the main usage of the
> gntdev extension is to:
> 
> 1. Create a dma-buf from a set of grant references.
> 2. Share dma-buf and get a list of grant references.
> 
> I think this set of operations could be broken into:
> 
> 1.1 Map grant references into user-space using the gntdev.
> 1.2 Create a dma-buf out of a set of user-space virtual addresses.
> 
> 2.1 Map a dma-buf into user-space.
> 2.2 Get grefs out of the user-space addresses where the dma-buf is
>   mapped.
> 
> So it seems like what's actually missing is a way to:
> 
>    - Create a dma-buf from a list of user-space virtual addresses.
>    - Allow to map a dma-buf into user-space, so it can then be used with
>  the gntdev.
> 
> I think this is generic enough that it could be implemented by a
> device not tied to Xen. AFAICT the hyper_dma guys also wanted
> something similar to this.
> >>Ok, so just to summarize, xen-zcopy/hyper-dmabuf as they are now,
> >>are no go from your POV?

FYI,

our use-case is "surface sharing" or "graphic obj sharing" where a client
application in one guest renders and export this render target(e.g. EGL surface)
as dma-buf. This dma-buf is then exported to another guest/host via hyper_dmabuf
drv where a compositor is running. This importing domain creates a dmabuf with
shared reference then it is imported as EGL image that later can be used as
texture object via EGL api. Mapping dmabuf to the userspace or vice versa
might be possible with modifying user space drivers/applications but it is an
unnecessary extra step from our perspective. Also, we want to keep all objects
in the kernel level.

> >My opinion is that there seems to be a more generic way to implement
> >this, and thus I would prefer that one.
> >
> >>Instead, we have to make all that fancy stuff
> >>with VAs <-> device-X and have that device-X driver live out of drivers/xen
> >>as it is not a Xen specific driver?
> >That would be my preference if feasible, simply because it can be
> >reused by other use-cases that need to create dma-bufs in user-space.
> There is a use-case I have: a display unit on my target has a DMA
> controller which can't do scatter-gather, e.g. it only expects a
> single starting address of the buffer.
> In order to create a dma-buf from grefs in this case
> I allocate memory with dma_alloc_xxx and then balloon pages of the
> buffer and finally map grefs onto this DMA buffer.
> This way I can give this shared buffer to the display unit as its bus
> addresses are contiguous.
> 
> With the proposed solution (gntdev + device-X) I won't be able to achieve
> this,
> as I have no control over from where gntdev/balloon drivers get the pages
> (even more, those can easily be out of DMA address space of the display
> unit).
> 
> Thus, even if implemented, I can't use this approach.
> >
> >In any case I just knew about dma-bufs this morning, there might be
> >things that I'm missing.
> >
> >Roger.
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Print unadorned pointers

2018-04-18 Thread Felix Kuehling

On 2018-04-18 05:24 AM, Alexey Brodkin wrote:
> After commit ad67b74 ("printk: hash addresses printed with %p")
> pointers are being hashed when printed. However, this makes
> debug output completely useless. Switch to %px in order to see the
> unadorned kernel pointers.
My understanding of the printk pointer hashing change was to force
people to think more carefully when they really need to print a kernel
pointer. When it is only used to identify an object, then a hash works
just fine. Most of the changes I see in amdgpu/amdkfd fall into this
category.

As I see it, changing all %p to %px by a script takes the thought
process out of it and subverts the intention of the original pointer
hashing change.

Regards,
  Felix


>
> This was done with the following one-liner:
>  find drivers/gpu/drm -type f -name "*.c" -exec sed -r -i 
> '/DRM_DEBUG|KERN_DEBUG|pr_debug/ s/%p\b/%px/g' {} +
>
> Signed-off-by: Alexey Brodkin 
> Cc: Borislav Petkov 
> Cc: Tobin C. Harding 
> Cc: Alex Deucher 
> Cc: Andrey Grodzovsky 
> Cc: Arnd Bergmann 
> Cc: Benjamin Gaignard 
> Cc: Chen-Yu Tsai 
> Cc: Christian Gmeiner 
> Cc: "Christian König" 
> Cc: Cihangir Akturk 
> Cc: CK Hu 
> Cc: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: David Airlie 
> Cc: "David (ChunMing) Zhou" 
> Cc: Gerd Hoffmann 
> Cc: Greg Kroah-Hartman 
> Cc: Gustavo Padovan 
> Cc: Harry Wentland 
> Cc: "Heiko Stübner" 
> Cc: Ingo Molnar 
> Cc: Jani Nikula 
> Cc: "Jerry (Fangzhi) Zuo" 
> Cc: Joonas Lahtinen 
> Cc: Krzysztof Kozlowski 
> Cc: "Leo (Sunpeng) Li" 
> Cc: Lucas Stach 
> Cc: Maarten Lankhorst 
> Cc: Matthias Brugger 
> Cc: Maxime Ripard 
> Cc: "Michel Dänzer" 
> Cc: Oded Gabbay 
> Cc: Philipp Zabel 
> Cc: Rob Clark 
> Cc: Rodrigo Vivi 
> Cc: Roger He 
> Cc: Roman Li 
> Cc: Russell King 
> Cc: Samuel Li 
> Cc: Sandy Huang 
> Cc: Sean Paul 
> Cc: Shirish S 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Cc: Tom Lendacky 
> Cc: Tony Cheng 
> Cc: Vincent Abriou 
> Cc: VMware Graphics 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: etna...@lists.freedesktop.org
> Cc: freedr...@lists.freedesktop.org
> Cc: amd-...@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org
> Cc: virtualizat...@lists.linux-foundation.org
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c   | 14 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c|  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c   |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c|  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c| 10 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c  |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_events.c|  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c  |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c   |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_queue.c | 18 -
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 14 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c|  2 +-
>  drivers/gpu/drm/armada/armada_gem.c| 12 +++---
>  drivers/gpu/drm/drm_atomic.c   | 44 
> +++---
>  drivers/gpu/drm/drm_bufs.c |  8 ++--
>  drivers/gpu/drm/drm_dp_mst_topology.c  |  4 +-
>  drivers/gpu/drm/drm_lease.c|  6 +--
>  drivers/gpu/drm/drm_lock.c |  2 +-
>  drivers/gpu/drm/drm_scatter.c  |  4 +-
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c  |  6 +--
>  drivers/gpu/drm/i810/i810_dma.c|  2 +-
>  drivers/gpu/drm/i915/i915_perf.c   |  2 +-
>  drivers/gpu/drm/i915/intel_display.c   |  2 +-
>  drivers/gpu/drm/i915/intel_guc_ct.c|  4 +-
>  drivers/gpu/drm/i915/intel_guc_submission.c|  2 +-
>  drivers/gpu/drm/i915/intel_uc_fw.c |  2 +-
>  drivers/gpu/drm/mediatek/mtk_drm_gem.c |  2 +-
>  drivers/gpu/drm/mga/mga_warp.c |  2 +-
>  drivers/gpu/drm/msm/msm_drv.c  |  4 +-
>  drivers/gpu/drm/qxl/qxl_cmd.c  |  4 +-
>  drivers/gpu/drm/qxl/qxl_fb.c   |  2 +-
>  drivers/gpu/drm/qxl/qxl_ttm.c  |  2 +-
>  drivers/gpu/drm/radeon/radeon_display.c|  2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c | 12 +++---
>  drivers/gpu/drm/radeon/radeon_object.c |  2 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c  |  2 +-
>  drivers/gpu/drm/savage/savage_bci.c|  2 +-
>  drivers/gpu/drm/sti/sti_gdp.c  |  4 +-
>  drivers/gpu/drm/sti/sti_mixer.c|  2 +-
>  drivers/gpu/drm/sun4i/sun4i_crtc.c |  4 +-
>  drivers/gpu/drm/ttm/ttm_page_alloc.c   |  2 +-
>  drivers/gpu/drm/udl/udl_fb.c

Re: [PATCH] drm: Print unadorned pointers

2018-04-18 Thread Greg Kroah-Hartman

On Wed, Apr 18, 2018 at 12:24:50PM +0300, Alexey Brodkin wrote:
> After commit ad67b74 ("printk: hash addresses printed with %p")
> pointers are being hashed when printed. However, this makes
> debug output completely useless. Switch to %px in order to see the
> unadorned kernel pointers.
> 
> This was done with the following one-liner:
>  find drivers/gpu/drm -type f -name "*.c" -exec sed -r -i 
> '/DRM_DEBUG|KERN_DEBUG|pr_debug/ s/%p\b/%px/g' {} +
> 
> Signed-off-by: Alexey Brodkin 
> Cc: Borislav Petkov 
> Cc: Tobin C. Harding 
> Cc: Alex Deucher 
> Cc: Andrey Grodzovsky 
> Cc: Arnd Bergmann 
> Cc: Benjamin Gaignard 
> Cc: Chen-Yu Tsai 
> Cc: Christian Gmeiner 
> Cc: "Christian König" 
> Cc: Cihangir Akturk 
> Cc: CK Hu 
> Cc: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: David Airlie 
> Cc: "David (ChunMing) Zhou" 
> Cc: Gerd Hoffmann 
> Cc: Greg Kroah-Hartman 
> Cc: Gustavo Padovan 
> Cc: Harry Wentland 
> Cc: "Heiko Stübner" 
> Cc: Ingo Molnar 
> Cc: Jani Nikula 
> Cc: "Jerry (Fangzhi) Zuo" 
> Cc: Joonas Lahtinen 
> Cc: Krzysztof Kozlowski 
> Cc: "Leo (Sunpeng) Li" 
> Cc: Lucas Stach 
> Cc: Maarten Lankhorst 
> Cc: Matthias Brugger 
> Cc: Maxime Ripard 
> Cc: "Michel Dänzer" 
> Cc: Oded Gabbay 
> Cc: Philipp Zabel 
> Cc: Rob Clark 
> Cc: Rodrigo Vivi 
> Cc: Roger He 
> Cc: Roman Li 
> Cc: Russell King 
> Cc: Samuel Li 
> Cc: Sandy Huang 
> Cc: Sean Paul 
> Cc: Shirish S 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Cc: Tom Lendacky 
> Cc: Tony Cheng 
> Cc: Vincent Abriou 
> Cc: VMware Graphics 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: etna...@lists.freedesktop.org
> Cc: freedr...@lists.freedesktop.org
> Cc: amd-...@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org
> Cc: virtualizat...@lists.linux-foundation.org
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c   | 14 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c|  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c   |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c|  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c| 10 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c  |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_events.c|  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c  |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c   |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_queue.c | 18 -
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 14 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c|  2 +-
>  drivers/gpu/drm/armada/armada_gem.c| 12 +++---
>  drivers/gpu/drm/drm_atomic.c   | 44 
> +++---
>  drivers/gpu/drm/drm_bufs.c |  8 ++--
>  drivers/gpu/drm/drm_dp_mst_topology.c  |  4 +-
>  drivers/gpu/drm/drm_lease.c|  6 +--
>  drivers/gpu/drm/drm_lock.c |  2 +-
>  drivers/gpu/drm/drm_scatter.c  |  4 +-
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c  |  6 +--
>  drivers/gpu/drm/i810/i810_dma.c|  2 +-
>  drivers/gpu/drm/i915/i915_perf.c   |  2 +-
>  drivers/gpu/drm/i915/intel_display.c   |  2 +-
>  drivers/gpu/drm/i915/intel_guc_ct.c|  4 +-
>  drivers/gpu/drm/i915/intel_guc_submission.c|  2 +-
>  drivers/gpu/drm/i915/intel_uc_fw.c |  2 +-
>  drivers/gpu/drm/mediatek/mtk_drm_gem.c |  2 +-
>  drivers/gpu/drm/mga/mga_warp.c |  2 +-
>  drivers/gpu/drm/msm/msm_drv.c  |  4 +-
>  drivers/gpu/drm/qxl/qxl_cmd.c  |  4 +-
>  drivers/gpu/drm/qxl/qxl_fb.c   |  2 +-
>  drivers/gpu/drm/qxl/qxl_ttm.c  |  2 +-
>  drivers/gpu/drm/radeon/radeon_display.c|  2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c | 12 +++---
>  drivers/gpu/drm/radeon/radeon_object.c |  2 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c  |  2 +-
>  drivers/gpu/drm/savage/savage_bci.c|  2 +-
>  drivers/gpu/drm/sti/sti_gdp.c  |  4 +-
>  drivers/gpu/drm/sti/sti_mixer.c|  2 +-
>  drivers/gpu/drm/sun4i/sun4i_crtc.c |  4 +-
>  drivers/gpu/drm/ttm/ttm_page_alloc.c   |  2 +-
>  drivers/gpu/drm/udl/udl_fb.c   |  2 +-
>  drivers/gpu/drm/via/via_dma.c  |  4 +-
>  drivers/gpu/drm/via/via_irq.c  |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c|  2 +-
>  45 files changed, 120 insertions(+), 120 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 1d6e1479da38..32e85fe83152 100644
> --- a/drivers

Re: mi0283qt driver issues

2018-04-18 Thread Tom Callaway

On 04/18/2018 06:22 AM, Noralf Trønnes wrote:
> It turns out that the reason for this is that the pitft has a hw reset
> circuit that resets only on power-on and not on each reboot through a
> gpio like rpi-display does.
> 
> When the driver is enabling the display pipeline it checks if the
> controller is already initialized, and skips setup if it is. This means
> that the rotation value is only applied after power-on on the pitft and
> not on each reboot.
> 
> Turning off the "display" before rebooting should also work:
> 
> $ # change rotation value
> $ echo "28" | sudo tee /sys/kernel/debug/dri/0/command
> $ sudo reboot
> 
> Rotation/mirror/flip/bgr is controlled by commmand 0x36. The current
> value is returned by command 0x0b:
> 
> $ sudo cat /sys/kernel/debug/dri/0/command | grep "0b:"
> 0b: f8
> 
> So to fix this in the driver the rotation value has to always be applied
> regardless of display "on" state. This can be done by moving the rotation
> code in mi0283qt_enable() down after the out: label.

Confirmed, turning off the display before rebooting works, thanks.

To fix this correctly, it seems like there are two paths:

* If it is always safe to apply the rotation value regardless of display
state, move the rotation code down after out: in the mi0283qt driver.

* If this fix is only safe on the pitft (and not on the other panels
driven by the mi0283qt driver), either conditionalizing this behavior
somehow, or making a separate ili9340 driver with this change.

I'm willing to make patches if necessary here, but it is not clear which
path is the correct one.

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

Re: [PATCH hwc v2 17/18] drm_hwcomposer: Flatten scene synchronously

On Wed, Apr 18, 2018 at 12:14:03PM +0100, Alexandru-Cosmin Gheorghe wrote:
> On Tue, Apr 17, 2018 at 01:47:46PM -0400, Sean Paul wrote:
> > On Wed, Apr 11, 2018 at 04:22:28PM +0100, Alexandru Gheorghe wrote:
> > > Flatten scene on the same CRTC as the one driving the display.
> > > The active composition is played back to the display with a buffer
> > > attached to the writeback connector.
> > > Then we build a composition that has only one plane enabled and that
> > > uses the result of the writeback as the input.
> > > 
> > > Signed-off-by: Alexandru Gheorghe 
> > > ---
> > >  drmdisplaycompositor.cpp | 203 
> > > +--
> > >  drmdisplaycompositor.h   |   7 +-
> > >  2 files changed, 204 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> > > index e535e8a..cb670e6 100644
> > > --- a/drmdisplaycompositor.cpp
> > > +++ b/drmdisplaycompositor.cpp
> > > @@ -36,6 +36,7 @@
> > >  #include "drmplane.h"
> > >  #include "drmresources.h"
> > >  #include "glworker.h"
> > > +static const uint32_t kWaitWritebackFence = 100;  // ms
> > >  
> > >  namespace android {
> > >  
> > > @@ -523,7 +524,9 @@ int 
> > > DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) {
> > >  }
> > >  
> > >  int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition 
> > > *display_comp,
> > > -  bool test_only) {
> > > +  bool test_only,
> > > +  DrmDisplayComposition 
> > > *writeback_comp,
> > > +  DrmConnector *writeback_conn) {
> > >ATRACE_CALL();
> > >  
> > >int ret = 0;
> > > @@ -532,6 +535,7 @@ int 
> > > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> > >std::vector &comp_planes =
> > >display_comp->composition_planes();
> > >uint64_t out_fences[drm_->crtcs().size()];
> > > +  int writeback_fence = -1;
> > >  
> > >DrmConnector *connector = drm_->GetConnectorForDisplay(display_);
> > >if (!connector) {
> > > @@ -550,9 +554,37 @@ int 
> > > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> > >  return -ENOMEM;
> > >}
> > >  
> > > +  if (writeback_comp != NULL) {
> > > +if (writeback_conn == NULL)
> > > +  return -EINVAL;
> > > +if (writeback_conn->writeback_fb_id().id() == 0 ||
> > > +writeback_conn->writeback_out_fence().id() == 0) {
> > > +  ALOGE("Writeback properties don't exit");
> > > +  return -EINVAL;
> > > +}
> > > +if (writeback_comp->layers().size() != 1) {
> > > +  ALOGE("Invalid number of layers for writeback composition");
> > > +  return -EINVAL;
> > > +}
> > > +ret = drmModeAtomicAddProperty(
> > > +pset, writeback_conn->id(), 
> > > writeback_conn->writeback_fb_id().id(),
> > > +writeback_comp->layers().back().buffer->fb_id);
> > > +if (ret < 0) {
> > > +  ALOGE("Failed to add writeback_fb_id");
> > > +  return ret;
> > > +}
> > > +ret = drmModeAtomicAddProperty(pset, writeback_conn->id(),
> > > +   
> > > writeback_conn->writeback_out_fence().id(),
> > > +   (uint64_t)&writeback_fence);
> > 
> > Upcasting int to u64 isn't a great idea, please go the other way (as we do 
> > with
> > out_fences).
> 
> Genuinely curious about this, why does it make a difference I'm
> upcasting the address not the int itself.

Right, for some reason I had inserted a * inside the parens with my head. Please
ignore :)

> More, the kernel does this s32 __user *fence_ptr = u64_to_user_ptr(val);
> To be completly fair it should have been int32_t.
> 
> 
> > 
> > > +if (ret < 0) {
> > > +  ALOGE("Failed to add writeback_out_fence");
> > > +  return ret;
> > > +}
> > > +  }
> > 
> > This would be more readable if it was split off into a function.
> > 
> > >if (crtc->out_fence_ptr_property().id() != 0) {
> > > -ret = drmModeAtomicAddProperty(pset, crtc->id(), 
> > > crtc->out_fence_ptr_property().id(),
> > > -   (uint64_t) &out_fences[crtc->pipe()]);
> > > +ret = drmModeAtomicAddProperty(pset, crtc->id(),
> > > +   crtc->out_fence_ptr_property().id(),
> > > +   (uint64_t)&out_fences[crtc->pipe()]);
> > >  if (ret < 0) {
> > >ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret);
> > >drmModeAtomicFree(pset);
> > > @@ -580,6 +612,15 @@ int 
> > > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> > >  }
> > >}
> > >  
> > > +  if (writeback_conn != NULL) {
> > > +ret = drmModeAtomicAddProperty(pset, writeback_conn->id(),
> > > +   
> > > writeback_conn->crtc_id_property().id(),
> > > +   crtc->id())

[PATCH] drm: omapdrm: silence unititialized variable warning

2018-04-18 Thread Dan Carpenter

Smatch complains that "area_free" could be used without being
initialized.  This code is several years old and premusably works fine
so this can't be a very serious bug.  But it's easy enough to silence
the warning.  If "area_free" is false at the end of the function then
we return -ENOMEM.

Signed-off-by: Dan Carpenter 

diff --git a/drivers/gpu/drm/omapdrm/tcm-sita.c 
b/drivers/gpu/drm/omapdrm/tcm-sita.c
index d7f7bc9f061a..817be3c41863 100644
--- a/drivers/gpu/drm/omapdrm/tcm-sita.c
+++ b/drivers/gpu/drm/omapdrm/tcm-sita.c
@@ -90,7 +90,7 @@ static int l2r_t2b(u16 w, u16 h, u16 a, s16 offset,
 {
int i;
unsigned long index;
-   bool area_free;
+   bool area_free = false;
unsigned long slots_per_band = PAGE_SIZE / slot_bytes;
unsigned long bit_offset = (offset > 0) ? offset / slot_bytes : 0;
unsigned long curr_bit = bit_offset;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [[RFC]DPU PATCH 0/4] Add suppport for sn65dsi86 bridge chip and Innolux 2k edp panel driver

On Wed, Apr 18, 2018 at 05:49:58PM +0530, Sandeep Panda wrote:
> Changelog:
> 
> v3 -> v4:

I didn't really bother to do a thorough review given that there are obvious
mistakes and ignored feedback, it's a waste of time, tbh. Please go through
the previous reviews and resend a more polished version.

Sean

> Current patchset separates out eDP panel specific resources from sn65dsi86
> bridge driver and creates a separate driver for the innloux edp panel.
> 
> Now bridge driver check if any panel is attached or not to get the supported
> modes. If any panel is attached then query from panel driver to get the modes,
> or else probe the provided i2c adapter to read the modes from EDID.
> 
> Removed hardcoding of bridge init sequence. With this patchset bridge driver 
> now
> will program the init sequence based on the current mode set by drm framework.
> 
> Current patchset is not tested on actual bridge chip/panel.
> 
> Sandeep Panda (4):
>   drm/bridge: add support for sn65dsi86 bridge driver
>   dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings
>   drm/panel: add Innolux TV123WAM eDP panel driver
>   dt-bindings: Add Innolux TV123WAM panel bindings
> 
>  .../bindings/display/bridge/ti,sn65dsi86.txt   |  60 ++
>  .../display/panel/innolux,edp-2k-panel.txt |  21 +
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c  | 742 
> +
>  drivers/gpu/drm/panel/panel-innolux-tv123wam.c | 299 +
>  4 files changed, 1122 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
>  create mode 100644 
> Documentation/devicetree/bindings/display/panel/innolux,edp-2k-panel.txt
>  create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
>  create mode 100644 drivers/gpu/drm/panel/panel-innolux-tv123wam.c
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
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: [[RFC]DPU PATCH 1/4] drm/bridge: add support for sn65dsi86 bridge driver

On Wed, Apr 18, 2018 at 05:49:59PM +0530, Sandeep Panda wrote:
> Add support for TI's sn65dsi86 dsi2edp bridge chip.
> The chip converts DSI transmitted signal to eDP signal,
> which is fed to the connected eDP panel.
> 
> This chip can be controlled via either i2c interface or
> dsi interface. Currently in driver all the control registers
> are being accessed through i2c interface only.
> Also as of now HPD support has not been added to bridge
> chip driver.
> 
> Signed-off-by: Sandeep Panda 
> ---
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c | 742 
> ++
>  1 file changed, 742 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/ti-sn65dsi86.c
> 
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c 
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> new file mode 100644
> index 000..c798f2f
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -0,0 +1,742 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 

I have a hard time believing that you need all of these includes. Off the top of
my head, you could probably remove types, kernel, init, platform_device, delay,
drmP, drm_atomic, drm_crtc_helper. You could probably trim it even further with
the help of your compiler. These should also be in alphabetical order.

> +
> +#define SN_BRIDGE_REVISION_ID 0x2
> +
> +/* Link Training specific registers */
> +#define SN_DEVICE_REV_REG0x08
> +#define SN_REFCLK_FREQ_REG   0x0A
> +#define SN_DSI_LANES_REG 0x10
> +#define SN_DSIA_CLK_FREQ_REG 0x12
> +#define SN_ENH_FRAME_REG 0x5A
> +#define SN_SSC_CONFIG_REG0x93
> +#define SN_DATARATE_CONFIG_REG   0x94
> +#define SN_PLL_ENABLE_REG0x0D
> +#define SN_SCRAMBLE_CONFIG_REG   0x95
> +#define SN_AUX_WDATA0_REG0x64
> +#define SN_AUX_ADDR_19_16_REG0x74
> +#define SN_AUX_ADDR_15_8_REG 0x75
> +#define SN_AUX_ADDR_7_0_REG  0x76
> +#define SN_AUX_LENGTH_REG0x77
> +#define SN_AUX_CMD_REG   0x78
> +#define SN_ML_TX_MODE_REG0x96
> +#define SN_PAGE_SELECT_REG   0xFF
> +#define SN_AUX_RDATA0_REG0x79
> +/* video config specific registers */
> +#define SN_CHA_ACTIVE_LINE_LENGTH_LOW_REG0x20
> +#define SN_CHA_ACTIVE_LINE_LENGTH_HIGH_REG   0x21
> +#define SN_CHA_VERTICAL_DISPLAY_SIZE_LOW_REG 0x24
> +#define SN_CHA_VERTICAL_DISPLAY_SIZE_HIGH_REG0x25
> +#define SN_CHA_HSYNC_PULSE_WIDTH_LOW_REG 0x2C
> +#define SN_CHA_HSYNC_PULSE_WIDTH_HIGH_REG0x2D
> +#define SN_CHA_VSYNC_PULSE_WIDTH_LOW_REG 0x30
> +#define SN_CHA_VSYNC_PULSE_WIDTH_HIGH_REG0x31
> +#define SN_CHA_HORIZONTAL_BACK_PORCH_REG 0x34
> +#define SN_CHA_VERTICAL_BACK_PORCH_REG   0x36
> +#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG0x38
> +#define SN_CHA_VERTICAL_FRONT_PORCH_REG  0x3A
> +#define SN_DATA_FORMAT_REG   0x5B
> +#define SN_COLOR_BAR_CONFIG_REG  0x3C
> +
> +#define DPPLL_CLK_SRC_REFCLK 0
> +#define DPPLL_CLK_SRC_DSICLK 1
> +#define MIN_DSI_CLK_FREQ_MHZ 40
> +
> +enum ti_sn_bridge_ref_clks {
> + SN_REF_CLK_12_MHZ = 0,
> + SN_REF_CLK_19_2_MHZ,
> + SN_REF_CLK_26_MHZ,
> + SN_REF_CLK_27_MHZ,
> + SN_REF_CLK_38_4_MHZ,
> + SN_REF_CLK_MAX,
> +};
> +
> +struct ti_sn_bridge {
> + struct device *dev;
> + struct drm_bridge bridge;
> + struct drm_connector connector;
> + struct device_node *host_node;
> + struct mipi_dsi_device *dsi;
> + /* handle to the connected eDP panel */
> + struct drm_panel *panel;
> + int irq;
> + struct gpio_desc *irq_gpio;
> + /* list of gpios needed to enable the bridge functionality */
> + struct gpio_descs *enable_gpios;

Why are the enable gpios a list?

> + unsigned int num_supplies;
> + struct regulator_bulk_data *supplies;
> + struct i2c_client *i2c_client;
> + struct i2c_adapter *ddc;
> + bool power_on;
> + u32 num_modes;
> + struct drm_display_mode curr_mode;
> +};
> +
> +static int ti_sn_bridge_write(struct ti_sn_bridge *pdata, u8 reg, u8 val)
> +{
> + struct i2c_client *client = pdata->i2c_client;
> + u8 buf[2] = {reg, val};
> + struct i2c_msg msg = {
> + .addr = client->addr,
> + .flags = 0,
> + .len = 2,
> + .buf = buf,
> + };
> +
> + if (i2c_transfer(client->adapter, &msg, 1) < 1) {
> + DRM_ERROR("i2c write failed\n");
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static int ti_sn_bridge_read(st

Re: [[RFC]DPU PATCH 2/4] dt-bindings: drm/bridge: Document sn65dsi86 bridge bindings

On Wed, Apr 18, 2018 at 05:50:00PM +0530, Sandeep Panda wrote:
> Document the bindings used for the sn65dsi86 DSI to eDP bridge.
> 

Please add a changelog to your patches so reviewers know what has changed
between patch versions. 

> Signed-off-by: Sandeep Panda 
> ---
>  .../bindings/display/bridge/ti,sn65dsi86.txt   | 60 
> ++
>  1 file changed, 60 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt 
> b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> new file mode 100644
> index 000..9e2612e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/ti,sn65dsi86.txt
> @@ -0,0 +1,60 @@
> +SN65DSI86 DSI to eDP bridge chip
> +
> +
> +This is the binding for Texas Instruments SN65DSI86 bridge.
> +
> +Required properties:
> +- compatible: Must be "ti,sn65dsi86"
> +- reg: i2c address of the chip, 0x2d as per datasheet
> +- enable-gpios: OF device-tree gpio specifications for bridge_en pin

The datasheet only has one enable pin, why gpios?

> +
> +- vccio-supply: A 1.8V supply that powers up the digital IOs.
> +- vcca-supply: A 1.2V supply that powers up the analog circuits.
> +
> +Optional properties:
> +
> +- irq-gpios: OF device-tree gpio specification for interrupt pin

From the last review, Rob said, 'Use "interrupts" property instead.'

You didn't provide responses to prior review comments, so it's unclear whether
you've intentionally or unintentionally ignored him :)

Sean

> +
> +Required nodes:
> +
> +This device has two video ports. Their connections are modelled using the
> +OF graph bindings specified in Documentation/devicetree/bindings/graph.txt.
> +
> +- Video port 0 for DSI input
> +- Video port 1 for eDP output
> +
> +Example
> +---
> +
> +edp-bridge@2d {
> + compatible = "ti,sn65dsi86";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x2d>;
> +
> + enable-gpios = <&msmgpio 33 GPIO_ACTIVE_HIGH>;
> +
> + vccio-supply = <&pm8916_l17>;
> + vcca-supply = <&pm8916_l6>;
> +
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + port@0 {
> + reg = <0>;
> +
> + edp_bridge_in: endpoint {
> + remote-endpoint = <&dsi_out>;
> + };
> + };
> +
> + port@1 {
> + reg = <1>;
> +
> + edp_bridge_out: endpoint {
> + remote-endpoint = <&edp_panel_in>;
> + };
> + };
> + };
> +}
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

-- 
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: [Intel-gfx] [PATCH] drm/i915: Fix LSPCON TMDS output buffer enabling from low-power state

2018-04-18 Thread Jani Nikula

On Mon, 16 Apr 2018, Clint Taylor  wrote:
> On 04/16/2018 08:53 AM, Imre Deak wrote:
>> LSPCON adapters in low-power state may ignore the first I2C write during
>> TMDS output buffer enabling, resulting in a blank screen even with an
>> otherwise enabled pipe. Fix this by reading back and validating the
>> written value a few times.
>>
>> The problem was noticed on GLK machines with an onboard LSPCON adapter
>> after entering/exiting DC5 power state. Doing an I2C read of the adapter
>> ID as the first transaction - instead of the I2C write to enable the
>> TMDS buffers - returns the correct value. Based on this we assume that
>> the transaction itself is sent properly, it's only the adapter that is
>> not ready for some reason to accept this first write after waking from
>> low-power state. In my case the second I2C write attempt always
>> succeeded.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105854
>> Cc: Clinton Taylor 
>> Cc: Ville Syrjälä 
>> Signed-off-by: Imre Deak 
>> ---
>>   drivers/gpu/drm/drm_dp_dual_mode_helper.c | 39 
>> +--
>>   1 file changed, 32 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_dual_mode_helper.c 
>> b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> index 02a50929af67..e7f4fe2848a5 100644
>> --- a/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_dual_mode_helper.c
>> @@ -350,19 +350,44 @@ int drm_dp_dual_mode_set_tmds_output(enum 
>> drm_dp_dual_mode_type type,
>>   {
>>  uint8_t tmds_oen = enable ? 0 : DP_DUAL_MODE_TMDS_DISABLE;
>>  ssize_t ret;
>> +int retry;
>>   
>>  if (type < DRM_DP_DUAL_MODE_TYPE2_DVI)
>>  return 0;
>>   
>> -ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
>> - &tmds_oen, sizeof(tmds_oen));
>> -if (ret) {
>> -DRM_DEBUG_KMS("Failed to %s TMDS output buffers\n",
>> -  enable ? "enable" : "disable");
>> -return ret;
>> +/*
>> + * LSPCON adapters in low-power state may ignore the first write, so
>> + * read back and verify the written value a few times.
>> + */
>> +for (retry = 0; retry < 3; retry++) {
>> +uint8_t tmp;
>> +
>> +ret = drm_dp_dual_mode_write(adapter, DP_DUAL_MODE_TMDS_OEN,
>> + &tmds_oen, sizeof(tmds_oen));
>> +if (ret) {
>> +DRM_DEBUG_KMS("Failed to %s TMDS output buffers (%d 
>> attempts)\n",
>> +  enable ? "enable" : "disable",
>> +  retry + 1);
>> +return ret;
>> +}
>> +
>> +ret = drm_dp_dual_mode_read(adapter, DP_DUAL_MODE_TMDS_OEN,
>> +&tmp, sizeof(tmp));
>> +if (ret) {
>> +DRM_DEBUG_KMS("I2C read failed during TMDS output 
>> buffer %s (%d attempts)\n",
>> +  enable ? "enabling" : "disabling",
>> +  retry + 1);
>> +return ret;
>> +}
>> +
>> +if (tmp == tmds_oen)
>> +return 0;
>>  }
>>   
>> -return 0;
>> +DRM_DEBUG_KMS("I2C write value mismatch during TMDS output buffer %s\n",
>> +  enable ? "enabling" : "disabling");
>> +
>> +return -EIO;
>>   }
>>   EXPORT_SYMBOL(drm_dp_dual_mode_set_tmds_output);
>>   
>
> Appears to fix the issue seen on GLK with LSPCON and DMC firmware 
> loaded. Customer was concerned about the fix being in DRM instead of 
> i915. However, there are no other SOCs that use this DRM function.
>
> Reviewed-by: Clint Taylor 
> Tested-by: Clint Taylor 

Pushed to drm-misc-fixes, thanks for the patch.

Clint and Ville, many thanks for your testing and review, alas I screwed
up and pushed this without the tags added to the commit. I'm sorry. I
expect better from myself. :(

BR,
Jani.


>
> -Clint
>
>
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Freedreno] [DPU PATCH v2 2/2] drm/panel: add backlight control support for truly panel

On Wed, Apr 18, 2018 at 11:52:18AM +0100, Daniel Thompson wrote:
> On Tue, Apr 17, 2018 at 05:42:04PM -0700, abhin...@codeaurora.org wrote:
> > Adding another point.
> > 
> > On 2018-04-17 17:04, abhin...@codeaurora.org wrote:
> > > Hi Bjorn
> > > 
> > > Apologies if the prev reply wasnt clear.
> > > 
> > > Hope this one is.
> > > 
> > > reply inline.
> > > 
> > > On 2018-04-17 14:29, Bjorn Andersson wrote:
> > > > On Tue 17 Apr 11:21 PDT 2018, abhin...@codeaurora.org wrote:
> > > > > On 2018-04-16 23:13, Bjorn Andersson wrote:
> > > > [..]
> > > > > > If the panel isn't actually a piece of backlight hardware then it 
> > > > > > should
> > > > > > not register a backlight device. Why do you need your own sysfs?
> > > > > >
> > > > > > Regards,
> > > > > > Bjorn
> > > > > [Abhinav] This particular panel isnt a piece of backlight hardware.
> > > > > But, we want to have our own sysfs to give flexibility to our
> > > > > userspace
> > > > > to write and read stuff for its proprietary uses.
> > > > 
> > > > Please be more specific in your replies, no one will accept code that
> > > > "does stuff" and expecting a reviewer to spend time guessing or
> > > > pulling
> > > > the information out of you is a sure way to get your patches ignored.
> > > > 
> > > > Exactly what kind of stuff are you talking about here and exactly
> > > > which
> > > > problem are you solving.
> > > > 
> > > > > Thats how our downstream has been working so far and hence to
> > > > > maintain
> > > > > the compatibility would like to have it.
> > > > 
> > > > Make your proprietary code work with the upstream kernel and you
> > > > shouldn't ever have to modify it.
> > > > 
> > > > Regards,
> > > > Bjorn
> > > 
> > > [Abhinav] We have a few userspace clients today for the backlight sysfs
> > > node
> > > which read/write directly to
> > > "/sys/class/backlight/panel0-backlight/brightness"
> > > When I said "stuff" I was only referring to the brightness value.
> > > This separate sysfs node allows us to validate those userspace features
> > > of ours
> > > which directly edit the backlight value on our reference platform.
> > > Since our reference platform uses this panel,hence having our own
> > > sysfs alias helps.
> > > Otherwise, whenever we try to use this panel with upstream code we
> > > will have to end up
> > > changing all those places in our userspace/framework to change the sysfs
> > > path.
> > > Hence we wanted to preserve that sysfs node name.
> > > The wled device is not created under /sys/class/backlight but under
> > > /sys/class/leds/wled.
> > > So we will have to change the name of this node across all our
> > > userspace.
> > > 
> > > If this isnt a convincing reason enough to have its own sysfs node
> > > path, I will use
> > > your approach.
> > > 
> > > Let me know your comments or if this is still not clear.
> > > 
> > [Abhinav] We also might not be using the brightness value "as-it-is".
> > 
> > We will potentially scale it up/down based on some requirements.
> > 
> > If we do not have our own sysfs alias for this, how do we account for
> > providing this interface for our chipset specific backlight dependent
> > feature.
> > 
> > Can you please comment on this?
> 
> Not easily. It's rather unclear what this chipset specific backlight
> dependent feature you have alluded to is so how can we suggest how to
> control or model it in the upstream kernel?
> 

The code is here:

https://gitlab.freedesktop.org/seanpaul/dpu-staging/blob/mtp-squashed/drivers/gpu/drm/msm/dsi-staging/dsi_display.c#L76

AFAICT, there's nothing fancy in the kernel aside from scaling the brightness
level down twice. I assume the magic is in userspace. My initial reaction was
that the scaling factor should just be applied in userspace. Especially since
the scaling factor reduces the resolution of the backlight, and that's not
immediately obvious by looking at "brightness".

Sean


> I can make a guess that is might be to do with brightness curves... but
> I'd really prefer not to have to guess.
> 
> There are some problems with the current interface because it is not
> well defined with the brightness control is linear or
> logarithmic/perceptual (patches welcome) but for other common embedded
> backlights (pwm_bl particularly) we expect calibration of the
> brightness curve to be a job for the device tree (because it is a
> property of the hardware it can be described in the DT) and there are
> patches pending to improve this.
> 
> 
> Daniel.
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
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: [Freedreno] [DPU PATCH v2 2/2] drm/panel: add backlight control support for truly panel

On Tue, Apr 17, 2018 at 05:04:56PM -0700, abhin...@codeaurora.org wrote:
> Hi Bjorn
> 
> Apologies if the prev reply wasnt clear.
> 
> Hope this one is.
> 
> reply inline.
> 
> On 2018-04-17 14:29, Bjorn Andersson wrote:
> > On Tue 17 Apr 11:21 PDT 2018, abhin...@codeaurora.org wrote:
> > > On 2018-04-16 23:13, Bjorn Andersson wrote:
> > [..]
> > > > If the panel isn't actually a piece of backlight hardware then it should
> > > > not register a backlight device. Why do you need your own sysfs?
> > > >
> > > > Regards,
> > > > Bjorn
> > > [Abhinav] This particular panel isnt a piece of backlight hardware.
> > > But, we want to have our own sysfs to give flexibility to our
> > > userspace
> > > to write and read stuff for its proprietary uses.
> > 
> > Please be more specific in your replies, no one will accept code that
> > "does stuff" and expecting a reviewer to spend time guessing or pulling
> > the information out of you is a sure way to get your patches ignored.
> > 
> > Exactly what kind of stuff are you talking about here and exactly which
> > problem are you solving.
> > 
> > > Thats how our downstream has been working so far and hence to maintain
> > > the compatibility would like to have it.
> > 
> > Make your proprietary code work with the upstream kernel and you
> > shouldn't ever have to modify it.
> > 
> > Regards,
> > Bjorn
> 
> [Abhinav] We have a few userspace clients today for the backlight sysfs node
> which read/write directly to
> "/sys/class/backlight/panel0-backlight/brightness"
> When I said "stuff" I was only referring to the brightness value.
> This separate sysfs node allows us to validate those userspace features of
> ours
> which directly edit the backlight value on our reference platform.
> Since our reference platform uses this panel,hence having our own sysfs
> alias helps.
> Otherwise, whenever we try to use this panel with upstream code we will have
> to end up
> changing all those places in our userspace/framework to change the sysfs
> path.
> Hence we wanted to preserve that sysfs node name.
> The wled device is not created under /sys/class/backlight but under
> /sys/class/leds/wled.
> So we will have to change the name of this node across all our userspace.

As I mentioned on the previous review, coding around closed source userspace
isn't something that we want to do. It's hard enough to accommodate open
source u/s as it is.

Sean

> 
> If this isnt a convincing reason enough to have its own sysfs node path, I
> will use
> your approach.
> 
> Let me know your comments or if this is still not clear.
> 
> > ___
> > Freedreno mailing list
> > freedr...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/freedreno
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
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: mi0283qt driver issues

2018-04-18 Thread Emil Velikov

On 18 April 2018 at 11:22, Noralf Trønnes  wrote:

>> os.putenv('SDL_VIDEODRIVER', 'fbcon')
>> os.putenv('SDL_FBDEV'  , '/dev/fb1')
>> pygame.init()
>> list = pygame.display.list_modes()
>> print "List of modes: %s" % (list)
>>
>> It shows an empty set ("List of modes: []").
>
>
> I couldn't find any source code for the sdl fbcon driver, so I don't know
> how it queries for modes.
>
The fbcon driver is an SDL1 thing. IMHO a wide move is to switch to
SDL2 and its KMSDRM video backend.

Disclaimer: I haven't tried how well the backend behaves.

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

Re: [PATCH 1/2 v2] drm/pl111: Support the Versatile Express

2018-04-18 Thread Liviu Dudau

On Wed, Apr 18, 2018 at 12:50:10PM +0100, Robin Murphy wrote:
> On 17/04/18 20:31, Linus Walleij wrote:
> > On Tue, Apr 17, 2018 at 3:12 PM, Robin Murphy  wrote:
> > > On 17/04/18 13:32, Linus Walleij wrote:
> > > [...]
> > > > 
> > > > Unfortunately there is just one single vexpress core tile in the
> > > > upstream kernel that define a CLCD controller, the CA9 (4xA9)
> > > > that I am using. All the others just use the MB CLCD.
> > > > 
> > > > I am thinking there is some never finished DTS upstreaming
> > > > here that ought to happen so we use the core tile CLCD on some
> > > > other boards as well.
> > > 
> > > 
> > > Barring custom FPGA images, I think V2P-CA9 *is* the only VExpress tile
> > > implementing PL11x; all the more recent test chips had HDLCD instead.
> > 
> > Yup it looks like that.
> > 
> > I am restructuring the code to look for any graphics on the
> > core tile and only turn on the motherboard CLCD if there is
> > no CLCD or HDLCD on the core tile, e.g. if that device node has
> > been set to "disabled" or the HDLCD driver is not compiled in
> > but the CLCD driver is.
> > 
> > I found some vague mentions of a LCD display that can be
> > connected to the motherboard, so that would then be sitting
> > on the CLCD, is this something you ARM guys have seen
> > around? In that case I guess the VExpress (etc) would actually
> > be able to have two screens, one LCD panel on the motherboard
> > CLCD and a DVI from the core tile.
> 
> I don't see anything in the V2M motherboard manual to suggest the IOFPGA
> CLCD output is routed anywhere other than the DVI mux, and there are
> certainly no unaccounted-for connectors on the VExpress version currently in
> front of me (HBI-0190D). The only things I'm immediately aware of having a
> panel connector directly on the board are Integrator/CP and RealView EB.

Yes, I think the last platform that could have had an LCD display
attached directly was the Versatile platform, not the VExpress one.

Regarding the mux and trying to figure out a clever way of deciding
which one should have priority: I would not bother adding that to the
CLCD driver, instead I would expect the bootloader to set the MUX
correctly. Long time ago, before Juno was a thing and the only platform
HDLCD was running on was VExpress, I did have a patch to use
vexpress_config_write() (API before vexpress-config got converted to
regmap) to set the MUX. Then Juno happened and I've realised it was not
worth baking into the driver the existence of the MUX, as it was a quirk
of the board.

Best regards,
Liviu

> 
> Robin.

-- 

| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---
¯\_(ツ)_/¯
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver


On 04/18/2018 01:55 PM, Roger Pau Monné wrote:

On Wed, Apr 18, 2018 at 01:39:35PM +0300, Oleksandr Andrushchenko wrote:

On 04/18/2018 01:18 PM, Paul Durrant wrote:

-Original Message-
From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
Of Roger Pau Monné
Sent: 18 April 2018 11:11
To: Oleksandr Andrushchenko 
Cc: jgr...@suse.com; Artem Mygaiev ;
Dongwon Kim ; airl...@linux.ie;
oleksandr_andrushche...@epam.com; linux-ker...@vger.kernel.org; dri-
de...@lists.freedesktop.org; Potrola, MateuszX
; xen-de...@lists.xenproject.org;
daniel.vet...@intel.com; boris.ostrov...@oracle.com; Matt Roper

Subject: Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy
helper DRM driver

On Wed, Apr 18, 2018 at 11:01:12AM +0300, Oleksandr Andrushchenko
wrote:

On 04/18/2018 10:35 AM, Roger Pau Monné wrote:

After speaking with Oleksandr on IRC, I think the main usage of the
gntdev extension is to:

1. Create a dma-buf from a set of grant references.
2. Share dma-buf and get a list of grant references.

I think this set of operations could be broken into:

1.1 Map grant references into user-space using the gntdev.
1.2 Create a dma-buf out of a set of user-space virtual addresses.

2.1 Map a dma-buf into user-space.
2.2 Get grefs out of the user-space addresses where the dma-buf is
  mapped.

So it seems like what's actually missing is a way to:

   - Create a dma-buf from a list of user-space virtual addresses.
   - Allow to map a dma-buf into user-space, so it can then be used with
 the gntdev.

I think this is generic enough that it could be implemented by a
device not tied to Xen. AFAICT the hyper_dma guys also wanted
something similar to this.

Ok, so just to summarize, xen-zcopy/hyper-dmabuf as they are now,
are no go from your POV?

My opinion is that there seems to be a more generic way to implement
this, and thus I would prefer that one.


Instead, we have to make all that fancy stuff
with VAs <-> device-X and have that device-X driver live out of drivers/xen
as it is not a Xen specific driver?

That would be my preference if feasible, simply because it can be
reused by other use-cases that need to create dma-bufs in user-space.

There is a use-case I have: a display unit on my target has a DMA
controller which can't do scatter-gather, e.g. it only expects a
single starting address of the buffer.
In order to create a dma-buf from grefs in this case
I allocate memory with dma_alloc_xxx and then balloon pages of the
buffer and finally map grefs onto this DMA buffer.
This way I can give this shared buffer to the display unit as its bus
addresses are contiguous.

With the proposed solution (gntdev + device-X) I won't be able to 
achieve this,

as I have no control over from where gntdev/balloon drivers get the pages
(even more, those can easily be out of DMA address space of the display 
unit).


Thus, even if implemented, I can't use this approach.


In any case I just knew about dma-bufs this morning, there might be
things that I'm missing.

Roger.


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

Re: [PATCH 1/2 v3] drm/pl111: Support the Versatile Express

2018-04-18 Thread Robin Murphy


On 18/04/18 09:52, Linus Walleij wrote:

The Versatile Express uses a special configuration controller
deeply embedded in the system motherboard FPGA to multiplex the
two to three (!) display controller instances out to the single
SiI9022 bridge.

Set up an extra file with the logic to probe to the FPGA mux
register on the system controller bus, then parse the device
tree to see if there is a CLCD or HDLCD instance on the core
tile (also known as the daughterboard) by looking in the
root of the device tree for compatible nodes.

- If there is a HDLCD on the core tile, and there is a driver
   for it, we exit probe and deactivate the motherboard CLCD.
   We do not touch the DVI mux in this case, to make sure we
   don't break HDLCD.

- If there is a CLCD on both the motherboard and the core tile
   (only the CA9 has this) the core tile CLCD takes precedence
   and get muxed to the DVI connector.

- Only if there is no working graphics on the core tile, the
   motherboard CLCD is probed and muxed to the DVI connector.


This approach seems reasonable (modulo a couple of nits below), and 
almost certainly sufficient for most realistic uses, but I can't help 
wondering whether it might be feasible to model the mux with its own 
separate connector/bridge driver which could handle prioritisation of 
multiple inputs itself (I'm pondering things like RK3288/RK3399 where 
the two VOPs feeding into the single HDMI block looks a fair bit like 
the V2P-CA9 situation if you squint at it).



Core tile graphics should always take precedence as it can
address all memory and is also faster, however the motherboard
CLCD is good to have around for diagnostics and testing.

It is possible to test the motherboard CLCD by setting the
status = "disabled" property on the core tile CLCD or
HDLCD.

Scale down the Versatile Express to 16BPP so we can support a
1024x768 display despite the bus bandwidth restrictions on this
platform. (The motherboard CLCD supports slightly lower
resolution.)

Cc: Liviu Dudau 
Cc: Pawel Moll 
Signed-off-by: Linus Walleij 
---
ChangeLog v2->v3:
- Rewrite CLCD detection and mux priority logic, look in
   the device tree root for core tile graphics.
ChangeLog v1->v2:
- No changes just reposting rebased on mainline changes.
---
  drivers/gpu/drm/pl111/Makefile  |   1 +
  drivers/gpu/drm/pl111/pl111_versatile.c |  48 +++-
  drivers/gpu/drm/pl111/pl111_vexpress.c  | 127 
  drivers/gpu/drm/pl111/pl111_vexpress.h  |  22 ++
  4 files changed, 197 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c
  create mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h

diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
index 9c5e8dba8ac6..19a8189dc54f 100644
--- a/drivers/gpu/drm/pl111/Makefile
+++ b/drivers/gpu/drm/pl111/Makefile
@@ -3,6 +3,7 @@ pl111_drm-y +=  pl111_display.o \
pl111_versatile.o \
pl111_drv.o
  
+pl111_drm-$(CONFIG_ARCH_VEXPRESS) += pl111_vexpress.o

  pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
  
  obj-$(CONFIG_DRM_PL111) += pl111_drm.o

diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c 
b/drivers/gpu/drm/pl111/pl111_versatile.c
index 9302f516045e..569edf02a36a 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -1,12 +1,14 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include "pl111_versatile.h"
+#include "pl111_vexpress.h"
  #include "pl111_drm.h"
  
  static struct regmap *versatile_syscon_map;

@@ -22,6 +24,7 @@ enum versatile_clcd {
REALVIEW_CLCD_PB11MP,
REALVIEW_CLCD_PBA8,
REALVIEW_CLCD_PBX,
+   VEXPRESS_CLCD_V2M,
  };
  
  static const struct of_device_id versatile_clcd_of_match[] = {

@@ -53,6 +56,10 @@ static const struct of_device_id versatile_clcd_of_match[] = 
{
.compatible = "arm,realview-pbx-syscon",
.data = (void *)REALVIEW_CLCD_PBX,
},
+   {
+   .compatible = "arm,vexpress-muxfpga",
+   .data = (void *)VEXPRESS_CLCD_V2M,
+   },
{},
  };
  
@@ -286,12 +293,26 @@ static const struct pl111_variant_data pl111_realview = {

.fb_bpp = 16,
  };
  
+/*

+ * Versatile Express PL111 variant, again we just push the maximum
+ * BPP to 16 to be able to get 1024x768 without saturating the memory
+ * bus. The clockdivider also seems broken on the Versatile Express.
+ */
+static const struct pl111_variant_data pl111_vexpress = {
+   .name = "PL111 Versatile Express",
+   .formats = pl111_realview_pixel_formats,
+   .nformats = ARRAY_SIZE(pl111_realview_pixel_formats),
+   .fb_bpp = 16,
+   .broken_clockdivider = true,
+};
+
  int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private 
*priv)
  {
const struct of_device_id *clcd_id;
enum versatile_clcd versatile_clcd_type

RE: [PATCH v2 2/2] i915: content-type property for HDMI connector

2018-04-18 Thread Lisovskiy, Stanislav



From: Hans Verkuil [hverk...@xs4all.nl]
Sent: Wednesday, April 18, 2018 3:35 PM
To: Lisovskiy, Stanislav; dri-devel@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Subject: Re: [PATCH v2 2/2] i915: content-type property for HDMI connector

>> Is the ITC bit set in the AVI InfoFrame? The content type bits are only 
>> valid if
>> the ITC bit is 1. And if there is no content type property, then ITC should 
>> be 0.
>
> That's a good question, as I understood it is not set, however the HDMI 1.4 
> spec
> says:
>
> ITC CN1, CN0 Content Type
>  00,  0  No Data
>
>  10,  0  IT content
>
>  X   0,  1  Photo
>  X   1,  0  Cinema
>  X   1,  1  Game
>
> "X denotes don't care"
>
> So I wonder, should I probably add additional property to control the itc bit?

> Yeah, that's wrong in the HDMI Spec. The correct standard to use is the CEA 
> (now CTA)
> 861. There it clearly defines that the CN bits are only valid if ITC=1.

> I know, I'm in the CTA-861 working group :-)

> I don't think anyone ever noticed this bug in the HDMI spec before (I 
> didn't). The
> 2.1 HDMI spec kind of corrects it in Appendix G.1 where it copies the 
> CTA-861-G
text.

> Anyway, in V4L2 we implement this with an extra value "NO_ITC" which sets ITC 
> to 0
> and sets CN0 and CN1 to 0 as well:

> https://hverkuil.home.xs4all.nl/spec/uapi/v4l/extended-controls.html#digital-video-control-ids

> The NO_ITC case is usually used for YCbCr video encoding. Note that most 
> displays will
> just ignore the content type.

Wow, I wouldn't even assume that this spec is wrong. Probably then I need to 
add some way 
to manipulate an ITC bit also. I will have a look, however so far I didn't see 
any property responsible
for this. 

>
>>   /* TODO: handle pixel repetition for YCBCR420 outputs */
>>   intel_write_infoframe(encoder, crtc_state, &frame);
>>  }
>> @@ -2065,6 +2067,7 @@ intel_hdmi_add_properties(struct intel_hdmi 
>> *intel_hdmi, struct drm_connector *c
>>   intel_attach_force_audio_property(connector);
>>   intel_attach_broadcast_rgb_property(connector);
>>   intel_attach_aspect_ratio_property(connector);
>> + drm_connector_attach_content_type_property(connector);
>>   connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>>  }
>>
>>
>
> Regards,
>
> Hans
> ___
> 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

[PATCH 1/2] drm: panel-orientation-quirks: Add quirk for Lenovo Ideapad Mixx 310

2018-04-18 Thread Hans de Goede

Some production batches of the Lenovo Ideapad Mixx 310 laptop use
a portrait LCD panel, add a quirk for this.

Signed-off-by: Hans de Goede 
---
 drivers/gpu/drm/drm_panel_orientation_quirks.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c 
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index 902cc1a71e45..9274237b7f57 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -60,6 +60,12 @@ static const struct drm_dmi_panel_orientation_data 
itworks_tw891 = {
.orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
 };
 
+static const struct drm_dmi_panel_orientation_data lenovo_ideapad_miix_310 = {
+   .width = 800,
+   .height = 1280,
+   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
+};
+
 static const struct drm_dmi_panel_orientation_data vios_lth17 = {
.width = 800,
.height = 1280,
@@ -102,6 +108,17 @@ static const struct dmi_system_id orientation_data[] = {
  DMI_EXACT_MATCH(DMI_BOARD_NAME, "TW891"),
},
.driver_data = (void *)&itworks_tw891,
+   }, {/*
+* Lenovo Ideapad Miix 310 laptop, only some production batches
+* have a portrait screen, the resolution checks makes the quirk
+* apply only to those batches.
+*/
+   .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "80SG"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "MIIX 310-10ICR"),
+   },
+   .driver_data = (void *)&lenovo_ideapad_miix_310,
}, {/* VIOS LTH17 */
.matches = {
  DMI_EXACT_MATCH(DMI_SYS_VENDOR, "VIOS"),
-- 
2.17.0

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

[PATCH 2/2] drm: panel-orientation-quirks: Add quirk for Lenovo Ideapad Mixx 320

2018-04-18 Thread Hans de Goede

The Lenovo Ideapad Mixx 320 laptop uses a portrait LCD panel, add a
quirk for this.

While at it instead of duplicating the same drm_dmi_panel_orientation_data
for 3 laptops add a generic lcd800x1280_rightside_up orientation_data and
use that for all 3 (including the new Mixx 320 entry).

Signed-off-by: Hans de Goede 
---
 .../gpu/drm/drm_panel_orientation_quirks.c| 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_panel_orientation_quirks.c 
b/drivers/gpu/drm/drm_panel_orientation_quirks.c
index 9274237b7f57..caebddda8bce 100644
--- a/drivers/gpu/drm/drm_panel_orientation_quirks.c
+++ b/drivers/gpu/drm/drm_panel_orientation_quirks.c
@@ -60,13 +60,7 @@ static const struct drm_dmi_panel_orientation_data 
itworks_tw891 = {
.orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
 };
 
-static const struct drm_dmi_panel_orientation_data lenovo_ideapad_miix_310 = {
-   .width = 800,
-   .height = 1280,
-   .orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
-};
-
-static const struct drm_dmi_panel_orientation_data vios_lth17 = {
+static const struct drm_dmi_panel_orientation_data lcd800x1280_rightside_up = {
.width = 800,
.height = 1280,
.orientation = DRM_MODE_PANEL_ORIENTATION_RIGHT_UP,
@@ -118,13 +112,20 @@ static const struct dmi_system_id orientation_data[] = {
  DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "80SG"),
  DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "MIIX 310-10ICR"),
},
-   .driver_data = (void *)&lenovo_ideapad_miix_310,
+   .driver_data = (void *)&lcd800x1280_rightside_up,
+   }, {/* Lenovo Ideapad Miix 320 */
+   .matches = {
+ DMI_EXACT_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "80XF"),
+ DMI_EXACT_MATCH(DMI_PRODUCT_VERSION, "Lenovo MIIX 320-10ICR"),
+   },
+   .driver_data = (void *)&lcd800x1280_rightside_up,
}, {/* VIOS LTH17 */
.matches = {
  DMI_EXACT_MATCH(DMI_SYS_VENDOR, "VIOS"),
  DMI_EXACT_MATCH(DMI_PRODUCT_NAME, "LTH17"),
},
-   .driver_data = (void *)&vios_lth17,
+   .driver_data = (void *)&lcd800x1280_rightside_up,
},
{}
 };
-- 
2.17.0

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

Re: [PATCH v2 2/2] i915: content-type property for HDMI connector

2018-04-18 Thread Hans Verkuil

On 04/18/18 14:01, Lisovskiy, Stanislav wrote:
> Hi, 
> 
> Please see my reply inline:
> 
> From: dri-devel [dri-devel-boun...@lists.freedesktop.org] on behalf of Hans 
> Verkuil [hverk...@xs4all.nl]
> Sent: Wednesday, April 18, 2018 2:35 PM
> To: Lisovskiy, Stanislav; dri-devel@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org
> Subject: Re: [PATCH v2 2/2] i915: content-type property for HDMI connector
> 
> On 04/18/18 18:51, StanLis wrote:
>> From: Stanislav Lisovskiy 
>>
>> Added encoding of drm content_type property from
>> drm_connector_state within AVI infoframe in order to properly handle
>> external HDMI TV content-type setting.
>>
>> Signed-off-by: Stanislav Lisovskiy 
>> ---
>>  drivers/gpu/drm/i915/intel_atomic.c | 1 +
>>  drivers/gpu/drm/i915/intel_hdmi.c   | 3 +++
>>  2 files changed, 4 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
>> b/drivers/gpu/drm/i915/intel_atomic.c
>> index 40285d1b91b7..61ddb5871d8a 100644
>> --- a/drivers/gpu/drm/i915/intel_atomic.c
>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
>> @@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct 
>> drm_connector *conn,
>>   if (new_conn_state->force_audio != old_conn_state->force_audio ||
>>   new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
>>   new_conn_state->base.picture_aspect_ratio != 
>> old_conn_state->base.picture_aspect_ratio ||
>> + new_conn_state->base.content_type != 
>> old_conn_state->base.content_type ||
>>   new_conn_state->base.scaling_mode != 
>> old_conn_state->base.scaling_mode)
>>   crtc_state->mode_changed = true;
>>
>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
>> b/drivers/gpu/drm/i915/intel_hdmi.c
>> index ee929f31f7db..5cc72da9c086 100644
>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>> @@ -491,6 +491,8 @@ static void intel_hdmi_set_avi_infoframe(struct 
>> drm_encoder *encoder,
>>  
>> intel_hdmi->rgb_quant_range_selectable,
>>  is_hdmi2_sink);
>>
>> + frame.avi.content_type = connector->state->content_type;
>> +
> 
>> Is the ITC bit set in the AVI InfoFrame? The content type bits are only 
>> valid if
>> the ITC bit is 1. And if there is no content type property, then ITC should 
>> be 0.
> 
> That's a good question, as I understood it is not set, however the HDMI 1.4 
> spec
> says:
> 
> ITC CN1, CN0 Content Type
>  00,  0  No Data
> 
>  10,  0  IT content
> 
>  X   0,  1  Photo
>  X   1,  0  Cinema
>  X   1,  1  Game
> 
> "X denotes don't care"
> 
> So I wonder, should I probably add additional property to control the itc bit?

Yeah, that's wrong in the HDMI Spec. The correct standard to use is the CEA 
(now CTA)
861. There it clearly defines that the CN bits are only valid if ITC=1.

I know, I'm in the CTA-861 working group :-)

I don't think anyone ever noticed this bug in the HDMI spec before (I didn't). 
The
2.1 HDMI spec kind of corrects it in Appendix G.1 where it copies the CTA-861-G
text.

Anyway, in V4L2 we implement this with an extra value "NO_ITC" which sets ITC 
to 0
and sets CN0 and CN1 to 0 as well:

https://hverkuil.home.xs4all.nl/spec/uapi/v4l/extended-controls.html#digital-video-control-ids

The NO_ITC case is usually used for YCbCr video encoding. Note that most 
displays will
just ignore the content type.

Regards,

Hans

> 
>>   /* TODO: handle pixel repetition for YCBCR420 outputs */
>>   intel_write_infoframe(encoder, crtc_state, &frame);
>>  }
>> @@ -2065,6 +2067,7 @@ intel_hdmi_add_properties(struct intel_hdmi 
>> *intel_hdmi, struct drm_connector *c
>>   intel_attach_force_audio_property(connector);
>>   intel_attach_broadcast_rgb_property(connector);
>>   intel_attach_aspect_ratio_property(connector);
>> + drm_connector_attach_content_type_property(connector);
>>   connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>>  }
>>
>>
> 
> Regards,
> 
> Hans
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

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

RE: [PATCH v2 1/2] drm: content-type property for HDMI connector

2018-04-18 Thread Lisovskiy, Stanislav

Totally agree about caps, thanks for spotting.

From: Jani Nikula [jani.nik...@linux.intel.com]
Sent: Wednesday, April 18, 2018 2:50 PM
To: Lisovskiy, Stanislav; dri-devel@lists.freedesktop.org
Cc: intel-...@linux.intel.com
Subject: Re: [PATCH v2 1/2] drm: content-type property for HDMI connector

On Wed, 18 Apr 2018, StanLis  wrote:
> +static const struct drm_prop_enum_list drm_content_type_enum_list[] = {
> + { DRM_MODE_CONTENT_TYPE_GRAPHICS, "GRAPHICS" },
> + { DRM_MODE_CONTENT_TYPE_PHOTO, "PHOTO" },
> + { DRM_MODE_CONTENT_TYPE_CINEMA, "CINEMA" },
> + { DRM_MODE_CONTENT_TYPE_GAME, "GAME" },
> +};
> +

Values all caps...

> +int drm_mode_create_content_type_property(struct drm_device *dev)
> +{
> + if (dev->mode_config.content_type_property)
> + return 0;
> +
> + dev->mode_config.content_type_property =
> + drm_property_create_enum(dev, 0, "content type",
> + drm_content_type_enum_list,
> + ARRAY_SIZE(drm_content_type_enum_list));

...and property name lower case with a space.

Can we please have a little more consistency at least within properties?
Overall it's a huge mess already, but it doesn't help to add to the
mess.

BR,
Jani.


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

RE: [PATCH v2 2/2] i915: content-type property for HDMI connector

2018-04-18 Thread Lisovskiy, Stanislav

Hi, 

Please see my reply inline:

From: dri-devel [dri-devel-boun...@lists.freedesktop.org] on behalf of Hans 
Verkuil [hverk...@xs4all.nl]
Sent: Wednesday, April 18, 2018 2:35 PM
To: Lisovskiy, Stanislav; dri-devel@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Subject: Re: [PATCH v2 2/2] i915: content-type property for HDMI connector

On 04/18/18 18:51, StanLis wrote:
> From: Stanislav Lisovskiy 
>
> Added encoding of drm content_type property from
> drm_connector_state within AVI infoframe in order to properly handle
> external HDMI TV content-type setting.
>
> Signed-off-by: Stanislav Lisovskiy 
> ---
>  drivers/gpu/drm/i915/intel_atomic.c | 1 +
>  drivers/gpu/drm/i915/intel_hdmi.c   | 3 +++
>  2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 40285d1b91b7..61ddb5871d8a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct 
> drm_connector *conn,
>   if (new_conn_state->force_audio != old_conn_state->force_audio ||
>   new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
>   new_conn_state->base.picture_aspect_ratio != 
> old_conn_state->base.picture_aspect_ratio ||
> + new_conn_state->base.content_type != 
> old_conn_state->base.content_type ||
>   new_conn_state->base.scaling_mode != 
> old_conn_state->base.scaling_mode)
>   crtc_state->mode_changed = true;
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index ee929f31f7db..5cc72da9c086 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -491,6 +491,8 @@ static void intel_hdmi_set_avi_infoframe(struct 
> drm_encoder *encoder,
>  
> intel_hdmi->rgb_quant_range_selectable,
>  is_hdmi2_sink);
>
> + frame.avi.content_type = connector->state->content_type;
> +

>Is the ITC bit set in the AVI InfoFrame? The content type bits are only valid 
>if
>the ITC bit is 1. And if there is no content type property, then ITC should be 
>0.

That's a good question, as I understood it is not set, however the HDMI 1.4 spec
says:

ITC CN1, CN0 Content Type
 00,  0  No Data

 10,  0  IT content

 X   0,  1  Photo
 X   1,  0  Cinema
 X   1,  1  Game

"X denotes don't care"

So I wonder, should I probably add additional property to control the itc bit?

>   /* TODO: handle pixel repetition for YCBCR420 outputs */
>   intel_write_infoframe(encoder, crtc_state, &frame);
>  }
> @@ -2065,6 +2067,7 @@ intel_hdmi_add_properties(struct intel_hdmi 
> *intel_hdmi, struct drm_connector *c
>   intel_attach_force_audio_property(connector);
>   intel_attach_broadcast_rgb_property(connector);
>   intel_attach_aspect_ratio_property(connector);
> + drm_connector_attach_content_type_property(connector);
>   connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>
>

Regards,

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

[Bug 199319] Flickering screen on AMDGPU and DC with Linux 4.16-2

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

--- Comment #17 from zxvfxwing (zxvfxw...@protonmail.com) ---
(In reply to Harry Wentland from comment #16)
> A fix should be in drm-next-4.17 of Alex's git repo at
> https://cgit.freedesktop.org/~agd5f/linux/?h=drm-next-4.17 and should make
> it into Linus's tree from there.
> 
> That fix should also make it into 4.16 stable.

Good to know! Thank you.

-- 
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 1/2 v2] drm/pl111: Support the Versatile Express

2018-04-18 Thread Robin Murphy


On 17/04/18 20:31, Linus Walleij wrote:

On Tue, Apr 17, 2018 at 3:12 PM, Robin Murphy  wrote:

On 17/04/18 13:32, Linus Walleij wrote:
[...]


Unfortunately there is just one single vexpress core tile in the
upstream kernel that define a CLCD controller, the CA9 (4xA9)
that I am using. All the others just use the MB CLCD.

I am thinking there is some never finished DTS upstreaming
here that ought to happen so we use the core tile CLCD on some
other boards as well.



Barring custom FPGA images, I think V2P-CA9 *is* the only VExpress tile
implementing PL11x; all the more recent test chips had HDLCD instead.


Yup it looks like that.

I am restructuring the code to look for any graphics on the
core tile and only turn on the motherboard CLCD if there is
no CLCD or HDLCD on the core tile, e.g. if that device node has
been set to "disabled" or the HDLCD driver is not compiled in
but the CLCD driver is.

I found some vague mentions of a LCD display that can be
connected to the motherboard, so that would then be sitting
on the CLCD, is this something you ARM guys have seen
around? In that case I guess the VExpress (etc) would actually
be able to have two screens, one LCD panel on the motherboard
CLCD and a DVI from the core tile.


I don't see anything in the V2M motherboard manual to suggest the IOFPGA 
CLCD output is routed anywhere other than the DVI mux, and there are 
certainly no unaccounted-for connectors on the VExpress version 
currently in front of me (HBI-0190D). The only things I'm immediately 
aware of having a panel connector directly on the board are 
Integrator/CP and RealView EB.


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

Re: [PATCH v2 1/2] drm: content-type property for HDMI connector

2018-04-18 Thread Jani Nikula

On Wed, 18 Apr 2018, StanLis  wrote:
> +static const struct drm_prop_enum_list drm_content_type_enum_list[] = {
> + { DRM_MODE_CONTENT_TYPE_GRAPHICS, "GRAPHICS" },
> + { DRM_MODE_CONTENT_TYPE_PHOTO, "PHOTO" },
> + { DRM_MODE_CONTENT_TYPE_CINEMA, "CINEMA" },
> + { DRM_MODE_CONTENT_TYPE_GAME, "GAME" },
> +};
> +

Values all caps...

> +int drm_mode_create_content_type_property(struct drm_device *dev)
> +{
> + if (dev->mode_config.content_type_property)
> + return 0;
> +
> + dev->mode_config.content_type_property =
> + drm_property_create_enum(dev, 0, "content type",
> + drm_content_type_enum_list,
> + ARRAY_SIZE(drm_content_type_enum_list));

...and property name lower case with a space.

Can we please have a little more consistency at least within properties?
Overall it's a huge mess already, but it doesn't help to add to the
mess.

BR,
Jani.


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

Re: [PATCH v2 1/2] drm: content-type property for HDMI connector

2018-04-18 Thread Hans Verkuil

On 04/18/18 18:51, StanLis wrote:
> From: Stanislav Lisovskiy 
> 
> Added content_type property to
> drm_connector_state in order to properly handle
> external HDMI TV content-type setting.
> 
> Signed-off-by: Stanislav Lisovskiy 
> ---
>  Documentation/gpu/kms-properties.csv |  1 +
>  drivers/gpu/drm/drm_atomic.c |  4 +++
>  drivers/gpu/drm/drm_connector.c  | 50 
>  drivers/gpu/drm/drm_edid.c   |  1 +
>  include/drm/drm_connector.h  | 11 ++
>  include/drm/drm_mode_config.h|  5 +++
>  include/uapi/drm/drm_mode.h  |  5 +++
>  7 files changed, 77 insertions(+)
> 
> diff --git a/Documentation/gpu/kms-properties.csv 
> b/Documentation/gpu/kms-properties.csv
> index 6b28b014cb7d..7a02b2782f33 100644
> --- a/Documentation/gpu/kms-properties.csv
> +++ b/Documentation/gpu/kms-properties.csv
> @@ -17,6 +17,7 @@ Owner Module/Drivers,Group,Property Name,Type,Property 
> Values,Object attached,De
>  ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0x",Connector,property 
> to suggest an X offset for a connector
>  ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to suggest 
> an Y offset for a connector
>  ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" 
> }",Connector,TDB
> +,Optional,"""content type""",ENUM,"{ ""Graphics"", ""Photo"", ""Cinema"", 
> ""Game"" }",Connector,TDB
>  i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited 
> 16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is 
> set, the hardware will be programmed with the result of the multiplication of 
> CTM by the limited range matrix to ensure the pixels normaly in the range 
> 0..1.0 are remapped to the range 16/255..235/255."
>  ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
>  ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } 
> etc.",Connector,TBD
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 7d25c42f22db..72fd2a1c801f 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1266,6 +1266,8 @@ static int drm_atomic_connector_set_property(struct 
> drm_connector *connector,
>   state->link_status = val;
>   } else if (property == config->aspect_ratio_property) {
>   state->picture_aspect_ratio = val;
> + } else if (property == config->content_type_property) {
> + state->content_type = val;
>   } else if (property == connector->scaling_mode_property) {
>   state->scaling_mode = val;
>   } else if (property == connector->content_protection_property) {
> @@ -1351,6 +1353,8 @@ drm_atomic_connector_get_property(struct drm_connector 
> *connector,
>   *val = state->link_status;
>   } else if (property == config->aspect_ratio_property) {
>   *val = state->picture_aspect_ratio;
> + } else if (property == config->content_type_property) {
> + *val = state->content_type;
>   } else if (property == connector->scaling_mode_property) {
>   *val = state->scaling_mode;
>   } else if (property == connector->content_protection_property) {
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index b3cde897cd80..5633494f6d78 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -720,6 +720,13 @@ static const struct drm_prop_enum_list 
> drm_aspect_ratio_enum_list[] = {
>   { DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
>  };
>  
> +static const struct drm_prop_enum_list drm_content_type_enum_list[] = {
> + { DRM_MODE_CONTENT_TYPE_GRAPHICS, "GRAPHICS" },
> + { DRM_MODE_CONTENT_TYPE_PHOTO, "PHOTO" },
> + { DRM_MODE_CONTENT_TYPE_CINEMA, "CINEMA" },
> + { DRM_MODE_CONTENT_TYPE_GAME, "GAME" },

Just use "Graphics", "Photo", etc. Same as is used in drivers/video/hdmi.c
for logging the AVI InfoFrame.

Regards,

Hans

> +};
> +
>  static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {
>   { DRM_MODE_PANEL_ORIENTATION_NORMAL,"Normal"},
>   { DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down"   },
> @@ -996,6 +1003,22 @@ int drm_mode_create_dvi_i_properties(struct drm_device 
> *dev)
>  }
>  EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
>  
> +/**
> + * drm_connector_attach_content_type_property - attach content-type property
> + * @dev: DRM device
> + *
> + * Called by a driver the first time a HDMI connector is made.
> + */
> +int drm_connector_attach_content_type_property(struct drm_connector 
> *connector)
> +{
> + if (!drm_mode_create_content_type_property(connector->dev))
> + drm_object_attach_property(&connector->base,
> + connector->dev->mode_config.content_type_property,
> + DRM_MODE_CONTENT_TYPE_GRAPHICS);
> + return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_cont

Re: [PATCH v2 2/2] i915: content-type property for HDMI connector

2018-04-18 Thread Hans Verkuil

On 04/18/18 18:51, StanLis wrote:
> From: Stanislav Lisovskiy 
> 
> Added encoding of drm content_type property from
> drm_connector_state within AVI infoframe in order to properly handle
> external HDMI TV content-type setting.
> 
> Signed-off-by: Stanislav Lisovskiy 
> ---
>  drivers/gpu/drm/i915/intel_atomic.c | 1 +
>  drivers/gpu/drm/i915/intel_hdmi.c   | 3 +++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
> b/drivers/gpu/drm/i915/intel_atomic.c
> index 40285d1b91b7..61ddb5871d8a 100644
> --- a/drivers/gpu/drm/i915/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> @@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct 
> drm_connector *conn,
>   if (new_conn_state->force_audio != old_conn_state->force_audio ||
>   new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
>   new_conn_state->base.picture_aspect_ratio != 
> old_conn_state->base.picture_aspect_ratio ||
> + new_conn_state->base.content_type != 
> old_conn_state->base.content_type ||
>   new_conn_state->base.scaling_mode != 
> old_conn_state->base.scaling_mode)
>   crtc_state->mode_changed = true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index ee929f31f7db..5cc72da9c086 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -491,6 +491,8 @@ static void intel_hdmi_set_avi_infoframe(struct 
> drm_encoder *encoder,
>  
> intel_hdmi->rgb_quant_range_selectable,
>  is_hdmi2_sink);
>  
> + frame.avi.content_type = connector->state->content_type;
> +

Is the ITC bit set in the AVI InfoFrame? The content type bits are only valid if
the ITC bit is 1. And if there is no content type property, then ITC should be 
0.

>   /* TODO: handle pixel repetition for YCBCR420 outputs */
>   intel_write_infoframe(encoder, crtc_state, &frame);
>  }
> @@ -2065,6 +2067,7 @@ intel_hdmi_add_properties(struct intel_hdmi 
> *intel_hdmi, struct drm_connector *c
>   intel_attach_force_audio_property(connector);
>   intel_attach_broadcast_rgb_property(connector);
>   intel_attach_aspect_ratio_property(connector);
> + drm_connector_attach_content_type_property(connector);
>   connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
>  }
>  
> 

Regards,

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

Re: [PATCH hwc v2 17/18] drm_hwcomposer: Flatten scene synchronously

On Tue, Apr 17, 2018 at 01:47:46PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:28PM +0100, Alexandru Gheorghe wrote:
> > Flatten scene on the same CRTC as the one driving the display.
> > The active composition is played back to the display with a buffer
> > attached to the writeback connector.
> > Then we build a composition that has only one plane enabled and that
> > uses the result of the writeback as the input.
> > 
> > Signed-off-by: Alexandru Gheorghe 
> > ---
> >  drmdisplaycompositor.cpp | 203 
> > +--
> >  drmdisplaycompositor.h   |   7 +-
> >  2 files changed, 204 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> > index e535e8a..cb670e6 100644
> > --- a/drmdisplaycompositor.cpp
> > +++ b/drmdisplaycompositor.cpp
> > @@ -36,6 +36,7 @@
> >  #include "drmplane.h"
> >  #include "drmresources.h"
> >  #include "glworker.h"
> > +static const uint32_t kWaitWritebackFence = 100;  // ms
> >  
> >  namespace android {
> >  
> > @@ -523,7 +524,9 @@ int 
> > DrmDisplayCompositor::PrepareFrame(DrmDisplayComposition *display_comp) {
> >  }
> >  
> >  int DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> > -  bool test_only) {
> > +  bool test_only,
> > +  DrmDisplayComposition 
> > *writeback_comp,
> > +  DrmConnector *writeback_conn) {
> >ATRACE_CALL();
> >  
> >int ret = 0;
> > @@ -532,6 +535,7 @@ int 
> > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> >std::vector &comp_planes =
> >display_comp->composition_planes();
> >uint64_t out_fences[drm_->crtcs().size()];
> > +  int writeback_fence = -1;
> >  
> >DrmConnector *connector = drm_->GetConnectorForDisplay(display_);
> >if (!connector) {
> > @@ -550,9 +554,37 @@ int 
> > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> >  return -ENOMEM;
> >}
> >  
> > +  if (writeback_comp != NULL) {
> > +if (writeback_conn == NULL)
> > +  return -EINVAL;
> > +if (writeback_conn->writeback_fb_id().id() == 0 ||
> > +writeback_conn->writeback_out_fence().id() == 0) {
> > +  ALOGE("Writeback properties don't exit");
> > +  return -EINVAL;
> > +}
> > +if (writeback_comp->layers().size() != 1) {
> > +  ALOGE("Invalid number of layers for writeback composition");
> > +  return -EINVAL;
> > +}
> > +ret = drmModeAtomicAddProperty(
> > +pset, writeback_conn->id(), writeback_conn->writeback_fb_id().id(),
> > +writeback_comp->layers().back().buffer->fb_id);
> > +if (ret < 0) {
> > +  ALOGE("Failed to add writeback_fb_id");
> > +  return ret;
> > +}
> > +ret = drmModeAtomicAddProperty(pset, writeback_conn->id(),
> > +   
> > writeback_conn->writeback_out_fence().id(),
> > +   (uint64_t)&writeback_fence);
> 
> Upcasting int to u64 isn't a great idea, please go the other way (as we do 
> with
> out_fences).

Genuinely curious about this, why does it make a difference I'm
upcasting the address not the int itself.
More, the kernel does this s32 __user *fence_ptr = u64_to_user_ptr(val);
To be completly fair it should have been int32_t.


> 
> > +if (ret < 0) {
> > +  ALOGE("Failed to add writeback_out_fence");
> > +  return ret;
> > +}
> > +  }
> 
> This would be more readable if it was split off into a function.
> 
> >if (crtc->out_fence_ptr_property().id() != 0) {
> > -ret = drmModeAtomicAddProperty(pset, crtc->id(), 
> > crtc->out_fence_ptr_property().id(),
> > -   (uint64_t) &out_fences[crtc->pipe()]);
> > +ret = drmModeAtomicAddProperty(pset, crtc->id(),
> > +   crtc->out_fence_ptr_property().id(),
> > +   (uint64_t)&out_fences[crtc->pipe()]);
> >  if (ret < 0) {
> >ALOGE("Failed to add OUT_FENCE_PTR property to pset: %d", ret);
> >drmModeAtomicFree(pset);
> > @@ -580,6 +612,15 @@ int 
> > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *display_comp,
> >  }
> >}
> >  
> > +  if (writeback_conn != NULL) {
> > +ret = drmModeAtomicAddProperty(pset, writeback_conn->id(),
> > +   writeback_conn->crtc_id_property().id(),
> > +   crtc->id());
> > +if (ret < 0) {
> > +  ALOGE("Failed to  attach writeback");
> > +}
> > +  }
> 
> Can you do this above with the rest of the writeback properties?
> 
> > +
> >for (DrmCompositionPlane &comp_plane : comp_planes) {
> >  DrmPlane *plane = comp_plane.plane();
> >  DrmCrtc *crtc = comp_plane.crtc();
> > @@ -729,8 +770,18 @@ int 
> > DrmDisplayCompositor::CommitFrame(DrmDisplayComposition *disp

Re: [Freedreno] [DPU PATCH v2 2/2] drm/panel: add backlight control support for truly panel

2018-04-18 Thread Daniel Thompson

On Tue, Apr 17, 2018 at 05:42:04PM -0700, abhin...@codeaurora.org wrote:
> Adding another point.
> 
> On 2018-04-17 17:04, abhin...@codeaurora.org wrote:
> > Hi Bjorn
> > 
> > Apologies if the prev reply wasnt clear.
> > 
> > Hope this one is.
> > 
> > reply inline.
> > 
> > On 2018-04-17 14:29, Bjorn Andersson wrote:
> > > On Tue 17 Apr 11:21 PDT 2018, abhin...@codeaurora.org wrote:
> > > > On 2018-04-16 23:13, Bjorn Andersson wrote:
> > > [..]
> > > > > If the panel isn't actually a piece of backlight hardware then it 
> > > > > should
> > > > > not register a backlight device. Why do you need your own sysfs?
> > > > >
> > > > > Regards,
> > > > > Bjorn
> > > > [Abhinav] This particular panel isnt a piece of backlight hardware.
> > > > But, we want to have our own sysfs to give flexibility to our
> > > > userspace
> > > > to write and read stuff for its proprietary uses.
> > > 
> > > Please be more specific in your replies, no one will accept code that
> > > "does stuff" and expecting a reviewer to spend time guessing or
> > > pulling
> > > the information out of you is a sure way to get your patches ignored.
> > > 
> > > Exactly what kind of stuff are you talking about here and exactly
> > > which
> > > problem are you solving.
> > > 
> > > > Thats how our downstream has been working so far and hence to
> > > > maintain
> > > > the compatibility would like to have it.
> > > 
> > > Make your proprietary code work with the upstream kernel and you
> > > shouldn't ever have to modify it.
> > > 
> > > Regards,
> > > Bjorn
> > 
> > [Abhinav] We have a few userspace clients today for the backlight sysfs
> > node
> > which read/write directly to
> > "/sys/class/backlight/panel0-backlight/brightness"
> > When I said "stuff" I was only referring to the brightness value.
> > This separate sysfs node allows us to validate those userspace features
> > of ours
> > which directly edit the backlight value on our reference platform.
> > Since our reference platform uses this panel,hence having our own
> > sysfs alias helps.
> > Otherwise, whenever we try to use this panel with upstream code we
> > will have to end up
> > changing all those places in our userspace/framework to change the sysfs
> > path.
> > Hence we wanted to preserve that sysfs node name.
> > The wled device is not created under /sys/class/backlight but under
> > /sys/class/leds/wled.
> > So we will have to change the name of this node across all our
> > userspace.
> > 
> > If this isnt a convincing reason enough to have its own sysfs node
> > path, I will use
> > your approach.
> > 
> > Let me know your comments or if this is still not clear.
> > 
> [Abhinav] We also might not be using the brightness value "as-it-is".
> 
> We will potentially scale it up/down based on some requirements.
> 
> If we do not have our own sysfs alias for this, how do we account for
> providing this interface for our chipset specific backlight dependent
> feature.
> 
> Can you please comment on this?

Not easily. It's rather unclear what this chipset specific backlight
dependent feature you have alluded to is so how can we suggest how to
control or model it in the upstream kernel?

I can make a guess that is might be to do with brightness curves... but
I'd really prefer not to have to guess.

There are some problems with the current interface because it is not
well defined with the brightness control is linear or
logarithmic/perceptual (patches welcome) but for other common embedded
backlights (pwm_bl particularly) we expect calibration of the
brightness curve to be a job for the device tree (because it is a
property of the hardware it can be described in the DT) and there are
patches pending to improve this.


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

Re: [PATCH hwc v2 14/18] drm_hwcomposer: Fix race in ApplyFrame

On Tue, Apr 17, 2018 at 01:02:18PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:25PM +0100, Alexandru Gheorghe wrote:
> > ApplyFrame holds the lock just when it swaps the value of
> > active_composition_, in a multithread context we could end up in a
> > situation where something is shown on the screen, but something else
> > is set in active_composition_. Fix it by holding the lock during
> > CommitFrame.
> > 
> > Signed-off-by: Alexandru Gheorghe 
> > ---
> >  drmdisplaycompositor.cpp | 40 +---
> >  drmdisplaycompositor.h   |  2 +-
> >  2 files changed, 18 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> > index afd3b05..576539b 100644
> > --- a/drmdisplaycompositor.cpp
> > +++ b/drmdisplaycompositor.cpp
> > @@ -791,11 +791,6 @@ std::tuple 
> > DrmDisplayCompositor::CreateModeBlob(
> >  }
> >  
> >  void DrmDisplayCompositor::ClearDisplay() {
> > -  AutoLock lock(&lock_, "compositor");
> > -  int ret = lock.Lock();
> > -  if (ret)
> > -return;
> > -
> >if (!active_composition_)
> >  return;
> >  
> > @@ -808,11 +803,25 @@ void DrmDisplayCompositor::ClearDisplay() {
> >  }
> >  
> >  void DrmDisplayCompositor::ApplyFrame(
> > -std::unique_ptr composition, int status) {
> > +std::unique_ptr composition, int status,
> > +bool writeback) {
> 
> The writeback argument addition seems unrelated to this change.

Agree.

> 
> > +  AutoLock lock(&lock_, __FUNCTION__);
> > +  if (lock.Lock())
> > +return;
> >int ret = status;
> > -
> > -  if (!ret)
> > +  if (!ret) {
> > +if (writeback && !CountdownExpired()) {
> > +  ALOGE("Abort playing back scene");
> > +  return;
> > +}
> >  ret = CommitFrame(composition.get(), false);
> > +if (!ret) {
> > +  ++dump_frames_composited_;
> > +  if (active_composition_)
> > +active_composition_->SignalCompositionDone();
> > +  active_composition_.swap(composition);
> 
> Why move this stuff?

Because both CommitFrame and swap need the lock, the code in between
don't need that.

> 
> > +}
> > +  }
> >  
> >if (ret) {
> >  ALOGE("Composite failed for display %d", display_);
> > @@ -821,21 +830,6 @@ void DrmDisplayCompositor::ApplyFrame(
> >  ClearDisplay();
> >  return;
> >}
> > -  ++dump_frames_composited_;
> > -
> > -  if (active_composition_)
> > -active_composition_->SignalCompositionDone();
> > -
> > -  ret = pthread_mutex_lock(&lock_);
> > -  if (ret)
> > -ALOGE("Failed to acquire lock for active_composition swap");
> > -
> > -  active_composition_.swap(composition);
> > -
> > -  if (!ret)
> > -ret = pthread_mutex_unlock(&lock_);
> > -  if (ret)
> > -ALOGE("Failed to release lock for active_composition swap");
> >  }
> >  
> >  int DrmDisplayCompositor::ApplyComposition(
> > diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> > index 0f8daad..b35ef70 100644
> > --- a/drmdisplaycompositor.h
> > +++ b/drmdisplaycompositor.h
> > @@ -127,7 +127,7 @@ class DrmDisplayCompositor {
> >  
> >void ClearDisplay();
> >void ApplyFrame(std::unique_ptr composition,
> > -  int status);
> > +  int status, bool writeback = false);
> >  
> >std::tuple CreateModeBlob(const DrmMode &mode);
> >  
> > -- 
> > 2.7.4
> > 
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

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

[PATCH] drm/tegra: hub: Use state directly

2018-04-18 Thread Stefan Schake

Using drm_atomic_get_private_obj_state after state has been swapped
will return old state.

Fixes: 0281c4149021 ("drm/tegra: hub: Use private object for global state")
Signed-off-by: Stefan Schake 
---
 drivers/gpu/drm/tegra/hub.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/hub.c b/drivers/gpu/drm/tegra/hub.c
index 9a3f23d4780f..bdd2cdd0745c 100644
--- a/drivers/gpu/drm/tegra/hub.c
+++ b/drivers/gpu/drm/tegra/hub.c
@@ -683,12 +683,11 @@ void tegra_display_hub_atomic_commit(struct drm_device 
*drm,
 {
struct tegra_drm *tegra = drm->dev_private;
struct tegra_display_hub *hub = tegra->hub;
-   struct tegra_display_hub_state *hub_state;
+   struct tegra_display_hub_state *hub_state =
+   to_tegra_display_hub_state(hub->base.state);
struct device *dev = hub->client.dev;
int err;
 
-   hub_state = tegra_display_hub_get_state(hub, state);
-
if (hub_state->clk) {
err = clk_set_rate(hub_state->clk, hub_state->rate);
if (err < 0)
-- 
2.14.1

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

Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver

On 04/18/2018 01:18 PM, Paul Durrant wrote:

-Original Message-
From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
Of Roger Pau Monné
Sent: 18 April 2018 11:11
To: Oleksandr Andrushchenko 
Cc: jgr...@suse.com; Artem Mygaiev ;
Dongwon Kim ; airl...@linux.ie;
oleksandr_andrushche...@epam.com; linux-ker...@vger.kernel.org; dri-
de...@lists.freedesktop.org; Potrola, MateuszX
; xen-de...@lists.xenproject.org;
daniel.vet...@intel.com; boris.ostrov...@oracle.com; Matt Roper

Subject: Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy
helper DRM driver

On Wed, Apr 18, 2018 at 11:01:12AM +0300, Oleksandr Andrushchenko
wrote:

On 04/18/2018 10:35 AM, Roger Pau Monné wrote:

On Wed, Apr 18, 2018 at 09:38:39AM +0300, Oleksandr Andrushchenko

wrote:

On 04/17/2018 11:57 PM, Dongwon Kim wrote:

On Tue, Apr 17, 2018 at 09:59:28AM +0200, Daniel Vetter wrote:

On Mon, Apr 16, 2018 at 12:29:05PM -0700, Dongwon Kim wrote:

3.2 Backend exports dma-buf to xen-front

In this case Dom0 pages are shared with DomU. As before, DomU can

only write

to these pages, not any other page from Dom0, so it can be still

considered

safe.
But, the following must be considered (highlighted in xen-front's Kernel
documentation):
   - If guest domain dies then pages/grants received from the backend

cannot

     be claimed back - think of it as memory lost to Dom0 (won't be used

for

any
     other guest)
   - Misbehaving guest may send too many requests to the backend

exhausting

     its grant references and memory (consider this from security POV).

As the

     backend runs in the trusted domain we also assume that it is trusted

as

well,
     e.g. must take measures to prevent DDoS attacks.

I cannot parse the above sentence:

"As the backend runs in the trusted domain we also assume that it is
trusted as well, e.g. must take measures to prevent DDoS attacks."

What's the relation between being trusted and protecting from DoS
attacks?

I mean that we trust the backend that it can prevent Dom0
from crashing in case DomU's frontend misbehaves, e.g.
if the frontend sends too many memory requests etc.

In any case, all? PV protocols are implemented with the frontend
sharing pages to the backend, and I think there's a reason why this
model is used, and it should continue to be used.

This is the first use-case above. But there are real-world
use-cases (embedded in my case) when physically contiguous memory
needs to be shared, one of the possible ways to achieve this is
to share contiguous memory from Dom0 to DomU (the second use-case

above)

Having to add logic in the backend to prevent such attacks means
that:

   - We need more code in the backend, which increases complexity and
 chances of bugs.
   - Such code/logic could be wrong, thus allowing DoS.

You can live without this code at all, but this is then up to
backend which may make Dom0 down because of DomU's frontend doing

evil

things

IMO we should design protocols that do not allow such attacks instead
of having to defend against them.

4. xen-front/backend/xen-zcopy synchronization

4.1. As I already said in 2) all the inter VM communication happens

between

xen-front and the backend, xen-zcopy is NOT involved in that.
When xen-front wants to destroy a display buffer (dumb/dma-buf) it

issues a

XENDISPL_OP_DBUF_DESTROY command (opposite to

XENDISPL_OP_DBUF_CREATE).

This call is synchronous, so xen-front expects that backend does free

the

buffer pages on return.

4.2. Backend, on XENDISPL_OP_DBUF_DESTROY:
    - closes all dumb handles/fd's of the buffer according to [3]
    - issues DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE IOCTL to xen-

zcopy to make

sure
      the buffer is freed (think of it as it waits for dma-buf->release
callback)

So this zcopy thing keeps some kind of track of the memory usage? Why
can't the user-space backend keep track of the buffer usage?

Because there is no dma-buf UAPI which allows to track the buffer life cycle
(e.g. wait until dma-buf's .release callback is called)

    - replies to xen-front that the buffer can be destroyed.
This way deletion of the buffer happens synchronously on both Dom0

and DomU

sides. In case if DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE returns

with time-out

error
(BTW, wait time is a parameter of this IOCTL), Xen will defer grant
reference
removal and will retry later until those are free.

Hope this helps understand how buffers are synchronously deleted in

case

of xen-zcopy with a single protocol command.

I think the above logic can also be re-used by the hyper-dmabuf driver

with

some additional work:

1. xen-zcopy can be split into 2 parts and extend:
1.1. Xen gntdev driver [4], [5] to allow creating dma-buf from grefs and
vise versa,

I don't know much about the dma-buf implementation in Linux, but
gntdev is a user-space device, and AFAICT user-space applications
don't have any notion of dma buffers. How are such buffers useful for
user-space? Why can't this just be cal

Re: [PATCH hwc v2 12/18] drm_hwcomposer: Add utility function to create an initialized composition

On Tue, Apr 17, 2018 at 12:37:15PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:23PM +0100, Alexandru Gheorghe wrote:
> > There is a lot of boilerplate for creating an initialized
> > drmdisplaycomposition. This patch gathers that in a separate method.
> > 
> > Signed-off-by: Alexandru Gheorghe 
> > ---
> >  drmdisplaycompositor.cpp | 23 +++
> >  drmdisplaycompositor.h   |  2 ++
> >  2 files changed, 25 insertions(+)
> > 
> > diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
> > index e556e86..6e5be24 100644
> > --- a/drmdisplaycompositor.cpp
> > +++ b/drmdisplaycompositor.cpp
> > @@ -221,6 +221,7 @@ int DrmDisplayCompositor::Init(DrmResources *drm, int 
> > display) {
> >  ALOGE("Failed to initialize drm compositor lock %d\n", ret);
> >  return ret;
> >}
> > +  planner_ = Planner::CreateInstance(drm);
> 
> What's this?

We need a planner for the function bellow, I could re-write this to be
an argument of CreateComposition if you think that helps.

> 
> >  
> >initialized_ = true;
> >return 0;
> > @@ -231,6 +232,28 @@ std::unique_ptr 
> > DrmDisplayCompositor::CreateComposition()
> >return std::unique_ptr(new 
> > DrmDisplayComposition());
> >  }
> >  
> > +std::unique_ptr
> > +DrmDisplayCompositor::CreateInitializedComposition() const {
> > +  DrmCrtc *crtc = drm_->GetCrtcForDisplay(display_);
> > +  if (!crtc) {
> > +ALOGE("Failed to find crtc for display = %d", display_);
> > +return std::unique_ptr();
> > +  }
> > +  std::unique_ptr comp = CreateComposition();
> > +  std::shared_ptr importer =
> > +  drm_->resource_manager()->GetImporter(display_);
> > +  if (!importer) {
> > +ALOGE("Failed to find resources for display = %d", display_);
> > +return std::unique_ptr();
> > +  }
> > +  int ret = comp->Init(drm_, crtc, importer.get(), planner_.get(), 0);
> > +  if (ret) {
> > +ALOGE("Failed to init composition for display = %d", display_);
> > +return std::unique_ptr();
> > +  }
> > +  return comp;
> > +}
> > +
> 
> This seems sufficiently small that you can squash it into the patch that uses
> it. The same can be said for some of the other "Add function to do X" which
> don't use the function.
> 
> >  std::tuple
> >  DrmDisplayCompositor::GetActiveModeResolution() {
> >DrmConnector *connector = drm_->GetConnectorForDisplay(display_);
> > diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
> > index f1965fb..ccaffb4 100644
> > --- a/drmdisplaycompositor.h
> > +++ b/drmdisplaycompositor.h
> > @@ -87,6 +87,7 @@ class DrmDisplayCompositor {
> >int Init(DrmResources *drm, int display);
> >  
> >std::unique_ptr CreateComposition() const;
> > +  std::unique_ptr CreateInitializedComposition() 
> > const;
> >int ApplyComposition(std::unique_ptr composition);
> >int Composite();
> >int SquashAll();
> > @@ -155,6 +156,7 @@ class DrmDisplayCompositor {
> >// we need to reset them on every Dump() call.
> >mutable uint64_t dump_frames_composited_;
> >mutable uint64_t dump_last_timestamp_ns_;
> > +  std::unique_ptr planner_;
> >  };
> >  }
> >  
> > -- 
> > 2.7.4
> > 
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

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

[Bug 104520] Intermittent X crashes: GPU HANG: ecode 9:0:0x85dffffb, in Xorg [443], reason: Hang on rcs0, action: reset

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

--- Comment #21 from hfe...@cannon.com ---
Created attachment 138905
  --> https://bugs.freedesktop.org/attachment.cgi?id=138905&action=edit
/sys/class/drm/card0/error

-- 
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 104520] Intermittent X crashes: GPU HANG: ecode 9:0:0x85dffffb, in Xorg [443], reason: Hang on rcs0, action: reset

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

--- Comment #20 from hfe...@cannon.com ---
same problem here.
I am using Intel(R) Celeron(R) CPU  N3160
reproduced on Linux (kernel) 4.16.2 and 4.14.34, (Mesa 17.3.8)
problem can be reproduced by starting any application using OPEN GL ES

root@ca-linux:/home/cannon$ glmark2-es2-drm 
===
glmark2 2014.03
===
OpenGL Information
GL_VENDOR: Intel Open Source Technology Center
GL_RENDERER:   Mesa DRI Intel(R) HD Graphics 400 (Braswell) 
GL_VERSION:OpenGL ES 3.1 Mesa 17.3.8
===
[build] use-vbo=false:i965: Failed to submit batchbuffer: Input/output error

dmesg output:
[   38.859784] [drm] GPU HANG: ecode 8:0:0xe757feff, in glmark2-es2-drm [346],
reason: Hang on rcs0, action: reset
[   38.859788] [drm] GPU hangs can indicate a bug anywhere in the entire gfx
stack, including userspace.
[   38.859789] [drm] Please file a _new_ bug report on bugs.freedesktop.org
against DRI -> DRM/Intel
[   38.859791] [drm] drm/i915 developers can then reassign to the right
component if it's not a kernel issue.
[   38.859792] [drm] The gpu crash dump is required to analyze gpu hangs, so
please always attach it.
[   38.859794] [drm] GPU crash dump saved to /sys/class/drm/card0/error
[   38.859869] i915 :00:02.0: Resetting rcs0 after gpu hang
[   41.905511] i915 :00:02.0: Resetting chip after gpu hang
[   41.952424] asynchronous wait on fence i915:glmark2-es2-drm[346]/2:2 timed
out
[   47.072637] i915 :00:02.0: i915_reset_device timed out, cancelling all
in-flight rendering.
[   51.372311] i915 :00:02.0: Failed to reset chip

-- 
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 hwc v2 10/18] drm_hwcomposer: hwcutils: Add function for cloning a DrmHwcLayer

On Tue, Apr 17, 2018 at 12:14:14PM -0400, Sean Paul wrote:
> On Wed, Apr 11, 2018 at 04:22:21PM +0100, Alexandru Gheorghe wrote:
> > When doing flattening of a composition on a different CRTC we need to be
> > able to clone a layer in order to import it and then pass it to another 
> > CRTC.
> > 
> > Signed-off-by: Alexandru Gheorghe 
> > ---
> >  drmhwcomposer.h |  1 +
> >  hwcutils.cpp| 11 +++
> >  2 files changed, 12 insertions(+)
> > 
> > diff --git a/drmhwcomposer.h b/drmhwcomposer.h
> > index f8440fb..b256caf 100644
> > --- a/drmhwcomposer.h
> > +++ b/drmhwcomposer.h
> > @@ -150,6 +150,7 @@ struct DrmHwcLayer {
> >  
> >int InitFromHwcLayer(hwc_layer_1_t *sf_layer, Importer *importer,
> > const gralloc_module_t *gralloc);
> > +  int PopulateFromDrmHwcLayer(DrmHwcLayer *layer);
> >int ImportBuffer(Importer *importer, const gralloc_module_t *gralloc);
> >  
> >void SetTransform(int32_t sf_transform);
> > diff --git a/hwcutils.cpp b/hwcutils.cpp
> > index 53a7d82..ff37c3b 100644
> > --- a/hwcutils.cpp
> > +++ b/hwcutils.cpp
> > @@ -149,6 +149,17 @@ int DrmHwcLayer::InitFromHwcLayer(hwc_layer_1_t 
> > *sf_layer, Importer *importer,
> >return ImportBuffer(importer, gralloc);
> >  }
> >  
> > +int DrmHwcLayer::PopulateFromDrmHwcLayer(DrmHwcLayer *src_layer) {
> > +  blending = src_layer->blending;
> > +  sf_handle = src_layer->sf_handle;
> > +  acquire_fence = dup(src_layer->acquire_fence.get());
> 
> Hmm, I think this is the only place where we duplicate a UniqueFd. I _think_
> this will be Ok, but do you need to use this? Could you instead defer trying 
> to
> flatten if the original acquire_fence hasn't fired? Also, you don't check the
> return value.

Thinking about it, I don't think there is any way we could try
flattening, without acquire_fence being already fired.

> 
> > +  display_frame = src_layer->display_frame;
> > +  alpha = src_layer->alpha;
> > +  source_crop = src_layer->source_crop;
> > +  transform = src_layer->transform;
> 
> It also doesn't seem like you're populating all of the fields so the function
> name is misleading.
> 

Which one are you referring to? This follows the properties copied by
HwcLayer::PopulateDrmLayer which I assume was correct.

> 
> > +  return 0;
> > +}
> > +
> >  int DrmHwcLayer::ImportBuffer(Importer *importer,
> >const gralloc_module_t *gralloc) {
> >int ret = buffer.ImportBuffer(sf_handle, importer);
> > -- 
> > 2.7.4
> > 
> 
> -- 
> Sean Paul, Software Engineer, Google / Chromium OS

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

Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver

On 04/18/2018 01:18 PM, Paul Durrant wrote:

-Original Message-
From: Xen-devel [mailto:xen-devel-boun...@lists.xenproject.org] On Behalf
Of Roger Pau Monné
Sent: 18 April 2018 11:11
To: Oleksandr Andrushchenko 
Cc: jgr...@suse.com; Artem Mygaiev ;
Dongwon Kim ; airl...@linux.ie;
oleksandr_andrushche...@epam.com; linux-ker...@vger.kernel.org; dri-
de...@lists.freedesktop.org; Potrola, MateuszX
; xen-de...@lists.xenproject.org;
daniel.vet...@intel.com; boris.ostrov...@oracle.com; Matt Roper

Subject: Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy
helper DRM driver

On Wed, Apr 18, 2018 at 11:01:12AM +0300, Oleksandr Andrushchenko
wrote:

On 04/18/2018 10:35 AM, Roger Pau Monné wrote:

On Wed, Apr 18, 2018 at 09:38:39AM +0300, Oleksandr Andrushchenko

wrote:

On 04/17/2018 11:57 PM, Dongwon Kim wrote:

On Tue, Apr 17, 2018 at 09:59:28AM +0200, Daniel Vetter wrote:

On Mon, Apr 16, 2018 at 12:29:05PM -0700, Dongwon Kim wrote:

3.2 Backend exports dma-buf to xen-front

In this case Dom0 pages are shared with DomU. As before, DomU can

only write

to these pages, not any other page from Dom0, so it can be still

considered

safe.
But, the following must be considered (highlighted in xen-front's Kernel
documentation):
   - If guest domain dies then pages/grants received from the backend

cannot

     be claimed back - think of it as memory lost to Dom0 (won't be used

for

any
     other guest)
   - Misbehaving guest may send too many requests to the backend

exhausting

     its grant references and memory (consider this from security POV).

As the

     backend runs in the trusted domain we also assume that it is trusted

as

well,
     e.g. must take measures to prevent DDoS attacks.

I cannot parse the above sentence:

"As the backend runs in the trusted domain we also assume that it is
trusted as well, e.g. must take measures to prevent DDoS attacks."

What's the relation between being trusted and protecting from DoS
attacks?

I mean that we trust the backend that it can prevent Dom0
from crashing in case DomU's frontend misbehaves, e.g.
if the frontend sends too many memory requests etc.

In any case, all? PV protocols are implemented with the frontend
sharing pages to the backend, and I think there's a reason why this
model is used, and it should continue to be used.

This is the first use-case above. But there are real-world
use-cases (embedded in my case) when physically contiguous memory
needs to be shared, one of the possible ways to achieve this is
to share contiguous memory from Dom0 to DomU (the second use-case

above)

Having to add logic in the backend to prevent such attacks means
that:

   - We need more code in the backend, which increases complexity and
 chances of bugs.
   - Such code/logic could be wrong, thus allowing DoS.

You can live without this code at all, but this is then up to
backend which may make Dom0 down because of DomU's frontend doing

evil

things

IMO we should design protocols that do not allow such attacks instead
of having to defend against them.

4. xen-front/backend/xen-zcopy synchronization

4.1. As I already said in 2) all the inter VM communication happens

between

xen-front and the backend, xen-zcopy is NOT involved in that.
When xen-front wants to destroy a display buffer (dumb/dma-buf) it

issues a

XENDISPL_OP_DBUF_DESTROY command (opposite to

XENDISPL_OP_DBUF_CREATE).

This call is synchronous, so xen-front expects that backend does free

the

buffer pages on return.

4.2. Backend, on XENDISPL_OP_DBUF_DESTROY:
    - closes all dumb handles/fd's of the buffer according to [3]
    - issues DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE IOCTL to xen-

zcopy to make

sure
      the buffer is freed (think of it as it waits for dma-buf->release
callback)

So this zcopy thing keeps some kind of track of the memory usage? Why
can't the user-space backend keep track of the buffer usage?

Because there is no dma-buf UAPI which allows to track the buffer life cycle
(e.g. wait until dma-buf's .release callback is called)

    - replies to xen-front that the buffer can be destroyed.
This way deletion of the buffer happens synchronously on both Dom0

and DomU

sides. In case if DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE returns

with time-out

error
(BTW, wait time is a parameter of this IOCTL), Xen will defer grant
reference
removal and will retry later until those are free.

Hope this helps understand how buffers are synchronously deleted in

case

of xen-zcopy with a single protocol command.

I think the above logic can also be re-used by the hyper-dmabuf driver

with

some additional work:

1. xen-zcopy can be split into 2 parts and extend:
1.1. Xen gntdev driver [4], [5] to allow creating dma-buf from grefs and
vise versa,

I don't know much about the dma-buf implementation in Linux, but
gntdev is a user-space device, and AFAICT user-space applications
don't have any notion of dma buffers. How are such buffers useful for
user-space? Why can't this just be cal

Re: mi0283qt driver issues

2018-04-18 Thread Noralf Trønnes



Den 16.04.2018 18.44, skrev Tom Callaway:

Noralf,

Thanks for the guidance on how to use the mi0283qt driver to run the
panel included in the Adafruit 2.8" capacitive PiTFT Plus (running on
top of a Raspberry Pi 3).

I am working with 4.16.0 (Fedora 28). I can get the driver to load, and
have console output appearing on it, but I have run into some issues
that I am unclear how to resolve.

Specifically:

1) If I change the rotation value to anything other than 90 (0x5a), the
screen output from the console is corrupt.


It turns out that the reason for this is that the pitft has a hw reset
circuit that resets only on power-on and not on each reboot through a
gpio like rpi-display does.

When the driver is enabling the display pipeline it checks if the
controller is already initialized, and skips setup if it is. This means
that the rotation value is only applied after power-on on the pitft and
not on each reboot.

Turning off the "display" before rebooting should also work:

$ # change rotation value
$ echo "28" | sudo tee /sys/kernel/debug/dri/0/command
$ sudo reboot

Rotation/mirror/flip/bgr is controlled by commmand 0x36. The current
value is returned by command 0x0b:

$ sudo cat /sys/kernel/debug/dri/0/command | grep "0b:"
0b: f8

So to fix this in the driver the rotation value has to always be applied
regardless of display "on" state. This can be done by moving the rotation
code in mi0283qt_enable() down after the out: label.


2) The resolution is set to 240x320, not the 320x240 that I expected.


mi0283qt doesn't match fb_ili9340 when it comes to rotation.
rotation=90 on mi0283qt is 240x320
rotate=90 on fb_ili9340 is 320x240


3) pygame cannot run a display.set_mode call, always returning:

No video mode large enough for 240x320

When I run this python code:

import os
import pygame
os.putenv('SDL_VIDEODRIVER', 'fbcon')
os.putenv('SDL_FBDEV'  , '/dev/fb1')
pygame.init()
list = pygame.display.list_modes()
print "List of modes: %s" % (list)

It shows an empty set ("List of modes: []").


I couldn't find any source code for the sdl fbcon driver, so I don't know
how it queries for modes.

Noralf.



Do you have any advice on how to resolve these issues?

Thanks in advance,

~tom

For debugging purposes:

I have the following device tree entry for the device (taken from dtc
-Ifs /proc/device-tree):

pitft@0 {
 compatible = "multi-inno,mi0283qt";
 buswidth = <0x8>;
 rotation = <0x5a>;
 bgr;
 fps = <0x1e>;
 reg = <0x0>;
 pinctrl-0 = <0x10>;
 debug = <0x0>;
 dc-gpios = <0xf 0x19 0x0>;
 spi-max-frequency = <0x3d09000>;
 pinctrl-names = "default";
};

*** modetest output ***

[root@localhost ~]# modetest -M "mi0283qt" -c
Connectors:
id  encoder status  namesize (mm)   modes   encoders
28  31  connected   Virtual-1   43x58   1   31
   modes:
name refresh (Hz) hdisp hss hse htot vdisp vss vse vtot)
   240x320 0 240 240 240 240 320 320 320 320 1 flags: ; type: preferred,
driver
   props:
2 DPMS:
flags: enum
enums: On=0 Standby=1 Suspend=2 Off=3
value: 0
5 link-status:
flags: enum
enums: Good=0 Bad=1
value: 0
6 non-desktop:
flags: immutable range
values: 0 1
value: 0


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

Re: [PATCH hwc v2 04/18] drm_hwcomposer: Add resource manager class

2018-04-18 Thread Robert Foss




On 04/18/2018 12:12 PM, Alexandru-Cosmin Gheorghe wrote:

On Tue, Apr 17, 2018 at 06:08:06PM +0200, Robert Foss wrote:

Hey,

On 04/17/2018 05:33 PM, Sean Paul wrote:

On Wed, Apr 11, 2018 at 04:22:15PM +0100, Alexandru Gheorghe wrote:

Add a resource manager object that is responsible for detecting all
kms devices and allocates unique display numbers for every detected
display.

This is controlled by the value of hwc.drm.device property, if it ends
with a %, it will try to open minor devices until and error is detected.
E.g: /dev/dri/card%


I'm a bit on the fence as to whether to use the %, or add a new
hwc.drm.scan_devices property. I guess since we need to convey the path anyways
this is fine, but it really needs to be better documented.


I'm looking at this stuff in another series about DRM Node probing[1],
and I'll look into using properties to define what we are looking for.

But those properties won't be paths, but rather PCI IDs and driver vendor names.

As for what to do in the series, I don't have much of an opinion. But I'm
likely to try to change it in the not too distant future.


[1] https://www.spinics.net/lists/dri-devel/msg172496.html


Aren't this two complementary?
This series try to go through all available nodes and yours provides a
mechanism to check if file descriptor match some expectations.
You still have to open them somehow.


Yes, they are, I'm just noting that I'll prod around this area and change it in 
not too long, whatever decision is made here I'm likely to tweak it :)



Rob.









Additionally, this will be used for finding an available writeback
connector that will be used for the flattening of the currently
displayed scene.

Signed-off-by: Alexandru Gheorghe 
---
  Android.mk  |  1 +
  resourcemanager.cpp | 71 +
  resourcemanager.h   | 29 ++
  3 files changed, 101 insertions(+)
  create mode 100644 resourcemanager.cpp
  create mode 100644 resourcemanager.h

diff --git a/Android.mk b/Android.mk
index 1add286..736fe24 100644
--- a/Android.mk
+++ b/Android.mk
@@ -52,6 +52,7 @@ LOCAL_C_INCLUDES := \
  LOCAL_SRC_FILES := \
autolock.cpp \
+   resourcemanager.cpp \
drmresources.cpp \
drmconnector.cpp \
drmcrtc.cpp \
diff --git a/resourcemanager.cpp b/resourcemanager.cpp
new file mode 100644
index 000..e7b654e
--- /dev/null
+++ b/resourcemanager.cpp
@@ -0,0 +1,71 @@
+#include "resourcemanager.h"
+#include 
+#include 
+
+namespace android {
+
+ResourceManager::ResourceManager() : gralloc_(NULL) {
+}
+
+int ResourceManager::Init() {
+  char path_pattern[PROPERTY_VALUE_MAX];
+  property_get("hwc.drm.device", path_pattern, "/dev/dri/card%");


We probably also don't want to default to scanning, since that might change
behavior in existing boards.


+
+  uint8_t minor = 0;


Please use unsigned/int instead of fixed-size types. Unless the number of bits
is relevant to use of the variable, let the compiler handle it.


+  int last_display_index = 0;


Could we just call this num_displays? It was kind of hard for me to reason
through this, especially when it's call "start_display_index" when you jump into
drm::Init(). I also think drm->Init's return pair should return
 instead of , so each time you call
Init(), you're adding to num_displays.


+  int last_char = strlen(path_pattern) - 1;
+  do {
+char path[PROPERTY_VALUE_MAX];


Please use string


+if (path_pattern[last_char] == '%') {
+  path_pattern[last_char] = '\0';
+  snprintf(path, PROPERTY_VALUE_MAX, "%s%d", path_pattern, minor);
+  path_pattern[last_char] = '%';


This doesn't work for minor > 10.


+} else {
+  snprintf(path, PROPERTY_VALUE_MAX, "%s", path_pattern);
+}
+std::unique_ptr drm = std::make_unique();
+last_display_index = drm->Init(this, path, last_display_index);
+if (last_display_index < 0) {
+  break;
+}
+std::shared_ptr importer;
+importer.reset(Importer::CreateInstance(drm.get()));
+if (!importer) {
+  ALOGE("Failed to create importer instance");
+  break;
+}
+importers_.push_back(importer);
+drms_.push_back(std::move(drm));
+minor++;
+last_display_index++;
+  } while (path_pattern[last_char] == '%');
+
+  if (!drms_.size()) {
+ALOGE("Failed to find any working drm device");
+return -EINVAL;
+  }
+
+  return hw_get_module(GRALLOC_HARDWARE_MODULE_ID,
+   (const hw_module_t **)&gralloc_);
+}


Consider the following:

int ResourceManager::AddDrmDevice(std::string path) {
   std::unique_ptr drm = std::make_unique();
   int displays_added, ret;
   std::tie(displays_added, ret) = drm->Init(path.c_str(), num_displays_);
   if (ret)
 return ret;

   std::shared_ptr importer;
   importer.reset(Importer::CreateInstance(drm.get()));
   if (!importer) {
 ALOGE("Failed to create importer instance");
 return -ENODEV;
   }

   importers_.push_back(importer);
   drms_.

Re: [PATCH hwc v2 04/18] drm_hwcomposer: Add resource manager class

On Tue, Apr 17, 2018 at 06:08:06PM +0200, Robert Foss wrote:
> Hey,
> 
> On 04/17/2018 05:33 PM, Sean Paul wrote:
> >On Wed, Apr 11, 2018 at 04:22:15PM +0100, Alexandru Gheorghe wrote:
> >>Add a resource manager object that is responsible for detecting all
> >>kms devices and allocates unique display numbers for every detected
> >>display.
> >>
> >>This is controlled by the value of hwc.drm.device property, if it ends
> >>with a %, it will try to open minor devices until and error is detected.
> >>E.g: /dev/dri/card%
> >
> >I'm a bit on the fence as to whether to use the %, or add a new
> >hwc.drm.scan_devices property. I guess since we need to convey the path 
> >anyways
> >this is fine, but it really needs to be better documented.
> 
> I'm looking at this stuff in another series about DRM Node probing[1],
> and I'll look into using properties to define what we are looking for.
> 
> But those properties won't be paths, but rather PCI IDs and driver vendor 
> names.
> 
> As for what to do in the series, I don't have much of an opinion. But I'm
> likely to try to change it in the not too distant future.
> 
> 
> [1] https://www.spinics.net/lists/dri-devel/msg172496.html

Aren't this two complementary? 
This series try to go through all available nodes and yours provides a
mechanism to check if file descriptor match some expectations.
You still have to open them somehow.

> 
> >
> >>
> >>Additionally, this will be used for finding an available writeback
> >>connector that will be used for the flattening of the currently
> >>displayed scene.
> >>
> >>Signed-off-by: Alexandru Gheorghe 
> >>---
> >>  Android.mk  |  1 +
> >>  resourcemanager.cpp | 71 
> >> +
> >>  resourcemanager.h   | 29 ++
> >>  3 files changed, 101 insertions(+)
> >>  create mode 100644 resourcemanager.cpp
> >>  create mode 100644 resourcemanager.h
> >>
> >>diff --git a/Android.mk b/Android.mk
> >>index 1add286..736fe24 100644
> >>--- a/Android.mk
> >>+++ b/Android.mk
> >>@@ -52,6 +52,7 @@ LOCAL_C_INCLUDES := \
> >>  LOCAL_SRC_FILES := \
> >>autolock.cpp \
> >>+   resourcemanager.cpp \
> >>drmresources.cpp \
> >>drmconnector.cpp \
> >>drmcrtc.cpp \
> >>diff --git a/resourcemanager.cpp b/resourcemanager.cpp
> >>new file mode 100644
> >>index 000..e7b654e
> >>--- /dev/null
> >>+++ b/resourcemanager.cpp
> >>@@ -0,0 +1,71 @@
> >>+#include "resourcemanager.h"
> >>+#include 
> >>+#include 
> >>+
> >>+namespace android {
> >>+
> >>+ResourceManager::ResourceManager() : gralloc_(NULL) {
> >>+}
> >>+
> >>+int ResourceManager::Init() {
> >>+  char path_pattern[PROPERTY_VALUE_MAX];
> >>+  property_get("hwc.drm.device", path_pattern, "/dev/dri/card%");
> >
> >We probably also don't want to default to scanning, since that might change
> >behavior in existing boards.
> >
> >>+
> >>+  uint8_t minor = 0;
> >
> >Please use unsigned/int instead of fixed-size types. Unless the number of 
> >bits
> >is relevant to use of the variable, let the compiler handle it.
> >
> >>+  int last_display_index = 0;
> >
> >Could we just call this num_displays? It was kind of hard for me to reason
> >through this, especially when it's call "start_display_index" when you jump 
> >into
> >drm::Init(). I also think drm->Init's return pair should return
> > instead of , so each time you call
> >Init(), you're adding to num_displays.
> >
> >>+  int last_char = strlen(path_pattern) - 1;
> >>+  do {
> >>+char path[PROPERTY_VALUE_MAX];
> >
> >Please use string
> >
> >>+if (path_pattern[last_char] == '%') {
> >>+  path_pattern[last_char] = '\0';
> >>+  snprintf(path, PROPERTY_VALUE_MAX, "%s%d", path_pattern, minor);
> >>+  path_pattern[last_char] = '%';
> >
> >This doesn't work for minor > 10.
> >
> >>+} else {
> >>+  snprintf(path, PROPERTY_VALUE_MAX, "%s", path_pattern);
> >>+}
> >>+std::unique_ptr drm = std::make_unique();
> >>+last_display_index = drm->Init(this, path, last_display_index);
> >>+if (last_display_index < 0) {
> >>+  break;
> >>+}
> >>+std::shared_ptr importer;
> >>+importer.reset(Importer::CreateInstance(drm.get()));
> >>+if (!importer) {
> >>+  ALOGE("Failed to create importer instance");
> >>+  break;
> >>+}
> >>+importers_.push_back(importer);
> >>+drms_.push_back(std::move(drm));
> >>+minor++;
> >>+last_display_index++;
> >>+  } while (path_pattern[last_char] == '%');
> >>+
> >>+  if (!drms_.size()) {
> >>+ALOGE("Failed to find any working drm device");
> >>+return -EINVAL;
> >>+  }
> >>+
> >>+  return hw_get_module(GRALLOC_HARDWARE_MODULE_ID,
> >>+   (const hw_module_t **)&gralloc_);
> >>+}
> >
> >Consider the following:
> >
> >int ResourceManager::AddDrmDevice(std::string path) {
> >   std::unique_ptr drm = std::make_unique();
> >   int displays_added, ret;
> >   std::tie(displays_added, ret) = drm->Init(path.c_str(), num_displays_);
> >   if (ret)

Re: [PATCH] drm: Print unadorned pointers

2018-04-18 Thread Maarten Lankhorst

Op 18-04-18 om 11:59 schreef Alexey Brodkin:
> On Wed, 2018-04-18 at 11:29 +0200, Maarten Lankhorst wrote:
>> Op 18-04-18 om 11:24 schreef Alexey Brodkin:
>>> After commit ad67b74 ("printk: hash addresses printed with %p")
>>> pointers are being hashed when printed. However, this makes
>>> debug output completely useless. Switch to %px in order to see the
>>> unadorned kernel pointers.
>>>
>>> This was done with the following one-liner:
>>>  find drivers/gpu/drm -type f -name "*.c" -exec sed -r -i 
>>> '/DRM_DEBUG|KERN_DEBUG|pr_debug/ s/%p\b/%px/g' {} +
>> So first we plug a kernel information leak hole, then we introduce it again? 
>> Seems like a terrible idea..
> Well security concerns are good but what about us poor kernel developers?
> Those debug prints are supposed to help us to deal with stuff we develop or 
> fix.
>
> Frankly I was quite surprised when first saw what was "unique hashed ID" 
> instead
> of real pointer value. And what's worse there's no way to get previous 
> behavior back.
> So now we have to manually patch sources here and there to get meaningful 
> data, right?
>
> I wouldn't bother sending this patch if there was Kconfig option reverting %p 
> behavior
> but that was never implemented.
>
> -Alexey

There's %pK for kernel pointers. It does what you want and can be toggled with 
kptr_restrict, without introducing security holes. :)

Perhaps split up this patch per driver, and handle the other variations as well?

DRM_NOTE DRM_WARN DRM_INFO DRM_ERROR.


~Maarten

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

[Bug 103743] Nier:Automata - "if" in fragment shader incorrectly evaluated, causes artifacts

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

--- Comment #12 from Samuel Pitoiset  ---
Any news on this? The two LLVM patches have been pushed ~2 weeks ago. Can you
still reproduce the issue? Thanks!

-- 
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: Print unadorned pointers

2018-04-18 Thread Maarten Lankhorst

Op 18-04-18 om 11:24 schreef Alexey Brodkin:
> After commit ad67b74 ("printk: hash addresses printed with %p")
> pointers are being hashed when printed. However, this makes
> debug output completely useless. Switch to %px in order to see the
> unadorned kernel pointers.
>
> This was done with the following one-liner:
>  find drivers/gpu/drm -type f -name "*.c" -exec sed -r -i 
> '/DRM_DEBUG|KERN_DEBUG|pr_debug/ s/%p\b/%px/g' {} +
So first we plug a kernel information leak hole, then we introduce it again? 
Seems like a terrible idea..
> Signed-off-by: Alexey Brodkin 
> Cc: Borislav Petkov 
> Cc: Tobin C. Harding 
> Cc: Alex Deucher 
> Cc: Andrey Grodzovsky 
> Cc: Arnd Bergmann 
> Cc: Benjamin Gaignard 
> Cc: Chen-Yu Tsai 
> Cc: Christian Gmeiner 
> Cc: "Christian König" 
> Cc: Cihangir Akturk 
> Cc: CK Hu 
> Cc: Daniel Vetter 
> Cc: Dave Airlie 
> Cc: David Airlie 
> Cc: "David (ChunMing) Zhou" 
> Cc: Gerd Hoffmann 
> Cc: Greg Kroah-Hartman 
> Cc: Gustavo Padovan 
> Cc: Harry Wentland 
> Cc: "Heiko Stübner" 
> Cc: Ingo Molnar 
> Cc: Jani Nikula 
> Cc: "Jerry (Fangzhi) Zuo" 
> Cc: Joonas Lahtinen 
> Cc: Krzysztof Kozlowski 
> Cc: "Leo (Sunpeng) Li" 
> Cc: Lucas Stach 
> Cc: Maarten Lankhorst 
> Cc: Matthias Brugger 
> Cc: Maxime Ripard 
> Cc: "Michel Dänzer" 
> Cc: Oded Gabbay 
> Cc: Philipp Zabel 
> Cc: Rob Clark 
> Cc: Rodrigo Vivi 
> Cc: Roger He 
> Cc: Roman Li 
> Cc: Russell King 
> Cc: Samuel Li 
> Cc: Sandy Huang 
> Cc: Sean Paul 
> Cc: Shirish S 
> Cc: Sinclair Yeh 
> Cc: Thomas Hellstrom 
> Cc: Tom Lendacky 
> Cc: Tony Cheng 
> Cc: Vincent Abriou 
> Cc: VMware Graphics 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-arm-...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> Cc: linux-media...@lists.infradead.org
> Cc: linux-rockc...@lists.infradead.org
> Cc: etna...@lists.freedesktop.org
> Cc: freedr...@lists.freedesktop.org
> Cc: amd-...@lists.freedesktop.org
> Cc: intel-...@lists.freedesktop.org
> Cc: virtualizat...@lists.linux-foundation.org
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c   | 14 +++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_display.c|  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c   |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c|  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_device.c| 10 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_doorbell.c  |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_events.c|  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c  |  2 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_process.c   |  4 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_queue.c | 18 -
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  | 14 +++
>  .../amd/display/amdgpu_dm/amdgpu_dm_mst_types.c|  2 +-
>  drivers/gpu/drm/armada/armada_gem.c| 12 +++---
>  drivers/gpu/drm/drm_atomic.c   | 44 
> +++---
>  drivers/gpu/drm/drm_bufs.c |  8 ++--
>  drivers/gpu/drm/drm_dp_mst_topology.c  |  4 +-
>  drivers/gpu/drm/drm_lease.c|  6 +--
>  drivers/gpu/drm/drm_lock.c |  2 +-
>  drivers/gpu/drm/drm_scatter.c  |  4 +-
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c  |  6 +--
>  drivers/gpu/drm/i810/i810_dma.c|  2 +-
>  drivers/gpu/drm/i915/i915_perf.c   |  2 +-
>  drivers/gpu/drm/i915/intel_display.c   |  2 +-
>  drivers/gpu/drm/i915/intel_guc_ct.c|  4 +-
>  drivers/gpu/drm/i915/intel_guc_submission.c|  2 +-
>  drivers/gpu/drm/i915/intel_uc_fw.c |  2 +-
>  drivers/gpu/drm/mediatek/mtk_drm_gem.c |  2 +-
>  drivers/gpu/drm/mga/mga_warp.c |  2 +-
>  drivers/gpu/drm/msm/msm_drv.c  |  4 +-
>  drivers/gpu/drm/qxl/qxl_cmd.c  |  4 +-
>  drivers/gpu/drm/qxl/qxl_fb.c   |  2 +-
>  drivers/gpu/drm/qxl/qxl_ttm.c  |  2 +-
>  drivers/gpu/drm/radeon/radeon_display.c|  2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c | 12 +++---
>  drivers/gpu/drm/radeon/radeon_object.c |  2 +-
>  drivers/gpu/drm/rockchip/rockchip_drm_fbdev.c  |  2 +-
>  drivers/gpu/drm/savage/savage_bci.c|  2 +-
>  drivers/gpu/drm/sti/sti_gdp.c  |  4 +-
>  drivers/gpu/drm/sti/sti_mixer.c|  2 +-
>  drivers/gpu/drm/sun4i/sun4i_crtc.c |  4 +-
>  drivers/gpu/drm/ttm/ttm_page_alloc.c   |  2 +-
>  drivers/gpu/drm/udl/udl_fb.c   |  2 +-
>  drivers/gpu/drm/via/via_dma.c  |  4 +-
>  drivers/gpu/drm/via/via_irq.c  |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_overlay.c|  2 +-
>  45 files changed, 120 insertions(+), 120 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c 
> b/drivers/gpu/drm/amd/amd

Re: [PATCH v6 28/30] drm/rockchip: Disable PSR from reboot notifier

2018-04-18 Thread Enric Balletbo Serra

Hi Andrzej, Tomasz

2018-04-16 15:12 GMT+02:00 Tomasz Figa :
> Hi Andrzej,
>
> On Mon, Apr 16, 2018 at 6:57 PM Andrzej Hajda  wrote:
>
>> On 05.04.2018 11:49, Enric Balletbo i Serra wrote:
>> > From: Tomasz Figa 
>> >
>> > It looks like the driver subsystem detaches devices from power domains
>> > at shutdown without consent of the drivers.
>
>> It looks bit strange. Could you elaborate more on it. Could you show the
>> code performing the detach?
>
> It not only looks strange, but it is strange. The code was present in 4.4:
>
> https://elixir.bootlin.com/linux/v4.4.128/source/drivers/base/platform.c#L553
>
> but was apparently removed in 4.5:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/base/platform.c?h=next-20180416&id=2d30bb0b3889adf09b342722b2ce596c0763bc93
>
> So we might not need this patch anymore.
>

Right, seems that we don't need this patch anymore, I'll do more few
tests and likely remove this patch from this series. Thanks for
catching this.

Best regards,
  Enric

> Best regards,
> Tomasz
> ___
> 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

[PATCH 2/2 v3] drm/pl111: Enable device-specific assigned memory

2018-04-18 Thread Linus Walleij

The Versatile Express has 8 MB of dedicated video RAM (VRAM)
on the motherboard, which is what we should be using for the
PL111 if available. On this platform, the memory backplane
is constructed so that only this memory will work properly
with the CLCD on the motherboard, using any other memory
area just gives random snow on the display.

The CA9 Versatile Express also has a PL111 instance on its
core tile that can address all memory, and this does not
have the restriction.

The memory is assigned to the device using the memory-region
device tree property and a "shared-dma-pool" reserved
memory pool like this:

reserved-memory {
#address-cells = <1>;
#size-cells = <1>;
ranges;

vram: vram@4800 {
compatible = "shared-dma-pool";
reg = <0x4800 0x0080>;
no-map;
};
};

clcd@1f000 {
compatible = "arm,pl111", "arm,primecell";
(...)
memory-region = <&vram>;
}·;

Cc: Liviu Dudau 
Cc: Mali DP Maintainers 
Signed-off-by: Linus Walleij 
---
ChangeLog v2->v3:
- Fix error path so we uref the memory properly.
- Augment the GEM buffer import to return an error pointer
  if we try to import a buffer when using device-assigned
  memory: we can only scan out the special memory and the
  GEM buffers are not copied but referenced by pointer.
ChangeLog v1->v2:
- Make sure to also call of_reserved_mem_device_release() at
  remove() and errorpath.
---
 drivers/gpu/drm/pl111/pl111_drm.h |  1 +
 drivers/gpu/drm/pl111/pl111_drv.c | 33 +++--
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/pl111/pl111_drm.h 
b/drivers/gpu/drm/pl111/pl111_drm.h
index 8639b2d4ddf7..ce4501d0ab48 100644
--- a/drivers/gpu/drm/pl111/pl111_drm.h
+++ b/drivers/gpu/drm/pl111/pl111_drm.h
@@ -79,6 +79,7 @@ struct pl111_drm_dev_private {
const struct pl111_variant_data *variant;
void (*variant_display_enable) (struct drm_device *drm, u32 format);
void (*variant_display_disable) (struct drm_device *drm);
+   bool use_device_memory;
 };
 
 int pl111_display_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/pl111/pl111_drv.c 
b/drivers/gpu/drm/pl111/pl111_drv.c
index 4621259d5387..5fcf21837746 100644
--- a/drivers/gpu/drm/pl111/pl111_drv.c
+++ b/drivers/gpu/drm/pl111/pl111_drv.c
@@ -60,6 +60,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -207,6 +208,23 @@ static int pl111_modeset_init(struct drm_device *dev)
return ret;
 }
 
+struct drm_gem_object *pl111_gem_import_sg_table(struct drm_device *dev,
+struct dma_buf_attachment 
*attach,
+struct sg_table *sgt)
+{
+   struct pl111_drm_dev_private *priv = dev->dev_private;
+
+   /*
+* When using device-specific reserved memory we can't import
+* DMA buffers: those are passed by reference in any global
+* memory and we can only handle a specific range of memory.
+*/
+   if (priv->use_device_memory)
+   return ERR_PTR(-EINVAL);
+
+   return drm_gem_cma_prime_import_sg_table(dev, attach, sgt);
+}
+
 DEFINE_DRM_GEM_CMA_FOPS(drm_fops);
 
 static struct drm_driver pl111_drm_driver = {
@@ -227,7 +245,7 @@ static struct drm_driver pl111_drm_driver = {
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
.gem_prime_import = drm_gem_prime_import,
-   .gem_prime_import_sg_table = drm_gem_cma_prime_import_sg_table,
+   .gem_prime_import_sg_table = pl111_gem_import_sg_table,
.gem_prime_export = drm_gem_prime_export,
.gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
 
@@ -257,6 +275,12 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
drm->dev_private = priv;
priv->variant = variant;
 
+   ret = of_reserved_mem_device_init(dev);
+   if (!ret) {
+   dev_info(dev, "using device-specific reserved memory\n");
+   priv->use_device_memory = true;
+   }
+
if (of_property_read_u32(dev->of_node, "max-memory-bandwidth",
 &priv->memory_bw)) {
dev_info(dev, "no max memory bandwidth specified, assume 
unlimited\n");
@@ -275,7 +299,8 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
priv->regs = devm_ioremap_resource(dev, &amba_dev->res);
if (IS_ERR(priv->regs)) {
dev_err(dev, "%s failed mmio\n", __func__);
-   return PTR_ERR(priv->regs);
+   ret = PTR_ERR(priv->regs);
+   goto dev_unref;
}
 
/* This may override some variant settings */
@@ -305,11 +330,14 @@ static int pl111_amba_probe(struct amba_device *amba_dev,
 
 dev_unref:
drm_dev_unref(drm);
+   of_reserved_mem_device_release(dev);
+
return ret;
 }
 
 s

[PATCH 1/2 v3] drm/pl111: Support the Versatile Express

2018-04-18 Thread Linus Walleij

The Versatile Express uses a special configuration controller
deeply embedded in the system motherboard FPGA to multiplex the
two to three (!) display controller instances out to the single
SiI9022 bridge.

Set up an extra file with the logic to probe to the FPGA mux
register on the system controller bus, then parse the device
tree to see if there is a CLCD or HDLCD instance on the core
tile (also known as the daughterboard) by looking in the
root of the device tree for compatible nodes.

- If there is a HDLCD on the core tile, and there is a driver
  for it, we exit probe and deactivate the motherboard CLCD.
  We do not touch the DVI mux in this case, to make sure we
  don't break HDLCD.

- If there is a CLCD on both the motherboard and the core tile
  (only the CA9 has this) the core tile CLCD takes precedence
  and get muxed to the DVI connector.

- Only if there is no working graphics on the core tile, the
  motherboard CLCD is probed and muxed to the DVI connector.

Core tile graphics should always take precedence as it can
address all memory and is also faster, however the motherboard
CLCD is good to have around for diagnostics and testing.

It is possible to test the motherboard CLCD by setting the
status = "disabled" property on the core tile CLCD or
HDLCD.

Scale down the Versatile Express to 16BPP so we can support a
1024x768 display despite the bus bandwidth restrictions on this
platform. (The motherboard CLCD supports slightly lower
resolution.)

Cc: Liviu Dudau 
Cc: Pawel Moll 
Signed-off-by: Linus Walleij 
---
ChangeLog v2->v3:
- Rewrite CLCD detection and mux priority logic, look in
  the device tree root for core tile graphics.
ChangeLog v1->v2:
- No changes just reposting rebased on mainline changes.
---
 drivers/gpu/drm/pl111/Makefile  |   1 +
 drivers/gpu/drm/pl111/pl111_versatile.c |  48 +++-
 drivers/gpu/drm/pl111/pl111_vexpress.c  | 127 
 drivers/gpu/drm/pl111/pl111_vexpress.h  |  22 ++
 4 files changed, 197 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c
 create mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h

diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile
index 9c5e8dba8ac6..19a8189dc54f 100644
--- a/drivers/gpu/drm/pl111/Makefile
+++ b/drivers/gpu/drm/pl111/Makefile
@@ -3,6 +3,7 @@ pl111_drm-y +=  pl111_display.o \
pl111_versatile.o \
pl111_drv.o
 
+pl111_drm-$(CONFIG_ARCH_VEXPRESS) += pl111_vexpress.o
 pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o
 
 obj-$(CONFIG_DRM_PL111) += pl111_drm.o
diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c 
b/drivers/gpu/drm/pl111/pl111_versatile.c
index 9302f516045e..569edf02a36a 100644
--- a/drivers/gpu/drm/pl111/pl111_versatile.c
+++ b/drivers/gpu/drm/pl111/pl111_versatile.c
@@ -1,12 +1,14 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include "pl111_versatile.h"
+#include "pl111_vexpress.h"
 #include "pl111_drm.h"
 
 static struct regmap *versatile_syscon_map;
@@ -22,6 +24,7 @@ enum versatile_clcd {
REALVIEW_CLCD_PB11MP,
REALVIEW_CLCD_PBA8,
REALVIEW_CLCD_PBX,
+   VEXPRESS_CLCD_V2M,
 };
 
 static const struct of_device_id versatile_clcd_of_match[] = {
@@ -53,6 +56,10 @@ static const struct of_device_id versatile_clcd_of_match[] = 
{
.compatible = "arm,realview-pbx-syscon",
.data = (void *)REALVIEW_CLCD_PBX,
},
+   {
+   .compatible = "arm,vexpress-muxfpga",
+   .data = (void *)VEXPRESS_CLCD_V2M,
+   },
{},
 };
 
@@ -286,12 +293,26 @@ static const struct pl111_variant_data pl111_realview = {
.fb_bpp = 16,
 };
 
+/*
+ * Versatile Express PL111 variant, again we just push the maximum
+ * BPP to 16 to be able to get 1024x768 without saturating the memory
+ * bus. The clockdivider also seems broken on the Versatile Express.
+ */
+static const struct pl111_variant_data pl111_vexpress = {
+   .name = "PL111 Versatile Express",
+   .formats = pl111_realview_pixel_formats,
+   .nformats = ARRAY_SIZE(pl111_realview_pixel_formats),
+   .fb_bpp = 16,
+   .broken_clockdivider = true,
+};
+
 int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private 
*priv)
 {
const struct of_device_id *clcd_id;
enum versatile_clcd versatile_clcd_type;
struct device_node *np;
struct regmap *map;
+   int ret;
 
np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match,
 &clcd_id);
@@ -301,7 +322,25 @@ int pl111_versatile_init(struct device *dev, struct 
pl111_drm_dev_private *priv)
}
versatile_clcd_type = (enum versatile_clcd)clcd_id->data;
 
-   map = syscon_node_to_regmap(np);
+   /* Versatile Express special handling */
+   if (versatile_clcd_type == VEXPRESS_CLCD_V2M) {
+   st

Re: GPU/DRM issue with DSI commands on msm 8916

2018-04-18 Thread Archit Taneja




On Wednesday 18 April 2018 01:58 PM, Daniel Mack wrote:

On Wednesday, April 18, 2018 10:06 AM, Archit Taneja wrote:

On Tuesday 17 April 2018 05:51 PM, Daniel Mack wrote:
Thanks for debugging this so thoroughly.


It shows an underlying problem in the msm driver's clock components
though, because without this patch, the clocks will be effectively
slightly off from what was requested. For instance, when
gcc_mdss_byte0_clk is configured to 5625 Hz by the msm drm driver,
the clk core will let the DSI PLL recalculate its actual rate which is
56246337 Hz. Hence, clk_set_rate() will always end up fiddling with the
knobs of that clock provider. That's what happens every time DSI
commands are sent, and that causes the image to flicker.


If I understood right, the main cause of the flicker is that we always
end up re-locking/reconfiguring the DSI PLL every time we send a command
(since dsi_link_clk_enable() is called in msm_dsi_host_xfer_prepare())?
The re-configuration results in a glitch on the DSI clock, which is
probably unacceptable for DSI Video mode transfer, especially for panels
that don't have their own timing generators, which rely entirely on
DSI clock lanes for scanning out the pixel data.

According to you, the reason why the reconfiguration happens is because
the DSI PLL was never set exactly to 56.25 Mhz in the first place, and
the core clock framework notices a difference in the requested rate and
the current rate (56.246 Mhz), and goes ahead to configure the PLL
again when it's not needed. And this was averted in the downstream
patch you mentioned as a side affect?


Yes, exactly.


The same problem applies to other clocks too. dsi0vco_clk for example
will be 449970703 rather than the requested 45000 etc.



One easy way to get around this would be to not try to set the clock
rates every time we try to send a command. We just enable/disable them.


Yes, that could work as well, but how about rounding the rates in the
callback that exists for that purpose? We're off by a fraction of a
permille only, after all.


Sorry, forgot to respond to that in your last mail. I wasn't fully
clear about how we'd do it.

Do you mean that we call clk_round_rate() on the byte and pixel
clocks in dsi_link_clk_enable_6g() after we set the rates?

Archit




Thanks,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

[Bug 106110] vaapi encoding with gstreamer 1.14 doesn't work

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

--- Comment #5 from Andy Furniss  ---
Yea, I filed a bug for the gastreamer-vaapi regression

https://bugzilla.gnome.org/show_bug.cgi?id=795340

-- 
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 105317] The GPU Vega 56 was hang while try to pass #GraphicsFuzz shader15 test

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

--- Comment #9 from mikhail.v.gavri...@gmail.com ---
I don't thinks so because if it happens again by another reason GPU again will
hang.
I will be happy if it this case GPU reset code will present in driver.

-- 
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 105317] The GPU Vega 56 was hang while try to pass #GraphicsFuzz shader15 test

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

--- Comment #8 from Juan A. Suarez  ---
This already landed in Mesa. Can we close this as fixed?

-- 
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/6] drm/atmel-hlcdc: add support for connecting to tda998x HDMI encoder

On Wed, 18 Apr 2018 10:02:12 +0200
Peter Rosin  wrote:

> On 2018-04-18 09:36, Boris Brezillon wrote:
> > On Tue, 17 Apr 2018 15:10:51 +0200
> > Peter Rosin  wrote:
> >   
> >> When the of-graph points to a tda998x-compatible HDMI encoder, register
> >> as a component master and bind to the encoder/connector provided by
> >> the tda998x driver.  
> > 
> > Can't we do the opposite: make the tda998x driver expose its devices as
> > drm bridges. I'd rather not add another way to connect external
> > encoders (or bridges) to display controller drivers, especially since,
> > when I asked DRM maintainers/devs what was the good approach to
> > represent such external encoders they pointed me to the drm_bridge
> > interface.  
> 
> From the cover letter:
> 
> "However, I don't know if the tilcdc driver is interfacing with the
> tda998x driver in a sane and modern way"
> 
> So, which way is the future? Should bridges become components or should
> existing bridge-like components no longer be components? Are there others?

Well, what I've been told a while ago is that drm_bridge will take over
drm_encoder_slave and custom drm_encoder/drm_connector implementations
when it comes to representing bridges.

AFAIU, using the component framework to bind all elements of the
pipeline to the display controller is orthogonal to how you represent
elements in the pipeline. I mean, you could have a bridge that
registers as a component so that display controllers drivers who want
to use the component framework don't have to re-code the
component-to-bridge glue every time, and those who don't use the
component framework can still get access to the bridge.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 2/6] dt-bindings: display: atmel: optional video-interface of endpoints

On Wed, 18 Apr 2018 09:31:53 +0200
Peter Rosin  wrote:

> On 2018-04-18 09:16, Boris Brezillon wrote:
> > Hi Peter,
> > 
> > On Tue, 17 Apr 2018 15:10:48 +0200
> > Peter Rosin  wrote:
> >   
> >> With bus-type/bus-width properties in the endpoint nodes, the video-
> >> interface of the connection can be specified for cases where the
> >> heuristic fails to select the correct output mode. This can happen
> >> e.g. if not all RGB pins are routed on the PCB; the driver has no
> >> way of knowing this, and needs to be told explicitly.
> >>
> >> This is critical for the devices that have the "conflicting output
> >> formats" issue (SAM9N12, SAM9X5, SAMA5D3), since the most significant
> >> RGB bits move around depending on the selected output mode. For
> >> devices that do not have the "conflicting output formats" issue
> >> (SAMA5D2, SAMA5D4), this is completely irrelevant.
> >>
> >> Signed-off-by: Peter Rosin 
> >> ---
> >>  Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt | 8 
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt 
> >> b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
> >> index 82f2acb3d374..244b48869eb4 100644
> >> --- a/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
> >> +++ b/Documentation/devicetree/bindings/display/atmel/hlcdc-dc.txt
> >> @@ -15,6 +15,14 @@ Required children nodes:
> >>   to external devices using the OF graph reprensentation (see 
> >> ../graph.txt).
> >>   At least one port node is required.
> >>  
> >> +Optional properties in grandchild nodes:
> >> + Any endpoint grandchild node may specify a desired video interface
> >> + according to ../../media/video-interfaces.txt, specifically
> >> + - bus-type: must be <0>.
> >> + - bus-width: recognized values are <12>, <16>, <18> and <24>, and
> >> +   override any output mode selection hueristic, forcing "rgb444",  
> 
> heuristic, I'll fix that for v3, so please review as if it wasn't there...
> 
> >> +   "rgb565", "rgb666" and "rgb888" respectively.
> >> +  
> > 
> > Can you add an example or update the existing one to show how this
> > should be defined?  
> 
> For v3, I'll extend the binding with this after the preexisting example:
> 
> --8<-
> Example 2: With a video interface override to force rgb565, as above
> but with these changes/additions:
> 
> &hlcdc {
>   hlcdc-display-controller {
>   pinctrl-names = "default";
>   pinctrl-0 = <&pinctrl_lcd_base &pinctrl_lcd_rgb565>;
> 
>   port@0 {
>   hlcdc_panel_output: endpoint@0 {
>   bus-type = <0>;
>   bus-width = <16>;
>   };
>   };
>   };
> };
> --8<-
> 
> Is that a good plan, or should I perhaps duplicate the whole example?

Looks good to me, no need to add a new example.

Thanks,

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

Re: [PATCH v2 4/6] drm/atmel-hlcdc: support bus-width (12/16/18/24) in endpoint nodes

On Wed, 18 Apr 2018 09:46:09 +0200
Peter Rosin  wrote:

> On 2018-04-18 09:29, Boris Brezillon wrote:
> > On Tue, 17 Apr 2018 15:10:50 +0200
> > Peter Rosin  wrote:
> >   
> >> This beats the heuristic that the connector is involved in what format
> >> should be output for cases where this fails.
> >>
> >> E.g. if there is a bridge that changes format between the encoder and the
> >> connector, or if some of the RGB pins between the lcd controller and the
> >> encoder are not routed on the PCB.
> >>
> >> This is critical for the devices that have the "conflicting output
> >> formats" issue (SAM9N12, SAM9X5, SAMA5D3), since the most significant
> >> RGB bits move around depending on the selected output mode. For
> >> devices that do not have the "conflicting output formats" issue
> >> (SAMA5D2, SAMA5D4), this is completely irrelevant.
> >>
> >> Signed-off-by: Peter Rosin 
> >> ---
> >>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 85 
> >> --
> >>  1 file changed, 65 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> >> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> index d73281095fac..2e718959981e 100644
> >> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> >> @@ -19,12 +19,14 @@
> >>   */
> >>  
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >>  
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  
> >>  #include 
> >> @@ -226,6 +228,68 @@ static void atmel_hlcdc_crtc_atomic_enable(struct 
> >> drm_crtc *c,
> >>  #define ATMEL_HLCDC_RGB888_OUTPUT BIT(3)
> >>  #define ATMEL_HLCDC_OUTPUT_MODE_MASK  GENMASK(3, 0)
> >>  
> >> +static int atmel_hlcdc_connector_output_mode(struct drm_connector_state 
> >> *state)
> >> +{
> >> +  struct drm_connector *connector = state->connector;
> >> +  struct drm_display_info *info = &connector->display_info;
> >> +  unsigned int supported_fmts = 0;
> >> +  struct device_node *ep;
> >> +  int j;
> >> +
> >> +  /*
> >> +   * Use the connector index as an approximation of the
> >> +   * endpoint node index. We know it's true for our case
> >> +   * depending on the driver implementation.
> >> +   */
> >> +  ep = of_graph_get_endpoint_by_regs(connector->dev->dev->of_node, 0,
> >> + connector->index);
> >> +  
> > 
> > Hm, this sounds a bit fragile. Can't we have a reference to the of_node
> > attached to the connector? Or maybe we can parse this earlier and set a
> > constraint on the accepted modes.
> >   
> >> +  if (ep) {
> >> +  int bus_fmt = drm_of_media_bus_fmt(ep);  
> > 
> > Hm, you're extracting this piece of information from the DT every time
> > an atomic modeset is done. I'd really prefer to have this done once at  
> 
> Yes, not happy about it either. I looked for other sensible places too
> hook the info at probe time, but this was just the simplest. I'll take
> another look...
> 
> > probe time. Since this property is attached to the connector, maybe we
> > should overwrite the info->bus_formats[] array or mark some of its
> > entries as invalid.  
> 
> I find it very wrong to mix the connector format with what you want to
> output. In my mind it's a broken assumption that they are related. It is
> only correct for trivial cases. Also note my comment about the connector
> index and the endpoint index, they are only coincidentally the same
> based on our implementation. If the driver has more than one port or
> initializes endpoints out of order for some reason, this is no longer
> true.
> 
> I think it would be better to store this info somewhere near the encoder,
> since that is what I find closest to what I'm trying to change.

Totally agree with you on that: the connector has nothing to do with
how intermediate encoders/bridges exchange data with each other.

> 
> As I said, I'll take another look and see if I can hook this in at some
> other place.

Okay, cool.

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

Re: [PATCH] gpu: drm: tegra: Adding new typedef vm_fault_t

2018-04-18 Thread Thierry Reding

On Tue, Apr 17, 2018 at 07:17:55PM +0530, Souptick Joarder wrote:
> Use new return type vm_fault_t for fault handler. For
> now, this is just documenting that the function returns
> a VM_FAULT value rather than an errno. Once all instances
> are converted, vm_fault_t will become a distinct type.
> 
> Reference id -> 1c8f422059ae ("mm: change return type to
> vm_fault_t")
> 
> Previously vm_insert_page() returns err which driver
> mapped into VM_FAULT_* type. The new function 
> vmf_insert_page() will replace this inefficiency by
> returning VM_FAULT_* type.
> 
> Signed-off-by: Souptick Joarder 
> ---
>  drivers/gpu/drm/tegra/gem.c | 18 ++
>  1 file changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> index 49b9bf2..6121493 100644
> --- a/drivers/gpu/drm/tegra/gem.c
> +++ b/drivers/gpu/drm/tegra/gem.c
> @@ -422,14 +422,13 @@ int tegra_bo_dumb_create(struct drm_file *file, struct 
> drm_device *drm,
>   return 0;
>  }
> 
> -static int tegra_bo_fault(struct vm_fault *vmf)
> +static vm_fault_t tegra_bo_fault(struct vm_fault *vmf)
>  {
>   struct vm_area_struct *vma = vmf->vma;
>   struct drm_gem_object *gem = vma->vm_private_data;
>   struct tegra_bo *bo = to_tegra_bo(gem);
>   struct page *page;
>   pgoff_t offset;
> - int err;
> 
>   if (!bo->pages)
>   return VM_FAULT_SIGBUS;
> @@ -437,20 +436,7 @@ static int tegra_bo_fault(struct vm_fault *vmf)
>   offset = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
>   page = bo->pages[offset];
> 
> - err = vm_insert_page(vma, vmf->address, page);
> - switch (err) {
> - case -EAGAIN:
> - case 0:
> - case -ERESTARTSYS:
> - case -EINTR:
> - case -EBUSY:
> - return VM_FAULT_NOPAGE;
> -
> - case -ENOMEM:
> - return VM_FAULT_OOM;
> - }
> -
> - return VM_FAULT_SIGBUS;
> + return vmf_insert_page(vma, vmf->address, page);
>  }

This new function returns VM_FAULT_NOPAGE only for 0 and -EBUSY, whereas
we used to return VM_FAULT_NOPAGE for -EAGAIN, -ERESTARTSYS and -EINTR
as well. Was this previously wrong?

Thierry


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

Re: GPU/DRM issue with DSI commands on msm 8916

2018-04-18 Thread Archit Taneja

Hi Daniel,

On Tuesday 17 April 2018 05:51 PM, Daniel Mack wrote:

(cc Stephen)

Hi Archit,

On Monday, April 16, 2018 07:06 PM, Daniel Mack wrote:

On Monday, April 09, 2018 03:08 PM, Archit Taneja wrote:

You could comment out the pm_runtime_put_sync() calls in
drivers/gpu/drm/msm/dsi/dsi_host.c, in case some registers got
reset during put_sync and weren't restored correctly after get_sync().

That was my first suspicion too, but unfortunately, that's not the culprit.
I think it would be good to comment out the put_sync() calls in

drivers/gpu/drm/msm/mdp/mdp5/mdp5_mdss.c and
drivers/gpu/drm/msm/msm_drv.c too, since there is a parent-child
hierarchy between DSI
and the top level MDSS block. Commenting out the put_syncs() just
in put_sync() might still result in the MDSS GDSC to switch off.

I spent some more time debugging this today and it turns out that
calling into dsi_link_clk_enable() from msm_dsi_host_xfer_prepare() is
already causing the drop-outs, even when no command buffers, DMA
transfers etc. are involved. I then drilled further down and it showed
that at least

clk_set_rate(msm_host->byte_clk, msm_host->byte_clk_rate);

in dsi_link_clk_enable_6g() one of the culprits. If I don't touch the
clocks anymore after the initialization is done, everything is fine.

Okay, I finally found the bits between the two trees that make the
difference. It's a downstream patch that is included in the Linaro 4.9
branch but that's missing upstream and in their 4.14 branch:

https://git.linaro.org/landing-teams/working/qualcomm/kernel.git/commit/?h=release/qcomlt-4.9&id=4ce2d6108d

What this patch does as a side-effect is that is sets the clock's actual
rate to the requested rate (core->rate = core->new_rate), and that fixes
the problem I'm seeing.

Thanks for debugging this so thoroughly.

It shows an underlying problem in the msm driver's clock components
though, because without this patch, the clocks will be effectively
slightly off from what was requested. For instance, when
gcc_mdss_byte0_clk is configured to 5625 Hz by the msm drm driver,
the clk core will let the DSI PLL recalculate its actual rate which is
56246337 Hz. Hence, clk_set_rate() will always end up fiddling with the
knobs of that clock provider. That's what happens every time DSI
commands are sent, and that causes the image to flicker.

If I understood right, the main cause of the flicker is that we always
end up re-locking/reconfiguring the DSI PLL every time we send a command
(since dsi_link_clk_enable() is called in msm_dsi_host_xfer_prepare())?
The re-configuration results in a glitch on the DSI clock, which is
probably unacceptable for DSI Video mode transfer, especially for panels
that don't have their own timing generators, which rely entirely on
DSI clock lanes for scanning out the pixel data.

According to you, the reason why the reconfiguration happens is because
the DSI PLL was never set exactly to 56.25 Mhz in the first place, and
the core clock framework notices a difference in the requested rate and
the current rate (56.246 Mhz), and goes ahead to configure the PLL

again when it's not needed. And this was averted in the downstream
patch you mentioned as a side affect?

The same problem applies to other clocks too. dsi0vco_clk for example
will be 449970703 rather than the requested 45000 etc.

One easy way to get around this would be to not try to set the clock
rates every time we try to send a command. We just enable/disable them.
It would hide the real problem, though.

I guess a way to fix this properly would be to use
msm_dsi_pll_helper_clk_round_rate() to actually round the rates, but I'm
not sure.

Stephen,

Do you have any suggestions on how we can manage this? I.e we set a
clock rate, it goes through, but it's very slightly different than what
we asked for. The next time we try to set the same rate, the clock
provider driver goes through the whole ordeal of reconfiguring the
HW to reach to the same configuration.

Thanks,
Archit

Thanks,
Daniel

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

Re: [Xen-devel] [PATCH 0/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver


On 04/18/2018 10:35 AM, Roger Pau Monné wrote:

On Wed, Apr 18, 2018 at 09:38:39AM +0300, Oleksandr Andrushchenko wrote:

On 04/17/2018 11:57 PM, Dongwon Kim wrote:

On Tue, Apr 17, 2018 at 09:59:28AM +0200, Daniel Vetter wrote:

On Mon, Apr 16, 2018 at 12:29:05PM -0700, Dongwon Kim wrote:

3.2 Backend exports dma-buf to xen-front

In this case Dom0 pages are shared with DomU. As before, DomU can only write
to these pages, not any other page from Dom0, so it can be still considered
safe.
But, the following must be considered (highlighted in xen-front's Kernel
documentation):
  - If guest domain dies then pages/grants received from the backend cannot
    be claimed back - think of it as memory lost to Dom0 (won't be used for
any
    other guest)
  - Misbehaving guest may send too many requests to the backend exhausting
    its grant references and memory (consider this from security POV). As the
    backend runs in the trusted domain we also assume that it is trusted as
well,
    e.g. must take measures to prevent DDoS attacks.

I cannot parse the above sentence:

"As the backend runs in the trusted domain we also assume that it is
trusted as well, e.g. must take measures to prevent DDoS attacks."

What's the relation between being trusted and protecting from DoS
attacks?

I mean that we trust the backend that it can prevent Dom0
from crashing in case DomU's frontend misbehaves, e.g.
if the frontend sends too many memory requests etc.

In any case, all? PV protocols are implemented with the frontend
sharing pages to the backend, and I think there's a reason why this
model is used, and it should continue to be used.

This is the first use-case above. But there are real-world
use-cases (embedded in my case) when physically contiguous memory
needs to be shared, one of the possible ways to achieve this is
to share contiguous memory from Dom0 to DomU (the second use-case above)

Having to add logic in the backend to prevent such attacks means
that:

  - We need more code in the backend, which increases complexity and
chances of bugs.
  - Such code/logic could be wrong, thus allowing DoS.

You can live without this code at all, but this is then up to
backend which may make Dom0 down because of DomU's frontend doing evil 
things

4. xen-front/backend/xen-zcopy synchronization

4.1. As I already said in 2) all the inter VM communication happens between
xen-front and the backend, xen-zcopy is NOT involved in that.
When xen-front wants to destroy a display buffer (dumb/dma-buf) it issues a
XENDISPL_OP_DBUF_DESTROY command (opposite to XENDISPL_OP_DBUF_CREATE).
This call is synchronous, so xen-front expects that backend does free the
buffer pages on return.

4.2. Backend, on XENDISPL_OP_DBUF_DESTROY:
   - closes all dumb handles/fd's of the buffer according to [3]
   - issues DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE IOCTL to xen-zcopy to make
sure
     the buffer is freed (think of it as it waits for dma-buf->release
callback)

So this zcopy thing keeps some kind of track of the memory usage? Why
can't the user-space backend keep track of the buffer usage?

Because there is no dma-buf UAPI which allows to track the buffer life cycle
(e.g. wait until dma-buf's .release callback is called)

   - replies to xen-front that the buffer can be destroyed.
This way deletion of the buffer happens synchronously on both Dom0 and DomU
sides. In case if DRM_IOCTL_XEN_ZCOPY_DUMB_WAIT_FREE returns with time-out
error
(BTW, wait time is a parameter of this IOCTL), Xen will defer grant
reference
removal and will retry later until those are free.

Hope this helps understand how buffers are synchronously deleted in case
of xen-zcopy with a single protocol command.

I think the above logic can also be re-used by the hyper-dmabuf driver with
some additional work:

1. xen-zcopy can be split into 2 parts and extend:
1.1. Xen gntdev driver [4], [5] to allow creating dma-buf from grefs and
vise versa,

I don't know much about the dma-buf implementation in Linux, but
gntdev is a user-space device, and AFAICT user-space applications
don't have any notion of dma buffers. How are such buffers useful for
user-space? Why can't this just be called memory?

A dma-buf is seen by user-space as a file descriptor and you can
pass it to different drivers then. For example, you can share a buffer
used by a display driver for scanout with a GPU, to compose a picture
into it:
1. User-space (US) allocates a display buffer from display driver
2. US asks display driver to export the dma-buf which backs up that buffer,
US gets buffer's fd: dma_buf_fd
3. US asks GPU driver to import a buffer and provides it with dma_buf_fd
4. GPU renders contents into display buffer (dma_buf_fd)

Finally, this is indeed some memory, but a bit more [1]


Also, (with my FreeBSD maintainer hat) how is this going to translate
to other OSes? So far the operations performed by the gntdev device
are mostly OS-agnostic because this just map/unmap memory, and in fact
they are implemented by

[PATCH v2 2/2] i915: content-type property for HDMI connector

From: Stanislav Lisovskiy 

Added encoding of drm content_type property from
drm_connector_state within AVI infoframe in order to properly handle
external HDMI TV content-type setting.

Signed-off-by: Stanislav Lisovskiy 
---
 drivers/gpu/drm/i915/intel_atomic.c | 1 +
 drivers/gpu/drm/i915/intel_hdmi.c   | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
b/drivers/gpu/drm/i915/intel_atomic.c
index 40285d1b91b7..61ddb5871d8a 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct 
drm_connector *conn,
if (new_conn_state->force_audio != old_conn_state->force_audio ||
new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
new_conn_state->base.picture_aspect_ratio != 
old_conn_state->base.picture_aspect_ratio ||
+   new_conn_state->base.content_type != 
old_conn_state->base.content_type ||
new_conn_state->base.scaling_mode != 
old_conn_state->base.scaling_mode)
crtc_state->mode_changed = true;
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index ee929f31f7db..5cc72da9c086 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -491,6 +491,8 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder 
*encoder,
   
intel_hdmi->rgb_quant_range_selectable,
   is_hdmi2_sink);
 
+   frame.avi.content_type = connector->state->content_type;
+
/* TODO: handle pixel repetition for YCBCR420 outputs */
intel_write_infoframe(encoder, crtc_state, &frame);
 }
@@ -2065,6 +2067,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
intel_attach_aspect_ratio_property(connector);
+   drm_connector_attach_content_type_property(connector);
connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
-- 
2.17.0

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

[PATCH v2 1/2] drm: content-type property for HDMI connector

From: Stanislav Lisovskiy 

Added content_type property to
drm_connector_state in order to properly handle
external HDMI TV content-type setting.

Signed-off-by: Stanislav Lisovskiy 
---
 Documentation/gpu/kms-properties.csv |  1 +
 drivers/gpu/drm/drm_atomic.c |  4 +++
 drivers/gpu/drm/drm_connector.c  | 50 
 drivers/gpu/drm/drm_edid.c   |  1 +
 include/drm/drm_connector.h  | 11 ++
 include/drm/drm_mode_config.h|  5 +++
 include/uapi/drm/drm_mode.h  |  5 +++
 7 files changed, 77 insertions(+)

diff --git a/Documentation/gpu/kms-properties.csv 
b/Documentation/gpu/kms-properties.csv
index 6b28b014cb7d..7a02b2782f33 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -17,6 +17,7 @@ Owner Module/Drivers,Group,Property Name,Type,Property 
Values,Object attached,De
 ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0x",Connector,property to 
suggest an X offset for a connector
 ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to suggest an 
Y offset for a connector
 ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" 
}",Connector,TDB
+,Optional,"""content type""",ENUM,"{ ""Graphics"", ""Photo"", ""Cinema"", 
""Game"" }",Connector,TDB
 i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited 
16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is 
set, the hardware will be programmed with the result of the multiplication of 
CTM by the limited range matrix to ensure the pixels normaly in the range 
0..1.0 are remapped to the range 16/255..235/255."
 ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
 ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } 
etc.",Connector,TBD
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7d25c42f22db..72fd2a1c801f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1266,6 +1266,8 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
state->link_status = val;
} else if (property == config->aspect_ratio_property) {
state->picture_aspect_ratio = val;
+   } else if (property == config->content_type_property) {
+   state->content_type = val;
} else if (property == connector->scaling_mode_property) {
state->scaling_mode = val;
} else if (property == connector->content_protection_property) {
@@ -1351,6 +1353,8 @@ drm_atomic_connector_get_property(struct drm_connector 
*connector,
*val = state->link_status;
} else if (property == config->aspect_ratio_property) {
*val = state->picture_aspect_ratio;
+   } else if (property == config->content_type_property) {
+   *val = state->content_type;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
} else if (property == connector->content_protection_property) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b3cde897cd80..5633494f6d78 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -720,6 +720,13 @@ static const struct drm_prop_enum_list 
drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
 };
 
+static const struct drm_prop_enum_list drm_content_type_enum_list[] = {
+   { DRM_MODE_CONTENT_TYPE_GRAPHICS, "GRAPHICS" },
+   { DRM_MODE_CONTENT_TYPE_PHOTO, "PHOTO" },
+   { DRM_MODE_CONTENT_TYPE_CINEMA, "CINEMA" },
+   { DRM_MODE_CONTENT_TYPE_GAME, "GAME" },
+};
+
 static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {
{ DRM_MODE_PANEL_ORIENTATION_NORMAL,"Normal"},
{ DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down"   },
@@ -996,6 +1003,22 @@ int drm_mode_create_dvi_i_properties(struct drm_device 
*dev)
 }
 EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
 
+/**
+ * drm_connector_attach_content_type_property - attach content-type property
+ * @dev: DRM device
+ *
+ * Called by a driver the first time a HDMI connector is made.
+ */
+int drm_connector_attach_content_type_property(struct drm_connector *connector)
+{
+   if (!drm_mode_create_content_type_property(connector->dev))
+   drm_object_attach_property(&connector->base,
+   connector->dev->mode_config.content_type_property,
+   DRM_MODE_CONTENT_TYPE_GRAPHICS);
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_content_type_property);
+
 /**
  * drm_create_tv_properties - create TV specific connector properties
  * @dev: DRM device
@@ -1260,6 +1283,33 @@ int drm_mode_create_aspect_ratio_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
 
+/**
+ * drm_mode_create_content_type_property - c

[PATCH v2 0/2] Enabling content-type setting for HDMI displays.

From: Stanislav Lisovskiy 

Added content type setting property to drm_connector(part 1)
and enabled transmitting it with HDMI AVI infoframes
for i915(part 2).

rev 2:
Moved helper function which attaches content type property
to the drm core, as was suggested.
Removed redundant connector state initialization.

Stanislav Lisovskiy (2):
  drm: content-type property for HDMI connector
  i915: content-type property for HDMI connector

 Documentation/gpu/kms-properties.csv |  1 +
 drivers/gpu/drm/drm_atomic.c |  4 +++
 drivers/gpu/drm/drm_connector.c  | 50 
 drivers/gpu/drm/drm_edid.c   |  1 +
 drivers/gpu/drm/i915/intel_atomic.c  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c|  3 ++
 include/drm/drm_connector.h  | 11 ++
 include/drm/drm_mode_config.h|  5 +++
 include/uapi/drm/drm_mode.h  |  5 +++
 9 files changed, 81 insertions(+)

-- 
2.17.0

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

[PATCH v2 1/2] drm: content-type property for HDMI connector

From: Stanislav Lisovskiy 

Added content_type property to
drm_connector_state in order to properly handle
external HDMI TV content-type setting.

Signed-off-by: Stanislav Lisovskiy 
---
 Documentation/gpu/kms-properties.csv |  1 +
 drivers/gpu/drm/drm_atomic.c |  4 +++
 drivers/gpu/drm/drm_connector.c  | 50 
 drivers/gpu/drm/drm_edid.c   |  1 +
 include/drm/drm_connector.h  | 11 ++
 include/drm/drm_mode_config.h|  5 +++
 include/uapi/drm/drm_mode.h  |  5 +++
 7 files changed, 77 insertions(+)

diff --git a/Documentation/gpu/kms-properties.csv 
b/Documentation/gpu/kms-properties.csv
index 6b28b014cb7d..7a02b2782f33 100644
--- a/Documentation/gpu/kms-properties.csv
+++ b/Documentation/gpu/kms-properties.csv
@@ -17,6 +17,7 @@ Owner Module/Drivers,Group,Property Name,Type,Property 
Values,Object attached,De
 ,Virtual GPU,“suggested X”,RANGE,"Min=0, Max=0x",Connector,property to 
suggest an X offset for a connector
 ,,“suggested Y”,RANGE,"Min=0, Max=0x",Connector,property to suggest an 
Y offset for a connector
 ,Optional,"""aspect ratio""",ENUM,"{ ""None"", ""4:3"", ""16:9"" 
}",Connector,TDB
+,Optional,"""content type""",ENUM,"{ ""Graphics"", ""Photo"", ""Cinema"", 
""Game"" }",Connector,TDB
 i915,Generic,"""Broadcast RGB""",ENUM,"{ ""Automatic"", ""Full"", ""Limited 
16:235"" }",Connector,"When this property is set to Limited 16:235 and CTM is 
set, the hardware will be programmed with the result of the multiplication of 
CTM by the limited range matrix to ensure the pixels normaly in the range 
0..1.0 are remapped to the range 16/255..235/255."
 ,,“audio”,ENUM,"{ ""force-dvi"", ""off"", ""auto"", ""on"" }",Connector,TBD
 ,SDVO-TV,“mode”,ENUM,"{ ""NTSC_M"", ""NTSC_J"", ""NTSC_443"", ""PAL_B"" } 
etc.",Connector,TBD
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7d25c42f22db..72fd2a1c801f 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1266,6 +1266,8 @@ static int drm_atomic_connector_set_property(struct 
drm_connector *connector,
state->link_status = val;
} else if (property == config->aspect_ratio_property) {
state->picture_aspect_ratio = val;
+   } else if (property == config->content_type_property) {
+   state->content_type = val;
} else if (property == connector->scaling_mode_property) {
state->scaling_mode = val;
} else if (property == connector->content_protection_property) {
@@ -1351,6 +1353,8 @@ drm_atomic_connector_get_property(struct drm_connector 
*connector,
*val = state->link_status;
} else if (property == config->aspect_ratio_property) {
*val = state->picture_aspect_ratio;
+   } else if (property == config->content_type_property) {
+   *val = state->content_type;
} else if (property == connector->scaling_mode_property) {
*val = state->scaling_mode;
} else if (property == connector->content_protection_property) {
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index b3cde897cd80..5633494f6d78 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -720,6 +720,13 @@ static const struct drm_prop_enum_list 
drm_aspect_ratio_enum_list[] = {
{ DRM_MODE_PICTURE_ASPECT_16_9, "16:9" },
 };
 
+static const struct drm_prop_enum_list drm_content_type_enum_list[] = {
+   { DRM_MODE_CONTENT_TYPE_GRAPHICS, "GRAPHICS" },
+   { DRM_MODE_CONTENT_TYPE_PHOTO, "PHOTO" },
+   { DRM_MODE_CONTENT_TYPE_CINEMA, "CINEMA" },
+   { DRM_MODE_CONTENT_TYPE_GAME, "GAME" },
+};
+
 static const struct drm_prop_enum_list drm_panel_orientation_enum_list[] = {
{ DRM_MODE_PANEL_ORIENTATION_NORMAL,"Normal"},
{ DRM_MODE_PANEL_ORIENTATION_BOTTOM_UP, "Upside Down"   },
@@ -996,6 +1003,22 @@ int drm_mode_create_dvi_i_properties(struct drm_device 
*dev)
 }
 EXPORT_SYMBOL(drm_mode_create_dvi_i_properties);
 
+/**
+ * drm_connector_attach_content_type_property - attach content-type property
+ * @dev: DRM device
+ *
+ * Called by a driver the first time a HDMI connector is made.
+ */
+int drm_connector_attach_content_type_property(struct drm_connector *connector)
+{
+   if (!drm_mode_create_content_type_property(connector->dev))
+   drm_object_attach_property(&connector->base,
+   connector->dev->mode_config.content_type_property,
+   DRM_MODE_CONTENT_TYPE_GRAPHICS);
+   return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_content_type_property);
+
 /**
  * drm_create_tv_properties - create TV specific connector properties
  * @dev: DRM device
@@ -1260,6 +1283,33 @@ int drm_mode_create_aspect_ratio_property(struct 
drm_device *dev)
 }
 EXPORT_SYMBOL(drm_mode_create_aspect_ratio_property);
 
+/**
+ * drm_mode_create_content_type_property - c

[PATCH v2 2/2] i915: content-type property for HDMI connector

From: Stanislav Lisovskiy 

Added encoding of drm content_type property from
drm_connector_state within AVI infoframe in order to properly handle
external HDMI TV content-type setting.

Signed-off-by: Stanislav Lisovskiy 
---
 drivers/gpu/drm/i915/intel_atomic.c | 1 +
 drivers/gpu/drm/i915/intel_hdmi.c   | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c 
b/drivers/gpu/drm/i915/intel_atomic.c
index 40285d1b91b7..61ddb5871d8a 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -124,6 +124,7 @@ int intel_digital_connector_atomic_check(struct 
drm_connector *conn,
if (new_conn_state->force_audio != old_conn_state->force_audio ||
new_conn_state->broadcast_rgb != old_conn_state->broadcast_rgb ||
new_conn_state->base.picture_aspect_ratio != 
old_conn_state->base.picture_aspect_ratio ||
+   new_conn_state->base.content_type != 
old_conn_state->base.content_type ||
new_conn_state->base.scaling_mode != 
old_conn_state->base.scaling_mode)
crtc_state->mode_changed = true;
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
b/drivers/gpu/drm/i915/intel_hdmi.c
index ee929f31f7db..5cc72da9c086 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -491,6 +491,8 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder 
*encoder,
   
intel_hdmi->rgb_quant_range_selectable,
   is_hdmi2_sink);
 
+   frame.avi.content_type = connector->state->content_type;
+
/* TODO: handle pixel repetition for YCBCR420 outputs */
intel_write_infoframe(encoder, crtc_state, &frame);
 }
@@ -2065,6 +2067,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, 
struct drm_connector *c
intel_attach_force_audio_property(connector);
intel_attach_broadcast_rgb_property(connector);
intel_attach_aspect_ratio_property(connector);
+   drm_connector_attach_content_type_property(connector);
connector->state->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
 }
 
-- 
2.17.0

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

[PATCH v2 0/2] Enabling content-type setting for HDMI displays.

From: Stanislav Lisovskiy 

Added content type setting property to drm_connector(part 1)
and enabled transmitting it with HDMI AVI infoframes
for i915(part 2).

rev 2:
Moved helper function which attaches content type property
to the drm core, as was suggested.
Removed redundant connector state initialization.

Stanislav Lisovskiy (2):
  drm: content-type property for HDMI connector
  i915: content-type property for HDMI connector

 Documentation/gpu/kms-properties.csv |  1 +
 drivers/gpu/drm/drm_atomic.c |  4 +++
 drivers/gpu/drm/drm_connector.c  | 50 
 drivers/gpu/drm/drm_edid.c   |  1 +
 drivers/gpu/drm/i915/intel_atomic.c  |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c|  3 ++
 include/drm/drm_connector.h  | 11 ++
 include/drm/drm_mode_config.h|  5 +++
 include/uapi/drm/drm_mode.h  |  5 +++
 9 files changed, 81 insertions(+)

-- 
2.17.0

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

Re: [PATCH v2 6/6] drm/atmel-hlcdc: fix broken release date

On Tue, 17 Apr 2018 15:10:52 +0200
Peter Rosin  wrote:

> Bump the minor version while at it.
> 
> Signed-off-by: Peter Rosin 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index 8523c40fac94..aa48f287b5ca 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -763,9 +763,9 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>   .fops = &fops,
>   .name = "atmel-hlcdc",
>   .desc = "Atmel HLCD Controller DRM",
> - .date = "20141504",

I guess I used DDMM format :-).

> + .date = "20180409",
>   .major = 1,
> - .minor = 0,
> + .minor = 1,

Don't know what the strategy is regarding date and minor version
updates. I never had to update that before, so I guess it's not
important to userspace anyway.

>  };
>  
>  static int atmel_hlcdc_dc_drm_init(struct platform_device *pdev)

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

Re: RFC for a render API to support adaptive sync and VRR

2018-04-18 Thread Daniel Vetter

On Wed, Apr 18, 2018 at 5:58 AM, Keith Packard  wrote:
> Michel Dänzer  writes:
>> Time-based presentation seems to be the right approach for preventing
>> micro-stutter in games as well, Croteam developers have been researching
>> this.
>
> Both the Vulkan GOOGLE_display_timing extension and X11 Present
> extension offer the ability to specify the desired display time in
> seconds.
>
> Similarly, I'd suggest that the min/max display refresh rate values be
> advertised as time between frames rather than frames per second.
>
> I'd also encourage using a single unit for all of these values,
> preferably nanoseconds. Absolute times should all be referenced to
> CLOCK_MONOTONIC.

+1 on everything Keith said. I got somehow dragged in khr vk
discussions around preventing micro-stuttering, and consensus seems to
be that timestamps for scheduling frames is the way to go, most likely
absolute ones (not everything is running Linux unfortunately, so can't
go outright and claim it's guaranteed to be CLOCK_MONOTONIC).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 5/6] drm/atmel-hlcdc: add support for connecting to tda998x HDMI encoder

On Tue, 17 Apr 2018 15:10:51 +0200
Peter Rosin  wrote:

> When the of-graph points to a tda998x-compatible HDMI encoder, register
> as a component master and bind to the encoder/connector provided by
> the tda998x driver.

Can't we do the opposite: make the tda998x driver expose its devices as
drm bridges. I'd rather not add another way to connect external
encoders (or bridges) to display controller drivers, especially since,
when I asked DRM maintainers/devs what was the good approach to
represent such external encoders they pointed me to the drm_bridge
interface.

> 
> Signed-off-by: Peter Rosin 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c |  81 --
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h |  15 +++
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_output.c | 130 
> +++
>  3 files changed, 220 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> index c1ea5c36b006..8523c40fac94 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
> @@ -20,6 +20,7 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -568,10 +569,13 @@ static int atmel_hlcdc_dc_modeset_init(struct 
> drm_device *dev)
>  
>   drm_mode_config_init(dev);
>  
> - ret = atmel_hlcdc_create_outputs(dev);
> - if (ret) {
> - dev_err(dev->dev, "failed to create HLCDC outputs: %d\n", ret);
> - return ret;
> + if (!dc->is_componentized) {
> + ret = atmel_hlcdc_create_outputs(dev);
> + if (ret) {
> + dev_err(dev->dev,
> + "failed to create HLCDC outputs: %d\n", ret);
> + return ret;
> + }
>   }
>  
>   ret = atmel_hlcdc_create_planes(dev);
> @@ -586,6 +590,16 @@ static int atmel_hlcdc_dc_modeset_init(struct drm_device 
> *dev)
>   return ret;
>   }
>  
> + if (dc->is_componentized) {
> + ret = component_bind_all(dev->dev, dev);
> + if (ret < 0)
> + return ret;
> +
> + ret = atmel_hlcdc_add_component_encoder(dev);
> + if (ret < 0)
> + return ret;
> + }
> +
>   dev->mode_config.min_width = dc->desc->min_width;
>   dev->mode_config.min_height = dc->desc->min_height;
>   dev->mode_config.max_width = dc->desc->max_width;
> @@ -617,6 +631,9 @@ static int atmel_hlcdc_dc_load(struct drm_device *dev)
>   if (!dc)
>   return -ENOMEM;
>  
> + dc->is_componentized =
> + atmel_hlcdc_get_external_components(dev->dev, NULL) > 0;
> +
>   dc->wq = alloc_ordered_workqueue("atmel-hlcdc-dc", 0);
>   if (!dc->wq)
>   return -ENOMEM;
> @@ -751,7 +768,7 @@ static struct drm_driver atmel_hlcdc_dc_driver = {
>   .minor = 0,
>  };
>  
> -static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev)
> +static int atmel_hlcdc_dc_drm_init(struct platform_device *pdev)
>  {
>   struct drm_device *ddev;
>   int ret;
> @@ -779,7 +796,7 @@ static int atmel_hlcdc_dc_drm_probe(struct 
> platform_device *pdev)
>   return ret;
>  }
>  
> -static int atmel_hlcdc_dc_drm_remove(struct platform_device *pdev)
> +static int atmel_hlcdc_dc_drm_fini(struct platform_device *pdev)
>  {
>   struct drm_device *ddev = platform_get_drvdata(pdev);
>  
> @@ -790,6 +807,58 @@ static int atmel_hlcdc_dc_drm_remove(struct 
> platform_device *pdev)
>   return 0;
>  }
>  
> +static int atmel_hlcdc_bind(struct device *dev)
> +{
> + return atmel_hlcdc_dc_drm_init(to_platform_device(dev));
> +}
> +
> +static void atmel_hlcdc_unbind(struct device *dev)
> +{
> + struct drm_device *ddev = dev_get_drvdata(dev);
> +
> + /* Check if a subcomponent has already triggered the unloading. */
> + if (!ddev->dev_private)
> + return;
> +
> + atmel_hlcdc_dc_drm_fini(to_platform_device(dev));
> +}
> +
> +static const struct component_master_ops atmel_hlcdc_comp_ops = {
> + .bind = atmel_hlcdc_bind,
> + .unbind = atmel_hlcdc_unbind,
> +};
> +
> +static int atmel_hlcdc_dc_drm_probe(struct platform_device *pdev)
> +{
> + struct component_match *match = NULL;
> + int ret;
> +
> + ret = atmel_hlcdc_get_external_components(&pdev->dev, &match);
> + if (ret < 0)
> + return ret;
> + else if (ret)
> + return component_master_add_with_match(&pdev->dev,
> +&atmel_hlcdc_comp_ops,
> +match);
> + else
> + return atmel_hlcdc_dc_drm_init(pdev);
> +}
> +
> +static int atmel_hlcdc_dc_drm_remove(struct platform_device *pdev)
> +{
> + int ret;
> +
> + ret = atmel_hlcdc_get_external_components(&pdev->dev, NULL);
> + if (ret < 0)
> + return ret;
> +

Re: [PATCH v2 4/6] drm/atmel-hlcdc: support bus-width (12/16/18/24) in endpoint nodes

On Tue, 17 Apr 2018 15:10:50 +0200
Peter Rosin  wrote:

> This beats the heuristic that the connector is involved in what format
> should be output for cases where this fails.
> 
> E.g. if there is a bridge that changes format between the encoder and the
> connector, or if some of the RGB pins between the lcd controller and the
> encoder are not routed on the PCB.
> 
> This is critical for the devices that have the "conflicting output
> formats" issue (SAM9N12, SAM9X5, SAMA5D3), since the most significant
> RGB bits move around depending on the selected output mode. For
> devices that do not have the "conflicting output formats" issue
> (SAMA5D2, SAMA5D4), this is completely irrelevant.
> 
> Signed-off-by: Peter Rosin 
> ---
>  drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c | 85 
> --
>  1 file changed, 65 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c 
> b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> index d73281095fac..2e718959981e 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c
> @@ -19,12 +19,14 @@
>   */
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  #include 
> @@ -226,6 +228,68 @@ static void atmel_hlcdc_crtc_atomic_enable(struct 
> drm_crtc *c,
>  #define ATMEL_HLCDC_RGB888_OUTPUTBIT(3)
>  #define ATMEL_HLCDC_OUTPUT_MODE_MASK GENMASK(3, 0)
>  
> +static int atmel_hlcdc_connector_output_mode(struct drm_connector_state 
> *state)
> +{
> + struct drm_connector *connector = state->connector;
> + struct drm_display_info *info = &connector->display_info;
> + unsigned int supported_fmts = 0;
> + struct device_node *ep;
> + int j;
> +
> + /*
> +  * Use the connector index as an approximation of the
> +  * endpoint node index. We know it's true for our case
> +  * depending on the driver implementation.
> +  */
> + ep = of_graph_get_endpoint_by_regs(connector->dev->dev->of_node, 0,
> +connector->index);
> +

Hm, this sounds a bit fragile. Can't we have a reference to the of_node
attached to the connector? Or maybe we can parse this earlier and set a
constraint on the accepted modes.

> + if (ep) {
> + int bus_fmt = drm_of_media_bus_fmt(ep);

Hm, you're extracting this piece of information from the DT every time
an atomic modeset is done. I'd really prefer to have this done once at
probe time. Since this property is attached to the connector, maybe we
should overwrite the info->bus_formats[] array or mark some of its
entries as invalid.

> +
> + of_node_put(ep);
> +
> + if (bus_fmt < 0)
> + return bus_fmt;
> +
> + switch (bus_fmt) {
> + case 0:
> + break;
> + case MEDIA_BUS_FMT_RGB444_1X12:
> + return ATMEL_HLCDC_RGB444_OUTPUT;
> + case MEDIA_BUS_FMT_RGB565_1X16:
> + return ATMEL_HLCDC_RGB565_OUTPUT;
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + return ATMEL_HLCDC_RGB666_OUTPUT;
> + case MEDIA_BUS_FMT_RGB888_1X24:
> + return ATMEL_HLCDC_RGB888_OUTPUT;
> + default:
> + return -EINVAL;
> + }
> + }
> +
> + for (j = 0; j < info->num_bus_formats; j++) {
> + switch (info->bus_formats[j]) {
> + case MEDIA_BUS_FMT_RGB444_1X12:
> + supported_fmts |= ATMEL_HLCDC_RGB444_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB565_1X16:
> + supported_fmts |= ATMEL_HLCDC_RGB565_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB666_1X18:
> + supported_fmts |= ATMEL_HLCDC_RGB666_OUTPUT;
> + break;
> + case MEDIA_BUS_FMT_RGB888_1X24:
> + supported_fmts |= ATMEL_HLCDC_RGB888_OUTPUT;
> + break;
> + default:
> + break;
> + }
> + }
> +
> + return supported_fmts;
> +}
> +
>  static int atmel_hlcdc_crtc_select_output_mode(struct drm_crtc_state *state)
>  {
>   unsigned int output_fmts = ATMEL_HLCDC_OUTPUT_MODE_MASK;
> @@ -238,31 +302,12 @@ static int atmel_hlcdc_crtc_select_output_mode(struct 
> drm_crtc_state *state)
>   crtc = drm_crtc_to_atmel_hlcdc_crtc(state->crtc);
>  
>   for_each_new_connector_in_state(state->state, connector, cstate, i) {
> - struct drm_display_info *info = &connector->display_info;
>   unsigned int supported_fmts = 0;
> - int j;
>  
>   if (!cstate->crtc)
>   continue;
>  
> - for (j = 0; j < info->num_bus_formats; j++) {
> - switch (info->bus_formats[j]) {

Re: [PATCH] drm/xen-front: Remove CMA support