A bunch of fixes for vc4 fixing some coexistence issue between wifi and

HDMI, unsupported modes, and vblank timeouts, a fix for ast to reload
 the gamma LUT after changing the plane format and a double-free fix for
 nouveau
 -----BEGIN PGP SIGNATURE-----
 
 iHUEABYIAB0WIQRcEzekXsqa64kGDp7j7w1vZxhRxQUCX79tPAAKCRDj7w1vZxhR
 xdp6AP0Txa4ssBQzkXzPwxeoHIO4KvRNVG4N/s73UEfVr5ovFAEAlEyL4ku0BOlb
 Lyk9MdmhSMl/YRuWzqHK/N1wBk6M5Qk=
 =7635
 -----END PGP SIGNATURE-----

Merge tag 'drm-misc-fixes-2020-11-26' of ssh://git.freedesktop.org/git/drm/drm-misc into drm-fixes

A bunch of fixes for vc4 fixing some coexistence issue between wifi and
HDMI, unsupported modes, and vblank timeouts, a fix for ast to reload
the gamma LUT after changing the plane format and a double-free fix for
nouveau

Signed-off-by: Dave Airlie <airlied@redhat.com>

From: Maxime Ripard <maxime@cerno.tech>
Link: https://patchwork.freedesktop.org/patch/msgid/20201126085450.r3i7wvj7pizsa4l6@gilmour
This commit is contained in:
Dave Airlie 2020-11-27 09:39:39 +10:00
commit 9595930db4
7 changed files with 272 additions and 68 deletions

View File

@ -76,6 +76,12 @@ properties:
resets:
maxItems: 1
wifi-2.4ghz-coexistence:
type: boolean
description: >
Should the pixel frequencies in the WiFi frequencies range be
avoided?
required:
- compatible
- reg

View File

@ -742,7 +742,6 @@ static void ast_crtc_dpms(struct drm_crtc *crtc, int mode)
case DRM_MODE_DPMS_SUSPEND:
if (ast->tx_chip_type == AST_TX_DP501)
ast_set_dp501_video_output(crtc->dev, 1);
ast_crtc_load_lut(ast, crtc);
break;
case DRM_MODE_DPMS_OFF:
if (ast->tx_chip_type == AST_TX_DP501)
@ -777,6 +776,21 @@ static int ast_crtc_helper_atomic_check(struct drm_crtc *crtc,
return 0;
}
static void
ast_crtc_helper_atomic_flush(struct drm_crtc *crtc, struct drm_crtc_state *old_crtc_state)
{
struct ast_private *ast = to_ast_private(crtc->dev);
struct ast_crtc_state *ast_crtc_state = to_ast_crtc_state(crtc->state);
struct ast_crtc_state *old_ast_crtc_state = to_ast_crtc_state(old_crtc_state);
/*
* The gamma LUT has to be reloaded after changing the primary
* plane's color format.
*/
if (old_ast_crtc_state->format != ast_crtc_state->format)
ast_crtc_load_lut(ast, crtc);
}
static void
ast_crtc_helper_atomic_enable(struct drm_crtc *crtc,
struct drm_crtc_state *old_crtc_state)
@ -830,6 +844,7 @@ ast_crtc_helper_atomic_disable(struct drm_crtc *crtc,
static const struct drm_crtc_helper_funcs ast_crtc_helper_funcs = {
.atomic_check = ast_crtc_helper_atomic_check,
.atomic_flush = ast_crtc_helper_atomic_flush,
.atomic_enable = ast_crtc_helper_atomic_enable,
.atomic_disable = ast_crtc_helper_atomic_disable,
};

View File

@ -558,8 +558,10 @@ nouveau_gem_pushbuf_validate(struct nouveau_channel *chan,
NV_PRINTK(err, cli, "validating bo list\n");
validate_fini(op, chan, NULL, NULL);
return ret;
} else if (ret > 0) {
*apply_relocs = true;
}
*apply_relocs = ret;
return 0;
}
@ -662,7 +664,6 @@ nouveau_gem_pushbuf_reloc_apply(struct nouveau_cli *cli,
nouveau_bo_wr32(nvbo, r->reloc_bo_offset >> 2, data);
}
u_free(reloc);
return ret;
}
@ -872,9 +873,10 @@ out:
break;
}
}
u_free(reloc);
}
out_prevalid:
if (!IS_ERR(reloc))
u_free(reloc);
u_free(bo);
u_free(push);

View File

@ -219,6 +219,7 @@ struct vc4_dev {
struct drm_modeset_lock ctm_state_lock;
struct drm_private_obj ctm_manager;
struct drm_private_obj hvs_channels;
struct drm_private_obj load_tracker;
/* List of vc4_debugfs_info_entry for adding to debugfs once
@ -531,6 +532,9 @@ struct vc4_crtc_state {
unsigned int top;
unsigned int bottom;
} margins;
/* Transitional state below, only valid during atomic commits */
bool update_muxing;
};
#define VC4_HVS_CHANNEL_DISABLED ((unsigned int)-1)

View File

@ -760,12 +760,54 @@ static void vc4_hdmi_encoder_enable(struct drm_encoder *encoder)
{
}
#define WIFI_2_4GHz_CH1_MIN_FREQ 2400000000ULL
#define WIFI_2_4GHz_CH1_MAX_FREQ 2422000000ULL
static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder,
struct drm_crtc_state *crtc_state,
struct drm_connector_state *conn_state)
{
struct drm_display_mode *mode = &crtc_state->adjusted_mode;
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
unsigned long long pixel_rate = mode->clock * 1000;
unsigned long long tmds_rate;
if (vc4_hdmi->variant->unsupported_odd_h_timings &&
((mode->hdisplay % 2) || (mode->hsync_start % 2) ||
(mode->hsync_end % 2) || (mode->htotal % 2)))
return -EINVAL;
/*
* The 1440p@60 pixel rate is in the same range than the first
* WiFi channel (between 2.4GHz and 2.422GHz with 22MHz
* bandwidth). Slightly lower the frequency to bring it out of
* the WiFi range.
*/
tmds_rate = pixel_rate * 10;
if (vc4_hdmi->disable_wifi_frequencies &&
(tmds_rate >= WIFI_2_4GHz_CH1_MIN_FREQ &&
tmds_rate <= WIFI_2_4GHz_CH1_MAX_FREQ)) {
mode->clock = 238560;
pixel_rate = mode->clock * 1000;
}
if (pixel_rate > vc4_hdmi->variant->max_pixel_clock)
return -EINVAL;
return 0;
}
static enum drm_mode_status
vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder,
const struct drm_display_mode *mode)
{
struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder);
if (vc4_hdmi->variant->unsupported_odd_h_timings &&
((mode->hdisplay % 2) || (mode->hsync_start % 2) ||
(mode->hsync_end % 2) || (mode->htotal % 2)))
return MODE_H_ILLEGAL;
if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock)
return MODE_CLOCK_HIGH;
@ -773,6 +815,7 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder,
}
static const struct drm_encoder_helper_funcs vc4_hdmi_encoder_helper_funcs = {
.atomic_check = vc4_hdmi_encoder_atomic_check,
.mode_valid = vc4_hdmi_encoder_mode_valid,
.disable = vc4_hdmi_encoder_disable,
.enable = vc4_hdmi_encoder_enable,
@ -1694,6 +1737,9 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
vc4_hdmi->hpd_active_low = hpd_gpio_flags & OF_GPIO_ACTIVE_LOW;
}
vc4_hdmi->disable_wifi_frequencies =
of_property_read_bool(dev->of_node, "wifi-2.4ghz-coexistence");
pm_runtime_enable(dev);
drm_simple_encoder_init(drm, encoder, DRM_MODE_ENCODER_TMDS);
@ -1817,6 +1863,7 @@ static const struct vc4_hdmi_variant bcm2711_hdmi0_variant = {
PHY_LANE_2,
PHY_LANE_CK,
},
.unsupported_odd_h_timings = true,
.init_resources = vc5_hdmi_init_resources,
.csc_setup = vc5_hdmi_csc_setup,
@ -1842,6 +1889,7 @@ static const struct vc4_hdmi_variant bcm2711_hdmi1_variant = {
PHY_LANE_CK,
PHY_LANE_2,
},
.unsupported_odd_h_timings = true,
.init_resources = vc5_hdmi_init_resources,
.csc_setup = vc5_hdmi_csc_setup,

View File

