date:20160908

On Thu, Sep 08, 2016 at 02:14:43AM +0200, Mario Kleiner wrote:
> amdgpu-kms uses shared fences for its prime exported dmabufs,
> instead of an exclusive fence. Therefore we need to wait for
> all fences of the dmabuf reservation object to prevent
> unsynchronized rendering and flipping.

No. Fix the root cause as this affects not just flips but copies -
this implies that everybody using the resv object must wait for all
fences. The resv object is not just used for prime, but all fencing, so
this breaks the ability to schedule parallel operations across engine.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[Bug 97610] [CIK] Fullscreen video with Firefox only shows "tiled pixel garbage"

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

Michel DÃ¤nzer  changed:

   What|Removed |Added

 QA Contact|xorg-team at lists.x.org   |dri-devel at 
lists.freedesktop
   ||.org
   Assignee|xorg-driver-ati at lists.x.org |dri-devel at 
lists.freedesktop
   ||.org
Product|xorg|Mesa
  Component|Driver/AMDgpu   |Drivers/Gallium/radeonsi

--- Comment #3 from Michel DÃ¤nzer  ---
I can reproduce the problem on Kaveri, but not on Tonga.

It does look like some kind of tiling parameter mismatch, but since
xf86-video-amdgpu doesn't deal with tiling parameters directly, it's more
likely that the problem is in Mesa or even lower in the stack. Reassigning to
Mesa for now.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/2a5eb144/attachment-0001.html>

[PATCH v3] drm: modify drm_global_item_ref to avoid two times of writing ref->object

On Wed, Sep 07, 2016 at 10:07:57PM -0400, Huang Rui wrote:
> In previous drm_global_item_ref, there are two times of writing
> ref->object if item->refcount is 0. So this patch does a minor update
> to put alloc and init ref firstly, and then to modify the item of glob
> array. Use "else" to avoid two times of writing ref->object. It can
> make the code logic more clearly.
> 
> Signed-off-by: Huang Rui 
> ---
> 
> Changes from V2 -> V3:
> - Use duplicate mutex release to avoid "goto" in non-error patch.
> - Rename error label
> 
> ---
>  drivers/gpu/drm/drm_global.c | 24 ++--
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_global.c b/drivers/gpu/drm/drm_global.c
> index 3d2e91c..b404287 100644
> --- a/drivers/gpu/drm/drm_global.c
> +++ b/drivers/gpu/drm/drm_global.c
> @@ -65,30 +65,34 @@ void drm_global_release(void)
>  
>  int drm_global_item_ref(struct drm_global_reference *ref)
>  {
> - int ret;
> + int ret = 0;
>   struct drm_global_item *item = &glob[ref->global_type];
>  
>   mutex_lock(&item->mutex);
>   if (item->refcount == 0) {
> - item->object = kzalloc(ref->size, GFP_KERNEL);
> - if (unlikely(item->object == NULL)) {
> + ref->object = kzalloc(ref->size, GFP_KERNEL);

So the item refcount is 0, we operate on ref, whereas previous we
inspected item and operated on item. Not an improvement.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 001/001] drivers/gpu/radeon: NULL pointer deference workaround

Am 07.09.2016 um 19:38 schrieb Mark Fortescue:
>
> On an LV-683 (AMD Dual-core G-T56N) Mini-ITX board, I get a Kernel 
> Oops because Connector 0 (LCD Panel interface) does not have DDC.

I'm not an expert on this, but that is really odd cause even LCD Panels 
should have a DDC interface.

>
> Ubuntu 16.04 LTS Kernel (4.4 series):
>
> ...
> [ 8.262990] [drm] ib test on ring 5 succeeded
> [ 8.288897] [drm] Radeon Display Connectors
> [ 8.293175] [drm] Connector 0:
> [ 8.296252] [drm] DP-1

Especially since the BIOS claims that this is a displayport connector 
and there is no physical way to have a DP without an DDC as far as I know.

Please open a bug report on FDO and attach you BIOS image.

Alex can probably take a look when he's back from vacation.

Regards,
Christian.

> [ 8.298791] [drm] Encoders:
> [ 8.301770] [drm] DFP1: INTERNAL_UNIPHY
> [ 8.305973] [drm] Connector 1:
> [ 8.309043] [drm] DP-2
> [ 8.311598] [drm] HPD2
> [ 8.314169] [drm] DDC: 0x6440 0x6440 0x6444 0x6444 0x6448 0x6448 
> 0x644c 0x644c
> [ 8.321609] [drm] Encoders:
> [ 8.324589] [drm] DFP2: INTERNAL_UNIPHY
> [ 8.328793] [drm] Connector 2:
> [ 8.331856] [drm] VGA-1
> [ 8.342947] [drm] DDC: 0x64d8 0x64d8 0x64dc 0x64dc 0x64e0 0x64e0 
> 0x64e4 0x64e4
> [ 8.350341] [drm] Encoders:
> [ 8.353310] [drm] CRT1: INTERNAL_KLDSCP_DAC1
> [ 8.358195] BUG: unable to handle kernel NULL pointer dereference at 
> 0409
> [ 8.409733] [] radeon_dp_getsinktype+0x1a/0x30 [radeon]
> [ 8.416805] PGD 0
> [ 8.418841] Oops:  [#1] SMP
> ...
>
> This patch prevents Kernel failures due to a connector not having a 
> DDC interface by changing the code so that ddc_bus is always checked 
> before use.
> The problem was first identified using the uBuntu MATE 14.04 LTS (3.16 
> series kernels) but not dealt with at that time. On attempting to 
> install uBuntu MATE 16.04 LTS (4.4 series kernels), it became clear 
> that using various workarounds to allow the issue to be ignored were 
> not viable so more effort was put in to sorting the issue resulting in 
> this patch. See https://bugs.launchpad.net/bugs/1587885 for more details.
>
> Signed-off-by: Mark Fortescue 
> Tested-by: Mark Fortescue 
>
> ---
>
> Looks like Thunderbird may have made a mess of the patch when pasting 
> the contents into the mail message - my alternate mail client (pine) 
> also has strict line length handling and trashes non-MIME encoded 
> patches.
>
> This may not be the correct approach to solving the issue but it is 
> clean in that it ensures that ddc_bus is never used when NULL 
> regardless of how the code ended up at the point of use.
>
> If it helps with back porting, I have patches for the uBuntu 14.04 LTS 
> [3.13 series], uBuntu MATE 14.04 LTS [3.16 series] and uBuntu 16.04 
> LTS [4.4 series] kernels.
>
> Test Hardware:
> Commell LV-683 Mini-ITX with onboard AMD Dual-core G-T56N
> 4G Ram, 2x1TB Disk, HANNS-G HC194D 1280x1024 LCD (VGA).
> 4.8.0-rc5 with patch boots without error.
>
>   drivers/gpu/drm/radeon/atombios_dp.c   |   60 ---
>   drivers/gpu/drm/radeon/radeon_connectors.c |   46 +++---
>   drivers/gpu/drm/radeon/radeon_dp_mst.c |9 ++
>   drivers/gpu/drm/radeon/radeon_i2c.c|3
>   4 files changed, 73 insertions(+), 45 deletions(-)
>
> Patch:
> diff --git a/drivers/gpu/drm/radeon/atombios_dp.c 
> b/drivers/gpu/drm/radeon/atombios_dp.c
> index cead089a..98b3c0e 100644
> --- a/drivers/gpu/drm/radeon/atombios_dp.c
> +++ b/drivers/gpu/drm/radeon/atombios_dp.c
> @@ -232,6 +232,9 @@ void radeon_dp_aux_init(struct radeon_connector 
> *radeon_connector)
>   struct radeon_device *rdev = dev->dev_private;
>   int ret;
>
> +if (!radeon_connector->ddc_bus)
> +return;
> +
>   radeon_connector->ddc_bus->rec.hpd = radeon_connector->hpd.hpd;
>   radeon_connector->ddc_bus->aux.dev = radeon_connector->base.kdev;
>   if (ASIC_IS_DCE5(rdev)) {
> @@ -364,6 +367,9 @@ u8 radeon_dp_getsinktype(struct radeon_connector 
> *radeon_connector)
>   struct drm_device *dev = radeon_connector->base.dev;
>   struct radeon_device *rdev = dev->dev_private;
>
> +if (!radeon_connector->ddc_bus)
> +return 0;
> +
>   return radeon_dp_encoder_service(rdev, 
> ATOM_DP_ACTION_GET_SINK_TYPE, 0,
> radeon_connector->ddc_bus->rec.i2c_id, 0);
>   }
> @@ -376,6 +382,9 @@ static void radeon_dp_probe_oui(struct 
> radeon_connector *radeon_connector)
>   if (!(dig_connector->dpcd[DP_DOWN_STREAM_PORT_COUNT] & 
> DP_OUI_SUPPORT))
>   return;
>
> +if (!radeon_connector->ddc_bus)
> +return;
> +
>   if (drm_dp_dpcd_read(&radeon_connector->ddc_bus->aux, 
> DP_SINK_OUI, buf, 3) == 3)
>   DRM_DEBUG_KMS("Sink OUI: %02hx%02hx%02hx\n",
> buf[0], buf[1], buf[2]);
> @@ -391,6 +400,9 @@ bool radeon_dp_getdpcd(struct radeon_connector 
> *radeon_connector)
>   u8 msg[DP_DPCD_SIZE];
>   int ret, i;
>
> +if (!radeon_connector->ddc_bus)
> +return false;
> +

[Bug 97594] [amdgpu SI] "drm/amd/amdgpu: Add GRBM lock to various SI functions" breaks amdgpu support for SI

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

Christian KÃ¶nig  changed:

   What|Removed |Added

 Status|NEW |RESOLVED
 Resolution|--- |FIXED

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/d43fb769/attachment-0001.html>

[PATCH v3] drm/fsl-dcu: Implement gamma_lut atomic crtc properties

2016-09-08 Thread Meng Yi

> 
> > diff --git a/Documentation/devicetree/bindings/display/fsl,dcu.txt
> > b/Documentation/devicetree/bindings/display/fsl,dcu.txt
> > index 63ec2a6..1b1321a 100644
> > --- a/Documentation/devicetree/bindings/display/fsl,dcu.txt
> > +++ b/Documentation/devicetree/bindings/display/fsl,dcu.txt
> > @@ -20,7 +20,11 @@ Optional properties:
> >  Examples:
> >  dcu: dcu at 2ce {
> > compatible = "fsl,ls1021a-dcu";
> > -   reg = <0x0 0x2ce 0x0 0x1>;
> > +   reg = <0x0 0x2ce 0x0 0x2000>,
> > + <0x0 0x2ce2000 0x0 0x2000>,
> > + <0x0 0x2ce4000 0x0 0xc00>,
> > + <0x0 0x2ce4c00 0x0 0x400>;
> > +   reg-names = "regs", "palette", "gamma", "cursor";
> > clocks = <&platform_clk 0>, <&platform_clk 0>;
> > clock-names = "dcu", "pix";
> > big-endian;
> 
> Looks good to me, I feel splitting up the register map makes sense anyway. It 
> is
> also documented that way:
> 
> 36.4 Memory Map
> Table 36-5. Memory map
> 
> ParameterAddress Range
> Register address space   0x â 0x1FFF
> Palette/Tile address space   0x2000 â 0x3FFF
> Gamma_R address space0x4000 â 0x43FF
> Gamma_G address space0x4400 â 0x47FF
> Gamma_B address space0x4800 â 0x4BFF
> Cursor address space 0x4C00 â 0x4FFF

Thanks!^_^

> 
> Can I have an Ack from the device tree maintainers here?
> 
> 
> 
> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c
> > @@ -22,6 +22,22 @@
> >  #include "fsl_dcu_drm_drv.h"
> >  #include "fsl_dcu_drm_plane.h"
> >
> > +static void fsl_crtc_gamma_set(struct drm_crtc *crtc, struct
> > drm_color_lut *lut,
> > + uint32_t size)
> > +{
> > +   struct fsl_dcu_drm_device *fsl_dev = crtc->dev->dev_private;
> > +   unsigned int i;
> > +
> > +   for (i = 0; i < size; i++) {
> > +   regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_R + 4 *
> i,
> > +lut[i].red);
> > +   regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_G + 4
> * i,
> > +lut[i].green);
> > +   regmap_write(fsl_dev->regmap_gamma, FSL_GAMMA_B + 4 *
> i,
> > +lut[i].blue);
> > +   }
> > +}
> > +
> >  static void fsl_dcu_drm_crtc_atomic_flush(struct drm_crtc *crtc,
> >   struct drm_crtc_state *old_crtc_state)
> {
> 
> 
> 
> > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
> > @@ -48,6 +48,15 @@ static const struct regmap_config
> fsl_dcu_regmap_config = {
> > .volatile_reg = fsl_dcu_drm_is_volatile_reg,  };
> >
> > +static const struct regmap_config fsl_dcu_regmap_gamma_config = {
> > +   .reg_bits = 32,
> > +   .reg_stride = 4,
> > +   .val_bits = 32,
> > +   .val_format_endian = REGMAP_ENDIAN_LITTLE,
> 
> I would like to have a comment here why we force little endian.
> 

Oh, I was going to do that, but forgotten anyway.

> > +
> > +   .volatile_reg = fsl_dcu_drm_is_volatile_reg, };
> > +
> 
> 
> 
> > @@ -365,6 +374,25 @@ static int fsl_dcu_drm_probe(struct platform_device
> *pdev)
> > return PTR_ERR(fsl_dev->regmap);
> > }
> >
> > +   res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "gamma");
> > +   if (!res) {
> > +   dev_err(dev, "could not get gamma memory resource\n");
> > +   return -ENODEV;
> > +   }
> > +
> > +   base_gamma = devm_ioremap_resource(dev, res);
> > +   if (IS_ERR(base_gamma)) {
> > +   ret = PTR_ERR(base_gamma);
> > +   return ret;
> > +   }
> > +
> > +   fsl_dev->regmap_gamma = devm_regmap_init_mmio(dev,
> base_gamma,
> > +   &fsl_dcu_regmap_config);
> > +   if (IS_ERR(fsl_dev->regmap_gamma)) {
> > +   dev_err(dev, "regmap_gamma init failed\n");
> > +   return PTR_ERR(fsl_dev->regmap_gamma);
> > +   }
> > +
> 
> Mark, what do you think, is this a reasonable approach to work around this
> errata?

I was thinking is it possible to define a different endianness for each 
register map.

Best Regards,
Meng

[PATCH v3] drm: modify drm_global_item_ref to avoid two times of writing ref->object

2016-09-08 Thread Huang Rui

On Thu, Sep 08, 2016 at 02:36:06PM +0800, Chris Wilson wrote:
> On Wed, Sep 07, 2016 at 10:07:57PM -0400, Huang Rui wrote:
> > In previous drm_global_item_ref, there are two times of writing
> > ref->object if item->refcount is 0. So this patch does a minor update
> > to put alloc and init ref firstly, and then to modify the item of glob
> > array. Use "else" to avoid two times of writing ref->object. It can
> > make the code logic more clearly.
> > 
> > Signed-off-by: Huang Rui 
> > ---
> > 
> > Changes from V2 -> V3:
> > - Use duplicate mutex release to avoid "goto" in non-error patch.
> > - Rename error label
> > 
> > ---
> >  drivers/gpu/drm/drm_global.c | 24 ++--
> >  1 file changed, 14 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_global.c b/drivers/gpu/drm/drm_global.c
> > index 3d2e91c..b404287 100644
> > --- a/drivers/gpu/drm/drm_global.c
> > +++ b/drivers/gpu/drm/drm_global.c
> > @@ -65,30 +65,34 @@ void drm_global_release(void)
> >  
> >  int drm_global_item_ref(struct drm_global_reference *ref)
> >  {
> > -   int ret;
> > +   int ret = 0;
> > struct drm_global_item *item = &glob[ref->global_type];
> >  
> > mutex_lock(&item->mutex);
> > if (item->refcount == 0) {
> > -   item->object = kzalloc(ref->size, GFP_KERNEL);
> > -   if (unlikely(item->object == NULL)) {
> > +   ref->object = kzalloc(ref->size, GFP_KERNEL);
> 
> So the item refcount is 0, we operate on ref, whereas previous we
> inspected item and operated on item. Not an improvement.

Hmm, when item refcount is 0, we operate on ref to create object and init
it for item, so although item->object and ref->object here should point the
same thing, but we should alloc and init ref firstly and pass the
ref->object address to item (item actually points a static area). This make
the code logic more clearly and readable. So I updated it to "ref" here.
:-)

Thanks,
Rui

[PATCH v3] drm: modify drm_global_item_ref to avoid two times of writing ref->object

Am 08.09.2016 um 04:07 schrieb Huang Rui:
> In previous drm_global_item_ref, there are two times of writing
> ref->object if item->refcount is 0. So this patch does a minor update
> to put alloc and init ref firstly, and then to modify the item of glob
> array. Use "else" to avoid two times of writing ref->object. It can
> make the code logic more clearly.
>
> Signed-off-by: Huang Rui 

Reviewed-by: Christian KÃ¶nig .

> ---
>
> Changes from V2 -> V3:
> - Use duplicate mutex release to avoid "goto" in non-error patch.
> - Rename error label
>
> ---
>   drivers/gpu/drm/drm_global.c | 24 ++--
>   1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_global.c b/drivers/gpu/drm/drm_global.c
> index 3d2e91c..b404287 100644
> --- a/drivers/gpu/drm/drm_global.c
> +++ b/drivers/gpu/drm/drm_global.c
> @@ -65,30 +65,34 @@ void drm_global_release(void)
>   
>   int drm_global_item_ref(struct drm_global_reference *ref)
>   {
> - int ret;
> + int ret = 0;
>   struct drm_global_item *item = &glob[ref->global_type];
>   
>   mutex_lock(&item->mutex);
>   if (item->refcount == 0) {
> - item->object = kzalloc(ref->size, GFP_KERNEL);
> - if (unlikely(item->object == NULL)) {
> + ref->object = kzalloc(ref->size, GFP_KERNEL);
> + if (unlikely(ref->object == NULL)) {
>   ret = -ENOMEM;
> - goto out_err;
> + goto error_unlock;
>   }
> -
> - ref->object = item->object;
>   ret = ref->init(ref);
>   if (unlikely(ret != 0))
> - goto out_err;
> + goto error_free;
>   
> + item->object = ref->object;
> + } else {
> + ref->object = item->object;
>   }
> +
>   ++item->refcount;
> - ref->object = item->object;
>   mutex_unlock(&item->mutex);
>   return 0;
> -out_err:
> +
> +error_free:
> + kfree(ref->object);
> + ref->object = NULL;
> +error_unlock:
>   mutex_unlock(&item->mutex);
> - item->object = NULL;
>   return ret;
>   }
>   EXPORT_SYMBOL(drm_global_item_ref);

[PATCH v2 2/9] drm/sun4i: support A33 tcon

On Tue, Sep 06, 2016 at 11:16:33PM +0800, Chen-Yu Tsai wrote:
> On Tue, Sep 6, 2016 at 10:46 PM, Maxime Ripard
>  wrote:
> > The A33 has a significantly different pipeline, with components that differ
> > too.
> >
> > Make sure we had compatible for them.
> >
> > Signed-off-by: Maxime Ripard 
> > ---
> >  Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt | 7 ++-
> >  drivers/gpu/drm/sun4i/sun4i_backend.c | 1 +
> >  drivers/gpu/drm/sun4i/sun4i_drv.c | 8 +---
> >  drivers/gpu/drm/sun4i/sun4i_tcon.c| 8 +++-
> >  4 files changed, 19 insertions(+), 5 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt 
> > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > index df8f4aeefe4c..bd3136a5cba5 100644
> > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt
> > @@ -26,7 +26,9 @@ TCON
> >  The TCON acts as a timing controller for RGB, LVDS and TV interfaces.
> >
> >  Required properties:
> > - - compatible: value should be "allwinner,sun5i-a13-tcon".
> > + - compatible: value must be either:
> > +   * allwinner,sun5i-a13-tcon
> > +   * allwinner,sun8i-a33-tcon
> >   - reg: base address and size of memory-mapped region
> >   - interrupts: interrupt associated to this IP
> >   - clocks: phandles to the clocks feeding the TCON. Three are needed:
> 
> You should probably put a note saying the A33 only needs 2 clocks.
> You can keep my ack after fixing it.

I did while applying, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/eeaaa040/attachment.sig>

[PATCH v3] drm: modify drm_global_item_ref to avoid two times of writing ref->object

On Thu, Sep 08, 2016 at 03:22:48PM +0800, Huang Rui wrote:
> On Thu, Sep 08, 2016 at 02:36:06PM +0800, Chris Wilson wrote:
> > On Wed, Sep 07, 2016 at 10:07:57PM -0400, Huang Rui wrote:
> > > In previous drm_global_item_ref, there are two times of writing
> > > ref->object if item->refcount is 0. So this patch does a minor update
> > > to put alloc and init ref firstly, and then to modify the item of glob
> > > array. Use "else" to avoid two times of writing ref->object. It can
> > > make the code logic more clearly.
> > > 
> > > Signed-off-by: Huang Rui 
> > > ---
> > > 
> > > Changes from V2 -> V3:
> > > - Use duplicate mutex release to avoid "goto" in non-error patch.
> > > - Rename error label
> > > 
> > > ---
> > >  drivers/gpu/drm/drm_global.c | 24 ++--
> > >  1 file changed, 14 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_global.c b/drivers/gpu/drm/drm_global.c
> > > index 3d2e91c..b404287 100644
> > > --- a/drivers/gpu/drm/drm_global.c
> > > +++ b/drivers/gpu/drm/drm_global.c
> > > @@ -65,30 +65,34 @@ void drm_global_release(void)
> > >  
> > >  int drm_global_item_ref(struct drm_global_reference *ref)
> > >  {
> > > - int ret;
> > > + int ret = 0;
> > >   struct drm_global_item *item = &glob[ref->global_type];
> > >  
> > >   mutex_lock(&item->mutex);
> > >   if (item->refcount == 0) {
> > > - item->object = kzalloc(ref->size, GFP_KERNEL);
> > > - if (unlikely(item->object == NULL)) {
> > > + ref->object = kzalloc(ref->size, GFP_KERNEL);
> > 
> > So the item refcount is 0, we operate on ref, whereas previous we
> > inspected item and operated on item. Not an improvement.
> 
> Hmm, when item refcount is 0, we operate on ref to create object and init
> it for item, so although item->object and ref->object here should point the
> same thing, but we should alloc and init ref firstly and pass the
> ref->object address to item (item actually points a static area). This make
> the code logic more clearly and readable. So I updated it to "ref" here.
> :-)

The object is owned by item. ref is just a pointer to it.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH v3] drm: modify drm_global_item_ref to avoid two times of writing ref->object

Am 08.09.2016 um 09:35 schrieb Chris Wilson:
> On Thu, Sep 08, 2016 at 03:22:48PM +0800, Huang Rui wrote:
>> On Thu, Sep 08, 2016 at 02:36:06PM +0800, Chris Wilson wrote:
>>> On Wed, Sep 07, 2016 at 10:07:57PM -0400, Huang Rui wrote:
 In previous drm_global_item_ref, there are two times of writing
 ref->object if item->refcount is 0. So this patch does a minor update
 to put alloc and init ref firstly, and then to modify the item of glob
 array. Use "else" to avoid two times of writing ref->object. It can
 make the code logic more clearly.

 Signed-off-by: Huang Rui 
 ---

 Changes from V2 -> V3:
 - Use duplicate mutex release to avoid "goto" in non-error patch.
 - Rename error label

 ---
   drivers/gpu/drm/drm_global.c | 24 ++--
   1 file changed, 14 insertions(+), 10 deletions(-)

 diff --git a/drivers/gpu/drm/drm_global.c b/drivers/gpu/drm/drm_global.c
 index 3d2e91c..b404287 100644
 --- a/drivers/gpu/drm/drm_global.c
 +++ b/drivers/gpu/drm/drm_global.c
 @@ -65,30 +65,34 @@ void drm_global_release(void)
   
   int drm_global_item_ref(struct drm_global_reference *ref)
   {
 -  int ret;
 +  int ret = 0;
struct drm_global_item *item = &glob[ref->global_type];
   
mutex_lock(&item->mutex);
if (item->refcount == 0) {
 -  item->object = kzalloc(ref->size, GFP_KERNEL);
 -  if (unlikely(item->object == NULL)) {
 +  ref->object = kzalloc(ref->size, GFP_KERNEL);
>>> So the item refcount is 0, we operate on ref, whereas previous we
>>> inspected item and operated on item. Not an improvement.
>> Hmm, when item refcount is 0, we operate on ref to create object and init
>> it for item, so although item->object and ref->object here should point the
>> same thing, but we should alloc and init ref firstly and pass the
>> ref->object address to item (item actually points a static area). This make
>> the code logic more clearly and readable. So I updated it to "ref" here.
>> :-)
> The object is owned by item. ref is just a pointer to it.

Yeah, so what? We initialized ref->item when the refcount was 0 before 
as well.

Why not create the reference first and initialize the item later on?

Regards,
Christian.

> -Chris
>

[PATCH v3] drm: modify drm_global_item_ref to avoid two times of writing ref->object

On Thu, Sep 08, 2016 at 09:43:52AM +0200, Christian KÃ¶nig wrote:
> Am 08.09.2016 um 09:35 schrieb Chris Wilson:
> >On Thu, Sep 08, 2016 at 03:22:48PM +0800, Huang Rui wrote:
> >>On Thu, Sep 08, 2016 at 02:36:06PM +0800, Chris Wilson wrote:
> >>>On Wed, Sep 07, 2016 at 10:07:57PM -0400, Huang Rui wrote:
> In previous drm_global_item_ref, there are two times of writing
> ref->object if item->refcount is 0. So this patch does a minor update
> to put alloc and init ref firstly, and then to modify the item of glob
> array. Use "else" to avoid two times of writing ref->object. It can
> make the code logic more clearly.
> 
> Signed-off-by: Huang Rui 
> ---
> 
> Changes from V2 -> V3:
> - Use duplicate mutex release to avoid "goto" in non-error patch.
> - Rename error label
> 
> ---
>   drivers/gpu/drm/drm_global.c | 24 ++--
>   1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_global.c b/drivers/gpu/drm/drm_global.c
> index 3d2e91c..b404287 100644
> --- a/drivers/gpu/drm/drm_global.c
> +++ b/drivers/gpu/drm/drm_global.c
> @@ -65,30 +65,34 @@ void drm_global_release(void)
>   int drm_global_item_ref(struct drm_global_reference *ref)
>   {
> - int ret;
> + int ret = 0;
>   struct drm_global_item *item = &glob[ref->global_type];
>   mutex_lock(&item->mutex);
>   if (item->refcount == 0) {
> - item->object = kzalloc(ref->size, GFP_KERNEL);
> - if (unlikely(item->object == NULL)) {
> + ref->object = kzalloc(ref->size, GFP_KERNEL);
> >>>So the item refcount is 0, we operate on ref, whereas previous we
> >>>inspected item and operated on item. Not an improvement.
> >>Hmm, when item refcount is 0, we operate on ref to create object and init
> >>it for item, so although item->object and ref->object here should point the
> >>same thing, but we should alloc and init ref firstly and pass the
> >>ref->object address to item (item actually points a static area). This make
> >>the code logic more clearly and readable. So I updated it to "ref" here.
> >>:-)
> >The object is owned by item. ref is just a pointer to it.
> 
> Yeah, so what? We initialized ref->item when the refcount was 0
> before as well.
> 
> Why not create the reference first and initialize the item later on?

More to the point, there is a genuine bug in the code. It happens to be
fixed by this patch, but not purported to be. There is also a leak of
item->object, and the globals conflict between drivers. If radeon
instantiates the DRM_GLOBAL_TTM_MEM, nouveau bumps the reference and
radeon subsequently unloaded, when nouveau unloads it will crash due to
executing a random address. As a singleton class for ttm, it could do
with a revamp.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH v2 0/9] drm/sun4i: Introduce A33 display driver

On Tue, Sep 06, 2016 at 04:46:11PM +0200, Maxime Ripard wrote:
> Hi everyone,
> 
> This serie introduces the support in the sun4i-drm driver for the A33.
> 
> Beside the new IPs and special cases for the A33 new IPs, there's
> nothing really outstanding, and is now at feature parity with the A13.
> 
> This serie is based on my A33 CCU patches posted earlier today here:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/453208.html
> 
> Let me know what you think,
> Maxime

Merged the patches 1-4.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/55a51584/attachment-0001.sig>

[Intel-gfx] [PATCH] drm: Fix error path in drm_mode_page_flip_ioctl()

2016-09-08 Thread Jani Nikula

On Thu, 08 Sep 2016, Michel DÃ¤nzer  wrote:
> On 08/09/16 02:23 AM, Imre Deak wrote:
>> This fixes the error path for platforms that don't define the new
>> page_flip_target() hook.
>> 
>> Fixes: c229bfbbd04 ("drm: Add page_flip_target CRTC hook v2")
>> Testcase: igt/kms_flip/basic-flip-vs-dpms
>> CC: Michel DÃ¤nzer 
>> Signed-off-by: Imre Deak 
>> ---
>>  drivers/gpu/drm/drm_crtc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index 0f797ee..d84a0ea 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -2047,7 +2047,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
>>  }
>>  
>>  out:
>> -if (ret)
>> +if (ret && crtc->funcs->page_flip_target)
>>  drm_crtc_vblank_put(crtc);
>>  if (fb)
>>  drm_framebuffer_unreference(fb);
>> 
>
> Nice catch, thanks!
>
> Reviewed-by: Michel DÃ¤nzer 

Picked up to drm-misc, thanks for the patch and review.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center

[PULL] drm-intel-fixes

2016-09-08 Thread Jani Nikula


Hi Dave, some i915 fixes for v4.8.

BR,
Jani.

The following changes since commit 3eab887a55424fc2c27553b7bfe32330df83f7b8:

  Linux 4.8-rc4 (2016-08-28 15:04:33 -0700)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/drm-intel-fixes-2016-09-08

for you to fetch changes up to fc2780b66b15092ac68272644a522c1624c48547:

  drm/i915: Add GEN7_PCODE_MIN_FREQ_TABLE_GT_RATIO_OUT_OF_RANGE to SNB 
(2016-09-07 17:40:43 +0300)


Chris Wilson (2):
  drm/i915/dvo: Remove dangling call to drm_encoder_cleanup()
  drm/i915: Add GEN7_PCODE_MIN_FREQ_TABLE_GT_RATIO_OUT_OF_RANGE to SNB

Ping Gao (1):
  drm/i915: enable vGPU detection for all

Zhi Wang (1):
  drm/i915: disable 48bit full PPGTT when vGPU is active

 drivers/gpu/drm/i915/i915_gem_gtt.c | 9 ++---
 drivers/gpu/drm/i915/i915_vgpu.c| 3 ---
 drivers/gpu/drm/i915/intel_dvo.c| 1 -
 drivers/gpu/drm/i915/intel_pm.c | 1 +
 4 files changed, 7 insertions(+), 7 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center

[Bug 97634] [amdgpu SI] multigpu setup crashes during boot when dpm=1

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

Bug ID: 97634
   Summary: [amdgpu SI] multigpu setup crashes during boot when
dpm=1
   Product: DRI
   Version: DRI git
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: arek.rusi at gmail.com

Created attachment 126300
  --> https://bugs.freedesktop.org/attachment.cgi?id=126300&action=edit
kernel log

tested on drm-next-4.9-wip:
1) 832c6ef + 2 patches from Tom and Michel (another bugs)
2) 2c0d731 
the same behavior on both. 

With dpm=0 amdgpu doesn't complain and works with intel. 

Hard to say it's regression because when I tried DRI_PRIME few month ago dpm
didn't work at all.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/b2e1ad29/attachment.html>

[PULL] topic/drm-misc

2016-09-08 Thread Jani Nikula


Hi Dave -

Here's a drm-misc pull request mainly to get some fixes moving forward.

BR,
Jani.

The following changes since commit 2b2fd56d7e92f134ecaae5c89e20f64dd0f95aa2:

  Revert "drm: make DRI1 drivers depend on BROKEN" (2016-09-01 06:16:12 +1000)

are available in the git repository at:

  git://anongit.freedesktop.org/drm-intel tags/topic/drm-misc-2016-09-08

for you to fetch changes up to dec90ea1456b5a5d990d94ade2e45a2457cfd149:

  drm: Fix error path in drm_mode_page_flip_ioctl() (2016-09-08 11:57:13 +0300)


Haixia Shi (1):
  drm/udl: implement usb_driver suspend/resume.

Imre Deak (1):
  drm: Fix error path in drm_mode_page_flip_ioctl()

Maarten Lankhorst (2):
  drm/atomic: Reject properties not part of the object.
  Revert "drm: Unify handling of blob and object properties"

Tomeu Vizoso (1):
  drm/doc: Add a few words on validation with IGT

Xie XiuQi (1):
  drm: fix signed integer overflow

 Documentation/gpu/drm-uapi.rst| 37 +
 drivers/gpu/drm/drm_atomic.c  | 11 ++-
 drivers/gpu/drm/drm_crtc.c|  2 +-
 drivers/gpu/drm/drm_hashtab.c |  2 +-
 drivers/gpu/drm/drm_property.c| 23 ++-
 drivers/gpu/drm/udl/udl_drv.c | 16 
 drivers/gpu/drm/udl/udl_drv.h |  2 ++
 drivers/gpu/drm/udl/udl_modeset.c | 14 ++
 8 files changed, 99 insertions(+), 8 deletions(-)

-- 
Jani Nikula, Intel Open Source Technology Center

[PATCH] drm/sti: mark symbols static where possible

[Trimming down the CC list]

Hi Baoyou,

On 7 September 2016 at 12:05, Baoyou Xie  wrote:
> We get 2 warnings when building kernel with W=1:
As you're going through DRM I was wondering if you have a rough number
of warnings we get at the various W levels 1,2,...

Hope you'll have the time/interest to sort some of the W>1 ones as well :-)
Thanks
Emil

[PATCH 6/6] drm: Add DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE/RELATIVE flags v2

2016-09-08 Thread Pekka Paalanen

On Tue, 16 Aug 2016 10:46:13 +0900
Michel DÃ¤nzer  wrote:

> On 16/08/16 10:32 AM, Mario Kleiner wrote:
> > Cc'ing Daniel Stone and Pekka Paalanen, because this relates to wayland.
> > 
> > Wrt. having a new pageflip parameter struct, i wonder if it wouldn't
> > make sense to then already prepare some space in it for specifying an
> > absolute target time, e.g., in u64 microseconds? Or make this part of
> > the atomic pageflip framework?  
> 
> I feel like this is too much hassle for the DRM_IOCTL_MODE_PAGE_FLIP
> ioctl, probably makes sense to only deal with this in the atomic API.
> 
> 
> > In Wayland we now have the presentation_feedback extension (maybe it got
> > a new name?), which is great to get feedback when and how presentation
> > was completed, essentially the equivalent of X11's GLX_INTEL_swap_events
> > + some useful extra info / the feedback half of OML_sync_control.
> > 
> > That was supposed to be complemented by a presentation_queue extension
> > which would provide the other half of OML_sync_control, the part where
> > an app can tell the compositor when and how to present surface updates.
> > I remember that a year ago inclusion of that extension was blocked on
> > some other more urgent important blocker bugs? Did this make progress?

Hi,

sorry, I'm pretty poor at following dri-devel at .

Yeah, the Wayland extension for queueing frames to be presented at
specific times has not been going anywhere for a long time. IIRC the
blockers have since been resolved, now it would need dusting off and
seeing how it interacts with those protocol additions. I wasn't too
happy with the design at the time, either, because of the question of
which state to queue and which not.

Nowadays presentation_feedback lives in
https://cgit.freedesktop.org/wayland/wayland-protocols/tree/stable/presentation-time
and has been declared stable.

> > For timing sensitive applications such an extension is even more
> > important in a wayland world than under X11. On X11 most desktops allow
> > unredirecting fullscreen windows, so an app can drive the flip timing
> > rather direct. At least on Weston as i remember it from my last tests a
> > year ago, that wasn't possible, and an app that doesn't want to present
> > at each video refresh, but at specific target times had to guess what
> > the specific compositors scheduling strategy might be and then "play
> > games" wrt. to timing surface commit's to trick the compositor into sort
> > of doing the right thing - very fragile.
> > 
> > Anyway, the idea of presentation_queue was to specify the requested time
> > of presentation as actual time, not as a target vblank count. This
> > because applications that care about precise presentation timing, like
> > my kind of neuroscience/medical visual stimulation software, or also
> > video players, or e.g., at least the VDPAU video presentation api
> > "think" in absolute time, not in refresh cycles. Also a target vblank
> > count for presentation is less meaningful than a target time for things
> > like variable framerate displays/adaptive sync, or displays which don't
> > have a classic refresh cycle at all. It might also be useful if one
> > thinks about something like VR compositors, where precise timing control
> > could help for some of the tricks ("time warping" iirc?) they use to
> > hide/cope with latency from head tracking -> display.
> > 
> > It would be nice if the kernel could help compositors which would
> > implement presentation_queue or similar to be robust/precise in face of
> > future stuff like Freesync, or of added delays due to Optimus/Prime
> > hybrid-graphics setups. If we wanted to synchronize presentation of
> > multiple displays in a Freesync type display setup, absolute target
> > times could also be helpful.  
> 
> I agree with all of this though.

I'm very happy to hear the idea has support!


Thanks,
pq


> I'd like to add that the X11 Present extension specification already
> includes support for specifying a target time instead of a target
> refresh cycle. This isn't implemented yet though, but it should be
> relatively straightforward to implement using the kind of kernel API you
> describe.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/060525fe/attachment.sig>

[PATCH 5/6] ARM: dts: Add NextThing GR8 dtsi

Hi Javier,

On Wed, Sep 07, 2016 at 04:51:55PM +0200, Javier Martinez Canillas wrote:
> Hello Maxime,
> 
> On Wed, Aug 31, 2016 at 10:18 AM, Maxime Ripard
>  wrote:
> 
> [snip]
> 
> > +
> > +#include "skeleton.dtsi"
> > +
> 
> The skeleton.dtsi has been deprecated and shouldn't be used in new DTS files:
> 
> http://www.spinics.net/lists/arm-kernel/msg528080.html

Ok, thanks, I'll change it in the v3.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/d7268678/attachment-0001.sig>

[PATCH 5/6] ARM: dts: Add NextThing GR8 dtsi

On Wed, Sep 07, 2016 at 07:51:48PM +0200, Rask Ingemann Lambertsen wrote:
> On Wed, Aug 31, 2016 at 4:18 PM, Maxime Ripard
>  wrote:
> > From: MylÃ¨ne Josserand 
> >
> > The GR8 is an SoC made by Nextthing loosely based on the sun5i family.
> >
> > Since it's not clear yet what we can factor out and merge with the A10s and
> > A13 support, let's keep it out of the sun5i.dtsi include tree. We will
> > figure out what can be shared when things settle down.
> >
> > Signed-off-by: MylÃ¨ne Josserand 
> > Signed-off-by: Maxime Ripard 
> > ---
> >  arch/arm/boot/dts/gr8.dtsi | 1080 
> > 
> >  1 file changed, 1080 insertions(+)
> >  create mode 100644 arch/arm/boot/dts/gr8.dtsi
> >
> > diff --git a/arch/arm/boot/dts/gr8.dtsi b/arch/arm/boot/dts/gr8.dtsi
> > new file mode 100644
> > index ..d21cfa3f3c14
> > --- /dev/null
> > +++ b/arch/arm/boot/dts/gr8.dtsi
> 
> In the node names, you sometimes use underscores and sometimes use hyphens.
> Here are the ones I spotted:
> 
> > +   osc3M: osc3M_clk {
> > +   pll3x2: pll3x2_clk {
> > +   pll7x2: pll7x2_clk {
> > +   display-engine {
> > +   sram-controller at 01c0 {
> > +   otg_sram: sram-section at  {
> > +   dma: dma-controller at 01c02000 {
> > +   tve0: tv-encoder at 01c0a000 {
> > +   tcon0: lcd-controller at 01c0c000 {
> > +   intc: interrupt-controller at 01c20400 {
> > +   lcd_rgb666_pins: lcd_rgb666 at 0 {
> > +   nand_pins_a: nand_base0 at 0 {
> > +   nand_cs0_pins_a: nand_cs at 0 {
> > +   nand_rb0_pins_a: nand_rb at 0 {
> > +   uart1_cts_rts_pins_a: uart1-cts-rts at 0 {
> > +   fe0: display-frontend at 01e0 {
> > +   be0: display-backend at 01e6 {
> 
> Underscores should not be used in node names. [1][2] Since you're adding a
> new file here, please use hyphens instead.

I wonder what the rationale behind this is. The ePAPR clearly
documents the underscore as being a valid character for the node
names.

I'll change the few inconsistencies though.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/aae07d1e/attachment.sig>

[PATCH] drm/sti: mark symbols static where possible

2016-09-08 Thread Arnd Bergmann

On Thursday, September 8, 2016 10:35:17 AM CEST Emil Velikov wrote:
> On 7 September 2016 at 12:05, Baoyou Xie  wrote:
> > We get 2 warnings when building kernel with W=1:
> As you're going through DRM I was wondering if you have a rough number
> of warnings we get at the various W levels 1,2,...

I've looked at the W=1 warnings overall, and the count I got a
month ago was 648 warnings for drivers/gpu/::

471 -Werror=missing-prototypes
 12 -Werror=type-limits
124 -Werror=unused-but-set-variable
 41 -Werror=unused-const-variable=

vs for the whole kernel

   2033 -Werror=missing-prototypes
 58 -Werror=suggest-attribute=format
167 -Werror=type-limits
   1398 -Werror=unused-but-set-variable
   1526 -Werror=unused-const-variable=

but that was after I had already fixed some of the other warnings
locally. It shouldn't be hard to fix all of them for any given
subsystem, often a single line change gets rid of a number
of individual warnings.

My basic idea however is not to do it by subsystem but instead
do it one warning at a time for the entire kernel and then enable
that warning by default without W=1.

> Hope you'll have the time/interest to sort some of the W>1 ones as well 

I suggested to Baoyou that he starts looking at missing-prototype
warnings across the kernel, as these are likely to find the most
actual bugs out of the W=1 warnings we get.

Arnd

[PATCH] drm/sti: mark symbols static where possible

2016-09-08 Thread Benjamin Gaignard

Acked-by: Benjamin Gaignard 

2016-09-07 13:05 GMT+02:00 Baoyou Xie :
> We get 2 warnings when building kernel with W=1:
> drivers/gpu/drm/sti/sti_mixer.c:361:6: warning: no previous prototype for 
> 'sti_mixer_set_matrix' [-Wmissing-prototypes]
> drivers/gpu/drm/sti/sti_dvo.c:109:5: warning: no previous prototype for 
> 'dvo_awg_generate_code' [-Wmissing-prototypes]
>
> In fact, these functions are only used in the file in which they are
> declared and don't need a declaration, but can be made static.
> So this patch marks these functions with 'static'.
>
> Signed-off-by: Baoyou Xie 
> ---
>  drivers/gpu/drm/sti/sti_dvo.c   | 3 ++-
>  drivers/gpu/drm/sti/sti_mixer.c | 2 +-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> index 00881eb..4545ad0 100644
> --- a/drivers/gpu/drm/sti/sti_dvo.c
> +++ b/drivers/gpu/drm/sti/sti_dvo.c
> @@ -106,7 +106,8 @@ struct sti_dvo_connector {
> container_of(x, struct sti_dvo_connector, drm_connector)
>
>  #define BLANKING_LEVEL 16
> -int dvo_awg_generate_code(struct sti_dvo *dvo, u8 *ram_size, u32 *ram_code)
> +static int
> +dvo_awg_generate_code(struct sti_dvo *dvo, u8 *ram_size, u32 *ram_code)
>  {
> struct drm_display_mode *mode = &dvo->mode;
> struct dvo_config *config = dvo->config;
> diff --git a/drivers/gpu/drm/sti/sti_mixer.c b/drivers/gpu/drm/sti/sti_mixer.c
> index 7d9aea8..b78cec5 100644
> --- a/drivers/gpu/drm/sti/sti_mixer.c
> +++ b/drivers/gpu/drm/sti/sti_mixer.c
> @@ -358,7 +358,7 @@ int sti_mixer_set_plane_status(struct sti_mixer *mixer,
> return 0;
>  }
>
> -void sti_mixer_set_matrix(struct sti_mixer *mixer)
> +static void sti_mixer_set_matrix(struct sti_mixer *mixer)
>  {
> unsigned int i;
>
> --
> 2.7.4
>



-- 
Benjamin Gaignard

Graphic Study Group

Linaro.org â Open source software for ARM SoCs

Follow Linaro: Facebook | Twitter | Blog

[PATCH 1/1] drm: i915: don't use OpRegion for XPS 13 IvyBridge

On Thu, Sep 08, 2016 at 12:04:39PM +0200, Martin van Es wrote:
> On dinsdag 6 september 2016 21:40:48 CEST Ville SyrjÃ¤lÃ¤ wrote:
> > On Tue, Sep 06, 2016 at 01:56:20PM +0300, Ville SyrjÃ¤lÃ¤ wrote:
> > > Actually I just cooked up another branch [2]. It just throws in some
> > > memory barriers to the opregion code, and adds a a new debug print so
> > > to show the response from the BIOS. I'm not too optimistic that the
> > > memory barriers would fix it, but at least we'd get to see the full
> > > response from the BIOS. Can you give this a try?
> > > 
> > > [2] git://github.com/vsyrjala/linux.git opregion_panel_type_stuff
> 
> This kernel doesn't boot (for me).
> 
> I cloned the repo, copied .config from 4.7 kernel, make oldconfig, accepted 
> all defaults and made+installed the kernel. This installed an image 
> 4.0.0-rc7+ 
> (is that correct?) that was unbootable (halts at loading ramdisk).

The version should be 4.7 or 4.8 something. Maybe you used the wrong
branch?

Anywyas I pushed a new branch "opregion_panel_type_quirk" which I'm
hoping will be the final fix we go with. Just waiting for confirmation
that I got the quirk right and that the original machine fixed by the
OpRegion stuff still works. You might want to test that one as well.

-- 
Ville SyrjÃ¤lÃ¤
Intel OTC

[PATCH 001/001] drivers/gpu/radeon: NULL pointer deference workaround

Am 08.09.2016 um 11:09 schrieb Mark Fortescue:
> Hi Christian,
>
> Thank you for the feedback.
>
> On 08/09/16 08:14, Christian KÃ¶nig wrote:
>> Am 07.09.2016 um 19:38 schrieb Mark Fortescue:
>>>
>>> On an LV-683 (AMD Dual-core G-T56N) Mini-ITX board, I get a Kernel
>>> Oops because Connector 0 (LCD Panel interface) does not have DDC.
>>
>> I'm not an expert on this, but that is really odd cause even LCD Panels
>> should have a DDC interface.
>>
>>>
>>> Ubuntu 16.04 LTS Kernel (4.4 series):
>>>
>>> ...
>>> [ 8.262990] [drm] ib test on ring 5 succeeded
>>> [ 8.288897] [drm] Radeon Display Connectors
>>> [ 8.293175] [drm] Connector 0:
>>> [ 8.296252] [drm] DP-1
>>
>> Especially since the BIOS claims that this is a displayport connector
>> and there is no physical way to have a DP without an DDC as far as I 
>> know.
>>
>> Please open a bug report on FDO and attach you BIOS image.
>
> FDO ? I am not familiar with this. Please can you enlighten me.
>

See here: http://bugs.freedesktop.org/

> I do not have a BIOS image so will need some assistance in 
> understanding what is required here and how I extract the BIOS 
> information you are after.
>

Sorry my fault. Mullins is an APU, so you don't have a dedicated video 
BIOS. As usually I didn't got enough sleep :) But please open up a bug 
report anyway.

> On this motherboard, DP-1 is a single channel 18bit LVDS LCD panel 
> interface and DP-2 is a DVI interface (which I can connect to my 
> monitor if testing this is useful). There are no displayport connectors.

Yeah, but from the driver point of view there are only DP connectors on 
the chip. The LVDS and DVI are probably realized with external DP to 
whatever converter ICs.

>
> On industrial motherboards, I have noticed that it is not uncommon to 
> hard code the information for the LCD panel into the BIOS so no DDC is 
> required. In this case, there is no LCD panel connected to the 
> interface anyway.
>

That is correct and as far as I know well supported by Radeon, but the 
crux is there should still be a DDC channel even if there isn't anything 
attached to it.

See with displayport you got four LVDS channels to submit the actual 
picture and an AUX channel to configure and query the device. The DDC is 
just represented as certain packets over the AUX channel.

If the AUX channel doesn't work or isn't connect then the link training 
wouldn't be possible as well and so you wouldn't be able to get any 
picture on the LVDS.

> See http://www.commell.com.tw/product/SBC/LV-683.HTM for more 
> information on the board. Looking at the web site, there is a BIOS 
> image available form Commell if that is of use.

Alex clearly needs to take a look on this. I think for the time being 
you could hack together a patch which just ignores DP connectors during 
probing if they don't have an associated DDC instead of changing the 
code everywhere the DDC object is required.

Regards,
Christian.

>
>>
>> Alex can probably take a look when he's back from vacation.
>>
>> Regards,
>> Christian.
>>
>>> [ 8.298791] [drm] Encoders:
>>> [ 8.301770] [drm] DFP1: INTERNAL_UNIPHY
>>> [ 8.305973] [drm] Connector 1:
>>> [ 8.309043] [drm] DP-2
>>> [ 8.311598] [drm] HPD2
>>> [ 8.314169] [drm] DDC: 0x6440 0x6440 0x6444 0x6444 0x6448 0x6448
>>> 0x644c 0x644c
>>> [ 8.321609] [drm] Encoders:
>>> [ 8.324589] [drm] DFP2: INTERNAL_UNIPHY
>>> [ 8.328793] [drm] Connector 2:
>>> [ 8.331856] [drm] VGA-1
>>> [ 8.342947] [drm] DDC: 0x64d8 0x64d8 0x64dc 0x64dc 0x64e0 0x64e0
>>> 0x64e4 0x64e4
>>> [ 8.350341] [drm] Encoders:
>>> [ 8.353310] [drm] CRT1: INTERNAL_KLDSCP_DAC1
>>> [ 8.358195] BUG: unable to handle kernel NULL pointer dereference at
>>> 0409
>>> [ 8.409733] [] radeon_dp_getsinktype+0x1a/0x30 
>>> [radeon]
>>> [ 8.416805] PGD 0
>>> [ 8.418841] Oops:  [#1] SMP
>>> ...
>>>
>>> This patch prevents Kernel failures due to a connector not having a
>>> DDC interface by changing the code so that ddc_bus is always checked
>>> before use.
>>> The problem was first identified using the uBuntu MATE 14.04 LTS (3.16
>>> series kernels) but not dealt with at that time. On attempting to
>>> install uBuntu MATE 16.04 LTS (4.4 series kernels), it became clear
>>> that using various workarounds to allow the issue to be ignored were
>>> not viable so more effort was put in to sorting the issue resulting in
>>> this patch. See https://bugs.launchpad.net/bugs/1587885 for more 
>>> details.
>>>
>>> Signed-off-by: Mark Fortescue 
>>> Tested-by: Mark Fortescue 
>>>
>>> ---
>>>
>>> Looks like Thunderbird may have made a mess of the patch when pasting
>>> the contents into the mail message - my alternate mail client (pine)
>>> also has strict line length handling and trashes non-MIME encoded
>>> patches.
>>>
>>> This may not be the correct approach to solving the issue but it is
>>> clean in that it ensures that ddc_bus is never used when NULL
>>> regardless of how the code ended up at the point of use.
>>>
>>> If it helps with back porting, I have

[PATCH v2] drm: Move property validation to a helper, v2.

2016-09-08 Thread Maarten Lankhorst

Property lifetimes are equal to the device lifetime, so the separate
drm_property_find is not needed. The pointer can be retrieved from
the properties member, which saves us some locking and a extra lookup.
The lifetime for properties is until the device is destroyed, which
happens late in the device unload path.

kms_atomic is also testing for invalid properties which returns -ENOENT,
to be consistent return -ENOENT for valid properties that don't appear
on the object property list.

Changes since v1:
- Return -ENOENT for invalid properties to make kms_atomic pass.
- Change commit message slightly to take this into account.

Testcase: kms_atomic
Testcase: kms_properties
Fixes: 4e9951d96093 ("drm/atomic: Reject properties not part of the object.")
Suggested-by: Ville SyrjÃ¤lÃ¤ 
Signed-off-by: Maarten Lankhorst 
---
 drivers/gpu/drm/drm_atomic.c| 13 ++---
 drivers/gpu/drm/drm_crtc_internal.h |  2 ++
 drivers/gpu/drm/drm_mode_object.c   | 31 ---
 3 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index fac156c43506..23739609427d 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1609,7 +1609,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
struct drm_crtc_state *crtc_state;
unsigned plane_mask;
int ret = 0;
-   unsigned int i, j, k;
+   unsigned int i, j;

/* disallow for drivers not supporting atomic: */
if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
@@ -1691,16 +1691,7 @@ retry:
goto out;
}

-   for (k = 0; k < obj->properties->count; k++)
-   if (obj->properties->properties[k]->base.id == 
prop_id)
-   break;
-
-   if (k == obj->properties->count) {
-   ret = -EINVAL;
-   goto out;
-   }
-
-   prop = drm_property_find(dev, prop_id);
+   prop = drm_mode_obj_find_prop_id(obj, prop_id);
if (!prop) {
drm_mode_object_unreference(obj);
ret = -ENOENT;
diff --git a/drivers/gpu/drm/drm_crtc_internal.h 
b/drivers/gpu/drm/drm_crtc_internal.h
index a3622644bccf..444e609078cc 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -115,6 +115,8 @@ int drm_mode_object_get_properties(struct drm_mode_object 
*obj, bool atomic,
   uint32_t __user *prop_ptr,
   uint64_t __user *prop_values,
   uint32_t *arg_count_props);
+struct drm_property *drm_mode_obj_find_prop_id(struct drm_mode_object *obj,
+  uint32_t prop_id);

 /* IOCTL */

diff --git a/drivers/gpu/drm/drm_mode_object.c 
b/drivers/gpu/drm/drm_mode_object.c
index 6edda8382a4c..9f17085b1fdd 100644
--- a/drivers/gpu/drm/drm_mode_object.c
+++ b/drivers/gpu/drm/drm_mode_object.c
@@ -372,14 +372,25 @@ out:
return ret;
 }

+struct drm_property *drm_mode_obj_find_prop_id(struct drm_mode_object *obj,
+  uint32_t prop_id)
+{
+   int i;
+
+   for (i = 0; i < obj->properties->count; i++)
+   if (obj->properties->properties[i]->base.id == prop_id)
+   return obj->properties->properties[i];
+
+   return NULL;
+}
+
 int drm_mode_obj_set_property_ioctl(struct drm_device *dev, void *data,
struct drm_file *file_priv)
 {
struct drm_mode_obj_set_property *arg = data;
struct drm_mode_object *arg_obj;
-   struct drm_mode_object *prop_obj;
struct drm_property *property;
-   int i, ret = -EINVAL;
+   int ret = -EINVAL;
struct drm_mode_object *ref;

if (!drm_core_check_feature(dev, DRIVER_MODESET))
@@ -392,23 +403,13 @@ int drm_mode_obj_set_property_ioctl(struct drm_device 
*dev, void *data,
ret = -ENOENT;
goto out;
}
-   if (!arg_obj->properties)
-   goto out_unref;
-
-   for (i = 0; i < arg_obj->properties->count; i++)
-   if (arg_obj->properties->properties[i]->base.id == arg->prop_id)
-   break;

-   if (i == arg_obj->properties->count)
+   if (!arg_obj->properties)
goto out_unref;

-   prop_obj = drm_mode_object_find(dev, arg->prop_id,
-   DRM_MODE_OBJECT_PROPERTY);
-   if (!prop_obj) {
-   ret = -ENOENT;
+   property = drm_mode_obj_find_prop_id(arg_obj, arg->prop_id);
+   if (!property)
goto out_unref;
-   }
-   property = obj_to_property(prop_obj);

if (!drm_pro

[Bug 97635] radeon fails to initialize some DisplayPort monitors

see what's relevant. If you want to
know exactly how the lines were filtered you can look at the script
./gather-info-for-diagnostics.sh.

./logs/timing-stripped
==
The above raw log files with the timing at the beginning of each line
removed. This makes using diff programs easier (I use meld on Linux). If you
want to know exactly how this was done you can look at the script
./gather-info-for-diagnostics.sh.

./logs/timing-stripped/filtered-drm
===
Some of the above raw log files with the timing at the beginning of each
line removed and lines that do not contain radeon information removed. Again,
makes it easier to see what's relevant. If you want to know exactly how this
was done you can look at the script ./gather-info-for-diagnostics.sh.

Scripts
===

./gather-info-for-diagnostics.sh

Does all the heavy lifting in gathering the info.

./display-on.sh
---
This was a curious discovery and may make fixing the issue easier. This is
because I found when the script was like this:

xrandr --output DisplayPort-${1} --mode 1920x1080
xrandr --output DisplayPort-${1} --mode 2560x1440

it sometimes it would turn the display on but others it would turn it off. To
consistantly turn the display on I had to change it to this:

xrandr --output DisplayPort-${1} --mode 1920x1080
sleep 5
xrandr --output DisplayPort-${1} --mode 2560x1440

suggesting there might be a timing problem that needs to be addressed. Even
though running this script can turn the display on that was erroneously off
during boot the display will turn itself back off after a few seconds or so so
it's not a usable workaround. I guess there is some status flag during boot in
the kernel that ultimately can't be changed or overridden that eventually
reasserts itself.

Update: It may not be that the 5 second delay solved the issue. It may be that
just running it again was the solution. Perhaps the first time some cache got
cleared, I'm not really sure, some experimenting is in need on this one.

File Names
==

File names take the form of:
__.txt
E.g. The file:

screens-0-4-good-5-bad_kernel-4.7.2-1-default_logo.nologo-radeon.audio=0-debug-debug_objects_dmsg.txt

can be broken down to:
screens-0-4-good-5-bad = The first 5 of the 6 screens came on as
they should during boot but the 6th one (number 5) did not.
kernel-4.7.2-1-default_logo.nologo-radeon.audio=0-debug-debug_objects
= shows most of the boot command line
dmsg= A key indicating the file contents, from
dmesg in this case
.txt= That this is a text file

If the file starts off with something like this:
screens-0-5-good-after-5-fixed-with_display-on.sh it means after booting and
logging in I ran the script ./display-on.sh to turn on the display and then
gathered all the log information. I will have gathered the log information
prior to running the script as well so you will also see files prefixed with
just screens-0-5-good in such a case.

Let me know what else I can do to help.

--
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL:
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/21cbe7b4/attachment.html>

[Bug 95306] Random Blank(black) screens on "Carrizo"

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

--- Comment #15 from Tom  ---
I did some more testing with kernel 4.8rc5 and it seems it is not related to X
after all. Cold boot to console has exactly the same issues as cold boot to X
(screen blanks). The last message I see on the screen is âfb: switching to
amdgpudrmfb from EFI VGAâ then it turns off and on again but it is blank. So
far console didn't blank after successful start like X does unless I suspended
it.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/6f5680a0/attachment-0001.html>

[Bug 69076] [r300g] RS690: triangle flickering

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

Max Staudt  changed:

   What|Removed |Added

 CC||mstaudt at suse.de

--- Comment #12 from Max Staudt  ---
I have sent a patch to [mesa-dev] that should resolve this issue, no matter
whether the DDX with EXA is used or not.

https://lists.freedesktop.org/archives/mesa-dev/2016-September/128205.html

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/bbef4d4e/attachment.html>

[PATCH v8 08/12] drm/i915: Read DP branch device SW revision

2016-09-08 Thread Kahola, Mika



> -Original Message-
> From: Jim Bride [mailto:jim.bride at linux.intel.com]
> Sent: Thursday, September 8, 2016 12:20 AM
> To: Kahola, Mika 
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
> ville.syrjala at linux.intel.com; daniel.vetter at ffwll.ch
> Subject: Re: [PATCH v8 08/12] drm/i915: Read DP branch device SW revision
> 
> On Wed, Aug 17, 2016 at 01:49:44PM +0300, Mika Kahola wrote:
> > SW revision is mandatory field for DisplayPort branch devices. This is
> > defined in DPCD register field 0x50A.
> 
> To be precise, the revision info is in 0x50A and 0x50B. Since both the major
> and minor versions are called out separately in the DP spec it's probably
> worth mentioning both addresses in the commit message.
You're right. I will update the commit message to be more exact.

> 
> >
> > v2: move drm_dp_ds_revision structure to be part of
> > drm_dp_link structure (Daniel)
> > v3: remove dependency to drm_dp_helper but instead parse
> > DPCD and print SW revision info to dmesg (Ville)
> >
> > Signed-off-by: Mika Kahola 
> 
> With the commit message change requested above, this is:
> 
> Reviewed-by: Jim Bride 
> 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 20 
> >  include/drm/drm_dp_helper.h |  1 +
> >  2 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index 9aebdf6..91ffb79 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1438,6 +1438,25 @@ static void intel_dp_print_hw_revision(struct
> intel_dp *intel_dp)
> > DRM_DEBUG_KMS("sink hw revision: %d.%d\n", (rev & 0xf0) >> 4,
> rev &
> > 0xf);  }
> >
> > +static void intel_dp_print_sw_revision(struct intel_dp *intel_dp) {
> > +   uint8_t rev[2];
> > +   int len;
> > +
> > +   if ((drm_debug & DRM_UT_KMS) == 0)
> > +   return;
> > +
> > +   if (!(intel_dp->dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > + DP_DWN_STRM_PORT_PRESENT))
> > +   return;
> > +
> > +   len = drm_dp_dpcd_read(&intel_dp->aux, DP_BRANCH_SW_REV,
> &rev, 2);
> > +   if (len < 0)
> > +   return;
> > +
> > +   DRM_DEBUG_KMS("sink sw revision: %d.%d\n", rev[0], rev[1]); }
> > +
> >  static int rate_to_index(int find, const int *rates)  {
> > int i = 0;
> > @@ -4302,6 +4321,7 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
> > intel_dp_probe_oui(intel_dp);
> >
> > intel_dp_print_hw_revision(intel_dp);
> > +   intel_dp_print_sw_revision(intel_dp);
> >
> > intel_dp_configure_mst(intel_dp);
> >
> > diff --git a/include/drm/drm_dp_helper.h
> b/include/drm/drm_dp_helper.h
> > index 19ac599..215202f 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -447,6 +447,7 @@
> >  #define DP_BRANCH_OUI  0x500
> >  #define DP_BRANCH_ID0x503
> >  #define DP_BRANCH_HW_REV0x509
> > +#define DP_BRANCH_SW_REV0x50A
> >
> >  #define DP_SET_POWER0x600
> >  # define DP_SET_POWER_D00x1
> > --
> > 1.9.1

[PATCH v3 0/4] drm: Add Support for Passive RGB to VGA bridges

Hi,

This serie is about adding support for the RGB to VGA bridge found in
the A13-Olinuxino and the CHIP VGA adapter.

Both these boards rely on an entirely passive bridge made out of
resitor ladders that do not require any initialisation. The only thing
needed is to get the timings from the screen if available (and if not,
fall back on XGA standards), set up the display pipeline to output on
the RGB bus with the proper timings, and you're done.

This serie also fixes a bunch of bugs uncovered when trying to
increase the resolution, and hence the pixel clock, of our
pipeline. It also fixes a few bugs in the DRM driver itself that went
unnoticed before.

Let me know what you think,
Maxime

Changes from v2:
  - Changed the compatible as suggested
  - Rebased on top 4.8

Changes from v1:
  - Switch to using a vga-connector
  - Use drm_encoder bridge pointer instead of doing our own
  - Report the connector status as unknown instead of connected by
default, and as connected only if we can retrieve the EDID.
  - Switch to of_i2c_get_adapter by node, and put the reference when done
  - Rebased on linux-next 

Maxime Ripard (4):
  drm/bridge: Add RGB to VGA bridge support
  ARM: sun5i: a13-olinuxino: Enable VGA bridge
  ARM: multi_v7: enable VGA bridge
  ARM: sunxi: Enable VGA bridge

 .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  52 +
 arch/arm/boot/dts/sun5i-a13-olinuxino.dts  |  60 ++
 arch/arm/configs/multi_v7_defconfig|   1 +
 arch/arm/configs/sunxi_defconfig   |   1 +
 drivers/gpu/drm/bridge/Kconfig |   6 +
 drivers/gpu/drm/bridge/Makefile|   1 +
 drivers/gpu/drm/bridge/rgb-to-vga.c| 232 +
 7 files changed, 353 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
 create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c

-- 
2.9.3

[PATCH v3 1/4] drm/bridge: Add RGB to VGA bridge support

Some boards have an entirely passive RGB to VGA bridge, based on either
DACs or resistor ladders.

Those might or might not have an i2c bus routed to the VGA connector in
order to access the screen EDIDs.

Add a bridge that doesn't do anything but expose the modes available on the
screen, either based on the EDIDs if available, or based on the XGA
standards.

Acked-by: Rob Herring 
Signed-off-by: Maxime Ripard 
---
 .../bindings/display/bridge/rgb-to-vga-bridge.txt  |  52 +
 drivers/gpu/drm/bridge/Kconfig |   6 +
 drivers/gpu/drm/bridge/Makefile|   1 +
 drivers/gpu/drm/bridge/rgb-to-vga.c| 232 +
 4 files changed, 291 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
 create mode 100644 drivers/gpu/drm/bridge/rgb-to-vga.c

diff --git 
a/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt 
b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
new file mode 100644
index ..83a053fb51a0
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/rgb-to-vga-bridge.txt
@@ -0,0 +1,52 @@
+Passive RGB to VGA bridge
+-
+
+This binding is aimed for entirely passive RGB to VGA bridges that do not
+require any configuration.
+
+Required properties:
+
+- compatible: Must be "rgb-to-vga-bridge"
+
+Required nodes:
+
+This device has two video ports. Their connections are modeled using the OF
+graph bindings specified in Documentation/devicetree/bindings/graph.txt.
+
+- Video port 0 for RGB input
+- Video port 1 for VGA output
+
+
+Example
+---
+
+bridge {
+   compatible = "rgb-to-vga-bridge";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port at 0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   vga_bridge_in: endpoint {
+   remote-endpoint = <&tcon0_out_vga>;
+   };
+   };
+
+   port at 1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   vga_bridge_out: endpoint {
+   remote-endpoint = <&vga_con_in>;
+   };
+   };
+   };
+};
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index b590e678052d..42b95adf5091 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -17,6 +17,12 @@ config DRM_ANALOGIX_ANX78XX
  the HDMI output of an application processor to MyDP
  or DisplayPort.

+config DRM_RGB_TO_VGA
+   tristate "Dumb RGB to VGA Bridge support"
+   select DRM_KMS_HELPER
+   help
+ Support for passive RGB to VGA bridges
+
 config DRM_DW_HDMI
tristate
select DRM_KMS_HELPER
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index efdb07e878f5..3bb8cbe09fe9 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -1,6 +1,7 @@
 ccflags-y := -Iinclude/drm

 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
+obj-$(CONFIG_DRM_RGB_TO_VGA) += rgb-to-vga.o
 obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
diff --git a/drivers/gpu/drm/bridge/rgb-to-vga.c 
b/drivers/gpu/drm/bridge/rgb-to-vga.c
new file mode 100644
index ..84b1b10198a4
--- /dev/null
+++ b/drivers/gpu/drm/bridge/rgb-to-vga.c
@@ -0,0 +1,232 @@
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+struct dumb_vga {
+   struct drm_bridge   bridge;
+   struct drm_connectorconnector;
+
+   struct i2c_adapter  *ddc;
+};
+
+static inline struct dumb_vga *
+drm_bridge_to_dumb_vga(struct drm_bridge *bridge)
+{
+   return container_of(bridge, struct dumb_vga, bridge);
+}
+
+static inline struct dumb_vga *
+drm_connector_to_dumb_vga(struct drm_connector *connector)
+{
+   return container_of(connector, struct dumb_vga, connector);
+}
+
+static int dumb_vga_get_modes(struct drm_connector *connector)
+{
+   struct dumb_vga *vga = drm_connector_to_dumb_vga(connector);
+   struct edid *edid;
+   int ret;
+
+   if (IS_ERR(vga->ddc))
+   goto fallback;
+
+   edid = drm_get_edid(connector, vga->ddc);
+   if (!edid) {
+   DRM_INFO("EDID readout failed, falling back to standard 
modes\n");
+   goto fallback;
+   }
+
+   drm_mode_connector_update_edid_property(connector, edid);
+   return drm_add_edid_modes(connector, edid);
+
+fallback:
+   /*
+* In case we cannot retrieve the EDIDs (broken or missing i2c
+

[PATCH v3 2/4] ARM: sun5i: a13-olinuxino: Enable VGA bridge

Now that we have support for the VGA bridges using our DRM driver, enable
the display engine for the Olimex A13-Olinuxino.

Signed-off-by: Maxime Ripard 
---
 arch/arm/boot/dts/sun5i-a13-olinuxino.dts | 60 +++
 1 file changed, 60 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts 
b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
index b3c234c65ea1..1cc7ff361f49 100644
--- a/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
+++ b/arch/arm/boot/dts/sun5i-a13-olinuxino.dts
@@ -72,6 +72,53 @@
default-state = "on";
};
};
+
+   bridge {
+   compatible = "rgb-to-vga-bridge";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   ports {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   port at 0 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <0>;
+
+   vga_bridge_in: endpoint at 0 {
+   reg = <0>;
+   remote-endpoint = <&tcon0_out_vga>;
+   };
+   };
+
+   port at 1 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   reg = <1>;
+
+   vga_bridge_out: endpoint at 0 {
+   reg = <0>;
+   remote-endpoint = <&vga_con_in>;
+   };
+   };
+   };
+   };
+
+   vga {
+   compatible = "vga-connector";
+
+   port {
+   vga_con_in: endpoint {
+   remote-endpoint = <&vga_bridge_out>;
+   };
+   };
+   };
+};
+
+&be0 {
+   status = "okay";
 };

 &ehci0 {
@@ -211,6 +258,19 @@
status = "okay";
 };

+&tcon0 {
+   pinctrl-names = "default";
+   pinctrl-0 = <&lcd_rgb666_pins>;
+   status = "okay";
+};
+
+&tcon0_out {
+   tcon0_out_vga: endpoint at 0 {
+   reg = <0>;
+   remote-endpoint = <&vga_bridge_in>;
+   };
+};
+
 &uart1 {
pinctrl-names = "default";
pinctrl-0 = <&uart1_pins_b>;
-- 
2.9.3

[PATCH v3 3/4] ARM: multi_v7: enable VGA bridge

Enable the RGB to VGA bridge driver in the defconfig

Signed-off-by: Maxime Ripard 
---
 arch/arm/configs/multi_v7_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/multi_v7_defconfig 
b/arch/arm/configs/multi_v7_defconfig
index 2c8665cd9dc5..22ef41afc658 100644
--- a/arch/arm/configs/multi_v7_defconfig
+++ b/arch/arm/configs/multi_v7_defconfig
@@ -567,6 +567,7 @@ CONFIG_DRM=y
 CONFIG_DRM_I2C_ADV7511=m
 # CONFIG_DRM_I2C_CH7006 is not set
 # CONFIG_DRM_I2C_SIL164 is not set
+CONFIG_DRM_RGB_TO_VGA=m
 CONFIG_DRM_NXP_PTN3460=m
 CONFIG_DRM_PARADE_PS8622=m
 CONFIG_DRM_NOUVEAU=m
-- 
2.9.3

[PATCH v3 4/4] ARM: sunxi: Enable VGA bridge

Enable the VGA bridge used on the A13-Olinuxino in the sunxi defconfig

Signed-off-by: Maxime Ripard 
---
 arch/arm/configs/sunxi_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/sunxi_defconfig b/arch/arm/configs/sunxi_defconfig
index 714da336ec86..d830e258db59 100644
--- a/arch/arm/configs/sunxi_defconfig
+++ b/arch/arm/configs/sunxi_defconfig
@@ -98,6 +98,7 @@ CONFIG_MEDIA_RC_SUPPORT=y
 CONFIG_RC_DEVICES=y
 CONFIG_IR_SUNXI=y
 CONFIG_DRM=y
+CONFIG_DRM_RGB_TO_VGA=y
 CONFIG_DRM_SUN4I=y
 CONFIG_FB=y
 CONFIG_FB_SIMPLE=y
-- 
2.9.3

[PATCH v8 10/12] drm/i915: Update bits per component for display info

2016-09-08 Thread Kahola, Mika

Thanks for the review. I'll fix those indentations.

> -Original Message-
> From: Jim Bride [mailto:jim.bride at linux.intel.com]
> Sent: Thursday, September 8, 2016 12:27 AM
> To: Kahola, Mika 
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
> ville.syrjala at linux.intel.com; daniel.vetter at ffwll.ch
> Subject: Re: [PATCH v8 10/12] drm/i915: Update bits per component for
> display info
> 
> On Wed, Aug 17, 2016 at 01:49:47PM +0300, Mika Kahola wrote:
> > DisplayPort branch device may define max supported bits per component.
> > Update display info based on this value if bpc is defined.
> >
> > v2: cleanup to match the drm_dp_helper.c patches introduced
> > earlier in this series
> > v3: Fill bpc for connector's display info in separate
> > drm_dp_helper function (Daniel)
> > v4: remove updating bpc for display info as it may be overridden
> > when parsing EDID. Instead, check bpc for DP branch device
> > during compute_config
> >
> > Signed-off-by: Mika Kahola 
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 17 -
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index 25f459e..17110d1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1524,6 +1524,20 @@ void intel_dp_compute_rate(struct intel_dp
> *intel_dp, int port_clock,
> > }
> >  }
> >
> > +int intel_dp_compute_bpp(struct intel_dp *intel_dp,
> > +struct intel_crtc_state *pipe_config)
> 
> Indentation seems off.
> 
> > +{
> > +   int bpp, bpc;
> > +
> > +   bpp = pipe_config->pipe_bpp;
> > +   bpc = drm_dp_downstream_max_bpc(intel_dp->dpcd,
> > +intel_dp->downstream_ports);
> > +
> > +   if (bpc > 0)
> 
> Do we need to ensure that bpp is sane before this calculation as well?
drm_dp_downstream_max_bpc() routine returns 0 if we can't find bpc from DPCD. 
Therefore bpc sanity is checked so to ensure that bpp has some meaningful value 
other than 0.

> 
> > +   bpp = min(bpp, 3*bpc);
> > +
> > +   return bpp;
> > +}
> > +
> >  bool
> >  intel_dp_compute_config(struct intel_encoder *encoder,
> > struct intel_crtc_state *pipe_config)
> 
> Indentation again.
> 
> Jim
> 
> > @@ -1589,7 +1603,8 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> >
> > /* Walk through all bpp values. Luckily they're all nicely spaced with 2
> >  * bpc in between. */
> > -   bpp = pipe_config->pipe_bpp;
> > +   bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
> > +
> > if (is_edp(intel_dp)) {
> >
> > /* Get bpp from vbt only for panels that dont have bpp in
> edid */
> > --
> > 1.9.1

[PATCH v8 12/12] drm/i915: Check TMDS clock DP to HDMI dongle

On Wed, Aug 17, 2016 at 01:49:49PM +0300, Mika Kahola wrote:
> Respect max TMDS clock frequency from DPCD for active
> DP to HDMI adapters.
> 
> Signed-off-by: Mika Kahola 
> ---
>  drivers/gpu/drm/i915/intel_drv.h  |  3 +++
>  drivers/gpu/drm/i915/intel_hdmi.c | 27 +++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 1c700b0..b7fd551 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -817,6 +817,9 @@ struct intel_hdmi {
>   i915_reg_t hdmi_reg;
>   int ddc_bus;
>   struct {
> + int max_tmds_clock;
> + } dp_to_hdmi;
> + struct {
>   enum drm_dp_dual_mode_type type;
>   int max_tmds_clock;
>   } dp_dual_mode;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c 
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 4df9f38..1469d00 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1204,6 +1204,9 @@ static int hdmi_port_clock_limit(struct intel_hdmi 
> *hdmi,
>   int max_tmds_clock = intel_hdmi_source_max_tmds_clock(to_i915(dev));
>  
>   if (respect_downstream_limits) {
> + if (hdmi->dp_to_hdmi.max_tmds_clock)
> + max_tmds_clock = min(max_tmds_clock,
> +  hdmi->dp_to_hdmi.max_tmds_clock);
>   if (hdmi->dp_dual_mode.max_tmds_clock)
>   max_tmds_clock = min(max_tmds_clock,
>hdmi->dp_dual_mode.max_tmds_clock);
> @@ -1373,11 +1376,33 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
>   intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE;
>   intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
>  
> + intel_hdmi->dp_to_hdmi.max_tmds_clock = 0;
> +
>   kfree(to_intel_connector(connector)->detect_edid);
>   to_intel_connector(connector)->detect_edid = NULL;
>  }
>  
>  static void
> +intel_hdmi_dp_adapter_detect(struct drm_connector *connector)
> +{
> + struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> + struct intel_digital_port *intel_dig_port =
> + hdmi_to_dig_port(intel_hdmi);
> + struct intel_dp *intel_dp = &intel_dig_port->dp;
> + int type = intel_dp->downstream_ports[0] & DP_DS_PORT_TYPE_MASK;
> +
> + if (type != DP_DS_PORT_TYPE_HDMI)
> + return;
> +
> + intel_hdmi->dp_to_hdmi.max_tmds_clock =
> + drm_dp_downstream_max_clock(intel_dp->dpcd,
> + intel_dp->downstream_ports);

Poets driven by intel_hdmi don't have DPCD, so I don't know what this
is supposed to achieve.

> +
> + DRM_DEBUG_KMS("DP HDMI adaptor detected (max TMDS clock : %d kHz\n",
> +   intel_hdmi->dp_to_hdmi.max_tmds_clock);
> +}
> +
> +static void
>  intel_hdmi_dp_dual_mode_detect(struct drm_connector *connector, bool 
> has_edid)
>  {
>   struct drm_i915_private *dev_priv = to_i915(connector->dev);
> @@ -1438,6 +1463,8 @@ intel_hdmi_set_edid(struct drm_connector *connector, 
> bool force)
>  
>   intel_hdmi_dp_dual_mode_detect(connector, edid != NULL);
>  
> + intel_hdmi_dp_adapter_detect(connector);
> +
>   intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>   }
>  
> -- 
> 1.9.1

-- 
Ville SyrjÃ¤lÃ¤
Intel OTC

[PATCH] drm/sun4i: add missing header dependencies

On Thu, Sep 08, 2016 at 06:59:22PM +0800, Baoyou Xie wrote:
> We get 5 warnings when building kernel with W=1:
> drivers/gpu/drm/sun4i/sun4i_framebuffer.c:33:23: warning: no previous 
> prototype for 'sun4i_framebuffer_init' [-Wmissing-prototypes]
> drivers/gpu/drm/sun4i/sun4i_framebuffer.c:47:6: warning: no previous 
> prototype for 'sun4i_framebuffer_free' [-Wmissing-prototypes]
> drivers/gpu/drm/sun4i/sun4i_rgb.c:202:5: warning: no previous prototype for 
> 'sun4i_rgb_init' [-Wmissing-prototypes]
> drivers/gpu/drm/sun4i/sun4i_dotclock.c:151:5: warning: no previous prototype 
> for 'sun4i_dclk_create' [-Wmissing-prototypes]
> drivers/gpu/drm/sun4i/sun4i_dotclock.c:186:5: warning: no previous prototype 
> for 'sun4i_dclk_free' [-Wmissing-prototypes]
> 
> In fact, these functions are declared in
> drivers/gpu/drm/sun4i/sun4i_framebuffer.h,
> drivers/gpu/drm/sun4i/sun4i_rgb.h,
> drivers/gpu/drm/sun4i/sun4i_dotclock.h,
> so this patch adds missing header dependencies.
> 
> Signed-off-by: Baoyou Xie 

Applied, thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/1db356f3/attachment.sig>

[PATCH] drm/tilcdc: add missing header dependencies

2016-09-08 Thread Jyri Sarha

On 09/08/16 14:08, Baoyou Xie wrote:
> We get 4 warnings when building kernel with W=1:
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c:393:12: warning: no previous prototype 
> for 'tilcdc_tfp410_init' [-Wmissing-prototypes]
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.c:398:13: warning: no previous prototype 
> for 'tilcdc_tfp410_fini' [-Wmissing-prototypes]
> drivers/gpu/drm/tilcdc/tilcdc_panel.c:443:12: warning: no previous prototype 
> for 'tilcdc_panel_init' [-Wmissing-prototypes]
> drivers/gpu/drm/tilcdc/tilcdc_panel.c:448:13: warning: no previous prototype 
> for 'tilcdc_panel_fini' [-Wmissing-prototypes]
> 
> In fact, these functions are declared in
> drivers/gpu/drm/tilcdc/tilcdc_tfp410.h,
> drivers/gpu/drm/tilcdc/tilcdc_panel.h,
> so this patch adds missing header dependencies.
> 
> Signed-off-by: Baoyou Xie 

I'll pick this up.
Thanks,
Jyri


> ---
>  drivers/gpu/drm/tilcdc/tilcdc_panel.c  | 1 +
>  drivers/gpu/drm/tilcdc/tilcdc_tfp410.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_panel.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> index ff7774c..979394d 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_panel.c
> @@ -24,6 +24,7 @@
>  #include 
>  
>  #include "tilcdc_drv.h"
> +#include "tilcdc_panel.h"
>  
>  struct panel_module {
>   struct tilcdc_module base;
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> index 6b8c5b3..455f032 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_tfp410.c
> @@ -22,6 +22,7 @@
>  #include 
>  
>  #include "tilcdc_drv.h"
> +#include "tilcdc_tfp410.h"
>  
>  struct tfp410_module {
>   struct tilcdc_module base;
>

[PATCH] drm/tilcdc: mark symbols static where possible

2016-09-08 Thread Jyri Sarha

On 09/08/16 14:29, Baoyou Xie wrote:
> We get 3 warnings when building kernel with W=1:
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c:142:29: warning: no previous 
> prototype for 'tilcdc_get_overlay' [-Wmissing-prototypes]
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c:198:13: warning: no previous 
> prototype for 'tilcdc_convert_slave_node' [-Wmissing-prototypes]
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c:264:12: warning: no previous 
> prototype for 'tilcdc_slave_compat_init' [-Wmissing-prototypes]
> 
> In fact, these functions are only used in the file in which they are
> declared and don't need a declaration, but can be made static.
> So this patch marks these functions with 'static'.
> 
> Signed-off-by: Baoyou Xie 

I'll pick this up.
Thanks,
Jyri


> ---
>  drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c 
> b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> index f9c79da..dd8de260 100644
> --- a/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> +++ b/drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c
> @@ -139,7 +139,7 @@ static void __init tilcdc_node_disable(struct device_node 
> *node)
>   of_update_property(node, prop);
>  }
>  
> -struct device_node * __init tilcdc_get_overlay(struct kfree_table *kft)
> +static struct device_node * __init tilcdc_get_overlay(struct kfree_table 
> *kft)
>  {
>   const int size = __dtb_tilcdc_slave_compat_end -
>   __dtb_tilcdc_slave_compat_begin;
> @@ -195,7 +195,7 @@ static const char * const tilcdc_slave_props[] 
> __initconst = {
>   NULL
>  };
>  
> -void __init tilcdc_convert_slave_node(void)
> +static void __init tilcdc_convert_slave_node(void)
>  {
>   struct device_node *slave = NULL, *lcdc = NULL;
>   struct device_node *i2c = NULL, *fragment = NULL;
> @@ -261,7 +261,7 @@ out:
>   of_node_put(fragment);
>  }
>  
> -int __init tilcdc_slave_compat_init(void)
> +static int __init tilcdc_slave_compat_init(void)
>  {
>   tilcdc_convert_slave_node();
>   return 0;
>

[PATCH v8 03/12] drm: Helper to read max clock rate

On Wed, Aug 17, 2016 at 01:49:39PM +0300, Mika Kahola wrote:
> Helper routine to read out maximum supported pixel rate
> for DisplayPort legay VGA converter or TMDS clock rate
> for other digital legacy converters. The helper returns
> clock rate in kHz.
> 
> v2: Return early if detailed port cap info is not available.
> Replace if-else ladder with switch-case (Ville)
> 
> Reviewed-by: Jim Bride 
> Signed-off-by: Mika Kahola 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 33 +
>  include/drm/drm_dp_helper.h |  2 ++
>  2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 031c4d3..7497490 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -439,6 +439,39 @@ int drm_dp_link_configure(struct drm_dp_aux *aux, struct 
> drm_dp_link *link)
>  }
>  EXPORT_SYMBOL(drm_dp_link_configure);
>  
> +/**
> + * drm_dp_downstream_max_clock() - extract branch device max
> + * pixel rate for legacy VGA
> + * converter or max TMDS clock
> + * rate for others
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + *
> + * Returns max clock in kHz on success or 0 if max clock not defined
> + */
> +int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> + const u8 port_cap[4])
> +{
> + int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> + bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> + DP_DETAILED_CAP_INFO_AVAILABLE;
> +
> + if (!detailed_cap_info)
> + return 0;
> +
> + switch (type) {
> + case DP_DS_PORT_TYPE_VGA:
> + return port_cap[1] * 8 * 1000;
> + case DP_DS_PORT_TYPE_DVI:
> + case DP_DS_PORT_TYPE_HDMI:
> + case DP_DS_PORT_TYPE_DP_DUALMODE:
> + return port_cap[1] * 2500;

The spec says that if the detailed_cap_info==0, then DVI/HDMI/DP++ must
support at least 165 MHz TMDS clock. I was thinking we might want to
limit things to 165 in that case to guarantee that we advertize only
modes guaranteed to work.

> + default:
> + return 0;
> + }
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_max_clock);
> +
>  /*
>   * I2C-over-AUX implementation
>   */
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 0d84046..60dd9dc 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -815,6 +815,8 @@ int drm_dp_link_probe(struct drm_dp_aux *aux, struct 
> drm_dp_link *link);
>  int drm_dp_link_power_up(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_power_down(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
> +int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> + const u8 port_cap[4]);
>  
>  void drm_dp_aux_init(struct drm_dp_aux *aux);
>  int drm_dp_aux_register(struct drm_dp_aux *aux);
> -- 
> 1.9.1

-- 
Ville SyrjÃ¤lÃ¤
Intel OTC

[PATCH v8 04/12] drm: Helper to read max bits per component

On Wed, Aug 17, 2016 at 01:49:40PM +0300, Mika Kahola wrote:
> Helper routine to read out maximum supported bits per
> component for DisplayPort legay converters.
> 
> v2: Return early if detailed port cap info is not available.
> Replace if-else ladder with switch-case (Ville)
> 
> Reviewed-by: Jim Bride 
> Signed-off-by: Mika Kahola 
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 42 
> +
>  include/drm/drm_dp_helper.h |  2 ++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 7497490..14e8ea0 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -472,6 +472,48 @@ int drm_dp_downstream_max_clock(const u8 
> dpcd[DP_RECEIVER_CAP_SIZE],
>  }
>  EXPORT_SYMBOL(drm_dp_downstream_max_clock);
>  
> +/**
> + * drm_dp_downstream_max_bpc() - extract branch device max
> + *   bits per component
> + * @dpcd: DisplayPort configuration data
> + * @port_cap: port capabilities
> + *
> + * Returns max bpc on success or 0 if max bpc not defined
> + */
> +int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +   const u8 port_cap[4])
> +{
> + int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> + bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> + DP_DETAILED_CAP_INFO_AVAILABLE;
> + int bpc;
> +
> + if (!detailed_cap_info)
> + return 0;
> +
> + switch (type) {
> + case DP_DS_PORT_TYPE_VGA:
> + case DP_DS_PORT_TYPE_DVI:
> + case DP_DS_PORT_TYPE_HDMI:
> + case DP_DS_PORT_TYPE_DP_DUALMODE:

I think we might want return 8; for the detailed_cap_info==0 case with
these port types.

> + bpc = port_cap[2] & DP_DS_MAX_BPC_MASK;
> +
> + switch (bpc) {
> + case DP_DS_8BPC:
> + return 8;
> + case DP_DS_10BPC:
> + return 10;
> + case DP_DS_12BPC:
> + return 12;
> + case DP_DS_16BPC:
> + return 16;
> + }
> + default:
> + return 0;
> + }
> +}
> +EXPORT_SYMBOL(drm_dp_downstream_max_bpc);
> +
>  /*
>   * I2C-over-AUX implementation
>   */
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index 60dd9dc..f3d1424 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -817,6 +817,8 @@ int drm_dp_link_power_down(struct drm_dp_aux *aux, struct 
> drm_dp_link *link);
>  int drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link *link);
>  int drm_dp_downstream_max_clock(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
>   const u8 port_cap[4]);
> +int drm_dp_downstream_max_bpc(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> +   const u8 port_cap[4]);
>  
>  void drm_dp_aux_init(struct drm_dp_aux *aux);
>  int drm_dp_aux_register(struct drm_dp_aux *aux);
> -- 
> 1.9.1

-- 
Ville SyrjÃ¤lÃ¤
Intel OTC

[PATCH v8 03/12] drm: Helper to read max clock rate

2016-09-08 Thread Kahola, Mika


> -Original Message-
> From: Ville SyrjÃ¤lÃ¤ [mailto:ville.syrjala at linux.intel.com]
> Sent: Thursday, September 8, 2016 4:02 PM
> To: Kahola, Mika 
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
> jim.bride at linux.intel.com; daniel.vetter at ffwll.ch
> Subject: Re: [PATCH v8 03/12] drm: Helper to read max clock rate
> 
> On Wed, Aug 17, 2016 at 01:49:39PM +0300, Mika Kahola wrote:
> > Helper routine to read out maximum supported pixel rate for
> > DisplayPort legay VGA converter or TMDS clock rate for other digital
> > legacy converters. The helper returns clock rate in kHz.
> >
> > v2: Return early if detailed port cap info is not available.
> > Replace if-else ladder with switch-case (Ville)
> >
> > Reviewed-by: Jim Bride 
> > Signed-off-by: Mika Kahola 
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 33
> +
> >  include/drm/drm_dp_helper.h |  2 ++
> >  2 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c
> > b/drivers/gpu/drm/drm_dp_helper.c index 031c4d3..7497490 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -439,6 +439,39 @@ int drm_dp_link_configure(struct drm_dp_aux
> *aux,
> > struct drm_dp_link *link)  }  EXPORT_SYMBOL(drm_dp_link_configure);
> >
> > +/**
> > + * drm_dp_downstream_max_clock() - extract branch device max
> > + * pixel rate for legacy VGA
> > + * converter or max TMDS clock
> > + * rate for others
> > + * @dpcd: DisplayPort configuration data
> > + * @port_cap: port capabilities
> > + *
> > + * Returns max clock in kHz on success or 0 if max clock not defined
> > +*/ int drm_dp_downstream_max_clock(const u8
> > +dpcd[DP_RECEIVER_CAP_SIZE],
> > +   const u8 port_cap[4])
> > +{
> > +   int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> > +   bool detailed_cap_info = dpcd[DP_DOWNSTREAMPORT_PRESENT] &
> > +   DP_DETAILED_CAP_INFO_AVAILABLE;
> > +
> > +   if (!detailed_cap_info)
> > +   return 0;
> > +
> > +   switch (type) {
> > +   case DP_DS_PORT_TYPE_VGA:
> > +   return port_cap[1] * 8 * 1000;
> > +   case DP_DS_PORT_TYPE_DVI:
> > +   case DP_DS_PORT_TYPE_HDMI:
> > +   case DP_DS_PORT_TYPE_DP_DUALMODE:
> > +   return port_cap[1] * 2500;
> 
> The spec says that if the detailed_cap_info==0, then DVI/HDMI/DP++ must
> support at least 165 MHz TMDS clock. I was thinking we might want to limit
> things to 165 in that case to guarantee that we advertize only modes
> guaranteed to work.
That is a valid point. What I had in mind was by returning 0 from this function 
it is indicated that the value is not found. With the remaining patches that 
call this function the check is in place so we can skip the step if explicit 
clock rate is not found. 

> 
> > +   default:
> > +   return 0;
> > +   }
> > +}
> > +EXPORT_SYMBOL(drm_dp_downstream_max_clock);
> > +
> >  /*
> >   * I2C-over-AUX implementation
> >   */
> > diff --git a/include/drm/drm_dp_helper.h
> b/include/drm/drm_dp_helper.h
> > index 0d84046..60dd9dc 100644
> > --- a/include/drm/drm_dp_helper.h
> > +++ b/include/drm/drm_dp_helper.h
> > @@ -815,6 +815,8 @@ int drm_dp_link_probe(struct drm_dp_aux *aux,
> > struct drm_dp_link *link);  int drm_dp_link_power_up(struct drm_dp_aux
> > *aux, struct drm_dp_link *link);  int drm_dp_link_power_down(struct
> > drm_dp_aux *aux, struct drm_dp_link *link);  int
> > drm_dp_link_configure(struct drm_dp_aux *aux, struct drm_dp_link
> > *link);
> > +int drm_dp_downstream_max_clock(const u8
> dpcd[DP_RECEIVER_CAP_SIZE],
> > +   const u8 port_cap[4]);
> >
> >  void drm_dp_aux_init(struct drm_dp_aux *aux);  int
> > drm_dp_aux_register(struct drm_dp_aux *aux);
> > --
> > 1.9.1
> 
> --
> Ville SyrjÃ¤lÃ¤
> Intel OTC

[PATCH v6 2/4] drm: Add API for capturing frame CRCs

Hi Tomeu,

A couple of small nitpicks and a rather nasty looking bug, related to
your earlier question.

On 7 September 2016 at 11:27, Tomeu Vizoso  
wrote:

> +static ssize_t crc_control_write(struct file *file, const char __user *ubuf,
> +size_t len, loff_t *offp)
> +{

> +   if (source[len] == '\n')
> +   source[len] = '\0';
> +
Considering the bug below, I'm considering if there's a case were we
don't want to explicitly set the terminating byte ?


> +/*
> + * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, 
> space
> + * separated, with a newline at the end and null-terminated.
> + */
NULL-terminated what the things that was missing, explaining the
maths. Yet note the code sort of contradicts it.

TL;DR: above we conditionally NULL terminate the data, yet (below) we
feed garbage (always) instead of \0 byte.

> +#define LINE_LEN(values_cnt)   (10 + 11 * values_cnt + 1 + 1)
> +#define MAX_LINE_LEN   (LINE_LEN(DRM_MAX_CRC_NR))
> +
> +static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
> +size_t count, loff_t *pos)
> +{
> +   struct drm_crtc *crtc = filep->f_inode->i_private;
> +   struct drm_crtc_crc *crc = &crtc->crc;
> +   struct drm_crtc_crc_entry *entry;
> +   char buf[MAX_LINE_LEN];
Here buf is filled with garbage...


> +   if (entry->has_frame_counter)
> +   sprintf(buf, "0x%08x", entry->frame);
> +   else
> +   sprintf(buf, "XX");
> +
> +   for (i = 0; i < crc->values_cnt; i++)
> +   sprintf(buf + 10 + i * 11, " 0x%08x", entry->crcs[i]);
> +   sprintf(buf + 10 + crc->values_cnt * 11, "\n");
> +
... and now all the data is in, incl. the \n byte.

> +   if (copy_to_user(user_buf, buf, LINE_LEN(crc->values_cnt)))
And here we copy the whole thing incl. the 'should be \0 but is
actually garbage' byte.

This doesn't look good :-\

> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c

> @@ -221,6 +222,14 @@ static int drm_minor_register(struct drm_device *dev, 
> unsigned int type)
> return ret;
> }
>
> +   if (type == DRM_MINOR_PRIMARY) {
> +   drm_for_each_crtc(crtc, dev) {
> +   ret = drm_debugfs_crtc_add(crtc);
> +   if (ret)
> +   DRM_ERROR("DRM: Failed to initialize CRC 
> debugfs.\n");
> +   }
> +   }
> +
Minor: We're missing teardown in the error path.


> --- /dev/null
> +++ b/include/drm/drm_debugfs_crc.h

> +static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc)
Minor: The function is internal only and used only when
defined(CONFIG_DEBUG_FS). Thus it should stay in
drivers/gpu/drm/foo.h.

Rule of thumb: include/drm defines the API used by the drivers, while
drivers/gpu/drm/foo_internal.h the internal API between the core DRM
modules.


Regards,
Emil

[Intel-gfx] [PATCH v6 3/4] drm/i915: Use new CRC debugfs API

Hi Tomeu,

Just a couple of nitpicks. Nothing that has to be fixed or (if you
agree) cannot be done on top/later on.

On 7 September 2016 at 11:27, Tomeu Vizoso  
wrote:
> The core provides now an ABI to userspace for generation of frame CRCs,
> so implement the ->set_crc_source() callback and reuse as much code as
> possible with the previous ABI implementation.
>
> v2:
> - Leave the legacy implementation in place as the ABI implementation
>   in the core is incompatible with it.
> v3:
> - Use the "cooked" vblank counter so we have a whole 32 bits.
> - Make sure we don't mess with the state of the legacy CRC capture
>   ABI implementation.
> v4:
> - Keep use of get_vblank_counter as in the legacy code, will be
>   changed in a followup commit.
>
> v5:
> - Skip first frame or two as it's known that they contain wrong
>   data.
Even if the frames are only skipped in the new code, it doesn't
explain why one'd need it in the first place and/or how it isn't
required with the current code. Might be worth poking the original
authors and/or adding a big WARNING/NOTE/XXX/HACK to make things more
prominent.


> - A few fixes suggested by Emil Velikov.
>
> v6:
> - Rework programming of the HW registers to preserve previous
>   behavior.
>
Huge thanks for this.


> @@ -791,7 +797,7 @@ display_crc_ctl_parse_object(const char *buf, enum 
> intel_pipe_crc_object *o)
> if (!strcmp(buf, pipe_crc_objects[i])) {
> *o = i;
> return 0;
> -   }
> +   }
>
Looks like newly introduced whitespace changes, should have been part of 1/4 ?

> return -EINVAL;
>  }
> @@ -813,11 +819,16 @@ display_crc_ctl_parse_source(const char *buf, enum 
> intel_pipe_crc_source *s)
>  {
> int i;

> if (!strcmp(buf, pipe_crc_sources[i])) {
> *s = i;
> return 0;
> -   }
> +   }
>
Ditto ?

Thanks
Emil

[Intel-gfx] [PATCH v6 0/4] New debugfs API for capturing CRC of frames

On 7 September 2016 at 11:27, Tomeu Vizoso  
wrote:
> Hi,
>
> this series basically takes the facility for continuously capturing CRCs
> of frames from the i915 driver and into the DRM core.
>
> The idea is that test suites such as IGT use this information to check
> that frames that are exected to be identical, also have identical CRC
> values.
>
> Other drivers for hardware that can provide frame CRCs (including eDP
> panels that support self-refresh) can easily implement the new callback
> and provide userspace with the CRC values.
>
> Thanks,
>
> Tomeu
>
> Tomeu Vizoso (4):
>   drm/i915/debugfs: Move out pipe CRC code
>   drm: Add API for capturing frame CRCs
>   drm/i915: Use new CRC debugfs API
>   drm/i915: Put "cooked" vlank counters in frame CRC lines
>
Thanks for the nice work and addressing my suggestions Tomeu. I think
I've spotted a bug in 2/4, plus there's a couple of trivial nitpicks
in 2/4 and 3/4 - either of which can be fixed as a follow up (if I
haven't lost it of course).

With the bug trivially fixed the series is:
Reviewed-by: Emil Velikov 

-Emil

[Bug 97639] Intermittent flickering artifacts with AMD R7 260x

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

Bug ID: 97639
   Summary: Intermittent flickering artifacts with AMD R7 260x
   Product: DRI
   Version: unspecified
  Hardware: x86-64 (AMD64)
OS: Linux (All)
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/AMDgpu
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: nucrap at hotmail.com

Created attachment 126306
  --> https://bugs.freedesktop.org/attachment.cgi?id=126306&action=edit
my Xorg.0.log

Hi, I am using an MSI R7 260x OC with the new amdgpu driver. Everything seems
to run fine (I have yet to experience any crashes), however there constantly
flicker some artifacts on my screen.
In the Xorg.0.log I have noticed many suspicious error messages similar to
this:

AMDGPU(0): amdgpu_dri2_flip_event_handler: Pageflip completion event has
impossible msc 100169 < target_msc 100170

Looking at the description in the source code, it fits the bug I experience:
> Check for too small vblank count of pageflip completion, taking wraparound
> into account. This usually means some defective kms pageflip completion,
> causing wrong (msc, ust) return values and possible visual corruption.

I saw there was a similar bug (bug#91540), but it has been marked as fixed some
time ago.

My config:
Linux v4.7.2 (amdgpu, cik and powerplay enabled)
xf86-video-ati v7.7.0
mesa v12.0.1
xorg-drivers v1.17
xserver v1.17.4
KDE Plasma 5 (kwin)

If you need any further info please ask.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/488078a1/attachment.html>

[PATCH 2/2] drm/ttm: move placement structures into ttm_placement.h

From: Christian KÃ¶nig 

Makes more sense to keep that together.

Signed-off-by: Christian KÃ¶nig 
---
 include/drm/ttm/ttm_bo_api.h| 32 +---
 include/drm/ttm/ttm_placement.h | 35 +++
 2 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 97aaf5c..d73c7c2 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -45,37 +45,7 @@ struct ttm_bo_device;

 struct drm_mm_node;

-/**
- * struct ttm_place
- *
- * @fpfn:  first valid page frame number to put the object
- * @lpfn:  last valid page frame number to put the object
- * @flags: memory domain and caching flags for the object
- *
- * Structure indicating a possible place to put an object.
- */
-struct ttm_place {
-   unsignedfpfn;
-   unsignedlpfn;
-   uint32_tflags;
-};
-
-/**
- * struct ttm_placement
- *
- * @num_placement: number of preferred placements
- * @placement: preferred placements
- * @num_busy_placement:number of preferred placements when need to 
evict buffer
- * @busy_placement:preferred placements when need to evict buffer
- *
- * Structure indicating the placement you request for an object.
- */
-struct ttm_placement {
-   unsignednum_placement;
-   const struct ttm_place  *placement;
-   unsignednum_busy_placement;
-   const struct ttm_place  *busy_placement;
-};
+struct ttm_placement;

 /**
  * struct ttm_bus_placement
diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
index 20219d9..ff5195c 100644
--- a/include/drm/ttm/ttm_placement.h
+++ b/include/drm/ttm/ttm_placement.h
@@ -30,6 +30,9 @@

 #ifndef _TTM_PLACEMENT_H_
 #define _TTM_PLACEMENT_H_
+
+#include 
+
 /*
  * Memory regions for data placement.
  */
@@ -73,4 +76,36 @@

 #define TTM_PL_MASK_MEMTYPE (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)

+/**
+ * struct ttm_place
+ *
+ * @fpfn:  first valid page frame number to put the object
+ * @lpfn:  last valid page frame number to put the object
+ * @flags: memory domain and caching flags for the object
+ *
+ * Structure indicating a possible place to put an object.
+ */
+struct ttm_place {
+   unsignedfpfn;
+   unsignedlpfn;
+   uint32_tflags;
+};
+
+/**
+ * struct ttm_placement
+ *
+ * @num_placement: number of preferred placements
+ * @placement: preferred placements
+ * @num_busy_placement:number of preferred placements when need to 
evict buffer
+ * @busy_placement:preferred placements when need to evict buffer
+ *
+ * Structure indicating the placement you request for an object.
+ */
+struct ttm_placement {
+   unsignednum_placement;
+   const struct ttm_place  *placement;
+   unsignednum_busy_placement;
+   const struct ttm_place  *busy_placement;
+};
+
 #endif
-- 
2.5.0

[PATCH 1/2] drm/ttm: remove unused placement flags

From: Christian KÃ¶nig 

Either never used or not used in quite a while.

Signed-off-by: Christian KÃ¶nig 
---
 drivers/gpu/drm/ttm/ttm_bo.c|  2 +-
 include/drm/ttm/ttm_placement.h | 19 ---
 2 files changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index bc37f02..4d2e8f2 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -59,7 +59,7 @@ static inline int ttm_mem_type_from_place(const struct 
ttm_place *place,
 {
int i;

-   for (i = 0; i <= TTM_PL_PRIV5; i++)
+   for (i = 0; i <= TTM_PL_PRIV2; i++)
if (place->flags & (1 << i)) {
*mem_type = i;
return 0;
diff --git a/include/drm/ttm/ttm_placement.h b/include/drm/ttm/ttm_placement.h
index 8ed44f9..20219d9 100644
--- a/include/drm/ttm/ttm_placement.h
+++ b/include/drm/ttm/ttm_placement.h
@@ -40,10 +40,6 @@
 #define TTM_PL_PRIV03
 #define TTM_PL_PRIV14
 #define TTM_PL_PRIV25
-#define TTM_PL_PRIV36
-#define TTM_PL_PRIV47
-#define TTM_PL_PRIV58
-#define TTM_PL_SWAPPED  15

 #define TTM_PL_FLAG_SYSTEM  (1 << TTM_PL_SYSTEM)
 #define TTM_PL_FLAG_TT  (1 << TTM_PL_TT)
@@ -51,10 +47,6 @@
 #define TTM_PL_FLAG_PRIV0   (1 << TTM_PL_PRIV0)
 #define TTM_PL_FLAG_PRIV1   (1 << TTM_PL_PRIV1)
 #define TTM_PL_FLAG_PRIV2   (1 << TTM_PL_PRIV2)
-#define TTM_PL_FLAG_PRIV3   (1 << TTM_PL_PRIV3)
-#define TTM_PL_FLAG_PRIV4   (1 << TTM_PL_PRIV4)
-#define TTM_PL_FLAG_PRIV5   (1 << TTM_PL_PRIV5)
-#define TTM_PL_FLAG_SWAPPED (1 << TTM_PL_SWAPPED)
 #define TTM_PL_MASK_MEM 0x

 /*
@@ -72,7 +64,6 @@
 #define TTM_PL_FLAG_CACHED  (1 << 16)
 #define TTM_PL_FLAG_UNCACHED(1 << 17)
 #define TTM_PL_FLAG_WC  (1 << 18)
-#define TTM_PL_FLAG_SHARED  (1 << 20)
 #define TTM_PL_FLAG_NO_EVICT(1 << 21)
 #define TTM_PL_FLAG_TOPDOWN (1 << 22)

@@ -82,14 +73,4 @@

 #define TTM_PL_MASK_MEMTYPE (TTM_PL_MASK_MEM | TTM_PL_MASK_CACHING)

-/*
- * Access flags to be used for CPU- and GPU- mappings.
- * The idea is that the TTM synchronization mechanism will
- * allow concurrent READ access and exclusive write access.
- * Currently GPU- and CPU accesses are exclusive.
- */
-
-#define TTM_ACCESS_READ (1 << 0)
-#define TTM_ACCESS_WRITE(1 << 1)
-
 #endif
-- 
2.5.0

[Bug 97639] Intermittent flickering artifacts with AMD R7 260x

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

--- Comment #1 from nucrap at hotmail.com ---
Forgot one info:
xf86-video-amdgpu v1.1.0 (glamor)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/0e0363fb/attachment.html>

[PATCH v3 02/15] drm: Centralize format information

Hello Daniel and Ville,

On Thursday 09 Jun 2016 17:13:15 Ville SyrjÃ¤lÃ¤ wrote:
> On Thu, Jun 09, 2016 at 03:29:10PM +0200, Daniel Vetter wrote:
> > On Thu, Jun 09, 2016 at 04:05:11PM +0300, Ville SyrjÃ¤lÃ¤ wrote:
> >> On Thu, Jun 09, 2016 at 02:40:28PM +0200, Daniel Vetter wrote:
> >>> On Thu, Jun 09, 2016 at 03:23:17PM +0300, Ville SyrjÃ¤lÃ¤ wrote:
>  On Thu, Jun 09, 2016 at 10:52:23AM +0200, Daniel Vetter wrote:
> > On Thu, Jun 09, 2016 at 02:32:06AM +0300, Laurent Pinchart wrote:
> >> Various pieces of information about DRM formats (number of
> >> planes, color depth, chroma subsampling, ...) are scattered
> >> across different helper functions in the DRM core. Callers of
> >> those functions often need to access more than a single
> >> parameter of the format, leading to
> >> inefficiencies due to multiple lookups.
> >> 
> >> Centralize all format information in a data structure and create
> >> a function to look up information based on the format 4CC.
> >> 
> >> Signed-off-by: Laurent Pinchart
> >> 
> >> ---
> >> 
> >>  drivers/gpu/drm/drm_fourcc.c | 84 
> >>  include/drm/drm_fourcc.h | 19 ++
> >>  2 files changed, 103 insertions(+)
> >> 
> >> diff --git a/drivers/gpu/drm/drm_fourcc.c
> >> b/drivers/gpu/drm/drm_fourcc.c
> >> index 0645c85d5f95..47b9abd743be 100644
> >> --- a/drivers/gpu/drm/drm_fourcc.c
> >> +++ b/drivers/gpu/drm/drm_fourcc.c
> >> @@ -62,6 +62,90 @@ const char *drm_get_format_name(uint32_t
> >> format)
> >> 
> >>  EXPORT_SYMBOL(drm_get_format_name);
> >>  
> >>  /**
> >> + * drm_format_info - query information for a given format
> >> + * @format: pixel format (DRM_FORMAT_*)
> >> + *
> >> + * Returns:
> >> + * The instance of struct drm_format_info that describes the pixel
> >> format, or
> >> + * NULL if the format is unsupported.
> >> + */
> >> +const struct drm_format_info *drm_format_info(u32 format)
> > 
> > Bikeshed on your pixel format description table.

As much as I like a good bikeshed myself, we haven't reached a conclusion 
here, so I'll keep the current approach until someone proposes something 
better :-)

> > I think the
> > approach I've seen in gallium/mesa to describe pixel formats is a
> > lot more generic, and we might as well adopt it when we change.
> > Idea is to have a block size measure in pixels (both h and v), and
> > then cpp is bytes_per_block. This is essentially what you have
> > with hsub and vsub already, except confusing names, more ill-
> > defined (since it only makes sense for yuv) and less generic. A
> > few examples:
> 
>  I think you have your confusion backwards. Calling something a block
>  in planar formats would be more confusing. The only thing that
>  really matters is the relative position of the samples between the
>  planes. So there really is no "block" in there.
> >>> 
> >>> Atm U/V planes have a cpp of 1, which is definitely not true. There's
> >>> much less than 1 byte per visible pixel in those planes. And that's
> >>> the part that annoys me.
> >> 
> >> That's exactly as it should be. The cpp value isn't some average thing
> >> for the whole, it's per-plane.
> > 
> > On a 4x subsampled U or V plane you have 1 byte for 4 pixels. That's just
> > plain not 1 character-per-pixel, even when you just look at that plane.
> 
> OK. So let's stop calling it a pixel and call it a sample instead.
> It's 1 byte per sample, which is the only thing that should matter
> to anyone.
> 
> >>> block here is an entirely free-standing concept that just means "group
> >>> of pixels over which the bytes-per-group is counted in each group".
> >>> It's a concept stolen from gallium and makes a lot more sense when
> >>> talking about compressed formats. But I think it also makes sense when
> >>> talking about yuv formats.
> >> 
> >> For packed YUV formats the usual term I've heard is macropixel, and
> >> there it does make sense. I could live with calling it a block. So I
> >> guess eg. for 422 packed formats we'd have h_block_size=2
> >> v_block_size=1, and bytes_per_block=4.
> >> 
> >> For planar formats, each plane should be considered individually,
> >> and trying to come up with some kind of cpp value etc. for the whole
> >> thing is pointless. I think eg. with for all the NVxx formats the
> >> chroma plane should have h_block_size=2 v_block_size=1 bytes_per_block=2
> >> regardless the sub-sampling factor.
> 
> Actually meant to write h_block_size=1 here obviously. 1 sample of
> each chroma component, 2 bytes in total.
> 
> > Hm yeah NV12 is a case where 2 have 2 bytes per 2 pixel block (since
> > they're together in 1 2ndary plane), but still a subsampling of 2x. Maybe
> > we'd need both? And then perhaps define subsampling per color channel, but
> > block size and bytes-per-block per plane?
>

[PATCH v2 1/2] drm/bridge: analogix_dp: Remove duplicated code v2

2016-09-08 Thread Sean Paul

On Wed, Sep 7, 2016 at 11:48 PM, Yakir Yang  wrote:
> From: Tomeu Vizoso 
>
> Remove code for reading the EDID and DPCD fields and use the helpers
> instead.
>
> Besides the obvious code reduction, other helpers are being added to the
> core that could be used in this driver and will be good to be able to
> use them instead of duplicating them.
>
> Signed-off-by: Tomeu Vizoso 
> Cc: Javier Martinez Canillas 
> Cc: Mika Kahola 
> Cc: Yakir Yang 
> Cc: Daniel Vetter 
>
> Reviewed-by: Sean Paul 
> Reviewed-by: Yakir Yang 
> Tested-by: Javier Martinez Canillas 
> Tested-by: Sean Paul 
> ---
> Changes in v2:
> - A bunch of good fixes from Sean and Yakir
> - Moved the transfer function to analogix_dp_reg.c
> - Removed reference to the EDID from the dp struct
> - Rebase on Sean's next tree
> git://people.freedesktop.org/~seanpaul/dogwood
>


This patch has already been merged to my 'base' branch, I suppose
since no one has picked it up yet, I'll pull it in my rockchip
for-next.

Thanks,

Sean




>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 263 
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  41 +-
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 451 
> ++---
>  3 files changed, 204 insertions(+), 551 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index efac8ab..5fe3982 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -31,6 +31,7 @@
>  #include 
>
>  #include "analogix_dp_core.h"
> +#include "analogix_dp_reg.h"
>
>  #define to_dp(nm)  container_of(nm, struct analogix_dp_device, nm)
>
> @@ -174,150 +175,21 @@ static void analogix_dp_enable_sink_psr(struct 
> analogix_dp_device *dp)
> analogix_dp_enable_psr_crc(dp);
>  }
>
> -static unsigned char analogix_dp_calc_edid_check_sum(unsigned char 
> *edid_data)
> -{
> -   int i;
> -   unsigned char sum = 0;
> -
> -   for (i = 0; i < EDID_BLOCK_LENGTH; i++)
> -   sum = sum + edid_data[i];
> -
> -   return sum;
> -}
> -
> -static int analogix_dp_read_edid(struct analogix_dp_device *dp)
> -{
> -   unsigned char *edid = dp->edid;
> -   unsigned int extend_block = 0;
> -   unsigned char sum;
> -   unsigned char test_vector;
> -   int retval;
> -
> -   /*
> -* EDID device address is 0x50.
> -* However, if necessary, you must have set upper address
> -* into E-EDID in I2C device, 0x30.
> -*/
> -
> -   /* Read Extension Flag, Number of 128-byte EDID extension blocks */
> -   retval = analogix_dp_read_byte_from_i2c(dp, I2C_EDID_DEVICE_ADDR,
> -   EDID_EXTENSION_FLAG,
> -   &extend_block);
> -   if (retval)
> -   return retval;
> -
> -   if (extend_block > 0) {
> -   dev_dbg(dp->dev, "EDID data includes a single extension!\n");
> -
> -   /* Read EDID data */
> -   retval = analogix_dp_read_bytes_from_i2c(dp,
> -   I2C_EDID_DEVICE_ADDR,
> -   EDID_HEADER_PATTERN,
> -   EDID_BLOCK_LENGTH,
> -   &edid[EDID_HEADER_PATTERN]);
> -   if (retval != 0) {
> -   dev_err(dp->dev, "EDID Read failed!\n");
> -   return -EIO;
> -   }
> -   sum = analogix_dp_calc_edid_check_sum(edid);
> -   if (sum != 0) {
> -   dev_err(dp->dev, "EDID bad checksum!\n");
> -   return -EIO;
> -   }
> -
> -   /* Read additional EDID data */
> -   retval = analogix_dp_read_bytes_from_i2c(dp,
> -   I2C_EDID_DEVICE_ADDR,
> -   EDID_BLOCK_LENGTH,
> -   EDID_BLOCK_LENGTH,
> -   &edid[EDID_BLOCK_LENGTH]);
> -   if (retval != 0) {
> -   dev_err(dp->dev, "EDID Read failed!\n");
> -   return -EIO;
> -   }
> -   sum = 
> analogix_dp_calc_edid_check_sum(&edid[EDID_BLOCK_LENGTH]);
> -   if (sum != 0) {
> -   dev_err(dp->dev, "EDID bad checksum!\n");
> -   return -EIO;
> -   }
> -
> -   analogix_dp_read_byte_from_dpcd(dp, DP_TEST_REQUEST,
> -   &test_vector);
> -   if (test_vector & DP_TEST_LINK_EDID_READ) {
> -   analogix_dp_write_byte_to_dpcd(dp,
> -   DP_TEST_EDID_CHECKSUM,
> -   edid[EDID_BLOCK_LENGTH + EDID_CHECKSUM]);
> -

[PATCH v2 2/2] drm/bridge: analogix_dp: detect Sink PSR state after configuring the PSR

2016-09-08 Thread Sean Paul

On Wed, Sep 7, 2016 at 11:48 PM, Yakir Yang  wrote:
> Make sure the request PSR state could effect in analogix_dp_send_psr_spd()
> function, or printing the error Sink PSR state if we failed to effect
> the request PSR setting.
>


Let's change to:

Make sure the request PSR state takes effect in analogix_dp_send_psr_spd()
function, or print the sink PSR error state if we failed to apply the
requested PSR
setting.

> Signed-off-by: Yakir Yang 
> ---
> Changes in v2:
> - A bunch of good fixes from Sean
>
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.c |  6 ++
>  drivers/gpu/drm/bridge/analogix/analogix_dp_core.h |  4 ++--
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c  | 25 
> --
>  3 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 5fe3982..c0ce16a 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -116,8 +116,7 @@ int analogix_dp_enable_psr(struct device *dev)
> psr_vsc.DB0 = 0;
> psr_vsc.DB1 = EDP_VSC_PSR_STATE_ACTIVE | EDP_VSC_PSR_CRC_VALUES_VALID;
>
> -   analogix_dp_send_psr_spd(dp, &psr_vsc);
> -   return 0;
> +   return analogix_dp_send_psr_spd(dp, &psr_vsc);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_enable_psr);
>
> @@ -139,8 +138,7 @@ int analogix_dp_disable_psr(struct device *dev)
> psr_vsc.DB0 = 0;
> psr_vsc.DB1 = 0;
>
> -   analogix_dp_send_psr_spd(dp, &psr_vsc);
> -   return 0;
> +   return analogix_dp_send_psr_spd(dp, &psr_vsc);
>  }
>  EXPORT_SYMBOL_GPL(analogix_dp_disable_psr);
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> index a15f076..6c07a50 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.h
> @@ -247,8 +247,8 @@ void analogix_dp_config_video_slave_mode(struct 
> analogix_dp_device *dp);
>  void analogix_dp_enable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_disable_scrambling(struct analogix_dp_device *dp);
>  void analogix_dp_enable_psr_crc(struct analogix_dp_device *dp);
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc);
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +struct edp_vsc_psr *vsc);
>  ssize_t analogix_dp_transfer(struct analogix_dp_device *dp,
>  struct drm_dp_aux_msg *msg);
>  #endif /* _ANALOGIX_DP_CORE_H */
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c 
> b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index a4d17b8..09d703b 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -1004,10 +1004,12 @@ void analogix_dp_enable_psr_crc(struct 
> analogix_dp_device *dp)
> writel(PSR_VID_CRC_ENABLE, dp->reg_base + ANALOGIX_DP_CRC_CON);
>  }
>
> -void analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> - struct edp_vsc_psr *vsc)
> +int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
> +struct edp_vsc_psr *vsc)
>  {
> +   unsigned long timeout;
> unsigned int val;
> +   u8 sink;
>
> /* don't send info frame */
> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> @@ -1048,6 +1050,25 @@ void analogix_dp_send_psr_spd(struct 
> analogix_dp_device *dp,
> val = readl(dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> val |= IF_EN;
> writel(val, dp->reg_base + ANALOGIX_DP_PKT_SEND_CTL);
> +
> +   timeout = jiffies + msecs_to_jiffies(DP_TIMEOUT_LOOP_COUNT);

Mismatched units here. DP_TIMEOUT_LOOP_COUNT is defined as number of
retries, whereas you're using it as number of ms. Fortunately, the
retry number is so high that this works out :)

In a separate patch preceding this one, can you change
DP_TIMEOUT_LOOP_COUNT to DP_TIMEOUT_LOOP_MS and alter the other
timeout loops to use time_before() like this one instead of blindly
looping 100 times? After that, you can use DP_TIMEOUT_LOOP_MS here.

> +   while (time_before(jiffies, timeout)) {
> +   val = drm_dp_dpcd_readb(&dp->aux, DP_PSR_STATUS, &sink);
> +   if (val != 1) {
> +   dev_err(dp->dev, "PSR_STATUS read failed ret=%d", 
> val);
> +   return val;

Ok, since this is my snippet this comment is my fault, and I apologize
for that :). However, this could return 0. If drm_dp_dpcd_readb
returns 0, you probably want to retry (same as -EBUSY).

> +   }
> +
> +   if (vsc->DB1 && sink == DP_PSR_SINK_ACTIVE_RFB ||
> +   !vsc->DB1 && sink == DP_PSR_SINK_INACTIVE)
> +   break;
> +
> +

[Intel-gfx] [PATCH 1/1] drm/i915/dsi: silence a warning about uninitialized return value

2016-09-08 Thread Dave Gordon

On 08/09/16 00:02, Nicolas Iooss wrote:
> On 07/09/16 18:03, Dave Gordon wrote:
>> On 06/09/16 21:36, Nicolas Iooss wrote:
>>> On 06/09/16 12:21, Dave Gordon wrote:
 On 04/09/16 19:58, Nicolas Iooss wrote:
> When building the kernel with clang and some warning flags, the
> compiler
> reports that the return value of dcs_get_backlight() may be
> uninitialized:
>
> drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c:53:2: error:
> variable
> 'data' is used uninitialized whenever 'for' loop exits because its
> condition is false [-Werror,-Wsometimes-uninitialized]
> for_each_dsi_port(port, intel_dsi->dcs_backlight_ports) {
> ^~~
> drivers/gpu/drm/i915/intel_dsi.h:126:49: note: expanded from macro
> 'for_each_dsi_port'
> #define for_each_dsi_port(__port, __ports_mask)
> for_each_port_masked(__port,
> __ports_mask)
>
> ^~
> drivers/gpu/drm/i915/i915_drv.h:322:26: note: expanded from macro
> 'for_each_port_masked'
> for ((__port) = PORT_A; (__port) < I915_MAX_PORTS;
> (__port)++)  \
> ^
> drivers/gpu/drm/i915/intel_dsi_dcs_backlight.c:60:9: note:
> uninitialized use occurs here
> return data;
>^~~~
>
> As intel_dsi->dcs_backlight_ports seems to be always initialized to a
> non-null value, the content of the for loop is always executed and
> there
> is no bug in the current code. Nevertheless the compiler has no way of
> knowing that assumption, so initialize variable 'data' to silence the
> warning here.
>
> Signed-off-by: Nicolas Iooss 

 Interesting ... there are two things that could lead to this (possibly)
 incorrect analysis. Either it thinks the loop could be executed zero
 times, which would be a deficiency in the compiler, as the initialiser
 and loop bound are both known (different) constants:

 enum port {
 PORT_A = 0,
 PORT_B,
 PORT_C,
 PORT_D,
 PORT_E,
 I915_MAX_PORTS
 };

 or, it doesn't understand that because we've passed &data to another
 function, it can have been set by the callee. It may be extra confusing
 that the callee takes (void *); or it may be being ultra-sophisticated
 in its analysis and noted that in one error path data is *not* set (and
 we then discard the error and use data anyway). As an experiment, you
 could try:
>>>
>>> The code that the compiler sees is not a simple loop other enum 'port'
>>> but "for_each_dsi_port(port, intel_dsi->dcs_backlight_ports) {", which
>>> is expanded [1] to:
>>>
>>> for ((port) = PORT_A; (port) < I915_MAX_PORTS; (port)++)
>>>   if (!((intel_dsi->dcs_backlight_ports) & (1 << (port {} else {
>>>
>>> This is why I spoke of intel_dsi->dcs_backlight_ports in my description:
>>> if it is zero, the body of the loop is never run.
>>>
>>> As for the analyses of calls using &data, clang does not warn about the
>>> variable being maybe uninitialized following a call. This is quite
>>> expected as this would lead to too many false positives, even though it
>>> may miss some bugs.
>>>
 static u8 mipi_dsi_dcs_read1(struct mipi_dsi_device *dsi_device, u8 cmd)
 {
 u8 data = 0;

 mipi_dsi_dcs_read(dsi_device, cmd, &data, sizeof(data));

 return data;
 }

 static u32 dcs_get_backlight(struct intel_connector *connector)
 {
 struct intel_encoder *encoder = connector->encoder;
 struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
 struct mipi_dsi_device *dsi_device;
 enum port port;
 u8 data;

 /* FIXME: Need to take care of 16 bit brightness level */
 for_each_dsi_port(port, intel_dsi->dcs_backlight_ports) {
 dsi_device = intel_dsi->dsi_hosts[port]->device;
 data = mipi_dsi_dcs_read1(dsi_device,
 MIPI_DCS_GET_DISPLAY_BRIGHTNESS);
 break;
 }

 return data;
 }

 If it complains about that then it's a shortcoming in the loop analysis.
>>>
>>> It complains (in dcs_get_backlight), because for_each_dsi_port() still
>>> hides an 'if' condition.
>>
>> So it does, In that case the complaint is really quite reasonable.
>>
 If not you could try:

 static u8 mipi_dsi_dcs_read1(struct mipi_dsi_device *dsi_device, u8 cmd)
 {
 u8 data;
 ssize_t nbytes = sizeof(data);

 nbytes = mipi_dsi_dcs_read(dsi_device, cmd, &data, nbytes);
 return nbytes == sizeof(data) ? data : 0;
 }

 and if complai

[PATCH v4 00/14] Centralize format information

Hello,

Various pieces of information about DRM formats (number of planes, color
depth, chroma subsampling, ...) are scattered across different helper
functions in the DRM core. Callers of those functions often need to access
more than a single parameter of the format, leading to inefficiencies due to
multiple lookups.

This patch series addresses this issue by centralizing all format information
in a single data structure (01/14). It reimplements the existing format helper
functions based on that structure (02/14) and converts the DRM core code to
use the new structure (03/14). The DRM core now WARNs when a driver tries to
query information about an unsupported format (04/14).

The second part of the patch series removes the drm_fb_get_bpp_depth() legacy
function that shouldn't be used directly by drivers. It modifies all its users
to use the appropriate API instead (05/14 to 13/14) and finally merges the
function into its only caller in the DRM core (14/14).

The new API is also useful for drivers as shown by the "[PATCH v2 00/20] OMAP
DRM fixes and improvements" patch series previously posted (and which I have
rebased and will repost soon).

Changes since v3:

- Rebased on top of latest drm/master branch
- Collected acks
- Dropped "drm: Move format-related helpers to drm_fourcc.c" and
  "drm/msm: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()" that
  have been merged already
- Added new "drm/arm: mali-dp: Replace drm_fb_get_bpp_depth() with
  drm_format_plane_cpp()" patch
- Coding style fixes and variable renames

Changes since v2:

- Remove bpp field from drm_format_info structure
- Replace all users of drm_fb_get_bpp_depth() with the appropriate API
- Merge drm_fb_get_bpp_depth() into its only caller

Changes since v1:

- Move format-related helpers to drm_fourcc.c
- Use named initializers for the formats array
- WARN when calling drm_format_info() for an unsupported format
- Don't drop the drm_format_plane_width() and drm_format_plane_height()
  helpers

Laurent Pinchart (14):
  drm: Centralize format information
  drm: Implement the drm_format_*() helpers as drm_format_info()
wrappers
  drm: Use drm_format_info() in DRM core code
  drm: WARN when calling drm_format_info() for an unsupported format
  drm: sti: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()
  drm: hdlcd: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()
  drm: tilcdc: Replace drm_fb_get_bpp_depth() with
drm_format_plane_cpp()
  drm: cirrus: Replace drm_fb_get_bpp_depth() with
drm_format_plane_cpp()
  drm: gma500: Replace drm_fb_get_bpp_depth() with drm_format_info()
  drm: amdgpu: Replace drm_fb_get_bpp_depth() with
drm_format_plane_cpp()
  drm: radeon: Replace drm_fb_get_bpp_depth() with
drm_format_plane_cpp()
  drm: vmwgfx: Replace drm_fb_get_bpp_depth() with drm_format_info()
  drm/arm: mali-dp: Replace drm_fb_get_bpp_depth() with
drm_format_plane_cpp()
  drm: Don't export the drm_fb_get_bpp_depth() function

 Documentation/gpu/drm-kms.rst   |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  |  14 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |   3 +-
 drivers/gpu/drm/arm/hdlcd_crtc.c|   5 +-
 drivers/gpu/drm/arm/malidp_hw.c |   7 +-
 drivers/gpu/drm/cirrus/cirrus_fbdev.c   |   6 +-
 drivers/gpu/drm/cirrus/cirrus_main.c|   4 +-
 drivers/gpu/drm/drm_fb_cma_helper.c |  23 +--
 drivers/gpu/drm/drm_fourcc.c| 281 ++--
 drivers/gpu/drm/drm_framebuffer.c   | 102 ++--
 drivers/gpu/drm/drm_modeset_helper.c|  17 +-
 drivers/gpu/drm/gma500/framebuffer.c|  20 +--
 drivers/gpu/drm/radeon/radeon_fb.c  |  20 +--
 drivers/gpu/drm/radeon/radeon_gem.c |   3 +-
 drivers/gpu/drm/sti/sti_gdp.c   |   6 +-
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c|  15 +-
 drivers/gpu/drm/tilcdc/tilcdc_plane.c   |   7 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |  12 +-
 include/drm/drm_fourcc.h|  21 ++-
 19 files changed, 242 insertions(+), 327 deletions(-)

-- 
Regards,

Laurent Pinchart

[PATCH v4 01/14] drm: Centralize format information

Various pieces of information about DRM formats (number of planes, color
depth, chroma subsampling, ...) are scattered across different helper
functions in the DRM core. Callers of those functions often need to
access more than a single parameter of the format, leading to
inefficiencies due to multiple lookups.

Centralize all format information in a data structure and create a
function to look up information based on the format 4CC.

Signed-off-by: Laurent Pinchart 
---
 Documentation/gpu/drm-kms.rst |  3 ++
 drivers/gpu/drm/drm_fourcc.c  | 84 +++
 include/drm/drm_fourcc.h  | 19 ++
 3 files changed, 106 insertions(+)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index f9a991bb87d4..85c4c49f4436 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -63,6 +63,9 @@ Frame Buffer Functions Reference
 DRM Format Handling
 ===

+.. kernel-doc:: include/drm/drm_fourcc.h
+   :internal:
+
 .. kernel-doc:: drivers/gpu/drm/drm_fourcc.c
:export:

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 29c56b4331e0..6b91bd8a510d 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -103,6 +103,90 @@ char *drm_get_format_name(uint32_t format)
 EXPORT_SYMBOL(drm_get_format_name);

 /**
+ * drm_format_info - query information for a given format
+ * @format: pixel format (DRM_FORMAT_*)
+ *
+ * Returns:
+ * The instance of struct drm_format_info that describes the pixel format, or
+ * NULL if the format is unsupported.
+ */
+const struct drm_format_info *drm_format_info(u32 format)
+{
+   static const struct drm_format_info formats[] = {
+   { .format = DRM_FORMAT_C8,  .depth = 8,  
.num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_RGB332,  .depth = 8,  
.num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_BGR233,  .depth = 8,  
.num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_XRGB,.depth = 12, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_XBGR,.depth = 12, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_RGBX,.depth = 12, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_BGRX,.depth = 12, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_ARGB,.depth = 12, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_ABGR,.depth = 12, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_RGBA,.depth = 12, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_BGRA,.depth = 12, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_XRGB1555,.depth = 15, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_XBGR1555,.depth = 15, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_RGBX5551,.depth = 15, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_BGRX5551,.depth = 15, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_ARGB1555,.depth = 15, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_ABGR1555,.depth = 15, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_RGBA5551,.depth = 15, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_BGRA5551,.depth = 15, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_RGB565,  .depth = 16, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_BGR565,  .depth = 16, 
.num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_RGB888,  .depth = 24, 
.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_BGR888,  .depth = 24, 
.num_planes = 1, .cpp = { 3, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_XRGB,.depth = 24, 
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1 },
+   { .format = DRM_FORMAT_XBGR,.depth = 24, 
.num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub =

[PATCH v4 02/14] drm: Implement the drm_format_*() helpers as drm_format_info() wrappers

Turn the drm_format_*() helpers into wrappers around the drm_format_info
lookup function to centralize all format information in a single place.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/drm_fourcc.c | 186 +--
 1 file changed, 37 insertions(+), 149 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 6b91bd8a510d..bf91c5044d84 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -198,69 +198,22 @@ EXPORT_SYMBOL(drm_format_info);
 void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
  int *bpp)
 {
-   char *format_name;
-
-   switch (format) {
-   case DRM_FORMAT_C8:
-   case DRM_FORMAT_RGB332:
-   case DRM_FORMAT_BGR233:
-   *depth = 8;
-   *bpp = 8;
-   break;
-   case DRM_FORMAT_XRGB1555:
-   case DRM_FORMAT_XBGR1555:
-   case DRM_FORMAT_RGBX5551:
-   case DRM_FORMAT_BGRX5551:
-   case DRM_FORMAT_ARGB1555:
-   case DRM_FORMAT_ABGR1555:
-   case DRM_FORMAT_RGBA5551:
-   case DRM_FORMAT_BGRA5551:
-   *depth = 15;
-   *bpp = 16;
-   break;
-   case DRM_FORMAT_RGB565:
-   case DRM_FORMAT_BGR565:
-   *depth = 16;
-   *bpp = 16;
-   break;
-   case DRM_FORMAT_RGB888:
-   case DRM_FORMAT_BGR888:
-   *depth = 24;
-   *bpp = 24;
-   break;
-   case DRM_FORMAT_XRGB:
-   case DRM_FORMAT_XBGR:
-   case DRM_FORMAT_RGBX:
-   case DRM_FORMAT_BGRX:
-   *depth = 24;
-   *bpp = 32;
-   break;
-   case DRM_FORMAT_XRGB2101010:
-   case DRM_FORMAT_XBGR2101010:
-   case DRM_FORMAT_RGBX1010102:
-   case DRM_FORMAT_BGRX1010102:
-   case DRM_FORMAT_ARGB2101010:
-   case DRM_FORMAT_ABGR2101010:
-   case DRM_FORMAT_RGBA1010102:
-   case DRM_FORMAT_BGRA1010102:
-   *depth = 30;
-   *bpp = 32;
-   break;
-   case DRM_FORMAT_ARGB:
-   case DRM_FORMAT_ABGR:
-   case DRM_FORMAT_RGBA:
-   case DRM_FORMAT_BGRA:
-   *depth = 32;
-   *bpp = 32;
-   break;
-   default:
-   format_name = drm_get_format_name(format);
+   const struct drm_format_info *info;
+
+   info = drm_format_info(format);
+   if (!info || !info->depth) {
+   char *format_name = drm_get_format_name(format);
+
DRM_DEBUG_KMS("unsupported pixel format %s\n", format_name);
kfree(format_name);
+
*depth = 0;
*bpp = 0;
-   break;
+   return;
}
+
+   *depth = info->depth;
+   *bpp = info->cpp[0] << 3;
 }
 EXPORT_SYMBOL(drm_fb_get_bpp_depth);

@@ -273,28 +226,10 @@ EXPORT_SYMBOL(drm_fb_get_bpp_depth);
  */
 int drm_format_num_planes(uint32_t format)
 {
-   switch (format) {
-   case DRM_FORMAT_YUV410:
-   case DRM_FORMAT_YVU410:
-   case DRM_FORMAT_YUV411:
-   case DRM_FORMAT_YVU411:
-   case DRM_FORMAT_YUV420:
-   case DRM_FORMAT_YVU420:
-   case DRM_FORMAT_YUV422:
-   case DRM_FORMAT_YVU422:
-   case DRM_FORMAT_YUV444:
-   case DRM_FORMAT_YVU444:
-   return 3;
-   case DRM_FORMAT_NV12:
-   case DRM_FORMAT_NV21:
-   case DRM_FORMAT_NV16:
-   case DRM_FORMAT_NV61:
-   case DRM_FORMAT_NV24:
-   case DRM_FORMAT_NV42:
-   return 2;
-   default:
-   return 1;
-   }
+   const struct drm_format_info *info;
+
+   info = drm_format_info(format);
+   return info ? info->num_planes : 1;
 }
 EXPORT_SYMBOL(drm_format_num_planes);

@@ -308,40 +243,13 @@ EXPORT_SYMBOL(drm_format_num_planes);
  */
 int drm_format_plane_cpp(uint32_t format, int plane)
 {
-   unsigned int depth;
-   int bpp;
+   const struct drm_format_info *info;

-   if (plane >= drm_format_num_planes(format))
+   info = drm_format_info(format);
+   if (!info || plane >= info->num_planes)
return 0;

-   switch (format) {
-   case DRM_FORMAT_YUYV:
-   case DRM_FORMAT_YVYU:
-   case DRM_FORMAT_UYVY:
-   case DRM_FORMAT_VYUY:
-   return 2;
-   case DRM_FORMAT_NV12:
-   case DRM_FORMAT_NV21:
-   case DRM_FORMAT_NV16:
-   case DRM_FORMAT_NV61:
-   case DRM_FORMAT_NV24:
-   case DRM_FORMAT_NV42:
-   return plane ? 2 : 1;
-   case DRM_FORMAT_YUV410:
-   case DRM_FORMAT_YVU410:
-   case DRM_FORMAT_YUV411:
-   case DRM_FORMAT_YVU411:
-   case DRM_FORMAT_YUV420:
-   case DRM_FORMAT_YVU420:
-   case DRM_FORMAT_YUV422:
-   case DRM_FORMAT_YVU422:
-   case DRM_FORMAT_YUV444:
-   case DRM_FORMAT_YVU444:
-   return 1;
-   default:
-   drm_fb_get_bp

[PATCH v4 04/14] drm: WARN when calling drm_format_info() for an unsupported format

The format helpers have historically treated unsupported formats as part
of the default case, returning values that are likely wrong. We can't
change this behaviour now without risking breaking drivers in difficult
to detect ways, but we can WARN on unsupported formats to catch faulty
callers.

The only exception is the framebuffer_check() function that calls
drm_format_info() to validate the format passed from userspace. This is
a valid use case that shouldn't generate a warning.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/drm_fourcc.c  | 32 
 drivers/gpu/drm/drm_framebuffer.c |  2 +-
 include/drm/drm_fourcc.h  |  1 +
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index bf91c5044d84..0355b7ede8e4 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -102,15 +102,11 @@ char *drm_get_format_name(uint32_t format)
 }
 EXPORT_SYMBOL(drm_get_format_name);

-/**
- * drm_format_info - query information for a given format
- * @format: pixel format (DRM_FORMAT_*)
- *
- * Returns:
- * The instance of struct drm_format_info that describes the pixel format, or
- * NULL if the format is unsupported.
+/*
+ * Internal function to query information for a given format. See
+ * drm_format_info() for the public API.
  */
-const struct drm_format_info *drm_format_info(u32 format)
+const struct drm_format_info *__drm_format_info(u32 format)
 {
static const struct drm_format_info formats[] = {
{ .format = DRM_FORMAT_C8,  .depth = 8,  
.num_planes = 1, .cpp = { 1, 0, 0 }, .hsub = 1, .vsub = 1 },
@@ -184,6 +180,26 @@ const struct drm_format_info *drm_format_info(u32 format)

return NULL;
 }
+
+/**
+ * drm_format_info - query information for a given format
+ * @format: pixel format (DRM_FORMAT_*)
+ *
+ * The caller should only pass a supported pixel format to this function.
+ * Unsupported pixel formats will generate a warning in the kernel log.
+ *
+ * Returns:
+ * The instance of struct drm_format_info that describes the pixel format, or
+ * NULL if the format is unsupported.
+ */
+const struct drm_format_info *drm_format_info(u32 format)
+{
+   const struct drm_format_info *info;
+
+   info = __drm_format_info(format);
+   WARN_ON(!info);
+   return info;
+}
 EXPORT_SYMBOL(drm_format_info);

 /**
diff --git a/drivers/gpu/drm/drm_framebuffer.c 
b/drivers/gpu/drm/drm_framebuffer.c
index ce8045228642..da29bdac2009 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -105,7 +105,7 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 
*r)
const struct drm_format_info *info;
int i;

-   info = drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
+   info = __drm_format_info(r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN);
if (!info) {
char *format_name = drm_get_format_name(r->pixel_format);
DRM_DEBUG_KMS("bad framebuffer format %s\n", format_name);
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 4e79d6159856..7ffb114d2468 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -43,6 +43,7 @@ struct drm_format_info {
u8 vsub;
 };

+const struct drm_format_info *__drm_format_info(u32 format);
 const struct drm_format_info *drm_format_info(u32 format);
 uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
 void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
-- 
Regards,

Laurent Pinchart

[PATCH v4 05/14] drm: sti: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

The driver needs the number of bytes per pixel, not the bpp and depth
info meant for fbdev compatibility. Use the right API.

Signed-off-by: Laurent Pinchart 
Acked-by: Vincent Abriou 
---
 drivers/gpu/drm/sti/sti_gdp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Cc: Benjamin Gaignard 
Cc: Vincent Abriou 

diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c
index b8d942ca45e8..3fc62c1ea9c2 100644
--- a/drivers/gpu/drm/sti/sti_gdp.c
+++ b/drivers/gpu/drm/sti/sti_gdp.c
@@ -719,7 +719,7 @@ static void sti_gdp_atomic_update(struct drm_plane 
*drm_plane,
u32 dma_updated_top;
u32 dma_updated_btm;
int format;
-   unsigned int depth, bpp;
+   unsigned int bpp;
u32 ydo, xdo, yds, xds;

if (!crtc || !fb)
@@ -758,9 +758,9 @@ static void sti_gdp_atomic_update(struct drm_plane 
*drm_plane,
 (unsigned long)cma_obj->paddr);

/* pixel memory location */
-   drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
+   bpp = drm_format_plane_cpp(fb->pixel_format, 0);
top_field->gam_gdp_pml = (u32)cma_obj->paddr + fb->offsets[0];
-   top_field->gam_gdp_pml += src_x * (bpp >> 3);
+   top_field->gam_gdp_pml += src_x * bpp;
top_field->gam_gdp_pml += src_y * fb->pitches[0];

/* output parameters (clamped / cropped) */
-- 
Regards,

Laurent Pinchart

[PATCH v4 03/14] drm: Use drm_format_info() in DRM core code

Replace calls to the drm_format_*() helper functions with direct use of
the drm_format_info structure. This improves efficiency by removing
duplicate lookups.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/drm_fb_cma_helper.c |  23 
 drivers/gpu/drm/drm_framebuffer.c   | 102 +---
 2 files changed, 25 insertions(+), 100 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_cma_helper.c 
b/drivers/gpu/drm/drm_fb_cma_helper.c
index 1fd6eac1400c..fac4f06f8485 100644
--- a/drivers/gpu/drm/drm_fb_cma_helper.c
+++ b/drivers/gpu/drm/drm_fb_cma_helper.c
@@ -176,20 +176,20 @@ struct drm_framebuffer 
*drm_fb_cma_create_with_funcs(struct drm_device *dev,
struct drm_file *file_priv, const struct drm_mode_fb_cmd2 *mode_cmd,
const struct drm_framebuffer_funcs *funcs)
 {
+   const struct drm_format_info *info;
struct drm_fb_cma *fb_cma;
struct drm_gem_cma_object *objs[4];
struct drm_gem_object *obj;
-   unsigned int hsub;
-   unsigned int vsub;
int ret;
int i;

-   hsub = drm_format_horz_chroma_subsampling(mode_cmd->pixel_format);
-   vsub = drm_format_vert_chroma_subsampling(mode_cmd->pixel_format);
+   info = drm_format_info(mode_cmd->pixel_format);
+   if (!info)
+   return ERR_PTR(-EINVAL);

-   for (i = 0; i < drm_format_num_planes(mode_cmd->pixel_format); i++) {
-   unsigned int width = mode_cmd->width / (i ? hsub : 1);
-   unsigned int height = mode_cmd->height / (i ? vsub : 1);
+   for (i = 0; i < info->num_planes; i++) {
+   unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
+   unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
unsigned int min_size;

obj = drm_gem_object_lookup(file_priv, mode_cmd->handles[i]);
@@ -200,7 +200,7 @@ struct drm_framebuffer *drm_fb_cma_create_with_funcs(struct 
drm_device *dev,
}

min_size = (height - 1) * mode_cmd->pitches[i]
-+ width * drm_format_plane_cpp(mode_cmd->pixel_format, 
i)
++ width * info->cpp[i]
 + mode_cmd->offsets[i];

if (obj->size < min_size) {
@@ -269,12 +269,15 @@ EXPORT_SYMBOL_GPL(drm_fb_cma_get_gem_obj);
 static void drm_fb_cma_describe(struct drm_framebuffer *fb, struct seq_file *m)
 {
struct drm_fb_cma *fb_cma = to_fb_cma(fb);
-   int i, n = drm_format_num_planes(fb->pixel_format);
+   const struct drm_format_info *info;
+   int i;

seq_printf(m, "fb: %dx%d@%4.4s\n", fb->width, fb->height,
(char *)&fb->pixel_format);

-   for (i = 0; i < n; i++) {
+   info = drm_format_info(fb->pixel_format);
+
+   for (i = 0; i < info->num_planes; i++) {
seq_printf(m, "   %d: offset=%d pitch=%d, obj: ",
i, fb->offsets[i], fb->pitches[i]);
drm_gem_cma_describe(fb_cma->obj[i], m);
diff --git a/drivers/gpu/drm/drm_framebuffer.c 
b/drivers/gpu/drm/drm_framebuffer.c
index 30dc01e6eb5d..ce8045228642 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -100,111 +100,33 @@ int drm_mode_addfb(struct drm_device *dev,
return 0;
 }

-static int format_check(const struct drm_mode_fb_cmd2 *r)
-{
-   uint32_t format = r->pixel_format & ~DRM_FORMAT_BIG_ENDIAN;
-   char *format_name;
-
-   switch (format) {
-   case DRM_FORMAT_C8:
-   case DRM_FORMAT_RGB332:
-   case DRM_FORMAT_BGR233:
-   case DRM_FORMAT_XRGB:
-   case DRM_FORMAT_XBGR:
-   case DRM_FORMAT_RGBX:
-   case DRM_FORMAT_BGRX:
-   case DRM_FORMAT_ARGB:
-   case DRM_FORMAT_ABGR:
-   case DRM_FORMAT_RGBA:
-   case DRM_FORMAT_BGRA:
-   case DRM_FORMAT_XRGB1555:
-   case DRM_FORMAT_XBGR1555:
-   case DRM_FORMAT_RGBX5551:
-   case DRM_FORMAT_BGRX5551:
-   case DRM_FORMAT_ARGB1555:
-   case DRM_FORMAT_ABGR1555:
-   case DRM_FORMAT_RGBA5551:
-   case DRM_FORMAT_BGRA5551:
-   case DRM_FORMAT_RGB565:
-   case DRM_FORMAT_BGR565:
-   case DRM_FORMAT_RGB888:
-   case DRM_FORMAT_BGR888:
-   case DRM_FORMAT_XRGB:
-   case DRM_FORMAT_XBGR:
-   case DRM_FORMAT_RGBX:
-   case DRM_FORMAT_BGRX:
-   case DRM_FORMAT_ARGB:
-   case DRM_FORMAT_ABGR:
-   case DRM_FORMAT_RGBA:
-   case DRM_FORMAT_BGRA:
-   case DRM_FORMAT_XRGB2101010:
-   case DRM_FORMAT_XBGR2101010:
-   case DRM_FORMAT_RGBX1010102:
-   case DRM_FORMAT_BGRX1010102:
-   case DRM_FORMAT_ARGB2101010:
-   case DRM_FORMAT_ABGR2101010:
-   case DRM_FORMAT_RGBA1010102:
-   case DRM_FORMAT_BGRA1010102:
-   case DRM_FORMAT_YUYV:
-   case DRM_FORMAT_YVYU:
-   case DRM_FORMAT_UYVY:
-   case DRM_FORMAT_VYUY:
-   case DR

[PATCH v4 06/14] drm: hdlcd: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

The driver needs the number of bytes per pixel, not the bpp and depth
info meant for fbdev compatibility. Use the right API.

Signed-off-by: Laurent Pinchart 
Acked-by: Liviu Dudau 
---
 drivers/gpu/drm/arm/hdlcd_crtc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Cc: Liviu Dudau 

diff --git a/drivers/gpu/drm/arm/hdlcd_crtc.c b/drivers/gpu/drm/arm/hdlcd_crtc.c
index 48019ae22ddb..bbaa55add2d2 100644
--- a/drivers/gpu/drm/arm/hdlcd_crtc.c
+++ b/drivers/gpu/drm/arm/hdlcd_crtc.c
@@ -223,14 +223,12 @@ static void hdlcd_plane_atomic_update(struct drm_plane 
*plane,
 {
struct hdlcd_drm_private *hdlcd;
struct drm_gem_cma_object *gem;
-   unsigned int depth, bpp;
u32 src_w, src_h, dest_w, dest_h;
dma_addr_t scanout_start;

if (!plane->state->fb)
return;

-   drm_fb_get_bpp_depth(plane->state->fb->pixel_format, &depth, &bpp);
src_w = plane->state->src_w >> 16;
src_h = plane->state->src_h >> 16;
dest_w = plane->state->crtc_w;
@@ -238,7 +236,8 @@ static void hdlcd_plane_atomic_update(struct drm_plane 
*plane,
gem = drm_fb_cma_get_gem_obj(plane->state->fb, 0);
scanout_start = gem->paddr + plane->state->fb->offsets[0] +
plane->state->crtc_y * plane->state->fb->pitches[0] +
-   plane->state->crtc_x * bpp / 8;
+   plane->state->crtc_x *
+   drm_format_plane_cpp(plane->state->fb->pixel_format, 0);

hdlcd = plane->dev->dev_private;
hdlcd_write(hdlcd, HDLCD_REG_FB_LINE_LENGTH, 
plane->state->fb->pitches[0]);
-- 
Regards,

Laurent Pinchart

[PATCH v4 07/14] drm: tilcdc: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

The driver needs the number of bytes per pixel, not the bpp and depth
info meant for fbdev compatibility. Use the right API.

In the tilcdc_crtc_mode_set() function compute the hardware register
value directly from the pixel format instead of computing the number of
bits per pixels first.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Tomi Valkeinen 
---
Changes since v3:

- Removed DRM_FORMAT_ARGB support
- Fixed coding style
- Renamed min_pitch to pitch
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c  | 15 +--
 drivers/gpu/drm/tilcdc/tilcdc_plane.c |  7 ---
 2 files changed, 9 insertions(+), 13 deletions(-)

Cc: Jyri Sarha 

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c 
b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 25d6b220ee8a..a64718630cdb 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -67,15 +67,13 @@ static void set_scanout(struct drm_crtc *crtc, struct 
drm_framebuffer *fb)
struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
struct drm_device *dev = crtc->dev;
struct drm_gem_cma_object *gem;
-   unsigned int depth, bpp;
dma_addr_t start, end;

-   drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
gem = drm_fb_cma_get_gem_obj(fb, 0);

start = gem->paddr + fb->offsets[0] +
crtc->y * fb->pitches[0] +
-   crtc->x * bpp / 8;
+   crtc->x * drm_format_plane_cpp(fb->pixel_format, 0);

end = start + (crtc->mode.vdisplay * fb->pitches[0]);

@@ -404,16 +402,13 @@ static void tilcdc_crtc_mode_set_nofb(struct drm_crtc 
*crtc)
if (info->tft_alt_mode)
reg |= LCDC_TFT_ALT_ENABLE;
if (priv->rev == 2) {
-   unsigned int depth, bpp;
-
-   drm_fb_get_bpp_depth(fb->pixel_format, &depth, &bpp);
-   switch (bpp) {
-   case 16:
+   switch (fb->pixel_format) {
+   case DRM_FORMAT_RGB565:
break;
-   case 32:
+   case DRM_FORMAT_XRGB:
reg |= LCDC_V2_TFT_24BPP_UNPACK;
/* fallthrough */
-   case 24:
+   case DRM_FORMAT_RGB888:
reg |= LCDC_V2_TFT_24BPP_MODE;
break;
default:
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_plane.c 
b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
index 41911e3110e8..604074089112 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_plane.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_plane.c
@@ -43,7 +43,7 @@ static int tilcdc_plane_atomic_check(struct drm_plane *plane,
 {
struct drm_crtc_state *crtc_state;
struct drm_plane_state *old_state = plane->state;
-   unsigned int depth, bpp;
+   unsigned int pitch;

if (!state->crtc)
return 0;
@@ -72,8 +72,9 @@ static int tilcdc_plane_atomic_check(struct drm_plane *plane,
return -EINVAL;
}

-   drm_fb_get_bpp_depth(state->fb->pixel_format, &depth, &bpp);
-   if (state->fb->pitches[0] != crtc_state->mode.hdisplay * bpp / 8) {
+   pitch = crtc_state->mode.hdisplay *
+   drm_format_plane_cpp(state->fb->pixel_format, 0);
+   if (state->fb->pitches[0] != pitch) {
dev_err(plane->dev->dev,
"Invalid pitch: fb and crtc widths must be the same");
return -EINVAL;
-- 
Regards,

Laurent Pinchart

[PATCH v4 08/14] drm: cirrus: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

The driver doesn't need the color depth, only the number of bits per
pixel. Use the right API.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/cirrus/cirrus_fbdev.c | 6 +++---
 drivers/gpu/drm/cirrus/cirrus_main.c  | 4 ++--
 2 files changed, 5 insertions(+), 5 deletions(-)

Cc: Dave Airlie 

diff --git a/drivers/gpu/drm/cirrus/cirrus_fbdev.c 
b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
index daecf1ad76a4..3a6309d7d8e4 100644
--- a/drivers/gpu/drm/cirrus/cirrus_fbdev.c
+++ b/drivers/gpu/drm/cirrus/cirrus_fbdev.c
@@ -138,12 +138,12 @@ static int cirrusfb_create_object(struct cirrus_fbdev 
*afbdev,
 {
struct drm_device *dev = afbdev->helper.dev;
struct cirrus_device *cdev = dev->dev_private;
-   u32 bpp, depth;
+   u32 bpp;
u32 size;
struct drm_gem_object *gobj;
-
int ret = 0;
-   drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+
+   bpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0) * 8;

if (!cirrus_check_framebuffer(cdev, mode_cmd->width, mode_cmd->height,
  bpp, mode_cmd->pitches[0]))
diff --git a/drivers/gpu/drm/cirrus/cirrus_main.c 
b/drivers/gpu/drm/cirrus/cirrus_main.c
index 76bcb43e7c06..2c3c0d4072ce 100644
--- a/drivers/gpu/drm/cirrus/cirrus_main.c
+++ b/drivers/gpu/drm/cirrus/cirrus_main.c
@@ -52,10 +52,10 @@ cirrus_user_framebuffer_create(struct drm_device *dev,
struct cirrus_device *cdev = dev->dev_private;
struct drm_gem_object *obj;
struct cirrus_framebuffer *cirrus_fb;
+   u32 bpp;
int ret;
-   u32 bpp, depth;

-   drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+   bpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0) * 8;

if (!cirrus_check_framebuffer(cdev, mode_cmd->width, mode_cmd->height,
  bpp, mode_cmd->pitches[0]))
-- 
Regards,

Laurent Pinchart

[PATCH v4 09/14] drm: gma500: Replace drm_fb_get_bpp_depth() with drm_format_info()

The driver uses drm_fb_get_bpp_depth() to check whether it can support
the format requested by userspace when creating a framebuffer. This
isn't the right API, as it doesn't differentiate between RGB formats
other than on a depth and bpp basis.

Fixing this requires non trivial changes to the drivers internals. As a
first step, replace usage of the drm_fb_get_bpp_depth() function with an
equivalent check based on drm_format_info(). This is part of a wider
effort to remove usage of the drm_fb_get_bpp_depth() function in
drivers.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/gma500/framebuffer.c | 20 +---
 1 file changed, 9 insertions(+), 11 deletions(-)

Cc: Patrik Jakobsson 

diff --git a/drivers/gpu/drm/gma500/framebuffer.c 
b/drivers/gpu/drm/gma500/framebuffer.c
index 3a44e705db53..6cb92cc0bef8 100644
--- a/drivers/gpu/drm/gma500/framebuffer.c
+++ b/drivers/gpu/drm/gma500/framebuffer.c
@@ -236,22 +236,20 @@ static int psb_framebuffer_init(struct drm_device *dev,
const struct drm_mode_fb_cmd2 *mode_cmd,
struct gtt_range *gt)
 {
-   u32 bpp, depth;
+   const struct drm_format_info *info;
int ret;

-   drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+   /*
+* Reject unknown formats, YUV formats, and formats with more than
+* 4 bytes per pixel.
+*/
+   info = drm_format_info(mode_cmd->pixel_format);
+   if (!info || !info->depth || info->cpp[0] > 4)
+   return -EINVAL;

if (mode_cmd->pitches[0] & 63)
return -EINVAL;
-   switch (bpp) {
-   case 8:
-   case 16:
-   case 24:
-   case 32:
-   break;
-   default:
-   return -EINVAL;
-   }
+
drm_helper_mode_fill_fb_struct(&fb->base, mode_cmd);
fb->gtt = gt;
ret = drm_framebuffer_init(dev, &fb->base, &psb_fb_funcs);
-- 
Regards,

Laurent Pinchart

[PATCH v4 10/14] drm: amdgpu: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

The driver needs the number of bytes per pixel, not the bpp and depth
info meant for fbdev compatibility. Use the right API.

Signed-off-by: Laurent Pinchart 
---
Changes since v3:

- Renamed bpp to cpp
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c  | 14 +++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c |  3 ++-
 2 files changed, 9 insertions(+), 8 deletions(-)

Cc: Alex Deucher 
Cc: Christian KÃ¶nig 
Cc: Michel DÃ¤nzer 

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
index bf033b58056c..0727946db189 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fb.c
@@ -62,12 +62,12 @@ static struct fb_ops amdgpufb_ops = {
 };


-int amdgpu_align_pitch(struct amdgpu_device *adev, int width, int bpp, bool 
tiled)
+int amdgpu_align_pitch(struct amdgpu_device *adev, int width, int cpp, bool 
tiled)
 {
int aligned = width;
int pitch_mask = 0;

-   switch (bpp / 8) {
+   switch (cpp) {
case 1:
pitch_mask = 255;
break;
@@ -82,7 +82,7 @@ int amdgpu_align_pitch(struct amdgpu_device *adev, int width, 
int bpp, bool tile

aligned += pitch_mask;
aligned &= ~pitch_mask;
-   return aligned;
+   return aligned * cpp;
 }

 static void amdgpufb_destroy_pinned_object(struct drm_gem_object *gobj)
@@ -111,13 +111,13 @@ static int amdgpufb_create_pinned_object(struct 
amdgpu_fbdev *rfbdev,
int ret;
int aligned_size, size;
int height = mode_cmd->height;
-   u32 bpp, depth;
+   u32 cpp;

-   drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+   cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);

/* need to align pitch with crtc limits */
-   mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, bpp,
- fb_tiled) * ((bpp + 1) / 8);
+   mode_cmd->pitches[0] = amdgpu_align_pitch(adev, mode_cmd->width, cpp,
+ fb_tiled);

height = ALIGN(mode_cmd->height, 8);
size = mode_cmd->pitches[0] * height;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 88fbed2389c0..20a4e569b245 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -704,7 +704,8 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
uint32_t handle;
int r;

-   args->pitch = amdgpu_align_pitch(adev, args->width, args->bpp, 0) * 
((args->bpp + 1) / 8);
+   args->pitch = amdgpu_align_pitch(adev, args->width,
+DIV_ROUND_UP(args->bpp, 8), 0);
args->size = (u64)args->pitch * args->height;
args->size = ALIGN(args->size, PAGE_SIZE);

-- 
Regards,

Laurent Pinchart

[PATCH v4 11/14] drm: radeon: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

The driver needs the number of bytes per pixel, not the bpp and depth
info meant for fbdev compatibility. Use the right API.

Signed-off-by: Laurent Pinchart 
---
Changes since v3:

- Renamed bpp to cpp
---
 drivers/gpu/drm/radeon/radeon_fb.c  | 20 ++--
 drivers/gpu/drm/radeon/radeon_gem.c |  3 ++-
 2 files changed, 12 insertions(+), 11 deletions(-)

Cc: Alex Deucher 
Cc: Christian KÃ¶nig 
Cc: Michel DÃ¤nzer 

diff --git a/drivers/gpu/drm/radeon/radeon_fb.c 
b/drivers/gpu/drm/radeon/radeon_fb.c
index 568e036d547e..f4d6899ce3bb 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -61,13 +61,13 @@ static struct fb_ops radeonfb_ops = {
 };


-int radeon_align_pitch(struct radeon_device *rdev, int width, int bpp, bool 
tiled)
+int radeon_align_pitch(struct radeon_device *rdev, int width, int cpp, bool 
tiled)
 {
int aligned = width;
int align_large = (ASIC_IS_AVIVO(rdev)) || tiled;
int pitch_mask = 0;

-   switch (bpp / 8) {
+   switch (cpp) {
case 1:
pitch_mask = align_large ? 255 : 127;
break;
@@ -82,7 +82,7 @@ int radeon_align_pitch(struct radeon_device *rdev, int width, 
int bpp, bool tile

aligned += pitch_mask;
aligned &= ~pitch_mask;
-   return aligned;
+   return aligned * cpp;
 }

 static void radeonfb_destroy_pinned_object(struct drm_gem_object *gobj)
@@ -111,13 +111,13 @@ static int radeonfb_create_pinned_object(struct 
radeon_fbdev *rfbdev,
int ret;
int aligned_size, size;
int height = mode_cmd->height;
-   u32 bpp, depth;
+   u32 cpp;

-   drm_fb_get_bpp_depth(mode_cmd->pixel_format, &depth, &bpp);
+   cpp = drm_format_plane_cpp(mode_cmd->pixel_format, 0);

/* need to align pitch with crtc limits */
-   mode_cmd->pitches[0] = radeon_align_pitch(rdev, mode_cmd->width, bpp,
- fb_tiled) * ((bpp + 1) / 8);
+   mode_cmd->pitches[0] = radeon_align_pitch(rdev, mode_cmd->width, cpp,
+ fb_tiled);

if (rdev->family >= CHIP_R600)
height = ALIGN(mode_cmd->height, 8);
@@ -137,11 +137,11 @@ static int radeonfb_create_pinned_object(struct 
radeon_fbdev *rfbdev,
tiling_flags = RADEON_TILING_MACRO;

 #ifdef __BIG_ENDIAN
-   switch (bpp) {
-   case 32:
+   switch (cpp) {
+   case 4:
tiling_flags |= RADEON_TILING_SWAP_32BIT;
break;
-   case 16:
+   case 2:
tiling_flags |= RADEON_TILING_SWAP_16BIT;
default:
break;
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c 
b/drivers/gpu/drm/radeon/radeon_gem.c
index deb9511725c9..0bcffd8a7bd3 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -745,7 +745,8 @@ int radeon_mode_dumb_create(struct drm_file *file_priv,
uint32_t handle;
int r;

-   args->pitch = radeon_align_pitch(rdev, args->width, args->bpp, 0) * 
((args->bpp + 1) / 8);
+   args->pitch = radeon_align_pitch(rdev, args->width,
+DIV_ROUND_UP(args->bpp, 8), 0);
args->size = args->pitch * args->height;
args->size = ALIGN(args->size, PAGE_SIZE);

-- 
Regards,

Laurent Pinchart

[PATCH v4 13/14] drm/arm: mali-dp: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

The driver doesn't need the color depth, only the number of bits per
pixel. Use the right API.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/arm/malidp_hw.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c
index a6132f1d58c1..be815d0cc772 100644
--- a/drivers/gpu/drm/arm/malidp_hw.c
+++ b/drivers/gpu/drm/arm/malidp_hw.c
@@ -198,9 +198,6 @@ static void malidp500_modeset(struct malidp_hw_device 
*hwdev, struct videomode *

 static int malidp500_rotmem_required(struct malidp_hw_device *hwdev, u16 w, 
u16 h, u32 fmt)
 {
-   unsigned int depth;
-   int bpp;
-
/* RGB888 or BGR888 can't be rotated */
if ((fmt == DRM_FORMAT_RGB888) || (fmt == DRM_FORMAT_BGR888))
return -EINVAL;
@@ -210,9 +207,7 @@ static int malidp500_rotmem_required(struct 
malidp_hw_device *hwdev, u16 w, u16
 * worth of pixel data. Required size is then:
 *size = rotated_width * (bpp / 8) * 8;
 */
-   drm_fb_get_bpp_depth(fmt, &depth, &bpp);
-
-   return w * bpp;
+   return w * drm_format_plane_cpp(fmt, 0) * 8;
 }

 static int malidp550_query_hw(struct malidp_hw_device *hwdev)
-- 
Regards,

Laurent Pinchart

[PATCH v4 14/14] drm: Don't export the drm_fb_get_bpp_depth() function

The function is only used by the drm_helper_mode_fill_fb_struct() core
function to fill the drm_framebuffer bpp and depth fields, used by
drivers that haven't been converted to use pixel formats directly yet.
It should not be used by new drivers, so inline it in its only caller.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/drm_fourcc.c | 31 ---
 drivers/gpu/drm/drm_modeset_helper.c | 17 +++--
 include/drm/drm_fourcc.h |  1 -
 3 files changed, 15 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
index 0355b7ede8e4..239d1aab8386 100644
--- a/drivers/gpu/drm/drm_fourcc.c
+++ b/drivers/gpu/drm/drm_fourcc.c
@@ -203,37 +203,6 @@ const struct drm_format_info *drm_format_info(u32 format)
 EXPORT_SYMBOL(drm_format_info);

 /**
- * drm_fb_get_bpp_depth - get the bpp/depth values for format
- * @format: pixel format (DRM_FORMAT_*)
- * @depth: storage for the depth value
- * @bpp: storage for the bpp value
- *
- * This only supports RGB formats here for compat with code that doesn't use
- * pixel formats directly yet.
- */
-void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth,
- int *bpp)
-{
-   const struct drm_format_info *info;
-
-   info = drm_format_info(format);
-   if (!info || !info->depth) {
-   char *format_name = drm_get_format_name(format);
-
-   DRM_DEBUG_KMS("unsupported pixel format %s\n", format_name);
-   kfree(format_name);
-
-   *depth = 0;
-   *bpp = 0;
-   return;
-   }
-
-   *depth = info->depth;
-   *bpp = info->cpp[0] << 3;
-}
-EXPORT_SYMBOL(drm_fb_get_bpp_depth);
-
-/**
  * drm_format_num_planes - get the number of planes for format
  * @format: pixel format (DRM_FORMAT_*)
  *
diff --git a/drivers/gpu/drm/drm_modeset_helper.c 
b/drivers/gpu/drm/drm_modeset_helper.c
index 1d45738f8f98..442325f10231 100644
--- a/drivers/gpu/drm/drm_modeset_helper.c
+++ b/drivers/gpu/drm/drm_modeset_helper.c
@@ -70,8 +70,23 @@ EXPORT_SYMBOL(drm_helper_move_panel_connectors_to_head);
 void drm_helper_mode_fill_fb_struct(struct drm_framebuffer *fb,
const struct drm_mode_fb_cmd2 *mode_cmd)
 {
+   const struct drm_format_info *info;
int i;

+   info = drm_format_info(mode_cmd->pixel_format);
+   if (!info || !info->depth) {
+   char *format_name = drm_get_format_name(mode_cmd->pixel_format);
+
+   DRM_DEBUG_KMS("non-RGB pixel format %s\n", format_name);
+   kfree(format_name);
+
+   fb->depth = 0;
+   fb->bits_per_pixel = 0;
+   } else {
+   fb->depth = info->depth;
+   fb->bits_per_pixel = info->cpp[0] << 3;
+   }
+
fb->width = mode_cmd->width;
fb->height = mode_cmd->height;
for (i = 0; i < 4; i++) {
@@ -79,8 +94,6 @@ void drm_helper_mode_fill_fb_struct(struct drm_framebuffer 
*fb,
fb->offsets[i] = mode_cmd->offsets[i];
fb->modifier[i] = mode_cmd->modifier[i];
}
-   drm_fb_get_bpp_depth(mode_cmd->pixel_format, &fb->depth,
-   &fb->bits_per_pixel);
fb->pixel_format = mode_cmd->pixel_format;
fb->flags = mode_cmd->flags;
 }
diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
index 7ffb114d2468..1fbd04924b4d 100644
--- a/include/drm/drm_fourcc.h
+++ b/include/drm/drm_fourcc.h
@@ -46,7 +46,6 @@ struct drm_format_info {
 const struct drm_format_info *__drm_format_info(u32 format);
 const struct drm_format_info *drm_format_info(u32 format);
 uint32_t drm_mode_legacy_fb_format(uint32_t bpp, uint32_t depth);
-void drm_fb_get_bpp_depth(uint32_t format, unsigned int *depth, int *bpp);
 int drm_format_num_planes(uint32_t format);
 int drm_format_plane_cpp(uint32_t format, int plane);
 int drm_format_horz_chroma_subsampling(uint32_t format);
-- 
Regards,

Laurent Pinchart

[PATCH v4 12/14] drm: vmwgfx: Replace drm_fb_get_bpp_depth() with drm_format_info()

The driver is the last users of the drm_fb_get_bpp_depth() function. It
should ideally be converted to use struct drm_mode_fb_cmd2 instead of
the legacy struct drm_mode_fb_cmd internally, but that will require
broad changes across the code base. As a first step, replace
drm_fb_get_bpp_depth() with drm_format_info() in order to stop exporting
the function to drivers.

The new DRM_ERROR() message comes from the vmw_create_dmabuf_proxy(),
vmw_kms_new_framebuffer_surface() and vmw_kms_new_framebuffer_dmabuf()
functions that currently print an error if the pixel format is
unsupported.

Signed-off-by: Laurent Pinchart 
Reviewed-by: Sinclair Yeh 
---
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Cc: VMware Graphics 
Cc: Sinclair Yeh 
Cc: Thomas Hellstrom 

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index bf28ccc150df..c965514b82be 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -980,14 +980,22 @@ static struct drm_framebuffer *vmw_kms_fb_create(struct 
drm_device *dev,
struct vmw_dma_buffer *bo = NULL;
struct ttm_base_object *user_obj;
struct drm_mode_fb_cmd mode_cmd;
+   const struct drm_format_info *info;
int ret;

+   info = drm_format_info(mode_cmd2->pixel_format);
+   if (!info || !info->depth) {
+   DRM_ERROR("Unsupported framebuffer format %s\n",
+ drm_get_format_name(mode_cmd2->pixel_format));
+   return ERR_PTR(-EINVAL);
+   }
+
mode_cmd.width = mode_cmd2->width;
mode_cmd.height = mode_cmd2->height;
mode_cmd.pitch = mode_cmd2->pitches[0];
mode_cmd.handle = mode_cmd2->handles[0];
-   drm_fb_get_bpp_depth(mode_cmd2->pixel_format, &mode_cmd.depth,
-   &mode_cmd.bpp);
+   mode_cmd.depth = info->depth;
+   mode_cmd.bpp = info->cpp[0] * 8;

/**
 * This code should be conditioned on Screen Objects not being used.
-- 
Regards,

Laurent Pinchart

[PATCH v3 08/15] drm: hdlcd: Replace drm_fb_get_bpp_depth() with drm_format_plane_cpp()

Hi Liviu,

On Monday 25 Jul 2016 12:10:24 Liviu Dudau wrote:
> On Thu, Jun 09, 2016 at 10:01:40AM +0100, Liviu Dudau wrote:
> > On Thu, Jun 09, 2016 at 02:32:12AM +0300, Laurent Pinchart wrote:
> > > The driver needs the number of bytes per pixel, not the bpp and depth
> > > info meant for fbdev compatibility. Use the right API.
> > > 
> > > Signed-off-by: Laurent Pinchart 
> > > ---
> > > 
> > >  drivers/gpu/drm/arm/hdlcd_crtc.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > Cc: Liviu Dudau 
> > 
> > Acked-by: Liviu Dudau 
> > 
> > Thanks for the cleanup!
> > 
> > Best regards,
> > Liviu
> 
> Hi Laurent,
> 
> What is the status of this patchset? Are you going to send it for v4.8?

I'm afraid not :-) I have just sent a rebased version (v4).

-- 
Regards,

Laurent Pinchart

[PATCH v7 0/4] New debugfs API for capturing CRC of frames

Hi,

this series basically takes the facility for continuously capturing CRCs
of frames from the i915 driver and into the DRM core.

The idea is that test suites such as IGT use this information to check
that frames that are exected to be identical, also have identical CRC
values.

Other drivers for hardware that can provide frame CRCs (including eDP
panels that support self-refresh) can easily implement the new callback
and provide userspace with the CRC values.

Thanks,

Tomeu

Tomeu Vizoso (4):
  drm/i915/debugfs: Move out pipe CRC code
  drm: Add API for capturing frame CRCs
  drm/i915: Use new CRC debugfs API
  drm/i915: Put "cooked" vlank counters in frame CRC lines

 Documentation/gpu/drm-uapi.rst|6 +
 drivers/gpu/drm/Makefile  |3 +-
 drivers/gpu/drm/drm_crtc.c|   29 +-
 drivers/gpu/drm/drm_debugfs.c |   34 +-
 drivers/gpu/drm/drm_debugfs_crc.c |  351 
 drivers/gpu/drm/drm_drv.c |   15 +
 drivers/gpu/drm/drm_internal.h|   16 +
 drivers/gpu/drm/i915/Makefile |2 +-
 drivers/gpu/drm/i915/i915_debugfs.c   |  886 +---
 drivers/gpu/drm/i915/i915_drv.c   |2 +-
 drivers/gpu/drm/i915/i915_drv.h   |3 +-
 drivers/gpu/drm/i915/i915_irq.c   |   83 ++-
 drivers/gpu/drm/i915/intel_display.c  |1 +
 drivers/gpu/drm/i915/intel_drv.h  |7 +
 drivers/gpu/drm/i915/intel_pipe_crc.c | 1014 +
 include/drm/drm_crtc.h|   41 ++
 include/drm/drm_debugfs_crc.h |   73 +++
 17 files changed, 1652 insertions(+), 914 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c
 create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c
 create mode 100644 include/drm/drm_debugfs_crc.h

-- 
2.7.4

[PATCH v7 1/4] drm/i915/debugfs: Move out pipe CRC code

In preparation to using a generic API in the DRM core for continuous CRC
generation, move the related code out of i915_debugfs.c into a new file.

Eventually, only the Intel-specific code will remain in this new file.

v2: Rebased.

v6: Rebased.

v7: Fix whitespace issue.

Signed-off-by: Tomeu Vizoso 
Reviewed-by: Emil Velikov 
---

 drivers/gpu/drm/i915/Makefile |   2 +-
 drivers/gpu/drm/i915/i915_debugfs.c   | 886 +--
 drivers/gpu/drm/i915/i915_drv.c   |   2 +-
 drivers/gpu/drm/i915/i915_drv.h   |   2 +-
 drivers/gpu/drm/i915/intel_drv.h  |   5 +
 drivers/gpu/drm/i915/intel_pipe_crc.c | 944 ++
 6 files changed, 956 insertions(+), 885 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_pipe_crc.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a7da24640e88..6238f8042426 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -23,7 +23,7 @@ i915-y := i915_drv.o \
  intel_runtime_pm.o

 i915-$(CONFIG_COMPAT)   += i915_ioc32.o
-i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
+i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o intel_pipe_crc.o

 # GEM code
 i915-y += i915_cmd_parser.o \
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c 
b/drivers/gpu/drm/i915/i915_debugfs.c
index 7d7b4d9280e9..d8073cddffeb 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -26,19 +26,9 @@
  *
  */

-#include 
-#include 
-#include 
 #include 
-#include 
-#include 
 #include 
-#include 
-#include 
 #include "intel_drv.h"
-#include "intel_ringbuffer.h"
-#include 
-#include "i915_drv.h"

 static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
 {
@@ -3401,12 +3391,6 @@ static int i915_drrs_status(struct seq_file *m, void 
*unused)
return 0;
 }

-struct pipe_crc_info {
-   const char *name;
-   struct drm_i915_private *dev_priv;
-   enum pipe pipe;
-};
-
 static int i915_dp_mst_info(struct seq_file *m, void *unused)
 {
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -3436,848 +3420,6 @@ static int i915_dp_mst_info(struct seq_file *m, void 
*unused)
return 0;
 }

-static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
-{
-   struct pipe_crc_info *info = inode->i_private;
-   struct drm_i915_private *dev_priv = info->dev_priv;
-   struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
-   if (info->pipe >= INTEL_INFO(dev_priv)->num_pipes)
-   return -ENODEV;
-
-   spin_lock_irq(&pipe_crc->lock);
-
-   if (pipe_crc->opened) {
-   spin_unlock_irq(&pipe_crc->lock);
-   return -EBUSY; /* already open */
-   }
-
-   pipe_crc->opened = true;
-   filep->private_data = inode->i_private;
-
-   spin_unlock_irq(&pipe_crc->lock);
-
-   return 0;
-}
-
-static int i915_pipe_crc_release(struct inode *inode, struct file *filep)
-{
-   struct pipe_crc_info *info = inode->i_private;
-   struct drm_i915_private *dev_priv = info->dev_priv;
-   struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-
-   spin_lock_irq(&pipe_crc->lock);
-   pipe_crc->opened = false;
-   spin_unlock_irq(&pipe_crc->lock);
-
-   return 0;
-}
-
-/* (6 fields, 8 chars each, space separated (5) + '\n') */
-#define PIPE_CRC_LINE_LEN  (6 * 8 + 5 + 1)
-/* account for \'0' */
-#define PIPE_CRC_BUFFER_LEN(PIPE_CRC_LINE_LEN + 1)
-
-static int pipe_crc_data_count(struct intel_pipe_crc *pipe_crc)
-{
-   assert_spin_locked(&pipe_crc->lock);
-   return CIRC_CNT(pipe_crc->head, pipe_crc->tail,
-   INTEL_PIPE_CRC_ENTRIES_NR);
-}
-
-static ssize_t
-i915_pipe_crc_read(struct file *filep, char __user *user_buf, size_t count,
-  loff_t *pos)
-{
-   struct pipe_crc_info *info = filep->private_data;
-   struct drm_i915_private *dev_priv = info->dev_priv;
-   struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[info->pipe];
-   char buf[PIPE_CRC_BUFFER_LEN];
-   int n_entries;
-   ssize_t bytes_read;
-
-   /*
-* Don't allow user space to provide buffers not big enough to hold
-* a line of data.
-*/
-   if (count < PIPE_CRC_LINE_LEN)
-   return -EINVAL;
-
-   if (pipe_crc->source == INTEL_PIPE_CRC_SOURCE_NONE)
-   return 0;
-
-   /* nothing to read */
-   spin_lock_irq(&pipe_crc->lock);
-   while (pipe_crc_data_count(pipe_crc) == 0) {
-   int ret;
-
-   if (filep->f_flags & O_NONBLOCK) {
-   spin_unlock_irq(&pipe_crc->lock);
-   return -EAGAIN;
-   }
-
-   ret = wait_event_interruptible_lock_irq(pipe_crc->wq,
-   pipe_crc_data_count(pipe_crc), pipe_crc->lock);
-   if (ret) {
-   spin_unlock_irq(&pipe_cr

[PATCH v7 2/4] drm: Add API for capturing frame CRCs

Adds files and directories to debugfs for controlling and reading frame
CRCs, per CRTC:

dri/0/crtc-0/crc
dri/0/crtc-0/crc/control
dri/0/crtc-0/crc/data

Drivers can implement the set_crc_source callback() in drm_crtc_funcs to
start and stop generating frame CRCs and can add entries to the output
by calling drm_crtc_add_crc_entry.

v2:
- Lots of good fixes suggested by Thierry.
- Added documentation.
- Changed the debugfs layout.
- Moved to allocate the entries circular queue once when frame
  generation gets enabled for the first time.
v3:
- Use the control file just to select the source, and start and stop
  capture when the data file is opened and closed, respectively.
- Make variable the number of CRC values per entry, per source.
- Allocate entries queue each time we start capturing as now there
  isn't a fixed number of CRC values per entry.
- Store the frame counter in the data file as a 8-digit hex number.
- For sources that cannot provide useful frame numbers, place
   in the frame field.

v4:
- Build only if CONFIG_DEBUG_FS is enabled.
- Use memdup_user_nul.
- Consolidate calculation of the size of an entry in a helper.
- Add 0x prefix to hex numbers in the data file.
- Remove unnecessary snprintf and strlen usage in read callback.

v5:
- Made the crcs array in drm_crtc_crc_entry fixed-size
- Lots of other smaller improvements suggested by Emil Velikov

v7:
- Move definition of drm_debugfs_crtc_crc_add to drm_internal.h

Signed-off-by: Tomeu Vizoso 
Reviewed-by: Emil Velikov 
---

 Documentation/gpu/drm-uapi.rst|   6 +
 drivers/gpu/drm/Makefile  |   3 +-
 drivers/gpu/drm/drm_crtc.c|  29 +++-
 drivers/gpu/drm/drm_debugfs.c |  34 +++-
 drivers/gpu/drm/drm_debugfs_crc.c | 351 ++
 drivers/gpu/drm/drm_drv.c |  15 ++
 drivers/gpu/drm/drm_internal.h|  16 ++
 include/drm/drm_crtc.h|  41 +
 include/drm/drm_debugfs_crc.h |  73 
 9 files changed, 565 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_debugfs_crc.c
 create mode 100644 include/drm/drm_debugfs_crc.h

diff --git a/Documentation/gpu/drm-uapi.rst b/Documentation/gpu/drm-uapi.rst
index 12b47c30fe2e..4d5f61b6c0f2 100644
--- a/Documentation/gpu/drm-uapi.rst
+++ b/Documentation/gpu/drm-uapi.rst
@@ -179,3 +179,9 @@ interfaces. Especially since all hardware-acceleration 
interfaces to
 userspace are driver specific for efficiency and other reasons these
 interfaces can be rather substantial. Hence every driver has its own
 chapter.
+
+Testing and validation
+==
+
+.. kernel-doc:: drivers/gpu/drm/drm_debugfs_crc.c
+   :doc: CRC ABI
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 4054c94a2301..9c99b051ce87 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -9,7 +9,7 @@ drm-y   :=  drm_auth.o drm_bufs.o drm_cache.o \
drm_scatter.o drm_pci.o \
drm_platform.o drm_sysfs.o drm_hashtab.o drm_mm.o \
drm_crtc.o drm_fourcc.o drm_modes.o drm_edid.o \
-   drm_info.o drm_debugfs.o drm_encoder_slave.o \
+   drm_info.o drm_encoder_slave.o \
drm_trace_points.o drm_global.o drm_prime.o \
drm_rect.o drm_vma_manager.o drm_flip_work.o \
drm_modeset_lock.o drm_atomic.o drm_bridge.o \
@@ -21,6 +21,7 @@ drm-$(CONFIG_PCI) += ati_pcigart.o
 drm-$(CONFIG_DRM_PANEL) += drm_panel.o
 drm-$(CONFIG_OF) += drm_of.o
 drm-$(CONFIG_AGP) += drm_agpsupport.o
+drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o

 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o \
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 7f2510524f09..17cc6891dfbb 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -40,7 +40,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 

 #include "drm_crtc_internal.h"
 #include "drm_internal.h"
@@ -309,6 +309,25 @@ static void drm_crtc_unregister_all(struct drm_device *dev)
}
 }

+static int drm_crtc_crc_init(struct drm_crtc *crtc)
+{
+#ifdef CONFIG_DEBUG_FS
+   spin_lock_init(&crtc->crc.lock);
+   init_waitqueue_head(&crtc->crc.wq);
+   crtc->crc.source = kstrdup("auto", GFP_KERNEL);
+   if (!crtc->crc.source)
+   return -ENOMEM;
+#endif
+   return 0;
+}
+
+static void drm_crtc_crc_fini(struct drm_crtc *crtc)
+{
+#ifdef CONFIG_DEBUG_FS
+   kfree(crtc->crc.source);
+#endif
+}
+
 /**
  * drm_crtc_init_with_planes - Initialise a new CRTC object with
  *specified primary and cursor planes.
@@ -374,6 +393,12 @@ int drm_crtc_init_with_planes(struct drm_device *dev, 
struct drm_crtc *crtc,
if (cursor)
cursor->possible_crtcs = 1 << drm_crtc_index(crtc);

+

[PATCH v7 3/4] drm/i915: Use new CRC debugfs API

The core provides now an ABI to userspace for generation of frame CRCs,
so implement the ->set_crc_source() callback and reuse as much code as
possible with the previous ABI implementation.

When handling the pageflip interrupt, we skip 1 or 2 frames depending on
the HW because they contain wrong values. For the legacy ABI for
generating frame CRCs, this was done in userspace but now that we have a
generic ABI it's better if it's not exposed by the kernel.

v2:
- Leave the legacy implementation in place as the ABI implementation
  in the core is incompatible with it.
v3:
- Use the "cooked" vblank counter so we have a whole 32 bits.
- Make sure we don't mess with the state of the legacy CRC capture
  ABI implementation.
v4:
- Keep use of get_vblank_counter as in the legacy code, will be
  changed in a followup commit.

v5:
- Skip first frame or two as it's known that they contain wrong
  data.
- A few fixes suggested by Emil Velikov.

v6:
- Rework programming of the HW registers to preserve previous
  behavior.

v7:
- Address whitespace issue.
- Added a comment on why in the implementation of the new ABI we
  skip the 1st or 2nd frames.

Signed-off-by: Tomeu Vizoso 
Reviewed-by: Emil Velikov 
---

 drivers/gpu/drm/i915/i915_drv.h   |  1 +
 drivers/gpu/drm/i915/i915_irq.c   | 83 +--
 drivers/gpu/drm/i915/intel_display.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h  |  2 +
 drivers/gpu/drm/i915/intel_pipe_crc.c | 94 ++-
 5 files changed, 143 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 403c074a29f4..77d05807adc6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1672,6 +1672,7 @@ struct intel_pipe_crc {
enum intel_pipe_crc_source source;
int head, tail;
wait_queue_head_t wq;
+   int skipped;
 };

 struct i915_frontbuffer_tracking {
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 7610eca4f687..413667497ce0 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1485,41 +1485,72 @@ static void display_pipe_crc_irq_handler(struct 
drm_i915_private *dev_priv,
 {
struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
struct intel_pipe_crc_entry *entry;
-   int head, tail;
+   struct drm_crtc *crtc = intel_get_crtc_for_pipe(&dev_priv->drm, pipe);
+   struct drm_driver *driver = dev_priv->drm.driver;
+   uint32_t crcs[5];
+   int head, tail, ret;
+   u32 frame;

spin_lock(&pipe_crc->lock);
+   if (pipe_crc->source) {
+   if (!pipe_crc->entries) {
+   spin_unlock(&pipe_crc->lock);
+   DRM_DEBUG_KMS("spurious interrupt\n");
+   return;
+   }

-   if (!pipe_crc->entries) {
-   spin_unlock(&pipe_crc->lock);
-   DRM_DEBUG_KMS("spurious interrupt\n");
-   return;
-   }
-
-   head = pipe_crc->head;
-   tail = pipe_crc->tail;
+   head = pipe_crc->head;
+   tail = pipe_crc->tail;

-   if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
-   spin_unlock(&pipe_crc->lock);
-   DRM_ERROR("CRC buffer overflowing\n");
-   return;
-   }
+   if (CIRC_SPACE(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) < 1) {
+   spin_unlock(&pipe_crc->lock);
+   DRM_ERROR("CRC buffer overflowing\n");
+   return;
+   }

-   entry = &pipe_crc->entries[head];
+   entry = &pipe_crc->entries[head];

-   entry->frame = dev_priv->drm.driver->get_vblank_counter(&dev_priv->drm,
-pipe);
-   entry->crc[0] = crc0;
-   entry->crc[1] = crc1;
-   entry->crc[2] = crc2;
-   entry->crc[3] = crc3;
-   entry->crc[4] = crc4;
+   entry->frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
+   entry->crc[0] = crc0;
+   entry->crc[1] = crc1;
+   entry->crc[2] = crc2;
+   entry->crc[3] = crc3;
+   entry->crc[4] = crc4;

-   head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
-   pipe_crc->head = head;
+   head = (head + 1) & (INTEL_PIPE_CRC_ENTRIES_NR - 1);
+   pipe_crc->head = head;

-   spin_unlock(&pipe_crc->lock);
+   spin_unlock(&pipe_crc->lock);

-   wake_up_interruptible(&pipe_crc->wq);
+   wake_up_interruptible(&pipe_crc->wq);
+   } else {
+   /*
+* For some not yet identified reason, the first CRC is
+* bonkers. So let's just wait for the next vblank and read
+* out the buggy result.
+

[PATCH v7 4/4] drm/i915: Put "cooked" vlank counters in frame CRC lines

Use drm_accurate_vblank_count so we have the full 32 bit to represent
the frame counter and userspace has a simpler way of knowing when the
counter wraps around.

Signed-off-by: Tomeu Vizoso 
Reviewed-by: Emil Velikov 
---

 drivers/gpu/drm/i915/i915_irq.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 413667497ce0..32a5f4634d6d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1489,7 +1489,6 @@ static void display_pipe_crc_irq_handler(struct 
drm_i915_private *dev_priv,
struct drm_driver *driver = dev_priv->drm.driver;
uint32_t crcs[5];
int head, tail, ret;
-   u32 frame;

spin_lock(&pipe_crc->lock);
if (pipe_crc->source) {
@@ -1545,8 +1544,9 @@ static void display_pipe_crc_irq_handler(struct 
drm_i915_private *dev_priv,
crcs[2] = crc2;
crcs[3] = crc3;
crcs[4] = crc4;
-   frame = driver->get_vblank_counter(&dev_priv->drm, pipe);
-   ret = drm_crtc_add_crc_entry(crtc, true, frame, crcs);
+   ret = drm_crtc_add_crc_entry(crtc, true,
+drm_accurate_vblank_count(crtc),
+crcs);
spin_unlock(&crtc->crc.lock);
if (!ret)
wake_up_interruptible(&crtc->crc.wq);
-- 
2.7.4

[Intel-gfx] [PATCH v6 3/4] drm/i915: Use new CRC debugfs API

On 8 September 2016 at 15:35, Emil Velikov  wrote:
> Hi Tomeu,
>
> Just a couple of nitpicks. Nothing that has to be fixed or (if you
> agree) cannot be done on top/later on.
>
> On 7 September 2016 at 11:27, Tomeu Vizoso  
> wrote:
>> The core provides now an ABI to userspace for generation of frame CRCs,
>> so implement the ->set_crc_source() callback and reuse as much code as
>> possible with the previous ABI implementation.
>>
>> v2:
>> - Leave the legacy implementation in place as the ABI implementation
>>   in the core is incompatible with it.
>> v3:
>> - Use the "cooked" vblank counter so we have a whole 32 bits.
>> - Make sure we don't mess with the state of the legacy CRC capture
>>   ABI implementation.
>> v4:
>> - Keep use of get_vblank_counter as in the legacy code, will be
>>   changed in a followup commit.
>>
>> v5:
>> - Skip first frame or two as it's known that they contain wrong
>>   data.
> Even if the frames are only skipped in the new code, it doesn't
> explain why one'd need it in the first place and/or how it isn't
> required with the current code. Might be worth poking the original
> authors and/or adding a big WARNING/NOTE/XXX/HACK to make things more
> prominent.

Have added a note to the commit message, as once the legacy codepath
is removed, it could be confusing.

>
>> - A few fixes suggested by Emil Velikov.
>>
>> v6:
>> - Rework programming of the HW registers to preserve previous
>>   behavior.
>>
> Huge thanks for this.
>
>
>> @@ -791,7 +797,7 @@ display_crc_ctl_parse_object(const char *buf, enum 
>> intel_pipe_crc_object *o)
>> if (!strcmp(buf, pipe_crc_objects[i])) {
>> *o = i;
>> return 0;
>> -   }
>> +   }
>>
> Looks like newly introduced whitespace changes, should have been part of 1/4 ?

Oops, both instances fixed in v7.

Thanks,

Tomeu

>> return -EINVAL;
>>  }
>> @@ -813,11 +819,16 @@ display_crc_ctl_parse_source(const char *buf, enum 
>> intel_pipe_crc_source *s)
>>  {
>> int i;
>
>> if (!strcmp(buf, pipe_crc_sources[i])) {
>> *s = i;
>> return 0;
>> -   }
>> +   }
>>
> Ditto ?
>
> Thanks
> Emil
> ___
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v6 2/4] drm: Add API for capturing frame CRCs

On 8 September 2016 at 15:24, Emil Velikov  wrote:
> Hi Tomeu,
>
> A couple of small nitpicks and a rather nasty looking bug, related to
> your earlier question.
>
> On 7 September 2016 at 11:27, Tomeu Vizoso  
> wrote:
>
>> +static ssize_t crc_control_write(struct file *file, const char __user *ubuf,
>> +size_t len, loff_t *offp)
>> +{
>
>> +   if (source[len] == '\n')
>> +   source[len] = '\0';
>> +
> Considering the bug below, I'm considering if there's a case were we
> don't want to explicitly set the terminating byte ?
>
>
>> +/*
>> + * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, 
>> space
>> + * separated, with a newline at the end and null-terminated.
>> + */
> NULL-terminated what the things that was missing, explaining the
> maths. Yet note the code sort of contradicts it.
>
> TL;DR: above we conditionally NULL terminate the data, yet (below) we
> feed garbage (always) instead of \0 byte.
>
>> +#define LINE_LEN(values_cnt)   (10 + 11 * values_cnt + 1 + 1)
>> +#define MAX_LINE_LEN   (LINE_LEN(DRM_MAX_CRC_NR))
>> +
>> +static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
>> +size_t count, loff_t *pos)
>> +{
>> +   struct drm_crtc *crtc = filep->f_inode->i_private;
>> +   struct drm_crtc_crc *crc = &crtc->crc;
>> +   struct drm_crtc_crc_entry *entry;
>> +   char buf[MAX_LINE_LEN];
> Here buf is filled with garbage...
>
>
>> +   if (entry->has_frame_counter)
>> +   sprintf(buf, "0x%08x", entry->frame);
>> +   else
>> +   sprintf(buf, "XX");
>> +
>> +   for (i = 0; i < crc->values_cnt; i++)
>> +   sprintf(buf + 10 + i * 11, " 0x%08x", entry->crcs[i]);
>> +   sprintf(buf + 10 + crc->values_cnt * 11, "\n");
>> +
> ... and now all the data is in, incl. the \n byte.
>
>> +   if (copy_to_user(user_buf, buf, LINE_LEN(crc->values_cnt)))
> And here we copy the whole thing incl. the 'should be \0 but is
> actually garbage' byte.

As discussed offline, sprintf does terminate the string for us, so I
think this is fine.

> This doesn't look good :-\
>
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>
>> @@ -221,6 +222,14 @@ static int drm_minor_register(struct drm_device *dev, 
>> unsigned int type)
>> return ret;
>> }
>>
>> +   if (type == DRM_MINOR_PRIMARY) {
>> +   drm_for_each_crtc(crtc, dev) {
>> +   ret = drm_debugfs_crtc_add(crtc);
>> +   if (ret)
>> +   DRM_ERROR("DRM: Failed to initialize CRC 
>> debugfs.\n");
>> +   }
>> +   }
>> +
> Minor: We're missing teardown in the error path.

Isn't drm_debugfs_cleanup taking care of it?

>
>> --- /dev/null
>> +++ b/include/drm/drm_debugfs_crc.h
>
>> +static inline int drm_debugfs_crtc_crc_add(struct drm_crtc *crtc)
> Minor: The function is internal only and used only when
> defined(CONFIG_DEBUG_FS). Thus it should stay in
> drivers/gpu/drm/foo.h.
>
> Rule of thumb: include/drm defines the API used by the drivers, while
> drivers/gpu/drm/foo_internal.h the internal API between the core DRM
> modules.

Good one.

Thanks!

Tomeu

[PATCH v6 2/4] drm: Add API for capturing frame CRCs

On 8 September 2016 at 15:49, Tomeu Vizoso  
wrote:
> On 8 September 2016 at 15:24, Emil Velikov  
> wrote:
>> Hi Tomeu,
>>
>> A couple of small nitpicks and a rather nasty looking bug, related to
>> your earlier question.
>>
>> On 7 September 2016 at 11:27, Tomeu Vizoso  
>> wrote:
>>
>>> +static ssize_t crc_control_write(struct file *file, const char __user 
>>> *ubuf,
>>> +size_t len, loff_t *offp)
>>> +{
>>
>>> +   if (source[len] == '\n')
>>> +   source[len] = '\0';
>>> +
>> Considering the bug below, I'm considering if there's a case were we
>> don't want to explicitly set the terminating byte ?
>>
>>
>>> +/*
>>> + * 1 frame field of 10 chars plus a number of CRC fields of 10 chars each, 
>>> space
>>> + * separated, with a newline at the end and null-terminated.
>>> + */
>> NULL-terminated what the things that was missing, explaining the
>> maths. Yet note the code sort of contradicts it.
>>
>> TL;DR: above we conditionally NULL terminate the data, yet (below) we
>> feed garbage (always) instead of \0 byte.
>>
>>> +#define LINE_LEN(values_cnt)   (10 + 11 * values_cnt + 1 + 1)
>>> +#define MAX_LINE_LEN   (LINE_LEN(DRM_MAX_CRC_NR))
>>> +
>>> +static ssize_t crtc_crc_read(struct file *filep, char __user *user_buf,
>>> +size_t count, loff_t *pos)
>>> +{
>>> +   struct drm_crtc *crtc = filep->f_inode->i_private;
>>> +   struct drm_crtc_crc *crc = &crtc->crc;
>>> +   struct drm_crtc_crc_entry *entry;
>>> +   char buf[MAX_LINE_LEN];
>> Here buf is filled with garbage...
>>
>>
>>> +   if (entry->has_frame_counter)
>>> +   sprintf(buf, "0x%08x", entry->frame);
>>> +   else
>>> +   sprintf(buf, "XX");
>>> +
>>> +   for (i = 0; i < crc->values_cnt; i++)
>>> +   sprintf(buf + 10 + i * 11, " 0x%08x", entry->crcs[i]);
>>> +   sprintf(buf + 10 + crc->values_cnt * 11, "\n");
>>> +
>> ... and now all the data is in, incl. the \n byte.
>>
>>> +   if (copy_to_user(user_buf, buf, LINE_LEN(crc->values_cnt)))
>> And here we copy the whole thing incl. the 'should be \0 but is
>> actually garbage' byte.
>
> As discussed offline, sprintf does terminate the string for us, so I
> think this is fine.
>
Forget I said anything /me hides in shame

>> This doesn't look good :-\
>>
>>> --- a/drivers/gpu/drm/drm_drv.c
>>> +++ b/drivers/gpu/drm/drm_drv.c
>>
>>> @@ -221,6 +222,14 @@ static int drm_minor_register(struct drm_device *dev, 
>>> unsigned int type)
>>> return ret;
>>> }
>>>
>>> +   if (type == DRM_MINOR_PRIMARY) {
>>> +   drm_for_each_crtc(crtc, dev) {
>>> +   ret = drm_debugfs_crtc_add(crtc);
>>> +   if (ret)
>>> +   DRM_ERROR("DRM: Failed to initialize CRC 
>>> debugfs.\n");
>>> +   }
>>> +   }
>>> +
>> Minor: We're missing teardown in the error path.
>
> Isn't drm_debugfs_cleanup taking care of it?
>
Maybe ? It goes through the debugfs_list and pulls down individual
files. Yet most/all of drm users of debugfs_create_dir explicitly
cleanup after themselves via debugfs_remove_recursive() so I'd play
along.

Thanks
Emil

[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

2016-09-08 Thread Mario Kleiner

On 09/08/2016 08:30 AM, Chris Wilson wrote:
> On Thu, Sep 08, 2016 at 02:14:43AM +0200, Mario Kleiner wrote:
>> amdgpu-kms uses shared fences for its prime exported dmabufs,
>> instead of an exclusive fence. Therefore we need to wait for
>> all fences of the dmabuf reservation object to prevent
>> unsynchronized rendering and flipping.
>
> No. Fix the root cause as this affects not just flips but copies -
> this implies that everybody using the resv object must wait for all
> fences. The resv object is not just used for prime, but all fencing, so
> this breaks the ability to schedule parallel operations across engine.
> -Chris
>

Ok. I think i now understand the difference, but let's check: The 
exclusive fence is essentially acting a bit like a write-lock, and the 
shared fences as readers-locks? So you can have multiple readers but 
only one writer at a time?

Ie.:

Writer must wait for all fences before starting write access to a 
buffer, then attach the exclusive fence and signal it on end of write 
access. E.g., write to renderbuffer, write to texture etc.

Readers must wait for exclusive fence, then attach a shared fence per 
reader and signal it on end of read access? E.g., read from texture, fb, 
scanout?

Is that correct? In that case we'd have a missing exclusive fence in 
amdgpu for the linear target dmabuf? Probably beyond my level of 
knowledge to fix this?

thanks,
-mario

[Bug 69076] [r300g] RS690: triangle flickering

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

--- Comment #13 from David Heidelberg (okias)  ---
great, it makes sense. Please post when patch will be incorporated into
Mesa-git tree

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/502e205c/attachment-0001.html>

[PATCH] drm/i915: Before pageflip, also wait for shared dmabuf fences.

On Thu, Sep 08, 2016 at 05:21:42PM +0200, Mario Kleiner wrote:
> On 09/08/2016 08:30 AM, Chris Wilson wrote:
> >On Thu, Sep 08, 2016 at 02:14:43AM +0200, Mario Kleiner wrote:
> >>amdgpu-kms uses shared fences for its prime exported dmabufs,
> >>instead of an exclusive fence. Therefore we need to wait for
> >>all fences of the dmabuf reservation object to prevent
> >>unsynchronized rendering and flipping.
> >
> >No. Fix the root cause as this affects not just flips but copies -
> >this implies that everybody using the resv object must wait for all
> >fences. The resv object is not just used for prime, but all fencing, so
> >this breaks the ability to schedule parallel operations across engine.
> >-Chris
> >
> 
> Ok. I think i now understand the difference, but let's check: The
> exclusive fence is essentially acting a bit like a write-lock, and
> the shared fences as readers-locks? So you can have multiple readers
> but only one writer at a time?

That's how we (i915.ko and I hope the rest of the world) are using them.
In the model where here is just one reservation object on the GEM
object, that reservation object is then shared between internal driver
scheduling and external. We are reliant on being able to use buffers on
multiple engines through the virtue of the shared fences, and to serialise
after writes by waiting on the exclusive fence. (So we can have concurrent
reads on the display engine, render engines and on the CPU - or
alternatively an exclusive writer.)

In the near future, i915 flips will wait on the common reservation object
not only for dma-bufs, but also its own GEM objects.

> Ie.:
> 
> Writer must wait for all fences before starting write access to a
> buffer, then attach the exclusive fence and signal it on end of
> write access. E.g., write to renderbuffer, write to texture etc.

Yes.

> Readers must wait for exclusive fence, then attach a shared fence
> per reader and signal it on end of read access? E.g., read from
> texture, fb, scanout?

Yes.

> Is that correct? In that case we'd have a missing exclusive fence in
> amdgpu for the linear target dmabuf? Probably beyond my level of
> knowledge to fix this?

i915.ko requires the client to mark which buffers are written to.

In ttm, there are ttm_validate_buffer objects which mark whether they
should be using shared or exclusive fences. Afaict, in amdgpu they are
all set to shared, the relevant user interface seems to be
amdgpu_bo_list_set().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[Bug 97471] kworker consumes 100% of a cpu core when screen sleeps with amdgpu kernel driver.

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

--- Comment #3 from bugs.freedesktop.org at crystalgamma.de ---
I am hitting this bug on Carrizo as well.
In particular, it only takes effect if the disabled output is the integrated
screen (eDP-1). Disabling the DP-1 output (which is actually an HDMI port)
connected to an external screen has no visible effects.

My system is Arch Linux, with the default kernel (Vanilla Linux 4.7.2 with a
one-line patch changing the default log level).

I can provide additional information about my system if necessary.
I can also try out kernel patches if that will help.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/6e3dd999/attachment.html>

[PATCH 1/2] libdrm: add etnaviv drm support

2016-09-08 Thread Rob Herring

On Thu, Sep 1, 2016 at 2:08 PM, Christian Gmeiner
 wrote:
> Hi Emil,
>
> thanks a lot for the review.
>
> 2016-08-30 15:03 GMT+02:00 Emil Velikov :
>> On 30 August 2016 at 08:14, Christian Gmeiner
>>  wrote:
>>> From: The etnaviv authors 
>>>
>>> Add the libdrm_etnaviv helper library to encapsulate etnaviv-specific 
>>> interfaces to the DRM.
>>>
>>> Signed-off-by: Christian Gmeiner 
>>> Signed-off-by: Lucas Stach 
>> Just double-checking:
>>  - you've looked that all the relevant freedreno patches have been
>> ported over, correct ?
>>  - the feature checking bug (mentioned on IRC) has been fixed ?
>>
>>> diff --git a/configure.ac b/configure.ac
>>> index e3048c7..64f3e6c 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>
>>> @@ -274,6 +279,9 @@ if test "x$drm_cv_atomic_primitives" = "xnone"; then
>>>
>>> LIBDRM_ATOMICS_NOT_FOUND_MSG($TEGRA, tegra, NVIDIA Tegra, 
>>> tegra-experimental-api)
>>> TEGRA=no
>>> +
>>> +   LIBDRM_ATOMICS_NOT_FOUND_MSG($ETNAVIV, etnaviv, Vivante, 
>>> etnaviv-experimental-api)
>> Reading this hunk reminds me what a bad name I've used. Then again
>> nothing better comes up atm. If you can think of any please shout.
>>
>>> +++ b/etnaviv/Android.mk
>> Have you tried building/using etna on Android ?
>>
>
> No.. if it is an easy job I would give it a try. Shall I drop it?

But I have. libdrm just needs this patch (for master and N):

@@ -9,7 +9,7 @@ LOCAL_MODULE_TAGS := optional

 LOCAL_SHARED_LIBRARIES := libdrm

-LOCAL_SRC_FILES := $(LIBDRM_ETNAVIV_FILES)
+LOCAL_SRC_FILES := $(patsubst %.h, , $(LIBDRM_ETNAVIV_FILES))
 LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)

 LOCAL_CFLAGS := \


I've got mesa building on Android, too. It's a few patches so far of
Android.mk additions and things that break with clang or post 12.0.
The etnaviv branch also breaks other drivers with the max vertex
buffer capability addition.

Rob

[PATCH 1/2] libdrm: add etnaviv drm support

2016-09-08 Thread Rob Herring

On Thu, Sep 8, 2016 at 1:52 PM, Rob Herring  wrote:
> On Thu, Sep 1, 2016 at 2:08 PM, Christian Gmeiner
>  wrote:
>> Hi Emil,
>>
>> thanks a lot for the review.
>>
>> 2016-08-30 15:03 GMT+02:00 Emil Velikov :
>>> On 30 August 2016 at 08:14, Christian Gmeiner
>>>  wrote:
 From: The etnaviv authors 

 Add the libdrm_etnaviv helper library to encapsulate etnaviv-specific 
 interfaces to the DRM.

 Signed-off-by: Christian Gmeiner 
 Signed-off-by: Lucas Stach 
>>> Just double-checking:
>>>  - you've looked that all the relevant freedreno patches have been
>>> ported over, correct ?
>>>  - the feature checking bug (mentioned on IRC) has been fixed ?
>>>
 diff --git a/configure.ac b/configure.ac
 index e3048c7..64f3e6c 100644
 --- a/configure.ac
 +++ b/configure.ac
>>>
 @@ -274,6 +279,9 @@ if test "x$drm_cv_atomic_primitives" = "xnone"; then

 LIBDRM_ATOMICS_NOT_FOUND_MSG($TEGRA, tegra, NVIDIA Tegra, 
 tegra-experimental-api)
 TEGRA=no
 +
 +   LIBDRM_ATOMICS_NOT_FOUND_MSG($ETNAVIV, etnaviv, Vivante, 
 etnaviv-experimental-api)
>>> Reading this hunk reminds me what a bad name I've used. Then again
>>> nothing better comes up atm. If you can think of any please shout.
>>>
 +++ b/etnaviv/Android.mk
>>> Have you tried building/using etna on Android ?
>>>
>>
>> No.. if it is an easy job I would give it a try. Shall I drop it?
>
> But I have. libdrm just needs this patch (for master and N):

NM. I see you already have that change in this patch, so Android build
should be fine. I'll give v2 patch a try.

Rob

[PATCH v2 0/9] drm/sun4i: Introduce A33 display driver

On Tue, Sep 06, 2016 at 04:46:11PM +0200, Maxime Ripard wrote:
> Hi everyone,
> 
> This serie introduces the support in the sun4i-drm driver for the A33.
> 
> Beside the new IPs and special cases for the A33 new IPs, there's
> nothing really outstanding, and is now at feature parity with the A13.
> 
> This serie is based on my A33 CCU patches posted earlier today here:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2016-September/453208.html
> 
> Let me know what you think,
> Maxime

And I applied the patches 7 and 8.

We'll figure the panel stuff later.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/3317dcfb/attachment.sig>

[PATCH 1/2] libdrm: add etnaviv drm support

2016-09-08 Thread Christian Gmeiner

Hi Rob,

2016-09-08 20:52 GMT+02:00 Rob Herring :
> On Thu, Sep 1, 2016 at 2:08 PM, Christian Gmeiner
>  wrote:
>> Hi Emil,
>>
>> thanks a lot for the review.
>>
>> 2016-08-30 15:03 GMT+02:00 Emil Velikov :
>>> On 30 August 2016 at 08:14, Christian Gmeiner
>>>  wrote:
 From: The etnaviv authors 

 Add the libdrm_etnaviv helper library to encapsulate etnaviv-specific 
 interfaces to the DRM.

 Signed-off-by: Christian Gmeiner 
 Signed-off-by: Lucas Stach 
>>> Just double-checking:
>>>  - you've looked that all the relevant freedreno patches have been
>>> ported over, correct ?
>>>  - the feature checking bug (mentioned on IRC) has been fixed ?
>>>
 diff --git a/configure.ac b/configure.ac
 index e3048c7..64f3e6c 100644
 --- a/configure.ac
 +++ b/configure.ac
>>>
 @@ -274,6 +279,9 @@ if test "x$drm_cv_atomic_primitives" = "xnone"; then

 LIBDRM_ATOMICS_NOT_FOUND_MSG($TEGRA, tegra, NVIDIA Tegra, 
 tegra-experimental-api)
 TEGRA=no
 +
 +   LIBDRM_ATOMICS_NOT_FOUND_MSG($ETNAVIV, etnaviv, Vivante, 
 etnaviv-experimental-api)
>>> Reading this hunk reminds me what a bad name I've used. Then again
>>> nothing better comes up atm. If you can think of any please shout.
>>>
 +++ b/etnaviv/Android.mk
>>> Have you tried building/using etna on Android ?
>>>
>>
>> No.. if it is an easy job I would give it a try. Shall I drop it?
>
> But I have. libdrm just needs this patch (for master and N):
>

Great!

> @@ -9,7 +9,7 @@ LOCAL_MODULE_TAGS := optional
>
>  LOCAL_SHARED_LIBRARIES := libdrm
>
> -LOCAL_SRC_FILES := $(LIBDRM_ETNAVIV_FILES)
> +LOCAL_SRC_FILES := $(patsubst %.h, , $(LIBDRM_ETNAVIV_FILES))
>  LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)
>
>  LOCAL_CFLAGS := \
>
>
> I've got mesa building on Android, too. It's a few patches so far of
> Android.mk additions and things that break with clang or post 12.0.
> The etnaviv branch also breaks other drivers with the max vertex
> buffer capability addition.
>

Yeah I am aware of that and I am currently working on a fix for that.

thanks
--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner

[PATCH v2 1/2] mm: fix cache mode of dax pmd mappings

2016-09-08 Thread Andrew Morton

On Wed, 07 Sep 2016 15:26:14 -0700 Dan Williams  
wrote:

> track_pfn_insert() in vmf_insert_pfn_pmd() is marking dax mappings as
> uncacheable rendering them impractical for application usage.  DAX-pte
> mappings are cached and the goal of establishing DAX-pmd mappings is to
> attain more performance, not dramatically less (3 orders of magnitude).
> 
> track_pfn_insert() relies on a previous call to reserve_memtype() to
> establish the expected page_cache_mode for the range.  While memremap()
> arranges for reserve_memtype() to be called, devm_memremap_pages() does
> not.  So, teach track_pfn_insert() and untrack_pfn() how to handle
> tracking without a vma, and arrange for devm_memremap_pages() to
> establish the write-back-cache reservation in the memtype tree.

Acked-by: Andrew Morton 

I'll grab [2/2].

[Bug 97681] !Aleppo! Hp Printer(1844)(305)(5565)Support Phone Number Massachusetts,New Jersey,Connecticut,Vermont

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

Bug ID: 97681
   Summary: !Aleppo! Hp Printer(1844)(305)(5565)Support Phone
Number Massachusetts,New Jersey,Connecticut,Vermont
   Product: DRI
   Version: unspecified
  Hardware: Other
OS: All
Status: NEW
  Severity: normal
  Priority: medium
 Component: DRM/amdkfd
  Assignee: dri-devel at lists.freedesktop.org
  Reporter: cufutedato at polyfaust.com

Created attachment 126353
  --> https://bugs.freedesktop.org/attachment.cgi?id=126353&action=edit
HP+(1844-305-5565) printer helpline phone number USA  HP+(1844-305-5565)
printer tech ÈÅ±ppÆ ÅÅ¦ Number New York  HP+(1844-305-5565) pÅÄºÅÅ¢Ã«Å 
customer
service phone number California  HP+(1844-305-5565)

-- 
You are receiving this mail because:
You are the assignee for the bug.
-- next part --
An HTML attachment was scrubbed...
URL: 
<https://lists.freedesktop.org/archives/dri-devel/attachments/20160908/0ec6fe45/attachment.html>

[Bug 97681] !Aleppo! Hp Printer(1844)(305)(5565)Support Phone Number Massachusetts,New Jersey,Connecticut,Vermont