[Bug 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #12 from Hi-Angel  ---
(EDIT: replaced tabs with space in the table)

So, actually there're 4 different combinations:

 | usual run | nosb  |
HEAD |   fails   | passes|
HEAD eb8aa93c03 reverted |   passes  | fails |

With that said, I've attached dumps for all 4 configurations. But I do actually
recall that "nosb" option was buggy, see this
https://bugs.freedesktop.org/show_bug.cgi?id=93715

-- 
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 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #11 from Hi-Angel  ---
So, actually there're 4 different combinations:

 | usual run | nosb  |
HEAD |   fails   | passes|
HEAD eb8aa93c03 reverted |   passes  | fails |

With that said, I've attached dumps for all 4 configurations. But I do actually
recall that "nosb" option was buggy, see this
https://bugs.freedesktop.org/show_bug.cgi?id=93715

-- 
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 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #10 from Hi-Angel  ---
Not sure why, but I can't get it passing with "nosb" anymore :(

-- 
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 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #9 from Hi-Angel  ---
Hmm, wait. Now for some reason it fails with nosb also.

-- 
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 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #8 from Hi-Angel  ---
Created attachment 131063
  --> https://bugs.freedesktop.org/attachment.cgi?id=131063=edit
ps,vs,nosb dump HEAD with reverted eb8aa93c03

-- 
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 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #7 from Hi-Angel  ---
Created attachment 131062
  --> https://bugs.freedesktop.org/attachment.cgi?id=131062=edit
ps,vs,nosb dump HEAD

-- 
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 v5 07/10] drm: bridge: dw-hdmi: Add support for custom PHY configuration

[郑阳]

Hi, Jose Abreu:

Thanks for your patch, it works fine on RK3399 / RK3368。

在 2017年04月26日 19:04, Jose Abreu 写道:

Hi,


On 24-04-2017 08:46, 郑阳 wrote:

Hi, Laurent Pinchart:

在 2017年03月04日 01:20, Laurent Pinchart 写道:

From: Kieran Bingham 

The DWC HDMI TX controller interfaces with a companion PHY. While
Synopsys provides multiple standard PHYs, SoC vendors can also
integrate
a custom PHY.

Modularize PHY configuration to support vendor PHYs through
platform
data. The existing PHY configuration code was originally
written to
support the DWC HDMI 3D TX PHY, and seems to be compatible
with the DWC
MLP PHY. The HDMI 2.0 PHY will require a separate configuration
function.

Signed-off-by: Kieran Bingham

Signed-off-by: Laurent Pinchart

Tested-by: Neil Armstrong 
Reviewed-by: Jose Abreu 
---
Changes since v1:

- Check pdata->phy_configure in hdmi_phy_configure() to avoid
dereferencing NULL pointer.
---
   drivers/gpu/drm/bridge/dw-hdmi.c | 109
++-
   include/drm/bridge/dw_hdmi.h |   7 +++
   2 files changed, 81 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
b/drivers/gpu/drm/bridge/dw-hdmi.c
index cb2703862be2..b835d81bb471 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -118,6 +118,9 @@ struct dw_hdmi_phy_data {
   const char *name;
   unsigned int gen;
   bool has_svsret;
+int (*configure)(struct dw_hdmi *hdmi,
+ const struct dw_hdmi_plat_data *pdata,
+ unsigned long mpixelclock);
   };
 struct dw_hdmi {
@@ -860,8 +863,8 @@ static bool hdmi_phy_wait_i2c_done(struct
dw_hdmi *hdmi, int msec)
   return true;
   }
   -static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi,
unsigned short data,
- unsigned char addr)
+void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned
short data,
+   unsigned char addr)
   {
   hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0);
   hdmi_writeb(hdmi, addr, HDMI_PHY_I2CM_ADDRESS_ADDR);
@@ -873,6 +876,7 @@ static void hdmi_phy_i2c_write(struct
dw_hdmi *hdmi, unsigned short data,
   HDMI_PHY_I2CM_OPERATION_ADDR);
   hdmi_phy_wait_i2c_done(hdmi, 1000);
   }
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_write);
 static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi
*hdmi, bool enable)
   {
@@ -993,37 +997,67 @@ static int dw_hdmi_phy_power_on(struct
dw_hdmi *hdmi)
   return 0;
   }
   -static int hdmi_phy_configure(struct dw_hdmi *hdmi)
+/*
+ * PHY configuration function for the DWC HDMI 3D TX PHY.
Based on the available
+ * information the DWC MHL PHY has the same register layout
and is thus also
+ * supported by this function.
+ */
+static int hdmi_phy_configure_dwc_hdmi_3d_tx(struct dw_hdmi
*hdmi,
+const struct dw_hdmi_plat_data *pdata,
+unsigned long mpixelclock)
   {
-const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
-const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
   const struct dw_hdmi_mpll_config *mpll_config =
pdata->mpll_cfg;
   const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
   const struct dw_hdmi_phy_config *phy_config =
pdata->phy_config;
 /* PLL/MPLL Cfg - always match on final entry */
   for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
-if (hdmi->hdmi_data.video_mode.mpixelclock <=
-mpll_config->mpixelclock)
+if (mpixelclock <= mpll_config->mpixelclock)
   break;
 for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++)
-if (hdmi->hdmi_data.video_mode.mpixelclock <=
-curr_ctrl->mpixelclock)
+if (mpixelclock <= curr_ctrl->mpixelclock)
   break;
 for (; phy_config->mpixelclock != ~0UL; phy_config++)
-if (hdmi->hdmi_data.video_mode.mpixelclock <=
-phy_config->mpixelclock)
+if (mpixelclock <= phy_config->mpixelclock)
   break;
 if (mpll_config->mpixelclock == ~0UL ||
   curr_ctrl->mpixelclock == ~0UL ||
-phy_config->mpixelclock == ~0UL) {
-dev_err(hdmi->dev, "Pixel clock %d - unsupported by
HDMI\n",
-hdmi->hdmi_data.video_mode.mpixelclock);
+phy_config->mpixelclock == ~0UL)
   return -EINVAL;
-}
+
+dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce,
+  HDMI_3D_TX_PHY_CPCE_CTRL);
+dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp,
+  HDMI_3D_TX_PHY_GMPCTRL);
+dw_hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0],
+  HDMI_3D_TX_PHY_CURRCTRL);
+
+dw_hdmi_phy_i2c_write(hdmi, 0, HDMI_3D_TX_PHY_PLLPHBYCTRL);
+dw_hdmi_phy_i2c_write(hdmi,
HDMI_3D_TX_PHY_MSM_CTRL_CKO_SEL_FB_CLK,
+  HDMI_3D_TX_PHY_MSM_CTRL);
+
+dw_hdmi_phy_i2c_write(hdmi, 

Re: [PATCH -next] drm/nouveau/secboot/gm20b: fix the error return code in gm20b_secboot_tegra_read_wpr()

[Ben Skeggs]

On 04/26/2017 12:36 AM, Wei Yongjun wrote:

From: Wei Yongjun 

The error return code PTR_ERR(mc) is always 0 since mc is
equal to 0 in this error handling case.

Thank you!  I've merged the patch in my tree.

Ben.



Signed-off-by: Wei Yongjun 
---
 drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c 
b/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c
index b10ed59..30491d1 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/secboot/gm20b.c
@@ -48,7 +48,7 @@ gm20b_secboot_tegra_read_wpr(struct gm200_secboot *gsb, u32 
mc_base)
mc = ioremap(mc_base, 0xd00);
if (!mc) {
nvkm_error(>subdev, "Cannot map Tegra MC registers\n");
-   return PTR_ERR(mc);
+   return -ENOMEM;
}
sb->wpr_addr = ioread32_native(mc + MC_SECURITY_CARVEOUT2_BOM_0) |
  ((u64)ioread32_native(mc + MC_SECURITY_CARVEOUT2_BOM_HI_0) << 32);



___
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 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Michel Dänzer]

On 26/04/17 09:11 PM, Gerd Hoffmann wrote:
>   Hi,
> 
 Just to reiterate, this won't work for the radeon driver, which programs
 the GPU to use (effectively, per the current definition that these are
 little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
 DRM_FORMAT_BGRX with >= R600.
>>>
>>> Hmm, ok, how does bigendian fbdev emulation work on pre-R600 then?
>>
>> Using a GPU byte swapping mechanism which only affects CPU access to
>> video RAM.
> 
> That is done using the RADEON_TILING_SWAP_{16,32}BIT flag mentioned in
> another thread?

Right.


> What about dumb bos?  You've mentioned the swap flag isn't used for
> those.  Which implies they are in little endian byte order (both gpu and
> cpu view).

Right, AFAICT from looking at the code.


> Does the modesetting driver work correctly on that hardware?

Not sure.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/2] drm: arcpgu: Use crtc->mode_valid() callback

[Jose Abreu]

Now that we have a callback to check if crtc supports a given mode
we can use it in arcpgu so that we restrict the number of probbed
modes to the ones we can actually display.

This is specially useful because arcpgu crtc is responsible to set
a clock value in the commit() stage but unfortunatelly this clock
does not support all the needed ranges.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Alexey Brodkin 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 
---
 drivers/gpu/drm/arc/arcpgu_crtc.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/arc/arcpgu_crtc.c 
b/drivers/gpu/drm/arc/arcpgu_crtc.c
index ad9a959..dea7a18 100644
--- a/drivers/gpu/drm/arc/arcpgu_crtc.c
+++ b/drivers/gpu/drm/arc/arcpgu_crtc.c
@@ -64,6 +64,19 @@ static void arc_pgu_set_pxl_fmt(struct drm_crtc *crtc)
.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
+enum drm_mode_status arc_pgu_crtc_mode_valid(struct drm_crtc *crtc,
+struct drm_display_mode *mode)
+{
+   struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
+   long rate, clk_rate = mode->clock * 1000;
+
+   rate = clk_round_rate(arcpgu->clk, clk_rate);
+   if (rate != clk_rate)
+   return MODE_NOCLOCK;
+
+   return MODE_OK;
+}
+
 static void arc_pgu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
struct arcpgu_drm_private *arcpgu = crtc_to_arcpgu_priv(crtc);
@@ -158,6 +171,7 @@ static void arc_pgu_crtc_atomic_begin(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_helper_funcs arc_pgu_crtc_helper_funcs = {
+   .mode_valid = arc_pgu_crtc_mode_valid,
.mode_set   = drm_helper_crtc_mode_set,
.mode_set_base  = drm_helper_crtc_mode_set_base,
.mode_set_nofb  = arc_pgu_crtc_mode_set_nofb,
-- 
1.9.1


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

Re: [PATCH v2 02/21] libiscsi: Add an internal error code

[Christoph Hellwig]

On Tue, Apr 25, 2017 at 12:20:49PM -0600, Logan Gunthorpe wrote:
> This is a prep patch to add a new error code to libiscsi. We want to
> rework some kmap calls to be able to fail. When we do, we'd like to
> use this error code.

The kmap case in iscsi_tcp_segment_map can already fail.  Please add
handling of that failure to this patch, and we should get it merged
ASAP.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/2] drm: Introduce crtc->mode_valid() callback

[Jose Abreu]

Some crtc's may have restrictions in the mode they can display. In
this patch a new callback (crtc->mode_valid()) is introduced that
is called at the same stage of connector->mode_valid() callback.

This shall be implemented if the crtc has some sort of restriction
so that we don't probe modes that will fail in the commit() stage.
For example: A given crtc may be responsible to set a clock value.
If the clock can not produce all the values for the available
modes then this callback can be used to restrict the number of
probbed modes to only the ones that can be displayed.

If the crtc does not implement the callback then the behaviour will
remain the same. Also, for a given set of crtcs that can be bound to
the connector, if at least one can display the mode then the mode
will be probbed.

Signed-off-by: Jose Abreu 
Cc: Carlos Palminha 
Cc: Alexey Brodkin 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 
---
 drivers/gpu/drm/drm_probe_helper.c   | 44 
 include/drm/drm_modeset_helper_vtables.h | 26 +++
 2 files changed, 70 insertions(+)

diff --git a/drivers/gpu/drm/drm_probe_helper.c 
b/drivers/gpu/drm/drm_probe_helper.c
index 1b0c14a..61eac30 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -80,6 +80,46 @@
return MODE_OK;
 }
 
+static enum drm_mode_status drm_mode_validate_connector_crtc(
+   struct drm_connector *connector,
+   struct drm_display_mode *mode)
+{
+   const struct drm_crtc_helper_funcs *crtc_funcs = NULL;
+   enum drm_mode_status mode_status = MODE_ERROR;
+   struct drm_device *dev = connector->dev;
+   struct drm_encoder *encoder;
+   struct drm_crtc *crtc;
+   bool callback_found = false;
+   int i;
+
+   for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
+   encoder = drm_encoder_find(dev, connector->encoder_ids[i]);
+
+   if (!encoder)
+   continue;
+
+   drm_for_each_crtc(crtc, dev) {
+   crtc_funcs = crtc->helper_private;
+
+   if (!drm_encoder_crtc_ok(encoder, crtc))
+   continue;
+   if (!crtc_funcs || !crtc_funcs->mode_valid)
+   continue;
+
+   /* MODE_OK=0 and default mode_status=MODE_ERROR=-1
+* so if at least one crtc accepts the mode we get
+* MODE_OK */
+   mode_status &= crtc_funcs->mode_valid(crtc, mode);
+   callback_found |= true;
+   }
+   }
+
+   /* We can reach here without calling mode_valid if there is no
+* registered crtc with this callback, lets return MODE_OK in this
+* case */
+   return callback_found ? mode_status : MODE_OK;
+}
+
 static int drm_helper_probe_add_cmdline_mode(struct drm_connector *connector)
 {
struct drm_cmdline_mode *cmdline_mode;
@@ -431,6 +471,10 @@ int drm_helper_probe_single_connector_modes(struct 
drm_connector *connector,
if (mode->status == MODE_OK && connector_funcs->mode_valid)
mode->status = connector_funcs->mode_valid(connector,
   mode);
+
+   if (mode->status == MODE_OK)
+   mode->status = drm_mode_validate_connector_crtc(
+   connector, mode);
}
 
 prune:
diff --git a/include/drm/drm_modeset_helper_vtables.h 
b/include/drm/drm_modeset_helper_vtables.h
index c01c328..59fffba 100644
--- a/include/drm/drm_modeset_helper_vtables.h
+++ b/include/drm/drm_modeset_helper_vtables.h
@@ -106,6 +106,32 @@ struct drm_crtc_helper_funcs {
void (*commit)(struct drm_crtc *crtc);
 
/**
+* @mode_valid:
+*
+* This callback should be implemented if the crtc has some sort of
+* restriction in the modes it can display. For example, a given crtc
+* may be responsible to set a clock value. If the clock can not
+* produce all the values for the available modes then this callback
+* can be used to restrict the number of probbed modes to only the ones
+* that can be displayed.
+*
+* This is directly called at the same stage of connector->mode_valid
+* callback.
+*
+* NOTE:
+*
+* For a given set of crtc's in a drm_device, if at least one does not
+* have the mode_valid callback, or, at least one returns MODE_OK then
+* the mode will be probbed.
+*
+* RETURNS:
+*
+* drm_mode_status Enum
+*/
+   enum drm_mode_status (*mode_valid)(struct drm_crtc *crtc,
+

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Logan Gunthorpe]

On 26/04/17 02:59 AM,   wrote:
> Good to know that somebody is working on this. Those problems troubled
> us as well.

Thanks Christian. It's a daunting problem and a there's a lot of work to
do before we will ever be where we need to be so any help, even an ack,
is greatly appreciated.

Logan

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

Re: [Intel-gfx] GPU hangs and X shot down with 4.11-rc6

[Michal Hocko]

On Tue 25-04-17 21:03:32, Chris Wilson wrote:
> On Tue, Apr 25, 2017 at 06:41:20PM +0200, Michal Hocko wrote:
> > Hi,
> > I have just experienced X being shut down once with 4.11-rc2 and 2 times
> > with 4.11-rc6 kernel.  I do not remember seeing something like this
> > before but it is quite possible I was just lucky to not trigger this
> > issue before. It always happened while I was working on a presentation
> > in LibreOffice which I do very seldom. The kernel log contains:
> > 
> > [ 7456.721893] [drm] GPU HANG: ecode 9:0:0x86dd, in Xorg [3594], 
> > reason: Hang on render ring, action: reset
> > [ 7456.721897] [drm] GPU hangs can indicate a bug anywhere in the entire 
> > gfx stack, including userspace.
> > [ 7456.721898] [drm] Please file a _new_ bug report on bugs.freedesktop.org 
> > against DRI -> DRM/Intel
> > [ 7456.721900] [drm] drm/i915 developers can then reassign to the right 
> > component if it's not a kernel issue.
> > [ 7456.721901] [drm] The gpu crash dump is required to analyze gpu hangs, 
> > so please always attach it.
> > [ 7456.721902] [drm] GPU crash dump saved to /sys/class/drm/card0/error
> > [ 7456.721925] drm/i915: Resetting chip after gpu hang
> > [ 7456.722117] [drm] RC6 on
> > [ 7456.734588] [drm] GuC firmware load skipped
> > [ 7464.686209] drm/i915: Resetting chip after gpu hang
> > [ 7464.686284] [drm] RC6 on
> > [ 7464.702469] [drm] GuC firmware load skipped
> > [ 7472.686180] drm/i915: Resetting chip after gpu hang
> > [ 7472.686241] [drm] RC6 on
> > [ 7472.704565] [drm] GuC firmware load skipped
> > [ 7480.686179] drm/i915: Resetting chip after gpu hang
> > [ 7480.686241] [drm] RC6 on
> > [ 7480.704583] [drm] GuC firmware load skipped
> > [ 7493.678130] drm/i915: Resetting chip after gpu hang
> > [ 7493.678206] [drm] RC6 on
> > [ 7493.696505] [drm] GuC firmware load skipped
> > 
> > The kernel message tells that the problem might be anywhere and I should
> > report to freedesktop but I haven't changed the userspace recently so it
> > smells more like a kernel bug to me. Does this ring bells? The GPU crash
> > dump is attached in case it is useful.
> 
> There are lots of very similar GPU hangs for mesa across a wide range of
> kernels, with several reports noting a correlation with libreoffice.
> 
> At first glance, I would say you were just unlucky to hit it.

OK, good to know. Thanks!
-- 
Michal Hocko
SUSE Labs
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/2] drm: Make fbdev inherit the crtc's initial rotation

[Bastien Nocera]

On Mon, 2017-04-24 at 15:48 +0300, Ville Syrjälä wrote:
> 


> > I've a patch for iio-sensor-proxy which fixes the rotation under
> > Xorg /
> > Wayland when using a desktop environment which honors iio-sensor-
> > proxy's
> > rotation detection:
> > https://github.com/hadess/iio-sensor-proxy/pull/162
> 
> Or is it just this thing that clobbers what the DDX inherited from
> the
> kernel as the initial rotation?

I think it's mostly got to do with the compositor (or X) not knowing
what "normal" or "0 degrees rotation" corresponds to.

I would expect the baseline test for this wouldn't involve iio-sensor-
proxy at all, and boot to the desktop with the expected orientation
showing up in the desktop env/XRandR as "not rotated".

Eg. for the GPD Win which does have this problem, this should appear as
being rotated (image 1):
https://c.slashgear.com/wp-content/uploads/2016/11/gpd-win-book-2.jpg
But not this (image 2):
https://c.slashgear.com/wp-content/uploads/2016/11/gpd-win-0-800x420.jpg

After that, the problem is whether the accelerometer is mounted the
same way as the "non-rotated screen" (option 2), or the "non-rotated
screen" (option 1), which would show what quirking is needed.

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

Re: [PATCH 1/2] drm/fb-helper: Make fbdev inherit the crtc's rotation

[Bastien Nocera]

On Sun, 2017-04-23 at 18:11 +0200, Hans de Goede wrote:
> From: Ville Syrjala 
> 
> If a connector added through drm_fb_helper_add_one_connector() has
> a crtc attached and that crtc has a rotation configured make the
> fbdev inherit the crtc's rotation.
> 
> This is useful on e.g. some tablets which have their lcd panel
> mounted
> upside down, which before this commit would result in the kernel boot
> messages switching from being shown the right way up in efifb to
> being
> shown upside down as soon as a native kms driver loads.
> 
> BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=94894
> Cc: Ville Syrjala 
> [hdego...@redhat.com: Split the drm/fb-helper bits out of Ville's
>  "drm/fb-helper: Inherit rotation wip" patch]
> Tested-by: Hans de Goede 
> Signed-off-by: Hans de Goede 
> ---
>  drivers/gpu/drm/drm_fb_helper.c | 51
> ++---
>  include/drm/drm_fb_helper.h |  2 ++
>  2 files changed, 50 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c
> index 324a688..c97e00ab 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -159,6 +159,8 @@ int drm_fb_helper_add_one_connector(struct
> drm_fb_helper *fb_helper, struct drm_
>  {
>   struct drm_fb_helper_connector **temp;
>   struct drm_fb_helper_connector *fb_helper_connector;
> + struct drm_crtc *crtc = connector->encoder ?
> + connector->encoder->crtc : NULL;
>  
>   if (!drm_fbdev_emulation)
>   return 0;
> @@ -180,6 +182,11 @@ int drm_fb_helper_add_one_connector(struct
> drm_fb_helper *fb_helper, struct drm_
>  
>   drm_connector_reference(connector);
>   fb_helper_connector->connector = connector;
> + if (crtc && crtc->primary->state)
> + fb_helper_connector->rotation = crtc->primary-
> >state->rotation;
> + if (!fb_helper_connector->rotation)
> + fb_helper_connector->rotation = DRM_ROTATE_0;

That's equivalent to:
if (fb_helper_connector->rotation = DRM_ROTATE_0)
   fb_helper_connector->rotation = DRM_ROTATE_0;

Maybe:
if (crtc && crtc->primary->state)
   ...
else
   ...->rotation = DRM_ROTATE_0;
would be clearer? Or simply omit it if the connector is zero'ed.   


> diff --git a/include/drm/drm_fb_helper.h
> b/include/drm/drm_fb_helper.h
> index 6f5aceb..19fc313 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -48,6 +48,7 @@ struct drm_fb_helper_crtc {
>   struct drm_mode_set mode_set;
>   struct drm_display_mode *desired_mode;
>   int x, y;
> + uint8_t rotation;
>  };
>  
>  /**
> @@ -159,6 +160,7 @@ struct drm_fb_helper_funcs {
>  
>  struct drm_fb_helper_connector {
>   struct drm_connector *connector;
> + uint8_t rotation;

In both those cases, I'd add a mention of the type of enum/mask etc.
for the rotation, for example:
uint8_t rotation; /* DRM_ROTATE_* */
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [linux-sunxi] [PATCH 13/15] drm/sun4i: Add HDMI support

[Chen-Yu Tsai]

On Wed, Apr 26, 2017 at 2:50 PM, Maxime Ripard
 wrote:
> Hi Chen-Yu,
>
> On Fri, Apr 21, 2017 at 11:17:17PM +0800, Chen-Yu Tsai wrote:
>> Hi,
>>
>> On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard
>>  wrote:
>> > The earlier Allwinner SoCs (A10, A10s, A20, A31) have an embedded HDMI
>> > controller.
>> >
>> > That HDMI controller is able to do audio and CEC, but those have been left
>> > out for now.
>> >
>> > Signed-off-by: Maxime Ripard 
>> > ---
>> >  drivers/gpu/drm/sun4i/Makefile  |   5 +-
>> >  drivers/gpu/drm/sun4i/sun4i_hdmi.h  | 124 ++-
>> >  drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c  | 128 ++-
>> >  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c  | 449 +-
>> >  drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 +++-
>> >  5 files changed, 942 insertions(+), 0 deletions(-)
>> >  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h
>> >  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
>> >  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
>> >  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
>>
>> Applying patch #9608371 using 'git am'
>> Description: [13/15] drm/sun4i: Add HDMI support
>> Applying: drm/sun4i: Add HDMI support
>> .git/rebase-apply/patch:116: trailing whitespace.
>>
>> .git/rebase-apply/patch:531: trailing whitespace.
>>
>> .git/rebase-apply/patch:701: trailing whitespace.
>>
>> warning: 3 lines add whitespace errors.
>
> Fixed.
>
>> > +int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *parent)
>> > +{
>> > +   struct clk_init_data init;
>> > +   struct sun4i_ddc *ddc;
>> > +   const char *parent_name;
>> > +
>> > +   parent_name = __clk_get_name(parent);
>> > +   if (!parent_name)
>> > +   return -ENODEV;
>> > +
>> > +   ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL);
>> > +   if (!ddc)
>> > +   return -ENOMEM;
>> > +
>> > +   init.name = "hdmi-ddc";
>> > +   init.ops = _ddc_ops;
>> > +   init.parent_names = _name;
>> > +   init.num_parents = 1;
>> > +   init.flags = CLK_SET_RATE_PARENT;
>>
>> I don't think this is really needed. It probably doesn't hurt though,
>> since DDC is used when HDMI is not used for displaying, but it might
>> affect any upstream PLLs, which theoretically may affect other users
>> of said PLLs. The DDC clock is slow enough that we should be able to
>> generate a usable clock rate anyway.
>
> Good point, I removed it.
>
>> > +   writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |
>> > +  SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),
>> > +  hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG);
>> > +
>> > +   x = mode->htotal - mode->hsync_start;
>> > +   y = mode->vtotal - mode->vsync_start;
>>
>> I'm a bit skeptical about this one. All the other parameters are not
>> inclusive of other, why would this one be different? Shouldn't it
>> be "Xtotal - Xsync_end" instead?
>
> By the usual meaning of backporch, you're right. However, Allwinner's
> seems to have it's own, which is actually the backporch + sync length.
>
> We also have that on all the other connectors (and TCON), and this was
> confirmed at the time using a scope on an RGB signal.

Yes. On the later SoCs such as the A31, the user manual actually has
timing diagrams showing this.

Unlike the TCON, the HDMI controller's timings lists the front porch
separately, instead of an all inclusive Xtotal. This is what made me
look twice. This should be easy to confirm though. Since the HDMI modes
are well known and can be exactly reproduced on our hardware, we can
just check for any distortions or refresh rate errors.

>
>>
>> > +   writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
>> > +  hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG);
>> > +
>> > +   x = mode->hsync_start - mode->hdisplay;
>> > +   y = mode->vsync_start - mode->vdisplay;
>> > +   writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
>> > +  hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG);
>> > +
>> > +   x = mode->hsync_end - mode->hsync_start;
>> > +   y = mode->vsync_end - mode->vsync_start;
>> > +   writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
>> > +  hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG);
>> > +
>> > +   val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK;
>> > +   if (mode->flags & DRM_MODE_FLAG_PHSYNC)
>> > +   val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC;
>> > +
>> > +   if (mode->flags & DRM_MODE_FLAG_PVSYNC)
>> > +   val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC;
>> > +
>> > +   writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);
>>
>> You don't handle the interlaced video here, even though you set
>>
>> hdmi->connector.interlace_allowed = true
>>
>> later.
>
> I'll fix that.
>
>> The double clock and double scan 

