lkml.org 
[lkml]   [2018]   [Mar]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 7/9] drm/xen-front: Implement KMS/connector handling
From
Date
On 03/05/2018 11:23 AM, Daniel Vetter wrote:
> On Wed, Feb 21, 2018 at 10:03:40AM +0200, Oleksandr Andrushchenko wrote:
>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>>
>> Implement kernel modesetiing/connector handling using
>> DRM simple KMS helper pipeline:
>>
>> - implement KMS part of the driver with the help of DRM
>> simple pipepline helper which is possible due to the fact
>> that the para-virtualized driver only supports a single
>> (primary) plane:
>> - initialize connectors according to XenStore configuration
>> - handle frame done events from the backend
>> - generate vblank events
>> - create and destroy frame buffers and propagate those
>> to the backend
>> - propagate set/reset mode configuration to the backend on display
>> enable/disable callbacks
>> - send page flip request to the backend and implement logic for
>> reporting backend IO errors on prepare fb callback
>>
>> - implement virtual connector handling:
>> - support only pixel formats suitable for single plane modes
>> - make sure the connector is always connected
>> - support a single video mode as per para-virtualized driver
>> configuration
>>
>> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> I think once you've removed the midlayer in the previous patch it would
> makes sense to merge the 2 patches into 1.
ok, will squash the two
>
> Bunch more comments below.
> -Daniel
>
>> ---
>> drivers/gpu/drm/xen/Makefile | 2 +
>> drivers/gpu/drm/xen/xen_drm_front_conn.c | 125 +++++++++++++
>> drivers/gpu/drm/xen/xen_drm_front_conn.h | 35 ++++
>> drivers/gpu/drm/xen/xen_drm_front_drv.c | 15 ++
>> drivers/gpu/drm/xen/xen_drm_front_drv.h | 12 ++
>> drivers/gpu/drm/xen/xen_drm_front_kms.c | 299 +++++++++++++++++++++++++++++++
>> drivers/gpu/drm/xen/xen_drm_front_kms.h | 30 ++++
>> 7 files changed, 518 insertions(+)
>> create mode 100644 drivers/gpu/drm/xen/xen_drm_front_conn.c
>> create mode 100644 drivers/gpu/drm/xen/xen_drm_front_conn.h
>> create mode 100644 drivers/gpu/drm/xen/xen_drm_front_kms.c
>> create mode 100644 drivers/gpu/drm/xen/xen_drm_front_kms.h
>>
>> diff --git a/drivers/gpu/drm/xen/Makefile b/drivers/gpu/drm/xen/Makefile
>> index d3068202590f..4fcb0da1a9c5 100644
>> --- a/drivers/gpu/drm/xen/Makefile
>> +++ b/drivers/gpu/drm/xen/Makefile
>> @@ -2,6 +2,8 @@
>>
>> drm_xen_front-objs := xen_drm_front.o \
>> xen_drm_front_drv.o \
>> + xen_drm_front_kms.o \
>> + xen_drm_front_conn.o \
>> xen_drm_front_evtchnl.o \
>> xen_drm_front_shbuf.o \
>> xen_drm_front_cfg.o
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_conn.c b/drivers/gpu/drm/xen/xen_drm_front_conn.c
>> new file mode 100644
>> index 000000000000..d9986a2e1a3b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_conn.c
>> @@ -0,0 +1,125 @@
>> +/*
>> + * Xen para-virtual DRM device
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * Copyright (C) 2016-2018 EPAM Systems Inc.
>> + *
>> + * Author: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> + */
>> +
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_crtc_helper.h>
>> +
>> +#include <video/videomode.h>
>> +
>> +#include "xen_drm_front_conn.h"
>> +#include "xen_drm_front_drv.h"
>> +
>> +static struct xen_drm_front_drm_pipeline *
>> +to_xen_drm_pipeline(struct drm_connector *connector)
>> +{
>> + return container_of(connector, struct xen_drm_front_drm_pipeline, conn);
>> +}
>> +
>> +static const uint32_t plane_formats[] = {
>> + DRM_FORMAT_RGB565,
>> + DRM_FORMAT_RGB888,
>> + DRM_FORMAT_XRGB8888,
>> + DRM_FORMAT_ARGB8888,
>> + DRM_FORMAT_XRGB4444,
>> + DRM_FORMAT_ARGB4444,
>> + DRM_FORMAT_XRGB1555,
>> + DRM_FORMAT_ARGB1555,
>> +};
>> +
>> +const uint32_t *xen_drm_front_conn_get_formats(int *format_count)
>> +{
>> + *format_count = ARRAY_SIZE(plane_formats);
>> + return plane_formats;
>> +}
>> +
>> +static enum drm_connector_status connector_detect(
>> + struct drm_connector *connector, bool force)
>> +{
>> + if (drm_dev_is_unplugged(connector->dev))
>> + return connector_status_disconnected;
>> +
>> + return connector_status_connected;
>> +}
>> +
>> +#define XEN_DRM_NUM_VIDEO_MODES 1
>> +#define XEN_DRM_CRTC_VREFRESH_HZ 60
>> +
>> +static int connector_get_modes(struct drm_connector *connector)
>> +{
>> + struct xen_drm_front_drm_pipeline *pipeline =
>> + to_xen_drm_pipeline(connector);
>> + struct drm_display_mode *mode;
>> + struct videomode videomode;
>> + int width, height;
>> +
>> + mode = drm_mode_create(connector->dev);
>> + if (!mode)
>> + return 0;
>> +
>> + memset(&videomode, 0, sizeof(videomode));
>> + videomode.hactive = pipeline->width;
>> + videomode.vactive = pipeline->height;
>> + width = videomode.hactive + videomode.hfront_porch +
>> + videomode.hback_porch + videomode.hsync_len;
>> + height = videomode.vactive + videomode.vfront_porch +
>> + videomode.vback_porch + videomode.vsync_len;
>> + videomode.pixelclock = width * height * XEN_DRM_CRTC_VREFRESH_HZ;
>> + mode->type = DRM_MODE_TYPE_PREFERRED | DRM_MODE_TYPE_DRIVER;
>> +
>> + drm_display_mode_from_videomode(&videomode, mode);
>> + drm_mode_probed_add(connector, mode);
>> + return XEN_DRM_NUM_VIDEO_MODES;
>> +}
>> +
>> +static int connector_mode_valid(struct drm_connector *connector,
>> + struct drm_display_mode *mode)
>> +{
>> + struct xen_drm_front_drm_pipeline *pipeline =
>> + to_xen_drm_pipeline(connector);
>> +
>> + if (mode->hdisplay != pipeline->width)
>> + return MODE_ERROR;
>> +
>> + if (mode->vdisplay != pipeline->height)
>> + return MODE_ERROR;
>> +
>> + return MODE_OK;
>> +}
>> +
>> +static const struct drm_connector_helper_funcs connector_helper_funcs = {
>> + .get_modes = connector_get_modes,
>> + .mode_valid = connector_mode_valid,
>> +};
>> +
>> +static const struct drm_connector_funcs connector_funcs = {
>> + .detect = connector_detect,
>> + .fill_modes = drm_helper_probe_single_connector_modes,
>> + .destroy = drm_connector_cleanup,
>> + .reset = drm_atomic_helper_connector_reset,
>> + .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>> + .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> +};
>> +
>> +int xen_drm_front_conn_init(struct xen_drm_front_drm_info *drm_info,
>> + struct drm_connector *connector)
>> +{
>> + drm_connector_helper_add(connector, &connector_helper_funcs);
>> +
>> + return drm_connector_init(drm_info->drm_dev, connector,
>> + &connector_funcs, DRM_MODE_CONNECTOR_VIRTUAL);
>> +}
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_conn.h b/drivers/gpu/drm/xen/xen_drm_front_conn.h
>> new file mode 100644
>> index 000000000000..708e80d45985
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_conn.h
>> @@ -0,0 +1,35 @@
>> +/*
>> + * Xen para-virtual DRM device
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * Copyright (C) 2016-2018 EPAM Systems Inc.
>> + *
>> + * Author: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> + */
>> +
>> +#ifndef __XEN_DRM_FRONT_CONN_H_
>> +#define __XEN_DRM_FRONT_CONN_H_
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_encoder.h>
>> +
>> +#include <linux/wait.h>
>> +
>> +struct xen_drm_front_drm_info;
>> +
>> +const uint32_t *xen_drm_front_conn_get_formats(int *format_count);
>> +
>> +int xen_drm_front_conn_init(struct xen_drm_front_drm_info *drm_info,
>> + struct drm_connector *connector);
>> +
>> +#endif /* __XEN_DRM_FRONT_CONN_H_ */
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_drv.c b/drivers/gpu/drm/xen/xen_drm_front_drv.c
>> index b3764d5ed0f6..e8862d26ba27 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front_drv.c
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_drv.c
>> @@ -23,6 +23,7 @@
>> #include "xen_drm_front.h"
>> #include "xen_drm_front_cfg.h"
>> #include "xen_drm_front_drv.h"
>> +#include "xen_drm_front_kms.h"
>>
>> static int dumb_create(struct drm_file *filp,
>> struct drm_device *dev, struct drm_mode_create_dumb *args)
>> @@ -41,6 +42,13 @@ static void free_object(struct drm_gem_object *obj)
>> static void on_frame_done(struct platform_device *pdev,
>> int conn_idx, uint64_t fb_cookie)
>> {
>> + struct xen_drm_front_drm_info *drm_info = platform_get_drvdata(pdev);
>> +
>> + if (unlikely(conn_idx >= drm_info->cfg->num_connectors))
>> + return;
>> +
>> + xen_drm_front_kms_on_frame_done(&drm_info->pipeline[conn_idx],
>> + fb_cookie);
>> }
>>
>> static void lastclose(struct drm_device *dev)
>> @@ -157,6 +165,12 @@ int xen_drm_front_drv_probe(struct platform_device *pdev,
>> return ret;
>> }
>>
>> + ret = xen_drm_front_kms_init(drm_info);
>> + if (ret) {
>> + DRM_ERROR("Failed to initialize DRM/KMS, ret %d\n", ret);
>> + goto fail_modeset;
>> + }
>> +
>> dev->irq_enabled = 1;
>>
>> ret = drm_dev_register(dev, 0);
>> @@ -172,6 +186,7 @@ int xen_drm_front_drv_probe(struct platform_device *pdev,
>>
>> fail_register:
>> drm_dev_unregister(dev);
>> +fail_modeset:
>> drm_mode_config_cleanup(dev);
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_drv.h b/drivers/gpu/drm/xen/xen_drm_front_drv.h
>> index aaa476535c13..563318b19f34 100644
>> --- a/drivers/gpu/drm/xen/xen_drm_front_drv.h
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_drv.h
>> @@ -20,14 +20,24 @@
>> #define __XEN_DRM_FRONT_DRV_H_
>>
>> #include <drm/drmP.h>
>> +#include <drm/drm_simple_kms_helper.h>
>>
>> #include "xen_drm_front.h"
>> #include "xen_drm_front_cfg.h"
>> +#include "xen_drm_front_conn.h"
>>
>> struct xen_drm_front_drm_pipeline {
>> struct xen_drm_front_drm_info *drm_info;
>>
>> int index;
>> +
>> + struct drm_simple_display_pipe pipe;
>> +
>> + struct drm_connector conn;
>> + /* these are only for connector mode checking */
>> + int width, height;
>> + /* last backend error seen on page flip */
>> + int pgflip_last_error;
>> };
>>
>> struct xen_drm_front_drm_info {
>> @@ -35,6 +45,8 @@ struct xen_drm_front_drm_info {
>> struct xen_drm_front_ops *front_ops;
>> struct drm_device *drm_dev;
>> struct xen_drm_front_cfg *cfg;
>> +
>> + struct xen_drm_front_drm_pipeline pipeline[XEN_DRM_FRONT_MAX_CRTCS];
>> };
>>
>> static inline uint64_t xen_drm_front_fb_to_cookie(
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.c b/drivers/gpu/drm/xen/xen_drm_front_kms.c
>> new file mode 100644
>> index 000000000000..ad94c28835cd
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.c
>> @@ -0,0 +1,299 @@
>> +/*
>> + * Xen para-virtual DRM device
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * Copyright (C) 2016-2018 EPAM Systems Inc.
>> + *
>> + * Author: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> + */
>> +
>> +#include "xen_drm_front_kms.h"
>> +
>> +#include <drm/drmP.h>
>> +#include <drm/drm_atomic.h>
>> +#include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_gem.h>
>> +#include <drm/drm_gem_framebuffer_helper.h>
>> +
>> +#include "xen_drm_front.h"
>> +#include "xen_drm_front_conn.h"
>> +#include "xen_drm_front_drv.h"
>> +
>> +static struct xen_drm_front_drm_pipeline *
>> +to_xen_drm_pipeline(struct drm_simple_display_pipe *pipe)
>> +{
>> + return container_of(pipe, struct xen_drm_front_drm_pipeline, pipe);
>> +}
>> +
>> +static void fb_destroy(struct drm_framebuffer *fb)
>> +{
>> + struct xen_drm_front_drm_info *drm_info = fb->dev->dev_private;
>> +
>> + drm_info->front_ops->fb_detach(drm_info->front_info,
>> + xen_drm_front_fb_to_cookie(fb));
>> + drm_gem_fb_destroy(fb);
>> +}
>> +
>> +static struct drm_framebuffer_funcs fb_funcs = {
>> + .destroy = fb_destroy,
>> +};
>> +
>> +static struct drm_framebuffer *fb_create(struct drm_device *dev,
>> + struct drm_file *filp, const struct drm_mode_fb_cmd2 *mode_cmd)
>> +{
>> + struct xen_drm_front_drm_info *drm_info = dev->dev_private;
>> + static struct drm_framebuffer *fb;
>> + struct drm_gem_object *gem_obj;
>> + int ret;
>> +
>> + fb = drm_gem_fb_create_with_funcs(dev, filp, mode_cmd, &fb_funcs);
>> + if (IS_ERR_OR_NULL(fb))
>> + return fb;
>> +
>> + gem_obj = drm_gem_object_lookup(filp, mode_cmd->handles[0]);
>> + if (!gem_obj) {
>> + DRM_ERROR("Failed to lookup GEM object\n");
>> + ret = -ENOENT;
>> + goto fail;
>> + }
>> +
>> + drm_gem_object_unreference_unlocked(gem_obj);
>> +
>> + ret = drm_info->front_ops->fb_attach(
>> + drm_info->front_info,
>> + xen_drm_front_dbuf_to_cookie(gem_obj),
>> + xen_drm_front_fb_to_cookie(fb),
>> + fb->width, fb->height, fb->format->format);
>> + if (ret < 0) {
>> + DRM_ERROR("Back failed to attach FB %p: %d\n", fb, ret);
>> + goto fail;
>> + }
>> +
>> + return fb;
>> +
>> +fail:
>> + drm_gem_fb_destroy(fb);
>> + return ERR_PTR(ret);
>> +}
>> +
>> +static const struct drm_mode_config_funcs mode_config_funcs = {
>> + .fb_create = fb_create,
>> + .atomic_check = drm_atomic_helper_check,
>> + .atomic_commit = drm_atomic_helper_commit,
>> +};
>> +
>> +static int display_set_config(struct drm_simple_display_pipe *pipe,
>> + struct drm_framebuffer *fb)
>> +{
>> + struct xen_drm_front_drm_pipeline *pipeline =
>> + to_xen_drm_pipeline(pipe);
>> + struct drm_crtc *crtc = &pipe->crtc;
>> + struct xen_drm_front_drm_info *drm_info = pipeline->drm_info;
>> + int ret;
>> +
>> + if (fb)
>> + ret = drm_info->front_ops->mode_set(pipeline,
>> + crtc->x, crtc->y,
>> + fb->width, fb->height, fb->format->cpp[0] * 8,
>> + xen_drm_front_fb_to_cookie(fb));
>> + else
>> + ret = drm_info->front_ops->mode_set(pipeline,
>> + 0, 0, 0, 0, 0,
>> + xen_drm_front_fb_to_cookie(NULL));
> This is a bit much layering, the if (fb) case corresponds to the
> display_enable/disable hooks, pls fold that in instead of the indirection.
> simple helpers guarantee that when the display is on, then you have an fb.
1. Ok, the only reason for having this function was to keep
front_ops->mode_set calls at one place (will be refactored
to be a direct call, not via front_ops).
2. The if (fb) check was meant not to check if simple helpers
may give us some wrong value when we do not expect: there is
nothing wrong with them. The check was for 2 cases when this
function was called: with fb != NULL on display enable and
with fb == NULL on display disable, e.g. fb was used as a
flag in this check.
3. I will remove this function at all and will make direct calls
to the backend on .display_{enable|disable}
>
> Maybe we need to fix the docs, pls check and if that's not clear, submit a
> kernel-doc patch for the simple pipe helpers.
no, nothing wrong here, just see my reasoning above
>> +
>> + if (ret)
>> + DRM_ERROR("Failed to set mode to back: %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static void display_enable(struct drm_simple_display_pipe *pipe,
>> + struct drm_crtc_state *crtc_state)
>> +{
>> + struct drm_crtc *crtc = &pipe->crtc;
>> + struct drm_framebuffer *fb = pipe->plane.state->fb;
>> +
>> + if (display_set_config(pipe, fb) == 0)
>> + drm_crtc_vblank_on(crtc);
> I get the impression your driver doesn't support vblanks (the page flip
> code at least looks like it's only generating a single event),
yes, this is true
> you also
> don't have a enable/disable_vblank implementation.
this is because with my previous patches [1] these are now handled
by simple helpers, so no need to provide dummy ones in the driver
> If there's no vblank
> handling then this shouldn't be needed.
yes, I will rework the code, please see below
>> + else
>> + DRM_ERROR("Failed to enable display\n");
>> +}
>> +
>> +static void display_disable(struct drm_simple_display_pipe *pipe)
>> +{
>> + struct drm_crtc *crtc = &pipe->crtc;
>> +
>> + display_set_config(pipe, NULL);
>> + drm_crtc_vblank_off(crtc);
>> + /* final check for stalled events */
>> + if (crtc->state->event && !crtc->state->active) {
>> + unsigned long flags;
>> +
>> + spin_lock_irqsave(&crtc->dev->event_lock, flags);
>> + drm_crtc_send_vblank_event(crtc, crtc->state->event);
>> + spin_unlock_irqrestore(&crtc->dev->event_lock, flags);
>> + crtc->state->event = NULL;
>> + }
>> +}
>> +
>> +void xen_drm_front_kms_on_frame_done(
>> + struct xen_drm_front_drm_pipeline *pipeline,
>> + uint64_t fb_cookie)
>> +{
>> + drm_crtc_handle_vblank(&pipeline->pipe.crtc);
> Hm, again this doesn't look like real vblank, but only a page-flip done
> event. If that's correct then please don't use the vblank machinery, but
> just store the event internally (protected with your own private spinlock)
Why can't I use &dev->event_lock? Anyways for handling
page-flip events I will need to lock on it, so I can do
drm_crtc_send_vblank_event?
> and send it out using drm_crtc_send_vblank_event directly. No calls to
> arm_vblank_event or any of the other vblank infrastructure should be
> needed.
will re-work, e.g. will store drm_pending_vblank_event
on .display_update and send out on page flip event from the
backend
> Also please remove the drm_vblank_init() call, since your hw doesn't
> really have vblanks. And exposing vblanks to userspace without
> implementing them is confusing.
will remove all vblank handling at all with the re-work above
>
>> +}
>> +
>> +static void display_send_page_flip(struct drm_simple_display_pipe *pipe,
>> + struct drm_plane_state *old_plane_state)
>> +{
>> + struct drm_plane_state *plane_state = drm_atomic_get_new_plane_state(
>> + old_plane_state->state, &pipe->plane);
>> +
>> + /*
>> + * If old_plane_state->fb is NULL and plane_state->fb is not,
>> + * then this is an atomic commit which will enable display.
>> + * If old_plane_state->fb is not NULL and plane_state->fb is,
>> + * then this is an atomic commit which will disable display.
>> + * Ignore these and do not send page flip as this framebuffer will be
>> + * sent to the backend as a part of display_set_config call.
>> + */
>> + if (old_plane_state->fb && plane_state->fb) {
>> + struct xen_drm_front_drm_pipeline *pipeline =
>> + to_xen_drm_pipeline(pipe);
>> + struct xen_drm_front_drm_info *drm_info = pipeline->drm_info;
>> + int ret;
>> +
>> + ret = drm_info->front_ops->page_flip(drm_info->front_info,
>> + pipeline->index,
>> + xen_drm_front_fb_to_cookie(plane_state->fb));
>> + pipeline->pgflip_last_error = ret;
>> + if (ret) {
>> + DRM_ERROR("Failed to send page flip request to backend: %d\n", ret);
>> + /*
>> + * As we are at commit stage the DRM core will anyways
>> + * wait for the vblank and knows nothing about our
>> + * failure. The best we can do is to handle
>> + * vblank now, so there is no vblank/flip_done
>> + * time outs
>> + */
>> + drm_crtc_handle_vblank(&pipeline->pipe.crtc);
>> + }
>> + }
>> +}
>> +
>> +static int display_prepare_fb(struct drm_simple_display_pipe *pipe,
>> + struct drm_plane_state *plane_state)
>> +{
>> + struct xen_drm_front_drm_pipeline *pipeline =
>> + to_xen_drm_pipeline(pipe);
>> +
>> + if (pipeline->pgflip_last_error) {
>> + int ret;
>> +
>> + /* if previous page flip didn't succeed then report the error */
>> + ret = pipeline->pgflip_last_error;
>> + /* and let us try to page flip next time */
>> + pipeline->pgflip_last_error = 0;
>> + return ret;
>> + }
> Nope, this isn't how the uapi works. If your flips fail then we might need
> to add some error status thing to the drm events, but you can't make the
> next flip fail.
Well, yes, there is no way for me to tell that the page flip
has failed, so this is why I tried to do this workaround with
the next page-flip. The reason for that is that if, for example,
we are disconnected from the backend for some reason, there is
no way for me to tell the user-space that hey, please, do not
send any other page flips. If backend can recover and that was
a one time error then yes, the code I have will do wrong thing
(fail the current page flip), but if the error state is persistent
then I will be able to tell the user-space to stop by returning errors.
This is kind of trade-off which I am not sure how to solve correctly.

Do you think I can remove this workaround completely?
> -Daniel
>
>> + return drm_gem_fb_prepare_fb(&pipe->plane, plane_state);
>> +}
>> +
>> +static void display_update(struct drm_simple_display_pipe *pipe,
>> + struct drm_plane_state *old_plane_state)
>> +{
>> + struct drm_crtc *crtc = &pipe->crtc;
>> + struct drm_pending_vblank_event *event;
>> +
>> + event = crtc->state->event;
>> + if (event) {
>> + struct drm_device *dev = crtc->dev;
>> + unsigned long flags;
>> +
>> + crtc->state->event = NULL;
>> +
>> + spin_lock_irqsave(&dev->event_lock, flags);
>> + if (drm_crtc_vblank_get(crtc) == 0)
>> + drm_crtc_arm_vblank_event(crtc, event);
>> + else
>> + drm_crtc_send_vblank_event(crtc, event);
>> + spin_unlock_irqrestore(&dev->event_lock, flags);
>> + }
>> + /*
>> + * Send page flip request to the backend *after* we have event armed/
>> + * sent above, so on page flip done event from the backend we can
>> + * deliver it while handling vblank.
>> + */
>> + display_send_page_flip(pipe, old_plane_state);
>> +}
>> +
>> +static const struct drm_simple_display_pipe_funcs display_funcs = {
>> + .enable = display_enable,
>> + .disable = display_disable,
>> + .prepare_fb = display_prepare_fb,
>> + .update = display_update,
>> +};
>> +
>> +static int display_pipe_init(struct xen_drm_front_drm_info *drm_info,
>> + int index, struct xen_drm_front_cfg_connector *cfg,
>> + struct xen_drm_front_drm_pipeline *pipeline)
>> +{
>> + struct drm_device *dev = drm_info->drm_dev;
>> + const uint32_t *formats;
>> + int format_count;
>> + int ret;
>> +
>> + pipeline->drm_info = drm_info;
>> + pipeline->index = index;
>> + pipeline->height = cfg->height;
>> + pipeline->width = cfg->width;
>> +
>> + ret = xen_drm_front_conn_init(drm_info, &pipeline->conn);
>> + if (ret)
>> + return ret;
>> +
>> + formats = xen_drm_front_conn_get_formats(&format_count);
>> +
>> + return drm_simple_display_pipe_init(dev, &pipeline->pipe,
>> + &display_funcs, formats, format_count,
>> + NULL, &pipeline->conn);
>> +}
>> +
>> +int xen_drm_front_kms_init(struct xen_drm_front_drm_info *drm_info)
>> +{
>> + struct drm_device *dev = drm_info->drm_dev;
>> + int i, ret;
>> +
>> + drm_mode_config_init(dev);
>> +
>> + dev->mode_config.min_width = 0;
>> + dev->mode_config.min_height = 0;
>> + dev->mode_config.max_width = 4095;
>> + dev->mode_config.max_height = 2047;
>> + dev->mode_config.funcs = &mode_config_funcs;
>> +
>> + for (i = 0; i < drm_info->cfg->num_connectors; i++) {
>> + struct xen_drm_front_cfg_connector *cfg =
>> + &drm_info->cfg->connectors[i];
>> + struct xen_drm_front_drm_pipeline *pipeline =
>> + &drm_info->pipeline[i];
>> +
>> + ret = display_pipe_init(drm_info, i, cfg, pipeline);
>> + if (ret) {
>> + drm_mode_config_cleanup(dev);
>> + return ret;
>> + }
>> + }
>> +
>> + drm_mode_config_reset(dev);
>> + return 0;
>> +}
>> diff --git a/drivers/gpu/drm/xen/xen_drm_front_kms.h b/drivers/gpu/drm/xen/xen_drm_front_kms.h
>> new file mode 100644
>> index 000000000000..65a50033bb9b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xen/xen_drm_front_kms.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Xen para-virtual DRM device
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * Copyright (C) 2016-2018 EPAM Systems Inc.
>> + *
>> + * Author: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
>> + */
>> +
>> +#ifndef __XEN_DRM_FRONT_KMS_H_
>> +#define __XEN_DRM_FRONT_KMS_H_
>> +
>> +#include "xen_drm_front_drv.h"
>> +
>> +int xen_drm_front_kms_init(struct xen_drm_front_drm_info *drm_info);
>> +
>> +void xen_drm_front_kms_on_frame_done(
>> + struct xen_drm_front_drm_pipeline *pipeline,
>> + uint64_t fb_cookie);
>> +
>> +#endif /* __XEN_DRM_FRONT_KMS_H_ */
>> --
>> 2.7.4
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
[1] https://patchwork.kernel.org/patch/10211997/

\
 
 \ /
  Last update: 2018-03-05 13:59    [W:0.125 / U:1.928 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site