Re: [Intel-gfx] [PATCH v3 13/13] drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
On Fri, Oct 22, 2021 at 03:22:57PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 22, 2021 at 03:01:52PM +0300, Ville Syrjälä wrote:
> > On Fri, Oct 22, 2021 at 12:25:33PM +0200, Claudio Suarez wrote:
> > > On Thu, Oct 21, 2021 at 04:49:59PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Oct 20, 2021 at 12:51:21AM +0200, Claudio Suarez wrote:
> > > > > drm_get_edid() internally calls to
> > > > > drm_connector_update_edid_property()
> > > > > and then drm_add_display_info(), which parses the EDID.
> > > > > This happens in the function intel_hdmi_set_edid() and
> > > > > intel_sdvo_tmds_sink_detect() (via intel_sdvo_get_edid()).
> > > > >
> > > > > Once EDID is parsed, the monitor HDMI support information is available
> > > > > through drm_display_info.is_hdmi. Retriving the same information with
> > > > > drm_detect_hdmi_monitor() is less efficient. Change to
> > > > > drm_display_info.is_hdmi
> > > >
> > > > I meant we need to examine all call chains that can lead to
> > > > .detect() to make sure all of them do in fact update the
> > > > display_info beforehand.
> > >
> > > Well, I studied it carefully and, yes, all call chains that can lead to
> > > drm_display_info.is_hdmi / drm_detect_hdmi_monitor() update display_info
> > > beforehand. In the case that this doesn't happen, the code is unchanged.
> > >
> > > Do you want I explain the changes in the code here again ? Or do you want
> > > to me change the commit message to be more clear ? In the first case, I
> > > can
> > > write here a detailed explanation. In the second case I can make a longer
> > > commit
> > > message.
> > >
> > > Or both?
> >
> > I want all those call chains explained in the commit message,
> > otherwise I have no easy way to confirm whether the change
> > is correct or not.
>
> Hmm. OK, so I had a bit of a dig around and seems that what we do now
> .detect()->drm_get_edid()->drm_connector_update_edid_property()->drm_add_display_info()
Yes. I said before that I felt something was wrong when I read the
documentation and then the code. To be more explicit now, I expected that
drm_connector_update_edid_property() will be done in the
fill_modes/get_modes phase instead of when reading the edid.
The documentation suggests that but the code reads the edid in the
detect phase.
Now, since drm_connector_update_edid_property() is called in the detect
phase, it is not necessary to keep the edid data in the private connector
struct. It is in struct drm_connector from the beginning.
But this is topic for another patch.
> Now the question is when did that start happening? Looks like it was
> commit 4b4df570b41d ("drm: Update edid-derived drm_display_info fields
> at edid property set [v2]") that started to call drm_add_display_info()
> from drm_connector_update_edid_property(), and then commit 5186421cbfe2
> ("drm: Introduce epoch counter to drm_connector") started to call
> drm_connector_update_edid_property() from drm_get_edid(). Before both
> of those commits were in place display_info would still contain
> some stale garbage during .detect().
>
> That is the story I think we want in these commit messages since it
> a) explains why the old code was directly parsing the edid instead
> b) why it's now safe to change this
--commit-message?
drm/i915: replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
Commit a92d083d08b0 created the new flag is_hdmi in drm_display_info
which is set when sink compliant with CEA-861 (EDID) shall be treated
as an HDMI sink.
>From that day, this value can be used in some cases instead of
calling drm_detect_hdmi_monitor() and a second parse is avoided
because drm_detect_hdmi_monitor() parses. A TODO task was
registered in Documentation/gpu/todo.rst to perform that task in
the future.
The flag drm_display_info.is_hdmi is set in the function
drm_add_display_info(), which is called from
drm_connector_update_edid_property(). Since commit 5186421cbfe2,
drm_get_edid() calls drm_connector_update_edid_property() when
reading the edid data from an i2c adapter. Therefore, in these
cases drm_display_info.is_hdmi is updated to its correct
value when returning from drm_get_edid().
Replace drm_detect_hdmi_monitor() with drm_display_info.is_hdmi
in the cases when drm_detect_hdmi_monitor() is called after a
read from an i2c adapter using drm_get_edid() in the i915 driver.
---
>
> PS. connector->force handling in drm_get_edid() looks a bit busted
> since it doesn't call drm_connector_update_edid_property() at all
> in some cases. I think there might be some path that leads there
> anywya if/when we change connector->force, but we should fix
> drm_get_edid() to do the right thing regarless.
In those cases, the edid isn't read and NULL is returned by drm_get_edid().
No problem because display_info.is_hdmi is inside an if (edid != NULL).
BTW, struct intel_connector is allocated with kzalloc, so the initial
value of is_hdmi is zero.
[drm-tip:drm-tip 1291/1307] drivers/dma-buf/st-dma-resv.c:295:7: warning: variable 'i' is used uninitialized whenever 'if' condition is true
tree: git://anongit.freedesktop.org/drm/drm-tip drm-tip
head: 6eff63a9b932a4aa1e1f6e521cd919aaf57c058f
commit: 1d51775cd3f51899ce85afab686c7f641ff32d4e [1291/1307] dma-buf: add
dma_resv selftest v4
config: arm-buildonly-randconfig-r006-20211025 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project
a461fa64bb37cffd73f683c74f6b0780379fc2ca)
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 arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
git remote add drm-tip git://anongit.freedesktop.org/drm/drm-tip
git fetch --no-tags drm-tip drm-tip
git checkout 1d51775cd3f51899ce85afab686c7f641ff32d4e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
>> drivers/dma-buf/st-dma-resv.c:295:7: warning: variable 'i' is used
>> uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (r) {
^
drivers/dma-buf/st-dma-resv.c:336:9: note: uninitialized use occurs here
while (i--)
^
drivers/dma-buf/st-dma-resv.c:295:3: note: remove the 'if' if its condition
is always false
if (r) {
^~~~
drivers/dma-buf/st-dma-resv.c:288:6: warning: variable 'i' is used
uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
if (r) {
^
drivers/dma-buf/st-dma-resv.c:336:9: note: uninitialized use occurs here
while (i--)
^
drivers/dma-buf/st-dma-resv.c:288:2: note: remove the 'if' if its condition
is always false
if (r) {
^~~~
drivers/dma-buf/st-dma-resv.c:280:10: note: initialize the variable 'i' to
silence this warning
int r, i;
^
= 0
2 warnings generated.
vim +295 drivers/dma-buf/st-dma-resv.c
275
276 static int test_get_fences(void *arg, bool shared)
277 {
278 struct dma_fence *f, *excl = NULL, **fences = NULL;
279 struct dma_resv resv;
280 int r, i;
281
282 f = alloc_fence();
283 if (!f)
284 return -ENOMEM;
285
286 dma_resv_init(&resv);
287 r = dma_resv_lock(&resv, NULL);
288 if (r) {
289 pr_err("Resv locking failed\n");
290 goto err_free;
291 }
292
293 if (shared) {
294 r = dma_resv_reserve_shared(&resv, 1);
> 295 if (r) {
296 pr_err("Resv shared slot allocation failed\n");
297 dma_resv_unlock(&resv);
298 goto err_free;
299 }
300
301 dma_resv_add_shared_fence(&resv, f);
302 } else {
303 dma_resv_add_excl_fence(&resv, f);
304 }
305 dma_resv_unlock(&resv);
306
307 r = dma_resv_get_fences(&resv, &excl, &i, &fences);
308 if (r) {
309 pr_err("get_fences failed\n");
310 goto err_free;
311 }
312
313 if (shared) {
314 if (excl != NULL) {
315 pr_err("get_fences returned unexpected excl
fence\n");
316 goto err_free;
317 }
318 if (i != 1 || fences[0] != f) {
319 pr_err("get_fences returned unexpected shared
fence\n");
320 goto err_free;
321 }
322 } else {
323 if (excl != f) {
324 pr_err("get_fences returned unexpected excl
fence\n");
325 goto err_free;
326 }
327 if (i != 0) {
328 pr_err("get_fences returned unexpected shared
fence\n");
329 goto err_free;
330 }
331 }
332
333 dma_fence_signal(f);
334 err_free:
335 dma_fence_put(excl);
336 while (i--)
337 dma_fence_put(fences[i]);
338 kfree(fences);
339 dma_resv_fini(&resv);
340 dma_fence_put(f);
341 return r;
342 }
343
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH v11 09/16] soc: mediatek: add mtk-mmsys support for mt8195 vdosys0
Hi Fei, Thanks for the reviews. On Mon, 2021-10-25 at 13:05 +0800, Fei Shao wrote: > On Fri, Oct 22, 2021 at 6:13 PM Jason-JH Lin < > jason-jh@mediatek.com> wrote: > > > > Hi Angelo, > > > > Thanks for the reviews. > > > > > > On Thu, 2021-10-14 at 16:05 +0200, AngeloGioacchino Del Regno > > wrote: > > > > Add mt8195 vdosys0 clock driver name and routing table to > > > > the driver data of mtk-mmsys. > > > > > > > > [snip] > > > > > > > > > > --- > > > > > > Hello Jason, > > > thanks for the patch! However, there are a few things to improve: > > > > > > > [snip] > > > > > > +#define > > > > MT8195_VDO0_SEL_IN 0xf34 > > > > +#define MT8195_SEL_IN_VPP_MERGE_FROM_DSC_WRAP0_OUT (0 > > > > << > > > > 0) > > > > > > Bitshifting 0 by 0 bits == 0, so this is simply 0. > > > > > > > +#define MT8195_SEL_IN_VPP_MERGE_FROM_DISP_DITHER1 (1 > > > > << > > > > 0) > > > > > > I would write 0x1 here > > > > > > > +#define MT8195_SEL_IN_VPP_MERGE_FROM_VDO1_VIRTUAL0 (2 > > > > << > > > > 0) > > > > > > and 0x2 here: bitshifting of 0 bits makes little sense. > > > > > > > +#define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_DISP_DITHER0 > > > > (0 << 4) > > > > > > Bitshifting 0 by 4 bits is still 0, so this is again 0. > > > This is repeated too many times, so I will not list it for all of > > > the > > > occurrences. > > > > > > > +#define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_VPP_MERGE (1 > > > > << > > > > 4) > > > > > > This is BIT(4). > > > > > > > +#define MT8195_SEL_IN_DSC_WRAP1_IN_FROM_DISP_DITHER1 > > > > (0 << 5) > +#define MT8195_SEL_IN_DSC_WRAP1_IN_FROM_VPP_MERGE > > > > (1 << 5) > > > > > > ...and this is BIT(5) > > > > > > > +#define MT8195_SEL_IN_SINA_VIRTUAL0_FROM_VPP_MERGE (0 > > > > << > > > > 8) > > > > +#define MT8195_SEL_IN_SINA_VIRTUAL0_FROM_DSC_WRAP1_OUT > > > > (1 << 8) > > > > > > BIT(8) > > > > > > > +#define MT8195_SEL_IN_SINB_VIRTUAL0_FROM_DSC_WRAP0_OUT > > > > (0 << 9) > > > > +#define MT8195_SEL_IN_DP_INTF0_FROM_DSC_WRAP1_OUT (0 > > > > << > > > > 12) > > > > +#define MT8195_SEL_IN_DP_INTF0_FROM_VPP_MERGE > > > > (1 << 12) > > > > > > BIT(12) > > > > > > > +#define MT8195_SEL_IN_DP_INTF0_FROM_VDO1_VIRTUAL0 (2 > > > > << > > > > 12) > > > > > > BIT(13) > > > > > > ... and please, use the BIT(nr) macro for all these bit > > > definitions, > > > it's way more > > > readable like that. > > > > > > Regards, > > > - Angelo > > > > Because the HW register design of MT8195_VDO0_SEL_IN 0xf34 is like > > this: > > > > bit[1:0] as MT8195_SEL_IN_VPP_MERGE and > > value: 0 as MT8195_SEL_IN_VPP_MERGE_FROM_DSC_WRAP0_OUT > > value: 1 as MT8195_SEL_IN_VPP_MERGE_FROM_DISP_DITHER1 > > value: 2 as MT8195_SEL_IN_VPP_MERGE_FROM_VDO1_VIRTUAL0 > > bit[4:4] as MT8195_SEL_IN_DSC_WRAP0_IN and > > value 0 as MT8195_SEL_IN_DSC_WRAP0_IN_FROM_DISP_DITHER0 > > value 1 as MT8195_SEL_IN_DSC_WRAP0_IN_FROM_VPP_MERGE > > bit[5:5] as MT8195_SEL_IN_DSC_WRAP1_IN and > > value 0 as > > MT8195_SEL_IN_DSC_WRAP1_IN_FROM_DISP_DITHER1 > > value 1 as > > MT8195_SEL_IN_DSC_WRAP1_IN_FROM_VPP_MERGE > > and so on... > > > > I think using BIT(nr) macro directly is not easy to debug. > > > > > > Is it better to define another MACRO like this? > > > > #define BIT_VAL(val, bit) ((val) << (bit)) > > #define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_DISP_DITHER0 BIT_VAL(0, 4) > > #define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_VPP_MERGE BIT_VAL(1, 4) > > ... > > > > or > > > > #define MT8195_SEL_IN_DSC_WRAP0_IN (4) > > #define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_DISP_DITHER0 (0 > > << MT8195_SEL_IN_DSC_WRAP0_IN) > > #define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_VPP_MERGE (1 << > > MT8195_SEL_IN_DSC_WRAP0_IN) > > ... > > > > What do you think? > > Hi Jason, > > If that's the case you can still use BIT(nr) for the definitions and > describe their usage in the comment, so both code readability and the > ease of maintenance are preserved, and people can easily tell if > there > are duplicated/missing definitions while reading through the code. > Adding informative comments is never a bad thing. > > I would do something like this (and further split the definitions > into > sections by their functionalities with blank lines for visual > comfort): > > /* > * MT8195_VDO0_SEL_IN[1:0]: VPP_MERGE > * 0x0 : DSC_WRAP0_OUT > * 0x1 : DISP_DITHER1 > * 0x10: VDO1_VIRTUAL0 > */ > #define MT8195_SEL_IN_VPP_MERGE_FROM_DSC_WRAP0_OUT 0 > #define MT8195_SEL_IN_VPP_MERGE_FROM_DISP_DITHER1BIT(0) > #define MT8195_SEL_IN_VPP_MERGE_FROM_VDO1_VIRTUAL0 BIT(1) > > /* > * MT8195_VDO0_SEL_IN[4:4]: DSC_WRAP0_IN > * 0x0: DISP_DITHER0 > * 0x1: VPP_MERGE > */ > #define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_DISP_DITHER0 0 > #define MT8195_SEL_IN_DSC_WRAP0_IN_FROM_VPP_MERGEBIT(4) > ... and so on. > > Regards, > Fei > OK, I'll fix it. > > > > > > Regards, > > Jason-JH Lin > > -- Jason-JH Lin
[PATCH] mm/migrate.c: Remove MIGRATE_PFN_LOCKED
MIGRATE_PFN_LOCKED is used to indicate to migrate_vma_prepare() that a
source page was already locked during migrate_vma_collect(). If it
wasn't then the a second attempt is made to lock the page. However if
the first attempt failed it's unlikely a second attempt will succeed,
and the retry adds complexity. So clean this up by removing the retry
and MIGRATE_PFN_LOCKED flag.
Destination pages are also meant to have the MIGRATE_PFN_LOCKED flag
set, but nothing actually checks that.
Signed-off-by: Alistair Popple
---
Documentation/vm/hmm.rst | 2 +-
arch/powerpc/kvm/book3s_hv_uvmem.c | 4 +-
drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 2 -
drivers/gpu/drm/nouveau/nouveau_dmem.c | 4 +-
include/linux/migrate.h | 1 -
lib/test_hmm.c | 5 +-
mm/migrate.c | 145 +--
7 files changed, 35 insertions(+), 128 deletions(-)
diff --git a/Documentation/vm/hmm.rst b/Documentation/vm/hmm.rst
index a14c2938e7af..f2a59ed82ed3 100644
--- a/Documentation/vm/hmm.rst
+++ b/Documentation/vm/hmm.rst
@@ -360,7 +360,7 @@ between device driver specific code and shared common code:
system memory page, locks the page with ``lock_page()``, and fills in the
``dst`` array entry with::
- dst[i] = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
+ dst[i] = migrate_pfn(page_to_pfn(dpage));
Now that the driver knows that this page is being migrated, it can
invalidate device private MMU mappings and copy device private memory
diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c
b/arch/powerpc/kvm/book3s_hv_uvmem.c
index a7061ee3b157..28c436df9935 100644
--- a/arch/powerpc/kvm/book3s_hv_uvmem.c
+++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
@@ -560,7 +560,7 @@ static int __kvmppc_svm_page_out(struct vm_area_struct *vma,
gpa, 0, page_shift);
if (ret == U_SUCCESS)
- *mig.dst = migrate_pfn(pfn) | MIGRATE_PFN_LOCKED;
+ *mig.dst = migrate_pfn(pfn);
else {
unlock_page(dpage);
__free_page(dpage);
@@ -774,7 +774,7 @@ static int kvmppc_svm_page_in(struct vm_area_struct *vma,
}
}
- *mig.dst = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
+ *mig.dst = migrate_pfn(page_to_pfn(dpage));
migrate_vma_pages(&mig);
out_finalize:
migrate_vma_finalize(&mig);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
index 4a16e3c257b9..41d9417f182b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
@@ -300,7 +300,6 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct
svm_range *prange,
migrate->dst[i] = svm_migrate_addr_to_pfn(adev, dst[i]);
svm_migrate_get_vram_page(prange, migrate->dst[i]);
migrate->dst[i] = migrate_pfn(migrate->dst[i]);
- migrate->dst[i] |= MIGRATE_PFN_LOCKED;
src[i] = dma_map_page(dev, spage, 0, PAGE_SIZE,
DMA_TO_DEVICE);
r = dma_mapping_error(dev, src[i]);
@@ -580,7 +579,6 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, struct
svm_range *prange,
dst[i] >> PAGE_SHIFT, page_to_pfn(dpage));
migrate->dst[i] = migrate_pfn(page_to_pfn(dpage));
- migrate->dst[i] |= MIGRATE_PFN_LOCKED;
j++;
}
diff --git a/drivers/gpu/drm/nouveau/nouveau_dmem.c
b/drivers/gpu/drm/nouveau/nouveau_dmem.c
index 92987daa5e17..3828aafd3ac4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_dmem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_dmem.c
@@ -166,7 +166,7 @@ static vm_fault_t nouveau_dmem_fault_copy_one(struct
nouveau_drm *drm,
goto error_dma_unmap;
mutex_unlock(&svmm->mutex);
- args->dst[0] = migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
+ args->dst[0] = migrate_pfn(page_to_pfn(dpage));
return 0;
error_dma_unmap:
@@ -602,7 +602,7 @@ static unsigned long nouveau_dmem_migrate_copy_one(struct
nouveau_drm *drm,
((paddr >> PAGE_SHIFT) << NVIF_VMM_PFNMAP_V0_ADDR_SHIFT);
if (src & MIGRATE_PFN_WRITE)
*pfn |= NVIF_VMM_PFNMAP_V0_W;
- return migrate_pfn(page_to_pfn(dpage)) | MIGRATE_PFN_LOCKED;
+ return migrate_pfn(page_to_pfn(dpage));
out_dma_unmap:
dma_unmap_page(dev, *dma_addr, PAGE_SIZE, DMA_BIDIRECTIONAL);
diff --git a/include/linux/migrate.h b/include/linux/migrate.h
index c8077e936691..479b861ae490 100644
--- a/include/linux/migrate.h
+++ b/include/linux/migrate.h
@@ -119,7 +119,6 @@ static inline int migrate_misplaced_page(struct page *page,
*/
#define MIGRATE_PFN_VALID (1UL << 0)
#define MIGRATE_PFN_MIGRATE(1UL << 1)
-#define MIGRATE_PFN_LOCKED
Re: [PATCH v8 04/12] iommu/mediatek: Add device_link between the consumer and the larb devices
On Mon, 2021-10-18 at 09:13 +0200, Dafna Hirschfeld wrote:
>
> On 16.10.21 04:23, Yong Wu wrote:
> > On Mon, 2021-10-11 at 14:36 +0200, Dafna Hirschfeld wrote:
> > >
> > > On 29.09.21 03:37, Yong Wu wrote:
> > > > MediaTek IOMMU-SMI diagram is like below. all the consumer
> > > > connect
> > > > with
> > > > smi-larb, then connect with smi-common.
> > > >
> > > > M4U
> > > >|
> > > > smi-common
> > > >|
> > > > -
> > > > | |...
> > > > | |
> > > > larb1 larb2
> > > > | |
> > > > vdec venc
> > > >
> > > > When the consumer works, it should enable the smi-larb's power
> > > > which
> > > > also need enable the smi-common's power firstly.
> > > >
> > > > Thus, First of all, use the device link connect the consumer
> > > > and
> > > > the
> > > > smi-larbs. then add device link between the smi-larb and smi-
> > > > common.
> > > >
> > > > This patch adds device_link between the consumer and the larbs.
> > > >
> > > > When device_link_add, I add the flag DL_FLAG_STATELESS to avoid
> > > > calling
> > > > pm_runtime_xx to keep the original status of clocks. It can
> > > > avoid
> > > > two
> > > > issues:
> > > > 1) Display HW show fastlogo abnormally reported in [1]. At the
> > > > beggining,
> > > > all the clocks are enabled before entering kernel, but the
> > > > clocks
> > > > for
> > > > display HW(always in larb0) will be gated after clk_enable and
> > > > clk_disable
> > > > called from device_link_add(->pm_runtime_resume) and rpm_idle.
> > > > The
> > > > clock
> > > > operation happened before display driver probe. At that time,
> > > > the
> > > > display
> > > > HW will be abnormal.
> > > >
> > > > 2) A deadlock issue reported in [2]. Use DL_FLAG_STATELESS to
> > > > skip
> > > > pm_runtime_xx to avoid the deadlock.
> > > >
> > > > Corresponding, DL_FLAG_AUTOREMOVE_CONSUMER can't be added, then
> > > > device_link_removed should be added explicitly.
> > > >
> > > > [1]
> > > >
https://lore.kernel.org/linux-mediatek/1564213888.22908.4.camel@mhfsdcap03/
> > > > [2] https://lore.kernel.org/patchwork/patch/1086569/
> > > >
> > > > Suggested-by: Tomasz Figa
> > > > Signed-off-by: Yong Wu
> > > > Tested-by: Frank Wunderlich # BPI-
> > > > R2/MT7623
> > > > ---
> > > >drivers/iommu/mtk_iommu.c| 22 ++
> > > >drivers/iommu/mtk_iommu_v1.c | 20 +++-
> > > >2 files changed, 41 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/iommu/mtk_iommu.c
> > > > b/drivers/iommu/mtk_iommu.c
> > > > index d5848f78a677..a2fa55899434 100644
> > > > --- a/drivers/iommu/mtk_iommu.c
> > > > +++ b/drivers/iommu/mtk_iommu.c
> > > > @@ -560,22 +560,44 @@ static struct iommu_device
> > > > *mtk_iommu_probe_device(struct device *dev)
> > > >{
> > > > struct iommu_fwspec *fwspec =
> > > > dev_iommu_fwspec_get(dev);
> > > > struct mtk_iommu_data *data;
> > > > + struct device_link *link;
> > > > + struct device *larbdev;
> > > > + unsigned int larbid;
> > > >
> > > > if (!fwspec || fwspec->ops != &mtk_iommu_ops)
> > > > return ERR_PTR(-ENODEV); /* Not a iommu client
> > > > device
> > > > */
> > > >
> > > > data = dev_iommu_priv_get(dev);
> > > >
> > > > + /*
> > > > +* Link the consumer device with the smi-larb
> > > > device(supplier)
> > > > +* The device in each a larb is a independent HW. thus
> > > > only
> > > > link
> > > > +* one larb here.
> > > > +*/
> > > > + larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);
> > >
> > > so larbid is always the same for all the ids of a device?
> >
> > Yes. For me, each a dtsi node should represent a HW unit which can
> > only
> > connect one larb.
> >
> > > If so maybe it worth testing it and return error if this is not
> > > the
> > > case.
> >
> > Thanks for the suggestion. This is very helpful. I did see someone
> > put
> > the different larbs in one node. I will check this, and add return
>
> I am working on bugs found on media drivers, could you please point
> me to
> that wrong node?
> Will you send a fix to that node in the dtsi?
sorry. I mean it happened in the internal branch and it has already
been fixed internally, all the upstream nodes are ok for this.
Thanks
>
>
> Thanks,
> Dafna
>
> > EINVAL for this case.
>
>
>
> >
> > >
> > > Thanks,
> > > Dafna
> >
> >
> > > >
[PATCH] drm: bridge: fix unmet dependency on DRM_KMS_HELPER for DRM_PANEL_BRIDGE
When DRM_CHIPONE_ICN6211 is selected, and DRM_KMS_HELPER is not selected, Kbuild gives the following warning: WARNING: unmet direct dependencies detected for DRM_PANEL_BRIDGE Depends on [n]: HAS_IOMEM [=y] && DRM_BRIDGE [=y] && DRM_KMS_HELPER [=n] Selected by [y]: - DRM_CHIPONE_ICN6211 [=y] && HAS_IOMEM [=y] && DRM [=y] && DRM_BRIDGE [=y] && OF [=y] This is because DRM_CHIPONE_ICN6211 selects DRM_PANEL_BRIDGE without depending on or selecting DRM_KMS_HELPER, despite DRM_PANEL_BRIDGE depending on DRM_KMS_HELPER. This unmet dependency bug was detected by Kismet, a static analysis tool for Kconfig. Please advise if this is not the appropriate solution. Signed-off-by: Julian Braha --- drivers/gpu/drm/bridge/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 431b6e12a81f..a630cb8fd1c8 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -30,6 +30,7 @@ config DRM_CDNS_DSI config DRM_CHIPONE_ICN6211 tristate "Chipone ICN6211 MIPI-DSI/RGB Converter bridge" depends on OF + select DRM_KMS_HELPER select DRM_MIPI_DSI select DRM_PANEL_BRIDGE help -- 2.30.2
[PATCH v2] mailbox: remove the error message when gce clk is defer
Remove the error message when gce clk is defer.
Signed-off-by: jason-jh.lin
---
drivers/mailbox/mtk-cmdq-mailbox.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
b/drivers/mailbox/mtk-cmdq-mailbox.c
index c591dab9d5a4..2004bc112ba1 100644
--- a/drivers/mailbox/mtk-cmdq-mailbox.c
+++ b/drivers/mailbox/mtk-cmdq-mailbox.c
@@ -577,7 +577,9 @@ static int cmdq_probe(struct platform_device *pdev)
cmdq->clocks[alias_id].id = clk_id;
cmdq->clocks[alias_id].clk = of_clk_get(node,
0);
if (IS_ERR(cmdq->clocks[alias_id].clk)) {
- dev_err(dev, "failed to get gce clk:
%d\n", alias_id);
+ if (PTR_ERR(cmdq->clocks[alias_id].clk)
!= -EPROBE_DEFER)
+ dev_err(dev, "failed to get gce
clk: %d\n",
+ alias_id);
return
PTR_ERR(cmdq->clocks[alias_id].clk);
}
}
@@ -586,7 +588,8 @@ static int cmdq_probe(struct platform_device *pdev)
cmdq->clocks[alias_id].id = clk_name;
cmdq->clocks[alias_id].clk = devm_clk_get(&pdev->dev, clk_name);
if (IS_ERR(cmdq->clocks[alias_id].clk)) {
- dev_err(dev, "failed to get gce clk\n");
+ if (PTR_ERR(cmdq->clocks[alias_id].clk) !=
-EPROBE_DEFER)
+ dev_err(dev, "failed to get gce clk\n");
return PTR_ERR(cmdq->clocks[alias_id].clk);
}
}
--
2.18.0
Re: [PATCH] mailbox: remove the error message when gce clk is defer
Hi Russell,
Thanks for the reviews.
On Sun, 2021-10-24 at 21:25 +0100, Russell King (Oracle) wrote:
> On Sun, Oct 24, 2021 at 12:48:31AM +0800, jason-jh.lin wrote:
> > Remove the error message when gce clk is defer.
> >
> > Signed-off-by: jason-jh.lin
> > ---
> > drivers/mailbox/mtk-cmdq-mailbox.c | 6 --
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
> > b/drivers/mailbox/mtk-cmdq-mailbox.c
> > index fd5576a9f8b4..684b8aa1e445 100644
> > --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> > +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> > @@ -577,7 +577,8 @@ static int cmdq_probe(struct platform_device
> > *pdev)
> > snprintf(clk_id, sizeof(clk_id),
> > "%s%d", clk_name, alias_id);
> > cmdq->clocks[alias_id].id = clk_id;
> > cmdq->clocks[alias_id].clk =
> > of_clk_get(node, 0);
> > - if (IS_ERR(cmdq->clocks[alias_id].clk))
> > {
> > + if (IS_ERR(cmdq->clocks[alias_id].clk)
> > &&
> > + PTR_ERR(cmdq->clocks[alias_id].clk)
> > != -EPROBE_DEFER) {
> > dev_err(dev, "failed to get gce
> > clk: %d\n", alias_id);
> > return PTR_ERR(cmdq-
> > >clocks[alias_id].clk);
> > }
>
> So when you get -EPROBE_DEFER, you omit the error message _and_
> ignore
> the -EPROBE_DEFER. Is that really what you want to do?
>
Oh, you're right. I should return the -EPROBE_DEFER.
I just want to ignore the error msg because gce clk may not ready while
gce probing.
I'll update this patch at next version.
--
Regrads,
Jason-JH Lin
Re: [PATCH v6 14/16] drm/mediatek: add ovl_adaptor support for MT8195
Hi Angelo,
Thanks for the review.
On Fri, 2021-10-15 at 09:49 +0200, AngeloGioacchino Del Regno wrote:
> > Add ovl_adaptor driver for MT8195.
> > Ovl_adaptor is an encapsulated module and designed for simplified
> > DRM control flow. This module is composed of 8 RDMAs, 4 MERGEs and
> > an ETHDR. Two RDMAs merge into one layer, so this module support 4
> > layers.
> >
> > Signed-off-by: Nancy.Lin
> > ---
> > drivers/gpu/drm/mediatek/Makefile | 1 +
> > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 16 +
> > .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c | 498
> > ++
> > drivers/gpu/drm/mediatek/mtk_drm_drv.h| 1 +
> > 4 files changed, 516 insertions(+)
> > create mode 100644
> > drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> >
> > diff --git a/drivers/gpu/drm/mediatek/Makefile
> > b/drivers/gpu/drm/mediatek/Makefile
> > index fb158a1e7f06..3abd27d7c91d 100644
> > --- a/drivers/gpu/drm/mediatek/Makefile
> > +++ b/drivers/gpu/drm/mediatek/Makefile
> > @@ -6,6 +6,7 @@ mediatek-drm-y := mtk_disp_aal.o \
> > mtk_disp_gamma.o \
> > mtk_disp_merge.o \
> > mtk_disp_ovl.o \
> > + mtk_disp_ovl_adaptor.o \
> > mtk_disp_rdma.o \
> > mtk_drm_crtc.o \
> > mtk_drm_ddp_comp.o \
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > index 2446ad0a4977..6a4f4c42aedb 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -113,6 +113,22 @@ void mtk_rdma_enable_vblank(struct device
> > *dev,
> > void *vblank_cb_data);
> > void mtk_rdma_disable_vblank(struct device *dev);
> >
> > +int mtk_ovl_adaptor_clk_enable(struct device *dev);
> > +void mtk_ovl_adaptor_clk_disable(struct device *dev);
> > +void mtk_ovl_adaptor_config(struct device *dev, unsigned int w,
> > + unsigned int h, unsigned int vrefresh,
> > + unsigned int bpc, struct cmdq_pkt
> > *cmdq_pkt);
> > +void mtk_ovl_adaptor_layer_config(struct device *dev, unsigned int
> > idx,
> > + struct mtk_plane_state *state,
> > + struct cmdq_pkt *cmdq_pkt);
> > +void mtk_ovl_adaptor_enable_vblank(struct device *dev,
> > + void (*vblank_cb)(void *),
> > + void *vblank_cb_data);
> > +void mtk_ovl_adaptor_disable_vblank(struct device *dev);
> > +void mtk_ovl_adaptor_start(struct device *dev);
> > +void mtk_ovl_adaptor_stop(struct device *dev);
> > +unsigned int mtk_ovl_adaptor_layer_nr(struct device *dev);
> > +
> > int mtk_mdp_rdma_clk_enable(struct device *dev);
> > void mtk_mdp_rdma_clk_disable(struct device *dev);
> > void mtk_mdp_rdma_start(struct device *dev, struct cmdq_pkt
> > *cmdq_pkt);
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> > new file mode 100644
> > index ..bfb5a9d29c26
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> > @@ -0,0 +1,498 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2021 MediaTek Inc.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "mtk_drm_drv.h"
> > +#include "mtk_drm_crtc.h"
> > +#include "mtk_drm_ddp_comp.h"
> > +#include "mtk_disp_drv.h"
> > +#include "mtk_ethdr.h"
> > +
> > +#define MTK_OVL_ADAPTOR_RDMA_MAX_WIDTH 1920
> > +#define MTK_OVL_ADAPTOR_LAYER_NUM 4
> > +
> > +enum mtk_ovl_adaptor_comp_type {
> > + OVL_ADAPTOR_TYPE_RDMA = 0,
> > + OVL_ADAPTOR_TYPE_MERGE,
> > + OVL_ADAPTOR_TYPE_ETHDR,
> > + OVL_ADAPTOR_TYPE_NUM,
> > +};
> > +
> > +enum mtk_ovl_adaptor_comp_id {
> > + OVL_ADAPTOR_MDP_RDMA0,
> > + OVL_ADAPTOR_MDP_RDMA1,
> > + OVL_ADAPTOR_MDP_RDMA2,
> > + OVL_ADAPTOR_MDP_RDMA3,
> > + OVL_ADAPTOR_MDP_RDMA4,
> > + OVL_ADAPTOR_MDP_RDMA5,
> > + OVL_ADAPTOR_MDP_RDMA6,
> > + OVL_ADAPTOR_MDP_RDMA7,
> > + OVL_ADAPTOR_MERGE0,
> > + OVL_ADAPTOR_MERGE1,
> > + OVL_ADAPTOR_MERGE2,
> > + OVL_ADAPTOR_MERGE3,
> > + OVL_ADAPTOR_ETHDR0,
> > + OVL_ADAPTOR_ID_MAX
> > +};
> > +
> > +struct ovl_adaptor_comp_match {
> > + enum mtk_ovl_adaptor_comp_type type;
> > + int alias_id;
> > +};
> > +
> > +struct mtk_disp_ovl_adaptor {
> > + struct device *ovl_adaptor_comp[OVL_ADAPTOR_ID_MAX];
> > + struct device *mmsys_dev;
> > +};
> > +
> > +static const char * const ovl_adaptor_comp_str[] = {
> > + "OVL_ADAPTOR_MDP_RDMA0",
> > + "OVL_ADAPTOR_MDP_RDMA1",
> > + "OVL_ADAPTOR_MDP_RDMA2",
> > + "OVL_ADAPTOR_MDP_RDMA3",
> > + "OVL_ADAPTOR_MDP_RDMA4",
> > + "OVL_ADAPTOR_MDP_RDMA5",
> > + "OVL_ADAPTOR_MDP_RDMA6",
> > + "OVL_ADAPTOR_MDP_RDMA7",
> > + "OVL_ADAPTOR_MERGE0",
Re: [PATCH v6 13/16] drm/mediatek: add ETHDR support for MT8195
Hi Chun-Kuang,
Thanks for the review.
On Thu, 2021-10-21 at 23:44 +0800, Chun-Kuang Hu wrote:
> Hi, Nancy:
>
> Nancy.Lin 於 2021年10月4日 週一 下午2:22寫道：
> >
> > ETHDR is a part of ovl_adaptor.
> > ETHDR is designed for HDR video and graphics conversion in the
> > external
> > display path. It handles multiple HDR input types and performs tone
> > mapping, color space/color format conversion, and then combine
> > different layers, output the required HDR or SDR signal to the
> > subsequent display path.
> >
> > Signed-off-by: Nancy.Lin
> > ---
> > drivers/gpu/drm/mediatek/Makefile| 1 +
> > drivers/gpu/drm/mediatek/mtk_ethdr.c | 403
> > +++
> > drivers/gpu/drm/mediatek/mtk_ethdr.h | 25 ++
> > 3 files changed, 429 insertions(+)
> > create mode 100644 drivers/gpu/drm/mediatek/mtk_ethdr.c
> > create mode 100644 drivers/gpu/drm/mediatek/mtk_ethdr.h
> >
> > diff --git a/drivers/gpu/drm/mediatek/Makefile
> > b/drivers/gpu/drm/mediatek/Makefile
> > index 6e604a933ed0..fb158a1e7f06 100644
> > --- a/drivers/gpu/drm/mediatek/Makefile
> > +++ b/drivers/gpu/drm/mediatek/Makefile
> > @@ -14,6 +14,7 @@ mediatek-drm-y := mtk_disp_aal.o \
> > mtk_drm_plane.o \
> > mtk_dsi.o \
> > mtk_dpi.o \
> > + mtk_ethdr.o \
> > mtk_mdp_rdma.o
> >
> > obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o
> > diff --git a/drivers/gpu/drm/mediatek/mtk_ethdr.c
> > b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> > new file mode 100644
> > index ..99e5a95aebed
> > --- /dev/null
> > +++ b/drivers/gpu/drm/mediatek/mtk_ethdr.c
> > @@ -0,0 +1,403 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2021 MediaTek Inc.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "mtk_drm_crtc.h"
> > +#include "mtk_drm_ddp_comp.h"
> > +#include "mtk_drm_drv.h"
> > +#include "mtk_ethdr.h"
> > +
> > +#define MIX_INTEN 0x4
> > + #define MIX_FME_CPL_INTEN BIT(1)
>
> Align the indent with mdp rdma driver.
>
OK.
> > +#define MIX_INTSTA 0x8
> > +#define MIX_EN 0xc
> > +#define MIX_RST0x14
> > +#define MIX_ROI_SIZE 0x18
> > +#define MIX_DATAPATH_CON 0x1c
> > + #define OUTPUT_NO_RND BIT(3)
> > + #define SOURCE_RGB_SEL BIT(7)
> > + #define BACKGROUND_RELAY(4 << 9)
> > +#define MIX_ROI_BGCLR 0x20
> > + #define BGCLR_BLACK 0xff00
> > +#define MIX_SRC_CON0x24
> > + #define MIX_SRC_L0_EN BIT(0)
> > +#define MIX_L_SRC_CON(n) (0x28 + 0x18 * (n))
> > + #define NON_PREMULTI_SOURCE (2 << 12)
> > +#define MIX_L_SRC_SIZE(n) (0x30 + 0x18 * (n))
> > +#define MIX_L_SRC_OFFSET(n)(0x34 + 0x18 * (n))
> > +#define MIX_FUNC_DCM0 0x120
> > +#define MIX_FUNC_DCM1 0x124
> > + #define MIX_FUNC_DCM_ENABLE 0x
> > +
> > +#define HDR_VDO_FE_0804_HDR_DM_FE 0x804
> > + #define HDR_VDO_FE_0804_BYPASS_ALL 0xfd
> > +#define HDR_GFX_FE_0204_GFX_HDR_FE 0x204
> > + #define HDR_GFX_FE_0204_BYPASS_ALL 0xfd
> > +#define HDR_VDO_BE_0204_VDO_DM_BE 0x204
> > + #define HDR_VDO_BE_0204_BYPASS_ALL 0x7e
> > +
> > +#define MIXER_INx_MODE_BYPASS 0
>
> MIXER_INX_MODE_BYPASS
>
OK.
> > +#define MIXER_INx_MODE_EVEN_EXTEND 1
> > +#define MIXER_INx_MODE_ODD_EXTEND 2
> > +#define DEFAULT_9BIT_ALPHA 0x100
> > +#defineMIXER_ALPHA_AEN BIT(8)
> > +#defineMIXER_ALPHA 0xff
> > +#define ETHDR_CLK_NUM 13
> > +
> > +enum mtk_ethdr_comp_id {
> > + ETHDR_MIXER,
> > + ETHDR_VDO_FE0,
> > + ETHDR_VDO_FE1,
> > + ETHDR_GFX_FE0,
> > + ETHDR_GFX_FE1,
> > + ETHDR_VDO_BE,
> > + ETHDR_ADL_DS,
> > + ETHDR_ID_MAX
> > +};
> > +
> > +struct mtk_ethdr_comp {
> > + struct device *dev;
> > + void __iomem *regs;
> > + struct cmdq_client_reg cmdq_base;
> > +};
> > +
> > +struct mtk_ethdr {
> > + struct mtk_ethdr_comp ethdr_comp[ETHDR_ID_MAX];
> > + struct clk_bulk_data ethdr_clk[ETHDR_CLK_NUM];
> > + struct device *mmsys_dev;
> > + spinlock_t lock; /* protects vblank_cb and vblank_cb_data
> > */
> > + void (*vblank_cb)(void *data);
> > + void *vblank_cb_data;
> > + int irq;
> > +};
> > +
> > +static const char * const ethdr_comp_str[] = {
> > + "ETHDR_MIXER",
> > + "ETHDR_VDO_FE0",
> > + "ETHDR_VDO_FE1",
> > + "ETHDR_GFX_FE0",
> > + "ETHDR_GFX_FE1",
> > + "ETHDR_VDO_BE",
> > + "ETHDR_ADL_DS",
> > + "ETHDR_ID_MAX"
> > +};
> > +
> > +static const char * const ethdr_clk_str[] = {
> > + "ethdr_top",
> > + "mixer",
> > + "vdo_fe0",
> > + "vdo_fe1",
> > + "gfx_fe0",
> > + "gfx_fe1",
> > +
Re: [PATCH v6 12/16] drm/mediatek: add display merge api support for MT8195
Hi Chun-Kuang,
Thanks for the review.
On Thu, 2021-10-21 at 23:02 +0800, Chun-Kuang Hu wrote:
> >
> Hi, Nancy:
>
> Nancy.Lin 於 2021年10月4日 週一 下午2:21寫道：
> >
> > Add merge new API.
> > 1. Vdosys1 merge1~merge4 support HW mute function, so add unmute
> > API.
> > 2. Add merge new advance config API. The original merge API is
> >mtk_ddp_comp_funcs function prototype. The API interface
> > parameters
> >cannot be modified, so add a new config API for extension.
> > 3. Add merge enable/disable API for cmdq support. The ovl_adaptor
> > merges
> >are configured with each drm plane update. Need to
> > enable/disable
> >merge with cmdq making sure all the settings taken effect in the
> >same vblank.
>
> Separate this patch into three patches.
>
OK.
> > Signed-off-by: Nancy.Lin
> > ---
> > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 6 ++
> > drivers/gpu/drm/mediatek/mtk_disp_merge.c | 86
> > ---
> > 2 files changed, 82 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > index b3a372cab0bd..2446ad0a4977 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h
> > @@ -63,6 +63,12 @@ void mtk_merge_config(struct device *dev,
> > unsigned int width,
> > unsigned int bpc, struct cmdq_pkt *cmdq_pkt);
> > void mtk_merge_start(struct device *dev);
> > void mtk_merge_stop(struct device *dev);
> > +void mtk_merge_advance_config(struct device *dev, unsigned int
> > l_w, unsigned int r_w,
> > + unsigned int h, unsigned int
> > vrefresh, unsigned int bpc,
> > + struct cmdq_pkt *cmdq_pkt);
> > +void mtk_merge_unmute(struct device *dev, struct cmdq_pkt
> > *cmdq_pkt);
> > +void mtk_merge_enable(struct device *dev, struct cmdq_pkt
> > *cmdq_pkt);
> > +void mtk_merge_disable(struct device *dev, struct cmdq_pkt
> > *cmdq_pkt);
> >
> > void mtk_ovl_bgclr_in_on(struct device *dev);
> > void mtk_ovl_bgclr_in_off(struct device *dev);
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> > index b05e1df79c3d..696bb948352b 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_merge.c
> > @@ -17,6 +17,7 @@
> > #define DISP_REG_MERGE_CTRL0x000
> > #define MERGE_EN 1
> > #define DISP_REG_MERGE_CFG_0 0x010
> > +#define DISP_REG_MERGE_CFG_1 0x014
> > #define DISP_REG_MERGE_CFG_4 0x020
> > #define DISP_REG_MERGE_CFG_10 0x038
> > /* no swap */
> > @@ -25,9 +26,12 @@
> > #define DISP_REG_MERGE_CFG_12 0x040
> > #define CFG_10_10_1PI_2PO_BUF_MODE 6
> > #define CFG_10_10_2PI_2PO_BUF_MODE 8
> > +#define CFG_11_10_1PI_2PO_MERGE18
> > #define FLD_CFG_MERGE_MODE GENMASK(4, 0)
> > #define DISP_REG_MERGE_CFG_24 0x070
> > #define DISP_REG_MERGE_CFG_25 0x074
> > +#define DISP_REG_MERGE_CFG_26 0x078
> > +#define DISP_REG_MERGE_CFG_27 0x07c
> > #define DISP_REG_MERGE_CFG_36 0x0a0
> > #define ULTRA_EN BIT(0)
> > #define PREULTRA_ENBIT(4)
> > @@ -54,26 +58,52 @@
> > #define FLD_PREULTRA_TH_LOWGENMASK(15, 0)
> > #define FLD_PREULTRA_TH_HIGH GENMASK(31, 16)
> >
> > +#define DISP_REG_MERGE_MUTE_0 0xf00
> > +
> > struct mtk_disp_merge {
> > void __iomem *regs;
> > struct clk *clk;
> > struct clk *async_clk;
> > struct cmdq_client_reg cmdq_reg;
> > boolfifo_en;
> > + boolmute_support;
>
> Align indent of members.
>
OK.
> > };
> >
> > void mtk_merge_start(struct device *dev)
> > +{
> > + mtk_merge_enable(dev, NULL);
> > +}
> > +
> > +void mtk_merge_stop(struct device *dev)
> > {
> > struct mtk_disp_merge *priv = dev_get_drvdata(dev);
> >
> > - writel(MERGE_EN, priv->regs + DISP_REG_MERGE_CTRL);
> > + mtk_merge_disable(dev, NULL);
> > }
> >
> > -void mtk_merge_stop(struct device *dev)
> > +void mtk_merge_enable(struct device *dev, struct cmdq_pkt
> > *cmdq_pkt)
>
> The difference of mtk_merge_enable() and mtk_merge_start() is cmdq
> support, but the naming make them so different. So I would like this
> function name to be mtk_merge_start_cmdq().
>
OK.
> > +{
> > + struct mtk_disp_merge *priv = dev_get_drvdata(dev);
> > +
> > + mtk_ddp_write(cmdq_pkt, 1, &priv->cmdq_reg, priv->regs,
> > + DISP_REG_MERGE_CTRL);
> > +}
> > +
> > +void mtk_merge_disable(struct device *dev, struct cmdq_pkt
> > *cmdq_pkt)
>
> Ditto.
OK.
>
> > {
> > struct mtk_disp_merge *priv = dev_get_drvdata(dev);
> >
> >
[PATCH] drm: Fix wrong use of connector in vop_get_edp_connector
>From the comments of drm_for_each_connector_iter(), we know
that "connector is only valid within the list body, if you
want to use connector after calling drm_connector_list_iter_end()
then you need to grab your own reference first using
drm_connector_get()". So fix the wrong use of connector
according to the comments and then call drm_connector_put()
after using connector finishes.
Signed-off-by: He Ying
Fixes: 2cbeb64f6c70 ("drm/rockchip: use drm_for_each_connector_iter()")
---
drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index a25b98b7f5bd..469922ad29fa 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -1581,19 +1581,19 @@ static void vop_crtc_reset(struct drm_crtc *crtc)
#ifdef CONFIG_DRM_ANALOGIX_DP
static struct drm_connector *vop_get_edp_connector(struct vop *vop)
{
- struct drm_connector *connector;
+ struct drm_connector *connector = NULL;
struct drm_connector_list_iter conn_iter;
drm_connector_list_iter_begin(vop->drm_dev, &conn_iter);
drm_for_each_connector_iter(connector, &conn_iter) {
if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
- drm_connector_list_iter_end(&conn_iter);
- return connector;
+ drm_connector_get(connector);
+ break;
}
}
drm_connector_list_iter_end(&conn_iter);
- return NULL;
+ return connector;
}
static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
@@ -1614,6 +1614,7 @@ static int vop_crtc_set_crc_source(struct drm_crtc *crtc,
else
ret = -EINVAL;
+ drm_connector_put(connector);
return ret;
}
--
2.17.1
Re: [PATCH v6 11/16] drm/mediatek: add display MDP RDMA support for MT8195
Hi Chun-Kuang, Thanks for the review. On Wed, 2021-10-20 at 00:38 +0800, Chun-Kuang Hu wrote: > Hi, Nancy: > > Nancy.Lin 於 2021年10月4日 週一 下午2:21寫道： > > > > Add MDP_RDMA driver for MT8195. MDP_RDMA is the DMA engine of > > the ovl_adaptor component. > > > > Signed-off-by: Nancy.Lin > > --- > > drivers/gpu/drm/mediatek/Makefile | 3 +- > > drivers/gpu/drm/mediatek/mtk_disp_drv.h | 7 + > > drivers/gpu/drm/mediatek/mtk_mdp_rdma.c | 305 > > > > drivers/gpu/drm/mediatek/mtk_mdp_rdma.h | 19 ++ > > 4 files changed, 333 insertions(+), 1 deletion(-) > > create mode 100644 drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > > create mode 100644 drivers/gpu/drm/mediatek/mtk_mdp_rdma.h > > > > diff --git a/drivers/gpu/drm/mediatek/Makefile > > b/drivers/gpu/drm/mediatek/Makefile > > index a38e88e82d12..6e604a933ed0 100644 > > --- a/drivers/gpu/drm/mediatek/Makefile > > +++ b/drivers/gpu/drm/mediatek/Makefile > > @@ -13,7 +13,8 @@ mediatek-drm-y := mtk_disp_aal.o \ > > mtk_drm_gem.o \ > > mtk_drm_plane.o \ > > mtk_dsi.o \ > > - mtk_dpi.o > > + mtk_dpi.o \ > > + mtk_mdp_rdma.o > > > > obj-$(CONFIG_DRM_MEDIATEK) += mediatek-drm.o > > > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > index a33b13fe2b6e..b3a372cab0bd 100644 > > --- a/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > +++ b/drivers/gpu/drm/mediatek/mtk_disp_drv.h > > @@ -8,6 +8,7 @@ > > > > #include > > #include "mtk_drm_plane.h" > > +#include "mtk_mdp_rdma.h" > > > > int mtk_aal_clk_enable(struct device *dev); > > void mtk_aal_clk_disable(struct device *dev); > > @@ -106,4 +107,10 @@ void mtk_rdma_enable_vblank(struct device > > *dev, > > void *vblank_cb_data); > > void mtk_rdma_disable_vblank(struct device *dev); > > > > +int mtk_mdp_rdma_clk_enable(struct device *dev); > > +void mtk_mdp_rdma_clk_disable(struct device *dev); > > +void mtk_mdp_rdma_start(struct device *dev, struct cmdq_pkt > > *cmdq_pkt); > > +void mtk_mdp_rdma_stop(struct device *dev, struct cmdq_pkt > > *cmdq_pkt); > > +void mtk_mdp_rdma_config(struct device *dev, struct > > mtk_mdp_rdma_cfg *cfg, > > +struct cmdq_pkt *cmdq_pkt); > > #endif > > diff --git a/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > > b/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > > new file mode 100644 > > index ..d05b1ef976bc > > --- /dev/null > > +++ b/drivers/gpu/drm/mediatek/mtk_mdp_rdma.c > > @@ -0,0 +1,305 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2021 MediaTek Inc. > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include "mtk_drm_drv.h" > > +#include "mtk_disp_drv.h" > > Alphabetic order. OK. > > > +#include "mtk_mdp_rdma.h" > > + > > +#define MDP_RDMA_EN0x000 > > +#define FLD_ROT_ENABLE BIT(0) > > Use one 'tab' to replace 8 space. OK. > > > +#define MDP_RDMA_RESET 0x008 > > +#define MDP_RDMA_CON 0x020 > > +#define FLD_OUTPUT_10B BIT(5) > > +#define FLD_SIMPLE_MODEBIT(4) > > +#define MDP_RDMA_GMCIF_CON 0x028 > > +#define FLD_COMMAND_DIVBIT(0) > > +#define FLD_EXT_PREULTRA_ENBIT(3) > > +#define FLD_RD_REQ_TYPEGENMASK(7, > > 4) > > +#define VAL_RD_REQ_TYPE_BURST_8_ACCESS 7 > > +#define FLD_ULTRA_EN GENMASK(13, > > 12) > > +#define VAL_ULTRA_EN_ENABLE1 > > +#define FLD_PRE_ULTRA_EN GENMASK(17, > > 16) > > +#define VAL_PRE_ULTRA_EN_ENABLE1 > > +#define FLD_EXT_ULTRA_EN BIT(18) > > +#define MDP_RDMA_SRC_CON 0x030 > > +#define FLD_OUTPUT_ARGBBIT(25) > > +#define FLD_BIT_NUMBER GENMASK(19, > > 18) > > +#define FLD_SWAP BIT(14) > > +#define FLD_UNIFORM_CONFIG BIT(17) > > +#define RDMA_INPUT_10BIT BIT(18) > > +#define FLD_SRC_FORMAT GENMASK(3, > > 0) > > +#define MDP_RDMA_COMP_CON 0x038 > > +#define FLD_AFBC_ENBIT(22) > > +#define FLD_AFBC_YUV_TRANSFORM BIT(21) > > +#define FLD_UFBDC_EN BIT(12) > > +#define MDP_RDMA_MF_BKGD_SIZE_IN_BYTE 0x060 > > +#define FLD_MF_BKGD_WB GENMASK(22, > > 0) > > +#define MDP_RDMA_MF_SRC_SIZE
Re: [RFC PATCH] drm/aperture: Add param to disable conflicting framebuffers removal
On Sun, Oct 24, 2021 at 4:40 PM Javier Martinez Canillas wrote: > > Hello Thomas, > > Thanks a lot for your feedback. > > On 10/22/21 21:05, Thomas Zimmermann wrote: > > > There's still the question of the semantics of this parameter. It's a > > bit fuzzy. > > > > If you use 'disable_handover' (as you mentioned in another mail), it > > would mean that only the handover itself is disabled. So if simpledrm is > > not bound to the device, then a native driver should load. That would be > > hard to implement with the current code base, where we have to take old > > fbdev drivers into account. > > > > (And to be pedantic, we don't really do a handover of the device. We > > hot-unplug the generic platform device, so that the driver for the > > native device can operate the HW without interference.) > > > > Simpledrm only acquires an aperture, but never removes a driver. If > > there is a driver already, simpledrm would fail. Only native drivers try > > > to remove drivers and would trigger the test. So your patch is more > > something like 'disable_native_drivers'. > > > > I'd go with 'disable_native_drivers', or maybe 'disable_device_handover' > > That works for me and "drm.disable_native_drivers" is also what Neal meant > with his "drm.noplatformdrv", but yours is much easier to remember / type. > I'm good with that too. :) -- 真実はいつも一つ！/ Always, there's only one truth!
Re: [RFC PATCH] drm/aperture: Add param to disable conflicting framebuffers removal
Hello Thomas,
Thanks a lot for your feedback.
On 10/22/21 21:05, Thomas Zimmermann wrote:
[snip]
>>
>> +static bool drm_aperture_remove_fb = true;
>
> Global variables should default to zero if somehow possible. This way,
> they can all be stored in the BSS segment and backed by a single shared
> zero-filled page. Otherwise they require actual memory. In the worst
> case, you'd allocate a full page to hold a single boolean.
>
>> +module_param_named(remove_fb, drm_aperture_remove_fb, bool, 0600);
>> +MODULE_PARM_DESC(remove_fb,
>> + "Allow conflicting framebuffers removal [default=true]");
>> +
>
> And with variables set to zero, a command-line parameter enables
> non-default behavior (i.e., "drm-param=1"). That more logical than the
> other way around IMHO.
>
Agreed. I'll change that.
>> /**
>>* DOC: overview
>>*
>> @@ -283,6 +288,9 @@ static void drm_aperture_detach_drivers(resource_size_t
>> base, resource_size_t si
>>* This function removes graphics device drivers which use memory range
>> described by
>>* @base and @size.
>>*
>> + * The conflicting framebuffers removal can be disabled by setting the
>> drm.remove_fb=0 kernel
>> + * command line option. When this is disabled, the function will return an
>> -EINVAL errno code.
>
> Please use -EBUSY for the error. That's what the acquire function
> returns in case of a conflict.
>
Sure, makes sense. I was pondering between -EINVAL, -EBUSY and -EPERM.
>> + *
>>* Returns:
>>* 0 on success, or a negative errno code otherwise
>>*/
>> @@ -292,7 +300,12 @@ int
>> drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_
>> #if IS_REACHABLE(CONFIG_FB)
>
> Rather not split up this block. It's better style to put the
> fbdev-related code into a helper and call it unconditionally.
>
> static drm_aperture_remove_conflicting_fbdev_framebuffers()
> {
> #if (FB)
> ...
> #endif
> return 0;
> }
>
Ok.
>> struct apertures_struct *a;
>> int ret;
>> +#endif
>> +
>> +if (!drm_aperture_remove_fb)
>> +return -EINVAL;
>
> There's still the question of the semantics of this parameter. It's a
> bit fuzzy.
>
> If you use 'disable_handover' (as you mentioned in another mail), it
> would mean that only the handover itself is disabled. So if simpledrm is
> not bound to the device, then a native driver should load. That would be
> hard to implement with the current code base, where we have to take old
> fbdev drivers into account.
>
> (And to be pedantic, we don't really do a handover of the device. We
> hot-unplug the generic platform device, so that the driver for the
> native device can operate the HW without interference.)
>
> Simpledrm only acquires an aperture, but never removes a driver. If
> there is a driver already, simpledrm would fail. Only native drivers try > to
> remove drivers and would trigger the test. So your patch is more
> something like 'disable_native_drivers'.
>
> I'd go with 'disable_native_drivers', or maybe 'disable_device_handover'
That works for me and "drm.disable_native_drivers" is also what Neal meant
with his "drm.noplatformdrv", but yours is much easier to remember / type.
Best regards,
--
Javier Martinez Canillas
Linux Engineering
Red Hat
Re: [RFC PATCH] drm/aperture: Add param to disable conflicting framebuffers removal
Hello Ville, On 10/22/21 21:12, Ville Syrjälä wrote: > On Fri, Oct 22, 2021 at 04:40:40PM +0200, Javier Martinez Canillas wrote: >> The simpledrm driver allows to use the frame buffer that was set-up by the >> firmware. This gives early video output before the platform DRM driver is >> probed and takes over. >> >> But it would be useful to have a way to disable this take over by the real >> DRM drivers. For example, there may be bugs in the DRM drivers that could >> cause the display output to not work correctly. >> >> For those cases, it would be good to keep the simpledrm driver instead and >> at least get a working display as set-up by the firmware. >> >> Let's add a drm.remove_fb boolean kernel command line parameter, that when >> set to false will prevent the conflicting framebuffers to being removed. >> >> Since the drivers call drm_aperture_remove_conflicting_framebuffers() very >> early in their probe callback, this will cause the drivers' probe to fail. > > Why is that better than just modprobe.blacklisting those drivers? > Because would allow to deny list all native (as Thomas called it) DRM drivers and only allow the simpledrm driver to be probed. This is useful for distros, since could add a "Basic graphics mode" to the boot menu entries, that could boot the kernel passing a "drm.disable_native_drivers=1" cmdline option. That way, if there's any problem with a given DRM driver, the distro may be installed and booted using the simpledrm driver and troubleshoot why a native DRM driver is not working. Or try updating the kernel package, etc. Currently what most distros do is to pass "nomodeset" in this mode. But now that we have the simpledrm driver, would be nice to just use the frame buffer set by the system firmware in those cases. Best regards, -- Javier Martinez Canillas Linux Engineering Red Hat
Re: [PATCH] mailbox: remove the error message when gce clk is defer
On Sun, Oct 24, 2021 at 12:48:31AM +0800, jason-jh.lin wrote:
> Remove the error message when gce clk is defer.
>
> Signed-off-by: jason-jh.lin
> ---
> drivers/mailbox/mtk-cmdq-mailbox.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c
> b/drivers/mailbox/mtk-cmdq-mailbox.c
> index fd5576a9f8b4..684b8aa1e445 100644
> --- a/drivers/mailbox/mtk-cmdq-mailbox.c
> +++ b/drivers/mailbox/mtk-cmdq-mailbox.c
> @@ -577,7 +577,8 @@ static int cmdq_probe(struct platform_device *pdev)
> snprintf(clk_id, sizeof(clk_id), "%s%d",
> clk_name, alias_id);
> cmdq->clocks[alias_id].id = clk_id;
> cmdq->clocks[alias_id].clk = of_clk_get(node,
> 0);
> - if (IS_ERR(cmdq->clocks[alias_id].clk)) {
> + if (IS_ERR(cmdq->clocks[alias_id].clk) &&
> + PTR_ERR(cmdq->clocks[alias_id].clk) !=
> -EPROBE_DEFER) {
> dev_err(dev, "failed to get gce clk:
> %d\n", alias_id);
> return
> PTR_ERR(cmdq->clocks[alias_id].clk);
> }
So when you get -EPROBE_DEFER, you omit the error message _and_ ignore
the -EPROBE_DEFER. Is that really what you want to do?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
[PATCH] drm/i915/vlv_dsi: Double pixelclock on read-back for dual-link panels
In intel_dsi_get_config() double the pclk returned by foo_dsi_get_pclk()
for dual-link panels. This fixes the following WARN triggering:
i915 :00:02.0: [drm] *ERROR* [CRTC:51:pipe A] mismatch in pixel_rate
(expected 235710, found 118056)
i915 :00:02.0: [drm] *ERROR* [CRTC:51:pipe A] mismatch in
hw.pipe_mode.crtc_clock (expected 235710, found 118056)
i915 :00:02.0: [drm] *ERROR* [CRTC:51:pipe A] mismatch in
hw.adjusted_mode.crtc_clock (expected 235710, found 118056)
i915 :00:02.0: [drm] *ERROR* [CRTC:51:pipe A] mismatch in port_clock
(expected 235710, found 118056)
[ cut here ]
pipe state doesn't match!
WARNING: CPU: 3 PID: 136 at drivers/gpu/drm/i915/display/intel_display.c:9125
intel_display_finish_reset+0x1bd3/0x2050 [i915]
...
This has been tested on a Xiaomi Mi Pad 2 (with CHT x5-Z8500 SoC) tablet,
with a 1536x2048 dual-link DSI panel.
Note this fix was taken from icl_dsi.c which does the same in
its get_config().
Cc: Tsuchiya Yuto
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/i915/display/vlv_dsi.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c
b/drivers/gpu/drm/i915/display/vlv_dsi.c
index 3e646a58b38a..2b7909bc52ff 100644
--- a/drivers/gpu/drm/i915/display/vlv_dsi.c
+++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
@@ -1265,7 +1265,9 @@ static void intel_dsi_get_config(struct intel_encoder
*encoder,
struct intel_crtc_state *pipe_config)
{
struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+ struct intel_dsi *intel_dsi = enc_to_intel_dsi(encoder);
u32 pclk;
+
drm_dbg_kms(&dev_priv->drm, "\n");
pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
@@ -1277,6 +1279,9 @@ static void intel_dsi_get_config(struct intel_encoder
*encoder,
pclk = vlv_dsi_get_pclk(encoder, pipe_config);
}
+ if (intel_dsi->dual_link)
+ pclk *= 2;
+
if (pclk) {
pipe_config->hw.adjusted_mode.crtc_clock = pclk;
pipe_config->port_clock = pclk;
--
2.31.1
[PATCH] drm/i915: Add NO_VLV_DISP_PW_DPIO_CMN_BC_INIT quirk
Add a NO_VLV_DISP_PW_DPIO_CMN_BC_INIT quirk to fix i915 not working on
the Xiaomi Mi Pad 2 (with CHT x5-Z8500 SoC).
The Xiaomi Mi Pad 2 uses quite an unusual hardware-design for a Cherry
Trail tablet. It deviates from the typical reference design based tablets
in many ways.
The Mi Pad 2 does not have any DisplayPort or HDMI outouts. I suspect that
as part of its unusual design it also has some supply rail which is only
used for DisplayPort or HDMI not connected.
Force-enabling the dpio-common-bc powerwell as the i915 normal does at boot
appears to cause the P-Unit to hang. When booting with a serial-usb console
the following errors are logged before the system freezes:
i915 :00:02.0: [drm] *ERROR* timeout setting power well state
(f3ff)
i915 :00:02.0: [drm] *ERROR* Display PHY 0 is not power up
[ cut here ]
i915 :00:02.0: DPIO read pipe A reg 0x8170 == 0x
WARNING: CPU: 3 PID: 258 at drivers/gpu/drm/i915/intel_sideband.c:257
vlv_dpio_read+0x95/0xb0 [i915]
...
Call Trace:
chv_dpio_cmn_power_well_enable+0xab/0x210 [i915]
__intel_display_power_get_domain.part.0+0xa0/0xc0 [i915]
intel_power_domains_init_hw+0x26d/0x760 [i915]
intel_modeset_init_noirq+0x5d/0x270 [i915]
i915_driver_probe+0x6b6/0xd10 [i915]
...
If I disable the WARN about the register being 0x, so that the
system can log some more dmesg output over the serial console before
freezing, the following errors are also logged:
i915 :00:02.0: [drm] *ERROR* timeout setting power well state
(fcfff3ff)
i915 :00:02.0: [drm] *ERROR* Display PHY 1 is not power up
With this patch to disable the force-enabling of the PHY 0 / dpio-common-bc
powerwell in place, this error for PHY 1 goes away. So it seems that trying
the force-enabling of the PHY 0 / dpio-common-bc powerwell freezes the
P-Unit, causing the subsequent enabling of PHY 1 to also fail (and causing
the entire system to freeze within seconds).
With this patch the PHY 1 error disappears and the entire system works.
Note this change also moves the intel_init_quirks() call a bit up inside
intel_modeset_init_noirq() this is necessary so that the quirk is set
before the intel_power_domains_init_hw() call. This is harmless, all that
intel_init_quirks() does is set some bits in drm_i915_private.quirks and
make some drm_info() log calls.
Reported-by: Tsuchiya Yuto
Signed-off-by: Hans de Goede
---
drivers/gpu/drm/i915/display/intel_display.c | 4 ++--
.../gpu/drm/i915/display/intel_display_power.c | 16 ++--
drivers/gpu/drm/i915/display/intel_quirks.c | 10 ++
drivers/gpu/drm/i915/i915_drv.h | 1 +
4 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 015854b5078c..1fb885cc86c9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12467,6 +12467,8 @@ int intel_modeset_init_noirq(struct drm_i915_private
*i915)
if (ret)
goto cleanup_bios;
+ intel_init_quirks(i915);
+
/* FIXME: completely on the wrong abstraction layer */
intel_power_domains_init_hw(i915, false);
@@ -12501,8 +12503,6 @@ int intel_modeset_init_noirq(struct drm_i915_private
*i915)
INIT_WORK(&i915->atomic_helper.free_work,
intel_atomic_helper_free_state_worker);
- intel_init_quirks(i915);
-
intel_fbc_init(i915);
return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c
b/drivers/gpu/drm/i915/display/intel_display_power.c
index cce1a926fcc1..eeaba3dc064b 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -2090,8 +2090,14 @@ __intel_display_power_get_domain(struct drm_i915_private
*dev_priv,
if (intel_display_power_grab_async_put_ref(dev_priv, domain))
return;
- for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain))
+ for_each_power_domain_well(dev_priv, power_well, BIT_ULL(domain)) {
+ if (domain == POWER_DOMAIN_INIT &&
+ (dev_priv->quirks & QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT)
&&
+ power_well->desc->id == VLV_DISP_PW_DPIO_CMN_BC)
+ continue;
+
intel_power_well_get(dev_priv, power_well);
+ }
power_domains->domain_use_count[domain]++;
}
@@ -2184,8 +2190,14 @@ __intel_display_power_put_domain(struct drm_i915_private
*dev_priv,
power_domains->domain_use_count[domain]--;
- for_each_power_domain_well_reverse(dev_priv, power_well,
BIT_ULL(domain))
+ for_each_power_domain_well_reverse(dev_priv, power_well,
BIT_ULL(domain)) {
+ if (domain == POWER_DOMAIN_INIT &&
+ (dev_priv->quirks & QUIRK_NO_VLV_DISP_PW_DPIO_CMN_BC_INIT)
&&
+
Re: [PATCH v5 2/7] dt-bindings: mediatek, dp: Add Display Port binding
On Fri, Oct 22, 2021 at 06:19:13PM -0500, Rob Herring wrote: > On Thu, 21 Oct 2021 11:27:02 +0200, Markus Schneider-Pargmann wrote: > > This controller is present on several mediatek hardware. Currently > > mt8195 and mt8395 have this controller without a functional difference, > > so only one compatible field is added. > > > > The controller can have two forms, as a normal display port and as an > > embedded display port. > > > > Signed-off-by: Markus Schneider-Pargmann > > --- > > > > Notes: > > Changes v4 -> v5: > > - Removed "status" in the example > > - Remove edp_tx compatible. > > - Rename dp_tx compatible to dp-tx. > > > > .../display/mediatek/mediatek,dp.yaml | 87 +++ > > 1 file changed, 87 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/mediatek/mediatek,dp.yaml > > > > Reviewed-by: Rob Herring > > But can't be applied without the dependent header (or remove the > dependency). Ok, thank you Rob. Maybe I will remove the dependency. Thanks, Markus
Re: [PATCH 7/9] drm/simpledrm: Enable FB_DAMAGE_CLIPS property
Den 22.10.2021 15.28, skrev Thomas Zimmermann: > Enable the FB_DAMAGE_CLIPS property to reduce display-update > overhead. Also fixes a warning in the kernel log. > > simple-framebuffer simple-framebuffer.0: [drm] > drm_plane_enable_fb_damage_clips() not called > > Fix the computation of the blit rectangle. This wasn't an issue so > far, as simpledrm always blitted the full framebuffer. The code now > supports damage clipping and virtual screen sizes. > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/tiny/simpledrm.c | 30 ++ > 1 file changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/tiny/simpledrm.c > b/drivers/gpu/drm/tiny/simpledrm.c > index 571f716ff427..e872121e9fb0 100644 > --- a/drivers/gpu/drm/tiny/simpledrm.c > +++ b/drivers/gpu/drm/tiny/simpledrm.c > @@ -642,7 +642,7 @@ simpledrm_simple_display_pipe_enable(struct > drm_simple_display_pipe *pipe, > void *vmap = shadow_plane_state->data[0].vaddr; /* TODO: Use mapping > abstraction */ > struct drm_device *dev = &sdev->dev; > void __iomem *dst = sdev->screen_base; > - struct drm_rect clip; > + struct drm_rect src_clip, dst_clip; > int idx; > > if (!fb) > @@ -651,10 +651,14 @@ simpledrm_simple_display_pipe_enable(struct > drm_simple_display_pipe *pipe, > if (!drm_dev_enter(dev, &idx)) > return; > > - drm_rect_init(&clip, 0, 0, fb->width, fb->height); > + drm_rect_fp_to_int(&src_clip, &plane_state->src); > > - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); > - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, > &clip); > + dst_clip = plane_state->dst; > + if (!drm_rect_intersect(&dst_clip, &src_clip)) > + return; > + > + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); > + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, > &src_clip); > > drm_dev_exit(idx); > } > @@ -686,20 +690,28 @@ simpledrm_simple_display_pipe_update(struct > drm_simple_display_pipe *pipe, > struct drm_framebuffer *fb = plane_state->fb; > struct drm_device *dev = &sdev->dev; > void __iomem *dst = sdev->screen_base; > - struct drm_rect clip; > + struct drm_rect damage_clip, src_clip, dst_clip; > int idx; > > if (!fb) > return; > > - if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, > &clip)) > + if (!drm_atomic_helper_damage_merged(old_plane_state, plane_state, > &damage_clip)) > + return; > + I'm afraid I don't understand what's going on here, but isn't it possible to put this logic into drm_atomic_helper_damage_merged()? Noralf. > + drm_rect_fp_to_int(&src_clip, &plane_state->src); > + if (!drm_rect_intersect(&src_clip, &damage_clip)) > + return; > + > + dst_clip = plane_state->dst; > + if (!drm_rect_intersect(&dst_clip, &src_clip)) > return; > > if (!drm_dev_enter(dev, &idx)) > return; > > - dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &clip); > - drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, > &clip); > + dst += drm_fb_clip_offset(sdev->pitch, sdev->format, &dst_clip); > + drm_fb_blit_toio(dst, sdev->pitch, sdev->format->format, vmap, fb, > &src_clip); > > drm_dev_exit(idx); > } > @@ -794,6 +806,8 @@ static int simpledrm_device_init_modeset(struct > simpledrm_device *sdev) > if (ret) > return ret; > > + drm_plane_enable_fb_damage_clips(&pipe->plane); > + > drm_mode_config_reset(dev); > > return 0; >
Re: [PATCH 6/9] drm/fb-helper: Allocate shadow buffer of surface height
Den 22.10.2021 15.28, skrev Thomas Zimmermann: > Allocating a shadow buffer of the height of the buffer object does > not support fbdev overallocation. Use surface height instead. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Noralf Trønnes
Re: [PATCH 5/9] drm/format-helper: Streamline blit-helper interface
Den 22.10.2021 15.28, skrev Thomas Zimmermann: > Move destination-buffer clipping from format-helper blit function into > caller. Rename drm_fb_blit_rect_dstclip() to drm_fb_blit_toio(). Done for > consistency with the rest of the interface. Remove drm_fb_blit_dstclip(), > which isn't required. > > Signed-off-by: Thomas Zimmermann > --- Reviewed-by: Noralf Trønnes
amdgpu "Fatal error during GPU init"; Ryzen 5600G integrated GPU + kernel 5.14.13
i've a dual gpu system inxi -GS System:Host: ws05 Kernel: 5.14.13-200.fc34.x86_64 x86_64 bits: 64 Console: tty pts/0 Distro: Fedora release 34 (Thirty Four) (1) Graphics: Device-1: NVIDIA GK208B [GeForce GT 710] driver: nvidia v: 470.74 (2)Device-2: Advanced Micro Devices [AMD/ATI] Cezanne driver: N/A Display: server: X.org 1.20.11 driver: loaded: nvidia unloaded: fbdev,modesetting,vesa Message: Advanced graphics data unavailable for root. running on cpu:Ryzen 5 5600G mobo: ASRockRack X470D4U bios: vP4.20, 04/14/2021 kernel: 5.14.13-200.fc34.x86_64 x86_64 where, the nvidia is a PCIe card the amdgpu is the Ryzen-integrated gpu the nvidia PCI is currently my primary it's screen-attached, and boots/functions correctly lsmod | grep nvidia nvidia_drm 69632 0 nvidia_modeset 1200128 1 nvidia_drm nvidia 35332096 1 nvidia_modeset drm_kms_helper303104 2 amdgpu,nvidia_drm drm 630784 8 gpu_sched,drm_kms_helper,nvidia,amdgpu,drm_ttm_helper,nvidia_drm,ttm dmesg | grep -i nvidia [5.755494] nvidia: loading out-of-tree module taints kernel. [5.755503] nvidia: module license 'NVIDIA' taints kernel. [5.759769] nvidia: module verification failed: signature and/or required key missing - tainting kernel [5.774894] nvidia-nvlink: Nvlink Core is being initialized, major device number 234 [5.775299] nvidia :10:00.0: vgaarb: changed VGA decodes: olddecodes=io+mem,decodes=none:owns=io+mem [5.975449] NVRM: loading NVIDIA UNIX x86_64 Kernel Module 470.74 Mon Sep 13 23:09:15 UTC 2021 [6.013181] nvidia-modeset: Loading NVIDIA Kernel Mode Setting Driver for UNIX platforms 470.74 Mon Sep 13 22:59:50 UTC 2021 [6.016444] [drm] [nvidia-drm] [GPU ID 0x1000] Loading driver [6.227295] caller _nv000723rm+0x1ad/0x200 [nvidia] mapping multiple BARs [6.954906] [drm] Initialized nvidia-drm 0.0.0 20160202 for :10:00.0 on minor 0 [ 16.820758] input: HDA NVidia HDMI/DP,pcm=3 as /devices/pci:00/:00:01.1/:10:00.1/sound/card0/input13 [ 16.820776] input: HDA NVidia HDMI/DP,pcm=7 as /devices/pci:00/:00:01.1/:10:00.1/sound/card0/input14 [ 16.820808] input: HDA NVidia HDMI/DP,pcm=8 as /devices/pci:00/:00:01.1/:10:00.1/sound/card0/input15 [ 16.820826] input: HDA NVidia HDMI/DP,pcm=9 as /devices/pci:00/:00:01.1/:10:00.1/sound/card0/input16 [ 16.820841] input: HDA NVidia HDMI/DP,pcm=10 as /devices/pci:00/:00:01.1/:10:00.1/sound/card0/input17 the amdgpu is not (currently/yet) in use; no attached screen in BIOS, currently, 'PCI Express' (nvidia gpu) is selected as primary 'HybridGraphics' is enabled 'OnBoard VGA' is enabled on boot, mods are loaded lsmod | grep gpu amdgpu 7802880 0 drm_ttm_helper 16384 1 amdgpu ttm81920 2 amdgpu,drm_ttm_helper iommu_v2 24576 1 amdgpu gpu_sched 45056 1 amdgpu drm_kms_helper303104 2 amdgpu,nvidia_drm drm 630784 8 gpu_sched,drm_kms_helper,nvidia,amdgpu,drm_ttm_helper,nvidia_drm,ttm i2c_algo_bit 16384 2 igb,amdgpu but i see a 'fatal error' and 'failed' probe, dmesg | grep -i amdgpu [5.161923] [drm] amdgpu kernel modesetting enabled. [5.162097] amdgpu: Virtual CRAT table created for CPU [5.162104] amdgpu: Topology: Add CPU node [5.162197] amdgpu :30:00.0: enabling device ( -> 0003) [5.162232] amdgpu :30:00.0: amdgpu: Trusted Memory Zone (TMZ) feature enabled [5.169105] amdgpu :30:00.0: BAR 6: can't assign [??? 0x flags 0x2000] (bogus alignment) [5.174413] amdgpu :30:00.0: amdgpu: Unable to locate a BIOS ROM [5.174415] amdgpu :30:00.0: amdgpu: Fatal error during GPU init [5.174416] amdgpu :30:00.0: amdgpu: amdgpu: finishing device. [5.174425] Modules linked in: amdgpu(+) uas usb_storage fjes(-) raid1 drm_ttm_helper ttm iommu_v2 gpu_sched drm_kms_helper crct10dif_pclmul crc32_pclmul igb crc32c_intel cec ghash_clmulni_intel drm sp5100_tco dca ccp i2c_algo_bit wmi video su
Re: [PATCH 4/9] drm/format-helper: Rework format-helper conversion functions
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Move destination-buffer clipping from all format-helper conversion
> functions into callers. Support destination-buffer pitch. Only
> distinguish between system and I/O memory, but use same logic
> everywhere.
>
> Simply harmonize the interface and semantics of the existing code.
> Not all conversion helpers support all combinations of parameters.
> We have to add additional features when we need them.
>
> Signed-off-by: Thomas Zimmermann
> ---
> /**
> * drm_fb_xrgb_to_gray8 - Convert XRGB to grayscale
> * @dst: 8-bit grayscale destination buffer
> + * @dst_pitch: Number of bytes between two consecutive scanlines within dst
> * @vaddr: XRGB source buffer
> * @fb: DRM framebuffer
> * @clip: Clip rectangle area to copy
> @@ -415,16 +417,21 @@ EXPORT_SYMBOL(drm_fb_xrgb_to_rgb888_dstclip);
> *
> * ITU BT.601 is used for the RGB -> luma (brightness) conversion.
> */
> -void drm_fb_xrgb_to_gray8(u8 *dst, void *vaddr, struct drm_framebuffer
> *fb,
> -struct drm_rect *clip)
> +void drm_fb_xrgb_to_gray8(void *dst, unsigned int dst_pitch, const void
> *vaddr,
> + const struct drm_framebuffer *fb, const struct
> drm_rect *clip)
> {
> unsigned int len = (clip->x2 - clip->x1) * sizeof(u32);
> unsigned int x, y;
> void *buf;
> - u32 *src;
> + u8 *dst8;
> + u32 *src32;
>
> if (WARN_ON(fb->format->format != DRM_FORMAT_XRGB))
> return;
> +
> + if (!dst_pitch)
len is source length (should really have been named src_len) which
results in a kernel crash:
> + dst_pitch = len;
This works:
dst_pitch = drm_rect_width(clip);
With that fixed:
Tested-by: Noralf Trønnes
Reviewed-by: Noralf Trønnes
> +
> /*
>* The cma memory is write-combined so reads are uncached.
>* Speed up by fetching one line at a time.
> @@ -433,20 +440,22 @@ void drm_fb_xrgb_to_gray8(u8 *dst, void *vaddr,
> struct drm_framebuffer *fb,
> if (!buf)
> return;
>
> + vaddr += clip_offset(clip, fb->pitches[0], sizeof(u32));
> for (y = clip->y1; y < clip->y2; y++) {
> - src = vaddr + (y * fb->pitches[0]);
> - src += clip->x1;
> - memcpy(buf, src, len);
> - src = buf;
> + dst8 = dst;
> + src32 = memcpy(buf, vaddr, len);
> for (x = clip->x1; x < clip->x2; x++) {
> - u8 r = (*src & 0x00ff) >> 16;
> - u8 g = (*src & 0xff00) >> 8;
> - u8 b = *src & 0x00ff;
> + u8 r = (*src32 & 0x00ff) >> 16;
> + u8 g = (*src32 & 0xff00) >> 8;
> + u8 b = *src32 & 0x00ff;
>
> /* ITU BT.601: Y = 0.299 R + 0.587 G + 0.114 B */
> - *dst++ = (3 * r + 6 * g + b) / 10;
> - src++;
> + *dst8++ = (3 * r + 6 * g + b) / 10;
> + src32++;
> }
> +
> + vaddr += fb->pitches[0];
> + dst += dst_pitch;
> }
>
> kfree(buf);
Re: [PATCH 3/9] drm/format-helper: Add destination-buffer pitch to drm_fb_swab()
Den 22.10.2021 15.28, skrev Thomas Zimmermann: > Add destination-buffer pitch as argument to drm_fb_swab(). Done for > consistency with the rest of the interface. > > Signed-off-by: Thomas Zimmermann > --- > drivers/gpu/drm/drm_format_helper.c | 19 +++ > drivers/gpu/drm/drm_mipi_dbi.c | 2 +- > drivers/gpu/drm/gud/gud_pipe.c | 2 +- > include/drm/drm_format_helper.h | 5 +++-- > 4 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/drm_format_helper.c > b/drivers/gpu/drm/drm_format_helper.c > index 38c8055f6fa8..79869ed553d9 100644 > --- a/drivers/gpu/drm/drm_format_helper.c > +++ b/drivers/gpu/drm/drm_format_helper.c > @@ -92,6 +92,7 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio); > /** > * drm_fb_swab - Swap bytes into clip buffer > * @dst: Destination buffer > + * @dst_pitch: Number of bytes between two consecutive scanlines within dst > * @src: Source buffer > * @fb: DRM framebuffer > * @clip: Clip rectangle area to copy > @@ -103,19 +104,25 @@ EXPORT_SYMBOL(drm_fb_memcpy_toio); > * This function does not apply clipping on dst, i.e. the destination You have changed this line on the other functions, maybe you just missed it here: > * is a small buffer containing the clip rect only. > */ > -void drm_fb_swab(void *dst, void *src, struct drm_framebuffer *fb, > - struct drm_rect *clip, bool cached) > +void drm_fb_swab(void *dst, unsigned int dst_pitch, const void *src, > + const struct drm_framebuffer *fb, const struct drm_rect *clip, > + bool cached) Tested-by: Noralf Trønnes Reviewed-by: Noralf Trønnes
Re: [PATCH 2/9] drm/format-helper: Rework format-helper memcpy functions
Den 22.10.2021 15.28, skrev Thomas Zimmermann: > Move destination-buffer clipping from all format-helper memcpy > function into callers. Support destination-buffer pitch. Only > distinguish between system and I/O memory, but use same logic > everywhere. > > Signed-off-by: Thomas Zimmermann > --- Tested-by: Noralf Trønnes Reviewed-by: Noralf Trønnes
Re: [PATCH 1/9] drm/format-helper: Export drm_fb_clip_offset()
Den 22.10.2021 15.28, skrev Thomas Zimmermann:
> Provide a function that computes the offset into a blit destination
> buffer. This will allow to move destination-buffer clipping into the
> format-helper callers.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/drm_format_helper.c | 10 --
> include/drm/drm_format_helper.h | 4
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_format_helper.c
> b/drivers/gpu/drm/drm_format_helper.c
> index 69fde60e36b3..28e9d0d89270 100644
> --- a/drivers/gpu/drm/drm_format_helper.c
> +++ b/drivers/gpu/drm/drm_format_helper.c
> @@ -17,12 +17,18 @@
> #include
> #include
>
> -static unsigned int clip_offset(struct drm_rect *clip,
> - unsigned int pitch, unsigned int cpp)
> +static unsigned int clip_offset(const struct drm_rect *clip, unsigned int
> pitch, unsigned int cpp)
> {
> return clip->y1 * pitch + clip->x1 * cpp;
> }
>
> +unsigned long drm_fb_clip_offset(unsigned int pitch, const struct
> drm_format_info *format,
Like Sam I wonder about the unsigned long here.
Noralf.
> + const struct drm_rect *clip)
> +{
> + return clip_offset(clip, pitch, format->cpp[0]);
> +}
> +EXPORT_SYMBOL(drm_fb_clip_offset);
> +
[Bug 214801] New: polaris11 (rx560) regression
https://bugzilla.kernel.org/show_bug.cgi?id=214801 Bug ID: 214801 Summary: polaris11 (rx560) regression Product: Drivers Version: 2.5 Kernel Version: 5.14.14 Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: roxm...@list.ru Regression: No Created attachment 299307 --> https://bugzilla.kernel.org/attachment.cgi?id=299307&action=edit dmesg After a long using 4.19.122, i am trying to move for a newer kernel (my xhci greatly improved since), currently it is 5.14.14 and there are noticeable slowdown. For a numbers the Unigine Heaven Benchmark 4.0 (linux OpenGL) was used. 4.19.122: FPS: 43.9 Score: 1105 Min FPS: 15.2 Max FPS: 90.7 5.14.14: FPS: 31.4 Score: 790 Min FPS: 22.4 Max FPS: 60.1 And there are flickering artifacts visible while benchmarking with newer kernel. -- You may reply to this email to add a comment. You are receiving this mail because: You are watching the assignee of the bug.
