Re: [PATCH] dt-bindings: display: renesas: Add r8a774b1 support

2019-10-04 Thread Laurent Pinchart
Hi Biju,

Thank you for the patch.

On Mon, Sep 30, 2019 at 10:28:51AM +0100, Biju Das wrote:
> Document RZ/G2N (R8A774B1) SoC bindings.
> 
> Signed-off-by: Biju Das 

I really like how your RZ patches are simple, they're painless to
review, it's all very pleasurable :-)

Reviewed-by: Laurent Pinchart 

And applied to my tree.

> ---
>  Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt 
> b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt
> index db68041..819f3e3 100644
> --- a/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt
> +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt
> @@ -13,6 +13,7 @@ Required properties:
>  
>  - compatible : Shall contain one or more of
>- "renesas,r8a774a1-hdmi" for R8A774A1 (RZ/G2M) compatible HDMI TX
> +  - "renesas,r8a774b1-hdmi" for R8A774B1 (RZ/G2N) compatible HDMI TX
>- "renesas,r8a7795-hdmi" for R8A7795 (R-Car H3) compatible HDMI TX
>- "renesas,r8a7796-hdmi" for R8A7796 (R-Car M3-W) compatible HDMI TX
>- "renesas,r8a77965-hdmi" for R8A77965 (R-Car M3-N) compatible HDMI TX

-- 
Regards,

Laurent Pinchart


Re: [PATCH] drm/bridge: sii902x: Variable status in sii902x_connector_detect() could be uninitialized if regmap_read() fails

2019-10-04 Thread Laurent Pinchart
Hi Yizhuo,

Thank you for the patch.

On Sun, Sep 29, 2019 at 09:45:02PM -0700, Yizhuo wrote:
> In function sii902x_connector_detect(), variable "status" could be
> initialized if regmap_read() fails. However, "status" is used to

I assume you meant "could be uninitialized" ?