Re: [PATCH 2/2] drm/i915: Make get_initial_plane_config also get the initial rotation config

[Bastien Nocera]

On Sun, 2017-04-23 at 18:11 +0200, Hans de Goede wrote:
> From: Ville Syrjala 

> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 344f238..63623dd 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -418,6 +418,7 @@ struct intel_initial_plane_config {
>   unsigned int tiling;
>   int size;
>   u32 base;
> + uint8_t rotation;

Mentioning what this is (DRM_ROTATE_* or something else) is even more
important here.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v7 4/4] drm/dp: Track MST link bandwidth

[Pandiyan, Dhinakaran]

On Tue, 2017-04-25 at 09:51 +0200, Maarten Lankhorst wrote:
> On 21-04-17 07:51, Dhinakaran Pandiyan wrote:
> > From: "Pandiyan, Dhinakaran" 
> >
> > Use the added helpers to track MST link bandwidth for atomic modesets.
> > Link bw is acquired in the ->atomic_check() phase when CRTCs are being
> > enabled with drm_atomic_find_vcpi_slots() instead of drm_find_vcpi_slots().
> > Similarly, link bw is released during ->atomic_check() with the connector
> > helper callback ->atomic_release() when CRTCs are disabled.
> >
> > v5: Implement connector->atomic_check() in place of atomic_release()
> > v4: Return an int from intel_dp_mst_atomic_release() (Maarten)
> > v3:
> >  Handled the case where ->atomic_release() is called more than once
> >  during an atomic_check() for the same state.
> > v2:
> >  Squashed atomic_release() implementation and caller (Daniel)
> >  Fixed logic for connector-crtc switching case (Daniel)
> >  Fixed logic for suspend-resume case.
> >
> > Cc: Daniel Vetter 
> > Cc: Maarten Lankhorst 
> > Cc: Archit Taneja 
> > Cc: Chris Wilson 
> > Cc: Harry Wentland 
> > Signed-off-by: Dhinakaran Pandiyan 
> > ---
> >  drivers/gpu/drm/i915/intel_dp_mst.c | 57 
> > +
> >  1 file changed, 51 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c 
> > b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 5af22a7..20c557c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -39,9 +39,9 @@ static bool intel_dp_mst_compute_config(struct 
> > intel_encoder *encoder,
> > struct intel_dp *intel_dp = _dig_port->dp;
> > struct intel_connector *connector =
> > to_intel_connector(conn_state->connector);
> > -   struct drm_atomic_state *state;
> > +   struct drm_atomic_state *state = pipe_config->base.state;
> > int bpp;
> > -   int lane_count, slots;
> > +   int lane_count, slots = 0;
> > const struct drm_display_mode *adjusted_mode = 
> > _config->base.adjusted_mode;
> > int mst_pbn;
> >  
> > @@ -63,24 +63,68 @@ static bool intel_dp_mst_compute_config(struct 
> > intel_encoder *encoder,
> > pipe_config->pipe_bpp = bpp;
> > pipe_config->port_clock = intel_dp_max_link_rate(intel_dp);
> >  
> > -   state = pipe_config->base.state;
> > -
> > if (drm_dp_mst_port_has_audio(_dp->mst_mgr, connector->port))
> > pipe_config->has_audio = true;
> > -   mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
> >  
> > +   mst_pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock, bpp);
> > pipe_config->pbn = mst_pbn;
> > -   slots = drm_dp_find_vcpi_slots(_dp->mst_mgr, mst_pbn);
> >  
> > intel_link_compute_m_n(bpp, lane_count,
> >adjusted_mode->crtc_clock,
> >pipe_config->port_clock,
> >_config->dp_m_n);
> >  
> > +   if (pipe_config->base.active) {
> Is this check needed? If so it needs a comment as to why.
> 
> I know that the output won't actually turn on, but if the crtc is configured 
> vcpi slots should be allocated if possible.
> 
> Else the following will fail in the wrong place:
> 1. Configure CRTC with a valid mode and DP-MST connector, but set 
> active=false. connectors_changed = true, mode_changed = true. Not enough VCPI 
> slots to enable all crtc's at the same time now, but this returns OK.
> 2. Commit with active property set to true. There are too few vcpi slots to 
> make this work, this atomic commit returns -EINVAL. This might happen on a 
> different crtc than the newly configured one in stap 1, if they were all 
> previously set to !active.
> 
Thanks for the review. 
 
I am not sure if it's supposed to fail earlier i.e, when active=false,
connectors_changed = true, mode_changed = true.

In your example, 
1. We have enough vcpi slots for crtc1 and crtc2- for both active=false
and enable=true.
2. Now, crtc3 is also configured with active=false and it does not have
enough bandwidth to be active along with crtc1 and crtc2. 
3. You are saying the problem is that if there is a commit with crtc3,
crtc2, crtc1, in that order, setting active=true, then the commit will
fail?


I don't know how likely this scenario is, but what if userspace is not
going to set crtc1 and crtc2 active=true at all and we return -EINVAL in
Step2 for crtc3?


iiuc we disable pipes, drop payload allocations, disable shared_dpll
when active goes from true->false, even if enable is still true. For eg.
this happens when I 'xset dpms force standby' without these patches. If
we are dropping resources when active goes true->false with enable=true,
shouldn't we also acquire only when active changes from false->true ?


-DK

> > +   slots = drm_dp_atomic_find_vcpi_slots(state, 

Re: [RFC 0/4] Exynos DRM: add Picture Processor extension

[Nicolas Dufresne]

Le mercredi 26 avril 2017 à 18:52 +0200, Tobias Jakobi a écrit :
> Hello again,
> 
> 
> Nicolas Dufresne wrote:
> > Le mercredi 26 avril 2017 à 01:21 +0300, Sakari Ailus a écrit :
> > > Hi Marek,
> > > 
> > > On Thu, Apr 20, 2017 at 01:23:09PM +0200, Marek Szyprowski wrote:
> > > > Hi Laurent,
> > > > 
> > > > On 2017-04-20 12:25, Laurent Pinchart wrote:
> > > > > Hi Marek,
> > > > > 
> > > > > (CC'ing Sakari Ailus)
> > > > > 
> > > > > Thank you for the patches.
> > > > > 
> > > > > On Thursday 20 Apr 2017 11:13:36 Marek Szyprowski wrote:
> > > > > > Dear all,
> > > > > > 
> > > > > > This is an updated proposal for extending EXYNOS DRM API with 
> > > > > > generic
> > > > > > support for hardware modules, which can be used for processing 
> > > > > > image data
> > > > > > from the one memory buffer to another. Typical memory-to-memory 
> > > > > > operations
> > > > > > are: rotation, scaling, colour space conversion or mix of them. 
> > > > > > This is a
> > > > > > follow-up of my previous proposal "[RFC 0/2] New feature: 
> > > > > > Framebuffer
> > > > > > processors", which has been rejected as "not really needed in the 
> > > > > > DRM
> > > > > > core":
> > > > > > http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg146286.html
> > > > > > 
> > > > > > In this proposal I moved all the code to Exynos DRM driver, so now 
> > > > > > this
> > > > > > will be specific only to Exynos DRM. I've also changed the name from
> > > > > > framebuffer processor (fbproc) to picture processor (pp) to avoid 
> > > > > > confusion
> > > > > > with fbdev API.
> > > > > > 
> > > > > > Here is a bit more information what picture processors are:
> > > > > > 
> > > > > > Embedded SoCs are known to have a number of hardware blocks, which 
> > > > > > perform
> > > > > > such operations. They can be used in paralel to the main GPU module 
> > > > > > to
> > > > > > offload CPU from processing grapics or video data. One of example 
> > > > > > use of
> > > > > > such modules is implementing video overlay, which usually requires 
> > > > > > color
> > > > > > space conversion from NV12 (or similar) to RGB32 color space and 
> > > > > > scaling to
> > > > > > target window size.
> > > > > > 
> > > > > > The proposed API is heavily inspired by atomic KMS approach - it is 
> > > > > > also
> > > > > > based on DRM objects and their properties. A new DRM object is 
> > > > > > introduced:
> > > > > > picture processor (called pp for convenience). Such objects have a 
> > > > > > set of
> > > > > > standard DRM properties, which describes the operation to be 
> > > > > > performed by
> > > > > > respective hardware module. In typical case those properties are a 
> > > > > > source
> > > > > > fb id and rectangle (x, y, width, height) and destination fb id and
> > > > > > rectangle. Optionally a rotation property can be also specified if
> > > > > > supported by the given hardware. To perform an operation on image 
> > > > > > data,
> > > > > > userspace provides a set of properties and their values for given 
> > > > > > fbproc
> > > > > > object in a similar way as object and properties are provided for
> > > > > > performing atomic page flip / mode setting.
> > > > > > 
> > > > > > The proposed API consists of the 3 new ioctls:
> > > > > > - DRM_IOCTL_EXYNOS_PP_GET_RESOURCES: to enumerate all available 
> > > > > > picture
> > > > > >   processors,
> > > > > > - DRM_IOCTL_EXYNOS_PP_GET: to query capabilities of given picture
> > > > > >   processor,
> > > > > > - DRM_IOCTL_EXYNOS_PP_COMMIT: to perform operation described by 
> > > > > > given
> > > > > >   property set.
> > > > > > 
> > > > > > The proposed API is extensible. Drivers can attach their own, custom
> > > > > > properties to add support for more advanced picture processing (for 
> > > > > > example
> > > > > > blending).
> > > > > > 
> > > > > > This proposal aims to replace Exynos DRM IPP (Image Post Processing)
> > > > > > subsystem. IPP API is over-engineered in general, but not really 
> > > > > > extensible
> > > > > > on the other side. It is also buggy, with significant design flaws 
> > > > > > - the
> > > > > > biggest issue is the fact that the API covers memory-2-memory 
> > > > > > picture
> > > > > > operations together with CRTC writeback and duplicating features, 
> > > > > > which
> > > > > > belongs to video plane. Comparing with IPP subsystem, the PP 
> > > > > > framework is
> > > > > > smaller (1807 vs 778 lines) and allows driver simplification (Exynos
> > > > > > rotator driver smaller by over 200 lines).
> > 
> > Just a side note, we have written code in GStreamer using the Exnynos 4
> > FIMC IPP driver. I don't know how many, if any, deployment still exist
> > (Exynos 4 is relatively old now), but there exist userspace for the
> > FIMC driver.
> 
> I was searching for this code, but I didn't find anything. Are you sure
> you really mean the FIMC IPP in Exynos DRM, and not just the FIMC driver
> from the V4L2 subsystem?

Oops, 

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Logan Gunthorpe]

On 26/04/17 01:44 AM, Christoph Hellwig wrote:
> I think we'll at least need a draft of those to make sense of these
> patches.  Otherwise they just look very clumsy.

Ok, I'll work up a draft proposal and send it in a couple days. But
without a lot of cleanup such as this series it's not going to even be
able to compile.

> I'm sorry but this API is just a trainwreck.  Right now we have the
> nice little kmap_atomic API, which never fails and has a very nice
> calling convention where we just pass back the return address, but does
> not support sleeping inside the critical section.
> 
> And kmap, whіch may fail and requires the original page to be passed
> back.  Anything that mixes these two concepts up is simply a non-starter.

Ok, well for starters I think you are mistaken about kmap being able to
fail. I'm having a hard time finding many users of that function that
bother to check for an error when calling it. The main difficulty we
have now is that neither of those functions are expected to fail and we
need them to be able to in cases where the page doesn't map to system
RAM. This patch series is trying to address it for users of scatterlist.
I'm certainly open to other suggestions.

I also have to disagree that kmap and kmap_atomic are all that "nice".
Except for the sleeping restriction and performance, they effectively do
the same thing. And it was necessary to write a macro wrapper around
kunmap_atomic to ensure that users of that function don't screw it up.
(See 597781f3e5.) I'd say the kmap/kmap_atomic functions are the
trainwreck and I'm trying to do my best to cleanup a few cases.

There are a fair number of cases in the kernel that do something like:

if (something)
x = kmap(page);
else
x = kmap_atomic(page);
...
if (something)
kunmap(page)
else
kunmap_atomic(x)

Which just seems cumbersome to me.

In any case, if you can accept an sg_kmap and sg_kmap_atomic api just
say so and I'll make the change. But I'll still need a flags variable
for SG_MAP_MUST_NOT_FAIL to support legacy cases that have no fail path
and both of those functions will need to be pretty nearly replicas of
each other.

Logan


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

Re: [RFC 0/4] Exynos DRM: add Picture Processor extension

[Nicolas Dufresne]

Le mercredi 26 avril 2017 à 01:21 +0300, Sakari Ailus a écrit :
> Hi Marek,
> 
> On Thu, Apr 20, 2017 at 01:23:09PM +0200, Marek Szyprowski wrote:
> > Hi Laurent,
> > 
> > On 2017-04-20 12:25, Laurent Pinchart wrote:
> > > Hi Marek,
> > > 
> > > (CC'ing Sakari Ailus)
> > > 
> > > Thank you for the patches.
> > > 
> > > On Thursday 20 Apr 2017 11:13:36 Marek Szyprowski wrote:
> > > > Dear all,
> > > > 
> > > > This is an updated proposal for extending EXYNOS DRM API with generic
> > > > support for hardware modules, which can be used for processing image 
> > > > data
> > > > from the one memory buffer to another. Typical memory-to-memory 
> > > > operations
> > > > are: rotation, scaling, colour space conversion or mix of them. This is 
> > > > a
> > > > follow-up of my previous proposal "[RFC 0/2] New feature: Framebuffer
> > > > processors", which has been rejected as "not really needed in the DRM
> > > > core":
> > > > http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg146286.html
> > > > 
> > > > In this proposal I moved all the code to Exynos DRM driver, so now this
> > > > will be specific only to Exynos DRM. I've also changed the name from
> > > > framebuffer processor (fbproc) to picture processor (pp) to avoid 
> > > > confusion
> > > > with fbdev API.
> > > > 
> > > > Here is a bit more information what picture processors are:
> > > > 
> > > > Embedded SoCs are known to have a number of hardware blocks, which 
> > > > perform
> > > > such operations. They can be used in paralel to the main GPU module to
> > > > offload CPU from processing grapics or video data. One of example use of
> > > > such modules is implementing video overlay, which usually requires color
> > > > space conversion from NV12 (or similar) to RGB32 color space and 
> > > > scaling to
> > > > target window size.
> > > > 
> > > > The proposed API is heavily inspired by atomic KMS approach - it is also
> > > > based on DRM objects and their properties. A new DRM object is 
> > > > introduced:
> > > > picture processor (called pp for convenience). Such objects have a set 
> > > > of
> > > > standard DRM properties, which describes the operation to be performed 
> > > > by
> > > > respective hardware module. In typical case those properties are a 
> > > > source
> > > > fb id and rectangle (x, y, width, height) and destination fb id and
> > > > rectangle. Optionally a rotation property can be also specified if
> > > > supported by the given hardware. To perform an operation on image data,
> > > > userspace provides a set of properties and their values for given fbproc
> > > > object in a similar way as object and properties are provided for
> > > > performing atomic page flip / mode setting.
> > > > 
> > > > The proposed API consists of the 3 new ioctls:
> > > > - DRM_IOCTL_EXYNOS_PP_GET_RESOURCES: to enumerate all available picture
> > > >   processors,
> > > > - DRM_IOCTL_EXYNOS_PP_GET: to query capabilities of given picture
> > > >   processor,
> > > > - DRM_IOCTL_EXYNOS_PP_COMMIT: to perform operation described by given
> > > >   property set.
> > > > 
> > > > The proposed API is extensible. Drivers can attach their own, custom
> > > > properties to add support for more advanced picture processing (for 
> > > > example
> > > > blending).
> > > > 
> > > > This proposal aims to replace Exynos DRM IPP (Image Post Processing)
> > > > subsystem. IPP API is over-engineered in general, but not really 
> > > > extensible
> > > > on the other side. It is also buggy, with significant design flaws - the
> > > > biggest issue is the fact that the API covers memory-2-memory picture
> > > > operations together with CRTC writeback and duplicating features, which
> > > > belongs to video plane. Comparing with IPP subsystem, the PP framework 
> > > > is
> > > > smaller (1807 vs 778 lines) and allows driver simplification (Exynos
> > > > rotator driver smaller by over 200 lines).

Just a side note, we have written code in GStreamer using the Exnynos 4
FIMC IPP driver. I don't know how many, if any, deployment still exist
(Exynos 4 is relatively old now), but there exist userspace for the
FIMC driver. We use this for color transformation (from tiled to
linear) and scaling. The FIMC driver is in fact quite stable in
upstream kernel today. The GScaler V4L2 M2M driver on Exynos 5 is
largely based on it and has received some maintenance to properly work
in GStreamer. unlike this DRM API, you can reuse the same userspace
code across multiple platforms (which we do already). We have also
integrated this driver in Chromium in the past (not upstream though).

I am well aware that the blitter driver has not got much attention
though. But again, V4L2 offers a generic interface to userspace
application. Fixing this driver could enable some work like this one:

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

This work in progress feature is a generic hardware accelerated video
mixer. It has been tested with IMX.6 v4l2 m2m blitter driver 

Re: [RFC 0/4] Exynos DRM: add Picture Processor extension

[Nicolas Dufresne]

Le mercredi 26 avril 2017 à 21:31 +0200, Tobias Jakobi a écrit :
> I'm pretty sure you have misread Marek's description of the patchset.
> The picture processor API should replaced/deprecate the IPP API that is
> currently implemented in the Exynos DRM.
> 
> In particular this affects the following files:
> - drivers/gpu/drm/exynos/exynos_drm_ipp.{c,h}
> - drivers/gpu/drm/exynos/exynos_drm_fimc.{c,h}
> - drivers/gpu/drm/exynos/exynos_drm_gsc.{c,h}
> - drivers/gpu/drm/exynos/exynos_drm_rotator.{c,h}
> 
> I know only two places where the IPP API is actually used. Tizen and my
> experimental mpv backend.

Sorry for the noise then.

regards,
Nicolas

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

Re: [PATCH v5 07/10] drm: bridge: dw-hdmi: Add support for custom PHY configuration

[Jose Abreu]

Hi,


On 24-04-2017 08:46, 郑阳 wrote:
> Hi, Laurent Pinchart:
>
> 在 2017年03月04日 01:20, Laurent Pinchart 写道:
>> From: Kieran Bingham 
>>
>> The DWC HDMI TX controller interfaces with a companion PHY. While
>> Synopsys provides multiple standard PHYs, SoC vendors can also
>> integrate
>> a custom PHY.
>>
>> Modularize PHY configuration to support vendor PHYs through
>> platform
>> data. The existing PHY configuration code was originally
>> written to
>> support the DWC HDMI 3D TX PHY, and seems to be compatible
>> with the DWC
>> MLP PHY. The HDMI 2.0 PHY will require a separate configuration
>> function.
>>
>> Signed-off-by: Kieran Bingham
>> 
>> Signed-off-by: Laurent Pinchart
>> 
>> Tested-by: Neil Armstrong 
>> Reviewed-by: Jose Abreu 
>> ---
>> Changes since v1:
>>
>> - Check pdata->phy_configure in hdmi_phy_configure() to avoid
>>dereferencing NULL pointer.
>> ---
>>   drivers/gpu/drm/bridge/dw-hdmi.c | 109
>> ++-
>>   include/drm/bridge/dw_hdmi.h |   7 +++
>>   2 files changed, 81 insertions(+), 35 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
>> b/drivers/gpu/drm/bridge/dw-hdmi.c
>> index cb2703862be2..b835d81bb471 100644
>> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
>> @@ -118,6 +118,9 @@ struct dw_hdmi_phy_data {
>>   const char *name;
>>   unsigned int gen;
>>   bool has_svsret;
>> +int (*configure)(struct dw_hdmi *hdmi,
>> + const struct dw_hdmi_plat_data *pdata,
>> + unsigned long mpixelclock);
>>   };
>> struct dw_hdmi {
>> @@ -860,8 +863,8 @@ static bool hdmi_phy_wait_i2c_done(struct
>> dw_hdmi *hdmi, int msec)
>>   return true;
>>   }
>>   -static void hdmi_phy_i2c_write(struct dw_hdmi *hdmi,
>> unsigned short data,
>> - unsigned char addr)
>> +void dw_hdmi_phy_i2c_write(struct dw_hdmi *hdmi, unsigned
>> short data,
>> +   unsigned char addr)
>>   {
>>   hdmi_writeb(hdmi, 0xFF, HDMI_IH_I2CMPHY_STAT0);
>>   hdmi_writeb(hdmi, addr, HDMI_PHY_I2CM_ADDRESS_ADDR);
>> @@ -873,6 +876,7 @@ static void hdmi_phy_i2c_write(struct
>> dw_hdmi *hdmi, unsigned short data,
>>   HDMI_PHY_I2CM_OPERATION_ADDR);
>>   hdmi_phy_wait_i2c_done(hdmi, 1000);
>>   }
>> +EXPORT_SYMBOL_GPL(dw_hdmi_phy_i2c_write);
>> static void dw_hdmi_phy_enable_powerdown(struct dw_hdmi
>> *hdmi, bool enable)
>>   {
>> @@ -993,37 +997,67 @@ static int dw_hdmi_phy_power_on(struct
>> dw_hdmi *hdmi)
>>   return 0;
>>   }
>>   -static int hdmi_phy_configure(struct dw_hdmi *hdmi)
>> +/*
>> + * PHY configuration function for the DWC HDMI 3D TX PHY.
>> Based on the available
>> + * information the DWC MHL PHY has the same register layout
>> and is thus also
>> + * supported by this function.
>> + */
>> +static int hdmi_phy_configure_dwc_hdmi_3d_tx(struct dw_hdmi
>> *hdmi,
>> +const struct dw_hdmi_plat_data *pdata,
>> +unsigned long mpixelclock)
>>   {
>> -const struct dw_hdmi_phy_data *phy = hdmi->phy.data;
>> -const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
>>   const struct dw_hdmi_mpll_config *mpll_config =
>> pdata->mpll_cfg;
>>   const struct dw_hdmi_curr_ctrl *curr_ctrl = pdata->cur_ctr;
>>   const struct dw_hdmi_phy_config *phy_config =
>> pdata->phy_config;
>> /* PLL/MPLL Cfg - always match on final entry */
>>   for (; mpll_config->mpixelclock != ~0UL; mpll_config++)
>> -if (hdmi->hdmi_data.video_mode.mpixelclock <=
>> -mpll_config->mpixelclock)
>> +if (mpixelclock <= mpll_config->mpixelclock)
>>   break;
>> for (; curr_ctrl->mpixelclock != ~0UL; curr_ctrl++)
>> -if (hdmi->hdmi_data.video_mode.mpixelclock <=
>> -curr_ctrl->mpixelclock)
>> +if (mpixelclock <= curr_ctrl->mpixelclock)
>>   break;
>> for (; phy_config->mpixelclock != ~0UL; phy_config++)
>> -if (hdmi->hdmi_data.video_mode.mpixelclock <=
>> -phy_config->mpixelclock)
>> +if (mpixelclock <= phy_config->mpixelclock)
>>   break;
>> if (mpll_config->mpixelclock == ~0UL ||
>>   curr_ctrl->mpixelclock == ~0UL ||
>> -phy_config->mpixelclock == ~0UL) {
>> -dev_err(hdmi->dev, "Pixel clock %d - unsupported by
>> HDMI\n",
>> -hdmi->hdmi_data.video_mode.mpixelclock);
>> +phy_config->mpixelclock == ~0UL)
>>   return -EINVAL;
>> -}
>> +
>> +dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].cpce,
>> +  HDMI_3D_TX_PHY_CPCE_CTRL);
>> +dw_hdmi_phy_i2c_write(hdmi, mpll_config->res[0].gmp,
>> +  HDMI_3D_TX_PHY_GMPCTRL);
>> +dw_hdmi_phy_i2c_write(hdmi, curr_ctrl->curr[0],
>> +  HDMI_3D_TX_PHY_CURRCTRL);
>> +
>> 

Re: [PATCH v2 15/21] xen-blkfront: Make use of the new sg_map helper function

[Roger Pau Monné]

On Tue, Apr 25, 2017 at 12:21:02PM -0600, Logan Gunthorpe wrote:
> Straightforward conversion to the new helper, except due to the lack
> of error path, we have to use SG_MAP_MUST_NOT_FAIL which may BUG_ON in
> certain cases in the future.
> 
> Signed-off-by: Logan Gunthorpe 
> Cc: Boris Ostrovsky 
> Cc: Juergen Gross 
> Cc: Konrad Rzeszutek Wilk 
> Cc: "Roger Pau Monné" 
> ---
>  drivers/block/xen-blkfront.c | 20 +++-
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 3945963..ed62175 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -816,8 +816,9 @@ static int blkif_queue_rw_req(struct request *req, struct 
> blkfront_ring_info *ri
>   BUG_ON(sg->offset + sg->length > PAGE_SIZE);
>  
>   if (setup.need_copy) {
> - setup.bvec_off = sg->offset;
> - setup.bvec_data = kmap_atomic(sg_page(sg));
> + setup.bvec_off = 0;
> + setup.bvec_data = sg_map(sg, 0, SG_KMAP_ATOMIC |
> +  SG_MAP_MUST_NOT_FAIL);

I assume that sg_map already adds sg->offset to the address?

Also wondering whether we can get rid of bvec_off and just increment bvec_data,
adding Julien who IIRC added this code.

>   }
>  
>   gnttab_foreach_grant_in_range(sg_page(sg),
> @@ -827,7 +828,7 @@ static int blkif_queue_rw_req(struct request *req, struct 
> blkfront_ring_info *ri
> );
>  
>   if (setup.need_copy)
> - kunmap_atomic(setup.bvec_data);
> + sg_unmap(sg, setup.bvec_data, 0, SG_KMAP_ATOMIC);
>   }
>   if (setup.segments)
>   kunmap_atomic(setup.segments);
> @@ -1053,7 +1054,7 @@ static int xen_translate_vdev(int vdevice, int *minor, 
> unsigned int *offset)
>   case XEN_SCSI_DISK5_MAJOR:
>   case XEN_SCSI_DISK6_MAJOR:
>   case XEN_SCSI_DISK7_MAJOR:
> - *offset = (*minor / PARTS_PER_DISK) + 
> + *offset = (*minor / PARTS_PER_DISK) +
>   ((major - XEN_SCSI_DISK1_MAJOR + 1) * 16) +
>   EMULATED_SD_DISK_NAME_OFFSET;
>   *minor = *minor +
> @@ -1068,7 +1069,7 @@ static int xen_translate_vdev(int vdevice, int *minor, 
> unsigned int *offset)
>   case XEN_SCSI_DISK13_MAJOR:
>   case XEN_SCSI_DISK14_MAJOR:
>   case XEN_SCSI_DISK15_MAJOR:
> - *offset = (*minor / PARTS_PER_DISK) + 
> + *offset = (*minor / PARTS_PER_DISK) +
>   ((major - XEN_SCSI_DISK8_MAJOR + 8) * 16) +
>   EMULATED_SD_DISK_NAME_OFFSET;
>   *minor = *minor +
> @@ -1119,7 +1120,7 @@ static int xlvbd_alloc_gendisk(blkif_sector_t capacity,
>   if (!VDEV_IS_EXTENDED(info->vdevice)) {
>   err = xen_translate_vdev(info->vdevice, , );
>   if (err)
> - return err; 
> + return err;

Cosmetic changes should go in a separate patch please.

Roger.

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

[PATCH 0/2] Introduce crtc->mode_valid() callback

[Jose Abreu]

This patchset introduces a new callback for crtc, called mode_valid()
that is responsible to limit the number of probbed modes. Just like
connector->mode_valid(), this new callback is called at mode probbing
stage so that we can validate the mode.

This is specially useful because arcpgu crtc is responsible to set a
clock value in the commit() stage but unfortunatelly this clock does
not support all the needed ranges. This way we can restrict the number
of modes that are handed to userspace, so that we dont hand a mode
that will fail the commit() stage.

I guess this can also happen for other drivers so we introduce the
callback in the core.

The behaviour remains the same for crtcs that don't have the callback.
Also, for a given set of crtcs that can be bound to the connector, if
at least one can display the mode then the mode will be probbed.

For more info about why this is needed in arcpgu, please refer here [1].

[1] https://patchwork.kernel.org/patch/9694177/

Jose Abreu (2):
  drm: Introduce crtc->mode_valid() callback
  drm: arcpgu: Use crtc->mode_valid() callback

Cc: Carlos Palminha 
Cc: Alexey Brodkin 
Cc: Ville Syrjälä 
Cc: Daniel Vetter 
Cc: Dave Airlie 

 drivers/gpu/drm/arc/arcpgu_crtc.c| 14 ++
 drivers/gpu/drm/drm_probe_helper.c   | 44 
 include/drm/drm_modeset_helper_vtables.h | 26 +++
 3 files changed, 84 insertions(+)

-- 
1.9.1


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

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Christoph Hellwig]

On Tue, Apr 25, 2017 at 12:20:48PM -0600, Logan Gunthorpe wrote:
> This patch introduces functions which kmap the pages inside an sgl.
> These functions replace a common pattern of kmap(sg_page(sg)) that is
> used in more than 50 places within the kernel.
> 
> The motivation for this work is to eventually safely support sgls that
> contain io memory. In order for that to work, any access to the contents
> of an iomem SGL will need to be done with iomemcpy or hit some warning.
> (The exact details of how this will work have yet to be worked out.)

I think we'll at least need a draft of those to make sense of these
patches.  Otherwise they just look very clumsy.

> + *   Use this function to map a page in the scatterlist at the specified
> + *   offset. sg->offset is already added for you. Note: the semantics of
> + *   this function are that it may fail. Thus, its output should be checked
> + *   with IS_ERR and PTR_ERR. Otherwise, a pointer to the specified offset
> + *   in the mapped page is returned.
> + *
> + *   Flags can be any of:
> + *   * SG_KMAP   - Use kmap to create the mapping
> + *   * SG_KMAP_ATOMIC- Use kmap_atomic to map the page atommically.
> + * Thus, the rules of that function apply: the
> + * cpu may not sleep until it is unmaped.
> + *   * SG_MAP_MUST_NOT_FAIL  - Indicate that sg_map must not fail.
> + * If it does, it will issue a BUG_ON instead.
> + * This is intended for legacy code only, it
> + * is not to be used in new code.

I'm sorry but this API is just a trainwreck.  Right now we have the
nice little kmap_atomic API, which never fails and has a very nice
calling convention where we just pass back the return address, but does
not support sleeping inside the critical section.

And kmap, whіch may fail and requires the original page to be passed
back.  Anything that mixes these two concepts up is simply a non-starter.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3] drm/nouveau: Add support for clockgating on Fermi+

[Lyude]

This adds support for enabling automatic clockgating on nvidia GPUs for
Fermi and later generations. This saves a little bit of power, bringing
my fermi GPU's power consumption from ~28.3W on idle to ~27W, and my
kepler's idle power consumption from ~23.6W to ~21.65W.

Similar to how the nvidia driver seems to handle this, we enable
clockgating for each engine that supports it after it's initialization.

Changes since v1:
- Move function pointers for clockgating functions out of nvkm_therm,
  just expose one less complex function to callers:
  nvkm_therm_clkgate_engine()
- Use 0x44 for disabling clockgating instead of just shutting all of
  nvidia's power management for each gate off, since that's what the
  nvidia blob does
Changes since v2:
- Disable clockgating in nvkm_engine_fini, don't enable it!

Signed-off-by: Lyude 
---
 .../gpu/drm/nouveau/include/nvkm/subdev/therm.h|  2 +
 drivers/gpu/drm/nouveau/nvkm/core/engine.c | 12 +++-
 drivers/gpu/drm/nouveau/nvkm/engine/device/base.c  | 14 ++--
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/Kbuild   |  2 +
 .../gpu/drm/nouveau/nvkm/subdev/therm/clkgate.c| 32 +
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf100.c  | 81 ++
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c  |  1 +
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gm107.c  |  1 +
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gt215.c  |  2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/priv.h   |  5 ++
 10 files changed, 143 insertions(+), 9 deletions(-)
 create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/therm/clkgate.c
 create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf100.c

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h 
b/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
index b268b96..0e2574d 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
@@ -89,11 +89,13 @@ struct nvkm_therm {
 int nvkm_therm_temp_get(struct nvkm_therm *);
 int nvkm_therm_fan_sense(struct nvkm_therm *);
 int nvkm_therm_cstate(struct nvkm_therm *, int, int);
+void nvkm_therm_clkgate_engine(struct nvkm_therm *, enum nvkm_devidx, bool);
 
 int nv40_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int nv50_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int g84_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int gt215_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
+int gf100_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int gf119_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int gm107_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 #endif
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/engine.c 
b/drivers/gpu/drm/nouveau/nvkm/core/engine.c
index b6c9169..38b4cd1 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/engine.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/engine.c
@@ -26,6 +26,7 @@
 #include 
 
 #include 
+#include 
 
 bool
 nvkm_engine_chsw_load(struct nvkm_engine *engine)
@@ -86,6 +87,9 @@ static int
 nvkm_engine_fini(struct nvkm_subdev *subdev, bool suspend)
 {
struct nvkm_engine *engine = nvkm_engine(subdev);
+
+   nvkm_therm_clkgate_engine(subdev->device->therm, subdev->index, false);
+
if (engine->func->fini)
return engine->func->fini(engine, suspend);
return 0;
@@ -96,12 +100,13 @@ nvkm_engine_init(struct nvkm_subdev *subdev)
 {
struct nvkm_engine *engine = nvkm_engine(subdev);
struct nvkm_fb *fb = subdev->device->fb;
+   struct nvkm_therm *therm = subdev->device->therm;
int ret = 0, i;
s64 time;
 
if (!engine->usecount) {
nvkm_trace(subdev, "init skipped, engine has no users\n");
-   return ret;
+   goto finish;
}
 
if (engine->func->oneinit && !engine->subdev.oneinit) {
@@ -123,6 +128,11 @@ nvkm_engine_init(struct nvkm_subdev *subdev)
 
for (i = 0; fb && i < fb->tile.regions; i++)
nvkm_engine_tile(engine, i);
+
+finish:
+   if (!ret)
+   nvkm_therm_clkgate_engine(therm, subdev->index, true);
+
return ret;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
index b690bc1..d133016 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
@@ -1355,7 +1355,7 @@ nvc0_chipset = {
.mxm = nv50_mxm_new,
.pci = gf100_pci_new,
.pmu = gf100_pmu_new,
-   .therm = gt215_therm_new,
+   .therm = gf100_therm_new,
.timer = nv41_timer_new,
.volt = gf100_volt_new,
.ce[0] = gf100_ce_new,
@@ -1392,7 +1392,7 @@ nvc1_chipset = {
.mxm = nv50_mxm_new,
.pci = gf106_pci_new,
.pmu = gf100_pmu_new,
-   .therm = gt215_therm_new,
+   .therm = gf100_therm_new,
.timer = nv41_timer_new,

[Bug 93826] 2560x1440 @144Hz graphic glitches and bad refresh rate

[bugzilla-daemon]

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

--- Comment #45 from DesiOtaku  ---
I can confirm this bug also affects A12-9800E APU when you use the integrated
graphic card's port.

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

[PATCH v2] drm/nouveau: Add support for clockgating on Fermi+

[Lyude]

This adds support for enabling automatic clockgating on nvidia GPUs for
Fermi and later generations. This saves a little bit of power, bringing
my fermi GPU's power consumption from ~28.3W on idle to ~27W, and my
kepler's idle power consumption from ~23.6W to ~21.65W.

Similar to how the nvidia driver seems to handle this, we enable
clockgating for each engine that supports it after it's initialization.

Changes since v1:
- Move function pointers for clockgating functions out of nvkm_therm,
  just expose one less complex function to callers:
  nvkm_therm_clkgate_engine()
- Use 0x44 for disabling clockgating instead of just shutting all of
  nvidia's power management for each gate off, since that's what the
  nvidia blob does

Signed-off-by: Lyude 
---
 .../gpu/drm/nouveau/include/nvkm/subdev/therm.h|  2 +
 drivers/gpu/drm/nouveau/nvkm/core/engine.c | 12 +++-
 drivers/gpu/drm/nouveau/nvkm/engine/device/base.c  | 14 ++--
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/Kbuild   |  2 +
 .../gpu/drm/nouveau/nvkm/subdev/therm/clkgate.c| 32 +
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf100.c  | 81 ++
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c  |  1 +
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gm107.c  |  1 +
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gt215.c  |  2 +-
 drivers/gpu/drm/nouveau/nvkm/subdev/therm/priv.h   |  5 ++
 10 files changed, 143 insertions(+), 9 deletions(-)
 create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/therm/clkgate.c
 create mode 100644 drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf100.c

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h 
b/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
index b268b96..0e2574d 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
@@ -89,11 +89,13 @@ struct nvkm_therm {
 int nvkm_therm_temp_get(struct nvkm_therm *);
 int nvkm_therm_fan_sense(struct nvkm_therm *);
 int nvkm_therm_cstate(struct nvkm_therm *, int, int);
+void nvkm_therm_clkgate_engine(struct nvkm_therm *, enum nvkm_devidx, bool);
 
 int nv40_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int nv50_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int g84_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int gt215_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
+int gf100_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int gf119_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 int gm107_therm_new(struct nvkm_device *, int, struct nvkm_therm **);
 #endif
diff --git a/drivers/gpu/drm/nouveau/nvkm/core/engine.c 
b/drivers/gpu/drm/nouveau/nvkm/core/engine.c
index b6c9169..e3d52c1 100644
--- a/drivers/gpu/drm/nouveau/nvkm/core/engine.c
+++ b/drivers/gpu/drm/nouveau/nvkm/core/engine.c
@@ -26,6 +26,7 @@
 #include 
 
 #include 
+#include 
 
 bool
 nvkm_engine_chsw_load(struct nvkm_engine *engine)
@@ -86,6 +87,9 @@ static int
 nvkm_engine_fini(struct nvkm_subdev *subdev, bool suspend)
 {
struct nvkm_engine *engine = nvkm_engine(subdev);
+
+   nvkm_therm_clkgate_engine(subdev->device->therm, subdev->index, true);
+
if (engine->func->fini)
return engine->func->fini(engine, suspend);
return 0;
@@ -96,12 +100,13 @@ nvkm_engine_init(struct nvkm_subdev *subdev)
 {
struct nvkm_engine *engine = nvkm_engine(subdev);
struct nvkm_fb *fb = subdev->device->fb;
+   struct nvkm_therm *therm = subdev->device->therm;
int ret = 0, i;
s64 time;
 
if (!engine->usecount) {
nvkm_trace(subdev, "init skipped, engine has no users\n");
-   return ret;
+   goto finish;
}
 
if (engine->func->oneinit && !engine->subdev.oneinit) {
@@ -123,6 +128,11 @@ nvkm_engine_init(struct nvkm_subdev *subdev)
 
for (i = 0; fb && i < fb->tile.regions; i++)
nvkm_engine_tile(engine, i);
+
+finish:
+   if (!ret)
+   nvkm_therm_clkgate_engine(therm, subdev->index, true);
+
return ret;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c 
b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
index b690bc1..d133016 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
@@ -1355,7 +1355,7 @@ nvc0_chipset = {
.mxm = nv50_mxm_new,
.pci = gf100_pci_new,
.pmu = gf100_pmu_new,
-   .therm = gt215_therm_new,
+   .therm = gf100_therm_new,
.timer = nv41_timer_new,
.volt = gf100_volt_new,
.ce[0] = gf100_ce_new,
@@ -1392,7 +1392,7 @@ nvc1_chipset = {
.mxm = nv50_mxm_new,
.pci = gf106_pci_new,
.pmu = gf100_pmu_new,
-   .therm = gt215_therm_new,
+   .therm = gf100_therm_new,
.timer = nv41_timer_new,
.volt = gf100_volt_new,
.ce[0] = gf100_ce_new,
@@ -1428,7 +1428,7 

[Bug 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #6 from Timothy Arceri  ---
Thanks. Looks like a bug in sb optimisations so the output of 
R600_DEBUG=vs,ps,nosb on HEAD would also be useful.

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

[RFC PATCHv2 2/3] drm/prime: Introduce drm_gem_prime_import_platform

[Laura Abbott]

The existing drm_gem_prime_import function uses the underlying
struct device of a drm_device for attaching to a dma_buf. Some drivers
(notably vgem) may not have an underlying device structure. Offer
an alternate function to attach using a platform device associated
with drm_device.

Signed-off-by: Laura Abbott 
---
New approach for v2
---
 drivers/gpu/drm/drm_prime.c | 23 ---
 include/drm/drmP.h  |  5 +
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 25aa455..5486248 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -601,8 +601,9 @@ EXPORT_SYMBOL(drm_gem_prime_handle_to_fd);
  * This is the implementation of the gem_prime_import functions for GEM drivers
  * using the PRIME helpers.
  */
-struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
-   struct dma_buf *dma_buf)
+static struct drm_gem_object *__drm_gem_prime_import(struct drm_device *dev,
+   struct dma_buf *dma_buf,
+   struct device *attach_dev)
 {
struct dma_buf_attachment *attach;
struct sg_table *sgt;
@@ -624,7 +625,7 @@ struct drm_gem_object *drm_gem_prime_import(struct 
drm_device *dev,
if (!dev->driver->gem_prime_import_sg_table)
return ERR_PTR(-EINVAL);
 
-   attach = dma_buf_attach(dma_buf, dev->dev);
+   attach = dma_buf_attach(dma_buf, attach_dev);
if (IS_ERR(attach))
return ERR_CAST(attach);
 
@@ -654,8 +655,24 @@ struct drm_gem_object *drm_gem_prime_import(struct 
drm_device *dev,
 
return ERR_PTR(ret);
 }
+
+struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
+   struct dma_buf *dma_buf)
+{
+   return __drm_gem_prime_import(dev, dma_buf, dev->dev);
+}
 EXPORT_SYMBOL(drm_gem_prime_import);
 
+struct drm_gem_object *drm_gem_prime_import_platform(struct drm_device *dev,
+   struct dma_buf *dma_buf)
+{
+   if (WARN_ON_ONCE(!dev->platformdev))
+   return NULL;
+
+   return __drm_gem_prime_import(dev, dma_buf, >platformdev->dev);
+}
+EXPORT_SYMBOL(drm_gem_prime_import_platform);
+
 /**
  * drm_gem_prime_fd_to_handle - PRIME import function for GEM drivers
  * @dev: dev to export the buffer from
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 6105c05..f4ac30f 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -769,6 +769,11 @@ extern int drm_gem_prime_handle_to_fd(struct drm_device 
*dev,
int *prime_fd);
 extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev,
struct dma_buf *dma_buf);
+
+extern struct drm_gem_object *drm_gem_prime_import_platform(
+   struct drm_device *dev,
+   struct dma_buf *dma_buf);
+
 extern int drm_gem_prime_fd_to_handle(struct drm_device *dev,
struct drm_file *file_priv, int prime_fd, uint32_t *handle);
 struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev,
-- 
2.7.4

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

[RFC PATCHv2 3/3] drm/vgem: Enable dmabuf import interfaces

[Laura Abbott]

Enable the GEM dma-buf import interfaces in addition to the export
interfaces. This lets vgem be used as a test source for other allocators
(e.g. Ion).

Signed-off-by: Laura Abbott 
---
v2: Don't require vgem allocated buffers to existing in memory before importing.
---
 drivers/gpu/drm/vgem/vgem_drv.c | 133 +++-
 drivers/gpu/drm/vgem/vgem_drv.h |   2 +
 2 files changed, 106 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index 1b02e56..73a619a 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -34,6 +34,9 @@
 #include 
 #include 
 #include 
+
+#include 
+
 #include "vgem_drv.h"
 
 #define DRIVER_NAME"vgem"
@@ -46,6 +49,12 @@ static void vgem_gem_free_object(struct drm_gem_object *obj)
 {
struct drm_vgem_gem_object *vgem_obj = to_vgem_bo(obj);
 
+   if (vgem_obj->pages)
+   drm_free_large(vgem_obj->pages);
+
+   if (obj->import_attach)
+   drm_prime_gem_destroy(obj, vgem_obj->table);
+
drm_gem_object_release(obj);
kfree(vgem_obj);
 }
@@ -56,26 +65,48 @@ static int vgem_gem_fault(struct vm_fault *vmf)
struct drm_vgem_gem_object *obj = vma->vm_private_data;
/* We don't use vmf->pgoff since that has the fake offset */
unsigned long vaddr = vmf->address;
-   struct page *page;
-
-   page = shmem_read_mapping_page(file_inode(obj->base.filp)->i_mapping,
-  (vaddr - vma->vm_start) >> PAGE_SHIFT);
-   if (!IS_ERR(page)) {
-   vmf->page = page;
-   return 0;
-   } else switch (PTR_ERR(page)) {
-   case -ENOSPC:
-   case -ENOMEM:
-   return VM_FAULT_OOM;
-   case -EBUSY:
-   return VM_FAULT_RETRY;
-   case -EFAULT:
-   case -EINVAL:
-   return VM_FAULT_SIGBUS;
-   default:
-   WARN_ON_ONCE(PTR_ERR(page));
-   return VM_FAULT_SIGBUS;
+   int ret;
+   loff_t num_pages;
+   pgoff_t page_offset;
+   page_offset = (vaddr - vma->vm_start) >> PAGE_SHIFT;
+
+   num_pages = DIV_ROUND_UP(obj->base.size, PAGE_SIZE);
+
+   if (page_offset > num_pages)
+   return VM_FAULT_SIGBUS;
+
+   if (obj->pages) {
+   get_page(obj->pages[page_offset]);
+   vmf->page = obj->pages[page_offset];
+   ret = 0;
+   } else {
+   struct page *page;
+
+   page = shmem_read_mapping_page(
+   file_inode(obj->base.filp)->i_mapping,
+   page_offset);
+   if (!IS_ERR(page)) {
+   vmf->page = page;
+   ret = 0;
+   } else switch (PTR_ERR(page)) {
+   case -ENOSPC:
+   case -ENOMEM:
+   ret = VM_FAULT_OOM;
+   break;
+   case -EBUSY:
+   ret = VM_FAULT_RETRY;
+   break;
+   case -EFAULT:
+   case -EINVAL:
+   ret = VM_FAULT_SIGBUS;
+   break;
+   default:
+   WARN_ON(PTR_ERR(page));
+   ret = VM_FAULT_SIGBUS;
+   }
+
}
+   return ret;
 }
 
 static const struct vm_operations_struct vgem_gem_vm_ops = {
@@ -112,12 +143,8 @@ static void vgem_preclose(struct drm_device *dev, struct 
drm_file *file)
kfree(vfile);
 }
 
-/* ioctls */
-
-static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
- struct drm_file *file,
- unsigned int *handle,
- unsigned long size)
+static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device *dev,
+   unsigned long size)
 {
struct drm_vgem_gem_object *obj;
int ret;
@@ -127,8 +154,31 @@ static struct drm_gem_object *vgem_gem_create(struct 
drm_device *dev,
return ERR_PTR(-ENOMEM);
 
ret = drm_gem_object_init(dev, >base, roundup(size, PAGE_SIZE));
-   if (ret)
-   goto err_free;
+   if (ret) {
+   kfree(obj);
+   return ERR_PTR(ret);
+   }
+
+   return obj;
+}
+
+static void __vgem_gem_destroy(struct drm_vgem_gem_object *obj)
+{
+   drm_gem_object_release(>base);
+   kfree(obj);
+}
+
+static struct drm_gem_object *vgem_gem_create(struct drm_device *dev,
+ struct drm_file *file,
+  

[RFC PATCHv2 1/3] drm/vgem: Add a dummy platform device

[Laura Abbott]

The vgem driver is currently registered independent of any actual
device. Some usage of the dmabuf APIs require an actual device structure
to do anything. Register a dummy platform device for use with dmabuf.

Signed-off-by: Laura Abbott 
---
v2: Store the platform device in the platformdev field instead of the
regular device structure.
---
 drivers/gpu/drm/vgem/vgem_drv.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vgem/vgem_drv.c b/drivers/gpu/drm/vgem/vgem_drv.c
index a1f42d1..1b02e56 100644
--- a/drivers/gpu/drm/vgem/vgem_drv.c
+++ b/drivers/gpu/drm/vgem/vgem_drv.c
@@ -335,11 +335,20 @@ static int __init vgem_init(void)
int ret;
 
vgem_device = drm_dev_alloc(_driver, NULL);
-   if (IS_ERR(vgem_device)) {
-   ret = PTR_ERR(vgem_device);
+   if (IS_ERR(vgem_device))
+   return PTR_ERR(vgem_device);
+
+   vgem_device->platformdev = platform_device_register_simple("vgem",
+   -1, NULL, 0);
+
+   if (!vgem_device->platformdev) {
+   ret = -ENODEV;
goto out;
}
 
+   dma_coerce_mask_and_coherent(_device->platformdev->dev,
+   DMA_BIT_MASK(64));
+
ret  = drm_dev_register(vgem_device, 0);
if (ret)
goto out_unref;
@@ -347,13 +356,15 @@ static int __init vgem_init(void)
return 0;
 
 out_unref:
-   drm_dev_unref(vgem_device);
+   platform_device_unregister(vgem_device->platformdev);
 out:
+   drm_dev_unref(vgem_device);
return ret;
 }
 
 static void __exit vgem_exit(void)
 {
+   platform_device_unregister(vgem_device->platformdev);
drm_dev_unregister(vgem_device);
drm_dev_unref(vgem_device);
 }
-- 
2.7.4

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

[RFC PATCHv2 0/3] dma_buf import support for vgem

[Laura Abbott]

Hi,

This is v2 of my proposal to add dma_buf import functions for vgem.
Big changes from v1:

- A device is required for dma_buf attach to work. The existing vgem driver
intentionally does not use one as it provides a good way to test the DRM
framework. This approach instead puts a dummy platform device in the existing
drm_device->platformdev field and uses that for attaching.
- Native vgem buffers can still be faulted in a page at a time without
requiring the entire buffer be resident in memory.

I'm still marking this as RFC as I haven't had a chance to finish
a userspace test that can be integrated into igt.

Feedback appreciated as always.

Thanks,
Laura

Laura Abbott (3):
  drm/vgem: Add a dummy platform device
  drm/prime: Introduce drm_gem_prime_import_platform
  drm/vgem: Enable dmabuf import interfaces

 drivers/gpu/drm/drm_prime.c |  23 +-
 drivers/gpu/drm/vgem/vgem_drv.c | 150 +++-
 drivers/gpu/drm/vgem/vgem_drv.h |   2 +
 include/drm/drmP.h  |   5 ++
 4 files changed, 145 insertions(+), 35 deletions(-)

-- 
2.7.4

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

Re: [PATCH 1/2] drm/radeon: Avoid overflows/divide-by-zero in latency_watermark calculations.

[Alex Deucher]

On Sun, Apr 23, 2017 at 7:33 PM, Mario Kleiner
 wrote:
> At dot clocks > approx. 250 Mhz, some of these calcs will overflow and
> cause miscalculation of latency watermarks, and for some overflows also
> divide-by-zero driver crash. Make calcs more overflow resistant.
>
> This is a direct port of the corresponding patch from amdgpu-kms,
> copy-paste for cik from dce-8 and si from dce-6, with a slightly
> simpler variant for evergreen dce-4/5.
>
> Only tested on DCE-4 evergreen with a Radeon HD-5770.
>
> Signed-off-by: Mario Kleiner 
> Cc: Alex Deucher 

Applied the series.  thanks!

Alex

> ---
>  drivers/gpu/drm/radeon/cik.c   | 19 +++
>  drivers/gpu/drm/radeon/evergreen.c |  8 +---
>  drivers/gpu/drm/radeon/si.c| 19 +++
>  3 files changed, 7 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/cik.c b/drivers/gpu/drm/radeon/cik.c
> index 53710dd..4f034cb 100644
> --- a/drivers/gpu/drm/radeon/cik.c
> +++ b/drivers/gpu/drm/radeon/cik.c
> @@ -9150,23 +9150,10 @@ static u32 dce8_latency_watermark(struct 
> dce8_wm_params *wm)
> a.full = dfixed_const(available_bandwidth);
> b.full = dfixed_const(wm->num_heads);
> a.full = dfixed_div(a, b);
> +   tmp = div_u64((u64) dmif_size * (u64) wm->disp_clk, mc_latency + 512);
> +   tmp = min(dfixed_trunc(a), tmp);
>
> -   b.full = dfixed_const(mc_latency + 512);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(b, c);
> -
> -   c.full = dfixed_const(dmif_size);
> -   b.full = dfixed_div(c, b);
> -
> -   tmp = min(dfixed_trunc(a), dfixed_trunc(b));
> -
> -   b.full = dfixed_const(1000);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(c, b);
> -   c.full = dfixed_const(wm->bytes_per_pixel);
> -   b.full = dfixed_mul(b, c);
> -
> -   lb_fill_bw = min(tmp, dfixed_trunc(b));
> +   lb_fill_bw = min(tmp, wm->disp_clk * wm->bytes_per_pixel / 1000);
>
> a.full = dfixed_const(max_src_lines_per_dst_line * wm->src_width * 
> wm->bytes_per_pixel);
> b.full = dfixed_const(1000);
> diff --git a/drivers/gpu/drm/radeon/evergreen.c 
> b/drivers/gpu/drm/radeon/evergreen.c
> index d1b1e0c..3c9c133 100644
> --- a/drivers/gpu/drm/radeon/evergreen.c
> +++ b/drivers/gpu/drm/radeon/evergreen.c
> @@ -2188,13 +2188,7 @@ static u32 evergreen_latency_watermark(struct 
> evergreen_wm_params *wm)
> b.full = dfixed_const(wm->num_heads);
> a.full = dfixed_div(a, b);
>
> -   b.full = dfixed_const(1000);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(c, b);
> -   c.full = dfixed_const(wm->bytes_per_pixel);
> -   b.full = dfixed_mul(b, c);
> -
> -   lb_fill_bw = min(dfixed_trunc(a), dfixed_trunc(b));
> +   lb_fill_bw = min(dfixed_trunc(a), wm->disp_clk * wm->bytes_per_pixel 
> / 1000);
>
> a.full = dfixed_const(max_src_lines_per_dst_line * wm->src_width * 
> wm->bytes_per_pixel);
> b.full = dfixed_const(1000);
> diff --git a/drivers/gpu/drm/radeon/si.c b/drivers/gpu/drm/radeon/si.c
> index 528e5a4..3efdfd0 100644
> --- a/drivers/gpu/drm/radeon/si.c
> +++ b/drivers/gpu/drm/radeon/si.c
> @@ -2204,23 +2204,10 @@ static u32 dce6_latency_watermark(struct 
> dce6_wm_params *wm)
> a.full = dfixed_const(available_bandwidth);
> b.full = dfixed_const(wm->num_heads);
> a.full = dfixed_div(a, b);
> +   tmp = div_u64((u64) dmif_size * (u64) wm->disp_clk, mc_latency + 512);
> +   tmp = min(dfixed_trunc(a), tmp);
>
> -   b.full = dfixed_const(mc_latency + 512);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(b, c);
> -
> -   c.full = dfixed_const(dmif_size);
> -   b.full = dfixed_div(c, b);
> -
> -   tmp = min(dfixed_trunc(a), dfixed_trunc(b));
> -
> -   b.full = dfixed_const(1000);
> -   c.full = dfixed_const(wm->disp_clk);
> -   b.full = dfixed_div(c, b);
> -   c.full = dfixed_const(wm->bytes_per_pixel);
> -   b.full = dfixed_mul(b, c);
> -
> -   lb_fill_bw = min(tmp, dfixed_trunc(b));
> +   lb_fill_bw = min(tmp, wm->disp_clk * wm->bytes_per_pixel / 1000);
>
> a.full = dfixed_const(max_src_lines_per_dst_line * wm->src_width * 
> wm->bytes_per_pixel);
> b.full = dfixed_const(1000);
> --
> 2.7.4
>
> ___
> 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 1/2] drm/amdgpu: Add missing lb_vblank_lead_lines setup to DCE-6 path.

[Alex Deucher]

On Sun, Apr 23, 2017 at 7:02 PM, Mario Kleiner
 wrote:
> This apparently got lost when implementing the new DCE-6 support
> and would cause failures in pageflip scheduling and timestamping.
>
> Signed-off-by: Mario Kleiner 
> Cc: Alex Deucher 
> Cc: sta...@vger.kernel.org

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c 
> b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 307269b..e146d25 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -979,7 +979,7 @@ static void dce_v6_0_program_watermarks(struct 
> amdgpu_device *adev,
> u32 priority_a_mark = 0, priority_b_mark = 0;
> u32 priority_a_cnt = PRIORITY_OFF;
> u32 priority_b_cnt = PRIORITY_OFF;
> -   u32 tmp, arb_control3;
> +   u32 tmp, arb_control3, lb_vblank_lead_lines = 0;
> fixed20_12 a, b, c;
>
> if (amdgpu_crtc->base.enabled && num_heads && mode) {
> @@ -1091,6 +1091,8 @@ static void dce_v6_0_program_watermarks(struct 
> amdgpu_device *adev,
> c.full = dfixed_div(c, a);
> priority_b_mark = dfixed_trunc(c);
> priority_b_cnt |= priority_b_mark & PRIORITY_MARK_MASK;
> +
> +   lb_vblank_lead_lines = DIV_ROUND_UP(lb_size, 
> mode->crtc_hdisplay);
> }
>
> /* select wm A */
> @@ -1120,6 +1122,9 @@ static void dce_v6_0_program_watermarks(struct 
> amdgpu_device *adev,
> /* save values for DPM */
> amdgpu_crtc->line_time = line_time;
> amdgpu_crtc->wm_high = latency_watermark_a;
> +
> +   /* Save number of lines the linebuffer leads before the scanout */
> +   amdgpu_crtc->lb_vblank_lead_lines = lb_vblank_lead_lines;
>  }
>
>  /* watermark setup */
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 100802] [regression] mostly blank graphics on Faeria

[bugzilla-daemon]

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

Bug ID: 100802
   Summary: [regression] mostly blank graphics on Faeria
   Product: Mesa
   Version: 17.0
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: Drivers/Gallium/radeonsi
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: tjaal...@ubuntu.com
QA Contact: dri-devel@lists.freedesktop.org

Running free-to-play Faeria from Steam on Ubuntu the opening graphics shows
only the login window, background is totally blank. Mesa 12.0.6 was fine,
17.0.4 is not.

Tested on R7 260X (Bonaire), i965 is working fine with both Mesa versions.

-- 
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 v8 1/2] drm: Unplug drm device when unregistering it

[Sean Paul]

On Wed, Apr 26, 2017 at 10:43:31PM +0300, Ville Syrjälä wrote:
> On Wed, Apr 12, 2017 at 10:55:29AM +0800, Jeffy Chen wrote:
> > After unbinding drm, the user space may still owns the drm dev fd, and
> > may still be able to call drm ioctl.
> > 
> > We're using an unplugged state to prevent something like that, so let's
> > reuse it here.
> > 
> > Also drop drm_unplug_dev, because it would be unused after other changes.
> > 
> > Signed-off-by: Jeffy Chen 
> > Reviewed-by: Sean Paul 
> > 
> > ---
> > 
> > Changes in v8:
> > Fix hang when unregistering drm dev with open_count 0
> > 
> > Changes in v7:
> > Address Sean Paul 's comments.
> > 
> > Changes in v6:
> > Address Daniel Vetter 's comments.
> > 
> > Changes in v5:
> > Fix wrong git account.
> > 
> > Changes in v2:
> > Fix some commit messages.
> > 
> >  drivers/gpu/drm/drm_drv.c | 19 +++
> >  drivers/gpu/drm/udl/udl_drv.c |  2 +-
> >  include/drm/drmP.h|  5 +++--
> >  include/drm/drm_drv.h |  1 -
> >  4 files changed, 7 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index b5c6bb4..cc2d018 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -355,22 +355,6 @@ void drm_put_dev(struct drm_device *dev)
> >  }
> >  EXPORT_SYMBOL(drm_put_dev);
> >  
> > -void drm_unplug_dev(struct drm_device *dev)
> > -{
> > -   /* for a USB device */
> > -   drm_dev_unregister(dev);
> > -
> > -   mutex_lock(_global_mutex);
> > -
> > -   drm_device_set_unplugged(dev);
> > -
> > -   if (dev->open_count == 0) {
> > -   drm_put_dev(dev);
> > -   }
> > -   mutex_unlock(_global_mutex);
> > -}
> > -EXPORT_SYMBOL(drm_unplug_dev);
> > -
> >  /*
> >   * DRM internal mount
> >   * We want to be able to allocate our own "struct address_space" to control
> > @@ -787,6 +771,8 @@ int drm_dev_register(struct drm_device *dev, unsigned 
> > long flags)
> > if (drm_core_check_feature(dev, DRIVER_MODESET))
> > drm_modeset_register_all(dev);
> >  
> > +   drm_device_set_plug_state(dev, true);
> 
> This makes me think this has something to do with actual plugs, be
> they the bath tub kind or some *ahem* other kind.
> 
> /methinks this should at least be called set_plugged_state or
> something like that. Or maybe there's an even better name that
> could be used?

thanks for reviewing this, Ville. fwiw, we decided this patch wasn't
worth carrying upstream (see my response to v11 in
<20170414151503.lmpp3udfuycavfki@art_vandelay>).

Sean

> 
> > +
> > ret = 0;
> >  
> > DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
> > @@ -826,6 +812,7 @@ void drm_dev_unregister(struct drm_device *dev)
> > drm_lastclose(dev);
> >  
> > dev->registered = false;
> > +   drm_device_set_plug_state(dev, false);
> >  
> > if (drm_core_check_feature(dev, DRIVER_MODESET))
> > drm_modeset_unregister_all(dev);
> > diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> > index cd8b017..5dbd916 100644
> > --- a/drivers/gpu/drm/udl/udl_drv.c
> > +++ b/drivers/gpu/drm/udl/udl_drv.c
> > @@ -108,7 +108,7 @@ static void udl_usb_disconnect(struct usb_interface 
> > *interface)
> > drm_kms_helper_poll_disable(dev);
> > udl_fbdev_unplug(dev);
> > udl_drop_usb(dev);
> > -   drm_unplug_dev(dev);
> > +   drm_dev_unregister(dev);
> >  }
> >  
> >  /*
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 3bfafcd..a9a5a64 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -488,10 +488,11 @@ static __inline__ int drm_core_check_feature(struct 
> > drm_device *dev,
> > return ((dev->driver->driver_features & feature) ? 1 : 0);
> >  }
> >  
> > -static inline void drm_device_set_unplugged(struct drm_device *dev)
> > +static inline void drm_device_set_plug_state(struct drm_device *dev,
> > +bool plugged)
> >  {
> > smp_wmb();
> > -   atomic_set(>unplugged, 1);
> > +   atomic_set(>unplugged, !plugged);
> >  }
> >  
> >  static inline int drm_device_is_unplugged(struct drm_device *dev)
> > diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> > index 0fefc3f..eb63078 100644
> > --- a/include/drm/drm_drv.h
> > +++ b/include/drm/drm_drv.h
> > @@ -544,7 +544,6 @@ void drm_dev_unregister(struct drm_device *dev);
> >  void drm_dev_ref(struct drm_device *dev);
> >  void drm_dev_unref(struct drm_device *dev);
> >  void drm_put_dev(struct drm_device *dev);
> > -void drm_unplug_dev(struct drm_device *dev);
> >  
> >  int drm_dev_set_unique(struct drm_device *dev, const char *name);
> >  
> > -- 
> > 2.1.4
> > 
> > 
> > ___
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Sean Paul, 

[Bug 100800] With KMS:No link with displayport and A6-3600 APU from 4.4.x to 4.11.0.rc8, unless nomodeset kernel boot parameter

[bugzilla-daemon]

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

--- Comment #1 from Alex Deucher  ---
Please attach your xorg log and dmesg output.

-- 
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 v8 1/2] drm: Unplug drm device when unregistering it

[Ville Syrjälä]

On Wed, Apr 12, 2017 at 10:55:29AM +0800, Jeffy Chen wrote:
> After unbinding drm, the user space may still owns the drm dev fd, and
> may still be able to call drm ioctl.
> 
> We're using an unplugged state to prevent something like that, so let's
> reuse it here.
> 
> Also drop drm_unplug_dev, because it would be unused after other changes.
> 
> Signed-off-by: Jeffy Chen 
> Reviewed-by: Sean Paul 
> 
> ---
> 
> Changes in v8:
> Fix hang when unregistering drm dev with open_count 0
> 
> Changes in v7:
> Address Sean Paul 's comments.
> 
> Changes in v6:
> Address Daniel Vetter 's comments.
> 
> Changes in v5:
> Fix wrong git account.
> 
> Changes in v2:
> Fix some commit messages.
> 
>  drivers/gpu/drm/drm_drv.c | 19 +++
>  drivers/gpu/drm/udl/udl_drv.c |  2 +-
>  include/drm/drmP.h|  5 +++--
>  include/drm/drm_drv.h |  1 -
>  4 files changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index b5c6bb4..cc2d018 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -355,22 +355,6 @@ void drm_put_dev(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_put_dev);
>  
> -void drm_unplug_dev(struct drm_device *dev)
> -{
> - /* for a USB device */
> - drm_dev_unregister(dev);
> -
> - mutex_lock(_global_mutex);
> -
> - drm_device_set_unplugged(dev);
> -
> - if (dev->open_count == 0) {
> - drm_put_dev(dev);
> - }
> - mutex_unlock(_global_mutex);
> -}
> -EXPORT_SYMBOL(drm_unplug_dev);
> -
>  /*
>   * DRM internal mount
>   * We want to be able to allocate our own "struct address_space" to control
> @@ -787,6 +771,8 @@ int drm_dev_register(struct drm_device *dev, unsigned 
> long flags)
>   if (drm_core_check_feature(dev, DRIVER_MODESET))
>   drm_modeset_register_all(dev);
>  
> + drm_device_set_plug_state(dev, true);

This makes me think this has something to do with actual plugs, be
they the bath tub kind or some *ahem* other kind.

/methinks this should at least be called set_plugged_state or
something like that. Or maybe there's an even better name that
could be used?

> +
>   ret = 0;
>  
>   DRM_INFO("Initialized %s %d.%d.%d %s for %s on minor %d\n",
> @@ -826,6 +812,7 @@ void drm_dev_unregister(struct drm_device *dev)
>   drm_lastclose(dev);
>  
>   dev->registered = false;
> + drm_device_set_plug_state(dev, false);
>  
>   if (drm_core_check_feature(dev, DRIVER_MODESET))
>   drm_modeset_unregister_all(dev);
> diff --git a/drivers/gpu/drm/udl/udl_drv.c b/drivers/gpu/drm/udl/udl_drv.c
> index cd8b017..5dbd916 100644
> --- a/drivers/gpu/drm/udl/udl_drv.c
> +++ b/drivers/gpu/drm/udl/udl_drv.c
> @@ -108,7 +108,7 @@ static void udl_usb_disconnect(struct usb_interface 
> *interface)
>   drm_kms_helper_poll_disable(dev);
>   udl_fbdev_unplug(dev);
>   udl_drop_usb(dev);
> - drm_unplug_dev(dev);
> + drm_dev_unregister(dev);
>  }
>  
>  /*
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 3bfafcd..a9a5a64 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -488,10 +488,11 @@ static __inline__ int drm_core_check_feature(struct 
> drm_device *dev,
>   return ((dev->driver->driver_features & feature) ? 1 : 0);
>  }
>  
> -static inline void drm_device_set_unplugged(struct drm_device *dev)
> +static inline void drm_device_set_plug_state(struct drm_device *dev,
> +  bool plugged)
>  {
>   smp_wmb();
> - atomic_set(>unplugged, 1);
> + atomic_set(>unplugged, !plugged);
>  }
>  
>  static inline int drm_device_is_unplugged(struct drm_device *dev)
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 0fefc3f..eb63078 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -544,7 +544,6 @@ void drm_dev_unregister(struct drm_device *dev);
>  void drm_dev_ref(struct drm_device *dev);
>  void drm_dev_unref(struct drm_device *dev);
>  void drm_put_dev(struct drm_device *dev);
> -void drm_unplug_dev(struct drm_device *dev);
>  
>  int drm_dev_set_unique(struct drm_device *dev, const char *name);
>  
> -- 
> 2.1.4
> 
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [RFC 0/4] Exynos DRM: add Picture Processor extension

[Tobias Jakobi]

Hey,

Nicolas Dufresne wrote:
> Le mercredi 26 avril 2017 à 18:52 +0200, Tobias Jakobi a écrit :
>> Hello again,
>>
>>
>> Nicolas Dufresne wrote:
>>> Le mercredi 26 avril 2017 à 01:21 +0300, Sakari Ailus a écrit :
 Hi Marek,

 On Thu, Apr 20, 2017 at 01:23:09PM +0200, Marek Szyprowski wrote:
> Hi Laurent,
>
> On 2017-04-20 12:25, Laurent Pinchart wrote:
>> Hi Marek,
>>
>> (CC'ing Sakari Ailus)
>>
>> Thank you for the patches.
>>
>> On Thursday 20 Apr 2017 11:13:36 Marek Szyprowski wrote:
>>> Dear all,
>>>
>>> This is an updated proposal for extending EXYNOS DRM API with generic
>>> support for hardware modules, which can be used for processing image 
>>> data
>>> from the one memory buffer to another. Typical memory-to-memory 
>>> operations
>>> are: rotation, scaling, colour space conversion or mix of them. This is 
>>> a
>>> follow-up of my previous proposal "[RFC 0/2] New feature: Framebuffer
>>> processors", which has been rejected as "not really needed in the DRM
>>> core":
>>> http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg146286.html
>>>
>>> In this proposal I moved all the code to Exynos DRM driver, so now this
>>> will be specific only to Exynos DRM. I've also changed the name from
>>> framebuffer processor (fbproc) to picture processor (pp) to avoid 
>>> confusion
>>> with fbdev API.
>>>
>>> Here is a bit more information what picture processors are:
>>>
>>> Embedded SoCs are known to have a number of hardware blocks, which 
>>> perform
>>> such operations. They can be used in paralel to the main GPU module to
>>> offload CPU from processing grapics or video data. One of example use of
>>> such modules is implementing video overlay, which usually requires color
>>> space conversion from NV12 (or similar) to RGB32 color space and 
>>> scaling to
>>> target window size.
>>>
>>> The proposed API is heavily inspired by atomic KMS approach - it is also
>>> based on DRM objects and their properties. A new DRM object is 
>>> introduced:
>>> picture processor (called pp for convenience). Such objects have a set 
>>> of
>>> standard DRM properties, which describes the operation to be performed 
>>> by
>>> respective hardware module. In typical case those properties are a 
>>> source
>>> fb id and rectangle (x, y, width, height) and destination fb id and
>>> rectangle. Optionally a rotation property can be also specified if
>>> supported by the given hardware. To perform an operation on image data,
>>> userspace provides a set of properties and their values for given fbproc
>>> object in a similar way as object and properties are provided for
>>> performing atomic page flip / mode setting.
>>>
>>> The proposed API consists of the 3 new ioctls:
>>> - DRM_IOCTL_EXYNOS_PP_GET_RESOURCES: to enumerate all available picture
>>>   processors,
>>> - DRM_IOCTL_EXYNOS_PP_GET: to query capabilities of given picture
>>>   processor,
>>> - DRM_IOCTL_EXYNOS_PP_COMMIT: to perform operation described by given
>>>   property set.
>>>
>>> The proposed API is extensible. Drivers can attach their own, custom
>>> properties to add support for more advanced picture processing (for 
>>> example
>>> blending).
>>>
>>> This proposal aims to replace Exynos DRM IPP (Image Post Processing)
>>> subsystem. IPP API is over-engineered in general, but not really 
>>> extensible
>>> on the other side. It is also buggy, with significant design flaws - the
>>> biggest issue is the fact that the API covers memory-2-memory picture
>>> operations together with CRTC writeback and duplicating features, which
>>> belongs to video plane. Comparing with IPP subsystem, the PP framework 
>>> is
>>> smaller (1807 vs 778 lines) and allows driver simplification (Exynos
>>> rotator driver smaller by over 200 lines).
>>>
>>> Just a side note, we have written code in GStreamer using the Exnynos 4
>>> FIMC IPP driver. I don't know how many, if any, deployment still exist
>>> (Exynos 4 is relatively old now), but there exist userspace for the
>>> FIMC driver.
>>
>> I was searching for this code, but I didn't find anything. Are you sure
>> you really mean the FIMC IPP in Exynos DRM, and not just the FIMC driver
>> from the V4L2 subsystem?
> 
> Oops, I manage to be unclear. Having two drivers on the same IP isn't
> helping. We wrote code around the FIMC driver on V4L2 side. This
> driver:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/exynos4-is/fimc-m2m.c
> 
> And this code:
> 
> https://cgit.freedesktop.org/gstreamer/gst-plugins-good/tree/sys/v4l2/gstv4l2transform.c
> 
> Unless I have miss-read, the proposal here is to deprecate the V4L side
> and improve the DRM 

Re: [PATCH] drm/nouveau: Add support for clockgating on Fermi+

[Lyude Paul]

On Wed, 2017-04-26 at 00:49 +0200, Karol Herbst wrote:
> Hi Lyude,
> 
> thanks for the great work. Just a view comments inline.
> 
> 2017-04-25 20:38 GMT+02:00 Lyude :
> > This adds support for enabling automatic clockgating on nvidia GPUs
> > for
> > Fermi and later generations. This saves a little bit of power,
> > bringing
> > my fermi GPU's power consumption from ~28.3W on idle to ~27W, and
> > my
> > kepler's idle power consumption from ~23.6W to ~21.65W.
> > 
> > Similar to how the nvidia driver seems to handle this, we enable
> > clockgating for each engine that supports it after it's
> > initialization.
> > 
> > Signed-off-by: Lyude 
> > ---
> >  .../gpu/drm/nouveau/include/nvkm/subdev/therm.h|  4 ++
> >  drivers/gpu/drm/nouveau/nvkm/core/engine.c | 20 +-
> >  drivers/gpu/drm/nouveau/nvkm/engine/device/base.c  | 14 ++--
> >  drivers/gpu/drm/nouveau/nvkm/subdev/therm/Kbuild   |  2 +
> >  drivers/gpu/drm/nouveau/nvkm/subdev/therm/base.c   |  2 +
> >  .../gpu/drm/nouveau/nvkm/subdev/therm/clkgate.c| 49
> > ++
> >  drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf100.c  | 77
> > ++
> >  drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf119.c  |  2 +
> >  drivers/gpu/drm/nouveau/nvkm/subdev/therm/gm107.c  |  2 +
> >  drivers/gpu/drm/nouveau/nvkm/subdev/therm/gt215.c  |  2 +-
> >  drivers/gpu/drm/nouveau/nvkm/subdev/therm/priv.h   | 10 +++
> >  11 files changed, 175 insertions(+), 9 deletions(-)
> >  create mode 100644
> > drivers/gpu/drm/nouveau/nvkm/subdev/therm/clkgate.c
> >  create mode 100644
> > drivers/gpu/drm/nouveau/nvkm/subdev/therm/gf100.c
> > 
> > diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
> > b/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
> > index b268b96..904aa56 100644
> > --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
> > +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/therm.h
> > @@ -84,6 +84,9 @@ struct nvkm_therm {
> > 
> > int (*attr_get)(struct nvkm_therm *, enum
> > nvkm_therm_attr_type);
> > int (*attr_set)(struct nvkm_therm *, enum
> > nvkm_therm_attr_type, int);
> > +
> > +   int  (*clkgate_engine)(struct nvkm_therm *, enum
> > nvkm_devidx);
> > +   void (*clkgate_set)(struct nvkm_therm *, int gate_idx, bool
> > enable);
> 
> remove those and have a simple "nvkm_therm_clkgate_engine" function
> 
> This way you know that every user calls this function and don't have
> to check for silly function pointers like you currently do in
> engine.c
> 
> >  };
> > 
> >  int nvkm_therm_temp_get(struct nvkm_therm *);
> > @@ -94,6 +97,7 @@ int nv40_therm_new(struct nvkm_device *, int,
> > struct nvkm_therm **);
> >  int nv50_therm_new(struct nvkm_device *, int, struct nvkm_therm
> > **);
> >  int g84_therm_new(struct nvkm_device *, int, struct nvkm_therm
> > **);
> >  int gt215_therm_new(struct nvkm_device *, int, struct nvkm_therm
> > **);
> > +int gf100_therm_new(struct nvkm_device *, int, struct nvkm_therm
> > **);
> >  int gf119_therm_new(struct nvkm_device *, int, struct nvkm_therm
> > **);
> >  int gm107_therm_new(struct nvkm_device *, int, struct nvkm_therm
> > **);
> >  #endif
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/core/engine.c
> > b/drivers/gpu/drm/nouveau/nvkm/core/engine.c
> > index b6c9169..473ad3e 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/core/engine.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/core/engine.c
> > @@ -26,6 +26,7 @@
> >  #include 
> > 
> >  #include 
> > +#include 
> > 
> >  bool
> >  nvkm_engine_chsw_load(struct nvkm_engine *engine)
> > @@ -86,6 +87,13 @@ static int
> >  nvkm_engine_fini(struct nvkm_subdev *subdev, bool suspend)
> >  {
> > struct nvkm_engine *engine = nvkm_engine(subdev);
> > +   struct nvkm_therm *therm = subdev->device->therm;
> > +   int gate_idx;
> > +
> > +   gate_idx = therm->clkgate_engine(therm, subdev->index);
> > +   if (gate_idx != -1)
> > +   therm->clkgate_set(therm, gate_idx, false);
> > +
> 
> move this code inside "nvkm_therm_clkgate_engine". Nobody outside
> nvkm_therm should even care about the index.
> 
> > if (engine->func->fini)
> > return engine->func->fini(engine, suspend);
> > return 0;
> > @@ -96,12 +104,13 @@ nvkm_engine_init(struct nvkm_subdev *subdev)
> >  {
> > struct nvkm_engine *engine = nvkm_engine(subdev);
> > struct nvkm_fb *fb = subdev->device->fb;
> > +   struct nvkm_therm *therm = subdev->device->therm;
> > int ret = 0, i;
> > s64 time;
> > 
> > if (!engine->usecount) {
> > nvkm_trace(subdev, "init skipped, engine has no
> > users\n");
> > -   return ret;
> > +   goto finish;
> > }
> > 
> > if (engine->func->oneinit && !engine->subdev.oneinit) {
> > @@ -123,6 +132,15 @@ nvkm_engine_init(struct nvkm_subdev *subdev)
> > 
> > for (i = 0; fb && i < fb->tile.regions; i++)
> > 

Re: [PATCH RFC 3/6] drm: Plane YCbCr to RGB conversion related properties

[Ville Syrjälä]

On Wed, Apr 26, 2017 at 03:56:54PM +0300, Jyri Sarha wrote:
> On 04/24/17 19:55, Ville Syrjälä wrote:
> >> In fact we have plane specific YCbCr to RGB CSC (only preoffset
> >> possible), then (per crtc) gamma table, and finally a (per crtc) RGB to
> >> YCbCr CSC with optional post offset (so it can be used either as CSC or
> >> CTM).
> > So with that plane hw you could perhaps do:
> > - YCbCr->RGB if you input is not linear, but then you must
> >   blend using non-linear data
> > - colorspace conversion if your input is alredy linear
> > 
> > And with your crtc hw you could do:
> > - degamma + CTM
> > - gamma + RGB->YCbCr
> 
> 
> Just a generic question. Shouldn't - in an ideal HW - the degamma phase
> and the CTM be a plane specific property?
> 
> I mean, isn't the purpose of normalizing the non linear RGB to linear
> (and possibly converting the color space) to have same format for all
> plane data before blending and composing them together?
> 
> Of course it does not matter if all the planes use the same color space,
> which then should be converted to something else for the output.
> 
> ... or have I misunderstood something?

I think the pipeline we want to expose is

 planes:crtc:
 YCbCr->RGB -> degamma -> CTM -> (gamma) \
 YCbCr->RGB -> degamma -> CTM -> (gamma) -> (degamma) -> CTM -> gamma -> 
RGB->YCbCr
 ... /

I put the plane gamma and crtc degamma in parentheses because ideally
you perhaps wouldn't need them (since you want to blend with linear
data). But we have hardware which has them, and might be lacking some
of the other stages so they can actually be useful. Eg. if you don't
have plane gamma/degamma and you want to use the crtc CTM to change
the colorspace you would also use the crtc degamma.

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

Re: [PATCH 3/4] x86/PCI: Enable a 64bit BAR on AMD Family 15h (Models 30h-3fh) Processors v2

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 4:19 PM, Christian König
 wrote:
> From: Christian König 
>
> Most BIOS don't enable this because of compatibility reasons.
>
> Manually enable a 64bit BAR of 64GB size so that we have
> enough room for PCI devices.

> +static void pci_amd_enable_64bit_bar(struct pci_dev *dev)
> +{

> +   uint32_t base, limit, high;

Is u32 or uint32_t used actually in this file?

> +   struct resource *res, *conflict;

> +   unsigned i;

unsigned int i;

Reversed tree order.

> +   for (i = 0; i < 8; ++i) {
> +   pci_read_config_dword(dev, 0x80 + i * 0x8, );
> +   pci_read_config_dword(dev, 0x180 + i * 0x4, );
> +
> +   /* Is this slot free? */
> +   if ((base & 0x3) == 0x0)
> +   break;
> +
> +   base >>= 8;
> +   base |= high << 24;
> +
> +   /* Abort if a slot already configures a 64bit BAR. */
> +   if (base > 0x1)
> +   return;
> +   }
> +   if (i == 8)
> +   return;
> +
> +   res = kzalloc(sizeof(*res), GFP_KERNEL);
> +   if (!res)
> +   return;
> +
> +   res->name = "PCI Bus :00";

> +   res->flags = IORESOURCE_PREFETCH | IORESOURCE_MEM |
> +   IORESOURCE_MEM_64 | IORESOURCE_WINDOW;

You are using this constant twice AFAICS, perhaps define it in generic
pci.h and re-use?

> +   res->start = 0x1;
> +   res->end = 0xfd - 1;

Perhaps you forgot ul / ull in many places in your code. Care to
compile for 32-bit and fix the warnings?

> +   dev_info(>dev, "adding root bus resource %pR\n", res);

Some of your messages I think are rather dev_dbg() than dev_info().

> +   base = ((res->start >> 8) & 0xff00) | 0x3;
> +   limit = ((res->end + 1) >> 8) & 0xff00;
> +   high = ((res->start >> 40) & 0xff) |
> +   res->end + 1) >> 40) & 0xff) << 16);

It would be nice to have the magics defined (just on top of this function) like

#define PCI_AMD_BAR_XXX_MASK GENMASK(...)
#define PCI_AMD_BAR_XXX_SHIFT nn

> +   pci_write_config_dword(dev, 0x180 + i * 0x4, high);
> +   pci_write_config_dword(dev, 0x84 + i * 0x8, limit);
> +   pci_write_config_dword(dev, 0x80 + i * 0x8, base);

Ditto for pos:s.

> +
> +   pci_bus_add_resource(dev->bus, res, 0);
> +}

-- 
With Best Regards,
Andy Shevchenko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/pl111: Register the clock divider and use it.

[Eric Anholt]

Linus Walleij  writes:

> On Mon, Apr 24, 2017 at 9:45 PM, Eric Anholt  wrote:
>
>> This is required for the panel to work on bcm911360, where CLCDCLK is
>> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
>> CLCDCLK, for platforms that have a settable rate on that one.
>>
>> Signed-off-by: Eric Anholt 
>
> I like this, it is pretty.

An aside, for anyone else considering common clk and thinking "this is a
bit more complicated than I need": It's really nice for platform
debugging to have your divider or mux or whatever show up in
/debug/clk/clk_summary.  I don't know how many times I've been saved by
diffing that file between a good and bad state of my platform.


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

[GIT PULL v2] drm/mali-dp changes for v4.12 (or drm-next)

[Liviu Dudau]

Hi Dave,

Updated pull request to fix the all-modconfig build on 32bit arm
builds.

This pull request includes the latest updates on Mali DP, adding
support for colour management, plane scaling and power management.
The patches have trickled into the mailing list since February, and
have baked on linux-next for a few weeks (strangely, the 64bit div
was only caught yesterday).

Please pull.

Many thanks,
Liviu

The following changes since commit 6b1462700b4a6a1244c018cdd2fa97ded43090a0:

  Merge tag 'drm-misc-next-fixes-2017-04-20' of 
git://anongit.freedesktop.org/git/drm-misc into drm-next (2017-04-21 13:51:59 
+1000)

are available in the git repository at:

  git://linux-arm.org/linux-ld.git for-upstream/mali-dp

for you to fetch changes up to 763656d30b3d220a52e4b2bd1fc174a4cd6c0f43:

  drm: mali-dp: use div_u64 for expensive 64-bit divisions (2017-04-26 17:54:58 
+0100)


Arnd Bergmann (2):
  drm: mali-dp: remove unused variable
  drm: mali-dp: use div_u64 for expensive 64-bit divisions

Liviu Dudau (3):
  drm: mali-dp: Update the state of all planes before re-enabling active 
CRTCs.
  drm: mali-dp: Enable power management for the device.
  drm/mali-dp: Add core_id file to the sysfs interface

Mihail Atanassov (8):
  drm: mali-dp: add atomic_print_state for planes
  drm: mali-dp: add custom reset hook for planes
  drm: mali-dp: add malidp_crtc_state struct
  drm: mali-dp: enable gamma support
  drm: mali-dp: Add CTM support
  drm: mali-dp: Add plane upscaling support
  drm: mali-dp: Enable image enhancement when scaling
  drm: mali-dp: Check the mclk rate and allow up/down scaling

 drivers/gpu/drm/arm/malidp_crtc.c   | 341 ++--
 drivers/gpu/drm/arm/malidp_drv.c| 307 +---
 drivers/gpu/drm/arm/malidp_drv.h|  13 ++
 drivers/gpu/drm/arm/malidp_hw.c | 213 ++
 drivers/gpu/drm/arm/malidp_hw.h |  81 +
 drivers/gpu/drm/arm/malidp_planes.c | 108 ++--
 drivers/gpu/drm/arm/malidp_regs.h   |  72 +++-
 7 files changed, 1073 insertions(+), 62 deletions(-)

-- 

| 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: [PATCH 2/4] PCI: add functionality for resizing resources v3

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 4:19 PM, Christian König
 wrote:
> From: Christian König 
>
> This allows device drivers to request resizing their BARs.
>
> The function only tries to reprogram the windows of the bridge directly above
> the requesting device and only the BAR of the same type (usually mem, 64bit,
> prefetchable). This is done to make sure not to disturb other drivers by
> changing the BARs of their devices.
>
> If reprogramming the bridge BAR fails the old status is restored and -ENOSPC
> returned to the calling device driver.

> +int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
> +{
> +   const unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
> +   IORESOURCE_PREFETCH | IORESOURCE_MEM_64;

> +

Redundant.

> +   struct pci_dev_resource *dev_res;
> +   LIST_HEAD(saved);
> +   LIST_HEAD(added);
> +   LIST_HEAD(failed);

> +   unsigned i;

unsigned int i;

> +   int ret = 0;
> +
> +   /* Walk to the root BUS, releasing bridge BARs when possible */

> +   while (1) {

This raises red flag. Care to refactor?
Also do {} while () syntax allows faster to get that the loop body
goes at least once.

> +   for (i = PCI_BRIDGE_RESOURCES; i < PCI_BRIDGE_RESOURCE_END;
> +i++) {

> +   struct resource *res = >resource[i];
> +

> +   if ((res->flags & type_mask) != (type & type_mask))

I would rather go with

if ((res->flags ^ type) & type_mask)

> +   continue;
> +
> +   /* Ignore BARs which are still in use */
> +   if (res->child)
> +   continue;
> +
> +   ret = add_to_list(, bridge, res, 0, 0);
> +   if (ret)
> +   goto cleanup;
> +
> +   if (res->parent)
> +   release_resource(res);

> +   res->start = 0;
> +   res->end = 0;

> +   dev_info(>dev, "BAR %d: released %pR\n",
> +i, res);

I doesn't make much sense to me after zeroing a range.

> +   break;
> +   }
> +   if (i == PCI_BRIDGE_RESOURCE_END)
> +   break;
> +
> +   if (!bridge->bus || !bridge->bus->self)
> +   break;
> +
> +   bridge = bridge->bus->self;
> +   }
> +
> +   if (list_empty())
> +   return -ENOENT;
> +
> +   __pci_bus_size_bridges(bridge->subordinate, );
> +   __pci_bridge_assign_resources(bridge, , );
> +   BUG_ON(!list_empty());
> +
> +   if (!list_empty()) {
> +   ret = -ENOSPC;
> +   goto cleanup;
> +   }

> +
> +

Remove extra empty line.

> +   list_for_each_entry(dev_res, , list) {
> +   /* Skip the bridge we just assigned resources for. */
> +   if (bridge == dev_res->dev)
> +   continue;
> +
> +   bridge = dev_res->dev;
> +   pci_setup_bridge(bridge->subordinate);
> +   }
> +

> +   free_list();
> +   free_list();
> +   return ret;

You might re-use two lines with below, but perhaps better to show
which case returns 0 explicitly and drop assignment ret = 0 above.

> +
> +cleanup:
> +   /* restore size and flags */
> +   list_for_each_entry(dev_res, , list) {
> +   struct resource *res = dev_res->res;
> +
> +   res->start = dev_res->start;
> +   res->end = dev_res->end;
> +   res->flags = dev_res->flags;
> +   }
> +   free_list();
> +
> +   /* Revert to the old configuration */
> +   list_for_each_entry(dev_res, , list) {
> +   struct resource *res = dev_res->res;
> +
> +   bridge = dev_res->dev;
> +   i = res - bridge->resource;
> +
> +   res->start = dev_res->start;
> +   res->end = dev_res->end;
> +   res->flags = dev_res->flags;
> +
> +   pci_claim_resource(bridge, i);
> +   pci_setup_bridge(bridge->subordinate);
> +   }

> +   free_list();
> +
> +   return ret;

> +}

> +void pci_release_resource(struct pci_dev *dev, int resno)
> +{
> +   struct resource *res = dev->resource + resno;
> +
> +   release_resource(res);

> +   res->end = resource_size(res) - 1;
> +   res->start = 0;
> +   res->flags |= IORESOURCE_UNSET;

> +   dev_info(>dev, "BAR %d: released %pR\n", resno, res);

Makes little sense to me after you cleared information out.

> +}
> +EXPORT_SYMBOL(pci_release_resource);
> +
> +int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> +{
> +   struct resource *res = dev->resource + resno;

> +   u64 bytes = 1ULL << (size + 20);

> +   

Re: [PATCH] drm/pl111: Register the clock divider and use it.

[Eric Anholt]

Linus Walleij  writes:

> On Mon, Apr 24, 2017 at 9:45 PM, Eric Anholt  wrote:
>> +static long pl111_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
>> +unsigned long *prate)
>> +{
>> +   int div = pl111_clk_div_choose_div(hw, rate, prate, true);
>
> ...which we seem to assume that we can, actually why do you pass
> this parameter set_parent at all? It is always true in this code.

Because the other caller just below passes false: When we're being asked
to set_rate, the parent rate has been set by the core and we just need
to choose our best divider given that.

>> +static int pl111_clk_div_set_rate(struct clk_hw *hw, unsigned long rate,
>> + unsigned long prate)
>> +{
>> +   struct pl111_drm_dev_private *priv =
>> +   container_of(hw, struct pl111_drm_dev_private, clk_div);
>> +   int div = pl111_clk_div_choose_div(hw, rate, , false);
>> +   u32 tim2;
>> +
>> +   spin_lock(>tim2_lock);
>> +   tim2 = readl(priv->regs + CLCD_TIM2);
>> +   tim2 &= ~(TIM2_BCD | TIM2_PCD_LO_MASK | TIM2_PCD_HI_MASK);
>> +
>> +   if (div == 1) {
>> +   tim2 |= TIM2_BCD;
>> +   } else {
>> +   div -= 2;
>> +   tim2 |= div & TIM2_PCD_LO_MASK;
>> +   tim2 |= (div >> TIM2_PCD_LO_BITS) << TIM2_PCD_HI_SHIFT;
>> +   }
>> +
>> +   writel(tim2, priv->regs + CLCD_TIM2);
>> +   spin_unlock(>tim2_lock);
>> +
>> +   return 0;
>> +}
>
> So this will write the divisor, which is nice. But what if we also need
> to change the rate of the parent?

The clk core will have already done that.

>> +static int
>> +pl111_init_clock_divider(struct drm_device *drm)
>> +{
>> +   struct pl111_drm_dev_private *priv = drm->dev_private;
>> +   struct clk *parent = devm_clk_get(drm->dev, "clcdclk");
>> +   struct clk_hw *div = >clk_div;
>> +   const char *parent_name;
>> +   struct clk_init_data init = {
>> +   .name = "pl111_div",
>> +   .ops = _clk_div_ops,
>> +   .parent_names = _name,
>> +   .num_parents = 1,
>> +   };
>
> I think it is necessary to set .flags in the init data to
> .flags = CLK_SET_RATE_PARENT,
> for this code to work with a parent that can change rate.

I was thinking this flag was used internally in things like
clk-divider.c, but the core uses it too.  I'll fix that, thanks!

>> - * - Use the internal clock divisor to reduce power consumption by
>> - *   using HCLK (apb_pclk) when appropriate.
>> + * - Use the CLKSEL bit to support switching between the two external
>> + *   clock parents.
>
> OK so that remains to be done. We discussed this previously
> so I got a bit confused. The divisor code seems fine, after this
> we only need some more code to choose the best parent for
> the divided clock.
>
> It seems that what would pe Perfect(TM) would be to calculate the
> best end result using clock A and the best end result using clock B,
> both utilizing the divisor, and then choose the best of those two.
>
> I think struct clk_mux is supposed to do that so it would eventually
> be:
>
> CLK A -|\_ clk_mux --> clk_divider --> pixel clock
> CLK B -|/

I agree.  This patch got things going for this platform, without needing
the bindings change (and helped confirm for me that I do understand the
platform design correctly).


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

Re: [RFC 0/4] Exynos DRM: add Picture Processor extension

[Tobias Jakobi]

Hello again,


Nicolas Dufresne wrote:
> Le mercredi 26 avril 2017 à 01:21 +0300, Sakari Ailus a écrit :
>> Hi Marek,
>>
>> On Thu, Apr 20, 2017 at 01:23:09PM +0200, Marek Szyprowski wrote:
>>> Hi Laurent,
>>>
>>> On 2017-04-20 12:25, Laurent Pinchart wrote:
 Hi Marek,

 (CC'ing Sakari Ailus)

 Thank you for the patches.

 On Thursday 20 Apr 2017 11:13:36 Marek Szyprowski wrote:
> Dear all,
>
> This is an updated proposal for extending EXYNOS DRM API with generic
> support for hardware modules, which can be used for processing image data
> from the one memory buffer to another. Typical memory-to-memory operations
> are: rotation, scaling, colour space conversion or mix of them. This is a
> follow-up of my previous proposal "[RFC 0/2] New feature: Framebuffer
> processors", which has been rejected as "not really needed in the DRM
> core":
> http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg146286.html
>
> In this proposal I moved all the code to Exynos DRM driver, so now this
> will be specific only to Exynos DRM. I've also changed the name from
> framebuffer processor (fbproc) to picture processor (pp) to avoid 
> confusion
> with fbdev API.
>
> Here is a bit more information what picture processors are:
>
> Embedded SoCs are known to have a number of hardware blocks, which perform
> such operations. They can be used in paralel to the main GPU module to
> offload CPU from processing grapics or video data. One of example use of
> such modules is implementing video overlay, which usually requires color
> space conversion from NV12 (or similar) to RGB32 color space and scaling 
> to
> target window size.
>
> The proposed API is heavily inspired by atomic KMS approach - it is also
> based on DRM objects and their properties. A new DRM object is introduced:
> picture processor (called pp for convenience). Such objects have a set of
> standard DRM properties, which describes the operation to be performed by
> respective hardware module. In typical case those properties are a source
> fb id and rectangle (x, y, width, height) and destination fb id and
> rectangle. Optionally a rotation property can be also specified if
> supported by the given hardware. To perform an operation on image data,
> userspace provides a set of properties and their values for given fbproc
> object in a similar way as object and properties are provided for
> performing atomic page flip / mode setting.
>
> The proposed API consists of the 3 new ioctls:
> - DRM_IOCTL_EXYNOS_PP_GET_RESOURCES: to enumerate all available picture
>   processors,
> - DRM_IOCTL_EXYNOS_PP_GET: to query capabilities of given picture
>   processor,
> - DRM_IOCTL_EXYNOS_PP_COMMIT: to perform operation described by given
>   property set.
>
> The proposed API is extensible. Drivers can attach their own, custom
> properties to add support for more advanced picture processing (for 
> example
> blending).
>
> This proposal aims to replace Exynos DRM IPP (Image Post Processing)
> subsystem. IPP API is over-engineered in general, but not really 
> extensible
> on the other side. It is also buggy, with significant design flaws - the
> biggest issue is the fact that the API covers memory-2-memory picture
> operations together with CRTC writeback and duplicating features, which
> belongs to video plane. Comparing with IPP subsystem, the PP framework is
> smaller (1807 vs 778 lines) and allows driver simplification (Exynos
> rotator driver smaller by over 200 lines).
> 
> Just a side note, we have written code in GStreamer using the Exnynos 4
> FIMC IPP driver. I don't know how many, if any, deployment still exist
> (Exynos 4 is relatively old now), but there exist userspace for the
> FIMC driver.
I was searching for this code, but I didn't find anything. Are you sure
you really mean the FIMC IPP in Exynos DRM, and not just the FIMC driver
from the V4L2 subsystem?


With best wishes,
Tobias



> We use this for color transformation (from tiled to
> linear) and scaling. The FIMC driver is in fact quite stable in
> upstream kernel today. The GScaler V4L2 M2M driver on Exynos 5 is
> largely based on it and has received some maintenance to properly work
> in GStreamer. unlike this DRM API, you can reuse the same userspace
> code across multiple platforms (which we do already). We have also
> integrated this driver in Chromium in the past (not upstream though).
> 
> I am well aware that the blitter driver has not got much attention
> though. But again, V4L2 offers a generic interface to userspace
> application. Fixing this driver could enable some work like this one:
> 
> https://bugzilla.gnome.org/show_bug.cgi?id=772766
> 
> This work in progress feature is a generic hardware accelerated video
> 

[PATCH v5 3/5] nouveau_hwmon: Remove old code, add .write/.read operations

[Oscar Salvador]

This patch removes old code related to the old api and transforms the
functions for the new api. It also adds the .write and .read operations.

Signed-off-by: Oscar Salvador 
---
 drivers/gpu/drm/nouveau/nouveau_hwmon.c | 722 +++-
 1 file changed, 249 insertions(+), 473 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c 
b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
index e8ea8d0..4db65fb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
@@ -38,21 +38,17 @@
 #include 
 
 #if defined(CONFIG_HWMON) || (defined(MODULE) && defined(CONFIG_HWMON_MODULE))
-static ssize_t
-nouveau_hwmon_show_temp(struct device *d, struct device_attribute *a, char 
*buf)
+static int
+nouveau_hwmon_show_temp(struct nouveau_drm *drm)
 {
-   struct drm_device *dev = dev_get_drvdata(d);
-   struct nouveau_drm *drm = nouveau_drm(dev);
struct nvkm_therm *therm = nvxx_therm(>client.device);
int temp = nvkm_therm_temp_get(therm);
 
if (temp < 0)
return temp;
 
-   return snprintf(buf, PAGE_SIZE, "%d\n", temp * 1000);
+   return (temp * 1000);
 }
-static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, nouveau_hwmon_show_temp,
- NULL, 0);
 
 static ssize_t
 nouveau_hwmon_show_temp1_auto_point1_pwm(struct device *d,
@@ -129,312 +125,100 @@ static SENSOR_DEVICE_ATTR(temp1_auto_point1_temp_hyst, 
S_IRUGO | S_IWUSR,
  nouveau_hwmon_temp1_auto_point1_temp_hyst,
  nouveau_hwmon_set_temp1_auto_point1_temp_hyst, 0);
 
-static ssize_t
-nouveau_hwmon_max_temp(struct device *d, struct device_attribute *a, char *buf)
-{
-   struct drm_device *dev = dev_get_drvdata(d);
-   struct nouveau_drm *drm = nouveau_drm(dev);
-   struct nvkm_therm *therm = nvxx_therm(>client.device);
-
-   return snprintf(buf, PAGE_SIZE, "%d\n",
-  therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK) * 1000);
-}
-static ssize_t
-nouveau_hwmon_set_max_temp(struct device *d, struct device_attribute *a,
-   const char *buf, size_t count)
+static int
+nouveau_hwmon_max_temp(struct nouveau_drm *drm)
 {
-   struct drm_device *dev = dev_get_drvdata(d);
-   struct nouveau_drm *drm = nouveau_drm(dev);
struct nvkm_therm *therm = nvxx_therm(>client.device);
-   long value;
 
-   if (kstrtol(buf, 10, ) == -EINVAL)
-   return count;
-
-   therm->attr_set(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK, value / 1000);
-
-   return count;
+   return therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK) * 1000;
 }
-static SENSOR_DEVICE_ATTR(temp1_max, S_IRUGO | S_IWUSR, nouveau_hwmon_max_temp,
- nouveau_hwmon_set_max_temp,
- 0);
 
-static ssize_t
-nouveau_hwmon_max_temp_hyst(struct device *d, struct device_attribute *a,
-   char *buf)
-{
-   struct drm_device *dev = dev_get_drvdata(d);
-   struct nouveau_drm *drm = nouveau_drm(dev);
-   struct nvkm_therm *therm = nvxx_therm(>client.device);
-
-   return snprintf(buf, PAGE_SIZE, "%d\n",
- therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK_HYST) * 1000);
-}
-static ssize_t
-nouveau_hwmon_set_max_temp_hyst(struct device *d, struct device_attribute *a,
-   const char *buf, size_t count)
+static int
+nouveau_hwmon_max_temp_hyst(struct nouveau_drm *drm)
 {
-   struct drm_device *dev = dev_get_drvdata(d);
-   struct nouveau_drm *drm = nouveau_drm(dev);
struct nvkm_therm *therm = nvxx_therm(>client.device);
-   long value;
-
-   if (kstrtol(buf, 10, ) == -EINVAL)
-   return count;
 
-   therm->attr_set(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK_HYST,
-   value / 1000);
-
-   return count;
+   return therm->attr_get(therm, NVKM_THERM_ATTR_THRS_DOWN_CLK_HYST) * 
1000;
 }
-static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IRUGO | S_IWUSR,
- nouveau_hwmon_max_temp_hyst,
- nouveau_hwmon_set_max_temp_hyst, 0);
-
-static ssize_t
-nouveau_hwmon_critical_temp(struct device *d, struct device_attribute *a,
-   char *buf)
-{
-   struct drm_device *dev = dev_get_drvdata(d);
-   struct nouveau_drm *drm = nouveau_drm(dev);
-   struct nvkm_therm *therm = nvxx_therm(>client.device);
 
-   return snprintf(buf, PAGE_SIZE, "%d\n",
-  therm->attr_get(therm, NVKM_THERM_ATTR_THRS_CRITICAL) * 1000);
-}
-static ssize_t
-nouveau_hwmon_set_critical_temp(struct device *d, struct device_attribute *a,
-   const char *buf,
-   size_t 

[PATCH v5 4/5] nouveau_hwmon: Add support for auto_point attributes

[Oscar Salvador]

This patch creates a special group attributes for attrs like "*auto_point*".
We check if we have support for them, and if we do, we gather them all in
an attribute_group's structure which is the parameter regarding special groups
of hwmon_device_register_with_info.

Signed-off-by: Oscar Salvador 
---
 drivers/gpu/drm/nouveau/nouveau_hwmon.c | 29 -
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c 
b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
index 4db65fb..9142779 100644
--- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
@@ -358,6 +358,23 @@ nouveau_hwmon_get_power1_crit(struct nouveau_drm *drm)
return iccsense->power_w_crit;
 }
 
+static struct attribute *pwm_fan_sensor_attrs[] = {
+   _dev_attr_pwm1_min.dev_attr.attr,
+   _dev_attr_pwm1_max.dev_attr.attr,
+   NULL
+};
+ATTRIBUTE_GROUPS(pwm_fan_sensor);
+
+static struct attribute *temp1_auto_point_sensor_attrs[] = {
+   _dev_attr_temp1_auto_point1_pwm.dev_attr.attr,
+   _dev_attr_temp1_auto_point1_temp.dev_attr.attr,
+   _dev_attr_temp1_auto_point1_temp_hyst.dev_attr.attr,
+   NULL
+};
+ATTRIBUTE_GROUPS(temp1_auto_point_sensor);
+
+#define N_ATTR_GROUPS   3
+
 static const u32 nouveau_config_chip[] = {
HWMON_C_UPDATE_INTERVAL,
0
@@ -792,17 +809,27 @@ nouveau_hwmon_init(struct drm_device *dev)
 #if defined(CONFIG_HWMON) || (defined(MODULE) && defined(CONFIG_HWMON_MODULE))
struct nouveau_drm *drm = nouveau_drm(dev);
struct nvkm_therm *therm = nvxx_therm(>client.device);
+   const struct attribute_group *special_groups[N_ATTR_GROUPS];
struct nouveau_hwmon *hwmon;
struct device *hwmon_dev;
int ret = 0;
+   int i = 0;
 
hwmon = drm->hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL);
if (!hwmon)
return -ENOMEM;
hwmon->dev = dev;
 
+   if (therm && therm->attr_get && therm->attr_set) {
+   if (nvkm_therm_temp_get(therm) >= 0)
+   special_groups[i++] = _auto_point_sensor_group;
+   if (therm->fan_get && therm->fan_get(therm) >= 0)
+   special_groups[i++] = _fan_sensor_group;
+   }
+
+   special_groups[i] = 0;
hwmon_dev = hwmon_device_register_with_info(dev->dev, "nouveau", dev,
-   _chip_info, NULL);
+   _chip_info, special_groups);
if (IS_ERR(hwmon_dev)) {
ret = PTR_ERR(hwmon_dev);
NV_ERROR(drm, "Unable to register hwmon device: %d\n", ret);
-- 
2.1.4

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

[PATCH v5 5/5] nouveau_hwmon: Change permissions to numeric

[Oscar Salvador]

This patch replaces the symbolic permissions with the numeric ones,
and adds me to the authors too.

Signed-off-by: Oscar Salvador 
---
 drivers/gpu/drm/nouveau/nouveau_hwmon.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c 
b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
index 9142779..45b5c85 100644
--- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
@@ -1,5 +1,6 @@
 /*
- * Copyright 2010 Red Hat Inc.
+ * Copyright 2010 Red Hat Inc. (Ben Skeggs)
+ * Copyright 2017 Oscar Salvador
  *
  * Permission is hereby granted, free of charge, to any person obtaining a
  * copy of this software and associated documentation files (the "Software"),
@@ -19,7 +20,6 @@
  * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
  * OTHER DEALINGS IN THE SOFTWARE.
  *
- * Authors: Ben Skeggs
  */
 
 #ifdef CONFIG_ACPI
@@ -56,7 +56,7 @@ nouveau_hwmon_show_temp1_auto_point1_pwm(struct device *d,
 {
return snprintf(buf, PAGE_SIZE, "%d\n", 100);
 }
-static SENSOR_DEVICE_ATTR(temp1_auto_point1_pwm, S_IRUGO,
+static SENSOR_DEVICE_ATTR(temp1_auto_point1_pwm, 0444,
  nouveau_hwmon_show_temp1_auto_point1_pwm, NULL, 0);
 
 static ssize_t
@@ -88,7 +88,7 @@ nouveau_hwmon_set_temp1_auto_point1_temp(struct device *d,
 
return count;
 }
-static SENSOR_DEVICE_ATTR(temp1_auto_point1_temp, S_IRUGO | S_IWUSR,
+static SENSOR_DEVICE_ATTR(temp1_auto_point1_temp, 0644,
  nouveau_hwmon_temp1_auto_point1_temp,
  nouveau_hwmon_set_temp1_auto_point1_temp, 0);
 
@@ -121,7 +121,7 @@ nouveau_hwmon_set_temp1_auto_point1_temp_hyst(struct device 
*d,
 
return count;
 }
-static SENSOR_DEVICE_ATTR(temp1_auto_point1_temp_hyst, S_IRUGO | S_IWUSR,
+static SENSOR_DEVICE_ATTR(temp1_auto_point1_temp_hyst, 0644,
  nouveau_hwmon_temp1_auto_point1_temp_hyst,
  nouveau_hwmon_set_temp1_auto_point1_temp_hyst, 0);
 
@@ -255,7 +255,7 @@ nouveau_hwmon_set_pwm1_min(struct device *d, struct 
device_attribute *a,
return count;
 }
 
-static SENSOR_DEVICE_ATTR(pwm1_min, S_IRUGO | S_IWUSR,
+static SENSOR_DEVICE_ATTR(pwm1_min, 0644,
  nouveau_hwmon_get_pwm1_min,
  nouveau_hwmon_set_pwm1_min, 0);
 
@@ -295,7 +295,7 @@ nouveau_hwmon_set_pwm1_max(struct device *d, struct 
device_attribute *a,
return count;
 }
 
-static SENSOR_DEVICE_ATTR(pwm1_max, S_IRUGO | S_IWUSR,
+static SENSOR_DEVICE_ATTR(pwm1_max, 0644,
  nouveau_hwmon_get_pwm1_max,
  nouveau_hwmon_set_pwm1_max, 0);
 
-- 
2.1.4

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

[PATCH v5 2/5] nouveau_hwmon: Add nouveau_hwmon_ops structure with .is_visible/.read_string

[Oscar Salvador]

This patch introduces the nouveau_hwmon_ops structure, sets up
.is_visible and .read_string operations and adds all the functions
for these operations.
This is also a preparation for the next patches, where most of the
work is being done.
This code doesn't interacture with the old one.
It's just to make easier the review of all patches.

Signed-off-by: Oscar Salvador 
---
 drivers/gpu/drm/nouveau/nouveau_hwmon.c | 170 
 1 file changed, 170 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c 
b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
index 24b40c5..e8ea8d0 100644
--- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
@@ -764,6 +764,176 @@ static const struct hwmon_channel_info *nouveau_info[] = {
_power,
NULL
 };
+
+static umode_t
+nouveau_chip_is_visible(const void *data, u32 attr, int channel)
+{
+   switch (attr) {
+   case hwmon_chip_update_interval:
+   return 0444;
+   default:
+   return 0;
+   }
+}
+
+static umode_t
+nouveau_power_is_visible(const void *data, u32 attr, int channel)
+{
+   struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+   struct nvkm_iccsense *iccsense = nvxx_iccsense(>client.device);
+
+   if (!iccsense)
+   return 0;
+
+   switch (attr) {
+   case hwmon_power_input:
+   if (iccsense->data_valid &&
+   !list_empty(>rails))
+   return 0444;
+   return 0;
+   case hwmon_power_max:
+   if (iccsense->power_w_max)
+   return 0444;
+   return 0;
+   case hwmon_power_crit:
+   if (iccsense->power_w_crit)
+   return 0444;
+   return 0;
+   default:
+   return 0;
+   }
+}
+
+static umode_t
+nouveau_temp_is_visible(const void *data, u32 attr, int channel)
+{
+   struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+   struct nvkm_therm *therm = nvxx_therm(>client.device);
+
+   if (therm &&
+   therm->attr_get &&
+   nvkm_therm_temp_get(therm) < 0)
+   return 0;
+
+   switch (attr) {
+   case hwmon_temp_input:
+   case hwmon_temp_max:
+   case hwmon_temp_max_hyst:
+   case hwmon_temp_crit:
+   case hwmon_temp_crit_hyst:
+   case hwmon_temp_emergency:
+   case hwmon_temp_emergency_hyst:
+   return 0444;
+   default:
+   return 0;
+   }
+}
+
+static umode_t
+nouveau_pwm_is_visible(const void *data, u32 attr, int channel)
+{
+   struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+   struct nvkm_therm *therm = nvxx_therm(>client.device);
+
+   if (therm &&
+   therm->attr_get &&
+   therm->fan_get &&
+   therm->fan_get(therm) < 0)
+   return 0;
+
+   switch (attr) {
+   case hwmon_pwm_enable:
+   case hwmon_pwm_input:
+   return 0644;
+   default:
+   return 0;
+   }
+}
+
+static umode_t
+nouveau_input_is_visible(const void *data, u32 attr, int channel)
+{
+   struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+   struct nvkm_volt *volt = nvxx_volt(>client.device);
+
+   if (!volt || nvkm_volt_get(volt) < 0)
+   return 0;
+
+   switch (attr) {
+   case hwmon_in_input:
+   case hwmon_in_label:
+   case hwmon_in_min:
+   case hwmon_in_max:
+   return 0444;
+   default:
+   return 0;
+   }
+}
+
+static umode_t
+nouveau_fan_is_visible(const void *data, u32 attr, int channel)
+{
+   struct nouveau_drm *drm = nouveau_drm((struct drm_device *)data);
+   struct nvkm_therm *therm = nvxx_therm(>client.device);
+
+   if (!therm || !therm->attr_get || nvkm_therm_fan_sense(therm) < 0)
+   return 0;
+
+   switch (attr) {
+   case hwmon_fan_input:
+   return 0444;
+   default:
+   return 0;
+   }
+}
+
+static umode_t
+nouveau_is_visible(const void *data, enum hwmon_sensor_types type, u32 attr,
+   int channel)
+{
+   switch (type) {
+   case hwmon_chip:
+   return nouveau_chip_is_visible(data, attr, channel);
+   case hwmon_temp:
+   return nouveau_temp_is_visible(data, attr, channel);
+   case hwmon_fan:
+   return nouveau_fan_is_visible(data, attr, channel);
+   case hwmon_in:
+   return nouveau_input_is_visible(data, attr, channel);
+   case hwmon_pwm:
+   return nouveau_pwm_is_visible(data, attr, channel);
+   case hwmon_power:
+   return nouveau_power_is_visible(data, attr, channel);
+   default:
+   return 0;
+   }
+}
+
+static const char 

[PATCH v5 1/5] nouveau_hwmon: Add config for all sensors and their settings

[Oscar Salvador]

This is a preparation for the next patches. It just adds the sensors with
their possible configurable settings and then fills the struct 
hwmon_channel_info
with all this information.

Signed-off-by: Oscar Salvador 
---
 drivers/gpu/drm/nouveau/nouveau_hwmon.c | 72 +
 1 file changed, 72 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nouveau_hwmon.c 
b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
index 23b1670..24b40c5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_hwmon.c
+++ b/drivers/gpu/drm/nouveau/nouveau_hwmon.c
@@ -692,6 +692,78 @@ static const struct attribute_group hwmon_power_attrgroup 
= {
 static const struct attribute_group hwmon_power_caps_attrgroup = {
.attrs = hwmon_power_caps_attributes,
 };
+
+static const u32 nouveau_config_chip[] = {
+   HWMON_C_UPDATE_INTERVAL,
+   0
+};
+
+static const u32 nouveau_config_in[] = {
+   HWMON_I_INPUT | HWMON_I_MIN | HWMON_I_MAX | HWMON_I_LABEL,
+   0
+};
+
+static const u32 nouveau_config_temp[] = {
+   HWMON_T_INPUT | HWMON_T_MAX | HWMON_T_MAX_HYST |
+   HWMON_T_CRIT | HWMON_T_CRIT_HYST | HWMON_T_EMERGENCY |
+   HWMON_T_EMERGENCY_HYST,
+   0
+};
+
+static const u32 nouveau_config_fan[] = {
+   HWMON_F_INPUT,
+   0
+};
+
+static const u32 nouveau_config_pwm[] = {
+   HWMON_PWM_INPUT | HWMON_PWM_ENABLE,
+   0
+};
+
+static const u32 nouveau_config_power[] = {
+   HWMON_P_INPUT | HWMON_P_CAP_MAX | HWMON_P_CRIT,
+   0
+};
+
+static const struct hwmon_channel_info nouveau_chip = {
+   .type = hwmon_chip,
+   .config = nouveau_config_chip,
+};
+
+static const struct hwmon_channel_info nouveau_temp = {
+   .type = hwmon_temp,
+   .config = nouveau_config_temp,
+};
+
+static const struct hwmon_channel_info nouveau_fan = {
+   .type = hwmon_fan,
+   .config = nouveau_config_fan,
+};
+
+static const struct hwmon_channel_info nouveau_in = {
+   .type = hwmon_in,
+   .config = nouveau_config_in,
+};
+
+static const struct hwmon_channel_info nouveau_pwm = {
+   .type = hwmon_pwm,
+   .config = nouveau_config_pwm,
+};
+
+static const struct hwmon_channel_info nouveau_power = {
+   .type = hwmon_power,
+   .config = nouveau_config_power,
+};
+
+static const struct hwmon_channel_info *nouveau_info[] = {
+   _chip,
+   _temp,
+   _fan,
+   _in,
+   _pwm,
+   _power,
+   NULL
+};
 #endif
 
 int
-- 
2.1.4

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

[PATCH v5 0/5] replace hwmon_device_register for hwmon_device_register_with_info

[Oscar Salvador]

This v5 drops a check for attr_set.

Versions:

v1 -> v2:
* Keep temp attrs as read only
v2 -> v3:
* Code fix-ups: struct and string as const and add return within switch
due to fallthrough
* Add Signed-off-by to all commits
v3 -> v4:
* Rever const to struct attribute. Kbuild complains.
v4 -> v5:
* Drops a check for attr_set in "nouveau_temp_is_visible".

This patchseries replaces the deprecated hwmon_device_register function with the
new one hwmon_device_register_with_info.
It also does some cleanup.

Oscar Salvador (5):
  nouveau_hwmon: Add config for all sensors and their settings
  nouveau_hwmon: Add nouveau_hwmon_ops structure with
.is_visible/.read_string
  nouveau_hwmon: Remove old code, add .write/.read operations
  nouveau_hwmon: Add support for auto_point attributes
  nouveau_hwmon: Change permissions to numeric

 drivers/gpu/drm/nouveau/nouveau_hwmon.c | 953 +---
 1 file changed, 499 insertions(+), 454 deletions(-)

-- 
2.1.4

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

Re: [PATCH 1/4] PCI: add resizeable BAR infrastructure v4

[Andy Shevchenko]

On Tue, Apr 25, 2017 at 4:19 PM, Christian König
 wrote:
> From: Christian König 
>
> Just the defines and helper functions to read the possible sizes of a BAR and
> update it's size.
>
> See 
> https://pcisig.com/sites/default/files/specification_documents/ECN_Resizable-BAR_24Apr2008.pdf
> and PCIe r3.1, sec 7.22.
>
> This is useful for hardware with large local storage (mostly GFX) which only
> expose 256MB BARs initially to be compatible with 32bit systems.

> +u32 pci_rbar_get_possible_sizes(struct pci_dev *pdev, int bar)
> +{

> +   unsigned pos, nbars;
> +   u32 ctrl, cap;
> +   unsigned i;

Are we supposed to use plain 'unsigned' nowadays? I would go with
'unsigned int'.

> +}

> + * Returns size if found or negativ error code.

Typo: negative.

> +int pci_rbar_get_current_size(struct pci_dev *pdev, int bar)
> +{
> +   unsigned pos, nbars;

> +   u32 ctrl;
> +   unsigned i;

Reversed tree order?

> +   for (i = 0; i < nbars; ++i, pos += 8) {
> +   int bar_idx;
> +
> +   pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, );
> +   bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
> +   PCI_REBAR_CTRL_BAR_IDX_SHIFT;
> +   if (bar_idx != bar)
> +   continue;
> +
> +   return (ctrl & PCI_REBAR_CTRL_BAR_SIZE_MASK) >>
> +   PCI_REBAR_CTRL_BAR_SIZE_SHIFT;
> +   }

This one the same as previous function, the difference only in what is
returned. CAre to split static helper function for both?

> +   return -ENOENT;
> +}

> +/**
> + * pci_rbar_set_size - set a new size for a BAR
> + * @dev: PCI device
> + * @bar: BAR to set size to

> + * @size: new size as defined in the spec (log2(size in bytes) - 20)

Not clear is it rounded up / down. I would go with "...in the spec
(0=1MB, 19=512GB)".

> + *
> + * Set the new size of a BAR as defined in the spec (0=1MB, 19=512GB).
> + * Returns true if resizing was successful, false otherwise.
> + */

> +int pci_rbar_set_size(struct pci_dev *pdev, int bar, int size)
> +{
> +   unsigned pos, nbars;
> +   u32 ctrl;
> +   unsigned i;


> +
> +   pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_REBAR);
> +   if (!pos)
> +   return -ENOTSUPP;
> +
> +   pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, );
> +   nbars = (ctrl & PCI_REBAR_CTRL_NBAR_MASK) >> 
> PCI_REBAR_CTRL_NBAR_SHIFT;
> +
> +   for (i = 0; i < nbars; ++i, pos += 8) {
> +   int bar_idx;
> +
> +   pci_read_config_dword(pdev, pos + PCI_REBAR_CTRL, );
> +   bar_idx = (ctrl & PCI_REBAR_CTRL_BAR_IDX_MASK) >>
> +   PCI_REBAR_CTRL_BAR_IDX_SHIFT;
> +   if (bar_idx != bar)
> +   continue;

Above is duplicating previous.

So,
static int ..._find_rbar(..., u32 *ctrl)
{
}

Returns: (i.e.) 0 - found, 1 - not found, -ERRNO.

ret = _find_rbar();
if (ret < 0)
 return ret;
if (ret)
 return -ENOENT;
...
return 0;

So, please refactor.

> +
> +   ctrl &= ~PCI_REBAR_CTRL_BAR_SIZE_MASK;
> +   ctrl |= size << PCI_REBAR_CTRL_BAR_SIZE_SHIFT;
> +   pci_write_config_dword(pdev, pos + PCI_REBAR_CTRL, ctrl);
> +   return 0;
> +   }
> +
> +   return -ENOENT;
> +}

> -#define  PCI_REBAR_CTRL_NBAR_MASK  (7 << 5)/* mask for # bars */
> -#define  PCI_REBAR_CTRL_NBAR_SHIFT 5   /* shift for # bars */

> +#define  PCI_REBAR_CTRL_NBAR_MASK  (7 << 5)/* mask for # BARs */
> +#define  PCI_REBAR_CTRL_NBAR_SHIFT 5   /* shift for # BARs */

I understand why, but I dunno it worth to do.

-- 
With Best Regards,
Andy Shevchenko
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 100800] With KMS:No link with displayport and A6-3600 APU from 4.4.x to 4.11.0.rc8, unless nomodeset kernel boot parameter

[bugzilla-daemon]

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

abitt...@opensuse.org changed:

   What|Removed |Added

   Hardware|Other   |x86-64 (AMD64)
 OS|All |Linux (All)

-- 
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 100800] With KMS:No link with displayport and A6-3600 APU from 4.4.x to 4.11.0.rc8, unless nomodeset kernel boot parameter

[bugzilla-daemon]

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

Bug ID: 100800
   Summary: With KMS:No link with displayport and A6-3600 APU from
4.4.x to 4.11.0.rc8, unless nomodeset kernel boot
parameter
   Product: DRI
   Version: XOrg git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/Radeon
  Assignee: dri-devel@lists.freedesktop.org
  Reporter: abitt...@opensuse.org

With KMS:No link with displayport and A6-3600 APU from 4.4.x to 4.11.0.rc8,
unless nomodeset kernel boot parameter


Please see my original distribution bugreport at


Tested with OpenSuSE Leap 42.2 (all x64) kernels 4.4.x up to 4.11.0.rc8 kernels

Hardware:
Motherboard: F1A75-V PRO by Asus, with AMD cpu/gpu/apu  all-in-one: AMD A6-3600
APU with Radeon(tm) HD Graphics

hdmi port on motherboard and on the same monitor with hdmi cable works and
produces link, same motherboard with displayport cable and ports and same
monitor at its displayport port gives no link, unless one uses the "nomodeset"
kernel boot parameter.

Thank you.

-- 
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 8/8] drm: Remove redundant NULL check during atomic plane commit

[Ville Syrjälä]

On Wed, Apr 26, 2017 at 04:40:13PM +0300, Imre Deak wrote:
> plane_state can't be NULL anywhere in this function, so the NULL check
> at the end is redundant, remove it.
> 
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Imre Deak 
> ---
>  drivers/gpu/drm/drm_plane_helper.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_plane_helper.c 
> b/drivers/gpu/drm/drm_plane_helper.c
> index 9854a50..2c27f6f 100644
> --- a/drivers/gpu/drm/drm_plane_helper.c
> +++ b/drivers/gpu/drm/drm_plane_helper.c
> @@ -511,12 +511,10 @@ int drm_plane_helper_commit(struct drm_plane *plane,
>   if (plane_funcs->cleanup_fb)
>   plane_funcs->cleanup_fb(plane, plane_state);
>  out:
> - if (plane_state) {
> - if (plane->funcs->atomic_destroy_state)
> - plane->funcs->atomic_destroy_state(plane, plane_state);
> - else
> - drm_atomic_helper_plane_destroy_state(plane, 
> plane_state);
> - }
> + if (plane->funcs->atomic_destroy_state)
> + plane->funcs->atomic_destroy_state(plane, plane_state);
> + else
> + drm_atomic_helper_plane_destroy_state(plane, plane_state);

Hmm. If plane->state was NULL, then we could swap that with plane_state,
and thus plane_state could become NULL. But that would actually oops in
drm_atomic_plane_disabling() already, so yeah, no way this could work.

My only concern is the buggy drm_atomic_helper_plane_reset() which can't
fail gracefully if the kmalloc fails. But failure that would probably
lead to explosions all over anyway.

Reviewed-by: Ville Syrjälä 


>  
>   return ret;
>  }
> -- 
> 2.5.0
> 
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [RFC 0/4] Exynos DRM: add Picture Processor extension

[Tobias Jakobi]

Hello everyone,


Nicolas Dufresne wrote:
> Le mercredi 26 avril 2017 à 01:21 +0300, Sakari Ailus a écrit :
>> Hi Marek,
>>
>> On Thu, Apr 20, 2017 at 01:23:09PM +0200, Marek Szyprowski wrote:
>>> Hi Laurent,
>>>
>>> On 2017-04-20 12:25, Laurent Pinchart wrote:
 Hi Marek,

 (CC'ing Sakari Ailus)

 Thank you for the patches.

 On Thursday 20 Apr 2017 11:13:36 Marek Szyprowski wrote:
> Dear all,
>
> This is an updated proposal for extending EXYNOS DRM API with generic
> support for hardware modules, which can be used for processing image data
> from the one memory buffer to another. Typical memory-to-memory operations
> are: rotation, scaling, colour space conversion or mix of them. This is a
> follow-up of my previous proposal "[RFC 0/2] New feature: Framebuffer
> processors", which has been rejected as "not really needed in the DRM
> core":
> http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg146286.html
>
> In this proposal I moved all the code to Exynos DRM driver, so now this
> will be specific only to Exynos DRM. I've also changed the name from
> framebuffer processor (fbproc) to picture processor (pp) to avoid 
> confusion
> with fbdev API.
>
> Here is a bit more information what picture processors are:
>
> Embedded SoCs are known to have a number of hardware blocks, which perform
> such operations. They can be used in paralel to the main GPU module to
> offload CPU from processing grapics or video data. One of example use of
> such modules is implementing video overlay, which usually requires color
> space conversion from NV12 (or similar) to RGB32 color space and scaling 
> to
> target window size.
>
> The proposed API is heavily inspired by atomic KMS approach - it is also
> based on DRM objects and their properties. A new DRM object is introduced:
> picture processor (called pp for convenience). Such objects have a set of
> standard DRM properties, which describes the operation to be performed by
> respective hardware module. In typical case those properties are a source
> fb id and rectangle (x, y, width, height) and destination fb id and
> rectangle. Optionally a rotation property can be also specified if
> supported by the given hardware. To perform an operation on image data,
> userspace provides a set of properties and their values for given fbproc
> object in a similar way as object and properties are provided for
> performing atomic page flip / mode setting.
>
> The proposed API consists of the 3 new ioctls:
> - DRM_IOCTL_EXYNOS_PP_GET_RESOURCES: to enumerate all available picture
>   processors,
> - DRM_IOCTL_EXYNOS_PP_GET: to query capabilities of given picture
>   processor,
> - DRM_IOCTL_EXYNOS_PP_COMMIT: to perform operation described by given
>   property set.
>
> The proposed API is extensible. Drivers can attach their own, custom
> properties to add support for more advanced picture processing (for 
> example
> blending).
>
> This proposal aims to replace Exynos DRM IPP (Image Post Processing)
> subsystem. IPP API is over-engineered in general, but not really 
> extensible
> on the other side. It is also buggy, with significant design flaws - the
> biggest issue is the fact that the API covers memory-2-memory picture
> operations together with CRTC writeback and duplicating features, which
> belongs to video plane. Comparing with IPP subsystem, the PP framework is
> smaller (1807 vs 778 lines) and allows driver simplification (Exynos
> rotator driver smaller by over 200 lines).
> 
> Just a side note, we have written code in GStreamer using the Exnynos 4
> FIMC IPP driver. I don't know how many, if any, deployment still exist
> (Exynos 4 is relatively old now), but there exist userspace for the
> FIMC driver. We use this for color transformation (from tiled to
> linear) and scaling. The FIMC driver is in fact quite stable in
> upstream kernel today. The GScaler V4L2 M2M driver on Exynos 5 is
> largely based on it and has received some maintenance to properly work
> in GStreamer. unlike this DRM API, you can reuse the same userspace
> code across multiple platforms (which we do already). We have also
> integrated this driver in Chromium in the past (not upstream though).
> 
> I am well aware that the blitter driver has not got much attention
> though. But again, V4L2 offers a generic interface to userspace
> application. Fixing this driver could enable some work like this one:
> 
> https://bugzilla.gnome.org/show_bug.cgi?id=772766
> 
> This work in progress feature is a generic hardware accelerated video
> mixer. It has been tested with IMX.6 v4l2 m2m blitter driver (which I
> believe is in staging right now). Again, unlike the exynos/drm, this
> code could be reused between platforms.
> 
> In general, 

Re: [rfc] drm sync objects (search for spock)

[Christian König]

Am 26.04.2017 um 11:57 schrieb Dave Airlie:

On 26 April 2017 at 18:45, Christian König  wrote:

Am 26.04.2017 um 05:28 schrieb Dave Airlie:

Okay I've gone around the sun with these a few times, and
pretty much implemented what I said last week.

This is pretty much a complete revamp.

1. sync objects are self contained drm objects, they
have a file reference so can be passed between processes.

2. Added a sync object wait interface modelled on the vulkan
fence waiting API.

3. sync_file interaction is explicitly different than
opaque fd passing, you import a sync file state into an
existing syncobj, or create a new sync_file from an
existing syncobj. This means no touching the sync file code
at all. \o/


Doesn't that break the Vulkan requirement that if a sync_obj is exported to
an fd and imported on the other side we get the same sync_obj again?

In other words the fd is exported and imported only once and the expectation
is that we fence containing it will change on each signaling operation.

As far as I can see with the current implementation you get two sync_objects
on in the exporting process and one in the importing one.

Are you talking about using sync_file interop for vulkan, then yes
that would be wrong.

But that isn't how this works, see 1. above the sync obj has a 1:1
mapping with an
opaque fd (non-sync_file) that is only used for interprocess passing
of the syncobj.


Ah, ok that makes some more sense.


You can interoperate with sync_files by importing/exporting the
syncobj fence into
and out of them but that aren't meant for passing the fds around, more
for where you
get a sync_file fd from somewhere else and you want to use it and vice-versa.


I would prefer dealing only with one type of fd in userspace, but that 
approach should work as well.



I'd like to move any drm APIs away from sync_file to avoid the fd wastages,


That sounds like it make sense, but I would still rather vote for 
extending the sync_file interface than deprecating it with another one.



I'd also like to move the driver specific fences to syncobj where I can.


I'm pretty sure that is not a good idea.

Beating the sequence based fence values we currently use for CPU sync in 
performance would be pretty hard. E.g. at least on amdgpu we can even 
check those by doing a simple 64bit read from a memory address, without 
IOCTL or any other overhead involved.


Regards,
Christian.



Dave.



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

Re: [PATCH] dma-buf: avoid scheduling on fence status query v2

[Christian König]

Am 26.04.2017 um 16:46 schrieb Andres Rodriguez:

When a timeout of zero is specified, the caller is only interested in
the fence status.

In the current implementation, dma_fence_default_wait will always call
schedule_timeout() at least once for an unsignaled fence. This adds a
significant overhead to a fence status query.

Avoid this overhead by returning early if a zero timeout is specified.

v2: move early return after enable_signaling

Signed-off-by: Andres Rodriguez 


Reviewed-by: Christian König 


---

  If I'm understanding correctly, I don't think we need to register the
  default wait callback. But if that isn't the case please let me know.

  This patch has the same perf improvements as v1.

  drivers/dma-buf/dma-fence.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 0918d3f..57da14c 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -402,6 +402,11 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, 
signed long timeout)
}
}
  
+	if (!timeout) {

+   ret = 0;
+   goto out;
+   }
+
cb.base.func = dma_fence_default_wait_cb;
cb.task = current;
list_add(, >cb_list);



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

[PATCH] dma-buf: avoid scheduling on fence status query v2

[Andres Rodriguez]

When a timeout of zero is specified, the caller is only interested in
the fence status.

In the current implementation, dma_fence_default_wait will always call
schedule_timeout() at least once for an unsignaled fence. This adds a
significant overhead to a fence status query.

Avoid this overhead by returning early if a zero timeout is specified.

v2: move early return after enable_signaling

Signed-off-by: Andres Rodriguez 
---

 If I'm understanding correctly, I don't think we need to register the
 default wait callback. But if that isn't the case please let me know.

 This patch has the same perf improvements as v1.

 drivers/dma-buf/dma-fence.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 0918d3f..57da14c 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -402,6 +402,11 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, 
signed long timeout)
}
}
 
+   if (!timeout) {
+   ret = 0;
+   goto out;
+   }
+
cb.base.func = dma_fence_default_wait_cb;
cb.task = current;
list_add(, >cb_list);
-- 
2.9.3

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

Re: [PATCH] drm: mali-dp: use div_u64 for expensive 64-bit divisions

[Liviu Dudau]

On Tue, Apr 25, 2017 at 09:56:53PM +0200, Arnd Bergmann wrote:
> On 32-bit machines, we can't divide 64-bit integers:
> 
> drivers/gpu/drm/arm/malidp_crtc.o: In function `malidp_crtc_atomic_check':
> malidp_crtc.c:(.text.malidp_crtc_atomic_check+0x3c0): undefined reference to 
> `__aeabi_uldivmod'
> malidp_crtc.c:(.text.malidp_crtc_atomic_check+0x3dc): undefined reference to 
> `__aeabi_uldivmod'
> 
> This calls the div_u64 function explicitly instead.
> 
> Fixes: 4cea4e9f6690 ("drm: mali-dp: Add plane upscaling support")
> Signed-off-by: Arnd Bergmann 

Acked-by: Liviu Dudau 

I'll push the patch to the mali-dp repository. Dave A., if you have pulled the 
mali-dp
patches then this will need to go in as well, otherwise I will send an updated 
pull
request soon.

Best regards,
Liviu

> ---
>  drivers/gpu/drm/arm/malidp_crtc.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/malidp_crtc.c 
> b/drivers/gpu/drm/arm/malidp_crtc.c
> index 19f1f3b34691..9446a673d469 100644
> --- a/drivers/gpu/drm/arm/malidp_crtc.c
> +++ b/drivers/gpu/drm/arm/malidp_crtc.c
> @@ -266,7 +266,6 @@ static int malidp_crtc_atomic_check_scaling(struct 
> drm_crtc *crtc,
>  
>   drm_atomic_crtc_state_for_each_plane_state(plane, pstate, state) {
>   struct malidp_plane *mp = to_malidp_plane(plane);
> - u64 crtc_w, crtc_h;
>   u32 phase;
>  
>   if (!(mp->layer->id & scaling))
> @@ -276,10 +275,10 @@ static int malidp_crtc_atomic_check_scaling(struct 
> drm_crtc *crtc,
>* Convert crtc_[w|h] to U32.32, then divide by U16.16 src_[w|h]
>* to get the U16.16 result.
>*/
> - crtc_w = (u64)pstate->crtc_w << 32;
> - crtc_h = (u64)pstate->crtc_h << 32;
> - h_upscale_factor = (u32)(crtc_w / pstate->src_w);
> - v_upscale_factor = (u32)(crtc_h / pstate->src_h);
> + h_upscale_factor = div_u64((u64)pstate->crtc_w << 32,
> +pstate->src_w);
> + v_upscale_factor = div_u64((u64)pstate->crtc_h << 32,
> +pstate->src_h);
>  
>   s->enhancer_enable = ((h_upscale_factor >> 16) >= 2 ||
> (v_upscale_factor >> 16) >= 2);
> -- 
> 2.9.0
> 

-- 

| 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: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Ville Syrjälä]

On Wed, Apr 26, 2017 at 11:00:09AM +0900, Michel Dänzer wrote:
> On 25/04/17 06:52 PM, Ville Syrjälä wrote:
> > On Tue, Apr 25, 2017 at 12:18:52PM +0900, Michel Dänzer wrote:
> >> On 24/04/17 03:25 PM, Gerd Hoffmann wrote:
> >>> +#ifdef __BIG_ENDIAN
> >>> + switch (bpp) {
> >>> + case 8:
> >>> + fmt = DRM_FORMAT_C8;
> >>> + break;
> >>> + case 24:
> >>> + fmt = DRM_FORMAT_BGR888;
> >>> + break;
> >>
> >> BTW, endianness as a concept cannot apply to 8 or 24 bpp formats.
> > 
> > To 8bpp no, but it can easily apply to 24bpp.
> 
> Any byte swapping rips apart the bytes of a 24bpp pixel, so those
> formats only make sense as straight array formats.

In my book little endian just means "lsb is stored in the lowest
memory address". The fact that your CPU/GPU can't do 3 byte swaps
is not relevant for that definition IMO.

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

Re: [PATCH] dma-buf: avoid scheduling on fence status query

[Andres Rodriguez]

On 2017-04-26 06:13 AM, Christian König wrote:

Am 26.04.2017 um 11:59 schrieb Dave Airlie:

On 26 April 2017 at 17:20, Christian König 
wrote:

NAK, I'm wondering how often I have to reject that change. We should
probably add a comment here.

Even with a zero timeout we still need to enable signaling, otherwise
some
fence will never signal if userspace just polls on them.

If a caller is only interested in the fence status without enabling the
signaling it should call dma_fence_is_signaled() instead.

Can we not move the return 0 (with spin unlock) down after we enabling
signalling, but before
we enter the schedule_timeout(1)?


Yes, that would be an option.



I was actually arguing with Dave about this on IRC yesterday. Seems like 
I owe him a beer now.


-Andres


Christian.



Dave.




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

Re: [PATCH] drm/pl111: Register the clock divider and use it.

[Linus Walleij]

On Mon, Apr 24, 2017 at 9:45 PM, Eric Anholt  wrote:

> This is required for the panel to work on bcm911360, where CLCDCLK is
> the fixed 200Mhz AXI41 clock.  The rate set is still passed up to the
> CLCDCLK, for platforms that have a settable rate on that one.
>
> Signed-off-by: Eric Anholt 

I like this, it is pretty.

> +static int pl111_clk_div_choose_div(struct clk_hw *hw, unsigned long rate,
> +   unsigned long *prate, bool set_parent)
> +{
> +   int best_div = 1, div;
> +   struct clk_hw *parent = clk_hw_get_parent(hw);
> +   unsigned long best_prate = 0;
> +   unsigned long best_diff = ~0ul;
> +   int max_div = (1 << (TIM2_PCD_LO_BITS + TIM2_PCD_HI_BITS)) - 1;
> +
> +   for (div = 1; div < max_div; div++) {
> +   unsigned long this_prate, div_rate, diff;
> +
> +   if (set_parent)
> +   this_prate = clk_hw_round_rate(parent, rate * div);
> +   else
> +   this_prate = *prate;
> +   div_rate = DIV_ROUND_UP_ULL(this_prate, div);
> +   diff = abs(rate - div_rate);
> +
> +   if (diff < best_diff) {
> +   best_div = div;
> +   best_diff = diff;
> +   best_prate = this_prate;
> +   }
> +   }
> +
> +   *prate = best_prate;
> +   return best_div;
> +}

So since this will choose a divisor using clk_hw_round_rate() on the parent
if we can set the rate of the parent...

> +static long pl111_clk_div_round_rate(struct clk_hw *hw, unsigned long rate,
> +unsigned long *prate)
> +{
> +   int div = pl111_clk_div_choose_div(hw, rate, prate, true);

...which we seem to assume that we can, actually why do you pass
this parameter set_parent at all? It is always true in this code.

You should not need to know whether we can set the rate of the
parent or not: clk_hw_round_rate() will simply return the fixed
rate anyway.

And if the divider can make the set rate even more precise:
all the better!

So I think the parameter can go.

> +static int pl111_clk_div_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long prate)
> +{
> +   struct pl111_drm_dev_private *priv =
> +   container_of(hw, struct pl111_drm_dev_private, clk_div);
> +   int div = pl111_clk_div_choose_div(hw, rate, , false);
> +   u32 tim2;
> +
> +   spin_lock(>tim2_lock);
> +   tim2 = readl(priv->regs + CLCD_TIM2);
> +   tim2 &= ~(TIM2_BCD | TIM2_PCD_LO_MASK | TIM2_PCD_HI_MASK);
> +
> +   if (div == 1) {
> +   tim2 |= TIM2_BCD;
> +   } else {
> +   div -= 2;
> +   tim2 |= div & TIM2_PCD_LO_MASK;
> +   tim2 |= (div >> TIM2_PCD_LO_BITS) << TIM2_PCD_HI_SHIFT;
> +   }
> +
> +   writel(tim2, priv->regs + CLCD_TIM2);
> +   spin_unlock(>tim2_lock);
> +
> +   return 0;
> +}

So this will write the divisor, which is nice. But what if we also need
to change the rate of the parent?

> +static int
> +pl111_init_clock_divider(struct drm_device *drm)
> +{
> +   struct pl111_drm_dev_private *priv = drm->dev_private;
> +   struct clk *parent = devm_clk_get(drm->dev, "clcdclk");
> +   struct clk_hw *div = >clk_div;
> +   const char *parent_name;
> +   struct clk_init_data init = {
> +   .name = "pl111_div",
> +   .ops = _clk_div_ops,
> +   .parent_names = _name,
> +   .num_parents = 1,
> +   };

I think it is necessary to set .flags in the init data to
.flags = CLK_SET_RATE_PARENT,
for this code to work with a parent that can change rate.

> void *regs;
> +   /* The pixel clock (a reference to our clock divider off of CLCDCLK). 
> */
> struct clk *clk;

Maybe we should simply rename it pixclk so it is clear what it's for,
but that can be a separate patch.

> - * - Use the internal clock divisor to reduce power consumption by
> - *   using HCLK (apb_pclk) when appropriate.
> + * - Use the CLKSEL bit to support switching between the two external
> + *   clock parents.

OK so that remains to be done. We discussed this previously
so I got a bit confused. The divisor code seems fine, after this
we only need some more code to choose the best parent for
the divided clock.

It seems that what would pe Perfect(TM) would be to calculate the
best end result using clock A and the best end result using clock B,
both utilizing the divisor, and then choose the best of those two.

I think struct clk_mux is supposed to do that so it would eventually
be:

CLK A -|\_ clk_mux --> clk_divider --> pixel clock
CLK B -|/

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

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Gerd Hoffmann]

>   uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
>   {
>   uint32_t fmt;
>   #ifdef __BIG_ENDIAN
>   enum { LITTLE_ENDIAN = 0 };
>   #else
>   enum { LITTLE_ENDIAN = 1 };
>   #endif
>   /* ... */
> 
> (using an enum for compile-time constness)
> 
> and then
>   fmt = DRM_FORMAT_ARGB;
> becomes
>   fmt = LITTLE_ENDIAN ? DRM_FORMAT_ARGB : DRM_FORMAT_BGRA;
> 
> Might be easier to read than duplicating the whole switch?

Well, there are more differences, like rgb565 and xrgb2101010 not being
supported for bigendian, so it isn't *that* simple.

cheers,
  Gerd

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

[PATCH 8/8] drm: Remove redundant NULL check during atomic plane commit

[Imre Deak]

plane_state can't be NULL anywhere in this function, so the NULL check
at the end is redundant, remove it.

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Imre Deak 
---
 drivers/gpu/drm/drm_plane_helper.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_plane_helper.c 
b/drivers/gpu/drm/drm_plane_helper.c
index 9854a50..2c27f6f 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -511,12 +511,10 @@ int drm_plane_helper_commit(struct drm_plane *plane,
if (plane_funcs->cleanup_fb)
plane_funcs->cleanup_fb(plane, plane_state);
 out:
-   if (plane_state) {
-   if (plane->funcs->atomic_destroy_state)
-   plane->funcs->atomic_destroy_state(plane, plane_state);
-   else
-   drm_atomic_helper_plane_destroy_state(plane, 
plane_state);
-   }
+   if (plane->funcs->atomic_destroy_state)
+   plane->funcs->atomic_destroy_state(plane, plane_state);
+   else
+   drm_atomic_helper_plane_destroy_state(plane, plane_state);
 
return ret;
 }
-- 
2.5.0

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

[Bug 90741] Radeon: System pauses on TAHITI

[bugzilla-daemon]

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

Julien Isorce (julien.iso...@gmail.com) changed:

   What|Removed |Added

 CC||julien.iso...@gmail.com

--- Comment #80 from Julien Isorce (julien.iso...@gmail.com) ---
This bug should be closed:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.4=0586915ec10d0ae60de5cd3381ad25a704760402

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

[Bug 90861] Panic on suspend from KDE with radeon

[bugzilla-daemon]

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

Julien Isorce (julien.iso...@gmail.com) changed:

   What|Removed |Added

 CC||julien.iso...@gmail.com

--- Comment #17 from Julien Isorce (julien.iso...@gmail.com) ---
This bug should be closed:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.4=b6610101718d4ab90d793c482625e98eb1262cad

-- 
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 RFC 3/6] drm: Plane YCbCr to RGB conversion related properties

[Sharma, Shashank]

Regards

Shashank


On 4/26/2017 6:26 PM, Jyri Sarha wrote:

On 04/24/17 19:55, Ville Syrjälä wrote:

In fact we have plane specific YCbCr to RGB CSC (only preoffset
possible), then (per crtc) gamma table, and finally a (per crtc) RGB to
YCbCr CSC with optional post offset (so it can be used either as CSC or
CTM).

So with that plane hw you could perhaps do:
- YCbCr->RGB if you input is not linear, but then you must
   blend using non-linear data
- colorspace conversion if your input is alredy linear

And with your crtc hw you could do:
- degamma + CTM
- gamma + RGB->YCbCr


Just a generic question. Shouldn't - in an ideal HW - the degamma phase
and the CTM be a plane specific property?

I mean, isn't the purpose of normalizing the non linear RGB to linear
(and possibly converting the color space) to have same format for all
plane data before blending and composing them together?

I think Ville's point is something else.
There are two types of color operations possible:
- color space matching so that you can blend the plane data accurately.
- color correction or transformation to enhance display, or change 
output from one format to other format.


Now, when you want to blend, you have to make sure that, you blend among 
the same, that means:

- all the framebuffers should be in one state, either linear or non-linear
- all the framebuffers should be in one color space (REC 709 or 601 or 
2020), else you have to do gamut mapping

- Gamut mapping is possible on Linear data only.

Else, if we try to blend one REC2020 buffer and one REC709 buffer, its 
like adding 2CM + 3MM and making = 5CM


So, we can use the plane level properties in such a way that, you should 
have similar data to the input of the blender (linear Or non-linear)
And then, once blending is done, you can use CRTC level operations for 
color correction and transformation
(Like RGB->YCBCR conversion using CRTC level CSC, for YCBCR420 kind of 
outputs)


Does it helps in explanation ?

- Shashank


Of course it does not matter if all the planes use the same color space,
which then should be converted to something else for the output.

... or have I misunderstood something?

Cheers,
Jyri


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

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Eric Engestrom]

On Wednesday, 2017-04-26 07:53:10 +0200, Gerd Hoffmann wrote:
> On Di, 2017-04-25 at 12:18 +0900, Michel Dänzer wrote:
> > On 24/04/17 03:25 PM, Gerd Hoffmann wrote:
> > > Return correct fourcc codes on bigendian.  Drivers must be adapted to
> > > this change.
> > > 
> > > Signed-off-by: Gerd Hoffmann 
> > 
> > Just to reiterate, this won't work for the radeon driver, which programs
> > the GPU to use (effectively, per the current definition that these are
> > little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
> > DRM_FORMAT_BGRX with >= R600.
> 
> Hmm, ok, how does bigendian fbdev emulation work on pre-R600 then?
> 
> > > +#ifdef __BIG_ENDIAN
> > > + switch (bpp) {
> > > + case 8:
> > > + fmt = DRM_FORMAT_C8;
> > > + break;
> > > + case 24:
> > > + fmt = DRM_FORMAT_BGR888;
> > > + break;
> > 
> > BTW, endianness as a concept cannot apply to 8 or 24 bpp formats.
> 
> I could move the 8 bpp case out of the #ifdef somehow, but code
> readability will suffer then I think ...

How about something like this?

uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth)
{
uint32_t fmt;
#ifdef __BIG_ENDIAN
enum { LITTLE_ENDIAN = 0 };
#else
enum { LITTLE_ENDIAN = 1 };
#endif
/* ... */

(using an enum for compile-time constness)

and then
fmt = DRM_FORMAT_ARGB;
becomes
fmt = LITTLE_ENDIAN ? DRM_FORMAT_ARGB : DRM_FORMAT_BGRA;

Might be easier to read than duplicating the whole switch?

> 
> For 24 we have different byte orderings, but yes, you can't switch from
> one to the other with byteswapping.  Probably one of the reasons why
> this format is pretty much out of fashion these days ...
> 
> cheers,
>   Gerd
> 
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH RFC 3/6] drm: Plane YCbCr to RGB conversion related properties

[Jyri Sarha]

On 04/24/17 19:55, Ville Syrjälä wrote:
>> In fact we have plane specific YCbCr to RGB CSC (only preoffset
>> possible), then (per crtc) gamma table, and finally a (per crtc) RGB to
>> YCbCr CSC with optional post offset (so it can be used either as CSC or
>> CTM).
> So with that plane hw you could perhaps do:
> - YCbCr->RGB if you input is not linear, but then you must
>   blend using non-linear data
> - colorspace conversion if your input is alredy linear
> 
> And with your crtc hw you could do:
> - degamma + CTM
> - gamma + RGB->YCbCr


Just a generic question. Shouldn't - in an ideal HW - the degamma phase
and the CTM be a plane specific property?

I mean, isn't the purpose of normalizing the non linear RGB to linear
(and possibly converting the color space) to have same format for all
plane data before blending and composing them together?

Of course it does not matter if all the planes use the same color space,
which then should be converted to something else for the output.

... or have I misunderstood something?

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

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Gerd Hoffmann]

  Hi,

> >> Just to reiterate, this won't work for the radeon driver, which programs
> >> the GPU to use (effectively, per the current definition that these are
> >> little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
> >> DRM_FORMAT_BGRX with >= R600.
> > 
> > Hmm, ok, how does bigendian fbdev emulation work on pre-R600 then?
> 
> Using a GPU byte swapping mechanism which only affects CPU access to
> video RAM.

That is done using the RADEON_TILING_SWAP_{16,32}BIT flag mentioned in
another thread?

Ok, so the cpu view to fbdev is DRM_FORMAT_BGRX in all cases.

What about dumb bos?  You've mentioned the swap flag isn't used for
those.  Which implies they are in little endian byte order (both gpu and
cpu view).  Does the modesetting driver work correctly on that hardware?

cheers,
  Gerd

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

Re: [PATCH] dma-buf: avoid scheduling on fence status query

[Christian König]

Am 26.04.2017 um 11:59 schrieb Dave Airlie:

On 26 April 2017 at 17:20, Christian König  wrote:

NAK, I'm wondering how often I have to reject that change. We should
probably add a comment here.

Even with a zero timeout we still need to enable signaling, otherwise some
fence will never signal if userspace just polls on them.

If a caller is only interested in the fence status without enabling the
signaling it should call dma_fence_is_signaled() instead.

Can we not move the return 0 (with spin unlock) down after we enabling
signalling, but before
we enter the schedule_timeout(1)?


Yes, that would be an option.

Christian.



Dave.



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

Re: [PATCH] dma-buf: avoid scheduling on fence status query

[Dave Airlie]

On 26 April 2017 at 17:20, Christian König  wrote:
> NAK, I'm wondering how often I have to reject that change. We should
> probably add a comment here.
>
> Even with a zero timeout we still need to enable signaling, otherwise some
> fence will never signal if userspace just polls on them.
>
> If a caller is only interested in the fence status without enabling the
> signaling it should call dma_fence_is_signaled() instead.

Can we not move the return 0 (with spin unlock) down after we enabling
signalling, but before
we enter the schedule_timeout(1)?

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

Re: [rfc] drm sync objects (search for spock)

[Dave Airlie]

On 26 April 2017 at 18:45, Christian König  wrote:
> Am 26.04.2017 um 05:28 schrieb Dave Airlie:
>>
>> Okay I've gone around the sun with these a few times, and
>> pretty much implemented what I said last week.
>>
>> This is pretty much a complete revamp.
>>
>> 1. sync objects are self contained drm objects, they
>> have a file reference so can be passed between processes.
>>
>> 2. Added a sync object wait interface modelled on the vulkan
>> fence waiting API.
>>
>> 3. sync_file interaction is explicitly different than
>> opaque fd passing, you import a sync file state into an
>> existing syncobj, or create a new sync_file from an
>> existing syncobj. This means no touching the sync file code
>> at all. \o/
>
>
> Doesn't that break the Vulkan requirement that if a sync_obj is exported to
> an fd and imported on the other side we get the same sync_obj again?
>
> In other words the fd is exported and imported only once and the expectation
> is that we fence containing it will change on each signaling operation.
>
> As far as I can see with the current implementation you get two sync_objects
> on in the exporting process and one in the importing one.

Are you talking about using sync_file interop for vulkan, then yes
that would be wrong.

But that isn't how this works, see 1. above the sync obj has a 1:1
mapping with an
opaque fd (non-sync_file) that is only used for interprocess passing
of the syncobj.

You can interoperate with sync_files by importing/exporting the
syncobj fence into
and out of them but that aren't meant for passing the fds around, more
for where you
get a sync_file fd from somewhere else and you want to use it and vice-versa.

I'd like to move any drm APIs away from sync_file to avoid the fd wastages, I'd
also like to move the driver specific fences to syncobj where I can.

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

Re: [PATCH 3/6] drm: fourcc byteorder: add bigendian support to drm_mode_legacy_fb_format

[Michel Dänzer]

On 26/04/17 02:53 PM, Gerd Hoffmann wrote:
> On Di, 2017-04-25 at 12:18 +0900, Michel Dänzer wrote:
>> On 24/04/17 03:25 PM, Gerd Hoffmann wrote:
>>> Return correct fourcc codes on bigendian.  Drivers must be adapted to
>>> this change.
>>>
>>> Signed-off-by: Gerd Hoffmann 
>>
>> Just to reiterate, this won't work for the radeon driver, which programs
>> the GPU to use (effectively, per the current definition that these are
>> little endian GPU formats) DRM_FORMAT_XRGB with pre-R600 and
>> DRM_FORMAT_BGRX with >= R600.
> 
> Hmm, ok, how does bigendian fbdev emulation work on pre-R600 then?

Using a GPU byte swapping mechanism which only affects CPU access to
video RAM.


>>> +#ifdef __BIG_ENDIAN
>>> +   switch (bpp) {
>>> +   case 8:
>>> +   fmt = DRM_FORMAT_C8;
>>> +   break;
>>> +   case 24:
>>> +   fmt = DRM_FORMAT_BGR888;
>>> +   break;
>>
>> BTW, endianness as a concept cannot apply to 8 or 24 bpp formats.
> 
> I could move the 8 bpp case out of the #ifdef somehow, but code
> readability will suffer then I think ...

How so?

At least it would make clearer which formats are affected by endianness
and which aren't.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

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

Re: [PATCH 1/5] drm: introduce sync objects

[zhoucm1]

On 2017年04月26日 11:28, Dave Airlie wrote:

From: Dave Airlie 

Sync objects are new toplevel drm object, that contain a
pointer to a fence. This fence can be updated via command
submission ioctls via drivers.

There is also a generic wait obj API modelled on the vulkan
wait API (with code modelled on some amdgpu code).

These objects can be converted to an opaque fd that can be
passes between processes.

TODO: define sync_file interaction.

Signed-off-by: Dave Airlie 
---
  Documentation/gpu/drm-internals.rst |   3 +
  Documentation/gpu/drm-mm.rst|   6 +
  drivers/gpu/drm/Makefile|   2 +-
  drivers/gpu/drm/drm_fops.c  |   8 +
  drivers/gpu/drm/drm_internal.h  |  13 ++
  drivers/gpu/drm/drm_ioctl.c |  12 ++
  drivers/gpu/drm/drm_syncobj.c   | 374 
  include/drm/drmP.h  |   5 +
  include/drm/drm_drv.h   |   1 +
  include/drm/drm_syncobj.h   | 104 ++
  include/uapi/drm/drm.h  |  25 +++
  11 files changed, 552 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/drm_syncobj.c
  create mode 100644 include/drm/drm_syncobj.h

diff --git a/Documentation/gpu/drm-internals.rst 
b/Documentation/gpu/drm-internals.rst
index e35920d..2ea3bce 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -98,6 +98,9 @@ DRIVER_ATOMIC
  implement appropriate obj->atomic_get_property() vfuncs for any
  modeset objects with driver specific properties.
  
+DRIVER_SYNCOBJ

+Driver support drm sync objects.
+
  Major, Minor and Patchlevel
  ~~~
  
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst

index f5760b1..28aebe8 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -483,3 +483,9 @@ DRM Cache Handling
  
  .. kernel-doc:: drivers/gpu/drm/drm_cache.c

 :export:
+
+DRM Sync Objects
+===
+
+.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
+   :export:
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 3ee9579..b5e565c 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,7 +16,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_framebuffer.o drm_connector.o drm_blend.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
-   drm_dumb_buffers.o drm_mode_config.o
+   drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o
  
  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o

  drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index afdf5b1..9a61df2 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct 
drm_minor *minor)
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_open(dev, priv);
  
+	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))

+   drm_syncobj_open(priv);
+
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_init_file_private(>prime);
  
@@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)

  out_prime_destroy:
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_destroy_file_private(>prime);
+   if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   drm_syncobj_release(priv);
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, priv);
put_pid(priv->pid);
@@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)
drm_property_destroy_user_blobs(dev, file_priv);
}
  
+	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))

+   drm_syncobj_release(file_priv);
+
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, file_priv);
  
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h

index f37388c..44ef903 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct drm_crtc 
*crtc)
  {
return 0;
  }
+
  #endif
+
+/* drm_syncobj.c */
+void drm_syncobj_open(struct drm_file *file_private);
+void drm_syncobj_release(struct drm_file *file_private);
+int drm_syncobj_create_ioctl(struct drm_device *dev, void *data,
+struct drm_file *file_private);
+int drm_syncobj_destroy_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
+  struct drm_file *file_private);
+int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void 

Re: [PATCH v2 01/21] scatterlist: Introduce sg_map helper functions

[Christian König]

Am 25.04.2017 um 20:20 schrieb Logan Gunthorpe:

This patch introduces functions which kmap the pages inside an sgl.
These functions replace a common pattern of kmap(sg_page(sg)) that is
used in more than 50 places within the kernel.

The motivation for this work is to eventually safely support sgls that
contain io memory. In order for that to work, any access to the contents
of an iomem SGL will need to be done with iomemcpy or hit some warning.
(The exact details of how this will work have yet to be worked out.)
Having all the kmaps in one place is just a first step in that
direction. Additionally, seeing this helps cut down the users of sg_page,
it should make any effort to go to struct-page-less DMAs a little
easier (should that idea ever swing back into favour again).

A flags option is added to select between a regular or atomic mapping so
these functions can replace kmap(sg_page or kmap_atomic(sg_page.
Future work may expand this to have flags for using page_address or
vmap. We include a flag to require the function not to fail to
support legacy code that has no easy error path. Much further in the
future, there may be a flag to allocate memory and copy the data
from/to iomem.

We also add the semantic that sg_map can fail to create a mapping,
despite the fact that the current code this is replacing is assumed to
never fail and the current version of these functions cannot fail. This
is to support iomem which may either have to fail to create the mapping or
allocate memory as a bounce buffer which itself can fail.

Also, in terms of cleanup, a few of the existing kmap(sg_page) users
play things a bit loose in terms of whether they apply sg->offset
so using these helper functions should help avoid such issues.

Signed-off-by: Logan Gunthorpe 
---


Good to know that somebody is working on this. Those problems troubled 
us as well.


Patch is Acked-by: Christian König .

Regards,
Christian.


  include/linux/scatterlist.h | 85 +
  1 file changed, 85 insertions(+)

diff --git a/include/linux/scatterlist.h b/include/linux/scatterlist.h
index cb3c8fe..fad170b 100644
--- a/include/linux/scatterlist.h
+++ b/include/linux/scatterlist.h
@@ -5,6 +5,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  
  struct scatterlist {

@@ -126,6 +127,90 @@ static inline struct page *sg_page(struct scatterlist *sg)
return (struct page *)((sg)->page_link & ~0x3);
  }
  
+#define SG_KMAP		 (1 << 0)	/* create a mapping with kmap */

+#define SG_KMAP_ATOMIC  (1 << 1) /* create a mapping with kmap_atomic 
*/
+#define SG_MAP_MUST_NOT_FAIL (1 << 2)/* indicate sg_map should not fail */
+
+/**
+ * sg_map - kmap a page inside an sgl
+ * @sg:SG entry
+ * @offset:Offset into entry
+ * @flags: Flags for creating the mapping
+ *
+ * Description:
+ *   Use this function to map a page in the scatterlist at the specified
+ *   offset. sg->offset is already added for you. Note: the semantics of
+ *   this function are that it may fail. Thus, its output should be checked
+ *   with IS_ERR and PTR_ERR. Otherwise, a pointer to the specified offset
+ *   in the mapped page is returned.
+ *
+ *   Flags can be any of:
+ * * SG_KMAP   - Use kmap to create the mapping
+ * * SG_KMAP_ATOMIC- Use kmap_atomic to map the page atommically.
+ *   Thus, the rules of that function apply: the
+ *   cpu may not sleep until it is unmaped.
+ * * SG_MAP_MUST_NOT_FAIL  - Indicate that sg_map must not fail.
+ *   If it does, it will issue a BUG_ON instead.
+ *   This is intended for legacy code only, it
+ *   is not to be used in new code.
+ *
+ *   Also, consider carefully whether this function is appropriate. It is
+ *   largely not recommended for new code and if the sgl came from another
+ *   subsystem and you don't know what kind of memory might be in the list
+ *   then you definitely should not call it. Non-mappable memory may be in
+ *   the sgl and thus this function may fail unexpectedly. Consider using
+ *   sg_copy_to_buffer instead.
+ **/
+static inline void *sg_map(struct scatterlist *sg, size_t offset, int flags)
+{
+   struct page *pg;
+   unsigned int pg_off;
+   void *ret;
+
+   offset += sg->offset;
+   pg = nth_page(sg_page(sg), offset >> PAGE_SHIFT);
+   pg_off = offset_in_page(offset);
+
+   if (flags & SG_KMAP_ATOMIC)
+   ret = kmap_atomic(pg) + pg_off;
+   else if (flags & SG_KMAP)
+   ret = kmap(pg) + pg_off;
+   else
+   ret = ERR_PTR(-EINVAL);
+
+   /*
+* In theory, this can't happen yet. Once we start adding
+* unmapable memory, it also shouldn't happen unless developers
+* start putting unmappable struct pages in 

Re: [rfc] drm sync objects (search for spock)

[Christian König]

Am 26.04.2017 um 05:28 schrieb Dave Airlie:

Okay I've gone around the sun with these a few times, and
pretty much implemented what I said last week.

This is pretty much a complete revamp.

1. sync objects are self contained drm objects, they
have a file reference so can be passed between processes.

2. Added a sync object wait interface modelled on the vulkan
fence waiting API.

3. sync_file interaction is explicitly different than
opaque fd passing, you import a sync file state into an
existing syncobj, or create a new sync_file from an
existing syncobj. This means no touching the sync file code
at all. \o/


Doesn't that break the Vulkan requirement that if a sync_obj is exported 
to an fd and imported on the other side we get the same sync_obj again?


In other words the fd is exported and imported only once and the 
expectation is that we fence containing it will change on each signaling 
operation.


As far as I can see with the current implementation you get two 
sync_objects on in the exporting process and one in the importing one.


Regards,
Christian.



I haven't used rcu anywhere here, I've used xchg to swap
fence pointers in the hope that's safe. If this does need
rcu'ing I suggest we do it in a follow on patch to minimise
the review pain.

Dave.

___
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 195581] NULL pointer dereference when amdgpu driver calls drm_load_edid_firmware

[bugzilla-daemon]

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

--- Comment #3 from Lutz Vieweg (l...@5t9.de) ---
BTW: Is there a reason why the EDID is read anew every time one uses "xrandr
--ouput ... --mode ... --rate ..." to just switch the refresh rate, and also
when switching between X11 and the text console? 

(I would have thought that reading the EDID is required only when a new monitor
connection is made.)

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

[Bug 195581] NULL pointer dereference when amdgpu driver calls drm_load_edid_firmware

[bugzilla-daemon]

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

--- Comment #2 from Lutz Vieweg (l...@5t9.de) ---
Indeed, https://bugs.freedesktop.org/show_bug.cgi?id=100375 looks like it is
the same issue.

I cannot say whether this is a regression from an older kernel, because I only
recently put this AMD RX-460 GPU into the system, and significantly older
kernels don't work that well with Ryzen CPUs...

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

[Bug 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #5 from Hi-Angel  ---
Created attachment 131044
  --> https://bugs.freedesktop.org/attachment.cgi?id=131044=edit
ps,vs dump HEAD with reverted eb8aa93c03

-- 
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 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #4 from Hi-Angel  ---
Created attachment 131043
  --> https://bugs.freedesktop.org/attachment.cgi?id=131043=edit
ps,vs dump HEAD

-- 
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 100785] [regression, bisected] arb_gpu_shader5 piglit fail

[bugzilla-daemon]

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

--- Comment #3 from Hi-Angel  ---
(In reply to Timothy Arceri from comment #2)
> Does it work if you set:
> 
> R600_DEBUG=nosb

Yeah, this way it does pass.

> Also it would be helpful if you could attach a before and after shader dump
> using.
> 
> R600_DEBUG=vs,ps

Attaching.

-- 
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 4/5] amdgpu/cs: split out fence dependency checking

[zhoucm1]

On 2017年04月26日 11:28, Dave Airlie wrote:

From: Dave Airlie 

This just splits out the fence depenency checking into it's
own function to make it easier to add semaphore dependencies.

Reviewed-by: Christian König 
Signed-off-by: Dave Airlie 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 85 +++---
  1 file changed, 47 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 99424cb..df25b32 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -963,56 +963,65 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
return 0;
  }
  
-static int amdgpu_cs_dependencies(struct amdgpu_device *adev,

- struct amdgpu_cs_parser *p)
+static int amdgpu_process_fence_dep(struct amdgpu_cs_parser *p,
To consistent, we'd like amdgpu_cs prefix in amdgpu_cs.c, same as other 
patches.


Regards,
David Zhou

+   struct amdgpu_cs_chunk *chunk)
  {
struct amdgpu_fpriv *fpriv = p->filp->driver_priv;
-   int i, j, r;
-
-   for (i = 0; i < p->nchunks; ++i) {
-   struct drm_amdgpu_cs_chunk_dep *deps;
-   struct amdgpu_cs_chunk *chunk;
-   unsigned num_deps;
+   unsigned num_deps;
+   int i, r;
+   struct drm_amdgpu_cs_chunk_dep *deps;
  
-		chunk = >chunks[i];

+   deps = (struct drm_amdgpu_cs_chunk_dep *)chunk->kdata;
+   num_deps = chunk->length_dw * 4 /
+   sizeof(struct drm_amdgpu_cs_chunk_dep);
  
-		if (chunk->chunk_id != AMDGPU_CHUNK_ID_DEPENDENCIES)

-   continue;
+   for (i = 0; i < num_deps; ++i) {
+   struct amdgpu_ring *ring;
+   struct amdgpu_ctx *ctx;
+   struct dma_fence *fence;
  
-		deps = (struct drm_amdgpu_cs_chunk_dep *)chunk->kdata;

-   num_deps = chunk->length_dw * 4 /
-   sizeof(struct drm_amdgpu_cs_chunk_dep);
+   r = amdgpu_cs_get_ring(p->adev, deps[i].ip_type,
+  deps[i].ip_instance,
+  deps[i].ring, );
+   if (r)
+   return r;
  
-		for (j = 0; j < num_deps; ++j) {

-   struct amdgpu_ring *ring;
-   struct amdgpu_ctx *ctx;
-   struct dma_fence *fence;
+   ctx = amdgpu_ctx_get(fpriv, deps[i].ctx_id);
+   if (ctx == NULL)
+   return -EINVAL;
  
-			r = amdgpu_cs_get_ring(adev, deps[j].ip_type,

-  deps[j].ip_instance,
-  deps[j].ring, );
+   fence = amdgpu_ctx_get_fence(ctx, ring,
+deps[i].handle);
+   if (IS_ERR(fence)) {
+   r = PTR_ERR(fence);
+   amdgpu_ctx_put(ctx);
+   return r;
+   } else if (fence) {
+   r = amdgpu_sync_fence(p->adev, >job->sync,
+ fence);
+   dma_fence_put(fence);
+   amdgpu_ctx_put(ctx);
if (r)
return r;
+   }
+   }
+   return 0;
+}
  
-			ctx = amdgpu_ctx_get(fpriv, deps[j].ctx_id);

-   if (ctx == NULL)
-   return -EINVAL;
+static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
+ struct amdgpu_cs_parser *p)
+{
+   int i, r;
  
-			fence = amdgpu_ctx_get_fence(ctx, ring,

-deps[j].handle);
-   if (IS_ERR(fence)) {
-   r = PTR_ERR(fence);
-   amdgpu_ctx_put(ctx);
-   return r;
+   for (i = 0; i < p->nchunks; ++i) {
+   struct amdgpu_cs_chunk *chunk;
  
-			} else if (fence) {

-   r = amdgpu_sync_fence(adev, >job->sync,
- fence);
-   dma_fence_put(fence);
-   amdgpu_ctx_put(ctx);
-   if (r)
-   return r;
-   }
+   chunk = >chunks[i];
+
+   if (chunk->chunk_id == AMDGPU_CHUNK_ID_DEPENDENCIES) {
+   r = amdgpu_process_fence_dep(p, chunk);
+   if (r)
+   return r;
}
}
  


___
dri-devel mailing list
dri-devel@lists.freedesktop.org

Re: [PATCH] dma-buf: avoid scheduling on fence status query

[Christian König]

NAK, I'm wondering how often I have to reject that change. We should 
probably add a comment here.


Even with a zero timeout we still need to enable signaling, otherwise 
some fence will never signal if userspace just polls on them.


If a caller is only interested in the fence status without enabling the 
signaling it should call dma_fence_is_signaled() instead.


Regards,
Christian.

Am 26.04.2017 um 04:50 schrieb Andres Rodriguez:

CC a few extra lists I missed.

Regards,
Andres

On 2017-04-25 09:36 PM, Andres Rodriguez wrote:

When a timeout of zero is specified, the caller is only interested in
the fence status.

In the current implementation, dma_fence_default_wait will always call
schedule_timeout() at least once for an unsignaled fence. This adds a
significant overhead to a fence status query.

Avoid this overhead by returning early if a zero timeout is specified.

Signed-off-by: Andres Rodriguez 
---

This heavily affects the performance of the Source2 engine running on
radv.

This patch improves dota2(radv) perf on a i7-6700k+RX480 system from
72fps->81fps.

 drivers/dma-buf/dma-fence.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 0918d3f..348e9e2 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -380,6 +380,9 @@ dma_fence_default_wait(struct dma_fence *fence, 
bool intr, signed long timeout)

 if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, >flags))
 return ret;

+if (!timeout)
+return 0;
+
 spin_lock_irqsave(fence->lock, flags);

 if (intr && signal_pending(current)) {



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

Re: [PATCH 1/5] drm: introduce sync objects

[zhoucm1]

On 2017年04月26日 11:28, Dave Airlie wrote:

From: Dave Airlie 

Sync objects are new toplevel drm object, that contain a
pointer to a fence. This fence can be updated via command
submission ioctls via drivers.

There is also a generic wait obj API modelled on the vulkan
wait API (with code modelled on some amdgpu code).

These objects can be converted to an opaque fd that can be
passes between processes.

TODO: define sync_file interaction.

Signed-off-by: Dave Airlie 
---
  Documentation/gpu/drm-internals.rst |   3 +
  Documentation/gpu/drm-mm.rst|   6 +
  drivers/gpu/drm/Makefile|   2 +-
  drivers/gpu/drm/drm_fops.c  |   8 +
  drivers/gpu/drm/drm_internal.h  |  13 ++
  drivers/gpu/drm/drm_ioctl.c |  12 ++
  drivers/gpu/drm/drm_syncobj.c   | 374 
  include/drm/drmP.h  |   5 +
  include/drm/drm_drv.h   |   1 +
  include/drm/drm_syncobj.h   | 104 ++
  include/uapi/drm/drm.h  |  25 +++
  11 files changed, 552 insertions(+), 1 deletion(-)
  create mode 100644 drivers/gpu/drm/drm_syncobj.c
  create mode 100644 include/drm/drm_syncobj.h

diff --git a/Documentation/gpu/drm-internals.rst 
b/Documentation/gpu/drm-internals.rst
index e35920d..2ea3bce 100644
--- a/Documentation/gpu/drm-internals.rst
+++ b/Documentation/gpu/drm-internals.rst
@@ -98,6 +98,9 @@ DRIVER_ATOMIC
  implement appropriate obj->atomic_get_property() vfuncs for any
  modeset objects with driver specific properties.
  
+DRIVER_SYNCOBJ

+Driver support drm sync objects.
+
  Major, Minor and Patchlevel
  ~~~
  
diff --git a/Documentation/gpu/drm-mm.rst b/Documentation/gpu/drm-mm.rst

index f5760b1..28aebe8 100644
--- a/Documentation/gpu/drm-mm.rst
+++ b/Documentation/gpu/drm-mm.rst
@@ -483,3 +483,9 @@ DRM Cache Handling
  
  .. kernel-doc:: drivers/gpu/drm/drm_cache.c

 :export:
+
+DRM Sync Objects
+===
+
+.. kernel-doc:: drivers/gpu/drm/drm_syncobj.c
+   :export:
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 3ee9579..b5e565c 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -16,7 +16,7 @@ drm-y   :=drm_auth.o drm_bufs.o drm_cache.o \
drm_framebuffer.o drm_connector.o drm_blend.o \
drm_encoder.o drm_mode_object.o drm_property.o \
drm_plane.o drm_color_mgmt.o drm_print.o \
-   drm_dumb_buffers.o drm_mode_config.o
+   drm_dumb_buffers.o drm_mode_config.o drm_syncobj.o
  
  drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o

  drm-$(CONFIG_DRM_VM) += drm_vm.o
diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
index afdf5b1..9a61df2 100644
--- a/drivers/gpu/drm/drm_fops.c
+++ b/drivers/gpu/drm/drm_fops.c
@@ -219,6 +219,9 @@ static int drm_open_helper(struct file *filp, struct 
drm_minor *minor)
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_open(dev, priv);
  
+	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))

+   drm_syncobj_open(priv);
+
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_init_file_private(>prime);
  
@@ -266,6 +269,8 @@ static int drm_open_helper(struct file *filp, struct drm_minor *minor)

  out_prime_destroy:
if (drm_core_check_feature(dev, DRIVER_PRIME))
drm_prime_destroy_file_private(>prime);
+   if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))
+   drm_syncobj_release(priv);
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, priv);
put_pid(priv->pid);
@@ -400,6 +405,9 @@ int drm_release(struct inode *inode, struct file *filp)
drm_property_destroy_user_blobs(dev, file_priv);
}
  
+	if (drm_core_check_feature(dev, DRIVER_SYNCOBJ))

+   drm_syncobj_release(file_priv);
+
if (drm_core_check_feature(dev, DRIVER_GEM))
drm_gem_release(dev, file_priv);
  
diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h

index f37388c..44ef903 100644
--- a/drivers/gpu/drm/drm_internal.h
+++ b/drivers/gpu/drm/drm_internal.h
@@ -142,4 +142,17 @@ static inline int drm_debugfs_crtc_crc_add(struct drm_crtc 
*crtc)
  {
return 0;
  }
+
  #endif
+
+/* drm_syncobj.c */
+void drm_syncobj_open(struct drm_file *file_private);
+void drm_syncobj_release(struct drm_file *file_private);
+int drm_syncobj_create_ioctl(struct drm_device *dev, void *data,
+struct drm_file *file_private);
+int drm_syncobj_destroy_ioctl(struct drm_device *dev, void *data,
+ struct drm_file *file_private);
+int drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data,
+  struct drm_file *file_private);
+int drm_syncobj_fd_to_handle_ioctl(struct drm_device *dev, void 

Re: [linux-sunxi] [PATCH 13/15] drm/sun4i: Add HDMI support

[Maxime Ripard]

Hi Chen-Yu,

On Fri, Apr 21, 2017 at 11:17:17PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Tue, Mar 7, 2017 at 4:56 PM, Maxime Ripard
>  wrote:
> > The earlier Allwinner SoCs (A10, A10s, A20, A31) have an embedded HDMI
> > controller.
> >
> > That HDMI controller is able to do audio and CEC, but those have been left
> > out for now.
> >
> > Signed-off-by: Maxime Ripard 
> > ---
> >  drivers/gpu/drm/sun4i/Makefile  |   5 +-
> >  drivers/gpu/drm/sun4i/sun4i_hdmi.h  | 124 ++-
> >  drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c  | 128 ++-
> >  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c  | 449 +-
> >  drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 236 +++-
> >  5 files changed, 942 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi.h
> >  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_ddc_clk.c
> >  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> >  create mode 100644 drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c
> 
> Applying patch #9608371 using 'git am'
> Description: [13/15] drm/sun4i: Add HDMI support
> Applying: drm/sun4i: Add HDMI support
> .git/rebase-apply/patch:116: trailing whitespace.
> 
> .git/rebase-apply/patch:531: trailing whitespace.
> 
> .git/rebase-apply/patch:701: trailing whitespace.
> 
> warning: 3 lines add whitespace errors.

Fixed.

> > +int sun4i_ddc_create(struct sun4i_hdmi *hdmi, struct clk *parent)
> > +{
> > +   struct clk_init_data init;
> > +   struct sun4i_ddc *ddc;
> > +   const char *parent_name;
> > +
> > +   parent_name = __clk_get_name(parent);
> > +   if (!parent_name)
> > +   return -ENODEV;
> > +
> > +   ddc = devm_kzalloc(hdmi->dev, sizeof(*ddc), GFP_KERNEL);
> > +   if (!ddc)
> > +   return -ENOMEM;
> > +
> > +   init.name = "hdmi-ddc";
> > +   init.ops = _ddc_ops;
> > +   init.parent_names = _name;
> > +   init.num_parents = 1;
> > +   init.flags = CLK_SET_RATE_PARENT;
> 
> I don't think this is really needed. It probably doesn't hurt though,
> since DDC is used when HDMI is not used for displaying, but it might
> affect any upstream PLLs, which theoretically may affect other users
> of said PLLs. The DDC clock is slow enough that we should be able to
> generate a usable clock rate anyway.

Good point, I removed it.

> > +   writel(SUN4I_HDMI_VID_TIMING_X(mode->hdisplay) |
> > +  SUN4I_HDMI_VID_TIMING_Y(mode->vdisplay),
> > +  hdmi->base + SUN4I_HDMI_VID_TIMING_ACT_REG);
> > +
> > +   x = mode->htotal - mode->hsync_start;
> > +   y = mode->vtotal - mode->vsync_start;
> 
> I'm a bit skeptical about this one. All the other parameters are not
> inclusive of other, why would this one be different? Shouldn't it
> be "Xtotal - Xsync_end" instead?

By the usual meaning of backporch, you're right. However, Allwinner's
seems to have it's own, which is actually the backporch + sync length.

We also have that on all the other connectors (and TCON), and this was
confirmed at the time using a scope on an RGB signal.

> 
> > +   writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
> > +  hdmi->base + SUN4I_HDMI_VID_TIMING_BP_REG);
> > +
> > +   x = mode->hsync_start - mode->hdisplay;
> > +   y = mode->vsync_start - mode->vdisplay;
> > +   writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
> > +  hdmi->base + SUN4I_HDMI_VID_TIMING_FP_REG);
> > +
> > +   x = mode->hsync_end - mode->hsync_start;
> > +   y = mode->vsync_end - mode->vsync_start;
> > +   writel(SUN4I_HDMI_VID_TIMING_X(x) | SUN4I_HDMI_VID_TIMING_Y(y),
> > +  hdmi->base + SUN4I_HDMI_VID_TIMING_SPW_REG);
> > +
> > +   val = SUN4I_HDMI_VID_TIMING_POL_TX_CLK;
> > +   if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > +   val |= SUN4I_HDMI_VID_TIMING_POL_HSYNC;
> > +
> > +   if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > +   val |= SUN4I_HDMI_VID_TIMING_POL_VSYNC;
> > +
> > +   writel(val, hdmi->base + SUN4I_HDMI_VID_TIMING_POL_REG);
> 
> You don't handle the interlaced video here, even though you set
> 
> hdmi->connector.interlace_allowed = true
> 
> later.

I'll fix that.

> The double clock and double scan flags aren't handled either, though
> I don't understand which one is supposed to represent the need for the
> HDMI pixel repeater. AFAIK this is required for resolutions with pixel
> clocks lower than 25 MHz, the lower limit of HDMI's TMDS link.

I'm not sure about this one though. I'd like to keep things quite
simple for now and build up on that once the basis is working. Is it
common in the wild?

> > +  hdmi->base + SUN4I_HDMI_DDC_FIFO_CTRL_REG);
> > +   writel(SUN4I_HDMI_DDC_ADDR_SEGMENT(offset >> 8) |
> > +  SUN4I_HDMI_DDC_ADDR_EDDC(0x60) |
> > +  SUN4I_HDMI_DDC_ADDR_OFFSET(offset) |
> 

Re: [PATCH 0/3] drm/sun4i: More cleanups and fixes

[Maxime Ripard]

On Tue, Apr 25, 2017 at 11:25:02PM +0800, Chen-Yu Tsai wrote:
> Hi Maxime,
> 
> The subject is probably getting old. Here are a few more cleanups.
> 
> Patch 1 should have been part of the patch
> 
> drm/sun4i: Use lists to track registered display backends and TCONs
> 
> from my multiple pipeline support series. Please squash it in if you can.
> 
> Patch 2 just moves setting the TCON clocks back inside the TCON driver.
> 
> Patch 3 cleans up a DRM driver debug message.

Applied all, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


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