Re: [v4,1/3] drm/msm: Add support for GPU cooling
On 10/30/2020 2:18 AM, m...@chromium.org wrote: On Thu, Oct 29, 2020 at 01:37:19PM +0530, Akhil P Oommen wrote: Register GPU as a devfreq cooling device so that it can be passively cooled by the thermal framework. Signed-off-by: Akhil P Oommen Reviewed-by: Matthias Kaehlcke Wait, I did not post a 'Reviewed-by' tag for this patch! I think the patch should be ok, but I'm still not super happy about the resource management involving devfreq in general (see discussion on https://patchwork.freedesktop.org/patch/394291/?series=82476=1). It's not really something introduced by this patch, but if it ever gets fixed releasing the cooling device at the end of msm_gpu_cleanup() after everything else might cause trouble. In summary, I'm supportive of landing this patch, but reluctant to 'sign it off' because of the above. In any case: Tested-by: Matthias Kaehlcke Sorry, Matthias. My mistake. You shared the reviewed tag for the dt-bindings update. Will fix this ASAP. Thanks for verifying this. -Akhil. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH -next] drm/amdgpu/swsmu: Remove unused static struct 'navi10_i2c_algo'
[AMD Official Use Only - Internal Distribution Only] Other used APIs should be also dropped together. navi10_i2c_func() navi10_i2c_xfer() navi10_i2c_write_data() navi10_i2c_read_data() Regards, Evan -Original Message- From: amd-gfx On Behalf Of Zou Wei Sent: Thursday, October 29, 2020 8:00 PM To: Deucher, Alexander ; Koenig, Christian ; airl...@linux.ie; dan...@ffwll.ch Cc: Zou Wei ; dri-devel@lists.freedesktop.org; amd-...@lists.freedesktop.org; linux-ker...@vger.kernel.org Subject: [PATCH -next] drm/amdgpu/swsmu: Remove unused static struct 'navi10_i2c_algo' Fixes the following W=1 kernel build warning(s): drivers/gpu/drm/amd/amdgpu/../pm/swsmu/smu11/navi10_ppt.c:2527:35: warning: ‘navi10_i2c_algo’ defined but not used [-Wunused-const-variable=] static const struct i2c_algorithm navi10_i2c_algo = { ^~~ Reported-by: Hulk Robot Signed-off-by: Zou Wei --- drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c index ef1a62e..bec63f2 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c @@ -2523,12 +2523,6 @@ static u32 navi10_i2c_func(struct i2c_adapter *adap) return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL; } - -static const struct i2c_algorithm navi10_i2c_algo = { -.master_xfer = navi10_i2c_xfer, -.functionality = navi10_i2c_func, -}; - static ssize_t navi10_get_gpu_metrics(struct smu_context *smu, void **table) { -- 2.6.2 ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfxdata=04%7C01%7Cevan.quan%40amd.com%7C1b891bdcddd04c65dcc608d87c0ced25%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637395742921994883%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000sdata=v6O4EVJvUGg%2Byas3QxIcvO16%2FavfZcvXfeiIh8SfZYo%3Dreserved=0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: Fixes tag needs some work in the drm-fixes tree
Hi all, In commit 65d437b83b2b ("drm/amdgpu/pm: fix the fan speed in fan1_input in manual mode for navi1x") Fixes tag Fixes: 3033e9f1c2de ("drm/amdgpu/swsmu: handle manual fan readback on SMU11") has these problem(s): - Target SHA1 does not exist Mayne you meant Fixes: f6eb433954bf ("drm/amdgpu/swsmu: handle manual fan readback on SMU11") -- Cheers, Stephen Rothwell pgpJsxSz6EWiV.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drivers: amdgpu: Correct spelling defalut to default in comment
Applied. Thanks! Alex On Thu, Oct 29, 2020 at 9:17 AM Bhaskar Chowdhury wrote: > > Correct spelling in one of the comment. > > s/defalut/default/p > > Signed-off-by: Bhaskar Chowdhury > --- > CCing Greg becasue it touched drivers file. Trivial though. > > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > index 8cd646eef096..cdc8dd220a77 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c > @@ -556,7 +556,7 @@ static ssize_t dp_phy_test_pattern_debugfs_write(struct > file *f, const char __us > bool disable_hpd = false; > bool valid_test_pattern = false; > uint8_t param_nums = 0; > - /* init with defalut 80bit custom pattern */ > + /* init with default 80bit custom pattern */ > uint8_t custom_pattern[10] = { > 0x1f, 0x7c, 0xf0, 0xc1, 0x07, > 0x1f, 0x7c, 0xf0, 0xc1, 0x07 > -- > 2.26.2 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 7/7] dma-buf: system_heap: Add a system-uncached heap re-using the system heap
On Thu, Oct 29, 2020 at 7:48 PM Hillf Danton wrote: > On Thu, 29 Oct 2020 15:28:34 -0700 John Stultz wrote: > > On Thu, Oct 29, 2020 at 12:10 AM Hillf Danton wrote: > > > On Thu, 29 Oct 2020 00:16:24 + John Stultz wrote: > > > > @@ -194,6 +210,9 @@ static int system_heap_mmap(struct dma_buf *dmabuf, > > > > struct vm_area_struct *vma) > > > > struct sg_page_iter piter; > > > > int ret; > > > > > > > > + if (buffer->uncached) > > > > + vma->vm_page_prot = > > > > pgprot_writecombine(vma->vm_page_prot); > > > > + > > > > > > Wonder why you turn back to dma_mmap_wc() and friends? > > > > Sorry, can you expand on what you are proposing here instead? I'm not > > sure I see how dma_alloc/mmap/*_wc() quite fits here. > > I just wondered if *_wc() could save you two minutes or three. Can you > shed some light on your concerns about their unfitness? Sorry, I feel a bit daft here. I'm still not exactly sure what you're proposing, and your reply of saving minutes doesn't really clarify things. So I'm not sure it's a good use of time to try to (most likely, incorrectly) refute all the possible things you might be suggesting. :) But I'll try to share my thoughts: So the system heap allows for allocation of non-contiguous buffers (currently allocated from page_alloc), which we keep track using sglists. Since the resulting dmabufs are shared between multiple devices, we want to provide a *specific type of memory* (in this case non-contiguous system memory), rather than what the underlying dma_alloc_attr() allocates for a specific device. My sense is dma_mmap_wc() likely ought to be paired with switching to using dma_alloc_wc() as well, which calls down to dma_alloc_attr(). Maybe one could use dma_alloc_attr against the heap device to allocate chunks that we track in the sglist. But I'm not sure how that saves us much other than possibly swapping dma_mmap_wc() for remap_pfn_range()? But again, I suspect I've mischaracterized what you're actually suggesting. So please let me know what you're thinking and I'm happy to consider it. thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/amdgpu: fix build_coefficients() argument
On Mon, Oct 26, 2020 at 5:01 PM Arnd Bergmann wrote: > > From: Arnd Bergmann > > gcc -Wextra warns about a function taking an enum argument > being called with a bool: > > drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c: In > function 'apply_degamma_for_user_regamma': > drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.c:1617:29: > warning: implicit conversion from 'enum ' to 'enum > dc_transfer_func_predefined' [-Wenum-conversion] > 1617 | build_coefficients(, true); > > It appears that a patch was added using the old calling conventions > after the type was changed, and the value should actually be 0 > (TRANSFER_FUNCTION_SRGB) here instead of 1 (true). This looks correct to me. Harry, Leo? Alex > > Fixes: 55a01d4023ce ("drm/amd/display: Add user_regamma to color module") > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/drm/amd/display/modules/color/color_gamma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c > b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c > index b8695660b480..09bc2c249e1a 100644 > --- a/drivers/gpu/drm/amd/display/modules/color/color_gamma.c > +++ b/drivers/gpu/drm/amd/display/modules/color/color_gamma.c > @@ -1614,7 +1614,7 @@ static void apply_degamma_for_user_regamma(struct > pwl_float_data_ex *rgb_regamma > struct pwl_float_data_ex *rgb = rgb_regamma; > const struct hw_x_point *coord_x = coordinates_x; > > - build_coefficients(, true); > + build_coefficients(, TRANSFER_FUNCTION_SRGB); > > i = 0; > while (i != hw_points_num + 1) { > -- > 2.27.0 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] drm/amdgpu: fix incorrect enum type
Applied. Thanks! Alex On Mon, Oct 26, 2020 at 5:01 PM Arnd Bergmann wrote: > > From: Arnd Bergmann > > core_link_write_dpcd() returns enum dc_status, not ddc_result: > > display/dc/core/dc_link_dp.c: In function 'dp_set_panel_mode': > display/dc/core/dc_link_dp.c:4237:11: warning: implicit conversion from 'enum > dc_status' to 'enum ddc_result' > [-Wenum-conversion] > > Avoid the warning by using the correct enum in the caller. > > Fixes: 0b226322434c ("drm/amd/display: Synchronous DisplayPort Link Training") > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > index ff1e9963ec7a..98464886341f 100644 > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > @@ -4230,7 +4230,7 @@ void dp_set_panel_mode(struct dc_link *link, enum > dp_panel_mode panel_mode) > > if (edp_config_set.bits.PANEL_MODE_EDP > != panel_mode_edp) { > - enum ddc_result result = DDC_RESULT_UNKNOWN; > + enum dc_status result = DC_ERROR_UNEXPECTED; > > edp_config_set.bits.PANEL_MODE_EDP = > panel_mode_edp; > @@ -4240,7 +4240,7 @@ void dp_set_panel_mode(struct dc_link *link, enum > dp_panel_mode panel_mode) > _config_set.raw, > sizeof(edp_config_set.raw)); > > - ASSERT(result == DDC_RESULT_SUCESSFULL); > + ASSERT(result == DC_OK); > } > } > DC_LOG_DETECTION_DP_CAPS("Link: %d eDP panel mode supported: %d " > -- > 2.27.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amd/display: remove unneeded semicolon
Applied. Thanks! Alex On Tue, Oct 27, 2020 at 4:07 PM wrote: > > From: Tom Rix > > A semicolon is not needed after a switch statement. > > Signed-off-by: Tom Rix > --- > drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c | 2 +- > drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c > b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c > index 7b4b2304bbff..5feb804af4be 100644 > --- a/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c > +++ b/drivers/gpu/drm/amd/display/dc/dce112/dce112_resource.c > @@ -858,7 +858,7 @@ static struct clock_source *find_matching_pll( > return pool->clock_sources[DCE112_CLK_SRC_PLL5]; > default: > return NULL; > - }; > + } > > return 0; > } > diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c > b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c > index fb6a19d020f9..ee5230ccf3c4 100644 > --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c > +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_log.c > @@ -280,6 +280,6 @@ char *mod_hdcp_state_id_to_str(int32_t id) > return "D2_A9_VALIDATE_STREAM_READY"; > default: > return "UNKNOWN_STATE_ID"; > - }; > + } > } > > -- > 2.18.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: remove unneeded semicolon
Applied. I dropped the first hunk as that fix had already been submitted by someone else. Alex On Tue, Oct 27, 2020 at 3:07 PM wrote: > > From: Tom Rix > > A semicolon is not needed after a switch statement. > > Signed-off-by: Tom Rix > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > index 1b213c4ddfcb..19c0a3655228 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pmu.c > @@ -654,7 +654,7 @@ int amdgpu_pmu_init(struct amdgpu_device *adev) > > default: > return 0; > - }; > + } > > return ret; > } > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c > index 8bf6a7c056bc..a61cf8cfbfc3 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c > @@ -953,7 +953,7 @@ static char *amdgpu_ras_badpage_flags_str(unsigned int > flags) > case AMDGPU_RAS_RETIRE_PAGE_FAULT: > default: > return "F"; > - }; > + } > } > > /** > -- > 2.18.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/7] dma-buf: system_heap: Allocate higher order pages if available
On Thu, Oct 29, 2020 at 7:34 PM Hillf Danton wrote: > On Thu, 29 Oct 2020 12:34:51 -0700 John Stultz wrote: > > As for your comment on HPAGE_PMD_ORDER (9 on arm64/arm) and > > PAGE_ALLOC_COSTLY_ORDER(3), I'm not totally sure I understand your > > question? Are you suggesting those values would be more natural orders > > to choose from? > > The numbers, 9 and 3, are not magic themselves but under the mm diretory > they draw more attentions than others do. Sometimes it would take two > minutes for me to work out that HPAGE_PMD_ORDER does not mean 1MiB, on > platforms like arm64 or not. Yes, I can say it took me longer than two minutes to dig around and work out HPAGE_PMD_ORDER for my last reply. :) Though I'm still a bit unsure if you are proposing something more than just a comment to explain why order 8 and order 4 allocations are used in my patch? Please let me know if so. thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[git pull] drm fixes for 5.10-rc2
Hi Linus, A busier rc2 than normal, have larger sets of fixes for amdgpu + nouveau, along with some i915, docs, core, panel, sun4i, v3d, vc4 fixes. Nothing spooky though or pumpkin related. Dave. drm-fixes-2020-10-30-1: drm fixes for 5.10-rc2 docs: - kernel doc fixes core: - fix shmem helpers dma-buf mmap bug amdgpu: - Add new navi1x PCI ID - GPUVM reserved area fixes - Misc display fixes - Fix bad interactions between display code and CONFIG_KGDB - Fixes for SMU manual fan control and i2c nouveau: - endian regression fix for old gpus - buffer object refcount fix - uapi start/end alignment fix - display notifier fix - display clock checking fixes i915: - Fix max memory region size calculation - Restore ILK-M RPS support, restoring performance - Reject 90/270 degree rotated initial fbs panel: - mantix reset fixes sun4i: - scalar fix vc4: - hdmi audio fixes v3d: - fix double free The following changes since commit 3650b228f83adda7e5ee532e2b90429c03f7b9ec: Linux 5.10-rc1 (2020-10-25 15:14:11 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm tags/drm-fixes-2020-10-30-1 for you to fetch changes up to 7babd126327b8b5a3904d2f8f01c95235801af2a: Merge tag 'drm-intel-fixes-2020-10-29' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes (2020-10-30 11:54:11 +1000) drm fixes for 5.10-rc2 docs: - kernel doc fixes core: - fix shmem helpers dma-buf mmap bug amdgpu: - Add new navi1x PCI ID - GPUVM reserved area fixes - Misc display fixes - Fix bad interactions between display code and CONFIG_KGDB - Fixes for SMU manual fan control and i2c nouveau: - endian regression fix for old gpus - buffer object refcount fix - uapi start/end alignment fix - display notifier fix - display clock checking fixes i915: - Fix max memory region size calculation - Restore ILK-M RPS support, restoring performance - Reject 90/270 degreerotated initial fbs panel: - mantix reset fixes sun4i: - scalar fix vc4: - hdmi audio fixes v3d: - fix double free Alex Deucher (3): drm/amdgpu/display: use kvzalloc again in dc_create_state drm/amdgpu/swsmu: drop smu i2c bus on navi1x drm/amdgpu/pm: fix the fan speed in fan1_input in manual mode for navi1x Christian König (1): drm/amdgpu: increase the reserved VM size to 2MB Dan Carpenter (1): drm/v3d: Fix double free in v3d_submit_cl_ioctl() Daniel Vetter (1): drm/shme-helpers: Fix dma_buf_mmap forwarding bug Dave Airlie (4): Merge tag 'amd-drm-fixes-5.10-2020-10-29' of git://people.freedesktop.org/~agd5f/linux into drm-fixes Merge tag 'drm-misc-fixes-2020-10-29' of git://anongit.freedesktop.org/drm/drm-misc into drm-fixes Merge branch 'linux-5.10' of git://github.com/skeggsb/linux into drm-fixes Merge tag 'drm-intel-fixes-2020-10-29' of git://anongit.freedesktop.org/drm/drm-intel into drm-fixes David Galiffi (1): drm/amd/display: Fixed panic during seamless boot. Dmytro Laktyushkin (1): drm/amd/display: prevent null pointer access Guido Günther (3): drm/panel: mantix: Don't dereference NULL mode drm/panel: mantix: Fix panel reset dt-binding: display: Require two resets on mantix panel Hoegeun Kwon (1): drm/vc4: drv: Add error handding for bind Karol Herbst (2): drm/nouveau/gem: fix "refcount_t: underflow; use-after-free" drm/nouveau/device: fix changing endianess code to work on older GPUs Kenneth Feng (1): drm/amd/pm: fix the wrong fan speed in fan1_input Lyude Paul (3): drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps drm/nouveau/kms/nv50-: Get rid of bogus nouveau_conn_mode_valid() drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid() Madhav Chauhan (1): drm/amdgpu: don't map BO in reserved region Martin Leung (1): drm/amd/display: adding ddc_gpio_vga_reg_list to ddc reg def'ns Matthew Auld (1): drm/i915/region: fix max size calculation Mauro Carvalho Chehab (7): drm: kernel-doc: document drm_dp_set_subconnector_property() params drm/dp: fix kernel-doc warnings at drm_dp_helper.c drm/dp: fix a kernel-doc issue at drm_edid.c drm: drm_edid: remove a duplicated kernel-doc declaration drm: kernel-doc: add description for a new function parameter drm: kernel-doc: drm_dp_helper.h: fix a typo drm: drm_print.h: fix kernel-doc markups Maxime Ripard (7): Merge remote-tracking branch 'drm-misc/drm-misc-next-fixes' into drm-misc-fixes drm/sun4i: frontend: Rework a bit the phase data drm/sun4i: frontend: Reuse the ch0 phase for RGB formats drm/sun4i: frontend: Fix the scaler phase on A33 drm/vc4: hdmi: Avoid sleeping in atomic context drm/vc4: hdmi: Add a name to the codec DAI component drm/vc4: Rework the structure
Re: [PATCH 1/4] drm: allow drm_atomic_print_state() to accept any drm_printer
Hi Daniel On 2020-10-22 03:38, Daniel Vetter wrote: On Wed, Oct 21, 2020 at 10:01:45PM -0700, Abhinav Kumar wrote: Currently drm_atomic_print_state() internally allocates and uses a drm_info printer. Allow it to accept any drm_printer type so that the API can be leveraged even for taking drm snapshot. Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/drm_atomic.c| 17 - drivers/gpu/drm/drm_atomic_uapi.c | 4 +++- drivers/gpu/drm/drm_crtc_internal.h | 4 +++- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 58527f151984..e7079a5f439c 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1,6 +1,7 @@ /* * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1543,9 +1544,9 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, } EXPORT_SYMBOL(__drm_atomic_helper_set_config); -void drm_atomic_print_state(const struct drm_atomic_state *state) +void drm_atomic_print_state(const struct drm_atomic_state *state, + struct drm_printer *p) Please add a nice kerneldoc for this newly exported function. Specifically this kerneldoc needs to include a warning that state updates after call drm_atomic_state_helper_commit_hw_done() is unsafe to print using this function, because it looks at the new state objects. Only the old state structures will stay like this. So maybe rename the function to say print_new_state() to make this completely clear. That way we can eventually add a print_old_state() when needed. Otherwise I think this makes sense, and nicely avoids the locking issue of looking at ->state pointers without the right locking. -Daniel Thanks for the review, I have addressed these comments and posted a V2. -Abhinav { - struct drm_printer p = drm_info_printer(state->dev->dev); struct drm_plane *plane; struct drm_plane_state *plane_state; struct drm_crtc *crtc; @@ -1554,17 +1555,23 @@ void drm_atomic_print_state(const struct drm_atomic_state *state) struct drm_connector_state *connector_state; int i; + if (!p) { + DRM_ERROR("invalid drm printer\n"); + return; + } + DRM_DEBUG_ATOMIC("checking %p\n", state); for_each_new_plane_in_state(state, plane, plane_state, i) - drm_atomic_plane_print_state(, plane_state); + drm_atomic_plane_print_state(p, plane_state); for_each_new_crtc_in_state(state, crtc, crtc_state, i) - drm_atomic_crtc_print_state(, crtc_state); + drm_atomic_crtc_print_state(p, crtc_state); for_each_new_connector_in_state(state, connector, connector_state, i) - drm_atomic_connector_print_state(, connector_state); + drm_atomic_connector_print_state(p, connector_state); } +EXPORT_SYMBOL(drm_atomic_print_state); static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p, bool take_locks) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 25c269bc4681..d9ae86c92608 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -2,6 +2,7 @@ * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. * Copyright (C) 2018 Intel Corp. + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1294,6 +1295,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, struct drm_out_fence_state *fence_state; int ret = 0; unsigned int i, j, num_fences; + struct drm_printer p = drm_info_printer(dev->dev); /* disallow for drivers not supporting atomic: */ if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) @@ -1413,7 +1415,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ret = drm_atomic_nonblocking_commit(state); } else { if (drm_debug_enabled(DRM_UT_STATE)) - drm_atomic_print_state(state); + drm_atomic_print_state(state, ); ret = drm_atomic_commit(state); } diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index da96b2f64d7e..d34215366936 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -5,6 +5,7 @@ * Jesse Barnes * Copyright © 2014 Intel Corporation * Daniel Vetter + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is
[PATCH v2 3/4] drm/msm: register the base address with dpu_dbg module
Register the base address of various dpu sub-modules with the dpu_dbg module so that it can be dumped out during error scenarios. changes in v2: - Fix an issue where the same dsi client was getting registered multiple times to the dpu_dbg module Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 4 +-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c| 6 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 7 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 5 +++- .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 6 - drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c| 7 - drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 12 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 ++- drivers/gpu/drm/msm/dp/dp_catalog.c | 12 + drivers/gpu/drm/msm/dp/dp_catalog.h | 4 +++ drivers/gpu/drm/msm/dp/dp_display.c | 2 ++ drivers/gpu/drm/msm/dsi/dsi.c | 1 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c| 15 ++- drivers/gpu/drm/msm/msm_drv.c | 26 ++- drivers/gpu/drm/msm/msm_drv.h | 3 ++- 17 files changed, 108 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c index f83682668e87..06d28efb45c9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c @@ -263,7 +263,7 @@ int dpu_dbg_reg_register_base(const char *name, void __iomem *base, /* Initialize list to make sure check for null list will be valid */ INIT_LIST_HEAD(_base->sub_range_list); - pr_debug("%s base: %pK max_offset 0x%zX\n", reg_base->name, + pr_info("%s base: %pK max_offset 0x%zX\n", reg_base->name, reg_base->base, reg_base->max_offset); list_add(_base->reg_base_head, _base->reg_base_list); @@ -310,7 +310,7 @@ void dpu_dbg_reg_register_dump_range(const char *base_name, range->xin_id = xin_id; list_add_tail(>head, _base->sub_range_list); - pr_debug("base %s, range %s, start 0x%X, end 0x%X\n", + pr_info("base_name %s, range_name %s, start 0x%X, end 0x%X\n", base_name, range->range_name, range->offset.start, range->offset.end); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c index 758c355b4fd8..3a827ea96723 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. +/* Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved. */ #include @@ -7,6 +7,7 @@ #include "dpu_hw_ctl.h" #include "dpu_kms.h" #include "dpu_trace.h" +#include "dpu_dbg.h" #define CTL_LAYER(lm) \ (((lm) == LM_5) ? (0x024) : (((lm) - LM_0) * 0x004)) @@ -588,6 +589,9 @@ struct dpu_hw_ctl *dpu_hw_ctl_init(enum dpu_ctl idx, dpu_hw_blk_init(>base, DPU_HW_BLK_CTL, idx, _hw_ops); + dpu_dbg_reg_register_dump_range(DPU_DBG_NAME, cfg->name, c->hw.blk_off, + c->hw.blk_off + c->hw.length, c->hw.xin_id); + return c; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c index a7a24539921f..ee9ae02f5e7f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. +/* Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved. */ #include "dpu_hwio.h" @@ -7,7 +7,7 @@ #include "dpu_hw_lm.h" #include "dpu_hw_dspp.h" #include "dpu_kms.h" - +#include "dpu_dbg.h" /* DSPP_PCC */ #define PCC_EN BIT(0) @@ -115,6 +115,9 @@ struct dpu_hw_dspp *dpu_hw_dspp_init(enum dpu_dspp idx, dpu_hw_blk_init(>base, DPU_HW_BLK_DSPP, idx, _hw_ops); + dpu_dbg_reg_register_dump_range(DPU_DBG_NAME, cfg->name, c->hw.blk_off, + c->hw.blk_off + c->hw.length, c->hw.xin_id); + return c; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c index 6f0f54588124..d8198d4d42bc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c @@ -1,11 +1,12 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. +/* Copyright (c) 2015-2020, The Linux Foundation. All rights reserved. */ #include "dpu_hwio.h" #include "dpu_hw_catalog.h" #include "dpu_hw_intf.h" #include "dpu_kms.h" +#include "dpu_dbg.h" #define
[PATCH v2 2/4] drm/msm/dpu: add support to dump dpu registers
Add the dpu_dbg module which adds supports to dump dpu registers which can be used in case of error conditions. changes in v2: Fix kbot errors Reported-by: kernel test robot Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 316 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 273 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 314 + .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 4 +- drivers/gpu/drm/msm/msm_drv.c | 6 +- 6 files changed, 913 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile index 340682cd0f32..96bd1398edac 100644 --- a/drivers/gpu/drm/msm/Makefile +++ b/drivers/gpu/drm/msm/Makefile @@ -54,6 +54,8 @@ msm-y := \ disp/dpu1/dpu_core_irq.o \ disp/dpu1/dpu_core_perf.o \ disp/dpu1/dpu_crtc.o \ + disp/dpu1/dpu_dbg.o \ + disp/dpu1/dpu_dbg_util.o \ disp/dpu1/dpu_encoder.o \ disp/dpu1/dpu_encoder_phys_cmd.o \ disp/dpu1/dpu_encoder_phys_vid.o \ diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c new file mode 100644 index ..f83682668e87 --- /dev/null +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c @@ -0,0 +1,316 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright (c) 2009-2020, The Linux Foundation. All rights reserved. + */ + +#define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__ + +#include "dpu_dbg.h" +#include "dpu_hw_catalog.h" + +/* global dpu debug base structure */ +static struct dpu_dbg_base dpu_dbg; + + +#ifdef CONFIG_DEV_COREDUMP +static ssize_t dpu_devcoredump_read(char *buffer, loff_t offset, + size_t count, void *data, size_t datalen) +{ + struct drm_print_iterator iter; + struct drm_printer p; + + iter.data = buffer; + iter.offset = 0; + iter.start = offset; + iter.remain = count; + + p = drm_coredump_printer(); + + drm_printf(, "---\n"); + + drm_printf(, "module: " KBUILD_MODNAME "\n"); + drm_printf(, "dpu devcoredump\n"); + drm_printf(, "timestamp %lld\n", ktime_to_ns(dpu_dbg.timestamp)); + + dpu_dbg.dpu_dbg_printer = + dpu_dbg.enable_reg_dump = DPU_DBG_DUMP_IN_COREDUMP; + + drm_printf(, "===dpu regs\n"); + + _dpu_dump_array(_dbg, dpu_dbg.req_dump_blks, + ARRAY_SIZE(dpu_dbg.req_dump_blks), + dpu_dbg.work_panic, "evtlog_workitem", + dpu_dbg.dump_all); + + drm_printf(, "===dpu drm state\n"); + + if (dpu_dbg.atomic_state) + drm_atomic_print_new_state(dpu_dbg.atomic_state, + ); + + return count - iter.remain; +} + +static void dpu_devcoredump_free(void *data) +{ + if (dpu_dbg.atomic_state) { + drm_atomic_state_put(dpu_dbg.atomic_state); + dpu_dbg.atomic_state = NULL; + } + dpu_dbg.coredump_pending = false; +} + +static void dpu_devcoredump_capture_state(void) +{ + struct drm_device *ddev; + struct drm_modeset_acquire_ctx ctx; + + dpu_dbg.timestamp = ktime_get(); + + ddev = dpu_dbg.drm_dev; + + drm_modeset_acquire_init(, 0); + + while (drm_modeset_lock_all_ctx(ddev, ) != 0) + drm_modeset_backoff(); + + dpu_dbg.atomic_state = drm_atomic_helper_duplicate_state(ddev, + ); + drm_modeset_drop_locks(); + drm_modeset_acquire_fini(); +} +#else +static void dpu_devcoredump_capture_state(void) +{ +} +#endif /* CONFIG_DEV_COREDUMP */ + +/** + * _dpu_dump_work - deferred dump work function + * @work: work structure + */ +static void _dpu_dump_work(struct kthread_work *work) +{ + /* reset the enable_reg_dump to default before every dump */ + dpu_dbg.enable_reg_dump = DEFAULT_REGDUMP; + + _dpu_dump_array(_dbg, dpu_dbg.req_dump_blks, + ARRAY_SIZE(dpu_dbg.req_dump_blks), + dpu_dbg.work_panic, "evtlog_workitem", + dpu_dbg.dump_all); + + dpu_devcoredump_capture_state(); + +#ifdef CONFIG_DEV_COREDUMP + if (dpu_dbg.enable_reg_dump & DPU_DBG_DUMP_IN_MEM) { + dev_coredumpm(dpu_dbg.dev, THIS_MODULE, _dbg, 0, GFP_KERNEL, + dpu_devcoredump_read, dpu_devcoredump_free); + dpu_dbg.coredump_pending = true; + } +#endif +} + +void dpu_dbg_dump(enum dpu_dbg_dump_context dump_mode, const char *name, ...) +{ + int i, index = 0; + bool do_panic = false; + bool dump_all = false; + va_list args; + char *blk_name = NULL; +
[PATCH v2 4/4] drm/msm/dpu: add dpu_dbg points across dpu driver
Add dpu_dbg points across dpu driver to trigger dumps when critical errors are hit. changes in v2: none Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 ++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 5 +++-- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 5 - 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c index f7f5c258b553..a2ee1af73c9f 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2014-2018, The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2018, 2020 The Linux Foundation. All rights reserved. * Copyright (C) 2013 Red Hat * Author: Rob Clark */ @@ -26,6 +26,7 @@ #include "dpu_crtc.h" #include "dpu_trace.h" #include "dpu_core_irq.h" +#include "dpu_dbg.h" #define DPU_DEBUG_ENC(e, fmt, ...) DPU_DEBUG("enc%d " fmt,\ (e) ? (e)->base.base.id : -1, ##__VA_ARGS__) @@ -1313,6 +1314,11 @@ static void dpu_encoder_underrun_callback(struct drm_encoder *drm_enc, DPU_ATRACE_BEGIN("encoder_underrun_callback"); atomic_inc(_enc->underrun_cnt); + + /* trigger dump only on the first underrun */ + if (atomic_read(_enc->underrun_cnt) == 1) + DPU_DBG_DUMP("all"); + trace_dpu_enc_underrun_cb(DRMID(drm_enc), atomic_read(_enc->underrun_cnt)); DPU_ATRACE_END("encoder_underrun_callback"); @@ -1553,8 +1559,10 @@ static void dpu_encoder_helper_hw_reset(struct dpu_encoder_phys *phys_enc) ctl->idx); rc = ctl->ops.reset(ctl); - if (rc) + if (rc) { DPU_ERROR_ENC(dpu_enc, "ctl %d reset failure\n", ctl->idx); + DPU_DBG_DUMP("all"); + } phys_enc->enable_state = DPU_ENC_ENABLED; } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c index 8493d68ad841..58f79557b560 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Copyright (c) 2015-2018 The Linux Foundation. All rights reserved. + * Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__ @@ -9,6 +9,7 @@ #include "dpu_core_irq.h" #include "dpu_formats.h" #include "dpu_trace.h" +#include "dpu_dbg.h" #define DPU_DEBUG_CMDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \ (e) && (e)->base.parent ? \ @@ -213,7 +214,7 @@ static int _dpu_encoder_phys_cmd_handle_ppdone_timeout( phys_enc->hw_ctl->idx - CTL_0, cmd_enc->pp_timeout_report_cnt, atomic_read(_enc->pending_kickoff_cnt)); - + DPU_DBG_DUMP("all"); dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_RDPTR); } diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c index 805e059b50b7..46c5320150fa 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c @@ -1,5 +1,5 @@ // SPDX-License-Identifier: GPL-2.0-only -/* Copyright (c) 2015-2018, The Linux Foundation. All rights reserved. +/* Copyright (c) 2015-2018, 2020 The Linux Foundation. All rights reserved. */ #define pr_fmt(fmt)"[drm:%s:%d] " fmt, __func__, __LINE__ @@ -8,6 +8,7 @@ #include "dpu_core_irq.h" #include "dpu_formats.h" #include "dpu_trace.h" +#include "dpu_dbg.h" #define DPU_DEBUG_VIDENC(e, fmt, ...) DPU_DEBUG("enc%d intf%d " fmt, \ (e) && (e)->parent ? \ @@ -467,6 +468,7 @@ static void dpu_encoder_phys_vid_enable(struct dpu_encoder_phys *phys_enc) "update pending flush ctl %d flush_mask 0%x intf_mask 0x%x\n", ctl->idx - CTL_0, flush_mask, intf_flush_mask); + atomic_set(_enc->underrun_cnt, 0); /* ctl_flush & timing engine enable will be triggered by framework */ if (phys_enc->enable_state == DPU_ENC_DISABLED) @@ -549,6 +551,7 @@ static void dpu_encoder_phys_vid_prepare_for_kickoff( if (rc) { DPU_ERROR_VIDENC(phys_enc, "ctl %d reset failure: %d\n", ctl->idx, rc); + DPU_DBG_DUMP("all"); dpu_encoder_helper_unregister_irq(phys_enc, INTR_IDX_VSYNC); } } -- 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
[PATCH v2 1/4] drm: allow drm_atomic_print_state() to accept any drm_printer
Currently drm_atomic_print_state() internally allocates and uses a drm_info printer. Allow it to accept any drm_printer type so that the API can be leveraged even for taking drm snapshot. Rename the drm_atomic_print_state() to drm_atomic_print_new_state() so that it reflects its functionality better. changes in v2: - Rename the function drm_atomic_print_state to drm_atomic_print_new_state and update the commit text - Fix kbot errors - Add kernel doc for the newly exported function Reported-by: kernel test robot Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/drm_atomic.c | 29 +++ drivers/gpu/drm/drm_atomic_uapi.c | 4 ++- drivers/gpu/drm/drm_crtc_internal.h | 4 ++- .../gpu/drm/selftests/test-drm_framebuffer.c | 1 + 4 files changed, 31 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 58527f151984..5df7b67ced78 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1,6 +1,7 @@ /* * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1543,9 +1544,21 @@ int __drm_atomic_helper_set_config(struct drm_mode_set *set, } EXPORT_SYMBOL(__drm_atomic_helper_set_config); -void drm_atomic_print_state(const struct drm_atomic_state *state) +/** + * drm_atomic_print_new_state - prints drm atomic state + * @state: atomic configuration to check + * @p: drm printer + * + * This functions prints the drm atomic state snapshot using the drm printer + * which is passed to it. This snapshot can be used for debugging purposes. + * + * Note that this function looks into the new state objects and hence its not + * safe to be used after the call to drm_atomic_helper_commit_hw_done(). + * + */ +void drm_atomic_print_new_state(const struct drm_atomic_state *state, + struct drm_printer *p) { - struct drm_printer p = drm_info_printer(state->dev->dev); struct drm_plane *plane; struct drm_plane_state *plane_state; struct drm_crtc *crtc; @@ -1554,17 +1567,23 @@ void drm_atomic_print_state(const struct drm_atomic_state *state) struct drm_connector_state *connector_state; int i; + if (!p) { + DRM_ERROR("invalid drm printer\n"); + return; + } + DRM_DEBUG_ATOMIC("checking %p\n", state); for_each_new_plane_in_state(state, plane, plane_state, i) - drm_atomic_plane_print_state(, plane_state); + drm_atomic_plane_print_state(p, plane_state); for_each_new_crtc_in_state(state, crtc, crtc_state, i) - drm_atomic_crtc_print_state(, crtc_state); + drm_atomic_crtc_print_state(p, crtc_state); for_each_new_connector_in_state(state, connector, connector_state, i) - drm_atomic_connector_print_state(, connector_state); + drm_atomic_connector_print_state(p, connector_state); } +EXPORT_SYMBOL(drm_atomic_print_new_state); static void __drm_state_dump(struct drm_device *dev, struct drm_printer *p, bool take_locks) diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c index 25c269bc4681..b4b3cb28a8ea 100644 --- a/drivers/gpu/drm/drm_atomic_uapi.c +++ b/drivers/gpu/drm/drm_atomic_uapi.c @@ -2,6 +2,7 @@ * Copyright (C) 2014 Red Hat * Copyright (C) 2014 Intel Corp. * Copyright (C) 2018 Intel Corp. + * Copyright (c) 2020, The Linux Foundation. All rights reserved. * * Permission is hereby granted, free of charge, to any person obtaining a * copy of this software and associated documentation files (the "Software"), @@ -1294,6 +1295,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, struct drm_out_fence_state *fence_state; int ret = 0; unsigned int i, j, num_fences; + struct drm_printer p = drm_info_printer(dev->dev); /* disallow for drivers not supporting atomic: */ if (!drm_core_check_feature(dev, DRIVER_ATOMIC)) @@ -1413,7 +1415,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, ret = drm_atomic_nonblocking_commit(state); } else { if (drm_debug_enabled(DRM_UT_STATE)) - drm_atomic_print_state(state); + drm_atomic_print_new_state(state, ); ret = drm_atomic_commit(state); } diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h index da96b2f64d7e..2bd56ec9fb0e 100644 --- a/drivers/gpu/drm/drm_crtc_internal.h +++ b/drivers/gpu/drm/drm_crtc_internal.h @@ -5,6 +5,7 @@ * Jesse Barnes * Copyright © 2014 Intel Corporation * Daniel Vetter + * Copyright (c) 2020, The Linux
[PATCH v2 0/4] Add devcoredump support for DPU
This series adds support to use devcoredump for DPU driver. It introduces the dpu_dbg module which assists in the capturing of register dumps during error scenarios. When a display related error happens, the dpu_dbg module captures all the relevant register dumps along with the snapshot of the drm atomic state and triggers a devcoredump. changes in v2: - Fix kbot errors - Rename drm_atomic_print_state function and add kernel doc for it - Fix multiple dsi registration issue with dpu_dbg Abhinav Kumar (4): drm: allow drm_atomic_print_state() to accept any drm_printer drm/msm/dpu: add support to dump dpu registers drm/msm: register the base address with dpu_dbg module drm/msm/dpu: add dpu_dbg points across dpu driver drivers/gpu/drm/drm_atomic.c | 29 +- drivers/gpu/drm/drm_atomic_uapi.c | 4 +- drivers/gpu/drm/drm_crtc_internal.h | 4 +- drivers/gpu/drm/msm/Makefile | 2 + drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c | 316 ++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h | 273 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c | 314 + drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 12 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 5 +- .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 5 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h| 4 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_ctl.c| 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_dspp.c | 7 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 5 +- .../gpu/drm/msm/disp/dpu1/dpu_hw_pingpong.c | 6 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_sspp.c | 8 +- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_top.c| 7 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c | 12 +- drivers/gpu/drm/msm/disp/dpu1/dpu_kms.h | 4 +- drivers/gpu/drm/msm/dp/dp_catalog.c | 12 + drivers/gpu/drm/msm/dp/dp_catalog.h | 4 + drivers/gpu/drm/msm/dp/dp_display.c | 2 + drivers/gpu/drm/msm/dsi/dsi.c | 1 + drivers/gpu/drm/msm/dsi/dsi.h | 1 + drivers/gpu/drm/msm/dsi/dsi_host.c| 15 +- drivers/gpu/drm/msm/msm_drv.c | 30 +- drivers/gpu/drm/msm/msm_drv.h | 3 +- .../gpu/drm/selftests/test-drm_framebuffer.c | 1 + 28 files changed, 1066 insertions(+), 26 deletions(-) create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.c create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg.h create mode 100644 drivers/gpu/drm/msm/disp/dpu1/dpu_dbg_util.c -- 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
[PULL] drm-intel-fixes
Hi Dave and Daniel, Only 3 patches on this first round. I had blocked a few patches because CI results were strange and I had blocked GVT pull request for having a bad-formatted commit message. So we might see a bigger number of patches next week. Thanks, Rodrigo. drm-intel-fixes-2020-10-29: - Fix max memory region size calculation (Matt) - Restore ILK-M RPS support, restoring performance (Ville) - Reject 90/270 degreerotated initial fbs (Ville) The following changes since commit 3650b228f83adda7e5ee532e2b90429c03f7b9ec: Linux 5.10-rc1 (2020-10-25 15:14:11 -0700) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-intel tags/drm-intel-fixes-2020-10-29 for you to fetch changes up to 61334ed227a5852100115180f5535b1396ed5227: drm/i915: Reject 90/270 degree rotated initial fbs (2020-10-29 14:20:24 -0400) - Fix max memory region size calculation (Matt) - Restore ILK-M RPS support, restoring performance (Ville) - Reject 90/270 degreerotated initial fbs (Ville) Matthew Auld (1): drm/i915/region: fix max size calculation Ville Syrjälä (2): drm/i915: Restore ILK-M RPS support drm/i915: Reject 90/270 degree rotated initial fbs drivers/gpu/drm/i915/display/intel_display.c | 4 ++ drivers/gpu/drm/i915/i915_pci.c| 1 + drivers/gpu/drm/i915/intel_memory_region.c | 2 +- .../gpu/drm/i915/selftests/intel_memory_region.c | 77 ++ drivers/gpu/drm/i915/selftests/mock_region.c | 2 +- 5 files changed, 84 insertions(+), 2 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers
Hi Daniel, I love your patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master v5.10-rc1 next-20201029] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: powerpc-randconfig-r015-20201029 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 772aaa602383cf82795572ebcd86b0e660f3579f) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install powerpc cross compiling tool for clang build # apt-get install binutils-powerpc-linux-gnu # https://github.com/0day-ci/linux/commit/188b22d2b66860695df5d07bf2b7115976790b2c git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618 git checkout 188b22d2b66860695df5d07bf2b7115976790b2c # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/qxl/qxl_drv.c:31: >> drivers/gpu/drm/qxl/qxl_drv.h:178:35: warning: declaration of 'struct >> qxl_device' will not be visible outside of this function [-Wvisibility] int qxl_debugfs_fence_init(struct qxl_device *rdev); ^ 1 warning generated. vim +178 drivers/gpu/drm/qxl/qxl_drv.h f64122c1f6ade30 Dave Airlie 2013-02-25 177 f64122c1f6ade30 Dave Airlie 2013-02-25 @178 int qxl_debugfs_fence_init(struct qxl_device *rdev); f64122c1f6ade30 Dave Airlie 2013-02-25 179 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] fbdev/sh_mobile: Drop unused include
The driver includes but doesn't use any symbols from this file. Cc: Magnus Damm Cc: Geert Uytterhoeven Cc: linux-renesas-...@vger.kernel.org Signed-off-by: Linus Walleij --- drivers/video/fbdev/sh_mobile_lcdcfb.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/video/fbdev/sh_mobile_lcdcfb.c b/drivers/video/fbdev/sh_mobile_lcdcfb.c index c1043420dbd3..027c74d7c010 100644 --- a/drivers/video/fbdev/sh_mobile_lcdcfb.c +++ b/drivers/video/fbdev/sh_mobile_lcdcfb.c @@ -16,7 +16,6 @@ #include #include #include -#include #include #include #include -- 2.26.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7] backlight: lms283gf05: Convert to GPIO descriptors
This converts the lms283gf05 backlight driver to use GPIO descriptors and switches the single PXA Palm Z2 device over to defining these. Since the platform data was only used to convey GPIO information we can delete the platform data header. Notice that we define the proper active low semantics in the board file GPIO descriptor table (active low) and assert the reset line by bringing it to "1" (asserted). Cc: Marek Vasut Cc: Daniel Mack Cc: Haojian Zhuang Cc: Robert Jarzmik Reviewed-by: Daniel Thompson Signed-off-by: Linus Walleij --- ChangeLog v6->v7: - Rebase onto v5.10-rc1 ChangeLog v5->v6: - Rebase onto v5.9-rc1 ChangeLog v4->v5: - Rebase on v5.8-rc1 - Collected Daniel's Reviewed-by tag. ChangeLog v3->v4: - Check IS_ERR() on the returned GPIO descriptor. - Unconditionally set consumer name since the API tolerates NULL. ChangeLog v2->v3: - Fix a use-before-allocated bug discovered by compile tests. - Remove unused ret variable as autobuilders complained. ChangeLog v1->v2: - Bring up the GPIO de-asserted in probe() Marek: I saw this was written by you, are you regularly testing the Z2 device? --- arch/arm/mach-pxa/z2.c | 12 +--- drivers/video/backlight/lms283gf05.c | 43 +++- include/linux/spi/lms283gf05.h | 16 --- 3 files changed, 25 insertions(+), 46 deletions(-) delete mode 100644 include/linux/spi/lms283gf05.h diff --git a/arch/arm/mach-pxa/z2.c b/arch/arm/mach-pxa/z2.c index 21fd76bb09cd..89eb5243c85f 100644 --- a/arch/arm/mach-pxa/z2.c +++ b/arch/arm/mach-pxa/z2.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include @@ -578,8 +577,13 @@ static struct pxa2xx_spi_chip lms283_chip_info = { .gpio_cs= GPIO88_ZIPITZ2_LCD_CS, }; -static const struct lms283gf05_pdata lms283_pdata = { - .reset_gpio = GPIO19_ZIPITZ2_LCD_RESET, +static struct gpiod_lookup_table lms283_gpio_table = { + .dev_id = "spi2.0", /* SPI bus 2 chip select 0 */ + .table = { + GPIO_LOOKUP("gpio-pxa", GPIO19_ZIPITZ2_LCD_RESET, + "reset", GPIO_ACTIVE_LOW), + { }, + }, }; static struct spi_board_info spi_board_info[] __initdata = { @@ -595,7 +599,6 @@ static struct spi_board_info spi_board_info[] __initdata = { { .modalias = "lms283gf05", .controller_data= _chip_info, - .platform_data = _pdata, .max_speed_hz = 40, .bus_num= 2, .chip_select= 0, @@ -615,6 +618,7 @@ static void __init z2_spi_init(void) { pxa2xx_set_spi_info(1, _ssp1_master_info); pxa2xx_set_spi_info(2, _ssp2_master_info); + gpiod_add_lookup_table(_gpio_table); spi_register_board_info(spi_board_info, ARRAY_SIZE(spi_board_info)); } #else diff --git a/drivers/video/backlight/lms283gf05.c b/drivers/video/backlight/lms283gf05.c index 0e45685bcc1c..36856962ed83 100644 --- a/drivers/video/backlight/lms283gf05.c +++ b/drivers/video/backlight/lms283gf05.c @@ -9,16 +9,16 @@ #include #include #include -#include +#include #include #include -#include #include struct lms283gf05_state { struct spi_device *spi; struct lcd_device *ld; + struct gpio_desc*reset; }; struct lms283gf05_seq { @@ -90,13 +90,13 @@ static const struct lms283gf05_seq disp_pdwnseq[] = { }; -static void lms283gf05_reset(unsigned long gpio, bool inverted) +static void lms283gf05_reset(struct gpio_desc *gpiod) { - gpio_set_value(gpio, !inverted); + gpiod_set_value(gpiod, 0); /* De-asserted */ mdelay(100); - gpio_set_value(gpio, inverted); + gpiod_set_value(gpiod, 1); /* Asserted */ mdelay(20); - gpio_set_value(gpio, !inverted); + gpiod_set_value(gpiod, 0); /* De-asserted */ mdelay(20); } @@ -125,18 +125,15 @@ static int lms283gf05_power_set(struct lcd_device *ld, int power) { struct lms283gf05_state *st = lcd_get_data(ld); struct spi_device *spi = st->spi; - struct lms283gf05_pdata *pdata = dev_get_platdata(>dev); if (power <= FB_BLANK_NORMAL) { - if (pdata) - lms283gf05_reset(pdata->reset_gpio, - pdata->reset_inverted); + if (st->reset) + lms283gf05_reset(st->reset); lms283gf05_toggle(spi, disp_initseq, ARRAY_SIZE(disp_initseq)); } else { lms283gf05_toggle(spi, disp_pdwnseq, ARRAY_SIZE(disp_pdwnseq)); - if (pdata) - gpio_set_value(pdata->reset_gpio, - pdata->reset_inverted); + if (st->reset) + gpiod_set_value(st->reset, 1); /* Asserted */ } return 0; @@ -150,24 +147,18 @@ static struct lcd_ops lms_ops = { static int
[Bug 209457] AMDGPU resume fail with RX 580 GPU
https://bugzilla.kernel.org/show_bug.cgi?id=209457 Robert M. Muncrief (rmuncr...@humanavance.com) changed: What|Removed |Added Kernel Version|5.8.12 to 5.9-rc6 |5.8.12 to 5.10-rc1 -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
linux-next: build failure after merge of the drm-misc tree
Hi all, After merging the drm-misc tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/gpu/drm/nouveau/nouveau_ttm.c: In function 'nouveau_ttm_init': drivers/gpu/drm/nouveau/nouveau_ttm.c:320:19: error: implicit declaration of function 'swiotlb_nr_tbl' [-Werror=implicit-function-declaration] 320 | need_swiotlb = !!swiotlb_nr_tbl(); | ^~ Caused by commit ee5d2a8e549e ("drm/ttm: wire up the new pool as default one v2") I have used the drm-misc tree from next-20201029 for today. -- Cheers, Stephen Rothwell pgp1yNmirzl6f.pgp Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209457] AMDGPU resume fail with RX 580 GPU
https://bugzilla.kernel.org/show_bug.cgi?id=209457 --- Comment #13 from Robert M. Muncrief (rmuncr...@humanavance.com) --- (In reply to dark_sylinc from comment #12) > Btw I reported that I was experiencing this too in my RX 560, however for me > it went away with 5.8.15 > > I think my problem was unrelated to the one in this ticket, sorry. > > Btw it may be worth writing down whether the GPU requires an extra PCIE > power plug, as this may be relevant. > My RX560 requires one (and is plugged). I have two XFX Radeon RX 580 GTS XXX cards, one for Linux and one for a KVM Windows VM. They have a single 8 pin power connector. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] nouveau-fixes 5.10
The following changes since commit 3650b228f83adda7e5ee532e2b90429c03f7b9ec: Linux 5.10-rc1 (2020-10-25 15:14:11 -0700) are available in the Git repository at: git://github.com/skeggsb/linux linux-5.10 for you to fetch changes up to d7787cc04e0a1f2043264d1550465081096bd065: drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid() (2020-10-30 09:34:13 +1000) Karol Herbst (2): drm/nouveau/gem: fix "refcount_t: underflow; use-after-free" drm/nouveau/device: fix changing endianess code to work on older GPUs Lyude Paul (3): drm/nouveau/kms/nv50-: Program notifier offset before requesting disp caps drm/nouveau/kms/nv50-: Get rid of bogus nouveau_conn_mode_valid() drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid() Ralph Campbell (1): drm/nouveau/nouveau: fix the start/end range for migration drivers/gpu/drm/nouveau/dispnv50/core.h | 2 ++ drivers/gpu/drm/nouveau/dispnv50/core507d.c | 41 +++-- drivers/gpu/drm/nouveau/dispnv50/core907d.c | 36 +++- drivers/gpu/drm/nouveau/dispnv50/core917d.c | 2 +- drivers/gpu/drm/nouveau/include/nvhw/class/cl507d.h | 5 - drivers/gpu/drm/nouveau/include/nvhw/class/cl907d.h | 4 drivers/gpu/drm/nouveau/nouveau_connector.c | 36 ++-- drivers/gpu/drm/nouveau/nouveau_dp.c| 31 +++ drivers/gpu/drm/nouveau/nouveau_gem.c | 3 ++- drivers/gpu/drm/nouveau/nouveau_svm.c | 14 +++--- drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 39 ++- 11 files changed, 145 insertions(+), 68 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209457] AMDGPU resume fail with RX 580 GPU
https://bugzilla.kernel.org/show_bug.cgi?id=209457 --- Comment #12 from dark_syl...@yahoo.com.ar --- Btw I reported that I was experiencing this too in my RX 560, however for me it went away with 5.8.15 I think my problem was unrelated to the one in this ticket, sorry. Btw it may be worth writing down whether the GPU requires an extra PCIE power plug, as this may be relevant. My RX560 requires one (and is plugged). -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209457] AMDGPU resume fail with RX 580 GPU
https://bugzilla.kernel.org/show_bug.cgi?id=209457 --- Comment #11 from Robert M. Muncrief (rmuncr...@humanavance.com) --- This bug still exists in kernel 5.10-rc1. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/5] drm/tidss: Set bus_format correctly from bridge/connector
Hi Nikhil, Thank you for the patch. On Fri, Oct 16, 2020 at 04:09:14PM +0530, Nikhil Devshatwar wrote: > When there is a chain of bridges attached to the encoder, > the bus_format should be ideally set from the input format of the > first bridge in the chain. > > Use the bridge state to get the negotiated bus_format. > If the bridge does not support format negotiation, error out > and fail. > > Signed-off-by: Nikhil Devshatwar > --- > drivers/gpu/drm/tidss/tidss_encoder.c | 16 +++- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/tidss/tidss_encoder.c > b/drivers/gpu/drm/tidss/tidss_encoder.c > index e278a9c89476..ae7f134754b7 100644 > --- a/drivers/gpu/drm/tidss/tidss_encoder.c > +++ b/drivers/gpu/drm/tidss/tidss_encoder.c > @@ -22,6 +22,7 @@ static int tidss_encoder_atomic_check(struct drm_encoder > *encoder, > struct drm_device *ddev = encoder->dev; > struct tidss_crtc_state *tcrtc_state = to_tidss_crtc_state(crtc_state); > struct drm_display_info *di = _state->connector->display_info; > + struct drm_bridge_state *bstate; > struct drm_bridge *bridge; > bool bus_flags_set = false; > > @@ -41,14 +42,19 @@ static int tidss_encoder_atomic_check(struct drm_encoder > *encoder, > break; > } > > - if (!di->bus_formats || di->num_bus_formats == 0) { > - dev_err(ddev->dev, "%s: No bus_formats in connected display\n", > - __func__); > + /* Copy the bus_format from the input_bus_format of first bridge */ > + bridge = drm_bridge_chain_get_first_bridge(encoder); > + bstate = drm_atomic_get_new_bridge_state(crtc_state->state, bridge); > + if (bstate) > + tcrtc_state->bus_format = bstate->input_bus_cfg.format; > + > + if (tcrtc_state->bus_format == 0 || > + tcrtc_state->bus_format == MEDIA_BUS_FMT_FIXED) { > + > + dev_err(ddev->dev, "Bridge connected to the encoder did not > specify media bus format\n"); > return -EINVAL; > } > > - // XXX any cleaner way to set bus format and flags? > - tcrtc_state->bus_format = di->bus_formats[0]; > if (!bus_flags_set) > tcrtc_state->bus_flags = di->bus_flags; Shouldn't the flags also be retrieved from the bridge state ? > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/5] drm/tidss: Move to newer connector model
Hi Nikhil, Thank you for the patch. On Fri, Oct 16, 2020 at 04:09:13PM +0530, Nikhil Devshatwar wrote: > To be able to support connector operations across multiple > bridges, it is recommended that the connector should be > created by the SoC driver instead of the bridges. > > Modify the tidss modesetting initialization sequence to > create the connector and attach bridges with flag > DRM_BRIDGE_ATTACH_NO_CONNECTOR > > Signed-off-by: Nikhil Devshatwar > --- > drivers/gpu/drm/tidss/tidss_drv.h | 3 +++ > drivers/gpu/drm/tidss/tidss_kms.c | 15 ++- > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/tidss/tidss_drv.h > b/drivers/gpu/drm/tidss/tidss_drv.h > index 7de4bba52e6f..cfbf85a4d92b 100644 > --- a/drivers/gpu/drm/tidss/tidss_drv.h > +++ b/drivers/gpu/drm/tidss/tidss_drv.h > @@ -27,6 +27,9 @@ struct tidss_device { > unsigned int num_planes; > struct drm_plane *planes[TIDSS_MAX_PLANES]; > > + unsigned int num_connectors; > + struct drm_connector *connectors[TIDSS_MAX_PORTS]; > + > spinlock_t wait_lock; /* protects the irq masks */ > dispc_irq_t irq_mask; /* enabled irqs in addition to wait_list */ > }; > diff --git a/drivers/gpu/drm/tidss/tidss_kms.c > b/drivers/gpu/drm/tidss/tidss_kms.c > index 09485c7f0d6f..51c24b4a6a21 100644 > --- a/drivers/gpu/drm/tidss/tidss_kms.c > +++ b/drivers/gpu/drm/tidss/tidss_kms.c > @@ -7,6 +7,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -192,6 +193,7 @@ static int tidss_dispc_modeset_init(struct tidss_device > *tidss) > for (i = 0; i < num_pipes; ++i) { > struct tidss_plane *tplane; > struct tidss_crtc *tcrtc; > + struct drm_connector *connector; > struct drm_encoder *enc; > u32 hw_plane_id = feat->vid_order[tidss->num_planes]; > int ret; > @@ -222,11 +224,22 @@ static int tidss_dispc_modeset_init(struct tidss_device > *tidss) > return PTR_ERR(enc); > } > > - ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, 0); > + ret = drm_bridge_attach(enc, pipes[i].bridge, NULL, > + DRM_BRIDGE_ATTACH_NO_CONNECTOR); > if (ret) { > dev_err(tidss->dev, "bridge attach failed: %d\n", ret); > return ret; > } > + > + connector = drm_bridge_connector_init(>ddev, enc); > + if (IS_ERR(connector)) { > + dev_err(tidss->dev, "bridge_connector create failed\n"); > + return PTR_ERR(connector); > + } > + > + tidss->connectors[tidss->num_connectors++] = connector; > + > + drm_connector_attach_encoder(connector, enc); Apart from the issue reported by Tomi, the patch looks goood to me. Fix this fixed, and the series reordered to move this to the end, Reviewed-by: Laurent Pinchart > } > > /* create overlay planes of the leftover planes */ -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/5] drm: bridge: Propagate the bus flags from bridge->timings
Hello, On Wed, Oct 28, 2020 at 08:04:53PM +0530, Nikhil Devshatwar wrote: > On 14:31-20201021, Tomi Valkeinen wrote: > > On 16/10/2020 13:39, Nikhil Devshatwar wrote: > > > When the next bridge does not specify any bus flags, use the > > > bridge->timings->input_bus_flags as fallback when propagating > > > bus flags from next bridge to current bridge. > > > > > > Signed-off-by: Nikhil Devshatwar > > > --- > > > drivers/gpu/drm/drm_bridge.c | 7 +++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > > > index 64f0effb52ac..8353723323ab 100644 > > > --- a/drivers/gpu/drm/drm_bridge.c > > > +++ b/drivers/gpu/drm/drm_bridge.c > > > @@ -975,6 +975,13 @@ drm_atomic_bridge_propagate_bus_flags(struct > > > drm_bridge *bridge, > > >* duplicate the "dummy propagation" logic. > > >*/ > > > bridge_state->input_bus_cfg.flags = output_flags; > > > + > > > + /* > > > + * Use the bridge->timings->input_bus_flags as fallback if the next > > > bridge > > > + * does not specify the flags > > > + */ > > > + if (!bridge_state->input_bus_cfg.flags) > > > + bridge_state->input_bus_cfg.flags = > > > bridge->timings->input_bus_flags; > > > > According to docs, timings can be NULL. Correct. > > And, hmm... It's too easy to get confused with these, but... If the bridge > > defines timings, and > > timings->input_bus_flags != 0, should we always pick that, even if we got > > something via > > output_flags? Logic being, if this bridge defines timings->input_bus_flags, > > it probably wants that > > to be used regardless whether we got something from the next bridge. timings->input_bus_flags is an API that predates format and flags propagation along the pipeline. I would assume that drivers that implement propagation should do so in a way that prioritize that API, and either not report flags in the timings (in which case giving priority to one or another wouldn't make a difference as both would never be provided together), or would report flags in the timings as a best effort fallback for display controller drivers that would look at them exclusively without supporting the new API. I would thus think that the flags obtained through format negotiation should be prioritized. > Got it, the flags from timings if present should be prioritized rather > than treating them as fallback. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/5] drm/bridge: mhdp8564: Support format negotiation
Hi Nikhil, Thank you for the patch. On Fri, Oct 16, 2020 at 04:09:17PM +0530, Nikhil Devshatwar wrote: > With new connector model, mhdp bridge will not create the connector and > SoC driver will rely on format negotiation to setup the encoder format. > > Support format negotiations hooks in the drm_bridge_funcs. > Support a single format for input. > > Signed-off-by: Nikhil Devshatwar > --- > .../drm/bridge/cadence/cdns-mhdp8546-core.c | 29 +++ > 1 file changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > index d0c65610ebb5..230f6e28f82f 100644 > --- a/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > +++ b/drivers/gpu/drm/bridge/cadence/cdns-mhdp8546-core.c > @@ -2078,6 +2078,34 @@ cdns_mhdp_bridge_atomic_reset(struct drm_bridge > *bridge) > return _mhdp_state->base; > } > > +static u32 *cdns_mhdp_get_input_bus_fmts(struct drm_bridge *bridge, > + struct drm_bridge_state *bridge_state, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state, > + u32 output_fmt, > + unsigned int *num_input_fmts) > +{ > + u32 *input_fmts; > + u32 default_bus_format = MEDIA_BUS_FMT_RGB121212_1X36; Display port supports quite a few different formats. See my reply to 4/5, except it's worse in the DP case :-) Especially given that multiple displays need to be taken into account. I'm afraid we need to decide how to map media bus formats to different DP use cases, possibly adding new bus formats as part of this exercise, and then revisit this patch. > + > + *num_input_fmts = 0; > + > + /* > + * This bridge does not support media_bus_format conversion > + * Propagate only if supported > + */ > + if (output_fmt != default_bus_format && output_fmt != > MEDIA_BUS_FMT_FIXED) > + return NULL; > + > + input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL); > + if (!input_fmts) > + return NULL; > + > + *num_input_fmts = 1; > + input_fmts[0] = default_bus_format; > + return input_fmts; > +} > + > static int cdns_mhdp_atomic_check(struct drm_bridge *bridge, > struct drm_bridge_state *bridge_state, > struct drm_crtc_state *crtc_state, > @@ -2142,6 +2170,7 @@ static const struct drm_bridge_funcs > cdns_mhdp_bridge_funcs = { > .atomic_duplicate_state = cdns_mhdp_bridge_atomic_duplicate_state, > .atomic_destroy_state = cdns_mhdp_bridge_atomic_destroy_state, > .atomic_reset = cdns_mhdp_bridge_atomic_reset, > + .atomic_get_input_bus_fmts = cdns_mhdp_get_input_bus_fmts, > .detect = cdns_mhdp_bridge_detect, > .get_edid = cdns_mhdp_bridge_get_edid, > .hpd_enable = cdns_mhdp_bridge_hpd_enable, -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/5] drm/bridge: tfp410: Support format negotiation
Hi Nikhil, Thank you for the patch. On Fri, Oct 16, 2020 at 04:09:16PM +0530, Nikhil Devshatwar wrote: > With new connector model, tfp410 will not create the connector and > SoC driver will rely on format negotiation to setup the encoder format. > > Support format negotiations hooks in the drm_bridge_funcs. > Use helper functions for state management. > Support one of the two RGB formats as selected from DT bindings. > > Signed-off-by: Nikhil Devshatwar > --- > drivers/gpu/drm/bridge/ti-tfp410.c | 32 ++ > 1 file changed, 32 insertions(+) > > diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c > b/drivers/gpu/drm/bridge/ti-tfp410.c > index ba3fa2a9b8a4..b65e48e080c7 100644 > --- a/drivers/gpu/drm/bridge/ti-tfp410.c > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > @@ -204,12 +204,44 @@ static enum drm_mode_status tfp410_mode_valid(struct > drm_bridge *bridge, > return MODE_OK; > } > > +static u32 *tfp410_get_input_bus_fmts(struct drm_bridge *bridge, > + struct drm_bridge_state *bridge_state, > + struct drm_crtc_state *crtc_state, > + struct drm_connector_state *conn_state, > + u32 output_fmt, > + unsigned int *num_input_fmts) > +{ > + struct tfp410 *dvi = drm_bridge_to_tfp410(bridge); > + u32 *input_fmts; > + > + *num_input_fmts = 0; > + > + /* > + * This bridge does not support media_bus_format conversion > + * Propagate only if supported > + */ > + if (output_fmt != dvi->bus_format && output_fmt != MEDIA_BUS_FMT_FIXED) > + return NULL; On the output side, DVI transmits RGB24 data over three TMDS links (plus one link for the clock). There's no media bus format that specifically describe this, but among the possible values for dvi->bus_format (MEDIA_BUS_FMT_RGB888_2X12_LE and MEDIA_BUS_FMT_RGB888_1X24), MEDIA_BUS_FMT_RGB888_2X12_LE doesn't make any sense to describe the output. We probably would need to introduce a specific media bus format if we wanted to describe this accurately (MEDIA_BUS_FMT_RGB888_DVI_SINGLE ?), which seems a bit overkill to support single link DVI. If we take dual link DVI into account, more bit depths are supported, as well as faster rates by transmitting to RGB888 pixels per clock, so more codes would be needed. With support for single-link DVI only, we could decide, subsystem-wide, to always use MEDIA_BUS_FMT_FIXED for DVI. We could also decide to additional accept MEDIA_BUS_FMT_RGB888_1X24 to describe single-link DVI, as a convention. If the goal of the above check is to make the format negotiation fail when the desired output format can't be supported by the TFP410, then I would use if (output_fmt != dvi->MEDIA_BUS_FMT_RGB888_1X24 && output_fmt != MEDIA_BUS_FMT_FIXED) return NULL; or simply if (output_fmt != MEDIA_BUS_FMT_FIXED) return NULL; depending on what convention we enforce in the subsystem for DVI media bus formats. I however wonder if this is needed at all, are there cases where the output could support multiple bus formats and the tfp410 driver would need to make sure that only the 24-bit single link DVI gets selected ? I suppose there are if we consider dual link DVI, but the DVI connector driver (drivers/gpu/drm/bridge/display-connector.c) doesn't report bus formats anyway. > + > + input_fmts = kzalloc(sizeof(*input_fmts), GFP_KERNEL); > + if (!input_fmts) > + return NULL; > + > + *num_input_fmts = 1; > + input_fmts[0] = dvi->bus_format; > + return input_fmts; > +} > + > static const struct drm_bridge_funcs tfp410_bridge_funcs = { > .attach = tfp410_attach, > .detach = tfp410_detach, > .enable = tfp410_enable, > .disable= tfp410_disable, > .mode_valid = tfp410_mode_valid, > + .atomic_reset = drm_atomic_helper_bridge_reset, > + .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state, > + .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state, > + .atomic_get_input_bus_fmts = tfp410_get_input_bus_fmts, > }; > > static const struct drm_bridge_timings tfp410_default_timings = { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 5/6] drm/kmb: Mipi DSI part of the display driver
Hi Anitha et all. On Thu, Oct 29, 2020 at 02:27:56PM -0700, Anitha Chrisanthus wrote: > Initializes Mipi DSI and sets up connects to ADV bridge > > v2: removed license text > upclassed dev_private, removed HAVE_IRQ. (Sam) > > v3: Squashed all 59 commits to one > > v4: review changes from Sam Ravnborg > renamed dev_p to kmb > > v5: corrected spellings > v6: corrected checkpatch warnings > v7: review changes Sam Ravnborg and Thomas Zimmerman > removed unnecessary logs and defines and ifdef codes (Sam) > split dphy_init_sequence smaller (Sam) > removed redundant checks in kmb_dsi (Sam) > changed kmb_dsi_init to drm_bridge_connector_init and > drm_connector_attach_encoder to bridge's connector (Sam) > v8: call drm_bridge_attach with DRM_BRIDGE_ATTACH_NO_CONNECTOR > v9: renamed kmb_dsi_hw_init to kmb_dsi_mode_set (Daniel V) > v10: changes in driver to accommodate changes in DT to separate DSI > entries (Sam R) > added comments to clarify empty dsi host functions > review changes from Sam to separate out DSI part, > removed dependencies on drm side (Sam R) > > Signed-off-by: Anitha Chrisanthus > Cc: Sam Ravnborg > Cc: Thomas Zimmermann > Cc: Daniel Vetter Like for the previous patch Anitha and I have had some private forth and back and I really like how things are now much more separated in a display driver part and a mipi<->dsi part. There are several things to improve but this is IMO something we can do after it hits drm-misc-next so it is done a bit more in the open and as smaller incremental changes. As with the previous patch I hope someone else will take a look. Sam Reviewed-by: Sam Ravnborg > --- > drivers/gpu/drm/kmb/kmb_dsi.c | 1540 > + > drivers/gpu/drm/kmb/kmb_dsi.h | 387 +++ > 2 files changed, 1927 insertions(+) > create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.c > create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.h > > diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c > new file mode 100644 > index 000..b65236f > --- /dev/null > +++ b/drivers/gpu/drm/kmb/kmb_dsi.c > @@ -0,0 +1,1540 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright © 2019-2020 Intel Corporation > + */ > + > +#include > +#include > +#include > +#include > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#include "kmb_dsi.h" > +#include "kmb_regs.h" > + > +static struct mipi_dsi_host *dsi_host; > +static struct mipi_dsi_device *dsi_device; > +static struct drm_bridge *adv_bridge; > + > +/* Default setting is 1080p, 4 lanes */ > +#define IMG_HEIGHT_LINES 1080 > +#define IMG_WIDTH_PX 1920 > +#define MIPI_TX_ACTIVE_LANES 4 > + > +static struct mipi_tx_frame_section_cfg mipi_tx_frame0_sect_cfg = { > + .width_pixels = IMG_WIDTH_PX, > + .height_lines = IMG_HEIGHT_LINES, > + .data_type = DSI_LP_DT_PPS_RGB888_24B, > + .data_mode = MIPI_DATA_MODE1, > + .dma_packed = 0 > +}; > + > +static struct mipi_tx_frame_cfg mipitx_frame0_cfg = { > + .sections[0] = _tx_frame0_sect_cfg, > + .sections[1] = NULL, > + .sections[2] = NULL, > + .sections[3] = NULL, > + .vsync_width = 5, > + .v_backporch = 36, > + .v_frontporch = 4, > + .hsync_width = 44, > + .h_backporch = 148, > + .h_frontporch = 88 > +}; > + > +static const struct mipi_tx_dsi_cfg mipitx_dsi_cfg = { > + .hfp_blank_en = 0, > + .eotp_en = 0, > + .lpm_last_vfp_line = 0, > + .lpm_first_vsa_line = 0, > + .sync_pulse_eventn = DSI_VIDEO_MODE_NO_BURST_EVENT, > + .hfp_blanking = SEND_BLANK_PACKET, > + .hbp_blanking = SEND_BLANK_PACKET, > + .hsa_blanking = SEND_BLANK_PACKET, > + .v_blanking = SEND_BLANK_PACKET, > +}; > + > +static struct mipi_ctrl_cfg mipi_tx_init_cfg = { > + .active_lanes = MIPI_TX_ACTIVE_LANES, > + .lane_rate_mbps = MIPI_TX_LANE_DATA_RATE_MBPS, > + .ref_clk_khz = MIPI_TX_REF_CLK_KHZ, > + .cfg_clk_khz = MIPI_TX_CFG_CLK_KHZ, > + .tx_ctrl_cfg = { > + .frames[0] = _frame0_cfg, > + .frames[1] = NULL, > + .frames[2] = NULL, > + .frames[3] = NULL, > + .tx_dsi_cfg = _dsi_cfg, > + .line_sync_pkt_en = 0, > + .line_counter_active = 0, > + .frame_counter_active = 0, > + .tx_always_use_hact = 1, > + .tx_hact_wait_stop = 1, > + } > +}; > + > +struct mipi_hs_freq_range_cfg { > + u16 default_bit_rate_mbps; > + u8 hsfreqrange_code; > +}; > + > +struct vco_params { > + u32 freq; > + u32 range; > + u32 divider; > +}; > + > +static const struct vco_params vco_table[] = { > + {52, 0x3f, 8}, > + {80, 0x39, 8}, > + {105, 0x2f, 4}, > + {160, 0x29, 4}, > + {210, 0x1f, 2}, > + {320, 0x19, 2}, > +
Re: [PATCH v10 6/6] drm/kmb: Build files for KeemBay Display driver
On Thu, Oct 29, 2020 at 02:27:57PM -0700, Anitha Chrisanthus wrote: > v2: Added Maintainer entry > v3: Added one more Maintainer entry > v3: drop videomode_helpers > > Cc: Sam Ravnborg > Signed-off-by: Anitha Chrisanthus > Reviewed-by: Bob Paauwe Reviewed-by: Sam Ravnborg ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 7/7] dma-buf: system_heap: Add a system-uncached heap re-using the system heap
On Thu, Oct 29, 2020 at 12:10 AM Hillf Danton wrote: > On Thu, 29 Oct 2020 00:16:24 + John Stultz wrote: > > @@ -194,6 +210,9 @@ static int system_heap_mmap(struct dma_buf *dmabuf, > > struct vm_area_struct *vma) > > struct sg_page_iter piter; > > int ret; > > > > + if (buffer->uncached) > > + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); > > + > > Wonder why you turn back to dma_mmap_wc() and friends? Sorry, can you expand on what you are proposing here instead? I'm not sure I see how dma_alloc/mmap/*_wc() quite fits here. thanks -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 4/6] drm/kmb: Add support for KeemBay Display
Hi Anitha, On Thu, Oct 29, 2020 at 02:27:55PM -0700, Anitha Chrisanthus wrote: > This is a basic KMS atomic modesetting display driver for KeemBay family of > SOCs. Driver has no 2D or 3D graphics. It calls into the ADV bridge > driver at the connector level. > > Single CRTC with LCD controller->mipi DSI->ADV bridge > > Only 1080p resolution and single plane is supported at this time. > > v10: call drm_crtc_arm_vblank_event in atomic_flush (Daniel V) > moved global vars to kmb_private and added locks (Daniel V) > changes in driver to accommodate changes in DT to separate DSI > entries (Sam R) > review changes to separate mipi DSI (Sam R) Anitha and I have had some private forth and back here and this looks now much better than what was first submitted. We could do more but this is IMO good enough to hit drm-misc-next and then we can work on any remaining issues in-tree. I would love that at least one other to take a look before we run ahead and apply it. Sam Reviewed-by: Sam Ravnborg ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 3/6] drm/kmb: Keem Bay driver register definition
Hi Anitha. On Thu, Oct 29, 2020 at 02:27:54PM -0700, Anitha Chrisanthus wrote: > Register definitions for Keem Bay display driver > > v2: removed license text (Sam) > v3: Squashed all 59 commits to one > v4: review changes from Sam Ravnborg > renamed dev_p to kmb > v5: corrected spellings > v6: corrected checkpatch warnings > v7: removed redundant definitions > v8: removed redundant definitions, clean up (Sam R) > > Signed-off-by: Anitha Chrisanthus > Cc: Sam Ravnborg > Cc: Thomas Zimmermann > Cc: Daniel Vetter Just a bunch of register noise - but looks fine. Reviewed-by: Sam Ravnborg > --- > drivers/gpu/drm/kmb/kmb_regs.h | 725 > + > 1 file changed, 725 insertions(+) > create mode 100644 drivers/gpu/drm/kmb/kmb_regs.h > > diff --git a/drivers/gpu/drm/kmb/kmb_regs.h b/drivers/gpu/drm/kmb/kmb_regs.h > new file mode 100644 > index 000..4815056 > --- /dev/null > +++ b/drivers/gpu/drm/kmb/kmb_regs.h > @@ -0,0 +1,725 @@ > +/* SPDX-License-Identifier: GPL-2.0-only > + * > + * Copyright © 2018-2020 Intel Corporation > + */ > + > +#ifndef __KMB_REGS_H__ > +#define __KMB_REGS_H__ > + > +/*** > + * LCD controller control register defines > + ***/ > +#define LCD_CONTROL (0x4 * 0x000) > +#define LCD_CTRL_PROGRESSIVE (0 << 0) > +#define LCD_CTRL_INTERLACEDBIT(0) > +#define LCD_CTRL_ENABLEBIT(1) > +#define LCD_CTRL_VL1_ENABLEBIT(2) > +#define LCD_CTRL_VL2_ENABLEBIT(3) > +#define LCD_CTRL_GL1_ENABLEBIT(4) > +#define LCD_CTRL_GL2_ENABLEBIT(5) > +#define LCD_CTRL_ALPHA_BLEND_VL1 (0 << 6) > +#define LCD_CTRL_ALPHA_BLEND_VL2 BIT(6) > +#define LCD_CTRL_ALPHA_BLEND_GL1 (2 << 6) > +#define LCD_CTRL_ALPHA_BLEND_GL2 (3 << 6) > +#define LCD_CTRL_ALPHA_TOP_VL1 (0 << 8) > +#define LCD_CTRL_ALPHA_TOP_VL2 BIT(8) > +#define LCD_CTRL_ALPHA_TOP_GL1 (2 << 8) > +#define LCD_CTRL_ALPHA_TOP_GL2 (3 << 8) > +#define LCD_CTRL_ALPHA_MIDDLE_VL1 (0 << 10) > +#define LCD_CTRL_ALPHA_MIDDLE_VL2 BIT(10) > +#define LCD_CTRL_ALPHA_MIDDLE_GL1 (2 << 10) > +#define LCD_CTRL_ALPHA_MIDDLE_GL2 (3 << 10) > +#define LCD_CTRL_ALPHA_BOTTOM_VL1 (0 << 12) > +#define LCD_CTRL_ALPHA_BOTTOM_VL2 BIT(12) > +#define LCD_CTRL_ALPHA_BOTTOM_GL1 (2 << 12) > +#define LCD_CTRL_ALPHA_BOTTOM_GL2 (3 << 12) > +#define LCD_CTRL_TIM_GEN_ENABLEBIT(14) > +#define LCD_CTRL_CONTINUOUS(0 << 15) > +#define LCD_CTRL_ONE_SHOT BIT(15) > +#define LCD_CTRL_PWM0_EN BIT(16) > +#define LCD_CTRL_PWM1_EN BIT(17) > +#define LCD_CTRL_PWM2_EN BIT(18) > +#define LCD_CTRL_OUTPUT_DISABLED (0 << 19) > +#define LCD_CTRL_OUTPUT_ENABLEDBIT(19) > +#define LCD_CTRL_BPORCH_ENABLE BIT(21) > +#define LCD_CTRL_FPORCH_ENABLE BIT(22) > +#define LCD_CTRL_PIPELINE_DMA BIT(28) > +#define LCD_CTRL_VHSYNC_IDLE_LVL BIT(31) > + > +/* interrupts */ > +#define LCD_INT_STATUS (0x4 * 0x001) > +#define LCD_INT_EOFBIT(0) > +#define LCD_INT_LINE_CMP BIT(1) > +#define LCD_INT_VERT_COMP BIT(2) > +#define LAYER0_DMA_DONEBIT(3) > +#define LAYER0_DMA_IDLEBIT(4) > +#define LAYER0_DMA_FIFO_OVERFLOW BIT(5) > +#define LAYER0_DMA_FIFO_UNDERFLOW BIT(6) > +#define LAYER0_DMA_CB_FIFO_OVERFLOWBIT(7) > +#define LAYER0_DMA_CB_FIFO_UNDERFLOW BIT(8) > +#define LAYER0_DMA_CR_FIFO_OVERFLOWBIT(9) > +#define LAYER0_DMA_CR_FIFO_UNDERFLOW BIT(10) > +#define LAYER1_DMA_DONEBIT(11) > +#define LAYER1_DMA_IDLEBIT(12) > +#define LAYER1_DMA_FIFO_OVERFLOW BIT(13) > +#define LAYER1_DMA_FIFO_UNDERFLOW BIT(14) > +#define LAYER1_DMA_CB_FIFO_OVERFLOWBIT(15) > +#define LAYER1_DMA_CB_FIFO_UNDERFLOW BIT(16) > +#define LAYER1_DMA_CR_FIFO_OVERFLOWBIT(17) > +#define LAYER1_DMA_CR_FIFO_UNDERFLOW BIT(18) > +#define LAYER2_DMA_DONEBIT(19) > +#define LAYER2_DMA_IDLEBIT(20) > +#define LAYER2_DMA_FIFO_OVERFLOW BIT(21) > +#define
Re: [PATCH v10 2/6] dt-bindings: display: bridge: Intel KeemBay DSI
Hi Anitha. On Thu, Oct 29, 2020 at 02:27:53PM -0700, Anitha Chrisanthus wrote: > This patch adds bindings for Intel KeemBay MIPI DSI > > Signed-off-by: Anitha Chrisanthus > Cc: Sam Ravnborg > Cc: Thomas Zimmermann > Cc: Daniel Vetter This again matches my understanding of the HW - good. One small nit below, with that addressed: Reviewed-by: Sam Ravnborg > --- > .../bindings/display/bridge/intel,keembay-dsi.yaml | 101 > + > 1 file changed, 101 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/intel,keembay-dsi.yaml > > diff --git > a/Documentation/devicetree/bindings/display/bridge/intel,keembay-dsi.yaml > b/Documentation/devicetree/bindings/display/bridge/intel,keembay-dsi.yaml > new file mode 100644 > index 000..4cef64e > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/intel,keembay-dsi.yaml > @@ -0,0 +1,101 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/bridge/intel,keembay-dsi.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Devicetree bindings for Intel Keem Bay mipi dsi controller > + > +maintainers: > + - Anitha Chrisanthus > + - Edmond J Dea > + > +properties: > + compatible: > +const: intel,keembay-dsi > + > + reg: > +items: > + - description: MIPI registers range > + > + reg-names: > +items: > + - const: mipi > + > + clocks: > +items: > + - description: MIPI DSI clock > + - description: MIPI DSI econfig clock > + - description: MIPI DSI config clock > + > + clock-names: > +items: > + - const: clk_mipi > + - const: clk_mipi_ecfg > + - const: clk_mipi_cfg > + > + ports: > +type: object > + > +properties: > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > + port@0: > +type: object > +description: MIPI DSI input port. > + > + port@1: > +type: object > +description: DSI output port to adv7535. Drop the mention of adv7535 - the DT decide what the port is connected to. > + > +required: > + - port@0 > + - port@1 > + > +additionalProperties: false > + > +required: > + - compatible > + - reg > + - reg-names > + - clocks > + - clock-names > + - ports > + > +additionalProperties: false > + > +examples: > + - | > +mipi-dsi@2090 { > +compatible = "intel,keembay-dsi"; > +reg = <0x2090 0x4000>; > +reg-names = "mipi"; > +clocks = <_clk 0x86>, > + <_clk 0x88>, > + <_clk 0x89>; > +clock-names = "clk_mipi", "clk_mipi_ecfg", > + "clk_mipi_cfg"; > + > +ports { > +#address-cells = <1>; > +#size-cells = <0>; > + > +port@0 { > +reg = <0>; > +dsi_in: endpoint { > +remote-endpoint = <_out>; > +}; > +}; > + > +port@1 { > +reg = <1>; > +dsi_out: endpoint { > +remote-endpoint = <_input>; > +}; > +}; > +}; > +}; > -- > 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v10 1/6] dt-bindings: display: Add support for Intel KeemBay Display
Hi Anitha. On Thu, Oct 29, 2020 at 02:27:52PM -0700, Anitha Chrisanthus wrote: > This patch adds bindings for Intel KeemBay Display > > v2: review changes from Rob Herring > v3: review changes from Sam Ravnborg (removed mipi dsi entries, and > encoder entry, connect port to dsi) > MSSCAM is part of the display submodule and its used to reset LCD > and MIPI DSI clocks, so its best to be on this device tree. > > Signed-off-by: Anitha Chrisanthus > Cc: Sam Ravnborg > Cc: Thomas Zimmermann > Cc: Daniel Vetter Looks good - and the split betwwen the display and the mipi<->dsi parts matches the understanding of the HW I have developed. Reviewed-by: Sam Ravnborg > --- > .../bindings/display/intel,keembay-display.yaml| 75 > ++ > 1 file changed, 75 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/intel,keembay-display.yaml > > diff --git > a/Documentation/devicetree/bindings/display/intel,keembay-display.yaml > b/Documentation/devicetree/bindings/display/intel,keembay-display.yaml > new file mode 100644 > index 000..8a8effe > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/intel,keembay-display.yaml > @@ -0,0 +1,75 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/display/intel,keembay-display.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Devicetree bindings for Intel Keem Bay display controller > + > +maintainers: > + - Anitha Chrisanthus > + - Edmond J Dea > + > +properties: > + compatible: > +const: intel,keembay-display > + > + reg: > +items: > + - description: LCD registers range > + - description: Msscam registers range > + > + reg-names: > +items: > + - const: lcd > + - const: msscam > + > + clocks: > +items: > + - description: LCD controller clock > + - description: pll0 clock > + > + clock-names: > +items: > + - const: clk_lcd > + - const: clk_pll0 > + > + interrupts: > +maxItems: 1 > + > + port: > +type: object > +description: Display output node to DSI. > + > +required: > + - compatible > + - reg > + - reg-names > + - clocks > + - clock-names > + - interrupts > + - port > + > +additionalProperties: false > + > +examples: > + - | > +#include > +#include > + > +display@2093 { > +compatible = "intel,keembay-display"; > +reg = <0x2093 0x3000>, > + <0x2091 0x30>; > +reg-names = "lcd", "msscam"; > +interrupts = ; > +clocks = <_clk 0x83>, > + <_clk 0x0>; > +clock-names = "clk_lcd", "clk_pll0"; > + > +port { > +disp_out: endpoint { > +remote-endpoint = <_in>; > +}; > +}; > +}; > -- > 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH][next] drm/amd/pm: fix out-of-bound read on pptable->SkuReserved
Applied. Thanks! On Wed, Oct 28, 2020 at 8:43 AM Colin King wrote: > > From: Colin Ian King > > A recent change added two uint16_t elements to PPTable_t and reduced the > uint32_t array down to 8 elements. This results in the dev_info printing > of pptable->SkuReserved[8] accessing a value that is out-of-range on > array SkuReserved. The array has been shrunk by 1 element, so remove > this extraneous dev_info message. > > Addresses-Coverity: ("Out-of-bounds read") > Fixes: 1dc3c5a95b08 ("drm/amd/pm: update driver if file for sienna cichlid") > Signed-off-by: Colin Ian King > --- > drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c > b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c > index fa3842f460fc..0600befc6e4c 100644 > --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c > +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c > @@ -2279,7 +2279,6 @@ static void sienna_cichlid_dump_pptable(struct > smu_context *smu) > dev_info(smu->adev->dev, "SkuReserved[5] = 0x%x\n", > pptable->SkuReserved[5]); > dev_info(smu->adev->dev, "SkuReserved[6] = 0x%x\n", > pptable->SkuReserved[6]); > dev_info(smu->adev->dev, "SkuReserved[7] = 0x%x\n", > pptable->SkuReserved[7]); > - dev_info(smu->adev->dev, "SkuReserved[8] = 0x%x\n", > pptable->SkuReserved[8]); > > dev_info(smu->adev->dev, "GamingClk[0] = 0x%x\n", > pptable->GamingClk[0]); > dev_info(smu->adev->dev, "GamingClk[1] = 0x%x\n", > pptable->GamingClk[1]); > -- > 2.27.0 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH -next] drm/amd/display: remove useless if/else
Applied. Thanks! Alex On Wed, Oct 28, 2020 at 2:56 PM Zou Wei wrote: > > Fix the following coccinelle report: > > ./drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c:1367:3-5: > WARNING: possible condition with no effect (if == else) > > Both branches are the same, so remove the if/else altogether. > > Fixes: 81875979f0b2 ("drm/amd/display: Remove extra pairs of parentheses in > dce_calcs.c") > Reported-by: Hulk Robot > Signed-off-by: Zou Wei > --- > drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c > b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c > index 2c6db37..e4f29cd 100644 > --- a/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c > +++ b/drivers/gpu/drm/amd/display/dc/calcs/dce_calcs.c > @@ -1364,13 +1364,10 @@ static void calculate_bandwidth( > /*if stutter and dram clock state change are gated before cursor then > the cursor latency hiding does not limit stutter or dram clock state change*/ > for (i = 0; i <= maximum_number_of_surfaces - 1; i++) { > if (data->enable[i]) { > - if > (dceip->graphics_lb_nodownscaling_multi_line_prefetching == 1) { > - data->maximum_latency_hiding[i] = > bw_add(data->minimum_latency_hiding[i], bw_mul(bw_frc_to_fixed(5, 10), > data->total_dmifmc_urgent_latency)); > - } > - else { > - /*maximum_latency_hiding(i) = > minimum_latency_hiding(i) + 1 / vsr(i) * h_total(i) / pixel_rate(i) + 0.5 * > total_dmifmc_urgent_latency*/ > - data->maximum_latency_hiding[i] = > bw_add(data->minimum_latency_hiding[i], bw_mul(bw_frc_to_fixed(5, 10), > data->total_dmifmc_urgent_latency)); > - } > + /*maximum_latency_hiding(i) = > minimum_latency_hiding(i) + 1 / vsr(i) **/ > + /* h_total(i) / pixel_rate(i) + 0.5 * > total_dmifmc_urgent_latency*/ > + data->maximum_latency_hiding[i] = > bw_add(data->minimum_latency_hiding[i], > + bw_mul(bw_frc_to_fixed(5, 10), > data->total_dmifmc_urgent_latency)); > data->maximum_latency_hiding_with_cursor[i] = > bw_min2(data->maximum_latency_hiding[i], data->cursor_latency_hiding[i]); > } > } > -- > 2.6.2 > > ___ > amd-gfx mailing list > amd-...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 5/6] drm/kmb: Mipi DSI part of the display driver
Initializes Mipi DSI and sets up connects to ADV bridge v2: removed license text upclassed dev_private, removed HAVE_IRQ. (Sam) v3: Squashed all 59 commits to one v4: review changes from Sam Ravnborg renamed dev_p to kmb v5: corrected spellings v6: corrected checkpatch warnings v7: review changes Sam Ravnborg and Thomas Zimmerman removed unnecessary logs and defines and ifdef codes (Sam) split dphy_init_sequence smaller (Sam) removed redundant checks in kmb_dsi (Sam) changed kmb_dsi_init to drm_bridge_connector_init and drm_connector_attach_encoder to bridge's connector (Sam) v8: call drm_bridge_attach with DRM_BRIDGE_ATTACH_NO_CONNECTOR v9: renamed kmb_dsi_hw_init to kmb_dsi_mode_set (Daniel V) v10: changes in driver to accommodate changes in DT to separate DSI entries (Sam R) added comments to clarify empty dsi host functions review changes from Sam to separate out DSI part, removed dependencies on drm side (Sam R) Signed-off-by: Anitha Chrisanthus Cc: Sam Ravnborg Cc: Thomas Zimmermann Cc: Daniel Vetter --- drivers/gpu/drm/kmb/kmb_dsi.c | 1540 + drivers/gpu/drm/kmb/kmb_dsi.h | 387 +++ 2 files changed, 1927 insertions(+) create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.c create mode 100644 drivers/gpu/drm/kmb/kmb_dsi.h diff --git a/drivers/gpu/drm/kmb/kmb_dsi.c b/drivers/gpu/drm/kmb/kmb_dsi.c new file mode 100644 index 000..b65236f --- /dev/null +++ b/drivers/gpu/drm/kmb/kmb_dsi.c @@ -0,0 +1,1540 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright © 2019-2020 Intel Corporation + */ + +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "kmb_dsi.h" +#include "kmb_regs.h" + +static struct mipi_dsi_host *dsi_host; +static struct mipi_dsi_device *dsi_device; +static struct drm_bridge *adv_bridge; + +/* Default setting is 1080p, 4 lanes */ +#define IMG_HEIGHT_LINES 1080 +#define IMG_WIDTH_PX 1920 +#define MIPI_TX_ACTIVE_LANES 4 + +static struct mipi_tx_frame_section_cfg mipi_tx_frame0_sect_cfg = { + .width_pixels = IMG_WIDTH_PX, + .height_lines = IMG_HEIGHT_LINES, + .data_type = DSI_LP_DT_PPS_RGB888_24B, + .data_mode = MIPI_DATA_MODE1, + .dma_packed = 0 +}; + +static struct mipi_tx_frame_cfg mipitx_frame0_cfg = { + .sections[0] = _tx_frame0_sect_cfg, + .sections[1] = NULL, + .sections[2] = NULL, + .sections[3] = NULL, + .vsync_width = 5, + .v_backporch = 36, + .v_frontporch = 4, + .hsync_width = 44, + .h_backporch = 148, + .h_frontporch = 88 +}; + +static const struct mipi_tx_dsi_cfg mipitx_dsi_cfg = { + .hfp_blank_en = 0, + .eotp_en = 0, + .lpm_last_vfp_line = 0, + .lpm_first_vsa_line = 0, + .sync_pulse_eventn = DSI_VIDEO_MODE_NO_BURST_EVENT, + .hfp_blanking = SEND_BLANK_PACKET, + .hbp_blanking = SEND_BLANK_PACKET, + .hsa_blanking = SEND_BLANK_PACKET, + .v_blanking = SEND_BLANK_PACKET, +}; + +static struct mipi_ctrl_cfg mipi_tx_init_cfg = { + .active_lanes = MIPI_TX_ACTIVE_LANES, + .lane_rate_mbps = MIPI_TX_LANE_DATA_RATE_MBPS, + .ref_clk_khz = MIPI_TX_REF_CLK_KHZ, + .cfg_clk_khz = MIPI_TX_CFG_CLK_KHZ, + .tx_ctrl_cfg = { + .frames[0] = _frame0_cfg, + .frames[1] = NULL, + .frames[2] = NULL, + .frames[3] = NULL, + .tx_dsi_cfg = _dsi_cfg, + .line_sync_pkt_en = 0, + .line_counter_active = 0, + .frame_counter_active = 0, + .tx_always_use_hact = 1, + .tx_hact_wait_stop = 1, + } +}; + +struct mipi_hs_freq_range_cfg { + u16 default_bit_rate_mbps; + u8 hsfreqrange_code; +}; + +struct vco_params { + u32 freq; + u32 range; + u32 divider; +}; + +static const struct vco_params vco_table[] = { + {52, 0x3f, 8}, + {80, 0x39, 8}, + {105, 0x2f, 4}, + {160, 0x29, 4}, + {210, 0x1f, 2}, + {320, 0x19, 2}, + {420, 0x0f, 1}, + {630, 0x09, 1}, + {1100, 0x03, 1}, + {0x, 0x01, 1}, +}; + +static const struct mipi_hs_freq_range_cfg +mipi_hs_freq_range[MIPI_DPHY_DEFAULT_BIT_RATES] = { + {.default_bit_rate_mbps = 80, .hsfreqrange_code = 0x00}, + {.default_bit_rate_mbps = 90, .hsfreqrange_code = 0x10}, + {.default_bit_rate_mbps = 100, .hsfreqrange_code = 0x20}, + {.default_bit_rate_mbps = 110, .hsfreqrange_code = 0x30}, + {.default_bit_rate_mbps = 120, .hsfreqrange_code = 0x01}, + {.default_bit_rate_mbps = 130, .hsfreqrange_code = 0x11}, + {.default_bit_rate_mbps = 140, .hsfreqrange_code = 0x21}, + {.default_bit_rate_mbps = 150, .hsfreqrange_code = 0x31}, +
[PATCH v10 3/6] drm/kmb: Keem Bay driver register definition
Register definitions for Keem Bay display driver v2: removed license text (Sam) v3: Squashed all 59 commits to one v4: review changes from Sam Ravnborg renamed dev_p to kmb v5: corrected spellings v6: corrected checkpatch warnings v7: removed redundant definitions v8: removed redundant definitions, clean up (Sam R) Signed-off-by: Anitha Chrisanthus Cc: Sam Ravnborg Cc: Thomas Zimmermann Cc: Daniel Vetter --- drivers/gpu/drm/kmb/kmb_regs.h | 725 + 1 file changed, 725 insertions(+) create mode 100644 drivers/gpu/drm/kmb/kmb_regs.h diff --git a/drivers/gpu/drm/kmb/kmb_regs.h b/drivers/gpu/drm/kmb/kmb_regs.h new file mode 100644 index 000..4815056 --- /dev/null +++ b/drivers/gpu/drm/kmb/kmb_regs.h @@ -0,0 +1,725 @@ +/* SPDX-License-Identifier: GPL-2.0-only + * + * Copyright © 2018-2020 Intel Corporation + */ + +#ifndef __KMB_REGS_H__ +#define __KMB_REGS_H__ + +/*** + *LCD controller control register defines + ***/ +#define LCD_CONTROL(0x4 * 0x000) +#define LCD_CTRL_PROGRESSIVE (0 << 0) +#define LCD_CTRL_INTERLACED BIT(0) +#define LCD_CTRL_ENABLE BIT(1) +#define LCD_CTRL_VL1_ENABLE BIT(2) +#define LCD_CTRL_VL2_ENABLE BIT(3) +#define LCD_CTRL_GL1_ENABLE BIT(4) +#define LCD_CTRL_GL2_ENABLE BIT(5) +#define LCD_CTRL_ALPHA_BLEND_VL1 (0 << 6) +#define LCD_CTRL_ALPHA_BLEND_VL2 BIT(6) +#define LCD_CTRL_ALPHA_BLEND_GL1 (2 << 6) +#define LCD_CTRL_ALPHA_BLEND_GL2 (3 << 6) +#define LCD_CTRL_ALPHA_TOP_VL1 (0 << 8) +#define LCD_CTRL_ALPHA_TOP_VL2 BIT(8) +#define LCD_CTRL_ALPHA_TOP_GL1 (2 << 8) +#define LCD_CTRL_ALPHA_TOP_GL2 (3 << 8) +#define LCD_CTRL_ALPHA_MIDDLE_VL1(0 << 10) +#define LCD_CTRL_ALPHA_MIDDLE_VL2BIT(10) +#define LCD_CTRL_ALPHA_MIDDLE_GL1(2 << 10) +#define LCD_CTRL_ALPHA_MIDDLE_GL2(3 << 10) +#define LCD_CTRL_ALPHA_BOTTOM_VL1(0 << 12) +#define LCD_CTRL_ALPHA_BOTTOM_VL2BIT(12) +#define LCD_CTRL_ALPHA_BOTTOM_GL1(2 << 12) +#define LCD_CTRL_ALPHA_BOTTOM_GL2(3 << 12) +#define LCD_CTRL_TIM_GEN_ENABLE BIT(14) +#define LCD_CTRL_CONTINUOUS (0 << 15) +#define LCD_CTRL_ONE_SHOTBIT(15) +#define LCD_CTRL_PWM0_EN BIT(16) +#define LCD_CTRL_PWM1_EN BIT(17) +#define LCD_CTRL_PWM2_EN BIT(18) +#define LCD_CTRL_OUTPUT_DISABLED (0 << 19) +#define LCD_CTRL_OUTPUT_ENABLED BIT(19) +#define LCD_CTRL_BPORCH_ENABLE BIT(21) +#define LCD_CTRL_FPORCH_ENABLE BIT(22) +#define LCD_CTRL_PIPELINE_DMABIT(28) +#define LCD_CTRL_VHSYNC_IDLE_LVL BIT(31) + +/* interrupts */ +#define LCD_INT_STATUS (0x4 * 0x001) +#define LCD_INT_EOF BIT(0) +#define LCD_INT_LINE_CMP BIT(1) +#define LCD_INT_VERT_COMPBIT(2) +#define LAYER0_DMA_DONE BIT(3) +#define LAYER0_DMA_IDLE BIT(4) +#define LAYER0_DMA_FIFO_OVERFLOW BIT(5) +#define LAYER0_DMA_FIFO_UNDERFLOWBIT(6) +#define LAYER0_DMA_CB_FIFO_OVERFLOW BIT(7) +#define LAYER0_DMA_CB_FIFO_UNDERFLOW BIT(8) +#define LAYER0_DMA_CR_FIFO_OVERFLOW BIT(9) +#define LAYER0_DMA_CR_FIFO_UNDERFLOW BIT(10) +#define LAYER1_DMA_DONE BIT(11) +#define LAYER1_DMA_IDLE BIT(12) +#define LAYER1_DMA_FIFO_OVERFLOW BIT(13) +#define LAYER1_DMA_FIFO_UNDERFLOWBIT(14) +#define LAYER1_DMA_CB_FIFO_OVERFLOW BIT(15) +#define LAYER1_DMA_CB_FIFO_UNDERFLOW BIT(16) +#define LAYER1_DMA_CR_FIFO_OVERFLOW BIT(17) +#define LAYER1_DMA_CR_FIFO_UNDERFLOW BIT(18) +#define LAYER2_DMA_DONE BIT(19) +#define LAYER2_DMA_IDLE BIT(20) +#define LAYER2_DMA_FIFO_OVERFLOW BIT(21) +#define LAYER2_DMA_FIFO_UNDERFLOWBIT(22) +#define LAYER3_DMA_DONE BIT(23) +#define LAYER3_DMA_IDLE BIT(24) +#define LAYER3_DMA_FIFO_OVERFLOW BIT(25) +#define LAYER3_DMA_FIFO_UNDERFLOWBIT(26) +#define
[PATCH v10 4/6] drm/kmb: Add support for KeemBay Display
This is a basic KMS atomic modesetting display driver for KeemBay family of SOCs. Driver has no 2D or 3D graphics. It calls into the ADV bridge driver at the connector level. Single CRTC with LCD controller->mipi DSI->ADV bridge Only 1080p resolution and single plane is supported at this time. v2: moved extern to .h, removed license text use drm_dev_init, upclassed dev_private, removed HAVE_IRQ.(Sam) v3: Squashed all 59 commits to one v4: review changes from Sam Ravnborg renamed dev_p to kmb moved clocks under kmb_clock, consolidated clk initializations use drmm functions use DRM_GEM_CMA_DRIVER_OPS_VMAP v5: corrected spellings v6: corrected checkpatch warnings v7: review changes Sam Ravnborg and Thomas Zimmerman removed kmb_crtc.h kmb_crtc_cleanup (Thomas) renamed mode_set, kmb_load, inlined unload (Thomas) moved remaining logging to drm_*(Thomas) re-orged driver initialization (Thomas) moved plane_status to drm_private (Sam) removed unnecessary logs and defines and ifdef codes (Sam) call helper_check in plane_atomic_check (Sam) renamed set to get for bpp and format functions(Sam) use drm helper functions for reset, duplicate/destroy state instead of kmb functions (Sam) removed kmb_priv from kmb_plane and removed kmb_plane_state (Sam) v8: get clk_pll0 from display node in dt v9: moved csc_coef_lcd to plane.c (Daniel Vetter) call drm_atomic_helper_shutdown in remove (Daniel V) use drm_crtc_handle_vblank (Daniel V) renamed kmb_dsi_hw_init to kmb_dsi_mode_set (Daniel V) complimentary changes to device tree changes (Rob) v10: call drm_crtc_arm_vblank_event in atomic_flush (Daniel V) moved global vars to kmb_private and added locks (Daniel V) changes in driver to accommodate changes in DT to separate DSI entries (Sam R) review changes to separate mipi DSI (Sam R) Signed-off-by: Anitha Chrisanthus Cc: Sam Ravnborg Cc: Thomas Zimmermann Cc: Daniel Vetter --- drivers/gpu/drm/kmb/kmb_crtc.c | 232 drivers/gpu/drm/kmb/kmb_drv.c | 603 drivers/gpu/drm/kmb/kmb_drv.h | 109 drivers/gpu/drm/kmb/kmb_plane.c | 490 drivers/gpu/drm/kmb/kmb_plane.h | 99 +++ 5 files changed, 1533 insertions(+) create mode 100644 drivers/gpu/drm/kmb/kmb_crtc.c create mode 100644 drivers/gpu/drm/kmb/kmb_drv.c create mode 100644 drivers/gpu/drm/kmb/kmb_drv.h create mode 100644 drivers/gpu/drm/kmb/kmb_plane.c create mode 100644 drivers/gpu/drm/kmb/kmb_plane.h diff --git a/drivers/gpu/drm/kmb/kmb_crtc.c b/drivers/gpu/drm/kmb/kmb_crtc.c new file mode 100644 index 000..6ace64e --- /dev/null +++ b/drivers/gpu/drm/kmb/kmb_crtc.c @@ -0,0 +1,232 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Copyright © 2018-2020 Intel Corporation + */ + +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "kmb_drv.h" +#include "kmb_dsi.h" +#include "kmb_plane.h" +#include "kmb_regs.h" + +struct kmb_crtc_timing { + u32 vfront_porch; + u32 vback_porch; + u32 vsync_len; + u32 hfront_porch; + u32 hback_porch; + u32 hsync_len; +}; + +static int kmb_crtc_enable_vblank(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct kmb_drm_private *kmb = to_kmb(dev); + + /* Clear interrupt */ + kmb_write_lcd(kmb, LCD_INT_CLEAR, LCD_INT_VERT_COMP); + /* Set which interval to generate vertical interrupt */ + kmb_write_lcd(kmb, LCD_VSTATUS_COMPARE, + LCD_VSTATUS_COMPARE_VSYNC); + /* Enable vertical interrupt */ + kmb_set_bitmask_lcd(kmb, LCD_INT_ENABLE, + LCD_INT_VERT_COMP); + return 0; +} + +static void kmb_crtc_disable_vblank(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct kmb_drm_private *kmb = to_kmb(dev); + + /* Clear interrupt */ + kmb_write_lcd(kmb, LCD_INT_CLEAR, LCD_INT_VERT_COMP); + /* Disable vertical interrupt */ + kmb_clr_bitmask_lcd(kmb, LCD_INT_ENABLE, + LCD_INT_VERT_COMP); +} + +static const struct drm_crtc_funcs kmb_crtc_funcs = { + .destroy = drm_crtc_cleanup, + .set_config = drm_atomic_helper_set_config, + .page_flip = drm_atomic_helper_page_flip, + .reset = drm_atomic_helper_crtc_reset, + .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state, + .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state, + .enable_vblank = kmb_crtc_enable_vblank, + .disable_vblank = kmb_crtc_disable_vblank, +}; + +static void connect_lcd_to_mipi(struct kmb_drm_private *kmb) +{ + /* DISABLE MIPI->CIF CONNECTION */ + kmb_write_msscam(kmb, MSS_MIPI_CIF_CFG, 0); + + /* ENABLE LCD->MIPI CONNECTION */ + kmb_write_msscam(kmb,
[PATCH v10 2/6] dt-bindings: display: bridge: Intel KeemBay DSI
This patch adds bindings for Intel KeemBay MIPI DSI Signed-off-by: Anitha Chrisanthus Cc: Sam Ravnborg Cc: Thomas Zimmermann Cc: Daniel Vetter --- .../bindings/display/bridge/intel,keembay-dsi.yaml | 101 + 1 file changed, 101 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/intel,keembay-dsi.yaml diff --git a/Documentation/devicetree/bindings/display/bridge/intel,keembay-dsi.yaml b/Documentation/devicetree/bindings/display/bridge/intel,keembay-dsi.yaml new file mode 100644 index 000..4cef64e --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/intel,keembay-dsi.yaml @@ -0,0 +1,101 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/bridge/intel,keembay-dsi.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Devicetree bindings for Intel Keem Bay mipi dsi controller + +maintainers: + - Anitha Chrisanthus + - Edmond J Dea + +properties: + compatible: +const: intel,keembay-dsi + + reg: +items: + - description: MIPI registers range + + reg-names: +items: + - const: mipi + + clocks: +items: + - description: MIPI DSI clock + - description: MIPI DSI econfig clock + - description: MIPI DSI config clock + + clock-names: +items: + - const: clk_mipi + - const: clk_mipi_ecfg + - const: clk_mipi_cfg + + ports: +type: object + +properties: + '#address-cells': + const: 1 + + '#size-cells': + const: 0 + + port@0: +type: object +description: MIPI DSI input port. + + port@1: +type: object +description: DSI output port to adv7535. + +required: + - port@0 + - port@1 + +additionalProperties: false + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - ports + +additionalProperties: false + +examples: + - | +mipi-dsi@2090 { +compatible = "intel,keembay-dsi"; +reg = <0x2090 0x4000>; +reg-names = "mipi"; +clocks = <_clk 0x86>, + <_clk 0x88>, + <_clk 0x89>; +clock-names = "clk_mipi", "clk_mipi_ecfg", + "clk_mipi_cfg"; + +ports { +#address-cells = <1>; +#size-cells = <0>; + +port@0 { +reg = <0>; +dsi_in: endpoint { +remote-endpoint = <_out>; +}; +}; + +port@1 { +reg = <1>; +dsi_out: endpoint { +remote-endpoint = <_input>; +}; +}; +}; +}; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 6/6] drm/kmb: Build files for KeemBay Display driver
v2: Added Maintainer entry v3: Added one more Maintainer entry v3: drop videomode_helpers Cc: Sam Ravnborg Signed-off-by: Anitha Chrisanthus Reviewed-by: Bob Paauwe --- MAINTAINERS | 7 +++ drivers/gpu/drm/Kconfig | 2 ++ drivers/gpu/drm/Makefile | 1 + drivers/gpu/drm/kmb/Kconfig | 12 drivers/gpu/drm/kmb/Makefile | 2 ++ 5 files changed, 24 insertions(+) create mode 100644 drivers/gpu/drm/kmb/Kconfig create mode 100644 drivers/gpu/drm/kmb/Makefile diff --git a/MAINTAINERS b/MAINTAINERS index bca7bda..442f0b9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8965,6 +8965,13 @@ M: Deepak Saxena S: Maintained F: drivers/char/hw_random/ixp4xx-rng.c +INTEL KEEMBAY DRM DRIVER +M: Anitha Chrisanthus +M: Edmund Dea +S: Maintained +F: Documentation/devicetree/bindings/display/intel,kmb_display.yaml +F: drivers/gpu/drm/kmb/ + INTEL MANAGEMENT ENGINE (mei) M: Tomas Winkler L: linux-ker...@vger.kernel.org diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index 64376dd..3dc293c 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -268,6 +268,8 @@ source "drivers/gpu/drm/nouveau/Kconfig" source "drivers/gpu/drm/i915/Kconfig" +source "drivers/gpu/drm/kmb/Kconfig" + config DRM_VGEM tristate "Virtual GEM provider" depends on DRM diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 8156900..fefaff4c 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -71,6 +71,7 @@ obj-$(CONFIG_DRM_AMDGPU)+= amd/amdgpu/ obj-$(CONFIG_DRM_MGA) += mga/ obj-$(CONFIG_DRM_I810) += i810/ obj-$(CONFIG_DRM_I915) += i915/ +obj-$(CONFIG_DRM_KMB_DISPLAY) += kmb/ obj-$(CONFIG_DRM_MGAG200) += mgag200/ obj-$(CONFIG_DRM_V3D) += v3d/ obj-$(CONFIG_DRM_VC4) += vc4/ diff --git a/drivers/gpu/drm/kmb/Kconfig b/drivers/gpu/drm/kmb/Kconfig new file mode 100644 index 000..2dd7e68 --- /dev/null +++ b/drivers/gpu/drm/kmb/Kconfig @@ -0,0 +1,12 @@ +config DRM_KMB_DISPLAY + tristate "INTEL KEEMBAY DISPLAY" + depends on DRM && OF && (ARM || ARM64) + depends on COMMON_CLK + select DRM_KMS_HELPER + select DRM_KMS_CMA_HELPER + select DRM_GEM_CMA_HELPER + help + Choose this option if you have Intel's KeemBay SOC which integrates + an ARM Cortex A53 CPU with an Intel Movidius VPU. + + If M is selected the module will be called kmb-drm. diff --git a/drivers/gpu/drm/kmb/Makefile b/drivers/gpu/drm/kmb/Makefile new file mode 100644 index 000..527d737 --- /dev/null +++ b/drivers/gpu/drm/kmb/Makefile @@ -0,0 +1,2 @@ +kmb-drm-y := kmb_crtc.o kmb_drv.o kmb_plane.o kmb_dsi.o +obj-$(CONFIG_DRM_KMB_DISPLAY) += kmb-drm.o -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 1/6] dt-bindings: display: Add support for Intel KeemBay Display
This patch adds bindings for Intel KeemBay Display v2: review changes from Rob Herring v3: review changes from Sam Ravnborg (removed mipi dsi entries, and encoder entry, connect port to dsi) MSSCAM is part of the display submodule and its used to reset LCD and MIPI DSI clocks, so its best to be on this device tree. Signed-off-by: Anitha Chrisanthus Cc: Sam Ravnborg Cc: Thomas Zimmermann Cc: Daniel Vetter --- .../bindings/display/intel,keembay-display.yaml| 75 ++ 1 file changed, 75 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/intel,keembay-display.yaml diff --git a/Documentation/devicetree/bindings/display/intel,keembay-display.yaml b/Documentation/devicetree/bindings/display/intel,keembay-display.yaml new file mode 100644 index 000..8a8effe --- /dev/null +++ b/Documentation/devicetree/bindings/display/intel,keembay-display.yaml @@ -0,0 +1,75 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/display/intel,keembay-display.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Devicetree bindings for Intel Keem Bay display controller + +maintainers: + - Anitha Chrisanthus + - Edmond J Dea + +properties: + compatible: +const: intel,keembay-display + + reg: +items: + - description: LCD registers range + - description: Msscam registers range + + reg-names: +items: + - const: lcd + - const: msscam + + clocks: +items: + - description: LCD controller clock + - description: pll0 clock + + clock-names: +items: + - const: clk_lcd + - const: clk_pll0 + + interrupts: +maxItems: 1 + + port: +type: object +description: Display output node to DSI. + +required: + - compatible + - reg + - reg-names + - clocks + - clock-names + - interrupts + - port + +additionalProperties: false + +examples: + - | +#include +#include + +display@2093 { +compatible = "intel,keembay-display"; +reg = <0x2093 0x3000>, + <0x2091 0x30>; +reg-names = "lcd", "msscam"; +interrupts = ; +clocks = <_clk 0x83>, + <_clk 0x0>; +clock-names = "clk_lcd", "clk_pll0"; + +port { +disp_out: endpoint { +remote-endpoint = <_in>; +}; +}; +}; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 0/6] Add support for KeemBay DRM driver
This is a new DRM driver for Intel's KeemBay SOC. The SoC couples an ARM Cortex A53 CPU with an Intel Movidius VPU. This driver is tested with the KMB EVM board which is the refernce baord for Keem Bay SOC. The SOC's display pipeline is as follows +--++-++---+ |LCD controller| -> |Mipi DSI | -> |Mipi to HDMI Converter | +--++-++---+ LCD controller and Mipi DSI transmitter are part of the SOC and mipi to HDMI converter is ADV7535 for KMB EVM board. The DRM driver is a basic KMS atomic modesetting display driver and has no 2D or 3D graphics.It calls into the ADV bridge driver at the connector level. Only 1080p resolution and single plane is supported at this time. The usecase is for debugging video and camera outputs. Device tree patches are under review here https://lore.kernel.org/linux-arm-kernel/20200708175020.194436-1-daniele.alessandre...@linux.intel.com/T/ Changes since v1: - Removed redundant license text, updated license - Rearranged include blocks - renamed global vars and removed extern in c - Used upclassing for dev_private - Used drm_dev_init in drm device create - minor cleanups Changes since v2: - squashed all commits to a single commit - logging changed to drm_info, drm_dbg etc. - used devm_drm_dev_alloc() - removed commented out sections and general cleanup Changes since v3: - renamed dev_p to kmb - moved clocks under kmb_clock, consolidated clk initializations - use drmm functions - use DRM_GEM_CMA_DRIVER_OPS_VMAP - more cleanups Changes since v4: - corrected spellings Changes since v5: - corrected checkpatch warnings/checks -Please ignore checkpatch checks on Camelcase - this is how it is named in the databook - Please ignore checkpatch warnings on misspelled for hsa, dout, widthn etc. - they are spelled as in the databook - Please ignore checkpatch checks on macro arguments reuse - its confirmed ok Changes since v6: - review changes Sam Ravnborg and Thomas Zimmerman split patch into 4 parts, part1 register definitions, part2 display driver files, part3 mipi dsi, part4 build files (Sam) removed kmb_crtc.h kmb_crtc_cleanup (Thomas) renamed mode_set, kmb_load, inlined unload (Thomas) moved remaining logging to drm_*(Thomas) re-orged driver initialization (Thomas) moved plane_status to drm_private (Sam) removed unnecessary logs and defines and ifdef codes (Sam) split dphy_init_sequence smaller (Sam) removed redundant checks in kmb_dsi (Sam) changed kmb_dsi_init to drm_bridge_connector_init and drm_connector_attach_encoder to bridge's connector (Sam) call helper_check in plane_atomic_check (Sam) renamed set to get for bpp and format functions(Sam) use drm helper functions for reset, duplicate/destroy state instead of kmb functions (Sam) removed kmb_priv from kmb_plane and removed kmb_plane_state (Sam) Changes since v7: - tested with 5.9 kernel and made the following changes get clk_pll0 from display node in dt call drm_bridge_attach with DRM_BRIDGE_ATTACH_NO_CONNECTOR Also added Maintainer entry Changes since v8: DT review changes (Rob) renamed kmb_dsi_hw_init to kmb_dsi_mode_set (Daniel V) moved csc_coef_lcd to plane.c (Daniel Vetter) call drm_atomic_helper_shutdown in remove (Daniel V) use drm_crtc_handle_vblank (Daniel V) renamed kmb_dsi_hw_init to kmb_dsi_mode_set (Daniel V) complimentary changes to device tree changes (Rob) removed redundant definitions in kmb_dsi.h Changes since v9: Review changes from Sam Ravnborg which are: DT is separated to display and Mipi DSI as per Sam suggestion and the driver has changes to reflect this separation. Also most of the MIPI DSI code is isolated and separated from the main driver, worked closely with Sam on these changes. This split is to ease review and driver is only buildable after the last patch (build files). call drm_crtc_arm_vblank_event in atomic_flush (Daniel V) moved global vars to kmb_private and added locks (Daniel V) added comments to clarify empty dsi host functions (Daniel V) Anitha Chrisanthus (6): dt-bindings: display: Add support for Intel KeemBay Display dt-bindings: display: bridge: Intel KeemBay DSI drm/kmb: Keem Bay driver register definition drm/kmb: Add support for KeemBay Display drm/kmb: Mipi DSI part of the display driver drm/kmb: Build files for KeemBay Display driver .../bindings/display/bridge/intel,keembay-dsi.yaml | 101 ++ .../bindings/display/intel,keembay-display.yaml| 75 + MAINTAINERS|7 + drivers/gpu/drm/Kconfig|2 + drivers/gpu/drm/Makefile |1 +
[Bug 209939] radeontop causes kernel panic
https://bugzilla.kernel.org/show_bug.cgi?id=209939 --- Comment #3 from Alex Deucher (alexdeuc...@gmail.com) --- Does setting amdgpu.ppfeaturemask=0x3fff on the kernel command line in grub fix it? -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/msm/dp: do not notify audio subsystem if sink doesn't support audio
For sinks that do not support audio, there is no need to notify audio subsystem of the connection event. This will make sure that audio routes only to the primary display when connected to such sinks. Signed-off-by: Abhinav Kumar --- drivers/gpu/drm/msm/dp/dp_display.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c index 4a5735564be2..d970980b0ca5 100644 --- a/drivers/gpu/drm/msm/dp/dp_display.c +++ b/drivers/gpu/drm/msm/dp/dp_display.c @@ -555,8 +555,16 @@ static int dp_connect_pending_timeout(struct dp_display_private *dp, u32 data) static void dp_display_handle_plugged_change(struct msm_dp *dp_display, bool plugged) { - if (dp_display->plugged_cb && dp_display->codec_dev) - dp_display->plugged_cb(dp_display->codec_dev, plugged); + struct dp_display_private *dp; + + dp = container_of(g_dp_display, + struct dp_display_private, dp_display); + + if (dp_display->plugged_cb && dp_display->codec_dev) { + /* notify audio subsystem only if sink supports audio */ + if (dp->audio_supported) + dp_display->plugged_cb(dp_display->codec_dev, plugged); + } } static int dp_hpd_unplug_handle(struct dp_display_private *dp, u32 data) -- 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
Re: [v4,1/3] drm/msm: Add support for GPU cooling
On Thu, Oct 29, 2020 at 01:37:19PM +0530, Akhil P Oommen wrote: > Register GPU as a devfreq cooling device so that it can be passively > cooled by the thermal framework. > > Signed-off-by: Akhil P Oommen > Reviewed-by: Matthias Kaehlcke Wait, I did not post a 'Reviewed-by' tag for this patch! I think the patch should be ok, but I'm still not super happy about the resource management involving devfreq in general (see discussion on https://patchwork.freedesktop.org/patch/394291/?series=82476=1). It's not really something introduced by this patch, but if it ever gets fixed releasing the cooling device at the end of msm_gpu_cleanup() after everything else might cause trouble. In summary, I'm supportive of landing this patch, but reluctant to 'sign it off' because of the above. In any case: Tested-by: Matthias Kaehlcke ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209939] radeontop causes kernel panic
https://bugzilla.kernel.org/show_bug.cgi?id=209939 --- Comment #2 from Janpieter Sollie (janpieter.sol...@edpnet.be) --- I am running radeontop the usual way - without arguments, default compile. amdgpu.runpm=0 has no effect -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 5/7] dma-buf: system_heap: Allocate higher order pages if available
On Thu, Oct 29, 2020 at 12:02 AM Hillf Danton wrote: > > On Thu, 29 Oct 2020 00:16:22 + John Stultz wrote: > > > > +#define HIGH_ORDER_GFP (((GFP_HIGHUSER | __GFP_ZERO | __GFP_NOWARN \ > > + | __GFP_NORETRY) & ~__GFP_RECLAIM) \ > > + | __GFP_COMP) > > +#define LOW_ORDER_GFP (GFP_HIGHUSER | __GFP_ZERO | __GFP_COMP) > > +static gfp_t order_flags[] = {HIGH_ORDER_GFP, LOW_ORDER_GFP, > > LOW_ORDER_GFP}; > > +static const unsigned int orders[] = {8, 4, 0}; > > +#define NUM_ORDERS ARRAY_SIZE(orders) > > A two-line comment helps much understand the ORDERs above if it specifies the > reasons behind the detour to HPAGE_PMD_ORDER and PAGE_ALLOC_COSTLY_ORDER. Thanks so much for the review and feedback! So yes, this was pulled from ION's system heap: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/android/ion/ion_system_heap.c#n20 But adding __GFP_COMP as that's added by ION in the pagepool code I didn't include: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/staging/android/ion/ion_page_pool.c#n146 I unfortunately don't have a lot of detail on the exact rationale (other than what I can pull from the commit log), I suspect it has to do experiential knowledge of the majority of graphics buffers being small multiples of 1M or 64K. But I do agree some rationale in a comment would be helpful, and will try to add that. As for your comment on HPAGE_PMD_ORDER (9 on arm64/arm) and PAGE_ALLOC_COSTLY_ORDER(3), I'm not totally sure I understand your question? Are you suggesting those values would be more natural orders to choose from? Thanks again! -john ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] fbcon: Disable accelerated scrolling
Am 29.10.20 um 11:14 schrieb Daniel Vetter: > So ever since syzbot discovered fbcon, we have solid proof that it's > full of bugs. And often the solution is to just delete code and remove > features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code"). > > Now the problem is that most modern-ish drivers really only treat > fbcon as an dumb kernel console until userspace takes over, and Oops > printer for some emergencies. Looking at drm drivers and the basic > vesa/efi fbdev drivers shows that only 3 drivers support any kind of > acceleration: > > - nouveau, seems to be enabled by default > - omapdrm, when a DMM remapper exists using remapper rewriting for > y/xpanning > - gma500, but that is getting deleted now for the GTT remapper trick, > and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA > flag, so unused (and could be deleted already I think). > > No other driver supportes accelerated fbcon. And fbcon is the only > user of this accel code (it's not exposed as uapi through ioctls), > which means we could garbage collect fairly enormous amounts of code > if we kill this. > > Plus because syzbot only runs on virtual hardware, and none of the > drivers for that have acceleration, we'd remove a huge gap in testing. > And there's no other even remotely comprehensive testing aside from > syzbot. > > This patch here just disables the acceleration code by always > redrawing when scrolling. The plan is that once this has been merged > for well over a year in released kernels, we can start to go around > and delete a lot of code. > > v2: > - Drop a few more unused local variables, somehow I missed the > compiler warnings (Sam) > - Fix typo in comment (Jiri) > - add a todo entry for the cleanup (Thomas) Thanks :) > > Reviewed-by: Thomas Zimmermann > Reviewed-by: Greg Kroah-Hartman > Acked-by: Sam Ravnborg > Cc: Jiri Slaby > Cc: Bartlomiej Zolnierkiewicz > Cc: Greg Kroah-Hartman > Cc: Linus Torvalds > Cc: Ben Skeggs > Cc: nouv...@lists.freedesktop.org > Cc: Tomi Valkeinen > Cc: Daniel Vetter > Cc: Jiri Slaby > Cc: "Gustavo A. R. Silva" > Cc: Tetsuo Handa > Cc: Peilin Ye > Cc: George Kennedy > Cc: Nathan Chancellor > Cc: Peter Rosin > Signed-off-by: Daniel Vetter > --- > Documentation/gpu/todo.rst | 18 ++ > drivers/video/fbdev/core/fbcon.c | 42 ++-- > 2 files changed, 25 insertions(+), 35 deletions(-) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 6b224ef14455..bec99341a904 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -277,6 +277,24 @@ Contact: Daniel Vetter, Noralf Tronnes > > Level: Advanced > > +Garbage collect fbdev scrolling acceleration > + > + > +Scroll acceleration is disabled in fbcon by hard-wiring p->scrollmode = > +SCROLL_REDRAW. There's a ton of code this will allow us to remove: > +- lots of code in fbcon.c > +- a bunch of the hooks in fbcon_ops, maybe the remaining hooks could be > called > + directly instead of the function table (with a switch on p->rotate) > +- fb_copyarea is unused after this, and can be deleted from all drivers > + > +Note that not all acceleration code can be deleted, since clearing and cursor > +support is still accelerated, which might be good candidates for further > +deletion projects. > + > +Contact: Daniel Vetter > + > +Level: Intermediate > + > idr_init_base() > --- > > diff --git a/drivers/video/fbdev/core/fbcon.c > b/drivers/video/fbdev/core/fbcon.c > index cef437817b0d..a68253485244 100644 > --- a/drivers/video/fbdev/core/fbcon.c > +++ b/drivers/video/fbdev/core/fbcon.c > @@ -1147,11 +1147,13 @@ static void fbcon_init(struct vc_data *vc, int init) > > ops->graphics = 0; > > - if ((cap & FBINFO_HWACCEL_COPYAREA) && > - !(cap & FBINFO_HWACCEL_DISABLED)) > - p->scrollmode = SCROLL_MOVE; > - else /* default to something safe */ > - p->scrollmode = SCROLL_REDRAW; > + /* > + * No more hw acceleration for fbcon. > + * > + * FIXME: Garbage collect all the now dead code after sufficient time > + * has passed. > + */ > + p->scrollmode = SCROLL_REDRAW; > > /* >* ++guenther: console.c:vc_allocate() relies on initializing > @@ -1961,45 +1963,15 @@ static void updatescrollmode(struct fbcon_display *p, > { > struct fbcon_ops *ops = info->fbcon_par; > int fh = vc->vc_font.height; > - int cap = info->flags; > - u16 t = 0; > - int ypan = FBCON_SWAP(ops->rotate, info->fix.ypanstep, > - info->fix.xpanstep); > - int ywrap = FBCON_SWAP(ops->rotate, info->fix.ywrapstep, t); > int yres = FBCON_SWAP(ops->rotate, info->var.yres, info->var.xres); > int vyres = FBCON_SWAP(ops->rotate, info->var.yres_virtual, > info->var.xres_virtual); > - int
Re: [PATCH 2/3] fbcon: Drop EXPORT_SYMBOL
Am 29.10.20 um 11:14 schrieb Daniel Vetter: > Every since > > commit 6104c37094e729f3d4ce65797002112735d49cd1 > Author: Daniel Vetter > Date: Tue Aug 1 17:32:07 2017 +0200 > > fbcon: Make fbcon a built-time depency for fbdev > > these are no longer distinct loadable modules, so exporting symbols is > kinda pointless. > > Signed-off-by: Daniel Vetter > Cc: Greg Kroah-Hartman > Cc: Jiri Slaby > Cc: Daniel Vetter > Cc: Tetsuo Handa > Cc: Helge Deller > Cc: Peilin Ye Acked-by: Thomas Zimmermann > --- > drivers/video/fbdev/core/bitblit.c | 3 --- > drivers/video/fbdev/core/fbcon_ccw.c| 1 - > drivers/video/fbdev/core/fbcon_cw.c | 1 - > drivers/video/fbdev/core/fbcon_rotate.c | 1 - > drivers/video/fbdev/core/fbcon_ud.c | 1 - > drivers/video/fbdev/core/softcursor.c | 2 -- > drivers/video/fbdev/core/tileblit.c | 2 -- > 7 files changed, 11 deletions(-) > > diff --git a/drivers/video/fbdev/core/bitblit.c > b/drivers/video/fbdev/core/bitblit.c > index 9725ecd1255b..f98e8f298bc1 100644 > --- a/drivers/video/fbdev/core/bitblit.c > +++ b/drivers/video/fbdev/core/bitblit.c > @@ -404,6 +404,3 @@ void fbcon_set_bitops(struct fbcon_ops *ops) > if (ops->rotate) > fbcon_set_rotate(ops); > } > - > -EXPORT_SYMBOL(fbcon_set_bitops); > - > diff --git a/drivers/video/fbdev/core/fbcon_ccw.c > b/drivers/video/fbdev/core/fbcon_ccw.c > index bbd869efd03b..9cd2c4b05c32 100644 > --- a/drivers/video/fbdev/core/fbcon_ccw.c > +++ b/drivers/video/fbdev/core/fbcon_ccw.c > @@ -409,4 +409,3 @@ void fbcon_rotate_ccw(struct fbcon_ops *ops) > ops->cursor = ccw_cursor; > ops->update_start = ccw_update_start; > } > -EXPORT_SYMBOL(fbcon_rotate_ccw); > diff --git a/drivers/video/fbdev/core/fbcon_cw.c > b/drivers/video/fbdev/core/fbcon_cw.c > index a34cbe8e9874..88d89fad3f05 100644 > --- a/drivers/video/fbdev/core/fbcon_cw.c > +++ b/drivers/video/fbdev/core/fbcon_cw.c > @@ -392,4 +392,3 @@ void fbcon_rotate_cw(struct fbcon_ops *ops) > ops->cursor = cw_cursor; > ops->update_start = cw_update_start; > } > -EXPORT_SYMBOL(fbcon_rotate_cw); > diff --git a/drivers/video/fbdev/core/fbcon_rotate.c > b/drivers/video/fbdev/core/fbcon_rotate.c > index ac72d4f85f7d..df6b469aa2c2 100644 > --- a/drivers/video/fbdev/core/fbcon_rotate.c > +++ b/drivers/video/fbdev/core/fbcon_rotate.c > @@ -110,4 +110,3 @@ void fbcon_set_rotate(struct fbcon_ops *ops) > break; > } > } > -EXPORT_SYMBOL(fbcon_set_rotate); > diff --git a/drivers/video/fbdev/core/fbcon_ud.c > b/drivers/video/fbdev/core/fbcon_ud.c > index 199cbc7abe35..8d5e66b1bdfb 100644 > --- a/drivers/video/fbdev/core/fbcon_ud.c > +++ b/drivers/video/fbdev/core/fbcon_ud.c > @@ -436,4 +436,3 @@ void fbcon_rotate_ud(struct fbcon_ops *ops) > ops->cursor = ud_cursor; > ops->update_start = ud_update_start; > } > -EXPORT_SYMBOL(fbcon_rotate_ud); > diff --git a/drivers/video/fbdev/core/softcursor.c > b/drivers/video/fbdev/core/softcursor.c > index fc93f254498e..29e5b21cf373 100644 > --- a/drivers/video/fbdev/core/softcursor.c > +++ b/drivers/video/fbdev/core/softcursor.c > @@ -74,5 +74,3 @@ int soft_cursor(struct fb_info *info, struct fb_cursor > *cursor) > info->fbops->fb_imageblit(info, image); > return 0; > } > - > -EXPORT_SYMBOL(soft_cursor); > diff --git a/drivers/video/fbdev/core/tileblit.c > b/drivers/video/fbdev/core/tileblit.c > index 628fe5e010c0..7539ae9040f8 100644 > --- a/drivers/video/fbdev/core/tileblit.c > +++ b/drivers/video/fbdev/core/tileblit.c > @@ -151,5 +151,3 @@ void fbcon_set_tileops(struct vc_data *vc, struct fb_info > *info) > info->tileops->fb_settile(info, ); > } > } > - > -EXPORT_SYMBOL(fbcon_set_tileops); > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_0x680DC11D530B7A23.asc Description: application/pgp-keys OpenPGP_signature Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/hisilicon: Adding a const declaration to an invariant construct
Hi Am 29.10.20 um 08:11 schrieb Tian Tao: > Some constructs cannot be changed after being assigned a value, > so add const declarations to invariant constructs. > > Signed-off-by: Tian Tao Reviewed-by: Thomas Zimmermann > --- > drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c | 2 +- > drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > index a1eabad..ef18b47 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_de.c > @@ -139,7 +139,7 @@ static const u32 channel_formats1[] = { > DRM_FORMAT_ABGR > }; > > -static struct drm_plane_funcs hibmc_plane_funcs = { > +static const struct drm_plane_funcs hibmc_plane_funcs = { > .update_plane = drm_atomic_helper_update_plane, > .disable_plane = drm_atomic_helper_disable_plane, > .destroy = drm_plane_cleanup, > diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > index 0c1b40d..fee6fe8 100644 > --- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > +++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c > @@ -369,7 +369,7 @@ static void hibmc_pci_remove(struct pci_dev *pdev) > drm_dev_put(dev); > } > > -static struct pci_device_id hibmc_pci_table[] = { > +static const struct pci_device_id hibmc_pci_table[] = { > { PCI_VDEVICE(HUAWEI, 0x1711) }, > {0,} > }; > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Felix Imendörffer OpenPGP_0x680DC11D530B7A23.asc Description: application/pgp-keys OpenPGP_signature Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Remove SCATTERLIST_MAX_SEGMENT
On Thu, Oct 29, 2020 at 7:20 PM Jason Gunthorpe wrote: > > On Wed, Oct 28, 2020 at 08:49:11PM +0100, Daniel Vetter wrote: > > On Wed, Oct 28, 2020 at 04:15:26PM -0300, Jason Gunthorpe wrote: > > > Since commit 9a40401cfa13 ("lib/scatterlist: Do not limit max_segment to > > > PAGE_ALIGNED values") the max_segment input to sg_alloc_table_from_pages() > > > does not have to be any special value. The new algorithm will always > > > create something less than what the user provides. Thus eliminate this > > > confusing constant. > > > > > > - vmwgfx should use the HW capability, not mix in the OS page size for > > > calling dma_set_max_seg_size() > > > > > > - i915 uses i915_sg_segment_size() both for sg_alloc_table_from_pages > > > and for some open coded sgl construction. This doesn't change the value > > > since rounddown(size, UINT_MAX) == SCATTERLIST_MAX_SEGMENT > > > > > > - drm_prime_pages_to_sg uses it as a default if max_segment is zero, > > > UINT_MAX is fine to use directly. > > > > > > Cc: Gerd Hoffmann > > > Cc: Daniel Vetter > > > Cc: Thomas Hellstrom > > > Cc: Qian Cai > > > Cc: "Ursulin, Tvrtko" > > > Suggested-by: Christoph Hellwig > > > Signed-off-by: Jason Gunthorpe > > > > lgtm. Do you want to push this through some other queue, or should I put > > this into drm trees? Prefer 5.10 or 5.11? > > I think DRM tree is best Ok, I'll try to remember and apply this to -next after -rc2. -rc1 is supremely busted for us, I want to wait with pulling the merge window into the -next pile until that's settled. Please ping if your patch isn't in linux-next within a week in case I forget. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
On Thu, Oct 29, 2020 at 09:59:09AM -0700, Rob Clark wrote: > On Thu, Oct 29, 2020 at 9:14 AM Daniel Vetter wrote: > > > > On Mon, Oct 26, 2020 at 10:34 AM Daniel Vetter wrote: > > > > > > On Fri, Oct 23, 2020 at 08:49:14PM -0700, Rob Clark wrote: > > > > On Fri, Oct 23, 2020 at 11:20 AM Lucas Stach > > > > wrote: > > > > > > > > > > On Fr, 2020-10-23 at 09:51 -0700, Rob Clark wrote: > > > > > > From: Rob Clark > > > > > > > > > > > > If there is only a single ring (no-preemption), everything is FIFO > > > > > > order > > > > > > and there is no need to implicit-sync. > > > > > > > > > > > > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as > > > > > > behavior > > > > > > is undefined when fences are not used to synchronize buffer usage > > > > > > across > > > > > > contexts (which is the only case where multiple different priority > > > > > > rings > > > > > > could come into play). > > > > > > > > > > Really, doesn't this break cross-device implicit sync? Okay, you may > > > > > not have many peripherals that rely on implicit sync on devices where > > > > > Adreno is usually found, but it seems rather heavy-handed. > > > > > > > > > > Wouldn't it be better to only ignore fences from your own ring context > > > > > in the implicit sync, like we do in the common DRM scheduler > > > > > (drm_sched_dependency_optimized)? > > > > > > > > we already do this.. as was discussed on an earlier iteration of this > > > > patchset > > > > > > > > But I'm not aware of any other non-gpu related implicit sync use-case > > > > (even on imx devices where display is decoupled from gpu).. I'll > > > > revert the patch if someone comes up with one, but otherwise lets let > > > > the implicit sync baggage die > > > > > > The thing is, dma_resv won't die, even if implicit sync is dead. We're > > > using internally for activity tracking and memory management. If you don't > > > set these, then we can't share generic code with msm, and I think everyone > > > inventing their own memory management is a bit a mistake. > > > > > > Now you only kill the implicit write sync stuff here, but I'm not sure > > > that's worth much since you still install all the read fences for > > > consistency. And if userspace doesn't want to be synced, they can set the > > > flag and do this on their own: I think you should be able to achieve > > > exactly the same thing in mesa. > > > > > > Aside: If you're worried about overhead, you can do O(1) submit if you > > > manage your ppgtt like amdgpu does. > > > > So just remember a use-case which is maybe a bit yucky, but it is > > actually possible to implement race-free. If you have implicit sync. > > > > There's screen-capture tool in mplayer and obs which capture your > > compositor by running getfb2 in a loop. It works, and after some > > initial screaming I realized it does actually work race-free. If you > > have implicit sync. > > > > I really don't think you can sunset this, as much as you want to. And > > sunsetting it inconsistently is probably the worst. > > For the case where you only have a single ring, as long as it is > importing the fb in to egl to read it (which it would need to do to > get a linear view), this would still all work Hm right we still have the implicit sync of the ringbuffer. At least until you add a submit scheduler to msm ... > (but I may drop this patch because it is just a micro-optimization and > seems to cause more confusion) Yeah I'd say without numbers to justify it it feels a bit on thin ice :-) -Daniel > > BR, > -R > > > > -Daniel > > > > > -Daniel > > > > > > > > > > > BR, > > > > -R > > > > > > > > > > > > > > > > > > > > > > Regards, > > > > > Lucas > > > > > > > > > > > Signed-off-by: Rob Clark > > > > > > Reviewed-by: Kristian H. Kristensen > > > > > > --- > > > > > > drivers/gpu/drm/msm/msm_gem_submit.c | 7 --- > > > > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > > > > > > b/drivers/gpu/drm/msm/msm_gem_submit.c > > > > > > index d04c349d8112..b6babc7f9bb8 100644 > > > > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > > > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > > > > > @@ -283,7 +283,7 @@ static int submit_lock_objects(struct > > > > > > msm_gem_submit *submit) > > > > > > return ret; > > > > > > } > > > > > > > > > > > > -static int submit_fence_sync(struct msm_gem_submit *submit, bool > > > > > > no_implicit) > > > > > > +static int submit_fence_sync(struct msm_gem_submit *submit, bool > > > > > > implicit_sync) > > > > > > { > > > > > > int i, ret = 0; > > > > > > > > > > > > @@ -303,7 +303,7 @@ static int submit_fence_sync(struct > > > > > > msm_gem_submit *submit, bool no_implicit) > > > > > > return ret; > > > > > > } > > > > > > > > > > > > - if (no_implicit) > > > > > > + if (!implicit_sync) > > > > > >
Re: [RFC PATCH 2/2] drm: etnaviv: Unmap gems on gem_close
On Thu, Oct 29, 2020 at 03:38:21PM +0100, Lucas Stach wrote: > Hi Guido, > > Am Donnerstag, den 29.10.2020, 15:20 +0100 schrieb Guido Günther: > > So far the unmap from gpu address space only happened when dropping the > > last ref in gem_free_object_unlocked, however that is skipped if there's > > still multiple handles to the same GEM object. > > > > Since userspace (here mesa) in the case of softpin hands back the memory > > region to the pool of available GPU virtual memory closing the handle > > via DRM_IOCTL_GEM_CLOSE this can lead to etnaviv_iommu_insert_exact > > failing later since userspace thinks the vaddr is available while the > > kernel thinks it isn't making the submit fail like > > > > [E] submit failed: -14 (No space left on device) > > (etna_cmd_stream_flush:244) > > > > Fix this by unmapping the memory via the .gem_close_object callback. > > > > Signed-off-by: Guido Günther > > --- > > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 + > > drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 + > > drivers/gpu/drm/etnaviv/etnaviv_gem.c | 32 +++ > > 3 files changed, 34 insertions(+) > > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > index a9a3afaef9a1..fdcc6405068c 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > > @@ -491,6 +491,7 @@ static struct drm_driver etnaviv_drm_driver = { > > .open = etnaviv_open, > > .postclose = etnaviv_postclose, > > .gem_free_object_unlocked = etnaviv_gem_free_object, > > + .gem_close_object = etnaviv_gem_close_object, > > .gem_vm_ops = _ops, > > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h > > b/drivers/gpu/drm/etnaviv/etnaviv_drv.h > > index 4d8dc9236e5f..2226a9af0d63 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h > > @@ -65,6 +65,7 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 > > op, > > struct drm_etnaviv_timespec *timeout); > > int etnaviv_gem_cpu_fini(struct drm_gem_object *obj); > > void etnaviv_gem_free_object(struct drm_gem_object *obj); > > +void etnaviv_gem_close_object(struct drm_gem_object *obj, struct drm_file > > *file); > > int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, > > u32 size, u32 flags, u32 *handle); > > int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file, > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > index f06e19e7be04..5aec4a23c77e 100644 > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > > @@ -515,6 +515,38 @@ static const struct etnaviv_gem_ops > > etnaviv_gem_shmem_ops = { > > .mmap = etnaviv_gem_mmap_obj, > > }; > > > > +void etnaviv_gem_close_object(struct drm_gem_object *obj, struct drm_file > > *unused) > > +{ > > + struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj); > > + struct etnaviv_vram_mapping *mapping, *tmp; > > + > > + /* Handle this via etnaviv_gem_free_object */ > > + if (obj->handle_count == 1) > > + return; > > + > > + WARN_ON(is_active(etnaviv_obj)); > > + > > + /* > > +* userspace wants to release the handle so we need to remove > > +* the mapping from the gpu's virtual address space to stay > > +* in sync. > > +*/ > > + list_for_each_entry_safe(mapping, tmp, _obj->vram_list, > > +obj_node) { > > + struct etnaviv_iommu_context *context = mapping->context; > > + > > + WARN_ON(mapping->use); > > + > > + if (context) { > > + etnaviv_iommu_unmap_gem(context, mapping); > > + etnaviv_iommu_context_put(context); > > I see the issue you are trying to fix here, but this is not a viable > fix. While userspace may close the handle, the GPU may still be > processing jobs referening the BO, so we can't just unmap it from the > address space. > > I think what we need to do here is waiting for the current jobs/fences > of the BO when we hit the case where userspace tries to assign a new > address to the BO. Basically wait for current jobs -> unamp from the > address space -> map at new userspace assigned address. Yeah was about to say the same. There's two solutions to this: - let the kernel manage the VA space. This is what amdgpu does in some cases (but still no relocations) - pipeline the VA/PTE updates in your driver, because userspace has a somewhat hard time figuring out when a buffer is done. Doing that would either at complexity or stalls when the kernel is doing all the tracking already anyway. Minimal fix is to do what Lucas explained above, but importantly with the kernel solution we have the option
[PATCH] drm/panfrost: Don't corrupt the queue mutex on open/close
The mutex within the panfrost_queue_state should have the lifetime of the queue, however it was erroneously initialised/destroyed during panfrost_job_{open,close} which is called every time a client opens/closes the drm node. Move the initialisation/destruction to panfrost_job_{init,fini} where it belongs. Fixes: 1a11a88cfd9a ("drm/panfrost: Fix job timeout handling") Signed-off-by: Steven Price Reviewed-by: Boris Brezillon --- drivers/gpu/drm/panfrost/panfrost_job.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index cfb431624eea..145ad37eda6a 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -595,6 +595,8 @@ int panfrost_job_init(struct panfrost_device *pfdev) } for (j = 0; j < NUM_JOB_SLOTS; j++) { + mutex_init(>queue[j].lock); + js->queue[j].fence_context = dma_fence_context_alloc(1); ret = drm_sched_init(>queue[j].sched, @@ -625,8 +627,10 @@ void panfrost_job_fini(struct panfrost_device *pfdev) job_write(pfdev, JOB_INT_MASK, 0); - for (j = 0; j < NUM_JOB_SLOTS; j++) + for (j = 0; j < NUM_JOB_SLOTS; j++) { drm_sched_fini(>queue[j].sched); + mutex_destroy(>queue[j].lock); + } } @@ -638,7 +642,6 @@ int panfrost_job_open(struct panfrost_file_priv *panfrost_priv) int ret, i; for (i = 0; i < NUM_JOB_SLOTS; i++) { - mutex_init(>queue[i].lock); sched = >queue[i].sched; ret = drm_sched_entity_init(_priv->sched_entity[i], DRM_SCHED_PRIORITY_NORMAL, , @@ -657,7 +660,6 @@ void panfrost_job_close(struct panfrost_file_priv *panfrost_priv) for (i = 0; i < NUM_JOB_SLOTS; i++) { drm_sched_entity_destroy(_priv->sched_entity[i]); - mutex_destroy(>queue[i].lock); /* Ensure any timeouts relating to this client have completed */ flush_delayed_work(>queue[i].sched.work_tdr); } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] drm: Quieten [zero] EDID carping
On Thu, Oct 29, 2020 at 04:44:17PM +, Chris Wilson wrote: > We have a few displays in CI that always report their EDID as a bunch of > zeroes. This is consistent behaviour, so one assumes intentional > indication of an "absent" EDID. Flagging these consistent warnings > detracts from CI. > > One option would be to ignore the zero EDIDs as intentional behaviour, > but Ville would like to keep the information available for debugging. > The simple alternative then is to reduce the loglevel for all the EDID > dumping from WARN to DEBUG so the information is present but not annoy > CI. Note that the bad EDID dumping is already only shown if > drm.debug=KMS, it's just the loglevel chosen was set to be caught by CI > if it ever occurred as it was expected to be an internal error not > external. Indeed. That makes this even less controversial. Reviewed-by: Ville Syrjälä > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2203 > Signed-off-by: Chris Wilson > Cc: Ville Syrjälä > --- > drivers/gpu/drm/drm_edid.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 631125b46e04..c7363af731b4 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -1844,7 +1844,7 @@ static void connector_bad_edid(struct drm_connector > *connector, > if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS)) > return; > > - drm_warn(connector->dev, "%s: EDID is invalid:\n", connector->name); > + drm_dbg_kms(connector->dev, "%s: EDID is invalid:\n", connector->name); > for (i = 0; i < num_blocks; i++) { > u8 *block = edid + i * EDID_LENGTH; > char prefix[20]; > @@ -1856,7 +1856,7 @@ static void connector_bad_edid(struct drm_connector > *connector, > else > sprintf(prefix, "\t[%02x] GOOD ", i); > > - print_hex_dump(KERN_WARNING, > + print_hex_dump(KERN_DEBUG, > prefix, DUMP_PREFIX_NONE, 16, 1, > block, EDID_LENGTH, false); > } > -- > 2.20.1 -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
On Thu, Oct 29, 2020 at 9:14 AM Daniel Vetter wrote: > > On Mon, Oct 26, 2020 at 10:34 AM Daniel Vetter wrote: > > > > On Fri, Oct 23, 2020 at 08:49:14PM -0700, Rob Clark wrote: > > > On Fri, Oct 23, 2020 at 11:20 AM Lucas Stach > > > wrote: > > > > > > > > On Fr, 2020-10-23 at 09:51 -0700, Rob Clark wrote: > > > > > From: Rob Clark > > > > > > > > > > If there is only a single ring (no-preemption), everything is FIFO > > > > > order > > > > > and there is no need to implicit-sync. > > > > > > > > > > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as > > > > > behavior > > > > > is undefined when fences are not used to synchronize buffer usage > > > > > across > > > > > contexts (which is the only case where multiple different priority > > > > > rings > > > > > could come into play). > > > > > > > > Really, doesn't this break cross-device implicit sync? Okay, you may > > > > not have many peripherals that rely on implicit sync on devices where > > > > Adreno is usually found, but it seems rather heavy-handed. > > > > > > > > Wouldn't it be better to only ignore fences from your own ring context > > > > in the implicit sync, like we do in the common DRM scheduler > > > > (drm_sched_dependency_optimized)? > > > > > > we already do this.. as was discussed on an earlier iteration of this > > > patchset > > > > > > But I'm not aware of any other non-gpu related implicit sync use-case > > > (even on imx devices where display is decoupled from gpu).. I'll > > > revert the patch if someone comes up with one, but otherwise lets let > > > the implicit sync baggage die > > > > The thing is, dma_resv won't die, even if implicit sync is dead. We're > > using internally for activity tracking and memory management. If you don't > > set these, then we can't share generic code with msm, and I think everyone > > inventing their own memory management is a bit a mistake. > > > > Now you only kill the implicit write sync stuff here, but I'm not sure > > that's worth much since you still install all the read fences for > > consistency. And if userspace doesn't want to be synced, they can set the > > flag and do this on their own: I think you should be able to achieve > > exactly the same thing in mesa. > > > > Aside: If you're worried about overhead, you can do O(1) submit if you > > manage your ppgtt like amdgpu does. > > So just remember a use-case which is maybe a bit yucky, but it is > actually possible to implement race-free. If you have implicit sync. > > There's screen-capture tool in mplayer and obs which capture your > compositor by running getfb2 in a loop. It works, and after some > initial screaming I realized it does actually work race-free. If you > have implicit sync. > > I really don't think you can sunset this, as much as you want to. And > sunsetting it inconsistently is probably the worst. For the case where you only have a single ring, as long as it is importing the fb in to egl to read it (which it would need to do to get a linear view), this would still all work (but I may drop this patch because it is just a micro-optimization and seems to cause more confusion) BR, -R > -Daniel > > > -Daniel > > > > > > > > BR, > > > -R > > > > > > > > > > > > > > > > > Regards, > > > > Lucas > > > > > > > > > Signed-off-by: Rob Clark > > > > > Reviewed-by: Kristian H. Kristensen > > > > > --- > > > > > drivers/gpu/drm/msm/msm_gem_submit.c | 7 --- > > > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > > > > > b/drivers/gpu/drm/msm/msm_gem_submit.c > > > > > index d04c349d8112..b6babc7f9bb8 100644 > > > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > > > > @@ -283,7 +283,7 @@ static int submit_lock_objects(struct > > > > > msm_gem_submit *submit) > > > > > return ret; > > > > > } > > > > > > > > > > -static int submit_fence_sync(struct msm_gem_submit *submit, bool > > > > > no_implicit) > > > > > +static int submit_fence_sync(struct msm_gem_submit *submit, bool > > > > > implicit_sync) > > > > > { > > > > > int i, ret = 0; > > > > > > > > > > @@ -303,7 +303,7 @@ static int submit_fence_sync(struct > > > > > msm_gem_submit *submit, bool no_implicit) > > > > > return ret; > > > > > } > > > > > > > > > > - if (no_implicit) > > > > > + if (!implicit_sync) > > > > > continue; > > > > > > > > > > ret = msm_gem_sync_object(_obj->base, > > > > > submit->ring->fctx, > > > > > @@ -774,7 +774,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, > > > > > void *data, > > > > > if (ret) > > > > > goto out; > > > > > > > > > > - ret = submit_fence_sync(submit, !!(args->flags & > > > > > MSM_SUBMIT_NO_IMPLICIT)); > > > > > + ret = submit_fence_sync(submit, (gpu->nr_rings > 1) && > > > > > +
[PATCH v2] drm: Quieten [zero] EDID carping
We have a few displays in CI that always report their EDID as a bunch of zeroes. This is consistent behaviour, so one assumes intentional indication of an "absent" EDID. Flagging these consistent warnings detracts from CI. One option would be to ignore the zero EDIDs as intentional behaviour, but Ville would like to keep the information available for debugging. The simple alternative then is to reduce the loglevel for all the EDID dumping from WARN to DEBUG so the information is present but not annoy CI. Note that the bad EDID dumping is already only shown if drm.debug=KMS, it's just the loglevel chosen was set to be caught by CI if it ever occurred as it was expected to be an internal error not external. Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/2203 Signed-off-by: Chris Wilson Cc: Ville Syrjälä --- drivers/gpu/drm/drm_edid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 631125b46e04..c7363af731b4 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1844,7 +1844,7 @@ static void connector_bad_edid(struct drm_connector *connector, if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS)) return; - drm_warn(connector->dev, "%s: EDID is invalid:\n", connector->name); + drm_dbg_kms(connector->dev, "%s: EDID is invalid:\n", connector->name); for (i = 0; i < num_blocks; i++) { u8 *block = edid + i * EDID_LENGTH; char prefix[20]; @@ -1856,7 +1856,7 @@ static void connector_bad_edid(struct drm_connector *connector, else sprintf(prefix, "\t[%02x] GOOD ", i); - print_hex_dump(KERN_WARNING, + print_hex_dump(KERN_DEBUG, prefix, DUMP_PREFIX_NONE, 16, 1, block, EDID_LENGTH, false); } -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/bridge: ti-sn65dsi86: Update reply on aux failures
Hi, On Wed, Oct 28, 2020 at 6:12 PM Stephen Boyd wrote: > > We should be setting the drm_dp_aux_msg::reply field if a NACK or a > SHORT reply happens. I don't think you update the "reply" field for SHORT, right? You just return a different size? > Update the error bit handling logic in > ti_sn_aux_transfer() to handle these cases and notify upper layers that > such errors have happened. This helps the retry logic understand that a > timeout has happened, or to shorten the read length if the panel isn't > able to handle the longest read possible. > > Cc: Douglas Anderson > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Sean Paul > Signed-off-by: Stephen Boyd > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 31 +++ > 1 file changed, 27 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > index 6b6e98ca2881..19737bc01b8f 100644 > --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c > +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c > @@ -878,6 +878,7 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux *aux, > case DP_AUX_NATIVE_READ: > case DP_AUX_I2C_READ: > regmap_write(pdata->regmap, SN_AUX_CMD_REG, request_val); > + msg->reply = 0; /* Assume it's good */ > break; > default: > return -EINVAL; > @@ -909,10 +910,32 @@ static ssize_t ti_sn_aux_transfer(struct drm_dp_aux > *aux, > ret = regmap_read(pdata->regmap, SN_AUX_CMD_STATUS_REG, ); > if (ret) > return ret; > - else if ((val & AUX_IRQ_STATUS_NAT_I2C_FAIL) > -|| (val & AUX_IRQ_STATUS_AUX_RPLY_TOUT) > -|| (val & AUX_IRQ_STATUS_AUX_SHORT)) > - return -ENXIO; > + > + if (val & AUX_IRQ_STATUS_AUX_RPLY_TOUT) { > + /* > +* The hardware tried the message seven times per the DP spec > +* but it hit a timeout. We ignore defers here because they're > +* handled in hardware. > +*/ > + return -ETIMEDOUT; > + } > + if (val & AUX_IRQ_STATUS_AUX_SHORT) { > + ret = regmap_read(pdata->regmap, SN_AUX_LENGTH_REG, ); > + if (ret) > + return ret; IIUC, your digging through the code showed that in order to fully handle the "SHORT" case you also needed to add support for "DP_AUX_I2C_WRITE_STATUS_UPDATE", right? Even without handling "DP_AUX_I2C_WRITE_STATUS_UPDATE" though, this patch seems to be an improvement and I'd support landing it. Oh, I guess one other thing: I think this is all from code inspection, right? You didn't manage to reproduce anything that would tickle one of these code paths? Might be worth mentioning, even if "after the cut"? -Doug ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
On Mon, Oct 26, 2020 at 10:34 AM Daniel Vetter wrote: > > On Fri, Oct 23, 2020 at 08:49:14PM -0700, Rob Clark wrote: > > On Fri, Oct 23, 2020 at 11:20 AM Lucas Stach wrote: > > > > > > On Fr, 2020-10-23 at 09:51 -0700, Rob Clark wrote: > > > > From: Rob Clark > > > > > > > > If there is only a single ring (no-preemption), everything is FIFO order > > > > and there is no need to implicit-sync. > > > > > > > > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior > > > > is undefined when fences are not used to synchronize buffer usage across > > > > contexts (which is the only case where multiple different priority rings > > > > could come into play). > > > > > > Really, doesn't this break cross-device implicit sync? Okay, you may > > > not have many peripherals that rely on implicit sync on devices where > > > Adreno is usually found, but it seems rather heavy-handed. > > > > > > Wouldn't it be better to only ignore fences from your own ring context > > > in the implicit sync, like we do in the common DRM scheduler > > > (drm_sched_dependency_optimized)? > > > > we already do this.. as was discussed on an earlier iteration of this > > patchset > > > > But I'm not aware of any other non-gpu related implicit sync use-case > > (even on imx devices where display is decoupled from gpu).. I'll > > revert the patch if someone comes up with one, but otherwise lets let > > the implicit sync baggage die > > The thing is, dma_resv won't die, even if implicit sync is dead. We're > using internally for activity tracking and memory management. If you don't > set these, then we can't share generic code with msm, and I think everyone > inventing their own memory management is a bit a mistake. > > Now you only kill the implicit write sync stuff here, but I'm not sure > that's worth much since you still install all the read fences for > consistency. And if userspace doesn't want to be synced, they can set the > flag and do this on their own: I think you should be able to achieve > exactly the same thing in mesa. > > Aside: If you're worried about overhead, you can do O(1) submit if you > manage your ppgtt like amdgpu does. So just remember a use-case which is maybe a bit yucky, but it is actually possible to implement race-free. If you have implicit sync. There's screen-capture tool in mplayer and obs which capture your compositor by running getfb2 in a loop. It works, and after some initial screaming I realized it does actually work race-free. If you have implicit sync. I really don't think you can sunset this, as much as you want to. And sunsetting it inconsistently is probably the worst. -Daniel > -Daniel > > > > > BR, > > -R > > > > > > > > > > > > Regards, > > > Lucas > > > > > > > Signed-off-by: Rob Clark > > > > Reviewed-by: Kristian H. Kristensen > > > > --- > > > > drivers/gpu/drm/msm/msm_gem_submit.c | 7 --- > > > > 1 file changed, 4 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c > > > > b/drivers/gpu/drm/msm/msm_gem_submit.c > > > > index d04c349d8112..b6babc7f9bb8 100644 > > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > > > @@ -283,7 +283,7 @@ static int submit_lock_objects(struct > > > > msm_gem_submit *submit) > > > > return ret; > > > > } > > > > > > > > -static int submit_fence_sync(struct msm_gem_submit *submit, bool > > > > no_implicit) > > > > +static int submit_fence_sync(struct msm_gem_submit *submit, bool > > > > implicit_sync) > > > > { > > > > int i, ret = 0; > > > > > > > > @@ -303,7 +303,7 @@ static int submit_fence_sync(struct msm_gem_submit > > > > *submit, bool no_implicit) > > > > return ret; > > > > } > > > > > > > > - if (no_implicit) > > > > + if (!implicit_sync) > > > > continue; > > > > > > > > ret = msm_gem_sync_object(_obj->base, > > > > submit->ring->fctx, > > > > @@ -774,7 +774,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, > > > > void *data, > > > > if (ret) > > > > goto out; > > > > > > > > - ret = submit_fence_sync(submit, !!(args->flags & > > > > MSM_SUBMIT_NO_IMPLICIT)); > > > > + ret = submit_fence_sync(submit, (gpu->nr_rings > 1) && > > > > + !(args->flags & MSM_SUBMIT_NO_IMPLICIT)); > > > > if (ret) > > > > goto out; > > > > > > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/bridge: ti-sn65dsi86: Read EDID blob over DDC
Hi, On Wed, Oct 28, 2020 at 6:12 PM Stephen Boyd wrote: > > Use the DDC connection to read the EDID from the eDP panel instead of > relying on the panel to tell us the modes. > > Cc: Douglas Anderson > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Sean Paul > Signed-off-by: Stephen Boyd > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 20 > 1 file changed, 20 insertions(+) Since we already did some early reviews off-list, it's not a surprise that I have no comments. ;-) Reviewed-by: Douglas Anderson ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/bridge: ti-sn65dsi86: Make polling a busy loop
Hi, On Wed, Oct 28, 2020 at 6:12 PM Stephen Boyd wrote: > > There's no reason we need to wait here to poll a register over i2c. The > i2c bus is inherently slow and delays are practically part of the > protocol because we have to wait for the device to respond to any > request for a register. Let's rely on the sleeping of the i2c controller > instead of adding any sort of delay here in the bridge driver. > > Cc: Douglas Anderson > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Sean Paul > Signed-off-by: Stephen Boyd > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Since we already did some early reviews off-list, it's not a surprise that I have no comments. ;-) Reviewed-by: Douglas Anderson ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/bridge: ti-sn65dsi86: Combine register accesses in ti_sn_aux_transfer()
Hi, On Wed, Oct 28, 2020 at 6:11 PM Stephen Boyd wrote: > > These register reads and writes are sometimes directly next to each > other in the register address space. Let's use regmap bulk read/write > APIs to get the data with one transfer instead of multiple i2c > transfers. This helps cut down on the number of transfers in the case of > something like reading an EDID where we read in blocks of 16 bytes at a > time and the last for loop here is sending an i2c transfer for each of > those 16 bytes, one at a time. Ouch! > > Cc: Douglas Anderson > Cc: Laurent Pinchart > Cc: Jonas Karlman > Cc: Jernej Skrabec > Cc: Sean Paul +Sam Ravnborg has helped land a few changes to this bridge drive in the past, so maybe good to CC him if you have future spins? > Signed-off-by: Stephen Boyd > --- > drivers/gpu/drm/bridge/ti-sn65dsi86.c | 50 --- > 1 file changed, 22 insertions(+), 28 deletions(-) Since we already did some early reviews off-list, it's not a surprise that I have no comments. ;-) Reviewed-by: Douglas Anderson ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm: Silence zero EDID carping
On Thu, Oct 29, 2020 at 04:01:23PM +, Chris Wilson wrote: > Quoting Ville Syrjälä (2020-10-29 14:07:46) > > On Thu, Oct 29, 2020 at 11:00:30AM +, Chris Wilson wrote: > > > We have a few displays in CI that always report their EDID as a bunch of > > > zeroes. This is consistent behavioud, so one assumes intentional > > > indication of an "absent" EDID. Let us treat is as such by silently > > > reporting the zero edid using connector->null_edid_counter, leaving the > > > loud carp to EDID that violate their checksums or otherwise return > > > unexpected illegal data upon reading. These are more likely to be > > > inconsistent bad connections rather than being intended. > > > > I don't think null_edid_counter is actually used by anything. > > So apart from wondering why the mode list has turned strange > > is there some way I can still see from the logs that the > > EDID has become all zeroes? > > The ones in question, it's every time we read the EDID it comes back > zero. I am betting that transient everything-is-zero rather than > spurious data is rare enough not to worry about. > > An alternative would be to pass the log level to the bad_edid dumper, or > just make it debug for even gibberish edids? I suspect debug should be good enough for this. The user is probably going to notice some problem with their display resolution if the EDID is bad/zero, so we should still get the bug report. -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH] drm: Silence zero EDID carping
Quoting Ville Syrjälä (2020-10-29 14:07:46) > On Thu, Oct 29, 2020 at 11:00:30AM +, Chris Wilson wrote: > > We have a few displays in CI that always report their EDID as a bunch of > > zeroes. This is consistent behavioud, so one assumes intentional > > indication of an "absent" EDID. Let us treat is as such by silently > > reporting the zero edid using connector->null_edid_counter, leaving the > > loud carp to EDID that violate their checksums or otherwise return > > unexpected illegal data upon reading. These are more likely to be > > inconsistent bad connections rather than being intended. > > I don't think null_edid_counter is actually used by anything. > So apart from wondering why the mode list has turned strange > is there some way I can still see from the logs that the > EDID has become all zeroes? The ones in question, it's every time we read the EDID it comes back zero. I am betting that transient everything-is-zero rather than spurious data is rare enough not to worry about. An alternative would be to pass the log level to the bad_edid dumper, or just make it debug for even gibberish edids? -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/3] MAINTAINERS: add files for Mediatek DRM drivers
Mediatek MIPI DSI phy driver is moved from drivers/gpu/drm/mediatek to drivers/phy/mediatek, so add the new folder to the Mediatek DRM drivers' information. Signed-off-by: Chun-Kuang Hu --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index e73636b75f29..14f5018c01b6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5867,6 +5867,7 @@ S:Supported F: Documentation/devicetree/bindings/display/mediatek/ F: drivers/gpu/drm/mediatek/ F: drivers/phy/mediatek/phy-mtk-hdmi* +F: drivers/phy/mediatek/phy-mtk-mipi* DRM DRIVERS FOR NVIDIA TEGRA M: Thierry Reding -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] phy: mediatek: Move mtk_mipi_dsi_phy driver into drivers/phy/mediatek folder
mtk_mipi_dsi_phy is currently placed inside mediatek drm driver, but it's more suitable to place a phy driver into phy driver folder, so move mtk_mipi_dsi_phy driver into phy driver folder. Signed-off-by: Chun-Kuang Hu --- drivers/gpu/drm/mediatek/Kconfig | 7 --- drivers/gpu/drm/mediatek/Makefile | 6 -- drivers/phy/mediatek/Kconfig | 7 +++ drivers/phy/mediatek/Makefile | 5 + .../mediatek/phy-mtk-mipi-dsi-mt8173.c}| 2 +- .../mediatek/phy-mtk-mipi-dsi-mt8183.c}| 2 +- .../mtk_mipi_tx.c => phy/mediatek/phy-mtk-mipi-dsi.c} | 2 +- .../mtk_mipi_tx.h => phy/mediatek/phy-mtk-mipi-dsi.h} | 0 8 files changed, 15 insertions(+), 16 deletions(-) rename drivers/{gpu/drm/mediatek/mtk_mt8173_mipi_tx.c => phy/mediatek/phy-mtk-mipi-dsi-mt8173.c} (99%) rename drivers/{gpu/drm/mediatek/mtk_mt8183_mipi_tx.c => phy/mediatek/phy-mtk-mipi-dsi-mt8183.c} (99%) rename drivers/{gpu/drm/mediatek/mtk_mipi_tx.c => phy/mediatek/phy-mtk-mipi-dsi.c} (99%) rename drivers/{gpu/drm/mediatek/mtk_mipi_tx.h => phy/mediatek/phy-mtk-mipi-dsi.h} (100%) diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig index 24c4890a6e65..2976d21e9a34 100644 --- a/drivers/gpu/drm/mediatek/Kconfig +++ b/drivers/gpu/drm/mediatek/Kconfig @@ -28,10 +28,3 @@ config DRM_MEDIATEK_HDMI select PHY_MTK_HDMI help DRM/KMS HDMI driver for Mediatek SoCs - -config PHY_MTK_MIPI_DSI - tristate "Mediatek MIPI-DSI-PHY Driver" - depends on ARCH_MEDIATEK && OF - select GENERIC_PHY - help - Support MIPI DSI PHY for Mediatek SoCs. diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile index baa188000543..a892edec5563 100644 --- a/drivers/gpu/drm/mediatek/Makefile +++ b/drivers/gpu/drm/mediatek/Makefile @@ -19,9 +19,3 @@ mediatek-drm-hdmi-objs := mtk_cec.o \ mtk_hdmi_ddc.o obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o - -phy-mtk-mipi-dsi-drv-objs := mtk_mipi_tx.o \ -mtk_mt8173_mipi_tx.o \ -mtk_mt8183_mipi_tx.o - -obj-$(CONFIG_PHY_MTK_MIPI_DSI) += phy-mtk-mipi-dsi-drv.o diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig index 50c5e9306e19..574b8e6398d2 100644 --- a/drivers/phy/mediatek/Kconfig +++ b/drivers/phy/mediatek/Kconfig @@ -42,3 +42,10 @@ config PHY_MTK_HDMI select GENERIC_PHY help Support HDMI PHY for Mediatek SoCs. + +config PHY_MTK_MIPI_DSI + tristate "MediaTek MIPI-DSI Driver" + depends on ARCH_MEDIATEK && OF + select GENERIC_PHY + help + Support MIPI DSI for Mediatek SoCs. diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile index 6325e38709ed..ace660fbed3a 100644 --- a/drivers/phy/mediatek/Makefile +++ b/drivers/phy/mediatek/Makefile @@ -11,3 +11,8 @@ phy-mtk-hdmi-drv-y:= phy-mtk-hdmi.o phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt2701.o phy-mtk-hdmi-drv-y += phy-mtk-hdmi-mt8173.o obj-$(CONFIG_PHY_MTK_HDMI) += phy-mtk-hdmi-drv.o + +phy-mtk-mipi-dsi-drv-y := phy-mtk-mipi-dsi.o +phy-mtk-mipi-dsi-drv-y += phy-mtk-mipi-dsi-mt8173.o +phy-mtk-mipi-dsi-drv-y += phy-mtk-mipi-dsi-mt8183.o +obj-$(CONFIG_PHY_MTK_MIPI_DSI) += phy-mtk-mipi-dsi-drv.o diff --git a/drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c b/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8173.c similarity index 99% rename from drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c rename to drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8173.c index f18db14d8b63..7a847954594f 100644 --- a/drivers/gpu/drm/mediatek/mtk_mt8173_mipi_tx.c +++ b/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8173.c @@ -4,7 +4,7 @@ * Author: jitao.shi */ -#include "mtk_mipi_tx.h" +#include "phy-mtk-mipi-dsi.h" #define MIPITX_DSI_CON 0x00 #define RG_DSI_LDOCORE_EN BIT(0) diff --git a/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c b/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8183.c similarity index 99% rename from drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c rename to drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8183.c index 9f3e55aeebb2..99108426d57c 100644 --- a/drivers/gpu/drm/mediatek/mtk_mt8183_mipi_tx.c +++ b/drivers/phy/mediatek/phy-mtk-mipi-dsi-mt8183.c @@ -4,7 +4,7 @@ * Author: jitao.shi */ -#include "mtk_mipi_tx.h" +#include "phy-mtk-mipi-dsi.h" #define MIPITX_LANE_CON0x000c #define RG_DSI_CPHY_T1DRV_EN BIT(0) diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/phy/mediatek/phy-mtk-mipi-dsi.c similarity index 99% rename from drivers/gpu/drm/mediatek/mtk_mipi_tx.c rename to drivers/phy/mediatek/phy-mtk-mipi-dsi.c index f2a892e16c27..18c481251f04 100644 ---
[PATCH 1/3] drm/mediatek: Separate mtk_mipi_tx to an independent module
mtk_mipi_tx is a part of mtk_drm module, but phy driver should be an independent module rather than be part of drm module, so separate the phy driver to an independent module. Signed-off-by: Chun-Kuang Hu --- drivers/gpu/drm/mediatek/Kconfig | 8 drivers/gpu/drm/mediatek/Makefile | 9 ++--- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 - drivers/gpu/drm/mediatek/mtk_mipi_tx.c | 3 +++ 5 files changed, 17 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/mediatek/Kconfig b/drivers/gpu/drm/mediatek/Kconfig index 65cd03a4be29..24c4890a6e65 100644 --- a/drivers/gpu/drm/mediatek/Kconfig +++ b/drivers/gpu/drm/mediatek/Kconfig @@ -13,6 +13,7 @@ config DRM_MEDIATEK select DRM_PANEL select MEMORY select MTK_SMI + select PHY_MTK_MIPI_DSI select VIDEOMODE_HELPERS help Choose this option if you have a Mediatek SoCs. @@ -27,3 +28,10 @@ config DRM_MEDIATEK_HDMI select PHY_MTK_HDMI help DRM/KMS HDMI driver for Mediatek SoCs + +config PHY_MTK_MIPI_DSI + tristate "Mediatek MIPI-DSI-PHY Driver" + depends on ARCH_MEDIATEK && OF + select GENERIC_PHY + help + Support MIPI DSI PHY for Mediatek SoCs. diff --git a/drivers/gpu/drm/mediatek/Makefile b/drivers/gpu/drm/mediatek/Makefile index 77b0fd86063d..baa188000543 100644 --- a/drivers/gpu/drm/mediatek/Makefile +++ b/drivers/gpu/drm/mediatek/Makefile @@ -10,9 +10,6 @@ mediatek-drm-y := mtk_disp_color.o \ mtk_drm_gem.o \ mtk_drm_plane.o \ mtk_dsi.o \ - mtk_mipi_tx.o \ - mtk_mt8173_mipi_tx.o \ - mtk_mt8183_mipi_tx.o \ mtk_dpi.o obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o @@ -22,3 +19,9 @@ mediatek-drm-hdmi-objs := mtk_cec.o \ mtk_hdmi_ddc.o obj-$(CONFIG_DRM_MEDIATEK_HDMI) += mediatek-drm-hdmi.o + +phy-mtk-mipi-dsi-drv-objs := mtk_mipi_tx.o \ +mtk_mt8173_mipi_tx.o \ +mtk_mt8183_mipi_tx.o + +obj-$(CONFIG_PHY_MTK_MIPI_DSI) += phy-mtk-mipi-dsi-drv.o diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.c b/drivers/gpu/drm/mediatek/mtk_drm_drv.c index 59c85c63b7cc..bad75c5be090 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.c +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.c @@ -636,7 +636,6 @@ static struct platform_driver * const mtk_drm_drivers[] = { _disp_rdma_driver, _dpi_driver, _drm_platform_driver, - _mipi_tx_driver, _dsi_driver, }; diff --git a/drivers/gpu/drm/mediatek/mtk_drm_drv.h b/drivers/gpu/drm/mediatek/mtk_drm_drv.h index b5be63e53176..6ff98a68444b 100644 --- a/drivers/gpu/drm/mediatek/mtk_drm_drv.h +++ b/drivers/gpu/drm/mediatek/mtk_drm_drv.h @@ -54,6 +54,5 @@ extern struct platform_driver mtk_disp_ovl_driver; extern struct platform_driver mtk_disp_rdma_driver; extern struct platform_driver mtk_dpi_driver; extern struct platform_driver mtk_dsi_driver; -extern struct platform_driver mtk_mipi_tx_driver; #endif /* MTK_DRM_DRV_H */ diff --git a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c index 8cee2591e728..f2a892e16c27 100644 --- a/drivers/gpu/drm/mediatek/mtk_mipi_tx.c +++ b/drivers/gpu/drm/mediatek/mtk_mipi_tx.c @@ -242,4 +242,7 @@ struct platform_driver mtk_mipi_tx_driver = { .of_match_table = mtk_mipi_tx_match, }, }; +module_platform_driver(mtk_mipi_tx_driver); +MODULE_DESCRIPTION("MediaTek MIPI TX Driver"); +MODULE_LICENSE("GPL v2"); -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/3] Move Mediatek MIPI DSI PHY driver from DRM folder to PHY folder
mtk_mipi_dsi_phy is currently placed inside mediatek drm driver, but it's more suitable to place a phy driver into phy driver folder, so move mtk_mipi_dsi_phy driver into phy driver folder. Chun-Kuang Hu (3): drm/mediatek: Separate mtk_mipi_tx to an independent module phy: mediatek: Move mtk_mipi_dsi_phy driver into drivers/phy/mediatek folder MAINTAINERS: add files for Mediatek DRM drivers MAINTAINERS| 1 + drivers/gpu/drm/mediatek/Kconfig | 1 + drivers/gpu/drm/mediatek/Makefile | 3 --- drivers/gpu/drm/mediatek/mtk_drm_drv.c | 1 - drivers/gpu/drm/mediatek/mtk_drm_drv.h | 1 - drivers/phy/mediatek/Kconfig | 7 +++ drivers/phy/mediatek/Makefile | 5 + .../mediatek/phy-mtk-mipi-dsi-mt8173.c}| 2 +- .../mediatek/phy-mtk-mipi-dsi-mt8183.c}| 2 +- .../mtk_mipi_tx.c => phy/mediatek/phy-mtk-mipi-dsi.c} | 5 - .../mtk_mipi_tx.h => phy/mediatek/phy-mtk-mipi-dsi.h} | 0 11 files changed, 20 insertions(+), 8 deletions(-) rename drivers/{gpu/drm/mediatek/mtk_mt8173_mipi_tx.c => phy/mediatek/phy-mtk-mipi-dsi-mt8173.c} (99%) rename drivers/{gpu/drm/mediatek/mtk_mt8183_mipi_tx.c => phy/mediatek/phy-mtk-mipi-dsi-mt8183.c} (99%) rename drivers/{gpu/drm/mediatek/mtk_mipi_tx.c => phy/mediatek/phy-mtk-mipi-dsi.c} (97%) rename drivers/{gpu/drm/mediatek/mtk_mipi_tx.h => phy/mediatek/phy-mtk-mipi-dsi.h} (100%) -- 2.17.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RFC PATCH 2/2] drm: etnaviv: Unmap gems on gem_close
Hi Guido, Am Donnerstag, den 29.10.2020, 15:20 +0100 schrieb Guido Günther: > So far the unmap from gpu address space only happened when dropping the > last ref in gem_free_object_unlocked, however that is skipped if there's > still multiple handles to the same GEM object. > > Since userspace (here mesa) in the case of softpin hands back the memory > region to the pool of available GPU virtual memory closing the handle > via DRM_IOCTL_GEM_CLOSE this can lead to etnaviv_iommu_insert_exact > failing later since userspace thinks the vaddr is available while the > kernel thinks it isn't making the submit fail like > > [E] submit failed: -14 (No space left on device) (etna_cmd_stream_flush:244) > > Fix this by unmapping the memory via the .gem_close_object callback. > > Signed-off-by: Guido Günther > --- > drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 + > drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 + > drivers/gpu/drm/etnaviv/etnaviv_gem.c | 32 +++ > 3 files changed, 34 insertions(+) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > index a9a3afaef9a1..fdcc6405068c 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c > @@ -491,6 +491,7 @@ static struct drm_driver etnaviv_drm_driver = { > .open = etnaviv_open, > .postclose = etnaviv_postclose, > .gem_free_object_unlocked = etnaviv_gem_free_object, > + .gem_close_object = etnaviv_gem_close_object, > .gem_vm_ops = _ops, > .prime_handle_to_fd = drm_gem_prime_handle_to_fd, > .prime_fd_to_handle = drm_gem_prime_fd_to_handle, > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h > b/drivers/gpu/drm/etnaviv/etnaviv_drv.h > index 4d8dc9236e5f..2226a9af0d63 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h > @@ -65,6 +65,7 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op, > struct drm_etnaviv_timespec *timeout); > int etnaviv_gem_cpu_fini(struct drm_gem_object *obj); > void etnaviv_gem_free_object(struct drm_gem_object *obj); > +void etnaviv_gem_close_object(struct drm_gem_object *obj, struct drm_file > *file); > int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, > u32 size, u32 flags, u32 *handle); > int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file, > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c > b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > index f06e19e7be04..5aec4a23c77e 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c > @@ -515,6 +515,38 @@ static const struct etnaviv_gem_ops > etnaviv_gem_shmem_ops = { > .mmap = etnaviv_gem_mmap_obj, > }; > > +void etnaviv_gem_close_object(struct drm_gem_object *obj, struct drm_file > *unused) > +{ > + struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj); > + struct etnaviv_vram_mapping *mapping, *tmp; > + > + /* Handle this via etnaviv_gem_free_object */ > + if (obj->handle_count == 1) > + return; > + > + WARN_ON(is_active(etnaviv_obj)); > + > + /* > + * userspace wants to release the handle so we need to remove > + * the mapping from the gpu's virtual address space to stay > + * in sync. > + */ > + list_for_each_entry_safe(mapping, tmp, _obj->vram_list, > + obj_node) { > + struct etnaviv_iommu_context *context = mapping->context; > + > + WARN_ON(mapping->use); > + > + if (context) { > + etnaviv_iommu_unmap_gem(context, mapping); > + etnaviv_iommu_context_put(context); I see the issue you are trying to fix here, but this is not a viable fix. While userspace may close the handle, the GPU may still be processing jobs referening the BO, so we can't just unmap it from the address space. I think what we need to do here is waiting for the current jobs/fences of the BO when we hit the case where userspace tries to assign a new address to the BO. Basically wait for current jobs -> unamp from the address space -> map at new userspace assigned address. Regards, Lucas > + } > + > + list_del(>obj_node); > + kfree(mapping); > + } > +} > + > void etnaviv_gem_free_object(struct drm_gem_object *obj) > { > struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 2/2] drm: etnaviv: Unmap gems on gem_close
So far the unmap from gpu address space only happened when dropping the last ref in gem_free_object_unlocked, however that is skipped if there's still multiple handles to the same GEM object. Since userspace (here mesa) in the case of softpin hands back the memory region to the pool of available GPU virtual memory closing the handle via DRM_IOCTL_GEM_CLOSE this can lead to etnaviv_iommu_insert_exact failing later since userspace thinks the vaddr is available while the kernel thinks it isn't making the submit fail like [E] submit failed: -14 (No space left on device) (etna_cmd_stream_flush:244) Fix this by unmapping the memory via the .gem_close_object callback. Signed-off-by: Guido Günther --- drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 + drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 + drivers/gpu/drm/etnaviv/etnaviv_gem.c | 32 +++ 3 files changed, 34 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c index a9a3afaef9a1..fdcc6405068c 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c @@ -491,6 +491,7 @@ static struct drm_driver etnaviv_drm_driver = { .open = etnaviv_open, .postclose = etnaviv_postclose, .gem_free_object_unlocked = etnaviv_gem_free_object, + .gem_close_object = etnaviv_gem_close_object, .gem_vm_ops = _ops, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h index 4d8dc9236e5f..2226a9af0d63 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h @@ -65,6 +65,7 @@ int etnaviv_gem_cpu_prep(struct drm_gem_object *obj, u32 op, struct drm_etnaviv_timespec *timeout); int etnaviv_gem_cpu_fini(struct drm_gem_object *obj); void etnaviv_gem_free_object(struct drm_gem_object *obj); +void etnaviv_gem_close_object(struct drm_gem_object *obj, struct drm_file *file); int etnaviv_gem_new_handle(struct drm_device *dev, struct drm_file *file, u32 size, u32 flags, u32 *handle); int etnaviv_gem_new_userptr(struct drm_device *dev, struct drm_file *file, diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c b/drivers/gpu/drm/etnaviv/etnaviv_gem.c index f06e19e7be04..5aec4a23c77e 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.c @@ -515,6 +515,38 @@ static const struct etnaviv_gem_ops etnaviv_gem_shmem_ops = { .mmap = etnaviv_gem_mmap_obj, }; +void etnaviv_gem_close_object(struct drm_gem_object *obj, struct drm_file *unused) +{ + struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj); + struct etnaviv_vram_mapping *mapping, *tmp; + + /* Handle this via etnaviv_gem_free_object */ + if (obj->handle_count == 1) + return; + + WARN_ON(is_active(etnaviv_obj)); + + /* +* userspace wants to release the handle so we need to remove +* the mapping from the gpu's virtual address space to stay +* in sync. +*/ + list_for_each_entry_safe(mapping, tmp, _obj->vram_list, +obj_node) { + struct etnaviv_iommu_context *context = mapping->context; + + WARN_ON(mapping->use); + + if (context) { + etnaviv_iommu_unmap_gem(context, mapping); + etnaviv_iommu_context_put(context); + } + + list_del(>obj_node); + kfree(mapping); + } +} + void etnaviv_gem_free_object(struct drm_gem_object *obj) { struct etnaviv_gem_object *etnaviv_obj = to_etnaviv_bo(obj); -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 0/2] drm: etnaviv: Unmap gems on gem_close
This is meant as a RFC since i'm not sure if this is the right way to fix the problem: So far the unmap from gpu address space only happened when dropping the last ref in gem_free_object_unlocked, however that is skipped if there's still multiple handles to the same GEM object. Since userspace (here mesa) in the case of softpin hands back the memory region to the pool of available GPU virtual memory closing the handle via DRM_IOCTL_GEM_CLOSE this can lead to etnaviv_iommu_insert_exact failing later since userspace thinks the vaddr is available while the kernel thinks it isn't making the submit fail like [E] submit failed: -14 (No space left on device) (etna_cmd_stream_flush:244) Fix this by unmapping the memory via the .gem_close_object callback. The patch is against 5.9 and will need to be redone for drm-misc-next due to the conversion to GEM object functions but i'm happy to do that it looks like the right approach. I can trigger the problem when plugging/unplugging a DP screen driven by DCSS while DSI is driven by mxsfb. It preferably happens with 4k since this allocates bigger chunks. I also folded in a commit checking for the context->lock in etnaviv_iommu_insert_exact and etnaviv_iommu_remove_mapping too to make it match etnaviv_iommu_find_iova. To: Lucas Stach ,Russell King ,Christian Gmeiner ,David Airlie ,Daniel Vetter ,etna...@lists.freedesktop.org, dri-devel@lists.freedesktop.org Guido Günther (2): drm: etnaviv: Add lockdep annotations for context lock drm: etnaviv: Unmap gems on gem_close drivers/gpu/drm/etnaviv/etnaviv_drv.c | 1 + drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 + drivers/gpu/drm/etnaviv/etnaviv_gem.c | 32 +++ drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 4 4 files changed, 38 insertions(+) -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[RFC PATCH 1/2] drm: etnaviv: Add lockdep annotations for context lock
etnaviv_iommu_find_iova has it so etnaviv_iommu_insert_exact and lockdep_assert_held should have it as well. Signed-off-by: Guido Günther --- drivers/gpu/drm/etnaviv/etnaviv_mmu.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c index 3607d348c298..cd599ac04663 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c @@ -131,6 +131,8 @@ static void etnaviv_iommu_remove_mapping(struct etnaviv_iommu_context *context, { struct etnaviv_gem_object *etnaviv_obj = mapping->object; + lockdep_assert_held(>lock); + etnaviv_iommu_unmap(context, mapping->vram_node.start, etnaviv_obj->sgt, etnaviv_obj->base.size); drm_mm_remove_node(>vram_node); @@ -223,6 +225,8 @@ static int etnaviv_iommu_find_iova(struct etnaviv_iommu_context *context, static int etnaviv_iommu_insert_exact(struct etnaviv_iommu_context *context, struct drm_mm_node *node, size_t size, u64 va) { + lockdep_assert_held(>lock); + return drm_mm_insert_node_in_range(>mm, node, size, 0, 0, va, va + size, DRM_MM_INSERT_LOWEST); } -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Silence zero EDID carping
On Thu, Oct 29, 2020 at 11:00:30AM +, Chris Wilson wrote: > We have a few displays in CI that always report their EDID as a bunch of > zeroes. This is consistent behavioud, so one assumes intentional > indication of an "absent" EDID. Let us treat is as such by silently > reporting the zero edid using connector->null_edid_counter, leaving the > loud carp to EDID that violate their checksums or otherwise return > unexpected illegal data upon reading. These are more likely to be > inconsistent bad connections rather than being intended. I don't think null_edid_counter is actually used by anything. So apart from wondering why the mode list has turned strange is there some way I can still see from the logs that the EDID has become all zeroes? > > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/drm_edid.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c > index 631125b46e04..94549805a204 100644 > --- a/drivers/gpu/drm/drm_edid.c > +++ b/drivers/gpu/drm/drm_edid.c > @@ -1951,7 +1951,7 @@ struct edid *drm_do_get_edid(struct drm_connector > *connector, > break; > if (i == 0 && drm_edid_is_zero(edid, EDID_LENGTH)) { > connector->null_edid_counter++; > - goto carp; > + goto out; > } > } > if (i == 4) > -- > 2.20.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers
Hi Daniel, I love your patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.10-rc1 next-20201028] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: alpha-randconfig-r003-20201029 (attached as .config) compiler: alpha-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/188b22d2b66860695df5d07bf2b7115976790b2c git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618 git checkout 188b22d2b66860695df5d07bf2b7115976790b2c # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): In file included from drivers/gpu/drm/qxl/qxl_drv.c:31: >> drivers/gpu/drm/qxl/qxl_drv.h:178:35: warning: 'struct qxl_device' declared >> inside parameter list will not be visible outside of this definition or >> declaration 178 | int qxl_debugfs_fence_init(struct qxl_device *rdev); | ^~ vim +178 drivers/gpu/drm/qxl/qxl_drv.h f64122c1f6ade30 Dave Airlie 2013-02-25 177 f64122c1f6ade30 Dave Airlie 2013-02-25 @178 int qxl_debugfs_fence_init(struct qxl_device *rdev); f64122c1f6ade30 Dave Airlie 2013-02-25 179 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/atomic: Pass the full state to CRTC atomic begin and flush
On Wed, Oct 28, 2020 at 01:32:22PM +0100, Maxime Ripard wrote: > The current atomic helpers have either their object state being passed as > an argument or the full atomic state. > > The former is the pattern that was done at first, before switching to the > latter for new hooks or when it was needed. > > Let's start convert all the remaining helpers to provide a consistent > interface, starting with the CRTC's atomic_begin and atomic_flush. > > The conversion was done using the coccinelle script below, built tested on > all the drivers and actually tested on vc4. > > @@ -323,26 +323,27 @@ static void ingenic_drm_crtc_atomic_begin(struct > drm_crtc *crtc, > } > > static void ingenic_drm_crtc_atomic_flush(struct drm_crtc *crtc, > - struct drm_crtc_state *oldstate) > + struct drm_atomic_state *state) > { > struct ingenic_drm *priv = drm_crtc_get_priv(crtc); > - struct drm_crtc_state *state = crtc->state; > - struct drm_pending_vblank_event *event = state->event; > + struct drm_crtc_state *crtc_state = crtc->state; Looks like quite a few places could use a followup to switch to get_{old,new}_crtc_state(). Patch lgtm Reviewed-by: Ville Syrjälä -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] drm/atomic: Pass the full state to CRTC atomic_check
On Wed, Oct 28, 2020 at 01:32:21PM +0100, Maxime Ripard wrote: > The current atomic helpers have either their object state being passed as > an argument or the full atomic state. > > The former is the pattern that was done at first, before switching to the > latter for new hooks or when it was needed. > > Let's start convert all the remaining helpers to provide a consistent > interface, starting with the CRTC's atomic_check. > > The conversion was done using the coccinelle script below, > built tested on all the drivers and actually tested on vc4. > > virtual report ? > @ depends on crtc_atomic_func @ > identifier crtc_atomic_func.func; > expression E; > type T; > @@ > > int func(...) > { > ... > - T state = E; > + T crtc_state = E; > <+... > - state > + crtc_state > ...+> > } > > @ depends on crtc_atomic_func @ > identifier crtc_atomic_func.func; > type T; > @@ > > int func(...) > { > ... > - T state; > + T crtc_state; > <+... > - state > + crtc_state > ...+> > } These two seem a bit fuzzy. AFAICS 'state' could be any kind of state given the constrainsts. Though I guess the fact that this is the crtc .atomic_check() it's most likely to either the crtc state or the whole atomic state. Not sure what would be the best way to tighten this up. Maybe a regex thing on the assignment? But I'm not sure you can even do that on an expression. Anyways, doesn't look like this went wrong anywhere, so seems good enough for a onetime job. > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > index 956f631997f2..b0757f84a979 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c > @@ -269,17 +269,19 @@ static void mxsfb_crtc_mode_set_nofb(struct > mxsfb_drm_private *mxsfb) > } > > static int mxsfb_crtc_atomic_check(struct drm_crtc *crtc, > -struct drm_crtc_state *state) > +struct drm_atomic_state *state) > { > - bool has_primary = state->plane_mask & > + struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, > + crtc); > + bool has_primary = crtc_state->plane_mask & > drm_plane_mask(crtc->primary); > > /* The primary plane has to be enabled when the CRTC is active. */ > - if (state->active && !has_primary) > + if (crtc_state->active && !has_primary) > return -EINVAL; > > /* TODO: Is this needed ? */ > - return drm_atomic_add_affected_planes(state->state, crtc); > + return drm_atomic_add_affected_planes(crtc_state->state, crtc); Could also s/crtc_state->state/state/ in various places. But that could done as a followup as well. Didn't spot any mistakes: Reviewed-by: Ville Syrjälä -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/qxl: Remove fbcon acceleration leftovers
These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev emulation code"). v2: Somehow these structs provided the struct qxl_device pre-decl, reorder the header to not anger compilers. Acked-by: Gerd Hoffmann Signed-off-by: Daniel Vetter Cc: Dave Airlie Cc: Gerd Hoffmann Cc: virtualizat...@lists.linux-foundation.org Cc: spice-de...@lists.freedesktop.org --- drivers/gpu/drm/qxl/qxl_drv.h | 18 ++ 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index 3602e8b34189..6239626503ef 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -166,20 +166,6 @@ struct qxl_drm_image { struct list_head chunk_list; }; -struct qxl_fb_image { - struct qxl_device *qdev; - uint32_t pseudo_palette[16]; - struct fb_image fb_image; - uint32_t visual; -}; - -struct qxl_draw_fill { - struct qxl_device *qdev; - struct qxl_rect rect; - uint32_t color; - uint16_t rop; -}; - /* * Debugfs */ @@ -188,8 +174,6 @@ struct qxl_debugfs { unsigned int num_files; }; -int qxl_debugfs_fence_init(struct qxl_device *rdev); - struct qxl_device { struct drm_device ddev; @@ -271,6 +255,8 @@ struct qxl_device { #define to_qxl(dev) container_of(dev, struct qxl_device, ddev) +int qxl_debugfs_fence_init(struct qxl_device *rdev); + extern const struct drm_ioctl_desc qxl_ioctls[]; extern int qxl_max_ioctl; -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209939] radeontop causes kernel panic
https://bugzilla.kernel.org/show_bug.cgi?id=209939 Alex Deucher (alexdeuc...@gmail.com) changed: What|Removed |Added CC||alexdeuc...@gmail.com --- Comment #1 from Alex Deucher (alexdeuc...@gmail.com) --- Does setting amdgpu.runpm=0 on the kernel command line in grub fix the issue? How are you running radeontop? If you are running it such that it tries to access MMIO space directly rather than going through the kernel, that could cause an issue. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/7] drm/vc4: kms: Document the muxing corner cases
On Thu, Oct 29, 2020 at 11:47:28AM +0100, Maxime Ripard wrote: > Hi! > > Thanks for your review > > On Thu, Oct 29, 2020 at 09:51:04AM +0100, Daniel Vetter wrote: > > On Wed, Oct 28, 2020 at 01:41:01PM +0100, Maxime Ripard wrote: > > > We've had a number of muxing corner-cases with specific ways to reproduce > > > them, so let's document them to make sure they aren't lost and introduce > > > regressions later on. > > > > > > Signed-off-by: Maxime Ripard > > > --- > > > drivers/gpu/drm/vc4/vc4_kms.c | 22 ++ > > > 1 file changed, 22 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/vc4/vc4_kms.c b/drivers/gpu/drm/vc4/vc4_kms.c > > > index 4aa0577bd055..b0043abec16d 100644 > > > --- a/drivers/gpu/drm/vc4/vc4_kms.c > > > +++ b/drivers/gpu/drm/vc4/vc4_kms.c > > > @@ -612,6 +612,28 @@ static const struct drm_private_state_funcs > > > vc4_load_tracker_state_funcs = { > > > }; > > > > > > > > > +/* > > > + * The BCM2711 HVS has up to 7 output connected to the pixelvalves and > > > + * the TXP (and therefore all the CRTCs found on that platform). > > > + * > > > + * The naive (and our initial) implementation would just iterate over > > > + * all the active CRTCs, try to find a suitable FIFO, and then remove it > > > + * from the available FIFOs pool. However, there's a few corner cases > > > + * that need to be considered: > > > + * > > > + * - When running in a dual-display setup (so with two CRTCs involved), > > > + * we can update the state of a single CRTC (for example by changing > > > + * its mode using xrandr under X11) without affecting the other. In > > > + * this case, the other CRTC wouldn't be in the state at all, so we > > > + * need to consider all the running CRTCs in the DRM device to assign > > > + * a FIFO, not just the one in the state. > > > + * > > > + * - Since we need the pixelvalve to be disabled and enabled back when > > > + * the FIFO is changed, we should keep the FIFO assigned for as long > > > + * as the CRTC is enabled, only considering it free again once that > > > + * CRTC has been disabled. This can be tested by booting X11 on a > > > + * single display, and changing the resolution down and then back up. > > > > This is a bit much. > > What do you find to be a bit much? > > > With planes we have the same problem, and we're solving this with the > > drm_plane_state->commit tracker. If you have one of these per fifo > > then you can easily sync against the disabling crtc if the fifo > > becomes free. > > > > Note to avoid locking headaches this would mean you'd need a per-fifo > > private state object. You can avoid this if you just track the > > ->disabling_commit per fifo, and sync against that when enabling it on a > > different fifo. > > > > Note that it's somewhat tricky to do this correctly, since you need to > > copy that commit on each state duplication around, until it's either used > > in a new crtc (and hence tracked under that) or the commit has completed > > (but this is only an optimization, you could just keep them around forever > > for unused fifo that have been used in the past, it's a tiny struct with > > nothing hanging of it). > > I'm not really following you here. The hardware that does the blending > (HVS) and the timing generation (pixelvalve) is mostly transparent to > DRM and plugged as a CRTC, with the pixelvalve being the device tied to > that CRTC, and the pixelvalve hooks calling into the HVS code when > needed. > > The FIFO is in the HVS itself since it can only blend 3 different > scenes, and then you get to choose the output of that FIFO to send it to > the proper pixelvalve that matches the encoder you eventually want to > use. > > So yeah, this FIFO is entirely internal to the CRTC as far as DRM is > concerned. So why do you dynamically assign it in a global state object? It sounded like you assign these things dynamically, and that there's a problem with sync when you move it from one crtc to the other. And that kind of problem is solved by tracking the last crtc using a given resource that can be used by different crtc with a drm_crtc_commit *last_user pointer. Otherwise I think if you follow 2 crtc commits, one that disables a CRTC user and releases a FIFO, and the next crtc (a different one) that uses it right away, both nonblocking, then the 2nd crtc might start using your shared resources before the first one actually stopped using it. The other thing is also if you need multiple of these shared resources on a CRTC, and dynamically reassigning them, then forcing them to be assigned until the crtc is completely off is a bit too much synchronization. E.g. we don't have that rule for drm planes. Now I have no idea whether your CRTC:FIFO is 1:1 or 1:n, so maybe you only have the sync issue and not the over-sync issue :-) Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list
Re: [PATCH -next] drm/i915: Remove unused variable ret
On Thu, Oct 29, 2020 at 10:18:45AM +0800, Zou Wei wrote: > This patch fixes below warnings reported by coccicheck > > ./drivers/gpu/drm/i915/i915_debugfs.c:789:5-8: Unneeded variable: "ret". > Return "0" on line 1012 > > Reported-by: Hulk Robot > Signed-off-by: Zou Wei Thanks. Applied. > --- > drivers/gpu/drm/i915/i915_debugfs.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c > b/drivers/gpu/drm/i915/i915_debugfs.c > index ea46916..200f6b8 100644 > --- a/drivers/gpu/drm/i915/i915_debugfs.c > +++ b/drivers/gpu/drm/i915/i915_debugfs.c > @@ -786,7 +786,6 @@ static int i915_frequency_info(struct seq_file *m, void > *unused) > struct intel_uncore *uncore = _priv->uncore; > struct intel_rps *rps = _priv->gt.rps; > intel_wakeref_t wakeref; > - int ret = 0; > > wakeref = intel_runtime_pm_get(_priv->runtime_pm); > > @@ -1009,7 +1008,7 @@ static int i915_frequency_info(struct seq_file *m, void > *unused) > seq_printf(m, "Max pixel clock frequency: %d kHz\n", > dev_priv->max_dotclk_freq); > > intel_runtime_pm_put(_priv->runtime_pm, wakeref); > - return ret; > + return 0; > } > > static int i915_ring_freq_table(struct seq_file *m, void *unused) > -- > 2.6.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 209939] New: radeontop causes kernel panic
https://bugzilla.kernel.org/show_bug.cgi?id=209939 Bug ID: 209939 Summary: radeontop causes kernel panic Product: Drivers Version: 2.5 Kernel Version: 5.9.1 Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: janpieter.sol...@edpnet.be Regression: No Created attachment 293297 --> https://bugzilla.kernel.org/attachment.cgi?id=293297=edit kernel .config file of 3 PCs (view 3 .config files) > PC1: problem pc, Ryzen 2400GE APU with Vega 11 and 5.9.1 kernel (Xorg > running) > PC2: working pc, ryzen V1605 APU with vega 8 and 5.8.14 kernel (Xorg running) > PC3: working pc, Threadripper 1950 + Fiji GPU and 5.9.1 kernel (CLI only) As the subject states: on PC1, the kernel can't handle the radeontop program, one way or another, these methods work / do not on PC1: > - while hardware-accelerated content is running, panic > - When in console mode, it's fine > - when switching from console to X, it's fine for a few moments > - when trying it early (X running sddm, radeontop via ssh), panic with *panic*, I mean: the PC does not react anymore: the num lock trigger is no longer working, no input is accepted, the clock on the GUI does not change anymore, no SSH. I tried everything: > - pstore is empty > - dd if=/dev/kmsg of=/dev/sdb1 & while [ 1]; do echo s > /proc/sysrq-trigger; > sleep 10; done & radeontop (and pulling it out of this partition afterwards) The mainboard does not have a RS232 port, so debugging this way is not possible; also, I doubt I'd be able to use KDB if the screen stucks at GUI mode ... If I can do anything to gather more info, let me know -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v5 1/1] drm/mediatek: dsi: fix scrolling of panel with small hfp or hbp
Hi, Jitao: Jitao Shi 於 2020年10月13日 週二 下午6:06寫道: > > Replace horizontal_backporch_byte with vm->hback_porch * bpp to aovid > flowing judgement negative number. > > if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) > > data_phy_cycles * dsi->lanes + delta) > > Signed-off-by: Jitao Shi > --- > drivers/gpu/drm/mediatek/mtk_dsi.c | 65 > +++--- > 1 file changed, 25 insertions(+), 40 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index 80b7a082e874..f69ebeaf 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -445,6 +445,7 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi *dsi) > u32 horizontal_backporch_byte; > u32 horizontal_frontporch_byte; > u32 dsi_tmp_buf_bpp, data_phy_cycles; > + u32 delta; > struct mtk_phy_timing *timing = >phy_timing; > > struct videomode *vm = >vm; > @@ -466,50 +467,34 @@ static void mtk_dsi_config_vdo_timing(struct mtk_dsi > *dsi) > horizontal_sync_active_byte = (vm->hsync_len * dsi_tmp_buf_bpp - 10); > > if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) > - horizontal_backporch_byte = vm->hback_porch * dsi_tmp_buf_bpp; > + horizontal_backporch_byte = > + (vm->hback_porch * dsi_tmp_buf_bpp - 10); > else > - horizontal_backporch_byte = (vm->hback_porch + vm->hsync_len) > * > - dsi_tmp_buf_bpp; > + horizontal_backporch_byte = ((vm->hback_porch + > vm->hsync_len) * > + dsi_tmp_buf_bpp - 10); > > data_phy_cycles = timing->lpx + timing->da_hs_prepare + > - timing->da_hs_zero + timing->da_hs_exit; > - > - if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) { > - if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp > > - data_phy_cycles * dsi->lanes + 18) { > - horizontal_frontporch_byte = > - vm->hfront_porch * dsi_tmp_buf_bpp - > - (data_phy_cycles * dsi->lanes + 18) * > - vm->hfront_porch / > - (vm->hfront_porch + vm->hback_porch); > - > - horizontal_backporch_byte = > - horizontal_backporch_byte - > - (data_phy_cycles * dsi->lanes + 18) * > - vm->hback_porch / > - (vm->hfront_porch + vm->hback_porch); > - } else { > - DRM_WARN("HFP less than d-phy, FPS will under > 60Hz\n"); > - horizontal_frontporch_byte = vm->hfront_porch * > -dsi_tmp_buf_bpp; > - } > + timing->da_hs_zero + timing->da_hs_exit + 3; > + > + delta = (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_BURST) ? 18 : 12; > + > + if ((vm->hfront_porch * dsi_tmp_buf_bpp + horizontal_backporch_byte) > > + data_phy_cycles * dsi->lanes + delta) { > + horizontal_frontporch_byte = > + vm->hfront_porch * dsi_tmp_buf_bpp - > + (data_phy_cycles * dsi->lanes + delta) * > + vm->hfront_porch / > + (vm->hfront_porch + vm->hback_porch); > + > + horizontal_backporch_byte = > + horizontal_backporch_byte - > + (data_phy_cycles * dsi->lanes + delta) * > + vm->hback_porch / > + (vm->hfront_porch + vm->hback_porch); > } else { > - if ((vm->hfront_porch + vm->hback_porch) * dsi_tmp_buf_bpp > > - data_phy_cycles * dsi->lanes + 12) { > - horizontal_frontporch_byte = > - vm->hfront_porch * dsi_tmp_buf_bpp - > - (data_phy_cycles * dsi->lanes + 12) * > - vm->hfront_porch / > - (vm->hfront_porch + vm->hback_porch); > - horizontal_backporch_byte = horizontal_backporch_byte > - > - (data_phy_cycles * dsi->lanes + 12) * > - vm->hback_porch / > - (vm->hfront_porch + vm->hback_porch); > - } else { > - DRM_WARN("HFP less than d-phy, FPS will under > 60Hz\n"); > - horizontal_frontporch_byte = vm->hfront_porch * > -dsi_tmp_buf_bpp; > - } > + DRM_WARN("HFP + HBP less than d-phy, FPS will under 60Hz\n"); > +
[PATCH] fbcon: Disable accelerated scrolling
So ever since syzbot discovered fbcon, we have solid proof that it's full of bugs. And often the solution is to just delete code and remove features, e.g. 50145474f6ef ("fbcon: remove soft scrollback code"). Now the problem is that most modern-ish drivers really only treat fbcon as an dumb kernel console until userspace takes over, and Oops printer for some emergencies. Looking at drm drivers and the basic vesa/efi fbdev drivers shows that only 3 drivers support any kind of acceleration: - nouveau, seems to be enabled by default - omapdrm, when a DMM remapper exists using remapper rewriting for y/xpanning - gma500, but that is getting deleted now for the GTT remapper trick, and the accelerated copyarea never set the FBINFO_HWACCEL_COPYAREA flag, so unused (and could be deleted already I think). No other driver supportes accelerated fbcon. And fbcon is the only user of this accel code (it's not exposed as uapi through ioctls), which means we could garbage collect fairly enormous amounts of code if we kill this. Plus because syzbot only runs on virtual hardware, and none of the drivers for that have acceleration, we'd remove a huge gap in testing. And there's no other even remotely comprehensive testing aside from syzbot. This patch here just disables the acceleration code by always redrawing when scrolling. The plan is that once this has been merged for well over a year in released kernels, we can start to go around and delete a lot of code. v2: - Drop a few more unused local variables, somehow I missed the compiler warnings (Sam) - Fix typo in comment (Jiri) - add a todo entry for the cleanup (Thomas) v3: Remove more unused variables (0day) Reviewed-by: Thomas Zimmermann Reviewed-by: Greg Kroah-Hartman Acked-by: Sam Ravnborg Cc: Jiri Slaby Cc: Bartlomiej Zolnierkiewicz Cc: Greg Kroah-Hartman Cc: Linus Torvalds Cc: Ben Skeggs Cc: nouv...@lists.freedesktop.org Cc: Tomi Valkeinen Cc: Daniel Vetter Cc: Jiri Slaby Cc: "Gustavo A. R. Silva" Cc: Tetsuo Handa Cc: Peilin Ye Cc: George Kennedy Cc: Nathan Chancellor Cc: Peter Rosin Signed-off-by: Daniel Vetter --- Documentation/gpu/todo.rst | 18 + drivers/video/fbdev/core/fbcon.c | 45 ++-- 2 files changed, 26 insertions(+), 37 deletions(-) diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 6b224ef14455..bec99341a904 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -277,6 +277,24 @@ Contact: Daniel Vetter, Noralf Tronnes Level: Advanced +Garbage collect fbdev scrolling acceleration + + +Scroll acceleration is disabled in fbcon by hard-wiring p->scrollmode = +SCROLL_REDRAW. There's a ton of code this will allow us to remove: +- lots of code in fbcon.c +- a bunch of the hooks in fbcon_ops, maybe the remaining hooks could be called + directly instead of the function table (with a switch on p->rotate) +- fb_copyarea is unused after this, and can be deleted from all drivers + +Note that not all acceleration code can be deleted, since clearing and cursor +support is still accelerated, which might be good candidates for further +deletion projects. + +Contact: Daniel Vetter + +Level: Intermediate + idr_init_base() --- diff --git a/drivers/video/fbdev/core/fbcon.c b/drivers/video/fbdev/core/fbcon.c index cef437817b0d..8d1ae973041a 100644 --- a/drivers/video/fbdev/core/fbcon.c +++ b/drivers/video/fbdev/core/fbcon.c @@ -1033,7 +1033,7 @@ static void fbcon_init(struct vc_data *vc, int init) struct vc_data *svc = *default_mode; struct fbcon_display *t, *p = _display[vc->vc_num]; int logo = 1, new_rows, new_cols, rows, cols, charcnt = 256; - int cap, ret; + int ret; if (WARN_ON(info_idx == -1)) return; @@ -1042,7 +1042,6 @@ static void fbcon_init(struct vc_data *vc, int init) con2fb_map[vc->vc_num] = info_idx; info = registered_fb[con2fb_map[vc->vc_num]]; - cap = info->flags; if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET) logo_shown = FBCON_LOGO_DONTSHOW; @@ -1147,11 +1146,13 @@ static void fbcon_init(struct vc_data *vc, int init) ops->graphics = 0; - if ((cap & FBINFO_HWACCEL_COPYAREA) && - !(cap & FBINFO_HWACCEL_DISABLED)) - p->scrollmode = SCROLL_MOVE; - else /* default to something safe */ - p->scrollmode = SCROLL_REDRAW; + /* +* No more hw acceleration for fbcon. +* +* FIXME: Garbage collect all the now dead code after sufficient time +* has passed. +*/ + p->scrollmode = SCROLL_REDRAW; /* * ++guenther: console.c:vc_allocate() relies on initializing @@ -1961,45 +1962,15 @@ static void updatescrollmode(struct fbcon_display *p, { struct fbcon_ops *ops = info->fbcon_par; int fh = vc->vc_font.height; -
Re: [Intel-gfx] [PATCH 1/3] fbcon: Disable accelerated scrolling
Hi Daniel, I love your patch! Perhaps something to improve: [auto build test WARNING on drm-intel/for-linux-next] [also build test WARNING on drm-tip/drm-tip drm-exynos/exynos-drm-next tegra-drm/drm/tegra/for-next linus/master drm/drm-next v5.10-rc1 next-20201028] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: sparc64-randconfig-r005-20201029 (attached as .config) compiler: sparc64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/b457f0ea024ca7202fa63f5a94f9d5abf65529f8 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Daniel-Vetter/fbcon-Disable-accelerated-scrolling/20201029-181618 git checkout b457f0ea024ca7202fa63f5a94f9d5abf65529f8 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=sparc64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): drivers/video/fbdev/core/fbcon.c: In function 'fbcon_init': >> drivers/video/fbdev/core/fbcon.c:1089:6: warning: variable 'cap' set but not >> used [-Wunused-but-set-variable] 1089 | int cap, ret; | ^~~ drivers/video/fbdev/core/fbcon.c: In function 'fbcon_exit': drivers/video/fbdev/core/fbcon.c:3646:7: warning: variable 'pending' set but not used [-Wunused-but-set-variable] 3646 | int pending = 0; | ^~~ vim +/cap +1089 drivers/video/fbdev/core/fbcon.c ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1080 ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1081 static void fbcon_init(struct vc_data *vc, int init) ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1082 { 1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1083 struct fb_info *info; ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1084 struct fbcon_ops *ops; ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1085 struct vc_data **default_mode = vc->vc_display_fg; ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1086 struct vc_data *svc = *default_mode; 50233393f0cf9b drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1087 struct fbcon_display *t, *p = _display[vc->vc_num]; ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1088 int logo = 1, new_rows, new_cols, rows, cols, charcnt = 256; 0fcf6ada2b8eb4 drivers/video/console/fbcon.cFlorian Tobias Schandinat 2009-09-22 @1089 int cap, ret; ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1090 1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1091 if (WARN_ON(info_idx == -1)) ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1092 return; 306958e8e8d150 drivers/video/console/fbcon.cAdrian Bunk 2005-05-01 1093 1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1094 if (con2fb_map[vc->vc_num] == -1) 1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1095 con2fb_map[vc->vc_num] = info_idx; 1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1096 1f4ed2fb01f80f drivers/video/fbdev/core/fbcon.c Daniel Vetter 2019-05-28 1097 info = registered_fb[con2fb_map[vc->vc_num]]; 306958e8e8d150 drivers/video/console/fbcon.cAdrian Bunk 2005-05-01 1098 cap = info->flags; 306958e8e8d150 drivers/video/console/fbcon.cAdrian Bunk 2005-05-01 1099 3c5a1b111373e6 drivers/video/fbdev/core/fbcon.c Andreas Schwab 2019-05-06 1100 if (logo_shown < 0 && console_loglevel <= CONSOLE_LOGLEVEL_QUIET) 10993504d64735 drivers/video/fbdev/core/fbcon.c Prarit Bhargava 2019-02-08 1101 logo_shown = FBCON_LOGO_DONTSHOW; 10993504d64735 drivers/video/fbdev/core/fbcon.c Prarit Bhargava 2019-02-08 1102 ^1da177e4c3f41 drivers/video/console/fbcon.cLinus Torvalds 2005-04-16 1103
Re: [PATCH 1/1] video: fbdev: fix divide error in fbcon_switch
On Thu, Oct 29, 2020 at 01:13:01PM +0200, Jani Nikula wrote: > On Thu, 29 Oct 2020, Greg KH wrote: > > On Tue, Oct 27, 2020 at 10:12:49AM -0700, Saeed Mirzamohammadi wrote: > >> Hi Greg, > >> > >> Sorry for the confusion. I’m requesting stable maintainers to cherry-pick > >> this patch into stable 5.4 and 5.8. > >> commit cc07057c7c88fb8eff3b1991131ded0f0bcfa7e3 > >> Author: Saeed Mirzamohammadi > >> Date: Wed Oct 21 16:57:58 2020 -0700 > >> > >> video: fbdev: fix divide error in fbcon_switch > > > > I do not see that commit in Linus's tree, do you? > > It's in drm-misc-next, IIUC heading for Linus' tree in the next merge > window in 6-8 weeks. Which is to say this should probably have been > applied to drm-misc-fixes branch heading for v5.10-rcX, with a Cc: > stable tag, to begin with. Ok, nothing I can do with this now, please email sta...@vger.kernel.org when it hits Linus's tree and we can take it then. Saeed, please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly. thanks, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] drm/tidss: Use new connector model for tidss
On 28/10/2020 16:19, Nikhil Devshatwar wrote: >> Also, with J7 EVM and DP, when I unload the modules I see: >> > I confirm the same issue. > I doubt if it is because of the change I did. > Will try to revert the patches and confirm again My guess is that it's not your patches as such, but that the mhdp driver does not do irq related cleanups properly and your patches bring the issue up. Tomi -- 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: [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers
On Thu, Oct 29, 2020 at 11:14:28AM +0100, Daniel Vetter wrote: > These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev > emulation code"). Acked-by: Gerd Hoffmann ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] video: fbdev: fix divide error in fbcon_switch
On Thu, 29 Oct 2020, Greg KH wrote: > On Tue, Oct 27, 2020 at 10:12:49AM -0700, Saeed Mirzamohammadi wrote: >> Hi Greg, >> >> Sorry for the confusion. I’m requesting stable maintainers to cherry-pick >> this patch into stable 5.4 and 5.8. >> commit cc07057c7c88fb8eff3b1991131ded0f0bcfa7e3 >> Author: Saeed Mirzamohammadi >> Date: Wed Oct 21 16:57:58 2020 -0700 >> >> video: fbdev: fix divide error in fbcon_switch > > I do not see that commit in Linus's tree, do you? It's in drm-misc-next, IIUC heading for Linus' tree in the next merge window in 6-8 weeks. Which is to say this should probably have been applied to drm-misc-fixes branch heading for v5.10-rcX, with a Cc: stable tag, to begin with. BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/1] video: fbdev: fix divide error in fbcon_switch
On Tue, Oct 27, 2020 at 10:12:49AM -0700, Saeed Mirzamohammadi wrote: > Hi Greg, > > Sorry for the confusion. I’m requesting stable maintainers to cherry-pick > this patch into stable 5.4 and 5.8. > commit cc07057c7c88fb8eff3b1991131ded0f0bcfa7e3 > Author: Saeed Mirzamohammadi > Date: Wed Oct 21 16:57:58 2020 -0700 > > video: fbdev: fix divide error in fbcon_switch I do not see that commit in Linus's tree, do you? confused, greg k-h ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Silence zero EDID carping
We have a few displays in CI that always report their EDID as a bunch of zeroes. This is consistent behavioud, so one assumes intentional indication of an "absent" EDID. Let us treat is as such by silently reporting the zero edid using connector->null_edid_counter, leaving the loud carp to EDID that violate their checksums or otherwise return unexpected illegal data upon reading. These are more likely to be inconsistent bad connections rather than being intended. Signed-off-by: Chris Wilson --- drivers/gpu/drm/drm_edid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 631125b46e04..94549805a204 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -1951,7 +1951,7 @@ struct edid *drm_do_get_edid(struct drm_connector *connector, break; if (i == 0 && drm_edid_is_zero(edid, EDID_LENGTH)) { connector->null_edid_counter++; - goto carp; + goto out; } } if (i == 4) -- 2.20.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/vc4: hdmi: Block odd horizontal timings
On Thu, 29 Oct 2020 at 09:17, Maxime Ripard wrote: > > Hi! > > On Wed, Oct 28, 2020 at 01:42:20PM +, Dave Stevenson wrote: > > Hi Maxime > > > > On Fri, 25 Sep 2020 at 14:00, Maxime Ripard wrote: > > > > > > The FIFO between the pixelvalve and the HDMI controller runs at 2 pixels > > > per clock cycle, and cannot deal with odd timings. > > > > > > Let's reject any mode with such timings. > > > > > > Signed-off-by: Maxime Ripard > > Thanks for your review > > > It's unsupported due to the architecture rather than broken. > > Would you prefer s/broken/unsupported/ then? If you needed to respin then yes, but it's not that big a deal. Dave ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2] drm/panfrost: Fix a race in the job timeout handling (again)
In our last attempt to fix races in the panfrost_job_timedout() path we overlooked the case where a re-submitted job immediately triggers a fault. This lead to a situation where we try to stop a scheduler that's not resumed yet and lose the 'timedout' event without restarting the timeout, thus blocking the whole queue. Let's fix that by tracking timeouts occurring between the drm_sched_resubmit_jobs() and drm_sched_start() calls. v2: - Fix another race (reported by Steven) Fixes: 1a11a88cfd9a ("drm/panfrost: Fix job timeout handling") Cc: Signed-off-by: Boris Brezillon --- drivers/gpu/drm/panfrost/panfrost_job.c | 61 + 1 file changed, 43 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index d0469e944143..0f9a34f5c6d0 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -26,6 +26,7 @@ struct panfrost_queue_state { struct drm_gpu_scheduler sched; bool stopped; + bool timedout; struct mutex lock; u64 fence_context; u64 emit_seqno; @@ -383,11 +384,33 @@ static bool panfrost_scheduler_stop(struct panfrost_queue_state *queue, queue->stopped = true; stopped = true; } + queue->timedout = true; mutex_unlock(>lock); return stopped; } +static void panfrost_scheduler_start(struct panfrost_queue_state *queue) +{ + if (WARN_ON(!queue->stopped)) + return; + + mutex_lock(>lock); + drm_sched_start(>sched, true); + + /* +* We might have missed fault-timeouts (AKA immediate timeouts) while +* the scheduler was stopped. Let's fake a new fault to trigger an +* immediate reset. +*/ + if (queue->timedout) + drm_sched_fault(>sched); + + queue->timedout = false; + queue->stopped = false; + mutex_unlock(>lock); +} + static void panfrost_job_timedout(struct drm_sched_job *sched_job) { struct panfrost_job *job = to_panfrost_job(sched_job); @@ -422,27 +445,20 @@ static void panfrost_job_timedout(struct drm_sched_job *sched_job) struct drm_gpu_scheduler *sched = >js->queue[i].sched; /* -* If the queue is still active, make sure we wait for any -* pending timeouts. +* Stop the scheduler and wait for any pending timeout handler +* to return. */ - if (!pfdev->js->queue[i].stopped) + panfrost_scheduler_stop(>js->queue[i], NULL); + if (i != js) cancel_delayed_work_sync(>work_tdr); /* -* If the scheduler was not already stopped, there's a tiny -* chance a timeout has expired just before we stopped it, and -* drm_sched_stop() does not flush pending works. Let's flush -* them now so the timeout handler doesn't get called in the -* middle of a reset. +* We do another stop after cancel_delayed_work_sync() to make +* sure we don't race against another thread finishing its +* reset (the restart queue steps are not protected by the +* reset lock). */ - if (panfrost_scheduler_stop(>js->queue[i], NULL)) - cancel_delayed_work_sync(>work_tdr); - - /* -* Now that we cancelled the pending timeouts, we can safely -* reset the stopped state. -*/ - pfdev->js->queue[i].stopped = false; + panfrost_scheduler_stop(>js->queue[i], NULL); } spin_lock_irqsave(>js->job_lock, flags); @@ -457,14 +473,23 @@ static void panfrost_job_timedout(struct drm_sched_job *sched_job) panfrost_device_reset(pfdev); - for (i = 0; i < NUM_JOB_SLOTS; i++) + for (i = 0; i < NUM_JOB_SLOTS; i++) { + /* +* The GPU is idle, and the scheduler is stopped, we can safely +* reset the ->timedout state without taking any lock. We need +* to do that before calling drm_sched_resubmit_jobs() though, +* because the resubmission might trigger immediate faults +* which we want to catch. +*/ + pfdev->js->queue[i].timedout = false; drm_sched_resubmit_jobs(>js->queue[i].sched); + } mutex_unlock(>reset_lock); /* restart scheduler after GPU is usable again */ for (i = 0; i < NUM_JOB_SLOTS; i++) - drm_sched_start(>js->queue[i].sched, true); + panfrost_scheduler_start(>js->queue[i]); } static const struct drm_sched_backend_ops panfrost_sched_ops = { -- 2.26.2
[PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers
These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev emulation code"). Signed-off-by: Daniel Vetter Cc: Dave Airlie Cc: Gerd Hoffmann Cc: virtualizat...@lists.linux-foundation.org Cc: spice-de...@lists.freedesktop.org --- drivers/gpu/drm/qxl/qxl_drv.h | 14 -- 1 file changed, 14 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h index 3602e8b34189..86eee66ecbad 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.h +++ b/drivers/gpu/drm/qxl/qxl_drv.h @@ -166,20 +166,6 @@ struct qxl_drm_image { struct list_head chunk_list; }; -struct qxl_fb_image { - struct qxl_device *qdev; - uint32_t pseudo_palette[16]; - struct fb_image fb_image; - uint32_t visual; -}; - -struct qxl_draw_fill { - struct qxl_device *qdev; - struct qxl_rect rect; - uint32_t color; - uint16_t rop; -}; - /* * Debugfs */ -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/3] fbcon: Drop EXPORT_SYMBOL
Every since commit 6104c37094e729f3d4ce65797002112735d49cd1 Author: Daniel Vetter Date: Tue Aug 1 17:32:07 2017 +0200 fbcon: Make fbcon a built-time depency for fbdev these are no longer distinct loadable modules, so exporting symbols is kinda pointless. Signed-off-by: Daniel Vetter Cc: Greg Kroah-Hartman Cc: Jiri Slaby Cc: Daniel Vetter Cc: Tetsuo Handa Cc: Helge Deller Cc: Peilin Ye --- drivers/video/fbdev/core/bitblit.c | 3 --- drivers/video/fbdev/core/fbcon_ccw.c| 1 - drivers/video/fbdev/core/fbcon_cw.c | 1 - drivers/video/fbdev/core/fbcon_rotate.c | 1 - drivers/video/fbdev/core/fbcon_ud.c | 1 - drivers/video/fbdev/core/softcursor.c | 2 -- drivers/video/fbdev/core/tileblit.c | 2 -- 7 files changed, 11 deletions(-) diff --git a/drivers/video/fbdev/core/bitblit.c b/drivers/video/fbdev/core/bitblit.c index 9725ecd1255b..f98e8f298bc1 100644 --- a/drivers/video/fbdev/core/bitblit.c +++ b/drivers/video/fbdev/core/bitblit.c @@ -404,6 +404,3 @@ void fbcon_set_bitops(struct fbcon_ops *ops) if (ops->rotate) fbcon_set_rotate(ops); } - -EXPORT_SYMBOL(fbcon_set_bitops); - diff --git a/drivers/video/fbdev/core/fbcon_ccw.c b/drivers/video/fbdev/core/fbcon_ccw.c index bbd869efd03b..9cd2c4b05c32 100644 --- a/drivers/video/fbdev/core/fbcon_ccw.c +++ b/drivers/video/fbdev/core/fbcon_ccw.c @@ -409,4 +409,3 @@ void fbcon_rotate_ccw(struct fbcon_ops *ops) ops->cursor = ccw_cursor; ops->update_start = ccw_update_start; } -EXPORT_SYMBOL(fbcon_rotate_ccw); diff --git a/drivers/video/fbdev/core/fbcon_cw.c b/drivers/video/fbdev/core/fbcon_cw.c index a34cbe8e9874..88d89fad3f05 100644 --- a/drivers/video/fbdev/core/fbcon_cw.c +++ b/drivers/video/fbdev/core/fbcon_cw.c @@ -392,4 +392,3 @@ void fbcon_rotate_cw(struct fbcon_ops *ops) ops->cursor = cw_cursor; ops->update_start = cw_update_start; } -EXPORT_SYMBOL(fbcon_rotate_cw); diff --git a/drivers/video/fbdev/core/fbcon_rotate.c b/drivers/video/fbdev/core/fbcon_rotate.c index ac72d4f85f7d..df6b469aa2c2 100644 --- a/drivers/video/fbdev/core/fbcon_rotate.c +++ b/drivers/video/fbdev/core/fbcon_rotate.c @@ -110,4 +110,3 @@ void fbcon_set_rotate(struct fbcon_ops *ops) break; } } -EXPORT_SYMBOL(fbcon_set_rotate); diff --git a/drivers/video/fbdev/core/fbcon_ud.c b/drivers/video/fbdev/core/fbcon_ud.c index 199cbc7abe35..8d5e66b1bdfb 100644 --- a/drivers/video/fbdev/core/fbcon_ud.c +++ b/drivers/video/fbdev/core/fbcon_ud.c @@ -436,4 +436,3 @@ void fbcon_rotate_ud(struct fbcon_ops *ops) ops->cursor = ud_cursor; ops->update_start = ud_update_start; } -EXPORT_SYMBOL(fbcon_rotate_ud); diff --git a/drivers/video/fbdev/core/softcursor.c b/drivers/video/fbdev/core/softcursor.c index fc93f254498e..29e5b21cf373 100644 --- a/drivers/video/fbdev/core/softcursor.c +++ b/drivers/video/fbdev/core/softcursor.c @@ -74,5 +74,3 @@ int soft_cursor(struct fb_info *info, struct fb_cursor *cursor) info->fbops->fb_imageblit(info, image); return 0; } - -EXPORT_SYMBOL(soft_cursor); diff --git a/drivers/video/fbdev/core/tileblit.c b/drivers/video/fbdev/core/tileblit.c index 628fe5e010c0..7539ae9040f8 100644 --- a/drivers/video/fbdev/core/tileblit.c +++ b/drivers/video/fbdev/core/tileblit.c @@ -151,5 +151,3 @@ void fbcon_set_tileops(struct vc_data *vc, struct fb_info *info) info->tileops->fb_settile(info, ); } } - -EXPORT_SYMBOL(fbcon_set_tileops); -- 2.28.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel