Re: [PATCH v4] drm/omap: plane zpos/zorder management improvements
Hi Laurent, On 2018-01-05 16:04, Laurent Pinchart wrote: > Hi Peter, > > Thank you for the patch and sorry for the late review. > > On Friday, 5 January 2018 13:30:37 EET Peter Ujfalusi wrote: >> Use the plane index as default zpos for all planes. Even if the >> application is not setting zpos/zorder explicitly we will have unique zpos >> for each plane. >> >> Enforce that all planes must have unique zpos on the given crtc. > > Could you explain the rationale for that in the commit message, what's wrong > with duplicate zpos values ? Planes with identical zpos is only 'valid' _if_ they are not overlapping, if they do overlap then it is - imho - not a valid configuration anyway (which one should be on top?). Based on my tests the plane with lower planeID is going to disappear from the screen if we have duplicated zpos. > Isn't there a risk of breaking the non-atomic userspace with this ? Without > atomic commits userspace can't change the zpos of multiple planes in one go, > so it might be impossible to reorder planes without going through a state > that > has duplicated zpos values. Two planes occupying the same position on the screen is not valid (again, imho). If application wants to swap two planes, then it must disable one, move the other to the new position, then enable and move the first plane. > >> Signed-off-by: Peter Ujfalusi >> --- >> Hi, >> >> Changes since v3: >> - Use drm_plane_index() instead of storing the same index wothin omap_plane >> struct >> - Optimize the zpos validation loop so we avoid extra checks. >> >> Changes since v2: >> - The check for duplicate zpos is moved to omap_crtc >> >> Changes since v1: >> - Dropped the zpos normalization related patches >> - New patch based on the discussion, see commit message. >> >> Regards, >> Peter >> >> drivers/gpu/drm/omapdrm/omap_crtc.c | 36 - >> drivers/gpu/drm/omapdrm/omap_plane.c | 15 --- >> 2 files changed, 39 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c >> b/drivers/gpu/drm/omapdrm/omap_crtc.c index cc85c16cbc2a..c68e3a4f5b7d >> 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_crtc.c >> +++ b/drivers/gpu/drm/omapdrm/omap_crtc.c >> @@ -452,6 +452,40 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc >> *crtc) } >> } >> >> +static int omap_crtc_validate_zpos(struct drm_crtc *crtc, >> + struct drm_crtc_state *state) >> +{ >> +struct drm_device *ddev = crtc->dev; >> +struct drm_plane *p1; >> +const struct drm_plane_state *pstate1; >> + >> +/* Check the crts's planes against duplicated zpos value */ >> +drm_atomic_crtc_state_for_each_plane_state(p1, pstate1, state) { >> +struct drm_plane *p2 = p1; >> + >> +list_for_each_entry_continue(p2, &ddev->mode_config.plane_list, >> + head) { >> +const struct drm_plane_state *pstate2; >> + >> +if (!(state->plane_mask & (1 << drm_plane_index(p2 >> +continue; >> + >> +pstate2 = __drm_atomic_get_current_plane_state( >> +state->state, p2); >> +if (!pstate2) >> +continue; >> + >> +if (pstate1->zpos == pstate2->zpos) { >> +DBG("crtc%u: zpos must be unique (zpos: %u)", >> +crtc->index, pstate1->zpos); >> +return -EINVAL; >> +} >> +} >> +} > > That seems a bit complicated to me. Couldn't we use a single loop and a zpos > bitfield ? Something like the following (untested) code. > > struct drm_device *ddev = crtc->dev; > const struct drm_plane_state *plane_state; > struct drm_plane *plane; > unsigned int zpos_mask = 0; > > /* Check the crts's planes against duplicated zpos value */ > drm_atomic_crtc_state_for_each_plane_state(plane, plane_state, state) { > if (zpos_mask & BIT(plane_state->zpos)) { > DBG("crtc%u: zpos must be unique (zpos: %u)", > crtc->index, plane_state->zpos); > return -EINVAL; > } > > zpos_mask |= BIT(plane_state->zpos); > } > > return 0; Yes, it is much simpler. > >> +return 0; >> +} >> + >> static int omap_crtc_atomic_check(struct drm_crtc *crtc, >> struct drm_crtc_state *state) >> { >> @@ -475,7 +509,7 @@ static int omap_crtc_atomic_check(struct drm_crtc *crtc, >> omap_crtc_state->rotation = pri_state->rotation; >> } >> >> -return 0; >> +return omap_crtc_validate_zpos(crtc, state); >> } >> >> static void omap_crtc_atomic_begin(struct drm_crtc *crtc, >> diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c >> b/drivers/gpu/drm/omapdrm/oma
[Bug 198387] New: nouveau + GTX 960: reproducible system freeze
https://bugzilla.kernel.org/show_bug.cgi?id=198387 Bug ID: 198387 Summary: nouveau + GTX 960: reproducible system freeze Product: Drivers Version: 2.5 Kernel Version: 4.15.0-0.rc6.git2.1.fc28.x86_64 Hardware: Intel OS: Linux Tree: Mainline Status: NEW Severity: high Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: peter.eszl...@gmail.com Regression: No Using Fedora 28 Rawhide Live, I can freeze my system reproducible, doing the following: 1. start Firefox 2. open Google maps 3. keep resizing the window of Firefox till freeze $ journalctl -f Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: gr: TRAP ch 3 [00ff325000 Xwayland[1738]] Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: gr: GPC0/TPC0/TEX: 8041 Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: gr: GPC0/TPC1/TEX: 8041 Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: gr: GPC0/TPC2/TEX: 8041 Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: gr: GPC0/TPC3/TEX: 8041 Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: gr: GPC1/TPC0/TEX: 8041 Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: gr: GPC1/TPC1/TEX: 8041 Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: gr: GPC1/TPC2/TEX: 8041 Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: gr: GPC1/TPC3/TEX: 8041 Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: fifo: read fault at 0004baf000 engine 00 [GR] client 01 [GPC1/T1_0] reason 02 [PTE] on channel 3 [00ff325000 Xwayland[1738]] Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: fifo: channel 3: killed Jan 08 02:56:03 localhost-live kernel: nouveau :01:00.0: fifo: runlist 0: scheduled for recovery $ lspci -v 01:00.0 VGA compatible controller: NVIDIA Corporation GM206 [GeForce GTX 960] (rev a1) (prog-if 00 [VGA controller]) Subsystem: Micro-Star International Co., Ltd. [MSI] GM206 [GeForce GTX 960] Flags: bus master, fast devsel, latency 0, IRQ 31 Memory at f600 (32-bit, non-prefetchable) [size=16M] Memory at e000 (64-bit, prefetchable) [size=256M] Memory at f000 (64-bit, prefetchable) [size=32M] I/O ports at e000 [size=128] [virtual] Expansion ROM at 000c [disabled] [size=128K] $ cat /proc/cpuinfo model name : Intel(R) Core(TM) i7-4790K CPU @ 4.00GHz -- 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 0/1] i915: pm: Be less agressive with clockfreq changes on Bay Trail
Hi All, So this patch, which I already submitted a while ago and back then it got one comment from Chris Wilson: > Basically you want a limit on the frequency of ... pcode access? > As presented, you ultimately do not trust any write and the only > solution is to disable all writes. No RPS whatsoever, run at max and > hope rc6 works (maybe even decrease the rc6 threshold). > > One of the ideas we floated was moving the pcode access to a worker and > ratelimiting the updates. Has finally seen some testing by users affected by the infamous "intel_idle.max_cstate=1 required to prevent crashes" bug: https://bugzilla.kernel.org/show_bug.cgi?id=109051 And so far the reports are that it does help to make the users stable for 2/3 users and it not being effective for 1/3 users. Now that we've some test results, I believe that it is worthwhile to get this simple patch mainlined, hence this re-submission. Regards, Hans ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] vboxvideo: Fix incorrect type in assignment sparse warning
On Mon, Dec 25, 2017 at 04:42:59PM +0200, Alexander Kapshuk wrote: > Sparse emitted the following warning: > ../drivers/staging/vboxvideo/vbox_fb.c:173:27: warning: incorrect type in > assignment (different address spaces) > ../drivers/staging/vboxvideo/vbox_fb.c:173:27:expected char [noderef] > *screen_base > ../drivers/staging/vboxvideo/vbox_fb.c:173:27:got void *virtual > > The vbox_bo buffer object kernel mapping is handled by a call > to ttm_bo_kmap() prior to the assignment of bo->kmap.virtual to > info->screen_base of type char __iomem*. > Casting bo->kmap.virtual to char __iomem* in this assignment fixes > the warning. > > vboxvideo: Fix address space of expression removal sparse warning > > Sparse emitted the following warning: > ../drivers/staging/vboxvideo/vbox_main.c:64:25: warning: cast removes address > space of expression > > vbox->vbva_buffers iomapping is handled by calling vbox_accel_init() > from vbox_hw_init(). > __force attribute is used in assignment to vbva to fix the warning. > > Signed-off-by: Alexander Kapshuk > --- > drivers/staging/vboxvideo/vbox_fb.c | 2 +- > drivers/staging/vboxvideo/vbox_main.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/vboxvideo/vbox_fb.c > b/drivers/staging/vboxvideo/vbox_fb.c > index 8aed248db6e2..43c39eca4ae1 100644 > --- a/drivers/staging/vboxvideo/vbox_fb.c > +++ b/drivers/staging/vboxvideo/vbox_fb.c > @@ -170,7 +170,7 @@ static int vboxfb_create(struct drm_fb_helper *helper, > drm_fb_helper_fill_var(info, &fbdev->helper, sizes->fb_width, > sizes->fb_height); > > - info->screen_base = bo->kmap.virtual; > + info->screen_base = (char __iomem *)bo->kmap.virtual; > info->screen_size = size; > > #ifdef CONFIG_DRM_KMS_FB_HELPER > diff --git a/drivers/staging/vboxvideo/vbox_main.c > b/drivers/staging/vboxvideo/vbox_main.c > index 80bd039fa08e..973b3bcc04b1 100644 > --- a/drivers/staging/vboxvideo/vbox_main.c > +++ b/drivers/staging/vboxvideo/vbox_main.c > @@ -61,7 +61,7 @@ void vbox_enable_accel(struct vbox_private *vbox) > if (vbox->vbva_info[i].vbva) > continue; > > - vbva = (void *)vbox->vbva_buffers + i * VBVA_MIN_BUFFER_SIZE; > + vbva = (void __force *)vbox->vbva_buffers + i * > VBVA_MIN_BUFFER_SIZE; > if (!vbva_enable(&vbox->vbva_info[i], >vbox->guest_pool, vbva, i)) { > /* very old host or driver error. */ > -- > 2.13.6 > Ping. Could someone please comment on the patch above? Thanks. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Bug 198123] Console is the wrong color at boot with radeon 6670
December 22, 2017 4:35 PM, "Michel Dänzer" wrote: > "Deposite Pirate", do the attached (only compile tested) patches work? Hi, Sorry for the delay. I was not at home for a while. So, I've compiled and booted a kernel with both of these patches and the issue is still present. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm_hwcomposer] [PATCH] Update external connectors list
DVID, DVII and VGA are required by discrete and integrated GPUs --- drmconnector.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drmconnector.cpp b/drmconnector.cpp index 247f56b..145518f 100644 --- a/drmconnector.cpp +++ b/drmconnector.cpp @@ -73,7 +73,9 @@ bool DrmConnector::internal() const { } bool DrmConnector::external() const { - return type_ == DRM_MODE_CONNECTOR_HDMIA; + return type_ == DRM_MODE_CONNECTOR_HDMIA || type_ == DRM_MODE_CONNECTOR_DisplayPort || + type_ == DRM_MODE_CONNECTOR_DVID || type_ == DRM_MODE_CONNECTOR_DVII || + type_ == DRM_MODE_CONNECTOR_VGA; } bool DrmConnector::valid_type() const { -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/3] drm/sun4i: hdmi: Check for unset best_parent in sun4i_tmds_determine_rate
Hi Maxime, On 5 January 2018 at 21:03, Maxime Ripard wrote: > On Fri, Jan 05, 2018 at 09:44:39AM +1100, Jonathan Liu wrote: >> On 5 January 2018 at 06:56, Maxime Ripard >> wrote: >> > On Tue, Dec 26, 2017 at 10:12:25PM +1100, Jonathan Liu wrote: >> >> We should check if the best match has been set before comparing it. >> >> >> >> Fixes: 9c5681011a0c ("drm/sun4i: Add HDMI support") >> >> Signed-off-by: Jonathan Liu >> >> --- >> >> drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c | 2 +- >> >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c >> >> b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c >> >> index dc332ea56f6c..4d235e5ea31c 100644 >> >> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c >> >> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_tmds_clk.c >> >> @@ -102,7 +102,7 @@ static int sun4i_tmds_determine_rate(struct clk_hw >> >> *hw, >> >> goto out; >> >> } >> >> >> >> - if (abs(rate - rounded / i) < >> >> + if (!best_parent || abs(rate - rounded / i) >> >> < >> > >> > Why is that causing any issue? >> > >> > If best_parent is set to 0... >> > >> >> abs(rate - best_parent / best_div)) { >> > >> > ... the value returned here is going to be rate, which is going to be >> > higher than the first part of the comparison meaning ... >> > >> >> best_parent = rounded; >> > >> > ... that best_parent is going to be set there. >> >> Consider the following: >> rate = 8350 >> rounded = ideal * 2 >> >> It is possible that if "rounded = clk_hw_round_rate(parent, ideal)" >> gives high enough values that the condition "abs(rate - rounded / i) < >> abs(rate - best_parent / best_div)" is never met. >> >> Then you can end up with: >> req->rate = 0 >> req->best_parent_rate = 0 >> req->best_parent_hw = ... >> >> Also, the sun4i_tmds_calc_divider function has a similar check. > > Ok. That explanation must be part of your commit log, or at least > which problem you're trying to address and in which situation it will > trigger. Ok. I have added amended the commit message with explanation for v3. Regards, Jonathan ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vboxvideo: Fix incorrect type in assignment sparse warning
On Sat, Jan 6, 2018 at 9:43 PM, Hans de Goede wrote: > HI, > > > On 06-01-18 20:30, Alexander Kapshuk wrote: >> >> On Sat, Jan 6, 2018 at 6:00 PM, Hans de Goede wrote: >>> >>> Hi, >>> >>> >>> On 06-01-18 15:20, Alexander Kapshuk wrote: On Mon, Dec 25, 2017 at 04:42:59PM +0200, Alexander Kapshuk wrote: > > > Sparse emitted the following warning: > ../drivers/staging/vboxvideo/vbox_fb.c:173:27: warning: incorrect type > in > assignment (different address spaces) > ../drivers/staging/vboxvideo/vbox_fb.c:173:27:expected char > [noderef] > *screen_base > ../drivers/staging/vboxvideo/vbox_fb.c:173:27:got void *virtual > > The vbox_bo buffer object kernel mapping is handled by a call > to ttm_bo_kmap() prior to the assignment of bo->kmap.virtual to > info->screen_base of type char __iomem*. > Casting bo->kmap.virtual to char __iomem* in this assignment fixes > the warning. > > vboxvideo: Fix address space of expression removal sparse warning > > Sparse emitted the following warning: > ../drivers/staging/vboxvideo/vbox_main.c:64:25: warning: cast removes > address space of expression > > vbox->vbva_buffers iomapping is handled by calling vbox_accel_init() > from vbox_hw_init(). > __force attribute is used in assignment to vbva to fix the warning. > > Signed-off-by: Alexander Kapshuk > --- >drivers/staging/vboxvideo/vbox_fb.c | 2 +- >drivers/staging/vboxvideo/vbox_main.c | 2 +- >2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/staging/vboxvideo/vbox_fb.c > b/drivers/staging/vboxvideo/vbox_fb.c > index 8aed248db6e2..43c39eca4ae1 100644 > --- a/drivers/staging/vboxvideo/vbox_fb.c > +++ b/drivers/staging/vboxvideo/vbox_fb.c > @@ -170,7 +170,7 @@ static int vboxfb_create(struct drm_fb_helper > *helper, > drm_fb_helper_fill_var(info, &fbdev->helper, sizes->fb_width, > sizes->fb_height); > > - info->screen_base = bo->kmap.virtual; > + info->screen_base = (char __iomem *)bo->kmap.virtual; > info->screen_size = size; > >#ifdef CONFIG_DRM_KMS_FB_HELPER >>> >>> >>> >>> This fix looks good to me. >>> > diff --git a/drivers/staging/vboxvideo/vbox_main.c > b/drivers/staging/vboxvideo/vbox_main.c > index 80bd039fa08e..973b3bcc04b1 100644 > --- a/drivers/staging/vboxvideo/vbox_main.c > +++ b/drivers/staging/vboxvideo/vbox_main.c > @@ -61,7 +61,7 @@ void vbox_enable_accel(struct vbox_private *vbox) > if (vbox->vbva_info[i].vbva) > continue; > > - vbva = (void *)vbox->vbva_buffers + i * > VBVA_MIN_BUFFER_SIZE; > + vbva = (void __force *)vbox->vbva_buffers + i * > VBVA_MIN_BUFFER_SIZE; > if (!vbva_enable(&vbox->vbva_info[i], > vbox->guest_pool, vbva, i)) { > /* very old host or driver error. */ >>> >>> >>> >>> Hmm, isn't there a cleaner way to fix this ? Maybe make vbva_enable's >>> argument (and the local vbva variable) of type u8 __iomem * too ? >>> >>> Regards, >>> >>> Hans >> >> >> Hi Hans, >> >> Thanks for your prompt response. >> >> I had a good look at the vbva_enable() function's definition and to >> the best of my knowledge, changing vbva's type from 'struct >> vbva_buffer *' to 'u8 __iomem *' wouldn't work. >> vbva_enable() relies on vbva's type being a pointer to 'struct >> vbva_buffer': >> vbva's memory gets set to zero; >> some of vbva's members are initialised to particular values; >> vbva_ctx->vbva expects vbva to be a pointer to 'struct vbva_buffer' as >> well; >> >> Or am I misreading this? > > > No your not misreading this I did not check the function myself before > commenting myself, my bad. Thanks for confirming my understanding of this. > >> What are your thoughts on this? > > > Lets just move ahead with your original fix: Did you want me to resend the patch, or is someone going to pull the original one I sent into their tree? Thanks. > > Reviewed-by: Hans de Goede > > Regards, > > Hans > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] vboxvideo: Fix incorrect type in assignment sparse warning
On Sat, Jan 6, 2018 at 6:00 PM, Hans de Goede wrote: > Hi, > > > On 06-01-18 15:20, Alexander Kapshuk wrote: >> >> On Mon, Dec 25, 2017 at 04:42:59PM +0200, Alexander Kapshuk wrote: >>> >>> Sparse emitted the following warning: >>> ../drivers/staging/vboxvideo/vbox_fb.c:173:27: warning: incorrect type in >>> assignment (different address spaces) >>> ../drivers/staging/vboxvideo/vbox_fb.c:173:27:expected char [noderef] >>> *screen_base >>> ../drivers/staging/vboxvideo/vbox_fb.c:173:27:got void *virtual >>> >>> The vbox_bo buffer object kernel mapping is handled by a call >>> to ttm_bo_kmap() prior to the assignment of bo->kmap.virtual to >>> info->screen_base of type char __iomem*. >>> Casting bo->kmap.virtual to char __iomem* in this assignment fixes >>> the warning. >>> >>> vboxvideo: Fix address space of expression removal sparse warning >>> >>> Sparse emitted the following warning: >>> ../drivers/staging/vboxvideo/vbox_main.c:64:25: warning: cast removes >>> address space of expression >>> >>> vbox->vbva_buffers iomapping is handled by calling vbox_accel_init() >>> from vbox_hw_init(). >>> __force attribute is used in assignment to vbva to fix the warning. >>> >>> Signed-off-by: Alexander Kapshuk >>> --- >>> drivers/staging/vboxvideo/vbox_fb.c | 2 +- >>> drivers/staging/vboxvideo/vbox_main.c | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/staging/vboxvideo/vbox_fb.c >>> b/drivers/staging/vboxvideo/vbox_fb.c >>> index 8aed248db6e2..43c39eca4ae1 100644 >>> --- a/drivers/staging/vboxvideo/vbox_fb.c >>> +++ b/drivers/staging/vboxvideo/vbox_fb.c >>> @@ -170,7 +170,7 @@ static int vboxfb_create(struct drm_fb_helper >>> *helper, >>> drm_fb_helper_fill_var(info, &fbdev->helper, sizes->fb_width, >>>sizes->fb_height); >>> >>> - info->screen_base = bo->kmap.virtual; >>> + info->screen_base = (char __iomem *)bo->kmap.virtual; >>> info->screen_size = size; >>> >>> #ifdef CONFIG_DRM_KMS_FB_HELPER > > > This fix looks good to me. > >>> diff --git a/drivers/staging/vboxvideo/vbox_main.c >>> b/drivers/staging/vboxvideo/vbox_main.c >>> index 80bd039fa08e..973b3bcc04b1 100644 >>> --- a/drivers/staging/vboxvideo/vbox_main.c >>> +++ b/drivers/staging/vboxvideo/vbox_main.c >>> @@ -61,7 +61,7 @@ void vbox_enable_accel(struct vbox_private *vbox) >>> if (vbox->vbva_info[i].vbva) >>> continue; >>> >>> - vbva = (void *)vbox->vbva_buffers + i * >>> VBVA_MIN_BUFFER_SIZE; >>> + vbva = (void __force *)vbox->vbva_buffers + i * >>> VBVA_MIN_BUFFER_SIZE; >>> if (!vbva_enable(&vbox->vbva_info[i], >>> vbox->guest_pool, vbva, i)) { >>> /* very old host or driver error. */ > > > Hmm, isn't there a cleaner way to fix this ? Maybe make vbva_enable's > argument (and the local vbva variable) of type u8 __iomem * too ? > > Regards, > > Hans Hi Hans, Thanks for your prompt response. I had a good look at the vbva_enable() function's definition and to the best of my knowledge, changing vbva's type from 'struct vbva_buffer *' to 'u8 __iomem *' wouldn't work. vbva_enable() relies on vbva's type being a pointer to 'struct vbva_buffer': vbva's memory gets set to zero; some of vbva's members are initialised to particular values; vbva_ctx->vbva expects vbva to be a pointer to 'struct vbva_buffer' as well; Or am I misreading this? What are your thoughts on this? Thanks. Alexander. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[drm_hwcomposer] [PATCH] Take Connection state into account. (v2)
Porting of original commit 76fb87e675 of Jim Bish in android-ia master to fdo Original commit message: "There are various places where we should be really taking connection state into account before querying the properties or assuming it as primary. This patch fixes them." (v2) checks on connection state are applied for both internal and external connectors, in order to select the correct primary, as opposed to setting, independently from its state, the first connector as primary This is essential to avoid following logcat errors on integrated and dedicated GPUs: ... 2245 2245 E hwc-drm-resources: Could not find a suitable encoder/crtc for display 2 ... 2245 2245 E hwc-drm-resources: Failed CreateDisplayPipe 56 with -19 ... 2245 2245 E hwcomposer-drm: Can't initialize Drm object -19 Tested with i965 on Sandybridge and nouveau on GT120, GT610 --- drmresources.cpp | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drmresources.cpp b/drmresources.cpp index 32dd376..d582cfe 100644 --- a/drmresources.cpp +++ b/drmresources.cpp @@ -159,7 +159,7 @@ int DrmResources::Init() { // First look for primary amongst internal connectors for (auto &conn : connectors_) { -if (conn->internal() && !found_primary) { +if (conn->state() == DRM_MODE_CONNECTED && conn->internal() && !found_primary) { conn->set_display(0); found_primary = true; } else { @@ -170,7 +170,7 @@ int DrmResources::Init() { // Then look for primary amongst external connectors for (auto &conn : connectors_) { -if (conn->external() && !found_primary) { +if (conn->state() == DRM_MODE_CONNECTED && conn->external() && !found_primary) { conn->set_display(0); found_primary = true; } @@ -288,6 +288,11 @@ int DrmResources::TryEncoderForDisplay(int display, DrmEncoder *enc) { int DrmResources::CreateDisplayPipe(DrmConnector *connector) { int display = connector->display(); + + // skip not connected + if (connector->state() == DRM_MODE_DISCONNECTED) +return 0; + /* Try to use current setup first */ if (connector->encoder()) { int ret = TryEncoderForDisplay(display, connector->encoder()); -- 2.14.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] i915: pm: Be less agressive with clockfreq changes on Bay Trail
Bay Trail devices are known to hang when changing the frequency often, this is discussed in great length in: https://bugzilla.kernel.org/show_bug.cgi?id=109051 Commit 6067a27d1f01 ("drm/i915: Avoid tweaking evaluation thresholds on Baytrail v3") is an attempt to workaround this. Several users in bko109051 report that an earlier version of this patch, v1: https://bugzilla.kernel.org/attachment.cgi?id=251471 Works better for them and they still see hangs with the merged v3. Comparing the 2 versions shows that they are indeed not equivalent, v1 not only skips writing the GEN6_RP* registers from valleyview_set_rps, as v3 does. It also contained these modifications to i915_irq.c: if (pm_iir & GEN6_PM_RP_DOWN_EI_EXPIRED) { if (!vlv_c0_above(dev_priv, &dev_priv->rps.down_ei, &now, - dev_priv->rps.down_threshold)) + VLV_RP_DOWN_EI_THRESHOLD)) events |= GEN6_PM_RP_DOWN_THRESHOLD; dev_priv->rps.down_ei = now; } if (pm_iir & GEN6_PM_RP_UP_EI_EXPIRED) { if (vlv_c0_above(dev_priv, &dev_priv->rps.up_ei, &now, - dev_priv->rps.up_threshold)) + VLV_RP_UP_EI_THRESHOLD)) events |= GEN6_PM_RP_UP_THRESHOLD; dev_priv->rps.up_ei = now; } Which use less aggressive up/down thresholds, which results in less GEN6_PM_RP_*_THRESHOLD events and thus in less calls to intel_set_rps() -> valleyview_set_rps() -> vlv_punit_write(PUNIT_REG_GPU_FREQ_REQ). With the last call being the likely cause of the hang. This commit hardcodes the threshold_up and _down values for Bay Trail to less aggressive values, reducing the amount of clock frequency changes, thus avoiding the hangs some people are still seeing with the merged fix. Buglink: https://bugzilla.kernel.org/show_bug.cgi?id=109051 Signed-off-by: Hans de Goede --- drivers/gpu/drm/i915/i915_reg.h | 3 +++ drivers/gpu/drm/i915/intel_pm.c | 5 - 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 505c605eff98..acafc8408e43 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1390,6 +1390,9 @@ enum i915_power_well_id { #defineVLV_BIAS_CPU_125_SOC_875 (6 << 2) #defineCHV_BIAS_CPU_50_SOC_50 (3 << 2) +#define VLV_RP_UP_EI_THRESHOLD 90 +#define VLV_RP_DOWN_EI_THRESHOLD 70 + /* vlv2 north clock has */ #define CCK_FUSE_REG 0x8 #define CCK_FUSE_HPLL_FREQ_MASK 0x3 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 1db79a860b96..b06379622301 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6116,8 +6116,11 @@ static void gen6_set_rps_thresholds(struct drm_i915_private *dev_priv, u8 val) /* When byt can survive without system hang with dynamic * sw freq adjustments, this restriction can be lifted. */ - if (IS_VALLEYVIEW(dev_priv)) + if (IS_VALLEYVIEW(dev_priv)) { + threshold_up = VLV_RP_UP_EI_THRESHOLD; + threshold_down = VLV_RP_DOWN_EI_THRESHOLD; goto skip_hw_write; + } I915_WRITE(GEN6_RP_UP_EI, GT_INTERVAL_FROM_US(dev_priv, ei_up)); -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] vboxvideo: Fix incorrect type in assignment sparse warning
On Sat, Jan 6, 2018 at 10:00 PM, Hans de Goede wrote: > Hi, > > > On 06-01-18 20:56, Alexander Kapshuk wrote: >> >> On Sat, Jan 6, 2018 at 9:43 PM, Hans de Goede wrote: >>> >>> HI, >>> >>> >>> On 06-01-18 20:30, Alexander Kapshuk wrote: On Sat, Jan 6, 2018 at 6:00 PM, Hans de Goede wrote: > > > Hi, > > > On 06-01-18 15:20, Alexander Kapshuk wrote: >> >> >> >> On Mon, Dec 25, 2017 at 04:42:59PM +0200, Alexander Kapshuk wrote: >>> >>> >>> >>> Sparse emitted the following warning: >>> ../drivers/staging/vboxvideo/vbox_fb.c:173:27: warning: incorrect >>> type >>> in >>> assignment (different address spaces) >>> ../drivers/staging/vboxvideo/vbox_fb.c:173:27:expected char >>> [noderef] >>> *screen_base >>> ../drivers/staging/vboxvideo/vbox_fb.c:173:27:got void *virtual >>> >>> The vbox_bo buffer object kernel mapping is handled by a call >>> to ttm_bo_kmap() prior to the assignment of bo->kmap.virtual to >>> info->screen_base of type char __iomem*. >>> Casting bo->kmap.virtual to char __iomem* in this assignment fixes >>> the warning. >>> >>> vboxvideo: Fix address space of expression removal sparse warning >>> >>> Sparse emitted the following warning: >>> ../drivers/staging/vboxvideo/vbox_main.c:64:25: warning: cast removes >>> address space of expression >>> >>> vbox->vbva_buffers iomapping is handled by calling vbox_accel_init() >>> from vbox_hw_init(). >>> __force attribute is used in assignment to vbva to fix the warning. >>> >>> Signed-off-by: Alexander Kapshuk >>> --- >>> drivers/staging/vboxvideo/vbox_fb.c | 2 +- >>> drivers/staging/vboxvideo/vbox_main.c | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/staging/vboxvideo/vbox_fb.c >>> b/drivers/staging/vboxvideo/vbox_fb.c >>> index 8aed248db6e2..43c39eca4ae1 100644 >>> --- a/drivers/staging/vboxvideo/vbox_fb.c >>> +++ b/drivers/staging/vboxvideo/vbox_fb.c >>> @@ -170,7 +170,7 @@ static int vboxfb_create(struct drm_fb_helper >>> *helper, >>> drm_fb_helper_fill_var(info, &fbdev->helper, >>> sizes->fb_width, >>> sizes->fb_height); >>> >>> - info->screen_base = bo->kmap.virtual; >>> + info->screen_base = (char __iomem *)bo->kmap.virtual; >>> info->screen_size = size; >>> >>> #ifdef CONFIG_DRM_KMS_FB_HELPER > > > > > This fix looks good to me. > >>> diff --git a/drivers/staging/vboxvideo/vbox_main.c >>> b/drivers/staging/vboxvideo/vbox_main.c >>> index 80bd039fa08e..973b3bcc04b1 100644 >>> --- a/drivers/staging/vboxvideo/vbox_main.c >>> +++ b/drivers/staging/vboxvideo/vbox_main.c >>> @@ -61,7 +61,7 @@ void vbox_enable_accel(struct vbox_private *vbox) >>> if (vbox->vbva_info[i].vbva) >>> continue; >>> >>> - vbva = (void *)vbox->vbva_buffers + i * >>> VBVA_MIN_BUFFER_SIZE; >>> + vbva = (void __force *)vbox->vbva_buffers + i * >>> VBVA_MIN_BUFFER_SIZE; >>> if (!vbva_enable(&vbox->vbva_info[i], >>>vbox->guest_pool, vbva, i)) { >>> /* very old host or driver error. */ > > > > > Hmm, isn't there a cleaner way to fix this ? Maybe make vbva_enable's > argument (and the local vbva variable) of type u8 __iomem * too ? > > Regards, > > Hans Hi Hans, Thanks for your prompt response. I had a good look at the vbva_enable() function's definition and to the best of my knowledge, changing vbva's type from 'struct vbva_buffer *' to 'u8 __iomem *' wouldn't work. vbva_enable() relies on vbva's type being a pointer to 'struct vbva_buffer': vbva's memory gets set to zero; some of vbva's members are initialised to particular values; vbva_ctx->vbva expects vbva to be a pointer to 'struct vbva_buffer' as well; Or am I misreading this? >>> >>> >>> >>> No your not misreading this I did not check the function myself before >>> commenting myself, my bad. >> >> >> Thanks for confirming my understanding of this. >> >>> What are your thoughts on this? >>> >>> >>> >>> Lets just move ahead with your original fix: >> >> >> Did you want me to resend the patch, or is someone going to pull the >> original one I sent into their tree? >> Thanks. > > > I expect Greg to pick it up without a resend. But with all the Spectre > stuff going on right now he might miss it, so I would say give it 14 days > and if he has not picked it up by then, resend it with my reviewed-by > added. > > Regards, > > Hans Understood. Thanks. ___
[PATCH] gpu: drm: i915: intel_hotplug: avoid NULL pointer dereference
I observed the following crash on my laptop after undocking it: BUG: unable to handle kernel NULL pointer dereference at 00e4 IP: i915_hpd_poll_init_work+0x8f/0x100 [i915] PGD 0 P4D 0 Oops: [#1] PREEMPT SMP Modules linked in: ppp_mppe ppp_async ppp_generic slhc nf_conntrack_pptp nf_conntrack_proto_gre veth ipt_MASQUERADE nf_nat_masquerade_ipv4 nf_conntrack_netlink nfnetlink xfrm_user kvm irqbypass crct10dif_pclmul iwlmvm crc32_pclmul ghash_clmulni_intel mei_wdt iTCO_wdt iTCO_vendor_support mac80211 wmi_bmof pcbc i2c_algo_bit snd_hda_intel drm_kms_helper snd_hd usbcore usb_common i8042 serio vfat fat trusted tpm crc32c_generic crc32c_intel btrfs xor zstd_decompress zstd_compress xxhash raid6_pq CPU: 0 PID: 37 Comm: kworker/0:1 Tainted: G U O4.14.9-1-ARCH #1 Hardware name: LENOVO 20F9CTO1WW/20F9CTO1WW, BIOS N1CET56W (1.24 ) 04/19/2017 Workqueue: events i915_hpd_poll_init_work [i915] task: a0bd09132dc0 task.stack: b177032b RIP: 0010:i915_hpd_poll_init_work+0x8f/0x100 [i915] RSP: 0018:b177032b3e58 EFLAGS: 00010202 RAX: a0bcf5b2d800 RBX: 0001 RCX: 0056 RDX: RSI: 0001 RDI: c0a9d8f7 RBP: a0bcf8f1abc0 R08: 0003 R09: 0002 R10: a0bcf8f182f8 R11: 0c00 R12: a0bcf8f18000 R13: R14: a0bcf8f181f8 R15: a0bd092843c0 FS: () GS:a0bd2140() knlGS: CS: 0010 DS: ES: CR0: 80050033 CR2: 00e4 CR3: 000157c09002 CR4: 003606f0 Call Trace: process_one_work+0x1db/0x410 worker_thread+0x2b/0x3d0 ? process_one_work+0x410/0x410 kthread+0x118/0x130 ? kthread_create_on_node+0x70/0x70 ret_from_fork+0x1f/0x30 Code: 0f b6 90 a0 04 00 00 48 83 b8 b0 04 00 00 00 88 90 e8 02 00 00 75 dc 84 d2 75 d8 f6 85 70 da ff ff 02 74 cf 48 8b 90 00 04 00 00 <8b> 92 e4 00 00 00 85 d2 74 be 88 98 e8 02 0 RIP: i915_hpd_poll_init_work+0x8f/0x100 [i915] RSP: b177032b3e58 CR2: 00e4 ---[ end trace 098075e41d0a597e ]--- The values of the registers seems to indicate that it tries to dereference the hpd_pin field of the encoder structure. The value 0xe4 corresponds to the offset of the field hpd_pin in the encoder structure. It appears that encoder->hpd_pin is accessed without checking first that encoder is not NULL. The same issue may happen in intel_hpd_irq_storm_reenable_work(). This patch should fix these issues. Signed-off-by: Olivier Sobrie --- drivers/gpu/drm/i915/intel_hotplug.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c index 875d5d218d5c..ab7e8dd0cf5b 100644 --- a/drivers/gpu/drm/i915/intel_hotplug.c +++ b/drivers/gpu/drm/i915/intel_hotplug.c @@ -245,7 +245,8 @@ static void intel_hpd_irq_storm_reenable_work(struct work_struct *work) drm_for_each_connector_iter(connector, &conn_iter) { struct intel_connector *intel_connector = to_intel_connector(connector); - if (intel_connector->encoder->hpd_pin == i) { + if (intel_connector->encoder && + (intel_connector->encoder->hpd_pin == i)) { if (connector->polled != intel_connector->polled) DRM_DEBUG_DRIVER("Reenabling HPD on connector %s\n", connector->name); @@ -546,6 +547,7 @@ static void i915_hpd_poll_init_work(struct work_struct *work) continue; if (!connector->polled && I915_HAS_HOTPLUG(dev_priv) && + intel_connector->encoder && intel_connector->encoder->hpd_pin > HPD_NONE) { connector->polled = enabled ? DRM_CONNECTOR_POLL_CONNECT | -- 2.15.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
On 05.01.2018 17:17, Thierry Reding wrote: > Hi Dave, > > The following changes since commit 9428088c90b6f7d5edd2a1b0d742c75339b36f6e: > > drm/qxl: reapply cursor after resetting primary (2017-12-08 13:37:02 +1000) > > are available in the Git repository at: > > git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-4.16-rc1 > > for you to fetch changes up to ebae8d07435ae91314f4a28d69b530d09c625815: > > drm/tegra: dc: Implement legacy blending (2017-12-21 14:55:55 +0100) > > Thanks, > Thierry > > > drm/tegra: Changes for v4.16-rc1 > > The bulk of these changes are preparation work and addition of support > for Tegra186. Currently only HDMI output (the primary output on Jetson > TX2) is supported, but the hardware is also capable of doing DSI and > DisplayPort. > > Tegra DRM now also uses the atomic commit helpers instead of the open- > coded variant that was only doing half its job. As a bit of a byproduct > of the Tegra186 support the driver also gained HDMI 2.0 as well as zpos > property support. > > Along the way there are also a few patches to clean up a few things and > fix minor issues. > > > Arnd Bergmann (2): > drm/tegra: Mark Tegra186 display hub PM functions __maybe_unused > drm/tegra: Fix non-debugfs builds > > Dmitry Osipenko (3): > drm/tegra: dc: Link DC1 to DC0 on Tegra20 > drm/tegra: gem: Correct iommu_map_sg() error checking > drm/tegra: Correct timeout in tegra_syncpt_wait > > Thierry Reding (43): > drm/fourcc: Fix fourcc_mod_code() definition > drm/tegra: Sanitize format modifiers > gpu: host1x: Rewrite conditional for better readability > gpu: host1x: Cleanup on initialization failure > dt-bindings: display: tegra: Update SOR for Tegra186 > drm/tegra: dc: Move register definitions into a table > drm/tegra: dsi: Move register definitions into a table > drm/tegra: hdmi: Move register definitions into a table > drm/tegra: sor: Move register definitions into a table > drm/tegra: dc: Reshuffle some code > drm/tegra: dc: Register debugfs in ->late_register() > drm/tegra: dsi: Register debugfs in ->late_register() > drm/tegra: hdmi: Register debugfs in ->late_register() > drm/tegra: sor: Root debugfs files at the connector > drm/tegra: sor: Register debugfs in ->late_register() > drm/tegra: Do not wrap lines unnecessarily > drm/tegra: vic: Properly align arguments > drm/tegra: dc: Support background color > drm/tegra: Use atomic commit helpers > drm/tegra: Remove custom page-flip handler > drm/tegra: dc: Remove tegra_primary_plane_destroy() > drm/tegra: dc: Remove duplicate plane funcs > drm/tegra: dc: Remove tegra_overlay_plane_destroy() > drm/tegra: dc: Remove duplicate plane funcs > drm/tegra: dc: Move state definition to header > drm/tegra: Move common plane code to separate file > drm/tegra: Add Tegra186 display hub support > drm/tegra: dc: Add Tegra186 support > drm/tegra: Support ARGB and ABGR formats > drm/tegra: sor: Parameterize register offsets > drm/tegra: sor: Add Tegra186 support > drm/tegra: sor: Support HDMI 2.0 modes > drm/tegra: dpaux: Implement runtime PM > drm/tegra: dpaux: Add Tegra186 support > drm/tegra: fb: Force alpha formats > drm/tegra: dc: Support more formats > drm/tegra: dc: Use direct offset to plane registers > drm/tegra: dc: Remove redundant spinlock > drm/tegra: Implement zpos property > gpu: host1x: Use IOMMU groups > drm/tegra: Use IOMMU groups > drm/tegra: dpaux: Keep reset defaults for hybrid pad parameters > drm/tegra: dc: Implement legacy blending Please hold on this patch. First of all it doesn't work correctly, see my last reply to the patch. Secondly, it introduces bug that breaks YUV plane. [snip] ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/3] drm/omap: Make omapdss API more generic
On 07/01/18 22:14, Jyri Sarha wrote: >>> +static u64 dispc_api_read_irqstatus(u64 clearmask) >>> +{ >>> + u32 hw_clearmask = dispc_api_to_hw_irq(clearmask); >>> + u32 hw_status = dispc_read_irqstatus(); >>> + >>> + dispc_clear_irqstatus(hw_clearmask & hw_status); >>> + >>> + return dispc_hw_to_api_irq(hw_status); >>> +} >> >> I think we always want to read the whole irqstatus, and clear it. You >> can do that with the function above, but I'm not sure if clearmask >> offers us anything we ever need to use. >> > > But the semantically correct way is to clear only the interrupts we > handle. In theory there could be some other entity following some other > interrupts and clearing the interrupts we are not handling could ruin If that would be the case, then we need to ensure that the irqenable and irqstatus are handled race free, and we need to make sure those entities never touch the same bits, even if they'd use proper locking. Yes, it's possible, but we don't do it and I hope we never do. > that. But in practice at the moment everything would work fine without > the clearmask too. Is that reason enough to remove it? Well, I would ask if there's enough reason to add it? What's the scenario you see that it would be used (not theoretical, but real one)? Or is there any other downside to just clearing all irqstatus bits? My main worry with this is that we somehow (well, bug) would end up having a dispc irq enabled in irqenable, and we would not have that bit in clearmask. The end result would be an endless irq flood. If in the future there's a use case that needs this, it would be trivial to add it at that point of time. So my guideline would be to keep things as simple as possible, and only add unused features if there's a realistic near-future use case for it which you know you'll be implementing. 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] gpu: ipu-v3: make array 'int_reg' static, shrinks object size
Hi Colin, On Sat, 2018-01-06 at 17:28 +, Colin King wrote: > From: Colin Ian King > > Don't populate the const read-only array 'int_reg' on the stack but instead > make it static. Makes the object code smaller by over 80 bytes: > > Before: >text data bss dec hex filename > 28024 8936 192 371529120 drivers/gpu/ipu-v3/ipu-common.o > > After: >text data bss dec hex filename > 27794 9080 192 3706690ca drivers/gpu/ipu-v3/ipu-common.o > > (gcc version 7.2.0 x86_64) > > Signed-off-by: Colin Ian King Thank you for the patch. > --- > drivers/gpu/ipu-v3/ipu-common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c > index 658fa2d3e40c..babf1aee7ca5 100644 > --- a/drivers/gpu/ipu-v3/ipu-common.c > +++ b/drivers/gpu/ipu-v3/ipu-common.c > @@ -1089,7 +1089,7 @@ static void ipu_irq_handler(struct irq_desc *desc) > { > struct ipu_soc *ipu = irq_desc_get_handler_data(desc); > struct irq_chip *chip = irq_desc_get_chip(desc); > - const int int_reg[] = { 0, 1, 2, 3, 10, 11, 12, 13, 14}; > + static const int int_reg[] = { 0, 1, 2, 3, 10, 11, 12, 13, 14}; > > chained_irq_enter(chip, desc); There's the same issue in ipu_irq_err_handler right below: --8<-- diff --git a/drivers/gpu/ipu-v3/ipu-common.c b/drivers/gpu/ipu-v3/ipu-common.c index babf1aee7ca58..48685cddbad1b 100644 --- a/drivers/gpu/ipu-v3/ipu-common.c +++ b/drivers/gpu/ipu-v3/ipu-common.c @@ -1102,7 +1102,7 @@ static void ipu_err_irq_handler(struct irq_desc *desc) { struct ipu_soc *ipu = irq_desc_get_handler_data(desc); struct irq_chip *chip = irq_desc_get_chip(desc); - const int int_reg[] = { 4, 5, 8, 9}; + static const int int_reg[] = { 4, 5, 8, 9}; chained_irq_enter(chip, desc); -->8-- If you don't mind, I'll amend this change. regards Philipp ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/omap: plane zpos/zorder management improvements
On 08/01/18 10:20, Peter Ujfalusi wrote: > Hi Laurent, > > On 2018-01-05 16:04, Laurent Pinchart wrote: >> Hi Peter, >> >> Thank you for the patch and sorry for the late review. >> >> On Friday, 5 January 2018 13:30:37 EET Peter Ujfalusi wrote: >>> Use the plane index as default zpos for all planes. Even if the >>> application is not setting zpos/zorder explicitly we will have unique zpos >>> for each plane. >>> >>> Enforce that all planes must have unique zpos on the given crtc. >> >> Could you explain the rationale for that in the commit message, what's wrong >> with duplicate zpos values ? > > Planes with identical zpos is only 'valid' _if_ they are not > overlapping, if they do overlap then it is - imho - not a valid > configuration anyway (which one should be on top?). For DSS it's clear. It is an invalid HW configuration to have multiple planes with the same zpos in the same crtc. I believe the result is undefined HW behavior. So we either return an error, or the kernel normalizes zpos'es. Normalizing means the kernel is guessing what the end result should be, so I like error better. 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 v2 1/5] drm/etnaviv: track fences by IDR instead of seqno
On Sun, 2018-01-07 at 15:51 +0100, Lucas Stach wrote: > This moves away from using the internal seqno as the userspace fence > reference. By moving to a generic ID, we can later replace the internal > fence by something different than the etnaviv seqno fence. > > Signed-off-by: Lucas Stach Reviewed-by: Philipp Zabel regards Philipp > --- > drivers/gpu/drm/etnaviv/etnaviv_gem.h| 1 + > drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 2 +- > drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 56 > +++- > drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 1 + > 4 files changed, 42 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h > b/drivers/gpu/drm/etnaviv/etnaviv_gem.h > index be72a9833f2b..c30964152381 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h > @@ -104,6 +104,7 @@ struct etnaviv_gem_submit { > struct kref refcount; > struct etnaviv_gpu *gpu; > struct dma_fence *out_fence, *in_fence; > + int out_fence_id; > struct list_head node; /* GPU active submit list */ > struct etnaviv_cmdbuf cmdbuf; > bool runtime_resumed; > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > index 1f8202bca061..919c8dc39f32 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c > @@ -563,7 +563,7 @@ int etnaviv_ioctl_gem_submit(struct drm_device *dev, void > *data, > } > > args->fence_fd = out_fence_fd; > - args->fence = submit->out_fence->seqno; > + args->fence = submit->out_fence_id; > > err_submit_objects: > etnaviv_submit_put(submit); > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > index 21d0d22f1168..935d99be748e 100644 > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c > @@ -1010,6 +1010,7 @@ static void hangcheck_disable(struct etnaviv_gpu *gpu) > /* fence object management */ > struct etnaviv_fence { > struct etnaviv_gpu *gpu; > + int id; > struct dma_fence base; > }; > > @@ -1046,6 +1047,11 @@ static void etnaviv_fence_release(struct dma_fence > *fence) > { > struct etnaviv_fence *f = to_etnaviv_fence(fence); > > + /* first remove from IDR, so fence can not be looked up anymore */ > + mutex_lock(&f->gpu->lock); > + idr_remove(&f->gpu->fence_idr, f->id); > + mutex_unlock(&f->gpu->lock); > + > kfree_rcu(f, base.rcu); > } > > @@ -1072,6 +1078,11 @@ static struct dma_fence > *etnaviv_gpu_fence_alloc(struct etnaviv_gpu *gpu) > if (!f) > return NULL; > > + f->id = idr_alloc_cyclic(&gpu->fence_idr, &f->base, 0, INT_MAX, > GFP_KERNEL); > + if (f->id < 0) { > + kfree(f); > + return NULL; > + } > f->gpu = gpu; > > dma_fence_init(&f->base, &etnaviv_fence_ops, &gpu->fence_spinlock, > @@ -1220,35 +1231,43 @@ static void retire_worker(struct work_struct *work) > } > > int etnaviv_gpu_wait_fence_interruptible(struct etnaviv_gpu *gpu, > - u32 fence, struct timespec *timeout) > + u32 id, struct timespec *timeout) > { > + struct dma_fence *fence; > int ret; > > - if (fence_after(fence, gpu->next_fence)) { > - DRM_ERROR("waiting on invalid fence: %u (of %u)\n", > - fence, gpu->next_fence); > - return -EINVAL; > - } > + /* > + * Look up the fence and take a reference. The mutex only synchronizes > + * the IDR lookup with the fence release. We might still find a fence > + * whose refcount has already dropped to zero. dma_fence_get_rcu > + * pretends we didn't find a fence in that case. > + */ > + ret = mutex_lock_interruptible(&gpu->lock); > + if (ret) > + return ret; > + fence = idr_find(&gpu->fence_idr, id); > + if (fence) > + fence = dma_fence_get_rcu(fence); > + mutex_unlock(&gpu->lock); > + > + if (!fence) > + return 0; > > if (!timeout) { > /* No timeout was requested: just test for completion */ > - ret = fence_completed(gpu, fence) ? 0 : -EBUSY; > + ret = dma_fence_is_signaled(fence) ? 0 : -EBUSY; > } else { > unsigned long remaining = etnaviv_timeout_to_jiffies(timeout); > > - ret = wait_event_interruptible_timeout(gpu->fence_event, > - fence_completed(gpu, fence), > - remaining); > - if (ret == 0) { > - DBG("timeout waiting for fence: %u (retired: %u > completed: %u)", > - fence, gpu->retired_fence, > - gpu->completed_fence); > + ret = dma_fence_wait_timeout(fence, true,
Re: [linux-sunxi] Re: [PATCH 01/11] clk: sunxi-ng: Don't set k if width is 0 for nkmp plls
On Fri, Jan 5, 2018 at 3:28 AM, Jernej Škrabec wrote: > Hi, > > Dne četrtek, 04. januar 2018 ob 15:45:18 CET je Chen-Yu Tsai napisal(a): >> On Sun, Dec 31, 2017 at 5:01 AM, Jernej Skrabec > wrote: >> > For example, A83T have nmp plls which are modelled as nkmp plls. Since k >> > is not specified, it has offset 0, shift 0 and lowest value 1. This >> > means that LSB bit is always set to 1, which may change clock rate. >> > >> > Fix that by applying k factor only if k width is greater than 0. >> > >> > Signed-off-by: Jernej Skrabec >> > --- >> > >> > drivers/clk/sunxi-ng/ccu_nkmp.c | 21 + >> > 1 file changed, 13 insertions(+), 8 deletions(-) >> > >> > diff --git a/drivers/clk/sunxi-ng/ccu_nkmp.c >> > b/drivers/clk/sunxi-ng/ccu_nkmp.c index e58c95787f94..709f528af2b3 100644 >> > --- a/drivers/clk/sunxi-ng/ccu_nkmp.c >> > +++ b/drivers/clk/sunxi-ng/ccu_nkmp.c >> > @@ -81,7 +81,7 @@ static unsigned long ccu_nkmp_recalc_rate(struct clk_hw >> > *hw,> >> > unsigned long parent_rate) >> > >> > { >> > >> > struct ccu_nkmp *nkmp = hw_to_ccu_nkmp(hw); >> > >> > - unsigned long n, m, k, p; >> > + unsigned long n, m, k = 1, p; >> > >> > u32 reg; >> > >> > reg = readl(nkmp->common.base + nkmp->common.reg); >> > >> > @@ -92,11 +92,13 @@ static unsigned long ccu_nkmp_recalc_rate(struct >> > clk_hw *hw,> >> > if (!n) >> > >> > n++; >> > >> > - k = reg >> nkmp->k.shift; >> > - k &= (1 << nkmp->k.width) - 1; >> > - k += nkmp->k.offset; >> > - if (!k) >> > - k++; >> > + if (nkmp->k.width) { >> > + k = reg >> nkmp->k.shift; >> > + k &= (1 << nkmp->k.width) - 1; >> > + k += nkmp->k.offset; >> > + if (!k) >> > + k++; >> > + } >> >> The conditional shouldn't be necessary. With nkmp->k.width = 0, >> you'd simply get k & 0, which is 0, which then gets bumped up to 1, >> unless k.offset > 1, which would be a bug. >> >> > m = reg >> nkmp->m.shift; >> > m &= (1 << nkmp->m.width) - 1; >> > >> > @@ -153,12 +155,15 @@ static int ccu_nkmp_set_rate(struct clk_hw *hw, >> > unsigned long rate,> >> > reg = readl(nkmp->common.base + nkmp->common.reg); >> > reg &= ~GENMASK(nkmp->n.width + nkmp->n.shift - 1, nkmp->n.shift); >> > >> > - reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift); >> > + if (nkmp->k.width) >> > + reg &= ~GENMASK(nkmp->k.width + nkmp->k.shift - 1, >> > + nkmp->k.shift); >> > >> > reg &= ~GENMASK(nkmp->m.width + nkmp->m.shift - 1, nkmp->m.shift); >> > reg &= ~GENMASK(nkmp->p.width + nkmp->p.shift - 1, nkmp->p.shift); >> > >> > reg |= (_nkmp.n - nkmp->n.offset) << nkmp->n.shift; >> > >> > - reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift; >> > + if (nkmp->k.width) >> > + reg |= (_nkmp.k - nkmp->k.offset) << nkmp->k.shift; >> >> I think a better way would be >> >> reg |= ((_nkmp.k - nkmp->k.offset) << nkmp->k.shift) & >>GENMASK(nkmp->k.width + nkmp->k.shift - 1, nkmp->k.shift); >> >> And do this for all the factors, not just k. This pattern is what >> regmap_update_bits does, which seems much safer. I wonder what >> GENMASK() with a negative value would do though... > > You're right, GENMASK(-1, 0) equals 0 (calculated by hand, not tested). This > seems much more elegant solution. > > Semi-related question: All nmp PLLs have much wider N range than real nkmp > PLLs. This causes integer overflow when using nkmp formula from datasheet. > Usually, N is 1-256 for nmp PLLs, which means that for very high N factors, it > overflows. This also causes issue that M factor is never higher than 1. Sounds like we can't use u8 for storing the factors. At least the intermediate values we use to calculate the rates. > > I was wondering, if patch would be acceptable which would change this formula: > > RATE = (24MHz * N * K) / (M * P) > > to this: > > RATE ((24MHz / M) * N * K) / P > > I checked all M factors and are all in 1-4 or 1-2 range, which means it > wouldn't have any impact for real nkmp PLLs when parent is 24 MHz clock which > is probably always. > > What do you think? I think this is acceptable. M is normally the pre-divider, so this actually fits how the hardware works, including possible rounding errors. ChenYu > I discovered that when I tried to set A83T PLL_VIDEO to 346.5 MHz which is > possible only when above formula is changed. > > Best regards, > Jernej > >> >> ChenYu >> >> > reg |= (_nkmp.m - nkmp->m.offset) << nkmp->m.shift; >> > reg |= ilog2(_nkmp.p) << nkmp->p.shift; >> > >> > -- >> > 2.15.1 > > > > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send a
Re: [Mesa-dev] Allocator Nouveau driver, Mesa EXT_external_objects, and DRM metadata import interfaces
Just wanted to clarify this one thing here, otherwise I think Rob/krh covered it all. On Thu, Dec 28, 2017 at 10:24:38AM -0800, Miguel Angel Vico wrote: > Daniel Vetter wrote: > > I think in the interim figuring out how to expose kms capabilities > > better (and necessarily standardizing at least some of them which > > matter at the compositor level, like size limits of framebuffers) > > feels like the place to push the ecosystem forward. In some way > > Miguel's proposal looks a bit backwards, since it adds the pitch > > capabilities to addfb, but at addfb time you've allocated everything > > already, so way too late to fix things up. With modifiers we've added > > a very simple per-plane property to list which modifiers can be > > combined with which pixel formats. Tiny start, but obviously very far > > from all that we'll need. > > Not sure whether I might be misunderstanding your statement, but one of > the allocator main features is negotiation of nearly optimal allocation > parameters given a set of uses on different devices/engines by the > capability merge operation. A client should have queried what every > device/engine is capable of for the given uses, find the optimal set of > capabilities, and use it for allocating a buffer. At the moment these > parameters are given to KMS, they are expected to be good. If they > aren't, the client didn't do things right. Your example code has a new capability for PITCH_ALIGNMENT. That looks wrong for addfb (which should only received the the computed intersection of all requirements, not the requirements itself). And since that was the only thing in your example code besides the bare boilerplate to wire it all up it looks a bit confused. Maybe we need to distinguish capabilities into constraints on properties (like pitch alignment, or power-of-two pitch) and properties (like pitch) themselves. -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
[PULL] drm-misc-fixes
Hi Dave, drm-misc-fixes-2018-01-08: Just one vc4 fix. As expected, nothing really happened over vacations. For -next we'll probably need to send one late pull request and then that's done too. Cheers, Daniel The following changes since commit e7cdf5c82f1773c3386b93bbcf13b9bfff29fa31: drm/syncobj: Stop reusing the same struct file for all syncobj -> fd (2017-12-22 14:14:39 +1000) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-fixes-2018-01-08 for you to fetch changes up to ce9caf2f79a5aa170a4b6456a03db639eed9c988: drm/vc4: Move IRQ enable to PM path (2018-01-03 15:56:03 -0800) Just one vc4 fix. Stefan Schake (1): drm/vc4: Move IRQ enable to PM path drivers/gpu/drm/vc4/vc4_irq.c | 3 --- drivers/gpu/drm/vc4/vc4_v3d.c | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) -- 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] drm/omap: plane zpos/zorder management improvements
Hello, On Monday, 8 January 2018 10:59:29 EET Tomi Valkeinen wrote: > On 08/01/18 10:20, Peter Ujfalusi wrote: > > On 2018-01-05 16:04, Laurent Pinchart wrote: > >> On Friday, 5 January 2018 13:30:37 EET Peter Ujfalusi wrote: > >>> Use the plane index as default zpos for all planes. Even if the > >>> application is not setting zpos/zorder explicitly we will have unique > >>> zpos for each plane. > >>> > >>> Enforce that all planes must have unique zpos on the given crtc. > >> > >> Could you explain the rationale for that in the commit message, what's > >> wrong with duplicate zpos values ? > > > > Planes with identical zpos is only 'valid' _if_ they are not > > overlapping, if they do overlap then it is - imho - not a valid > > configuration anyway (which one should be on top?). > > For DSS it's clear. It is an invalid HW configuration to have multiple > planes with the same zpos in the same crtc. I believe the result is > undefined HW behavior. > > So we either return an error, or the kernel normalizes zpos'es. > Normalizing means the kernel is guessing what the end result should be, > so I like error better. I wouldn't call that guessing :-) Duplicate zpos values result in plane being sorted based on the plane ID (this is obviously implementation-dependent, I mean this is the currently implemented behaviour), which I don't think is an issue in itself. A userspace zpos API that resolves conflicting zpos values that way isn't a broken API, even if its behaviour might be considered a bit complex or cumbersome. I'm not against forbidding duplicate zpos values, but I think the rationale should be captured in the kernel message. There's also a risk of breaking non-atomic userspace (as explained in my previous e-mail), as well as atomic userspace that doesn't set zpos explicitly if run after an application that changed the zpos values. True, that would today result in an undefined behaviour, so this might not be considered a problem. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/omap: plane zpos/zorder management improvements
Hi Peter, On Monday, 8 January 2018 10:20:24 EET Peter Ujfalusi wrote: > On 2018-01-05 16:04, Laurent Pinchart wrote: > > On Friday, 5 January 2018 13:30:37 EET Peter Ujfalusi wrote: > >> Use the plane index as default zpos for all planes. Even if the > >> application is not setting zpos/zorder explicitly we will have unique > >> zpos for each plane. > >> > >> Enforce that all planes must have unique zpos on the given crtc. > > > > Could you explain the rationale for that in the commit message, what's > > wrong with duplicate zpos values ? > > Planes with identical zpos is only 'valid' _if_ they are not > overlapping, if they do overlap then it is - imho - not a valid > configuration anyway (which one should be on top?). > Based on my tests the plane with lower planeID is going to disappear > from the screen if we have duplicated zpos. Please see my reply to Tomi on this topic. I'm not against the change, but I think the rationale should be captured in the commit message. > > Isn't there a risk of breaking the non-atomic userspace with this ? > > Without atomic commits userspace can't change the zpos of multiple planes > > in one go, so it might be impossible to reorder planes without going > > through a state that has duplicated zpos values. > > Two planes occupying the same position on the screen is not valid > (again, imho). At the hardware level for the DSS, sure. According to the KMS API, however, it is valid, even if the conflict resolution is driver-dependent. > If application wants to swap two planes, then it must disable one, move the > other to the new position, then enable and move the first plane. Applications don't do that at the moment, so there's a risk of breakage. As the current behaviour is undefined we might not considered that as a problem, but there's a risk of returning an error for an operation that currently succeeds. Personally I think all applications should move to the atomic API and handle zpos explicitly so I don't mind too much :) > >> Signed-off-by: Peter Ujfalusi > >> --- > >> Hi, > >> > >> Changes since v3: > >> - Use drm_plane_index() instead of storing the same index wothin > >> omap_plane struct > >> - Optimize the zpos validation loop so we avoid extra checks. > >> > >> Changes since v2: > >> - The check for duplicate zpos is moved to omap_crtc > >> > >> Changes since v1: > >> - Dropped the zpos normalization related patches > >> - New patch based on the discussion, see commit message. > >> > >> Regards, > >> Peter > >> > >> drivers/gpu/drm/omapdrm/omap_crtc.c | 36 +- > >> drivers/gpu/drm/omapdrm/omap_plane.c | 15 --- > >> 2 files changed, 39 insertions(+), 12 deletions(-) -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/omap: plane zpos/zorder management improvements
On 08/01/18 12:43, Laurent Pinchart wrote: > Hello, > > On Monday, 8 January 2018 10:59:29 EET Tomi Valkeinen wrote: >> On 08/01/18 10:20, Peter Ujfalusi wrote: >>> On 2018-01-05 16:04, Laurent Pinchart wrote: On Friday, 5 January 2018 13:30:37 EET Peter Ujfalusi wrote: > Use the plane index as default zpos for all planes. Even if the > application is not setting zpos/zorder explicitly we will have unique > zpos for each plane. > > Enforce that all planes must have unique zpos on the given crtc. Could you explain the rationale for that in the commit message, what's wrong with duplicate zpos values ? >>> >>> Planes with identical zpos is only 'valid' _if_ they are not >>> overlapping, if they do overlap then it is - imho - not a valid >>> configuration anyway (which one should be on top?). >> >> For DSS it's clear. It is an invalid HW configuration to have multiple >> planes with the same zpos in the same crtc. I believe the result is >> undefined HW behavior. >> >> So we either return an error, or the kernel normalizes zpos'es. >> Normalizing means the kernel is guessing what the end result should be, >> so I like error better. > > I wouldn't call that guessing :-) Duplicate zpos values result in plane being > sorted based on the plane ID (this is obviously implementation-dependent, I > mean this is the currently implemented behaviour), which I don't think is an > issue in itself. A userspace zpos API that resolves conflicting zpos values > that way isn't a broken API, even if its behaviour might be considered a bit > complex or cumbersome. > > I'm not against forbidding duplicate zpos values, but I think the rationale > should be captured in the kernel message. > > There's also a risk of breaking non-atomic userspace (as explained in my I think they are broken already (on omapdrm): the driver doesn't deal with this at the moment in any way, which leads to undefined behavior. I may remember wrong, but I think I have seen sync losts connected to bad z setup. But you are right, it's also possible that nothing bad seems to happen, if the only side effect is just that a plane disappears for a moment. And if this behavior for non-atocic apps is normal, and other drivers allow it, then I do agree that we have to normalize instead of returning an error. > previous e-mail), as well as atomic userspace that doesn't set zpos > explicitly > if run after an application that changed the zpos values. True, that would > today result in an undefined behaviour, so this might not be considered a > problem. Yes, this is a subject I have complained a few times: DRM keeping the state. I think that will be a problem with many other properties too. An app could change a platform specific property, which no other app is aware of, and after that no other app would work correctly. I believe each app has to know all the DRM properties and set them accordingly on startup, and/or each app has to reset the properties when exiting. Unfortunately I think both cases are not realistic. 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 v4] drm/omap: plane zpos/zorder management improvements
Hi Tomi, On Monday, 8 January 2018 12:58:28 EET Tomi Valkeinen wrote: > On 08/01/18 12:43, Laurent Pinchart wrote: > > On Monday, 8 January 2018 10:59:29 EET Tomi Valkeinen wrote: > >> On 08/01/18 10:20, Peter Ujfalusi wrote: > >>> On 2018-01-05 16:04, Laurent Pinchart wrote: > On Friday, 5 January 2018 13:30:37 EET Peter Ujfalusi wrote: > > Use the plane index as default zpos for all planes. Even if the > > application is not setting zpos/zorder explicitly we will have unique > > zpos for each plane. > > > > Enforce that all planes must have unique zpos on the given crtc. > > Could you explain the rationale for that in the commit message, what's > wrong with duplicate zpos values ? > >>> > >>> Planes with identical zpos is only 'valid' _if_ they are not > >>> overlapping, if they do overlap then it is - imho - not a valid > >>> configuration anyway (which one should be on top?). > >> > >> For DSS it's clear. It is an invalid HW configuration to have multiple > >> planes with the same zpos in the same crtc. I believe the result is > >> undefined HW behavior. > >> > >> So we either return an error, or the kernel normalizes zpos'es. > >> Normalizing means the kernel is guessing what the end result should be, > >> so I like error better. > > > > I wouldn't call that guessing :-) Duplicate zpos values result in plane > > being sorted based on the plane ID (this is obviously > > implementation-dependent, I mean this is the currently implemented > > behaviour), which I don't think is an issue in itself. A userspace zpos > > API that resolves conflicting zpos values that way isn't a broken API, > > even if its behaviour might be considered a bit complex or cumbersome. > > > > I'm not against forbidding duplicate zpos values, but I think the > > rationale > > should be captured in the kernel message. > > > > There's also a risk of breaking non-atomic userspace (as explained in my > > I think they are broken already (on omapdrm): the driver doesn't deal > with this at the moment in any way, which leads to undefined behavior. I > may remember wrong, but I think I have seen sync losts connected to bad > z setup. > > But you are right, it's also possible that nothing bad seems to happen, > if the only side effect is just that a plane disappears for a moment. > > And if this behavior for non-atocic apps is normal, and other drivers > allow it, then I do agree that we have to normalize instead of returning > an error. > > > previous e-mail), as well as atomic userspace that doesn't set zpos > > explicitly if run after an application that changed the zpos values. > > True, that would today result in an undefined behaviour, so this might > > not be considered a problem. > > Yes, this is a subject I have complained a few times: DRM keeping the > state. I think that will be a problem with many other properties too. An > app could change a platform specific property, which no other app is > aware of, and after that no other app would work correctly. > > I believe each app has to know all the DRM properties and set them > accordingly on startup, and/or each app has to reset the properties when > exiting. Unfortunately I think both cases are not realistic. I agree with you. I'd prefer restoring a sane default state on last close. Is there any reason not to do so ? -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
On Mon, Jan 08, 2018 at 08:42:50AM +0100, Thierry Reding wrote: > On Fri, Jan 05, 2018 at 05:58:17PM +0300, Dmitry Osipenko wrote: > > On 05.01.2018 17:17, Thierry Reding wrote: > > > Hi Dave, > > > > > > The following changes since commit > > > 9428088c90b6f7d5edd2a1b0d742c75339b36f6e: > > > > > > drm/qxl: reapply cursor after resetting primary (2017-12-08 13:37:02 > > > +1000) > > > > > > are available in the Git repository at: > > > > > > git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-4.16-rc1 > > > > > > for you to fetch changes up to ebae8d07435ae91314f4a28d69b530d09c625815: > > > > > > drm/tegra: dc: Implement legacy blending (2017-12-21 14:55:55 +0100) > > > > > > Thanks, > > > Thierry > > > > > > > > > drm/tegra: Changes for v4.16-rc1 > > > > > > The bulk of these changes are preparation work and addition of support > > > for Tegra186. Currently only HDMI output (the primary output on Jetson > > > TX2) is supported, but the hardware is also capable of doing DSI and > > > DisplayPort. > > > > > > Tegra DRM now also uses the atomic commit helpers instead of the open- > > > coded variant that was only doing half its job. As a bit of a byproduct > > > of the Tegra186 support the driver also gained HDMI 2.0 as well as zpos > > > property support. > > > > > > Along the way there are also a few patches to clean up a few things and > > > fix minor issues. > > > > > > > > > Arnd Bergmann (2): > > > drm/tegra: Mark Tegra186 display hub PM functions __maybe_unused > > > drm/tegra: Fix non-debugfs builds > > > > > > Dmitry Osipenko (3): > > > drm/tegra: dc: Link DC1 to DC0 on Tegra20 > > > drm/tegra: gem: Correct iommu_map_sg() error checking > > > drm/tegra: Correct timeout in tegra_syncpt_wait > > > > > > Thierry Reding (43): > > > drm/fourcc: Fix fourcc_mod_code() definition > > > drm/tegra: Sanitize format modifiers > > > gpu: host1x: Rewrite conditional for better readability > > > gpu: host1x: Cleanup on initialization failure > > > dt-bindings: display: tegra: Update SOR for Tegra186 > > > drm/tegra: dc: Move register definitions into a table > > > drm/tegra: dsi: Move register definitions into a table > > > drm/tegra: hdmi: Move register definitions into a table > > > drm/tegra: sor: Move register definitions into a table > > > drm/tegra: dc: Reshuffle some code > > > drm/tegra: dc: Register debugfs in ->late_register() > > > drm/tegra: dsi: Register debugfs in ->late_register() > > > drm/tegra: hdmi: Register debugfs in ->late_register() > > > drm/tegra: sor: Root debugfs files at the connector > > > drm/tegra: sor: Register debugfs in ->late_register() > > > drm/tegra: Do not wrap lines unnecessarily > > > drm/tegra: vic: Properly align arguments > > > drm/tegra: dc: Support background color > > > drm/tegra: Use atomic commit helpers > > > drm/tegra: Remove custom page-flip handler > > > drm/tegra: dc: Remove tegra_primary_plane_destroy() > > > drm/tegra: dc: Remove duplicate plane funcs > > > drm/tegra: dc: Remove tegra_overlay_plane_destroy() > > > drm/tegra: dc: Remove duplicate plane funcs > > > drm/tegra: dc: Move state definition to header > > > drm/tegra: Move common plane code to separate file > > > drm/tegra: Add Tegra186 display hub support > > > drm/tegra: dc: Add Tegra186 support > > > drm/tegra: Support ARGB and ABGR formats > > > drm/tegra: sor: Parameterize register offsets > > > drm/tegra: sor: Add Tegra186 support > > > drm/tegra: sor: Support HDMI 2.0 modes > > > drm/tegra: dpaux: Implement runtime PM > > > drm/tegra: dpaux: Add Tegra186 support > > > drm/tegra: fb: Force alpha formats > > > drm/tegra: dc: Support more formats > > > drm/tegra: dc: Use direct offset to plane registers > > > drm/tegra: dc: Remove redundant spinlock > > > drm/tegra: Implement zpos property > > > gpu: host1x: Use IOMMU groups > > > drm/tegra: Use IOMMU groups > > > drm/tegra: dpaux: Keep reset defaults for hybrid pad parameters > > > > > > > drm/tegra: dc: Implement legacy blending > > > > Please hold on this patch. First of all it doesn't work correctly, see my > > last > > reply to the patch. Secondly, it introduces bug that breaks YUV plane. > > I thought we had already concluded that this version doesn't cause any > regressions. And since this is new functionality I'm not too worried if > it doesn't work in all cases, we've got plenty of time to fix those up. > > As for the YUV plane bug, can you point me at a test case, or describe > what exactly you're trying to do so that I can reproduce and fix it? > > I'd like to make forward progress on this
[PATCH] drm/amdgpu: use %pap format string for phys_addr_t
The newly added get_local_mem_info() function prints a phys_addr_t using 0x%llx, which is wrong on most 32-bit systems, as shown by this warning: drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c: In function 'get_local_mem_info': include/linux/kern_levels.h:5:18: error: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type 'resource_size_t {aka unsigned int}' [-Werror=format=] drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c:297:31: note: format string is defined here pr_debug("Address base: 0x%llx limit 0x%llx public 0x%llx private 0x%llx\n", Passing the address by reference to the special %pap format string will produce the correct output and avoid the warning. Fixes: 30f1c0421ec5 ("drm/amdgpu: Implement get_local_mem_info") Signed-off-by: Arnd Bergmann --- drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c index 335e454e2ee1..1d605e1c1d66 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c @@ -294,8 +294,8 @@ void get_local_mem_info(struct kgd_dev *kgd, } mem_info->vram_width = adev->mc.vram_width; - pr_debug("Address base: 0x%llx limit 0x%llx public 0x%llx private 0x%llx\n", - adev->mc.aper_base, aper_limit, + pr_debug("Address base: %pap limit %pap public 0x%llx private 0x%llx\n", + &adev->mc.aper_base, &aper_limit, mem_info->local_mem_size_public, mem_info->local_mem_size_private); -- 2.9.0 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [drm_hwcomposer] [PATCH] Take Connection state into account. (v2)
Hey Mauro! Thanks for the v2, I would like to merge this, but the commit message is a little bit wonky still :) Let me clean it up for you, and if you're fine with me adding your S-o-B I'll push it. Also, if you want to avoid the slow mailing list back and forth, I would happily help out over IRC too. You can find me on freenode with the nick robertfoss. #dri-devel is also a good channel for this kind of work. On 1/6/18 12:59 AM, Mauro Rossi wrote: Porting of original commit 76fb87e675 of Jim Bish in android-ia master to fdo Original commit message: "There are various places where we should be really taking connection state into account before querying the properties or assuming it as primary. This patch fixes them." (v2) checks on connection state are applied for both internal and external connectors, in order to select the correct primary, as opposed to setting, independently from its state, the first connector as primary This is essential to avoid following logcat errors on integrated and dedicated GPUs: ... 2245 2245 E hwc-drm-resources: Could not find a suitable encoder/crtc for display 2 ... 2245 2245 E hwc-drm-resources: Failed CreateDisplayPipe 56 with -19 ... 2245 2245 E hwcomposer-drm: Can't initialize Drm object -19 Tested with i965 on Sandybridge and nouveau on GT120, GT610 This is what I would expect the commit message to look like: Take Connection state into account There are various places where we should be really taking connection state into account before querying the properties or assuming it as primary. This patch fixes them. Checks on connection state are applied for both internal and external connectors, in order to select the correct primary, as opposed to setting, independently from its state, the first connector as primary. This is essential to avoid following logcat errors on integrated and dedicated GPUs: ... 2245 2245 E hwc-drm-resources: Could not find a suitable encoder/crtc for display 2 ... 2245 2245 E hwc-drm-resources: Failed CreateDisplayPipe 56 with -19 ... 2245 2245 E hwcomposer-drm: Can't initialize Drm object -19 Tested with i965 on Sandybridge and nouveau on GT120, GT610 Signed-off-by: Jim Bish Signed-off-by: Mauro Rossi --- drmresources.cpp | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drmresources.cpp b/drmresources.cpp index 32dd376..d582cfe 100644 --- a/drmresources.cpp +++ b/drmresources.cpp @@ -159,7 +159,7 @@ int DrmResources::Init() { // First look for primary amongst internal connectors for (auto &conn : connectors_) { -if (conn->internal() && !found_primary) { +if (conn->state() == DRM_MODE_CONNECTED && conn->internal() && !found_primary) { conn->set_display(0); found_primary = true; } else { @@ -170,7 +170,7 @@ int DrmResources::Init() { // Then look for primary amongst external connectors for (auto &conn : connectors_) { -if (conn->external() && !found_primary) { +if (conn->state() == DRM_MODE_CONNECTED && conn->external() && !found_primary) { conn->set_display(0); found_primary = true; } @@ -288,6 +288,11 @@ int DrmResources::TryEncoderForDisplay(int display, DrmEncoder *enc) { int DrmResources::CreateDisplayPipe(DrmConnector *connector) { int display = connector->display(); + + // skip not connected + if (connector->state() == DRM_MODE_DISCONNECTED) +return 0; + /* Try to use current setup first */ if (connector->encoder()) { int ret = TryEncoderForDisplay(display, connector->encoder()); ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next
Hi Dave, This is a late pull request for 4.16. It our final one. Due to holidays we failed to send it before. In terms of features nothing really big/important apart from the addition of the Ilitek ILI9322 panel driver, that have been tested at linux-next for more than two weeks. The changes committed last week are driver specific and quite small. Regards, Gustavo drm-misc-next-2018-01-08: drm-misc-next for 4.16: Cross-subsystem Changes: - some dt-binding changes for Ilitek and sun4i devices Core Changes: - panel_orientation_quirks: fix tainted kernel Driver Changes: - panel changes - A83T and LVDS support to sun4i The following changes since commit 8d44e9e69aacecd522bb2797eb2febc5c6c46558: drm/framebuffer: Print task that allocated the fb in debug info. (2017-12-20 15:30:17 +0100) are available in the Git repository at: git://anongit.freedesktop.org/drm/drm-misc tags/drm-misc-next-2018-01-08 for you to fetch changes up to a1c55bccf6004ec9fbcf892328f9658767aa22bb: drm/panel: lvds: Add support for the power-supply property (2018-01-05 10:00:14 +0100) drm-misc-next for 4.16: Cross-subsystem Changes: - some dt-binding changes for Ilitek and sun4i devices Core Changes: - panel_orientation_quirks: fix tainted kernel Driver Changes: - panel changes - A83T and LVDS support to sun4i David Lechner (7): drm: fix tainted kernel caused by drm_panel_orientation_quirks.c dt-bindings: Add "vot" vendor prefix dt-bindings: update compatible string for ILI9225 drm/tinydrm: Update ILI9225 compatible string dt-bindings: add jianda vendor prefix dt-bindings: Add binding for Sitronix ST7735R display panels drm/tinydrm: add driver for ST7735R panels Linus Walleij (2): drm/panel: Add DT bindings for Ilitek ILI9322 drm/panel: Add Ilitek ILI9322 driver Maxime Ripard (8): dt-bindings: display: sun4i-drm: Add LVDS properties dt-bindings: display: sun4i-drm: Add A83T pipeline drm/sun4i: Force the mixer rate at 150MHz drm/sun4i: Create minimal multipliers and dividers drm/sun4i: Add LVDS support drm/sun4i: Add A83T support dt-bindings: panel: lvds: Document power-supply property drm/panel: lvds: Add support for the power-supply property .../devicetree/bindings/display/ilitek,ili9225.txt | 4 +- .../bindings/display/panel/ilitek,ili9322.txt | 49 ++ .../bindings/display/panel/panel-common.txt| 10 + .../bindings/display/panel/panel-lvds.txt | 1 + .../bindings/display/panel/simple-panel.txt| 2 +- .../bindings/display/sitronix,st7735r.txt | 35 + .../bindings/display/sunxi/sun4i-drm.txt | 11 + .../devicetree/bindings/vendor-prefixes.txt| 2 + MAINTAINERS| 6 + drivers/gpu/drm/drm_panel_orientation_quirks.c | 3 + drivers/gpu/drm/panel/Kconfig | 8 + drivers/gpu/drm/panel/Makefile | 1 + drivers/gpu/drm/panel/panel-ilitek-ili9322.c | 962 + drivers/gpu/drm/panel/panel-lvds.c | 23 + drivers/gpu/drm/sun4i/Makefile | 1 + drivers/gpu/drm/sun4i/sun4i_dotclock.c | 10 +- drivers/gpu/drm/sun4i/sun4i_drv.c | 1 + drivers/gpu/drm/sun4i/sun4i_lvds.c | 177 drivers/gpu/drm/sun4i/sun4i_lvds.h | 12 + drivers/gpu/drm/sun4i/sun4i_tcon.c | 244 +- drivers/gpu/drm/sun4i/sun4i_tcon.h | 31 + drivers/gpu/drm/sun4i/sun8i_mixer.c| 21 + drivers/gpu/drm/sun4i/sun8i_mixer.h| 3 + drivers/gpu/drm/tinydrm/Kconfig| 10 + drivers/gpu/drm/tinydrm/Makefile | 1 + drivers/gpu/drm/tinydrm/ili9225.c | 4 +- drivers/gpu/drm/tinydrm/st7735r.c | 215 + 27 files changed, 1838 insertions(+), 9 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/panel/ilitek,ili9322.txt create mode 100644 Documentation/devicetree/bindings/display/sitronix,st7735r.txt create mode 100644 drivers/gpu/drm/panel/panel-ilitek-ili9322.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_lvds.c create mode 100644 drivers/gpu/drm/sun4i/sun4i_lvds.h create mode 100644 drivers/gpu/drm/tinydrm/st7735r.c ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [drm_hwcomposer] [PATCH] Update external connectors list
Hey Mauro, This patch looks good to me apart from the commit message formatting. If you tell me I can add your SOB, I'll merge it with the below commit message. On 1/6/18 12:59 AM, Mauro Rossi wrote: > DVID, DVII and VGA are required by discrete and integrated GPUs I would expect something like: Update external connectors list VID, DVII and VGA are required by discrete and integrated GPUs. Signed-off-by: Mauro Rossi > > --- > drmconnector.cpp | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drmconnector.cpp b/drmconnector.cpp > index 247f56b..145518f 100644 > --- a/drmconnector.cpp > +++ b/drmconnector.cpp > @@ -73,7 +73,9 @@ bool DrmConnector::internal() const { > } > > bool DrmConnector::external() const { > - return type_ == DRM_MODE_CONNECTOR_HDMIA; > + return type_ == DRM_MODE_CONNECTOR_HDMIA || type_ == DRM_MODE_CONNECTOR_DisplayPort || > + type_ == DRM_MODE_CONNECTOR_DVID || type_ == DRM_MODE_CONNECTOR_DVII || > + type_ == DRM_MODE_CONNECTOR_VGA; > } > > bool DrmConnector::valid_type() const { > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104540] Corrupted colors in OBS-Studio window capture
https://bugs.freedesktop.org/show_bug.cgi?id=104540 Bug ID: 104540 Summary: Corrupted colors in OBS-Studio window capture Product: Mesa Version: git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/radeonsi Assignee: dri-devel@lists.freedesktop.org Reporter: network...@rkmail.ru QA Contact: dri-devel@lists.freedesktop.org Created attachment 136614 --> https://bugs.freedesktop.org/attachment.cgi?id=136614&action=edit Corrupted colors I see corrupted colors when using obs-studio to record a video of a window. It used to work fine a week or two ago. HW: Radeon HD7950 Kernel: 4.14.12 Mesa: git master commit 87efa71001d9a597304a2e8d7d63aaf1fa519056 LLVM: svn r321960 -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
On Mon, Jan 08, 2018 at 04:47:32PM +0300, Dmitry Osipenko wrote: > On 08.01.2018 10:42, Thierry Reding wrote: > > On Fri, Jan 05, 2018 at 05:58:17PM +0300, Dmitry Osipenko wrote: > >> On 05.01.2018 17:17, Thierry Reding wrote: > >>> Hi Dave, > >>> > >>> The following changes since commit > >>> 9428088c90b6f7d5edd2a1b0d742c75339b36f6e: > >>> > >>> drm/qxl: reapply cursor after resetting primary (2017-12-08 13:37:02 > >>> +1000) > >>> > >>> are available in the Git repository at: > >>> > >>> git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-4.16-rc1 > >>> > >>> for you to fetch changes up to ebae8d07435ae91314f4a28d69b530d09c625815: > >>> > >>> drm/tegra: dc: Implement legacy blending (2017-12-21 14:55:55 +0100) > >>> > >>> Thanks, > >>> Thierry > >>> > >>> > >>> drm/tegra: Changes for v4.16-rc1 > >>> > >>> The bulk of these changes are preparation work and addition of support > >>> for Tegra186. Currently only HDMI output (the primary output on Jetson > >>> TX2) is supported, but the hardware is also capable of doing DSI and > >>> DisplayPort. > >>> > >>> Tegra DRM now also uses the atomic commit helpers instead of the open- > >>> coded variant that was only doing half its job. As a bit of a byproduct > >>> of the Tegra186 support the driver also gained HDMI 2.0 as well as zpos > >>> property support. > >>> > >>> Along the way there are also a few patches to clean up a few things and > >>> fix minor issues. > >>> > >>> > >>> Arnd Bergmann (2): > >>> drm/tegra: Mark Tegra186 display hub PM functions __maybe_unused > >>> drm/tegra: Fix non-debugfs builds > >>> > >>> Dmitry Osipenko (3): > >>> drm/tegra: dc: Link DC1 to DC0 on Tegra20 > >>> drm/tegra: gem: Correct iommu_map_sg() error checking > >>> drm/tegra: Correct timeout in tegra_syncpt_wait > >>> > >>> Thierry Reding (43): > >>> drm/fourcc: Fix fourcc_mod_code() definition > >>> drm/tegra: Sanitize format modifiers > >>> gpu: host1x: Rewrite conditional for better readability > >>> gpu: host1x: Cleanup on initialization failure > >>> dt-bindings: display: tegra: Update SOR for Tegra186 > >>> drm/tegra: dc: Move register definitions into a table > >>> drm/tegra: dsi: Move register definitions into a table > >>> drm/tegra: hdmi: Move register definitions into a table > >>> drm/tegra: sor: Move register definitions into a table > >>> drm/tegra: dc: Reshuffle some code > >>> drm/tegra: dc: Register debugfs in ->late_register() > >>> drm/tegra: dsi: Register debugfs in ->late_register() > >>> drm/tegra: hdmi: Register debugfs in ->late_register() > >>> drm/tegra: sor: Root debugfs files at the connector > >>> drm/tegra: sor: Register debugfs in ->late_register() > >>> drm/tegra: Do not wrap lines unnecessarily > >>> drm/tegra: vic: Properly align arguments > >>> drm/tegra: dc: Support background color > >>> drm/tegra: Use atomic commit helpers > >>> drm/tegra: Remove custom page-flip handler > >>> drm/tegra: dc: Remove tegra_primary_plane_destroy() > >>> drm/tegra: dc: Remove duplicate plane funcs > >>> drm/tegra: dc: Remove tegra_overlay_plane_destroy() > >>> drm/tegra: dc: Remove duplicate plane funcs > >>> drm/tegra: dc: Move state definition to header > >>> drm/tegra: Move common plane code to separate file > >>> drm/tegra: Add Tegra186 display hub support > >>> drm/tegra: dc: Add Tegra186 support > >>> drm/tegra: Support ARGB and ABGR formats > >>> drm/tegra: sor: Parameterize register offsets > >>> drm/tegra: sor: Add Tegra186 support > >>> drm/tegra: sor: Support HDMI 2.0 modes > >>> drm/tegra: dpaux: Implement runtime PM > >>> drm/tegra: dpaux: Add Tegra186 support > >>> drm/tegra: fb: Force alpha formats > >>> drm/tegra: dc: Support more formats > >>> drm/tegra: dc: Use direct offset to plane registers > >>> drm/tegra: dc: Remove redundant spinlock > >>> drm/tegra: Implement zpos property > >>> gpu: host1x: Use IOMMU groups > >>> drm/tegra: Use IOMMU groups > >>> drm/tegra: dpaux: Keep reset defaults for hybrid pad parameters > >> > >> > >>> drm/tegra: dc: Implement legacy blending > >> > >> Please hold on this patch. First of all it doesn't work correctly, see my > >> last > >> reply to the patch. Secondly, it introduces bug that breaks YUV plane. > > > > I thought we had already concluded that this version doesn't cause any > > regressions. And since this is new functionality I'm not too worried if > > it doesn't work in all cases, we've got plenty of time to fix those up. > > My expectation was that you'll update the patch. I'm not sure why you'd want > to > go with something half-working while there is already a v
Re: [GIT PULL] drm/tegra: Changes for v4.16-rc1
On Mon, Jan 08, 2018 at 04:47:42PM +0300, Dmitry Osipenko wrote: > On 08.01.2018 15:39, Thierry Reding wrote: > > On Mon, Jan 08, 2018 at 08:42:50AM +0100, Thierry Reding wrote: > >> On Fri, Jan 05, 2018 at 05:58:17PM +0300, Dmitry Osipenko wrote: > >>> On 05.01.2018 17:17, Thierry Reding wrote: > Hi Dave, > > The following changes since commit > 9428088c90b6f7d5edd2a1b0d742c75339b36f6e: > > drm/qxl: reapply cursor after resetting primary (2017-12-08 13:37:02 > +1000) > > are available in the Git repository at: > > git://anongit.freedesktop.org/tegra/linux tags/drm/tegra/for-4.16-rc1 > > for you to fetch changes up to ebae8d07435ae91314f4a28d69b530d09c625815: > > drm/tegra: dc: Implement legacy blending (2017-12-21 14:55:55 +0100) > > Thanks, > Thierry > > > drm/tegra: Changes for v4.16-rc1 > > The bulk of these changes are preparation work and addition of support > for Tegra186. Currently only HDMI output (the primary output on Jetson > TX2) is supported, but the hardware is also capable of doing DSI and > DisplayPort. > > Tegra DRM now also uses the atomic commit helpers instead of the open- > coded variant that was only doing half its job. As a bit of a byproduct > of the Tegra186 support the driver also gained HDMI 2.0 as well as zpos > property support. > > Along the way there are also a few patches to clean up a few things and > fix minor issues. > > > Arnd Bergmann (2): > drm/tegra: Mark Tegra186 display hub PM functions __maybe_unused > drm/tegra: Fix non-debugfs builds > > Dmitry Osipenko (3): > drm/tegra: dc: Link DC1 to DC0 on Tegra20 > drm/tegra: gem: Correct iommu_map_sg() error checking > drm/tegra: Correct timeout in tegra_syncpt_wait > > Thierry Reding (43): > drm/fourcc: Fix fourcc_mod_code() definition > drm/tegra: Sanitize format modifiers > gpu: host1x: Rewrite conditional for better readability > gpu: host1x: Cleanup on initialization failure > dt-bindings: display: tegra: Update SOR for Tegra186 > drm/tegra: dc: Move register definitions into a table > drm/tegra: dsi: Move register definitions into a table > drm/tegra: hdmi: Move register definitions into a table > drm/tegra: sor: Move register definitions into a table > drm/tegra: dc: Reshuffle some code > drm/tegra: dc: Register debugfs in ->late_register() > drm/tegra: dsi: Register debugfs in ->late_register() > drm/tegra: hdmi: Register debugfs in ->late_register() > drm/tegra: sor: Root debugfs files at the connector > drm/tegra: sor: Register debugfs in ->late_register() > drm/tegra: Do not wrap lines unnecessarily > drm/tegra: vic: Properly align arguments > drm/tegra: dc: Support background color > drm/tegra: Use atomic commit helpers > drm/tegra: Remove custom page-flip handler > drm/tegra: dc: Remove tegra_primary_plane_destroy() > drm/tegra: dc: Remove duplicate plane funcs > drm/tegra: dc: Remove tegra_overlay_plane_destroy() > drm/tegra: dc: Remove duplicate plane funcs > drm/tegra: dc: Move state definition to header > drm/tegra: Move common plane code to separate file > drm/tegra: Add Tegra186 display hub support > drm/tegra: dc: Add Tegra186 support > drm/tegra: Support ARGB and ABGR formats > drm/tegra: sor: Parameterize register offsets > drm/tegra: sor: Add Tegra186 support > drm/tegra: sor: Support HDMI 2.0 modes > drm/tegra: dpaux: Implement runtime PM > drm/tegra: dpaux: Add Tegra186 support > drm/tegra: fb: Force alpha formats > drm/tegra: dc: Support more formats > drm/tegra: dc: Use direct offset to plane registers > drm/tegra: dc: Remove redundant spinlock > drm/tegra: Implement zpos property > gpu: host1x: Use IOMMU groups > drm/tegra: Use IOMMU groups > drm/tegra: dpaux: Keep reset defaults for hybrid pad parameters > >>> > >>> > drm/tegra: dc: Implement legacy blending > >>> > >>> Please hold on this patch. First of all it doesn't work correctly, see my > >>> last > >>> reply to the patch. Secondly, it introduces bug that breaks YUV plane. > >> > >> I thought we had already concluded that this version doesn't cause any > >> regressions. And since this is new functionality I'm not too worried if > >> it doesn't work in all cases, we've got plenty of t
[PATCH v5 0/9] drm/i915: Implement HDCP
Mostly a minor revision. The biggest item is that I've changed the property value "Off" to "Undesired". This reflects what Chrome is using (discovered by backporting the set to our downstream kernel and testing it). "Off" came from a botched backport someone did to the CrOS kernel a little while ago. I've dispelled all of the "Off" demons and everyone is using the correct nomenclature now. Also in this set are whitespace fixes and the R-b tags I've collected along the way. Fingers crossed, Sean Sean Paul (9): drm: Fix link-status kerneldoc line lengths drm/i915: Add more control to wait_for routines drm: Add Content Protection property drm: Add some HDCP related #defines drm/i915: Add HDCP framework + base implementation drm/i915: Make use of indexed write GMBUS feature drm/i915: Add function to output Aksv over GMBUS drm/i915: Implement HDCP for HDMI drm/i915: Implement HDCP for DisplayPort drivers/gpu/drm/drm_atomic.c | 8 + drivers/gpu/drm/drm_connector.c | 87 +++- drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 85 drivers/gpu/drm/i915/intel_atomic.c | 2 + drivers/gpu/drm/i915/intel_ddi.c | 36 ++ drivers/gpu/drm/i915/intel_display.c | 4 + drivers/gpu/drm/i915/intel_dp.c | 244 +++- drivers/gpu/drm/i915/intel_drv.h | 104 - drivers/gpu/drm/i915/intel_hdcp.c| 740 +++ drivers/gpu/drm/i915/intel_hdmi.c| 250 drivers/gpu/drm/i915/intel_i2c.c | 81 +++- drivers/gpu/drm/i915/intel_uncore.c | 23 +- drivers/gpu/drm/i915/intel_uncore.h | 14 +- include/drm/drm_connector.h | 16 + include/drm/drm_dp_helper.h | 17 + include/drm/drm_hdcp.h | 56 +++ include/uapi/drm/drm_mode.h | 4 + 19 files changed, 1730 insertions(+), 43 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_hdcp.c create mode 100644 include/drm/drm_hdcp.h -- 2.16.0.rc0.223.g4a4ac83678-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 1/9] drm: Fix link-status kerneldoc line lengths
I'm adding some stuff below it and it's killing my editor's vibe. Changes in v2: - Added to the series Changes in v3: - None Changes in v4: - None Changes in v5: - None Cc: Manasi Navare Acked-by: Daniel Vetter Signed-off-by: Sean Paul --- drivers/gpu/drm/drm_connector.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index e6a21e69059c..2559c615d984 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -817,10 +817,11 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name, * should update this value using drm_mode_connector_set_tile_property(). * Userspace cannot change this property. * link-status: - * Connector link-status property to indicate the status of link. The default - * value of link-status is "GOOD". If something fails during or after modeset, - * the kernel driver may set this to "BAD" and issue a hotplug uevent. Drivers - * should update this value using drm_mode_connector_set_link_status_property(). + * Connector link-status property to indicate the status of link. The + * default value of link-status is "GOOD". If something fails during or + * after modeset, the kernel driver may set this to "BAD" and issue a + * hotplug uevent. Drivers should update this value using + * drm_mode_connector_set_link_status_property(). * non_desktop: * Indicates the output should be ignored for purposes of displaying a * standard desktop environment or console. This is most likely because -- 2.16.0.rc0.223.g4a4ac83678-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5 3/9] drm: Add Content Protection property
This patch adds a new optional connector property to allow userspace to enable protection over the content it is displaying. This will typically be implemented by the driver using HDCP. The property is a tri-state with the following values: - OFF: Self explanatory, no content protection - DESIRED: Userspace requests that the driver enable protection - ENABLED: Once the driver has authenticated the link, it sets this value The driver is responsible for downgrading ENABLED to DESIRED if the link becomes unprotected. The driver should also maintain the desiredness of protection across hotplug/dpms/suspend. If this looks familiar, I posted [1] this 3 years ago. We have been using this in ChromeOS across exynos, mediatek, and rockchip over that time. Changes in v2: - Pimp kerneldoc for content_protection_property (Daniel) - Drop sysfs attribute Changes in v3: - None Changes in v4: - Changed kerneldoc to recommend userspace polling (Daniel) - Changed kerneldoc to briefly describe how to attach the property (Daniel) Changes in v5: - checkpatch whitespace noise - Change DRM_MODE_CONTENT_PROTECTION_OFF to DRM_MODE_CONTENT_PROTECTION_UNDESIRED Reviewed-by: Daniel Vetter Signed-off-by: Sean Paul [1] https://lists.freedesktop.org/archives/dri-devel/2014-December/073336.html --- drivers/gpu/drm/drm_atomic.c| 8 + drivers/gpu/drm/drm_connector.c | 78 + include/drm/drm_connector.h | 16 + include/uapi/drm/drm_mode.h | 4 +++ 4 files changed, 106 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index b76d49218cf1..69ff763a834e 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1224,6 +1224,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, state->picture_aspect_ratio = val; } else if (property == connector->scaling_mode_property) { state->scaling_mode = val; + } else if (property == connector->content_protection_property) { + if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) { + DRM_DEBUG_KMS("only drivers can set CP Enabled\n"); + return -EINVAL; + } + state->content_protection = val; } else if (connector->funcs->atomic_set_property) { return connector->funcs->atomic_set_property(connector, state, property, val); @@ -1303,6 +1309,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, *val = state->picture_aspect_ratio; } else if (property == connector->scaling_mode_property) { *val = state->scaling_mode; + } else if (property == connector->content_protection_property) { + *val = state->content_protection; } else if (connector->funcs->atomic_get_property) { return connector->funcs->atomic_get_property(connector, state, property, val); diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 2559c615d984..b85a7749709d 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -756,6 +756,13 @@ static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = { DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name, drm_tv_subconnector_enum_list) +static struct drm_prop_enum_list drm_cp_enum_list[] = { + { DRM_MODE_CONTENT_PROTECTION_UNDESIRED, "Undesired" }, + { DRM_MODE_CONTENT_PROTECTION_DESIRED, "Desired" }, + { DRM_MODE_CONTENT_PROTECTION_ENABLED, "Enabled" }, +}; +DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list) + /** * DOC: standard connector properties * @@ -826,6 +833,41 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name, * Indicates the output should be ignored for purposes of displaying a * standard desktop environment or console. This is most likely because * the output device is not rectilinear. + * Content Protection: + * This property is used by userspace to request the kernel protect future + * content communicated over the link. When requested, kernel will apply + * the appropriate means of protection (most often HDCP), and use the + * property to tell userspace the protection is active. + * + * Drivers can set this up by calling + * drm_connector_attach_content_protection_property() on initialization. + * + * The value of this property can be one of the following: + * + * - DRM_MODE_CONTENT_PROTECTION_UNDESIRED = 0 + * The link is not protected, content is transmitted in the clear. + * - DRM_MODE_CONTENT_PROTECTION_DESIRED = 1 + * Userspace has requested content protection, but the link is not + * currently protected. When in this state, kernel should enable + * Content Protection as soon as possible.
[PATCH v5 2/9] drm/i915: Add more control to wait_for routines
This patch adds a little more control to a couple wait_for routines such that we can avoid open-coding read/wait/timeout patterns which: - need the value of the register after the wait_for - run arbitrary operation for the read portion This patch also chooses the correct sleep function (based on timers-howto.txt) for the polling interval the caller specifies. Changes in v2: - Added to the series Changes in v3: - Rebased on drm-intel-next-queued and the new Wmin/max _wait_for - Removed msleep option Changes in v4: - Removed ; for OP in _wait_for (Chris) - Moved reg_value definition above ret (Chris) Changes in v4: - checkpatch whitespace fix Changes in v5: - None Suggested-by: Chris Wilson Reviewed-by: Daniel Vetter Reviewed-by: Chris Wilson Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/intel_drv.h| 17 ++--- drivers/gpu/drm/i915/intel_uncore.c | 23 --- drivers/gpu/drm/i915/intel_uncore.h | 14 +- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 30f791f89d64..9848e8eb0074 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -41,20 +41,21 @@ #include /** - * _wait_for - magic (register) wait macro + * __wait_for - magic wait macro * - * Does the right thing for modeset paths when run under kdgb or similar atomic - * contexts. Note that it's important that we check the condition again after - * having timed out, since the timeout could be due to preemption or similar and - * we've never had a chance to check the condition before the timeout. + * Macro to help avoid open coding check/wait/timeout patterns. Note that it's + * important that we check the condition again after having timed out, since the + * timeout could be due to preemption or similar and we've never had a chance to + * check the condition before the timeout. */ -#define _wait_for(COND, US, Wmin, Wmax) ({ \ +#define __wait_for(OP, COND, US, Wmin, Wmax) ({ \ unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1; \ long wait__ = (Wmin); /* recommended min for usleep is 10 us */ \ int ret__; \ might_sleep(); \ for (;;) { \ bool expired__ = time_after(jiffies, timeout__);\ + OP; \ if (COND) { \ ret__ = 0; \ break; \ @@ -70,7 +71,9 @@ ret__; \ }) -#define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 10, 1000) +#define _wait_for(COND, US, Wmin, Wmax)__wait_for(, (COND), (US), (Wmin), \ + (Wmax)) +#define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 10, 1000) /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */ #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 94e1fb3a2936..1c524ed1e1da 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1767,12 +1767,14 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv, } /** - * intel_wait_for_register - wait until register matches expected state + * __intel_wait_for_register - wait until register matches expected state * @dev_priv: the i915 device * @reg: the register to read * @mask: mask to apply to register value * @value: expected value - * @timeout_ms: timeout in millisecond + * @fast_timeout_us: fast timeout in microsecond for atomic/tight wait + * @slow_timeout_ms: slow timeout in millisecond + * @out_value: optional placeholder to hold registry value * * This routine waits until the target register @reg contains the expected * @value after applying the @mask, i.e. it waits until :: @@ -1783,14 +1785,17 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv, * * Returns 0 if the register matches the desired condition, or -ETIMEOUT. */ -int intel_wait_for_register(struct drm_i915_private *dev_priv, +int __intel_wait_for_register(struct drm_i915_private *dev_priv, i915_reg_t reg, u32 mask, u32 value, - unsigned int timeout_ms) + unsigned int fast_timeout_us, + unsigned int slow_timeout_ms, + u32 *out_value) { unsigned fw = intel_uncore_forcewake_
[PATCH v5 5/9] drm/i915: Add HDCP framework + base implementation
This patch adds the framework required to add HDCP support to intel connectors. It implements Aksv loading from fuse, and parts 1/2/3 of the HDCP authentication scheme. Note that without shim implementations, this does not actually implement HDCP. That will come in subsequent patches. Changes in v2: - Don't open code wait_fors (Chris) - drm_hdcp.c under MIT license (Daniel) - Move intel_hdcp_disable() call above ddi_disable (Ram) - Fix // comments (I wore a cone of shame for 12 hours to atone) (Daniel) - Justify intel_hdcp_shim with comments (Daniel) - Fixed async locking issues by adding hdcp_mutex (Daniel) - Don't alter connector_state in enable/disable (Daniel) Changes in v3: - Added hdcp_mutex/hdcp_value to make async reasonable - Added hdcp_prop_work to separate link checking & property setting - Added new helper for atomic_check state tracking (Daniel) - Moved enable/disable into atomic_commit with matching helpers - Moved intel_hdcp_check_link out of all locks when called from dp - Bumped up ksv_fifo timeout (noticed failure on one of my dongles) Changes in v4: - Remove SKL_ prefix from most register names (Daniel) - Move enable/disable back to modeset path (Daniel) - s/get_random_long/get_random_u32/ (Daniel) - Remove mode_config.mutex lock in prop_work (Daniel) - Add intel_hdcp_init to handle init of conn components (Daniel) - Actually check return value of attach_property - Check Bksv is valid before trying to authenticate (Ram) Changes in v5: - checkpatch whitespace changes - s/DRM_MODE_CONTENT_PROTECTION_OFF/DRM_MODE_CONTENT_PROTECTION_UNDESIRED/ - Fix ksv list wait timeout (actually wait 5s) - Increase the R0 timeout to 300ms (Ram) Cc: Chris Wilson Reviewed-by: Ramalingam C Reviewed-by: Daniel Vetter Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_reg.h | 83 drivers/gpu/drm/i915/intel_atomic.c | 2 + drivers/gpu/drm/i915/intel_ddi.c | 7 + drivers/gpu/drm/i915/intel_display.c | 4 + drivers/gpu/drm/i915/intel_drv.h | 85 drivers/gpu/drm/i915/intel_hdcp.c| 740 +++ 7 files changed, 922 insertions(+) create mode 100644 drivers/gpu/drm/i915/intel_hdcp.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 4d9e2f855e9d..3bddd8a06806 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -109,6 +109,7 @@ i915-y += intel_audio.o \ intel_fbc.o \ intel_fifo_underrun.o \ intel_frontbuffer.o \ + intel_hdcp.o \ intel_hotplug.o \ intel_modes.o \ intel_overlay.o \ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 505c605eff98..5db8d18c73ab 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8046,6 +8046,7 @@ enum { #define GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT 8 #define GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT 16 #define GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT 24 +#define SKL_PCODE_LOAD_HDCP_KEYS 0x5 #define SKL_PCODE_CDCLK_CONTROL 0x7 #define SKL_CDCLK_PREPARE_FOR_CHANGE 0x3 #define SKL_CDCLK_READY_FOR_CHANGE 0x1 @@ -8348,6 +8349,88 @@ enum skl_power_gate { #define SKL_PW_TO_PG(pw) ((pw) - SKL_DISP_PW_1 + SKL_PG1) #define SKL_FUSE_PG_DIST_STATUS(pg) (1 << (27 - (pg))) + +/* HDCP Key Registers */ +#define HDCP_KEY_CONF _MMIO(0x66c00) +#define HDCP_AKSV_SEND_TRIGGERBIT(31) +#define HDCP_CLEAR_KEYS_TRIGGER BIT(30) +#define HDCP_KEY_STATUS_MMIO(0x66c04) +#define HDCP_FUSE_IN_PROGRESS BIT(7) +#define HDCP_FUSE_ERROR BIT(6) +#define HDCP_FUSE_DONEBIT(5) +#define HDCP_KEY_LOAD_STATUS BIT(1) +#define HDCP_KEY_LOAD_DONEBIT(0) +#define HDCP_AKSV_LO _MMIO(0x66c10) +#define HDCP_AKSV_HI _MMIO(0x66c14) + +/* HDCP Repeater Registers */ +#define HDCP_REP_CTL _MMIO(0x66d00) +#define HDCP_DDIB_REP_PRESENT BIT(30) +#define HDCP_DDIA_REP_PRESENT BIT(29) +#define HDCP_DDIC_REP_PRESENT BIT(28) +#define HDCP_DDID_REP_PRESENT BIT(27) +#define HDCP_DDIF_REP_PRESENT BIT(26) +#define HDCP_DDIE_REP_PRESENT BIT(25) +#define HDCP_DDIB_SHA1_M0 (1 << 20) +#define HDCP_DDIA_SHA1_M0 (2 << 20) +#define HDCP_DDIC_SHA1_M0 (3 << 20) +#define HDCP_DDID_SHA1_M0 (4 << 20) +#define HDCP_DDIF_SHA1_M0 (5 << 20) +#define HDCP_DDIE_SHA1_M0 (6 << 20) /* Bspec says 5? */ +#define HDCP_SHA1_BUSYBIT(16) +#define HDCP_SHA1_READY BIT(17) +#define HDCP_SHA1_COMPLETEBIT(18) +#define HDCP_SHA1_V_MATCH BIT(19) +#define HDCP_SHA1_TEXT_32 (1 << 1) +#define HDCP_SHA1_COMPLETE_HASH (2 << 1) +#define HDCP_SHA1_TEXT_24 (4 << 1) +#define HDCP_SHA1_TEXT_16
[PATCH v5 7/9] drm/i915: Add function to output Aksv over GMBUS
Once the Aksv is available in the PCH, we need to get it on the wire to the receiver via DDC. The hardware doesn't allow us to read the value directly, so we need to tell GMBUS to source the Aksv internally and send it to the right offset on the receiver. The way we do this is to initiate an indexed write where the index is the Aksv register offset. We write dummy values to GMBUS3 as if we were sending the key, and the hardware slips in the "real" values when it goes out. Changes in v2: - None Changes in v3: - Uses new index write feature (Ville) Changes in v4: - None Changes in v5: - checkpatch whitespace fix Cc: Ville Syrjälä Reviewed-by: Daniel Vetter Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_i2c.c | 47 +--- 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index caebd5825279..a689396d0ff6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3644,6 +3644,7 @@ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv); extern void intel_teardown_gmbus(struct drm_i915_private *dev_priv); extern bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv, unsigned int pin); +extern int intel_gmbus_output_aksv(struct i2c_adapter *adapter); extern struct i2c_adapter * intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned int pin); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5db8d18c73ab..571e5097887c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3043,6 +3043,7 @@ enum i915_power_well_id { # define GPIO_DATA_PULLUP_DISABLE (1 << 13) #define GMBUS0 _MMIO(dev_priv->gpio_mmio_base + 0x5100) /* clock/port select */ +#define GMBUS_AKSV_SELECT(1<<11) #define GMBUS_RATE_100KHZ(0<<8) #define GMBUS_RATE_50KHZ (1<<8) #define GMBUS_RATE_400KHZ(2<<8) /* reserved on Pineview */ diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index d9e6993fa718..6f7ef4e225ee 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -30,6 +30,7 @@ #include #include #include +#include #include "intel_drv.h" #include #include "i915_drv.h" @@ -507,7 +508,8 @@ gmbus_index_xfer(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) } static int -do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) +do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num, + u32 gmbus0_source) { struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus, @@ -524,7 +526,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) pch_gmbus_clock_gating(dev_priv, false); retry: - I915_WRITE_FW(GMBUS0, bus->reg0); + I915_WRITE_FW(GMBUS0, gmbus0_source | bus->reg0); for (; i < num; i += inc) { inc = 1; @@ -649,7 +651,7 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) if (ret < 0) bus->force_bit &= ~GMBUS_FORCE_BIT_RETRY; } else { - ret = do_gmbus_xfer(adapter, msgs, num); + ret = do_gmbus_xfer(adapter, msgs, num, 0); if (ret == -EAGAIN) bus->force_bit |= GMBUS_FORCE_BIT_RETRY; } @@ -659,6 +661,45 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) return ret; } +int intel_gmbus_output_aksv(struct i2c_adapter *adapter) +{ + struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus, + adapter); + struct drm_i915_private *dev_priv = bus->dev_priv; + int ret; + u8 cmd = DRM_HDCP_DDC_AKSV; + u8 buf[DRM_HDCP_KSV_LEN] = { 0 }; + struct i2c_msg msgs[] = { + { + .addr = DRM_HDCP_DDC_ADDR, + .flags = 0, + .len = sizeof(cmd), + .buf = &cmd, + }, + { + .addr = DRM_HDCP_DDC_ADDR, + .flags = 0, + .len = sizeof(buf), + .buf = buf, + } + }; + + intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); + mutex_lock(&dev_priv->gmbus_mutex); + + /* +* In order to output Aksv to the receiver, use an indexed write to +* pass the i2c command, and tell GMBUS to use the HW-provided value +* instead of sourcing GMBUS3 for the data. +*/ + ret = do_gmbus_xfer(adapter, msgs, ARRAY_SIZE(msgs), GMBUS_AKSV_SELECT); + + mutex_
[PATCH v5 6/9] drm/i915: Make use of indexed write GMBUS feature
This patch enables the indexed write feature of the GMBUS to concatenate 2 consecutive messages into one. The criteria for an indexed write is that both messages are writes, the first is length == 1, and the second is length > 0. The first message is sent out by the GMBUS as the slave command, and the second one is sent via the GMBUS FIFO as usual. Changes in v3: - Added to series Changes in v4: - Combine indexed reads and writes (Ville) Changes in v5: - checkpatch whitespace nits Reviewed-by: Daniel Vetter Suggested-by: Ville Syrjälä Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/intel_i2c.c | 34 -- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index ef9f91a0b0c9..d9e6993fa718 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -402,7 +402,8 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, static int gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv, - unsigned short addr, u8 *buf, unsigned int len) + unsigned short addr, u8 *buf, unsigned int len, + u32 gmbus1_index) { unsigned int chunk_size = len; u32 val, loop; @@ -415,7 +416,7 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv, I915_WRITE_FW(GMBUS3, val); I915_WRITE_FW(GMBUS1, - GMBUS_CYCLE_WAIT | + gmbus1_index | GMBUS_CYCLE_WAIT | (chunk_size << GMBUS_BYTE_COUNT_SHIFT) | (addr << GMBUS_SLAVE_ADDR_SHIFT) | GMBUS_SLAVE_WRITE | GMBUS_SW_RDY); @@ -438,7 +439,8 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv, } static int -gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) +gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg, +u32 gmbus1_index) { u8 *buf = msg->buf; unsigned int tx_size = msg->len; @@ -448,7 +450,8 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) do { len = min(tx_size, GMBUS_BYTE_COUNT_MAX); - ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len); + ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len, +gmbus1_index); if (ret) return ret; @@ -460,21 +463,21 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) } /* - * The gmbus controller can combine a 1 or 2 byte write with a read that - * immediately follows it by using an "INDEX" cycle. + * The gmbus controller can combine a 1 or 2 byte write with another read/write + * that immediately follows it by using an "INDEX" cycle. */ static bool -gmbus_is_index_read(struct i2c_msg *msgs, int i, int num) +gmbus_is_index_xfer(struct i2c_msg *msgs, int i, int num) { return (i + 1 < num && msgs[i].addr == msgs[i + 1].addr && !(msgs[i].flags & I2C_M_RD) && (msgs[i].len == 1 || msgs[i].len == 2) && - (msgs[i + 1].flags & I2C_M_RD)); + msgs[i + 1].len > 0); } static int -gmbus_xfer_index_read(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) +gmbus_index_xfer(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) { u32 gmbus1_index = 0; u32 gmbus5 = 0; @@ -491,7 +494,10 @@ gmbus_xfer_index_read(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) if (gmbus5) I915_WRITE_FW(GMBUS5, gmbus5); - ret = gmbus_xfer_read(dev_priv, &msgs[1], gmbus1_index); + if (msgs[1].flags & I2C_M_RD) + ret = gmbus_xfer_read(dev_priv, &msgs[1], gmbus1_index); + else + ret = gmbus_xfer_write(dev_priv, &msgs[1], gmbus1_index); /* Clear GMBUS5 after each index transfer */ if (gmbus5) @@ -522,13 +528,13 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) for (; i < num; i += inc) { inc = 1; - if (gmbus_is_index_read(msgs, i, num)) { - ret = gmbus_xfer_index_read(dev_priv, &msgs[i]); - inc = 2; /* an index read is two msgs */ + if (gmbus_is_index_xfer(msgs, i, num)) { + ret = gmbus_index_xfer(dev_priv, &msgs[i]); + inc = 2; /* an index transmission is two msgs */ } else if (msgs[i].flags & I2C_M_RD) { ret = gmbus_xfer_read(dev_priv, &msgs[i], 0); } else { - ret = gmbus_xfer_write(dev_priv, &msgs[i]); + ret = gmbus_xfer_write(dev_priv, &msgs[i], 0); } if (!ret) -- 2.16.0.rc0.223.g4a4ac83678-goog ___
[PATCH v5 8/9] drm/i915: Implement HDCP for HDMI
This patch adds HDCP support for HDMI connectors by implementing the intel_hdcp_shim. Nothing too special, just a bunch of DDC reads/writes. Changes in v2: - Rebased on drm-intel-next Changes in v3: - Initialize new worker Changes in v4: - Remove SKL_ prefix from most register names (Daniel) - Wrap sanity checks in WARN_ON (Daniel) - Consolidate the enable/disable functions into one toggle fn - Use intel_hdcp_init (Daniel) Changes in v5: - checkpatch whitespace nits Cc: Daniel Vetter Reviewed-by: Ramalingam C Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ddi.c | 29 + drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_hdmi.c | 250 ++ 4 files changed, 282 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 571e5097887c..f773f2265af3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8463,6 +8463,7 @@ enum skl_power_gate { #define TRANS_DDI_EDP_INPUT_A_ONOFF (4<<12) #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12) #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) +#define TRANS_DDI_HDCP_SIGNALLING (1<<9) #define TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8) #define TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7) #define TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 2766b5e5c0ac..6260a882fbe4 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1615,6 +1615,35 @@ void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv, I915_WRITE(reg, val); } +int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder, +bool enable) +{ + struct drm_device *dev = intel_encoder->base.dev; + struct drm_i915_private *dev_priv = to_i915(dev); + enum pipe pipe = 0; + int ret = 0; + uint32_t tmp; + + if (WARN_ON(!intel_display_power_get_if_enabled(dev_priv, + intel_encoder->power_domain))) + return -ENXIO; + + if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) { + ret = -EIO; + goto out; + } + + tmp = I915_READ(TRANS_DDI_FUNC_CTL(pipe)); + if (enable) + tmp |= TRANS_DDI_HDCP_SIGNALLING; + else + tmp &= ~TRANS_DDI_HDCP_SIGNALLING; + I915_WRITE(TRANS_DDI_FUNC_CTL(pipe), tmp); +out: + intel_display_power_put(dev_priv, intel_encoder->power_domain); + return ret; +} + bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector) { struct drm_device *dev = intel_connector->base.dev; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index c59277c5b63e..731dc36d7129 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1377,6 +1377,8 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv, u32 bxt_signal_levels(struct intel_dp *intel_dp); uint32_t ddi_signal_levels(struct intel_dp *intel_dp); u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder); +int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder, +bool enable); unsigned int intel_fb_align_height(const struct drm_framebuffer *fb, int plane, unsigned int height); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index bced7b954d93..22251ad48b3b 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include "intel_drv.h" #include @@ -876,6 +877,248 @@ void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable) adapter, enable); } +static int intel_hdmi_hdcp_read(struct intel_digital_port *intel_dig_port, + unsigned int offset, void *buffer, size_t size) +{ + struct intel_hdmi *hdmi = &intel_dig_port->hdmi; + struct drm_i915_private *dev_priv = + intel_dig_port->base.base.dev->dev_private; + struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv, + hdmi->ddc_bus); + int ret; + u8 start = offset & 0xff; + struct i2c_msg msgs[] = { + { + .addr = DRM_HDCP_DDC_ADDR, + .flags = 0, + .len = 1, + .buf = &start, + }, + { + .addr = DRM_HDCP_DDC_ADDR, + .flags = I2C_M_RD, + .len = size, + .buf = buffer + }
[PATCH v5 9/9] drm/i915: Implement HDCP for DisplayPort
This patch adds HDCP support for DisplayPort connectors by implementing the intel_hdcp_shim. Most of this is straightforward read/write from/to DPCD registers. One thing worth pointing out is the Aksv output bit. It wasn't easily separable like it's HDMI counterpart, so it's crammed in with the rest of it. Changes in v2: - Moved intel_hdcp_check_link out of intel_dp_check_link and only call it on short pulse. Since intel_hdcp_check_link does its own locking, this ensures we don't deadlock when intel_dp_check_link is called holding connection_mutex. - Rebased on drm-intel-next Changes in v3: - Initialize new worker Changes in v4: - Use intel_hdcp_init (Daniel) - Check for reauth requests in check_link (Ram) Changes in v5: - None Cc: Daniel Vetter Reviewed-by: Ramalingam C Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/intel_dp.c | 244 ++-- 1 file changed, 237 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 35c5299feab6..1f2718c7d883 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -36,7 +36,9 @@ #include #include #include +#include #include +#include #include "intel_drv.h" #include #include "i915_drv.h" @@ -1025,10 +1027,29 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp, DP_AUX_CH_CTL_SYNC_PULSE_SKL(32); } +static uint32_t intel_dp_get_aux_send_ctl(struct intel_dp *intel_dp, + bool has_aux_irq, + int send_bytes, + uint32_t aux_clock_divider, + bool aksv_write) +{ + uint32_t val = 0; + + if (aksv_write) { + send_bytes += 5; + val |= DP_AUX_CH_CTL_AUX_AKSV_SELECT; + } + + return val | intel_dp->get_aux_send_ctl(intel_dp, + has_aux_irq, + send_bytes, + aux_clock_divider); +} + static int intel_dp_aux_ch(struct intel_dp *intel_dp, const uint8_t *send, int send_bytes, - uint8_t *recv, int recv_size) + uint8_t *recv, int recv_size, bool aksv_write) { struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct drm_i915_private *dev_priv = @@ -1088,10 +1109,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, } while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) { - u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, - has_aux_irq, - send_bytes, - aux_clock_divider); + u32 send_ctl = intel_dp_get_aux_send_ctl(intel_dp, +has_aux_irq, +send_bytes, +aux_clock_divider, +aksv_write); /* Must try at least 3 times according to DP spec */ for (try = 0; try < 5; try++) { @@ -1228,7 +1250,8 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) if (msg->buffer) memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size); - ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize, + false); if (ret > 0) { msg->reply = rxbuf[0] >> 4; @@ -1250,7 +1273,8 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) if (WARN_ON(rxsize > 20)) return -E2BIG; - ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize, + false); if (ret > 0) { msg->reply = rxbuf[0] >> 4; /* @@ -4985,6 +5009,203 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder) pps_unlock(intel_dp); } +static +int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port, + u8 *an) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base); + uint8_t txbuf[4], rxbuf[2], reply = 0; + ssize_t dpcd_ret; + int ret; + + /* Output An first, that's easy */ + dpcd_ret = drm_dp_dpcd_write(&intel_dig_port->dp.aux, DP_AUX_HDCP_AN, +
[PATCH v5 4/9] drm: Add some HDCP related #defines
In preparation for implementing HDCP in i915, add some HDCP related register offsets and defines. The dpcd register offsets will go in drm_dp_helper.h whereas the ddc offsets along with generic HDCP stuff will get stuffed in drm_hdcp.h, which is new. Changes in v2: - drm_hdcp.h gets MIT license (Daniel) Changes in v3: - None Changes in v4: - None Changes in v5: - None Cc: Daniel Vetter Reviewed-by: Ramalingam C Signed-off-by: Sean Paul --- include/drm/drm_dp_helper.h | 17 ++ include/drm/drm_hdcp.h | 56 + 2 files changed, 73 insertions(+) create mode 100644 include/drm/drm_hdcp.h diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index da58a428c8d7..9d3ce3b9b121 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -836,6 +836,23 @@ #define DP_CEC_TX_MESSAGE_BUFFER 0x3020 #define DP_CEC_MESSAGE_BUFFER_LENGTH 0x10 +#define DP_AUX_HDCP_BKSV 0x68000 +#define DP_AUX_HDCP_RI_PRIME 0x68005 +#define DP_AUX_HDCP_AKSV 0x68007 +#define DP_AUX_HDCP_AN 0x6800C +#define DP_AUX_HDCP_V_PRIME(h) (0x68014 + h * 4) +#define DP_AUX_HDCP_BCAPS 0x68028 +# define DP_BCAPS_REPEATER_PRESENT BIT(1) +# define DP_BCAPS_HDCP_CAPABLE BIT(0) +#define DP_AUX_HDCP_BSTATUS0x68029 +# define DP_BSTATUS_REAUTH_REQ BIT(3) +# define DP_BSTATUS_LINK_FAILURE BIT(2) +# define DP_BSTATUS_R0_PRIME_READY BIT(1) +# define DP_BSTATUS_READY BIT(0) +#define DP_AUX_HDCP_BINFO 0x6802A +#define DP_AUX_HDCP_KSV_FIFO 0x6802C +#define DP_AUX_HDCP_AINFO 0x6803B + /* DP 1.2 Sideband message defines */ /* peer device type - DP 1.2a Table 2-92 */ #define DP_PEER_DEVICE_NONE0x0 diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h new file mode 100644 index ..c9b2484240d4 --- /dev/null +++ b/include/drm/drm_hdcp.h @@ -0,0 +1,56 @@ +/* + * Copyright (C) 2017 Google, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR + * OTHER DEALINGS IN THE SOFTWARE. + * + * Authors: + * Sean Paul + */ + +#ifndef _DRM_HDCP_H_INCLUDED_ +#define _DRM_HDCP_H_INCLUDED_ + +/* Period of hdcp checks (to ensure we're still authenticated) */ +#define DRM_HDCP_CHECK_PERIOD_MS (128 * 16) + +/* Shared lengths/masks between HDMI/DVI/DisplayPort */ +#define DRM_HDCP_AN_LEN8 +#define DRM_HDCP_BSTATUS_LEN 2 +#define DRM_HDCP_KSV_LEN 5 +#define DRM_HDCP_RI_LEN2 +#define DRM_HDCP_V_PRIME_PART_LEN 4 +#define DRM_HDCP_V_PRIME_NUM_PARTS 5 +#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x3f) + +/* Slave address for the HDCP registers in the receiver */ +#define DRM_HDCP_DDC_ADDR 0x3A + +/* HDCP register offsets for HDMI/DVI devices */ +#define DRM_HDCP_DDC_BKSV 0x00 +#define DRM_HDCP_DDC_RI_PRIME 0x08 +#define DRM_HDCP_DDC_AKSV 0x10 +#define DRM_HDCP_DDC_AN0x18 +#define DRM_HDCP_DDC_V_PRIME(h)(0x20 + h * 4) +#define DRM_HDCP_DDC_BCAPS 0x40 +#define DRM_HDCP_DDC_BCAPS_REPEATER_PRESENT BIT(6) +#define DRM_HDCP_DDC_BCAPS_KSV_FIFO_READY BIT(5) +#define DRM_HDCP_DDC_BSTATUS 0x41 +#define DRM_HDCP_DDC_KSV_FIFO 0x43 + +#endif -- 2.16.0.rc0.223.g4a4ac83678-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm] amdgpu: fix not to add amdgpu.ids when building without amdgpu
On 2018-01-04 07:31 AM, Seung-Woo Kim wrote: > The amdgpu.ids is only required when building with amdgpu support. > Fix not to add it without amdgpu. > > Signed-off-by: Seung-Woo Kim > --- > data/Makefile.am |2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/data/Makefile.am b/data/Makefile.am > index eba915d..897a7f3 100644 > --- a/data/Makefile.am > +++ b/data/Makefile.am > @@ -20,4 +20,6 @@ > # CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. > > libdrmdatadir = @libdrmdatadir@ > +if HAVE_AMDGPU > dist_libdrmdata_DATA = amdgpu.ids > +endif > Reviewed and pushed, thanks! -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104285] Euro Truck Simulator 2 and American Truck Simulator artifacts (Mesa 17.3, AMD R9 280X)
https://bugs.freedesktop.org/show_bug.cgi?id=104285 --- Comment #4 from Gregor Münch --- Cant confirm with Radeon HD 7970 on mesa git and llvm git from today. ATS with shadows to high. Maybe its LLVM? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 0/9] drm/i915: Implement HDCP
This is The One. Differences between v6 and v5 include fixing a printk formatting issue that 0-day found, and changing the verbose licenses in the new files to SPDX tags. I'll push this to a topic branch in the drm-misc tree and send a pull request to Dave for 4.17 after the 4.16 merge window is over. Sincere thanks to Ram, Daniel, and others for their review. Now the fun part, testing and fixing bugs! Sean Sean Paul (9): drm: Fix link-status kerneldoc line lengths drm/i915: Add more control to wait_for routines drm: Add Content Protection property drm: Add some HDCP related #defines drm/i915: Add HDCP framework + base implementation drm/i915: Make use of indexed write GMBUS feature drm/i915: Add function to output Aksv over GMBUS drm/i915: Implement HDCP for HDMI drm/i915: Implement HDCP for DisplayPort drivers/gpu/drm/drm_atomic.c | 8 + drivers/gpu/drm/drm_connector.c | 87 - drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 85 drivers/gpu/drm/i915/intel_atomic.c | 2 + drivers/gpu/drm/i915/intel_ddi.c | 36 ++ drivers/gpu/drm/i915/intel_display.c | 4 + drivers/gpu/drm/i915/intel_dp.c | 244 +++- drivers/gpu/drm/i915/intel_drv.h | 104 - drivers/gpu/drm/i915/intel_hdcp.c| 723 +++ drivers/gpu/drm/i915/intel_hdmi.c| 250 drivers/gpu/drm/i915/intel_i2c.c | 81 +++- drivers/gpu/drm/i915/intel_uncore.c | 23 +- drivers/gpu/drm/i915/intel_uncore.h | 14 +- include/drm/drm_connector.h | 16 + include/drm/drm_dp_helper.h | 17 + include/drm/drm_hdcp.h | 39 ++ include/uapi/drm/drm_mode.h | 4 + 19 files changed, 1696 insertions(+), 43 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_hdcp.c create mode 100644 include/drm/drm_hdcp.h -- 2.16.0.rc0.223.g4a4ac83678-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 2/9] drm/i915: Add more control to wait_for routines
This patch adds a little more control to a couple wait_for routines such that we can avoid open-coding read/wait/timeout patterns which: - need the value of the register after the wait_for - run arbitrary operation for the read portion This patch also chooses the correct sleep function (based on timers-howto.txt) for the polling interval the caller specifies. Changes in v2: - Added to the series Changes in v3: - Rebased on drm-intel-next-queued and the new Wmin/max _wait_for - Removed msleep option Changes in v4: - Removed ; for OP in _wait_for (Chris) - Moved reg_value definition above ret (Chris) Changes in v4: - checkpatch whitespace fix Changes in v5: - None Changes in v6: - None Suggested-by: Chris Wilson Reviewed-by: Daniel Vetter Reviewed-by: Chris Wilson Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/intel_drv.h| 17 ++--- drivers/gpu/drm/i915/intel_uncore.c | 23 --- drivers/gpu/drm/i915/intel_uncore.h | 14 +- 3 files changed, 39 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 30f791f89d64..9848e8eb0074 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -41,20 +41,21 @@ #include /** - * _wait_for - magic (register) wait macro + * __wait_for - magic wait macro * - * Does the right thing for modeset paths when run under kdgb or similar atomic - * contexts. Note that it's important that we check the condition again after - * having timed out, since the timeout could be due to preemption or similar and - * we've never had a chance to check the condition before the timeout. + * Macro to help avoid open coding check/wait/timeout patterns. Note that it's + * important that we check the condition again after having timed out, since the + * timeout could be due to preemption or similar and we've never had a chance to + * check the condition before the timeout. */ -#define _wait_for(COND, US, Wmin, Wmax) ({ \ +#define __wait_for(OP, COND, US, Wmin, Wmax) ({ \ unsigned long timeout__ = jiffies + usecs_to_jiffies(US) + 1; \ long wait__ = (Wmin); /* recommended min for usleep is 10 us */ \ int ret__; \ might_sleep(); \ for (;;) { \ bool expired__ = time_after(jiffies, timeout__);\ + OP; \ if (COND) { \ ret__ = 0; \ break; \ @@ -70,7 +71,9 @@ ret__; \ }) -#define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 10, 1000) +#define _wait_for(COND, US, Wmin, Wmax)__wait_for(, (COND), (US), (Wmin), \ + (Wmax)) +#define wait_for(COND, MS) _wait_for((COND), (MS) * 1000, 10, 1000) /* If CONFIG_PREEMPT_COUNT is disabled, in_atomic() always reports false. */ #if defined(CONFIG_DRM_I915_DEBUG) && defined(CONFIG_PREEMPT_COUNT) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 94e1fb3a2936..1c524ed1e1da 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1767,12 +1767,14 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv, } /** - * intel_wait_for_register - wait until register matches expected state + * __intel_wait_for_register - wait until register matches expected state * @dev_priv: the i915 device * @reg: the register to read * @mask: mask to apply to register value * @value: expected value - * @timeout_ms: timeout in millisecond + * @fast_timeout_us: fast timeout in microsecond for atomic/tight wait + * @slow_timeout_ms: slow timeout in millisecond + * @out_value: optional placeholder to hold registry value * * This routine waits until the target register @reg contains the expected * @value after applying the @mask, i.e. it waits until :: @@ -1783,14 +1785,17 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv, * * Returns 0 if the register matches the desired condition, or -ETIMEOUT. */ -int intel_wait_for_register(struct drm_i915_private *dev_priv, +int __intel_wait_for_register(struct drm_i915_private *dev_priv, i915_reg_t reg, u32 mask, u32 value, - unsigned int timeout_ms) + unsigned int fast_timeout_us, + unsigned int slow_timeout_ms, + u32 *out_value) { unsigned fw = i
[PATCH v6 1/9] drm: Fix link-status kerneldoc line lengths
I'm adding some stuff below it and it's killing my editor's vibe. Changes in v2: - Added to the series Changes in v3: - None Changes in v4: - None Changes in v5: - None Changes in v6: - None Cc: Manasi Navare Acked-by: Daniel Vetter Signed-off-by: Sean Paul --- drivers/gpu/drm/drm_connector.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index e6a21e69059c..2559c615d984 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -817,10 +817,11 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name, * should update this value using drm_mode_connector_set_tile_property(). * Userspace cannot change this property. * link-status: - * Connector link-status property to indicate the status of link. The default - * value of link-status is "GOOD". If something fails during or after modeset, - * the kernel driver may set this to "BAD" and issue a hotplug uevent. Drivers - * should update this value using drm_mode_connector_set_link_status_property(). + * Connector link-status property to indicate the status of link. The + * default value of link-status is "GOOD". If something fails during or + * after modeset, the kernel driver may set this to "BAD" and issue a + * hotplug uevent. Drivers should update this value using + * drm_mode_connector_set_link_status_property(). * non_desktop: * Indicates the output should be ignored for purposes of displaying a * standard desktop environment or console. This is most likely because -- 2.16.0.rc0.223.g4a4ac83678-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 3/9] drm: Add Content Protection property
This patch adds a new optional connector property to allow userspace to enable protection over the content it is displaying. This will typically be implemented by the driver using HDCP. The property is a tri-state with the following values: - OFF: Self explanatory, no content protection - DESIRED: Userspace requests that the driver enable protection - ENABLED: Once the driver has authenticated the link, it sets this value The driver is responsible for downgrading ENABLED to DESIRED if the link becomes unprotected. The driver should also maintain the desiredness of protection across hotplug/dpms/suspend. If this looks familiar, I posted [1] this 3 years ago. We have been using this in ChromeOS across exynos, mediatek, and rockchip over that time. Changes in v2: - Pimp kerneldoc for content_protection_property (Daniel) - Drop sysfs attribute Changes in v3: - None Changes in v4: - Changed kerneldoc to recommend userspace polling (Daniel) - Changed kerneldoc to briefly describe how to attach the property (Daniel) Changes in v5: - checkpatch whitespace noise - Change DRM_MODE_CONTENT_PROTECTION_OFF to DRM_MODE_CONTENT_PROTECTION_UNDESIRED Changes in v6: - None Reviewed-by: Daniel Vetter Signed-off-by: Sean Paul [1] https://lists.freedesktop.org/archives/dri-devel/2014-December/073336.html --- drivers/gpu/drm/drm_atomic.c| 8 + drivers/gpu/drm/drm_connector.c | 78 + include/drm/drm_connector.h | 16 + include/uapi/drm/drm_mode.h | 4 +++ 4 files changed, 106 insertions(+) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index b76d49218cf1..69ff763a834e 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1224,6 +1224,12 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, state->picture_aspect_ratio = val; } else if (property == connector->scaling_mode_property) { state->scaling_mode = val; + } else if (property == connector->content_protection_property) { + if (val == DRM_MODE_CONTENT_PROTECTION_ENABLED) { + DRM_DEBUG_KMS("only drivers can set CP Enabled\n"); + return -EINVAL; + } + state->content_protection = val; } else if (connector->funcs->atomic_set_property) { return connector->funcs->atomic_set_property(connector, state, property, val); @@ -1303,6 +1309,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, *val = state->picture_aspect_ratio; } else if (property == connector->scaling_mode_property) { *val = state->scaling_mode; + } else if (property == connector->content_protection_property) { + *val = state->content_protection; } else if (connector->funcs->atomic_get_property) { return connector->funcs->atomic_get_property(connector, state, property, val); diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index 2559c615d984..b85a7749709d 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -756,6 +756,13 @@ static const struct drm_prop_enum_list drm_tv_subconnector_enum_list[] = { DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name, drm_tv_subconnector_enum_list) +static struct drm_prop_enum_list drm_cp_enum_list[] = { + { DRM_MODE_CONTENT_PROTECTION_UNDESIRED, "Undesired" }, + { DRM_MODE_CONTENT_PROTECTION_DESIRED, "Desired" }, + { DRM_MODE_CONTENT_PROTECTION_ENABLED, "Enabled" }, +}; +DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list) + /** * DOC: standard connector properties * @@ -826,6 +833,41 @@ DRM_ENUM_NAME_FN(drm_get_tv_subconnector_name, * Indicates the output should be ignored for purposes of displaying a * standard desktop environment or console. This is most likely because * the output device is not rectilinear. + * Content Protection: + * This property is used by userspace to request the kernel protect future + * content communicated over the link. When requested, kernel will apply + * the appropriate means of protection (most often HDCP), and use the + * property to tell userspace the protection is active. + * + * Drivers can set this up by calling + * drm_connector_attach_content_protection_property() on initialization. + * + * The value of this property can be one of the following: + * + * - DRM_MODE_CONTENT_PROTECTION_UNDESIRED = 0 + * The link is not protected, content is transmitted in the clear. + * - DRM_MODE_CONTENT_PROTECTION_DESIRED = 1 + * Userspace has requested content protection, but the link is not + * currently protected. When in this state, kernel should enable + * Content Protectio
[PATCH v6 4/9] drm: Add some HDCP related #defines
In preparation for implementing HDCP in i915, add some HDCP related register offsets and defines. The dpcd register offsets will go in drm_dp_helper.h whereas the ddc offsets along with generic HDCP stuff will get stuffed in drm_hdcp.h, which is new. Changes in v2: - drm_hdcp.h gets MIT license (Daniel) Changes in v3: - None Changes in v4: - None Changes in v5: - None Changes in v6: - SPDX license Cc: Daniel Vetter Reviewed-by: Ramalingam C Signed-off-by: Sean Paul --- include/drm/drm_dp_helper.h | 17 + include/drm/drm_hdcp.h | 39 +++ 2 files changed, 56 insertions(+) create mode 100644 include/drm/drm_hdcp.h diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index da58a428c8d7..9d3ce3b9b121 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -836,6 +836,23 @@ #define DP_CEC_TX_MESSAGE_BUFFER 0x3020 #define DP_CEC_MESSAGE_BUFFER_LENGTH 0x10 +#define DP_AUX_HDCP_BKSV 0x68000 +#define DP_AUX_HDCP_RI_PRIME 0x68005 +#define DP_AUX_HDCP_AKSV 0x68007 +#define DP_AUX_HDCP_AN 0x6800C +#define DP_AUX_HDCP_V_PRIME(h) (0x68014 + h * 4) +#define DP_AUX_HDCP_BCAPS 0x68028 +# define DP_BCAPS_REPEATER_PRESENT BIT(1) +# define DP_BCAPS_HDCP_CAPABLE BIT(0) +#define DP_AUX_HDCP_BSTATUS0x68029 +# define DP_BSTATUS_REAUTH_REQ BIT(3) +# define DP_BSTATUS_LINK_FAILURE BIT(2) +# define DP_BSTATUS_R0_PRIME_READY BIT(1) +# define DP_BSTATUS_READY BIT(0) +#define DP_AUX_HDCP_BINFO 0x6802A +#define DP_AUX_HDCP_KSV_FIFO 0x6802C +#define DP_AUX_HDCP_AINFO 0x6803B + /* DP 1.2 Sideband message defines */ /* peer device type - DP 1.2a Table 2-92 */ #define DP_PEER_DEVICE_NONE0x0 diff --git a/include/drm/drm_hdcp.h b/include/drm/drm_hdcp.h new file mode 100644 index ..43f7bd902b41 --- /dev/null +++ b/include/drm/drm_hdcp.h @@ -0,0 +1,39 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright (C) 2017 Google, Inc. + * + * Authors: + * Sean Paul + */ + +#ifndef _DRM_HDCP_H_INCLUDED_ +#define _DRM_HDCP_H_INCLUDED_ + +/* Period of hdcp checks (to ensure we're still authenticated) */ +#define DRM_HDCP_CHECK_PERIOD_MS (128 * 16) + +/* Shared lengths/masks between HDMI/DVI/DisplayPort */ +#define DRM_HDCP_AN_LEN8 +#define DRM_HDCP_BSTATUS_LEN 2 +#define DRM_HDCP_KSV_LEN 5 +#define DRM_HDCP_RI_LEN2 +#define DRM_HDCP_V_PRIME_PART_LEN 4 +#define DRM_HDCP_V_PRIME_NUM_PARTS 5 +#define DRM_HDCP_NUM_DOWNSTREAM(x) (x & 0x3f) + +/* Slave address for the HDCP registers in the receiver */ +#define DRM_HDCP_DDC_ADDR 0x3A + +/* HDCP register offsets for HDMI/DVI devices */ +#define DRM_HDCP_DDC_BKSV 0x00 +#define DRM_HDCP_DDC_RI_PRIME 0x08 +#define DRM_HDCP_DDC_AKSV 0x10 +#define DRM_HDCP_DDC_AN0x18 +#define DRM_HDCP_DDC_V_PRIME(h)(0x20 + h * 4) +#define DRM_HDCP_DDC_BCAPS 0x40 +#define DRM_HDCP_DDC_BCAPS_REPEATER_PRESENT BIT(6) +#define DRM_HDCP_DDC_BCAPS_KSV_FIFO_READY BIT(5) +#define DRM_HDCP_DDC_BSTATUS 0x41 +#define DRM_HDCP_DDC_KSV_FIFO 0x43 + +#endif -- 2.16.0.rc0.223.g4a4ac83678-goog ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v6 8/9] drm/i915: Implement HDCP for HDMI
This patch adds HDCP support for HDMI connectors by implementing the intel_hdcp_shim. Nothing too special, just a bunch of DDC reads/writes. Changes in v2: - Rebased on drm-intel-next Changes in v3: - Initialize new worker Changes in v4: - Remove SKL_ prefix from most register names (Daniel) - Wrap sanity checks in WARN_ON (Daniel) - Consolidate the enable/disable functions into one toggle fn - Use intel_hdcp_init (Daniel) Changes in v5: - checkpatch whitespace nits Changes in v6: - None Cc: Daniel Vetter Reviewed-by: Ramalingam C Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ddi.c | 29 + drivers/gpu/drm/i915/intel_drv.h | 2 + drivers/gpu/drm/i915/intel_hdmi.c | 250 ++ 4 files changed, 282 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 571e5097887c..f773f2265af3 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8463,6 +8463,7 @@ enum skl_power_gate { #define TRANS_DDI_EDP_INPUT_A_ONOFF (4<<12) #define TRANS_DDI_EDP_INPUT_B_ONOFF (5<<12) #define TRANS_DDI_EDP_INPUT_C_ONOFF (6<<12) +#define TRANS_DDI_HDCP_SIGNALLING (1<<9) #define TRANS_DDI_DP_VC_PAYLOAD_ALLOC (1<<8) #define TRANS_DDI_HDMI_SCRAMBLER_CTS_ENABLE (1<<7) #define TRANS_DDI_HDMI_SCRAMBLER_RESET_FREQ (1<<6) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 2766b5e5c0ac..6260a882fbe4 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -1615,6 +1615,35 @@ void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv, I915_WRITE(reg, val); } +int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder, +bool enable) +{ + struct drm_device *dev = intel_encoder->base.dev; + struct drm_i915_private *dev_priv = to_i915(dev); + enum pipe pipe = 0; + int ret = 0; + uint32_t tmp; + + if (WARN_ON(!intel_display_power_get_if_enabled(dev_priv, + intel_encoder->power_domain))) + return -ENXIO; + + if (WARN_ON(!intel_encoder->get_hw_state(intel_encoder, &pipe))) { + ret = -EIO; + goto out; + } + + tmp = I915_READ(TRANS_DDI_FUNC_CTL(pipe)); + if (enable) + tmp |= TRANS_DDI_HDCP_SIGNALLING; + else + tmp &= ~TRANS_DDI_HDCP_SIGNALLING; + I915_WRITE(TRANS_DDI_FUNC_CTL(pipe), tmp); +out: + intel_display_power_put(dev_priv, intel_encoder->power_domain); + return ret; +} + bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector) { struct drm_device *dev = intel_connector->base.dev; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index c59277c5b63e..731dc36d7129 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1377,6 +1377,8 @@ void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv, u32 bxt_signal_levels(struct intel_dp *intel_dp); uint32_t ddi_signal_levels(struct intel_dp *intel_dp); u8 intel_ddi_dp_voltage_max(struct intel_encoder *encoder); +int intel_ddi_toggle_hdcp_signalling(struct intel_encoder *intel_encoder, +bool enable); unsigned int intel_fb_align_height(const struct drm_framebuffer *fb, int plane, unsigned int height); diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c index bced7b954d93..22251ad48b3b 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c +++ b/drivers/gpu/drm/i915/intel_hdmi.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include "intel_drv.h" #include @@ -876,6 +877,248 @@ void intel_dp_dual_mode_set_tmds_output(struct intel_hdmi *hdmi, bool enable) adapter, enable); } +static int intel_hdmi_hdcp_read(struct intel_digital_port *intel_dig_port, + unsigned int offset, void *buffer, size_t size) +{ + struct intel_hdmi *hdmi = &intel_dig_port->hdmi; + struct drm_i915_private *dev_priv = + intel_dig_port->base.base.dev->dev_private; + struct i2c_adapter *adapter = intel_gmbus_get_adapter(dev_priv, + hdmi->ddc_bus); + int ret; + u8 start = offset & 0xff; + struct i2c_msg msgs[] = { + { + .addr = DRM_HDCP_DDC_ADDR, + .flags = 0, + .len = 1, + .buf = &start, + }, + { + .addr = DRM_HDCP_DDC_ADDR, + .flags = I2C_M_RD, + .len = size, + .buf = buf
[PATCH v6 9/9] drm/i915: Implement HDCP for DisplayPort
This patch adds HDCP support for DisplayPort connectors by implementing the intel_hdcp_shim. Most of this is straightforward read/write from/to DPCD registers. One thing worth pointing out is the Aksv output bit. It wasn't easily separable like it's HDMI counterpart, so it's crammed in with the rest of it. Changes in v2: - Moved intel_hdcp_check_link out of intel_dp_check_link and only call it on short pulse. Since intel_hdcp_check_link does its own locking, this ensures we don't deadlock when intel_dp_check_link is called holding connection_mutex. - Rebased on drm-intel-next Changes in v3: - Initialize new worker Changes in v4: - Use intel_hdcp_init (Daniel) - Check for reauth requests in check_link (Ram) Changes in v5: - None Changes in v6: - Fix build warnings when printing ssize_t Cc: Daniel Vetter Reviewed-by: Ramalingam C Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/intel_dp.c | 244 ++-- 1 file changed, 237 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 35c5299feab6..68229f53d5b8 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -36,7 +36,9 @@ #include #include #include +#include #include +#include #include "intel_drv.h" #include #include "i915_drv.h" @@ -1025,10 +1027,29 @@ static uint32_t skl_get_aux_send_ctl(struct intel_dp *intel_dp, DP_AUX_CH_CTL_SYNC_PULSE_SKL(32); } +static uint32_t intel_dp_get_aux_send_ctl(struct intel_dp *intel_dp, + bool has_aux_irq, + int send_bytes, + uint32_t aux_clock_divider, + bool aksv_write) +{ + uint32_t val = 0; + + if (aksv_write) { + send_bytes += 5; + val |= DP_AUX_CH_CTL_AUX_AKSV_SELECT; + } + + return val | intel_dp->get_aux_send_ctl(intel_dp, + has_aux_irq, + send_bytes, + aux_clock_divider); +} + static int intel_dp_aux_ch(struct intel_dp *intel_dp, const uint8_t *send, int send_bytes, - uint8_t *recv, int recv_size) + uint8_t *recv, int recv_size, bool aksv_write) { struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp); struct drm_i915_private *dev_priv = @@ -1088,10 +1109,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp, } while ((aux_clock_divider = intel_dp->get_aux_clock_divider(intel_dp, clock++))) { - u32 send_ctl = intel_dp->get_aux_send_ctl(intel_dp, - has_aux_irq, - send_bytes, - aux_clock_divider); + u32 send_ctl = intel_dp_get_aux_send_ctl(intel_dp, +has_aux_irq, +send_bytes, +aux_clock_divider, +aksv_write); /* Must try at least 3 times according to DP spec */ for (try = 0; try < 5; try++) { @@ -1228,7 +1250,8 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) if (msg->buffer) memcpy(txbuf + HEADER_SIZE, msg->buffer, msg->size); - ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize, + false); if (ret > 0) { msg->reply = rxbuf[0] >> 4; @@ -1250,7 +1273,8 @@ intel_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg) if (WARN_ON(rxsize > 20)) return -E2BIG; - ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize); + ret = intel_dp_aux_ch(intel_dp, txbuf, txsize, rxbuf, rxsize, + false); if (ret > 0) { msg->reply = rxbuf[0] >> 4; /* @@ -4985,6 +5009,203 @@ void intel_dp_encoder_suspend(struct intel_encoder *intel_encoder) pps_unlock(intel_dp); } +static +int intel_dp_hdcp_write_an_aksv(struct intel_digital_port *intel_dig_port, + u8 *an) +{ + struct intel_dp *intel_dp = enc_to_intel_dp(&intel_dig_port->base.base); + uint8_t txbuf[4], rxbuf[2], reply = 0; + ssize_t dpcd_ret; + int ret; + + /* Output An first, that's easy */ + dpcd_ret = drm_dp_dpcd_write(&intel_dig_p
[PATCH v6 7/9] drm/i915: Add function to output Aksv over GMBUS
Once the Aksv is available in the PCH, we need to get it on the wire to the receiver via DDC. The hardware doesn't allow us to read the value directly, so we need to tell GMBUS to source the Aksv internally and send it to the right offset on the receiver. The way we do this is to initiate an indexed write where the index is the Aksv register offset. We write dummy values to GMBUS3 as if we were sending the key, and the hardware slips in the "real" values when it goes out. Changes in v2: - None Changes in v3: - Uses new index write feature (Ville) Changes in v4: - None Changes in v5: - checkpatch whitespace fix Changes in v6: - None Cc: Ville Syrjälä Reviewed-by: Daniel Vetter Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/i915_drv.h | 1 + drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_i2c.c | 47 +--- 3 files changed, 46 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index caebd5825279..a689396d0ff6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3644,6 +3644,7 @@ extern int intel_setup_gmbus(struct drm_i915_private *dev_priv); extern void intel_teardown_gmbus(struct drm_i915_private *dev_priv); extern bool intel_gmbus_is_valid_pin(struct drm_i915_private *dev_priv, unsigned int pin); +extern int intel_gmbus_output_aksv(struct i2c_adapter *adapter); extern struct i2c_adapter * intel_gmbus_get_adapter(struct drm_i915_private *dev_priv, unsigned int pin); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 5db8d18c73ab..571e5097887c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -3043,6 +3043,7 @@ enum i915_power_well_id { # define GPIO_DATA_PULLUP_DISABLE (1 << 13) #define GMBUS0 _MMIO(dev_priv->gpio_mmio_base + 0x5100) /* clock/port select */ +#define GMBUS_AKSV_SELECT(1<<11) #define GMBUS_RATE_100KHZ(0<<8) #define GMBUS_RATE_50KHZ (1<<8) #define GMBUS_RATE_400KHZ(2<<8) /* reserved on Pineview */ diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index d9e6993fa718..6f7ef4e225ee 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -30,6 +30,7 @@ #include #include #include +#include #include "intel_drv.h" #include #include "i915_drv.h" @@ -507,7 +508,8 @@ gmbus_index_xfer(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) } static int -do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) +do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num, + u32 gmbus0_source) { struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus, @@ -524,7 +526,7 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) pch_gmbus_clock_gating(dev_priv, false); retry: - I915_WRITE_FW(GMBUS0, bus->reg0); + I915_WRITE_FW(GMBUS0, gmbus0_source | bus->reg0); for (; i < num; i += inc) { inc = 1; @@ -649,7 +651,7 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) if (ret < 0) bus->force_bit &= ~GMBUS_FORCE_BIT_RETRY; } else { - ret = do_gmbus_xfer(adapter, msgs, num); + ret = do_gmbus_xfer(adapter, msgs, num, 0); if (ret == -EAGAIN) bus->force_bit |= GMBUS_FORCE_BIT_RETRY; } @@ -659,6 +661,45 @@ gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) return ret; } +int intel_gmbus_output_aksv(struct i2c_adapter *adapter) +{ + struct intel_gmbus *bus = container_of(adapter, struct intel_gmbus, + adapter); + struct drm_i915_private *dev_priv = bus->dev_priv; + int ret; + u8 cmd = DRM_HDCP_DDC_AKSV; + u8 buf[DRM_HDCP_KSV_LEN] = { 0 }; + struct i2c_msg msgs[] = { + { + .addr = DRM_HDCP_DDC_ADDR, + .flags = 0, + .len = sizeof(cmd), + .buf = &cmd, + }, + { + .addr = DRM_HDCP_DDC_ADDR, + .flags = 0, + .len = sizeof(buf), + .buf = buf, + } + }; + + intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS); + mutex_lock(&dev_priv->gmbus_mutex); + + /* +* In order to output Aksv to the receiver, use an indexed write to +* pass the i2c command, and tell GMBUS to use the HW-provided value +* instead of sourcing GMBUS3 for the data. +*/ + ret = do_gmbus_xfer(adapter, msgs, ARRAY_SIZE(msgs), GMBUS_AKSV_SEL
[PATCH v6 5/9] drm/i915: Add HDCP framework + base implementation
This patch adds the framework required to add HDCP support to intel connectors. It implements Aksv loading from fuse, and parts 1/2/3 of the HDCP authentication scheme. Note that without shim implementations, this does not actually implement HDCP. That will come in subsequent patches. Changes in v2: - Don't open code wait_fors (Chris) - drm_hdcp.c under MIT license (Daniel) - Move intel_hdcp_disable() call above ddi_disable (Ram) - Fix // comments (I wore a cone of shame for 12 hours to atone) (Daniel) - Justify intel_hdcp_shim with comments (Daniel) - Fixed async locking issues by adding hdcp_mutex (Daniel) - Don't alter connector_state in enable/disable (Daniel) Changes in v3: - Added hdcp_mutex/hdcp_value to make async reasonable - Added hdcp_prop_work to separate link checking & property setting - Added new helper for atomic_check state tracking (Daniel) - Moved enable/disable into atomic_commit with matching helpers - Moved intel_hdcp_check_link out of all locks when called from dp - Bumped up ksv_fifo timeout (noticed failure on one of my dongles) Changes in v4: - Remove SKL_ prefix from most register names (Daniel) - Move enable/disable back to modeset path (Daniel) - s/get_random_long/get_random_u32/ (Daniel) - Remove mode_config.mutex lock in prop_work (Daniel) - Add intel_hdcp_init to handle init of conn components (Daniel) - Actually check return value of attach_property - Check Bksv is valid before trying to authenticate (Ram) Changes in v5: - checkpatch whitespace changes - s/DRM_MODE_CONTENT_PROTECTION_OFF/DRM_MODE_CONTENT_PROTECTION_UNDESIRED/ - Fix ksv list wait timeout (actually wait 5s) - Increase the R0 timeout to 300ms (Ram) Changes in v6: - SPDX license Cc: Chris Wilson Reviewed-by: Ramalingam C Reviewed-by: Daniel Vetter Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/Makefile| 1 + drivers/gpu/drm/i915/i915_reg.h | 83 drivers/gpu/drm/i915/intel_atomic.c | 2 + drivers/gpu/drm/i915/intel_ddi.c | 7 + drivers/gpu/drm/i915/intel_display.c | 4 + drivers/gpu/drm/i915/intel_drv.h | 85 drivers/gpu/drm/i915/intel_hdcp.c| 723 +++ 7 files changed, 905 insertions(+) create mode 100644 drivers/gpu/drm/i915/intel_hdcp.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 4d9e2f855e9d..3bddd8a06806 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -109,6 +109,7 @@ i915-y += intel_audio.o \ intel_fbc.o \ intel_fifo_underrun.o \ intel_frontbuffer.o \ + intel_hdcp.o \ intel_hotplug.o \ intel_modes.o \ intel_overlay.o \ diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 505c605eff98..5db8d18c73ab 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -8046,6 +8046,7 @@ enum { #define GEN9_MEM_LATENCY_LEVEL_1_5_SHIFT 8 #define GEN9_MEM_LATENCY_LEVEL_2_6_SHIFT 16 #define GEN9_MEM_LATENCY_LEVEL_3_7_SHIFT 24 +#define SKL_PCODE_LOAD_HDCP_KEYS 0x5 #define SKL_PCODE_CDCLK_CONTROL 0x7 #define SKL_CDCLK_PREPARE_FOR_CHANGE 0x3 #define SKL_CDCLK_READY_FOR_CHANGE 0x1 @@ -8348,6 +8349,88 @@ enum skl_power_gate { #define SKL_PW_TO_PG(pw) ((pw) - SKL_DISP_PW_1 + SKL_PG1) #define SKL_FUSE_PG_DIST_STATUS(pg) (1 << (27 - (pg))) + +/* HDCP Key Registers */ +#define HDCP_KEY_CONF _MMIO(0x66c00) +#define HDCP_AKSV_SEND_TRIGGERBIT(31) +#define HDCP_CLEAR_KEYS_TRIGGER BIT(30) +#define HDCP_KEY_STATUS_MMIO(0x66c04) +#define HDCP_FUSE_IN_PROGRESS BIT(7) +#define HDCP_FUSE_ERROR BIT(6) +#define HDCP_FUSE_DONEBIT(5) +#define HDCP_KEY_LOAD_STATUS BIT(1) +#define HDCP_KEY_LOAD_DONEBIT(0) +#define HDCP_AKSV_LO _MMIO(0x66c10) +#define HDCP_AKSV_HI _MMIO(0x66c14) + +/* HDCP Repeater Registers */ +#define HDCP_REP_CTL _MMIO(0x66d00) +#define HDCP_DDIB_REP_PRESENT BIT(30) +#define HDCP_DDIA_REP_PRESENT BIT(29) +#define HDCP_DDIC_REP_PRESENT BIT(28) +#define HDCP_DDID_REP_PRESENT BIT(27) +#define HDCP_DDIF_REP_PRESENT BIT(26) +#define HDCP_DDIE_REP_PRESENT BIT(25) +#define HDCP_DDIB_SHA1_M0 (1 << 20) +#define HDCP_DDIA_SHA1_M0 (2 << 20) +#define HDCP_DDIC_SHA1_M0 (3 << 20) +#define HDCP_DDID_SHA1_M0 (4 << 20) +#define HDCP_DDIF_SHA1_M0 (5 << 20) +#define HDCP_DDIE_SHA1_M0 (6 << 20) /* Bspec says 5? */ +#define HDCP_SHA1_BUSYBIT(16) +#define HDCP_SHA1_READY BIT(17) +#define HDCP_SHA1_COMPLETEBIT(18) +#define HDCP_SHA1_V_MATCH BIT(19) +#define HDCP_SHA1_TEXT_32 (1 << 1) +#define HDCP_SHA1_COMPLETE_HASH (2 << 1) +#define HDCP_SHA1_TEXT_24 (4 << 1) +#
[PATCH v6 6/9] drm/i915: Make use of indexed write GMBUS feature
This patch enables the indexed write feature of the GMBUS to concatenate 2 consecutive messages into one. The criteria for an indexed write is that both messages are writes, the first is length == 1, and the second is length > 0. The first message is sent out by the GMBUS as the slave command, and the second one is sent via the GMBUS FIFO as usual. Changes in v3: - Added to series Changes in v4: - Combine indexed reads and writes (Ville) Changes in v5: - checkpatch whitespace nits Changes in v6: - None Reviewed-by: Daniel Vetter Suggested-by: Ville Syrjälä Signed-off-by: Sean Paul --- drivers/gpu/drm/i915/intel_i2c.c | 34 -- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index ef9f91a0b0c9..d9e6993fa718 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -402,7 +402,8 @@ gmbus_xfer_read(struct drm_i915_private *dev_priv, struct i2c_msg *msg, static int gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv, - unsigned short addr, u8 *buf, unsigned int len) + unsigned short addr, u8 *buf, unsigned int len, + u32 gmbus1_index) { unsigned int chunk_size = len; u32 val, loop; @@ -415,7 +416,7 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv, I915_WRITE_FW(GMBUS3, val); I915_WRITE_FW(GMBUS1, - GMBUS_CYCLE_WAIT | + gmbus1_index | GMBUS_CYCLE_WAIT | (chunk_size << GMBUS_BYTE_COUNT_SHIFT) | (addr << GMBUS_SLAVE_ADDR_SHIFT) | GMBUS_SLAVE_WRITE | GMBUS_SW_RDY); @@ -438,7 +439,8 @@ gmbus_xfer_write_chunk(struct drm_i915_private *dev_priv, } static int -gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) +gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg, +u32 gmbus1_index) { u8 *buf = msg->buf; unsigned int tx_size = msg->len; @@ -448,7 +450,8 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) do { len = min(tx_size, GMBUS_BYTE_COUNT_MAX); - ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len); + ret = gmbus_xfer_write_chunk(dev_priv, msg->addr, buf, len, +gmbus1_index); if (ret) return ret; @@ -460,21 +463,21 @@ gmbus_xfer_write(struct drm_i915_private *dev_priv, struct i2c_msg *msg) } /* - * The gmbus controller can combine a 1 or 2 byte write with a read that - * immediately follows it by using an "INDEX" cycle. + * The gmbus controller can combine a 1 or 2 byte write with another read/write + * that immediately follows it by using an "INDEX" cycle. */ static bool -gmbus_is_index_read(struct i2c_msg *msgs, int i, int num) +gmbus_is_index_xfer(struct i2c_msg *msgs, int i, int num) { return (i + 1 < num && msgs[i].addr == msgs[i + 1].addr && !(msgs[i].flags & I2C_M_RD) && (msgs[i].len == 1 || msgs[i].len == 2) && - (msgs[i + 1].flags & I2C_M_RD)); + msgs[i + 1].len > 0); } static int -gmbus_xfer_index_read(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) +gmbus_index_xfer(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) { u32 gmbus1_index = 0; u32 gmbus5 = 0; @@ -491,7 +494,10 @@ gmbus_xfer_index_read(struct drm_i915_private *dev_priv, struct i2c_msg *msgs) if (gmbus5) I915_WRITE_FW(GMBUS5, gmbus5); - ret = gmbus_xfer_read(dev_priv, &msgs[1], gmbus1_index); + if (msgs[1].flags & I2C_M_RD) + ret = gmbus_xfer_read(dev_priv, &msgs[1], gmbus1_index); + else + ret = gmbus_xfer_write(dev_priv, &msgs[1], gmbus1_index); /* Clear GMBUS5 after each index transfer */ if (gmbus5) @@ -522,13 +528,13 @@ do_gmbus_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs, int num) for (; i < num; i += inc) { inc = 1; - if (gmbus_is_index_read(msgs, i, num)) { - ret = gmbus_xfer_index_read(dev_priv, &msgs[i]); - inc = 2; /* an index read is two msgs */ + if (gmbus_is_index_xfer(msgs, i, num)) { + ret = gmbus_index_xfer(dev_priv, &msgs[i]); + inc = 2; /* an index transmission is two msgs */ } else if (msgs[i].flags & I2C_M_RD) { ret = gmbus_xfer_read(dev_priv, &msgs[i], 0); } else { - ret = gmbus_xfer_write(dev_priv, &msgs[i]); + ret = gmbus_xfer_write(dev_priv, &msgs[i], 0); } if (!ret) -- 2.16.0.rc0.223.g4a4ac83678-goog _
Re: [PATCH v6 0/9] drm/i915: Implement HDCP
On Mon, Jan 8, 2018 at 2:55 PM, Sean Paul wrote: > This is The One. Differences between v6 and v5 include fixing a printk > formatting issue that 0-day found, and changing the verbose licenses in the > new > files to SPDX tags. > > I'll push this to a topic branch in the drm-misc tree and send a pull request > to > Dave for 4.17 after the 4.16 merge window is over. > Applied to https://cgit.freedesktop.org/drm-misc/log/?h=topic/hdcp Sean > Sincere thanks to Ram, Daniel, and others for their review. Now the fun part, > testing and fixing bugs! > > Sean > > Sean Paul (9): > drm: Fix link-status kerneldoc line lengths > drm/i915: Add more control to wait_for routines > drm: Add Content Protection property > drm: Add some HDCP related #defines > drm/i915: Add HDCP framework + base implementation > drm/i915: Make use of indexed write GMBUS feature > drm/i915: Add function to output Aksv over GMBUS > drm/i915: Implement HDCP for HDMI > drm/i915: Implement HDCP for DisplayPort > > drivers/gpu/drm/drm_atomic.c | 8 + > drivers/gpu/drm/drm_connector.c | 87 - > drivers/gpu/drm/i915/Makefile| 1 + > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_reg.h | 85 > drivers/gpu/drm/i915/intel_atomic.c | 2 + > drivers/gpu/drm/i915/intel_ddi.c | 36 ++ > drivers/gpu/drm/i915/intel_display.c | 4 + > drivers/gpu/drm/i915/intel_dp.c | 244 +++- > drivers/gpu/drm/i915/intel_drv.h | 104 - > drivers/gpu/drm/i915/intel_hdcp.c| 723 > +++ > drivers/gpu/drm/i915/intel_hdmi.c| 250 > drivers/gpu/drm/i915/intel_i2c.c | 81 +++- > drivers/gpu/drm/i915/intel_uncore.c | 23 +- > drivers/gpu/drm/i915/intel_uncore.h | 14 +- > include/drm/drm_connector.h | 16 + > include/drm/drm_dp_helper.h | 17 + > include/drm/drm_hdcp.h | 39 ++ > include/uapi/drm/drm_mode.h | 4 + > 19 files changed, 1696 insertions(+), 43 deletions(-) > create mode 100644 drivers/gpu/drm/i915/intel_hdcp.c > create mode 100644 include/drm/drm_hdcp.h > > -- > 2.16.0.rc0.223.g4a4ac83678-goog > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [drm_hwcomposer] [PATCH] Take Connection state into account. (v2)
On Sat, Jan 06, 2018 at 12:59:58AM +0100, Mauro Rossi wrote: > Porting of original commit 76fb87e675 of Jim Bish in android-ia master to fdo > > Original commit message: > "There are various places where we should be really taking connection > state into account before querying the properties or assuming it > as primary. This patch fixes them." > > (v2) checks on connection state are applied for both internal and external > connectors, in order to select the correct primary, as opposed to setting, > independently from its state, the first connector as primary > > This is essential to avoid following logcat errors on integrated and > dedicated GPUs: > > ... 2245 2245 E hwc-drm-resources: Could not find a suitable encoder/crtc > for display 2 > ... 2245 2245 E hwc-drm-resources: Failed CreateDisplayPipe 56 with -19 > ... 2245 2245 E hwcomposer-drm: Can't initialize Drm object -19 > > Tested with i965 on Sandybridge and nouveau on GT120, GT610 > --- > drmresources.cpp | 9 +++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drmresources.cpp b/drmresources.cpp > index 32dd376..d582cfe 100644 > --- a/drmresources.cpp > +++ b/drmresources.cpp > @@ -159,7 +159,7 @@ int DrmResources::Init() { > >// First look for primary amongst internal connectors >for (auto &conn : connectors_) { > -if (conn->internal() && !found_primary) { > +if (conn->state() == DRM_MODE_CONNECTED && conn->internal() && > !found_primary) { >conn->set_display(0); >found_primary = true; > } else { > @@ -170,7 +170,7 @@ int DrmResources::Init() { > >// Then look for primary amongst external connectors >for (auto &conn : connectors_) { > -if (conn->external() && !found_primary) { > +if (conn->state() == DRM_MODE_CONNECTED && conn->external() && > !found_primary) { These 2 lines exceed the max character limit. Did you run clang-format? Anyways, I think it'd be nice to add a connected() helper to the connector object which would look cleaner and solve the long lines. Sean >conn->set_display(0); >found_primary = true; > } > @@ -288,6 +288,11 @@ int DrmResources::TryEncoderForDisplay(int display, > DrmEncoder *enc) { > > int DrmResources::CreateDisplayPipe(DrmConnector *connector) { >int display = connector->display(); > + > + // skip not connected > + if (connector->state() == DRM_MODE_DISCONNECTED) > +return 0; > + >/* Try to use current setup first */ >if (connector->encoder()) { > int ret = TryEncoderForDisplay(display, connector->encoder()); > -- > 2.14.1 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [drm_hwcomposer] [PATCH] Take Connection state into account. (v2)
On Mon, Jan 08, 2018 at 03:41:49PM -0500, Sean Paul wrote: > On Sat, Jan 06, 2018 at 12:59:58AM +0100, Mauro Rossi wrote: > > Porting of original commit 76fb87e675 of Jim Bish in android-ia master to > > fdo > > > > Original commit message: > > "There are various places where we should be really taking connection > > state into account before querying the properties or assuming it > > as primary. This patch fixes them." > > > > (v2) checks on connection state are applied for both internal and external > > connectors, in order to select the correct primary, as opposed to setting, > > independently from its state, the first connector as primary > > > > This is essential to avoid following logcat errors on integrated and > > dedicated GPUs: > > > > ... 2245 2245 E hwc-drm-resources: Could not find a suitable encoder/crtc > > for display 2 > > ... 2245 2245 E hwc-drm-resources: Failed CreateDisplayPipe 56 with -19 > > ... 2245 2245 E hwcomposer-drm: Can't initialize Drm object -19 > > > > Tested with i965 on Sandybridge and nouveau on GT120, GT610 > > --- > > drmresources.cpp | 9 +++-- > > 1 file changed, 7 insertions(+), 2 deletions(-) > > > > diff --git a/drmresources.cpp b/drmresources.cpp > > index 32dd376..d582cfe 100644 > > --- a/drmresources.cpp > > +++ b/drmresources.cpp > > @@ -159,7 +159,7 @@ int DrmResources::Init() { > > > >// First look for primary amongst internal connectors > >for (auto &conn : connectors_) { > > -if (conn->internal() && !found_primary) { > > +if (conn->state() == DRM_MODE_CONNECTED && conn->internal() && > > !found_primary) { One more thing. How do you know this is the right thing to do? What if the internal panel is not connected initially, but becomes connected in the future? IIUC, you'll end up numbering it incorrectly. Sean > >conn->set_display(0); > >found_primary = true; > > } else { > > @@ -170,7 +170,7 @@ int DrmResources::Init() { > > > >// Then look for primary amongst external connectors > >for (auto &conn : connectors_) { > > -if (conn->external() && !found_primary) { > > +if (conn->state() == DRM_MODE_CONNECTED && conn->external() && > > !found_primary) { > > These 2 lines exceed the max character limit. Did you run clang-format? > > Anyways, I think it'd be nice to add a connected() helper to the connector > object which would look cleaner and solve the long lines. > > Sean > > >conn->set_display(0); > >found_primary = true; > > } > > @@ -288,6 +288,11 @@ int DrmResources::TryEncoderForDisplay(int display, > > DrmEncoder *enc) { > > > > int DrmResources::CreateDisplayPipe(DrmConnector *connector) { > >int display = connector->display(); > > + > > + // skip not connected > > + if (connector->state() == DRM_MODE_DISCONNECTED) > > +return 0; > > + > >/* Try to use current setup first */ > >if (connector->encoder()) { > > int ret = TryEncoderForDisplay(display, connector->encoder()); > > -- > > 2.14.1 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Sean Paul, Software Engineer, Google / Chromium OS -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/atomic: Fix memleak on ERESTARTSYS during non-blocking commits
On 2018-01-04 02:47 PM, sunpeng...@amd.com wrote: > From: "Leo (Sunpeng) Li" > > During a non-blocking commit, it is possible to return before the > commit_tail work is queued (-ERESTARTSYS, for example). > > Since a reference on the crtc commit object is obtained for the pending > vblank event when preparing the commit, the above situation will leave > us with an extra reference. > > Therefore, if the commit_tail worker has not consumed the event at the > end of a commit, release it's reference. > > Signed-off-by: Leo (Sunpeng) Li No expert on this but looks sane to me. Acked-by: Harry Wentland Harry > --- > drivers/gpu/drm/drm_atomic_helper.c | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index ab40321..4253f57 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -3421,6 +3421,15 @@ EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state); > void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc_state *state) > { > if (state->commit) { > + /* > + * In the event that a non-blocking commit returns > + * -ERESTARTSYS before the commit_tail work is queued, we will > + * have an extra reference to the commit object. Release it, if > + * the event has not been consumed by the worker. > + */ > + if (state->event) > + drm_crtc_commit_put(state->commit); > + > kfree(state->commit->event); > state->commit->event = NULL; > drm_crtc_commit_put(state->commit); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104185] CS:GO randomly locks up the entire system requiring forced reboot
https://bugs.freedesktop.org/show_bug.cgi?id=104185 --- Comment #3 from quen...@intrepide.io --- I have the same issue I have a Ryzen 1800x with Vega 56 with different config ubuntu 17.10 with mainline 4.15rc kernel and amdgpu with mesa sable (17.2) or git (17.4) (from different ppa) with llvm 5 fedora 27 with rawhide 4.15rc kernel and amdgpu with mesa git from copr repo (https://copr.fedorainfracloud.org/coprs/che/mesa/) with llvm 6 ubuntu 16.04.3 with official amd amdgpu 17.50 (no pro, with amd mesa 17.2) same result, crash when I play Cache map on loading and somtime random crash on other map When I install and play with Ubuntu 16.04.3 with amdgpu-pro 17.50 and Centos 7.4 with amdgpu-pro 17.50, no problem -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104185] CS:GO randomly locks up the entire system requiring forced reboot
https://bugs.freedesktop.org/show_bug.cgi?id=104185 --- Comment #4 from omin...@autistici.org --- For those who experience a freeze, please try the following: - Switch to a terminal with Ctrl + Alt + F7 - Switch back to your original environment with Ctrl + Alt + F1 I am experience some freezing but I am not entirely sure if it is the same freezing as in this thread. Doing the above trick unfreezes the system. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104185] CS:GO randomly locks up the entire system requiring forced reboot
https://bugs.freedesktop.org/show_bug.cgi?id=104185 --- Comment #5 from quen...@intrepide.io --- all system freeze, can't do anything, so I can't switch tty with Ctrl+Alt+FX(In reply to ominous from comment #4) > For those who experience a freeze, please try the following: > > - Switch to a terminal with Ctrl + Alt + F7 > - Switch back to your original environment with Ctrl + Alt + F1 > > I am experience some freezing but I am not entirely sure if it is the same > freezing as in this thread. Doing the above trick unfreezes the system. all system freeze, can't do anything, sound freeze too so I can't switch tty with Ctrl+Alt+FX (I use it too when xserver or gnome freeze/crash) -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/amdgpu: use %pap format string for phys_addr_t
This commit is Reviewed-by: Felix Kuehling Thanks, Felix On 2018-01-08 07:53 AM, Arnd Bergmann wrote: > The newly added get_local_mem_info() function prints a phys_addr_t > using 0x%llx, which is wrong on most 32-bit systems, as shown by > this warning: > > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c: In function 'get_local_mem_info': > include/linux/kern_levels.h:5:18: error: format '%llx' expects argument of > type 'long long unsigned int', but argument 2 has type 'resource_size_t {aka > unsigned int}' [-Werror=format=] > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c:297:31: note: format string is > defined here > pr_debug("Address base: 0x%llx limit 0x%llx public 0x%llx private 0x%llx\n", > > Passing the address by reference to the special %pap format string will > produce the correct output and avoid the warning. > > Fixes: 30f1c0421ec5 ("drm/amdgpu: Implement get_local_mem_info") > Signed-off-by: Arnd Bergmann > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > index 335e454e2ee1..1d605e1c1d66 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.c > @@ -294,8 +294,8 @@ void get_local_mem_info(struct kgd_dev *kgd, > } > mem_info->vram_width = adev->mc.vram_width; > > - pr_debug("Address base: 0x%llx limit 0x%llx public 0x%llx private > 0x%llx\n", > - adev->mc.aper_base, aper_limit, > + pr_debug("Address base: %pap limit %pap public 0x%llx private 0x%llx\n", > + &adev->mc.aper_base, &aper_limit, > mem_info->local_mem_size_public, > mem_info->local_mem_size_private); > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 1/5] drm/dp: Add DP DSC DPCD receiver capability size define and missing SHIFT
On Thu, Jan 04, 2018 at 12:23:16AM -0800, Manasi Navare wrote: > This patch defines the DP DSC receiver capability size that gives > total number of DP DSC DPCD registers. > This also adds a missing SHIFT define missed in the > commit id (ab6a46ea6842ce "Add DPCD definitions for DP 1.4 DSC feature") > > v2: > * Missed the SHIFT define that I mentioned in the message > > Cc: Jani Nikula > Cc: Ville Syrjala > Cc: Anusha Srivatsa > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Manasi Navare Checked with the spec. Looks good to me. Reviewed-by: Anusha Srivatsa > --- > include/drm/drm_dp_helper.h | 4 > 1 file changed, 4 insertions(+) > > diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h > index da58a42..06e41b2 100644 > --- a/include/drm/drm_dp_helper.h > +++ b/include/drm/drm_dp_helper.h > @@ -223,6 +223,8 @@ > #define DP_DSC_MAX_BITS_PER_PIXEL_LOW 0x067 /* eDP 1.4 */ > > #define DP_DSC_MAX_BITS_PER_PIXEL_HI0x068 /* eDP 1.4 */ > +# define DP_DSC_MAX_BITS_PER_PIXEL_HI_MASK (0x3 << 0) > +# define DP_DSC_MAX_BITS_PER_PIXEL_HI_SHIFT 8 > > #define DP_DSC_DEC_COLOR_FORMAT_CAP 0x069 > # define DP_DSC_RGB (1 << 0) > @@ -271,6 +273,7 @@ > # define DP_DSC_THROUGHPUT_MODE_1_1000 (14 << 4) > > #define DP_DSC_MAX_SLICE_WIDTH 0x06C > +#define DP_DSC_MAX_SLICE_WIDTH_VALUE2560 > > #define DP_DSC_SLICE_CAP_2 0x06D > # define DP_DSC_16_PER_DP_DSC_SINK (1 << 0) > @@ -894,6 +897,7 @@ u8 drm_dp_get_adjust_request_pre_emphasis(const u8 > link_status[DP_LINK_STATUS_SI > > #define DP_BRANCH_OUI_HEADER_SIZE0xc > #define DP_RECEIVER_CAP_SIZE 0xf > +#define DP_DSC_RECEIVER_CAP_SIZE0xf > #define EDP_PSR_RECEIVER_CAP_SIZE2 > #define EDP_DISPLAY_CTL_CAP_SIZE 3 > > -- > 2.7.4 > -- Anusha Srivatsa ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104306] Mesa 17.3 breaks Firefox and other Xwayland apps on AMD HD7750
https://bugs.freedesktop.org/show_bug.cgi?id=104306 --- Comment #12 from eric vz --- Subsequent bisects (starting at different ranges so different commits would be tested) have led me to the same place. No idea why I thought 255573996 worked for a while; all new builds from it break for me. I have Pacman packages for every build I've made if anyone would find them useful. I narrowed the problem to the final return statement by replacing it with "return true;" which resulted in a working build. I also tried to rule out side-effects from the call by adding "bind = screen->check_resource_capability(screen, image->texture, bind) ? 1 : 0;" before the return true so the method would be called but its return value ignored, assuming the compiler is not smart enough to optimize that out. That build worked too. I assume that capability check must sometimes be returning false in some unexpected situations, and something is not handling that. I tried making a debug build of mesa-demos, then gdb glxinfo and set a breakpoint on dri2_validate_usage, but that method appears not to be called before the process hangs. I assume something during system startup calls it, caches the result, and leads to these issues downstream? If anyone has any guidance on next steps, I'd appreciate it. -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 3/6] drm/tinydrm/mipi-dbi: Add mipi_dbi_enable_flush()
On 01/07/2018 11:44 AM, Noralf Trønnes wrote: Add and use a function for enabling, flushing and turning on backlight. Signed-off-by: Noralf Trønnes --- Reviewed-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 4/6] drm/tinydrm/mipi-dbi: Add poweron-reset functions
On 01/07/2018 11:44 AM, Noralf Trønnes wrote: Split out common poweron-reset functionality. Signed-off-by: Noralf Trønnes --- drivers/gpu/drm/tinydrm/mi0283qt.c | 20 ++-- drivers/gpu/drm/tinydrm/mipi-dbi.c | 63 ++ drivers/gpu/drm/tinydrm/st7586.c | 9 ++ drivers/gpu/drm/tinydrm/st7735r.c | 8 ++--- include/drm/tinydrm/mipi-dbi.h | 2 ++ 5 files changed, 73 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index c69a4d958f24..2a78bcd35045 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -49,31 +49,17 @@ static int mi0283qt_init(struct mipi_dbi *mipi) { - struct tinydrm_device *tdev = &mipi->tinydrm; - struct device *dev = tdev->drm->dev; u8 addr_mode; int ret; DRM_DEBUG_KMS("\n"); - ret = regulator_enable(mipi->regulator); - if (ret) { - DRM_DEV_ERROR(dev, "Failed to enable regulator %d\n", ret); + ret = mipi_dbi_poweron_conditional_reset(mipi); + if (ret < 0) return ret; - } - - /* Avoid flicker by skipping setup if the bootloader has done it */ - if (mipi_dbi_display_is_on(mipi)) + if (ret > 0) return 0; If I am reading the patch right, it looks like there are two if (ret > 0) return 0; in a row with nothing in between when this is applied. - mipi_dbi_hw_reset(mipi); - ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET); - if (ret) { - DRM_DEV_ERROR(dev, "Error sending command %d\n", ret); - regulator_disable(mipi->regulator); - return ret; - } - msleep(20); mipi_dbi_command(mipi, MIPI_DCS_SET_DISPLAY_OFF); diff --git a/drivers/gpu/drm/tinydrm/mipi-dbi.c b/drivers/gpu/drm/tinydrm/mipi-dbi.c index ecc5c81a93ac..eea6803ff223 100644 --- a/drivers/gpu/drm/tinydrm/mipi-dbi.c +++ b/drivers/gpu/drm/tinydrm/mipi-dbi.c @@ -463,6 +463,7 @@ bool mipi_dbi_display_is_on(struct mipi_dbi *mipi) val &= ~DCS_POWER_MODE_RESERVED_MASK; + /* The poweron/reset value is 08h DCS_POWER_MODE_DISPLAY_NORMAL_MODE */ if (val != (DCS_POWER_MODE_DISPLAY | DCS_POWER_MODE_DISPLAY_NORMAL_MODE | DCS_POWER_MODE_SLEEP_MODE)) return false; @@ -473,6 +474,68 @@ bool mipi_dbi_display_is_on(struct mipi_dbi *mipi) } EXPORT_SYMBOL(mipi_dbi_display_is_on); +static int mipi_dbi_por_conditional(struct mipi_dbi *mipi, bool cond) I know it is long, but it would be nice to spell out poweron_reset here instead of "por". +{ + struct device *dev = mipi->tinydrm.drm->dev; + int ret; + + if (mipi->regulator) { + ret = regulator_enable(mipi->regulator); + if (ret) { + DRM_DEV_ERROR(dev, "Failed to enable regulator (%d)\n", ret); + return ret; + } + } + + if (cond && mipi_dbi_display_is_on(mipi)) + return 1; + + mipi_dbi_hw_reset(mipi); + ret = mipi_dbi_command(mipi, MIPI_DCS_SOFT_RESET); It seems unnecessary to do a soft reset after a hard reset, but I suppose some displays don't have a hard reset and the extra soft reset shouldn't hurt anything. + if (ret) { + DRM_DEV_ERROR(dev, "Failed to send reset command (%d)\n", ret); + if (mipi->regulator) + regulator_disable(mipi->regulator); + return ret; + } + + return 0; +} + +/** + * mipi_dbi_poweron_reset - MIPI DBI poweron and reset + * @mipi: MIPI DBI structure + * + * This function enables the regulator if used and does a hardware and software + * reset. + * + * Returns: + * Zero on success, or a negative error code. + */ +int mipi_dbi_poweron_reset(struct mipi_dbi *mipi) +{ + return mipi_dbi_por_conditional(mipi, false); +} +EXPORT_SYMBOL(mipi_dbi_poweron_reset); + +/** + * mipi_dbi_poweron_conditional_reset - MIPI DBI poweron and conditional reset + * @mipi: MIPI DBI structure + * + * This function enables the regulator if used and if the display is off, it + * does a hardware and software reset. If mipi_dbi_display_is_on() determines + * that the display is on, no reset is performed. + * + * Returns: + * Zero if the controller was reset, 1 if the display was already on, or a + * negative error code. + */ +int mipi_dbi_poweron_conditional_reset(struct mipi_dbi *mipi) +{ + return mipi_dbi_por_conditional(mipi, true); +} +EXPORT_SYMBOL(mipi_dbi_poweron_conditional_reset); + #if IS_ENABLED(CONFIG_SPI) /** diff --git a/drivers/gpu/drm/tinydrm/st7586.c b/drivers/gpu/drm/tinydrm/st7586.c index 9fd4423c8e70..a6396ef9cc4a 100644 --- a/drivers/gpu/drm/tinydrm/st7586.c +++ b/drivers/gpu/drm/tinydrm/st7586.c @@ -179,19 +179,16 @@ static void st7586_pipe_enable(struct drm_simple_display_pipe *pipe, {
Re: [PATCH v2 4/6] drm/tinydrm/mipi-dbi: Add poweron-reset functions
On 01/08/2018 07:38 PM, David Lechner wrote: On 01/07/2018 11:44 AM, Noralf Trønnes wrote: Split out common poweron-reset functionality. Signed-off-by: Noralf Trønnes --- drivers/gpu/drm/tinydrm/mi0283qt.c | 20 ++-- drivers/gpu/drm/tinydrm/mipi-dbi.c | 63 ++ drivers/gpu/drm/tinydrm/st7586.c | 9 ++ drivers/gpu/drm/tinydrm/st7735r.c | 8 ++--- include/drm/tinydrm/mipi-dbi.h | 2 ++ 5 files changed, 73 insertions(+), 29 deletions(-) diff --git a/drivers/gpu/drm/tinydrm/mi0283qt.c b/drivers/gpu/drm/tinydrm/mi0283qt.c index c69a4d958f24..2a78bcd35045 100644 --- a/drivers/gpu/drm/tinydrm/mi0283qt.c +++ b/drivers/gpu/drm/tinydrm/mi0283qt.c @@ -49,31 +49,17 @@ static int mi0283qt_init(struct mipi_dbi *mipi) { - struct tinydrm_device *tdev = &mipi->tinydrm; - struct device *dev = tdev->drm->dev; u8 addr_mode; int ret; DRM_DEBUG_KMS("\n"); - ret = regulator_enable(mipi->regulator); - if (ret) { - DRM_DEV_ERROR(dev, "Failed to enable regulator %d\n", ret); + ret = mipi_dbi_poweron_conditional_reset(mipi); + if (ret < 0) return ret; - } - - /* Avoid flicker by skipping setup if the bootloader has done it */ - if (mipi_dbi_display_is_on(mipi)) + if (ret > 0) return 0; If I am reading the patch right, it looks like there are two if (ret > 0) return 0; in a row with nothing in between when this is applied. I see now that I missed < vs. >. Probably better to say (ret == 1) instead of (ret > 0). ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 5/6] drm/tinydrm/mi0283qt: Let the display pipe handle power
On 01/07/2018 11:44 AM, Noralf Trønnes wrote: It's better to leave power handling and controller init to the modesetting machinery using the simple pipe .enable and .disable callbacks. Remove unused mipi_dbi_pipe_enable(). Signed-off-by: Noralf Trønnes --- Reviewed-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 6/6] drm/tinydrm: Embed the mode in tinydrm_connector
On 01/07/2018 11:44 AM, Noralf Trønnes wrote: Embed the mode in tinydrm_connector instead of doing an devm_ allocation. Remove unnecessary use of ret variable at the end of tinydrm_display_pipe_init(). Signed-off-by: Noralf Trønnes --- Reviewed-by: David Lechner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/radeon: fill in rb backend map on evergreen/ni.
From: Dave Airlie This looks to have never gotten filled in, and it seems to trigger a bug in mesa. Reported-by: Roland Scheidegger Signed-off-by: Dave Airlie --- drivers/gpu/drm/radeon/evergreen.c | 1 + drivers/gpu/drm/radeon/ni.c| 1 + 2 files changed, 2 insertions(+) diff --git a/drivers/gpu/drm/radeon/evergreen.c b/drivers/gpu/drm/radeon/evergreen.c index 24fe66c..5712d63 100644 --- a/drivers/gpu/drm/radeon/evergreen.c +++ b/drivers/gpu/drm/radeon/evergreen.c @@ -3513,6 +3513,7 @@ static void evergreen_gpu_init(struct radeon_device *rdev) tmp = r6xx_remap_render_backend(rdev, tmp, rdev->config.evergreen.max_backends, EVERGREEN_MAX_BACKENDS, disabled_rb_mask); } + rdev->config.evergreen.backend_map = tmp; WREG32(GB_BACKEND_MAP, tmp); WREG32(CGTS_SYS_TCC_DISABLE, 0); diff --git a/drivers/gpu/drm/radeon/ni.c b/drivers/gpu/drm/radeon/ni.c index 9eccd0c..381b0255 100644 --- a/drivers/gpu/drm/radeon/ni.c +++ b/drivers/gpu/drm/radeon/ni.c @@ -1148,6 +1148,7 @@ static void cayman_gpu_init(struct radeon_device *rdev) rdev->config.cayman.max_shader_engines, CAYMAN_MAX_BACKENDS, disabled_rb_mask); } + rdev->config.cayman.backend_map = tmp; WREG32(GB_BACKEND_MAP, tmp); cgts_tcc_disable = 0x; -- 2.9.5 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 01/10] dt-bindings: display: xlnx: Add Xilinx kms bindings
On Thu, Jan 04, 2018 at 06:05:50PM -0800, Hyun Kwon wrote: > The dt binding for Xilinx DRM KMS driver. Bindings are for h/w, not drivers. > > Signed-off-by: Hyun Kwon > --- > .../devicetree/bindings/display/xlnx/xlnx,kms.txt| 20 > > 1 file changed, 20 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/xlnx/xlnx,kms.txt > > diff --git a/Documentation/devicetree/bindings/display/xlnx/xlnx,kms.txt > b/Documentation/devicetree/bindings/display/xlnx/xlnx,kms.txt > new file mode 100644 > index 000..8dcd552 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,kms.txt > @@ -0,0 +1,20 @@ > +Xilinx KMS Pipeline > +--- > + > +Xilinx display pipelines can be designed with hardened video IPs and soft > video > +IPs in programmable logic. This KMS module provides the common functionality > +of individual subdevice drivers, and glue logics between them. > + > +Required properties: > + > +- compatible: Must be "xlnx,kms". > + > +- ports: phandles for CRTC ports, using the DT bindings defined in > + Documentation/devicetree/bindings/graph.txt. This use of ports is not part of the graph binding. > + > +Example: > + > + xlnx_drm: xlnx_drm { > + compatible = "xlnx,kms"; drm and kms are Linuxisms. Why do you need this node? > + ports = <&crtc_port>; > + }; > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 06/10] dt-bindings: display: xlnx: Add ZynqMP DP subsystem bindings
On Thu, Jan 04, 2018 at 06:05:55PM -0800, Hyun Kwon wrote: > This add a dt binding for ZynqMP DP subsystem. > > Signed-off-by: Hyun Kwon > --- > .../bindings/display/xlnx/xlnx,zynqmp-dpsub.txt| 94 > ++ > 1 file changed, 94 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt > > diff --git > a/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt > b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt > new file mode 100644 > index 000..4e478ca > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/xlnx/xlnx,zynqmp-dpsub.txt > @@ -0,0 +1,94 @@ > +Xilinx ZynqMP DisplayPort subsystem > +--- > + > +Required properties: > + > +- compatible: Must be "xlnx,zynqmp-dpsub-1.7". > + > +- reg: Physical base address and length of the registers set for the device. > +- reg-names: Must be "dp", "blend", "av_buf", and "aud" to map logical > register > + partitions. > + > +- interrupts: Interrupt number. > +- interrupts-parent: phandle for interrupt controller. > + > +- clocks: phandles for axi, audio, non-live video, and live video clocks. > + axi clock is required. Audio clock is optional. If not present, audio will > + be disabled. One of non-live or live video clock should be present. > +- clock-names: The identification strings are required. "aclk" for axi clock. > + "dp_aud_clk" for audio clock. "dp_vtc_pixel_clk_in" for non-live video > clock. > + "dp_live_video_in_clk" for live video clock (clock from programmable > logic). "_clk" is redundant. > + > +- phys: phandles for phy specifier. > +- phy-names: The identifier strings. "dp-phy" followed by index. > + > +- power-domains: phandle for the corresponding power domain > + > +- ports: crtc and encoder ports are required using DT bindings defined in > + Documentation/devicetree/bindings/graph.txt. Isn't the connection crtc->encoder? Also, crtc is pretty much a DRM term. Don't use that in bindings. Describe the h/w blocks. > + > +- vid-layer, gfx-layer: Required to represent available layers > + > +Required layer properties > + > +- dmas: phandles for DMA channels as defined in > + Documentation/devicetree/bindings/dma/dma.txt. > +- dma-names: The identifier strings are required. "graphics0" for graphics > + layer. "video" followed by index for video layer > + > +Optional child node > + > +- The driver populates any child device node in this node. This can be used, > + for example, to populate the sound device from the DisplayPort subsystem > + driver. > + > +Example: > + zynqmp_dpsub: zynqmp_dpsub@fd4a { display-controller@... > + compatible = "xlnx,zynqmp-dpsub-1.7"; > + reg = <0x0 0xfd4a 0x0 0x1000>, > + <0x0 0xfd4aa000 0x0 0x1000>, > + <0x0 0xfd4ab000 0x0 0x1000>, > + <0x0 0xfd4ac000 0x0 0x1000>; > + reg-names = "dp", "blend", "av_buf", "aud"; > + interrupts = <0 119 4>; > + interrupt-parent = <&gic>; > + > + clock-names = "dp_apb_clk", "dp_aud_clk", > "dp_live_video_in_clk"; > + clocks = <&dp_aclk>, <&clkc 17>, <&si570_1>; > + > + phys = <&lane1 PHY_TYPE_DP 0 1 2700>, > +<&lane0 PHY_TYPE_DP 1 1 2700>; > + phy-names = "dp-phy0", "dp-phy1"; > + > + power-domains = <&pd_dp>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + vid-layer { > + dma-names = "vid0", "vid1", "vid2"; > + dmas = <&xlnx_dpdma 0>, > +<&xlnx_dpdma 1>, > +<&xlnx_dpdma 2>; > + }; > + > + gfx-layer { These 2 nodes don't seem necessary. Just make "dmas" take 4 values and define where the gfx0 channel is located (index 0 or 3?). > + dma-names = "gfx0"; > + dmas = <&xlnx_dpdma 3>; > + }; > + > + crtc_port: port@0 { > + reg = <0>; > + crtc: endpoint { > + remote-endpoint = <&encoder>; > + }; > + }; > + port@1 { Multiple port nodes should be under a ports node especially if you have other child nodes. > + reg = <1>; > + encoder: endpoint { > + remote-endpoint = <&crtc>; > + }; > + }; > + }; > +}; > + > -- > 2.7.4 > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/