Re: [PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
On Fri, Apr 6, 2018 at 7:51 PM, Keith Packardwrote: > Jason Ekstrand writes: > > > Is the given sequence number guaranteed to be hit in finite time? > > Certainly, it's a finite value... > > However, realistically, it's just like all of the other vblank > interfaces where you can specify a crazy sequence and block for a very > long time. So, no different from the current situation. > > Of course, the only vulkan API available today only lets you wait for > the next vblank, so we'll be asking for a sequence which is one more > than the current sequence. > > > Just to make sure I've got this straight, the client queues a syncobj > with > > queue_syncobj to fire at a given sequence number. Once the syncobj has > > fired (which it can find out by waiting on it), it then calls get_syncobj > > to figure out when it was fired? > > If it cares, it can ask. It might not care at all, in which case it > wouldn't have to ask. The syncobj API doesn't provide any direct > information about the state of the object in the wait call. > Yeah, I suppose an application could ask for 10k frames in the future or something ridiculous like that. For sync_file, people strongly want a finite time guarantee for security/deadlock reasons. I don't know how happy they would be with a finite time of 10 minutes... > > If so, what happens if a syncobj is re-used? Do you just loose the > > information? > > You can't reuse one of these -- the 'queue_syncobj' API creates a new > one each time. > Ok, that's not expected... Part of the point of sync objects is that they're re-usable. The client is free to not re-use them or to be very careful about the recycling process. > > If we have a finite time guarantee, I'm kind-of thinking a sync_file > might > > be better as it's a one-shot and doesn't have the information loss > > problem. I'm not actually sure though. > > It's a one-shot, once signaled, you're done with it. > > > This whole "wait for a syncobj and then ask when it fired" thing is a bit > > odd. I'll have to think on it. > > Yeah, the event mechanism has the nice feature that you get data with > each event. I guess the alternative would be to have a way to get an > event when a sync object was ready; perhaps a call which provided a set > of syncobjs and delivered a DRM event when any of them was ready? > > That has a lot of appeal; it turns the poll-like nature of the current > API into an epoll-like system. Hrm. > Is the event the important part or the moderately accurate timestamp? We could probably modify drm_syncobj to have a "last signaled" timestamp that's queryable through an IOCTL. Is the sequence number returned necessary? Will it ever be different from the one requested? Sorry, lots of questions. KMS isn't a domain about which I know a great deal. --Jason ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 104391] DC R9 285 HDMI audio regression since drm/amd/display: try to find matching audio inst for enc inst first
https://bugs.freedesktop.org/show_bug.cgi?id=104391 Charlenechanged: What|Removed |Added Status|NEW |ASSIGNED --- Comment #2 from Charlene --- This issue is related to AFMT_CNTL's AFMT_AUDIO_CLOCK_EN. sequence issue. fixed. -- 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: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
Jason Ekstrandwrites: > Is the given sequence number guaranteed to be hit in finite time? Certainly, it's a finite value... However, realistically, it's just like all of the other vblank interfaces where you can specify a crazy sequence and block for a very long time. So, no different from the current situation. Of course, the only vulkan API available today only lets you wait for the next vblank, so we'll be asking for a sequence which is one more than the current sequence. > Just to make sure I've got this straight, the client queues a syncobj with > queue_syncobj to fire at a given sequence number. Once the syncobj has > fired (which it can find out by waiting on it), it then calls get_syncobj > to figure out when it was fired? If it cares, it can ask. It might not care at all, in which case it wouldn't have to ask. The syncobj API doesn't provide any direct information about the state of the object in the wait call. > If so, what happens if a syncobj is re-used? Do you just loose the > information? You can't reuse one of these -- the 'queue_syncobj' API creates a new one each time. > If we have a finite time guarantee, I'm kind-of thinking a sync_file might > be better as it's a one-shot and doesn't have the information loss > problem. I'm not actually sure though. It's a one-shot, once signaled, you're done with it. > This whole "wait for a syncobj and then ask when it fired" thing is a bit > odd. I'll have to think on it. Yeah, the event mechanism has the nice feature that you get data with each event. I guess the alternative would be to have a way to get an event when a sync object was ready; perhaps a call which provided a set of syncobjs and delivered a DRM event when any of them was ready? That has a lot of appeal; it turns the poll-like nature of the current API into an epoll-like system. Hrm. -- -keith signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v5] drm/i915/dp: Send DPCD ON for MST before phy_up
When doing a modeset where the sink is transitioning from D3 to D0 , it would sometimes be possible for the initial power_up_phy() to start timing out. This would only be observed in the last action before the sink went into D3 mode was intel_dp_sink_dpms(DRM_MODE_DPMS_OFF). We originally thought this might be an issue with us accidentally shutting off the aux block when putting the sink into D3, but since the DP spec mandates that sinks must wake up within 1ms while we have 100ms to respond to an ESI irq, this didn't really add up. Turns out that the problem is more subtle then that: It turns out that the timeout is from us not enabling DPMS on the MST hub before actually trying to initiate sideband communications. This would cause the first sideband communication (power_up_phy()), to start timing out because the sink wasn't ready to respond. Afterwards, we would call intel_dp_sink_dpms(DRM_MODE_DPMS_ON) in intel_ddi_pre_enable_dp(), which would actually result in waking up the sink so that sideband requests would work again. Since DPMS is what lets us actually bring the hub up into a state where sideband communications become functional again, we just need to make sure to enable DPMS on the display before attempting to perform sideband communications. Changes since v1: - Remove comment above if (!intel_dp->is_mst) - vsryjala - Move intel_dp_sink_dpms() for MST into intel_dp_post_disable_mst() to keep enable/disable paths symmetrical - Improve commit message - dhnkrn Changes since v2: - Only send DPMS off when we're disabling the last sink, and only send DPMS on when we're enabling the first sink - dhnkrn Changes since v3: - Check against is_mst, not intel_dp->is_mst - dhnkrn/vsyrjala Signed-off-by: Lyude PaulReviewed-by: Dhinakaran Pandiyan Reviewed-by: Ville Syrjälä Tested-by: Laura Abbott Cc: sta...@vger.kernel.org Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.") --- No actual changes other than t-b and r-bs. Resending because I don't have access to the "test latest revision again" button and I'm very much sure these CI results are bogus. drivers/gpu/drm/i915/intel_ddi.c| 8 ++-- drivers/gpu/drm/i915/intel_dp_mst.c | 8 +++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index a6672a9abd85..92cb26b18a9b 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2324,7 +2324,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, intel_prepare_dp_ddi_buffers(encoder, crtc_state); intel_ddi_init_dp_buf_reg(encoder); - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + if (!is_mst) + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); intel_dp_start_link_train(intel_dp); if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) intel_dp_stop_link_train(intel_dp); @@ -2422,12 +2423,15 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder, struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_digital_port *dig_port = enc_to_dig_port(>base); struct intel_dp *intel_dp = _port->dp; + bool is_mst = intel_crtc_has_type(old_crtc_state, + INTEL_OUTPUT_DP_MST); /* * Power down sink before disabling the port, otherwise we end * up getting interrupts from the sink on detecting link loss. */ - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); + if (!is_mst) + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); intel_disable_ddi_buf(encoder); diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index c3de0918ee13..9e6956c08688 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -180,9 +180,11 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder, intel_dp->active_mst_links--; intel_mst->connector = NULL; - if (intel_dp->active_mst_links == 0) + if (intel_dp->active_mst_links == 0) { + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); intel_dig_port->base.post_disable(_dig_port->base, old_crtc_state, NULL); + } DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links); } @@ -223,7 +225,11 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder, DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links); + if (intel_dp->active_mst_links == 0) + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + drm_dp_send_power_updown_phy(_dp->mst_mgr, connector->port, true); + if (intel_dp->active_mst_links == 0)
Re: [PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
On Fri, Apr 6, 2018 at 4:56 PM, Keith Packardwrote: > crtc_queue_syncobj creates a new syncobj that will get signaled at a > specified vblank sequence count. > > crtc_get_syncobj returns the time and vblank sequence count when the > syncobj was signaled. > > The pair of these allows use of syncobjs instead of events for > monitoring vblank activity. > > Signed-off-by: Keith Packard > --- > drivers/gpu/drm/drm_file.c | 5 + > drivers/gpu/drm/drm_internal.h | 4 + > drivers/gpu/drm/drm_ioctl.c| 2 + > drivers/gpu/drm/drm_syncobj.c | 13 ++- > drivers/gpu/drm/drm_vblank.c | 212 ++ > +++ > include/drm/drm_file.h | 11 ++- > include/drm/drm_syncobj.h | 13 +++ > include/uapi/drm/drm.h | 17 > 8 files changed, 253 insertions(+), 24 deletions(-) > [...] > diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h > index 6fdff5945c8a..7a996f73e972 100644 > --- a/include/uapi/drm/drm.h > +++ b/include/uapi/drm/drm.h > @@ -759,6 +759,21 @@ struct drm_crtc_queue_sequence { > __u64 user_data;/* user data passed to event */ > }; > > +struct drm_crtc_queue_syncobj { > + __u32 crtc_id; > + __u32 flags; > + __u64 sequence; > Is the given sequence number guaranteed to be hit in finite time? > + __u32 handle; /* return syncobj handle */ > + __u32 pad; > +}; > + > +struct drm_crtc_get_syncobj { > + __u32 handle; /* signaled syncobj */ > + __u32 pad; > + __u64 sequence; /* return: sequence when syncobj was > signaled */ > + __u64 sequence_ns; /* return: time when syncobj was signaled > */ > Just to make sure I've got this straight, the client queues a syncobj with queue_syncobj to fire at a given sequence number. Once the syncobj has fired (which it can find out by waiting on it), it then calls get_syncobj to figure out when it was fired? If so, what happens if a syncobj is re-used? Do you just loose the information? If we have a finite time guarantee, I'm kind-of thinking a sync_file might be better as it's a one-shot and doesn't have the information loss problem. I'm not actually sure though. This whole "wait for a syncobj and then ask when it fired" thing is a bit odd. I'll have to think on it. --Jason > +}; > + > #if defined(__cplusplus) > } > #endif > @@ -843,6 +858,8 @@ extern "C" { > > #define DRM_IOCTL_CRTC_GET_SEQUENCEDRM_IOWR(0x3b, struct > drm_crtc_get_sequence) > #define DRM_IOCTL_CRTC_QUEUE_SEQUENCE DRM_IOWR(0x3c, struct > drm_crtc_queue_sequence) > +#define DRM_IOCTL_CRTC_QUEUE_SYNCOBJ DRM_IOWR(0x3d, struct > drm_crtc_queue_syncobj) > +#define DRM_IOCTL_CRTC_GET_SYNCOBJ DRM_IOWR(0x3e, struct > drm_crtc_get_syncobj) > > #define DRM_IOCTL_UPDATE_DRAW DRM_IOW(0x3f, struct > drm_update_draw) > > -- > 2.16.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 0/1] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
(This is an RFC on whether this pair of ioctls seems reasonable. The code compiles, but I haven't tested it as I'm away from home this weekend.) I'm rewriting my implementation of the Vulkan EXT_display_control extension, which provides a way to signal a Vulkan fence at vblank time. I had implemented it using events, but that isn't great as the Vulkan API includes the ability to wait for any of a set of fences to be signaled. As the other Vulkan fences are implemented using dma_fences in the kernel, and (eventually) using syncobj up in user space, it seems reasonable to use syncobjs for everything and hook vblank to those. In any case, I'm proposing two new syncobj/vblank ioctls (the names aren't great; suggestions welcome, as usual): DRM_IOCTL_CRTC_QUEUE_SYNCOBJ Create a new syncobj that will be signaled at (or after) the specified vblank sequence value. This uses the same parameters to specify the target sequence as DRM_IOCTL_CRTC_QUEUE_SEQUENCE. DRM_IOCTL_CRTC_GET_SYNCOBJ Once the above syncobj has been signaled, this ioctl allows the application to find out when that happened, returning both the vblank sequence count and time (in ns). I'd like to hear comments on whether this seems reasonable, or whether I should go in some other direction. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: Add crtc_queue_syncobj and crtc_get_syncobj ioctls
crtc_queue_syncobj creates a new syncobj that will get signaled at a specified vblank sequence count. crtc_get_syncobj returns the time and vblank sequence count when the syncobj was signaled. The pair of these allows use of syncobjs instead of events for monitoring vblank activity. Signed-off-by: Keith Packard--- drivers/gpu/drm/drm_file.c | 5 + drivers/gpu/drm/drm_internal.h | 4 + drivers/gpu/drm/drm_ioctl.c| 2 + drivers/gpu/drm/drm_syncobj.c | 13 ++- drivers/gpu/drm/drm_vblank.c | 212 + include/drm/drm_file.h | 11 ++- include/drm/drm_syncobj.h | 13 +++ include/uapi/drm/drm.h | 17 8 files changed, 253 insertions(+), 24 deletions(-) diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c index b3c6e997ccdb..c1ada3fe70b0 100644 --- a/drivers/gpu/drm/drm_file.c +++ b/drivers/gpu/drm/drm_file.c @@ -37,6 +37,7 @@ #include #include +#include #include "drm_legacy.h" #include "drm_internal.h" @@ -711,6 +712,10 @@ void drm_send_event_locked(struct drm_device *dev, struct drm_pending_event *e) dma_fence_put(e->fence); } + if (e->syncobj) { + drm_syncobj_put(e->syncobj); + } + if (!e->file_priv) { kfree(e); return; diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h index c9d5a6cd4d41..71b9435b5b37 100644 --- a/drivers/gpu/drm/drm_internal.h +++ b/drivers/gpu/drm/drm_internal.h @@ -75,6 +75,10 @@ int drm_crtc_get_sequence_ioctl(struct drm_device *dev, void *data, int drm_crtc_queue_sequence_ioctl(struct drm_device *dev, void *data, struct drm_file *filp); +int drm_crtc_queue_syncobj_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); +int drm_crtc_get_syncobj_ioctl(struct drm_device *dev, void *data, +struct drm_file *filp); /* drm_auth.c */ int drm_getmagic(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index 4aafe4802099..309611fb5d0d 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -665,6 +665,8 @@ static const struct drm_ioctl_desc drm_ioctls[] = { DRM_UNLOCKED|DRM_RENDER_ALLOW), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SEQUENCE, drm_crtc_get_sequence_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SEQUENCE, drm_crtc_queue_sequence_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_QUEUE_SYNCOBJ, drm_crtc_queue_syncobj_ioctl, DRM_UNLOCKED), + DRM_IOCTL_DEF(DRM_IOCTL_CRTC_GET_SYNCOBJ, drm_crtc_get_syncobj_ioctl, DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_CREATE_LEASE, drm_mode_create_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_LIST_LESSEES, drm_mode_list_lessees_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GET_LEASE, drm_mode_get_lease_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED), diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index cb4d09c70fd4..e197b007079d 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -208,7 +208,7 @@ static const struct dma_fence_ops drm_syncobj_null_fence_ops = { .release = NULL, }; -static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) +int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj, bool signal) { struct drm_syncobj_null_fence *fence; fence = kzalloc(sizeof(*fence), GFP_KERNEL); @@ -218,7 +218,8 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) spin_lock_init(>lock); dma_fence_init(>base, _syncobj_null_fence_ops, >lock, 0, 0); - dma_fence_signal(>base); + if (signal) + dma_fence_signal(>base); drm_syncobj_replace_fence(syncobj, >base); @@ -226,6 +227,7 @@ static int drm_syncobj_assign_null_handle(struct drm_syncobj *syncobj) return 0; } +EXPORT_SYMBOL(drm_syncobj_assign_null_handle); int drm_syncobj_find_fence(struct drm_file *file_private, u32 handle, @@ -283,7 +285,7 @@ int drm_syncobj_create(struct drm_syncobj **out_syncobj, uint32_t flags, spin_lock_init(>lock); if (flags & DRM_SYNCOBJ_CREATE_SIGNALED) { - ret = drm_syncobj_assign_null_handle(syncobj); + ret = drm_syncobj_assign_null_handle(syncobj, true); if (ret < 0) { drm_syncobj_put(syncobj); return ret; @@ -341,7 +343,7 @@ static int drm_syncobj_create_as_handle(struct drm_file *file_private, return ret; } -static int drm_syncobj_destroy(struct drm_file *file_private, +int drm_syncobj_destroy(struct drm_file
[Bug 105934] Gpu Hang after two compute dispatches on Intel HD 5500
https://bugs.freedesktop.org/show_bug.cgi?id=105934 Bug ID: 105934 Summary: Gpu Hang after two compute dispatches on Intel HD 5500 Product: Mesa Version: unspecified Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: major Priority: medium Component: Drivers/DRI/i915 Assignee: dri-devel@lists.freedesktop.org Reporter: mank...@kolumbus.fi QA Contact: dri-devel@lists.freedesktop.org Running OpenGL program with two glDispatch() calls, causes gpu hang on HD Graphics 5500. Program basically does: if (1) { glUseProgram(computeprogram); glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, ssbo_test); glDispatchCompute(256/4, 256/4, 256/2); glMemoryBarrier(GL_SHADER_STORAGE_BARRIER_BIT); } if (1) { glUseProgram(computeprogram); glBindBufferBase(GL_SHADER_STORAGE_BUFFER, 1, ssbo_test); glDispatchCompute(256/4, 256/4, 256/2); glMemoryBarrier(GL_SHADER_STORAGE_BARRIER_BIT); } .. and then simple one-triangle fullscreen pass to main framebuffer (0) using that SSBO. When both of those are enabled, my computer nearly hangs and dmesg reports: [127049.481163] drm/i915: Resetting chip after gpu hang. But if only one of those is enabled, everything is ok, compute dispatch takes 13ms and debug output renders on the screen. -- 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 105869] clang crashes when compiling OpenCL kernel
https://bugs.freedesktop.org/show_bug.cgi?id=105869 --- Comment #10 from Lyberta--- Created attachment 138665 --> https://bugs.freedesktop.org/attachment.cgi?id=138665=edit OpenCL dump.ll -- 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 105869] clang crashes when compiling OpenCL kernel
https://bugs.freedesktop.org/show_bug.cgi?id=105869 --- Comment #9 from Lyberta--- Created attachment 138664 --> https://bugs.freedesktop.org/attachment.cgi?id=138664=edit OpenCL dump.link-0.ll -- 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 105869] clang crashes when compiling OpenCL kernel
https://bugs.freedesktop.org/show_bug.cgi?id=105869 --- Comment #8 from Lyberta--- Created attachment 138663 --> https://bugs.freedesktop.org/attachment.cgi?id=138663=edit OpenCL dump.cl -- 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 v2.1 09/15] v4l: vsp1: Move DRM pipeline output setup code to a function
In order to make the vsp1_du_setup_lif() easier to read, and for symmetry with the DRM pipeline input setup, move the pipeline output setup code to a separate function. Signed-off-by: Laurent PinchartReviewed-by: Kieran Bingham -- Changes since v2: - Moved vsp1_du_pipeline_setup_input() rename to earlier patch Changes since v1: - Rename vsp1_du_pipeline_setup_input() to vsp1_du_pipeline_setup_inputs() - Initialize format local variable to 0 in vsp1_du_pipeline_setup_output() --- drivers/media/platform/vsp1/vsp1_drm.c | 106 +++-- 1 file changed, 60 insertions(+), 46 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 4a628bbf7e47..a79b05ef 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -276,6 +276,65 @@ static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, return 0; } +/* Setup the output side of the pipeline (WPF and LIF). */ +static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1, +struct vsp1_pipeline *pipe) +{ + struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe); + struct v4l2_subdev_format format = { 0, }; + int ret; + + format.which = V4L2_SUBDEV_FORMAT_ACTIVE; + format.pad = RWPF_PAD_SINK; + format.format.width = drm_pipe->width; + format.format.height = drm_pipe->height; + format.format.code = MEDIA_BUS_FMT_ARGB_1X32; + format.format.field = V4L2_FIELD_NONE; + + ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n", + __func__, format.format.width, format.format.height, + format.format.code, pipe->output->entity.index); + + format.pad = RWPF_PAD_SOURCE; + ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n", + __func__, format.format.width, format.format.height, + format.format.code, pipe->output->entity.index); + + format.pad = LIF_PAD_SINK; + ret = v4l2_subdev_call(>lif->subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on LIF%u sink\n", + __func__, format.format.width, format.format.height, + format.format.code, pipe->lif->index); + + /* +* Verify that the format at the output of the pipeline matches the +* requested frame size and media bus code. +*/ + if (format.format.width != drm_pipe->width || + format.format.height != drm_pipe->height || + format.format.code != MEDIA_BUS_FMT_ARGB_1X32) { + dev_dbg(vsp1->dev, "%s: format mismatch on LIF%u\n", __func__, + pipe->lif->index); + return -EPIPE; + } + + return 0; +} + /* Configure all entities in the pipeline. */ static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) { @@ -356,7 +415,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, struct vsp1_drm_pipeline *drm_pipe; struct vsp1_pipeline *pipe; struct vsp1_bru *bru; - struct v4l2_subdev_format format; unsigned long flags; unsigned int i; int ret; @@ -417,54 +475,10 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int pipe_index, if (ret < 0) return ret; - memset(, 0, sizeof(format)); - format.which = V4L2_SUBDEV_FORMAT_ACTIVE; - format.pad = RWPF_PAD_SINK; - format.format.width = cfg->width; - format.format.height = cfg->height; - format.format.code = MEDIA_BUS_FMT_ARGB_1X32; - format.format.field = V4L2_FIELD_NONE; - - ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL, - ); - if (ret < 0) - return ret; - - dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n", - __func__, format.format.width, format.format.height, - format.format.code, pipe->output->entity.index); - - format.pad = RWPF_PAD_SOURCE; - ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL, - ); - if (ret < 0) - return ret; - - dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n", - __func__, format.format.width, format.format.height, - format.format.code,
[PATCH v2.1 06/15] v4l: vsp1: Move DRM atomic commit pipeline setup to separate function
The DRM pipeline setup code used at atomic commit time is similar to the setup code used when enabling the pipeline. Move it to a separate function in order to share it. Signed-off-by: Laurent PinchartReviewed-by: Kieran Bingham --- Changes since v2: - Rename vsp1_du_pipeline_setup_input() to vsp1_du_pipeline_setup_inputs() --- drivers/media/platform/vsp1/vsp1_drm.c | 347 + 1 file changed, 180 insertions(+), 167 deletions(-) diff --git a/drivers/media/platform/vsp1/vsp1_drm.c b/drivers/media/platform/vsp1/vsp1_drm.c index 9a043a915c0b..d99278f45bd8 100644 --- a/drivers/media/platform/vsp1/vsp1_drm.c +++ b/drivers/media/platform/vsp1/vsp1_drm.c @@ -46,6 +46,185 @@ static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe, * Pipeline Configuration */ +/* Setup one RPF and the connected BRU sink pad. */ +static int vsp1_du_pipeline_setup_rpf(struct vsp1_device *vsp1, + struct vsp1_pipeline *pipe, + struct vsp1_rwpf *rpf, + unsigned int bru_input) +{ + struct v4l2_subdev_selection sel; + struct v4l2_subdev_format format; + const struct v4l2_rect *crop; + int ret; + + /* +* Configure the format on the RPF sink pad and propagate it up to the +* BRU sink pad. +*/ + crop = >drm->inputs[rpf->entity.index].crop; + + memset(, 0, sizeof(format)); + format.which = V4L2_SUBDEV_FORMAT_ACTIVE; + format.pad = RWPF_PAD_SINK; + format.format.width = crop->width + crop->left; + format.format.height = crop->height + crop->top; + format.format.code = rpf->fmtinfo->mbus; + format.format.field = V4L2_FIELD_NONE; + + ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, + "%s: set format %ux%u (%x) on RPF%u sink\n", + __func__, format.format.width, format.format.height, + format.format.code, rpf->entity.index); + + memset(, 0, sizeof(sel)); + sel.which = V4L2_SUBDEV_FORMAT_ACTIVE; + sel.pad = RWPF_PAD_SINK; + sel.target = V4L2_SEL_TGT_CROP; + sel.r = *crop; + + ret = v4l2_subdev_call(>entity.subdev, pad, set_selection, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, + "%s: set selection (%u,%u)/%ux%u on RPF%u sink\n", + __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height, + rpf->entity.index); + + /* +* RPF source, hardcode the format to ARGB to turn on format +* conversion if needed. +*/ + format.pad = RWPF_PAD_SOURCE; + + ret = v4l2_subdev_call(>entity.subdev, pad, get_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, + "%s: got format %ux%u (%x) on RPF%u source\n", + __func__, format.format.width, format.format.height, + format.format.code, rpf->entity.index); + + format.format.code = MEDIA_BUS_FMT_ARGB_1X32; + + ret = v4l2_subdev_call(>entity.subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + /* BRU sink, propagate the format from the RPF source. */ + format.pad = bru_input; + + ret = v4l2_subdev_call(>bru->subdev, pad, set_fmt, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on %s pad %u\n", + __func__, format.format.width, format.format.height, + format.format.code, BRU_NAME(pipe->bru), format.pad); + + sel.pad = bru_input; + sel.target = V4L2_SEL_TGT_COMPOSE; + sel.r = vsp1->drm->inputs[rpf->entity.index].compose; + + ret = v4l2_subdev_call(>bru->subdev, pad, set_selection, NULL, + ); + if (ret < 0) + return ret; + + dev_dbg(vsp1->dev, "%s: set selection (%u,%u)/%ux%u on %s pad %u\n", + __func__, sel.r.left, sel.r.top, sel.r.width, sel.r.height, + BRU_NAME(pipe->bru), sel.pad); + + return 0; +} + +static unsigned int rpf_zpos(struct vsp1_device *vsp1, struct vsp1_rwpf *rpf) +{ + return vsp1->drm->inputs[rpf->entity.index].zpos; +} + +/* Setup the input side of the pipeline (RPFs and BRU sink pads). */ +static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, +struct vsp1_pipeline *pipe) +{ + struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, }; + struct vsp1_bru *bru = to_bru(>bru->subdev); + unsigned int
Re: [PATCH v4] drm/i915/dp: Send DPCD ON for MST before phy_up
On Fri, 2018-04-06 at 12:48 -0700, Laura Abbott wrote: > On 04/06/2018 11:52 AM, Lyude Paul wrote: > > When doing a modeset where the sink is transitioning from D3 to D0 , it > > would sometimes be possible for the initial power_up_phy() to start > > timing out. This would only be observed in the last action before the > > sink went into D3 mode was intel_dp_sink_dpms(DRM_MODE_DPMS_OFF). We > > originally thought this might be an issue with us accidentally shutting > > off the aux block when putting the sink into D3, but since the DP spec > > mandates that sinks must wake up within 1ms while we have 100ms to > > respond to an ESI irq, this didn't really add up. Turns out that the > > problem is more subtle then that: > > > > It turns out that the timeout is from us not enabling DPMS on the MST > > hub before actually trying to initiate sideband communications. This > > would cause the first sideband communication (power_up_phy()), to start > > timing out because the sink wasn't ready to respond. Afterwards, we > > would call intel_dp_sink_dpms(DRM_MODE_DPMS_ON) in > > intel_ddi_pre_enable_dp(), which would actually result in waking up the > > sink so that sideband requests would work again. > > > > Since DPMS is what lets us actually bring the hub up into a state where > > sideband communications become functional again, we just need to make > > sure to enable DPMS on the display before attempting to perform sideband > > communications. > > > > Changes since v1: > > - Remove comment above if (!intel_dp->is_mst) - vsryjala > > - Move intel_dp_sink_dpms() for MST into intel_dp_post_disable_mst() to > >keep enable/disable paths symmetrical > > - Improve commit message - dhnkrn > > Changes since v2: > > - Only send DPMS off when we're disabling the last sink, and only send > >DPMS on when we're enabling the first sink - dhnkrn > > Changes since v3: > > - Check against is_mst, not intel_dp->is_mst - dhnkrn/vsyrjala > > > > For the booting docked with lid down case: > > Tested-by: Laura AbbottAwesome, will push once the CI run finishes. Thanks for the help! > > > Signed-off-by: Lyude Paul > > Cc: Dhinakaran Pandiyan > > Cc: Ville Syrjälä > > Cc: Laura Abbott > > Cc: sta...@vger.kernel.org > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST > > hub.") > > --- > > drivers/gpu/drm/i915/intel_ddi.c| 8 ++-- > > drivers/gpu/drm/i915/intel_dp_mst.c | 8 +++- > > 2 files changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index a6672a9abd85..92cb26b18a9b 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -2324,7 +2324,8 @@ static void intel_ddi_pre_enable_dp(struct > > intel_encoder *encoder, > > intel_prepare_dp_ddi_buffers(encoder, crtc_state); > > > > intel_ddi_init_dp_buf_reg(encoder); > > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > + if (!is_mst) > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > intel_dp_start_link_train(intel_dp); > > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > > intel_dp_stop_link_train(intel_dp); > > @@ -2422,12 +2423,15 @@ static void intel_ddi_post_disable_dp(struct > > intel_encoder *encoder, > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > struct intel_digital_port *dig_port = enc_to_dig_port( > > >base); > > struct intel_dp *intel_dp = _port->dp; > > + bool is_mst = intel_crtc_has_type(old_crtc_state, > > + INTEL_OUTPUT_DP_MST); > > > > /* > > * Power down sink before disabling the port, otherwise we end > > * up getting interrupts from the sink on detecting link loss. > > */ > > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > > + if (!is_mst) > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > > > > intel_disable_ddi_buf(encoder); > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c > > b/drivers/gpu/drm/i915/intel_dp_mst.c > > index c3de0918ee13..9e6956c08688 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > > @@ -180,9 +180,11 @@ static void intel_mst_post_disable_dp(struct > > intel_encoder *encoder, > > intel_dp->active_mst_links--; > > > > intel_mst->connector = NULL; > > - if (intel_dp->active_mst_links == 0) > > + if (intel_dp->active_mst_links == 0) { > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > > intel_dig_port->base.post_disable(_dig_port->base, > > old_crtc_state, NULL); > > + } > > > > DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links); > > } > > @@ -223,7 +225,11 @@ static void
Re: [PATCH v4] drm/i915/dp: Send DPCD ON for MST before phy_up
On 04/06/2018 11:52 AM, Lyude Paul wrote: When doing a modeset where the sink is transitioning from D3 to D0 , it would sometimes be possible for the initial power_up_phy() to start timing out. This would only be observed in the last action before the sink went into D3 mode was intel_dp_sink_dpms(DRM_MODE_DPMS_OFF). We originally thought this might be an issue with us accidentally shutting off the aux block when putting the sink into D3, but since the DP spec mandates that sinks must wake up within 1ms while we have 100ms to respond to an ESI irq, this didn't really add up. Turns out that the problem is more subtle then that: It turns out that the timeout is from us not enabling DPMS on the MST hub before actually trying to initiate sideband communications. This would cause the first sideband communication (power_up_phy()), to start timing out because the sink wasn't ready to respond. Afterwards, we would call intel_dp_sink_dpms(DRM_MODE_DPMS_ON) in intel_ddi_pre_enable_dp(), which would actually result in waking up the sink so that sideband requests would work again. Since DPMS is what lets us actually bring the hub up into a state where sideband communications become functional again, we just need to make sure to enable DPMS on the display before attempting to perform sideband communications. Changes since v1: - Remove comment above if (!intel_dp->is_mst) - vsryjala - Move intel_dp_sink_dpms() for MST into intel_dp_post_disable_mst() to keep enable/disable paths symmetrical - Improve commit message - dhnkrn Changes since v2: - Only send DPMS off when we're disabling the last sink, and only send DPMS on when we're enabling the first sink - dhnkrn Changes since v3: - Check against is_mst, not intel_dp->is_mst - dhnkrn/vsyrjala For the booting docked with lid down case: Tested-by: Laura AbbottSigned-off-by: Lyude Paul Cc: Dhinakaran Pandiyan Cc: Ville Syrjälä Cc: Laura Abbott Cc: sta...@vger.kernel.org Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.") --- drivers/gpu/drm/i915/intel_ddi.c| 8 ++-- drivers/gpu/drm/i915/intel_dp_mst.c | 8 +++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index a6672a9abd85..92cb26b18a9b 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2324,7 +2324,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, intel_prepare_dp_ddi_buffers(encoder, crtc_state); intel_ddi_init_dp_buf_reg(encoder); - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + if (!is_mst) + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); intel_dp_start_link_train(intel_dp); if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) intel_dp_stop_link_train(intel_dp); @@ -2422,12 +2423,15 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder, struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_digital_port *dig_port = enc_to_dig_port(>base); struct intel_dp *intel_dp = _port->dp; + bool is_mst = intel_crtc_has_type(old_crtc_state, + INTEL_OUTPUT_DP_MST); /* * Power down sink before disabling the port, otherwise we end * up getting interrupts from the sink on detecting link loss. */ - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); + if (!is_mst) + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); intel_disable_ddi_buf(encoder); diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index c3de0918ee13..9e6956c08688 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -180,9 +180,11 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder, intel_dp->active_mst_links--; intel_mst->connector = NULL; - if (intel_dp->active_mst_links == 0) + if (intel_dp->active_mst_links == 0) { + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); intel_dig_port->base.post_disable(_dig_port->base, old_crtc_state, NULL); + } DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links); } @@ -223,7 +225,11 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder, DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links); + if (intel_dp->active_mst_links == 0) + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + drm_dp_send_power_updown_phy(_dp->mst_mgr, connector->port, true); + if (intel_dp->active_mst_links == 0) intel_dig_port->base.pre_enable(_dig_port->base,
Re: [PATCH 5/7] drm/vmwgfx: Stop updating plane->fb
On Fri, Apr 06, 2018 at 07:14:51PM +, Deepak Singh Rawat wrote: > This makes sense once we got rid of plane->fb > > Will this go to drm-next? The plan is to push to drm-misc-next once we get all the ducks in a row. > Could you please CC > me so that I can do some testing myself. Thanks. Here's a branch if you want a head start: git://github.com/vsyrjala/linux.git plane_fb_crtc_nuke_2 I'd definitely appreciate some testing of this stuff. Wouldn't want to break you stuff accidentally. > > Reviewed-by: Deepak Rawat> > > > > > From: Ville Syrjälä > > > > We want to get rid of plane->fb on atomic drivers. Stop setting it. > > > > Cc: Thomas Hellstrom > > Cc: Sinclair Yeh > > Cc: VMware Graphics > > Cc: Daniel Vetter > > Signed-off-by: Ville Syrjälä > > --- > > drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 -- > > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 -- > > 2 files changed, 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > > b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > > index 648f8127f65a..bbd3f19b1a0b 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > > @@ -525,8 +525,6 @@ vmw_sou_primary_plane_atomic_update(struct > > drm_plane *plane, > > */ > > if (ret != 0) > > DRM_ERROR("Failed to update screen.\n"); > > - > > - crtc->primary->fb = plane->state->fb; > > } else { > > /* > > * When disabling a plane, CRTC and FB should always be > > NULL > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > > b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > > index 67331f01ef32..90445bc590cb 100644 > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > > @@ -1285,8 +1285,6 @@ vmw_stdu_primary_plane_atomic_update(struct > > drm_plane *plane, > > 1, 1, NULL, crtc); > > if (ret) > > DRM_ERROR("Failed to update STDU.\n"); > > - > > - crtc->primary->fb = plane->state->fb; > > } else { > > crtc = old_state->crtc; > > stdu = vmw_crtc_to_stdu(crtc); > > -- > > 2.16.1 -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 15/19] omap2: omapfb: allow building it with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 >> drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c:230:23: >> sparse: cast to restricted __be32 -- >> drivers/video/fbdev/omap2/omapfb/dss/dispc.c:289:9: sparse: context >> imbalance in 'mgr_fld_write' - different lock contexts for basic block vim +230 drivers/video/fbdev/omap2/omapfb/displays/panel-sony-acx565akm.c f76ee892 Tomi Valkeinen 2015-12-09 222 f76ee892 Tomi Valkeinen 2015-12-09 223 static int panel_enabled(struct panel_drv_data *ddata) f76ee892 Tomi Valkeinen 2015-12-09 224 { f76ee892 Tomi Valkeinen 2015-12-09 225 u32 disp_status; f76ee892 Tomi Valkeinen 2015-12-09 226 int enabled; f76ee892 Tomi Valkeinen 2015-12-09 227 f76ee892 Tomi Valkeinen 2015-12-09 228 acx565akm_read(ddata, MIPID_CMD_READ_DISP_STATUS, f76ee892 Tomi Valkeinen 2015-12-09 229 (u8 *)_status, 4); f76ee892 Tomi Valkeinen 2015-12-09 @230 disp_status = __be32_to_cpu(disp_status); f76ee892 Tomi Valkeinen 2015-12-09 231 enabled = (disp_status & (1 << 17)) && (disp_status & (1 << 10)); f76ee892 Tomi Valkeinen 2015-12-09 232 dev_dbg(>spi->dev, f76ee892 Tomi Valkeinen 2015-12-09 233 "LCD panel %senabled by bootloader (status 0x%04x)\n", f76ee892 Tomi Valkeinen 2015-12-09 234 enabled ? "" : "not ", disp_status); f76ee892 Tomi Valkeinen 2015-12-09 235 return enabled; f76ee892 Tomi Valkeinen 2015-12-09 236 } f76ee892 Tomi Valkeinen 2015-12-09 237 :: The code at line 230 was first introduced by commit :: f76ee892a99e68b55402b8d4b8aeffcae2aff34d omapfb: copy omapdss & displays for omapfb :: TO: Tomi Valkeinen <tomi.valkei...@ti.com> :: CC: Tomi Valkeinen <tomi.valkei...@ti.com> --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/i915/dp: Send DPCD ON for MST before phy_up
On Fri, Apr 06, 2018 at 12:28:30PM -0700, Dhinakaran Pandiyan wrote: > > > > On Fri, 2018-04-06 at 14:52 -0400, Lyude Paul wrote: > > When doing a modeset where the sink is transitioning from D3 to D0 , it > > would sometimes be possible for the initial power_up_phy() to start > > timing out. This would only be observed in the last action before the > > sink went into D3 mode was intel_dp_sink_dpms(DRM_MODE_DPMS_OFF). We > > originally thought this might be an issue with us accidentally shutting > > off the aux block when putting the sink into D3, but since the DP spec > > mandates that sinks must wake up within 1ms while we have 100ms to > > respond to an ESI irq, this didn't really add up. Turns out that the > > problem is more subtle then that: > > > > It turns out that the timeout is from us not enabling DPMS on the MST > > hub before actually trying to initiate sideband communications. This > > would cause the first sideband communication (power_up_phy()), to start > > timing out because the sink wasn't ready to respond. Afterwards, we > > would call intel_dp_sink_dpms(DRM_MODE_DPMS_ON) in > > intel_ddi_pre_enable_dp(), which would actually result in waking up the > > sink so that sideband requests would work again. > > > > Since DPMS is what lets us actually bring the hub up into a state where > > sideband communications become functional again, we just need to make > > sure to enable DPMS on the display before attempting to perform sideband > > communications. > > > > Matches my understanding of the problem > > Reviewed-by: Dhinakaran Pandiyan> > It's better to get an ack from Ville considering I was okay with the > D3_AUX_ON solution too. lgtm Reviewed-by: Ville Syrjälä > > > > Changes since v1: > > - Remove comment above if (!intel_dp->is_mst) - vsryjala > > - Move intel_dp_sink_dpms() for MST into intel_dp_post_disable_mst() to > > keep enable/disable paths symmetrical > > - Improve commit message - dhnkrn > > Changes since v2: > > - Only send DPMS off when we're disabling the last sink, and only send > > DPMS on when we're enabling the first sink - dhnkrn > > Changes since v3: > > - Check against is_mst, not intel_dp->is_mst - dhnkrn/vsyrjala > > > > Signed-off-by: Lyude Paul > > Cc: Dhinakaran Pandiyan > > Cc: Ville Syrjälä > > Cc: Laura Abbott > > Cc: sta...@vger.kernel.org > > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST > > hub.") > > --- > > drivers/gpu/drm/i915/intel_ddi.c| 8 ++-- > > drivers/gpu/drm/i915/intel_dp_mst.c | 8 +++- > > 2 files changed, 13 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > > b/drivers/gpu/drm/i915/intel_ddi.c > > index a6672a9abd85..92cb26b18a9b 100644 > > --- a/drivers/gpu/drm/i915/intel_ddi.c > > +++ b/drivers/gpu/drm/i915/intel_ddi.c > > @@ -2324,7 +2324,8 @@ static void intel_ddi_pre_enable_dp(struct > > intel_encoder *encoder, > > intel_prepare_dp_ddi_buffers(encoder, crtc_state); > > > > intel_ddi_init_dp_buf_reg(encoder); > > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > + if (!is_mst) > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > > intel_dp_start_link_train(intel_dp); > > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > > intel_dp_stop_link_train(intel_dp); > > @@ -2422,12 +2423,15 @@ static void intel_ddi_post_disable_dp(struct > > intel_encoder *encoder, > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > struct intel_digital_port *dig_port = enc_to_dig_port(>base); > > struct intel_dp *intel_dp = _port->dp; > > + bool is_mst = intel_crtc_has_type(old_crtc_state, > > + INTEL_OUTPUT_DP_MST); > > > > /* > > * Power down sink before disabling the port, otherwise we end > > * up getting interrupts from the sink on detecting link loss. > > */ > > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > > + if (!is_mst) > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > > > > intel_disable_ddi_buf(encoder); > > > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c > > b/drivers/gpu/drm/i915/intel_dp_mst.c > > index c3de0918ee13..9e6956c08688 100644 > > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > > @@ -180,9 +180,11 @@ static void intel_mst_post_disable_dp(struct > > intel_encoder *encoder, > > intel_dp->active_mst_links--; > > > > intel_mst->connector = NULL; > > - if (intel_dp->active_mst_links == 0) > > + if (intel_dp->active_mst_links == 0) { > > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > > intel_dig_port->base.post_disable(_dig_port->base, > > old_crtc_state, NULL);
RE: [PATCH 5/7] drm/vmwgfx: Stop updating plane->fb
This makes sense once we got rid of plane->fb Will this go to drm-next? Could you please CC me so that I can do some testing myself. Thanks. Reviewed-by: Deepak Rawat> > From: Ville Syrjälä > > We want to get rid of plane->fb on atomic drivers. Stop setting it. > > Cc: Thomas Hellstrom > Cc: Sinclair Yeh > Cc: VMware Graphics > Cc: Daniel Vetter > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c | 2 -- > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 2 -- > 2 files changed, 4 deletions(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > index 648f8127f65a..bbd3f19b1a0b 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c > @@ -525,8 +525,6 @@ vmw_sou_primary_plane_atomic_update(struct > drm_plane *plane, >*/ > if (ret != 0) > DRM_ERROR("Failed to update screen.\n"); > - > - crtc->primary->fb = plane->state->fb; > } else { > /* >* When disabling a plane, CRTC and FB should always be > NULL > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > index 67331f01ef32..90445bc590cb 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > @@ -1285,8 +1285,6 @@ vmw_stdu_primary_plane_atomic_update(struct > drm_plane *plane, >1, 1, NULL, crtc); > if (ret) > DRM_ERROR("Failed to update STDU.\n"); > - > - crtc->primary->fb = plane->state->fb; > } else { > crtc = old_state->crtc; > stdu = vmw_crtc_to_stdu(crtc); > -- > 2.16.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v4] drm/i915/dp: Send DPCD ON for MST before phy_up
On Fri, 2018-04-06 at 14:52 -0400, Lyude Paul wrote: > When doing a modeset where the sink is transitioning from D3 to D0 , it > would sometimes be possible for the initial power_up_phy() to start > timing out. This would only be observed in the last action before the > sink went into D3 mode was intel_dp_sink_dpms(DRM_MODE_DPMS_OFF). We > originally thought this might be an issue with us accidentally shutting > off the aux block when putting the sink into D3, but since the DP spec > mandates that sinks must wake up within 1ms while we have 100ms to > respond to an ESI irq, this didn't really add up. Turns out that the > problem is more subtle then that: > > It turns out that the timeout is from us not enabling DPMS on the MST > hub before actually trying to initiate sideband communications. This > would cause the first sideband communication (power_up_phy()), to start > timing out because the sink wasn't ready to respond. Afterwards, we > would call intel_dp_sink_dpms(DRM_MODE_DPMS_ON) in > intel_ddi_pre_enable_dp(), which would actually result in waking up the > sink so that sideband requests would work again. > > Since DPMS is what lets us actually bring the hub up into a state where > sideband communications become functional again, we just need to make > sure to enable DPMS on the display before attempting to perform sideband > communications. > Matches my understanding of the problem Reviewed-by: Dhinakaran PandiyanIt's better to get an ack from Ville considering I was okay with the D3_AUX_ON solution too. > Changes since v1: > - Remove comment above if (!intel_dp->is_mst) - vsryjala > - Move intel_dp_sink_dpms() for MST into intel_dp_post_disable_mst() to > keep enable/disable paths symmetrical > - Improve commit message - dhnkrn > Changes since v2: > - Only send DPMS off when we're disabling the last sink, and only send > DPMS on when we're enabling the first sink - dhnkrn > Changes since v3: > - Check against is_mst, not intel_dp->is_mst - dhnkrn/vsyrjala > > Signed-off-by: Lyude Paul > Cc: Dhinakaran Pandiyan > Cc: Ville Syrjälä > Cc: Laura Abbott > Cc: sta...@vger.kernel.org > Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST > hub.") > --- > drivers/gpu/drm/i915/intel_ddi.c| 8 ++-- > drivers/gpu/drm/i915/intel_dp_mst.c | 8 +++- > 2 files changed, 13 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ddi.c > b/drivers/gpu/drm/i915/intel_ddi.c > index a6672a9abd85..92cb26b18a9b 100644 > --- a/drivers/gpu/drm/i915/intel_ddi.c > +++ b/drivers/gpu/drm/i915/intel_ddi.c > @@ -2324,7 +2324,8 @@ static void intel_ddi_pre_enable_dp(struct > intel_encoder *encoder, > intel_prepare_dp_ddi_buffers(encoder, crtc_state); > > intel_ddi_init_dp_buf_reg(encoder); > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > + if (!is_mst) > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); > intel_dp_start_link_train(intel_dp); > if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) > intel_dp_stop_link_train(intel_dp); > @@ -2422,12 +2423,15 @@ static void intel_ddi_post_disable_dp(struct > intel_encoder *encoder, > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_digital_port *dig_port = enc_to_dig_port(>base); > struct intel_dp *intel_dp = _port->dp; > + bool is_mst = intel_crtc_has_type(old_crtc_state, > + INTEL_OUTPUT_DP_MST); > > /* >* Power down sink before disabling the port, otherwise we end >* up getting interrupts from the sink on detecting link loss. >*/ > - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > + if (!is_mst) > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > > intel_disable_ddi_buf(encoder); > > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c > b/drivers/gpu/drm/i915/intel_dp_mst.c > index c3de0918ee13..9e6956c08688 100644 > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > @@ -180,9 +180,11 @@ static void intel_mst_post_disable_dp(struct > intel_encoder *encoder, > intel_dp->active_mst_links--; > > intel_mst->connector = NULL; > - if (intel_dp->active_mst_links == 0) > + if (intel_dp->active_mst_links == 0) { > + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); > intel_dig_port->base.post_disable(_dig_port->base, > old_crtc_state, NULL); > + } > > DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links); > } > @@ -223,7 +225,11 @@ static void intel_mst_pre_enable_dp(struct intel_encoder > *encoder, > > DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links); > > + if
Re: [PATCH 8/8] drm/arm/malidp: Added the late system pm functions
On Tue, Mar 27, 2018 at 01:09:36PM +0200, Daniel Vetter wrote: > On Tue, Mar 27, 2018 at 11:59 AM, Ayan Halderwrote: > > On Tue, Mar 27, 2018 at 10:29:03AM +0200, Daniel Vetter wrote: > >> On Mon, Mar 26, 2018 at 06:03:20PM +0100, Ayan Kumar Halder wrote: > >> > malidp_pm_suspend_late checks if the runtime status is not suspended > >> > and if so, invokes malidp_runtime_pm_suspend which disables the > >> > display engine/core interrupts and the clocks. It sets the runtime status > >> > as suspended. Subsequently, malidp_pm_resume_early will invoke > >> > malidp_runtime_pm_resume which enables the clocks and the interrupts > >> > (previously disabled) and sets the runtime status as active. > >> > > >> > Signed-off-by: Ayan Kumar Halder > >> > Change-Id: I5f8c3d28f076314a1c9da2a46760a9c37039ccda > >> > >> Why exactly do you need late/early hooks? If you have dependencies with > >> other devices, pls consider adding device_links instead. This here > >> shouldn't be necessary. > >> -Daniel > > We need to late/early hooks to disable malidp interrupts and the > > clocks. > > Yes, but why this ordering constraint? Why can't you just disable the > interrupts/clocks in the normal suspend code. I see that the patch > does this, I want to understand why it does it. > -Daniel Apologies for my delayed response on this. With reference to https://lwn.net/Articles/505683/ :- 1. "suspend() should leave the device in a quiescent state." We invoke drm_mode_config_helper_suspend() which deactivates the crtc. I understand that this is the quiescent state. 2. "suspend_late() can often be the same as runtime_suspend()." We invoke runtime suspend/resume calls in late/early hooks. > >> > --- > >> > drivers/gpu/drm/arm/malidp_drv.c | 17 + > >> > 1 file changed, 17 insertions(+) > >> > > >> > diff --git a/drivers/gpu/drm/arm/malidp_drv.c > >> > b/drivers/gpu/drm/arm/malidp_drv.c > >> > index bd44a6d..f6124d8 100644 > >> > --- a/drivers/gpu/drm/arm/malidp_drv.c > >> > +++ b/drivers/gpu/drm/arm/malidp_drv.c > >> > @@ -766,8 +766,25 @@ static int __maybe_unused malidp_pm_resume(struct > >> > device *dev) > >> > return 0; > >> > } > >> > > >> > +static int __maybe_unused malidp_pm_suspend_late(struct device *dev) > >> > +{ > >> > + if (!pm_runtime_status_suspended(dev)) { > >> > + malidp_runtime_pm_suspend(dev); > >> > + pm_runtime_set_suspended(dev); > >> > + } > >> > + return 0; > >> > +} > >> > + > >> > +static int __maybe_unused malidp_pm_resume_early(struct device *dev) > >> > +{ > >> > + malidp_runtime_pm_resume(dev); > >> > + pm_runtime_set_active(dev); > >> > + return 0; > >> > +} > >> > + > >> > static const struct dev_pm_ops malidp_pm_ops = { > >> > SET_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend, malidp_pm_resume) \ > >> > + SET_LATE_SYSTEM_SLEEP_PM_OPS(malidp_pm_suspend_late, > >> > malidp_pm_resume_early) \ > >> > SET_RUNTIME_PM_OPS(malidp_runtime_pm_suspend, > >> > malidp_runtime_pm_resume, NULL) > >> > }; > >> > > >> > -- > >> > 2.7.4 > >> > > >> > ___ > >> > dri-devel mailing list > >> > dri-devel@lists.freedesktop.org > >> > https://lists.freedesktop.org/mailman/listinfo/dri-devel > >> > >> -- > >> Daniel Vetter > >> Software Engineer, Intel Corporation > >> http://blog.ffwll.ch > >> ___ > >> dri-devel mailing list > >> dri-devel@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > > IMPORTANT NOTICE: The contents of this email and any attachments are > > confidential and may also be privileged. If you are not the intended > > recipient, please notify the sender immediately and do not disclose the > > contents to any other person, use it for any purpose, or store or copy the > > information in any medium. Thank you. > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RfC PATCH] Add udmabuf misc device
On Fri, Apr 06, 2018 at 03:36:03PM +0300, Oleksandr Andrushchenko wrote: > On 04/06/2018 02:57 PM, Gerd Hoffmann wrote: > > Hi, > > > >>>I fail to see any common ground for xen-zcopy and udmabuf ... > >>Does the above mean you can assume that xen-zcopy and udmabuf > >>can co-exist as two different solutions? > >Well, udmabuf route isn't fully clear yet, but yes. > > > >See also gvt (intel vgpu), where the hypervisor interface is abstracted > >away into a separate kernel modules even though most of the actual vgpu > >emulation code is common. > Thank you for your input, I'm just trying to figure out > which of the three z-copy solutions intersect and how much > >>And what about hyper-dmabuf? xen z-copy solution is pretty similar fundamentally to hyper_dmabuf in terms of these core sharing feature: 1. the sharing process - import prime/dmabuf from the producer -> extract underlying pages and get those shared -> return references for shared pages 2. the page sharing mechanism - it uses Xen-grant-table. And to give you a quick summary of differences as far as I understand between two implementations (please correct me if I am wrong, Oleksandr.) 1. xen-zcopy is DRM specific - can import only DRM prime buffer while hyper_dmabuf can export any dmabuf regardless of originator 2. xen-zcopy doesn't seem to have dma-buf synchronization between two VMs while (as danvet called it as remote dmabuf api sharing) hyper_dmabuf sends out synchronization message to the exporting VM for synchronization. 3. 1-level references - when using grant-table for sharing pages, there will be same # of refs (each 8 byte) as # of shared pages, which is passed to the userspace to be shared with importing VM in case of xen-zcopy. Compared to this, hyper_dmabuf does multiple level addressing to generate only one reference id that represents all shared pages. 4. inter VM messaging (hype_dmabuf only) - hyper_dmabuf has inter-vm msg communication defined for dmabuf synchronization and private data (meta info that Matt Roper mentioned) exchange. 5. driver-to-driver notification (hyper_dmabuf only) - importing VM gets notified when newdmabuf is exported from other VM - uevent can be optionally generated when this happens. 6. structure - hyper_dmabuf is targetting to provide a generic solution for inter-domain dmabuf sharing for most hypervisors, which is why it has two layers as mattrope mentioned, front-end that contains standard API and backend that is specific to hypervisor. > >No idea, didn't look at it in detail. > > > >Looks pretty complex from a distant view. Maybe because it tries to > >build a communication framework using dma-bufs instead of a simple > >dma-buf passing mechanism. we started with simple dma-buf sharing but realized there are many things we need to consider in real use-case, so we added communication , notification and dma-buf synchronization then re-structured it to front-end and back-end (this made things more compicated..) since Xen was not our only target. Also, we thought passing the reference for the buffer (hyper_dmabuf_id) is not secure so added uvent mechanism later. > Yes, I am looking at it now, trying to figure out the full story > and its implementation. BTW, Intel guys were about to share some > test application for hyper-dmabuf, maybe I have missed one. > It could probably better explain the use-cases and the complexity > they have in hyper-dmabuf. One example is actually in github. If you want take a look at it, please visit: https://github.com/downor/linux_hyper_dmabuf_test/tree/xen/simple_export > > > >Like xen-zcopy it seems to depend on the idea that the hypervisor > >manages all memory it is easy for guests to share pages with the help of > >the hypervisor. > So, for xen-zcopy we were not trying to make it generic, > it just solves display (dumb) zero-copying use-cases for Xen. > We implemented it as a DRM helper driver because we can't see any > other use-cases as of now. > For example, we also have Xen para-virtualized sound driver, but > its buffer memory usage is not comparable to what display wants > and it works somewhat differently (e.g. there is no "frame done" > event, so one can't tell when the sound buffer can be "flipped"). > At the same time, we do not use virtio-gpu, so this could probably > be one more candidate for shared dma-bufs some day. > > Which simply isn't the case on kvm. > > > >hyper-dmabuf and xen-zcopy could maybe share code, or hyper-dmabuf build > >on top of xen-zcopy. > Hm, I can imagine that: xen-zcopy could be a library code for hyper-dmabuf > in terms of implementing all that page sharing fun in multiple directions, > e.g. Host->Guest, Guest->Host, Guest<->Guest. > But I'll let Matt and Dongwon to comment on that. I think we can definitely collaborate. Especially, maybe we are using some outdated sharing mechanism/grant-table mechanism in our Xen backend (thanks for bringing that up Oleksandr). However, the question is once we collaborate
RE: [PATCH 6/7] drm/vmwgfx: Stop using plane->fb in atomic_enable()
> > From: Ville Syrjälä> > Instead of looking at the (soon to be deprecated) plane->fb we'll > examing plane->state->fb instead. We can do this because > vmw_du_crtc_atomic_check() prevents us from enabling a crtc > without the primary plane also being enabled. > > Due to that same reason, I'm actually not sure what the checks here are > for NULL fb. If we can't enable the crtc without an enabled plane > we should always have an fb. But I'll leave that for someone else > to figure out. Hi Ville, AFAIK the NULL check is set or clear the implicit framebuffer property which is specific to vmwgfx and for current hardware version is disabled. I have this future TODO work item to get rid of implicit placement property or at least make it read only. I still don’t have complete understanding of atomic state but this patch looks good to me. Reviewed-by: Deepak Rawat > > Cc: Thomas Hellstrom > Cc: Sinclair Yeh > Cc: VMware Graphics > Cc: Daniel Vetter > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > index 90445bc590cb..152e96cb1c01 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c > @@ -414,6 +414,7 @@ static void vmw_stdu_crtc_helper_prepare(struct > drm_crtc *crtc) > static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc, > struct drm_crtc_state *old_state) > { > + struct drm_plane_state *plane_state = crtc->primary->state; > struct vmw_private *dev_priv; > struct vmw_screen_target_display_unit *stdu; > struct vmw_framebuffer *vfb; > @@ -422,7 +423,7 @@ static void vmw_stdu_crtc_atomic_enable(struct > drm_crtc *crtc, > > stdu = vmw_crtc_to_stdu(crtc); > dev_priv = vmw_priv(crtc->dev); > - fb = crtc->primary->fb; > + fb = plane_state->fb; > > vfb = (fb) ? vmw_framebuffer_to_vfb(fb) : NULL; > > -- > 2.16.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4] drm/i915/dp: Send DPCD ON for MST before phy_up
When doing a modeset where the sink is transitioning from D3 to D0 , it would sometimes be possible for the initial power_up_phy() to start timing out. This would only be observed in the last action before the sink went into D3 mode was intel_dp_sink_dpms(DRM_MODE_DPMS_OFF). We originally thought this might be an issue with us accidentally shutting off the aux block when putting the sink into D3, but since the DP spec mandates that sinks must wake up within 1ms while we have 100ms to respond to an ESI irq, this didn't really add up. Turns out that the problem is more subtle then that: It turns out that the timeout is from us not enabling DPMS on the MST hub before actually trying to initiate sideband communications. This would cause the first sideband communication (power_up_phy()), to start timing out because the sink wasn't ready to respond. Afterwards, we would call intel_dp_sink_dpms(DRM_MODE_DPMS_ON) in intel_ddi_pre_enable_dp(), which would actually result in waking up the sink so that sideband requests would work again. Since DPMS is what lets us actually bring the hub up into a state where sideband communications become functional again, we just need to make sure to enable DPMS on the display before attempting to perform sideband communications. Changes since v1: - Remove comment above if (!intel_dp->is_mst) - vsryjala - Move intel_dp_sink_dpms() for MST into intel_dp_post_disable_mst() to keep enable/disable paths symmetrical - Improve commit message - dhnkrn Changes since v2: - Only send DPMS off when we're disabling the last sink, and only send DPMS on when we're enabling the first sink - dhnkrn Changes since v3: - Check against is_mst, not intel_dp->is_mst - dhnkrn/vsyrjala Signed-off-by: Lyude PaulCc: Dhinakaran Pandiyan Cc: Ville Syrjälä Cc: Laura Abbott Cc: sta...@vger.kernel.org Fixes: ad260ab32a4d9 ("drm/i915/dp: Write to SET_POWER dpcd to enable MST hub.") --- drivers/gpu/drm/i915/intel_ddi.c| 8 ++-- drivers/gpu/drm/i915/intel_dp_mst.c | 8 +++- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index a6672a9abd85..92cb26b18a9b 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -2324,7 +2324,8 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder, intel_prepare_dp_ddi_buffers(encoder, crtc_state); intel_ddi_init_dp_buf_reg(encoder); - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + if (!is_mst) + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); intel_dp_start_link_train(intel_dp); if (port != PORT_A || INTEL_GEN(dev_priv) >= 9) intel_dp_stop_link_train(intel_dp); @@ -2422,12 +2423,15 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder, struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_digital_port *dig_port = enc_to_dig_port(>base); struct intel_dp *intel_dp = _port->dp; + bool is_mst = intel_crtc_has_type(old_crtc_state, + INTEL_OUTPUT_DP_MST); /* * Power down sink before disabling the port, otherwise we end * up getting interrupts from the sink on detecting link loss. */ - intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); + if (!is_mst) + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); intel_disable_ddi_buf(encoder); diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c index c3de0918ee13..9e6956c08688 100644 --- a/drivers/gpu/drm/i915/intel_dp_mst.c +++ b/drivers/gpu/drm/i915/intel_dp_mst.c @@ -180,9 +180,11 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder, intel_dp->active_mst_links--; intel_mst->connector = NULL; - if (intel_dp->active_mst_links == 0) + if (intel_dp->active_mst_links == 0) { + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF); intel_dig_port->base.post_disable(_dig_port->base, old_crtc_state, NULL); + } DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links); } @@ -223,7 +225,11 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder, DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links); + if (intel_dp->active_mst_links == 0) + intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON); + drm_dp_send_power_updown_phy(_dp->mst_mgr, connector->port, true); + if (intel_dp->active_mst_links == 0) intel_dig_port->base.pre_enable(_dig_port->base, pipe_config, NULL); -- 2.14.3 ___ dri-devel
[Bug 105869] clang crashes when compiling OpenCL kernel
https://bugs.freedesktop.org/show_bug.cgi?id=105869 --- Comment #7 from Jan Vesely--- (In reply to Lyberta from comment #6) > I'm 100% sure it is PulseWave because that's the only kernel I use to one of > my programs and it still crashes at cl::Program::build. Is the posted snippet all that is compiled? can you run with CLOVER_DEBUG=clc,llvm CLOVER_DEBUG_FILE=dump and attached the created dump.{cl,ll} files? > How to upgrade to llvm/clang 6? either there is a distro specific way (for your distro) to try testing packages. Packages for popular distros are also available here: http://releases.llvm.org/download.html You can also build from source. Note that you'll need to rebuild mesa and libclc after the upgrade. -- 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 4/7] drm/vmwgfx: Stop using plane->fb in vmw_kms_update_implicit_fb()
Reviewed-by: Deepak Rawat> > From: Ville Syrjälä > > The only caller of vmw_kms_update_implicit_fb() is the page_flip > hook which itself gets called with the plane mutex already held. > Hence we can look at plane->state safely. Toss in a lockdep assert > to make the situation more clear. > > Cc: Thomas Hellstrom > Cc: Sinclair Yeh > Cc: VMware Graphics > Cc: Daniel Vetter > Signed-off-by: Ville Syrjälä > --- > drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > index 5a824125c231..a93d290b0f35 100644 > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c > @@ -2811,14 +2811,17 @@ void vmw_kms_update_implicit_fb(struct > vmw_private *dev_priv, > struct drm_crtc *crtc) > { > struct vmw_display_unit *du = vmw_crtc_to_du(crtc); > + struct drm_plane *plane = crtc->primary; > struct vmw_framebuffer *vfb; > > + lockdep_assert_held(>mutex); > + > mutex_lock(_priv->global_kms_state_mutex); > > if (!du->is_implicit) > goto out_unlock; > > - vfb = vmw_framebuffer_to_vfb(crtc->primary->fb); > + vfb = vmw_framebuffer_to_vfb(plane->state->fb); > WARN_ON_ONCE(dev_priv->num_implicit != 1 && >dev_priv->implicit_fb != vfb); > > -- > 2.16.1 > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/2] media: omapfb: relax compilation if COMPILE_TEST
The dependency of DRM_OMAP = n can be relaxed for just compilation test. This allows building the omap3isp driver with allyesconfig on ARM. Signed-off-by: Mauro Carvalho Chehab--- drivers/video/fbdev/omap2/omapfb/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/video/fbdev/omap2/omapfb/Kconfig b/drivers/video/fbdev/omap2/omapfb/Kconfig index e6226aeed17e..e42794a5e26c 100644 --- a/drivers/video/fbdev/omap2/omapfb/Kconfig +++ b/drivers/video/fbdev/omap2/omapfb/Kconfig @@ -4,7 +4,7 @@ config OMAP2_VRFB menuconfig FB_OMAP2 tristate "OMAP2+ frame buffer support" depends on FB -depends on DRM_OMAP = n +depends on DRM_OMAP = n || COMPILE_TEST select FB_OMAP2_DSS select OMAP2_VRFB if ARCH_OMAP2 || ARCH_OMAP3 -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/7] arm64: dts: renesas: r8a77970: add DU support
From: Sergei ShtylyovDefine the generic R8A77970 part of the DU device node. Based on the original (and large) patch by Daisuke Matsushita . Signed-off-by: Vladimir Barinov Signed-off-by: Sergei Shtylyov Signed-off-by: Niklas Söderlund --- arch/arm64/boot/dts/renesas/r8a77970.dtsi | 28 1 file changed, 28 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi b/arch/arm64/boot/dts/renesas/r8a77970.dtsi index db06c94..e649e86 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi @@ -635,6 +635,34 @@ resets = < 623>; renesas,fcp = <>; }; + + du: display@feb0 { + compatible = "renesas,du-r8a77970"; + reg = <0 0xfeb0 0 0x8>; + interrupts = ; + clocks = < CPG_MOD 724>; + clock-names = "du.0"; + power-domains = < R8A77970_PD_ALWAYS_ON>; + vsps = <>; + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + du_out_rgb: endpoint { + }; + }; + + port@1 { + reg = <1>; + du_out_lvds: endpoint { + }; + }; + }; + }; }; timer { -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/7] arm64: dts: renesas: eagle: Enable DU
Enable DU for Renesas R-Car V3M Eagle board. Signed-off-by: Jacopo Mondi--- arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 11 +++ 1 file changed, 11 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index 3c5f598..144b847 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts @@ -76,6 +76,11 @@ function = "i2c0"; }; + du_pins: du { + groups = "du_rgb666", "du_sync", "du_oddf", "du_clk_out"; + function = "du"; + }; + scif0_pins: scif0 { groups = "scif0_data"; function = "scif0"; @@ -93,3 +98,9 @@ status = "okay"; }; + + { + pinctrl-0 = <_pins>; + pinctrl-names = "default"; + status = "okay"; +}; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/7] arm64: dts: renesas: r8a77970: add FCPVD support
From: Sergei ShtylyovDescribe FCPVD0 in the R8A77970 device tree; it will be used by VSPD0 in the next patch... Based on the original (and large) patch by Daisuke Matsushita . Signed-off-by: Vladimir Barinov Signed-off-by: Sergei Shtylyov Signed-off-by: Niklas Söderlund --- arch/arm64/boot/dts/renesas/r8a77970.dtsi | 8 1 file changed, 8 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi b/arch/arm64/boot/dts/renesas/r8a77970.dtsi index e8358d9..71f466d 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi @@ -617,6 +617,14 @@ #address-cells = <1>; #size-cells = <0>; }; + + fcpvd0: fcp@fea27000 { + compatible = "renesas,fcpv"; + reg = <0 0xfea27000 0 0x200>; + clocks = < CPG_MOD 603>; + power-domains = < R8A77970_PD_ALWAYS_ON>; + resets = < 603>; + }; }; timer { -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 1/2] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder
Document Thine THC63LVD1024 LVDS decoder device tree bindings. Signed-off-by: Jacopo MondiReviewed-by: Andrzej Hajda Reviewed-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- .../bindings/display/bridge/thine,thc63lvd1024.txt | 60 ++ 1 file changed, 60 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt diff --git a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt new file mode 100644 index 000..1191f17 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt @@ -0,0 +1,60 @@ +Thine Electronics THC63LVD1024 LVDS decoder +--- + +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS streams +to parallel data outputs. The chip supports single/dual input/output modes, +handling up to two two input LVDS stream and up to two digital CMOS/TTL outputs. + +Single or dual operation modes, output data mapping and DDR output modes are +configured through input signals and the chip does not expose any control bus. + +Required properties: +- compatible: Shall be "thine,thc63lvd1024" + +Optional properties: +- vcc-supply: Power supply for TTL output, TTL CLOCKOUT signal, LVDS input, + PPL and digital circuitry +- powerdown-gpios: Power down GPIO signal, pin name "/PDWN". Active low +- enable-gpios: Output enable GPIO signal, pin name "OE". Active high + +The THC63LVD1024 video port connections are modeled according +to OF graph bindings specified by Documentation/devicetree/bindings/graph.txt + +Required video port nodes: +- port@0: First LVDS input port +- port@2: First digital CMOS/TTL parallel output + +Optional video port nodes: +- port@1: Second LVDS input port +- port@3: Second digital CMOS/TTL parallel output + +Example: + + + thc63lvd1024: lvds-decoder { + compatible = "thine,thc63lvd1024"; + + vcc-supply = <_lvds_vcc>; + powerdown-gpios = < 15 GPIO_ACTIVE_LOW>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + lvds_dec_in_0: endpoint { + remote-endpoint = <_out>; + }; + }; + + port@2{ + reg = <2>; + + lvds_dec_out_2: endpoint { + remote-endpoint = <_in>; + }; + }; + }; + }; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/7] arm64: dts: renesas: r8a77970: add the LVDS instance
From: Niklas SöderlundAdd the LVDS device to r8a77970.dtsi in a disabled state. Also connect the it to the LVDS output of the DU. While at it align the endpoint name of the du to du_out_lvds0 which is used in other Renesas DTS files to describe this link. Signed-off-by: Niklas Söderlund --- arch/arm64/boot/dts/renesas/r8a77970.dtsi | 29 - 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi b/arch/arm64/boot/dts/renesas/r8a77970.dtsi index e649e86..b48d62c 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi @@ -658,7 +658,34 @@ port@1 { reg = <1>; - du_out_lvds: endpoint { + du_out_lvds0: endpoint { + remote-endpoint = <_in>; + }; + }; + }; + }; + + lvds0: lvds@feb9 { + compatible = "renesas,r8a77970-lvds"; + reg = <0 0xfeb9 0 0x14>; + clocks = < CPG_MOD 727>; + power-domains = < R8A77970_PD_ALWAYS_ON>; + resets = < 727>; + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + lvds0_in: endpoint { + remote-endpoint = <_out_lvds0>; + }; + }; + port@1 { + reg = <1>; + lvds0_out: endpoint { }; }; }; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 0/2] drm: Add Thine THC63LVD1024 LVDS decoder bridge
Hello, this series enables HDMI display on V3M Eagle board. The series is based on Geert's "renesas-drivers-2018-04-03-v4.16" with THC63LVD1024 driver on top (cfr. my in review series: "[PATCH v7 0/2] drm: Add Thine THC63LVD1024 LVDS decoder bridge") This series includes some preliminary work from Sergei and Niklas. I have reworked the two final patches from Niklas to enable DU first, add the LVDS decoder node, and finally add the ADV7511W chip and enable HDMI output. A branch for testing is available at: git://jmondi.org/linux v3m/renesas-drivers-2018-04-03-v4.16/v7-eagle-dts Thanks j Jacopo Mondi (2): arm64: dts: renesas: eagle: Enable DU arm64: dts: renesas: eagle: Add LVDS decoder Niklas Söderlund (2): arm64: dts: renesas: r8a77970: add the LVDS instance arm64: dts: renesas: eagle: Add ADV7511W and HDMI output Sergei Shtylyov (3): arm64: dts: renesas: r8a77970: add FCPVD support arm64: dts: renesas: r8a77970: add VSPD support arm64: dts: renesas: r8a77970: add DU support arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 89 ++ arch/arm64/boot/dts/renesas/r8a77970.dtsi | 73 + 2 files changed, 162 insertions(+) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: AMD graphics performance regression in 4.15 and later
Hi Christian, Is there a way to turn off these huge pages at boot-time/run-time? Right now the recent kernels are making Firefox pretty much unusable for me. I've been able to revert the patch from 4.15 but it's not really a long-term solution. You mention that the purpose of the patch is to improve performance, but I haven't actually noticed anything running faster on my system. Is there any particular test where I'm supposed to see an improvement compared to 4.14? I'm not sure what you mean by "We mitigated the problem by avoiding the slow coherent DMA code path on almost all platforms on newer kernels". I tested up to 4.16 and the performance regression is just as bad as it is for 4.15. Unlike the older hardware reported on kernel bug 198511, the hardware I have is quite recent (RX 560) and still being sold. I've also confirmed that neither nvidia (on the same machine) nor intel GPUs (on a less powerful machine) are affected, so it seems like there's a way to avoid that slow performance. I'm not saying that what Firefox is doing is ideal (I don't know what it does and why), but it still seems like something that should still be avoided in the kernel. Cheers, Jean-Marc On 04/06/2018 04:03 AM, Christian König wrote: > Hi Jean, > > yeah, that is a known problem. Using huge pages improves the performance > because of better TLB usage, but for the cost of higher allocation > overhead. > > What we found is that firefox is doing something rather strange by > allocating large textures and then just trowing them away again > immediately. > > We mitigated the problem by avoiding the slow coherent DMA code path on > almost all platforms on newer kernels, but essentially somebody needs to > figure out why firefox and/or the user space stack is doing this > constant allocation/freeing of memory. > > There is also a bug tracker on bugs.kernel.org about this, but I can't > find it any more of hand. > > Regards, > Christian. > > Am 06.04.2018 um 02:30 schrieb Jean-Marc Valin: >> Hi, >> >> I noticed a serious graphics performance regression between 4.14 and >> 4.15. It is most noticeable with Firefox (tried FF57 through FF60) and >> causes scrolling to be really choppy/sluggish. I've confirmed that the >> problem is also there on 4.16, while 4.13 works fine. >> >> After a bisection, I've narrowed the regression down to this commit: >> >> commit 648bc3574716400acc06f99915815f80d9563783 >> Author: Christian König>> Date: Thu Jul 6 09:59:43 2017 +0200 >> >> drm/ttm: add transparent huge page support for DMA allocations v2 >> >> >> Some details about my system: >> Distro: Fedora 27 (up-to-date) >> Video: MSI Radeon RX 560 AERO >> CPU: Dual-socket Xeon E5-2640 v4 (20 cores total) >> RAM: 128 GB ECC >> >> >> As a comparison, when running Firefox with 4.15 on a Lenovo W540 laptop >> (with Intel graphics only) the responsiveness is much better then what >> I'm getting on the Xeon machine above with the Radeon card, so this >> really seems to be an AMD-only issue. >> >> Any way to fix the issue? >> >> Thanks, >> >> Jean-Marc >> ___ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 6/7] arm64: dts: renesas: eagle: Add LVDS decoder
The R-Car V3M Eagle board includes a transparent THC63LVD1024 LVDS decoder, connected to the on-chip LVDS encoder output on one side and to the not-yet-described HDMI encoder ADV7511W on the other one. As the decoder does not need any configuration it has been so-far omitted from DTS. Now that a driver is available, describe it in DT as well. Signed-off-by: Jacopo MondiReviewed-by: Andrzej Hajda --- arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 29 ++ 1 file changed, 29 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index 144b847..9d0e65d 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts @@ -31,6 +31,23 @@ /* first 128MB is reserved for secure area. */ reg = <0x0 0x4800 0x0 0x3800>; }; + + thc63lvd1024: lvds-decoder { + compatible = "thine,thc63lvd1024"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + + thc63lvd1024_in: endpoint { + remote-endpoint = <_out>; + }; + }; + }; + }; }; { @@ -104,3 +121,15 @@ pinctrl-names = "default"; status = "okay"; }; + + { + status = "okay"; + + ports { + port@1 { + lvds0_out: endpoint { + remote-endpoint = <_in>; + }; + }; + }; +}; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] drm/arc: Stop consulting plane->fb
Hi Ville, On Thu, 2018-04-05 at 22:50 +0300, Ville Syrjala wrote: > From: Ville Syrjälä> > We want to stop using plane->fb with atomic driver, so stop looking at > it. > > I have no idea what this code is trying to achieve. There is no > corresponding check in the enable path. Also since > arc_pgu_set_pxl_fmt() will anyway oops if there is no fb I'm going > to assuming that I can just remove the check entirely. There seems > to be a general shortage of .atomic_check() in this driver... > > Cc: Alexey Brodkin > Cc: Daniel Vetter > Signed-off-by: Ville Syrjälä Acked-by: Alexey Brodkin ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: AMD graphics performance regression in 4.15 and later
Hi Christian, On 04/09/2018 07:48 AM, Christian König wrote: > Am 06.04.2018 um 17:30 schrieb Jean-Marc Valin: >> Hi Christian, >> >> Is there a way to turn off these huge pages at boot-time/run-time? > > Only at compile time by not setting CONFIG_TRANSPARENT_HUGEPAGE. Any reason why echo never > /sys/kernel/mm/transparent_hugepage/enabled doesn't solve the problem? Also, I assume that disabling CONFIG_TRANSPARENT_HUGEPAGE will disable them for everything and not just what your patch added, right? >> I'm not sure what you mean by "We mitigated the problem by avoiding the >> slow coherent DMA code path on almost all platforms on newer kernels". I >> tested up to 4.16 and the performance regression is just as bad as it is >> for 4.15. > > Indeed 4.16 still doesn't have that. You could use the > amd-staging-drm-next branch or wait for 4.17. Is there a way to pull just that change or is there too much interactions with other changes? > That isn't related to the GFX hardware, but to your CPU/motherboard and > whatever else you have in the system. Well, I have an nvidia GPU in the same system (normally only used for CUDA) and if I use it instead of my RX 560 then I'm not seeing any performance issue with 4.15. > Some part of your system needs SWIOTLB and that makes allocating memory > much slower. What would that part be? FTR, I have a complete description of my system at https://jmvalin.dreamwidth.org/15583.html I don't know if it's related, but I can maybe see one thing in common between my machine and the Core 2 Quad from the other bug report and that's the "NUMA part". I have a dual-socket Xeon and (AFAIK) the Core 2 Quad is made of two two-core CPUs glued together with little communication between them. > Intel doesn't use TTM because they don't have dedicated VRAM, but the > open source nvidia driver should be affected as well. I'm using the proprietary nvidia driver (because CUDA). Is that supposed to be affected as well? > We already mitigated that problem and I don't see any solution which > will arrive faster than 4.17. Is that supposed to make the slowdown unnoticeable or just slightly better? > The only quick workaround I can see is to avoid firefox, chrome for > example is reported to work perfectly fine. Or use an unaffected GPU/driver ;-) Cheers, Jean-Marc ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 7/7] arm64: dts: renesas: eagle: Add ADV7511W and HDMI output
From: Niklas SöderlundEnable HDMI output adding the HDMI connector and the ADV7511W, connected to THC63LVD1024 LVDS decoder output. Signed-off-by: Niklas Söderlund Signed-off-by: Jacopo Mondi --- arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 51 +- 1 file changed, 50 insertions(+), 1 deletion(-) diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index 9d0e65d..e9f7b83 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts @@ -32,6 +32,17 @@ reg = <0x0 0x4800 0x0 0x3800>; }; + hdmi-out { + compatible = "hdmi-connector"; + type = "a"; + + port { + hdmi_con_out: endpoint { + remote-endpoint = <_out>; + }; + }; + }; + thc63lvd1024: lvds-decoder { compatible = "thine,thc63lvd1024"; @@ -41,11 +52,17 @@ port@0 { reg = <0>; - thc63lvd1024_in: endpoint { remote-endpoint = <_out>; }; }; + + port@2 { + reg = <2>; + thc63lvd1024_out: endpoint { + remote-endpoint = <_in>; + }; + }; }; }; }; @@ -85,6 +102,38 @@ gpio-controller; #gpio-cells = <2>; }; + + hdmi@39 { + compatible = "adi,adv7511w"; + reg = <0x39>; + interrupt-parent = <>; + interrupts = <20 IRQ_TYPE_LEVEL_LOW>; + + adi,input-depth = <8>; + adi,input-colorspace = "rgb"; + adi,input-clock = "1x"; + adi,input-style = <1>; + adi,input-justification = "evenly"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + adv7511_in: endpoint { + remote-endpoint = <_out>; + }; + }; + + port@1 { + reg = <1>; + adv7511_out: endpoint { + remote-endpoint = <_con_out>; + }; + }; + }; + }; }; { -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 0/2] drm: Add Thine THC63LVD1024 LVDS decoder bridge
Hello, this new version moves the driver and its bindings to use semi-standard names for powerdown and output enable GPIOs, as result of the discussion with Laurent, Vladimir and Rob. I kept the actual pin names in the bindings description for reference, even if there are no huge ambiguities on which chip pin is actually an enable and which one a power down. I have reworked the regulator management, making the 'vcc' supply the only requested one, and all other optional supplies have been removed as suggested by Laurent. It is unlikely a dedicated regulator is to be installed for each power supply, and in case some HW design requires this, it's an easy add to be implemented in future. Contrary to what discussed on v6, the 'vcc' supply is still described as optional in dt bindings, and the driver is now using 'regulator_get(NORMAL_GET)' in place of the _optional() version that was used before. With the 'NORMAL_GET' version the regulator core provides a dummy regulator in case an actual one is not available. This simplifies integration in designs where the chip power supplies are directly connected to some power rail. At the same time it makes easier to forget to add a regulator if there's actually one there, and someone could find herself wondering why the chip does not work even if probe completes properly. I removed the Eagle display enablement patch from the series, I'll send it separately squashed on top of Niklas' series that addresses the issue. Thanks j v6 -> v7: - Use semi-standard names for powerdown and output enable GPIOs as suggested by Rob and Vladimir - Use 'regulator_get()' not the optional version, and list only 'vcc' as requested supply - Addressed Laurent's review comments and removed Eagle display enablement patch to be sent separately v5 -> v6: - Drop check for CONFIG_OF as it is a Kconfig dependency - Add Niklas Reviewed-by tags - List [3/3] depenencies below commit message to ease integration v4 -> v5: - Fix punctuation in bindings documentation - Add small statement to bindings document to clarify the chip has no control bus - Print regulator name in enable/disable routines error path - Add Andrzej Reviewed-by tag v3 -> v4: - Rename permutations of "pdwn" to just "pdwn" everywhere in the series - Improve power enable/disable routines as suggested by Andrzej and Sergei - Change "pdwn" gpio initialization to use the logical output level - Change Kconfig description v2 -> v3: - Drop support for "lvds-decoder" and make the driver THC63LVD1024 specific -- Rework bindings to describe multiple input/output ports -- Rename driver and remove "lvds-decoder" references -- Rework Eagle DTS to use new bindings v1 -> v2: - Drop support for THC63LVD1024 Jacopo Mondi (2): dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder drm: bridge: Add thc63lvd1024 LVDS decoder driver .../bindings/display/bridge/thine,thc63lvd1024.txt | 60 ++ drivers/gpu/drm/bridge/Kconfig | 6 + drivers/gpu/drm/bridge/Makefile| 1 + drivers/gpu/drm/bridge/thc63lvd1024.c | 212 + 4 files changed, 279 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v7 2/2] drm: bridge: Add thc63lvd1024 LVDS decoder driver
Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel output converter. Signed-off-by: Jacopo MondiReviewed-by: Andrzej Hajda Reviewed-by: Niklas Söderlund --- drivers/gpu/drm/bridge/Kconfig| 6 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/thc63lvd1024.c | 212 ++ 3 files changed, 219 insertions(+) create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index 3aa65bd..42c9c2d 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -93,6 +93,12 @@ config DRM_SII9234 It is an I2C driver, that detects connection of MHL bridge and starts encapsulation of HDMI signal. +config DRM_THINE_THC63LVD1024 + tristate "Thine THC63LVD1024 LVDS decoder bridge" + depends on OF + ---help--- + Thine THC63LVD1024 LVDS/parallel converter driver. + config DRM_TOSHIBA_TC358767 tristate "Toshiba TC358767 eDP bridge" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 373eb28..fd90b16 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o obj-$(CONFIG_DRM_SII902X) += sii902x.o obj-$(CONFIG_DRM_SII9234) += sii9234.o +obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c b/drivers/gpu/drm/bridge/thc63lvd1024.c new file mode 100644 index 000..c8fad9c --- /dev/null +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c @@ -0,0 +1,212 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * THC63LVD1024 LVDS to parallel data DRM bridge driver. + * + * Copyright (C) 2018 Jacopo Mondi + */ + +#include +#include +#include + +#include +#include +#include + +enum thc63_ports { + THC63_LVDS_IN0, + THC63_LVDS_IN1, + THC63_RGB_OUT0, + THC63_RGB_OUT1, +}; + +struct thc63_dev { + struct device *dev; + + struct regulator *vcc; + + struct gpio_desc *pdwn; + struct gpio_desc *oe; + + struct drm_bridge bridge; + struct drm_bridge *next; +}; + +static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge) +{ + return container_of(bridge, struct thc63_dev, bridge); +} + +static int thc63_attach(struct drm_bridge *bridge) +{ + struct thc63_dev *thc63 = to_thc63(bridge); + + return drm_bridge_attach(bridge->encoder, thc63->next, bridge); +} + +static void thc63_enable(struct drm_bridge *bridge) +{ + struct thc63_dev *thc63 = to_thc63(bridge); + int ret; + + ret = regulator_enable(thc63->vcc); + if (ret) { + dev_err(thc63->dev, + "Failed to enable regulator \"vcc\": %d\n", ret); + return; + } + + if (thc63->pdwn) + gpiod_set_value(thc63->pdwn, 0); + + if (thc63->oe) + gpiod_set_value(thc63->oe, 1); +} + +static void thc63_disable(struct drm_bridge *bridge) +{ + struct thc63_dev *thc63 = to_thc63(bridge); + int ret; + + if (thc63->oe) + gpiod_set_value(thc63->oe, 0); + + if (thc63->pdwn) + gpiod_set_value(thc63->pdwn, 1); + + ret = regulator_disable(thc63->vcc); + if (ret) + dev_err(thc63->dev, + "Failed to disable regulator \"vcc\": %d\n", ret); +} + +static const struct drm_bridge_funcs thc63_bridge_func = { + .attach = thc63_attach, + .enable = thc63_enable, + .disable = thc63_disable, +}; + +static int thc63_parse_dt(struct thc63_dev *thc63) +{ + struct device_node *thc63_out; + struct device_node *remote; + + thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node, + THC63_RGB_OUT0, -1); + if (!thc63_out) { + dev_err(thc63->dev, "Missing endpoint in port@%u\n", + THC63_RGB_OUT0); + return -ENODEV; + } + + remote = of_graph_get_remote_port_parent(thc63_out); + of_node_put(thc63_out); + if (!remote) { + dev_err(thc63->dev, "Endpoint in port@%u unconnected\n", + THC63_RGB_OUT0); + return -ENODEV; + } + + if (!of_device_is_available(remote)) { + dev_err(thc63->dev, "port@%u remote endpoint is disabled\n", + THC63_RGB_OUT0); + of_node_put(remote); + return -ENODEV; + } + + thc63->next = of_drm_find_bridge(remote); +
[PATCH 2/7] arm64: dts: renesas: r8a77970: add VSPD support
From: Sergei ShtylyovDescribe VSPD0 in the R8A77970 device tree; it will be used by DU in the next patch... Based on the original (and large) patch by Daisuke Matsushita . Signed-off-by: Vladimir Barinov Signed-off-by: Sergei Shtylyov Signed-off-by: Niklas Söderlund --- arch/arm64/boot/dts/renesas/r8a77970.dtsi | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi b/arch/arm64/boot/dts/renesas/r8a77970.dtsi index 71f466d..db06c94 100644 --- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi +++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi @@ -625,6 +625,16 @@ power-domains = < R8A77970_PD_ALWAYS_ON>; resets = < 603>; }; + + vspd0: vsp@fea2 { + compatible = "renesas,vsp2"; + reg = <0 0xfea2 0 0x4000>; + interrupts = ; + clocks = < CPG_MOD 623>; + power-domains = < R8A77970_PD_ALWAYS_ON>; + resets = < 623>; + renesas,fcp = <>; + }; }; timer { -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer
On Fri, Apr 06, 2018 at 10:55:14PM +0530, Nautiyal, Ankit K wrote: > This patch is causing failure of IGT test kms_3d. The kms_3d test > expects the no. of 3d modes to be 13. > > (The test has hard-coded value for expected no. of 3d modes as 13) > > But due to the addition of "matching aspect_ratio" in drm_mode_equal in > this patch, the total no. of > > modes in the connector modelist is increased by 2, resulting in failure > of assertion 'mode_count==13'. If kms_3d isn't setting the aspect ratio cap how is it affected by these changes? > > Perhaps this need to be handled in the test. > > -Regards, > > Ankit > > > On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote: > > From: "Sharma, Shashank"> > > > Current DRM layer functions don't parse aspect ratio information > > while converting a user mode->kernel mode or vice versa. This > > causes modeset to pick mode with wrong aspect ratio, eventually > > causing failures in HDMI compliance test cases, due to wrong VIC. > > > > This patch adds aspect ratio information in DRM's mode conversion > > and mode comparision functions, to make sure kernel picks mode > > with right aspect ratio (as per the VIC). > > > > Background: > > This patch was once reviewed and merged, and later reverted due to > > lack of DRM cap protection. This is a re-spin of this patch, this > > time with DRM cap protection, to avoid aspect ratio information, when > > the client doesn't request for it. > > > > Review link: https://pw-emeril.freedesktop.org/patch/104068/ > > Background discussion: https://patchwork.kernel.org/patch/9379057/ > > > > Signed-off-by: Shashank Sharma > > Signed-off-by: Lin, Jia > > Signed-off-by: Akashdeep Sharma > > Reviewed-by: Jim Bride (V2) > > Reviewed-by: Jose Abreu (V4) > > > > Cc: Ville Syrjala > > Cc: Jim Bride > > Cc: Jose Abreu > > Cc: Ankit Nautiyal > > > > V3: modified the aspect-ratio check in drm_mode_equal as per new flags > > provided by Ville. https://patchwork.freedesktop.org/patch/188043/ > > V4: rebase > > V5: rebase > > V6: As recommended by Ville, avoided matching of aspect-ratio in > > drm_fb_helper, while trying to find a common mode among connectors > > for the target clone mode. > > V7: rebase > > V8: rebase > > V9: rebase > > V10: rebase > > --- > > drivers/gpu/drm/drm_fb_helper.c | 12 ++-- > > drivers/gpu/drm/drm_modes.c | 35 ++- > > 2 files changed, 44 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_fb_helper.c > > b/drivers/gpu/drm/drm_fb_helper.c > > index 0646b10..2ee1eaa 100644 > > --- a/drivers/gpu/drm/drm_fb_helper.c > > +++ b/drivers/gpu/drm/drm_fb_helper.c > > @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper > > *fb_helper, > > for (j = 0; j < i; j++) { > > if (!enabled[j]) > > continue; > > - if (!drm_mode_equal(modes[j], modes[i])) > > + if (!drm_mode_match(modes[j], modes[i], > > + DRM_MODE_MATCH_TIMINGS | > > + DRM_MODE_MATCH_CLOCK | > > + DRM_MODE_MATCH_FLAGS | > > + DRM_MODE_MATCH_3D_FLAGS)) > > can_clone = false; > > } > > } > > @@ -2203,7 +2207,11 @@ static bool drm_target_cloned(struct drm_fb_helper > > *fb_helper, > > > > fb_helper_conn = fb_helper->connector_info[i]; > > list_for_each_entry(mode, _helper_conn->connector->modes, > > head) { > > - if (drm_mode_equal(mode, dmt_mode)) > > + if (drm_mode_match(mode, dmt_mode, > > + DRM_MODE_MATCH_TIMINGS | > > + DRM_MODE_MATCH_CLOCK | > > + DRM_MODE_MATCH_FLAGS | > > + DRM_MODE_MATCH_3D_FLAGS)) > > modes[i] = mode; > > } > > if (!modes[i]) > > diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c > > index d6133e8..454f2ff 100644 > > --- a/drivers/gpu/drm/drm_modes.c > > +++ b/drivers/gpu/drm/drm_modes.c > > @@ -1049,7 +1049,8 @@ bool drm_mode_equal(const struct drm_display_mode > > *mode1, > > DRM_MODE_MATCH_TIMINGS | > > DRM_MODE_MATCH_CLOCK | > > DRM_MODE_MATCH_FLAGS | > > - DRM_MODE_MATCH_3D_FLAGS); > > + DRM_MODE_MATCH_3D_FLAGS| > > +
Re: [PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer
This patch is causing failure of IGT test kms_3d. The kms_3d test expects the no. of 3d modes to be 13. (The test has hard-coded value for expected no. of 3d modes as 13) But due to the addition of "matching aspect_ratio" in drm_mode_equal in this patch, the total no. of modes in the connector modelist is increased by 2, resulting in failure of assertion 'mode_count==13'. Perhaps this need to be handled in the test. -Regards, Ankit On 4/6/2018 10:34 PM, Nautiyal, Ankit K wrote: From: "Sharma, Shashank"Current DRM layer functions don't parse aspect ratio information while converting a user mode->kernel mode or vice versa. This causes modeset to pick mode with wrong aspect ratio, eventually causing failures in HDMI compliance test cases, due to wrong VIC. This patch adds aspect ratio information in DRM's mode conversion and mode comparision functions, to make sure kernel picks mode with right aspect ratio (as per the VIC). Background: This patch was once reviewed and merged, and later reverted due to lack of DRM cap protection. This is a re-spin of this patch, this time with DRM cap protection, to avoid aspect ratio information, when the client doesn't request for it. Review link: https://pw-emeril.freedesktop.org/patch/104068/ Background discussion: https://patchwork.kernel.org/patch/9379057/ Signed-off-by: Shashank Sharma Signed-off-by: Lin, Jia Signed-off-by: Akashdeep Sharma Reviewed-by: Jim Bride (V2) Reviewed-by: Jose Abreu (V4) Cc: Ville Syrjala Cc: Jim Bride Cc: Jose Abreu Cc: Ankit Nautiyal V3: modified the aspect-ratio check in drm_mode_equal as per new flags provided by Ville. https://patchwork.freedesktop.org/patch/188043/ V4: rebase V5: rebase V6: As recommended by Ville, avoided matching of aspect-ratio in drm_fb_helper, while trying to find a common mode among connectors for the target clone mode. V7: rebase V8: rebase V9: rebase V10: rebase --- drivers/gpu/drm/drm_fb_helper.c | 12 ++-- drivers/gpu/drm/drm_modes.c | 35 ++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0646b10..2ee1eaa 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper *fb_helper, for (j = 0; j < i; j++) { if (!enabled[j]) continue; - if (!drm_mode_equal(modes[j], modes[i])) + if (!drm_mode_match(modes[j], modes[i], + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_CLOCK | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS)) can_clone = false; } } @@ -2203,7 +2207,11 @@ static bool drm_target_cloned(struct drm_fb_helper *fb_helper, fb_helper_conn = fb_helper->connector_info[i]; list_for_each_entry(mode, _helper_conn->connector->modes, head) { - if (drm_mode_equal(mode, dmt_mode)) + if (drm_mode_match(mode, dmt_mode, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_CLOCK | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS)) modes[i] = mode; } if (!modes[i]) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index d6133e8..454f2ff 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1049,7 +1049,8 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_CLOCK | DRM_MODE_MATCH_FLAGS | - DRM_MODE_MATCH_3D_FLAGS); + DRM_MODE_MATCH_3D_FLAGS| + DRM_MODE_MATCH_ASPECT_RATIO); } EXPORT_SYMBOL(drm_mode_equal); @@ -1647,6 +1648,20 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, out->vrefresh = in->vrefresh; out->flags = in->flags; out->type = in->type; + + switch (in->picture_aspect_ratio) { + case HDMI_PICTURE_ASPECT_4_3: + out->flags |= DRM_MODE_FLAG_PIC_AR_4_3; + break; + case HDMI_PICTURE_ASPECT_16_9: +
Re: AMD graphics performance regression in 4.15 and later
Am 06.04.2018 um 18:42 schrieb Jean-Marc Valin: Hi Christian, On 04/09/2018 07:48 AM, Christian König wrote: Am 06.04.2018 um 17:30 schrieb Jean-Marc Valin: Hi Christian, Is there a way to turn off these huge pages at boot-time/run-time? Only at compile time by not setting CONFIG_TRANSPARENT_HUGEPAGE. Any reason why echo never > /sys/kernel/mm/transparent_hugepage/enabled doesn't solve the problem? Because we unfortunately try to allocate huge pages anyway, we unfortunately just fail in 100% of all cases. That basically gives you both, the extra allocation overhead and the still bad throughput. Also, I assume that disabling CONFIG_TRANSPARENT_HUGEPAGE will disable them for everything and not just what your patch added, right? Correct, that's why I wrote that disabling SWIOTLBs might be better. I'm not sure what you mean by "We mitigated the problem by avoiding the slow coherent DMA code path on almost all platforms on newer kernels". I tested up to 4.16 and the performance regression is just as bad as it is for 4.15. Indeed 4.16 still doesn't have that. You could use the amd-staging-drm-next branch or wait for 4.17. Is there a way to pull just that change or is there too much interactions with other changes? It adds a new detection if memory allocation needs to be coherent or not, that is not something you can easily pull into older versions. That isn't related to the GFX hardware, but to your CPU/motherboard and whatever else you have in the system. Well, I have an nvidia GPU in the same system (normally only used for CUDA) and if I use it instead of my RX 560 then I'm not seeing any performance issue with 4.15. That's because you are probably using the Nvidia binary driver which has a completely separate code base. Some part of your system needs SWIOTLB and that makes allocating memory much slower. What would that part be? FTR, I have a complete description of my system at https://jmvalin.dreamwidth.org/15583.html I don't know if it's related, but I can maybe see one thing in common between my machine and the Core 2 Quad from the other bug report and that's the "NUMA part". I have a dual-socket Xeon and (AFAIK) the Core 2 Quad is made of two two-core CPUs glued together with little communication between them. Yeah, that is probably the reason. Intel doesn't use TTM because they don't have dedicated VRAM, but the open source nvidia driver should be affected as well. I'm using the proprietary nvidia driver (because CUDA). Is that supposed to be affected as well? No. We already mitigated that problem and I don't see any solution which will arrive faster than 4.17. Is that supposed to make the slowdown unnoticeable or just slightly better? It completely goes away. The issue with the coherent path is that it tries to always allocate the lowest possible memory to make sure that it fits into the DMA constrains of all devices in the system. But since AMD GPU can handle 40bits of addresses you would need at least 1TB of memory in the system to trigger that (or a NUMA where some system is low and some in a high area). Christian. The only quick workaround I can see is to avoid firefox, chrome for example is reported to work perfectly fine. Or use an unaffected GPU/driver ;-) Cheers, Jean-Marc ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 11/11] drm: Add and handle new aspect ratios in DRM layer
From: "Sharma, Shashank"HDMI 2.0/CEA-861-F introduces two new aspect ratios: - 64:27 - 256:135 This patch: - Adds new DRM flags for to represent these new aspect ratios. - Adds new cases to handle these aspect ratios while converting from user->kernel mode or vise versa. This patch was once reviewed and merged, and later reverted due to lack of DRM client protection, while adding aspect ratio bits in user modes. This is a re-spin of the series, with DRM client cap protection. The previous series can be found here: https://pw-emeril.freedesktop.org/series/10850/ Signed-off-by: Shashank Sharma Reviewed-by: Sean Paul (V2) Reviewed-by: Jose Abreu (V2) Cc: Ville Syrjala Cc: Sean Paul Cc: Jose Abreu Cc: Ankit Nautiyal V3: rebase V4: rebase V5: corrected the macro name for an aspect ratio, in a switch case. V6: rebase V7: rebase V8: rebase V9: rebase V10: rebase --- drivers/gpu/drm/drm_modes.c | 12 include/uapi/drm/drm_mode.h | 6 ++ 2 files changed, 18 insertions(+) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index 454f2ff..21cc84b 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1656,6 +1656,12 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, case HDMI_PICTURE_ASPECT_16_9: out->flags |= DRM_MODE_FLAG_PIC_AR_16_9; break; + case HDMI_PICTURE_ASPECT_64_27: + out->flags |= DRM_MODE_FLAG_PIC_AR_64_27; + break; + case HDMI_PICTURE_ASPECT_256_135: + out->flags |= DRM_MODE_FLAG_PIC_AR_256_135; + break; case HDMI_PICTURE_ASPECT_RESERVED: default: out->flags |= DRM_MODE_FLAG_PIC_AR_NONE; @@ -1721,6 +1727,12 @@ int drm_mode_convert_umode(struct drm_device *dev, case DRM_MODE_FLAG_PIC_AR_16_9: out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_16_9; break; + case DRM_MODE_FLAG_PIC_AR_64_27: + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_64_27; + break; + case DRM_MODE_FLAG_PIC_AR_256_135: + out->picture_aspect_ratio |= HDMI_PICTURE_ASPECT_256_135; + break; default: out->picture_aspect_ratio = HDMI_PICTURE_ASPECT_NONE; break; diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 50bcf42..4b3a1bb 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -93,6 +93,8 @@ extern "C" { #define DRM_MODE_PICTURE_ASPECT_NONE 0 #define DRM_MODE_PICTURE_ASPECT_4_31 #define DRM_MODE_PICTURE_ASPECT_16_9 2 +#define DRM_MODE_PICTURE_ASPECT_64_27 3 +#define DRM_MODE_PICTURE_ASPECT_256_1354 /* Aspect ratio flag bitmask (4 bits 22:19) */ #define DRM_MODE_FLAG_PIC_AR_MASK (0x0F<<19) @@ -102,6 +104,10 @@ extern "C" { (DRM_MODE_PICTURE_ASPECT_4_3<<19) #define DRM_MODE_FLAG_PIC_AR_16_9 \ (DRM_MODE_PICTURE_ASPECT_16_9<<19) +#define DRM_MODE_FLAG_PIC_AR_64_27 \ + (DRM_MODE_PICTURE_ASPECT_64_27<<19) +#define DRM_MODE_FLAG_PIC_AR_256_135 \ + (DRM_MODE_PICTURE_ASPECT_256_135<<19) #define DRM_MODE_FLAG_ALL (DRM_MODE_FLAG_PHSYNC | \ DRM_MODE_FLAG_NHSYNC | \ -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 07/11] drm: Add helper functions to handle aspect-ratio flag bits
From: Ankit NautiyalThis patch adds helper functions for determining if aspect-ratio is expected in user-mode and for allowing/disallowing the aspect-ratio, if its not expected. Signed-off-by: Ankit Nautiyal --- drivers/gpu/drm/drm_modes.c | 47 + include/drm/drm_modes.h | 4 2 files changed, 51 insertions(+) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index c395a24..d6133e8 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1759,3 +1759,50 @@ bool drm_mode_is_420(const struct drm_display_info *display, drm_mode_is_420_also(display, mode); } EXPORT_SYMBOL(drm_mode_is_420); + +/** + * drm_mode_aspect_ratio_allowed - checks if the aspect-ratio information + * is expected from the user-mode. + * + * If the user has set aspect-ratio cap, then the flag of the user-mode is + * allowed to contain aspect-ratio value. + * If the user does not set aspect-ratio cap, then the only value allowed in the + * flags bits is aspect-ratio NONE. + * + * @file_priv: file private structure to get the user capabilities + * @umode: drm_mode_modeinfo struct, whose flag carry the aspect ratio + * information. + * + * Returns: + * true if the aspect-ratio info is allowed in the user-mode flags. + * false, otherwise. + */ +bool +drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv, + struct drm_mode_modeinfo *umode) +{ + return file_priv->aspect_ratio_allowed || (umode->flags & + DRM_MODE_FLAG_PIC_AR_MASK) == DRM_MODE_FLAG_PIC_AR_NONE; +} +EXPORT_SYMBOL(drm_mode_aspect_ratio_allowed); + +/** + * drm_mode_filter_aspect_ratio_flags - filters the aspect-ratio bits in the + * user-mode flags. + * + * Checks if the aspect-ratio information is allowed. Resets the aspect-ratio + * bits in the user-mode flags, if aspect-ratio info is not allowed. + * + * @file_priv: file private structure to get the user capabilities. + * @umode: drm_mode_modeinfo struct, whose flags' aspect-ratio bits needs to + * be filtered. + * + */ +void +drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv, + struct drm_mode_modeinfo *umode) +{ + if (!drm_mode_aspect_ratio_allowed(file_priv, umode)) + umode->flags &= ~DRM_MODE_FLAG_PIC_AR_MASK; +} +EXPORT_SYMBOL(drm_mode_filter_aspect_ratio_flags); diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 2f78b7e..e0b060d 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -461,6 +461,10 @@ bool drm_mode_is_420_also(const struct drm_display_info *display, const struct drm_display_mode *mode); bool drm_mode_is_420(const struct drm_display_info *display, const struct drm_display_mode *mode); +bool drm_mode_aspect_ratio_allowed(const struct drm_file *file_priv, + struct drm_mode_modeinfo *umode); +void drm_mode_filter_aspect_ratio_flags(const struct drm_file *file_priv, + struct drm_mode_modeinfo *umode); struct drm_display_mode *drm_cvt_mode(struct drm_device *dev, int hdisplay, int vdisplay, int vrefresh, -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 08/11] drm: Handle aspect ratio info in legacy and atomic modeset paths
From: Ankit NautiyalIf the user-space does not support aspect-ratio, and requests for a modeset with mode having aspect ratio bits set, then the given user-mode must be rejected. Secondly, while preparing a user-mode from kernel mode, the aspect-ratio info must not be given, if aspect-ratio is not supported by the user. Note: In case, a user-space asks for a video-mode blob, from the getblob ioctl, the aspect-ratio bits in the video-mode blob are passed to the user as it is, without any filtering. However, no such case is present in most of the atomic user-spaces. Currently atomic path of Xserver, Wayland/weston, Hardware-Composer are checked, and none of them are using getblob ioctl to get the video-mode blob. This patch: 1. passes the file_priv structure from the drm_mode_atomic_ioctl till the drm_mode_crtc_set_mode_prop, to get the user capability. 2. rejects the modes with aspect-ratio info, during modeset, if the user does not support aspect ratio. 3. does not load the aspect-ratio info in user-mode structure, if aspect ratio is not supported. Signed-off-by: Ankit Nautiyal V3: Addressed review comments from Ville: Do not corrupt the current crtc state by updating aspect-ratio on the fly. V4: rebase V5: As suggested by Ville, rejected the modeset calls for modes with aspect ratio, if the user does not set aspect-ratio cap. V6: Used the helper functions for determining if aspect-ratio is expected in the user-mode. V7: rebase V8: rebase V9: rebase v10: Modified the commit-message --- drivers/gpu/drm/drm_atomic.c| 34 +- drivers/gpu/drm/drm_atomic_helper.c | 6 +++--- drivers/gpu/drm/drm_crtc.c | 8 drivers/gpu/drm/drm_crtc_internal.h | 3 ++- drivers/gpu/drm/drm_mode_object.c | 9 ++--- include/drm/drm_atomic.h| 5 +++-- 6 files changed, 47 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 7d25c42..5863072 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -368,6 +368,7 @@ EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc); * drm_atomic_set_mode_prop_for_crtc - set mode for CRTC * @state: the CRTC whose incoming state to update * @blob: pointer to blob property to use for mode + * @file_priv: file priv structure, to get the userspace capabilities * * Set a mode (originating from a blob property) on the desired CRTC state. * This function will take a reference on the blob property for the CRTC state, @@ -378,7 +379,8 @@ EXPORT_SYMBOL(drm_atomic_set_mode_for_crtc); * Zero on success, error code on failure. Cannot return -EDEADLK. */ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, - struct drm_property_blob *blob) + struct drm_property_blob *blob, + struct drm_file *file_priv) { if (blob == state->mode_blob) return 0; @@ -389,9 +391,21 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state, memset(>mode, 0, sizeof(state->mode)); if (blob) { - if (blob->length != sizeof(struct drm_mode_modeinfo) || - drm_mode_convert_umode(state->crtc->dev, >mode, - blob->data)) + struct drm_mode_modeinfo *u_mode; + + if (blob->length != sizeof(struct drm_mode_modeinfo)) + return -EINVAL; + + u_mode = (struct drm_mode_modeinfo *) blob->data; + if (!drm_mode_aspect_ratio_allowed(file_priv, + u_mode)) { + DRM_DEBUG_ATOMIC("Unexpected aspect-ratio flag bits\n"); + return -EINVAL; + } + + if (drm_mode_convert_umode(state->crtc->dev, >mode, + (const struct drm_mode_modeinfo *) + u_mode)) return -EINVAL; state->mode_blob = drm_property_blob_get(blob); @@ -471,6 +485,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev, * @state: the state object to update with the new property value * @property: the property to set * @val: the new property value + * @file_priv: the file private structure, to get the user capabilities * * This function handles generic/core properties and calls out to driver's * _crtc_funcs.atomic_set_property for driver properties. To ensure @@ -482,7 +497,7 @@ drm_atomic_replace_property_blob_from_id(struct drm_device *dev, */ int drm_atomic_crtc_set_property(struct drm_crtc *crtc, struct drm_crtc_state *state, struct drm_property *property, - uint64_t val) + uint64_t val, struct drm_file
[PATCH v10 09/11] drm: Expose modes with aspect ratio, only if requested
From: Ankit NautiyalWe parse the EDID and add all the modes in the connector's modelist. This adds CEA modes with aspect ratio information too, regadless of whether user space requested this information or not. This patch prunes the modes with aspect-ratio information, from a connector's modelist, if the user-space has not set the aspect ratio DRM client cap. Cc: Ville Syrjala Cc: Shashank Sharma Cc: Jose Abreu Signed-off-by: Ankit Nautiyal V3: As suggested by Ville, modified the mechanism of pruning of modes with aspect-ratio, if the aspect-ratio is not supported. Instead of straight away pruning such a mode, the mode is retained with aspect ratio bits set to zero, provided it is unique. V4: rebase V5: Addressed review comments from Ville: -used a pointer to store last valid mode. -avoided, modifying of picture_aspect_ratio in kernel mode, instead only flags bits of user mode are reset (if aspect-ratio is not supported). V6: As suggested by Ville, corrected the mode pruning logic and elaborated the mode pruning logic and the assumptions taken. V7: rebase V8: rebase V9: rebase V10: rebase --- drivers/gpu/drm/drm_connector.c | 40 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c index b3cde89..5420325 100644 --- a/drivers/gpu/drm/drm_connector.c +++ b/drivers/gpu/drm/drm_connector.c @@ -1531,8 +1531,10 @@ static struct drm_encoder *drm_connector_get_encoder(struct drm_connector *conne return connector->encoder; } -static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, -const struct drm_file *file_priv) +static bool +drm_mode_expose_to_userspace(const struct drm_display_mode *mode, +const struct drm_display_mode *last_mode, +const struct drm_file *file_priv) { /* * If user-space hasn't configured the driver to expose the stereo 3D @@ -1540,6 +1542,26 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode, */ if (!file_priv->stereo_allowed && drm_mode_is_stereo(mode)) return false; + /* +* If user-space hasn't configured the driver to expose the modes with +* aspect-ratio, don't expose them. But in case of a unique mode, let +* the mode be passed, so that it can be enumerated with aspect-ratio +* bits erased. +* +* It is assumed here, that the list of modes for a given connector, is +* sorted, such that modes that have different aspect-ratios, but are +* otherwise identical, are back to back. +* This way, saving the last valid mode, and matching it with the +* current mode will help in determining, if the current mode is unique. +*/ + if (!file_priv->aspect_ratio_allowed && + mode->picture_aspect_ratio != HDMI_PICTURE_ASPECT_NONE && + last_mode && drm_mode_match(mode, last_mode, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_CLOCK | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS)) + return false; return true; } @@ -1551,6 +1573,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, struct drm_connector *connector; struct drm_encoder *encoder; struct drm_display_mode *mode; + struct drm_display_mode *last_valid_mode; int mode_count = 0; int encoders_count = 0; int ret = 0; @@ -1606,9 +1629,13 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, out_resp->connection = connector->status; /* delayed so we get modes regardless of pre-fill_modes state */ + last_valid_mode = NULL; list_for_each_entry(mode, >modes, head) - if (drm_mode_expose_to_userspace(mode, file_priv)) + if (drm_mode_expose_to_userspace(mode, last_valid_mode, +file_priv)) { mode_count++; + last_valid_mode = mode; + } /* * This ioctl is called twice, once to determine how much space is @@ -1617,11 +1644,15 @@ int drm_mode_getconnector(struct drm_device *dev, void *data, if ((out_resp->count_modes >= mode_count) && mode_count) { copied = 0; mode_ptr = (struct drm_mode_modeinfo __user *)(unsigned long)out_resp->modes_ptr; + last_valid_mode = NULL; list_for_each_entry(mode, >modes, head) { - if
[PATCH v10 10/11] drm: Add aspect ratio parsing in DRM layer
From: "Sharma, Shashank"Current DRM layer functions don't parse aspect ratio information while converting a user mode->kernel mode or vice versa. This causes modeset to pick mode with wrong aspect ratio, eventually causing failures in HDMI compliance test cases, due to wrong VIC. This patch adds aspect ratio information in DRM's mode conversion and mode comparision functions, to make sure kernel picks mode with right aspect ratio (as per the VIC). Background: This patch was once reviewed and merged, and later reverted due to lack of DRM cap protection. This is a re-spin of this patch, this time with DRM cap protection, to avoid aspect ratio information, when the client doesn't request for it. Review link: https://pw-emeril.freedesktop.org/patch/104068/ Background discussion: https://patchwork.kernel.org/patch/9379057/ Signed-off-by: Shashank Sharma Signed-off-by: Lin, Jia Signed-off-by: Akashdeep Sharma Reviewed-by: Jim Bride (V2) Reviewed-by: Jose Abreu (V4) Cc: Ville Syrjala Cc: Jim Bride Cc: Jose Abreu Cc: Ankit Nautiyal V3: modified the aspect-ratio check in drm_mode_equal as per new flags provided by Ville. https://patchwork.freedesktop.org/patch/188043/ V4: rebase V5: rebase V6: As recommended by Ville, avoided matching of aspect-ratio in drm_fb_helper, while trying to find a common mode among connectors for the target clone mode. V7: rebase V8: rebase V9: rebase V10: rebase --- drivers/gpu/drm/drm_fb_helper.c | 12 ++-- drivers/gpu/drm/drm_modes.c | 35 ++- 2 files changed, 44 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c index 0646b10..2ee1eaa 100644 --- a/drivers/gpu/drm/drm_fb_helper.c +++ b/drivers/gpu/drm/drm_fb_helper.c @@ -2183,7 +2183,11 @@ static bool drm_target_cloned(struct drm_fb_helper *fb_helper, for (j = 0; j < i; j++) { if (!enabled[j]) continue; - if (!drm_mode_equal(modes[j], modes[i])) + if (!drm_mode_match(modes[j], modes[i], + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_CLOCK | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS)) can_clone = false; } } @@ -2203,7 +2207,11 @@ static bool drm_target_cloned(struct drm_fb_helper *fb_helper, fb_helper_conn = fb_helper->connector_info[i]; list_for_each_entry(mode, _helper_conn->connector->modes, head) { - if (drm_mode_equal(mode, dmt_mode)) + if (drm_mode_match(mode, dmt_mode, + DRM_MODE_MATCH_TIMINGS | + DRM_MODE_MATCH_CLOCK | + DRM_MODE_MATCH_FLAGS | + DRM_MODE_MATCH_3D_FLAGS)) modes[i] = mode; } if (!modes[i]) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index d6133e8..454f2ff 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -1049,7 +1049,8 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_CLOCK | DRM_MODE_MATCH_FLAGS | - DRM_MODE_MATCH_3D_FLAGS); + DRM_MODE_MATCH_3D_FLAGS| + DRM_MODE_MATCH_ASPECT_RATIO); } EXPORT_SYMBOL(drm_mode_equal); @@ -1647,6 +1648,20 @@ void drm_mode_convert_to_umode(struct drm_mode_modeinfo *out, out->vrefresh = in->vrefresh; out->flags = in->flags; out->type = in->type; + + switch (in->picture_aspect_ratio) { + case HDMI_PICTURE_ASPECT_4_3: + out->flags |= DRM_MODE_FLAG_PIC_AR_4_3; + break; + case HDMI_PICTURE_ASPECT_16_9: + out->flags |= DRM_MODE_FLAG_PIC_AR_16_9; + break; + case HDMI_PICTURE_ASPECT_RESERVED: + default: + out->flags |= DRM_MODE_FLAG_PIC_AR_NONE; + break; + } + strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN); out->name[DRM_DISPLAY_MODE_LEN-1] = 0; } @@ -1693,6 +1708,24 @@ int drm_mode_convert_umode(struct drm_device *dev, strncpy(out->name, in->name, DRM_DISPLAY_MODE_LEN);
[PATCH v10 04/11] drm/edid: Don't send bogus aspect ratios in AVI infoframes
From: Ville SyrjäläIf the user mode would specify an aspect ratio other than 4:3 or 16:9 we now silently ignore it. Maybe a better apporoach is to return an error? Let's try that. Also we must be careful that we don't try to send illegal picture aspect in the infoframe as it's only capable of signalling none, 4:3, and 16:9. Currently we're sending these bogus infoframes whenever the cea mode specifies some other aspect ratio. Cc: Shashank Sharma Cc: Sean Paul Cc: Jose Abreu Cc: Daniel Vetter Cc: Emil Velikov Signed-off-by: Ville Syrjälä Reviewed-by: Shashank Sharma --- drivers/gpu/drm/drm_edid.c | 23 +-- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 29c88eb..d5757aa 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -4840,6 +4840,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, const struct drm_display_mode *mode, bool is_hdmi2_sink) { + enum hdmi_picture_aspect picture_aspect; int err; if (!frame || !mode) @@ -4882,13 +4883,23 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame, * Populate picture aspect ratio from either * user input (if specified) or from the CEA mode list. */ - if (mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_4_3 || - mode->picture_aspect_ratio == HDMI_PICTURE_ASPECT_16_9) - frame->picture_aspect = mode->picture_aspect_ratio; - else if (frame->video_code > 0) - frame->picture_aspect = drm_get_cea_aspect_ratio( - frame->video_code); + picture_aspect = mode->picture_aspect_ratio; + if (picture_aspect == HDMI_PICTURE_ASPECT_NONE) + picture_aspect = drm_get_cea_aspect_ratio(frame->video_code); + /* +* The infoframe can't convey anything but none, 4:3 +* and 16:9, so if the user has asked for anything else +* we can only satisfy it by specifying the right VIC. +*/ + if (picture_aspect > HDMI_PICTURE_ASPECT_16_9) { + if (picture_aspect != + drm_get_cea_aspect_ratio(frame->video_code)) + return -EINVAL; + picture_aspect = HDMI_PICTURE_ASPECT_NONE; + } + + frame->picture_aspect = picture_aspect; frame->active_aspect = HDMI_ACTIVE_ASPECT_PICTURE; frame->scan_mode = HDMI_SCAN_MODE_UNDERSCAN; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 06/11] drm: Add DRM client cap for aspect-ratio
From: Ankit NautiyalTo enable aspect-ratio support in DRM, blindly exposing the aspect ratio information along with mode, can break things in existing user-spaces which have no intention or support to use this aspect ratio information. To avoid this, a new drm client cap is required to enable a user-space to advertise if it supports modes with aspect-ratio. Based on this cap value, the kernel will take a call on exposing the aspect ratio info in modes or not. This patch adds the client cap for aspect-ratio. Cc: Ville Syrjala Cc: Shashank Sharma Signed-off-by: Ankit Nautiyal V3: rebase V4: As suggested by Marteen Lankhorst modified the commit message explaining the need to use the DRM cap for aspect-ratio. Also, tweaked the comment lines in the code for better understanding and clarity, as recommended by Shashank Sharma. V5: rebase V6: rebase V7: rebase V8: rebase V9: rebase V10: added comment explaining that no userspace breaks on aspect-ratio mode bits. Reviewed-by: Shashank Sharma --- drivers/gpu/drm/drm_ioctl.c | 9 + include/drm/drm_file.h | 8 include/uapi/drm/drm.h | 7 +++ 3 files changed, 24 insertions(+) diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index af78291..39c8eab 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -325,6 +325,15 @@ drm_setclientcap(struct drm_device *dev, void *data, struct drm_file *file_priv) file_priv->atomic = req->value; file_priv->universal_planes = req->value; break; + case DRM_CLIENT_CAP_ASPECT_RATIO: + if (req->value > 1) + return -EINVAL; + /* +* No Atomic userspace blows up on aspect ratio mode bits. Checked in +* wayland/weston, xserver, and hardware-composer modeset paths. +*/ + file_priv->aspect_ratio_allowed = req->value; + break; default: return -EINVAL; } diff --git a/include/drm/drm_file.h b/include/drm/drm_file.h index 5176c37..02b7dde 100644 --- a/include/drm/drm_file.h +++ b/include/drm/drm_file.h @@ -182,6 +182,14 @@ struct drm_file { unsigned atomic:1; /** +* @aspect_ratio_allowed: +* +* True, if client can handle picture aspect ratios, and has requested +* to pass this information along with the mode. +*/ + unsigned aspect_ratio_allowed:1; + + /** * @is_master: * * This client is the creator of @master. Protected by struct diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 6fdff59..9c660e1 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -680,6 +680,13 @@ struct drm_get_cap { */ #define DRM_CLIENT_CAP_ATOMIC 3 +/** + * DRM_CLIENT_CAP_ASPECT_RATIO + * + * If set to 1, the DRM core will provide aspect ratio information in modes. + */ +#define DRM_CLIENT_CAP_ASPECT_RATIO4 + /** DRM_IOCTL_SET_CLIENT_CAP ioctl argument type */ struct drm_set_client_cap { __u64 capability; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 02/11] drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy
From: Ville SyrjäläUse drm_mode_equal_no_clocks_no_stereo() in drm_match_hdmi_mode_clock_tolerance() for consistency as we also use it in drm_match_hdmi_mode() and the cea mode matching functions. This doesn't actually change anything since the input mode comes from detailed timings and we match it against edid_4k_modes[] which. So none of those modes can have stereo flags set. Signed-off-by: Ville Syrjälä Reviewed-by: Shashank Sharma --- drivers/gpu/drm/drm_edid.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index 134069f..c35d3bc 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -3047,7 +3047,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ abs(to_match->clock - clock2) > clock_tolerance) continue; - if (drm_mode_equal_no_clocks(to_match, hdmi_mode)) + if (drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) return vic; } -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 03/11] drm/edid: Fix cea mode aspect ratio handling
From: Ville Syrjäläcommit 6dffd431e229 ("drm: Add aspect ratio parsing in DRM layer") cause us to not send out any VICs in the AVI infoframes. That commit was since reverted, but if and when we add aspect ratio handing back we need to be more careful. Let's handle this by considering the aspect ratio as a requirement for cea mode matching only if the passed in mode actually has a non-zero aspect ratio field. This will keep userspace that doesn't provide an aspect ratio working as before by matching it to the first otherwise equal cea mode. And once userspace starts to provide the aspect ratio it will be considerd a hard requirement for the match. Also change the hdmi mode matching to use drm_mode_match() for consistency, but we don't match on aspect ratio there since the spec doesn't list a specific aspect ratio for those modes. Cc: Shashank Sharma Cc: "Lin, Jia" Cc: Akashdeep Sharma Cc: Jim Bride Cc: Jose Abreu Cc: Daniel Vetter Cc: Emil Velikov Signed-off-by: Ville Syrjälä Reviewed-by: Shashank Sharma --- drivers/gpu/drm/drm_edid.c | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c index c35d3bc..29c88eb 100644 --- a/drivers/gpu/drm/drm_edid.c +++ b/drivers/gpu/drm/drm_edid.c @@ -2930,11 +2930,15 @@ cea_mode_alternate_timings(u8 vic, struct drm_display_mode *mode) static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS; u8 vic; if (!to_match->clock) return 0; + if (to_match->picture_aspect_ratio) + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { struct drm_display_mode cea_mode = edid_cea_modes[vic]; unsigned int clock1, clock2; @@ -2948,7 +2952,7 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m continue; do { - if (drm_mode_equal_no_clocks_no_stereo(to_match, _mode)) + if (drm_mode_match(to_match, _mode, match_flags)) return vic; } while (cea_mode_alternate_timings(vic, _mode)); } @@ -2965,11 +2969,15 @@ static u8 drm_match_cea_mode_clock_tolerance(const struct drm_display_mode *to_m */ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) { + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS; u8 vic; if (!to_match->clock) return 0; + if (to_match->picture_aspect_ratio) + match_flags |= DRM_MODE_MATCH_ASPECT_RATIO; + for (vic = 1; vic < ARRAY_SIZE(edid_cea_modes); vic++) { struct drm_display_mode cea_mode = edid_cea_modes[vic]; unsigned int clock1, clock2; @@ -2983,7 +2991,7 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match) continue; do { - if (drm_mode_equal_no_clocks_no_stereo(to_match, _mode)) + if (drm_mode_match(to_match, _mode, match_flags)) return vic; } while (cea_mode_alternate_timings(vic, _mode)); } @@ -3030,6 +3038,7 @@ hdmi_mode_alternate_clock(const struct drm_display_mode *hdmi_mode) static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_match, unsigned int clock_tolerance) { + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS; u8 vic; if (!to_match->clock) @@ -3047,7 +3056,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ abs(to_match->clock - clock2) > clock_tolerance) continue; - if (drm_mode_equal_no_clocks_no_stereo(to_match, hdmi_mode)) + if (drm_mode_match(to_match, hdmi_mode, match_flags)) return vic; } @@ -3064,6 +3073,7 @@ static u8 drm_match_hdmi_mode_clock_tolerance(const struct drm_display_mode *to_ */ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) { + unsigned int match_flags = DRM_MODE_MATCH_TIMINGS | DRM_MODE_MATCH_FLAGS; u8 vic; if (!to_match->clock) @@ -3079,7 +3089,7 @@ static u8 drm_match_hdmi_mode(const struct drm_display_mode *to_match) if
[PATCH v10 05/11] video/hdmi: Reject illegal picture aspect ratios
From: Ville SyrjäläAVI infoframe can only carry none, 4:3, or 16:9 picture aspect ratios. Return an error if the user asked for something different. Cc: Shashank Sharma Cc: "Lin, Jia" Cc: Akashdeep Sharma Cc: Jim Bride Cc: Jose Abreu Cc: Daniel Vetter Cc: Emil Velikov Cc: Thierry Reding Cc: Hans Verkuil Cc: linux-me...@vger.kernel.org Signed-off-by: Ville Syrjälä Reviewed-by: Jose Abreu --- drivers/video/hdmi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/video/hdmi.c b/drivers/video/hdmi.c index 111a0ab..38716eb5 100644 --- a/drivers/video/hdmi.c +++ b/drivers/video/hdmi.c @@ -93,6 +93,9 @@ ssize_t hdmi_avi_infoframe_pack(struct hdmi_avi_infoframe *frame, void *buffer, if (size < length) return -ENOSPC; + if (frame->picture_aspect > HDMI_PICTURE_ASPECT_16_9) + return -EINVAL; + memset(buffer, 0, size); ptr[0] = frame->type; -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v10 01/11] drm/modes: Introduce drm_mode_match()
From: Ville SyrjäläMake mode matching less confusing by allowing the caller to specify which parts of the modes should match via some flags. Signed-off-by: Ville Syrjälä Reviewed-by: Shashank Sharma --- drivers/gpu/drm/drm_modes.c | 134 ++-- include/drm/drm_modes.h | 9 +++ 2 files changed, 112 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/drm_modes.c b/drivers/gpu/drm/drm_modes.c index e82b61e..c395a24 100644 --- a/drivers/gpu/drm/drm_modes.c +++ b/drivers/gpu/drm/drm_modes.c @@ -939,17 +939,68 @@ struct drm_display_mode *drm_mode_duplicate(struct drm_device *dev, } EXPORT_SYMBOL(drm_mode_duplicate); +static bool drm_mode_match_timings(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return mode1->hdisplay == mode2->hdisplay && + mode1->hsync_start == mode2->hsync_start && + mode1->hsync_end == mode2->hsync_end && + mode1->htotal == mode2->htotal && + mode1->hskew == mode2->hskew && + mode1->vdisplay == mode2->vdisplay && + mode1->vsync_start == mode2->vsync_start && + mode1->vsync_end == mode2->vsync_end && + mode1->vtotal == mode2->vtotal && + mode1->vscan == mode2->vscan; +} + +static bool drm_mode_match_clock(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + /* +* do clock check convert to PICOS +* so fb modes get matched the same +*/ + if (mode1->clock && mode2->clock) + return KHZ2PICOS(mode1->clock) == KHZ2PICOS(mode2->clock); + else + return mode1->clock == mode2->clock; +} + +static bool drm_mode_match_flags(const struct drm_display_mode *mode1, +const struct drm_display_mode *mode2) +{ + return (mode1->flags & ~DRM_MODE_FLAG_3D_MASK) == + (mode2->flags & ~DRM_MODE_FLAG_3D_MASK); +} + +static bool drm_mode_match_3d_flags(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return (mode1->flags & DRM_MODE_FLAG_3D_MASK) == + (mode2->flags & DRM_MODE_FLAG_3D_MASK); +} + +static bool drm_mode_match_aspect_ratio(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2) +{ + return mode1->picture_aspect_ratio == mode2->picture_aspect_ratio; +} + /** - * drm_mode_equal - test modes for equality + * drm_mode_match - test modes for (partial) equality * @mode1: first mode * @mode2: second mode + * @match_flags: which parts need to match (DRM_MODE_MATCH_*) * * Check to see if @mode1 and @mode2 are equivalent. * * Returns: - * True if the modes are equal, false otherwise. + * True if the modes are (partially) equal, false otherwise. */ -bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_display_mode *mode2) +bool drm_mode_match(const struct drm_display_mode *mode1, + const struct drm_display_mode *mode2, + unsigned int match_flags) { if (!mode1 && !mode2) return true; @@ -957,15 +1008,48 @@ bool drm_mode_equal(const struct drm_display_mode *mode1, const struct drm_displ if (!mode1 || !mode2) return false; - /* do clock check convert to PICOS so fb modes get matched -* the same */ - if (mode1->clock && mode2->clock) { - if (KHZ2PICOS(mode1->clock) != KHZ2PICOS(mode2->clock)) - return false; - } else if (mode1->clock != mode2->clock) + if (match_flags & DRM_MODE_MATCH_TIMINGS && + !drm_mode_match_timings(mode1, mode2)) return false; - return drm_mode_equal_no_clocks(mode1, mode2); + if (match_flags & DRM_MODE_MATCH_CLOCK && + !drm_mode_match_clock(mode1, mode2)) + return false; + + if (match_flags & DRM_MODE_MATCH_FLAGS && + !drm_mode_match_flags(mode1, mode2)) + return false; + + if (match_flags & DRM_MODE_MATCH_3D_FLAGS && + !drm_mode_match_3d_flags(mode1, mode2)) + return false; + + if (match_flags & DRM_MODE_MATCH_ASPECT_RATIO && + !drm_mode_match_aspect_ratio(mode1, mode2)) + return false; + + return true; +} +EXPORT_SYMBOL(drm_mode_match); + +/** + * drm_mode_equal - test modes for equality + * @mode1: first mode + * @mode2: second mode + * + * Check to see if @mode1 and @mode2 are equivalent. + * + * Returns: + * True if the modes are equal, false otherwise. + */ +bool drm_mode_equal(const struct drm_display_mode *mode1, + const struct drm_display_mode
[PATCH v10 00/11] Aspect ratio support in DRM layer
From: Ankit NautiyalThis patch series is a re-attempt to enable aspect ratio support in DRM layer. Currently the aspect ratio information gets lost in translation during a user->kernel mode or vice versa. The old patch series (https://pw-emeril.freedesktop.org/series/10850/) had 4 patches, out of which 2 patches were reverted due to lack of drm client protection while loading the aspect information. This patch series also includes 5 patches from Ville Syrjälä's series for 'Info-frame cleanup and fixes': https://patchwork.freedesktop.org/series/33730/ which fixes the mode matching mechanism via flags, and also ensures that no bogus aspect-ratios are sent in the AVI infoframes. This patch series, adds a DRM client option for aspect ratio, and loads aspect ratio flags, only when the client sets this cap. To test this patch, the testdiplay IGT test is modified to have an option to do a modeset with only aspect ratio modes. Also, there is a userspace implementation in Wayland/weston layer: https://patchwork.freedesktop.org/patch/188125/ (Which is already ACK'ed by wayland community.) This, helps us in passing HDMI compliance test cases like 7-27, where the test equipment applies a CEA mode, and expects the exact VIC in the AVI infoframes. Ankit Nautiyal (4): drm: Add DRM client cap for aspect-ratio drm: Add helper functions to handle aspect-ratio flag bits drm: Handle aspect ratio info in legacy and atomic modeset paths drm: Expose modes with aspect ratio, only if requested Sharma, Shashank (2): drm: Add aspect ratio parsing in DRM layer drm: Add and handle new aspect ratios in DRM layer Ville Syrjälä (5): drm/modes: Introduce drm_mode_match() drm/edid: Use drm_mode_match_no_clocks_no_stereo() for consistentcy drm/edid: Fix cea mode aspect ratio handling drm/edid: Don't send bogus aspect ratios in AVI infoframes video/hdmi: Reject illegal picture aspect ratios drivers/gpu/drm/drm_atomic.c| 34 -- drivers/gpu/drm/drm_atomic_helper.c | 6 +- drivers/gpu/drm/drm_connector.c | 40 ++- drivers/gpu/drm/drm_crtc.c | 8 ++ drivers/gpu/drm/drm_crtc_internal.h | 3 +- drivers/gpu/drm/drm_edid.c | 41 +-- drivers/gpu/drm/drm_fb_helper.c | 12 +- drivers/gpu/drm/drm_ioctl.c | 9 ++ drivers/gpu/drm/drm_mode_object.c | 9 +- drivers/gpu/drm/drm_modes.c | 226 +++- drivers/video/hdmi.c| 3 + include/drm/drm_atomic.h| 5 +- include/drm/drm_file.h | 8 ++ include/drm/drm_modes.h | 13 +++ include/uapi/drm/drm.h | 7 ++ include/uapi/drm/drm_mode.h | 6 + 16 files changed, 365 insertions(+), 65 deletions(-) -- 2.7.4 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: bridge: Constify mode arguments to bridge .mode_set() operation
The mode and ajusted_mode passed to the bridge .mode_set() operation should never be modified by the bridge (and are not in any of the existing bridge drivers). Make them const to make this clear. Signed-off-by: Laurent Pinchart--- drivers/gpu/drm/bridge/adv7511/adv7511.h | 4 ++-- drivers/gpu/drm/bridge/adv7511/adv7511_drv.c | 8 drivers/gpu/drm/bridge/adv7511/adv7533.c | 2 +- drivers/gpu/drm/bridge/analogix-anx78xx.c | 4 ++-- drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 4 ++-- drivers/gpu/drm/bridge/sii902x.c | 4 ++-- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 4 ++-- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 16 drivers/gpu/drm/bridge/tc358767.c | 9 + drivers/gpu/drm/drm_bridge.c | 4 ++-- drivers/gpu/drm/exynos/exynos_drm_mic.c| 4 ++-- drivers/gpu/drm/mediatek/mtk_hdmi.c| 4 ++-- drivers/gpu/drm/msm/dsi/dsi.h | 2 +- drivers/gpu/drm/msm/dsi/dsi_host.c | 2 +- drivers/gpu/drm/msm/dsi/dsi_manager.c | 4 ++-- drivers/gpu/drm/msm/edp/edp_bridge.c | 4 ++-- drivers/gpu/drm/msm/hdmi/hdmi_bridge.c | 4 ++-- drivers/gpu/drm/rcar-du/rcar_lvds.c| 4 ++-- drivers/gpu/drm/sti/sti_dvo.c | 4 ++-- drivers/gpu/drm/sti/sti_hda.c | 4 ++-- drivers/gpu/drm/sti/sti_hdmi.c | 4 ++-- drivers/gpu/drm/stm/dw_mipi_dsi-stm.c | 2 +- include/drm/bridge/dw_mipi_dsi.h | 3 ++- include/drm/drm_bridge.h | 8 24 files changed, 57 insertions(+), 55 deletions(-) Philippe, I wrote this patch while reviewing your adjusted_mode documentation. I initially wanted to document the fact that the adjusted_mode argument to the bridge .mode_set() operation should not be modified by the bridge, and then realized that constifying it would be a better way to express that. I thus wouldn't mind if you took that patch in your tree and pushed it with the documentation patch once we get the necessary acks :-) diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h index d034b2cb5eee..f60d29defd18 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511.h +++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h @@ -389,7 +389,7 @@ static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511) #ifdef CONFIG_DRM_I2C_ADV7533 void adv7533_dsi_power_on(struct adv7511 *adv); void adv7533_dsi_power_off(struct adv7511 *adv); -void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode *mode); +void adv7533_mode_set(struct adv7511 *adv, const struct drm_display_mode *mode); int adv7533_patch_registers(struct adv7511 *adv); int adv7533_patch_cec_registers(struct adv7511 *adv); int adv7533_attach_dsi(struct adv7511 *adv); @@ -405,7 +405,7 @@ static inline void adv7533_dsi_power_off(struct adv7511 *adv) } static inline void adv7533_mode_set(struct adv7511 *adv, - struct drm_display_mode *mode) + const struct drm_display_mode *mode) { } diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c index efa29db5fc2b..99028d36ed74 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c @@ -664,8 +664,8 @@ static int adv7511_mode_valid(struct adv7511 *adv7511, } static void adv7511_mode_set(struct adv7511 *adv7511, -struct drm_display_mode *mode, -struct drm_display_mode *adj_mode) +const struct drm_display_mode *mode, +const struct drm_display_mode *adj_mode) { unsigned int low_refresh_rate; unsigned int hsync_polarity = 0; @@ -827,8 +827,8 @@ static void adv7511_bridge_disable(struct drm_bridge *bridge) } static void adv7511_bridge_mode_set(struct drm_bridge *bridge, - struct drm_display_mode *mode, - struct drm_display_mode *adj_mode) + const struct drm_display_mode *mode, + const struct drm_display_mode *adj_mode) { struct adv7511 *adv = bridge_to_adv7511(bridge); diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c index 185b6d842166..5d5e7d9eded2 100644 --- a/drivers/gpu/drm/bridge/adv7511/adv7533.c +++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c @@ -108,7 +108,7 @@ void adv7533_dsi_power_off(struct adv7511 *adv) regmap_write(adv->regmap_cec, 0x27, 0x0b); } -void adv7533_mode_set(struct adv7511 *adv, struct drm_display_mode
Re: [PATCH v2 11/15] v4l: vsp1: Add per-display list internal completion notification support
Hi Laurent, On 05/04/18 10:18, Laurent Pinchart wrote: > Display list completion is already reported to the frame end handler, > but that mechanism is global to all display lists. In order to implement > BRU and BRS reassignment in DRM pipelines we will need to commit a > display list and wait for its completion internally, without reporting > it to the DRM driver. Extend the display list API to support such an > internal use of the display list. > > Signed-off-by: Laurent Pinchart> --- > Changes since v1: > > - Use the frame end status flags to report notification > - Rename the notify flag to internal This reads much better to me. Thanks for the respin. Reviewed-by: Kieran Bingham > --- > drivers/media/platform/vsp1/vsp1_dl.c| 23 ++- > drivers/media/platform/vsp1/vsp1_dl.h| 3 ++- > drivers/media/platform/vsp1/vsp1_drm.c | 2 +- > drivers/media/platform/vsp1/vsp1_video.c | 2 +- > 4 files changed, 26 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c > index 662fa2a347c9..30ad491605ff 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -72,6 +72,7 @@ struct vsp1_dl_body { > * @fragments: list of extra display list bodies > * @has_chain: if true, indicates that there's a partition chain > * @chain: entry in the display list partition chain > + * @internal: whether the display list is used for internal purpose > */ > struct vsp1_dl_list { > struct list_head list; > @@ -85,6 +86,8 @@ struct vsp1_dl_list { > > bool has_chain; > struct list_head chain; > + > + bool internal; > }; > > enum vsp1_dl_mode { > @@ -550,8 +553,16 @@ static void vsp1_dl_list_commit_continuous(struct > vsp1_dl_list *dl) >* case we can't replace the queued list by the new one, as we could >* race with the hardware. We thus mark the update as pending, it will >* be queued up to the hardware by the frame end interrupt handler. > + * > + * If a display list is already pending we simply drop it as the new > + * display list is assumed to contain a more recent configuration. It is > + * an error if the already pending list has the internal flag set, as > + * there is then a process waiting for that list to complete. This > + * shouldn't happen as the waiting process should perform proper > + * locking, but warn just in case. >*/ > if (vsp1_dl_list_hw_update_pending(dlm)) { > + WARN_ON(dlm->pending && dlm->pending->internal); > __vsp1_dl_list_put(dlm->pending); > dlm->pending = dl; > return; > @@ -581,7 +592,7 @@ static void vsp1_dl_list_commit_singleshot(struct > vsp1_dl_list *dl) > dlm->active = dl; > } > > -void vsp1_dl_list_commit(struct vsp1_dl_list *dl) > +void vsp1_dl_list_commit(struct vsp1_dl_list *dl, bool internal) > { > struct vsp1_dl_manager *dlm = dl->dlm; > struct vsp1_dl_list *dl_child; > @@ -598,6 +609,8 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl) > } > } > > + dl->internal = internal; > + > spin_lock_irqsave(>lock, flags); > > if (dlm->singleshot) > @@ -624,6 +637,10 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl) > * raced with the frame end interrupt. The function always returns with the > flag > * set in header mode as display list processing is then not continuous and > * races never occur. > + * > + * The VSP1_DL_FRAME_END_INTERNAL flag indicates that the previous display > list > + * has completed and had been queued with the internal notification flag. > + * Internal notification is only supported for continuous mode. > */ > unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > { > @@ -656,6 +673,10 @@ unsigned int vsp1_dlm_irq_frame_end(struct > vsp1_dl_manager *dlm) >* frame end interrupt. The display list thus becomes active. >*/ > if (dlm->queued) { > + if (dlm->queued->internal) > + flags |= VSP1_DL_FRAME_END_INTERNAL; > + dlm->queued->internal = false; > + > __vsp1_dl_list_put(dlm->active); > dlm->active = dlm->queued; > dlm->queued = NULL; > diff --git a/drivers/media/platform/vsp1/vsp1_dl.h > b/drivers/media/platform/vsp1/vsp1_dl.h > index cbc2fc53e10b..1a5bbd5ddb7b 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.h > +++ b/drivers/media/platform/vsp1/vsp1_dl.h > @@ -21,6 +21,7 @@ struct vsp1_dl_list; > struct vsp1_dl_manager; > > #define VSP1_DL_FRAME_END_COMPLETED BIT(0) > +#define VSP1_DL_FRAME_END_INTERNAL BIT(1) > > void vsp1_dlm_setup(struct vsp1_device *vsp1); > > @@ -34,7 +35,7 @@ unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager >
Re: [PATCH v2 10/15] v4l: vsp1: Turn frame end completion status into a bitfield
Hi Laurent, Thanks for this enhancement. On 05/04/18 10:18, Laurent Pinchart wrote: > We will soon need to return more than a boolean completion status from > the vsp1_dlm_irq_frame_end() IRQ handler. Turn the return value into a > bitfield to prepare for that. No functional change is introduced here. I think this is a good solution! Reviewed-by: Kieran Bingham> Signed-off-by: Laurent Pinchart > --- > drivers/media/platform/vsp1/vsp1_dl.c| 22 +- > drivers/media/platform/vsp1/vsp1_dl.h| 4 +++- > drivers/media/platform/vsp1/vsp1_drm.c | 5 +++-- > drivers/media/platform/vsp1/vsp1_pipe.c | 8 > drivers/media/platform/vsp1/vsp1_pipe.h | 2 +- > drivers/media/platform/vsp1/vsp1_video.c | 4 ++-- > 6 files changed, 26 insertions(+), 19 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c > b/drivers/media/platform/vsp1/vsp1_dl.c > index 0b86ed01e85d..662fa2a347c9 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.c > +++ b/drivers/media/platform/vsp1/vsp1_dl.c > @@ -616,14 +616,18 @@ void vsp1_dl_list_commit(struct vsp1_dl_list *dl) > * vsp1_dlm_irq_frame_end - Display list handler for the frame end interrupt > * @dlm: the display list manager > * > - * Return true if the previous display list has completed at frame end, or > false > - * if it has been delayed by one frame because the display list commit raced > - * with the frame end interrupt. The function always returns true in header > mode > - * as display list processing is then not continuous and races never occur. > + * Return a set of flags that indicates display list completion status. > + * > + * The VSP1_DL_FRAME_END_COMPLETED flag indicates that the previous display > list > + * has completed at frame end. If the flag is not returned display list > + * completion has been delayed by one frame because the display list commit > + * raced with the frame end interrupt. The function always returns with the > flag > + * set in header mode as display list processing is then not continuous and > + * races never occur. > */ > -bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > +unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > { > - bool completed = false; > + unsigned int flags = 0; > > spin_lock(>lock); > > @@ -634,7 +638,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > if (dlm->singleshot) { > __vsp1_dl_list_put(dlm->active); > dlm->active = NULL; > - completed = true; > + flags |= VSP1_DL_FRAME_END_COMPLETED; > goto done; > } > > @@ -655,7 +659,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > __vsp1_dl_list_put(dlm->active); > dlm->active = dlm->queued; > dlm->queued = NULL; > - completed = true; > + flags |= VSP1_DL_FRAME_END_COMPLETED; > } > > /* > @@ -672,7 +676,7 @@ bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm) > done: > spin_unlock(>lock); > > - return completed; > + return flags; > } > > /* Hardware Setup */ > diff --git a/drivers/media/platform/vsp1/vsp1_dl.h > b/drivers/media/platform/vsp1/vsp1_dl.h > index ee3508172f0a..cbc2fc53e10b 100644 > --- a/drivers/media/platform/vsp1/vsp1_dl.h > +++ b/drivers/media/platform/vsp1/vsp1_dl.h > @@ -20,6 +20,8 @@ struct vsp1_dl_fragment; > struct vsp1_dl_list; > struct vsp1_dl_manager; > > +#define VSP1_DL_FRAME_END_COMPLETED BIT(0) > + > void vsp1_dlm_setup(struct vsp1_device *vsp1); > > struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device *vsp1, > @@ -27,7 +29,7 @@ struct vsp1_dl_manager *vsp1_dlm_create(struct vsp1_device > *vsp1, > unsigned int prealloc); > void vsp1_dlm_destroy(struct vsp1_dl_manager *dlm); > void vsp1_dlm_reset(struct vsp1_dl_manager *dlm); > -bool vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm); > +unsigned int vsp1_dlm_irq_frame_end(struct vsp1_dl_manager *dlm); > > struct vsp1_dl_list *vsp1_dl_list_get(struct vsp1_dl_manager *dlm); > void vsp1_dl_list_put(struct vsp1_dl_list *dl); > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > b/drivers/media/platform/vsp1/vsp1_drm.c > index a79b05ef..541473b1df67 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -34,12 +34,13 @@ > */ > > static void vsp1_du_pipeline_frame_end(struct vsp1_pipeline *pipe, > -bool completed) > +unsigned int completion) > { > struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe); > > if (drm_pipe->du_complete) > - drm_pipe->du_complete(drm_pipe->du_private, completed); > + drm_pipe->du_complete(drm_pipe->du_private, > +
Re: [PATCH v2 09/15] v4l: vsp1: Move DRM pipeline output setup code to a function
Hi Laurent, Thanks for the updates On 05/04/18 10:18, Laurent Pinchart wrote: > In order to make the vsp1_du_setup_lif() easier to read, and for > symmetry with the DRM pipeline input setup, move the pipeline output > setup code to a separate function. > > Signed-off-by: Laurent Pinchart> Reviewed-by: Kieran Bingham > -- > Changes since v1: > > - Rename vsp1_du_pipeline_setup_input() to > vsp1_du_pipeline_setup_inputs() Hrm ... I perhaps would have expected this to happen in [PATCH 06/15] v4l: vsp1: Move DRM atomic commit pipeline setup to separate function But I think that's being quite pedantic - so unless you have a need to respin, I wouldn't worry about it. -- Kieran > - Initialize format local variable to 0 in > vsp1_du_pipeline_setup_output() > --- > drivers/media/platform/vsp1/vsp1_drm.c | 114 > ++--- > 1 file changed, 64 insertions(+), 50 deletions(-) > > diff --git a/drivers/media/platform/vsp1/vsp1_drm.c > b/drivers/media/platform/vsp1/vsp1_drm.c > index 00ce99bd1605..a79b05ef 100644 > --- a/drivers/media/platform/vsp1/vsp1_drm.c > +++ b/drivers/media/platform/vsp1/vsp1_drm.c > @@ -193,8 +193,8 @@ static unsigned int rpf_zpos(struct vsp1_device *vsp1, > struct vsp1_rwpf *rpf) > } > > /* Setup the input side of the pipeline (RPFs and BRU). */ > -static int vsp1_du_pipeline_setup_input(struct vsp1_device *vsp1, > - struct vsp1_pipeline *pipe) > +static int vsp1_du_pipeline_setup_inputs(struct vsp1_device *vsp1, > + struct vsp1_pipeline *pipe) > { > struct vsp1_rwpf *inputs[VSP1_MAX_RPF] = { NULL, }; > struct vsp1_bru *bru = to_bru(>bru->subdev); > @@ -276,6 +276,65 @@ static int vsp1_du_pipeline_setup_input(struct > vsp1_device *vsp1, > return 0; > } > > +/* Setup the output side of the pipeline (WPF and LIF). */ > +static int vsp1_du_pipeline_setup_output(struct vsp1_device *vsp1, > + struct vsp1_pipeline *pipe) > +{ > + struct vsp1_drm_pipeline *drm_pipe = to_vsp1_drm_pipeline(pipe); > + struct v4l2_subdev_format format = { 0, }; > + int ret; > + > + format.which = V4L2_SUBDEV_FORMAT_ACTIVE; > + format.pad = RWPF_PAD_SINK; > + format.format.width = drm_pipe->width; > + format.format.height = drm_pipe->height; > + format.format.code = MEDIA_BUS_FMT_ARGB_1X32; > + format.format.field = V4L2_FIELD_NONE; > + > + ret = v4l2_subdev_call(>output->entity.subdev, pad, set_fmt, NULL, > +); > + if (ret < 0) > + return ret; > + > + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on WPF%u sink\n", > + __func__, format.format.width, format.format.height, > + format.format.code, pipe->output->entity.index); > + > + format.pad = RWPF_PAD_SOURCE; > + ret = v4l2_subdev_call(>output->entity.subdev, pad, get_fmt, NULL, > +); > + if (ret < 0) > + return ret; > + > + dev_dbg(vsp1->dev, "%s: got format %ux%u (%x) on WPF%u source\n", > + __func__, format.format.width, format.format.height, > + format.format.code, pipe->output->entity.index); > + > + format.pad = LIF_PAD_SINK; > + ret = v4l2_subdev_call(>lif->subdev, pad, set_fmt, NULL, > +); > + if (ret < 0) > + return ret; > + > + dev_dbg(vsp1->dev, "%s: set format %ux%u (%x) on LIF%u sink\n", > + __func__, format.format.width, format.format.height, > + format.format.code, pipe->lif->index); > + > + /* > + * Verify that the format at the output of the pipeline matches the > + * requested frame size and media bus code. > + */ > + if (format.format.width != drm_pipe->width || > + format.format.height != drm_pipe->height || > + format.format.code != MEDIA_BUS_FMT_ARGB_1X32) { > + dev_dbg(vsp1->dev, "%s: format mismatch on LIF%u\n", __func__, > + pipe->lif->index); > + return -EPIPE; > + } > + > + return 0; > +} > + > /* Configure all entities in the pipeline. */ > static void vsp1_du_pipeline_configure(struct vsp1_pipeline *pipe) > { > @@ -356,7 +415,6 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int > pipe_index, > struct vsp1_drm_pipeline *drm_pipe; > struct vsp1_pipeline *pipe; > struct vsp1_bru *bru; > - struct v4l2_subdev_format format; > unsigned long flags; > unsigned int i; > int ret; > @@ -413,58 +471,14 @@ int vsp1_du_setup_lif(struct device *dev, unsigned int > pipe_index, > __func__, pipe_index, cfg->width, cfg->height); > > /* Setup formats through the pipeline. */ > - ret = vsp1_du_pipeline_setup_input(vsp1, pipe); > - if (ret < 0) > -
Re: [Intel-gfx] [PATCH] drm/i915: Do NOT skip the first 4k of stolen memory for pre-allocated buffers
On Thu, Apr 05, 2018 at 01:37:31PM +0200, Hans de Goede wrote: > Hi, > > On 04-04-18 22:49, Ville Syrjälä wrote: > > On Wed, Apr 04, 2018 at 10:06:29PM +0200, Hans de Goede wrote: > >> Hi, > >> > >> On 04-04-18 17:50, Ville Syrjälä wrote: > >>> On Fri, Mar 30, 2018 at 04:26:53PM +0200, Hans de Goede wrote: > Hi, > > On 30-03-18 15:25, Hans de Goede wrote: > > Hi, > > > > On 30-03-18 14:44, Chris Wilson wrote: > >> Quoting Hans de Goede (2018-03-30 13:37:40) > >>> Hi, > >>> > >>> On 30-03-18 14:30, Chris Wilson wrote: > Quoting Hans de Goede (2018-03-30 13:27:15) > > Before this commit the WaSkipStolenMemoryFirstPage workaround code > > was > > skipping the first 4k by passing 4096 as start of the address range > > passed > > to drm_mm_init(). This means that calling drm_mm_reserve_node() to > > try and > > reserve the firmware framebuffer so that we can inherit it would > > always > > fail, as the firmware framebuffer starts at address 0. > > > > Commit d43537610470 ("drm/i915: skip the first 4k of stolen memory > > on > > everything >= gen8") says in its commit message: "This is confirmed > > to fix > > Skylake screen flickering issues (probably caused by the fact that > > we > > initialized a ring in the first page of stolen, but I didn't 100% > > confirm > > this theory)." > > > > Which suggests that it is safe to use the first page for a linear > > framebuffer as the firmware is doing. > > > > This commit always passes 0 as start to drm_mm_init() and works > > around > > WaSkipStolenMemoryFirstPage in > > i915_gem_stolen_insert_node_in_range() > > by insuring the start address passed by to > > drm_mm_insert_node_in_range() > > is always 4k or more. All entry points to i915_gem_stolen.c go > > through > > i915_gem_stolen_insert_node_in_range(), so that any newly allocated > > objects such as ring-buffers will not be allocated in the first 4k. > > > > The one exception is > > i915_gem_object_create_stolen_for_preallocated() > > which directly calls drm_mm_reserve_node() which now will be able to > > use the first 4k. > > > > This fixes the i915 driver no longer being able to inherit the > > firmware > > framebuffer on gen8+, which fixes the video output changing from the > > vendor logo to a black screen as soon as the i915 driver is loaded > > (on systems without fbcon). > > We've been told by the HW guys not to use the first page. (That's my > understanding from every time this gets questioned.) > >>> > >>> Yet the GOP is happily using the first page. I think we may need to > >>> make > >>> a difference here between the GPU not using the first page and the > >>> display engine/pipeline not using the first page. Note that my patch > >>> only influences the inheriting of the initial framebuffer as allocated > >>> by the GOP. It does not influence any other allocations from the > >>> reserved range, those will still all avoid the first page. > >>> > >>> Without this patch fastboot / flickerfree support is essentially > >>> broken > >>> on any gen8+ hardware given that one of the goals of atomic is to be > >>> able to do flickerfree transitions I think that this warrants a closer > >>> look then just simply saying never use the first page. > >> > >> The concern is what else (i.e. nothing that we allocated ourselves) > >> that > >> may be in the first page... > > > > Given that the GOP has put its framebuffer there at least at boot there > > is nothing there, otherwise it would show up on the display. > > > > We have a whole bunch of code to inherit the BIOS fb in intel_display.c > > and AFAIK that code is there because this inheriting the BIOS fb is > > deemed an important feature. So I'm not happy at all with the handwavy > > best to not touch it answer I'm getting to this patch. > > > > Unless there are some clear answer from the hardware folks which > > specifically > > say we cannot put a framebuffer there (and then why is the GOP doing > > it?) > > then I believe that the best way forward here is to get various people > > to > > test with this patch and the best way to do that is probably to put it > > in next. Note I deliberately did not add a Cc stable. > > To elaborate on this, the excluding of the first 4k of the stolen memory > region causes intel_alloc_initial_plane_obj() from intel_display.c to > fail, > which in turn causes intel_find_initial_plane_obj() to call >
Re: [PATCH v2 3/4] drm/i915: Check hdcp key loadability
On Mon, Apr 02, 2018 at 02:35:42PM +0530, Ramalingam C wrote: > > > On Thursday 29 March 2018 07:54 PM, Ville Syrjälä wrote: > > On Thu, Mar 29, 2018 at 07:39:07PM +0530, Ramalingam C wrote: > >> HDCP1.4 key can be loaded, only when Power well #1 is enabled and cdclk > >> is enabled. Using the I915 power well infrastruture, above requirement > >> is verified. > >> > >> This patch enables the hdcp initialization for HSW, BDW, and BXT. > >> > >> v2: > >>Choose the PW# based on the platform. > >> > >> Signed-off-by: Ramalingam C> >> Reviewed-by: Sean Paul > >> --- > >> drivers/gpu/drm/i915/intel_hdcp.c | 41 > >> +-- > >> 1 file changed, 39 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/intel_hdcp.c > >> b/drivers/gpu/drm/i915/intel_hdcp.c > >> index f77d956b2b18..d8af09b46a44 100644 > >> --- a/drivers/gpu/drm/i915/intel_hdcp.c > >> +++ b/drivers/gpu/drm/i915/intel_hdcp.c > >> @@ -37,6 +37,43 @@ static int intel_hdcp_poll_ksv_fifo(struct > >> intel_digital_port *intel_dig_port, > >>return 0; > >> } > >> > >> +static bool hdcp_key_loadable(struct drm_i915_private *dev_priv) > >> +{ > >> + struct i915_power_domains *power_domains = _priv->power_domains; > >> + struct i915_power_well *power_well; > >> + enum i915_power_well_id id; > >> + bool enabled = false; > >> + > >> + /* > >> + * On HSW and BDW, Display HW loads the Key as soon as Display resumes. > >> + * On all BXT+, SW can load the keys only when the PW#1 is turned on. > >> + */ > >> + if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv)) > >> + id = HSW_DISP_PW_GLOBAL; > >> + else > >> + id = SKL_DISP_PW_1; > >> + > >> + mutex_lock(_domains->lock); > >> + > >> + /* PG1 (power well #1) needs to be enabled */ > >> + for_each_power_well(dev_priv, power_well) { > >> + if (power_well->id == id) { > >> + enabled = power_well->ops->is_enabled(dev_priv, > >> +power_well); > >> + break; > >> + } > >> + } > >> + mutex_unlock(_domains->lock); > >> + > >> + /* > >> + * Another req for hdcp key loadability is enabled state of pll for > >> + * cdclk. Without active crtc we wont land here. So we are assuming that > >> + * cdclk is already on. > >> + */ > >> + > >> + return enabled; > >> +} > >> + > >> static void intel_hdcp_clear_keys(struct drm_i915_private *dev_priv) > >> { > >>I915_WRITE(HDCP_KEY_CONF, HDCP_CLEAR_KEYS_TRIGGER); > >> @@ -615,8 +652,8 @@ static int _intel_hdcp_enable(struct intel_connector > >> *connector) > >>DRM_DEBUG_KMS("[%s:%d] HDCP is being enabled...\n", > >> connector->base.name, connector->base.base.id); > >> > >> - if (!(I915_READ(SKL_FUSE_STATUS) & SKL_FUSE_PG_DIST_STATUS(1))) { > >> - DRM_ERROR("PG1 is disabled, cannot load keys\n"); > >> + if (!hdcp_key_loadable(dev_priv)) { > >> + DRM_ERROR("HDCP key Load is not possible\n"); > >>return -ENXIO; > >>} > > If you need the power well then why aren't you grabbing the correct > > power domain reference somewhere? > > Ville, > > As HDCP is supposed to be enabled after the basic display is up, power well > #1 is supposed to be enabled. > If not that is mostly an error w.r.t HDCP. > > So at this point we just want to verify whether required PW is up and HDCP > key can be loaded from the HW. Else fail the HDCP request. So this is in practice dead code to deal with a hypothetical bug elsewhere in the driver? -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: AMD graphics performance regression in 4.15 and later
Am 06.04.2018 um 17:30 schrieb Jean-Marc Valin: Hi Christian, Is there a way to turn off these huge pages at boot-time/run-time? Only at compile time by not setting CONFIG_TRANSPARENT_HUGEPAGE. Alternatively you can avoid enabling CONFIG_SWIOTLB which will avoid the slow DMA path as well. Right now the recent kernels are making Firefox pretty much unusable for me. I've been able to revert the patch from 4.15 but it's not really a long-term solution. You mention that the purpose of the patch is to improve performance, but I haven't actually noticed anything running faster on my system. Is there any particular test where I'm supposed to see an improvement compared to 4.14? Mostly crypto mining, maybe some games as well. I'm not sure what you mean by "We mitigated the problem by avoiding the slow coherent DMA code path on almost all platforms on newer kernels". I tested up to 4.16 and the performance regression is just as bad as it is for 4.15. Indeed 4.16 still doesn't have that. You could use the amd-staging-drm-next branch or wait for 4.17. Unlike the older hardware reported on kernel bug 198511, the hardware I have is quite recent (RX 560) and still being sold. That isn't related to the GFX hardware, but to your CPU/motherboard and whatever else you have in the system. Some part of your system needs SWIOTLB and that makes allocating memory much slower. I've also confirmed that neither nvidia (on the same machine) nor intel GPUs (on a less powerful machine) are affected, so it seems like there's a way to avoid that slow performance. Intel doesn't use TTM because they don't have dedicated VRAM, but the open source nvidia driver should be affected as well. I'm not saying that what Firefox is doing is ideal (I don't know what it does and why), but it still seems like something that should still be avoided in the kernel. We already mitigated that problem and I don't see any solution which will arrive faster than 4.17. The only quick workaround I can see is to avoid firefox, chrome for example is reported to work perfectly fine. Christian. Cheers, Jean-Marc On 04/06/2018 04:03 AM, Christian König wrote: Hi Jean, yeah, that is a known problem. Using huge pages improves the performance because of better TLB usage, but for the cost of higher allocation overhead. What we found is that firefox is doing something rather strange by allocating large textures and then just trowing them away again immediately. We mitigated the problem by avoiding the slow coherent DMA code path on almost all platforms on newer kernels, but essentially somebody needs to figure out why firefox and/or the user space stack is doing this constant allocation/freeing of memory. There is also a bug tracker on bugs.kernel.org about this, but I can't find it any more of hand. Regards, Christian. Am 06.04.2018 um 02:30 schrieb Jean-Marc Valin: Hi, I noticed a serious graphics performance regression between 4.14 and 4.15. It is most noticeable with Firefox (tried FF57 through FF60) and causes scrolling to be really choppy/sluggish. I've confirmed that the problem is also there on 4.16, while 4.13 works fine. After a bisection, I've narrowed the regression down to this commit: commit 648bc3574716400acc06f99915815f80d9563783 Author: Christian KönigDate: Thu Jul 6 09:59:43 2017 +0200 drm/ttm: add transparent huge page support for DMA allocations v2 Some details about my system: Distro: Fedora 27 (up-to-date) Video: MSI Radeon RX 560 AERO CPU: Dual-socket Xeon E5-2640 v4 (20 cores total) RAM: 128 GB ECC As a comparison, when running Firefox with 4.15 on a Lenovo W540 laptop (with Intel graphics only) the responsiveness is much better then what I'm getting on the Xeon machine above with the Radeon card, so this really seems to be an AMD-only issue. Any way to fix the issue? Thanks, Jean-Marc ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 1/2] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder
Hi Jacopo, (CC'ing Mark Brown) On Friday, 6 April 2018 17:25:58 EEST jacopo mondi wrote: > On Fri, Apr 06, 2018 at 04:15:35PM +0300, Laurent Pinchart wrote: > > On Friday, 6 April 2018 15:41:56 EEST Jacopo Mondi wrote: > >> Document Thine THC63LVD1024 LVDS decoder device tree bindings. > >> > >> Signed-off-by: Jacopo Mondi> >> Reviewed-by: Andrzej Hajda > >> Reviewed-by: Niklas Söderlund > >> Reviewed-by: Laurent Pinchart > >> --- > >> > >> .../bindings/display/bridge/thine,thc63lvd1024.txt | 60 > >> +++ > >> 1 file changed, 60 insertions(+) > >> create mode 100644 > >> > >> Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > >> > >> diff --git > >> a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.tx > >> t > >> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.tx > >> t > >> new file mode 100644 > >> index 000..1191f17 > >> --- /dev/null > >> +++ > >> b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.tx > >> t > >> @@ -0,0 +1,60 @@ > >> +Thine Electronics THC63LVD1024 LVDS decoder > >> +--- > >> + > >> +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS > >> streams > >> +to parallel data outputs. The chip supports single/dual input/output > >> modes, > >> +handling up to two two input LVDS stream and up to two digital CMOS/TTL > > > > s/to two two/to two/ > > s/stream/streams/ > > > >> outputs. > >> + > >> +Single or dual operation modes, output data mapping and DDR output > >> modes are > >> +configured through input signals and the chip does not expose any > >> control bus. > >> + > >> +Required properties: > >> +- compatible: Shall be "thine,thc63lvd1024" > >> + > >> +Optional properties: > >> +- vcc-supply: Power supply for TTL output, TTL CLOCKOUT signal, LVDS > >> input, > >> + PPL and digital circuitry > >> +- powerdown-gpios: Power down GPIO signal, pin name "/PDWN". Active low > >> +- enable-gpios: Output enable GPIO signal, pin name "OE". Active high > > > > As Rob mentioned in a reply to v6, we currently use "enable" as the > > inverse of "powerdown". I would call this one oe-gpios instead. Quoting > > Rob: > > > > "Debating "oe" vs. "output-enable" is bikeshedding IMO. Anyone familiar > > with h/w design should recognize OE." > > I got a different understanding of what Rob meant. I thought "anyone > familiar with h/w design should recognize OE" as that nobody would get > confused if a pin named OE in the chip manual is descibed by an > 'enable' property. > > But as discussed offline, enable has probably to be used as the > opposite of powerdown for complete chip sleep, not just for output > pad. > > Anyway, we spent enough time on naming issues, starting from my first > stupid 'pdwn' permutations then on this semi-standard names. > > I'll send next version with 'powerdown-gpios' and 'oe-gpios' > properties hoping that would be finally accepted by everyone. I certainly won't complain (as long as you write pwdn instead of pdwn in the driver :-)). > Same on the mandatory/optional VCC supply thing. Let's try to make > next version the final one. If the optional property with the dummy > regulator doesn't satisfy you and it is preferred to have a fixed-regulator > anyhow in DT I'll do in next version, othewise let's try not to change > it again. I'll just remark here that in the current Eagle design vcc is > connected to a power rail with no regulator at all :) I don't like the dummy regulator much, as it generates a dev_warn(), which makes me believe that it's a hack rather than a proper solution. You might want to ask Mark Brown for his opinion. > >> + > >> +The THC63LVD1024 video port connections are modeled according > >> +to OF graph bindings specified by Documentation/devicetree/bindings/ > >> graph.txt > >> + > >> +Required video port nodes: > >> +- port@0: First LVDS input port > >> +- port@2: First digital CMOS/TTL parallel output > >> + > >> +Optional video port nodes: > >> +- port@1: Second LVDS input port > >> +- port@3: Second digital CMOS/TTL parallel output > >> + > >> +Example: > >> + > >> + > >> + thc63lvd1024: lvds-decoder { > >> + compatible = "thine,thc63lvd1024"; > >> + > >> + vcc-supply = <_lvds_vcc>; > >> + powerdown-gpios = < 15 GPIO_ACTIVE_LOW>; > >> + > >> + ports { > >> + #address-cells = <1>; > >> + #size-cells = <0>; > >> + > >> + port@0 { > >> + reg = <0>; > >> + > >> + lvds_dec_in_0: endpoint { > >> + remote-endpoint = <_out>; > >> + }; > >> + }; > >> + > >> + port@2{ > >> + reg = <2>; > >> + > >>
Re: [PATCH] drm: clarify adjusted_mode for a bridge connected to a crtc
Hi Laurent, On 04/06/2018 04:53 PM, Laurent Pinchart wrote: > Hi Philippe, > > Thank you for the patch. > > On Monday, 26 February 2018 14:16:04 EEST Philippe Cornu wrote: >> This patch clarifies the adjusted_mode documentation >> for a bridge directly connected to a crtc. >> >> Signed-off-by: Philippe Cornu>> --- >> This patch is linked to the discussion https://lkml.org/lkml/2018/1/25/367 >> >> include/drm/drm_bridge.h | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h >> index 3270fec46979..b5f3c070467c 100644 >> --- a/include/drm/drm_bridge.h >> +++ b/include/drm/drm_bridge.h >> @@ -177,7 +177,8 @@ struct drm_bridge_funcs { >> * pipeline has been called already. If the bridge is the first element >> * then this would be _encoder_helper_funcs.mode_set. The display >> * pipe (i.e. clocks and timing signals) is off when this function is >> - * called. >> + * called. If the bridge is connected to the crtc, the adjusted_mode >> + * parameter is the one defined in _crtc_state.adjusted_mode. > > Unless I'm mistaken this will always be the mode stored in > _crtc_state.adjusted_mode (at least for atomic drivers), regardless of > whether the bridge is the first in the chain (connected to the CRTC) or not. > What is important to document is that we have a single adjusted_mode for the > whole chain of bridges, and that it corresponds to the mode output by the CRTC > for the first bridge. Bridges further in the chain can look at that mode, > although there will probably be nothing of interest to them there. > > How about the following text ? > > /** > * @mode_set: > * > * This callback should set the given mode on the bridge. It is called > * after the @mode_set callback for the preceding element in the display > * pipeline has been called already. If the bridge is the first element > * then this would be _encoder_helper_funcs.mode_set. The display > * pipe (i.e. clocks and timing signals) is off when this function is > * called. > * > * The adjusted_mode parameter corresponds to the mode output by the CRTC > * for the first bridge in the chain. It can be different from the mode > * parameter that contains the desired mode for the connector at the end > * of the bridges chain, for instance when the first bridge in the chain > * performs scaling. The adjusted mode is mostly useful for the first > * bridge in the chain and is likely irrelevant for the other bridges. > * > * For atomic drivers the adjusted_mode is the mode stored in > * _crtc_state.adjusted_mode. > * > * NOTE: > * > * If a need arises to store and access modes adjusted for other > locations > * than the connection between the CRTC and the first bridge, the DRM > * framework will have to be extended with DRM bridge states. >*/ > > Then I think we should also update the documentation of > drm_crtc_state.adjusted_mode accordingly: > > /* > * @adjusted_mode: > * > * Internal display timings which can be used by the driver to handle > * differences between the mode requested by userspace in @mode and what > * is actually programmed into the hardware. > * > * For drivers using drm_bridge, this store the hardware display timings > * used between the CRTC and the first bridge. For other drivers, the > * meaning of the adjusted_mode field is purely driver implementation > * defined information, and will usually be used to store the hardware > * display timings used between the CRTC and encoder blocks. > */ > Your proposal is very clear and understandable. I will make a new patch version based on it. Big thanks Philippe :-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99403] Graphical glitches in witcher-1 with wine nine and r600g (rv740).
https://bugs.freedesktop.org/show_bug.cgi?id=99403 --- Comment #7 from i...@yahoo.com --- It's not driver specific bug. I should have followed the hint that there is a registry entry to workaround this problem with wined3d. It turns out that this registry entry is specifically for this bug. The analysis and possible solutions could be found here: https://bugs.winehq.org/show_bug.cgi?id=34052#c42 Also, this bug is also tracked at: https://github.com/iXit/Mesa-3D/issues/312 -- 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: clarify adjusted_mode for a bridge connected to a crtc
Hi Philippe, Thank you for the patch. On Monday, 26 February 2018 14:16:04 EEST Philippe Cornu wrote: > This patch clarifies the adjusted_mode documentation > for a bridge directly connected to a crtc. > > Signed-off-by: Philippe Cornu> --- > This patch is linked to the discussion https://lkml.org/lkml/2018/1/25/367 > > include/drm/drm_bridge.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 3270fec46979..b5f3c070467c 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -177,7 +177,8 @@ struct drm_bridge_funcs { >* pipeline has been called already. If the bridge is the first element >* then this would be _encoder_helper_funcs.mode_set. The display >* pipe (i.e. clocks and timing signals) is off when this function is > - * called. > + * called. If the bridge is connected to the crtc, the adjusted_mode > + * parameter is the one defined in _crtc_state.adjusted_mode. Unless I'm mistaken this will always be the mode stored in _crtc_state.adjusted_mode (at least for atomic drivers), regardless of whether the bridge is the first in the chain (connected to the CRTC) or not. What is important to document is that we have a single adjusted_mode for the whole chain of bridges, and that it corresponds to the mode output by the CRTC for the first bridge. Bridges further in the chain can look at that mode, although there will probably be nothing of interest to them there. How about the following text ? /** * @mode_set: * * This callback should set the given mode on the bridge. It is called * after the @mode_set callback for the preceding element in the display * pipeline has been called already. If the bridge is the first element * then this would be _encoder_helper_funcs.mode_set. The display * pipe (i.e. clocks and timing signals) is off when this function is * called. * * The adjusted_mode parameter corresponds to the mode output by the CRTC * for the first bridge in the chain. It can be different from the mode * parameter that contains the desired mode for the connector at the end * of the bridges chain, for instance when the first bridge in the chain * performs scaling. The adjusted mode is mostly useful for the first * bridge in the chain and is likely irrelevant for the other bridges. * * For atomic drivers the adjusted_mode is the mode stored in * _crtc_state.adjusted_mode. * * NOTE: * * If a need arises to store and access modes adjusted for other locations * than the connection between the CRTC and the first bridge, the DRM * framework will have to be extended with DRM bridge states. */ Then I think we should also update the documentation of drm_crtc_state.adjusted_mode accordingly: /* * @adjusted_mode: * * Internal display timings which can be used by the driver to handle * differences between the mode requested by userspace in @mode and what * is actually programmed into the hardware. * * For drivers using drm_bridge, this store the hardware display timings * used between the CRTC and the first bridge. For other drivers, the * meaning of the adjusted_mode field is purely driver implementation * defined information, and will usually be used to store the hardware * display timings used between the CRTC and encoder blocks. */ -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 1/2] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder
Hi Laurent, On Fri, Apr 06, 2018 at 04:15:35PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Friday, 6 April 2018 15:41:56 EEST Jacopo Mondi wrote: > > Document Thine THC63LVD1024 LVDS decoder device tree bindings. > > > > Signed-off-by: Jacopo Mondi> > Reviewed-by: Andrzej Hajda > > Reviewed-by: Niklas Söderlund > > Reviewed-by: Laurent Pinchart > > --- > > .../bindings/display/bridge/thine,thc63lvd1024.txt | 60 +++ > > 1 file changed, 60 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > > > > diff --git > > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > > new file mode 100644 > > index 000..1191f17 > > --- /dev/null > > +++ > > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > > @@ -0,0 +1,60 @@ > > +Thine Electronics THC63LVD1024 LVDS decoder > > +--- > > + > > +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS > > streams > > +to parallel data outputs. The chip supports single/dual input/output modes, > > +handling up to two two input LVDS stream and up to two digital CMOS/TTL > > s/to two two/to two/ > s/stream/streams/ > > > outputs. > > + > > +Single or dual operation modes, output data mapping and DDR output modes > > are > > +configured through input signals and the chip does not expose any control > > bus. > > + > > +Required properties: > > +- compatible: Shall be "thine,thc63lvd1024" > > + > > +Optional properties: > > +- vcc-supply: Power supply for TTL output, TTL CLOCKOUT signal, LVDS input, > > + PPL and digital circuitry > > +- powerdown-gpios: Power down GPIO signal, pin name "/PDWN". Active low > > +- enable-gpios: Output enable GPIO signal, pin name "OE". Active high > > As Rob mentioned in a reply to v6, we currently use "enable" as the inverse of > "powerdown". I would call this one oe-gpios instead. Quoting Rob: > > "Debating "oe" vs. "output-enable" is bikeshedding IMO. Anyone familiar with > h/w design should recognize OE." > I got a different understanding of what Rob meant. I thought "anyone familiar with h/w design should recognize OE" as that nobody would get confused if a pin named OE in the chip manual is descibed by an 'enable' property. But as discussed offline, enable has probably to be used as the opposite of powerdown for complete chip sleep, not just for output pad. Anyway, we spent enough time on naming issues, starting from my first stupid 'pdwn' permutations then on this semi-standard names. I'll send next version with 'powerdown-gpios' and 'oe-gpios' properties hoping that would be finally accepted by everyone. Same on the mandatory/optional VCC supply thing. Let's try to make next version the final one. If the optional property with the dummy regulator doesn't satisfy you and it is preferred to have a fixed-regulator anyhow in DT I'll do in next version, othewise let's try not to change it again. I'll just remark here that in the current Eagle design vcc is connected to a power rail with no regulator at all :) Thanks j > > + > > +The THC63LVD1024 video port connections are modeled according > > +to OF graph bindings specified by Documentation/devicetree/bindings/ > > graph.txt > > + > > +Required video port nodes: > > +- port@0: First LVDS input port > > +- port@2: First digital CMOS/TTL parallel output > > + > > +Optional video port nodes: > > +- port@1: Second LVDS input port > > +- port@3: Second digital CMOS/TTL parallel output > > + > > +Example: > > + > > + > > + thc63lvd1024: lvds-decoder { > > + compatible = "thine,thc63lvd1024"; > > + > > + vcc-supply = <_lvds_vcc>; > > + powerdown-gpios = < 15 GPIO_ACTIVE_LOW>; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + > > + lvds_dec_in_0: endpoint { > > + remote-endpoint = <_out>; > > + }; > > + }; > > + > > + port@2{ > > + reg = <2>; > > + > > + lvds_dec_out_2: endpoint { > > + remote-endpoint = <_in>; > > + }; > > + }; > > + }; > > + }; > > -- > Regards, > > Laurent Pinchart > > > signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/2 v2] drm/pl111: Support the Versatile Express
The Versatile Express uses a special configuration controller deeply embedded in the system motherboard FPGA to multiplex the two to three (!) CLCD instances out to the single SiI9022 bridge. Set up an extra file with the logic to probe to the FPGA mux register on the system controller bus, then parse the memory range argument to figure out what path the CLCD signal is actually taking, and set up the mux accordingly. If there is a CLCD instance on the core tile (the daughterboard with the CPUs fitted) then that CLCD instance will take precedence since it can address all memory. Scale down the Versatile Express to 16BPP so we can support a 1024x768 display despite the bus bandwidth restrictions on this platform. Cc: Liviu DudauCc: Pawel Moll Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - No changes just reposting rebased on mainline changes. --- drivers/gpu/drm/pl111/Makefile | 1 + drivers/gpu/drm/pl111/pl111_drm.h | 3 +- drivers/gpu/drm/pl111/pl111_versatile.c | 48 ++- drivers/gpu/drm/pl111/pl111_vexpress.c | 106 drivers/gpu/drm/pl111/pl111_vexpress.h | 22 +++ 5 files changed, 178 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.c create mode 100644 drivers/gpu/drm/pl111/pl111_vexpress.h diff --git a/drivers/gpu/drm/pl111/Makefile b/drivers/gpu/drm/pl111/Makefile index 9c5e8dba8ac6..19a8189dc54f 100644 --- a/drivers/gpu/drm/pl111/Makefile +++ b/drivers/gpu/drm/pl111/Makefile @@ -3,6 +3,7 @@ pl111_drm-y += pl111_display.o \ pl111_versatile.o \ pl111_drv.o +pl111_drm-$(CONFIG_ARCH_VEXPRESS) += pl111_vexpress.o pl111_drm-$(CONFIG_DEBUG_FS) += pl111_debugfs.o obj-$(CONFIG_DRM_PL111) += pl111_drm.o diff --git a/drivers/gpu/drm/pl111/pl111_drm.h b/drivers/gpu/drm/pl111/pl111_drm.h index 8639b2d4ddf7..916154ac733a 100644 --- a/drivers/gpu/drm/pl111/pl111_drm.h +++ b/drivers/gpu/drm/pl111/pl111_drm.h @@ -64,7 +64,8 @@ struct pl111_drm_dev_private { struct drm_bridge *bridge; struct drm_simple_display_pipe pipe; - void *regs; + void __iomem *clcd_memory; + void __iomem *regs; u32 memory_bw; u32 ienb; u32 ctrl; diff --git a/drivers/gpu/drm/pl111/pl111_versatile.c b/drivers/gpu/drm/pl111/pl111_versatile.c index 9302f516045e..569edf02a36a 100644 --- a/drivers/gpu/drm/pl111/pl111_versatile.c +++ b/drivers/gpu/drm/pl111/pl111_versatile.c @@ -1,12 +1,14 @@ #include #include #include +#include #include #include #include #include #include #include "pl111_versatile.h" +#include "pl111_vexpress.h" #include "pl111_drm.h" static struct regmap *versatile_syscon_map; @@ -22,6 +24,7 @@ enum versatile_clcd { REALVIEW_CLCD_PB11MP, REALVIEW_CLCD_PBA8, REALVIEW_CLCD_PBX, + VEXPRESS_CLCD_V2M, }; static const struct of_device_id versatile_clcd_of_match[] = { @@ -53,6 +56,10 @@ static const struct of_device_id versatile_clcd_of_match[] = { .compatible = "arm,realview-pbx-syscon", .data = (void *)REALVIEW_CLCD_PBX, }, + { + .compatible = "arm,vexpress-muxfpga", + .data = (void *)VEXPRESS_CLCD_V2M, + }, {}, }; @@ -286,12 +293,26 @@ static const struct pl111_variant_data pl111_realview = { .fb_bpp = 16, }; +/* + * Versatile Express PL111 variant, again we just push the maximum + * BPP to 16 to be able to get 1024x768 without saturating the memory + * bus. The clockdivider also seems broken on the Versatile Express. + */ +static const struct pl111_variant_data pl111_vexpress = { + .name = "PL111 Versatile Express", + .formats = pl111_realview_pixel_formats, + .nformats = ARRAY_SIZE(pl111_realview_pixel_formats), + .fb_bpp = 16, + .broken_clockdivider = true, +}; + int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv) { const struct of_device_id *clcd_id; enum versatile_clcd versatile_clcd_type; struct device_node *np; struct regmap *map; + int ret; np = of_find_matching_node_and_match(NULL, versatile_clcd_of_match, _id); @@ -301,7 +322,25 @@ int pl111_versatile_init(struct device *dev, struct pl111_drm_dev_private *priv) } versatile_clcd_type = (enum versatile_clcd)clcd_id->data; - map = syscon_node_to_regmap(np); + /* Versatile Express special handling */ + if (versatile_clcd_type == VEXPRESS_CLCD_V2M) { + struct platform_device *pdev; + + /* Call into deep Vexpress configuration API */ + pdev = of_find_device_by_node(np); + if (!pdev) { + dev_err(dev, "can't find the sysreg device, deferring\n"); +
[PATCH 2/2 v2] drm/pl111: Enable device-specific assigned memory
The Versatile Express has 8 MB of dedicated video RAM (VRAM) on the motherboard, which is what we should be using for the PL111 if available. On this platform, the memory backplane is constructed so that only this memory will work properly with the CLCD on the motherboard, using any other memory region just gives random snow on the display. The CA9 Versatile Express also has a PL111 instance on its core tile. This is OK, it has been tested with the motherboard VRAM and that works just as fine as regular CMA memory. The memory is assigned to the device using the memory-region device tree property and a "shared-dma-pool" reserved memory pool like this: reserved-memory { #address-cells = <1>; #size-cells = <1>; ranges; vram: vram@4800 { compatible = "shared-dma-pool"; reg = <0x4800 0x0080>; no-map; }; }; clcd@1f000 { compatible = "arm,pl111", "arm,primecell"; (...) memory-region = <>; }·; Cc: Liviu DudauCc: Mali DP Maintainers Signed-off-by: Linus Walleij --- ChangeLog v1->v2: - Make sure to also call of_reserved_mem_device_release() at remove() and errorpath. --- drivers/gpu/drm/pl111/pl111_drv.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/pl111/pl111_drv.c b/drivers/gpu/drm/pl111/pl111_drv.c index 4621259d5387..bc57c15d9fe4 100644 --- a/drivers/gpu/drm/pl111/pl111_drv.c +++ b/drivers/gpu/drm/pl111/pl111_drv.c @@ -60,6 +60,7 @@ #include #include #include +#include #include #include @@ -257,6 +258,10 @@ static int pl111_amba_probe(struct amba_device *amba_dev, drm->dev_private = priv; priv->variant = variant; + ret = of_reserved_mem_device_init(dev); + if (!ret) + dev_info(dev, "using device-specific reserved memory\n"); + if (of_property_read_u32(dev->of_node, "max-memory-bandwidth", >memory_bw)) { dev_info(dev, "no max memory bandwidth specified, assume unlimited\n"); @@ -275,7 +280,8 @@ static int pl111_amba_probe(struct amba_device *amba_dev, priv->regs = devm_ioremap_resource(dev, _dev->res); if (IS_ERR(priv->regs)) { dev_err(dev, "%s failed mmio\n", __func__); - return PTR_ERR(priv->regs); + ret = PTR_ERR(priv->regs); + goto mem_rel; } /* This may override some variant settings */ @@ -305,11 +311,15 @@ static int pl111_amba_probe(struct amba_device *amba_dev, dev_unref: drm_dev_unref(drm); +mem_rel: + of_reserved_mem_device_release(dev); + return ret; } static int pl111_amba_remove(struct amba_device *amba_dev) { + struct device *dev = _dev->dev; struct drm_device *drm = amba_get_drvdata(amba_dev); struct pl111_drm_dev_private *priv = drm->dev_private; @@ -319,6 +329,7 @@ static int pl111_amba_remove(struct amba_device *amba_dev) drm_panel_bridge_remove(priv->bridge); drm_mode_config_cleanup(drm); drm_dev_unref(drm); + of_reserved_mem_device_release(dev); return 0; } -- 2.14.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/7] arm64: dts: renesas: eagle: Add ADV7511W and HDMI output
Hi Laurent, On Fri, Apr 06, 2018 at 04:51:11PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Friday, 6 April 2018 16:08:12 EEST Jacopo Mondi wrote: > > From: Niklas Söderlund> > > > Enable HDMI output adding the HDMI connector and the ADV7511W, connected > > to THC63LVD1024 LVDS decoder output. > > > > Signed-off-by: Niklas Söderlund > > Signed-off-by: Jacopo Mondi > > --- > > arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 51 ++- > > 1 file changed, 50 insertions(+), 1 deletion(-) > > > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > > b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index 9d0e65d..e9f7b83 > > 100644 > > --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > > +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > > @@ -32,6 +32,17 @@ > > reg = <0x0 0x4800 0x0 0x3800>; > > }; > > > > + hdmi-out { > > + compatible = "hdmi-connector"; > > + type = "a"; > > + > > + port { > > + hdmi_con_out: endpoint { > > + remote-endpoint = <_out>; > > + }; > > + }; > > + }; > > + > > thc63lvd1024: lvds-decoder { > > compatible = "thine,thc63lvd1024"; > > > > @@ -41,11 +52,17 @@ > > > > port@0 { > > reg = <0>; > > - > > This is unrelated, if you don't want a blank line here remove it from patch > 6/7 :-) No, you're right, this is a leftover from me splitting a single a patch in 3. According to your comments on other patches in the series I shouldn't have done that to begin with :) Thanks j > > > thc63lvd1024_in: endpoint { > > remote-endpoint = <_out>; > > }; > > }; > > + > > + port@2 { > > + reg = <2>; > > + thc63lvd1024_out: endpoint { > > + remote-endpoint = <_in>; > > + }; > > + }; > > }; > > }; > > }; > > @@ -85,6 +102,38 @@ > > gpio-controller; > > #gpio-cells = <2>; > > }; > > + > > + hdmi@39 { > > + compatible = "adi,adv7511w"; > > + reg = <0x39>; > > + interrupt-parent = <>; > > + interrupts = <20 IRQ_TYPE_LEVEL_LOW>; > > + > > + adi,input-depth = <8>; > > + adi,input-colorspace = "rgb"; > > + adi,input-clock = "1x"; > > + adi,input-style = <1>; > > + adi,input-justification = "evenly"; > > + > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + adv7511_in: endpoint { > > + remote-endpoint = <_out>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + adv7511_out: endpoint { > > + remote-endpoint = <_con_out>; > > + }; > > + }; > > + }; > > + }; > > }; > > > > { > > With patches 5/7, 6/7 and 7/7 merged together and the pinmux removed, > > Reviewed-by: Laurent Pinchart > > -- > Regards, > > Laurent Pinchart > > > signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 199101] AMDGPU Fury X random screen flicker on Linux kernel 4.16rc5
https://bugzilla.kernel.org/show_bug.cgi?id=199101 --- Comment #15 from Harry Wentland (harry.wentl...@amd.com) --- We reproduced the issue and have someone looking into it. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 0/2] drm: Add Thine THC63LVD1024 LVDS decoder bridge
Hi Laurent, On Fri, Apr 06, 2018 at 04:53:43PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > On Friday, 6 April 2018 16:08:05 EEST Jacopo Mondi wrote: > > Hello, > >this series enables HDMI display on V3M Eagle board. > > > > The series is based on Geert's "renesas-drivers-2018-04-03-v4.16" with > > THC63LVD1024 driver on top (cfr. my in review series: > > "[PATCH v7 0/2] drm: Add Thine THC63LVD1024 LVDS decoder bridge") > > This isn't a good base for development, as you would pull way too many > dependencies in. Could you please base v8 on top of v4.17-rc1 (or if you get > to post it before v4.17-rc1 gets merged, you can use Linus' master, as the > ARM64 DT pull requests for v4.17-rc1 have been merged) ? It will then be ready > for Simon to pull in his v4.18 branch. I used renesas-drivers as it already contains partial r8a77970 support which is not there in v4.16 (PFC, GPIO, SCIF...) I should wait for v4.17-rc1 to come out and re-propose on top of that probably. > > > This series includes some preliminary work from Sergei and Niklas. I have > > reworked the two final patches from Niklas to enable DU first, add the LVDS > > decoder node, and finally add the ADV7511W chip and enable HDMI output. > > > > A branch for testing is available at: > > git://jmondi.org/linux v3m/renesas-drivers-2018-04-03-v4.16/v7-eagle-dts > > > > Thanks > >j > > > > Jacopo Mondi (2): > > arm64: dts: renesas: eagle: Enable DU > > arm64: dts: renesas: eagle: Add LVDS decoder > > > > Niklas Söderlund (2): > > arm64: dts: renesas: r8a77970: add the LVDS instance > > arm64: dts: renesas: eagle: Add ADV7511W and HDMI output > > > > Sergei Shtylyov (3): > > arm64: dts: renesas: r8a77970: add FCPVD support > > arm64: dts: renesas: r8a77970: add VSPD support > > arm64: dts: renesas: r8a77970: add DU support > > > > arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 89 +++ > > arch/arm64/boot/dts/renesas/r8a77970.dtsi | 73 + > > 2 files changed, 162 insertions(+) > > > > -- > Regards, > > Laurent Pinchart > > > signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 105911] DVI-D monitor connected to DVI-I output not detected (regression from kernel 4.14.32 to kernel 4.16.0 using graphic core)
https://bugs.freedesktop.org/show_bug.cgi?id=105911 --- Comment #9 from Harry Wentland--- Can you capture the 4.16 dmesg with DC again with the amdgpu.dc_log=1 module option? Do you see any activity on dmesg with dc_log=1 when you hotplug the non-detected display? -- 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 v7 0/2] drm: Add Thine THC63LVD1024 LVDS decoder bridge
Hi Jacopo, On Friday, 6 April 2018 16:08:05 EEST Jacopo Mondi wrote: > Hello, >this series enables HDMI display on V3M Eagle board. > > The series is based on Geert's "renesas-drivers-2018-04-03-v4.16" with > THC63LVD1024 driver on top (cfr. my in review series: > "[PATCH v7 0/2] drm: Add Thine THC63LVD1024 LVDS decoder bridge") This isn't a good base for development, as you would pull way too many dependencies in. Could you please base v8 on top of v4.17-rc1 (or if you get to post it before v4.17-rc1 gets merged, you can use Linus' master, as the ARM64 DT pull requests for v4.17-rc1 have been merged) ? It will then be ready for Simon to pull in his v4.18 branch. > This series includes some preliminary work from Sergei and Niklas. I have > reworked the two final patches from Niklas to enable DU first, add the LVDS > decoder node, and finally add the ADV7511W chip and enable HDMI output. > > A branch for testing is available at: > git://jmondi.org/linux v3m/renesas-drivers-2018-04-03-v4.16/v7-eagle-dts > > Thanks >j > > Jacopo Mondi (2): > arm64: dts: renesas: eagle: Enable DU > arm64: dts: renesas: eagle: Add LVDS decoder > > Niklas Söderlund (2): > arm64: dts: renesas: r8a77970: add the LVDS instance > arm64: dts: renesas: eagle: Add ADV7511W and HDMI output > > Sergei Shtylyov (3): > arm64: dts: renesas: r8a77970: add FCPVD support > arm64: dts: renesas: r8a77970: add VSPD support > arm64: dts: renesas: r8a77970: add DU support > > arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 89 +++ > arch/arm64/boot/dts/renesas/r8a77970.dtsi | 73 + > 2 files changed, 162 insertions(+) > -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 7/7] arm64: dts: renesas: eagle: Add ADV7511W and HDMI output
Hi Jacopo, Thank you for the patch. On Friday, 6 April 2018 16:08:12 EEST Jacopo Mondi wrote: > From: Niklas Söderlund> > Enable HDMI output adding the HDMI connector and the ADV7511W, connected > to THC63LVD1024 LVDS decoder output. > > Signed-off-by: Niklas Söderlund > Signed-off-by: Jacopo Mondi > --- > arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 51 ++- > 1 file changed, 50 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index 9d0e65d..e9f7b83 > 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > @@ -32,6 +32,17 @@ > reg = <0x0 0x4800 0x0 0x3800>; > }; > > + hdmi-out { > + compatible = "hdmi-connector"; > + type = "a"; > + > + port { > + hdmi_con_out: endpoint { > + remote-endpoint = <_out>; > + }; > + }; > + }; > + > thc63lvd1024: lvds-decoder { > compatible = "thine,thc63lvd1024"; > > @@ -41,11 +52,17 @@ > > port@0 { > reg = <0>; > - This is unrelated, if you don't want a blank line here remove it from patch 6/7 :-) > thc63lvd1024_in: endpoint { > remote-endpoint = <_out>; > }; > }; > + > + port@2 { > + reg = <2>; > + thc63lvd1024_out: endpoint { > + remote-endpoint = <_in>; > + }; > + }; > }; > }; > }; > @@ -85,6 +102,38 @@ > gpio-controller; > #gpio-cells = <2>; > }; > + > + hdmi@39 { > + compatible = "adi,adv7511w"; > + reg = <0x39>; > + interrupt-parent = <>; > + interrupts = <20 IRQ_TYPE_LEVEL_LOW>; > + > + adi,input-depth = <8>; > + adi,input-colorspace = "rgb"; > + adi,input-clock = "1x"; > + adi,input-style = <1>; > + adi,input-justification = "evenly"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + adv7511_in: endpoint { > + remote-endpoint = <_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + adv7511_out: endpoint { > + remote-endpoint = <_con_out>; > + }; > + }; > + }; > + }; > }; > > { With patches 5/7, 6/7 and 7/7 merged together and the pinmux removed, Reviewed-by: Laurent Pinchart -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 6/7] arm64: dts: renesas: eagle: Add LVDS decoder
Hi Jacopo, Thank you for the patch. On Friday, 6 April 2018 16:08:11 EEST Jacopo Mondi wrote: > The R-Car V3M Eagle board includes a transparent THC63LVD1024 LVDS > decoder, connected to the on-chip LVDS encoder output on one side > and to the not-yet-described HDMI encoder ADV7511W on the other one. > > As the decoder does not need any configuration it has been so-far > omitted from DTS. Now that a driver is available, describe it in DT > as well. As explained in my review of patch 5/7, I'd merge 5/7, 6/7 and 7/7 all together as there's little use for enabling the LVDS decoder if there's nothing connected at its output. Note also how this patch alone, without 7/7, wouldn't comply with the LVDS decoder DT bindings that state that port@2 is mandatory. > Signed-off-by: Jacopo Mondi> Reviewed-by: Andrzej Hajda > --- > arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 29 +++ > 1 file changed, 29 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index 144b847..9d0e65d > 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > @@ -31,6 +31,23 @@ > /* first 128MB is reserved for secure area. */ > reg = <0x0 0x4800 0x0 0x3800>; > }; > + > + thc63lvd1024: lvds-decoder { > + compatible = "thine,thc63lvd1024"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + thc63lvd1024_in: endpoint { > + remote-endpoint = <_out>; > + }; > + }; > + }; > + }; > }; > > { > @@ -104,3 +121,15 @@ > pinctrl-names = "default"; > status = "okay"; > }; > + > + { > + status = "okay"; > + > + ports { > + port@1 { > + lvds0_out: endpoint { > + remote-endpoint = <_in>; > + }; > + }; > + }; > +}; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/7] arm64: dts: renesas: eagle: Enable DU
Hi again, On Friday, 6 April 2018 16:45:16 EEST Laurent Pinchart wrote: > On Friday, 6 April 2018 16:08:10 EEST Jacopo Mondi wrote: > > Enable DU for Renesas R-Car V3M Eagle board. > > > > Signed-off-by: Jacopo Mondi> > --- > > > > arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 11 +++ > > 1 file changed, 11 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > > b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index 3c5f598..144b847 > > 100644 > > --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > > +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > > @@ -76,6 +76,11 @@ > > > > function = "i2c0"; > > > > }; > > > > + du_pins: du { > > + groups = "du_rgb666", "du_sync", "du_oddf", "du_clk_out"; > > + function = "du"; > > + }; > > As far as I can tell the DU parallel output isn't used on the Eagle board, > but is used on the Eagle expansion board. I would move this to patch 7/7 in > this series. My bad, patch 7/7 describes the on-board HDMI encoder, not the one on the expansion board. I would thus drop pinmux completely for now until we add support for the expansion board. > > scif0_pins: scif0 { > > > > groups = "scif0_data"; > > function = "scif0"; > > > > @@ -93,3 +98,9 @@ > > > > status = "okay"; > > > > }; > > > > + > > + { > > + pinctrl-0 = <_pins>; > > + pinctrl-names = "default"; > > These two properties should be moved to patch 7/7 too. So this should be removed. > > + status = "okay"; > > +}; > > There's little use for enabling the DU in DT if you have no output port > described. I'd move this to patch 6/7. And I'd merge the status attribute and patches 6/7 and 7/7 all together. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 5/7] arm64: dts: renesas: eagle: Enable DU
Hi Jacopo, Thank you for the patch. On Friday, 6 April 2018 16:08:10 EEST Jacopo Mondi wrote: > Enable DU for Renesas R-Car V3M Eagle board. > > Signed-off-by: Jacopo Mondi> --- > arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts index 3c5f598..144b847 > 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > +++ b/arch/arm64/boot/dts/renesas/r8a77970-eagle.dts > @@ -76,6 +76,11 @@ > function = "i2c0"; > }; > > + du_pins: du { > + groups = "du_rgb666", "du_sync", "du_oddf", "du_clk_out"; > + function = "du"; > + }; As far as I can tell the DU parallel output isn't used on the Eagle board, but is used on the Eagle expansion board. I would move this to patch 7/7 in this series. > scif0_pins: scif0 { > groups = "scif0_data"; > function = "scif0"; > @@ -93,3 +98,9 @@ > > status = "okay"; > }; > + > + { > + pinctrl-0 = <_pins>; > + pinctrl-names = "default"; These two properties should be moved to patch 7/7 too. > + status = "okay"; > +}; There's little use for enabling the DU in DT if you have no output port described. I'd move this to patch 6/7. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/7] arm64: dts: renesas: r8a77970: add the LVDS instance
Hi Jacopo, Thank you for the patch. On Friday, 6 April 2018 16:08:09 EEST Jacopo Mondi wrote: > From: Niklas Söderlund> > Add the LVDS device to r8a77970.dtsi in a disabled state. Also connect > the it to the LVDS output of the DU. While at it align the endpoint name > of the du to du_out_lvds0 which is used in other Renesas DTS files to > describe this link. The endpoint could be renamed in patch 3/7, but it's not a big deal. > Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart > --- > arch/arm64/boot/dts/renesas/r8a77970.dtsi | 29 +++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi > b/arch/arm64/boot/dts/renesas/r8a77970.dtsi index e649e86..b48d62c 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi > @@ -658,7 +658,34 @@ > > port@1 { > reg = <1>; > - du_out_lvds: endpoint { > + du_out_lvds0: endpoint { > + remote-endpoint = <_in>; > + }; > + }; > + }; > + }; > + > + lvds0: lvds@feb9 { > + compatible = "renesas,r8a77970-lvds"; > + reg = <0 0xfeb9 0 0x14>; > + clocks = < CPG_MOD 727>; > + power-domains = < R8A77970_PD_ALWAYS_ON>; > + resets = < 727>; > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + lvds0_in: endpoint { > + remote-endpoint = > <_out_lvds0>; > + }; > + }; > + port@1 { > + reg = <1>; > + lvds0_out: endpoint { > }; > }; > }; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/7] arm64: dts: renesas: r8a77970: add DU support
Hi Jacopo, Thank you for the patch. On Friday, 6 April 2018 16:08:08 EEST Jacopo Mondi wrote: > From: Sergei Shtylyov> > Define the generic R8A77970 part of the DU device node. > > Based on the original (and large) patch by Daisuke Matsushita > . > > Signed-off-by: Vladimir Barinov > Signed-off-by: Sergei Shtylyov > Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart > --- > arch/arm64/boot/dts/renesas/r8a77970.dtsi | 28 > 1 file changed, 28 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi > b/arch/arm64/boot/dts/renesas/r8a77970.dtsi index db06c94..e649e86 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi > @@ -635,6 +635,34 @@ > resets = < 623>; > renesas,fcp = <>; > }; > + > + du: display@feb0 { > + compatible = "renesas,du-r8a77970"; > + reg = <0 0xfeb0 0 0x8>; > + interrupts = ; > + clocks = < CPG_MOD 724>; > + clock-names = "du.0"; > + power-domains = < R8A77970_PD_ALWAYS_ON>; > + vsps = <>; > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + du_out_rgb: endpoint { > + }; > + }; > + > + port@1 { > + reg = <1>; > + du_out_lvds: endpoint { > + }; > + }; > + }; > + }; > }; > > timer { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/7] arm64: dts: renesas: r8a77970: add FCPVD support
Hi Jacopo, Thank you for the patch. On Friday, 6 April 2018 16:08:06 EEST Jacopo Mondi wrote: > From: Sergei Shtylyov> > Describe FCPVD0 in the R8A77970 device tree; it will be used by VSPD0 in > the next patch... > > Based on the original (and large) patch by Daisuke Matsushita > . > > Signed-off-by: Vladimir Barinov > Signed-off-by: Sergei Shtylyov > Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart > --- > arch/arm64/boot/dts/renesas/r8a77970.dtsi | 8 > 1 file changed, 8 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi > b/arch/arm64/boot/dts/renesas/r8a77970.dtsi index e8358d9..71f466d 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi > @@ -617,6 +617,14 @@ > #address-cells = <1>; > #size-cells = <0>; > }; > + > + fcpvd0: fcp@fea27000 { > + compatible = "renesas,fcpv"; > + reg = <0 0xfea27000 0 0x200>; > + clocks = < CPG_MOD 603>; > + power-domains = < R8A77970_PD_ALWAYS_ON>; > + resets = < 603>; > + }; > }; > > timer { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/7] arm64: dts: renesas: r8a77970: add VSPD support
On Friday, 6 April 2018 16:08:07 EEST Jacopo Mondi wrote: > From: Sergei Shtylyov> > Describe VSPD0 in the R8A77970 device tree; it will be used by DU in > the next patch... > > Based on the original (and large) patch by Daisuke Matsushita > . > > Signed-off-by: Vladimir Barinov > Signed-off-by: Sergei Shtylyov > Signed-off-by: Niklas Söderlund > --- > arch/arm64/boot/dts/renesas/r8a77970.dtsi | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/arm64/boot/dts/renesas/r8a77970.dtsi > b/arch/arm64/boot/dts/renesas/r8a77970.dtsi index 71f466d..db06c94 100644 > --- a/arch/arm64/boot/dts/renesas/r8a77970.dtsi > +++ b/arch/arm64/boot/dts/renesas/r8a77970.dtsi > @@ -625,6 +625,16 @@ > power-domains = < R8A77970_PD_ALWAYS_ON>; > resets = < 603>; > }; > + > + vspd0: vsp@fea2 { > + compatible = "renesas,vsp2"; > + reg = <0 0xfea2 0 0x4000>; You need to extend the memory region to include the V6_CLUTn_TBL* registers. I would recommend simply extending it to 0x8000 as all other VSP instances, even if the registers at 0x7000-0x7fff are not implemented. Apart from that, Reviewed-by: Laurent Pinchart > + interrupts = ; > + clocks = < CPG_MOD 623>; > + power-domains = < R8A77970_PD_ALWAYS_ON>; > + resets = < 623>; > + renesas,fcp = <>; > + }; > }; > > timer { -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 2/2] drm: bridge: Add thc63lvd1024 LVDS decoder driver
Hi Jacopo, Thank you for the patch. On Friday, 6 April 2018 15:41:57 EEST Jacopo Mondi wrote: > Add DRM bridge driver for Thine THC63LVD1024 LVDS to digital parallel > output converter. > > Signed-off-by: Jacopo Mondi> Reviewed-by: Andrzej Hajda > Reviewed-by: Niklas Söderlund > --- > drivers/gpu/drm/bridge/Kconfig| 6 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/thc63lvd1024.c | 212 +++ > 3 files changed, 219 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/thc63lvd1024.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index 3aa65bd..42c9c2d 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -93,6 +93,12 @@ config DRM_SII9234 > It is an I2C driver, that detects connection of MHL bridge > and starts encapsulation of HDMI signal. > > +config DRM_THINE_THC63LVD1024 > + tristate "Thine THC63LVD1024 LVDS decoder bridge" > + depends on OF > + ---help--- > + Thine THC63LVD1024 LVDS/parallel converter driver. > + > config DRM_TOSHIBA_TC358767 > tristate "Toshiba TC358767 eDP bridge" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile > b/drivers/gpu/drm/bridge/Makefile index 373eb28..fd90b16 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -8,6 +8,7 @@ obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o > obj-$(CONFIG_DRM_SIL_SII8620) += sil-sii8620.o > obj-$(CONFIG_DRM_SII902X) += sii902x.o > obj-$(CONFIG_DRM_SII9234) += sii9234.o > +obj-$(CONFIG_DRM_THINE_THC63LVD1024) += thc63lvd1024.o > obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/ > obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/ > diff --git a/drivers/gpu/drm/bridge/thc63lvd1024.c > b/drivers/gpu/drm/bridge/thc63lvd1024.c new file mode 100644 > index 000..c8fad9c > --- /dev/null > +++ b/drivers/gpu/drm/bridge/thc63lvd1024.c > @@ -0,0 +1,212 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * THC63LVD1024 LVDS to parallel data DRM bridge driver. > + * > + * Copyright (C) 2018 Jacopo Mondi > + */ > + > +#include > +#include > +#include > + > +#include > +#include > +#include You should include slab.h as you're using devm_kzalloc(). > + > +enum thc63_ports { > + THC63_LVDS_IN0, > + THC63_LVDS_IN1, > + THC63_RGB_OUT0, > + THC63_RGB_OUT1, > +}; > + > +struct thc63_dev { > + struct device *dev; > + > + struct regulator *vcc; > + > + struct gpio_desc *pdwn; pwdn ? > + struct gpio_desc *oe; > + > + struct drm_bridge bridge; > + struct drm_bridge *next; > +}; > + > +static inline struct thc63_dev *to_thc63(struct drm_bridge *bridge) > +{ > + return container_of(bridge, struct thc63_dev, bridge); > +} > + > +static int thc63_attach(struct drm_bridge *bridge) > +{ > + struct thc63_dev *thc63 = to_thc63(bridge); > + > + return drm_bridge_attach(bridge->encoder, thc63->next, bridge); > +} > + > +static void thc63_enable(struct drm_bridge *bridge) > +{ > + struct thc63_dev *thc63 = to_thc63(bridge); > + int ret; > + > + ret = regulator_enable(thc63->vcc); > + if (ret) { > + dev_err(thc63->dev, > + "Failed to enable regulator \"vcc\": %d\n", ret); > + return; > + } > + > + if (thc63->pdwn) > + gpiod_set_value(thc63->pdwn, 0); > + > + if (thc63->oe) > + gpiod_set_value(thc63->oe, 1); You don't need to check the gpio_desc pointers first, gpiod_set_value() is a no-op if the pointer is NULL. > +} > + > +static void thc63_disable(struct drm_bridge *bridge) > +{ > + struct thc63_dev *thc63 = to_thc63(bridge); > + int ret; > + > + if (thc63->oe) > + gpiod_set_value(thc63->oe, 0); > + > + if (thc63->pdwn) > + gpiod_set_value(thc63->pdwn, 1); Same here. > + ret = regulator_disable(thc63->vcc); > + if (ret) > + dev_err(thc63->dev, > + "Failed to disable regulator \"vcc\": %d\n", ret); > +} > + > +static const struct drm_bridge_funcs thc63_bridge_func = { > + .attach = thc63_attach, > + .enable = thc63_enable, > + .disable = thc63_disable, > +}; > + > +static int thc63_parse_dt(struct thc63_dev *thc63) > +{ > + struct device_node *thc63_out; > + struct device_node *remote; > + > + thc63_out = of_graph_get_endpoint_by_regs(thc63->dev->of_node, > + THC63_RGB_OUT0, -1); > + if (!thc63_out) { > + dev_err(thc63->dev, "Missing endpoint in port@%u\n", > + THC63_RGB_OUT0); > + return -ENODEV; > + } > + > + remote = of_graph_get_remote_port_parent(thc63_out); > + of_node_put(thc63_out); > + if
Re: [PATCH v7 0/2] drm: Add Thine THC63LVD1024 LVDS decoder bridge
Sorry for the mess subject should have been Subject: [PATCH 0/7] V3M-Eagle display enablement I copied the wrong one from another cover letter... On Fri, Apr 06, 2018 at 03:08:05PM +0200, Jacopo Mondi wrote: > Hello, >this series enables HDMI display on V3M Eagle board. > > The series is based on Geert's "renesas-drivers-2018-04-03-v4.16" with > THC63LVD1024 driver on top (cfr. my in review series: > "[PATCH v7 0/2] drm: Add Thine THC63LVD1024 LVDS decoder bridge") > > This series includes some preliminary work from Sergei and Niklas. I have > reworked the two final patches from Niklas to enable DU first, add the LVDS > decoder node, and finally add the ADV7511W chip and enable HDMI output. > > A branch for testing is available at: > git://jmondi.org/linux v3m/renesas-drivers-2018-04-03-v4.16/v7-eagle-dts > > Thanks >j > > Jacopo Mondi (2): > arm64: dts: renesas: eagle: Enable DU > arm64: dts: renesas: eagle: Add LVDS decoder > > Niklas Söderlund (2): > arm64: dts: renesas: r8a77970: add the LVDS instance > arm64: dts: renesas: eagle: Add ADV7511W and HDMI output > > Sergei Shtylyov (3): > arm64: dts: renesas: r8a77970: add FCPVD support > arm64: dts: renesas: r8a77970: add VSPD support > arm64: dts: renesas: r8a77970: add DU support > > arch/arm64/boot/dts/renesas/r8a77970-eagle.dts | 89 > ++ > arch/arm64/boot/dts/renesas/r8a77970.dtsi | 73 + > 2 files changed, 162 insertions(+) > > -- > 2.7.4 > signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v7 1/2] dt-bindings: display: bridge: Document THC63LVD1024 LVDS decoder
Hi Jacopo, Thank you for the patch. On Friday, 6 April 2018 15:41:56 EEST Jacopo Mondi wrote: > Document Thine THC63LVD1024 LVDS decoder device tree bindings. > > Signed-off-by: Jacopo Mondi> Reviewed-by: Andrzej Hajda > Reviewed-by: Niklas Söderlund > Reviewed-by: Laurent Pinchart > --- > .../bindings/display/bridge/thine,thc63lvd1024.txt | 60 +++ > 1 file changed, 60 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > > diff --git > a/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > new file mode 100644 > index 000..1191f17 > --- /dev/null > +++ > b/Documentation/devicetree/bindings/display/bridge/thine,thc63lvd1024.txt > @@ -0,0 +1,60 @@ > +Thine Electronics THC63LVD1024 LVDS decoder > +--- > + > +The THC63LVD1024 is a dual link LVDS receiver designed to convert LVDS > streams > +to parallel data outputs. The chip supports single/dual input/output modes, > +handling up to two two input LVDS stream and up to two digital CMOS/TTL s/to two two/to two/ s/stream/streams/ > outputs. > + > +Single or dual operation modes, output data mapping and DDR output modes > are > +configured through input signals and the chip does not expose any control > bus. > + > +Required properties: > +- compatible: Shall be "thine,thc63lvd1024" > + > +Optional properties: > +- vcc-supply: Power supply for TTL output, TTL CLOCKOUT signal, LVDS input, > + PPL and digital circuitry > +- powerdown-gpios: Power down GPIO signal, pin name "/PDWN". Active low > +- enable-gpios: Output enable GPIO signal, pin name "OE". Active high As Rob mentioned in a reply to v6, we currently use "enable" as the inverse of "powerdown". I would call this one oe-gpios instead. Quoting Rob: "Debating "oe" vs. "output-enable" is bikeshedding IMO. Anyone familiar with h/w design should recognize OE." > + > +The THC63LVD1024 video port connections are modeled according > +to OF graph bindings specified by Documentation/devicetree/bindings/ > graph.txt > + > +Required video port nodes: > +- port@0: First LVDS input port > +- port@2: First digital CMOS/TTL parallel output > + > +Optional video port nodes: > +- port@1: Second LVDS input port > +- port@3: Second digital CMOS/TTL parallel output > + > +Example: > + > + > + thc63lvd1024: lvds-decoder { > + compatible = "thine,thc63lvd1024"; > + > + vcc-supply = <_lvds_vcc>; > + powerdown-gpios = < 15 GPIO_ACTIVE_LOW>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + > + lvds_dec_in_0: endpoint { > + remote-endpoint = <_out>; > + }; > + }; > + > + port@2{ > + reg = <2>; > + > + lvds_dec_out_2: endpoint { > + remote-endpoint = <_in>; > + }; > + }; > + }; > + }; -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/virtio: remove drm_dev_set_unique workaround
Hi, > What I could see as justified is a loud comment in drm_virtio_init(), > just above the call to drm_dev_set_unique(), explaining why it is > necessary to set "unique" manually. The reason is that virtio-vga > technically has "virtio_bus", not "pci_bus_type", for bus type, and so > the generic PCI BusID-setting will not cover it. Yep, that sums up the underlying problem. You can also see that in sysfs: root@fedora ~# readlink /sys/class/drm/card0 ../../devices/pci:00/:00:01.0/virtio0/drm/card0 ^^^ extra level here. That is the reason why drm_virtio_init() goes lookup the parent device, checks if it happens to be pci, if so use the parent device to construct the "pci:..." unique id for the virtio device (and also use the pci device ressources to kick out the firmware framebuffer). cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/virtio: remove drm_dev_set_unique workaround
Hi Laszlo, On 6 April 2018 at 13:15, Laszlo Ersekwrote: > On 04/04/18 19:29, Laszlo Ersek wrote: >> Hi Emil, >> >> On 04/03/18 19:13, Emil Velikov wrote: >>> On 29 March 2018 at 12:17, Laszlo Ersek wrote: On 03/28/18 16:35, Emil Velikov wrote: > On 28 March 2018 at 11:27, Laszlo Ersek wrote: >> On 03/28/18 03:24, Emil Velikov wrote: >>> Gents, can someone double-check this please? Just in case. >> >> I guess I should test whether this series regresses the use case >> described in c2cbc38b97; is that correct? >> > Precisely. > [3] https://github.com/evelikov/linux/commits/ioctl-cleanups Unfortunately, this series seems to reintroduce the regression fixed / described earlier in commit c2cbc38b97. >>> Thank you very much for testing. >>> >>> Believe I've tracked it down to a broken commit from 2014 ;-) >>> Please try the following branch [1] - it's untested, but I'm 99% sure >>> it will work like a charm. >> >> Alas, it does not work. > > I've done some more digging. Let me quote the commit message on the > proposed patch again: > >> Ealier commit a325725633c26aa66ab940f762a6b0778edf76c0 did not attribute >> that virtio can be either PCI or a platform device and removed the >> .set_busid hook. Whereas only the "platform" instance should have been >> removed. >> >> Since then, two things have happened: >> - the driver manually calls drm_dev_set_unique, addressing the Xserver >> regression - see commit 9785b4321b0bd701f8d21d3d3c676a7739a5cf22 >> - core itself calls drm_pci_set_busid, on drm_set_busid IOCTL setting >> the busid, so we don't need to fallback to dev->unique - see commit >> 5c484cee7ef9c4fd29fa0ba09640d55960977145 >> >> With that in place we can remove the local workaround. > > This write-up of events is not precise enough. Instead, I think the > timeline is as follows: > > (1) Commit a325725633c2 ("drm: Lobotomize set_busid nonsense for !pci > drivers", 2016-06-21) introduced the regression. By removing > drm_virtio_set_busid(), commit a325725633c2 changed the behavior of > the following function: > >> static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv) >> { >> struct drm_master *master = file_priv->master; >> int ret; >> >> if (master->unique != NULL) >> drm_unset_busid(dev, master); >> >> if (dev->driver->set_busid) { >> ret = dev->driver->set_busid(dev, master); >> if (ret) { >> drm_unset_busid(dev, master); >> return ret; >> } >> } else { >> WARN_ON(!dev->unique); >> master->unique = kstrdup(dev->unique, GFP_KERNEL); >> if (master->unique) >> master->unique_len = strlen(dev->unique); >> } >> >> return 0; >> } > > When a325725633c2 removed drm_virtio_set_busid(), drm_set_busid() > started taking the second branch, which wasn't doing the right thing > for virtio-vga at the time. > > (2) There were two ways to fix the regression: either (a) return > drm_set_busid() to taking the first branch, or (b) tweak the > virtio-vga driver so that the second branch in drm_set_busid() start > to behave right. > > My commit c2cbc38b9715 ("drm: virtio: reinstate > drm_virtio_set_busid()", 2016-10-04) implemented (a), by reverting a > few chunks of a325725633c2. > > (3) Gerd thought that approach (b) was superior (and I totally defer to > him on this, now that I'm learning of his patches in the first place > :) ). Namely, in commit 9785b4321b0b ("drm/virtio: fix busid > regression", 2016-11-15), he implemented approach (b), and right > after, in commit 1775db074a32 ("Revert "drm: virtio: reinstate > drm_virtio_set_busid()"", 2016-11-15), he undid my commit > c2cbc38b9715. > > In other words, Gerd replaced approach (a) with approach (b). > > (4) Subsequently, commit 5c484cee7ef9 ("drm: Remove > drm_driver->set_busid hook", 2017-06-20), changed drm_set_busid() > to the following: > >> static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv) >> { >> struct drm_master *master = file_priv->master; >> int ret; >> >> if (master->unique != NULL) >> drm_unset_busid(dev, master); >> >> if (dev_is_pci(dev->dev)) { >> ret = drm_pci_set_busid(dev, master); >> if (ret) { >> drm_unset_busid(dev, master); >> return ret; >> } >> } else { >> WARN_ON(!dev->unique); >> master->unique = kstrdup(dev->unique, GFP_KERNEL); >> if (master->unique) >> master->unique_len = strlen(dev->unique); >> } >> >> return 0; >> } > > Perhaps surprisingly, this change did not
Re: [RfC PATCH] Add udmabuf misc device
On 04/06/2018 02:57 PM, Gerd Hoffmann wrote: Hi, I fail to see any common ground for xen-zcopy and udmabuf ... Does the above mean you can assume that xen-zcopy and udmabuf can co-exist as two different solutions? Well, udmabuf route isn't fully clear yet, but yes. See also gvt (intel vgpu), where the hypervisor interface is abstracted away into a separate kernel modules even though most of the actual vgpu emulation code is common. Thank you for your input, I'm just trying to figure out which of the three z-copy solutions intersect and how much And what about hyper-dmabuf? No idea, didn't look at it in detail. Looks pretty complex from a distant view. Maybe because it tries to build a communication framework using dma-bufs instead of a simple dma-buf passing mechanism. Yes, I am looking at it now, trying to figure out the full story and its implementation. BTW, Intel guys were about to share some test application for hyper-dmabuf, maybe I have missed one. It could probably better explain the use-cases and the complexity they have in hyper-dmabuf. Like xen-zcopy it seems to depend on the idea that the hypervisor manages all memory it is easy for guests to share pages with the help of the hypervisor. So, for xen-zcopy we were not trying to make it generic, it just solves display (dumb) zero-copying use-cases for Xen. We implemented it as a DRM helper driver because we can't see any other use-cases as of now. For example, we also have Xen para-virtualized sound driver, but its buffer memory usage is not comparable to what display wants and it works somewhat differently (e.g. there is no "frame done" event, so one can't tell when the sound buffer can be "flipped"). At the same time, we do not use virtio-gpu, so this could probably be one more candidate for shared dma-bufs some day. Which simply isn't the case on kvm. hyper-dmabuf and xen-zcopy could maybe share code, or hyper-dmabuf build on top of xen-zcopy. Hm, I can imagine that: xen-zcopy could be a library code for hyper-dmabuf in terms of implementing all that page sharing fun in multiple directions, e.g. Host->Guest, Guest->Host, Guest<->Guest. But I'll let Matt and Dongwon to comment on that. cheers, Gerd Thank you, Oleksandr P.S. Sorry for making your original mail thread to discuss things much broader than your RFC... ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2] Add udmabuf misc device
Am 06.04.2018 um 11:33 schrieb Gerd Hoffmann: Hi, The pages backing a DMA-buf are not allowed to move (at least not without a patch set I'm currently working on), but for certain MM operations to work correctly you must be able to modify the page tables entries and move the pages backing them around. For example try to use fork() with some copy on write pages with this approach. You will find that you have only two options to correctly handle this. The fork() issue should go away with shared memory pages (no cow). I guess this is the reason why vgem is internally backed by shmem. Yes, exactly that is also an approach which should work fine. Just don't try to get this working with get_user_pages(). Hmm. So I could try to limit the udmabuf driver to shmem too (i.e. have the ioctl take a shmem filehandle and offset instead of a virtual address). But maybe it is better then to just extend vgem, i.e. add support to create gem objects from existing shmem. Comments? Yes, extending vgem instead of creating something new sounds like a good idea to me as well. Regards, Christian. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/virtio: remove drm_dev_set_unique workaround
On 04/04/18 19:29, Laszlo Ersek wrote: > Hi Emil, > > On 04/03/18 19:13, Emil Velikov wrote: >> On 29 March 2018 at 12:17, Laszlo Ersekwrote: >>> On 03/28/18 16:35, Emil Velikov wrote: On 28 March 2018 at 11:27, Laszlo Ersek wrote: > On 03/28/18 03:24, Emil Velikov wrote: >>> >> Gents, can someone double-check this please? Just in case. > > I guess I should test whether this series regresses the use case > described in c2cbc38b97; is that correct? > Precisely. >>> [3] https://github.com/evelikov/linux/commits/ioctl-cleanups >>> >>> Unfortunately, this series seems to reintroduce the regression fixed >>> / described earlier in commit c2cbc38b97. >>> >> Thank you very much for testing. >> >> Believe I've tracked it down to a broken commit from 2014 ;-) >> Please try the following branch [1] - it's untested, but I'm 99% sure >> it will work like a charm. > > Alas, it does not work. I've done some more digging. Let me quote the commit message on the proposed patch again: > Ealier commit a325725633c26aa66ab940f762a6b0778edf76c0 did not attribute > that virtio can be either PCI or a platform device and removed the > .set_busid hook. Whereas only the "platform" instance should have been > removed. > > Since then, two things have happened: > - the driver manually calls drm_dev_set_unique, addressing the Xserver > regression - see commit 9785b4321b0bd701f8d21d3d3c676a7739a5cf22 > - core itself calls drm_pci_set_busid, on drm_set_busid IOCTL setting > the busid, so we don't need to fallback to dev->unique - see commit > 5c484cee7ef9c4fd29fa0ba09640d55960977145 > > With that in place we can remove the local workaround. This write-up of events is not precise enough. Instead, I think the timeline is as follows: (1) Commit a325725633c2 ("drm: Lobotomize set_busid nonsense for !pci drivers", 2016-06-21) introduced the regression. By removing drm_virtio_set_busid(), commit a325725633c2 changed the behavior of the following function: > static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv) > { > struct drm_master *master = file_priv->master; > int ret; > > if (master->unique != NULL) > drm_unset_busid(dev, master); > > if (dev->driver->set_busid) { > ret = dev->driver->set_busid(dev, master); > if (ret) { > drm_unset_busid(dev, master); > return ret; > } > } else { > WARN_ON(!dev->unique); > master->unique = kstrdup(dev->unique, GFP_KERNEL); > if (master->unique) > master->unique_len = strlen(dev->unique); > } > > return 0; > } When a325725633c2 removed drm_virtio_set_busid(), drm_set_busid() started taking the second branch, which wasn't doing the right thing for virtio-vga at the time. (2) There were two ways to fix the regression: either (a) return drm_set_busid() to taking the first branch, or (b) tweak the virtio-vga driver so that the second branch in drm_set_busid() start to behave right. My commit c2cbc38b9715 ("drm: virtio: reinstate drm_virtio_set_busid()", 2016-10-04) implemented (a), by reverting a few chunks of a325725633c2. (3) Gerd thought that approach (b) was superior (and I totally defer to him on this, now that I'm learning of his patches in the first place :) ). Namely, in commit 9785b4321b0b ("drm/virtio: fix busid regression", 2016-11-15), he implemented approach (b), and right after, in commit 1775db074a32 ("Revert "drm: virtio: reinstate drm_virtio_set_busid()"", 2016-11-15), he undid my commit c2cbc38b9715. In other words, Gerd replaced approach (a) with approach (b). (4) Subsequently, commit 5c484cee7ef9 ("drm: Remove drm_driver->set_busid hook", 2017-06-20), changed drm_set_busid() to the following: > static int drm_set_busid(struct drm_device *dev, struct drm_file *file_priv) > { > struct drm_master *master = file_priv->master; > int ret; > > if (master->unique != NULL) > drm_unset_busid(dev, master); > > if (dev_is_pci(dev->dev)) { > ret = drm_pci_set_busid(dev, master); > if (ret) { > drm_unset_busid(dev, master); > return ret; > } > } else { > WARN_ON(!dev->unique); > master->unique = kstrdup(dev->unique, GFP_KERNEL); > if (master->unique) > master->unique_len = strlen(dev->unique); > } > > return 0; > } Perhaps surprisingly, this change did not affect (or "help") virtio-vga at all, despite the fact that drm_virtio_set_busid() also used to call drm_pci_set_busid(). The reason for commit 5c484cee7ef9 not affecting virtio-vga is that the first branch would not be taken
Re: [PATCH 10/10] drm/msm/gpu: Add the buffer objects from the submit to the crash dump
Hi Jordan, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on robclark/msm-next] [also build test WARNING on next-20180406] [cannot apply to v4.16] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Jordan-Crouse/drm-msm-GPU-crash-state/20180406-170513 base: git://people.freedesktop.org/~robclark/linux msm-next config: arm-multi_v7_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=arm All warnings (new ones prefixed by >>): drivers/gpu/drm/msm/adreno/adreno_gpu.c: In function 'adreno_show': >> drivers/gpu/drm/msm/adreno/adreno_gpu.c:533:31: warning: format '%ld' >> expects argument of type 'long int', but argument 3 has type 'size_t {aka >> unsigned int}' [-Wformat=] drm_printf(p, "size: %ld\n", state->bos[i].size); ~~^ ~~ %d vim +533 drivers/gpu/drm/msm/adreno/adreno_gpu.c 496 497 void adreno_show(struct msm_gpu *gpu, struct msm_gpu_state *state, 498 struct drm_printer *p) 499 { 500 struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); 501 int i; 502 503 if (IS_ERR_OR_NULL(state)) 504 return; 505 506 drm_printf(p, "revision: %d (%d.%d.%d.%d)\n", 507 adreno_gpu->info->revn, adreno_gpu->rev.core, 508 adreno_gpu->rev.major, adreno_gpu->rev.minor, 509 adreno_gpu->rev.patchid); 510 511 drm_printf(p, "rbbm-status: 0x%08x\n", state->rbbm_status); 512 513 drm_printf(p, "ringbuffer:\n"); 514 515 for (i = 0; i < gpu->nr_rings; i++) { 516 drm_printf(p, " - id: %d\n", i); 517 drm_printf(p, "last-fence: %d\n", state->ring[i].seqno); 518 drm_printf(p, "retired-fence: %d\n", state->ring[i].fence); 519 drm_printf(p, "rptr: %d\n", state->ring[i].rptr); 520 drm_printf(p, "wptr: %d\n", state->ring[i].wptr); 521 drm_printf(p, "size: %d\n", MSM_GPU_RINGBUFFER_SZ); 522 523 adreno_show_object(p, state->ring[i].data, 524 state->ring[i].data_size); 525 } 526 527 if (state->bos) { 528 drm_printf(p, "bos:\n"); 529 530 for (i = 0; i < state->nr_bos; i++) { 531 drm_printf(p, " - iova: 0x%016llx\n", 532 state->bos[i].iova); > 533 drm_printf(p, "size: %ld\n", > state->bos[i].size); 534 535 adreno_show_object(p, state->bos[i].data, 536 state->bos[i].size); 537 } 538 } 539 540 drm_printf(p, "registers:\n"); 541 drm_printf(p, " - [offset, value]\n"); 542 543 for (i = 0; i < state->nr_registers; i++) { 544 drm_printf(p, " - [0x%04x, 0x%08x]\n", 545 state->registers[i * 2] << 2, 546 state->registers[(i * 2) + 1]); 547 } 548 } 549 #endif 550 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RfC PATCH] Add udmabuf misc device
Hi, > > I fail to see any common ground for xen-zcopy and udmabuf ... > Does the above mean you can assume that xen-zcopy and udmabuf > can co-exist as two different solutions? Well, udmabuf route isn't fully clear yet, but yes. See also gvt (intel vgpu), where the hypervisor interface is abstracted away into a separate kernel modules even though most of the actual vgpu emulation code is common. > And what about hyper-dmabuf? No idea, didn't look at it in detail. Looks pretty complex from a distant view. Maybe because it tries to build a communication framework using dma-bufs instead of a simple dma-buf passing mechanism. Like xen-zcopy it seems to depend on the idea that the hypervisor manages all memory it is easy for guests to share pages with the help of the hypervisor. Which simply isn't the case on kvm. hyper-dmabuf and xen-zcopy could maybe share code, or hyper-dmabuf build on top of xen-zcopy. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 15/19] omap2: omapfb: allow building it with COMPILE_TEST
Hi Mauro, I love your patch! Perhaps something to improve: [auto build test WARNING on linuxtv-media/master] [also build test WARNING on v4.16 next-20180406] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Mauro-Carvalho-Chehab/Make-all-media-drivers-build-with-COMPILE_TEST/20180406-163048 base: git://linuxtv.org/media_tree.git master config: mips-allmodconfig (attached as .config) compiler: mips-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=mips All warnings (new ones prefixed by >>): drivers/video/fbdev/omap2/omapfb/dss/dispc.c: In function 'pixinc': >> drivers/video/fbdev/omap2/omapfb/dss/dispc.c:1859:2: warning: this 'else' >> clause does not guard... [-Wmisleading-indentation] else ^~~~ drivers/video/fbdev/omap2/omapfb/dss/dispc.c:1861:3: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'else' return 0; ^~ vim +/else +1859 drivers/video/fbdev/omap2/omapfb/dss/dispc.c f76ee892 Tomi Valkeinen 2015-12-09 1850 f76ee892 Tomi Valkeinen 2015-12-09 1851 static s32 pixinc(int pixels, u8 ps) f76ee892 Tomi Valkeinen 2015-12-09 1852 { f76ee892 Tomi Valkeinen 2015-12-09 1853if (pixels == 1) f76ee892 Tomi Valkeinen 2015-12-09 1854return 1; f76ee892 Tomi Valkeinen 2015-12-09 1855else if (pixels > 1) f76ee892 Tomi Valkeinen 2015-12-09 1856return 1 + (pixels - 1) * ps; f76ee892 Tomi Valkeinen 2015-12-09 1857else if (pixels < 0) f76ee892 Tomi Valkeinen 2015-12-09 1858return 1 - (-pixels + 1) * ps; f76ee892 Tomi Valkeinen 2015-12-09 @1859else f76ee892 Tomi Valkeinen 2015-12-09 1860BUG(); f76ee892 Tomi Valkeinen 2015-12-09 1861return 0; f76ee892 Tomi Valkeinen 2015-12-09 1862 } f76ee892 Tomi Valkeinen 2015-12-09 1863 :: The code at line 1859 was first introduced by commit :: f76ee892a99e68b55402b8d4b8aeffcae2aff34d omapfb: copy omapdss & displays for omapfb :: TO: Tomi Valkeinen <tomi.valkei...@ti.com> :: CC: Tomi Valkeinen <tomi.valkei...@ti.com> --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: GPU/DRM issue with DSI commands on msm 8916
Hi, On Thursday 05 April 2018 08:28 PM, Daniel Mack wrote: Hi, I'm having issues with the GPU/DRM drivers on a msm8916 based platform which is very similar to the DragonBoard 410c. In my setup, a DSI display is directly connected to the SoC, and the video link is stable. However, when the host sends DCS commands down to the DSI panel (for instance to set the backlight brightness), the image drops out, most of the time only in terms of a short flicker, but sometimes it will completely kill the image. In the latter case, only restarting the Wayland compositor in userspace helps. This is also quite reproducible; sending a NOP command once a second would give a visual flicker in 90% of the cases, and it needs at most a minute to make the screen turn black. The interesting thing is that this used to work in a v4.9 based version, but it broke somewhere on the way to v4.14. Unfortunately, the platform does not boot a vanilla kernel, so I can't really bisect this. We currently depend on the Linaro downstream patches which can be found here: The major change that happened between qcomlt-4.9 and qcomlt-4.14 from a DSI point of view was probably the addition of runtime PM support. The register configurations that are responsible for interleaving DCS commands while video mode is still on should be the same. You could comment out the pm_runtime_put_sync() calls in drivers/gpu/drm/msm/dsi/dsi_host.c, in case some registers got reset during put_sync and weren't restored correctly after get_sync(). Does your device initialize a splash screen in the bootloader? You could also compare the reg dumps between 4.9 and 4.14 by enabling the config CONFIG_DRM_MSM_REGISTER_LOGGING and check if there are any register configuration differences between the two. One (rather unlikely) possibility I can think of is if somehow the buffers used to send/receive DCS commands aren't mapped/unmapped correctly. There have been some msm_gem changes, and the IOMMU driver is new. That's the main reason why I'm wondering if the contents of the DCS buffers somehow got corrupt. Is the panel initialized using DCS commands too? Thanks, Archit http://git.linaro.org/landing-teams/working/qualcomm/kernel.git/log/?h=release/qcomlt-4.9 http://git.linaro.org/landing-teams/working/qualcomm/kernel.git/log/?h=release/qcomlt-4.14 I've looked at the development that has happened in the area for some time now, but I can't really pin-point any specific commit. Also, I cherry-picked most of the patches to these drivers that came in after v4.14, but that didn't help either. Has this has been observed before? A pointer what to investigate on would be very much appreciated. If there is any more information I can provide, please let me know. Thanks, Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" 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 1/1] drm/xen-zcopy: Add Xen zero-copy helper DRM driver
On 04/03/2018 12:47 PM, Daniel Vetter wrote: On Thu, Mar 29, 2018 at 04:19:31PM +0300, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko+static int to_refs_grant_foreign_access(struct xen_gem_object *xen_obj) +{ + grant_ref_t priv_gref_head; + int ret, j, cur_ref, num_pages; + struct sg_page_iter sg_iter; + + ret = gnttab_alloc_grant_references(xen_obj->num_pages, + _gref_head); + if (ret < 0) { + DRM_ERROR("Cannot allocate grant references\n"); + return ret; + } + + j = 0; + num_pages = xen_obj->num_pages; + for_each_sg_page(xen_obj->sgt->sgl, _iter, xen_obj->sgt->nents, 0) { + struct page *page; + + page = sg_page_iter_page(_iter); Quick drive-by: You can't assume that an sgt is struct page backed. Do you mean that someone could give me sgt which never seen sg_assign_page for its entries? What are the other use-cases for that to happen? And you probably want to check this at import/attach time. The check you mean is to make sure that when I call page = sg_page_iter_page(_iter); I have to make sure that I get a valid page? -Daniel Thank you, Oleksandr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RfC PATCH] Add udmabuf misc device
On 04/06/2018 12:07 PM, Gerd Hoffmann wrote: I'm not sure we can create something which works on both kvm and xen. The memory management model is quite different ... On xen the hypervisor manages all memory. Guests can allow other guests to access specific pages (using grant tables). In theory any guest <=> guest communication is possible. In practice is mostly guest <=> dom0 because guests access their virtual hardware that way. dom0 is the priviledged guest which owns any hardware not managed by xen itself. Xen guests can ask the hypervisor to update the mapping of guest physical pages. They can ballon down (unmap and free pages). They can ballon up (ask the hypervisor to map fresh pages). They can map pages exported by other guests using grant tables. xen-zcopy makes heavy use of this. It balloons down, to make room in the guest physical address space, then goes map the exported pages there, finally composes a dma-buf. On kvm qemu manages all guest memory. qemu also has all guest memory mapped, so a grant-table like mechanism isn't needed to implement virtual devices. qemu can decide how it backs memory for the guest. qemu propagates the guest memory map to the kvm driver in the linux kernel. kvm guests have some control over the guest memory map, for example they can map pci bars wherever they want in their guest physical address space by programming the base registers accordingly, but unlike xen guests they can't ask the host to remap individual pages. Due to qemu having all guest memory mapped virtual devices are typically designed to have the guest allocate resources, then notify the host where they are located. This is where the udmabuf idea comes from: Guest tells the host (qemu) where the gem object is, and qemu then can create a dmabuf backed by those pages to pass it on to other processes such as the wayland display server. Possibly even without the guest explicitly asking for it, i.e. export the framebuffer placed by the guest in the (virtual) vga pci memory bar as dma-buf. And I can imagine that this is useful outsize virtualization too. I fail to see any common ground for xen-zcopy and udmabuf ... Does the above mean you can assume that xen-zcopy and udmabuf can co-exist as two different solutions? And what about hyper-dmabuf? Thank you, Oleksandr ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 07/10] drm/msm/adreno: Convert the show/crash file format
Quoting Jordan Crouse (2018-04-05 23:00:53) > Convert the format of the 'show' debugfs file and the crash > dump to a format resembling YAML. This should be easier to > parse and be more flexible for future changes and expansions. > > Signed-off-by: Jordan Crouse+1, I've been trying to work up the courage to make the conversion to yaml for i915 as well. My gut feeling says we at least want to move the indentation handling to drm_print. > for (i = 0; i < state->nr_registers; i++) { > - drm_printf(p, "IO:R %08x %08x\n", > + drm_printf(p, " - [0x%04x, 0x%08x]\n", Hmm, sequence of sequences. The alternative would be - {offset:%x, value:%x} May I ask why you would pick one over the other? I like the verbosity of having the nodes, it should also mean that the order is immaterial and we could extend it with say name:, comments: or whatever, without affecting the parser. Otoh, the [] are more compact. -Chris ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 199101] AMDGPU Fury X random screen flicker on Linux kernel 4.16rc5
https://bugzilla.kernel.org/show_bug.cgi?id=199101 Paweł (pawel.p...@gmail.com) changed: What|Removed |Added CC||pawel.p...@gmail.com --- Comment #14 from Paweł (pawel.p...@gmail.com) --- Since nobody cared I bisected the issue: >commit 36cc549d59864b7161f0e23d710c1c4d1b9cf022 >Author: Shirish S>Date: Wed Feb 28 12:14:58 2018 +0530 > >drm/amd/display: disable CRTCs with NULL FB on their primary plane (V2) > >The below commit > >"drm/atomic: Try to preserve the crtc enabled state in >>drm_atomic_remove_fb, v2" > >introduces a slight behavioral change to rmfb. Instead of disabling a crtc >when the primary plane is disabled, it now preserves it. > >This change leads to BUG hit while performing atomic commit on amd driver. > >As a fix this patch ensures that we disable the CRTC's with NULL FB by >>returning >-EINVAL and hence triggering fall back to the old behavior and turning off >>the >crtc in atomic_remove_fb(). > >V2: Added error check for plane_state and removed sanity check for crtc. > >Signed-off-by: Shirish S >Signed-off-by: Pratik Vishwakarma >Reviewed-by: Harry Wentland >Signed-off-by: Alex Deucher > >:04 04 9b8fd67908699d2651daa93fab59b21e7a76b1c6 >>21bbcb69561e67e5acf63d56344c7ba7ac4146a6 M drivers It makes my AMD Radeon RX 480 flicker a lot. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [RfC PATCH] Add udmabuf misc device
On Fri, Apr 06, 2018 at 10:52:21AM +0100, Daniel Stone wrote: > Hi Gerd, > > On 14 March 2018 at 08:03, Gerd Hoffmannwrote: > >> Either mlock account (because it's mlocked defacto), and get_user_pages > >> won't do that for you. > >> > >> Or you write the full-blown userptr implementation, including mmu_notifier > >> support (see i915 or amdgpu), but that also requires Christian Königs > >> latest ->invalidate_mapping RFC for dma-buf (since atm exporting userptr > >> buffers is a no-go). > > > > I guess I'll look at mlock accounting for starters then. Easier for > > now, and leaves the door open to switch to userptr later as this should > > be transparent to userspace. > > Out of interest, do you have usecases for full userptr support? Maybe > another way would be to allow creation of dmabufs from memfds. I have two things in mind. One is vga emulation. I have virtual pci memory bar for the virtual vga. qemu backs vga memory with anonymous pages right now, switching that to shmem should be easy though if that makes things easier. Guest places the framebuffer somewhere in the pci bar, and I want export the chunk which represents the framebuffer as dma-buf to display it on the host without copying around data. Framebuffer is linear in guest physical memory, so a single block only. That is the simpler case. The more difficuilt one is virtio-gpu ressources. virtio-gpu resources live in host memory (guest has no direct access). The guest can optionally specify guest memory pages as backing storage for the resource. Guest backing storage is allowed to be scattered. Commands exist to copy both ways between host storage and guest backing. With virgl (opengl acceleration) enabled the guest will send rendering commands to fill the framebuffer ressource, so there is no need to copy content to the framebuffer ressource. The guest may fill other resources such as textures used for rendering with copy commands. Without acceleration the guest does software-rendering to the backing storage, then sends a command to copy the framebuffer content from guest backing storage to host ressource. Now it would be useful to allow a shared mapping, so no copying between guest backing storage and host resource is needed, especially for the software rendering case (i.e. dumb gem buffers). Being able to export guest dumb buffers to other host processes would be useful too, for example to display guest windows seamlessly on the host wayland server. So getting a dma-buf for the guest backing storage via udmabuf looked like a useful approach. We can export the guest gem buffers to other host processes that way. qemu itself could map it too, to get a linear representation of the scattered guest backing storage. The other obvious approach would be to do it the other way around and allow the guest map the host resource somehow. On the host side qemu could use vgem to allocate resource memory, so it'll be a gem object already. Mapping that into the guest isn't that straight-forward though. The guest manages its physical address space, so the guest would need to find a free spot and ask the host to place the resource there. Then the guest needs page structs covering the mapped resource, so it can work with it. Didn't investigate how difficuilt that is. Use memory hotplug maybe? Can we easily unmap the resource then? Also I think updating the guests physical memory layout (which we would need to do on every resource map/unmap) isn't an exactly cheap operation ... cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel