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 :-) 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
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
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
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
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
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
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
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
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
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
> 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
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
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
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
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
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
ś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
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
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
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
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
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
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
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)
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
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
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
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()
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
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
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)
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?
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
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
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)
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
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
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
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
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
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
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()
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()
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
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
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()
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
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
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
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+
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
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
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())
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
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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()
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
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
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
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?
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
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
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
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
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
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
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
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
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
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()
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)
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
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
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
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
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
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
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
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
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
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
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?
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
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
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
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