> decide the return value, which is potentially unsafe.
> 
> Signed-off-by: Yizhuo 
> ---
>  drivers/gpu/drm/bridge/sii902x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/sii902x.c 
> b/drivers/gpu/drm/bridge/sii902x.c
> index 38f75ac580df..afce64f51ff2 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -246,7 +246,7 @@ static enum drm_connector_status
>  sii902x_connector_detect(struct drm_connector *connector, bool force)
>  {
>   struct sii902x *sii902x = connector_to_sii902x(connector);
> - unsigned int status;
> + unsigned int status = 0;
>  
>   mutex_lock(&sii902x->mutex);

I'll add a bit more context:

>   regmap_read(sii902x->regmap, SII902X_INT_STATUS, &status);
>
>   mutex_unlock(&sii902x->mutex);
>
>   return (status & SII902X_PLUGGED_STATUS) ?
>  connector_status_connected : connector_status_disconnected;

If regmap read fails, shouldn't we return connector_status_unknown ?

-- 
Regards,

Laurent Pinchart


Re: [PATCH 0/4] Add RZ/G2N DU support

2019-10-04 Thread Laurent Pinchart
Hi Biju,

Thank you for the patches;

On Mon, Sep 30, 2019 at 10:15:01AM +0100, Biju Das wrote:
> This patch series aims to add binding/driver support for
> R8A774B1(a.k.a RZ/G2N) DU (which is very similar to the R8A77965 DU);
> it has one RGB output, one LVDS output and one HDMI output.
> 
> Biju Das (4):
>   dt-bindings: display: renesas: du: Document the r8a774b1 bindings
>   drm: rcar-du: Add R8A774B1 support
>   dt-bindings: display: renesas: lvds: Document r8a774b1 bindings
>   drm: rcar-du: lvds: Add r8a774b1 support

For the whole series,

Reviewed-by: Laurent Pinchart 

and applied to my tree.

>  .../bindings/display/bridge/renesas,lvds.txt   |  1 +
>  .../devicetree/bindings/display/renesas,du.txt |  2 ++
>  drivers/gpu/drm/rcar-du/rcar_du_drv.c  | 30 
> ++
>  drivers/gpu/drm/rcar-du/rcar_lvds.c|  1 +
>  4 files changed, 34 insertions(+)

-- 
Regards,

Laurent Pinchart


Re: [PATCHv2 2/7] drm/omap: avoid copy in mgr_fld_read/write

2019-10-04 Thread Laurent Pinchart
Hi Tomi,

Thank you for the patch.

On Mon, Sep 30, 2019 at 01:38:35PM +0300, Tomi Valkeinen wrote:
> Avoid unnecessary copy in mgr_fld_read/write by taking a pointer to the
> reg_resc and using that.
> 
> Signed-off-by: Tomi Valkeinen 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/dss/dispc.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dispc.c 
> b/drivers/gpu/drm/omapdrm/dss/dispc.c
> index 0dc0272569f6..3c9315b17ef2 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dispc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dispc.c
> @@ -365,17 +365,17 @@ static inline u32 dispc_read_reg(struct dispc_device 
> *dispc, u16 idx)
>  static u32 mgr_fld_read(struct dispc_device *dispc, enum omap_channel 
> channel,
>   enum mgr_reg_fields regfld)
>  {
> - const struct dispc_reg_field rfld = mgr_desc[channel].reg_desc[regfld];
> + const struct dispc_reg_field *rfld = 
> &mgr_desc[channel].reg_desc[regfld];
>  
> - return REG_GET(dispc, rfld.reg, rfld.high, rfld.low);
> + return REG_GET(dispc, rfld->reg, rfld->high, rfld->low);
>  }
>  
>  static void mgr_fld_write(struct dispc_device *dispc, enum omap_channel 
> channel,
> enum mgr_reg_fields regfld, int val)
>  {
> - const struct dispc_reg_field rfld = mgr_desc[channel].reg_desc[regfld];
> + const struct dispc_reg_field *rfld = 
> &mgr_desc[channel].reg_desc[regfld];
>  
> - REG_FLD_MOD(dispc, rfld.reg, val, rfld.high, rfld.low);
> + REG_FLD_MOD(dispc, rfld->reg, val, rfld->high, rfld->low);
>  }
>  
>  static int dispc_get_num_ovls(struct dispc_device *dispc)

-- 
Regards,

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

Re: [PATCHv2 7/7] drm/omap: hdmi4: fix use of uninitialized var

2019-10-04 Thread Laurent Pinchart
Hi Tomi,

Thank you for the patch.

On Mon, Sep 30, 2019 at 01:38:40PM +0300, Tomi Valkeinen wrote:
> If use_mclk is false, mclk_mode is written to a register without
> initialization. This doesn't cause any ill effects as the written value
> is not used when use_mclk is false.
> 
> To fix this, write use_mclk only when use_mclk is true.
> 
> Signed-off-by: Tomi Valkeinen 

Reviewed-by: Laurent Pinchart 

> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c 
> b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> index 5d5d5588ebc1..c4ffe96e28bc 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
> @@ -542,8 +542,9 @@ static void hdmi_core_audio_config(struct hdmi_core_data 
> *core,
>   }
>  
>   /* Set ACR clock divisor */
> - REG_FLD_MOD(av_base,
> - HDMI_CORE_AV_FREQ_SVAL, cfg->mclk_mode, 2, 0);
> + if (cfg->use_mclk)
> + REG_FLD_MOD(av_base, HDMI_CORE_AV_FREQ_SVAL,
> + cfg->mclk_mode, 2, 0);
>  
>   r = hdmi_read_reg(av_base, HDMI_CORE_AV_ACR_CTRL);
>   /*

-- 
Regards,

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

Re: [PATCH v1 2/2] drm/panel: simple: add display timings for logic technologies displays

2019-10-04 Thread Philippe Schenker
On Thu, 2019-10-03 at 10:59 -0500, Rob Herring wrote:
> On Wed, Oct 2, 2019 at 5:27 AM Philippe Schenker
>  wrote:
> > On Tue, 2019-10-01 at 17:05 -0500, Rob Herring wrote:
> > > On Fri, Sep 20, 2019 at 09:54:11AM +0200, Marcel Ziswiler wrote:
> > > > From: Marcel Ziswiler 
> > > > 
> > > > Add display timings for the following 3 display panels
> > > > manufactured
> > > > by
> > > > Logic Technologies Limited:
> > > > 
> > > > - LT161010-2NHC e.g. as found in the Toradex Capacitive Touch
> > > > Display
> > > >   7" Parallel [1]
> > > > - LT161010-2NHR e.g. as found in the Toradex Resistive Touch
> > > > Display
> > > > 7"
> > > >   Parallel [2]
> > > > - LT170410-2WHC e.g. as found in the Toradex Capacitive Touch
> > > > Display
> > > >   10.1" LVDS [3]
> > > > 
> > > > Those panels may also be distributed by Endrich Bauelemente
> > > > Vertriebs
> > > > GmbH [4].
> > > > 
> > > > [1]
> > > > https://docs.toradex.com/104497-7-inch-parallel-capacitive-touch-display-800x480-datasheet.pdf
> > > > [2]
> > > > https://docs.toradex.com/104498-7-inch-parallel-resistive-touch-display-800x480.pdf
> > > > [3]
> > > > https://docs.toradex.com/105952-10-1-inch-lvds-capacitive-touch-display-1280x800-datasheet.pdf
> > > > [4]
> > > > https://www.endrich.com/isi50_isi30_tft-displays/lt170410-1whc_isi30
> > > > 
> > > > Signed-off-by: Marcel Ziswiler 
> > > > 
> > > > ---
> > > > 
> > > >  drivers/gpu/drm/panel/panel-simple.c | 65
> > > > 
> > > >  1 file changed, 65 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/panel/panel-simple.c
> > > > b/drivers/gpu/drm/panel/panel-simple.c
> > > > index 28fa6ba7b767..42bd0de25167 100644
> > > > --- a/drivers/gpu/drm/panel/panel-simple.c
> > > > +++ b/drivers/gpu/drm/panel/panel-simple.c
> > > > @@ -2034,6 +2034,62 @@ static const struct panel_desc lg_lp129qe
> > > > = {
> > > > },
> > > >  };
> > > > 
> > > > +static const struct display_timing
> > > > logictechno_lt161010_2nh_timing
> > > > = {
> > > > +   .pixelclock = { 2640, 3330, 4680 },
> > > > +   .hactive = { 800, 800, 800 },
> > > > +   .hfront_porch = { 16, 210, 354 },
> > > > +   .hback_porch = { 46, 46, 46 },
> > > > +   .hsync_len = { 1, 20, 40 },
> > > > +   .vactive = { 480, 480, 480 },
> > > > +   .vfront_porch = { 7, 22, 147 },
> > > > +   .vback_porch = { 23, 23, 23 },
> > > > +   .vsync_len = { 1, 10, 20 },
> > > > +   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
> > > > +DISPLAY_FLAGS_DE_HIGH |
> > > > DISPLAY_FLAGS_PIXDATA_POSEDGE |
> > > > +DISPLAY_FLAGS_SYNC_POSEDGE,
> > > > +};
> > > > +
> > > > +static const struct panel_desc logictechno_lt161010_2nh = {
> > > > +   .timings = &logictechno_lt161010_2nh_timing,
> > > > +   .num_timings = 1,
> > > > +   .size = {
> > > > +   .width = 154,
> > > > +   .height = 86,
> > > > +   },
> > > > +   .bus_format = MEDIA_BUS_FMT_RGB666_1X18,
> > > > +   .bus_flags = DRM_BUS_FLAG_DE_HIGH |
> > > > +DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
> > > > +DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE,
> > > > +};
> > > > +
> > > > +static const struct display_timing
> > > > logictechno_lt170410_2whc_timing
> > > > = {
> > > > +   .pixelclock = { 6890, 7110, 734 },
> > > > +   .hactive = { 1280, 1280, 1280 },
> > > > +   .hfront_porch = { 23, 60, 71 },
> > > > +   .hback_porch = { 23, 60, 71 },
> > > > +   .hsync_len = { 15, 40, 47 },
> > > > +   .vactive = { 800, 800, 800 },
> > > > +   .vfront_porch = { 5, 7, 10 },
> > > > +   .vback_porch = { 5, 7, 10 },
> > > > +   .vsync_len = { 6, 9, 12 },
> > > > +   .flags = DISPLAY_FLAGS_HSYNC_LOW | DISPLAY_FLAGS_VSYNC_LOW |
> > > > +DISPLAY_FLAGS_DE_HIGH |
> > > > DISPLAY_FLAGS_PIXDATA_POSEDGE |
> > > > +DISPLAY_FLAGS_SYNC_POSEDGE,
> > > > +};
> > > > +
> > > > +static const struct panel_desc logictechno_lt170410_2whc = {
> > > > +   .timings = &logictechno_lt170410_2whc_timing,
> > > > +   .num_timings = 1,
> > > > +   .size = {
> > > > +   .width = 217,
> > > > +   .height = 136,
> > > > +   },
> > > > +   .bus_format = MEDIA_BUS_FMT_RGB888_1X7X4_SPWG,
> > > > +   .bus_flags = DRM_BUS_FLAG_DE_HIGH |
> > > > +DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE |
> > > > +DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE,
> > > > +};
> > > > +
> > > >  static const struct drm_display_mode mitsubishi_aa070mc01_mode
> > > > = {
> > > > .clock = 30400,
> > > > .hdisplay = 800,
> > > > @@ -3264,6 +3320,15 @@ static const struct of_device_id
> > > > platform_of_match[] = {
> > > > }, {
> > > > .compatible = "lg,lp129qe",
> > > > .data = &lg_lp129qe,
> > > > +   }, {
> > > > +   .compatible = "logictechno,lt161010-2nhc",
> > > > +   .data = &logictechno_lt161010_2nh,
> > > > +   }, {
> > > > +   .compatible = "logictechno,lt161010-2nhr",
> > > > +   .data = &logictechno_lt161010_2nh,
> > > > +   }

Re: [PATCH 09/11] lib/interval-tree: convert interval_tree to half closed intervals

2019-10-04 Thread Koenig, Christian
Am 04.10.19 um 08:57 schrieb Christian König:
> Am 03.10.19 um 22:18 schrieb Davidlohr Bueso:
>> The generic tree tree really wants [a, b) intervals, not fully closed.
>> As such convert it to use the new interval_tree_gen.h. Most of the
>> conversions are straightforward, with the exception of perhaps
>> radeon_vm_bo_set_addr(), but semantics have been tried to be left
>> untouched.
>
> NAK, the whole thing won't work.
>
> See we need to handle the full device address space which means we 
> have values in the range of 0x0-0x.
>
> If you make this a closed interval then the end would wrap around to 
> 0x0 if long is only 32bit.

Well I've just now re-read the subject line. From that it sounds like 
you are actually trying to fix the interval tree to use a half closed 
interval, e.g. something like [a, b[

But your code changes sometimes doesn't seem to reflect that.

Regards,
Christian.

>
> Regards,
> Christian.
>
>>
>> Cc: "Christian König" 
>> Cc: Alex Deucher 
>> Cc: David Airlie 
>> Cc: Daniel Vetter 
>> Cc: Doug Ledford 
>> Cc: Joerg Roedel 
>> Cc: "Jérôme Glisse" 
>> Cc: dri-devel@lists.freedesktop.org
>> Cc: linux-r...@vger.kernel.org
>> Signed-off-by: Davidlohr Bueso 
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c  | 12 +++-
>>   drivers/gpu/drm/i915/gem/i915_gem_userptr.c |  5 +
>>   drivers/gpu/drm/radeon/radeon_mn.c  | 11 ---
>>   drivers/gpu/drm/radeon/radeon_trace.h   |  2 +-
>>   drivers/gpu/drm/radeon/radeon_vm.c  | 26 
>> +-
>>   drivers/infiniband/core/umem_odp.c  | 21 +++--
>>   drivers/iommu/virtio-iommu.c    |  6 +++---
>>   include/linux/interval_tree.h   |  2 +-
>>   include/rdma/ib_umem_odp.h  |  4 ++--
>>   lib/interval_tree.c |  6 +++---
>>   10 files changed, 38 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> index 31d4deb5d294..4bbaa9ffa570 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mn.c
>> @@ -205,9 +205,6 @@ amdgpu_mn_sync_pagetables_gfx(struct hmm_mirror 
>> *mirror,
>>   bool blockable = mmu_notifier_range_blockable(update);
>>   struct interval_tree_node *it;
>>   -    /* notification is exclusive, but interval is inclusive */
>> -    end -= 1;
>> -
>>   /* TODO we should be able to split locking for interval tree and
>>    * amdgpu_mn_invalidate_node
>>    */
>> @@ -254,9 +251,6 @@ amdgpu_mn_sync_pagetables_hsa(struct hmm_mirror 
>> *mirror,
>>   bool blockable = mmu_notifier_range_blockable(update);
>>   struct interval_tree_node *it;
>>   -    /* notification is exclusive, but interval is inclusive */
>> -    end -= 1;
>> -
>>   if (amdgpu_mn_read_lock(amn, blockable))
>>   return -EAGAIN;
>>   @@ -374,7 +368,7 @@ struct amdgpu_mn *amdgpu_mn_get(struct 
>> amdgpu_device *adev,
>>    */
>>   int amdgpu_mn_register(struct amdgpu_bo *bo, unsigned long addr)
>>   {
>> -    unsigned long end = addr + amdgpu_bo_size(bo) - 1;
>> +    unsigned long end = addr + amdgpu_bo_size(bo);
>>   struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>   enum amdgpu_mn_type type =
>>   bo->kfd_bo ? AMDGPU_MN_TYPE_HSA : AMDGPU_MN_TYPE_GFX;
>> @@ -400,7 +394,7 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, 
>> unsigned long addr)
>>   node = container_of(it, struct amdgpu_mn_node, it);
>>   interval_tree_remove(&node->it, &amn->objects);
>>   addr = min(it->start, addr);
>> -    end = max(it->last, end);
>> +    end = max(it->end, end);
>>   list_splice(&node->bos, &bos);
>>   }
>>   @@ -412,7 +406,7 @@ int amdgpu_mn_register(struct amdgpu_bo *bo, 
>> unsigned long addr)
>>   bo->mn = amn;
>>     node->it.start = addr;
>> -    node->it.last = end;
>> +    node->it.end = end;
>>   INIT_LIST_HEAD(&node->bos);
>>   list_splice(&bos, &node->bos);
>>   list_add(&bo->mn_list, &node->bos);
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c 
>> b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> index 11b231c187c5..818ff6b33102 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_userptr.c
>> @@ -99,9 +99,6 @@ userptr_mn_invalidate_range_start(struct 
>> mmu_notifier *_mn,
>>   if (RB_EMPTY_ROOT(&mn->objects.rb_root))
>>   return 0;
>>   -    /* interval ranges are inclusive, but invalidate range is 
>> exclusive */
>> -    end = range->end - 1;
>> -
>>   spin_lock(&mn->lock);
>>   it = interval_tree_iter_first(&mn->objects, range->start, end);
>>   while (it) {
>> @@ -275,7 +272,7 @@ i915_gem_userptr_init__mmu_notifier(struct 
>> drm_i915_gem_object *obj,
>>   mo->mn = mn;
>>   mo->obj = obj;
>>   mo->it.start = obj->userptr.ptr;
>> -    mo->it.last = obj->userptr.ptr + obj->b

Re: [PATCH][next] drm/amdgpu: fix uninitialized variable pasid_mapping_needed

2019-10-04 Thread Koenig, Christian
Am 03.10.19 um 23:52 schrieb Colin King:
> From: Colin Ian King 
>
> The boolean variable pasid_mapping_needed is not initialized and
> there are code paths that do not assign it any value before it is
> is read later.  Fix this by initializing pasid_mapping_needed to
> false.
>
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: 6817bf283b2b ("drm/amdgpu: grab the id mgr lock while accessing 
> passid_mapping")
> Signed-off-by: Colin Ian King 

Reviewed-by: Christian König 

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index a2c797e34a29..be10e4b9a94d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1055,7 +1055,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct 
> amdgpu_job *job,
>   id->oa_size != job->oa_size);
>   bool vm_flush_needed = job->vm_needs_flush;
>   struct dma_fence *fence = NULL;
> - bool pasid_mapping_needed;
> + bool pasid_mapping_needed = false;
>   unsigned patch_offset = 0;
>   int r;
>   

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

Re: [PATCH][next] drm/amdgpu: remove redundant variable r and redundant return statement

2019-10-04 Thread Koenig, Christian
Am 03.10.19 um 23:40 schrieb Colin King:
> From: Colin Ian King 
>
> There is a return statement that is not reachable and a variable that
> is not used.  Remove them.
>
> Addresses-Coverity: ("Structurally dead code")
> Fixes: de7b45babd9b ("drm/amdgpu: cleanup creating BOs at fixed location 
> (v2)")
> Signed-off-by: Colin Ian King 

Reviewed-by: Christian König 

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 --
>   1 file changed, 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 481e4c381083..814159f15633 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1636,7 +1636,6 @@ static void amdgpu_ttm_fw_reserve_vram_fini(struct 
> amdgpu_device *adev)
>   static int amdgpu_ttm_fw_reserve_vram_init(struct amdgpu_device *adev)
>   {
>   uint64_t vram_size = adev->gmc.visible_vram_size;
> - int r;
>   
>   adev->fw_vram_usage.va = NULL;
>   adev->fw_vram_usage.reserved_bo = NULL;
> @@ -1651,7 +1650,6 @@ static int amdgpu_ttm_fw_reserve_vram_init(struct 
> amdgpu_device *adev)
> AMDGPU_GEM_DOMAIN_VRAM,
> &adev->fw_vram_usage.reserved_bo,
> &adev->fw_vram_usage.va);
> - return r;
>   }
>   
>   /**

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

Re: [PATCH 6/6] [RESEND] drm/amdgpu: work around llvm bug #42576

2019-10-04 Thread Nathan Chancellor
On Wed, Oct 02, 2019 at 09:51:37AM -0700, 'Nick Desaulniers' via Clang Built 
Linux wrote:
> > Apparently this bug is still present in both the released clang-9
> > and the current development version of clang-10.
> > I was hoping we would not need a workaround in clang-9+, but
> > it seems that we do.
> 
> I think I'd rather:
> 1. mark AMDGPU BROKEN if CC_IS_CLANG. There are numerous other issues building
>a working driver here.

The only reason I am not thrilled about this is we will lose out on
warning coverage while the compiler bug gets fixed. I think the AMDGPU
drivers have been the single biggest source of clang warnings.

I think something like:

depends on CC_IS_GCC || (CC_IS_CLANG && COMPILE_TEST)

would end up avoiding the runtime issues and give us warning coverage.
The only issue is that we would still need this patch...

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

Re: [PATCH 6/6] [RESEND] drm/amdgpu: work around llvm bug #42576

2019-10-04 Thread Nick Desaulniers
> Apparently this bug is still present in both the released clang-9
> and the current development version of clang-10.
> I was hoping we would not need a workaround in clang-9+, but
> it seems that we do.

I think I'd rather:
1. mark AMDGPU BROKEN if CC_IS_CLANG. There are numerous other issues building
   a working driver here.
2. Fix the compiler bug.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/11] drm/amdgpu: convert amdgpu_vm_it to half closed intervals

2019-10-04 Thread Davidlohr Bueso
The amdgpu_vm interval tree really wants [a, b) intervals,
not fully closed ones. As such convert it to use the new
interval_tree_gen.h, and also rename the 'last' endpoint
in the node to 'end', which is both a more suitable name
for the half closed interval and also reduces the chances
of missing a conversion when doing insertion or lookup.

Cc: Jerome Glisse 
Cc: Alex Deucher 
Cc: "Christian König" 
Cc: Daniel Vetter 
Cc: amd-...@lists.freedesktop.org
Signed-off-by: Davidlohr Bueso 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_object.h |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h  | 18 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c|  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vce.c|  3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 46 +++---
 6 files changed, 36 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
index 49b767b7238f..290bfe820890 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
@@ -756,7 +756,7 @@ static int amdgpu_cs_vm_handling(struct amdgpu_cs_parser *p)
}
 
if ((va_start + chunk_ib->ib_bytes) >
-   (m->last + 1) * AMDGPU_GPU_PAGE_SIZE) {
+   m->end * AMDGPU_GPU_PAGE_SIZE) {
DRM_ERROR("IB va_start+ib_bytes is invalid\n");
return -EINVAL;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
index 7e99f6c58c48..60b73bc4d11a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.h
@@ -51,7 +51,7 @@ struct amdgpu_bo_va_mapping {
struct list_headlist;
struct rb_node  rb;
uint64_tstart;
-   uint64_tlast;
+   uint64_tend;
uint64_t__subtree_last;
uint64_toffset;
uint64_tflags;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
index 8227ebd0f511..c5b0e88d019c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_trace.h
@@ -247,7 +247,7 @@ TRACE_EVENT(amdgpu_vm_bo_map,
TP_STRUCT__entry(
 __field(struct amdgpu_bo *, bo)
 __field(long, start)
-__field(long, last)
+__field(long, end)
 __field(u64, offset)
 __field(u64, flags)
 ),
@@ -255,12 +255,12 @@ TRACE_EVENT(amdgpu_vm_bo_map,
TP_fast_assign(
   __entry->bo = bo_va ? bo_va->base.bo : NULL;
   __entry->start = mapping->start;
-  __entry->last = mapping->last;
+  __entry->end = mapping->end;
   __entry->offset = mapping->offset;
   __entry->flags = mapping->flags;
   ),
-   TP_printk("bo=%p, start=%lx, last=%lx, offset=%010llx, flags=%llx",
- __entry->bo, __entry->start, __entry->last,
+   TP_printk("bo=%p, start=%lx, end=%lx, offset=%010llx, flags=%llx",
+ __entry->bo, __entry->start, __entry->end,
  __entry->offset, __entry->flags)
 );
 
@@ -271,7 +271,7 @@ TRACE_EVENT(amdgpu_vm_bo_unmap,
TP_STRUCT__entry(
 __field(struct amdgpu_bo *, bo)
 __field(long, start)
-__field(long, last)
+__field(long, end)
 __field(u64, offset)
 __field(u64, flags)
 ),
@@ -279,12 +279,12 @@ TRACE_EVENT(amdgpu_vm_bo_unmap,
TP_fast_assign(
   __entry->bo = bo_va ? bo_va->base.bo : NULL;
   __entry->start = mapping->start;
-  __entry->last = mapping->last;
+  __entry->end = mapping->end;
   __entry->offset = mapping->offset;
   __entry->flags = mapping->flags;
   ),
-   TP_printk("bo=%p, start=%lx, last=%lx, offset=%010llx, flags=%llx",
- __entry->bo, __entry->start, __entry->last,
+   TP_printk("bo=%p, start=%lx, end=%lx, offset=%010llx, flags=%llx",
+ __entry->bo, __entry->start, __entry->end,
  __entry->offset, __entry->flags)
 );
 
@@ -299,7 +299,7 @@ D

RE: [PATCH] dt-bindings: display: renesas: Add r8a774b1 support

2019-10-04 Thread Biju Das
Hi Laurent,

Thanks for the feedback.

> Subject: Re: [PATCH] dt-bindings: display: renesas: Add r8a774b1 support
> 
> Hi Biju,
> 
> Thank you for the patch.
> 
> On Mon, Sep 30, 2019 at 10:28:51AM +0100, Biju Das wrote:
> > Document RZ/G2N (R8A774B1) SoC bindings.
> >
> > Signed-off-by: Biju Das 
> 
> I really like how your RZ patches are simple, they're painless to review, 
> it's all
> very pleasurable :-)

Yes I agree, It is because of the good work done by you and your team.

Regards,
Biju

> Reviewed-by: Laurent Pinchart 
> 
> And applied to my tree.
> 
> > ---
> >  Documentation/devicetree/bindings/display/bridge/renesas,dw-hdmi.txt
> > | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git
> > a/Documentation/devicetree/bindings/display/bridge/renesas,dw-
> hdmi.txt
> > b/Documentation/devicetree/bindings/display/bridge/renesas,dw-
> hdmi.txt
> > index db68041..819f3e3 100644
> > ---
> > a/Documentation/devicetree/bindings/display/bridge/renesas,dw-
> hdmi.txt
> > +++ b/Documentation/devicetree/bindings/display/bridge/renesas,dw-
> hdmi
> > +++ .txt
> > @@ -13,6 +13,7 @@ Required properties:
> >
> >  - compatible : Shall contain one or more of
> >- "renesas,r8a774a1-hdmi" for R8A774A1 (RZ/G2M) compatible HDMI TX
> > +  - "renesas,r8a774b1-hdmi" for R8A774B1 (RZ/G2N) compatible HDMI TX
> >- "renesas,r8a7795-hdmi" for R8A7795 (R-Car H3) compatible HDMI TX
> >- "renesas,r8a7796-hdmi" for R8A7796 (R-Car M3-W) compatible HDMI TX
> >- "renesas,r8a77965-hdmi" for R8A77965 (R-Car M3-N) compatible HDMI
> > TX
> 
> --
> Regards,
> 
> Laurent Pinchart


Re: [Spice-devel] Xorg indefinitely hangs in kernelspace

2019-10-04 Thread Hillf Danton

On Thu, 3 Oct 2019 09:45:55 +0300 Jaak Ristioja wrote:
> On 30.09.19 16:29, Frediano Ziglio wrote:
> >   Why didn't you update bug at 
> > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1813620?
> > I know it can seem tedious but would help tracking it.
> 
> I suppose the lack on centralized tracking and handling of Linux kernel
> bugs is a delicate topic, so I don't want to rant much more on that.
> Updating that bug would tedious and time-consuming indeed, which is why
> I haven't done that. To be honest, I don't have enough time and motivation.

Give the diff below a go only when it is convenient and only if it makes
a bit of sense to you.

--- a/drivers/gpu/drm/ttm/ttm_execbuf_util.c
+++ b/drivers/gpu/drm/ttm/ttm_execbuf_util.c
@@ -110,6 +110,7 @@ int ttm_eu_reserve_buffers(struct ww_acq
ww_acquire_init(ticket, &reservation_ww_class);
 
list_for_each_entry(entry, list, head) {
+   bool lockon = false;
struct ttm_buffer_object *bo = entry->bo;
 
ret = __ttm_bo_reserve(bo, intr, (ticket == NULL), ticket);
@@ -150,6 +151,7 @@ int ttm_eu_reserve_buffers(struct ww_acq
dma_resv_lock_slow(bo->base.resv, ticket);
ret = 0;
}
+   lockon = !ret;
}
 
if (!ret && entry->num_shared)
@@ -157,6 +159,8 @@ int ttm_eu_reserve_buffers(struct ww_acq

entry->num_shared);
 
if (unlikely(ret != 0)) {
+   if (lockon)
+   dma_resv_unlock(bo->base.resv);
if (ret == -EINTR)
ret = -ERESTARTSYS;
if (ticket) {

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

Re: [PATCH 6/6] [RESEND] drm/amdgpu: work around llvm bug #42576

2019-10-04 Thread Nick Desaulniers
On Wed, Oct 2, 2019 at 10:07 AM Nathan Chancellor
 wrote:
>
> On Wed, Oct 02, 2019 at 09:51:37AM -0700, 'Nick Desaulniers' via Clang Built 
> Linux wrote:
> > > Apparently this bug is still present in both the released clang-9
> > > and the current development version of clang-10.
> > > I was hoping we would not need a workaround in clang-9+, but
> > > it seems that we do.

Here's a fix: https://reviews.llvm.org/D68356
Can't possibly land until clang-10 though.

> >
> > I think I'd rather:
> > 1. mark AMDGPU BROKEN if CC_IS_CLANG. There are numerous other issues 
> > building
> >a working driver here.
>
> The only reason I am not thrilled about this is we will lose out on
> warning coverage while the compiler bug gets fixed. I think the AMDGPU
> drivers have been the single biggest source of clang warnings.
>
> I think something like:
>
> depends on CC_IS_GCC || (CC_IS_CLANG && COMPILE_TEST)
>
> would end up avoiding the runtime issues and give us warning coverage.
> The only issue is that we would still need this patch...
>
> Cheers,
> Nathan



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

[PATCH v3] drm/amd/display: fix struct init in update_bounding_box

2019-10-04 Thread Raul E Rangel
dcn20_resource.c:2636:9: error: missing braces around initializer 
[-Werror=missing-braces]
  struct _vcs_dpi_voltage_scaling_st calculated_states[MAX_CLOCK_LIMIT_STATES] 
= {0};
 ^

Fixes: 7ed4e6352c16f ("drm/amd/display: Add DCN2 HW Sequencer and Resource")

Signed-off-by: Raul E Rangel 

---

Changes in v3:
- Use memset

Changes in v2:
- Use {{0}} instead of {}

 drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
index b949e202d6cb7..f72c26ae41def 100644
--- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
+++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
@@ -2633,7 +2633,7 @@ static void cap_soc_clocks(
 static void update_bounding_box(struct dc *dc, struct 
_vcs_dpi_soc_bounding_box_st *bb,
struct pp_smu_nv_clock_table *max_clocks, unsigned int 
*uclk_states, unsigned int num_states)
 {
-   struct _vcs_dpi_voltage_scaling_st 
calculated_states[MAX_CLOCK_LIMIT_STATES] = {0};
+   struct _vcs_dpi_voltage_scaling_st 
calculated_states[MAX_CLOCK_LIMIT_STATES];
int i;
int num_calculated_states = 0;
int min_dcfclk = 0;
@@ -2641,6 +2641,8 @@ static void update_bounding_box(struct dc *dc, struct 
_vcs_dpi_soc_bounding_box_
if (num_states == 0)
return;
 
+   memset(calculated_states, 0, sizeof(calculated_states));
+
if (dc->bb_overrides.min_dcfclk_mhz > 0)
min_dcfclk = dc->bb_overrides.min_dcfclk_mhz;
else
-- 
2.23.0.444.g18eeb5a265-goog

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

Re: [PATCH v4 7/7] backlight: gpio: pull gpio_backlight_initial_power_state() into probe

2019-10-04 Thread Bartosz Golaszewski
śr., 2 paź 2019 o 16:40 Daniel Thompson  napisał(a):
>
> On Wed, Oct 02, 2019 at 01:46:17PM +0200, Bartosz Golaszewski wrote:
> > śr., 2 paź 2019 o 12:33 Daniel Thompson  
> > napisał(a):
> > >
> > > On Tue, Oct 01, 2019 at 02:58:37PM +0200, Bartosz Golaszewski wrote:
> > > > From: Bartosz Golaszewski 
> > > >
> > > > The probe function in the gpio-backlight driver is quite short. If we
> > > > pull gpio_backlight_initial_power_state() into probe we can drop two
> > > > more fields from struct gpio_backlight and shrink the driver code.
> > > >
> > > > Signed-off-by: Bartosz Golaszewski 
> > > > ---
> > > >  drivers/video/backlight/gpio_backlight.c | 36 
> > > >  1 file changed, 12 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/video/backlight/gpio_backlight.c 
> > > > b/drivers/video/backlight/gpio_backlight.c
> > > > index 6247687b6330..37ec184f0c5c 100644
> > > > --- a/drivers/video/backlight/gpio_backlight.c
> > > > +++ b/drivers/video/backlight/gpio_backlight.c
> > > > @@ -17,11 +17,8 @@
> > > >  #include 
> > > >
> > > >  struct gpio_backlight {
> > > > - struct device *dev;
> > > >   struct device *fbdev;
> > > > -
> > > >   struct gpio_desc *gpiod;
> > > > - int def_value;
> > > >  };
> > > >
> > > >  static int gpio_backlight_update_status(struct backlight_device *bl)
> > > > @@ -53,41 +50,24 @@ static const struct backlight_ops 
> > > > gpio_backlight_ops = {
> > > >   .check_fb   = gpio_backlight_check_fb,
> > > >  };
> > > >
> > > > -static int gpio_backlight_initial_power_state(struct gpio_backlight 
> > > > *gbl)
> > >
> > > I'm inclined to view deleting this function as removing a comment (e.g.
> > > the function name helps us to read the code)!
> > >
> >
> > Right, but why not just add a comment then?
>
> I guess you could add a comment but keeping it pulled out in a function
> makes it easier to compare against equivalent code in other drivers
> (such as pwm_bl).
>

The pwm driver seems to be the only one that has this function and
it's also much more complicated. Unless it's a hard NACK, I think that
pulling all initialization into probe looks better and shrinks the
driver visually.

Bart

>
> Daniel.
>
>
> > The probe function is 50
> > lines long, there's really no need to split it. This will get inlined
> > anyway too.
> >
> > Bart
> >
> > > Removing the variables from the context structure is good but why not
> > > just pass them to the function and let the compiler decided whether or
> > > not to inline.
> > >
> > >
> > > Daniel.
> > >
> > >
> > > > -{
> > > > - struct device_node *node = gbl->dev->of_node;
> > > > -
> > > > - /* Not booted with device tree or no phandle link to the node */
> > > > - if (!node || !node->phandle)
> > > > - return gbl->def_value ? FB_BLANK_UNBLANK : 
> > > > FB_BLANK_POWERDOWN;
> > > > -
> > > > - /* if the enable GPIO is disabled, do not enable the backlight */
> > > > - if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
> > > > - return FB_BLANK_POWERDOWN;
> > > > -
> > > > - return FB_BLANK_UNBLANK;
> > > > -}
> > > > -
> > > > -
> > > >  static int gpio_backlight_probe(struct platform_device *pdev)
> > > >  {
> > > >   struct device *dev = &pdev->dev;
> > > >   struct gpio_backlight_platform_data *pdata = 
> > > > dev_get_platdata(dev);
> > > > + struct device_node *of_node = dev->of_node;
> > > >   struct backlight_properties props;
> > > >   struct backlight_device *bl;
> > > >   struct gpio_backlight *gbl;
> > > > - int ret;
> > > > + int ret, def_value;
> > > >
> > > >   gbl = devm_kzalloc(dev, sizeof(*gbl), GFP_KERNEL);
> > > >   if (gbl == NULL)
> > > >   return -ENOMEM;
> > > >
> > > > - gbl->dev = dev;
> > > > -
> > > >   if (pdata)
> > > >   gbl->fbdev = pdata->fbdev;
> > > >
> > > > - gbl->def_value = device_property_read_bool(dev, "default-on");
> > > > + def_value = device_property_read_bool(dev, "default-on");
> > > >
> > > >   gbl->gpiod = devm_gpiod_get(dev, NULL, GPIOD_ASIS);
> > > >   if (IS_ERR(gbl->gpiod)) {
> > > > @@ -109,7 +89,15 @@ static int gpio_backlight_probe(struct 
> > > > platform_device *pdev)
> > > >   return PTR_ERR(bl);
> > > >   }
> > > >
> > > > - bl->props.power = gpio_backlight_initial_power_state(gbl);
> > > > + /* Not booted with device tree or no phandle link to the node */
> > > > + if (!of_node || !of_node->phandle)
> > > > + bl->props.power = def_value ? FB_BLANK_UNBLANK
> > > > + : FB_BLANK_POWERDOWN;
> > > > + else if (gpiod_get_value_cansleep(gbl->gpiod) == 0)
> > > > + bl->props.power = FB_BLANK_POWERDOWN;
> > > > + else
> > > > + bl->props.power = FB_BLANK_UNBLANK;
> > > > +
> > > >   bl->props.brightness = 1;
> > > >
> > > >   backlight_update_status(bl);
> > > > --
> > > > 2.23.0
> > 

[PATCH] dt-bindings: display: Convert stm32 display bindings to json-schema

2019-10-04 Thread Benjamin Gaignard
Convert the STM32 display binding to DT schema format using json-schema.
Split the original bindings in two yaml files:
- one for display controller (ltdc)
- one for DSI controller

Signed-off-by: Benjamin Gaignard 
---
 .../devicetree/bindings/display/st,stm32-dsi.yaml  | 130 +++
 .../devicetree/bindings/display/st,stm32-ltdc.txt  | 144 -
 .../devicetree/bindings/display/st,stm32-ltdc.yaml |  82 
 3 files changed, 212 insertions(+), 144 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/display/st,stm32-dsi.yaml
 delete mode 100644 Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
 create mode 100644 Documentation/devicetree/bindings/display/st,stm32-ltdc.yaml

diff --git a/Documentation/devicetree/bindings/display/st,stm32-dsi.yaml 
b/Documentation/devicetree/bindings/display/st,stm32-dsi.yaml
new file mode 100644
index ..8143cf6f0ce7
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/st,stm32-dsi.yaml
@@ -0,0 +1,130 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/st,stm32-dsi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32 DSI host controller
+
+maintainers:
+  - Philippe Cornu 
+  - Yannick Fertre 
+
+properties:
+  "#address-cells": true
+  "#size-cells": true
+
+  compatible:
+const: st,stm32-dsi
+
+  reg:
+maxItems: 1
+
+  clocks:
+items:
+  - description: Module Clock
+  - description: DSI bus clock
+  - description: Pixel clock
+minItems: 2
+maxItems: 3
+
+  clock-names:
+items:
+  - const: pclk
+  - const: ref
+  - const: px_clk
+minItems: 2
+maxItems: 3
+
+  resets:
+maxItems: 1
+
+  reset-names:
+items:
+  - const: apb
+
+  phy-dsi-supply:
+maxItems: 1
+description:
+Phandle of the regulator that provides the supply voltage.
+
+  ports:
+type: object
+description:
+A node containing DSI input & output port nodes with endpoint 
+definitions as documented in
+Documentation/devicetree/bindings/media/video-interfaces.txt
+Documentation/devicetree/bindings/graph.txt
+
+  port:
+type: object
+description:
+  "A port node with endpoint definitions as defined in
+  Documentation/devicetree/bindings/media/video-interfaces.txt.
+  port@0: DSI input port node, connected to the ltdc rgb output port.
+  port@1: DSI output port node, connected to a panel or a bridge input 
port"
+
+required:
+  - "#address-cells"
+  - "#size-cells"
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - ports
+
+examples:
+  - |
+#include 
+#include 
+#include 
+#include 
+dsi: dsi@5a00 {
+compatible = "st,stm32-dsi";
+reg = <0x5a00 0x800>;
+clocks = <&rcc DSI_K>, <&clk_hse>, <&rcc DSI_PX>;
+clock-names = "pclk", "ref", "px_clk";
+resets = <&rcc DSI_R>;
+reset-names = "apb";
+phy-dsi-supply = <®18>;
+
+#address-cells = <1>;
+#size-cells = <0>;
+
+ports {
+  #address-cells = <1>;
+  #size-cells = <0>;
+
+  port@0 {
+reg = <0>;
+dsi_in: endpoint {
+remote-endpoint = <;
+};
+  };
+
+  port@1 {
+reg = <1>;
+dsi_out: endpoint {
+remote-endpoint = <&panel_in>;
+};
+  };
+};
+
+panel {
+  compatible = "orisetech,otm8009a";
+  reg = <0>;
+  reset-gpios = <&gpioe 4 GPIO_ACTIVE_LOW>;
+  power-supply = <&v3v3>;
+
+  port {
+panel_in: endpoint {
+remote-endpoint = <&dsi_out>;
+};
+  };
+};
+};
+
+...
+
+
diff --git a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt 
b/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
deleted file mode 100644
index 60c54da4e526..
--- a/Documentation/devicetree/bindings/display/st,stm32-ltdc.txt
+++ /dev/null
@@ -1,144 +0,0 @@
-* STMicroelectronics STM32 lcd-tft display controller
-
-- ltdc: lcd-tft display controller host
-  Required properties:
-  - compatible: "st,stm32-ltdc"
-  - reg: Physical base address of the IP registers and length of memory mapped 
region.
-  - clocks: A list of phandle + clock-specifier pairs, one for each
-entry in 'clock-names'.
-  - clock-names: A list of clock names. For ltdc it should contain:
-  - "lcd" for the clock feeding the output pixel clock & IP clock.
-  - resets: reset to be used by the device (defined by use of RCC macro).
-  Required nodes:
-  - Video port for DPI RGB output: ltdc has one video port with up to 2
-endpoints:
-  - for e

Re: [PATCH 4/6] drm/amd/display: fix dcn21 Makefile for clang

2019-10-04 Thread Nick Desaulniers
On Wed, Oct 2, 2019 at 5:03 AM Arnd Bergmann  wrote:
>
> Just like all the other variants, this one passes invalid
> compile-time options with clang after the new code got
> merged:
>
> clang: error: unknown argument: '-mpreferred-stack-boundary=4'
> scripts/Makefile.build:265: recipe for target 
> 'drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o' failed
>
> Use the same variant that we have for dcn20 to fix compilation.
>
> Fixes: eced51f9babb ("drm/amd/display: Add hubp block for Renoir (v2)")
> Signed-off-by: Arnd Bergmann 

Thanks for the patch!
Reviewed-by: Nick Desaulniers 
Tested-by: Nick Desaulniers 
(Though I think it's already been merged)

Alex, do you know why the AMDGPU driver uses a different stack
alignment (16B) than the rest of the x86 kernel?  (see
arch/x86/Makefile which uses 8B stack alignment).

> ---
>  drivers/gpu/drm/amd/display/dc/dcn21/Makefile | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile 
> b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> index 8cd9de8b1a7a..ef673bffc241 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> +++ b/drivers/gpu/drm/amd/display/dc/dcn21/Makefile
> @@ -3,7 +3,17 @@
>
>  DCN21 = dcn21_hubp.o dcn21_hubbub.o dcn21_resource.o
>
> -CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse 
> -mpreferred-stack-boundary=4
> +ifneq ($(call cc-option, -mpreferred-stack-boundary=4),)
> +   cc_stack_align := -mpreferred-stack-boundary=4
> +else ifneq ($(call cc-option, -mstack-alignment=16),)
> +   cc_stack_align := -mstack-alignment=16
> +endif
> +
> +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o := -mhard-float -msse 
> $(cc_stack_align)
> +
> +ifdef CONFIG_CC_IS_CLANG
> +CFLAGS_$(AMDDALPATH)/dc/dcn21/dcn21_resource.o += -msse2
> +endif
>
>  AMD_DAL_DCN21 = $(addprefix $(AMDDALPATH)/dc/dcn21/,$(DCN21))
>
> --
> 2.20.0
>
> --
> You received this message because you are subscribed to the Google Groups 
> "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to clang-built-linux+unsubscr...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/clang-built-linux/20191002120136.1777161-5-arnd%40arndb.de.



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

Re: [PATCH 4/6] drm/amd/display: fix dcn21 Makefile for clang

2019-10-04 Thread Nick Desaulniers
On Wed, Oct 2, 2019 at 2:24 PM Alex Deucher  wrote:
>
> On Wed, Oct 2, 2019 at 5:19 PM Nick Desaulniers  
> wrote:
> >
> > Alex, do you know why the AMDGPU driver uses a different stack
> > alignment (16B) than the rest of the x86 kernel?  (see
> > arch/x86/Makefile which uses 8B stack alignment).
>
> Not sure.  Maybe Harry can comment.  I think it was added for the
> floating point stuff.  Not sure if it's strictly required or not.

Can you find out for me please who knows more about this and setup a
chat with all of us? (I don't want to deride this patch's review
thread, so let's start a new thread once we know more) We're facing
some interesting runtime issues when built with Clang.

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

[Bug 111900] clutterrerddd

2019-10-04 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111900

Andre Klapper  changed:

   What|Removed |Added

 Resolution|--- |INVALID
 Status|ASSIGNED|RESOLVED
Version|XOrg git|unspecified
  Group||spam
Product|DRI |Spam
  Component|General |Two

--- Comment #1 from Andre Klapper  ---
Go away and test somewhere else.

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

[Bug 111902] too much trees

2019-10-04 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111902

Andre Klapper  changed:

   What|Removed |Added

 Resolution|--- |INVALID
 Status|ASSIGNED|RESOLVED
Version|XOrg git|unspecified
Product|DRI |Spam
  Group||spam
  Component|General |Two

--- Comment #1 from Andre Klapper  ---
Go away and test somewhere else.

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

[git pull] drm fixes for 5.4-rc2

2019-10-04 Thread Dave Airlie
Hey Linus,

Been offline for 3 days, got back and had some fixes queued up,
nothing too major, the i915 dp-mst fix is important, and amdgpu has a
bulk move speedup fix and some regressions, but nothing too insane for
an rc2 pull. The intel fixes are also 2 weeks worth, they missed the
boat last week.

Dave.

drm-fixes-2019-10-04:
drm fixes for 5.4-rc2

core:
- writeback fixes

i915:
- Fix DP-MST crtc_mask
- Fix dsc dpp calculations
- Fix g4x sprite scaling stride check with GTT remapping
- Fix concurrence on cases where requests where getting retired at
same time as resubmitted to HW
- Fix gen9 display resolutions by setting the right max plane width
- Fix GPU hang on preemption
- Mark contents as dirty on a write fault. This was breaking cursor
sprite with dumb buffers.

komeda:
- memory leak fix

tilcdc:
- include fix

amdgpu:
- Enable bulk moves
- Power metrics fixes for Navi
- Fix S4 regression
- Add query for tcc disabled mask
- Fix several leaks in error paths
- randconfig fixes
- clang fixes
The following changes since commit 54ecb8f7028c5eb3d740bb82b0f1d90f2df63c5c:

  Linux 5.4-rc1 (2019-09-30 10:35:40 -0700)

are available in the Git repository at:

  git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2019-10-04

for you to fetch changes up to 07bba341c99694a4fe6b07edfa4f97ca90c8784c:

  Merge tag 'drm-intel-fixes-2019-10-03-1' of
git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2019-10-04
16:31:06 +1000)


drm fixes for 5.4-rc2

core:
- writeback fixes

i915:
- Fix DP-MST crtc_mask
- Fix dsc dpp calculations
- Fix g4x sprite scaling stride check with GTT remapping
- Fix concurrence on cases where requests where getting retired at
same time as resubmitted to HW
- Fix gen9 display resolutions by setting the right max plane width
- Fix GPU hang on preemption
- Mark contents as dirty on a write fault. This was breaking cursor
sprite with dumb buffers.

komeda:
- memory leak fix

tilcdc:
- include fix

amdgpu:
- Enable bulk moves
- Power metrics fixes for Navi
- Fix S4 regression
- Add query for tcc disabled mask
- Fix several leaks in error paths
- randconfig fixes
- clang fixes


Aaron Liu (1):
  Revert "drm/amdgpu: disable stutter mode for renoir"

Alex Deucher (1):
  drm/amdgpu: don't increment vram lost if we are in hibernation

Arnd Bergmann (6):
  drm/tilcdc: include linux/pinctrl/consumer.h again
  drm/amdgpu: make pmu support optional, again
  drm/amdgpu: hide another #warning
  drm/amdgpu: display_mode_vba_21: remove uint typedef
  drm/amd/display: hide an unused variable
  drm/amd/display: fix dcn21 Makefile for clang

Christian König (1):
  drm/amdgpu: revert "disable bulk moves for now"

Dave Airlie (3):
  Merge tag 'drm-fixes-5.4-2019-10-02' of
git://people.freedesktop.org/~agd5f/linux into drm-fixes
  Merge tag 'drm-misc-fixes-2019-10-03' of
git://anongit.freedesktop.org/drm/drm-misc into drm-fixes
  Merge tag 'drm-intel-fixes-2019-10-03-1' of
git://anongit.freedesktop.org/drm/drm-intel into drm-fixes

Kevin Wang (2):
  drm/amd/powerplay: change metrics update period from 1ms to 100ms
  drm/amd/powerplay: add sensor lock support for smu

Lowry Li (Arm Technology China) (2):
  drm: Free the writeback_job when it with an empty fb
  drm: Clear the fence pointer when writeback job signaled

Maarten Lankhorst (1):
  drm/i915/dp: Fix dsc bpp calculations, v5.

Marek Olšák (1):
  drm/amdgpu: return tcc_disabled_mask to userspace

Maxime Ripard (2):
  Merge drm/drm-fixes into drm-misc-fixes
  Merge drm-misc-next-fixes-2019-10-02 into drm-misc-fixes

Navid Emamdoost (3):
  drm/komeda: prevent memory leak in komeda_wb_connector_add
  drm/amdgpu: fix multiple memory leaks in acp_hw_init
  drm/amd/display: memory leak

Tomi Valkeinen (1):
  drm/omap: fix max fclk divider for omap36xx

Ville Syrjälä (2):
  drm/i915: Fix g4x sprite scaling stride check with GTT remapping
  Revert "drm/i915: Fix DP-MST crtc_mask"

 drivers/gpu/drm/amd/amdgpu/Makefile|   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_acp.c|  34 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c|   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h|   1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c|   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c |   2 -
 drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c |  12 ++
 drivers/gpu/drm/amd/amdgpu/nv.c|   6 +-
 drivers/gpu/drm/amd/amdgpu/soc15.c |   8 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |   4 +-
 .../drm/amd/display/dc/dce100/dce100_resource.c|   1 +
 .../drm/amd/display/dc/dce110/dce110_resource.c|   1 +
 .../drm/amd/display/dc/dce112/dce112_resource.c|   1 +
 .../drm/amd/display/dc/dce120/dce120_resource.c|   1 +
 .../gpu/drm/

[PATCH] drm: sti: fix spelling mistake: rejec -> rejection

2019-10-04 Thread Colin King
From: Colin Ian King 

In other places of the driver the string hdmi_rejection_pll is
used instead of the truncated hdmi_rejec_pll, so use this string
instead to be consistent.

Signed-off-by: Colin Ian King 
---
 drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 814560ead4e1..e2018e4a3ec5 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -886,7 +886,7 @@ static void sti_hdmi_pre_enable(struct drm_bridge *bridge)
if (clk_prepare_enable(hdmi->clk_tmds))
DRM_ERROR("Failed to prepare/enable hdmi_tmds clk\n");
if (clk_prepare_enable(hdmi->clk_phy))
-   DRM_ERROR("Failed to prepare/enable hdmi_rejec_pll clk\n");
+   DRM_ERROR("Failed to prepare/enable hdmi_rejection_pll clk\n");
 
hdmi->enabled = true;
 
-- 
2.20.1



Re: [Intel-gfx] [PATCH 2/5] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)

2019-10-04 Thread Tvrtko Ursulin


On 03/10/2019 22:00, Chris Wilson wrote:

Make dma_fence_enable_sw_signaling() behave like its
dma_fence_add_callback() and dma_fence_default_wait() counterparts and
perform the test to enable signaling under the fence->lock, along with
the action to do so. This ensure that should an implementation be trying
to flush the cb_list (by signaling) on retirement before freeing the
fence, it can do so in a race-free manner.

See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
with dma_fence_signal").

v2: Refactor all 3 enable_signaling paths to use a common function.

Signed-off-by: Chris Wilson 
---
  drivers/dma-buf/dma-fence.c | 78 +
  1 file changed, 35 insertions(+), 43 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 2c136aee3e79..b58528c1cc9d 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -273,6 +273,30 @@ void dma_fence_free(struct dma_fence *fence)
  }
  EXPORT_SYMBOL(dma_fence_free);
  
+static bool __dma_fence_enable_signaling(struct dma_fence *fence)

+{
+   bool was_set;
+
+   lockdep_assert_held(fence->lock);
+
+   was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+  &fence->flags);
+
+   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+   return false;
+
+   if (!was_set && fence->ops->enable_signaling) {
+   if (!fence->ops->enable_signaling(fence)) {
+   dma_fence_signal_locked(fence);
+   return false;
+   }
+
+   trace_dma_fence_enable_signal(fence);


Tracepoint used to come before the enable_signaling call so probably 
best to keep it like that.



+   }
+
+   return true;
+}
+
  /**
   * dma_fence_enable_sw_signaling - enable signaling on fence
   * @fence: the fence to enable
@@ -285,19 +309,12 @@ void dma_fence_enable_sw_signaling(struct dma_fence 
*fence)
  {
unsigned long flags;
  
-	if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,

- &fence->flags) &&
-   !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
-   fence->ops->enable_signaling) {
-   trace_dma_fence_enable_signal(fence);
-
-   spin_lock_irqsave(fence->lock, flags);
-
-   if (!fence->ops->enable_signaling(fence))
-   dma_fence_signal_locked(fence);
+   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+   return;
  
-		spin_unlock_irqrestore(fence->lock, flags);

-   }
+   spin_lock_irqsave(fence->lock, flags);
+   __dma_fence_enable_signaling(fence);
+   spin_unlock_irqrestore(fence->lock, flags);
  }
  EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
  
@@ -331,7 +348,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,

  {
unsigned long flags;
int ret = 0;
-   bool was_set;
  
  	if (WARN_ON(!fence || !func))

return -EINVAL;
@@ -343,25 +359,14 @@ int dma_fence_add_callback(struct dma_fence *fence, 
struct dma_fence_cb *cb,
  
  	spin_lock_irqsave(fence->lock, flags);
  
-	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,

-  &fence->flags);
-
-   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
-   ret = -ENOENT;
-   else if (!was_set && fence->ops->enable_signaling) {
-   trace_dma_fence_enable_signal(fence);
-
-   if (!fence->ops->enable_signaling(fence)) {
-   dma_fence_signal_locked(fence);
-   ret = -ENOENT;
-   }
-   }
-
-   if (!ret) {
+   if (__dma_fence_enable_signaling(fence)) {
cb->func = func;
list_add_tail(&cb->node, &fence->cb_list);
-   } else
+   } else {
INIT_LIST_HEAD(&cb->node);
+   ret = -ENOENT;
+   }
+
spin_unlock_irqrestore(fence->lock, flags);
  
  	return ret;

@@ -461,7 +466,6 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, 
signed long timeout)
struct default_wait_cb cb;
unsigned long flags;
signed long ret = timeout ? timeout : 1;
-   bool was_set;
  
  	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))

return ret;
@@ -473,21 +477,9 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, 
signed long timeout)
goto out;
}
  
-	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,

-  &fence->flags);
-
-   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+   if (!__dma_fence_enable_signaling(fence))
goto out;
  
-	if (!was_set && fence->ops->enable_signaling) {

-   trace_dma_fence_enable_signal(fence);
-
-   if (!fence->ops->enable_signaling(fence)) {
-

Re: [Intel-gfx] [PATCH 3/5] drm/mm: Use helpers for drm_mm_node booleans

2019-10-04 Thread Tvrtko Ursulin


On 03/10/2019 22:00, Chris Wilson wrote:

In preparation for rearranging the booleans into a flags field, ensure
all the current users are using the inline helpers and not directly
accessing the members.

Signed-off-by: Chris Wilson 
---
  drivers/gpu/drm/drm_mm.c  | 19 ---
  .../gpu/drm/i915/gem/i915_gem_execbuffer.c|  4 ++--
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  |  2 +-
  drivers/gpu/drm/i915/i915_gem.c   | 12 ++--
  drivers/gpu/drm/i915/i915_gem_evict.c |  2 +-
  drivers/gpu/drm/i915/i915_vma.c   |  2 +-
  drivers/gpu/drm/i915/i915_vma.h   |  4 ++--
  drivers/gpu/drm/selftests/test-drm_mm.c   | 14 +++---
  drivers/gpu/drm/vc4/vc4_crtc.c|  2 +-
  drivers/gpu/drm/vc4/vc4_hvs.c |  2 +-
  drivers/gpu/drm/vc4/vc4_plane.c   |  4 ++--
  11 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 4581c5387372..99312bdc6273 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -174,7 +174,7 @@ static void drm_mm_interval_tree_add_node(struct 
drm_mm_node *hole_node,
  
  	node->__subtree_last = LAST(node);
  
-	if (hole_node->allocated) {

+   if (drm_mm_node_allocated(hole_node)) {
rb = &hole_node->rb;
while (rb) {
parent = rb_entry(rb, struct drm_mm_node, rb);
@@ -561,6 +561,11 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
  }
  EXPORT_SYMBOL(drm_mm_insert_node_in_range);
  
+static inline bool drm_mm_node_scanned_block(const struct drm_mm_node *node)

+{
+   return node->scanned_block;
+}
+
  /**
   * drm_mm_remove_node - Remove a memory node from the allocator.
   * @node: drm_mm_node to remove
@@ -574,8 +579,8 @@ void drm_mm_remove_node(struct drm_mm_node *node)
struct drm_mm *mm = node->mm;
struct drm_mm_node *prev_node;
  
-	DRM_MM_BUG_ON(!node->allocated);

-   DRM_MM_BUG_ON(node->scanned_block);
+   DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
+   DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
  
  	prev_node = list_prev_entry(node, node_list);
  
@@ -605,7 +610,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)

  {
struct drm_mm *mm = old->mm;
  
-	DRM_MM_BUG_ON(!old->allocated);

+   DRM_MM_BUG_ON(!drm_mm_node_allocated(old));
  
  	*new = *old;
  
@@ -731,8 +736,8 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,

u64 adj_start, adj_end;
  
  	DRM_MM_BUG_ON(node->mm != mm);

-   DRM_MM_BUG_ON(!node->allocated);
-   DRM_MM_BUG_ON(node->scanned_block);
+   DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
+   DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
node->scanned_block = true;
mm->scan_active++;
  
@@ -818,7 +823,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,

struct drm_mm_node *prev_node;
  
  	DRM_MM_BUG_ON(node->mm != scan->mm);

-   DRM_MM_BUG_ON(!node->scanned_block);
+   DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));
node->scanned_block = false;
  
  	DRM_MM_BUG_ON(!node->mm->scan_active);

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 27dbcb508055..20d8a6297985 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -968,7 +968,7 @@ static void reloc_cache_reset(struct reloc_cache *cache)
intel_gt_flush_ggtt_writes(ggtt->vm.gt);
io_mapping_unmap_atomic((void __iomem *)vaddr);
  
-		if (cache->node.allocated) {

+   if (drm_mm_node_allocated(&cache->node)) {
ggtt->vm.clear_range(&ggtt->vm,
 cache->node.start,
 cache->node.size);
@@ -1061,7 +1061,7 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj,
}
  
  	offset = cache->node.start;

-   if (cache->node.allocated) {
+   if (drm_mm_node_allocated(&cache->node)) {
ggtt->vm.insert_page(&ggtt->vm,
 i915_gem_object_get_dma_address(obj, page),
 offset, I915_CACHE_NONE, 0);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c 
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index bb878119f06c..bb4889d2346d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -387,7 +387,7 @@ static u32 uc_fw_ggtt_offset(struct intel_uc_fw *uc_fw, 
struct i915_ggtt *ggtt)
  {
struct drm_mm_node *node = &ggtt->uc_fw;
  
-	GEM_BUG_ON(!node->allocated);

+   GEM_BUG_ON(!drm_mm_node_allocated(node));
GEM_BUG_ON(upper_32_bits(node->start));
GEM_BUG_ON(upper_32_bits(node->start + node->size - 1));
  
diff --git a/drivers/gpu/drm/i915/i915_

Re: [Intel-gfx] [PATCH 4/5] drm/mm: Convert drm_mm_node booleans to bitops

2019-10-04 Thread Tvrtko Ursulin


On 03/10/2019 22:00, Chris Wilson wrote:

A straightforward conversion of assignment and checking of the boolean
state flags (allocated, scanned) into non-atomic bitops. The caller
remains responsible for all locking around the drm_mm and its nodes.

Signed-off-by: Chris Wilson 
---
  drivers/gpu/drm/drm_mm.c   | 18 +-
  drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c |  2 +-
  drivers/gpu/drm/i915/i915_gem.c|  4 ++--
  include/drm/drm_mm.h   |  7 ---
  4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index 99312bdc6273..a9cab5e53731 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -426,7 +426,7 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct 
drm_mm_node *node)
  
  	list_add(&node->node_list, &hole->node_list);

drm_mm_interval_tree_add_node(hole, node);
-   node->allocated = true;
+   __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
node->hole_size = 0;
  
  	rm_hole(hole);

@@ -545,7 +545,7 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
  
  		list_add(&node->node_list, &hole->node_list);

drm_mm_interval_tree_add_node(hole, node);
-   node->allocated = true;
+   __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
  
  		rm_hole(hole);

if (adj_start > hole_start)
@@ -563,7 +563,7 @@ EXPORT_SYMBOL(drm_mm_insert_node_in_range);
  
  static inline bool drm_mm_node_scanned_block(const struct drm_mm_node *node)

  {
-   return node->scanned_block;
+   return test_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
  }
  
  /**

@@ -589,7 +589,7 @@ void drm_mm_remove_node(struct drm_mm_node *node)
  
  	drm_mm_interval_tree_remove(node, &mm->interval_tree);

list_del(&node->node_list);
-   node->allocated = false;
+   __clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
  
  	if (drm_mm_hole_follows(prev_node))

rm_hole(prev_node);
@@ -627,8 +627,8 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct 
drm_mm_node *new)
&mm->holes_addr);
}
  
-	old->allocated = false;

-   new->allocated = true;
+   __clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
+   __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags);
  }
  EXPORT_SYMBOL(drm_mm_replace_node);
  
@@ -738,7 +738,7 @@ bool drm_mm_scan_add_block(struct drm_mm_scan *scan,

DRM_MM_BUG_ON(node->mm != mm);
DRM_MM_BUG_ON(!drm_mm_node_allocated(node));
DRM_MM_BUG_ON(drm_mm_node_scanned_block(node));
-   node->scanned_block = true;
+   __set_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
mm->scan_active++;
  
  	/* Remove this block from the node_list so that we enlarge the hole

@@ -824,7 +824,7 @@ bool drm_mm_scan_remove_block(struct drm_mm_scan *scan,
  
  	DRM_MM_BUG_ON(node->mm != scan->mm);

DRM_MM_BUG_ON(!drm_mm_node_scanned_block(node));
-   node->scanned_block = false;
+   __clear_bit(DRM_MM_NODE_SCANNED_BIT, &node->flags);
  
  	DRM_MM_BUG_ON(!node->mm->scan_active);

node->mm->scan_active--;
@@ -922,7 +922,7 @@ void drm_mm_init(struct drm_mm *mm, u64 start, u64 size)
  
  	/* Clever trick to avoid a special case in the free hole tracking. */

INIT_LIST_HEAD(&mm->head_node.node_list);
-   mm->head_node.allocated = false;
+   mm->head_node.flags = 0;
mm->head_node.mm = mm;
mm->head_node.start = start + size;
mm->head_node.size = -size;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index 20d8a6297985..c049199a1df5 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -906,7 +906,7 @@ static void reloc_cache_init(struct reloc_cache *cache,
cache->use_64bit_reloc = HAS_64BIT_RELOC(i915);
cache->has_fence = cache->gen < 4;
cache->needs_unfenced = INTEL_INFO(i915)->unfenced_needs_alignment;
-   cache->node.allocated = false;
+   cache->node.flags = 0;
cache->ce = NULL;
cache->rq = NULL;
cache->rq_size = 0;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fa8e028ac0b5..7046067f70c1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -351,7 +351,7 @@ i915_gem_gtt_pread(struct drm_i915_gem_object *obj,
   PIN_NOEVICT);
if (!IS_ERR(vma)) {
node.start = i915_ggtt_offset(vma);
-   node.allocated = false;
+   node.flags = 0;
} else {
ret = insert_mappable_node(ggtt, &node, PAGE_SIZE);
if (ret)
@@ -561,7 +561,7 @@ i915_gem_gtt_pwrite_fast(struct drm_i915_gem_object *obj,
   PIN_NOEVICT);
   

Re: [PATCH v7 7/9] drm: tegra: use cec_notifier_conn_(un)register

2019-10-04 Thread Hans Verkuil
Hi Thierry,

Just a reminder: this patch hasn't been merged yet for v5.5.

Thanks!

Hans

On 8/28/19 1:54 PM, Thierry Reding wrote:
> On Wed, Aug 28, 2019 at 12:06:34PM +0200, Hans Verkuil wrote:
>> On 8/28/19 11:38 AM, Thierry Reding wrote:
>>> On Wed, Aug 28, 2019 at 10:09:30AM +0200, Hans Verkuil wrote:
 Thierry,

 Can you review this patch?

 Thanks!

Hans
>>>
>>> Did you want me to pick this up into the drm/tegra tree? Or do you want
>>> to pick it up into your tree?
>>
>> Can you pick it up for the next cycle? As you mentioned, we missed the
>> deadline for 5.4, so this feature won't be enabled in the public CEC API
>> until 5.5.
>>
>> Thanks!
> 
> Sure, will do.
> 
> Thierry
> 



Re: [Intel-gfx] [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()

2019-10-04 Thread Tvrtko Ursulin


On 03/10/2019 22:01, Chris Wilson wrote:

A few callers need to serialise the destruction of their drm_mm_node and
ensure it is removed from the drm_mm before freeing. However, to be
completely sure that any access from another thread is complete before
we free the struct, we require the RELEASE semantics of
clear_bit_unlock().

This allows the conditional locking such as

Thread AThread B
 mutex_lock(mm_lock);if (drm_mm_node_allocated(node)) {
 drm_mm_node_remove(node);   mutex_lock(mm_lock);
 mutex_unlock(mm_lock);  drm_mm_node_remove(node);
 mutex_unlock(mm_lock);
  }
  kfree(node);


My understanding is that release semantics on node allocated mean 1 -> 0 
transition is guaranteed to be seen only when thread A 
drm_mm_node_remove is fully done with the removal. But if it is in the 
middle of removal, node is still seen as allocated outside and thread B 
can enter the if-body, wait for the lock, and then drm_mm_node_remove 
will attempt a double removal. So I think another drm_mm_node_allocated 
under the lock is needed.


But the fkree part looks correct. There can be no false negatives with 
the release semantics on the allocated bit.


Regards,

Tvrtko


to serialise correctly without any lingering accesses from A to the
freed node. Allocation / insertion of the node is assumed never to race
with removal or eviction scanning.

Signed-off-by: Chris Wilson 
---
  drivers/gpu/drm/drm_mm.c | 11 ++-
  1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_mm.c b/drivers/gpu/drm/drm_mm.c
index a9cab5e53731..2a6e34663146 100644
--- a/drivers/gpu/drm/drm_mm.c
+++ b/drivers/gpu/drm/drm_mm.c
@@ -424,9 +424,9 @@ int drm_mm_reserve_node(struct drm_mm *mm, struct 
drm_mm_node *node)
  
  	node->mm = mm;
  
+	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);

list_add(&node->node_list, &hole->node_list);
drm_mm_interval_tree_add_node(hole, node);
-   __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
node->hole_size = 0;
  
  	rm_hole(hole);

@@ -543,9 +543,9 @@ int drm_mm_insert_node_in_range(struct drm_mm * const mm,
node->color = color;
node->hole_size = 0;
  
+		__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);

list_add(&node->node_list, &hole->node_list);
drm_mm_interval_tree_add_node(hole, node);
-   __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
  
  		rm_hole(hole);

if (adj_start > hole_start)
@@ -589,11 +589,12 @@ void drm_mm_remove_node(struct drm_mm_node *node)
  
  	drm_mm_interval_tree_remove(node, &mm->interval_tree);

list_del(&node->node_list);
-   __clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
  
  	if (drm_mm_hole_follows(prev_node))

rm_hole(prev_node);
add_hole(prev_node);
+
+   clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &node->flags);
  }
  EXPORT_SYMBOL(drm_mm_remove_node);
  
@@ -614,6 +615,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)
  
  	*new = *old;
  
+	__set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags);

list_replace(&old->node_list, &new->node_list);
rb_replace_node_cached(&old->rb, &new->rb, &mm->interval_tree);
  
@@ -627,8 +629,7 @@ void drm_mm_replace_node(struct drm_mm_node *old, struct drm_mm_node *new)

&mm->holes_addr);
}
  
