Re: [PATCHv4 3/3] drm/vgem: Enable dmabuf import interfaces
On Thu, May 04, 2017 at 09:25:03PM +0100, Chris Wilson wrote: > On Thu, May 04, 2017 at 11:45:48AM -0700, Laura Abbott wrote: > > > > Enable the GEM dma-buf import interfaces in addition to the export > > interfaces. This lets vgem be used as a test source for other allocators > > (e.g. Ion). > > > > Reviewed-by: Chris Wilson > > Signed-off-by: Laura Abbott > > --- > > v4: Use new drm_gem_prime_import_dev function > > --- > > static const struct vm_operations_struct vgem_gem_vm_ops = { > > @@ -114,12 +142,8 @@ static void vgem_postclose(struct drm_device *dev, > > struct drm_file *file) > > kfree(vfile); > > } > > > > -/* ioctls */ > > - > > -static struct drm_gem_object *vgem_gem_create(struct drm_device *dev, > > - struct drm_file *file, > > - unsigned int *handle, > > - unsigned long size) > > +static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device > > *dev, > > + unsigned long size) > > I'm going to guess that doesn't line up anymore. If checkpatch isn't > complaining, then sorry for the noise. > > > +static struct drm_gem_object *vgem_gem_create(struct drm_device *dev, > > + struct drm_file *file, > > + unsigned int *handle, > > + unsigned long size) > > Ditto. > > Lgtm, so r-b still good. I applied all three as-is, I think we can polish more as follow-ups. Thanks, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100941] Improve time to suspend on
https://bugs.freedesktop.org/show_bug.cgi?id=100941 Bug ID: 100941 Summary: Improve time to suspend on Product: DRI Version: DRI git Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: DRM/Radeon Assignee: dri-devel@lists.freedesktop.org Reporter: paulepan...@users.sourceforge.net Created attachment 131223 --> https://bugs.freedesktop.org/attachment.cgi?id=131223&action=edit TML page generated by pm-graph (`sudo ./analyze_suspend.py -config config/suspend-callgraph.cfg`) The ASRock E350M1 has a Radeon HD 6310. ``` $ sudo lspci -s 0:01.0 -nn -v 00:01.0 VGA compatible controller [0300]: Advanced Micro Devices, Inc. [AMD/ATI] Wrestler [Radeon HD 6310] [1002:9802] (prog-if 00 [VGA controller]) Subsystem: Advanced Micro Devices, Inc. [AMD/ATI] Wrestler [Radeon HD 6310] [1002:9802] Flags: bus master, fast devsel, latency 0, IRQ 28 Memory at e000 (32-bit, prefetchable) [size=256M] I/O ports at 2000 [size=256] Memory at f010 (32-bit, non-prefetchable) [size=256K] [virtual] Expansion ROM at 000c [disabled] [size=128K] Capabilities: [50] Power Management version 3 Capabilities: [58] Express Root Complex Integrated Endpoint, MSI 00 Capabilities: [a0] MSI: Enable+ Count=1/1 Maskable- 64bit+ Capabilities: [100] Vendor Specific Information: ID=0001 Rev=1 Len=010 Kernel driver in use: radeon Kernel modules: radeon ``` With Debian Sid/unstable with Linux 4.9.25 suspend and resume times are benchmarked with pm-graph [1], and the command below. ``` sudo ./analyze_suspend.py -config config/suspend-callgraph.cfg ``` It turns out that with 258 ms the radeon module takes the majority of the time during suspend. In that cycle one `radeon_bo_evict_vram` call takes the longest with 198 ms. ``` […] 459.186778 | 0) kworker-1495 | 0.974 us| radeon_fence_wait_empty [radeon](); 459.186780 | 0) kworker-1495 | 0.440 us| radeon_fence_wait_empty [radeon](); 459.186783 | 0) kworker-1495 | 0.501 us| radeon_fence_wait_empty [radeon](); 459.186785 | 0) kworker-1495 | 5.424 us| radeon_save_bios_scratch_regs [radeon](); 459.186793 | 0) kworker-1495 | 7625.511 us | evergreen_suspend [radeon](); 459.194422 | 0) kworker-1495 | 10.158 us | evergreen_hpd_fini [radeon](); 459.194434 | 0) kworker-1495 | 198203.3 us | radeon_bo_evict_vram [radeon](); […] ``` Please see the attached files for more details. -- 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 100941] Improve time to suspend on
https://bugs.freedesktop.org/show_bug.cgi?id=100941 Paul Menzel changed: What|Removed |Added CC||paulepanter@users.sourcefor ||ge.net Attachment #131223|TML page generated by |HTML page generated by description|pm-graph (`sudo |pm-graph (`sudo |./analyze_suspend.py|./analyze_suspend.py |-config |-config |config/suspend-callgraph.cf |config/suspend-callgraph.cf |g`) |g`) -- 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 100941] Improve time to suspend on
https://bugs.freedesktop.org/show_bug.cgi?id=100941 --- Comment #1 from Paul Menzel --- Created attachment 131224 --> https://bugs.freedesktop.org/attachment.cgi?id=131224&action=edit ftrace messages generated by pm-graph (`sudo ./analyze_suspend.py -config config/suspend-callgraph.cfg`) -- 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 RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane
Regards Shashank On 5/5/2017 12:28 PM, Daniel Vetter wrote: On Thu, May 04, 2017 at 05:51:45PM +0300, Ville Syrjälä wrote: On Thu, May 04, 2017 at 03:22:45PM +0200, Daniel Vetter wrote: On Thu, May 04, 2017 at 10:14:26AM +0300, Jyri Sarha wrote: Add standard optinal property blobs for YCbCr to RGB conversion CSC matrix and YCbCr preoffset vector in DRM planes. New enums are defined to YCBCR_ENCODING property to activate the CSC and preoffset properties. For simplicity the new properties are stored in the drm_plane object, because the YCBCR_ENCODING is already there. The blob contents are defined in the uapi/drm/drm_mode.h header. Signed-off-by: Jyri Sarha Not sure we want to make this yuv2rgb specific, plenty of planes have generic degamma/offset, csc, gamme/offset hw. I think what we want instead is the generic csc support, plus a ycbcr2rgb mode of "bypass". My idea is to not expose this. And instead just expose a normal ctm, and then just refuse any combination of YCbCr->RGB/degamma/ctm that can't be done by the hw. But that means we'd need to have a bit-perfect match with a canonical conversion matrix (even if maybe the hw has a rounding bug and implements something slightly different than the standard). That seems a bit an awkward interface. Or is your idea that hw with a ctm would simply not expose the colorspace enum, if it doesn't also have fixed-function bits on top of the ctm? -Daniel I think the idea is to have separate properties for CTM and Gamut Mapping, so that we can have more control over linear and non-linear data blocks. All transformation should happen in one property whereas all gamut mapping should go into other, which IMHO seems to be the correct way to operate. - Shashank And as usual, this needs some userspace compositor which actually uses it (not just a demo, since there's a huge difference between a demo and something that has to Just Work like a real compositor). -Daniel --- drivers/gpu/drm/drm_atomic.c| 19 + drivers/gpu/drm/drm_atomic_helper.c | 9 ++ drivers/gpu/drm/drm_color_mgmt.c| 55 +++-- drivers/gpu/drm/drm_plane.c | 6 include/drm/drm_color_mgmt.h| 3 ++ include/drm/drm_plane.h | 4 +++ include/uapi/drm/drm_mode.h | 12 7 files changed, 106 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index bcef93d..87a2d55 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -733,6 +733,7 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, struct drm_device *dev = plane->dev; struct drm_mode_config *config = &dev->mode_config; int ret; + bool dummy; if (property == config->prop_fb_id) { struct drm_framebuffer *fb = drm_framebuffer_lookup(dev, val); @@ -777,6 +778,18 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, state->zpos = val; } else if (property == plane->ycbcr_encoding_property) { state->ycbcr_encoding = val; + } else if (property == plane->ycbcr_decode_csc_property) { + ret = drm_atomic_replace_property_blob_from_id(dev, + &state->ycbcr_decode_csc, val, + sizeof(struct drm_ycbcr_decode_csc), + &dummy); + return ret; + } else if (property == plane->ycbcr_csc_preoffset_property) { + ret = drm_atomic_replace_property_blob_from_id(dev, + &state->ycbcr_csc_preoffset, val, + sizeof(struct drm_ycbcr_csc_preoffset), + &dummy); + return ret; } else if (plane->funcs->atomic_set_property) { return plane->funcs->atomic_set_property(plane, state, property, val); @@ -839,6 +852,12 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, *val = state->zpos; } else if (property == plane->ycbcr_encoding_property) { *val = state->ycbcr_encoding; + } else if (property == plane->ycbcr_decode_csc_property) { + *val = state->ycbcr_decode_csc ? + state->ycbcr_decode_csc->base.id : 0; + } else if (property == plane->ycbcr_csc_preoffset_property) { + *val = state->ycbcr_csc_preoffset ? + state->ycbcr_csc_preoffset->base.id : 0; } else if (plane->funcs->atomic_get_property) { return plane->funcs->atomic_get_property(plane, state, property, val); } else { diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index 8be9719..6ecc32f 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3238,6 +3238,12 @@ void __drm_atomic_helper_plane
Re: [PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane
On Fri, May 5, 2017 at 9:06 AM, Sharma, Shashank wrote: > On 5/5/2017 12:28 PM, Daniel Vetter wrote: >> >> On Thu, May 04, 2017 at 05:51:45PM +0300, Ville Syrjälä wrote: >>> >>> On Thu, May 04, 2017 at 03:22:45PM +0200, Daniel Vetter wrote: On Thu, May 04, 2017 at 10:14:26AM +0300, Jyri Sarha wrote: > > Add standard optinal property blobs for YCbCr to RGB conversion CSC > matrix and YCbCr preoffset vector in DRM planes. New enums are defined > to YCBCR_ENCODING property to activate the CSC and preoffset > properties. For simplicity the new properties are stored in the > drm_plane object, because the YCBCR_ENCODING is already there. The > blob contents are defined in the uapi/drm/drm_mode.h header. > > Signed-off-by: Jyri Sarha Not sure we want to make this yuv2rgb specific, plenty of planes have generic degamma/offset, csc, gamme/offset hw. I think what we want instead is the generic csc support, plus a ycbcr2rgb mode of "bypass". >>> >>> My idea is to not expose this. And instead just expose a normal >>> ctm, and then just refuse any combination of YCbCr->RGB/degamma/ctm >>> that can't be done by the hw. >> >> But that means we'd need to have a bit-perfect match with a canonical >> conversion matrix (even if maybe the hw has a rounding bug and implements >> something slightly different than the standard). That seems a bit an >> awkward interface. Or is your idea that hw with a ctm would simply not >> expose the colorspace enum, if it doesn't also have fixed-function bits on >> top of the ctm? >> -Daniel > > I think the idea is to have separate properties for CTM and Gamut Mapping, > so that we can have more control > over linear and non-linear data blocks. All transformation should happen in > one property whereas all gamut > mapping should go into other, which IMHO seems to be the correct way to > operate. Yeah, for the programmable hw we should probably go with the degamma/ctm/gamma combo, like we have on the CRTC already. My question was how this interactions with some fixed-function color space conversion the hw might also have, and how these two sets of properties are mean to interact. On that topic, I think it'd be good if we put the helper functions and property documentation into drm_color_mgmt.c, so that it is all in one place, and to make sure we don't accidentally have different meanings for gamma table and ctm blobs. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane
Regards Shashank On 5/5/2017 12:43 PM, Daniel Vetter wrote: On Fri, May 5, 2017 at 9:06 AM, Sharma, Shashank wrote: On 5/5/2017 12:28 PM, Daniel Vetter wrote: On Thu, May 04, 2017 at 05:51:45PM +0300, Ville Syrjälä wrote: On Thu, May 04, 2017 at 03:22:45PM +0200, Daniel Vetter wrote: On Thu, May 04, 2017 at 10:14:26AM +0300, Jyri Sarha wrote: Add standard optinal property blobs for YCbCr to RGB conversion CSC matrix and YCbCr preoffset vector in DRM planes. New enums are defined to YCBCR_ENCODING property to activate the CSC and preoffset properties. For simplicity the new properties are stored in the drm_plane object, because the YCBCR_ENCODING is already there. The blob contents are defined in the uapi/drm/drm_mode.h header. Signed-off-by: Jyri Sarha Not sure we want to make this yuv2rgb specific, plenty of planes have generic degamma/offset, csc, gamme/offset hw. I think what we want instead is the generic csc support, plus a ycbcr2rgb mode of "bypass". My idea is to not expose this. And instead just expose a normal ctm, and then just refuse any combination of YCbCr->RGB/degamma/ctm that can't be done by the hw. But that means we'd need to have a bit-perfect match with a canonical conversion matrix (even if maybe the hw has a rounding bug and implements something slightly different than the standard). That seems a bit an awkward interface. Or is your idea that hw with a ctm would simply not expose the colorspace enum, if it doesn't also have fixed-function bits on top of the ctm? -Daniel I think the idea is to have separate properties for CTM and Gamut Mapping, so that we can have more control over linear and non-linear data blocks. All transformation should happen in one property whereas all gamut mapping should go into other, which IMHO seems to be the correct way to operate. Yeah, for the programmable hw we should probably go with the degamma/ctm/gamma combo, like we have on the CRTC already. My question was how this interactions with some fixed-function color space conversion the hw might also have, and how these two sets of properties are mean to interact. Even for the fixed function HW we can have this segregation, and the core driver can take care of understanding the enum, and conversion into driving a fix-function block. I had proposed one block diagram, which was once prepared by Ville / Me to handle this situation, but unfortunately we could not discus much there. https://patchwork.kernel.org/patch/9695875/ On that topic, I think it'd be good if we put the helper functions and property documentation into drm_color_mgmt.c, so that it is all in one place, and to make sure we don't accidentally have different meanings for gamma table and ctm blobs. I agree, that would be good place. - Shashank -Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100936] Radeonsi rendering corruption in unigine heaven (regression bissected)
https://bugs.freedesktop.org/show_bug.cgi?id=100936 Samuel Pitoiset changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #2 from Samuel Pitoiset --- Yeah, it was funky. :) Should be fixed with: https://cgit.freedesktop.org/mesa/mesa/commit/?id=92ab06e782c31fe0209e5d0181967a2ff6739c9b -- 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 195659] nouveau fence error
https://bugzilla.kernel.org/show_bug.cgi?id=195659 Pierre Moreau (pierre.mor...@free.fr) changed: What|Removed |Added CC||pierre.mor...@free.fr --- Comment #2 from Pierre Moreau (pierre.mor...@free.fr) --- Do you happen to use xf86-video-nouveau <= 1.0.13? If so, could you try using instead the modesetting DDX or xf86-video-nouveau >= 1.0.14? Nouveau switched to atomic modesetting in 4.10, revealing a bug in the Nouveau DDX which got patched in 1.0.14. For your information, reporting bugs against Nouveau should be done on the Freedesktop bugzilla, see https://nouveau.freedesktop.org/wiki/Bugs/. -- 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: Proposal for RandR version 1.6, Leases and EDID-based output grabs
On Thu, 04 May 2017 11:02:44 -0700 Keith Packard wrote: > Pekka Paalanen writes: > > > That means you need an explicit key to denote HMDs. More below. > > I don't think so. Presumably the VR system will have a known list of > HMDs it works with, and so it will probably have a list of EDID > information; I expect that's where the data for monitors-to-ignore will > come from in many cases. After all, the goal is to prevent the desktop > From shifting onto a display only to get kicked off as the VR app takes > over the HMD. Hi, I disagree on the details, more below. > > That way the desktop would extend there automatically, but it would > > also be advertised as a HMD to clients. If it's not listed at all, > > would it be possible to advertise it as a HMD and DRM-leased out etc.? > > I think all we'd do is offer a new RandR request which listed "all" > monitors, including those hidden from the desktop, and expect that the > monitor-specific applications would use that to detect the presence of > their magic outputs and use them. For an HMD, you'll also have other > devices connected, so we just need some way of positively associating > the various input devices with the specific monitor (in case there is > more than one). Such a RandR request is something I would not like to have to replicate on Wayland. The display server contains the policy, it should not just expose everything up for grabs. This is a fundamental difference between X11 and Wayland architectures, and I think the output information database should support both ways. > > At least for Wayland, I'd like to not advertise "everything" as > > possible HMDs but only those that really are. > > I don't think the window system needs to know that the displays are HMD, > only that it shouldn't present them as part of the desktop. Everything > else about them can be left outside of this spec. I disagree. Wayland will likely have a special protocol extension exclusively for advertising HMDs, so the display server will need to know which ones are HMDs and which one are not, regardless whether the desktop is extended there or not. This extension could also offer the DRM leasing capabilities. I gave the PSVR as an example of hardware where one both can meaningfully extend the desktop there and offer it as a HMD. When that output is taken into HMD use, the display will automatically stop putting the desktop there - but only temporarily. All windows that were on that extended output should still be there when the VR app/compositor finishes. This is window management policy, so the window manager must know what is going on. (That is similar to the fullscreening with video mode changing mechanism, where the video mode requested by an application is temporary, and normality will be restored automatically when e.g. the window disappears by client crash or the user switches to another window.) Having the window manager know what is a HMD and which client is active on it will let it make better management decisions and even allow switching between VR apps, or temporarily switching from VR app to the desktop when supported, and back. > > I also read some discussions about 3D TV modes, but couldn't really get > > a conclusion if they are supposed to render like if you were in a 3D > > cinema theatre or not. Apparently that was enabled in a firmware > > upgrade and maybe had some extra requirements. > > Hard to imagine this being relevant for the Linux desktop at least... Depends on how a video player will deliver the 3D movie content. Can it be done as a normal GUI app providing stereo buffers, or does one need to turn the video player into a VR app and reimplement the cinematic mode in software. The latter option works for any HMD, but the former option needs less power from the computer as head tracking etc. is outsourced to the "HMD" itself. > Thanks again; I'm sensing that a simple 'ignore this monitor for the > desktop' might suffice for now, but that we'll end up wanting something > more complicated in the future. I think the key is to try and avoid > making that harder by what we do now, but I don't think we should be > trying to implement a larger solution too soon. Yes, that I agree with. Ultimately I would envision an output device database describing what kind of a device it is (a normal monitor, a video projector, a HMD, a TV, ...) and then the software that implements the desktop or UI policy will decide how that will be used. Some policy examples: - A projector: do not extend desktop UI, but have the output normally available, so one can put regular windows there (presentation software). Turned on by default. - A HMD: Do not extend desktop, do not expose to normal apps, keep it off until special request. - A HMD with cinematic mode support: Extend desktop, turn on by default, allow special HMD requests. - A TV: Try to disable overscan or try to compensate for it by default. Also, a normal monitor and maybe a TV
Re: [PATCH 1/6] drm: Add writeback connector type
On Fri, 25 Nov 2016 16:48:59 + Brian Starkey wrote: > +/** > + * drm_writeback_connector_init - Initialize a writeback connector and its > properties > + * @dev: DRM device > + * @wb_connector: Writeback connector to initialize > + * @funcs: Connector funcs vtable > + * @formats: Array of supported pixel formats for the writeback engine > + * @n_formats: Length of the formats array > + * > + * This function creates the writeback-connector-specific properties if they > + * have not been already created, initializes the connector as > + * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the property > + * values. > + * > + * Drivers should always use this function instead of drm_connector_init() to > + * set up writeback connectors. > + * > + * Returns: 0 on success, or a negative error code > + */ > +int drm_writeback_connector_init(struct drm_device *dev, > + struct drm_writeback_connector *wb_connector, > + const struct drm_connector_funcs *funcs, > + u32 *formats, int n_formats) This should probably be 'const u32 *formats', since developers are likely to define a this array with a 'static const' specifier in their driver. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCHv4 3/3] drm/vgem: Enable dmabuf import interfaces
On Thu, May 04, 2017 at 09:09:54PM -0700, Joe Perches wrote: > On Thu, 2017-05-04 at 21:25 +0100, Chris Wilson wrote: > > On Thu, May 04, 2017 at 11:45:48AM -0700, Laura Abbott wrote: > > > > > > Enable the GEM dma-buf import interfaces in addition to the export > > > interfaces. This lets vgem be used as a test source for other allocators > > > (e.g. Ion). > > > > > > Reviewed-by: Chris Wilson > > > Signed-off-by: Laura Abbott > > > --- > > > v4: Use new drm_gem_prime_import_dev function > > > --- > > > static const struct vm_operations_struct vgem_gem_vm_ops = { > > > @@ -114,12 +142,8 @@ static void vgem_postclose(struct drm_device *dev, > > > struct drm_file *file) > > > kfree(vfile); > > > } > > > > > > -/* ioctls */ > > > - > > > -static struct drm_gem_object *vgem_gem_create(struct drm_device *dev, > > > - struct drm_file *file, > > > - unsigned int *handle, > > > - unsigned long size) > > > +static struct drm_vgem_gem_object *__vgem_gem_create(struct drm_device > > > *dev, > > > + unsigned long size) > > > > I'm going to guess that doesn't line up anymore. If checkpatch isn't > > complaining, then sorry for the noise. > > Because of the very long identifiers, perhaps a > nicer way to write this is like: > > static struct drm_vgem_gem_object * > __vgen_gem_create(struct drm_device *dev, unsigned long size); Yes, we frequently use that pattern for very_long_function_names. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v2 1/2] drm: Add optinal YCBCR_ENCODING property to drm_plane
Hi Jyri, Thank you for the patch. On Thursday 04 May 2017 10:14:25 Jyri Sarha wrote: > Add a standard optinal property to control YCbCr conversion in DRM > planes. The property is stored to drm_plane object to allow different > set of supported conversion modes for different planes on the device. > > Signed-off-by: Jyri Sarha > --- > drivers/gpu/drm/drm_atomic.c | 5 > drivers/gpu/drm/drm_color_mgmt.c | 59 > drivers/gpu/drm/drm_plane.c | 3 ++ > include/drm/drm_color_mgmt.h | 14 ++ > include/drm/drm_plane.h | 6 > 5 files changed, 87 insertions(+) [snip] > diff --git a/drivers/gpu/drm/drm_color_mgmt.c > b/drivers/gpu/drm/drm_color_mgmt.c index 533f3a3..245b14a 100644 > --- a/drivers/gpu/drm/drm_color_mgmt.c > +++ b/drivers/gpu/drm/drm_color_mgmt.c [snip] > @@ -85,6 +90,13 @@ > * drm_mode_crtc_set_gamma_size(). Drivers which support both should use > * drm_atomic_helper_legacy_gamma_set() to alias the legacy gamma ramp with > the > * "GAMMA_LUT" property above. > + * > + * The &drm_plane object's properties are: > + * > + * "YCBCR_ENCODING" > + * Optional plane enum property to control YCbCr to RGB > + * conversion. The driver provides a subset of standard > + * enum values supported by the DRM plane. > */ As already mentioned by Hans Verkuil, I also recommend not mixing the encoding and quantization in a single property. If you split them, I would then drop the YCBCR_ prefix (or replace it by something more generic) at least for the quantization property, as it would apply to RGB as well. For the encoding property, we have support in V4L2 for a two HSV encodings, so it could make sense to drop or replace the YCBCR_ prefix, but on the other hand I doubt we'll see any display hardware with native support for HSV :-) > /** > @@ -333,3 +345,50 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev, > drm_modeset_unlock(&crtc->mutex); > return ret; > } > + > +static char *ycbcr_encoding_name[] = { > + [DRM_PLANE_YCBCR_BT601_FULL_RANGE] = "BT.601 full range", > + [DRM_PLANE_YCBCR_BT601_LIMITED_RANGE] = "BT.601 limited range", > + [DRM_PLANE_YCBCR_BT709_LIMITED_RANGE] = "BT.709 limited range", > + [DRM_PLANE_YCBCR_BT2020_LIMITED_RANGE] = "BT.2020 limited range", > +}; > + > +/** > + * drm_plane_create_ycbcr_properties - ycbcr related plane properties > + * @enum_list: drm_prop_enum_list array of supported modes without names > + * @enum_list_len: length of enum_list array > + * @default_mode: default YCbCr encoding > + * > + * Create and attach plane specific YCBCR_ENCODING property to to the > + * drm_plane object. The supported encodings should be provided in the > + * enum_list parameter. The enum_list parameter should not contain the > + * enum names, because the standard names are added by this function. > + */ > +int drm_plane_create_ycbcr_properties(struct drm_plane *plane, > + struct drm_prop_enum_list *enum_list, > + unsigned int enum_list_len, > + enum drm_plane_ycbcr_encoding default_mode) I wonder whether we shouldn't simplify the API by passing a bitmask of supported encodings. Sure, you would have to allocate the array of drm_prop_enum_list internally in the function, but driver code would be simpler. Even if you don't like bitmasks, I think we should pass a const pointer and duplicate the array internally to fill the names. Drivers will in many cases pass the same array for all planes, modifying it in the function is asking for trouble (even if it should be OK with the current implementation). By the way, for drivers that support the same encodings for all planes, would it be worth it to support allocation of a single property instead of allocating one per plane ? > +{ > + struct drm_device *dev = plane->dev; > + struct drm_property *prop; > + unsigned int i; > + > + if (WARN_ON(plane->ycbcr_encoding_property != NULL)) > + return -EEXIST; > + > + for (i = 0; i < enum_list_len; i++) { > + enum drm_plane_ycbcr_encoding encoding = enum_list[i].type; > + > + enum_list[i].name = ycbcr_encoding_name[encoding]; > + } > + > + prop = drm_property_create_enum(dev, DRM_MODE_PROP_ATOMIC, > + "YCBCR_ENCODING", > + enum_list, enum_list_len); > + if (!prop) > + return -ENOMEM; > + plane->ycbcr_encoding_property = prop; > + drm_object_attach_property(&plane->base, prop, default_mode); > + > + return 0; > +} > diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c > index fedd4d6..007c4d7 100644 > --- a/drivers/gpu/drm/drm_plane.c > +++ b/drivers/gpu/drm/drm_plane.c > @@ -244,6 +244,9 @@ void drm_plane_cleanup(struct drm_plane *plane) > > kfree(plane->name); > > + if (plane->ycbcr_encoding_property) > + d
Re: [Intel-gfx] [PATCH v5 4/9] drm/i915: Allow choosing how to adjust brightness if both supported
Hi Puthikorn, [auto build test ERROR on drm-intel/for-linux-next] [also build test ERROR on next-20170504] [cannot apply to v4.11] [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/Puthikorn-Voravootivat/Enhancement-to-intel_dp_aux_backlight-driver/20170505-003007 base: git://anongit.freedesktop.org/drm-intel for-linux-next config: x86_64-rhel (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): In file included from include/linux/module.h:18:0, from include/drm/drmP.h:59, from drivers/gpu/drm/i915/i915_drv.h:47, from drivers/gpu/drm/i915/i915_params.c:26: drivers/gpu/drm/i915/i915_params.c: In function '__check_enable_dpcd_backlight': >> include/linux/moduleparam.h:146:27: error: return from incompatible pointer >> type [-Werror=incompatible-pointer-types] param_check_##type(name, &(value)); \ ^ include/linux/moduleparam.h:344:68: note: in definition of macro '__param_check' static inline type __always_unused *__check_##name(void) { return(p); } ^ include/linux/moduleparam.h:146:2: note: in expansion of macro 'param_check_bool' param_check_##type(name, &(value)); \ ^~~~ drivers/gpu/drm/i915/i915_params.c:249:1: note: in expansion of macro 'module_param_named' module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600); ^~ cc1: some warnings being treated as errors -- In file included from include/linux/module.h:18:0, from include/drm/drmP.h:59, from drivers/gpu//drm/i915/i915_drv.h:47, from drivers/gpu//drm/i915/i915_params.c:26: drivers/gpu//drm/i915/i915_params.c: In function '__check_enable_dpcd_backlight': >> include/linux/moduleparam.h:146:27: error: return from incompatible pointer >> type [-Werror=incompatible-pointer-types] param_check_##type(name, &(value)); \ ^ include/linux/moduleparam.h:344:68: note: in definition of macro '__param_check' static inline type __always_unused *__check_##name(void) { return(p); } ^ include/linux/moduleparam.h:146:2: note: in expansion of macro 'param_check_bool' param_check_##type(name, &(value)); \ ^~~~ drivers/gpu//drm/i915/i915_params.c:249:1: note: in expansion of macro 'module_param_named' module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600); ^~ cc1: some warnings being treated as errors vim +146 include/linux/moduleparam.h 3baee201 Jani Nikula 2014-08-27 130 */ 3baee201 Jani Nikula 2014-08-27 131 #define module_param_unsafe(name, type, perm) \ 3baee201 Jani Nikula 2014-08-27 132 module_param_named_unsafe(name, name, type, perm) 3baee201 Jani Nikula 2014-08-27 133 3baee201 Jani Nikula 2014-08-27 134 /** 546970bc Rusty Russell 2010-08-11 135 * module_param_named - typesafe helper for a renamed module/cmdline parameter 546970bc Rusty Russell 2010-08-11 136 * @name: a valid C identifier which is the parameter name. 546970bc Rusty Russell 2010-08-11 137 * @value: the actual lvalue to alter. 546970bc Rusty Russell 2010-08-11 138 * @type: the type of the parameter 546970bc Rusty Russell 2010-08-11 139 * @perm: visibility in sysfs. 546970bc Rusty Russell 2010-08-11 140 * 546970bc Rusty Russell 2010-08-11 141 * Usually it's a good idea to have variable names and user-exposed names the 546970bc Rusty Russell 2010-08-11 142 * same, but that's harder if the variable must be non-static or is inside a 546970bc Rusty Russell 2010-08-11 143 * structure. This allows exposure under a different name. 546970bc Rusty Russell 2010-08-11 144 */ 546970bc Rusty Russell 2010-08-11 145 #define module_param_named(name, value, type, perm)\ 546970bc Rusty Russell 2010-08-11 @146 param_check_##type(name, &(value));\ 546970bc Rusty Russell 2010-08-11 147 module_param_cb(name, ¶m_ops_##type, &value, perm);\ 546970bc Rusty Russell 2010-08-11 148 __MODULE_PARM_TYPE(name, #type) 546970bc Rusty Russell 2010-08-11 149 546970bc Rusty Russell 2010-08-11 150 /** 3baee201 Jani Nikula 2014-08-27 151 * module_param_named_unsafe - same as module_param_named but taints kerne
[Bug 100937] Mesa fails to build with GCC 4.8
https://bugs.freedesktop.org/show_bug.cgi?id=100937 --- Comment #1 from Samuel Pitoiset --- Created attachment 131225 --> https://bugs.freedesktop.org/attachment.cgi?id=131225&action=edit patch Can you try this patch? -- 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: i915 4.9 regression: DP AUX CH sanitization no longer working on Asus desktops
On Thu, May 04, 2017 at 02:52:09PM -0600, Daniel Drake wrote: > On Thu, May 4, 2017 at 2:37 PM, Ville Syrjälä > wrote: > > Please check if commit bb1d132935c2 ("drm/i915/vbt: split out defaults > > that are set when there is no VBT") fixes things for you. > > I think this is not going to help. This would only make a difference > when there is no VBT at all at which point we would see this message > in the logs: > > DRM_INFO("Failed to find VBIOS tables (VBT)\n"); No, the patch will in fact make a difference only when there is a VBT. > > but in this case we have a VBT for ports B, C and E. > > [drm:intel_bios_init [i915]] Port B VBT info: DP:1 HDMI:1 DVI:1 EDP:0 CRT:0 > [drm:intel_bios_init [i915]] VBT HDMI level shift for port B: 8 > [drm:intel_bios_init [i915]] Port C VBT info: DP:0 HDMI:1 DVI:1 EDP:0 CRT:0 > [drm:intel_bios_init [i915]] VBT HDMI level shift for port C: 8 > [drm:intel_bios_init [i915]] Port E VBT info: DP:1 HDMI:0 DVI:0 EDP:0 CRT:0 > [drm:intel_bios_init [i915]] VBT HDMI level shift for port E: 0 > > Let me know if I'm missing something and we will test it anyway Please do. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane
On Fri, May 05, 2017 at 08:58:17AM +0200, Daniel Vetter wrote: > On Thu, May 04, 2017 at 05:51:45PM +0300, Ville Syrjälä wrote: > > On Thu, May 04, 2017 at 03:22:45PM +0200, Daniel Vetter wrote: > > > On Thu, May 04, 2017 at 10:14:26AM +0300, Jyri Sarha wrote: > > > > Add standard optinal property blobs for YCbCr to RGB conversion CSC > > > > matrix and YCbCr preoffset vector in DRM planes. New enums are defined > > > > to YCBCR_ENCODING property to activate the CSC and preoffset > > > > properties. For simplicity the new properties are stored in the > > > > drm_plane object, because the YCBCR_ENCODING is already there. The > > > > blob contents are defined in the uapi/drm/drm_mode.h header. > > > > > > > > Signed-off-by: Jyri Sarha > > > > > > Not sure we want to make this yuv2rgb specific, plenty of planes have > > > generic degamma/offset, csc, gamme/offset hw. I think what we want instead > > > is the generic csc support, plus a ycbcr2rgb mode of "bypass". > > > > My idea is to not expose this. And instead just expose a normal > > ctm, and then just refuse any combination of YCbCr->RGB/degamma/ctm > > that can't be done by the hw. > > But that means we'd need to have a bit-perfect match with a canonical > conversion matrix (even if maybe the hw has a rounding bug and implements > something slightly different than the standard). I don't see what this has to do with bit correctness. Different hw is likely to use different precision for this stuff anyway, so I'm 100% sure we'll not get the same results from all hardware. And really, getting 100% same output doesn't seem all that important here anyway. Anyway, all I'm saying is that we should expose a standard pipeline: ycbcr->rgb -> degamma -> ctm -> gamma And each driver can then expose whatever parts of that they wish. If you want to expoe a programmable matrix you expose it as the ctm. I don't think having a programmable matrix as both ycbcr->rgb and ctm would really benefit us. > That seems a bit an > awkward interface. Or is your idea that hw with a ctm would simply not > expose the colorspace enum, if it doesn't also have fixed-function bits on > top of the ctm? > -Daniel > > > > > > > > > And as usual, this needs some userspace compositor which actually uses it > > > (not just a demo, since there's a huge difference between a demo and > > > something that has to Just Work like a real compositor). > > > -Daniel > > > > > > > --- > > > > drivers/gpu/drm/drm_atomic.c| 19 + > > > > drivers/gpu/drm/drm_atomic_helper.c | 9 ++ > > > > drivers/gpu/drm/drm_color_mgmt.c| 55 > > > > +++-- > > > > drivers/gpu/drm/drm_plane.c | 6 > > > > include/drm/drm_color_mgmt.h| 3 ++ > > > > include/drm/drm_plane.h | 4 +++ > > > > include/uapi/drm/drm_mode.h | 12 > > > > 7 files changed, 106 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c > > > > index bcef93d..87a2d55 100644 > > > > --- a/drivers/gpu/drm/drm_atomic.c > > > > +++ b/drivers/gpu/drm/drm_atomic.c > > > > @@ -733,6 +733,7 @@ int drm_atomic_plane_set_property(struct drm_plane > > > > *plane, > > > > struct drm_device *dev = plane->dev; > > > > struct drm_mode_config *config = &dev->mode_config; > > > > int ret; > > > > + bool dummy; > > > > > > > > if (property == config->prop_fb_id) { > > > > struct drm_framebuffer *fb = > > > > drm_framebuffer_lookup(dev, val); > > > > @@ -777,6 +778,18 @@ int drm_atomic_plane_set_property(struct drm_plane > > > > *plane, > > > > state->zpos = val; > > > > } else if (property == plane->ycbcr_encoding_property) { > > > > state->ycbcr_encoding = val; > > > > + } else if (property == plane->ycbcr_decode_csc_property) { > > > > + ret = drm_atomic_replace_property_blob_from_id(dev, > > > > + &state->ycbcr_decode_csc, val, > > > > + sizeof(struct drm_ycbcr_decode_csc), > > > > + &dummy); > > > > + return ret; > > > > + } else if (property == plane->ycbcr_csc_preoffset_property) { > > > > + ret = drm_atomic_replace_property_blob_from_id(dev, > > > > + &state->ycbcr_csc_preoffset, val, > > > > + sizeof(struct drm_ycbcr_csc_preoffset), > > > > + &dummy); > > > > + return ret; > > > > } else if (plane->funcs->atomic_set_property) { > > > > return plane->funcs->atomic_set_property(plane, state, > > > > property, val); > > > > @@ -839,6 +852,12 @@ int drm_atomic_plane_set_property(struct drm_plane > > > > *plane, > > > > *val = state->zpos; > > > > } else if (prop
Re: [PATCHv4 2/3] drm/prime: Introduce drm_gem_prime_import_dev
Hi Laura, [auto build test WARNING on drm/drm-next] [also build test WARNING on next-20170505] [cannot apply to v4.11] [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/Laura-Abbott/dma_buf-import-support-for-vgem/20170505-173856 base: git://people.freedesktop.org/~airlied/linux.git drm-next config: arm-at91_dt_defconfig (attached as .config) compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/01org/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 >>): In file included from include/drm/drm_file.h:38:0, from drivers/gpu/drm/drm_file.c:38: >> include/drm/drm_prime.h:71:14: warning: 'struct device' declared inside >> parameter list will not be visible outside of this definition or declaration struct device *attach_dev); ^~ vim +71 include/drm/drm_prime.h 55 56 struct drm_device; 57 struct drm_gem_object; 58 struct drm_file; 59 60 struct dma_buf *drm_gem_prime_export(struct drm_device *dev, 61 struct drm_gem_object *obj, 62 int flags); 63 int drm_gem_prime_handle_to_fd(struct drm_device *dev, 64 struct drm_file *file_priv, uint32_t handle, uint32_t flags, 65 int *prime_fd); 66 struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, 67 struct dma_buf *dma_buf); 68 69 struct drm_gem_object *drm_gem_prime_import_dev(struct drm_device *dev, 70 struct dma_buf *dma_buf, > 71 struct device *attach_dev); 72 73 int drm_gem_prime_fd_to_handle(struct drm_device *dev, 74 struct drm_file *file_priv, int prime_fd, uint32_t *handle); 75 struct dma_buf *drm_gem_dmabuf_export(struct drm_device *dev, 76struct dma_buf_export_info *exp_info); 77 void drm_gem_dmabuf_release(struct dma_buf *dma_buf); 78 79 int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages, --- 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
[Bug 100937] Mesa fails to build with GCC 4.8
https://bugs.freedesktop.org/show_bug.cgi?id=100937 --- Comment #2 from network...@rkmail.ru --- (In reply to Samuel Pitoiset from comment #1) > Created attachment 131225 [details] [review] > patch > > Can you try this patch? It did build successfully with the proposed patch. -- 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 RFC v2 1/2] drm: Add optinal YCBCR_ENCODING property to drm_plane
On 05/05/17 12:07, Laurent Pinchart wrote: > Hi Jyri, > > Thank you for the patch. > > On Thursday 04 May 2017 10:14:25 Jyri Sarha wrote: >> Add a standard optinal property to control YCbCr conversion in DRM >> planes. The property is stored to drm_plane object to allow different >> set of supported conversion modes for different planes on the device. >> >> Signed-off-by: Jyri Sarha >> --- >> drivers/gpu/drm/drm_atomic.c | 5 >> drivers/gpu/drm/drm_color_mgmt.c | 59 >> drivers/gpu/drm/drm_plane.c | 3 ++ >> include/drm/drm_color_mgmt.h | 14 ++ >> include/drm/drm_plane.h | 6 >> 5 files changed, 87 insertions(+) > > [snip] > >> diff --git a/drivers/gpu/drm/drm_color_mgmt.c >> b/drivers/gpu/drm/drm_color_mgmt.c index 533f3a3..245b14a 100644 >> --- a/drivers/gpu/drm/drm_color_mgmt.c >> +++ b/drivers/gpu/drm/drm_color_mgmt.c > > [snip] > >> @@ -85,6 +90,13 @@ >> * drm_mode_crtc_set_gamma_size(). Drivers which support both should use >> * drm_atomic_helper_legacy_gamma_set() to alias the legacy gamma ramp with >> the >> * "GAMMA_LUT" property above. >> + * >> + * The &drm_plane object's properties are: >> + * >> + * "YCBCR_ENCODING" >> + * Optional plane enum property to control YCbCr to RGB >> + * conversion. The driver provides a subset of standard >> + * enum values supported by the DRM plane. >> */ > > As already mentioned by Hans Verkuil, I also recommend not mixing the > encoding > and quantization in a single property. If you split them, I would then drop > the YCBCR_ prefix (or replace it by something more generic) at least for the > quantization property, as it would apply to RGB as well. For the encoding > property, we have support in V4L2 for a two HSV encodings, so it could make > sense to drop or replace the YCBCR_ prefix, but on the other hand I doubt > we'll see any display hardware with native support for HSV :-) > COLOR_ENCODING? Why not, the YCbCr could then be in the enum names. >> /** >> @@ -333,3 +345,50 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev, >> drm_modeset_unlock(&crtc->mutex); >> return ret; >> } >> + >> +static char *ycbcr_encoding_name[] = { >> +[DRM_PLANE_YCBCR_BT601_FULL_RANGE] = "BT.601 full range", >> +[DRM_PLANE_YCBCR_BT601_LIMITED_RANGE] = "BT.601 limited range", >> +[DRM_PLANE_YCBCR_BT709_LIMITED_RANGE] = "BT.709 limited range", >> +[DRM_PLANE_YCBCR_BT2020_LIMITED_RANGE] = "BT.2020 limited range", >> +}; >> + >> +/** >> + * drm_plane_create_ycbcr_properties - ycbcr related plane properties >> + * @enum_list: drm_prop_enum_list array of supported modes without names >> + * @enum_list_len: length of enum_list array >> + * @default_mode: default YCbCr encoding >> + * >> + * Create and attach plane specific YCBCR_ENCODING property to to the >> + * drm_plane object. The supported encodings should be provided in the >> + * enum_list parameter. The enum_list parameter should not contain the >> + * enum names, because the standard names are added by this function. >> + */ >> +int drm_plane_create_ycbcr_properties(struct drm_plane *plane, >> +struct drm_prop_enum_list *enum_list, >> +unsigned int enum_list_len, >> +enum drm_plane_ycbcr_encoding default_mode) > > I wonder whether we shouldn't simplify the API by passing a bitmask of > supported encodings. Sure, you would have to allocate the array of > drm_prop_enum_list internally in the function, but driver code would be > simpler. Even if you don't like bitmasks, I think we should pass a const > pointer and duplicate the array internally to fill the names. Drivers will in > many cases pass the same array for all planes, modifying it in the function > is > asking for trouble (even if it should be OK with the current implementation). > I used a bitmask property first, but abandoned it because the encodings do not behave like bitmasks. You can not have BT.601 | BT.709 at the same time. A bitmask in the function API would certainly work and be probably better, but I've tried to keep the implementation simple, while we are still discussing what we should actually do. > By the way, for drivers that support the same encodings for all planes, would > it be worth it to support allocation of a single property instead of > allocating one per plane ? I was thinking of that, but AFAIK there is really not that many planes on the know HW that it would justify the complexity. > >> +{ >> +struct drm_device *dev = plane->dev; >> +struct drm_property *prop; >> +unsigned int i; >> + >> +if (WARN_ON(plane->ycbcr_encoding_property != NULL)) >> +return -EEXIST; >> + >> +for (i = 0; i < enum_list_len; i++) { >> +enum drm_plane_ycbcr_encoding encoding = enum_list[i].type; >> + >> +enum_list[i].name = ycbcr_encoding_name[encoding]; >> +} >> + >> +prop = drm_property_cre
Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC
On Fri, May 05, 2017 at 04:53:43PM +0800, icen...@aosc.io wrote: > > > + de2_clocks: clock@100 { > > > + compatible = "allwinner,sun50i-h5-de2-clk"; > > > > I am a bit skeptical about this. Since the V3S only has one mixer, do > > the clocks > > for the second one even exist? > > It's described in the de_clock.c in the BSP source code, and in hardware > these bits can be really set (although without clock output). > > So I use this compatible which has still the extra clocks. If it's not usable, then it shouldn't be in the code, it's basically dead code. -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 08/13] drm/sun4i: add support for Allwinner DE2 mixers
On Fri, May 05, 2017 at 12:50:51AM +0800, icen...@aosc.io wrote: > > > +void sun8i_mixer_layer_enable(struct sun8i_mixer *mixer, > > > + int layer, bool enable) > > > +{ > > > + u32 val; > > > + /* Currently the first UI channel is used */ > > > + int chan = mixer->cfg->vi_num; > > > + > > > + DRM_DEBUG_DRIVER("Enabling layer %d in channel %d\n", layer, chan); > > > + > > > + if (enable) > > > + val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN; > > > + else > > > + val = 0; > > > + > > > + regmap_update_bits(mixer->engine.regs, > > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR(chan, layer), > > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val); > > > + > > > + /* Set the alpha configuration */ > > > + regmap_update_bits(mixer->engine.regs, > > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR(chan, layer), > > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK, > > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_DEF); > > > + regmap_update_bits(mixer->engine.regs, > > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR(chan, layer), > > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK, > > > +SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_DEF); > > > +} > > > > This one too. > > It's called from sun8i_layer.c, so it cannot be static. Fair enough. > > > + /* Set base coordinates */ > > > + DRM_DEBUG_DRIVER("Layer coordinates X: %d Y: %d\n", > > > + state->crtc_x, state->crtc_y); > > > + regmap_write(mixer->engine.regs, > > > + SUN8I_MIXER_CHAN_UI_LAYER_COORD(chan, layer), > > > + SUN8I_MIXER_COORD(state->crtc_x, state->crtc_y)); > > > > X and Y are fixed point numbers. You want to keep only the higher 16 > > bits there. > > Do you mean "lower 16 bits"? Thus should I (x & 0x) or ((u16)x) ? Nevermind, I got confused with src_x and src_y. > P.S. The negative coordinates are broken, how should I deal with it? or > is the coordinates promised to be not negative? Adjust the buffer base address, and use a shorter line. You have such an example in the sun4i code. Maxime -- Maxime Ripard, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/6] drm: Add writeback connector type
On Fri, May 05, 2017 at 10:22:19AM +0200, Boris Brezillon wrote: > On Fri, 25 Nov 2016 16:48:59 + > Brian Starkey wrote: > > > +/** > > + * drm_writeback_connector_init - Initialize a writeback connector and its > > properties > > + * @dev: DRM device > > + * @wb_connector: Writeback connector to initialize > > + * @funcs: Connector funcs vtable > > + * @formats: Array of supported pixel formats for the writeback engine > > + * @n_formats: Length of the formats array > > + * > > + * This function creates the writeback-connector-specific properties if > > they > > + * have not been already created, initializes the connector as > > + * type DRM_MODE_CONNECTOR_WRITEBACK, and correctly initializes the > > property > > + * values. > > + * > > + * Drivers should always use this function instead of drm_connector_init() > > to > > + * set up writeback connectors. > > + * > > + * Returns: 0 on success, or a negative error code > > + */ > > +int drm_writeback_connector_init(struct drm_device *dev, > > +struct drm_writeback_connector *wb_connector, > > +const struct drm_connector_funcs *funcs, > > +u32 *formats, int n_formats) > > This should probably be 'const u32 *formats', since developers are > likely to define a this array with a 'static const' specifier in their > driver. Fixed in the v4. Thanks, Liviu -- | I would like to | | fix the world, | | but they're not | | giving me the | \ source code! / --- ¯\_(ツ)_/¯ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v2 1/2] drm: Add optinal YCBCR_ENCODING property to drm_plane
Hi Jyri, On Friday 05 May 2017 15:11:06 Jyri Sarha wrote: > On 05/05/17 12:07, Laurent Pinchart wrote: > > On Thursday 04 May 2017 10:14:25 Jyri Sarha wrote: > >> Add a standard optinal property to control YCbCr conversion in DRM > >> planes. The property is stored to drm_plane object to allow different > >> set of supported conversion modes for different planes on the device. > >> > >> Signed-off-by: Jyri Sarha > >> --- > >> > >> drivers/gpu/drm/drm_atomic.c | 5 > >> drivers/gpu/drm/drm_color_mgmt.c | 59 ++ > >> drivers/gpu/drm/drm_plane.c | 3 ++ > >> include/drm/drm_color_mgmt.h | 14 ++ > >> include/drm/drm_plane.h | 6 > >> 5 files changed, 87 insertions(+) > > > > [snip] > > > >> diff --git a/drivers/gpu/drm/drm_color_mgmt.c > >> b/drivers/gpu/drm/drm_color_mgmt.c index 533f3a3..245b14a 100644 > >> --- a/drivers/gpu/drm/drm_color_mgmt.c > >> +++ b/drivers/gpu/drm/drm_color_mgmt.c > > > > [snip] > > > >> @@ -85,6 +90,13 @@ > >> * drm_mode_crtc_set_gamma_size(). Drivers which support both should use > >> * drm_atomic_helper_legacy_gamma_set() to alias the legacy gamma ramp > >> with the > >> * "GAMMA_LUT" property above. > >> + * > >> + * The &drm_plane object's properties are: > >> + * > >> + * "YCBCR_ENCODING" > >> + *Optional plane enum property to control YCbCr to RGB > >> + *conversion. The driver provides a subset of standard > >> + *enum values supported by the DRM plane. > >> */ > > > > As already mentioned by Hans Verkuil, I also recommend not mixing the > > encoding and quantization in a single property. If you split them, I > > would then drop the YCBCR_ prefix (or replace it by something more > > generic) at least for the quantization property, as it would apply to RGB > > as well. For the encoding property, we have support in V4L2 for a two HSV > > encodings, so it could make sense to drop or replace the YCBCR_ prefix, > > but on the other hand I doubt we'll see any display hardware with native > > support for HSV :-) > > COLOR_ENCODING? Why not, the YCbCr could then be in the enum names. Sounds good to me. > >> /** > >> > >> @@ -333,3 +345,50 @@ int drm_mode_gamma_get_ioctl(struct drm_device *dev, > >>drm_modeset_unlock(&crtc->mutex); > >>return ret; > >> } > >> + > >> +static char *ycbcr_encoding_name[] = { > >> + [DRM_PLANE_YCBCR_BT601_FULL_RANGE] = "BT.601 full range", > >> + [DRM_PLANE_YCBCR_BT601_LIMITED_RANGE] = "BT.601 limited range", > >> + [DRM_PLANE_YCBCR_BT709_LIMITED_RANGE] = "BT.709 limited range", > >> + [DRM_PLANE_YCBCR_BT2020_LIMITED_RANGE] = "BT.2020 limited range", > >> +}; > >> + > >> +/** > >> + * drm_plane_create_ycbcr_properties - ycbcr related plane properties > >> + * @enum_list: drm_prop_enum_list array of supported modes without names > >> + * @enum_list_len: length of enum_list array > >> + * @default_mode: default YCbCr encoding > >> + * > >> + * Create and attach plane specific YCBCR_ENCODING property to to the > >> + * drm_plane object. The supported encodings should be provided in the > >> + * enum_list parameter. The enum_list parameter should not contain the > >> + * enum names, because the standard names are added by this function. > >> + */ > >> +int drm_plane_create_ycbcr_properties(struct drm_plane *plane, > >> + struct drm_prop_enum_list *enum_list, > >> + unsigned int enum_list_len, > >> + enum drm_plane_ycbcr_encoding default_mode) > > > > I wonder whether we shouldn't simplify the API by passing a bitmask of > > supported encodings. Sure, you would have to allocate the array of > > drm_prop_enum_list internally in the function, but driver code would be > > simpler. Even if you don't like bitmasks, I think we should pass a const > > pointer and duplicate the array internally to fill the names. Drivers will > > in many cases pass the same array for all planes, modifying it in the > > function is asking for trouble (even if it should be OK with the current > > implementation). > > I used a bitmask property first, but abandoned it because the encodings > do not behave like bitmasks. You can not have BT.601 | BT.709 at the > same time. Sorry, I should have expressed myself more clearly, I meant an enum property with a bitmask in the function API. I agree that a bitmask property isn't a good match. > A bitmask in the function API would certainly work and be probably > better, but I've tried to keep the implementation simple, while we are > still discussing what we should actually do. I think we're getting there with 1/2, so feel free to update that in the next version :-) > > By the way, for drivers that support the same encodings for all planes, > > would it be worth it to support allocation of a single property instead > > of allocating one per plane ? > > I was thinking of that, but AFAIK there is really not that many planes > on the know HW that it would
Re: [PATCH 6/8] omapdrm: hdmi4: refcount hdmi_power_on/off_core
On 04/28/17 13:30, Tomi Valkeinen wrote: > On 14/04/17 13:25, Hans Verkuil wrote: >> From: Hans Verkuil >> >> The hdmi_power_on/off_core functions can be called multiple times: >> when the HPD changes and when the HDMI CEC support needs to power >> the HDMI core. >> >> So use a counter to know when to really power on or off the HDMI core. >> >> Also call hdmi4_core_powerdown_disable() in hdmi_power_on_core() to >> power up the HDMI core (needed for CEC). >> >> Signed-off-by: Hans Verkuil >> --- >> drivers/gpu/drm/omapdrm/dss/hdmi4.c | 12 +++- >> 1 file changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c >> b/drivers/gpu/drm/omapdrm/dss/hdmi4.c >> index 4a164dc01f15..e371b47ff6ff 100644 >> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c >> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c >> @@ -124,14 +124,19 @@ static int hdmi_power_on_core(struct omap_dss_device >> *dssdev) >> { >> int r; >> >> +if (hdmi.core.core_pwr_cnt++) >> +return 0; >> + > > How's the locking between the CEC side and the DRM side? Normally these > functions are protected with the DRM modesetting locks, but CEC doesn't > come from there. We have the hdmi.lock, did you check that it's held > when CEC side calls shared functions? Yes, the hdmi_power_on/off_core functions are all called from other functions with the hdmi.lock taken. The CEC code calls those higher level functions (hdmi4_core_enable/disable). > >> r = regulator_enable(hdmi.vdda_reg); >> if (r) >> -return r; >> +goto err_reg_enable; >> >> r = hdmi_runtime_get(); >> if (r) >> goto err_runtime_get; >> >> +hdmi4_core_powerdown_disable(&hdmi.core); > > I'd like to have the powerdown_disable as a separate patch. Will do. > Also, now > that you call it here, I believe it can be dropped from hdmi4_configure(). I was a bit scared of messing with that function. But if you say it can be removed, then who am I to argue? :-) > > Hmm, but in hdmi4_configure we call hdmi_core_swreset_assert() before > hdmi4_core_powerdown_disable(). I wonder what exactly that does, and > whether we end up resetting also the CEC parts when we change the videomode. Good one. I'll attempt to check this. Regards, Hans > > Tomi > ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100802] [regression] mostly blank graphics on Faeria
https://bugs.freedesktop.org/show_bug.cgi?id=100802 --- Comment #4 from Timo Aaltonen --- Backporting the full X stack (including kernel 4.10, xorg 1.19.3) from 17.04 to 16.04 didn't break it, so looks like the breakage is due to some compiler issue on 17.04? That sounds like a treat to track down.. -- 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 0/5] GPU-DRM-STI: Fine-tuning for some function implementations
From: Markus Elfring Date: Fri, 5 May 2017 15:45:45 +0200 A few update suggestions were taken into account from static source code analysis. Markus Elfring (5): Reduce function calls for sequence output at five places Replace 17 seq_puts() calls by seq_putc() Fix a typo in a comment line Fix typos in a comment line Adjust two checks for null pointers in sti_hqvdp_probe() drivers/gpu/drm/sti/sti_cursor.c | 5 ++--- drivers/gpu/drm/sti/sti_dvo.c| 3 +-- drivers/gpu/drm/sti/sti_gdp.c| 3 +-- drivers/gpu/drm/sti/sti_hda.c| 9 +++-- drivers/gpu/drm/sti/sti_hdmi.c | 23 ++- drivers/gpu/drm/sti/sti_hqvdp.c | 7 +++ drivers/gpu/drm/sti/sti_mixer.c | 3 +-- drivers/gpu/drm/sti/sti_tvout.c | 7 +++ drivers/gpu/drm/sti/sti_vid.c| 5 ++--- 9 files changed, 26 insertions(+), 39 deletions(-) -- 2.12.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 1/5] drm/sti: Reduce function calls for sequence output at five places
From: Markus Elfring Date: Fri, 5 May 2017 14:54:52 +0200 Some data were put into a sequence by separate function calls. Print the same data by five single function calls instead. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/gpu/drm/sti/sti_gdp.c | 3 +-- drivers/gpu/drm/sti/sti_hda.c | 6 ++ drivers/gpu/drm/sti/sti_hdmi.c | 6 ++ 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_gdp.c b/drivers/gpu/drm/sti/sti_gdp.c index 88f16cdf6a4b..3caced5f2e86 100644 --- a/drivers/gpu/drm/sti/sti_gdp.c +++ b/drivers/gpu/drm/sti/sti_gdp.c @@ -266,8 +266,7 @@ static void gdp_node_dump_node(struct seq_file *s, struct sti_gdp_node *node) seq_printf(s, "\n\tKEY2 0x%08X", node->gam_gdp_key2); seq_printf(s, "\n\tPPT 0x%08X", node->gam_gdp_ppt); gdp_dbg_ppt(s, node->gam_gdp_ppt); - seq_printf(s, "\n\tCML 0x%08X", node->gam_gdp_cml); - seq_puts(s, "\n"); + seq_printf(s, "\n\tCML 0x%08X\n", node->gam_gdp_cml); } static int gdp_node_dbg_show(struct seq_file *s, void *arg) diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c index 0c0a75bc8bc3..e3475a17eaeb 100644 --- a/drivers/gpu/drm/sti/sti_hda.c +++ b/drivers/gpu/drm/sti/sti_hda.c @@ -320,8 +320,7 @@ static void hda_dbg_awg_microcode(struct seq_file *s, void __iomem *reg) { unsigned int i; - seq_puts(s, "\n\n"); - seq_puts(s, " HDA AWG microcode:"); + seq_puts(s, "\n\n HDA AWG microcode:"); for (i = 0; i < AWG_MAX_INST; i++) { if (i % 8 == 0) seq_printf(s, "\n %04X:", i); @@ -333,8 +332,7 @@ static void hda_dbg_video_dacs_ctrl(struct seq_file *s, void __iomem *reg) { u32 val = readl(reg); - seq_puts(s, "\n"); - seq_printf(s, "\n %-25s 0x%08X", "VIDEO_DACS_CONTROL", val); + seq_printf(s, "\n\n %-25s 0x%08X", "VIDEO_DACS_CONTROL", val); seq_puts(s, "\tHD DACs "); seq_puts(s, val & DAC_CFG_HD_HZUVW_OFF_MASK ? "disabled" : "enabled"); } diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index 243905b6ae59..52cdff651c0d 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -692,8 +692,7 @@ static int hdmi_dbg_show(struct seq_file *s, void *data) DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD4, HDMI_IFRAME_SLOT_AVI); DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD5, HDMI_IFRAME_SLOT_AVI); DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD6, HDMI_IFRAME_SLOT_AVI); - seq_puts(s, "\n"); - seq_printf(s, "\n AUDIO Infoframe (Data Island slot N=%d):", + seq_printf(s, "\n\n AUDIO Infoframe (Data Island slot N=%d):", HDMI_IFRAME_SLOT_AUDIO); DBGFS_DUMP_DI(HDMI_SW_DI_N_HEAD_WORD, HDMI_IFRAME_SLOT_AUDIO); DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD0, HDMI_IFRAME_SLOT_AUDIO); @@ -703,8 +702,7 @@ static int hdmi_dbg_show(struct seq_file *s, void *data) DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD4, HDMI_IFRAME_SLOT_AUDIO); DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD5, HDMI_IFRAME_SLOT_AUDIO); DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD6, HDMI_IFRAME_SLOT_AUDIO); - seq_puts(s, "\n"); - seq_printf(s, "\n VENDOR SPECIFIC Infoframe (Data Island slot N=%d):", + seq_printf(s, "\n\n VENDOR SPECIFIC Infoframe (Data Island slot N=%d):", HDMI_IFRAME_SLOT_VENDOR); DBGFS_DUMP_DI(HDMI_SW_DI_N_HEAD_WORD, HDMI_IFRAME_SLOT_VENDOR); DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD0, HDMI_IFRAME_SLOT_VENDOR); -- 2.12.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/5] drm/sti: Replace 17 seq_puts() calls by seq_putc()
From: Markus Elfring Date: Fri, 5 May 2017 15:00:46 +0200 Single characters should be put into a sequence at several places. Thus use the corresponding function "seq_putc". This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- drivers/gpu/drm/sti/sti_cursor.c | 3 +-- drivers/gpu/drm/sti/sti_dvo.c| 3 +-- drivers/gpu/drm/sti/sti_hda.c| 3 +-- drivers/gpu/drm/sti/sti_hdmi.c | 17 - drivers/gpu/drm/sti/sti_hqvdp.c | 3 +-- drivers/gpu/drm/sti/sti_mixer.c | 3 +-- drivers/gpu/drm/sti/sti_tvout.c | 5 ++--- drivers/gpu/drm/sti/sti_vid.c| 5 ++--- 8 files changed, 17 insertions(+), 25 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c index cca75bddb9ad..cd35b9d9de26 100644 --- a/drivers/gpu/drm/sti/sti_cursor.c +++ b/drivers/gpu/drm/sti/sti_cursor.c @@ -121,8 +121,7 @@ static int cursor_dbg_show(struct seq_file *s, void *data) cursor_dbg_cml(s, cursor, readl(cursor->regs + CUR_CML)); DBGFS_DUMP(CUR_AWS); DBGFS_DUMP(CUR_AWE); - seq_puts(s, "\n"); - + seq_putc(s, '\n'); return 0; } diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c index bb23318a44b7..24ebc6b2f34d 100644 --- a/drivers/gpu/drm/sti/sti_dvo.c +++ b/drivers/gpu/drm/sti/sti_dvo.c @@ -186,8 +186,7 @@ static int dvo_dbg_show(struct seq_file *s, void *data) DBGFS_DUMP(DVO_LUT_PROG_MID); DBGFS_DUMP(DVO_LUT_PROG_HIGH); dvo_dbg_awg_microcode(s, dvo->regs + DVO_DIGSYNC_INSTR_I); - seq_puts(s, "\n"); - + seq_putc(s, '\n'); return 0; } diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c index e3475a17eaeb..d6ed909d9d75 100644 --- a/drivers/gpu/drm/sti/sti_hda.c +++ b/drivers/gpu/drm/sti/sti_hda.c @@ -354,8 +354,7 @@ static int hda_dbg_show(struct seq_file *s, void *data) hda_dbg_awg_microcode(s, hda->regs + HDA_SYNC_AWGI); if (hda->video_dacs_ctrl) hda_dbg_video_dacs_ctrl(s, hda->video_dacs_ctrl); - seq_puts(s, "\n"); - + seq_putc(s, '\n'); return 0; } diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c index 52cdff651c0d..a59c95a8081b 100644 --- a/drivers/gpu/drm/sti/sti_hdmi.c +++ b/drivers/gpu/drm/sti/sti_hdmi.c @@ -592,7 +592,7 @@ static void hdmi_dbg_cfg(struct seq_file *s, int val) { int tmp; - seq_puts(s, "\t"); + seq_putc(s, '\t'); tmp = val & HDMI_CFG_HDMI_NOT_DVI; DBGFS_PRINT_STR("mode:", tmp ? "HDMI" : "DVI"); seq_puts(s, "\t\t\t\t\t"); @@ -616,7 +616,7 @@ static void hdmi_dbg_sta(struct seq_file *s, int val) { int tmp; - seq_puts(s, "\t"); + seq_putc(s, '\t'); tmp = (val & HDMI_STA_DLL_LCK); DBGFS_PRINT_STR("pll:", tmp ? "locked" : "not locked"); seq_puts(s, "\t\t\t\t\t"); @@ -632,7 +632,7 @@ static void hdmi_dbg_sw_di_cfg(struct seq_file *s, int val) "once every field", "once every frame"}; - seq_puts(s, "\t"); + seq_putc(s, '\t'); tmp = (val & HDMI_IFRAME_CFG_DI_N(HDMI_IFRAME_MASK, 1)); DBGFS_PRINT_STR("Data island 1:", en_di[tmp]); seq_puts(s, "\t\t\t\t\t"); @@ -664,16 +664,16 @@ static int hdmi_dbg_show(struct seq_file *s, void *data) DBGFS_DUMP("\n", HDMI_STA); hdmi_dbg_sta(s, hdmi_read(hdmi, HDMI_STA)); DBGFS_DUMP("", HDMI_ACTIVE_VID_XMIN); - seq_puts(s, "\t"); + seq_putc(s, '\t'); DBGFS_PRINT_INT("Xmin:", hdmi_read(hdmi, HDMI_ACTIVE_VID_XMIN)); DBGFS_DUMP("", HDMI_ACTIVE_VID_XMAX); - seq_puts(s, "\t"); + seq_putc(s, '\t'); DBGFS_PRINT_INT("Xmax:", hdmi_read(hdmi, HDMI_ACTIVE_VID_XMAX)); DBGFS_DUMP("", HDMI_ACTIVE_VID_YMIN); - seq_puts(s, "\t"); + seq_putc(s, '\t'); DBGFS_PRINT_INT("Ymin:", hdmi_read(hdmi, HDMI_ACTIVE_VID_YMIN)); DBGFS_DUMP("", HDMI_ACTIVE_VID_YMAX); - seq_puts(s, "\t"); + seq_putc(s, '\t'); DBGFS_PRINT_INT("Ymax:", hdmi_read(hdmi, HDMI_ACTIVE_VID_YMAX)); DBGFS_DUMP("", HDMI_SW_DI_CFG); hdmi_dbg_sw_di_cfg(s, hdmi_read(hdmi, HDMI_SW_DI_CFG)); @@ -712,8 +712,7 @@ static int hdmi_dbg_show(struct seq_file *s, void *data) DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD4, HDMI_IFRAME_SLOT_VENDOR); DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD5, HDMI_IFRAME_SLOT_VENDOR); DBGFS_DUMP_DI(HDMI_SW_DI_N_PKT_WORD6, HDMI_IFRAME_SLOT_VENDOR); - seq_puts(s, "\n"); - + seq_putc(s, '\n'); return 0; } diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index 66f843148ef7..372ea294da80 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -625,8 +625,7 @@ static int hqvdp_dbg_show(struct seq_file *s, void *data) hqvdp_dbg_dump_cmd(s, (struct sti_hqvdp_cmd *)virt); }
Re: [linux-sunxi] [PATCH v6 10/13] drm/sun4i: tcon: add support for V3s TCON
On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: > Allwinner V3s SoC features a TCON without channel 1. > > Add support for it. > > Signed-off-by: Icenowy Zheng Reviewed-by: Chen-Yu Tsai ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC
On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: > Allwinner V3s SoC features a "Display Engine 2.0" with only one TCON > which have RGB LCD output. Please also mention that it only has one mixer. For the subject, you could just say "Add device nodes for the display pipeline". > > Add device nodes for it as well as the TCON. > > Signed-off-by: Icenowy Zheng > --- > arch/arm/boot/dts/sun8i-v3s.dtsi | 87 > > 1 file changed, 87 insertions(+) > > diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi > b/arch/arm/boot/dts/sun8i-v3s.dtsi > index 71075969e5e6..0a895179d8ae 100644 > --- a/arch/arm/boot/dts/sun8i-v3s.dtsi > +++ b/arch/arm/boot/dts/sun8i-v3s.dtsi > @@ -41,6 +41,10 @@ > */ > > #include > +#include > +#include > +#include > +#include > > / { > #address-cells = <1>; > @@ -59,6 +63,12 @@ > }; > }; > > + de: display-engine { > + compatible = "allwinner,sun8i-v3s-display-engine"; > + allwinner,pipelines = <&de2_mixer0>; > + status = "disabled"; > + }; > + > timer { > compatible = "arm,armv7-timer"; > interrupts = IRQ_TYPE_LEVEL_LOW)>, > @@ -93,6 +103,83 @@ > #size-cells = <1>; > ranges; > > + de2_clocks: clock@100 { > + compatible = "allwinner,sun50i-h5-de2-clk"; I am a bit skeptical about this. Since the V3S only has one mixer, do the clocks for the second one even exist? > + reg = <0x0100 0x10>; > + clocks = <&ccu CLK_DE>, > +<&ccu CLK_BUS_DE>; > + clock-names = "mod", > + "bus"; > + resets = <&ccu RST_BUS_DE>; > + #clock-cells = <1>; > + #reset-cells = <1>; > + }; > + > + de2_mixer0: mixer@110 { > + compatible = "allwinner,sun8i-v3s-de2-mixer"; > + reg = <0x0110 0x10>; > + clocks = <&de2_clocks CLK_MIXER0>, > +<&de2_clocks CLK_BUS_MIXER0>; > + clock-names = "mod", > + "bus"; Nit: could you list the bus clock first? Regards ChenYu > + resets = <&de2_clocks RST_MIXER0>; > + assigned-clocks = <&de2_clocks CLK_MIXER0>; > + assigned-clock-rates = <15000>; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + mixer0_out: port@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + > + mixer0_out_tcon0: endpoint@0 { > + reg = <0>; > + remote-endpoint = > <&tcon0_in_mixer0>; > + }; > + }; > + }; > + }; > + > + tcon0: lcd-controller@1c0c000 { > + compatible = "allwinner,sun8i-v3s-tcon"; > + reg = <0x01c0c000 0x1000>; > + interrupts = ; > + clocks = <&ccu CLK_BUS_TCON0>, > +<&ccu CLK_TCON0>; > + clock-names = "ahb", > + "tcon-ch0"; > + clock-output-names = "tcon-pixel-clock"; > + resets = <&ccu RST_BUS_TCON0>; > + reset-names = "lcd"; > + status = "disabled"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + tcon0_in: port@0 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <0>; > + > + tcon0_in_mixer0: endpoint@0 { > + reg = <0>; > + remote-endpoint = > <&mixer0_out_tcon0>; > + }; > + }; > + > + tcon0_out: port@1 { > + #address-cells = <1>; > + #size-cells = <0>; > + reg = <1>; > + }; > +
Re: [linux-sunxi] [PATCH v6 07/13] drm/sun4i: add a Kconfig option for sun4i-backend
On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: > As sun4i-backend is now a dedicated module, add an Kconfig option for > it to make it optional, since some build may only use other engines. > > Signed-off-by: Icenowy Zheng > --- > Splited out patch. > > drivers/gpu/drm/sun4i/Kconfig | 10 ++ > drivers/gpu/drm/sun4i/Makefile | 2 +- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/sun4i/Kconfig b/drivers/gpu/drm/sun4i/Kconfig > index a4b357db8856..5a8227f37cc4 100644 > --- a/drivers/gpu/drm/sun4i/Kconfig > +++ b/drivers/gpu/drm/sun4i/Kconfig > @@ -12,3 +12,13 @@ config DRM_SUN4I > Choose this option if you have an Allwinner SoC with a > Display Engine. If M is selected the module will be called > sun4i-drm. > + > +config DRM_SUN4I_BACKEND > + tristate "Support for Allwinner A10 Display Engine Backend" > + depends on DRM_SUN4I > + default DRM_SUN4I > + help > + Choose this option if you have an Allwinner SoC with the > + original Allwinner Display Engine, which has a backend to > + do some alpha blending and feed graphics to TCON. If M is > + selected the module will be called sun4i-backend. > diff --git a/drivers/gpu/drm/sun4i/Makefile b/drivers/gpu/drm/sun4i/Makefile > index a251fb36c951..a08df56759e3 100644 > --- a/drivers/gpu/drm/sun4i/Makefile > +++ b/drivers/gpu/drm/sun4i/Makefile > @@ -9,6 +9,6 @@ sun4i-tcon-y += sun4i_crtc.o > sun4i-backend-y += sun4i_backend.o sun4i_layer.o > > obj-$(CONFIG_DRM_SUN4I)+= sun4i-drm.o sun4i-tcon.o > -obj-$(CONFIG_DRM_SUN4I)+= sun4i-backend.o > +obj-$(CONFIG_DRM_SUN4I_BACKEND)+= sun4i-backend.o > obj-$(CONFIG_DRM_SUN4I)+= sun6i_drc.o > obj-$(CONFIG_DRM_SUN4I)+= sun4i_tv.o Can you move sun4i-backend to the bottom in a separate section? The idea is to have a bunch of core or common stuff, then platform specific modules. Thanks ChenYu ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v6 05/13] drm/sun4i: abstract a engine type
On Fri, May 5, 2017 at 4:36 PM, wrote: > 在 2017-05-05 10:56,Chen-Yu Tsai 写道: >> >> On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: >>> >>> As we are going to add support for the Allwinner DE2 engine in sun4i-drm >>> driver, we will finally have two types of display engines -- the DE1 >>> backend and the DE2 mixer. They both do some display blending and feed >>> graphics data to TCON, so I choose to call them both "engine" here. >> >> >> These engines composite different layers into a final image which is >> then sent out to the TCONs. As such, "compositor" would be an accurate >> name. >> >> However, "engine" is OK, since Allwinner calls this stuff Display Engine >> 1.0 and 2.0. Hope there won't be a 3.0 ... >> >> Maybe you should note that in your commit message. That is justifies the >> name. >> >>> >>> Abstract the engine type to a new struct with an ops struct, which >>> contains >>> functions that should be called outside the engine-specified code (in >>> TCON, CRTC or TV Encoder code). >>> >>> Signed-off-by: Icenowy Zheng >>> --- >>> Changes in v6: >>> - Rebased on wens's multi-pipeline patchset. >>> - Split out Makefile changes. >>> Changes in v5: >>> - Really made a sunxi_engine struct type, and moved ops pointer >>> into it. >>> - Added checked ops wrappers. >>> - Changed the second parameter of layers_init from crtc to engine. >>> Changes in v4: >>> - Comments to tag the color correction functions as optional. >>> - Check before calling the optional functions. >>> - Change layers_init to satisfy new PATCH v4 04/11. >>> >>> drivers/gpu/drm/sun4i/sun4i_backend.c | 68 - >>> drivers/gpu/drm/sun4i/sun4i_backend.h | 17 +++--- >>> drivers/gpu/drm/sun4i/sun4i_crtc.c| 11 ++-- >>> drivers/gpu/drm/sun4i/sun4i_crtc.h| 4 +- >>> drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- >>> drivers/gpu/drm/sun4i/sun4i_drv.h | 2 +- >>> drivers/gpu/drm/sun4i/sun4i_layer.c | 8 +-- >>> drivers/gpu/drm/sun4i/sun4i_layer.h | 5 +- >>> drivers/gpu/drm/sun4i/sun4i_tcon.c| 36 ++- >>> drivers/gpu/drm/sun4i/sun4i_tv.c | 9 ++- >>> drivers/gpu/drm/sun4i/sunxi_engine.h | 112 >>> ++ >>> 11 files changed, 198 insertions(+), 76 deletions(-) >>> create mode 100644 drivers/gpu/drm/sun4i/sunxi_engine.h >>> [...] >>> diff --git a/drivers/gpu/drm/sun4i/sun4i_layer.h >>> b/drivers/gpu/drm/sun4i/sun4i_layer.h >>> index 5ea5c994d6ea..004b7cfe8ffb 100644 >>> --- a/drivers/gpu/drm/sun4i/sun4i_layer.h >>> +++ b/drivers/gpu/drm/sun4i/sun4i_layer.h >>> @@ -13,6 +13,8 @@ >>> #ifndef _SUN4I_LAYER_H_ >>> #define _SUN4I_LAYER_H_ >>> >>> +struct sunxi_engine; >>> + >>> struct sun4i_layer { >>> struct drm_planeplane; >>> struct sun4i_drv*drv; >>> @@ -27,6 +29,5 @@ plane_to_sun4i_layer(struct drm_plane *plane) >>> } >>> >>> struct drm_plane **sun4i_layers_init(struct drm_device *drm, >>> -struct sun4i_crtc *crtc); >>> - >>> +struct sunxi_engine *engine); >> >> >> Please keep the newline. >> >>> #endif /* _SUN4I_LAYER_H_ */ >>> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c >>> b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>> index 29fd829aa54c..c48135a10fda 100644 >>> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c >>> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c >>> @@ -26,12 +26,12 @@ >>> #include >>> #include >>> >>> -#include "sun4i_backend.h" >>> #include "sun4i_crtc.h" >>> #include "sun4i_dotclock.h" >>> #include "sun4i_drv.h" >>> #include "sun4i_rgb.h" >>> #include "sun4i_tcon.h" >>> +#include "sunxi_engine.h" >> >> >> Please keep the headers in alphabetical order. > > > sunxi is of course after sun4i. Sorry. My bad. :( ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v6 06/13] drm/sun4i: add a dedicated module for sun4i-backend and sun4i-layer
On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: > Currently the direct call from CRTC code to layer code has disappeared, > instead the layer's init function is called via the backend's ops. > > Add a dedicated module for sun4i-backend and sun4i-layer, and drop the > EXPORT_SYMBOL from backend code to layer code. > > Signed-off-by: Icenowy Zheng Reviewed-by: Chen-Yu Tsai ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC
在 2017-05-05 11:31,Chen-Yu Tsai 写道: On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: Allwinner V3s SoC features a "Display Engine 2.0" with only one TCON which have RGB LCD output. Please also mention that it only has one mixer. For the subject, you could just say "Add device nodes for the display pipeline". Add device nodes for it as well as the TCON. Signed-off-by: Icenowy Zheng --- arch/arm/boot/dts/sun8i-v3s.dtsi | 87 1 file changed, 87 insertions(+) diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi b/arch/arm/boot/dts/sun8i-v3s.dtsi index 71075969e5e6..0a895179d8ae 100644 --- a/arch/arm/boot/dts/sun8i-v3s.dtsi +++ b/arch/arm/boot/dts/sun8i-v3s.dtsi @@ -41,6 +41,10 @@ */ #include +#include +#include +#include +#include / { #address-cells = <1>; @@ -59,6 +63,12 @@ }; }; + de: display-engine { + compatible = "allwinner,sun8i-v3s-display-engine"; + allwinner,pipelines = <&de2_mixer0>; + status = "disabled"; + }; + timer { compatible = "arm,armv7-timer"; interrupts = IRQ_TYPE_LEVEL_LOW)>, @@ -93,6 +103,83 @@ #size-cells = <1>; ranges; + de2_clocks: clock@100 { + compatible = "allwinner,sun50i-h5-de2-clk"; I am a bit skeptical about this. Since the V3S only has one mixer, do the clocks for the second one even exist? It's described in the de_clock.c in the BSP source code, and in hardware these bits can be really set (although without clock output). So I use this compatible which has still the extra clocks. + reg = <0x0100 0x10>; + clocks = <&ccu CLK_DE>, +<&ccu CLK_BUS_DE>; + clock-names = "mod", + "bus"; + resets = <&ccu RST_BUS_DE>; + #clock-cells = <1>; + #reset-cells = <1>; + }; + + de2_mixer0: mixer@110 { + compatible = "allwinner,sun8i-v3s-de2-mixer"; + reg = <0x0110 0x10>; + clocks = <&de2_clocks CLK_MIXER0>, +<&de2_clocks CLK_BUS_MIXER0>; + clock-names = "mod", + "bus"; Nit: could you list the bus clock first? Regards ChenYu + resets = <&de2_clocks RST_MIXER0>; + assigned-clocks = <&de2_clocks CLK_MIXER0>; + assigned-clock-rates = <15000>; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + mixer0_out: port@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + + mixer0_out_tcon0: endpoint@0 { + reg = <0>; + remote-endpoint = <&tcon0_in_mixer0>; + }; + }; + }; + }; + + tcon0: lcd-controller@1c0c000 { + compatible = "allwinner,sun8i-v3s-tcon"; + reg = <0x01c0c000 0x1000>; + interrupts = ; + clocks = <&ccu CLK_BUS_TCON0>, +<&ccu CLK_TCON0>; + clock-names = "ahb", + "tcon-ch0"; + clock-output-names = "tcon-pixel-clock"; + resets = <&ccu RST_BUS_TCON0>; + reset-names = "lcd"; + status = "disabled"; + + ports { + #address-cells = <1>; + #size-cells = <0>; + + tcon0_in: port@0 { + #address-cells = <1>; + #size-cells = <0>; + reg = <0>; + + tcon0_in_mixer0: endpoint@0 { + reg = <0>; + remote-endpoint = <&mixer0_out_tcon0>; + }; + }; + + tcon0_out: port@1 { + #address-cells = <1>; + #size-cells = <0>; + reg = <1>; + }; +
Re: [linux-sunxi] [PATCH 04/13] drm/sun4i: return only planes for layers created
On Thu, May 4, 2017 at 7:41 PM, Icenowy Zheng wrote: > As we are going to add support for the Allwinner DE2 Mixer in sun4i-drm > driver, we will finally have two types of layers. > > Each layer is bound to a drm_plane that is CRTC-specific, so we create > them when initializing CRTC (calling sun4i_layers_init, which will be > generalized in next patch). The drm_plane's will be used when creating > CRTC, but the CRTC initialization code do not care other properties of > the layer, so we let the sun4i_layers_init function return drm_plane's > only. > > As we have no need to trace the layers after the CRTC is properly > created, we drop the layers pointer in sun4i_crtc struct. > > Doing these things makes the CRTC code independent to the type of layer Doing this uncouples the CRTC code from the type of layer. > (the sun4i_layers_init function name is still hardcoded and will be > changed in the next patch), so that we can finally gain support for the > mixer in DE2, which will has different layers. ... which has ... > > Signed-off-by: Icenowy Zheng Otherwise, Reviewed-by: Chen-Yu Tsai ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v6 08/13] drm/sun4i: add support for Allwinner DE2 mixers
于 2017年5月5日 GMT+08:00 下午8:36:18, Maxime Ripard 写到: >On Fri, May 05, 2017 at 12:50:51AM +0800, icen...@aosc.io wrote: >> > > +void sun8i_mixer_layer_enable(struct sun8i_mixer *mixer, >> > > +int layer, bool enable) >> > > +{ >> > > +u32 val; >> > > +/* Currently the first UI channel is used */ >> > > +int chan = mixer->cfg->vi_num; >> > > + >> > > +DRM_DEBUG_DRIVER("Enabling layer %d in channel %d\n", layer, >chan); >> > > + >> > > +if (enable) >> > > +val = SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN; >> > > +else >> > > +val = 0; >> > > + >> > > +regmap_update_bits(mixer->engine.regs, >> > > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR(chan, layer), >> > > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_EN, val); >> > > + >> > > +/* Set the alpha configuration */ >> > > +regmap_update_bits(mixer->engine.regs, >> > > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR(chan, layer), >> > > + >> > > SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_MASK, >> > > + >> > > SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MODE_DEF); >> > > +regmap_update_bits(mixer->engine.regs, >> > > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR(chan, layer), >> > > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_MASK, >> > > + SUN8I_MIXER_CHAN_UI_LAYER_ATTR_ALPHA_DEF); >> > > +} >> > >> > This one too. >> >> It's called from sun8i_layer.c, so it cannot be static. > >Fair enough. > >> > > +/* Set base coordinates */ >> > > +DRM_DEBUG_DRIVER("Layer coordinates X: %d Y: %d\n", >> > > + state->crtc_x, state->crtc_y); >> > > +regmap_write(mixer->engine.regs, >> > > + SUN8I_MIXER_CHAN_UI_LAYER_COORD(chan, layer), >> > > + SUN8I_MIXER_COORD(state->crtc_x, state->crtc_y)); >> > >> > X and Y are fixed point numbers. You want to keep only the higher >16 >> > bits there. >> >> Do you mean "lower 16 bits"? Thus should I (x & 0x) or ((u16)x) ? > >Nevermind, I got confused with src_x and src_y. > >> P.S. The negative coordinates are broken, how should I deal with it? >or >> is the coordinates promised to be not negative? > >Adjust the buffer base address, and use a shorter line. You have such >an example in the sun4i code. Are they these two lines: ``` paddr += (state->src_x >> 16) * bpp; paddr += (state->src_y >> 16) * fb->pitches[0]; ``` I think I copied them here, so I don't need to mind this problem any more, right? > >Maxime ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v6 03/13] dt-bindings: add bindings for DE2 on V3s SoC
On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: > Allwinner V3s SoC have a display engine which have a different pipeline > with older SoCs. > > Add document for it (new compatibles and the new "mixer" part). > > Signed-off-by: Icenowy Zheng > Acked-by: Rob Herring > --- > Changes in v4: > - Removed the refactor at TCON chapter. > Changes in v3: > - Remove the description of having a BE directly as allwinner,pipeline. > > .../bindings/display/sunxi/sun4i-drm.txt | 29 > -- > 1 file changed, 27 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > index 7acdbf14ae1c..33452884b96e 100644 > --- a/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > +++ b/Documentation/devicetree/bindings/display/sunxi/sun4i-drm.txt > @@ -41,6 +41,7 @@ Required properties: > * allwinner,sun6i-a31-tcon > * allwinner,sun6i-a31s-tcon > * allwinner,sun8i-a33-tcon > + * allwinner,sun8i-v3s-tcon > - reg: base address and size of memory-mapped region > - interrupts: interrupt associated to this IP > - clocks: phandles to the clocks feeding the TCON. Three are needed: > @@ -62,7 +63,7 @@ Required properties: >second the block connected to the TCON channel 1 (usually the TV >encoder) > > -On SoCs other than the A33, there is one more clock required: > +On SoCs other than the A33 and V3s, there is one more clock required: > - 'tcon-ch1': The clock driving the TCON channel 1 > > DRC > @@ -148,6 +149,26 @@ Required properties: >Documentation/devicetree/bindings/media/video-interfaces.txt. The >first port should be the input endpoints, the second one the outputs > > +Display Engine 2.0 Mixer > + > + > +The DE2 mixer have many functionalities, currently only layer blending is > +supported. > + > +Required properties: > + - compatible: value must be one of: > +* allwinner,sun8i-v3s-de2-mixer > + - reg: base address and size of the memory-mapped region. > + - clocks: phandles to the clocks feeding the frontend and backend > +* bus: the backend interface clock > +* ram: the backend DRAM clock You probably mean "mixer" here. > + - clock-names: the clock names mentioned above > + - resets: phandles to the reset controllers driving the backend And here. > + > +- ports: A ports node with endpoint definitions as defined in > + Documentation/devicetree/bindings/media/video-interfaces.txt. The > + first port should be the input endpoints, the second one the output > + > > Display Engine Pipeline > --- > @@ -162,9 +183,13 @@ Required properties: > * allwinner,sun6i-a31-display-engine > * allwinner,sun6i-a31s-display-engine > * allwinner,sun8i-a33-display-engine > +* allwinner,sun8i-v3s-display-engine > >- allwinner,pipelines: list of phandle to the display engine > -frontends available. > +pipeline entry point. For SoCs with original DE (currently > +all SoCs supported by display engine except V3s), this > +phandle should be a display frontend; for SoCs with DE2, > +this phandle should be a mixer. You could simplify this to "list of phandles to the display engine frontends (DE 1.0) or mixers (DE 2.0) available". Regards ChenYu > > Example: > > -- > 2.12.2 > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to linux-sunxi+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v6 11/13] ARM: dts: sun8i: add DE2 nodes for V3s SoC
于 2017年5月5日 GMT+08:00 下午8:30:35, Maxime Ripard 写到: >On Fri, May 05, 2017 at 04:53:43PM +0800, icen...@aosc.io wrote: >> > > + de2_clocks: clock@100 { >> > > + compatible = >"allwinner,sun50i-h5-de2-clk"; >> > >> > I am a bit skeptical about this. Since the V3S only has one mixer, >do >> > the clocks >> > for the second one even exist? >> >> It's described in the de_clock.c in the BSP source code, and in >hardware >> these bits can be really set (although without clock output). >> >> So I use this compatible which has still the extra clocks. > >If it's not usable, then it shouldn't be in the code, it's basically >dead code. Thus should we have one more DE2 CCU compatible without mixer1 clocks for V3s? ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] Re: [PATCH v6 08/13] drm/sun4i: add support for Allwinner DE2 mixers
On Fri, May 5, 2017 at 12:52 AM, wrote: > 在 2017-05-04 21:05,Maxime Ripard 写道: >> >> On Thu, May 04, 2017 at 07:48:53PM +0800, Icenowy Zheng wrote: >>> >>> Allwinner have a new "Display Engine 2.0" in their new SoCs, which comes >>> with mixers to do graphic processing and feed data to TCON, like the old >>> backends and frontends. >>> >>> Add support for the mixer on Allwinner V3s SoC; it's the simplest one. >>> >>> Currently a lot of functions are still missing -- more investigations >>> are needed to gain enough information for them. >>> >>> Signed-off-by: Icenowy Zheng >>> --- >>> Changes in v6: >>> - Rebased on wens's multi-pipeline patchset. >>> Changes in v5: >>> - Changed some code alignment. >>> - Request real 32-bit DMA (prepare for 64-bit SoCs). >>> Changes in v4: >>> - Killed some dead code according to Jernej. >>> >>> drivers/gpu/drm/sun4i/Kconfig | 10 + >>> drivers/gpu/drm/sun4i/Makefile | 3 + >>> drivers/gpu/drm/sun4i/sun8i_layer.c | 140 + >>> drivers/gpu/drm/sun4i/sun8i_layer.h | 36 >>> drivers/gpu/drm/sun4i/sun8i_mixer.c | 394 >>> >>> drivers/gpu/drm/sun4i/sun8i_mixer.h | 137 + >>> 6 files changed, 720 insertions(+) >>> create mode 100644 drivers/gpu/drm/sun4i/sun8i_layer.c >>> create mode 100644 drivers/gpu/drm/sun4i/sun8i_layer.h >>> create mode 100644 drivers/gpu/drm/sun4i/sun8i_mixer.c >>> create mode 100644 drivers/gpu/drm/sun4i/sun8i_mixer.h >>> [...] >>> diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c >>> b/drivers/gpu/drm/sun4i/sun8i_mixer.c >>> new file mode 100644 >>> index ..e216b84d5bb2 >>> --- /dev/null >>> +++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c >>> @@ -0,0 +1,394 @@ [...] >>> + list_add_tail(&mixer->engine.list, &drv->engine_list); >> >> >> You didn't call INIT_LIST_HEAD on that list. > > > So didn't the sun4i_backend driver... > > I think the mixer->engine.list only means an item in the > engine_list, and the drv->engine_list is initialized in the > sun4i_drv source code. I read [1] that if the item is subsequently added to a list, you could omit the INIT_LIST_HEAD call. Makes sense, though you have to be sure you aren't doing anything else with the list element. ChenYu [1] https://isis.poly.edu/kulesh/stuff/src/klist/ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 14/20] drm/sun4i: tcon: multiply the vtotal when not in interlace
在 2017-05-03 19:59,Maxime Ripard 写道: It appears that the total vertical resolution needs to be doubled when we're not in interlaced. Make sure that is the case. Signed-off-by: Maxime Ripard --- drivers/gpu/drm/sun4i/sun4i_tcon.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c index 0f91ec8a4b26..efa079c1a3f5 100644 --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c @@ -272,9 +272,9 @@ void sun4i_tcon1_mode_set(struct sun4i_tcon *tcon, /* Set vertical display timings */ bp = mode->crtc_vtotal - mode->crtc_vsync_start; DRM_DEBUG_DRIVER("Setting vertical total %d, backporch %d\n", -mode->vtotal, bp); +mode->crtc_vtotal, bp); regmap_write(tcon->regs, SUN4I_TCON1_BASIC4_REG, -SUN4I_TCON1_BASIC4_V_TOTAL(mode->vtotal) | +SUN4I_TCON1_BASIC4_V_TOTAL(mode->crtc_vtotal * 2) | For TVE the value should directly be mode->vtotal, but not mode->crtc_vtotal * 2. vtotal is 625 when PAL. crtc_vtotal is thus 312, but if we restore the vtotal value by doubling crtv_vtotal, we got 624, which will lead to instability of the image displayed. (the image will loop to go higher and then go lower, because wrong vtotal value) Tested on patched H3 TV encoder. I used a logic slightly changed from your v1 code: ``` val = mode->vtotal; if (!(mode->flags & DRM_MODE_FLAG_INTERLACE)) val = val * 2; regmap_write(tcon->regs, SUN4I_TCON1_BASIC4_REG, SUN4I_TCON1_BASIC4_V_TOTAL(val) | SUN4I_TCON1_BASIC4_V_BACKPORCH(bp)); ``` SUN4I_TCON1_BASIC4_V_BACKPORCH(bp)); /* Set Hsync and Vsync length */ ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v6 05/13] drm/sun4i: abstract a engine type
On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: > As we are going to add support for the Allwinner DE2 engine in sun4i-drm > driver, we will finally have two types of display engines -- the DE1 > backend and the DE2 mixer. They both do some display blending and feed > graphics data to TCON, so I choose to call them both "engine" here. These engines composite different layers into a final image which is then sent out to the TCONs. As such, "compositor" would be an accurate name. However, "engine" is OK, since Allwinner calls this stuff Display Engine 1.0 and 2.0. Hope there won't be a 3.0 ... Maybe you should note that in your commit message. That is justifies the name. > > Abstract the engine type to a new struct with an ops struct, which contains > functions that should be called outside the engine-specified code (in > TCON, CRTC or TV Encoder code). > > Signed-off-by: Icenowy Zheng > --- > Changes in v6: > - Rebased on wens's multi-pipeline patchset. > - Split out Makefile changes. > Changes in v5: > - Really made a sunxi_engine struct type, and moved ops pointer > into it. > - Added checked ops wrappers. > - Changed the second parameter of layers_init from crtc to engine. > Changes in v4: > - Comments to tag the color correction functions as optional. > - Check before calling the optional functions. > - Change layers_init to satisfy new PATCH v4 04/11. > > drivers/gpu/drm/sun4i/sun4i_backend.c | 68 - > drivers/gpu/drm/sun4i/sun4i_backend.h | 17 +++--- > drivers/gpu/drm/sun4i/sun4i_crtc.c| 11 ++-- > drivers/gpu/drm/sun4i/sun4i_crtc.h| 4 +- > drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- > drivers/gpu/drm/sun4i/sun4i_drv.h | 2 +- > drivers/gpu/drm/sun4i/sun4i_layer.c | 8 +-- > drivers/gpu/drm/sun4i/sun4i_layer.h | 5 +- > drivers/gpu/drm/sun4i/sun4i_tcon.c| 36 ++- > drivers/gpu/drm/sun4i/sun4i_tv.c | 9 ++- > drivers/gpu/drm/sun4i/sunxi_engine.h | 112 > ++ > 11 files changed, 198 insertions(+), 76 deletions(-) > create mode 100644 drivers/gpu/drm/sun4i/sunxi_engine.h > > diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c > b/drivers/gpu/drm/sun4i/sun4i_backend.c > index e53107418add..611cdcb9c182 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_backend.c > +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c > @@ -25,6 +25,8 @@ > > #include "sun4i_backend.h" > #include "sun4i_drv.h" > +#include "sun4i_layer.h" > +#include "sunxi_engine.h" > > static const u32 sunxi_rgb2yuv_coef[12] = { > 0x0107, 0x0204, 0x0064, 0x0108, > @@ -32,41 +34,38 @@ static const u32 sunxi_rgb2yuv_coef[12] = { > 0x01c1, 0x3e88, 0x3fb8, 0x0808 > }; > > -void sun4i_backend_apply_color_correction(struct sun4i_backend *backend) > +static void sun4i_backend_apply_color_correction(struct sunxi_engine *engine) > { > int i; > > DRM_DEBUG_DRIVER("Applying RGB to YUV color correction\n"); > > /* Set color correction */ > - regmap_write(backend->regs, SUN4I_BACKEND_OCCTL_REG, > + regmap_write(engine->regs, SUN4I_BACKEND_OCCTL_REG, > SUN4I_BACKEND_OCCTL_ENABLE); > > for (i = 0; i < 12; i++) > - regmap_write(backend->regs, SUN4I_BACKEND_OCRCOEF_REG(i), > + regmap_write(engine->regs, SUN4I_BACKEND_OCRCOEF_REG(i), > sunxi_rgb2yuv_coef[i]); > } > -EXPORT_SYMBOL(sun4i_backend_apply_color_correction); > > -void sun4i_backend_disable_color_correction(struct sun4i_backend *backend) > +static void sun4i_backend_disable_color_correction(struct sunxi_engine > *engine) > { > DRM_DEBUG_DRIVER("Disabling color correction\n"); > > /* Disable color correction */ > - regmap_update_bits(backend->regs, SUN4I_BACKEND_OCCTL_REG, > + regmap_update_bits(engine->regs, SUN4I_BACKEND_OCCTL_REG, >SUN4I_BACKEND_OCCTL_ENABLE, 0); > } > -EXPORT_SYMBOL(sun4i_backend_disable_color_correction); > > -void sun4i_backend_commit(struct sun4i_backend *backend) > +static void sun4i_backend_commit(struct sunxi_engine *engine) > { > DRM_DEBUG_DRIVER("Committing changes\n"); > > - regmap_write(backend->regs, SUN4I_BACKEND_REGBUFFCTL_REG, > + regmap_write(engine->regs, SUN4I_BACKEND_REGBUFFCTL_REG, > SUN4I_BACKEND_REGBUFFCTL_AUTOLOAD_DIS | > SUN4I_BACKEND_REGBUFFCTL_LOADCTL); > } > -EXPORT_SYMBOL(sun4i_backend_commit); > > void sun4i_backend_layer_enable(struct sun4i_backend *backend, > int layer, bool enable) > @@ -81,7 +80,7 @@ void sun4i_backend_layer_enable(struct sun4i_backend > *backend, > else > val = 0; > > - regmap_update_bits(backend->regs, SUN4I_BACKEND_MODCTL_REG, > + regmap_update_bits(backend->engine.regs, SUN4I_BACKEND_MODCTL_REG, >SUN4I_BACKEND_MODCTL_LAY_EN(layer), val)
Re: [linux-sunxi] [PATCH v6 05/13] drm/sun4i: abstract a engine type
在 2017-05-05 10:56,Chen-Yu Tsai 写道: On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: As we are going to add support for the Allwinner DE2 engine in sun4i-drm driver, we will finally have two types of display engines -- the DE1 backend and the DE2 mixer. They both do some display blending and feed graphics data to TCON, so I choose to call them both "engine" here. These engines composite different layers into a final image which is then sent out to the TCONs. As such, "compositor" would be an accurate name. However, "engine" is OK, since Allwinner calls this stuff Display Engine 1.0 and 2.0. Hope there won't be a 3.0 ... Maybe you should note that in your commit message. That is justifies the name. Abstract the engine type to a new struct with an ops struct, which contains functions that should be called outside the engine-specified code (in TCON, CRTC or TV Encoder code). Signed-off-by: Icenowy Zheng --- Changes in v6: - Rebased on wens's multi-pipeline patchset. - Split out Makefile changes. Changes in v5: - Really made a sunxi_engine struct type, and moved ops pointer into it. - Added checked ops wrappers. - Changed the second parameter of layers_init from crtc to engine. Changes in v4: - Comments to tag the color correction functions as optional. - Check before calling the optional functions. - Change layers_init to satisfy new PATCH v4 04/11. drivers/gpu/drm/sun4i/sun4i_backend.c | 68 - drivers/gpu/drm/sun4i/sun4i_backend.h | 17 +++--- drivers/gpu/drm/sun4i/sun4i_crtc.c| 11 ++-- drivers/gpu/drm/sun4i/sun4i_crtc.h| 4 +- drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +- drivers/gpu/drm/sun4i/sun4i_drv.h | 2 +- drivers/gpu/drm/sun4i/sun4i_layer.c | 8 +-- drivers/gpu/drm/sun4i/sun4i_layer.h | 5 +- drivers/gpu/drm/sun4i/sun4i_tcon.c| 36 ++- drivers/gpu/drm/sun4i/sun4i_tv.c | 9 ++- drivers/gpu/drm/sun4i/sunxi_engine.h | 112 ++ 11 files changed, 198 insertions(+), 76 deletions(-) create mode 100644 drivers/gpu/drm/sun4i/sunxi_engine.h diff --git a/drivers/gpu/drm/sun4i/sun4i_backend.c b/drivers/gpu/drm/sun4i/sun4i_backend.c index e53107418add..611cdcb9c182 100644 --- a/drivers/gpu/drm/sun4i/sun4i_backend.c +++ b/drivers/gpu/drm/sun4i/sun4i_backend.c @@ -25,6 +25,8 @@ #include "sun4i_backend.h" #include "sun4i_drv.h" +#include "sun4i_layer.h" +#include "sunxi_engine.h" static const u32 sunxi_rgb2yuv_coef[12] = { 0x0107, 0x0204, 0x0064, 0x0108, @@ -32,41 +34,38 @@ static const u32 sunxi_rgb2yuv_coef[12] = { 0x01c1, 0x3e88, 0x3fb8, 0x0808 }; -void sun4i_backend_apply_color_correction(struct sun4i_backend *backend) +static void sun4i_backend_apply_color_correction(struct sunxi_engine *engine) { int i; DRM_DEBUG_DRIVER("Applying RGB to YUV color correction\n"); /* Set color correction */ - regmap_write(backend->regs, SUN4I_BACKEND_OCCTL_REG, + regmap_write(engine->regs, SUN4I_BACKEND_OCCTL_REG, SUN4I_BACKEND_OCCTL_ENABLE); for (i = 0; i < 12; i++) - regmap_write(backend->regs, SUN4I_BACKEND_OCRCOEF_REG(i), + regmap_write(engine->regs, SUN4I_BACKEND_OCRCOEF_REG(i), sunxi_rgb2yuv_coef[i]); } -EXPORT_SYMBOL(sun4i_backend_apply_color_correction); -void sun4i_backend_disable_color_correction(struct sun4i_backend *backend) +static void sun4i_backend_disable_color_correction(struct sunxi_engine *engine) { DRM_DEBUG_DRIVER("Disabling color correction\n"); /* Disable color correction */ - regmap_update_bits(backend->regs, SUN4I_BACKEND_OCCTL_REG, + regmap_update_bits(engine->regs, SUN4I_BACKEND_OCCTL_REG, SUN4I_BACKEND_OCCTL_ENABLE, 0); } -EXPORT_SYMBOL(sun4i_backend_disable_color_correction); -void sun4i_backend_commit(struct sun4i_backend *backend) +static void sun4i_backend_commit(struct sunxi_engine *engine) { DRM_DEBUG_DRIVER("Committing changes\n"); - regmap_write(backend->regs, SUN4I_BACKEND_REGBUFFCTL_REG, + regmap_write(engine->regs, SUN4I_BACKEND_REGBUFFCTL_REG, SUN4I_BACKEND_REGBUFFCTL_AUTOLOAD_DIS | SUN4I_BACKEND_REGBUFFCTL_LOADCTL); } -EXPORT_SYMBOL(sun4i_backend_commit); void sun4i_backend_layer_enable(struct sun4i_backend *backend, int layer, bool enable) @@ -81,7 +80,7 @@ void sun4i_backend_layer_enable(struct sun4i_backend *backend, else val = 0; - regmap_update_bits(backend->regs, SUN4I_BACKEND_MODCTL_REG, + regmap_update_bits(backend->engine.regs, SUN4I_BACKEND_MODCTL_REG, SUN4I_BACKEND_MODCTL_LAY_EN(layer), val); } EXPORT_SYMBOL(sun4i_backend_layer_enable); @@ -144,27 +143,28 @@ int sun4i_backend_update_layer_coord(struct sun4i_backend *backend,
[PATCH 3/5] drm/sti: Fix a typo in a comment line
From: Markus Elfring Date: Fri, 5 May 2017 15:30:44 +0200 Add a missing character in this description for a data structure. Signed-off-by: Markus Elfring --- drivers/gpu/drm/sti/sti_cursor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sti/sti_cursor.c b/drivers/gpu/drm/sti/sti_cursor.c index cd35b9d9de26..5b3a41f74f21 100644 --- a/drivers/gpu/drm/sti/sti_cursor.c +++ b/drivers/gpu/drm/sti/sti_cursor.c @@ -33,7 +33,7 @@ #define STI_CURS_MAX_SIZE 128 /* - * pixmap dma buffer stucture + * pixmap dma buffer structure * * @paddr: physical address * @size: buffer size -- 2.12.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/5] drm/sti: Fix typos in a comment line
From: Markus Elfring Date: Fri, 5 May 2017 15:32:08 +0200 Adjust this description for a function call. Signed-off-by: Markus Elfring --- drivers/gpu/drm/sti/sti_tvout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c index 63b982048743..8959fcc743a8 100644 --- a/drivers/gpu/drm/sti/sti_tvout.c +++ b/drivers/gpu/drm/sti/sti_tvout.c @@ -846,7 +846,7 @@ static int sti_tvout_probe(struct platform_device *pdev) tvout->dev = dev; - /* get Memory ressources */ + /* get memory resources */ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "tvout-reg"); if (!res) { DRM_ERROR("Invalid glue resource\n"); -- 2.12.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [linux-sunxi] [PATCH v6 12/13] ARM: dts: sun8i: add pinmux for LCD pins of V3s SoC
On Thu, May 4, 2017 at 7:48 PM, Icenowy Zheng wrote: > Allwinner V3s SoC features a set of pins that have functionality of RGB > LCD, the pins are at different pin ban than other SoCs. > > Add pinctrl node for them. > > Signed-off-by: Icenowy Zheng > --- > arch/arm/boot/dts/sun8i-v3s.dtsi | 9 + > 1 file changed, 9 insertions(+) > > diff --git a/arch/arm/boot/dts/sun8i-v3s.dtsi > b/arch/arm/boot/dts/sun8i-v3s.dtsi > index 0a895179d8ae..a37d68b227bc 100644 > --- a/arch/arm/boot/dts/sun8i-v3s.dtsi > +++ b/arch/arm/boot/dts/sun8i-v3s.dtsi > @@ -297,6 +297,15 @@ > function = "i2c0"; > }; > > + lcd_rgb666_pins: lcd_rgb666@0 { Drop the trailing "@0". Otherwise, Acked-by: Chen-Yu Tsai > + pins = "PE0", "PE1", "PE2", "PE3", "PE4", > + "PE5", "PE6", "PE7", "PE8", "PE9", > + "PE10", "PE11", "PE12", "PE13", "PE14", > + "PE15", "PE16", "PE17", "PE18", "PE19", > + "PE23", "PE24"; > + function = "lcd"; > + }; > + > uart0_pins_a: uart0@0 { > pins = "PB8", "PB9"; > function = "uart0"; > -- > 2.12.2 > > -- > You received this message because you are subscribed to the Google Groups > "linux-sunxi" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to linux-sunxi+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 5/5] drm/sti: Adjust two checks for null pointers in sti_hqvdp_probe()
From: Markus Elfring Date: Fri, 5 May 2017 15:33:19 +0200 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The script “checkpatch.pl” pointed information out like the following. Comparison to NULL could be written !… Thus fix the affected source code places. Signed-off-by: Markus Elfring --- drivers/gpu/drm/sti/sti_hqvdp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index 372ea294da80..a1c161f77804 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -1356,12 +1356,12 @@ static int sti_hqvdp_probe(struct platform_device *pdev) /* Get Memory resources */ res = platform_get_resource(pdev, IORESOURCE_MEM, 0); - if (res == NULL) { + if (!res) { DRM_ERROR("Get memory resource failed\n"); return -ENXIO; } hqvdp->regs = devm_ioremap(dev, res->start, resource_size(res)); - if (hqvdp->regs == NULL) { + if (!hqvdp->regs) { DRM_ERROR("Register mapping failed\n"); return -ENXIO; } -- 2.12.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/5] GPU-DRM-STI: Fine-tuning for some function implementations
On Fri, May 05, 2017 at 03:50:49PM +0200, SF Markus Elfring wrote: > From: Markus Elfring > Date: Fri, 5 May 2017 15:45:45 +0200 > > A few update suggestions were taken into account > from static source code analysis. > Hi Markus, It seems like you're back to submitting cocci patches again :) We left off in September asking you to contact the list before sending these types of patches, and to ensure they were properly tested [1]. I don't see any evidence that either of these things have been done. I don't want to waste your time by ignoring your patches, so please ensure that your patches provide value and that they are tested. Sean [1]- http://www.spinics.net/lists/linux-kernel-janitors/msg28134.html > Markus Elfring (5): > Reduce function calls for sequence output at five places > Replace 17 seq_puts() calls by seq_putc() > Fix a typo in a comment line > Fix typos in a comment line > Adjust two checks for null pointers in sti_hqvdp_probe() > > drivers/gpu/drm/sti/sti_cursor.c | 5 ++--- > drivers/gpu/drm/sti/sti_dvo.c| 3 +-- > drivers/gpu/drm/sti/sti_gdp.c| 3 +-- > drivers/gpu/drm/sti/sti_hda.c| 9 +++-- > drivers/gpu/drm/sti/sti_hdmi.c | 23 ++- > drivers/gpu/drm/sti/sti_hqvdp.c | 7 +++ > drivers/gpu/drm/sti/sti_mixer.c | 3 +-- > drivers/gpu/drm/sti/sti_tvout.c | 7 +++ > drivers/gpu/drm/sti/sti_vid.c| 5 ++--- > 9 files changed, 26 insertions(+), 39 deletions(-) > > -- > 2.12.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane
On Fri, May 5, 2017 at 12:45 PM, Ville Syrjälä wrote: > On Fri, May 05, 2017 at 08:58:17AM +0200, Daniel Vetter wrote: >> On Thu, May 04, 2017 at 05:51:45PM +0300, Ville Syrjälä wrote: >> > On Thu, May 04, 2017 at 03:22:45PM +0200, Daniel Vetter wrote: >> > > On Thu, May 04, 2017 at 10:14:26AM +0300, Jyri Sarha wrote: >> > > > Add standard optinal property blobs for YCbCr to RGB conversion CSC >> > > > matrix and YCbCr preoffset vector in DRM planes. New enums are defined >> > > > to YCBCR_ENCODING property to activate the CSC and preoffset >> > > > properties. For simplicity the new properties are stored in the >> > > > drm_plane object, because the YCBCR_ENCODING is already there. The >> > > > blob contents are defined in the uapi/drm/drm_mode.h header. >> > > > >> > > > Signed-off-by: Jyri Sarha >> > > >> > > Not sure we want to make this yuv2rgb specific, plenty of planes have >> > > generic degamma/offset, csc, gamme/offset hw. I think what we want >> > > instead >> > > is the generic csc support, plus a ycbcr2rgb mode of "bypass". >> > >> > My idea is to not expose this. And instead just expose a normal >> > ctm, and then just refuse any combination of YCbCr->RGB/degamma/ctm >> > that can't be done by the hw. >> >> But that means we'd need to have a bit-perfect match with a canonical >> conversion matrix (even if maybe the hw has a rounding bug and implements >> something slightly different than the standard). > > I don't see what this has to do with bit correctness. Different hw is > likely to use different precision for this stuff anyway, so I'm 100% > sure we'll not get the same results from all hardware. And really, > getting 100% same output doesn't seem all that important here > anyway. > > Anyway, all I'm saying is that we should expose a standard pipeline: > ycbcr->rgb -> degamma -> ctm -> gamma > > And each driver can then expose whatever parts of that they wish. If you > want to expoe a programmable matrix you expose it as the ctm. I don't > think having a programmable matrix as both ycbcr->rgb and ctm would > really benefit us. Ok, that makes sense. I was somehow assuming you're suggesting we only expose a colors -> degamma -> ctm -> gamma pipeline and drivers with only fixed function ycbcr2rbg conversion would then need to infer which exact btsomethingsomething standard it should pick. The above makes perfect sense, sorry for the confusion. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: Proposal for RandR version 1.6, Leases and EDID-based output grabs
Pekka Paalanen writes: > I disagree on the details, more below. > Such a RandR request is something I would not like to have to replicate > on Wayland. The display server contains the policy, it should not just > expose everything up for grabs. This is a fundamental difference > between X11 and Wayland architectures, and I think the output > information database should support both ways. Sounds like a good idea; if you want to work on how this should appear in Wayland, that would be great as it would provide two implementations, rather than just one. > I disagree. Wayland will likely have a special protocol extension > exclusively for advertising HMDs, so the display server will need to > know which ones are HMDs and which one are not, regardless whether the > desktop is extended there or not. This extension could also offer the > DRM leasing capabilities. What I was thinking that a display which the window system cannot drive natively should probably just be ignored entirely. An HMD which can natively handle a desktop (such as the PSVR system) might well be advertised as 'desktop capable', even though it is an HMD. However, I also think you're right -- while the window system can't deal with it *today*, that doesn't mean the window system won't be able to do that in the future. Hrm. I think the important distinction here is that the user installed an application which is designed to talk directly to this display. From that, we should probably infer that the user would like to use that application with the display. > Having the window manager know what is a HMD and which client is active > on it will let it make better management decisions and even allow > switching between VR apps, or temporarily switching from VR app to the > desktop when supported, and back. The window system will know when outputs are leased to another client, so it can tell when to bring them back to the desktop. In the PSVR instance, you'd list the device as 'desktop', but still allow leasing. When no lease was active, the desktop could extend into the PSVR environment. When the custom application started, it would request a lease at which time the desktop would move off of the PSVR. >> Thanks again; I'm sensing that a simple 'ignore this monitor for the >> desktop' might suffice for now, but that we'll end up wanting something >> more complicated in the future. I think the key is to try and avoid >> making that harder by what we do now, but I don't think we should be >> trying to implement a larger solution too soon. > > Yes, that I agree with. I guess that's the question -- is a simple command to ignore a monitor for purposes of the desktop sufficient for now? Or do we need to worry about a possible future in which the window system implements native HMD support? > Ultimately I would envision an output device database describing > what kind of a device it is (a normal monitor, a video projector, a > HMD, a TV, ...) and then the software that implements the desktop or UI > policy will decide how that will be used. Some policy examples: > > - A projector: do not extend desktop UI, but have the output normally > available, so one can put regular windows there (presentation > software). Turned on by default. > > - A HMD: Do not extend desktop, do not expose to normal apps, keep it > off until special request. > > - A HMD with cinematic mode support: Extend desktop, turn on by > default, allow special HMD requests. > > - A TV: Try to disable overscan or try to compensate for it by > default. Those all sound useful. I wonder how much we might be able to guess from EDID info and whether we want to programmatically do some of this (especially the TV thing; that's really annoying :-). In any case, a problem for the future. > I would suggest to have a "output device type" attribute in the > database, and support only one value for now: "HMD". Then all display > servers can encode the policy to hide all HMDs by default. > Implementationwise it is no different from your idea, but separating > device description from usage policy would be a good thing in my > opinion. You can still document suggested policies in the spec if you > wish, only the wording will be more of a recommendation than a > requirement. The only issue here is that now we're encoding policy in code, which is hard to change for the average user, rather than in a configuration file, which is easy to change. However, as we've defined it, these files are installed by the system and it would be nice if they weren't expected to be overridden by the user, so encoding policy there is almost worse than in the code. Hrm. How about we adopt your design and encode the device type in the file, provide a fixed policy in the window system for now and consider the possibility of changing the window system in the future to support more advanced policy mechanisms. I don't think that shuts any doors permanently. -- -keith signature.asc Description: PGP signature ___
Re: [PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane
Hi Jyri, On Thursday 04 May 2017 18:23:21 Jyri Sarha wrote: > On 05/04/17 17:51, Ville Syrjälä wrote: > > On Thu, May 04, 2017 at 03:22:45PM +0200, Daniel Vetter wrote: > >> On Thu, May 04, 2017 at 10:14:26AM +0300, Jyri Sarha wrote: > >>> Add standard optinal property blobs for YCbCr to RGB conversion CSC > >>> matrix and YCbCr preoffset vector in DRM planes. New enums are defined > >>> to YCBCR_ENCODING property to activate the CSC and preoffset > >>> properties. For simplicity the new properties are stored in the > >>> drm_plane object, because the YCBCR_ENCODING is already there. The > >>> blob contents are defined in the uapi/drm/drm_mode.h header. > >>> > >>> Signed-off-by: Jyri Sarha > >> > >> Not sure we want to make this yuv2rgb specific, plenty of planes have > >> generic degamma/offset, csc, gamme/offset hw. I think what we want > >> instead is the generic csc support, plus a ycbcr2rgb mode of "bypass". > > > > My idea is to not expose this. And instead just expose a normal > > ctm, and then just refuse any combination of YCbCr->RGB/degamma/ctm > > that can't be done by the hw. > > So, pulling the suggestions together the properties would now look like > this: > > YCBCR_RANGE > - Full > - Limited > > YCBCR_ENCODING > - BT.601 > - BT.709 > - BT.2020 > > CTM (blob) > > And all these properties could be added to individual planes. I'll try > to come up with a new patch to add simple enum properties for > YCBCR_PREOFFSET and YCBCR_ENCODING for planes. Should we need any more > enum values for those? > > The only functional thing that is missing from the above proposal is the > possibility to define an arbitrary YCbCr preoffset vector, but that can > be added later if there ever is any real need. > > The other thing that could be added for special purposes would be adding > an "Identity" enum to YCBCR_ENCODING, that would allow setting the YCbCr > CSC trough CTM without any driver level multiplication getting in way, > but that can also be added if there is a need. I'm a bit concerned about this. The YCBCR_ENCODING property specifies how the content of the framebuffer is encoded. If I understand correctly, you're proposing adding an enumeration value that tells the driver not to try to be clever and multiply the CTM matrix by the CSC matrix corresponding to the encoding. That would probably work in most cases, but it would combine two pieces of information in a single property. The driver would then lose the knowledge of how the plane framebuffer is encoded. Couldn't there be cases where that knowledge is needed for other purposes than picking the right CSC matrix ? If so, it might be better to always set the YCBCR_ENCODING property to the actual encoding, and have another property to tell the driver to skip multiplication by the CSC matrix. Or could that be conveyed through the CTM blob property ? Some kind of flag that would essentially tell that the CTM matrix has been pre-multiplied already ? Before I forget, how do you plan to handle backward compatibility with userspace that won't set the YCBCR_ENCODING property ? Is that done by picking a driver-specific default value ? Do you think there would be a need for drivers to know that the property has not been set ? -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next for 4.13
Hi Dave, Here's the first pull request for 4.13 from misc-next. It's surprisingly small given that we had an extra week of feature freeze. The highlights are below, and aside from these we had miscellaneous (heh) fixes sprinkled throughout. A bit of administrivia for you: We now have a standard template for misc pulls which break out UAPI/Cross- subsystem/Core/Driver changes. Aside from hopefully making it easier for you, this will remind me to explicitly think about the different changes I should call out. Secondly, I've Cc'd the authors of the sets I've summarized below. This should provide an extra layer of protection against the misc maintainer summarizing incorrectly (which is rightly a concern of yours). Feedback is most welcome. drm-misc-next-2017-05-05: UAPI Changes: - Return -ENODEV instead of -ENXIO when creating cma fb w/o valid gem (Daniel) Core Changes: - Add Laurent as bridge reviewer and Andrzej as bridge maintainer (Archit) - Maintain new STM driver through -misc (Yannick) - Misc doc improvements (as is tradition) (Daniel) Driver Changes: - Add out-fence support to vc4 V3D rendering (Eric) - Add support for stm32f429 display hw and am-480272h3tmqw-t01h panel (Yannick) - Remove 256MB cma limit from vc4 (Eric) - Disable dw-hdmi audio when inactive, instead of always enabled (Romain) - Add support for VGA to the ZTE driver (Shawn) Cc: Archit Taneja Cc: Eric Anholt Cc: Yannick Fertre Cc: Daniel Vetter Cc: Romain Perier Cc: Navare, Manasi D Cc: Shawn Guo Happy weekend, Sean The following changes since commit 8b03d1ed2c43a2ba5ef3381322ee4515b97381bf: Merge branch 'linux-4.12' of git://github.com/skeggsb/linux into drm-next (2017-05-02 04:46:01 +1000) are available in the git repository at: git://anongit.freedesktop.org/git/drm-misc tags/drm-misc-next-2017-05-05 for you to fetch changes up to 3c390df3337e54130e4b511ea3bbb868643cc5ea: Merge tag 'drm-for-v4.12' of git://people.freedesktop.org/~airlied/linux into drm-misc-next (2017-05-04 08:42:49 -0400) UAPI Changes: - Return -ENODEV instead of -ENXIO when creating cma fb w/o valid gem (Daniel) Core Changes: - Add Laurent as bridge reviewer and Andrzej as bridge maintainer (Archit) - Maintain new STM driver through -misc (Yannick) - Misc doc improvements (as is tradition) (Daniel) Driver Changes: - Add out-fence support to vc4 V3D rendering (Eric) - Add support for stm32f429 display hw and am-480272h3tmqw-t01h panel (Yannick) - Remove 256MB cma limit from vc4 (Eric) - Disable dw-hdmi audio when inactive, instead of always enabled (Romain) - Add support for VGA to the ZTE driver (Shawn) Cc: Archit Taneja Cc: Eric Anholt Cc: Yannick Fertre Cc: Daniel Vetter Cc: Romain Perier Cc: Navare, Manasi D Cc: Shawn Guo Andres Rodriguez (1): dma-buf: avoid scheduling on fence status query v2 Archit Taneja (1): MAINTAINERS: Update maintainers/reviewers for bridge drivers Boris Brezillon (1): drm/vc4: Add runtime PM support to the HDMI encoder driver Chris Wilson (1): drm/mm: Split up long running selftests with cond_resched() Clint Taylor (1): drm/cec: Add CEC over Aux register definitions Colin Ian King (1): drm: fix spelling mistake: "committing" Daniel Vetter (3): drm/doc: Fix missing @ctx documentation drm/doc: Interlink color manager docs better drm/cma-helper: Return ENOENT for "no such gem obj" Dave Airlie (1): sync_file: get rid of internal reference count. Eric Anholt (4): drm/vc4: Expose dma-buf fences for V3D rendering. drm/cma: Fix recent regression of mmap() in the MMU case. drm/vc4: Fix refcounting of runtime PM get if it errors out. drm/vc4: Allow using more than 256MB of CMA memory. Gustavo Padovan (1): drm/atomic: fix doc to use new name for commit types Jeffy Chen (2): drm/rockchip: Set line flag config register in vop_crtc_enable drm/rockchip: analogix_dp: Remove unused check and variables Jyri Sarha (2): drm: drm_color_mgmt.h needs struct drm_crtc declaration drm: Make drm_atomic_replace_property_blob_from_id() more generic Liu Ying (1): drm/bridge: sii902x: Add missing \n to the end of some dev_err messages Navare, Manasi D (1): drm: Add DPCD definitions for DP 1.4 DSC feature Romain Perier (2): drm: dw-hdmi: add specific I2S and AHB functions for stream handling drm: dw-hdmi: gate audio clock from the I2S enablement callbacks Sean Paul (1): Merge tag 'drm-for-v4.12' of git://people.freedesktop.org/~airlied/linux into drm-misc-next Shawn Guo (4): drm: zte: do not enable clock auto-gating by default drm: zte: move CSC register definitions into a common header dt-bindings: display: add support for ZTE VGA device drm: zte: add VGA driver support Yannick Fertre (5): drm/cma: Update DEFIN
Re: GPU-DRM-STI: Fine-tuning for some function implementations
> It seems like you're back to submitting cocci patches again :) My contribution activities are varying also for Linux software over time. ;-) The corresponding source code search patterns get different popularity. > I don't want to waste your time by ignoring your patches, so please ensure > that > your patches provide value and that they are tested. Which benchmarks and system tests would you find representative for this patch series? How do you think generally about the proposed change possibilities? Regards, Markus ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next-fixes for 4.12
Hi Dave, Many apologies for missing your initial PR. Just one patch to fix up the panel for HP zBook 17 G2. drm-misc-next-fixes-2017-05-05: Core Changes: - Add quirk for LGD 764 panel to default 10bpc (Mario) Cc: Mario Kleiner Cheers, Sean The following changes since commit 8b03d1ed2c43a2ba5ef3381322ee4515b97381bf: Merge branch 'linux-4.12' of git://github.com/skeggsb/linux into drm-next (2017-05-02 04:46:01 +1000) are available in the git repository at: git://anongit.freedesktop.org/git/drm-misc tags/drm-misc-next-fixes-2017-05-05 for you to fetch changes up to e345da82bd6bdfa8492f80b3ce4370acfd868d95: drm/edid: Add 10 bpc quirk for LGD 764 panel in HP zBook 17 G2 (2017-05-02 10:37:45 +0200) Core Changes: - Add quirk for LGD 764 panel to default 10bpc (Mario) Cc: Mario Kleiner Mario Kleiner (1): drm/edid: Add 10 bpc quirk for LGD 764 panel in HP zBook 17 G2 drivers/gpu/drm/drm_edid.c | 8 1 file changed, 8 insertions(+) -- Sean Paul, Software Engineer, Google / Chromium OS ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100937] Mesa fails to build with GCC 4.8
https://bugs.freedesktop.org/show_bug.cgi?id=100937 Samuel Pitoiset changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #3 from Samuel Pitoiset --- Fixed with: https://cgit.freedesktop.org/mesa/mesa/commit/?id=485ece83aceb3a35792efbc6fe2bca57ba46c04a -- 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 RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane
On Fri, May 5, 2017 at 4:36 PM, Laurent Pinchart wrote: > I'm a bit concerned about this. The YCBCR_ENCODING property specifies how the > content of the framebuffer is encoded. If I understand correctly, you're > proposing adding an enumeration value that tells the driver not to try to be > clever and multiply the CTM matrix by the CSC matrix corresponding to the > encoding. That would probably work in most cases, but it would combine two > pieces of information in a single property. The driver would then lose the > knowledge of how the plane framebuffer is encoded. Couldn't there be cases > where that knowledge is needed for other purposes than picking the right CSC > matrix ? If so, it might be better to always set the YCBCR_ENCODING property > to the actual encoding, and have another property to tell the driver to skip > multiplication by the CSC matrix. Or could that be conveyed through the CTM > blob property ? Some kind of flag that would essentially tell that the CTM > matrix has been pre-multiplied already ? > > Before I forget, how do you plan to handle backward compatibility with > userspace that won't set the YCBCR_ENCODING property ? Is that done by picking > a driver-specific default value ? Do you think there would be a need for > drivers to know that the property has not been set ? Where do we need this? Afaik the encoding is to spec the yuv2rbg conversion function, and that's it. But I'm fairly ignorant about video and yuv and all these things. so does this specify something else? If not, I don't see any possibilities that someone could retrofit more meaning onto these conversion rules. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH RFC v2 2/2] drm: Add YCBCR_DECODE_CSC and YCBCR_CSC_PREOFFSET properties to drm_plane
On Fri, May 05, 2017 at 07:17:43PM +0200, Daniel Vetter wrote: > On Fri, May 5, 2017 at 4:36 PM, Laurent Pinchart > wrote: > > I'm a bit concerned about this. The YCBCR_ENCODING property specifies how > > the > > content of the framebuffer is encoded. If I understand correctly, you're > > proposing adding an enumeration value that tells the driver not to try to be > > clever and multiply the CTM matrix by the CSC matrix corresponding to the > > encoding. That would probably work in most cases, but it would combine two > > pieces of information in a single property. The driver would then lose the > > knowledge of how the plane framebuffer is encoded. Couldn't there be cases > > where that knowledge is needed for other purposes than picking the right CSC > > matrix ? If so, it might be better to always set the YCBCR_ENCODING property > > to the actual encoding, and have another property to tell the driver to skip > > multiplication by the CSC matrix. Or could that be conveyed through the CTM > > blob property ? Some kind of flag that would essentially tell that the CTM > > matrix has been pre-multiplied already ? > > > > Before I forget, how do you plan to handle backward compatibility with > > userspace that won't set the YCBCR_ENCODING property ? Is that done by > > picking > > a driver-specific default value ? Do you think there would be a need for > > drivers to know that the property has not been set ? > > Where do we need this? Afaik the encoding is to spec the yuv2rbg > conversion function, and that's it. But I'm fairly ignorant about > video and yuv and all these things. so does this specify something > else? If not, I don't see any possibilities that someone could > retrofit more meaning onto these conversion rules. The question, I believe, is how do we deal with existing userspace that doesn't know about this knob. I think BT.709 is probably what we should use as the default since I'm expecting that would match most non-ancient source material out there. i915 currently uses BT.601 for no particular good reason, but I'm very happy to change that to BT.709. I think for the old video overlay (which isn't exposed as a plane currently) we actually default to BT.709. The only problatic case is when the hardware can't to BT.709 but I'm not sure if that's relevant for any currently supported hardware. I guess we could have the core pick the default based on some priority list eg. BT.709->BT.601->BT.2020. The other option of course being that we just let each driver pick their own default. I guess that might make sense if there's some userspace already out there that expects eg. BT.601. But frankly the difference between 601 and 709 is small enough that I wouldn't expect anyone to scream too loudly if we end up changing the default for someone. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH RESEND] drm/i915: Fix pipe/transcoder enum mismatches
On Fri, May 05, 2017 at 10:26:36AM -0700, Matthias Kaehlcke wrote: > El Thu, Apr 20, 2017 at 02:56:05PM -0700 Matthias Kaehlcke ha dit: > > > In several instances the driver passes an 'enum pipe' value to a > > function expecting an 'enum transcoder' and viceversa. Since PIPE_x and > > TRANSCODER_x have the same values this doesn't cause functional > > problems. Still it is incorrect and causes clang to generate warnings > > like this: > > > > drivers/gpu/drm/i915/intel_display.c:1844:34: warning: implicit > > conversion from enumeration type 'enum transcoder' to different > > enumeration type 'enum pipe' [-Wenum-conversion] > > assert_fdi_rx_enabled(dev_priv, TRANSCODER_A); > > > > Change the code to pass values of the type expected by the callee. > > > > Signed-off-by: Matthias Kaehlcke > > --- > > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > > drivers/gpu/drm/i915/intel_dp.c | 6 -- > > drivers/gpu/drm/i915/intel_hdmi.c| 6 -- > > drivers/gpu/drm/i915/intel_sdvo.c| 6 -- > > 4 files changed, 14 insertions(+), 8 deletions(-) > > Ping, any comments on this patch? I'm not convinced the patch is making things any better really. To fix this really properly, I think we'd need to introduce a new enum pch_transcoder and thus avoid the confusion of which type of transcoder we're talking about. Currently most places expect an enum pipe when dealing with PCH transcoders, and enum transcoder when dealing with CPU transcoders. But there are some exceptions of course. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/2] drm: Add writeback connector type
From: Brian Starkey Writeback connectors represent writeback engines which can write the CRTC output to a memory framebuffer. Add a writeback connector type and related support functions. Drivers should initialize a writeback connector with drm_writeback_connector_init() which takes care of setting up all the writeback-specific details on top of the normal functionality of drm_connector_init(). Writeback connectors have a WRITEBACK_FB_ID property, used to set the output framebuffer, and a WRITEBACK_PIXEL_FORMATS blob used to expose the supported writeback formats to userspace. When a framebuffer is attached to a writeback connector with the WRITEBACK_FB_ID property, it is used only once (for the commit in which it was included), and userspace can never read back the value of WRITEBACK_FB_ID. WRITEBACK_FB_ID can only be set if the connector is attached to a CRTC. Changes since v1: - Added drm_writeback.c + documentation - Added helper to initialize writeback connector in one go - Added core checks - Squashed into a single commit - Dropped the client cap - Writeback framebuffers are no longer persistent Changes since v2: Daniel Vetter: - Subclass drm_connector to drm_writeback_connector - Relax check to allow CRTC to be set without an FB - Add some writeback_ prefixes - Drop PIXEL_FORMATS_SIZE property, as it was unnecessary Gustavo Padovan: - Add drm_writeback_job to handle writeback signalling centrally Changes since v3: - Rebased - Rename PIXEL_FORMATS -> WRITEBACK_PIXEL_FORMATS Signed-off-by: Brian Starkey [rebased and fixed conflicts] Signed-off-by: Mihail Atanassov Signed-off-by: Liviu Dudau --- Documentation/gpu/drm-kms.rst | 9 ++ drivers/gpu/drm/Makefile| 2 +- drivers/gpu/drm/drm_atomic.c| 130 drivers/gpu/drm/drm_atomic_helper.c | 6 + drivers/gpu/drm/drm_connector.c | 4 +- drivers/gpu/drm/drm_writeback.c | 230 include/drm/drm_atomic.h| 3 + include/drm/drm_connector.h | 13 ++ include/drm/drm_mode_config.h | 14 +++ include/drm/drm_writeback.h | 76 include/uapi/drm/drm_mode.h | 1 + 11 files changed, 486 insertions(+), 2 deletions(-) create mode 100644 drivers/gpu/drm/drm_writeback.c create mode 100644 include/drm/drm_writeback.h diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst index bfecd21a8cdf..7a8ef1a4fe17 100644 --- a/Documentation/gpu/drm-kms.rst +++ b/Documentation/gpu/drm-kms.rst @@ -364,6 +364,15 @@ Connector Functions Reference .. kernel-doc:: drivers/gpu/drm/drm_connector.c :export: +Writeback Connectors + + +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c + :doc: overview + +.. kernel-doc:: drivers/gpu/drm/drm_writeback.c + :export: + Encoder Abstraction === diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile index 59f0f9b696eb..99ef500ae8b2 100644 --- a/drivers/gpu/drm/Makefile +++ b/drivers/gpu/drm/Makefile @@ -16,7 +16,7 @@ drm-y :=drm_auth.o drm_bufs.o drm_cache.o \ drm_framebuffer.o drm_connector.o drm_blend.o \ drm_encoder.o drm_mode_object.o drm_property.o \ drm_plane.o drm_color_mgmt.o drm_print.o \ - drm_dumb_buffers.o drm_mode_config.o + drm_dumb_buffers.o drm_mode_config.o drm_writeback.o drm-$(CONFIG_DRM_LIB_RANDOM) += lib/drm_random.o drm-$(CONFIG_DRM_VM) += drm_vm.o diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index f32506a7c1d6..317ae625b417 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -31,6 +31,7 @@ #include #include #include +#include #include #include "drm_crtc_internal.h" @@ -654,6 +655,46 @@ static void drm_atomic_crtc_print_state(struct drm_printer *p, } /** + * drm_atomic_connector_check - check connector state + * @connector: connector to check + * @state: connector state to check + * + * Provides core sanity checks for connector state. + * + * RETURNS: + * Zero on success, error code on failure + */ +static int drm_atomic_connector_check(struct drm_connector *connector, + struct drm_connector_state *state) +{ + struct drm_crtc_state *crtc_state; + struct drm_writeback_job *writeback_job = state->writeback_job; + + if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || + !writeback_job) + return 0; + + if (writeback_job->fb && !state->crtc) { + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] framebuffer without CRTC\n", +connector->base.id, connector->name); + return -EINVAL; + } + + if (state->crtc) + crtc_state = drm_atomic_get_existing_crtc_state(state->state, + state->crtc); + + if (writeback_job-
[PATCH v4 2/2] drm: writeback: Add out-fences for writeback connectors
From: Brian Starkey Add the WRITEBACK_OUT_FENCE_PTR property to writeback connectors, to enable userspace to get a fence which will signal once the writeback is complete. It is not allowed to request an out-fence without a framebuffer attached to the connector. A timeline is added to drm_writeback_connector for use by the writeback out-fences. In the case of a commit failure or DRM_MODE_ATOMIC_TEST_ONLY, the fence is set to -1. Changes from v2: - Rebase onto Gustavo Padovan's v9 explicit sync series - Change out_fence_ptr type to s32 __user * - Set *out_fence_ptr to -1 in drm_atomic_connector_set_property - Store fence in drm_writeback_job Gustavo Padovan: - Move out_fence_ptr out of connector_state - Signal fence from drm_writeback_signal_completion instead of in driver directly Changes from v3: - Rebase (change out_fence_ptr to s32 __user *, for real this time.) - Update documentation around WRITEBACK_OUT_FENCE_PTR Signed-off-by: Brian Starkey [rebased and fixed conflicts] Signed-off-by: Mihail Atanassov Signed-off-by: Liviu Dudau --- drivers/gpu/drm/drm_atomic.c| 99 drivers/gpu/drm/drm_writeback.c | 109 +++- include/drm/drm_atomic.h| 8 +++ include/drm/drm_connector.h | 8 +-- include/drm/drm_mode_config.h | 8 +++ include/drm/drm_writeback.h | 41 ++- 6 files changed, 257 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 317ae625b417..7132ffa106d5 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -305,6 +305,35 @@ static s32 __user *get_out_fence_for_crtc(struct drm_atomic_state *state, return fence_ptr; } +static int set_out_fence_for_connector(struct drm_atomic_state *state, + struct drm_connector *connector, + s32 __user *fence_ptr) +{ + unsigned int index = drm_connector_index(connector); + + if (!fence_ptr) + return 0; + + if (put_user(-1, fence_ptr)) + return -EFAULT; + + state->connectors[index].out_fence_ptr = fence_ptr; + + return 0; +} + +static s32 __user *get_out_fence_for_connector(struct drm_atomic_state *state, + struct drm_connector *connector) +{ + unsigned int index = drm_connector_index(connector); + s32 __user *fence_ptr; + + fence_ptr = state->connectors[index].out_fence_ptr; + state->connectors[index].out_fence_ptr = NULL; + + return fence_ptr; +} + /** * drm_atomic_set_mode_for_crtc - set mode for CRTC * @state: the CRTC whose incoming state to update @@ -691,6 +720,12 @@ static int drm_atomic_connector_check(struct drm_connector *connector, return -EINVAL; } + if (writeback_job->out_fence && !writeback_job->fb) { + DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] requesting out-fence without framebuffer\n", +connector->base.id, connector->name); + return -EINVAL; + } + return 0; } @@ -1170,6 +1205,11 @@ int drm_atomic_connector_set_property(struct drm_connector *connector, if (fb) drm_framebuffer_unreference(fb); return ret; + } else if (property == config->writeback_out_fence_ptr_property) { + s32 __user *fence_ptr = u64_to_user_ptr(val); + + return set_out_fence_for_connector(state->state, connector, + fence_ptr); } else if (connector->funcs->atomic_set_property) { return connector->funcs->atomic_set_property(connector, state, property, val); @@ -1249,6 +1289,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, } else if (property == config->writeback_fb_id_property) { /* Writeback framebuffer is one-shot, write and forget */ *val = 0; + } else if (property == config->writeback_out_fence_ptr_property) { + *val = 0; } else if (connector->funcs->atomic_get_property) { return connector->funcs->atomic_get_property(connector, state, property, val); @@ -2086,7 +2128,7 @@ static int setup_out_fence(struct drm_out_fence_state *fence_state, return 0; } -static int prepare_crtc_signaling(struct drm_device *dev, +static int prepare_signaling(struct drm_device *dev, struct drm_atomic_state *state, struct drm_mode_atomic *arg, struct drm_file *file_priv, @@ -2095,6 +2137,8 @@ static int prepare_crtc_signaling(struct drm_device *dev, { struct drm_crtc *crtc; struct drm_crtc_state *crtc_state;
[PATCH v4 0/2] drm: Introduce writeback connectors
Hi, This is an update of Brian Starkey's RFC v3 series introducing support for memory writeback engines as special connector. My contribution has been minor compared to previous version, mostly rebasing, addressing some of the comments and cleaning up the behaviour of set_out_fence_for_connector() function to match what the set_out_fence_for_crtc() callers do. I've split out the Mali DP patches that use this series into a separate patchset to be sent shortly after this. I'm taking over from Brian on this, so please send the comments to me but don't be afraid to Cc him on them. Best regards, Liviu Brian Starkey (2): drm: Add writeback connector type drm: writeback: Add out-fences for writeback connectors Documentation/gpu/drm-kms.rst | 9 + drivers/gpu/drm/Makefile| 2 +- drivers/gpu/drm/drm_atomic.c| 229 +++- drivers/gpu/drm/drm_atomic_helper.c | 6 + drivers/gpu/drm/drm_connector.c | 4 +- drivers/gpu/drm/drm_writeback.c | 335 include/drm/drm_atomic.h| 11 ++ include/drm/drm_connector.h | 13 ++ include/drm/drm_mode_config.h | 22 +++ include/drm/drm_writeback.h | 115 + include/uapi/drm/drm_mode.h | 1 + 11 files changed, 736 insertions(+), 11 deletions(-) create mode 100644 drivers/gpu/drm/drm_writeback.c create mode 100644 include/drm/drm_writeback.h -- 2.12.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 1/3] drm: mali-dp: Add support for writeback on DP550/DP650
Mali-DP display processors are able to write the composition result to a memory buffer via the SE. Add entry points in the HAL for enabling/disabling this feature, and implement support for it on DP650 and DP550. DP500 acts differently and so is omitted from this change. Changes since v3: - Fix missing vsync interrupt for DP550 Signed-off-by: Liviu Dudau Signed-off-by: Brian Starkey [rebased and fixed conflicts] Signed-off-by: Mihail Atanassov --- drivers/gpu/drm/arm/malidp_hw.c | 53 +-- drivers/gpu/drm/arm/malidp_hw.h | 17 + drivers/gpu/drm/arm/malidp_regs.h | 15 +++ 3 files changed, 83 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c index 28360b8542f7..accb57e2aa30 100644 --- a/drivers/gpu/drm/arm/malidp_hw.c +++ b/drivers/gpu/drm/arm/malidp_hw.c @@ -588,6 +588,48 @@ static long malidp550_se_calc_mclk(struct malidp_hw_device *hwdev, return ret; } +static int malidp550_enable_memwrite(struct malidp_hw_device *hwdev, +dma_addr_t *addrs, s32 *pitches, +int num_planes, u16 w, u16 h, u32 fmt_id) +{ + u32 base = MALIDP550_SE_MEMWRITE_BASE; + u32 de_base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK); + + /* enable the scaling engine block */ + malidp_hw_setbits(hwdev, MALIDP_SCALE_ENGINE_EN, de_base + MALIDP_DE_DISPLAY_FUNC); + + malidp_hw_write(hwdev, fmt_id, base + MALIDP_MW_FORMAT); + switch (num_planes) { + case 2: + malidp_hw_write(hwdev, lower_32_bits(addrs[1]), base + MALIDP_MW_P2_PTR_LOW); + malidp_hw_write(hwdev, upper_32_bits(addrs[1]), base + MALIDP_MW_P2_PTR_HIGH); + malidp_hw_write(hwdev, pitches[1], base + MALIDP_MW_P2_STRIDE); + /* fall through */ + case 1: + malidp_hw_write(hwdev, lower_32_bits(addrs[0]), base + MALIDP_MW_P1_PTR_LOW); + malidp_hw_write(hwdev, upper_32_bits(addrs[0]), base + MALIDP_MW_P1_PTR_HIGH); + malidp_hw_write(hwdev, pitches[0], base + MALIDP_MW_P1_STRIDE); + break; + default: + WARN(1, "Invalid number of planes"); + } + + malidp_hw_write(hwdev, MALIDP_DE_H_ACTIVE(w) | MALIDP_DE_V_ACTIVE(h), + MALIDP550_SE_MEMWRITE_OUT_SIZE); + malidp_hw_setbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | MALIDP_SE_MEMWRITE_EN, + MALIDP550_SE_CONTROL); + + return 0; +} + +static void malidp550_disable_memwrite(struct malidp_hw_device *hwdev) +{ + u32 base = malidp_get_block_base(hwdev, MALIDP_DE_BLOCK); + malidp_hw_clearbits(hwdev, MALIDP550_SE_MEMWRITE_ONESHOT | MALIDP_SE_MEMWRITE_EN, + MALIDP550_SE_CONTROL); + malidp_hw_clearbits(hwdev, MALIDP_SCALE_ENGINE_EN, base + MALIDP_DE_DISPLAY_FUNC); +} + static int malidp650_query_hw(struct malidp_hw_device *hwdev) { u32 conf = malidp_hw_read(hwdev, MALIDP550_CONFIG_ID); @@ -673,9 +715,11 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = { .se_irq_map = { .irq_mask = MALIDP550_SE_IRQ_EOW | MALIDP550_SE_IRQ_AXI_ERR, + .vsync_irq = MALIDP550_SE_IRQ_EOW, }, .dc_irq_map = { - .irq_mask = MALIDP550_DC_IRQ_CONF_VALID, + .irq_mask = MALIDP550_DC_IRQ_CONF_VALID | + MALIDP550_DC_IRQ_SE, .vsync_irq = MALIDP550_DC_IRQ_CONF_VALID, }, .pixel_formats = malidp550_de_formats, @@ -691,6 +735,8 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = { .rotmem_required = malidp550_rotmem_required, .se_set_scaling_coeffs = malidp550_se_set_scaling_coeffs, .se_calc_mclk = malidp550_se_calc_mclk, + .enable_memwrite = malidp550_enable_memwrite, + .disable_memwrite = malidp550_disable_memwrite, .features = 0, }, [MALIDP_650] = { @@ -713,7 +759,8 @@ const struct malidp_hw_device malidp_device[MALIDP_MAX_DEVICES] = { MALIDP550_SE_IRQ_AXI_ERR, }, .dc_irq_map = { - .irq_mask = MALIDP550_DC_IRQ_CONF_VALID, + .irq_mask = MALIDP550_DC_IRQ_CONF_VALID | + MALIDP550_DC_IRQ_SE, .vsync_irq = MALIDP550_DC_IRQ_CONF_VALID, }, .pixel_formats = malidp550_de_formats, @@ -729,6 +776,8 @@ const struct malidp_hw_
[PATCH v4 0/3] drm/mali-dp: Add support for memory writeback engine
Hi, This series introduces support for Mali DP's memory writeback engine using the generic writeback connector support introduced in the "[PATCH v4 0/2] drm: Introduce writeback connectors" series [1] I've rebased the previous work that Brian has done on top of drm-next and the generic series. I will take over the work, so please add me in any comments you might have Best regards, Liviu [1] https://lists.freedesktop.org/archives/dri-devel/2017-May/140850.html Brian Starkey (2): drm: mali-dp: Add RGB writeback formats for DP550/DP650 drm: mali-dp: Add writeback connector Liviu Dudau (1): drm: mali-dp: Add support for writeback on DP550/DP650 drivers/gpu/drm/arm/Makefile | 1 + drivers/gpu/drm/arm/malidp_crtc.c | 9 ++ drivers/gpu/drm/arm/malidp_drv.c | 19 ++- drivers/gpu/drm/arm/malidp_drv.h | 4 + drivers/gpu/drm/arm/malidp_hw.c | 88 +--- drivers/gpu/drm/arm/malidp_hw.h | 18 +++ drivers/gpu/drm/arm/malidp_mw.c | 279 ++ drivers/gpu/drm/arm/malidp_mw.h | 18 +++ drivers/gpu/drm/arm/malidp_regs.h | 15 ++ 9 files changed, 430 insertions(+), 21 deletions(-) create mode 100644 drivers/gpu/drm/arm/malidp_mw.c create mode 100644 drivers/gpu/drm/arm/malidp_mw.h -- 2.12.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v4 3/3] drm: mali-dp: Add writeback connector
From: Brian Starkey Mali-DP has a memory writeback engine which can be used to write the composition result to a memory buffer. Expose this functionality as a DRM writeback connector on supported hardware. Changes since v1: Daniel Vetter: - Don't require a modeset when writeback routing changes - Make writeback connector always disconnected Changes since v2: - Rebase onto new drm_writeback_connector - Add reset callback, allocating subclassed state Daniel Vetter: - Squash out-fence support into this commit Gustavo Padovan: - Don't signal fence directly from driver (and drop malidp_mw_job) Changes since v3: - Modifications to fit with Mali-DP commit tail changes Signed-off-by: Brian Starkey [rebased and fixed conflicts] Signed-off-by: Mihail Atanassov Signed-off-by: Liviu Dudau --- drivers/gpu/drm/arm/Makefile | 1 + drivers/gpu/drm/arm/malidp_crtc.c | 9 ++ drivers/gpu/drm/arm/malidp_drv.c | 19 ++- drivers/gpu/drm/arm/malidp_drv.h | 4 + drivers/gpu/drm/arm/malidp_hw.c | 7 +- drivers/gpu/drm/arm/malidp_mw.c | 279 ++ drivers/gpu/drm/arm/malidp_mw.h | 18 +++ 7 files changed, 332 insertions(+), 5 deletions(-) create mode 100644 drivers/gpu/drm/arm/malidp_mw.c create mode 100644 drivers/gpu/drm/arm/malidp_mw.h diff --git a/drivers/gpu/drm/arm/Makefile b/drivers/gpu/drm/arm/Makefile index bb8b158ff90d..3bf31d1a4722 100644 --- a/drivers/gpu/drm/arm/Makefile +++ b/drivers/gpu/drm/arm/Makefile @@ -1,4 +1,5 @@ hdlcd-y := hdlcd_drv.o hdlcd_crtc.o obj-$(CONFIG_DRM_HDLCD)+= hdlcd.o mali-dp-y := malidp_drv.o malidp_hw.o malidp_planes.o malidp_crtc.o +mali-dp-y += malidp_mw.o obj-$(CONFIG_DRM_MALI_DISPLAY) += mali-dp.o diff --git a/drivers/gpu/drm/arm/malidp_crtc.c b/drivers/gpu/drm/arm/malidp_crtc.c index 9446a673d469..ddbeb4d4e565 100644 --- a/drivers/gpu/drm/arm/malidp_crtc.c +++ b/drivers/gpu/drm/arm/malidp_crtc.c @@ -400,6 +400,15 @@ static int malidp_crtc_atomic_check(struct drm_crtc *crtc, } } + /* If only the writeback routing has changed, we don't need a modeset */ + if (state->connectors_changed) { + u32 old_mask = crtc->state->connector_mask; + u32 new_mask = state->connector_mask; + if ((old_mask ^ new_mask) == + (1 << drm_connector_index(&malidp->mw_connector.base))) + state->connectors_changed = false; + } + ret = malidp_crtc_atomic_check_gamma(crtc, state); ret = ret ? ret : malidp_crtc_atomic_check_ctm(crtc, state); ret = ret ? ret : malidp_crtc_atomic_check_scaling(crtc, state); diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c index 0d3eb537d08b..6a80e06f8956 100644 --- a/drivers/gpu/drm/arm/malidp_drv.c +++ b/drivers/gpu/drm/arm/malidp_drv.c @@ -29,6 +29,7 @@ #include #include "malidp_drv.h" +#include "malidp_mw.h" #include "malidp_regs.h" #include "malidp_hw.h" @@ -231,6 +232,8 @@ static void malidp_atomic_commit_tail(struct drm_atomic_state *state) malidp_atomic_commit_se_config(crtc, old_crtc_state); } + malidp_mw_atomic_commit(drm, state); + drm_atomic_helper_commit_planes(drm, state, 0); drm_atomic_helper_commit_modeset_enables(drm, state); @@ -271,12 +274,20 @@ static int malidp_init(struct drm_device *drm) drm->mode_config.helper_private = &malidp_mode_config_helpers; ret = malidp_crtc_init(drm); - if (ret) { - drm_mode_config_cleanup(drm); - return ret; - } + if (ret) + goto crtc_fail; + + ret = malidp_mw_connector_init(drm); + if (ret) + goto mw_fail; return 0; + +mw_fail: + malidp_de_planes_destroy(drm); +crtc_fail: + drm_mode_config_cleanup(drm); + return ret; } static void malidp_fini(struct drm_device *drm) diff --git a/drivers/gpu/drm/arm/malidp_drv.h b/drivers/gpu/drm/arm/malidp_drv.h index 040311ffcaec..9f7d10eecbc5 100644 --- a/drivers/gpu/drm/arm/malidp_drv.h +++ b/drivers/gpu/drm/arm/malidp_drv.h @@ -13,6 +13,8 @@ #ifndef __MALIDP_DRV_H__ #define __MALIDP_DRV_H__ +#include +#include #include #include #include @@ -22,6 +24,8 @@ struct malidp_drm { struct malidp_hw_device *dev; struct drm_fbdev_cma *fbdev; struct drm_crtc crtc; + struct drm_encoder mw_encoder; + struct drm_writeback_connector mw_connector; wait_queue_head_t wq; atomic_t config_valid; struct drm_atomic_state *pm_state; diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c index d6645395e274..580c77c96cd7 100644 --- a/drivers/gpu/drm/arm/malidp_hw.c +++ b/drivers/gpu/drm/arm/malidp_hw.c @@ -21,6 +21,7 @@ #include "malidp_drv.h" #include "malidp_hw.h" +#include "malidp_mw.h" static const struct malidp_format_id malidp500_de_formats[] = { /*
[PATCH v4 2/3] drm: mali-dp: Add RGB writeback formats for DP550/DP650
From: Brian Starkey Add a layer bit for the SE memory-write, and add it to the pixel format matrix for DP550/DP650. Signed-off-by: Brian Starkey [rebased and fixed conflicts] Signed-off-by: Mihail Atanassov Signed-off-by: Liviu Dudau --- drivers/gpu/drm/arm/malidp_hw.c | 28 ++-- drivers/gpu/drm/arm/malidp_hw.h | 1 + 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/arm/malidp_hw.c b/drivers/gpu/drm/arm/malidp_hw.c index accb57e2aa30..d6645395e274 100644 --- a/drivers/gpu/drm/arm/malidp_hw.c +++ b/drivers/gpu/drm/arm/malidp_hw.c @@ -47,20 +47,20 @@ static const struct malidp_format_id malidp500_de_formats[] = { #define MALIDP_COMMON_FORMATS \ /*fourcc, layers supporting the format, internal id */ \ - { DRM_FORMAT_ARGB2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(0, 0) }, \ - { DRM_FORMAT_ABGR2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(0, 1) }, \ - { DRM_FORMAT_RGBA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(0, 2) }, \ - { DRM_FORMAT_BGRA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(0, 3) }, \ - { DRM_FORMAT_ARGB, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(1, 0) }, \ - { DRM_FORMAT_ABGR, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(1, 1) }, \ - { DRM_FORMAT_RGBA, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(1, 2) }, \ - { DRM_FORMAT_BGRA, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(1, 3) }, \ - { DRM_FORMAT_XRGB, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(2, 0) }, \ - { DRM_FORMAT_XBGR, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(2, 1) }, \ - { DRM_FORMAT_RGBX, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(2, 2) }, \ - { DRM_FORMAT_BGRX, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART, MALIDP_ID(2, 3) }, \ - { DRM_FORMAT_RGB888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(3, 0) }, \ - { DRM_FORMAT_BGR888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(3, 1) }, \ + { DRM_FORMAT_ARGB2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(0, 0) }, \ + { DRM_FORMAT_ABGR2101010, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(0, 1) }, \ + { DRM_FORMAT_RGBA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(0, 2) }, \ + { DRM_FORMAT_BGRA1010102, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(0, 3) }, \ + { DRM_FORMAT_ARGB, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(1, 0) }, \ + { DRM_FORMAT_ABGR, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(1, 1) }, \ + { DRM_FORMAT_RGBA, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(1, 2) }, \ + { DRM_FORMAT_BGRA, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(1, 3) }, \ + { DRM_FORMAT_XRGB, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(2, 0) }, \ + { DRM_FORMAT_XBGR, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(2, 1) }, \ + { DRM_FORMAT_RGBX, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(2, 2) }, \ + { DRM_FORMAT_BGRX, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | DE_SMART | SE_MEMWRITE, MALIDP_ID(2, 3) }, \ + { DRM_FORMAT_RGB888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(3, 0) }, \ + { DRM_FORMAT_BGR888, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2 | SE_MEMWRITE, MALIDP_ID(3, 1) }, \ { DRM_FORMAT_RGBA5551, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(4, 0) }, \ { DRM_FORMAT_ABGR1555, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(4, 1) }, \ { DRM_FORMAT_RGB565, DE_VIDEO1 | DE_GRAPHICS1 | DE_VIDEO2, MALIDP_ID(4, 2) }, \ diff --git a/drivers/gpu/drm/arm/malidp_hw.h b/drivers/gpu/drm/arm/malidp_hw.h index a10f5636fa36..1a47ce5a8edd 100644 --- a/drivers/gpu/drm/arm/malidp_hw.h +++ b/drivers/gpu/drm/arm/malidp_hw.h @@ -33,6 +33,7 @@ enum { DE_GRAPHICS2 = BIT(2), /* used only in DP500 */ DE_VIDEO2 = BIT(3), DE_SMART = BIT(4), + SE_MEMWRITE = BIT(5), }; struct malidp_format_id { -- 2.12.2 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 194721] Reading /sys/bus/pci/devices/0000:01:00.0/config wakes up Radeon dGPU
https://bugzilla.kernel.org/show_bug.cgi?id=194721 mathieu.westp...@gmail.com changed: What|Removed |Added CC||mathieu.westp...@gmail.com --- Comment #6 from mathieu.westp...@gmail.com --- I reproduce this bug as well. lspci hangs and freeze the system. -- You are receiving this mail because: You are watching the assignee of the bug. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100949] Power management problem on CIK/SI
https://bugs.freedesktop.org/show_bug.cgi?id=100949 Bug ID: 100949 Summary: Power management problem on CIK/SI Product: DRI Version: DRI git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Severity: major Priority: medium Component: DRM/AMDgpu Assignee: dri-devel@lists.freedesktop.org Reporter: l...@fedoraproject.org Created attachment 131232 --> https://bugs.freedesktop.org/attachment.cgi?id=131232&action=edit dmesg boot from X550Z Recent kernel broke the handling of power management for CIK/SI hybrid laptop using the latest 4.9.25-20170502 from https://cgit.freedesktop.org/~agd5f/linux/?h=amd-staging-4.9 Notable highlight: [ 25.671428] kfd kfd: Allocated 3944480 bytes on gart for device(1002:130d) [ 25.671449] kfd kfd: error getting iommu info. is the iommu enabled? [ 25.671493] kfd kfd: Error initializing iommuv2 for device (1002:130d) [ 25.671558] Creating topology SYSFS entries [ 25.671642] kfd kfd: device (1002:130d) NOT added due to errors [ 695.257618] [drm:amdgpu_atombios_dp_link_train [amdgpu]] *ERROR* displayport link status failed [ 695.257654] [drm:amdgpu_atombios_dp_link_train [amdgpu]] *ERROR* clock recovery failed For a while, suspend/resume will lock up in back screen forcing a hard reset. Laptop is Asus X500ZE equipped with Kaveri and Hainan dual GPU. -- 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 100949] Power management problem on CIK/SI hybrid laptop
https://bugs.freedesktop.org/show_bug.cgi?id=100949 Luya Tshimbalanga changed: What|Removed |Added Summary|Power management problem on |Power management problem on |CIK/SI |CIK/SI hybrid laptop -- 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 1/3] RFC: drm: Restrict vblank ioctl to master
On Mon, Jul 25, 2016 at 1:15 PM, Rainer Hochecker wrote: > Am 25.07.2016 08:38 schrieb "Michel Dänzer" : >> >> On 13.07.2016 18:49, Rainer Hochecker wrote: >> > We have been using this for years now and did not observe issues you >> > mentioned. I would be surprised if a child window refreshes in a >> > different rate than the main window >> >> The Xorg driver decides which CRTC to synchronize with based on the >> window geometry. For a window with no visible geometry, it may choose a >> different CRTC than for the visible output window. In the case of DRI3, >> the Xorg Present extension code may even fall back to the fake CRTC in >> that case, which only generates a fake vblank event once every second. >> >> >> -- >> Earthling Michel Dänzer | http://www.amd.com >> Libre software enthusiast | Mesa and X developer >> > > That means that we won't have a solution for X11 with EGL for Intel systems > anymore. I didn't know about this, but a proper EGL extension, implemented on X11 exists already: https://codereview.chromium.org/2867513002/ It's not in the khronos registry, but mesa implements it, and semantics match oml_sync (and wayland present), so we already have the proper extension to implement this without digging around in device files and making a guess about which pipe you might be running on. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH RESEND] drm/i915: Fix pipe/transcoder enum mismatches
On Fri, May 05, 2017 at 12:12:49PM -0700, Grant Grundler wrote: > On Fri, May 5, 2017 at 10:40 AM, Ville Syrjälä > wrote: > > On Fri, May 05, 2017 at 10:26:36AM -0700, Matthias Kaehlcke wrote: > >> El Thu, Apr 20, 2017 at 02:56:05PM -0700 Matthias Kaehlcke ha dit: > >> > >> > In several instances the driver passes an 'enum pipe' value to a > >> > function expecting an 'enum transcoder' and viceversa. Since PIPE_x and > >> > TRANSCODER_x have the same values this doesn't cause functional > >> > problems. Still it is incorrect and causes clang to generate warnings > >> > like this: > >> > > >> > drivers/gpu/drm/i915/intel_display.c:1844:34: warning: implicit > >> > conversion from enumeration type 'enum transcoder' to different > >> > enumeration type 'enum pipe' [-Wenum-conversion] > >> > assert_fdi_rx_enabled(dev_priv, TRANSCODER_A); > >> > > >> > Change the code to pass values of the type expected by the callee. > >> > > >> > Signed-off-by: Matthias Kaehlcke > >> > --- > >> > drivers/gpu/drm/i915/intel_display.c | 4 ++-- > >> > drivers/gpu/drm/i915/intel_dp.c | 6 -- > >> > drivers/gpu/drm/i915/intel_hdmi.c| 6 -- > >> > drivers/gpu/drm/i915/intel_sdvo.c| 6 -- > >> > 4 files changed, 14 insertions(+), 8 deletions(-) > >> > >> Ping, any comments on this patch? > > > > I'm not convinced the patch is making things any better really. To > > fix this really properly, I think we'd need to introduce a new enum > > pch_transcoder and thus avoid the confusion of which type of > > transcoder we're talking about. > > Is an enum better than coding an explicit conversion in an inline function? The point of the enum would be to make it more clear which piece of hardware we're talking to in each case. But this would require going through the entire PCH code and changing things to use the right type in each case. Quite a bit of work with little measurable gain I'd say. > Then the code can do some sanity checking as well. Something like: > > enum transcoder pch_to_cpu_enum(enum pipe) > { > WARN_ON(pipe > FOO); > return (enum transcoder) pipe; > } That would have to be called pipe_to_pch_transcoder() or something like that. > > > Currently most places expect an > > enum pipe when dealing with PCH transcoders, and enum transcoder > > when dealing with CPU transcoders. But there are some exceptions > > of course. > > cheers, > grant > > > > -- > > Ville Syrjälä > > Intel OTC -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/3] RFC: drm: Restrict vblank ioctl to master
On 05/05/2017 09:35 PM, Daniel Vetter wrote: On Mon, Jul 25, 2016 at 1:15 PM, Rainer Hochecker wrote: Am 25.07.2016 08:38 schrieb "Michel Dänzer" : On 13.07.2016 18:49, Rainer Hochecker wrote: We have been using this for years now and did not observe issues you mentioned. I would be surprised if a child window refreshes in a different rate than the main window The Xorg driver decides which CRTC to synchronize with based on the window geometry. For a window with no visible geometry, it may choose a different CRTC than for the visible output window. In the case of DRI3, the Xorg Present extension code may even fall back to the fake CRTC in that case, which only generates a fake vblank event once every second. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer That means that we won't have a solution for X11 with EGL for Intel systems anymore. I didn't know about this, but a proper EGL extension, implemented on X11 exists already: https://codereview.chromium.org/2867513002/ It's not in the khronos registry, but mesa implements it, and semantics match oml_sync (and wayland present), so we already have the proper extension to implement this without digging around in device files and making a guess about which pipe you might be running on. Cheers, Daniel Yep. The weird thing about that extension is that it only implements the query vblank count/timestamp part of GLX_OML_sync_control. Sounds like the least useful bit of all in isolation, although the extension as a whole could be useful, just as on GLX. Also note this comment on that commit referenced by Daniel from just 2 hours ago: "marcheu 2 hours, 35 minutes ago (2017-05-05 18:52:45 UTC) #10 lgtm By the way, there is discussion on dropping this extension from mesa. You should let Chad know if you need it in the long run. Chrome OS stopped requiring it when we switched to freon." Cc'ing Chad Versace on the off-chance he is the mentioned Chad. Also, the VDPAU api for video playback implements functions for precise timing of video frame presentation. I don't have any practical experience with them, but iirc Mesa's VDPAU video presentation api implements those via DRI3/Present extension, so hooks into drmWaitVblank et al. If Kodi uses VDPAU for playback, it could use those conveniently for timing. -mario ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 100387] War Thunder game has visual errors, missing textures
https://bugs.freedesktop.org/show_bug.cgi?id=100387 --- Comment #3 from russianneuroman...@ya.ru --- Created attachment 131233 --> https://bugs.freedesktop.org/attachment.cgi?id=131233&action=edit War Thunder on Radeon HD 8470D with Mesa 17.1.0rc2 Same issue on Radeon 8470D. Software: Kubuntu 17.0 x86_64 Linux 4.10 Mesa: 17.1.0rc2 libdrm: 2.4.80 xserver-xorg-video-radeon: 7.9.0 xserver-xorg-core: 1.19.3 Hardware: AMD A6-6400K with Radeon HD 8470D (ARUBA) -- 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 100953] [ARUBA] Light shaders is very bright in GRID Autosport on Radeon 8470D
https://bugs.freedesktop.org/show_bug.cgi?id=100953 Bug ID: 100953 Summary: [ARUBA] Light shaders is very bright in GRID Autosport on Radeon 8470D Product: Mesa Version: 17.1 Hardware: Other OS: All Status: NEW Severity: normal Priority: medium Component: Drivers/Gallium/r600 Assignee: dri-devel@lists.freedesktop.org Reporter: russianneuroman...@ya.ru QA Contact: dri-devel@lists.freedesktop.org Created attachment 131234 --> https://bugs.freedesktop.org/attachment.cgi?id=131234&action=edit GRID Autosport on Radeon 8470D Hello! Please look into attached screenshot. This issue is not reproducible on Radeon HD 6650M. Software: Kubuntu 17.0 x86_64 Linux 4.10 Mesa: 17.1.0rc2 libdrm: 2.4.80 xserver-xorg-video-radeon: 7.9.0 xserver-xorg-core: 1.19.3 Hardware: AMD A6-6400K with Radeon HD 8470D (ARUBA) -- 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