Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions drivers/gpu/drm/apple/apple_drv.c
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,10 @@ static int apple_probe_per_dcp(struct device *dev,
if (ret)
return ret;

ret = drm_connector_attach_vrr_capable_property(&connector->base);
if (ret)
return ret;

connector->base.polled = DRM_CONNECTOR_POLL_HPD;
connector->connected = false;
connector->dcp = dcp;
Expand Down
1 change: 1 addition & 0 deletions drivers/gpu/drm/apple/dcp-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ struct apple_dcp {
bool during_modeset;
bool valid_mode;
bool use_timestamps;
bool vrr_enabled;
struct dcp_set_digital_out_mode_req mode;

/* completion for active turning true */
Expand Down
9 changes: 9 additions & 0 deletions drivers/gpu/drm/apple/dcp.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ static bool unstable_edid = true;
module_param(unstable_edid, bool, 0644);
MODULE_PARM_DESC(unstable_edid, "Enable unstable EDID retrival support");

bool vrr;
module_param(vrr, bool, 0644);
MODULE_PARM_DESC(vrr, "Enable Adaptive Sync/ProMotion on supported displays");

/* copied and simplified from drm_vblank.c */
static void send_vblank_event(struct drm_device *dev,
struct drm_pending_vblank_event *e,
Expand Down Expand Up @@ -361,6 +365,11 @@ int dcp_crtc_atomic_check(struct drm_crtc *crtc, struct drm_atomic_state *state)
return -EINVAL;
}

if (dcp->vrr_enabled != crtc_state->vrr_enabled) {
dcp->vrr_enabled = crtc_state->vrr_enabled;
crtc_state->mode_changed = true;
}

return 0;
}

Expand Down
1 change: 1 addition & 0 deletions drivers/gpu/drm/apple/iomfb.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ void dcp_hotplug(struct work_struct *work)

if (!connector->connected) {
drm_edid_free(connector->drm_edid);
drm_connector_set_vrr_capable_property(&connector->base, false);
connector->drm_edid = NULL;
}

Expand Down
9 changes: 9 additions & 0 deletions drivers/gpu/drm/apple/iomfb.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,15 @@ enum dcpep_type {
IOMFB_MESSAGE_TYPE_MSG = 2,
};

/*
* IOMFB supports the setting of a number of parameters
* that alter various aspects of the connected sink's
* behaviour at runtime.
*/
enum iomfb_parameter {
IOMFBPARAM_ADAPTIVE_SYNC = 14,
};

#define IOMFB_MESSAGE_TYPE GENMASK_ULL( 3, 0)

/* Message */
Expand Down
104 changes: 69 additions & 35 deletions drivers/gpu/drm/apple/iomfb_template.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
/* Register defines used in bandwidth setup structure */
#define REG_DOORBELL_BIT(idx) (2 + (idx))

extern bool vrr;

struct dcp_wait_cookie {
struct kref refcount;
struct completion done;
Expand Down Expand Up @@ -546,8 +548,9 @@ static u8 dcpep_cb_prop_chunk(struct apple_dcp *dcp,
static bool dcpep_process_chunks(struct apple_dcp *dcp,
struct dcp_set_dcpav_prop_end_req *req)
{
// struct apple_connector *connector = dcp->connector;
struct dcp_parse_ctx ctx;
int ret;
int ret; //, i;

if (!dcp->chunks.data) {
dev_warn(dcp->dev, "ignoring spurious end\n");
Expand Down Expand Up @@ -589,6 +592,15 @@ static bool dcpep_process_chunks(struct apple_dcp *dcp,
dcp_set_dimensions(dcp);
}

// if (connector) {
// for (i = 0; i < dcp->nr_modes; i++) {
// if (dcp->modes[i].vrr) {
// drm_connector_set_vrr_capable_property(&connector->base, true);
// break;
// }
// }
// }

return true;
}

Expand Down Expand Up @@ -784,26 +796,11 @@ static void dcp_on_set_power_state(struct apple_dcp *dcp, void *out, void *cooki
dcp_set_power_state(dcp, false, &req, dcp_on_final, cookie);
}

static void dcp_on_set_parameter(struct apple_dcp *dcp, void *out, void *cookie)
{
struct dcp_set_parameter_dcp param = {
.param = 14,
.value = { 0 },
#if DCP_FW_VER >= DCP_FW_VERSION(13, 2, 0)
.count = 3,
#else
.count = 1,
#endif
};

dcp_set_parameter_dcp(dcp, false, &param, dcp_on_set_power_state, cookie);
}

void DCP_FW_NAME(iomfb_poweron)(struct apple_dcp *dcp)
{
struct dcp_wait_cookie *cookie;
int ret;
u32 handle;
u32 handle = 0;
dev_info(dcp->dev, "dcp_poweron() starting\n");

cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
Expand All @@ -815,15 +812,12 @@ void DCP_FW_NAME(iomfb_poweron)(struct apple_dcp *dcp)
/* increase refcount to ensure the receiver has a reference */
kref_get(&cookie->refcount);

if (dcp->main_display) {
handle = 0;
dcp_set_display_device(dcp, false, &handle, dcp_on_set_power_state,
cookie);
} else {
if (!dcp->main_display)
handle = 2;
dcp_set_display_device(dcp, false, &handle,
dcp_on_set_parameter, cookie);
}

dcp_set_display_device(dcp, false, &handle, dcp_on_set_power_state,
cookie);

ret = wait_for_completion_timeout(&cookie->done, msecs_to_jiffies(10000));

if (ret == 0) {
Expand Down Expand Up @@ -1190,6 +1184,33 @@ static void complete_set_digital_out_mode(struct apple_dcp *dcp, void *data,
}
}

/* Changes to Adaptive Sync require a trip through set_digital_out_mode */
static void dcp_on_set_adaptive_sync(struct apple_dcp *dcp, void *out, void *cookie)
{
dcp_set_digital_out_mode(dcp, false, &dcp->mode,
complete_set_digital_out_mode, cookie);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is unfortunately a full modeset. I initially assumed it wouldn't trigger full modeset like when trying to set the current mode without changing the minimal refresh rate.
It is even on macOS a full modeset both with USB-C/DP and HDMI (on M2 Pro) so I fear we can't make it work.
What we could possibly do is add a force_vrr module parameter and always set minRR when setting a VRR mode.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had an old implementation that set mode_changed and forced a full modeset. I will go back to that in the first instance if we can't get minRR to do anything without a modeset, and then add the module parameter to force VRR on at all times, ignoring userspace. I wonder if this has improved at all with newer firmware...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to force a modeset on VRR_ENABLED causes something to lock up, but since this is not expected to work anyway...

}

static void dcp_set_adaptive_sync(struct apple_dcp *dcp, u64 rate, void *cookie)
{
struct dcp_set_parameter_dcp param = {
.param = IOMFBPARAM_ADAPTIVE_SYNC,
.value = {
rate & 0xffffffff, /* minRR */
0, /* mediaTargetRate */
0, /* Fractional Rate (?) */
0, /* unused */
},
#if DCP_FW_VER >= DCP_FW_VERSION(13, 2, 0)
.count = 3,
#else
.count = 1,
#endif
};

dcp_set_parameter_dcp(dcp, false, &param, dcp_on_set_adaptive_sync, cookie);
}

int DCP_FW_NAME(iomfb_modeset)(struct apple_dcp *dcp,
struct drm_crtc_state *crtc_state)
{
Expand Down Expand Up @@ -1229,8 +1250,8 @@ int DCP_FW_NAME(iomfb_modeset)(struct apple_dcp *dcp,
.timing_mode_id = mode->timing_mode_id
};

/* Keep track of suspected vrr modes */
dcp->use_timestamps = mode->vrr;
/* Use DCP swap timestamps on MacBook Pros with VRR */
dcp->use_timestamps = mode->vrr && dcp->main_display;

cookie = kzalloc(sizeof(*cookie), GFP_KERNEL);
if (!cookie) {
Expand All @@ -1244,8 +1265,11 @@ int DCP_FW_NAME(iomfb_modeset)(struct apple_dcp *dcp,

dcp->during_modeset = true;

dcp_set_digital_out_mode(dcp, false, &dcp->mode,
complete_set_digital_out_mode, cookie);
if (mode->vrr && vrr)
dcp_set_adaptive_sync(dcp, mode->min_vrr, cookie);
else
dcp_set_digital_out_mode(dcp, false, &dcp->mode,
complete_set_digital_out_mode, cookie);

/*
* The DCP firmware has an internal timeout of ~8 seconds for
Expand Down Expand Up @@ -1277,6 +1301,15 @@ int DCP_FW_NAME(iomfb_modeset)(struct apple_dcp *dcp,
return 0;
}

/*
* DCP timestamps are expressed in system timer ticks. Approximate
* this by converting from ktime nanoseconds to 24 MHz ticks.
*/
static u64 ns_to_mach(u64 ns)
{
return ns * 3 / 125;
}

void DCP_FW_NAME(iomfb_flush)(struct apple_dcp *dcp, struct drm_crtc *crtc, struct drm_atomic_state *state)
{
struct drm_plane *plane;
Expand Down Expand Up @@ -1391,14 +1424,15 @@ void DCP_FW_NAME(iomfb_flush)(struct apple_dcp *dcp, struct drm_crtc *crtc, stru
req->clear = 1;
}

if (has_surface && dcp->use_timestamps) {
if (has_surface && (dcp->use_timestamps || crtc_state->vrr_enabled || vrr)) {
/*
* Fake timstamps to get 120hz refresh rate. It looks
* like the actual value does not matter, as long as it is non zero.
* TODO: ascertain with certainty what these timestamps
* are. These names are guesses based on what macOS populates
* them with. These values seem to work well with VRR.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that these names make sense. Since setting them all to the same value enabled 120 Hz refresh rate on the ProMotion display my guess is that of the timestamps is the time base (could be submit time). The other two could intended/earliest display time and latest display time.

*/
req->swap.ts1 = 120;
req->swap.ts2 = 120;
req->swap.ts3 = 120;
req->swap.presentation_time = ns_to_mach(ktime_get_ns());
req->swap.last_pres_time = ns_to_mach(ktime_to_ns(dcp->swap_start));
req->swap.submit_time = req->swap.presentation_time;
}

/* These fields should be set together */
Expand Down
6 changes: 3 additions & 3 deletions drivers/gpu/drm/apple/iomfb_template.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@
#include "version_utils.h"

struct DCP_FW_NAME(dcp_swap) {
u64 ts1;
u64 ts2;
u64 presentation_time;
u64 last_pres_time;

u64 unk_10;
u64 unk_18;
u64 ts64_unk;
u64 unk_28;
u64 ts3;
u64 submit_time;
u64 unk_38;

u64 flags1;
Expand Down
17 changes: 13 additions & 4 deletions drivers/gpu/drm/apple/parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,10 @@ static int parse_mode(struct dcp_parse_ctx *handle,
ret = parse_dimension(it.handle, &horiz);
else if (!strcmp(key, "VerticalAttributes"))
ret = parse_dimension(it.handle, &vert);
else if (!strcmp(key, "MinimumVariableRefreshRate"))
ret = parse_int(it.handle, &out->min_vrr);
else if (!strcmp(key, "MaximumVariableRefreshRate"))
ret = parse_int(it.handle, &out->max_vrr);
else if (!strcmp(key, "ColorModes"))
ret = parse_color_modes(it.handle, out);
else if (!strcmp(key, "ID"))
Expand Down Expand Up @@ -502,13 +506,18 @@ static int parse_mode(struct dcp_parse_ctx *handle,
/*
* HACK:
* Mark the 120 Hz mode on j314/j316 (identified by resolution) as vrr.
* We still do not know how to drive VRR but at least seetinng timestamps
* in the the swap_surface message to non-zero values drives the display
* at 120 fps.
* Setting timestamps in the the swap_surface message to non-zero
* values drives the display at 120 fps.
*/
if (vert.precise_sync_rate >> 16 == 120 &&
((horiz.active == 3024 && vert.active == 1964) ||
(horiz.active == 3456 && vert.active == 2234)))
(horiz.active == 3456 && vert.active == 2234))) {
out->min_vrr = 24 << 16;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the minimal refresh rate for ProMotion was was 10 Hz

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only on the iPhones and new iPad Pros apparently. There's scant concrete information available for the MacBooks, but Wikipedia lists 24 Hz and this matches what I've experienced. Setting it any lower results in stuttering.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apple documents 24 Hz as minimum for iPad Pros so it's probably the same for macbooks.

out->max_vrr = 120 << 16;
out->vrr = true;
}

if (out->min_vrr && out->max_vrr)
out->vrr = true;

vert.active -= notch_height;
Expand Down
2 changes: 2 additions & 0 deletions drivers/gpu/drm/apple/parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ struct dcp_display_mode {
struct dcp_color_mode sdr;
struct dcp_color_mode best;
bool vrr;
s64 min_vrr;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

32 bit should be enough and I assume this should be an unsigned value

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are expressed in TimingElements as 64-bit signed integers, so it is simpler to just store them the same way.

s64 max_vrr;
};

struct dimension {
Expand Down