-	__clear_bit(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);

-   __set_bit(DRM_MM_NODE_ALLOCATED_BIT, &new->flags);
+   clear_bit_unlock(DRM_MM_NODE_ALLOCATED_BIT, &old->flags);
  }
  EXPORT_SYMBOL(drm_mm_replace_node);
  


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

Re: [PATCH] backlight: pwm_bl: Add missing curly branches in else branch

2019-10-04 Thread Daniel Thompson
On Thu, Oct 03, 2019 at 02:35:02PM -0700, Matthias Kaehlcke wrote:
> Add curly braces to an 'else' branch in pwm_backlight_update_status()
> to match the corresponding 'if' branch.
> 
> Signed-off-by: Matthias Kaehlcke 

Reviewed-by: Daniel Thompson 

> ---
> 
>  drivers/video/backlight/pwm_bl.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/video/backlight/pwm_bl.c 
> b/drivers/video/backlight/pwm_bl.c
> index 746eebc411df..130abff2705f 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -125,8 +125,9 @@ static int pwm_backlight_update_status(struct 
> backlight_device *bl)
>   state.duty_cycle = compute_duty_cycle(pb, brightness);
>   pwm_apply_state(pb->pwm, &state);
>   pwm_backlight_power_on(pb);
> - } else
> + } else {
>   pwm_backlight_power_off(pb);
> + }
>  
>   if (pb->notify_after)
>   pb->notify_after(pb->dev, brightness);
> -- 
> 2.23.0.444.g18eeb5a265-goog
> 


Re: [Intel-gfx] [PATCH 1/5] drm/i915/execlists: Skip redundant resubmission

2019-10-04 Thread Tvrtko Ursulin


On 03/10/2019 22:00, Chris Wilson wrote:

If we unwind the active requests, and on resubmission discover that we
intend to preempt the active context with itself, simply skip the ELSP
submission.

Signed-off-by: Chris Wilson 
---
  drivers/gpu/drm/i915/gt/intel_lrc.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c 
b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 431d3b8c3371..3cfea1758fd2 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1739,11 +1739,26 @@ static void execlists_dequeue(struct intel_engine_cs 
*engine)
  
  	if (submit) {

*port = execlists_schedule_in(last, port - execlists->pending);
-   memset(port + 1, 0, (last_port - port) * sizeof(*port));
execlists->switch_priority_hint =
switch_prio(engine, *execlists->pending);
+
+   /*
+* Skip if we ended up with exactly the same set of requests,
+* e.g. trying to timeslice a pair of ordered contexts
+*/
+   if (!memcmp(execlists->active, execlists->pending,
+   (port - execlists->pending + 1) * sizeof(*port))) {
+   do
+   execlists_schedule_out(fetch_and_zero(port));
+   while (port-- != execlists->pending);
+
+   goto skip_submit;
+   }
+
+   memset(port + 1, 0, (last_port - port) * sizeof(*port));
execlists_submit_ports(engine);
} else {
+skip_submit:
ring_set_paused(engine, 0);
}
  }



A little bit of tracepoint noise with in/out but looks correct after I 
read the new code. And I wonder if trace.pl still works after last 
couple months of refactors.. :)


Reviewed-by: Tvrtko Ursulin 

Regards,

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

[PATCH] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)

2019-10-04 Thread Chris Wilson
Make dma_fence_enable_sw_signaling() behave like its
dma_fence_add_callback() and dma_fence_default_wait() counterparts and
perform the test to enable signaling under the fence->lock, along with
the action to do so. This ensure that should an implementation be trying
to flush the cb_list (by signaling) on retirement before freeing the
fence, it can do so in a race-free manner.

See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
with dma_fence_signal").

v2: Refactor all 3 enable_signaling paths to use a common function.
v3: Don't argue, just keep the tracepoint in the existing spot.

Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
---
 drivers/dma-buf/dma-fence.c | 78 +
 1 file changed, 35 insertions(+), 43 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 2c136aee3e79..052a41e2451c 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -273,6 +273,30 @@ void dma_fence_free(struct dma_fence *fence)
 }
 EXPORT_SYMBOL(dma_fence_free);
 
