[PATCH] drm/omap: fix crash if there's no video PLL
Commit 8a7eda7686675b73d74c22c0d5b83059f9d783f6 ("drm: omapdrm: dispc:
Pass DISPC pointer to remaining dispc API functions") made dpi.c use
ctx->pll even when there's no PLL, causing a crash at modeset on AM4
EVM, and presumably all OMAP2/3 boards.
Fix this by having struct dpi_data pointer in the ctx instead, giving
access to dispc without going through the pll.
Signed-off-by: Tomi Valkeinen
Reported-by: Keerthy
Cc: Laurent Pinchart
---
drivers/gpu/drm/omapdrm/dss/dpi.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c
b/drivers/gpu/drm/omapdrm/dss/dpi.c
index fb1c27f69e3a..3d662e6805eb 100644
--- a/drivers/gpu/drm/omapdrm/dss/dpi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
@@ -142,7 +142,7 @@ static enum dss_clk_source dpi_get_clk_src(struct dpi_data
*dpi)
}
struct dpi_clk_calc_ctx {
- struct dss_pll *pll;
+ struct dpi_data *dpi;
unsigned int clkout_idx;
/* inputs */
@@ -191,7 +191,7 @@ static bool dpi_calc_hsdiv_cb(int m_dispc, unsigned long
dispc,
ctx->pll_cinfo.mX[ctx->clkout_idx] = m_dispc;
ctx->pll_cinfo.clkout[ctx->clkout_idx] = dispc;
- return dispc_div_calc(ctx->pll->dss->dispc, dispc,
+ return dispc_div_calc(ctx->dpi->dss->dispc, dispc,
ctx->pck_min, ctx->pck_max,
dpi_calc_dispc_cb, ctx);
}
@@ -208,8 +208,8 @@ static bool dpi_calc_pll_cb(int n, int m, unsigned long
fint,
ctx->pll_cinfo.fint = fint;
ctx->pll_cinfo.clkdco = clkdco;
- return dss_pll_hsdiv_calc_a(ctx->pll, clkdco,
- ctx->pck_min, dss_get_max_fck_rate(ctx->pll->dss),
+ return dss_pll_hsdiv_calc_a(ctx->dpi->pll, clkdco,
+ ctx->pck_min, dss_get_max_fck_rate(ctx->dpi->dss),
dpi_calc_hsdiv_cb, ctx);
}
@@ -219,7 +219,7 @@ static bool dpi_calc_dss_cb(unsigned long fck, void *data)
ctx->fck = fck;
- return dispc_div_calc(ctx->pll->dss->dispc, fck,
+ return dispc_div_calc(ctx->dpi->dss->dispc, fck,
ctx->pck_min, ctx->pck_max,
dpi_calc_dispc_cb, ctx);
}
@@ -230,7 +230,7 @@ static bool dpi_pll_clk_calc(struct dpi_data *dpi, unsigned
long pck,
unsigned long clkin;
memset(ctx, 0, sizeof(*ctx));
- ctx->pll = dpi->pll;
+ ctx->dpi = dpi;
ctx->clkout_idx = dss_pll_get_clkout_idx_for_src(dpi->clk_src);
clkin = clk_get_rate(dpi->pll->clkin);
@@ -244,7 +244,7 @@ static bool dpi_pll_clk_calc(struct dpi_data *dpi, unsigned
long pck,
pll_min = 0;
pll_max = 0;
- return dss_pll_calc_a(ctx->pll, clkin,
+ return dss_pll_calc_a(ctx->dpi->pll, clkin,
pll_min, pll_max,
dpi_calc_pll_cb, ctx);
} else { /* DSS_PLL_TYPE_B */
@@ -275,6 +275,7 @@ static bool dpi_dss_clk_calc(struct dpi_data *dpi, unsigned
long pck,
bool ok;
memset(ctx, 0, sizeof(*ctx));
+ ctx->dpi = dpi;
if (pck > 1000 * i * i * i)
ctx->pck_min = max(pck - 1000 * i * i * i, 0lu);
else
--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: ww-mutex performance tests?
Am 04.04.2018 um 20:02 schrieb Daniel Vetter: On Tue, Apr 3, 2018 at 10:37 AM, Maarten Lankhorst wrote: Op 02-04-18 om 19:49 schreef Thomas Hellstrom: Maarten, Daniel, Do we have any ww-mutex performance tests somewhere that can be used to test the impact of implementation details on various locking scenarios? Thanks, /Thomas The thing that comes to my mind are some of the kms_cursor_legacy tests that have been proven to be sensitive to locking issues before. All subtests with (pipe-*/all-pipes)-(single/forked/torture)-(bo/move) try to do cursor updates as fast as possible, and report how many updates have been done. AMD folks have a bunch of tests to exercise their CS paths I think, that should be interesting for the multi-ww_mutex/backoff paths, adding Christian et al. Well not a dedicated test for this, but at least I usually run glmark with thread offloading disabled to measure the command submission overhead. Except for the usual stuff like copying things from userspace the last time I looked our command submission overhead was dominated by work done for individual BOs. Not sure how much of that accounts for locking the ww_mutex of the BOs, but I would guess it is quite a bit. Christian. -Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM_UDL and GPU under Xserver
On Wed, Apr 4, 2018 at 10:06 PM, Alexey Brodkin wrote: > Hello, > > We're trying to use DisplayLink USB2-to-HDMI adapter to render > GPU-accelerated graphics. > Hardware setup is as simple as a devboard + DisplayLink adapter. > Devboards we use for this experiment are: > * Wandboard Quad (based on IMX6 SoC with Vivante GPU) or > * HSDK (based on Synopsys ARC HS38 SoC with Vivante GPU as well) > > I'm sure any other board with DRM supported GPU will work, those we just used > as the very recent Linux kernels could be easily run on them both. > > Basically the problem is UDL needs to be explicitly notified about new data > to be rendered on the screen compared to typical bit-streamers that infinitely > scan a dedicated buffer in memory. > > In case of UDL there're just 2 ways for this notification: > 1) DRM_IOCTL_MODE_PAGE_FLIP that calls drm_crtc_funcs->page_flip() > 2) DRM_IOCTL_MODE_DIRTYFB that calls drm_framebuffer_funcs->dirty() > > But neither of IOCTLs happen when we run Xserver with xf86-video-armada driver > (see > http://git.arm.linux.org.uk/cgit/xf86-video-armada.git/log/?h=unstable-devel). > > Is it something missing in Xserver or in UDL driver? Use the -modesetting driverr for UDL, that one works correctly. Kernel-driver specific X drivers are kinda deprecated, and stuff like this (and other bugfixes and improvements that don't propagate around) are the reason for that. -Daniel > > Regards, > Alexey > > > > > -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/sched: Extend the documentation.
On Thu, Apr 5, 2018 at 12:32 AM, Eric Anholt wrote:
> These comments answer all the questions I had for myself when
> implementing a driver using the GPU scheduler.
>
> Signed-off-by: Eric Anholt
Pulling all these comments into the generated kerneldoc would be
awesome, maybe as a new "GPU Scheduler" chapter at the end of
drm-mm.rst? Would mean a bit of busywork to convert the existing raw
comments into proper kerneldoc. Also has the benefit that 0day will
complain when you forget to update the comment when editing the
function prototype - kerneldoc which isn't included anywhere in .rst
won't be checked automatically.
-Daniel
> ---
> include/drm/gpu_scheduler.h | 46 +
> 1 file changed, 42 insertions(+), 4 deletions(-)
>
> diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
> index dfd54fb94e10..c053a32341bf 100644
> --- a/include/drm/gpu_scheduler.h
> +++ b/include/drm/gpu_scheduler.h
> @@ -43,10 +43,12 @@ enum drm_sched_priority {
> };
>
> /**
> - * A scheduler entity is a wrapper around a job queue or a group
> - * of other entities. Entities take turns emitting jobs from their
> - * job queues to corresponding hardware ring based on scheduling
> - * policy.
> + * drm_sched_entity - A wrapper around a job queue (typically attached
> + * to the DRM file_priv).
> + *
> + * Entities will emit jobs in order to their corresponding hardware
> + * ring, and the scheduler will alternate between entities based on
> + * scheduling policy.
> */
> struct drm_sched_entity {
> struct list_headlist;
> @@ -78,7 +80,18 @@ struct drm_sched_rq {
>
> struct drm_sched_fence {
> struct dma_fencescheduled;
> +
> + /* This fence is what will be signaled by the scheduler when
> +* the job is completed.
> +*
> +* When setting up an out fence for the job, you should use
> +* this, since it's available immediately upon
> +* drm_sched_job_init(), and the fence returned by the driver
> +* from run_job() won't be created until the dependencies have
> +* resolved.
> +*/
> struct dma_fencefinished;
> +
> struct dma_fence_cb cb;
> struct dma_fence*parent;
> struct drm_gpu_scheduler*sched;
> @@ -88,6 +101,13 @@ struct drm_sched_fence {
>
> struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
>
> +/**
> + * drm_sched_job - A job to be run by an entity.
> + *
> + * A job is created by the driver using drm_sched_job_init(), and
> + * should call drm_sched_entity_push_job() once it wants the scheduler
> + * to schedule the job.
> + */
> struct drm_sched_job {
> struct spsc_nodequeue_node;
> struct drm_gpu_scheduler*sched;
> @@ -112,10 +132,28 @@ static inline bool drm_sched_invalidate_job(struct
> drm_sched_job *s_job,
> * these functions should be implemented in driver side
> */
> struct drm_sched_backend_ops {
> + /* Called when the scheduler is considering scheduling this
> +* job next, to get another struct dma_fence for this job to
> +* block on. Once it returns NULL, run_job() may be called.
> +*/
> struct dma_fence *(*dependency)(struct drm_sched_job *sched_job,
> struct drm_sched_entity *s_entity);
> +
> + /* Called to execute the job once all of the dependencies have
> +* been resolved. This may be called multiple times, if
> +* timedout_job() has happened and drm_sched_job_recovery()
> +* decides to try it again.
> +*/
> struct dma_fence *(*run_job)(struct drm_sched_job *sched_job);
> +
> + /* Called when a job has taken too long to execute, to trigger
> +* GPU recovery.
> +*/
> void (*timedout_job)(struct drm_sched_job *sched_job);
> +
> + /* Called once the job's finished fence has been signaled and
> +* it's time to clean it up.
> +*/
> void (*free_job)(struct drm_sched_job *sched_job);
> };
>
> --
> 2.17.0
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105680] [CI] igt@kms_frontbuffer_tracking@* - fail - FBC disabled: mode too large for compression
https://bugs.freedesktop.org/show_bug.cgi?id=105680 --- Comment #7 from Marta Löfstedt --- (In reply to Jose Roberto de Souza from comment #6) > Done: https://patchwork.freedesktop.org/series/41156/ Thanks for the patch, we'll need a drmtip run to see if it works, since the affected machine doesn't run those test on regular basis in BAT. -- 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 93216] Artifacts with DCC when render target also texture
https://bugs.freedesktop.org/show_bug.cgi?id=93216 Axel Davy changed: What|Removed |Added Resolution|--- |FIXED Status|NEEDINFO|RESOLVED --- Comment #8 from Axel Davy --- The bug seems indeed to have been corrected since. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Freedreno] [PATCH 5/5] drm/msm/A6xx: Add support for using system cache(llc)
On 4/4/2018 2:54 AM, Jordan Crouse wrote:
On Fri, Mar 23, 2018 at 12:49:51PM +0530, Sharat Masetty wrote:
The last level system cache can be partitioned to 32
different slices of which GPU has two slices preallocated.
The "gpu" slice is used for caching GPU buffers and
the "gpuhtw" slice is used for caching the GPU SMMU
pagetables. This patch talks to the core system cache
driver to acquire the slice handles, configure the SCID's
to those slices and activates and deactivates the slices
upon GPU power collapse and restore.
Some support from the IOMMU driver is also needed to
make use of the system cache. IOMMU_UPSTREAM_HINT is
a buffer protection flag which enables caching GPU data
buffers in the system cache with memory attributes such
as outer cacheable, read-allocate, write-allocate for buffers.
The GPU then has the ability to override a few cacheability
parameters which it does to override write-allocate to
write-no-allocate as the GPU hardware does not benefit much
from it.
Similarly DOMAIN_ATTR_USE_UPSTREAM_HINT is another domain level
attribute used by the IOMMU driver to set the right attributes
to cache the hardware pagetables into the system cache.
This has a dependency on the LLCC driver and the API to that may change (it is
under review now). When it does, this will have to naturally change as well but
that'll be a minor tweek and won't affect the functionality of this driver so
pending those changes..
Thanks for the review Jordan. Vivek will also submit the SMMU changes
for the UPSTREAM_HINT support to the mailing list soon. So once the
dependencies are sorted out, I will review and submit a fresh patch set
if needed.
Reviewed-by: Jordan Crouse >
Signed-off-by: Sharat Masetty
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 162 +-
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 9 ++
drivers/gpu/drm/msm/msm_iommu.c | 13 +++
drivers/gpu/drm/msm/msm_mmu.h | 3 +
4 files changed, 186 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index bd50674..e4554eb 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -13,6 +13,7 @@
#include
#include
+#include
#include "msm_gem.h"
#include "msm_mmu.h"
@@ -913,6 +914,154 @@ static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
~0
};
+#define A6XX_LLC_NUM_GPU_SCIDS 5
+#define A6XX_GPU_LLC_SCID_NUM_BITS 5
+
+#define A6XX_GPU_LLC_SCID_MASK \
+ ((1 << (A6XX_LLC_NUM_GPU_SCIDS * A6XX_GPU_LLC_SCID_NUM_BITS)) - 1)
+
+#define A6XX_GPUHTW_LLC_SCID_SHIFT 25
+#define A6XX_GPUHTW_LLC_SCID_MASK \
+ (((1 << A6XX_GPU_LLC_SCID_NUM_BITS) - 1) << A6XX_GPUHTW_LLC_SCID_SHIFT)
+
+static inline void a6xx_gpu_cx_rmw(struct a6xx_llc *llc,
+ u32 reg, u32 mask, u32 or)
+{
+ msm_rmw(llc->mmio + (reg << 2), mask, or);
+}
+
+static void a6xx_llc_deactivate(struct msm_gpu *gpu)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+ struct a6xx_llc *llc = &a6xx_gpu->llc;
+
+ llcc_slice_deactivate(llc->gpu_llc_slice);
+ llcc_slice_deactivate(llc->gpuhtw_llc_slice);
+}
+
+static void a6xx_llc_activate(struct msm_gpu *gpu)
+{
+ struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
+ struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
+ struct a6xx_llc *llc = &a6xx_gpu->llc;
+
+ if (!llc->mmio)
+ return;
+
+ if (llc->gpu_llc_slice)
+ if (!llcc_slice_activate(llc->gpu_llc_slice))
+ /* Program the sub-cache ID for all GPU blocks */
+ a6xx_gpu_cx_rmw(llc,
+ REG_A6XX_GPU_CX_MISC_SYSTEM_CACHE_CNTL_1,
+ A6XX_GPU_LLC_SCID_MASK,
+ (llc->cntl1_regval &
+ A6XX_GPU_LLC_SCID_MASK));
+
+ if (llc->gpuhtw_llc_slice)
+ if (!llcc_slice_activate(llc->gpuhtw_llc_slice))
+ /* Program the sub-cache ID for GPU pagetables */
+ a6xx_gpu_cx_rmw(llc,
+ REG_A6XX_GPU_CX_MISC_SYSTEM_CACHE_CNTL_1,
+ A6XX_GPUHTW_LLC_SCID_MASK,
+ (llc->cntl1_regval &
+ A6XX_GPUHTW_LLC_SCID_MASK));
+
+ /* Program cacheability overrides */
+ a6xx_gpu_cx_rmw(llc, REG_A6XX_GPU_CX_MISC_SYSTEM_CACHE_CNTL_0, 0xF,
+ llc->cntl0_regval);
+}
+
+void a6xx_llc_slices_destroy(struct a6xx_llc *llc)
+{
+ if (llc->mmio) {
+ iounmap(llc->mmio);
+ llc->mmio = NULL;
+ }
+
+ llcc_slice_putd(llc->gpu_llc_slice);
+ llc->gpu_llc_slice = NULL;
+
+ llcc_slice_putd(llc->gpuhtw_llc_slice);
+ llc->gpuhtw_llc_slice = NULL;
+}
+
+static int a6xx
Re: [Freedreno] [PATCH 2/5] arm64:dts:sdm845: Add support for GPU LLCC
On 4/4/2018 2:52 AM, Jordan Crouse wrote: On Fri, Mar 23, 2018 at 12:49:48PM +0530, Sharat Masetty wrote: Add client side bindings required for the GPU to use the last level system cache. Also add a register range in the GPU CX domain. Reviewed-by: Jordan Crouse Also, these should go the the devicetree lists for review (but maybe wait until the other changes have gotten further through the process). Thanks Jordan for the review and the reminder, I will send this specific patch out to the devicetree mailing list for review. Signed-off-by: Sharat Masetty --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index eb0a1b2..7e2d938 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -887,8 +887,8 @@ compatible = "qcom,adreno-630.2", "qcom,adreno"; #stream-id-cells = <16>; - reg = <0x500 0x4>; - reg-names = "kgsl_3d0_reg_memory"; + reg = <0x500 0x4>, <0x509e000 0x10>; + reg-names = "kgsl_3d0_reg_memory", "cx_mem"; /* * Look ma, no clocks! The GPU clocks and power are controlled @@ -898,6 +898,10 @@ interrupts = <0 300 0>; interrupt-names = "kgsl_3d0_irq"; + /* GPU related llc slices */ + cache-slice-names = "gpu", "gpuhtw"; + cache-slices = <&llcc 12>, <&llcc 11>; + iommus = <&kgsl_smmu 0>; operating-points-v2 = <&gpu_opp_table>; -- 1.9.1 ___ Freedreno mailing list freedr...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/freedreno -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105883] booting with kernel using amd-staging-drm-next on 2400G hangs
https://bugs.freedesktop.org/show_bug.cgi?id=105883 --- Comment #4 from Edward Kigwana --- Try options amdgpu dpm=0 dc=1 and seee if it still locks up. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH v2 0/2] Remove DPU RSC support
On 2018-04-04 17:49, Daniel Vetter wrote: On Wed, Apr 04, 2018 at 04:53:51PM +0530, rya...@codeaurora.org wrote: On 2018-04-04 15:56, Daniel Vetter wrote: > On Wed, Apr 04, 2018 at 02:34:40PM +0530, Rajesh Yadav wrote: > > MSM display controller hardware (DPU) has an inbuilt RSC block > > which can control power resources and bus bandwidth voting > > based on frame timing parameters w/o DPU driver intervention. > > In absence of RSC HW, DPU driver controls these resources. > > > > Downstream driver relies on RSC driver for controlling these > > resources (via RSC HW block) for better power benefits. > > > > Since, DPU driver can control these resources, removing RSC > > driver support. Corresponding devicetree binding are also removed. > > If it has benefits, why remove the support for this? > -Daniel Currently, the dpu driver has custom implementation for power management. We are planning to move to runtime_pm and this change is 1st step toward that goal. We will re-introduce the RSC support at a later stage when all the dependencies are sent upstream. That kind of context would be really good to explain in the commit message and cover letter. -Daniel Noted. Will take care of it going forward. Thanks, Rajesh Thanks, Rajesh > > > > > Details for DPU driver upstreaming: > > https://lists.freedesktop.org/archives/freedreno/2018-February/001678.html > > > > Changes in v2: > > - Remove last reference to dpu_power_rsc_update > > - Add DPU PATCH tag for better filtering > > - Rebase on tip of for-next-staging > > > > Rajesh Yadav (2): > > dt-bindings: msm/disp: Remove DPU RSC device bindings > > drm/msm: Remove RSC support from DPU driver > > > > .../devicetree/bindings/display/msm/dpu-rsc.txt| 96 -- > > drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 130 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |6 - > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 14 - > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |9 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 242 +--- > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h|7 - > > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h |1 - > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 20 +- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h |3 - > > drivers/gpu/drm/msm/dpu_dbg.c | 27 - > > drivers/gpu/drm/msm/dpu_dbg.h | 10 - > > drivers/gpu/drm/msm/dpu_power_handle.c | 73 +- > > drivers/gpu/drm/msm/dpu_power_handle.h |4 - > > drivers/gpu/drm/msm/dpu_rsc.c | 1367 > > > > drivers/gpu/drm/msm/dpu_rsc_hw.c | 818 > > > > drivers/gpu/drm/msm/dpu_rsc_priv.h | 191 --- > > include/linux/dpu_rsc.h| 302 - > > 18 files changed, 42 insertions(+), 3278 deletions(-) > > delete mode 100644 > > Documentation/devicetree/bindings/display/msm/dpu-rsc.txt > > delete mode 100644 drivers/gpu/drm/msm/dpu_rsc.c > > delete mode 100644 drivers/gpu/drm/msm/dpu_rsc_hw.c > > delete mode 100644 drivers/gpu/drm/msm/dpu_rsc_priv.h > > delete mode 100644 include/linux/dpu_rsc.h > > > > -- > > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora > > Forum, > > a Linux Foundation Collaborative Project > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH v2 0/2] Remove DPU RSC support
On 2018-04-05 01:51, Sean Paul wrote: On Wed, Apr 04, 2018 at 02:34:40PM +0530, Rajesh Yadav wrote: MSM display controller hardware (DPU) has an inbuilt RSC block which can control power resources and bus bandwidth voting based on frame timing parameters w/o DPU driver intervention. In absence of RSC HW, DPU driver controls these resources. Downstream driver relies on RSC driver for controlling these resources (via RSC HW block) for better power benefits. Since, DPU driver can control these resources, removing RSC driver support. Corresponding devicetree binding are also removed. Details for DPU driver upstreaming: https://lists.freedesktop.org/archives/freedreno/2018-February/001678.html Changes in v2: - Remove last reference to dpu_power_rsc_update - Add DPU PATCH tag for better filtering - Rebase on tip of for-next-staging Hi Rajesh, Unrelated to this change, but I've noticed the threading seems off on the patch sets you're sending. Are you sending the emails one-by-one, or specifying --no-thread in git send-email? Sean Hi Sean, I had sent the patches one by one, I'll take care of this going forward. Thanks, Rajesh Rajesh Yadav (2): dt-bindings: msm/disp: Remove DPU RSC device bindings drm/msm: Remove RSC support from DPU driver .../devicetree/bindings/display/msm/dpu-rsc.txt| 96 -- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 130 +- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |6 - drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 14 - drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |9 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 242 +--- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h|7 - drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h |1 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 20 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h |3 - drivers/gpu/drm/msm/dpu_dbg.c | 27 - drivers/gpu/drm/msm/dpu_dbg.h | 10 - drivers/gpu/drm/msm/dpu_power_handle.c | 73 +- drivers/gpu/drm/msm/dpu_power_handle.h |4 - drivers/gpu/drm/msm/dpu_rsc.c | 1367 drivers/gpu/drm/msm/dpu_rsc_hw.c | 818 drivers/gpu/drm/msm/dpu_rsc_priv.h | 191 --- include/linux/dpu_rsc.h| 302 - 18 files changed, 42 insertions(+), 3278 deletions(-) delete mode 100644 Documentation/devicetree/bindings/display/msm/dpu-rsc.txt delete mode 100644 drivers/gpu/drm/msm/dpu_rsc.c delete mode 100644 drivers/gpu/drm/msm/dpu_rsc_hw.c delete mode 100644 drivers/gpu/drm/msm/dpu_rsc_priv.h delete mode 100644 include/linux/dpu_rsc.h -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105869] clang crashes when compiling OpenCL kernel
https://bugs.freedesktop.org/show_bug.cgi?id=105869 --- Comment #2 from Pierre-Loup A. Griffais --- [18:11:28] can someone reply here https://bugs.freedesktop.org/show_bug.cgi?id=105869 that version is 5.0.1-4 from Debian Testing, if I ever find another bug I'll register myself -- 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 99549] pp: Failed to translate a shader
https://bugs.freedesktop.org/show_bug.cgi?id=99549 Timothy Arceri changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #4 from Timothy Arceri --- Fixed by: author Marek Olšák commit 8cd58df2f2592bf5a19ef7431eccaca6b8a445cc gallium/pp: fix MLAA shaders Reviewed-by: Ilia Mirkin Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99549 -- 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 105026] glxgears asserts with pp_jimenezmlaa=1
https://bugs.freedesktop.org/show_bug.cgi?id=105026 Timothy Arceri changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #1 from Timothy Arceri --- Fixed by: author Marek Olšák gallium/pp: use user constant buffers This fixes a radeonsi crash. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105026 -- 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 104034] Mesa/Radeonsi does not compile on musl due to endianess check
https://bugs.freedesktop.org/show_bug.cgi?id=104034 Timothy Arceri changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #2 from Timothy Arceri --- Should be fixed by: commit cbee1bfb34274668a05995b9d4c78ddec9e5ea4c Author: Eric Engestrom Date: Wed Mar 21 17:04:06 2018 + meson/configure: detect endian.h instead of trying to guess when it's available Cc: Maxin B. John Cc: Khem Raj Cc: Rob Herring Suggested-by: Jon Turney Signed-off-by: Eric Engestrom Reviewed-by: Emil Velikov Reviewed-by: Dylan Baker Cc: -- 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 93216] Artifacts with DCC when render target also texture
https://bugs.freedesktop.org/show_bug.cgi?id=93216 Timothy Arceri changed: What|Removed |Added Status|NEW |NEEDINFO --- Comment #7 from Timothy Arceri --- (In reply to Bas Nieuwenhuizen from comment #6) > Does the series available at > > https://patchwork.freedesktop.org/series/8048/ > > fix this issue? Hi Axel, those patches landed a long time ago. Is this issue still happening or can we close this bug? -- 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 100105] Make Theano OpenCL support work on Clover and RadeonSI
https://bugs.freedesktop.org/show_bug.cgi?id=100105 Jan Vesely changed: What|Removed |Added See Also||https://github.com/Theano/l ||ibgpuarray/issues/462 -- 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 100105] Make Theano OpenCL support work on Clover and RadeonSI
https://bugs.freedesktop.org/show_bug.cgi?id=100105 Jan Vesely changed: What|Removed |Added See Also||https://github.com/Theano/l ||ibgpuarray/issues/491 -- 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 100105] Make Theano OpenCL support work on Clover and RadeonSI
https://bugs.freedesktop.org/show_bug.cgi?id=100105 --- Comment #3 from Jan Vesely --- (In reply to Jan Vesely from comment #2) > It looks like our vstore_half_rtn is not working as expected, which is weird > because it passes CTS. I take this back. vstore_half_rtn rounds to negative infinity (towards 0 for positive numbers). Changing line 53 in cluda_opencl.h: - vstore_half_rtn(f, 0, &r.data); + vstore_half_rte(f, 0, &r.data); fixes the two failures. Other than advertising OCL1.2 the remaining failures are NOTOURBUG. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 3/3] drm: Add helper to validate damage during modeset_check
This patch adds a helper which should be called by driver which enable
damage (by calling drm_plane_enable_damage_clips) from atomic_check
hook. This helper for now set the damage to NULL for the planes on crtc
which need full modeset.
The driver also need to check for other crtc properties which can
affect damage in atomic_check hook, like gamma, ctm, etc. Plane related
properties which affect damage can be handled in damage iterator.
Signed-off-by: Deepak Rawat
---
drivers/gpu/drm/drm_atomic_helper.c | 47 +
include/drm/drm_atomic_helper.h | 2 ++
2 files changed, 49 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 355b514..23f44ab 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3987,3 +3987,50 @@ drm_atomic_helper_damage_iter_next(struct
drm_atomic_helper_damage_iter *iter,
return true;
}
EXPORT_SYMBOL(drm_atomic_helper_damage_iter_next);
+
+/**
+ * drm_atomic_helper_check_damage - validate state object for damage changes
+ * @dev: DRM device
+ * @state: the driver state object
+ *
+ * Check the state object if damage is required or not. In case damage is not
+ * required e.g. need modeset, the damage blob is set to NULL.
+ *
+ * NOTE: This helper is not called by core. Those driver which enable damage
+ * using drm_plane_enable_damage_clips should call this from their
@atomic_check
+ * hook.
+ */
+int drm_atomic_helper_check_damage(struct drm_device *dev,
+ struct drm_atomic_state *state)
+{
+ struct drm_crtc *crtc;
+ struct drm_plane *plane;
+ struct drm_crtc_state *crtc_state;
+ unsigned plane_mask;
+ int i;
+
+ for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
+ if (!drm_atomic_crtc_needs_modeset(crtc_state))
+ continue;
+
+ plane_mask = crtc_state->plane_mask;
+ plane_mask |= crtc->state->plane_mask;
+
+ drm_for_each_plane_mask(plane, dev, plane_mask) {
+ struct drm_plane_state *plane_state =
+ drm_atomic_get_plane_state(state, plane);
+
+ if (IS_ERR(plane_state))
+ return PTR_ERR(plane_state);
+
+ if (plane_state->damage_clips) {
+
drm_property_blob_put(plane_state->damage_clips);
+ plane_state->damage_clips = NULL;
+ plane_state->num_clips = 0;
+ }
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check_damage);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index ebd4b66..b12335c 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -224,6 +224,8 @@ drm_atomic_helper_damage_iter_init(struct
drm_atomic_helper_damage_iter *iter,
bool
drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter *iter,
struct drm_rect *rect);
+int drm_atomic_helper_check_damage(struct drm_device *dev,
+ struct drm_atomic_state *state);
/**
* drm_atomic_crtc_for_each_plane - iterate over planes currently attached to
CRTC
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC 1/3] drm: Add DAMAGE_CLIPS property to plane
From: Lukasz Spintzyk
Optional plane property to mark damaged regions on the plane in
framebuffer coordinates of the framebuffer attached to the plane.
The layout of blob data is simply an array of drm_mode_rect with maximum
array size limited by DRM_MODE_FB_DIRTY_MAX_CLIPS. Unlike plane src
coordinates, damage clips are not in 16.16 fixed point.
Damage clips are a hint to kernel as which area of framebuffer has
changed since last page-flip. This should be helpful for some drivers
especially for virtual devices where each framebuffer change needs to
be transmitted over network, usb, etc.
Driver which are interested in enabling DAMAGE_CLIPS property for a
plane should enable this property using drm_plane_enable_damage_clips.
Signed-off-by: Lukasz Spintzyk
Signed-off-by: Deepak Rawat
---
drivers/gpu/drm/drm_atomic.c| 42 +
drivers/gpu/drm/drm_atomic_helper.c | 4
drivers/gpu/drm/drm_mode_config.c | 5 +
drivers/gpu/drm/drm_plane.c | 12 +++
include/drm/drm_mode_config.h | 15 +
include/drm/drm_plane.h | 16 ++
include/uapi/drm/drm_mode.h | 15 +
7 files changed, 109 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 7d25c42..9226d24 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -669,6 +669,40 @@ static void drm_atomic_crtc_print_state(struct drm_printer
*p,
}
/**
+ * drm_atomic_set_damage_for_plane - sets the damage clips property to plane
+ * @state: plane state
+ * @blob: damage clips in framebuffer coordinates
+ *
+ * Returns:
+ *
+ * Zero on success, error code on failure.
+ */
+static int drm_atomic_set_damage_for_plane(struct drm_plane_state *state,
+ struct drm_property_blob *blob)
+{
+ if (blob == state->damage_clips)
+ return 0;
+
+ drm_property_blob_put(state->damage_clips);
+ state->damage_clips = NULL;
+
+ if (blob) {
+ uint32_t count = blob->length/sizeof(struct drm_rect);
+
+ if (count > DRM_MODE_FB_DIRTY_MAX_CLIPS)
+ return -EINVAL;
+
+ state->damage_clips = drm_property_blob_get(blob);
+ state->num_clips = count;
+ } else {
+ state->damage_clips = NULL;
+ state->num_clips = 0;
+ }
+
+ return 0;
+}
+
+/**
* drm_atomic_get_plane_state - get plane state
* @state: global atomic state object
* @plane: plane to get state object for
@@ -793,6 +827,12 @@ static int drm_atomic_plane_set_property(struct drm_plane
*plane,
state->color_encoding = val;
} else if (property == plane->color_range_property) {
state->color_range = val;
+ } else if (property == config->prop_damage_clips) {
+ struct drm_property_blob *blob =
+ drm_property_lookup_blob(dev, val);
+ int ret = drm_atomic_set_damage_for_plane(state, blob);
+ drm_property_blob_put(blob);
+ return ret;
} else if (plane->funcs->atomic_set_property) {
return plane->funcs->atomic_set_property(plane, state,
property, val);
@@ -856,6 +896,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
*val = state->color_encoding;
} else if (property == plane->color_range_property) {
*val = state->color_range;
+ } else if (property == config->prop_damage_clips) {
+ *val = (state->damage_clips) ? state->damage_clips->base.id : 0;
} else if (plane->funcs->atomic_get_property) {
return plane->funcs->atomic_get_property(plane, state,
property, val);
} else {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index c356545..55b44e3 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3506,6 +3506,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct
drm_plane *plane,
state->fence = NULL;
state->commit = NULL;
+ state->damage_clips = NULL;
+ state->num_clips = 0;
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
@@ -3550,6 +3552,8 @@ void __drm_atomic_helper_plane_destroy_state(struct
drm_plane_state *state)
if (state->commit)
drm_crtc_commit_put(state->commit);
+
+ drm_property_blob_put(state->damage_clips);
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_mode_config.c
b/drivers/gpu/drm/drm_mode_config.c
index e5c6533..e93b127 100644
--- a/drivers/gpu/drm/drm_mode_config.c
+++ b/drivers/gpu/drm/drm_mode_config.c
@@ -293,6 +293,11 @@ static int drm_mode_create_standard_properties(struct
drm_device *dev)
return -ENOMEM;
dev->mode_config.prop
[RFC 2/3] drm: Add helper iterator functions to iterate over plane damage.
With damage property in drm_plane_state, this patch adds helper iterator
to traverse the damage clips. Iterator will return the damage rectangles
in framebuffer, plane or crtc coordinates as need by driver
implementation.
Signed-off-by: Deepak Rawat
---
drivers/gpu/drm/drm_atomic_helper.c | 122
include/drm/drm_atomic_helper.h | 39
2 files changed, 161 insertions(+)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 55b44e3..355b514 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3865,3 +3865,125 @@ void
__drm_atomic_helper_private_obj_duplicate_state(struct drm_private_obj *obj
memcpy(state, obj->state, sizeof(*state));
}
EXPORT_SYMBOL(__drm_atomic_helper_private_obj_duplicate_state);
+
+/**
+ * drm_atomic_helper_damage_iter_init - initialize the damage iterator
+ * @iter: The iterator to initialize.
+ * @type: Coordinate type caller is interested in.
+ * @state: plane_state from which to iterate the damage clips.
+ * @hdisplay: Width of crtc on which plane is scanned out.
+ * @vdisplay: Height of crtc on which plane is scanned out.
+ *
+ * Initialize an iterator that is used to translate and clip a set of damage
+ * rectangles in framebuffer coordinates to plane and crtc coordinates. The
type
+ * argument specify which type of coordinate to iterate in.
+ *
+ * Returns: 0 on success and negative error code on error. If an error code is
+ * returned then it means the plane state should not update.
+ */
+int
+drm_atomic_helper_damage_iter_init(struct drm_atomic_helper_damage_iter *iter,
+ enum drm_atomic_helper_damage_clip_type type,
+ const struct drm_plane_state *state,
+ uint32_t hdisplay, uint32_t vdisplay)
+{
+ if (!state || !state->crtc || !state->fb)
+ return -EINVAL;
+
+ memset(iter, 0, sizeof(*iter));
+ iter->clips = (struct drm_rect *)state->damage_clips->data;
+ iter->num_clips = state->num_clips;
+ iter->type = type;
+
+ /*
+* Full update in case of scaling or rotation. In future support for
+* scaling/rotating damage clips can be added
+*/
+ if (state->crtc_w != (state->src_w >> 16) ||
+ state->crtc_h != state->src_h >> 16 || state->rotation != 0) {
+ iter->curr_clip = iter->num_clips;
+ return 0;
+ }
+
+ iter->fb_src.x1 = 0;
+ iter->fb_src.y1 = 0;
+ iter->fb_src.x2 = state->fb->width;
+ iter->fb_src.y2 = state->fb->height;
+
+ iter->plane_src.x1 = state->src_x >> 16;
+ iter->plane_src.y1 = state->src_y >> 16;
+ iter->plane_src.x2 = iter->plane_src.x1 + (state->src_w >> 16);
+ iter->plane_src.y2 = iter->plane_src.y1 + (state->src_h >> 16);
+ iter->translate_plane_x = -iter->plane_src.x1;
+ iter->translate_plane_y = -iter->plane_src.y1;
+
+ /* Clip plane src rect to fb dimensions */
+ drm_rect_intersect(&iter->plane_src, &iter->fb_src);
+
+ iter->crtc_src.x1 = 0;
+ iter->crtc_src.y1 = 0;
+ iter->crtc_src.x2 = hdisplay;
+ iter->crtc_src.y2 = vdisplay;
+ iter->translate_crtc_x = -(iter->plane_src.x1 - state->crtc_x);
+ iter->translate_crtc_x = -(iter->plane_src.y1 - state->crtc_y);
+
+ /* Clip crtc src rect to plane dimensions */
+ drm_rect_translate(&iter->crtc_src, -iter->translate_crtc_x,
+ -iter->translate_crtc_x);
+ drm_rect_intersect(&iter->crtc_src, &iter->plane_src);
+
+ return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_damage_iter_init);
+
+/**
+ * drm_atomic_helper_damage_iter_next - advance the damage iterator
+ * @iter: The iterator to advance.
+ * @rect: Return a rectangle in coordinate specified during iterator init.
+ *
+ * Returns: true if the output is valid, false if we've reached the end of the
+ * rectangle list. If the first call return false, means need full update.
+ */
+bool
+drm_atomic_helper_damage_iter_next(struct drm_atomic_helper_damage_iter *iter,
+ struct drm_rect *rect)
+{
+ const struct drm_rect *curr_clip;
+
+next_clip:
+ if (iter->curr_clip >= iter->num_clips)
+ return false;
+
+ curr_clip = &iter->clips[iter->curr_clip];
+ iter->curr_clip++;
+
+ rect->x1 = curr_clip->x1;
+ rect->x2 = curr_clip->x2;
+ rect->y1 = curr_clip->y1;
+ rect->y2 = curr_clip->y2;
+
+ /* Clip damage rect within fb limit */
+ if (!drm_rect_intersect(rect, &iter->fb_src))
+ goto next_clip;
+ else if (iter->type & DRM_ATOMIC_HELPER_DAMAGE_CLIP_TYPE_FB)
+ return true;
+
+ /* Clip damage rect within plane limit */
+ if (!drm_rect_intersect(rect, &iter->plane_src))
+ goto next_clip;
+ else if (ite
[RFC 0/3] drm: page-flip with damage
Hi All, This is extension to Lukasz Spintzyk earlier draft of damage interface for drm. Bascially a new plane property is added called "DAMAGE_CLIPS" which is simply an array of drm_rect (exported to userspace as drm_mode_rect). The clips represents damage in framebuffer coordinate of attached fb to the plane. Helper iterator is added to traverse the damage rectangles and get the damage clips in framebuffer, plane or crtc coordinates as need by driver implementation. Finally a helper to reset damage in case need full update is required. Drivers interested in page-flip with damage should call this from atomic_check hook. With the RFC for atomic implementation of dirtyfb ioctl I was thinking should we need to consider dirty_fb flags, especially DRM_MODE_FB_DIRTY_ANNOTATE_COPY to be passed with atomic DAMAGE_CLIPS property blob? I didn't considered that untill now. If no driver uses that in my opinion for simplicity this can be ignored? About overlaping of damage rectangles is also not finalized. This really depends on driver specific implementation and can be left open-ended? My knowledge is limited to vmwgfx so would like to hear about other driver use cases and this can be modified in keeping other drivers need. Going forward driver implementation for vmwgfx and user-space implementation of kmscube/weston will be next step to test the changes. Thanks, Deepak Deepak Rawat (2): drm: Add helper iterator functions to iterate over plane damage. drm: Add helper to validate damage during modeset_check Lukasz Spintzyk (1): drm: Add DAMAGE_CLIPS property to plane drivers/gpu/drm/drm_atomic.c| 42 + drivers/gpu/drm/drm_atomic_helper.c | 173 drivers/gpu/drm/drm_mode_config.c | 5 ++ drivers/gpu/drm/drm_plane.c | 12 +++ include/drm/drm_atomic_helper.h | 41 + include/drm/drm_mode_config.h | 15 include/drm/drm_plane.h | 16 include/uapi/drm/drm_mode.h | 15 8 files changed, 319 insertions(+) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100105] Make Theano OpenCL support work on Clover and RadeonSI
https://bugs.freedesktop.org/show_bug.cgi?id=100105
--- Comment #2 from Jan Vesely ---
Latest update:
diff --git a/src/cluda_opencl.h b/src/cluda_opencl.h
index 6e0095c..8ba2d14 100644
--- a/src/cluda_opencl.h
+++ b/src/cluda_opencl.h
@@ -48,7 +48,7 @@ typedef struct _ga_half {
} ga_half;
#define ga_half2float(p) vload_half(0, &((p).data))
-static inline ga_half ga_float2half(ga_float f) {
+inline ga_half ga_float2half(ga_float f) {
ga_half r;
vstore_half_rtn(f, 0, &r.data);
return r;
diff --git a/src/gpuarray_buffer_opencl.c b/src/gpuarray_buffer_opencl.c
index 8f12811..2041ca2 100644
--- a/src/gpuarray_buffer_opencl.c
+++ b/src/gpuarray_buffer_opencl.c
@@ -146,7 +146,7 @@ cl_ctx *cl_make_ctx(cl_context ctx, gpucontext_props *p) {
CL_CHECKN(global_err, clGetDeviceInfo(id, CL_DEVICE_VERSION,
device_version_size,
device_version, NULL));
- if (device_version[7] == '1' && device_version[9] < '2') {
+ if (device_version[7] == '1' && device_version[9] < '1') {
error_set(global_err, GA_UNSUPPORTED_ERROR,
"We only support OpenCL 1.2 and up");
return NULL
>>> pygpu.test()
pygpu is installed in
/home/jvesely/.local/lib/python3.6/site-packages/pygpu-0.7.5+12.g6f0132c.dirty-py3.6-linux-x86_64.egg/pygpu
NumPy version 1.13.3
NumPy relaxed strides checking option: True
NumPy is installed in /usr/lib64/python3.6/site-packages/numpy
Python version 3.6.4 (default, Mar 13 2018, 18:18:20) [GCC 7.3.1 20180303 (Red
Hat 7.3.1-5)]
nose version 1.3.7
*** Testing for AMD Radeon R7 Graphics (CARRIZO / DRM 3.23.0 /
4.15.14-300.fc27.x86_64, LLVM 6.0.0)
--
Ran 6670 tests in 995.728s
FAILED (SKIP=12, errors=580, failures=2)
All errors are: TypeError: This is for CUDA arrays.
The two failures are:
FAIL: pygpu.tests.test_elemwise.test_elemwise_f16(,
'float16', 'float16', (50,))
FAIL: pygpu.tests.test_elemwise.test_elemwise_f16(,
'float16', 'float16', (50,))
Which fail on half precision rounding error. for example:
7.0390625+7.20703125 is expected to be 14.25 but gpu returns 14.2421875
the fp32 result is 14.24609375.
The GPU result is rounded down (towards zero)
The CPU result is rounded up (away from zero)
It looks like our vstore_half_rtn is not working as expected, which is weird
because it passes CTS.
--
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] [RESEND] drm/i915/dp: Send DPCD ON for MST before phy_up
As it turns out, the aux block being off was not the real problem here,
as transition from D3 to D0 is mandated by the DP spec to take a maximum
of 1ms, whereas we're allowed a 100ms timeframe to respond to ESI irqs.
The real problem here is a bit more subtle.
When doing a modeset where the problem of the sink timing out to our
sideband requests when transitioning from D3 to D0 occurs, the timeout
is from the aux block not coming on. However, nothing else times out
other than the initial phy_up message because the DPCD on call in
intel_ddi_enable_dp() ends up waking up the AUX block on the hub, not
the phy_up sideband message. This means that the real fix we need is to
use the DPMS on before sending a phy_up to ensure that the hub is ready
to accept sideband messages.
Signed-off-by: Lyude Paul
Cc: Dhinakaran Pandiyan
Cc: Ville Syrjälä
Cc: Laura Abbott
Cc: sta...@vger.kernel.org
Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.")
---
drivers/gpu/drm/i915/intel_ddi.c| 6 +-
drivers/gpu/drm/i915/intel_dp_mst.c | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index a6672a9abd85..9bd675f73f7b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2324,7 +2324,11 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder
*encoder,
intel_prepare_dp_ddi_buffers(encoder, crtc_state);
intel_ddi_init_dp_buf_reg(encoder);
- intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+ /* for MST, we do DPMS_ON outside of here so that DPMS_ON can happen
+* before drm_dp_send_power_updown_phy()
+*/
+ if (!intel_dp->is_mst)
+ intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
intel_dp_stop_link_train(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index c3de0918ee13..eff9a4eae1f0 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -223,6 +223,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder
*encoder,
DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
+ intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port, true);
if (intel_dp->active_mst_links == 0)
intel_dig_port->base.pre_enable(&intel_dig_port->base,
--
2.14.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/i915/dp: Send DPCD ON for MST before phy_up
As it turns out, the aux block being off was not the real problem here,
as transition from D3 to D0 is mandated by the DP spec to take a maximum
of 1ms, whereas we're allowed a 100ms timeframe to respond to ESI irqs.
The real problem here is a bit more subtle.
When doing a modeset where the problem of the sink timing out to our
sideband requests when transitioning from D3 to D0 occurs, the timeout
is from the aux block not coming on. However, nothing else times out
other than the initial phy_up message because the DPCD on call in
intel_ddi_enable_dp() ends up waking up the AUX block on the hub, not
the phy_up sideband message. This means that the real fix we need is to
use the DPMS on before sending a phy_up to ensure that the hub is ready
to accept sideband messages.
Signed-off-by: Lyude Paul
Cc: Dhinakaran Pandiyan
Cc: Ville Syrjälä
Cc: Laura Abbott
Cc: sta...@vger.kernel.org
Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.")
---
drivers/gpu/drm/i915/intel_ddi.c| 6 +-
drivers/gpu/drm/i915/intel_dp_mst.c | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index a6672a9abd85..9bd675f73f7b 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2324,7 +2324,11 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder
*encoder,
intel_prepare_dp_ddi_buffers(encoder, crtc_state);
intel_ddi_init_dp_buf_reg(encoder);
- intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+ /* for MST, we do DPMS_ON outside of here so that DPMS_ON can happen
+* before drm_dp_send_power_updown_phy()
+*/
+ if (!intel_dp->is_mst)
+ intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
intel_dp_start_link_train(intel_dp);
if (port != PORT_A || INTEL_GEN(dev_priv) >= 9)
intel_dp_stop_link_train(intel_dp);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index c3de0918ee13..eff9a4eae1f0 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -223,6 +223,7 @@ static void intel_mst_pre_enable_dp(struct intel_encoder
*encoder,
DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
+ intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
drm_dp_send_power_updown_phy(&intel_dp->mst_mgr, connector->port, true);
if (intel_dp->active_mst_links == 0)
intel_dig_port->base.pre_enable(&intel_dig_port->base,
--
2.14.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm: Fix downstream dev count read
On Thursday 05 April 2018 12:53 AM, Sean Paul wrote: On Wed, Apr 04, 2018 at 12:07:41PM -0700, Rodrigo Vivi wrote: On Wed, Apr 04, 2018 at 11:57:42PM +0530, Ramalingam C wrote: In both HDMI and DP, device count is represented by 6:0 bits of a register(BInfo/Bstatus) So macro for bitmasking the device_count is fixed(0x3F->0x7F). v3: Retained the Rb-ed. v4: %s/drm\/i915/drm [rodrigo] Shouldn't this patch have a "Fixes:" ? Yes, I think that'd be good. Will add Fixes: 495eb7f877ab drm: Add some HDCP related #defines cc: stable? It couldn't hurt. Sorry what is needed here? I pushed first 3 patches on the series to dinq. I believe this one here could be there with Dave's ack or maybe on drm-misc-fixes? Meh. The severity of this isn't too big, given that I doubt people care _too_ much about plugging in more than 64 HDCP-enabled devices. If you want to drop it in -misc-next-fixes, I can send it out next week. While we're asking for a respin, could we add HDCP somewhere in the subject? will change the sub to drm: Fix HDCP downstream dev count read --Ram Sean Signed-off-by: Ramalingam C cc: Sean Paul Reviewed-by: Sean Paul --- include/drm/drm_hdcp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h index 562fa7df2637..98e63d870139 100644 --- a/include/drm/drm_hdcp.h +++ b/include/drm/drm_hdcp.h @@ -19,7 +19,7 @@ #define DRM_HDCP_RI_LEN 2 #define DRM_HDCP_V_PRIME_PART_LEN 4 #define DRM_HDCP_V_PRIME_NUM_PARTS5 -#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x3f) +#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x7f) #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x) (x & BIT(3)) #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x) (x & BIT(7)) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/sched: Extend the documentation.
These comments answer all the questions I had for myself when
implementing a driver using the GPU scheduler.
Signed-off-by: Eric Anholt
---
include/drm/gpu_scheduler.h | 46 +
1 file changed, 42 insertions(+), 4 deletions(-)
diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h
index dfd54fb94e10..c053a32341bf 100644
--- a/include/drm/gpu_scheduler.h
+++ b/include/drm/gpu_scheduler.h
@@ -43,10 +43,12 @@ enum drm_sched_priority {
};
/**
- * A scheduler entity is a wrapper around a job queue or a group
- * of other entities. Entities take turns emitting jobs from their
- * job queues to corresponding hardware ring based on scheduling
- * policy.
+ * drm_sched_entity - A wrapper around a job queue (typically attached
+ * to the DRM file_priv).
+ *
+ * Entities will emit jobs in order to their corresponding hardware
+ * ring, and the scheduler will alternate between entities based on
+ * scheduling policy.
*/
struct drm_sched_entity {
struct list_headlist;
@@ -78,7 +80,18 @@ struct drm_sched_rq {
struct drm_sched_fence {
struct dma_fencescheduled;
+
+ /* This fence is what will be signaled by the scheduler when
+* the job is completed.
+*
+* When setting up an out fence for the job, you should use
+* this, since it's available immediately upon
+* drm_sched_job_init(), and the fence returned by the driver
+* from run_job() won't be created until the dependencies have
+* resolved.
+*/
struct dma_fencefinished;
+
struct dma_fence_cb cb;
struct dma_fence*parent;
struct drm_gpu_scheduler*sched;
@@ -88,6 +101,13 @@ struct drm_sched_fence {
struct drm_sched_fence *to_drm_sched_fence(struct dma_fence *f);
+/**
+ * drm_sched_job - A job to be run by an entity.
+ *
+ * A job is created by the driver using drm_sched_job_init(), and
+ * should call drm_sched_entity_push_job() once it wants the scheduler
+ * to schedule the job.
+ */
struct drm_sched_job {
struct spsc_nodequeue_node;
struct drm_gpu_scheduler*sched;
@@ -112,10 +132,28 @@ static inline bool drm_sched_invalidate_job(struct
drm_sched_job *s_job,
* these functions should be implemented in driver side
*/
struct drm_sched_backend_ops {
+ /* Called when the scheduler is considering scheduling this
+* job next, to get another struct dma_fence for this job to
+* block on. Once it returns NULL, run_job() may be called.
+*/
struct dma_fence *(*dependency)(struct drm_sched_job *sched_job,
struct drm_sched_entity *s_entity);
+
+ /* Called to execute the job once all of the dependencies have
+* been resolved. This may be called multiple times, if
+* timedout_job() has happened and drm_sched_job_recovery()
+* decides to try it again.
+*/
struct dma_fence *(*run_job)(struct drm_sched_job *sched_job);
+
+ /* Called when a job has taken too long to execute, to trigger
+* GPU recovery.
+*/
void (*timedout_job)(struct drm_sched_job *sched_job);
+
+ /* Called once the job's finished fence has been signaled and
+* it's time to clean it up.
+*/
void (*free_job)(struct drm_sched_job *sched_job);
};
--
2.17.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 13/15] v4l: vsp1: Assign BRU and BRS to pipelines dynamically
Hi Kieran,
On Wednesday, 4 April 2018 19:00:10 EEST Kieran Bingham wrote:
> Hi Laurent,
>
> Thank you for the patch.
>
> I don't envy you on having to deal with this one ... it's a bit of a pain
> ...
Yes it was a bit painful :-/ The devil was both in the big picture and the
details this time.
> On 26/02/18 21:45, Laurent Pinchart wrote:
> > The VSPDL variant drives two DU channels through two LIF and two
> > blenders, BRU and BRS. The DU channels thus share the five available
> > VSPDL inputs and expose them as five KMS planes.
> >
> > The current implementation assigns the BRS to the second LIF and thus
> > artificially limits the number of planes for the second display channel
> > to two at most.
> >
> > Lift this artificial limitation by assigning the BRU and BRS to the
> > display pipelines on demand based on the number of planes used by each
> > pipeline. When a display pipeline needs more than two inputs and the BRU
> > is already in use by the other pipeline, this requires reconfiguring the
> > other pipeline to free the BRU before processing, which can result in
> > frame drop on both pipelines.
>
> So this is a hard one!
> - Having to dynamically reconfigure "someone else's" pipes ...
>
> > Signed-off-by: Laurent Pinchart
> >
>
> Except for the recursion, which is unavoidable, and the lock handling across
> function calls which is ... unavoidable I think as well (at least for the
> moment), my only quibble is the naming of the 'notify' variable, which is
> not particularly clear in terms of who it notifies. (Internal, vs DRM)
>
> I'll leave it up to you to decide whether or not to rename it though, and if
> you're happy with the naming then fine.
I agree with you, please see below.
> Reviewed-by: Kieran Bingham
>
> > ---
> >
> > drivers/media/platform/vsp1/vsp1_drm.c | 160 ++--
> > drivers/media/platform/vsp1/vsp1_drm.h | 9 ++
> > 2 files changed, 144 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> > b/drivers/media/platform/vsp1/vsp1_drm.c index 6c60b72b6f50..87e31ba0ddf5
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_drm.c
> > +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> > @@ -39,7 +39,13 @@ static void vsp1_du_pipeline_frame_end(struct
> > vsp1_pipeline *pipe,>
> > struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
> >
> > if (drm_pipe->du_complete)
> >
> > - drm_pipe->du_complete(drm_pipe->du_private, completed);
> > + drm_pipe->du_complete(drm_pipe->du_private,
> > + completed && !notify);
> > +
> > + if (notify) {
> > + drm_pipe->force_bru_release = false;
> > + wake_up(&drm_pipe->wait_queue);
> > + }
>
> Notify seems such a nondescript verb to use here, and confuses me against
> who we are notifying - and why (it's an internal notification, but notify
> makes me think we are 'notifying' the DU - which is exactly the opposite).
>
> (Perhaps this is actually a comment for the previous patch, but I've gone
> out-of-order, due to the complexities here...)
>
> Could this be 'internal', 'released' or 'reconfigured', or something to
> distinguish that this frame-end is not a normal frame-completion event ?
I think internal is a better name than notify. I'll pick that.
> > }
> >
> > /* --
> > @@ -149,6 +155,10 @@ static int vsp1_du_pipeline_setup_rpf(struct
> > vsp1_device *vsp1,
> > }
> >
> > /* Setup the BRU source pad. */
> > +static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1,
> > + struct vsp1_pipeline *pipe);
> > +static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe);
> > +
>
> Ohhh lovely, recursion...
> Ohhh lovely, recursion...
I wanted to avoid it but haven't found a better way.
> > static int vsp1_du_pipeline_setup_bru(struct vsp1_device *vsp1,
> > struct vsp1_pipeline *pipe)
> > {
> > @@ -156,8 +166,93 @@ static int vsp1_du_pipeline_setup_bru(struct
> > vsp1_device *vsp1,
> > struct v4l2_subdev_format format = {
> > .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> > };
> > + struct vsp1_entity *bru;
> > int ret;
> >
> > + /*
> > +* Pick a BRU:
> > +* - If we need more than two inputs, use the main BRU.
> > +* - Otherwise, if we are not forced to release our BRU, keep it.
> > +* - Else, use any free BRU (randomly starting with the main BRU).
> > +*/
> > + if (pipe->num_inputs > 2)
> > + bru = &vsp1->bru->entity;
> > + else if (pipe->bru && !drm_pipe->force_bru_release)
> > + bru = pipe->bru;
> > + else if (!vsp1->bru->entity.pipe)
> > + bru = &vsp1->bru->entity;
> > + else
> > + bru = &vsp1->brs->entity;
>
> Ok - that tooks some iterations to go through - but I think it covers all
> the bases.
>
> > +
> > + /* Switc
[Bug 104082] amdgpu 0000:07:00.0: swiotlb buffer is full (sz: 2097152 bytes)
https://bugs.freedesktop.org/show_bug.cgi?id=104082 b...@nord-west.org changed: What|Removed |Added Status|RESOLVED|REOPENED Resolution|FIXED |--- --- Comment #39 from b...@nord-west.org --- Issue appears here on 4.16 too: [19470.368310] radeon :00:01.0: swiotlb buffer is full (sz: 2097152 bytes) [19470.368334] radeon :00:01.0: swiotlb: coherent allocation failed, size=2097152 [19470.368341] CPU: 0 PID: 815 Comm: Xorg Not tainted 4.16.0-1-ARCH #1 [19470.368343] Hardware name: LENOVO 20BLS00400/20BLS00400, BIOS GSET69WW (2.14 ) 09/28/2017 [19470.368345] Call Trace: [19470.368362] dump_stack+0x5c/0x85 [19470.368369] swiotlb_alloc_coherent+0x1be/0x1d0 [19470.368387] ttm_dma_pool_get_pages+0x215/0x5d0 [ttm] [19470.368401] ttm_dma_populate+0x25b/0x350 [ttm] [19470.368412] ttm_tt_bind+0x2c/0x60 [ttm] [19470.368423] ttm_bo_handle_move_mem+0x577/0x5b0 [ttm] [19470.368436] ttm_bo_validate+0x120/0x130 [ttm] [19470.368474] ? drm_vma_offset_add+0x41/0x60 [drm] [19470.368478] ? acpi_os_map_iomem+0x52/0x180 [19470.368489] ttm_bo_init_reserved+0x395/0x460 [ttm] [19470.368500] ttm_bo_init+0x62/0xe0 [ttm] [19470.368549] ? radeon_update_memory_usage.isra.0+0x50/0x50 [radeon] [19470.368584] radeon_bo_create+0x180/0x240 [radeon] [19470.368622] ? radeon_update_memory_usage.isra.0+0x50/0x50 [radeon] [19470.368660] radeon_gem_object_create+0xa7/0x190 [radeon] [19470.368700] ? radeon_gem_pwrite_ioctl+0x30/0x30 [radeon] [19470.368737] radeon_gem_create_ioctl+0x66/0x100 [radeon] [19470.368776] ? radeon_gem_pwrite_ioctl+0x30/0x30 [radeon] [19470.368798] drm_ioctl_kernel+0x5b/0xb0 [drm] [19470.368823] drm_ioctl+0x2d5/0x370 [drm] [19470.368862] ? radeon_gem_pwrite_ioctl+0x30/0x30 [radeon] [19470.368869] ? vfs_writev+0xb9/0x110 [19470.368901] radeon_drm_ioctl+0x49/0x80 [radeon] [19470.368909] do_vfs_ioctl+0xa4/0x630 [19470.368915] ? __sys_recvmsg+0x4e/0x90 [19470.368919] ? __sys_recvmsg+0x7d/0x90 [19470.368924] SyS_ioctl+0x74/0x80 [19470.368931] do_syscall_64+0x74/0x190 [19470.368937] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 [19470.368942] RIP: 0033:0x7f7b3e17ad87 [19470.368945] RSP: 002b:7ffe18b59e08 EFLAGS: 3246 ORIG_RAX: 0010 [19470.368950] RAX: ffda RBX: 0002 RCX: 7f7b3e17ad87 [19470.368952] RDX: 7ffe18b59e80 RSI: c020645d RDI: 0010 [19470.368954] RBP: 7ffe18b59e80 R08: 0002 R09: [19470.368956] R10: 00207000 R11: 3246 R12: c020645d [19470.368958] R13: 0010 R14: 55c9b1526e90 R15: 55c9b1526e90 [19615.347634] swiotlb_tbl_map_single: 46 callbacks suppressed -- 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 11/15] v4l: vsp1: Add per-display list completion notification support
Hi Kieran,
On Wednesday, 4 April 2018 19:16:46 EEST Kieran Bingham wrote:
> On 26/02/18 21:45, Laurent Pinchart wrote:
> > Display list completion is already reported to the frame end handler,
> > but that mechanism is global to all display lists. In order to implement
> > BRU and BRS reassignment in DRM pipelines we will need to wait for
> > completion of a particular display list. Extend the display list and
> > frame end handler APIs to support such a notification.
> >
> > Signed-off-by: Laurent Pinchart
> >
> > ---
> >
> > drivers/media/platform/vsp1/vsp1_dl.c| 27 +--
> > drivers/media/platform/vsp1/vsp1_dl.h| 4 ++--
> > drivers/media/platform/vsp1/vsp1_drm.c | 4 ++--
> > drivers/media/platform/vsp1/vsp1_pipe.c | 5 +++--
> > drivers/media/platform/vsp1/vsp1_pipe.h | 3 ++-
> > drivers/media/platform/vsp1/vsp1_video.c | 4 ++--
> > 6 files changed, 36 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> > b/drivers/media/platform/vsp1/vsp1_dl.c index 0b86ed01e85d..eb2971218e28
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_dl.c
> > +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> > @@ -72,6 +72,7 @@ struct vsp1_dl_body {
> > * @fragments: list of extra display list bodies
> > * @has_chain: if true, indicates that there's a partition chain
> > * @chain: entry in the display list partition chain
> > + * @notify: whether the display list completion should be notified
> > */
> > struct vsp1_dl_list {
> > struct list_head list;
> > @@ -85,6 +86,8 @@ struct vsp1_dl_list {
> > bool has_chain;
> > struct list_head chain;
> > +
> > + bool notify;
> > };
> >
> > enum vsp1_dl_mode {
> > @@ -550,8 +553,16 @@ static void vsp1_dl_list_commit_continuous(struct
> > vsp1_dl_list *dl)>
> > * case we can't replace the queued list by the new one, as we could
> > * race with the hardware. We thus mark the update as pending, it will
> > * be queued up to the hardware by the frame end interrupt handler.
> > +*
> > +* If a display list is already pending we simply drop it as the new
> > +* display list is assumed to contain a more recent configuration. It
> > is
> > +* an error if the already pending list has the notify flag set, as
> > +* there is then a process waiting for that list to complete. This
> > +* shouldn't happen as the waiting process should perform proper
> > +* locking, but warn just in case.
> > */
> > if (vsp1_dl_list_hw_update_pending(dlm)) {
> > + WARN_ON(dlm->pending && dlm->pending->notify);
> > __vsp1_dl_list_put(dlm->pending);
> > dlm->pending = dl;
> >
> > @@ -581,7 +592,7 @@ static void vsp1_dl_list_commit_singleshot(struct
> > vsp1_dl_list *dl)
> > dlm->active = dl;
> > }
> >
> > -void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
> > +void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool notify)
>
> Rather than changing the vsp1_dl_list_commit() function - would it be nicer
> to have an API to request or set the notify property? :
>
> @@..@@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
> ...
> + /* The BRx will be released, without sending an update to DRM */
> + if (drm_pipe->force_bru_release)
> + vsp1_dl_list_request_internal_notify(dl);
>
> vsp1_dl_list_commit(dl);
> ...
That's not a bad idea, but I wonder if it's worth it as we'll have to call an
extra function for what is essentially an internal API. On the other hand this
isn't a common case, so it's not a hot code path. We could also argue equally
that it is the commit that is internal or that it is the display list that is
for internal purpose. Do you think an extra function call is better ? If you
do I'll change it.
> > {
> > struct vsp1_dl_manager *dlm = dl->dlm;
> > struct vsp1_dl_list *dl_child;
> > @@ -598,6 +609,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
> > }
> > }
> >
> > + dl->notify = notify;
> > +
> > spin_lock_irqsave(&dlm->lock, flags);
> >
> > if (dlm->singleshot)
> > @@ -615,16 +628,23 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
> > /**
> > * vsp1_dlm_irq_frame_end - Display list handler for the frame end
> > interrupt
> > * @dlm: the display list manager
> > + * @notify: whether the display list that completed has notification
> > enabled
> > *
> > * Return true if the previous display list has completed at frame end,
> > or false
> > * if it has been delayed by one frame because the display list commit
> > raced
> > * with the frame end interrupt. The function always returns true in
> > header mode
> > * as display list processing is then not continuous and races never
> > occur.
> > + *
> > + * Upon return, the @notify parameter is set to true if the previous
> > display
> > + * list has completed and had been queued with the notify flag, or to
> > false
> > + * otherwi
[Bug 105200] [r600g] Regression: ImageMagick OpenCL kernel no longer compiles
https://bugs.freedesktop.org/show_bug.cgi?id=105200 --- Comment #12 from Jan Vesely --- (In reply to nixscripter from comment #11) > I finally got back to this, and sure enough, upgrading LLVM to r327238 seems > to have fixed it. Thanks for your efforts. > > Marking as RESOLVED FIXED. Note that it will probably give wrong results if you compiled without assertions. -- 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 libdrm v2 21/23] meson: build exynos by default
For exynos and omap, are they in active use anymore? I'm pretty sure that development of omap (the hardware) stopped, and others have told me exynos has stopped too. I also don't think there's any open source consumer, since there is no mesa driver for either of these. Dylan Quoting Eric Engestrom (2018-04-04 08:38:16) > Signed-off-by: Eric Engestrom > --- > meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meson.build b/meson.build > index 24688535a329ac530c10..7b26977a9e84290fdd37 100644 > --- a/meson.build > +++ b/meson.build > @@ -76,7 +76,7 @@ foreach d : [ >['nouveau', true, true], >['vmwgfx', false, true], >['omap', true, true], > - ['exynos', false, false], > + ['exynos', false, true], >['freedreno', true, ['arm', > 'aarch64'].contains(host_machine.cpu_family())], >['tegra', true, false], >['vc4', false, ['arm', 'aarch64'].contains(host_machine.cpu_family())], > -- > Cheers, > Eric > signature.asc Description: signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm v2 22/23] meson: build tegra by default
Please CC Thierry or someone else from nvidia about this, Also I think this should be ['arm', 'aarch64'], like vc4 and freedreno. Quoting Eric Engestrom (2018-04-04 08:38:17) > Signed-off-by: Eric Engestrom > --- > meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meson.build b/meson.build > index 7b26977a9e84290fdd37..e816740cb240922bf98a 100644 > --- a/meson.build > +++ b/meson.build > @@ -78,7 +78,7 @@ foreach d : [ >['omap', true, true], >['exynos', false, true], >['freedreno', true, ['arm', > 'aarch64'].contains(host_machine.cpu_family())], > - ['tegra', true, false], > + ['tegra', true, true], >['vc4', false, ['arm', 'aarch64'].contains(host_machine.cpu_family())], >['etnaviv', true, false], > ] > -- > Cheers, > Eric > signature.asc Description: signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm v2 23/23] meson: build etnaviv by default
Please CC the etnaviv maintainers on this as well. I think this should be ['arm', 'aarch64']... like vc4 and fredreno Quoting Eric Engestrom (2018-04-04 08:38:18) > Signed-off-by: Eric Engestrom > --- > meson.build | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/meson.build b/meson.build > index e816740cb240922bf98a..a725f05d342bbec4df18 100644 > --- a/meson.build > +++ b/meson.build > @@ -80,7 +80,7 @@ foreach d : [ >['freedreno', true, ['arm', > 'aarch64'].contains(host_machine.cpu_family())], >['tegra', true, true], >['vc4', false, ['arm', 'aarch64'].contains(host_machine.cpu_family())], > - ['etnaviv', true, false], > + ['etnaviv', true, true], > ] >driver = d[0] >require_atomics = d[1] > -- > Cheers, > Eric > signature.asc Description: signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 10/15] v4l: vsp1: Move DRM pipeline output setup code to a function
Hi Kieran,
On Wednesday, 4 April 2018 19:15:19 EEST Kieran Bingham wrote:
> On 02/04/18 13:35, Laurent Pinchart wrote:
>
>
>
> >>> +/* Setup the output side of the pipeline (WPF and LIF). */
> >>> +static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1,
> >>> + struct vsp1_pipeline *pipe)
> >>> +{
> >>> + struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
> >>> + struct v4l2_subdev_format format = {
> >>> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
> >>
> >> Why do you initialise this .which here, but all the other member
> >> variables below.
> >>
> >> Wouldn't it make more sense to group all of this initialisation together?
> >> or is there a distinction in keeping the .which separate.
> >>
> >> (Perhaps this is just a way to initialise the rest of the structure to 0,
> >> without using the memset?)
> >
> > The initialization of the .which field is indeed there to avoid the
> > memset, but other than that there's no particular reason. I find it
> > clearer to keep the initialization of the structure close to the code that
> > makes use of it (the next v4l2_subdev_call in this case).
> >
> > As initializing all members when declaring the variable doesn't make a
> > change in code size (gcc 6.4.0) but increases .rodata by 18 bytes and
> > decreases __modver by the same amount, I'm tempted to leave it as-is
> > unless you think it should be changed.
>
> I'm happy to leave it as is - the query was as much to understand why the
> change was the way it was :D
>
> But on that logic (reducing .rodata, or rather not increasing it) what's the
> benefit of initialising with one (random/psuedo random) member variable
> over initialising to all zero, then initialising the .which alongside the
> rest of them? Wouldn't the compiler just use the zero page or such to
> initialise then?
I've just tested that, and it seems to generate the exact same code. I'll
initialize the structure to 0 when declaring it and move the which field
initialization with the other fields.
> This way is fine if you are happy with how it reads :D
>
> >>> + };
> >>> + int ret;
> >>> +
> >>> + format.pad = RWPF_PAD_SINK;
> >>> + format.format.width = drm_pipe->width;
> >>> + format.format.height = drm_pipe->height;
> >>> + format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
> >>> + format.format.field = V4L2_FIELD_NONE;
> >>> +
> >>> + ret = v4l2_subdev_call(&pipe->output->entity.subdev, pad, set_fmt,
> >
> > NULL,
> >
> >>> +&format);
> >>> + if (ret < 0)
> >>> + return ret;
> >>> +
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm v2 19/23] meson: use simple option handling for etnaviv
You can ignore my comments on the first couple of patches if you like, I think
the result is much nicer anyway.
1-19 are:
Reviewed-by: Dylan Baker
Quoting Eric Engestrom (2018-04-04 08:38:14)
> Signed-off-by: Eric Engestrom
> ---
> meson.build | 10 +-
> 1 file changed, 1 insertion(+), 9 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index f3747736f5bed7c01143..f659c02bc82660d038cc 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -80,6 +80,7 @@ foreach d : [
>['freedreno', true, ['arm',
> 'aarch64'].contains(host_machine.cpu_family())],
>['tegra', true, false],
>['vc4', false, ['arm', 'aarch64'].contains(host_machine.cpu_family())],
> + ['etnaviv', true, false],
> ]
>driver = d[0]
>require_atomics = d[1]
> @@ -100,15 +101,6 @@ foreach d : [
>endif
> endforeach
>
> -with_etnaviv = false
> -_etnaviv = get_option('etnaviv')
> -if _etnaviv == 'true'
> - if not with_atomics
> -error('libdrm_etnaviv requires atomics.')
> - endif
> - with_etnaviv = true
> -endif
> -
> with_exynos = get_option('exynos') == 'true'
>
> # XXX: Aparently only freebsd and dragonfly bsd actually need this (and
> --
> Cheers,
> Eric
>
signature.asc
Description: signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm v2 02/23] meson: don't enable libdrm_radeon without atomic support
Quoting Eric Engestrom (2018-04-04 08:37:57)
> In the 'auto' case, the `with_atomic` check was bypassed.
>
> Signed-off-by: Eric Engestrom
> ---
> meson.build | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/meson.build b/meson.build
> index e762dcc44bff5deac4d1..72cdd14a3ba834abde4d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -82,11 +82,13 @@ endif
>
> with_radeon = false
> _radeon = get_option('radeon')
> -if _radeon != 'false'
> - if _radeon == 'true' and not with_atomics
> -error('libdrm_radeon requires atomics.')
> - endif
> - with_radeon = true
What about just change this to `with_radeon = with_atomics`? We've already
verified that if radeon == true that atomics are present.
> +if _radeon == 'auto'
> + with_radeon = with_atomics
> +else
> + with_radeon = _radeon == 'true'
> +endif
> +if with_radeon and not with_atomics
> + error('libdrm_radeon requires atomics.')
> endif
>
> with_amdgpu = false
> --
> Cheers,
> Eric
>
signature.asc
Description: signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Closing and repoening laptop lid causes scanout corruption (regression since 4.15.12)
Hi, ever since I upgraded my system to kernel version 4.15.12 I'm experiencing a kind of interesting problem with my laptop's graphics output. The system is a Dell Latitude E6400 with Intel GM45 graphics. Kernel version is 4.15.12, X.org driver is xf86-video-intel-2.99.917.812, X.org server is xorg-server-1.19.6 Output of `lspci -v`: 00:02.0 VGA compatible controller: Intel Corporation Mobile 4 Series Chipset Integrated Graphics Controller (rev 07) (prog-if 00 [VGA controller]) Subsystem: Dell Device 0233 Flags: bus master, fast devsel, latency 0, IRQ 16 Memory at f6c0 (64-bit, non-prefetchable) [size=4M] Memory at e000 (64-bit, prefetchable) [size=256M] I/O ports at ef98 [size=8] [virtual] Expansion ROM at 000c [disabled] [size=128K] Capabilities: [90] MSI: Enable- Count=1/1 Maskable- 64bit- Capabilities: [d0] Power Management version 3 Kernel driver in use: i915 Kernel modules: i915 Problem description: Whenever the laptop's lid is closed and opened the lower part of the graphics output is rapidly flickering between (corrupted) scanout buffers, with the expected output appearing intermittently inbetween. If running X.org there's distinct horizontal tear between where the graphics output is in order and below which it's corrupted. The vertical position coincides and follows the top boundary of the cursor. Switching the VT between graphics mode to text and back to graphics restores the graphics output into a sane state. However the graphics returns to a sane state only after switching back to graphics. While in fbcon "text" mode it remains corrupted. This behavior happens consistently and reproducibly regardless of power management or ACPI daemons in user space. Putting the system into single-user mode, terminating all daemons (including udev and other hardware management stuff) still allows to reproduce the behavior. Interestingly enough when closing the lid a completely unrelated subsystem produced kernel messages: The PC-Card / PCMCIA / yenta_cardbus subsystem, emits these messages when the laptop lid is closed and opened yenta_cardbus :03:01.0: CardBus bridge to [bus 04] yenta_cardbus :03:01.0: bridge window [io 0x5000-0x50ff] yenta_cardbus :03:01.0: bridge window [io 0x5400-0x54ff] yenta_cardbus :03:01.0: bridge window [mem 0xf0c0-0xf0ff] yenta_cardbus :03:01.0: bridge window [mem 0xf100-0xf13f] yenta_cardbus :03:01.0: CardBus bridge to [bus 04] yenta_cardbus :03:01.0: bridge window [io 0x5000-0x50ff] yenta_cardbus :03:01.0: bridge window [io 0x5400-0x54ff] yenta_cardbus :03:01.0: bridge window [mem 0xf0c0-0xf0ff] yenta_cardbus :03:01.0: bridge window [mem 0xf100-0xf13f] I'm really puzzled by these messages, I don't see how the lid is in any way related to this subsystem. After that the graphics output is corrupted, but no further kernel messages produced. However after switching from graphics to text mode and back (and only after having switched back) the following messages are logged; the backtrace is logged exactly twice! [ cut here ] cursor A assertion failure (expected off, current on) WARNING: CPU: 1 PID: 423 at drivers/gpu/drm/i915/intel_display.c:1247 assert_plane+0x90/0xa0 [i915] Modules linked in: 8021q garp mrp stp llc ext2 joydev i915 dell_smbios_wmi dell_smm_hwmon dell_wmi i2c_algo_bit iTCO_wdt gpio_ich iTCO_vendor_support sparse_keymap wmi_bmof dell_rbtn dell_wmi_descriptor uvcvideo videobuf2_vmalloc videobuf2_memops drm_kms_helper dell_laptop dell_smbios_smm dell_smbios dcdbas videobuf2_v4l2 coretemp hwmon videobuf2_core input_leds psmouse evdev pcspkr mac_hid videodev yenta_socket usbkbd media usbmouse pcmcia_rsrc i2c_i801 drm pcmcia_core lpc_ich snd_hda_codec_hdmi intel_agp intel_gtt shpchp agpgart syscopyarea sysfillrect sysimgblt fb_sys_fops snd_hda_intel wmi thermal button battery video ac acpi_cpufreq kvm_intel kvm irqbypass snd_hda_codec_idt snd_hda_codec_generic snd_hda_codec snd_hda_core snd_hwdep snd_pcm ctr ccm arc4 iwldvm iwlwifi mac80211 cfg80211 snd_seq snd_seq_device snd_timer snd soundcore vhost_vsock vmw_vsock_virtio_transport_common vsock vhost_net vhost tap uhid hci_vhci bluetooth ecdh_generic rfkill vfio_iommu_type1 vfio dm_mod uinput userio ppp_generic slhc tun loop crc32c_generic btrfs xor zstd_compress raid6_pq zstd_decompress xxhash cuse fuse ext4 crc16 mbcache jbd2 sd_mod sr_mod cdrom hid_generic usbhid hid uhci_hcd ahci serio_raw libahci sdhci_pci libata firewire_ohci ehci_pci sdhci ehci_hcd scsi_mod firewire_core mmc_core crc_itu_t usbcore CPU: 1 PID: 423 Comm: Xorg Not tainted 4.15.12_1 #1 Hardware name: Dell Inc. Latitude E6400 /0W620R, BIOS A25 06/04/2010 RIP: 0010:assert_plane+0x90/0xa0 [i915] RSP: 0018:b092c124f7a8 EFLAGS: 00010282 RAX: RBX: 9
Re: [PATCH 1/1] drm/i915: Do not use kfree() to free kmem_cache_alloc() return value
Quoting Xidong Wang (2018-04-04 08:37:54) > In eb_lookup_vmas(), the return value of kmem_cache_alloc() is freed > with kfree(). I think the expected paired function is kmem_cahce_free(). > > Signed-off-by: Xidong Wang Thank you for the fix; applied. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105733] Amdgpu randomly hangs and only ssh works. Mouse cursor moves sometimes but does nothing. Keyboard stops working.
https://bugs.freedesktop.org/show_bug.cgi?id=105733 --- Comment #7 from bernhardu --- Just a note, that this might be a similar issue as my #104345: - both using Ryzen CPUs, - amdgpu kernel module with Polaris GPUs, - with unkillable processes - "GPU fault detected" messages followed by "task ... blocked for more than 120 seconds" -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm/i915: Do NOT skip the first 4k of stolen memory for pre-allocated buffers
On Wed, Apr 04, 2018 at 10:06:29PM +0200, Hans de Goede wrote:
> Hi,
>
> On 04-04-18 17:50, Ville Syrjälä wrote:
> > On Fri, Mar 30, 2018 at 04:26:53PM +0200, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 30-03-18 15:25, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 30-03-18 14:44, Chris Wilson wrote:
> Quoting Hans de Goede (2018-03-30 13:37:40)
> > Hi,
> >
> > On 30-03-18 14:30, Chris Wilson wrote:
> >> Quoting Hans de Goede (2018-03-30 13:27:15)
> >>> Before this commit the WaSkipStolenMemoryFirstPage workaround code was
> >>> skipping the first 4k by passing 4096 as start of the address range
> >>> passed
> >>> to drm_mm_init(). This means that calling drm_mm_reserve_node() to
> >>> try and
> >>> reserve the firmware framebuffer so that we can inherit it would
> >>> always
> >>> fail, as the firmware framebuffer starts at address 0.
> >>>
> >>> Commit d43537610470 ("drm/i915: skip the first 4k of stolen memory on
> >>> everything >= gen8") says in its commit message: "This is confirmed
> >>> to fix
> >>> Skylake screen flickering issues (probably caused by the fact that we
> >>> initialized a ring in the first page of stolen, but I didn't 100%
> >>> confirm
> >>> this theory)."
> >>>
> >>> Which suggests that it is safe to use the first page for a linear
> >>> framebuffer as the firmware is doing.
> >>>
> >>> This commit always passes 0 as start to drm_mm_init() and works around
> >>> WaSkipStolenMemoryFirstPage in i915_gem_stolen_insert_node_in_range()
> >>> by insuring the start address passed by to
> >>> drm_mm_insert_node_in_range()
> >>> is always 4k or more. All entry points to i915_gem_stolen.c go through
> >>> i915_gem_stolen_insert_node_in_range(), so that any newly allocated
> >>> objects such as ring-buffers will not be allocated in the first 4k.
> >>>
> >>> The one exception is i915_gem_object_create_stolen_for_preallocated()
> >>> which directly calls drm_mm_reserve_node() which now will be able to
> >>> use the first 4k.
> >>>
> >>> This fixes the i915 driver no longer being able to inherit the
> >>> firmware
> >>> framebuffer on gen8+, which fixes the video output changing from the
> >>> vendor logo to a black screen as soon as the i915 driver is loaded
> >>> (on systems without fbcon).
> >>
> >> We've been told by the HW guys not to use the first page. (That's my
> >> understanding from every time this gets questioned.)
> >
> > Yet the GOP is happily using the first page. I think we may need to make
> > a difference here between the GPU not using the first page and the
> > display engine/pipeline not using the first page. Note that my patch
> > only influences the inheriting of the initial framebuffer as allocated
> > by the GOP. It does not influence any other allocations from the
> > reserved range, those will still all avoid the first page.
> >
> > Without this patch fastboot / flickerfree support is essentially broken
> > on any gen8+ hardware given that one of the goals of atomic is to be
> > able to do flickerfree transitions I think that this warrants a closer
> > look then just simply saying never use the first page.
>
> The concern is what else (i.e. nothing that we allocated ourselves) that
> may be in the first page...
> >>>
> >>> Given that the GOP has put its framebuffer there at least at boot there
> >>> is nothing there, otherwise it would show up on the display.
> >>>
> >>> We have a whole bunch of code to inherit the BIOS fb in intel_display.c
> >>> and AFAIK that code is there because this inheriting the BIOS fb is
> >>> deemed an important feature. So I'm not happy at all with the handwavy
> >>> best to not touch it answer I'm getting to this patch.
> >>>
> >>> Unless there are some clear answer from the hardware folks which
> >>> specifically
> >>> say we cannot put a framebuffer there (and then why is the GOP doing it?)
> >>> then I believe that the best way forward here is to get various people to
> >>> test with this patch and the best way to do that is probably to put it
> >>> in next. Note I deliberately did not add a Cc stable.
> >>
> >> To elaborate on this, the excluding of the first 4k of the stolen memory
> >> region causes intel_alloc_initial_plane_obj() from intel_display.c to fail,
> >> which in turn causes intel_find_initial_plane_obj() to call
> >> intel_plane_disable_noatomic(intel_crtc, intel_plane); which temporarily
> >> completely turns off the display which leads to a very ugly flickering
> >> of the display at boot (as well as replacing the vendor logo with a
> >> black screen).
> >>
> >> I think we can all agree that this behavior is undesirable and even a
> >> regression in behavior caused by the fix to exclude the first 4k.
> >>
> >> Chris, if my patch is not an acceptable way to fix this, then ho
[Bug 105680] [CI] igt@kms_frontbuffer_tracking@* - fail - FBC disabled: mode too large for compression
https://bugs.freedesktop.org/show_bug.cgi?id=105680 --- Comment #6 from Jose Roberto de Souza --- Done: https://patchwork.freedesktop.org/series/41156/ -- 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 1/1] drm/i915: Keep AUX block running when disabling DPMS for MST
While enabling/disabling DPMS before link training with MST hubs is
perfectly valid; unfortunately disabling DPMS results in some devices
disabling their AUX CH block as well. For SST this isn't as much of a
problem, but for MST we need to be able to continue handling aux
transactions even when none of the sinks are turned on since it's
possible for us to have a single atomic commit which results in
disabling each downstream sink, followed by subsequently re-enabling
each sink.
If we don't do this, we'll end up stalling any pending ESI interrupts
from the sink for up to 1ms. Unfortunately, dropping ESIs during this
timespan makes it so that link fallback retraining for MST (which I will
be submitting to the ML shortly) fails due to the channel EQ failure
interrupts potentially getting dropped. Additionally, when performing a
modeset that brings the hub status's link status from bad -> good having
ESIs disabled for that long causes us to miss the hub's response to us
trying to start link training as well.
Since any sink with MST is going to support DisplayPort 1.2 anyway, save
us the hassle of trying to wait until the sink comes back up and just
never shut the aux block down.
Changes since v2:
- Fix patch name, no functional changes
Changes since v3:
- acks, t-b's
Signed-off-by: Lyude Paul
Acked-by: Dhinakaran Pandiyan
Tested-by: Laura Abbott
Cc: Ville Syrjälä
Cc: sta...@vger.kernel.org
Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.")
---
drivers/gpu/drm/i915/intel_dp.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 62f82c4298ac..0479c377981b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int
mode)
return;
if (mode != DRM_MODE_DPMS_ON) {
+ unsigned char data = intel_dp->is_mst ?
+ DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
+
if (downstream_hpd_needs_d0(intel_dp))
return;
- ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
-DP_SET_POWER_D3);
+ ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, data);
} else {
struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
--
2.14.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/1] Keep AUX on while disabling DPMS for MST
Resend that was required anyway since I missed some t-b's and some acks. Trying to figure out what patchwork is doing that's making this patch not get noticed. Maybe it's the lack of cover letters on the previous versions of this Lyude Paul (1): drm/i915: Keep AUX block running when disabling DPMS for MST drivers/gpu/drm/i915/intel_dp.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH v2 0/2] Remove DPU RSC support
On Wed, Apr 04, 2018 at 02:34:40PM +0530, Rajesh Yadav wrote: > MSM display controller hardware (DPU) has an inbuilt RSC block > which can control power resources and bus bandwidth voting > based on frame timing parameters w/o DPU driver intervention. > In absence of RSC HW, DPU driver controls these resources. > > Downstream driver relies on RSC driver for controlling these > resources (via RSC HW block) for better power benefits. > > Since, DPU driver can control these resources, removing RSC > driver support. Corresponding devicetree binding are also removed. > > Details for DPU driver upstreaming: > https://lists.freedesktop.org/archives/freedreno/2018-February/001678.html > > Changes in v2: > - Remove last reference to dpu_power_rsc_update > - Add DPU PATCH tag for better filtering > - Rebase on tip of for-next-staging Hi Rajesh, Unrelated to this change, but I've noticed the threading seems off on the patch sets you're sending. Are you sending the emails one-by-one, or specifying --no-thread in git send-email? Sean > > Rajesh Yadav (2): > dt-bindings: msm/disp: Remove DPU RSC device bindings > drm/msm: Remove RSC support from DPU driver > > .../devicetree/bindings/display/msm/dpu-rsc.txt| 96 -- > drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 130 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |6 - > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 14 - > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h |9 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 242 +--- > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h|7 - > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h |1 - > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c | 20 +- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.h |3 - > drivers/gpu/drm/msm/dpu_dbg.c | 27 - > drivers/gpu/drm/msm/dpu_dbg.h | 10 - > drivers/gpu/drm/msm/dpu_power_handle.c | 73 +- > drivers/gpu/drm/msm/dpu_power_handle.h |4 - > drivers/gpu/drm/msm/dpu_rsc.c | 1367 > > drivers/gpu/drm/msm/dpu_rsc_hw.c | 818 > drivers/gpu/drm/msm/dpu_rsc_priv.h | 191 --- > include/linux/dpu_rsc.h| 302 - > 18 files changed, 42 insertions(+), 3278 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/display/msm/dpu-rsc.txt > delete mode 100644 drivers/gpu/drm/msm/dpu_rsc.c > delete mode 100644 drivers/gpu/drm/msm/dpu_rsc_hw.c > delete mode 100644 drivers/gpu/drm/msm/dpu_rsc_priv.h > delete mode 100644 include/linux/dpu_rsc.h > > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2][RESEND] drm/i915: Keep AUX block running when disabling DPMS for MST
While enabling/disabling DPMS before link training with MST hubs is
perfectly valid; unfortunately disabling DPMS results in some devices
disabling their AUX CH block as well. For SST this isn't as much of a
problem, but for MST we need to be able to continue handling aux
transactions even when none of the sinks are turned on since it's
possible for us to have a single atomic commit which results in
disabling each downstream sink, followed by subsequently re-enabling
each sink.
If we don't do this, we'll end up stalling any pending ESI interrupts
from the sink for up to 1ms. Unfortunately, dropping ESIs during this
timespan makes it so that link fallback retraining for MST (which I will
be submitting to the ML shortly) fails due to the channel EQ failure
interrupts potentially getting dropped. Additionally, when performing a
modeset that brings the hub status's link status from bad -> good having
ESIs disabled for that long causes us to miss the hub's response to us
trying to start link training as well.
Since any sink with MST is going to support DisplayPort 1.2 anyway, save
us the hassle of trying to wait until the sink comes back up and just
never shut the aux block down.
Changes since v2:
- Fix patch name, no functional changes
Signed-off-by: Lyude Paul
Cc: Laura Abbott
Cc: Dhinakaran Pandiyan
Cc: Ville Syrjälä
Cc: sta...@vger.kernel.org
Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.")
---
drivers/gpu/drm/i915/intel_dp.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 62f82c4298ac..0479c377981b 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp, int
mode)
return;
if (mode != DRM_MODE_DPMS_ON) {
+ unsigned char data = intel_dp->is_mst ?
+ DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
+
if (downstream_hpd_needs_d0(intel_dp))
return;
- ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
-DP_SET_POWER_D3);
+ ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, data);
} else {
struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
--
2.14.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105869] clang crashes when compiling OpenCL kernel
https://bugs.freedesktop.org/show_bug.cgi?id=105869
--- Comment #1 from Jan Vesely ---
What is the clang/llvm version? I cannot reproduce using clang-5.0.1:
$ cat foo.cl
#pragma OPENCL EXTENSION cl_khr_fp64: enable
kernel void PulseWave(global float* buffer, size_t num_harmonics,
float duty_cycle)
{
size_t index = get_global_id(0);
float phase = buffer[index];
float sample = duty_cycle;
float precompute1 = (phase - duty_cycle / 2.0) * 2.0 * M_PI;
for (size_t i = 1; i <= num_harmonics; ++i)
{
float harmonic = 2.0 / (i * M_PI) * sin(M_PI * i * duty_cycle)
*
cos(i * precompute1);
sample += harmonic;
}
buffer[index] = sample * 2.0 - 1.0;
}
$ clang-5.0 -target amdgcn-mesa-mesa3d -Xclang -mlink-bitcode-file -Xclang
/usr/lib64/clc/carrizo-amdgcn-mesa-mesa3d.bc -include clc/clc.h -S foo.cl
$ clang-5.0 --version
clang version 5.0.1 (tags/RELEASE_501/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
--
You are receiving this mail because:
You are the assignee for the bug.___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, 2018-04-04 at 22:35 +0300, Ville Syrjälä wrote:
> On Wed, Apr 04, 2018 at 03:00:12PM -0400, Lyude Paul wrote:
> > On Wed, 2018-04-04 at 21:53 +0300, Ville Syrjälä wrote:
> > > On Wed, Apr 04, 2018 at 02:37:41PM -0400, Lyude Paul wrote:
> > > > On Wed, 2018-04-04 at 18:34 +0300, Ville Syrjälä wrote:
> > > > > On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > > > > > While enabling/disabling DPMS before link training with MST hubs
> > > > > > is
> > > > > > perfectly valid; unfortunately disabling DPMS results in some
> > > > > > devices
> > > > > > disabling their AUX CH block as well. For SST this isn't as much
> > > > > > of a
> > > > > > problem, but for MST we need to be able to continue handling aux
> > > > > > transactions even when none of the sinks are turned on since it's
> > > > > > possible for us to have a single atomic commit which results in
> > > > > > disabling each downstream sink, followed by subsequently re-
> > > > > > enabling
> > > > > > each sink.
> > > > > >
> > > > > > If we don't do this, we'll end up stalling any pending ESI
> > > > > > interrupts
> > > > > > from the sink for up to 1ms. Unfortunately, dropping ESIs during
> > > > > > this
> > > > > > timespan makes it so that link fallback retraining for MST (which
> > > > > > I
> > > > > > will
> > > > > > be submitting to the ML shortly) fails due to the channel EQ
> > > > > > failure
> > > > > > interrupts potentially getting dropped. Additionally, when
> > > > > > performing
> > > > > > a
> > > > > > modeset that brings the hub status's link status from bad -> good
> > > > > > having
> > > > > > ESIs disabled for that long causes us to miss the hub's response
> > > > > > to us
> > > > > > trying to start link training as well.
> > > > > >
> > > > > > Since any sink with MST is going to support DisplayPort 1.2
> > > > > > anyway,
> > > > > > save
> > > > > > us the hassle of trying to wait until the sink comes back up and
> > > > > > just
> > > > > > never shut the aux block down.
> > > > > >
> > > > > > Changes since v2:
> > > > > > - Fix patch name, no functional changes
> > > > > >
> > > > > > Signed-off-by: Lyude Paul
> > > > > > Cc: Laura Abbott
> > > > > > Cc: Dhinakaran Pandiyan
> > > > > > Cc: Ville Syrjälä
> > > > > > Cc: sta...@vger.kernel.org
> > > > > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to
> > > > > > enable
> > > > > > MST
> > > > > > hub.")
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > > > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > index 62f82c4298ac..0479c377981b 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp
> > > > > > *intel_dp,
> > > > > > int mode)
> > > > > > return;
> > > > > >
> > > > > > if (mode != DRM_MODE_DPMS_ON) {
> > > > > > + unsigned char data = intel_dp->is_mst ?
> > > > > > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
> > > > >
> > > > > This smells like a workaround for an actual bug somewhere. Why
> > > > > exactly
> > > > > is the slower wakeup or the AUX block a problem for MST but not for
> > > > > SST
> > > > > when the link training is exactly the same for SST and MST?
> > > >
> > > > I actually thought about this but I still think this is the
> > > > appropriate
> > > > fix.
> > > > So; the real reason for the wakeup not being a problem with SST is
> > > > that
> > > > for
> > > > DPMS on with SST, we actually do a wait to make sure that the hub is
> > > > ready
> > > > before continuing. And yes: I'm fairly sure SST does actually have
> > > > around
> > > > the
> > > > same wakeup time that MST does, but with the wait we do it doesn't
> > > > reallhy
> > > > make a difference. With MST, we could do this but there's a few
> > > > reasons I
> > > > don't think we should:
> > > > * We don't need to. D3_AUX_ON is a part of the 1.2 spec, so any hub
> > > > that
> > > > has
> > > >MST is going to be guaranteed to have this.
> > > > * Turning off the aux block means that there's a high chance we're
> > > > going
> > > > to
> > > >miss ESIs from sinks
> > >
> > > And how exactly do we lose irqs? The hub/whatever throws the up req msgs
> > > away if we don't read them within some really short time?
> >
> > Oh-additionally I did forget to mention that i have actually witnessed the
> > channel eq failures in the ESI getting dropped without this patch.
>
> Not sure what that means. I don't think there is any sideband messaging
> involved in link training so not sure what is dropped in this case. The
> link status/etc. are just polled directly by the upstream device.
no, no nononono they are not always with MST. mdnavare is right regarding the
channel EQ occasionally being the only in
Re: [Intel-gfx] [PATCH] drm/i915: Do NOT skip the first 4k of stolen memory for pre-allocated buffers
Hi,
On 04-04-18 17:50, Ville Syrjälä wrote:
On Fri, Mar 30, 2018 at 04:26:53PM +0200, Hans de Goede wrote:
Hi,
On 30-03-18 15:25, Hans de Goede wrote:
Hi,
On 30-03-18 14:44, Chris Wilson wrote:
Quoting Hans de Goede (2018-03-30 13:37:40)
Hi,
On 30-03-18 14:30, Chris Wilson wrote:
Quoting Hans de Goede (2018-03-30 13:27:15)
Before this commit the WaSkipStolenMemoryFirstPage workaround code was
skipping the first 4k by passing 4096 as start of the address range passed
to drm_mm_init(). This means that calling drm_mm_reserve_node() to try and
reserve the firmware framebuffer so that we can inherit it would always
fail, as the firmware framebuffer starts at address 0.
Commit d43537610470 ("drm/i915: skip the first 4k of stolen memory on
everything >= gen8") says in its commit message: "This is confirmed to fix
Skylake screen flickering issues (probably caused by the fact that we
initialized a ring in the first page of stolen, but I didn't 100% confirm
this theory)."
Which suggests that it is safe to use the first page for a linear
framebuffer as the firmware is doing.
This commit always passes 0 as start to drm_mm_init() and works around
WaSkipStolenMemoryFirstPage in i915_gem_stolen_insert_node_in_range()
by insuring the start address passed by to drm_mm_insert_node_in_range()
is always 4k or more. All entry points to i915_gem_stolen.c go through
i915_gem_stolen_insert_node_in_range(), so that any newly allocated
objects such as ring-buffers will not be allocated in the first 4k.
The one exception is i915_gem_object_create_stolen_for_preallocated()
which directly calls drm_mm_reserve_node() which now will be able to
use the first 4k.
This fixes the i915 driver no longer being able to inherit the firmware
framebuffer on gen8+, which fixes the video output changing from the
vendor logo to a black screen as soon as the i915 driver is loaded
(on systems without fbcon).
We've been told by the HW guys not to use the first page. (That's my
understanding from every time this gets questioned.)
Yet the GOP is happily using the first page. I think we may need to make
a difference here between the GPU not using the first page and the
display engine/pipeline not using the first page. Note that my patch
only influences the inheriting of the initial framebuffer as allocated
by the GOP. It does not influence any other allocations from the
reserved range, those will still all avoid the first page.
Without this patch fastboot / flickerfree support is essentially broken
on any gen8+ hardware given that one of the goals of atomic is to be
able to do flickerfree transitions I think that this warrants a closer
look then just simply saying never use the first page.
The concern is what else (i.e. nothing that we allocated ourselves) that
may be in the first page...
Given that the GOP has put its framebuffer there at least at boot there
is nothing there, otherwise it would show up on the display.
We have a whole bunch of code to inherit the BIOS fb in intel_display.c
and AFAIK that code is there because this inheriting the BIOS fb is
deemed an important feature. So I'm not happy at all with the handwavy
best to not touch it answer I'm getting to this patch.
Unless there are some clear answer from the hardware folks which specifically
say we cannot put a framebuffer there (and then why is the GOP doing it?)
then I believe that the best way forward here is to get various people to
test with this patch and the best way to do that is probably to put it
in next. Note I deliberately did not add a Cc stable.
To elaborate on this, the excluding of the first 4k of the stolen memory
region causes intel_alloc_initial_plane_obj() from intel_display.c to fail,
which in turn causes intel_find_initial_plane_obj() to call
intel_plane_disable_noatomic(intel_crtc, intel_plane); which temporarily
completely turns off the display which leads to a very ugly flickering
of the display at boot (as well as replacing the vendor logo with a
black screen).
I think we can all agree that this behavior is undesirable and even a
regression in behavior caused by the fix to exclude the first 4k.
Chris, if my patch is not an acceptable way to fix this, then how do you
suggest that we fix this?
Digging a bit deeper I found this:
https://01.org/sites/default/files/documentation/intel-gfx-prm-osrc-kbl-vol16-workarounds.pdf
Which says:
"WaSkipStolenMemoryFirstPage:
WA to skip the first page of stolen
memory due to sporadic HW write on *CS Idle"
And also about FBC:
"First line of FBC getting corrupted when FBC
compressed frame buffer offset is programmed to
zero. Command streamers are doing flush writes to
base of stolen.
WA: New restriction to program FBC compressed
frame buffer offset to at least 4KB."
So using the first 4kB for the *framebuffer* as done by the GOP will
not cause any major problems (freezes, hangs, etc.), and commit
d43537610470 ("drm/i915: skip the first 4k of stolen memory on
everything >= gen8") was correct in d
Re: [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, 2018-04-04 at 22:31 +0300, Ville Syrjälä wrote:
> On Wed, Apr 04, 2018 at 02:59:09PM -0400, Lyude Paul wrote:
> > On Wed, 2018-04-04 at 21:53 +0300, Ville Syrjälä wrote:
> > > On Wed, Apr 04, 2018 at 02:37:41PM -0400, Lyude Paul wrote:
> > > > On Wed, 2018-04-04 at 18:34 +0300, Ville Syrjälä wrote:
> > > > > On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > > > > > While enabling/disabling DPMS before link training with MST hubs
> > > > > > is
> > > > > > perfectly valid; unfortunately disabling DPMS results in some
> > > > > > devices
> > > > > > disabling their AUX CH block as well. For SST this isn't as much
> > > > > > of a
> > > > > > problem, but for MST we need to be able to continue handling aux
> > > > > > transactions even when none of the sinks are turned on since it's
> > > > > > possible for us to have a single atomic commit which results in
> > > > > > disabling each downstream sink, followed by subsequently re-
> > > > > > enabling
> > > > > > each sink.
> > > > > >
> > > > > > If we don't do this, we'll end up stalling any pending ESI
> > > > > > interrupts
> > > > > > from the sink for up to 1ms. Unfortunately, dropping ESIs during
> > > > > > this
> > > > > > timespan makes it so that link fallback retraining for MST (which
> > > > > > I
> > > > > > will
> > > > > > be submitting to the ML shortly) fails due to the channel EQ
> > > > > > failure
> > > > > > interrupts potentially getting dropped. Additionally, when
> > > > > > performing
> > > > > > a
> > > > > > modeset that brings the hub status's link status from bad -> good
> > > > > > having
> > > > > > ESIs disabled for that long causes us to miss the hub's response
> > > > > > to us
> > > > > > trying to start link training as well.
> > > > > >
> > > > > > Since any sink with MST is going to support DisplayPort 1.2
> > > > > > anyway,
> > > > > > save
> > > > > > us the hassle of trying to wait until the sink comes back up and
> > > > > > just
> > > > > > never shut the aux block down.
> > > > > >
> > > > > > Changes since v2:
> > > > > > - Fix patch name, no functional changes
> > > > > >
> > > > > > Signed-off-by: Lyude Paul
> > > > > > Cc: Laura Abbott
> > > > > > Cc: Dhinakaran Pandiyan
> > > > > > Cc: Ville Syrjälä
> > > > > > Cc: sta...@vger.kernel.org
> > > > > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to
> > > > > > enable
> > > > > > MST
> > > > > > hub.")
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > > > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > index 62f82c4298ac..0479c377981b 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp
> > > > > > *intel_dp,
> > > > > > int mode)
> > > > > > return;
> > > > > >
> > > > > > if (mode != DRM_MODE_DPMS_ON) {
> > > > > > + unsigned char data = intel_dp->is_mst ?
> > > > > > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
> > > > >
> > > > > This smells like a workaround for an actual bug somewhere. Why
> > > > > exactly
> > > > > is the slower wakeup or the AUX block a problem for MST but not for
> > > > > SST
> > > > > when the link training is exactly the same for SST and MST?
> > > >
> > > > I actually thought about this but I still think this is the
> > > > appropriate
> > > > fix.
> > > > So; the real reason for the wakeup not being a problem with SST is
> > > > that
> > > > for
> > > > DPMS on with SST, we actually do a wait to make sure that the hub is
> > > > ready
> > > > before continuing. And yes: I'm fairly sure SST does actually have
> > > > around
> > > > the
> > > > same wakeup time that MST does, but with the wait we do it doesn't
> > > > reallhy
> > > > make a difference. With MST, we could do this but there's a few
> > > > reasons I
> > > > don't think we should:
> > > > * We don't need to. D3_AUX_ON is a part of the 1.2 spec, so any hub
> > > > that
> > > > has
> > > >MST is going to be guaranteed to have this.
> > > > * Turning off the aux block means that there's a high chance we're
> > > > going
> > > > to
> > > >miss ESIs from sinks
> > >
> > > And how exactly do we lose irqs? The hub/whatever throws the up req msgs
> > > away if we don't read them within some really short time?
> >
> > That's my hypothesis at least. I'm betting that on the fact that when I
> > was
> > implementing MST retraining before we put the intel_dp_check_mst_status()
> > (or
> > whatever it's called) into the dig workqueue, getting the sink to go down
> > and
> > come back up was a lot more unreliable whenever I introduced anything that
> > would block the esi handler for longer then a very brief period of time
> > (say,
> > 50-100ms?). I've seen some notes elsewhere too that seemed to i
Re: [PATCH libdrm] etnaviv: remove never-implemented etna_bo_from_handle()
2018-04-04 17:33 GMT+02:00 Eric Engestrom : > Entrypoint has been listed since the etnaviv support was added, but was > never implemented, and therefore never exported either. > > Fixes: 95e2cc6a801d92a0634b "libdrm: add etnaviv drm support" > Cc: Christian Gmeiner > Cc: Lucas Stach > Signed-off-by: Eric Engestrom Reviewed-by: Christian Gmeiner > --- > Christian, Lucas: if you prefer, you could also implement it (please cc me), > but since it's never been implemented, I expect no driver needs it. > --- > etnaviv/etnaviv-symbol-check | 1 - > etnaviv/etnaviv_drmif.h | 2 -- > 2 files changed, 3 deletions(-) > > diff --git a/etnaviv/etnaviv-symbol-check b/etnaviv/etnaviv-symbol-check > index bc50961590870f4cf3d4..b1598ce52838c847096a 100755 > --- a/etnaviv/etnaviv-symbol-check > +++ b/etnaviv/etnaviv-symbol-check > @@ -23,7 +23,6 @@ etna_pipe_del > etna_pipe_wait > etna_pipe_wait_ns > etna_bo_new > -etna_bo_from_handle > etna_bo_from_name > etna_bo_from_dmabuf > etna_bo_ref > diff --git a/etnaviv/etnaviv_drmif.h b/etnaviv/etnaviv_drmif.h > index 5a6bef8d12d1bce40fb4..80aedc1cfc971afcae98 100644 > --- a/etnaviv/etnaviv_drmif.h > +++ b/etnaviv/etnaviv_drmif.h > @@ -115,8 +115,6 @@ int etna_pipe_wait_ns(struct etna_pipe *pipe, uint32_t > timestamp, uint64_t ns); > > struct etna_bo *etna_bo_new(struct etna_device *dev, > uint32_t size, uint32_t flags); > -struct etna_bo *etna_bo_from_handle(struct etna_device *dev, > - uint32_t handle, uint32_t size); > struct etna_bo *etna_bo_from_name(struct etna_device *dev, uint32_t name); > struct etna_bo *etna_bo_from_dmabuf(struct etna_device *dev, int fd); > struct etna_bo *etna_bo_ref(struct etna_bo *bo); > -- > Cheers, > Eric > -- greets -- Christian Gmeiner, MSc https://christian-gmeiner.info ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, Apr 04, 2018 at 03:00:12PM -0400, Lyude Paul wrote:
> On Wed, 2018-04-04 at 21:53 +0300, Ville Syrjälä wrote:
> > On Wed, Apr 04, 2018 at 02:37:41PM -0400, Lyude Paul wrote:
> > > On Wed, 2018-04-04 at 18:34 +0300, Ville Syrjälä wrote:
> > > > On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > > > > While enabling/disabling DPMS before link training with MST hubs is
> > > > > perfectly valid; unfortunately disabling DPMS results in some devices
> > > > > disabling their AUX CH block as well. For SST this isn't as much of a
> > > > > problem, but for MST we need to be able to continue handling aux
> > > > > transactions even when none of the sinks are turned on since it's
> > > > > possible for us to have a single atomic commit which results in
> > > > > disabling each downstream sink, followed by subsequently re-enabling
> > > > > each sink.
> > > > >
> > > > > If we don't do this, we'll end up stalling any pending ESI interrupts
> > > > > from the sink for up to 1ms. Unfortunately, dropping ESIs during this
> > > > > timespan makes it so that link fallback retraining for MST (which I
> > > > > will
> > > > > be submitting to the ML shortly) fails due to the channel EQ failure
> > > > > interrupts potentially getting dropped. Additionally, when performing
> > > > > a
> > > > > modeset that brings the hub status's link status from bad -> good
> > > > > having
> > > > > ESIs disabled for that long causes us to miss the hub's response to us
> > > > > trying to start link training as well.
> > > > >
> > > > > Since any sink with MST is going to support DisplayPort 1.2 anyway,
> > > > > save
> > > > > us the hassle of trying to wait until the sink comes back up and just
> > > > > never shut the aux block down.
> > > > >
> > > > > Changes since v2:
> > > > > - Fix patch name, no functional changes
> > > > >
> > > > > Signed-off-by: Lyude Paul
> > > > > Cc: Laura Abbott
> > > > > Cc: Dhinakaran Pandiyan
> > > > > Cc: Ville Syrjälä
> > > > > Cc: sta...@vger.kernel.org
> > > > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable
> > > > > MST
> > > > > hub.")
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index 62f82c4298ac..0479c377981b 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp
> > > > > *intel_dp,
> > > > > int mode)
> > > > > return;
> > > > >
> > > > > if (mode != DRM_MODE_DPMS_ON) {
> > > > > + unsigned char data = intel_dp->is_mst ?
> > > > > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
> > > >
> > > > This smells like a workaround for an actual bug somewhere. Why exactly
> > > > is the slower wakeup or the AUX block a problem for MST but not for SST
> > > > when the link training is exactly the same for SST and MST?
> > >
> > > I actually thought about this but I still think this is the appropriate
> > > fix.
> > > So; the real reason for the wakeup not being a problem with SST is that
> > > for
> > > DPMS on with SST, we actually do a wait to make sure that the hub is ready
> > > before continuing. And yes: I'm fairly sure SST does actually have around
> > > the
> > > same wakeup time that MST does, but with the wait we do it doesn't reallhy
> > > make a difference. With MST, we could do this but there's a few reasons I
> > > don't think we should:
> > > * We don't need to. D3_AUX_ON is a part of the 1.2 spec, so any hub that
> > > has
> > >MST is going to be guaranteed to have this.
> > > * Turning off the aux block means that there's a high chance we're going
> > > to
> > >miss ESIs from sinks
> >
> > And how exactly do we lose irqs? The hub/whatever throws the up req msgs
> > away if we don't read them within some really short time?
> Oh-additionally I did forget to mention that i have actually witnessed the
> channel eq failures in the ESI getting dropped without this patch.
Not sure what that means. I don't think there is any sideband messaging
involved in link training so not sure what is dropped in this case. The
link status/etc. are just polled directly by the upstream device.
> Meaning if
> we miss them, there's a chance the hub may just not choose to send them again
> for whatever reason.
> >
> > > * It's faster to keep the aux block on anyway
> > >
> > >
> > > >
> > > > > +
> > > > > if (downstream_hpd_needs_d0(intel_dp))
> > > > > return;
> > > > >
> > > > > - ret = drm_dp_dpcd_writeb(&intel_dp->aux,
> > > > > DP_SET_POWER,
> > > > > - DP_SET_POWER_D3);
> > > > > + ret = drm_dp_dpcd_writeb(&intel_dp->aux,
> > > > > DP_SET_POWER,
> > > > > data);
> >
Re: [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, Apr 04, 2018 at 02:59:09PM -0400, Lyude Paul wrote:
> On Wed, 2018-04-04 at 21:53 +0300, Ville Syrjälä wrote:
> > On Wed, Apr 04, 2018 at 02:37:41PM -0400, Lyude Paul wrote:
> > > On Wed, 2018-04-04 at 18:34 +0300, Ville Syrjälä wrote:
> > > > On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > > > > While enabling/disabling DPMS before link training with MST hubs is
> > > > > perfectly valid; unfortunately disabling DPMS results in some devices
> > > > > disabling their AUX CH block as well. For SST this isn't as much of a
> > > > > problem, but for MST we need to be able to continue handling aux
> > > > > transactions even when none of the sinks are turned on since it's
> > > > > possible for us to have a single atomic commit which results in
> > > > > disabling each downstream sink, followed by subsequently re-enabling
> > > > > each sink.
> > > > >
> > > > > If we don't do this, we'll end up stalling any pending ESI interrupts
> > > > > from the sink for up to 1ms. Unfortunately, dropping ESIs during this
> > > > > timespan makes it so that link fallback retraining for MST (which I
> > > > > will
> > > > > be submitting to the ML shortly) fails due to the channel EQ failure
> > > > > interrupts potentially getting dropped. Additionally, when performing
> > > > > a
> > > > > modeset that brings the hub status's link status from bad -> good
> > > > > having
> > > > > ESIs disabled for that long causes us to miss the hub's response to us
> > > > > trying to start link training as well.
> > > > >
> > > > > Since any sink with MST is going to support DisplayPort 1.2 anyway,
> > > > > save
> > > > > us the hassle of trying to wait until the sink comes back up and just
> > > > > never shut the aux block down.
> > > > >
> > > > > Changes since v2:
> > > > > - Fix patch name, no functional changes
> > > > >
> > > > > Signed-off-by: Lyude Paul
> > > > > Cc: Laura Abbott
> > > > > Cc: Dhinakaran Pandiyan
> > > > > Cc: Ville Syrjälä
> > > > > Cc: sta...@vger.kernel.org
> > > > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable
> > > > > MST
> > > > > hub.")
> > > > > ---
> > > > > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > index 62f82c4298ac..0479c377981b 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp
> > > > > *intel_dp,
> > > > > int mode)
> > > > > return;
> > > > >
> > > > > if (mode != DRM_MODE_DPMS_ON) {
> > > > > + unsigned char data = intel_dp->is_mst ?
> > > > > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
> > > >
> > > > This smells like a workaround for an actual bug somewhere. Why exactly
> > > > is the slower wakeup or the AUX block a problem for MST but not for SST
> > > > when the link training is exactly the same for SST and MST?
> > >
> > > I actually thought about this but I still think this is the appropriate
> > > fix.
> > > So; the real reason for the wakeup not being a problem with SST is that
> > > for
> > > DPMS on with SST, we actually do a wait to make sure that the hub is ready
> > > before continuing. And yes: I'm fairly sure SST does actually have around
> > > the
> > > same wakeup time that MST does, but with the wait we do it doesn't reallhy
> > > make a difference. With MST, we could do this but there's a few reasons I
> > > don't think we should:
> > > * We don't need to. D3_AUX_ON is a part of the 1.2 spec, so any hub that
> > > has
> > >MST is going to be guaranteed to have this.
> > > * Turning off the aux block means that there's a high chance we're going
> > > to
> > >miss ESIs from sinks
> >
> > And how exactly do we lose irqs? The hub/whatever throws the up req msgs
> > away if we don't read them within some really short time?
> That's my hypothesis at least. I'm betting that on the fact that when I was
> implementing MST retraining before we put the intel_dp_check_mst_status() (or
> whatever it's called) into the dig workqueue, getting the sink to go down and
> come back up was a lot more unreliable whenever I introduced anything that
> would block the esi handler for longer then a very brief period of time (say,
> 50-100ms?). I've seen some notes elsewhere too that seemed to imply for SST,
> things were pretty sensitive to irq latency (line 1050, intel_dp.c) so it
> wouldn't be terribly surprising if it's the same for MST. At the very least,
> now that we've got the ESI handler running in the dig worker things seem to
> have gotten a /lot/ more reliable now that we can basically go the whole
> modeset without blocking the ESI handler for very long.
Hmm. OK, so the spec seems to be saying that we have 100ms to read
the UP_REQ/DOWN_REPLY msg after the IRQ_
Re: [PATCH v4] drm: Fix downstream dev count read
On Wed, Apr 04, 2018 at 12:07:41PM -0700, Rodrigo Vivi wrote: > On Wed, Apr 04, 2018 at 11:57:42PM +0530, Ramalingam C wrote: > > In both HDMI and DP, device count is represented by 6:0 bits of a > > register(BInfo/Bstatus) > > > > So macro for bitmasking the device_count is fixed(0x3F->0x7F). > > > > v3: > > Retained the Rb-ed. > > v4: > > %s/drm\/i915/drm [rodrigo] > > > > Shouldn't this patch have a "Fixes:" ? Yes, I think that'd be good. > cc: stable? It couldn't hurt. > > I pushed first 3 patches on the series to dinq. > I believe this one here could be there with Dave's ack or > maybe on drm-misc-fixes? Meh. The severity of this isn't too big, given that I doubt people care _too_ much about plugging in more than 64 HDCP-enabled devices. If you want to drop it in -misc-next-fixes, I can send it out next week. While we're asking for a respin, could we add HDCP somewhere in the subject? Sean > > > Signed-off-by: Ramalingam C > > cc: Sean Paul > > Reviewed-by: Sean Paul > > --- > > include/drm/drm_hdcp.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h > > index 562fa7df2637..98e63d870139 100644 > > --- a/include/drm/drm_hdcp.h > > +++ b/include/drm/drm_hdcp.h > > @@ -19,7 +19,7 @@ > > #define DRM_HDCP_RI_LEN2 > > #define DRM_HDCP_V_PRIME_PART_LEN 4 > > #define DRM_HDCP_V_PRIME_NUM_PARTS 5 > > -#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x3f) > > +#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x7f) > > #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x) (x & BIT(3)) > > #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x)(x & BIT(7)) > > > > -- > > 2.7.4 > > -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] drm/atomic+msm: add helper to implement legacy dirtyfb
On Wed, Apr 4, 2018 at 1:41 PM, Noralf Trønnes wrote:
>
>
> Den 04.04.2018 00.42, skrev Rob Clark:
>>
>> Add an atomic helper to implement dirtyfb support. This is needed to
>> support DSI command-mode panels with x11 userspace (ie. when we can't
>> rely on pageflips to trigger a flush to the panel).
>>
>> To signal to the driver that the async atomic update needs to
>> synchronize with fences, even though the fb didn't change, the
>> drm_atomic_state::dirty flag is added.
>>
>> Signed-off-by: Rob Clark
>> ---
>> Background: there are a number of different folks working on getting
>> upstream kernel working on various different phones/tablets with qcom
>> SoC's.. many of them have command mode panels, so we kind of need a
>> way to support the legacy dirtyfb ioctl for x11 support.
>>
>> I know there is work on a proprer non-legacy atomic property for
>> userspace to communicate dirty-rect(s) to the kernel, so this can
>> be improved from triggering a full-frame flush once that is in
>> place. But we kinda needa a stop-gap solution.
>>
>> I had considered an in-driver solution for this, but things get a
>> bit tricky if userspace ands up combining dirtyfb ioctls with page-
>> flips, because we need to synchronize setting various CTL.FLUSH bits
>> with setting the CTL.START bit. (ie. really all we need to do for
>> cmd mode panels is bang CTL.START, but is this ends up racing with
>> pageflips setting FLUSH bits, then bad things.) The easiest soln
>> is to wrap this up as an atomic commit and rely on the worker to
>> serialize things. Hence adding an atomic dirtyfb helper.
>>
>> I guess at least the helper, with some small addition to translate
>> and pass-thru the dirty rect(s) is useful to the final atomic dirty-
>> rect property solution. Depending on how far off that is, a stop-
>> gap solution could be useful.
>
>
> I have been waiting for someone to address this since I'm not skilled
> enough myself to tackle the atomic machinery. It would be be nice to do
> all flushing during commit since that would mean that the tinydrm drivers
> could be driven solely through drm_simple_display_pipe_funcs. I wouldn't
> have to wire through the dirty callback like I do now.
>
> I see that you use a nonblocking commit. What happens if a new dirtyfb
> comes in before the previous is done?
I'm relying on the workqueue for committing the async part of
non-blocking commits to serialize things. This was actually something
I pretty badly need for msm (otherwise pageflip + dirtyfb causes races
for settings various FLUSH/START bits which need to happen in a
certain order)
This was what killed my earlier lazy plan of handling dirtyfb in drm/msm ;-)
> If we could save the dirty clips, then I could use this in tinydrm.
> A full flush over SPI takes ~50ms so I need to shave off where I can.
>
> This works for me in my tinydrm world:
>
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index c64225274807..218cb36757fa 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -28,6 +28,7 @@
> #include
> #include
>
> +struct drm_clip_rect;
> struct drm_crtc;
> struct drm_printer;
> struct drm_modeset_acquire_ctx;
> @@ -85,6 +86,9 @@ struct drm_plane_state {
> */
> bool dirty;
>
> + struct drm_clip_rect *dirty_clips;
> + unsigned int num_dirty_clips;
> +
> /**
> * @fence:
> *
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 8066c508ab50..e00b8247b7c5 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3521,6 +3521,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct
> drm_plane *plane,
> drm_framebuffer_get(state->fb);
>
> state->dirty = false;
> + state->dirty_clips = NULL;
> + state->num_dirty_clips = 0;
> state->fence = NULL;
> state->commit = NULL;
> }
> @@ -3567,6 +3569,8 @@ void __drm_atomic_helper_plane_destroy_state(struct
> drm_plane_state *state)
>
> if (state->commit)
> drm_crtc_commit_put(state->commit);
> +
> + kfree(state->dirty_clips);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
> @@ -3877,8 +3881,20 @@ int drm_atomic_helper_dirtyfb(struct drm_framebuffer
> *fb,
> struct drm_modeset_acquire_ctx ctx;
> struct drm_atomic_state *state;
> struct drm_plane *plane;
> + bool fb_found = false;
> int ret = 0;
>
> + /* fbdev can flush even when we don't want it to */
> + drm_for_each_plane(plane, fb->dev) {
> + if (plane->state->fb == fb) {
> + fb_found = true;
> + break;
> + }
> + }
> +
> + if (!fb_found)
> + return 0;
> +
> /*
> * When called from ioctl, we are interruptable, but not when
> * called internally (ie. defio worker)
> @@ -3907,6 +
[PULL] drm-misc-next-fixes
Hi Dave, After fixing the dim bug, I have a sane pull request for you this week :). Just one patch again, this time fixing a build warning in hda_intel. Takashi requested this go through drm-misc as the original switcheroo came through here too. drm-misc-next-fixes-2018-04-04: hda_intel: Don't declare azx PM ops if VGA_SWITCHEROO configured (Lukas) Cc: Lukas Wunner Cc: Takashi Iwai Cheers, Sean The following changes since commit a01c47737a9ca118ab75c6fd6e75739b824de830: drm: Fix uabi regression by allowing garbage mode->type from userspace (2018-03-23 13:51:12 +0200) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-fixes-2018-04-04 for you to fetch changes up to 8cd1b5bd70cccda6854088825e725a513ec919c6: ALSA: hda - Silence PM ops build warning (2018-03-29 21:47:53 +0200) hda_intel: Don't declare azx PM ops if VGA_SWITCHEROO configured (Lukas) Cc: Lukas Wunner Cc: Takashi Iwai Lukas Wunner (1): ALSA: hda - Silence PM ops build warning sound/pci/hda/hda_intel.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm: Fix downstream dev count read
On Wed, Apr 04, 2018 at 11:57:42PM +0530, Ramalingam C wrote: > In both HDMI and DP, device count is represented by 6:0 bits of a > register(BInfo/Bstatus) > > So macro for bitmasking the device_count is fixed(0x3F->0x7F). > > v3: > Retained the Rb-ed. > v4: > %s/drm\/i915/drm [rodrigo] > Shouldn't this patch have a "Fixes:" ? cc: stable? I pushed first 3 patches on the series to dinq. I believe this one here could be there with Dave's ack or maybe on drm-misc-fixes? > Signed-off-by: Ramalingam C > cc: Sean Paul > Reviewed-by: Sean Paul > --- > include/drm/drm_hdcp.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h > index 562fa7df2637..98e63d870139 100644 > --- a/include/drm/drm_hdcp.h > +++ b/include/drm/drm_hdcp.h > @@ -19,7 +19,7 @@ > #define DRM_HDCP_RI_LEN 2 > #define DRM_HDCP_V_PRIME_PART_LEN4 > #define DRM_HDCP_V_PRIME_NUM_PARTS 5 > -#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x3f) > +#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x7f) > #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x) (x & BIT(3)) > #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x) (x & BIT(7)) > > -- > 2.7.4 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, 2018-04-04 at 21:53 +0300, Ville Syrjälä wrote:
> On Wed, Apr 04, 2018 at 02:37:41PM -0400, Lyude Paul wrote:
> > On Wed, 2018-04-04 at 18:34 +0300, Ville Syrjälä wrote:
> > > On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > > > While enabling/disabling DPMS before link training with MST hubs is
> > > > perfectly valid; unfortunately disabling DPMS results in some devices
> > > > disabling their AUX CH block as well. For SST this isn't as much of a
> > > > problem, but for MST we need to be able to continue handling aux
> > > > transactions even when none of the sinks are turned on since it's
> > > > possible for us to have a single atomic commit which results in
> > > > disabling each downstream sink, followed by subsequently re-enabling
> > > > each sink.
> > > >
> > > > If we don't do this, we'll end up stalling any pending ESI interrupts
> > > > from the sink for up to 1ms. Unfortunately, dropping ESIs during this
> > > > timespan makes it so that link fallback retraining for MST (which I
> > > > will
> > > > be submitting to the ML shortly) fails due to the channel EQ failure
> > > > interrupts potentially getting dropped. Additionally, when performing
> > > > a
> > > > modeset that brings the hub status's link status from bad -> good
> > > > having
> > > > ESIs disabled for that long causes us to miss the hub's response to us
> > > > trying to start link training as well.
> > > >
> > > > Since any sink with MST is going to support DisplayPort 1.2 anyway,
> > > > save
> > > > us the hassle of trying to wait until the sink comes back up and just
> > > > never shut the aux block down.
> > > >
> > > > Changes since v2:
> > > > - Fix patch name, no functional changes
> > > >
> > > > Signed-off-by: Lyude Paul
> > > > Cc: Laura Abbott
> > > > Cc: Dhinakaran Pandiyan
> > > > Cc: Ville Syrjälä
> > > > Cc: sta...@vger.kernel.org
> > > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable
> > > > MST
> > > > hub.")
> > > > ---
> > > > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 62f82c4298ac..0479c377981b 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp
> > > > *intel_dp,
> > > > int mode)
> > > > return;
> > > >
> > > > if (mode != DRM_MODE_DPMS_ON) {
> > > > + unsigned char data = intel_dp->is_mst ?
> > > > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
> > >
> > > This smells like a workaround for an actual bug somewhere. Why exactly
> > > is the slower wakeup or the AUX block a problem for MST but not for SST
> > > when the link training is exactly the same for SST and MST?
> >
> > I actually thought about this but I still think this is the appropriate
> > fix.
> > So; the real reason for the wakeup not being a problem with SST is that
> > for
> > DPMS on with SST, we actually do a wait to make sure that the hub is ready
> > before continuing. And yes: I'm fairly sure SST does actually have around
> > the
> > same wakeup time that MST does, but with the wait we do it doesn't reallhy
> > make a difference. With MST, we could do this but there's a few reasons I
> > don't think we should:
> > * We don't need to. D3_AUX_ON is a part of the 1.2 spec, so any hub that
> > has
> >MST is going to be guaranteed to have this.
> > * Turning off the aux block means that there's a high chance we're going
> > to
> >miss ESIs from sinks
>
> And how exactly do we lose irqs? The hub/whatever throws the up req msgs
> away if we don't read them within some really short time?
Oh-additionally I did forget to mention that i have actually witnessed the
channel eq failures in the ESI getting dropped without this patch. Meaning if
we miss them, there's a chance the hub may just not choose to send them again
for whatever reason.
>
> > * It's faster to keep the aux block on anyway
> >
> >
> > >
> > > > +
> > > > if (downstream_hpd_needs_d0(intel_dp))
> > > > return;
> > > >
> > > > - ret = drm_dp_dpcd_writeb(&intel_dp->aux,
> > > > DP_SET_POWER,
> > > > -DP_SET_POWER_D3);
> > > > + ret = drm_dp_dpcd_writeb(&intel_dp->aux,
> > > > DP_SET_POWER,
> > > > data);
> > > > } else {
> > > > struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
> > > >
> > > > --
> > > > 2.14.3
> > >
> > >
> >
> > --
> > Cheers,
> > Lyude Paul
>
>
--
Cheers,
Lyude Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, 2018-04-04 at 21:53 +0300, Ville Syrjälä wrote:
> On Wed, Apr 04, 2018 at 02:37:41PM -0400, Lyude Paul wrote:
> > On Wed, 2018-04-04 at 18:34 +0300, Ville Syrjälä wrote:
> > > On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > > > While enabling/disabling DPMS before link training with MST hubs is
> > > > perfectly valid; unfortunately disabling DPMS results in some devices
> > > > disabling their AUX CH block as well. For SST this isn't as much of a
> > > > problem, but for MST we need to be able to continue handling aux
> > > > transactions even when none of the sinks are turned on since it's
> > > > possible for us to have a single atomic commit which results in
> > > > disabling each downstream sink, followed by subsequently re-enabling
> > > > each sink.
> > > >
> > > > If we don't do this, we'll end up stalling any pending ESI interrupts
> > > > from the sink for up to 1ms. Unfortunately, dropping ESIs during this
> > > > timespan makes it so that link fallback retraining for MST (which I
> > > > will
> > > > be submitting to the ML shortly) fails due to the channel EQ failure
> > > > interrupts potentially getting dropped. Additionally, when performing
> > > > a
> > > > modeset that brings the hub status's link status from bad -> good
> > > > having
> > > > ESIs disabled for that long causes us to miss the hub's response to us
> > > > trying to start link training as well.
> > > >
> > > > Since any sink with MST is going to support DisplayPort 1.2 anyway,
> > > > save
> > > > us the hassle of trying to wait until the sink comes back up and just
> > > > never shut the aux block down.
> > > >
> > > > Changes since v2:
> > > > - Fix patch name, no functional changes
> > > >
> > > > Signed-off-by: Lyude Paul
> > > > Cc: Laura Abbott
> > > > Cc: Dhinakaran Pandiyan
> > > > Cc: Ville Syrjälä
> > > > Cc: sta...@vger.kernel.org
> > > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable
> > > > MST
> > > > hub.")
> > > > ---
> > > > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > index 62f82c4298ac..0479c377981b 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp
> > > > *intel_dp,
> > > > int mode)
> > > > return;
> > > >
> > > > if (mode != DRM_MODE_DPMS_ON) {
> > > > + unsigned char data = intel_dp->is_mst ?
> > > > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
> > >
> > > This smells like a workaround for an actual bug somewhere. Why exactly
> > > is the slower wakeup or the AUX block a problem for MST but not for SST
> > > when the link training is exactly the same for SST and MST?
> >
> > I actually thought about this but I still think this is the appropriate
> > fix.
> > So; the real reason for the wakeup not being a problem with SST is that
> > for
> > DPMS on with SST, we actually do a wait to make sure that the hub is ready
> > before continuing. And yes: I'm fairly sure SST does actually have around
> > the
> > same wakeup time that MST does, but with the wait we do it doesn't reallhy
> > make a difference. With MST, we could do this but there's a few reasons I
> > don't think we should:
> > * We don't need to. D3_AUX_ON is a part of the 1.2 spec, so any hub that
> > has
> >MST is going to be guaranteed to have this.
> > * Turning off the aux block means that there's a high chance we're going
> > to
> >miss ESIs from sinks
>
> And how exactly do we lose irqs? The hub/whatever throws the up req msgs
> away if we don't read them within some really short time?
That's my hypothesis at least. I'm betting that on the fact that when I was
implementing MST retraining before we put the intel_dp_check_mst_status() (or
whatever it's called) into the dig workqueue, getting the sink to go down and
come back up was a lot more unreliable whenever I introduced anything that
would block the esi handler for longer then a very brief period of time (say,
50-100ms?). I've seen some notes elsewhere too that seemed to imply for SST,
things were pretty sensitive to irq latency (line 1050, intel_dp.c) so it
wouldn't be terribly surprising if it's the same for MST. At the very least,
now that we've got the ESI handler running in the dig worker things seem to
have gotten a /lot/ more reliable now that we can basically go the whole
modeset without blocking the ESI handler for very long.
>
> > * It's faster to keep the aux block on anyway
> >
> >
> > >
> > > > +
> > > > if (downstream_hpd_needs_d0(intel_dp))
> > > > return;
> > > >
> > > > - ret = drm_dp_dpcd_writeb(&intel_dp->aux,
> > > > DP_SET_POWER,
> > > > -DP_SET_POWER_D3)
Re: [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, Apr 04, 2018 at 02:37:41PM -0400, Lyude Paul wrote:
> On Wed, 2018-04-04 at 18:34 +0300, Ville Syrjälä wrote:
> > On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > > While enabling/disabling DPMS before link training with MST hubs is
> > > perfectly valid; unfortunately disabling DPMS results in some devices
> > > disabling their AUX CH block as well. For SST this isn't as much of a
> > > problem, but for MST we need to be able to continue handling aux
> > > transactions even when none of the sinks are turned on since it's
> > > possible for us to have a single atomic commit which results in
> > > disabling each downstream sink, followed by subsequently re-enabling
> > > each sink.
> > >
> > > If we don't do this, we'll end up stalling any pending ESI interrupts
> > > from the sink for up to 1ms. Unfortunately, dropping ESIs during this
> > > timespan makes it so that link fallback retraining for MST (which I will
> > > be submitting to the ML shortly) fails due to the channel EQ failure
> > > interrupts potentially getting dropped. Additionally, when performing a
> > > modeset that brings the hub status's link status from bad -> good having
> > > ESIs disabled for that long causes us to miss the hub's response to us
> > > trying to start link training as well.
> > >
> > > Since any sink with MST is going to support DisplayPort 1.2 anyway, save
> > > us the hassle of trying to wait until the sink comes back up and just
> > > never shut the aux block down.
> > >
> > > Changes since v2:
> > > - Fix patch name, no functional changes
> > >
> > > Signed-off-by: Lyude Paul
> > > Cc: Laura Abbott
> > > Cc: Dhinakaran Pandiyan
> > > Cc: Ville Syrjälä
> > > Cc: sta...@vger.kernel.org
> > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST
> > > hub.")
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 62f82c4298ac..0479c377981b 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp,
> > > int mode)
> > > return;
> > >
> > > if (mode != DRM_MODE_DPMS_ON) {
> > > + unsigned char data = intel_dp->is_mst ?
> > > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
> >
> > This smells like a workaround for an actual bug somewhere. Why exactly
> > is the slower wakeup or the AUX block a problem for MST but not for SST
> > when the link training is exactly the same for SST and MST?
> I actually thought about this but I still think this is the appropriate fix.
> So; the real reason for the wakeup not being a problem with SST is that for
> DPMS on with SST, we actually do a wait to make sure that the hub is ready
> before continuing. And yes: I'm fairly sure SST does actually have around the
> same wakeup time that MST does, but with the wait we do it doesn't reallhy
> make a difference. With MST, we could do this but there's a few reasons I
> don't think we should:
> * We don't need to. D3_AUX_ON is a part of the 1.2 spec, so any hub that has
>MST is going to be guaranteed to have this.
> * Turning off the aux block means that there's a high chance we're going to
>miss ESIs from sinks
And how exactly do we lose irqs? The hub/whatever throws the up req msgs
away if we don't read them within some really short time?
> * It's faster to keep the aux block on anyway
>
>
> >
> > > +
> > > if (downstream_hpd_needs_d0(intel_dp))
> > > return;
> > >
> > > - ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > > - DP_SET_POWER_D3);
> > > + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > > data);
> > > } else {
> > > struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
> > >
> > > --
> > > 2.14.3
> >
> >
> --
> Cheers,
> Lyude Paul
--
Ville Syrjälä
Intel OTC
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] drm/atomic+msm: add helper to implement legacy dirtyfb
Den 04.04.2018 19.56, skrev Daniel Vetter: On Wed, Apr 4, 2018 at 7:41 PM, Noralf Trønnes wrote: Den 04.04.2018 00.42, skrev Rob Clark: Add an atomic helper to implement dirtyfb support. This is needed to support DSI command-mode panels with x11 userspace (ie. when we can't rely on pageflips to trigger a flush to the panel). To signal to the driver that the async atomic update needs to synchronize with fences, even though the fb didn't change, the drm_atomic_state::dirty flag is added. Signed-off-by: Rob Clark --- Background: there are a number of different folks working on getting upstream kernel working on various different phones/tablets with qcom SoC's.. many of them have command mode panels, so we kind of need a way to support the legacy dirtyfb ioctl for x11 support. I know there is work on a proprer non-legacy atomic property for userspace to communicate dirty-rect(s) to the kernel, so this can be improved from triggering a full-frame flush once that is in place. But we kinda needa a stop-gap solution. I had considered an in-driver solution for this, but things get a bit tricky if userspace ands up combining dirtyfb ioctls with page- flips, because we need to synchronize setting various CTL.FLUSH bits with setting the CTL.START bit. (ie. really all we need to do for cmd mode panels is bang CTL.START, but is this ends up racing with pageflips setting FLUSH bits, then bad things.) The easiest soln is to wrap this up as an atomic commit and rely on the worker to serialize things. Hence adding an atomic dirtyfb helper. I guess at least the helper, with some small addition to translate and pass-thru the dirty rect(s) is useful to the final atomic dirty- rect property solution. Depending on how far off that is, a stop- gap solution could be useful. I have been waiting for someone to address this since I'm not skilled enough myself to tackle the atomic machinery. It would be be nice to do all flushing during commit since that would mean that the tinydrm drivers could be driven solely through drm_simple_display_pipe_funcs. I wouldn't have to wire through the dirty callback like I do now. I see that you use a nonblocking commit. What happens if a new dirtyfb comes in before the previous is done? We could reuse the same trick we're doing in the fbdev code, of pushing the commit to a worker and doing it in a blocking fashion. Or at least wait for it to finish (can be done with the crtc_state->event stuff). That way userspace can pile us up in dirtyfb calls, but we'd do at most one per frame. More isn't really useful anyway. If we could save the dirty clips, then I could use this in tinydrm. A full flush over SPI takes ~50ms so I need to shave off where I can. This works for me in my tinydrm world: One thing I thought you've had around somewhere is some additional tracking code, so we don't have to take all the plane mutexes in the ->dirtyfb callback. Does that exist, or was that just a figment of my imagination? I haven't looked at this at all since I know way too little about how atomic works and after the discussion started with damage on page flips, I knew I just had to wait for someone other than me to do this. And I hardly know anything about how the multitude of userspace operates. So I'm sorry, but I can't add much to this discussion, I fall off rather quickly when the details and corner cases are discussed. All I can do is state the needs of tinydrm :-) Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, 2018-04-04 at 11:44 -0700, Manasi Navare wrote:
> On Wed, Apr 04, 2018 at 06:34:29PM +0300, Ville Syrjälä wrote:
> > On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > > While enabling/disabling DPMS before link training with MST hubs is
> > > perfectly valid; unfortunately disabling DPMS results in some devices
> > > disabling their AUX CH block as well. For SST this isn't as much of a
> > > problem, but for MST we need to be able to continue handling aux
> > > transactions even when none of the sinks are turned on since it's
> > > possible for us to have a single atomic commit which results in
> > > disabling each downstream sink, followed by subsequently re-enabling
> > > each sink.
> > >
> > > If we don't do this, we'll end up stalling any pending ESI interrupts
> > > from the sink for up to 1ms. Unfortunately, dropping ESIs during this
> > > timespan makes it so that link fallback retraining for MST (which I will
> > > be submitting to the ML shortly) fails due to the channel EQ failure
> > > interrupts potentially getting dropped. Additionally, when performing a
> > > modeset that brings the hub status's link status from bad -> good having
> > > ESIs disabled for that long causes us to miss the hub's response to us
> > > trying to start link training as well.
> > >
> > > Since any sink with MST is going to support DisplayPort 1.2 anyway, save
> > > us the hassle of trying to wait until the sink comes back up and just
> > > never shut the aux block down.
> > >
> > > Changes since v2:
> > > - Fix patch name, no functional changes
> > >
> > > Signed-off-by: Lyude Paul
> > > Cc: Laura Abbott
> > > Cc: Dhinakaran Pandiyan
> > > Cc: Ville Syrjälä
> > > Cc: sta...@vger.kernel.org
> > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable
> > > MST hub.")
> > > ---
> > > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > b/drivers/gpu/drm/i915/intel_dp.c
> > > index 62f82c4298ac..0479c377981b 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp
> > > *intel_dp, int mode)
> > > return;
> > >
> > > if (mode != DRM_MODE_DPMS_ON) {
> > > + unsigned char data = intel_dp->is_mst ?
> > > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
> >
> > This smells like a workaround for an actual bug somewhere. Why exactly
> > is the slower wakeup or the AUX block a problem for MST but not for SST
> > when the link training is exactly the same for SST and MST?
> >
>
> The problem occurs only in case of MST because the Channel EQ failure is
> notified
> through ESI sideband AUX messages which potentially can get dropped because
> of disabling
> DPMS. But in case of SST, we detect the channel EQ failure write during the
> EQ TPS sequence
> which happens on the main link.
mhm- that is the big problem it causes, at least with this patchset. I've been
considering maybe looking into probing downstream sinks with remote dpcd reads
to see their link training status, as I think that might actually be the real
reason for why there's this weird difference between the channel eq status in
the esi and the actual link training status of the hub in the dpcd registers
that are shared with SST. but that's for a later date :)
>
> Manasi
>
> > > +
> > > if (downstream_hpd_needs_d0(intel_dp))
> > > return;
> > >
> > > - ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > > - DP_SET_POWER_D3);
> > > + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > > data);
> > > } else {
> > > struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
> > >
> > > --
> > > 2.14.3
> >
> > --
> > Ville Syrjälä
> > Intel OTC
> > ___
> > Intel-gfx mailing list
> > intel-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Cheers,
Lyude Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, Apr 04, 2018 at 06:34:29PM +0300, Ville Syrjälä wrote:
> On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > While enabling/disabling DPMS before link training with MST hubs is
> > perfectly valid; unfortunately disabling DPMS results in some devices
> > disabling their AUX CH block as well. For SST this isn't as much of a
> > problem, but for MST we need to be able to continue handling aux
> > transactions even when none of the sinks are turned on since it's
> > possible for us to have a single atomic commit which results in
> > disabling each downstream sink, followed by subsequently re-enabling
> > each sink.
> >
> > If we don't do this, we'll end up stalling any pending ESI interrupts
> > from the sink for up to 1ms. Unfortunately, dropping ESIs during this
> > timespan makes it so that link fallback retraining for MST (which I will
> > be submitting to the ML shortly) fails due to the channel EQ failure
> > interrupts potentially getting dropped. Additionally, when performing a
> > modeset that brings the hub status's link status from bad -> good having
> > ESIs disabled for that long causes us to miss the hub's response to us
> > trying to start link training as well.
> >
> > Since any sink with MST is going to support DisplayPort 1.2 anyway, save
> > us the hassle of trying to wait until the sink comes back up and just
> > never shut the aux block down.
> >
> > Changes since v2:
> > - Fix patch name, no functional changes
> >
> > Signed-off-by: Lyude Paul
> > Cc: Laura Abbott
> > Cc: Dhinakaran Pandiyan
> > Cc: Ville Syrjälä
> > Cc: sta...@vger.kernel.org
> > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST
> > hub.")
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 62f82c4298ac..0479c377981b 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp,
> > int mode)
> > return;
> >
> > if (mode != DRM_MODE_DPMS_ON) {
> > + unsigned char data = intel_dp->is_mst ?
> > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
>
> This smells like a workaround for an actual bug somewhere. Why exactly
> is the slower wakeup or the AUX block a problem for MST but not for SST
> when the link training is exactly the same for SST and MST?
>
The problem occurs only in case of MST because the Channel EQ failure is
notified
through ESI sideband AUX messages which potentially can get dropped because of
disabling
DPMS. But in case of SST, we detect the channel EQ failure write during the EQ
TPS sequence
which happens on the main link.
Manasi
> > +
> > if (downstream_hpd_needs_d0(intel_dp))
> > return;
> >
> > - ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > -DP_SET_POWER_D3);
> > + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, data);
> > } else {
> > struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
> >
> > --
> > 2.14.3
>
> --
> Ville Syrjälä
> Intel OTC
> ___
> Intel-gfx mailing list
> intel-...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm/i915: Keep AUX block running when disabling DPMS for MST
On Wed, 2018-04-04 at 18:34 +0300, Ville Syrjälä wrote:
> On Mon, Apr 02, 2018 at 05:26:16PM -0400, Lyude Paul wrote:
> > While enabling/disabling DPMS before link training with MST hubs is
> > perfectly valid; unfortunately disabling DPMS results in some devices
> > disabling their AUX CH block as well. For SST this isn't as much of a
> > problem, but for MST we need to be able to continue handling aux
> > transactions even when none of the sinks are turned on since it's
> > possible for us to have a single atomic commit which results in
> > disabling each downstream sink, followed by subsequently re-enabling
> > each sink.
> >
> > If we don't do this, we'll end up stalling any pending ESI interrupts
> > from the sink for up to 1ms. Unfortunately, dropping ESIs during this
> > timespan makes it so that link fallback retraining for MST (which I will
> > be submitting to the ML shortly) fails due to the channel EQ failure
> > interrupts potentially getting dropped. Additionally, when performing a
> > modeset that brings the hub status's link status from bad -> good having
> > ESIs disabled for that long causes us to miss the hub's response to us
> > trying to start link training as well.
> >
> > Since any sink with MST is going to support DisplayPort 1.2 anyway, save
> > us the hassle of trying to wait until the sink comes back up and just
> > never shut the aux block down.
> >
> > Changes since v2:
> > - Fix patch name, no functional changes
> >
> > Signed-off-by: Lyude Paul
> > Cc: Laura Abbott
> > Cc: Dhinakaran Pandiyan
> > Cc: Ville Syrjälä
> > Cc: sta...@vger.kernel.org
> > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST
> > hub.")
> > ---
> > drivers/gpu/drm/i915/intel_dp.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c
> > index 62f82c4298ac..0479c377981b 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2589,11 +2589,13 @@ void intel_dp_sink_dpms(struct intel_dp *intel_dp,
> > int mode)
> > return;
> >
> > if (mode != DRM_MODE_DPMS_ON) {
> > + unsigned char data = intel_dp->is_mst ?
> > + DP_SET_POWER_D3_AUX_ON : DP_SET_POWER_D3;
>
> This smells like a workaround for an actual bug somewhere. Why exactly
> is the slower wakeup or the AUX block a problem for MST but not for SST
> when the link training is exactly the same for SST and MST?
I actually thought about this but I still think this is the appropriate fix.
So; the real reason for the wakeup not being a problem with SST is that for
DPMS on with SST, we actually do a wait to make sure that the hub is ready
before continuing. And yes: I'm fairly sure SST does actually have around the
same wakeup time that MST does, but with the wait we do it doesn't reallhy
make a difference. With MST, we could do this but there's a few reasons I
don't think we should:
* We don't need to. D3_AUX_ON is a part of the 1.2 spec, so any hub that has
MST is going to be guaranteed to have this.
* Turning off the aux block means that there's a high chance we're going to
miss ESIs from sinks
* It's faster to keep the aux block on anyway
>
> > +
> > if (downstream_hpd_needs_d0(intel_dp))
> > return;
> >
> > - ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > -DP_SET_POWER_D3);
> > + ret = drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > data);
> > } else {
> > struct intel_lspcon *lspcon = dp_to_lspcon(intel_dp);
> >
> > --
> > 2.14.3
>
>
--
Cheers,
Lyude Paul
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4] drm: Fix downstream dev count read
In both HDMI and DP, device count is represented by 6:0 bits of a register(BInfo/Bstatus) So macro for bitmasking the device_count is fixed(0x3F->0x7F). v3: Retained the Rb-ed. v4: %s/drm\/i915/drm [rodrigo] Signed-off-by: Ramalingam C cc: Sean Paul Reviewed-by: Sean Paul --- include/drm/drm_hdcp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h index 562fa7df2637..98e63d870139 100644 --- a/include/drm/drm_hdcp.h +++ b/include/drm/drm_hdcp.h @@ -19,7 +19,7 @@ #define DRM_HDCP_RI_LEN2 #define DRM_HDCP_V_PRIME_PART_LEN 4 #define DRM_HDCP_V_PRIME_NUM_PARTS 5 -#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x3f) +#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x7f) #define DRM_HDCP_MAX_CASCADE_EXCEEDED(x) (x & BIT(3)) #define DRM_HDCP_MAX_DEVICE_EXCEEDED(x)(x & BIT(7)) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: ww-mutex performance tests?
On Tue, Apr 3, 2018 at 10:37 AM, Maarten Lankhorst wrote: > Op 02-04-18 om 19:49 schreef Thomas Hellstrom: >> Maarten, Daniel, >> >> Do we have any ww-mutex performance tests somewhere that can be used to test >> the impact of implementation details on various locking scenarios? >> >> Thanks, >> >> /Thomas >> >> > The thing that comes to my mind are some of the kms_cursor_legacy tests that > have been proven to be sensitive to locking issues before. > All subtests with (pipe-*/all-pipes)-(single/forked/torture)-(bo/move) try to > do cursor updates as fast as possible, and report how many updates have been > done. AMD folks have a bunch of tests to exercise their CS paths I think, that should be interesting for the multi-ww_mutex/backoff paths, adding Christian et al. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC] drm/atomic+msm: add helper to implement legacy dirtyfb
On Wed, Apr 4, 2018 at 7:41 PM, Noralf Trønnes wrote:
>
>
> Den 04.04.2018 00.42, skrev Rob Clark:
>>
>> Add an atomic helper to implement dirtyfb support. This is needed to
>> support DSI command-mode panels with x11 userspace (ie. when we can't
>> rely on pageflips to trigger a flush to the panel).
>>
>> To signal to the driver that the async atomic update needs to
>> synchronize with fences, even though the fb didn't change, the
>> drm_atomic_state::dirty flag is added.
>>
>> Signed-off-by: Rob Clark
>> ---
>> Background: there are a number of different folks working on getting
>> upstream kernel working on various different phones/tablets with qcom
>> SoC's.. many of them have command mode panels, so we kind of need a
>> way to support the legacy dirtyfb ioctl for x11 support.
>>
>> I know there is work on a proprer non-legacy atomic property for
>> userspace to communicate dirty-rect(s) to the kernel, so this can
>> be improved from triggering a full-frame flush once that is in
>> place. But we kinda needa a stop-gap solution.
>>
>> I had considered an in-driver solution for this, but things get a
>> bit tricky if userspace ands up combining dirtyfb ioctls with page-
>> flips, because we need to synchronize setting various CTL.FLUSH bits
>> with setting the CTL.START bit. (ie. really all we need to do for
>> cmd mode panels is bang CTL.START, but is this ends up racing with
>> pageflips setting FLUSH bits, then bad things.) The easiest soln
>> is to wrap this up as an atomic commit and rely on the worker to
>> serialize things. Hence adding an atomic dirtyfb helper.
>>
>> I guess at least the helper, with some small addition to translate
>> and pass-thru the dirty rect(s) is useful to the final atomic dirty-
>> rect property solution. Depending on how far off that is, a stop-
>> gap solution could be useful.
>
>
> I have been waiting for someone to address this since I'm not skilled
> enough myself to tackle the atomic machinery. It would be be nice to do
> all flushing during commit since that would mean that the tinydrm drivers
> could be driven solely through drm_simple_display_pipe_funcs. I wouldn't
> have to wire through the dirty callback like I do now.
>
> I see that you use a nonblocking commit. What happens if a new dirtyfb
> comes in before the previous is done?
We could reuse the same trick we're doing in the fbdev code, of
pushing the commit to a worker and doing it in a blocking fashion. Or
at least wait for it to finish (can be done with the crtc_state->event
stuff). That way userspace can pile us up in dirtyfb calls, but we'd
do at most one per frame. More isn't really useful anyway.
> If we could save the dirty clips, then I could use this in tinydrm.
> A full flush over SPI takes ~50ms so I need to shave off where I can.
>
> This works for me in my tinydrm world:
One thing I thought you've had around somewhere is some additional
tracking code, so we don't have to take all the plane mutexes in the
->dirtyfb callback. Does that exist, or was that just a figment of my
imagination?
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index c64225274807..218cb36757fa 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -28,6 +28,7 @@
> #include
> #include
>
> +struct drm_clip_rect;
> struct drm_crtc;
> struct drm_printer;
> struct drm_modeset_acquire_ctx;
> @@ -85,6 +86,9 @@ struct drm_plane_state {
> */
> bool dirty;
>
> + struct drm_clip_rect *dirty_clips;
> + unsigned int num_dirty_clips;
> +
> /**
> * @fence:
> *
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 8066c508ab50..e00b8247b7c5 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3521,6 +3521,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct
> drm_plane *plane,
> drm_framebuffer_get(state->fb);
>
> state->dirty = false;
> + state->dirty_clips = NULL;
> + state->num_dirty_clips = 0;
> state->fence = NULL;
> state->commit = NULL;
> }
> @@ -3567,6 +3569,8 @@ void __drm_atomic_helper_plane_destroy_state(struct
> drm_plane_state *state)
>
> if (state->commit)
> drm_crtc_commit_put(state->commit);
> +
> + kfree(state->dirty_clips);
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
>
> @@ -3877,8 +3881,20 @@ int drm_atomic_helper_dirtyfb(struct drm_framebuffer
> *fb,
> struct drm_modeset_acquire_ctx ctx;
> struct drm_atomic_state *state;
> struct drm_plane *plane;
> + bool fb_found = false;
> int ret = 0;
>
> + /* fbdev can flush even when we don't want it to */
> + drm_for_each_plane(plane, fb->dev) {
> + if (plane->state->fb == fb) {
> + fb_found = true;
> + break;
> + }
> + }
> +
> + if (!
Re: Signal handling in a page fault handler
On Wed, Apr 4, 2018 at 6:24 PM, Matthew Wilcox wrote:
> On Wed, Apr 04, 2018 at 05:15:46PM +0200, Daniel Vetter wrote:
>> On Wed, Apr 4, 2018 at 4:39 PM, Matthew Wilcox wrote:
>> > I actually have plans to allow mutex_lock_{interruptible,killable} to
>> > return -EWOULDBLOCK if a flag is set. So this doesn't seem entirely
>> > unrelated. Something like this perhaps:
>> >
>> > struct task_struct {
>> > + unsigned int sleep_state;
>> > };
>> >
>> > static noinline int __sched
>> > -__mutex_lock_interruptible_slowpath(struct mutex *lock)
>> > +__mutex_lock_slowpath(struct mutex *lock, long state)
>> > {
>> > - return __mutex_lock(lock, TASK_INTERRUPTIBLE, 0, NULL, _RET_IP_);
>> > + if (state == TASK_NOBLOCK)
>> > + return -EWOULDBLOCK;
>> > + return __mutex_lock(lock, state, 0, NULL, _RET_IP_);
>> > }
>> >
>> > +int __sched mutex_lock_state(struct mutex *lock, long state)
>> > +{
>> > + might_sleep();
>> > +
>> > + if (__mutex_trylock_fast(lock))
>> > + return 0;
>> > +
>> > + return __mutex_lock_slowpath(lock, state);
>> > +}
>> > +EXPORT_SYMBOL(mutex_lock_state);
>> >
>> > Then the page fault handler can do something like:
>> >
>> > old_state = current->sleep_state;
>> > current->sleep_state = TASK_INTERRUPTIBLE;
>> > ...
>> > current->sleep_state = old_state;
>> >
>> >
>> > This has the page-fault-in-a-signal-handler problem. I don't know if
>> > there's a way to determine if we're already in a signal handler and use
>> > a different sleep_state ...?
>>
>> Not sure what problem you're trying to solve, but I don't think that's
>> the one we have. The only way what we do goes wrong is if the fault
>> originates from kernel context. For faults from the signal handler I
>> think you just get to keep the pieces. Faults form kernel we can
>> detect through FAULT_FLAG_USER.
>
> Gah, I didn't explain well enough ;-(
>
> From the get_user_pages (and similar) handlers, we'd do
>
> old_state = current->sleep_state;
> current->sleep_state = TASK_KILLABLE;
> ...
> current->sleep_state = old_state;
>
> So you wouldn't need to discriminate on whether FAULT_FLAG_USER was set,
> but could just use current->sleep_state.
>
>> The issue I'm seeing is the following:
>> 1. Some kernel code does copy_*_user, and it points at a gpu mmap region.
>> 2. We fault and go into the gpu driver fault handler. That refuses to
>> insert the pte because a signal is pending (because of all the
>> interruptible waits and locks).
>> 3. Fixup section runs, which afaict tries to do the copy once more
>> using copy_user_handle_tail.
>> 4. We fault again, because the pte is still not present.
>> 5. GPU driver is still refusing to install the pte because signals are
>> pending.
>> 6. Fixup section for copy_user_handle_tail just bails out.
>> 7. copy_*_user returns and indicates that that not all bytes have been
>> copied.
>> 8. syscall (or whatever it is) bails out and returns to userspace,
>> most likely with -EFAULT (but this ofc depends upon the syscall and
>> what it should do when userspace access faults.
>> 9. Signal finally gets handled, but the syscall already failed, and no
>> one will restart it. If userspace is prudent, it might fail (or maybe
>> hit an assert or something).
>
> I think my patch above fixes this. It makes the syscall killable rather
> than interruptible, so it can never observe the short read / -EFAULT
> return if it gets a fatal signal, and the non-fatal signal will be held
> off until the syscall completes.
>
>> Or maybe I'm confused by your diff, since nothing seems to use
>> current->sleep_state. The problem is also that it's any sleep we do
>> (they all tend to be interruptible, at least when waiting for the gpu
>> or taking any locks that might be held while waiting for the gpu, or
>> anything else that might be blocked waiting for the gpu really). So
>> only patching mutex_lock won't fix this.
>
> Sure, I was only patching mutex_lock_state in as an illustration.
> I've also got a 'state' equivalent for wait_on_page_bit() (although
> I'm not sure you care ...).
>
> Looks like you'd need wait_for_completion_state() and
> wait_event_state_timeout() as well.
Yeah, plus ww_mutex_lock_state, and same for the dma_fence wait
functions, but we can patch those once the core stuff has landed.
The part I'm wondering about, that you didn't really illustrate: Are
mutex_lock_interruptible&friends supposed to automatically use
current->sleep_state, or are we somehow supposed to pass that through
the call stack? Or is the idea that we'd mass-convert all the gpu code
that can be reached from the fault handlers to use the new _state
variants, which will internally consult sleep_state?
Asking since the former would be best for us: Most paths can do
interruptible, but some can't, so changing the current->state logic to
take the most limiting of both the explicitly passed sleep_state a
Re: [RFC] drm/atomic+msm: add helper to implement legacy dirtyfb
Den 04.04.2018 00.42, skrev Rob Clark:
Add an atomic helper to implement dirtyfb support. This is needed to
support DSI command-mode panels with x11 userspace (ie. when we can't
rely on pageflips to trigger a flush to the panel).
To signal to the driver that the async atomic update needs to
synchronize with fences, even though the fb didn't change, the
drm_atomic_state::dirty flag is added.
Signed-off-by: Rob Clark
---
Background: there are a number of different folks working on getting
upstream kernel working on various different phones/tablets with qcom
SoC's.. many of them have command mode panels, so we kind of need a
way to support the legacy dirtyfb ioctl for x11 support.
I know there is work on a proprer non-legacy atomic property for
userspace to communicate dirty-rect(s) to the kernel, so this can
be improved from triggering a full-frame flush once that is in
place. But we kinda needa a stop-gap solution.
I had considered an in-driver solution for this, but things get a
bit tricky if userspace ands up combining dirtyfb ioctls with page-
flips, because we need to synchronize setting various CTL.FLUSH bits
with setting the CTL.START bit. (ie. really all we need to do for
cmd mode panels is bang CTL.START, but is this ends up racing with
pageflips setting FLUSH bits, then bad things.) The easiest soln
is to wrap this up as an atomic commit and rely on the worker to
serialize things. Hence adding an atomic dirtyfb helper.
I guess at least the helper, with some small addition to translate
and pass-thru the dirty rect(s) is useful to the final atomic dirty-
rect property solution. Depending on how far off that is, a stop-
gap solution could be useful.
I have been waiting for someone to address this since I'm not skilled
enough myself to tackle the atomic machinery. It would be be nice to do
all flushing during commit since that would mean that the tinydrm drivers
could be driven solely through drm_simple_display_pipe_funcs. I wouldn't
have to wire through the dirty callback like I do now.
I see that you use a nonblocking commit. What happens if a new dirtyfb
comes in before the previous is done?
If we could save the dirty clips, then I could use this in tinydrm.
A full flush over SPI takes ~50ms so I need to shave off where I can.
This works for me in my tinydrm world:
diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
index c64225274807..218cb36757fa 100644
--- a/include/drm/drm_plane.h
+++ b/include/drm/drm_plane.h
@@ -28,6 +28,7 @@
#include
#include
+struct drm_clip_rect;
struct drm_crtc;
struct drm_printer;
struct drm_modeset_acquire_ctx;
@@ -85,6 +86,9 @@ struct drm_plane_state {
*/
bool dirty;
+ struct drm_clip_rect *dirty_clips;
+ unsigned int num_dirty_clips;
+
/**
* @fence:
*
diff --git a/drivers/gpu/drm/drm_atomic_helper.c
b/drivers/gpu/drm/drm_atomic_helper.c
index 8066c508ab50..e00b8247b7c5 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3521,6 +3521,8 @@ void
__drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
drm_framebuffer_get(state->fb);
state->dirty = false;
+ state->dirty_clips = NULL;
+ state->num_dirty_clips = 0;
state->fence = NULL;
state->commit = NULL;
}
@@ -3567,6 +3569,8 @@ void
__drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
if (state->commit)
drm_crtc_commit_put(state->commit);
+
+ kfree(state->dirty_clips);
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
@@ -3877,8 +3881,20 @@ int drm_atomic_helper_dirtyfb(struct
drm_framebuffer *fb,
struct drm_modeset_acquire_ctx ctx;
struct drm_atomic_state *state;
struct drm_plane *plane;
+ bool fb_found = false;
int ret = 0;
+ /* fbdev can flush even when we don't want it to */
+ drm_for_each_plane(plane, fb->dev) {
+ if (plane->state->fb == fb) {
+ fb_found = true;
+ break;
+ }
+ }
+
+ if (!fb_found)
+ return 0;
+
/*
* When called from ioctl, we are interruptable, but not when
* called internally (ie. defio worker)
@@ -3907,6 +3923,25 @@ int drm_atomic_helper_dirtyfb(struct
drm_framebuffer *fb,
}
plane_state->dirty = true;
+ if (clips && num_clips)
+ plane_state->dirty_clips = kmemdup(clips,
num_clips * sizeof(*clips),
+ GFP_KERNEL);
+ else
+ plane_state->dirty_clips =
kzalloc(sizeof(*clips), GFP_KERNEL);
+
+ if (!plane_state->dirty_clips) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ if (clips && num_clips) {
+ plane_state->num_dirty_clips = num_clips;
+ } el
Re: [PATCH libdrm 01/11] symbols-check: add new meta-script
On 4 April 2018 at 17:28, Eric Engestrom wrote:
> On Wednesday, 2018-04-04 16:41:35 +0100, Eric Engestrom wrote:
>> Note: copied from Mesa
>>
>> Signed-off-by: Eric Engestrom
>> ---
>> meson.build | 1 +
>> symbols-check | 79
>> +++
>> 2 files changed, 80 insertions(+)
>> create mode 100755 symbols-check
>>
>> diff --git a/meson.build b/meson.build
>> index a725f05d342bbec4df18..c035a00c6747b8d46a9b 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -272,6 +272,7 @@ pkg.generate(
>>
>> env_test = environment()
>> env_test.set('NM', find_program('nm').path())
>> +env_test.set('top_srcdir', meson.source_root())
>>
>> if with_libkms
>>subdir('libkms')
>> diff --git a/symbols-check b/symbols-check
>> new file mode 100755
>> index ..bac466d93dcb45cee0bb
>> --- /dev/null
>> +++ b/symbols-check
>> @@ -0,0 +1,79 @@
>> +#!/bin/sh
>> +set -eu
>> +set -o pipefail
>> +
>> +# Platform specific symbols
>> +# These will simply be ignored
>> +PLAT_FUNCS="
>> +__bss_start
>> +_init
>> +_fini
>> +_end
>> +_edata
>> +
>> +# From tegra-symbol-check
>> +__bss_end__
>> +__bss_start__
>> +__bss_start
>> +__end__
>> +_bss_end__
>> +_edata
>> +_end
>> +_fini
>> +_init
>
> Haha, oops... I had noticed that one of the old scripts had more platform
> symbols than the rest, and I wanted to check if/when those were needed
> so I just stuffed them here in the mean time, but then I forgot :]
>
> I'll check this when I have the time (not this week), or if anyone knows..?
>
> In the mean time, please review the rest and ignore these 10 lines :)
>
The extra __ suffix/prefixed ones are ARM specific. They seems to be
introduced from day 1 in glibc, without any obvious reason.
They're just aliases - __end__, _bss_end__, etc are identical to _end
If you want to cater for GNU/Hurd - the following three should be
listed as well.
_fbss
_fdata
_ftext
I haven't had time to read through the patches, although +1 on the overall idea.
Will try to get to it sometime tomorrow.
-Emil
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105425] 3D & games produce periodic GPU crashes (Radeon R7 370)
https://bugs.freedesktop.org/show_bug.cgi?id=105425 --- Comment #15 from MirceaKitsune --- Today I've ran two tests to ensure that frequencies and DPM are not a factor. - Setting the DPM profile to low by running the following commands as root: echo battery > /sys/class/drm/card0/device/power_dpm_state echo low > /sys/class/drm/card0/device/power_dpm_force_performance_level - Booting my system with the following Kernel parameters to disable DPM: radeon.dpm=0 amdgpu.dpm=0 Just like with everything else, they made absolutely no difference: Xonotic froze the machine after only 8 minutes of running each time. The settings are applied and visible by checking /sys/kernel/debug/dri/0/amdgpu_pm_info, and are even reflected in the performance which was reduced from 60 FPS to below 30 FPS. This is NOT a hardware failure: The freezes occur identically even if both the core (GPU) and memory (VRAM) clocks are under-clocked to very safe frequencies. The key must be something in the Linux firmware for this card. -- 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: [RFC][PATCH v2] drm_hwcomposer: Rework platformdrmgeneric.cpp to use libdrm's gralloc handle
On Wed, Apr 4, 2018 at 2:24 AM, Robert Foss wrote: > Hey, > > Builds for hikey960, pushed upstream. > Much appreciated! Thanks so much! -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/virtio: remove drm_dev_set_unique workaround
Hi Emil,
On 04/03/18 19:13, Emil Velikov wrote:
> On 29 March 2018 at 12:17, Laszlo Ersek wrote:
>> On 03/28/18 16:35, Emil Velikov wrote:
>>> On 28 March 2018 at 11:27, Laszlo Ersek wrote:
On 03/28/18 03:24, Emil Velikov wrote:
>>
> Gents, can someone double-check this please? Just in case.
I guess I should test whether this series regresses the use case
described in c2cbc38b97; is that correct?
>>> Precisely.
>>
>>> [3] https://github.com/evelikov/linux/commits/ioctl-cleanups
>>
>> Unfortunately, this series seems to reintroduce the regression fixed
>> / described earlier in commit c2cbc38b97.
>>
> Thank you very much for testing.
>
> Believe I've tracked it down to a broken commit from 2014 ;-)
> Please try the following branch [1] - it's untested, but I'm 99% sure
> it will work like a charm.
Alas, it does not work.
> If it doesn't please share the dmesg of
> a) the updated branch, and
> b) baseline + the debug patch [2]
> [1] https://github.com/evelikov/linux/commits/ioctl-cleanups-v2
> [2]
> https://github.com/evelikov/linux/commit/c2bc545d677001ffe5387aebb708d1adeb12b35b
(a) The dmesg from the kernel that is built exactly at [1], i.e., commit
bb6ec6da7086 ("WIP: prefix device unique with pci:", 2018-04-03):
> Linux version 4.16.0-rc1+ (la...@lacos-laptop-7.usersys.redhat.com) (gcc
> version 4.8.5 20150623 (Red Hat 4.8.5-16) (GCC)) #1 SMP Wed Apr 4 17:18:30
> CEST 2018
> Command line: BOOT_IMAGE=/vmlinuz-4.16.0-rc1+ root=/dev/mapper/fedora-root ro
> rd.lvm.lv=fedora/swap vconsole.font=latarcyrheb-sun16 rd.lvm.lv=fedora/root
> video=800x600 no_console_suspend console=tty console=ttyS0,115200
> ignore_loglevel efi=debug LANG=en_US.UTF-8
> x86/fpu: Supporting XSAVE feature 0x001: 'x87 floating point registers'
> x86/fpu: Supporting XSAVE feature 0x002: 'SSE registers'
> x86/fpu: Supporting XSAVE feature 0x004: 'AVX registers'
> x86/fpu: xstate_offset[2]: 576, xstate_sizes[2]: 256
> x86/fpu: Enabled xstate features 0x7, context size is 832 bytes, using
> 'standard' format.
> e820: BIOS-provided physical RAM map:
> BIOS-e820: [mem 0x-0x0009] usable
> BIOS-e820: [mem 0x0010-0x007f] usable
> BIOS-e820: [mem 0x0080-0x00807fff] ACPI NVS
> BIOS-e820: [mem 0x00808000-0x0080] usable
> BIOS-e820: [mem 0x0081-0x008f] ACPI NVS
> BIOS-e820: [mem 0x0090-0xbf7eefff] usable
> BIOS-e820: [mem 0xbf7ef000-0xbfbccfff] reserved
> BIOS-e820: [mem 0xbfbcd000-0xbfbd4fff] ACPI data
> BIOS-e820: [mem 0xbfbd5000-0xbfbfefff] ACPI NVS
> BIOS-e820: [mem 0xbfbff000-0xbff3] usable
> BIOS-e820: [mem 0xbff4-0xbfff] ACPI NVS
> BIOS-e820: [mem 0xffc0-0x] reserved
> BIOS-e820: [mem 0x0001-0x00017fff] usable
> debug: ignoring loglevel setting.
> NX (Execute Disable) protection: active
> e820: update [mem 0xbd28a018-0xbd293857] usable ==> usable
> e820: update [mem 0xbd28a018-0xbd293857] usable ==> usable
> extended physical RAM map:
> reserve setup_data: [mem 0x-0x0009] usable
> reserve setup_data: [mem 0x0010-0x007f] usable
> reserve setup_data: [mem 0x0080-0x00807fff] ACPI NVS
> reserve setup_data: [mem 0x00808000-0x0080] usable
> reserve setup_data: [mem 0x0081-0x008f] ACPI NVS
> reserve setup_data: [mem 0x0090-0xbd28a017] usable
> reserve setup_data: [mem 0xbd28a018-0xbd293857] usable
> reserve setup_data: [mem 0xbd293858-0xbf7eefff] usable
> reserve setup_data: [mem 0xbf7ef000-0xbfbccfff] reserved
> reserve setup_data: [mem 0xbfbcd000-0xbfbd4fff] ACPI data
> reserve setup_data: [mem 0xbfbd5000-0xbfbfefff] ACPI NVS
> reserve setup_data: [mem 0xbfbff000-0xbff3] usable
> reserve setup_data: [mem 0xbff4-0xbfff] ACPI NVS
> reserve setup_data: [mem 0xffc0-0x] reserved
> reserve setup_data: [mem 0x0001-0x00017fff] usable
> efi: EFI v2.70 by EDK II
> efi: SMBIOS=0xbf9b9000 ACPI=0xbfbd4000 ACPI 2.0=0xbfbd4014
> MEMATTR=0xbe2f1298
> efi: mem00: [Boot Code | | | | | | | | |WB|WT|WC|UC]
> range=[0x-0x0fff] (0MB)
> efi: mem01: [Loader Data| | | | | | | | |WB|WT|WC|UC]
> range=[0x1000-0x1fff] (0MB)
> efi: mem02: [Conventional Memory| | | | | | | | |WB|WT|WC|UC]
> range=[0x2000-0x0009] (0MB)
> efi: mem03: [Conventional Memory| | | | | | | | |WB|WT|WC|UC]
> range=[0x0010-0x007f] (7MB)
> efi: mem04: [ACPI Memory NVS| | | | | | | | |WB|WT|WC|UC]
> range=[0
Re: [Intel-gfx] [PATCH] drm/i915: Do NOT skip the first 4k of stolen memory for pre-allocated buffers
On Wed, Apr 04, 2018 at 06:50:16PM +0300, Ville Syrjälä wrote:
> On Fri, Mar 30, 2018 at 04:26:53PM +0200, Hans de Goede wrote:
> > Hi,
> >
> > On 30-03-18 15:25, Hans de Goede wrote:
> > > Hi,
> > >
> > > On 30-03-18 14:44, Chris Wilson wrote:
> > >> Quoting Hans de Goede (2018-03-30 13:37:40)
> > >>> Hi,
> > >>>
> > >>> On 30-03-18 14:30, Chris Wilson wrote:
> > Quoting Hans de Goede (2018-03-30 13:27:15)
> > > Before this commit the WaSkipStolenMemoryFirstPage workaround code was
> > > skipping the first 4k by passing 4096 as start of the address range
> > > passed
> > > to drm_mm_init(). This means that calling drm_mm_reserve_node() to
> > > try and
> > > reserve the firmware framebuffer so that we can inherit it would
> > > always
> > > fail, as the firmware framebuffer starts at address 0.
> > >
> > > Commit d43537610470 ("drm/i915: skip the first 4k of stolen memory on
> > > everything >= gen8") says in its commit message: "This is confirmed
> > > to fix
> > > Skylake screen flickering issues (probably caused by the fact that we
> > > initialized a ring in the first page of stolen, but I didn't 100%
> > > confirm
> > > this theory)."
> > >
> > > Which suggests that it is safe to use the first page for a linear
> > > framebuffer as the firmware is doing.
> > >
> > > This commit always passes 0 as start to drm_mm_init() and works around
> > > WaSkipStolenMemoryFirstPage in i915_gem_stolen_insert_node_in_range()
> > > by insuring the start address passed by to
> > > drm_mm_insert_node_in_range()
> > > is always 4k or more. All entry points to i915_gem_stolen.c go through
> > > i915_gem_stolen_insert_node_in_range(), so that any newly allocated
> > > objects such as ring-buffers will not be allocated in the first 4k.
> > >
> > > The one exception is i915_gem_object_create_stolen_for_preallocated()
> > > which directly calls drm_mm_reserve_node() which now will be able to
> > > use the first 4k.
> > >
> > > This fixes the i915 driver no longer being able to inherit the
> > > firmware
> > > framebuffer on gen8+, which fixes the video output changing from the
> > > vendor logo to a black screen as soon as the i915 driver is loaded
> > > (on systems without fbcon).
> >
> > We've been told by the HW guys not to use the first page. (That's my
> > understanding from every time this gets questioned.)
> > >>>
> > >>> Yet the GOP is happily using the first page. I think we may need to make
> > >>> a difference here between the GPU not using the first page and the
> > >>> display engine/pipeline not using the first page. Note that my patch
> > >>> only influences the inheriting of the initial framebuffer as allocated
> > >>> by the GOP. It does not influence any other allocations from the
> > >>> reserved range, those will still all avoid the first page.
> > >>>
> > >>> Without this patch fastboot / flickerfree support is essentially broken
> > >>> on any gen8+ hardware given that one of the goals of atomic is to be
> > >>> able to do flickerfree transitions I think that this warrants a closer
> > >>> look then just simply saying never use the first page.
> > >>
> > >> The concern is what else (i.e. nothing that we allocated ourselves) that
> > >> may be in the first page...
> > >
> > > Given that the GOP has put its framebuffer there at least at boot there
> > > is nothing there, otherwise it would show up on the display.
> > >
> > > We have a whole bunch of code to inherit the BIOS fb in intel_display.c
> > > and AFAIK that code is there because this inheriting the BIOS fb is
> > > deemed an important feature. So I'm not happy at all with the handwavy
> > > best to not touch it answer I'm getting to this patch.
> > >
> > > Unless there are some clear answer from the hardware folks which
> > > specifically
> > > say we cannot put a framebuffer there (and then why is the GOP doing it?)
> > > then I believe that the best way forward here is to get various people to
> > > test with this patch and the best way to do that is probably to put it
> > > in next. Note I deliberately did not add a Cc stable.
> >
> > To elaborate on this, the excluding of the first 4k of the stolen memory
> > region causes intel_alloc_initial_plane_obj() from intel_display.c to fail,
> > which in turn causes intel_find_initial_plane_obj() to call
> > intel_plane_disable_noatomic(intel_crtc, intel_plane); which temporarily
> > completely turns off the display which leads to a very ugly flickering
> > of the display at boot (as well as replacing the vendor logo with a
> > black screen).
> >
> > I think we can all agree that this behavior is undesirable and even a
> > regression in behavior caused by the fix to exclude the first 4k.
> >
> > Chris, if my patch is not an acceptable way to fix this, then how do you
> > suggest that we fix this?
> >
> > Digging a bit
[Bug 105883] booting with kernel using amd-staging-drm-next on 2400G hangs
https://bugs.freedesktop.org/show_bug.cgi?id=105883 --- Comment #3 from Joshua Lee --- How do I find an prior boot's dmesg? -- 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 105883] booting with kernel using amd-staging-drm-next on 2400G hangs
https://bugs.freedesktop.org/show_bug.cgi?id=105883 --- Comment #2 from Joshua Lee --- Where is my kernel log located? I successfully use a 4.16 mainline kernel... it's dri-next-staging that's causing problems. I am using mesa from git also... though I don't know if it ever gets around to using 3d graphics before the driver fails anyway. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH 3/3] drm/msm/dsi-staging: Gate bus scale code
On Wed, Mar 28, 2018 at 11:48:48AM +0530, Rajesh Yadav wrote:
> DSI driver relies on downstream bus scaling
> driver (msm_bus) for bus bandwidth voting.
> Gate the bus bandwidth voting code under
> CONFIG_QCOM_BUS_SCALING.
>
> Signed-off-by: Rajesh Yadav
Thank you for your patch. Since dsi-staging isn't going upstream, let's hold
off on this.
Sean
> ---
> drivers/gpu/drm/msm/dsi-staging/dsi_clk_manager.c | 8
> drivers/gpu/drm/msm/dsi-staging/dsi_ctrl.c| 7 ++-
> drivers/gpu/drm/msm/dsi-staging/dsi_phy.c | 2 ++
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/dsi-staging/dsi_clk_manager.c
> b/drivers/gpu/drm/msm/dsi-staging/dsi_clk_manager.c
> index 919de1e..047f759 100644
> --- a/drivers/gpu/drm/msm/dsi-staging/dsi_clk_manager.c
> +++ b/drivers/gpu/drm/msm/dsi-staging/dsi_clk_manager.c
> @@ -17,7 +17,9 @@
> #include
> #include "dsi_clk.h"
>
> +#ifdef CONFIG_QCOM_BUS_SCALING
> #include
> +#endif
>
> struct dsi_core_clks {
> struct dsi_core_clk_info clks;
> @@ -226,6 +228,7 @@ int dsi_core_clk_start(struct dsi_core_clks *c_clks)
> }
> }
>
> +#ifdef CONFIG_QCOM_BUS_SCALING
> if (c_clks->bus_handle) {
> rc = msm_bus_scale_client_update_request(c_clks->bus_handle, 1);
> if (rc) {
> @@ -233,11 +236,14 @@ int dsi_core_clk_start(struct dsi_core_clks *c_clks)
> goto error_disable_mmss_clk;
> }
> }
> +#endif
> return rc;
>
> +#ifdef CONFIG_QCOM_BUS_SCALING
> error_disable_mmss_clk:
> if (c_clks->clks.core_mmss_clk)
> clk_disable_unprepare(c_clks->clks.core_mmss_clk);
> +#endif
>
> error_disable_bus_clk:
> if (c_clks->clks.bus_clk)
> @@ -259,6 +265,7 @@ int dsi_core_clk_stop(struct dsi_core_clks *c_clks)
> {
> int rc = 0;
>
> +#ifdef CONFIG_QCOM_BUS_SCALING
> if (c_clks->bus_handle) {
> rc = msm_bus_scale_client_update_request(c_clks->bus_handle, 0);
> if (rc) {
> @@ -266,6 +273,7 @@ int dsi_core_clk_stop(struct dsi_core_clks *c_clks)
> return rc;
> }
> }
> +#endif
>
> if (c_clks->clks.core_mmss_clk)
> clk_disable_unprepare(c_clks->clks.core_mmss_clk);
> diff --git a/drivers/gpu/drm/msm/dsi-staging/dsi_ctrl.c
> b/drivers/gpu/drm/msm/dsi-staging/dsi_ctrl.c
> index fae1b565..0ab92bb 100644
> --- a/drivers/gpu/drm/msm/dsi-staging/dsi_ctrl.c
> +++ b/drivers/gpu/drm/msm/dsi-staging/dsi_ctrl.c
> @@ -17,7 +17,9 @@
> #include
> #include
> #include
> +#ifdef CONFIG_QCOM_BUS_SCALING
> #include
> +#endif
> #include
> #include
>
> @@ -716,6 +718,7 @@ static int dsi_ctrl_axi_bus_client_init(struct
> platform_device *pdev,
> struct dsi_ctrl *ctrl)
> {
> int rc = 0;
> +#ifdef CONFIG_QCOM_BUS_SCALING
> struct dsi_ctrl_bus_scale_info *bus = &ctrl->axi_bus_info;
>
> bus->bus_scale_table = msm_bus_cl_get_pdata(pdev);
> @@ -731,12 +734,13 @@ static int dsi_ctrl_axi_bus_client_init(struct
> platform_device *pdev,
> rc = -EINVAL;
> pr_err("failed to register axi bus client\n");
> }
> -
> +#endif
> return rc;
> }
>
> static int dsi_ctrl_axi_bus_client_deinit(struct dsi_ctrl *ctrl)
> {
> +#ifdef CONFIG_QCOM_BUS_SCALING
> struct dsi_ctrl_bus_scale_info *bus = &ctrl->axi_bus_info;
>
> if (bus->bus_handle) {
> @@ -744,6 +748,7 @@ static int dsi_ctrl_axi_bus_client_deinit(struct dsi_ctrl
> *ctrl)
>
> bus->bus_handle = 0;
> }
> +#endif
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/msm/dsi-staging/dsi_phy.c
> b/drivers/gpu/drm/msm/dsi-staging/dsi_phy.c
> index c13e5bb..e712c61 100644
> --- a/drivers/gpu/drm/msm/dsi-staging/dsi_phy.c
> +++ b/drivers/gpu/drm/msm/dsi-staging/dsi_phy.c
> @@ -17,7 +17,9 @@
> #include
> #include
> #include
> +#ifdef CONFIG_QCOM_BUS_SCALING
> #include
> +#endif
> #include
>
> #include "msm_drv.h"
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH 2/3] drm/msm: Fix return type mismatch for dpu_kms_init
On Wed, Mar 28, 2018 at 11:48:13AM +0530, Rajesh Yadav wrote:
> dpu_kms_init returns pointer to struct msm_kms but
> incase of platform_get_irq() failure, int was returned.
> Fix the return type to avoid compilation error.
>
> Signed-off-by: Rajesh Yadav
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 8ef75f5..531efa9 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -2027,7 +2027,7 @@ struct msm_kms *dpu_kms_init(struct drm_device *dev)
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> DPU_ERROR("failed to get irq: %d\n", irq);
> - return irq;
> + return ERR_PTR(irq);
> }
>
> priv = dev->dev_private;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [DPU PATCH 1/3] drm/msm: Remove unused variables
On Wed, Mar 28, 2018 at 11:47:46AM +0530, Rajesh Yadav wrote:
> Fix compilation errors due to unused variables.
>
> Signed-off-by: Rajesh Yadav
Reviewed-by: Sean Paul
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c| 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c | 1 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 1 -
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 1 -
> 4 files changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index bf46cf1..51cffc4 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -2587,7 +2587,7 @@ static void dpu_crtc_disable(struct drm_crtc *crtc)
> struct dpu_crtc_irq_info *node = NULL;
> struct drm_event event;
> u32 power_on;
> - int ret, i;
> + int ret;
>
> if (!crtc || !crtc->dev || !crtc->dev->dev_private || !crtc->state) {
> DPU_ERROR("invalid crtc\n");
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> index 2bc5894..3b1212b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c
> @@ -264,7 +264,6 @@ static void dpu_encoder_phys_wb_setup_fb(struct
> dpu_encoder_phys *phys_enc,
> const struct msm_format *format;
> int ret;
> struct msm_gem_address_space *aspace;
> - u32 fb_mode;
>
> if (!phys_enc || !phys_enc->dpu_kms || !phys_enc->dpu_kms->catalog ||
> !phys_enc->connector) {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> index 7186c64..8ef75f5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> @@ -426,7 +426,6 @@ static void dpu_kms_commit(struct msm_kms *kms, struct
> drm_atomic_state *state)
> {
> struct drm_crtc *crtc;
> struct drm_crtc_state *crtc_state;
> - struct dpu_crtc_state *cstate;
> int i;
>
> for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index c657e6b..b11a918 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -2742,7 +2742,6 @@ static void dpu_plane_destroy_state(struct drm_plane
> *plane,
> struct dpu_plane *pdpu;
> struct dpu_plane_state *pstate;
> struct dpu_plane_state *old_state;
> - struct drm_property *drm_prop;
>
> if (!plane) {
> DPU_ERROR("invalid plane\n");
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
>
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: ww-mutex performance tests?
On 04/03/2018 10:37 AM, Maarten Lankhorst wrote: Op 02-04-18 om 19:49 schreef Thomas Hellstrom: Maarten, Daniel, Do we have any ww-mutex performance tests somewhere that can be used to test the impact of implementation details on various locking scenarios? Thanks, /Thomas The thing that comes to my mind are some of the kms_cursor_legacy tests that have been proven to be sensitive to locking issues before. All subtests with (pipe-*/all-pipes)-(single/forked/torture)-(bo/move) try to do cursor updates as fast as possible, and report how many updates have been done. ~Maarten Thanks, Maarten. /Thomas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99923] HITMAN (2016) having lighting and artefacting, and overly light room.
https://bugs.freedesktop.org/show_bug.cgi?id=99923 Martin Pilarski changed: What|Removed |Added CC||mpilar...@arcor.de --- Comment #27 from Martin Pilarski --- Possibly, but I actually don't know what is meant with "the temp dumps". -- 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 105883] booting with kernel using amd-staging-drm-next on 2400G hangs
https://bugs.freedesktop.org/show_bug.cgi?id=105883 --- Comment #1 from Alex Deucher --- Can you attach your kernel log or dmesg output from the boot? Do other kernels work? -- 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 v4 9/9] drm/msm: Switch to atomic_helper_commit()
Now that all of the msm-specific goo is tucked safely away we can switch
over to using the atomic helper commit directly. \o/
Changes in v2:
- None
Changes in v3:
- Rebased on Archit's private_obj set
Changes in v4:
- None
Cc: Abhinav Kumar
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/msm_atomic.c | 139 +--
drivers/gpu/drm/msm/msm_drv.c| 7 +-
drivers/gpu/drm/msm/msm_drv.h| 3 +-
3 files changed, 8 insertions(+), 141 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 95c7868445dd..f0635c3da7f4 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -18,8 +18,6 @@
#include "msm_drv.h"
#include "msm_gem.h"
#include "msm_kms.h"
-#include "msm_gem.h"
-#include "msm_fence.h"
static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
struct drm_atomic_state *old_state)
@@ -59,7 +57,7 @@ int msm_atomic_prepare_fb(struct drm_plane *plane,
return msm_framebuffer_prepare(new_state->fb, kms->aspace);
}
-static void msm_atomic_commit_tail(struct drm_atomic_state *state)
+void msm_atomic_commit_tail(struct drm_atomic_state *state)
{
struct drm_device *dev = state->dev;
struct msm_drm_private *priv = dev->dev_private;
@@ -73,19 +71,6 @@ static void msm_atomic_commit_tail(struct drm_atomic_state
*state)
drm_atomic_helper_commit_modeset_enables(dev, state);
- /* NOTE: _wait_for_vblanks() only waits for vblank on
-* enabled CRTCs. So we end up faulting when disabling
-* due to (potentially) unref'ing the outgoing fb's
-* before the vblank when the disable has latched.
-*
-* But if it did wait on disabled (or newly disabled)
-* CRTCs, that would be racy (ie. we could have missed
-* the irq. We need some way to poll for pipe shut
-* down. Or just live with occasionally hitting the
-* timeout in the CRTC disable path (which really should
-* not be critical path)
-*/
-
msm_atomic_wait_for_commit_done(dev, state);
kms->funcs->complete_commit(kms, state);
@@ -96,125 +81,3 @@ static void msm_atomic_commit_tail(struct drm_atomic_state
*state)
drm_atomic_helper_cleanup_planes(dev, state);
}
-
-/* The (potentially) asynchronous part of the commit. At this point
- * nothing can fail short of armageddon.
- */
-static void commit_tail(struct drm_atomic_state *state)
-{
- drm_atomic_helper_wait_for_fences(state->dev, state, false);
-
- drm_atomic_helper_wait_for_dependencies(state);
-
- msm_atomic_commit_tail(state);
-
- drm_atomic_helper_commit_cleanup_done(state);
-
- drm_atomic_state_put(state);
-}
-
-static void commit_work(struct work_struct *work)
-{
- struct drm_atomic_state *state = container_of(work,
- struct drm_atomic_state,
- commit_work);
- commit_tail(state);
-}
-
-/**
- * drm_atomic_helper_commit - commit validated state object
- * @dev: DRM device
- * @state: the driver state object
- * @nonblock: nonblocking commit
- *
- * This function commits a with drm_atomic_helper_check() pre-validated state
- * object. This can still fail when e.g. the framebuffer reservation fails.
- *
- * RETURNS
- * Zero for success or -errno.
- */
-int msm_atomic_commit(struct drm_device *dev,
- struct drm_atomic_state *state, bool nonblock)
-{
- struct msm_drm_private *priv = dev->dev_private;
- struct drm_crtc *crtc;
- struct drm_crtc_state *crtc_state;
- struct drm_plane *plane;
- struct drm_plane_state *old_plane_state, *new_plane_state;
- int i, ret;
-
- /*
-* Note that plane->atomic_async_check() should fail if we need
-* to re-assign hwpipe or anything that touches global atomic
-* state, so we'll never go down the async update path in those
-* cases.
-*/
- if (state->async_update) {
- ret = drm_atomic_helper_prepare_planes(dev, state);
- if (ret)
- return ret;
-
- drm_atomic_helper_async_commit(dev, state);
- drm_atomic_helper_cleanup_planes(dev, state);
- return 0;
- }
-
- ret = drm_atomic_helper_setup_commit(state, nonblock);
- if (ret)
- return ret;
-
- INIT_WORK(&state->commit_work, commit_work);
-
- ret = drm_atomic_helper_prepare_planes(dev, state);
- if (ret)
- return ret;
-
- if (!nonblock) {
- ret = drm_atomic_helper_wait_for_fences(dev, state, true);
- if (ret)
- goto error;
- }
-
- /*
-* This is the point of no return - everything below never fails except
-* when the hw goes bonghits. Which means we can commit the new st
[PATCH v4 5/9] drm/msm: Move implicit sync handling to prepare_fb
In preparation for moving to atomic helpers, move the implicit sync
fence handling out of atomic commit and into the plane->prepare_fb()
hook. While we're at it, de-duplicate the mdp*_prepare_fb functions.
Changes in v4:
- Added
Reported-by: Rob Clark
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 17 +
drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 16 +---
drivers/gpu/drm/msm/msm_atomic.c | 22 ++
drivers/gpu/drm/msm/msm_drv.h | 2 ++
4 files changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
index 7a1ad3af08e3..20e956e14c21 100644
--- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c
@@ -98,21 +98,6 @@ static const struct drm_plane_funcs mdp4_plane_funcs = {
.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
};
-static int mdp4_plane_prepare_fb(struct drm_plane *plane,
-struct drm_plane_state *new_state)
-{
- struct mdp4_plane *mdp4_plane = to_mdp4_plane(plane);
- struct mdp4_kms *mdp4_kms = get_kms(plane);
- struct msm_kms *kms = &mdp4_kms->base.base;
- struct drm_framebuffer *fb = new_state->fb;
-
- if (!fb)
- return 0;
-
- DBG("%s: prepare: FB[%u]", mdp4_plane->name, fb->base.id);
- return msm_framebuffer_prepare(fb, kms->aspace);
-}
-
static void mdp4_plane_cleanup_fb(struct drm_plane *plane,
struct drm_plane_state *old_state)
{
@@ -152,7 +137,7 @@ static void mdp4_plane_atomic_update(struct drm_plane
*plane,
}
static const struct drm_plane_helper_funcs mdp4_plane_helper_funcs = {
- .prepare_fb = mdp4_plane_prepare_fb,
+ .prepare_fb = msm_atomic_prepare_fb,
.cleanup_fb = mdp4_plane_cleanup_fb,
.atomic_check = mdp4_plane_atomic_check,
.atomic_update = mdp4_plane_atomic_update,
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
index 5dc42d89b588..d1006ed69aad 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c
@@ -245,20 +245,6 @@ static const struct drm_plane_funcs mdp5_plane_funcs = {
.atomic_print_state = mdp5_plane_atomic_print_state,
};
-static int mdp5_plane_prepare_fb(struct drm_plane *plane,
-struct drm_plane_state *new_state)
-{
- struct mdp5_kms *mdp5_kms = get_kms(plane);
- struct msm_kms *kms = &mdp5_kms->base.base;
- struct drm_framebuffer *fb = new_state->fb;
-
- if (!new_state->fb)
- return 0;
-
- DBG("%s: prepare: FB[%u]", plane->name, fb->base.id);
- return msm_framebuffer_prepare(fb, kms->aspace);
-}
-
static void mdp5_plane_cleanup_fb(struct drm_plane *plane,
struct drm_plane_state *old_state)
{
@@ -553,7 +539,7 @@ static void mdp5_plane_atomic_async_update(struct drm_plane
*plane,
}
static const struct drm_plane_helper_funcs mdp5_plane_helper_funcs = {
- .prepare_fb = mdp5_plane_prepare_fb,
+ .prepare_fb = msm_atomic_prepare_fb,
.cleanup_fb = mdp5_plane_cleanup_fb,
.atomic_check = mdp5_plane_atomic_check,
.atomic_update = mdp5_plane_atomic_update,
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index c18f0bee20d4..94f9c3e0e7bf 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -16,6 +16,7 @@
*/
#include "msm_drv.h"
+#include "msm_gem.h"
#include "msm_kms.h"
#include "msm_gem.h"
#include "msm_fence.h"
@@ -97,6 +98,27 @@ static void msm_atomic_wait_for_commit_done(struct
drm_device *dev,
}
}
+int msm_atomic_prepare_fb(struct drm_plane *plane,
+ struct drm_plane_state *new_state)
+{
+ struct msm_drm_private *priv = plane->dev->dev_private;
+ struct msm_kms *kms = priv->kms;
+ struct drm_gem_object *obj;
+ struct msm_gem_object *msm_obj;
+ struct dma_fence *fence;
+
+ if (!new_state->fb)
+ return 0;
+
+ obj = msm_framebuffer_bo(new_state->fb, 0);
+ msm_obj = to_msm_bo(obj);
+ fence = reservation_object_get_excl_rcu(msm_obj->resv);
+
+ drm_atomic_set_fence_for_plane(new_state, fence);
+
+ return msm_framebuffer_prepare(new_state->fb, kms->aspace);
+}
+
static void msm_atomic_commit_tail(struct drm_atomic_state *state)
{
struct drm_device *dev = state->dev;
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index 48ed5b9a8580..98e82230b904 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -160,6 +160,8 @@ struct msm_format {
uint32_t pixel_
[PATCH v4 3/9] drm/msm: Don't subclass drm_atomic_state anymore
From: Archit Taneja
With the addition of "private_objs" in drm_atomic_state, we no longer
need to subclass drm_atomic_state to store state of share resources
that don't perfectly fit within planes/crtc/connector state information.
We can now save this state within drm_atomic_state itself using
the private objects.
Remove the infrastructure that allowed subclassing of drm_atomic_state
in the driver.
Changes in v3:
- Added to the msm atomic helper patch set
Changes in v4:
- None
Signed-off-by: Archit Taneja
Signed-off-by: Rob Clark
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 46
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 22
drivers/gpu/drm/msm/msm_atomic.c | 31
drivers/gpu/drm/msm/msm_drv.c| 3 --
drivers/gpu/drm/msm/msm_kms.h| 14
5 files changed, 116 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 6ada098dba0b..6e12e275deba 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -70,42 +70,6 @@ static int mdp5_hw_init(struct msm_kms *kms)
return 0;
}
-struct mdp5_state *mdp5_get_state(struct drm_atomic_state *s)
-{
- struct msm_drm_private *priv = s->dev->dev_private;
- struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct msm_kms_state *state = to_kms_state(s);
- struct mdp5_state *new_state;
- int ret;
-
- if (state->state)
- return state->state;
-
- ret = drm_modeset_lock(&mdp5_kms->state_lock, s->acquire_ctx);
- if (ret)
- return ERR_PTR(ret);
-
- new_state = kmalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
- if (!new_state)
- return ERR_PTR(-ENOMEM);
-
- /* Copy state: */
- new_state->hwpipe = mdp5_kms->state->hwpipe;
- new_state->hwmixer = mdp5_kms->state->hwmixer;
- if (mdp5_kms->smp)
- new_state->smp = mdp5_kms->state->smp;
-
- state->state = new_state;
-
- return new_state;
-}
-
-static void mdp5_swap_state(struct msm_kms *kms, struct drm_atomic_state
*state)
-{
- struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
- swap(to_kms_state(state)->state, mdp5_kms->state);
-}
-
/* Global/shared object state funcs */
/*
@@ -315,7 +279,6 @@ static const struct mdp_kms_funcs kms_funcs = {
.irq = mdp5_irq,
.enable_vblank = mdp5_enable_vblank,
.disable_vblank = mdp5_disable_vblank,
- .swap_state = mdp5_swap_state,
.prepare_commit = mdp5_prepare_commit,
.complete_commit = mdp5_complete_commit,
.wait_for_crtc_commit_done = mdp5_wait_for_crtc_commit_done,
@@ -815,8 +778,6 @@ static void mdp5_destroy(struct platform_device *pdev)
drm_atomic_private_obj_fini(&mdp5_kms->glob_state);
drm_modeset_lock_fini(&mdp5_kms->glob_state_lock);
-
- kfree(mdp5_kms->state);
}
static int construct_pipes(struct mdp5_kms *mdp5_kms, int cnt,
@@ -969,13 +930,6 @@ static int mdp5_init(struct platform_device *pdev, struct
drm_device *dev)
mdp5_kms->dev = dev;
mdp5_kms->pdev = pdev;
- drm_modeset_lock_init(&mdp5_kms->state_lock);
- mdp5_kms->state = kzalloc(sizeof(*mdp5_kms->state), GFP_KERNEL);
- if (!mdp5_kms->state) {
- ret = -ENOMEM;
- goto fail;
- }
-
ret = mdp5_global_obj_init(mdp5_kms);
if (ret)
goto fail;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
index 76f0ddfca322..854dfd30e829 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
@@ -28,8 +28,6 @@
#include "mdp5_ctl.h"
#include "mdp5_smp.h"
-struct mdp5_state;
-
struct mdp5_kms {
struct mdp_kms base;
@@ -49,12 +47,6 @@ struct mdp5_kms {
struct mdp5_cfg_handler *cfg;
uint32_t caps; /* MDP capabilities (MDP_CAP_XXX bits) */
- /**
-* Global atomic state. Do not access directly, use mdp5_get_state()
-*/
- struct mdp5_state *state;
- struct drm_modeset_lock state_lock;
-
/*
* Global private object state, Do not access directly, use
* mdp5_global_get_state()
@@ -88,20 +80,6 @@ struct mdp5_kms {
};
#define to_mdp5_kms(x) container_of(x, struct mdp5_kms, base)
-/* Global atomic state for tracking resources that are shared across
- * multiple kms objects (planes/crtcs/etc).
- *
- * For atomic updates which require modifying global state,
- */
-struct mdp5_state {
- struct mdp5_hw_pipe_state hwpipe;
- struct mdp5_hw_mixer_state hwmixer;
- struct mdp5_smp_state smp;
-};
-
-struct mdp5_state *__must_check
-mdp5_get_state(struct drm_atomic_state *s);
-
/* Global priv
[PATCH v4 4/9] drm/msm: Refactor complete_commit() to look more the helpers
Factor out the commit_tail() portions of complete_commit() into a
separate function to facilitate moving to the atomic helpers in future
patches.
Changes in v2:
- None
Changes in v3:
- Rebased on Archit's private_obj set
Changes in v4:
- None
Cc: Jeykumar Sankaran
Reviewed-by: Archit Taneja
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/msm_atomic.c | 25 -
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 9d0a0ca1f0cb..c18f0bee20d4 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -97,18 +97,12 @@ static void msm_atomic_wait_for_commit_done(struct
drm_device *dev,
}
}
-/* The (potentially) asynchronous part of the commit. At this point
- * nothing can fail short of armageddon.
- */
-static void complete_commit(struct msm_commit *c, bool async)
+static void msm_atomic_commit_tail(struct drm_atomic_state *state)
{
- struct drm_atomic_state *state = c->state;
struct drm_device *dev = state->dev;
struct msm_drm_private *priv = dev->dev_private;
struct msm_kms *kms = priv->kms;
- drm_atomic_helper_wait_for_fences(dev, state, false);
-
kms->funcs->prepare_commit(kms, state);
drm_atomic_helper_commit_modeset_disables(dev, state);
@@ -135,6 +129,19 @@ static void complete_commit(struct msm_commit *c, bool
async)
drm_atomic_helper_cleanup_planes(dev, state);
kms->funcs->complete_commit(kms, state);
+}
+
+/* The (potentially) asynchronous part of the commit. At this point
+ * nothing can fail short of armageddon.
+ */
+static void complete_commit(struct msm_commit *c)
+{
+ struct drm_atomic_state *state = c->state;
+ struct drm_device *dev = state->dev;
+
+ drm_atomic_helper_wait_for_fences(dev, state, false);
+
+ msm_atomic_commit_tail(state);
drm_atomic_state_put(state);
@@ -143,7 +150,7 @@ static void complete_commit(struct msm_commit *c, bool
async)
static void commit_worker(struct work_struct *work)
{
- complete_commit(container_of(work, struct msm_commit, work), true);
+ complete_commit(container_of(work, struct msm_commit, work));
}
/**
@@ -248,7 +255,7 @@ int msm_atomic_commit(struct drm_device *dev,
return 0;
}
- complete_commit(c, false);
+ complete_commit(c);
return 0;
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 8/9] drm/msm: Remove msm_commit/worker, use atomic helper commit
Moving further towards switching fully to the the atomic helpers, this
patch removes the hand-rolled worker nonblock commit code and uses the
atomic helpers commit_work model.
Changes in v2:
- Remove commit_destroy()
- Shuffle order of commit_tail calls to further serialize commits
- Use stall in swap_state to avoid abandoned events on disable
Changes in v3:
- Rebased on Archit's private_obj set
Changes in v4:
- None
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/msm_atomic.c | 153 +--
drivers/gpu/drm/msm/msm_drv.c| 1 -
drivers/gpu/drm/msm/msm_drv.h| 4 -
3 files changed, 42 insertions(+), 116 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 94f9c3e0e7bf..95c7868445dd 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -21,66 +21,6 @@
#include "msm_gem.h"
#include "msm_fence.h"
-struct msm_commit {
- struct drm_device *dev;
- struct drm_atomic_state *state;
- struct work_struct work;
- uint32_t crtc_mask;
-};
-
-static void commit_worker(struct work_struct *work);
-
-/* block until specified crtcs are no longer pending update, and
- * atomically mark them as pending update
- */
-static int start_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
- int ret;
-
- spin_lock(&priv->pending_crtcs_event.lock);
- ret = wait_event_interruptible_locked(priv->pending_crtcs_event,
- !(priv->pending_crtcs & crtc_mask));
- if (ret == 0) {
- DBG("start: %08x", crtc_mask);
- priv->pending_crtcs |= crtc_mask;
- }
- spin_unlock(&priv->pending_crtcs_event.lock);
-
- return ret;
-}
-
-/* clear specified crtcs (no longer pending update)
- */
-static void end_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
-{
- spin_lock(&priv->pending_crtcs_event.lock);
- DBG("end: %08x", crtc_mask);
- priv->pending_crtcs &= ~crtc_mask;
- wake_up_all_locked(&priv->pending_crtcs_event);
- spin_unlock(&priv->pending_crtcs_event.lock);
-}
-
-static struct msm_commit *commit_init(struct drm_atomic_state *state)
-{
- struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL);
-
- if (!c)
- return NULL;
-
- c->dev = state->dev;
- c->state = state;
-
- INIT_WORK(&c->work, commit_worker);
-
- return c;
-}
-
-static void commit_destroy(struct msm_commit *c)
-{
- end_atomic(c->dev->dev_private, c->crtc_mask);
- kfree(c);
-}
-
static void msm_atomic_wait_for_commit_done(struct drm_device *dev,
struct drm_atomic_state *old_state)
{
@@ -148,31 +88,37 @@ static void msm_atomic_commit_tail(struct drm_atomic_state
*state)
msm_atomic_wait_for_commit_done(dev, state);
- drm_atomic_helper_cleanup_planes(dev, state);
-
kms->funcs->complete_commit(kms, state);
+
+ drm_atomic_helper_wait_for_vblanks(dev, state);
+
+ drm_atomic_helper_commit_hw_done(state);
+
+ drm_atomic_helper_cleanup_planes(dev, state);
}
/* The (potentially) asynchronous part of the commit. At this point
* nothing can fail short of armageddon.
*/
-static void complete_commit(struct msm_commit *c)
+static void commit_tail(struct drm_atomic_state *state)
{
- struct drm_atomic_state *state = c->state;
- struct drm_device *dev = state->dev;
+ drm_atomic_helper_wait_for_fences(state->dev, state, false);
- drm_atomic_helper_wait_for_fences(dev, state, false);
+ drm_atomic_helper_wait_for_dependencies(state);
msm_atomic_commit_tail(state);
- drm_atomic_state_put(state);
+ drm_atomic_helper_commit_cleanup_done(state);
- commit_destroy(c);
+ drm_atomic_state_put(state);
}
-static void commit_worker(struct work_struct *work)
+static void commit_work(struct work_struct *work)
{
- complete_commit(container_of(work, struct msm_commit, work));
+ struct drm_atomic_state *state = container_of(work,
+ struct drm_atomic_state,
+ commit_work);
+ commit_tail(state);
}
/**
@@ -191,17 +137,12 @@ int msm_atomic_commit(struct drm_device *dev,
struct drm_atomic_state *state, bool nonblock)
{
struct msm_drm_private *priv = dev->dev_private;
- struct msm_commit *c;
struct drm_crtc *crtc;
struct drm_crtc_state *crtc_state;
struct drm_plane *plane;
struct drm_plane_state *old_plane_state, *new_plane_state;
int i, ret;
- ret = drm_atomic_helper_prepare_planes(dev, state);
- if (ret)
- return ret;
-
/*
* Note that plane->atomic_async_check() should fail if we need
* to re-assign hwpipe or anything that touches global atomic
@@ -209,45 +150,39 @@ int msm_atomic_commit(struct drm_de
[PATCH v4 0/9] drm/msm: Switch to atomic helpers
Fourth version of the atomic helpers patch. This version adds back implicit sync which I accidentally nuked in previous versions. Implicit sync is now done in prepare_fb(). Sean Archit Taneja (3): drm/msm/mdp5: Add global state as a private atomic object drm/msm/mdp5: Use the new private_obj state drm/msm: Don't subclass drm_atomic_state anymore Sean Paul (6): drm/msm: Refactor complete_commit() to look more the helpers drm/msm: Move implicit sync handling to prepare_fb drm/msm: Mark the crtc->state->event consumed drm/msm: Issue queued events when disabling crtc drm/msm: Remove msm_commit/worker, use atomic helper commit drm/msm: Switch to atomic_helper_commit() drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 + drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 17 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 10 + drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 105 ++--- drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 27 ++- drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 12 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 20 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 16 +- drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 17 +- drivers/gpu/drm/msm/msm_atomic.c | 258 ++--- drivers/gpu/drm/msm/msm_drv.c | 11 +- drivers/gpu/drm/msm/msm_drv.h | 9 +- drivers/gpu/drm/msm/msm_kms.h | 14 -- 13 files changed, 167 insertions(+), 350 deletions(-) -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 2/9] drm/msm/mdp5: Use the new private_obj state
From: Archit Taneja
This replaces the usage of the subclassed atomic state (mdp5_state)
with a private_obj state embedded within drm_atomic_state. The latter
method is the preferred approach, since it's simpler to implement
and less prone to errors.
The new API replaces the older and equivalent mdp5_state usage in the
following pattern:
- References to "mdp5_kms->state" (i.e, the old/existing state) is
replaced with mdp5_get_existing_global_state(). In the atomic_check
path, this should be called with the glob_state_lock drm_modeset_lock
alredy taken.
- References to "mdp5_get_state()" are replaced with
mdp5_get_global_state(). This acquires glob_state_lock and uses
drm_atomic_get_private_obj_state() to create a new duplicated state.
Changes in v3:
- Acquire glob_state_lock in mdp5_smp.c
- Added to the msm atomic helper patch set
Changes in v4:
- None
Signed-off-by: Archit Taneja
Signed-off-by: Rob Clark
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 10 --
drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c | 12 ++--
drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c | 20 +++-
drivers/gpu/drm/msm/disp/mdp5/mdp5_smp.c | 17 -
4 files changed, 37 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index fcbdef385a8a..6ada098dba0b 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -190,20 +190,26 @@ static void mdp5_prepare_commit(struct msm_kms *kms,
struct drm_atomic_state *st
{
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
struct device *dev = &mdp5_kms->pdev->dev;
+ struct mdp5_global_state *global_state;
+
+ global_state = mdp5_get_existing_global_state(mdp5_kms);
pm_runtime_get_sync(dev);
if (mdp5_kms->smp)
- mdp5_smp_prepare_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
+ mdp5_smp_prepare_commit(mdp5_kms->smp, &global_state->smp);
}
static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state
*state)
{
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
struct device *dev = &mdp5_kms->pdev->dev;
+ struct mdp5_global_state *global_state;
+
+ global_state = mdp5_get_existing_global_state(mdp5_kms);
if (mdp5_kms->smp)
- mdp5_smp_complete_commit(mdp5_kms->smp, &mdp5_kms->state->smp);
+ mdp5_smp_complete_commit(mdp5_kms->smp, &global_state->smp);
pm_runtime_put_sync(dev);
}
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
index 8a00991f03c7..113e6b569562 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_mixer.c
@@ -52,14 +52,14 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct
drm_crtc *crtc,
{
struct msm_drm_private *priv = s->dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct mdp5_state *state = mdp5_get_state(s);
+ struct mdp5_global_state *global_state = mdp5_get_global_state(s);
struct mdp5_hw_mixer_state *new_state;
int i;
- if (IS_ERR(state))
- return PTR_ERR(state);
+ if (IS_ERR(global_state))
+ return PTR_ERR(global_state);
- new_state = &state->hwmixer;
+ new_state = &global_state->hwmixer;
for (i = 0; i < mdp5_kms->num_hwmixers; i++) {
struct mdp5_hw_mixer *cur = mdp5_kms->hwmixers[i];
@@ -129,8 +129,8 @@ int mdp5_mixer_assign(struct drm_atomic_state *s, struct
drm_crtc *crtc,
void mdp5_mixer_release(struct drm_atomic_state *s, struct mdp5_hw_mixer
*mixer)
{
- struct mdp5_state *state = mdp5_get_state(s);
- struct mdp5_hw_mixer_state *new_state = &state->hwmixer;
+ struct mdp5_global_state *global_state = mdp5_get_global_state(s);
+ struct mdp5_hw_mixer_state *new_state = &global_state->hwmixer;
if (!mixer)
return;
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
index ff52c49095f9..1ef26bc63163 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_pipe.c
@@ -24,17 +24,19 @@ int mdp5_pipe_assign(struct drm_atomic_state *s, struct
drm_plane *plane,
{
struct msm_drm_private *priv = s->dev->dev_private;
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
- struct mdp5_state *state;
+ struct mdp5_global_state *new_global_state, *old_global_state;
struct mdp5_hw_pipe_state *old_state, *new_state;
int i, j;
- state = mdp5_get_state(s);
- if (IS_ERR(state))
- return PTR_ERR(state);
+ new_global_state = mdp5_get_global_state(s);
+ if (IS_ERR(new_global_state))
+ return PTR_E
[PATCH v4 6/9] drm/msm: Mark the crtc->state->event consumed
Don't leave the event != NULL once it's consumed, this is used a signal to the atomic helpers that the event will be handled by the driver. Changes in v2: - None Changes in v3: - Rebased on Archit's private_obj set Changes in v4: - None Cc: Jeykumar Sankaran Reviewed-by: Archit Taneja Signed-off-by: Sean Paul --- drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c | 1 + drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c index 6e5e1aa54ce1..b001699297c4 100644 --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_crtc.c @@ -351,6 +351,7 @@ static void mdp4_crtc_atomic_flush(struct drm_crtc *crtc, spin_lock_irqsave(&dev->event_lock, flags); mdp4_crtc->event = crtc->state->event; + crtc->state->event = NULL; spin_unlock_irqrestore(&dev->event_lock, flags); blend_setup(crtc); diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c index 9893e43ba6c5..76b96081916f 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c @@ -708,6 +708,7 @@ static void mdp5_crtc_atomic_flush(struct drm_crtc *crtc, spin_lock_irqsave(&dev->event_lock, flags); mdp5_crtc->event = crtc->state->event; + crtc->state->event = NULL; spin_unlock_irqrestore(&dev->event_lock, flags); /* -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/9] drm/msm/mdp5: Add global state as a private atomic object
From: Archit Taneja
Global shared resources (hwpipes, hwmixers and SMP) for MDP5 are
implemented as a part of atomic state by subclassing drm_atomic_state.
The preferred approach is to use the drm_private_obj infrastructure
available in the atomic core.
mdp5_global_state is introduced as a drm atomic private object. The two
funcs mdp5_get_global_state() and mdp5_get_existing_global_state() are
the two variants that will be used to access mdp5_global_state.
This will replace the existing mdp5_state struct (which subclasses
drm_atomic_state) and the funcs around it. These will be removed later
once we mdp5_global_state is put to use everywhere.
Changes in v3:
- Added glob_state_lock instead of pushing it into the core
- Added to the msm atomic helper patch set
Changes in v4:
- None
Signed-off-by: Archit Taneja
Signed-off-by: Rob Clark
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c | 87
drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h | 25 +++
2 files changed, 112 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
index 6d8e3a9a6fc0..fcbdef385a8a 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.c
@@ -106,6 +106,86 @@ static void mdp5_swap_state(struct msm_kms *kms, struct
drm_atomic_state *state)
swap(to_kms_state(state)->state, mdp5_kms->state);
}
+/* Global/shared object state funcs */
+
+/*
+ * This is a helper that returns the private state currently in operation.
+ * Note that this would return the "old_state" if called in the atomic check
+ * path, and the "new_state" after the atomic swap has been done.
+ */
+struct mdp5_global_state *
+mdp5_get_existing_global_state(struct mdp5_kms *mdp5_kms)
+{
+ return to_mdp5_global_state(mdp5_kms->glob_state.state);
+}
+
+/*
+ * This acquires the modeset lock set aside for global state, creates
+ * a new duplicated private object state.
+ */
+struct mdp5_global_state *mdp5_get_global_state(struct drm_atomic_state *s)
+{
+ struct msm_drm_private *priv = s->dev->dev_private;
+ struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(priv->kms));
+ struct drm_private_state *priv_state;
+ int ret;
+
+ ret = drm_modeset_lock(&mdp5_kms->glob_state_lock, s->acquire_ctx);
+ if (ret)
+ return ERR_PTR(ret);
+
+ priv_state = drm_atomic_get_private_obj_state(s, &mdp5_kms->glob_state);
+ if (IS_ERR(priv_state))
+ return ERR_CAST(priv_state);
+
+ return to_mdp5_global_state(priv_state);
+}
+
+static struct drm_private_state *
+mdp5_global_duplicate_state(struct drm_private_obj *obj)
+{
+ struct mdp5_global_state *state;
+
+ state = kmemdup(obj->state, sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return NULL;
+
+ __drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);
+
+ return &state->base;
+}
+
+static void mdp5_global_destroy_state(struct drm_private_obj *obj,
+ struct drm_private_state *state)
+{
+ struct mdp5_global_state *mdp5_state = to_mdp5_global_state(state);
+
+ kfree(mdp5_state);
+}
+
+static const struct drm_private_state_funcs mdp5_global_state_funcs = {
+ .atomic_duplicate_state = mdp5_global_duplicate_state,
+ .atomic_destroy_state = mdp5_global_destroy_state,
+};
+
+static int mdp5_global_obj_init(struct mdp5_kms *mdp5_kms)
+{
+ struct mdp5_global_state *state;
+
+ drm_modeset_lock_init(&mdp5_kms->glob_state_lock);
+
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+ if (!state)
+ return -ENOMEM;
+
+ state->mdp5_kms = mdp5_kms;
+
+ drm_atomic_private_obj_init(&mdp5_kms->glob_state,
+ &state->base,
+ &mdp5_global_state_funcs);
+ return 0;
+}
+
static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state
*state)
{
struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
@@ -727,6 +807,9 @@ static void mdp5_destroy(struct platform_device *pdev)
if (mdp5_kms->rpm_enabled)
pm_runtime_disable(&pdev->dev);
+ drm_atomic_private_obj_fini(&mdp5_kms->glob_state);
+ drm_modeset_lock_fini(&mdp5_kms->glob_state_lock);
+
kfree(mdp5_kms->state);
}
@@ -887,6 +970,10 @@ static int mdp5_init(struct platform_device *pdev, struct
drm_device *dev)
goto fail;
}
+ ret = mdp5_global_obj_init(mdp5_kms);
+ if (ret)
+ goto fail;
+
mdp5_kms->mmio = msm_ioremap(pdev, "mdp_phys", "MDP5");
if (IS_ERR(mdp5_kms->mmio)) {
ret = PTR_ERR(mdp5_kms->mmio);
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
index 425a03d213e5..76f0ddfca322 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_kms.h
+++ b/drive
[PATCH v4 7/9] drm/msm: Issue queued events when disabling crtc
Ensure that any queued events are issued when disabling the crtc. This
avoids timeouts when we come back and wait for dependencies (like the
previous frame's flip_done).
Changes in v2:
- None
Changes in v3:
- Rebased on Archit's private_obj set
Changes in v4:
- None
Reviewed-by: Archit Taneja
Signed-off-by: Sean Paul
---
drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
index 76b96081916f..10271359789e 100644
--- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c
@@ -430,6 +430,7 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
struct mdp5_kms *mdp5_kms = get_kms(crtc);
struct device *dev = &mdp5_kms->pdev->dev;
+ unsigned long flags;
DBG("%s", crtc->name);
@@ -445,6 +446,14 @@ static void mdp5_crtc_atomic_disable(struct drm_crtc *crtc,
mdp_irq_unregister(&mdp5_kms->base, &mdp5_crtc->err);
pm_runtime_put_sync(dev);
+ if (crtc->state->event && !crtc->state->active) {
+ WARN_ON(mdp5_crtc->event);
+ spin_lock_irqsave(&mdp5_kms->dev->event_lock, flags);
+ drm_crtc_send_vblank_event(crtc, crtc->state->event);
+ crtc->state->event = NULL;
+ spin_unlock_irqrestore(&mdp5_kms->dev->event_lock, flags);
+ }
+
mdp5_crtc->enabled = false;
}
--
Sean Paul, Software Engineer, Google / Chromium OS
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 01/11] symbols-check: add new meta-script
On Wednesday, 2018-04-04 16:41:35 +0100, Eric Engestrom wrote:
> Note: copied from Mesa
>
> Signed-off-by: Eric Engestrom
> ---
> meson.build | 1 +
> symbols-check | 79
> +++
> 2 files changed, 80 insertions(+)
> create mode 100755 symbols-check
>
> diff --git a/meson.build b/meson.build
> index a725f05d342bbec4df18..c035a00c6747b8d46a9b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -272,6 +272,7 @@ pkg.generate(
>
> env_test = environment()
> env_test.set('NM', find_program('nm').path())
> +env_test.set('top_srcdir', meson.source_root())
>
> if with_libkms
>subdir('libkms')
> diff --git a/symbols-check b/symbols-check
> new file mode 100755
> index ..bac466d93dcb45cee0bb
> --- /dev/null
> +++ b/symbols-check
> @@ -0,0 +1,79 @@
> +#!/bin/sh
> +set -eu
> +set -o pipefail
> +
> +# Platform specific symbols
> +# These will simply be ignored
> +PLAT_FUNCS="
> +__bss_start
> +_init
> +_fini
> +_end
> +_edata
> +
> +# From tegra-symbol-check
> +__bss_end__
> +__bss_start__
> +__bss_start
> +__end__
> +_bss_end__
> +_edata
> +_end
> +_fini
> +_init
Haha, oops... I had noticed that one of the old scripts had more platform
symbols than the rest, and I wanted to check if/when those were needed
so I just stuffed them here in the mean time, but then I forgot :]
I'll check this when I have the time (not this week), or if anyone knows..?
In the mean time, please review the rest and ignore these 10 lines :)
> +"
> +
> +if [ -z "$LIB" ]; then
> + echo "\$LIB needs to be defined for autotools to be able to run this test"
> + exit 1
> +fi
> +
> +# The lib name is passed in with Meson but autotools doesn't support that
> +# so it needs to be hardcoded and overwritten here
> +if [ $# -ge 1 ]; then
> + LIB=$1
> +fi
> +
> +if ! [ -f "$LIB" ]; then
> + echo "lib $LIB doesn't exist"
> + exit 1
> +fi
> +
> +if [ -z "$NM" ]; then
> + echo "\$NM is undefined or empty"
> + exit 1
> +elif ! command -v $NM >/dev/null; then
> + echo "\$NM is not a valid command"
> + exit 1
> +fi
> +
> +AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
> +
> +NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
> + echo "$REQ_FUNCS" | grep -q "^$func$" && continue
> + echo "$PLAT_FUNCS" | grep -q "^$func$" && continue
> +
> + echo $func
> +done)
> +
> +REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
> + echo "$AVAIL_FUNCS" | grep -q "^$func$" && continue
> +
> + echo $func
> +done)
> +
> +if [ -n "$NEW_ABI" ]; then
> + echo "New ABI detected - If intentional, update the test."
> + echo "$NEW_ABI"
> +fi
> +
> +if [ -n "$REMOVED_ABI" ]; then
> + echo "ABI break detected - Required symbol(s) no longer exported!"
> + echo "$REMOVED_ABI"
> +fi
> +
> +if [ -z "$NEW_ABI" ] && [ -z "$REMOVED_ABI" ]; then
> + exit 0
> +else
> + exit 1
> +fi
> --
> Cheers,
> Eric
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 15/15] v4l: vsp1: Rename BRU to BRx
Hi Laurent,
And so - the final patch (of the series)
On 26/02/18 21:45, Laurent Pinchart wrote:
> Some VSP instances have two blending units named BRU (Blend/ROP Unit)
> and BRS (Blend/ROP Sub unit). The BRS is a smaller version of the BRU
> with only two inputs, but otherwise offers similar features and offers
> the same register interface. The BRU and BRS can be used exchangeably in> VSP
> pipelines (provided no more than two inputs are needed).
>
> Due to historical reasons, the VSP1 driver implements support for both
> the BRU and BRS through objects named vsp1_bru. The code uses the name
> BRU to refer to either the BRU or the BRS, except in a few places where
> noted explicitly. This creates confusion.
>
> In an effort to avoid confusion, rename the vsp1_bru object and the
> corresponding API to vsp1_brx, and use BRx to refer to blend unit
> instances regardless of their type. The names BRU and BRS are retained
> where reference to a particular blend unit type is needed, as well as in
> hardware registers to stay close to the datasheet.
>
I fully approve of this change, though I can see there will be some conflicts /
fixups needed when you rebase this - and depending upon when other series get
in.
Thus - rather than do a full review, for now I'll just say:
Acked-by: Kieran Bingham
> Signed-off-by: Laurent Pinchart
> ---
> drivers/media/platform/vsp1/Makefile | 2 +-
> drivers/media/platform/vsp1/vsp1.h | 6 +-
> .../media/platform/vsp1/{vsp1_bru.c => vsp1_brx.c} | 202
> ++---
> .../media/platform/vsp1/{vsp1_bru.h => vsp1_brx.h} | 18 +-
> drivers/media/platform/vsp1/vsp1_drm.c | 172 +-
> drivers/media/platform/vsp1/vsp1_drm.h | 6 +-
> drivers/media/platform/vsp1/vsp1_drv.c | 6 +-
> drivers/media/platform/vsp1/vsp1_pipe.c| 12 +-
> drivers/media/platform/vsp1/vsp1_pipe.h| 4 +-
> drivers/media/platform/vsp1/vsp1_rpf.c | 12 +-
> drivers/media/platform/vsp1/vsp1_rwpf.h| 2 +-
> drivers/media/platform/vsp1/vsp1_video.c | 16 +-
> drivers/media/platform/vsp1/vsp1_wpf.c | 8 +-
> 13 files changed, 233 insertions(+), 233 deletions(-)
> rename drivers/media/platform/vsp1/{vsp1_bru.c => vsp1_brx.c} (63%)
> rename drivers/media/platform/vsp1/{vsp1_bru.h => vsp1_brx.h} (66%)
>
> diff --git a/drivers/media/platform/vsp1/Makefile
> b/drivers/media/platform/vsp1/Makefile
> index f5cd6f0491cb..596775f932c0 100644
> --- a/drivers/media/platform/vsp1/Makefile
> +++ b/drivers/media/platform/vsp1/Makefile
> @@ -3,7 +3,7 @@ vsp1-y:= vsp1_drv.o
> vsp1_entity.o vsp1_pipe.o
> vsp1-y += vsp1_dl.o vsp1_drm.o
> vsp1_video.o
> vsp1-y += vsp1_rpf.o vsp1_rwpf.o
> vsp1_wpf.o
> vsp1-y += vsp1_clu.o vsp1_hsit.o
> vsp1_lut.o
> -vsp1-y += vsp1_bru.o vsp1_sru.o
> vsp1_uds.o
> +vsp1-y += vsp1_brx.o vsp1_sru.o
> vsp1_uds.o
> vsp1-y += vsp1_hgo.o vsp1_hgt.o
> vsp1_histo.o
> vsp1-y += vsp1_lif.o
>
> diff --git a/drivers/media/platform/vsp1/vsp1.h
> b/drivers/media/platform/vsp1/vsp1.h
> index 78ef838416b3..894cc725c2d4 100644
> --- a/drivers/media/platform/vsp1/vsp1.h
> +++ b/drivers/media/platform/vsp1/vsp1.h
> @@ -30,7 +30,7 @@ struct rcar_fcp_device;
> struct vsp1_drm;
> struct vsp1_entity;
> struct vsp1_platform_data;
> -struct vsp1_bru;
> +struct vsp1_brx;
> struct vsp1_clu;
> struct vsp1_hgo;
> struct vsp1_hgt;
> @@ -78,8 +78,8 @@ struct vsp1_device {
> struct rcar_fcp_device *fcp;
> struct device *bus_master;
>
> - struct vsp1_bru *brs;
> - struct vsp1_bru *bru;
> + struct vsp1_brx *brs;
> + struct vsp1_brx *bru;
> struct vsp1_clu *clu;
> struct vsp1_hgo *hgo;
> struct vsp1_hgt *hgt;
> diff --git a/drivers/media/platform/vsp1/vsp1_bru.c
> b/drivers/media/platform/vsp1/vsp1_brx.c
> similarity index 63%
> rename from drivers/media/platform/vsp1/vsp1_bru.c
> rename to drivers/media/platform/vsp1/vsp1_brx.c
> index e8fd2ae3b3eb..b4af1d546022 100644
> --- a/drivers/media/platform/vsp1/vsp1_bru.c
> +++ b/drivers/media/platform/vsp1/vsp1_brx.c
> @@ -1,5 +1,5 @@
> /*
> - * vsp1_bru.c -- R-Car VSP1 Blend ROP Unit
> + * vsp1_brx.c -- R-Car VSP1 Blend ROP Unit (BRU and BRS)
> *
> * Copyright (C) 2013 Renesas Corporation
> *
> @@ -17,45 +17,45 @@
> #include
>
> #include "vsp1.h"
> -#include "vsp1_bru.h"
> +#include "vsp1_brx.h"
> #include "vsp1_dl.h"
> #include "vsp1_pipe.h"
> #include "vsp1_rwpf.h"
> #include "vsp1_video.h"
>
> -#define BRU_MIN_SIZE 1U
> -#define BRU_MAX_SIZE 8190U
> +#define BRX
Re: [PATCH 2/6] drm/omap: gem: Merge __omap_gem_get_pages() and omap_gem_attach_pages()
On Wed, Apr 4, 2018 at 3:37 PM, Lucas Stach wrote:
> Am Montag, den 02.04.2018, 21:50 +0300 schrieb Laurent Pinchart:
>> The __omap_gem_get_pages() function is a wrapper around
>> omap_gem_attach_pages() that returns the omap_obj->pages pointer through
>> a function argument. Some callers don't need the pages pointer, and all
>> of them can access omap_obj->pages directly. To simplify the code merge
>> the __omap_gem_get_pages() wrapper with omap_gem_attach_pages() and
>> update the callers accordingly.
>>
>> > Signed-off-by: Laurent Pinchart
>> ---
>> drivers/gpu/drm/omapdrm/omap_gem.c | 62
>> ++
>> 1 file changed, 23 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
>> b/drivers/gpu/drm/omapdrm/omap_gem.c
>> index 6cfcf60cffe3..13fea207343e 100644
>> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
>> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
>> @@ -222,7 +222,10 @@ static void omap_gem_evict(struct drm_gem_object *obj)
>> * Page Management
>> */
>>
>> -/** ensure backing pages are allocated */
>> +/*
>> + * Ensure backing pages are allocated. Must be called with the omap_obj.lock
>> + * held.
>> + */
>
> Drive-by comment: I always find it hard to validate those comment-only
> lock prerequisites, especially if callstacks get deeper.
>
> What we do in etnaviv is to make those lock comments executable by
> using lockdep_assert_held() to validate the locking assumptions. This
> makes sure that if you ever manage to violate the locking in a code
> rework, a lockdep enabled debug build will explode right at the spot.
+1 on this. I've gone as far as removing all the locking related
comments in core drm code because most of it was misleading or
outright wrong. The runtime checks have a much higher chance of
actually being correct :-)
-Daniel
>
> Regards,
> Lucas
>
>> static int omap_gem_attach_pages(struct drm_gem_object *obj)
>> {
>> > struct drm_device *dev = obj->dev;
>> @@ -232,7 +235,12 @@ static int omap_gem_attach_pages(struct drm_gem_object
>> *obj)
>> > int i, ret;
>> > dma_addr_t *addrs;
>>
>> > - WARN_ON(omap_obj->pages);
>> > + /*
>> > +* If not using shmem (in which case backing pages don't need to be
>> > +* allocated) or if pages are already allocated we're done.
>> > +*/
>> > + if (!(omap_obj->flags & OMAP_BO_MEM_SHMEM) || omap_obj->pages)
>> > + return 0;
>>
>> > pages = drm_gem_get_pages(obj);
>> > if (IS_ERR(pages)) {
>> @@ -288,29 +296,6 @@ static int omap_gem_attach_pages(struct drm_gem_object
>> *obj)
>> > return ret;
>> }
>>
>> -/* acquire pages when needed (for example, for DMA where physically
>> - * contiguous buffer is not required
>> - */
>> -static int __omap_gem_get_pages(struct drm_gem_object *obj,
>> > - struct page ***pages)
>> -{
>> > - struct omap_gem_object *omap_obj = to_omap_bo(obj);
>> > - int ret = 0;
>> -
>> > - if ((omap_obj->flags & OMAP_BO_MEM_SHMEM) && !omap_obj->pages) {
>> > - ret = omap_gem_attach_pages(obj);
>> > - if (ret) {
>> > - dev_err(obj->dev->dev, "could not attach pages\n");
>> > - return ret;
>> > - }
>> > - }
>> -
>> > - /* TODO: even phys-contig.. we should have a list of pages? */
>> > - *pages = omap_obj->pages;
>> -
>> > - return 0;
>> -}
>> -
>> /** release backing pages */
>> static void omap_gem_detach_pages(struct drm_gem_object *obj)
>> {
>> @@ -516,7 +501,6 @@ int omap_gem_fault(struct vm_fault *vmf)
>> > struct drm_gem_object *obj = vma->vm_private_data;
>> > struct omap_gem_object *omap_obj = to_omap_bo(obj);
>> > struct drm_device *dev = obj->dev;
>> > - struct page **pages;
>> > int ret;
>>
>> > /* Make sure we don't parallel update on a fault, nor move or remove
>> @@ -525,7 +509,7 @@ int omap_gem_fault(struct vm_fault *vmf)
>> > mutex_lock(&dev->struct_mutex);
>>
>> > /* if a shmem backed object, make sure we have pages attached now */
>> > - ret = __omap_gem_get_pages(obj, &pages);
>> > + ret = omap_gem_attach_pages(obj);
>> > if (ret)
>> > goto fail;
>>
>> @@ -694,12 +678,12 @@ int omap_gem_roll(struct drm_gem_object *obj, u32 roll)
>>
>> > /* if we aren't mapped yet, we don't need to do anything */
>> > if (omap_obj->block) {
>> > - struct page **pages;
>> -
>> > - ret = __omap_gem_get_pages(obj, &pages);
>> > + ret = omap_gem_attach_pages(obj);
>> > if (ret)
>> > goto fail;
>> > - ret = tiler_pin(omap_obj->block, pages, npages, roll, true);
>> +
>> > + ret = tiler_pin(omap_obj->block, omap_obj->pages, npages,
>> > + roll, true);
>> > if (ret)
>> > dev_err(obj->dev->dev, "could not repin: %d\n", ret);
>> > }
>> @@ -810,14 +794,13 @@ int omap_gem_pin(struct drm_gem_object *obj,
>> dma_addr_t *dma_add
Re: [PATCH 14/15] v4l: vsp1: Add BRx dynamic assignment debugging messages
Hi Laurent,
On 26/02/18 21:45, Laurent Pinchart wrote:
> Dynamic assignment of the BRU and BRS to pipelines is prone to
> regressions, add messages to make debugging easier. Keep it as a
> separate commit to ease removal of those messages once the code will
> deem to be completely stable.
>
> Signed-off-by: Laurent Pinchart
Not really a review required here so much, so I'll just :
Acked-by: Kieran Bingham
> ---
> drivers/media/platform/vsp1/vsp1_drm.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_drm.c
> b/drivers/media/platform/vsp1/vsp1_drm.c
> index 87e31ba0ddf5..521bbc227110 100644
> --- a/drivers/media/platform/vsp1/vsp1_drm.c
> +++ b/drivers/media/platform/vsp1/vsp1_drm.c
> @@ -190,6 +190,10 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device
> *vsp1,
>
> /* Release our BRU if we have one. */
> if (pipe->bru) {
> + dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
> + __func__, pipe->lif->index,
> + BRU_NAME(pipe->bru));
> +
> /*
>* The BRU might be acquired by the other pipeline in
>* the next step. We must thus remove it from the list
> @@ -219,6 +223,9 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device
> *vsp1,
> if (bru->pipe) {
> struct vsp1_drm_pipeline *owner_pipe;
>
> + dev_dbg(vsp1->dev, "%s: pipe %u: waiting for %s\n",
> + __func__, pipe->lif->index, BRU_NAME(bru));
> +
> owner_pipe = to_vsp1_drm_pipeline(bru->pipe);
> owner_pipe->force_bru_release = true;
>
> @@ -245,6 +252,9 @@ static int vsp1_du_pipeline_setup_bru(struct vsp1_device
> *vsp1,
> &pipe->entities);
>
> /* Add the BRU to the pipeline. */
> + dev_dbg(vsp1->dev, "%s: pipe %u: acquired %s\n",
> + __func__, pipe->lif->index, BRU_NAME(bru));
> +
> pipe->bru = bru;
> pipe->bru->pipe = pipe;
> pipe->bru->sink = &pipe->output->entity;
> @@ -549,6 +559,10 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int
> pipe_index,
> drm_pipe->du_complete = NULL;
> pipe->num_inputs = 0;
>
> + dev_dbg(vsp1->dev, "%s: pipe %u: releasing %s\n",
> + __func__, pipe->lif->index,
> + BRU_NAME(pipe->bru));
> +
> list_del(&pipe->bru->list_pipe);
> pipe->bru->pipe = NULL;
> pipe->bru = NULL;
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 11/15] v4l: vsp1: Add per-display list completion notification support
Hi Laurent,
On 26/02/18 21:45, Laurent Pinchart wrote:
> Display list completion is already reported to the frame end handler,
> but that mechanism is global to all display lists. In order to implement
> BRU and BRS reassignment in DRM pipelines we will need to wait for
> completion of a particular display list. Extend the display list and
> frame end handler APIs to support such a notification.
>
> Signed-off-by: Laurent Pinchart
> ---
> drivers/media/platform/vsp1/vsp1_dl.c| 27 +--
> drivers/media/platform/vsp1/vsp1_dl.h| 4 ++--
> drivers/media/platform/vsp1/vsp1_drm.c | 4 ++--
> drivers/media/platform/vsp1/vsp1_pipe.c | 5 +++--
> drivers/media/platform/vsp1/vsp1_pipe.h | 3 ++-
> drivers/media/platform/vsp1/vsp1_video.c | 4 ++--
> 6 files changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> b/drivers/media/platform/vsp1/vsp1_dl.c
> index 0b86ed01e85d..eb2971218e28 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -72,6 +72,7 @@ struct vsp1_dl_body {
> * @fragments: list of extra display list bodies
> * @has_chain: if true, indicates that there's a partition chain
> * @chain: entry in the display list partition chain
> + * @notify: whether the display list completion should be notified
> */
> struct vsp1_dl_list {
> struct list_head list;
> @@ -85,6 +86,8 @@ struct vsp1_dl_list {
>
> bool has_chain;
> struct list_head chain;
> +
> + bool notify;
> };
>
> enum vsp1_dl_mode {
> @@ -550,8 +553,16 @@ static void vsp1_dl_list_commit_continuous(struct
> vsp1_dl_list *dl)
>* case we can't replace the queued list by the new one, as we could
>* race with the hardware. We thus mark the update as pending, it will
>* be queued up to the hardware by the frame end interrupt handler.
> + *
> + * If a display list is already pending we simply drop it as the new
> + * display list is assumed to contain a more recent configuration. It is
> + * an error if the already pending list has the notify flag set, as
> + * there is then a process waiting for that list to complete. This
> + * shouldn't happen as the waiting process should perform proper
> + * locking, but warn just in case.
>*/
> if (vsp1_dl_list_hw_update_pending(dlm)) {
> + WARN_ON(dlm->pending && dlm->pending->notify);
> __vsp1_dl_list_put(dlm->pending);
> dlm->pending = dl;
> return;
> @@ -581,7 +592,7 @@ static void vsp1_dl_list_commit_singleshot(struct
> vsp1_dl_list *dl)
> dlm->active = dl;
> }
>
> -void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
> +void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool notify)
Rather than changing the vsp1_dl_list_commit() function - would it be nicer to
have an API to request or set the notify property? :
@@..@@ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe)
...
+ /* The BRx will be released, without sending an update to DRM */
+ if (drm_pipe->force_bru_release)
+ vsp1_dl_list_request_internal_notify(dl);
vsp1_dl_list_commit(dl);
...
> {
> struct vsp1_dl_manager *dlm = dl->dlm;
> struct vsp1_dl_list *dl_child;
> @@ -598,6 +609,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
> }
> }
>
> + dl->notify = notify;
> +
> spin_lock_irqsave(&dlm->lock, flags);
>
> if (dlm->singleshot)
> @@ -615,16 +628,23 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl)
> /**
> * vsp1_dlm_irq_frame_end - Display list handler for the frame end interrupt
> * @dlm: the display list manager
> + * @notify: whether the display list that completed has notification enabled
> *
> * Return true if the previous display list has completed at frame end, or
> false
> * if it has been delayed by one frame because the display list commit raced
> * with the frame end interrupt. The function always returns true in header
> mode
> * as display list processing is then not continuous and races never occur.
> + *
> + * Upon return, the @notify parameter is set to true if the previous display
> + * list has completed and had been queued with the notify flag, or to false
> + * otherwise. Notification is only supported for continuous mode.
> */
> -bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
> +bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm, bool *notify)
> {
> bool completed = false;
>
> + *notify = false;
> +
> spin_lock(&dlm->lock);
>
> /*
> @@ -652,6 +672,9 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm)
>* frame end interrupt. The display list thus becomes active.
>*/
> if (dlm->queued) {
> + *notify = dlm->queued->notify;
> + dlm->queued->notify = false;
> +
> __vsp1_dl_list_put(dlm->act
Re: [PATCH 10/15] v4l: vsp1: Move DRM pipeline output setup code to a function
Hi Laurent,
On 02/04/18 13:35, Laurent Pinchart wrote:
>>> +/* Setup the output side of the pipeline (WPF and LIF). */
>>> +static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1,
>>> +struct vsp1_pipeline *pipe)
>>> +{
>>> + struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe);
>>> + struct v4l2_subdev_format format = {
>>> + .which = V4L2_SUBDEV_FORMAT_ACTIVE,
>>
>> Why do you initialise this .which here, but all the other member variables
>> below.
>>
>> Wouldn't it make more sense to group all of this initialisation together? or
>> is there a distinction in keeping the .which separate.
>>
>> (Perhaps this is just a way to initialise the rest of the structure to 0,
>> without using the memset?)
>
> The initialization of the .which field is indeed there to avoid the memset,
> but other than that there's no particular reason. I find it clearer to keep
> the initialization of the structure close to the code that makes use of it
> (the next v4l2_subdev_call in this case).
>
> As initializing all members when declaring the variable doesn't make a change
> in code size (gcc 6.4.0) but increases .rodata by 18 bytes and decreases
> __modver by the same amount, I'm tempted to leave it as-is unless you think
> it
> should be changed.
I'm happy to leave it as is - the query was as much to understand why the change
was the way it was :D
But on that logic (reducing .rodata, or rather not increasing it) what's the
benefit of initialising with one (random/psuedo random) member variable over
initialising to all zero, then initialising the .which alongside the rest of
them? Wouldn't the compiler just use the zero page or such to initialise then?
This way is fine if you are happy with how it reads :D
--
Kieran
>>> + };
>>> + int ret;
>>> +
>>> + format.pad = RWPF_PAD_SINK;
>>> + format.format.width = drm_pipe->width;
>>> + format.format.height = drm_pipe->height;
>>> + format.format.code = MEDIA_BUS_FMT_ARGB_1X32;
>>> + format.format.field = V4L2_FIELD_NONE;
>>> +
>>> + ret = v4l2_subdev_call(&pipe->output->entity.subdev, pad, set_fmt,
> NULL,
>>> + &format);
>>> + if (ret < 0)
>>> + return ret;
>>> +
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105254] [r350] piglit tests/sanity.py fail (AGP, ppc, mesa-18.0.0)
https://bugs.freedesktop.org/show_bug.cgi?id=105254 --- Comment #5 from erhar...@mailbox.org --- Created attachment 138589 --> https://bugs.freedesktop.org/attachment.cgi?id=138589&action=edit html summary from 'pigllit run sanity' (18.0.0) -- 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
