Re: [PATCH v3] drm/edid: Complete CEA modedb(VIC 1-107)
On 29.01.2017 06:41, Shashank Sharma wrote: > CEA-861-F specs defines new 4k video modes to be used with > HDMI 2.0 EDIDs. These modes start at VIC=93 and go all the > way till VIC=107. > > Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now > to be able to parse 4k modes using the existing techniques, we have > to complete the modedb (VIC=65 onwards). > > This patch adds: > - Timings for existing CEA video modes (from VIC=65 till VIC=92) > - Newly added 4k modes (from VIC=93 to VIC=107). > > Cc: Jose Abreu > Cc: Alex Deucher > Cc: Andrzej Hajda > > V2: Addressed review comments from Jose: > - fix the timings for VIC 83, 90 and 91 > - fix formatting for VIC 93-107 > > V3: Rebase on drm-tip > > Signed-off-by: Shashank Sharma > Signed-off-by: Sonika Jindal > Reviewed-by: Jose Abreu > Reviewed-by: Alex Deucher > --- Have you addressed in some other patch my concerns regarding polluting infoframes generated for HDMI 1.4 devices (CEA-861-E) with VICs specific only for HDMI 2.0 (CEA-861-F) ? It will happen for all UHD modes, see [1][2] for previous discussion. [1]: https://www.spinics.net/lists/intel-gfx/msg110524.html [2]: https://www.spinics.net/lists/intel-gfx/msg110529.html Regards Andrzej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: make fbdev/fbcon switchable per driver?
Hi, > The vgaarb code has a concept of a vga_default_device(), it's rather > PCI-centric, but maybe better than nothing. This is typically the > first VGA class code device found with I/O and MMIO enabled. If fbcon > defaulted to running on the vga_default_device(), a user could select > which to use by re-ordering the VM hardware. The qemu drivers don't register as vgaarb clients though. Which easily explains why igd always wins the primary selection, no matter how you order your hardware. So, should they register? The drivers don't need access to the vga registers[1][2]. cheers, Gerd [1] Well, except cirrus, but that is deprecated anyway. [2] And bochs-drm, when running on old qemu versions, where the stdvga has no mmio bar yet. But those qemu versions don't support igd pass-through. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm: qxl: Drop misleading comment
Hi, > Do you want to merge from drm-misc or should we push this one > directly via drm-misc? Merging through drm-misc is fine with me. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [Intel-gfx] [PATCH 0/3] drm/i915: Handle hanging during nonblocking modeset correctly.
On Fri, Jan 27, 2017 at 03:08:45PM +, Chris Wilson wrote: > On Fri, Jan 27, 2017 at 03:58:08PM +0100, Daniel Vetter wrote: > > On Fri, Jan 27, 2017 at 02:31:55PM +, Chris Wilson wrote: > > > On Fri, Jan 27, 2017 at 03:21:29PM +0100, Daniel Vetter wrote: > > > > On Fri, Jan 27, 2017 at 09:30:50AM +, Chris Wilson wrote: > > > > > On Thu, Jan 26, 2017 at 04:59:21PM +0100, Maarten Lankhorst wrote: > > > > > > When writing some testcases for nonblocking modesets. I found out > > > > > > that the > > > > > > infinite wait on the old fb was causing issues. > > > > > > > > > > The crux of the issue here is the locked wait for old dependencies and > > > > > the inability to inject the intel_prepare_reset disabling of all > > > > > planes. > > > > > There are a couple of locked waits on struct_mutex within the modeset > > > > > locks for intel_overlay and if we happen to be using the display plane > > > > > for the first time. > > > > > > > > > > The first I suggested solving using fences to track dependencies and > > > > > keep the order between atomic states. Cancelling the outstanding > > > > > modesets, replacing with a disable and then on restore jumping to the > > > > > final state look doable. It also requires avoiding the struct_mutex > > > > > for > > > > > disabling, which is quite easy. To avoid the wait under struct_mutex, > > > > > we've talked about switching to mmio, but for starters we could move > > > > > the > > > > > wait from inside intel_overlay into the fence for the atomic > > > > > operation. > > > > > (But's that a little more surgery than we would like for > > > > > intel_overlay I > > > > > guess - dig out Ville's patches for overlay planes?) And to prevent > > > > > the > > > > > wait under struct_mutex for pin_to_display_plane, my plane is to move > > > > > that to an async fenced operation that is then naturally waited upon > > > > > by > > > > > the atomic modeset. > > > > > > > > A bit more a hack, but a different idea, and I think hack for gen234.0 > > > > is > > > > ok: > > > > > > > > We complete all the requests before we start the hw reset with > > > > fence.error > > > > = -EIO. But we do this only when we need to get at the display locks. A > > > > slightly more elegant solution would be to trylock modeset locks, and if > > > > one of them fails (and only then) complete all requests with -EIO to get > > > > the concurrent modeset to proceed before we reset the hardware. That's > > > > essentially the logic we had before all the reworks, and it worked. But > > > > I > > > > didn't look at how scary that all would be to make it work again ... > > > > > > The modeset lock may not just be waiting on our requests (even on pnv we > > > can expect that there are already users celebrating that pnv+nouveau > > > finally works ;) and that the display is not the only user/observer of > > > those requests. Using the requests to break the modeset lock just feels > > > like the wrong approach. > > > > It's a cycle, and we need to break it somewhere. Another option might be > > to break the cycle the same way we do it for gem locks: Wake up everyone > > and restart the modeset ioctl. Since the trouble only happens for > > synchronous modesets where we hold the locks while waiting for fences, we > > can also break out of that and restart. And I also don't think that would > > leak to other drivers, after all our gem locking restart dances also don't > > leak to other drivers - it's just our own driver's lock which are affected > > by these special wakupe semantics. > > It's a queue of nonblocking modesets that we need to worry about, afaik. > Moving the wait for blocking modeset outside of modeset lock is easily > achievable (and avoiding the other waits under both the modeset + > struct_mutex I have at least an idea for). So the challenge is how to > inject all-planes-off for gen3 and then allow the queue to continue again > afterwards. Hm right, I missed the nonblocking updates which don't take locks. But assuming we do the display reset for gpu resets as a full modeset (i.e. going through ->atomic_commit) it should still work out correctly: Starting state: gpu is hung, nonblocking modeset waiting for some requests to complete. 1. hangcheck kicks in, fires off reset work. 2. We complete all requests with fence.error = -EIO and wake up any waiters. That means no re-queueing for older platforms, but oh well. 3. We grab all the display locks. Nothing happens yet. 4. We reset the chip, display dies. 5. We run ->atomic_commit to restore things. This will also force the nonblocking commit worker to complete before this display restore touches anything. The only trouble I see is that the nonblocking worker can still touch the display block while we kill it, which isn't awesome. But we can fix that by waiting for all pending nonblocking commits in step 3 manually (without calling into atomic_commit), as long as we do step 2. So completing everything with EIO u
Re: RFC: drm-misc for small drivers?
On Fri, Jan 27, 2017 at 11:50:42AM -0500, Alex Deucher wrote: > On Fri, Jan 27, 2017 at 1:52 AM, Daniel Vetter wrote: > > On Thu, Jan 26, 2017 at 8:48 PM, Eric Anholt wrote: > >>> - Should we require review or at least acks for patches committed by > >>> the author? We have a bunch of drivers with effectively just 1 person > >>> working on it, where getting real review is hard. But otoh a few of > >>> those 1-person drivers will become popular, and then it's good to > >>> start with establishing peer-review early on. I also think that > >>> requiring peer-review is good to share best practices and knowledge > >>> between different people in our community, not just to make sure the > >>> code is correct. For all these reasons I'm leaning towards not making > >>> an exception for drivers, and requiring the same amount of review for > >>> them if they go in through drm-misc as for any other patch. > >> > >> This is the only part I'm concerned about. Would anyone review vc4 > >> patches? Voluntarily? Actually thinking about the > >> functionality/structure of the code, not just style? > >> > >> It sucks today that as part of the kernel process I send out patches > >> "for review", knowing that I won't ever get review, and I just have to > >> wait a while until I think people won't complain at me for sending a PR > >> too quickly. But if the change is to just start blocking my patches on > >> review, I'm concerned I won't be able to get them in at all. > >> > >> I think there's a middle ground, where you graduate to waiting for > >> review once you have multiple involved in that area of the code. This > >> is what we do in Mesa -- vc4 and freedreno push directly, but on the > >> code we share (tgsi_to_nir, for example), we do review. > >> > >> (This is also the point at which I'll offer: Any other ARM drivers that > >> want to do review exchange with me, let me know and I'll start paying > >> attention to your stuff) > > > > So part of the evil goal here is really to kickstart a tit-for-tat > > review economy. That'd would mean we'd need at least 2-3 drivers to > > volunteer for starters, and in many cases a full review is not going > > to happen (or would just unduly delay things for no gain at all). What > > I think would be great though is something much less formal along the > > lines of "read your patch, looks all reasonable, seems to use core drm > > code&concepts how it's supposed, ack". Maybe a few recommendations how > > code can be simplified/clarified, but definitely no multi-round > > bikeshedding tour. So not review to ensure code correctness, but just > > as an information and best practice sharing tool. Also this should be > > a good way to catch good opportunities for subsystem wide refactorings > > when someone copypastes the same few lines for the umpteenth time > > (that's how we e.g. got rid of all the dummy callback > > implementations). And it shouldn't be much work, when I review a new > > driver submission it's about 15 minutes to scroll through patches and > > drop a few comments/suggestions on it. > > > > So much less review than for drm core, and I think somehow getting > > this off the ground would be really good for the community overall. > > But if it doesn't work out, then I'm ok with dropping it, but I think > > we really should try first. I think drm core had similar troubles with > > review, and with drm-misc collaborations seems to improve already. > > I like the idea, but I'm concerned it would just be work for the sake > of work. I'm not sure how much value it provides. I'm worried it'll > just turn into acking for the sake of acking. For core driver changes > that are independent of anything shared, I'm not really sure what the > ack provides other than a way to slap an ack label on your patch > unless the reviewer really digs into the driver. I guess an extra set > of eyes never hurts. Yeah, it would be a bit of a checkbox step, but then there's not that many drivers which really only have 1 contributor. I think as soon as you have 2-3, group maintainership with real peer review within the group should be the expectation, whether that's within drm-misc or in a separate driver repo doesn't matter. And for single-contributor drivers I still think it's useful, both to share best practices (display hw all deals with the same problems in the end, so even when the register don't work the same, the overall logic often does), and to make sure that when there's more contributors there's no process change in switching to a real group maintainership model. And I'm happy to help out kickstarting a small-driver review market by reviewing patches in exchange for review on my cleanup/core/doc stuff :-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm: qxl: Drop misleading comment
On Fri, Jan 27, 2017 at 02:41:29PM -0200, Gabriel Krisman Bertazi wrote:
> Daniel Vetter writes:
> > On Fri, Jan 27, 2017 at 09:20:42AM +0100, Gerd Hoffmann wrote:
> >> On Do, 2017-01-26 at 23:05 -0200, Gabriel Krisman Bertazi wrote:
> >> > No longer true since commit 07f8d9bdb235 ("drm/qxl: add support for > 1
> >> > output"). qxl_num_crtc defaults to 4 and is configurable as a module
> >> > parameter.
> >>
> >> Picked up patches 1-3 for drm-qemu branch.
> >>
> >> Patch 4 doesn't apply and seems to have a dependency on out-of-tree
> >> changes, git am fails to do a 3way merge due to missing commits (trying
> >> to apply to 4.10-rc5).
> >
> > It needs the demidlayer that's already in drm-next ...
>
> Thank you guys for the review.
>
> Yes, my patches were based on drm-misc next branch, I should have
> mentioned that in the original patch. It actually depends on stuff
> already in drm-misc, so I won't be able to rebase just this to drm-qemu
> tree.
>
> Do you want to merge from drm-misc or should we push this one
> directly via drm-misc?
I guess I could ask Gerd whether we wants to be part of the experiment to
maintain small drivers in drm-misc, to avoid these kinds of coordination
issues? Since some of the patches are in drm-misc and some in the qemu
tree we need to wait for them to resync now (I'll send out another
drm-misc pull later today).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM Atomic property for color-space conversion
On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: > Hi, > > We're looking to enable the per-plane color management hardware in > Mali-DP with atomic properties, which has sparked some conversation > around how to handle YCbCr formats. > > As it stands today, it's assumed that a driver will implicitly "do the > right thing" to display a YCbCr buffer. > > YCbCr data often uses different gamma curves and signal ranges (e.g. > BT.609, BT.701, BT.2020, studio range, full-range), so its desirable > to be able to explicitly control the YCbCr to RGB conversion process > from userspace. > > We're proposing adding a "CSC" (color-space conversion) property to > control this - primarily per-plane for framebuffer->pipeline CSC, but > perhaps one per CRTC too for devices which have an RGB pipeline and > want to output in YUV to the display: > > Name: "CSC" > Type: ENUM | ATOMIC; > Enum values (representative): > "default": > Same behaviour as now. "Some kind" of YCbCr->RGB conversion > for YCbCr buffers, bypass for RGB buffers > "disable": > Explicitly disable all colorspace conversion (i.e. use an > identity matrix). > "YCbCr to RGB: BT.709": > Only valid for YCbCr formats. CSC in accordance with BT.709 > using [16..235] for (8-bit) luma values, and [16..240] for > 8-bit chroma values. For 10-bit formats, the range limits are > multiplied by 4. > "YCbCr to RGB: BT.709 full-swing": > Only valid for YCbCr formats. CSC in accordance with BT.709, > but using the full range of each channel. We already have a standardized property for broadcast vs. full range. So needs to be taken out of this one here to avoid silly interactions. It's not yet atomic-ified though (i.e. not tracked in drm_connector_state). It's a bit annoying since it's on the connector, but with sufficient glue we can fix this. Your helper should probably take that into account too. > "YCbCr to RGB: Use CTM":* > Only valid for YCbCr formats. Use the matrix applied via the > plane's CTM property > "RGB to RGB: Use CTM":* > Only valid for RGB formats. Use the matrix applied via the > plane's CTM property > "Use CTM":* > Valid for any format. Use the matrix applied via the plane's > CTM property > ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as > they are required. > > *This assumes color-management is enabled per-plane. We're currently > working on patches to add this mostly to be able to use per-plane > degamma, but it would be analogous to the CRTC color-management code > and so also be able to expose a per-plane CTM property. > > Our hardware implements the color-space conversion as a 3x3 matrix, so > we can implement a helper to convert a CSC enum value to a CTM matrix > for use by any hardware which has a programmable CSC matrix. For any > other hardware, the driver simply needs to map the enum value to > whatever selector bits are available. > > It's expected that the "Use CTM" value(s) are *not* the common case, > and most of the time userspace will use one of the provided "standard" > enum values. The three different flavours of "Use CTM" allow us to > support hardware whose CSC hardware can only be used on e.g. YCbCr > data. > > Drivers can of course filter the enum list to expose whichever subset > the hardware can support. > > Having thrashed this out a bit on IRC with Ville, I think the above > approach is flexible enough to support at least Mali-DP and i915, > without burdening userspace any more than necessary. It also provides > the "default" behaviour which is backwards compatible, and allows for > fully custom CSC matrices where that is supported. > > We can obviously implement this as a Mali-DP driver private property, > but it would be good to come up with something to go into the core if > possible. It needs userspace either way :-) And I think the tricky bit here is interaction with the CTM stuff we already have, i.e. what happens when userspace sets both this and the CTM? Would our helper need to be able to merge the explicit CTM with the implicit one here with some matrix multiplication? If we go with "kernel multiplies them" then we could simplify things a lot I think, since all we'd need is just "none, BT.709, BT.601, ..." and none of the special values that define interactions with the CTM. Or we just chicken out for now and say you can't have a per-plane CTM if you expose this :-) Since there's no per-plane CTM userspace yet, this is probably the simplest option ... -Daniel > > I'd be happy to hear any feedback, > > Thanks, > Brian > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.fr
Re: [PATCH] MAINTAINERS: add dma-fence* files to Sync File maintainership
On Fri, Jan 27, 2017 at 03:54:44PM -0200, Gustavo Padovan wrote: > From: Gustavo Padovan > > As Sync File is highly dependent on dma-fence* tracks it > under SYNC FILE_FRAMEWORK as well. > > Signed-off-by: Gustavo Padovan Acked-by: Daniel Vetter I guess wait for an ack from Sumit, then push. -Daniel > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index bdc4843..c1c000d 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3976,6 +3976,7 @@ S: Maintained > L: linux-me...@vger.kernel.org > L: dri-devel@lists.freedesktop.org > F: drivers/dma-buf/sync_* > +F: drivers/dma-buf/dma-fence* > F: drivers/dma-buf/sw_sync.c > F: include/linux/sync_file.h > F: include/uapi/linux/sync_file.h > -- > 2.9.3 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 0/3] some -pro patches for integration
Reviewed-by: Christian König . Am 29.01.2017 um 23:51 schrieb Edward O'Callaghan: This series is, Reviewed-by: Edward O'Callaghan On 01/29/2017 06:49 AM, Grazvydas Ignotas wrote: I've taken several patches from amdgpu-pro libdrm that look useful to me and I think can be applied already. The only things I did was rebasing, fixing some typos and dropping Change-Id. Alex Xie (3): amdgpu: Free/uninit vamgr_32 in theoretically correct order amdgpu: vamgr can be a struct instead of a pointer amdgpu: vamgr_32 can be a struct instead of a pointer amdgpu/amdgpu_device.c | 24 +++- amdgpu/amdgpu_internal.h | 4 ++-- amdgpu/amdgpu_vamgr.c| 6 +++--- 3 files changed, 12 insertions(+), 22 deletions(-) ___ amd-gfx mailing list amd-...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/2] drm: Add support for tiny LCD displays
Hi Noralf, On Fri, Jan 27, 2017 at 08:56:29PM +0100, Noralf Trønnes wrote: > This is an attempt at providing a DRM version of drivers/staging/fbtft. > > The tinydrm library provides a very simplified view of DRM in particular > for tiny displays that has onboard video memory and is connected through > a slow bus like SPI/I2C. > > Only core patches this time. > > > Noralf. > > > Changes since version 1: > - Add tinydrm.rst > - Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that). Hm, this sounds like a buglet in the drm framework ... how do we call lastclose when the driver is disappearing? I do see a drm_lastclose call at the beginning of drm_dev_unregister (which we might want to remove for KMS drivers, it doesn't make much sense imo), but that shouldn't result in troubles. > - Remove some DRM_DEBUG*() > - Write-combined memory has uncached reads, so speed up by copying/buffering > one pixel line before conversion. Hm, why are you using write-combining memory? Or is that needed so that you can (if available) use hw spi engines? Either way, I think this all looks good, pls submit a pull request to Dave with these two patches as soon as latest drm-misc has landed (I'll send a pull request for that later today). Another one: Do you want to maintain tinydrm as part of the drm-misc group, i.e. want commit rights there? That would also help a bit with pushing all your great drm refactoring patches through the machinery ... Cheers, Daniel > > > Noralf Trønnes (2): > drm: Add DRM support for tiny LCD displays > drm/tinydrm: Add helper functions > > Documentation/gpu/index.rst| 1 + > Documentation/gpu/tinydrm.rst | 30 ++ > drivers/gpu/drm/Kconfig| 2 + > drivers/gpu/drm/Makefile | 1 + > drivers/gpu/drm/tinydrm/Kconfig| 8 + > drivers/gpu/drm/tinydrm/Makefile | 1 + > drivers/gpu/drm/tinydrm/core/Makefile | 3 + > drivers/gpu/drm/tinydrm/core/tinydrm-core.c| 377 > drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c | 462 > + > drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c| 234 + > include/drm/tinydrm/tinydrm-helpers.h | 100 ++ > include/drm/tinydrm/tinydrm.h | 115 ++ > 12 files changed, 1334 insertions(+) > create mode 100644 Documentation/gpu/tinydrm.rst > create mode 100644 drivers/gpu/drm/tinydrm/Kconfig > create mode 100644 drivers/gpu/drm/tinydrm/Makefile > create mode 100644 drivers/gpu/drm/tinydrm/core/Makefile > create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-core.c > create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-helpers.c > create mode 100644 drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c > create mode 100644 include/drm/tinydrm/tinydrm-helpers.h > create mode 100644 include/drm/tinydrm/tinydrm.h > > -- > 2.10.2 > > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/edid: Complete CEA modedb(VIC 1-107)
Regards Shashank On 1/30/2017 1:30 PM, Andrzej Hajda wrote: On 29.01.2017 06:41, Shashank Sharma wrote: CEA-861-F specs defines new 4k video modes to be used with HDMI 2.0 EDIDs. These modes start at VIC=93 and go all the way till VIC=107. Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now to be able to parse 4k modes using the existing techniques, we have to complete the modedb (VIC=65 onwards). This patch adds: - Timings for existing CEA video modes (from VIC=65 till VIC=92) - Newly added 4k modes (from VIC=93 to VIC=107). Cc: Jose Abreu Cc: Alex Deucher Cc: Andrzej Hajda V2: Addressed review comments from Jose: - fix the timings for VIC 83, 90 and 91 - fix formatting for VIC 93-107 V3: Rebase on drm-tip Signed-off-by: Shashank Sharma Signed-off-by: Sonika Jindal Reviewed-by: Jose Abreu Reviewed-by: Alex Deucher --- Have you addressed in some other patch my concerns regarding polluting infoframes generated for HDMI 1.4 devices (CEA-861-E) with VICs specific only for HDMI 2.0 (CEA-861-F) ? It will happen for all UHD modes, see [1][2] for previous discussion. Hello Andrzej As you already know, the aspect ratio is indexed with the VIC, which comes from EDID. Now, why would a HDMI 1.4 monitor, contain a HDMI 2.0 VIC ? If it does, its not compliant. - Shashank [1]: https://www.spinics.net/lists/intel-gfx/msg110524.html [2]: https://www.spinics.net/lists/intel-gfx/msg110529.html Regards Andrzej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/edid: Complete CEA modedb(VIC 1-107)
Fixed typo while adding Ville's mail address. Regards Shashank On 1/30/2017 2:15 PM, Sharma, Shashank wrote: Regards Shashank On 1/30/2017 1:30 PM, Andrzej Hajda wrote: On 29.01.2017 06:41, Shashank Sharma wrote: CEA-861-F specs defines new 4k video modes to be used with HDMI 2.0 EDIDs. These modes start at VIC=93 and go all the way till VIC=107. Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now to be able to parse 4k modes using the existing techniques, we have to complete the modedb (VIC=65 onwards). This patch adds: - Timings for existing CEA video modes (from VIC=65 till VIC=92) - Newly added 4k modes (from VIC=93 to VIC=107). Cc: Jose Abreu Cc: Alex Deucher Cc: Andrzej Hajda V2: Addressed review comments from Jose: - fix the timings for VIC 83, 90 and 91 - fix formatting for VIC 93-107 V3: Rebase on drm-tip Signed-off-by: Shashank Sharma Signed-off-by: Sonika Jindal Reviewed-by: Jose Abreu Reviewed-by: Alex Deucher --- Have you addressed in some other patch my concerns regarding polluting infoframes generated for HDMI 1.4 devices (CEA-861-E) with VICs specific only for HDMI 2.0 (CEA-861-F) ? It will happen for all UHD modes, see [1][2] for previous discussion. Hello Andrzej As you already know, the aspect ratio is indexed with the VIC, which comes from EDID. Now, why would a HDMI 1.4 monitor, contain a HDMI 2.0 VIC ? If it does, its not compliant. - Shashank [1]: https://www.spinics.net/lists/intel-gfx/msg110524.html [2]: https://www.spinics.net/lists/intel-gfx/msg110529.html Regards Andrzej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 16/19] drm/vc4: Remove vc4_debugfs_cleanup()
On Fri, Jan 27, 2017 at 09:56:07AM -0800, Eric Anholt wrote: > Noralf Trønnes writes: > > > drm_debugfs_cleanup() now removes all minor->debugfs_list entries > > automatically, so the drm_driver.debugfs_cleanup callback is not > > needed. > > Nice! > > Reviewed-by: Eric Anholt Applied to drm-misc, 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
Re: [PATCH 1/2] Revert "drm: omapdrm: Let the DRM core skip plane commit on inactive CRTCs"
On 28/01/17 18:11, Laurent Pinchart wrote: > Hi Jyri, > > Thank you for the patch. > > On Friday 27 Jan 2017 12:04:54 Jyri Sarha wrote: >> This reverts commit dadf4659d0608e034b6633f30300c2eff2dafb4c. >> >> If planes are not disabled when the they are not on any crtc anymore >> they will remain active and may show as "ghosts" when the crtc they >> were last on is active again. > > Sorry for the breakage. > > The drm_atomic_helper_commit_planes() helper documentation states > > * Unless otherwise needed, drivers are advised to set the ACTIVE_ONLY flag in > * @flags in order not to receive plane update notifications related to a > * disabled CRTC. This avoids the need to manually ignore plane updates in > * driver code when the driver and/or hardware can't or just don't need to > * deal with updates on disabled CRTCs, for example when supporting runtime > * PM. > > I wonder what this implies when CRTCs are being disabled. I see very few > cases > where the hardware wouldn't need the plane atomic disable operation being > called when a plane is being disabled due to its CRTC being disabled. Maybe > this should thus be addressed in the core. Daniel, any comment on this ? Similar change was done to mali in "drm: mali-dp: Don't set DRM_PLANE_COMMIT_ACTIVE_ONLY", so at least omapdrm is not alone here. I also wonder if this behavior is correct. Tomi signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
On 28/01/17 18:17, Laurent Pinchart wrote: > Hi Jyri, > > Thank you for the patch. > > On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote: >> Move drm_atomic_helper_commit_modeset_enables() call to before >> drm_atomic_helper_commit_planes() call and have a >> omap_atomic_wait_for_completion() call after both. >> >> With the current dss dispc implementation we have to enable the new >> modeset before we can commit planes. The dispc ovl configuration >> relies on the video mode configuration been written into the HW when >> the ovl configuration is calculated. >> >> This approach is not ideal because after a mode change the plane >> update is executed only after the first vblank interrupt. The dispc >> implementation should be fixed so that it is able use uncommitted drm >> state information. information. >> >> Signed-off-by: Jyri Sarha >> --- >> drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++- >> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct >> omap_atomic_state_commit *commit) dispc_runtime_get(); >> >> drm_atomic_helper_commit_modeset_disables(dev, old_state); >> -drm_atomic_helper_commit_planes(dev, old_state, 0); >> + >> +/* With the current dss dispc implementation we have to enable >> + * the new modeset before we can commit planes. The dispc ovl >> + * configuration relies on the video mode configuration been >> + * written into the HW when the ovl configuration is >> + * calculated. >> + * >> + * This approach is not ideal because after a mode change the >> + * plane update is executed only after the first vblank >> + * interrupt. The dispc implementation should be fixed so that >> + * it is able use uncommitted drm state information. > > Any chance you could fix the dispc implementation ? ;-) Otherwise, could you Dispc change is on our todo, but the size of the change won't fit into "fix" definition =). There's a lot of state that the dispc side is looking at, which at the moment doesn't come from drm side. Tomi signature.asc Description: OpenPGP digital signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm-misc-next
Hi Dave, Another round of -misc stuff: - Noralf debugfs cleanup cleanup (not yet everything, some more driver patches awaiting acks). - More doc work. - edid/infoframe fixes from Ville. - misc 1-patch fixes all over, as usual Noralf needs this for his tinydrm pull request. And as discussed last week on irc, please backmerge -rc6: - drm-intel needs it for 4.11 gvt fixes (due to the late refactor in virtio code from Alex) - drm-misc needs it for a cleanup patch from Takashi for fixes in -rc. - we need it to resolve the connector_list vs. nouveau mess. Cheers, Daniel The following changes since commit 6f897f51c4181397bf811d260eb7fef8d7ccd14f: drm: qxl: Open code teardown function for qxl (2017-01-19 15:33:30 -0200) are available in the git repository at: git://anongit.freedesktop.org/git/drm-misc tags/drm-misc-next-2017-01-30 for you to fetch changes up to 55d6616585363ff2aa9430bfe1ff1345bdc0599a: drm/vc4: Remove vc4_debugfs_cleanup() (2017-01-30 09:48:48 +0100) Arnd Bergmann (1): drm: bridge: dw-hdmi: fix building without CONFIG_OF Chris Wilson (6): lib/prime_numbers: Suppress warn on kmalloc failure drm: Show leaked connectors upon unload drm: Update drm_cache.c to pull in the new drm_cache.h drm/vgem: Switch to reservation_object_lock() helpers drm: Silence the compiler for drm_mode_get_hv_timings() dma/fence: Export enable-signaling tracepoint for emission by drivers Daniel Vetter (14): drm/kms-helpers: Use recommened kerneldoc for struct member refs drm/gem|prime|mm: Use recommened kerneldoc for struct member refs drm/core: Use recommened kerneldoc for struct member refs drm/kms-core: Use recommened kerneldoc for struct member refs drm/gma500: Nuke device_is_agp callback drm: Update kerneldoc for drm_crtc.[hc] drm/doc: Clarify connector overview drm/i810: drop device_is_agp callback drm/mga: remove device_is_agp callback drm: remove device_is_agp callback drm: Nuke ums vgaarb support drm/moc: Mark legacy fields in drm_driver as such drm: s/drm_crtc_get_hv_timings/drm_mode_get_hv_timings/ drm/doc: Fix typos for early_unregister doc Dhinakaran Pandiyan (1): drm/dp: Store drm_device in MST topology manager Michel Dänzer (1): drm/ttm: Make sure BOs being swapped out are cacheable Noralf Trønnes (15): drm/fb-cma-helper: Add drm_fbdev_cma_set_suspend_unlocked() drm/simple-helpers: Add missing includes drm: debugfs: Remove all files automatically on cleanup drm: drm_minor_register(): Clean up debugfs on failure drm/atomic: Remove drm_atomic_debugfs_cleanup() drm/amd/amdgpu: Remove drm_debugfs_remove_files() call drm/etnaviv: allow build with COMPILE_TEST drm/etnaviv: Remove etnaviv_debugfs_cleanup() drm/hdlcd: Remove hdlcd_debugfs_cleanup() drm/omap: Remove omap_debugfs_cleanup() drm/radeon: Remove drm_debugfs_remove_files() call drm/sti: Remove drm_debugfs_remove_files() calls drm/tegra: Remove tegra_debugfs_cleanup() drm/tilcdc: Remove tilcdc_debugfs_cleanup() drm/vc4: Remove vc4_debugfs_cleanup() Oleksandr Andrushchenko (1): drm/prime: Clarify DMA-BUF/GEM Object lifetime Philipp Zabel (1): drm/fourcc: add vivante tiled layout format modifiers Tobias Jakobi (1): drm/exynos: Remove Kconfig deps for FIMD and DECON7 Ville Syrjälä (5): drm/edid: Have drm_edid.h include hdmi.h drm/edid: Introduce drm_default_rgb_quant_range() drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range() drm/edid: Set AVI infoframe Q even when QS=0 drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F Wei Yongjun (2): drm/atomic: make release_crtc_commit() static drm/hisilicon/hibmc: Fix wrong pointer passed to PTR_ERR() Documentation/gpu/drm-kms.rst | 8 +- drivers/dma-buf/dma-fence.c | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 - drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 20 --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 - drivers/gpu/drm/arm/hdlcd_drv.c | 7 -- drivers/gpu/drm/bridge/dw-hdmi.c | 2 + drivers/gpu/drm/drm_agpsupport.c | 2 + drivers/gpu/drm/drm_atomic.c | 85 ++--- drivers/gpu/drm/drm_atomic_helper.c | 101 --- drivers/gpu/drm/drm_auth.c| 4 +- drivers/gpu/drm/drm_blend.c | 11 +- drivers/gpu/drm/drm_cache.c | 4 +- drivers/gpu/drm/drm_connector.c | 21 ++-- drivers/gpu/drm/drm_crtc.c| 51 drivers/gpu/drm/drm_crtc_helper.c | 28 ++--- drivers/gpu/drm/drm_crtc_internal.h |
Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup
On Fri, Jan 27, 2017 at 03:05:46PM +0100, Daniel Vetter wrote: > On Fri, Jan 27, 2017 at 10:36:16AM +0100, Thierry Reding wrote: > > On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote: > > > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote: > > > > This patchset removes the need for drivers to clean up their debugfs > > > > files on exit. It is done automatically in drm_debugfs_cleanup(). > > > > This funtion is also called should the driver error out in it's > > > > drm_driver.debugfs_init callback. > > > > > > > > Two drivers still use drm_debugfs_remove_files(): > > > > - tegra in it's connectors, not sure if I can remove it. > > > > > > I read through them, and they're removed on the component device nodes > > > stuff. That looks somewhat fishy from a lifetime point of view, and I > > > think removing all that code would be better, too. > > > > What makes you think that's problematic from a lifetime point of view? > > The component device is tied to the DRM device, so these callbacks are > > called at the right time. > > debugfs is a userspace interface, which should disappear when > drm_dev_unregister gets called. I'm not sure at all whether that lines up > with the cleanup of all your component nodes, but otoh it's rather > academic since you can't hotplug a tegra. > > > That said, I think it's safe to remove the other debugfs files from > > Tegra. It might not be possible to remove the cleanup functions > > altogether, though, because they have to do a special dance involving > > kmemdup() drm_debugfs_create_files() and kfree() in order to support > > debugfs files for multiple instances of subdevices. > > Hm, that entire "do debugfs on the minor" thing makes almost never sense. > All the things we have left in modern drivers are either per-fd, or > per-device. Nothing of interest is per-minor. Or do you mean something > else? I'm not sure I understand what you're saying. We have plenty of code that adds debugfs files to the connector's debugfs entry. And that's within the minor's debugfs root. Am I missing something? Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup
On Mon, Jan 30, 2017 at 09:58:48AM +0100, Thierry Reding wrote: > On Fri, Jan 27, 2017 at 03:05:46PM +0100, Daniel Vetter wrote: > > On Fri, Jan 27, 2017 at 10:36:16AM +0100, Thierry Reding wrote: > > > On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote: > > > > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote: > > > > > This patchset removes the need for drivers to clean up their debugfs > > > > > files on exit. It is done automatically in drm_debugfs_cleanup(). > > > > > This funtion is also called should the driver error out in it's > > > > > drm_driver.debugfs_init callback. > > > > > > > > > > Two drivers still use drm_debugfs_remove_files(): > > > > > - tegra in it's connectors, not sure if I can remove it. > > > > > > > > I read through them, and they're removed on the component device nodes > > > > stuff. That looks somewhat fishy from a lifetime point of view, and I > > > > think removing all that code would be better, too. > > > > > > What makes you think that's problematic from a lifetime point of view? > > > The component device is tied to the DRM device, so these callbacks are > > > called at the right time. > > > > debugfs is a userspace interface, which should disappear when > > drm_dev_unregister gets called. I'm not sure at all whether that lines up > > with the cleanup of all your component nodes, but otoh it's rather > > academic since you can't hotplug a tegra. > > > > > That said, I think it's safe to remove the other debugfs files from > > > Tegra. It might not be possible to remove the cleanup functions > > > altogether, though, because they have to do a special dance involving > > > kmemdup() drm_debugfs_create_files() and kfree() in order to support > > > debugfs files for multiple instances of subdevices. > > > > Hm, that entire "do debugfs on the minor" thing makes almost never sense. > > All the things we have left in modern drivers are either per-fd, or > > per-device. Nothing of interest is per-minor. Or do you mean something > > else? > > I'm not sure I understand what you're saying. We have plenty of code > that adds debugfs files to the connector's debugfs entry. And that's > within the minor's debugfs root. > > Am I missing something? Per-connector entries are fine, per-minor imo not. This is a historical accident, but it also doesn't really hurt anyone. I think it'd make much more sense to move everything into a per-devices entry (with maybe backwards compat links from minor to devices). But really, this is 100% orthogonal to the cleanup here. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup
On Mon, Jan 30, 2017 at 10:03:44AM +0100, Daniel Vetter wrote: > On Mon, Jan 30, 2017 at 09:58:48AM +0100, Thierry Reding wrote: > > On Fri, Jan 27, 2017 at 03:05:46PM +0100, Daniel Vetter wrote: > > > On Fri, Jan 27, 2017 at 10:36:16AM +0100, Thierry Reding wrote: > > > > On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote: > > > > > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote: > > > > > > This patchset removes the need for drivers to clean up their debugfs > > > > > > files on exit. It is done automatically in drm_debugfs_cleanup(). > > > > > > This funtion is also called should the driver error out in it's > > > > > > drm_driver.debugfs_init callback. > > > > > > > > > > > > Two drivers still use drm_debugfs_remove_files(): > > > > > > - tegra in it's connectors, not sure if I can remove it. > > > > > > > > > > I read through them, and they're removed on the component device nodes > > > > > stuff. That looks somewhat fishy from a lifetime point of view, and I > > > > > think removing all that code would be better, too. > > > > > > > > What makes you think that's problematic from a lifetime point of view? > > > > The component device is tied to the DRM device, so these callbacks are > > > > called at the right time. > > > > > > debugfs is a userspace interface, which should disappear when > > > drm_dev_unregister gets called. I'm not sure at all whether that lines up > > > with the cleanup of all your component nodes, but otoh it's rather > > > academic since you can't hotplug a tegra. > > > > > > > That said, I think it's safe to remove the other debugfs files from > > > > Tegra. It might not be possible to remove the cleanup functions > > > > altogether, though, because they have to do a special dance involving > > > > kmemdup() drm_debugfs_create_files() and kfree() in order to support > > > > debugfs files for multiple instances of subdevices. > > > > > > Hm, that entire "do debugfs on the minor" thing makes almost never sense. > > > All the things we have left in modern drivers are either per-fd, or > > > per-device. Nothing of interest is per-minor. Or do you mean something > > > else? > > > > I'm not sure I understand what you're saying. We have plenty of code > > that adds debugfs files to the connector's debugfs entry. And that's > > within the minor's debugfs root. > > > > Am I missing something? > > Per-connector entries are fine, per-minor imo not. Most, if not all, debugfs files in Tegra a per-connector. We have a couple that are per-CRTC. And then we have two files that are on the minor, which is something I had copied from i915, if I remember correctly, though I can't seem to find the original anymore. Maybe that was moved somewhere else in the meantime? > This is a historical accident, but it also doesn't really hurt anyone. > I think it'd make much more sense to move everything into a > per-devices entry (with maybe backwards compat links from minor to > devices). With per-device entries you mean rooted at the device backing the CRTC, encoder, connector, ...? > But really, this is 100% orthogonal to the cleanup here. If we want to get rid of the remainder of the cleanup, then it's not entirely orthogonal anymore. =) Not to say that this cleanup isn't useful in its own right. Thierry signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: Don't race connector registration
On Thu, Jan 26, 2017 at 12:34:29PM -0800, Dave Hansen wrote:
> On 01/25/2017 07:38 AM, Daniel Vetter wrote:
> > On Wed, Jan 25, 2017 at 07:20:45AM -0800, Dave Hansen wrote:
> >> On 01/24/2017 10:21 PM, Daniel Vetter wrote:
> >>> Hi Dave,
> >>>
> >>> Still waiting for your testing results on this one here ...
> >>
> >> It's definitely stable with that patch applied. No more crashes.
> >>
> >> But, it's also definitely having difficulty re-probing to find the
> >> monitor that's attached to the dock in some cases. Whatever is going on
> >> isn't fixed by poking it with xrandr.
> >
> > Is this new? When exactly does this happen? Does the mst sink connector no
> > longer show up, or is the connected/disconnected status all wrong?
>
> It's hard to say whether it's new or not. I *think* it worked better
> before, but it also crashed pretty often, so it's hard to say.
Ok, I guess that's good enough to push at least the crash fix forward.
> And, yeah, I think it just gets the connected status wrong. The
> connector is still there.
Hm, I thought I replied here but I didn't:
- Is this just after boot (and then the connector is stuck forever), or
starts to happen after suspend/resume, or some other system change like
that? Or do they just crop up eventually?
- Does this only happen once the connector is destroyed? Please trace
intel_dp_destroy_mst_connector with something like:
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c
b/drivers/gpu/drm/i915/intel_dp_mst.c
index 38e3ca2f6f18..24ac2d1ce3ad 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -502,6 +502,8 @@ static void intel_dp_destroy_mst_connector(struct
drm_dp_mst_topology_mgr *mgr,
drm_connector_unregister(connector);
+ printk("mst connector getting destroyed: %s\n", connector->name);
+
/* need to nuke the connector */
drm_modeset_lock_all(dev);
intel_connector_remove_from_fbdev(intel_connector);
- If it's not that then something in intel_dp_mst_detect (well, it's
helper implementation drm_dp_mst_detect_port) is probably going wrong.
Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v3 PATCH 0/3] Allow ASYNC flip with atomic helpers.
Hi Andrey, Unrelated suggestion: A handy trick - to save yourself a bit of time (and "get it right") you can use `git format-patch -vX ...' [it also works with send-email] to have the version in the subject prefix. Feel free to share it with the team - it seems that many people manually edit the patch(es). -Emil ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/edid: Complete CEA modedb(VIC 1-107)
On 30.01.2017 09:45, Sharma, Shashank wrote: > Regards > > Shashank > > > On 1/30/2017 1:30 PM, Andrzej Hajda wrote: >> On 29.01.2017 06:41, Shashank Sharma wrote: >>> CEA-861-F specs defines new 4k video modes to be used with >>> HDMI 2.0 EDIDs. These modes start at VIC=93 and go all the >>> way till VIC=107. >>> >>> Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now >>> to be able to parse 4k modes using the existing techniques, we have >>> to complete the modedb (VIC=65 onwards). >>> >>> This patch adds: >>> - Timings for existing CEA video modes (from VIC=65 till VIC=92) >>> - Newly added 4k modes (from VIC=93 to VIC=107). >>> >>> Cc: Jose Abreu >>> Cc: Alex Deucher >>> Cc: Andrzej Hajda >>> >>> V2: Addressed review comments from Jose: >>> - fix the timings for VIC 83, 90 and 91 >>> - fix formatting for VIC 93-107 >>> >>> V3: Rebase on drm-tip >>> >>> Signed-off-by: Shashank Sharma >>> Signed-off-by: Sonika Jindal >>> Reviewed-by: Jose Abreu >>> Reviewed-by: Alex Deucher >>> --- >> Have you addressed in some other patch my concerns regarding polluting >> infoframes generated for HDMI 1.4 devices (CEA-861-E) with VICs specific >> only for HDMI 2.0 (CEA-861-F) ? It will happen >> for all UHD modes, see [1][2] for previous discussion. > Hello Andrzej > > As you already know, the aspect ratio is indexed with the VIC, which > comes from EDID. > Now, why would a HDMI 1.4 monitor, contain a HDMI 2.0 VIC ? If it does, > its not compliant. Have you forgot our previous discussion? Let me cite myself: For example 3840x2160@30Hz has no VIC in HDMI 1.4 but it can be present in HDMI vendor specific block with HDMI_VIC 1, on the other side it has VIC 95 in HDMI 2.0. So before your patch AVI infoframe.video_code is set to 0, after your patch is set to 95. And your discussion with Ville: > >/>>> No. The user is free to specify any mode they wish. It doesn't > have to/ > >/>>> come from the EDID. Not sure specifying a modern VIC for an older/ > >/>>> display is a good idea or not. If not, we could always check the > cea_rev/ > >/>>> assuming it changed whenever the list if VICs was expanded./ > >/>> I agree, that user can specify a mode, out of EDID too./ > >/>> I am anyways planning to add a patch, where before loading HDMI 2.0 > VICs/ > >/>> in AVI IF, we are checking the EDID rev./ > >/>> That should solve our problem./ > >/>>/ > >/>> So if edid_rev < 2.0/ > >/>> do_not_load VICs from 93 - 107, but keep it 0./ > >/> Why edid_rev and not cea_rev?/ > >/We can do that also, but in the current structure, we are already / > >/caching EDID rev, its just about re-using it./ > >/AFAIK, we don't have CEA extension cached anywhere./ > > Yes we do. It does not seem to be addressed. Regards Andrzej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: RFC: drm-misc for small drivers?
Hi, > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html apache throws 403. cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm: qxl: Drop misleading comment
Hi, > I guess I could ask Gerd whether we wants to be part of the experiment to > maintain small drivers in drm-misc, to avoid these kinds of coordination > issues? Sounds reasonable. There isn't much activity, that's why I've grouped all four qemu drivers into a single branch already. The docs throw 403 access denied though atm. So, how would that process look like then? Would you merge my drm-qemu pulls into drm-misc? Or would I commit to drm-misc directly? cheers, Gerd ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/edid: Complete CEA modedb(VIC 1-107)
Regards Shashank On 1/30/2017 2:52 PM, Andrzej Hajda wrote: On 30.01.2017 09:45, Sharma, Shashank wrote: Regards Shashank On 1/30/2017 1:30 PM, Andrzej Hajda wrote: On 29.01.2017 06:41, Shashank Sharma wrote: CEA-861-F specs defines new 4k video modes to be used with HDMI 2.0 EDIDs. These modes start at VIC=93 and go all the way till VIC=107. Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now to be able to parse 4k modes using the existing techniques, we have to complete the modedb (VIC=65 onwards). This patch adds: - Timings for existing CEA video modes (from VIC=65 till VIC=92) - Newly added 4k modes (from VIC=93 to VIC=107). Cc: Jose Abreu Cc: Alex Deucher Cc: Andrzej Hajda V2: Addressed review comments from Jose: - fix the timings for VIC 83, 90 and 91 - fix formatting for VIC 93-107 V3: Rebase on drm-tip Signed-off-by: Shashank Sharma Signed-off-by: Sonika Jindal Reviewed-by: Jose Abreu Reviewed-by: Alex Deucher --- Have you addressed in some other patch my concerns regarding polluting infoframes generated for HDMI 1.4 devices (CEA-861-E) with VICs specific only for HDMI 2.0 (CEA-861-F) ? It will happen for all UHD modes, see [1][2] for previous discussion. Hello Andrzej As you already know, the aspect ratio is indexed with the VIC, which comes from EDID. Now, why would a HDMI 1.4 monitor, contain a HDMI 2.0 VIC ? If it does, its not compliant. Have you forgot our previous discussion? Let me cite myself: For example 3840x2160@30Hz has no VIC in HDMI 1.4 but it can be present in HDMI vendor specific block with HDMI_VIC 1, on the other side it has VIC 95 in HDMI 2.0. So before your patch AVI infoframe.video_code is set to 0, after your patch is set to 95. And your discussion with Ville: />>> No. The user is free to specify any mode they wish. It doesn't have to/ />>> come from the EDID. Not sure specifying a modern VIC for an older/ />>> display is a good idea or not. If not, we could always check the cea_rev/ />>> assuming it changed whenever the list if VICs was expanded./ />> I agree, that user can specify a mode, out of EDID too./ />> I am anyways planning to add a patch, where before loading HDMI 2.0 VICs/ />> in AVI IF, we are checking the EDID rev./ />> That should solve our problem./ />>/ />> So if edid_rev < 2.0/ />> do_not_load VICs from 93 - 107, but keep it 0./ /> Why edid_rev and not cea_rev?/ /We can do that also, but in the current structure, we are already / /caching EDID rev, its just about re-using it./ /AFAIK, we don't have CEA extension cached anywhere./ Yes we do. It does not seem to be addressed. Regards Andrzej Hey, thanks for bringing this out again. I checked the specs again, there are three things here: - The spec has a section on this, and it states that in such cases CEA VIC should be supplied to the AVI IF. - Also, for HDMI infoframe blocks, we dont refer to cea_edid_modes[] rather we go to edid_4k_modes[] - Again, as there is no aspect ratio parsing layer in DRM right now (after the revert), we are doing the right thing to give aspect as per CEA, which is suggested by spec too. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: RFC: drm-misc for small drivers?
On Mon, Jan 30, 2017 at 10:30:43AM +0100, Gerd Hoffmann wrote: > Hi, > > > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/drm-misc.html > > https://01.org/linuxgraphics/gfx-docs/maintainer-tools/dim.html > > apache throws 403. We're looking into it. Adding Tomi, who's herding the autobuilder for these docs. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm: qxl: Drop misleading comment
On Mon, Jan 30, 2017 at 10:41:18AM +0100, Gerd Hoffmann wrote: > Hi, > > > I guess I could ask Gerd whether we wants to be part of the experiment to > > maintain small drivers in drm-misc, to avoid these kinds of coordination > > issues? > > Sounds reasonable. There isn't much activity, that's why I've grouped > all four qemu drivers into a single branch already. > > The docs throw 403 access denied though atm. So, how would that process > look like then? Would you merge my drm-qemu pulls into drm-misc? Or > would I commit to drm-misc directly? drm-misc runs with the committer model, i.e. a few maintainers to do pull requests and backmerges, a big pile of people directly pushing patches. Someone wreaked the entire 01.org domain, but you can get at all the tooling and documentation with git://anongit.freedesktop.org/drm-intel maintainer-tools And then run make in there. We're not yet clear how exactly drivers within drm-misc would look like (wrt which drivers and how much review and stuff like that), hence the RFC. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PULL] drm/atmel-hlcdc: Changes for 4.11
Hi Dave, Here is a tiny PR for 4.11. Regards, Boris The following changes since commit 7ce7d89f48834cefece7804d38fc5d85382edf77: Linux 4.10-rc1 (2016-12-25 16:13:08 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/bbrezillon/linux.git tags/drm/atmel-hlcdc/for-4.11 for you to fetch changes up to db02b7614a54bf0bf548db07bc8d3e7518fd9481: drm/atmel-hlcdc: Rework the fbdev creation logic (2017-01-30 10:49:34 +0100) Contains a single patch to create the fbdev at driver's registration time instead of waiting for the connector status change. Boris Brezillon (1): drm/atmel-hlcdc: Rework the fbdev creation logic drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 18 +++--- 1 file changed, 7 insertions(+), 11 deletions(-) ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: RFC: drm-misc for small drivers?
Hi Daniel, On Thu, 26 Jan 2017 18:08:42 +0100 Daniel Vetter wrote: > Hi all, > > We've discussed this a bit at LCA (with Dave and Eric), and it's > probably best if I just summarize all the questions and opens and > throw them out here for discussions: > > - When's a driver small enough for a shared tree, and when is a > separate tree a good idea? i915 and amdgpu are definitely big, and > there's definitely drivers who are really small and in-between it's > unclear. Personally I think this is easy to do with a sliding scale, > with using topic branches (we can do them in drm-misc easily) for > bigger stuff, and if that's a common thing, split out the driver > (thanks to the drm-tip integration tree there's not much of a > difference in handling conflicts due to that anyway). > > - Should it be an entire separate tree for soc drivers? Problem here > is that we lack a volunteer group (and imo it really should be a group > to avoid the single-maintainer troubles) to run that. I think it's > easier to proof the process first, and if we want a separate tree, > split that out later on. This is the same thing we've done with > drm-misc, first with a topic branch in drm-intel.git, then separate. I > think it worked really well. > > - Should we require review or at least acks for patches committed by > the author? We have a bunch of drivers with effectively just 1 person > working on it, where getting real review is hard. But otoh a few of > those 1-person drivers will become popular, and then it's good to > start with establishing peer-review early on. I also think that > requiring peer-review is good to share best practices and knowledge > between different people in our community, not just to make sure the > code is correct. For all these reasons I'm leaning towards not making > an exception for drivers, and requiring the same amount of review for > them if they go in through drm-misc as for any other patch. > > - Who's elligible? I think we could start small with a few volunteers > and their drivers, and then anyone who's willing. I'd be happy to have the atmel-hlcdc driver maintained in this drm-misc tree. I just had to send a PR containing a single patch for 4.11, and it really feels like these simple fixes/improvements patches do not deserve a dedicated PR (not to mention that sometime I forget to send the PR and miss a release :-)). Now, regarding the peer-review thing, I'm not against reviewing a few simple patches from time to time, but I don't think I'll have time to review entire new drivers, and I guess that's the kind of thing your looking for :-/. > > - Should we force new submissions to be managed in that shared treee? > I think for initial submission a separate pull request for > approval-by-Dave is good (but we could do that with topic branches > too). And it's also way too early to tell, probably better to first > figure out how well this goes. > > - CI, needed? It would be great, but we're not there yet :( Atm > drm-misc just has a bunch of defconfigs that need to always compile, > and that's it. Long term I definitely want more, but we're just not > there yet. And it's a problem in general for drm-misc. > > - dim scripts. Since we don't have a github flow where we can > reasonably automate stuff on the server side we need something to > automate on the client side. Thus far almost everyone seemed ok with > the scripting that's used to drive drm-misc/intel/tip, but we can > always improve things. And long term we can rework the approach > however we want to really. > > - Other stuff I've missed? > > Cheers, Daniel ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/edid: Complete CEA modedb(VIC 1-107)
On 30.01.2017 10:42, Sharma, Shashank wrote: > Regards > > Shashank > > > On 1/30/2017 2:52 PM, Andrzej Hajda wrote: >> On 30.01.2017 09:45, Sharma, Shashank wrote: >>> Regards >>> >>> Shashank >>> >>> >>> On 1/30/2017 1:30 PM, Andrzej Hajda wrote: On 29.01.2017 06:41, Shashank Sharma wrote: > CEA-861-F specs defines new 4k video modes to be used with > HDMI 2.0 EDIDs. These modes start at VIC=93 and go all the > way till VIC=107. > > Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now > to be able to parse 4k modes using the existing techniques, we have > to complete the modedb (VIC=65 onwards). > > This patch adds: > - Timings for existing CEA video modes (from VIC=65 till VIC=92) > - Newly added 4k modes (from VIC=93 to VIC=107). > > Cc: Jose Abreu > Cc: Alex Deucher > Cc: Andrzej Hajda > > V2: Addressed review comments from Jose: > - fix the timings for VIC 83, 90 and 91 > - fix formatting for VIC 93-107 > > V3: Rebase on drm-tip > > Signed-off-by: Shashank Sharma > Signed-off-by: Sonika Jindal > Reviewed-by: Jose Abreu > Reviewed-by: Alex Deucher > --- Have you addressed in some other patch my concerns regarding polluting infoframes generated for HDMI 1.4 devices (CEA-861-E) with VICs specific only for HDMI 2.0 (CEA-861-F) ? It will happen for all UHD modes, see [1][2] for previous discussion. >>> Hello Andrzej >>> >>> As you already know, the aspect ratio is indexed with the VIC, which >>> comes from EDID. >>> Now, why would a HDMI 1.4 monitor, contain a HDMI 2.0 VIC ? If it does, >>> its not compliant. >> Have you forgot our previous discussion? Let me cite myself: >> >> For example 3840x2160@30Hz has no VIC in HDMI 1.4 but it can >> be present in HDMI vendor specific block with HDMI_VIC 1, on the >> other side it has VIC 95 in HDMI 2.0. So before your patch >> AVI infoframe.video_code is set to 0, after your patch is set to 95. >> >> >> And your discussion with Ville: >> />>> No. The user is free to specify any mode they wish. It doesn't >>> have to/ />>> come from the EDID. Not sure specifying a modern VIC for an older/ />>> display is a good idea or not. If not, we could always check the >>> cea_rev/ />>> assuming it changed whenever the list if VICs was expanded./ />> I agree, that user can specify a mode, out of EDID too./ />> I am anyways planning to add a patch, where before loading HDMI 2.0 >>> VICs/ />> in AVI IF, we are checking the EDID rev./ />> That should solve our problem./ />>/ />> So if edid_rev < 2.0/ />> do_not_load VICs from 93 - 107, but keep it 0./ /> Why edid_rev and not cea_rev?/ /We can do that also, but in the current structure, we are already / /caching EDID rev, its just about re-using it./ /AFAIK, we don't have CEA extension cached anywhere./ >>> Yes we do. >> It does not seem to be addressed. >> >> Regards >> Andrzej > Hey, thanks for bringing this out again. I checked the specs again, > there are three things here: > - The spec has a section on this, and it states that in such cases CEA > VIC should be supplied to the AVI IF. This phrase is not precise, but if it means that in case of 3840x2160@30Hz source should set VIC to 95 also for HDMI1.4 compliant sinks it is in opposition to statement from HDMI1.4 specification: > When transmitting any extended video format indicated through use of > the HDMI_VIC field > in the HDMI Vendor Specific InfoFrame or any other format which is not > described in the > above cases, an HDMI Source shall set the AVI InfoFrame VIC field to zero. I do not think HDMI20 could invalidate claims from HDMI1.4. > - Also, for HDMI infoframe blocks, we dont refer to cea_edid_modes[] > rather we go to edid_4k_modes[] cea_edid_modes is used in generating AVI infoframe, edid_4k_modes is used in generating vendor infoframe > - Again, as there is no aspect ratio parsing layer in DRM right now > (after the revert), we are doing the right thing to give aspect as per > CEA, which is suggested by spec too. > I do not understand this phrase. Are you talking about aspect ratios or about video format? Regards Andrzej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH libdrm 2/3] tests/util: Make util_open() use drmDevice
From: Thierry Reding
The util_open() helper is used in a couple of test programs to open an
appropriate device. It takes a device path and a module name, both are
optional, as parameters. If a device path is specified, it will try to
open the given device. Otherwise it will try all available devices. If
only a specific subset is desired, the module parameter can be used as
a filter. The function will use it to open only devices whose kernel
driver matches the given module name.
Instead of relying on the legacy drmOpen() function to do this, convert
util_open() to use the new drmDevice helpers. This gets it functionally
much closer to what other DRM/KMS users, such as the X.Org Server or a
Wayland server, do.
Signed-off-by: Thierry Reding
---
tests/util/kms.c | 167 +--
tests/util/kms.h | 43 ++
2 files changed, 168 insertions(+), 42 deletions(-)
diff --git a/tests/util/kms.c b/tests/util/kms.c
index d866398237bb..c5d35ab616d1 100644
--- a/tests/util/kms.c
+++ b/tests/util/kms.c
@@ -42,15 +42,18 @@
#endif
#include
+#include
#include
#include
#include
#include
+#include
#include "xf86drm.h"
#include "xf86drmMode.h"
#include "common.h"
+#include "kms.h"
struct type_name {
unsigned int type;
@@ -125,58 +128,138 @@ const char *util_lookup_connector_type_name(unsigned int
type)
ARRAY_SIZE(connector_type_names));
}
-static const char * const modules[] = {
-"i915",
-"amdgpu",
-"radeon",
-"nouveau",
-"vmwgfx",
-"omapdrm",
-"exynos",
-"tilcdc",
-"msm",
-"sti",
-"tegra",
-"imx-drm",
-"rockchip",
-"atmel-hlcdc",
-"fsl-dcu-drm",
-"vc4",
-"virtio_gpu",
-"mediatek",
-"meson",
-};
+char *util_get_driver(int fd)
+{
+drmVersionPtr version;
+char *driver;
-int util_open(const char *device, const char *module)
+version = drmGetVersion(fd);
+if (!version)
+return NULL;
+
+driver = strdup(version->name);
+
+drmFreeVersion(version);
+
+return driver;
+}
+
+int util_get_devices(drmDevicePtr **devicesp, uint32_t flags)
+{
+drmDevicePtr *devices;
+unsigned int count;
+int err;
+
+err = drmGetDevices2(flags, NULL, 0);
+if (err < 0)
+return err;
+
+/*
+ * If the caller hasn't specified a return pointer for the new devices
+ * array, all of the below is pointless, so simply return the number of
+ * devices available.
+ */
+if (!devicesp)
+return err;
+
+count = err;
+
+devices = calloc(count, sizeof(*devices));
+if (!devices)
+return -ENOMEM;
+
+err = drmGetDevices2(flags, devices, count);
+if (err < 0) {
+free(devices);
+return err;
+}
+
+if (devicesp)
+*devicesp = devices;
+else
+free(devices);
+
+return count;
+}
+
+void util_free_devices(drmDevicePtr *devices, unsigned int count)
+{
+drmFreeDevices(devices, count);
+free(devices);
+}
+
+int util_open_with_module(const char *device, const char *module)
{
-int fd;
+int fd, err = 0;
+
+if (module)
+printf("trying to open `%s' with `%s'...", device, module);
+else
+printf("trying to open `%s'...", device);
+
+fd = open(device, O_RDWR);
+if (fd < 0) {
+err = -errno;
+goto out;
+}
if (module) {
-fd = drmOpen(module, device);
-if (fd < 0) {
-fprintf(stderr, "failed to open device '%s': %s\n",
-module, strerror(errno));
-return -errno;
+char *driver = util_get_driver(fd);
+if (!driver) {
+err = -EINVAL;
+goto close;
}
-} else {
-unsigned int i;
-for (i = 0; i < ARRAY_SIZE(modules); i++) {
-printf("trying to open device '%s'...", modules[i]);
+if (strcmp(module, driver) != 0)
+err = -EINVAL;
+
+free(driver);
+
+if (err < 0)
+goto close;
+}
+
+printf("done\n");
+return fd;
+
+close:
+close(fd);
+out:
+printf("failed\n");
+return err;
+}
-fd = drmOpen(modules[i], device);
-if (fd < 0) {
-printf("failed\n");
-} else {
-printf("done\n");
+int util_open(const char *device, const char *module)
+{
+int fd, err;
+
+if (!device) {
+drmDevicePtr *devices, dev;
+unsigned int count, i, j;
+const char *node;
+
+err = util_get_devices(&devices, 0);
+if (err < 0)
+return err;
+
+count = err;
+
+util_for_each_device(dev, i, devices, count) {
+node = util_device_get_node(dev, DRM_NODE_PRIMARY);
+if (!node)
+continue;
+
+fd = util_open_with_module(node, module);
+if (fd >= 0)
break;
-}
}
-i
[PATCH libdrm 1/3] tests/util: Reindent using editorconfig settings
From: Thierry Reding
Apply editorconfig settings to the tests/util library code in order to
facilitate making subsequent changes.
Signed-off-by: Thierry Reding
---
tests/util/format.c | 128 ++---
tests/util/format.h | 36 +-
tests/util/kms.c | 186 +++
tests/util/pattern.c | 1446 +-
tests/util/pattern.h | 10 +-
5 files changed, 899 insertions(+), 907 deletions(-)
diff --git a/tests/util/format.c b/tests/util/format.c
index 043cfe7f1eaf..b1055cfc8bbd 100644
--- a/tests/util/format.c
+++ b/tests/util/format.c
@@ -37,84 +37,84 @@
#include "format.h"
#define MAKE_RGB_INFO(rl, ro, gl, go, bl, bo, al, ao) \
- .rgb = { { (rl), (ro) }, { (gl), (go) }, { (bl), (bo) }, { (al), (ao) }
}
+.rgb = { { (rl), (ro) }, { (gl), (go) }, { (bl), (bo) }, { (al), (ao) } }
#define MAKE_YUV_INFO(order, xsub, ysub, chroma_stride) \
- .yuv = { (order), (xsub), (ysub), (chroma_stride) }
+.yuv = { (order), (xsub), (ysub), (chroma_stride) }
static const struct util_format_info format_info[] = {
- /* YUV packed */
- { DRM_FORMAT_UYVY, "UYVY", MAKE_YUV_INFO(YUV_YCbCr | YUV_CY, 2, 2, 2) },
- { DRM_FORMAT_VYUY, "VYUY", MAKE_YUV_INFO(YUV_YCrCb | YUV_CY, 2, 2, 2) },
- { DRM_FORMAT_YUYV, "YUYV", MAKE_YUV_INFO(YUV_YCbCr | YUV_YC, 2, 2, 2) },
- { DRM_FORMAT_YVYU, "YVYU", MAKE_YUV_INFO(YUV_YCrCb | YUV_YC, 2, 2, 2) },
- /* YUV semi-planar */
- { DRM_FORMAT_NV12, "NV12", MAKE_YUV_INFO(YUV_YCbCr, 2, 2, 2) },
- { DRM_FORMAT_NV21, "NV21", MAKE_YUV_INFO(YUV_YCrCb, 2, 2, 2) },
- { DRM_FORMAT_NV16, "NV16", MAKE_YUV_INFO(YUV_YCbCr, 2, 1, 2) },
- { DRM_FORMAT_NV61, "NV61", MAKE_YUV_INFO(YUV_YCrCb, 2, 1, 2) },
- /* YUV planar */
- { DRM_FORMAT_YUV420, "YU12", MAKE_YUV_INFO(YUV_YCbCr, 2, 2, 1) },
- { DRM_FORMAT_YVU420, "YV12", MAKE_YUV_INFO(YUV_YCrCb, 2, 2, 1) },
- /* RGB16 */
- { DRM_FORMAT_ARGB, "AR12", MAKE_RGB_INFO(4, 8, 4, 4, 4, 0, 4, 12) },
- { DRM_FORMAT_XRGB, "XR12", MAKE_RGB_INFO(4, 8, 4, 4, 4, 0, 0, 0) },
- { DRM_FORMAT_ABGR, "AB12", MAKE_RGB_INFO(4, 0, 4, 4, 4, 8, 4, 12) },
- { DRM_FORMAT_XBGR, "XB12", MAKE_RGB_INFO(4, 0, 4, 4, 4, 8, 0, 0) },
- { DRM_FORMAT_RGBA, "RA12", MAKE_RGB_INFO(4, 12, 4, 8, 4, 4, 4, 0) },
- { DRM_FORMAT_RGBX, "RX12", MAKE_RGB_INFO(4, 12, 4, 8, 4, 4, 0, 0) },
- { DRM_FORMAT_BGRA, "BA12", MAKE_RGB_INFO(4, 4, 4, 8, 4, 12, 4, 0) },
- { DRM_FORMAT_BGRX, "BX12", MAKE_RGB_INFO(4, 4, 4, 8, 4, 12, 0, 0) },
- { DRM_FORMAT_ARGB1555, "AR15", MAKE_RGB_INFO(5, 10, 5, 5, 5, 0, 1, 15)
},
- { DRM_FORMAT_XRGB1555, "XR15", MAKE_RGB_INFO(5, 10, 5, 5, 5, 0, 0, 0) },
- { DRM_FORMAT_ABGR1555, "AB15", MAKE_RGB_INFO(5, 0, 5, 5, 5, 10, 1, 15)
},
- { DRM_FORMAT_XBGR1555, "XB15", MAKE_RGB_INFO(5, 0, 5, 5, 5, 10, 0, 0) },
- { DRM_FORMAT_RGBA5551, "RA15", MAKE_RGB_INFO(5, 11, 5, 6, 5, 1, 1, 0) },
- { DRM_FORMAT_RGBX5551, "RX15", MAKE_RGB_INFO(5, 11, 5, 6, 5, 1, 0, 0) },
- { DRM_FORMAT_BGRA5551, "BA15", MAKE_RGB_INFO(5, 1, 5, 6, 5, 11, 1, 0) },
- { DRM_FORMAT_BGRX5551, "BX15", MAKE_RGB_INFO(5, 1, 5, 6, 5, 11, 0, 0) },
- { DRM_FORMAT_RGB565, "RG16", MAKE_RGB_INFO(5, 11, 6, 5, 5, 0, 0, 0) },
- { DRM_FORMAT_BGR565, "BG16", MAKE_RGB_INFO(5, 0, 6, 5, 5, 11, 0, 0) },
- /* RGB24 */
- { DRM_FORMAT_BGR888, "BG24", MAKE_RGB_INFO(8, 0, 8, 8, 8, 16, 0, 0) },
- { DRM_FORMAT_RGB888, "RG24", MAKE_RGB_INFO(8, 16, 8, 8, 8, 0, 0, 0) },
- /* RGB32 */
- { DRM_FORMAT_ARGB, "AR24", MAKE_RGB_INFO(8, 16, 8, 8, 8, 0, 8, 24)
},
- { DRM_FORMAT_XRGB, "XR24", MAKE_RGB_INFO(8, 16, 8, 8, 8, 0, 0, 0) },
- { DRM_FORMAT_ABGR, "AB24", MAKE_RGB_INFO(8, 0, 8, 8, 8, 16, 8, 24)
},
- { DRM_FORMAT_XBGR, "XB24", MAKE_RGB_INFO(8, 0, 8, 8, 8, 16, 0, 0) },
- { DRM_FORMAT_RGBA, "RA24", MAKE_RGB_INFO(8, 24, 8, 16, 8, 8, 8, 0)
},
- { DRM_FORMAT_RGBX, "RX24", MAKE_RGB_INFO(8, 24, 8, 16, 8, 8, 0, 0)
},
- { DRM_FORMAT_BGRA, "BA24", MAKE_RGB_INFO(8, 8, 8, 16, 8, 24, 8, 0)
},
- { DRM_FORMAT_BGRX, "BX24", MAKE_RGB_INFO(8, 8, 8, 16, 8, 24, 0, 0)
},
- { DRM_FORMAT_ARGB2101010, "AR30", MAKE_RGB_INFO(10, 20, 10, 10, 10, 0,
2, 30) },
- { DRM_FORMAT_XRGB2101010, "XR30", MAKE_RGB_INFO(10, 20, 10, 10, 10, 0,
0, 0) },
- { DRM_FORMAT_ABGR2101010, "AB30", MAKE_RGB_INFO(10, 0, 10, 10, 10, 20,
2, 30) },
- { DRM_FORMAT_XBGR2101010, "XB30", MAKE_RGB_INFO(10, 0, 10, 10, 10, 20,
0, 0) },
- { DRM_FORMAT_RGBA1010102, "RA30", MAKE_RGB_INFO(10, 22, 10, 12, 10, 2,
2, 0) },
- { DRM_FORMAT_RGBX1010102, "RX30", MAKE_RGB_INFO(10, 22, 10, 12, 10, 2,
0, 0) },
- { DRM_FORMAT_BGRA1010102, "BA30", MAKE_RGB_INFO(10, 2, 10, 12, 10, 22,
2, 0) },
- { DRM_FORMAT_BGRX1010102, "BX30", MAKE_RGB_INFO(10, 2, 10, 12, 10, 22,
0, 0) },
+/* YUV packed */
+
[PATCH libdrm 3/3] tests/util: Add some API documentation
From: Thierry Reding
None of the helpers currently have any documentation. Add some to make
it more obvious how these should be used.
Signed-off-by: Thierry Reding
---
tests/util/kms.c | 51 +++
1 file changed, 51 insertions(+)
diff --git a/tests/util/kms.c b/tests/util/kms.c
index c5d35ab616d1..226386519d49 100644
--- a/tests/util/kms.c
+++ b/tests/util/kms.c
@@ -128,6 +128,13 @@ const char *util_lookup_connector_type_name(unsigned int
type)
ARRAY_SIZE(connector_type_names));
}
+/**
+ * Retrieve the name of the kernel driver for a DRM device.
+ *
+ * \param fd the file descriptor of the DRM device
+ *
+ * \return On success, returns the name of the kernel driver.
+ */
char *util_get_driver(int fd)
{
drmVersionPtr version;
@@ -144,6 +151,16 @@ char *util_get_driver(int fd)
return driver;
}
+/**
+ * Retrieve a list of available DRM devices.
+ *
+ * \param devicesp return location for the list of DRM devices
+ * \param flags bitmask of flags controlling operation
+ *
+ * \return On success, returns the number of devices found. The list of DRM
+ * devices is dynamically allocated and needs to be freed by a call to the
+ * util_free_devices() function.
+ */
int util_get_devices(drmDevicePtr **devicesp, uint32_t flags)
{
drmDevicePtr *devices;
@@ -182,12 +199,32 @@ int util_get_devices(drmDevicePtr **devicesp, uint32_t
flags)
return count;
}
+/**
+ * Free a list of DRM devices.
+ *
+ * \param devices a list of DRM devices, allocated by util_get_devices()
+ * \param count the number of DRM devices in \p devices
+ */
void util_free_devices(drmDevicePtr *devices, unsigned int count)
{
drmFreeDevices(devices, count);
free(devices);
}
+/**
+ * Opens a DRM device, optionally checking against its kernel driver.
+ *
+ * \param device path to DRM device
+ * \param module optional name of kernel driver
+ *
+ * \return On success, a file descriptor to \p device, or a negative error
+ * code on failure.
+ *
+ * If the caller doesn't care about the kernel driver associated with the DRM
+ * device, then the \p module parameter can be NULL. If \p module is not NULL
+ * this function will retrieve the name of the kernel driver and compare it
+ * to the \p module parameter. If the names don't match, the call will fail.
+ */
int util_open_with_module(const char *device, const char *module)
{
int fd, err = 0;
@@ -229,6 +266,20 @@ out:
return err;
}
+/**
+ * Opens a DRM device, optionally checking against its kernel driver.
+ *
+ * \param device
+ * \param module
+ *
+ * \return On success, a file descriptor to \p device, or a negative error
+ * code on failure.
+ *
+ * This is similar to util_open_with_module(), except that \p device can be
+ * NULL, in which case all available devices will be tried. If \p module is
+ * specified it will act as a filter. Otherwise the first available device
+ * that can be opened will be used.
+ */
int util_open(const char *device, const char *module)
{
int fd, err;
--
2.11.0
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH libdrm 1/3] tests/util: Reindent using editorconfig settings
On Mon, Jan 30, 2017 at 11:29:21AM +0100, Thierry Reding wrote: > From: Thierry Reding > > Apply editorconfig settings to the tests/util library code in order to > facilitate making subsequent changes. > > Signed-off-by: Thierry Reding > --- > tests/util/format.c | 128 ++--- > tests/util/format.h | 36 +- > tests/util/kms.c | 186 +++ > tests/util/pattern.c | 1446 > +- > tests/util/pattern.h | 10 +- > 5 files changed, 899 insertions(+), 907 deletions(-) This is obviously a lot of churn, but for reference, here's the diff ignoring all whitespace changes (--ignore-all-space): --- >8 --- commit f920fb0b72ddd12f18c42ddd96114213868331e3 Author: Thierry Reding Date: Fri Jan 27 11:36:10 2017 +0100 tests/util: Reindent using editorconfig settings Apply editorconfig settings to the tests/util library code in order to facilitate making subsequent changes. Signed-off-by: Thierry Reding diff --git a/tests/util/pattern.c b/tests/util/pattern.c index 00b08a8cb8eb..5d653afbee24 100644 --- a/tests/util/pattern.c +++ b/tests/util/pattern.c @@ -472,8 +472,8 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3], case DRM_FORMAT_VYUY: case DRM_FORMAT_YUYV: case DRM_FORMAT_YVYU: - return fill_smpte_yuv_packed(&info->yuv, planes[0], width, -height, stride); +return fill_smpte_yuv_packed(&info->yuv, planes[0], width, height, + stride); case DRM_FORMAT_NV12: case DRM_FORMAT_NV21: @@ -481,8 +481,8 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3], case DRM_FORMAT_NV61: u = info->yuv.order & YUV_YCbCr ? planes[1] : planes[1] + 1; v = info->yuv.order & YUV_YCrCb ? planes[1] : planes[1] + 1; - return fill_smpte_yuv_planar(&info->yuv, planes[0], u, v, -width, height, stride); +return fill_smpte_yuv_planar(&info->yuv, planes[0], u, v, width, + height, stride); case DRM_FORMAT_YUV420: return fill_smpte_yuv_planar(&info->yuv, planes[0], planes[1], @@ -510,13 +510,12 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3], case DRM_FORMAT_RGBX5551: case DRM_FORMAT_BGRA5551: case DRM_FORMAT_BGRX5551: - return fill_smpte_rgb16(&info->rgb, planes[0], - width, height, stride); +return fill_smpte_rgb16(&info->rgb, planes[0], width, height, stride); case DRM_FORMAT_BGR888: case DRM_FORMAT_RGB888: - return fill_smpte_rgb24(&info->rgb, planes[0], - width, height, stride); +return fill_smpte_rgb24(&info->rgb, planes[0], width, height, stride); + case DRM_FORMAT_ARGB: case DRM_FORMAT_XRGB: case DRM_FORMAT_ABGR: @@ -533,8 +532,7 @@ static void fill_smpte(const struct util_format_info *info, void *planes[3], case DRM_FORMAT_RGBX1010102: case DRM_FORMAT_BGRA1010102: case DRM_FORMAT_BGRX1010102: - return fill_smpte_rgb32(&info->rgb, planes[0], - width, height, stride); +return fill_smpte_rgb32(&info->rgb, planes[0], width, height, stride); } } @@ -568,10 +566,8 @@ static void make_pwetty(void *data, unsigned int width, unsigned int height, return; } - surface = cairo_image_surface_create_for_data(data, - cairo_format, - width, height, - stride); +surface = cairo_image_surface_create_for_data(data, cairo_format, width, + height, stride); cr = cairo_create(surface); cairo_surface_destroy(surface); @@ -758,8 +754,7 @@ static void fill_tiles(const struct util_format_info *info, void *planes[3], case DRM_FORMAT_VYUY: case DRM_FORMAT_YUYV: case DRM_FORMAT_YVYU: - return fill_tiles_yuv_packed(info, planes[0], -width, height, stride); +return fill_tiles_yuv_packed(info, planes[0], width, height, stride); case DRM_FORMAT_NV12: case DRM_FORMAT_NV21: @@ -767,16 +762,16 @@ static void fill_tiles(const struct util_format_info *info, void *planes[3], case DRM_FORMAT_NV61: u = info->yuv.order & YUV_YCbCr ? planes[1] : planes[1] + 1; v = info->yuv.order & YUV_YCrCb ? planes[1] : planes[1] + 1; - return fill_tiles_yuv_planar(info, planes[0], u, v, -width, height, stride); +return fill_tiles_yuv_planar(info, planes[0], u, v, width, height, +
Re: [PATCH 01/10] drm/etnaviv: add uapi for register read feature
Hi Christian,
Am Freitag, den 09.12.2016, 12:21 +0100 schrieb Christian Gmeiner:
> We need to readout some registers _after_ the submited command
> buffer got executed in order to support perf counters.
> There is no way to read register via command stream - even the
> Vivante kernel driver does it via a special ioctl.
>
> Signed-off-by: Christian Gmeiner
> ---
> include/uapi/drm/etnaviv_drm.h | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> index 2584c1c..0d30604 100644
> --- a/include/uapi/drm/etnaviv_drm.h
> +++ b/include/uapi/drm/etnaviv_drm.h
> @@ -150,6 +150,13 @@ struct drm_etnaviv_gem_submit_bo {
> __u64 presumed; /* in/out, presumed buffer address */
> };
>
> +struct drm_etnaviv_gem_submit_readback {
> + __u32 readback_offset;/* in, offset from readback_bo */
> + __u32 readback_idx; /* in, index of readback_bo buffer */
Why do we need this readback BO? Wouldn't it be easier to just have the
space for the readback value allocated in this struct and have the
kernel fill it in? We are reading the values by using the CPU anyways,
so I don't see the need for this indirection with a BO. We removed the
command BO with the same justification.
Regards,
Lucas
> + __u32 reg;/* in, register to read */
> + __u32 flags; /* in, needs to be 0 */
> +};
> +
> /* Each cmdstream submit consists of a table of buffers involved, and
> * one or more cmdstream buffers. This allows for conditional execution
> * (context-restore), and IB buffers needed for per tile/bin draw cmds.
> @@ -167,6 +174,9 @@ struct drm_etnaviv_gem_submit {
> __u64 bos;/* in, ptr to array of submit_bo's */
> __u64 relocs; /* in, ptr to array of submit_reloc's */
> __u64 stream; /* in, ptr to cmdstream */
> + __u64 readbacks; /* in, ptr to array of submit_readback's */
> + __u32 nr_readbacks; /* in, number of submit_readback's */
> + __u32 padding;
> };
>
> /* The normal way to synchronize with the GPU is just to CPU_PREP on
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 09/10] drm/etnaviv: validate readback register address
Am Freitag, den 09.12.2016, 12:21 +0100 schrieb Christian Gmeiner:
> Reading some registers end in a system crash ala:
>
> Unhandled fault: external abort on non-linefetch (0x1028) at 0xfe641000
> Internal error: : 1028 [#1] PREEMPT ARM
>
> Avoid those by register validation.
Avoiding crashes is one thing, but I believe this needs to go further
and avoid reads from any register that isn't a performance counter. This
probably isn't a big hole, but we want to avoid leaking the GPU state to
userspace.
Regards,
Lucas
>
> Signed-off-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 26 ++
> 1 file changed, 26 insertions(+)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 08f9b3d..4383c0d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -277,6 +277,27 @@ static int submit_reloc(struct etnaviv_gem_submit
> *submit, void *stream,
> return 0;
> }
>
> +static int readback_reg_valid(unsigned reg)
> +{
> + /*
> + * 0x000..0x200: ok
> + * 0x200..0x400: crash
> + * 0x400..0x800: ok
> + * 0x800..0xa00: crash
> + * 0xa00..0xc00: crash
> + * 0xc00..0xe00: crash
> + * 0xe00..0x1000:crash
> + * everything above: crash
> + */
> + if (reg >= 0x200 && reg < 400)
> + return 0;
> +
> + if (reg >= 0x800)
> + return 0;
> +
> + return 1;
> +}
> +
> static int submit_readback(struct etnaviv_gem_submit *submit,
> struct etnaviv_cmdbuf *cmdbuf,
> const struct drm_etnaviv_gem_submit_readback *readbacks,
> @@ -303,6 +324,11 @@ static int submit_readback(struct etnaviv_gem_submit
> *submit,
> return -EINVAL;
> }
>
> + if (!readback_reg_valid(r->reg)) {
> + DRM_ERROR("invalid readback reg (would cause crash)");
> + return -EINVAL;
> + }
> +
> cmdbuf->readbacks[i].bo_vma = etnaviv_gem_vmap(&bo->obj->base);
> cmdbuf->readbacks[i].offset = r->readback_offset;
> cmdbuf->readbacks[i].reg = r->reg;
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 08/10] drm/etnaviv: make it possible to reconfigure perf counter
Am Freitag, den 09.12.2016, 12:21 +0100 schrieb Christian Gmeiner:
> Each perf counter 'unit' consits of a multipler configuration register
> and a register to read the selected value. Extend the uapi to handle
> this case gracefully. Before the readback is done the mux (config_reg)
> get reconfigured (vale).
>
> Signed-off-by: Christian Gmeiner
> ---
> drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c | 7 +--
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c| 3 +++
> drivers/gpu/drm/etnaviv/etnaviv_gpu.h| 3 +++
> include/uapi/drm/etnaviv_drm.h | 6 +-
> 4 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index a69eff7..08f9b3d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -298,14 +298,17 @@ static int submit_readback(struct etnaviv_gem_submit
> *submit,
> return -EINVAL;
> }
>
> - if (r->flags) {
> - DRM_ERROR("readback flags not 0");
> + if (r->flags > ETNA_READBACK_PERF) {
> + DRM_ERROR("invalid readback flags");
> return -EINVAL;
> }
>
> cmdbuf->readbacks[i].bo_vma = etnaviv_gem_vmap(&bo->obj->base);
> cmdbuf->readbacks[i].offset = r->readback_offset;
> cmdbuf->readbacks[i].reg = r->reg;
> + cmdbuf->readbacks[i].flags = r->flags;
> + cmdbuf->readbacks[i].perf_reg = r->perf_reg;
> + cmdbuf->readbacks[i].perf_value = r->perf_value;
Woha, this absolutely _needs_ validation. We can't allow userspace to
freely mess with the GPU state like this.
Regards,
Lucas
> }
>
> return 0;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 2aa1a26..b22212c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1379,6 +1379,9 @@ static void etnaviv_process_readbacks(struct
> etnaviv_gpu *gpu,
> const u32 val = gpu_read(gpu, readback->reg);
> u32 *bo = readback->bo_vma;
>
> + if (readback->flags & ETNA_READBACK_PERF)
> + gpu_write(gpu, readback->perf_reg,
> readback->perf_value);
> +
> *(bo + readback->offset) = val;
> }
> }
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 21a4314..02f15c0 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -91,6 +91,9 @@ struct etnaviv_readback {
> u32 *bo_vma;
> u32 offset;
> u32 reg;
> + u32 flags;
> + u32 perf_reg;
> + u32 perf_value;
> };
>
> struct etnaviv_event {
> diff --git a/include/uapi/drm/etnaviv_drm.h b/include/uapi/drm/etnaviv_drm.h
> index 0d30604..f2e24bb 100644
> --- a/include/uapi/drm/etnaviv_drm.h
> +++ b/include/uapi/drm/etnaviv_drm.h
> @@ -150,11 +150,15 @@ struct drm_etnaviv_gem_submit_bo {
> __u64 presumed; /* in/out, presumed buffer address */
> };
>
> +#define ETNA_READBACK_ONLY 0x
> +#define ETNA_READBACK_PERF 0x0001
> struct drm_etnaviv_gem_submit_readback {
> __u32 readback_offset;/* in, offset from readback_bo */
> __u32 readback_idx; /* in, index of readback_bo buffer */
> __u32 reg;/* in, register to read */
> - __u32 flags; /* in, needs to be 0 */
> + __u32 flags; /* in, ETNA_READBACK_* */
> + __u32 perf_reg; /* in, register to write */
> + __u32 perf_value; /* in, value to write */
> };
>
> /* Each cmdstream submit consists of a table of buffers involved, and
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/edid: Complete CEA modedb(VIC 1-107)
Regards Shashank On 1/30/2017 3:47 PM, Andrzej Hajda wrote: On 30.01.2017 10:42, Sharma, Shashank wrote: Regards Shashank On 1/30/2017 2:52 PM, Andrzej Hajda wrote: On 30.01.2017 09:45, Sharma, Shashank wrote: Regards Shashank On 1/30/2017 1:30 PM, Andrzej Hajda wrote: On 29.01.2017 06:41, Shashank Sharma wrote: CEA-861-F specs defines new 4k video modes to be used with HDMI 2.0 EDIDs. These modes start at VIC=93 and go all the way till VIC=107. Our existing CEA modedb contains only 64 modes (VIC=1 to VIC=64). Now to be able to parse 4k modes using the existing techniques, we have to complete the modedb (VIC=65 onwards). This patch adds: - Timings for existing CEA video modes (from VIC=65 till VIC=92) - Newly added 4k modes (from VIC=93 to VIC=107). Cc: Jose Abreu Cc: Alex Deucher Cc: Andrzej Hajda V2: Addressed review comments from Jose: - fix the timings for VIC 83, 90 and 91 - fix formatting for VIC 93-107 V3: Rebase on drm-tip Signed-off-by: Shashank Sharma Signed-off-by: Sonika Jindal Reviewed-by: Jose Abreu Reviewed-by: Alex Deucher --- Have you addressed in some other patch my concerns regarding polluting infoframes generated for HDMI 1.4 devices (CEA-861-E) with VICs specific only for HDMI 2.0 (CEA-861-F) ? It will happen for all UHD modes, see [1][2] for previous discussion. Hello Andrzej As you already know, the aspect ratio is indexed with the VIC, which comes from EDID. Now, why would a HDMI 1.4 monitor, contain a HDMI 2.0 VIC ? If it does, its not compliant. Have you forgot our previous discussion? Let me cite myself: For example 3840x2160@30Hz has no VIC in HDMI 1.4 but it can be present in HDMI vendor specific block with HDMI_VIC 1, on the other side it has VIC 95 in HDMI 2.0. So before your patch AVI infoframe.video_code is set to 0, after your patch is set to 95. And your discussion with Ville: />>> No. The user is free to specify any mode they wish. It doesn't have to/ />>> come from the EDID. Not sure specifying a modern VIC for an older/ />>> display is a good idea or not. If not, we could always check the cea_rev/ />>> assuming it changed whenever the list if VICs was expanded./ />> I agree, that user can specify a mode, out of EDID too./ />> I am anyways planning to add a patch, where before loading HDMI 2.0 VICs/ />> in AVI IF, we are checking the EDID rev./ />> That should solve our problem./ />>/ />> So if edid_rev < 2.0/ />> do_not_load VICs from 93 - 107, but keep it 0./ /> Why edid_rev and not cea_rev?/ /We can do that also, but in the current structure, we are already / /caching EDID rev, its just about re-using it./ /AFAIK, we don't have CEA extension cached anywhere./ Yes we do. It does not seem to be addressed. Regards Andrzej Hey, thanks for bringing this out again. I checked the specs again, there are three things here: - The spec has a section on this, and it states that in such cases CEA VIC should be supplied to the AVI IF. This phrase is not precise, but if it means that in case of 3840x2160@30Hz source should set VIC to 95 also for HDMI1.4 compliant sinks it is in opposition to statement from HDMI1.4 specification: When transmitting any extended video format indicated through use of the HDMI_VIC field in the HDMI Vendor Specific InfoFrame or any other format which is not described in the above cases, an HDMI Source shall set the AVI InfoFrame VIC field to zero. I do not think HDMI20 could invalidate claims from HDMI1.4. These claims are not made in HDMI 2.0 specs, CEA is the body which defines VICs, AVI-IF and aspects, so its defined in CEA-861-F spec IF you check the CEA-861-F spec, you will find this section. - Also, for HDMI infoframe blocks, we dont refer to cea_edid_modes[] rather we go to edid_4k_modes[] cea_edid_modes is used in generating AVI infoframe, edid_4k_modes is used in generating vendor infoframe - Again, as there is no aspect ratio parsing layer in DRM right now (after the revert), we are doing the right thing to give aspect as per CEA, which is suggested by spec too. I do not understand this phrase. Are you talking about aspect ratios or about video format? As edid_cea_modes is being used in drm_hdmi_avi_if_from_display_mode, only place where its being used to load AVI IF (with aspect ratio information), I was talking about both. But lets talk about VIC only from here. Shashank Regards Andrzej ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 4/4] drm: bridge: dw-hdmi: Take input format from plat_data
Some display pipelines can only provide non-RBG input pixels to the HDMI TX
Controller, this patch takes the pixel format from the plat_data if provided.
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/bridge/dw-hdmi.c | 55
include/drm/bridge/dw_hdmi.h | 14 --
2 files changed, 40 insertions(+), 29 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index f75e5f9..f4ba019 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -35,12 +35,6 @@
#define HDMI_EDID_LEN 512
-#define RGB0
-#define YCBCR444 1
-#define YCBCR422_16BITS2
-#define YCBCR422_8BITS 3
-#define XVYCC444 4
-
enum hdmi_datamap {
RGB444_8B = 0x01,
RGB444_10B = 0x03,
@@ -548,7 +542,7 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
int color_format = 0;
u8 val;
- if (hdmi->hdmi_data.enc_in_format == RGB) {
+ if (hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_RGB) {
if (hdmi->hdmi_data.enc_color_depth == 8)
color_format = 0x01;
else if (hdmi->hdmi_data.enc_color_depth == 10)
@@ -559,7 +553,7 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
color_format = 0x07;
else
return;
- } else if (hdmi->hdmi_data.enc_in_format == YCBCR444) {
+ } else if (hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_YCBCR444) {
if (hdmi->hdmi_data.enc_color_depth == 8)
color_format = 0x09;
else if (hdmi->hdmi_data.enc_color_depth == 10)
@@ -570,7 +564,8 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
color_format = 0x0F;
else
return;
- } else if (hdmi->hdmi_data.enc_in_format == YCBCR422_8BITS) {
+ } else if (hdmi->hdmi_data.enc_in_format ==
+ DW_HDMI_ENC_FMT_YCBCR422_8BITS) {
if (hdmi->hdmi_data.enc_color_depth == 8)
color_format = 0x16;
else if (hdmi->hdmi_data.enc_color_depth == 10)
@@ -606,20 +601,20 @@ static int is_color_space_conversion(struct dw_hdmi *hdmi)
static int is_color_space_decimation(struct dw_hdmi *hdmi)
{
- if (hdmi->hdmi_data.enc_out_format != YCBCR422_8BITS)
+ if (hdmi->hdmi_data.enc_out_format != DW_HDMI_ENC_FMT_YCBCR422_8BITS)
return 0;
- if (hdmi->hdmi_data.enc_in_format == RGB ||
- hdmi->hdmi_data.enc_in_format == YCBCR444)
+ if (hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_RGB ||
+ hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_YCBCR444)
return 1;
return 0;
}
static int is_color_space_interpolation(struct dw_hdmi *hdmi)
{
- if (hdmi->hdmi_data.enc_in_format != YCBCR422_8BITS)
+ if (hdmi->hdmi_data.enc_in_format != DW_HDMI_ENC_FMT_YCBCR422_8BITS)
return 0;
- if (hdmi->hdmi_data.enc_out_format == RGB ||
- hdmi->hdmi_data.enc_out_format == YCBCR444)
+ if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_RGB ||
+ hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_YCBCR444)
return 1;
return 0;
}
@@ -631,13 +626,14 @@ static void dw_hdmi_update_csc_coeffs(struct dw_hdmi
*hdmi)
u32 csc_scale = 1;
if (is_color_space_conversion(hdmi)) {
- if (hdmi->hdmi_data.enc_out_format == RGB) {
+ if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_RGB) {
if (hdmi->hdmi_data.colorimetry ==
HDMI_COLORIMETRY_ITU_601)
csc_coeff = &csc_coeff_rgb_out_eitu601;
else
csc_coeff = &csc_coeff_rgb_out_eitu709;
- } else if (hdmi->hdmi_data.enc_in_format == RGB) {
+ } else if (hdmi->hdmi_data.enc_in_format ==
+ DW_HDMI_ENC_FMT_RGB) {
if (hdmi->hdmi_data.colorimetry ==
HDMI_COLORIMETRY_ITU_601)
csc_coeff = &csc_coeff_rgb_in_eitu601;
@@ -709,8 +705,8 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
struct hdmi_data_info *hdmi_data = &hdmi->hdmi_data;
u8 val, vp_conf;
- if (hdmi_data->enc_out_format == RGB ||
- hdmi_data->enc_out_format == YCBCR444) {
+ if (hdmi_data->enc_out_format == DW_HDMI_ENC_FMT_RGB ||
+ hdmi_data->enc_out_format == DW_HDMI_ENC_FMT_YCBCR444) {
if (!hdmi_data->enc_color_depth) {
output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS;
} else if (hdmi_data->enc_color_depth == 8) {
@
Re: [v3 PATCH 1/3] drm/atomic: Save flip flags in drm_crtct_state
Hi Andrey,
Thank you for the patch.
On Saturday 28 Jan 2017 21:26:49 Andrey Grodzovsky wrote:
> Allows using atomic flip helpers for drivers
> using ASYNC flip.
> Remove ASYNC_FLIP restriction in helpers and
> caches the page flip flags in drm_crtc_state
> to be used in the low level drivers.
>
> v2:
> Resending the patch since the original was broken.
>
> v3:
> Save flag in crtc_state instead of plane_state
>
> Signed-off-by: Andrey Grodzovsky
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 19 +--
> include/drm/drm_crtc.h | 8 +++-
> include/drm/drm_plane.h | 1 +
> 3 files changed, 13 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index a4e5477..28065ee 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2737,7 +2737,8 @@ static int page_flip_common(
> struct drm_atomic_state *state,
> struct drm_crtc *crtc,
> struct drm_framebuffer *fb,
> - struct drm_pending_vblank_event *event)
> + struct drm_pending_vblank_event *event,
> + uint32_t flags)
> {
> struct drm_plane *plane = crtc->primary;
> struct drm_plane_state *plane_state;
> @@ -2749,12 +2750,12 @@ static int page_flip_common(
> return PTR_ERR(crtc_state);
>
> crtc_state->event = event;
> + crtc_state->pflip_flags = flags;
>
> plane_state = drm_atomic_get_plane_state(state, plane);
> if (IS_ERR(plane_state))
> return PTR_ERR(plane_state);
>
> -
> ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> if (ret != 0)
> return ret;
> @@ -2781,10 +2782,6 @@ static int page_flip_common(
> * Provides a default &drm_crtc_funcs.page_flip implementation
> * using the atomic driver interface.
> *
> - * Note that for now so called async page flips (i.e. updates which are not
> - * synchronized to vblank) are not supported, since the atomic interfaces
> have - * no provisions for this yet.
> - *
> * Returns:
> * Returns 0 on success, negative errno numbers on failure.
> *
> @@ -2800,9 +2797,6 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> struct drm_atomic_state *state;
> int ret = 0;
>
> - if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> - return -EINVAL;
> -
> state = drm_atomic_state_alloc(plane->dev);
> if (!state)
> return -ENOMEM;
> @@ -2810,7 +2804,7 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
> state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
>
> retry:
> - ret = page_flip_common(state, crtc, fb, event);
> + ret = page_flip_common(state, crtc, fb, event, flags);
> if (ret != 0)
> goto fail;
>
> @@ -2865,9 +2859,6 @@ int drm_atomic_helper_page_flip_target(
> struct drm_crtc_state *crtc_state;
> int ret = 0;
>
> - if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
> - return -EINVAL;
> -
> state = drm_atomic_state_alloc(plane->dev);
> if (!state)
> return -ENOMEM;
> @@ -2875,7 +2866,7 @@ int drm_atomic_helper_page_flip_target(
> state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
>
> retry:
> - ret = page_flip_common(state, crtc, fb, event);
> + ret = page_flip_common(state, crtc, fb, event, flags);
> if (ret != 0)
> goto fail;
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 5c77c3f..76457a4 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -162,10 +162,16 @@ struct drm_crtc_state {
>* Target vertical blank period when a page flip
>* should take effect.
>*/
> -
> u32 target_vblank;
>
> /**
> + * @pflip_flags:
> + *
> + * Flip related config options
This isn't detailed enough. I propose something along the lines of
"DRM_MODE_PAGE_FLIP_* page flip flags, as passed to the page flip ioctl.
Always zero for atomic commits that don't originate from a page flip ioctl."
You will then also need to reset the field to 0 at an appropriate point, as
it's more an atomic commit transaction information than a state. Apart from
that this patch looks good to me.
> + */
> + u32 pflip_flags;
> +
> + /**
>* @event:
>*
>* Optional pointer to a DRM event to signal upon completion of the
> diff --git a/include/drm/drm_plane.h b/include/drm/drm_plane.h
> index db3bbde..57414ae 100644
> --- a/include/drm/drm_plane.h
> +++ b/include/drm/drm_plane.h
> @@ -122,6 +122,7 @@ struct drm_plane_state {
>*/
> bool visible;
>
> struct drm_atomic_state *state;
> };
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freed
[PATCH 0/4] drm: bridge: dw-hdmi: Add support for Custom PHYs
The Amlogic GX SoCs implements a Synopsys DesignWare HDMI TX Controller in combination with a very custom PHY. Thanks to Laurent Pinchart's changes, the HW report the following : Detected HDMI TX controller v2.01a with HDCP (Vendor PHY) The following differs from common PHY integration as managed in the current driver : - Amlogic PHY is not configured through the internal I2C link - Amlogic PHY do not use the ENTMDS, SVSRET, PDDQ, ... signals from the controller - Amlogic PHY do not export HPD ands RxSense signals to the controller And finally, concerning the controller integration : - the Controller registers are not flat memory-mapped, and uses an addr+read/write register pair to write all registers. - Inputs only YUV444 pixel data This is why the following patchset implements : - Conversion to regmap for register access - Add more callbacks ops to handle Custom PHYs - Configure the Input format from the plat_data - Fixes a bug that considers the input to be always RBG and sends bad pixel format to a DVI sink by disabling CSC This patchset makes the Amlogix GX SoCs HDMI output successfully work, but I do not have access to Renesas, i.MX or RockChip SoCs to test against potentiel regressions, like the regmap conversion. This patchset is based on the latest Laurent Pinchart dw-hdmi serie at [1] that was recently merged into drm-misc. Changes since RFC at [2] : - Regmap fixup for 4bytes register access, tested on RK3288 SoC - Move phy callbacks to phy_ops and move Synopsys PHY calls into default ops - Move HDMI link data into shared header - Move Pixel Encoding enum to shared header [1] http://lkml.kernel.org/r/20170117082910.27023-1-laurent.pinchart+rene...@ideasonboard.com [2] http://lkml.kernel.org/r/1484656294-6140-1-git-send-email-narmstr...@baylibre.com Neil Armstrong (4): drm: bridge: dw-hdmi: Switch to regmap for register access drm: bridge: dw-hdmi: Add support for custom PHY handling drm: bridge: dw-hdmi: Enable CSC even for DVI drm: bridge: dw-hdmi: Take input format from plat_data drivers/gpu/drm/bridge/dw-hdmi.c | 313 ++- include/drm/bridge/dw_hdmi.h | 46 ++ 2 files changed, 226 insertions(+), 133 deletions(-) -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm: bridge: dw-hdmi: Add support for custom PHY handling
The Synopsys DesignWare HDMI TX Controller support various Transceivers (PHY)
attached to the controller, but also allows fully custom PHYs to be connected.
Add PHY init and disable functions in phy_ops structure passed in plat_data
to handle fully custom PHY init or provide the default one.
Some custom PHYs also handles the HPD and RxSense separately and do not use
the internal signals exported in the Controller's IRQ stat, so a read_hpd
is provided to switch to HPD status polling.
To complete the implementation, add a function to mimic the Controllers
interrupt HPD and RxSense handling from the vendor PHY wrapper code.
Signed-off-by: Neil Armstrong
---
drivers/gpu/drm/bridge/dw-hdmi.c | 145 +--
include/drm/bridge/dw_hdmi.h | 35 ++
2 files changed, 127 insertions(+), 53 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index 674ab05..a8083f4 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -85,24 +85,6 @@ enum hdmi_datamap {
{ 0x6756, 0x78ab, 0x2000, 0x0200 }
};
-struct hdmi_vmode {
- bool mdataenablepolarity;
-
- unsigned int mpixelclock;
- unsigned int mpixelrepetitioninput;
- unsigned int mpixelrepetitionoutput;
-};
-
-struct hdmi_data_info {
- unsigned int enc_in_format;
- unsigned int enc_out_format;
- unsigned int enc_color_depth;
- unsigned int colorimetry;
- unsigned int pix_repet_factor;
- unsigned int hdcp_enable;
- struct hdmi_vmode video_mode;
-};
-
struct dw_hdmi_i2c {
struct i2c_adapter adap;
@@ -144,6 +126,7 @@ struct dw_hdmi {
u8 edid[HDMI_EDID_LEN];
bool cable_plugin;
+ const struct dw_hdmi_phy_ops *phy_ops;
const struct dw_hdmi_phy_data *phy;
bool phy_enabled;
@@ -926,7 +909,8 @@ static void dw_hdmi_phy_sel_interface_control(struct
dw_hdmi *hdmi, u8 enable)
HDMI_PHY_CONF0_SELDIPIF_MASK);
}
-static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi)
+static void dw_hdmi_phy_power_off(struct dw_hdmi *hdmi,
+ const struct dw_hdmi_plat_data *pdata)
{
if (hdmi->phy->gen == 1) {
dw_hdmi_phy_enable_tmds(hdmi, 0);
@@ -1006,14 +990,15 @@ static int hdmi_phy_configure_dwc_hdmi_3d_tx(struct
dw_hdmi *hdmi,
return 0;
}
-static int hdmi_phy_configure(struct dw_hdmi *hdmi)
+static int hdmi_phy_configure(struct dw_hdmi *hdmi,
+ const struct dw_hdmi_plat_data *pdata,
+ struct hdmi_data_info *hdmi_data)
{
- const struct dw_hdmi_plat_data *pdata = hdmi->plat_data;
- unsigned long mpixelclock = hdmi->hdmi_data.video_mode.mpixelclock;
+ unsigned long mpixelclock = hdmi_data->video_mode.mpixelclock;
u8 val, msec;
int ret;
- dw_hdmi_phy_power_off(hdmi);
+ dw_hdmi_phy_power_off(hdmi, pdata);
/* Leave low power consumption mode by asserting SVSRET. */
if (hdmi->phy->has_svsret)
@@ -1062,7 +1047,10 @@ static int hdmi_phy_configure(struct dw_hdmi *hdmi)
return 0;
}
-static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
+static int dw_hdmi_phy_init(struct dw_hdmi *hdmi,
+ const struct dw_hdmi_plat_data *pdata,
+ struct drm_display_mode *mode,
+ struct hdmi_data_info *hdmi_data)
{
int i, ret;
@@ -1071,15 +1059,21 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi)
dw_hdmi_phy_sel_data_en_pol(hdmi, 1);
dw_hdmi_phy_sel_interface_control(hdmi, 0);
- ret = hdmi_phy_configure(hdmi);
+ ret = hdmi_phy_configure(hdmi, pdata, hdmi_data);
if (ret)
return ret;
}
- hdmi->phy_enabled = true;
return 0;
}
+/* Synopsys DW-HDMI PHY Control Ops */
+const struct dw_hdmi_phy_ops dw_hdmi_phy_ops = {
+ .phy_init = dw_hdmi_phy_init,
+ .phy_disable = dw_hdmi_phy_power_off,
+};
+EXPORT_SYMBOL_GPL(dw_hdmi_phy_ops);
+
static void hdmi_tx_hdcp_config(struct dw_hdmi *hdmi)
{
u8 de;
@@ -1294,15 +1288,6 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
hdmi_writeb(hdmi, vsync_len, HDMI_FC_VSYNCINWIDTH);
}
-static void dw_hdmi_phy_disable(struct dw_hdmi *hdmi)
-{
- if (!hdmi->phy_enabled)
- return;
-
- dw_hdmi_phy_power_off(hdmi);
- hdmi->phy_enabled = false;
-}
-
/* HDMI Initialization Step B.4 */
static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi)
{
@@ -1434,10 +1419,15 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi, struct
drm_display_mode *mode)
/* HDMI Initialization Step B.1 */
hdmi_av_composer(hdmi, mode);
- /* HDMI Initializateion Step B.2 */
- ret = dw_hdmi_phy_init(hdmi);
- if (ret)
- return ret;
+
[PATCH 1/4] drm: bridge: dw-hdmi: Switch to regmap for register access
The Synopsys Designware HDMI TX Controller does not enforce register
access on platforms instanciating it. The current driver supports two
different types of memory-mapped flat register access, but in order to
support the Amlogic Meson SoCs integration, and provide a more generic
way to handle all sorts of register mapping, switch the register access
to use the regmap infrastructure.
In the case of registers that are not flat memory-mapped or do not
conform to the current driver implementation, a regmap struct can be
given in the plat_data and be used at probe or bind.
Since the AHB audio driver is only available with direct memory access,
only allow the I2S audio driver to be registered is directly
memory-mapped.
Signed-off-by: Neil Armstrong
Reviewed-by: Laurent Pinchart
Tested-by: Laurent Pinchart
---
drivers/gpu/drm/bridge/dw-hdmi.c | 109 +--
include/drm/bridge/dw_hdmi.h | 1 +
2 files changed, 59 insertions(+), 51 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index ca9d0ce..674ab05 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -19,6 +19,7 @@
#include
#include
#include
+#include
#include
#include
@@ -167,8 +168,8 @@ struct dw_hdmi {
unsigned int audio_n;
bool audio_enable;
- void (*write)(struct dw_hdmi *hdmi, u8 val, int offset);
- u8 (*read)(struct dw_hdmi *hdmi, int offset);
+ unsigned int reg_shift;
+ struct regmap *regm;
};
#define HDMI_IH_PHY_STAT0_RX_SENSE \
@@ -179,42 +180,23 @@ struct dw_hdmi {
(HDMI_PHY_RX_SENSE0 | HDMI_PHY_RX_SENSE1 | \
HDMI_PHY_RX_SENSE2 | HDMI_PHY_RX_SENSE3)
-static void dw_hdmi_writel(struct dw_hdmi *hdmi, u8 val, int offset)
-{
- writel(val, hdmi->regs + (offset << 2));
-}
-
-static u8 dw_hdmi_readl(struct dw_hdmi *hdmi, int offset)
-{
- return readl(hdmi->regs + (offset << 2));
-}
-
-static void dw_hdmi_writeb(struct dw_hdmi *hdmi, u8 val, int offset)
-{
- writeb(val, hdmi->regs + offset);
-}
-
-static u8 dw_hdmi_readb(struct dw_hdmi *hdmi, int offset)
-{
- return readb(hdmi->regs + offset);
-}
-
static inline void hdmi_writeb(struct dw_hdmi *hdmi, u8 val, int offset)
{
- hdmi->write(hdmi, val, offset);
+ regmap_write(hdmi->regm, offset << hdmi->reg_shift, val);
}
static inline u8 hdmi_readb(struct dw_hdmi *hdmi, int offset)
{
- return hdmi->read(hdmi, offset);
+ unsigned int val = 0;
+
+ regmap_read(hdmi->regm, offset << hdmi->reg_shift, &val);
+
+ return val;
}
static void hdmi_modb(struct dw_hdmi *hdmi, u8 data, u8 mask, unsigned reg)
{
- u8 val = hdmi_readb(hdmi, reg) & ~mask;
-
- val |= data & mask;
- hdmi_writeb(hdmi, val, reg);
+ regmap_update_bits(hdmi->regm, reg << hdmi->reg_shift, mask, data);
}
static void hdmi_mask_writeb(struct dw_hdmi *hdmi, u8 data, unsigned int reg,
@@ -1949,6 +1931,20 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
return 0;
}
+static const struct regmap_config hdmi_regmap_8bit_config = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .reg_stride = 1,
+ .max_register = HDMI_I2CM_FS_SCL_LCNT_0_ADDR,
+};
+
+static const struct regmap_config hdmi_regmap_32bit_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .max_register = HDMI_I2CM_FS_SCL_LCNT_0_ADDR << 2,
+};
+
static struct dw_hdmi *
__dw_hdmi_probe(struct platform_device *pdev,
const struct dw_hdmi_plat_data *plat_data)
@@ -1958,7 +1954,7 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
struct platform_device_info pdevinfo;
struct device_node *ddc_node;
struct dw_hdmi *hdmi;
- struct resource *iores;
+ struct resource *iores = NULL;
int irq;
int ret;
u32 val = 1;
@@ -1982,22 +1978,6 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
mutex_init(&hdmi->audio_mutex);
spin_lock_init(&hdmi->audio_lock);
- of_property_read_u32(np, "reg-io-width", &val);
-
- switch (val) {
- case 4:
- hdmi->write = dw_hdmi_writel;
- hdmi->read = dw_hdmi_readl;
- break;
- case 1:
- hdmi->write = dw_hdmi_writeb;
- hdmi->read = dw_hdmi_readb;
- break;
- default:
- dev_err(dev, "reg-io-width must be 1 or 4\n");
- return ERR_PTR(-EINVAL);
- }
-
ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
if (ddc_node) {
hdmi->ddc = of_get_i2c_adapter_by_node(ddc_node);
@@ -2011,11 +1991,38 @@ static int dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
dev_dbg(hdmi->dev, "no ddc property found\n");
}
- iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- hdmi->regs = devm_ioremap_resource
[PATCH 3/4] drm: bridge: dw-hdmi: Enable CSC even for DVI
If the input pixel format is not RGB, the CSC must be enabled in order to provide valid pixel to DVI sinks. This patch removes the hdmi only dependency on the CSC enabling. Reviewed-by: Jose Abreu Reviewed-by: Laurent Pinchart Signed-off-by: Neil Armstrong --- drivers/gpu/drm/bridge/dw-hdmi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c index a8083f4..f75e5f9 100644 --- a/drivers/gpu/drm/bridge/dw-hdmi.c +++ b/drivers/gpu/drm/bridge/dw-hdmi.c @@ -1317,8 +1317,8 @@ static void dw_hdmi_enable_video_path(struct dw_hdmi *hdmi) hdmi_writeb(hdmi, clkdis, HDMI_MC_CLKDIS); } - /* Enable color space conversion if needed (for HDMI sinks only). */ - if (hdmi->sink_is_hdmi && is_color_space_conversion(hdmi)) + /* Enable color space conversion if needed */ + if (is_color_space_conversion(hdmi)) hdmi_writeb(hdmi, HDMI_MC_FLOWCTRL_FEED_THROUGH_OFF_CSC_IN_PATH, HDMI_MC_FLOWCTRL); else -- 1.9.1 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
On 01/28/17 18:17, Laurent Pinchart wrote: > Hi Jyri, > > Thank you for the patch. > > On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote: >> Move drm_atomic_helper_commit_modeset_enables() call to before >> drm_atomic_helper_commit_planes() call and have a >> omap_atomic_wait_for_completion() call after both. >> >> With the current dss dispc implementation we have to enable the new >> modeset before we can commit planes. The dispc ovl configuration >> relies on the video mode configuration been written into the HW when >> the ovl configuration is calculated. >> >> This approach is not ideal because after a mode change the plane >> update is executed only after the first vblank interrupt. The dispc >> implementation should be fixed so that it is able use uncommitted drm >> state information. information. >> >> Signed-off-by: Jyri Sarha >> --- >> drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++- >> 1 file changed, 15 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644 >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c >> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct >> omap_atomic_state_commit *commit) dispc_runtime_get(); >> >> drm_atomic_helper_commit_modeset_disables(dev, old_state); >> -drm_atomic_helper_commit_planes(dev, old_state, 0); >> + >> +/* With the current dss dispc implementation we have to enable >> + * the new modeset before we can commit planes. The dispc ovl >> + * configuration relies on the video mode configuration been >> + * written into the HW when the ovl configuration is >> + * calculated. >> + * >> + * This approach is not ideal because after a mode change the >> + * plane update is executed only after the first vblank >> + * interrupt. The dispc implementation should be fixed so that >> + * it is able use uncommitted drm state information. > Any chance you could fix the dispc implementation ? ;-) Otherwise, could you I am currently studying how to do it, but it tricky... really tricky. In DRM terms, the plane configuration is affected by the connector where the crtc is connected to. The whole DSS should have to made understand drm state structures. There certainly wont be any "fix" for 4.10, probably nothing for 4.11 either. > rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and > zpos" ? > Thanks, I'll do that. BR, Jyri ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
Hi Jyri, On Monday 30 Jan 2017 13:11:56 Jyri Sarha wrote: > On 01/28/17 18:17, Laurent Pinchart wrote: > > On Friday 27 Jan 2017 12:04:55 Jyri Sarha wrote: > >> Move drm_atomic_helper_commit_modeset_enables() call to before > >> drm_atomic_helper_commit_planes() call and have a > >> omap_atomic_wait_for_completion() call after both. > >> > >> With the current dss dispc implementation we have to enable the new > >> modeset before we can commit planes. The dispc ovl configuration > >> relies on the video mode configuration been written into the HW when > >> the ovl configuration is calculated. > >> > >> This approach is not ideal because after a mode change the plane > >> update is executed only after the first vblank interrupt. The dispc > >> implementation should be fixed so that it is able use uncommitted drm > >> state information. information. > >> > >> Signed-off-by: Jyri Sarha > >> --- > >> > >> drivers/gpu/drm/omapdrm/omap_drv.c | 16 +++- > >> 1 file changed, 15 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c > >> b/drivers/gpu/drm/omapdrm/omap_drv.c index 5e55f1b..8c15735 100644 > >> --- a/drivers/gpu/drm/omapdrm/omap_drv.c > >> +++ b/drivers/gpu/drm/omapdrm/omap_drv.c > >> @@ -96,8 +96,22 @@ static void omap_atomic_complete(struct > >> omap_atomic_state_commit *commit) dispc_runtime_get(); > >> > >>drm_atomic_helper_commit_modeset_disables(dev, old_state); > >> > >> - drm_atomic_helper_commit_planes(dev, old_state, 0); > >> + > >> + /* With the current dss dispc implementation we have to enable > >> + * the new modeset before we can commit planes. The dispc ovl > >> + * configuration relies on the video mode configuration been > >> + * written into the HW when the ovl configuration is > >> + * calculated. > >> + * > >> + * This approach is not ideal because after a mode change the > >> + * plane update is executed only after the first vblank > >> + * interrupt. The dispc implementation should be fixed so that > >> + * it is able use uncommitted drm state information. > > > > Any chance you could fix the dispc implementation ? ;-) Otherwise, could > > you > > I am currently studying how to do it, but it tricky... really tricky. In > DRM terms, the plane configuration is affected by the connector where > the crtc is connected to. The whole DSS should have to made understand > drm state structures. There certainly wont be any "fix" for 4.10, > probably nothing for 4.11 either. > > > rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and > > zpos" ? > > Thanks, I'll do that. If you intend on merging this patch as a v4.10 fix then there's no need to rebase. If it targets v4.11, the above-mentioned series will likely go in first. -- Regards, Laurent Pinchart ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3] drm/edid: Complete CEA modedb(VIC 1-107)
I just realized that the CEA spec talks about picking HDMI 2.0/861-F VIC over HDMI14b VIC in case of 3D side-by-side format (not in 2D), In this case, we might have to add a check while loading the VIC filed in AVI IF (as you suggested) Will come back with a V2 (if required), once I have more information on this. For example 3840x2160@30Hz has no VIC in HDMI 1.4 but it can be present in HDMI vendor specific block with HDMI_VIC 1, on the other side it has VIC 95 in HDMI 2.0. So before your patch AVI infoframe.video_code is set to 0, after your patch is set to 95. Regards Shashank ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Etnaviv cmdbuf suballocator
Am Mittwoch, den 18.01.2017, 12:25 +0100 schrieb Lucas Stach: > Hi all, > > the following patches introduce a cmduf suballocator in the Etnaviv > kernel driver, which has the following benefits: > > 1. Allocating and freeing a CMA buffer for each user command submission >is taking a big toll on the CPU, as CMA is not exactly low overhead. >By suballocating a single buffer we avoid all this overhead. > > 2. Less TLB flushes on MMUv2. Each time a new buffer gets mapped into >the GPU address space on MMUv2 the TLBs need to be flushed. Mapping >the suballocated area once allows to skip the TLB flushes (at least >as long as userspace re-uses existing buffers). > > 3. No workarounds for GC3000 required anymore. The FE TLB flush on >GC3000 doesn't work reliably, which required us to map the cmdbufs >into the GPU address space at specific positions, which also isn't >guaranteed to work if the address space is already crowded. Having >a single static area for the cmdbufs side-steps the erratum completely. > > If I can get reviews and/or enough testing, I would like to include this > code in kernel 4.11. If this helps in testing this change, I have pushed my queue out into my public git repo at: https://git.pengutronix.de/git/lst/linux drm-etnaviv-next The branch is based on v4.10-rc1, but is trivial to rebase onto something more recent. Regards, Lucas ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 0/6] Etnaviv cmdbuf suballocator
Hi Lucas, 2017-01-30 12:48 GMT+01:00 Lucas Stach : > Am Mittwoch, den 18.01.2017, 12:25 +0100 schrieb Lucas Stach: >> Hi all, >> >> the following patches introduce a cmduf suballocator in the Etnaviv >> kernel driver, which has the following benefits: >> >> 1. Allocating and freeing a CMA buffer for each user command submission >>is taking a big toll on the CPU, as CMA is not exactly low overhead. >>By suballocating a single buffer we avoid all this overhead. >> >> 2. Less TLB flushes on MMUv2. Each time a new buffer gets mapped into >>the GPU address space on MMUv2 the TLBs need to be flushed. Mapping >>the suballocated area once allows to skip the TLB flushes (at least >>as long as userspace re-uses existing buffers). >> >> 3. No workarounds for GC3000 required anymore. The FE TLB flush on >>GC3000 doesn't work reliably, which required us to map the cmdbufs >>into the GPU address space at specific positions, which also isn't >>guaranteed to work if the address space is already crowded. Having >>a single static area for the cmdbufs side-steps the erratum completely. >> >> If I can get reviews and/or enough testing, I would like to include this >> code in kernel 4.11. > > If this helps in testing this change, I have pushed my queue out into my > public git repo at: > > https://git.pengutronix.de/git/lst/linux drm-etnaviv-next > > The branch is based on v4.10-rc1, but is trivial to rebase onto > something more recent. > Will have a look at it later the day. greets -- Christian Gmeiner, MSc https://www.youtube.com/user/AloryOFFICIAL https://soundcloud.com/christian-gmeiner ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/2] drm/omapdrm: Move commit_modeset_enables() before commit_planes()
On 01/30/17 13:15, Laurent Pinchart wrote: >>> rebase (and retest) this patch on top of "[PATCH 0/5] omapdrm: fences and >>> zpos" ? >> Thanks, I'll do that. > If you intend on merging this patch as a v4.10 fix then there's no need to > rebase. If it targets v4.11, the above-mentioned series will likely go in > first. Well perhaps this patch should go to v4.10 already. Because of the bug a plane commit may fail if the screen is blanked and there is no pclk available at the time the plane HW is configured. Tomi, what do you think? BR, Jyri ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] Gpu: drm: exynos - Fix possible NULL derefrence.
Hello Shailendra,
The subject line is wrong, please always use the convention used in
previous commits, i.e: git log --oneline drivers/gpu/drm/exynos/
On 01/30/2017 02:02 AM, Shailendra Verma wrote:
> of_device_get_match_data could return NULL, and so can cause
> a NULL pointer dereference later.
>
> Signed-off-by: Shailendra Verma
> ---
> drivers/gpu/drm/exynos/exynos_drm_dsi.c |4
> drivers/gpu/drm/exynos/exynos_drm_fimd.c |4
> drivers/gpu/drm/exynos/exynos_hdmi.c |4
> drivers/gpu/drm/exynos/exynos_mixer.c|4
> 4 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> index e07cb1f..fba0ffc 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
> @@ -1765,6 +1765,10 @@ static int exynos_dsi_probe(struct platform_device
> *pdev)
>
> dsi->dev = dev;
> dsi->driver_data = of_device_get_match_data(dev);
> + if (!dsi->driver_data) {
> + dev_err(dev, "no device match found\n");
> + return -ENODEV;
> + }
>
I don't think this makes sense for the Exynos driver since Exynos is a
DT-only platform and so the probe callback can only be called if a dev
node with a compatible string in the OF device id table was registered.
All the struct of_device_id in the table have a .data and so this can't
be NULL in this driver.
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 99368] Full aspect scaling introduces interlacing on specific resolutions
https://bugs.freedesktop.org/show_bug.cgi?id=99368 Platin changed: What|Removed |Added Attachment #128898|0 |1 is obsolete|| --- Comment #5 from Platin --- Created attachment 129229 --> https://bugs.freedesktop.org/attachment.cgi?id=129229&action=edit Correct Xorg log Didn't know that GDM doesn't store the Xorg log in the usual location. So I accidentally uploaded an obsolete log. Replaced it with the correct log. I tried lower resolutions with the amdgpu driver and the problem shows even there. So I'm not sure if the problem exists on both drivers or if the source of the problem lies elsewhere. Does anyone have an idea? -- You are receiving this mail because: You are the assignee for the bug.___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings
On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:
> Hi,
>
> On 01/28/2017 05:25 PM, Hans de Goede wrote:
> > Hi,
> >
> > On 01/27/2017 02:51 PM, Ville Syrjälä wrote:
> >> On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:
> >>> Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
> >>> request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
> >>> around P-Unit write accesses.
> >>
> >> Can't we just stuff the calls into the actual punit write function
> >> rather than sprinkling them all over the place?
> >
> > punit access is acquired across sections like this:
> >
> > iosf_mbi_punit_acquire();
> >
> > val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> > val &= ~DSPFREQGUAR_MASK;
> > val |= (cmd << DSPFREQGUAR_SHIFT);
> > vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> > if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
> > DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
> > 50)) {
> > DRM_ERROR("timed out waiting for CDclk change\n");
> > }
> > iosf_mbi_punit_release();
> >
> > Where we want to wait for the requested change to have taken
> > effect before releasing the punit.
Hmm. That's somewhat unfortunate. It also highlights a problem with the
patch wrt. RPS. We don't wait for the GPU to actually change frequencies
in set_rps() because that would slow things down too much. So I have to
wonder how much luck is needed to make this workaround really effective.
> >
> >>
> >> + a comment would be nice why it's there.
> >
> > I will add comments to the acquire calls.
> >
> >> Do we need a kconfig select/depends on the iosf_mbi thing? Or some
> >> ifdefs?
> >
> > No, the iosf_mbi header defines empty inline versions of
> > iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
> > this does mean that iosf_mbi must be builtin if the i915
> > driver is. I'll add:
> >
> > depends on DRM_I915=IOSF_MBI || IOSF_MBI=y
> >
> > To the i915 Kconfig to enforce this.
>
> Hmm, ok so that does not work (long cyclic dependency through the
> selection of ACPI_VIDEO).
>
> So I've now added this instead:
>
> # iosf_mbi needs to be builtin if we are builtin
> select IOSF_MBI if DRM_I915=y
That's probably not going to help anyone since i915 is usually a module.
>
> Regards,
>
> Hans
>
>
> >>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
> >>> Signed-off-by: Hans de Goede
> >>> Tested-by: tagorereddy
> >>> ---
> >>> Changes in v2:
> >>> -Spelling: P-Unit, PMIC
> >>> -Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
> >>> ---
> >>> drivers/gpu/drm/i915/intel_display.c| 6 ++
> >>> drivers/gpu/drm/i915/intel_pm.c | 9 +
> >>> drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +
> >>> 3 files changed, 24 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c
> >>> b/drivers/gpu/drm/i915/intel_display.c
> >>> index 5604701..13e5152 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -47,6 +47,7 @@
> >>> #include
> >>> #include
> >>> #include
> >>> +#include
> >>>
> >>> static bool is_mmio_work(struct intel_flip_work *work)
> >>> {
> >>> @@ -6421,6 +6422,8 @@ static void valleyview_set_cdclk(struct drm_device
> >>> *dev, int cdclk)
> >>> cmd = 0;
> >>>
> >>> mutex_lock(&dev_priv->rps.hw_lock);
> >>> +iosf_mbi_punit_acquire();
> >>> +
> >>> val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> >>> val &= ~DSPFREQGUAR_MASK;
> >>> val |= (cmd << DSPFREQGUAR_SHIFT);
> >>> @@ -6430,6 +6433,7 @@ static void valleyview_set_cdclk(struct drm_device
> >>> *dev, int cdclk)
> >>> 50)) {
> >>> DRM_ERROR("timed out waiting for CDclk change\n");
> >>> }
> >>> +iosf_mbi_punit_release();
> >>> mutex_unlock(&dev_priv->rps.hw_lock);
> >>>
> >>> mutex_lock(&dev_priv->sb_lock);
> >>> @@ -6497,6 +6501,7 @@ static void cherryview_set_cdclk(struct drm_device
> >>> *dev, int cdclk)
> >>> cmd = DIV_ROUND_CLOSEST(dev_priv->hpll_freq << 1, cdclk) - 1;
> >>>
> >>> mutex_lock(&dev_priv->rps.hw_lock);
> >>> +iosf_mbi_punit_acquire();
> >>> val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> >>> val &= ~DSPFREQGUAR_MASK_CHV;
> >>> val |= (cmd << DSPFREQGUAR_SHIFT_CHV);
> >>> @@ -6506,6 +6511,7 @@ static void cherryview_set_cdclk(struct drm_device
> >>> *dev, int cdclk)
> >>> 50)) {
> >>> DRM_ERROR("timed out waiting for CDclk change\n");
> >>> }
> >>> +iosf_mbi_punit_release();
> >>> mutex_unlock(&dev_priv->rps.hw_lock);
> >>>
> >>> intel_update_cdclk(dev_priv);
> >>> diff --git a/drivers/gpu/drm/i915/intel_pm.c
> >>> b/drivers/gpu/drm/i915/intel_pm.c
> >>> index 249623d..adff84a 100644
> >>> --- a/drivers/gpu/drm/i915/intel_pm.c
> >>> +++
Re: [PATCH] MAINTAINERS: add dma-fence* files to Sync File maintainership
Hi Gustavo, Daniel, On 30 January 2017 at 14:07, Daniel Vetter wrote: > On Fri, Jan 27, 2017 at 03:54:44PM -0200, Gustavo Padovan wrote: > > From: Gustavo Padovan > > > > As Sync File is highly dependent on dma-fence* tracks it > > under SYNC FILE_FRAMEWORK as well. > > > > Signed-off-by: Gustavo Padovan > > Acked-by: Daniel Vetter > > Of course, feel free to add my Acked-by: Sumit Semwal > I guess wait for an ack from Sumit, then push. > -Daniel > > --- > > MAINTAINERS | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index bdc4843..c1c000d 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -3976,6 +3976,7 @@ S: Maintained > > L: linux-me...@vger.kernel.org > > L: dri-devel@lists.freedesktop.org > > F: drivers/dma-buf/sync_* > > +F: drivers/dma-buf/dma-fence* > > F: drivers/dma-buf/sw_sync.c > > F: include/linux/sync_file.h > > F: include/uapi/linux/sync_file.h > > -- > > 2.9.3 > > > > ___ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch > ___ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Thanks and regards, Sumit Semwal Linaro Mobile Group - Kernel Team Lead Linaro.org │ Open source software for ARM SoCs ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: DRM Atomic property for color-space conversion
On Fri, Jan 27, 2017 at 05:23:24PM +, Brian Starkey wrote: > Hi, > > We're looking to enable the per-plane color management hardware in > Mali-DP with atomic properties, which has sparked some conversation > around how to handle YCbCr formats. > > As it stands today, it's assumed that a driver will implicitly "do the > right thing" to display a YCbCr buffer. > > YCbCr data often uses different gamma curves and signal ranges (e.g. > BT.609, BT.701, BT.2020, studio range, full-range), so its desirable > to be able to explicitly control the YCbCr to RGB conversion process > from userspace. > > We're proposing adding a "CSC" (color-space conversion) property to > control this - primarily per-plane for framebuffer->pipeline CSC, but > perhaps one per CRTC too for devices which have an RGB pipeline and > want to output in YUV to the display: > > Name: "CSC" > Type: ENUM | ATOMIC; > Enum values (representative): > "default": > Same behaviour as now. "Some kind" of YCbCr->RGB conversion > for YCbCr buffers, bypass for RGB buffers > "disable": > Explicitly disable all colorspace conversion (i.e. use an > identity matrix). > "YCbCr to RGB: BT.709": > Only valid for YCbCr formats. CSC in accordance with BT.709 > using [16..235] for (8-bit) luma values, and [16..240] for > 8-bit chroma values. For 10-bit formats, the range limits are > multiplied by 4. > "YCbCr to RGB: BT.709 full-swing": > Only valid for YCbCr formats. CSC in accordance with BT.709, > but using the full range of each channel. > "YCbCr to RGB: Use CTM":* > Only valid for YCbCr formats. Use the matrix applied via the > plane's CTM property > "RGB to RGB: Use CTM":* > Only valid for RGB formats. Use the matrix applied via the > plane's CTM property > "Use CTM":* > Valid for any format. Use the matrix applied via the plane's > CTM property > ... any other values for BT.601, BT.2020, RGB to YCbCr etc. etc. as > they are required. Having some RGB2RGB and YCBCR2RGB things in the same property seems weird. I would just go with something very simple like: YCBCR_TO_RGB_CSC: * BT.601 * BT.709 * custom matrix And trying to use the same thing for the crtc stuff is probably not going to end well. Like Daniel said we already have the 'Broadcast RGB' property muddying the waters there, and that stuff also ties in with what colorspace we signal to the sink via infoframes/whatever the DP thing was called. So my gut feeling is that trying to use the same property everywhere will just end up messy. -- Ville Syrjälä Intel OTC ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[Bug 193651] New: Amdgpu error messages at boot with Amd RX460
https://bugzilla.kernel.org/show_bug.cgi?id=193651 Bug ID: 193651 Summary: Amdgpu error messages at boot with Amd RX460 Product: Drivers Version: 2.5 Kernel Version: 4.11-wip Hardware: x86-64 OS: Linux Tree: Mainline Status: NEW Severity: low Priority: P1 Component: Video(DRI - non Intel) Assignee: drivers_video-...@kernel-bugs.osdl.org Reporter: fin4...@hotmail.com Regression: No Created attachment 253581 --> https://bugzilla.kernel.org/attachment.cgi?id=253581&action=edit dmesg logfile I have Gigabyte RX460 2GB gpu card, Debian testing Xfce and adg5f drm-next-4.11-wip kernel downloaded and compiled as today. Computer works ok but the dmesg command shows the following boot errors that might interest amdgou driver developers. Mounting my home partiton fails amdgpu IB tests: [7.001953] [drm] ib test on ring 12 succeeded [7.055163] EXT4-fs (sda5): mounted filesystem with ordered data mode. Opts: (null) [8.011874] [drm:0xa01360ce] *ERROR* amdgpu: IB test timed out. [8.011910] [drm:0xa00e1b4b] *ERROR* amdgpu: failed testing IB on ring 13 (-110). [8.011943] [drm:0xa00be574] *ERROR* ib ring test failed (-110). Some powerplay errors: [4.888584] amdgpu: [powerplay] [AVFS] Something is broken. See log! [4.891452] amdgpu: [powerplay] Can't find requested voltage id in vdd_dep_on_sclk table! [4.894807] amdgpu: [powerplay] failed to send message 309 ret is 254 [4.894824] amdgpu: [powerplay] failed to send pre message 14e ret is 254 Bios recognition errors: [4.729628] [drm] BIOS signature incorrect 20 7 [4.729635] amdgpu :01:00.0: Invalid PCI ROM header signature: expecting 0xaa55, got 0x -- 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: [Intel-gfx] [PATCH 0/3] drm/i915: Handle hanging during nonblocking modeset correctly.
Op 30-01-17 om 09:17 schreef Daniel Vetter: > On Fri, Jan 27, 2017 at 03:08:45PM +, Chris Wilson wrote: >> On Fri, Jan 27, 2017 at 03:58:08PM +0100, Daniel Vetter wrote: >>> On Fri, Jan 27, 2017 at 02:31:55PM +, Chris Wilson wrote: On Fri, Jan 27, 2017 at 03:21:29PM +0100, Daniel Vetter wrote: > On Fri, Jan 27, 2017 at 09:30:50AM +, Chris Wilson wrote: >> On Thu, Jan 26, 2017 at 04:59:21PM +0100, Maarten Lankhorst wrote: >>> When writing some testcases for nonblocking modesets. I found out that >>> the >>> infinite wait on the old fb was causing issues. >> The crux of the issue here is the locked wait for old dependencies and >> the inability to inject the intel_prepare_reset disabling of all planes. >> There are a couple of locked waits on struct_mutex within the modeset >> locks for intel_overlay and if we happen to be using the display plane >> for the first time. >> >> The first I suggested solving using fences to track dependencies and >> keep the order between atomic states. Cancelling the outstanding >> modesets, replacing with a disable and then on restore jumping to the >> final state look doable. It also requires avoiding the struct_mutex for >> disabling, which is quite easy. To avoid the wait under struct_mutex, >> we've talked about switching to mmio, but for starters we could move the >> wait from inside intel_overlay into the fence for the atomic operation. >> (But's that a little more surgery than we would like for intel_overlay I >> guess - dig out Ville's patches for overlay planes?) And to prevent the >> wait under struct_mutex for pin_to_display_plane, my plane is to move >> that to an async fenced operation that is then naturally waited upon by >> the atomic modeset. > A bit more a hack, but a different idea, and I think hack for gen234.0 is > ok: > > We complete all the requests before we start the hw reset with fence.error > = -EIO. But we do this only when we need to get at the display locks. A > slightly more elegant solution would be to trylock modeset locks, and if > one of them fails (and only then) complete all requests with -EIO to get > the concurrent modeset to proceed before we reset the hardware. That's > essentially the logic we had before all the reworks, and it worked. But I > didn't look at how scary that all would be to make it work again ... The modeset lock may not just be waiting on our requests (even on pnv we can expect that there are already users celebrating that pnv+nouveau finally works ;) and that the display is not the only user/observer of those requests. Using the requests to break the modeset lock just feels like the wrong approach. >>> It's a cycle, and we need to break it somewhere. Another option might be >>> to break the cycle the same way we do it for gem locks: Wake up everyone >>> and restart the modeset ioctl. Since the trouble only happens for >>> synchronous modesets where we hold the locks while waiting for fences, we >>> can also break out of that and restart. And I also don't think that would >>> leak to other drivers, after all our gem locking restart dances also don't >>> leak to other drivers - it's just our own driver's lock which are affected >>> by these special wakupe semantics. >> It's a queue of nonblocking modesets that we need to worry about, afaik. >> Moving the wait for blocking modeset outside of modeset lock is easily >> achievable (and avoiding the other waits under both the modeset + >> struct_mutex I have at least an idea for). So the challenge is how to >> inject all-planes-off for gen3 and then allow the queue to continue again >> afterwards. > Hm right, I missed the nonblocking updates which don't take locks. But > assuming we do the display reset for gpu resets as a full modeset (i.e. > going through ->atomic_commit) it should still work out correctly: > > Starting state: gpu is hung, nonblocking modeset waiting for some requests > to complete. Missing one evil detail here, else things would have moved forward.. A unrelated thread performs a blocking commit, and holds all locks until the nonblocking modeset completes. > 1. hangcheck kicks in, fires off reset work. > > 2. We complete all requests with fence.error = -EIO and wake up any > waiters. That means no re-queueing for older platforms, but oh well. > > 3. We grab all the display locks. Nothing happens yet. > > 4. We reset the chip, display dies. > > 5. We run ->atomic_commit to restore things. This will also force the > nonblocking commit worker to complete before this display restore touches > anything. > > The only trouble I see is that the nonblocking worker can still touch the > display block while we kill it, which isn't awesome. But we can fix that > by waiting for all pending nonblocking commits in step 3 manually (without > calling into atomic_commit), as long as we do step 2. > >
Re: [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings
Hi,
On 30-01-17 14:10, Ville Syrjälä wrote:
On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:
Hi,
On 01/28/2017 05:25 PM, Hans de Goede wrote:
Hi,
On 01/27/2017 02:51 PM, Ville Syrjälä wrote:
On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:
Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
around P-Unit write accesses.
Can't we just stuff the calls into the actual punit write function
rather than sprinkling them all over the place?
punit access is acquired across sections like this:
iosf_mbi_punit_acquire();
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
val &= ~DSPFREQGUAR_MASK;
val |= (cmd << DSPFREQGUAR_SHIFT);
vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
50)) {
DRM_ERROR("timed out waiting for CDclk change\n");
}
iosf_mbi_punit_release();
Where we want to wait for the requested change to have taken
effect before releasing the punit.
Hmm. That's somewhat unfortunate. It also highlights a problem with the
patch wrt. RPS. We don't wait for the GPU to actually change frequencies
in set_rps() because that would slow things down too much. So I have to
wonder how much luck is needed to make this workaround really effective.
So the history of this patch-set is that I wrote this patch before
writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
active (patch 12/13). Since a lot of testing was done with this
patch included in the patch-set and since it seemed a good idea
regardless (given my experience with accessing the punit vs
pmic bus accesses) I decided to leave it in.
Possibly just the patch to get FORCEWAKE_ALL is enough, that one
actually fixed things for me. That is also why I made this the
last patch in the set. I asked tagorereddy to test his system
without this patch, but he did not get around to that.
After all we do tell the punit to not touch the bus by acquiring
the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
for RPS freq changes it honors that and properly waits. Maybe it
honors that for all punit requests i915 does and the only real
problem is the forcewake stuff ?
I can try to drop this patch from my queue and run without it
for a while and see if things don't regress. And also ask
tagorereddy again to test his system that way.
Does that (dropping this patch for now) sound like a good idea?
+ a comment would be nice why it's there.
I will add comments to the acquire calls.
Do we need a kconfig select/depends on the iosf_mbi thing? Or some
ifdefs?
No, the iosf_mbi header defines empty inline versions of
iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
this does mean that iosf_mbi must be builtin if the i915
driver is. I'll add:
depends on DRM_I915=IOSF_MBI || IOSF_MBI=y
To the i915 Kconfig to enforce this.
Hmm, ok so that does not work (long cyclic dependency through the
selection of ACPI_VIDEO).
So I've now added this instead:
# iosf_mbi needs to be builtin if we are builtin
select IOSF_MBI if DRM_I915=y
That's probably not going to help anyone since i915 is usually a module.
Right, that is fine, then either the IOSF_MBI symbols are available,
or IOSF_MBI is disabled and we get the inline nops from the header.
The problem scenario is DRM_I915=y and IOSF_MBI=m, which is not very
realistic IMHO, but will get triggered by the random-config testing
several contributors do and lead to an unresolved symbol error there.
Hmm, thinking about this, this hunk actually belongs in 12/13 as that
is the first patch to use iosf_mbi functions.
Regards,
Hans
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede
Tested-by: tagorereddy
---
Changes in v2:
-Spelling: P-Unit, PMIC
-Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
---
drivers/gpu/drm/i915/intel_display.c| 6 ++
drivers/gpu/drm/i915/intel_pm.c | 9 +
drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +
3 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index 5604701..13e5152 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -47,6 +47,7 @@
#include
#include
#include
+#include
static bool is_mmio_work(struct intel_flip_work *work)
{
@@ -6421,6 +6422,8 @@ static void valleyview_set_cdclk(struct drm_device *dev,
int cdclk)
cmd = 0;
mutex_lock(&dev_priv->rps.hw_lock);
+iosf_mbi_punit_acquire();
+
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
val &= ~DSPFREQGUAR_MASK;
val |= (cmd << DSPFREQGUAR_SHIFT);
@@ -6430,6 +6433,7 @@ static void valleyview_set_
Re: [PATCH v2 0/2] drm: Add support for tiny LCD displays
Den 30.01.2017 09.44, skrev Daniel Vetter:
Hi Noralf,
On Fri, Jan 27, 2017 at 08:56:29PM +0100, Noralf Trønnes wrote:
This is an attempt at providing a DRM version of drivers/staging/fbtft.
The tinydrm library provides a very simplified view of DRM in particular
for tiny displays that has onboard video memory and is connected through
a slow bus like SPI/I2C.
Only core patches this time.
Noralf.
Changes since version 1:
- Add tinydrm.rst
- Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that).
Hm, this sounds like a buglet in the drm framework ... how do we call
lastclose when the driver is disappearing? I do see a drm_lastclose call
at the beginning of drm_dev_unregister (which we might want to remove for
KMS drivers, it doesn't make much sense imo), but that shouldn't result in
troubles.
Ah, I see, I'm tearing down fbdev before unregistering drm.
That should be reversed.
static void tinydrm_unregister(struct tinydrm_device *tdev)
{
drm_crtc_force_disable_all(tdev->drm);
if (tdev->fbdev_cma) {
drm_fbdev_cma_fini(tdev->fbdev_cma);
tdev->fbdev_cma = NULL;
}
drm_dev_unregister(tdev->drm);
}
- Remove some DRM_DEBUG*()
- Write-combined memory has uncached reads, so speed up by copying/buffering
one pixel line before conversion.
Hm, why are you using write-combining memory? Or is that needed so that
you can (if available) use hw spi engines?
That comes with using the CMA helper:
drm_fbdev_cma_create_with_funcs() -> drm_gem_cma_create() -> dma_alloc_wc()
Here's a comment I have added in the code for why I set the DMA mask on
the SPI device and why it will work:
/*
* Even though it's not the SPI device that does DMA (the master does),
* the dma mask is necessary for the dma_alloc_wc() in
* drm_gem_cma_create(). The dma_addr returned will be a physical
* adddress which might be different from the bus address, but this is
* not a problem since the address will not be used.
* The virtual address is used in the transfer and the SPI core
* re-maps it on the SPI master device using the DMA streaming API
* (spi_map_buf()).
*/
Either way, I think this all looks good, pls submit a pull request to Dave
with these two patches as soon as latest drm-misc has landed (I'll send a
pull request for that later today).
I'm confused, I thought you wanted the core patches through drm-misc
and the rest as a pull request to Dave.
This is what you said:
Looks all pretty. A bunch of ideas below, but all optional. For
merging I
think simplest to first get the core patches in through drm-misc,
and then
you can submit a pull request to Dave for tinydrm+backends (just
needs an
ack for the dt parts from dt maintainers), including MAINTAINERS entry.
Ack from my side.
Another one: Do you want to maintain tinydrm as part of the drm-misc
group, i.e. want commit rights there? That would also help a bit with
pushing all your great drm refactoring patches through the machinery ...
My goal is to port staging/fbtft to drm. Whether or not I will do work
in drm core (refactoring) besides the tinydrm drivers when that is
accomplished, I don't know. Working on drm as a hobbyist is very
difficult (for me at least) because it is very complex, it changes all
the time and on top of that it has a high volume mailinglist.
It takes effort to keep up.
So I will probably end up doing only tinydrm maintanence.
As for how that is best done, I don't know. Once I have covered a 3-4
controller types, a new driver is just a copy of a similar one with a
different register initialization. This means that it's easy to review
patches. They will all look the same, more or less.
So for me it's ofc best if I can review the patches and then commit
them myself. As for my own patches, if I need that tit for tat
reviewing to get them in, it will be difficult for me to review other
drivers because I have no idea how a GPU operates, and to keep up with
drm best pratices will be next to impossible for me.
If the Device Tree guys allows me to add fbtft support to tinydrm, then
there won't be much activity once the fbtft drivers have been ported
over. The uncertainty stems from the fact that the fbtft drivers are
written as controller drivers, but they contain panel specific things
like register setup and how to do rotation.
So compatible = "fb_ili9341", supports a controller with a specific
panel, but it can support other panels through the 'init' DT property
which encodes register values and delays (which is a no-no).
Noralf.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings
On Mon, Jan 30, 2017 at 04:02:19PM +0100, Hans de Goede wrote:
> Hi,
>
> On 30-01-17 14:10, Ville Syrjälä wrote:
> > On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 01/28/2017 05:25 PM, Hans de Goede wrote:
> >>> Hi,
> >>>
> >>> On 01/27/2017 02:51 PM, Ville Syrjälä wrote:
> On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:
> > Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
> > request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
> > around P-Unit write accesses.
>
> Can't we just stuff the calls into the actual punit write function
> rather than sprinkling them all over the place?
> >>>
> >>> punit access is acquired across sections like this:
> >>>
> >>> iosf_mbi_punit_acquire();
> >>>
> >>> val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> >>> val &= ~DSPFREQGUAR_MASK;
> >>> val |= (cmd << DSPFREQGUAR_SHIFT);
> >>> vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> >>> if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
> >>> DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
> >>> 50)) {
> >>> DRM_ERROR("timed out waiting for CDclk change\n");
> >>> }
> >>> iosf_mbi_punit_release();
> >>>
> >>> Where we want to wait for the requested change to have taken
> >>> effect before releasing the punit.
> >
> > Hmm. That's somewhat unfortunate. It also highlights a problem with the
> > patch wrt. RPS. We don't wait for the GPU to actually change frequencies
> > in set_rps() because that would slow things down too much. So I have to
> > wonder how much luck is needed to make this workaround really effective.
>
> So the history of this patch-set is that I wrote this patch before
> writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
> active (patch 12/13). Since a lot of testing was done with this
> patch included in the patch-set and since it seemed a good idea
> regardless (given my experience with accessing the punit vs
> pmic bus accesses) I decided to leave it in.
>
> Possibly just the patch to get FORCEWAKE_ALL is enough, that one
> actually fixed things for me. That is also why I made this the
> last patch in the set. I asked tagorereddy to test his system
> without this patch, but he did not get around to that.
>
> After all we do tell the punit to not touch the bus by acquiring
> the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
> for RPS freq changes it honors that and properly waits. Maybe it
> honors that for all punit requests i915 does and the only real
> problem is the forcewake stuff ?
>
> I can try to drop this patch from my queue and run without it
> for a while and see if things don't regress. And also ask
> tagorereddy again to test his system that way.
>
> Does that (dropping this patch for now) sound like a good idea?
More test results couldn't hurt at least. It also makes me wonder if
just bumping the timeouts to some ridiculously high value would fix
the problem as well.
>
> + a comment would be nice why it's there.
> >>>
> >>> I will add comments to the acquire calls.
> >>>
> Do we need a kconfig select/depends on the iosf_mbi thing? Or some
> ifdefs?
> >>>
> >>> No, the iosf_mbi header defines empty inline versions of
> >>> iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
> >>> this does mean that iosf_mbi must be builtin if the i915
> >>> driver is. I'll add:
> >>>
> >>> depends on DRM_I915=IOSF_MBI || IOSF_MBI=y
> >>>
> >>> To the i915 Kconfig to enforce this.
> >>
> >> Hmm, ok so that does not work (long cyclic dependency through the
> >> selection of ACPI_VIDEO).
> >>
> >> So I've now added this instead:
> >>
> >># iosf_mbi needs to be builtin if we are builtin
> >>select IOSF_MBI if DRM_I915=y
> >
> > That's probably not going to help anyone since i915 is usually a module.
>
> Right, that is fine, then either the IOSF_MBI symbols are available,
> or IOSF_MBI is disabled and we get the inline nops from the header.
>
> The problem scenario is DRM_I915=y and IOSF_MBI=m, which is not very
> realistic IMHO, but will get triggered by the random-config testing
> several contributors do and lead to an unresolved symbol error there.
Well, from the user POV anything with IOSF_MBI==n can be a problem.
So I'm not sure if we should allow that.
>
> Hmm, thinking about this, this hunk actually belongs in 12/13 as that
> is the first patch to use iosf_mbi functions.
>
> Regards,
>
> Hans
>
>
>
> > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
> > Signed-off-by: Hans de Goede
> > Tested-by: tagorereddy
> > ---
> > Changes in v2:
> > -Spelling: P-Unit, PMIC
> > -Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
> > ---
> > drivers/gpu/drm/i915/intel_display.c| 6 ++
> > drivers/gpu/drm/
[PATCH] drm/i915: Skip modeset locking when atomic pageflips are used.
With the atomic helper for pageflips there are no CS flips when
that require resetting, so on most platforms we can completely
skip the locking.
Because we may end up reverting the page_flip patch add an explicit
function pointer check so that if someone reverts the page flip patch
there will not be any issues if this is forgotten.
Signed-off-by: Maarten Lankhorst
Testcase: kms_busy.extended-modeset-hang-oldfb-*
---
This is a standalone patch to fix modeset hangs on g4x+.
The path for gen4 and lower is simulated in
kms_busy.extended-modeset-hang-oldfb-with-reset
and still fails.
drivers/gpu/drm/i915/intel_display.c | 12
1 file changed, 12 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
b/drivers/gpu/drm/i915/intel_display.c
index d8db1caec1b8..1dd480a6752a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3529,6 +3529,8 @@ static bool gpu_reset_clobbers_display(struct
drm_i915_private *dev_priv)
INTEL_GEN(dev_priv) < 5 && !IS_G4X(dev_priv);
}
+static const struct drm_crtc_funcs intel_crtc_funcs;
+
void intel_prepare_reset(struct drm_i915_private *dev_priv)
{
struct drm_device *dev = &dev_priv->drm;
@@ -3536,6 +3538,11 @@ void intel_prepare_reset(struct drm_i915_private
*dev_priv)
struct drm_atomic_state *state;
int ret;
+ if (intel_crtc_funcs.page_flip == drm_atomic_helper_page_flip &&
+ !i915.force_reset_modeset_test &&
+ !gpu_reset_clobbers_display(dev_priv))
+ return;
+
/*
* Need mode_config.mutex so that we don't
* trample ongoing ->detect() and whatnot.
@@ -3584,6 +3591,11 @@ void intel_finish_reset(struct drm_i915_private
*dev_priv)
struct drm_atomic_state *state = dev_priv->modeset_restore_state;
int ret;
+ if (intel_crtc_funcs.page_flip == drm_atomic_helper_page_flip &&
+ !i915.force_reset_modeset_test &&
+ !gpu_reset_clobbers_display(dev_priv))
+ return;
+
/*
* Flips in the rings will be nuked by the reset,
* so complete all pending flips so that user space
--
2.7.4
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 24/24] drm/rockchip: dw-mipi-dsi: support read commands
On Sun, Jan 29, 2017 at 01:24:44PM +, John Keeping wrote:
> I haven't found any method for getting the length of a response, so this
> just uses the requested rx_len
>
> Signed-off-by: John Keeping
> ---
> v3:
> - Fix checkpatch warnings
> Unchanged in v2
>
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 56
> ++
> 1 file changed, 56 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index cf3ca6b0cbdb..cc58ada75425 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -678,6 +678,56 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi
> *dsi,
> return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val);
> }
>
> +static int dw_mipi_dsi_dcs_read(struct dw_mipi_dsi *dsi,
> + const struct mipi_dsi_msg *msg)
> +{
> + const u8 *tx_buf = msg->tx_buf;
> + u8 *rx_buf = msg->rx_buf;
> + size_t i;
> + int ret, val;
> +
> + dsi_write(dsi, DSI_PCKHDL_CFG, EN_CRC_RX | EN_ECC_RX | EN_BTA);
> + dsi_write(dsi, DSI_GEN_HDR,
> + GEN_HDATA(tx_buf[0]) | GEN_HTYPE(msg->type));
> +
> + ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> + val, !(val & GEN_RD_CMD_BUSY), 1000,
> + CMD_PKT_STATUS_TIMEOUT_US);
> + if (ret < 0) {
> + dev_err(dsi->dev, "failed to read command response\n");
> + return ret;
> + }
> +
> + for (i = 0; i < msg->rx_len;) {
> + u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
> +
> + while (i < msg->rx_len) {
> + rx_buf[i] = pld & 0xff;
> + pld >>= 8;
> + i++;
> + }
> + }
AFAICT, the outer for loop just initializes i and ensures msg->rx_len is
non-zero?
I think the following would be easier to read (and safe against the case where
msg->rx_len > sizeof(pld) (even though this shouldn't happen according to DCS
spec)).
if (msg->rx_len > 0) {
u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
memcpy(rx_buf, &pld, MIN(msg->rx_len, sizeof(pld));
}
> +
> + return msg->rx_len;
> +}
> +
> +static int dw_mipi_dsi_set_max_return_packet_size(struct dw_mipi_dsi *dsi,
> + size_t len)
> +{
> + u8 val[] = { len & 0xff, (len >> 8) & 0xff };
> + struct mipi_dsi_msg msg = {
> + .channel = dsi->channel,
> + .type = MIPI_DSI_SET_MAXIMUM_RETURN_PACKET_SIZE,
> + .tx_buf = val,
> + .tx_len = 2,
> + };
> +
> + if (len > 0x)
> + return -EINVAL;
> +
> + return dw_mipi_dsi_dcs_short_write(dsi, &msg);
> +}
> +
> static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
>const struct mipi_dsi_msg *msg)
> {
> @@ -695,6 +745,12 @@ static ssize_t dw_mipi_dsi_host_transfer(struct
> mipi_dsi_host *host,
> case MIPI_DSI_DCS_LONG_WRITE:
> ret = dw_mipi_dsi_dcs_long_write(dsi, msg);
> break;
> + case MIPI_DSI_DCS_READ:
> + ret = dw_mipi_dsi_set_max_return_packet_size(dsi, msg->rx_len);
> + if (ret < 0)
> + return ret;
> + ret = dw_mipi_dsi_dcs_read(dsi, msg);
> + break;
> default:
> dev_err(dsi->dev, "unsupported message type 0x%02x\n",
> msg->type);
> --
> 2.11.0.197.gb556de5.dirty
>
> ___
> 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 v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings
Hi,
On 30-01-17 16:11, Ville Syrjälä wrote:
On Mon, Jan 30, 2017 at 04:02:19PM +0100, Hans de Goede wrote:
Hi,
On 30-01-17 14:10, Ville Syrjälä wrote:
On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:
Hi,
On 01/28/2017 05:25 PM, Hans de Goede wrote:
Hi,
On 01/27/2017 02:51 PM, Ville Syrjälä wrote:
On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:
Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
around P-Unit write accesses.
Can't we just stuff the calls into the actual punit write function
rather than sprinkling them all over the place?
punit access is acquired across sections like this:
iosf_mbi_punit_acquire();
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
val &= ~DSPFREQGUAR_MASK;
val |= (cmd << DSPFREQGUAR_SHIFT);
vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
50)) {
DRM_ERROR("timed out waiting for CDclk change\n");
}
iosf_mbi_punit_release();
Where we want to wait for the requested change to have taken
effect before releasing the punit.
Hmm. That's somewhat unfortunate. It also highlights a problem with the
patch wrt. RPS. We don't wait for the GPU to actually change frequencies
in set_rps() because that would slow things down too much. So I have to
wonder how much luck is needed to make this workaround really effective.
So the history of this patch-set is that I wrote this patch before
writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
active (patch 12/13). Since a lot of testing was done with this
patch included in the patch-set and since it seemed a good idea
regardless (given my experience with accessing the punit vs
pmic bus accesses) I decided to leave it in.
Possibly just the patch to get FORCEWAKE_ALL is enough, that one
actually fixed things for me. That is also why I made this the
last patch in the set. I asked tagorereddy to test his system
without this patch, but he did not get around to that.
After all we do tell the punit to not touch the bus by acquiring
the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
for RPS freq changes it honors that and properly waits. Maybe it
honors that for all punit requests i915 does and the only real
problem is the forcewake stuff ?
I can try to drop this patch from my queue and run without it
for a while and see if things don't regress. And also ask
tagorereddy again to test his system that way.
Does that (dropping this patch for now) sound like a good idea?
More test results couldn't hurt at least. It also makes me wonder if
just bumping the timeouts to some ridiculously high value would fix
the problem as well.
I've already tried bumping the forcewake timeout from 50 to 250ms,
before writing the patch to just get forcewake_all before the pmic
bus access begins, that does not fix things, and since we busy wait
for this timeout from non-sleeping context 250ms already is way too
high.
+ a comment would be nice why it's there.
I will add comments to the acquire calls.
Do we need a kconfig select/depends on the iosf_mbi thing? Or some
ifdefs?
No, the iosf_mbi header defines empty inline versions of
iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
this does mean that iosf_mbi must be builtin if the i915
driver is. I'll add:
depends on DRM_I915=IOSF_MBI || IOSF_MBI=y
To the i915 Kconfig to enforce this.
Hmm, ok so that does not work (long cyclic dependency through the
selection of ACPI_VIDEO).
So I've now added this instead:
# iosf_mbi needs to be builtin if we are builtin
select IOSF_MBI if DRM_I915=y
That's probably not going to help anyone since i915 is usually a module.
Right, that is fine, then either the IOSF_MBI symbols are available,
or IOSF_MBI is disabled and we get the inline nops from the header.
The problem scenario is DRM_I915=y and IOSF_MBI=m, which is not very
realistic IMHO, but will get triggered by the random-config testing
several contributors do and lead to an unresolved symbol error there.
Well, from the user POV anything with IOSF_MBI==n can be a problem.
So I'm not sure if we should allow that.
So you're suggesting we just add an unconditional "select IOSF_MBI"
to the i915 Kconfig entry?
Regards,
Hans
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede
Tested-by: tagorereddy
---
Changes in v2:
-Spelling: P-Unit, PMIC
-Adjust for iosf_mbi_punit_lock/_unlock to _acquire/_release rename
---
drivers/gpu/drm/i915/intel_display.c| 6 ++
drivers/gpu/drm/i915/intel_pm.c | 9 +
drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +
3 files changed, 24 insertions(+)
diff --git a/drivers/gpu/drm/i915/intel_display.c
Re: [PATCH v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings
On Mon, Jan 30, 2017 at 04:27:58PM +0100, Hans de Goede wrote:
> Hi,
>
> On 30-01-17 16:11, Ville Syrjälä wrote:
> > On Mon, Jan 30, 2017 at 04:02:19PM +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 30-01-17 14:10, Ville Syrjälä wrote:
> >>> On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:
> Hi,
>
> On 01/28/2017 05:25 PM, Hans de Goede wrote:
> > Hi,
> >
> > On 01/27/2017 02:51 PM, Ville Syrjälä wrote:
> >> On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:
> >>> Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
> >>> request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
> >>> around P-Unit write accesses.
> >>
> >> Can't we just stuff the calls into the actual punit write function
> >> rather than sprinkling them all over the place?
> >
> > punit access is acquired across sections like this:
> >
> > iosf_mbi_punit_acquire();
> >
> > val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
> > val &= ~DSPFREQGUAR_MASK;
> > val |= (cmd << DSPFREQGUAR_SHIFT);
> > vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
> > if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
> > DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
> > 50)) {
> > DRM_ERROR("timed out waiting for CDclk change\n");
> > }
> > iosf_mbi_punit_release();
> >
> > Where we want to wait for the requested change to have taken
> > effect before releasing the punit.
> >>>
> >>> Hmm. That's somewhat unfortunate. It also highlights a problem with the
> >>> patch wrt. RPS. We don't wait for the GPU to actually change frequencies
> >>> in set_rps() because that would slow things down too much. So I have to
> >>> wonder how much luck is needed to make this workaround really effective.
> >>
> >> So the history of this patch-set is that I wrote this patch before
> >> writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
> >> active (patch 12/13). Since a lot of testing was done with this
> >> patch included in the patch-set and since it seemed a good idea
> >> regardless (given my experience with accessing the punit vs
> >> pmic bus accesses) I decided to leave it in.
> >>
> >> Possibly just the patch to get FORCEWAKE_ALL is enough, that one
> >> actually fixed things for me. That is also why I made this the
> >> last patch in the set. I asked tagorereddy to test his system
> >> without this patch, but he did not get around to that.
> >>
> >> After all we do tell the punit to not touch the bus by acquiring
> >> the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
> >> for RPS freq changes it honors that and properly waits. Maybe it
> >> honors that for all punit requests i915 does and the only real
> >> problem is the forcewake stuff ?
> >>
> >> I can try to drop this patch from my queue and run without it
> >> for a while and see if things don't regress. And also ask
> >> tagorereddy again to test his system that way.
> >>
> >> Does that (dropping this patch for now) sound like a good idea?
> >
> > More test results couldn't hurt at least. It also makes me wonder if
> > just bumping the timeouts to some ridiculously high value would fix
> > the problem as well.
>
> I've already tried bumping the forcewake timeout from 50 to 250ms,
> before writing the patch to just get forcewake_all before the pmic
> bus access begins, that does not fix things,
And you bumped the i2c mutex timeout as well? Or does that fail somehow
gracefully if it can't get the mutex?
> and since we busy wait
> for this timeout from non-sleeping context 250ms already is way too
> high.
Sure, but I'm just trying to understand if the problem is simply caused
by proceeding with some hardware access without getting the i2c mutex.
>
> >> + a comment would be nice why it's there.
> >
> > I will add comments to the acquire calls.
> >
> >> Do we need a kconfig select/depends on the iosf_mbi thing? Or some
> >> ifdefs?
> >
> > No, the iosf_mbi header defines empty inline versions of
> > iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
> > this does mean that iosf_mbi must be builtin if the i915
> > driver is. I'll add:
> >
> > depends on DRM_I915=IOSF_MBI || IOSF_MBI=y
> >
> > To the i915 Kconfig to enforce this.
>
> Hmm, ok so that does not work (long cyclic dependency through the
> selection of ACPI_VIDEO).
>
> So I've now added this instead:
>
> # iosf_mbi needs to be builtin if we are builtin
> select IOSF_MBI if DRM_I915=y
> >>>
> >>> That's probably not going to help anyone since i915 is usually a module.
> >>
> >> Right, that is fine, then either the IOSF_MBI symbols are available,
> >> or IOSF_MBI is disabled and we get the inline
Re: [PATCH v3 03/24] drm/rockchip: dw-mipi-dsi: remove mode_set hook
On Sun, Jan 29, 2017 at 01:24:23PM +, John Keeping wrote:
> This is not needed since we can access the mode via the CRTC from the
> enable hook. Also remove the "mode" field that is no longer used.
>
Reviewed-by: Sean Paul
> Signed-off-by: John Keeping
> Reviewed-by: Chris Zhong
> ---
> v3:
> - Add Chris' Reviewed-by
> New in v2
>
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 13 +
> 1 file changed, 1 insertion(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index cdbd25087e83..bd92e58b64f3 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -286,7 +286,6 @@ struct dw_mipi_dsi {
> u32 format;
> u16 input_div;
> u16 feedback_div;
> - struct drm_display_mode *mode;
>
> const struct dw_mipi_dsi_plat_data *pdata;
> };
> @@ -816,15 +815,6 @@ static void dw_mipi_dsi_clear_err(struct dw_mipi_dsi
> *dsi)
> dsi_write(dsi, DSI_INT_MSK1, 0);
> }
>
> -static void dw_mipi_dsi_encoder_mode_set(struct drm_encoder *encoder,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> -{
> - struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> -
> - dsi->mode = adjusted_mode;
> -}
> -
> static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
> {
> struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> @@ -854,7 +844,7 @@ static void dw_mipi_dsi_encoder_disable(struct
> drm_encoder *encoder)
> static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
> {
> struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> - struct drm_display_mode *mode = dsi->mode;
> + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
> int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
> u32 val;
> int ret;
> @@ -930,7 +920,6 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder
> *encoder,
> static struct drm_encoder_helper_funcs
> dw_mipi_dsi_encoder_helper_funcs = {
> .enable = dw_mipi_dsi_encoder_enable,
> - .mode_set = dw_mipi_dsi_encoder_mode_set,
> .disable = dw_mipi_dsi_encoder_disable,
> .atomic_check = dw_mipi_dsi_encoder_atomic_check,
> };
> --
> 2.11.0.197.gb556de5.dirty
>
> ___
> 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 v3 04/24] drm/rockchip: dw-mipi-dsi: fix command header writes
On Sun, Jan 29, 2017 at 01:24:24PM +, John Keeping wrote:
> In a couple of places here we use "val" for the value that is about to
> be written to a register but then reuse the same variable for the value
> of a status register before we get around to writing it. Rename the
> value to be written to so that we write the value we intend to and not
> what we have just read from the status register.
>
Oh my.
Reviewed-by: Sean Paul
> Signed-off-by: John Keeping
> Tested-by: Chris Zhong
> Reviewed-by: Chris Zhong
> ---
> Unchanged in v3
> Unchanged in v2
>
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index bd92e58b64f3..4cbbbcb619b7 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -542,9 +542,10 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host
> *host,
> return 0;
> }
>
> -static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32 val)
> +static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32
> hdr_val)
> {
> int ret;
> + u32 val;
>
> ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS,
>val, !(val & GEN_CMD_FULL), 1000,
> @@ -554,7 +555,7 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct
> dw_mipi_dsi *dsi, u32 val)
> return ret;
> }
>
> - dsi_write(dsi, DSI_GEN_HDR, val);
> + dsi_write(dsi, DSI_GEN_HDR, hdr_val);
>
> ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS,
>val, val & (GEN_CMD_EMPTY | GEN_PLD_W_EMPTY),
> @@ -587,8 +588,9 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi
> *dsi,
> {
> const u32 *tx_buf = msg->tx_buf;
> int len = msg->tx_len, pld_data_bytes = sizeof(*tx_buf), ret;
> - u32 val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type);
> + u32 hdr_val = GEN_HDATA(msg->tx_len) | GEN_HTYPE(msg->type);
> u32 remainder = 0;
> + u32 val;
>
> if (msg->tx_len < 3) {
> dev_err(dsi->dev, "wrong tx buf length %zu for long write\n",
> @@ -617,7 +619,7 @@ static int dw_mipi_dsi_dcs_long_write(struct dw_mipi_dsi
> *dsi,
> }
> }
>
> - return dw_mipi_dsi_gen_pkt_hdr_write(dsi, val);
> + return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val);
> }
>
> static ssize_t dw_mipi_dsi_host_transfer(struct mipi_dsi_host *host,
> --
> 2.11.0.197.gb556de5.dirty
>
> ___
> 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 v3 02/24] drm/rockchip: dw-mipi-dsi: pass mode in where needed
On Sun, Jan 29, 2017 at 01:24:22PM +, John Keeping wrote:
> This shows that we only use the mode from the enable function and
> prepares us to remove the "mode" field and the mode_set hook in the next
> commit.
>
Reviewed-by: Sean Paul
> Signed-off-by: John Keeping
> Reviewed-by: Chris Zhong
> ---
> v3:
> - Add Chris' Reviewed-by
> New in v2
>
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 41
> ++
> 1 file changed, 22 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index bbd992299f73..cdbd25087e83 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -330,11 +330,11 @@ static int max_mbps_to_testdin(unsigned int max_mbps)
> * The controller should generate 2 frames before
> * preparing the peripheral.
> */
> -static void dw_mipi_dsi_wait_for_two_frames(struct dw_mipi_dsi *dsi)
> +static void dw_mipi_dsi_wait_for_two_frames(struct drm_display_mode *mode)
> {
> int refresh, two_frames;
>
> - refresh = drm_mode_vrefresh(dsi->mode);
> + refresh = drm_mode_vrefresh(mode);
> two_frames = DIV_ROUND_UP(MSEC_PER_SEC, refresh) * 2;
> msleep(two_frames);
> }
> @@ -459,7 +459,8 @@ static int dw_mipi_dsi_phy_init(struct dw_mipi_dsi *dsi)
> return ret;
> }
>
> -static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi)
> +static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi *dsi,
> + struct drm_display_mode *mode)
> {
> unsigned int i, pre;
> unsigned long mpclk, pllref, tmp;
> @@ -474,7 +475,7 @@ static int dw_mipi_dsi_get_lane_bps(struct dw_mipi_dsi
> *dsi)
> return bpp;
> }
>
> - mpclk = DIV_ROUND_UP(dsi->mode->clock, MSEC_PER_SEC);
> + mpclk = DIV_ROUND_UP(mode->clock, MSEC_PER_SEC);
> if (mpclk) {
> /* take 1 / 0.9, since mbps must big than bandwidth of RGB */
> tmp = mpclk * (bpp / dsi->lanes) * 10 / 9;
> @@ -742,43 +743,44 @@ static void dw_mipi_dsi_command_mode_config(struct
> dw_mipi_dsi *dsi)
>
> /* Get lane byte clock cycles. */
> static u32 dw_mipi_dsi_get_hcomponent_lbcc(struct dw_mipi_dsi *dsi,
> +struct drm_display_mode *mode,
> u32 hcomponent)
> {
> u32 frac, lbcc;
>
> lbcc = hcomponent * dsi->lane_mbps * MSEC_PER_SEC / 8;
>
> - frac = lbcc % dsi->mode->clock;
> - lbcc = lbcc / dsi->mode->clock;
> + frac = lbcc % mode->clock;
> + lbcc = lbcc / mode->clock;
> if (frac)
> lbcc++;
>
> return lbcc;
> }
>
> -static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi)
> +static void dw_mipi_dsi_line_timer_config(struct dw_mipi_dsi *dsi,
> + struct drm_display_mode *mode)
> {
> u32 htotal, hsa, hbp, lbcc;
> - struct drm_display_mode *mode = dsi->mode;
>
> htotal = mode->htotal;
> hsa = mode->hsync_end - mode->hsync_start;
> hbp = mode->htotal - mode->hsync_end;
>
> - lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, htotal);
> + lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, htotal);
> dsi_write(dsi, DSI_VID_HLINE_TIME, lbcc);
>
> - lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, hsa);
> + lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hsa);
> dsi_write(dsi, DSI_VID_HSA_TIME, lbcc);
>
> - lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, hbp);
> + lbcc = dw_mipi_dsi_get_hcomponent_lbcc(dsi, mode, hbp);
> dsi_write(dsi, DSI_VID_HBP_TIME, lbcc);
> }
>
> -static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi)
> +static void dw_mipi_dsi_vertical_timing_config(struct dw_mipi_dsi *dsi,
> +struct drm_display_mode *mode)
> {
> u32 vactive, vsa, vfp, vbp;
> - struct drm_display_mode *mode = dsi->mode;
>
> vactive = mode->vdisplay;
> vsa = mode->vsync_end - mode->vsync_start;
> @@ -852,11 +854,12 @@ static void dw_mipi_dsi_encoder_disable(struct
> drm_encoder *encoder)
> static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
> {
> struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> + struct drm_display_mode *mode = dsi->mode;
> int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
> u32 val;
> int ret;
>
> - ret = dw_mipi_dsi_get_lane_bps(dsi);
> + ret = dw_mipi_dsi_get_lane_bps(dsi, mode);
> if (ret < 0)
> return;
>
> @@ -866,13 +869,13 @@ static void dw_mipi_dsi_encoder_enable(struct
> drm_encoder *encoder)
> }
>
> dw_mipi_dsi_init(dsi);
> - dw_mipi_dsi_dpi_config(dsi, dsi->mode);
> + dw_mipi_dsi_dpi_config(dsi, mode);
> dw_mipi_dsi_packet_handler_config(dsi);
> dw_mipi_dsi_video_mode_config(dsi);
> - dw_mip
Re: [PATCH v3 01/24] drm/rockchip: dw-mipi-dsi: don't configure hardware in mode_set for MIPI
On Sun, Jan 29, 2017 at 01:24:21PM +, John Keeping wrote:
> With atomic modesetting the hardware will be powered off when the
> mode_set function is called. We should configure the hardware in the
> enable function, which is the atomic version of "commit" so let's use
> the enable hook rather than commit while we're at it.
>
Reviewed-by: Sean Paul
> Signed-off-by: John Keeping
> ---
> v3:
> - Squash together with the commit to s/commit/enable/
> Unchanged in v2
>
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 49
> +++---
> 1 file changed, 21 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index d9aa382bb629..bbd992299f73 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -819,34 +819,8 @@ static void dw_mipi_dsi_encoder_mode_set(struct
> drm_encoder *encoder,
> struct drm_display_mode *adjusted_mode)
> {
> struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> - int ret;
>
> dsi->mode = adjusted_mode;
> -
> - ret = dw_mipi_dsi_get_lane_bps(dsi);
> - if (ret < 0)
> - return;
> -
> - if (clk_prepare_enable(dsi->pclk)) {
> - dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
> - return;
> - }
> -
> - dw_mipi_dsi_init(dsi);
> - dw_mipi_dsi_dpi_config(dsi, mode);
> - dw_mipi_dsi_packet_handler_config(dsi);
> - dw_mipi_dsi_video_mode_config(dsi);
> - dw_mipi_dsi_video_packet_config(dsi, mode);
> - dw_mipi_dsi_command_mode_config(dsi);
> - dw_mipi_dsi_line_timer_config(dsi);
> - dw_mipi_dsi_vertical_timing_config(dsi);
> - dw_mipi_dsi_dphy_timing_config(dsi);
> - dw_mipi_dsi_dphy_interface_config(dsi);
> - dw_mipi_dsi_clear_err(dsi);
> - if (drm_panel_prepare(dsi->panel))
> - dev_err(dsi->dev, "failed to prepare panel\n");
> -
> - clk_disable_unprepare(dsi->pclk);
> }
>
> static void dw_mipi_dsi_encoder_disable(struct drm_encoder *encoder)
> @@ -875,17 +849,36 @@ static void dw_mipi_dsi_encoder_disable(struct
> drm_encoder *encoder)
> clk_disable_unprepare(dsi->pclk);
> }
>
> -static void dw_mipi_dsi_encoder_commit(struct drm_encoder *encoder)
> +static void dw_mipi_dsi_encoder_enable(struct drm_encoder *encoder)
> {
> struct dw_mipi_dsi *dsi = encoder_to_dsi(encoder);
> int mux = drm_of_encoder_active_endpoint_id(dsi->dev->of_node, encoder);
> u32 val;
> + int ret;
> +
> + ret = dw_mipi_dsi_get_lane_bps(dsi);
> + if (ret < 0)
> + return;
>
> if (clk_prepare_enable(dsi->pclk)) {
> dev_err(dsi->dev, "%s: Failed to enable pclk\n", __func__);
> return;
> }
>
> + dw_mipi_dsi_init(dsi);
> + dw_mipi_dsi_dpi_config(dsi, dsi->mode);
> + dw_mipi_dsi_packet_handler_config(dsi);
> + dw_mipi_dsi_video_mode_config(dsi);
> + dw_mipi_dsi_video_packet_config(dsi, dsi->mode);
> + dw_mipi_dsi_command_mode_config(dsi);
> + dw_mipi_dsi_line_timer_config(dsi);
> + dw_mipi_dsi_vertical_timing_config(dsi);
> + dw_mipi_dsi_dphy_timing_config(dsi);
> + dw_mipi_dsi_dphy_interface_config(dsi);
> + dw_mipi_dsi_clear_err(dsi);
> + if (drm_panel_prepare(dsi->panel))
> + dev_err(dsi->dev, "failed to prepare panel\n");
> +
> dw_mipi_dsi_phy_init(dsi);
> dw_mipi_dsi_wait_for_two_frames(dsi);
>
> @@ -933,7 +926,7 @@ dw_mipi_dsi_encoder_atomic_check(struct drm_encoder
> *encoder,
>
> static struct drm_encoder_helper_funcs
> dw_mipi_dsi_encoder_helper_funcs = {
> - .commit = dw_mipi_dsi_encoder_commit,
> + .enable = dw_mipi_dsi_encoder_enable,
> .mode_set = dw_mipi_dsi_encoder_mode_set,
> .disable = dw_mipi_dsi_encoder_disable,
> .atomic_check = dw_mipi_dsi_encoder_atomic_check,
> --
> 2.11.0.197.gb556de5.dirty
>
> ___
> 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
[PATCH] drm: mali-dp: add custom plane ->reset hook
The reset hook needs to allocate space for a
malidp_plane_state, which is larger than drm_plane_state.
Otherwise, the hook is identical to the default one.
Signed-off-by: Mihail Atanassov
---
drivers/gpu/drm/arm/malidp_planes.c | 23 ++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/arm/malidp_planes.c
b/drivers/gpu/drm/arm/malidp_planes.c
index eff2fe4..686ff86 100644
--- a/drivers/gpu/drm/arm/malidp_planes.c
+++ b/drivers/gpu/drm/arm/malidp_planes.c
@@ -58,6 +58,27 @@ static void malidp_de_plane_destroy(struct drm_plane *plane)
devm_kfree(plane->dev->dev, mp);
}
+/*
+ * Replicate what the default ->reset hook does: free the state pointer and
+ * allocate a new empty object. We just need enough space to store
+ * a malidp_plane_state instead of a drm_plane_state.
+ */
+static void malidp_plane_reset(struct drm_plane *plane)
+{
+ struct malidp_plane_state *state = to_malidp_plane_state(plane->state);
+
+ if (state)
+ __drm_atomic_helper_plane_destroy_state(&state->base);
+ kfree(state);
+ plane->state = NULL;
+ state = kzalloc(sizeof(*state), GFP_KERNEL);
+ if (state) {
+ state->base.plane = plane;
+ state->base.rotation = DRM_ROTATE_0;
+ plane->state = &state->base;
+ }
+}
+
static struct
drm_plane_state *malidp_duplicate_plane_state(struct drm_plane *plane)
{
@@ -92,7 +113,7 @@ static void malidp_destroy_plane_state(struct drm_plane
*plane,
.disable_plane = drm_atomic_helper_disable_plane,
.set_property = drm_atomic_helper_plane_set_property,
.destroy = malidp_de_plane_destroy,
- .reset = drm_atomic_helper_plane_reset,
+ .reset = malidp_plane_reset,
.atomic_duplicate_state = malidp_duplicate_plane_state,
.atomic_destroy_state = malidp_destroy_plane_state,
};
--
1.9.1
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm: mali-dp: add custom plane ->reset hook
On Mon, Jan 30, 2017 at 03:42:26PM +, Mihail Atanassov wrote:
> The reset hook needs to allocate space for a
> malidp_plane_state, which is larger than drm_plane_state.
> Otherwise, the hook is identical to the default one.
>
> Signed-off-by: Mihail Atanassov
Acked-by: Liviu Dudau
Thanks,
Liviu
> ---
> drivers/gpu/drm/arm/malidp_planes.c | 23 ++-
> 1 file changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_planes.c
> b/drivers/gpu/drm/arm/malidp_planes.c
> index eff2fe4..686ff86 100644
> --- a/drivers/gpu/drm/arm/malidp_planes.c
> +++ b/drivers/gpu/drm/arm/malidp_planes.c
> @@ -58,6 +58,27 @@ static void malidp_de_plane_destroy(struct drm_plane
> *plane)
> devm_kfree(plane->dev->dev, mp);
> }
>
> +/*
> + * Replicate what the default ->reset hook does: free the state pointer and
> + * allocate a new empty object. We just need enough space to store
> + * a malidp_plane_state instead of a drm_plane_state.
> + */
> +static void malidp_plane_reset(struct drm_plane *plane)
> +{
> + struct malidp_plane_state *state = to_malidp_plane_state(plane->state);
> +
> + if (state)
> + __drm_atomic_helper_plane_destroy_state(&state->base);
> + kfree(state);
> + plane->state = NULL;
> + state = kzalloc(sizeof(*state), GFP_KERNEL);
> + if (state) {
> + state->base.plane = plane;
> + state->base.rotation = DRM_ROTATE_0;
> + plane->state = &state->base;
> + }
> +}
> +
> static struct
> drm_plane_state *malidp_duplicate_plane_state(struct drm_plane *plane)
> {
> @@ -92,7 +113,7 @@ static void malidp_destroy_plane_state(struct drm_plane
> *plane,
> .disable_plane = drm_atomic_helper_disable_plane,
> .set_property = drm_atomic_helper_plane_set_property,
> .destroy = malidp_de_plane_destroy,
> - .reset = drm_atomic_helper_plane_reset,
> + .reset = malidp_plane_reset,
> .atomic_duplicate_state = malidp_duplicate_plane_state,
> .atomic_destroy_state = malidp_destroy_plane_state,
> };
> --
> 1.9.1
>
--
| 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 v2 13/13] drm/i915: Acquire P-Unit access when modifying P-Unit settings
Hi,
On 30-01-17 16:38, Ville Syrjälä wrote:
On Mon, Jan 30, 2017 at 04:27:58PM +0100, Hans de Goede wrote:
Hi,
On 30-01-17 16:11, Ville Syrjälä wrote:
On Mon, Jan 30, 2017 at 04:02:19PM +0100, Hans de Goede wrote:
Hi,
On 30-01-17 14:10, Ville Syrjälä wrote:
On Sat, Jan 28, 2017 at 06:18:45PM +0100, Hans de Goede wrote:
Hi,
On 01/28/2017 05:25 PM, Hans de Goede wrote:
Hi,
On 01/27/2017 02:51 PM, Ville Syrjälä wrote:
On Mon, Jan 23, 2017 at 10:09:58PM +0100, Hans de Goede wrote:
Make sure the P-Unit or the PMIC i2c bus is not in use when we send a
request to the P-Unit by calling iosf_mbi_punit_acquire() / _release()
around P-Unit write accesses.
Can't we just stuff the calls into the actual punit write function
rather than sprinkling them all over the place?
punit access is acquired across sections like this:
iosf_mbi_punit_acquire();
val = vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ);
val &= ~DSPFREQGUAR_MASK;
val |= (cmd << DSPFREQGUAR_SHIFT);
vlv_punit_write(dev_priv, PUNIT_REG_DSPFREQ, val);
if (wait_for((vlv_punit_read(dev_priv, PUNIT_REG_DSPFREQ) &
DSPFREQSTAT_MASK) == (cmd << DSPFREQSTAT_SHIFT),
50)) {
DRM_ERROR("timed out waiting for CDclk change\n");
}
iosf_mbi_punit_release();
Where we want to wait for the requested change to have taken
effect before releasing the punit.
Hmm. That's somewhat unfortunate. It also highlights a problem with the
patch wrt. RPS. We don't wait for the GPU to actually change frequencies
in set_rps() because that would slow things down too much. So I have to
wonder how much luck is needed to make this workaround really effective.
So the history of this patch-set is that I wrote this patch before
writing the patch to get FORCEWAKE_ALL before the pmic bus becomes
active (patch 12/13). Since a lot of testing was done with this
patch included in the patch-set and since it seemed a good idea
regardless (given my experience with accessing the punit vs
pmic bus accesses) I decided to leave it in.
Possibly just the patch to get FORCEWAKE_ALL is enough, that one
actually fixed things for me. That is also why I made this the
last patch in the set. I asked tagorereddy to test his system
without this patch, but he did not get around to that.
After all we do tell the punit to not touch the bus by acquiring
the pmic bus semaphore from i2c-desigware-baytrail.c, so maybe
for RPS freq changes it honors that and properly waits. Maybe it
honors that for all punit requests i915 does and the only real
problem is the forcewake stuff ?
I can try to drop this patch from my queue and run without it
for a while and see if things don't regress. And also ask
tagorereddy again to test his system that way.
Does that (dropping this patch for now) sound like a good idea?
More test results couldn't hurt at least. It also makes me wonder if
just bumping the timeouts to some ridiculously high value would fix
the problem as well.
I've already tried bumping the forcewake timeout from 50 to 250ms,
before writing the patch to just get forcewake_all before the pmic
bus access begins, that does not fix things,
And you bumped the i2c mutex timeout as well? Or does that fail somehow
gracefully if it can't get the mutex?
It will fail the i2c transfer with -ETIMEOUT, which will make the driver
report an error instead of e.g. the battery level, but it should not
affect the forcewake calls and those still failed with the large
timeout. So yes basically the i2c mutex fails gracefully.
and since we busy wait
for this timeout from non-sleeping context 250ms already is way too
high.
Sure, but I'm just trying to understand if the problem is simply caused
by proceeding with some hardware access without getting the i2c mutex.
Understood.
+ a comment would be nice why it's there.
I will add comments to the acquire calls.
Do we need a kconfig select/depends on the iosf_mbi thing? Or some
ifdefs?
No, the iosf_mbi header defines empty inline versions of
iosf_mbi_punit_acquire / _release if IOSF_MBI is disabled,
this does mean that iosf_mbi must be builtin if the i915
driver is. I'll add:
depends on DRM_I915=IOSF_MBI || IOSF_MBI=y
To the i915 Kconfig to enforce this.
Hmm, ok so that does not work (long cyclic dependency through the
selection of ACPI_VIDEO).
So I've now added this instead:
# iosf_mbi needs to be builtin if we are builtin
select IOSF_MBI if DRM_I915=y
That's probably not going to help anyone since i915 is usually a module.
Right, that is fine, then either the IOSF_MBI symbols are available,
or IOSF_MBI is disabled and we get the inline nops from the header.
The problem scenario is DRM_I915=y and IOSF_MBI=m, which is not very
realistic IMHO, but will get triggered by the random-config testing
several contributors do and lead to an unresolved symbol error there.
Well, from the user POV anything with IOSF_MBI==n
[PATCH 0/4] drm/msm: cleanup gpu bindings
Existing bindings for the gpu are based on the downstream android kgsl driver (a subset of the downstream bindings). But that isn't really how we want the upstream bindings to look. For now we have held off on adding gpu nodes to upstream dt files, but now that all the dependencies are in place for some devices, it is time to clean things up so we can start adding this missing gpu nodes. Note that these patches preserve compatibility with downstream dt files because, at this point, it is easy and convenient to not break people's patchsets for upstream support of various devices. Rob Clark (4): drm/msm: remove qcom,gpu-pwrlevels bindings drm/msm: drop qcom,chipid drm/msm: drop quirks binding drm/msm: drop _clk suffix from clk names .../devicetree/bindings/display/msm/gpu.txt| 38 - drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 +- drivers/gpu/drm/msm/adreno/adreno_device.c | 65 -- drivers/gpu/drm/msm/adreno/adreno_gpu.c| 1 - drivers/gpu/drm/msm/adreno/adreno_gpu.h| 4 +- drivers/gpu/drm/msm/msm_drv.c | 20 +++ drivers/gpu/drm/msm/msm_drv.h | 1 + drivers/gpu/drm/msm/msm_gpu.c | 7 +-- 8 files changed, 88 insertions(+), 52 deletions(-) -- 2.9.3 ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 2/4] drm/msm: drop qcom,chipid
The original way we determined the gpu version was based on downstream
bindings from android kernel. A cleaner way is to get the version from
the compatible string.
Note that no upstream dtb uses these bindings. But the code still
supports falling back to the legacy bindings (with a warning), so that
we are still compatible with the gpu dt node from android device
kernels.
Signed-off-by: Rob Clark
---
.../devicetree/bindings/display/msm/gpu.txt| 11 +++---
drivers/gpu/drm/msm/adreno/adreno_device.c | 43 +-
drivers/gpu/drm/msm/msm_drv.c | 1 +
3 files changed, 49 insertions(+), 6 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt
b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 747b984..7ac3052 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -1,7 +1,11 @@
Qualcomm adreno/snapdragon GPU
Required properties:
-- compatible: "qcom,adreno-3xx"
+- compatible: "qcom,adreno-XYZ.W", "qcom,adreno"
+for example: "qcom,adreno-306.0", "qcom,adreno"
+ Note that you need to list the less specific "qcom,adreno" (since this
+ is what the device is matched on), in addition to the more specific
+ with the chip-id.
- reg: Physical base address and length of the controller's registers.
- interrupts: The interrupt signal from the gpu.
- clocks: device clocks
@@ -10,8 +14,6 @@ Required properties:
* "core_clk"
* "iface_clk"
* "mem_iface_clk"
-- qcom,chipid: gpu chip-id. Note this may become optional for future
- devices if we can reliably read the chipid from hw
Example:
@@ -19,7 +21,7 @@ Example:
...
gpu: qcom,kgsl-3d0@430 {
- compatible = "qcom,adreno-3xx";
+ compatible = "qcom,adreno-320.2", "qcom,adreno";
reg = <0x0430 0x2>;
reg-names = "kgsl_3d0_reg_memory";
interrupts = ;
@@ -32,6 +34,5 @@ Example:
<&mmcc GFX3D_CLK>,
<&mmcc GFX3D_AHB_CLK>,
<&mmcc MMSS_IMEM_AHB_CLK>;
- qcom,chipid = <0x03020100>;
};
};
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 7b9181b2..fdaef67 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -189,6 +189,46 @@ static const struct {
{ "qcom,gpu-quirk-fault-detect-mask", ADRENO_QUIRK_FAULT_DETECT_MASK },
};
+static int find_chipid(struct device *dev, u32 *chipid)
+{
+ struct device_node *node = dev->of_node;
+ struct property *prop;
+ const char *compat;
+ int ret;
+
+ /* first search the compat strings for qcom,adreno-XYZ.W: */
+ prop = of_find_property(node, "compatible", NULL);
+ for (compat = of_prop_next_string(prop, NULL); compat;
+compat = of_prop_next_string(prop, compat)) {
+ unsigned rev, patch;
+
+ if (sscanf(compat, "qcom,adreno-%u.%u", &rev, &patch) != 2)
+ continue;
+
+ *chipid = 0;
+ *chipid |= (rev / 100) << 24; /* core */
+ rev %= 100;
+ *chipid |= (rev / 10) << 16; /* major */
+ rev %= 10;
+ *chipid |= rev << 8; /* minor */
+ *chipid |= patch;
+
+ return 0;
+ }
+
+ /* and if that fails, fall back to legacy "qcom,chipid" property: */
+ ret = of_property_read_u32(node, "qcom,chipid", chipid);
+ if (ret)
+ return ret;
+
+ dev_warn(dev, "Using legacy qcom,chipid binding!\n");
+ dev_warn(dev, "Use compatible qcom,adreno-%u%u%u.%u instead.\n",
+ (*chipid >> 24) & 0xff, (*chipid >> 16) & 0xff,
+ (*chipid >> 8) & 0xff, *chipid & 0xff);
+
+ return 0;
+}
+
static int adreno_bind(struct device *dev, struct device *master, void *data)
{
static struct adreno_platform_config config = {};
@@ -196,7 +236,7 @@ static int adreno_bind(struct device *dev, struct device
*master, void *data)
u32 val;
int ret, i;
- ret = of_property_read_u32(node, "qcom,chipid", &val);
+ ret = find_chipid(dev, &val);
if (ret) {
dev_err(dev, "could not find chipid: %d\n", ret);
return ret;
@@ -265,6 +305,7 @@ static int adreno_remove(struct platform_device *pdev)
}
static const struct of_device_id dt_match[] = {
+ { .compatible = "qcom,adreno" },
{ .compatible = "qcom,adreno-3xx" },
/* for backwards compat w/ downstream kgsl DT files: */
{ .compatible = "qcom,kgsl-3d0" },
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index e29bb66..6b85c41 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -985,6 +985,7
[PATCH 4/4] drm/msm: drop _clk suffix from clk names
Suggested by Rob Herring. We still support the old names for
compatibility with downstream android dt files.
Cc: Rob Herring
Signed-off-by: Rob Clark
---
Documentation/devicetree/bindings/display/msm/gpu.txt | 12 ++--
drivers/gpu/drm/msm/msm_drv.c | 19 +++
drivers/gpu/drm/msm/msm_drv.h | 1 +
drivers/gpu/drm/msm/msm_gpu.c | 7 +++
4 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt
b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 7ac3052..43fac0f 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -11,9 +11,9 @@ Required properties:
- clocks: device clocks
See ../clocks/clock-bindings.txt for details.
- clock-names: the following clocks are required:
- * "core_clk"
- * "iface_clk"
- * "mem_iface_clk"
+ * "core"
+ * "iface"
+ * "mem_iface"
Example:
@@ -27,9 +27,9 @@ Example:
interrupts = ;
interrupt-names = "kgsl_3d0_irq";
clock-names =
- "core_clk",
- "iface_clk",
- "mem_iface_clk";
+ "core",
+ "iface",
+ "mem_iface";
clocks =
<&mmcc GFX3D_CLK>,
<&mmcc GFX3D_AHB_CLK>,
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 6b85c41..a51d783 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -91,6 +91,25 @@ module_param(dumpstate, bool, 0600);
* Util/helpers:
*/
+struct clk *msm_clk_get(struct platform_device *pdev, const char *name)
+{
+ struct clk *clk;
+ char name2[32];
+
+ clk = devm_clk_get(&pdev->dev, name);
+ if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
+ return clk;
+
+ snprintf(name2, sizeof(name2), "%s_clk", name);
+
+ clk = devm_clk_get(&pdev->dev, name2);
+ if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
+ dev_warn(&pdev->dev, "Using legacy clk name binding. Use "
+ "\"%s\" instead of \"%s\"\n", name, name2);
+
+ return clk;
+}
+
void __iomem *msm_ioremap(struct platform_device *pdev, const char *name,
const char *dbgname)
{
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index ed4dad3..5f6f48f 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -318,6 +318,7 @@ static inline int msm_debugfs_late_init(struct drm_device
*dev) { return 0; }
static inline void msm_rd_dump_submit(struct msm_gem_submit *submit) {}
#endif
+struct clk *msm_clk_get(struct platform_device *pdev, const char *name);
void __iomem *msm_ioremap(struct platform_device *pdev, const char *name,
const char *dbgname);
void msm_writel(u32 data, void __iomem *addr);
diff --git a/drivers/gpu/drm/msm/msm_gpu.c b/drivers/gpu/drm/msm/msm_gpu.c
index d8420be..7b29843 100644
--- a/drivers/gpu/drm/msm/msm_gpu.c
+++ b/drivers/gpu/drm/msm/msm_gpu.c
@@ -564,8 +564,7 @@ static irqreturn_t irq_handler(int irq, void *data)
}
static const char *clk_names[] = {
- "core_clk", "iface_clk", "rbbmtimer_clk", "mem_clk",
- "mem_iface_clk", "alt_mem_iface_clk",
+ "core", "iface", "rbbmtimer", "mem", "mem_iface", "alt_mem_iface",
};
int msm_gpu_init(struct drm_device *drm, struct platform_device *pdev,
@@ -629,13 +628,13 @@ int msm_gpu_init(struct drm_device *drm, struct
platform_device *pdev,
/* Acquire clocks: */
for (i = 0; i < ARRAY_SIZE(clk_names); i++) {
- gpu->grp_clks[i] = devm_clk_get(&pdev->dev, clk_names[i]);
+ gpu->grp_clks[i] = msm_clk_get(pdev, clk_names[i]);
DBG("grp_clks[%s]: %p", clk_names[i], gpu->grp_clks[i]);
if (IS_ERR(gpu->grp_clks[i]))
gpu->grp_clks[i] = NULL;
}
- gpu->ebi1_clk = devm_clk_get(&pdev->dev, "bus_clk");
+ gpu->ebi1_clk = msm_clk_get(pdev, "bus");
DBG("ebi1_clk: %p", gpu->ebi1_clk);
if (IS_ERR(gpu->ebi1_clk))
gpu->ebi1_clk = NULL;
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH 3/4] drm/msm: drop quirks binding
This was never documented or used in upstream dtb. It is used by
downstream bindings from android device kernels. But the quirks are
a property of the gpu revision, and as such are redundant to be listed
separately in dt. Instead, move the quirks to the device table.
Signed-off-by: Rob Clark
---
drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++--
drivers/gpu/drm/msm/adreno/adreno_device.c | 18 --
drivers/gpu/drm/msm/adreno/adreno_gpu.c| 1 -
drivers/gpu/drm/msm/adreno/adreno_gpu.h| 4 +---
4 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
index 5f8b368..71b30dd 100644
--- a/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a5xx_gpu.c
@@ -543,7 +543,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
/* Enable RBBM error reporting bits */
gpu_write(gpu, REG_A5XX_RBBM_AHB_CNTL0, 0x0001);
- if (adreno_gpu->quirks & ADRENO_QUIRK_FAULT_DETECT_MASK) {
+ if (adreno_gpu->info->quirks & ADRENO_QUIRK_FAULT_DETECT_MASK) {
/*
* Mask out the activity signals from RB1-3 to avoid false
* positives
@@ -597,7 +597,7 @@ static int a5xx_hw_init(struct msm_gpu *gpu)
gpu_write(gpu, REG_A5XX_PC_DBG_ECO_CNTL, (0x400 << 11 | 0x300 << 22));
- if (adreno_gpu->quirks & ADRENO_QUIRK_TWO_PASS_USE_WFI)
+ if (adreno_gpu->info->quirks & ADRENO_QUIRK_TWO_PASS_USE_WFI)
gpu_rmw(gpu, REG_A5XX_PC_DBG_ECO_CNTL, 0, (1 << 8));
gpu_write(gpu, REG_A5XX_PC_DBG_ECO_CNTL, 0xc0200100);
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index fdaef67..4d0745d9 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -75,12 +75,14 @@ static const struct adreno_info gpulist[] = {
.gmem = (SZ_1M + SZ_512K),
.init = a4xx_gpu_init,
}, {
- .rev = ADRENO_REV(5, 3, 0, ANY_ID),
+ .rev = ADRENO_REV(5, 3, 0, 2),
.revn = 530,
.name = "A530",
.pm4fw = "a530_pm4.fw",
.pfpfw = "a530_pfp.fw",
.gmem = SZ_1M,
+ .quirks = ADRENO_QUIRK_TWO_PASS_USE_WFI |
+ ADRENO_QUIRK_FAULT_DETECT_MASK,
.init = a5xx_gpu_init,
.gpmufw = "a530v3_gpmu.fw2",
},
@@ -181,14 +183,6 @@ static void set_gpu_pdev(struct drm_device *dev,
priv->gpu_pdev = pdev;
}
-static const struct {
- const char *str;
- uint32_t flag;
-} quirks[] = {
- { "qcom,gpu-quirk-two-pass-use-wfi", ADRENO_QUIRK_TWO_PASS_USE_WFI },
- { "qcom,gpu-quirk-fault-detect-mask", ADRENO_QUIRK_FAULT_DETECT_MASK },
-};
-
static int find_chipid(struct device *dev, u32 *chipid)
{
struct device_node *node = dev->of_node;
@@ -234,7 +228,7 @@ static int adreno_bind(struct device *dev, struct device
*master, void *data)
static struct adreno_platform_config config = {};
struct device_node *child, *node = dev->of_node;
u32 val;
- int ret, i;
+ int ret;
ret = find_chipid(dev, &val);
if (ret) {
@@ -270,10 +264,6 @@ static int adreno_bind(struct device *dev, struct device
*master, void *data)
config.slow_rate = 2700;
}
- for (i = 0; i < ARRAY_SIZE(quirks); i++)
- if (of_property_read_bool(node, quirks[i].str))
- config.quirks |= quirks[i].flag;
-
dev->platform_data = &config;
set_gpu_pdev(dev_get_drvdata(master), to_platform_device(dev));
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
index bc2224b..f67e6f8 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.c
@@ -352,7 +352,6 @@ int adreno_gpu_init(struct drm_device *drm, struct
platform_device *pdev,
adreno_gpu->gmem = adreno_gpu->info->gmem;
adreno_gpu->revn = adreno_gpu->info->revn;
adreno_gpu->rev = config->rev;
- adreno_gpu->quirks = config->quirks;
gpu->fast_rate = config->fast_rate;
gpu->slow_rate = config->slow_rate;
diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
index e8d55b0..42e444a 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
@@ -75,6 +75,7 @@ struct adreno_info {
const char *pm4fw, *pfpfw;
const char *gpmufw;
uint32_t gmem;
+ enum adreno_quirks quirks;
struct msm_gpu *(*init)(struct drm_device *dev);
};
@@ -116,8 +117,6 @@ struct adreno_gpu {
* code (a3xx_gpu.c) and stored in this common location.
*/
const unsigned int *reg_offsets;
-
- uint32_t quirks;
};
#define to_adreno_gpu(x) co
[PATCH 1/4] drm/msm: remove qcom,gpu-pwrlevels bindings
The plan is to use the OPP bindings. For now, remove the documentation
for qcom,gpu-pwrlevels, and make the driver fall back to a safe low
clock if the node is not present.
Note that no upstream dtb use this node. For now we keep compatibility
with this node to avoid breaking compatibility with downstream android
dt files.
Signed-off-by: Rob Clark
---
Documentation/devicetree/bindings/display/msm/gpu.txt | 15 ---
drivers/gpu/drm/msm/adreno/adreno_device.c| 6 --
2 files changed, 4 insertions(+), 17 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/msm/gpu.txt
b/Documentation/devicetree/bindings/display/msm/gpu.txt
index 67d0a58..747b984 100644
--- a/Documentation/devicetree/bindings/display/msm/gpu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gpu.txt
@@ -12,12 +12,6 @@ Required properties:
* "mem_iface_clk"
- qcom,chipid: gpu chip-id. Note this may become optional for future
devices if we can reliably read the chipid from hw
-- qcom,gpu-pwrlevels: list of operating points
- - compatible: "qcom,gpu-pwrlevels"
- - for each qcom,gpu-pwrlevel:
-- qcom,gpu-freq: requested gpu clock speed
-- NOTE: downstream android driver defines additional parameters to
- configure memory bandwidth scaling per OPP.
Example:
@@ -39,14 +33,5 @@ Example:
<&mmcc GFX3D_AHB_CLK>,
<&mmcc MMSS_IMEM_AHB_CLK>;
qcom,chipid = <0x03020100>;
- qcom,gpu-pwrlevels {
- compatible = "qcom,gpu-pwrlevels";
- qcom,gpu-pwrlevel@0 {
- qcom,gpu-freq = <45000>;
- };
- qcom,gpu-pwrlevel@1 {
- qcom,gpu-freq = <2700>;
- };
- };
};
};
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c
b/drivers/gpu/drm/msm/adreno/adreno_device.c
index e130b5e..7b9181b2 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -224,8 +224,10 @@ static int adreno_bind(struct device *dev, struct device
*master, void *data)
}
if (!config.fast_rate) {
- dev_err(dev, "could not find clk rates\n");
- return -ENXIO;
+ dev_warn(dev, "could not find clk rates\n");
+ /* This is a safe low speed for all devices: */
+ config.fast_rate = 2;
+ config.slow_rate = 2700;
}
for (i = 0; i < ARRAY_SIZE(quirks); i++)
--
2.9.3
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/2] dt-bindings: Document the VC4 DSI module nodes.
On Fri, Jan 27, 2017 at 8:41 PM, Eric Anholt wrote: > Rob Herring writes: > >> Need to cc DT list if you want it in my queue. >> >> On Mon, Jan 23, 2017 at 6:38 PM, Eric Anholt wrote: >>> These are part of the vc4 display pipeline. >>> >>> Signed-off-by: Eric Anholt >>> --- >>> .../devicetree/bindings/display/brcm,bcm-vc4.txt | 35 >>> ++ >>> 1 file changed, 35 insertions(+) >>> >>> diff --git a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt >>> b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt >>> index e2768703ac2b..34c7fddcea39 100644 >>> --- a/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt >>> +++ b/Documentation/devicetree/bindings/display/brcm,bcm-vc4.txt >>> @@ -56,6 +56,18 @@ Required properties for V3D: >>> - interrupts: The interrupt number >>> See >>> bindings/interrupt-controller/brcm,bcm2835-armctrl-ic.txt >>> >>> +Required properties for DSI: >>> +- compatible: Should be "brcm,bcm2835-dsi0" or "brcm,bcm2835-dsi1" >> >> Are the blocks different? > > They are from the same lineage, but very different (old dsi0 is 1 lane, > dsi1 is 4 lanes). You can see how much the registers move around and > change in the dsi->port conditional blocks in the driver code. Okay, can you add a note here with this detail. With that, Acked-by: Rob Herring ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [v3 PATCH 3/3] drm/amd/display: Switch to using atomic_helper for flip.
On 2017-01-28 09:26 PM, Andrey Grodzovsky wrote:
> Swicth to use atomic helper.
> Start using actual user's given target_vblank value for flip
> instead of current value.
>
> v3:
> Update for movig pflip flags to crtc_state
>
> Change-Id: I25dae6d8c29de5d022a42aa99a18a32674b56cda
> Signed-off-by: Andrey Grodzovsky
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 -
> .../drm/amd/display/amdgpu_dm/amdgpu_dm_types.c| 109
> -
> 2 files changed, 19 insertions(+), 91 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 4c0a86e..3ff3c14 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -443,7 +443,6 @@ struct amdgpu_crtc {
> enum amdgpu_interrupt_state vsync_timer_enabled;
>
> int otg_inst;
> - uint32_t flip_flags;
> /* After Set Mode target will be non-NULL */
> struct dc_target *target;
> };
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> index a443b70..148780d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_types.c
> @@ -1060,83 +1060,6 @@ static int dm_crtc_funcs_atomic_set_property(
> return 0;
> }
>
> -
> -static int amdgpu_atomic_helper_page_flip(struct drm_crtc *crtc,
> - struct drm_framebuffer *fb,
> - struct drm_pending_vblank_event *event,
> - uint32_t flags)
> -{
> - struct drm_plane *plane = crtc->primary;
> - struct amdgpu_crtc *acrtc = to_amdgpu_crtc(crtc);
> - struct drm_atomic_state *state;
> - struct drm_plane_state *plane_state;
> - struct drm_crtc_state *crtc_state;
> - int ret = 0;
> -
> - state = drm_atomic_state_alloc(plane->dev);
> - if (!state)
> - return -ENOMEM;
> -
> - ret = drm_crtc_vblank_get(crtc);
> - if (ret)
> - return ret;
> -
> - state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> -retry:
> - crtc_state = drm_atomic_get_crtc_state(state, crtc);
> - if (IS_ERR(crtc_state)) {
> - ret = PTR_ERR(crtc_state);
> - goto fail;
> - }
> - crtc_state->event = event;
> -
> - plane_state = drm_atomic_get_plane_state(state, plane);
> - if (IS_ERR(plane_state)) {
> - ret = PTR_ERR(plane_state);
> - goto fail;
> - }
> -
> - ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
> - if (ret != 0)
> - goto fail;
> - drm_atomic_set_fb_for_plane(plane_state, fb);
> -
> - /* Make sure we don't accidentally do a full modeset. */
> - state->allow_modeset = false;
> - if (!crtc_state->active) {
> - DRM_DEBUG_ATOMIC("[CRTC:%d] disabled, rejecting legacy flip\n",
> - crtc->base.id);
> - ret = -EINVAL;
> - goto fail;
> - }
> - acrtc->flip_flags = flags;
> -
> - ret = drm_atomic_nonblocking_commit(state);
> -
> -fail:
> - if (ret == -EDEADLK)
> - goto backoff;
> -
> - if (ret)
> - drm_crtc_vblank_put(crtc);
> -
> - drm_atomic_state_put(state);
> -
> - return ret;
> -backoff:
> - drm_atomic_state_clear(state);
> - drm_atomic_legacy_backoff(state);
> -
> - /*
> - * Someone might have exchanged the framebuffer while we dropped locks
> - * in the backoff code. We need to fix up the fb refcount tracking the
> - * core does for us.
> - */
> - plane->old_fb = plane->fb;
> -
> - goto retry;
> -}
> -
> /* Implemented only the options currently availible for the driver */
> static const struct drm_crtc_funcs amdgpu_dm_crtc_funcs = {
> .reset = drm_atomic_helper_crtc_reset,
> @@ -1145,7 +1068,7 @@ static int amdgpu_atomic_helper_page_flip(struct
> drm_crtc *crtc,
> .destroy = amdgpu_dm_crtc_destroy,
> .gamma_set = amdgpu_dm_atomic_crtc_gamma_set,
> .set_config = drm_atomic_helper_set_config,
> - .page_flip = amdgpu_atomic_helper_page_flip,
> + .page_flip_target = drm_atomic_helper_page_flip_target,
> .atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
> .atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
> .atomic_set_property = dm_crtc_funcs_atomic_set_property
> @@ -1626,7 +1549,8 @@ static void clear_unrelated_fields(struct
> drm_plane_state *state)
> static bool page_flip_needed(
> const struct drm_plane_state *new_state,
> const struct drm_plane_state *old_state,
> - bool commit_surface_required)
> + bool commit_surface_required,
> + uint32_t pflip_flags)
> {
> struct drm_plane_state old_state_tmp;
> struct drm_plane_state new_state_tmp;
> @@ -1679,7 +1603,7 @@ static bool page_flip_needed(
>
Re: [PATCH 1/4] drm: qxl: Drop misleading comment
Hi, > drm-misc runs with the committer model, i.e. a few maintainers to do pull > requests and backmerges, a big pile of people directly pushing patches. [ looked at docs too meanwhile ] Sounds good. I guess switching over simplifies things for all of us. We'll avoid issues like the one at hand. Patch flow would be faster too. Right now I'm only doing 1-2 drm-qemu pull requests per kernel release due to low patch volume even for all four qemu drivers combined. > Someone wreaked the entire 01.org domain, but you can get at all the > tooling and documentation with > > git://anongit.freedesktop.org/drm-intel maintainer-tools Hmm. On a quick glance most of dim (except apply-patch) seems to be more useful for maintainers which do merges etc, not so much for committers. I'm used to use https://github.com/stefanha/patches for qemu, and started using it for drm-qemu too. It makes applying patches easier. It manages a patch database, using notmuch mail storage, and can apply patches and patch series from the patch database. That way I don't have to save the patches as mbox somewhere. The tool also picks up [Reviewed,Tested,Acked}-by lines from replies, and it stores the message id (but unlike dim it doesn't build a patchwork link out of it). See bfac9f4fb4d87881375ccdc5c85d5ad59f2f115d for example. Would that format be acceptable for drm-misc? > And then run make in there. We're not yet clear how exactly drivers within > drm-misc would look like (wrt which drivers and how much review and stuff > like that), hence the RFC. Ok. How quickly could I start using drm-misc? I have some pending patches for the 4.11 merge window. Any chance I can push them through drm-misc-next? Or should I better send a pull req to Dave? cheers, Gerd PS: I'm kra...@freedesktop.org ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: make fbdev/fbcon switchable per driver?
On Mon, 30 Jan 2017 09:15:50 +0100 Gerd Hoffmann wrote: > Hi, > > > The vgaarb code has a concept of a vga_default_device(), it's rather > > PCI-centric, but maybe better than nothing. This is typically the > > first VGA class code device found with I/O and MMIO enabled. If fbcon > > defaulted to running on the vga_default_device(), a user could select > > which to use by re-ordering the VM hardware. > > The qemu drivers don't register as vgaarb clients though. Which easily > explains why igd always wins the primary selection, no matter how you > order your hardware. > > So, should they register? The drivers don't need access to the vga > registers[1][2]. The VGA arbiter sets up a notifier on the PCI bus and will add any VGA class code devices it finds. So even if the driver doesn't participate, it'll still be tracked and might be marked as primary. If a graphics driver claims a VGA device that does not depend on VGA region access then the driver should configure the device not to claim VGA accesses (maybe only relevant for integrated graphics - i915 gets this wrong), and register with the arbiter to opt-out of VGA arbitration. Picking a "primary" can be done w/o any of that latter if we agree on the arbiter algorithm of picking the first device with VGA routing to it (or it can be overridden by arch/platform code). Thanks, Alex ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 05/24] drm/rockchip: dw-mipi-dsi: fix generic packet status check
On Sun, Jan 29, 2017 at 01:24:25PM +, John Keeping wrote:
> We want to check that both the GEN_CMD_EMPTY and GEN_PLD_W_EMPTY bits
> are set so we can't just check "val & mask" because that will be true if
> either bit is set.
>
Reviewed-by: Sean Paul
> Signed-off-by: John Keeping
> Reviewed-by: Chris Zhong
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
>
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 4cbbbcb619b7..4be1ff3a42bb 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -545,7 +545,7 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host
> *host,
> static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32
> hdr_val)
> {
> int ret;
> - u32 val;
> + u32 val, mask;
>
> ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS,
>val, !(val & GEN_CMD_FULL), 1000,
> @@ -557,8 +557,9 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct
> dw_mipi_dsi *dsi, u32 hdr_val)
>
> dsi_write(dsi, DSI_GEN_HDR, hdr_val);
>
> + mask = GEN_CMD_EMPTY | GEN_PLD_W_EMPTY;
> ret = readx_poll_timeout(readl, dsi->base + DSI_CMD_PKT_STATUS,
> - val, val & (GEN_CMD_EMPTY | GEN_PLD_W_EMPTY),
> + val, (val & mask) == mask,
>1000, CMD_PKT_STATUS_TIMEOUT_US);
> if (ret < 0) {
> dev_err(dsi->dev, "failed to write command FIFO\n");
> --
> 2.11.0.197.gb556de5.dirty
>
> ___
> 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
[PATCH] drm/atomic: Fix double kfree on crtc_state->event
This is a bug Maarten reported, with the following slab debug backtrace:
[IGT] kms_rotation_crc: starting subtest primary-rotation-180
=
BUG kmalloc-128 (Tainted: G U ): Object already free
-
Disabling lock debugging due to kernel taint
INFO: Allocated in drm_atomic_helper_setup_commit+0x285/0x2f0 [drm_kms_helper]
age=0 cpu=3 pid=1529
___slab_alloc+0x308/0x3b0
__slab_alloc+0xd/0x20
kmem_cache_alloc_trace+0x92/0x1c0
drm_atomic_helper_setup_commit+0x285/0x2f0 [drm_kms_helper]
intel_atomic_commit+0x35/0x4f0 [i915]
drm_atomic_commit+0x46/0x50 [drm]
drm_mode_atomic_ioctl+0x7d4/0xab0 [drm]
drm_ioctl+0x2b3/0x490 [drm]
do_vfs_ioctl+0x69c/0x700
SyS_ioctl+0x4e/0x80
entry_SYSCALL_64_fastpath+0x13/0x94
INFO: Freed in drm_event_cancel_free+0xa3/0xb0 [drm] age=0 cpu=3 pid=1529
__slab_free+0x48/0x2e0
kfree+0x159/0x1a0
drm_event_cancel_free+0xa3/0xb0 [drm]
drm_mode_atomic_ioctl+0x86d/0xab0 [drm]
drm_ioctl+0x2b3/0x490 [drm]
do_vfs_ioctl+0x69c/0x700
SyS_ioctl+0x4e/0x80
entry_SYSCALL_64_fastpath+0x13/0x94
INFO: Slab 0xde1f0997b080 objects=17 used=2 fp=0x92fb65ec2578
flags=0x2008101
INFO: Object 0x92fb65ec2578 @offset=1400 fp=0x92fb65ec2ae8
Redzone 92fb65ec2570: bb bb bb bb bb bb bb bb
Object 92fb65ec2578: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 92fb65ec2588: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 92fb65ec2598: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 92fb65ec25a8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 92fb65ec25b8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 92fb65ec25c8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 92fb65ec25d8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b
Object 92fb65ec25e8: 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b 6b a5
kkk.
Redzone 92fb65ec25f8: bb bb bb bb bb bb bb bb
Padding 92fb65ec2738: 5a 5a 5a 5a 5a 5a 5a 5a
CPU: 3 PID: 180 Comm: kworker/3:2 Tainted: GBU 4.10.0-rc6-patser+
#5039
Hardware name: /NUC5PPYB, BIOS
PYBSWCEL.86A.0031.2015.0601.1712 06/01/2015
Workqueue: events intel_atomic_helper_free_state [i915]
Call Trace:
dump_stack+0x4d/0x6d
print_trailer+0x20c/0x220
free_debug_processing+0x1c6/0x330
? drm_atomic_state_default_clear+0xf7/0x1c0 [drm]
__slab_free+0x48/0x2e0
? drm_atomic_state_default_clear+0xf7/0x1c0 [drm]
kfree+0x159/0x1a0
drm_atomic_state_default_clear+0xf7/0x1c0 [drm]
? drm_atomic_state_clear+0x30/0x30 [drm]
intel_atomic_state_clear+0xd/0x20 [i915]
drm_atomic_state_clear+0x1a/0x30 [drm]
__drm_atomic_state_free+0x13/0x60 [drm]
intel_atomic_helper_free_state+0x5d/0x70 [i915]
process_one_work+0x260/0x4a0
worker_thread+0x2d1/0x4f0
kthread+0x127/0x130
? process_one_work+0x4a0/0x4a0
? kthread_stop+0x120/0x120
ret_from_fork+0x29/0x40
FIX kmalloc-128: Object at 0x92fb65ec2578 not freed
Reported-by: Maarten Lankhorst
Cc: Maarten Lankhorst
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/drm_atomic.c | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 026be94a7d15..366b4bf88206 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -159,15 +159,29 @@ void drm_atomic_state_default_clear(struct
drm_atomic_state *state)
if (!crtc)
continue;
- crtc->funcs->atomic_destroy_state(crtc,
- state->crtcs[i].state);
-
if (state->crtcs[i].commit) {
- kfree(state->crtcs[i].commit->event);
+ /*
+* We need to make sure we don't double-free, which we
+* do by checking for state->event, implicitly since
+* kfree can handle a NULL state->event. We also need to
+* make sure we only kfree the event if it's one created
+* for internal commit tracking (and hence won't be
+* cleared by the caller, like the atomic IOCTL or a
+* legacy pageflip. This is done by checking
+* commit->event.
+*
+* This only works if everyone else sets state->event to
+* NULL when they take it away.
+*/
+ if (state->crtcs[i].commit->event)
+ kfree(state->crtcs[i].commit-
Re: [PATCH v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf
On Sun, Jan 29, 2017 at 01:24:26PM +, John Keeping wrote:
> As a side-effect of this, encode the endianness explicitly rather than
> casting a u16.
>
> Signed-off-by: John Keeping
> Reviewed-by: Chris Zhong
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
>
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 9 +++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 4be1ff3a42bb..2e6ad4591ebf 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -572,8 +572,13 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct
> dw_mipi_dsi *dsi, u32 hdr_val)
> static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi,
> const struct mipi_dsi_msg *msg)
> {
> - const u16 *tx_buf = msg->tx_buf;
> - u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type);
> + const u8 *tx_buf = msg->tx_buf;
> + u32 val = GEN_HTYPE(msg->type);
> +
> + if (msg->tx_len > 0)
> + val |= GEN_HDATA(tx_buf[0]);
> + if (msg->tx_len > 1)
> + val |= GEN_HDATA(tx_buf[1] << 8);
You should probably update the mask inside GEN_HDATA to mask off 8 bits instead
of
16.
Sean
>
> if (msg->tx_len > 2) {
> dev_err(dsi->dev, "too long tx buf length %zu for short
> write\n",
> --
> 2.11.0.197.gb556de5.dirty
>
> ___
> 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 v3 07/24] drm/rockchip: dw-mipi-dsi: include bad value in error message
On Sun, Jan 29, 2017 at 01:24:27PM +, John Keeping wrote: > As an aid to debugging. Reviewed-by: Sean Paul > > Signed-off-by: John Keeping > Reviewed-by: Chris Zhong > --- > v3: > - Add Chris' Reviewed-by > Unchanged in v2 > > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > index 2e6ad4591ebf..92dbc3e56603 100644 > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c > @@ -644,7 +644,8 @@ static ssize_t dw_mipi_dsi_host_transfer(struct > mipi_dsi_host *host, > ret = dw_mipi_dsi_dcs_long_write(dsi, msg); > break; > default: > - dev_err(dsi->dev, "unsupported message type\n"); > + dev_err(dsi->dev, "unsupported message type 0x%02x\n", > + msg->type); > ret = -EINVAL; > } > > -- > 2.11.0.197.gb556de5.dirty > > ___ > 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 v2 0/2] drm: Add support for tiny LCD displays
On Mon, Jan 30, 2017 at 04:03:53PM +0100, Noralf Trønnes wrote:
>
> Den 30.01.2017 09.44, skrev Daniel Vetter:
> > Hi Noralf,
> >
> > On Fri, Jan 27, 2017 at 08:56:29PM +0100, Noralf Trønnes wrote:
> > > This is an attempt at providing a DRM version of drivers/staging/fbtft.
> > >
> > > The tinydrm library provides a very simplified view of DRM in particular
> > > for tiny displays that has onboard video memory and is connected through
> > > a slow bus like SPI/I2C.
> > >
> > > Only core patches this time.
> > >
> > >
> > > Noralf.
> > >
> > >
> > > Changes since version 1:
> > > - Add tinydrm.rst
> > > - Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that).
> > Hm, this sounds like a buglet in the drm framework ... how do we call
> > lastclose when the driver is disappearing? I do see a drm_lastclose call
> > at the beginning of drm_dev_unregister (which we might want to remove for
> > KMS drivers, it doesn't make much sense imo), but that shouldn't result in
> > troubles.
>
> Ah, I see, I'm tearing down fbdev before unregistering drm.
> That should be reversed.
>
> static void tinydrm_unregister(struct tinydrm_device *tdev)
> {
> drm_crtc_force_disable_all(tdev->drm);
>
> if (tdev->fbdev_cma) {
> drm_fbdev_cma_fini(tdev->fbdev_cma);
> tdev->fbdev_cma = NULL;
> }
>
> drm_dev_unregister(tdev->drm);
> }
>
> > > - Remove some DRM_DEBUG*()
> > > - Write-combined memory has uncached reads, so speed up by
> > > copying/buffering
> > >one pixel line before conversion.
> > Hm, why are you using write-combining memory? Or is that needed so that
> > you can (if available) use hw spi engines?
>
> That comes with using the CMA helper:
> drm_fbdev_cma_create_with_funcs() -> drm_gem_cma_create() -> dma_alloc_wc()
>
> Here's a comment I have added in the code for why I set the DMA mask on
> the SPI device and why it will work:
>
> /*
> * Even though it's not the SPI device that does DMA (the master does),
> * the dma mask is necessary for the dma_alloc_wc() in
> * drm_gem_cma_create(). The dma_addr returned will be a physical
> * adddress which might be different from the bus address, but this is
> * not a problem since the address will not be used.
> * The virtual address is used in the transfer and the SPI core
> * re-maps it on the SPI master device using the DMA streaming API
> * (spi_map_buf()).
> */
>
> > Either way, I think this all looks good, pls submit a pull request to Dave
> > with these two patches as soon as latest drm-misc has landed (I'll send a
> > pull request for that later today).
>
> I'm confused, I thought you wanted the core patches through drm-misc
> and the rest as a pull request to Dave.
>
> This is what you said:
>
> Looks all pretty. A bunch of ideas below, but all optional. For merging
> I
> think simplest to first get the core patches in through drm-misc, and
> then
> you can submit a pull request to Dave for tinydrm+backends (just needs
> an
> ack for the dt parts from dt maintainers), including MAINTAINERS entry.
> Ack from my side.
Ah, I thought we already have all the prep patches you need merged into
drm-misc. Still the same plan.
> > Another one: Do you want to maintain tinydrm as part of the drm-misc
> > group, i.e. want commit rights there? That would also help a bit with
> > pushing all your great drm refactoring patches through the machinery ...
>
> My goal is to port staging/fbtft to drm. Whether or not I will do work
> in drm core (refactoring) besides the tinydrm drivers when that is
> accomplished, I don't know. Working on drm as a hobbyist is very
> difficult (for me at least) because it is very complex, it changes all
> the time and on top of that it has a high volume mailinglist.
> It takes effort to keep up.
>
> So I will probably end up doing only tinydrm maintanence.
> As for how that is best done, I don't know. Once I have covered a 3-4
> controller types, a new driver is just a copy of a similar one with a
> different register initialization. This means that it's easy to review
> patches. They will all look the same, more or less.
> So for me it's ofc best if I can review the patches and then commit
> them myself. As for my own patches, if I need that tit for tat
> reviewing to get them in, it will be difficult for me to review other
> drivers because I have no idea how a GPU operates, and to keep up with
> drm best pratices will be next to impossible for me.
I think you're slightly understating your knowledge about the display side
of drm here a bit :-) We're (for now) also only talking about getting
small display drivers into drm-misc.
> If the Device Tree guys allows me to add fbtft support to tinydrm, then
> there won't be much activity once the fbtft drivers have been ported
> over. The uncertainty stems from the fact that the fbtft drivers are
> written as controller drivers, but they contain panel specific things
Re: [PATCH 2/4] drm/msm: drop qcom,chipid
Rob Clark writes: > The original way we determined the gpu version was based on downstream > bindings from android kernel. A cleaner way is to get the version from > the compatible string. > > Note that no upstream dtb uses these bindings. But the code still > supports falling back to the legacy bindings (with a warning), so that > we are still compatible with the gpu dt node from android device > kernels. The gpulist[] seems pretty short, like you could just have 7 compatible strings in dt_match[] and point them directly at corresponding the gpulist[] entry. Or are there lots of patch levels, with the same struct adreno_info values? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 3/4] drm/msm: drop quirks binding
Rob Clark writes: > This was never documented or used in upstream dtb. It is used by > downstream bindings from android device kernels. But the quirks are > a property of the gpu revision, and as such are redundant to be listed > separately in dt. Instead, move the quirks to the device table. > > Signed-off-by: Rob Clark > --- > drivers/gpu/drm/msm/adreno/a5xx_gpu.c | 4 ++-- > drivers/gpu/drm/msm/adreno/adreno_device.c | 18 -- > drivers/gpu/drm/msm/adreno/adreno_gpu.c| 1 - > drivers/gpu/drm/msm/adreno/adreno_gpu.h| 4 +--- > 4 files changed, 7 insertions(+), 20 deletions(-) Nice cleanup! Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 24/24] drm/rockchip: dw-mipi-dsi: support read commands
On Mon, 30 Jan 2017 10:26:11 -0500, Sean Paul wrote:
> On Sun, Jan 29, 2017 at 01:24:44PM +, John Keeping wrote:
> > I haven't found any method for getting the length of a response, so this
> > just uses the requested rx_len
> >
> > Signed-off-by: John Keeping
> > ---
> > v3:
> > - Fix checkpatch warnings
> > Unchanged in v2
> >
> > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 56
> > ++
> > 1 file changed, 56 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > index cf3ca6b0cbdb..cc58ada75425 100644
> > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > @@ -678,6 +678,56 @@ static int dw_mipi_dsi_dcs_long_write(struct
> > dw_mipi_dsi *dsi,
> > return dw_mipi_dsi_gen_pkt_hdr_write(dsi, hdr_val);
> > }
> >
> > +static int dw_mipi_dsi_dcs_read(struct dw_mipi_dsi *dsi,
> > + const struct mipi_dsi_msg *msg)
> > +{
> > + const u8 *tx_buf = msg->tx_buf;
> > + u8 *rx_buf = msg->rx_buf;
> > + size_t i;
> > + int ret, val;
> > +
> > + dsi_write(dsi, DSI_PCKHDL_CFG, EN_CRC_RX | EN_ECC_RX | EN_BTA);
> > + dsi_write(dsi, DSI_GEN_HDR,
> > + GEN_HDATA(tx_buf[0]) | GEN_HTYPE(msg->type));
> > +
> > + ret = readl_poll_timeout(dsi->base + DSI_CMD_PKT_STATUS,
> > +val, !(val & GEN_RD_CMD_BUSY), 1000,
> > +CMD_PKT_STATUS_TIMEOUT_US);
> > + if (ret < 0) {
> > + dev_err(dsi->dev, "failed to read command response\n");
> > + return ret;
> > + }
> > +
> > + for (i = 0; i < msg->rx_len;) {
> > + u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
> > +
> > + while (i < msg->rx_len) {
> > + rx_buf[i] = pld & 0xff;
> > + pld >>= 8;
> > + i++;
> > + }
> > + }
>
> AFAICT, the outer for loop just initializes i and ensures msg->rx_len is
> non-zero?
>
> I think the following would be easier to read (and safe against the case where
> msg->rx_len > sizeof(pld) (even though this shouldn't happen according to DCS
> spec)).
>
> if (msg->rx_len > 0) {
> u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
> memcpy(rx_buf, &pld, MIN(msg->rx_len, sizeof(pld));
> }
I think the intent was to handle rx_len > 4, but the patch is obvously
completely broken regarding that. As far as I can tell, rx_len is
limited by the maximum return packet size which can be any value up to
the maximum size of a long packet, so we may need to read from the FIFO
multiple times.
The loop should be something like this:
for (i = 0; i < msg->rx_len;) {
u32 pld = dsi_read(dsi, DSI_GEN_PLD_DATA);
int j;
for (j = 0; j < 4 && i < msg->rx_len; i++, j++) {
rx_buf[i] = pld & 0xff;
pld >>= 8;
}
}
I have successfully read 5 bytes from a DSI display using this code, but
I'm tempted to just drop this patch since I only used it for debugging
while bringing up a new panel.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/msm: drop _clk suffix from clk names
Rob Clark writes:
> Suggested by Rob Herring. We still support the old names for
> compatibility with downstream android dt files.
>
> Cc: Rob Herring
> Signed-off-by: Rob Clark
Huh, I don't think I would have cleaned up DT bindings in exchange for
adding driver code like this. But the code seems correct, so other than
one optional suggestion:
Reviewed-by: Eric Anholt
> +struct clk *msm_clk_get(struct platform_device *pdev, const char *name)
> +{
> + struct clk *clk;
> + char name2[32];
> +
> + clk = devm_clk_get(&pdev->dev, name);
> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
> + return clk;
> +
> + snprintf(name2, sizeof(name2), "%s_clk", name);
> +
> + clk = devm_clk_get(&pdev->dev, name2);
> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
> + dev_warn(&pdev->dev, "Using legacy clk name binding. Use "
> + "\"%s\" instead of \"%s\"\n", name, name2);
Drop the second "|| PTR_ERR(clk)" case, so that you only get one warning
printed at boot if deferring happens?
signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 06/24] drm/rockchip: dw-mipi-dsi: avoid out-of-bounds read on tx_buf
On Mon, 30 Jan 2017 13:01:46 -0500, Sean Paul wrote:
> On Sun, Jan 29, 2017 at 01:24:26PM +, John Keeping wrote:
> > As a side-effect of this, encode the endianness explicitly rather than
> > casting a u16.
> >
> > Signed-off-by: John Keeping
> > Reviewed-by: Chris Zhong
> > ---
> > v3:
> > - Add Chris' Reviewed-by
> > Unchanged in v2
> >
> > drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 9 +++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > index 4be1ff3a42bb..2e6ad4591ebf 100644
> > --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> > @@ -572,8 +572,13 @@ static int dw_mipi_dsi_gen_pkt_hdr_write(struct
> > dw_mipi_dsi *dsi, u32 hdr_val)
> > static int dw_mipi_dsi_dcs_short_write(struct dw_mipi_dsi *dsi,
> >const struct mipi_dsi_msg *msg)
> > {
> > - const u16 *tx_buf = msg->tx_buf;
> > - u32 val = GEN_HDATA(*tx_buf) | GEN_HTYPE(msg->type);
> > + const u8 *tx_buf = msg->tx_buf;
> > + u32 val = GEN_HTYPE(msg->type);
> > +
> > + if (msg->tx_len > 0)
> > + val |= GEN_HDATA(tx_buf[0]);
> > + if (msg->tx_len > 1)
> > + val |= GEN_HDATA(tx_buf[1] << 8);
>
> You should probably update the mask inside GEN_HDATA to mask off 8 bits
> instead of
> 16.
Won't that mask off the data written by "tx_buf[1] << 8"?
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/msm: drop qcom,chipid
On Mon, Jan 30, 2017 at 1:09 PM, Eric Anholt wrote: > Rob Clark writes: > >> The original way we determined the gpu version was based on downstream >> bindings from android kernel. A cleaner way is to get the version from >> the compatible string. >> >> Note that no upstream dtb uses these bindings. But the code still >> supports falling back to the legacy bindings (with a warning), so that >> we are still compatible with the gpu dt node from android device >> kernels. > > The gpulist[] seems pretty short, like you could just have 7 compatible > strings in dt_match[] and point them directly at corresponding the > gpulist[] entry. Or are there lots of patch levels, with the same > struct adreno_info values? The list currently is rather incomplete (which may or may not matter, I guess it comes down to how many different phones/tablets out there people try to get an upstream kernel working on). And it has those ANY_ID wildcard entries. A full list of all the gpu's including all their patchlevels would be quite long. We might end up wanting to split the quirks out differently, since those tend to apply to specific patchlevel's.. I had to change the existing entry for a530 from a530.* to a530.2 for this reason. But that won't effect the bindings so that is an implementation detail we can worry about later. BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v3 08/24] drm/rockchip: dw-mipi-dsi: respect message flags
On Sun, Jan 29, 2017 at 01:24:28PM +, John Keeping wrote:
> Instead of always sending commands in LP mode, respect the
> MIPI_DSI_MSG_USE_LPM flag to decide how to send each message. Also
> request acks if MIPI_DSI_MSG_REQ_ACK is set.
>
Reviewed-by: Sean Paul
> Signed-off-by: John Keeping
> Reviewed-by: Chris Zhong
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
>
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 16 +++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 92dbc3e56603..15d33c3c8cb7 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -542,6 +542,19 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host
> *host,
> return 0;
> }
>
> +static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
> +const struct mipi_dsi_msg *msg)
> +{
> + u32 val = 0;
> +
> + if (msg->flags & MIPI_DSI_MSG_REQ_ACK)
> + val |= EN_ACK_RQST;
> + if (msg->flags & MIPI_DSI_MSG_USE_LPM)
> + val |= CMD_MODE_ALL_LP;
> +
> + dsi_write(dsi, DSI_CMD_MODE_CFG, val);
> +}
> +
> static int dw_mipi_dsi_gen_pkt_hdr_write(struct dw_mipi_dsi *dsi, u32
> hdr_val)
> {
> int ret;
> @@ -634,6 +647,8 @@ static ssize_t dw_mipi_dsi_host_transfer(struct
> mipi_dsi_host *host,
> struct dw_mipi_dsi *dsi = host_to_dsi(host);
> int ret;
>
> + dw_mipi_message_config(dsi, msg);
> +
> switch (msg->type) {
> case MIPI_DSI_DCS_SHORT_WRITE:
> case MIPI_DSI_DCS_SHORT_WRITE_PARAM:
> @@ -745,7 +760,6 @@ static void dw_mipi_dsi_command_mode_config(struct
> dw_mipi_dsi *dsi)
> {
> dsi_write(dsi, DSI_TO_CNT_CFG, HSTX_TO_CNT(1000) | LPRX_TO_CNT(1000));
> dsi_write(dsi, DSI_BTA_TO_CNT, 0xd00);
> - dsi_write(dsi, DSI_CMD_MODE_CFG, CMD_MODE_ALL_LP);
> dsi_write(dsi, DSI_MODE_CFG, ENABLE_CMD_MODE);
> }
>
> --
> 2.11.0.197.gb556de5.dirty
>
> ___
> 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 v3 09/24] drm/rockchip: dw-mipi-dsi: only request HS clock when required
On Sun, Jan 29, 2017 at 01:24:29PM +, John Keeping wrote:
> Requesting the HS clock from the PHY before we initialize it causes an
> invalid signal to be sent out since the input clock is not yet
> configured. The PHY databook suggests only asserting this signal when
> performing HS transfers, so let's do that.
>
Reviewed-by: Sean Paul
> Signed-off-by: John Keeping
> Reviewed-by: Chris Zhong
> ---
> v3:
> - Add Chris' Reviewed-by
> Unchanged in v2
>
> drivers/gpu/drm/rockchip/dw-mipi-dsi.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> index 15d33c3c8cb7..03fc096fe1bd 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi.c
> @@ -545,13 +545,15 @@ static int dw_mipi_dsi_host_detach(struct mipi_dsi_host
> *host,
> static void dw_mipi_message_config(struct dw_mipi_dsi *dsi,
> const struct mipi_dsi_msg *msg)
> {
> + bool lpm = msg->flags & MIPI_DSI_MSG_USE_LPM;
> u32 val = 0;
>
> if (msg->flags & MIPI_DSI_MSG_REQ_ACK)
> val |= EN_ACK_RQST;
> - if (msg->flags & MIPI_DSI_MSG_USE_LPM)
> + if (lpm)
> val |= CMD_MODE_ALL_LP;
>
> + dsi_write(dsi, DSI_LPCLK_CTRL, lpm ? 0 : PHY_TXREQUESTCLKHS);
> dsi_write(dsi, DSI_CMD_MODE_CFG, val);
> }
>
> @@ -693,6 +695,7 @@ static void dw_mipi_dsi_set_mode(struct dw_mipi_dsi *dsi,
> dsi_write(dsi, DSI_PWR_UP, RESET);
> dsi_write(dsi, DSI_MODE_CFG, ENABLE_VIDEO_MODE);
> dw_mipi_dsi_video_mode_config(dsi);
> + dsi_write(dsi, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
> dsi_write(dsi, DSI_PWR_UP, POWERUP);
> }
> }
> @@ -710,7 +713,6 @@ static void dw_mipi_dsi_init(struct dw_mipi_dsi *dsi)
> | PHY_RSTZ | PHY_SHUTDOWNZ);
> dsi_write(dsi, DSI_CLKMGR_CFG, TO_CLK_DIVIDSION(10) |
> TX_ESC_CLK_DIVIDSION(7));
> - dsi_write(dsi, DSI_LPCLK_CTRL, PHY_TXREQUESTCLKHS);
> }
>
> static void dw_mipi_dsi_dpi_config(struct dw_mipi_dsi *dsi,
> --
> 2.11.0.197.gb556de5.dirty
>
> ___
> 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 1/4] drm/msm: remove qcom,gpu-pwrlevels bindings
Rob Clark writes: > The plan is to use the OPP bindings. For now, remove the documentation > for qcom,gpu-pwrlevels, and make the driver fall back to a safe low > clock if the node is not present. > > Note that no upstream dtb use this node. For now we keep compatibility > with this node to avoid breaking compatibility with downstream android > dt files. > > Signed-off-by: Rob Clark Will we need the bus frequency knobs that I see in the old pwrlevels node? If so, what would the plan be for doing that within OPP? signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 4/4] drm/msm: drop _clk suffix from clk names
On Mon, Jan 30, 2017 at 1:15 PM, Eric Anholt wrote:
> Rob Clark writes:
>
>> Suggested by Rob Herring. We still support the old names for
>> compatibility with downstream android dt files.
>>
>> Cc: Rob Herring
>> Signed-off-by: Rob Clark
>
> Huh, I don't think I would have cleaned up DT bindings in exchange for
> adding driver code like this. But the code seems correct, so other than
> one optional suggestion:
>
> Reviewed-by: Eric Anholt
>
>> +struct clk *msm_clk_get(struct platform_device *pdev, const char *name)
>> +{
>> + struct clk *clk;
>> + char name2[32];
>> +
>> + clk = devm_clk_get(&pdev->dev, name);
>> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
>> + return clk;
>> +
>> + snprintf(name2, sizeof(name2), "%s_clk", name);
>> +
>> + clk = devm_clk_get(&pdev->dev, name2);
>> + if (!IS_ERR(clk) || PTR_ERR(clk) == -EPROBE_DEFER)
>> + dev_warn(&pdev->dev, "Using legacy clk name binding. Use "
>> + "\"%s\" instead of \"%s\"\n", name, name2);
>
> Drop the second "|| PTR_ERR(clk)" case, so that you only get one warning
> printed at boot if deferring happens?
oh, good point.. I've fixed that up locally. I don't think we'd
actually hit this case currently for gpu clks, since for gpu this
codepath happens on first open() of the device file. But I should
mention I have a slighly sneaky ulterior motive, which is that we
could use the same cleanup for display related clks (which do
currently upstream use the _clk suffix).
BR,
-R
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 1/4] drm/msm: remove qcom,gpu-pwrlevels bindings
On Mon, Jan 30, 2017 at 1:21 PM, Eric Anholt wrote: > Rob Clark writes: > >> The plan is to use the OPP bindings. For now, remove the documentation >> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low >> clock if the node is not present. >> >> Note that no upstream dtb use this node. For now we keep compatibility >> with this node to avoid breaking compatibility with downstream android >> dt files. >> >> Signed-off-by: Rob Clark > > Will we need the bus frequency knobs that I see in the old pwrlevels > node? If so, what would the plan be for doing that within OPP? So, that I think is one of the open questions. Jordan knows this stuff a lot better than I, but my understanding is that bus and clk scale *basically* independently, except that a given gpu clk we want a different minimum bus clk. (I'm not sure if that is a functional requirement, or just what qcom arrived at after performance tuning..) There is some work ongoing to get some sort of upstream bus scaling scaling, although I'm not really sure yet what the bindings for this would look like. So basically short answer is "I don't know.. there are too many open questions". Maybe in the end we re-introduce qcom,gpu-pwrlevels. But I think for now the approach of not documenting it and have safe/slow clk fallback in the driver is a reasonable way to move forward with getting some basic gpu nodes into upstream dts files. BR, -R ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 0/2] drm: Add support for tiny LCD displays
Den 30.01.2017 19.06, skrev Daniel Vetter: On Mon, Jan 30, 2017 at 04:03:53PM +0100, Noralf Trønnes wrote: Den 30.01.2017 09.44, skrev Daniel Vetter: Hi Noralf, On Fri, Jan 27, 2017 at 08:56:29PM +0100, Noralf Trønnes wrote: This is an attempt at providing a DRM version of drivers/staging/fbtft. The tinydrm library provides a very simplified view of DRM in particular for tiny displays that has onboard video memory and is connected through a slow bus like SPI/I2C. Only core patches this time. Noralf. Changes since version 1: - Add tinydrm.rst - Set tdev->fbdev_cma=NULL on unregister (lastclose is called after that). Either way, I think this all looks good, pls submit a pull request to Dave with these two patches as soon as latest drm-misc has landed (I'll send a pull request for that later today). I'm confused, I thought you wanted the core patches through drm-misc and the rest as a pull request to Dave. This is what you said: Looks all pretty. A bunch of ideas below, but all optional. For merging I think simplest to first get the core patches in through drm-misc, and then you can submit a pull request to Dave for tinydrm+backends (just needs an ack for the dt parts from dt maintainers), including MAINTAINERS entry. Ack from my side. Ah, I thought we already have all the prep patches you need merged into drm-misc. Still the same plan. Oh, sorry, I mixed up core drm with core tinydrm. Yes, the core drm patches are in. I'll send a pull request for tinydrm when I get an ack from the DT maintainer. It all makes sense now :-) Noralf. ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH 2/4] drm/msm: drop qcom,chipid
Rob Clark writes: > On Mon, Jan 30, 2017 at 1:09 PM, Eric Anholt wrote: >> Rob Clark writes: >> >>> The original way we determined the gpu version was based on downstream >>> bindings from android kernel. A cleaner way is to get the version from >>> the compatible string. >>> >>> Note that no upstream dtb uses these bindings. But the code still >>> supports falling back to the legacy bindings (with a warning), so that >>> we are still compatible with the gpu dt node from android device >>> kernels. >> >> The gpulist[] seems pretty short, like you could just have 7 compatible >> strings in dt_match[] and point them directly at corresponding the >> gpulist[] entry. Or are there lots of patch levels, with the same >> struct adreno_info values? > > The list currently is rather incomplete (which may or may not matter, > I guess it comes down to how many different phones/tablets out there > people try to get an upstream kernel working on). And it has those > ANY_ID wildcard entries. > > A full list of all the gpu's including all their patchlevels would be > quite long. > > We might end up wanting to split the quirks out differently, since > those tend to apply to specific patchlevel's.. I had to change the > existing entry for a530 from a530.* to a530.2 for this reason. But > that won't effect the bindings so that is an implementation detail we > can worry about later. I think that using the of_match_device() mechanism from the specific strings listed as the compatible would make more sense than this string parsing. You have to write a gpulist[] entry anyway for the module to load. So that means adding about 7 values today to the compatible string dt_match, and the code to use of_match_device() to get your struct adreno_info. Then the current version number lookup loop would be just for the old DT compatibility and there's no string parsing. However, it's your driver. The code seems correct, and using specific compatible strings is an obviously good step. Reviewed-by: Eric Anholt 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/4] drm/msm: remove qcom,gpu-pwrlevels bindings
Rob Clark writes: > On Mon, Jan 30, 2017 at 1:21 PM, Eric Anholt wrote: >> Rob Clark writes: >> >>> The plan is to use the OPP bindings. For now, remove the documentation >>> for qcom,gpu-pwrlevels, and make the driver fall back to a safe low >>> clock if the node is not present. >>> >>> Note that no upstream dtb use this node. For now we keep compatibility >>> with this node to avoid breaking compatibility with downstream android >>> dt files. >>> >>> Signed-off-by: Rob Clark >> >> Will we need the bus frequency knobs that I see in the old pwrlevels >> node? If so, what would the plan be for doing that within OPP? > > So, that I think is one of the open questions. Jordan knows this > stuff a lot better than I, but my understanding is that bus and clk > scale *basically* independently, except that a given gpu clk we want a > different minimum bus clk. > > (I'm not sure if that is a functional requirement, or just what qcom > arrived at after performance tuning..) > > There is some work ongoing to get some sort of upstream bus scaling > scaling, although I'm not really sure yet what the bindings for this > would look like. > > So basically short answer is "I don't know.. there are too many open > questions". Maybe in the end we re-introduce qcom,gpu-pwrlevels. But > I think for now the approach of not documenting it and have safe/slow > clk fallback in the driver is a reasonable way to move forward with > getting some basic gpu nodes into upstream dts files. Yeah, letting the upstream DT bind without the custom OPP stuff for now seems like progress. If we find that the safe fast freq is too high then we can drop it down later, and that would just put more pressure on getting the OPP work finished. Reviewed-by: Eric Anholt signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] MAINTAINERS: add dma-fence* files to Sync File maintainership
2017-01-30 Sumit Semwal : > Hi Gustavo, Daniel, > > On 30 January 2017 at 14:07, Daniel Vetter wrote: > > > On Fri, Jan 27, 2017 at 03:54:44PM -0200, Gustavo Padovan wrote: > > > From: Gustavo Padovan > > > > > > As Sync File is highly dependent on dma-fence* tracks it > > > under SYNC FILE_FRAMEWORK as well. > > > > > > Signed-off-by: Gustavo Padovan > > > > Acked-by: Daniel Vetter > > > > Of course, feel free to add my > Acked-by: Sumit Semwal applied to drm-misc-next. Thanks. Gustavo ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