+static bool __dma_fence_enable_signaling(struct dma_fence *fence)
+{
+   bool was_set;
+
+   lockdep_assert_held(fence->lock);
+
+   was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+  &fence->flags);
+
+   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+   return false;
+
+   if (!was_set && fence->ops->enable_signaling) {
+   trace_dma_fence_enable_signal(fence);
+
+   if (!fence->ops->enable_signaling(fence)) {
+   dma_fence_signal_locked(fence);
+   return false;
+   }
+   }
+
+   return true;
+}
+
 /**
  * dma_fence_enable_sw_signaling - enable signaling on fence
  * @fence: the fence to enable
@@ -285,19 +309,12 @@ void dma_fence_enable_sw_signaling(struct dma_fence 
*fence)
 {
unsigned long flags;
 
-   if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
- &fence->flags) &&
-   !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
-   fence->ops->enable_signaling) {
-   trace_dma_fence_enable_signal(fence);
-
-   spin_lock_irqsave(fence->lock, flags);
-
-   if (!fence->ops->enable_signaling(fence))
-   dma_fence_signal_locked(fence);
+   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+   return;
 
-   spin_unlock_irqrestore(fence->lock, flags);
-   }
+   spin_lock_irqsave(fence->lock, flags);
+   __dma_fence_enable_signaling(fence);
+   spin_unlock_irqrestore(fence->lock, flags);
 }
 EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
 
@@ -331,7 +348,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct 
dma_fence_cb *cb,
 {
unsigned long flags;
int ret = 0;
-   bool was_set;
 
if (WARN_ON(!fence || !func))
return -EINVAL;
@@ -343,25 +359,14 @@ int dma_fence_add_callback(struct dma_fence *fence, 
struct dma_fence_cb *cb,
 
spin_lock_irqsave(fence->lock, flags);
 
-   was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
-  &fence->flags);
-
-   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
-   ret = -ENOENT;
-   else if (!was_set && fence->ops->enable_signaling) {
-   trace_dma_fence_enable_signal(fence);
-
-   if (!fence->ops->enable_signaling(fence)) {
-   dma_fence_signal_locked(fence);
-   ret = -ENOENT;
-   }
-   }
-
-   if (!ret) {
+   if (__dma_fence_enable_signaling(fence)) {
cb->func = func;
list_add_tail(&cb->node, &fence->cb_list);
-   } else
+   } else {
INIT_LIST_HEAD(&cb->node);
+   ret = -ENOENT;
+   }
+
spin_unlock_irqrestore(fence->lock, flags);
 
return ret;
@@ -461,7 +466,6 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, 
signed long timeout)
struct default_wait_cb cb;
unsigned long flags;
signed long ret = timeout ? timeout : 1;
-   bool was_set;
 
if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
return ret;
@@ -473,21 +477,9 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, 
signed long timeout)
goto out;
}
 
-   was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
-  &fence->flags);
-
-   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+   if (!__dma_fence_enable_signaling(fence))
goto out;
 
-   if (!was_set && fence->ops->enable_signaling) {
-   trace_dma_fence_enable_signal(fence);
-
-   if (!fence->ops->enable_signaling(fence)) {
-   dma_fence_signal_

Re: Should regulator core support parsing OF based fwnode?

2019-10-04 Thread Jean-Jacques Hiblot


On 03/10/2019 22:27, Jacek Anaszewski wrote:

On 10/3/19 9:41 PM, Mark Brown wrote:

On Thu, Oct 03, 2019 at 09:21:06PM +0200, Jacek Anaszewski wrote:

On 10/3/19 8:35 PM, Mark Brown wrote:

On Thu, Oct 03, 2019 at 07:43:17PM +0200, Jacek Anaszewski wrote:

On 10/3/19 2:47 PM, Jean-Jacques Hiblot wrote:

On 03/10/2019 12:42, Sebastian Reichel wrote:

On Thu, Oct 03, 2019 at 10:28:09AM +0200, Jean-Jacques Hiblot wrote:

This mail has nothing relevant in the subject line and pages of quotes
before the question for me, it's kind of lucky I noticed it

Isn't it all about creating proper filters?

My point there is that there's nothing obvious in the mail that suggests
it should get past filters - just being CCed on a mail isn't super
reliable, people often get pulled in due to things like checkpatch or
someone copying a CC list from an earlier patch series where there were
things were relevant.

OK, updated the subject.


I wonder if it wouldn't make sense to add support for fwnode
parsing to regulator core. Or maybe it is either somehow supported
or not supported on purpose?

Anything attempting to use the regulator DT bindings in ACPI has very
serious problems, ACPI has its own power model which isn't compatible
with that used in DT.

We have a means for checking if fwnode refers to of_node:
is_of_node(const struct fwnode_handle *fwnode)
Couldn't it be employed for OF case?

Why would we want to do that?  We'd continue to support only DT systems,
just with code that's less obviously DT only and would need to put
checks in.  I'm not seeing an upside here.

For instance few weeks ago we had a patch [0] in the LED core switching
from using struct device's of_node property to fwnode for conveying
device property data. And this transition to fwnode property API can be
observed as a frequent pattern across subsystems.

Recently there is an ongoing effort aiming to add generic support for
handling regulators in the LED core [1], but it turns out to require
bringing back initialization of of_node property for
devm_regulator_get_optional() to work properly.

Support for OF related fwnodes in regulator core could help reducing
this noise.


We could have this done in dev_of_node():

static inline struct device_node *dev_of_node(struct device *dev)
{
    if (!IS_ENABLED(CONFIG_OF) || !dev)
        return NULL;
    return dev->of_node ? dev->of_node : to_of_node(dev->fwnode);
}

Then it will only be a matter of using dev_of_node() instead of 
accessing directly dev->of_node





[0]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/leds/led-class.c?id=fd81d7e946c6bdb86dbf0bd88fee3e1a545e7979
[1]
https://lore.kernel.org/linux-leds/20190923102059.17818-4-jjhib...@ti.com/


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

[PATCH] drm/amdgpu: fix memory leak

2019-10-04 Thread Nirmoy Das
In amdgpu_bo_list_ioctl when idr_alloc fails
don't return without freeing bo list entry.

Fixes: 964d0fbf6301d ("drm/amdgpu: Allow to create BO lists in CS ioctl v3")

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
index 7bcf86c61999..c3e5ea544857 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
@@ -284,7 +284,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void *data,
mutex_unlock(&fpriv->bo_list_lock);
if (r < 0) {
amdgpu_bo_list_put(list);
-   return r;
+   goto error_free;
}
 
handle = r;
-- 
2.23.0



Re: imx6: hdmi black screen issue after resume

2019-10-04 Thread Pintu Agarwal
Hi,

On Sun, Sep 29, 2019 at 10:24 PM Pintu Agarwal  wrote:
>
> >
> > On Mon, Sep 23, 2019 at 1:28 PM Pintu Agarwal  wrote:
> > >
> > > Dear Philipp,
> > >
> > > I have a iMX6dl custom board with custom Linux Kernel 4.8.
> > > I have both LCD and HDMI connected to the board.
> > > And we are using weston/wayland as the display interface.
> > > In normal boot, both LCD and HDMI display is working fine.
> > >
> > > But, currently, for one of the requirement, I am trying to explore and
> > > support hibernation image booting on it.
> > > Currently, we are able to resume the system without display.
> > > Also, if we make the entire imx-drm as modules, and then install the
> > > modules after resume, even LCD is also coming up.
> > > But HDMI display is black out.
> > >

I just found the main root cause of the HDMI screen blackout issue
after system resume.
* HDMI is trying to get EDID data from the monitor using I2C interface.
* But its seems i2c_transfer is getting timeout after 5 retries.
* Thus EDID data is failing, and HDMI could not able to detect the monitor.

This is the logs:

[  441.104989] [drm:drm_helper_probe_single_connector_modes]
[CONNECTOR:29:HDMI-A-1] status updated from unknown to connected
[  441.116080]: drm_helper_probe_single_connector_modes - inside -
else override_edid
[  441.124416]: drm_helper_probe_single_connector_modes - inside -
else - drm_load_edid_firmware: count: 0
[  441.134546]: drm_helper_probe_single_connector_modes - calling - get_modes
[  441.142157]: dw_hdmi_connector_get_modes : called
[  441.147652]: dw_hdmi_connector_get_modes : called - calling -> drm_get_edid
[  441.155346]: drm_do_probe_ddc_edid : called!
[  441.660759]: drm_do_probe_ddc_edid : i2c_transfer: ret: -110, retry: 5
[  442.170758]: drm_do_probe_ddc_edid : i2c_transfer: ret: -110, retry: 4
[  442.680755]: drm_do_probe_ddc_edid : i2c_transfer: ret: -110, retry: 3
[  443.190755]: drm_do_probe_ddc_edid : i2c_transfer: ret: -110, retry: 2
[  443.700754]: drm_do_probe_ddc_edid : i2c_transfer: ret: -110, retry: 1
[  443.707989]: drm_get_edid : called - drm_probe_ddc - failed
[  443.714303] dwhdmi-imx 12.hdmi: failed to get edid

Is there any clue, how to resolve this i2c failure issue, after resume?
This does not happen in normal booting case.

These are the steps I follow:
* Boot the system normally (without display) and install all imx-drm as modules.
* Then uninstall the modules in reverse order.
* Take the snapshot of the system (suspend to disk).
* Reboot the system and boot with the image.
* Install all the modules again.
* Then launch the Weston.
* During the weston launch in the beginning we observe this error.


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

Re: [Intel-gfx] [PATCH] dma-fence: Serialise signal enabling (dma_fence_enable_sw_signaling)

2019-10-04 Thread Tvrtko Ursulin


On 04/10/2019 11:11, Chris Wilson wrote:

Make dma_fence_enable_sw_signaling() behave like its
dma_fence_add_callback() and dma_fence_default_wait() counterparts and
perform the test to enable signaling under the fence->lock, along with
the action to do so. This ensure that should an implementation be trying
to flush the cb_list (by signaling) on retirement before freeing the
fence, it can do so in a race-free manner.

See also 0fc89b6802ba ("dma-fence: Simply wrap dma_fence_signal_locked
with dma_fence_signal").

v2: Refactor all 3 enable_signaling paths to use a common function.
v3: Don't argue, just keep the tracepoint in the existing spot.


I would understand the argument but a) meh and b) why change the ABI, 
kind of.


Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko


Signed-off-by: Chris Wilson 
Cc: Tvrtko Ursulin 
---
  drivers/dma-buf/dma-fence.c | 78 +
  1 file changed, 35 insertions(+), 43 deletions(-)

diff --git a/drivers/dma-buf/dma-fence.c b/drivers/dma-buf/dma-fence.c
index 2c136aee3e79..052a41e2451c 100644
--- a/drivers/dma-buf/dma-fence.c
+++ b/drivers/dma-buf/dma-fence.c
@@ -273,6 +273,30 @@ void dma_fence_free(struct dma_fence *fence)
  }
  EXPORT_SYMBOL(dma_fence_free);
  
+static bool __dma_fence_enable_signaling(struct dma_fence *fence)

+{
+   bool was_set;
+
+   lockdep_assert_held(fence->lock);
+
+   was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,
+  &fence->flags);
+
+   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+   return false;
+
+   if (!was_set && fence->ops->enable_signaling) {
+   trace_dma_fence_enable_signal(fence);
+
+   if (!fence->ops->enable_signaling(fence)) {
+   dma_fence_signal_locked(fence);
+   return false;
+   }
+   }
+
+   return true;
+}
+
  /**
   * dma_fence_enable_sw_signaling - enable signaling on fence
   * @fence: the fence to enable
@@ -285,19 +309,12 @@ void dma_fence_enable_sw_signaling(struct dma_fence 
*fence)
  {
unsigned long flags;
  
-	if (!test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,

- &fence->flags) &&
-   !test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags) &&
-   fence->ops->enable_signaling) {
-   trace_dma_fence_enable_signal(fence);
-
-   spin_lock_irqsave(fence->lock, flags);
-
-   if (!fence->ops->enable_signaling(fence))
-   dma_fence_signal_locked(fence);
+   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+   return;
  
-		spin_unlock_irqrestore(fence->lock, flags);

-   }
+   spin_lock_irqsave(fence->lock, flags);
+   __dma_fence_enable_signaling(fence);
+   spin_unlock_irqrestore(fence->lock, flags);
  }
  EXPORT_SYMBOL(dma_fence_enable_sw_signaling);
  
@@ -331,7 +348,6 @@ int dma_fence_add_callback(struct dma_fence *fence, struct dma_fence_cb *cb,

  {
unsigned long flags;
int ret = 0;
-   bool was_set;
  
  	if (WARN_ON(!fence || !func))

return -EINVAL;
@@ -343,25 +359,14 @@ int dma_fence_add_callback(struct dma_fence *fence, 
struct dma_fence_cb *cb,
  
  	spin_lock_irqsave(fence->lock, flags);
  
-	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,

-  &fence->flags);
-
-   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
-   ret = -ENOENT;
-   else if (!was_set && fence->ops->enable_signaling) {
-   trace_dma_fence_enable_signal(fence);
-
-   if (!fence->ops->enable_signaling(fence)) {
-   dma_fence_signal_locked(fence);
-   ret = -ENOENT;
-   }
-   }
-
-   if (!ret) {
+   if (__dma_fence_enable_signaling(fence)) {
cb->func = func;
list_add_tail(&cb->node, &fence->cb_list);
-   } else
+   } else {
INIT_LIST_HEAD(&cb->node);
+   ret = -ENOENT;
+   }
+
spin_unlock_irqrestore(fence->lock, flags);
  
  	return ret;

@@ -461,7 +466,6 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, 
signed long timeout)
struct default_wait_cb cb;
unsigned long flags;
signed long ret = timeout ? timeout : 1;
-   bool was_set;
  
  	if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))

return ret;
@@ -473,21 +477,9 @@ dma_fence_default_wait(struct dma_fence *fence, bool intr, 
signed long timeout)
goto out;
}
  
-	was_set = test_and_set_bit(DMA_FENCE_FLAG_ENABLE_SIGNAL_BIT,

-  &fence->flags);
-
-   if (test_bit(DMA_FENCE_FLAG_SIGNALED_BIT, &fence->flags))
+   if (!__dma_fence_enable_signaling(fence))
goto out;
  
-	if (!was_set && fence->ops->enable_signaling) {

-  

[PATCH] drm/i810: Prevent underflow in ioctl

2019-10-04 Thread Dan Carpenter
The "used" variables here come from the user in the ioctl and it can be
negative.  It could result in an out of bounds write.

Signed-off-by: Dan Carpenter 
---
 drivers/gpu/drm/i810/i810_dma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c
index 2a77823b8e9a..e66c38332df4 100644
--- a/drivers/gpu/drm/i810/i810_dma.c
+++ b/drivers/gpu/drm/i810/i810_dma.c
@@ -728,7 +728,7 @@ static void i810_dma_dispatch_vertex(struct drm_device *dev,
if (nbox > I810_NR_SAREA_CLIPRECTS)
nbox = I810_NR_SAREA_CLIPRECTS;
 
-   if (used > 4 * 1024)
+   if (used < 0 || used > 4 * 1024)
used = 0;
 
if (sarea_priv->dirty)
@@ -1048,7 +1048,7 @@ static void i810_dma_dispatch_mc(struct drm_device *dev, 
struct drm_buf *buf, in
if (u != I810_BUF_CLIENT)
DRM_DEBUG("MC found buffer that isn't mine!\n");
 
-   if (used > 4 * 1024)
+   if (used < 0 || used > 4 * 1024)
used = 0;
 
sarea_priv->dirty = 0x7f;
-- 
2.20.1

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

Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0

2019-10-04 Thread Lin, Wayne



From: Ville Syrjälä 
Sent: Thursday, October 3, 2019 21:29
To: Lin, Wayne 
Cc: dri-devel@lists.freedesktop.org ; 
amd-...@lists.freedesktop.org ; Li, Sun peng 
(Leo) ; Kazlauskas, Nicholas 
Subject: Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0

On Thu, Oct 03, 2019 at 06:49:05AM +, Lin, Wayne wrote:
>
>
> 
> From: Ville Syrjälä 
> Sent: Wednesday, October 2, 2019 19:58
> To: Lin, Wayne 
> Cc: dri-devel@lists.freedesktop.org ; 
> amd-...@lists.freedesktop.org ; Li, Sun peng 
> (Leo) ; Kazlauskas, Nicholas 
> Subject: Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0
>
> On Tue, Sep 24, 2019 at 01:26:21PM +0800, Wayne Lin wrote:
> > In HDMI 1.4 defines 4k modes without specific aspect ratio.
> > However, in HDMI 2.0, adds aspect ratio attribute to distinguish different
> > 4k modes.
> >
> > According to Appendix E of HDMI 2.0 spec, source should use VSIF to
> > indicate VIC mode only when the mode is one defined in HDMI 1.4b 4K modes.
> > Otherwise, use AVI infoframes to convey VIC.
> >
> > eg: VIC_103 should use AVI infoframes and VIC_93 use VSIF
> >
> > When the sink is HDMI 2.0, current code in
> > drm_hdmi_avi_infoframe_from_display_mode will also force mode VIC_103 to
> > have VIC value 0. This violates the spec and needs to be corrected.
>
> > Where is that being done? We only set the AVI VIC to zero if we're going
> > to use the HDMI VIC instead.
>
> Appreciate for your time and apologize for not explaining it clearly.
> Current code in drm_hdmi_avi_infoframe_from_display_mode() will call
> drm_match_hdmi_mode() to set up vendor_if_vic. By checking
> drm_valid_hdmi_vic(vendor_if_vic) to see if the vic info should be conveyed 
> by avi
> or not.
>
> But in drm_match_hdmi_mode(), code doesn't enable match_flags with
> DRM_MODE_MATCH_ASPECT_RATIO. I think it's due to HDMI1.4b doesn't specify
> 4K mode conveyed by HDMI VIC with particular aspect ratio. But in Appendix E 
> of
> HDMI 2.0 spec, it specify only 4k modes with particular aspect ratio should 
> use VSIF to convey.
> Hence, when the sink support HDMI 2.0 and set the mode to be VIC_103, calling
> drm_match_hdmi_mode(mode) will return vendor_if_vic = 3 (VIC_93 and VIC_103 
> are having
> the same timing but different aspect ratio). Thereafter will set the  
> frame->video_code to 0.
> However, VIC_103 should use AVI VIC according to HDMI 2.0 spec (only VIC: 93, 
> 94, 95 &
> 98 should use VSIF).
>
> This patch try to revise that, when the sink support HDMI 2.0, 
> drm_match_hdmi_mode()
> should also take aspect ratio into consideration.
> But for easy reading, I add another function "drm_match_hdmi_1_4_mode" to do 
> so.

> Seems rather convoluted. I think we should just add the aspect ratios
> to edid_4k_modes[]. Or is there some problem with that approach?

Thanks for your time.

Since HDMI 1.4b doesn't require edid_4k_modes[] with specific aspect ratio, 
modes as the same
timing in edid_4k_modes[] but with different aspect ratios are also expected to 
convey VIC by
VSIF to HDMI 1.4 sink. Might can't guarantee that there wont't be any 
compatibility side effect with
that approach when the sink is HDMI 1.4b .

>
> > The same situation occurs in drm_hdmi_vendor_infoframe_from_display_mode
> > and should set HDMI_VIC when the mode is one defined in HDMI 1.4b 4K
> > modes.
>
> > Yes, and we do that. "vic = drm_match_hdmi_mode(mode);"
>
> > Apart from adding the aspect ratios I don't really understand what
> > you're trying to achieve here.
>
> For HDMI 2.0 sink, drm_match_hdmi_mode() should also take aspect ratio into 
> consideration.
> Once again, very appreciate for your time.
>
> > ---
> >  drivers/gpu/drm/drm_edid.c | 95 --
> >  1 file changed, 92 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 649cfd8b4200..0fea9bf4ec67 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -1306,6 +1306,37 @@ static const struct drm_display_mode edid_4k_modes[] 
> > = {
> >  .vrefresh = 24, },
> >  };
> >
> > +/*
> > + * 4k modes of HDMI 1.4 defined in HDMI 2.0. Index using the VIC.
> > + */
> > +static const struct drm_display_mode hdmi_1_4_edid_4k_modes[] = {
> > + /* 0 - dummy, VICs start at 1 */
> > + { },
> > + /* 1 - 3840x2160@30Hz */
> > + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
> > +3840, 4016, 4104, 4400, 0,
> > +2160, 2168, 2178, 2250, 0,
> > +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC),
> > +   .vrefresh = 30, .picture_aspect_ratio = HDMI_PICTURE_ASPECT_16_9, },
> > + /* 2 - 3840x2160@25Hz */
> > + { DRM_MODE("3840x2160", DRM_MODE_TYPE_DRIVER, 297000,
> > +3840, 4896, 4984, 5280, 0,
> > +2160, 2168, 2178, 2250, 0,
> > +DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC)

Re: [PATCH] drm/amdgpu: fix memory leak

2019-10-04 Thread Koenig, Christian
First of all please send mails regarding amdgpu to the amd-gfx mailing 
list and not lkml/dri-devel.

Am 04.10.19 um 12:17 schrieb Nirmoy Das:
> In amdgpu_bo_list_ioctl when idr_alloc fails
> don't return without freeing bo list entry.
>
> Fixes: 964d0fbf6301d ("drm/amdgpu: Allow to create BO lists in CS ioctl v3")
>
> Signed-off-by: Nirmoy Das 
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> index 7bcf86c61999..c3e5ea544857 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
> @@ -284,7 +284,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void 
> *data,
>   mutex_unlock(&fpriv->bo_list_lock);
>   if (r < 0) {
>   amdgpu_bo_list_put(list);
> - return r;
> + goto error_free;

NAK, that is a double free. The bo list entries are freed by 
amdgpu_bo_list_put().

Regards,
Christian.

>   }
>   
>   handle = r;

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

Re: [PATCH] drm: atomic helper: fix W=1 warnings

2019-10-04 Thread Benjamin Gaignard
Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
 a écrit :
>
> On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
> > Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
> >  a écrit :
> > >
> > > On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> > > > Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> > > >  a écrit :
> > > > >
> > > > > On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > > > > > Fix warnings with W=1.
> > > > > > Few for_each macro set variables that are never used later.
> > > > > > Prevent warning by marking these variables as __maybe_unused.
> > > > > >
> > > > > > Signed-off-by: Benjamin Gaignard 
> > > > > > ---
> > > > > >  drivers/gpu/drm/drm_atomic_helper.c | 36 
> > > > > > ++--
> > > > > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > > > > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > index aa16ea17ff9b..b69d17b0b9bd 100644
> > > > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> > > > > > struct drm_encoder *encoder)
> > > > > >  {
> > > > > >   struct drm_crtc_state *crtc_state;
> > > > > > - struct drm_connector *connector;
> > > > > > + struct drm_connector __maybe_unused *connector;
> > > > >
> > > > > Rather ugly. IMO would be nicer if we could hide something inside
> > > > > the iterator macros to suppress the warning.
> > > >
> > > > Ok but how ?
> > > > connector is assigned in the macros but not used later and we can't
> > > > set "__maybe_unused"
> > > > in the macro.
> > > > Does another keyword exist for that ?
> > >
> > > Stick a (void)(connector) into the macro?
> >
> > That could work but it will look strange inside the macro.
> >
> > >
> > > Another (arguably cleaner) idea would be to remove the 
> > > connector/crtc/plane
> > > argument from the iterators entirely since it's redundant, and instead 
> > > just
> > > extract it from the appropriate new/old state as needed.
> > >
> > > We could then also add a for_each_connector_in_state()/etc. which omit
> > > s the state arguments and just has the connector argument, for cases where
> > > you don't care about the states when iterating.
> >
> > That may lead to get a macro for each possible combination of used 
> > variables.
>
> We already have new/old/oldnew, so would "just" add one more.

Not just one, it will be one each new/old/oldnew macro to be able to distinguish
when connector is used or not.
And it will be the same for the for_each macros...

>
> >
> > >
> > > >
> > > > >
> > > > > >   struct drm_connector_state *old_connector_state, 
> > > > > > *new_connector_state;
> > > > > >   int i;
> > > > > >
> > > > > > @@ -412,7 +412,7 @@ mode_fixup(struct drm_atomic_state *state)
> > > > > >  {
> > > > > >   struct drm_crtc *crtc;
> > > > > >   struct drm_crtc_state *new_crtc_state;
> > > > > > - struct drm_connector *connector;
> > > > > > + struct drm_connector __maybe_unused *connector;
> > > > > >   struct drm_connector_state *new_conn_state;
> > > > > >   int i;
> > > > > >   int ret;
> > > > > > @@ -608,7 +608,7 @@ drm_atomic_helper_check_modeset(struct 
> > > > > > drm_device *dev,
> > > > > >  {
> > > > > >   struct drm_crtc *crtc;
> > > > > >   struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > > - struct drm_connector *connector;
> > > > > > + struct drm_connector __maybe_unused *connector;
> > > > > >   struct drm_connector_state *old_connector_state, 
> > > > > > *new_connector_state;
> > > > > >   int i, ret;
> > > > > >   unsigned connectors_mask = 0;
> > > > > > @@ -984,7 +984,7 @@ crtc_needs_disable(struct drm_crtc_state 
> > > > > > *old_state,
> > > > > >  static void
> > > > > >  disable_outputs(struct drm_device *dev, struct drm_atomic_state 
> > > > > > *old_state)
> > > > > >  {
> > > > > > - struct drm_connector *connector;
> > > > > > + struct drm_connector __maybe_unused *connector;
> > > > > >   struct drm_connector_state *old_conn_state, *new_conn_state;
> > > > > >   struct drm_crtc *crtc;
> > > > > >   struct drm_crtc_state *old_crtc_state, *new_crtc_state;
> > > > > > @@ -1173,7 +1173,7 @@ crtc_set_mode(struct drm_device *dev, struct 
> > > > > > drm_atomic_state *old_state)
> > > > > >  {
> > > > > >   struct drm_crtc *crtc;
> > > > > >   struct drm_crtc_state *new_crtc_state;
> > > > > > - struct drm_connector *connector;
> > > > > > + struct drm_connector __maybe_unused *connector;
> > > > > >   struct drm_connector_state *new_conn_state;
> > > > > >   int i;
> > > > > >
> > > > > > @@ -1294,7 +1294,7 @@ void 
> > > > > > drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
> > > > > >   struct drm_crtc *crtc;
> > > > > >   struct

Re: [PATCH] drm/amdgpu: fix memory leak

2019-10-04 Thread Nirmoy

On 10/4/19 12:44 PM, Koenig, Christian wrote:
> First of all please send mails regarding amdgpu to the amd-gfx mailing
> list and not lkml/dri-devel.
Okay.
> Am 04.10.19 um 12:17 schrieb Nirmoy Das:
>> In amdgpu_bo_list_ioctl when idr_alloc fails
>> don't return without freeing bo list entry.
>>
>> Fixes: 964d0fbf6301d ("drm/amdgpu: Allow to create BO lists in CS ioctl v3")
>>
>> Signed-off-by: Nirmoy Das 
>> ---
>>drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
>>1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> index 7bcf86c61999..c3e5ea544857 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>> @@ -284,7 +284,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void 
>> *data,
>>  mutex_unlock(&fpriv->bo_list_lock);
>>  if (r < 0) {
>>  amdgpu_bo_list_put(list);
>> -return r;
>> +goto error_free;
> NAK, that is a double free. The bo list entries are freed by
> amdgpu_bo_list_put().
Thanks, didn't realize that.
> Regards,
> Christian.

Regards,

Nirmoy

>>  }
>>
>>  handle = r;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/11] lib/interval-tree: add an equivalent tree with [a,b) intervals

2019-10-04 Thread Michel Lespinasse
On Thu, Oct 03, 2019 at 01:18:49PM -0700, Davidlohr Bueso wrote:
> +/* \
> + * Iterate over intervals intersecting [start;end) \
> + * \
> + * Note that a node's interval intersects [start;end) iff: \
> + *   Cond1: ITSTART(node) < end  
>   \
> + * and   
>   \
> + *   Cond2: start < ITEND(node)  
>   \
> + */\
> +   \
> +static ITSTRUCT *  \
> +ITPREFIX ## _subtree_search(ITSTRUCT *node, ITTYPE start, ITTYPE end)
>   \
> +{  \
> + while (true) {\
> + /*\
> +  * Loop invariant: start <= node->ITSUBTREE   \
Should be start < node->ITSUBTREE
> +  * (Cond2 is satisfied by one of the subtree nodes)   \
> +  */   \
> + if (node->ITRB.rb_left) { \
> + ITSTRUCT *left = rb_entry(node->ITRB.rb_left, \
> +   ITSTRUCT, ITRB);\
> + if (start < left->ITSUBTREE) {\
> + /*\
> +  * Some nodes in left subtree satisfy Cond2.  \
> +  * Iterate to find the leftmost such node N.  \
> +  * If it also satisfies Cond1, that's the \
> +  * match we are looking for. Otherwise, there \
> +  * is no matching interval as nodes to the\
> +  * right of N can't satisfy Cond1 either. \
> +  */   \
> + node = left;  \
> + continue; \
> + } \
> + } \
> + if (ITSTART(node) < end) {  /* Cond1 */   \
> + if (start < ITEND(node))/* Cond2 */   \
> + return node;/* node is leftmost match */  \
> + if (node->ITRB.rb_right) {\
> + node = rb_entry(node->ITRB.rb_right,  \
> + ITSTRUCT, ITRB);  \
> + if (start <= node->ITSUBTREE) \
Should be start < node->ITSUBTREE
> + continue; \
> + } \
> + } \
> + return NULL;/* No match */\
> + } \
> +}  \

Other than that, the change looks good to me.

This is something I might use, regardless of the status of converting
other current users.

The name "interval_tree_gen.h" makes it ambiguous wether gen stands
for "generic" or "generator". This may sounds like a criticism,
but it's not - I think it really stands for both :)

Reviewed-by: Michel Lespinasse 

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.


[PATCH] cec: add cec_adapter to cec_notifier_cec_adap_unregister()

2019-10-04 Thread Hans Verkuil
It is possible for one HDMI connector to have multiple CEC adapters. The
typical real-world scenario is that where one adapter is used when the device
is in standby, and one that's better/smarter when the device is powered up.

The cec-notifier changes were made with that in mind, but I missed that in
order to support this you need to tell cec_notifier_cec_adap_unregister()
which adapter you are unregistering from the notifier.

Add this additional argument. It is currently unused, but once all drivers
use this, the CEC core will be adapted for these use-cases.

Signed-off-by: Hans Verkuil 
---
This patch should go in via the drm subsystem since all CEC adapters in the
drm subsystem have been converted to use cec_notifier_cec_adap_unregister().
The media subsystem still has older drm drivers that weren't converted to use
cec_notifier_cec_adap_unregister().

This will only be a problem if a new CEC adapter driver is added to the media
subsystem for v5.5, but I am not aware of any plans for that. Should it happen,
then that just means that the media subsystem needs to resolve a fairly trivial
merge conflict.

Ville, Mauro, can you review/ack?
---
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c 
b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
index ac1e001d0882..70ab4fbdc23e 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
@@ -285,7 +285,7 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)

ret = cec_register_adapter(cec->adap, pdev->dev.parent);
if (ret < 0) {
-   cec_notifier_cec_adap_unregister(cec->notify);
+   cec_notifier_cec_adap_unregister(cec->notify, cec->adap);
return ret;
}

@@ -302,7 +302,7 @@ static int dw_hdmi_cec_remove(struct platform_device *pdev)
 {
struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);

-   cec_notifier_cec_adap_unregister(cec->notify);
+   cec_notifier_cec_adap_unregister(cec->notify, cec->adap);
cec_unregister_adapter(cec->adap);

return 0;
diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
index a5a75bdeb7a5..5b03fdd1eaa4 100644
--- a/drivers/gpu/drm/i2c/tda9950.c
+++ b/drivers/gpu/drm/i2c/tda9950.c
@@ -465,7 +465,7 @@ static int tda9950_probe(struct i2c_client *client,

ret = cec_register_adapter(priv->adap, priv->hdmi);
if (ret < 0) {
-   cec_notifier_cec_adap_unregister(priv->notify);
+   cec_notifier_cec_adap_unregister(priv->notify, priv->adap);
return ret;
}

@@ -482,7 +482,7 @@ static int tda9950_remove(struct i2c_client *client)
 {
struct tda9950_priv *priv = i2c_get_clientdata(client);

-   cec_notifier_cec_adap_unregister(priv->notify);
+   cec_notifier_cec_adap_unregister(priv->notify, priv->adap);
cec_unregister_adapter(priv->adap);

return 0;
diff --git a/drivers/media/cec/cec-notifier.c b/drivers/media/cec/cec-notifier.c
index 4d82a5522072..7cf42b133dbc 100644
--- a/drivers/media/cec/cec-notifier.c
+++ b/drivers/media/cec/cec-notifier.c
@@ -153,13 +153,14 @@ cec_notifier_cec_adap_register(struct device *hdmi_dev, 
const char *conn_name,
 }
 EXPORT_SYMBOL_GPL(cec_notifier_cec_adap_register);

-void cec_notifier_cec_adap_unregister(struct cec_notifier *n)
+void cec_notifier_cec_adap_unregister(struct cec_notifier *n,
+ struct cec_adapter *adap)
 {
if (!n)
return;

mutex_lock(&n->lock);
-   n->cec_adap->notifier = NULL;
+   adap->notifier = NULL;
n->cec_adap = NULL;
n->callback = NULL;
mutex_unlock(&n->lock);
diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c 
b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
index 4a3b3810fd89..f048e8994785 100644
--- a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
+++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
@@ -314,7 +314,8 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
return 0;

 out_probe_notify:
-   cec_notifier_cec_adap_unregister(cros_ec_cec->notify);
+   cec_notifier_cec_adap_unregister(cros_ec_cec->notify,
+cros_ec_cec->adap);
 out_probe_adapter:
cec_delete_adapter(cros_ec_cec->adap);
return ret;
@@ -335,7 +336,8 @@ static int cros_ec_cec_remove(struct platform_device *pdev)
return ret;
}

-   cec_notifier_cec_adap_unregister(cros_ec_cec->notify);
+   cec_notifier_cec_adap_unregister(cros_ec_cec->notify,
+cros_ec_cec->adap);
cec_unregister_adapter(cros_ec_cec->adap);

return 0;
diff --git a/drivers/media/platform/meson/ao-cec-g12a.c 
b/drivers/media/platform/meson/ao-cec-g12a.c
index 3b39e875292e..70f875b4a01e 100644
--- a/drivers/media/platform/meson/ao-cec-g12a.c
+++ b/drivers/media/platform/meson/ao-cec-g12a.c
@@ -736,7 +736

Re: [Intel-gfx] [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()

2019-10-04 Thread Chris Wilson
Quoting Tvrtko Ursulin (2019-10-04 10:15:20)
> 
> On 03/10/2019 22:01, Chris Wilson wrote:
> > A few callers need to serialise the destruction of their drm_mm_node and
> > ensure it is removed from the drm_mm before freeing. However, to be
> > completely sure that any access from another thread is complete before
> > we free the struct, we require the RELEASE semantics of
> > clear_bit_unlock().
> > 
> > This allows the conditional locking such as
> > 
> > Thread A  Thread B
> >  mutex_lock(mm_lock);if (drm_mm_node_allocated(node)) {
> >  drm_mm_node_remove(node);   mutex_lock(mm_lock);
> >  mutex_unlock(mm_lock);  drm_mm_node_remove(node);
> >  mutex_unlock(mm_lock);
> >   }
> >   kfree(node);
> 
> My understanding is that release semantics on node allocated mean 1 -> 0 
> transition is guaranteed to be seen only when thread A 
> drm_mm_node_remove is fully done with the removal. But if it is in the 
> middle of removal, node is still seen as allocated outside and thread B 
> can enter the if-body, wait for the lock, and then drm_mm_node_remove 
> will attempt a double removal. So I think another drm_mm_node_allocated 
> under the lock is needed.

Yes. Check after the lock is indeed required in this scenario. And
drm_mm_node_remove() insists the caller doesn't try a double remove.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amdgpu: fix memory leak

2019-10-04 Thread Koenig, Christian
Am 04.10.19 um 13:00 schrieb Das, Nirmoy:
> On 10/4/19 12:44 PM, Koenig, Christian wrote:
>> First of all please send mails regarding amdgpu to the amd-gfx mailing
>> list and not lkml/dri-devel.
> Okay.
>> Am 04.10.19 um 12:17 schrieb Nirmoy Das:
>>> In amdgpu_bo_list_ioctl when idr_alloc fails
>>> don't return without freeing bo list entry.
>>>
>>> Fixes: 964d0fbf6301d ("drm/amdgpu: Allow to create BO lists in CS ioctl v3")
>>>
>>> Signed-off-by: Nirmoy Das 
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>>> index 7bcf86c61999..c3e5ea544857 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c
>>> @@ -284,7 +284,7 @@ int amdgpu_bo_list_ioctl(struct drm_device *dev, void 
>>> *data,
>>> mutex_unlock(&fpriv->bo_list_lock);
>>> if (r < 0) {
>>> amdgpu_bo_list_put(list);
>>> -   return r;
>>> +   goto error_free;
>> NAK, that is a double free. The bo list entries are freed by
>> amdgpu_bo_list_put().
> Thanks, didn't realize that.

Wait a second, what entries are you talking about?

The entries in the list object are freed when amdgpu_bo_list_put() is 
called, but the temporary info array with the handles needs to be freed 
as well.

And it looks like that is indeed leaked here.

Regards,
Christian.

>> Regards,
>> Christian.
> Regards,
>
> Nirmoy
>
>>> }
>>> 
>>> handle = r;

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

[PATCH 1/2] drm: bridge: adv7511: Enable SPDIF DAI

2019-10-04 Thread Bogdan Togorean
ADV7511 support I2S or SPDIF as audio input interfaces. This commit
enable support for SPDIF.

Signed-off-by: Bogdan Togorean 
---
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
index a428185be2c1..96be7b005c50 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -119,6 +119,8 @@ int adv7511_hdmi_hw_params(struct device *dev, void *data,
audio_source = ADV7511_AUDIO_SOURCE_I2S;
i2s_format = ADV7511_I2S_FORMAT_LEFT_J;
break;
+   case HDMI_SPDIF:
+   audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
default:
return -EINVAL;
}
@@ -175,11 +177,21 @@ static int audio_startup(struct device *dev, void *data)
/* use Audio infoframe updated info */
regmap_update_bits(adv7511->regmap, ADV7511_REG_GC(1),
BIT(5), 0);
+   /* enable SPDIF receiver */
+   if (adv7511->audio_source == ADV7511_AUDIO_SOURCE_SPDIF)
+   regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
+  BIT(7), BIT(7));
+
return 0;
 }
 
 static void audio_shutdown(struct device *dev, void *data)
 {
+   struct adv7511 *adv7511 = dev_get_drvdata(dev);
+
+   if (adv7511->audio_source == ADV7511_AUDIO_SOURCE_SPDIF)
+   regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
+  BIT(7), 0);
 }
 
 static int adv7511_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
@@ -213,6 +225,7 @@ static const struct hdmi_codec_pdata codec_data = {
.ops = &adv7511_codec_ops,
.max_i2s_channels = 2,
.i2s = 1,
+   .spdif = 1,
 };
 
 int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511)
-- 
2.23.0



Re: [Intel-gfx] [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()

2019-10-04 Thread Chris Wilson
Quoting Chris Wilson (2019-10-04 12:07:10)
> Quoting Tvrtko Ursulin (2019-10-04 10:15:20)
> > 
> > On 03/10/2019 22:01, Chris Wilson wrote:
> > > A few callers need to serialise the destruction of their drm_mm_node and
> > > ensure it is removed from the drm_mm before freeing. However, to be
> > > completely sure that any access from another thread is complete before
> > > we free the struct, we require the RELEASE semantics of
> > > clear_bit_unlock().
> > > 
> > > This allows the conditional locking such as
> > > 
> > > Thread A  Thread B
> > >  mutex_lock(mm_lock);if (drm_mm_node_allocated(node)) 
> > > {
> > >  drm_mm_node_remove(node);   mutex_lock(mm_lock);
> > >  mutex_unlock(mm_lock);  drm_mm_node_remove(node);
> > >  mutex_unlock(mm_lock);
> > >   }
> > >   kfree(node);
> > 
> > My understanding is that release semantics on node allocated mean 1 -> 0 
> > transition is guaranteed to be seen only when thread A 
> > drm_mm_node_remove is fully done with the removal. But if it is in the 
> > middle of removal, node is still seen as allocated outside and thread B 
> > can enter the if-body, wait for the lock, and then drm_mm_node_remove 
> > will attempt a double removal. So I think another drm_mm_node_allocated 
> > under the lock is needed.
> 
> Yes. Check after the lock is indeed required in this scenario. And
> drm_mm_node_remove() insists the caller doesn't try a double remove.

I had to go back and double check the vma code, and that's fine.
(We hit this case where one thread is evicting and another thread is
destroying the object. And for us we do the check under the lock inside
__i915_vma_unbind() on the destroy path.)
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[Bug 111729] RX480 : random NULL pointer dereference on resume from suspend

2019-10-04 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111729

--- Comment #6 from p...@phi-gamma.net ---
I agree with SET that bisecting would not be feasible due to
the rarity of the bug. For reference, the affected box just
now crashed after a week of no issue with suspending and
resuming approx. twice a day.

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

[PATCH 2/2] drm: bridge: adv7511: Extend list of audio sample rates

2019-10-04 Thread Bogdan Togorean
ADV7511 support sample rates up to 192kHz. CTS and N parameters should
be computed accordingly so this commit extend the list up to maximum
supported sample rate.

Signed-off-by: Bogdan Togorean 
---
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c 
b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
index 96be7b005c50..f376ed7eb9da 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -27,6 +27,18 @@ static void adv7511_calc_cts_n(unsigned int f_tmds, unsigned 
int fs,
case 48000:
*n = 6144;
break;
+   case 88200:
+   *n = 12544;
+   break;
+   case 96000:
+   *n = 12288;
+   break;
+   case 176400:
+   *n = 25088;
+   break;
+   case 192000:
+   *n = 24576;
+   break;
}
 
*cts = ((f_tmds * *n) / (128 * fs)) * 1000;
-- 
2.23.0



Re: [PATCH] drm/amdgpu: fix memory leak

2019-10-04 Thread Nirmoy

On 10/4/19 1:13 PM, Koenig, Christian wrote:
>
>>> NAK, that is a double free. The bo list entries are freed by
>>> amdgpu_bo_list_put().
>> Thanks, didn't realize that.
> Wait a second, what entries are you talking about?
>
> The entries in the list object are freed when amdgpu_bo_list_put() is
> called, but the temporary info array with the handles needs to be freed
> as well.
>
> And it looks like that is indeed leaked here.
I am talking about the `info` array created by 
amdgpu_bo_create_list_entry_array().
> Regards,
> Christian.
>
>>> Regards,
>>> Christian.
>> Regards,
>>
>> Nirmoy
>>
}
  
handle = r;


[Bug 204885] ryzen 2500U cause graphics glitch in all browsers with kernel version 5.2.x+

2019-10-04 Thread bugzilla-daemon
https://bugzilla.kernel.org/show_bug.cgi?id=204885

no2stable (pro...@outlook.com) changed:

   What|Removed |Added

  Component|Video(DRI - non Intel)  |Other
   Hardware|All |x86-64
Product|Drivers |Other

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

Re: [PATCH] drm/amdgpu: fix memory leak

2019-10-04 Thread Koenig, Christian
Am 04.10.19 um 13:26 schrieb Das, Nirmoy:
> On 10/4/19 1:13 PM, Koenig, Christian wrote:
 NAK, that is a double free. The bo list entries are freed by
 amdgpu_bo_list_put().
>>> Thanks, didn't realize that.
>> Wait a second, what entries are you talking about?
>>
>> The entries in the list object are freed when amdgpu_bo_list_put() is
>> called, but the temporary info array with the handles needs to be freed
>> as well.
>>
>> And it looks like that is indeed leaked here.
> I am talking about the `info` array created by
> amdgpu_bo_create_list_entry_array().

Yeah, that are the handles and not the entries. Sorry that I was 
confused about that.

Your patch is correct, you should just update the commit message a bit.

BTW: Could you cleanup error handling here a bit more?

E.g. add an error_put_list handle and drop the "if (info)" and instead 
return directly if we fail to allocate info.

Thanks,
Christian.

>> Regards,
>> Christian.
>>
 Regards,
 Christian.
>>> Regards,
>>>
>>> Nirmoy
>>>
>   }
>   
>   handle = r;



Re: [PATCH] drm/amdgpu: fix memory leak

2019-10-04 Thread Nirmoy

On 10/4/19 1:30 PM, Koenig, Christian wrote:
> Am 04.10.19 um 13:26 schrieb Das, Nirmoy:
>> On 10/4/19 1:13 PM, Koenig, Christian wrote:
> NAK, that is a double free. The bo list entries are freed by
> amdgpu_bo_list_put().
 Thanks, didn't realize that.
>>> Wait a second, what entries are you talking about?
>>>
>>> The entries in the list object are freed when amdgpu_bo_list_put() is
>>> called, but the temporary info array with the handles needs to be freed
>>> as well.
>>>
>>> And it looks like that is indeed leaked here.
>> I am talking about the `info` array created by
>> amdgpu_bo_create_list_entry_array().
> Yeah, that are the handles and not the entries. Sorry that I was
> confused about that.
>
> Your patch is correct, you should just update the commit message a bit.
>
> BTW: Could you cleanup error handling here a bit more?
>
> E.g. add an error_put_list handle and drop the "if (info)" and instead
> return directly if we fail to allocate info.
Okay I will do that in v2 of this patch.
> Thanks,
> Christian.


Regards,

Nirmoy



Re: Should regulator core support parsing OF based fwnode? (was: Re: [PATCH v8 2/5] leds: Add of_led_get() and led_put())

2019-10-04 Thread Mark Brown
On Thu, Oct 03, 2019 at 10:27:26PM +0200, Jacek Anaszewski wrote:
> On 10/3/19 9:41 PM, Mark Brown wrote:

> > Why would we want to do that?  We'd continue to support only DT systems,
> > just with code that's less obviously DT only and would need to put
> > checks in.  I'm not seeing an upside here.

> For instance few weeks ago we had a patch [0] in the LED core switching
> from using struct device's of_node property to fwnode for conveying
> device property data. And this transition to fwnode property API can be
> observed as a frequent pattern across subsystems.

For most subsystems the intent is to reuse DT bindings on embedded ACPI
systems via _DSD.

> Recently there is an ongoing effort aiming to add generic support for
> handling regulators in the LED core [1], but it turns out to require
> bringing back initialization of of_node property for
> devm_regulator_get_optional() to work properly.

Consumers should just be able to request a regulator without having to
worry about how that's being provided - they should have no knowledge at
all of firmware bindings or platform data for defining this.  If they
do that suggests there's an abstraction issue somewhere, what makes you
think that doing something with of_node is required?

Further, unless you have LEDs that work without power you probably
shouldn't be using _get_optional() for their supply.  That interface is
intended only for supplies that may be physically absent.


signature.asc
Description: PGP signature


Re: [PATCH V6 5/8] backlight: qcom-wled: Restructure the driver for WLED3

2019-10-04 Thread kgunda

On 2019-10-01 20:47, Dan Murphy wrote:

Kiran

On 9/30/19 1:39 AM, Kiran Gunda wrote:

Restructure the driver to add the support for new WLED
peripherals.

Signed-off-by: Kiran Gunda 
Acked-by: Daniel Thompson 
---
  drivers/video/backlight/qcom-wled.c | 395 
++--

  1 file changed, 245 insertions(+), 150 deletions(-)

diff --git a/drivers/video/backlight/qcom-wled.c 
b/drivers/video/backlight/qcom-wled.c

index f191242..740f1b6 100644
--- a/drivers/video/backlight/qcom-wled.c
+++ b/drivers/video/backlight/qcom-wled.c
@@ -7,59 +7,71 @@
  #include 
  #include 
  #include 
+#include 
  #include 
/* From DT binding */
+#define WLED_MAX_STRINGS   4
+
  #define WLED_DEFAULT_BRIGHTNESS   2048
  -#define WLED3_SINK_REG_BRIGHT_MAX0xFFF
-#define WLED3_CTRL_REG_VAL_BASE0x40
+#define WLED_SINK_REG_BRIGHT_MAX   0xFFF


Why did you change some of these again?

Can you just change it to the final #define in patch 4/8?

Dan


Ok.. Looks like some thing went wrong with this series. I will re-upload 
it again.


Re: [PATCH 05/11] IB/hfi1: convert __mmu_int_rb to half closed intervals

2019-10-04 Thread Michel Lespinasse
On Thu, Oct 03, 2019 at 01:18:52PM -0700, Davidlohr Bueso wrote:
> diff --git a/drivers/infiniband/hw/hfi1/mmu_rb.c 
> b/drivers/infiniband/hw/hfi1/mmu_rb.c
> index 14d2a90964c3..fb6382b2d44e 100644
> --- a/drivers/infiniband/hw/hfi1/mmu_rb.c
> +++ b/drivers/infiniband/hw/hfi1/mmu_rb.c
> @@ -47,7 +47,7 @@
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  
>  #include "mmu_rb.h"
>  #include "trace.h"
> @@ -89,7 +89,7 @@ static unsigned long mmu_node_start(struct mmu_rb_node 
> *node)
>  
>  static unsigned long mmu_node_last(struct mmu_rb_node *node)
>  {
> - return PAGE_ALIGN(node->addr + node->len) - 1;
> + return PAGE_ALIGN(node->addr + node->len);
>  }

May as well rename the function mmu_node_end(). I was worried if it
was used anywhere else, but it turned out it's only used when defining
the interval tree.

I would also suggest moving this function (as well as mmu_node_first)
right before its use, rather than just after, which would allow you to
also remove the function prototype a few lines earlier.

Looks good to me otherwise.

Reviewed-by: Michel Lespinasse 

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.


Re: [Intel-gfx] [PATCH 5/5] drm/mm: Use clear_bit_unlock() for releasing the drm_mm_node()

2019-10-04 Thread Tvrtko Ursulin


On 04/10/2019 12:17, Chris Wilson wrote:

Quoting Chris Wilson (2019-10-04 12:07:10)

Quoting Tvrtko Ursulin (2019-10-04 10:15:20)


On 03/10/2019 22:01, Chris Wilson wrote:

A few callers need to serialise the destruction of their drm_mm_node and
ensure it is removed from the drm_mm before freeing. However, to be
completely sure that any access from another thread is complete before
we free the struct, we require the RELEASE semantics of
clear_bit_unlock().

This allows the conditional locking such as

Thread A  Thread B
  mutex_lock(mm_lock);if (drm_mm_node_allocated(node)) {
  drm_mm_node_remove(node);   mutex_lock(mm_lock);
  mutex_unlock(mm_lock);  drm_mm_node_remove(node);
  mutex_unlock(mm_lock);
   }
   kfree(node);


My understanding is that release semantics on node allocated mean 1 -> 0
transition is guaranteed to be seen only when thread A
drm_mm_node_remove is fully done with the removal. But if it is in the
middle of removal, node is still seen as allocated outside and thread B
can enter the if-body, wait for the lock, and then drm_mm_node_remove
will attempt a double removal. So I think another drm_mm_node_allocated
under the lock is needed.


Yes. Check after the lock is indeed required in this scenario. And
drm_mm_node_remove() insists the caller doesn't try a double remove.


I had to go back and double check the vma code, and that's fine.
(We hit this case where one thread is evicting and another thread is
destroying the object. And for us we do the check under the lock inside
__i915_vma_unbind() on the destroy path.)


So I think if you amend the commit message to contain the repeated check 
under the lock patch looks good to me. With that:


Reviewed-by: Tvrtko Ursulin 

Regards,

Tvrtko


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

Re: [PATCH 07/11] vhost: convert vhost_umem_interval_tree to half closed intervals

2019-10-04 Thread Michel Lespinasse
On Thu, Oct 03, 2019 at 01:18:54PM -0700, Davidlohr Bueso wrote:
> @@ -1320,15 +1320,14 @@ static bool iotlb_access_ok(struct vhost_virtqueue 
> *vq,
>  {
>   const struct vhost_umem_node *node;
>   struct vhost_umem *umem = vq->iotlb;
> - u64 s = 0, size, orig_addr = addr, last = addr + len - 1;
> + u64 s = 0, size, orig_addr = addr, last = addr + len;

maybe "end" or "end_addr" instead of "last".

> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index e9ed2722b633..bb36cb9ed5ec 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -53,13 +53,13 @@ struct vhost_log {
>  };
>  
>  #define START(node) ((node)->start)
> -#define LAST(node) ((node)->last)
> +#define END(node) ((node)->end)
>  
>  struct vhost_umem_node {
>   struct rb_node rb;
>   struct list_head link;
>   __u64 start;
> - __u64 last;
> + __u64 end;
>   __u64 size;
>   __u64 userspace_addr;
>   __u32 perm;

Preferably also rename __subtree_last to __subtree_end

Looks good to me otherwise.

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.


[PATCH] drm/drm_syncobj: Dead code removal

2019-10-04 Thread Zbigniew Kempczyński
Remove dead code, likely overseened during review process.

Signed-off-by: Zbigniew Kempczyński 
Cc: Chunming Zhou 
Cc: Daniel Vetter 
Cc: Jason Ekstrand 
---
 drivers/gpu/drm/drm_syncobj.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 4b5c7b0ed714..21a22e39c9fa 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -192,8 +192,6 @@ static void drm_syncobj_fence_add_wait(struct drm_syncobj 
*syncobj,
if (!fence || dma_fence_chain_find_seqno(&fence, wait->point)) {
dma_fence_put(fence);
list_add_tail(&wait->node, &syncobj->cb_list);
-   } else if (!fence) {
-   wait->fence = dma_fence_get_stub();
} else {
wait->fence = fence;
}
@@ -856,8 +854,6 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj 
*syncobj,
if (!fence || dma_fence_chain_find_seqno(&fence, wait->point)) {
dma_fence_put(fence);
return;
-   } else if (!fence) {
-   wait->fence = dma_fence_get_stub();
} else {
wait->fence = fence;
}
-- 
2.23.0

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

Re: [PATCH] drm/drm_syncobj: Dead code removal

2019-10-04 Thread Chris Wilson
Quoting Zbigniew Kempczyński (2019-10-04 13:16:52)
> Remove dead code, likely overseened during review process.

Hint: It's not dead.
-Chris
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/drm_syncobj: Dead code removal

2019-10-04 Thread Lionel Landwerlin

On 04/10/2019 15:16, Zbigniew Kempczyński wrote:

Remove dead code, likely overseened during review process.

Signed-off-by: Zbigniew Kempczyński 
Cc: Chunming Zhou 
Cc: Daniel Vetter 
Cc: Jason Ekstrand 
---
  drivers/gpu/drm/drm_syncobj.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c
index 4b5c7b0ed714..21a22e39c9fa 100644
--- a/drivers/gpu/drm/drm_syncobj.c
+++ b/drivers/gpu/drm/drm_syncobj.c
@@ -192,8 +192,6 @@ static void drm_syncobj_fence_add_wait(struct drm_syncobj 
*syncobj,
if (!fence || dma_fence_chain_find_seqno(&fence, wait->point)) {
dma_fence_put(fence);
list_add_tail(&wait->node, &syncobj->cb_list);
-   } else if (!fence) {
-   wait->fence = dma_fence_get_stub();
} else {
wait->fence = fence;
}
@@ -856,8 +854,6 @@ static void syncobj_wait_syncobj_func(struct drm_syncobj 
*syncobj,
if (!fence || dma_fence_chain_find_seqno(&fence, wait->point)) {
dma_fence_put(fence);
return;
-   } else if (!fence) {
-   wait->fence = dma_fence_get_stub();
} else {
wait->fence = fence;
}


Like Chris said, dma_fence_chain_find_seqno() will update the fence 
pointer, so a subsequent check might not be dealing with the same value.


A bit cheeky, but...


-Lionel

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

Re: [PATCH] drm: atomic helper: fix W=1 warnings

2019-10-04 Thread Ville Syrjälä
On Fri, Oct 04, 2019 at 12:48:02PM +0200, Benjamin Gaignard wrote:
> Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
>  a écrit :
> >
> > On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
> > > Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
> > >  a écrit :
> > > >
> > > > On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> > > > > Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> > > > >  a écrit :
> > > > > >
> > > > > > On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
> > > > > > > Fix warnings with W=1.
> > > > > > > Few for_each macro set variables that are never used later.
> > > > > > > Prevent warning by marking these variables as __maybe_unused.
> > > > > > >
> > > > > > > Signed-off-by: Benjamin Gaignard 
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/drm_atomic_helper.c | 36 
> > > > > > > ++--
> > > > > > >  1 file changed, 18 insertions(+), 18 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
> > > > > > > b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > index aa16ea17ff9b..b69d17b0b9bd 100644
> > > > > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > > > > @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
> > > > > > > struct drm_encoder *encoder)
> > > > > > >  {
> > > > > > >   struct drm_crtc_state *crtc_state;
> > > > > > > - struct drm_connector *connector;
> > > > > > > + struct drm_connector __maybe_unused *connector;
> > > > > >
> > > > > > Rather ugly. IMO would be nicer if we could hide something inside
> > > > > > the iterator macros to suppress the warning.
> > > > >
> > > > > Ok but how ?
> > > > > connector is assigned in the macros but not used later and we can't
> > > > > set "__maybe_unused"
> > > > > in the macro.
> > > > > Does another keyword exist for that ?
> > > >
> > > > Stick a (void)(connector) into the macro?
> > >
> > > That could work but it will look strange inside the macro.
> > >
> > > >
> > > > Another (arguably cleaner) idea would be to remove the 
> > > > connector/crtc/plane
> > > > argument from the iterators entirely since it's redundant, and instead 
> > > > just
> > > > extract it from the appropriate new/old state as needed.
> > > >
> > > > We could then also add a for_each_connector_in_state()/etc. which omit
> > > > s the state arguments and just has the connector argument, for cases 
> > > > where
> > > > you don't care about the states when iterating.
> > >
> > > That may lead to get a macro for each possible combination of used 
> > > variables.
> >
> > We already have new/old/oldnew, so would "just" add one more.
> 
> Not just one, it will be one each new/old/oldnew macro to be able to 
> distinguish
> when connector is used or not.

What I'm suggesting is this:
for_each_connector_in_state(state, connector, i)
for_each_old_connector_in_state(state, old_conn_state, i)
for_each_new_connector_in_state(state, new_conn_state, i)
for_each_oldnew_connector_in_state(state, old_conn_state, new_conn_state, i)

So only four in total for each object type, instead of the current
three.

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

Re: [PATCH] drm/amdgpu: Fix build error when !CONFIG_PERF_EVENTS

2019-10-04 Thread Alex Deucher
On Thu, Oct 3, 2019 at 10:13 PM Huacai Chen  wrote:
>
> In previous release amdgpu_pmu.o is only built when CONFIG_PERF_EVENTS
> is selected. But after commit 64f55e629237e4752db1 ("drm/amdgpu: Add RAS
> EEPROM table.") it is duplicated in amdgpu-y. This change causes a build
> error when !CONFIG_PERF_EVENTS, so fix it.
>
> Fixes: 64f55e629237e4752db1 ("drm/amdgpu: Add RAS EEPROM table.")
> Cc: Andrey Grodzovsky 
> Cc: Luben Tuikov 
> Signed-off-by: Huacai Chen 

Already fixed:
https://cgit.freedesktop.org/drm/drm/commit/?h=drm-fixes&id=ec3e5c0f0c2b716e768c0eee0fec30d572939ef5

Thanks!

Alex

> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile 
> b/drivers/gpu/drm/amd/amdgpu/Makefile
> index 42e2c1f..00962a6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -54,7 +54,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
> amdgpu_gtt_mgr.o amdgpu_vram_mgr.o amdgpu_virt.o 
> amdgpu_atomfirmware.o \
> amdgpu_vf_error.o amdgpu_sched.o amdgpu_debugfs.o amdgpu_ids.o \
> amdgpu_gmc.o amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o amdgpu_vm_cpu.o \
> -   amdgpu_vm_sdma.o amdgpu_pmu.o amdgpu_discovery.o amdgpu_ras_eeprom.o 
> smu_v11_0_i2c.o
> +   amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o 
> smu_v11_0_i2c.o
>
>  amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
>
> --
> 2.7.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: atomic helper: fix W=1 warnings

2019-10-04 Thread Benjamin GAIGNARD

On 10/4/19 2:27 PM, Ville Syrjälä wrote:
> On Fri, Oct 04, 2019 at 12:48:02PM +0200, Benjamin Gaignard wrote:
>> Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
>>  a écrit :
>>> On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
 Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
  a écrit :
> On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
>> Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
>>  a écrit :
>>> On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
 Fix warnings with W=1.
 Few for_each macro set variables that are never used later.
 Prevent warning by marking these variables as __maybe_unused.

 Signed-off-by: Benjamin Gaignard 
 ---
   drivers/gpu/drm/drm_atomic_helper.c | 36 
 ++--
   1 file changed, 18 insertions(+), 18 deletions(-)

 diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
 b/drivers/gpu/drm/drm_atomic_helper.c
 index aa16ea17ff9b..b69d17b0b9bd 100644
 --- a/drivers/gpu/drm/drm_atomic_helper.c
 +++ b/drivers/gpu/drm/drm_atomic_helper.c
 @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
  struct drm_encoder *encoder)
   {
struct drm_crtc_state *crtc_state;
 - struct drm_connector *connector;
 + struct drm_connector __maybe_unused *connector;
>>> Rather ugly. IMO would be nicer if we could hide something inside
>>> the iterator macros to suppress the warning.
>> Ok but how ?
>> connector is assigned in the macros but not used later and we can't
>> set "__maybe_unused"
>> in the macro.
>> Does another keyword exist for that ?
> Stick a (void)(connector) into the macro?
 That could work but it will look strange inside the macro.

> Another (arguably cleaner) idea would be to remove the 
> connector/crtc/plane
> argument from the iterators entirely since it's redundant, and instead 
> just
> extract it from the appropriate new/old state as needed.
>
> We could then also add a for_each_connector_in_state()/etc. which omit
> s the state arguments and just has the connector argument, for cases where
> you don't care about the states when iterating.
 That may lead to get a macro for each possible combination of used 
 variables.
>>> We already have new/old/oldnew, so would "just" add one more.
>> Not just one, it will be one each new/old/oldnew macro to be able to 
>> distinguish
>> when connector is used or not.
> What I'm suggesting is this:
> for_each_connector_in_state(state, connector, i)
> for_each_old_connector_in_state(state, old_conn_state, i)
> for_each_new_connector_in_state(state, new_conn_state, i)
> for_each_oldnew_connector_in_state(state, old_conn_state, new_conn_state, i)
>
> So only four in total for each object type, instead of the current
> three.

You are missing these cases: old and connector, new and connector,

old and new and connector are needed together.

>

Re: [PATCH 03/11] drm/amdgpu: convert amdgpu_vm_it to half closed intervals

2019-10-04 Thread Christian König

Hi Michel,

Am 04.10.19 um 13:36 schrieb Michel Lespinasse:

On Fri, Oct 04, 2019 at 06:54:54AM +, Koenig, Christian wrote:

Am 03.10.19 um 22:18 schrieb Davidlohr Bueso:

The amdgpu_vm interval tree really wants [a, b) intervals,

NAK, we explicitly do need an [a, b[ interval here.

Hi Christian,

Just wanted to confirm where you stand on this patch, since I think
you reconsidered your initial position after first looking at 9/11
from this series.

I do not know the amdgpu code well, but I think the changes should be
fine - in struct amdgpu_bo_va_mapping, the "end" field will hold what
was previously stored in the "last" field, plus one. The expectation
is that overflows should not be an issue there, as "end" is explicitly
declared as an uint64, and as the code was previously computing
"last + 1" in many places.

Does that seem workable to you ?


No, we computed last + 1 in a couple of debug places were it doesn't 
hurt us and IIRC we currently cheat a bit because we use pfn instead of 
addresses on some other places.


But that is only a leftover from radeon and we need to fix that sooner 
or later, cause essentially the physical address space of the device is 
really full 64bits, e.g. 0x0-0x.


So that only fits into a 64bit int when we use half open/closed 
intervals, but would wrap around to zero if we use a closed interval.


I initially thought that the set was changing the interval tree into 
always using a closed interval, but that seems to have been a 
misunderstanding.


Regards,
Christian.



Thanks,



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

[Bug 110674] Crashes / Resets From AMDGPU / Radeon VII

2019-10-04 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=110674

--- Comment #155 from ReddestDream  ---
So, I've done some tests with 5.4-rc1 and it seems like I'm getting similar
results to line...@xcpp.org and sehell...@gmail.com. I'm using GNOME with
Wayland (which works fine with only 1 display). Sometimes it works for a while.
Sometimes I can't see the mouse cursor. Sometimes I get glitches all over the
screen containing pieces and parts of previous framebuffers. But, I mean, it's
better than 5.3 was, which was so bad I never could see anything and I would
get stuck on blackscreen. At least on 5.4-rc1 I've been able to manually switch
to a virtual console and reboot rather than force a reboot with the power
button.

Still hoping for some fix for this, but it's become less important to me as
further improvements to GNOME and MESA have made the Radeon VII + iGPU setup
I've been using run smoother. I've also discovered further issues on Windows
regarding the high memory clock when using multiple monitors with Radeon VII,
and it's been affecting performance there too. I'm considering just sticking
with 1 monitor only with for this machine/card. lol

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

Re: [PATCH -next 00/11] lib/interval-tree: move to half closed intervals

2019-10-04 Thread Michel Lespinasse
On Thu, Oct 03, 2019 at 01:32:50PM -0700, Matthew Wilcox wrote:
> On Thu, Oct 03, 2019 at 01:18:47PM -0700, Davidlohr Bueso wrote:
> > It has been discussed[1,2] that almost all users of interval trees would 
> > better
> > be served if the intervals were actually not [a,b], but instead [a, b). This
> 
> So how does a user represent a range from ULONG_MAX to ULONG_MAX now?
> 
> I think the problem is that large parts of the kernel just don't consider
> integer overflow.  Because we write in C, it's natural to write:
> 
>   for (i = start; i < end; i++)
> 
> and just assume that we never need to hit ULONG_MAX or UINT_MAX.
> If we're storing addresses, that's generally true -- most architectures
> don't allow addresses in the -PAGE_SIZE to ULONG_MAX range (or they'd
> have trouble with PTR_ERR).  If you're looking at file sizes, that's
> not true on 32-bit machines, and we've definitely seen filesystem bugs
> with files nudging up on 16TB (on 32 bit with 4k page size).  Or block
> driver bugs with similarly sized block devices.
> 
> So, yeah, easier to use.  But damning corner cases.

Yeah, I wanted to ask - is the case where pgoff == ULONG_MAX (i.e.,
last block of a file that is exactly 16TB) currently supported on
32-bit archs ?
I have no idea if I am supposed to care about this or not...

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.


Re: [PATCH v3] drm/amd/display: fix struct init in update_bounding_box

2019-10-04 Thread Alex Deucher
On Thu, Oct 3, 2019 at 4:35 PM Raul E Rangel  wrote:
>
> dcn20_resource.c:2636:9: error: missing braces around initializer 
> [-Werror=missing-braces]
>   struct _vcs_dpi_voltage_scaling_st 
> calculated_states[MAX_CLOCK_LIMIT_STATES] = {0};
>  ^
>
> Fixes: 7ed4e6352c16f ("drm/amd/display: Add DCN2 HW Sequencer and Resource")
>
> Signed-off-by: Raul E Rangel 

Applied.  thanks!

Alex

>
> ---
>
> Changes in v3:
> - Use memset
>
> Changes in v2:
> - Use {{0}} instead of {}
>
>  drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c 
> b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> index b949e202d6cb7..f72c26ae41def 100644
> --- a/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> +++ b/drivers/gpu/drm/amd/display/dc/dcn20/dcn20_resource.c
> @@ -2633,7 +2633,7 @@ static void cap_soc_clocks(
>  static void update_bounding_box(struct dc *dc, struct 
> _vcs_dpi_soc_bounding_box_st *bb,
> struct pp_smu_nv_clock_table *max_clocks, unsigned int 
> *uclk_states, unsigned int num_states)
>  {
> -   struct _vcs_dpi_voltage_scaling_st 
> calculated_states[MAX_CLOCK_LIMIT_STATES] = {0};
> +   struct _vcs_dpi_voltage_scaling_st 
> calculated_states[MAX_CLOCK_LIMIT_STATES];
> int i;
> int num_calculated_states = 0;
> int min_dcfclk = 0;
> @@ -2641,6 +2641,8 @@ static void update_bounding_box(struct dc *dc, struct 
> _vcs_dpi_soc_bounding_box_
> if (num_states == 0)
> return;
>
> +   memset(calculated_states, 0, sizeof(calculated_states));
> +
> if (dc->bb_overrides.min_dcfclk_mhz > 0)
> min_dcfclk = dc->bb_overrides.min_dcfclk_mhz;
> else
> --
> 2.23.0.444.g18eeb5a265-goog
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/amd/display: Make some functions static

2019-10-04 Thread Alex Deucher
On Fri, Oct 4, 2019 at 8:25 AM zhengbin  wrote:
>
> Fix sparse warnings:
>
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c:32:6: 
> warning: symbol 'lp_write_i2c' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c:42:6: 
> warning: symbol 'lp_read_i2c' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c:52:6: 
> warning: symbol 'lp_write_dpcd' was not declared. Should it be static?
> drivers/gpu/drm/amd/amdgpu/../display/amdgpu_dm/amdgpu_dm_hdcp.c:59:6: 
> warning: symbol 'lp_read_dpcd' was not declared. Should it be static?
>
> Reported-by: Hulk Robot 
> Signed-off-by: zhengbin 

Applied.  thanks!

Alex

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c 
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> index 2443c23..77181dd 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> @@ -29,7 +29,8 @@
>  #include "dm_helpers.h"
>  #include 
>
> -bool lp_write_i2c(void *handle, uint32_t address, const uint8_t *data, 
> uint32_t size)
> +static bool
> +lp_write_i2c(void *handle, uint32_t address, const uint8_t *data, uint32_t 
> size)
>  {
>
> struct dc_link *link = handle;
> @@ -39,7 +40,8 @@ bool lp_write_i2c(void *handle, uint32_t address, const 
> uint8_t *data, uint32_t
> return dm_helpers_submit_i2c(link->ctx, link, &cmd);
>  }
>
> -bool lp_read_i2c(void *handle, uint32_t address, uint8_t offset, uint8_t 
> *data, uint32_t size)
> +static bool
> +lp_read_i2c(void *handle, uint32_t address, uint8_t offset, uint8_t *data, 
> uint32_t size)
>  {
> struct dc_link *link = handle;
>
> @@ -49,14 +51,16 @@ bool lp_read_i2c(void *handle, uint32_t address, uint8_t 
> offset, uint8_t *data,
> return dm_helpers_submit_i2c(link->ctx, link, &cmd);
>  }
>
> -bool lp_write_dpcd(void *handle, uint32_t address, const uint8_t *data, 
> uint32_t size)
> +static bool
> +lp_write_dpcd(void *handle, uint32_t address, const uint8_t *data, uint32_t 
> size)
>  {
> struct dc_link *link = handle;
>
> return dm_helpers_dp_write_dpcd(link->ctx, link, address, data, size);
>  }
>
> -bool lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t 
> size)
> +static bool
> +lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t size)
>  {
> struct dc_link *link = handle;
>
> --
> 2.7.4
>
> ___
> amd-gfx mailing list
> amd-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH][next] drm/amdgpu: fix uninitialized variable pasid_mapping_needed

2019-10-04 Thread Alex Deucher
On Fri, Oct 4, 2019 at 3:28 AM Koenig, Christian
 wrote:
>
> Am 03.10.19 um 23:52 schrieb Colin King:
> > From: Colin Ian King 
> >
> > The boolean variable pasid_mapping_needed is not initialized and
> > there are code paths that do not assign it any value before it is
> > is read later.  Fix this by initializing pasid_mapping_needed to
> > false.
> >
> > Addresses-Coverity: ("Uninitialized scalar variable")
> > Fixes: 6817bf283b2b ("drm/amdgpu: grab the id mgr lock while accessing 
> > passid_mapping")
> > Signed-off-by: Colin Ian King 
>
> Reviewed-by: Christian König 
>

Applied.  thanks!

Alex

> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > index a2c797e34a29..be10e4b9a94d 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> > @@ -1055,7 +1055,7 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct 
> > amdgpu_job *job,
> >   id->oa_size != job->oa_size);
> >   bool vm_flush_needed = job->vm_needs_flush;
> >   struct dma_fence *fence = NULL;
> > - bool pasid_mapping_needed;
> > + bool pasid_mapping_needed = false;
> >   unsigned patch_offset = 0;
> >   int r;
> >
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] cec: add cec_adapter to cec_notifier_cec_adap_unregister()

2019-10-04 Thread Ville Syrjälä
On Fri, Oct 04, 2019 at 01:04:24PM +0200, Hans Verkuil wrote:
> It is possible for one HDMI connector to have multiple CEC adapters. The
> typical real-world scenario is that where one adapter is used when the device
> is in standby, and one that's better/smarter when the device is powered up.
> 
> The cec-notifier changes were made with that in mind, but I missed that in
> order to support this you need to tell cec_notifier_cec_adap_unregister()
> which adapter you are unregistering from the notifier.
> 
> Add this additional argument. It is currently unused, but once all drivers
> use this, the CEC core will be adapted for these use-cases.
> 
> Signed-off-by: Hans Verkuil 
> ---
> This patch should go in via the drm subsystem since all CEC adapters in the
> drm subsystem have been converted to use cec_notifier_cec_adap_unregister().
> The media subsystem still has older drm drivers that weren't converted to use
> cec_notifier_cec_adap_unregister().
> 
> This will only be a problem if a new CEC adapter driver is added to the media
> subsystem for v5.5, but I am not aware of any plans for that. Should it 
> happen,
> then that just means that the media subsystem needs to resolve a fairly 
> trivial
> merge conflict.
> 
> Ville, Mauro, can you review/ack?

Looks harmless enough to me :)

Reviewed-by: Ville Syrjälä 

> ---
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c 
> b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> index ac1e001d0882..70ab4fbdc23e 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi-cec.c
> @@ -285,7 +285,7 @@ static int dw_hdmi_cec_probe(struct platform_device *pdev)
> 
>   ret = cec_register_adapter(cec->adap, pdev->dev.parent);
>   if (ret < 0) {
> - cec_notifier_cec_adap_unregister(cec->notify);
> + cec_notifier_cec_adap_unregister(cec->notify, cec->adap);
>   return ret;
>   }
> 
> @@ -302,7 +302,7 @@ static int dw_hdmi_cec_remove(struct platform_device 
> *pdev)
>  {
>   struct dw_hdmi_cec *cec = platform_get_drvdata(pdev);
> 
> - cec_notifier_cec_adap_unregister(cec->notify);
> + cec_notifier_cec_adap_unregister(cec->notify, cec->adap);
>   cec_unregister_adapter(cec->adap);
> 
>   return 0;
> diff --git a/drivers/gpu/drm/i2c/tda9950.c b/drivers/gpu/drm/i2c/tda9950.c
> index a5a75bdeb7a5..5b03fdd1eaa4 100644
> --- a/drivers/gpu/drm/i2c/tda9950.c
> +++ b/drivers/gpu/drm/i2c/tda9950.c
> @@ -465,7 +465,7 @@ static int tda9950_probe(struct i2c_client *client,
> 
>   ret = cec_register_adapter(priv->adap, priv->hdmi);
>   if (ret < 0) {
> - cec_notifier_cec_adap_unregister(priv->notify);
> + cec_notifier_cec_adap_unregister(priv->notify, priv->adap);
>   return ret;
>   }
> 
> @@ -482,7 +482,7 @@ static int tda9950_remove(struct i2c_client *client)
>  {
>   struct tda9950_priv *priv = i2c_get_clientdata(client);
> 
> - cec_notifier_cec_adap_unregister(priv->notify);
> + cec_notifier_cec_adap_unregister(priv->notify, priv->adap);
>   cec_unregister_adapter(priv->adap);
> 
>   return 0;
> diff --git a/drivers/media/cec/cec-notifier.c 
> b/drivers/media/cec/cec-notifier.c
> index 4d82a5522072..7cf42b133dbc 100644
> --- a/drivers/media/cec/cec-notifier.c
> +++ b/drivers/media/cec/cec-notifier.c
> @@ -153,13 +153,14 @@ cec_notifier_cec_adap_register(struct device *hdmi_dev, 
> const char *conn_name,
>  }
>  EXPORT_SYMBOL_GPL(cec_notifier_cec_adap_register);
> 
> -void cec_notifier_cec_adap_unregister(struct cec_notifier *n)
> +void cec_notifier_cec_adap_unregister(struct cec_notifier *n,
> +   struct cec_adapter *adap)
>  {
>   if (!n)
>   return;
> 
>   mutex_lock(&n->lock);
> - n->cec_adap->notifier = NULL;
> + adap->notifier = NULL;

Maybe assert that the notifier and adapter know each other?

>   n->cec_adap = NULL;
>   n->callback = NULL;
>   mutex_unlock(&n->lock);
> diff --git a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c 
> b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> index 4a3b3810fd89..f048e8994785 100644
> --- a/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> +++ b/drivers/media/platform/cros-ec-cec/cros-ec-cec.c
> @@ -314,7 +314,8 @@ static int cros_ec_cec_probe(struct platform_device *pdev)
>   return 0;
> 
>  out_probe_notify:
> - cec_notifier_cec_adap_unregister(cros_ec_cec->notify);
> + cec_notifier_cec_adap_unregister(cros_ec_cec->notify,
> +  cros_ec_cec->adap);
>  out_probe_adapter:
>   cec_delete_adapter(cros_ec_cec->adap);
>   return ret;
> @@ -335,7 +336,8 @@ static int cros_ec_cec_remove(struct platform_device 
> *pdev)
>   return ret;
>   }
> 
> - cec_notifier_cec_adap_unregister(cros_ec_cec->notify);
> + cec_notifier_cec_adap_unregister(cros_ec_cec->notify,
> + 

Re: [PATCH][next] drm/amdgpu: remove redundant variable r and redundant return statement

2019-10-04 Thread Alex Deucher
On Fri, Oct 4, 2019 at 3:29 AM Koenig, Christian
 wrote:
>
> Am 03.10.19 um 23:40 schrieb Colin King:
> > From: Colin Ian King 
> >
> > There is a return statement that is not reachable and a variable that
> > is not used.  Remove them.
> >
> > Addresses-Coverity: ("Structurally dead code")
> > Fixes: de7b45babd9b ("drm/amdgpu: cleanup creating BOs at fixed location 
> > (v2)")
> > Signed-off-by: Colin Ian King 
>
> Reviewed-by: Christian König 

Applied.  Thanks!

Alex

>
> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 2 --
> >   1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 481e4c381083..814159f15633 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -1636,7 +1636,6 @@ static void amdgpu_ttm_fw_reserve_vram_fini(struct 
> > amdgpu_device *adev)
> >   static int amdgpu_ttm_fw_reserve_vram_init(struct amdgpu_device *adev)
> >   {
> >   uint64_t vram_size = adev->gmc.visible_vram_size;
> > - int r;
> >
> >   adev->fw_vram_usage.va = NULL;
> >   adev->fw_vram_usage.reserved_bo = NULL;
> > @@ -1651,7 +1650,6 @@ static int amdgpu_ttm_fw_reserve_vram_init(struct 
> > amdgpu_device *adev)
> > AMDGPU_GEM_DOMAIN_VRAM,
> > &adev->fw_vram_usage.reserved_bo,
> > &adev->fw_vram_usage.va);
> > - return r;
> >   }
> >
> >   /**
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: atomic helper: fix W=1 warnings

2019-10-04 Thread Ville Syrjälä
On Fri, Oct 04, 2019 at 12:36:56PM +, Benjamin GAIGNARD wrote:
> 
> On 10/4/19 2:27 PM, Ville Syrjälä wrote:
> > On Fri, Oct 04, 2019 at 12:48:02PM +0200, Benjamin Gaignard wrote:
> >> Le jeu. 3 oct. 2019 à 17:46, Ville Syrjälä
> >>  a écrit :
> >>> On Thu, Oct 03, 2019 at 05:37:15PM +0200, Benjamin Gaignard wrote:
>  Le jeu. 3 oct. 2019 à 17:05, Ville Syrjälä
>   a écrit :
> > On Thu, Oct 03, 2019 at 04:46:54PM +0200, Benjamin Gaignard wrote:
> >> Le jeu. 3 oct. 2019 à 16:27, Ville Syrjälä
> >>  a écrit :
> >>> On Mon, Sep 09, 2019 at 03:52:05PM +0200, Benjamin Gaignard wrote:
>  Fix warnings with W=1.
>  Few for_each macro set variables that are never used later.
>  Prevent warning by marking these variables as __maybe_unused.
> 
>  Signed-off-by: Benjamin Gaignard 
>  ---
>    drivers/gpu/drm/drm_atomic_helper.c | 36 
>  ++--
>    1 file changed, 18 insertions(+), 18 deletions(-)
> 
>  diff --git a/drivers/gpu/drm/drm_atomic_helper.c 
>  b/drivers/gpu/drm/drm_atomic_helper.c
>  index aa16ea17ff9b..b69d17b0b9bd 100644
>  --- a/drivers/gpu/drm/drm_atomic_helper.c
>  +++ b/drivers/gpu/drm/drm_atomic_helper.c
>  @@ -262,7 +262,7 @@ steal_encoder(struct drm_atomic_state *state,
>   struct drm_encoder *encoder)
>    {
> struct drm_crtc_state *crtc_state;
>  - struct drm_connector *connector;
>  + struct drm_connector __maybe_unused *connector;
> >>> Rather ugly. IMO would be nicer if we could hide something inside
> >>> the iterator macros to suppress the warning.
> >> Ok but how ?
> >> connector is assigned in the macros but not used later and we can't
> >> set "__maybe_unused"
> >> in the macro.
> >> Does another keyword exist for that ?
> > Stick a (void)(connector) into the macro?
>  That could work but it will look strange inside the macro.
> 
> > Another (arguably cleaner) idea would be to remove the 
> > connector/crtc/plane
> > argument from the iterators entirely since it's redundant, and instead 
> > just
> > extract it from the appropriate new/old state as needed.
> >
> > We could then also add a for_each_connector_in_state()/etc. which omit
> > s the state arguments and just has the connector argument, for cases 
> > where
> > you don't care about the states when iterating.
>  That may lead to get a macro for each possible combination of used 
>  variables.
> >>> We already have new/old/oldnew, so would "just" add one more.
> >> Not just one, it will be one each new/old/oldnew macro to be able to 
> >> distinguish
> >> when connector is used or not.
> > What I'm suggesting is this:
> > for_each_connector_in_state(state, connector, i)
> > for_each_old_connector_in_state(state, old_conn_state, i)
> > for_each_new_connector_in_state(state, new_conn_state, i)
> > for_each_oldnew_connector_in_state(state, old_conn_state, new_conn_state, i)
> >
> > So only four in total for each object type, instead of the current
> > three.
> 
> You are missing these cases: old and connector, new and connector,
> 
> old and new and connector are needed together.

No, that's redundant. You can always get the connector from
old/new_conn_state->connector if you need it.

-- 
Ville Syrjälä
Intel


Re: [PATCH -next 00/11] lib/interval-tree: move to half closed intervals

2019-10-04 Thread Michel Lespinasse
Hi Jason,

On Thu, Oct 3, 2019 at 5:26 PM Jason Gunthorpe  wrote:
> Hurm, this is not entirely accurate. Most users do actually want
> overlapping and multiple ranges. I just studied this extensively:

(Just curious, are you the person we discussed this with after the
Maple Tree talk at LPC 2019 ?)

I think we have two separate API problems there:
- overlapping vs non-overlapping intervals (the interval tree API
supports overlapping intervals, but some users are confused about
this)
- closed vs half-open interval definitions

It looks like you have been looking mostly at the first issue, which I
expect could simplify several interval tree users considerably, while
Davidlohr is addressing the second issue here.

> radeon_mn actually wants overlapping but seems to mis-understand the
> interval_tree API and actively tries hard to prevent overlapping at
> great cost and complexity. I have a patch to delete all of this and
> just be overlapping.
>
> amdgpu_mn copied the wrongness from radeon_mn
>
> All the DRM drivers are basically the same here, tracking userspace
> controlled VAs, so overlapping is essential
>
> hfi1/mmu_rb definitely needs overlapping as it is dealing with
> userspace VA ranges under control of userspace. As do the other
> infiniband users.

Do you have a handle on what usnic is doing with its intervals ?
usnic_uiom_insert_interval() has some complicated logic to avoid
having overlapping intervals, which is very confusing to me.

> vhost probably doesn't overlap in the normal case, but again userspace
> could trigger overlap in some pathalogical case.
>
> The [start,last] allows the interval to cover up to ULONG_MAX. I don't
> know if this is needed however. Many users are using userspace VAs
> here. Is there any kernel configuration where ULONG_MAX is a valid
> userspace pointer? Ie 32 bit 4G userspace? I don't know.
>
> Many users seemed to have bugs where they were taking a userspace
> controlled start + length and converting them into a start/end for
> interval tree without overflow protection (woops)
>
> Also I have a series already cooking to delete several of these
> interval tree users, which will terribly conflict with this :\
>
> Is it really necessary to make such churn for such a tiny API change?

My take is that this (Davidlohr's) patch series does not necessarily
need to be applied all at once - we could get the first change in
(adding the interval_tree_gen.h header), and convert the first few
users, without getting them all at once, as long as we have a plan for
finishing the work. So, if you have cleanups in progress in some of
the files, just tell us which ones and we can leave them out from the
first pass.

Thanks,

-- 
Michel "Walken" Lespinasse
A program is never fully debugged until the last user dies.


Re: Should regulator core support parsing OF based fwnode?

2019-10-04 Thread Jean-Jacques Hiblot



On 04/10/2019 13:39, Mark Brown wrote:

On Thu, Oct 03, 2019 at 10:27:26PM +0200, Jacek Anaszewski wrote:

On 10/3/19 9:41 PM, Mark Brown wrote:

Why would we want to do that?  We'd continue to support only DT systems,
just with code that's less obviously DT only and would need to put
checks in.  I'm not seeing an upside here.

For instance few weeks ago we had a patch [0] in the LED core switching
from using struct device's of_node property to fwnode for conveying
device property data. And this transition to fwnode property API can be
observed as a frequent pattern across subsystems.

For most subsystems the intent is to reuse DT bindings on embedded ACPI
systems via _DSD.


Recently there is an ongoing effort aiming to add generic support for
handling regulators in the LED core [1], but it turns out to require
bringing back initialization of of_node property for
devm_regulator_get_optional() to work properly.

Consumers should just be able to request a regulator without having to
worry about how that's being provided - they should have no knowledge at
all of firmware bindings or platform data for defining this.  If they
do that suggests there's an abstraction issue somewhere, what makes you
think that doing something with of_node is required?


The regulator core accesses consumer->of_node to get a phandle to a 
regulator's node. The trouble arises from the fact that the LED core 
does not populate of_node anymore, instead it populates fwnode. This 
allows the LED core to be agnostic of ACPI or OF to get the properties 
of a LED.


IMO it is better to populate both of_node and fwnode in the LED core at 
the moment. It has already been fixed this way for the platform driver 
[0], MTD [1] and PCI-OF [2].




Further, unless you have LEDs that work without power you probably
shouldn't be using _get_optional() for their supply.  That interface is
intended only for supplies that may be physically absent.


Not all LEDs have a regulator to provide the power. The power can be 
supplied by the LED controller for example.



[0] f94277af03ead0d3bf2 of/platform: Initialise dev->fwnode appropriately

[1] c176c6d7e932662668 mfd: core: Set fwnode for created devices

[2] 9b099a6c75e4ddceea PCI: OF: Initialize dev->fwnode appropriately



[Bug 111729] RX480 : random NULL pointer dereference on resume from suspend

2019-10-04 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111729

--- Comment #7 from m...@cschwarz.com ---
I began bisecting yesterday and discovered another bug that happens on suspend,
which makes it hard to determine the good/bad status of a build with regards to
_this_ bug in a timely manner.
Hence aborting any bisection attempts.

Maybe a new crowd of people runs into this when upgrading their Ubuntu LTS
systems :(

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

[Bug 111244] amdgpu kernel 5.2 blank display after resume from suspend

2019-10-04 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111244

--- Comment #33 from Andrey Grodzovsky  ---
OOO today

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

[Bug 111244] amdgpu kernel 5.2 blank display after resume from suspend

2019-10-04 Thread bugzilla-daemon
https://bugs.freedesktop.org/show_bug.cgi?id=111244

--- Comment #32 from mib...@gmx.at ---
Cannot reproduce it anymore as of 5.4rc1, seems like it's fixed for me!

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

Re: general protection fault in veth_get_stats64

2019-10-04 Thread Willem de Bruijn
On Wed, Oct 2, 2019 at 5:45 PM syzbot
 wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:a32db7e1 Add linux-next specific files for 20191002
> git tree:   linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=175ab7cd60
> kernel config:  https://syzkaller.appspot.com/x/.config?x=599cf05035799eef
> dashboard link: https://syzkaller.appspot.com/bug?extid=3f3e5e77d793c7a6fe6c
> compiler:   gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro:  https://syzkaller.appspot.com/x/repro.syz?x=12f8b94360
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16981a2560
>
> The bug was bisected to:
>
> commit 84da111de0b4be15bd500deff773f5116f39f7be
> Author: Linus Torvalds 
> Date:   Sat Sep 21 17:07:42 2019 +
>
>  Merge tag 'for-linus-hmm' of
> git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma
>
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=17c5584760
> final crash:https://syzkaller.appspot.com/x/report.txt?x=1425584760
> console output: https://syzkaller.appspot.com/x/log.txt?x=1025584760
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+3f3e5e77d793c7a6f...@syzkaller.appspotmail.com
> Fixes: 84da111de0b4 ("Merge tag 'for-linus-hmm' of
> git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma")
>
> RSP: 002b:7fff0ba6c998 EFLAGS: 0246 ORIG_RAX: 002e
> RAX: ffda RBX: 0003 RCX: 004424a9
> RDX:  RSI: 2140 RDI: 0003
> RBP:  R08: 0002 R09: 
> R10:  R11: 0246 R12: 
> R13: 0004 R14:  R15: 
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault:  [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 8605 Comm: syz-executor330 Not tainted 5.4.0-rc1-next-20191002
> #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:veth_stats_rx drivers/net/veth.c:322 [inline]
> RIP: 0010:veth_get_stats64+0x523/0x900 drivers/net/veth.c:356
> Code: 89 85 60 ff ff ff e8 6c 74 31 fd 49 63 c7 48 69 c0 c0 02 00 00 48 03
> 85 60 ff ff ff 48 8d b8 a0 01 00 00 48 89 fa 48 c1 ea 03 <42> 80 3c 32 00
> 0f 85 c9 02 00 00 48 8d b8 a8 01 00 00 48 8b 90 a0
> RSP: 0018:88809996ed00 EFLAGS: 00010202
> RAX:  RBX: 0001 RCX: 84418daf
> RDX: 0034 RSI: 84418e04 RDI: 01a0
> RBP: 88809996ede0 R08: 888093182180 R09: ed1013202d6a
> R10: ed1013202d69 R11: 888099016b4f R12: 
> R13:  R14: dc00 R15: 
> FS:  01f4a880() GS:8880ae90() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 2140 CR3: 9a80b000 CR4: 001406e0
> DR0:  DR1:  DR2: 
> DR3:  DR6: fffe0ff0 DR7: 0400
> Call Trace:
>   dev_get_stats+0x8e/0x280 net/core/dev.c:9220
>   rtnl_fill_stats+0x4d/0xac0 net/core/rtnetlink.c:1191
>   rtnl_fill_ifinfo+0x10ad/0x3af0 net/core/rtnetlink.c:1717
>   rtmsg_ifinfo_build_skb+0xc9/0x1a0 net/core/rtnetlink.c:3635
>   rtmsg_ifinfo_event.part.0+0x43/0xe0 net/core/rtnetlink.c:3667
>   rtmsg_ifinfo_event net/core/rtnetlink.c:3678 [inline]
>   rtmsg_ifinfo+0x8d/0xa0 net/core/rtnetlink.c:3676
>   __dev_notify_flags+0x235/0x2c0 net/core/dev.c:7757
>   rtnl_configure_link+0x175/0x250 net/core/rtnetlink.c:2968
>   __rtnl_newlink+0x10c4/0x16d0 net/core/rtnetlink.c:3285
>   rtnl_newlink+0x69/0xa0 net/core/rtnetlink.c:3325
>   rtnetlink_rcv_msg+0x463/0xb00 net/core/rtnetlink.c:5386
>   netlink_rcv_skb+0x177/0x450 net/netlink/af_netlink.c:2477
>   rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5404
>   netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
>   netlink_unicast+0x531/0x710 net/netlink/af_netlink.c:1328
>   netlink_sendmsg+0x8a5/0xd60 net/netlink/af_netlink.c:1917
>   sock_sendmsg_nosec net/socket.c:638 [inline]
>   sock_sendmsg+0xd7/0x130 net/socket.c:658
>   ___sys_sendmsg+0x803/0x920 net/socket.c:2312
>   __sys_sendmsg+0x105/0x1d0 net/socket.c:2357
>   __do_sys_sendmsg net/socket.c:2366 [inline]
>   __se_sys_sendmsg net/socket.c:2364 [inline]
>   __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2364
>   do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
>   entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x4424a9
> Code: e8 9c 07 03 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7
> 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
> ff 0f 83 3b 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:7fff0ba6c998 EFLAGS: 0246 ORIG_RAX: 002e
> RAX: ffda RBX: 0003 RCX: 0044

[PATCH] drm/i915: customize DPCD brightness control for specific panel

2019-10-04 Thread Lee Shawn C
This panel (manufacturer is SDC, product ID is 0x4141)
used manufacturer defined DPCD register to control brightness
that not defined in eDP spec so far. This change follow panel
vendor's instruction to support brightness adjustment.

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

Cc: Jani Nikula 
Cc: Maarten Lankhorst 
Cc: Gustavo Padovan 
Cc: Cooper Chiou 
Signed-off-by: Lee Shawn C 
---
 drivers/gpu/drm/drm_edid.c|   6 +-
 drivers/gpu/drm/i915/display/intel_dp.c   |   7 +
 .../drm/i915/display/intel_dp_aux_backlight.c | 143 +-
 include/drm/drm_edid.h|   3 +
 4 files changed, 153 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3c9703b08491..5aee0ebc200e 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -211,6 +211,9 @@ static const struct edid_quirk {
 
/* OSVR HDK and HDK2 VR Headsets */
{ "SVR", 0x1019, EDID_QUIRK_NON_DESKTOP },
+
+   /* Samsung eDP panel */
+   { "SDC", 0x4141, EDID_QUIRK_NON_STD_BRIGHTNESS_CONTROL },
 };
 
 /*
@@ -1938,7 +1941,7 @@ static bool edid_vendor(const struct edid *edid, const 
char *vendor)
  *
  * This tells subsequent routines what fixes they need to apply.
  */
-static u32 edid_get_quirks(const struct edid *edid)
+u32 edid_get_quirks(const struct edid *edid)
 {
const struct edid_quirk *quirk;
int i;
@@ -1953,6 +1956,7 @@ static u32 edid_get_quirks(const struct edid *edid)
 
return 0;
 }
+EXPORT_SYMBOL(edid_get_quirks);
 
 #define MODE_SIZE(m) ((m)->hdisplay * (m)->vdisplay)
 #define MODE_REFRESH_DIFF(c,t) (abs((c) - (t)))
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c 
b/drivers/gpu/drm/i915/display/intel_dp.c
index 2b1e71f992b0..89193bd2d8ea 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7097,6 +7097,13 @@ static bool intel_edp_init_connector(struct intel_dp 
*intel_dp,
}
intel_connector->edid = edid;
 
+   if (edid_get_quirks(intel_connector->edid) ==
+   EDID_QUIRK_NON_STD_BRIGHTNESS_CONTROL) {
+   i915_modparams.enable_dpcd_backlight = true;
+   i915_modparams.fastboot = false;
+   DRM_DEBUG_KMS("Using specific DPCD to control brightness\n");
+   }
+
fixed_mode = intel_panel_edid_fixed_mode(intel_connector);
if (fixed_mode)
downclock_mode = intel_dp_drrs_init(intel_connector, 
fixed_mode);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c 
b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index 020422da2ae2..7d9a2249cfb1 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -25,6 +25,117 @@
 #include "intel_display_types.h"
 #include "intel_dp_aux_backlight.h"
 
+#define DPCD_EDP_GETSET_CTRL_PARAMS0x344
+#define DPCD_EDP_CONTENT_LUMINANCE 0x346
+#define DPCD_EDP_PANEL_LUMINANCE_OVERRIDE  0x34a
+#define DPCD_EDP_BRIGHTNESS_NITS   0x354
+#define DPCD_EDP_BRIGHTNESS_OPTIMIZATION   0x358
+
+#define EDP_CUSTOMIZE_MAX_BRIGHTNESS_LEVEL (512)
+
+static uint32_t intel_dp_aux_get_customize_backlight(struct intel_connector 
*connector)
+{
+   struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+   uint8_t read_val[2] = { 0x0 };
+
+   if (drm_dp_dpcd_read(&intel_dp->aux, DPCD_EDP_BRIGHTNESS_NITS,
+&read_val, sizeof(read_val)) < 0) {
+   DRM_DEBUG_KMS("Failed to read DPCD register %x\n",
+   DPCD_EDP_BRIGHTNESS_NITS);
+   return 0;
+   }
+
+   return (read_val[1] << 8 | read_val[0]);
+}
+
+static void
+intel_dp_aux_set_customize_backlight(const struct drm_connector_state 
*conn_state, u32 level)
+{
+   struct intel_connector *connector = 
to_intel_connector(conn_state->connector);
+   struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+   struct intel_panel *panel = &connector->panel;
+   uint8_t new_vals[4];
+
+   if (level > panel->backlight.max)
+   level = panel->backlight.max;
+
+   new_vals[0] = level & 0xFF;
+   new_vals[1] = (level & 0xFF00) >> 8;
+   new_vals[2] = 0;
+   new_vals[3] = 0;
+
+   if (drm_dp_dpcd_write(&intel_dp->aux, DPCD_EDP_BRIGHTNESS_NITS, 
new_vals, 4) < 0)
+   DRM_DEBUG_KMS("Failed to write aux backlight level\n");
+}
+
+static void intel_dp_aux_enable_customize_backlight(const struct 
intel_crtc_state *crtc_state,
+ const struct drm_connector_state 
*conn_state)
+{
+   struct intel_connector *connector = 
to_intel_connector(conn_state->connector);
+   struct intel_dp *intel_dp = enc_to_intel_dp(&connector->encoder->base);
+   uint8_t read_val[4], i;
+   uint8_t write_val[8] = {0x00, 0x00, 0xF0

Re: [PATCH 1/2] drm: bridge: adv7511: Enable SPDIF DAI

2019-10-04 Thread kbuild test robot
Hi Bogdan,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[cannot apply to v5.4-rc1 next-20191004]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:
https://github.com/0day-ci/linux/commits/Bogdan-Togorean/drm-bridge-adv7511-Enable-SPDIF-DAI/20191004-205455
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 7.4.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=xtensa 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/bridge/adv7511/adv7511_audio.c: In function 
'adv7511_hdmi_hw_params':
>> drivers/gpu/drm/bridge/adv7511/adv7511_audio.c:123:16: warning: this 
>> statement may fall through [-Wimplicit-fallthrough=]
  audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
   drivers/gpu/drm/bridge/adv7511/adv7511_audio.c:124:2: note: here
 default:
 ^~~

vim +123 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c

55  
56  int adv7511_hdmi_hw_params(struct device *dev, void *data,
57 struct hdmi_codec_daifmt *fmt,
58 struct hdmi_codec_params *hparms)
59  {
60  struct adv7511 *adv7511 = dev_get_drvdata(dev);
61  unsigned int audio_source, i2s_format = 0;
62  unsigned int invert_clock;
63  unsigned int rate;
64  unsigned int len;
65  
66  switch (hparms->sample_rate) {
67  case 32000:
68  rate = ADV7511_SAMPLE_FREQ_32000;
69  break;
70  case 44100:
71  rate = ADV7511_SAMPLE_FREQ_44100;
72  break;
73  case 48000:
74  rate = ADV7511_SAMPLE_FREQ_48000;
75  break;
76  case 88200:
77  rate = ADV7511_SAMPLE_FREQ_88200;
78  break;
79  case 96000:
80  rate = ADV7511_SAMPLE_FREQ_96000;
81  break;
82  case 176400:
83  rate = ADV7511_SAMPLE_FREQ_176400;
84  break;
85  case 192000:
86  rate = ADV7511_SAMPLE_FREQ_192000;
87  break;
88  default:
89  return -EINVAL;
90  }
91  
92  switch (hparms->sample_width) {
93  case 16:
94  len = ADV7511_I2S_SAMPLE_LEN_16;
95  break;
96  case 18:
97  len = ADV7511_I2S_SAMPLE_LEN_18;
98  break;
99  case 20:
   100  len = ADV7511_I2S_SAMPLE_LEN_20;
   101  break;
   102  case 24:
   103  len = ADV7511_I2S_SAMPLE_LEN_24;
   104  break;
   105  default:
   106  return -EINVAL;
   107  }
   108  
   109  switch (fmt->fmt) {
   110  case HDMI_I2S:
   111  audio_source = ADV7511_AUDIO_SOURCE_I2S;
   112  i2s_format = ADV7511_I2S_FORMAT_I2S;
   113  break;
   114  case HDMI_RIGHT_J:
   115  audio_source = ADV7511_AUDIO_SOURCE_I2S;
   116  i2s_format = ADV7511_I2S_FORMAT_RIGHT_J;
   117  break;
   118  case HDMI_LEFT_J:
   119  audio_source = ADV7511_AUDIO_SOURCE_I2S;
   120  i2s_format = ADV7511_I2S_FORMAT_LEFT_J;
   121  break;
   122  case HDMI_SPDIF:
 > 123  audio_source = ADV7511_AUDIO_SOURCE_SPDIF;
   124  default:
   125  return -EINVAL;
   126  }
   127  
   128  invert_clock = fmt->bit_clk_inv;
   129  
   130  regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_SOURCE, 
0x70,
   131 audio_source << 4);
   132  regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG, 
BIT(6),
   133 invert_clock << 6);
   134  regmap_update_bits(adv7511->regmap, ADV7511_REG_I2S_CONFIG, 
0x03,
   135 i2s_format);
   136  
   137  adv7511->audio_source = audio_source;
   138  
   139  adv7511->f_audio = hparms->sample_rate;
   140  
   141  adv7511_update_cts_n(adv7511);
   142  

Re: [PATCH] drm/i810: Prevent underflow in ioctl

2019-10-04 Thread Chris Wilson
Quoting Dan Carpenter (2019-10-04 11:22:51)
> The "used" variables here come from the user in the ioctl and it can be
> negative.  It could result in an out of bounds write.
> 
> Signed-off-by: Dan Carpenter 
> ---
>  drivers/gpu/drm/i810/i810_dma.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i810/i810_dma.c b/drivers/gpu/drm/i810/i810_dma.c
> index 2a77823b8e9a..e66c38332df4 100644
> --- a/drivers/gpu/drm/i810/i810_dma.c
> +++ b/drivers/gpu/drm/i810/i810_dma.c
> @@ -728,7 +728,7 @@ static void i810_dma_dispatch_vertex(struct drm_device 
> *dev,
> if (nbox > I810_NR_SAREA_CLIPRECTS)
> nbox = I810_NR_SAREA_CLIPRECTS;
>  
> -   if (used > 4 * 1024)
> +   if (used < 0 || used > 4 * 1024)
> used = 0;

Yes, as passed to the GPU instruction, negative used is invalid.

Then it is used as an offset into a memblock, where a negative offset
would be very bad.

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

[PATCH v3] drm/fourcc: Add Arm 16x16 block modifier

2019-10-04 Thread Ayan Halder
From: Raymond Smith 

Add the DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED modifier to
denote the 16x16 block u-interleaved format used in Arm Utgard and
Midgard GPUs.

Changes from v1:-
1. Reserved the upper four bits (out of the 56 bits assigned to each vendor)
to denote the category of Arm specific modifiers. Currently, we have two
categories ie AFBC and MISC.

Changes from v2:-
1. Preserved Ray's authorship
2. Cleanups/changes suggested by Brian
3. Added r-bs of Brian and Qiang

Signed-off-by: Raymond Smith 
Signed-off-by: Ayan kumar halder 
Reviewed-by: Brian Starkey 
Reviewed-by: Qiang Yu 
---
 include/uapi/drm/drm_fourcc.h | 26 +-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
index 3feeaa3f987a..2376d36ea573 100644
--- a/include/uapi/drm/drm_fourcc.h
+++ b/include/uapi/drm/drm_fourcc.h
@@ -648,7 +648,21 @@ extern "C" {
  * Further information on the use of AFBC modifiers can be found in
  * Documentation/gpu/afbc.rst
  */
-#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode)   fourcc_mod_code(ARM, 
__afbc_mode)
+
+/*
+ * The top 4 bits (out of the 56 bits alloted for specifying vendor specific
+ * modifiers) denote the category for modifiers. Currently we have only two
+ * categories of modifiers ie AFBC and MISC. We can have a maximum of sixteen
+ * different categories.
+ */
+#define DRM_FORMAT_MOD_ARM_CODE(__type, __val) \
+   fourcc_mod_code(ARM, ((__u64)(__type) << 52) | ((__val) & 
0x000fULL))
+
+#define DRM_FORMAT_MOD_ARM_TYPE_AFBC 0x00
+#define DRM_FORMAT_MOD_ARM_TYPE_MISC 0x01
+
+#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) \
+   DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_AFBC, __afbc_mode)
 
 /*
  * AFBC superblock size
@@ -742,6 +756,16 @@ extern "C" {
  */
 #define AFBC_FORMAT_MOD_BCH (1ULL << 11)
 
+/*
+ * Arm 16x16 Block U-Interleaved modifier
+ *
+ * This is used by Arm Mali Utgard and Midgard GPUs. It divides the image
+ * into 16x16 pixel blocks. Blocks are stored linearly in order, but pixels
+ * in the block are reordered.
+ */
+#define DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED \
+   DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_MISC, 1ULL)
+
 /*
  * Allwinner tiled modifier
  *
-- 
2.23.0



[Outreachy][VKMS] Apply to VKMS

2019-10-04 Thread Lorrany dos santos
Hi guys,

I'm Lorrany Azevedo, and I'm participating in the application phase for the
outreachy internships. I'm interested in making a contribution to VKMS, and
I would like to ask for tips on how I can do this,  I'm new to the
community and I never made any contribution to the FOSS.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm/edid: Extract drm_mode_cea_vic()

2019-10-04 Thread Ville Syrjala
From: Ville Syrjälä 

Extract the logic to compute the final CEA VIC to a small helper.
We'll reorder it a bit to make future modifications more
straightforward. No function changes.

Cc: Wayne Lin 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_edid.c | 53 +-
 1 file changed, 30 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 3df8267adab9..495b7fb4d9ef 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5160,6 +5160,35 @@ drm_hdmi_infoframe_set_hdr_metadata(struct 
hdmi_drm_infoframe *frame,
 }
 EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
 
+static u8 drm_mode_cea_vic(struct drm_connector *connector,
+  const struct drm_display_mode *mode)
+{
+   u8 vendor_if_vic = drm_match_hdmi_mode(mode);
+   bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
+   u8 vic;
+
+   /*
+* HDMI spec says if a mode is found in HDMI 1.4b 4K modes
+* we should send its VIC in vendor infoframes, else send the
+* VIC in AVI infoframes. Lets check if this mode is present in
+* HDMI 1.4b 4K modes
+*/
+   if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
+   return 0;
+
+   vic = drm_match_cea_mode(mode);
+
+   /*
+* HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
+* HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
+* have to make sure we dont break HDMI 1.4 sinks.
+*/
+   if (!is_hdmi2_sink(connector) && vic > 64)
+   return 0;
+
+   return vic;
+}
+
 /**
  * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
  *  data from a DRM display mode
@@ -5187,29 +5216,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct 
hdmi_avi_infoframe *frame,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
frame->pixel_repeat = 1;
 
-   frame->video_code = drm_match_cea_mode(mode);
-
-   /*
-* HDMI 1.4 VIC range: 1 <= VIC <= 64 (CEA-861-D) but
-* HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
-* have to make sure we dont break HDMI 1.4 sinks.
-*/
-   if (!is_hdmi2_sink(connector) && frame->video_code > 64)
-   frame->video_code = 0;
-
-   /*
-* HDMI spec says if a mode is found in HDMI 1.4b 4K modes
-* we should send its VIC in vendor infoframes, else send the
-* VIC in AVI infoframes. Lets check if this mode is present in
-* HDMI 1.4b 4K modes
-*/
-   if (frame->video_code) {
-   u8 vendor_if_vic = drm_match_hdmi_mode(mode);
-   bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
-
-   if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
-   frame->video_code = 0;
-   }
+   frame->video_code = drm_mode_cea_vic(connector, mode);
 
frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
 
-- 
2.21.0

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

[PATCH 4/4] drm/edid: Prep for HDMI VIC aspect ratio (WIP)

2019-10-04 Thread Ville Syrjala
From: Ville Syrjälä 

I think this should provide most of necessary logic for
adding aspecr ratios to the HDMI 4k modes.

Cc: Wayne Lin 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_edid.c | 37 +++--
 1 file changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index c7f9f7ca75a2..c76814edc784 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3210,6 +3210,11 @@ static enum hdmi_picture_aspect 
drm_get_cea_aspect_ratio(const u8 video_code)
return edid_cea_modes[video_code].picture_aspect_ratio;
 }
 
+static enum hdmi_picture_aspect drm_get_hdmi_aspect_ratio(const u8 video_code)
+{
+   return edid_4k_modes[video_code].picture_aspect_ratio;
+}
+
 /*
  * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
  * specific block).
@@ -3236,6 +3241,9 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const 
struct drm_display_mode *to_
if (!to_match->clock)
return 0;
 
+   if (to_match->picture_aspect_ratio)
+   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
unsigned int clock1, clock2;
@@ -3271,6 +3279,9 @@ static u8 drm_match_hdmi_mode(const struct 
drm_display_mode *to_match)
if (!to_match->clock)
return 0;
 
+   if (to_match->picture_aspect_ratio)
+   match_flags |= DRM_MODE_MATCH_ASPECT_RATIO;
+
for (vic = 1; vic < ARRAY_SIZE(edid_4k_modes); vic++) {
const struct drm_display_mode *hdmi_mode = &edid_4k_modes[vic];
unsigned int clock1, clock2;
@@ -5218,6 +5229,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct 
hdmi_avi_infoframe *frame,
 const struct drm_display_mode *mode)
 {
enum hdmi_picture_aspect picture_aspect;
+   u8 vic, hdmi_vic;
int err;
 
if (!frame || !mode)
@@ -5230,7 +5242,8 @@ drm_hdmi_avi_infoframe_from_display_mode(struct 
hdmi_avi_infoframe *frame,
if (mode->flags & DRM_MODE_FLAG_DBLCLK)
frame->pixel_repeat = 1;
 
-   frame->video_code = drm_mode_cea_vic(connector, mode);
+   vic = drm_mode_cea_vic(connector, mode);
+   hdmi_vic = drm_mode_hdmi_vic(connector, mode);
 
frame->picture_aspect = HDMI_PICTURE_ASPECT_NONE;
 
@@ -5244,11 +5257,15 @@ drm_hdmi_avi_infoframe_from_display_mode(struct 
hdmi_avi_infoframe *frame,
 
/*
 * Populate picture aspect ratio from either
-* user input (if specified) or from the CEA mode list.
+* user input (if specified) or from the CEA/HDMI mode lists.
 */
picture_aspect = mode->picture_aspect_ratio;
-   if (picture_aspect == HDMI_PICTURE_ASPECT_NONE)
-   picture_aspect = drm_get_cea_aspect_ratio(frame->video_code);
+   if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) {
+   if (vic)
+   picture_aspect = drm_get_cea_aspect_ratio(vic);
+   else if (hdmi_vic)
+   picture_aspect = drm_get_hdmi_aspect_ratio(hdmi_vic);
+   }
 
/*
 * The infoframe can't convey anything but none, 4:3
@@ -5256,12 +5273,20 @@ drm_hdmi_avi_infoframe_from_display_mode(struct 
hdmi_avi_infoframe *frame,
 * we can only satisfy it by specifying the right VIC.
 */
if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) {
-   if (picture_aspect !=
-   drm_get_cea_aspect_ratio(frame->video_code))
+   if (vic) {
+   if (picture_aspect != drm_get_cea_aspect_ratio(vic))
+   return -EINVAL;
+   } else if (hdmi_vic) {
+   if (picture_aspect != 
drm_get_hdmi_aspect_ratio(hdmi_vic))
+   return -EINVAL;
+   } else {
return -EINVAL;
+   }
+
picture_aspect = HDMI_PICTURE_ASPECT_NONE;
}
 
+   frame->video_code = vic;
frame->picture_aspect = picture_aspect;
frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE;
frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
-- 
2.21.0

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

[PATCH 1/4] drm/edid: Make drm_get_cea_aspect_ratio() static

2019-10-04 Thread Ville Syrjala
From: Ville Syrjälä 

drm_get_cea_aspect_ratio() is not used outside drm_edid.c.
Make it static.

Cc: Wayne Lin 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_edid.c | 10 +-
 include/drm/drm_edid.h |  1 -
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 0552175313cb..3df8267adab9 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3205,18 +3205,10 @@ static bool drm_valid_cea_vic(u8 vic)
return vic > 0 && vic < ARRAY_SIZE(edid_cea_modes);
 }
 
-/**
- * drm_get_cea_aspect_ratio - get the picture aspect ratio corresponding to
- * the input VIC from the CEA mode list
- * @video_code: ID given to each of the CEA modes
- *
- * Returns picture aspect ratio
- */
-enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code)
+static enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code)
 {
return edid_cea_modes[video_code].picture_aspect_ratio;
 }
-EXPORT_SYMBOL(drm_get_cea_aspect_ratio);
 
 /*
  * Calculate the alternate clock for HDMI modes (those from the HDMI vendor
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index b9719418c3d2..efce675abf07 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -481,7 +481,6 @@ int drm_add_edid_modes(struct drm_connector *connector, 
struct edid *edid);
 int drm_add_override_edid_modes(struct drm_connector *connector);
 
 u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
-enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
 bool drm_detect_hdmi_monitor(struct edid *edid);
 bool drm_detect_monitor_audio(struct edid *edid);
 enum hdmi_quantization_range
-- 
2.21.0

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

[PATCH 3/4] drm/edid: Fix HDMI VIC handling

2019-10-04 Thread Ville Syrjala
From: Ville Syrjälä 

Extract drm_mode_hdmi_vic() to correctly calculate the final HDMI
VIC for us. Currently this is being done a bit differently between
the AVI and HDMI infoframes. Let's get both to agree on this.

We need to allow the case where a mode is both 3D and has a HDMI
VIC. Currently we'll just refuse to generate the HDMI infoframe when
we really should be setting HDMI VIC to 0 and instead enabling 3D
stereo signalling.

If the sink doesn't even support the HDMI infoframe we should
not be picking the HDMI VIC in favor of the CEA VIC, because then
we'll end up not sending either VIC in the end.

Cc: Wayne Lin 
Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_edid.c | 37 +
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 495b7fb4d9ef..c7f9f7ca75a2 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -5160,11 +5160,25 @@ drm_hdmi_infoframe_set_hdr_metadata(struct 
hdmi_drm_infoframe *frame,
 }
 EXPORT_SYMBOL(drm_hdmi_infoframe_set_hdr_metadata);
 
+static u8 drm_mode_hdmi_vic(struct drm_connector *connector,
+   const struct drm_display_mode *mode)
+{
+   bool has_hdmi_infoframe = connector ?
+   connector->display_info.has_hdmi_infoframe : false;
+
+   if (!has_hdmi_infoframe)
+   return 0;
+
+   /* No HDMI VIC when signalling 3D video format */
+   if (mode->flags & DRM_MODE_FLAG_3D_MASK)
+   return 0;
+
+   return drm_match_hdmi_mode(mode);
+}
+
 static u8 drm_mode_cea_vic(struct drm_connector *connector,
   const struct drm_display_mode *mode)
 {
-   u8 vendor_if_vic = drm_match_hdmi_mode(mode);
-   bool is_s3d = mode->flags & DRM_MODE_FLAG_3D_MASK;
u8 vic;
 
/*
@@ -5173,7 +5187,7 @@ static u8 drm_mode_cea_vic(struct drm_connector 
*connector,
 * VIC in AVI infoframes. Lets check if this mode is present in
 * HDMI 1.4b 4K modes
 */
-   if (drm_valid_hdmi_vic(vendor_if_vic) && !is_s3d)
+   if (drm_mode_hdmi_vic(connector, mode))
return 0;
 
vic = drm_match_cea_mode(mode);
@@ -5433,8 +5447,6 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct 
hdmi_vendor_infoframe *frame,
bool has_hdmi_infoframe = connector ?
connector->display_info.has_hdmi_infoframe : false;
int err;
-   u32 s3d_flags;
-   u8 vic;
 
if (!frame || !mode)
return -EINVAL;
@@ -5442,8 +5454,9 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct 
hdmi_vendor_infoframe *frame,
if (!has_hdmi_infoframe)
return -EINVAL;
 
-   vic = drm_match_hdmi_mode(mode);
-   s3d_flags = mode->flags & DRM_MODE_FLAG_3D_MASK;
+   err = hdmi_vendor_infoframe_init(frame);
+   if (err < 0)
+   return err;
 
/*
 * Even if it's not absolutely necessary to send the infoframe
@@ -5454,15 +5467,7 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct 
hdmi_vendor_infoframe *frame,
 * mode if the source simply stops sending the infoframe when
 * it wants to switch from 3D to 2D.
 */
-
-   if (vic && s3d_flags)
-   return -EINVAL;
-
-   err = hdmi_vendor_infoframe_init(frame);
-   if (err < 0)
-   return err;
-
-   frame->vic = vic;
+   frame->vic = drm_mode_hdmi_vic(connector, mode);
frame->s3d_struct = s3d_structure_from_display_mode(mode);
 
return 0;
-- 
2.21.0

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

Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0

2019-10-04 Thread Ville Syrjälä
On Fri, Oct 04, 2019 at 10:41:20AM +, Lin, Wayne wrote:
> 
> 
> 
> From: Ville Syrjälä 
> Sent: Thursday, October 3, 2019 21:29
> To: Lin, Wayne 
> Cc: dri-devel@lists.freedesktop.org ; 
> amd-...@lists.freedesktop.org ; Li, Sun peng 
> (Leo) ; Kazlauskas, Nicholas 
> Subject: Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0
> 
> On Thu, Oct 03, 2019 at 06:49:05AM +, Lin, Wayne wrote:
> >
> >
> > 
> > From: Ville Syrjälä 
> > Sent: Wednesday, October 2, 2019 19:58
> > To: Lin, Wayne 
> > Cc: dri-devel@lists.freedesktop.org ; 
> > amd-...@lists.freedesktop.org ; Li, Sun peng 
> > (Leo) ; Kazlauskas, Nicholas 
> > 
> > Subject: Re: [PATCH] drm/drm_edid: correct VIC and HDMI_VIC under HDMI 2.0
> >
> > On Tue, Sep 24, 2019 at 01:26:21PM +0800, Wayne Lin wrote:
> > > In HDMI 1.4 defines 4k modes without specific aspect ratio.
> > > However, in HDMI 2.0, adds aspect ratio attribute to distinguish different
> > > 4k modes.
> > >
> > > According to Appendix E of HDMI 2.0 spec, source should use VSIF to
> > > indicate VIC mode only when the mode is one defined in HDMI 1.4b 4K modes.
> > > Otherwise, use AVI infoframes to convey VIC.
> > >
> > > eg: VIC_103 should use AVI infoframes and VIC_93 use VSIF
> > >
> > > When the sink is HDMI 2.0, current code in
> > > drm_hdmi_avi_infoframe_from_display_mode will also force mode VIC_103 to
> > > have VIC value 0. This violates the spec and needs to be corrected.
> >
> > > Where is that being done? We only set the AVI VIC to zero if we're going
> > > to use the HDMI VIC instead.
> >
> > Appreciate for your time and apologize for not explaining it clearly.
> > Current code in drm_hdmi_avi_infoframe_from_display_mode() will call
> > drm_match_hdmi_mode() to set up vendor_if_vic. By checking
> > drm_valid_hdmi_vic(vendor_if_vic) to see if the vic info should be conveyed 
> > by avi
> > or not.
> >
> > But in drm_match_hdmi_mode(), code doesn't enable match_flags with
> > DRM_MODE_MATCH_ASPECT_RATIO. I think it's due to HDMI1.4b doesn't specify
> > 4K mode conveyed by HDMI VIC with particular aspect ratio. But in Appendix 
> > E of
> > HDMI 2.0 spec, it specify only 4k modes with particular aspect ratio should 
> > use VSIF to convey.
> > Hence, when the sink support HDMI 2.0 and set the mode to be VIC_103, 
> > calling
> > drm_match_hdmi_mode(mode) will return vendor_if_vic = 3 (VIC_93 and VIC_103 
> > are having
> > the same timing but different aspect ratio). Thereafter will set the  
> > frame->video_code to 0.
> > However, VIC_103 should use AVI VIC according to HDMI 2.0 spec (only VIC: 
> > 93, 94, 95 &
> > 98 should use VSIF).
> >
> > This patch try to revise that, when the sink support HDMI 2.0, 
> > drm_match_hdmi_mode()
> > should also take aspect ratio into consideration.
> > But for easy reading, I add another function "drm_match_hdmi_1_4_mode" to 
> > do so.
> 
> > Seems rather convoluted. I think we should just add the aspect ratios
> > to edid_4k_modes[]. Or is there some problem with that approach?
> 
> Thanks for your time.
> 
> Since HDMI 1.4b doesn't require edid_4k_modes[] with specific aspect ratio, 
> modes as the same
> timing in edid_4k_modes[] but with different aspect ratios are also expected 
> to convey VIC by
> VSIF to HDMI 1.4 sink. Might can't guarantee that there wont't be any 
> compatibility side effect with
> that approach when the sink is HDMI 1.4b .

I think adding them should be fine. But while checking the existing
code I noticed a few problems, so I sent out some fixes (cc:d you).

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

Re: [PATCH v3] drm/fourcc: Add Arm 16x16 block modifier

2019-10-04 Thread Ayan Halder
On Fri, Oct 04, 2019 at 02:12:38PM +, Ayan Halder wrote:
> From: Raymond Smith 
> 
> Add the DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED modifier to
> denote the 16x16 block u-interleaved format used in Arm Utgard and
> Midgard GPUs.
> 
> Changes from v1:-
> 1. Reserved the upper four bits (out of the 56 bits assigned to each vendor)
> to denote the category of Arm specific modifiers. Currently, we have two
> categories ie AFBC and MISC.
> 
> Changes from v2:-
> 1. Preserved Ray's authorship
> 2. Cleanups/changes suggested by Brian
> 3. Added r-bs of Brian and Qiang
> 
> Signed-off-by: Raymond Smith 
> Signed-off-by: Ayan kumar halder 
> Reviewed-by: Brian Starkey 
> Reviewed-by: Qiang Yu 

Pushed to drm-misc-next - ba2a1c8706151ac3234d2d020873feab498ab1bb
> ---
>  include/uapi/drm/drm_fourcc.h | 26 +-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 3feeaa3f987a..2376d36ea573 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -648,7 +648,21 @@ extern "C" {
>   * Further information on the use of AFBC modifiers can be found in
>   * Documentation/gpu/afbc.rst
>   */
> -#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) fourcc_mod_code(ARM, 
> __afbc_mode)
> +
> +/*
> + * The top 4 bits (out of the 56 bits alloted for specifying vendor specific
> + * modifiers) denote the category for modifiers. Currently we have only two
> + * categories of modifiers ie AFBC and MISC. We can have a maximum of sixteen
> + * different categories.
> + */
> +#define DRM_FORMAT_MOD_ARM_CODE(__type, __val) \
> + fourcc_mod_code(ARM, ((__u64)(__type) << 52) | ((__val) & 
> 0x000fULL))
> +
> +#define DRM_FORMAT_MOD_ARM_TYPE_AFBC 0x00
> +#define DRM_FORMAT_MOD_ARM_TYPE_MISC 0x01
> +
> +#define DRM_FORMAT_MOD_ARM_AFBC(__afbc_mode) \
> + DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_AFBC, __afbc_mode)
>  
>  /*
>   * AFBC superblock size
> @@ -742,6 +756,16 @@ extern "C" {
>   */
>  #define AFBC_FORMAT_MOD_BCH (1ULL << 11)
>  
> +/*
> + * Arm 16x16 Block U-Interleaved modifier
> + *
> + * This is used by Arm Mali Utgard and Midgard GPUs. It divides the image
> + * into 16x16 pixel blocks. Blocks are stored linearly in order, but pixels
> + * in the block are reordered.
> + */
> +#define DRM_FORMAT_MOD_ARM_16X16_BLOCK_U_INTERLEAVED \
> + DRM_FORMAT_MOD_ARM_CODE(DRM_FORMAT_MOD_ARM_TYPE_MISC, 1ULL)
> +
>  /*
>   * Allwinner tiled modifier
>   *
> -- 
> 2.23.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[RFC PATCH 3/3] drm/komeda: Allow non-component drm_bridge only endpoints

2019-10-04 Thread Mihail Atanassov
To support transmitters other than the tda998x, we need to allow
non-component framework bridges to be attached to a dummy drm_encoder in
our driver.

For the existing supported encoder (tda998x), keep the behaviour as-is,
since there's no way to unbind if a drm_bridge module goes away under
our feet.

Signed-off-by: Mihail Atanassov 
---
 .../gpu/drm/arm/display/komeda/komeda_dev.h   |   5 +
 .../gpu/drm/arm/display/komeda/komeda_drv.c   |  58 ++--
 .../gpu/drm/arm/display/komeda/komeda_kms.c   | 133 +-
 .../gpu/drm/arm/display/komeda/komeda_kms.h   |   5 +
 4 files changed, 187 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h 
b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
index e392b8ffc372..64d2902e2e0c 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
@@ -176,6 +176,11 @@ struct komeda_dev {
 
/** @irq: irq number */
int irq;
+   /** @has_components:
+*
+* if true, use the component framework to bind/unbind driver
+*/
+   bool has_components;
 
/** @lock: used to protect dpmode */
struct mutex lock;
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index 9ed25ffe0e22..34cfc6a4c3e4 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -10,6 +10,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "komeda_dev.h"
 #include "komeda_kms.h"
 
@@ -69,18 +71,35 @@ static int compare_of(struct device *dev, void *data)
return dev->of_node == data;
 }
 
-static void komeda_add_slave(struct device *master,
-struct component_match **match,
-struct device_node *np,
-u32 port, u32 endpoint)
+static int komeda_add_slave(struct device *master,
+   struct komeda_drv *mdrv,
+   struct component_match **match,
+   struct device_node *np,
+   u32 port, u32 endpoint)
 {
struct device_node *remote;
+   struct drm_bridge *bridge;
+   int ret = 0;
 
remote = of_graph_get_remote_node(np, port, endpoint);
-   if (remote) {
+   if (!remote)
+   return 0;
+
+   if (komeda_remote_device_is_component(np, port, endpoint)) {
+   mdrv->mdev.has_components = true;
drm_of_component_match_add(master, match, compare_of, remote);
-   of_node_put(remote);
+   goto exit;
+   }
+
+   bridge = of_drm_find_bridge(remote);
+   if (!bridge) {
+   ret = -EPROBE_DEFER;
+   goto exit;
}
+
+exit:
+   of_node_put(remote);
+   return ret;
 }
 
 static int komeda_platform_probe(struct platform_device *pdev)
@@ -89,6 +108,7 @@ static int komeda_platform_probe(struct platform_device 
*pdev)
struct component_match *match = NULL;
struct device_node *child;
struct komeda_drv *mdrv;
+   int ret;
 
if (!dev->of_node)
return -ENODEV;
@@ -101,14 +121,27 @@ static int komeda_platform_probe(struct platform_device 
*pdev)
if (of_node_cmp(child->name, "pipeline") != 0)
continue;
 
-   /* add connector */
-   komeda_add_slave(dev, &match, child, KOMEDA_OF_PORT_OUTPUT, 0);
-   komeda_add_slave(dev, &match, child, KOMEDA_OF_PORT_OUTPUT, 1);
+   /* attach any remote devices if present */
+   ret = komeda_add_slave(dev, mdrv, &match, child,
+  KOMEDA_OF_PORT_OUTPUT, 0);
+   if (ret)
+   goto free_mdrv;
+   ret = komeda_add_slave(dev, mdrv, &match, child,
+  KOMEDA_OF_PORT_OUTPUT, 1);
+   if (ret)
+   goto free_mdrv;
}
 
dev_set_drvdata(dev, mdrv);
 
-   return component_master_add_with_match(dev, &komeda_master_ops, match);
+   return mdrv->mdev.has_components
+   ? component_master_add_with_match(
+   dev, &komeda_master_ops, match)
+   : komeda_bind(dev);
+
+free_mdrv:
+   devm_kfree(dev, mdrv);
+   return ret;
 }
 
 static int komeda_platform_remove(struct platform_device *pdev)
@@ -116,7 +149,10 @@ static int komeda_platform_remove(struct platform_device 
*pdev)
struct device *dev = &pdev->dev;
struct komeda_drv *mdrv = dev_get_drvdata(dev);
 
-   component_master_del(dev, &komeda_master_ops);
+   if (mdrv->mdev.has_components)
+   component_master_del(dev, &komeda_master_ops);
+   else
+   komeda_unbind(dev);
 
dev_set_drvdata(dev, NULL);
devm_kfree(dev, mdrv);
dif

[PATCH 1/3] drm/komeda: Consolidate struct komeda_drv allocations

2019-10-04 Thread Mihail Atanassov
struct komeda_drv has two pointer members only, and both
it and its members get allocated separately. Since both
members' types are in scope, there's not a lot to gain by
keeping the indirection around.

To avoid double-free issues, provide a barebones ->release()
hook for the driver.

Signed-off-by: Mihail Atanassov 
---
 .../gpu/drm/arm/display/komeda/komeda_dev.c   | 21 ---
 .../gpu/drm/arm/display/komeda/komeda_dev.h   |  4 +--
 .../gpu/drm/arm/display/komeda/komeda_drv.c   | 36 +--
 .../gpu/drm/arm/display/komeda/komeda_kms.c   | 26 --
 .../gpu/drm/arm/display/komeda/komeda_kms.h   |  4 +--
 5 files changed, 42 insertions(+), 49 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
index 937a6d4c4865..c84f978cacc3 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
@@ -179,28 +179,23 @@ static int komeda_parse_dt(struct device *dev, struct 
komeda_dev *mdev)
return ret;
 }
 
-struct komeda_dev *komeda_dev_create(struct device *dev)
+int komeda_dev_init(struct komeda_dev *mdev, struct device *dev)
 {
struct platform_device *pdev = to_platform_device(dev);
const struct komeda_product_data *product;
-   struct komeda_dev *mdev;
struct resource *io_res;
int err = 0;
 
product = of_device_get_match_data(dev);
if (!product)
-   return ERR_PTR(-ENODEV);
+   return -ENODEV;
 
io_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!io_res) {
DRM_ERROR("No registers defined.\n");
-   return ERR_PTR(-ENODEV);
+   return -ENODEV;
}
 
-   mdev = devm_kzalloc(dev, sizeof(*mdev), GFP_KERNEL);
-   if (!mdev)
-   return ERR_PTR(-ENOMEM);
-
mutex_init(&mdev->lock);
 
mdev->dev = dev;
@@ -284,16 +279,16 @@ struct komeda_dev *komeda_dev_create(struct device *dev)
komeda_debugfs_init(mdev);
 #endif
 
-   return mdev;
+   return 0;
 
 disable_clk:
clk_disable_unprepare(mdev->aclk);
 err_cleanup:
-   komeda_dev_destroy(mdev);
-   return ERR_PTR(err);
+   komeda_dev_fini(mdev);
+   return err;
 }
 
-void komeda_dev_destroy(struct komeda_dev *mdev)
+void komeda_dev_fini(struct komeda_dev *mdev)
 {
struct device *dev = mdev->dev;
const struct komeda_dev_funcs *funcs = mdev->funcs;
@@ -335,8 +330,6 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
devm_clk_put(dev, mdev->aclk);
mdev->aclk = NULL;
}
-
-   devm_kfree(dev, mdev);
 }
 
 int komeda_dev_resume(struct komeda_dev *mdev)
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h 
b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
index 414200233b64..e392b8ffc372 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.h
@@ -213,8 +213,8 @@ komeda_product_match(struct komeda_dev *mdev, u32 target)
 const struct komeda_dev_funcs *
 d71_identify(u32 __iomem *reg, struct komeda_chip_info *chip);
 
-struct komeda_dev *komeda_dev_create(struct device *dev);
-void komeda_dev_destroy(struct komeda_dev *mdev);
+int komeda_dev_init(struct komeda_dev *mdev, struct device *dev);
+void komeda_dev_fini(struct komeda_dev *mdev);
 
 struct komeda_dev *dev_to_mdev(struct device *dev);
 
diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index d6cc5d33..660181bdc008 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -14,15 +14,15 @@
 #include "komeda_kms.h"
 
 struct komeda_drv {
-   struct komeda_dev *mdev;
-   struct komeda_kms_dev *kms;
+   struct komeda_dev mdev;
+   struct komeda_kms_dev kms;
 };
 
 struct komeda_dev *dev_to_mdev(struct device *dev)
 {
struct komeda_drv *mdrv = dev_get_drvdata(dev);
 
-   return mdrv ? mdrv->mdev : NULL;
+   return mdrv ? &mdrv->mdev : NULL;
 }
 
 static void komeda_unbind(struct device *dev)
@@ -32,8 +32,8 @@ static void komeda_unbind(struct device *dev)
if (!mdrv)
return;
 
-   komeda_kms_detach(mdrv->kms);
-   komeda_dev_destroy(mdrv->mdev);
+   komeda_kms_fini(mdrv->kms);
+   komeda_dev_fini(mdrv->mdev);
 
dev_set_drvdata(dev, NULL);
devm_kfree(dev, mdrv);
@@ -48,24 +48,20 @@ static int komeda_bind(struct device *dev)
if (!mdrv)
return -ENOMEM;
 
-   mdrv->mdev = komeda_dev_create(dev);
-   if (IS_ERR(mdrv->mdev)) {
-   err = PTR_ERR(mdrv->mdev);
+   err = komeda_dev_init(&mdrv->mdev, dev);
+   if (err)
goto free_mdrv;
-   }
 
-   mdrv->kms = komeda_kms_attach(mdrv->mdev);
-   if (IS_ERR(mdrv->kms)) {
-   err = PTR_ERR(mdrv->kms)

[PATCH 0/3] drm/komeda: Support for drm_bridge endpoints

2019-10-04 Thread Mihail Atanassov
Greetings,

This series attempts to add support for endpoints (in the DT sense)
whose drivers only have a drm_bridge interface, unlike the tda998x,
which uses the component framework and provides all of drm_encoder,
drm_bridge, drm_connector.

1 & 2 should be fairly non-contentious, and I believe are valuable
enough on their own as they remove some pointer chasing and a few
allocations at the top level of komeda.

3 is the meat of this series, where I add the drm_bridge endpoint code.
This was tested with ti_tfp410 on the back end of a D71. I've tagged it
with an RFC since I drew inspiration from tilcdc, which does similar
shenanigans to detect tda998x vs non-tda998x, and is hence a precedent
for including similar handling, but I wasn't sure if there's a more well
established pattern.

Note that I opted not to remove the previous way of doing things for
tda998x, even though it could work with the drm_bridge handling
directly, since AFAIUI, device links haven't been implemented for
drm_bridge (see [1] for an attempt at doing that that never landed, and
I'm not aware of any refcounting having been added since), and therefore
getting a drm_bridge driver rmmod'ed while in use would be Bad(tm).

[1] https://lore.kernel.org/lkml/20180426223139.16740-1-p...@axentia.se/

Cc: Liviu Dudau 
Cc: Brian Starkey 
Cc: James (Qian) Wang 
Cc: Daniel Vetter 
Cc: David Airlie 
Cc: Maxime Ripard 
Cc: Maarten Lankhorst 
Cc: Sean Paul 


Mihail Atanassov (3):
  drm/komeda: Consolidate struct komeda_drv allocations
  drm/komeda: Memory manage struct komeda_drv in probe/remove
  drm/komeda: Allow non-component drm_bridge only endpoints

 .../gpu/drm/arm/display/komeda/komeda_dev.c   |  21 +--
 .../gpu/drm/arm/display/komeda/komeda_dev.h   |   9 +-
 .../gpu/drm/arm/display/komeda/komeda_drv.c   | 118 -
 .../gpu/drm/arm/display/komeda/komeda_kms.c   | 159 --
 .../gpu/drm/arm/display/komeda/komeda_kms.h   |   9 +-
 5 files changed, 243 insertions(+), 73 deletions(-)

-- 
2.23.0

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

[PATCH 2/3] drm/komeda: Memory manage struct komeda_drv in probe/remove

2019-10-04 Thread Mihail Atanassov
Some fields of komeda_drv members will be useful very early
in probe code, so make sure an instance is available.

Signed-off-by: Mihail Atanassov 
---
 .../gpu/drm/arm/display/komeda/komeda_drv.c   | 30 +++
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c 
b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
index 660181bdc008..9ed25ffe0e22 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
@@ -32,22 +32,15 @@ static void komeda_unbind(struct device *dev)
if (!mdrv)
return;
 
-   komeda_kms_fini(mdrv->kms);
-   komeda_dev_fini(mdrv->mdev);
-
-   dev_set_drvdata(dev, NULL);
-   devm_kfree(dev, mdrv);
+   komeda_kms_fini(&mdrv->kms);
+   komeda_dev_fini(&mdrv->mdev);
 }
 
 static int komeda_bind(struct device *dev)
 {
-   struct komeda_drv *mdrv;
+   struct komeda_drv *mdrv = dev_get_drvdata(dev);
int err;
 
-   mdrv = devm_kzalloc(dev, sizeof(*mdrv), GFP_KERNEL);
-   if (!mdrv)
-   return -ENOMEM;
-
err = komeda_dev_init(&mdrv->mdev, dev);
if (err)
goto free_mdrv;
@@ -56,8 +49,6 @@ static int komeda_bind(struct device *dev)
if (err)
goto fini_mdev;
 
-   dev_set_drvdata(dev, mdrv);
-
return 0;
 
 fini_mdev:
@@ -97,10 +88,15 @@ static int komeda_platform_probe(struct platform_device 
*pdev)
struct device *dev = &pdev->dev;
struct component_match *match = NULL;
struct device_node *child;
+   struct komeda_drv *mdrv;
 
if (!dev->of_node)
return -ENODEV;
 
+   mdrv = devm_kzalloc(dev, sizeof(*mdrv), GFP_KERNEL);
+   if (!mdrv)
+   return -ENOMEM;
+
for_each_available_child_of_node(dev->of_node, child) {
if (of_node_cmp(child->name, "pipeline") != 0)
continue;
@@ -110,12 +106,20 @@ static int komeda_platform_probe(struct platform_device 
*pdev)
komeda_add_slave(dev, &match, child, KOMEDA_OF_PORT_OUTPUT, 1);
}
 
+   dev_set_drvdata(dev, mdrv);
+
return component_master_add_with_match(dev, &komeda_master_ops, match);
 }
 
 static int komeda_platform_remove(struct platform_device *pdev)
 {
-   component_master_del(&pdev->dev, &komeda_master_ops);
+   struct device *dev = &pdev->dev;
+   struct komeda_drv *mdrv = dev_get_drvdata(dev);
+
+   component_master_del(dev, &komeda_master_ops);
+
+   dev_set_drvdata(dev, NULL);
+   devm_kfree(dev, mdrv);
return 0;
 }
 
-- 
2.23.0

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

Re: [PATCH v7 4/5] dt-bindings: backlight: Add led-backlight binding

2019-10-04 Thread Lee Jones
On Wed, 18 Sep 2019, Jean-Jacques Hiblot wrote:

> Add DT binding for led-backlight.
> 
> Signed-off-by: Jean-Jacques Hiblot 
> Reviewed-by: Daniel Thompson 
> ---
>  .../bindings/leds/backlight/led-backlight.txt | 28 +++
>  1 file changed, 28 insertions(+)
>  create mode 100644 
> Documentation/devicetree/bindings/leds/backlight/led-backlight.txt

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v7 5/5] backlight: add led-backlight driver

2019-10-04 Thread Lee Jones
On Wed, 18 Sep 2019, Jean-Jacques Hiblot wrote:

> From: Tomi Valkeinen 
> 
> This patch adds a led-backlight driver (led_bl), which is similar to
> pwm_bl except the driver uses a LED class driver to adjust the
> brightness in the HW. Multiple LEDs can be used for a single backlight.
> 
> Signed-off-by: Tomi Valkeinen 
> Signed-off-by: Jean-Jacques Hiblot 
> Acked-by: Pavel Machek 
> Reviewed-by: Daniel Thompson 
> ---
>  drivers/video/backlight/Kconfig  |   7 +
>  drivers/video/backlight/Makefile |   1 +
>  drivers/video/backlight/led_bl.c | 260 +++
>  3 files changed, 268 insertions(+)
>  create mode 100644 drivers/video/backlight/led_bl.c

Applied, thanks.

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: Should regulator core support parsing OF based fwnode?

2019-10-04 Thread Mark Brown
On Fri, Oct 04, 2019 at 03:33:13PM +0200, Jean-Jacques Hiblot wrote:
> On 04/10/2019 13:39, Mark Brown wrote:

> > Consumers should just be able to request a regulator without having to
> > worry about how that's being provided - they should have no knowledge at
> > all of firmware bindings or platform data for defining this.  If they
> > do that suggests there's an abstraction issue somewhere, what makes you
> > think that doing something with of_node is required?

> The regulator core accesses consumer->of_node to get a phandle to a
> regulator's node. The trouble arises from the fact that the LED core does
> not populate of_node anymore, instead it populates fwnode. This allows the
> LED core to be agnostic of ACPI or OF to get the properties of a LED.

Why is the LED core populating anything?  Is the LED core copying bits
out of the struct device for the actual device into a synthetic device
rather than passing the actual device in?  That really doesn't seem like
a good idea, it's likely to lead to things like this where you don't
copy something that's required (or worse where something directly in the
struct device that can't be copied is needed).

> IMO it is better to populate both of_node and fwnode in the LED core at the
> moment. It has already been fixed this way for the platform driver [0], MTD
> [1] and PCI-OF [2].

Yeah, if you're going to be copying stuff out of the real device I'd
copy the of_node as well.

> > Further, unless you have LEDs that work without power you probably
> > shouldn't be using _get_optional() for their supply.  That interface is
> > intended only for supplies that may be physically absent.

> Not all LEDs have a regulator to provide the power. The power can be
> supplied by the LED controller for example.

This code probably shouldn't be being run at all for LEDs like that, I
was assuming this was just for GPIO LEDs and similar rather than all
LEDs.


signature.asc
Description: PGP signature


[PATCH] drm/panfrost: Remove NULL check for regulator

2019-10-04 Thread Steven Price
devm_regulator_get() is used to populate pfdev->regulator which ensures
that this cannot be NULL (a dummy regulator will be returned if
necessary). So remove the check in panfrost_devfreq_target().

Signed-off-by: Steven Price 
---
This looks like it was accidentally reintroduced by the merge from
drm-next into drm-misc-next due to the duplication of "drm/panfrost: Add
missing check for pfdev-regulator" (commits c90f30812a79 and
52282163dfa6).
---
 drivers/gpu/drm/panfrost/panfrost_devfreq.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c 
b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
index c1eb8cfe6aeb..12ff77dacc95 100644
--- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c
+++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c
@@ -53,10 +53,8 @@ static int panfrost_devfreq_target(struct device *dev, 
unsigned long *freq,
if (err) {
dev_err(dev, "Cannot set frequency %lu (%d)\n", target_rate,
err);
-   if (pfdev->regulator)
-   regulator_set_voltage(pfdev->regulator,
- pfdev->devfreq.cur_volt,
- pfdev->devfreq.cur_volt);
+   regulator_set_voltage(pfdev->regulator, pfdev->devfreq.cur_volt,
+ pfdev->devfreq.cur_volt);
return err;
}
 
-- 
2.20.1



[PATCH TRIVIAL v2] gpu: Fix Kconfig indentation

2019-10-04 Thread Krzysztof Kozlowski
Adjust indentation from spaces to tab (+optional two spaces) as in
coding style with command like:
$ sed -e 's/^/\t/' -i */Kconfig

Signed-off-by: Krzysztof Kozlowski 

---

Changes since v1:
1. Fix also DRM_AMD_DC_HDCP (new arrival since v1).
---
 drivers/gpu/drm/Kconfig  |  10 +-
 drivers/gpu/drm/amd/display/Kconfig  |  32 ++---
 drivers/gpu/drm/bridge/Kconfig   |   8 +-
 drivers/gpu/drm/i915/Kconfig |  12 +-
 drivers/gpu/drm/i915/Kconfig.debug   | 144 +++
 drivers/gpu/drm/lima/Kconfig |   2 +-
 drivers/gpu/drm/mgag200/Kconfig  |   8 +-
 drivers/gpu/drm/nouveau/Kconfig  |   2 +-
 drivers/gpu/drm/omapdrm/displays/Kconfig |   6 +-
 drivers/gpu/drm/omapdrm/dss/Kconfig  |  12 +-
 drivers/gpu/drm/rockchip/Kconfig |   8 +-
 drivers/gpu/drm/udl/Kconfig  |   2 +-
 drivers/gpu/vga/Kconfig  |   2 +-
 13 files changed, 124 insertions(+), 124 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index e67c194c2aca..7cb6e4eb99e8 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -207,8 +207,8 @@ config DRM_RADEON
tristate "ATI Radeon"
depends on DRM && PCI && MMU
select FW_LOADER
-select DRM_KMS_HELPER
-select DRM_TTM
+   select DRM_KMS_HELPER
+   select DRM_TTM
select POWER_SUPPLY
select HWMON
select BACKLIGHT_CLASS_DEVICE
@@ -226,9 +226,9 @@ config DRM_AMDGPU
tristate "AMD GPU"
depends on DRM && PCI && MMU
select FW_LOADER
-select DRM_KMS_HELPER
+   select DRM_KMS_HELPER
select DRM_SCHED
-select DRM_TTM
+   select DRM_TTM
select POWER_SUPPLY
select HWMON
select BACKLIGHT_CLASS_DEVICE
@@ -266,7 +266,7 @@ config DRM_VKMS
  If M is selected the module will be called vkms.
 
 config DRM_ATI_PCIGART
-bool
+   bool
 
 source "drivers/gpu/drm/exynos/Kconfig"
 
diff --git a/drivers/gpu/drm/amd/display/Kconfig 
b/drivers/gpu/drm/amd/display/Kconfig
index 1bbe762ee6ba..313183b80032 100644
--- a/drivers/gpu/drm/amd/display/Kconfig
+++ b/drivers/gpu/drm/amd/display/Kconfig
@@ -23,16 +23,16 @@ config DRM_AMD_DC_DCN2_0
depends on DRM_AMD_DC && X86
depends on DRM_AMD_DC_DCN1_0
help
-   Choose this option if you want to have
-   Navi support for display engine
+ Choose this option if you want to have
+ Navi support for display engine
 
 config DRM_AMD_DC_DCN2_1
-bool "DCN 2.1 family"
-depends on DRM_AMD_DC && X86
-depends on DRM_AMD_DC_DCN2_0
-help
-Choose this option if you want to have
-Renoir support for display engine
+   bool "DCN 2.1 family"
+   depends on DRM_AMD_DC && X86
+   depends on DRM_AMD_DC_DCN2_0
+   help
+ Choose this option if you want to have
+ Renoir support for display engine
 
 config DRM_AMD_DC_DSC_SUPPORT
bool "DSC support"
@@ -41,16 +41,16 @@ config DRM_AMD_DC_DSC_SUPPORT
depends on DRM_AMD_DC_DCN1_0
depends on DRM_AMD_DC_DCN2_0
help
-   Choose this option if you want to have
-   Dynamic Stream Compression support
+ Choose this option if you want to have
+ Dynamic Stream Compression support
 
 config DRM_AMD_DC_HDCP
-bool "Enable HDCP support in DC"
-depends on DRM_AMD_DC
-help
- Choose this option
- if you want to support
- HDCP authentication
+   bool "Enable HDCP support in DC"
+   depends on DRM_AMD_DC
+   help
+Choose this option
+if you want to support
+HDCP authentication
 
 config DEBUG_KERNEL_DC
bool "Enable kgdb break in DC"
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 1cc9f502c1f2..a5aa7ec16000 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -60,10 +60,10 @@ config DRM_MEGACHIPS_STDP_GE_B850V3_FW
select DRM_KMS_HELPER
select DRM_PANEL
---help---
-  This is a driver for the display bridges of
-  GE B850v3 that convert dual channel LVDS
-  to DP++. This is used with the i.MX6 imx-ldb
-  driver. You are likely to say N here.
+ This is a driver for the display bridges of
+ GE B850v3 that convert dual channel LVDS
+ to DP++. This is used with the i.MX6 imx-ldb
+ driver. You are likely to say N here.
 
 config DRM_NXP_PTN3460
tristate "NXP PTN3460 DP/LVDS bridge"
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 0d21402945ab..3c6d57df262d 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -76,7 +76,7 @@ config DRM_I915_CAPTURE_ERROR
  This option enables capturing the GPU state when a hang is detected.
  This inf

Re: drm_sched with panfrost crash on T820

2019-10-04 Thread Grodzovsky, Andrey

On 10/3/19 4:34 AM, Neil Armstrong wrote:
> Hi Andrey,
>
> Le 02/10/2019 à 16:40, Grodzovsky, Andrey a écrit :
>> On 9/30/19 10:52 AM, Hillf Danton wrote:
>>> On Mon, 30 Sep 2019 11:17:45 +0200 Neil Armstrong wrote:
 Did a new run from 5.3:

 [   35.971972] Call trace:
 [   35.974391]  drm_sched_increase_karma+0x5c/0xf0
10667f3810667F94
drivers/gpu/drm/scheduler/sched_main.c:335

 The crashing line is :
   if (bad->s_fence->scheduled.context ==
   entity->fence_context) {

 Doesn't seem related to guilty job.
>>> Bail out if s_fence is no longer fresh.
>>>
>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>> @@ -333,6 +333,10 @@ void drm_sched_increase_karma(struct drm
>>>
>>> spin_lock(&rq->lock);
>>> list_for_each_entry_safe(entity, tmp, &rq->entities, 
>>> list) {
>>> +   if (!smp_load_acquire(&bad->s_fence)) {
>>> +   spin_unlock(&rq->lock);
>>> +   return;
>>> +   }
>>> if (bad->s_fence->scheduled.context ==
>>> entity->fence_context) {
>>> if (atomic_read(&bad->karma) >
>>> @@ -543,7 +547,7 @@ EXPORT_SYMBOL(drm_sched_job_init);
>>>void drm_sched_job_cleanup(struct drm_sched_job *job)
>>>{
>>> dma_fence_put(&job->s_fence->finished);
>>> -   job->s_fence = NULL;
>>> +   smp_store_release(&job->s_fence, 0);
>>>}
>>>EXPORT_SYMBOL(drm_sched_job_cleanup);
> This fixed the problem on the 10 CI runs.
>
> Neil


These are good news but I still fail to see how this fixes the problem - 
Hillf, do you mind explaining how you came up with this particular fix - 
what was the bug you saw ?

Andrey

>
>> Does this change help the problem ? Note that drm_sched_job_cleanup is
>> called from scheduler thread which is stopped at all times when work_tdr
>> thread is running and anyway the 'bad' job is still in the
>> ring_mirror_list while it's being accessed from
>> drm_sched_increase_karma so I don't think drm_sched_job_cleanup can be
>> called for it BEFORE or while drm_sched_increase_karma is executed.
>>
>> Andrey
>>
>>
>>>
>>> --
>>>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

  1   2   >