On Saturday 20 June 2015 04:20 AM, Matt Roper wrote:
On Mon, Jun 08, 2015 at 05:58:23PM -0700, Matt Roper wrote:
On Sat, Jun 06, 2015 at 05:34:45PM +0530, Sharma, Shashank wrote:
Regards
Shashank
On 6/6/2015 6:31 AM, Matt Roper wrote:
On Thu, Jun 04, 2015 at 07:12:37PM +0530, Kausal Malladi wrote:
From: Kausal Malladi <Kausal.Malladi@xxxxxxxxx>
The atomic CRTC set infrastructure is not available yet. So, the atomic
check path throws error for any non-plane property.
This patch adds a diversion to avoid commit path for DRM atomic set Gamma
property, until atomic CRTC infrastructure is ready.
This doesn't look right, but I don't quite understand what you're trying
to do from the commit message.
This function is what will implement legacy set_property ioctl's of a
CRTC on an atomic-based driver. The idea is that when the ioctl is
issued, we should get (i.e., make a duplicate of) the current CRTC
state, change some of the values in that state (which is what the
.atomic_set_property function takes care of), and then submit that
modified state to the driver for checking and hw-programming.
Okay, I just took a quick peek ahead in your patch set and I think I
understand the confusion now...it looks like you're trying to actually
perform the full hardware update in the .atomic_set_property call chain,
which isn't what we want to be doing in an atomic driver.
.atomic_set_property() is just a matter of updating the state structures
to reflect the changes you *want* to make (but not actually performing
those changes right away). It isn't until drm_atomic_commit() gets
called that we validate the new state structure and then write it to the
hardware if it looks okay.
Keep in mind that the overall goal behind atomic is that we want to be
able to supply several items to be updated (e.g., mode, CSC, gamma,
etc.) via the atomic ioctl and then have them all accepted (and
programmed) by the driver, or all rejected (so none get programmed) if
any of them are invalid. Also, if the collection of properties that
you're updating fall within the "nuclear pageflip" subset of
functionality, you'll also get a guarantee that those items all get
updated within the same vblank; updates that straddle a vblank could
cause unwanted flickering or other artifacts. The helper function
you're updating here only happens to update a single state value at a
time, but the same .atomic_set_property() is also used by the atomic
ioctl, so we need to make sure it's following the expected design.
Finally, keep in mind that the function you're updating here is a DRM
core function. Even though i915 isn't quite ready for full atomic yet
and might have a bit of brain damage in areas, other vendors' drivers do
have complete atomic modesetting support (I think?), so adding
i915-specific workarounds like this to the core function could actually
hamper them.
Matt
I understood this point. But in this case, we will be blocked until
set CRTC implementation comes in. Can you suggest a better way to
handle this without getting blocked for set_crtc() ?
Is the functionality you need still lacking with Maarten Lankhorst's
"convert to atomic, part 2" series that recently landed upstream? If
that series isn't sufficient, them I'm pretty sure that his "part 3"
series on the mailing list will fix it; that will hopefully be ready to
merge as soon as I get a chance to do a review of it.
I'll add Maarten to the Cc here in case he wants to comment...
Matt
Kausal/Shashank, have you guys had a chance to check out Maarten's
latest i915 atomic work? His "part 2" series is already merged and
"part 3" should be merged soon and those really help clean up a lot of
the Intel-specific pain points caused by the atomic transition. I think
we need to make sure you can handle properties properly in an atomic
manner, so if you find you're still blocked on the current code, we
should look into how to address that so that we don't have to bring any
temporary Intel hackiness into the shared DRM core code.
Matt
Thanks Matt, for helping with a fix for this issue. As suggested by you,
we will push this fix along with the other set of patches for color
management.
Kausal
Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
Signed-off-by: Kausal Malladi <Kausal.Malladi@xxxxxxxxx>
---
drivers/gpu/drm/drm_atomic_helper.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index a900858..37dba55 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1696,6 +1696,8 @@ drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc,
{
struct drm_atomic_state *state;
struct drm_crtc_state *crtc_state;
+ struct drm_device *dev = crtc->dev;
+ struct drm_mode_config *config = &dev->mode_config;
int ret = 0;
state = drm_atomic_state_alloc(crtc->dev);
@@ -1716,9 +1718,18 @@ retry:
if (ret)
goto fail;
- ret = drm_atomic_commit(state);
- if (ret != 0)
- goto fail;
+ /**
+ * FIXME : This is a hack, to avoid atomic commit
+ * for CRTC, because i915 supports only
+ * atomic plane operations at the moment
+ */
+ if (property == config->gamma_property)
+ ret = 0;
+ else {
+ ret = drm_atomic_commit(state);
+ if (ret != 0)
+ goto fail;
+ }
/* Driver takes ownership of state on successful commit. */
return 0;
--
2.4.2
--
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