@ -62,6 +62,9 @@ struct vc4_hdmi_variant {
*/
enum vc4_hdmi_phy_channel phy_lane_mapping[4];
/* The BCM2711 cannot deal with odd horizontal pixel timings */
bool unsupported_odd_h_timings;
/* Callback to get the resources (memory region, interrupts,
* clocks, etc) for that variant.
*/
@ -139,6 +142,14 @@ struct vc4_hdmi {
int hpd_gpio;
bool hpd_active_low;
/*
* On some systems (like the RPi4), some modes are in the same
* frequency range than the WiFi channels (1440p@60Hz for
* example). Should we take evasive actions because that system
* has a wifi adapter?
*/
bool disable_wifi_frequencies;
struct cec_adapter *cec_adap;
struct cec_msg cec_rx_msg;
bool cec_tx_ok;

View File

@ -24,6 +24,8 @@
#include "vc4_drv.h"
#include "vc4_regs.h"
#define HVS_NUM_CHANNELS 3
struct vc4_ctm_state {
struct drm_private_state base;
struct drm_color_ctm *ctm;
@ -35,6 +37,17 @@ static struct vc4_ctm_state *to_vc4_ctm_state(struct drm_private_state *priv)
return container_of(priv, struct vc4_ctm_state, base);
}
struct vc4_hvs_state {
struct drm_private_state base;
unsigned int unassigned_channels;
};
static struct vc4_hvs_state *
to_vc4_hvs_state(struct drm_private_state *priv)
{
return container_of(priv, struct vc4_hvs_state, base);
}
struct vc4_load_tracker_state {
struct drm_private_state base;
u64 hvs_load;
@ -113,7 +126,7 @@ static int vc4_ctm_obj_init(struct vc4_dev *vc4)
drm_atomic_private_obj_init(&vc4->base, &vc4->ctm_manager, &ctm_state->base,
&vc4_ctm_state_funcs);
return drmm_add_action(&vc4->base, vc4_ctm_obj_fini, NULL);
return drmm_add_action_or_reset(&vc4->base, vc4_ctm_obj_fini, NULL);
}
/* Converts a DRM S31.32 value to the HW S0.9 format. */
@ -169,6 +182,19 @@ vc4_ctm_commit(struct vc4_dev *vc4, struct drm_atomic_state *state)
VC4_SET_FIELD(ctm_state->fifo, SCALER_OLEDOFFS_DISPFIFO));
}
static struct vc4_hvs_state *
vc4_hvs_get_global_state(struct drm_atomic_state *state)
{
struct vc4_dev *vc4 = to_vc4_dev(state->dev);
struct drm_private_state *priv_state;
priv_state = drm_atomic_get_private_obj_state(state, &vc4->hvs_channels);
if (IS_ERR(priv_state))
return ERR_CAST(priv_state);
return to_vc4_hvs_state(priv_state);
}
static void vc4_hvs_pv_muxing_commit(struct vc4_dev *vc4,
struct drm_atomic_state *state)
{
@ -213,10 +239,7 @@ static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4,
{
struct drm_crtc_state *crtc_state;
struct drm_crtc *crtc;
unsigned char dsp2_mux = 0;
unsigned char dsp3_mux = 3;
unsigned char dsp4_mux = 3;
unsigned char dsp5_mux = 3;
unsigned char mux;
unsigned int i;
u32 reg;
@ -224,50 +247,59 @@ static void vc5_hvs_pv_muxing_commit(struct vc4_dev *vc4,
struct vc4_crtc_state *vc4_state = to_vc4_crtc_state(crtc_state);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
if (!crtc_state->active)
if (!vc4_state->update_muxing)
continue;
switch (vc4_crtc->data->hvs_output) {
case 2:
dsp2_mux = (vc4_state->assigned_channel == 2) ? 0 : 1;
mux = (vc4_state->assigned_channel == 2) ? 0 : 1;
reg = HVS_READ(SCALER_DISPECTRL);
HVS_WRITE(SCALER_DISPECTRL,
(reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) |
VC4_SET_FIELD(mux, SCALER_DISPECTRL_DSP2_MUX));
break;
case 3:
dsp3_mux = vc4_state->assigned_channel;
if (vc4_state->assigned_channel == VC4_HVS_CHANNEL_DISABLED)
mux = 3;
else
mux = vc4_state->assigned_channel;
reg = HVS_READ(SCALER_DISPCTRL);
HVS_WRITE(SCALER_DISPCTRL,
(reg & ~SCALER_DISPCTRL_DSP3_MUX_MASK) |
VC4_SET_FIELD(mux, SCALER_DISPCTRL_DSP3_MUX));
break;
case 4:
dsp4_mux = vc4_state->assigned_channel;
if (vc4_state->assigned_channel == VC4_HVS_CHANNEL_DISABLED)
mux = 3;
else
mux = vc4_state->assigned_channel;
reg = HVS_READ(SCALER_DISPEOLN);
HVS_WRITE(SCALER_DISPEOLN,
(reg & ~SCALER_DISPEOLN_DSP4_MUX_MASK) |
VC4_SET_FIELD(mux, SCALER_DISPEOLN_DSP4_MUX));
break;
case 5:
dsp5_mux = vc4_state->assigned_channel;
if (vc4_state->assigned_channel == VC4_HVS_CHANNEL_DISABLED)
mux = 3;
else
mux = vc4_state->assigned_channel;
reg = HVS_READ(SCALER_DISPDITHER);
HVS_WRITE(SCALER_DISPDITHER,
(reg & ~SCALER_DISPDITHER_DSP5_MUX_MASK) |
VC4_SET_FIELD(mux, SCALER_DISPDITHER_DSP5_MUX));
break;
default:
break;
}
}
reg = HVS_READ(SCALER_DISPECTRL);
HVS_WRITE(SCALER_DISPECTRL,
(reg & ~SCALER_DISPECTRL_DSP2_MUX_MASK) |
VC4_SET_FIELD(dsp2_mux, SCALER_DISPECTRL_DSP2_MUX));
reg = HVS_READ(SCALER_DISPCTRL);
HVS_WRITE(SCALER_DISPCTRL,
(reg & ~SCALER_DISPCTRL_DSP3_MUX_MASK) |
VC4_SET_FIELD(dsp3_mux, SCALER_DISPCTRL_DSP3_MUX));
reg = HVS_READ(SCALER_DISPEOLN);
HVS_WRITE(SCALER_DISPEOLN,
(reg & ~SCALER_DISPEOLN_DSP4_MUX_MASK) |
VC4_SET_FIELD(dsp4_mux, SCALER_DISPEOLN_DSP4_MUX));
reg = HVS_READ(SCALER_DISPDITHER);
HVS_WRITE(SCALER_DISPDITHER,
(reg & ~SCALER_DISPDITHER_DSP5_MUX_MASK) |
VC4_SET_FIELD(dsp5_mux, SCALER_DISPDITHER_DSP5_MUX));
}
static void
@ -657,53 +689,123 @@ static int vc4_load_tracker_obj_init(struct vc4_dev *vc4)
&load_state->base,
&vc4_load_tracker_state_funcs);
return drmm_add_action(&vc4->base, vc4_load_tracker_obj_fini, NULL);
return drmm_add_action_or_reset(&vc4->base, vc4_load_tracker_obj_fini, NULL);
}
#define NUM_OUTPUTS 6
#define NUM_CHANNELS 3
static int
vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
static struct drm_private_state *
vc4_hvs_channels_duplicate_state(struct drm_private_obj *obj)
{
unsigned long unassigned_channels = GENMASK(NUM_CHANNELS - 1, 0);
struct vc4_hvs_state *old_state = to_vc4_hvs_state(obj->state);
struct vc4_hvs_state *state;
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state)
return NULL;
__drm_atomic_helper_private_obj_duplicate_state(obj, &state->base);
state->unassigned_channels = old_state->unassigned_channels;
return &state->base;
}
static void vc4_hvs_channels_destroy_state(struct drm_private_obj *obj,
struct drm_private_state *state)
{
struct vc4_hvs_state *hvs_state = to_vc4_hvs_state(state);
kfree(hvs_state);
}
static const struct drm_private_state_funcs vc4_hvs_state_funcs = {
.atomic_duplicate_state = vc4_hvs_channels_duplicate_state,
.atomic_destroy_state = vc4_hvs_channels_destroy_state,
};
static void vc4_hvs_channels_obj_fini(struct drm_device *dev, void *unused)
{
struct vc4_dev *vc4 = to_vc4_dev(dev);
drm_atomic_private_obj_fini(&vc4->hvs_channels);
}
static int vc4_hvs_channels_obj_init(struct vc4_dev *vc4)
{
struct vc4_hvs_state *state;
state = kzalloc(sizeof(*state), GFP_KERNEL);
if (!state)
return -ENOMEM;
state->unassigned_channels = GENMASK(HVS_NUM_CHANNELS - 1, 0);
drm_atomic_private_obj_init(&vc4->base, &vc4->hvs_channels,
&state->base,
&vc4_hvs_state_funcs);
return drmm_add_action_or_reset(&vc4->base, vc4_hvs_channels_obj_fini, NULL);
}
/*
* The BCM2711 HVS has up to 7 outputs connected to the pixelvalves and
* the TXP (and therefore all the CRTCs found on that platform).
*
* The naive (and our initial) implementation would just iterate over
* all the active CRTCs, try to find a suitable FIFO, and then remove it
* from the pool of available FIFOs. However, there are a few corner
* cases that need to be considered:
*
* - When running in a dual-display setup (so with two CRTCs involved),
* we can update the state of a single CRTC (for example by changing
* its mode using xrandr under X11) without affecting the other. In
* this case, the other CRTC wouldn't be in the state at all, so we
* need to consider all the running CRTCs in the DRM device to assign
* a FIFO, not just the one in the state.
*
* - To fix the above, we can't use drm_atomic_get_crtc_state on all
* enabled CRTCs to pull their CRTC state into the global state, since
* a page flip would start considering their vblank to complete. Since
* we don't have a guarantee that they are actually active, that
* vblank might never happen, and shouldn't even be considered if we
* want to do a page flip on a single CRTC. That can be tested by
* doing a modetest -v first on HDMI1 and then on HDMI0.
*
* - Since we need the pixelvalve to be disabled and enabled back when
* the FIFO is changed, we should keep the FIFO assigned for as long
* as the CRTC is enabled, only considering it free again once that
* CRTC has been disabled. This can be tested by booting X11 on a
* single display, and changing the resolution down and then back up.
*/
static int vc4_pv_muxing_atomic_check(struct drm_device *dev,
struct drm_atomic_state *state)
{
struct vc4_hvs_state *hvs_new_state;
struct drm_crtc_state *old_crtc_state, *new_crtc_state;
struct drm_crtc *crtc;
int i, ret;
unsigned int i;
/*
* Since the HVS FIFOs are shared across all the pixelvalves and
* the TXP (and thus all the CRTCs), we need to pull the current
* state of all the enabled CRTCs so that an update to a single
* CRTC still keeps the previous FIFOs enabled and assigned to
* the same CRTCs, instead of evaluating only the CRTC being
* modified.
*/
list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
struct drm_crtc_state *crtc_state;
if (!crtc->state->enable)
continue;
crtc_state = drm_atomic_get_crtc_state(state, crtc);
if (IS_ERR(crtc_state))
return PTR_ERR(crtc_state);
}
hvs_new_state = vc4_hvs_get_global_state(state);
if (!hvs_new_state)
return -EINVAL;
for_each_oldnew_crtc_in_state(state, crtc, old_crtc_state, new_crtc_state, i) {
struct vc4_crtc_state *old_vc4_crtc_state =
to_vc4_crtc_state(old_crtc_state);
struct vc4_crtc_state *new_vc4_crtc_state =
to_vc4_crtc_state(new_crtc_state);
struct vc4_crtc *vc4_crtc = to_vc4_crtc(crtc);
unsigned int matching_channels;
if (old_crtc_state->enable && !new_crtc_state->enable)
new_vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
if (!new_crtc_state->enable)
/* Nothing to do here, let's skip it */
if (old_crtc_state->enable == new_crtc_state->enable)
continue;
if (new_vc4_crtc_state->assigned_channel != VC4_HVS_CHANNEL_DISABLED) {
unassigned_channels &= ~BIT(new_vc4_crtc_state->assigned_channel);
/* Muxing will need to be modified, mark it as such */
new_vc4_crtc_state->update_muxing = true;
/* If we're disabling our CRTC, we put back our channel */
if (!new_crtc_state->enable) {
hvs_new_state->unassigned_channels |= BIT(old_vc4_crtc_state->assigned_channel);
new_vc4_crtc_state->assigned_channel = VC4_HVS_CHANNEL_DISABLED;
continue;
}
@ -731,17 +833,29 @@ vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
* the future, we will need to have something smarter,
* but it works so far.
*/
matching_channels = unassigned_channels & vc4_crtc->data->hvs_available_channels;
matching_channels = hvs_new_state->unassigned_channels & vc4_crtc->data->hvs_available_channels;
if (matching_channels) {
unsigned int channel = ffs(matching_channels) - 1;
new_vc4_crtc_state->assigned_channel = channel;
unassigned_channels &= ~BIT(channel);
hvs_new_state->unassigned_channels &= ~BIT(channel);
} else {
return -EINVAL;
}
}
return 0;
}
static int
vc4_atomic_check(struct drm_device *dev, struct drm_atomic_state *state)
{
int ret;
ret = vc4_pv_muxing_atomic_check(dev, state);
if (ret)
return ret;
ret = vc4_ctm_atomic_check(dev, state);
if (ret < 0)
return ret;
@ -808,6 +922,10 @@ int vc4_kms_load(struct drm_device *dev)
if (ret)
return ret;
ret = vc4_hvs_channels_obj_init(vc4);
if (ret)
return ret;
drm_mode_config_reset(dev);
drm_kms_helper_poll_init(dev);